Skip to content

Fix task switch causing duplicate text renderings#3624

Merged
TheodoreSpeaks merged 3 commits intostagingfrom
fix/duplicate-stream
Mar 17, 2026
Merged

Fix task switch causing duplicate text renderings#3624
TheodoreSpeaks merged 3 commits intostagingfrom
fix/duplicate-stream

Conversation

@TheodoreSpeaks
Copy link
Collaborator

Summary

Previous changes to not abort any sse processors caused issues with duplicate processing. Use abort to clean up tasks now.

Decided to revert back processing all sse streams that are open - dev server is limited to 6 concurrent connections and i didn't want to introduce divergent behavior in prod. Think the correct long-term approach is to migrate sse streams to a single stream to handle this, but this only causes issues if a task is being ran in the background.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

Tested switching mothership tasks and validating no errors in duplicate text occurred.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

@vercel
Copy link

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 17, 2026 9:25am

Request Review

@cursor
Copy link

cursor bot commented Mar 17, 2026

PR Summary

Medium Risk
Touches chat streaming/reconnect state management and abort behavior; mistakes could drop/duplicate messages or leave streams running, but changes are localized to use-chat.ts.

Overview
Fixes duplicate assistant text when switching tasks by making SSE stream processing generation-aware and aborting prior streams on reconnect/unmount.

processSSEStream now accepts an expectedGen and ignores/cancels reads and state updates when the stream becomes stale, and reconnect/send paths pass the current generation while also aborting any existing AbortController before starting a new stream.

Written by Cursor Bugbot for commit ece64dc. This will update automatically on new commits. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR re-introduces AbortController.abort() on task-switch and adds a stream-generation counter (streamGenRef) to guard against stale SSE writes — fixing duplicate text that appeared when switching between tasks mid-stream. The fix is applied cleanly to the reconnect path but has a gap in the sendMessage path.

Key changes:

  • processSSEStream gains an optional expectedGen parameter; isStale() uses it to short-circuit flush() and cancel the reader in the main loop.
  • The chatHistory effect now calls abortControllerRef.current?.abort() before starting a fresh reconnect stream.
  • The hook's unmount cleanup now also aborts the controller and nullifies the ref before incrementing the generation counter.
  • The queryClient.setQueryData updater in sendMessage was refactored from an arrow expression to a block body (cosmetic, no behaviour change).

Issue:
sendMessage (line 1071) calls processSSEStream(reader, assistantId) without passing gen, even though gen is in scope. Because expectedGen is undefined for that call path, isStale() always returns false. The !sendingRef.current guard in the task-switch effect also prevents the new abort() call from running while a send is in-flight. Together, these two gaps mean the duplicate-text bug can still surface when the user switches tasks while actively typing / sending a message, not just during reconnects.

Confidence Score: 3/5

  • The reconnect path is fixed correctly, but the user-initiated send path still lacks staleness protection, leaving the duplicate-text bug reproducible during active sends.
  • The PR correctly solves the reconnect-specific duplicate by adding abort + generation-counter guards. However, sendMessage omits the gen argument when calling processSSEStream, and the task-switch effect skips the abort call while sendingRef is true — the exact conditions needed to reproduce the bug via an in-flight user message. Passing gen at line 1071 is a one-line fix that would close the remaining gap.
  • apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts — specifically line 1071 where gen is not forwarded to processSSEStream.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Adds stream-generation staleness guards and abort-on-task-switch for the reconnect path; the sendMessage path still omits gen from processSSEStream, leaving the staleness protection inactive for user-initiated sends.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Task Switch - chatHistory changes]) --> B{appliedChatIdRef equals chatHistory.id?}
    B -- yes --> Z([return - no-op])
    B -- no --> C{activeStreamId AND no snapshot AND sendingRef=false?}
    C -- yes --> D[invalidateQueries and return]
    C -- no --> E[setMessages to new chat messages]
    E --> F{activeStreamId AND sendingRef=false?}
    F -- yes --> G[abortControllerRef.abort - NEW in this PR]
    G --> H[Increment streamGenRef, new AbortController, sendingRef=true]
    H --> I[processSSEStreamRef called with reader, assistantId, gen - gen passed OK]
    I --> J{isStale? streamGenRef not equal expectedGen}
    J -- yes --> K[reader.cancel and break - no stale state update]
    J -- no --> L[flush updates to setMessages]
    F -- "no - sendingRef=true" --> M[No abort called]
    M --> N[sendMessage processSSEStream called with reader and assistantId only - gen NOT passed]
    N --> O{isStale check?}
    O -- "always false - expectedGen is undefined" --> P[setMessages appends stale reply to new chat - DUPLICATE BUG remains]
Loading

Comments Outside Diff (1)

  1. apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts, line 1071 (link)

    P1 gen not forwarded to processSSEStream for user-initiated sends

    The staleness guards (isStale() and the double-check in setMessages) added in this PR are only effective when expectedGen is provided. In the reconnect path (line 470) gen is correctly passed:

    await processSSEStreamRef.current(combinedStream.getReader(), assistantId, gen)

    But the sendMessage path still calls processSSEStream without gen:

    await processSSEStream(response.body.getReader(), assistantId)

    gen is already in scope here (line 967: const gen = ++streamGenRef.current), so expectedGen will always be undefined for user-initiated sends, making every isStale() call return false. This means:

    1. If the user switches tasks while a user-initiated sendMessage is in-flight, sendingRef.current is true — so the new task-switch effect block skips the abortControllerRef.current?.abort() call (the guard at line 399 is !sendingRef.current).
    2. With no abort and no staleness detection, the running processSSEStream appends the assistant reply to the new task's message list, reproducing the duplicate-text bug this PR is fixing.

    Suggested fix:

Last reviewed commit: f73be1a

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@TheodoreSpeaks TheodoreSpeaks merged commit b3d9e54 into staging Mar 17, 2026
12 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the fix/duplicate-stream branch March 17, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant