Skip to content

fix(sdk): revert agent-client-protocol to optional [acp] extra#2367

Open
simonrosenberg wants to merge 1 commit intomainfrom
sr/make-acp-optional-dependency
Open

fix(sdk): revert agent-client-protocol to optional [acp] extra#2367
simonrosenberg wants to merge 1 commit intomainfrom
sr/make-acp-optional-dependency

Conversation

@simonrosenberg
Copy link
Collaborator

Summary

  • Moves agent-client-protocol from a hard required dependency back to an optional extra (pip install openhands-sdk[acp])
  • SDK consumers who don't use ACPAgent are no longer forced to pull in agent-client-protocol>=0.8.1, unblocking downstream repos (e.g. the CLI) that pin agent-client-protocol<0.8.0
  • The lazy __getattr__ import in __init__.py now catches ImportError and provides a clear install instruction

Context

Commit e9cbac0 (Feb 20) promoted agent-client-protocol from an [acp] optional extra to a hard required dependency. This created a version conflict: the CLI pins agent-client-protocol>=0.7.0,<0.8.0 while the SDK now requires >=0.8.1. These constraints are mutually exclusive, preventing the CLI from upgrading to SDK v1.12.0+.

Changes

File Change
openhands-sdk/pyproject.toml Move agent-client-protocol>=0.8.1 from dependencies to [project.optional-dependencies] acp
openhands-sdk/.../agent/__init__.py Catch ImportError in __getattr__ with helpful install message
tests/sdk/agent/test_acp_agent.py Add pytest.importorskip("acp") to skip when not installed
examples/.../40_acp_agent_example.py Add install prerequisite note
pyproject.toml (root) Add agent-client-protocol to dev deps so CI tests still run
uv.lock Regenerated

Test plan

  • All 61 ACP agent tests pass (pytest tests/sdk/agent/test_acp_agent.py)
  • All 201 agent tests pass (pytest tests/sdk/agent/)
  • All pre-commit hooks pass (ruff, pycodestyle, pyright, import rules)
  • CI passes
  • Verify CLI can install SDK without pulling in ACP: pip install openhands-sdk should succeed without agent-client-protocol
  • Verify from openhands.sdk.agent import ACPAgent raises clear ImportError when ACP not installed

🤖 Generated with Claude Code

Move agent-client-protocol from a hard required dependency back to an
optional extra (pip install openhands-sdk[acp]). This unblocks
downstream consumers (e.g. the CLI) that pin ACP <0.8.0 from upgrading
to SDK v1.12.0+.

Changes:
- pyproject.toml: move agent-client-protocol to [project.optional-dependencies] acp
- __init__.py: catch ImportError in lazy __getattr__ with helpful message
- test_acp_agent.py: skip entire module when acp not installed
- example: add install prerequisite note
- root pyproject.toml: add agent-client-protocol to dev deps for CI

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

github-actions bot commented Mar 9, 2026

API breakage checks (Griffe)

Result: Passed

Action log

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

Agent server REST API breakage checks (OpenAPI)

Result: Failed

Log excerpt (first 1000 characters)
{"asctime": "2026-03-09 13:50:15,683", "levelname": "WARNING", "name": "openhands.agent_server.config", "filename": "config.py", "lineno": 173, "message": "\u26a0\ufe0f OH_SECRET_KEY was not defined. Secrets will not be persisted between restarts."}
::error title=openhands-agent-server REST API::Breaking REST API change detected without MINOR version bump (1.12.0 -> 1.12.0).

Breaking REST API changes detected compared to baseline release:
- the 'file' request property type/format changed from 'string'/'' to 'string'/'binary'
/home/runner/work/software-agent-sdk/software-agent-sdk/.venv/lib/python3.13/site-packages/litellm/llms/custom_httpx/async_client_cleanup.py:66: DeprecationWarning: There is no current event loop
  loop = asyncio.get_event_loop()

Action log

Copy link
Collaborator

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Acceptable - Pragmatic solution to a real version conflict problem.

This change makes agent-client-protocol optional, unblocking the CLI from upgrading. The implementation is straightforward:

  • ✅ Solves real problem (CLI pinned to <0.8.0, SDK now requires >=0.8.1)
  • ✅ Clear error message when dependency missing
  • ✅ Proper test handling with pytest.importorskip
  • ✅ Dev dependencies updated so CI tests still run

Missing Evidence

Your test plan has two unchecked manual verification steps. Add an Evidence section to the PR description showing:

  1. SDK installs without ACP:

    pip install openhands-sdk
    pip list | grep agent-client-protocol  # should be empty
  2. Import error is clear:

    python -c "from openhands.sdk.agent import ACPAgent"
    # should show: "The 'agent-client-protocol' package is required..."

Tests passing is good, but for packaging changes, seeing the actual install/import behavior confirms it works as intended.

Verdict: Code is correct and ready to merge once evidence is added. 🚢


Key Insight: This is dependency management pragmatism - not everyone needs every feature, and optional extras exist to prevent dependency conflicts. The implementation is sound.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-sdk/openhands/sdk/agent
   __init__.py13746%17–21, 27–28
TOTAL20808556973% 

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