fix(chat): keep steering, action and injected messages in the conversation - #4816
fix(chat): keep steering, action and injected messages in the conversation#4816ericallam wants to merge 7 commits into
Conversation
The seed from payload.headStartMessages had no coverage for agents that do not register hydrateMessages, and it reads unreachable: it sits inside if (!hydrateMessages && couldHavePriorState), and couldHavePriorState is false on a head-start run. It does fire, and this pins that. Records the shape a persisting app has to handle, which is the part that actually bites: by onTurnStart the accumulator is already ['user','assistant'], because the warm route's partial is spliced in before the hook, so the incoming user message is not the last one.
drainSteeringQueue used the injected uiMessage for span attributes, the injection-confirmation chunk, the injected-ids set and onInjected — never the accumulator. So the message reached the model and the browser, appeared in neither uiMessages nor newUIMessages, and an app persisting from onTurnComplete never learned it existed. The user steers, the agent obeys, the user reloads, and their instruction is gone from the transcript and from every later turn's context. The asymmetry is the tell: a message that finds no step boundary falls back to becoming its own turn and is accumulated normally. Only the path that worked lost data. Appended at injection time rather than turn end, so the order matches what happened: after the message that started the turn, before the response that answers it. Deduplicated by id, since a boundary can drain more than once. The injection path had no test coverage at all — shouldInject appeared only in ai.ts — because the harness had no way to deliver a message mid-turn. Adds harness.sendPendingMessage() for that, which is also what a customer needs to test steering in their own suite.
The snapshot is written on the turn-complete path, and an action is not a turn — the block literally ends 'if (!isAction)'. So a chat.history mutation from onAction lived only in the running worker's memory. Undo worked while that worker stayed warm, then the next continuation booted from a snapshot still holding the undone exchange and the messages came back. onAction is exactly where the docs tell you to call rollbackTo, so this is the documented path silently not persisting. Writes the snapshot right after the action's override is applied, awaited for the same reason as the turn-complete write: the agent may suspend straight after, and in-flight promises do not reliably survive that. An action has no turn cursor, so the write reuses the last one rather than writing undefined — that would drop the resume point and make the next boot replay from further back to rebuild what it could have read.
…ation Returning a StreamTextResult from onAction piped it to the browser and stopped there. The accumulator never saw it, no snapshot recorded it, and actions fire no onTurnComplete — so the user read a good answer that the model had no memory of, and the next turn carried on from the answer regenerate had just replaced. The disagreement between the screen and the conversation was invisible until that next turn contradicted it. The action branch now captures what it pipes, using the pipeChatAndCapture that already existed for exactly this, and appends the message to the accumulator. Persistence beyond the snapshot is still the app's job, since an action fires no turn hook — pipeAndCapture hands back the same message for that. Also folds the snapshot write added for rolled-back history into one helper used by both action paths, so a regenerate that both rolls back and answers writes once rather than twice, and the cursor-preservation rule lives in one place. The two fixes needed each other: with the rollback persisted but the response dropped, a regenerate left the snapshot empty rather than stale — still wrong, just differently.
chat.inject with role 'system' put the message into the conversation, which ai@7 rejects for every provider: standardizePrompt throws before any provider is called. The next turn died with an error chunk reading 'An error occurred.' and persisted an assistant message with no parts, so from the app's side the agent had simply stopped answering. The error message names the fix — use the instructions option — and Instructions is string | SystemModelMessage | Array<SystemModelMessage>, so an injected system block has a correct home. It is appended after the base prompt, which keeps the prompt's position for caching and reads as a later amendment. This makes the documented examples right rather than rewriting them to a workaround. It also answers whether trusted mid-conversation context is supportable: it is, and only this way. A message injected as 'user' is untrusted by construction, and a well-aligned model says so and re-derives the answer from tools instead. The docs now state which lane to use for facts and which for directives. A new instruction block changes the cached prefix, so the first call carrying it misses the prompt cache. Only turns that actually injected pay it.
🦋 Changeset detectedLatest commit: 02c2e6b 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 |
WalkthroughThe SDK routes Merge Risk: 🟡 Moderate · up to This change improves durable chat history, but steering messages may still be omitted from later raw turns, while cancellation or error recovery can lose an action-driven history change or replay already represented output. These bounded continuation and durability risks should be fixed or explicitly accepted before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is detailed and directly explains all four fixes, implementation approach, testing, and regression coverage. It does not include the template's issue-closing line, checklist, or screenshots section, but the core required information is present. Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 8 files. (1 skipped: 1 unsupported.)
✨ 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 |
ericallam
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 968af3c6-2942-4744-9a5d-0540b1d34454
📒 Files selected for processing (14)
.changeset/action-stream-into-conversation.md.changeset/inject-instructions-shape.md.changeset/inject-system-to-instructions.md.changeset/persist-action-history-mutations.md.changeset/steering-messages-accumulator.mddocs/ai-chat/background-injection.mdxpackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (29)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
🧰 Additional context used
📓 Path-based instructions (11)
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` or deprecated `client.defineJob`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/ai-chat/background-injection.mdxpackages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` (deprecated path alias)
📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
MDX documentation pages must include frontmatter with title (required), description (required), and sidebarTitle (optional) in YAML format
📄 CodeRabbit inference engine (docs/CLAUDE.md)
Files:
docs/ai-chat/background-injection.mdx
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/ai.ts
🧠 Learnings (3)
📚 Learning: 2026-08-16T18:36:58.179Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4537
File: packages/trigger-sdk/test/normalizeKeyString.test.ts:1-2
Timestamp: 2026-08-16T18:36:58.179Z
Learning: For related SDK `chat.agent` tests in the Trigger.dev repository—including chat channels, handover, snapshot, and transport-event coverage—keep new test files under `packages/trigger-sdk/test/` rather than colocating them with the `packages/trigger-sdk/src/v3/` source files.
Applied to files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-snapshot-cursor.test.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
packages/trigger-sdk/test/chatHandover.test.ts
📚 Learning: 2026-05-19T22:37:47.286Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3671
File: packages/trigger-sdk/test/recovery-boot.test.ts:456-457
Timestamp: 2026-05-19T22:37:47.286Z
Learning: In `packages/trigger-sdk` (Trigger.dev SDK), `logger.warn` (and other SDK logger methods) should route to the Trigger.dev structured logger sink, not to `console.warn`. In SDK tests, `vi.spyOn(console, "warn")` (or similar console spies) should only be used to suppress stray console output; reviewers should not suggest asserting on `console.warn` spies to verify SDK-internal warning/fallback log behavior. Use the SDK’s structured-logger outputs/capture approach instead of console spies.
Applied to files:
packages/trigger-sdk/src/v3/ai.ts
🪛 ast-grep (0.45.2)
packages/trigger-sdk/src/v3/ai.ts
[warning] 6675-6682: Avoid logging sensitive data
Context: logger.warn(
"chat.agent: snapshot write outside a turn failed; the change may not survive a continuation",
{
error: error instanceof Error ? error.message : String(error),
sessionId: sessionIdForSnapshot,
reason,
}
)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🔇 Additional comments (1)
packages/trigger-sdk/test/chatHandover.test.ts (1)
647-647: 🎯 Functional CorrectnessNo duplicate declaration exists. The file contains one
captureddeclaration at line 647.
| const systemBlocks = messages.filter( | ||
| (message): message is SystemModelMessage => message.role === "system" | ||
| ); | ||
| const conversational = messages.filter((message) => message.role !== "system"); | ||
|
|
||
| if (systemBlocks.length > 0) { | ||
| const instructions = locals.get(chatInjectedInstructionsKey) ?? []; | ||
| instructions.push(...systemBlocks); | ||
| locals.set(chatInjectedInstructionsKey, instructions); | ||
| } | ||
|
|
||
| if (conversational.length > 0) { | ||
| const queue = locals.get(chatBackgroundQueueKey) ?? []; | ||
| queue.push(...conversational); | ||
| locals.set(chatBackgroundQueueKey, queue); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep system injections available to direct streamText() callers.
This code removes system messages from chatBackgroundQueueKey. Only toStreamTextOptions() reads chatInjectedInstructionsKey. A supported run() implementation that calls streamText({ model, messages, abortSignal }) directly now receives neither the injected system message nor a system option.
Do not remove the automatic delivery path until the managed agent can supply the instruction lane to every supported run() path. Add a regression test that injects system context without calling chat.toStreamTextOptions().
There was a problem hiding this comment.
This one is real and I am leaving it open rather than resolving it, because the right answer is a judgement call rather than a quick fix.
Confirming the mechanism you describe: before this change, a system block went into chatBackgroundQueueKey, and the turn loop drains that queue into accumulatedMessages independently of toStreamTextOptions() — so a run() calling streamText({ model, messages, abortSignal }) directly did receive the injected content, as a system message inside messages. That is precisely the shape ai@7 rejects, so on ai@7 that path was already broken (AI_InvalidPromptError from standardizePrompt, before any provider call). On ai@6 it worked. After this change that caller gets nothing, so for ai@6 direct callers this trades a working delivery for a silent one.
The reason I am not simply restoring the old path: there is no way to put a system option on a streamText() call the SDK does not build. toStreamTextOptions() is the only seam. The options are (a) document that system-role injection requires spreading chat.toStreamTextOptions(), and warn when instructions are left unconsumed at turn end — now detectable, since the lane drains on apply as of ca7f086; (b) add a system field to the run() arguments so a direct caller can forward it; (c) keep automatic delivery for ai@6 and route to instructions only on ai@7, which needs a runtime version check this package deliberately avoids (import.meta.url is illegal in its CommonJS output, and a bundled task may have no resolvable ai).
My preference is (a) plus (b) as a follow-up, not in this PR — (b) is an API addition and this PR is a set of fixes. Flagging for a maintainer to choose before merge.
There was a problem hiding this comment.
@ericallam, thank you for the detailed confirmation.
Your analysis identifies the compatibility boundary correctly. The change fixes the ai@7 invalid prompt path, but ai@6 direct streamText() callers no longer receive injected system context.
I agree that restoring the previous queue behavior is not a safe fix. It would retain the ai@7 failure mode. The proposed follow-up is appropriate:
- Document that system-role
chat.inject()requireschat.toStreamTextOptions(). - Warn when a system instruction remains unconsumed at turn end.
- Add a
systemfield to the managedrun()payload so direct callers can forward it.
Would you like me to open a GitHub issue for that follow-up?
✏️ Learnings added
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4816
File: packages/trigger-sdk/src/v3/ai.ts:10031-10046
Timestamp: 2026-08-28T14:16:29.734Z
Learning: In `packages/trigger-sdk/src/v3/ai.ts`, system-role `chat.inject()` context is delivered through `chat.toStreamTextOptions()` because only that helper can supply `streamText`'s `system` option. Direct `streamText({ model, messages, abortSignal })` callers received system messages through `messages` on ai6, but that shape fails on ai7 because AI SDK 7 rejects system messages in `messages`. The intended follow-up is to document the `chat.toStreamTextOptions()` requirement, warn for unconsumed injected instructions, and consider a `system` field on managed `chat.agent` `run()` payloads.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4816
File: packages/trigger-sdk/test/inject-system-instructions.test.ts:3-8
Timestamp: 2026-08-28T14:16:14.265Z
Learning: In `packages/trigger-sdk` chat unit tests, `MockLanguageModelV3`, `simulateReadableStream`, and the `mockChatAgent` harness are approved deterministic test utilities. The AGENTS.md “never mock, use testcontainers” guidance applies to integration tests that use real infrastructure such as Redis or PostgreSQL, not fake language models. Existing chat coverage, including `chat.test.ts`, `mockChatAgent.test.ts`, and `chatHandover.test.ts`, uses this same pattern. Real deployed-agent verification can provide separate provider and transport integration evidence.
You are interacting with an AI system.
There was a problem hiding this comment.
Half of it is now in, in 02c2e6b: the docs page states that the instructions lane is delivered by chat.toStreamTextOptions(), so a run() that calls streamText({ model, messages, abortSignal }) without spreading it will not receive a role: "system" injection — and that the conversational lane has no such requirement. It also documents that an injection applies to the next inference call only, which is the drain behaviour from the previous commit.
Not in, and deliberately: the unconsumed-instruction warning and the system field on run()'s arguments. The warning is harder than it looks — the canonical pattern injects from onTurnComplete, so instructions pending at the end of a turn are the normal case, and a naive check would fire on the documented usage. Distinguishing that from a genuine loss needs per-turn tracking of whether toStreamTextOptions() was called, which is more machinery than I want to add to the turn loop in a fix PR. The system field is an API addition.
On opening an issue for the follow-up: leaving that to a maintainer rather than deciding it here.
There was a problem hiding this comment.
@ericallam, thanks for the clarification.
The documentation makes the new system-instruction contract explicit. It also distinguishes the conversational lane correctly.
I agree that an end-of-turn warning needs per-turn consumption tracking. A naive warning would report the documented onTurnComplete injection flow incorrectly.
The system field remains a separate public API change. I will keep this finding open for maintainer follow-up rather than treat the documentation change as a complete fix.
🐇
🧠 Learnings used
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4816
File: packages/trigger-sdk/src/v3/ai.ts:10031-10046
Timestamp: 2026-08-28T14:16:32.762Z
Learning: In `packages/trigger-sdk/src/v3/ai.ts`, system-role `chat.inject()` context is delivered through `chat.toStreamTextOptions()` because only that helper can supply `streamText`'s `system` option. Direct `streamText({ model, messages, abortSignal })` callers received system messages through `messages` on ai6, but that shape fails on ai7 because AI SDK 7 rejects system messages in `messages`. The intended follow-up is to document the `chat.toStreamTextOptions()` requirement, warn for unconsumed injected instructions, and consider a `system` field on managed `chat.agent` `run()` payloads.
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4816
File: packages/trigger-sdk/test/inject-system-instructions.test.ts:3-8
Timestamp: 2026-08-28T14:16:14.265Z
Learning: In `packages/trigger-sdk` chat unit tests, `MockLanguageModelV3`, `simulateReadableStream`, and the `mockChatAgent` harness are approved deterministic test utilities. The AGENTS.md “never mock, use testcontainers” guidance applies to integration tests that use real infrastructure such as Redis or PostgreSQL, not fake language models. Existing chat coverage, including `chat.test.ts`, `mockChatAgent.test.ts`, and `chatHandover.test.ts`, uses this same pattern. Real deployed-agent verification can provide separate provider and transport integration evidence.
You are interacting with an AI system.
… paths Record only the messages a steering drain actually claimed. The loop used the offered batch, so a record another consumer took while shouldInject() awaited was written into the accumulator for a turn it was never part of. Drain the injected instructions once applied, matching the conversational lane. Left in place they were re-applied by every later toStreamTextOptions() call in the run, growing the prompt and changing its cached prefix each turn. Clean a stopped action's partial response before it is committed, and skip committing at all once the run is cancelled.
@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: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trigger-sdk/src/v3/ai.ts (2)
4267-4276: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftKeep the model accumulator synchronized with injected steering messages.
Lines 4269-4276 update only
chatCurrentUIMessagesKey. They do not updateaccumulatedMessagesorturnNewModelMessages. After this response, the next turn in the same worker usesaccumulatedMessages, so it omits steering that influenced the prior response.onTurnComplete.messagesalso omits that message.Return the claimed UI and model messages from
drainSteeringQueue(). Update both accumulator representations in each caller. Add a regression test that sends a steering message, completes the turn, then verifies the nextrun()call receives that message.
4267-4276: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd required development crumbs.
Add approved crumb instrumentation for these new behavior paths. If no approved namespace fits, ask before adding one.
packages/trigger-sdk/src/v3/ai.ts#L4267-L4276: add crumbs for steering claim and accumulator updates.packages/trigger-sdk/test/inject-system-instructions.test.ts#L179-L234: add crumbs for the one-turn instruction-drain scenario.As per coding guidelines, “Add crumbs as you write code — not just when debugging.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f069755c-6da5-41bc-8e27-c3b1350588ff
📒 Files selected for processing (6)
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/chatHandover.test.tspackages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/test/steering-accumulator.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/trigger-sdk/test/action-stream-accumulator.test.ts
- packages/trigger-sdk/test/chatHandover.test.ts
- packages/trigger-sdk/test/action-snapshot.test.ts
- packages/trigger-sdk/test/steering-accumulator.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (43)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: runops-guard / runops-guard
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: code-quality / code-quality
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (10)
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` or deprecated `client.defineJob`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` (deprecated path alias)
📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/inject-system-instructions.test.tspackages/trigger-sdk/src/v3/ai.ts
🔇 Additional comments (1)
packages/trigger-sdk/test/inject-system-instructions.test.ts (1)
11-27: Replace mock-based chat coverage with the approved test strategy.This test uses
simulateReadableStreamandMockLanguageModelV3. Use the repository Testcontainers-backed fixture instead.As per coding guidelines, “We use vitest exclusively. Never mock anything - use testcontainers instead.”
Also applies to: 179-188
Source: Coding guidelines
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…finished pipeChatAndCapture returns a stream failure rather than throwing it, so a mid-stream failure in a response returned from onAction was committed as a complete answer, snapshotted, and followed by a normal turn-complete with no error — the browser saw the stream stop and the next turn built on the truncated text. The partial is still kept; the failure is now surfaced with it. Document that the instructions lane is delivered by chat.toStreamTextOptions(), and that an injection applies to the next inference call only.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trigger-sdk/src/v3/ai.ts (2)
4267-4276: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPersist claimed steering messages in raw accumulator paths.
drainSteeringQueue()updates onlychatCurrentUIMessagesKeyandchatTurnNewUIMessagesKey.ChatMessageAccumulator.prepareStep()andChatTurn.prepareStep()pass their own queue into this helper, but neither raw accumulator is connected to these locals.After a successful injection, the current inference sees the steering message. The next raw turn and caller-managed persistence do not see it because
uiMessagesandmodelMessageswere not updated.Return the claimed UI and model messages, or add a callback that updates each raw accumulator after a successful claim. Deduplicate by message ID.
8036-8087: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd crumb markers for this new state transition.
Add
//@Crumbsmarkers, or wrap this action stream capture and error path in a `// `#region` `@crumbsblock. This path changes persistence order and error propagation.As per coding guidelines, “Add crumbs as you write code — not just when debugging.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d29e0d9-c508-4a05-bf16-072254964879
📒 Files selected for processing (3)
docs/ai-chat/background-injection.mdxpackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/test/action-stream-accumulator.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (42)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: runops-guard / runops-guard
- GitHub Check: typecheck / typecheck
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
- GitHub Check: code-quality / code-quality
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (11)
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` or deprecated `client.defineJob`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
We use vitest exclusively. **Never mock anything** - use testcontainers instead.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.ts
**Prefer static imports over dynamic imports.** Only use dynamic `import()` when:
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging. Mark lines with
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/ai-chat/background-injection.mdxpackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
Always import from `@trigger.dev/sdk`. Never use `@trigger.dev/sdk/v3` (deprecated path alias)
📄 CodeRabbit inference engine (packages/trigger-sdk/CLAUDE.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
MDX documentation pages must include frontmatter with title (required), description (required), and sidebarTitle (optional) in YAML format
📄 CodeRabbit inference engine (docs/CLAUDE.md)
Files:
docs/ai-chat/background-injection.mdx
Use types over interfaces for TypeScript
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/src/v3/ai.ts
🧠 Learnings (2)
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
packages/trigger-sdk/test/action-stream-accumulator.test.ts
📚 Learning: 2026-08-16T18:36:58.179Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4537
File: packages/trigger-sdk/test/normalizeKeyString.test.ts:1-2
Timestamp: 2026-08-16T18:36:58.179Z
Learning: For related SDK `chat.agent` tests in the Trigger.dev repository—including chat channels, handover, snapshot, and transport-event coverage—keep new test files under `packages/trigger-sdk/test/` rather than colocating them with the `packages/trigger-sdk/src/v3/` source files.
Applied to files:
packages/trigger-sdk/test/action-stream-accumulator.test.ts
🔇 Additional comments (2)
docs/ai-chat/background-injection.mdx (1)
211-233: LGTM!packages/trigger-sdk/test/action-stream-accumulator.test.ts (1)
99-171: LGTM!
1 participant
Summary
Four
chat.agentfixes with one shape between them: a message reached the model or the browser without reaching the durable conversation, so the next turn contradicted what the user had just read.uiMessagesandnewUIMessagesnever held them and an app persisting at turn complete lost them.onActionwas never persisted. The snapshot is written on the turn-complete path, and an action is not a turn — so an undo held while the worker stayed warm, then came back on the next continuation, minutes later and with no error.StreamTextResultreturned fromonActionwas streamed and then dropped. Worst on regenerate: the user reads a new answer the model has no memory of, and the following turn carries on from the answer that was just replaced.chat.inject()with arole: "system"message broke the turn outright. It went intomessages, which ai@7 rejects for every provider (AI_InvalidPromptErrorfromstandardizePrompt, 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. These now go to the instructions lane — where the model also treats them as trusted, which is the point of injecting context in the first place.Fix
Each message is routed to the place it was already meant to reach. The two action fixes share a single snapshot write, so a regenerate that both rolls back history and streams a replacement writes once rather than twice, and that write leaves the resume cursor alone — an action has no turn cursor of its own, and writing an empty one would make the next boot replay from further back.
The first fix closes the durability half of the steering problem. The model-context half is unchanged and still covered by the expected-fail test added in #4795: on the managed path, an injected message still does not appear in the next turn's prompt.
Every fix has a test that fails without it. Each was also 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.