Skip to content

feat(server): response_enhancer callback (JS #2161 parity)#933

Merged
bokelley merged 2 commits into
mainfrom
claude/issue-926-response-enhancer
Jun 7, 2026
Merged

feat(server): response_enhancer callback (JS #2161 parity)#933
bokelley merged 2 commits into
mainfrom
claude/issue-926-response-enhancer

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented Jun 7, 2026

Summary

Adds a server-wide response_enhancer callback that stamps cross-cutting fields on the response classes that converge at the SDK's dispatch seams — framework-tool successes, custom-tool successes (get_task_status / list_tasks), the pre-auth get_adcp_capabilities discovery response, and raised-error responses (including credential-policy errors) — across the MCP and A2A transports. This is the Python parity for JS adcontextprotocol/adcp-client#2161.

Coverage (corrected — see adcp-client-python helpers.py ResponseEnhancer docstring for the authoritative list). An earlier draft of this summary described coverage as applying to "every response class … uniformly across MCP and A2A." That overstates it. Accurate coverage:

Enhanced (both MCP and A2A unless noted):

  • framework-tool successes;
  • custom-tool successes (get_task_status / list_tasks);
  • the pre-auth get_adcp_capabilities discovery response;
  • error responses from a raised AdcpError / ADCPTaskError (incl. credential-policy errors), stamped by the transport error finalizers. The common adcp_error() helper shape ({"errors": [...]}) has no top-level adcp_error key, so a handler that returns it is stamped on the success path;
  • the A2A comply_test_controller sandbox skill.

NOT enhanced (two seams):

  • S1 — the MCP comply_test_controller sandbox path (src/adcp/server/test_controller.py) returns the handler result directly with no success-path enhancer call, so its A2A counterpart is stamped but its MCP counterpart is not. Sandbox-only.
  • S2 — a handler that returns (rather than raises) a raw AdCP L3 envelope {"adcp_error": {...}}: the success-path guard (if "adcp_error" not in result) skips it, and a returned envelope never reaches the raised-error finalizers, so it ships un-enhanced. Raise the error (or return the {"errors": [...]} helper shape) to have it stamped.

Design

The Python SDK has no single response finalizer, so the enhancer is wired at the dispatch seams that converge the response classes, all after inject_context (so a buggy/malicious enhancer cannot re-introduce a stripped credential into the echo envelope):

  1. Successcreate_tool_caller (src/adcp/server/mcp_tools.py): after inject_context and before validate_response, so a conformance-breaking mutation surfaces as VALIDATION_ERROR rather than shipping malformed. This single site covers framework tools, custom tools, and get_adcp_capabilities on both MCP and A2A (all caller-build sites share call_tool). The guard if "adcp_error" not in result here is what produces the S2 gap above.
  2. MCP errorsbuild_mcp_error_result (src/adcp/server/translate.py).
  3. A2A errors_send_adcp_error (src/adcp/server/a2a_server.py).

Plus the A2A comply_test_controller bypass: that skill is served by _call_test_controller (not create_tool_caller), so the enhancer is applied there too — context echoed first to keep the credential-echo invariant. The MCP comply_test_controller path has no equivalent enhancer call, which is the S1 gap above.

Enhancer contract

  • ResponseEnhancer = Callable[[dict], None] | Callable[[str, dict, ToolContext | None], None] — dispatched by signature arity (1 positional → context-blind; 3 / *args → context-aware).
  • Synchronous, mutates the response dict in place, return value ignored.
  • A raised exception is caught and logged at WARNING; the un-enhanced response ships (a buggy enhancer must never become a transport error).
  • Tolerates an unauthenticated / None context (pre-auth discovery).

Threading

ServeConfig.response_enhancerserve()_serve_mcp / _serve_a2a / _serve_mcp_and_a2acreate_mcp_server / create_a2a_server → per-tool create_tool_caller + the error finalizers. adcp.decisioning.serve.serve forwards it via **serve_kwargs. Also threaded through the in-process build_asgi_app / build_test_client test harness. ResponseEnhancer is exported from adcp.server (matching ServeConfig / SkillMiddleware; the adcp.__all__ snapshot is unchanged).

Idempotency

The server-side idempotency cache commits the pre-enhancement response, so a replayed request re-runs the enhancer — non-idempotent enhancers diverge on replay. Documented on the ResponseEnhancer type.

Tests

tests/test_response_enhancer.py (37 tests): the enhanced response classes × both transports, 1-arg/3-arg arity dispatch, before-validate (enhancer-injected non-conformant field → VALIDATION_ERROR), throwing enhancer → logged warning + un-enhanced response (not a transport error), no-enhancer regression, credential-echo ordering (enhancer runs after inject_context), the A2A comply bypass, and config-threading to both transports. The two non-enhanced seams (S1 MCP comply, S2 returned raw L3 envelope) are documented rather than enhanced.

Verification: targeted suites + full suite (5608 passed) + make lint + make typecheck + make typecheck-all + make validate-generated, all green.

Closes #926

🤖 Generated with Claude Code

bokelley and others added 2 commits June 7, 2026 07:24
Add a server-wide response_enhancer callback that stamps cross-cutting
fields on every response class — framework-tool successes, custom-tool
successes (get_task_status / list_tasks), the pre-auth
get_adcp_capabilities discovery response, and structured adcp_error
responses — uniformly across the MCP and A2A transports.

The Python SDK has no single response finalizer, so the enhancer is wired
at the three dispatch seams that converge the response classes, all AFTER
inject_context (so a buggy enhancer cannot re-introduce a stripped
credential into the echo envelope):

1. Success — create_tool_caller (mcp_tools.py): after inject_context and
   before validate_response, so a conformance-breaking mutation surfaces
   as VALIDATION_ERROR rather than shipping malformed. Covers framework
   tools, custom tools, and get_adcp_capabilities on both transports.
2. MCP errors — build_mcp_error_result (translate.py).
3. A2A errors — _send_adcp_error (a2a_server.py).

Also closes the A2A comply_test_controller gap: that skill bypasses
create_tool_caller via _call_test_controller, so the enhancer is applied
there too (context echoed first to preserve the credential-echo invariant).

The enhancer supports both a context-blind Callable[[dict], None] and a
context-aware Callable[[str, dict, ToolContext | None], None], dispatched
by signature arity. It runs synchronously, mutates in place, and a raised
exception is caught + logged at WARNING — a buggy enhancer must not become
a transport error.

response_enhancer threads through ServeConfig, serve(), all three
_serve_* transport paths, create_mcp_server / create_a2a_server, and the
in-process build_asgi_app / build_test_client test harness.
adcp.decisioning.serve.serve forwards it via **serve_kwargs. The
ResponseEnhancer type alias is exported from adcp.server (matching
ServeConfig / SkillMiddleware; adcp.__all__ snapshot unchanged).

Idempotency note: the cache commits the pre-enhancement response, so a
replay re-runs the enhancer — non-idempotent enhancers diverge on replay.

Closes #926

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ResponseEnhancer / serve() docstrings claimed the enhancer applies to
"every response". Two paths are not enhanced:

- the MCP comply_test_controller sandbox path returns the handler result
  directly without a success-path enhancer call (its A2A counterpart is
  stamped);
- a handler that returns (vs raises) a raw L3 {"adcp_error": {...}}
  envelope is skipped by the success-path guard and never reaches the
  raised-error finalizers.

Document the exact enhanced paths and these two gaps on the canonical
ResponseEnhancer docstring, point _apply_response_enhancer at it, and
soften the serve() docstrings from "every response". Docstring-only; no
behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review June 7, 2026 11:38
@bokelley bokelley enabled auto-merge (squash) June 7, 2026 11:38
Copy link
Copy Markdown
Contributor

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean additive change, correctly wired. The architectural call that makes this safe: the enhancer runs strictly after inject_context at every seam, so a buggy or malicious enhancer cannot re-introduce a credential the echo step stripped — fail-closed beats fail-open.

Things I checked

  • Credential-echo ordering holds at all four sites. Success: inject_context at mcp_tools.py:2615 → enhancer immediately after, before validate_response. MCP error: translate.py inject_context → enhancer before CallToolResult. A2A error: _send_adcp_error echo → enhancer before _make_task. A2A comply bypass: manual inject_context → enhancer, and the later _send_result echo is a true no-op because inject_context is guarded by "context" not in response. security-reviewer: clean, no High.
  • A2A success reaches the enhancer. ADCPAgentExecutor routes regular skills through create_tool_caller — the same success seam — so one callback covers both transports. _send_result does not re-run it. No double pass anywhere: success enhances the returned dict and raises on error; the two error finalizers each run once on their raise paths.
  • if \"adcp_error\" not in result guard sits inside if isinstance(result, dict) (mcp_tools.py:2608), so it can't hit a non-dict, and it deliberately routes raised-error envelopes to the dedicated finalizers to avoid a double stamp.
  • Arity dispatch (_enhancer_is_context_aware, helpers.py) handles bound methods (signature drops self), *args → context-aware, and falls back to context-blind on signature() failure. Exception swallowing logs at WARNING with exc_info and ships the un-enhanced (strictly safer, addition-only) response.
  • Public surface is additive. New ResponseEnhancer export from adcp.server, new optional response_enhancer=None kwarg threaded through serve / create_mcp_server / create_a2a_server / build_asgi_app / build_test_client / decisioning.serve. No signature breaks, no required↔optional flips. feat(server): is the right semver signal — ad-tech-protocol-expert: sound-with-caveats, no wire-contract divergence.
  • Test plan. Full suite 5608 passed, lint/typecheck/typecheck-all/validate-generated green per the PR body; no unchecked manual boxes. tests/test_response_enhancer.py exercises real wire dicts through the executor across all four response classes and both transports, plus the before-validate conformance break and the throwing-enhancer fail-open.

Follow-ups (non-blocking — file as issues)

  • Third coverage gap: legacy-adapter reshape drops the stamp. The enhancer stamps result at mcp_tools.py:2615, but legacy_adapter.normalize_response(result) at :2652 reassigns result to a fresh dict that may not carry enhancer-added keys. For buyers on a legacy wire shape the stamp silently no-ops. Additive-only, so nothing breaks — but it's a real third gap alongside the documented S1/S2. Either move the enhancer after normalize_response or add it to the ResponseEnhancer coverage-gaps list.
  • Error-path output is unvalidated. Success-path enhancer output is schema-checked before shipping; error envelopes never are (pre-existing — error envelopes skip validation everywhere). So an enhancer that pops code/message off an adcp_error envelope ships malformed with no safety net. Acceptable under the current spec posture (errors aren't schema-gated anywhere), but a cheap WARNING when the L3 invariant is missing post-enhance would close the asymmetry.
  • Docs drift. ResponseEnhancer is a new public export — check AGENTS.md / README.md reflect it.

Minor nits (non-blocking)

  1. Scope the credential claim in the docstring. The ResponseEnhancer docstring (helpers.py) says the enhancer "cannot re-introduce a credential." True only for credentials echoed via context. The context-aware arity hands the enhancer a live ToolContext (and on AccountAwareToolContext, the seller account object) — copying a token out of that into the response is the enhancer author's footgun, not something the ordering guarantee prevents. Worth one sentence steering authors away from it, and toward the passed-context arity over ContextVar-sourced tenant state given the documented pre-enhancement idempotency commit.
  2. 2-arg enhancer fails open silently. A mistyped (method_name, result) signature classifies context-blind, gets one arg, raises TypeError, and is logged as an enhancer bug + shipped un-enhanced. Defensible under the documented two-arity contract, but a one-line docstring note would save an adopter a confused afternoon.

LGTM. Follow-ups noted below — the legacy-adapter gap is the one worth filing.

@bokelley bokelley merged commit 659cd7c into main Jun 7, 2026
28 checks passed
@bokelley bokelley deleted the claude/issue-926-response-enhancer branch June 7, 2026 11:44
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.

feat(server): response enhancer callback (JS #2161 parity)

1 participant