fix(cursor): replay external model tool continuations with fresh conversation ids - #318
Conversation
…ersation ids External Cursor models like gpt-5.6-sol can reject resumeAction on a stale server-side conversation with Connect invalid_argument after stepCompleted. Mint a fresh conversation id for external tool-result continuations, keep native composer resume unchanged, and retry once with full history replay.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesThe Cursor adapter now distinguishes native and external wire models, selects fresh conversation IDs for external tool-result continuations, and retries qualifying Cursor conversation classification and request options
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runTurn
participant createCursorRequest
participant CursorTransport
participant cursorContextUsageTracker
runTurn->>createCursorRequest: Build continuation request
createCursorRequest-->>runTurn: Return conversationId and request
runTurn->>CursorTransport: Execute Cursor turn
CursorTransport-->>runTurn: Return invalid_argument failure
runTurn->>createCursorRequest: Force fresh conversation
createCursorRequest-->>runTurn: Return regenerated conversationId and request
runTurn->>cursorContextUsageTracker: Rekey usage totals
runTurn->>CursorTransport: Retry turn once
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/cursor.ts`:
- Around line 79-121: Update the runOnce/fallback flow in the cursor adapter to
track whether any non-heartbeat event was forwarded through emit, and allow the
fresh-conversation retry only when no output has been committed. Preserve the
existing invalid-argument, external-model, and abort checks, but rethrow after
forwarded text or tool-call events. Add a regression test covering an event
emitted before invalid_argument and verify the turn is not replayed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9a316a7-4a99-42df-9750-8788b0b05cfe
📒 Files selected for processing (9)
src/adapters/cursor.tssrc/adapters/cursor/cursor-errors.tssrc/adapters/cursor/discovery.tssrc/adapters/cursor/protobuf-request.tssrc/adapters/cursor/request-builder.tstests/cursor-adapter.test.tstests/cursor-discovery.test.tstests/cursor-errors.test.tstests/cursor-request-builder.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29cb2dcd0f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…forward Address PR lidge-jun#318 review feedback: - retry invalid_argument only for external tool-result continuations with no committed output - track emitted non-heartbeat events to avoid duplicate text/tool replay - rekey Cursor context-usage carry-forward when conversation ids rotate - add regression coverage for both behaviors
Ingwannu
left a comment
There was a problem hiding this comment.
The fix is valuable, but the current one-shot fallback can replay already-committed output. runOnce() forwards every mapped event to emit immediately, while the catch block retries any qualifying external-model invalid_argument without checking whether text, thinking, or tool-call events were already emitted. If Cursor yields visible output and then throws, the second request replays the turn and duplicates/corrupts the client stream. The added test throws before yielding anything, so it does not cover this case.
Please track whether any non-heartbeat/output event has been forwarded and permit the fresh-conversation retry only before output is committed. Add a regression where attempt 1 emits text or a tool-call event and then throws invalid_argument; assert there is no second attempt and no duplicate output. Re-request review after that. I am leaving the PR open because the underlying external-model continuation fix is meaningful.
Ingwannu
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head. The retry is now gated to an external-model tool-result continuation, invalid_argument, no abort, and no emitted non-heartbeat event. The new regression proves that text followed by invalid_argument does not trigger a second attempt, eliminating the duplicate-output blocker. Conversation-id rotation also rekeys context-usage carry-forward instead of dropping it. Integrated on current dev: 81 focused Cursor tests passed, typecheck and privacy scan passed; the PR head is green on Linux/macOS/Windows. Approved.
…idge-jun#307/lidge-jun#309/lidge-jun#279/lidge-jun#303/lidge-jun#318/lidge-jun#319 + v2.7.34 joined with local lidge-jun#304 merge and lidge-jun#279 fixups (models-auth revert, abortable SSE decoder)
Summary
Cursor Connect tool-result continuations for external models like
cursor/gpt-5.6-solwere reusing the same conversation id viaresumeAction. That works for Cursor-native models (composer-*,auto/default), but external models often lose server-side conversation state and then fail afterstepCompletedwith:connect-end-stream invalid_argumentfollowed by reconnect +
getBlobArgsburst + another failed follow-up turn.Native openai
gpt-5.6-soland Cursorcomposer-2.5stayed stable. The bug is specific to external Cursor Connect continuation.Root cause
For tool-result-only turns, OpenCodex correctly uses
resumeActionand structured conversation history. It previously kept the remembered_cursorConversationIdfor every model.For external Cursor models, that resume path is brittle: Cursor hydrates blob/checkpoint state, then rejects the next step with a generic
invalid_argumentinstead of emittingturnEnded. OpenCodex then reconnects and can loop.Fix
composer-*,auto/default= native).conversationIdwhile still sending full replay history (conversationTurns+rootPromptMessagesJson) underresumeAction.invalid_argument, rebuild once withforceFreshConversation: true._cursorConversationIdto the conversation id actually used for the turn.wireModel,action,conversationId, turn type, andexternalModel.Why this is not "use openai instead"
Sol remains on the Cursor provider. Only the continuation strategy changes for external Cursor models.
Tests
bun run typecheckbun test tests/cursor-adapter.test.ts tests/cursor-request-builder.test.ts tests/cursor-discovery.test.ts tests/cursor-errors.test.ts tests/cursor-tool-continuation.test.tsAdded coverage for:
forceFreshConversationalways mints a new idFiles
src/adapters/cursor.tssrc/adapters/cursor/discovery.tssrc/adapters/cursor/request-builder.tssrc/adapters/cursor/protobuf-request.tssrc/adapters/cursor/cursor-errors.tsSummary by CodeRabbit
invalid_argumentfailures that can occur during resume, using a fresh conversation identifier when safe.