fix: preserve system messages when prepareStep returns messages #10798
Conversation
Match AI SDK v5 behavior where system is handled separately from messages. When prepareStep returns messages without a system override, the original system messages are preserved instead of being lost.
🦋 Changeset detectedLatest commit: c931e62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
|
@zshannon is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR patches Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/core/src/loop/test-utils/options.ts (1)
1888-2037: New prepareStep/system tests accurately exercise the updated behaviorBoth tests correctly validate the intended semantics:
- When
prepareSteponly returnsmessages, the original system message fromMessageListis preserved and the user message is transformed before reaching the model.- When
prepareStepreturns asystemvalue, that system string cleanly overrides the original system prompt, while the rest of the conversation remains intact.The way
doStreamCallsis captured and asserted matches existing patterns in this file. One subtle behavior to be aware of (and which seems intentional with the current implementation) is that explicitly returning something falsy forsystem(e.g.nullor'') will clear all system messages for that step; if that’s desired, no further changes are needed.packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts (1)
559-585: System/message reconstruction logic matches the documented behaviorThe new
prepareStepResultbranch correctly:
- Rebuilds a fresh
MessageListwhen eithermessagesorsystemis provided.- Preserves existing system messages via
getAllSystemMessages()whensystemisundefined.- Uses
prepareStepResult.systemas an explicit override (and allows clearing system instructions when it’s falsy but defined).- Filters out
role === 'system'fromnewMessagesso that system is consistently sourced from the dedicated system path, while preserving user/assistant/tool messages.This aligns with the changeset description and the new tests, with no obvious correctness or type‑safety issues.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/fix-prepare-step-system.md(1 hunks)packages/core/src/loop/test-utils/options.ts(2 hunks)packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm typecheckto validate TypeScript types across all packages
Files:
packages/core/src/loop/test-utils/options.tspackages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm prettier:formatto format code andpnpm formatto run linting with auto-fix across all packages
Files:
packages/core/src/loop/test-utils/options.tspackages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: All packages must use TypeScript with strict type checking enabled
Use telemetry decorators for observability across components
Files:
packages/core/src/loop/test-utils/options.tspackages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
.changeset/*.md
⚙️ CodeRabbit configuration file
.changeset/*.md: Changeset files are really important for keeping track of changes in the project. They'll be used to generate release notes and inform users about updates.Review the changeset file according to these guidelines:
- The target audience are developers
- Write short, direct sentences that anyone can understand. Avoid commit messages, technical jargon, and acronyms. Use action-oriented verbs (Added, Fixed, Improved, Deprecated, Removed)
- Avoid generic phrases like "Update code", "Miscellaneous improvements", or "Bug fixes"
- Highlight outcomes! What does change for the end user? Do not focus on internal implementation details
- Add context like links to issues or PRs when relevant
- If the change is a breaking change or is adding a new feature, ensure that a code example is provided. This code example should show the public API usage (the before and after). Do not show code examples of internal implementation details.
- Keep the formatting easy-to-read and scannable. If necessary, use bullet points or multiple paragraphs (Use bold text as the heading for these sections, do not use markdown headings).
- For larger, more substantial changes, also answer the "Why" behind the changes
- Each changeset file contains a YAML frontmatter at the top. It will be one or more package names followed by a colon and the type of change (patch, minor, major). Do not modify this frontmatter. Check that the description inside the changeset file only applies to the packages listed in the frontmatter. Do not allow descriptions that mention changes to packages not listed in the frontmatter. In these cases, the user must create a separate changeset file for those packages.
Files:
.changeset/fix-prepare-step-system.md
🧠 Learnings (4)
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : Create test fixtures by copying examples DIRECTLY from migration guides in `docs/src/content/en/guides/migrations/upgrade-to-v1/` without hallucinating or inventing changes
Applied to files:
packages/core/src/loop/test-utils/options.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/**/*.test.ts : Include test cases for multiple occurrences of the transformation pattern, aliased imports, type imports, and mixed imports to verify the codemod works consistently
Applied to files:
packages/core/src/loop/test-utils/options.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : In output fixtures, ensure all NEGATIVE test cases remain EXACTLY IDENTICAL to their input fixture counterparts to verify the codemod only transforms intended patterns
Applied to files:
packages/core/src/loop/test-utils/options.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : In input fixtures, include both POSITIVE test cases (patterns that should transform) and NEGATIVE test cases (unrelated code with similar names/patterns that should NOT transform)
Applied to files:
packages/core/src/loop/test-utils/options.ts
🧬 Code graph analysis (1)
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts (1)
packages/core/src/agent/message-list/index.ts (1)
MessageList(107-3357)
🔇 Additional comments (2)
.changeset/fix-prepare-step-system.md (1)
5-9: Changeset description is clear and scoped correctlyThe summary is concise, outcome‑focused, and correctly scoped to
@mastra/core, describing the observable behavior change (system messages preserved and aligned with AI SDK v5). No changes needed.packages/core/src/loop/test-utils/options.ts (1)
14-14: MessageList import usage looks correct
MessageListis used in the new tests to construct custom message lists, matching how the production class is intended to be exercised. No issues here.
|
@abhiaiyer91 this one is also a very tiny fix 🙇♂️ ty for merging my other one |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts (1)
559-585: System‑message preservation / override logic looks correct; consider clarifying the override checkThe new block correctly:
- Uses
prepareStepResult.systemas an explicit override (including allowingnull/falsy to effectively clear system messages).- Preserves existing system messages when
messagesare provided without asystemoverride.- Ignores any
system‑role entries inprepareStepResult.messages, matching the “system handled separately” semantics.This aligns with the PR’s stated behavior and AI SDK v5 expectations, and the use of
messageList.getAllSystemMessages()ensures both untagged and tagged system messages are preserved.If you want slightly sharper semantics around “explicit override vs. absence”, you could optionally distinguish property presence from value using an
incheck, which also avoids repeating the!== undefinedtest:- if (prepareStepResult.messages || prepareStepResult.system !== undefined) { - const newMessages = prepareStepResult.messages ?? messageList.get.all.aiV5.model(); - const newMessageList = new MessageList(); - - if (prepareStepResult.system !== undefined) { - if (prepareStepResult.system) { - newMessageList.addSystem(prepareStepResult.system); - } - } else { - for (const sysMsg of messageList.getAllSystemMessages()) { - newMessageList.addSystem(sysMsg); - } - } + const hasSystemOverride = 'system' in prepareStepResult; + if (prepareStepResult.messages || hasSystemOverride) { + const newMessages = prepareStepResult.messages ?? messageList.get.all.aiV5.model(); + const newMessageList = new MessageList(); + + if (hasSystemOverride) { + if (prepareStepResult.system) { + newMessageList.addSystem(prepareStepResult.system); + } + } else { + for (const sysMsg of messageList.getAllSystemMessages()) { + newMessageList.addSystem(sysMsg); + } + } }Not strictly necessary, but it makes the “explicit override” intent a bit clearer for future readers. Overall, the fix itself looks solid.
Also applies to: 573-577
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/loop/test-utils/options.ts(2 hunks)packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/loop/test-utils/options.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm typecheckto validate TypeScript types across all packages
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm prettier:formatto format code andpnpm formatto run linting with auto-fix across all packages
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: All packages must use TypeScript with strict type checking enabled
Use telemetry decorators for observability across components
Files:
packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts
1 participant
Critical bug:
When
prepareStepreturns modified messages, the agent's system prompt (instructions) is silently dropped. This causes agents to ignore their instructions entirely.Match AI SDK v5 behavior where system is handled separately from messages. When prepareStep returns messages without a system override, the original system messages are preserved instead of being lost.
Description
When
prepareStepreturns modifiedmessages, Mastra was creating a newMessageListfrom those messages but losing the original system messages (instructions). This differed from AI SDK v5's behavior where the system prompt is handled separately and preserved by default.The fix:
prepareStepreturnsmessageswithout asystemoverridesystemfromprepareStepif explicitly provided (existing behavior)messagesarray since system is handled separatelyRelated Issue(s)
None
Type of Change
Checklist
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.