fix(cli): offline-vs-no-env classifier keys on the probe name, not the remembered label (bugbot #266)#274
Conversation
…red label #267 made resolveHomeModel overwrite env.name with the remembered client name for display. But the offline-vs-no-env classifier reads `env.name != ""` to mean "the probe surfaced an environment name" — so a leftover ActiveClientName/ID with no cached namespace (provisioned=false) now masquerades as a reachable environment and renders Offline + full menu instead of the no-environment installer path. Capture the probe's own surfaced name (probeNamedEnv) BEFORE the display-name override and classify on that: `provisioned || probeNamedEnv`. A remembered display label with no namespace is no longer evidence of an environment. Adds a regression test for the leftover-name-without-namespace case. Fixes the Cursor Bugbot "Remembered name skews offline detection" finding on #266 (learned rule: commands targeting the active client must use bindActiveClientNamespace). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👋 Heads-up — Code review queue is at 36 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
aptracebloc
left a comment
There was a problem hiding this comment.
Verified the fix — approving.
probeNamedEnv := env.name != "" is captured before the display-name override, and the classifier now keys on provisioned || probeNamedEnv. That correctly decouples the classification signal from the display label: the remembered name still drives what's shown, but only a cached namespace (provisioned) or the probe's own surfaced name count as evidence of a reachable environment. So a stale ActiveClientName/ID with no cached namespace now falls through to the no-env installer path instead of Offline. TestResolveHomeModel_LeftoverNameNoNamespaceIsNoEnv is a proper mutation guard. This is the right structural fix (not just a patch) — it restores #244's "namespace is the signal" intent while keeping #267's display improvement.
Caveat / process note: the home-screen state machine (from #244) has now taken a few follow-on fixes off the #266 promote chain (#267 label unification → this classifier fix). Each is well-tested and it's converging, but the recurring theme is env.name being overloaded for both display and classification. Before #266 promotes, it's worth a quick holistic once-over of resolveHomeModel as a whole to confirm no other display/classification coupling is lurking — cheaper to catch now than as another promote-chain follow-up.
— drafted with Claude (Opus 4.8), sent by @aptracebloc
What
Follow-up to #267. Cursor Bugbot flagged (learned rule "commands targeting the active client must use bindActiveClientNamespace"):
resolveHomeModel's classifier usesprovisioned || env.name != "", whereenv.name != ""is meant to signal "the probe surfaced an environment name." #267 madeenv.namecarry the remembered display label, so a staleActiveClientName/IDwith no cached namespace now flips the no-env installer path to Offline.Fix
Capture the probe's own surfaced name (
probeNamedEnv) before the display-name override, and classify onprovisioned || probeNamedEnv. The remembered label still drives the display name; it no longer counts as evidence of a reachable environment.(In the real flow the probe only surfaces a name on
localLive/localDegraded, never on thelocalNoRelease/localUnreachablebranch this classifier runs on — so this restores the intended "namespace is the signal" behavior while preserving the synthetic probe-surfaced-a-name case the existing tests pin.)Verification
gofmt+go vetclean;go test ./internal/cli/green.TestResolveHomeModel_LeftoverNameNoNamespaceIsNoEnv(leftover name, no namespace →homeNoEnv) — fails on current develop (env.name="stale-id"→ Offline), passes on the fix. All existing state-machine tests still pass.Once merged to
develop, promote PR #266 picks it up.🤖 Generated with Claude Code
Note
Low Risk
Narrow CLI home state-machine fix with a dedicated regression test; no auth, data, or cluster mutation paths.
Overview
Fixes a regression where the bare
traceblochome screen could show Offline with the full menu when config still had a staleActiveClientName/ActiveClientIDbut no cachedActiveClientNamespace. After the display-name override,env.namewas non-empty, soprovisioned || env.name != ""treated that as evidence of an environment.resolveHomeModelnow recordsprobeNamedEnvfrom the cluster probe before applying the remembered display label, and usesprovisioned || probeNamedEnvfor the offline vs no-environment branch. Remembered names still driveenvNamefor display; they no longer affect classification.Adds
TestResolveHomeModel_LeftoverNameNoNamespaceIsNoEnvso leftover name without namespace maps tohomeNoEnv(installer path), nothomeOffline.Reviewed by Cursor Bugbot for commit 0f791a4. Bugbot is set up for automated code reviews on this repo. Configure here.