Skip to content

test(adcp): spec-conformance test for AdcpError code names against error-code.json#436

Draft
bokelley wants to merge 3 commits intomainfrom
claude/issue-416-error-code-conformance-test
Draft

test(adcp): spec-conformance test for AdcpError code names against error-code.json#436
bokelley wants to merge 3 commits intomainfrom
claude/issue-416-error-code-conformance-test

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 3, 2026

Closes #416

Summary

Adds tests/test_error_code_conformance.py — an AST-based conformance gate that walks every AdcpError(...) constructor call in src/adcp/ and asserts each literal code string is either in the canonical schemas/cache/enums/error-code.json enum or has the X_ vendor-extension prefix. Runs in CI; fails on any non-spec code.

To make the gate green, three pre-existing non-spec codes that were already acting as vendor extensions are renamed to carry the required X_ prefix:

Old code New code Files
INTERNAL_ERROR X_INTERNAL_ERROR dispatch.py, handler.py (4 raise sites)
BILLING_NOT_PERMITTED_FOR_AGENT X_BILLING_NOT_PERMITTED_FOR_AGENT registry.py (1 raise site)

AUTH_INVALID is not renamed — the spec's AUTH_REQUIRED enumDescription explicitly states "A future minor release splits this code into AUTH_MISSING (correctable) and AUTH_INVALID (terminal)". It is allowlisted in the test as a provisional future spec code with a citation comment. See open question below.

What was tested

  • pytest tests/test_error_code_conformance.py tests/test_tier2_spec_conformance.py tests/test_decisioning_dispatch.py tests/test_decisioning_handler.py tests/test_webhook_handling.py tests/test_buyer_agent_registry.py tests/test_translate.py231 passed, 15 skipped
  • ruff check src/adcp/ tests/test_error_code_conformance.py — clean

Pre-PR review

  • code-reviewer: approved — no blockers; raised stale comment cleanup (fixed in commit 3) and data["enum"] crash hardening (fixed)
  • ad-tech-protocol-expert: approved — X_ prefix is consistent with spec extension model; AUTH_INVALID allowlist approach is protocol-correct per spec forward declaration

Open question for reviewer

AUTH_INVALID treatment — two approaches are valid:

  1. Allowlist (this PR): Keep AUTH_INVALID as-is; the test treats it as a provisional spec code because the spec prose names it as a forthcoming split from AUTH_REQUIRED. Remove the allowlist entry when the spec schema formalizes the split. This preserves the semantic distinction the spec is planning to formalize.

  2. Rename to AUTH_REQUIRED (code-reviewer's alternative): Maps AUTH_INVALID to the current spec code, using AUTH_REQUIRED with recovery="terminal" for the rejected-credentials sub-case the spec describes. More conservative; no allowlist needed. Would require a rename when AUTH_INVALID lands in the spec.

Both are defensible. @bokelley — please leave a comment or push a fixup indicating your preference.

Out of scope (noted for follow-up)

  • examples/v3_reference_seller/ still uses INTERNAL_ERROR and AUTH_INVALID — the conformance test scopes to src/adcp/ per the issue spec. Updating examples is a separate task.
  • src/adcp/server/translate.py:67, a2a_server.py:407, test_controller.py:656 use "INTERNAL_ERROR" as raw string literals in non-AdcpError() call sites — outside the AST scan scope by design.

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_01SpT9n4csqsXxbogX4q6GHo


Generated by Claude Code

claude added 3 commits May 3, 2026 02:54
Adds tests/test_error_code_conformance.py, which AST-walks every
AdcpError() constructor call in src/adcp/ and asserts the code string
is either in schemas/cache/enums/error-code.json or has the X_ vendor
prefix. Catches future spec-drift like the AGENT_SUSPENDED /
AGENT_BLOCKED incident (issue #375, PR #393) at PR time instead of
in manual review.

To make the test green: renames three pre-existing non-spec codes
that were already in use as vendor extensions but lacked the required
prefix — INTERNAL_ERROR → X_INTERNAL_ERROR (dispatch + handler),
BILLING_NOT_PERMITTED_FOR_AGENT → X_BILLING_NOT_PERMITTED_FOR_AGENT
(registry). AUTH_INVALID is allowlisted in the test because the spec
names it explicitly as an upcoming code split from AUTH_REQUIRED.

Updates all test assertions to match the renamed codes.

Closes #416

https://claude.ai/code/session_01SpT9n4csqsXxbogX4q6GHo
…docstrings

Pre-PR review (ad-tech-protocol-expert) caught a missed assertion in
test_tier2_spec_conformance.py and stale INTERNAL_ERROR / BILLING references
in module docstrings. Also updates the remaining test_webhook_handling.py
A2A path that the replace_all pass missed.

https://claude.ai/code/session_01SpT9n4csqsXxbogX4q6GHo
- Update remaining stale INTERNAL_ERROR/BILLING references in handler.py
  and dispatch.py inline comments and docstrings
- Update test_translate.py fixtures + assertions to use X_INTERNAL_ERROR
  (they were testing with the old non-spec code the dispatch no longer emits)
- Harden spec_codes fixture to emit pytest.fail with useful message if
  schema file exists but has no 'enum' key, instead of bare KeyError

https://claude.ai/code/session_01SpT9n4csqsXxbogX4q6GHo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(testing): spec-conformance test for AdcpError code names against error-code.json enum

2 participants