Skip to content

fix(server): bind Cursor retry replay to the admitted OAuth account on 429 failover - #4954

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:agent/cursor-retry-replay-bind-20260918
Sep 18, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:agent/cursor-retry-replay-bind-20260918

Conversation

@luvs01

@luvs01 luvs01 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • applyFailoverSnapshot commits through commitResolvedOAuthSelection, which can admit a DIFFERENT account than the proposed one when a newer manual selection wins the proposal race. The helper already re-stamps route.provider, sentOAuthSnapshot, and replayOAuthCredentialSnapshot from the committed snapshot — but the runTurn preflight 429 rotation in run-turn-execution.ts still passed the REJECTED proposal's { accountId, generation } to bindRouteReasoningReplayScope, binding Cursor's credential-scoped replay/checkpoint identity to an account that never served the retry.
  • Return the admitted OAuthAccessSnapshot (or null) from applyFailoverSnapshot so the caller binds the replay scope to the same account the retry actually uses.
  • Extend the adapter-event failover test to observe continuation-owner credential identity and assert the replay is scoped to the admitted manual account, not the rejected proposal.

Verification

  • Head: 10cb5f6e93ae16170694292295e86c471b71afcf on dev 6467235a85df0f649a588c601532316d6ae0fce9 (merged 2026-09-18; clean ort merge, no conflicts).
  • bun x tsc --noEmit — clean.
  • bun run structure:check, bun run privacy:scan, bun scripts/file-size-ratchet.ts — passed.
  • bun test tests/oauth/adapter-event-oauth-failover.test.ts — 12 pass / 0 fail. The new assertions fail against the unfixed source: the replay attempt's continuation-owner credential identity matched the rejected proposal, not the admitted account.
  • bun test tests/oauth/generic-oauth-failover.test.ts tests/lib/reasoning-replay-scope-source.test.ts — 45 pass / 0 fail.
  • Fork CI: https://github.com/luvs01/opencodex/actions/runs/35291796244

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing. Local gates passed on this head (tsc, structure:check, privacy:scan, focused tests on the merged head).

  • I pushed my PR to the latest dev commit. The branch carries dev 4c0124acb.

  • I resolved all correct Codex and CodeRabbit findings. No unresolved review threads on this head.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved OAuth account failover when a manual account selection occurs during a pending request, ensuring the most recently selected account is honored.
    • Preserved conversation continuity by binding retries to the account and session state actually admitted after failover.
    • Added coverage for credential consistency across ongoing conversations and correct credential changes after switching active accounts.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9b103629-32bd-4dce-8de0-de80ee39e988

📥 Commits

Reviewing files that changed from the base of the PR and between f3cb520 and ddd630d.

📒 Files selected for processing (3)
  • src/server/responses/request-transport.ts
  • src/server/responses/run-turn-execution.ts
  • tests/oauth/adapter-event-oauth-failover.test.ts
 ____________________________________________________________________
< Never go full rewrite. You don't buy that? Ask Netscape Navigator. >
 --------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 17, 2026
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (3/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 3/4).

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

3/4 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

@lidge-jun
lidge-jun force-pushed the agent/cursor-retry-replay-bind-20260918 branch from bddb945 to ddd630d Compare September 18, 2026 09:36
@lidge-jun

Copy link
Copy Markdown
Owner

Rebased this branch onto current dev (f3cb520400) and force-pushed. No change to the diff; the rebase is the whole fix.

test 2/4 was failing on the file-size ratchet, and the offender was tests/server/management-provider-validation.test.ts at 5529 lines against a cap of 5506 — a file this branch does not touch. It branched from a dev commit that carried that state, and dev cleared it afterwards in #5018 by moving the newest cases into sibling files. The file is 5498 on current dev, so the branch inherited a defect that no longer exists.

Verified after the rebase: no file in the baseline exceeds its cap in this tree. CI re-approved at the new head.

@lidge-jun

Copy link
Copy Markdown
Owner

macos 1/2 reported cancelled at 20m16s against the 20-minute platform-macos budget, and the aggregate correctly refused to call that a pass. Same contention I created by approving nine Cross-platform runs at once on an account that runs one at a time; the same shard finished inside budget today on dev, #4926, #4938, #4994, #5050 and #5042.

Re-running the failed job rather than widening the budget.

@lidge-jun
lidge-jun marked this pull request as ready for review September 18, 2026 10:51
@lidge-jun
lidge-jun merged commit 3677075 into lidge-jun:dev Sep 18, 2026
48 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants