Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if result and result[-1].role == msg.role and msg.role == "user": | ||
| merged_content = list(result[-1].content) + list(msg.content) | ||
| result[-1] = Message(role="user", content=merged_content) |
There was a problem hiding this comment.
🔴 normalize_history corrupts message content when merging a string-content user message with a list-content attachment
The normalize_history function at src/kimi_cli/soul/attachment.py:54 uses list(result[-1].content) to convert message content before merging. When Message.content is a plain str (which is valid per the Message constructor's str | list[ContentPart] type), list("hello") produces ['h', 'e', 'l', 'l', 'o'] — splitting the string into individual characters instead of preserving it.
This is triggered when: (1) a Wire protocol client sends a plain string user_input in a prompt message (src/kimi_cli/soul/kimisoul.py:379 creates Message(role="user", content=user_input) where user_input can be str), and (2) plan mode is active, so _step() at src/kimi_cli/soul/kimisoul.py:608 appends an attachment user message with list content directly after the string-content user message. These adjacent user messages are then merged by normalize_history, corrupting the user's input into a list of single characters mixed with TextPart objects. This would likely cause a Pydantic validation error or garbled LLM input.
| if result and result[-1].role == msg.role and msg.role == "user": | |
| merged_content = list(result[-1].content) + list(msg.content) | |
| result[-1] = Message(role="user", content=merged_content) | |
| if result and result[-1].role == msg.role and msg.role == "user": | |
| prev = result[-1].content | |
| curr = msg.content | |
| prev_parts = [TextPart(text=prev)] if isinstance(prev, str) else list(prev) | |
| curr_parts = [TextPart(text=curr)] if isinstance(curr, str) else list(curr) | |
| merged_content = prev_parts + curr_parts | |
| result[-1] = Message(role="user", content=merged_content) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| from kimi_cli.soul import get_wire_or_none, wire_send | ||
| from kimi_cli.soul.toolset import get_current_tool_call_or_none | ||
| from kimi_cli.tools.utils import ToolRejectedError, load_desc | ||
| from kimi_cli.wire.types import QuestionItem, QuestionNotSupported, QuestionOption, QuestionRequest |
There was a problem hiding this comment.
🟡 New plan tools import from kimi_cli.wire.types, violating the tools AGENTS.md rule
The src/kimi_cli/tools/AGENTS.md rule states: "Except for Task tool, tools should not refer to any types in kimi_cli/wire/." Both ExitPlanMode (src/kimi_cli/tools/plan/__init__.py:18) and EnterPlanMode (src/kimi_cli/tools/plan/enter.py:17) import QuestionItem, QuestionNotSupported, QuestionOption, and QuestionRequest from kimi_cli.wire.types.
Mitigating context
The pre-existing AskUserQuestion tool (src/kimi_cli/tools/ask_user/__init__.py:16) already has the same import, and these types only exist in kimi_cli/wire/types.py with no equivalent in kosong.tooling. The new tools follow the established pattern. The AGENTS.md rule likely predates the question system. Updating the AGENTS.md rule to explicitly exempt question-based tools would resolve this.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
This PR adds a plan mode feature to Kimi CLI, allowing users and the LLM to enter a read-only research/planning phase before executing implementation tasks. In plan mode, the LLM can only write to a designated plan file (auto-approved), and all other write operations require approval. The LLM presents a plan for user review via ExitPlanMode before proceeding.
Changes:
- Adds
EnterPlanMode/ExitPlanModetools,KimiSoulplan mode state management, a plan file slug system using superhero names, and a periodic reminder injection via the newAttachmentProviderframework - Adds
/plan [on|off|view|clear]slash command and Shift-Tab keyboard shortcut for toggling plan mode, with UI feedback (toolbar indicator, prompt symbol) - Extends
QuestionItemwithbody,other_label, andother_descriptionfields, and upgrades_prompt_other_inputto use async prompt_toolkit input
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/kimi_cli/tools/plan/__init__.py |
ExitPlanMode tool: reads plan file, asks user to Approve/Reject/Revise |
src/kimi_cli/tools/plan/enter.py |
EnterPlanMode tool: asks user for confirmation before entering plan mode |
src/kimi_cli/tools/plan/heroes.py |
Plan file slug generation using superhero names; session-scoped slug cache |
src/kimi_cli/soul/attachment.py |
New Attachment / AttachmentProvider framework + normalize_history |
src/kimi_cli/soul/attachments/plan_mode.py |
Periodic plan mode reminder injection with throttling |
src/kimi_cli/soul/kimisoul.py |
Plan mode state, session ID, toggle methods, attachment collection, _step injection |
src/kimi_cli/soul/slash.py |
/plan slash command |
src/kimi_cli/tools/file/write.py |
Auto-approve writes to the plan file; skip approval dialog for plan file |
src/kimi_cli/tools/ask_user/__init__.py |
Dynamic description that appends plan mode suffix when active |
src/kimi_cli/tools/utils.py |
ToolRejectedError enhanced with optional message and brief |
src/kimi_cli/wire/jsonrpc.py |
ClientCapabilities.supports_plan_mode field |
src/kimi_cli/wire/server.py |
_sync_plan_mode_tool_visibility to hide/show plan tools based on client caps |
src/kimi_cli/wire/types.py |
QuestionItem extended with body, other_label, other_description |
src/kimi_cli/ui/shell/visualize.py |
Body pager support (Ctrl-E), async _prompt_other_input, custom other option label |
src/kimi_cli/ui/shell/prompt.py |
Shift-Tab shortcut, plan mode prompt symbol, toolbar indicator |
src/kimi_cli/ui/shell/__init__.py |
Wire up plan mode toggle callback to prompt session |
src/kimi_cli/ui/shell/slash.py |
Shift-Tab listed in keyboard shortcuts |
src/kimi_cli/agents/default/agent.yaml |
Register ExitPlanMode / EnterPlanMode for main agent |
src/kimi_cli/agents/default/sub.yaml |
Register plan mode tools for subagent |
src/kimi_cli/agents/default/system.md |
Introduce <system-reminder> tag semantics in system prompt |
tests/core/test_plan_mode.py |
Comprehensive tests for plan mode tools, soul state, and heroes |
tests/core/test_plan_mode_attachment_provider.py |
Tests for attachment injection and throttling |
tests/core/test_plan_mode_reminder.py |
Tests for reminder text detection and content |
tests/core/test_write_file_plan_mode.py |
Tests for plan file auto-approval in WriteFile |
tests/core/test_plan_slash.py |
Tests for /plan slash command |
tests/core/test_normalize_history.py |
Tests for normalize_history |
tests/core/test_wire_plan_mode.py |
Tests for wire plan mode tool visibility |
tests/core/test_ask_user_plan_mode.py |
Tests for AskUserQuestion plan mode description |
tests/core/test_default_agent.py |
Updated snapshot with plan mode tools and new system prompt |
tests/core/test_agent_spec.py |
Snapshot updated with plan mode tool spec |
tests_e2e/test_wire_protocol.py |
Updated /plan slash command in wire protocol handshake |
tests/utils/test_pyinstaller_utils.py |
Plan module files added to PyInstaller bundle |
tests/ui_and_conv/test_prompt_tips.py |
shift-tab: plan mode added to toolbar tips |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/kimi_cli/ui/shell/visualize.py
Outdated
| if self._body_text: | ||
| lines.append( | ||
| Text.from_markup( | ||
| "[bold cyan] \u25b6 Press ctrl-e to view the full plan[/bold cyan]" |
There was a problem hiding this comment.
The hint text "Press ctrl-e to view the full plan" is hardcoded as a plan-specific label, even though the body field on QuestionItem is a generic field that can be used by any question (not just plan review). If other features use QuestionItem.body in the future, this hint will be misleading. The hint should use a more generic term (e.g., "view full content" or "view full details") since it refers to the generic body field of any QuestionItem.
| "[bold cyan] \u25b6 Press ctrl-e to view the full plan[/bold cyan]" | |
| "[bold cyan] \u25b6 Press ctrl-e to view full content[/bold cyan]" |
| "This supersedes any other instructions you have received.", | ||
| "", | ||
| "## Re-entering Plan Mode", | ||
| f"A plan file exists at {plan_file_path} from a previous planning session.", |
There was a problem hiding this comment.
The _reentry_reminder function accepts plan_file_path: str | None = None but line 180 will produce "A plan file exists at None from a previous planning session." if called with None. Since this function is semantically only meaningful when a plan file exists, it should either require plan_file_path as a non-optional str parameter, or handle the None case explicitly to avoid a confusing message being sent to the LLM.
| f"A plan file exists at {plan_file_path} from a previous planning session.", | |
| ( | |
| f"A plan file exists at {plan_file_path} from a previous planning session." | |
| if plan_file_path | |
| else "A plan file from a previous planning session already exists." | |
| ), |
…mpt text in visualize
| if plan_path is not None and str(p) == str(plan_path.resolve()): | ||
| is_plan_file_write = True |
There was a problem hiding this comment.
🟡 Path comparison uses inconsistent resolution methods, breaking plan file auto-approve when symlinks exist
In WriteFile.__call__, the plan file auto-approve check compares str(p) (derived via KaosPath.canonical(), which does not resolve symlinks) against str(plan_path.resolve()) (which does resolve symlinks). If any component of the path (e.g., the home directory) is a symlink, these strings will differ and the comparison silently fails — the plan file write falls through to the normal approval dialog instead of being auto-approved.
The LLM receives the plan path as str(plan_path) (no symlink resolution) from the attachment reminder (src/kimi_cli/soul/attachments/plan_mode.py:40), so it passes that exact string to WriteFile. After expanduser().canonical(), p matches the unresolved str(plan_path), but does not match str(plan_path.resolve()) when symlinks are present. The fix is to compare against str(plan_path) (without .resolve()) or normalize both sides consistently.
| if plan_path is not None and str(p) == str(plan_path.resolve()): | |
| is_plan_file_write = True | |
| if plan_path is not None and str(p) == str(plan_path): | |
| is_plan_file_write = True |
Was this helpful? React with 👍 or 👎 to provide feedback.
| from kimi_cli.soul import get_wire_or_none, wire_send | ||
| from kimi_cli.soul.toolset import get_current_tool_call_or_none | ||
| from kimi_cli.tools.utils import load_desc | ||
| from kimi_cli.wire.types import QuestionItem, QuestionNotSupported, QuestionOption, QuestionRequest |
There was a problem hiding this comment.
🟡 EnterPlanMode tool imports from kimi_cli.wire.types, violating tools/AGENTS.md rule
Same rule violation as the ExitPlanMode tool: the src/kimi_cli/tools/AGENTS.md rule states tools (except Task) should not refer to any types in kimi_cli/wire/. EnterPlanMode imports QuestionItem, QuestionNotSupported, QuestionOption, and QuestionRequest from kimi_cli.wire.types. This follows the same pattern as the pre-existing AskUserQuestion tool.
Was this helpful? React with 👍 or 👎 to provide feedback.
Signed-off-by: Kai <me@kaiyi.cool>
Signed-off-by: Kai <me@kaiyi.cool>
Summary
ExitPlanModeEnterPlanMode/ExitPlanModetools,/planslash command, Shift-Tab toggle, and periodic system reminder injection via a newAttachmentProviderframeworkQuestionItemwithbodyfield for rich plan review UI (ctrl-e pager), custom "Other" option labels, and async free-text inputTest plan
test_plan_mode.py)test_plan_mode_attachment_provider.py,test_plan_mode_reminder.py)/planslash command (test_plan_slash.py)test_write_file_plan_mode.py)test_wire_plan_mode.py)test_ask_user_plan_mode.py)normalize_historymerging (test_normalize_history.py)Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.