Skip to content

fix(a2a-pb): keep the REQUIRED fields of ListTasksResponse and AgentCard - #416

Closed
gulldan wants to merge 10 commits into
a2aproject:mainfrom
gulldan:fix/protojson-required-fields
Closed

gulldan wants to merge 10 commits into
a2aproject:mainfrom
gulldan:fix/protojson-required-fields

Conversation

@gulldan

@gulldan gulldan commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Stack: #406 → #405 → #407 → #411 → #395 → #414 → this. Stacked on #414: review only the last commit.

Fixes #415.

Problem

proto3-JSON omits defaults, so an empty ListTasks page lost tasks and totalSize, and an extended card without skills or modes lost skills, defaultInputModes, defaultOutputModes and an empty description. §5.7: REQUIRED fields MUST be present.

Fix

The existing ListTasksResponse::normalize_json, which restores nextPageToken, now restores all four REQUIRED fields through a small insert_required helper; AgentCard gets the same for its REQUIRED top-level fields. Present values are never touched, and decoding is unchanged. Nested messages (e.g. an AgentSkill with no tags) are out of scope here — a generator-level emit for REQUIRED fields would cover them, which is a larger change.

Tests

  • New test_empty_list_tasks_response_keeps_every_required_field: an empty page serializes to {"tasks":[],"nextPageToken":"","pageSize":0,"totalSize":0}. On main: {"nextPageToken":""}.
  • New test_agent_card_keeps_empty_required_fields: a card with empty skills/modes/description keeps every REQUIRED key and round-trips. On main: description missing from {"capabilities":{},"name":"agent","version":"1.0.0"}.
  • helloworld-server: ListTasks now answers {"nextPageToken":"","pageSize":50,"tasks":[],"totalSize":0} on JSON-RPC and HTTP+JSON.
  • CI steps on Linux, stable 1.99.1, on this stack — all green (853 tests).

Overlap with the A2A 1.1 stack

Applies cleanly on top of #392.

gulldan and others added 10 commits October 31, 2026 13:20
The JSON-RPC transport never looked at the status of a response. An
error that something in front of the agent answers itself (an
authenticating proxy's 401, a gateway's 429 or 502) carries no
JSON-RPC envelope, so the caller got "failed to parse JSON-RPC
response" and could not tell a credential or rate-limit problem from
a broken server. A streaming call answered that way without a content
type was handed to the SSE parser and came back as an empty stream.

A body that is not a JSON-RPC envelope is now reported with its
status, as the REST transport already does ("HTTP 429 Too Many
Requests: <body>"); a JSON-RPC envelope is still decoded whatever the
status, and a non-success status is never treated as a stream.

Fixes: a2aproject#404

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The streaming test matched on the result and panicked in an `Ok` arm that a
passing run never reaches, so that line stayed uncovered. Taking the error
with `.err().expect()` keeps the assertion and needs no `Debug` on the stream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
google.protobuf.Value carries every number as a double, and
proto_value_to_json_value turned each one back into a JSON float, so
an integer in a data part or in metadata reached the executor, and a
client's decoded response, as 3600.0. serde then refuses it for an
integer type ("invalid type: floating point `3600.0`, expected u32").

A whole number within +-2^53, where a double is exact, is now given
back as an integer; other numbers stay floats. The round-trip tests
that used floats to work around this now use integers, and a new
protojson test decodes a JSON-RPC SendMessage whose data part holds an
integer and deserializes it into u32.

Fixes: a2aproject#399

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
InMemoryTaskStore never removed a task, and TaskStore has no delete,
so a long-running agent kept every task it ever served: 20,000 tasks
with a 4 KiB artifact each held 192 MiB.

InMemoryTaskStore::with_terminal_task_limit(n) keeps at most n tasks
in a terminal state and purges the one that reached its terminal state
earliest. Tasks that are not terminal are never purged, and a purged
task reads as TaskNotFoundError, which A2A 3.3.2 allows for a task
that was completed and purged. new() keeps every task, as before. With
a limit of 1,000 the same run stays at about 20 MiB.

Fixes: a2aproject#401

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
Only the JSON-RPC binding read A2A-Version: HTTP+JSON and gRPC served
a request with `A2A-Version: 99.0`, or with no version at all, as a
1.0 request, although 3.6.2 requires every interface to answer
VersionNotSupportedError and to read an empty value as 0.3.

The JSON-RPC check moves unchanged to middleware::check_a2a_version.
rest_router runs it ahead of every route and, when the header is
absent, reads the A2A-Version query parameter that 3.6.1 allows
instead; GrpcHandler runs it on the metadata of every method. Both
answer with the existing VERSION_NOT_SUPPORTED mapping (400 /
FAILED_PRECONDITION). Tests that call the transports directly now send
the version, as the JSON-RPC ones already do.

Fixes: a2aproject#410

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
`subscribe_to_task` resubscribed only to a task whose execution was
still registered. An execution that ends in an interrupted state
(INPUT_REQUIRED, AUTH_REQUIRED) is unregistered as soon as the executor
returns, and the task-store fallback turned only a terminal stored task
into UnsupportedOperationError, so a stored task that waits for input
was reported as TaskNotFoundError. A2A §3.1.6 allows subscribing to
any non-terminal task, and the stream must open with the task as it
stands.

A subscription that finds no registered execution is now registered
with the execution manager, under its lock and before the store is
read, so an execution that registers from then on attaches it: `start`
subscribes every waiter before the executor runs, so none of that
turn's events is missed, and from there the stream is an ordinary
resubscription. The stream opens with the stored task and ends where
the execution it attached to ends. A missing task is still
TaskNotFoundError, a terminal one still UnsupportedOperationError, and
a waiter dropped first unregisters itself.

Tested with handler unit tests (a turn that asks for input and one that
completes; two subscribers polled only after the next turn ran; a
dropped subscription; the unchanged errors), `just ci` on Linux, and
the official TCK against an agent built on the patched crates, where
STREAM-SUB-001/002 and STREAM-ORDER-002..004 now pass instead of being
skipped.

Fixes: a2aproject#393

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
A subscription to a task between turns waits for the task's next
execution. Cancelling such a task runs no execution, so the cancel's events
never reached the waiting subscription and its stream stayed open after the
task was canceled. CancelTask now hands the task's final snapshot to the
subscriptions still waiting, on success and when the executor's cancel
fails; a terminal snapshot ends their streams. Subscriptions register before
reading the store, so one that read the task before the cancel is waiting by
then, and one that reads it later finds it terminal.

The waiter stream drops a missing attachment with filter_map instead of an
arm no path reaches, and the tests assert whole event sequences.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The proto defines statusTimestampAfter as "greater than or equal to", but
InMemoryTaskStore dropped a task whose status timestamp equalled the bound.
A caller that polls with the timestamp of the last task it saw never gets
that task back. The filter now keeps the bound itself.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
proto3-JSON leaves out fields at their default, so an empty ListTasks page
came out as {"nextPageToken":"","pageSize":50} and an extended card without
skills or modes lost skills, defaultInputModes and defaultOutputModes.
Section 5.7 says REQUIRED fields MUST be present. The ListTasksResponse
normalisation that already restores nextPageToken now restores tasks,
pageSize and totalSize too, and AgentCard gets the same for its REQUIRED
top-level fields.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
@gulldan
gulldan requested a review from a team as a code owner November 1, 2026 15:59
@codecov

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@muscariello

Copy link
Copy Markdown
Member

Superseded by #446, which carries the same commit on an in-repo branch so the stack can be reviewed one commit at a time.

@muscariello muscariello closed this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ProtoJSON drops REQUIRED fields left at their default (empty ListTasks, extended card without skills)

2 participants