From b5049a5fdd7824e18bad557dcf1f65a49bfacfce Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 6 Jul 2026 15:54:48 +0500 Subject: [PATCH 1/5] =?UTF-8?q?feat(cli#128):=20select=20client=20by=20slu?= =?UTF-8?q?g=20or=20id=20+=20bind=20cluster=20info=20to=20it=20(=C2=A77.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Testing on a real box surfaced two §7.3 gaps: - `client use` only accepted the numeric id, but RFC-0001 §7.1/D3 makes the namespace slug the handle. Accept either the slug (e.g. tracebloc-amazon) or the id — non-breaking, and it matches what `client list` shows. - `cluster info` ignored the active client and diagnosed the kubeconfig's current-context namespace. Default it to the active client's namespace (bindActiveClientNamespace) when the user gave neither --namespace nor --context, so it diagnoses the selected client's cluster. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 19 ++++++++++++------- internal/cli/client_test.go | 21 +++++++++++++++++---- internal/cli/cluster.go | 11 ++++++----- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 8511d187..04162473 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -91,9 +91,12 @@ func newClientListCmd() *cobra.Command { func newClientUseCmd() *cobra.Command { return &cobra.Command{ - Use: "use ", - Short: "Enroll this machine as an existing client", - Args: cobra.ExactArgs(1), + Use: "use ", + Short: "Select the active client for this machine (by slug or id)", + Long: `Select which client this machine's data / cluster commands act on. +Accepts the client's namespace slug (e.g. tracebloc-amazon) — the handle shown +by ` + "`tracebloc client list`" + ` — or its numeric id.`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runClientUse(cmd.Context(), printerFor(cmd), args[0]) }, @@ -613,7 +616,9 @@ func clientStateLabel(status int) string { } } -func runClientUse(ctx context.Context, p *ui.Printer, id string) error { +// runClientUse selects the active client by its handle — the namespace slug +// (the RFC-0001 §7.1/D3 handle) or, for backward compatibility, its numeric id. +func runClientUse(ctx context.Context, p *ui.Printer, handle string) error { client, cfg, err := authedClient() if err != nil { return &exitError{code: 1, err: err} @@ -623,17 +628,17 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error { return &exitError{code: 1, err: err} } for _, c := range clients { - if strconv.Itoa(c.ID) == id { + if c.Namespace == handle || strconv.Itoa(c.ID) == handle { setActiveClient(cfg.Current(), &c) if serr := cfg.Save(); serr != nil { return &exitError{code: 1, err: serr} } - p.Successf("This machine is now set to enroll as client %s (%s).", id, c.Name) + p.Successf("This machine is now set to enroll as client %q (namespace %s).", c.Name, c.Namespace) return nil } } return &exitError{code: 1, err: fmt.Errorf( - "no client %s in your account — run `tracebloc client list` to see the ids", id)} + "no client %q in your account — run `tracebloc client list` to see the slugs and ids", handle)} } // setActiveClient points this env's profile at c, caching its namespace and diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index 5222510e..feee3ee7 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -251,17 +251,30 @@ func TestClientList(t *testing.T) { func TestClientUse(t *testing.T) { withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`[{"id":7,"first_name":"gamma","namespace":"gamma"}]`)) + _, _ = w.Write([]byte(`[{"id":7,"first_name":"gamma","namespace":"gamma-ns"}]`)) }) + + // By numeric id (backward compatible). if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "7"); err != nil { t.Fatal(err) } cfg, _ := config.Load() + if cfg.Current().ActiveClientID != "7" || cfg.Current().ActiveClientNamespace != "gamma-ns" { + t.Errorf("after use by id: %+v, want id=7 ns=gamma-ns", cfg.Current()) + } + + // By namespace slug — the RFC-0001 §7.1/D3 handle. + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "gamma-ns"); err != nil { + t.Fatalf("use by slug: %v", err) + } + cfg, _ = config.Load() if cfg.Current().ActiveClientID != "7" { - t.Errorf("active = %q, want 7", cfg.Current().ActiveClientID) + t.Errorf("after use by slug: active = %q, want 7", cfg.Current().ActiveClientID) } - if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "99"); err == nil { - t.Error("expected an error for an unknown client id") + + // Unknown handle → error. + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "nope"); err == nil { + t.Error("expected an error for an unknown client handle") } } diff --git a/internal/cli/cluster.go b/internal/cli/cluster.go index db2cd7ad..69ff20e0 100644 --- a/internal/cli/cluster.go +++ b/internal/cli/cluster.go @@ -122,11 +122,12 @@ func runClusterInfo( ) error { p.Banner("tracebloc", "cluster diagnostics") - resolved, err := cluster.Load(cluster.KubeconfigOptions{ - Path: kubeconfigPath, - Context: contextOverride, - Namespace: nsOverride, - }) + // Default the namespace to the active client's (RFC-0001 §7.3) when the user + // gave neither --namespace nor --context, so `cluster info` diagnoses the + // selected client's cluster rather than the kubeconfig's current-context ns. + opts := cluster.KubeconfigOptions{Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride} + bindActiveClientNamespace(&opts) + resolved, err := cluster.Load(opts) if err != nil { // Kubeconfig errors are exit-code-3 territory (file/parse // problem, same conceptual class as `ingest validate`'s From a5f8369af2a174bc6741079a38f9da44ac04f3c8 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 6 Jul 2026 15:54:48 +0500 Subject: [PATCH 2/5] fix(cli): list sign-in + client commands on the bare home screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare `tracebloc` home screen hand-listed only the data / cluster / ingest verbs, hiding the whole login + client lifecycle (login, client create/list/ use, auth status) — so a new user couldn't discover how to sign in or select a client. Add those, group the screen into "Sign in & pick a client" vs "Work with data", and point at `tracebloc --help` for the full command list. Co-Authored-By: Claude Opus 4.8 --- internal/cli/root.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/cli/root.go b/internal/cli/root.go index 2b7868dd..1af96bde 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -108,14 +108,20 @@ what's planned next.`, } p := printerFor(cmd) p.Banner("tracebloc", "interactive data ingestion for your cluster") - p.Section("Get started") + p.Section("Sign in & pick a client") + p.Infof("tracebloc login — sign in via your browser") + p.Infof("tracebloc client create — provision this machine as a client") + p.Infof("tracebloc client list — list your clients + their online state") + p.Infof("tracebloc client use — select the active client (by slug or id)") + p.Infof("tracebloc auth status — show who/where you're signed in") + p.Section("Work with data") p.Infof("tracebloc data ingest — stage + ingest a dataset interactively (or use --help to see flags)") p.Infof("tracebloc data list — list datasets ingested in the cluster") p.Infof("tracebloc data delete — delete an ingested dataset (its table + files)") - p.Infof("tracebloc cluster info — check the CLI can reach your cluster") + p.Infof("tracebloc cluster info — check the CLI can reach your active client's cluster") p.Infof("tracebloc ingest validate f.yaml — validate an ingest.yaml locally") p.Newline() - p.Hintf("Add --help to any command for the full flag list.") + p.Hintf("Run `tracebloc --help` for the full command list, or add --help to any command for its flags.") return nil } From 764c512e61f9aa8be057936e888a11aca8e5a6c1 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 6 Jul 2026 16:00:21 +0500 Subject: [PATCH 3/5] feat(cli#128): backfill the active client's namespace cache on `client list` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A config written before the namespace cache existed has active_client_id set but no cached namespace, so the §7.3 binding no-ops (cluster info / data commands fall back to the current-context namespace) until an explicit re-`use`. `client list` already fetches every client, so backfill the active client's namespace + name from that list when the cache is empty — no extra request, and binding then works without a manual re-select. Best-effort: no-op when already cached or the active id isn't in the account. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 26 ++++++++++++++++++++++ internal/cli/clustertarget_test.go | 35 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/internal/cli/client.go b/internal/cli/client.go index 04162473..91ca7645 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -576,6 +576,12 @@ func runClientList(ctx context.Context, p *ui.Printer) error { p.Hintf("No clients yet. Run `tracebloc client create`.") return nil } + // Backfill the active client's namespace cache if it's empty — e.g. a + // config written before the cache existed. `list` already has every + // client in hand, so this needs no extra request and lets `cluster info` + // / `data` commands bind (§7.3) without an explicit re-`use`. + backfillActiveClientCache(cfg, clients) + p.Section("Clients in your account") active := cfg.Current().ActiveClientID for _, c := range clients { @@ -650,6 +656,26 @@ func setActiveClient(p *config.Profile, c *api.ProvisionedClient) { p.ActiveClientName = c.Name } +// backfillActiveClientCache fills the active client's namespace/name cache from +// an already-fetched client list when it's missing (a config written before the +// cache existed, or the id set some other way). Best-effort: if the active +// client isn't in the list, or the cache is already populated, it's a no-op; a +// Save failure is ignored (the next `client use` will persist it anyway). +func backfillActiveClientCache(cfg *config.Config, clients []api.ProvisionedClient) { + p := cfg.Current() + if p.ActiveClientID == "" || p.ActiveClientNamespace != "" { + return + } + for i := range clients { + if strconv.Itoa(clients[i].ID) == p.ActiveClientID { + p.ActiveClientNamespace = clients[i].Namespace + p.ActiveClientName = clients[i].Name + _ = cfg.Save() + return + } + } +} + // renderClientReview shows the assembled inputs before the confirm prompt, so // the user sees the derived namespace and location before anything is created. func renderClientReview(p *ui.Printer, name, namespace, location, clusterID string) { diff --git a/internal/cli/clustertarget_test.go b/internal/cli/clustertarget_test.go index d0deb0f3..18e4ac97 100644 --- a/internal/cli/clustertarget_test.go +++ b/internal/cli/clustertarget_test.go @@ -18,6 +18,41 @@ func TestSetActiveClient_CachesNamespaceAndName(t *testing.T) { } } +func TestBackfillActiveClientCache(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) + clients := []api.ProvisionedClient{ + {ID: 1039, Name: "aws-ubuntu", Namespace: "tracebloc-amazon"}, + {ID: 1053, Name: "asad-macbook", Namespace: "asad-macbook-2"}, + } + + // Pre-cache config (old binary): id set, namespace empty → backfilled. + cfg := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "1039"}, + }} + backfillActiveClientCache(cfg, clients) + if cfg.Current().ActiveClientNamespace != "tracebloc-amazon" || cfg.Current().ActiveClientName != "aws-ubuntu" { + t.Errorf("backfill = %+v, want ns=tracebloc-amazon name=aws-ubuntu", cfg.Current()) + } + + // Already cached → left untouched (no clobber). + cached := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "1039", ActiveClientNamespace: "custom-ns", ActiveClientName: "custom"}, + }} + backfillActiveClientCache(cached, clients) + if cached.Current().ActiveClientNamespace != "custom-ns" { + t.Errorf("backfill clobbered an existing cache: %+v", cached.Current()) + } + + // Active id not in the list → no-op (no crash). + orphan := &config.Config{CurrentEnv: "prod", Profiles: map[string]*config.Profile{ + "prod": {Token: "t", ActiveClientID: "9999"}, + }} + backfillActiveClientCache(orphan, clients) + if orphan.Current().ActiveClientNamespace != "" { + t.Errorf("orphan id should not backfill, got %+v", orphan.Current()) + } +} + func TestClientStateLabel(t *testing.T) { cases := map[int]string{ clientStatusOnline: "online", From 53a88d10f3b4dde45143215c64a14c39d0b4cda8 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 6 Jul 2026 16:48:46 +0500 Subject: [PATCH 4/5] =?UTF-8?q?fix(cli#128):=20cluster=20info=20uses=20the?= =?UTF-8?q?=20=C2=A77.3=20explain=20+=20slug=20beats=20id=20collision=20(B?= =?UTF-8?q?ugbot)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Bugbot findings on #133: - cluster info discarded the binding and returned the raw "no release" error on exit 4, instead of the §7.3 "active client runs on another machine" guidance the data commands give. Route its discovery error through binding.explain. To make explain callable from both paths, gate it on the cluster.ErrNoParentRelease sentinel (errors.Is) rather than the internal noParentReleaseError wrapper — which is now removed, simplifying resolveClusterTarget back to a single exit-4 return. - `client use ` matched slug OR id in one pass, so a numeric handle that is one client's all-numeric slug and another's id resolved to whichever the API listed first. Match slug first (unique per account), fall back to id only if no slug matched — deterministic, slug wins. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 33 ++++++++++++++++++++++-------- internal/cli/client_test.go | 17 +++++++++++++++ internal/cli/cluster.go | 8 +++++--- internal/cli/clustertarget.go | 33 ++++++++---------------------- internal/cli/clustertarget_test.go | 5 ++++- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 91ca7645..4714e0ba 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -633,20 +633,37 @@ func runClientUse(ctx context.Context, p *ui.Printer, handle string) error { if err != nil { return &exitError{code: 1, err: err} } - for _, c := range clients { - if c.Namespace == handle || strconv.Itoa(c.ID) == handle { - setActiveClient(cfg.Current(), &c) - if serr := cfg.Save(); serr != nil { - return &exitError{code: 1, err: serr} - } - p.Successf("This machine is now set to enroll as client %q (namespace %s).", c.Name, c.Namespace) - return nil + // Slug takes precedence over id. Namespaces are unique per account, so a + // slug match is unambiguous; matching id in the same pass would let a + // client whose slug is all-numeric collide with another client's id and + // pick whichever the API listed first. Fall back to id only if no slug + // matched (backward compatibility). + var byID *api.ProvisionedClient + for i := range clients { + if clients[i].Namespace == handle { + return selectClient(p, cfg, &clients[i]) } + if byID == nil && strconv.Itoa(clients[i].ID) == handle { + byID = &clients[i] + } + } + if byID != nil { + return selectClient(p, cfg, byID) } return &exitError{code: 1, err: fmt.Errorf( "no client %q in your account — run `tracebloc client list` to see the slugs and ids", handle)} } +// selectClient points this machine at c and persists the choice. +func selectClient(p *ui.Printer, cfg *config.Config, c *api.ProvisionedClient) error { + setActiveClient(cfg.Current(), c) + if err := cfg.Save(); err != nil { + return &exitError{code: 1, err: err} + } + p.Successf("This machine is now set to enroll as client %q (namespace %s).", c.Name, c.Namespace) + return nil +} + // setActiveClient points this env's profile at c, caching its namespace and // display name alongside the id so the data commands can bind to the active // client's cluster (§7.3) without a backend round-trip. Callers Save() after. diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index feee3ee7..280ee254 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -278,6 +278,23 @@ func TestClientUse(t *testing.T) { } } +// A numeric handle that is one client's slug AND another client's id must +// resolve deterministically to the slug owner (slug > id), regardless of list +// order — not "whichever the API returned first". +func TestClientUse_SlugBeatsNumericIDCollision(t *testing.T) { + withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { + // id=1039 listed FIRST; the client whose slug is "1039" listed second. + _, _ = w.Write([]byte(`[{"id":1039,"first_name":"by-id","namespace":"other-ns"},{"id":42,"first_name":"by-slug","namespace":"1039"}]`)) + }) + if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "1039"); err != nil { + t.Fatal(err) + } + cfg, _ := config.Load() + if cfg.Current().ActiveClientID != "42" || cfg.Current().ActiveClientNamespace != "1039" { + t.Errorf("handle 1039 should select the slug owner (id 42), got %+v", cfg.Current()) + } +} + func TestClientCreate_Interactive(t *testing.T) { var body api.CreateClientRequest posted := false diff --git a/internal/cli/cluster.go b/internal/cli/cluster.go index 69ff20e0..85316a10 100644 --- a/internal/cli/cluster.go +++ b/internal/cli/cluster.go @@ -126,7 +126,7 @@ func runClusterInfo( // gave neither --namespace nor --context, so `cluster info` diagnoses the // selected client's cluster rather than the kubeconfig's current-context ns. opts := cluster.KubeconfigOptions{Path: kubeconfigPath, Context: contextOverride, Namespace: nsOverride} - bindActiveClientNamespace(&opts) + binding := bindActiveClientNamespace(&opts) resolved, err := cluster.Load(opts) if err != nil { // Kubeconfig errors are exit-code-3 territory (file/parse @@ -151,8 +151,10 @@ func runClusterInfo( // 4 = "cluster reachable, but no tracebloc release here." // Distinct from the kubeconfig error (3) so callers can // branch: 3 means "fix your kubeconfig", 4 means "install - // the parent chart first". - return &exitError{code: 4, err: err} + // the parent chart first". When the namespace came from the + // active-client binding, explain() rewrites this to the §7.3 + // "active client runs on another machine" guidance. + return binding.explain(&exitError{code: 4, err: err}) } // Apply the SA-name override here. Discovery doesn't read the diff --git a/internal/cli/clustertarget.go b/internal/cli/clustertarget.go index e24dc5da..fb9c75de 100644 --- a/internal/cli/clustertarget.go +++ b/internal/cli/clustertarget.go @@ -11,17 +11,6 @@ import ( "github.com/tracebloc/cli/internal/config" ) -// noParentReleaseError marks the exit-4 case where the reached cluster -// genuinely hosts no tracebloc release in the target namespace -// (cluster.ErrNoParentRelease) — as opposed to a present-but-PVC-missing -// release, an API/RBAC list failure, or an ambiguous multiple-release match. -// §7.3 uses it to turn an active-client binding miss into a clear "runs on -// another machine" message; the other failures keep their own diagnostics. -type noParentReleaseError struct{ err error } - -func (e *noParentReleaseError) Error() string { return e.err.Error() } -func (e *noParentReleaseError) Unwrap() error { return e.err } - // clusterTarget bundles the cluster handles the data commands resolve from a // kubeconfig before doing any work: the resolved config, a clientset, the // parent tracebloc release, and — when asked — the shared data PVC. @@ -53,12 +42,10 @@ func resolveClusterTarget(ctx context.Context, opts cluster.KubeconfigOptions, n } release, err := cluster.DiscoverParentRelease(ctx, cs, resolved.Namespace) if err != nil { - // Only a genuine "namespace has no release" maps to the §7.3 - // "runs elsewhere" rewrite; an API/RBAC list failure or an - // ambiguous multiple-release match keeps its own message. - if errors.Is(err, cluster.ErrNoParentRelease) { - return nil, &exitError{code: 4, err: &noParentReleaseError{err}} - } + // The error carries cluster.ErrNoParentRelease for a genuine + // "namespace has no release" (which explain() rewrites as §7.3 + // "runs elsewhere"); an API/RBAC list failure or ambiguous + // multiple-release match wraps a different error and passes through. return nil, &exitError{code: 4, err: err} } t := &clusterTarget{Resolved: resolved, Clientset: cs, Release: release} @@ -106,14 +93,12 @@ func bindActiveClientNamespace(opts *cluster.KubeconfigOptions) activeClientBind // explain rewrites a "no tracebloc release in namespace" failure (exit 4) into // §7.3's "client runs on another machine" guidance when the target namespace // came from the active-client binding: the cluster the kubeconfig reaches -// doesn't host that client. Non-binding errors (and PVC-missing, where the -// release *was* found) pass through unchanged. +// doesn't host that client. Gated on cluster.ErrNoParentRelease so a +// PVC-missing failure (release was found), an API/RBAC error, or an ambiguous +// multiple-release match all pass through unchanged. Callable by any command +// that binds — data ingest/list/delete and cluster info. func (b activeClientBinding) explain(err error) error { - if !b.applied { - return err - } - var npr *noParentReleaseError - if !errors.As(err, &npr) { + if !b.applied || !errors.Is(err, cluster.ErrNoParentRelease) { return err } handle := b.name diff --git a/internal/cli/clustertarget_test.go b/internal/cli/clustertarget_test.go index 18e4ac97..34853fdf 100644 --- a/internal/cli/clustertarget_test.go +++ b/internal/cli/clustertarget_test.go @@ -2,6 +2,7 @@ package cli import ( "errors" + "fmt" "strings" "testing" @@ -120,7 +121,9 @@ func TestBindActiveClientNamespace_NoActiveClient(t *testing.T) { } func TestActiveClientBinding_Explain(t *testing.T) { - noRelease := &exitError{code: 4, err: &noParentReleaseError{errors.New("no release")}} + // A discovery failure carrying the sentinel (as DiscoverParentRelease + // returns for a genuine not-found) — the case explain rewrites. + noRelease := &exitError{code: 4, err: fmt.Errorf("%w in namespace", cluster.ErrNoParentRelease)} pvcMissing := &exitError{code: 4, err: errors.New("shared PVC not bound")} // Applied + "no release here" → rewritten to the §7.3 guidance. From 17afcb222cfccd1dae50b1c6d98e264ff57a289a Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Mon, 6 Jul 2026 17:10:45 +0500 Subject: [PATCH 5/5] =?UTF-8?q?feat(cli#128):=20report=20the=20selected=20?= =?UTF-8?q?client's=20connection=20state=20on=20`client=20use`=20(=C2=A77.?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `client use` set the pointer and reported a bare "success" even for a client that isn't connected to any cluster yet, or that runs on a different machine — leaving the user to discover the problem via a later, confusing data-command failure. Now it reports the client's state non-fatally (selection still succeeds, per §7.3): - online + reachable here → "Connected on this machine — data commands will target namespace X." - online + not reachable → "runs on another machine — data commands here can't reach its cluster." - pending → "isn't connected to a cluster yet — install it on the target machine first." - offline → "currently offline — data commands will fail until it's back." Online-here vs online-elsewhere uses a bounded (3s), best-effort local cluster probe (stubbable var); the other states come from the backend status `client list` already returns. This is the §7.3 "soft warning on client use" the RFC left optional. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 57 +++++++++++++++++++++++++++++++++---- internal/cli/client_test.go | 35 +++++++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 4714e0ba..100f620c 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/spf13/cobra" @@ -641,29 +642,75 @@ func runClientUse(ctx context.Context, p *ui.Printer, handle string) error { var byID *api.ProvisionedClient for i := range clients { if clients[i].Namespace == handle { - return selectClient(p, cfg, &clients[i]) + return selectClient(ctx, p, cfg, &clients[i]) } if byID == nil && strconv.Itoa(clients[i].ID) == handle { byID = &clients[i] } } if byID != nil { - return selectClient(p, cfg, byID) + return selectClient(ctx, p, cfg, byID) } return &exitError{code: 1, err: fmt.Errorf( "no client %q in your account — run `tracebloc client list` to see the slugs and ids", handle)} } -// selectClient points this machine at c and persists the choice. -func selectClient(p *ui.Printer, cfg *config.Config, c *api.ProvisionedClient) error { +// selectClient points this machine at c, persists the choice, then reports — +// non-fatally — whether c is actually usable from here, so `client use` never +// leaves the user to discover an unreachable/unconnected client via a later +// data-command failure (§7.3). +func selectClient(ctx context.Context, p *ui.Printer, cfg *config.Config, c *api.ProvisionedClient) error { setActiveClient(cfg.Current(), c) if err := cfg.Save(); err != nil { return &exitError{code: 1, err: err} } - p.Successf("This machine is now set to enroll as client %q (namespace %s).", c.Name, c.Namespace) + p.Successf("Now using client %q (namespace %s).", c.Name, c.Namespace) + reportSelectedClientState(ctx, p, c) return nil } +// reportSelectedClientState prints an honest, non-fatal note about the selected +// client's connectivity. Backend status (from `client list`) tells us online / +// offline / pending for free; for an online client we additionally probe the +// local cluster to distinguish "connected here" from "runs on another machine". +func reportSelectedClientState(ctx context.Context, p *ui.Printer, c *api.ProvisionedClient) { + switch c.Status { + case clientStatusOnline: + if probeClientReachableHere(ctx, c.Namespace) { + p.Infof("Connected on this machine — data commands will target namespace %s.", c.Namespace) + return + } + p.Warnf("This client is online but runs on another machine — data commands here can't reach its cluster.") + p.Hintf("Run data commands where it's installed, or `tracebloc client use` a client on this machine.") + case clientStatusPending: + p.Warnf("This client isn't connected to a cluster yet.") + p.Hintf("Install it on the target machine first — data commands won't work until it's online.") + case clientStatusOffline: + p.Warnf("This client is currently offline (no recent heartbeat).") + p.Hintf("Data commands will fail until it's back online — run `tracebloc cluster doctor` to diagnose.") + } +} + +// probeClientReachableHere reports, best-effort, whether the client's namespace +// hosts its tracebloc release on the cluster the default kubeconfig points at +// ("connected here"). Bounded so `client use` never hangs on an unreachable +// cluster; any failure (no kubeconfig, unreachable, no release) → false. A +// package var so tests can stub it without a real cluster. +var probeClientReachableHere = func(ctx context.Context, namespace string) bool { + ctx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + resolved, err := cluster.Load(cluster.KubeconfigOptions{Namespace: namespace}) + if err != nil { + return false + } + cs, err := cluster.NewClientset(resolved) + if err != nil { + return false + } + _, err = cluster.DiscoverParentRelease(ctx, cs, namespace) + return err == nil +} + // setActiveClient points this env's profile at c, caching its namespace and // display name alongside the id so the data commands can bind to the active // client's cluster (§7.3) without a backend round-trip. Callers Save() after. diff --git a/internal/cli/client_test.go b/internal/cli/client_test.go index 280ee254..53093819 100644 --- a/internal/cli/client_test.go +++ b/internal/cli/client_test.go @@ -278,6 +278,41 @@ func TestClientUse(t *testing.T) { } } +// client use reports the selected client's connectivity (§7.3): online-here vs +// online-elsewhere (via the local probe), pending, and offline. +func TestClientUse_ReportsConnectionState(t *testing.T) { + cases := []struct { + name string + status int + reachable bool + wantSubstr string + }{ + {"online_here", clientStatusOnline, true, "Connected on this machine"}, + {"online_elsewhere", clientStatusOnline, false, "runs on another machine"}, + {"pending", clientStatusPending, false, "isn't connected to a cluster yet"}, + {"offline", clientStatusOffline, false, "currently offline"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + body := fmt.Sprintf(`[{"id":7,"first_name":"box","namespace":"ns7","status":%d}]`, tc.status) + withClientBackend(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + }) + orig := probeClientReachableHere + probeClientReachableHere = func(context.Context, string) bool { return tc.reachable } + t.Cleanup(func() { probeClientReachableHere = orig }) + + var out bytes.Buffer + if err := runClientUse(context.Background(), ui.New(&out), "ns7"); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), tc.wantSubstr) { + t.Errorf("want %q in output:\n%s", tc.wantSubstr, out.String()) + } + }) + } +} + // A numeric handle that is one client's slug AND another client's id must // resolve deterministically to the slug owner (slug > id), regardless of list // order — not "whichever the API returned first".