fix(core): use consistent error response for plan mode blocked tools - #6667
Conversation
|
Qwen precheck requires maintainer approval before automated triage/review. Head SHA: Reason:
A maintainer with write access can inspect the PR and manually request a run with |
When a non-read-only tool is called in plan mode, the LLM receives
{ output: "<system reminder text>" } instead of { error: "..." }.
The output key looks like success to the LLM, so it does not recognize
the tool was denied and may try alternative approaches to bypass the
restriction.
Use createErrorResponse() so the LLM sees a clear error signal with
error key, Error object, and errorType: EXECUTION_DENIED.
Preserve differentiated guidance per caller type:
- Plan-required teammates: "Call exit_plan_mode to exit plan mode..."
- SDK / ordinary subagents: "Present your plan directly to the caller..."
Signed-off-by: Alex <alex.tech.lab@outlook.com>
9f84bcb to
ad99b22
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| // teammates have a dedicated exit_plan_mode approval path. | ||
| const isPlanRequiredTeammate = | ||
| !shouldUsePlanOnlyReminderInSubagentContext() && | ||
| !this.config.getSdkMode(); |
There was a problem hiding this comment.
[Suggestion] Setting error and errorType here is the correct fix, but it causes a behavioral side effect: BaseJsonOutputAdapter.ts (lines 1039-1049) records into permission_denials whenever response.error is truthy and errorType === EXECUTION_DENIED. Plan-mode-blocked tool calls were previously invisible to that tracker (both fields were undefined). They will now appear alongside actual user-declined permission denials in SDK/non-interactive JSON output, with no discriminator field (e.g., reason: "plan_mode" vs reason: "user_declined").
If any SDK consumer iterates permission_denials to decide whether to re-prompt the user or abort, plan-mode blocks will be misclassified as user rejections.
Consider either filtering plan-mode blocks out of permissionDenials, or adding a discriminator to CLIPermissionDenial.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
|
Thanks for the PR! Template looks good ✓ Problem: observed in code. The old plan mode blocked path returned Direction: aligned. Making plan mode blocks use the same error format as all other denials is a straightforward consistency improvement — the LLM already understands Size: 32 production lines (18 additions + 14 deletions in Approach: minimal and focused. The change does one thing — swap Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:在代码中已观测到。旧的 plan mode 阻断路径通过 方向:对齐。让 plan mode 阻断使用与其他所有拒绝路径(权限拒绝、非交互模式拒绝、后台 agent 拒绝等)相同的错误格式——LLM 已能识别 规模:32 行生产代码( 方案:最小且聚焦。只做一件事——把 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: Replace Diff comparison: The PR matches this proposal exactly. The Reuse check: No issues found. The change is correct, minimal, and consistent with the codebase. TestingUnit Tests14 plan mode tests pass against the PR code: Dev Mode Sanity CheckCLI runs correctly with the PR code — no crashes, tool scheduler operates normally: Note on Real-Scenario TestingThis change modifies the internal error response format (what the LLM receives when plan mode blocks a tool), not user-visible terminal output. Plan mode entry requires interactive model decisions that can't be forced via 中文说明代码审查独立方案: 将 Diff 对比: PR 与独立方案完全一致。 复用检查: 未发现问题。变更正确、最小化,与代码库一致。 测试单元测试14 个 plan mode 测试在 PR 代码上全部通过。 Dev 模式健全性检查CLI 在 PR 代码下正常运行——无崩溃,工具调度器正常工作。 关于真实场景测试的说明此变更修改的是内部错误响应格式(LLM 在 plan mode 阻断工具时收到的内容),而非用户可见的终端输出。Plan mode 进入需要交互式模型决策,无法通过 — Qwen Code · qwen3.7-max |
|
This is a clean, well-scoped fix. The problem is real — plan mode was the only blocked path that returned The diff is minimal — 32 production lines doing one thing. No drive-by refactors, no scope creep. The Approving. 中文说明这是一个干净、范围良好的修复。问题是真实存在的——plan mode 是唯一一个返回 Diff 最小化——32 行生产代码只做一件事。没有顺手重构,没有范围蔓延。 批准通过。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Local verification report (maintainer)I built and tested this PR locally end-to-end. It behaves exactly as described and I recommend merging. Details below as a merge reference. Environment: macOS (darwin 24.6) · Node v22.23.1 · vitest v3.2.4 · isolated worktree at PR head What I ran# 1. Cited plan-mode suite
cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts -t "plan mode"
# → 14 passed | 244 skipped
# 2. Full test file (regression check)
npx vitest run src/core/coreToolScheduler.test.ts
# → 258 passed (258)
# 3. Typecheck + lint on the change
npm run typecheck --workspace @qwen-code/qwen-code-core # tsc --noEmit → pass
npx eslint packages/core/src/core/coreToolScheduler.ts \
packages/core/src/core/coreToolScheduler.test.ts --max-warnings 0 # → 0 warningsBehavioral evidence (real payload, not just assertions)I drove the actual
Before, the block is delivered under an I also replayed the new assertion ( Notes / observations (non-blocking)
Verdict: correct, well-scoped, well-tested, no regressions. 👍 LGTM. 🇨🇳 中文版本(点击展开)✅ 本地验证报告(维护者)我在本地完整构建并测试了这个 PR,行为与描述完全一致,建议合并。以下作为合并参考。 环境: macOS(darwin 24.6)· Node v22.23.1 · vitest v3.2.4 · 在 PR HEAD 执行的命令# 1. PR 里给出的 plan-mode 测试
cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts -t "plan mode"
# → 14 passed | 244 skipped
# 2. 整个测试文件(回归检查)
npx vitest run src/core/coreToolScheduler.test.ts
# → 258 passed (258)
# 3. 改动文件的类型检查 + lint
npm run typecheck --workspace @qwen-code/qwen-code-core # tsc --noEmit → 通过
npx eslint <两个改动文件> --max-warnings 0 # → 0 warning行为层面的证据(真实 payload,不只是断言)我用一个 plan mode 下的
修复前,阻断信息放在 我还把新断言( 补充观察(不影响合并)
结论: 正确、范围清晰、测试充分、无回归。👍 LGTM。 |

What this PR does
Changes the plan mode blocked tool response to use the same error format (
createErrorResponse()) as all other tool failures, instead of returning a long system reminder text as anoutputfield.Why it's needed
When a non-read-only tool is called in plan mode, the tool is correctly blocked, but the LLM receives a
{ output: "<30-line system reminder text>" }response rather than{ error: "..." }. Theoutputkey looks like a successful return to the LLM — there is noerrorkey, noErrorobject, and noerrorType. The LLM does not recognize this as a failure signal and may attempt alternative approaches to execute the blocked operation rather than understanding the restriction is plan mode.The fix uses
createErrorResponse()with a concise error message ("Tool blocked by plan mode: ...") so the LLM sees a clear{ error: "..." }function response and understands the tool was denied.Reviewer Test Plan
How to verify
Run the plan mode test suite:
Confirm the blocked response contains
{ error: "Tool blocked by plan mode: ..." }, anErrorobject in theerrorfield, anderrorType: ToolErrorType.EXECUTION_DENIED.Evidence (Before & After)
Before:
responsePartscontained{ output: "<system-reminder>Plan mode is active...Iterative Planning Workflow..." }— ~30 lines of duplicate system prompt text.After:
responsePartscontains{ error: "Tool blocked by plan mode: \"write_file\" is not a read-only tool. Only read-only tools (read_file, grep_search, glob, list_directory, web_fetch, etc.) are allowed in plan mode. Call exit_plan_mode to exit plan mode and execute this tool." }— single clear error message.Tested on
Environment (optional)
N/A — unit test only.
Risk & Scope
getPlanModeSystemReminder()from the blocked response path. SDK and subagent callers previously received this text; they now get the same concise error message. The system reminder is already in the system prompt, so repeating it in the tool response is redundant.Linked Issues
N/A
中文说明
修复 plan mode 下工具被阻断时响应格式不一致的问题。
问题原理:非只读工具在 plan mode 下被正确阻断,但 LLM 收到的响应是
{ output: "<30行 system reminder 文本>" }而非{ error: "..." }。outputkey 对 LLM 来说看起来像成功返回,没有errorkey、没有Error对象、没有errorType,导致 LLM 无法识别这是一个失败信号,可能会尝试换其他方式绕过阻断。修复方案:统一使用
createErrorResponse()返回{ error: "Tool blocked by plan mode: ..." }格式,让 LLM 明确知道工具被 plan mode 拒绝。测试:
packages/core下 14 个 plan mode 相关测试全部通过,响应现在包含errorkey +Error对象 +EXECUTION_DENIED类型。