feat(a2a-nats-http): add HTTP/JSON-RPC + REST + SSE binding#365
Merged
Conversation
The transports stack so far has been NATS-native (server/stdio); this adds the HTTP-facing binary so callers that can't speak NATS directly still hit the same A2aClient surface. JSON-RPC, REST and SSE bindings all funnel through the existing message_send / tasks_* / push_* paths so the protocol contract stays single-sourced. Coverage split mirrors the other binaries: env parsing is testable, connect-and-serve gated behind cfg(not(coverage)). Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
PR SummaryMedium Risk Overview JSON-RPC is served on Runtime wires NATS + JetStream, env-driven timeouts, and optional Reviewed by Cursor Bugbot for commit cbdabe0. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Review limit reached
More reviews will be available in 21 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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 refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughA new Changesa2a-nats-http: Axum HTTP adapter for A2A over NATS
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Router as Axum Router
participant Negotiate as negotiate middleware
participant Handler as handle_jsonrpc / REST handler
participant A2aClient as A2aClient<N,J>
participant NATS
Client->>Router: POST / (JSON-RPC) or GET/POST /v1/...
Router->>Router: rewrite_a2a_custom_method (path rewrite if needed)
Router->>Negotiate: negotiate middleware
Negotiate->>Negotiate: parse + validate a2a-version / a2a-extensions
alt version or required extension invalid
Negotiate-->>Client: HTTP 400 JSON-RPC error
else negotiation ok
Negotiate->>Handler: next.run(request) with NegotiatedSpec in extensions
Handler->>A2aClient: method call (e.g. message_send, task_get, message_stream)
A2aClient->>NATS: publish/subscribe
NATS-->>A2aClient: response or event stream
A2aClient-->>Handler: Result<T, ClientError>
alt streaming (SSE)
Handler-->>Client: SSE stream (typed_event_stream_to_sse → JSON-RPC envelopes)
else one-shot
Handler-->>Client: JSON-RPC result or error (HTTP 200)
end
Negotiate->>Negotiate: echo a2a-version + a2a-extensions to response headers
Negotiate-->>Client: final response with A2A headers
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Code Coverage SummaryDetailsDiff against mainResults for commit: cbdabe0 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@rsworkspace/crates/a2a-nats-http/README.md`:
- Around line 1-3: The README heading uses the incorrect crate name
"a2a-nats-server" when it should be "a2a-nats-http" to match the actual binary
name. Update the heading from "a2a-nats-server" to "a2a-nats-http" on the first
line, and also review lines 10-12 for any other references to "a2a-nats-server"
that need to be corrected to the proper crate/binary name "a2a-nats-http" to
ensure the documented startup path and examples are accurate.
In `@rsworkspace/crates/a2a-nats-http/src/handlers/mod.rs`:
- Around line 45-48: Add validation for the jsonrpc version field before
dispatching the method. After extracting id and params from the envelope but
before the match statement that dispatches on envelope.method, check that
envelope.jsonrpc exists and equals "2.0". If the jsonrpc field is missing or
does not match "2.0", return a JSON-RPC invalid-request error response with the
appropriate error code and message instead of proceeding to the method dispatch.
In `@rsworkspace/crates/a2a-nats-http/src/headers.rs`:
- Around line 88-100: The code does not validate that extracted URIs are
non-empty after parsing, which allows malformed headers like "?" or ";q=0" to
create invalid extension entries. In both branches of the condition (the
strip_prefix('?') case and the split_once(';') case), after trimming the URI
value, add a check to ensure the resulting string is not empty. If the URI is
empty, skip the parse and return None instead of creating the Self struct. This
validation should occur before constructing the Self object in both the first
branch (around line 90) and second branch (around line 99).
In `@rsworkspace/crates/a2a-nats-http/src/rest.rs`:
- Around line 270-287: The post_push_set function extracts the path id parameter
but discards it with the underscore prefix, allowing the untrusted JSON request
body to entirely control the write target. Remove the underscore from the _id
parameter in the Path(Path(_id)) binding, then validate that the task id from
the path matches the task id in the TaskPushNotificationConfig request body (or
overwrite the request body's task id with the path id) before calling
client.push_set to ensure the path constraint is enforced according to the
untrusted input boundary type guidelines.
In `@rsworkspace/crates/a2a-nats-http/src/runtime.rs`:
- Line 23: Replace the `InvalidGatewayCallerJwt(String)` error variant with a
typed variant that wraps the original JWT error directly (e.g., a struct field
containing the actual error type returned by JWT parsing/validation operations).
Update all locations where this variant is constructed (including lines 39-40
and 113-120) to pass the original error object instead of converting it to a
String. This preserves error context and enables proper source chaining for
debugging.
In `@rsworkspace/crates/a2a-nats-http/src/sse.rs`:
- Around line 21-27: The error handling in the unwrap_or_else closure returns
JSON-RPC error code -32700 (Parse error) with id set to null, but since this is
a server-side serialization failure (not a client parse error), it should return
error code -32603 (Internal error) instead. Additionally, preserve the original
request id from the envelope for proper request correlation. Modify the
serde_json::json! block to change the error code from -32700 to -32603 and set
the id field to the actual id from the envelope instead of null.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ca234df-968a-41e1-a429-2430b5433424
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
rsworkspace/crates/a2a-nats-http/Cargo.tomlrsworkspace/crates/a2a-nats-http/README.mdrsworkspace/crates/a2a-nats-http/src/handlers/mod.rsrsworkspace/crates/a2a-nats-http/src/headers.rsrsworkspace/crates/a2a-nats-http/src/lib.rsrsworkspace/crates/a2a-nats-http/src/main.rsrsworkspace/crates/a2a-nats-http/src/rest.rsrsworkspace/crates/a2a-nats-http/src/router.rsrsworkspace/crates/a2a-nats-http/src/runtime.rsrsworkspace/crates/a2a-nats-http/src/sse.rsrsworkspace/crates/a2a-nats-http/src/tests.rs
… headers, runtime, sse - handlers: read top-level lastSeq for tasks/resubscribe so the http and stdio binaries stay wire-compatible on the resume cursor - handlers: reject envelopes without jsonrpc='2.0' before dispatch so the bridge can't front another protocol's calls - handlers: agent-card failures now return a JSON error envelope so JSON clients can parse them, matching REST get_card and the success path - rest: post_push_set now binds writes to the path task id, rejecting a mismatched body taskId instead of letting untrusted JSON pick the target - rest: drop the spurious _keep_delete_chain noop route — it was router scaffolding, not an A2A operation - headers: skip extension tokens whose uri ends up empty after optional marker stripping (a bare '?' or ';q=0' must not negotiate implicitly) - runtime: replace InvalidGatewayCallerJwt(String) with InvalidGatewayCallerJwt(JwtError) so source chaining works and structured context isn't lost to format! - sse: server-side envelope serialization failure is -32603 Internal, not -32700 Parse error, and echoes the original id for stream correlation - README: crate/binary name reads a2a-nats-http, not a2a-nats-server Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
The handler was discarding the unary SendMessageResponse and only forwarding JetStream events, so HTTP callers never received the task handle that anchors subsequent notifications — out of sync with tasks/resubscribe and the stdio bridge, which both emit a bootstrap result before the stream. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Two follow-on streaming inconsistencies:
- REST subscribe was emitting the snapshot as a bare {result} frame
while later JetStream frames carried the full jsonrpc envelope, so a
single response stream mixed two wire formats.
- JetStream updates on message/stream and tasks/resubscribe rode as
repeated 'result' messages tagged with the request id. The stdio
bridge sends JSON-RPC notifications with method set to the originating
stream method so callers can route incremental events without
matching on id; HTTP now does the same.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d8890b2. Configure here.
…to REST JSON shape
The well-known agent-card route was special-casing AGENT_UNAVAILABLE
and bucketing every other ClientError as HTTP 500 while /v1/card
mapped extended-card-not-configured to 404, invalid-agent-response to
502, etc. — the same protocol failure returned different HTTP status
codes depending on which route the client hit. Both now share the REST
status table so the response contract is route-independent.
The /v1/tasks/{id}/subscribe route also returned a bare text body when
A2aTaskId::new rejected the path id; sibling REST routes return the
{"error":{code,message}} JSON shape, so clients couldn't parse this
one failure uniformly. Subscribe now matches the rest.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
added
the
rust:coverage-baseline-reset
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
a2a-nats-server,a2a-nats-stdio); HTTP-facing callers that can't dial NATS directly need a binding that funnels through the sameA2aClientso the protocol contract stays single-sourcedcfg(not(coverage))becausetrogon-nats::NatsJetStreamClientis excluded from coverage builds