Skip to content

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

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

iceteaSA wants to merge 7 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 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 13496a4. Summary will update on new commits.

Review in cubic

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.

@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
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