fix(server): accept trailing-slash JSON-RPC endpoint; enqueue Task in TCK SUT - #1165
fix(server): accept trailing-slash JSON-RPC endpoint; enqueue Task in TCK SUT#1165kuangmi-bit wants to merge 3 commits into
Conversation
🧪 Code Coverage (vs
|
|
Gentle ping on review. This is a small, TCK-unblocking server-side change (part of #666): |
… TCK SUT Two changes that together let the 1.0 TCK exercise the JSON-RPC SUT: 1. create_jsonrpc_routes now registers both the exact rpc_url and its trailing-slash variant. HTTP clients (httpx in particular) normalize an empty request path to a trailing slash, so POST /a2a/jsonrpc/ was previously 404 even though /a2a/jsonrpc worked. This is a protocol compatibility fix: the spec does not mandate one spelling over the other, and a 404 on the trailing-slash form breaks any client that does not strip it. 2. tck/sut_agent.py now enqueues the Task itself (via new_task_from_user_message) before emitting TaskStatusUpdateEvents. The SDK's active-task machinery requires this ordering (InvalidAgentResponseError otherwise), and the 1.0 TCK CORE-SEND tests assert it. Verified against a2a-tck 1.0.0.alpha2 (jsonrpc, must level): 53 failed -> 6 failed before this change, with the remaining failures being SUT feature gaps (artifacts) and one SDK error-code mapping gap, not transport issues.
…card
Two more 1.0 compatibility fixes surfaced by running the REST and gRPC
rows of the TCK:
- protocolBinding 'REST' -> 'HTTP+JSON': the 1.0 TCK's protocol binding
map only recognizes JSONRPC / GRPC / HTTP+JSON. The old name made the
whole REST transport untestable ("No usable transports after filtering").
- gRPC interface url 'http://localhost:50051' -> 'localhost:50051': the
gRPC client treats the url as a channel target; the http:// prefix
fails DNS resolution in grpcio.
Verified against a2a-tck 1.0.0.alpha2 (must level):
- jsonrpc: 6 failed / 67 passed
- http_json (REST): 5 failed / 61 passed
- grpc: 7 failed / 48 passed
Remaining failures are SUT feature gaps (artifact-carrying responses,
MessageResponse variants) plus two status/error-code mappings.
f49d8e4 to
860bd65
Compare
mykytanetipa
left a comment
mykytanetipa
left a comment
There was a problem hiding this comment.
Retracting my earlier approval - it was submitted in error while I was reviewing itk PRs in parallel. Switching to request-changes; see the points below.
| ), | ||
| Route( | ||
| path=f'{rpc_url}/', | ||
| endpoint=dispatcher.handle_requests, | ||
| methods=['POST'], | ||
| ), |
There was a problem hiding this comment.
major: adding a duplicated route is a behavioral change for existing clients that rely on public create_jsonrpc_routes, I would avoid this.
the route mismatch issue should be fixed from the clients side e.g. by using follow_redirects=True on the httpx.Client.
| ) | ||
| ), | ||
| Route( | ||
| path=f'{rpc_url}/', |
There was a problem hiding this comment.
will introduce malformed unreachable path in case of rpc_url='/'
2 participants
Summary
Two changes that let the A2A 1.0 TCK exercise the JSON-RPC SUT (part of the work tracked in #666):
create_jsonrpc_routesaccepts the trailing-slash endpoint variant. HTTP clients (httpx in particular) normalize an empty request path to a trailing slash, soPOST /a2a/jsonrpc/previously returned 404 even though/a2a/jsonrpcworked. The spec does not mandate one spelling over the other, and a 404 on the trailing-slash form breaks any spec-conformant client that does not strip it. Both variants now route to the same dispatcher.tck/sut_agent.pyenqueues the Task itself before emittingTaskStatusUpdateEvents (vianew_task_from_user_message). The SDK's active-task machinery requires this ordering (it raisesInvalidAgentResponseErrorotherwise), and the 1.0 TCKCORE-SEND-*requirements assert it.Verification
Ran the 1.0 TCK (
a2a-tcktag1.0.0.alpha2, jsonrpc transport, must level) against the SUT locally:The remaining 6 failures are not transport issues — they are SUT feature gaps (artifact-carrying responses, DM-ART-001) and one SDK error-code mapping gap (
ContentTypeNotSupportedErrorreported asParseError). Those are follow-ups; this PR fixes the transport-level blockers.Unit tests:
tests/server/routes/96 passed. Ruff clean on touched files.Related