fix(chat): stop losing a user message that arrived mid-turn - #4795
Merged
Conversation
🦋 Changeset detectedLatest commit: b0ecb91 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the two message-loss cases, the router and durability changes, verification results, and known limitations. It does not include the template checklist or an explicit issue closure, but it provides sufficient testing and change details. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
A message arriving while a turn was streaming was handed to the turn's push handler and parked in an in-memory array. The router counts a record handed to a handler as terminally decided, so it stopped holding the resume floor behind it and the turn boundary published a cursor past a message that existed only in this process. A crash before the next turn lost it silently. The handler is now attached only when there is a steering config to feed. Without one the record stays queued on the router, which holds the floor until a turn takes it, and both in-memory wire buffers go away. The wait path already takes from the queue before suspending, so a message that arrived mid-turn is still picked up as the next turn without a round trip. The floor also doubles as the wake cursor, so an over-advanced floor parked a waitpoint nothing would complete. It is now recorded on the wait span to make that diagnosable from a trace. Not addressed here: with a steering config, a declined message is still dropped rather than left queued. That path depends on an unresolved question about what declining should mean.
A message arriving mid-turn with a `pendingMessages` config was routed into a turn-local steering queue. If the batch was declined for injection it was discarded with the turn: never injected, never written to the wire buffer, never answered, and nothing raised at either end. Declining is also the default, since a config without `shouldInject` declines every batch, so the documented default behaviour was the losing one. Notification and consumption are now separate. `observe` on the router tells a consumer a record arrived without taking it, so the record stays queued and keeps holding the resume floor, and injection is the point of consumption: `take` removes exactly the records that were injected. A declined batch never reaches that line, so its records stay queued and become later turns, which is what the docs have always promised. `observe` is rejected on an at-arrival route. An observer there would either have to count as a listener, which would stop an unconsumed stop being discarded and bring back the wedged mailbox, or watch records it cannot affect.
The docs described a mid-turn message becoming the next turn only when there were no more step boundaries, and the client-side lifecycle credited the frontend with auto-sending it. Neither matched the behaviour: a message the agent declines to inject is now held on the backend and answered as the next turn, with no client re-send involved, and that covers an explicit `shouldInject: false` as well as a turn that never reaches a boundary. Also spells out that a declined message keeps its place in the queue, so it survives a crash rather than living only in the worker that received it.
ericallam
force-pushed
the
fix/mid-turn-resume-floor
branch
from
August 27, 2026 08:09
4d2027c to
3dd60c2
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
`shouldInject` and `prepare` can await, so a record could arrive between the batch being assembled and the injection being applied. That record was not in the batch and the callbacks never saw it, but it was taken and cleared along with them, so a message that should have become a later turn was discarded. The batch is now snapshotted before the callbacks run, and only its entries are taken from the router and removed from the queue. Also scopes the deferral guarantee in the docs and the changeset: it holds only when `pendingMessages` actually reaches `streamText`, via `chat.toStreamTextOptions()` or an explicit `prepareStep`. A config without that wiring has nothing to drain the queue and still loses messages. The resume-floor test now waits for the channel sequence to advance rather than to merely exist, since the first message had already advanced it and the old predicate could capture that sequence instead of the second message's.
`clearRoute` discards a queue outright. That is right for the handover route, whose window closes at a turn boundary, and wrong for any replayable route, where anything queued is still owed to a later boot and the resume floor is deliberately held behind it. Nothing calls it that way today, but the messages queue now holds real unanswered user input rather than being drained into an in-memory buffer, so a future caller would silently discard messages instead of merely losing a window. Enforced rather than left as a comment.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The previous commit said a `pendingMessages` config without `chat.toStreamTextOptions()` still loses mid-turn messages. That was true before this branch and is not true on it: the arrival path only observes now, so the record stays queued on the channel whatever happens to the steering queue, and the next turn takes it. Injection is the part that needs the spread. Without it nothing injects, so every mid-turn message is answered as the next turn, which is the documented default rather than a loss. Covers the shape a developer reaches by following the docs for `onReceived` alone, with no `shouldInject` and no spread, so nothing can drain the queue.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
`observe` notifies before the router decides where a record goes, so a record can be seen by an observer and then consumed by a waiting puller rather than queued. If that happened while `shouldInject` was awaiting, the entry was still injected even though its `take` failed, and the same message would be both injected and answered as a turn of its own. Claiming now happens before the transform, and only claimed entries are injected. A failed claim means something else is already answering that message, so dropping it here is what keeps it processed once. The injection chunk and `onInjected` report the claimed set rather than the offered one, and an entry with no `seqNum` (the accumulator's own queue) is kept, since there is no record to claim. Both mid-turn tests now assert no turn has completed before the second message is sent. A text delta having arrived does not prove the turn is still open, so a message landing late would take the ordinary next-turn path and pass without exercising the mid-turn one.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
An observer is notified before the router decides where a record goes, so a parked puller can consume a record an observer has just been told about, and a later take() for it correctly reports false. That ordering is the precondition for the steering drain injecting an entry it does not own, so it is worth holding in place rather than leaving it to be rediscovered by review. Testable here even though the SDK-side drain is not: it needs no model and no step boundary, only a waiter and an ingest.
Claiming before the transform fixed injecting an entry this drain does not own, but introduced a worse failure on the way: `prepare` is caller code and can throw, and by then the records have left the router, so a failing transform consumed the messages and they were never answered at all. Before the reorder a throw left them queued. `take` now returns the record it removed rather than a boolean, `untake` puts one back in sequence order, and the transform runs inside a try that returns every claim before rethrowing. `untake` ignores a record already queued, so a double return cannot duplicate one. The re-raise is deliberate: the turn should still fail, since the caller's transform failed. What changes is that the messages survive it and are answered by a later turn.
Injection had no unit coverage. A prepareStep boundary only exists on a turn that takes more than one step, and nothing in this package produced one, so every steering test asserted arrival and none could reach the drain. Both bugs found in that code during review were found by reading it, not by running it. `twoStepModel` gives a turn a real boundary: step one calls a tool, the tool blocks on a gate the test holds, and step two answers. Holding the tool open is what makes it deterministic, since a message appended while the gate is shut is queued before `prepareStep` runs with no reliance on stream timing. Three cases, each checked against the commit that introduced the bug it guards rather than only observed to pass: - a mid-turn message is injected and not also answered as its own turn - a message arriving after the batch was assembled is left for a later turn, which fails at 3dd60c2 - a claim is returned when `prepare` throws, which fails at e3e6ad7 The middle one needed two attempts. Asserting the late message eventually gets answered passes on the bug, because without the snapshot its model messages are injected into the first turn, so the text appears either way. A second turn-complete is the real discriminator.
…istory The deployed QA lane finds the two surfaces disagree: a `chat.createSession()` recap in the same run recalls a mid-turn steer, while the managed `chat.agent` loop denies it. This pins the managed half at unit level, which the tracked gap has never had. Turn 2's prompt comes back as the original message and the following one, with the injected one absent, so the message reaches the model inside turn 1 and then leaves no trace in history. Verified as a real assertion failure rather than trusting `it.fails`, which would also pass on a timeout. Not fixed here, and not caused by this branch: nothing in it touches the accumulator. Recorded so the day the managed path starts carrying it is noticed, and so the difference between the surfaces has a repro that needs no deployed environment.
ericallam
marked this pull request as ready for review
August 27, 2026 10:43
nicktrn
approved these changes
Aug 27, 2026
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Merged
3 tasks
ericallam
pushed a commit
that referenced
this pull request
Aug 28, 2026
## Summary 4 new features, 12 improvements, 5 bug fixes. ## Improvements - `trigger.dev deploy` now asks the server whether to build with Depot or the native build server unless `--native-build`, `--depot-build`, or `--local-build` is passed, so the native build server can be rolled out per organization without a CLI change. `--local-bundle` and `--detach` now require `--native-build`. ([#4803](#4803)) - Add an experimental `--local-bundle` deploy flag that runs the install and bundling steps on your machine and uploads only the build output; the image is still built remotely. Useful when your project's install step needs tooling or credentials that only exist locally. ([#4331](#4331)) - Send the CLI version header on all API requests so deployments are attributable to a CLI version ([#4778](#4778)) - A message that arrives mid-turn and is not injected into that turn is now answered as the next turn, instead of being dropped. This is what the `pendingMessages` docs have always described, and it applies to the default too: configuring `pendingMessages` without a `shouldInject` declines every batch, which previously meant every mid-turn message was lost with no error at either end. ([#4795](#4795)) ```ts chat.agent({ id: "my-chat", pendingMessages: { onReceived: ({ message }) => logger.info("arrived mid-turn", { id: message.id }), // Only interrupt once the agent has started calling tools. shouldInject: ({ steps }) => steps.length > 0, }, run: async ({ messages, signal }) => streamText({ model, messages, abortSignal: signal, // Required for injection. Without it nothing injects, and every // mid-turn message is answered as the next turn instead. ...chat.toStreamTextOptions(), }), }); ``` A declined message keeps its place in the queue, so it survives a crash and is answered by whichever run picks the conversation up. An injected one is consumed at the moment it is injected, so it is never also answered as a later turn. - Browser chats now keep the active turn open across page reloads when older completion records are replayed. ([#4643](#4643)) - Add `chat.endAndContinue()` so fully hand-rolled custom chat agents can hand a conversation off to a fresh run on the latest deployed task version while preserving unconsumed Session input. ([#4647](#4647)) - Custom chat agents now validate and parse client data declared with `chat.withClientData({ schema })` before passing it to agent code. ([#4646](#4646)) ## Bug fixes - Fixes a case where a chat could silently lose a message. If a message arrived while the agent was between turns and a stop arrived after it, the cursor the next boot resumed from could point past that message, so it was never answered and no error was raised. This affected `chat.agent`, not just custom agents. ([#4644](#4644)) Fixes a recovered answer being cut off. After a crash the agent replays the message it had not answered yet, but it was replaying the stop that arrived after that message too, so the turn answering it was aborted the moment it began. A stop is now only applied to the turn that was live when it arrived. That holds however the stop got there: sent after the last completed turn, or sent to a chat whose most recent turn was completed by an older version of the SDK. One limitation to know about: the recovered answer is persisted correctly, but a chat page that stayed open across the crash keeps showing the partial answer it had already received. Reload the page to see the full recovered answer. Also fixes a retried send being answered twice. When a send was retried and its idempotency claim was lost, the agent could consume the same message a second time. Custom agent loops can now inspect pending chat input without consuming it, and consume one record at a time, with `chat.messages.hasPending()` and `chat.messages.next()`. Records carry stable identifiers so a redelivery is recognisable. ```ts if (await chat.messages.hasPending()) { const record = await chat.messages.next({ timeoutInSeconds: 0 }); if (record) handle(record.payload); } ``` `hasPending()` answers for messages alone, so a message sitting behind a stop, or behind a record this version of the SDK does not recognise, still reports as pending and is still delivered. Anything the agent has no consumer for is discarded rather than left where it would make every message queued behind it undeliverable. `chat.messages.next()` returning `undefined` means no message became consumable before the timeout. `chat.writeTurnComplete()`'s `sessionInEventId` is the cursor that is safe to resume from, not the sequence of the record the turn answered. It is held back behind any message still waiting to be handled, so a value below the record you just handled is expected. - Fixed a chat agent hanging after an interrupted turn: when a run was killed mid-answer (out of memory, crash, or eviction) and only the one message it was answering was still outstanding, the new run never replied to it. That message is now re-answered on the new run. ([#4768](#4768)) - Fix chat transport discarding the next turn after stopping generation. `skipToTurnComplete` is now reset when a new message or action is sent, so a message sent after `stopGeneration` streams normally instead of leaving the chat stuck in a streaming state. ([#4744](#4744)) - Fixes a message sent while the agent was mid-answer being lost if the run then crashed. The cursor written at the end of each turn could point past a message that had arrived during that turn but had not been answered yet, so the next boot skipped it and no error was raised anywhere. Such a message is now held until a turn actually takes it. ([#4795](#4795)) This also removes the in-memory buffer those messages used to sit in, on both `chat.agent` and `chat.createSession()`, so a message waiting for its turn is durable rather than only present in the worker that received it. ## Server changes These changes affect the self-hosted Docker image and Trigger.dev Cloud: - Self-hosted instances can now disable the admin dashboard and user impersonation entirely. See the self-hosting docs for the new setting. ([#4774](#4774)) - The dashboard has two new themes, Black and White, plus appearance options for stronger colors and underlined links. ([#4547](#4547)) - Deployment logs no longer jump to the bottom while you are reading earlier output. Scroll up to pause auto-scroll, and scroll back down or use the new scroll-to-bottom button in the log header to resume following. ([#4776](#4776)) - Customize the runs list: show, hide, and reorder columns, and add smart columns that pull a value straight out of a run's payload, metadata, or output. Your column choices are saved in the page URL, so you can share a view, bookmark it, or save it straight to your favorites. ([#4652](#4652)) - Stop the browser offering to autofill or save environment variable values as saved credentials. ([#4777](#4777)) - Cut webapp CPU usage by about a quarter on the routes that workers call most, freeing headroom at the same request rate. Detailed event-loop blocking traces are no longer recorded by default, because producing them was itself a large part of that cost. ([#4746](#4746)) - When a runs list or runs.list API request spans too much data to complete, it now returns a clear, actionable error asking you to narrow the time range, instead of failing with a generic error. ([#4773](#4773)) - Improved the performance and reliability of the runs list and the runs.list API, especially for large projects and filtered views. ([#4763](#4763)) - New Vercel connections now get version skew protection turned on automatically, so each run uses the task version its deployment shipped with. Automatic atomic deployments are deprecated and no longer offered when you connect a project, but stay available in your Vercel integration settings. ([#4741](#4741)) - The Staging branch setting now shows an upgrade prompt on plans that don't include a Staging environment, instead of looking editable and then silently doing nothing when saved. ([#4784](#4784)) <details> <summary>Raw changeset output</summary> # Releases ## @trigger.dev/build@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.13` ## trigger.dev@4.5.13 ### Patch Changes - `trigger.dev deploy` now asks the server whether to build with Depot or the native build server unless `--native-build`, `--depot-build`, or `--local-build` is passed, so the native build server can be rolled out per organization without a CLI change. `--local-bundle` and `--detach` now require `--native-build`. ([#4803](#4803)) - Add an experimental `--local-bundle` deploy flag that runs the install and bundling steps on your machine and uploads only the build output; the image is still built remotely. Useful when your project's install step needs tooling or credentials that only exist locally. ([#4331](#4331)) - Send the CLI version header on all API requests so deployments are attributable to a CLI version ([#4778](#4778)) - Updated dependencies: - `@trigger.dev/core@4.5.13` - `@trigger.dev/build@4.5.13` - `@trigger.dev/schema-to-json@4.5.13` ## @trigger.dev/core@4.5.13 ### Patch Changes - `trigger.dev deploy` now asks the server whether to build with Depot or the native build server unless `--native-build`, `--depot-build`, or `--local-build` is passed, so the native build server can be rolled out per organization without a CLI change. `--local-bundle` and `--detach` now require `--native-build`. ([#4803](#4803)) - Add an experimental `--local-bundle` deploy flag that runs the install and bundling steps on your machine and uploads only the build output; the image is still built remotely. Useful when your project's install step needs tooling or credentials that only exist locally. ([#4331](#4331)) - A message that arrives mid-turn and is not injected into that turn is now answered as the next turn, instead of being dropped. This is what the `pendingMessages` docs have always described, and it applies to the default too: configuring `pendingMessages` without a `shouldInject` declines every batch, which previously meant every mid-turn message was lost with no error at either end. ([#4795](#4795)) ```ts chat.agent({ id: "my-chat", pendingMessages: { onReceived: ({ message }) => logger.info("arrived mid-turn", { id: message.id }), // Only interrupt once the agent has started calling tools. shouldInject: ({ steps }) => steps.length > 0, }, run: async ({ messages, signal }) => streamText({ model, messages, abortSignal: signal, // Required for injection. Without it nothing injects, and every // mid-turn message is answered as the next turn instead. ...chat.toStreamTextOptions(), }), }); ``` A declined message keeps its place in the queue, so it survives a crash and is answered by whichever run picks the conversation up. An injected one is consumed at the moment it is injected, so it is never also answered as a later turn. - Fixes a case where a chat could silently lose a message. If a message arrived while the agent was between turns and a stop arrived after it, the cursor the next boot resumed from could point past that message, so it was never answered and no error was raised. This affected `chat.agent`, not just custom agents. ([#4644](#4644)) Fixes a recovered answer being cut off. After a crash the agent replays the message it had not answered yet, but it was replaying the stop that arrived after that message too, so the turn answering it was aborted the moment it began. A stop is now only applied to the turn that was live when it arrived. That holds however the stop got there: sent after the last completed turn, or sent to a chat whose most recent turn was completed by an older version of the SDK. One limitation to know about: the recovered answer is persisted correctly, but a chat page that stayed open across the crash keeps showing the partial answer it had already received. Reload the page to see the full recovered answer. Also fixes a retried send being answered twice. When a send was retried and its idempotency claim was lost, the agent could consume the same message a second time. Custom agent loops can now inspect pending chat input without consuming it, and consume one record at a time, with `chat.messages.hasPending()` and `chat.messages.next()`. Records carry stable identifiers so a redelivery is recognisable. ```ts if (await chat.messages.hasPending()) { const record = await chat.messages.next({ timeoutInSeconds: 0 }); if (record) handle(record.payload); } ``` `hasPending()` answers for messages alone, so a message sitting behind a stop, or behind a record this version of the SDK does not recognise, still reports as pending and is still delivered. Anything the agent has no consumer for is discarded rather than left where it would make every message queued behind it undeliverable. `chat.messages.next()` returning `undefined` means no message became consumable before the timeout. `chat.writeTurnComplete()`'s `sessionInEventId` is the cursor that is safe to resume from, not the sequence of the record the turn answered. It is held back behind any message still waiting to be handled, so a value below the record you just handled is expected. ## @trigger.dev/python@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/sdk@4.5.13` - `@trigger.dev/core@4.5.13` - `@trigger.dev/build@4.5.13` ## @trigger.dev/react-hooks@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.13` ## @trigger.dev/redis-worker@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.13` ## @trigger.dev/rsc@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.13` ## @trigger.dev/schema-to-json@4.5.13 ### Patch Changes - Updated dependencies: - `@trigger.dev/core@4.5.13` ## @trigger.dev/sdk@4.5.13 ### Patch Changes - Fixed a chat agent hanging after an interrupted turn: when a run was killed mid-answer (out of memory, crash, or eviction) and only the one message it was answering was still outstanding, the new run never replied to it. That message is now re-answered on the new run. ([#4768](#4768)) - Browser chats now keep the active turn open across page reloads when older completion records are replayed. ([#4643](#4643)) - Add `chat.endAndContinue()` so fully hand-rolled custom chat agents can hand a conversation off to a fresh run on the latest deployed task version while preserving unconsumed Session input. ([#4647](#4647)) - Fix chat transport discarding the next turn after stopping generation. `skipToTurnComplete` is now reset when a new message or action is sent, so a message sent after `stopGeneration` streams normally instead of leaving the chat stuck in a streaming state. ([#4744](#4744)) - Custom chat agents now validate and parse client data declared with `chat.withClientData({ schema })` before passing it to agent code. ([#4646](#4646)) - Fixes a message sent while the agent was mid-answer being lost if the run then crashed. The cursor written at the end of each turn could point past a message that had arrived during that turn but had not been answered yet, so the next boot skipped it and no error was raised anywhere. Such a message is now held until a turn actually takes it. ([#4795](#4795)) This also removes the in-memory buffer those messages used to sit in, on both `chat.agent` and `chat.createSession()`, so a message waiting for its turn is durable rather than only present in the worker that received it. - A message that arrives mid-turn and is not injected into that turn is now answered as the next turn, instead of being dropped. This is what the `pendingMessages` docs have always described, and it applies to the default too: configuring `pendingMessages` without a `shouldInject` declines every batch, which previously meant every mid-turn message was lost with no error at either end. ([#4795](#4795)) ```ts chat.agent({ id: "my-chat", pendingMessages: { onReceived: ({ message }) => logger.info("arrived mid-turn", { id: message.id }), // Only interrupt once the agent has started calling tools. shouldInject: ({ steps }) => steps.length > 0, }, run: async ({ messages, signal }) => streamText({ model, messages, abortSignal: signal, // Required for injection. Without it nothing injects, and every // mid-turn message is answered as the next turn instead. ...chat.toStreamTextOptions(), }), }); ``` A declined message keeps its place in the queue, so it survives a crash and is answered by whichever run picks the conversation up. An injected one is consumed at the moment it is injected, so it is never also answered as a later turn. - Fixes a case where a chat could silently lose a message. If a message arrived while the agent was between turns and a stop arrived after it, the cursor the next boot resumed from could point past that message, so it was never answered and no error was raised. This affected `chat.agent`, not just custom agents. ([#4644](#4644)) Fixes a recovered answer being cut off. After a crash the agent replays the message it had not answered yet, but it was replaying the stop that arrived after that message too, so the turn answering it was aborted the moment it began. A stop is now only applied to the turn that was live when it arrived. That holds however the stop got there: sent after the last completed turn, or sent to a chat whose most recent turn was completed by an older version of the SDK. One limitation to know about: the recovered answer is persisted correctly, but a chat page that stayed open across the crash keeps showing the partial answer it had already received. Reload the page to see the full recovered answer. Also fixes a retried send being answered twice. When a send was retried and its idempotency claim was lost, the agent could consume the same message a second time. Custom agent loops can now inspect pending chat input without consuming it, and consume one record at a time, with `chat.messages.hasPending()` and `chat.messages.next()`. Records carry stable identifiers so a redelivery is recognisable. ```ts if (await chat.messages.hasPending()) { const record = await chat.messages.next({ timeoutInSeconds: 0 }); if (record) handle(record.payload); } ``` `hasPending()` answers for messages alone, so a message sitting behind a stop, or behind a record this version of the SDK does not recognise, still reports as pending and is still delivered. Anything the agent has no consumer for is discarded rather than left where it would make every message queued behind it undeliverable. `chat.messages.next()` returning `undefined` means no message became consumable before the timeout. `chat.writeTurnComplete()`'s `sessionInEventId` is the cursor that is safe to resume from, not the sequence of the record the turn answered. It is held back behind any message still waiting to be handled, so a value below the record you just handled is expected. - Updated dependencies: - `@trigger.dev/core@4.5.13` </details> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
pull Bot
pushed a commit
to Stars1233/trigger.dev
that referenced
this pull request
Sep 7, 2026
…ation (triggerdotdev#4816) ## Summary **A steering message sent while the agent was answering** ```ts onTurnComplete: async ({ newUIMessages }) => { await db.saveMessages(newUIMessages); }, ``` Before: the steer reached the model for the answer it steered, and reached the browser, but never `uiMessages` or `newUIMessages`, so it was never saved and it disappeared on reload. Now it is in both. The model also forgot it from the next turn onwards. `chat.agent` keeps a UI accumulator and a model accumulator, and the drain appended to the UI one only; the model saw the message through the `prepareStep` return value, which is per-step. The model lane is advanced by appending each turn's delta, so it never learned the message existed: ``` turn 1 accumulator → [user, steer, assistant] the UI, the snapshot and chat.history.* all have it turn 2 model prompt → [user, next-user] the model answers as though it was never sent ``` The drain now hands back what it claimed and the model lane is appended to before the response is, so the order stays steer-then-answer. Appended rather than rebuilt from the UI lane: compaction replaces the model lane with a summary and deliberately leaves the UI lane whole, so a rebuild restores every message the summary had replaced. A first version of this fix did exactly that, caught in review; the steer was present in the next prompt and so was the whole pre-compaction transcript. This is also the surface disagreement the QA lane reported: a recap in the same run recalled a mid-turn steer while the managed loop denied it. The recap was reading the persisted snapshot, which is written from the UI lane. Both now agree. **The same on `chat.createSession()` and `chat.MessageAccumulator`.** Those keep their own accumulator, and the drain recorded what it claimed by pushing into a locals array only `chat.agent` populates, so there the push was a silent no-op. A mid-turn steer shaped that turn's answer and then existed nowhere: not in `turn.uiMessages`, not in `turn.messages`, and not queued as its own turn either. The drain now returns what it claimed and each surface records it in both of its lanes, appending for the same reason as above. **A steer on a turn that then fails.** The error path built `newUIMessages` from the wire message and the partial only, so a turn that failed after a steer reported everything except the steer. It is now seeded from the per-turn list. This only affected a stream that rejects (a transport failure); an AI SDK error part completes the stream and was never affected. **An undo, edit, or regenerate** ```ts onAction: async ({ action }) => { if (action.type === "undo") chat.history.slice(0, -2); }, ``` Before: the rollback lived only in the running worker. It held while that worker stayed warm, then the next continuation booted from a snapshot that still contained the undone messages. They came back, minutes later, with no error. Now the action writes the snapshot. **The rollback fix is for platform-managed persistence.** With `hydrateMessages` the runtime deliberately does not write, because your store is the source of truth, so a rollback is still yours to save, and the answer that follows `chat.turn()` reaches your store through `onTurnComplete` like any turn's. The actions page now covers both models; it previously said only that persistence was your responsibility. **Injected system context** ```ts chat.inject([{ role: "system", content: "The user just upgraded to Pro." }]); ``` Before, on AI SDK 7: every provider rejected it (`AI_InvalidPromptError` from `standardizePrompt`, thrown before any provider call). The turn ended in the app's error fallback and persisted an assistant message with no parts, so the agent looked like it had stopped answering. Now it is appended to the model's instructions, where it is also treated as trusted, which is the reason to inject context in the first place. Instructions are delivered by the helper, so a system-role injection needs it: ```ts run: async ({ messages, signal }) => streamText({ ...chat.toStreamTextOptions(), // without this, a system injection never arrives model, messages, abortSignal: signal, }), ``` The conversational lane has no such requirement. An injection also applies to the next turn only, rather than repeating on every turn after it, and within that turn it is consumed once rather than once per read, so a `run()` that builds options more than once sees the same instructions in every build. **An edit-only action is not a turn.** It used to share the turn's completion path, which fired `onTurnComplete`, kept the turn number, and consumed the one-shot instruction lane. The action branch now writes its own snapshot and completion, so the next real turn is still the next turn and still receives an instruction injected before the action. **The snapshot cursor after a failed turn.** The error path wrote its snapshot with the failed turn's completion cursor but never updated the shared cursor, so a later history-changing action, whose snapshot is cursor-neutral and reuses it, wrote the cursor from before the failed turn. A continuation would then resume from there and replay output the failed turn had superseded. The cursor moves on the error path now. This one has unit coverage only: the value is decided in-process before the upload, and the test reads the same write directly. **A steer transformed by `pendingMessages.prepare`.** The steered turn saw the transformed form; later turns saw the raw message reconverted. The pending list now carries the model messages the drain actually injected, and reconciliation appends those, on both surfaces. **A steer on a turn that then fails, in the model lane.** The previous round reported it to the hook's `newUIMessages`; it was still left pending in the model lane, so the failed turn's `messages` lacked it and the next turn received it one slot late. The catch path reconciles it now, before the partial is considered. **The steer in `onTurnComplete.newMessages`, and a history edit after a steer.** The per-turn model delta the hook reports never received the steer's model form, so append-only persistence from `newMessages` lost the model's view of it. And a `chat.history` edit after a steer was drained rebuilt the model lane from the UI lane, which already held the steer, then appended it again, so later turns received it twice. Reconciliation now writes the delta too and skips the lane append for anything a rebuild already placed. **A prepared steer after a history edit, and in a failed turn's delta.** A `chat.history` edit rebuilt the model lane from the UI lane, which put the steer's raw form back and, when a compaction override replaced that lane in the same turn, left the steer with no form at all. The rebuild now leaves consumed steers out and reconciliation appends the prepared form once; a steer the edit removed stays removed. The failed-turn delta is likewise built from the recorded forms rather than by converting the UI list, so `newMessages` reports the same form the lane holds. **An action can become a turn.** `onAction` is a state edit. To answer after the edit, return `chat.turn()`: a turn runs on the edited history with everything a turn has, the agent's system prompt and tools, steering, compaction, injected instructions, `onTurnStart` and `onTurnComplete`, numbering and persistence. Returning a `StreamTextResult`, `string` or `UIMessage` from `onAction` is no longer supported and fails with a pointer to `chat.turn()`. That path was a turn without a turn's guarantees, each of which had to be re-added by hand, and its delivery to the browser was unreliable. The edit is snapshotted before the turn starts, so a turn cut short continues from the edited history, and `run()` receives the turn with `trigger: "action-turn"`, so a handler that returns early on `"action"` still answers. Before, a regenerate handler produced the answer itself: ```ts onAction: async ({ action, streamText }) => { if (action.type === "regenerate") { chat.history.slice(0, -1); return streamText({ model, messages: await convertToModelMessages(chat.history.all()) }); } } ``` After, it edits and hands off: ```ts onAction: async ({ action }) => { if (action.type === "regenerate") { chat.history.slice(0, -1); return chat.turn(); } } ``` **Actions travel on `useChat`'s own request path.** `TriggerChatTransport` recognises `body.action` on a `useChat` request and sends it as an action, so `useChat` owns the response and a turn that follows the action renders like a message turn. `useChatActions({ sendMessage })` wraps `sendMessage(undefined, { body: { action } })`; `regenerate({ body: { action } })` works the same way. The frontend docs had said `useChat` consumed the stream `transport.sendAction` returns; it never did, so an action's answer was never rendered by an app following them. `transport.sendAction` is unchanged for callers outside `useChat`. **Approving a tool call no longer undoes compaction.** A tool-approval response arrives as an update to the existing assistant message, and that path rebuilt the model lane from the UI lane, at the start of the continuation and again when its response was committed. A chat that had been summarised to fit the context window was sent the whole transcript on the next call. The replaced message's run of model messages is now swapped in place, with a fallback to the old reconversion if the lane's tail does not match what that message contributed. ## Verification Each of the four has a test that fails without it, and each was run end to end against a deployed agent twice, once with the fix present and once with only that fix reverted, so the tests are known to fail in its absence rather than merely to pass in its presence. A 46-scenario sweep of the surrounding chat surface came back clean. One later fix, recording only the steering messages a drain actually claimed, has unit coverage only: reproducing it needs a second consumer taking a record while `shouldInject()` awaits, which the deployed harness cannot produce. The steering fix closes both halves: the durability one, and the model-context one that [triggerdotdev#4795](triggerdotdev#4795) left behind as an expected-fail test. That test is now a passing test, verified red first (turn 2's user prompts came back without the steer). The model-context fix, the `createSession` fix, the compaction interaction on both surfaces, and the failed-turn path were each run end to end against a deployed agent in both directions, with a runId guard confirming the later turns belonged to the same live run. One bundle carried the compaction regression on the `createSession` surface only: on it the compaction leg failed and the no-compaction steering leg passed, which is a direct demonstration that the earlier steering coverage was blind to the compaction interaction. The second review round's fixes (failed action, prepared steer form, failed-turn reconciliation) were run the same way, deployed in both directions. The snapshot-cursor fix has unit coverage only: the value is decided in-process before the upload. The third round (the steer in `newMessages`, and once after a history edit) was run deployed in both directions too; the duplicate count under the reverted build doubles as proof the history-edit rebuild path ran. The fourth round (a prepared steer through a history edit, with and without compaction, and in a failed turn's delta) was run deployed in both directions; the history-edit case was proven against two different reverts, since removing one half of the old code produces a duplicate and removing both makes the steer vanish. The fifth round (the tool-approval continuation) was run deployed in both directions too; the approval case was proven against each replace site separately, with a following-turn assertion that catches the response-commit site, which the continuation's own prompt cannot see. The action-to-turn path and the `useChat` routing were run deployed in both directions: a regenerate action renders its new answer through `useChat` at the timing that broke the old path, and reverting either the fall-through into the turn or the transport's `body.action` routing makes it fail. The action-reply legs from the earlier rounds are retired with the feature they tested. An action that lands while a turn is still streaming is still spliced into that turn's request stream (a pre-existing client race, not addressed here). The docs for the action model live on triggerdotdev#4884, since those pages also carry that branch's changes. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #4644, now rebased onto main so the diff is just these three commits.
Summary
Two ways a chat could lose a user message, both pre-existing and both raised while reviewing #4644.
A message arriving while a turn was streaming was handed to that turn's push handler and parked in an in-memory array. The router counts a record handed to a handler as terminally decided, so it stopped holding the resume floor behind it, and the turn boundary published a cursor past a message that existed only in that process. A crash before the next turn lost it, silently. Measured: with the message at sequence 1, the boundary published
session-in-event-id: 1, so a resume skipped it.Separately, a message the agent declined to inject was discarded with the turn. Never injected, never written to the wire buffer, never answered. That was also the documented default, since a
pendingMessagesconfig withoutshouldInjectdeclines every batch.Design
Notification and consumption are now separate concerns on the router.
observereports that a record arrived without taking it, so the record stays queued and keeps holding the floor. It is rejected on anat-arrivalroute: an observer there would either have to count as a listener, which would stop an unconsumed stop being discarded and bring back a wedged mailbox, or watch records it cannot affect.takeremoves exactly one queued record.The managed loop and the
chat.createSession()iterator now only subscribe when there is a steering config to feed, and injection is the point of consumption. A declined batch never reaches the take, so its records stay queued and become later turns. Both in-memory wire buffers are gone, so a message waiting for its turn is durable rather than living in whichever worker received it.The floor doubles as the wake cursor:
awaitWakeregisters with it and the server completes the waitpoint immediately if anything sits after that sequence. An over-advanced floor was therefore also a missed wake. It is now recorded on the wait span so a run that never woke can be diagnosed from its trace.Verification
Both fixes have a red and green pair, each checked against the unmodified source rather than only observed to pass:
Also 8 new router tests for
observeandtake. Suites green at 385 for the SDK and 886 for core.Not addressed
A
pendingMessagesconfig with nochat.toStreamTextOptions()spread still swallows messages, because nothing drains the queue at all. Same shape, different trigger, tracked separately.