Skip to content

fix: bump agent-client-protocol to >=0.8.1#587

Merged
simonrosenberg merged 4 commits intomainfrom
fix/bump-acp-version
Mar 9, 2026
Merged

fix: bump agent-client-protocol to >=0.8.1#587
simonrosenberg merged 4 commits intomainfrom
fix/bump-acp-version

Conversation

@simonrosenberg
Copy link
Contributor

@simonrosenberg simonrosenberg commented Mar 9, 2026

Summary

  • Bumps agent-client-protocol from >=0.7.0,<0.8.0 to >=0.8.1
  • Resolves dependency conflict with openhands-sdk v1.12.0 which requires agent-client-protocol>=0.8.1
  • The previous upper bound (<0.8.0) prevented the CLI from upgrading to newer SDK versions, blocking releases that include other SDK fixes

Context

PR #2133 added agent-client-protocol>=0.8.1 as a required dependency of openhands-sdk. This created a mutual exclusion with the CLI's <0.8.0 constraint, making it impossible to release CLI versions that include SDK v1.12.0+.

What changed in ACP 0.8.1

All code changes in this PR are direct consequences of the agent-client-protocol 0.7.x → 0.8.1 upgrade:

Renamed symbols (runtime ImportError without these):

  • StdioMcpServerMcpServerStdio (1 production file, 2 test files)
  • SessionUpdate2SessionUpdate6 → named types: AgentMessageChunk, AgentThoughtChunk, ToolCallStart, ToolCallProgress, AgentPlanUpdate (1 test file)

Protocol signature changes (pyright errors without these):

  • new_session: mcp_servers parameter changed from list[Any] to list[...] | None = None
  • load_session: parameter order changed from (cwd, mcp_servers, session_id) to (cwd, session_id, mcp_servers), and mcp_servers became optional
  • All callers use keyword arguments, so the reorder is safe

New Protocol methods (pyright errors without these):

  • fork_session, resume_session, set_config_option were added to the Agent Protocol in 0.8.1 — stub implementations added to BaseOpenHandsACPAgent

Test plan

  • Verify uv lock resolves without conflicts
  • Verify pre-commit (pyright) passes
  • Verify unit tests pass
  • Verify E2E ACP server test passes in build

🤖 Generated with Claude Code


🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@fix/bump-acp-version

Resolves dependency conflict with openhands-sdk v1.12.0 which requires
agent-client-protocol>=0.8.1. The CLI's previous constraint (<0.8.0)
prevented upgrading the SDK and blocked CLI releases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@simonrosenberg simonrosenberg requested review from jpshackelford and removed request for jpshackelford March 9, 2026 14:03
simonrosenberg and others added 2 commits March 9, 2026 11:16
…ibility

- Rename StdioMcpServer -> McpServerStdio (renamed in ACP 0.8.1)
- Replace SessionUpdate2-6 with named types (AgentMessageChunk,
  AgentThoughtChunk, ToolCallStart, ToolCallProgress, AgentPlanUpdate)
- Add stub implementations for new Protocol methods (fork_session,
  resume_session, set_config_option) to satisfy pyright

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Import ForkSessionResponse/ResumeSessionResponse from acp.schema
  (not exported from acp top-level)
- Fix RequestError.method_not_found() calls (takes str, not dict)
- Fix new_session signature: mcp_servers is now optional (list | None)
- Fix load_session parameter order to match Protocol (cwd, session_id,
  mcp_servers) and make mcp_servers optional

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands_cli/acp_impl/agent
   base_agent.py2293086%132, 175, 216, 287–289, 349, 359, 369, 384–387, 391–398, 423, 505–506, 509–510, 586, 656–658
   local_agent.py84495%80, 144, 154, 168
   remote_agent.py1476357%85–86, 92–93, 97–98, 102, 104–108, 111–113, 123–124, 127–129, 133–135, 145–146, 161–163, 167, 169–170, 176, 180–181, 183, 192–193, 198–200, 211–212, 217, 219–220, 224, 231–232, 234–235, 238–240, 242, 250, 252–253, 284, 346–348, 354–355
openhands_cli/acp_impl/utils
   mcp.py25484%69–72
TOTAL664793185% 

@simonrosenberg simonrosenberg marked this pull request as draft March 9, 2026 14:59
@jpshackelford
Copy link
Contributor

jpshackelford commented Mar 9, 2026

I have run simple manual tests (single prompt) against the CLI, acp via Zed and acp via JetBrains IntelliJ IDEA. All looks good.

@jpshackelford
Copy link
Contributor

🟢 Good Taste - Clean, Mechanical Dependency Upgrade

Linus-Style Analysis:

This PR is boring in the best possible way. It's a dependency version bump that does exactly what it needs to do and nothing more. The changes fall into three categories:

1. Symbol Renames (Mechanical, correct)

  • StdioMcpServerMcpServerStdio
  • SessionUpdate2SessionUpdate6 → properly named types (AgentMessageChunk, AgentThoughtChunk, etc.)

These are upstream API renames. You have no choice. The new names are better anyway — SessionUpdate6 was garbage naming from the library author.

2. Signature Compliance (Mechanical, correct)

  • mcp_servers: list[Any]mcp_servers: list[Any] | None = None
  • Parameter reorder in load_session (callers use kwargs, so safe)

Again, upstream dictates this. The optional None default is fine — MCP servers are genuinely optional.

3. Protocol Stub Implementations (Correct approach)

async def fork_session(...) -> ForkSessionResponse:
    """Fork a session (not supported)."""
    raise RequestError.method_not_found("session/fork")

This is exactly what you should do when a Protocol forces you to implement methods you don't support. Raise method_not_found. Don't silently return garbage. Don't pretend you support it.

The only slight smell is set_config_option returning None instead of raising, but looking at the ACP Protocol, that seems to be the expected behavior for "not implemented" in that particular method.

What's not here (good):

  • No "while I'm here, let me also..." scope creep
  • No refactoring
  • No new features hiding in a dependency bump
  • Tests updated to match the renames (necessary, done)

✅ VERDICT: Worth merging

This is a dependency upgrade PR that doesn't try to be anything else. The code changes are 100% mechanical consequences of the upstream API changes. All CI checks pass. The PR description explains why every change exists.

KEY INSIGHT:
The author understood that dependency upgrades should be boring — the diff should be predictable from reading the upstream changelog, and this one is.


One Minor Note (Non-blocking):

[pyproject.toml, Line 26] The constraint changed from >=0.7.0,<0.8.0 to >=0.8.1 with no upper bound. This is fine for now since you're actively tracking ACP releases, but be aware that a future ACP 0.9.0 with breaking changes will auto-install and break things. Consider >=0.8.1,<0.9.0 if you want to be defensive. But that's a policy decision, not a code quality issue.

@jpshackelford
Copy link
Contributor

@simonrosenberg Since this agent-client-protocol does tend to be somewhat reckless, the suggestion of pinning with >=0.8.1,<0.9.0 or something more restrictive is a good idea.

Prevent future breakage from ACP minor version bumps which tend to
include breaking changes (pre-1.0 semver).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@simonrosenberg simonrosenberg marked this pull request as ready for review March 9, 2026 17:40
@simonrosenberg simonrosenberg merged commit a37f660 into main Mar 9, 2026
12 checks passed
@simonrosenberg simonrosenberg deleted the fix/bump-acp-version branch March 9, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants