Skip to content

fix(decisioning): wrap pydantic.ValidationError from delegates as INVALID_REQUEST#656

Merged
bokelley merged 3 commits into
mainfrom
claude/issue-652-validation-error-invalid-request
May 11, 2026
Merged

fix(decisioning): wrap pydantic.ValidationError from delegates as INVALID_REQUEST#656
bokelley merged 3 commits into
mainfrom
claude/issue-652-validation-error-invalid-request

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #652

Summary

Platform delegates that manually call RequestModel.model_validate() against a stricter subclass schema previously surfaced as INTERNAL_ERROR (terminal) when the buyer's input failed validation, stripping all field information from the wire envelope. This PR converts those pydantic.ValidationError exceptions to INVALID_REQUEST (correctable) with populated field and details.validation_errors, matching the behaviour already present in _coerce_params_to_platform_type for the annotation-coercion path.

Two changes:

  1. _validation_error_to_invalid_request helper — builds the AdcpError("INVALID_REQUEST") with narrow_union_errors-filtered field paths, with a safe fallback if the narrowing import fails. Input values are excluded from the error payload on both paths (include_input=False) to avoid leaking buyer-supplied data.

  2. except _ValidationError clause in _invoke_platform_method — inserted between except TypeError and except Exception, fires on_failure hook (so proposal-flow reservations are released) before re-raising the wrapped error.

Note: the framework cannot distinguish a buyer-request ValidationError from a seller-response construction ValidationError at this catch boundary. The tradeoff is intentional and consistent with _coerce_params_to_platform_type — surfacing INVALID_REQUEST correctable with field paths is strictly more informative than INTERNAL_ERROR terminal in either case.

What was tested

  • pytest tests/test_decisioning_dispatch.py56 passed (includes updated test_invoke_validation_error_surfaces_narrowed_field_paths and new test_invoke_validation_error_from_manual_model_validate_is_invalid_request)
  • pytest tests/ --ignore=tests/integration4298 passed, 0 new failures (1 pre-existing TLS integration test excluded)
  • ruff check src/adcp/decisioning/dispatch.py → all checks passed
  • mypy src/adcp/decisioning/dispatch.py --ignore-missing-imports → no new errors on changed lines

Pre-PR review

  • code-reviewer: approved after 1 blocker fixed — fallback exc.errors() call now passes include_input=False, include_context=False to prevent buyer-supplied data leaking into details.validation_errors; 1 nit noted (exc: Any signature loses static type info)
  • dx-expert: approved — field dot-join produces correct "extra_field" for extra='forbid' errors; details shape asymmetry from _internal_error_details is intentional; no suggestion field is consistent with all peer INVALID_REQUEST wraps in the same file

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_01J7nYYToyy7PCs6CgQGRAWN


Generated by Claude Code

claude added 2 commits May 11, 2026 07:24
…ALID_REQUEST

Closes #652

Platform methods that manually call `RequestModel.model_validate()` with
a buyer's payload against a stricter subclass schema now surface as
`INVALID_REQUEST` (correctable) with field paths populated from the
validation error, instead of the previous `INTERNAL_ERROR` (terminal)
that stripped all field information from the wire envelope.

The fix adds an `except pydantic.ValidationError` clause in
`_invoke_platform_method` between the existing `except TypeError` and
`except Exception` handlers, matching the behaviour already present in
`_coerce_params_to_platform_type` for the annotation-coercion path.
A shared `_validation_error_to_invalid_request` helper builds the
AdcpError with `narrow_union_errors`-filtered `details.validation_errors`
so buyers get the same structured field list on both paths.

https://claude.ai/code/session_01J7nYYToyy7PCs6CgQGRAWN
Pre-PR review blocker: the fallback exc.errors() call in
_validation_error_to_invalid_request was missing include_input=False
and include_context=False, which could expose buyer-supplied field
values in details.validation_errors on the wire when narrow_union_errors
is unavailable.

https://claude.ai/code/session_01J7nYYToyy7PCs6CgQGRAWN
…_REQUEST

Review feedback on PR #656. The nested try/except in
_validation_error_to_invalid_request swallowed errors from
narrow_union_errors() and a second call to exc.errors(), violating the
repo's no-fallbacks policy. Both APIs are stable (in-repo + Pydantic
public surface); a failure there is a real bug we want surfaced.

Also tighten the `exc` parameter from `Any` to `ValidationError` via the
existing TYPE_CHECKING import block — static type tools now flag misuse
at the call site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review May 11, 2026 07:43
@bokelley bokelley merged commit 976ab4f into main May 11, 2026
16 checks passed
@bokelley bokelley deleted the claude/issue-652-validation-error-invalid-request branch May 11, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Framework wraps pydantic.ValidationError as INTERNAL_ERROR; should be INVALID_REQUEST

2 participants