fix(harness): eliminate agent forking — use single shared agent across modes#18008
Conversation
…iders retain memory When Harness forks the config.agent into per-mode agents, it propagates memory and storage only to the forks. Signal providers connected to the base agent during construction remain pointed at the memoryless original instance, causing notification delivery to fail (e.g. GitHubSignals crash with 'computeStateSignal requires Mastra memory'). Now Harness.init() also propagates runtime services to config.agent so signal providers connected to it have full access to memory and storage. Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🦋 Changeset detectedLatest commit: 4bc2d9d The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughHarness refactors mode agent handling to reuse a single ChangesHarness mode agent sharing and runtime propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
PR triageLinked issue check skipped for excluded profile @devin-ai-integration[bot]. PR complexity score
Applied label: Changed test gateChanged tests failed against the base branch as expected. Label: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/harness/fork-clone-metadata.test.ts`:
- Around line 215-219: The test file
packages/core/src/harness/fork-clone-metadata.test.ts uses concrete model IDs
like 'gpt-4o' and 'openai/gpt-4o' instead of registered placeholder tokens.
Replace all literal model IDs in the baseAgent creation (where model provider is
set to 'openai' with name 'gpt-4o') and at the other affected location (line
238-239) with the appropriate placeholder tokens defined in
docs/src/plugins/remark-model-tokens/models.ts according to the repo's coding
guidelines for model names in tests and examples.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b7c534c-0be2-4cb3-8071-d691ac084bb7
📒 Files selected for processing (3)
.changeset/clear-buses-judge.mdpackages/core/src/harness/fork-clone-metadata.test.tspackages/core/src/harness/harness.ts
| const baseAgent = new Agent({ | ||
| name: 'base', | ||
| instructions: 'base agent', | ||
| model: { provider: 'openai', name: 'gpt-4o', toolChoice: 'auto' }, | ||
| signals: [signalProvider], |
There was a problem hiding this comment.
potential_issue: Replace newly added literal model IDs with registered placeholder tokens.
This test adds concrete model IDs (gpt-4o, openai/gpt-4o) instead of the tokenized placeholders required by repo policy.
As per coding guidelines, “When adding model names or IDs to examples, changesets, tests, or comments, use placeholder tokens from docs/src/plugins/remark-model-tokens/models.ts”.
Also applies to: 238-239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/harness/fork-clone-metadata.test.ts` around lines 215 -
219, The test file packages/core/src/harness/fork-clone-metadata.test.ts uses
concrete model IDs like 'gpt-4o' and 'openai/gpt-4o' instead of registered
placeholder tokens. Replace all literal model IDs in the baseAgent creation
(where model provider is set to 'openai' with name 'gpt-4o') and at the other
affected location (line 238-239) with the appropriate placeholder tokens defined
in docs/src/plugins/remark-model-tokens/models.ts according to the repo's coding
guidelines for model names in tests and examples.
Source: Coding guidelines
…s modes Instead of __fork()-ing config.agent per mode (which left signal providers connected to a memoryless base instance), the harness now reuses config.agent directly: - getAgentForMode() returns config.agent and applies mode instructions in place - Mode tools are delivered via buildToolsets() instead of __setTools() on forks - init() propagates runtime services (memory, storage, etc.) once to the shared agent — signal providers connected during construction retain full access - setBrowser() sets browser on the single agent instead of iterating forks The deprecated mode.agent path and the no-backing-agent construction path are unchanged. Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/harness/harness.ts`:
- Around line 570-576: The code currently propagates browser and runtime updates
only to `config.agent` when it exists, but fails to update explicit deprecated
`mode.agent` instances that may coexist in mixed configurations. In the
setBrowser method around lines 570-576 (the if-else block handling
config.agent), after updating the shared config.agent, also iterate through
this.config.modes and call setBrowser on any explicit mode.agent instances.
Apply the same logic to the init method at lines 639-650, ensuring that both
setBrowser and runtime initialization propagate to explicit mode.agent instances
to keep signal providers properly connected to backing agents in mixed mode
configurations.
- Around line 4283-4288: The code in the conditional block at lines 4283-4288
does not distinguish between replacement semantics (when mode.tools is used) and
augmentation semantics (when additionalTools is used), yet validateModes()
enforces this distinction. When currentMode.tools is defined instead of
currentMode.additionalTools, it indicates the mode intends to replace (restrict)
the agent's tools, but the current code just adds them to modeTools while
preserving the shared agent's own tools. Fix this by explicitly checking whether
currentMode.tools (replacement intent) or currentMode.additionalTools (augment
intent) is being used, and either implement a per-run tool mask to enforce the
replacement semantics, or reject/throw an error when mode.tools is used with a
shared agent to prevent unintended tool exposure until replacement enforcement
can be properly implemented.
- Around line 930-936: The applyModeInstructions method has a logic issue where
switching to a mode without instructions skips the __updateInstructions() call
due to the `if (instructions)` check, but still marks the mode as applied. This
leaves the shared agent with stale instructions from the previous mode. Remove
the conditional check around the __updateInstructions() call so that it is
invoked whenever a different mode is being applied (after the early return
check), ensuring that empty instructions properly clear any previous mode's
instructions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2d36820-6b0f-4d34-953d-278ca75ad47c
📒 Files selected for processing (2)
packages/core/src/harness/fork-clone-metadata.test.tspackages/core/src/harness/harness.ts
| if (this.config.agent) { | ||
| const currentMode = this.getCurrentMode(); | ||
| const modeTools = currentMode.tools ?? currentMode.additionalTools; | ||
| if (modeTools) { | ||
| result.modeTools = modeTools; | ||
| } |
There was a problem hiding this comment.
Preserve mode.tools replacement semantics with the shared agent.
validateModes() still treats tools as “replace” and additionalTools as “augment”, but this path maps both to modeTools while intentionally keeping the shared agent’s own tools. A mode using tools to restrict capabilities can still expose omitted base-agent tools; either honor replacement with a per-run tool mask or reject mode.tools for shared agents until replacement can be enforced.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/harness/harness.ts` around lines 4283 - 4288, The code in
the conditional block at lines 4283-4288 does not distinguish between
replacement semantics (when mode.tools is used) and augmentation semantics (when
additionalTools is used), yet validateModes() enforces this distinction. When
currentMode.tools is defined instead of currentMode.additionalTools, it
indicates the mode intends to replace (restrict) the agent's tools, but the
current code just adds them to modeTools while preserving the shared agent's own
tools. Fix this by explicitly checking whether currentMode.tools (replacement
intent) or currentMode.additionalTools (augment intent) is being used, and
either implement a per-run tool mask to enforce the replacement semantics, or
reject/throw an error when mode.tools is used with a shared agent to prevent
unintended tool exposure until replacement enforcement can be properly
implemented.
…gent on mode switch Replace __updateInstructions() mutation with a dynamic function that resolves the current mode's instructions at execution time. The function is installed once on config.agent and reads harness.currentModeId via closure, so no shared state is mutated on mode switch. Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
… agent Remove installDynamicModeInstructions and __updateInstructions usage. Mode instructions are now resolved in resolveCurrentModeInstructions() and passed via the instructions option in buildAgentMessageStreamOptions. The agent's own instructions are never touched by the harness. Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
…ehavior - agent instructions never mutated by mode switches - resolveCurrentModeInstructions returns correct per-mode instructions - mode tools included in toolsets for shared agent path - signal provider stays connected across mode switches - deprecated mode.agent path still works independently Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/harness/harness.ts (2)
565-574:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate runtime changes to explicit
mode.agentinstances too.Lines 567 and 638 still make a deprecated
mode.agentthe actual backing agent for that mode (viagetAgentForModeat lines 882-886) even whenconfig.agentexists, but these branches only updateconfig.agent. Mixed configs can leave those mode agents without browser/runtime services aftersetBrowser()orinit(), so signal providers attached to them can remain memoryless until a later lazygetCurrentAgent()call — the exact regression this PR fixes, but in a different scenario.Also applies to: 634-646
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/harness/harness.ts` around lines 565 - 574, The setBrowser() method only updates the shared config.agent when it exists, but deprecated mode.agent instances can still be backing agents for their modes (via getAgentForMode) and remain unsynchronized. When config.agent exists, also iterate through this.config.modes and call setBrowser() on the agent returned by getAgentForMode for each mode to ensure deprecated mode.agent instances receive the browser update. Apply the same fix to the other method affecting lines 634-646 (likely the init() method) to propagate runtime services consistently to both config.agent and any mode-specific agents in mixed configurations.
4282-4288:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve
mode.toolsreplacement semantics with the shared agent.
validateModes()at lines 86-91 still treatstoolsas "replace" andadditionalToolsas "augment", but line 4284 maps both tomodeToolswhile intentionally keeping the shared agent's own tools. A mode usingtoolsto restrict capabilities can still expose omitted base-agent tools; either honor replacement with a per-run tool mask or rejectmode.toolsfor shared agents until replacement can be enforced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/harness/harness.ts` around lines 4282 - 4288, The code at lines 4282-4288 in the if block checking this.config.agent needs to enforce the replacement semantics defined in validateModes(). Currently, when a mode specifies tools (intended to replace capabilities), the code still preserves the shared agent's own tools by mapping both currentMode.tools and currentMode.additionalTools equally to modeTools. To fix this, either implement a per-run tool mask that filters out tools not in the replacement set when currentMode.tools is explicitly defined (indicating replacement rather than augmentation), or add validation logic to reject configurations where mode.tools is used with a shared agent, preventing the capability restriction from being bypassed by the base agent's unrestricted tools.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/core/src/harness/harness.ts`:
- Around line 565-574: The setBrowser() method only updates the shared
config.agent when it exists, but deprecated mode.agent instances can still be
backing agents for their modes (via getAgentForMode) and remain unsynchronized.
When config.agent exists, also iterate through this.config.modes and call
setBrowser() on the agent returned by getAgentForMode for each mode to ensure
deprecated mode.agent instances receive the browser update. Apply the same fix
to the other method affecting lines 634-646 (likely the init() method) to
propagate runtime services consistently to both config.agent and any
mode-specific agents in mixed configurations.
- Around line 4282-4288: The code at lines 4282-4288 in the if block checking
this.config.agent needs to enforce the replacement semantics defined in
validateModes(). Currently, when a mode specifies tools (intended to replace
capabilities), the code still preserves the shared agent's own tools by mapping
both currentMode.tools and currentMode.additionalTools equally to modeTools. To
fix this, either implement a per-run tool mask that filters out tools not in the
replacement set when currentMode.tools is explicitly defined (indicating
replacement rather than augmentation), or add validation logic to reject
configurations where mode.tools is used with a shared agent, preventing the
capability restriction from being bypassed by the base agent's unrestricted
tools.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1122b605-8900-4315-bb9d-9734804788cb
📒 Files selected for processing (1)
packages/core/src/harness/harness.ts
Test Results — PR #18008Session: Devin Targeted fix verification (7 tests)
Regression suites
Escalations
|
Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
- setBrowser() and init() now propagate to all unique agents (shared config.agent + any deprecated mode.agent instances) via a Set, so mixed configs don't leave mode.agent instances without browser or runtime services. - buildToolsets() documents that mode.tools and mode.additionalTools are both treated as augmentation for the shared agent path; true 'replace' semantics would require per-run tool filtering in Agent which isn't supported yet. Co-Authored-By: Abhi Aiyer <abhi@mastra.ai>
|
Addressed CodeRabbit review comments in 4bc2d9d: Comment 9 (mixed Comment 10 (stale instructions on instructionless modes): Already resolved. The Comment 11 ( Comment 7 (model ID tokens in tests): Not applicable — the placeholder tokens in |
…ns (#18127) ## Description When using a shared backing agent (introduced in #18008), `buildAgentMessageStreamOptions` passed mode instructions via `options.instructions`, which the agent treats as a **full override** of its own instructions: ```ts // agent.ts:6267 const instructions = options.instructions || (await this.getInstructions({ requestContext })); ``` This meant the agent's own dynamic instructions (AGENTS.md, project context, mode-specific prompts from `getDynamicInstructions`) were completely lost — only the harness/mode static instructions survived. **Fix:** When mode instructions exist, resolve the agent's own `getInstructions()` first and combine both before passing at call time. When no mode instructions exist, don't pass `instructions` at all — the agent uses its own `getInstructions()` naturally. ```diff // buildAgentMessageStreamOptions -const modeInstructions = this.config.agent ? this.resolveCurrentModeInstructions() : undefined; -...(modeInstructions && { instructions: modeInstructions }), +if (this.config.agent) { + const modeInstructions = this.resolveCurrentModeInstructions(); + if (modeInstructions) { + const agentInstructions = await agent.getInstructions({ requestContext }); + callTimeInstructions = [agentStr, modeInstructions].filter(Boolean).join('\n'); + } +} +...(callTimeInstructions && { instructions: callTimeInstructions }), ``` ## Related issue(s) Regression from #18008 (eliminate agent forking). Reported in Slack #mastracode by Tyler Barnes. ## Type of change - [x] Bug fix (non-breaking change that fixes an issue) ## Checklist - [x] I have linked the related issue(s) in the description above - [x] I have made corresponding changes to the documentation (if applicable) - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have addressed all Coderabbit comments on this PR Link to Devin session: https://app.devin.ai/sessions/1eaa195dad2743feb71d55314e8a336f <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## ELI5 (Explain Like I'm 5) Imagine you have a robot that knows how to do many things on its own (its natural instructions), but also needs to follow special rules for different jobs (mode instructions). Previously, when giving it the special job rules, someone forgot to tell it about its natural instructions, so it only followed the job rules. This PR fixes that by making sure the robot remembers both its natural instructions AND the special job rules at the same time. ## What's Fixed This PR resolves a regression from `#18008` where shared backing agents were losing their dynamic instructions when using mode-specific instructions. The root cause was in `buildAgentMessageStreamOptions` in `harness.ts`, which was passing only mode instructions via `options.instructions`. Since the agent's instruction resolution logic uses an OR operator (`||`) rather than merging, the agent's own dynamic instructions—including AGENTS.md content, project context, and mode-specific prompts from `getDynamicInstructions`—were completely overridden. ## Changes Made **packages/core/src/harness/harness.ts:** - Added `AgentInstructions` to imports - Implemented a new private helper method `instructionsToString()` that normalizes `AgentInstructions` (which can be a string, string array, or system message object with `content` field) into a combined plain-string form - Modified `buildAgentMessageStreamOptions()` to: - When a backing agent exists and mode instructions are present: fetch the agent's current runtime instructions via `agent.getInstructions({ requestContext })`, convert them to a string, and combine them with the resolved mode instructions - When mode instructions do not exist: no longer pass the `instructions` option, allowing the agent to use its own `getInstructions()` naturally - Added `resolveCurrentModeInstructions()` private method to handle combining harness and mode instructions **.changeset/brave-mails-listen.md:** - New changeset documenting a patch version bump for `@mastra/core` - Documents the fix to `harness.buildAgentMessageStreamOptions` combining mode instructions with agent dynamic instructions rather than replacing them ## Impact The fix ensures that when harness uses mode-specific instructions, an agent's own dynamic instructions are preserved and merged together, rather than being overridden. This restores the intended behavior for shared backing agents that rely on their own instruction sources alongside mode-specific customizations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Abhi Aiyer <abhi@mastra.ai>
Description
Eliminates
__fork()from the shared-agent path in Harness. Instead of creating per-mode agent clones, the harness reusesconfig.agentdirectly and never mutates its instructions or tools.Root cause: PR #17892 introduced shared harness modes that fork the base agent per-mode. Signal providers connected to the base agent during construction skip re-connection on forks (
isConnectedguard), so they remain pointed at the original memoryless base agent. When GitHubSignals (or any signal provider) tries to deliver notifications, it crashes:Fix — no more forking, no agent mutation:
getAgentForMode(): returnsconfig.agentdirectly — no fork, no instructions/tools mutationbuildAgentMessageStreamOptions(): resolves mode instructions at call time viaresolveCurrentModeInstructions()and passes them as theinstructionsstream option — the agent's own instructions are never touchedbuildToolsets(): mode-specific tools (mode.tools/mode.additionalTools) are delivered through toolsets — the agent's own tools (including signal-provider tools) are never replacedinit(): propagates runtime services (memory, storage, etc.) once to the shared agent — signal providers connected during construction retain full accesssetBrowser(): sets browser on the single agent instead of iterating forksmode.agentand no-backing-agent construction paths are unchangedRelated issue(s)
Reported in Slack #mastracode by Tyler Barnes. Regression from #17892 (shared harness modes via
__fork()).Type of change
Checklist
Link to Devin session: https://app.devin.ai/sessions/a7472d903dcd4982bbed590b0bde4c1c
ELI5
Harness can run “agent work” in different modes (like different capabilities). A bug made Harness create separate agent copies per mode, but some notification components (signal providers) kept using the original agent that never got the required memory/services—so notifications crashed. This PR stops the per-mode agent cloning and instead reuses one shared agent, wiring mode-specific instructions/tools at runtime so the shared agent has memory and signals work again.
Summary
Fixes a regression where Harness signal providers (e.g.
GitHubSignals) could crash because they were still connected to a baseAgentthat lacked Mastra runtime services (especially memory) after PR#17892introduced per-mode agent forking/cloning.Key changes
config.agent) pathgetAgentForMode()returnsconfig.agentdirectly (no per-mode cloning).mode.agentpath remains supported via its existing per-mode caching behavior (agents can still be independent there).resolveCurrentModeInstructions()computes harness-level + current mode instructions on demand.buildAgentMessageStreamOptions()passes these as theinstructionsstream option, instead of updating the agent’s own instructions (avoids cross-mode instruction bleed).buildToolsets()supplies mode tool overrides at the toolset level (viaresult.modeTools) for the shared-agent path, rather than baking/replacing tools on the shared agent.init()propagates runtime services (memory/storage/workspace/browser/pubsub, etc.) once to the sharedconfig.agent.setBrowser()sets the browser on the single shared agent.Tests
Extended
packages/core/src/harness/fork-clone-metadata.test.tsto cover:config.agentinstance is reused across mode switches (no forking).SignalProvider-based regression coverage: a connected base/shared agent retains access to runtime services (memory) afterharness.init(), staying properly connected across mode switches.mode.agentpath still keeps agents independent per mode.Changeset
@mastra/coredocumenting that signal providers no longer lose memory access when Harness previously forked agents per mode.