feat(subagent): add fork subagent feature gate and "Don't peek / Don't race" prompt discipline#4574
Merged
Conversation
Contributor
📋 Review SummaryThis PR adds two critical capabilities to the fork subagent system: a feature gate that restricts fork usage to interactive sessions only, and prompt discipline instructions ("Don't peek / Don't race") that are injected into the Agent tool description when fork is enabled. The implementation is well-tested with three new unit tests covering the gate behavior and prompt injection logic. Overall, this is a solid, focused change that addresses real failure modes observed in production. 🔍 General Feedback
🎯 Specific Feedback🟢 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. |
wenshao
reviewed
May 27, 2026
|
|
||
| const llmText = partToString(result.llmContent); | ||
| expect(llmText).toContain('not available in non-interactive mode'); | ||
| expect(AgentHeadless.create).not.toHaveBeenCalled(); |
Collaborator
There was a problem hiding this comment.
[Suggestion] The "rejects fork in non-interactive mode" test verifies llmContent and that AgentHeadless.create was not called, but does not verify the returnDisplay shape. Other tests in this file (e.g., the subagent-not-found test) verify returnDisplay.status, returnDisplay.subagentName, and returnDisplay.terminateReason. This gap means a regression in the UI-facing error display would go undetected.
Suggested change
| expect(AgentHeadless.create).not.toHaveBeenCalled(); | |
| const llmText = partToString(result.llmContent); | |
| expect(llmText).toContain('not available in non-interactive mode'); | |
| expect(AgentHeadless.create).not.toHaveBeenCalled(); | |
| const display = result.returnDisplay as { | |
| status: string; | |
| subagentName: string; | |
| terminateReason: string; | |
| }; | |
| expect(display.status).toBe('failed'); | |
| expect(display.subagentName).toBe('fork'); | |
| expect(display.terminateReason).toBe( | |
| 'Fork subagent is not available in non-interactive mode', | |
| ); |
— qwen3.7-max via Qwen Code /review
Collaborator
Maintainer Verification ReportPR: #4574 — feat(subagent): add fork subagent feature gate and "Don't peek / Don't race" prompt discipline Build & Type Check
Unit Tests
New Test Coverage
Code Review NotesFeature gate (
Prompt discipline (description injection):
Scope & Risk:
VerdictReady to merge. Clean, minimal feature gate with proper error messaging, well-structured prompt discipline, and comprehensive test coverage. No regressions in existing 89 agent tests. Verified by: wenshao |
tanzhenxin
requested changes
Jun 1, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
Thanks for this — the gate and the fork discipline text are solid groundwork, and I want to keep both. After digging into how fork is meant to work, though, I'd like to take it in a more conservative direction before it lands.
Right now, omitting subagent_type silently forks — there's no gate and no guidance around it, so fork is effectively on by default in its least-validated form. This PR improves that with the interactive gate and the discipline text, but I don't think we yet understand the real usage scenarios well enough to have fork on by default at all, and I'm not confident the model will reliably choose to fork at the right moments from guidance prose alone. Making fork genuinely useful seems to need more scaffolding than discipline text — steering it as the default path for offloadable work, and not pointing the model at other delegation options in the same breath — and that's a bigger, deliberate change I'd rather make once we've confirmed the model wields fork well.
Worth noting what the upstream actually ships: we inspected the tool instructions exposed in a current Claude Code session, and the fork path isn't enabled there by default. Calling the agent tool without a subagent type returns a normal general-purpose agent — fresh context, not a fork — and none of the fork-specific guidance ("when to fork", "don't peek", "don't race") is present. This was an interactive session, so it's not just the headless restriction kicking in — the feature simply isn't on by default. If the implementation that originated fork keeps it off for users by default, that's a strong signal we should too until we've validated it ourselves.
So the direction I'd like:
- Put fork behind an explicit opt-in that defaults to off. Ship the capability dark and turn it on later, once we've validated when it helps and that the model uses it sensibly.
- When it's off (the default), an Agent call with no
subagent_typeshould fall back to a normalgeneral-purposesubagent rather than forking or returning an error — that's the safe, predictable behavior. - Keep the interactive-only restriction for when fork is enabled. That part's correct — fork needs a terminal for permission prompts — so it stays as a second condition on top of the opt-in.
- Inject the "When to fork / Don't peek / Don't race" guidance only when the feature is enabled, so the default prompt is unchanged.
Net effect: your gate becomes the on/off switch and the discipline rides along with it, but everything stays dormant by default. That keeps the groundwork you've built while letting us roll fork out intentionally instead of having it on before we understand it.
Verdict
Request changes — not on the gate or the prompt text (both are good), but on the default: fork opt-in and off by default, falling back to a normal general-purpose subagent when off, with the discipline injected only when it's turned on.
wenshao
reviewed
Jun 1, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Fork resume path bypasses gate.
resolveResumeTarget in background-agent-resume.ts:898 unconditionally returns FORK_AGENT without checking isForkSubagentEnabled. A fork created in interactive mode can be resumed in non-interactive mode (e.g., after daemon restart), hitting the exact failure modes (no terminal UI, no permission bubble-up) the gate was designed to prevent.
Consider adding the same gate check in resolveResumeTarget or at the resume-dispatch site. When the gate is closed, fail the resume gracefully with an explanatory message.
— qwen-latest-series-invite-beta-v38 via Qwen Code /review
| // Non-interactive sessions (qwen -p, SDK headless, CI/CD) lack a | ||
| // terminal UI for fork progress display and permission bubble-up. | ||
| if (!isForkSubagentEnabled(this.config)) { | ||
| return { |
Collaborator
There was a problem hiding this comment.
[Suggestion] The fork gate rejection returns an error to the LLM but emits no log or telemetry. Other tool-level events in this file use debugLogger.warn/info/error. In headless/SDK mode, nobody watches the returnDisplay — the LLM may retry the fork multiple times, burning tokens, with zero log trail for debugging.
Suggested change
| return { | |
| if (!isForkSubagentEnabled(this.config)) { | |
| debugLogger.warn( | |
| '[Agent] Fork subagent blocked: session is not interactive', | |
| ); | |
| return { |
— qwen-latest-series-invite-beta-v38 via Qwen Code /review
Fork subagent now requires explicit opt-in via QWEN_CODE_ENABLE_FORK_SUBAGENT=1 environment variable (or programmatic forkSubagentEnabled: true). When disabled (the default), omitting subagent_type falls back to a general-purpose subagent with fresh context instead of forking. This aligns with how Claude Code ships fork: dark by default, opt-in for internal validation. The interactive-only restriction remains as a second condition when fork is enabled. Changes: - Add forkSubagentEnabled field + isForkSubagentEnabled() to Config - isForkSubagentEnabled gate now checks both feature flag AND interactive mode - Agent execute: fallback to general-purpose when fork is off - Agent prompt: conditional description based on fork availability - Fix missing blank line before Example usage in prompt text - Update tests for new fallback behavior
Collaborator
Author
Thanks for the thorough review — completely agree with the direction. All the changes you asked for are now in place: 1、Fork defaults to off — isForkSubagentEnabled() requires an explicit opt-in via QWEN_CODE_ENABLE_FORK_SUBAGENT=1 env var (or programmatic forkSubagentEnabled: true). Without it, fork is dormant — ship dark, turn on later. 2、Fallback to general-purpose when off — omitting subagent_type now spawns a general-purpose agent (fresh context) instead of silently forking. This is the safe, predictable behavior that matches the upstream default. 3、Interactive-only stays as a second condition — when fork is enabled, it still requires isInteractive(), so both conditions must be true. 4、Discipline text rides with the feature flag — the "When to fork / Don't peek / Don't race" guidance is only injected when isForkSubagentEnabled() returns true. The default prompt is unchanged. |
DennisYu07
commented
Jun 3, 2026
Collaborator
Author
Verification reportVerified the fork-subagent opt-in behavior in both default-off and opt-in-on modes. 1. Fork disabled by defaultCommand used: Prompt used in the interactive session: Expected output / behavior:
This confirms that with no opt-in flag, fork is dormant and omission of subagent_type falls back to general-purpose. Snapshot: 2. Fork enabled via explicit opt-inCommand used: Prompt used in the interactive session: Expected output / behavior:
Snapshot: 
|
ff8494c to
36b30b2
Compare
tanzhenxin
requested changes
Jun 3, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
Thanks for the quick turnaround — I went through this against the latest commits, and the direction we discussed has landed cleanly. Fork is now opt-in and off by default, omitting subagent_type falls back to a normal general-purpose agent (no error) when it's off, the interactive check stays as a second condition on top of the opt-in, and the "When to fork / Don't peek / Don't race" guidance only appears when the feature is enabled. All four points addressed, and the new tests exercise both the fork-off and non-interactive fallback paths rather than just string-checking the description. Nice work.
One blocker remains, and it's the deconfliction with #4436 I'd flagged earlier — now concrete rather than hypothetical, since #4436 has merged to main.
1. Duplicate "Writing the prompt" section in the Agent tool description (severity: medium · confidence: very high)
#4436 added a "Writing the prompt" block to the same tool description, and it's now on main. After merging main into this branch, the description carries that block twice — two back-to-back "Writing the prompt" sections, each with its own near-identical "brief the agent like a smart colleague" and "never delegate understanding" paragraphs, differing only in wording. It ships on every Agent call, including when fork is off (the default), because this section isn't behind the feature flag — only the fork-specific guidance is. The current tests don't catch it: they assert the section is present, which stays true with two copies.
The fix is to land exactly one. Since #4436's version is already on main, the cleanest path is to fold this PR's fork-aware phrasing (the "when spawning a fresh agent…" wording) into that existing block and drop the duplicate this PR adds.
Verdict
Request changes — narrowly. The default-off direction is fully done and correct; the only thing left is collapsing the duplicated "Writing the prompt" guidance down to a single section.
Collaborator
Author
duplicate "writing prompt" is removed |
tanzhenxin
approved these changes
Jun 3, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
Thanks for the quick turnaround — the duplicate ## Writing the prompt section is gone, folded into a single block that keeps the existing wording and toggles the fork-specific phrasing by the feature flag. That was the only thing holding this up. The default-off gating from the previous round is intact, and the tests cover both the fork-off and non-interactive fallback paths.
Verdict
APPROVE — the duplicate prompt section is resolved exactly as suggested; the remaining nits (a stray one-line reformat in the hooks constants and the two same-named isForkSubagentEnabled helpers) are cosmetic and not worth holding the PR.
yiliang114
approved these changes
Jun 3, 2026
tanzhenxin
added a commit
that referenced
this pull request
Jun 8, 2026
… onto main Port the shell notification feature (#4355) from the old BackgroundShellRegistry class into the shell-task.ts free-function module. Update all call sites (Session.ts, useGeminiStream.ts, chatCompressionService.ts) and their tests to use the new module-level setters and unified TaskRegistry API. Also integrates AgentTerminateMode.SHUTDOWN grouping with CANCELLED (#4410) and fork-subagent feature gate (#4574) into the free-function architecture.
tanzhenxin
added a commit
that referenced
this pull request
Jun 8, 2026
… onto main Port the shell notification feature (#4355) from the old BackgroundShellRegistry class into the shell-task.ts free-function module. Update all call sites (Session.ts, useGeminiStream.ts, chatCompressionService.ts) and their tests to use the new module-level setters and unified TaskRegistry API. Also integrates AgentTerminateMode.SHUTDOWN grouping with CANCELLED (#4410) and fork-subagent feature gate (#4574) into the free-function architecture.
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
4 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.
What this PR does
Adds two missing capabilities to the fork subagent system, ported from Claude Code's architecture:
isForkSubagentEnabled): Fork subagents are now disabled in non-interactive sessions (qwen -p, SDK headless, CI/CD). Attempting to fork in these modes returns a clear error message advising the caller to use an explicitsubagent_typeinstead.Why it's needed
Without the feature gate, fork subagents could be triggered in non-interactive sessions where there is no terminal UI for progress display or permission bubble-up, potentially causing hangs or silent failures.
Without the prompt discipline, the main agent tends to "peek" at fork intermediate output (polluting its own context) or "race" ahead by fabricating fork results before the completion notification arrives — a natural LLM tendency that Claude Code addresses with explicit prompt constraints. These two failure modes degrade task quality and produce hallucinated outputs that users cannot distinguish from real results.
Reviewer Test Plan
How to verify
1、Unit tests — Run
npx vitest run packages/core/src/tools/agent/agent.test.ts. All 81 tests pass, including 3 new tests:rejects fork in non-interactive mode— verifies the gate blocks fork whenisInteractive()returns falseincludes "When to fork" section in description when interactive— verifies prompt contains "Don't peek", "Don't race", "Writing a fork prompt"omits fork discipline but keeps "Writing the prompt" when non-interactive— verifies fork-specific prompt is absent but general "Writing the prompt" section is present2、Type check — Run
npx tsc --noEmit --project packages/core/tsconfig.json. No errors.Evidence (Before & After)
N/A — Non-UI change (runtime behavior gate + prompt injection). Unit tests provide complete coverage.
Tested on
Environment (optional)
Risk & Scope
isInteractive()only). If Qwen Code adds a coordinator mode in the future, it should also be excluded — matching Claude Code'sisCoordinatorMode()check.subagent_type) will now receive an error. This is the intended behavior — such sessions should use explicitsubagent_typeinstead.Linked Issues
中文说明