fix(orchestration): show follow-up assistant replies after Cursor session resume#3642
Conversation
|
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟠 High
When the first thread.message-sent event for an attachments-only message arrives (empty text, streaming: false, no existing row), mergeThreadMessageProjection evaluates previousMessage.text on the !previousMessage branch because the condition incoming.streaming || incoming.text.length > 0 is false, so nextText falls through to turnChanged ? "" : previousMessage.text, which dereferences undefined. This throws during projection instead of persisting the message. The guard should also cover the case where there is no previous message and the incoming text is empty.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProjectionPipeline.ts around line 27:
When the first `thread.message-sent` event for an attachments-only message arrives (empty text, `streaming: false`, no existing row), `mergeThreadMessageProjection` evaluates `previousMessage.text` on the `!previousMessage` branch because the condition `incoming.streaming || incoming.text.length > 0` is false, so `nextText` falls through to `turnChanged ? "" : previousMessage.text`, which dereferences `undefined`. This throws during projection instead of persisting the message. The guard should also cover the case where there is no previous message and the incoming text is empty.
There was a problem hiding this comment.
Thanks — same issue flagged by Bugbot, and I agree it was worth fixing.
The failure mode is: no existing projection row, incoming thread.message-sent has empty text and streaming: false (attachments-only), and we incorrectly read previousMessage.text because turnChanged is false when previousMessage is undefined.
Addressed in 8c2d214 with optional chaining plus a unit test. The core Cursor resume behavior is unchanged.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3a401bf. Configure here.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR modifies runtime behavior in the orchestration pipeline (message ID generation and projection merging). Additionally, there are unresolved HIGH severity review comments identifying a potential null dereference issue that could cause exceptions during message projection. You can customize Macroscope's approvability policy. Learn more. |
|
Reviewed the bot feedback on this PR:
I pushed 8c2d214 to address the empty-first-upsert case with Happy to rebase onto latest |
| return previousMessage.turnId !== nextTurnId; | ||
| } | ||
|
|
||
| export function mergeThreadMessageProjection( |
There was a problem hiding this comment.
🟡 Medium orchestration/threadMessageProjection.ts:27
When an assistant message with attachments from an earlier turn is reused for a new turn, mergeThreadMessageProjection resets text and createdAt but does not signal the turn change, so projection callers that preserve previousMessage.attachments when the incoming event omits attachments keep stale attachments from the previous turn. The follow-up reply incorrectly shows files or images that belong to the earlier turn.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/threadMessageProjection.ts around line 27:
When an assistant message with attachments from an earlier turn is reused for a new turn, `mergeThreadMessageProjection` resets `text` and `createdAt` but does not signal the turn change, so projection callers that preserve `previousMessage.attachments` when the incoming event omits attachments keep stale attachments from the previous turn. The follow-up reply incorrectly shows files or images that belong to the earlier turn.
…sion resume When Cursor resumes a session, ACP reuses assistant segment IDs across turns. Scope assistant message IDs by turn and reset projection state on turn change so follow-up replies appear below the latest user message instead of updating stale segments sorted above it. Co-authored-by: Cursor <cursoragent@cursor.com>
Guard mergeThreadMessageProjection when there is no previous row and the incoming upsert has empty non-streaming text, so attachments-only first events persist instead of dereferencing undefined. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Rebased onto latest
No rebase conflicts. |
8c2d214 to
f3ce3fe
Compare
|
@juliusmarminge — small orchestration fix when you have a minute. #3642 fixes a Cursor-specific resume bug: after reconnecting a Cursor session, follow-up assistant replies were updating stale message segments above the latest user turn instead of appearing as new replies below it. Root cause is ACP reusing assistant segment IDs across turns; the fix turn-scopes those IDs and resets projection state on turn change. Macroscope/Bugbot also caught an edge case on the first empty non-streaming upsert — addressed in the second commit. I also have a couple of other features that might be interesting if you're up for it:
No pressure on any of it — just thought I'd share in case any of it's useful. Thanks for maintaining this; really enjoying the project. |
1 participant
Summary
segment:0,segment:1, …) across turns, so T3 projected later replies into the samemessage_idas turn 1 and kept the originalcreated_at, sorting them above newer user messages.assistant:{turnId}:{baseKey}) and adds merge logic that resets text/created_atwhen a reused ID starts a new turn.Changes
assistantMessageIds.ts— turn-scoped assistant message ID generationthreadMessageProjection.ts— shared merge helper for projection upsertsProviderRuntimeIngestion.ts— use turn-scoped IDs for segments, completions, and diff checkpointsProjectionPipeline.ts/projector.ts— apply merge helper onthread.message-sentTest plan
assistantMessageIdsandthreadMessageProjectionProviderRuntimeIngestionexpectations for turn-scoped IDsassistant:session-1:segment:0across two turns creates distinct messages with correct timestampsapps/serverorchestration testsMade with Cursor
Note
Fix follow-up assistant replies not appearing after Cursor session resume
ProviderRuntimeIngestion, so resumed sessions that reuse provider segment IDs produce distinct messages per turn instead of colliding.threadMessageProjection.tswith canonical turn-aware merge logic: streaming deltas append within the same turn, non-streaming updates replace text unless empty, and a turn change resets text andcreatedAt.ProjectionPipelineandprojector.tswith calls tomergeThreadMessageProjection.assistantMessageIds.tsto centralize ID construction in the formatassistant:<turnId>:<baseKey>.Macroscope summarized d02f77c.