fix(server,client): return Task/Message directly from message/send - #1196
Open
richardsonlima wants to merge 4 commits into
Open
fix(server,client): return Task/Message directly from message/send#1196richardsonlima wants to merge 4 commits into
richardsonlima wants to merge 4 commits into
Conversation
…stead of wrapping in SendMessageResponse oneof Also updates the JSON-RPC client transport to parse the unwrapped result, since the server no longer nests it under task/message.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/client/transports/jsonrpc.py | 93.29% | 93.40% | 🟢 +0.10% |
| src/a2a/server/routes/jsonrpc_dispatcher.py | 85.44% | 85.21% | 🔴 -0.23% |
| Total | 93.00% | 93.00% | ⚪️ 0.00% |
Generated by coverage-comment.yml
…end in SendMessageResponse
The ITK cross-SDK test caught a real gap: when this SDK's client calls
a peer that hasn't adopted the unwrapped response shape yet (older
SDKs, other language implementations), it now unwraps the legacy
{task: {...}} / {message: {...}} shape explicitly before falling
back to parsing the direct Task/Message payload this SDK's own server
returns.
…send responses Peers that already return the direct Task/Message shape (per the A2A spec) include a top-level "kind" field to disambiguate the two, since that's not encoded via a protobuf oneof on the wire. Our generated Task/Message types don't declare that field, so ParseDict rejected it on both attempts. Strip and use it (falling back to the same messageId/id field-presence heuristic the v0.3 compat transport already uses) before parsing.
…e top level The previous fix only popped "kind" off the root of the result dict. Spec-compliant peers stamp it on every nested Task/Message/Part too (TaskStatus.message, Task.history[], Message.parts[]), so ParseDict still rejected the payload as soon as it recursed into any of those. Added _strip_kind() to clean the whole tree before parsing.
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.
Fixes #1192
Right now message/send on the JSON-RPC transport returns the result nested one level too deep, like {"task": {...}} instead of just the Task object. Turns out jsonrpc_dispatcher.py was wrapping the return value in a SendMessageResponse before serializing it, but that oneof is meant for the streaming StreamResponse, not for the unary send. tasks/get and tasks/cancel already return their payload unwrapped, so message/send was the one behaving differently, and any client that follows the spec and reads result.status or result.artifacts just gets nothing back.
Fixed _handle_send_message to serialize task_or_message directly, same as the get/cancel handlers do.
While testing this I ran into something the original issue didn't mention: the client side transport (JsonRpcTransport.send_message) parses the result straight into a SendMessageResponse object, which only knows about the task/message fields. Once the server stopped wrapping the response, this started throwing a ParseError because fields like id and status don't exist on SendMessageResponse. So a client and server from this same SDK would break talking to each other after the fix.
I extended the client transport to handle this, then kept finding more shapes once I ran it against the ITK cross-SDK suite, which spins up real Go and Python peer implementations instead of mocks. Turns out real peers send the payload in at least three different shapes: the old wrapped {"task": {...}}, the new unwrapped shape this fix produces, and a spec-compliant unwrapped shape that carries a "kind": "task"/"message" discriminator on every Task, Message, and Part in the tree, including status.message, history entries, and message parts, none of which our protobuf-generated types declare. The client now checks for the wrapped shape first, then reads or infers the kind, strips it recursively from the whole payload, and parses into Task or Message accordingly, wrapping the result back into a SendMessageResponse so nothing downstream in BaseClient has to change. The v0.3 compat transport already does something similar for its own wire format, this just applies the same idea to the 1.0 JSON-RPC path.
One thing I want to flag instead of quietly leaving out: two ITK scenarios still fail after all of this, Star Topology Full JSONRPC & GRPC and Push Notification Test JSONRPC & GRPC, both specifically on the leg where current calls go_v10 over unary JSON-RPC. I went through the full raw logs and there is no exception anywhere, current's response to that call just comes back short. The same call to the same peer completes fully over grpc and http_json in the same run. Best explanation I have is that grpc always runs in streaming mode in itk/main.py so it waits for the terminal event, go_v10's REST handler evidently blocks until the task is done, but its JSON-RPC handler returns before the task completes. That is not a response shape bug, it looks like an async lifecycle behavior specific to go_v10's JSON-RPC handler, or a gap in the ITK test agent not polling after a non-terminal response, and either way it lives outside this repo. I do not think it should block this PR since what #1192 asked for is fixed and tested, but wanted to leave a clear trail in case a maintainer wants to chase it further or file it against a2a-go or a2a-itk.
Changed files:
src/a2a/server/routes/jsonrpc_dispatcher.py, the actual fix
src/a2a/client/transports/jsonrpc.py, client side parsing update covering the legacy wrapper, the kind discriminator, and the recursive strip
tests/client/transports/test_jsonrpc_client.py, new tests for the wrapped shape and for kind-discriminated payloads, both flat and nested
tests/server/test_integration.py
tests/integration/test_client_server_integration.py
tests/client/test_auth_interceptor.py
tests/integration/test_tenant.py
those last four just had mocks or assertions updated to match the new unwrapped shape.
Test plan:
ran the full suite, 1795 passed, nothing broken
ruff check and ruff format --check both clean
manually confirmed the round trip test that broke when I only fixed the server side now passes with both sides updated
ran against the ITK cross-SDK suite, went from 8 failing scenarios to 2, the remaining 2 are the go_v10 JSON-RPC timing issue described above, unrelated to response shape.