fix: skip memory persistence for delegated sub-agent calls#1154
fix: skip memory persistence for delegated sub-agent calls#1154NeuZhou wants to merge 1 commit intoVoltAgent:mainfrom
Conversation
…#1026) When a parent agent delegates a task to a sub-agent via delegate_task, the sub-agent was persisting its messages under the parent's conversationId, causing duplicate messages in the parent's history. This fix checks for parentAgentId on the OperationContext — which is already set during delegation — and skips memory persistence for sub-agent calls. Sub-agents acting as stateless task executors should not pollute the parent conversation's memory. Fixes VoltAgent#1026
|
📝 WalkthroughWalkthroughAdds a guard condition in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/agent/agent.ts`:
- Around line 3797-3802: The current guard checking oc.parentAgentId only
affects shouldPersistMemoryForContext paths, but delegated contexts still
persist input because prepareMessages calls prepareConversationContext(..., {
persistInput: shouldPersistMemory }) and queueSaveInput(...) uses
shouldPersistMemory which ignores parentAgentId; update those call sites
(prepareMessages, prepareConversationContext invocation, and queueSaveInput
usage) to also consult this.shouldPersistMemoryForContext(oc) — e.g., compute a
boolean like shouldPersist = this.shouldPersistMemoryForContext(oc) &&
shouldPersistMemory and pass that into prepareConversationContext and gate calls
to queueSaveInput to prevent persisting input when
shouldPersistMemoryForContext(oc) is false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b4a4334-7b75-46df-a49d-32f3cf15d074
📒 Files selected for processing (1)
packages/core/src/agent/agent.ts
| // Skip memory persistence for delegated sub-agent calls to prevent | ||
| // polluting the parent conversation's memory with duplicate messages. | ||
| // See: https://github.com/VoltAgent/voltagent/issues/1026 | ||
| if (oc.parentAgentId) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Delegated calls can still persist input despite this guard
Good direction, but this guard only covers paths that call shouldPersistMemoryForContext(). In delegated contexts (parentAgentId set), prepareMessages() still persists input via prepareConversationContext(..., { persistInput: shouldPersistMemory }) (Line 4490) and queueSaveInput(...) (Line 4533), where shouldPersistMemory only checks readOnly. That can still write delegated input into the parent conversation.
Consider gating those input-persistence paths with this.shouldPersistMemoryForContext(oc) as well.
Proposed fix
- const shouldPersistMemory = resolvedMemory.readOnly !== true;
+ const shouldPersistMemory =
+ resolvedMemory.readOnly !== true && this.shouldPersistMemoryForContext(oc);
...
- { persistInput: shouldPersistMemory },
+ { persistInput: shouldPersistMemory },
...
- if (isSemanticSearch && shouldPersistMemory && oc.userId && oc.conversationId) {
+ if (isSemanticSearch && shouldPersistMemory && oc.userId && oc.conversationId) {
try {
...
this.memoryManager.queueSaveInput(oc, inputForMemory, oc.userId, oc.conversationId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/agent/agent.ts` around lines 3797 - 3802, The current guard
checking oc.parentAgentId only affects shouldPersistMemoryForContext paths, but
delegated contexts still persist input because prepareMessages calls
prepareConversationContext(..., { persistInput: shouldPersistMemory }) and
queueSaveInput(...) uses shouldPersistMemory which ignores parentAgentId; update
those call sites (prepareMessages, prepareConversationContext invocation, and
queueSaveInput usage) to also consult this.shouldPersistMemoryForContext(oc) —
e.g., compute a boolean like shouldPersist =
this.shouldPersistMemoryForContext(oc) && shouldPersistMemory and pass that into
prepareConversationContext and gate calls to queueSaveInput to prevent
persisting input when shouldPersistMemoryForContext(oc) is false.
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/agent/agent.ts">
<violation number="1" location="packages/core/src/agent/agent.ts:3800">
P2: Delegated sub-agent input persistence is still enabled in `prepareMessages()` because it bypasses `shouldPersistMemoryForContext(oc)` and only checks read-only mode.</violation>
<violation number="2" location="packages/core/src/agent/agent.ts:3800">
P2: New persistence guard is incomplete: `toTool()` nested agent calls share parent conversation context without `parentAgentId`, so memory can still be persisted in delegated flows.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -3794,6 +3794,12 @@ export class Agent { | |||
| } | |||
There was a problem hiding this comment.
P2: Delegated sub-agent input persistence is still enabled in prepareMessages() because it bypasses shouldPersistMemoryForContext(oc) and only checks read-only mode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/agent/agent.ts, line 3800:
<comment>Delegated sub-agent input persistence is still enabled in `prepareMessages()` because it bypasses `shouldPersistMemoryForContext(oc)` and only checks read-only mode.</comment>
<file context>
@@ -3794,6 +3794,12 @@ export class Agent {
+ // Skip memory persistence for delegated sub-agent calls to prevent
+ // polluting the parent conversation's memory with duplicate messages.
+ // See: https://github.com/VoltAgent/voltagent/issues/1026
+ if (oc.parentAgentId) {
+ return false;
+ }
</file context>
| // Skip memory persistence for delegated sub-agent calls to prevent | ||
| // polluting the parent conversation's memory with duplicate messages. | ||
| // See: https://github.com/VoltAgent/voltagent/issues/1026 | ||
| if (oc.parentAgentId) { |
There was a problem hiding this comment.
P2: New persistence guard is incomplete: toTool() nested agent calls share parent conversation context without parentAgentId, so memory can still be persisted in delegated flows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/agent/agent.ts, line 3800:
<comment>New persistence guard is incomplete: `toTool()` nested agent calls share parent conversation context without `parentAgentId`, so memory can still be persisted in delegated flows.</comment>
<file context>
@@ -3794,6 +3794,12 @@ export class Agent {
+ // Skip memory persistence for delegated sub-agent calls to prevent
+ // polluting the parent conversation's memory with duplicate messages.
+ // See: https://github.com/VoltAgent/voltagent/issues/1026
+ if (oc.parentAgentId) {
+ return false;
+ }
</file context>
Problem
When a parent (supervisor) agent delegates a task to a sub-agent via \delegate_task, and the sub-agent has a \memory\ instance configured, the sub-agent persists its own user message and assistant response into the *same \conversationId* as the parent agent. On subsequent turns, the parent's \prepareMessages()\ loads these extra messages, causing the LLM to see duplicate/phantom messages.
Root Cause
In \SubAgentManager.handoffTask(), the parent's \conversationId\ is passed directly to the sub-agent via \�aseOptions. When the sub-agent has \memory, its persistence pipeline treats the delegated task as a new user message in the parent's conversation.
Fix
\shouldPersistMemoryForContext()\ now checks for \parentAgentId\ on the \OperationContext. This field is already set during delegation (line 3692/3742 in agent.ts), so no additional plumbing is needed. When a sub-agent is executing a delegated task, it skips memory persistence entirely.
This is the minimal, non-breaking fix — sub-agents acting as stateless task executors don't need to persist to the parent's conversation history. The existing \metadata.subAgentId\ / \metadata.subAgentName\ fields (from PR #786) remain available for any future read-side filtering needs.
Workaround (before this fix)
Remove \memory\ from sub-agents entirely.
Fixes #1026
Summary by cubic
Skip memory persistence for delegated sub‑agent calls to stop polluting the parent conversation and avoid duplicate/phantom messages.
Agent.shouldPersistMemoryForContextnow returns false whenOperationContext.parentAgentIdis set (fixes #1026).Written for commit 01ba714. Summary will update on new commits.
Summary by CodeRabbit