From ae8a916bfeb5b5a956bdff7fad768c3fd2b26426 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sat, 9 May 2026 04:30:04 -0400 Subject: [PATCH] fix(webhooks): add canceled/rejected/auth-required to A2A status map; fail fast on unknowns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #603. create_a2a_webhook_payload silently fell back to TASK_STATE_UNSPECIFIED (proto3 integer 0) for any AdCP status not in the lookup map — canceled, rejected, auth_required, and any genuinely unknown value. MessageToDict omits proto3 zero-value fields entirely, so the wire shape became ``{"status": {}}`` with no ``state`` field. A2A v0.3 receivers that validate against the schema reject this as a missing required field. Changes: - Add canceled, rejected, auth_required/auth-required to adcp_to_task_state (each has a valid pb.TaskState constant). - Raise ValueError for any unmapped status value with a directional message naming the eight valid states. ``unknown`` has no a2a-sdk 1.0 protobuf constant, so it is explicitly rejected; callers needing that wire state should build a Task manually and pass it through to_wire_dict. - Expand is_terminated to include canceled and rejected — both are terminal states per A2A v0.3, returning Task with artifacts rather than TaskStatusUpdateEvent. - Tighten status.value access (no string fallback) — the function signature is GeneratedTaskStatus and the docstring contract is enum members. - Update stale docstrings in create_a2a_webhook_payload and extract_webhook_result_data to list all four terminal states. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/webhooks.py | 66 ++++++++++++++------ tests/test_a2a_webhook_payload.py | 94 +++++++++++++++++++++++++++++ tests/test_webhooks_to_wire_dict.py | 14 ----- 3 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 tests/test_a2a_webhook_payload.py diff --git a/src/adcp/webhooks.py b/src/adcp/webhooks.py index 6b92e803..815a34a5 100644 --- a/src/adcp/webhooks.py +++ b/src/adcp/webhooks.py @@ -368,7 +368,7 @@ def extract_webhook_result_data(webhook_payload: dict[str, Any]) -> AdcpAsyncRes client initialization. Protocol Detection: - - A2A Task: Has "artifacts" field (terminated statuses: completed, failed) + - A2A Task: Has "artifacts" field (terminated statuses: completed, failed, canceled, rejected) - A2A TaskStatusUpdateEvent: Has nested "status.message" structure (intermediate statuses) - MCP: Has "result" field directly @@ -505,9 +505,10 @@ def create_a2a_webhook_payload( Create A2A webhook payload (Task or TaskStatusUpdateEvent). Per A2A specification: - - Terminated statuses (completed, failed): Returns Task with artifacts[].parts[] - - Intermediate statuses (working, input-required, submitted): Returns TaskStatusUpdateEvent - with status.message.parts[] + - Terminated statuses (completed, failed, canceled, rejected): Returns Task + with artifacts[].parts[] + - Intermediate statuses (working, input-required, submitted, auth-required): + Returns TaskStatusUpdateEvent with status.message.parts[] This function helps agent implementations construct properly formatted A2A webhook payloads for sending to clients. @@ -564,28 +565,47 @@ def create_a2a_webhook_payload( timestamp_proto = _isoformat_to_proto_timestamp(timestamp_str) if timestamp_str else None # Map GeneratedTaskStatus to A2A TaskState enum value. - status_value = status.value if hasattr(status, "value") else str(status) + # GeneratedTaskStatus is always an Enum so .value is guaranteed. + status_value = status.value adcp_to_task_state: dict[str, int] = { "completed": pb.TaskState.TASK_STATE_COMPLETED, "failed": pb.TaskState.TASK_STATE_FAILED, + "canceled": pb.TaskState.TASK_STATE_CANCELED, + "rejected": pb.TaskState.TASK_STATE_REJECTED, "working": pb.TaskState.TASK_STATE_WORKING, "submitted": pb.TaskState.TASK_STATE_SUBMITTED, + # GeneratedTaskStatus enum values are hyphenated ("input-required", + # "auth-required"). The underscore forms are accepted as a convenience + # for callers passing raw strings rather than enum members. "input_required": pb.TaskState.TASK_STATE_INPUT_REQUIRED, - # Tolerate the hyphenated form servers may echo back. "input-required": pb.TaskState.TASK_STATE_INPUT_REQUIRED, + "auth_required": pb.TaskState.TASK_STATE_AUTH_REQUIRED, + "auth-required": pb.TaskState.TASK_STATE_AUTH_REQUIRED, } - if status_value not in adcp_to_task_state: - # Falling back to TASK_STATE_UNSPECIFIED would normalize to the - # string ``"unspecified"`` on the wire, which is not a valid A2A - # v0.3 ``TaskState`` — buyer receivers validating against the - # spec reject the webhook. Fail loud at the builder boundary - # instead of producing a silently-broken envelope. + task_state_enum = adcp_to_task_state.get(status_value) + if task_state_enum is None: + # Falling back to TASK_STATE_UNSPECIFIED (proto3 zero) would be + # silently omitted by MessageToDict, producing an invalid wire + # shape ``{"status": {}}`` that A2A v0.3 receivers reject as + # missing the required ``state`` field. Fail loud at the builder + # boundary so callers can't ship a broken envelope. + known = [ + "submitted", + "working", + "input-required", + "completed", + "canceled", + "failed", + "rejected", + "auth-required", + ] raise ValueError( - f"Unknown AdCP task status {status_value!r}; expected one of " - f"{sorted(set(adcp_to_task_state))}. AdCP→A2A status mapping is " - "closed — an unknown value indicates a caller bug." + f"create_a2a_webhook_payload: unknown status {status_value!r}. " + f"Known AdCP→A2A states: {known}. " + "Note: 'unknown' has no a2a-sdk 1.0 protobuf constant; build a " + "Task manually and pass it through to_wire_dict if you need to " + "emit that state." ) - task_state_enum = adcp_to_task_state[status_value] # Build parts for the message/artifact. parts: list[pb.Part] = [] @@ -600,8 +620,14 @@ def create_a2a_webhook_payload( ParseDict(result_dict, value) parts.append(pb.Part(data=value)) - # Determine if this is a terminated status (Task) or intermediate (TaskStatusUpdateEvent) - is_terminated = status in [GeneratedTaskStatus.completed, GeneratedTaskStatus.failed] + # Determine if this is a terminated status (Task) or intermediate (TaskStatusUpdateEvent). + # canceled and rejected are terminal: the task will not continue. + is_terminated = status in ( + GeneratedTaskStatus.completed, + GeneratedTaskStatus.failed, + GeneratedTaskStatus.canceled, + GeneratedTaskStatus.rejected, + ) if is_terminated: status_kwargs: dict[str, Any] = {"state": task_state_enum} @@ -1111,7 +1137,9 @@ def _normalize_a2a_task_state_to_v03(payload: dict[str, Any]) -> None: state = status.get("state") if isinstance(state, str) and state.startswith("TASK_STATE_"): remainder = state[len("TASK_STATE_") :].lower() - # Spec uses hyphens for multi-word states. + # Spec uses hyphens for multi-word states (e.g. "auth-required"). + # Note: TASK_STATE_UNSPECIFIED (0) is the proto3 default and is + # silently omitted by MessageToDict, so it never reaches this branch. status["state"] = remainder.replace("_", "-") message = status.get("message") if isinstance(message, dict): diff --git a/tests/test_a2a_webhook_payload.py b/tests/test_a2a_webhook_payload.py new file mode 100644 index 00000000..27b9c2bb --- /dev/null +++ b/tests/test_a2a_webhook_payload.py @@ -0,0 +1,94 @@ +"""Tests for create_a2a_webhook_payload status mapping correctness. + +Guards against issue #603: previously, statuses not in the AdCP→A2A map +(canceled, rejected, auth_required) silently fell back to TASK_STATE_UNSPECIFIED +(proto3 zero value), which MessageToDict omits — producing an invalid +``{"status": {}}`` wire shape with no ``state`` field. Unknown statuses now +raise ValueError instead. +""" + +from __future__ import annotations + +import types + +import pytest +from a2a.types import Task, TaskStatusUpdateEvent +from google.protobuf.json_format import MessageToDict + +from adcp.types import GeneratedTaskStatus +from adcp.webhooks import create_a2a_webhook_payload + + +def _wire_state(obj: Task | TaskStatusUpdateEvent) -> str: + """Serialize to wire dict and return the normalized status.state string.""" + d = MessageToDict(obj, preserving_proto_field_name=False) + state = d.get("status", {}).get("state", "") + if isinstance(state, str) and state.startswith("TASK_STATE_"): + state = state[len("TASK_STATE_") :].lower().replace("_", "-") + return state + + +# --- terminal statuses return Task --- + + +def test_canceled_returns_task_with_canceled_state() -> None: + payload = create_a2a_webhook_payload( + task_id="t1", + status=GeneratedTaskStatus.canceled, + context_id="ctx1", + result={}, + ) + assert isinstance(payload, Task) + assert _wire_state(payload) == "canceled" + + +def test_rejected_returns_task_with_rejected_state() -> None: + payload = create_a2a_webhook_payload( + task_id="t2", + status=GeneratedTaskStatus.rejected, + context_id="ctx2", + result={}, + ) + assert isinstance(payload, Task) + assert _wire_state(payload) == "rejected" + + +# --- intermediate statuses return TaskStatusUpdateEvent --- + + +def test_auth_required_returns_event_with_auth_required_state() -> None: + payload = create_a2a_webhook_payload( + task_id="t3", + status=GeneratedTaskStatus.auth_required, + context_id="ctx3", + result={}, + ) + assert isinstance(payload, TaskStatusUpdateEvent) + assert _wire_state(payload) == "auth-required" + + +# --- unknown status raises ValueError --- + + +def test_unknown_status_value_raises_value_error() -> None: + # Simulate a caller passing an enum-like object whose .value is not in + # the AdCP→A2A map (e.g. a future enum member not yet supported). + fake_status = types.SimpleNamespace(value="bogus_status") + with pytest.raises(ValueError, match="unknown status"): + create_a2a_webhook_payload( + task_id="t4", + status=fake_status, # type: ignore[arg-type] + context_id="ctx4", + result={}, + ) + + +def test_unknown_status_error_message_names_known_states() -> None: + fake_status = types.SimpleNamespace(value="bogus_status") + with pytest.raises(ValueError, match="canceled"): + create_a2a_webhook_payload( + task_id="t5", + status=fake_status, # type: ignore[arg-type] + context_id="ctx5", + result={}, + ) diff --git a/tests/test_webhooks_to_wire_dict.py b/tests/test_webhooks_to_wire_dict.py index 846879eb..9acad60c 100644 --- a/tests/test_webhooks_to_wire_dict.py +++ b/tests/test_webhooks_to_wire_dict.py @@ -127,17 +127,3 @@ def test_unsupported_type_raises_type_error() -> None: """Silent fallthrough would mask integration bugs — fail loud.""" with pytest.raises(TypeError, match="Unsupported webhook payload type"): to_wire_dict("not a payload") # type: ignore[arg-type] - - -def test_a2a_unknown_status_raises_value_error() -> None: - """Unknown AdCP status must fail at the builder, not produce an - invalid-on-the-wire ``"unspecified"`` TaskState that buyer receivers - reject. (Issue #603.) - """ - with pytest.raises(ValueError, match="Unknown AdCP task status"): - create_a2a_webhook_payload( - task_id="task_123", - status="not-a-real-status", # type: ignore[arg-type] - context_id="ctx_456", - result={"media_buy_id": "mb_1"}, - )