Summary
Make ClaudeAuthStore's token refresh + credential writeback cancellation-safe, so a client disconnect mid-refresh cannot drop a server-side refresh-token rotation.
Background
Raised during PR #70 review (gemini flagged it HIGH). The credential writeback already uses an atomic temp-file + rename (write_auth_file_atomic), so a cancelled refresh cannot corrupt or partially write the credential file — it is always either the old valid credential or the new one. The over-broad refresh_lock scope was also fixed in PR #70.
The one remaining, narrow edge: if the upstream OAuth provider rotates the refresh token server-side and the request future is cancelled in the window after the server commits but before our writeback lands, the stored (now-invalid) refresh token would fail on the next refresh, requiring a re-login.
Why central, not per-call-site
This applies to every provider's OAuth refresh (codex, cursor, xai, claude), not just claude_oauth. Wrapping each call site in tokio::spawn (as suggested inline on the PR) would scatter the concern and add per-request overhead. Instead, make the refresh run to completion inside ClaudeAuthStore (and the sibling stores) independent of the caller's cancellation — e.g. perform the refresh HTTP call + writeback in a detached task the store owns, awaited by the caller but not cancelled when the caller is.
Acceptance
- A cancelled request during an in-flight refresh still persists the rotated credential.
- Applies uniformly across the OAuth stores.
- A test simulating caller cancellation mid-refresh asserts the writeback completes.
Summary
Make
ClaudeAuthStore's token refresh + credential writeback cancellation-safe, so a client disconnect mid-refresh cannot drop a server-side refresh-token rotation.Background
Raised during PR #70 review (gemini flagged it HIGH). The credential writeback already uses an atomic temp-file + rename (
write_auth_file_atomic), so a cancelled refresh cannot corrupt or partially write the credential file — it is always either the old valid credential or the new one. The over-broadrefresh_lockscope was also fixed in PR #70.The one remaining, narrow edge: if the upstream OAuth provider rotates the refresh token server-side and the request future is cancelled in the window after the server commits but before our writeback lands, the stored (now-invalid) refresh token would fail on the next refresh, requiring a re-login.
Why central, not per-call-site
This applies to every provider's OAuth refresh (codex, cursor, xai, claude), not just
claude_oauth. Wrapping each call site intokio::spawn(as suggested inline on the PR) would scatter the concern and add per-request overhead. Instead, make the refresh run to completion insideClaudeAuthStore(and the sibling stores) independent of the caller's cancellation — e.g. perform the refresh HTTP call + writeback in a detached task the store owns, awaited by the caller but not cancelled when the caller is.Acceptance