feat(chat): hand run() a streamText with the managed options already applied - #4884
feat(chat): hand run() a streamText with the managed options already applied#4884ericallam wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: f36bb10 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 now supplies managed Merge Risk: 🟡 Moderate · up to Some supported prompt configurations can be silently ignored, and copied action or Head Start examples can fail or use incorrect context. These issues should be corrected before merging the new public API. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (17 skipped: 17 unsupported.) Full details: Description checkExplanation The description provides a detailed summary, before-and-after examples, behavior details, API changes, and verification results. It does not include the template's Closes issue line, checklist, changelog, or screenshots sections, but the core change and testing information are complete.
✨ 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 |
Spreading chat.toStreamTextOptions() is the integration point for six things:
the managed prompt and its cache control, the resolved model, the prompt's
sampling config, telemetry, the skill tools, and the prepareStep that delivers
steering, compaction and injected context. Forgetting the spread drops all six
in silence, and spread order decides whether passing your own tools or
prepareStep clobbers the managed ones.
run() now receives a streamText with those options applied, so the managed
state cannot be lost by omission and the merge happens inside rather than at
the call site: tools go into the helper so skills survive, a caller system
becomes the base the prompt and injections append to, and a caller prepareStep
composes after the managed one instead of replacing it.
The signature is borrowed with typeof import("ai").streamText rather than
restated, so it resolves to whichever of ai v5/v6/v7 the user installed. The
runtime value rides the existing ESM/CJS shim that already isolates value
imports from ai.
PROTOTYPE. Typechecks and passes the suite on ai@6.0.116 and ai@7.0.66, but
adds a public registry option, does not settle what happens when caller and
managed system are both structured, and has no test for the composed
prepareStep.
4984f70 to
f36bb10
Compare
@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: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md (1)
45-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove remaining direct
streamTextreferences from managed-agent examples.
packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md#L45-L45: remove the unusedstreamTextimport fromai.packages/trigger-sdk/skills/trigger-chat-agent-advanced/SKILL.md#L261-L261: destructurestreamTextfromrunbefore calling it.docs/ai-chat/background-injection.mdx (1)
210-216: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the obsolete manual-options guidance.
Managed
streamTextin an agentrunalready applies the agent prompt, injections, skills, and resolved tools. These sections still requirechat.toStreamTextOptions()for that behavior. Keep that helper as an alternative for manual option assembly, but do not present it as required for managedstreamText.
docs/ai-chat/background-injection.mdx#L210-L216: State that callback-providedstreamTextdelivers system injections.docs/ai-chat/migrating-from-a-route-handler.mdx#L176-L177: State that resolvedtoolsgo directly to managedstreamText.docs/ai-chat/patterns/skills.mdx#L120-L120: Describe automatic skill-tool injection through managedstreamText.docs/ai-chat/patterns/skills.mdx#L164-L164: Instruct users to pass custom tools directly to managedstreamText.docs/ai-chat/patterns/skills.mdx#L181-L181: Describe managed tool merging and agent-config history conversion.docs/ai-chat/compaction.mdx (1)
48-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the
prepareStepcomposition contract.Managed
streamTextcomposes callerprepareSteplogic after the managed behavior. It does not let a caller override compaction or pending-message injection by property order. The current text gives users the wrong failure model.
docs/ai-chat/compaction.mdx#L48-L50: Replace the override statement with the composition behavior.docs/ai-chat/pending-messages.mdx#L54-L54: Replace the spread-order instruction with managedprepareStepcomposition behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: d40c7915-2a3c-4603-9293-5214ccaa08df
📒 Files selected for processing (23)
.changeset/managed-streamtext-in-run.mddocs/ai-chat/actions.mdxdocs/ai-chat/anatomy.mdxdocs/ai-chat/backend.mdxdocs/ai-chat/background-injection.mdxdocs/ai-chat/compaction.mdxdocs/ai-chat/fast-starts.mdxdocs/ai-chat/migrating-from-a-route-handler.mdxdocs/ai-chat/patterns/skills.mdxdocs/ai-chat/pending-messages.mdxdocs/ai-chat/prompt-caching.mdxdocs/ai-chat/quick-start.mdxdocs/ai-chat/reference.mdxdocs/ai-chat/tools.mdxpackages/cli-v3/skills/trigger-authoring-chat-agent/SKILL.mdpackages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.mdpackages/trigger-sdk/skills/trigger-chat-agent-advanced/SKILL.mdpackages/trigger-sdk/src/imports/ai-runtime-cjs.ctspackages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.test.tspackages/trigger-sdk/src/v3/ai.ts
We use vitest exclusively.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/test/bound-streamtext.test.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.test.tspackages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/ai-chat/quick-start.mdxpackages/trigger-sdk/skills/trigger-chat-agent-advanced/SKILL.mddocs/ai-chat/actions.mdxdocs/ai-chat/background-injection.mdxpackages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.mddocs/ai-chat/patterns/skills.mdxdocs/ai-chat/backend.mdxdocs/ai-chat/prompt-caching.mdxdocs/ai-chat/reference.mdxdocs/ai-chat/fast-starts.mdxdocs/ai-chat/migrating-from-a-route-handler.mdxdocs/ai-chat/anatomy.mdxpackages/trigger-sdk/src/imports/ai-runtime-cjs.ctsdocs/ai-chat/tools.mdxpackages/cli-v3/skills/trigger-authoring-chat-agent/SKILL.mdpackages/trigger-sdk/src/imports/ai-runtime.tsdocs/ai-chat/pending-messages.mdxpackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.test.tsdocs/ai-chat/compaction.mdxpackages/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/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.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/src/v3/chat-server.test.tspackages/trigger-sdk/test/bound-streamtext.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.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 Use Mintlify components for structured content: , , , , ,
📄 CodeRabbit inference engine (docs/CLAUDE.md)
Files:
docs/ai-chat/quick-start.mdxdocs/ai-chat/actions.mdxdocs/ai-chat/background-injection.mdxdocs/ai-chat/patterns/skills.mdxdocs/ai-chat/backend.mdxdocs/ai-chat/prompt-caching.mdxdocs/ai-chat/reference.mdxdocs/ai-chat/fast-starts.mdxdocs/ai-chat/migrating-from-a-route-handler.mdxdocs/ai-chat/anatomy.mdxdocs/ai-chat/tools.mdxdocs/ai-chat/pending-messages.mdxdocs/ai-chat/compaction.mdx
docs/ai-chat/quick-start.mdxdocs/ai-chat/actions.mdxdocs/ai-chat/background-injection.mdxdocs/ai-chat/patterns/skills.mdxdocs/ai-chat/backend.mdxdocs/ai-chat/prompt-caching.mdxdocs/ai-chat/reference.mdxdocs/ai-chat/fast-starts.mdxdocs/ai-chat/migrating-from-a-route-handler.mdxdocs/ai-chat/anatomy.mdxdocs/ai-chat/tools.mdxdocs/ai-chat/pending-messages.mdxdocs/ai-chat/compaction.mdxUse types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.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 Do not use high-cardinality attributes in OTEL metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/src/imports/ai-runtime.tspackages/trigger-sdk/src/v3/chat-server.test.tspackages/trigger-sdk/src/v3/chat-server.tspackages/trigger-sdk/test/bound-streamtext.test.tspackages/trigger-sdk/src/v3/ai.ts
🧠 Learnings (2)
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
packages/trigger-sdk/src/v3/chat-server.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/bound-streamtext.test.ts
🪛 LanguageTool
.changeset/managed-streamtext-in-run.md
[style] ~5-~5: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...eady applied, so they cannot be lost by leaving out the spread: ```ts run: async ({ messag...
(OMIT_EXCLUDE)
[style] ~12-~12: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ...instead, so neither can be turned off by accident. system` can be set at the call site,...
(EN_WORDINESS_PREMIUM_BY_ACCIDENT)
[style] ~14-~14: To form a complete sentence, be sure to include a subject.
Context: ...an be turned off by accident. system can be set at the call site, on `chat.agent...
(MISSING_IT_THERE)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...at.headStartandchat.startHeadStarthand theirrun` the same thing, carrying the four optio...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (14)
packages/trigger-sdk/src/imports/ai-runtime.ts (1)
23-23: LGTM!Also applies to: 38-38
packages/trigger-sdk/test/bound-streamtext.test.ts (1)
390-422: LGTM!packages/trigger-sdk/src/v3/chat-server.ts (1)
350-350: LGTM!Also applies to: 437-437
packages/trigger-sdk/src/v3/chat-server.test.ts (1)
209-252: LGTM!.changeset/managed-streamtext-in-run.md (1)
1-20: LGTM!docs/ai-chat/actions.mdx (1)
47-47: LGTM!Also applies to: 86-86, 112-112
docs/ai-chat/anatomy.mdx (1)
21-21: LGTM!Also applies to: 39-41
docs/ai-chat/backend.mdx (1)
33-34: LGTM!Also applies to: 137-137, 212-214, 235-240, 256-263, 301-301, 330-330, 343-343, 396-396, 432-437, 463-463, 481-482, 593-593, 618-618, 641-641, 652-652, 687-687, 718-718, 736-736, 756-756, 774-774, 799-800
docs/ai-chat/reference.mdx (1)
55-58: LGTM!Also applies to: 105-105, 248-248
docs/ai-chat/tools.mdx (1)
11-11: LGTM!Also applies to: 26-28, 83-85, 107-109, 134-137
packages/cli-v3/skills/trigger-authoring-chat-agent/SKILL.md (1)
5-5: LGTM!Also applies to: 19-22, 26-36, 40-42
packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md (1)
5-6: LGTM!Also applies to: 50-52, 125-137, 162-166, 207-208, 237-238, 247-259, 268-269
packages/trigger-sdk/skills/trigger-chat-agent-advanced/SKILL.md (2)
163-166: LGTM!Also applies to: 171-175, 193-200, 213-214, 234-241, 268-271
146-150: 🔒 Security & PrivacyAvoid injecting untrusted analysis as system instructions.
If
analyzeConversation(messages)can return user-influenced text, do not inject its free-form output withrole: "system". Use an untrusted message or validate a structured, allowlisted result first.
|
|
||
| ```ts | ||
| onAction: async ({ action, messages }) => { | ||
| onAction: async ({ action, messages, streamText }) => { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge triggerdotdev/trigger.dev /tmp/coderabbit-repo-knowledge/triggerdotdev-trigger-dev-0bdd0019/learnings
Length of output: 19367
🏁 Script executed:
#!/bin/bash
set -eu
file="$(git ls-files | rg '(^|/)docs/ai-chat/actions\.mdx$' | head -n 1)"
printf '%s\n' "FILE=$file"
cat -n "$file" | sed -n '1,145p'
printf '\n-- related symbols and examples --\n'
rg -n -C 4 'onAction|chat\.history|streamText|regenerate|messages' "$file"
printf '\n-- package and SDK references --\n'
rg -n -C 3 'onAction|streamText|chat\.history' --glob '!docs/ai-chat/actions.mdx' --glob '*.{ts,tsx,js,jsx,mts,md,mdx,json}' | head -n 240Repository: triggerdotdev/trigger.dev
Length of output: 16202
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '-- candidate files --'
git ls-files | rg '(ai-chat|chat|AgentChat|agent)' | head -n 240
printf '\n%s\n' '-- callback construction and history APIs --'
rg -n -C 5 'onAction|history\.slice|class .*History|function .*History|ChatHistory|action.*messages|messages.*action' --glob '*.{ts,tsx,js,jsx,mts,md,mdx}' . | head -n 320Repository: triggerdotdev/trigger.dev
Length of output: 41273
🏁 Script executed:
#!/bin/bash
set -eu
file=packages/trigger-sdk/src/v3/ai.ts
printf '%s\n' '-- action-related declarations and callback invocation --'
rg -n -C 8 'type .*Action|interface .*Action|onAction|actionSchema|chatHistory|chatCurrentUIMessagesKey|chatOverrideMessagesKey|apply.*override|overrideMessages' "$file" | head -n 420
printf '\n%s\n' '-- relevant implementation ranges --'
cat -n "$file" | sed -n '2840,2910p;3145,3275p;3400,3525p'
printf '\n%s\n' '-- exact onAction references in SDK --'
rg -n -C 12 'onAction' packages/trigger-sdk/src --glob '*.{ts,tsx}'Repository: triggerdotdev/trigger.dev
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -eu
file=packages/trigger-sdk/src/v3/ai.ts
printf '%s\n' '-- ActionEvent type --'
cat -n "$file" | sed -n '5495,5565p'
printf '\n%s\n' '-- action callback, override application, and response handling --'
cat -n "$file" | sed -n '7880,8075p'
printf '\n%s\n' '-- message conversion and public chat helpers --'
rg -n -C 5 'toModelMessages|modelMessages|toStreamTextOptions|messages:' "$file" | head -n 260Repository: triggerdotdev/trigger.dev
Length of output: 26219
🏁 Script executed:
#!/bin/bash
set -eu
file=packages/trigger-sdk/src/v3/ai.ts
printf '%s\n' '-- action result handling and snapshot write --'
cat -n "$file" | sed -n '8080,8235p;6855,6935p'
printf '\n%s\n' '-- stream result capture helpers --'
rg -n -C 10 'actionStreamResult|StreamTextResult|pipeAndCapture|writeSnapshotOutsideTurn|accumulatedMessages|turnNewModelMessages' "$file" | head -n 360Repository: triggerdotdev/trigger.dev
Length of output: 31187
Use post-mutation model messages for regeneration.
The runtime passes messages to onAction before applying chat.history.slice(). The examples therefore send the removed assistant message as model context. Rebuild the model messages from the post-mutation history in all three examples.
| export const simpleChat = chat.agent({ | ||
| id: "simple-chat", | ||
| run: async ({ messages, signal }) => { | ||
| run: async ({ messages, signal, streamText }) => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pass the managed streamText into runAgentLoop.
streamText is scoped to the run callback, but runAgentLoop calls it without receiving it. The example no longer has a direct import, so copied code fails with an out-of-scope reference.
Proposed fix
- await runAgentLoop(messages);
+ await runAgentLoop(messages, streamText);
-async function runAgentLoop(messages: ModelMessage[]) {
+async function runAgentLoop(messages: ModelMessage[], streamText: ChatStreamText) {| The managed `streamText` carries the stored prompt's `system`, `model` (resolved through the agent's `registry`), sampling config, and `experimental_telemetry`. Options you pass at the call site win, apart from `system`, which throws when the prompt already set one. | ||
|
|
||
| | Form | Use when | | ||
| |---|---| | ||
| | `chat.toStreamTextOptions()` | Default. Wires up `prepareStep` (compaction, steering, background injection), the stored prompt's `system` / `model` / `config`, and telemetry metadata. | | ||
| | `chat.toStreamTextOptions({ registry })` | You're using [Prompts](/ai/prompts) with a provider-prefixed model string (e.g. `"anthropic:claude-sonnet-4-5"`). The registry resolves the prefix to a real model instance via `createProviderRegistry({ anthropic, openai, ... })`. | | ||
| | `chat.toStreamTextOptions({ tools })` | You want HITL tool approvals — pass the same `tools` object you give to `streamText`. The SDK then knows which tool calls need to pause on `needsApproval: true`. | | ||
| | `chat.toStreamTextOptions({ registry, tools })` | Both of the above. | | ||
| `chat.toStreamTextOptions()` remains available for the same job, and is the only option in a [custom agent](#custom-agents) or a `chat.headStart` route, where there is no `run` argument to take it from. Pass `{ registry }` when a prompt names a provider-prefixed model, and `{ tools }` when you want HITL tool approvals, so the SDK knows which calls pause on `needsApproval`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align all authoring guidance with managed streamText. These sections still describe chat.toStreamTextOptions() as the primary path or incorrectly say that Head Start has no run argument.
docs/ai-chat/backend.mdx#L273-L275: state that Head Start receives managedstreamText, and document the specialtoolsandprepareStepmerge behavior.docs/ai-chat/tools.mdx#L48-L52: make the run-provided managed callback the canonical pattern forchat.agent().docs/ai-chat/tools.mdx#L161-L163: describechat.toStreamTextOptions()as the manual/custom-agent alternative.packages/cli-v3/skills/trigger-authoring-chat-agent/SKILL.md#L37-L38: remove the claim that Head Start lacks a run payload.packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md#L37-L38: remove the same claim.packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md#L263-L264: keep the helper guidance only for routes without managedstreamText.
📍 Affects 4 files
docs/ai-chat/backend.mdx#L273-L275(this comment)docs/ai-chat/tools.mdx#L48-L52docs/ai-chat/tools.mdx#L161-L163packages/cli-v3/skills/trigger-authoring-chat-agent/SKILL.md#L37-L38packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md#L37-L38packages/trigger-sdk/skills/trigger-authoring-chat-agent/SKILL.md#L263-L264
|
|
||
| <Warning> | ||
| Omitting `...chat.toStreamTextOptions()` throws no error — compaction, steering, and background injection just silently never run. Spread it first so any explicit override you write after it takes precedence. | ||
| Importing `streamText` from `ai` instead throws no error: compaction, steering and background injection never run. Spreading `chat.toStreamTextOptions()` into the imported one is the equivalent, and is what a `chat.headStart` route has to do, since it has no `run` argument. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the managed Head Start callback contract.
chat.headStart and chat.startHeadStart pass a bound streamText function to run. It owns messages, stopWhen, and abortSignal. These sections still say that Head Start has no run argument or require manual helper spreading.
docs/ai-chat/migrating-from-a-route-handler.mdx#L169-L169: Remove the claim that a Head Start route has norunargument.docs/ai-chat/fast-starts.mdx#L254-L254: Make callback-providedstreamTextthe documented default.docs/ai-chat/fast-starts.mdx#L637-L652: AddstreamTextto the callback arguments and describe its owned options.
📍 Affects 2 files
docs/ai-chat/migrating-from-a-route-handler.mdx#L169-L169(this comment)docs/ai-chat/fast-starts.mdx#L254-L254docs/ai-chat/fast-starts.mdx#L637-L652
| if (callerSystem !== undefined && managedSystem) { | ||
| throw new Error( | ||
| "chat.agent: `system` is already set " + | ||
| (promptSystem ? "by chat.prompt.set()" : "on chat.agent({ system })") + | ||
| ", so it cannot also be passed to the `streamText` given to run(). Set it in one place, and add " + | ||
| "per-turn context with chat.inject({ role: 'system' }) rather than a second system value." | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The conflict check misses the agent + chat.prompt.set() combination.
The documentation for ChatAgentOptions.system (Line 6209) states the system prompt may be set on the agent, at the call site, or through chat.prompt.set(), but only in one of them. The runtime check enforces this only when callerSystem is set. When an agent sets both system and chat.prompt.set(), no error is raised. toStreamTextOptions then resolves promptText = prompt?.text || baseSystemText (Line 4854), so the prompt wins and chat.agent({ system }) is discarded without any signal.
This is the same silent-drop failure the seam is meant to prevent. Either throw for that pair as well, or change the documentation to state the precedence.
🛠️ Proposed fix to enforce the documented rule
const promptSystem = locals.get(chatPromptKey)?.text;
const managedSystem = promptSystem || agentSystem;
if (callerSystem !== undefined && managedSystem) {
throw new Error(
"chat.agent: `system` is already set " +
(promptSystem ? "by chat.prompt.set()" : "on chat.agent({ system })") +
", so it cannot also be passed to the `streamText` given to run(). Set it in one place, and add " +
"per-turn context with chat.inject({ role: 'system' }) rather than a second system value."
);
}
+ if (promptSystem && agentSystem) {
+ throw new Error(
+ "chat.agent: `system` is set both on chat.agent({ system }) and by chat.prompt.set(). " +
+ "Set it in one place, and add per-turn context with chat.inject({ role: 'system' })."
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (callerSystem !== undefined && managedSystem) { | |
| throw new Error( | |
| "chat.agent: `system` is already set " + | |
| (promptSystem ? "by chat.prompt.set()" : "on chat.agent({ system })") + | |
| ", so it cannot also be passed to the `streamText` given to run(). Set it in one place, and add " + | |
| "per-turn context with chat.inject({ role: 'system' }) rather than a second system value." | |
| ); | |
| } | |
| if (callerSystem !== undefined && managedSystem) { | |
| throw new Error( | |
| "chat.agent: `system` is already set " + | |
| (promptSystem ? "by chat.prompt.set()" : "on chat.agent({ system })") + | |
| ", so it cannot also be passed to the `streamText` given to run(). Set it in one place, and add " + | |
| "per-turn context with chat.inject({ role: 'system' }) rather than a second system value." | |
| ); | |
| } | |
| if (promptSystem && agentSystem) { | |
| throw new Error( | |
| "chat.agent: `system` is set both on chat.agent({ system }) and by chat.prompt.set(). " + | |
| "Set it in one place, and add per-turn context with chat.inject({ role: 'system' })." | |
| ); | |
| } |
| const baseSystem = options?.system; | ||
| const baseSystemText = | ||
| typeof baseSystem === "string" | ||
| ? baseSystem | ||
| : typeof baseSystem?.content === "string" | ||
| ? baseSystem.content | ||
| : ""; | ||
| const promptText = prompt?.text || baseSystemText; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Preserve providerOptions from structured system messages.
ChatAgentOptions.system and ToStreamTextOptionsOptions.system accept a complete SystemModelMessage, but toStreamTextOptions rebuilds it from only content. On the agent streamText path, this drops message-level cache settings when no higher-precedence option is set. Add baseSystem.providerOptions after the existing chat.prompt.set() provider options. Otherwise, an Anthropic cache breakpoint is lost and the system prompt is sent without caching.
| type HeadStartStreamTextFn = ( | ||
| options: Omit<Parameters<AiStreamTextFn>[0], "messages" | "prompt" | "stopWhen" | "abortSignal"> | ||
| ) => ReturnType<AiStreamTextFn>; | ||
|
|
||
| /** The keys `buildStreamTextOptions` owns. Overriding any of them breaks handover. */ | ||
| const HEAD_START_OWNED_OPTIONS = ["messages", "stopWhen", "abortSignal"] as const; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
HEAD_START_OWNED_OPTIONS omits "prompt", so the runtime guard and the type disagree.
HeadStartStreamTextFn omits four keys: messages, prompt, stopWhen, and abortSignal. HEAD_START_OWNED_OPTIONS lists only three. prompt is missing.
Because prompt is not filtered, it stays in rest and is spread onto the built options at Line 155, so aiStreamText receives both the managed messages and the caller's prompt. The AI SDK rejects that pair during prompt standardization, so the caller gets an opaque SDK error instead of the guiding message. Any caller that reaches the function from JavaScript, or through a cast, bypasses the documented behavior. The doc at Lines 176-178 promises that passing one of these options throws, and its own wording is inconsistent with the constant — "the four options" at Line 169 against "the three it owns" at Line 178.
Add "prompt" to the constant and align the comment.
🐛 Proposed fix
/** The keys `buildStreamTextOptions` owns. Overriding any of them breaks handover. */
-const HEAD_START_OWNED_OPTIONS = ["messages", "stopWhen", "abortSignal"] as const;
+const HEAD_START_OWNED_OPTIONS = ["messages", "prompt", "stopWhen", "abortSignal"] as const;Extend the error text so prompt is explained:
- "them: `messages` is the converted wire payload, `stopWhen: stepCountIs(1)` stops after " +
+ "them: `messages` is the converted wire payload (so `prompt` cannot be set either), " +
+ "`stopWhen: stepCountIs(1)` stops after " +And correct the count in the HeadStartRunArgs.streamText doc:
- * instead, and note that the three it owns are a type error, not just a
+ * instead, and note that the four it owns are a type error, not just a
* runtime one.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type HeadStartStreamTextFn = ( | |
| options: Omit<Parameters<AiStreamTextFn>[0], "messages" | "prompt" | "stopWhen" | "abortSignal"> | |
| ) => ReturnType<AiStreamTextFn>; | |
| /** The keys `buildStreamTextOptions` owns. Overriding any of them breaks handover. */ | |
| const HEAD_START_OWNED_OPTIONS = ["messages", "stopWhen", "abortSignal"] as const; | |
| type HeadStartStreamTextFn = ( | |
| options: Omit<Parameters<AiStreamTextFn>[0], "messages" | "prompt" | "stopWhen" | "abortSignal"> | |
| ) => ReturnType<AiStreamTextFn>; | |
| /** The keys `buildStreamTextOptions` owns. Overriding any of them breaks handover. */ | |
| const HEAD_START_OWNED_OPTIONS = ["messages", "prompt", "stopWhen", "abortSignal"] as const; |
| await new Promise((r) => setTimeout(r, 40)); | ||
| await harness.sendMessage({ id: "u2", role: "user", parts: [{ type: "text", text: "two" }] }); | ||
| await new Promise((r) => setTimeout(r, 40)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Poll for model.doStreamCalls before reading the model call
chat.headStart returns the response while stream consumption and handoverWhenDone continue in the background. The fixed 200 ms delay does not guarantee that model.doStreamCalls contains an entry, so .at(-1)! can fail on a loaded runner. Replace the delay with a bounded poll for model.doStreamCalls.length > 0. The existing waitFor helper is file-local, so define or share an equivalent helper in chat-server.test.ts.
| const agent = chat.agent({ | ||
| id: "bound-streamtext-tools", | ||
| tools: { agentTool }, | ||
| run: async ({ messages, signal, streamText }) => | ||
| streamText({ model, messages, abortSignal: signal, tools: { callerTool } }), | ||
| }); | ||
|
|
||
| const harness = mockChatAgent(agent, { chatId: "bound-streamtext-tools" }); | ||
|
|
||
| try { | ||
| await harness.sendMessage({ id: "u1", role: "user", parts: [{ type: "text", text: "hi" }] }); | ||
| await new Promise((r) => setTimeout(r, 40)); | ||
|
|
||
| const names = (model.doStreamCalls.at(-1)!.tools ?? []).map((t) => t.name).sort(); | ||
| expect(names).toContain("callerTool"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the skill-tool merge path. chat.agent({ tools }) exposes tools on ChatTaskRunPayload; buildManagedStreamTextOptions does not inject them automatically. The test passes only callerTool, so its agentTool declaration is not exercised. Because no skills are configured, a regression in toStreamTextOptions can also pass. Set a resolved skill with chat.skills.set(...), pass the payload tools when testing agentTool, and assert agentTool, callerTool, loadSkill, readFile, and bash. Rename the test to match.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 New tests bypass the no-mocking rule
The repository bans mocks, but this suite replaces @trigger.dev/core/v3 with vi.mock. Rewrite the coverage using the approved test infrastructure.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| const baseSystem = options?.system; | ||
| const baseSystemText = | ||
| typeof baseSystem === "string" | ||
| ? baseSystem | ||
| : typeof baseSystem?.content === "string" | ||
| ? baseSystem.content | ||
| : ""; |
There was a problem hiding this comment.
🟡 Structured system settings are discarded
When either system source is structured, toStreamTextOptions keeps only its text and discards its providerOptions. Block-level caching and provider settings silently stop applying.
Prompt for agents
Preserve providerOptions from a structured system value passed through either chat.agent({ system }) or the managed streamText call. In packages/trigger-sdk/src/v3/ai.ts, toStreamTextOptions currently extracts only baseSystem.content and reconstructs the system message from separately configured cache options. Incorporate the structured message's providerOptions into the documented precedence without losing explicit systemProviderOptions, cacheControl, or chat.prompt.set() provider options. Add coverage for both agent-level and call-site structured system messages.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Every
run()had to spreadchat.toStreamTextOptions(), and leaving it out dropped six things with no error: the managed prompt and its cache control, the registry-resolved model, the prompt's sampling config, telemetry, the skill tools, and theprepareStepthat delivers steering, compaction and injected context.Before:
After:
streamTextcomes fromrun's argument and shadows the one imported fromai, so the correct call is now the shorter one and the managed options cannot be lost by omission.chat.toStreamTextOptions()is unchanged and still supported, and is still the only option in a custom agent.What changes when your options collide with the managed ones
Spread order decides the outcome today, and losing is silent:
The managed
streamTextmerges instead.toolsare passed into the helper so skill tools survive, and aprepareStepyou pass runs after the managed one rather than replacing it. Everything else you name is left alone and wins, telemetry included.systemis the exception: it can be set onchat.agent({ system }), throughchat.prompt.set(), or at the call site, but only in one of them. Two at once throws and names the one that already owns it. No shape merges two system values across every supported AI SDK version, since v5 rejects an array of blocks and a structured block carries the provider options that make prompt caching work.onAction
A response produced from an action gets the same
streamText. Before, a regenerate answered with no system prompt and no skill tools, so the replacement answer came from a differently configured model than every other turn.Before:
After:
The only edit is the destructure. The two calls look the same and produce answers configured differently.
chat.headStart and chat.startHeadStart
buildStreamTextOptionssuppliesmessages,stopWhen: stepCountIs(1)andabortSignal. Step 1 belongs to the route handler and step 2 onward to the agent, so re-settingstopWhenafter a spread hands over a stream that has already run past step 1.Before:
After:
Passing
messages,stopWhenorabortSignalto thatstreamTextis a type error, with a runtime throw behind it for JavaScript callers. The old shape only warned in prose.Also in here
chat.agent()takessystem,registry,cacheControlandsystemProviderOptions, so a managed prompt's model and its cache breakpoint no longer have to be passed at the call site.ChatStreamTextis exported for typing a loop factored out ofrun.The signature is taken from the AI SDK's own declaration:
The peer range spans
aiv5, v6 and v7, whose options differ.typeofresolves to whichever version is installed, so generics and tool inference are the caller's own and a v8 option needs no change here.Verification
Typecheck and the full suite pass on both
ai@6.0.116andai@7.0.66. The option merge is a pure function so the merged object can be asserted directly, which is howexperimental_telemetrybeing dropped was caught: moststreamTextoptions never reach the provider, so a test that observes the model cannot see them.Run end to end against a deployed agent with every
runrewritten to the new form and no spread anywhere: steering, undo across a cold boot, and regenerate all still pass, a caller's ownprepareStepruns while managed steering still fires inside the turn, and consecutive injections arrive one per turn. The handover-owned options are pinned by@ts-expect-errorassertions in a typechecked test rather than only by the runtime throw.