fix(cli#128): client-lifecycle follow-ups from live testing (§7.3)

internal/cli/client.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/spf13/cobra"
 
@@ -91,9 +92,12 @@ func newClientListCmd() *cobra.Command {
 
 func newClientUseCmd() *cobra.Command {
 	return &cobra.Command{
- Use: "use <client-id>",
- Short: "Enroll this machine as an existing client",
- Args: cobra.ExactArgs(1),
+ Use: "use <client>",
+ 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])
  },
@@ -573,6 +577,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 {
@@ -613,7 +623,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}
@@ -622,18 +634,81 @@ func runClientUse(ctx context.Context, p *ui.Printer, id string) error {
 	if err != nil {
  return &exitError{code: 1, err: err}
 	}
-	for _, c := range clients {
- if strconv.Itoa(c.ID) == id {
- 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)
- 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(ctx, p, cfg, &clients[i])
+ }
+ if byID == nil && strconv.Itoa(clients[i].ID) == handle {
+ byID = &clients[i]
  }
 	}
+	if byID != nil {
+ return selectClient(ctx, p, cfg, byID)
+	}
 	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)}
+}
+
+// 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("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
@@ -645,6 +720,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) {

internal/cli/client_test.go

@@ -251,17 +251,82 @@ 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)
+	}
+
+	// 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")
+	}
+}
+
+// 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())
+ }
+ })
 	}
-	if err := runClientUse(context.Background(), ui.New(&bytes.Buffer{}), "99"); err == nil {
- t.Error("expected an error for an unknown client id")
+}
+
+// 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())
 	}
 }
 

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}
+	binding := 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
@@ -150,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

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

internal/cli/clustertarget_test.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -18,6 +19,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",
@@ -85,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.