feat: fork-agent-redesign — 新增 AgentTool fork 参数与 spec 设计文档#405
Merged
claude-code-best merged 1 commit intomainfrom May 3, 2026
Merged
feat: fork-agent-redesign — 新增 AgentTool fork 参数与 spec 设计文档#405claude-code-best merged 1 commit intomainfrom
claude-code-best merged 1 commit intomainfrom
Conversation
为 AgentTool 引入 fork 布尔参数，支持子代理从父对话上下文中 fork 出独立分支， 继承完整历史、系统提示和模型配置。重构 inputSchema 条件逻辑以适配 fork 模式。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
📝 WalkthroughWalkthroughThe PR redesigns fork agent invocation from implicit (triggered by omitting ChangesFork Agent Explicit Parameter Redesign
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx (1)
709-715:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCarry the new fork summarization rule into the sync→background path.
shouldRunAsyncnow allows forks to start in the foreground, but the later continuation at Lines 1093-1103 still enablesonCacheSafeParamsonly whengetSdkAgentProgressSummariesEnabled()is true. A long-running fork that gets backgrounded after launch will therefore miss summaries unless the SDK flag is on.Suggested follow-up
- onCacheSafeParams: getSdkAgentProgressSummariesEnabled() + onCacheSafeParams: + isCoordinator || isForkPath || getSdkAgentProgressSummariesEnabled() ? (params: CacheSafeParams) => { const { stop } = startAgentSummarization( backgroundedTaskId,Also applies to: 892-893
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx` around lines 709 - 715, The continuation path that moves a run from sync→background must apply the same fork-summarization rule used when computing shouldRunAsync so forks that start foreground then background still get onCacheSafeParams; update the continuation logic that currently gates setting onCacheSafeParams behind getSdkAgentProgressSummariesEnabled() to also enable it when the fork condition is met (reuse the same predicate used for shouldRunAsync or check the fork/wasForked flag used for forking), and make the same change at the other occurrence (the block referenced around the other continuation). Ensure you reference and reuse the symbols shouldRunAsync, onCacheSafeParams, and getSdkAgentProgressSummariesEnabled (or the fork flag) so the backgrounding path and the initial decision share the same rule.
🧹 Nitpick comments (1)
packages/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.ts (1)
2-7: ⚡ Quick winUse Bun-native file APIs in this test.
This suite currently depends on Node compatibility layers just to read
prompt.ts. In this repo, tests should stay on Bun-native APIs as well.As per coding guidelines, "All imports must use Bun APIs for imports, builds, and execution — do not use Node.js APIs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.ts` around lines 2 - 7, Replace the Node-specific file/path/url usage with Bun-native APIs: remove dirname, fileURLToPath, join, and readFileSync plus the __dirname hack, and instead resolve the module path with new URL('../prompt.ts', import.meta.url) and read the file using Bun.file(...).text() (or Bun.file(...).textSync() for a synchronous read). Update the symbol usages where promptSource is set so it uses Bun.file(new URL('../prompt.ts', import.meta.url)).text()/textSync() and delete references to readFileSync, join, dirname, and fileURLToPath.
🤖 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/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.ts`:
- Around line 51-59: The test currently allows promptSource.match(...) to be
null so deletion of the run_in_background guidance would pass undetected; update
the test in the 'background task condition does not include !forkEnabled' case
to first assert that the regex match (bgCondition returned from
promptSource.match(/!isEnvTruthy.*isInProcessTeammate[\s\S]*?run_in_background/))
is not null (e.g., expect(bgCondition).toBeTruthy() or
expect(bgCondition).not.toBeNull()) before calling
expect(bgCondition[0]).not.toContain('!forkEnabled'); this ensures the presence
of the run_in_background block is enforced before checking it does not contain
'!forkEnabled'.
In `@packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx`:
- Around line 415-420: The /fork command still relies on the old implicit fork
behavior, so update the AgentTool invocation in the fork command handler to
include fork: true in the options/payload so the routing logic (isForkPath
checks fork === true) takes the fork path; specifically, add fork: true
alongside or within the object that currently passes subagent_type/effectiveType
to AgentTool (the call site in the fork command), ensuring the payload contains
fork: true so AgentTool's isForkPath and effectiveType logic selects the fork
route.
In `@spec/feature_20260502_F001_fork-agent-redesign/spec-human-verify.md`:
- Around line 77-81: The current acceptance check uses grep -c 'omit' which
false-positives because the file AgentTool/prompt.ts legitimately contains other
"omit" text; update the check to search for the specific removed guidance
instead (e.g., use a regex like "omit.*subagent_type" or the exact phrase "omit
subagent_type") so the command targets the old implicit-fork guidance in
packages/builtin-tools/src/tools/AgentTool/prompt.ts rather than any occurrence
of "omit".
In `@spec/feature_20260502_F001_fork-agent-redesign/spec-plan.md`:
- Around line 257-259: The grep in the spec is too broad and will match other
legitimate uses of "omit", so update the verification to search for the specific
removed guidance instead of "omit" alone: change the check that references
prompt.ts to look for the regex "omit.*subagent_type" or the exact deleted
phrase (e.g., the original implicit-fork guidance string) to ensure only the
unwanted guidance is detected; locate the entry that currently runs grep -n
"omit" against prompt.ts and replace it with a targeted search for the precise
text.
---
Outside diff comments:
In `@packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx`:
- Around line 709-715: The continuation path that moves a run from
sync→background must apply the same fork-summarization rule used when computing
shouldRunAsync so forks that start foreground then background still get
onCacheSafeParams; update the continuation logic that currently gates setting
onCacheSafeParams behind getSdkAgentProgressSummariesEnabled() to also enable it
when the fork condition is met (reuse the same predicate used for shouldRunAsync
or check the fork/wasForked flag used for forking), and make the same change at
the other occurrence (the block referenced around the other continuation).
Ensure you reference and reuse the symbols shouldRunAsync, onCacheSafeParams,
and getSdkAgentProgressSummariesEnabled (or the fork flag) so the backgrounding
path and the initial decision share the same rule.
---
Nitpick comments:
In `@packages/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.ts`:
- Around line 2-7: Replace the Node-specific file/path/url usage with Bun-native
APIs: remove dirname, fileURLToPath, join, and readFileSync plus the __dirname
hack, and instead resolve the module path with new URL('../prompt.ts',
import.meta.url) and read the file using Bun.file(...).text() (or
Bun.file(...).textSync() for a synchronous read). Update the symbol usages where
promptSource is set so it uses Bun.file(new URL('../prompt.ts',
import.meta.url)).text()/textSync() and delete references to readFileSync, join,
dirname, and fileURLToPath.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e68594e-25d5-446e-bfe9-353e19c98bac
📒 Files selected for processing (7)
packages/builtin-tools/src/tools/AgentTool/AgentTool.tsxpackages/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.tspackages/builtin-tools/src/tools/AgentTool/prompt.tsscripts/defines.tsspec/feature_20260502_F001_fork-agent-redesign/spec-design.mdspec/feature_20260502_F001_fork-agent-redesign/spec-human-verify.mdspec/feature_20260502_F001_fork-agent-redesign/spec-plan.md
Comment on lines
+51
to
+59
| test('background task condition does not include !forkEnabled', () => { | ||
| // The condition for showing background task instructions should not exclude fork | ||
| const bgCondition = promptSource.match( | ||
| /!isEnvTruthy.*isInProcessTeammate[\s\S]*?run_in_background/, | ||
| ) | ||
| if (bgCondition) { | ||
| expect(bgCondition[0]).not.toContain('!forkEnabled') | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
Fail if the background-note block disappears.
If promptSource.match(...) returns null, this test still passes, so deleting the run_in_background guidance entirely would not be caught. Assert the match exists before checking that it does not contain !forkEnabled.
Suggested fix
test('background task condition does not include !forkEnabled', () => {
// The condition for showing background task instructions should not exclude fork
const bgCondition = promptSource.match(
/!isEnvTruthy.*isInProcessTeammate[\s\S]*?run_in_background/,
)
- if (bgCondition) {
- expect(bgCondition[0]).not.toContain('!forkEnabled')
- }
+ expect(bgCondition).not.toBeNull()
+ expect(bgCondition![0]).not.toContain('!forkEnabled')
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/builtin-tools/src/tools/AgentTool/__tests__/prompt.test.ts` around
lines 51 - 59, The test currently allows promptSource.match(...) to be null so
deletion of the run_in_background guidance would pass undetected; update the
test in the 'background task condition does not include !forkEnabled' case to
first assert that the regex match (bgCondition returned from
promptSource.match(/!isEnvTruthy.*isInProcessTeammate[\s\S]*?run_in_background/))
is not null (e.g., expect(bgCondition).toBeTruthy() or
expect(bgCondition).not.toBeNull()) before calling
expect(bgCondition[0]).not.toContain('!forkEnabled'); this ensures the presence
of the run_in_background block is enforced before checking it does not contain
'!forkEnabled'.
Comment on lines
+415
to
420
| // Fork routing: explicit `fork: true` parameter triggers the fork path | ||
| // (inherits parent context and model). Requires FORK_SUBAGENT flag. | ||
| // subagent_type is ignored when fork takes effect. | ||
| const isForkPath = fork === true && isForkSubagentEnabled(); | ||
| const effectiveType = subagent_type ?? GENERAL_PURPOSE_AGENT.agentType; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the routing logic in AgentTool.tsx around lines 415-420
cat -n packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx | sed -n '405,430p'Repository: claude-code-best/claude-code
Length of output: 1544
🏁 Script executed:
# Find and examine the fork.tsx caller
fd -t f fork.tsx | head -5Repository: claude-code-best/claude-code
Length of output: 99
🏁 Script executed:
# Also check for any inputSchema definitions to understand the implicit behavior
rg -n "inputSchema|subagent_type" packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx -A 2 -B 2 | head -100Repository: claude-code-best/claude-code
Length of output: 2893
🏁 Script executed:
# Examine the AgentTool.call() invocation in fork.tsx
cat -n src/commands/fork/fork.tsx | sed -n '1,100p'Repository: claude-code-best/claude-code
Length of output: 3289
🏁 Script executed:
# Look for the actual AgentTool.call() invocation
rg -n "AgentTool|call\(" src/commands/fork/fork.tsx -A 5 -B 5 | head -80Repository: claude-code-best/claude-code
Length of output: 1801
Add fork: true to the fork command's AgentTool invocation.
The routing logic in AgentTool.tsx changed from implicit (omit subagent_type to trigger fork) to explicit (fork: true parameter). The /fork command in src/commands/fork/fork.tsx (lines 42–48) has not been updated and still relies on the old implicit behavior. Without fork: true, the fork command will spawn a general-purpose agent instead of a fork, silently producing incorrect behavior.
Required fix
const input = {
prompt: directive,
+ fork: true,
run_in_background: true,
description: `Fork: ${directive.slice(0, 30)}${directive.length > 30 ? '...' : ''}`,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/builtin-tools/src/tools/AgentTool/AgentTool.tsx` around lines 415 -
420, The /fork command still relies on the old implicit fork behavior, so update
the AgentTool invocation in the fork command handler to include fork: true in
the options/payload so the routing logic (isForkPath checks fork === true) takes
the fork path; specifically, add fork: true alongside or within the object that
currently passes subagent_type/effectiveType to AgentTool (the call site in the
fork command), ensuring the payload contains fork: true so AgentTool's
isForkPath and effectiveType logic selects the fork route.
Comment on lines
+77
to
+81
| #### - [x] 3.1 不再包含 "omit subagent_type" 引导文本 | ||
| - **来源:** spec-plan.md Task 2 / spec-design.md §prompt 调整 | ||
| - **目的:** 确认隐式 fork 触发引导已移除 | ||
| - **操作步骤:** | ||
| 1. [A] `grep -c 'omit' packages/builtin-tools/src/tools/AgentTool/prompt.ts` → 期望精确: `0` |
Contributor
There was a problem hiding this comment.
The acceptance command will false-positive on If omitted.
packages/builtin-tools/src/tools/AgentTool/prompt.ts now legitimately contains If omitted, the general-purpose agent is used., so grep -c 'omit' no longer proves the old implicit-fork guidance is gone. Narrow this to omit.*subagent_type or the exact removed phrase.
Suggested edit
#### - [x] 3.1 不再包含 "omit subagent_type" 引导文本
- **来源:** spec-plan.md Task 2 / spec-design.md §prompt 调整
- **目的:** 确认隐式 fork 触发引导已移除
- **操作步骤:**
- 1. [A] `grep -c 'omit' packages/builtin-tools/src/tools/AgentTool/prompt.ts` → 期望精确: `0`
+ 1. [A] `grep -c 'omit.*subagent_type' packages/builtin-tools/src/tools/AgentTool/prompt.ts` → 期望精确: `0`📝 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.
Suggested change
| #### - [x] 3.1 不再包含 "omit subagent_type" 引导文本 | |
| - **来源:** spec-plan.md Task 2 / spec-design.md §prompt 调整 | |
| - **目的:** 确认隐式 fork 触发引导已移除 | |
| - **操作步骤:** | |
| 1. [A] `grep -c 'omit' packages/builtin-tools/src/tools/AgentTool/prompt.ts` → 期望精确: `0` | |
| #### - [x] 3.1 不再包含 "omit subagent_type" 引导文本 | |
| - **来源:** spec-plan.md Task 2 / spec-design.md §prompt 调整 | |
| - **目的:** 确认隐式 fork 触发引导已移除 | |
| - **操作步骤:** | |
| 1. [A] `grep -c 'omit.*subagent_type' packages/builtin-tools/src/tools/AgentTool/prompt.ts` → 期望精确: `0` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/feature_20260502_F001_fork-agent-redesign/spec-human-verify.md` around
lines 77 - 81, The current acceptance check uses grep -c 'omit' which
false-positives because the file AgentTool/prompt.ts legitimately contains other
"omit" text; update the check to search for the specific removed guidance
instead (e.g., use a regex like "omit.*subagent_type" or the exact phrase "omit
subagent_type") so the command targets the old implicit-fork guidance in
packages/builtin-tools/src/tools/AgentTool/prompt.ts rather than any occurrence
of "omit".
Comment on lines
+257
to
+259
| - [x] 验证 prompt 中不再包含 "omit `subagent_type`" 引导文本 | ||
| - `grep -n "omit" packages/builtin-tools/src/tools/AgentTool/prompt.ts` | ||
| - 预期: 无输出 |
Contributor
There was a problem hiding this comment.
This verification grep is too broad.
prompt.ts now contains If omitted, the general-purpose agent is used., so grep -n "omit" will still match even when the removed implicit-fork guidance is gone. Search for omit.*subagent_type or the exact deleted wording instead.
Suggested edit
-- [x] 验证 prompt 中不再包含 "omit `subagent_type`" 引导文本
- - `grep -n "omit" packages/builtin-tools/src/tools/AgentTool/prompt.ts`
+- [x] 验证 prompt 中不再包含 "omit `subagent_type`" 引导文本
+ - `grep -n 'omit.*subagent_type' packages/builtin-tools/src/tools/AgentTool/prompt.ts`
- 预期: 无输出📝 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.
Suggested change
| - [x] 验证 prompt 中不再包含 "omit `subagent_type`" 引导文本 | |
| - `grep -n "omit" packages/builtin-tools/src/tools/AgentTool/prompt.ts` | |
| - 预期: 无输出 | |
| - [x] 验证 prompt 中不再包含 "omit `subagent_type`" 引导文本 | |
| - `grep -n 'omit.*subagent_type' packages/builtin-tools/src/tools/AgentTool/prompt.ts` | |
| - 预期: 无输出 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/feature_20260502_F001_fork-agent-redesign/spec-plan.md` around lines 257
- 259, The grep in the spec is too broad and will match other legitimate uses of
"omit", so update the verification to search for the specific removed guidance
instead of "omit" alone: change the check that references prompt.ts to look for
the regex "omit.*subagent_type" or the exact deleted phrase (e.g., the original
implicit-fork guidance string) to ensure only the unwanted guidance is detected;
locate the entry that currently runs grep -n "omit" against prompt.ts and
replace it with a targeted search for the precise text.
6 tasks
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
2 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.
为 AgentTool 引入 fork 布尔参数，支持子代理从父对话上下文中 fork 出独立分支， 继承完整历史、系统提示和模型配置。重构 inputSchema 条件逻辑以适配 fork 模式。
Summary by CodeRabbit
Release Notes
New Features
forkparameter to the agent tool for explicit control over fork routing behavior.Improvements
fork: trueinstead of implicit triggering; fork mode now depends on feature flag enablement and parameter state.Tests
Documentation