test(adcp): spec-conformance test for AdcpError code names against error-code.json#436
Draft
test(adcp): spec-conformance test for AdcpError code names against error-code.json#436
Conversation
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
Open
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #416
Summary
Adds
tests/test_error_code_conformance.py— an AST-based conformance gate that walks everyAdcpError(...)constructor call insrc/adcp/and asserts each literal code string is either in the canonicalschemas/cache/enums/error-code.jsonenum or has theX_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:INTERNAL_ERRORX_INTERNAL_ERRORdispatch.py,handler.py(4 raise sites)BILLING_NOT_PERMITTED_FOR_AGENTX_BILLING_NOT_PERMITTED_FOR_AGENTregistry.py(1 raise site)AUTH_INVALIDis not renamed — the spec'sAUTH_REQUIREDenumDescriptionexplicitly states "A future minor release splits this code intoAUTH_MISSING(correctable) andAUTH_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.py— 231 passed, 15 skippedruff check src/adcp/ tests/test_error_code_conformance.py— cleanPre-PR review
data["enum"]crash hardening (fixed)X_prefix is consistent with spec extension model;AUTH_INVALIDallowlist approach is protocol-correct per spec forward declarationOpen question for reviewer
AUTH_INVALIDtreatment — two approaches are valid:Allowlist (this PR): Keep
AUTH_INVALIDas-is; the test treats it as a provisional spec code because the spec prose names it as a forthcoming split fromAUTH_REQUIRED. Remove the allowlist entry when the spec schema formalizes the split. This preserves the semantic distinction the spec is planning to formalize.Rename to
AUTH_REQUIRED(code-reviewer's alternative): MapsAUTH_INVALIDto the current spec code, usingAUTH_REQUIREDwithrecovery="terminal"for the rejected-credentials sub-case the spec describes. More conservative; no allowlist needed. Would require a rename whenAUTH_INVALIDlands 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 usesINTERNAL_ERRORandAUTH_INVALID— the conformance test scopes tosrc/adcp/per the issue spec. Updating examples is a separate task.src/adcp/server/translate.py:67,a2a_server.py:407,test_controller.py:656use"INTERNAL_ERROR"as raw string literals in non-AdcpError()call sites — outside the AST scan scope by design.Session: https://claude.ai/code/session_01SpT9n4csqsXxbogX4q6GHo
Generated by Claude Code