feat(plan): simplify and harden plan mode#275
Merged
Conversation
Make `submit_plan` the required terminal action for every Plan-mode turn. Read-only tools gather context, the tool provides structured state, and the harness renders the resulting Markdown. Remove prose parsing, special tags, and model-authored plan templates. --- Changes - Replace `PLAN_RESPONSE_TEMPLATE` and `PLAN_GENERATION_INSTRUCTION` with instructions to: - research discoverable facts before planning; - use `submit_plan` exactly once as a standalone call; - break executable work into ordered, verifiable tasks; - choose `ready`, `needs_input`, or `answer`; - never write the final plan as prose or Markdown. - Add the Plan-only `submit_plan` control tool with: - `kind`, `title`, and `summary`; - ordered tasks with stable IDs, descriptions, dependencies, and verification criteria; - test cases, assumptions, and user questions. - Validate submissions: - `ready` requires tasks with unique IDs and valid dependencies. - `needs_input` requires questions. - `answer` must not contain executable tasks. - Require tool submission as the terminal path. Retry once after prose or invalid arguments, then show a recoverable Plan-mode error. - Accept submission only as a standalone tool call. Complete any batched read calls, discard the premature submission, and request a replacement based on the results. - Render validated payloads with application-owned Markdown: - Ready: `## Proposed Plan`, Summary, Tasks, Test Plan, and Assumptions. - Needs input: `## Plan Needs Input`, Questions, What I Found, and optional Draft Tasks. - Answer: ordinary Markdown without plan review. - Render each task with its ID, description, dependencies, and verification criterion. Markdown is presentation-only and is never parsed back into state. - Remove heading-based helpers and the separate plan-generation model call. - Store the normalized plan in `PendingPlan`; execute the exact approved snapshot in Safe or Auto mode. - Reserve task IDs for a later `update_plan` execution tool without adding task-status tracking yet. - Update TUI documentation to describe tool-backed planning while keeping the internal tool protocol out of normal user instructions. --- Interfaces - Add `Plan`, `PlanTask`, and `PlanKind` types, validation, and rendering helpers. - Add `submit_plan` to Plan-mode tool definitions without classifying it as a filesystem read or write. - Change `PendingPlan` to hold the normalized plan and derived display Markdown. - Keep the existing Auto, Safe, and Continue planning actions. --- Test Plan - Test all three submission kinds and their rendered Markdown. - Reject missing questions, empty ready plans, duplicate IDs, invalid dependencies, executable answer payloads, and malformed arguments. - Verify research continues before submission and premature batched submissions are replaced. - Verify missing submissions receive one correction followed by a recoverable error. - Verify only ready plans open Plan Review. - Verify the renderer—not model output—controls plan headings and task formatting. - Verify Safe and Auto receive the exact approved snapshot. - Update prompt, plan-flow, renderer, review, and session-continuation tests.
Error: ``` Plan submission was not accepted: questions must be an array Error: Plan mode requires a valid standalone submit_plan tool call. ``` `gemma4:latest` called `submit_plan` without the `questions` array. The harness retried once, but the retry didn’t produce a valid tool call. This confirms a harness strictness issue. We should normalize optional arrays to `[]`: - `questions`, `tests`, and `assumptions` → default to `[]` - task `dependencies` → default to `[]` - Continue requiring questions only when `kind === "needs_input"` - Continue requiring tasks when `kind === "ready"` That would allow an informational `answer` submission like this one without weakening meaningful validation. Fix: Omitted optional arrays now default to `[]`, including task dependencies. Semantic validation remains strict for `ready` tasks and `needs_input` questions.
``` Error: Plan mode requires a valid standalone submit_plan tool call. ``` The harness now: - Retries with only `submit_plan` exposed. - Uses a short finalization-only prompt. - If the model still returns no tool call, requests schema-constrained JSON. - Validates that JSON through the same typed plan parser. - Never parses model-authored Markdown.
Bug: stopped execution with the approved task. The approved execution ran only `read_file`, then `gemma4` returned a thinking-only response that the harness interpreted as empty. Fixed by: - Setting `think: false` for normal and structured Ollama calls. - Continuing after an empty post-tool response. - Showing an explicit bounded error instead of silently stopping.
Error: Plan mode could not accept submit_plan: needs_input plans require at least one The harness now: - Explicitly tells the model that `needs_input` requires a non-empty `questions` array. - Feeds semantic validation errors back to structured generation. - Allows one corrected structured response before failing. - Reports the latest validation error if correction still fails.
Bug: the model response stopped because it was accepted as `kind: "answer"` instead of `needs_input`. Fixed at the harness level. - Explicit implementation requests can no longer terminate as `answer`. - Invalid `answer` submissions are corrected into `ready` or `needs_input`. - Informational requests can still use `answer`. - Prompts and tool descriptions reinforce the distinction. - Added regression coverage for the reported session pattern.
After approving a plan, an empty initial execution response now: - Receives a bounded retry instructing the model to execute the required tool. - Continues normally if the retry produces a tool call. - Shows an explicit error after repeated empty responses instead of silently stopping.
Bug: progress retries and uncalled-tool-intent retries currently share one counter, and a prose-only promise can terminate Auto mode after that counter is exhausted. Fix: - Empty responses and uncalled-tool promises now have separate retry budgets. - Phrases such as “I will proceed with editing” and “I will use the `edit_file` tool” are detected as uncalled tool intent. - Repeated tool promises now produce an explicit error instead of silently terminating.
Plans now render as: 1. **Read the current prompt** - Dependencies: None 2. **Condense PLAN_INSTRUCTION** - Dependencies: Step 1 Stable `task-1` IDs remain in structured state and approved snapshots but are hidden from the presentation.
Implemented selectable Plan clarifications. Key behavior: - `needs_input` supports one structured question with 2–4 optional choices. - Legacy string questions are normalized automatically. - Selecting an answer immediately resumes Plan mode. - “Type a custom response” or Escape returns to normal text input. - Clarification state is separate from Plan Review and tool approval.
…oices Options are optional, so the harness behaved as implemented. However, the model expressed a bounded choice in prose instead of structured options, defeating the selector feature. The right fix is stronger harness/schema enforcement: when a `needs_input` question offers explicit alternatives, require structured options or retry the submission. Prompt wording alone won’t be reliable. The harness now rejects `needs_input` questions that embed choices in prose without structured options, including: - “Select one” or “Choose one” - Multi-item `e.g.`, “for example,” or “such as” lists - “Which X or Y?” questions - Embedded bullet or numbered choices The existing correction flow then asks the model to resubmit structured options, which opens the selector.
- Plan validation now requires 2–4 selectable options when the user asks for suggestions. - Invalid submissions enter the existing correction flow. - Structured recovery failures now preserve the actual error instead of showing a generic `submit_plan` error.
- `submit_plan` now requires `tasks`, `tests`, `assumptions`, and `questions`. - Prompts require empty arrays when fields don’t apply. - `ready` and `answer` plans reject unexpected questions. - Structured recovery test now covers `needs_input` with an empty `questions` array.
- Normalizes Markdown before checking tool intent. - Recognizes I’ll, “start by reading,” and “try reading” variants. - Added the exact session phrasing as regression coverage.
Fixed all three failure modes: - `edit_file` now accepts a single MCP-style `edits` entry while preserving Safe-mode approval. Multiple replacements are rejected clearly. - Printed pseudo-tool calls are detected, summarized, and corrected—not executed or treated as completion. - Streaming updates are throttled to 50 ms, and serialized tool payloads stop incremental rendering to reduce flicker.
- Detects “perform/make/carry out/execute the edit/change/update” as uncalled tool intent. - Failed state-changing tools now instruct the model to either call a corrected tool or explicitly report why completion is impossible. - Added the exact session phrase and recovery-flow regressions.
Implemented required Plan verification. - Ready plans must include exact shell commands selected from `AGENTS.md` or project configuration. - Prose such as “Run the tests” is rejected. - Every approved command must succeed after the latest mutation. - A later edit resets the entire verification checklist. - Reading the file no longer counts as correctness verification. - Verification state survives Safe-mode approval boundaries. - The model may stop only by explicitly reporting incomplete work and why.
``` Error: Plan mode could not accept submit_plan: title must be a non-empty string ``` Implemented schema hardening. - Added `minLength: 1` to titles, summaries, task fields, question prompts, options, dependencies, tests, and assumptions. - Normal tool calls and structured recovery now receive the same constraints. - Semantic parsing still rejects whitespace-only values.
``` Error: Plan mode could not accept submit_plan: answer submissions cannot satisfy an implementation request; use ready or needs_input ``` Implemented request-aware Plan schemas. - Plan/change requests now restrict `submit_plan.kind` to `ready` or `needs_input`. - Informational requests still permit `answer`. - The restriction applies to direct submission and structured recovery. - Existing semantic validation remains as a fallback.
…tion backstop Fixed both failure paths. - `ready` plans containing placeholders or unresolved details are rejected and corrected to `needs_input`. - Approved plans now track whether a project mutation succeeded. - If execution stops before changing anything, the model gets one correction; a repeated stop produces an explicit error instead of silently ending. - Plan prompts now forbid deferred or placeholder implementation details.
``` Error: The model stopped before making any changes from the approved plan. ``` - MCP tool annotations are retained during discovery. - Tools with `readOnlyHint: true` remain read-only. - MCP tools without that annotation are conservatively treated as potentially mutating. - Successful MCP mutations now satisfy approved-plan execution and trigger planned verification. - The incorrect “stopped before making changes” error no longer appears after a successful MCP edit.
After the first recovery response selects a valid `kind`, the retry schema now enforces that outcome: - `answer`: all arrays must be empty. - `ready`: tasks/tests required; questions empty. - `needs_input`: exactly one question. - A kind excluded by the request-aware base schema is not re-enabled. The original `answer` plus tasks failure now recovers successfully on the second structured attempt.
Renamed the Plan discriminator cleanly: - `PlanKind` → `PlanOutcome` - `plan.kind` → `plan.outcome` - `submit_plan.kind` → `submit_plan.outcome` - Updated schemas, prompts, parsing, validation, rendering, recovery specialization, snapshots, and tests.
Add explicit failed-mutation state to the execution loop instead of relying on prose-intent detection. After a state-changing tool fails: - Set `failedMutationPending: true`. - Keep it set while the model performs read-only research. - Clear it only after a corrected mutation succeeds or the model explicitly reports a blocker. - If the model returns ordinary prose while it remains pending, prompt it to call a corrected tool. - After the retry limit, show: > The model stopped after a failed state change without retrying or reporting a blocker. This check should run before the approved-plan “no changes” backstop, producing the accurate error. Also improve the `edit_file` description to require a unique `oldText` block and instruct the model to reread and expand the match when it is ambiguous. The harness should not automatically guess which `}` to edit.
- Plan tasks now declare `action: inspect | change | verify`. - `change` tasks require concrete `targets`. - Informational requests reject executable `ready` plans and require `answer`. - Read-only plans no longer incorrectly require project mutations or test commands. - Auto mode derives `mutationRequired` from actual change tasks. - Recovery prompts name the pending change and target. - Approved-plan recovery now allows two corrective attempts.
- Plan recovery now verifies it still owns the active turn before committing. - Aborted structured responses cannot overwrite the interruption state. - Session changes and component cleanup abort active model calls. - Stale recovery callbacks no longer clear loading state for newer turns. - Added a regression test where structured recovery finishes after interruption; partial Plan prose is not persisted.
Plan mode now distinguishes between: - An incomplete research response: keeps read-only tools available and tells the model to either continue researching or call `submit_plan`. - An invalid/batched `submit_plan`: restricts the retry to `submit_plan` only. - A repeated missing submission: proceeds to structured recovery as before. Added an end-to-end test covering prose → additional `read_file` research → informational `submit_plan`.
- Tool constant and definition - Dispatcher handling - Plan prompts and correction messages - Structured recovery logic - Exported helper names - Errors and validation messages
…tion
- Failed `edit_file` calls now receive the exact required `{path, oldText, newText}` signature, prohibit
edits arrays, and recommend a focused reread.
- Approved plans can finish without mutation when all planned targets were inspected and the model
explicitly confirms the behavior already exists.
- After a failed mutation, every target must be reread before that no-op completion is accepted.
- Plan prompts now prohibit change tasks whose outcome already exists.
- Change plans now require a real lint, type-check, build, or test command; `grep` alone is rejected. - Failed planned checks are tracked and can be replaced by a successful recognized project check. - Arbitrary commands like `echo done` cannot satisfy verification. - Failed `run_shell` checks are no longer mislabeled as failed state changes. - Recovery explicitly requests exactly one `run_shell` tool call with no prose. - Tool schema and Plan-mode prompts reflect the stronger requirements.
Projects without lint/build/test scripts can now use deterministic alternatives such as: - `grep -q expected file` - `node --check file.js` - `./scripts/smoke.sh` - Piped validation commands Non-evidentiary commands like `echo done`, `pwd`, and plain `ls` remain rejected.
- Track completion for every planned mutation target - Preserve failed checks across repair edits - Allow tool-based recovery after verification failures - Require explicit resolution for unchanged targets - Add regression coverage for incomplete multi-file plans
- Ordinary answers and clarification questions now complete normally. - `finish_plan_mode` is optional and only submits ready plans. - Removed `outcome`, `questions`, structured recovery, retry loops, and the separate clarification UI. - Invalid proposals produce one visible error and return control. - Read-only research still supports multiple tool turns. - Approved plans retain execution and verification safeguards. - Removed about 4,000 lines of obsolete code and tests.
- Share streaming and incomplete-response handling across modes - Retry empty and uncalled-tool responses once - Validate plan proposals independently of latest-message wording - Prevent announced `finish_plan_mode` calls from ending the turn - Add regression coverage for plan continuation failures
- Validate terminal tool arguments through a shared result boundary - Retry malformed `finish_plan_mode` calls once with the validation error - Stop cleanly after repeated invalid or missing corrected calls - Add regression coverage for terminal plan recovery
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
- Describe Plan mode as conversational and read-only - Clarify that only ready plans enter review - Document approved-plan verification requirements
- Simplify ambiguous tool-name matching - Preserve built-in and MCP tool naming formats - Add regression coverage for names with single and double underscores
The previous workflow hid the cause because `bash -e` exited during command substitution before printing stdout, while stderr wasn’t captured. Both failures were for the same commit—one push run and one PR run. The CLI exited with status `1` after roughly two minutes, so it was not the 300-second timeout. The revised step now: - streams stdout and stderr into the Actions log - preserves the real exit status through `tee` - reports the status as an Actions error - retains the non-empty and `Error:` checks
- Retries one empty one-off CLI response. - Adds a corrective system message on retry. - Exits nonzero with a clear error after repeated empty responses. - CI now rejects whitespace-only output with an explicit annotation.
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
2 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.
What is the motivation for this pull request?
Plan mode was too dependent on model-formatted prose and brittle recovery paths. Invalid terminal calls, empty responses, failed edits, and incomplete approved-plan execution could stop silently or leave the UI in an inconsistent state.
This change makes Plan mode easier to understand and more resilient while keeping the harness responsible for validation, execution boundaries, and verification.
plan.md
What is the current behavior?
What is the new behavior?
finish_plan_modeis reserved for ready executable plans.Verification
npm run lint:fixnpm run buildnpm run lint:tscnpm run lint:packageThe full Chat/test runs can still encounter an existing intermittent multi-tool or Ink worker timeout; the affected focused tests pass independently.
Checklist: