Skip to content

fix(claustrum): retry rotated vault credentials before reporting a 401 - #233

Open
iceteaSA wants to merge 8 commits into
cortexkit:mainfrom
iceteaSA:feat/get-before-report
Open

iceteaSA wants to merge 8 commits into
cortexkit:mainfrom
iceteaSA:feat/get-before-report

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Stacked on #232 — review the top commit bb3033d9 only; the two below it are #232's.

The gap

Under vault custody the vault owns rotation, and Anthropic invalidates the prior access token the instant it rotates — measured on the live vault: a consumer holding v44 took a genuine 401 within 42s of the v45 commit, another at 0.1s. Our resident cache advances only on a get; a rotation does not push. So any process whose cache predates a rotation serves dead material on its next turn and the user sees a failed turn.

Before this commit we reported the 401 to the vault and returned it to the caller. No re-fetch, no retry.

Why now

This is not a new mitigation — it restores a recovery path custody deleted. Pre-custody we already did exactly this against auth.json (ARCHITECTURE.md:47: "a sticky request whose old access token receives 401 re-reads host auth and directly retries with a concurrently rotated, still-valid access token"). Custody replaced the authority that retry re-read with a tombstone and did not replace the mechanism, so every rotation since the flip has been unprotected.

Rotation period is forced by our own poll, not by a vault schedule: every ~60s each process calls cache.get(handle, 270m) (minTtl = getRefreshBeforeExpiryMs + 30m), and the vault refreshes at now + min_ttl >= expires_at, so an 8h token rotates every 480 - 270 = 210m. Confirmed on live data — 101 of 106 gaps exactly 210m on one account. Pre-custody our own loop used a 240m threshold, so the exposure change is 240m → 210m, a 1.14× increase in rotation frequency on a credential that now has no local fallback behind it.

Shape

sendWithAccessTokenOnce is the former send path with its immediate 401 report removed; it now only records the served credential. A wrapper sendWithAccessToken calls it and, on 401 + served, does a bounded cache.get(handle, …, { bypassCache: true }):

  • version advanced → the vault rotated under us: re-send once with fresh material, do not report
  • version unchanged → genuinely rejected: report exactly as before

bypassCache is new on ClaustrumCredentialCache.get and is load-bearing. A rotation does not change local expiry, so a plain get returns the dead cached credential and the feature would be inert.

Placing the retry at the wrapper rather than at each call site also fixes two latent defects in the sticky-route 401 arms without touching them. Arm A (main + vault-served) set permanentAuthFailure = true, classifying a transient, self-healing rotation race as permanent and migrating main away. Arm C (fallback + vault-served) "retried" via resolveClaustrumAccess, which is cache.peek — resident-only, so post-rotation it re-sent the same superseded token. Both now only ever see credentials that are genuinely dead, which makes A's classification correct and C's retry harmless.

Observability

Neither side could measure how often this race fires: our logs had no vault-served-401 logging, and the vault's chain never sees direct-path races because our own freshness check suppresses the report once the cache has rotated — a guard whose success erases the evidence it was needed. The new log records handle (redacted), served and current record versions, whether a retry was attempted and its outcome, and whether a report was suppressed and by which branch.

Verification

Five isolated-hunk revert proofs, one per behaviour:

rotated      Expected 200, received 401        (retry dispatch disabled)
unchanged    refresh RPC +1, received +0       (bypassCache removed)
retry-401    expected report v18, got v17      (report path reverted)
timeout      test times out                    (bounded race removed)
backoff      expected 2 RPCs, received 3       (per-handle backoff disabled)

Gates: core build 0, core 200 pass, opencode 1894 pass, typecheck 0, biome 0.

Independent cross-family review: APPROVE, 0 must / 0 should, with the choke-point claim verified by call-site enumeration — sendWithAccessTokenOnce is called exactly twice, both inside the wrapper, and the wrapper is the sole caller from all three external sites. Rate-limit bound checked at ~12 RPCs/min worst case per process against the vault's 64/60s limit; bypassCache: true reachable from exactly one call site.


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

Vault-served requests that received a 401 previously returned it immediately after reporting; replayable requests on the message, CacheKeep prewarm, and prime request paths now fetch fresh vault credentials and retry once when the record advances. Non-replayable requests — including streamed bodies — still return the original 401, while unchanged or failed refreshes are reported without repeated retries.

Bug Fixes

  • Authenticates streamed vault requests, which previously missed their bearer header on the served path.
  • Isolates 401 refreshes from shared in-flight loads, preserves the resident credential on refresh failure, and prevents stale records from overwriting newer ones.
  • Limits each affected handle to one credential.get within the timeout and backoff window.
  • Logs served and current record versions, retry outcomes, and report-suppression reasons; the vault-served 401 recovery message is pinned by tests because a peer system consumes it.
  • Treats vault-served accounts as credentialed for quota recovery, profile hydration, /claude-quota, and killswitch eager refresh.
  • Refuses to reuse a main vault bearer after a successful auth-failure report.

Written for commit dd89048. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Architecture diagram
sequenceDiagram
    participant Client as Client / Caller
    participant Wrapper as sendWithAccessToken
    participant SendOnce as sendWithAccessTokenOnce
    participant Cache as ClaustrumCredentialCache
    participant Vault as Vault (Credential Provider)
    participant Report as reportCapturedClaustrumAuthFailure

    Note over Client,Report: HIGH-LEVEL FLOW: 401 Handling with Rotation Retry & Bound

    Client->>Wrapper: Send request (with credential)
    Wrapper->>SendOnce: sendWithAccessTokenOnce(credential)
    SendOnce->>Client: Serve credential (record version tracked)
    SendOnce-->>Wrapper: Response
    
    alt 401 received
        Wrapper->>Cache: get(handle, 0, { bypassCache: true })
        Note over Cache: Bypass resident cache (rotation does not change local expiry)
        Cache->>Vault: Fetch fresh credential record
        Vault-->>Cache: New credential record (possibly advanced version)
        Cache-->>Wrapper: Fresh credential + version

        alt Version advanced (rotation detected)
            Wrapper->>SendOnce: Retry once with fresh credential
            SendOnce-->>Wrapper: Response (usually 200)
            Note over Wrapper: Do NOT report 401 (self-healing race)
        else Version unchanged OR retry also 401
            Wrapper->>Report: Report credential failure
            Report->>Vault: reportAuthFailure(record)
            Note over Report: Single-shot per served version<br/>Checks cache freshness before report
            Vault-->>Report: Acknowledged
            Report-->>Wrapper: Outcome
        end
    else Non-401 response
        Wrapper-->>Client: Return response
    end

    Note over Cache: Bounded by per-handle backoff & timeout<br/>At most 1 vault get per 401
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/core/src/claustrum.ts
Comment thread packages/opencode/src/index.ts
Comment thread packages/opencode/src/index.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread packages/opencode/src/tests/index.test.ts Outdated
Six gates treated local credential bytes as a proxy for usability. A vault-served account's local slot is the provider tombstone, so quota recovery, profile hydration, /claude-quota and the killswitch's eager refresh all skipped healthy accounts once custody emptied that slot. Each site now admits a live vault binding alongside local access; none admits an account with no credential anywhere.
liveMainVaultAccess hand-rolled its sibling and dropped the sibling's stale-version guard, so main profile hydration and /claude-quota could bearer a version already reported auth-failed. It now delegates to resolveClaustrumAccess, inheriting both the guard and the warm schedule. mainServedAccessToken is cleared on a successful main report only: a suppressed or failed report tells the vault nothing, so local belief must not diverge from what the vault received.
@iceteaSA
iceteaSA force-pushed the feat/get-before-report branch from 13496a4 to fe5ff5f Compare September 18, 2026 16:49
@iceteaSA

Copy link
Copy Markdown
Contributor Author

Pushed dd89048 — widens get-before-report from the model-request path to CacheKeep prewarm (including source-cache-warm) and /claude-prime fire, plus transition-only logging for latched background refreshes.

Why these two sites

This morning at 05:24:08Z and 05:24:19Z two live processes failed CacheKeep prewarm with 401 OAuth access token has been revoked, ~15s after the vault rotated v173 → v174. Three other processes prewarmed fine in the same window. The vault recorded both failures as stale reports (applied=0), which are not free — they consume its 64/60s connection limiter budget before handle resolution.

The retry wrapper already on this branch covered only sendWithAccessToken. CacheKeep and prime reported rotation-time 401s open-loop.

Quota poll and profile hydration are closed, not deferred

I set out to widen five sites. Two of them turn out to have no vault-report path at all:

// packages/core/src/accounts.ts:4805-4813 — quota poll catch
if (
  !message.includes('Claude quota check failed: 401') ||
  vaultEnabled ||                       // ← vault accounts disqualified
  access.source !== 'sidecar'
) {
  throw error
}

A vault-served quota 401 is re-thrown; the forced-refresh retry below runs only for sidecar credentials. oauth-profile.ts:28 throws on any non-OK status. Neither reports to the vault, so there is no raced-report hole to close — widening them would buy freshness (one stale quota tick, a tier string behind a 7-day TTL), not incident prevention.

That vaultEnabled || disqualifier is deliberate: a vault-owned account must not force a local OAuth refresh, because consuming Anthropic's single-use refresh token rotates the family away from the vault. A generic retry wrapper collides with it head-on — an earlier attempt at exactly that produced 11 suite failures, which was the suite defending a real invariant.

Semantics

  • retry get uses bypassCache: true — local TTL has not expired after a rotation, so a normal cache read returns the same stale token and the retry is decorative
  • retry once, only on a strictly advanced recordVersion
  • report only when the version did not advance, and report the send-time version, never the currently-cached one
  • sidecar-served 401s never report

Verification

Re-ran all three mutations myself rather than accepting the implementer's report:

mutation result
bypassCache: truefalse CacheKeep retries a vault 401 only after a bypassed get advances the served version — expected true, received false
report cache.peek() version instead of send-time a vault prime 401 reports the credential version sent before a cache rotation — expected-to-contain failed
drop the transition guard on latched logging logs a latched background refresh failure once until a successful refresh re-arms it — expected length 1, received 2

Gates: core 200/0 · opencode 1914/0 · typecheck clean · aft_inspect 0/0.

Note on the logging half

ClaustrumCredentialCache.#refreshIfApproachingExpiry handled background-refresh failures with void load.catch(() => {}) and zero logger calls, and #load's failure path throws before any #cache.set. So a broken or latched vault record produced no log output at all while the cached token kept serving — the failure only became visible at hard expiry.

The swallow stays correct for the transient case (a vault blip behind a valid cached token is a non-event). Only permanent / auth_required classes log, once on transition, re-armed by a successful refresh.

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