feat(core): Disallow plan lifecycle tools in subagents - #6087
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
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. |
|
Thanks for the PR, @doudouOUC! Template looks good ✓ On direction: This is a clean correctness fix. Plan mode lifecycle being caller-owned is the right invariant — a subagent flipping plan mode creates confusing loops and breaks the workflow "final text IS the return value" contract. Claude Code's CHANGELOG shows parallel work on restricting subagent tool surfaces (MCP server-level On approach: The scope feels right. A shared Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个干净的正确性修复。plan mode 生命周期归调用方所有是正确的不变量——subAgent 切换 plan mode 会造成混乱循环并破坏 workflow "最终文本即返回值" 契约。Claude Code 的 CHANGELOG 也有类似的 subAgent 工具面限制(MCP 方案:范围合理。共享的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Code ReviewIndependent proposal (before reading the diff): I would have added Comparison: The PR's approach matches my proposal and exceeds it. The Reuse check: The new policy module correctly reuses Findings: No critical blockers. The code is straightforward and well-tested (614 test assertions across the referenced suites, all passing). The One non-blocking note: the workflow orchestrator changes add Test ResultsUnit tests (worktree, PR branch Build + typecheck: Both pass cleanly. Real-Scenario Testing (tmux)This PR changes internal subagent behavior (tool restriction), not user-facing TUI. The best tmux verification is that the CLI still works and plan mode functions correctly for the main session. Before (installed build)After (this PR via built bundle)Plan mode (this PR, main session)Main-session plan mode works as expected — the model plans without executing tools, and the 中文说明代码审查独立方案(读 diff 之前): 我会把 比较: PR 的方案与我的匹配并且超出。 复用检查: 新策略模块正确复用了 发现: 无关键阻断。代码简洁且测试充分(614 个测试断言全部通过)。 一个非阻断备注:workflow orchestrator 的改动把 测试结果单元测试: 11 个测试文件,844 个测试全部通过。Build + typecheck 均通过。 真实场景测试 (tmux)此 PR 改变的是内部 subagent 行为(工具限制),非用户可见的 TUI。tmux 验证表明 CLI 仍然正常工作,plan mode 在主会话中功能正常(见上方英文部分终端输出)。 — Qwen Code · qwen3.7-max |
|
This is a well-executed correctness fix that's been through several rounds of refinement already. Going back to my independent proposal from Stage 2: the PR's approach matches it and exceeds it by centralizing the policy into a shared module rather than scattering checks across consumer files. That's the right call — it makes the enforcement logic auditable in one place and easy to extend if more plan-adjacent tools are added later. The test coverage is thorough: 844 assertions across 11 test files, covering every enforcement layer (tool declarations, explicit lists, inline declarations, direct calls, tool-search, scheduler, workflow floor). The Build and typecheck pass. The tmux smoke tests confirm the CLI still works and main-session plan mode is unaffected. The If I had to maintain this in six months, I'd thank the author — the shared policy module with clear function names ( Approving. ✅ 中文说明这是一个经过多轮打磨的高质量正确性修复。回到我在 Stage 2 的独立方案:PR 的方案与之匹配并通过将策略集中到共享模块而超越——这是正确的选择,让执行逻辑在一处可审计,也便于将来添加更多 plan 相关工具时扩展。 测试覆盖充分:11 个测试文件,844 个断言,覆盖每一层执行点。 Build 和 typecheck 通过。tmux 冒烟测试确认 CLI 仍正常工作,主会话 plan mode 不受影响。 如果六个月后需要维护这段代码,我会感谢作者——共享策略模块配合清晰的函数名让执行意图一目了然,测试套件在每一层都能捕获回归。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] Conflicting system instructions for subagents in plan mode
When a subagent inherits approvalMode=plan from the parent session, two code paths produce contradictory guidance:
client.ts:2106callsgetPlanModeSystemReminder(this.config.getSdkMode())— for CLI sessionsgetSdkMode()returnsfalse, so the system prompt tells the model: "call the exit_plan_mode tool".coreToolScheduler.ts:2272callsgetPlanModeSystemReminder(isSubagentLikeExecutionContext())— for subagents this returnstrue, so blocked tool messages say: "Present your plan directly".
The subagent receives both instructions in the same session: "call exit_plan_mode" from the system prompt and "don't call exit_plan_mode, present your plan directly" from the per-tool error. Since exit_plan_mode is blocked at the tool level (returns "not available"), the system prompt's instruction to call it creates a dead-end loop.
Suggested fix: In client.ts:2106, use the same context-aware predicate:
getPlanModeSystemReminder(isSubagentLikeExecutionContext() || this.config.getSdkMode())— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
DragonnZhang
left a comment
There was a problem hiding this comment.
Incremental review of commit 585aa25 (addressing follow-up PR review feedback): no new HIGH-CONFIDENCE findings.
This commit makes three targeted improvements:
-
Context-aware error messages (
agent-core.ts): ReplacesisSubagentPlanLifecycleToolwithisPlanLifecycleToolUnavailableInSubagent, so the subagent-specific error message is only shown when actually in a subagent context. Previously, a non-subagent context with an unregistered plan tool would get a misleading subagent error instead of the generic "not found" message. Clean semantic fix. -
Subagent plan reminder (
client.ts): When a subagent inherits PLAN mode from the parent,getPlanModeSystemRemindernow receivesplanOnly=true(viaisSubagentLikeExecutionContext()), telling the subagent to present its plan directly rather than attempting to callExitPlanMode(which is filtered out). Correct behavioral alignment. -
DRY in workflow-orchestrator.ts: Replaces hardcoded tool names with
...SUBAGENT_PLAN_LIFECYCLE_TOOLSfrom the shared policy module.
Tests are properly updated — processFunctionCalls tests now wrap in runWithAgentContext + runInAgentFrames to establish the subagent context required by the new context-aware check. The inline declaration test correctly exercises the full subagent exclusion floor (adding SEND_MESSAGE and TASK_UPDATE). The new client test verifies the subagent plan reminder path.
qqqys
left a comment
There was a problem hiding this comment.
The previous critical issue is resolved: subagents in plan mode now receive the plan-only reminder from the startup/system path as well as the blocked-tool path, so they are no longer instructed to call an unavailable exit_plan_mode tool. I did not find any new critical blocker in the latest delta.
— GPT-5 via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No Critical findings. One observability suggestion below — the new subagent-block execute guards return silently while every other error path in these files emits a debugLogger.warn/error call. The same gap exists in exitPlanMode.ts:206.
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Additional test coverage gaps not mappable to a single diff line:
- No unit test for
subagent-plan-tool-policy.ts— its 4 exports are only tested indirectly through 6+ consumer files. A focused canary test would pinpoint regressions to this module. getDefaultPermission()inexitPlanMode.test.tsnot tested insiderunWithTeammateIdentity(onlyrunWithAgentContext). TheisTeammate()branch is unverified.tool-search.test.tssubagent rejection tests don't assert onreturnDisplay(the TUI-visible string). No test for mixed scenario (select:read_file,enter_plan_mode).- No regression test for main-session
getPlanModeSystemReminder(false)inclient.test.ts— the only new test assertsplanOnly=truefor subagents.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
DragonnZhang
left a comment
There was a problem hiding this comment.
Incremental review (SHA 93d69db): No new HIGH-CONFIDENCE findings.
Three-pass analysis (correctness, security, code quality) confirms all identifiable issues were already surfaced by prior reviewers. Key observations:
- Defense-in-depth is solid: Plan lifecycle tools are blocked at three layers — tool declaration filtering (
prepareTools), runtime execution guards (execute()in both plan tools), andprocessFunctionCallserror differentiation. Thesubagent-plan-tool-policy.tsshared module keeps the policy consistent. - Workflow subagent wrapping: The
runWithAgentContext()addition inworkflow-orchestrator.tsis load-bearing and correct — it establishes the ALS frame thatisSubagentLikeExecutionContext()reads. TheMONITORaddition toWORKFLOW_SUBAGENT_DISALLOWED_TOOLScloses theownerAgentIdside-effect concern. - ToolSearch blocking: Correctly prevents subagents from discovering/loading plan lifecycle tools via
select:queries, and properly handles the mixed success/blocked case by only settingerrorwhenloaded.length === 0. - Previously flagged items still open (all Suggestion-level):
getDefaultPermissionreturning'allow'with semantic contradiction,WORKFLOW_SUBAGENT_DISALLOWED_TOOLSnot deriving from shared set, missing debug logging in some guard paths, andgetPlanModeSystemReminderparameter asymmetry betweenclient.tsandcoreToolScheduler.ts. None represent correctness bugs.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No new findings from this review pass. The code is well-structured with solid defense-in-depth across three layers (tool-list filtering, ToolSearch blocking, and execute-time guards). All 846 tests pass.
Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
✅ Maintainer local verification — real build, tests, mutation & live-binary E2EVerified at PR head 1. Targeted regression suite — greenRan the full PR test plan plus the two new files on real
2. Mutation testing — the guards are load-bearingEach guard was reverted individually; the relevant test flips red, then was restored (tree clean afterwards):
Mutation A also confirms the fix is genuinely centralized: emptying one 3. Live-binary E2E — real
|
| # | Actor (by system prompt) | #tools | agent |
enter_plan_mode |
exit_plan_mode |
|---|---|---|---|---|---|
| req-000 | MAIN (You are Qwen Code…) |
15 | ✅ | ✅ | ✅ |
| req-001 | SUBAGENT (general-purpose agent…) |
51 | ❌ | ❌ | ❌ |
| req-002 | SUBAGENT | 51 | ❌ | ❌ | ❌ |
| req-003 | MAIN | 15 | ✅ | ✅ | ✅ |
- Layer 1 (declaration filtering): the subagent's request advertises 51 deferred-inclusive tools yet none of
enter_plan_mode/exit_plan_mode/agent; the same binary's main session (req-000/req-003) keeps all three. Main-session plan mode is not regressed. - Layer 2 (direct-call guard): when the subagent emitted the forced
exit_plan_modecall, the runtime returned this exact string as therole:toolresult fed back into the subagent conversation (req-002) — a message only the PR's guard can produce:exit_plan_mode is not available inside subagents or team agents. Plan mode is owned by the caller/main session; return your plan, findings, or constraints to the caller in your normal response instead of entering or exiting plan mode. - The subagent's result then returned to MAIN (req-003) and the run completed cleanly.
(A real model would never call a tool absent from its declarations; forcing the call is a deliberate adversarial probe to exercise the defense-in-depth guard.)
4. Notes for the merge decision (non-blocking)
- The change also widens the explicit-list / inline-declaration filter from the old AGENT-only guard to the full subagent control-plane floor (coordination / task / cron / worktree / workflow / recursion / plan). This is a genuine isolation improvement, but it is broader than "plan tools" — the Breaking changes note only calls out
enter/exit_plan_mode; a custom subagent config that explicitly listed e.g.send_messagewill now have it stripped too. Intended per the PR description, worth a one-line mention in the migration note. monitorwas added to the workflow-subagent disallowed floor (beyond plan tools). Justified in-code (it depends on AgentTool-owned notification callbacks workflow subagents never register) and covered by the updated workflow test.- Teammate coordination tools (
send_message,task_*) are correctly kept for teammates while plan tools are excluded — confirmed by test and by the E2E discriminator. - CI: 9 checks green, 22 skipped (mac/win/Integration skip on PRs); no failures.
BLOCKEDis onlyREVIEW_REQUIRED.
Verdict: design is sound and the guards are proven load-bearing in both unit tests and the live binary. LGTM from a verification standpoint. 👍
🇨🇳 中文版(完整对应)
✅ 维护者本地验证 —— 真实构建、测试、变异与真实二进制 E2E
在 PR head 80695fc3a 的独立 worktree 里验证(npm ci + 完整 npm run build,避免与主 checkout 产生 split-brain)。以下每条结论都在本地重新跑过,不是复述测试计划。
1. 目标回归套件 —— 全绿
在真实 src 上(vitest alias 到源码,免构建)跑完整 PR 测试计划 + 两个新文件:
subagent-plan-tool-policy · agent-core · workflow-orchestrator · client · coreToolScheduler
enterPlanMode · exitPlanMode · tool-search · agent/agent · agent/agent-override · tool-registry
→ 11 个文件全过 | 846 个测试全过
core tsc --noEmit 干净;完整 npm run build 退出码 0。全量 core 套件:13784 通过;35 个失败全部落在与 PR 无关的子系统(gitDiff、filesearch/crawler、team-memory-sync、team-create),高并发满载 + coverage 所致,单独隔离全部通过(127/127)→ 环境抖动,非回归。
2. 变异测试 —— 守卫确实承重
逐个还原每处守卫,对应测试翻红,随后恢复(结束后工作树干净):
| # | 变异 | 结果 |
|---|---|---|
| A | 清空共用的 SUBAGENT_PLAN_LIFECYCLE_TOOLS set |
跨 6 文件 22 个失败(声明过滤 / 直接调用兜底 / execute() 守卫 / tool-search / workflow floor) |
| C | 把 plan reminder 退回只看 getSdkMode()(去掉 isSubagentLikeExecutionContext()) |
3 个失败——正好是 subagent+teammate;SDK 与主会话仍绿 |
| D | 去掉 workflow dispatch 里的 runWithAgentContext(workflowAgentId, …) |
1 个失败——workflow subagent 失去 ALS 帧,不再被识别为 subagent-like |
| E | 把 processFunctionCalls 强制退回纯 "not found" |
2 个失败——专用 "not available inside subagents" 信息 |
| F | 把 inline 声明过滤退回旧的只挡 AGENT | 1 个失败——send_message/task_update 的 inline 声明重新泄漏 |
变异 A 也证明修复真正做到了集中化:清空一个 Set 就能一次性瘫痪全部 6 处接入点。
3. 真实二进制 E2E —— tmux 里跑真 qwen,不是 mock
用本 PR 构建 dist/cli.js,在 tmux 里以 headless -p --yolo --auth-type openai 打向一个会记录每个请求的假 OpenAI 端点。假模型强制走:主会话 → 用 Agent 工具派生 subagent → 让 subagent 调 exit_plan_mode。判据是每轮的请求体(工具声明 + 回喂的工具结果),这些由运行时(而非 CLI 打印层)产生。
确定性 4 请求流,进程退出码 0,最终 stdout MAIN_DONE_6087:
| # | 角色(按 system prompt) | 工具数 | agent |
enter_plan_mode |
exit_plan_mode |
|---|---|---|---|---|---|
| req-000 | MAIN(You are Qwen Code…) |
15 | ✅ | ✅ | ✅ |
| req-001 | SUBAGENT(general-purpose agent…) |
51 | ❌ | ❌ | ❌ |
| req-002 | SUBAGENT | 51 | ❌ | ❌ | ❌ |
| req-003 | MAIN | 15 | ✅ | ✅ | ✅ |
- 第 1 层(声明过滤):subagent 的请求声明了 51 个(含 deferred)工具,却没有任何
enter_plan_mode/exit_plan_mode/agent;而同一个二进制的主会话(req-000/003)三者都在。主会话 plan mode 未受影响。 - 第 2 层(直接调用兜底):当 subagent 发出被强制的
exit_plan_mode调用时,运行时把下面这条精确串作为role:tool结果回喂进 subagent 对话(req-002)——只有本 PR 的守卫能产生:exit_plan_mode is not available inside subagents or team agents. Plan mode is owned by the caller/main session; return your plan, findings, or constraints to the caller in your normal response instead of entering or exiting plan mode. - subagent 结果随后返回 MAIN(req-003),整轮干净收尾。
(真实模型绝不会调用声明里没有的工具;强制这次调用是刻意的对抗性探针,用来触发这层纵深防御守卫。)
4. 合并决策参考(非阻塞)
- 本改动还把显式列表 / inline 声明过滤从旧的只挡 AGENT拓宽为完整的 subagent 控制面 floor(coordination / task / cron / worktree / workflow / recursion / plan)。这是实打实的隔离改进,但范围超出"plan 工具"——Breaking changes 只写了
enter/exit_plan_mode;显式列了比如send_message的自定义 subagent 配置现在也会被剥掉。这符合 PR 描述意图,建议在迁移说明里补一句。 monitor被加进 workflow-subagent 的 disallowed floor(超出 plan 工具)。代码内有理由(它依赖 AgentTool 拥有的通知回调,而 workflow subagent 从不注册),且更新后的 workflow 测试有覆盖。- teammate 的协作工具(
send_message、task_*)在排除 plan 工具的同时被正确保留——测试与 E2E 判据都确认了。 - CI:9 项绿、22 项 skip(mac/win/Integration 在 PR 上 skip),无失败。
BLOCKED仅因REVIEW_REQUIRED。
**结论:**设计合理,守卫在单测与真实二进制中都被证明承重。从验证角度 LGTM。👍
What this PR does
This PR makes plan mode lifecycle ownership stay with the caller/main session by preventing ordinary subagents, workflow subagents, and in-process teammates from entering or exiting plan mode themselves. Subagents still inherit the parent PLAN constraints, but when they finish planning they now return the plan or findings to the caller instead of requesting
exit_plan_mode.It adds a shared runtime policy for subagent-like execution contexts, applies that policy consistently to subagent tool declaration filtering, direct filtered tool-call errors, runtime tool execution guards, ToolSearch exact selection, workflow subagent disallowed-tool floors, and plan-mode blocked-tool reminders.
It also applies the existing subagent/teammate control-plane exclusion floor to inline
FunctionDeclarationtool configs, so inline declarations cannot bypass restrictions for coordination, task, cron, worktree, workflow, agent recursion, or plan lifecycle tools.Why it's needed
Issue #6083 tracks the next phase after the subagent approval-mode state fix. The remaining risk is that a subagent can still be instructed to use plan lifecycle tools that should belong to the main session, which can create confusing loops or workflow contract breaks when a subagent is in PLAN mode but should simply return its plan to the caller.
Keeping plan lifecycle tools out of subagent and team-agent tool surfaces aligns the behavior with caller-owned approval flow while preserving the existing main-session plan mode behavior and the
exit_plan_modealways-loaded regression guard.Reviewer Test Plan
How to verify
Run the targeted core regression suite and confirm ordinary subagent, teammate, workflow, ToolSearch, scheduler, and existing agent override tests pass. In particular, verify subagent tool declarations do not include
enter_plan_modeorexit_plan_mode, explicit or inline tool configs cannot re-enable them, direct calls return a dedicated unavailable message, ToolSearch does not reveal or setTools for these tools in subagent contexts, and main-sessionexit_plan_modeinspection still works.Run the root build and typecheck to confirm the repository still compiles.
Evidence (Before & After)
N/A
Tested on
Environment (optional)
macOS, Node.js/npm workspace. Commands run locally:
cd packages/core && npx vitest run src/agents/runtime/agent-core.test.ts src/tools/enterPlanMode.test.ts src/tools/exitPlanMode.test.ts src/tools/tool-search.test.ts src/agents/runtime/workflow-orchestrator.test.ts src/core/coreToolScheduler.test.ts src/tools/agent/agent.test.ts src/tools/agent/agent-override.test.ts src/tools/tool-registry.test.ts;npm run build && npm run typecheck.Risk & Scope
enter_plan_modeorexit_plan_modemust return the plan or findings to the caller instead.Linked Issues
Closes #6083
中文说明
What this PR does
这个 PR 让 plan mode 生命周期继续归调用方/主会话所有,普通 subAgent、workflow subAgent 和进程内 teammate 不能再自行进入或退出 plan mode。subAgent 仍然继承父会话的 PLAN 约束,但完成规划后会把计划或发现返回给调用方,而不是请求
exit_plan_mode。它新增了一个共享的 subAgent-like 运行时策略,并把该策略一致应用到 subAgent 工具声明过滤、被过滤工具的直接调用错误、工具运行时兜底、ToolSearch 精确选择、workflow subAgent 的 disallowed-tool floor,以及 plan-mode 阻断工具时的 reminder。
它也会把既有 subAgent/teammate 控制面 exclusion floor 应用到 inline
FunctionDeclaration工具配置,因此 inline 声明不能绕过 coordination、task、cron、worktree、workflow、agent recursion 或 plan 生命周期工具限制。Why it's needed
Issue #6083 记录的是子 Agent approval-mode 状态修复后的下一阶段。剩余风险是 subAgent 仍可能被指示使用本应属于主会话的 plan 生命周期工具,这会在 subAgent 处于 PLAN 模式但本应直接把计划返回给调用方时造成困惑循环或破坏 workflow 返回值契约。
把 plan 生命周期工具从 subAgent 和 team-agent 工具面中移除,可以让审批流保持调用方所有,同时保留现有主会话 plan mode 行为和
exit_plan_modealways-loaded 回归保护。Reviewer Test Plan
How to verify
运行目标 core 回归测试并确认普通 subAgent、teammate、workflow、ToolSearch、scheduler 和既有 agent override 测试都通过。重点确认 subAgent 工具声明不包含
enter_plan_mode或exit_plan_mode,显式或 inline 工具配置不能重新启用它们,直接调用会返回专用不可用信息,ToolSearch 在 subAgent 上下文不会 reveal 或 setTools 这些工具,并且主会话仍然可以 inspectexit_plan_mode。运行根目录 build 和 typecheck,确认仓库仍可编译。
Evidence (Before & After)
N/A
Tested on
Environment (optional)
macOS,Node.js/npm workspace。本地运行命令:
cd packages/core && npx vitest run src/agents/runtime/agent-core.test.ts src/tools/enterPlanMode.test.ts src/tools/exitPlanMode.test.ts src/tools/tool-search.test.ts src/agents/runtime/workflow-orchestrator.test.ts src/core/coreToolScheduler.test.ts src/tools/agent/agent.test.ts src/tools/agent/agent-override.test.ts src/tools/tool-registry.test.ts;npm run build && npm run typecheck。Risk & Scope
enter_plan_mode或exit_plan_mode的自定义 subAgent 或 teammate 配置需要改为把计划或发现返回给调用方。Linked Issues
Closes #6083