Repository navigation
Conversation
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>
ListTasks returned every stored artifact whatever includeArtifacts said. Section 3.1.4 of the spec makes the default false and says the artifacts field MUST be omitted entirely then. DefaultRequestHandler::list_tasks now drops them unless the request sets includeArtifacts to true, next to the historyLength trim, so any TaskStore gets the same behaviour. Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
HttpPushSender sent an Authorization header only for Bearer and Basic and silently dropped every other scheme, so a webhook expecting e.g. DPoP got an unauthenticated request. Section 4.3.3 says the agent MUST include the credentials of TaskPushNotificationConfig.authentication. Any scheme is now sent as "<scheme> <credentials>"; Bearer and Basic keep their canonical spelling, and a scheme that is not a single token is rejected like CR/LF in the credentials. Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
gulldan
force-pushed
the
fix/push-any-auth-scheme
branch
from
November 1, 2026 16:54
4abf16a to
6880e73
Compare
This was referenced Nov 1, 2026
Member
|
Superseded by #448, which carries the same commit on an in-repo branch so the stack can be reviewed one commit at a time. |
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
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 #419.
Problem
HttpPushSendersentAuthorizationonly for Bearer and Basic and silently dropped every other scheme. §4.3.3: the agent MUST include the configured credentials.Fix
Any scheme is sent as
<scheme> <credentials>;bearer/basickeep their canonicalBearer/Basicspelling as before. The scheme opens the header, so one that is empty or not a single token (whitespace, CR/LF) is rejected withInvalidParams, next to the existing CR/LF check on the credentials (BUG-34).Tests
test_send_push_sends_credentials_of_any_scheme: a local webhook recordsAuthorizationforbearer,BasicandDPoP. Onmainit fails atscheme DPoP.test_send_push_rejects_a_scheme_that_is_not_one_token:"","Bearer x"and a CR/LF scheme areInvalidParams.test_send_push_with_unknown_auth_schemeis replaced by the first test: it asserted onlyOkagainst a closed port.Overlap with the A2A 1.1 stack
The stack touches
sender.rsand keeps the same Bearer/Basic match; this diff conflicts textually there. Rebased on request.