Skip to content

test(server): regression test for context-echo on dispatcher-wrapped non-AdcpError#568

Merged
bokelley merged 2 commits intomainfrom
bokelley/issue-562-dispatcher-wrap-verify
May 4, 2026
Merged

test(server): regression test for context-echo on dispatcher-wrapped non-AdcpError#568
bokelley merged 2 commits intomainfrom
bokelley/issue-562-dispatcher-wrap-verify

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Closes #562 (verification follow-up to #560).

Why

PR #560 added context-echo on the AdcpError raise path. The follow-up question was: does the dispatcher's exception-wrap path preserve that echo when a non-AdcpError exception escapes a platform method?

What I verified

`_invoke_platform_method` (`src/adcp/decisioning/dispatch.py`) catches every non-AdcpError in its `except Exception` clause and wraps to `AdcpError("INTERNAL_ERROR", recovery="terminal")`. Once wrapped, the AdcpError travels up to `serve.py`'s decisioning branch, which builds the structured envelope via `build_mcp_error_result(exc, params=kwargs)` — so context is echoed onto the wire.

This test pins the chain end-to-end:

  1. `ValueError("oops")` raised inside the platform method
  2. Wrapped to `AdcpError("INTERNAL_ERROR")` by `_invoke_platform_method`
  3. Caught by `serve.py:1798`'s decisioning branch
  4. Projected via `build_mcp_error_result` with `params=kwargs`
  5. Wire result carries `structuredContent.adcp_error.code = "INTERNAL_ERROR"` AND `structuredContent.context = {"correlation_id": "buyer-req-562"}`

If a future refactor lets a non-AdcpError exception escape the dispatch wrap (early-return path, middleware short-circuit, asgi_middleware that catches and re-raises differently), this test fails first — adopters lose correlation IDs across the boundary and we know about it.

Scope

Test-only change, no production code modified. The DecisioningPlatform path is fully covered. Raw `ADCPHandler` users (no DecisioningPlatform) currently see FastMCP's generic error on un-wrapped exceptions — separate concern; not addressed here.

Test plan

  • `pytest tests/test_mcp_structured_error.py::test_non_adcp_error_via_dispatch_wrap_still_echoes_context` — pass
  • All existing structured-error tests — no regressions
  • `ruff check` — clean

🤖 Generated with Claude Code

bokelley and others added 2 commits May 4, 2026 07:31
…non-AdcpError (closes #562)

PR #560 added context-echo on the AdcpError raise path. This test
verifies the implicitly-wrapped path: a raw ValueError from a
DecisioningPlatform method goes through _invoke_platform_method's
except-Exception clause which wraps to AdcpError(INTERNAL_ERROR),
which then projects through serve.py's catch + build_mcp_error_result
with params=kwargs and echoes the request context.

If a future refactor lets a non-AdcpError exception escape the
dispatch wrap, this test fails first.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-reviewer caught that the original assertion (INTERNAL_ERROR +
context echoed) would pass even if the dispatcher wrap step were
skipped. Adds details.caused_by.type == 'ValueError' assertion which
is set only by _internal_error_details inside the wrap path.

Other review nits: executor.shutdown(wait=True) for cleanup; trim
PR-#560 history from docstring per CLAUDE.md; rename for symmetry.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 83ed3ca into main May 4, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

verify: dispatcher non-AdcpError wrap path preserves context echo on error

1 participant