Skip to content

fix(custody): make the withheld fallback refresh observable - #240

Open
iceteaSA wants to merge 2 commits into
cortexkit:mainfrom
iceteaSA:fix/custody-gate-observability
Open

iceteaSA wants to merge 2 commits into
cortexkit:mainfrom
iceteaSA:fix/custody-gate-observability

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Closes #239.

The problem

packages/opencode/src/index.ts decides at plugin construction whether to start the fallback-account background refresh. When it withholds, nothing observable says so — the process is indistinguishable from a healthy one.

Measured on main before this change:

'not-started' occurrences in index.ts .............. 1   (the assignment itself)
consumers of fallbackRefreshReady .................. 1   (__fallbackRefreshReady, a test export)
logger calls inside the gate block ................. 0
logger.{warn,info,debug}('claustrum', …) elsewhere . 16  ← the convention exists, this site opted out

The withheld state reached neither the log file nor sidebar state. Its only consumer was a test seam.

Why it matters

A boot-time flag gating a resource fails loud — the acquire blocks or errors. A boot-time flag gating a capability fails silent: nothing errors, a manager simply never exists, and everything downstream is merely quiet.

There is a live precedent for this exact gate. An earlier revision computed boot evidence as "every bound fallback has a usable resident credential right now", so a single cold account at boot returned provisional FAIL_CLOSED and suppressed startBackgroundRefresh() across all accounts — re-introducing a fleet-wide idle-expiry exposure. The logic was corrected to a structural-only gate; the silence was not.

The change

  • One logger.warn('claustrum', 'fallback refresh withheld at construction', …) at the decision, carrying the three dimensions that produced it (custodyMode, provisional, fallbacks) — so a reader can tell why it withheld, not just that it did. Follows the existing convention used at 16 other sites in the file.
  • A process-wide fallbackRefreshStructuralDark flag on SidebarState, using the house ...(cond && { field }) additive idiom.
  • Two tests pinning both signals.

The gate's boolean is unchanged. This attaches a signal to an existing decision; it does not re-decide it.

Verification

Three mutations, all re-run independently of the implementer's report, each restored to a clean tree before the next:

mutation result
delete the warn call withholding the fallback refresh logs the three dimensions that produced itReceived: undefined
remove the sidebar field the sidebar carries the structural-dark flagExpected: true, Received: undefined
keep the fields, falsify their values ('local', false, 'X') same log test — - Expected - 3 / + Received + 3

The third was not in the brief. It distinguishes a test that asserts a line fired from one that asserts the line carries the right dimensions — without it, the log could regress to useless content while staying green. It reddens, so the test checks values rather than presence.

Gates on the restored tree: core 199/0 · opencode 1896/0 · typecheck clean · aft_inspect 0/0.

Note on the test harness

The sidebar test triggers its write via claude-account add-apikey rather than the loader's own sidebar write: in the dark state the loader refuses with RESUME_TAKEOVER before reaching that write. Harness choice only — no production write path was altered.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Closes #239. Makes the withheld fallback refresh observable so a process that silently stopped refreshing no longer looks identical to a healthy one. The gate's boolean is unchanged; only its visibility changes.

  • Logs a claustrum warning at the gate with custodyMode, provisional, and fallbacks.
  • Adds a process-wide fallbackRefreshStructuralDark field to sidebar state and publishes it at construction, so a warm-vault dark boot exposes the decision without waiting for a command.
  • Non-dark boots emit no warning and omit the field.

Tests cover both dark and non-dark boots, including the construction-time publish.

Written for commit 2225d81. Summary will update on new commits.

Review in cubic

The construction-time gate that withholds the fallback-account background
refresh when vault residency is structurally unsafe emitted no signal, so a
process that had silently stopped refreshing looked identical from outside
to a healthy one.

Emit one claustrum warn at the decision carrying the three dimensions that
produced it (custody mode, provisional flag, fallbacks dimension), and add a
process-wide fallbackRefreshStructuralDark flag to sidebar state so the
condition is visible on the wire. The gate's boolean is unchanged.
Add a non-dark control test: an unconditional gate (if (true) / ...(true &&))
previously passed the whole suite, so nothing proved the signal is absent when
the gate does not withhold. The new test asserts no withheld warn and that the
sidebar key is omitted entirely.

Publish the boot decision to the sidebar at construction. The loader's own
sidebar write is unreachable while structurally dark (the custody reconcile
refuses first), so a warm-vault dark boot left the sidebar silent until a
command ran. A poll-based test covers the fire-and-forget boot publish.

Document at the projection site that the flag is a boot fact, never cleared.
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Pushed 2225d81 — closes both SHOULDs from review. Independent review verdict was APPROVE (0 must / 2 should / 2 nit); these are the two SHOULDs.

SHOULD 1 — the tests only proved absence-detection

The original three mutations all deleted something, so they demonstrated the tests notice a missing signal and said nothing about a false one. With the gate forced unconditional at both sites:

index.ts  if (fallbackRefreshStructuralDark)      → if (true)
index.ts  ...(fallbackRefreshStructuralDark && …) → ...(true && …)

fallback-refresh-observability.test.ts   2 pass / 0 fail

A regression firing the warn in local mode, or making the sidebar always claim dark, was invisible to the whole 1896-test suite.

Added a non-dark boot emits no withheld warn and omits the sidebar flag — a claustrum boot with a tombstoned fallback (dimension T), asserting the warn record is absent and the sidebar key is omitted rather than false, since ...(cond && {…}) drops the key entirely.

Mutation polarity is the point: for a positive assertion you delete to redden; for an absence assertion you must add the forbidden behavior. Reverting the warn cannot redden a test that requires no warn.

SHOULD 2 — the boot case had no sidebar write

Review traced that the loader's own sidebar write is unreachable while structurally dark: reconcileCustodyStartup throws for every dark combination (TAKEOVER_INCOMPLETE_MAIN_REAL / RESUME_TAKEOVER / TAKEOVER_INCOMPLETE_SLOT_ABSENT) and the loader returns first. Warm-vault probe: no sidebar file. Cold-vault probe: refreshVaultBackedOAuthAccounts republishes and the flag appears.

So an operator who booted dark and ran no command saw nothing in the sidebar. Fixed with a 3-line publish at the end of the boot sequence via the existing refreshSidebarQuota helper — the same pattern as the cold-vault republish. The refusal path is untouched; the diff adds no return, no reconcile change.

Verification

Three mutations re-run independently of the implementer's report, tree restored to clean between each:

mutation result
unconditional gate, both sites a non-dark boot emits no withheld warn and omits the sidebar flagExpected: false, Received: true
remove the boot write the boot decision reaches the sidebar without a commandExpected: true, Received: undefined
sidebar spread only unconditional, warn left gated same negative test reddens — Expected: false, Received: true

The third was not requested. It isolates the two halves of the negative test: without it, a single warn assertion could have been carrying the whole thing while the sidebar-absence assertion rode along untested. Each half fails on its own.

Gates on the restored tree: core 199/0 · opencode 1898/0 (1896 + 2) · typecheck clean · aft_inspect 0/0.

Nits

fallbackRefreshStructuralDark is a construction-time decision that is never cleared, so after recovery it still reads true — a lifetime comment now says so at the projection site, keeping the wire name aligned with the source variable rather than introducing a second term. The TUI not rendering the flag was left alone as out of scope for this PR.

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.

Custody boot gate withholds fallback background refresh silently — no log, no sidebar signal

1 participant