feat(core): implement fork subagent for context sharing#2936
Merged
Conversation
Contributor
📋 Review SummaryThis PR implements the Fork Subagent (P0) feature along with a 🔍 General Feedback
🎯 Specific Feedback🔴 Critical
🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- Make subagent_type optional in AgentTool - Add forkSubagent.ts to build identical tool result prefixes - Run fork processes in the background to preserve UX
…ubagent_type - Skip pathReader and edit tool permission tests when running as root - Fix agent.test.ts to correctly mock execute call with extraHistory - Remove unused imports in forkSubagent.ts
wenshao
force-pushed
the
feat/fork-subagent
branch
2 times, most recently
from
51728dc to
db79f3b
Compare
Collaborator
Author
Response to Review Feedback
🔴 Critical — Both Resolved1. Fork detection after FORK_AGENT assignment (agent.ts:520-530) 2. Background execution error boundary (agent.ts:588-600) 🟡 High — All Resolved3. "Your system prompt says 'default to forking.' IGNORE IT" (forkSubagent.ts:78-95) 4. Complex extraHistory branching (agent.ts:500-515) 5. prePlanMode tracking (config.ts:1662-1670) 🟢 Medium — Addressed or N/A6. Validation error messages (agent.ts:242-254) — The "Available subagents" message only shows when 7-8. savePlan / planCommand / exitPlanMode — No longer in this PR. 🔵 Low9. buildChildMessage template string — Kept as-is for readability. Extracting rules to a separate constant adds indirection without benefit for a single-use template. 10. Missing fork-specific tests — Acknowledged. The current tests cover the non-fork subagent path. Fork-specific tests (recursive detection, extraHistory construction, background execution) should be added in a follow-up. Additional Fixes Since ReviewBeyond the review feedback, the following bugs were found and fixed:
|
Bug fixes:
- Fix AgentParams.subagent_type type: string -> string? (match schema)
- Fix undefined agentType passed to hook system (fallback to subagentConfig.name)
- Fix hook continuation missing extraHistory parameter
- Fix functionResponse missing id field (match coreToolScheduler pattern)
- Fix consecutive user messages in Gemini API (ensure history ends with model)
- Fix duplicate task_prompt when directive already in extraHistory
- Fix FORK_AGENT.systemPrompt empty string causing createChat to throw
- Fix redundant dynamic import of forkSubagent.js (merge into single import)
- Fix non-fork agent returning empty string on execution failure
- Fix misleading fork child rule referencing non-existent system prompt config
- Fix functionResponse.response key from {result:} to {output:} for consistency
CacheSafeParams integration:
- Retrieve parent's generationConfig via getCacheSafeParams() for cache sharing
- Add generationConfigOverride to CreateChatOptions and AgentHeadless.execute()
- Add toolsOverride to AgentHeadless.execute() for parent tool declarations
- Fork API requests now share byte-identical prefix with parent (DashScope cache hits)
- Graceful degradation when CacheSafeParams unavailable (first turn)
Docs:
- Add Fork Subagent section to sub-agents.md user manual
- Add fork-subagent-design.md design document
wenshao
force-pushed
the
feat/fork-subagent
branch
from
db79f3b to
b6ba5d2
Compare
Collaborator
Author
Update: PR Reworked (rebase + additional fixes)What changed since the last response1. Rebased onto
2. CacheSafeParams integration — The initial Gemini implementation was missing the core cache optimization from the improvement report. Added:
3. 12 bugs fixed total — See previous response for the full list. Key fixes:
4. Documentation added:
Comparison with Claude Code referencePR description now includes a detailed comparison table. Summary of completion:
The gaps are documented in PR description under "Known Gaps" and in user docs under "Current Limitations". Test results
|
wenshao
commented
Apr 8, 2026
wenshao
force-pushed
the
feat/fork-subagent
branch
from
5fdbdf4 to
b9987b0
Compare
wenshao
commented
Apr 13, 2026
Collaborator
Author
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] The new forked subagent execution path still does not propagate ToolResult.modelOverride into subsequent sendMessageStream() rounds inside AgentCore. The main CLI flow handles model overrides, but fork/subagent loops continue using this.modelConfig.model || this.runtimeContext.getModel() || DEFAULT_QWEN_MODEL, so tool-driven model switches silently stop working in the new fork path.
Please persist the latest tool-provided modelOverride during tool-result processing and use it for later rounds in AgentCore, or move that propagation into shared runtime/chat logic so both main and subagent loops honor it consistently.
— gpt-5.4 via Qwen Code /review
Fork children were inheriting parent's cached tool declarations directly, bypassing prepareTools() filtering and gaining access to AgentTool and cron tools. Extract EXCLUDED_TOOLS_FOR_SUBAGENTS as a shared constant and apply it to forkToolsOverride.
wenshao
commented
Apr 13, 2026
Collaborator
Author
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.4 via Qwen Code /review
tanzhenxin
previously requested changes
Apr 14, 2026
tanzhenxin
reviewed
Apr 14, 2026
Collaborator
tanzhenxin
left a comment
tanzhenxin
left a comment
There was a problem hiding this comment.
Review (updated)
Architecture is sound — cache sharing via CacheSafeParams, the generationConfigOverride/toolsOverride plumbing, and defense-in-depth for recursive fork prevention are all well done. One bug to fix:
Startup context duplication on the no-cache fallback path
When getCacheSafeParams() returns null, forkGenerationConfig is undefined. In agent-core.ts:249, the guard options?.generationConfigOverride is falsy, so getInitialChatHistory() runs and adds the env bootstrap messages. But extraHistory (from getHistory(true)) already contains those same startup messages from the parent conversation. The fork gets duplicated env context.
Suggestion: make createChat() skip getInitialChatHistory() whenever extraHistory is provided, not just when generationConfigOverride is present.
(The known gaps — result feedback, write isolation, test coverage — are acknowledged in the PR description and deferred to follow-ups, which seems reasonable for a V1.)
Previously gated on generationConfigOverride, which meant the no-cache fallback path (CacheSafeParams unavailable) still ran getInitialChatHistory and duplicated env bootstrap messages already present in the parent's history. Gate on extraHistory instead so both fork paths skip env init.
Collaborator
Author
|
Fixed in d5eeb17. Changed |
The previous fix gated env-init skipping on the presence of extraHistory, but agent-interactive (arena) also passes extraHistory — its chatHistory is env-stripped by stripStartupContext() and DOES need fresh env init for the child's working directory. Skipping env there broke the interactive path. Replace the implicit gate with an explicit skipEnvHistory option that only fork sets (when extraHistory is present, since fork's history comes from getHistory(true) and already contains env).
Edge case: when the parent's rawHistory ends with a user message and has length 1, extraHistory becomes []. The previous gate (extraHistory !== undefined) would set skipEnvHistory: true, leaving the fork with neither env bootstrap nor parent history. Check length > 0 so empty arrays fall through to the normal env-init path.
The second subagent.execute() call in the SubagentStop retry loop was missing skipEnvHistory, so on retry the fork's env context would be duplicated — same bug as the initial tanzhenxin report, just on a less common code path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement Fork Subagent (P0) — omitting
subagent_typein the Agent tool triggers an implicit fork that inherits the parent's full conversation context, runs in the background, and shares the parent's prompt cache viaCacheSafeParams.Reference: improvement report item-2
Before / After
subagent_typeextraHistorygenerationConfigviaCacheSafeParams<fork-boilerplate>tag; returns errorComparison with Claude Code
subagent_type+ feature gate (FORK_SUBAGENT)subagent_type(always enabled)buildForkedMessages()clones assistant msg + placeholdertool_resultfunctionResponsewithidmatching)CacheSafeParamsthreads parent's rendered system prompt bytesgetCacheSafeParams()threads parent'sgenerationConfig(systemInstruction + tools)cache_controlmarkers, ~90% discount on cached tokensX-DashScope-CacheControl: enable, server-side prefix cachingrunAsyncAgentLifecycle()with progress tracking, task notification, summarizationvoid executeSubagent()fire-and-forget, display-only progress<task-notification>injects fork results into parent conversationpermissionMode: 'bubble'surfaces prompts to parent terminalPermissionMode.DefaultisCoordinatorMode()buildWorktreeNotice()for git worktree forksKnown Gaps (deferred to follow-up PRs)
<task-notification>to notify the parent when forks complete. Current implementation only updates display state — the parent LLM cannot observe fork results.PermissionMode.Defaultinstead of bubbling to the parent terminal.Key Design Decisions
generationConfigviagetCacheSafeParams()and passes it asgenerationConfigOverride+toolsOverride. SkipsgetInitialChatHistory()to avoid duplicating env context. Gracefully degrades when CacheSafeParams unavailable.[model(calls), user(responses+directive), model(ack)]inextraHistory. When no function calls, history ends with model and directive goes viatask_prompt.functionResponse.idmatching: Each function response includes theidfrom the corresponding function call, consistent withcoreToolScheduler.ts.createChat()throws on falsysystemPrompt. Used as fallback when CacheSafeParams is unavailable; overridden by parent's config when available.Files Changed
packages/core/src/agents/runtime/forkSubagent.tsbuildForkedMessages(),isInForkChild(),buildChildMessage()packages/core/src/tools/agent.tssubagent_typeoptional, fork path withextraHistoryconstruction, CacheSafeParams retrieval, background execution, error handlingpackages/core/src/agents/runtime/agent-headless.tsexecute()acceptsgenerationConfigOverride+toolsOverrideoptionspackages/core/src/agents/runtime/agent-core.tsCreateChatOptions.generationConfigOverride, skip env history for forkdocs/users/features/sub-agents.mddocs/design/fork-subagent/fork-subagent-design.mdTest Plan
agent.test.ts— 39 tests passagent-headless.test.ts— 23 tests passedit.test.ts/pathReader.test.ts— root execution test fixes pass摘要(中文)
实现 Fork 子代理(P0)—— Agent 工具省略
subagent_type参数时会触发隐式 fork:继承父会话的完整上下文、在后台运行,并通过CacheSafeParams共享父会话的 prompt 缓存。参考:改进报告 item-2
前后对比
subagent_typeextraHistory继承完整会话CacheSafeParams使用父会话完全相同的generationConfig<fork-boilerplate>标签检测;返回错误与 Claude Code 对比
subagent_type+ feature gate (FORK_SUBAGENT)subagent_type(始终启用)buildForkedMessages()克隆 assistant 消息 + 占位tool_resultfunctionResponse按id匹配)CacheSafeParams传递父会话渲染后的 system prompt 字节getCacheSafeParams()传递父会话的generationConfig(systemInstruction + tools)cache_control标记,缓存 token 约 90% 折扣X-DashScope-CacheControl: enable,服务端前缀缓存runAsyncAgentLifecycle(),含进度追踪、任务通知、摘要void executeSubagent()发后即忘,仅展示层进度<task-notification>将 fork 结果注入父会话permissionMode: 'bubble'把确认弹回父终端PermissionMode.DefaultisCoordinatorMode()时禁用 forkbuildWorktreeNotice()支持 git worktree fork已知差距(后续 PR 处理)
<task-notification>在 fork 完成时通知父会话。当前实现仅更新显示状态 —— 父 LLM 无法感知 fork 结果。PermissionMode.Default,不会冒泡到父终端。关键设计决策
getCacheSafeParams()拿到父会话缓存的generationConfig,作为generationConfigOverride+toolsOverride传入。跳过getInitialChatHistory()避免重复环境上下文。CacheSafeParams 不可用时平滑降级。extraHistory中构造[model(calls), user(responses+directive), model(ack)];无 function call 时,历史以 model 结尾,指令通过task_prompt下发。functionResponse.id匹配:每条 function response 都带上对应 function call 的id,与coreToolScheduler.ts行为一致。createChat()在systemPrompt为假值时会抛错。该字段作为 CacheSafeParams 不可用时的兜底;有父 config 时会被覆盖。变更文件
packages/core/src/agents/runtime/forkSubagent.tsbuildForkedMessages()、isInForkChild()、buildChildMessage()packages/core/src/tools/agent.tssubagent_type改为可选,fork 路径构造extraHistory,读取 CacheSafeParams,后台执行,错误处理packages/core/src/agents/runtime/agent-headless.tsexecute()支持generationConfigOverride+toolsOverride选项packages/core/src/agents/runtime/agent-core.tsCreateChatOptions.generationConfigOverride,fork 时跳过环境历史docs/users/features/sub-agents.mddocs/design/fork-subagent/fork-subagent-design.md测试计划
agent.test.ts—— 39 个测试通过agent-headless.test.ts—— 23 个测试通过edit.test.ts/pathReader.test.ts—— 根目录执行相关测试修复通过