From d6d59bae49fda9f13371625655174327badd5d37 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 29 Jun 2026 17:58:31 +0500 Subject: [PATCH 1/4] feat(cli): --verbose + cluster doctor auth checks + resume hint & install log (cli#101) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC-0001 §8.5 — make the zero-prompt connect flow's FAILURE path stand on its own (the installer-side streaming stays with backend#838, per the ticket). - `--verbose` / $TRACEBLOC_LOG_LEVEL: a root persistent flag that streams the device-flow → provision detail via a new verbose-gated ui.Detailf. Default output stays quiet (~the usual handful of ✔ lines). - `cluster doctor` now runs an "Auth & config" section FIRST — before the cluster checks, so it works even when no cluster is reachable (the failed-provision case): signed in? which env + account? active client set? plus a live token check (WhoAmI) — 401 → ✖ re-login, network error → ⚠. Its status folds into the overall verdict. - `client create` failure prints the exact, idempotent resume command (§7.2) + a `tracebloc cluster doctor` pointer, so a broken headless connect isn't a dead end. - Every `client create` run writes ~/.tracebloc/install-.log (0600) — a full trace on disk even when the terminal stayed quiet. Tests: ui Detailf gating; doctor auth (not-signed-in / valid / 401 / no-active-client); verbose-streams vs quiet-default; provision failure → resume command + install log. Closes #101. Co-Authored-By: Claude Opus 4.8 --- internal/cli/auth.go | 2 + internal/cli/client.go | 65 +++++++++++++++- internal/cli/doctor.go | 97 +++++++++++++++++++++-- internal/cli/doctor_test.go | 97 +++++++++++++++++++++++ internal/cli/installlog.go | 60 +++++++++++++++ internal/cli/root.go | 29 ++++++- internal/cli/verbose_install_test.go | 111 +++++++++++++++++++++++++++ internal/ui/ui.go | 27 ++++++- internal/ui/verbose_test.go | 27 +++++++ 9 files changed, 504 insertions(+), 11 deletions(-) create mode 100644 internal/cli/doctor_test.go create mode 100644 internal/cli/installlog.go create mode 100644 internal/cli/verbose_install_test.go create mode 100644 internal/ui/verbose_test.go diff --git a/internal/cli/auth.go b/internal/cli/auth.go index 0e414b1c..b30deb71 100644 --- a/internal/cli/auth.go +++ b/internal/cli/auth.go @@ -57,6 +57,7 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { } env := api.ResolveEnv(envFlag) client := newAPIClient(env) + p.Detailf("backend %s — requesting a device code …", client.BaseURL) dc, err := client.RequestDeviceCode(ctx) if err != nil { @@ -112,6 +113,7 @@ func runLogin(ctx context.Context, p *ui.Printer, envFlag string) error { // capture the account to show + store. Best-effort: don't fail a // successful sign-in just because this lookup couldn't run. client.Token = tok + p.Detailf("authorized — confirming the token with the backend …") if id, werr := client.WhoAmI(ctx); werr == nil { prof.Email = id.Email } diff --git a/internal/cli/client.go b/internal/cli/client.go index 9b3db184..815e2d45 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -130,11 +130,36 @@ func authedClient() (*api.Client, *config.Config, error) { return client, cfg, nil } -func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clientCreateOpts) error { +func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clientCreateOpts) (err error) { + // Always leave a full provision trace on disk, even on a quiet/headless run + // (RFC-0001 §8.5). On any failure, point at the (idempotent) resume command + // + `cluster doctor`, so a zero-prompt connect that breaks isn't a dead end. + ilog, logPath := newInstallLog() + defer ilog.Close() + ilog.Logf("client create: name=%q location=%q", opts.name, opts.location) + defer func() { + if err != nil { + ilog.Logf("FAILED: %v", err) + p.Newline() + p.Hintf("Provisioning didn't complete. Re-running is safe — on the same cluster it adopts the existing client instead of minting a duplicate (idempotent):") + p.Hintf(" %s", resumeCommand(opts)) + p.Hintf("Diagnose auth / cluster problems with: tracebloc cluster doctor") + if logPath != "" { + p.Hintf("Full log: %s", logPath) + } + return + } + ilog.Logf("done") + if logPath != "" { + p.Detailf("full log: %s", logPath) + } + }() + client, cfg, err := authedClient() if err != nil { return &exitError{code: 1, err: err} } + ilog.Logf("authenticated; provisioning against the signed-in account") name, location := opts.name, opts.location @@ -168,6 +193,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien if cidErr != nil { p.Hintf("Couldn't read the target cluster's identity — provisioning without a cluster anchor, so re-running won't be idempotent. Point --kubeconfig/--context at the reachable cluster to enable that.") } + ilog.Logf("cluster anchor: %q (read err: %v)", clusterID, cidErr) // Derive the namespace slug from the name, avoiding collisions with existing // clients (best-effort: if the list call fails we still derive a base slug). @@ -241,6 +267,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien // where the backend instead matches a live in-cluster TB_CLIENT_ID whose // cluster_id is still null and the CLI backfills it via PATCH, is the // installer's orchestration — #838 — not done here.) + ilog.Logf("adopted existing client id=%d namespace=%s", pc.ID, pc.Namespace) p.Successf("This cluster is already registered as client %q (namespace %s) — adopted it.", pc.Name, pc.Namespace) p.Hintf("No new credential issued; the existing one stands. This machine is set to enroll as client %d.", pc.ID) if opts.credentialFile != "" { @@ -287,12 +314,48 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien p.Field("username", pc.Username) p.Field("password", password) } + ilog.Logf("minted client id=%d namespace=%s", pc.ID, pc.Namespace) if serr := cfg.Save(); serr != nil { p.Hintf("Couldn't save the active-client pointer (%v) — run `tracebloc client use %d` to set it.", serr, pc.ID) } return nil } +// resumeCommand reconstructs the `tracebloc client create` invocation to retry a +// failed provision. Re-running is idempotent (RFC-0001 §7.2): on the same cluster +// it adopts the existing client rather than minting a duplicate. +func resumeCommand(opts clientCreateOpts) string { + parts := []string{"tracebloc client create"} + if opts.name != "" { + parts = append(parts, "--name "+shellArg(opts.name)) + } + if opts.location != "" { + parts = append(parts, "--location "+shellArg(opts.location)) + } + if opts.kubeconfigPath != "" { + parts = append(parts, "--kubeconfig "+shellArg(opts.kubeconfigPath)) + } + if opts.contextOverride != "" { + parts = append(parts, "--context "+shellArg(opts.contextOverride)) + } + if opts.credentialFile != "" { + parts = append(parts, "--credential-file "+shellArg(opts.credentialFile)) + } + if opts.yes { + parts = append(parts, "--yes") + } + return strings.Join(parts, " ") +} + +// shellArg single-quotes an argument containing whitespace so the resume command +// stays copy-pasteable for values like "Lab One". +func shellArg(s string) string { + if strings.ContainsAny(s, " \t") { + return "'" + s + "'" + } + return s +} + // writeClientCredential writes the machine credential to path (mode 0600) as a // shell-sourceable env file — the installer (#838) sources it to feed the chart, // so the secret lands in a 0600 file, never the terminal (RFC §9 never-show). The diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 71a484fb..db1c5348 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -2,11 +2,14 @@ package cli import ( "context" - "fmt" + "errors" + "net/http" "github.com/spf13/cobra" + "github.com/tracebloc/cli/internal/api" "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" "github.com/tracebloc/cli/internal/doctor" "github.com/tracebloc/cli/internal/ui" ) @@ -75,19 +78,30 @@ func runClusterDoctor( ) error { p.Banner("tracebloc", "cluster doctor") + // Auth / config checks run FIRST and don't need a cluster — so `doctor` can + // diagnose a failed provision (bad/expired token, wrong env, no active + // client) even before any cluster is reachable (RFC-0001 §8.5). + authStatus := runAuthChecks(ctx, p) + resolved, err := cluster.Load(cluster.KubeconfigOptions{ Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride, }) if err != nil { - // 3 = kubeconfig file/parse problem (same class as cluster info). - return &exitError{code: 3, err: fmt.Errorf("loading kubeconfig: %w", err)} + // 3 = kubeconfig file/parse problem (same class as cluster info). The + // auth section above already ran, so a kubeconfig issue doesn't hide it. + p.Section("Cluster") + p.Errorf("Kubeconfig — couldn't load it: %v", err) + p.Hintf(" point --kubeconfig / --context at your cluster, or fix ~/.kube/config") + return &exitError{code: 3, err: nil} } cs, err := cluster.NewClientset(resolved) if err != nil { - return &exitError{code: 3, err: err} + p.Section("Cluster") + p.Errorf("Kubeconfig — %v", err) + return &exitError{code: 3, err: nil} } p.Section("Kubeconfig") @@ -116,7 +130,9 @@ func runClusterDoctor( } p.Newline() - switch doctor.Worst(results) { + // Overall verdict folds in the auth section, so an auth ✖/⚠ counts even when + // the cluster itself is healthy. + switch worseStatus(authStatus, doctor.Worst(results)) { case doctor.StatusFail: p.Errorf("Problems found — fix the ✖ items above.") p.Hintf("For deeper triage, send tracebloc a support bundle: ./install-k8s.sh --diagnose") @@ -127,7 +143,76 @@ func runClusterDoctor( p.Warnf("Completed with warnings — review the ⚠ items above.") return nil default: - p.Successf("All checks passed — the cluster looks healthy.") + p.Successf("All checks passed — auth and cluster look healthy.") return nil } } + +// runAuthChecks reports on the CLI's own auth/config state (~/.tracebloc): are +// we signed in, to which env, is an active client selected, and does the backend +// still accept the token. It's the half of `cluster doctor` that diagnoses a +// failed *provision* rather than a sick cluster (RFC-0001 §8.5). Returns the +// worst status seen so the caller can fold it into the overall verdict. +func runAuthChecks(ctx context.Context, p *ui.Printer) doctor.Status { + p.Section("Auth & config") + + cfg, err := config.Load() + if err != nil { + p.Errorf("Config — couldn't read the CLI config: %v", err) + p.Hintf(" check ~/.tracebloc/config.json, or run `tracebloc login` to recreate it") + return doctor.StatusFail + } + if !cfg.SignedIn() { + p.Errorf("Sign-in — not signed in") + p.Hintf(" run `tracebloc login` (add --env dev|stg|prod for a non-prod backend)") + return doctor.StatusFail + } + + env := cfg.CurrentEnv + prof := cfg.Current() + if prof.Email != "" { + p.Successf("Sign-in — signed in to %s as %s", env, prof.Email) + } else { + p.Successf("Sign-in — signed in to %s", env) + } + + worst := doctor.StatusOK + if prof.ActiveClientID == "" { + p.Warnf("Active client — none selected for %s", env) + p.Hintf(" run `tracebloc client use ` (or `tracebloc client create`) to set the client this machine enrolls as") + worst = doctor.StatusWarn + } else { + p.Successf("Active client — %s", prof.ActiveClientID) + } + + // Live token check. Best-effort: an explicit 401 is a failure (expired / + // revoked → must re-login); a network/proxy error is only a warning, since + // we can't conclude the token itself is bad. + p.Detailf("verifying the token against %s …", api.BaseURL(env)) + client := newAPIClient(env) + client.Token = prof.Token + if _, werr := client.WhoAmI(ctx); werr != nil { + var ae *api.APIError + if errors.As(werr, &ae) && ae.StatusCode == http.StatusUnauthorized { + p.Errorf("Backend auth — %s rejected the token (401)", api.BaseURL(env)) + p.Hintf(" your session expired or was revoked — run `tracebloc login`") + return doctor.StatusFail + } + p.Warnf("Backend auth — couldn't verify the token: %v", werr) + p.Hintf(" the backend may be unreachable from here — check your network / HTTP(S)_PROXY") + return worseStatus(worst, doctor.StatusWarn) + } + p.Successf("Backend auth — token valid at %s", api.BaseURL(env)) + return worst +} + +// worseStatus returns the more severe of two doctor statuses (Fail > Warn > OK). +func worseStatus(a, b doctor.Status) doctor.Status { + if a == doctor.StatusFail || b == doctor.StatusFail { + return doctor.StatusFail + } + if a == doctor.StatusWarn || b == doctor.StatusWarn { + return doctor.StatusWarn + } + return doctor.StatusOK +} diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go new file mode 100644 index 00000000..eba87027 --- /dev/null +++ b/internal/cli/doctor_test.go @@ -0,0 +1,97 @@ +package cli + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/config" + "github.com/tracebloc/cli/internal/doctor" + "github.com/tracebloc/cli/internal/ui" +) + +// stubBackend points the newAPIClient seam at an httptest server for one test. +func stubBackend(t *testing.T, h http.HandlerFunc) { + t.Helper() + srv := httptest.NewServer(h) + t.Cleanup(srv.Close) + orig := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = orig }) +} + +// cli#101: `cluster doctor` auth/config/token checks (RFC-0001 §8.5). These pin +// runAuthChecks — the half of doctor that diagnoses a failed *provision*. + +func TestRunAuthChecks_NotSignedIn(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { + t.Errorf("not signed in → want Fail, got %v", st) + } + if !strings.Contains(out.String(), "Auth & config") || !strings.Contains(out.String(), "not signed in") { + t.Errorf("missing auth section / not-signed-in line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_TokenValid(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", Email: "a@b.io", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusOK { + t.Errorf("valid token + active client → want OK, got %v;\n%s", st, out.String()) + } + if !strings.Contains(out.String(), "token valid") { + t.Errorf("missing token-valid line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_TokenRejected401(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"detail":"Invalid token."}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusFail { + t.Errorf("token rejected (401) → want Fail, got %v", st) + } + if !strings.Contains(out.String(), "rejected the token (401)") { + t.Errorf("missing 401 line:\n%s", out.String()) + } +} + +func TestRunAuthChecks_NoActiveClientWarns(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x"}, // signed in, but no active client selected + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) + }) + var out bytes.Buffer + if st := runAuthChecks(context.Background(), ui.New(&out)); st != doctor.StatusWarn { + t.Errorf("valid token but no active client → want Warn, got %v;\n%s", st, out.String()) + } + if !strings.Contains(out.String(), "Active client — none") { + t.Errorf("missing no-active-client warning:\n%s", out.String()) + } +} diff --git a/internal/cli/installlog.go b/internal/cli/installlog.go new file mode 100644 index 00000000..3793cddf --- /dev/null +++ b/internal/cli/installlog.go @@ -0,0 +1,60 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "time" + + "github.com/tracebloc/cli/internal/config" +) + +// installLog is an append-only, timestamped record of a connect/provision run, +// always written to ~/.tracebloc/install-.log regardless of --verbose. The +// connect flow is zero-prompt on a headless box, so a failed run must leave a +// full trace on disk to inspect or send to support even when the terminal stayed +// quiet (RFC-0001 §8.5). +// +// A nil *installLog is a no-op on every method, so callers never have to guard: +// logging must never be what fails a provision. +type installLog struct { + f *os.File +} + +// newInstallLog creates ~/.tracebloc/install-.log (mode 0600 — it can carry +// hostnames and paths). It returns the log (nil if it couldn't be opened) and +// the path it used, so the caller can surface the path without ever failing the +// command over logging. +func newInstallLog() (*installLog, string) { + dir, err := config.Dir() + if err != nil { + return nil, "" + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, "" + } + path := filepath.Join(dir, "install-"+time.Now().UTC().Format("20060102-150405")+".log") + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) + if err != nil { + return nil, path + } + l := &installLog{f: f} + l.Logf("tracebloc connect/provision log") + return l, path +} + +// Logf appends a UTC-timestamped line. Safe on a nil receiver (no-op). +func (l *installLog) Logf(format string, a ...any) { + if l == nil || l.f == nil { + return + } + _, _ = fmt.Fprintf(l.f, "%s %s\n", time.Now().UTC().Format(time.RFC3339), fmt.Sprintf(format, a...)) +} + +// Close closes the underlying file. Safe on a nil receiver (no-op). +func (l *installLog) Close() { + if l == nil || l.f == nil { + return + } + _ = l.f.Close() +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 84279c3c..2b7868dd 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -8,6 +8,8 @@ package cli import ( "io" + "os" + "strings" "github.com/spf13/cobra" @@ -80,6 +82,11 @@ what's planned next.`, // CI / log capture where stdout might still look like a terminal. root.PersistentFlags().Bool("plain", false, "disable color and decorative output (also honors $NO_COLOR)") + // --verbose streams the per-step detail (device-flow → provision → install) + // that's hidden by default; also enabled by $TRACEBLOC_LOG_LEVEL=debug. The + // default output stays quiet — a handful of ✔ lines (RFC-0001 §8.5). + root.PersistentFlags().Bool("verbose", false, + "stream detailed step-by-step progress (also via $TRACEBLOC_LOG_LEVEL=debug)") // Subcommands. New phases append here. root.AddCommand(newVersionCmd(info)) @@ -127,8 +134,26 @@ func printerFor(cmd *cobra.Command) *ui.Printer { // dataset push's --output-json mode, which routes human output to // stderr so stdout carries only the JSON result. func printerForWriter(cmd *cobra.Command, w io.Writer) *ui.Printer { + var opts []ui.Option if plain, _ := cmd.Flags().GetBool("plain"); plain { - return ui.New(w, ui.WithColor(false)) + opts = append(opts, ui.WithColor(false)) } - return ui.New(w) + if verboseRequested(cmd) { + opts = append(opts, ui.WithVerbose(true)) + } + return ui.New(w, opts...) +} + +// verboseRequested reports whether the user asked for verbose output, via the +// --verbose flag or $TRACEBLOC_LOG_LEVEL (debug/trace/verbose). The flag wins; +// the env var lets a headless / scripted run opt in without editing the command. +func verboseRequested(cmd *cobra.Command) bool { + if v, err := cmd.Flags().GetBool("verbose"); err == nil && v { + return true + } + switch strings.ToLower(os.Getenv("TRACEBLOC_LOG_LEVEL")) { + case "debug", "trace", "verbose": + return true + } + return false } diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go new file mode 100644 index 00000000..5311d097 --- /dev/null +++ b/internal/cli/verbose_install_test.go @@ -0,0 +1,111 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/tracebloc/cli/internal/api" + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" + "github.com/tracebloc/cli/internal/ui" +) + +// loginStub wires a minimal happy-path device flow via the auth_test seam. +func loginStub(t *testing.T) { + withTestBackend(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/device/code": + _, _ = w.Write([]byte(`{"device_code":"dc","user_code":"X","verification_uri":"https://x/activate","interval":5}`)) + case "/device/token": + _, _ = w.Write([]byte(`{"token":"cat_v"}`)) + case "/userinfo/": + _, _ = w.Write([]byte(`{"email":"e@co","account":"A"}`)) + } + }) +} + +// cli#101 (RFC-0001 §8.5): --verbose streams the device-flow detail; the default +// output stays quiet. + +func TestLogin_VerboseStreamsDetail(t *testing.T) { + loginStub(t) + out, err := runCmd(t, "--verbose", "login") + if err != nil { + t.Fatalf("login: %v", err) + } + if !strings.Contains(out, "requesting a device code") { + t.Errorf("--verbose should stream the device-flow detail, got:\n%s", out) + } +} + +func TestLogin_QuietByDefault(t *testing.T) { + loginStub(t) + out, err := runCmd(t, "login") + if err != nil { + t.Fatalf("login: %v", err) + } + if strings.Contains(out, "requesting a device code") { + t.Errorf("default output should stay quiet (no verbose detail), got:\n%s", out) + } +} + +// TestClientCreate_FailurePrintsResumeAndWritesInstallLog pins the §8.5 failure +// path: a failed provision prints the (idempotent) resume command + the doctor +// pointer, and every run leaves an install-.log on disk. +func TestClientCreate_FailurePrintsResumeAndWritesInstallLog(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + // List succeeds; the provision POST 500s → create fails. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + var out bytes.Buffer + err := runClientCreate(context.Background(), ui.New(&out), nil, + clientCreateOpts{name: "My Client", location: "DE", yes: true}) + if err == nil { + t.Fatal("expected the provision to fail (POST 500)") + } + // Resume hint: the idempotent re-run command (name with a space gets quoted) + // + the doctor pointer. + if !strings.Contains(out.String(), "tracebloc client create --name 'My Client' --location DE") { + t.Errorf("missing / incorrect resume command:\n%s", out.String()) + } + if !strings.Contains(out.String(), "cluster doctor") { + t.Errorf("missing `cluster doctor` pointer:\n%s", out.String()) + } + // An install-.log is written, recording the failure. + logs, _ := filepath.Glob(filepath.Join(dir, "install-*.log")) + if len(logs) == 0 { + t.Fatal("no install-*.log written") + } + raw, _ := os.ReadFile(logs[0]) + if !strings.Contains(string(raw), "FAILED") { + t.Errorf("install log should record the failure:\n%s", raw) + } +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index e87122f1..7821fe8c 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -29,8 +29,9 @@ import ( // a command's call tree; it is not safe for concurrent writes to the // same underlying writer (neither is fmt.Fprintf). type Printer struct { - w io.Writer - color bool + w io.Writer + color bool + verbose bool } // Option customizes a Printer at construction. This is the functional- @@ -47,6 +48,13 @@ func WithColor(on bool) Option { return func(p *Printer) { p.color = on } } +// WithVerbose enables verbose output: Detailf lines print only when on. Wire it +// to a --verbose flag / $TRACEBLOC_LOG_LEVEL so the default happy path stays +// quiet (~6 status lines) while a streamed step-by-step view is opt-in. +func WithVerbose(on bool) Option { + return func(p *Printer) { p.verbose = on } +} + // New returns a Printer writing to w. By default it colorizes only when // w is a real terminal and the NO_COLOR env var is unset // (https://no-color.org). Options are applied after auto-detection, so @@ -143,6 +151,21 @@ func (p *Printer) Infof(format string, a ...any) { p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) } +// Detailf prints an indented, dim step-detail line — but ONLY in verbose mode +// (WithVerbose). Use for the streamed device-flow → provision → install trace +// (§8.5 R-verbose) that would be noise by default; the quiet path skips it. +func (p *Printer) Detailf(format string, a ...any) { + if !p.verbose { + return + } + p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) +} + +// Verbose reports whether this Printer is in verbose mode, so a caller can guard +// expensive detail (e.g. formatting a large value) it would otherwise build then +// discard. +func (p *Printer) Verbose() bool { return p.verbose } + // Errorf prints a bold-red ✖ error line. Unlike common.sh's error(), // it does NOT exit — surfacing the message is the UI's job; the command // still returns an *exitError so main() owns the process exit code. diff --git a/internal/ui/verbose_test.go b/internal/ui/verbose_test.go new file mode 100644 index 00000000..0c6e78a2 --- /dev/null +++ b/internal/ui/verbose_test.go @@ -0,0 +1,27 @@ +package ui + +import ( + "bytes" + "strings" + "testing" +) + +// TestDetailfVerboseGating: Detailf is silent by default and prints only under +// WithVerbose — the --verbose contract (RFC-0001 §8.5: the default stays quiet). +func TestDetailfVerboseGating(t *testing.T) { + var quiet bytes.Buffer + New(&quiet).Detailf("hidden %d", 1) + if quiet.Len() != 0 { + t.Errorf("Detailf must be silent without WithVerbose, got %q", quiet.String()) + } + + var loud bytes.Buffer + p := New(&loud, WithVerbose(true)) + if !p.Verbose() { + t.Error("Verbose() should report true under WithVerbose") + } + p.Detailf("shown %d", 2) + if !strings.Contains(loud.String(), "shown 2") { + t.Errorf("verbose Detailf should print, got %q", loud.String()) + } +} From ed2f73621ea2d6e85b1f9eb44844a8da23ef86c3 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 29 Jun 2026 18:08:18 +0500 Subject: [PATCH 2/4] fix(cli): don't log a cancelled provision as "done" (cli#101, Bugbot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The success-path defer blanket-logged "done" on err==nil, so declining the confirm prompt (which returns nil after "Cancelled.") recorded a user abort as a successful run in install-.log — misleading for support / post-mortems. Log the terminal outcome at each branch instead (minted / adopted / cancelled); the defer now only handles the failure case. Test: TestClientCreate_CancelLogsCancelledNotDone. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 5 ++- internal/cli/verbose_install_test.go | 51 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 815e2d45..18c8d57f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -149,7 +149,9 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien } return } - ilog.Logf("done") + // Success/cancel: the terminal outcome was already logged at its own + // branch (minted / adopted / cancelled), so don't blanket-log "done" + // here — a declined confirm must not read as a successful provision. if logPath != "" { p.Detailf("full log: %s", logPath) } @@ -224,6 +226,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return mapClientErr(cerr) } if !ok { + ilog.Logf("cancelled by user at the confirm prompt") p.Hintf("Cancelled.") return nil } diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go index 5311d097..fde2cb10 100644 --- a/internal/cli/verbose_install_test.go +++ b/internal/cli/verbose_install_test.go @@ -109,3 +109,54 @@ func TestClientCreate_FailurePrintsResumeAndWritesInstallLog(t *testing.T) { t.Errorf("install log should record the failure:\n%s", raw) } } + +// TestClientCreate_CancelLogsCancelledNotDone pins the Bugbot fix: declining the +// confirm prompt is a user abort, not a successful provision — the install log +// must record "cancelled", never "done". +func TestClientCreate_CancelLogsCancelledNotDone(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + posted := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + posted = true + } + _, _ = w.Write([]byte(`[]`)) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + confirmNo := false + pr := &fakePrompter{answers: map[string]string{}, confirm: &confirmNo} + var out bytes.Buffer + if err := runClientCreate(context.Background(), ui.New(&out), pr, + clientCreateOpts{name: "Lab", location: "DE"}); err != nil { + t.Fatalf("declining the confirm should be a clean exit, got: %v", err) + } + if posted { + t.Error("no client should be POSTed when the user declines") + } + logs, _ := filepath.Glob(filepath.Join(dir, "install-*.log")) + if len(logs) == 0 { + t.Fatal("no install-*.log written") + } + raw, _ := os.ReadFile(logs[0]) + if !strings.Contains(string(raw), "cancelled") { + t.Errorf("install log should record the cancel, got:\n%s", raw) + } + if strings.Contains(string(raw), "done") { + t.Errorf("a cancelled run must NOT be logged as 'done':\n%s", raw) + } +} From b9a1497320d970c3bec9a4f130bc63f63f034a54 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 29 Jun 2026 18:14:26 +0500 Subject: [PATCH 3/4] fix(cli): resume command includes prompted name/location, not just flags (cli#101, Bugbot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resumeCommand(opts) read opts, which carries only the flag values — so a failed interactive provision (name/location typed at a prompt) printed a resume command missing --name/--location, defeating the copy-paste-to-retry goal. Write the resolved values back into opts after gathering them, so the defer's resumeCommand reflects what the user actually entered. Test: TestClientCreate_ResumeCommandIncludesPromptedValues. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 4 +++ internal/cli/verbose_install_test.go | 43 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/internal/cli/client.go b/internal/cli/client.go index 18c8d57f..b211b29f 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -185,6 +185,10 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien return mapClientErr(err) } } + // Reflect the resolved (possibly prompted) name + location back into opts, so + // the failure-path resume command includes them — opts otherwise carries only + // the flags, omitting anything the user typed at a prompt (Bugbot). + opts.name, opts.location = name, location // Read the cluster anchor (kube-system UID) so create is get-or-create keyed on // it — re-running on the same cluster adopts the existing client instead of diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go index fde2cb10..c5bfb09b 100644 --- a/internal/cli/verbose_install_test.go +++ b/internal/cli/verbose_install_test.go @@ -160,3 +160,46 @@ func TestClientCreate_CancelLogsCancelledNotDone(t *testing.T) { t.Errorf("a cancelled run must NOT be logged as 'done':\n%s", raw) } } + +// TestClientCreate_ResumeCommandIncludesPromptedValues pins the Bugbot fix: when +// name/location come from interactive prompts (not flags), a failed provision's +// resume command must still include them — opts alone would omit them. +func TestClientCreate_ResumeCommandIncludesPromptedValues(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "tok"}, + }}).Save(); err != nil { + t.Fatal(err) + } + // list ok; the provision POST 500s after the user confirms. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(srv.Close) + origClient := newAPIClient + newAPIClient = func(string) *api.Client { return &api.Client{BaseURL: srv.URL, HTTP: srv.Client()} } + t.Cleanup(func() { newAPIClient = origClient }) + origCID := readClusterID + readClusterID = func(context.Context, cluster.KubeconfigOptions) (string, error) { + return "", errors.New("no cluster (test)") + } + t.Cleanup(func() { readClusterID = origCID }) + + confirmYes := true + pr := &fakePrompter{answers: map[string]string{ + "Client name": "Prompted Lab", + "Location zone (e.g. DE)": "FR", + }, confirm: &confirmYes} + var out bytes.Buffer + // No name/location flags — both come from the prompts. + if err := runClientCreate(context.Background(), ui.New(&out), pr, clientCreateOpts{}); err == nil { + t.Fatal("expected the provision to fail (POST 500)") + } + if !strings.Contains(out.String(), "--name 'Prompted Lab' --location FR") { + t.Errorf("resume command should carry the PROMPTED name + location, got:\n%s", out.String()) + } +} From c712dc7e5de02f1348f71f42860e1eba8ea3fe33 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 29 Jun 2026 18:23:15 +0500 Subject: [PATCH 4/4] fix(cli): doctor folds auth into the kubeconfig-exit code; install log advertises no path on open failure (cli#101, Bugbot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more Bugbot findings: - `cluster doctor` returned exit 3 on a kubeconfig load / clientset failure even when the auth section had failed (e.g. a 401), so automation could read a bad token as a kubeconfig-only problem. A failed auth section now escalates the exit to 2; exit 3 is kept for the auth-OK case (the documented contract). - newInstallLog returned the intended path even when opening the file failed, so the failure hint could print "Full log:" for a file that was never written. It now returns an empty path, and the caller's guard skips the hint. Tests: doctor kubeconfig-fail → exit 2 (auth fail) / exit 3 (auth OK); install log returns an empty path when the file can't be opened. Co-Authored-By: Claude Opus 4.8 --- internal/cli/doctor.go | 17 +++++++++++--- internal/cli/doctor_test.go | 35 ++++++++++++++++++++++++++++ internal/cli/installlog.go | 4 +++- internal/cli/verbose_install_test.go | 23 ++++++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index db1c5348..bf9e9628 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -90,18 +90,19 @@ func runClusterDoctor( }) if err != nil { // 3 = kubeconfig file/parse problem (same class as cluster info). The - // auth section above already ran, so a kubeconfig issue doesn't hide it. + // auth section above already ran; if IT also failed, escalate to 2 so + // automation doesn't read a real auth failure as a kubeconfig-only one. p.Section("Cluster") p.Errorf("Kubeconfig — couldn't load it: %v", err) p.Hintf(" point --kubeconfig / --context at your cluster, or fix ~/.kube/config") - return &exitError{code: 3, err: nil} + return &exitError{code: kubeconfigExitCode(authStatus), err: nil} } cs, err := cluster.NewClientset(resolved) if err != nil { p.Section("Cluster") p.Errorf("Kubeconfig — %v", err) - return &exitError{code: 3, err: nil} + return &exitError{code: kubeconfigExitCode(authStatus), err: nil} } p.Section("Kubeconfig") @@ -216,3 +217,13 @@ func worseStatus(a, b doctor.Status) doctor.Status { } return doctor.StatusOK } + +// kubeconfigExitCode is 3 ("kubeconfig could not be loaded") unless the auth +// section also failed — then it escalates to 2 ("a check failed"), so a bad +// token isn't masked behind a kubeconfig-only exit code (Bugbot). +func kubeconfigExitCode(authStatus doctor.Status) int { + if authStatus == doctor.StatusFail { + return 2 + } + return 3 +} diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index eba87027..22c4cf66 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "context" + "errors" "net/http" "net/http/httptest" "strings" @@ -95,3 +96,37 @@ func TestRunAuthChecks_NoActiveClientWarns(t *testing.T) { t.Errorf("missing no-active-client warning:\n%s", out.String()) } } + +// TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails pins the Bugbot fix: a +// kubeconfig load failure normally exits 3, but if the auth section ALSO failed +// (here: not signed in) it escalates to 2 so a bad token isn't masked as a +// kubeconfig-only problem. +func TestClusterDoctor_KubeconfigFailEscalatesWhenAuthFails(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // not signed in → auth Fail + var out bytes.Buffer + err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 2 { + t.Fatalf("kubeconfig-fail + auth-fail → want exit 2, got %v", err) + } +} + +// TestClusterDoctor_KubeconfigFailStays3WhenAuthOK: with auth healthy, a +// kubeconfig failure keeps the documented exit-3 contract. +func TestClusterDoctor_KubeconfigFailStays3WhenAuthOK(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + if err := (&config.Config{CurrentEnv: "dev", Profiles: map[string]*config.Profile{ + "dev": {Token: "x", ActiveClientID: "5"}, + }}).Save(); err != nil { + t.Fatal(err) + } + stubBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"email":"a@b.io","account":"Acme"}`)) // WhoAmI ok → auth OK + }) + var out bytes.Buffer + err := runClusterDoctor(context.Background(), ui.New(&out), "/nonexistent-kubeconfig-xyz", "", "") + var ee *exitError + if !errors.As(err, &ee) || ee.Code() != 3 { + t.Fatalf("kubeconfig-fail + auth-OK → want exit 3 (contract), got %v", err) + } +} diff --git a/internal/cli/installlog.go b/internal/cli/installlog.go index 3793cddf..8c45a232 100644 --- a/internal/cli/installlog.go +++ b/internal/cli/installlog.go @@ -36,7 +36,9 @@ func newInstallLog() (*installLog, string) { path := filepath.Join(dir, "install-"+time.Now().UTC().Format("20060102-150405")+".log") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) if err != nil { - return nil, path + // No file was created — return an empty path so the caller never + // advertises a "Full log:" location that doesn't exist (Bugbot). + return nil, "" } l := &installLog{f: f} l.Logf("tracebloc connect/provision log") diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go index c5bfb09b..2c1f241c 100644 --- a/internal/cli/verbose_install_test.go +++ b/internal/cli/verbose_install_test.go @@ -203,3 +203,26 @@ func TestClientCreate_ResumeCommandIncludesPromptedValues(t *testing.T) { t.Errorf("resume command should carry the PROMPTED name + location, got:\n%s", out.String()) } } + +// TestNewInstallLog_NoPathWhenFileOpenFails pins the Bugbot fix: when the log +// file can't be opened, newInstallLog returns an empty path (not a path to a +// file that was never written), so the failure hint won't advertise it. +func TestNewInstallLog_NoPathWhenFileOpenFails(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("runs as root — directory perms don't restrict file creation") + } + dir := t.TempDir() + if err := os.Chmod(dir, 0o500); err != nil { // read-only dir → OpenFile fails + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0o700) }) // restore so TempDir cleanup can remove it + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + + l, path := newInstallLog() + if l != nil { + l.Close() + } + if path != "" { + t.Errorf("OpenFile failed → path must be empty (no file created), got %q", path) + } +}