feat(tools): add A2A (Agent-to-Agent) protocol bridge tool#1048
feat(tools): add A2A (Agent-to-Agent) protocol bridge tool#1048cryptoSUN2049 wants to merge 1 commit intonearai:stagingfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Agent-to-Agent (A2A) protocol bridge tool, significantly expanding the system's capability to interact with external AI agents. By supporting the Google A2A protocol, it enables seamless communication and multi-agent workflows. The implementation prioritizes security through comprehensive validation and approval mechanisms, while offering high configurability to adapt to various external agent setups. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new built-in tool, the A2A (Agent-to-Agent) protocol bridge, enabling communication with remote agents using the Google A2A protocol. The implementation is well-structured, with separate modules for configuration, protocol parsing, and the tool bridge itself. Security is a strong focus, with robust SSRF protection and secret leak detection. The use of a background task for handling asynchronous SSE streams is a good design choice for long-running operations. My review includes a couple of suggestions for improvement: one for performance in the SSE parser, aligning with best practices for avoiding unnecessary heap allocations, and another to correct a faulty unit test.
| let block = buffer[..boundary].to_string(); | ||
| // Remove the block + both newlines from the buffer | ||
| *buffer = buffer[boundary + 2..].to_string(); |
There was a problem hiding this comment.
This implementation creates two new String allocations in each iteration of the loop (block.to_string() and the slice-and-reassign to *buffer), which can be inefficient if the buffer is large or contains many events. Consider processing a string slice (&str) of the buffer and then using buffer.drain(..) to remove the processed part. This avoids reallocating the entire remaining buffer on each iteration.
References
- To improve performance, avoid unnecessary heap allocations, especially when processing string parts, by using slices or iterators directly instead of collecting into new data structures.
| fn parse_multiline_data_concatenation() { | ||
| // Two data: lines in the same event block should be concatenated | ||
| let mut buf = "data: {\"key\":\ndata: \"value\"}\n\n".to_string(); | ||
| let events = parse_sse_events(&mut buf); | ||
| assert_eq!(events.len(), 1); | ||
| assert_eq!(events[0].raw["key"], "value"); |
There was a problem hiding this comment.
This test appears to have an incorrect assumption about parsing multiline JSON data. The buffer "data: {\"key\":\ndata: \"value\"}\n\n" results in the data string {\"key\":\n \"value\"} (note the space from data: ), which is not valid JSON. Consequently, serde_json::from_str will fail, and no event will be parsed. The test incorrectly expects one event to be parsed successfully.
A valid multiline JSON event would look like this:
data: {
data: "key": "value"
data: }
This would produce the data string {\n "key": "value"\n}, which is valid JSON.
|
Thanks for the review @gemini-code-assist! Both issues have been addressed in the latest commit (634a65b):
Additionally, this commit includes several security hardening fixes:
All 35 unit tests + 6 integration tests passing, clippy zero warnings. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
zmanian
left a comment
There was a problem hiding this comment.
Good security posture -- SSRF protection, leak scanning, and ApprovalRequirement::Always are all well done. Key items:
- Blocking DNS in async context: validate_agent_url calls to_socket_addrs() which blocks. Use tokio::net::lookup_host() instead.
- New dependency: futures crate -- justify vs using tokio-stream::StreamExt which may already be available.
- Rebase needed: 13 of 15 commits are merge-from-main noise. Please rebase onto staging HEAD and squash the merge commits.
- Verify is_unique_local() / is_unicast_link_local() compile on stable Rust.
- No real CI ran (fork PR -- classify/scope only).
634a65b to
4241c61
Compare
|
Thanks for the thorough review @zmanian! All items addressed: 1. Blocking DNS → async ✅Replaced 2.
|
|
@zmanian Gentle ping — all 5 review items have been addressed (see reply above). The branch has been rebased onto staging HEAD and squashed to a single commit. Could you re-review when you get a chance? Also, would appreciate if someone could trigger CI on this fork PR — local tests all pass (36 unit + 6 integration + live E2E against a LangGraph A2A server). Thanks! |
Implements a bridge tool connecting to remote agents via the A2A
protocol. Updated to A2A v1.0.0 specification (released 2026-03-12).
v1.0.0 compliance:
- Part format: flat type fields without `kind` discriminator
- Method: `message/send` (replaces `message/stream`)
- Event classification via `status.state` (replaces deprecated `final`)
- Handles terminal states: completed, failed, canceled, rejected
- `A2A-Version: 1.0.0` header sent on all requests
- Backward compatible with v0.x servers (`final` field still checked)
Security:
- SSRF defense: async DNS via tokio::net::lookup_host(), blocks private
IPs, localhost, link-local, AWS metadata, DNS rebinding
- Leak detection on outgoing queries and context
- No-redirect HTTP policy, ApprovalRequirement::Always
- Session-aware background SSE consumer cancellation
LangGraph compatibility:
- Accept header: `text/event-stream, application/json`
- JSON response path for servers that don't support SSE
- Text extraction from history[]/artifacts[] format
Files:
- src/tools/builtin/a2a/{bridge,protocol,mod}.rs
- src/config/a2a.rs, tests/a2a_bridge_integration.rs
- scripts/test-a2a-bridge.sh
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4241c61 to
1d0cde2
Compare
serrrfirat
left a comment
There was a problem hiding this comment.
Requesting changes because I do not think this belongs as a built-in right now.
IronClaw's architecture docs draw a fairly clear boundary here:
AGENTS.mdsays built-in Rust tools are for core capabilities tightly coupled to the runtime, while external server integrations should use MCP or extensions.src/tools/README.mdsays WASM tools are the recommended path and that tool-specific logic/config should stay out of the main agent codebase.src/extensions/mod.rsframes extensions as the user-facing layer for tools and MCP servers.
This PR adds a startup-wired, env-configured bridge for an external A2A/LangGraph-style service (src/config/a2a.rs, src/main.rs, src/tools/builtin/a2a/*). Even with the security work, that still looks like an extension or MCP integration rather than a core built-in.
I think the capability itself can be useful, but I do not see a strong reason to bake this specific external protocol bridge into the main binary. Please rework this toward the extension/MCP path, or make the case for why A2A is a core runtime capability rather than an integration.
Summary
inject_txpush pattern (same asjob_monitor)Security Impact
Network calls to external A2A agent. Mitigated by:
redirect::Policy::none()) prevents redirect-based SSRFrequires_approval = Always— user must approve every invocation since it sends content to an external serviceChange Type
File Changes
New files:
src/config/a2a.rsA2aConfig— no hardcoded defaults, URL + assistant ID required when enabledsrc/tools/builtin/a2a/mod.rssrc/tools/builtin/a2a/protocol.rssrc/tools/builtin/a2a/bridge.rsA2aBridgeToolimpl with all security checks (9 unit tests)tests/a2a_bridge_integration.rs#[ignore])Modified files:
src/config/mod.rsmod a2a,A2aConfigre-export,a2a: Option<A2aConfig>fieldsrc/tools/builtin/mod.rspub mod a2a,A2aBridgeToolre-exportsrc/main.rsFEATURE_PARITY.mdConfiguration
Design Decisions
\n\n, multi-linedata:concatenation,\r\n/\rnormalization, comment line (:-prefixed) skippingclassify_event()checks top-levelerrorfield beforeresult, preventing silent error swallowinginject_tx.is_closed()each iteration, stopping promptly when session endsprotocol.rs(reusable A2A parsing) separated frombridge.rs(Tool trait impl)Validation
cargo fmtcargo clippy --all --all-features -- -D warnings(zero warnings)cargo test --lib -- a2a(34 unit tests passed)cargo test --test a2a_bridge_integration(6 passed, 1 ignored)cargo check --no-default-features --features libsql.unwrap()/.expect()in production codeextension_manager_with_process_manager_constructs) confirmed on upstream/mainReview Track: B
New feature + new tools — needs 1 approval + CI green + test evidence.
🤖 Generated with Claude Code