Skip to content

Fix Cursor ACP thread rendering, thought output, and cancel delivery#3669

Open
taschaub wants to merge 3 commits into
pingdotgg:mainfrom
taschaub:fix-cursor-thread-rendering
Open

Fix Cursor ACP thread rendering, thought output, and cancel delivery#3669
taschaub wants to merge 3 commits into
pingdotgg:mainfrom
taschaub:fix-cursor-thread-rendering

Conversation

@taschaub

@taschaub taschaub commented Jul 3, 2026

Copy link
Copy Markdown

What Changed

Four related bug fixes in the Cursor CLI (ACP) integration, found while dogfooding resumed and cancelled threads:

  1. Cancel never reached the agent. session/cancel was sent as a JSON-RPC request (with an id), 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.
  2. Resumed sessions rendered new output above the latest user message. ACP assistant segment item ids were stable across runtime restarts, so a resumed session appended new chunks to message items from earlier runs. Segment ids are now tagged with a per-runtime tag.
  3. Agent reasoning was invisible. agent_thought_chunk updates are now parsed into channel-aware segments and surfaced as expandable thinking rows in the work log (reusing the existing Codex task.progress thinking affordance).
  4. Race on turn completion. CursorAdapter now drains queued ACP session updates before emitting 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.ts and a 4-line follow-through in GrokAdapter for the shared ingestion API. Unit tests added for each fix (AcpJsonRpcConnection, AcpRuntimeModel, AcpCoreRuntimeEvents, CursorAdapter), and the acp-mock-agent was 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 check and vp run typecheck pass.

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

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Open in Devin Review

Note

Fix Cursor ACP thread rendering, thought output, and cancel delivery

  • Adds support for ACP agent_thought_chunk events, segmenting thought and assistant channels separately in AcpSessionRuntime; thought segments accumulate text and emit it on completion as reasoning activities
  • Fixes outgoing JSON-RPC notifications in protocol.ts to omit the id field, preventing agents from rejecting cancel and other notifications as malformed requests
  • Adds a drain barrier in CursorAdapter.ts so content deltas and item completions are observed before turn.completed is emitted; suppresses turn.completed after session teardown
  • Persists completed reasoning items as reasoning activities with a truncated summary (max 120 chars) and capped detail (max 8000 chars) in ProviderRuntimeIngestion.ts
  • Assigns the thinking tone to reasoning activities in the work-log UI

Macroscope summarized 98a8e44.

- 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>
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 54637cac-10bf-4e41-900a-e5d726d921dc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 github-actions Bot added size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 3, 2026
Comment thread apps/server/src/provider/Layers/CursorAdapter.ts
Comment thread apps/server/src/provider/acp/AcpCoreRuntimeEvents.ts
@macroscopeapp

macroscopeapp Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: 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>
@taschaub

taschaub commented Jul 3, 2026

Copy link
Copy Markdown
Author

Addressed both Macroscope findings in 98a8e44:

  • CursorAdapter.ts (High): sendTurn now checks ctx.stopped after the drain race and bails out before touching turn state or emitting turn.completed. A torn-down session can no longer emit events after session.exited. The existing stop-during-pending-approval test was extended to assert no turn.completed is published after teardown (it fails without the fix).
  • AcpCoreRuntimeEvents.ts (Medium): Intentional — documented at the emit site. reasoning_text deltas are deliberately not appended to the assistant message by ingestion (same treatment as the Codex/Claude/OpenCode reasoning deltas). The full thought text accumulates in the runtime segment and is delivered via item.completed (itemType reasoning), which ingestion persists as the expandable thinking row. Segments close on channel switches, tool calls, and prompt settlement including cancellation, so reasoning from an interrupted turn is not lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant