Skip to content

feat(client): ADCPClient.from_mcp_client() factory for in-process MCP transport#293

Open
bokelley wants to merge 4 commits intomainfrom
claude/issue-291-from-mcp-client-factory
Open

feat(client): ADCPClient.from_mcp_client() factory for in-process MCP transport#293
bokelley wants to merge 4 commits intomainfrom
claude/issue-291-from-mcp-client-factory

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #291

Summary

Adds ADCPClient.from_mcp_client(client, *, agent_id=None, ...) — a classmethod that wraps a pre-connected mcp.ClientSession and returns a fully configured ADCPClient. Parity with JS AgentClient.fromMCPClient() (v5.19.0). The primary use case is compliance test fleets that need the full client path exercised against an in-process MCP server via mcp.shared.memory.create_client_server_memory_streams, without standing up a loopback HTTP server.

Mechanics: MCPAdapter._get_session() already short-circuits when self._session is not None. The factory injects the caller-owned session there and sets a _session_is_injected flag so close() skips _cleanup_failed_connection — the caller owns the lifecycle. All existing client surface (validation, idempotency, validate_features, capability cache) works unchanged through the factory path. Request signing is intentionally absent from the factory params: the hook is wired into the HTTP transport layer that is bypassed here.

What was tested

  • pytest tests/test_protocols.py::TestFromMcpClientFactory — 8 new tests: session injected, unique default agent_id, explicit agent_id, tool calls route through injected session, close() no-op, async with exit no-op, validation wired, unique idempotency tokens
  • pytest tests/ -x -q --ignore=tests/integration --ignore=tests/conformance/signing/test_ip_pinned_transport.py — 2185 passed, 21 skipped, 0 failures
  • ruff check src/adcp/protocols/mcp.py src/adcp/client.py — clean
  • mypy src/adcp/protocols/mcp.py src/adcp/client.py — clean

Nits surfaced (not fixed — reviewer discretion)

  • test_tool_call_routes_through_injected_session calls adapter._call_mcp_tool directly rather than the public get_products method; either level is valid, but the public surface is more refactor-stable
  • strict_idempotency=True wires idempotency_capability_check on the adapter; a test asserting the hook is set would complete coverage of that flag
  • No test for the uninitialized-session failure mode (callers who forget await session.initialize() get an opaque MCP error string in TaskResult.error)
  • testing-guide.md (if present) does not reference from_mcp_client — follow-on documentation task

Bucket gap

No client label exists in this repo. Bucket for this issue is client (ADCPClient surface). Flagged in run summary.

Pre-PR review

  • code-reviewer: approved — no blockers; 3 issues resolved in follow-up commits (None guard on agent_id, if not isinstance guard replacing assert, ClientSession type annotation via TYPE_CHECKING)
  • dx-expert: approved — no blockers; nit fixes applied (param ordering to match __init__, inline lifecycle comment in docstring example); server-wiring placeholder comment noted as adequate for first iteration

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_014Lazxs8FCWcVzzKi8MpLPi


Generated by Claude Code

claude added 3 commits April 29, 2026 01:21
- Use explicit None check for agent_id (empty string now preserved)
- Replace assert isinstance with explicit RuntimeError for -O safety

https://claude.ai/code/session_014Lazxs8FCWcVzzKi8MpLPi
…rom_mcp_client

- Type client as ClientSession via TYPE_CHECKING import
- Reorder params (capabilities_ttl, validate_features, strict_idempotency)
  to match ADCPClient.__init__ for consistency
- Add inline comment clarifying adcp_client lifecycle in docstring example

https://claude.ai/code/session_014Lazxs8FCWcVzzKi8MpLPi
Acting on dx-expert review of this PR:

- ``ADCPClient.from_mcp_client`` docstring: add a Warning block above
  Args. Surfaces three traps a docstring-skimmer would otherwise miss:
  (1) ``close()`` and ``async with`` exit are no-ops on injected
  sessions, (2) webhook + ``on_activity`` are intentionally not wired
  because there is no HTTP transport for a callback, (3) an
  uninitialized session surfaces as an opaque MCP error rather than a
  friendly one.
- Move ``from uuid import uuid4`` to module-level imports — the
  ``uuid4 as _uuid4`` rebind inside the method was unconventional with
  no benefit.
- ``docs/testing-guide.md``: new "In-Process MCP Fixtures" section
  showing the canonical ``contextlib.AsyncExitStack`` +
  ``create_client_server_memory_streams`` + ``ClientSession`` +
  ``ADCPClient.from_mcp_client`` pattern. Without this, the factory is
  invisible outside the docstring and someone reading the testing
  guide reaches for a loopback HTTP server (and re-files the parity
  issue, which is how this PR started).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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(client): AgentClient.from_mcp_client() factory for in-process MCP transport

2 participants