Fix cross-process Linear OAuth refresh wiping valid connections

[cursor]

Fixes ADE-63

Bug and impact

When ADE desktop and ade serve (headless runtime) are both active for the same project, a near-expiry Linear OAuth refresh can race. Linear rotates the refresh token on the first successful exchange; the second runtime retries with the now-invalid refresh token, gets invalid_grant, and clears the shared credential store — forcing the user to reconnect Linear even though the connection was still valid.

This was introduced by the automatic OAuth token refresh added in #395.

Root cause

• ensureFreshToken() deduped refreshes in-process only (refreshInFlight).
• Desktop and headless each use the shared EncryptedFileCredentialStore under .ade/secrets.
• On invalid_grant, both services unconditionally cleared all Linear credentials.

Fix

• Add a cross-process file lock (linear-oauth-refresh.lock) so only one runtime refreshes at a time.
• Re-read the credential store after invalid_grant; if the refresh token was rotated or the access token is now fresh, treat the failure as a stale race and do not clear the connection.
• Apply the same logic to desktop (linearCredentialService) and headless (headlessLinearServices).

Validation

• npm run test -- --run src/main/services/cto/linearTokenRefresh.test.ts src/main/services/cto/linearAuth.test.ts (45 tests passed)
• npm --prefix apps/ade-cli run test -- --run src/headlessLinearServices.test.ts (22 tests passed)
• npm --prefix apps/desktop run typecheck

Open in ADE · cursor/critical-correctness-bugs-0bf2 branch · PR #400

Greptile Summary

This PR fixes a cross-process race condition where a near-expiry Linear OAuth refresh triggered by both the ADE desktop app and ade serve simultaneously would cause the slower process to receive invalid_grant (Linear rotates the refresh token on each exchange) and incorrectly wipe the shared credential store.

• Cross-process file lock (linear-oauth-refresh.lock in the secrets dir) serializes refresh attempts across runtimes; both desktop and headless services acquire the lock, re-read credentials inside it, and skip the refresh if a peer already completed it.
• Stale-rotation detection (linearInvalidGrantLikelyStaleRotation) re-reads the credential store on invalid_grant and treats the failure as benign if the refresh token was already rotated by a peer or the access token expiry is freshly extended; this guard is intentionally bypassed on forced refreshes (force: true) to avoid masking genuinely dead tokens.
• Lock constants corrected: LOCK_STALE_MS (10 s) is now less than LOCK_TIMEOUT_MS (15 s), making stale-lock recovery actually reachable within the wait window, and the retry loop now uses async setTimeout-based sleep so the event loop is not blocked.

Confidence Score: 5/5

Safe to merge. The fix correctly serializes cross-process token refreshes and avoids wiping valid connections on a stale invalid_grant; the previous reviewer concerns about event-loop blocking, stale-constant ordering, and unhandled timeout rejections are all resolved.

The lock implementation, stale-rotation heuristic, and forced-refresh bypass are all well-reasoned and covered by 45+ tests. The one remaining finding (fd leak when writeFileSync fails after openSync) is a minor edge case that self-recovers via stale detection within 10 s and does not affect correctness in normal operation.

linearOAuthRefreshLock.ts — the lock acquisition block could be tightened to close and delete the lock file if writeFileSync fails before the finally block is reached.

Important Files Changed

Filename	Overview
apps/desktop/src/main/services/cto/linearOAuthRefreshLock.ts	New cross-process file-lock utility. Previous review concerns (sync Atomics.wait blocking the event loop, LOCK_STALE_MS > LOCK_TIMEOUT_MS, unhandled timeout rejection) are all addressed in this revision. One minor resource-management issue: an fd leak when writeFileSync throws after openSync succeeds.
apps/desktop/src/main/services/cto/linearTokenRefresh.ts	Adds linearInvalidGrantLikelyStaleRotation helper with well-tested logic distinguishing race-rotated tokens from genuine revocations; trustFreshExpiresAt=false correctly disables expiry-based heuristic on forced refreshes.
apps/desktop/src/main/services/cto/linearCredentialService.ts	Integrates file lock and stale-rotation check. Lock is only applied when a credentialStore is present (the shared-file path); the legacy Electron safeStorage path bypasses the lock correctly. Re-reads credentials inside the lock before refreshing, preventing redundant refreshes when a peer already refreshed.
apps/ade-cli/src/headlessLinearServices.ts	Headless credential service mirrors the desktop changes: file lock acquired, credentials re-read inside lock, tokenOverride updated from store on stale rotation, logger wired through for lock-timeout diagnostics.
apps/desktop/src/main/services/cto/linearTokenRefresh.test.ts	New test suite for linearInvalidGrantLikelyStaleRotation covers all four meaningful cases: rotated token, fresh expiry, forced refresh ignoring fresh expiry, and dead token.
apps/desktop/src/main/services/cto/linearAuth.test.ts	Two new integration tests: race-rotation scenario (peer writes new token while this process gets invalid_grant) and forced-refresh must still clear credentials even when expiry appears fresh.
apps/ade-cli/src/headlessLinearServices.test.ts	Adds headless counterpart to the forced-refresh/invalid_grant test; correctly verifies tokenStored=false after a dead token is cleared.
apps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.ts	Trivial test fixture fix: adds missing isClosed mock to satisfy an updated interface shape.

Sequence Diagram

sequenceDiagram
participant D as Desktop (ensureFreshToken)
participant L as Lock File (.ade/secrets/linear-oauth-refresh.lock)
participant S as ade serve (ensureFreshToken)
participant Linear as Linear OAuth API
participant Store as EncryptedFileCredentialStore
D->>L: openSync("wx") — acquired
S->>L: openSync("wx") — EEXIST, wait 25 ms loop
D->>Store: re-read latest refreshToken
D->>Linear: POST /oauth/token (rt_old)
Linear-->>D: "{access_token, refresh_token: rt_new}"
D->>Store: write at_new, rt_new, expiresAt
D->>L: unlinkSync — released
S->>L: openSync("wx") — acquired
S->>Store: re-read latest refreshToken → rt_new
Note over S: linearTokenNeedsRefresh(expiresAt) = false → skip refresh
S->>L: unlinkSync — released

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 1
apps/desktop/src/main/services/cto/linearOAuthRefreshLock.ts:47-55
**File descriptor leak on `writeFileSync` failure**`fs.openSync` may succeed (creating the lock file and assigning `fd`) and then `fs.writeFileSync` may throw (e.g., `ENOSPC`, permission error). In that case the `catch` block receives a non-`EEXIST` error and re-throws it, exiting the `while` loop entirely — so the `finally` block around `fn()` that calls `fs.closeSync(fd)` is never reached. Both the open file descriptor and the lock file are left behind. Recovery only happens after the stale timer fires (10 s), and in long-lived processes the leaked fd accumulates on repeated disk-error events.
A separate try/finally around the two file-system calls in the acquisition section would ensure `fd` is closed and the lock file is deleted on any acquisition failure before throwing.

_{Reviews (5): Last reviewed commit: "fix: make Linear OAuth refresh stale loc..." | Re-trigger Greptile}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
ade	Ignored	Preview	May 30, 2026 1:35am

[arul28]

@copilot review but do not make fixes

[cursor]

PR Review

Scope: 7 file(s), +352 / −45
Verdict: Looks good

This PR fixes a real cross-process race: when desktop and ade serve refresh Linear OAuth near expiry, the loser’s invalid_grant was clearing a still-valid shared connection. It adds a file lock under .ade/secrets, re-reads credentials inside the lock, and treats invalid_grant as benign when the store already shows rotation or a fresh expiry (except on forced refresh after auth failure). Tests cover the race, forced refresh, and headless parity.

---

Notes

• Tests: The concurrent-rotation and forced-refresh cases in linearAuth.test.ts and headlessLinearServices.test.ts match the failure mode described in the PR and give good regression coverage.
• Lock helper: linearOAuthRefreshLock.ts follows the same wx + stale-mtime pattern as apps/ade-cli/src/tuiClient/state.ts; lock contention should be rare (only when two runtimes refresh at once).
• Callers: linearClient already swallows ensureFreshToken errors, so a 15s lock timeout surfaces as a best-effort skip rather than a user-facing crash.
• Docs (non-blocking): docs/features/cto/linear-integration.md still says invalid_grant always clears the connection; behavior is now conditional on stale rotation detection.

_{Sent by Cursor Automation: BUGBOT in Versic}

[arul28]

@copilot review but do not make fixes

[capy-ai]

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

[linear-code]

ADE-63

[arul28]

@copilot review but do not make fixes

[arul28]

@copilot review but do not make fixes

[arul28]

@copilot review but do not make fixes