Commit 107ce63
authored
🤖 fix: enforce ask_user_question in Plan Mode (#1169)
This tightens Plan Mode guidance so agents:
- use `ask_user_question` for unresolved questions (instead of an “Open
Questions” section / inline questions)
- avoid duplicating plan content and plan file paths after calling
`propose_plan` (the Plan UI already renders both)
Validation:
- `make static-check`
---
<details>
<summary>📋 Implementation Plan</summary>
# Reinforce `ask_user_question` usage when the agent has open questions
(Plan Mode)
## Goal
Ensure that in **Plan Mode**, if the model has unanswered questions that
materially affect the plan, it **uses the `ask_user_question` tool**
(instead of outputting an “Open questions” section or asking inline in
chat).
## Recommended approach (A): tighten the Plan Mode system instruction
(primary)
**Why here:** `getPlanModeInstruction()` is injected into the system
message for *every* Plan Mode turn, so it’s the most reliable place to
enforce behavior across models.
**Change**: update `src/common/utils/ui/modeUtils.ts` →
`getPlanModeInstruction()` to add an explicit “no open questions in the
plan; use tool” rule.
**Proposed wording to insert** (exact text; adjust line wraps as
needed):
```text
If you need clarification from the user before you can finalize the plan, you MUST use the ask_user_question tool.
- Do not ask questions in a normal chat message.
- Do not include an "Open Questions" section in the plan.
- Ask up to 4 questions at a time (each with 2–4 options; "Other" is always available for free-form input).
- After you receive answers, update the plan file and only then call propose_plan.
- After calling propose_plan, do not repeat/paste the plan contents in chat; the UI already renders the full plan.
- After calling propose_plan, do not say “the plan is ready at <path>” or otherwise mention the plan file location; it’s already shown in the Plan UI.
```
**Net LoC (product code)**: +8 to +15 (string expansion only).
## Reinforcement (B): strengthen the tool description (secondary)
**Why:** tool descriptions are a strong hint for many models, and this
also helps when users add `Tool: ask_user_question` overrides (they’ll
see a better base description).
**Change**: update `src/common/utils/tools/toolDefinitions.ts` →
`ask_user_question.description`.
**Proposed wording tweak** (replace “should be used…” with a requirement
+ anti-pattern callout):
```text
This tool is intended for plan mode and MUST be used when you need user clarification to complete the plan.
Do not output a list of open questions; ask them via this tool instead.
```
**Net LoC (product code)**: ~0 to +3 (string edit).
## Tests
Add small unit tests so this behavior doesn’t regress during refactors.
1. `src/common/utils/ui/modeUtils.test.ts`
- Assert that `getPlanModeInstruction("/tmp/plan.md", false)` contains
`ask_user_question` and `MUST`.
- Assert that it includes the “don’t repeat/paste plan contents” +
“don’t mention plan file location” guidance (to prevent UI clutter).
2. `src/common/utils/tools/toolDefinitions.test.ts`
- Import the tool definitions map and assert
`ask_user_question.description` contains the “MUST be used …” phrasing.
**Net LoC (product code)**: 0 (tests only).
## Optional docs follow-up (not required for behavior)
If we want the user-facing docs to match the new behavior:
- `docs/plan-mode.mdx`: change “may call `ask_user_question` …” →
“should/must call … when it needs clarification before finalizing a
plan.”
**Net LoC (product code)**: 0 (docs only).
## Validation checklist (Exec Mode)
- Run `make typecheck`.
- Run targeted unit tests:
- `bun test src/common/utils/ui/modeUtils.test.ts`
- `bun test src/common/utils/tools/toolDefinitions.test.ts`
- Manual sanity check in mux:
- Enter Plan Mode and ask for a plan on an underspecified task.
- Confirm the agent calls `ask_user_question` instead of emitting an
“Open questions” section.
---
<details>
<summary>Alternative considered: backend enforcement / linting</summary>
**Idea:** detect “Open Questions” patterns in the assistant output and
block `propose_plan` unless `ask_user_question` was called.
**Why not (for now):**
- Hard to do safely without false positives/negatives.
- The backend can’t reliably synthesize the multiple-choice options the
tool requires.
This can be revisited if prompt-only enforcement proves insufficient.
**Net LoC (product code)**: ~50–150.
</details>
</details>
---
_Generated with `mux` • Model: openai:gpt-5.2 • Thinking: xhigh_
<!-- mux-attribution: model=openai:gpt-5.2 thinking=xhigh -->1 parent 9de8d23 commit 107ce63
File tree
4 files changed
+43
-2
lines changed- src/common/utils
- tools
- ui
4 files changed
+43
-2
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
228 | 228 | | |
229 | 229 | | |
230 | 230 | | |
231 | | - | |
| 231 | + | |
| 232 | + | |
232 | 233 | | |
233 | 234 | | |
234 | 235 | | |
235 | 236 | | |
236 | 237 | | |
237 | 238 | | |
238 | 239 | | |
239 | | - | |
| 240 | + | |
| 241 | + | |
240 | 242 | | |
241 | 243 | | |
242 | 244 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
20 | 28 | | |
21 | 29 | | |
22 | 30 | | |
| |||
0 commit comments