fix(chat): keep steering, action and injected messages in the conversation - #2
Conversation
…ation Source PR: triggerdotdev#4816 Source head: ee8f449
|
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
|
|
||
| Actions can now become turns. `onAction` edits history with `chat.history`; to answer after the edit, return `chat.turn()` and 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`, and persistence. A regenerate is `chat.history.slice(0, -1); return chat.turn();`. | ||
|
|
||
| ```ts |
There was a problem hiding this comment.
Shipwright · HIGH
The new 'chat.turn()' API is introduced in the changeset but the diff does not show its implementation or type signature.
Impact: The new 'chat.turn()' API is introduced in the changeset but the diff does not show its implementation or type signature. The changeset example 'chat.history.slice(0, -1); return chat.turn();' relies on 'chat.history.slice' mutating in place, but the existing docs and tests use 'chat.history.set(...)' and 'chat.history.all()' for mutation. A new hire cannot tell whether 'slice' returns a new array (no-op) or mutates…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| ## Returning a model response from an action | ||
|
|
||
| `onAction` can return a `StreamTextResult`, `string`, or `UIMessage` to produce a response. The returned stream is auto-piped to the frontend just like a normal turn, but the rest of the turn machinery (`onTurnStart`, `onTurnComplete`, etc.) still does not fire. | ||
| `onAction` can return a `StreamTextResult`, `string`, or `UIMessage` to produce a response. All three are sent to the frontend and added to the conversation just like a normal turn's answer, but the rest of the turn machinery (`onTurnStart`, `onTurnComplete`, etc.) still does not fire. A returned `UIMessage` must have `role: "assistant"`; its text and `data-*` parts are delivered, and other part types are dropped. |
There was a problem hiding this comment.
Shipwright · HIGH
The changeset for action-stream-into-conversation documents a breaking API change: returning StreamTextResult, string, or UIMessage from onAction now fails.
Impact: The changeset for action-stream-into-conversation documents a breaking API change: returning StreamTextResult, string, or UIMessage from onAction now fails. The docs/ai-chat/actions.mdx still contains a section titled 'Returning a model response from an action' with code examples showing 'return streamText(...)' and 'return { role: "assistant", ... }'. If shipped together, users following the docs will hit the new r…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| "@trigger.dev/sdk": patch | ||
| --- | ||
|
|
||
| `chat.inject()` with `role: "system"` now works. It previously put the system message into the conversation, which AI SDK 7 rejects for every provider: the next turn died with a generic "An error occurred." and persisted an empty assistant message, so the agent looked like it had stopped answering. System-role context is now appended to the model's instructions, which is also the only way to inject context the agent treats as trusted. |
There was a problem hiding this comment.
Shipwright · HIGH
The changeset 'inject-system-to-instructions.md' states that system-role injections are delivered only via 'chat.toStreamTextOptions()', and a 'run()' that calls 'streamText' witho
Impact: The changeset 'inject-system-to-instructions.md' states that system-role injections are delivered only via 'chat.toStreamTextOptions()', and a 'run()' that calls 'streamText' without spreading it silently drops the injection. This is a silent failure mode: the agent believes it has injected trusted context, but the model never sees it. The docs warn about this, but the SDK does not appear to throw or log when a syst…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| prompts.push(JSON.stringify(prompt)); | ||
| const isToolStep = step++ % 2 === 0; | ||
| return { | ||
| stream: simulateReadableStream({ |
There was a problem hiding this comment.
Shipwright · HIGH
The new 'chat.inject()' system-role lane appends injected content to the model's instructions, which the changeset explicitly calls 'the only way to inject context the agent treats
Impact: The new 'chat.inject()' system-role lane appends injected content to the model's instructions, which the changeset explicitly calls 'the only way to inject context the agent treats as trusted.' The diff does not show any validation, sanitization, or provenance tracking on injected system content. If any part of the injected content is derived from user input (e.g., pendingMessages.prepare output, tool results, or fr…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| * same append-only persistence hole the steering fix exists to close: the app | ||
| * stores what `onTurnComplete` hands it, the failed turn hands it everything | ||
| * except the steer, and the instruction is gone. | ||
| */ |
There was a problem hiding this comment.
Shipwright · LOW
The new test files duplicate a large amount of helper code ('deferred', 'waitFor', 'userMessage', 'toolCallChunks', 'textChunks', 'sendAndLand', 'USAGE') across 'steering-error-pat
Impact: The new test files duplicate a large amount of helper code ('deferred', 'waitFor', 'userMessage', 'toolCallChunks', 'textChunks', 'sendAndLand', 'USAGE') across 'steering-error-path.test.ts', 'steering-history-edit-once.test.ts', and 'steering-prepare-transform.test.ts'. This is a maintainability burden: a fix to the test harness must be applied in three places, and the duplication obscures what each test is actuall…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| sendAction({ type: "regenerate" }); | ||
| ``` | ||
|
|
||
| Previously the frontend docs 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` still returns a stream that callers outside `useChat` must read, and now accepts `{ abortSignal, metadata }`, with per-action metadata merged over the transport's `clientData`. |
There was a problem hiding this comment.
Shipwright · LOW
The changeset 'use-chat-actions.md' documents that 'transport.sendAction' now accepts '{ abortSignal, metadata }' with per-action metadata merged over the transport's 'clientData'.
Impact: The changeset 'use-chat-actions.md' documents that 'transport.sendAction' now accepts '{ abortSignal, metadata }' with per-action metadata merged over the transport's 'clientData'. The diff does not show any validation or size limits on this metadata, and the changeset does not mention whether metadata is persisted, logged, or exposed to other tenants. If metadata is stored in the session snapshot or logs without sa…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
Summary
A steering message sent while the agent was answering
Before: the steer reached the model for the answer it steered, and reached the browser, but never
uiMessagesornewUIMessages, 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.agentkeeps a UI accumulator and a model accumulator, and the drain appended to the UI one only; the model saw the message through theprepareStepreturn value, which is per-step. The model lane is advanced by appending each turn's delta, so it never learned the message existed: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()andchat.MessageAccumulator. Those keep their own accumulator, and the drain recorded what it claimed by pushing into a locals array onlychat.agentpopulates, so there the push was a silent no-op. A mid-turn steer shaped that turn's answer and then existed nowhere: not inturn.uiMessages, not inturn.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
newUIMessagesfrom 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
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
hydrateMessagesthe 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 followschat.turn()reaches your store throughonTurnCompletelike any turn's. The actions page now covers both models; it previously said only that persistence was your responsibility.Injected system context
Before, on AI SDK 7: every provider rejected it (
AI_InvalidPromptErrorfromstandardizePrompt, 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:
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'smessageslacked 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 fromnewMessageslost the model's view of it. And achat.historyedit 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.historyedit 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, sonewMessagesreports the same form the lane holds.An action can become a turn.
onActionis a state edit. To answer after the edit, returnchat.turn(): a turn runs on the edited history with everything a turn has, the agent's system prompt and tools, steering, compaction, injected instructions,onTurnStartandonTurnComplete, numbering and persistence. Returning aStreamTextResult,stringorUIMessagefromonActionis no longer supported and fails with a pointer tochat.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, andrun()receives the turn withtrigger: "action-turn", so a handler that returns early on"action"still answers.Before, a regenerate handler produced the answer itself:
After, it edits and hands off:
Actions travel on
useChat's own request path.TriggerChatTransportrecognisesbody.actionon auseChatrequest and sends it as an action, souseChatowns the response and a turn that follows the action renders like a message turn.useChatActions({ sendMessage })wrapssendMessage(undefined, { body: { action } });regenerate({ body: { action } })works the same way. The frontend docs had saiduseChatconsumed the streamtransport.sendActionreturns; it never did, so an action's answer was never rendered by an app following them.transport.sendActionis unchanged for callers outsideuseChat.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 #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
createSessionfix, 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 thecreateSessionsurface 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
useChatrouting were run deployed in both directions: a regenerate action renders its new answer throughuseChatat the timing that broke the old path, and reverting either the fall-through into the turn or the transport'sbody.actionrouting 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.Source merge-base:
f8aacacb8fa05d5044aa3853dce829eb71f61c48Source head:
ee8f449c5695968277c5c570799ddfaf422892f7