Skip to content

feat(server): expose RequestContext.transport and current_transport ContextVar#627

Merged
bokelley merged 2 commits intomainfrom
claude/issue-617-expose-requestcontext-transport
May 10, 2026
Merged

feat(server): expose RequestContext.transport and current_transport ContextVar#627
bokelley merged 2 commits intomainfrom
claude/issue-617-expose-requestcontext-transport

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #617

Summary

Adds RequestContext.transport: Literal["mcp", "a2a"] | None so handler methods can branch on the inbound wire protocol without 85 lines of ASGI URL-inference middleware. Also exports current_transport: ContextVar[Literal["mcp", "a2a"] | None] from adcp.server for webhook services and other code running outside a handler call stack. Fixes a pre-existing gap where current_principal and current_principal_metadata were only accessible via the private adcp.server.auth path rather than the public adcp.server surface.

# In a handler method:
async def create_media_buy(self, params, context):
    if context.transport == "a2a":
        await send_task_status_update(...)
    else:
        await send_mcp_webhook(...)

# In a webhook service outside the handler call stack:
from adcp.server import current_transport
transport = current_transport.get()  # "mcp" | "a2a" | None

What was tested

ruff check src/                       # All checks passed
mypy src/adcp/                        # No errors
pytest tests/test_decisioning_dispatch.py tests/test_auth_middleware.py -v
# 93 passed, 13 warnings
pytest tests/ -q                      # 4213 passed, 1 pre-existing TLS network test failed (unrelated), 33 skipped

Pre-PR review

  • code-reviewer: approved — no CI-blocking findings. Flagged (a) validation of raw metadata value, (b) SDK-internal keys leaking into handler-visible ctx.metadata, (c) missing ContextVar assertion in test. All three fixed before PR open.
  • dx-expert: approved — blocker was current_transport.set() without reset token; resolved via explanatory comment noting asyncio task-context isolation (each request task copies context at creation, so set() is task-scoped and cannot bleed across requests; resetting immediately after _build_request_context returns would undo the value before any handler code runs). Also fixed: cross-link to current_transport in field docstring, note that custom context_factory omitting metadata["transport"] also produces None.

Nits surfaced, not fixed:

  • current_tenant (from tenant_router.py) and current_principal (from auth.py) are heterogeneous in origin and type in __init__.py — pre-existing, noted for a future cleanup PR.
  • @pytest.mark.parametrize is now used for the transport extraction test (clearer failure messages vs. the original for loop).

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_01UifYgpi26gbfXmhrNRFDvK


Generated by Claude Code

@bokelley bokelley marked this pull request as ready for review May 10, 2026 13:09
@bokelley
Copy link
Copy Markdown
Contributor Author

Adopter confirmation (filed #617)

Read the diff. This kills our core/middleware/transport_detect.py cleanly (85 LOC of URL-path classification + ContextVar plumbing).

Two specific things I like:

  1. Both surfaces exposed: `ctx.transport` for handler-scope reads, `adcp.server.current_transport` ContextVar for code outside the call stack (our webhook service was the original need — see Fix admin authentication and routing for domain-based access prebid/salesagent#202). Either works in the right context; neither is forced.
  2. Stripping `metadata["transport"]` from handler-visible metadata — the right call. Adopters reading the typed field can't accidentally also read an undocumented dict path that might drift.

One verification request

The PR reads from `tool_ctx.metadata["transport"]`. This depends on the SDK's MCP and A2A dispatchers always populating that key on the `ToolContext` they hand to `_build_request_context`. The diff covers `_build_request_context` cleanly but I don't see the dispatcher-side population in the diff scope. Can you confirm both paths set `metadata["transport"]` before this lands? An adopter who hits a `transport=None` in production handler code would have a confusing time.

If the dispatcher wiring is in another PR, please link it; if it's already on `main`, the test count (15 passing) suggests it's fine.

Adoption plan

Once this lands and we bump:

  1. Replace `from core.middleware.transport_detect import current_transport` with `from adcp.server import current_transport` in our webhook service.
  2. Convert `platform._build_identity` to read `ctx.transport` directly instead of the ContextVar.
  3. Delete `core/middleware/transport_detect.py` and remove the entry from `_serve_kwargs`.

Small, mechanical change on our side. Ready when you are.

@bokelley
Copy link
Copy Markdown
Contributor Author

@bokelley — confirmed, no separate PR needed. The dispatcher-side wiring was already on main before this diff, in two places:

  1. Default context_factory (src/adcp/server/serve.py:487) — the factory serve() uses when no custom one is supplied:

    metadata={"tool_name": meta.tool_name, "transport": meta.transport}
  2. auth_context_factory (src/adcp/server/auth.py:420-423) — the auth-aware factory used when AuthConfig is provided:

    combined_metadata: dict[str, Any] = {
        **principal_metadata,
        "transport": meta.transport,
        ...
    }

Both receive RequestMetadata where transport: Literal["mcp", "a2a"] is a required, non-optional field populated by the server framework itself for every MCP and A2A call — not by user code. The 15 passing transport-extraction tests are a reliable signal.

The only None path (documented in the RequestContext.transport docstring added by this PR) is a custom context_factory that doesn't forward meta.transport into metadata. That's a user-owned factory, not an SDK dispatch path, so production SDK dispatch is always populated.


Generated by Claude Code

claude added 2 commits May 10, 2026 09:14
…ontextVar

Closes #617

Adopters building webhook services need to know whether the current
request arrived via MCP or A2A so they can select the correct payload
shape (Task/TaskStatusUpdateEvent vs McpWebhookPayload). The transport
was already known to the dispatcher (via RequestMetadata.transport) but
was buried in the opaque ToolContext.metadata dict with no typed surface.

- Add `transport: Literal["mcp", "a2a"] | None` field to RequestContext;
  populated in _build_request_context from tool_ctx.metadata["transport"]
  (always present in production paths; None only in bare test fixtures)
- Add `current_transport: ContextVar[Literal["mcp", "a2a"] | None]` to
  adcp.server.auth; set in _build_request_context so webhook services
  called from handlers can read it without a RequestContext in scope
- Export current_transport from adcp.server (alongside current_tenant)
- Also export current_principal and current_principal_metadata from
  adcp.server — pre-existing gap where these were accessible only via
  the private adcp.server.auth module path

https://claude.ai/code/session_01UifYgpi26gbfXmhrNRFDvK
- Validate that metadata["transport"] is "mcp", "a2a", or None before
  assigning to RequestContext.transport; raises ValueError on invalid
  values so misconfigured context_factories fail visibly
- Strip SDK-internal keys ("transport", "tool_name") from the
  handler-visible RequestContext.metadata so ctx.transport is the sole
  typed surface and adopters can't accidentally rely on the dict path
- Add Literal import to dispatch.py for the explicit transport annotation
- Add current_transport ContextVar assertion and "tool_name" not in
  ctx.metadata assertions to the parametrized transport extraction test
- Extend RequestContext.transport docstring to cross-link current_transport
  and note that custom context_factories that omit metadata["transport"]
  also produce None

https://claude.ai/code/session_01UifYgpi26gbfXmhrNRFDvK
@bokelley bokelley force-pushed the claude/issue-617-expose-requestcontext-transport branch from 2a9654a to 78ed926 Compare May 10, 2026 13:14
@bokelley
Copy link
Copy Markdown
Contributor Author

Verified dispatcher write sites

Confirmed metadata["transport"] is populated everywhere a RequestMetadata is constructed:

  • auth_context_factory (the canonical SDK-provided factory at src/adcp/server/auth.py consumed in src/adcp/server/serve.py:487):
    metadata={"tool_name": meta.tool_name, "transport": meta.transport}
  • MCP path when context_factory= is wired (src/adcp/server/serve.py:2049):
    meta = RequestMetadata(tool_name=name, transport="mcp")
  • A2A path when context_factory= is wired (src/adcp/server/a2a_server.py:294-298):
    meta = RequestMetadata(tool_name=skill_name, transport="a2a", request_id=request.task_id)

One caveat to call out — already documented in the field docstring

Adopters who do not wire a context_factory fall through to bare-ToolContext construction (ToolContext() in mcp_tools.call_tool, _tool_context_from_request(request) on the A2A side at a2a_server.py:539). Neither populates metadata["transport"], so ctx.transport resolves to None.

The PR's docstring on RequestContext.transport already calls this out:

None when RequestContext is constructed in tests without a transport-aware ToolContext, or when a custom context_factory omits metadata["transport"]. Production dispatch always populates this field.

The "production dispatch always populates" claim is true for adopters using the SDK-provided auth_context_factory or any factory that propagates meta.transport — which is the documented pattern. Custom factories that omit it get None; that's surfaced as a documented contract rather than a silent footgun.

Net: the read site (_build_request_context) is correctly paired with write sites at every dispatcher entry point. ✅

@bokelley
Copy link
Copy Markdown
Contributor Author

Thanks for the write-site audit, @bokelley. The None-on-missing-factory contract being documented in the field docstring rather than enforced with a runtime error is the right call — custom factories are a supported escape hatch and the docstring makes the gap explicit. No follow-up action needed from this side.


Triaged by Claude Code. Session: https://claude.ai/code/session_01QMJzJHYW6SzcPUZTAE9WTU


Generated by Claude Code

@bokelley bokelley merged commit 20e5d53 into main May 10, 2026
16 of 17 checks passed
@bokelley bokelley deleted the claude/issue-617-expose-requestcontext-transport branch May 10, 2026 15:14
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.

feat(server): expose RequestContext.transport (mcp | a2a)

2 participants