Skip to content

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

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

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
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Superseded by #429 (test(spec-conformance): AdcpError codes against canonical error-code enum), already merged to main.

@bokelley bokelley closed this May 4, 2026
@bokelley bokelley deleted the claude/issue-416-error-code-conformance-test branch May 4, 2026 08:51
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Acknowledged — noting that #429 landed the conformance gate and this PR is superseded. No further action needed here.


Generated by Claude Code

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