feat(server): route validation by wire adcp_version (stage 3)#660
Closed
bokelley wants to merge 3 commits into
Closed
feat(server): route validation by wire adcp_version (stage 3)#660bokelley wants to merge 3 commits into
bokelley wants to merge 3 commits into
Conversation
bokelley
added a commit
that referenced
this pull request
May 11, 2026
… echo Review feedback on PR #660. * String ``adcp_major_version`` (e.g. ``"3"``) used to fall through to ``None`` and silently get SDK-pin validation. A buyer that JSON-stringified the field deserves a loud ``VERSION_UNSUPPORTED``, not a quiet behaviour swap. Same for ``< 1`` values (below the spec's ``minimum: 1`` bound). * ``claimed_version`` in error details used to be ``str(exc.wire_value)``, which converted the int field to ``"4"``. Pass through verbatim so buyer telemetry sees the same type they sent. * Rename ``test_no_version_field_validator_uses_sdk_pin`` to actually test that claim — pass a ``ValidationHookConfig`` so the validator fires, and assert ``version=None`` is threaded. Split the original test's "no validation config means no validator runs" assertion into its own regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
Stage 3 of the versioned-schema-validation port. ``create_tool_caller`` now detects the buyer's claimed version off the request envelope and threads it through to the per-version validator added in Stage 2. Detection order (mirrors the TS SDK's ``applyVersionEnvelope`` and the spec text on every ``*-request`` schema): 1. ``adcp_version`` (3.1+ release-precision string) — normalized to release precision (``"3.0.7"`` → ``"3.0"``); must be in ``COMPATIBLE_ADCP_VERSIONS`` or ``VERSION_UNSUPPORTED`` is raised. 2. ``adcp_major_version`` (legacy integer) — maps to the highest supported minor for that major. Unknown major raises ``VERSION_UNSUPPORTED`` with structured details. 3. Neither field set — falls through to the SDK pin (existing behaviour). New module ``adcp.validation.envelope`` owns the detection logic; ``UnsupportedVersionError`` carries the wire value and supported set so the dispatcher can echo both into the error envelope's ``details``. Existing ``test_spec_compat_hooks`` payloads used ``adcp_major_version=4`` as a wire-shape placeholder — updated to ``3`` now that the dispatcher actually validates the field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… echo Review feedback on PR #660. * String ``adcp_major_version`` (e.g. ``"3"``) used to fall through to ``None`` and silently get SDK-pin validation. A buyer that JSON-stringified the field deserves a loud ``VERSION_UNSUPPORTED``, not a quiet behaviour swap. Same for ``< 1`` values (below the spec's ``minimum: 1`` bound). * ``claimed_version`` in error details used to be ``str(exc.wire_value)``, which converted the int field to ``"4"``. Pass through verbatim so buyer telemetry sees the same type they sent. * Rename ``test_no_version_field_validator_uses_sdk_pin`` to actually test that claim — pass a ``ValidationHookConfig`` so the validator fires, and assert ``version=None`` is threaded. Split the original test's "no validation config means no validator runs" assertion into its own regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
force-pushed
the
claude/versioned-schemas-stage-3
branch
from
fc7dd4c to
d0c1944
Compare
…T_VERSION_ENVELOPE) Downstream-impact audit on PR #660 flagged that switching from "wire schema accepts adcp_major_version 1-99" to "dispatcher rejects unknown versions" in one release would break test fixtures using ``4`` as a sentinel and any buyer claiming an unsupported version that happened to pass schema validation. Decouple legacy adapter routing (additive, ships now) from version-envelope strictness (subtractive, ships under a gate). * Default (``ADCP_STRICT_VERSION_ENVELOPE`` unset or ``"0"``): an unsupported wire version is logged at WARNING with a migration hint pointing at the env var. The dispatcher falls through to SDK-pin validation — same as 5.1.0 behaviour for that payload. * Strict (``ADCP_STRICT_VERSION_ENVELOPE=1``): raise the ``VERSION_UNSUPPORTED`` envelope the spec prescribes, before any handler dispatch. This becomes the default in 5.3. Existing strict-mode tests gain a ``strict_version_envelope`` fixture so the spec-prescribed behaviour stays covered. New ``test_unsupported_version_permissive_falls_through`` exercises the default-permissive path and asserts the migration hint shows up in logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
bokelley
added a commit
that referenced
this pull request
May 11, 2026
* feat(validation): per-version validator loader (stage 2) ``get_validator``, ``validate_request``, and ``validate_response`` now accept an optional ``version=`` kwarg. ``None`` defaults to the SDK's compile-time pin (existing behaviour, byte-identical for current call sites). A wire-version string (``"3.0.7"``, ``"2.5"``, ``"3.1.0-beta.1"``) routes to the per-bundle-key cache laid down by Stage 1 — each version's file index, compiled validators, and core registry live in their own ``_LoaderState`` so cross-version traffic doesn't share compilation state. The dispatcher call sites are unchanged today; Stage 3 will detect the buyer's ``adcp_major_version`` off the wire and thread it through. Also widens ``resolve_bundle_key`` to accept bare ``MAJOR.MINOR`` (already a bundle key — matches the wire envelope's release-precision ``adcp_version`` field, so the dispatcher can pass it straight through). Tests: * ``test_schema_loader_per_version`` lays down a synthetic ``schemas/cache/2.5/`` fixture, asserts validators for the synthetic legacy tool and the real SDK-pin tools coexist independently, and pins ``version=None`` behaviour to the SDK pin. * ``test_validation_version`` extended with the new pass-through and rejection cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(validation): isolate per-version loader fixture from repo working tree Review feedback on PR #659 (Stage 2). Two nits: * Fixture used to write into the repo's real ``schemas/cache/2.5/``, which means CI runs against an installed wheel would silently miss the synthetic bundle (packaged ``_schemas/`` wins) and the assertions would be vacuous. Move to ``tmp_path`` + ``monkeypatch.setattr`` on the loader's resolver instead. * Teardown used a sequence of ``rmdir`` calls that fail if anything else lands in the directories mid-run. ``shutil.rmtree(..., ignore_errors= True)`` is robust. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(server): route validation by wire ``adcp_version`` (stage 3) Stage 3 of the versioned-schema-validation port. ``create_tool_caller`` now detects the buyer's claimed version off the request envelope and threads it through to the per-version validator added in Stage 2. Detection order (mirrors the TS SDK's ``applyVersionEnvelope`` and the spec text on every ``*-request`` schema): 1. ``adcp_version`` (3.1+ release-precision string) — normalized to release precision (``"3.0.7"`` → ``"3.0"``); must be in ``COMPATIBLE_ADCP_VERSIONS`` or ``VERSION_UNSUPPORTED`` is raised. 2. ``adcp_major_version`` (legacy integer) — maps to the highest supported minor for that major. Unknown major raises ``VERSION_UNSUPPORTED`` with structured details. 3. Neither field set — falls through to the SDK pin (existing behaviour). New module ``adcp.validation.envelope`` owns the detection logic; ``UnsupportedVersionError`` carries the wire value and supported set so the dispatcher can echo both into the error envelope's ``details``. Existing ``test_spec_compat_hooks`` payloads used ``adcp_major_version=4`` as a wire-shape placeholder — updated to ``3`` now that the dispatcher actually validates the field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(envelope): reject string adcp_major_version, preserve int type in echo Review feedback on PR #660. * String ``adcp_major_version`` (e.g. ``"3"``) used to fall through to ``None`` and silently get SDK-pin validation. A buyer that JSON-stringified the field deserves a loud ``VERSION_UNSUPPORTED``, not a quiet behaviour swap. Same for ``< 1`` values (below the spec's ``minimum: 1`` bound). * ``claimed_version`` in error details used to be ``str(exc.wire_value)``, which converted the int field to ``"4"``. Pass through verbatim so buyer telemetry sees the same type they sent. * Rename ``test_no_version_field_validator_uses_sdk_pin`` to actually test that claim — pass a ``ValidationHookConfig`` so the validator fires, and assert ``version=None`` is threaded. Split the original test's "no validation config means no validator runs" assertion into its own regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(server): default-permissive VERSION_UNSUPPORTED gate (ADCP_STRICT_VERSION_ENVELOPE) Downstream-impact audit on PR #660 flagged that switching from "wire schema accepts adcp_major_version 1-99" to "dispatcher rejects unknown versions" in one release would break test fixtures using ``4`` as a sentinel and any buyer claiming an unsupported version that happened to pass schema validation. Decouple legacy adapter routing (additive, ships now) from version-envelope strictness (subtractive, ships under a gate). * Default (``ADCP_STRICT_VERSION_ENVELOPE`` unset or ``"0"``): an unsupported wire version is logged at WARNING with a migration hint pointing at the env var. The dispatcher falls through to SDK-pin validation — same as 5.1.0 behaviour for that payload. * Strict (``ADCP_STRICT_VERSION_ENVELOPE=1``): raise the ``VERSION_UNSUPPORTED`` envelope the spec prescribes, before any handler dispatch. This becomes the default in 5.3. Existing strict-mode tests gain a ``strict_version_envelope`` fixture so the spec-prescribed behaviour stays covered. New ``test_unsupported_version_permissive_falls_through`` exercises the default-permissive path and asserts the migration hint shows up in logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
May 11, 2026
* feat(validation): per-version validator loader (stage 2) ``get_validator``, ``validate_request``, and ``validate_response`` now accept an optional ``version=`` kwarg. ``None`` defaults to the SDK's compile-time pin (existing behaviour, byte-identical for current call sites). A wire-version string (``"3.0.7"``, ``"2.5"``, ``"3.1.0-beta.1"``) routes to the per-bundle-key cache laid down by Stage 1 — each version's file index, compiled validators, and core registry live in their own ``_LoaderState`` so cross-version traffic doesn't share compilation state. The dispatcher call sites are unchanged today; Stage 3 will detect the buyer's ``adcp_major_version`` off the wire and thread it through. Also widens ``resolve_bundle_key`` to accept bare ``MAJOR.MINOR`` (already a bundle key — matches the wire envelope's release-precision ``adcp_version`` field, so the dispatcher can pass it straight through). Tests: * ``test_schema_loader_per_version`` lays down a synthetic ``schemas/cache/2.5/`` fixture, asserts validators for the synthetic legacy tool and the real SDK-pin tools coexist independently, and pins ``version=None`` behaviour to the SDK pin. * ``test_validation_version`` extended with the new pass-through and rejection cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(validation): isolate per-version loader fixture from repo working tree Review feedback on PR #659 (Stage 2). Two nits: * Fixture used to write into the repo's real ``schemas/cache/2.5/``, which means CI runs against an installed wheel would silently miss the synthetic bundle (packaged ``_schemas/`` wins) and the assertions would be vacuous. Move to ``tmp_path`` + ``monkeypatch.setattr`` on the loader's resolver instead. * Teardown used a sequence of ``rmdir`` calls that fail if anything else lands in the directories mid-run. ``shutil.rmtree(..., ignore_errors= True)`` is robust. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(server): route validation by wire ``adcp_version`` (stage 3) Stage 3 of the versioned-schema-validation port. ``create_tool_caller`` now detects the buyer's claimed version off the request envelope and threads it through to the per-version validator added in Stage 2. Detection order (mirrors the TS SDK's ``applyVersionEnvelope`` and the spec text on every ``*-request`` schema): 1. ``adcp_version`` (3.1+ release-precision string) — normalized to release precision (``"3.0.7"`` → ``"3.0"``); must be in ``COMPATIBLE_ADCP_VERSIONS`` or ``VERSION_UNSUPPORTED`` is raised. 2. ``adcp_major_version`` (legacy integer) — maps to the highest supported minor for that major. Unknown major raises ``VERSION_UNSUPPORTED`` with structured details. 3. Neither field set — falls through to the SDK pin (existing behaviour). New module ``adcp.validation.envelope`` owns the detection logic; ``UnsupportedVersionError`` carries the wire value and supported set so the dispatcher can echo both into the error envelope's ``details``. Existing ``test_spec_compat_hooks`` payloads used ``adcp_major_version=4`` as a wire-shape placeholder — updated to ``3`` now that the dispatcher actually validates the field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(envelope): reject string adcp_major_version, preserve int type in echo Review feedback on PR #660. * String ``adcp_major_version`` (e.g. ``"3"``) used to fall through to ``None`` and silently get SDK-pin validation. A buyer that JSON-stringified the field deserves a loud ``VERSION_UNSUPPORTED``, not a quiet behaviour swap. Same for ``< 1`` values (below the spec's ``minimum: 1`` bound). * ``claimed_version`` in error details used to be ``str(exc.wire_value)``, which converted the int field to ``"4"``. Pass through verbatim so buyer telemetry sees the same type they sent. * Rename ``test_no_version_field_validator_uses_sdk_pin`` to actually test that claim — pass a ``ValidationHookConfig`` so the validator fires, and assert ``version=None`` is threaded. Split the original test's "no validation config means no validator runs" assertion into its own regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(server): default-permissive VERSION_UNSUPPORTED gate (ADCP_STRICT_VERSION_ENVELOPE) Downstream-impact audit on PR #660 flagged that switching from "wire schema accepts adcp_major_version 1-99" to "dispatcher rejects unknown versions" in one release would break test fixtures using ``4`` as a sentinel and any buyer claiming an unsupported version that happened to pass schema validation. Decouple legacy adapter routing (additive, ships now) from version-envelope strictness (subtractive, ships under a gate). * Default (``ADCP_STRICT_VERSION_ENVELOPE`` unset or ``"0"``): an unsupported wire version is logged at WARNING with a migration hint pointing at the env var. The dispatcher falls through to SDK-pin validation — same as 5.1.0 behaviour for that payload. * Strict (``ADCP_STRICT_VERSION_ENVELOPE=1``): raise the ``VERSION_UNSUPPORTED`` envelope the spec prescribes, before any handler dispatch. This becomes the default in 5.3. Existing strict-mode tests gain a ``strict_version_envelope`` fixture so the spec-prescribed behaviour stays covered. New ``test_unsupported_version_permissive_falls_through`` exercises the default-permissive path and asserts the migration hint shows up in logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(compat): AdapterPair pattern + v2.5 sync_creatives (stage 4) Stage 4 of the versioned-schema-validation port. Replaces the heuristic ``spec_compat_hooks()`` model with a typed per-tool adapter registry. What changed: * New package ``adcp.compat.legacy`` with the ``AdapterPair`` dataclass, registry (``get_legacy_adapter`` / ``list_legacy_adapter_tools``), and the ``LEGACY_ADAPTER_VERSIONS`` constant. Mirrors the TS SDK's ``src/lib/adapters/legacy/v2-5/``. * First concrete adapter: ``adcp.compat.legacy.v2_5.sync_creatives``. Three wire-shape coercions (bare-string ``format_id`` → structured, ``asset_type`` inference, ``image``-without-dims → ``url`` demotion). * ``validation.envelope.SUPPORTED_WIRE_VERSIONS`` now includes legacy versions, so ``detect_wire_version`` accepts both native and legacy claims. * ``create_tool_caller`` routes legacy-versioned requests through the adapter: ``adapt_request`` runs *after* version detection and *before* current-schema validation, so a buggy translator surfaces as ``INVALID_REQUEST`` with a field-level pointer. A legacy version with no adapter for the requested tool raises ``INVALID_REQUEST``. * Response side: optional ``normalize_response`` callable runs after current-schema validation. ``sync_creatives`` has the same response shape across v2.5/v3 so leaves it ``None``. Deferred (Stage 4b): real v2.5 schema bundle. Upstream CDN doesn't ship v2.5 tarballs; the JS SDK pins a GitHub commit SHA. Wire that fetch in a follow-up. Today the adapter validates against the v3 output only. Tests (25 new): registry contract, every v2.5 sync_creatives coercion via the public adapter surface, end-to-end dispatcher routing including missing-adapter and adapter-raises cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(compat): tighten AdapterPair contract, data-driven module loader Review feedback on Stage 4. * AdapterPair docstring states the contract: sync + pure (no mutation), no I/O, request raise → INVALID_REQUEST, response raise → INTERNAL_ERROR. * _ensure_loaded reads its work from _VERSION_MODULES dict — adding a tool to a version is a one-line append, not a control-flow change. * Dispatcher renames effective_version → post_adapter_validator_version with a comment explaining how Stage 4b (real v2.5 schema bundle) extends the variable's role. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Stacked on #659.
Summary
Stage 3 of the versioned-schema-validation port.
create_tool_callernow detects the buyer's claimed version off the request envelope and threads it through to the per-version validator added in Stage 2.Detection order (mirrors the TS SDK's
applyVersionEnvelopeand the spec text on every*-requestschema):adcp_version(3.1+ release-precision string) — normalized to release precision (\"3.0.7\"→\"3.0\"); must be inCOMPATIBLE_ADCP_VERSIONSorVERSION_UNSUPPORTEDis raised.adcp_major_version(legacy integer) — maps to the highest supported minor for that major. Unknown major raisesVERSION_UNSUPPORTEDwith structured details.New module
adcp.validation.envelopeowns the detection logic.UnsupportedVersionErrorcarries the wire value and supported set so the dispatcher can echo both into the error envelope'sdetails.What the dispatcher does now
VERSION_UNSUPPORTEDbefore any handler dispatch.validate_request(..., version=wire_version)— Stage 2 loader picks the per-version validator.validate_response(..., version=wire_version)— same version as request.Subsequent stages
v2.5.sync_creatives) wired so the dispatcher can validate against a v2.5 schema and translate to v3 internal types.spec_compat_hooks()deprecation.Test plan
pytest tests/— 4425 passed (one unrelated flaky timing test deselected; <1 microsecond drift)test_dispatcher_version_routing— 7 integration tests for the wire-level pathtest_validation_envelope— 12 unit tests for the detection helperruff check+mypytest_spec_compat_hookspayloads updated from placeholderadcp_major_version=4to a real supported version🤖 Generated with Claude Code