From 29e5b0e740d347084bdc1bf76655fa1f14cd2611 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 10:58:58 +0000 Subject: [PATCH 1/2] test(a2a): assert TaskResult content for rejected and auth-required states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #263 (item 3 — test coverage gaps for A2A 1.0 states). Prior tests for TASK_STATE_REJECTED and TASK_STATE_AUTH_REQUIRED only asserted adapter.active_task_id behavior. These new tests also capture the full TaskResult: status, data, message, and metadata["status"]. The gap-documenting test (adcp_error DataPart not extracted for non-COMPLETED states) explicitly asserts data=None today and flags the future fix path in a comment. https://claude.ai/code/session_01MxALGppjocjuZzDVp48tps --- tests/test_protocols.py | 92 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/test_protocols.py b/tests/test_protocols.py index e7c016ec..a06bad17 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -930,6 +930,98 @@ async def test_task_id_cleared_on_unknown_state(self, a2a_config): assert adapter.active_task_id is None + @pytest.mark.asyncio + async def test_rejected_task_result_content(self, a2a_config): + """TASK_STATE_REJECTED — adapter returns SUBMITTED status with 'rejected' + in metadata and the TextPart message. REJECTED is terminal (task_id is + cleared) but routes through the non-COMPLETED else-branch in + _process_task_response, so data=None.""" + adapter = A2AAdapter(a2a_config) + + rejected = create_mock_a2a_task( + task_id="task-rejected-content", + context_id="ctx-rej", + state="rejected", + parts=[TextPart(text="policy violation: brand safety")], + ) + mock_a2a_client = AsyncMock() + mock_a2a_client.send_message = AsyncMock( + return_value=SendMessageSuccessResponse(result=rejected) + ) + + with patch.object(adapter, "_get_a2a_client", return_value=mock_a2a_client): + result = await adapter._call_a2a_tool("create_media_buy", {}) + + assert result.status == TaskStatus.SUBMITTED + assert result.data is None + assert result.message == "policy violation: brand safety" + assert result.metadata is not None + assert result.metadata["status"] == "rejected" + + @pytest.mark.asyncio + async def test_rejected_task_adcp_error_datapart_not_extracted(self, a2a_config): + """TASK_STATE_REJECTED with a DataPart carrying adcp_error — the + DataPart is silently dropped because _process_task_response only + calls _extract_result_from_task for COMPLETED tasks. This test + documents the current gap: callers cannot read structured error + detail from a rejected task's artifact without a separate fix.""" + adapter = A2AAdapter(a2a_config) + + rejected = create_mock_a2a_task( + task_id="task-rejected-err", + context_id="ctx-rej-err", + state="rejected", + parts=[ + DataPart(data={"adcp_error": {"code": "POLICY_VIOLATION", "message": "rejected"}}), + TextPart(text="rejected by server"), + ], + ) + mock_a2a_client = AsyncMock() + mock_a2a_client.send_message = AsyncMock( + return_value=SendMessageSuccessResponse(result=rejected) + ) + + with patch.object(adapter, "_get_a2a_client", return_value=mock_a2a_client): + result = await adapter._call_a2a_tool("create_media_buy", {}) + + assert result.status == TaskStatus.SUBMITTED + # Gap: adcp_error DataPart is not extracted for non-COMPLETED states. + # A future fix should surface structured error detail here. + assert result.data is None + assert result.message == "rejected by server" + assert result.metadata is not None + assert result.metadata["status"] == "rejected" + + @pytest.mark.asyncio + async def test_auth_required_task_result_content(self, a2a_config): + """TASK_STATE_AUTH_REQUIRED — non-terminal state. Adapter returns + SUBMITTED status with 'auth-required' in metadata and the challenge + message from the TextPart. Callers should surface this to trigger + an auth flow before re-submitting.""" + adapter = A2AAdapter(a2a_config) + + auth_task = create_mock_a2a_task( + task_id="task-auth-content", + context_id="ctx-auth", + state="auth-required", + parts=[TextPart(text="OAuth required: redirect to https://auth.example.com")], + ) + mock_a2a_client = AsyncMock() + mock_a2a_client.send_message = AsyncMock( + return_value=SendMessageSuccessResponse(result=auth_task) + ) + + with patch.object(adapter, "_get_a2a_client", return_value=mock_a2a_client): + result = await adapter._call_a2a_tool("create_media_buy", {}) + + assert result.status == TaskStatus.SUBMITTED + assert result.data is None + assert result.message == "OAuth required: redirect to https://auth.example.com" + assert result.metadata is not None + assert result.metadata["status"] == "auth-required" + assert result.metadata["task_id"] == "task-auth-content" + assert result.metadata["context_id"] == "ctx-auth" + @pytest.mark.asyncio async def test_state_not_committed_when_post_processing_raises(self, a2a_config): """If _process_task_response raises, the adapter must NOT advance From 440dc5190f0e4df28713cd9c36d3fc914fc04fa6 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Wed, 29 Apr 2026 06:58:34 -0400 Subject: [PATCH 2/2] =?UTF-8?q?test(a2a):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20strict=20xfail=20+=20parity=20assertions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acting on code-reviewer feedback on this PR: - Convert the gap-documenting test from a passing assertion of buggy behavior to ``pytest.mark.xfail(strict=True)`` asserting the desired post-fix behavior. Strict mode flips the test to failure when someone fixes the gap, so we keep the regression-trip benefit without the test name reading as endorsement of the bug. - Split the original mixed-payload test into two: one strict-xfail for the structured-error extraction we want, one passing test that pins the TextPart-extraction half (regression guard for the human message surfacing across REJECTED with a DataPart present). - Add ``task_id``/``context_id`` assertions to the REJECTED test for parity with the AUTH_REQUIRED test — the else-branch in ``_process_task_response`` always sets both, regardless of state. - Add ``result.error is None`` and ``result.success is True`` assertions to both REJECTED and AUTH_REQUIRED tests. The ``success=True`` line is annotated as ``# FIXME: semantically wrong for REJECTED`` so future-us has a hook for the eventual fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_protocols.py | 84 ++++++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 17 deletions(-) diff --git a/tests/test_protocols.py b/tests/test_protocols.py index a06bad17..b2538711 100644 --- a/tests/test_protocols.py +++ b/tests/test_protocols.py @@ -932,10 +932,11 @@ async def test_task_id_cleared_on_unknown_state(self, a2a_config): @pytest.mark.asyncio async def test_rejected_task_result_content(self, a2a_config): - """TASK_STATE_REJECTED — adapter returns SUBMITTED status with 'rejected' - in metadata and the TextPart message. REJECTED is terminal (task_id is - cleared) but routes through the non-COMPLETED else-branch in - _process_task_response, so data=None.""" + """TASK_STATE_REJECTED — adapter routes through the non-COMPLETED + else-branch in _process_task_response (a2a.py:618-673), returning + ``status=SUBMITTED``, ``data=None``, ``success=True`` (the branch + hardcodes success — see follow-up tracker), and metadata carrying + the lowercased state, the task id, and the context id.""" adapter = A2AAdapter(a2a_config) rejected = create_mock_a2a_task( @@ -954,17 +955,33 @@ async def test_rejected_task_result_content(self, a2a_config): assert result.status == TaskStatus.SUBMITTED assert result.data is None + assert result.error is None + assert result.success is True # FIXME: semantically wrong for REJECTED assert result.message == "policy violation: brand safety" assert result.metadata is not None assert result.metadata["status"] == "rejected" - + assert result.metadata["task_id"] == "task-rejected-content" + assert result.metadata["context_id"] == "ctx-rej" + + @pytest.mark.xfail( + strict=True, + reason=( + "_process_task_response only extracts adcp_error DataParts for COMPLETED " + "tasks; rejected tasks lose structured error detail. When this gap is " + "fixed (issue #263), flip the test to assert the extracted error " + "instead of removing xfail." + ), + ) @pytest.mark.asyncio - async def test_rejected_task_adcp_error_datapart_not_extracted(self, a2a_config): - """TASK_STATE_REJECTED with a DataPart carrying adcp_error — the - DataPart is silently dropped because _process_task_response only - calls _extract_result_from_task for COMPLETED tasks. This test - documents the current gap: callers cannot read structured error - detail from a rejected task's artifact without a separate fix.""" + async def test_rejected_task_adcp_error_datapart_extracted(self, a2a_config): + """TASK_STATE_REJECTED with a DataPart carrying adcp_error. + + Strict-xfail: today the DataPart's structured error detail is silently + dropped because the adapter only calls ``_extract_result_from_task`` + for COMPLETED tasks. This test asserts the desired post-fix behavior + (error code accessible to callers); ``strict=True`` flips to failure + the moment someone fixes the gap so we don't keep documenting it as + endorsed.""" adapter = A2AAdapter(a2a_config) rejected = create_mock_a2a_task( @@ -981,23 +998,54 @@ async def test_rejected_task_adcp_error_datapart_not_extracted(self, a2a_config) return_value=SendMessageSuccessResponse(result=rejected) ) + with patch.object(adapter, "_get_a2a_client", return_value=mock_a2a_client): + result = await adapter._call_a2a_tool("create_media_buy", {}) + + # Desired post-fix: structured error surfaces on TaskResult. + assert result.error is not None + assert "POLICY_VIOLATION" in (result.error or "") + + @pytest.mark.asyncio + async def test_rejected_task_drops_datapart_keeps_textpart(self, a2a_config): + """TASK_STATE_REJECTED with both a DataPart and a TextPart — pin the + current behavior: the TextPart message is preserved, the DataPart's + structured payload is not extracted (tracked separately in the strict + xfail above). This guards against a regression that drops the human + message too.""" + adapter = A2AAdapter(a2a_config) + + rejected = create_mock_a2a_task( + task_id="task-rejected-mixed", + context_id="ctx-rej-mixed", + state="rejected", + parts=[ + DataPart(data={"adcp_error": {"code": "POLICY_VIOLATION", "message": "rejected"}}), + TextPart(text="rejected by server"), + ], + ) + mock_a2a_client = AsyncMock() + mock_a2a_client.send_message = AsyncMock( + return_value=SendMessageSuccessResponse(result=rejected) + ) + with patch.object(adapter, "_get_a2a_client", return_value=mock_a2a_client): result = await adapter._call_a2a_tool("create_media_buy", {}) assert result.status == TaskStatus.SUBMITTED - # Gap: adcp_error DataPart is not extracted for non-COMPLETED states. - # A future fix should surface structured error detail here. assert result.data is None assert result.message == "rejected by server" assert result.metadata is not None assert result.metadata["status"] == "rejected" + assert result.metadata["task_id"] == "task-rejected-mixed" + assert result.metadata["context_id"] == "ctx-rej-mixed" @pytest.mark.asyncio async def test_auth_required_task_result_content(self, a2a_config): - """TASK_STATE_AUTH_REQUIRED — non-terminal state. Adapter returns - SUBMITTED status with 'auth-required' in metadata and the challenge - message from the TextPart. Callers should surface this to trigger - an auth flow before re-submitting.""" + """TASK_STATE_AUTH_REQUIRED — non-terminal state. Same else-branch + as REJECTED in _process_task_response, so the assertions parallel + ``test_rejected_task_result_content``: status SUBMITTED, data None, + metadata with state, task id, and context id, plus the challenge + message extracted from the TextPart.""" adapter = A2AAdapter(a2a_config) auth_task = create_mock_a2a_task( @@ -1016,6 +1064,8 @@ async def test_auth_required_task_result_content(self, a2a_config): assert result.status == TaskStatus.SUBMITTED assert result.data is None + assert result.error is None + assert result.success is True assert result.message == "OAuth required: redirect to https://auth.example.com" assert result.metadata is not None assert result.metadata["status"] == "auth-required"