fix(cli): make plan follow-up refinement deterministic - #11808
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Both previously flagged issues are resolved as of
Files Reviewed (1 file changed in this update)
Previous Review Summaries (2 snapshots, latest commit febdc26)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit febdc26)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit ce09eb3)Status: 1 Issue Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
Files Reviewed (28 files)
Reviewed by claude-sonnet-5-20260630 · Input: 22 · Output: 4.3K · Cached: 450.5K Review guidance: REVIEW.md from base branch |
|
Addressed the ACP review concern in febdc26. The plan reminder now reuses the same client-support gate as Added a regression test covering both supported-client and ACP reminder text. |
| - If the user chooses "Finalize and save the plan", write the complete finalized Markdown plan to the chosen plan file, then call `plan_exit` as described above. | ||
| - If the user chooses "Continue refining", keep planning and do not write the final plan or call `plan_exit`. | ||
| - After `plan_exit`, rely on the client follow-up to ask whether the user wants to implement the saved plan in a new session. | ||
| - If the plan is implementation-ready, write the complete finalized Markdown plan to the chosen plan file, then call `plan_exit` as described above. Do not ask the user to choose between finalizing and refining in chat; the client follow-up after `plan_exit` asks whether to implement the saved plan or keep refining. |
There was a problem hiding this comment.
Thanks for the change. It is a great improvement. Just one comment on this part, I worry this part may cause a contradiction in regards to the branch created in code about supporting follow up vs not. We could potentially leave this part open and referencing the instructions that will be added conditionally?
e.g.
- If the plan is implementation-ready, write the complete finalized Markdown plan to the
chosen plan file, then call `plan_exit` as described above.
- Call `plan_exit` only when the goal, constraints, ... are addressed or out of scope.
- Follow the latest Plan File reminder for whether to confirm with the user before
finalizing, and for what happens after `plan_exit`.```
There was a problem hiding this comment.
Good catch, addressed in 82e58b2. The static native plan prompt no longer hardcodes the follow-up assumption — the finalize/refine and post-plan_exit behavior now defers to the dynamic "Plan File" reminder ("Follow the latest Plan File reminder for whether to confirm with the user before finalizing, and for what happens after plan_exit"), matching the existing "Follow the latest Plan File reminder for..." pattern in the same file.
This makes prompt.ts the single source of truth, so supported clients (cli/vscode/jetbrains) get the deterministic follow-up wording while ACP keeps the in-chat approval step, with no contradiction between the two reminders. Verified the dynamic reminder is always appended after the static prompt on the same user message, so the deferral target is always present.
fix(cli): make plan follow-up refinement deterministic
What
plan_exitfollow-up panel.Why
PR #11170 (commit
38459184f2,feat(cli): align native plan with architect) changed the native Plan prompt to tell the model to ask the user to choose between "Finalize and save the plan" and "Continue refining". That choice became prompt/text-driven rather than tool-driven, so faster/weaker models can simply print numbered choices in chat instead of showing the interactive ask panel.How it used to show
The existing
plan_exitfollow-up is engine-driven and already works across CLI, VS Code, and JetBrains. Routing the refinement decision through that panel makes the UX reliable for models that can callplan_exit, instead of relying on the model to choose the structuredquestiontool.Validation
bun test ./test/kilocode/plan-followup.test.ts ./test/kilocode/plan-exit-detection.test.tsfrompackages/opencodebun test ./tests/unit/plan-followup-locale-keys.test.ts ./tests/unit/question-dock-utils.test.ts ./tests/unit/followup-session.test.tsfrompackages/kilo-vscodebun run typecheckfrompackages/opencodebun run typecheckfrompackages/kilo-vscodebun run lintfrompackages/kilo-vscodebun run typecheckfrompackages/kilo-i18nbun run typecheckfrompackages/kilo-telemetry./gradlew typecheckfrompackages/kilo-jetbrainsbun turbo typecheck