Stop one missed credential write-back from killing a linked account

[imaustink]

The recurring "Your linked Claude Remote Control account's credential expired" prompt. This is not a keying bug (ADR 0029/0031 closed those) — it's a durability bug with a one-strike failure mode.

What's actually happening

Anthropic rotates the refresh token on every refresh, so the instant a run's CLI refreshes, the copy in the store is dead. That makes a missed write-back not a degradation but a kill: the stored refresh token has been spent, and only a human re-link recovers.

credentialsWriteback.ts already knew this — it's the first thing its doc comment says — but it staked the whole link on a single POST, once, after the turn:

• a pod killed, evicted, or hitting activeDeadlineSeconds mid-turn never reached it at all;
• it had no retry, and release.yml deploys on every push to main, so a POST arriving during a gateway rollout (ADR 0033 recorded eleven in fourteen hours) simply lost the refresh.

One strike, and the user re-links something they linked hours ago.

Evidence from prod (observed on #170)

Time	Event
00:38	Run succeeds, posts a full review
12:57:19	verdict:"authorized", CLAUDE_LOGIN_CREDENTIALS_JSON injected — the credential existed
~12:57:41	Anthropic rejects it ~20s in; orchestrator invalidates the record
12:58:30	verdict:"link-required"

The write-back route itself is correct (stores kind:"login", subject from the grant token, verified by read-back). The credential was genuinely stale on arrival.

The change

createCredentialsWritebackWatcher polls the credentials file during the turn (5s) and POSTs on change:

• shrinks the window from "the whole turn" to one interval — a pod that dies mid-turn has already reported the refresh the CLI did seconds in;
• is its own retry — a tick that fails to store does not advance lastPersisted, so the next tick re-sends the same blob;
• still flushes at the end, now with bounded retries, for a refresh landing in the last seconds where there is no next tick.

lastPersisted advances only on a confirmed store — that's what keeps a failure from being mistaken for "already persisted" — and the file is re-read after storing so the next comparison is against exactly what was stored. Ticks never stack (inFlight), and stop() waits out an in-flight tick so the final flush can't store a blob older than one already sent. When it runs out of attempts it says so loudly and names the consequence, since nothing downstream can tell.

The diagnostic whose absence made this expensive

The decisive question is always "was the credential already dead when we injected it, or did it die during the turn?" — and answering it needs the agent pod's logs, which are gone. So:

• the orchestrator's [authorization] line (which outlives the run) now carries claudeLoginExpiresAt;
• the agent logs the seeded credential's expiry at startup.

Expiry only. logVerdict handles the one structure holding every resolved credential for a run, and there's a test asserting the new field emits no token material.

Not fixed here — the actual cure

Every run still rotates a shared refresh token, so concurrent runs for one subject can still invalidate each other. The real fix is for the gateway to become the single serialized owner of refresh, handing runs a short-lived credential. That's an ADR, not a patch, and I'd rather land the containment first.

Tests

50 pass in claude-code-swe-agent (7 new for the watcher + expiry parsing), 42 in the orchestrator's authorization service (1 new). Repo-wide: 6 pre-existing failing files, identical to clean main.

Each watcher test is a way this used to break: a refresh mid-turn persisted without waiting for the turn to end; one store per refresh rather than one per tick; a failing gateway retried across ticks and stored on recovery; the final flush retried through a bad POST; a loud give-up; and inert when no grant was injected.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NyKcQdvju68R3edo4GXD3T

[imaustink]

Added: actual refresh, in the gateway (454fa0e)

To answer the question this commit came from — no, the write-back alone did not address auto refresh. It only persisted refreshes a pod happened to perform. It never caused one, which left two holes I couldn't reach from inside the agent:

• two concurrent runs for one subject each refresh from the same stored token, and whichever lands second invalidates the first;
• nothing refreshed between runs at all — and a refresh token isn't valid forever. The CLI names that failure itself: refresh_token_expired and refresh_token_expires_in are both strings in the shipped binary. An unexercised link is a link on a timer.

Both are now fixed by moving refresh into the one component that is single, durable, and able to serialize per subject. ClaudeCredentialRefresher.ensureFresh runs on the login read path — GET /claude-auth/api/token?mode=login and /api/wait, which is exactly what agent-orchestrator resolves a launch credential through — so a run is handed a freshly minted access token instead of whatever was left in the store.

The serialization is the point, not an optimization: concurrent callers share one in-flight promise per subject, so two simultaneous launches cost one refresh rather than two that rotate each other out.

What's verified, and what isn't — please read this bit

POST https://platform.claude.com/v1/oauth/token is confirmed present in the shipped CLI (v2.1.220), and the client_id is lifted from the authorize URL this gateway already hands users. Both observed, not assumed.

The exact request body the service expects is not something this repo can prove from outside. I dug through the bundle and the claudeAiOauth refresh path lives in a V8 bytecode region with a split string table — I could recover the enterprise-gateway JWT refresh verbatim, but not this one. So rather than ship a guess and hope, the code is built so a wrong guess is inert:

• anything but a 2xx carrying a usable access_token/expires_in → transient: stored credential untouched, caller served exactly what it would have been served before;
• a 200 we can't parse is transient too, never "refreshed" — claiming success would overwrite a working credential with a broken one;
• invalid_grant/refresh_token_expired → rejected and reported, but the record is deliberately not deleted (the orchestrator's re-auth path owns invalidation, and a hiccup that merely looked like a rejection must not cost a working link);
• on success the new blob is persisted and read back before returning, and if it won't persist we shout and still hand back the live blob — at that moment it's the only living copy;
• a response omitting a new refresh token carries the old one forward instead of writing undefined, and every field the file carried that this code has no business understanding is preserved, so runs get a blob shaped like claude auth login produces.

So worst case this is a no-op and you're where you were; best case the whole class is gone. Worth one manual curl against the token endpoint with a real refresh token before trusting it — if the body shape differs, it's one small function to change (refreshCredentialsBlob).

GATEWAY_CLAUDE_CREDENTIAL_REFRESH=false disables it; ..._MARGIN_MS tunes the due window (30 min default). A blob with no readable expiry is treated as due — an unknown expiry is far likelier to be an expired credential than a healthy one, and a needless refresh costs a round trip while a skipped one costs a re-link.

Still not covered

A link that goes completely unused still ages out, because refresh-on-read only fires when something reads. A background sweep needs to enumerate stored subjects, and SecretRecordStore is explicitly documented as having "no listing, no label-selector scan, no O(keyspace) anything". Adding one is a design decision to take on purpose, not to smuggle into this PR. Say the word and it's a small follow-up (label-selector list + the _recordKey field, scoped to the sweeper only).

Tests

17 new (credential-refresher.test.ts), covering each decision above — including the two that matter most: a 200 with no usable tokens must not be treated as a refresh, and concurrent callers must collapse to one refresh. Gateway suite: 214 pass, 3 pre-existing load failures (node-pty/marked missing in my checkout) identical to clean main. Repo-wide: 6 pre-existing failing files, same as baseline.

[imaustink]

Added: background auto-refresh (770a82a)

The gap I flagged is now closed. ClaudeCredentialSweeper renews stored login credentials on a schedule, independent of whether anything is using them — which is the only thing that keeps an unused link alive, since refresh-on-read fires only when something reads.

• Runs in the gateway (the one long-lived, single-instance component holding the store), hourly by default, plus a pass at startup — so a deployment neither waits an interval to begin nor waits one to discover the sweep is broken.
• Refreshes through ClaudeCredentialRefresher.ensureFresh, not directly, so a sweep landing next to a launch collapses onto that subject's single in-flight refresh instead of the two rotating each other's token out.
• Sequential, because there's no hurry and a scheduled burst of token requests is a worse neighbour than a slow loop.
• Margin is deliberately wider than the on-read one (4h vs 30m): the sweep's job is that a link is never found dead; the read path's job is only that the blob it's about to hand out works.

The design decision this required — please sanity-check it

This needed the one thing SecretRecordStore was built not to do. Its naming section says "no listing, no label-selector scan, no O(keyspace) anything", and listRecords is an explicit, documented exception.

My reasoning for taking it: that rule is about the read path, where an exact get by computed name is both possible and required. A sweep has no key to compute a name from — enumerating is the task. And commonLabels already existed in that file "so an operator — and a cleanup sweep — can find them all", so the store anticipated this. The scan is label-selector scoped to one record type, not the namespace, and runs once per interval. If you'd rather it not exist, the alternative is maintaining a subject index, which is more state to get wrong.

A record whose plaintext key was never stored is skipped, not guessed at — the object name is a one-way hash, so it can't be attributed to a subject, and acting on an unattributable credential is worse than leaving it alone.

Two things made loud on purpose

Both would otherwise look exactly like a healthy sweep, right up until someone's link expired:

• a store that can't enumerate makes the sweeper stand down with a message rather than silently never running;
• listSubjects deliberately does not swallow its error the way get/set/delete do. A list that failed is not an empty store, and reading it as one is how a sweep does nothing forever.

Deployment

No new RBAC. The gateway's Role already grants list on Secrets — the API server requires it for the watch the pending-link resume uses. I verified the rendered Role and updated its comment to say the sweep uses it directly now.

Tunable via GATEWAY_CLAUDE_CREDENTIAL_SWEEP_INTERVAL_MS / _SWEEP_MARGIN_MS; GATEWAY_CLAUDE_CREDENTIAL_REFRESH=false disables read-path refresh and the sweep together.

Tests

Gateway suite now 224 passing (up from 197 on main), same 3 pre-existing load failures (node-pty/marked missing locally). Repo-wide: 6 pre-existing failing files, unchanged from baseline. Gateway typecheck errors unchanged at 6, all pre-existing.

New coverage includes the enumeration (label-selector scoping, orphaned-key skip, failure propagates rather than reporting empty, absent list verb) and the sweep (renews the stale one and leaves the healthy one alone, goes through the refresher, reports a failed listing loudly, stands down audibly, sweeps at startup, doesn't overlap passes).

---

Full PR now covers all three layers: the run reports its refreshes continuously (ebeda4f), the gateway refreshes before serving (454fa0e), and the gateway refreshes on a timer so nothing has to ask (770a82a). The one caveat from before still stands and is the thing worth verifying before you trust it: the OAuth request body is unverified, so worst case the two gateway layers are inert no-ops and you're where ebeda4f left you.