fix(core): strengthen exit_plan_mode descriptions to prevent empty plan parameter

[Alex-ai-future]

What this PR does

Strengthens the tool description and parameter schema for exit_plan_mode to explicitly state that the plan parameter must contain actual plan content and that empty strings will be rejected. This reduces the likelihood of the model generating empty plan values, avoiding wasted LLM retry turns.

Why it's needed

When in plan mode, the model sometimes calls exit_plan_mode with an empty string for the plan parameter (e.g., { "plan": "" }). The tool's parameter validation correctly rejects this, but this wastes a full LLM turn (tokens + latency) and occasionally triggers the retry loop detector after repeated failures.

The current parameter description does not explicitly state the non-empty constraint. JSON Schema's required only ensures the key exists — it does not prevent empty string values. Weaker or smaller models may interpret { "plan": "" } as satisfying the schema requirement.

This is a follow-up to #4853, where Finding 3 noted that weaker models struggle with the plan mode exit path.

Reviewer Test Plan

How to verify

1. Run the unit tests to ensure the new constraint tests pass:

   cd packages/core && npx vitest run src/tools/exitPlanMode.test.ts

2. Verify the tool description and parameter schema contain the non-empty constraint text by inspecting packages/core/src/tools/exitPlanMode.ts.
3. Optionally, run npm run dev, enter plan mode with /plan, and observe whether the model still generates empty plan parameters.

Evidence (Before & After)

N/A — prompt-level change, no TUI impact. The modified text is part of the model-facing tool description, not user-visible UI.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

Environment (optional)

Local development, npm run dev.

Risk & Scope

• Main risk or tradeoff: Prompt-only change with no code logic modifications. Risk is minimal — the added text clarifies an existing constraint that the validation layer already enforces.
• Not validated / out of scope: Cross-model A/B testing to quantify the reduction in empty plan occurrences. If the prompt optimization proves insufficient, deeper fixes (system prompt reinforcement, error message improvement, or schema validation restoration) may be needed as follow-ups.
• Breaking changes / migration notes: None. No API or config changes.

Linked Issues

Fixes #5177

中文说明

这个 PR 做了什么

强化了 exit_plan_mode 的工具描述和参数 schema，明确声明 plan 参数必须包含实际的计划内容，空字符串将被拒绝。这降低了模型生成空 plan 值的可能性，避免浪费 LLM 重试轮次。

为什么需要

当处于 plan mode 时，模型有时会调用 exit_plan_mode 但传入空字符串的 plan 参数（如 { "plan": "" }）。工具的参数校验能正确拒绝这种情况，但这浪费了一个完整的 LLM 回合（token + 延迟），最坏情况下会在重复失败后触发重试循环检测器。

当前的参数描述没有显式声明非空约束。JSON Schema 的 required 仅确保键存在——不阻止空字符串值。较弱或较小的模型可能认为 { "plan": "" } 满足了 schema 要求。

这是 #4853 的后续工作，该 PR 的发现 3 指出较弱的模型在 plan mode 退出路径上存在困难。

审查者测试计划

如何验证

1. 运行单元测试确保新的约束测试通过：

   cd packages/core && npx vitest run src/tools/exitPlanMode.test.ts

2. 检查 packages/core/src/tools/exitPlanMode.ts 中的工具描述和参数 schema 是否包含非空约束文本。
3. 可选地，运行 npm run dev，使用 /plan 进入 plan mode，观察模型是否仍生成空 plan 参数。

证据（修改前后）

N/A — 纯提示词修改，无 TUI 影响。修改的文本是模型面向的工具描述的一部分，非用户可见 UI。

测试平台

系统	状态
🍏 macOS	✅
🪟 Windows	N/A
🐧 Linux	N/A

环境（可选）

本地开发环境，npm run dev。

风险与范围

• 主要风险或权衡：纯提示词修改，无代码逻辑变更。风险极低——新增文本澄清了验证层已经强制执行的现有约束。
• 未验证/范围外：跨模型 A/B 测试以量化空 plan 出现次数的减少。如果提示词优化效果不理想，可能需要后续进行更深层的修复（系统提示词强化、错误消息优化或 schema 校验恢复）。
• 破坏性变更：无。无 API 或配置变更。

关联 Issue

修复 #5177

[wenshao]

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR, @Alex-ai-future!

Template looks good ✓

On direction: solid alignment. Empty plan parameters wasting LLM turns is a real problem, especially for smaller models that don't pick up implicit constraints from JSON Schema required. Strengthening the prompt text to make the non-empty constraint explicit is the right lever — low cost, directly targets the failure mode. No CHANGELOG reference needed for this kind of prompt tuning, but the area (plan mode reliability) is clearly core.

On approach: the scope is tight — two lines of prompt text plus two tests asserting they exist. This is about as minimal as it gets. One thing worth noting: JSON Schema supports minLength: 1 which would enforce the constraint structurally rather than relying on the model reading the description. The PR body explicitly mentions this was considered and deferred ("schema validation restoration" as a possible follow-up). That's a reasonable call for now — the prompt change is low-risk and may be sufficient on its own.

Moving on to code review and testing. 🔍

中文说明

感谢贡献，@Alex-ai-future！

模板完整 ✓

方向：对齐。空 plan 参数浪费 LLM 轮次是真实问题，尤其对较小的模型来说 JSON Schema 的 required 不足以传达非空约束。通过强化提示文本明确非空约束是正确的做法——低成本，直接针对失败模式。这类提示调优无需 CHANGELOG 引用，但 plan mode 可靠性显然是核心领域。

方案：范围紧凑——两行提示文本加两个断言测试。这是最精简的改动了。值得注意的一点：JSON Schema 支持 minLength: 1，可以从结构上强制执行约束而不是依赖模型读取描述。PR 正文明确提到已考虑过此方案并推迟（"schema 校验恢复"作为后续）。当前判断合理——提示改动风险低，可能已足够。

进入代码审查和测试 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Clean. The diff is exactly what the PR describes — two additions of the constraint text "empty strings will be rejected" (one in the tool description template, one in the plan parameter schema description), plus two tests that assert those strings exist. No logic changes, no side effects, no hidden complexity.

The tests use a type cast to reach parametersJsonSchema on FunctionDeclaration, which is consistent with how the existing schema test works (using expect.objectContaining / expect.stringContaining). No correctness concerns.

One minor observation (not a blocker): the constraint is enforced at two layers now — prompt text (this PR) and runtime validation (validateToolParams already rejects empty strings). Adding minLength: 1 to the JSON Schema would be a third structural layer, but as the PR notes, that's a deliberate follow-up.

Testing

Unit Tests

✓ src/tools/exitPlanMode.test.ts (29 tests) 17ms
Test Files  1 passed (1)
     Tests  29 passed (29)

All 29 tests pass, including the 2 new constraint-text assertions.

Dev Build Sanity Check

This is a prompt-only change with no TUI impact (the PR correctly marks Evidence as N/A). Ran the dev build to confirm it loads and the tool schema is well-formed:

$ npm run dev -- 'Describe exit_plan_mode tool in one sentence' --max-session-turns 1

> @qwen-code/qwen-code@0.18.1 dev
> node scripts/dev.js Describe exit_plan_mode tool in one sentence --max-session-turns 1

DEV is set to true, but the React DevTools server is not running. Start it with:

$ npx react-devtools

`exit_plan_mode` is a tool used when in plan mode to signal that your plan is
finalized and you're ready to begin coding, prompting the user to approve
exiting plan mode.

Dev build starts cleanly, tool loads, model responds using the tool definition. No errors or warnings from the schema changes.

中文说明

代码审查

干净。diff 完全符合 PR 描述——两处添加约束文本 "empty strings will be rejected"（一处在工具描述模板，一处在 plan 参数 schema 描述），加两个断言这些字符串存在的测试。无逻辑变更，无副作用，无隐藏复杂度。

测试使用类型断言来访问 FunctionDeclaration 上的 parametersJsonSchema，与现有的 schema 测试方式一致（使用 expect.objectContaining / expect.stringContaining）。无正确性问题。

一个小观察（不是阻塞项）：约束现在在两层执行——提示文本（本 PR）和运行时校验（validateToolParams 已拒绝空字符串）。在 JSON Schema 中添加 minLength: 1 将是第三层结构性约束，但如 PR 所述，这是有意作为后续工作。

测试

单元测试

29/29 测试通过，包括 2 个新增的约束文本断言。

开发构建验证

纯提示词改动，无 TUI 影响（PR 正确将 Evidence 标记为 N/A）。运行开发构建确认加载正常且工具 schema 格式正确。构建正常启动，工具加载成功，无错误或警告。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Verdict: Approve ✅

This is a tight, well-scoped PR that does exactly one thing and does it cleanly.

Stepping back: the problem is real (models — especially smaller ones — sometimes call exit_plan_mode with { "plan": "" }, burning an LLM turn before validation catches it). The fix is the right first step — making the non-empty constraint explicit in the prompt text targets the failure mode directly, at near-zero risk. The minLength: 1 schema enforcement is a reasonable follow-up that the PR body acknowledges.

The diff is minimal: two string additions in the right places, two tests protecting them. No logic changes, no scope creep, no drive-by refactors. The tests pass, the dev build runs clean.

If I had to maintain this in six months, I'd thank the author for keeping it focused.

Shipping it. 🚀

中文说明

结论：批准 ✅

这是一个紧凑、范围明确的 PR，只做了一件事且做得干净。

退一步看：问题是真实的（模型——尤其是较小的模型——有时会用 { "plan": "" } 调用 exit_plan_mode，在验证捕获之前浪费一个 LLM 轮次）。修复是正确的第一步——在提示文本中明确非空约束直接针对失败模式，风险几乎为零。minLength: 1 的 schema 强制执行是合理的后续工作，PR 正文已说明。

diff 最精简：两处正确的字符串添加，两个保护性测试。无逻辑变更，无范围蔓延，无顺手重构。测试通过，开发构建运行正常。

如果六个月后需要维护这个改动，我会感谢作者保持了聚焦。

合并吧 🚀

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

Local verification — real tests + BEFORE/AFTER A/B

Built and ran the real suites locally (tmux + vitest) and did a BEFORE→AFTER A/B that checks (a) whether the new constraint text actually reaches the model-facing schema and (b) that the PR changes no enforcement logic. Bottom line: the change is correctly wired, the premise holds, and there's zero behavioral risk. Recommend merge (with one optional, non-blocking follow-up below).

Setup

• Verified as merged onto current main: PR head 14d57e4, merge-base 72da112, origin/main bd2c2e2cb. exitPlanMode.ts is unchanged on main since the branch point and git merge-tree is clean — applies as-is.
• packages/core via vitest run (v3.2.4) + tsc --noEmit. Node v22.22.2, tmux 3.5a.
• A/B method: one harness, swapping only exitPlanMode.ts between base and PR (sha-pinned).

1. Suites & typecheck

Check	Result
exitPlanMode.test.ts (PR)	✅ 29 passed (27 pre-existing + 2 new)
tsc --noEmit (core)	✅ clean (RC=0)

2. The new tests genuinely assert the new text (control-that-should-change)

PR's 2 new tests against the base implementation — both red without the change, exactly as expected:

× should mention non-empty constraint in plan parameter description
× should mention non-empty constraint in tool description
  → expected 'The plan you came up with…' to contain 'empty strings will be rejected'
Tests  2 failed | 27 passed (29)

3. A/B — the text reaches the model, enforcement is untouched

Constructed the real ExitPlanModeTool and inspected tool.schema (the serialized FunctionDeclaration that is sent to the model — BaseDeclarativeTool.get schema() returns {description, parametersJsonSchema}):

(a) Schema reachability

	tool description	plan param description
BEFORE	❌ no constraint	❌ no constraint
AFTER	✅ - The plan parameter MUST contain your actual plan content — empty strings will be rejected	✅ …The plan should be pretty concise. Must contain your actual plan content — empty strings will be rejected.

(b) Enforcement (validateToolParams) — IDENTICAL before & after

Input	BEFORE	AFTER
{plan:""}	rejected ✋	rejected ✋
{plan:" "} (whitespace)	rejected ✋	rejected ✋
{plan:"\n\n\t"}	rejected ✋	rejected ✋
{} (missing)	rejected ✋	rejected ✋
{plan:"1. …\n2. …"} (valid)	accepted ✓	accepted ✓

(rejection message unchanged: Parameter "plan" must be a non-empty string.)

What this confirms / what it can't

• ✅ The PR's premise is true: validateToolParams already rejects empty and whitespace-only plans (via .trim()), and that rejection round-trips to the model as Error: Invalid parameters provided … (i.e. a wasted turn) — so reducing the frequency is a real win.
• ✅ The strengthened text is genuinely in the model-facing declaration (not a dead constant), and the long tool description + the plan param description are the only places this string lives (no duplicate/snapshot left stale).
• ✅ Prompt-only: enforcement, required:["plan"], and all execution paths are byte-for-byte behavior-identical. Zero logic risk.
• ⚠️ Not verifiable here (and explicitly out of scope in the PR): whether models actually emit fewer empty plans. That's a probabilistic, live multi-model A/B — not unit-testable. The change is a reasonable low-cost nudge in the right direction.

Optional follow-up (non-blocking)

The schema is JSON Schema draft-07, so the constraint could also be made machine-enforceable by adding minLength: 1 (and/or a small pattern) to the plan property, so SDKs/providers that validate args against the schema reject {plan:""} before it ever costs a turn. It's complementary, not a replacement (whitespace-only still needs the existing .trim() check), and the PR body already flags "schema validation restoration" as possible future work. Fine to land this prose nudge first.

Recommendation

Approve & merge. Correctly wired, premise verified, no behavioral change, tests green.

中文版（点击展开）

本地验证 —— 真实测试 + 改动前后 A/B 对比

用 tmux + vitest 跑了真实测试，并做了「改动前→改动后」A/B，重点验证两点：(a) 新增的约束文本是否真的进入了模型可见的 schema；(b) PR 是否没有改动任何强制校验逻辑。结论：接线正确、前提成立、零行为风险，建议合并（附一个可选、非阻塞的后续建议）。

环境

• 按合并到当前 main 的状态验证：PR head 14d57e4，merge-base 72da112，origin/main bd2c2e2cb。exitPlanMode.ts 自分支点以来未变动，git merge-tree 干净，可直接合并。
• packages/core 跑 vitest run（v3.2.4）+ tsc --noEmit。Node v22.22.2、tmux 3.5a。
• A/B 方法：同一套 harness，只替换 exitPlanMode.ts（base 版 vs PR 版，按 sha 锁定）。

1. 测试套件与类型检查

• exitPlanMode.test.ts（PR）：✅ 29 通过（27 旧 + 2 新）
• tsc --noEmit（core）：✅ 干净（RC=0）

2. 新增测试确实在断言新文本（应当变红的对照）

把 PR 的 2 个新测试跑在 base 实现 上：没有改动时都为红，符合预期：

× should mention non-empty constraint in plan parameter description
× should mention non-empty constraint in tool description
  → expected 'The plan you came up with…' to contain 'empty strings will be rejected'
Tests  2 failed | 27 passed (29)

3. A/B —— 文本进入模型、强制校验未动

构造真实的 ExitPlanModeTool，检查 tool.schema（即发送给模型的 FunctionDeclaration；BaseDeclarativeTool.get schema() 返回 {description, parametersJsonSchema}）：

(a) Schema 可达性

	工具描述	plan 参数描述
改前	❌ 无约束	❌ 无约束
改后	✅ 含 empty strings will be rejected 条目	✅ 含 …Must contain your actual plan content — empty strings will be rejected.

(b) 强制校验 (validateToolParams) —— 前后完全一致

输入	改前	改后
{plan:""}	拒绝 ✋	拒绝 ✋
{plan:" "}（空白）	拒绝 ✋	拒绝 ✋
{plan:"\n\n\t"}	拒绝 ✋	拒绝 ✋
{}（缺失）	拒绝 ✋	拒绝 ✋
{plan:"1. …\n2. …"}（有效）	接受 ✓	接受 ✓

（拒绝消息不变：Parameter "plan" must be a non-empty string.）

这验证了什么 / 验证不了什么

• ✅ PR 的前提成立：validateToolParams 本就拒绝空值和纯空白（通过 .trim()），且该拒绝会以 Error: Invalid parameters provided … 回传给模型（即浪费一个回合）——所以降低其出现频率是实打实的收益。
• ✅ 强化文本确实进入了模型可见的声明（不是死常量），且这段字符串只存在于工具描述和 plan 参数描述两处（没有遗漏的重复/快照）。
• ✅ 纯提示词：强制校验、required:["plan"]、以及所有执行路径行为逐字节一致，零逻辑风险。
• ⚠️ 此处无法验证（PR 也明确声明范围外）：模型是否真的更少产生空 plan。这是概率性的、需要真实多模型 A/B，无法单测覆盖。本改动是一个低成本、方向正确的引导。

可选后续（非阻塞）

该 schema 是 JSON Schema draft-07，因此该约束还可以机器化强制：给 plan 属性加 minLength: 1（和/或一个简单 pattern），让按 schema 校验入参的 SDK/provider 在 {plan:""} 真正消耗一个回合之前就拒绝它。它是互补而非替代（纯空白仍需现有 .trim() 检查），且 PR 描述本身已把「schema 校验恢复」列为可能的后续工作。先合入这个提示词改动没问题。

建议

通过并合并。 接线正确、前提已验证、无行为变更、测试通过。

_{Verified locally via tmux + vitest with a base-vs-PR file swap; harness was temporary and not committed.}