--- id: http/review-mcp name: Review MCP adapters for ADR-037/041 conformance (streamable HTTP, 4-tool gateway, output handling) status: completed depends_on: [http/adapters/from-mcp, http/adapters/to-mcp] scope: moderate risk: low impact: phase level: review --- ## Description Review the MCP adapters (`from_mcp`, `to_mcp`) for spec conformance, pattern consistency, and correctness. This is the quality checkpoint for the feature-gated MCP work. ### Review Checklist 1. **`from_mcp` conformance** (http-mcp.md): - `FromMCP` struct with `endpoint`, `auth_token`, `namespace` - `OperationAdapter` impl (`async fn import`) - Connects via rmcp `StreamableHttpClientTransport::from_uri(endpoint)` - Connection failure → `AdapterError::DiscoveryFailed` - 401 → `AdapterError::Unauthorized` - Calls `tools/list` → MCP tools - For each tool: `HandlerRegistration` with `FromMCP` provenance - `spec.name` = tool name (or `namespace/tool_name`) - `spec.op_type` = `Mutation` (MCP tools are call/response) - `spec.visibility` = `Internal` (ADR-015) - `spec.input_schema` = tool's `inputSchema` - `spec.output_schema` = declared `outputSchema` if present, else `ContentBlock` union - `provenance` = `FromMCP`, `composition_authority: None`, `scoped_env: None` (ADR-022) - `capabilities` = bearer token (injected at registration) 2. **`from_mcp` output handling** (http-mcp.md §"Output handling"): - `structured_content` present → use as result (validated against `output_schema`) - `structured_content` absent → map `content: Vec` to `ContentBlock` union - No heuristic `JSON.parse` of text blocks (carry as `ContentBlock`) - `isError: true` → `CallError` (not the output handling path) - rmcp client connection maintained for registration lifetime 3. **`from_mcp` no-env-vars** (ADR-014): - Handler reads `context.capabilities`, never `std::env::var` - Bearer token from `Capabilities`, not env vars 4. **`to_mcp` conformance** (http-mcp.md, ADR-041): - Implements rmcp `ServerHandler` trait (`call_tool`, `list_tools`) - `tools/list` returns 4 fixed gateway tools (`search`, `schema`, `call`, `batch`) - `tools/list` does NOT return registry's operations (discovered via `search`) - `search` → dispatches `services/list` via `GatewayDispatch::invoke` - `search` results are `AccessControl::check(identity)`-filtered - `Subscription` ops filtered out of `search` results (ADR-041 §2) - `schema` → dispatches `services/schema` via `GatewayDispatch::invoke` - `call` → dispatches via `GatewayDispatch::invoke` (shared spine) - `call` result → `CallToolResult::structured(value)` for `Ok` - `call` error → `CallToolResult::structured_error(details)` for `Err(CallError)` - `batch` → loop over `invoke`, returns array - `to_mcp` is a pure projection (consumes registry, does not produce entries) 5. **`to_mcp` auth** (http-mcp.md, research §4.4): - Bearer auth via shared `bearer_auth_middleware` (applied around `nest_service`) - `Identity` read from `RequestContext.extensions` inside `call_tool` - Identity survives rmcp framing (research §6 #2 — confirmed by spike) - MCP client has no `PeerId` (not an alknet peer, ADR-034 §4) - `AccessControl` gates `search` results and `call` dispatch 6. **`to_mcp` rmcp integration** (http-mcp.md, research §4): - `StreamableHttpService` nested into axum `Router` at `/mcp` - `ServerHandler` impl is a gateway service (4 fixed tools) - rmcp owns JSON-RPC framing, session management, SSE priming - `to_mcp` owns `call_tool`/`list_tools` dispatch + `CallToolResult` mapping 7. **Streamable HTTP only** (ADR-037): - stdio transport NOT built (not a dependency, not optional, not behind a feature) - `mcp` feature pulls in rmcp with streamable HTTP transport features only - `transport-child-process` is not a dependency 8. **Feature gate isolation**: - `from_mcp`/`to_mcp` compile only with `mcp` feature - `cargo check -p alknet-http` (no `mcp`) succeeds — MCP code not compiled - `cargo check -p alknet-http --features mcp` succeeds 9. **Shared dispatch spine** (research §5.1): - `to_mcp` `call` uses `GatewayDispatch::invoke` (same spine as `to_openapi`) - `ResponseEnvelope` → `CallToolResult` mapping is `to_mcp`-specific - No `GatewayDispatch` trait (concrete struct) - Identity resolution, context construction, invoke are shared (security axis) - Wire framing, discovery, streaming are per-gateway (presentation axis) 10. **Error fidelity** (ADR-023): - `from_mcp` error_schemas from MCP tool error descriptions - `to_mcp` `call` error → `CallToolResult::structured_error` with typed `details` - No collision with protocol-level codes 11. **Security constraints**: - No-env-vars invariant (from_mcp reads context.capabilities, ADR-014) - MCP clients are not peers (no PeerId, ADR-034 §4) - AccessControl gates search results and call dispatch - No secret material in CallToolResult 12. **Test coverage**: - Unit tests for from_mcp (import, outputSchema present/absent, isError) - Unit tests for to_mcp (tools/list returns 4, search/schema/call/batch) - Integration test: MCP client → search → schema → call round-trip - Integration test: Bearer auth gates to_mcp service - Integration test: no-env-vars (no std::env::var in from_mcp) - Feature gate: cargo check without mcp succeeds ## Acceptance Criteria - [ ] from_mcp matches http-mcp.md (OperationAdapter, tools/list, outputSchema handling) - [ ] from_mcp output handling: structuredContent-preferred, ContentBlock fallback, no JSON.parse - [ ] from_mcp no-env-vars: reads context.capabilities, never std::env::var - [ ] to_mcp matches ADR-041 (4-tool gateway, not one tool per operation) - [ ] to_mcp Subscription excluded from search results - [ ] to_mcp uses GatewayDispatch::invoke (shared spine) - [ ] to_mcp CallToolResult mapping correct (structured/structured_error) - [ ] to_mcp auth: shared middleware, Identity survives rmcp framing - [ ] Streamable HTTP only (ADR-037 — stdio NOT built) - [ ] Feature gate isolation (mcp code not compiled without mcp feature) - [ ] No GatewayDispatch trait (concrete struct, research recommendation) - [ ] Error fidelity (ADR-023 — no collision with protocol codes) - [ ] MCP clients are not peers (no PeerId, ADR-034 §4) - [ ] AccessControl gates search and call - [ ] No secret material in CallToolResult - [ ] Test coverage adequate for all MCP functionality - [ ] `cargo fmt --check -p alknet-http --features mcp` passes - [ ] `cargo clippy -p alknet-http --features mcp` passes with no warnings - [ ] `cargo test -p alknet-http --features mcp` passes - [ ] `cargo check -p alknet-http` (no mcp) succeeds ## References - docs/architecture/crates/http/http-mcp.md — full MCP spec - docs/research/alknet-http-gateway-factoring/findings.md — §4 (rmcp constraints), §4.4 (auth sharing) - docs/architecture/decisions/037-mcp-stdio-transport-exclusion.md — ADR-037 - docs/architecture/decisions/041-mcp-tool-gateway-pattern.md — ADR-041 - docs/architecture/decisions/017-call-protocol-client-and-adapter-contract.md — ADR-017 §5 - docs/architecture/decisions/023-operation-error-schemas.md — ADR-023 - docs/architecture/decisions/014-secret-material-flow-and-capability-injection.md — ADR-014 - docs/architecture/decisions/034-outgoing-only-x509-and-three-peer-roles.md — ADR-034 §4 - /workspace/rust-sdk/ — rmcp SDK (ServerHandler, StreamableHttpService, CallToolResult) ## Notes > The MCP adapters are feature-gated and the most SDK-coupled part of > the crate (rmcp integration). The review should verify: (1) streamable > HTTP only (no stdio, ADR-037), (2) the 4-tool gateway pattern (ADR-041, > not one tool per operation), (3) the output handling > (structuredContent-preferred, ContentBlock fallback, no JSON.parse), > (4) the shared dispatch spine is used for to_mcp's call (same spine as > to_openapi), (5) the auth middleware is shared (Identity survives rmcp > framing — research §6 #2), (6) the no-env-vars invariant holds, (7) > feature gate isolation (MCP code not compiled without mcp). If > deviations are found, document and fix before considering the MCP > adapters complete. ## Summary > MCP adapters reviewed against all 12 checklist items. All conformance criteria > pass: from_mcp (OperationAdapter, tools/list, structuredContent-preferred output > handling, no JSON.parse, isError→CallError, no-env-vars ADR-014), to_mcp (4-tool > gateway ADR-041, ServerHandler, Subscription excluded, GatewayDispatch::invoke > shared spine, CallToolResult mapping, Identity survives rmcp framing research §6 > #2, no PeerId ADR-034 §4), streamable HTTP only (ADR-037), feature gate isolation, > GatewayDispatch concrete struct (not trait), error fidelity (ADR-023), test coverage > (223 mcp tests + 5 integration tests). One formatting fix applied to from_mcp/mod.rs. > cargo fmt --check, clippy --features mcp --all-targets, test --features mcp, and > cargo check (no mcp) all pass.