refactor(a2a)!: migrate A2A SDK from v0.3 to v1.1 - #3923
waitasecant wants to merge 5 commits into
Conversation
- Updated unit and integration tests. - Migrated to the new API contract
…n A2AServer tests
|
@strandly-the-agent review |
strandly-the-agent
left a comment
There was a problem hiding this comment.
Request changes — a faithful port of the v1 API (216 unit tests green, ruff/mypy clean, no test coverage silently lost), but four grounded blockers, and the change is framed as a non-breaking chore while it changes the wire contract.
- 🔴 The published pin doesn't import.
a2a-sdk>=1.0.0, butadd_a2a_routes_to_fastapionly exists from 1.1.0. Fresh installs resolve 1.1.2, so CI can't see it. - 🔴 Interrupt/failure text is silently dropped in compliant-streaming mode — the caller gets
stop_reason="interrupt"with no prompt. - 🔴
enable_v0_3_compatleftFalse, so every deployed v0.3 client breaks with zero user action — includingstrands-ts, which still pins@a2a-js/sdk ^0.3.10in this same monorepo. - 🔴 The one integration test cannot pass (missing
A2A-Version: 1.0; v1 nests the result undertask). TheRun integration testscheck on this head is stillwaiting— it has never run. - 🟡 Three more inline: int→float coercion reaching user hook code, eager
AgentCardin__init__(undocumented newValueError),artifact_update.appendignored. - 📋 API-review label gate: public surface changes here (
A2AResponsenarrowed,AgentCard.url→supported_interfaces, constructor failure timing) and there's noapi/needs-reviewlabel.griffe— the repo's own detector — flagstypes/a2a.py:9, but its workflow has nopull_requesttrigger, so nothing gated this. Please add the label; the breaking-vs-experimental framing call (Questions #1/#2) shouldn't be settled by one reviewer. - 📄 Docs are broken despite "Documentation PR: None":
agent-to-agent.mdx:428-441importsDataPartand usesPart(root=…), neither of which exists in v1 — and that's the one flow the page says requires the raw client.
Evidence ledger — what was actually verified
- ✅ Branch
review-3923@a256a62b(confirmed as PR head via API before posting), merge baseff2bdd09; diff matches the stated +706/−1556 across 11 files. - ✅
python -m pytest tests/strands/multiagent/a2a/ tests/strands/agent/test_a2a_agent.py -q→ 216 passed (a2a-sdk 1.1.2, py3.13). Re-run at aggregation time. - ✅ Floor, verified from real wheels:
a2a.helpersexportsnew_text_part/new_data_partfrom 1.0.3;a2a.server.routes.add_a2a_routes_to_fastapionly from 1.1.0. True floor = 1.1.0.pip install a2a-sdk==1.0.0→ the a2a test package fails at collection (ImportError). - ✅ Content-loss repro:
[artifact("partial answer"), status_update(INPUT_REQUIRED, msg="…need approval")]→stop_reason='interrupt',content=[{'text': 'partial answer'}]. Status-only control keeps the message. Reproduced end-to-end through a real server+client over ASGI, both streaming modes. - ✅
appendrepro: cumulative artifact re-send (append=False= replace) →[{'text':'Hel'},{'text':'Hello'},{'text':'Hello world'}]. - ✅ int→float repro (full park-then-resume round trip):
1→1.0,[1,2,3]→[1.0,2.0,3.0],{"count":3}→{'count':3.0};1.5/True/'yes'preserved. - ✅ Eager card:
A2AServer(agent=Agent())→ValueError: A2A agent description cannot be None or empty. Afters.agent_card_url = …, the handler's snapshot keeps the old URL whilepublic_agent_cardrebuilds with the new one. - ✅ Integ test repro: exact payload against the real
to_fastapi_app()→ jsonrpc error-32009without theA2A-Versionheader; with it, the result nests undertask. - ✅
COMPLETEDstatus followed byTaskwith unset status →a2a_task_stateregresses to'unspecified'. - ✅
a2a.types.DataPartabsent in v1;AgentCardhas nourlfield.agent-to-agent.mdx:428,439confirmed still using both. - ✅
griffe check --against ff2bdd09(run locally) flagstypes/a2a.py:9 A2AResponseandserver.py:129 request_handler;python-pr-and-push.ymlhas nopull_requesttrigger and no such check-run exists on this head. - ✅
parse-release-body.ts:41keysbreakingoff!in the subject → as titled, this publishes as non-breaking.strands-ts/package.json:196,257pin@a2a-js/sdk ^0.3.10. - ✅ ruff clean on all 11 changed files; mypy: no errors in any a2a file (28 pre-existing
import-not-foundelsewhere). - 🔴 Not verified:
hatch test --all(5-version matrix);tests_integagainst live Bedrock;tests_integ/a2a/test_multiagent_a2a.pyanda2a_server.py(neither touched nor run by this PR). - 🔴 Pass coverage: triage, context-build, issue-alignment, correctness, test-quality, api-bar-raiser all ran as independent passes. The adversarial pass timed out and was dropped; its priority surfaces (event interleavings, precedence, error paths, wire round trip) were covered with runnable repros by the correctness pass, but treat adversarial coverage as partial.
Per-pass breakdown
- Issue alignment (#3922): all six letter items land and are individually verified; ~81% of the 1556 deletions are test churn, not lost behaviour; the two removed base64 tests track a removed branch. Under-delivers on the issue's own compat promise ("without requiring consumers to change") and on the
strands-py/docs/A2A_SDK_V1_MIGRATION_PLAN.mdit claims already exists (no such file in any ref). - Correctness: 3 🔴 / 5 🟡, everything run against the real SDK over ASGI. Verified sound: the
A2AErrorexception contract and all 12 converted raise sites,HasFieldusage throughout, the client↔server round trip, and_task_state_to_strreproducing v0.3 kebab-case for all 8 real states. - Test quality: the unit suite is better than what it replaced (real protobufs, so
oneof/HasFieldsemantics are genuinely exercised rather than faked). 14 mutations, 8 killed, 6 survived — M1 (content precedence) is the one that matters and maps to the 🔴 above. - API bar-raiser:
team/FEATURE_LIFECYCLE.md:23-24makes this a major/breaking change; the "pay for play" exception (:35-54) doesn't apply because nothing is gated behind opt-in.team/COMPATIBILITY.md:61-72allows union extension, andA2AResponseis a narrowing. A2A isn't under the experimental contract as things stand (not instrands/experimental/, in__all__of a stable namespace, zero "experimental" in the docs page) — alogger.infoisn't a versioning contract. - Context build:
convert_response_to_agent_resultrename has zero remaining references anywhere; the deleted_is_complete_eventmachinery is fully private.strands/types/a2a.pyis published surface — the site's generator publishes every non-_module (api-generation-python.py:106-143).
Questions
Blocking (worth answering before merge)
- Is A2A actually under the experimental contract? If yes, should it move to
strands.experimentalor get a docs banner? If no, this needs the full breaking treatment —refactor(a2a)!:, thebreaking changelabel, and the Breaking Changes section thatteam/PR.md:165-167requires. - Is
enable_v0_3_compat=Trueas an internal default for one transition release acceptable, and which release removes it? (Recommend not exposing it as a constructor parameter — a public boolean whose only correct value changes over time is permanent surface bought for a temporary problem.) - Was the compliant-streaming interrupt/failure content loss considered, or did
test_converters.py:203— artifact + terminal status with no message — mask it? - int→float on
DataPartpayloads: coerce whole floats back to int at the boundary, or document it as a v1 protocol constraint? Either wayagent-to-agent.mdxneeds a note. - Who owns getting
strands-tsoff@a2a-js/sdk ^0.3.10, and is a cross-version window acceptable or must they ship together?
Non-blocking
- Is
A2AServer.__init__raising on a description-less agent intended? The head commit message suggests yes; theRaises:section doesn't say so. TASK_STATE_UNSPECIFIED→"unspecified"where v0.3 emitted"unknown"— intentional, or worth aliasing? (Raised by three passes.)- Do
card_modifier/extended_card_modifierresolve per request? If so, wiring the card through one keeps theagent_skills/agent_card_urlsetters meaningful. - Is
A2AResponseworth keeping as a public alias to a vendored protobuf type, or shouldstrands.types.a2abecome_a2ain a follow-up? (Don't solve it by excluding the module from the docs generator — that hides the page while the import path stays public.) - Vehicle for the disclosure:
!+breaking changelabel, or does a wire-protocol break want ateam/designs/note perCONTRIBUTING.md:51? - #3922 references
strands-py/docs/A2A_SDK_V1_MIGRATION_PLAN.md, which exists in no ref — drop the claim or ship the doc?
Reading order
strands-py/pyproject.toml— the pin decides which SDK versions everything below is even valid against. Start here.multiagent/a2a/server.py— the HTTP surface and the public constructor: the hand-rolled route assembly at:239-283(both app factories, and theenable_v0_3_compatdefault), then eager card construction at:140andpublic_agent_cardat:164-190.multiagent/a2a/_converters.py— client-side result reconstruction. Read the docstring at:126-140against the executor's two streaming modes; that's where the content-precedence blocker lives.agent/a2a_agent.py+types/a2a.py— how the converters are driven, and the publicA2AResponsenarrowing that griffe flags.multiagent/a2a/executor.py— biggest diff, most mechanical: exception contract (ServerError→ bareA2AErrorsubclasses) and thePartoneof migration.:676is the one place with a behavioural surprise.- Tests —
tests_integ/test_a2a_executor.pyfirst (it can't pass), thentests/strands/multiagent/a2a/test_converters.py:203for the test that masks the precedence bug.
Appendix — non-blocking (13)
Correctness / behaviour (⚪)
_converters.py:119— the last-chunk sentinelnew_text_part("")(executor.py:519) round-trips as a trailing{"text": ""}block;str(result)gains a blank line. Harmless in graph use, but an empty text block is invalid input to BedrockConverse, so appendingresult.messageto a conversation surprises the caller. Filtering empty text in_parts_to_contentfixes it. (Compliant-streaming only. The rewrite fixed a worse v0.3 bug here.)_converters.py:113—response.task.status.statereads0even whenstatusis unset, so atasksnapshot without status resets an already-observedcompletedto"unspecified". Gate onresponse.task.HasField("status"). Only a non-Strands peer emits this. Same fix should harmonizeif task_state(:175) vsif task_state is not None(:184) on the same variable.a2a_agent.py:166-169— the whole response stream is buffered for the call's lifetime; in compliant mode that's one proto per text delta. Per-event yielding is preserved (checked), so no added latency — folding the four content lists incrementally inside this loop removes the buffer and is also where the precedence andappendfixes naturally live.server.py:253—app_kwargs={"routes": [...]}is now aTypeError(got multiple values for keyword argument 'routes'), though:246documentsapp_kwargsas "additional keyword arguments to pass to the Starlette constructor" androutesworked underA2AStarletteApplication.build(**kwargs).server.py:252vs:280— the two app factories now disagree on HEAD: Starlette serves the card path as{GET, HEAD}, FastAPI as{GET}only. Driven by the SDK's helpers; pre-existing: unclear._converters.py:35—TASK_STATE_UNSPECIFIED→"unspecified"where v0.3 emitted"unknown": a silent value change in a string users compare against. One-line alias or one-line disclosure. (See Question 7.)
API / design (⚪)
server.py:185— teachingsupported_interfaces[0].urlbakes in "index 0 is the one you want"; the SDK already has REST routes, so a second interface would silently break every[0]reader. Point users atagent_card_url(the #2003 property) instead.pyproject.toml—protobufis now an implicit runtime dependency (executor.pyimportsgoogle.protobuf.json_formatdirectly, declared only transitively via a2a-sdk). Worth declaring.- Dropping legacy
/.well-known/agent.json: agreed, don't hand-roll it back — upstream deleted it deliberately (already# TODO: removein 0.3.26). But say so in the PR body. - Comments that narrate the migration rather than the invariant, against root
AGENTS.md:57:_converters.py:31-32,:129-130,executor.py:337. That reasoning belongs in the PR description.
Tests (⚪)
tests_integ/test_a2a_executor.py:78-96test_a2a_executor_image_roundtripis theatre: it base64-encodes and decodes with the stdlib and asserts the bytes match, never touching executor code — and it sits exactly where this PR deletes the executor's base64 decode. Replace it with the assertion that matters (a real PNG throughPart(raw=…)arriving byte-identical), and ideally with the highest-value missing test: park an interrupt and resume it over JSON-RPC. The resume payload's wire shape changed and nothing crosses HTTP today.- Surviving mutations with no test owed elsewhere:
Role.ROLE_USERon the content-blocks input path unasserted (test_converters.py:51checks only part count); theisinstance(data, dict)guard atexecutor.py:677unpinned (new_data_part(3.14)/new_data_part(["a", 1])build exactly such parts);test_executor.py:529,594assertisinstance(result, list)whereresult == []is what the names promise;_parts_to_contenthas no direct test; the route set is unasserted (/.well-known/agent.jsonnow 404s, was 200 in 0.3.26 — a one-line assertion makes the drop deliberate); nothing pins the version floor. task.status.messagenever read → empty result on a non-streaming Task reply: pre-existing, filed #4019.
Housekeeping
- #3313 (Dependabot's
a2a-sdkpin widening, still open) is superseded by this PR and should be closed with it.
| "a2a-sdk>=1.0.0,<2.0.0", | ||
| "a2a-sdk[sql]>=1.0.0,<2.0.0", |
There was a problem hiding this comment.
🔴 The declared floor doesn't import — a2a-sdk>=1.0.0 is not a version this code runs on.
add_a2a_routes_to_fastapi (imported in server.py) first appears in 1.1.0; new_text_part/new_data_part (imported in executor.py) are only re-exported from a2a.helpers in 1.0.3. Verified against the real wheels:
1.0.0–1.0.2 a2a.helpers: missing | a2a.server.routes: missing
1.0.3 helpers OK | still missing
1.1.0+ both present
Any install whose resolver lands on 1.0.x — an existing a2a-sdk~=1.0 constraint, an older lockfile, pip backtracking — gets an ImportError importing strands.multiagent.a2a or the client-only strands.agent.a2a_agent; fresh installs resolve 1.1.2, so CI never exercises the floor.
Suggestion: raise both bounds. Cost: a published lower bound can't be raised later without that itself being a breaking dependency change, so this is cheapest before release.
| "a2a-sdk>=1.0.0,<2.0.0", | |
| "a2a-sdk[sql]>=1.0.0,<2.0.0", | |
| "a2a-sdk>=1.1.0,<2.0.0", | |
| "a2a-sdk[sql]>=1.1.0,<2.0.0", |
There was a problem hiding this comment.
Addressed as suggested.
| elif response.HasField("message"): | ||
| message_content = _parts_to_content(response.message.parts) | ||
|
|
||
| content = artifact_content or status_message_content or task_content or message_content |
There was a problem hiding this comment.
🔴 A terminal status message is silently dropped whenever any artifact content exists — the caller gets stop_reason="interrupt" with no prompt text.
[artifact("partial answer"), status_update(INPUT_REQUIRED, msg="…need approval")]
→ stop_reason='interrupt' content=[{'text': 'partial answer'}] # prompt gone
[status_update alone]
→ content=[{'text': 'Agent requires input: …need approval'}] # control
Reached from A2AAgent.invoke_async() (and graph nodes) against A2AServer(enable_a2a_compliant_streaming=True) whose agent interrupts or fails: requires_input/failed put the actionable text in the terminal status message after artifact deltas have streamed, and the server warns users on every request to turn that flag on. v0.3 surfaced this text. The rationale in the docstring above holds for legacy WORKING narration but not for a terminal message.
Suggestion: capture a status message whose status.state is in _STATE_TO_STOP_REASON into its own accumulator and append it here, leaving non-terminal messages as today's fallback. A blanket precedence swap is not the fix — it drops the streamed answer instead. Cost: one extra local, plus the missing test (nothing covers artifact + status with a message — that mutation survives the suite).
There was a problem hiding this comment.
Replaced the flat artifact_content or status_message_content or task_content or message_content chain with a _ResponseAccumulator that tracks artifact content and terminal-status-message content separately.
Added a test covering artifact deltas + a terminal input_required message together, plus an end-to-end integ test (test_a2a_agent_interrupt_round_trip) that asserts the interrupt question text survives through A2AAgent.
| **app_kwargs or {} | ||
| ) | ||
| routes = create_agent_card_routes(self.public_agent_card) | ||
| routes.extend(create_jsonrpc_routes(self.request_handler, rpc_url="/")) |
There was a problem hiding this comment.
🔴 Every deployed v0.3 A2A client breaks with zero user action, and the SDK's one-line opt-out is unused.
The enable_v0_3_compat=False default leaves the v1 dispatcher's JSONRPC03Adapter (message/send, message/stream, tasks/get, tasks/cancel, …) switched off, so those methods now return MethodNotFound. This PR's own integ test had to switch message/send → SendMessage, which is the break in the diff.
Reached by any existing remote client of an upgraded Strands server — including strands-ts, which still pins @a2a-js/sdk ^0.3.10 in this same monorepo, so a TS A2AAgent cannot talk to a Python A2AServer within one release of the same product. And as titled chore(a2a):, the generated changelog will assert this release is non-breaking.
Suggestion: turn the flag on internally for one transition release (apply to the FastAPI path below too) — it's additive, v1 method names keep working, and upstream ships it precisely for this migration; recommend not adding a public constructor knob. If you keep the break instead, it needs the breaking-change treatment: refactor(a2a)!:, the breaking change label, api/needs-review, and the Breaking Changes section from team/PR.md.
| routes.extend(create_jsonrpc_routes(self.request_handler, rpc_url="/")) | |
| routes.extend(create_jsonrpc_routes(self.request_handler, rpc_url="/", enable_v0_3_compat=True)) |
There was a problem hiding this comment.
Addressed as suggested.
| assert response.status_code == 200 | ||
| response_data = response.json() | ||
| assert "completed" == response_data["result"]["status"]["state"] | ||
| assert "TASK_STATE_COMPLETED" == response_data["result"]["status"]["state"] |
There was a problem hiding this comment.
🔴 This rewritten test cannot pass — two independent v1 wire errors, and it has never run (the Run integration tests check on this head is still waiting).
Ran the exact payload against the real A2AServer.to_fastapi_app():
as written (no A2A-Version header): HTTP 200, jsonrpc error -32009
"A2A version '0.3' is not supported by this handler. Expected version '1.0'."
with A2A-Version: 1.0 : HTTP 200, result oneof key = ['task']
A hand-rolled client must send A2A-Version: 1.0 — a missing header is treated as 0.3 and rejected, and neither requests nor the unchanged requests.post(...) call above adds it. Then result nests under task in v1, so this line and the ["artifacts"] loop below both KeyError. Hits whoever next runs hatch test tests_integ. Also: status_code == 200 proves nothing here — JSON-RPC errors are HTTP 200 — and the try/except Exception: pytest.fail(...) wrapper at :74-75 hides the traceback; assert "error" not in response_data and drop the wrapper.
Suggestion (plus add "A2A-Version": "1.0" to the headers in the requests.post call above, and read ["result"]["task"]["artifacts"] below):
| assert response.status_code == 200 | |
| response_data = response.json() | |
| assert "completed" == response_data["result"]["status"]["state"] | |
| assert "TASK_STATE_COMPLETED" == response_data["result"]["status"]["state"] | |
| assert response.status_code == 200 | |
| response_data = response.json() | |
| assert "error" not in response_data | |
| assert "TASK_STATE_COMPLETED" == response_data["result"]["task"]["status"]["state"] |
There was a problem hiding this comment.
Addressed as suggested. Added A2A-Version: 1.0 to the request headers.
| for part in parts: | ||
| part_root = part.root | ||
| data = part_root.data if isinstance(part_root, DataPart) else None | ||
| data = MessageToDict(part.data) if part.HasField("data") else None |
There was a problem hiding this comment.
🟡 Every integer in a resumed interrupt payload arrives in user code as a float.
Verified through a full park-then-resume round trip; the value below is what the BeforeToolCallEvent hook's event.interrupt(...) returns:
sent 1 -> hook received 1.0 sent [1, 2, 3] -> [1.0, 2.0, 3.0]
sent 42 -> hook received 42.0 sent 1.5/True/'yes' -> preserved
sent {'count': 3, 'ok': True} -> {'count': 3.0, 'ok': True}
Reached by any served agent whose interrupt or data payload contains integers, resumed over A2A. v0.3 carried DataPart.data as a JSON-parsed dict and preserved ints; interrupt-id matching is unaffected (ids are strings), but isinstance(r, int), range(r), indexing and int-typed schema validation all break — and the float is what gets persisted into session state. The same coercion hits the [Structured Data] text further down.
Suggestion: it's inherent to protobuf Value, so the ask is a deliberate call rather than silence — either coerce whole floats back to int at this boundary, or document it as a v1 protocol constraint with a note in agent-to-agent.mdx. Cost of coercing: a genuine 1.0 float sent by a peer would arrive as 1; documenting has no code cost.
There was a problem hiding this comment.
This is inherent to protobuf Value (no integer type), not something we introduce. Rather than coerce (which would silently turn a genuine 1.0 from a peer into 1), I documented it as a v1 wire-format constraint: added a Note: section to _extract_interrupt_responses's docstring, and a test (test_execute_interrupt_response_numeric_values_arrive_as_float) that pins the behavior.
| queue_manager=queue_manager, | ||
| push_config_store=push_config_store, | ||
| push_sender=push_sender, | ||
| agent_card=self.public_agent_card, |
There was a problem hiding this comment.
🟡 A2AServer.__init__ now raises where it previously constructed fine, and the new exception isn't in its Raises:.
A2AServer(agent=Agent()) -> ValueError: A2A agent description cannot be None or empty
Evaluating public_agent_card here moves card validation into construction, and Agent().description defaults to None — previously this failed later, at to_fastapi_app()/card access. Reached by A2AServer(agent=Agent(...)) without an explicit description; tests_integ/a2a/a2a_server.py:10 survives only because it sets one. __init__'s Raises: lists only the agent/agent_factory and max_contexts errors, and the repo requires complete Raises: on public functions (strands-py/AGENTS.md).
Fail-fast is arguably the better DevX and v1's DefaultRequestHandler requires a card, so some snapshot is forced — the cost worth naming is that it's a snapshot: after s.agent_card_url = …, the handler keeps the old value while public_agent_card rebuilds with the new one (verified; inert today since the handler only reads the static capabilities, but the setters and the handler now disagree).
Suggestion: add the ValueError to __init__'s Raises: with one migration line about the timing change, and either re-assign the handler's card from the agent_skills/agent_card_url setters or deprecate them. Cost: none for the docstring; the setter fix is a few lines.
There was a problem hiding this comment.
Documented - added the ValueError to __init__'s Raises: section.
On the setter/snapshot concern: Tracing DefaultRequestHandler._agent_card confirmed it's only read for capabilities.streaming/capabilities.push_notifications/capabilities.extended_agent_card - never .url/.skills/.name/.description - and empirically confirmed agent_card_url/agent_skills set after construction are correctly reflected.
| task_state = state | ||
|
|
||
| if response.HasField("artifact_update"): | ||
| artifact_content.extend(_parts_to_content(response.artifact_update.artifact.parts)) |
There was a problem hiding this comment.
🟡 artifact_update.append is ignored, so a server that re-sends cumulative artifacts duplicates its whole answer.
server re-sends the full artifact each turn (append=False, i.e. replace):
content=[{'text': 'Hel'}, {'text': 'Hello'}, {'text': 'Hello world'}]
append=False means replace in the A2A schema, but every update unconditionally extends. Reached by A2AAgent against any third-party A2A server that streams cumulative rather than delta artifacts — interop with non-Strands peers is the client wrapper's whole purpose. The docstring's assumption ("a non-compliant server … sends the full text once") holds for A2AServer, not for arbitrary peers; last_chunk is likewise unused.
Suggestion: key accumulation on artifact.artifact_id and reset that entry when append is false. Cost: a dict keyed by artifact id instead of a flat list — which also handles multi-artifact responses, and pairs naturally with folding accumulation into the stream_async loop (removing its full-stream buffer).
There was a problem hiding this comment.
Fixed as part of the same _ResponseAccumulator change.
| a2a = [ | ||
| "a2a-sdk>=0.3.0,<0.4.0", | ||
| "a2a-sdk[sql]>=0.3.0,<0.4.0", | ||
| "a2a-sdk>=1.0.0,<2.0.0", |
There was a problem hiding this comment.
concern — this floor can't import. Two of the new imports do not exist at 1.0.0.
from a2a.server.routes import add_a2a_routes_to_fastapi(server.py:14). This lands ina2a/server/routes/fastapi_routes.pyin 1.1.0. At 1.0.0, 1.0.1, 1.0.2 and 1.0.3,routes/__init__.pyexports onlycreate_agent_card_routes,create_jsonrpc_routes,create_rest_routesand the two context builders.from a2a.helpers import new_data_part, ..., new_text_part(executor.py:21). Both are defined inproto_helpers.pyfrom 1.0.2, but only re-exported from thea2a.helperspackage in 1.0.3. At 1.0.2 the package__init__never imports them, sofrom a2a.helpers import new_text_partstill raises.
So any resolver that picks 1.0.0–1.0.3 fails at import strands.multiagent.a2a with ImportError. Your environment presumably has 1.1.x installed, which is why hatch test is green.
Please bump both entries to >=1.1.0,<2.0.0. The PR and issue titles are worth renaming too — this is a migration to 1.1, not 1.0.
There was a problem hiding this comment.
Thanks, I totally missed it because after adding this constrain, the dependency manager installed the latest version 1.1.2 so CI never caught it. I'll bump to >=1.1.0,<2.0.0 would ensure the missing functions being present.
I'll rename the issue and PR title and remove any false claim in the description.
| await self._execute_streaming(context, updater) | ||
| except ServerError: | ||
| # Re-raise ServerErrors (setup failures like missing input) | ||
| except A2AError: |
There was a problem hiding this comment.
concern — this catches far more than the ServerError it replaces, and it costs the failed transition.
In 0.3, ServerError was server-side only; a2a.client.errors.A2AClientError subclassed plain Exception. In 1.x it does not:
# a2a/client/errors.py @ v1.1.0
from a2a.utils.errors import A2AError
class A2AClientError(A2AError): ...
class AgentCardResolutionError(A2AClientError): ...
class A2AClientTimeoutError(A2AClientError): ...A Strands agent running inside A2AServer that calls a remote A2A agent — an A2AAgent graph node, or an agent-as-tool — raises A2AClientTimeoutError / A2AClientHTTPError / AgentCardResolutionError on any remote hiccup. Those are all A2AError now, so they hit this branch, escape execute(), and skip the updater.failed(...) below. The task never reaches a terminal state, and the client watches it sit in working until it gives up. Under 0.3 the same failure was a plain Exception, hit except Exception, and the task was correctly marked failed.
Catching what the docstring actually describes keeps the old behaviour:
except (InvalidParamsError, InternalError, UnsupportedOperationError):
raiseA test along the lines of "agent raises an a2a client error → task transitions to failed" would pin it.
There was a problem hiding this comment.
Sure, will do and add a test to pin it.
| elif response.HasField("message"): | ||
| message_content = _parts_to_content(response.message.parts) | ||
|
|
||
| content = artifact_content or status_message_content or task_content or message_content |
There was a problem hiding this comment.
concern — input_required loses the one thing the caller needs.
The docstring above treats status messages as duplicate narration. True for a terminal completed; not true for input_required / auth_required, where the status message is the payload. _handle_interrupt_result puts "what input is needed" in a text part and the interrupt ids in a data part, then calls updater.requires_input(...).
With enable_a2a_compliant_streaming=True the stream is:
task(submitted)artifact_update× N — the agent's text deltasstatus_updateTASK_STATE_INPUT_REQUIRED, carrying the question
artifact_content is non-empty from step 2, so or short-circuits and step 3's question is dropped. The caller gets stop_reason="interrupt" with content that never says what is wanted. Any agent that emits text before hitting an interrupt lands here — which is the normal case, since interrupts are raised from tools.
0.3 did not have this problem: _is_complete_event selected the last complete event, which was the input_required status update, so the question was the content.
Appending instead of falling back when the final state is an interrupt would restore it:
content = artifact_content or task_content or message_content
if _STATE_TO_STOP_REASON.get(task_state) == "interrupt" or not content:
content = content + status_message_contentOn tests: test_artifact_update_then_status_update_does_not_duplicate covers artifact + empty terminal status. The uncovered combination is artifact + input_required-carrying-a-message, which is precisely what the interrupt feature emits. Worth adding whichever way you resolve the behaviour.
There was a problem hiding this comment.
Will make a combined fix for this as well as #3923 (comment)
| queue_manager=queue_manager, | ||
| push_config_store=push_config_store, | ||
| push_sender=push_sender, | ||
| agent_card=self.public_agent_card, |
There was a problem hiding this comment.
concern — building the card here changes what A2AServer.__init__ does, and the tests were rewritten to match rather than the change being called out.
public_agent_card validates name and description, and agent_skills walks tool_registry.get_all_tools_config(). Doing that in __init__ means:
A2AServer(agent)now raisesValueErrorfor an agent with no description, andKeyErrorfor a malformed tool config, where before it constructed fine and failed later atto_starlette_app()/serve().Agent.descriptiondefaults toNone, so this is easy to hit.test_skills_not_loaded_during_initializationused to assert the tool registry is not touched during init (side_effect=Exception("Should not be called during init")). It now asserts the opposite. That is a real invariant being dropped — an expensive registry is now walked at construction.- The handler holds a snapshot.
agent_card_url,agent_skillsandcapabilitiesall have public setters, butrequest_handler._agent_cardkeeps the card built on this line, and 1.x gates on it (_agent_card.capabilities.streaming,.push_notifications,.extended_agent_card). Setcapabilitiesafter construction and the served card advertises a capability the handler then refuses.
DefaultRequestHandler does require agent_card, so something has to give here. Either document that these attributes must be set before construction, or hand the handler a card it re-reads. Either way it needs a release note, since the PR body says the public API is unchanged.
There was a problem hiding this comment.
-
Agreed this is a behavioral change.
Agent.descriptiondefaults toNone, soA2AServer(agent)without an explicit description now raises at construction rather than atserve()/to_*_app()time. I'll document this in theRaises:docstring - it was previously a latent error that surfaced in a less obvious place. -
test_skills_not_loaded_during_initialization: You're right that the test was rewritten to match rather than the invariant being preserved. However, with the factory pattern, the representative agent is already built at__init__time (line ~127:agent_factory(_AGENT_CARD_CONTEXT_ID)), so the registry already exists. Walkingget_all_tools_config()is O(n) on registered tools — not a network call or heavy operation. If you'd prefer to restore the lazy invariant I can defer skill resolution topublic_agent_cardaccess time (it's already a property), but the card must be built for the handler at construction. -
Handler snapshot vs. setters: Tracing
DefaultRequestHandlersource -self._agent_cardis only read in two@validatedecorators:_agent_card.capabilities.streamingand_agent_card.capabilities.push_notifications. It never reads.url,.skills,.name, or.descriptionfrom its stored copy. The card-serving routes are built into_starlette_app()/to_fastapi_app()which callself.public_agent_card(a property) at that point - soagent_card_urlandagent_skillssetters do work correctly when set beforeto_*_app(). -
capabilitiesdoesn't actually have a public setter - it's a bare attribute assigned in__init__( self.capabilities = AgentCapabilities(streaming=True)), not a@property. Although, since it is a public attribute, someone could reassign it. I'll make it private (self._capabilities) and expose it read-only via a property if you'd like to close that gap explicitly - or just document that it's set at construction and shouldn't be mutated afterward. -
I'll update the PR description to clarify: "public method API unchanged, but
__init__now validates eagerly (fail-fast) and thecapabilitiesattribute must be set at construction."
| **app_kwargs or {} | ||
| ) | ||
| routes = create_agent_card_routes(self.public_agent_card) | ||
| routes.extend(create_jsonrpc_routes(self.request_handler, rpc_url="/")) |
There was a problem hiding this comment.
concern — two 0.3-compatible surfaces disappear here, both silently.
/.well-known/agent.json now 404s. 0.3 served the legacy path alongside the new one — PREV_AGENT_CARD_WELL_KNOWN_PATH = '/.well-known/agent.json', handled in jsonrpc_app.py. 1.1 removed the constant, and create_agent_card_routes registers exactly one route at AGENT_CARD_WELL_KNOWN_PATH. The test diff is the evidence: every /.well-known/agent.json assertion had to become agent-card.json. Anything pinned to the old discovery URL breaks on upgrade. Keeping it is cheap — one extra Route with the old path.
enable_v0_3_compat is left at its False default. The parameter is documented as "Whether to enable v0.3 backward compatibility on the same endpoint." Off, the RPC endpoint stops speaking 0.3. The integ test shows how far the wire moved: "message/send" → "SendMessage", "completed" → "TASK_STATE_COMPLETED".
Note the SDK still injects 0.3 fields into the card — agent_card_to_dict merges to_compat_agent_card, which is the only reason the agent_data["url"] assertions in test_serve_at_root_alb_scenarios still pass after url moved into supported_interfaces. Upstream clearly still expects 0.3 clients in the wild.
Passing enable_v0_3_compat=True would keep them working. If cutting them off is deliberate, that is a reasonable call for a major migration — but it belongs in the PR description, not only in a test diff.
There was a problem hiding this comment.
Yes, this was miss. I'll pass enable_v0_3_compat no need to cut them off.
| ) | ||
| routes = create_agent_card_routes(self.public_agent_card) | ||
| routes.extend(create_jsonrpc_routes(self.request_handler, rpc_url="/")) | ||
| a2a_app = Starlette(routes=routes, **(app_kwargs or {})) |
There was a problem hiding this comment.
concern — app_kwargs quietly means something different now, and two plausible values crash.
0.3 forwarded it to build():
def build(self, agent_card_url=AGENT_CARD_WELL_KNOWN_PATH, rpc_url=DEFAULT_RPC_URL,
extended_agent_card_url=EXTENDED_AGENT_CARD_PATH, **kwargs) -> Starlette:
app = Starlette(**kwargs)So app_kwargs accepted rpc_url, agent_card_url and extended_agent_card_url, and passed everything else through. It now goes straight to the constructor:
to_starlette_app(app_kwargs={"rpc_url": "/a2a"})→TypeError: __init__() got an unexpected keyword argument 'rpc_url'to_starlette_app(app_kwargs={"routes": [...]})→TypeError: got multiple values for keyword argument 'routes', becauseroutes=routesis already bound on this line. That one used to work:build()created the app first, then added routes.
rpc_url is also pinned to "/" with no way to override it. The docstring says "Additional keyword arguments to pass to the Starlette constructor," which is now literally accurate but no longer matches what callers were passing.
Either split the known keys back out, or note the move in the release notes. There is no test covering app_kwargs on either builder, so nothing catches this.
There was a problem hiding this comment.
Added a runtime guard (_validate_app_kwargs) that rejects the known v0.3 build() keys with an actionable error message pointing to the migration path:
rpc_url→ usehttp_urlconstructor param for path-based mounting (the sub-app mounts at the path, RPC is at root within it)agent_card_url→ use theagent_card_urlproperty setter before callingto_*_app()routes→ add custom routes to the returned app object afterto_starlette_app()/to_fastapi_app()extended_agent_card_url→ not currently exposed; can add if there's demand
I considered splitting the known keys back out as explicit params on to_starlette_app()/to_fastapi_app(), but those concerns already have homes at the constructor level (http_url handles path mounting, agent_card_url has a setter). Re-accepting them on the app builders would duplicate configuration surface and create ambiguity about which one wins if both are set. A clear rejection with migration guidance keeps the API surface narrow.
Note this in the release notes as a breaking change for callers passing those keys via app_kwargs.
| for part in parts: | ||
| part_root = part.root | ||
| data = part_root.data if isinstance(part_root, DataPart) else None | ||
| data = MessageToDict(part.data) if part.HasField("data") else None |
There was a problem hiding this comment.
consider — every number in a data part comes back as a float now.
Part.data is a google.protobuf.Value (data: _struct_pb2.Value in a2a_pb2.pyi), and new_data_part does ParseDict(data, struct_pb2.Value()). Value numbers are doubles, and MessageToDict renders them through _ValueMessageToJsonObject → number_value → Python float.
So a client resuming with
{"interruptResponse": {"interruptId": "abc", "response": 3}}resumes the agent with 3.0, not 3. 0.3's Pydantic DataPart.data came from JSON parsing and kept the int. Same on the generic path at line 812 — json.dumps(MessageToDict(part.data), indent=2) renders {"count": 3.0} into the prompt text the model reads.
This is the v1 wire format's limitation rather than something you introduced, so I am not asking for a coercion layer. But this method is otherwise very precise about the resume contract — null responses, duplicate ids and mixed parts are each rejected explicitly — and a silent int→float is not visible to anyone reading it. A line in the _extract_interrupt_responses docstring plus one test that resumes with a non-string response value would save someone finding this in a debugger.
There was a problem hiding this comment.
Sure I'll update doctring and test.
| assert response.status_code == 200 | ||
| response_data = response.json() | ||
| assert "completed" == response_data["result"]["status"]["state"] | ||
| assert "TASK_STATE_COMPLETED" == response_data["result"]["status"]["state"] |
There was a problem hiding this comment.
consider — the part of this PR most likely to regress has no end-to-end assertion.
convert_responses_to_agent_result is a from-scratch rewrite of how the final answer is reconstructed, and it is only exercised against hand-built StreamResponse objects. No round-trip test reaches it:
- this test covers the non-streaming
SendMessagepath only tests_integ/a2a/test_multiagent_a2a.pydoes runA2AServer→A2AAgent, but only assertsstop_reason == "end_turn", never the contenttests_integ/a2a/a2a_server.pybuildsA2AServer(agent=agent, host=..., port=...), soenable_a2a_compliant_streamingstaysFalse. The compliant path — where artifacts arrive as deltas and accumulation order actually matters — has no round-trip coverage at all
Asserting the reply text through a real server, once with enable_a2a_compliant_streaming=True and once with False, would have caught the interrupt drop at _converters.py:174 and would catch any future chunk ordering or duplication problem. An interrupt round trip — park on input_required, resume with a data part — would be worth having too, given how much of the executor exists to serve that path.
There was a problem hiding this comment.
Sure would add.
- Narrow except clause to (InvalidParamsError, InternalError, UnsupportedOperationError) so client errors trigger failed transition - Introduce _ResponseAccumulator to preserve terminal status messages alongside artifact content (fixes input_required losing the question) - Add _coerce_whole_floats to restore int round-trip through protobuf - Pass enable_v0_3_compat=True in both app factories - Document eager AgentCard validation, app_kwargs constraints, and setter semantics in docstrings - Bump a2a-sdk floor to >=1.1.0 - Update agent-to-agent.mdx to v1 API - Add tests for client error → failed transition, float coercion, artifact+interrupt accumulation, and interrupt resume over HTTP
- Add _validate_app_kwargs guard that rejects v0.3 build() keys (routes, rpc_url, agent_card_url, extended_agent_card_url) with an actionable migration message - Remove _coerce_whole_floats helper; document int→float as a v1 wire-format limitation in _extract_interrupt_responses docstring - Update test to assert numeric values arrive as float - Add end-to-end integration tests exercising convert_responses_to_agent_result through A2AAgent with both streaming modes and interrupt round-trip
A2A peers may reply with a completed Task carrying text only in task.status.message (no artifacts). The converter previously sourced task_content exclusively from task.artifacts, dropping the answer. Now falls back to task.status.message.parts when artifacts are empty, matching the A2A spec which treats artifacts as optional.
Description
Migrates the A2A multiagent integration from
a2a-sdkv0.3 (Pydantic models,message/sendRPC, event tuples) to v1.1.0 (protobuf wire format,SendMessageRPC, flatStreamResponseevents).Key changes
a2a-sdk>=0.3.0,<0.4.0→>=1.1.0,<2.0.0_ResponseAccumulatordataclass that processes the fullStreamResponsesequence — fixes interrupt message loss and no-artifact task fallbackTextPart,DataPart,FilePart,ServerError) → v1.1 protobuf accessors (part.HasField("text"),new_text_part(), direct error types)A2AStarletteApplication/A2AFastAPIApplicationwrappers, usescreate_jsonrpc_routes()/add_a2a_routes_to_fastapi()directly. Added_validate_app_kwargs()to reject stale v0.3 build keys with actionable errorsagent-to-agent.mdxcode examples for v1.1 APIBug fixes included
A2AAgentcorrectly extracts content from tasks with no artifacts (third-party server compat)Related Issues
Closes #3922 #4019
Documentation PR
Updated
site/src/content/docs/user-guide/concepts/multi-agent/agent-to-agent.mdxto reflect v1.1 API changes (included in this PR).Type of Change
Breaking change
to_starlette_app(**kwargs)/to_fastapi_app(**kwargs)reject v0.3 keys (http_url,endpoint,agent_card)A2AServer(http_url=...),server.agent_card_url = ..., or configure the returned app directlyNOT breaking for v0.3 clients:
enable_v0_3_compat=True(default) routes v0.3 method names (message/send) to the upstream compat adapter — noA2A-Versionheader required.Testing
How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.