fix(cli): diagnose before advising — a wrong active-client pointer has a way back (cli#515)

.cursor/BUGBOT.md

@@ -73,6 +73,27 @@ Two things make this repo unusual and should shape every finding:
  (`internal/api/client.go`, `nextPath`). Where "empty" and "unknown" are different answers,
  prefer a three-valued return (`internal/cluster/discover.go:302`).
 
+- **"Couldn't confirm" used as "confirmed" — in a BRANCH, not just a return type.** The rule
+ above is about the value a function hands back; this is about the `if` that consumes it, and
+ it is the defect this repo produces most often. cli#515 shipped it three times in one PR,
+ each in a different file, each after the previous one was fixed: a failed cluster scan
+ reported as "no client is running here"; an empty `cluster_id` on a legacy client read as
+ "runs elsewhere" (`ProvisionedClient.ClusterID` is documented empty on not-yet-backfilled
+ records); and `reachStateOf(x) != ReachNoEnv` used to mean "an environment is here", when
+ `ReachState` also has `ReachUnreachable` and `ReachError`. Two concrete shapes to flag:
+ - **A negated comparison against ONE member of a multi-valued enum.** `!= ReachNoEnv`,
+ `!= StatusFail` and friends silently include every member added later. Compare against the
+ member you actually require (`== ReachOK`), and derive the test's input domain from the
+ enum's declared surface — mutation coverage cannot see a vocabulary gap.
+ - **A lenient "not found" default reused where the question is "may I believe this?"**
+ `reachStateOf` returns `ReachOK` for an ABSENT check, which is right for a verdict roll-up
+ and wrong for authorising a claim — hence the separate `reachConfirmedOK`
+ (`internal/cli/doctor.go`). The same default is rarely correct for both.
+
+ The customer-visible cost is never a wrong log line: on #515 each instance ended in advice to
+ run `client create` on a cluster nothing was confirmed on, where it MINTS rather than adopts —
+ i.e. the guidance manufactured the orphaned phantom of `backend#970`.
+
 - **A cross-repo contract change that only lands on one side.** `scripts/.data-ingestors-ref`,
  `scripts/.client-ref` and `scripts/.backend-ref` pin upstream refs deliberately so an
  unrelated upstream commit can't red every open PR. Flag a hand-edit to a generated artifact

VERSION

@@ -1 +1 @@
-0.10.8
+0.10.9

docs/cli-navigation.md

@@ -22,7 +22,7 @@ flowchart TD
  ACCT --> logout["logout"]
  ACCT --> authst["auth status"]
  ACCT --> clis["client status"]
- ACCT -.-> clcreate["client create"]:::hidden
+ ACCT --> clcreate["client create — point this machine at its client"]
  ACCT -.-> cllist["client list"]:::hidden
 
  ENVC --> di["data ingest"]
@@ -172,7 +172,7 @@ flowchart TD
 - **not signed in / token 401·403** → `login`
 - **426 upgrade-required** → upgrade the CLI
 - **kubeconfig (exit 3)** → fix `--kubeconfig`/`--context`, then `doctor`
-- **no client / environment (exit 4)** → run the installer (or `--namespace`); triage with `doctor`
+- **no client / environment (exit 4)** → the error now says what IS on the reached cluster before advising (cli#515): one client on a local cluster → `client create` repoints this machine (it adopts, no new credential); a client on a remote/shared cluster → `--namespace <ns>` only; nothing there → run the installer. Triage with `doctor`
 - **no token (exit 5)** → grant RBAC; diagnose with `cluster info` / `doctor`
 - **destination exists (exit 6)** → `--overwrite`, a different `--name`, or `data delete` first
 - **staging partial (exit 7)** → `data delete` then re-ingest

internal/cli/client.go

@@ -39,8 +39,9 @@ var readInClusterClient = cluster.DiscoverInClusterClient
 // The single-machine CLI (RFC-0001 §7.10) owns exactly one client, so there is
 // nothing to *select*: `client use` is withdrawn, and `client list` is hidden
 // (kept callable for the installer's one-client-per-machine pre-flight, off the
-// user-facing surface). `create` provisions this machine's client; offboarding
-// is the top-level `tracebloc delete`.
+// user-facing surface). `create` points this machine at its client — adopting
+// the one already on the cluster, which is the supported repoint (#515) —
+// and offboarding is the top-level `tracebloc delete`.
 func newClientCmd() *cobra.Command {
 	cmd := &cobra.Command{
  Use: "client",
@@ -63,15 +64,31 @@ func newClientCreateCmd() *cobra.Command {
 	var yes bool
 	cmd := &cobra.Command{
  Use: "create",
- Short: "Provision a tracebloc client for this machine (auto-named; no flags required)",
- // HIDDEN: provisioning is the installer's job — provision.sh calls this with
- // zero flags (cli#137). It stays fully callable (including `--help`, so the
- // installer's capability probe still works), but is kept off the user-facing
- // surface: a human running `client create` STANDALONE mints a client the
- // installer never deploys — an orphaned "phantom" (backend#970). Mirrors the
- // hidden `list`; leaves `tracebloc client` showing only the user-useful `status`.
- Hidden: true,
- Args: cobra.NoArgs,
+ Short: "Point this machine at its tracebloc client — adopts the one already on this cluster",
+ Long: `Point this machine at its tracebloc client.
+
+Keyed on the cluster your kubeconfig reaches: if a tracebloc client already runs
+there, this ADOPTS it — no prompt, no new credential, no duplicate — which is how
+you repoint a machine whose active client went stale. On a cluster that runs no
+client yet it provisions a new one, and asks first.
+
+Provisioning a brand-new machine is normally the installer's job — it calls this
+for you, with no flags.`,
+ // WAS HIDDEN (backend#970), and the reason still stands: a human running
+ // this STANDALONE on a cluster with no client mints one the installer never
+ // deploys — an orphaned "phantom". Hiding it was never what prevented that,
+ // though; the mint-path guards below are, and they are untouched:
+ // • on a TTY, the review + `Provision this client?` confirm (which a
+ // re-run on an already-registered cluster never reaches — it adopts
+ // before the prompt, so the repoint stays zero-friction);
+ // • off a TTY, a hard refusal without --yes/--credential-file, so a pipe
+ // or CI can never mint silently.
+ // What hiding DID cost is #515: the §7.3 "your active client runs on another
+ // machine" error had no supported way back, because the one command that
+ // repoints a machine was unlisted. Advice pointing at a hidden command is
+ // not advice, so it is listed now — described by what it does for a user
+ // (adopt/repoint) rather than by the installer's use of it.
+ Args: cobra.NoArgs,
  RunE: func(cmd *cobra.Command, _ []string) error {
  return runClientCreate(cmd.Context(), printerFor(cmd), clientPrompter(),
  clientCreateOpts{name: name, location: location, kubeconfigPath: kubeconfigPath, contextOverride: contextOverride, credentialFile: credentialFile, yes: yes})
@@ -747,21 +764,106 @@ func runClientList(ctx context.Context, p *ui.Printer) error {
 	}
 	p.Section("Clients in your account")
 	active := cfg.Current().ActiveClientID
+	// #515: "active" is a LOCAL POINTER, and this listing used to render it as
+	// "(active — this machine)" — a claim about location it never checked. When
+	// the pointer is stale that label sits next to a client provably not on the
+	// cluster this machine reaches, which is the state the whole ticket is about.
+	// Read the cluster anchor (kube-system UID, the §7.2 identity `client create`
+	// keys on) and mark residency separately from selection. A failed read is
+	// three-valued on purpose: unknown is not "elsewhere", so the marker then
+	// claims nothing about where anything runs.
+	clusterID, cidErr := readClusterID(ctx, cluster.KubeconfigOptions{})
+	hereKnown := cidErr == nil && clusterID != ""
+	activeElsewhere, anyHere := false, false
 	for _, c := range clients {
- marker := ""
- if strconv.Itoa(c.ID) == active {
- marker = " (active — this machine)"
+ isActive := strconv.Itoa(c.ID) == active
+ res := residencyOf(hereKnown, clusterID, c.ClusterID)
+ if isActive && res == resElsewhere {
+ activeElsewhere = true
  }
- p.Field(strconv.Itoa(c.ID)+marker,
+ if res == resHere {
+ anyHere = true
+ }
+ p.Field(strconv.Itoa(c.ID)+clientListMarker(isActive, res),
  fmt.Sprintf("%s state=%s namespace=%s location=%s",
  c.Name, clientStateLabel(c.Status), c.Namespace, c.Location))
 	}
 	// §7.3: separate "selected" (this machine's local pointer) from "connected"
 	// (the backend's last-heartbeat state) so a stale pointer is visible.
 	p.Hintf("\"active\" is this machine's selected client; state is its last reported status to tracebloc.")
+	switch {
+	case activeElsewhere && anyHere:
+ // The exact state #515 describes, and the one supported way out of it:
+ // re-running create on a cluster that already hosts a client adopts it —
+ // no prompt, no new credential (§7.2). anyHere is what earns the phrase
+ // "the client that IS there": without a row we KNOW is on this cluster,
+ // `client create` would fall through to the mint path and produce the
+ // phantom backend#970 is about (Bugbot).
+ p.Hintf("Your active client is not on the cluster your kubeconfig reaches. To point this machine at the client that IS there: %s client create", launcher())
+	case activeElsewhere:
+ // The pointer is provably wrong, but no listed client is provably here —
+ // either none is, or the ones that might be carry no anchor to prove it
+ // (resUnknown). Both are "we can't name a target", so name none: send
+ // them to the command whose whole job is to say what's on this cluster
+ // rather than advertise a repoint that may have nothing to adopt.
+ p.Hintf("Your active client is not on the cluster your kubeconfig reaches, and no client here is confirmed. Check your kubeconfig context, then run: %s doctor", launcher())
+	}
 	return nil
 }
 
+// residency answers "does this client run on the cluster the kubeconfig
+// reaches" in THREE values, because two of them are absences and an absence is
+// never a "no" (Bugbot, #515).
+type residency int
+
+const (
+	// resUnknown: we cannot tell. Either the local cluster anchor was
+	// unreadable (no kubeconfig, unreachable API server, RBAC on kube-system),
+	// or the CLIENT carries no anchor — `ProvisionedClient.ClusterID` is empty
+	// on legacy / not-yet-backfilled records (api/client.go), and a record that
+	// never learned where it lives is not a record that lives elsewhere.
+	resUnknown residency = iota
+	resHere
+	resElsewhere
+)
+
+// residencyOf compares the local cluster anchor with a client's, keeping both
+// missing-anchor cases at resUnknown. Collapsing either into "elsewhere" would
+// print "NOT on the cluster your kubeconfig reaches" — and the repoint hint —
+// next to a legacy client that may be running on this very machine.
+func residencyOf(hereKnown bool, localAnchor, clientAnchor string) residency {
+	if !hereKnown || clientAnchor == "" {
+ return resUnknown
+	}
+	if clientAnchor == localAnchor {
+ return resHere
+	}
+	return resElsewhere
+}
+
+// clientListMarker renders one row's suffix in `client list`, keeping SELECTION
+// (this machine's local pointer) and RESIDENCY (where the client actually runs)
+// as two separate facts (#515). Under resUnknown the marker degrades to bare
+// "(active)" — which says only what the local config actually knows — and
+// claims nothing about location in either direction.
+func clientListMarker(isActive bool, res residency) string {
+	switch {
+	case res == resUnknown:
+ if isActive {
+ return " (active)"
+ }
+ return ""
+	case isActive && res == resHere:
+ return " (active — on this cluster)"
+	case isActive:
+ return " (active — NOT on the cluster your kubeconfig reaches)"
+	case res == resHere:
+ return " (on this cluster)"
+	default:
+ return ""
+	}
+}
+
 // 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.