fix(client create): don't mint over a live client when the cluster UID read fails (#158)

[saadqbal]

Closes #158. Fixes the High-sev R7 gap Cursor Bugbot flagged on the promote PR (#155, internal/cli/client.go:267).

Problem

The §7.2 adopt-backfill ran only inside if clusterID != "". readClusterID (kube-system UID) needs a reachable cluster and RBAC on namespaces/kube-system. If it failed for a reason other than unreachability — RBAC forbidden, transient API error — with a tracebloc client already live on the cluster, the whole adopt block was skipped and client create minted a duplicate, orphaning the live client (worst under --yes).

Fix

adoptLiveInClusterClient already detects the live client via readInClusterClient, independently of the UID — only the backfill needs it. So:

• Run adopt unconditionally (drop the clusterID != "" gate).
• When clusterID == "" but a live owned client is found: adopt it as-is — no backfill PATCH, no anchor-mismatch check — and warn that the idempotency anchor was left unstamped (point --kubeconfig/--context at a cluster where kube-system is readable to stamp it). No mint.
• Existing guards intact: listErr fail-closed, cross-account refusal, different-cluster mismatch (only meaningful when the UID is known).
• Genuinely-unreachable cluster fails detection too → non-anchored mint, unchanged.

Tests

• New: UID read fails + live owned client ⇒ pure adopt (no POST, no PATCH, active client = the live one).
• Stubbed readInClusterClient in the cancel test that bypasses withClientBackend (adopt now always calls it).
• Full go build/vet/test ./... green.

Once this lands on develop I'll re-cut the promote (#155) so main gets it and the Bugbot thread there resolves.

---

Note

Medium Risk
Changes provisioning idempotency when cluster UID is unreadable but a live client exists; wrong adoption could affect the wrong machine credential, though ownership checks and no-mint-over-live invariant remain.

Overview
Fixes #158: when kube-system UID read fails (RBAC, transient errors) but a tracebloc client is already running, client create no longer skips R7 adopt-backfill and mints a duplicate.

runClientCreate always calls adoptLiveInClusterClient (removed the clusterID != "" gate). Live-client discovery uses readInClusterClient and does not depend on the anchor; only backfill needs the UID.

adoptLiveInClusterClient adds a clusterID == "" branch: if an owned in-cluster client exists, adopt it without PatchClientClusterID or anchor-mismatch checks, and hint that the idempotency anchor was left unstamped. Cross-account refusal and list fail-closed behavior are unchanged.

Tests add TestClientCreate_R7_UIDReadFailsAdoptsLive and stub readInClusterClient in the verbose install cancel test so adopt does not hit a real kubeconfig.

^{Reviewed by Cursor Bugbot for commit 97c745a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[aptracebloc]

cc @saadqbal @LukasWodka — post-merge review. The core fix is correct and safe to keep: live-client discovery genuinely doesn't need the UID, the switch-case ordering is right (empty-UID case before backfill/mismatch), the existing guards hold on the new path (listErr fail-closed fires before the switch, cross-account refusal is UID-independent, unreachable cluster still falls through to the unchanged non-anchored mint), and the verbose_install_test.go stub addition is exactly right. Nothing here needs a revert — but a few things are worth a follow-up.

🟡 Medium: the UID-fail path silently masks the different-cluster mismatch guard

When the UID is readable and the live client is anchored to a different cluster, the code hard-errors ("anchored to a different cluster … check you're targeting the right cluster", client.go:525-531). When the UID read fails, the new clusterID == "" case adopts as-is without inspecting owner.ClusterID at all — so the same cluster state silently adopts on the UID-fail path but hard-errors on the UID-success path.

Concretely: a client Deployment restored from backup onto a rebuilt cluster still carries the old cluster's anchor in the backend. The failure mode that causes the UID read to fail (broken RBAC after a cluster rebuild) correlates with exactly that restored-cluster scenario the mismatch guard exists for.

Cheap mitigation: in the new case, when owner.ClusterID != "", strengthen the warning — "this client is anchored to a cluster identity that couldn't be verified against this cluster" — or require confirmation instead of adopting under --yes.

⚪ Low: contradictory user-facing hints in the adopt-without-backfill flow

When cidErr != nil, runClientCreate prints "Couldn't read the target cluster's identity — provisioning without a cluster anchor, so re-running won't be idempotent" (client.go:194) before adopt runs. If adopt then succeeds, the user next sees "already running on this cluster — adopting it…". The first hint is now wrong in that flow: nothing is provisioned, and re-running is idempotent (it re-adopts). Pre-existing message, but this PR created the path where it lies — moving it after the adopt attempt (or printing it only on the mint path) would fix it.

⚪ Low: test coverage gaps on the newly-reachable paths

The new test covers the happy path well (no POST, no PATCH, active client adopted). But the gate removal makes two other paths newly reachable with changed behavior, neither pinned by a test:

• clusterID == "" + listErr != nil + live client → now a fail-closed error where it previously minted. Deliberate and good, but it's a behavior change hiding in the gate removal — deserves a pinning test.
• clusterID == "" + live client owned by a different account → cross-account refusal on the new path.

Also minor: the test captures out but never asserts the new Hintf text, and cfg, _ := config.Load() swallows the error (matches surrounding test style, admittedly).

Nits

• Unreachable-cluster latency: previously a UID-read failure short-circuited the adopt block; now readInClusterClient runs its own doomed API round-trip (a second timeout) before the non-anchored mint. Harmless, but a truly unreachable cluster now waits twice on create.
• The hint's "point --kubeconfig/--context at a cluster where kube-system is readable" reads as "use a different cluster" — the actual remedy is usually fixing RBAC on this cluster and re-running (which then correctly hits the backfill case). Wording nit.

What's good

• Minimal, well-scoped diff; the fix strictly expands adoption and cannot introduce a new mint-over-live path.
• The block comment explaining the incident, the invariant, and the unreachable-cluster fallthrough is exactly the right level of documentation.
• Catching the non-obvious test side effect (adopt now always calls readInClusterClient → the cancel test needed a stub so it doesn't touch the developer's real kubeconfig) rather than letting the test silently depend on the host environment.

🤖 Generated with Claude Code