feat(validation): schema-driven validation with client hooks and opt-in server middleware#256
Merged
Conversation
…in server middleware Port the TS SDK's schema validator (adcontextprotocol/adcp-client#694) so field-name drift is caught at the SDK layer instead of at storyboard-run time (closes #249). - Bundled AdCP JSON schemas drive validation via jsonschema; compiled validators cached by (tool, direction) and resolved against the packaged ``_schemas/`` tree or the repo-relative ``schemas/cache/``. - Client hooks: pre-send request validation + post-receive response validation on both MCP and A2A adapters, configurable per side (strict / warn / off). Threaded through ``ADCPClient(validation=...)``. - Server middleware: opt-in on ``create_tool_caller`` / ``create_mcp_tools`` — strict raises ``ADCPTaskError(VALIDATION_ERROR)`` with every issue under ``details.issues`` (JSON Pointer + keyword + schema path). - Async variants (``-submitted``, ``-working``, ``-input-required``) selected by payload ``status`` shape, not tool name alone. - ``scripts/bundle_schemas.py`` mirrors ``schemas/cache/`` → ``src/adcp/_schemas/`` before wheel build; wired into ``make regenerate-schemas``, ``make build``, and CI. Defaults diverge from the TS port: Python responses default to ``warn`` (not ``strict``) because the SDK had no pre-existing strict response validator to preserve — breaking existing tests that stub non-spec payloads would be too noisy. Set ``ADCP_ENV=dev`` or pass ``ValidationHookConfig(responses="strict")`` to opt in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pec-compliant
Flip ``responses`` default back to ``strict`` (matches TS port) and
update test stubs that were relying on non-spec payloads. CLAUDE.md's
"write tests to the spec, not to the code" rules out warn-by-default
as a shortcut around stub correctness.
- ``_default_response_mode`` now returns ``strict`` unless ``ADCP_ENV`` /
``PYTHON_ENV`` / ``ENV`` / ``ENVIRONMENT`` is set to ``production`` /
``prod``. Adapter base class mirrors the same default.
- Fix stubs in ``test_protocols.py``, ``test_idempotency.py``,
``test_idempotency_storyboard.py``, and
``tests/integration/test_a2a_context_id.py`` to use spec-compliant
payloads (e.g. ``{"products": []}`` instead of ``{"products":
[{"id": "prod1"}]}``, ``{"media_buy_id": ..., "packages": []}``
instead of bare ``{"media_buy_id": ...}``).
- Update my own schema-validation tests + ADCPClient docstring to the
restored defaults.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-reviewer + security-reviewer punch list for PR #256. Must Fix: - Sanitize jsonschema error messages before they hit the wire envelope or logs. ``jsonschema.ValidationError.message`` embeds the offending payload value verbatim (``"'Bearer sk-...' is not of type 'integer'"``), which the server middleware was packing into ``adcp_error.message`` and every ``details.issues[].message``. New ``_safe_message`` keeps structural facts (keyword, expected type, constraint bound) and drops the value. Matches the TS port's Pydantic ``INVALID_REQUEST`` hardening that already stripped ``input``/``ctx``/``url``. - Declare ``SchemaValidationError.details`` as a proper attribute set in ``__init__`` instead of a bolt-on via ``__setattr__``. Callers and static analyzers now see it. - Server middleware skips response validation when the handler returned an ``adcp_error`` L3 envelope — the envelope has its own shape enforced by ``Error`` and would false-positive against the per-tool success schema, converting a real protocol error into a fake VALIDATION_ERROR. Should Fix: - Cap issue list at 50 and payload node count at 10k to bound CPU on hostile or accidentally-huge payloads before ``iter_errors`` walks them. - Narrow env-var default-flip to ``ADCP_ENV`` only. Generic ``ENV`` / ``ENVIRONMENT`` / ``PYTHON_ENV`` are set by rails, postgres, and 12-factor tooling and would silently flip the SDK's default — footgun. - Suppress ``RefResolver`` DeprecationWarning locally so downstream projects running ``-W error::DeprecationWarning`` don't crash on import. Migration to ``referencing`` tracked as follow-up. - Add ``threading.Lock`` around the lazy schema-index init and the per-validator compile to keep behaviour deterministic under concurrent first-callers. - Replace the ``err.message.split("'")[1]`` required-key heuristic with a regex + schema-required-diff fallback in ``_missing_required_key``. Jsonschema message format drift no longer silently corrupts the JSON Pointer. - ``DebugLogEntry`` is now a ``TypedDict`` instead of a ``dict`` subclass with a ``type: ignore``. Zero runtime cost; better type contract. 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.
Summary
Defaults
Test plan
🤖 Generated with Claude Code