Fix Cursor ACP thread rendering, thought output, and cancel delivery#3669
Fix Cursor ACP thread rendering, thought output, and cancel delivery#3669taschaub wants to merge 3 commits into
Conversation
- Send session/cancel as a spec-compliant JSON-RPC notification (no id); the Cursor CLI dropped the malformed message so turns kept running after pausing a thread. - Tag ACP assistant segment item ids with a per-runtime tag so resumed sessions stop appending new output to messages from earlier runs, which pushed assistant text above the latest user message. - Parse agent_thought_chunk into channel-aware segments and surface the accumulated reasoning as expandable thinking rows in the work log. - Drain queued ACP session updates before emitting turn.completed in CursorAdapter (raced against the notification fiber to avoid hanging on mid-turn teardown). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces new thought/reasoning channel support across multiple system layers and includes concurrency fixes for session teardown. An unresolved high-severity comment identifies potential out-of-order event emission, and the scope extends beyond simple fixes to new streaming capability. You can customize Macroscope's approvability policy. Learn more. |
…ng deltas - sendTurn's drain race also settles when stopSessionInternal interrupts the notification fiber. Check ctx.stopped after the race and bail out before mutating turn state or emitting turn.completed, so a torn-down session can no longer produce events after session.exited. Covered by extending the stop-during-pending-approval test. - Document why reasoning_text content deltas are intentionally not appended to the assistant message by ingestion: the full thought text is accumulated in the runtime segment and persisted via item.completed, and segments close on prompt settlement so cancelled turns keep their reasoning. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed both Macroscope findings in 98a8e44:
|
What Changed
Four related bug fixes in the Cursor CLI (ACP) integration, found while dogfooding resumed and cancelled threads:
session/cancelwas sent as a JSON-RPC request (with anid), but the ACP spec defines it as a notification. The Cursor CLI silently dropped the malformed message, so pausing a thread left the turn running. It is now sent as a proper notification.agent_thought_chunkupdates are now parsed into channel-aware segments and surfaced as expandable thinking rows in the work log (reusing the existing Codextask.progressthinking affordance).turn.completed, raced against the notification fiber so mid-turn teardown cannot hang.Scope: server-side ACP runtime + Cursor adapter, plus a 2-line tone mapping in
apps/web/src/session-logic.tsand a 4-line follow-through in GrokAdapter for the shared ingestion API. Unit tests added for each fix (AcpJsonRpcConnection,AcpRuntimeModel,AcpCoreRuntimeEvents,CursorAdapter), and theacp-mock-agentwas extended to reproduce the resume/cancel scenarios.Why
Cursor threads were unusable in three common flows: pausing a turn did nothing (the cancel was dropped by the CLI), resuming a session scrambled message order, and reasoning output was silently discarded. Each fix targets the root cause (spec-compliant notification, runtime-scoped segment identity, channel-aware segment parsing, deterministic drain before turn completion) rather than patching symptoms in the UI.
vp checkandvp run typecheckpass.UI Changes
No new UI. The visible effects are bug fixes: assistant output appears below the latest user message again on resumed Cursor threads, and Cursor reasoning shows up as the same expandable thinking rows already used for Codex. Happy to add before/after screenshots on request.
Checklist
Note
Fix Cursor ACP thread rendering, thought output, and cancel delivery
agent_thought_chunkevents, segmenting thought and assistant channels separately inAcpSessionRuntime; thought segments accumulate text and emit it on completion asreasoningactivitiesprotocol.tsto omit theidfield, preventing agents from rejecting cancel and other notifications as malformed requestsCursorAdapter.tsso content deltas and item completions are observed beforeturn.completedis emitted; suppressesturn.completedafter session teardownreasoningactivities with a truncated summary (max 120 chars) and capped detail (max 8000 chars) inProviderRuntimeIngestion.tsthinkingtone toreasoningactivities in the work-log UIMacroscope summarized 98a8e44.