Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped Cursor session-lifecycle bug fix with a regression test: unexpected agent termination now retires the session, reports an error exit, and prevents reuse of the dead process. The other changes are test-only, with no product-default, schema, security, billing, or static-analysis-suppression impact. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Cursor adapter now retires sessions when the agent connection terminates, reports an error exit, and excludes terminated sessions from session queries. A mock-agent crash trigger and a test cover this behavior. ChangesCursor session termination
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant MockAgent as ACP mock agent
participant Adapter as CursorAdapter notification loop
participant Session as Cursor session
participant Consumer as Event consumer
MockAgent->>Adapter: ConnectionTerminated notification
Adapter->>Session: Mark terminated and stop session
Session->>Consumer: Emit session.exited with error
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change can proceed through normal checks; no actionable merge-blocking issue was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
When the
cursor-agentprocess dies (crash, OOM, killed from outside), the ACP runtime emitsConnectionTerminated, butCursorAdapterhas no case for it and drops it. The session stays insessionsandhasSessionstays true, and nosession.exitedis published, so the thread never goes tostopped. On the next messageProviderCommandReactorsees a live session and reuses it, and every prompt fails straight away with the stored "process exited" error. That lasts until the idle reaper stops the session after 30 minutes, the model/mode/cwd changes, or the server restarts.Cursor now handles
ConnectionTerminatedthe way Antigravity does, and the way #10607 does for Grok: it marks the session terminated, stops it outside the notification fiber, and emitssession.exitedwithexitKind: "error", so the next message starts a fresh process. The test crashes the mock agent mid-prompt using the sameT3_ACP_CRASH_PROMPThook as #10607 (identical lines, so the two merge cleanly).Summary by CodeRabbit