feat: add PlanDisplay wire type and inline rendering support - #1601
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new PlanDisplay wire event to render plan markdown inline across clients, and bumps the wire protocol to v1.7 to support it.
Changes:
- Introduces
PlanDisplayin Python wire types and updates protocol version to1.7. - Emits and renders inline plan content in CLI UIs (shell + print JSON) and web session stream.
- Adds serialization/deserialization coverage for the new wire message.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/hooks/wireTypes.ts | Adds PlanDisplayEvent and extends the WireEvent union. |
| web/src/hooks/useSessionStream.ts | Handles PlanDisplay by inserting an assistant message; bumps initialize protocol_version to 1.7. |
| tests/core/test_wire_message.py | Adds snapshot + roundtrip serde test for PlanDisplay. |
| src/kimi_cli/wire/types.py | Defines PlanDisplay model and includes it in the wire message unions/exports. |
| src/kimi_cli/wire/protocol.py | Bumps WIRE_PROTOCOL_VERSION from 1.6 to 1.7. |
| src/kimi_cli/ui/shell/visualize.py | Renders PlanDisplay as a Rich Panel in the shell UI. |
| src/kimi_cli/ui/shell/question_panel.py | Extends numeric key handling to include option 6 (supports more choices + “Other”). |
| src/kimi_cli/ui/print/visualize.py | Prints PlanDisplay as JSON in stream-json mode. |
| src/kimi_cli/tools/plan/init.py | Emits PlanDisplay before prompting; adds “Reject and Exit” handling and refactors option flow. |
| src/kimi_cli/acp/session.py | Adds PlanDisplay to the match, but currently does not forward it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,2 +1,2 @@ | |||
| WIRE_PROTOCOL_VERSION: str = "1.6" | |||
| WIRE_PROTOCOL_VERSION: str = "1.7" | |||
There was a problem hiding this comment.
Bumping the wire protocol to 1.7 will break tests/docs that assert 1.6. For example, tests_e2e/test_wire_protocol.py:39/44 and :131 hardcode "1.6" and will fail once the server reports 1.7; update those expectations (and any protocol-version docs/examples) as part of this change.
| WIRE_PROTOCOL_VERSION: str = "1.7" | |
| WIRE_PROTOCOL_VERSION: str = "1.6" |
| case PlanDisplay(): | ||
| pass |
There was a problem hiding this comment.
This PlanDisplay branch is currently a no-op, so ACP clients will never see the inline plan content. The PR description says ACP should forward PlanDisplay events; implement forwarding here (e.g., convert to a text chunk / session_update, potentially including the file_path) instead of pass.
| self.flush_content() | ||
| self.flush_finished_tool_calls() | ||
| plan_body = Markdown(msg.content) | ||
| subtitle = Text.from_markup(f"[dim]{msg.file_path}[/dim]") |
There was a problem hiding this comment.
Text.from_markup(f"[dim]{msg.file_path}[/dim]") treats the file path as Rich markup. If the path contains [/] (or other markup tokens), it can render incorrectly or inject styles. Use plain Text styling (e.g., Text(msg.file_path, style="dim")) or escape the path before passing it to from_markup.
| subtitle = Text.from_markup(f"[dim]{msg.file_path}[/dim]") | |
| subtitle = Text(msg.file_path, style="dim") |
| NAME = "ExitPlanMode" | ||
|
|
||
| _RESERVED_LABELS = {"reject", "revise", "approve"} | ||
| _RESERVED_LABELS = {"reject", "revise", "approve", "reject and exit"} |
There was a problem hiding this comment.
_RESERVED_LABELS now includes "reject and exit", but the validation error message below still only mentions Reject/Revise/Approve. Update the message so it matches the actual reserved set (and/or generate it from _RESERVED_LABELS) to avoid confusing tool authors.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55c512f0cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "User wants to revise the plan. Stay in plan mode. " | ||
| "Wait for the user's next message with feedback before revising." |
There was a problem hiding this comment.
Revise immediately when feedback is already captured
This branch now instructs the model to "wait for the user's next message" even though revise feedback may already be present in the current answers payload (and is appended right below). In practice, when a user chooses Revise and types feedback in the same prompt, this wording can make the agent stall for an extra turn instead of revising right away, which regresses the plan-review flow.
Useful? React with 👍 / 👎.
…ription for options
Signed-off-by: Kai <me@kaiyi.cool>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fc1270775
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| question="Approve this plan", | ||
| header="Plan", | ||
| body=plan_content, | ||
| options=question_options, |
There was a problem hiding this comment.
Preserve plan text for clients lacking PlanDisplay handling
This change removes QuestionItem.body and relies solely on a preceding PlanDisplay event for plan content, so clients that support plan mode/questions but have not yet implemented PlanDisplay will now receive an approval prompt without any plan text to review. Because wire initialization currently does not gate this behavior by a PlanDisplay capability, this is a backward-compatibility regression for existing 1.6-style clients; keep a body fallback (or gate by negotiated capability/protocol) so users can still review the plan before approving.
Useful? React with 👍 / 👎.
| turnCounterRef.current > 0 | ||
| ? turnCounterRef.current - 1 | ||
| : undefined, | ||
| content: planPayload.content, |
There was a problem hiding this comment.
Surface plan file path in web PlanDisplay rendering
The web handler consumes PlanDisplay but only renders content; file_path is dropped entirely, so users cannot see which on-disk plan file they are reviewing/editing when multiple plans exist. This undermines the new event contract (content + path) and the documented UX of showing the plan path for reference, so include file_path in the rendered message metadata or visible text.
Useful? React with 👍 / 👎.
Summary
Add a new
PlanDisplaywire message type and support inline rendering of plan content across all UI layers.Changes
PlanDisplaymessage type towire/types.pyand update protocol serialization.tools/planto emitPlanDisplaywire events for inline plan content rendering.PlanDisplayevents in shell visualizer and question panel.PlanDisplayevents in the ACP session layer.PlanDisplayEventtype inwireTypes.tsand handle it inuseSessionStream.PlanDisplay.