Skip to content

fix(server/a2a): structured error parity with MCP — emit field/details/retry_after, catch decisioning AdcpError#542

Closed
bokelley wants to merge 2 commits intomainfrom
claude/issue-530-a2a-structured-error-parity
Closed

fix(server/a2a): structured error parity with MCP — emit field/details/retry_after, catch decisioning AdcpError#542
bokelley wants to merge 2 commits intomainfrom
claude/issue-530-a2a-structured-error-parity

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Closes #530

Summary

The A2A transport had two parity gaps vs. the MCP structured-error path shipped in #525:

  1. execute() only caught adcp.exceptions.ADCPError — decisioning-layer adcp.decisioning.types.AdcpError (raised by DecisioningPlatform adopters) fell into the generic except Exception path and emitted a plain "Skill execution failed" text message with no structured envelope.

  2. _send_adcp_error only emitted code/message/recovery/suggestion — the field, details, and retry_after fields were dropped even when the raised error carried them, breaking wire-conformance for A2A storyboard structured-error assertions.

This PR:

  • Adds a except Exception branch in execute() that lazy-imports AdcpError from adcp.decisioning.types (matching the pattern in translate.py) and routes it through _send_adcp_error.
  • Refactors _send_adcp_error to delegate to _extract_structured_fields from translate.py (introduced in feat(server): MCP error responses populate structuredContent.adcp_error (closes #509) #525), producing the full {code, message, recovery, field, suggestion, details, retry_after} envelope — and always emitting recovery unconditionally to match the MCP path (transport-errors.mdx §A2A Binding).
  • Removes now-unused ADCPTaskError and STANDARD_ERROR_CODES imports.

What tested

  • pytest tests/test_a2a_server.py — 47 passed (7 new tests added)
  • pytest tests/test_server_idempotency.py — 45 passed (existing A2A conflict test still green)
  • pytest tests/ (non-integration, excluding 3 pre-existing missing-dep failures) — 3302 passed, 18 skipped
  • ruff check src/adcp/server/a2a_server.py — clean
  • mypy src/adcp/server/a2a_server.py — no new errors (pre-existing a2a/protobuf stub gaps unchanged)

Pre-PR review

  • code-reviewer: approved — no blockers. Nits noted: (1) lazy _DecAdcpError import inside except is redundant with _extract_structured_fields's own lazy import (harmless, Python module cache); (2) exc: Any on _send_adcp_error exposes a potential silent TypeError for future callers (internal callers are guarded); (3) prefer DecisioningAdcpError over _DecAdcpError for local var naming. All nits deferred.
  • ad-tech-protocol-expert: approved — one blocker found and fixed before push: recovery was guarded by if recovery: (could omit on falsy string) vs. MCP path which emits it unconditionally. Fixed in commit c4f01a2 by moving "recovery": recovery into the base dict literal. Wire shape now fully matches transport-errors.mdx §A2A Binding.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_019DMGxsA43mtT7QTRiNipBM


Generated by Claude Code

claude added 2 commits May 4, 2026 01:39
…s/retry_after, catch decisioning AdcpError (closes #530)

- `_send_adcp_error` now uses `_extract_structured_fields` from translate.py
  (introduced in #525) to populate the full adcp_error envelope: code,
  message, recovery, field, details, retry_after, suggestion.
- `execute()` adds a second `except Exception` branch that catches
  decisioning-layer `AdcpError` (separate hierarchy from
  `adcp.exceptions.ADCPError`) and routes it through `_send_adcp_error`
  instead of the generic "Skill execution failed" text path.
- Drop now-unused `ADCPTaskError` and `STANDARD_ERROR_CODES` imports.
- Tests: parametrized over MEDIA_BUY_NOT_FOUND, PACKAGE_NOT_FOUND,
  BUDGET_TOO_LOW, TERMS_REJECTED; decisioning AdcpError with retry_after;
  end-to-end execute() → DataPart round-trip for both paths.

https://claude.ai/code/session_019DMGxsA43mtT7QTRiNipBM
…sport parity

Mirror build_mcp_error_result: recovery is always included in the A2A DataPart
adcp_error dict (no falsy guard), matching the MCP path where recovery is a
required key per transport-errors.mdx §A2A Binding.

https://claude.ai/code/session_019DMGxsA43mtT7QTRiNipBM
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Superseded by #536 (feat(server/a2a): structured error parity with MCP — emit field/details/retry_after, catch decisioning AdcpError), already merged to main.

@bokelley bokelley closed this May 4, 2026
@bokelley bokelley deleted the claude/issue-530-a2a-structured-error-parity branch May 4, 2026 08:51
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Acknowledged — #536 supersedes this PR and is already merged to main. No further action needed here.


Triaged by Claude Code. Session: https://claude.ai/code/session_0189ZzC2Brh3uWR3b8fq1Xiv


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(server/a2a): structured error parity with MCP — emit field/details/retry_after, catch decisioning AdcpError

2 participants