fix(core): exit_plan_mode returns guidance error from execute() instead of permission deny#7673
Merged
Conversation
qwen-code-dev-bot
requested review from
LaZzyMan,
tanzhenxin,
wenshao and
yiliang114
as code owners
July 24, 2026 13:18
github-actions
Bot
added
the
review/self-reported
Collaborator
Collaborator
|
Thanks for the PR! Template: the body doesn't follow the exact template headings (missing "Reviewer Test Plan", "Risk & Scope", "Linked Issues" as separate sections), but the content is substantive and covers the intent — not blocking on this. Problem: observed bug with a linked issue (#7671). When the user manually switches out of plan mode and the model subsequently calls Direction: aligned — improving tool deny messages is squarely within the permission system's responsibility, and giving the model actionable context on denial is a clear UX win. CHANGELOG: no direct reference, but the area (tool permission flow) is core infrastructure. Size: 23 production lines (5 in permissionFlow.ts, 9 in exitPlanMode.ts, 9 in tools.ts) + 19 test lines. Well under any threshold. Not applicable. Approach: the scope feels right — add an optional Moving on to code review. 🔍 中文说明感谢贡献！ 模板：PR body 没有完全按照模板标题（缺少 "Reviewer Test Plan"、"Risk & Scope"、"Linked Issues" 独立段落），但内容充实，覆盖了模板意图——不因此阻塞。 问题：已观测到的 bug，关联 issue #7671。用户手动切换出 plan mode 后，模型再调用 方向：对齐——改进工具拒绝消息属于权限系统的职责，给模型提供可操作的上下文是明确的 UX 提升。 规模：23 行生产代码 + 19 行测试代码，远低于任何阈值。不适用。 方案：范围合理——在工具接口添加可选的 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code ReviewIndependent proposal: I'd add an optional method to the Critical: build failure — The PR adds Fix options (pick one):
Everything else in the diff is clean: the optional chaining in CI Test Evidence
The ubuntu failure is caused by this PR — the TS4113 error above breaks the build during Not verified: unit tests did not run (build failed first). The author reports 27 permissionFlow + 28 exitPlanMode tests passing locally — this is the author's claim, not verified evidence. 中文说明代码审查独立方案： 我会在 Critical：构建失败—— PR 在 修复方案（二选一）：
其余改动干净： CI 测试证据ubuntu 测试失败，由本 PR 引起——TS4113 错误在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 2/5 — right idea, minimal scope, but the build is broken. The design here is genuinely good — an optional But it doesn't compile. The Requesting changes for the build failure. Everything else is clean. 中文说明置信度：2/5 — 思路正确，范围精简，但构建失败。 设计本身很好——在工具接口上添加可选的 但代码无法编译。 因构建失败请求修改。其余部分干净。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
requested changes
Jul 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Build is broken: override getDenyMessage() in exitPlanMode.ts fails with TS4113 because BaseToolInvocation doesn't declare the method. Drop the override keyword or add the method to the base class. Everything else looks good — should be a quick fix. See my detailed notes above. 🙏
qwen-code-dev-bot
force-pushed
the
fix/plan-mode-exit-7671
branch
2 times, most recently
from
July 24, 2026 13:34
f47b6a7 to
922fae3
Compare
qwen-code-dev-bot
changed the title
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. |
qwen-code-dev-bot
force-pushed
the
fix/plan-mode-exit-7671
branch
from
July 24, 2026 14:04
922fae3 to
fd48f34
Compare
qwen-code-ci-bot
previously approved these changes
Jul 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
When the model calls exit_plan_mode outside plan mode (e.g. after the user manually switched modes via Shift+Tab), it previously got a generic permission deny error that gave zero guidance. Changed getDefaultPermission() to return 'allow' instead of 'deny' when not in plan mode — this is a state issue, not a security issue, so the permission layer should not block it. The execute() method now checks approvalMode !== PLAN (with no approval snapshot) and returns a context-aware errorResult telling the model what happened and what to do instead. This also fixes the YOLO mode issue where a permission deny error was semantically wrong.
qwen-code-dev-bot
force-pushed
the
fix/plan-mode-exit-7671
branch
from
July 24, 2026 14:56
fd48f34 to
2d9f6de
Compare
doudouOUC
reviewed
Jul 24, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Review Feedback — No Action Needed@qwen-code-ci-bot [CHANGES_REQUESTED]:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action requiredThe automated review found no blockers. The single Suggestion-level finding could not be anchored to a changed line and was dropped by the reviewer. There are no inline comments, no issue-level comments, no failed checks, and no still-red checks to address. No changes were made to the PR. 中文说明无需操作自动审查未发现阻塞问题。唯一的建议级别发现无法锚定到已更改的代码行，已被审查工具丢弃。没有行内评论、没有 Issue 级别的评论、没有失败的检查、也没有持续未通过的检查需要处理。 本次未对 PR 做任何更改。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
requested changes
Jul 24, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
The normal DEFAULT/AUTO/AUTO_EDIT/YOLO path is fixed, and the explicit Plan approval gate remains sound. However, the outside-Plan guidance is still bypassed by a supported permission path, so this is not ready to merge yet.
Additionally, this PR currently says Fixes #7671 while explicitly leaving Problem 1 unresolved: manually leaving Plan mode via Shift+Tab or /approval-mode still does not emit a one-shot reminder to the model. Please either implement that half or change the closing reference to a partial link and create/link a follow-up issue.
Please also add scheduler-level coverage for an outside-Plan permissions.ask rule and the Plan-to-non-Plan timing boundary. The direct execute() unit test cannot exercise the confirmation path where the defect occurs. The failure contract in docs/design/explicit-plan-exit-approval.md should be updated as well, since it still says outside-Plan calls are denied before confirmation construction.
Collaborator
Proposed minimal fixI recommend keeping the current state-error approach, but completing it at both entry points. There is no need to add a new generic permission-system abstraction for this one tool. Required semantics
Minimal implementation
For Regression coveragePlease test through the real
The Core and ACP schedulers share the permission/confirmation boundary, so the Core scheduler tests cover the defect itself; add one narrow ACP regression only if ACP has adapter-specific error conversion that could lose the guidance. Issue and documentation scopeThis PR fixes Problem 2 from #7671 but explicitly leaves Problem 1—the missing one-shot reminder after a manual Shift+Tab or Please also update |
…7671) The guidance error for outside-Plan exit_plan_mode calls was only reachable through execute(). A PM ask rule or a Plan-to-non-Plan mode switch between permission evaluation and confirmation construction routes through getConfirmationDetails() instead, which still threw a generic error. Extract outsidePlanGuidanceMessage() and reuse it in both getConfirmationDetails() (throw) and execute() (errorResult) so the model receives actionable guidance on every reachable path. Add scheduler-level regression tests for both bypass paths and update the design doc failure-behavior section.
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[rc:3646546685]
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/10 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[rc:3647037897]
|
qwen-code-ci-bot
approved these changes
Jul 24, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
doudouOUC
approved these changes
Jul 24, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
enabled auto-merge
July 24, 2026 19:12
Collaborator
Local verification report — PR #7673Verified as maintainer against head Verdict: the fix works, no regressions found — merge-ready. One nit below, non-blocking.
1. The reported bug reproduces on base and is fixed by this PRScenario S1 is the exact flow from #7671: start
Wire-level oracle (the
2. YOLO mode: the deny was semantically wrong, and now it isn't
3. The round-1 finding (
|
| scenario | base | PR |
|---|---|---|
PLAN + permissions.allow: ["exit_plan_mode"] |
dialog shown | dialog shown |
| PLAN + approve | User approved. You can now start coding… |
identical |
| PLAN + cancel (Esc) | Plan was rejected. Remaining in plan mode., footer still plan mode |
identical |
YOLO + permissions.deny |
…Matching deny rule: "exit_plan_mode". |
identical |
Explicit PM denies stay authoritative, and the concurrent-exit case still falls through to the stale-revision check (the !this.approval guard) — covered by the existing allows only the first of two concurrent approved exits test, which still passes.
5. Tests are load-bearing, not decorative
Reverting exitPlanMode.ts to base source while keeping the PR's test files: 5 failures (3 in exitPlanMode.test.ts, 2 in coreToolScheduler.test.ts). Reverting each hunk in isolation:
| mutation | PR tests failing |
|---|---|
M1 requiresUserInteraction() early-return → base |
2 |
M2 getDefaultPermission() → base ('ask'/'deny') |
3 |
M3 getConfirmationDetails() throw → plain Error |
3 |
M4 delete the execute() outside-plan guard |
1 |
M5 errorResult() drops type from the error payload |
1 |
No surviving mutants — every source change has a test that dies without it.
Green at head: exitPlanMode.test.ts 29 · coreToolScheduler.test.ts 304 · permissionFlow.test.ts 26; plan-related sweep across src/tools/src/core/src/agents 129 more. tsc --noEmit, eslint, and prettier --check all clean on the four changed files.
Nit (non-blocking): the plan-required-teammate branch is now shadowed
The new execute() guard sits above the isPlanRequiredTeammateContext() branch, so exitPlanMode.ts:284-286 ('Not in plan mode — no action taken.') is no longer reachable — this.approval is only ever set by the non-teammate confirmation flow. Probed with a real invocation under runWithTeammateIdentity({planModeRequired: true}) outside plan mode:
base: Not in plan mode — no action taken.
PR: You are not in plan mode (current mode: default). The user may have
manually switched modes via Shift+Tab or /approval-mode. …
Both are errors that take no action, so behavior is equivalent — but a teammate agent is now told "the user may have switched modes via Shift+Tab", which isn't what happened in that context, and lines 284-286 are dead. Fine to leave for a follow-up; worth a comment or a !isPlanRequiredTeammateContext() term on the guard if you want it tidy.
Also flagging for the record: this PR fixes problem 2 of #7671 only. Problem 1 (notifying the model on manual mode switch) is #7682, which I verified separately — they are independent and don't conflict.
Reproduction harness
- Isolated
HOMEwith pre-seeded~/.qwen/settings.json(theme,security.auth.selectedType: openai, auto-update off) to skip onboarding;OPENAI_BASE_URLpointed at a local mock. - Mock server logs every
/v1/chat/completionsbody to JSONL and replies with SSE; main-agent requests are identified by theYou are Qwen Codesystem-prompt prefix so the managed-memory-extraction subagent never consumes a scripted turn. - CLI driven via
@lydell/node-pty+@xterm/headless; frames captured cell-by-cell (fg/bg/bold/dim) and rendered to the screenshots above via Playwright — the terminal output in every image is real captured output, not a mock-up. - Base arm produced by overlaying merge-base
exitPlanMode.tsinto the same worktree and rebuilding onlypackages/core(the built CLI istscoutput resolving@qwen-code/qwen-code-core → packages/core/dist), so the two arms differ by exactly this PR's source change.
中文版本 · Chinese version
本地验证报告 —— PR #7673
以维护者身份对 head 59df4bf1（merge-base cc742ca3）在 macOS arm64 / Node 22.23.1 上验证。这不只是重跑 PR 自带的单测：我在真实 PTY 中驱动构建产物 CLI（packages/cli/dist/index.js），对接一个会记录每个 /chat/completions 请求体的 mock OpenAI 服务，因此判定依据是模型实际收到的字节，而不是被 mock 的 scheduler。
结论：修复有效，未发现回归 —— 可以合并。 下面有一个不阻塞的小问题。
1. 基线能复现该 bug，本 PR 修复了它
场景 S1 就是 #7671 描述的流程：以 --approval-mode plan 启动 → 发一条消息 → 按 Shift+Tab（直接向 pty 写入 ESC[Z，PLAN → DEFAULT）→ 再发一条消息 → 模型调用 exit_plan_mode。脚本与按键完全相同，两次运行只有 packages/core 不同。
线级判据（下一次请求中的 role: "tool" 消息）：
| 送达模型的文本 | |
|---|---|
base cc742ca3 |
Tool "exit_plan_mode" is denied: the tool's default permission is 'deny'. |
PR 59df4bf1 |
You are not in plan mode (current mode: default). The user may have manually switched modes via Shift+Tab or /approval-mode. Do not call exit_plan_mode again. Continue working in the current mode. |
2. YOLO 模式：原先的 permission deny 语义错误，现已纠正
current mode: yolo 被正确插值，说明该消息由状态推导得出，而非固定字符串。
3. 第 1 轮评审提出的 getConfirmationDetails() 绕过问题确已闭合
我用真实的 permissions.ask: ["exit_plan_mode"] 规则在非 plan 模式下复现了该路径。值得说明基线为何走不到那里：evaluatePermissionRules() 被 if (pm && defaultPermission !== 'deny')（permission-helpers.ts:134）保护，基线的 'deny' 默认值会直接短路掉 PM 规则，调用根本到不了 getConfirmationDetails()。本 PR 的 'allow' 默认值打开了这条路径 —— 而第 2 轮加入的 StructuredToolError 正是避免它被记为 UNHANDLED_EXCEPTION 的关键，因为 scheduler 会在 coreToolScheduler.ts:3125-3135 读取 error.errorType。两处改动都是必要的。
4. 无回归：getDefaultPermission() → 'allow' 之后 plan 审批闸门依然成立
这是本次改动最大的风险点，所以我用实跑而非阅读来验证。闸门现在完全依赖 requiresUserInteraction()，而所有自动放行路径都受它保护：
evaluatePermissionFlow()在requiresUserInteraction && finalPermission !== 'deny'时强制'ask'（permissionFlow.ts:85-88），因此 plan 模式下finalPermission不可能是'allow'，coreToolScheduler.ts:2527的 allow 快速通道不可达。needsConfirmation()在 YOLO 分支之前就对requiresUserInteraction返回true（permissionFlow.ts:131-142）。- AUTO 分类器（
:2560）、AUTO_EDIT（:2769）、allow hook（:2836）与兄弟调用自动放行（:5223）同样都检查它。
实测两个分支逐字相同：
| 场景 | base | PR |
|---|---|---|
PLAN + permissions.allow: ["exit_plan_mode"] |
弹出确认框 | 弹出确认框 |
| PLAN + 批准 | User approved. You can now start coding… |
相同 |
| PLAN + 取消（Esc） | Plan was rejected. Remaining in plan mode.，页脚仍为 plan mode |
相同 |
YOLO + permissions.deny |
…Matching deny rule: "exit_plan_mode". |
相同 |
显式 PM deny 依然具有最高优先级；并发退出场景仍会落到 stale-revision 检查（靠 !this.approval 这一条件），既有测试 allows only the first of two concurrent approved exits 仍然通过。
5. 测试确实有效，不是摆设
把 exitPlanMode.ts 换回基线源码、保留 PR 的测试文件：5 个失败（exitPlanMode.test.ts 3 个，coreToolScheduler.test.ts 2 个）。逐个 hunk 单独回退：
| 变异 | 失败的 PR 测试数 |
|---|---|
M1 requiresUserInteraction() 提前返回 → 基线 |
2 |
M2 getDefaultPermission() → 基线（'ask'/'deny'） |
3 |
M3 getConfirmationDetails() 抛出改回普通 Error |
3 |
M4 删除 execute() 中的 outside-plan 守卫 |
1 |
M5 errorResult() 丢弃 error payload 中的 type |
1 |
没有存活变异体 —— 每一处源码改动都有测试为它把关。
head 处全绿：exitPlanMode.test.ts 29 · coreToolScheduler.test.ts 304 · permissionFlow.test.ts 26；src/tools/src/core/src/agents 的 plan 相关扫描另有 129 个通过。四个改动文件的 tsc --noEmit、eslint、prettier --check 均干净。
小问题（不阻塞）：plan-required teammate 分支被遮蔽
新的 execute() 守卫位于 isPlanRequiredTeammateContext() 分支之上，因此 exitPlanMode.ts:284-286（'Not in plan mode — no action taken.'）已不可达 —— this.approval 只会由非 teammate 的确认流程设置。用真实 invocation 在 runWithTeammateIdentity({planModeRequired: true}) 且非 plan 模式下探测：
base: Not in plan mode — no action taken.
PR: You are not in plan mode (current mode: default). The user may have
manually switched modes via Shift+Tab or /approval-mode. …
两者都是「报错且不做任何动作」，行为等价；但 teammate agent 会被告知"用户可能通过 Shift+Tab 切换了模式"，在该上下文中并非事实，且 284-286 行成为死代码。留到后续处理即可；若想收拾干净，可在守卫上加一个 !isPlanRequiredTeammateContext() 条件或补一条注释。
另外备注：本 PR 只修复 #7671 的问题 2。问题 1（用户手动切换模式时通知模型）是 #7682，我已单独验证过；两者互相独立、无冲突。
复现环境
- 隔离
HOME并预置~/.qwen/settings.json（主题、security.auth.selectedType: openai、关闭自动更新）以跳过 onboarding；OPENAI_BASE_URL指向本地 mock。 - Mock 服务把每个
/v1/chat/completions请求体写入 JSONL 并以 SSE 应答；通过You are Qwen Code系统提示前缀识别主 agent 请求，避免 managed-memory 抽取子 agent 消耗脚本轮次。 - 通过
@lydell/node-pty+@xterm/headless驱动 CLI；逐单元格（前景/背景/粗体/暗色）捕获帧后用 Playwright 渲染为上方截图 —— 图中终端内容均为真实捕获输出，非手工绘制。 - 基线分支的做法：把 merge-base 的
exitPlanMode.ts覆盖进同一个 worktree，仅重建packages/core（构建产物 CLI 是tsc输出，通过@qwen-code/qwen-code-core → packages/core/dist解析），因此两个分支之间的差异恰好就是本 PR 的源码改动。
wenshao
approved these changes
Jul 25, 2026
pull Bot
pushed a commit
to Little-Star888/qwen-code
that referenced
this pull request
Jul 25, 2026
…nLM#7682) * fix(core): tell the model when the user manually exits plan mode While plan mode is active the plan-mode system reminder is re-injected on every model-bound turn, so after a manual exit (Shift+Tab, /approval-mode, /plan, ACP mode switch) the reminder just silently stops appearing — a non-signal models do not reliably notice. The model's most recent context still says plan mode is active, so it keeps calling exit_plan_mode and gets stuck (QwenLM#7671, problem 1). Config.setApprovalMode now queues a one-shot notice on every PLAN -> non-PLAN transition except the approved exit_plan_mode flow (which passes the new fromApprovedPlanExit option); re-entering plan mode clears a stale notice. GeminiClient's system-reminder assembly consumes the flag on the next turn and injects an explicit "the user has manually switched out of plan mode" reminder naming the new mode. Fixes the first half of QwenLM#7671; the deny-message half is QwenLM#7673. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(core): pin the leader-approval exit option and the exit-reminder text Adds the two review-suggested pins: a successful leader-approved exit asserts setApprovalMode receives { fromApprovedPlanExit: true } (only the regular approval site was pinned), and getManualPlanExitSystemReminder gets dedicated prompts tests covering the rendered mode name, the exit_plan_mode prohibition, and the reminder envelope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Summary
Fixes #7671.
When the model calls
exit_plan_modeoutside plan mode (e.g. after the user manually switched modes via Shift+Tab), it previously got a generic permission deny error:Tool "exit_plan_mode" is denied: the tool's default permission is 'deny'.This gave the model zero guidance and caused it to get stuck. In YOLO mode, a permission deny error was also semantically wrong — this is a state issue, not a security issue.Changes
exitPlanMode.ts— two changes:getDefaultPermission(): returns'allow'instead of'deny'when not in plan mode. No security concern, so the permission layer should not block it.execute(): added a check at the top — ifapprovalMode !== PLANand there's no approval snapshot (i.e. the tool wasn't called through the normal plan-mode confirmation flow), return anerrorResultwith guidance:The
!this.approvalguard ensures the concurrent-exit scenario (two approved exits racing) still falls through to the existing stale-revision check.exitPlanMode.test.ts— updated the "denies outside plan mode" test to verify the new behavior: permission returns'allow', execute returns the guidance error.What's NOT in this PR
Injecting a system reminder into the conversation when the user manually switches modes (the other half of #7671) requires mid-conversation context injection infrastructure and is left as a follow-up. The execute-level error alone prevents the model from getting stuck.
Tests