Skip to content

feat(server): thread ToolContext through TestControllerStore (closes #227)#237

Merged
bokelley merged 2 commits intomainfrom
bokelley/test-controller-header-compat
Apr 20, 2026
Merged

feat(server): thread ToolContext through TestControllerStore (closes #227)#237
bokelley merged 2 commits intomainfrom
bokelley/test-controller-header-compat

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Closes #227 — composes header-driven test runtime (e.g. salesagent's AdCPTestContext.from_headers(request.headers)) with the SDK's storyboard-driven TestControllerStore.

  • register_test_controller(mcp, store, *, context_factory=None) accepts the same context_factory as create_mcp_server. Per-call ToolContext threads into store methods that opt in.
  • Store methods MAY declare *, context: ToolContext | None = None. Dispatcher detects via inspect.signature — legacy stores without the kwarg keep working unchanged.
  • serve() auto-wires context_factory into register_test_controller.
  • A2A executor threads the same ToolContext it builds into _handle_test_controller.
  • New "Testing hooks" section in docs/handler-authoring.md showing the header-driven + storyboard-driven composition side-by-side.

Rationale

The issue proposed a parallel test_context_factory. Rejected in favor of reusing the existing context_factory because:

  1. One factory contract = one mental model for sellers.
  2. Header-derived state goes into ToolContext.metadata[\"test_context\"], readable by both regular handler methods AND store methods — not just the storyboard skill.
  3. Signature-based opt-in on store methods (*, context=None) keeps backward compat watertight.

Test plan

  • Unit: _accepts_context_kwarg detects keyword-only + **kwargs, rejects legacy signatures
  • Unit: context propagates into store methods that opt in
  • Unit: legacy store without context kwarg keeps working
  • Unit: context=None path doesn't pass None to legacy-shape methods
  • Integration: end-to-end register_test_controller with context_factory pattern using a ContextVar (the documented salesagent shape)
  • Integration: factory returning non-ToolContext fails loud at call time
  • All existing tests pass (1847/1847)
  • mypy clean (672 source files)

🤖 Generated with Claude Code

bokelley and others added 2 commits April 20, 2026 10:28
…227)

TestControllerStore is storyboard-shaped — scenarios dispatch via the
comply_test_controller skill. Downstream sellers whose test runtime
reads request headers (AdCPTestContext.from_headers) had no way to
compose header-driven mock state with the SDK's storyboard testing.

register_test_controller now accepts the same context_factory as
create_mcp_server. The dispatcher builds a ToolContext per call and
threads it into store methods that declare a `*, context` keyword.

Backward-compat: the dispatcher uses inspect.signature to detect the
opt-in. Stores that don't declare `context` keep working unchanged.

Also:
- serve() auto-wires context_factory into register_test_controller
- a2a_server executor now passes its ToolContext into the store dispatch
- docs/handler-authoring.md gets a "Testing hooks" section documenting
  the header-driven + storyboard-driven composition pattern

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code reviewer:
- _accepts_context_kwarg now rejects positional-only `context`. A
  method like `def fn(self, context, /, ...)` would previously register
  as opted-in but raise TypeError when the dispatcher passes
  `context=ctx` by keyword.
- Documented the functools.wraps caveat — inspect.signature follows
  __wrapped__, which is the intended contract but worth noting.
- Added tests: positional-only context, @functools.wraps-preserving
  decorator (both legacy and modern signatures), inherited override
  from an intermediate base class via mixin.

Security reviewer (L1):
- TestHeaderMiddleware docs example now resets the ContextVar token in
  finally. Without reset(token) the set value leaks into the next
  request reusing the asyncio task — same shape as the PR #232 cross-
  tenant idempotency scoping issue.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit a4c6489 into main Apr 20, 2026
10 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.

Phase 2: TestControllerStore + AdCPTestContext header-driven compatibility (PR-R)

1 participant