fix: stop concurrent instances from cascading into a dead-token deadlock by sylvaindd · Pull Request #267 · griffinmartin/opencode-claude-auth

sylvaindd · 2026-08-05T16:00:56Z

Summary

Fixes a cascade that leaves every OpenCode instance holding a dead refresh token, recoverable only by running claude by hand. It needs several instances running concurrently to trigger, which is probably why it hasn't shown up in normal use — I hit it with 7.

The cross-process refresh lock judges staleness by the lock file's mtime against the acquirer's TTL, which defaults to 20 s. But the work done while holding it can take 120 s:

acquireRefreshLock()                     TTL = 20_000 ms
  └─ performRefresh()
       ├─ OAuth attempt                  15_000 ms timeout
       └─ CLI fallback: execSync(claude)  60_000 ms × 2 attempts = 120_000 ms

So:

Instance A takes the lock and drops into the CLI fallback, blocking for up to 120 s.
At t=20 s A's lock looks stale. B logs refresh_lock_stale_takeover, deletes A's lock and refreshes concurrently. At t=40 s C takes over from B, and so on.
Each successful rotation invalidates the refresh token the others hold, so they get invalid_grant → classified terminal → no cooldown → straight into their own CLI spawns, against a token endpoint that is by then rate-limiting.

Two things make it worse:

release() deleted the lock file by path without checking ownership. Once A had been taken over, A's release deleted B's lock — freeing it for every waiting instance at once, which is the exact stampede the lock exists to prevent. After the first takeover there is effectively no mutual exclusion at all.
execSync blocks the event loop, so an instance in the CLI fallback is frozen outright, not merely failing to refresh.

The fix

1. The lock declares its own lease and can extend it (48a7797). It cannot be renewed as the work proceeds — the longest operation under the lock is an execSync, so no heartbeat timer would ever fire. The lease therefore has to be declared up front. performRefresh extends it to cover the CLI budget before the fallback starts (2cd6cf9), and that budget is derived from the timeout and attempt count rather than restated, since these numbers drifting apart is the whole bug.

Back-compat is preserved: a lock file written by an earlier version carries no lease and still ages out by mtime, and an unreadable one is treated as stale rather than wedging refreshes forever.

2. release() only removes its own lock (48a7797). Each acquisition carries an owner id; a holder that has already been taken over leaves the successor's file alone.

3. The proactive timer no longer cries wolf (4509d39). refreshIfNeeded returns null both on genuine failure and when it deliberately steps aside (lock held elsewhere, or 429 cooldown). All three printed "Proactive token refresh failed. Run claude to re-authenticate." Both deferrals are routine with multiple instances and leave a usable token in hand, so the message fired regularly while nothing was wrong. It now warns only when the token is actually past use.

I deliberately did not add a cooldown to terminal failures. That looked like a fourth fix — N instances spawning claude in lockstep — but once the lease actually holds, the lock serialises those spawns on its own, and delaying terminal recovery would make a genuinely dead token slower to fix.

Related issue

None.

Testing

make all passes on Linux: 336 tests, 0 failures.

Five new cases in src/refresh-lock.test.ts, written to fail against the current implementation first:

a release after a takeover leaves the successor's lock intact, and that lock still excludes a third acquirer — this is the mutual-exclusion loss, and it failed before the fix
an extended lease holds off takeover past the base TTL, then ages out once the extension elapses — failed before the fix
a lock file with no lease still ages out by mtime (back-compat)
an unreadable lease is treated as stale rather than blocking refreshes forever

Checklist

PR title follows Conventional Commits (feat:, fix:, docs:, chore:, etc.)
make all passes locally (runs lint, build, and test)
Tests added or updated where applicable
README or docs updated where applicable — no user-facing surface changes; OPENCODE_CLAUDE_AUTH_REFRESH_LOCK_TTL_MS is already documented and keeps its meaning as the base lease.

Two defects in the cross-process lock, both of which surface only when several OpenCode instances run at once. Staleness was judged purely by the lock file's mtime against the acquirer's TTL, so a holder doing work longer than the TTL was declared crashed and taken over mid-refresh. The lock now records its own expiry and can extend it up front. It cannot be renewed as the work proceeds: the longest operation under the lock is an execSync, which blocks the event loop, so no heartbeat timer would ever fire. Files written by earlier versions carry no lease and still age out by mtime, and an unreadable one is treated as stale rather than wedging refreshes forever. release() also deleted the lock file by path without checking whether it still owned it. A holder that had already been taken over would delete its successor's lock, freeing it for every waiting instance simultaneously — the stampede the lock exists to prevent. Each acquisition now carries an owner id and releases only its own. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

The CLI fallback runs up to two execSync attempts of 60s each, six times the lock's 20s base TTL. Every sibling instance therefore concluded the holder had crashed and refreshed concurrently — and because each rotation invalidates the refresh token the others hold, those siblings got invalid_grant, classified it terminal, and dropped into their own CLI spawns against a token endpoint that was by then rate-limiting. The observed end state is every instance holding a dead refresh token and the user having to run `claude` by hand. The lease is now extended to cover the CLI budget before the fallback starts, and that budget is derived from the timeout and attempt count rather than restated, since the failure mode is precisely these numbers drifting apart. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

refreshIfNeeded returns null both when a refresh genuinely failed and when it deliberately stepped aside — another instance holds the cross-process lock, or a recent 429 put the account in cooldown. The proactive timer treated all three alike and printed "Run `claude` to re-authenticate". Both deferrals are routine with several instances running, and both leave a usable token in hand, so the message fired regularly while nothing was wrong. It now warns only when the token is actually past use, which is the only case where re-authenticating is the answer. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

greptile-apps · 2026-08-05T16:04:04Z

Greptile Summary

The PR adds explicit refresh-lock leases, ownership-aware release, token-rotation recovery, and revised retry and diagnostics behavior.

Extends lock leases before blocking CLI refresh work.
Tracks rotated-away credentials to prevent dead-token adoption and repair failed write-back.
Adjusts token-endpoint retry cooldowns and proactive-refresh messaging.
Makes credential-file replacement atomic and improves multi-process diagnostics.

Confidence Score: 4/5

The PR is not yet safe to merge because release can still remove a successor's refresh lock after a failed lease-payload rewrite.

The reply reports the ownership race fixed, but the residual null-payload path provides a concrete counterexample: a suppressed lease write failure leaves peers using the shorter mtime lease while the holder trusts its longer in-memory lease, so takeover can occur between readPayload and unlinkSync and the successor's lock can still be deleted.

Files Needing Attention: src/refresh-lock.ts

Important Files Changed

Filename	Overview
src/refresh-lock.ts	Adds renewable leases and guarded release, but the null-payload release path leaves the reported check-and-unlink race reachable after a failed lease rewrite.
src/credentials.ts	Extends the refresh lease around blocking CLI work and adds dead-rotation recovery and cooldown handling.
src/refresh-lock.test.ts	Covers ordinary takeover and expired-release behavior but not an unreadable lease followed by takeover during release.
src/rotated-tokens.ts	Adds bounded process-local tracking of credential pairs invalidated by successful refresh-token rotation.
src/keychain.ts	Repairs known-dead stored credentials and atomically replaces file-backed credential data.

Prompt To Fix All With AI

### Issue 1
src/refresh-lock.ts:212-218
**Unreadable payload reopens release race**

If an extended lease is truncated but cannot be rewritten, peers apply the shorter mtime-based TTL while the holder retains its longer in-memory lease. A waiter can therefore reclaim the malformed lock between `readPayload()` and `unlinkSync()`, causing the old holder to delete the successor's lock and allowing concurrent token refreshes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (5): Last reviewed commit: "fix: log one line per refresh cooldown, ..." | Re-trigger Greptile}

The ownership check and the unlink are two operations, so a successor could replace an expired lock between them and have its file deleted by the previous holder — the same loss of mutual exclusion the ownership check was added to prevent, through a narrower window. There is no portable atomic check-and-delete, so the guard is on the invariant instead: a successor can only exist once this lease has expired, so an expired holder does not touch the path at all. A margin covers a lease lapsing during the release itself. The ownership check stays as a second line for the case the lease looks live but the file was replaced anyway, such as a clock jump. An abandoned file ages out by its own lease, so declining to delete wedges nothing. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

initLogger truncated the log on every plugin init. Every OpenCode process loads this plugin, so one instance starting up erased the log of an incident its siblings were still living through — which is the only time the log is worth having. It is now append-only, entries carry the pid that wrote them, and CLAUDE_AUTH_DEBUG=1 resolves to a per-process filename so concurrent writers do not interleave into something unattributable. The existing test asserted the truncation, so it is replaced by its inverse rather than removed: the behaviour it locked in was the defect. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

… store This is the mechanism that turns a momentary write-back failure into an outage only an interactive `claude` login recovers. performRefresh mints a new pair and, on a failed writeBackCredentials, logs and continues — memory holds the live pair, the store holds the pre-refresh one. Refresh tokens rotate, so the stored pair's refresh token died the instant the rotation succeeded. But its access token is often still an hour from expiry, so the "adopt credentials replaced externally" re-read at the top of refreshIfNeeded sees a healthy-looking blob and adopts it, discarding the only live refresh token in existence. Every later refresh answers invalid_grant, and the CLI fallback cannot help because `claude` authenticates from that same dead store. Four changes, all on that path: Record each rotation. The re-read could not tell "our write failed" from "a sibling rotated" — both present as store-disagrees-with-memory-and-usable — and the missing evidence was the one thing performRefresh discarded. Successful rotations now remember the access token they rotated away, and every reader rejects a blob carrying one. A failed write-back becomes self-healing, since the live pair stays in memory for the next attempt. Repair the store. A compare-and-swap refusing to overwrite a pair we can prove is dead protects a blob nobody can use, so writeBackCredentials now overwrites in exactly that case — scoped strictly to tokens with a recorded rotation, never to "the store disagrees with me". Write the credentials file atomically, via a temporary file and a rename. A plain write truncates first, and with several instances reading the file a few times a minute a reader can observe an empty or half-written blob: it parses to null, which reads as "no credentials", and on the write path fails the CAS and orphans a rotation. Surface lock contention as transient. refreshIfNeeded returned a bare null both when a holder was mid-refresh and when the refresh token was dead, so the request path reported a hard "credentials unavailable" for what was really "wait a moment". It also decouples the holder's lease from the waiter's budget: the lease may exceed REFRESH_WAIT_MS as long as exhausting that budget yields a retryable response. Also stops spawning the CLI fallback when it cannot possibly help. `claude -p` authenticates from the same file we just failed against, so on a dead refresh token it fails identically — after up to two minutes of blocked event loop, which freezes the whole instance rather than just the refresh. The pre-CLI re-read of the store no longer skips file sources either; that exclusion had no rationale by its own admission and removed the last recovery step before the CLI on the one platform that has no fallback account to borrow from. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

The credentials and index suites transplant sources into a temp directory and import from there, so a new module has to be added to their copy lists or every test in them fails to resolve it. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Captured from a real incident. A single OpenCode instance, token expired overnight with no process running to refresh it proactively, and every refresh attempt from then on answered: refresh_failed HTTP 429 rate_limit_error "Rate limited. Please try again later." The refresh token was never dead — a single request made by hand hours later returned 200 — but the plugin could not get back to a working state on its own, and the user had to log in interactively. Two things kept it there. fetchWithRetry's default of three attempts applied to the token endpoint, so every logical refresh became three POSTs two seconds apart. Retrying a rate_limit_error on that timescale cannot succeed, because the endpoint's window is minutes, and it triples the pressure holding the limit open. The refresh now makes one request and leaves backing off to the caller, which already has a schedule built for it. That schedule could not act either: MAX_COOLDOWN_MS was 60s while the proactive timer ticks every 5 minutes, so however far the backoff escalated, the cooldown had always lapsed before the next tick. Every process therefore hit the endpoint every 5 minutes for as long as the limit lasted. The ceiling is now derived from the tick it has to outlast rather than being a bare constant that could drift away from it, and a test asserts that relationship — the old value fails it. The exponential schedule still starts at BASE_COOLDOWN_MS, so a one-off blip is still retried within seconds; only a sustained limit backs off far. Also stops sending the user to a login prompt they do not need. "Run `claude` to re-authenticate" is the fix for a dead refresh token and a waste of time for a rate limit, where the credentials are intact and waiting is the entire remedy. The two cases now say different things. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

The request path polls every ~2.5s while it waits, and each poll that lands in an active cooldown logged a line. With the cooldown ceiling now able to reach several minutes, one outage produced a few hundred identical entries per process — burying, in the very log added to diagnose these incidents, the handful of events that explain one. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

greptile-apps · 2026-08-07T15:47:21Z

+        const current = readPayload(path)
+        if (current && current.owner !== undefined && current.owner !== owner) {
+          log("refresh_lock_release_skipped", { source, reason: "taken_over" })
+          return
+        }
        try {
          unlinkSync(path)


Unreadable payload reopens release race

If an extended lease is truncated but cannot be rewritten, peers apply the shorter mtime-based TTL while the holder retains its longer in-memory lease. A waiter can therefore reclaim the malformed lock between readPayload() and unlinkSync(), causing the old holder to delete the successor's lock and allowing concurrent token refreshes.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/refresh-lock.ts Line: 212-218 Comment: **Unreadable payload reopens release race** If an extended lease is truncated but cannot be rewritten, peers apply the shorter mtime-based TTL while the holder retains its longer in-memory lease. A waiter can therefore reclaim the malformed lock between `readPayload()` and `unlinkSync()`, causing the old holder to delete the successor's lock and allowing concurrent token refreshes. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

A 429 from the token endpoint has been observed carrying Anthropic's API error shape (type/message) rather than OAuth's (error/error_description), so it is not necessarily the token handler answering. Without the response headers there is no way to tell an edge rate limit from the account's own usage quota from a genuine limit on refreshing, and each implies a different fix. Allow-listed rather than filtered, so no credential can be logged by accident. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

greptile-apps

Your trial has ended. Reactivate Greptile to resume code reviews.

The token refresh sent no User-Agent. claude.ai sits behind bot protection that refuses an unidentified client with 429 and a `rate_limit_error` body, before the request reaches the token handler at all — so the plugin read a hard rejection as a passing rate limit, backed off, and retried forever. It never worked. Across every debug log on the affected machine: 83 refresh attempts, 83 rate_limit_error, zero successes, not one invalid_grant. A rate limit lets something through eventually; this let nothing through, because the request was never accepted. It went unnoticed because it cannot be reproduced outside the host. Node and Bun both supply a default User-Agent when a script runs directly, so the same code refreshing from a terminal succeeds every time — as `pnpm run validate:oauth` does. Only a compiled host that sends none is affected, which is exactly where this plugin runs. Proven in place rather than by inspection. Running the real refresh path inside OpenCode, invalid token, minutes apart on one machine: 429 rate_limit_error before, 400 invalid_grant after. Then the same path with the live token: refresh_success, writeback_success, expiry advanced — the first successful refresh that machine has recorded. The User-Agent builder moves to model-config.ts so the OAuth request and the API requests, which already sent one, cannot drift apart again. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

greptile-apps

Your trial has ended. Reactivate Greptile to resume code reviews.

The suite transplants sources into a temp directory and imports from there, so credentials.ts importing the shared User-Agent builder needs that module copied alongside it. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

greptile-apps

Your trial has ended. Reactivate Greptile to resume code reviews.

The refresh chain gave up in ways that left the user to run `claude` themselves, and sometimes spent another account's quota getting there. Cooldowns and terminal failures lived in per-process Maps, so each OpenCode instance rediscovered a rate limit the others had already hit. They are now one small JSON file per account, written atomically at 0600, which every instance and the next process read. A rate-limited refresh returned null and the fetch hook answered with a retryable 429. OpenCode retries that five times, and also retries any message matching "rate limit", so a single deferred refresh became a burst of provider calls that could not succeed until the cooldown ended. The response is now a plain 401 with no retry-after. An expired token may fall through to one `claude -p . --model haiku` while the OAuth cooldown is still running, since the CLI recovers where the endpoint is refusing us. Only one process gets that attempt: the cross-process lock serialises it, and the attempt is stamped in the shared file so a failure is not retried for a minute. The stamp is written after the process exits, not before, or a spawn that burns its whole 60s timeout would leave the cooldown already expired. Two ways the fallback could refresh the wrong account. The primary account was handed CLAUDE_CONFIG_DIR=~/.claude, which makes the CLI look for ~/.claude/.claude.json and report "Not logged in"; it now inherits no config dir at all. A keychain entry whose hex suffix is not 8 characters cannot be mapped back to a directory, and was falling back to the primary's, so its fallback rotated the primary's tokens. Selecting an account that has since disappeared, and borrowing another account's credentials when the selected one could not refresh, are both gone. Both signed the user in as somebody else and spent their quota. The lock also needed a lease it can extend: the base TTL is 20s and a CLI spawn takes up to 60s, so every sibling declared the holder crashed and refreshed concurrently. This overlaps griffinmartin#267, which fixes the same defect and adds an owner-guarded release this PR leaves alone.

Sylvain DUARTE and others added 3 commits August 5, 2026 17:59

greptile-apps Bot reviewed Aug 5, 2026

View reviewed changes

Comment thread src/refresh-lock.ts

Sylvain DUARTE and others added 4 commits August 5, 2026 18:10

sylvaindd force-pushed the fix/refresh-lock-cascade branch from 5795895 to e005654 Compare August 6, 2026 16:01

sylvaindd force-pushed the fix/refresh-lock-cascade branch from 96aef22 to 09cf970 Compare August 7, 2026 07:41

greptile-apps Bot reviewed Aug 7, 2026

View reviewed changes

greptile-apps Bot reviewed Aug 10, 2026

View reviewed changes

carson2222 mentioned this pull request Aug 25, 2026

fix: recover an expired Claude token without running claude by hand #276

Open

Navigation Menu

fix: stop concurrent instances from cascading into a dead-token deadlock - #267

fix: stop concurrent instances from cascading into a dead-token deadlock#267
sylvaindd wants to merge 12 commits into
griffinmartin:mainfrom
sylvaindd:fix/refresh-lock-cascade

sylvaindd commented Aug 5, 2026

Uh oh!

greptile-apps Bot commented Aug 5, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

greptile-apps Bot Aug 7, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sylvaindd commented Aug 5, 2026

Summary

The fix

Related issue

Testing

Checklist

Uh oh!

greptile-apps Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

greptile-apps Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Aug 5, 2026 •

edited

Loading