feat: ACP↔A2A bridge — front any ACP coding agent over A2A#18
Conversation
…ver A2A Add an `acp` backend that drives any agent speaking Zed's Agent Client Protocol (Claude Code, Gemini CLI, Codex, OpenHands, ...) as a subprocess and normalizes its session/update stream onto the existing event vocabulary: - agent_message_chunk -> TextDelta - tool_call / tool_call_update -> ToolUse (+ FileChange for diff content) - session/request_permission -> PermissionRequest (the input-required pause) - PromptResponse usage + usage_update cost -> Result The permission round trip lands on the existing BackendSession seam: the agent's request_permission parks on session.request_permission until the A2A caller answers, so one input-required exchange still spans two execute() calls with the agent session alive in between. `events_from_update` and `select_option` are pure and unit-tested without a live subprocess; cost capture and permission parking live in the Client. The backend is the new default (`serve` defaults to acp+claude); `--agent` picks a launch preset and `--agent-command` drives any other ACP agent. The Claude SDK backend moves to an optional extra (a2claude[claude]); echo is unchanged.
Reframe the README and CLAUDE.md around the bridge thesis: a2claude exposes any ACP coding agent to remote A2A callers, preserving the permission round trip and cost that ACP (editor-oriented) and most A2A wrappers (text-flattened) drop. Document the acp/claude/echo backends, the --agent / --agent-command switches, and per-agent card naming.
|
Warning Review limit reached
More reviews will be available in 31 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReframes a2claude as an A2A-served coding agent, adds an ACP subprocess backend, updates CLI/server wiring for agent selection and card metadata, revises documentation and package metadata, and adds ACP-focused event, permission, filesystem, and CLI tests. ChangesACP backend rollout
Sequence Diagram(s)sequenceDiagram
participant serve
participant ACPBackend
participant ACPProcess as ACP agent adapter subprocess
participant _BridgeClient
participant BackendSession
serve->>ACPBackend: drive(request)
ACPBackend->>ACPProcess: spawn_agent_process(...)
ACPProcess->>_BridgeClient: session/update
_BridgeClient->>BackendSession: emit normalized events
ACPProcess->>_BridgeClient: request_permission
_BridgeClient->>BackendSession: request_permission(...)
BackendSession-->>_BridgeClient: allowed or denied outcome
_BridgeClient-->>ACPProcess: option_id or cancelled outcome
ACPBackend-->>serve: Result(session_id, usage, cost_usd)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a vendor-neutral acp backend to a2claude, enabling support for any coding agent that speaks Zed's Agent Client Protocol (ACP) over the A2A protocol. Key changes include the new backend implementation, updated CLI options, dependency updates, and new unit tests. Feedback focuses on critical security and robustness improvements, specifically addressing path traversal vulnerabilities in file operations by restricting access to the workspace directory, resolving potential AttributeError issues with empty file diffs, preventing unexpected slicing with non-positive line numbers, and using shlex.split to correctly parse agent commands with spaces.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Address review on the ACP backend: - Contain fs/read_text_file and fs/write_text_file under the resolved workspace cwd; a path resolving outside it raises rather than reading or writing an arbitrary file. The backend now passes its cwd into the bridge client. - Clamp a non-positive line number to 0 so a read can't slice from the end. - Treat a null diff new_text as empty, matching old_text. - Parse --agent-command with shlex.split so quoted/spaced arguments survive. Add tests for in-workspace read/write, rejection of paths that escape it, and line/limit slicing.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new vendor-neutral acp backend to a2claude, enabling it to drive any coding agent that speaks Zed's Agent Client Protocol (ACP) as a subprocess. It updates the project's dependencies, CLI options, server configuration, and documentation to support this new backend, and adds a suite of unit tests. The review feedback highlights two main issues: first, blocking synchronous file and directory operations in src/a2claude/backends/acp.py are executed within async methods and should be offloaded to a thread pool using asyncio.to_thread; second, an empty or whitespace-only --agent-command in src/a2claude/cli.py can cause an IndexError during command parsing and requires a validation guard.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Second review round: - Run the synchronous file read/write and mkdir in fs/read_text_file and fs/write_text_file via asyncio.to_thread so they can't stall the event loop the ACP connection shares with concurrent tasks. - Reject a --agent-command that splits to nothing (empty or whitespace-only) with a typer.BadParameter instead of letting parts[0] raise IndexError.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new vendor-neutral acp backend that allows a2claude to drive any coding agent speaking Zed's Agent Client Protocol (ACP) as a subprocess. It updates dependencies, CLI options, and documentation to support this backend, while making the Claude SDK an optional dependency. Feedback on the changes highlights a potential bug in the file-reading logic where a negative limit could cause unexpected slicing from the end of the file, along with a suggestion to add a corresponding test case.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
A negative limit made `start + limit` a negative slice end, returning lines from the end of the file instead of nothing. Clamp it with max(0, limit) and cover it with a test.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/a2claude/cli.py (1)
80-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate
agent_commandas optional.
Noneis a real value here, soagent_command: stris an inaccurate type for the new option. Usestr | Noneto match the runtime contract and the repo’s Python typing rule. As per coding guidelines, "**/*.py: Target Python 3.13+, use full type hints, includefrom __future__ import annotations..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/a2claude/cli.py` around lines 80 - 81, The agent_command option is typed too narrowly in the CLI option definition, since None is a valid runtime value. Update the typer.Option declaration in the CLI entrypoint to use an optional string type (str | None) for agent_command, and make sure the module follows the Python 3.13 typing guideline with from __future__ import annotations and full type hints.Source: Coding guidelines
src/a2claude/backends/__init__.py (1)
38-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDon't describe ACP as an optional runtime dependency.
agent-client-protocolis still a required install dependency inpyproject.toml, so this lazy import only shields the optional Claude SDK path. Rewording this docstring (and the matching contributor docs) would avoid implying thatechocan run without ACP installed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/a2claude/backends/__init__.py` around lines 38 - 39, Update the docstring in the backends package initializer so it no longer says both ACP and Claude are optional runtime dependencies; only the Claude Agent SDK should be described as lazily imported, since the echo backend still requires ACP. Also adjust the matching contributor documentation to reflect that lazy importing only shields the Claude SDK path and avoid implying echo works without ACP installed.tests/test_acp.py (1)
35-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing annotations in the new tests.
The helper coroutine and the new pytest functions still omit the repo-required full annotations (
-> Noneon tests, plus parameter/return types on_FakeSession.request_permission). As per coding guidelines,**/*.py: Target Python 3.13+, use full type hints, includefrom __future__ import annotations, and follow Ruff rulesE, F, I, UP, B, SIMat line length 88.Also applies to: 115-117, 120-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_acp.py` around lines 35 - 107, The new tests and helper coroutine in this test module are missing required type annotations. Add explicit return annotations of None to each pytest test function, and add full parameter and return type hints to _FakeSession.request_permission so it matches the repo’s Python typing rules. Keep the changes aligned with the existing test names and helper symbols in this module, and ensure the file remains consistent with the project’s full-annotations requirement.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/a2claude/cli.py`:
- Line 119: Move the _validate_permission_mode(permission_mode) check so it runs
only inside the backend == "claude" path in the CLI flow, instead of
unconditionally during startup. Keep the validation near the Claude-specific
setup logic in the cli entrypoint so ACP startup no longer depends on a
Claude-only flag when that backend is not selected. Make sure the update
preserves the backend boundary in the same area as the backend dispatch and
Claude initialization code.
- Around line 122-130: When `backend == "acp"` and `--agent-command` overrides
the preset, the ACP card metadata is still being derived from `--agent` via
`card_name` and the default `build_app()` description, so the published agent
identity can be wrong. Update the CLI flow in `src/a2claude/cli.py` so custom
commands either require explicit ACP metadata or derive both `card_name` and
`card_description` from the selected profile, and pass both into `build_app()`
instead of relying on the default card text from `src/a2claude/card.py`. Ensure
the startup label and the published `agent-card.json` stay consistent with the
actual subprocess identity, especially in the `backend == "acp"` branch and the
related code around the later ACP setup.
- Around line 125-129: `agent_command` parsing in the CLI currently lets
`shlex.split()` raise a raw `ValueError` for malformed quoting, which escapes as
a traceback. Update the `cli.py` command handling around `agent_command` so the
`shlex.split()` call is wrapped and any parsing error is converted into a
`typer.BadParameter`, alongside the existing empty/whitespace-only check, using
the same `kwargs["command"]` and `kwargs["args"]` flow.
---
Nitpick comments:
In `@src/a2claude/backends/__init__.py`:
- Around line 38-39: Update the docstring in the backends package initializer so
it no longer says both ACP and Claude are optional runtime dependencies; only
the Claude Agent SDK should be described as lazily imported, since the echo
backend still requires ACP. Also adjust the matching contributor documentation
to reflect that lazy importing only shields the Claude SDK path and avoid
implying echo works without ACP installed.
In `@src/a2claude/cli.py`:
- Around line 80-81: The agent_command option is typed too narrowly in the CLI
option definition, since None is a valid runtime value. Update the typer.Option
declaration in the CLI entrypoint to use an optional string type (str | None)
for agent_command, and make sure the module follows the Python 3.13 typing
guideline with from __future__ import annotations and full type hints.
In `@tests/test_acp.py`:
- Around line 35-107: The new tests and helper coroutine in this test module are
missing required type annotations. Add explicit return annotations of None to
each pytest test function, and add full parameter and return type hints to
_FakeSession.request_permission so it matches the repo’s Python typing rules.
Keep the changes aligned with the existing test names and helper symbols in this
module, and ensure the file remains consistent with the project’s
full-annotations requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67125d11-5d9a-42a5-bb66-b46f454df575
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
CLAUDE.mdREADME.mdpyproject.tomlsrc/a2claude/backends/__init__.pysrc/a2claude/backends/acp.pysrc/a2claude/backends/diff.pysrc/a2claude/cli.pysrc/a2claude/server.pytests/test_acp.py
… path Address CodeRabbit review: - Validate --permission-mode only on the claude backend path; a Claude-only flag must not block an ACP (or echo) run just because the SDK extra is installed. - Don't advertise an arbitrary --agent-command under a preset name: its identity is unknown, so the card name falls back to a neutral "Coding Agent" instead of defaulting to "Claude Code". Make the default card description backend-neutral too, so non-Claude ACP agents aren't published as Claude Code. - Surface a malformed --agent-command (unmatched quotes -> shlex ValueError) as a typer.BadParameter instead of a traceback. Update the permission-mode CLI tests to exercise the claude path, and add tests for off-path tolerance and malformed agent-command handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_cli.py`:
- Line 29: The updated test functions in test_cli need full type hints: add
explicit annotations for the mock_run fixture parameter and make each test
return None. Update the affected test definitions, including
test_serve_rejects_invalid_permission_mode and the other changed tests in this
file, so they follow the Python typing guideline consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 903e7134-a318-4ee6-b65e-41462fc93d1d
📒 Files selected for processing (3)
src/a2claude/card.pysrc/a2claude/cli.pytests/test_cli.py
✅ Files skipped from review due to trivial changes (1)
- src/a2claude/card.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/a2claude/cli.py
Annotate the mock_run fixture and return types on the permission-mode and agent-command serve tests, per the project's full-type-hints guideline.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new vendor-neutral acp backend to support any coding agent speaking Zed's Agent Client Protocol (ACP) as a subprocess, moving the direct Claude Agent SDK integration to an optional dependency. It updates the CLI and server to handle ACP-specific options and presets, adds comprehensive unit tests for the new protocol mapping, and updates documentation. Feedback on the changes suggests optimizing the file-reading implementation in the ACP bridge client to read files line-by-line when line and limit parameters are specified, preventing potential memory issues with large files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
When fs/read_text_file is given a line/limit window, read the file line by line and stop once the window is filled, rather than reading the entire file into memory just to slice a few lines. A non-positive limit short-circuits to an empty read; a non-positive line still starts from the top. Full reads (no window) keep the single read_text call.
Why
Being Claude-specific is a long-term liability, and a generic multi-backend coding-agent abstraction already exists — Zed's Agent Client Protocol (ACP), which a dozen agents (Claude Code, Gemini CLI, Codex, OpenHands, Goose, Cline, ...) already speak. Hand-writing a backend per agent would be reinventing it.
But ACP targets human-driven editors that own the filesystem and click "approve". The lane nobody occupies: exposing an ACP agent to remote autonomous A2A callers, with the permission round-trip and cost preserved as first-class protocol citizens. That's this PR.
a2claude becomes a bridge between two Linux Foundation interop standards: ACP on the agent side, A2A on the caller side. Vendor-neutrality becomes a launch-command choice instead of a maintenance burden.
What changed
acpbackend (backends/acp.py) — drives any ACP agent as a subprocess and maps itssession/updatestream onto the existing event vocabulary:agent_message_chunk→TextDeltatool_call/tool_call_update→ToolUse(+FileChangefrom diff content)session/request_permission→PermissionRequest(theinput-requiredpause)PromptResponseusage +usage_updatecost →ResultBackendSessionseam: the agent'srequest_permissionparks onsession.request_permissionuntil the A2A caller answers — oneinput-requiredexchange still spans twoexecute()calls.events_from_updateandselect_optionare pure and unit-tested (12 new tests) without launching a subprocess; cost capture / permission parking live in theClient, not the translator.acpis the new default backend.--agent claude|gemini|codexpicks a launch preset;--agent-commanddrives any other ACP agent. Per-agent card naming.a2claude[claude]);echois unchanged.Breaking changes
servebackend is nowacp(wasclaude).--backend claudestill works afteruv sync --extra claude.claude-agent-sdkis no longer a core dependency — it's theclaudeextra.agent-client-protocolis now a core dependency.Verification
ruff check,ruff format --check,mypy,pytest(50 passed),markdownlint-cli2, anduv sync --lockedall green.Follow-ups (not in this PR)
dwmkerr/claude-code-agent.claudeSDK backend once ACP coverage is proven against real agents.Summary by CodeRabbit