fix: enable fork subagents by default#4963
fix: enable fork subagents by default#4963wenshao merged 2 commits intoQwenLM:mainQwenLM/qwen-code:mainfrom he-yufeng:fix/fork-subagent-default-bubblehe-yufeng/qwen-code:fix/fork-subagent-default-bubbleCopy head branch name to clipboard
Conversation
a0b4d6e to
b83dc5d
Compare
| */ | ||
| export function isForkSubagentEnabled(config: Config): boolean { | ||
| return config.isForkSubagentEnabled() && config.isInteractive(); | ||
| return config.isInteractive(); |
There was a problem hiding this comment.
[Suggestion] Config.isForkSubagentEnabled() and its backing field forkSubagentEnabled are now dead code. After this change, the standalone function only calls config.isInteractive() — nothing in production code calls the Config method anymore (verified via grep). Consider removing Config.isForkSubagentEnabled(), the forkSubagentEnabled field, and the forkSubagentEnabled constructor parameter in a follow-up cleanup.
— qwen3.7-max via Qwen Code /review
|
|
||
| it('omits fork discipline when interactive but fork flag is off', async () => { | ||
| it('includes fork discipline when interactive even if the legacy flag is off', async () => { | ||
| (config as unknown as Record<string, unknown>)['isInteractive'] = vi |
There was a problem hiding this comment.
[Suggestion] The mock for config.isForkSubagentEnabled in this test (and several others in this file) no longer controls the fork gate. The production path goes through the standalone isForkSubagentEnabled(config) in fork-subagent.ts, which only checks config.isInteractive(). These mocks are harmless but may mislead future readers into thinking the test is controlling fork availability via the config method. Consider removing the isForkSubagentEnabled mocks from tests that exercise the fork-gate decision — only the isInteractive mock matters.
— qwen3.7-max via Qwen Code /review
34ac0a4 to
2d3152d
Compare
|
Updated for the review feedback and rebased onto current upstream/main. The stale Config forkSubagentEnabled field/method and the related misleading test mocks are removed; fork availability now depends only on interactive mode through the standalone gate. Verified locally: prettier --check on the touched files, eslint on the touched files, typecheck for packages/core and packages/cli, core agent.test.ts (98 tests), cli forkCommand.test.ts (15 tests), and git diff --check. |
DragonnZhang
left a comment
There was a problem hiding this comment.
LGTM. Clean removal of the fork feature gate. All references to forkSubagentEnabled, isForkSubagentEnabled(), and the QWEN_CODE_ENABLE_FORK_SUBAGENT env var are consistently removed across config, commands, i18n (all locales), and tests. The isForkSubagentEnabled() helper now correctly gates only on isInteractive(), preserving the non-interactive safety guard. The explicit approvalMode: 'default' on FORK_AGENT with satisfies SubagentConfig is a good hardening — makes fork permissions deterministic rather than relying on the fallback path. Tests are well updated, including the renamed assertions for fork discipline and interactive-mode behavior.
✅ Verification report — built the real CLI and ran a live A/B for the fork gateI built the actual Method
Live A/B — the gate removal (env var unset in both)
The PR fork ran end-to-end: inherited the conversation, executed a tool, completed ( The other two claimsNon-interactive stays on general-purpose fallback ✅
Fork agent runs in
Tests & build (all green)
One thing for reviewers to confirm is intended (not a bug)The PR removes the VerdictLooks good to merge. The fix is verified live (faithful A/B) and by the full cited test suite; removal is complete with no dangling references or stale docs; the approval-mode behavior is sound and test-covered. 中文版验证报告（点击展开）✅ 验证报告 —— 构建真实 CLI 并对 fork 开关做了实测 A/B我从本 PR 的 HEAD 构建了真实的 方法
实测 A/B —— 开关移除(两次均未设环境变量)
PR 的 fork 端到端跑通:继承了对话、执行了工具、运行完成( 另外两个声明非交互模式仍走 general-purpose 回退 ✅
fork agent 在受信任目录下以
测试与构建(全绿)
一处建议 reviewer 确认是否为预期(并非 bug)本 PR 完全移除了 结论可以合并。 修复已通过实测(忠实 A/B)与全部所列测试验证;移除彻底、无悬空引用、无过时文档;审批模式行为正确且有测试覆盖。 |
* fix: enable fork subagents by default * fix: respect fork subagent config flag
…ype awaitable Enabling fork-by-default for every interactive session (QwenLM#4963) made an omitted `subagent_type` mean a fire-and-forget background fork. A fork's findings never flow back into the parent turn, so any caller that spawns parallel agents and then aggregates their results breaks — the bundled `review`/`simplify` skills, third-party skills, and the model's own delegation. The orchestrator waits forever for results that never arrive; nudged by the periodic todo reminder it can also spin on identical tool calls until DashScope rejects the request with `InternalError.Algo.InvalidParameter: Repetitive tool calls detected` (HTTP 400). Make forking an explicit, deliberate choice and stop steering the model toward it when it needs the results: - Dispatch: `subagent_type: "fork"` selects a fork (interactive only; otherwise general-purpose). Omitting `subagent_type` always resolves to the awaitable general-purpose subagent whose result returns inline — never a fork. `/fork` passes `subagent_type: "fork"` explicitly. - Prompt: drop the "launch parallel forks for research" guidance, reframe the tool description and "When to fork" around "never fork work whose output you need", and stop advertising `fork` in the subagent_type enum (it stays valid via validation for `/fork` and intentional use). - Skills: `review` and `simplify` launch their parallel agents with `subagent_type: "general-purpose"` and are told explicitly not to fork, since they aggregate findings. Forking stays a first-class, default-available feature for genuinely fire-and-forget work — it just isn't the silent default, an enum pick, or something the model is nudged toward when it must read the results.
…ype awaitable (#5155) Enabling fork-by-default for every interactive session (#4963) made an omitted `subagent_type` mean a fire-and-forget background fork. A fork's findings never flow back into the parent turn, so any caller that spawns parallel agents and then aggregates their results breaks — the bundled `review`/`simplify` skills, third-party skills, and the model's own delegation. The orchestrator waits forever for results that never arrive; nudged by the periodic todo reminder it can also spin on identical tool calls until DashScope rejects the request with `InternalError.Algo.InvalidParameter: Repetitive tool calls detected` (HTTP 400). Make forking an explicit, deliberate choice and stop steering the model toward it when it needs the results: - Dispatch: `subagent_type: "fork"` selects a fork (interactive only; otherwise general-purpose). Omitting `subagent_type` always resolves to the awaitable general-purpose subagent whose result returns inline — never a fork. `/fork` passes `subagent_type: "fork"` explicitly. - Prompt: drop the "launch parallel forks for research" guidance, reframe the tool description and "When to fork" around "never fork work whose output you need", and stop advertising `fork` in the subagent_type enum (it stays valid via validation for `/fork` and intentional use). - Skills: `review` and `simplify` launch their parallel agents with `subagent_type: "general-purpose"` and are told explicitly not to fork, since they aggregate findings. Forking stays a first-class, default-available feature for genuinely fire-and-forget work — it just isn't the silent default, an enum pick, or something the model is nudged toward when it must read the results.
Fixes #4956.
Summary
default, so trusted folders do not silently promote fork workers to auto-edit/forkfeature-gate error string from required i18n keys and locale filesCurrent main does not define a separate
bubbleapproval mode;defaultis the existing mode that routes tool calls through the normal permission prompt path.To verify
npm run generatenpm run build --workspace=@qwen-code/web-templatesnpm run build --workspace=@qwen-code/channel-basenpm run build --workspace=@qwen-code/channel-telegramnpm run build --workspace=@qwen-code/channel-weixinnpm run build --workspace=@qwen-code/channel-dingtalknpm run build --workspace=@qwen-code/channel-feishunpm run build --workspace=@qwen-code/channel-plugin-examplenpm run build --workspace=@qwen-code/acp-bridgenpm test -- src/tools/agent/agent.test.tsfrompackages/corenpm test -- src/ui/commands/forkCommand.test.ts src/i18n/mustTranslateKeys.test.tsfrompackages/clinpm run check-i18nfrompackages/clinpm run typecheck --workspace=@qwen-code/qwen-code-corenpm run typecheck --workspace=@qwen-code/qwen-codenpx eslinton the changed core/CLI/i18n filesgit diff --check