feat(cli): roll circuit-breaker state over across config wipes

[vanceingalls]

Split out of #2854 so this uncontested safety fix isn't blocked on the canary-mechanism policy discussion (Slack thread). Contains no percentage rollouts, no cohorts, no bucketing — only breaker survival and one churn-measurement property.

Problem

The DE parallel-router circuit breaker's tripped state lives in the same config file as the install id. Deleting ~/.hyperframes — the most common reset — re-enrolls the machine into an experimental path that already failed on it, and re-runs it until it fails again.

Fix

Mirror exactly two facts into a machine-local state file (~/.local/state/hyperframes/install-state.json) that a config wipe does not touch:

• deParallelRouterTrialFired — a tripped breaker stays tripped for the new install. Config corruption takes the same mint path, so it survives that too (while preserving main's fail-closed telemetryEnabled: false recovery semantics).
• markerAt — written unconditionally, so the fraction of fresh mints that find it directly measures recoverable id churn (config wiped, machine persisted) vs unrecoverable (fresh machine/container/new user). Emitted as install_predecessor_found on telemetry events; absent (not false) on configs predating the field.

The file deliberately holds no identity — no anonymousId, no counters (asserted by test: file keys are exactly those two, and the anonymousId never appears in the bytes). A wiped config still gets a fresh id unconditionally; only the safety fact about the machine survives.

Mechanics

• Sync happens inside writeConfigWithResult, so no breaker write site can forget it; memoized per process.
• Atomic tmp+rename, same rationale as the config write: a torn read must never exist since a corrupted state file reads as absent.
• Failures swallowed (telemetry must never break the CLI) but leave the memo unset so a later write retries.
• hyperframes telemetry status lists the state path for transparency.

Validation

10 new tests: wipe survival, corruption survival (including the fail-closed telemetry interaction), untripped-breaker-not-invented, no-identity assertion, marker written once, state-write failure never breaks the config write, absent-vs-false on legacy configs. Full CLI suite green except dependency-compat.test.ts, which fails identically on clean main (pre-existing).

This also unblocks the planned short-comp DE inversion ship (bench results in the DE plans dir): that change reuses the trial+breaker pattern at full exposure, and its breaker needs to survive wipes for the newly exposed population.

🤖 Generated with Claude Code

[james-russo-rames-d-jusso]

Reviewed at dfe92b2.

Clean split of the safety subset out of #2854. The rollover mechanics land correctly and the test coverage is genuinely thorough — the "predecessorFound absent-vs-false-vs-true trichotomy," the state-write-failure-doesn't-break-config-write path, and the anonymousId-never-appears-in-bytes assertion are all the right shape. The writeConfigWithResult sync point closes the "no breaker write site can forget" contract exactly where you want it. LGTM overall — two things worth surfacing:

Concerns

State-file corruption silently drops a tripped breaker. The it("a corrupted state file reads as absent rather than breaking the mint", ...) test on packages/cli/src/telemetry/config.test.ts demonstrates the recovery path: parse fails → readInstallState() returns null → the mint sees no predecessor, restores no breaker, and writes a fresh marker over the corrupted bytes. Config corruption is protected because it goes back through the same mint path that reads state; but a corrupted state file loses the very fact this feature exists to preserve, and there's no user-facing signal. This is a rare failure mode (atomic tmp+rename, owner-only 0o700 dir + 0o600 file, small payload), and the fail-closed alternative (keep breaker tripped forever when the state file breaks) has worse trade-offs than the fail-open one you picked. Not a blocker, and probably not worth code-changing — but two mitigations worth considering:

• Log a warning to stderr when readInstallState() throws (currently catch {} swallows) so an operator debugging "why did my breaker re-trip?" has a breadcrumb.
• Alternatively, in the telemetry-emitted install_predecessor_found, distinguish "state-file-absent" from "state-file-corrupted" (a third predecessorReadError bucket, or something equivalent). Downstream fleet analysis can then spot a corruption incident, rather than it hiding inside the "fresh mint" fraction.

Stack coupling to #2854 in the identity-free assertion. Line 214-ish in the new test block: expect(Object.keys(stateFile()).sort()).toEqual(["deParallelRouterTrialFired", "markerAt"]) — strict-equals-two-keys. When #2854 lands on top of this (adding bucketSeed), the assertion has to move to three keys and its "identity-free" spirit needs re-verifying (the assertion is currently "these are the fields," not "these fields aren't identity-flavored"). Not a defect for this PR — the split cleanly leaves that decision to #2854 — but flagging so the follow-up rebase doesn't slip the semantic check.

Nits

• __resetInstallStateSyncForTests is public API by way of the barrel. The double-underscore prefix marks intent, but production callers can reach it. Not worth restructuring; noted only.
• On Windows and macOS, ~/.local/state/hyperframes/install-state.json is nonstandard (XDG_STATE_HOME convention is Linux-native). This is symmetric with ~/.hyperframes/config.json also being non-XDG on Linux, so the "survives the common wipe" invariant holds on every platform the config wipe is documented for — the state simply lives at an obscure location on all three OSes. Leaving as-is is defensible; a follow-up that adopts env-paths or similar would only matter if you want cross-platform docs to name-check the state path.

What I didn't verify

• Whether any downstream tooling (Grafana dashboard, PostHog board) already keys off install_predecessor_found — the field wiring in client.ts is correct, but I didn't confirm the receiving end recognizes it.

— Review by Rames D Jusso

[miga-heygen]

SSOT -- Single Source of Truth

Verified: Two storage locations exist for deParallelRouterTrialFired (config file + state file), but the decision flow is unidirectional and the authority is clear:

• Runtime: the config file is authoritative. The state file is never consulted during normal operation.
• Mint (new/missing/corrupt config): mintConfig() consults the state file exactly once to roll over the breaker state. This is the sole read site for rollover.
• Sync (config → state): syncInstallState() is called from writeConfigWithResult(), which is the single write path. Every writeConfig call site in feedback, autoUpdate, client, and updateCheck flows through this funnel.
• nextInstallState: Correctly returns null (skip write) when the state file already reflects reality. Four cases verified: state absent → write marker, state present + already fired → skip, state present + not fired + don't want fired → skip, state present + not fired + want fired → upgrade.
• predecessorFound: Set once during mintConfig(), derived from state !== null, persisted in config. Never mutated afterward. Clean single-assignment fact.
• Memoization: stateMarkerSynced and stateFiredSynced correctly prevent redundant reads. Memos stay unset on failure so the next writeConfig retries. Early-return condition is correct for all state combinations.

No SSOT findings.

---

Correctness

readInstallState shape validation — all three failure modes return null:

• Missing file: existsSync check. ✓
• Corrupt JSON: JSON.parse throw caught. ✓
• Valid JSON, wrong shape: typeof parsed.markerAt !== "string" guard. ✓
• deParallelRouterTrialFired normalization: === true ? true : undefined — only propagates an explicit true. ✓

mintConfig():

• predecessorFound: state !== null — correctly distinguishes predecessor from fresh. ✓
• deParallelRouterTrialFired: state?.deParallelRouterTrialFired === true ? true : undefined — never invents a false. ✓

Corruption recovery path: Changed from inline to { ...mintConfig(), telemetryEnabled: false }. Routes corruption through mintConfig() so a tripped breaker survives config corruption. The telemetryEnabled: false override is preserved (fail-closed for privacy). If the state file is also corrupt, readInstallState() returns null → treats as fresh. No cascading failure. ✓

predecessorFound parsing: typeof parsed.predecessorFound === "boolean" ? parsed.predecessorFound : undefined — correctly handles the tristate: true (predecessor), false (fresh), undefined (legacy config predating the field). ✓

No correctness findings.

---

State management

• Atomic write: tmp+rename with ${STATE_FILE}.${process.pid}.tmp. Consistent with the existing config write pattern. ✓
• Write-once marker: state?.markerAt ?? new Date().toISOString() — existing timestamp preserved, new one created only on first write. Test confirms across multiple config writes. ✓
• Sync memo retry: Memos set AFTER write succeeds (inside try). Failure leaves memos unset → next writeConfig retries. Process crash between write and memo-set re-reads from disk on next process start (memos are in-process only). ✓
• Module-level memos: persist for process lifetime; external state file deletion not detected until restart. Acceptable — memos exist to prevent redundant I/O, and external deletion is not a supported path. ✓

No state management findings.

---

Security / Privacy

• No identity in state file: InstallState holds exactly markerAt (string) and deParallelRouterTrialFired (optional boolean). No anonymousId, no counters.
• File permissions: 0o700 dir, 0o600 file. Owner-only access. ✓
• Test assertion: Object.keys exactly ["deParallelRouterTrialFired", "markerAt"], plus JSON.stringify does not contain the anonymousId. ✓
• Privacy invariant: A config wipe gives a fresh id unconditionally. The state file carries only machine-level safety facts. Rollover cannot link old and new installs.

No security findings.

---

Standards

• as Partial<InstallState> after JSON.parse followed by shape validation — acceptable per project conventions.
• No bare as T without validation in production code. ✓
• No non-null assertions. ✓
• Conventional commit format (feat(cli):). ✓

---

Side effects

• syncInstallState is called from exactly one site (writeConfigWithResult), after the config write completes and cachedConfig is updated.
• Never throws (wrapped in try/catch) — state file I/O failures cannot propagate to callers or corrupt the config write.
• install_predecessor_found in trackEvent executes after the shouldTrack() guard and uses cached config (no disk I/O).
• Memoization prevents redundant I/O; only a breaker transition from untripped → tripped triggers a second sync.

No side-effect findings.

---

Tests

10 new tests covering all critical paths:

Test	Covers
Fresh install: predecessorFound false, marker written	Happy path
Config wipe: predecessorFound true, fresh anonymousId	Wipe survival + identity isolation
Tripped breaker survives config wipe	Core feature
Untripped breaker not invented by rollover	No false positives
Tripped breaker survives config corruption	Corruption recovery
State file holds no identity	Privacy invariant
Marker written once, not refreshed	Write-once semantics
Corrupted state file reads as absent	State file corruption
State-file write failure never breaks config write	Failure isolation
Legacy config: predecessorFound undefined, not false	Backward-compat tristate

No significant missing edge cases.

---

CI

All checks pass (44 completed: 37 success, 7 expected-skipped).

---

Nit

hadFired in nextInstallState's return expression (wantFired || hadFired || undefined) is always false when that line executes, because the early-return above catches all hadFired === true cases. Dead logic — defensively correct but misleading. Could simplify to wantFired || undefined.

---

Verdict

Approve. Minimal, precise state file design — carries exactly two facts (marker + breaker) without leaking identity. Config is authoritative at runtime, state file is a write-behind safety backup consulted only during mint, and the sync funnel through writeConfigWithResult ensures no write site can forget. Strong test coverage across all critical paths including corruption and failure isolation.

Builds naturally on the circuit-breaker state machine from #2840. Ready to ship.

-- Miga

[miguel-heygen]

R1 at dfe92b2aabcea8d934e4696f9dfe063307fccdf7 — clean.

I independently traced the storage boundary and write graph. Runtime decisions remain config-authoritative; the machine-local state is consulted only when minting after a missing/corrupt config, and every breaker transition reaches it through writeConfigWithResult. The state payload is limited to the marker plus an explicit tripped fact, preserves fresh-ID semantics, uses owner-only atomic writes, and leaves the memo unset after I/O failure so a later config write retries. The corruption path also preserves the existing telemetry fail-closed override.

The 10 focused cases pin the important invariants: wipe/corruption survival, no invented breaker, identity absence in bytes, write-once marker, failed state sync isolation, and the legacy absent-vs-false distinction. Exact-head CI is complete: 36 success, 10 expected skips, 0 failures.

Non-blocking nit: hadFired is unreachable as true in nextInstallState's returned object after the early return, so wantFired || hadFired || undefined can be simplified.

Verdict: APPROVE
Reasoning: The carryover is narrowly scoped, identity-free, correctly funneled, failure-isolated, and fully green at the reviewed head.

— Magi

[miguel-heygen]

R2 at exact head ec76985f4072469762388739f16b1c6904e84519 (delta from approved R1 dfe92b2a).

The only delta is the requested cleanup in packages/cli/src/telemetry/config.ts:88-98: nextInstallState now removes the unreachable hadFired merge term after the preceding guard has already excluded every hadFired === true path. The added comment accurately pins that control-flow invariant; marker preservation, one-way breaker promotion, retry-on-write-failure behavior, and the identity-free state shape are unchanged.

Exact-head CI is fully terminal and green, including CLI/unit/typecheck/Windows and all 9 regression shards.

Verdict: APPROVE
Reasoning: The previously approved carryover mechanism is unchanged, the sole delta correctly resolves the dead-logic review nit, and every exact-head gate passes.

— Magi

[vanceingalls]

Belated reply — this merged without one. Both reviewers flagged the same dead branch in nextInstallState: wantFired || hadFired || undefined, where hadFired can never be true at that point because the guard above returns early whenever it is. Simplified to wantFired || undefined in ec76985f4, with a comment naming why the merge isn't needed.

The other three suggestions I left alone deliberately, all non-blocking:

• stderr warning on corrupt state file and a predecessorReadError telemetry bucket (@james-russo-rames-d-jusso) — both reasonable; I skipped them because the failure mode is rare (atomic tmp+rename, 0o600) and I'd rather add the bucket if the fleet data shows corruption actually happening than pre-emptively widen the field's cardinality.
• ~/.local/state being non-XDG on macOS/Windows — agreed it's odd, but as you noted it's symmetric with ~/.hyperframes already being non-XDG on Linux, and the invariant that matters (survives the documented config wipe) holds on all three.
• __resetInstallStateSyncForTests reachable via the barrel — noted, not worth restructuring.

For the record on how the follow-up landed: the identity-free assertion you flagged as stack-coupled did need updating when #2875 added bucketSeed — it went to three keys, and I kept the spirit by additionally asserting the seed differs from anonymousId and that the id never appears in the file's bytes.