feat(acp): protocol version negotiation and session/resume#1100
feat(acp): protocol version negotiation and session/resume#1100
Conversation
Implement ACP version management framework so the server properly negotiates protocol_version instead of echoing the client's value. Fix SDK version declaration (0.7.0 → 0.8.0) to match the actual installed package, and add 13 new tests covering both the negotiation logic and end-to-end protocol V1 consistency via the real SDK client.
Extract shared session setup logic into _setup_session and add resume_session method that returns ResumeSessionResponse with modes/models state. Advertise the resume capability in initialize.
There was a problem hiding this comment.
Pull request overview
Implements ACP protocol improvements in kimi-cli by adding server-side protocol version negotiation and introducing an (unstable) session/resume endpoint, plus accompanying dependency bumps, tests, and documentation updates.
Changes:
- Add
ACPVersionSpec+negotiate_version()and update ACP serverinitializeto return the negotiated protocol version (and advertiseresumecapability). - Add
resume_sessionimplementation and refactor shared session setup logic into_setup_session. - Bump
agent-client-protocolto0.8.0and add new unit + end-to-end ACP protocol V1 tests; update changelogs/docs.
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates locked agent-client-protocol dependency to 0.8.0. |
pyproject.toml |
Pins agent-client-protocol==0.8.0 for runtime dependency consistency. |
src/kimi_cli/acp/version.py |
Introduces version spec model + negotiation logic used by ACP server initialization. |
src/kimi_cli/acp/server.py |
Uses negotiation in initialize, advertises resume, adds resume_session, and refactors session setup. |
tests/acp/conftest.py |
Adds fixtures for spawning a real ACP agent subprocess and a minimal client for callbacks. |
tests/acp/test_version.py |
Unit tests for version negotiation and invariants (immutability, min/max consistency). |
tests/acp/test_protocol_v1.py |
Async end-to-end protocol V1 flow tests using the real ACP SDK client, including resume and cancel. |
docs/zh/release-notes/changelog.md |
Adds unreleased notes for ACP negotiation + session resume. |
docs/en/release-notes/changelog.md |
Adds unreleased notes for ACP negotiation + session resume. |
CHANGELOG.md |
Adds unreleased notes for ACP negotiation + session resume. |
docs/zh/reference/kimi-web.md |
Updates Web UI reference timeline/details for the context usage indicator move. |
docs/en/reference/kimi-web.md |
Updates Web UI reference timeline/details for the context usage indicator move. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _kimi_bin() -> str: | ||
| """Return the path to the kimi entry-point script inside the venv.""" | ||
| return str(_repo_root() / ".venv" / "bin" / "kimi") |
There was a problem hiding this comment.
The test helper hard-codes the CLI path to .../.venv/bin/kimi, which is brittle outside the uv sync layout (and non-portable to other environments). Prefer resolving the CLI via sys.executable -m kimi_cli.cli (as used in other tests) or shutil.which("kimi"), with a clear fallback/error if not found.
| env=env, | ||
| cwd=str(_repo_root()), | ||
| use_unstable_protocol=True, | ||
| ) as (conn, process): |
There was a problem hiding this comment.
process is assigned in the async with ... as (conn, process) target but never used, which will trigger Ruff F841 (unused local variable) and fail CI. Rename it to _process/_ or use it (e.g., for debugging/termination assertions).
| ) as (conn, process): | |
| ) as (conn, _process): |
YoungY620
force-pushed
the
acp-upgrade
branch
from
dcac962 to
034d220
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| acp_session, model_id_conv = self.sessions[session_id] | ||
| config = acp_session.cli.soul.runtime.config | ||
| return acp.schema.ResumeSessionResponse( | ||
| modes=acp.schema.SessionModeState( | ||
| available_modes=[ | ||
| acp.schema.SessionMode( | ||
| id="default", | ||
| name="Default", | ||
| description="The default mode.", | ||
| ), | ||
| ], | ||
| current_mode_id="default", | ||
| ), | ||
| models=acp.schema.SessionModelState( | ||
| available_models=_expand_llm_models(config.models), | ||
| current_model_id=model_id_conv.to_acp_model_id(), |
There was a problem hiding this comment.
🟡 resume_session returns stale current_model_id after set_session_model
When resume_session is called for an already-loaded session, it reads model_id_conv from the cached tuple in self.sessions[session_id] at line 231. However, set_session_model (src/kimi_cli/acp/server.py:274-319) never updates the stored tuple after changing the model — it updates config.default_model and config.default_thinking (lines 313-314) but does not write back self.sessions[session_id] = (acp_session, model_id_conv).
Root Cause
The flow is:
new_sessionstores(acp_session, _ModelIDConv("original", False))inself.sessions.set_session_model("new_model")reads the tuple, creates a new LLM, updatesconfig.default_model, but never writes back the new_ModelIDConvtoself.sessions.resume_sessionreadsmodel_id_convfrom the tuple at line 231 and returnsmodel_id_conv.to_acp_model_id()ascurrent_model_idat line 246.
The client receives the original model ID instead of the current one. This causes the client UI to display the wrong active model after a model switch and session resume.
Impact: The ResumeSessionResponse.models.current_model_id will be incorrect whenever the model was changed via set_session_model before resume_session is called. The same stale-tuple issue also causes set_session_model's own equality check at line 287 (model_id_conv == current_model_id) to compare against the original model rather than the last-set model, but that pre-existing issue only affected set_session_model itself. The new resume_session exposes this stale data to the client.
Prompt for agents
In set_session_model (around line 274-319 of src/kimi_cli/acp/server.py), after successfully creating the new LLM and updating the config, add a line to update the stored tuple in self.sessions:
After line 311 (cli_instance.soul.runtime.llm = new_llm), add:
self.sessions[session_id] = (acp_session, model_id_conv)
This ensures that resume_session (and any future code reading from self.sessions) sees the correct model_id_conv after a model change.
Was this helpful? React with 👍 or 👎 to provide feedback.
YoungY620
force-pushed
the
acp-upgrade
branch
from
034d220 to
024fd92
Compare
- Fix new_session/load_session parameter order and defaults to match Agent protocol - Add fork_session stub to ACPServer and ACPServerSingleSession - Add resume_session stub to ACPServerSingleSession - Update ACPTestClient to match Client protocol signatures - Fix RequestPermissionResponse to use new outcome API
1 participant
Related Issue
N/A (internal ACP protocol improvement)
Description
Protocol version negotiation: Implement
ACPVersionSpecframework so the server properly negotiatesprotocol_versionwith the client instead of echoing it back. Fix SDK version declaration (0.7.0 → 0.8.0) to match the installed package.session/resume(unstable): Implement theresume_sessionmethod so clients can resume an existing session and receiveResumeSessionResponsewith modes/models state. Shared setup logic is extracted into_setup_sessionto avoid duplication withload_session. Theresumecapability is advertised in theinitializeresponse.Tests: Add 15 new tests covering version negotiation logic and end-to-end protocol V1 consistency (initialize, new_session, prompt, list_sessions, resume_session, cancel) via the real ACP SDK client.
Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.