feat(acp): support permission mode switching#2364
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| case ThinkPart(think=think): | ||
| await self._send_thinking(think) | ||
| replayed_updates += 1 | ||
| case TextPart(text=text): | ||
| await self._send_text(text) | ||
| replayed_updates += 1 | ||
| case ContentPart(): | ||
| logger.warning("Unsupported replay content part: {part}", part=msg) | ||
| await self._send_text(f"[{msg.__class__.__name__}]") | ||
| replayed_updates += 1 | ||
| case ToolCall(): | ||
| if turn_token is None: | ||
| turn_token = self._begin_replay_turn() | ||
| await self._send_tool_call(msg) | ||
| replayed_updates += 1 | ||
| case ToolCallPart(): | ||
| if self._turn_state is not None: | ||
| await self._send_tool_call_part(msg) | ||
| replayed_updates += 1 | ||
| case ToolResult(): | ||
| if self._turn_state is not None: | ||
| await self._send_tool_result(msg) | ||
| replayed_updates += 1 | ||
| case Notification(): | ||
| await self._send_notification(msg) | ||
| replayed_updates += 1 |
There was a problem hiding this comment.
🔴 AssertionError in replay_history when Notification/content appears outside a turn
In replay_history, after a TurnEnd or StepInterrupted message, self._turn_state is set to None at src/kimi_cli/acp/session.py:329. If a Notification record follows in the wire file, the call chain _send_notification → _send_text → _content_run_id (line 438) hits assert self._turn_state is not None and crashes with AssertionError.
Notifications from background tasks are pumped into the wire file asynchronously via _pump_notifications_to_wire (src/kimi_cli/soul/__init__.py:279) and can legitimately appear at any position — including after TurnEnd or before the first TurnBegin. The same crash applies to ThinkPart, TextPart, and ContentPart records that appear outside a turn boundary during replay. Unlike ToolCall and StepBegin (which have if turn_token is None: turn_token = self._begin_replay_turn() guards), these message types have no such protection.
Prompt for agents
In replay_history(), the ThinkPart, TextPart, ContentPart, and Notification handlers all call _send_thinking/_send_text which invoke _content_run_id, which asserts self._turn_state is not None. But self._turn_state can be None after TurnEnd/StepInterrupted sets it to None, or before the first TurnBegin.
The fix should mirror what ToolCall and StepBegin already do: check if turn_token is None and call self._begin_replay_turn() to create a new _TurnState before attempting to send. For example, add a guard like:
if turn_token is None:
turn_token = self._begin_replay_turn()
before each of the ThinkPart, TextPart, ContentPart, and Notification cases (lines 333-358 in session.py). This ensures _turn_state is always set when _content_run_id is called during replay.
Affected file: src/kimi_cli/acp/session.py, method replay_history(), lines 333-358.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73f6534022
ℹ️ 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".
| for task in pending: | ||
| task.cancel() | ||
| if pending: | ||
| await asyncio.gather(*pending, return_exceptions=True) |
There was a problem hiding this comment.
Cancelling every pending task here also cancels request_task, which is awaiting ApprovalRequest.wait() on the same future consumed by the approval bridge in KimiCLI.run. In the normal path where request_permission returns first, this cancellation marks the shared request future as cancelled, so the later request.resolve(...) calls in this function become no-ops and the bridge never resolves approval_runtime; tool calls that requested approval can then hang indefinitely waiting for a response. This regression is triggered whenever an ACP client responds to a permission dialog before any external mode switch resolves the request.
Useful? React with 👍 / 👎.
1 participant
Related Issue
Resolve #1414
Note that this stacks on top of #2363 - Each PR should be reviewed / merged in order. This is not a 700 line PR trying to change 3 things 😂
Description
I added protocol-level ACP permission mode switching for Kimi sessions.
This PR advertises
defaultandyoloACP session modes fromsession/new,session/load, andsession/resume, and implementssession/set_modeso ACP clients can switch Kimi's persisted yolo approval state without restarting the session. When switching intoyolo, Kimi also resolves currently pending approval requests so a permission dialog does not stay stuck after the mode change. The ACP approval bridge now races the client permission request with local request resolution so external mode changes can unblock an in-flight approval.I checked the prior open context before implementing this: issue #1414 requested switching directly into yolo mode from permission handling, and PR #1525 attempted that by adding an extra approval-dialog option. This implementation keeps the behavior at the ACP session-mode protocol layer instead.
This branch is stacked on the existing ACP message-id and session-load history commits, with this PR still targeting
main.Verification
uv run ruff check src/kimi_cli/acp/server.py src/kimi_cli/acp/session.py tests/acp/test_protocol_v1.py tests/acp/test_server_permission_modes.pyuv run pytest tests/acpuv run pytest tests/acp/test_protocol_v1.py tests/acp/test_server_permission_modes.pykimi acpthrough the ACP SDK, creating a session, switchingdefault -> yolo -> default, checking resumed mode state, and confirming an invalid mode is rejected:SMOKE_OK root=/tmp/kimi-acp-mode-smoke-qcJqmyChecklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.