Skip to content

fix(state): token rotation honors PILOT_PROJECT_STATE and never fails silently#34

Merged
lesquel merged 1 commit into
mainfrom
fix/state-token-rotation
May 17, 2026
Merged

fix(state): token rotation honors PILOT_PROJECT_STATE and never fails silently#34
lesquel merged 1 commit into
mainfrom
fix/state-token-rotation

Conversation

@lesquel

@lesquel lesquel commented May 17, 2026

Copy link
Copy Markdown
Owner

Summary

Resolves the Tier-1 state/token item of #33 (second-wave audit). Verified by hand and convergent across two independent review passes (domain-logic + silent-failures).

updateStateToken (core/state/store.ts) had two defects:

  1. Mode ignored — called writeState with the default mode="auto", ignoring PILOT_PROJECT_STATE. With off + an existing .opencode/, a token rotation could create the project pilot-state.json the user explicitly opted out of.
  2. Silent skipif (existing) with no else/log. When readState returned null (state file deleted/unreadable mid-session), rotation returned 200, updated the in-memory token, broadcast SSE — but the persisted state kept the old token, so the TUI /remote later served a dead URL. Invisible, violating AGENTS.md §"No silent failures".

Fix (core/ stays pure — observability at the boundary)

  • updateStateToken(dir, token, mode = "auto"): UpdateTokenResult — threads mode into writeState; returns a typed discriminated result { ok:true; write: WriteStateResult } | { ok:false; reason:"no-existing-state" }. No logger/HTTP import added to core/.
  • rotateAuthToken passes deps.config.projectStateMode and inspects the result. On no-existing-state or a global write failure it emits deps.logger.warn(...) + deps.audit.log("auth.token.rotated.persist_failed", { reason }). HTTP stays 200 (in-memory rotation + SSE are the real success path) — the persistence gap is now observable, not silent.

Testing

  • +10 tests. Deterministic null-path coverage: tests isolate XDG_STATE_HOME to a fresh temp dir (verified that's the env governing globalStatePath(), not XDG_CONFIG_HOME) so readState→null is forced and asserted unconditionally — not shape-only. Env restored in afterEach, no leak (other 624 tests unaffected).
  • bun test 634/0 · bunx tsc --noEmit clean
  • Independent fresh-context adversarial review: APPROVE, no blocking issues (mode-threading effective, null path observable, normal-path no regression, core purity intact).

Known follow-up (acknowledged, not blocking): the global-write-failed branch lacks direct coverage (needs FS-failure injection) — noted for a future test-hardening pass.

Refs #33 (Tier-1, partial — does not close; #33 tracks 6 PR groups).

…ail silently

Second-wave audit finding (#33 Tier-1), verified by hand + convergent
across two independent review passes.

updateStateToken had two defects:
- Called writeState with the default mode="auto", ignoring the user's
  PILOT_PROJECT_STATE. With `off` + an existing .opencode/, a token
  rotation could create the very project pilot-state.json the user
  opted out of.
- `if (existing)` with no else/log: when readState returned null
  (state file deleted/unreadable mid-session) rotation returned 200,
  mutated the in-memory token and broadcast SSE, but the persisted
  state kept the OLD token — TUI /remote then served a dead URL.
  Invisible, violating AGENTS.md "No silent failures".

Fix (keeps core/ pure — observability at the transport boundary):
- updateStateToken(dir, token, mode = "auto"): UpdateTokenResult,
  threads mode into writeState; returns a typed discriminated result
  ({ ok:true, write: WriteStateResult } | { ok:false, reason }).
- rotateAuthToken passes deps.config.projectStateMode and inspects the
  result: on no-existing-state OR a global write failure it emits
  deps.logger.warn + audit "auth.token.rotated.persist_failed". HTTP
  stays 200 (in-memory rotation + SSE are the real success path) but
  the persistence gap is now observable.

+10 deterministic tests (XDG_STATE_HOME-isolated so the readState→null
path is forced, not shape-only). bun test 634/0, tsc clean. Independent
fresh-context review: APPROVE, no blocking issues.

Refs #33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant