feat(server): per-skill middleware hook in ADCPAgentExecutor (#226)#233
Merged
feat(server): per-skill middleware hook in ADCPAgentExecutor (#226)#233
Conversation
…226) Third and final Phase 2 A2A blocker. With #224 (TaskStore), #225 (PushNotificationConfigStore), and this PR landed, A2A adoption reaches parity with MCP for production agents — salesagent's ~13-day A2A migration becomes ~5-day and net-positive. API - New SkillMiddleware type alias in adcp.server.serve exported from adcp.server. Signature:: async def middleware( skill_name: str, params: dict[str, Any], context: ToolContext, call_next: Callable[[], Awaitable[Any]], ) -> Any: ... Wraps call_next — can invoke it, skip it (short-circuit), wrap in try/except (exception observation), or transform the result. - ADCPAgentExecutor(..., middleware=[...]): sequence stored as a tuple so runtime can't mutate the dispatch chain. First entry wraps outermost — matches Starlette/ASGI ordering. - create_a2a_server(..., middleware=[...]) and serve(..., transport="a2a", middleware=[...]) surface the kwarg end-to-end. Dispatch - ADCPAgentExecutor._dispatch_with_middleware runs the handler wrapped in the chain via a small recursive stepper (no mutable indices, no lambdas closing over loop variables — reads the same whether you have zero or ten middlewares). - Fast path: zero middleware = direct dispatch, no per-call overhead. - Middleware exceptions propagate to execute()'s existing error path. ADCPError → failed task with adcp_error DataPart; other → opaque failed task per the spec's error-sanitisation rule. Tests — tests/test_a2a_server.py (33 passing, +6 this PR) - test_middleware_runs_and_sees_skill_context_and_result: happy path. - test_middleware_composes_outermost_first: locks Starlette/ASGI ordering — first entry enters first, exits last. - test_middleware_can_short_circuit_without_invoking_handler: rate limiter / feature flag case — handler NOT called when middleware returns without call_next. - test_middleware_observes_handler_exceptions: audit hook sees the exception and re-raises; executor's failed-task path still fires. - test_no_middleware_preserves_direct_dispatch: zero middleware = no behavior change. - test_create_a2a_server_threads_middleware_into_executor: kwarg reaches the executor through create_a2a_server. Docs — docs/handler-authoring.md - New "Per-skill middleware (audit, activity feeds, rate limiting, tracing)" subsection with the audit-with-exception-capture recipe. - "Semantics worth knowing" block covers composition, short-circuit, exception observation, context access. - Known-gaps list replaced: all three Phase-2 A2A hooks (#224, #225, #226) have landed. A2A reaches MCP parity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…transform tests Addresses code-reviewer + security-reviewer findings on PR #233. DOCS (security — moved from docs-only to import-site where callers see it) - SkillMiddleware docstring now includes four security callouts: (1) do NOT swallow ADCPError — serves fake success for failed mutations (IdempotencyConflictError, ADCPTaskError). (2) middleware is a data processor for the full skill payload — params contain buyer briefs, budgets, PII; context has caller_identity and tenant_id. Third-party middleware gets the complete surface; treat as controller-processor. (3) exception messages land in server logs verbatim via logger.exception before client-facing sanitisation — do not format params / caller_identity into exception text. (4) short-circuit caches MUST key on (skill_name, params, caller_identity, tenant_id). skill_name + params alone serves principal A's data to principal B. - Clarify params is request-side only; transforms happen on the return side of call_next. - Note ContextVars propagate through call_next (same asyncio task). DOCS — docs/handler-authoring.md - "Semantics worth knowing" expanded with the composition-order WHY (audit outermost so rate-limited rejected calls don't disappear from audit), short-circuit cache-key requirements, retry support, transform-on-return-not-input rule. - New "Security — middleware is a data processor" callout matches the import-site docstring. TESTS (+2, covering code-reviewer's gap list) - test_middleware_can_invoke_call_next_multiple_times_for_retry: retry-on-transient-error pattern. Middleware calls call_next 3 times; handler fails twice, succeeds on third. Locks the re-entrant composition contract a naive loop-variable closure would break. - test_middleware_can_transform_result_on_return_side: enriching middleware wraps the handler's return. Distinct code path from short-circuit (which never calls call_next). NITS - Removed stale `from a2a.types import TaskStatus # noqa: F401 (unused but document` line with truncated comment in test_middleware_can_short_circuit_without_invoking_handler. Deferred (not blocking this PR): - Protocol class for SkillMiddleware (do alongside ContextFactory to keep declaration style consistent). - Runtime validation of middleware return shape against skill output schemas. - ContextVar-propagation formal docs in a dedicated section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Closes 5 items from salesagent's feedback on adopting adcp.server in one cohesive server/transport surface change. SkillMiddleware parity across transports (#7) --------------------------------------------- The A2A executor's per-skill middleware (PR #233) is now available on MCP too. Same SkillMiddleware type alias, same composition semantics (outermost-first, _step recursion), same call_next contract — a middleware list written against one transport works unchanged on the other. - src/adcp/server/serve.py: new module-level _dispatch_with_middleware that A2A's _dispatch_with_middleware delegates to. - create_mcp_server, _register_handler_tools, _register_tool accept middleware=[SkillMiddleware]; _register_tool wraps caller in the chain between context build and handler invocation. - serve() already exposed the kwarg for A2A; now forwards to MCP too. BearerTokenAuthMiddleware in adcp.server.auth (#1) -------------------------------------------------- The pattern in examples/mcp_with_auth_middleware.py was four security-critical concerns (ContextVar carrier, constant-time compare, discovery bypass, reset-in-finally); every downstream copy-pasted it. Now shipped as a class. - src/adcp/server/auth.py: BearerTokenAuthMiddleware, Principal (frozen dataclass), TokenValidator, auth_context_factory, constant_time_token_match. Seller supplies validate_token; framework owns the ContextVar plumbing, RFC 7235 scheme parsing (case- insensitive + whitespace-folded), discovery bypass, peek_jsonrpc with explicit request._body cache, fail-closed validator exception handling, principal metadata that can't shadow SDK audit keys. - examples/mcp_with_auth_middleware.py shrunk 243 → 89 lines. A2A message_parser hook (#3) ---------------------------- ADCPAgentExecutor._parse_request was hardcoded to DataPart({'skill': ..., 'parameters': ...}). Sellers fronting JSON-RPC or vendor-specific shapes had to subclass privately. - src/adcp/server/a2a_server.py: new MessageParser type alias, message_parser= kwarg on ADCPAgentExecutor, create_a2a_server, _serve_a2a, serve(). Default = _default_parse_request (was inline). Startup advertised-tools log (#9) --------------------------------- - src/adcp/server/serve.py: _log_advertised_tools() runs from _register_handler_tools (MCP) and create_a2a_server (A2A). INFO: 'X of Y tools advertised'; DEBUG: list of unadvertised. Custom tools doc (#8) --------------------- docs/handler-authoring.md: new section covering the @mcp.tool() passthrough on create_mcp_server's return value. Expert-review followups (security + code review) ------------------------------------------------- - _parse_bearer_header: case-insensitive scheme, folded whitespace. - validator exceptions → 401 (no stack-trace leak). - principal metadata can't shadow SDK-owned keys (tool_name, transport). - explicit request._body = body after peek. - tests use regex to match log messages (not positional tokens). - Python 3.10 skipif on two new A2A create_a2a_server tests (a2a-sdk starlette integration requires 3.11+; matches pre-existing skip). Tests ----- +53 tests across three new/modified test files. 1990 tests passing, mypy clean. Closes #224, #225, #226, #240, #241 salesagent feedback items #1, #3, #7, #8, #9. 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
Closes #226 (Phase 2 PR-P). Third and final A2A hook salesagent needs. With #224 (TaskStore) + #225 (PushNotificationConfigStore) already merged and this PR, A2A adoption reaches parity with MCP for production agents.
New API:
Dispatch: small recursive stepper runs the chain (no mutable indices, no loop-variable closures). Fast path when no middleware — direct handler call, zero per-call overhead. Middleware exceptions propagate to the executor's existing error-handling path.
Recipes:
Test plan
Phase 2 status
With this merged, Phase 2 (A2A adoption blockers) is complete:
Remaining Phase 2 items from the SDK adoption roadmap (#227 TestController header compat, #228 MCP agent-card, #229 expanded authoring guide) are nice-to-haves, not adoption blockers. salesagent can start their A2A migration as soon as this lands.
🤖 Generated with Claude Code