docs: MCP gateway integration design by maralbahari · Pull Request #82 · vllm-project/agentic-api

maralbahari · 2026-07-02T05:20:33Z

Summary

Adds docs/design/mcp-gateway-integration.md outlining the design for integrating MCP into the agentic-api gateway. Covers McpClient, McpClientPool (HTTP and stdio transports), McpHandler/McpHandlerKind, ToolRegistry dispatch, ResponseAccumulator extension for tool dispatch, and the execute_with_mcp() agentic loop. Includes a roadmap for stdio server support and a two-PR split for infrastructure vs execution loop.

Test Plan

Design doc only, no code changes.

Signed-off-by: maral <maralbahari.98@gmail.com>

franciscojavierarceo

this lgtm as a first cut, let's iterate and see

ashwing · 2026-07-02T22:26:45Z

+## Implementing MCP and the First Built-in Tool
+
+### Step 1: `McpClient` (`mcp/client.rs`)
+


This embeds handler: Option<Arc<dyn GatewayExecutor>> directly in ToolEntry and adds ToolRegistry::dispatch(). PR #83 (currently open) goes the other direction: a separate HashMap<ToolType, Arc<dyn GatewayExecutor>> built at the call site and passed into dispatch_tools. Both are landing simultaneously and they're incompatible — one puts executor ownership in the registry, the other keeps it external. Before either the MCP loop PR or any downstream work can proceed cleanly, we need to pick one model and have the other converge to it. What's the intended resolution here?

ashwing · 2026-07-02T22:26:52Z

+```rust
+pub struct McpClient {
+    inner: Arc<rmcp::service::RunningService<RoleClient>>,
+    tool_timeout: Duration,


Adding handler to ToolEntry means every existing construction site in ToolRegistry::build() — for Function, WebSearch, FileSearch, CodeInterpreter — needs handler: None. Not a design concern, just a practical heads-up that this touches more of the codebase than the doc implies.

ashwing · 2026-07-02T22:27:01Z

+
+impl McpClientPool {
+    pub async fn from_params(params: &[McpToolParam]) -> Self
+    pub async fn from_config(servers: HashMap<String, McpServerEntry>) -> Self


build_mcp_registry() constructs an McpHandler for ReadResource using McpClient::placeholder() because that path routes through the pool and never uses the client directly. If placeholder() is ever called on a method that isn't expected to be a no-op, it silently misbehaves rather than failing. Could the client field on McpHandler be Option<Arc<McpClient>> for variants that don't need it, so the absence is explicit in the type rather than hidden behind a sentinel?

ashwing · 2026-07-02T22:27:08Z

+                }
+            }
+        })
+    }


The ordering guarantee needs to be explicit: if the LLM emits parallel tool calls in one turn, tool B blocks on tool A completing because dispatch is sequential. If that's intentional for this phase, document it explicitly so it doesn't get treated as a bug when someone later assumes concurrent dispatch.

ashwing · 2026-07-02T22:27:16Z

+```
+
+`McpClientPool` gains a config-based constructor for gateway-managed servers:
+


When rmcp is added to Cargo.toml, pin the version and flag that modelcontextprotocol/rust-sdk is pre-1.0 — API stability isn't guaranteed across minor bumps, and reviewers should evaluate that risk consciously at add time.

ashwing

The dispatch model in this doc conflicts with what PR #83 just introduced, and that's the main thread to pull before this moves forward. The other points are smaller — construction site impact, a fragile placeholder pattern, and the new rmcp dependency — but they're all downstream of resolving which dispatch interface wins.

Extract the gateway tool loop's per-round decision into a LoopDecision enum + classify_round() classifier (executor/gateway.rs) and drive run_until_gateway_tools_complete by matching on it, replacing the inline if/if/else branches. Reworked onto the merged per-call ToolRegistry::dispatch() (vllm-project#82/vllm-project#85) rather than a parallel dispatch surface, and consolidating vllm-project#84's inline client-action path onto LoopDecision::RequiresClientAction. Behavior: - Exhausting the round budget while the model still requests tools now returns status "incomplete" + incomplete_details.reason instead of Err, preserving accumulated output and recording the final round's gateway calls with their outputs so a continuation is never fed a dangling tool call. - Per-call timeout (GATEWAY_TOOL_TIMEOUT 60s; Duration::ZERO opts out): a hung tool becomes an error output fed back to the model, not a whole-request failure. - Replaced the hard >8 calls/round cap with a .buffered(5) sliding concurrency window that drains arbitrary fan-out without failing. - A gateway-owned tool declared with no registered handler now yields an error output too, making the "never a whole-request failure" contract total. Signed-off-by: Ashwin Giridharan <girida@amazon.com>

The framework shipped across vllm-project#80/vllm-project#82/vllm-project#84/vllm-project#85/vllm-project#91 (with vllm-project#83/vllm-project#89 in review) and diverged from the original proposal in several ways. Update the doc to the design-of-record: - Status: Proposal -> Accepted; add as-built preamble. - ToolHandler split into ToolHandler + GatewayExecutor (execute() only applies to gateway-owned types); Pin<Box<dyn Future>> not #[async_trait]; no discover()/event_prefix()/output_item_type() on the trait. - ToolType gains CodexNamespace (6 variants); ResponsesTool shown as the shipped #[non_exhaustive] 7-variant enum incl. namespace + Unknown catch-all and the web_search aliases. - ToolEntry carries handler: Option<Arc<dyn GatewayExecutor>>; dispatch is two layers (per-call registry.dispatch + multi-turn classify_round/ LoopDecision) with a diagram. - LoopDecision trimmed to 4 unit variants; mixed turns resolved by classify_round precedence, not ContinuePartial. - Add Implementation Status (PR mapping) and Future Work (layering ADR, GatewayAccumulator, per-type config, remaining handlers). - Mark Open Questions Q1-Q4 resolved; clarify collision handling is a hard error only for namespace-vs-top-level, warn-level last-write-wins otherwise. Rationale sections (Principles, Alternatives Considered) preserved. Signed-off-by: Ashwin Giridharan <girida@amazon.com>

## Summary Extracts the gateway tool loop's per-round decision into a `LoopDecision` enum + `classify_round()` classifier (in `executor/gateway.rs`), drives `run_until_gateway_tools_complete` by matching on it, and hardens the tool-execution path with a per-call timeout and bounded-concurrency fan-out. Reworked onto what's now on `main`: rather than a parallel dispatch mechanism, it **builds on the merged per-call `ToolRegistry::dispatch()` (#82/#85)** and layers multi-turn orchestration on top — the unification the MCP design doc (#82) calls for ("unified into `execute_loop()`"). Rebased onto the merged codex integration (#84) so its inline client-action path converges onto a single `LoopDecision::RequiresClientAction`. ## What changed **Loop control — `executor/gateway.rs` + `engine.rs`** - `LoopDecision` — `#[non_exhaustive]`: `Continue` / `Done` / `RequiresClientAction` / `Incomplete(String)` — and `classify_round(has_client_owned, gateway_results, round, max_rounds)`, one place that decides whether a turn loops, stops, hands back to the client, or exhausts the budget. Client-owned calls take precedence; the round cap only fires when gateway work remains. - `run_until_gateway_tools_complete` matches `classify_round(...)` instead of the previous inline `if/if/else` branches. Per-call dispatch stays on `registry.dispatch()`; SSE lifecycle emission is unchanged. - **Behavior change:** exhausting the round budget while the model still requests tools now returns `status: "incomplete"` + `incomplete_details.reason` instead of `Err`, matching the Responses API and preserving accumulated tool output. The final round's gateway calls and their outputs are recorded, so a continuation is never fed a dangling tool call. - `RequiresClientAction` covers both plain `function` and Codex `namespace` calls (`is_gateway_owned() == false`). In a **mixed turn** — a single response emitting both a gateway `web_search` and a client-owned Codex call — the gateway call is executed this turn and the client call handed back in the same `RequiresClientAction` round (no extra inference round-trip; the `web_search` result comes back already resolved). **Tool-execution hardening — `executor/gateway.rs`** - **Per-call timeout** (`GATEWAY_TOOL_TIMEOUT`, 60s; `Duration::ZERO` opts out). A tool exceeding the budget becomes an error output fed back to the model — one hung provider cannot stall the round. - **Removed the hard `>8 calls/round` cap** that returned an error. The `.buffered(MAX_CONCURRENT_GATEWAY_CALLS = 5)` sliding window already bounds outbound fan-out (admits the next call as one finishes); arbitrary fan-out drains without failing the request. Call count is bounded upstream by model output size. - A gateway-owned tool declared with **no registered handler** (server built without that executor) now yields an error output too, rather than failing the whole request — making the "never a whole-request failure" contract total. ## Test Plan - [x] `cargo test --workspace` — 325 pass, 0 fail (1 pre-existing vLLM cassette ignored) - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean - [x] `cargo fmt --check` — clean **Coverage:** - `gateway.rs` unit tests — all four `LoopDecision` arms + the final-round-no-work-is-`Done` edge; a per-call-timeout test (1ms budget vs a 50ms tool → error output fed back); a missing-handler test (gateway-owned tool with no executor → error output). - `tests/dispatch_loop_cassette_test.rs` (new) — 3 integration tests driving recorded OpenAI Responses wire bodies through the loop: single client-owned call terminates in one round, parallel calls preserved, mixed gateway + client-owned. - `tests/web_search_tool_test.rs` — round cap asserts the `status: "incomplete"` contract (blocking **and** streaming); a continuation test proves the persisted incomplete turn pairs every gateway call with its output (no dangling call); a 9-call fan-out test proves all execute despite the concurrency window of 5; a mixed-turn test covers `web_search` (gateway) + a Codex namespace call (client) resolving in one `RequiresClientAction` round, with the namespace call restored to `{namespace, name}` and the continuation carrying the persisted `web_search` output. ## Notes **Dispatch model:** no separate dispatch surface — uses the merged `ToolRegistry::dispatch()` for per-call resolution and owns only the multi-turn loop. `LoopDecision` is `pub(super)`, sufficient for #84's consolidated path and PR C (both under `executor`). **Follow-ups (non-blocking, out of scope here):** the per-call timeout and concurrency window are file-private consts — promoting them to per-tool-type config (as varied gateway providers land via #82) is an additive change on the existing `execute_gateway_call_with_timeout(timeout)` seam. There is no outer request-level deadline on the executor HTTP client yet; the per-call timeout bounds a single call, not total request time. AI assistance was used in drafting this PR. --------- Signed-off-by: Ashwin Giridharan <girida@amazon.com> Co-authored-by: Francisco Javier Arceo <arceofrancisco@gmail.com>

mcp design document

c6d447f

Signed-off-by: maral <maralbahari.98@gmail.com>

maralbahari requested review from bbrowning, franciscojavierarceo, jiahuei, leseb, noobHappylife, qandrew and tjtanaa as code owners July 2, 2026 05:20

franciscojavierarceo approved these changes Jul 2, 2026

View reviewed changes

ashwing reviewed Jul 2, 2026

View reviewed changes

ashwing mentioned this pull request Jul 2, 2026

refactor: drive gateway tool loop with LoopDecision (PR B) #83

Merged

3 tasks

maralbahari merged commit ca76f25 into vllm-project:main Jul 6, 2026
3 checks passed

ashwing mentioned this pull request Jul 7, 2026

feat: benchmark proxy payload scaling #88

Merged

ashwing mentioned this pull request Jul 10, 2026

docs: tool framework design — multi-type tool support #67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs: MCP gateway integration design#82

docs: MCP gateway integration design#82
maralbahari merged 1 commit into
vllm-project:mainfrom
EmbeddedLLM:mcp-design

maralbahari commented Jul 2, 2026

Uh oh!

franciscojavierarceo left a comment

Uh oh!

ashwing Jul 2, 2026

Uh oh!

ashwing Jul 2, 2026

Uh oh!

ashwing Jul 2, 2026

Uh oh!

ashwing Jul 2, 2026

Uh oh!

ashwing Jul 2, 2026

Uh oh!

ashwing left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		## Implementing MCP and the First Built-in Tool

		### Step 1: `McpClient` (`mcp/client.rs`)

		```

		`McpClientPool` gains a config-based constructor for gateway-managed servers:

Uh oh!

Conversation

maralbahari commented Jul 2, 2026

Summary

Test Plan

Uh oh!

franciscojavierarceo left a comment

Choose a reason for hiding this comment

Uh oh!

ashwing Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ashwing Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ashwing Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ashwing Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ashwing Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

ashwing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants