Skip to content

fix(server): keep the INPUT_REQUIRED question in task history - #434

Closed
gulldan wants to merge 19 commits into
a2aproject:mainfrom
gulldan:fix/keep-input-required-question
Closed

gulldan wants to merge 19 commits into
a2aproject:mainfrom
gulldan:fix/keep-input-required-question

Conversation

@gulldan

@gulldan gulldan commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Stack: #406 → #405 → #407 → #411 → #395 → #414 → #416 → #418 → #420 → #422 → #424 → #426 → #428 → #430 → #432 → this. Stacked on #432: review only the last commit.

Fixes #433.

Problem

The agent asks for input in the status message of its INPUT_REQUIRED update, and only there. A continuation appended the user's answer to history, and the next status update replaced task.status, so the question was lost: history read [ask, answer].

Fix

prepare_task_for_execution records the pending status.message in history ahead of the user's answer when it continues a task. A message already in history (same messageId) is not added twice.

Tests

  • New test_continuation_keeps_the_question_in_history: the first turn ends INPUT_REQUIRED with "Which file?", the answer completes the task, and history is ["ask", "Which file?", "main.rs"]. On main: ["ask", "main.rs"].
  • CI steps on Linux, stable 1.99.1 (a mirror of ci.yml), on this stack — all green (866 tests); every added coverable line runs under cargo llvm-cov.

Overlap

gulldan and others added 19 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>
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>
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>
An error inside an HTTP+JSON stream went out as a bare JSON-RPC error
object, and the REST client failed to parse it, so the caller saw
"SSE parse error" (-32603) instead of the typed error. Section 11.6 says
HTTP+JSON errors use the {"error": google.rpc.Status} body. The REST
binding now sends that body as the stream event, built by the same code as
a non-streaming error response, and the REST client reads it back into the
typed A2AError, through the same parser it uses for error responses.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The REST transport put `req.id` and `req.task_id` into the URL path as they
were, so `/`, `?`, `#` and `%` in an id changed the request: the id
`../admin?x=1` requested `/a2a/admin?x=1`, and `#` cut `:cancel` off into
the fragment. Every id is now one path segment with everything but the
RFC 3986 unreserved characters percent-encoded.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The HTTP+JSON handlers took their input through axum's Json and Query
extractors, so a malformed query, an undecodable body or a missing
Content-Type was answered by axum before the handler, as a plain-text
400/415/422. HTTP+JSON errors use the google.rpc.Status body (§11.6),
as a2aproject#272 made the JSON-RPC router do. The rejections now go through the
invalid-request-body response the handlers already return; a body over
the size limit keeps its 413.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
`service_params_to_metadata` called `MetadataMap::insert` once per value, and
`insert` replaces the previous value, so only the last one went out:
`A2A-Extensions: [urn:a, urn:b]` reached the server as `urn:b`. The values
are now joined into one comma-separated entry, as A2A §10.2 asks for gRPC
metadata.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The HTTP+JSON binding maps the whole CancelTaskRequest to the body
(`body: "*"`), but both cancel routes built the request with
`metadata: None` without reading it, so a caller's metadata never
reached the executor, while JSON-RPC and gRPC pass it. The body is now
decoded like the other methods' bodies; the path still names the task,
and an empty body is a cancel without metadata.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
An executor error, an event the server cannot apply, or a push failure
with fail_on_error ended the execution without a terminal state, so the
stored task stayed SUBMITTED or WORKING for good: GetTask and ListTasks
kept reporting it in progress and a returnImmediately caller polling it
never saw it end. The failed execution now saves the task as FAILED, with
the sanitized error as its status message, before the error reaches the
live subscribers as before.

Signed-off-by: gulldan <5641375+gulldan@users.noreply.github.com>
The agent asks for input in the status message of its INPUT_REQUIRED
update, and only there. A continuation appended the user's answer to
history and the next status update replaced the status, so the question
was lost: history read [ask, answer]. The continuation now records the
pending status message in history ahead of the answer, once.

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 18:34
@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

Replaced by #462.

@muscariello muscariello closed this Nov 3, 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]: the agent's INPUT_REQUIRED question disappears from task history after the answer

2 participants