[RFC/draft] MCP auto-exposure prototype (route harvesting + replay bridge) - #2053
Closed
adil-a wants to merge 3 commits into
Closed
[RFC/draft] MCP auto-exposure prototype (route harvesting + replay bridge)#2053adil-a wants to merge 3 commits into
adil-a wants to merge 3 commits into
Conversation
Self-contained proof-of-concept (prototypes/mcp_auto_exposure/, wired into nothing) for an alternative to PR #2002's @gym_tool dual registration: auto-expose every existing FastAPI tool route as an MCP tool with zero decorator changes and byte-identical handlers, by replaying each MCP tools/call as an internal in-process HTTP request through the app's own ASGI stack (token -> minted session cookie -> full-stack replay -> MCP result mapping). MCP-side engine is the official SDK's public low-level Server (no private attrs). 37/37 live acceptance checks pass against pristine origin/main handler code imported directly from resources_servers/ (finance = typed routes, workplace = 27-tool dispatcher via the mcp_tool_inventory override). Trade-offs (2x replay pass, plumbing-route exposure hazard, body_field coupling) documented in the README for design discussion. Signed-off-by: Codex <adasif@nvidia.com> Signed-off-by: Codex <codex@openai.com>
Replaces the always-replay path with the middle-ground design the middle-ground workflow validated: run each frozen origin/main handler directly ONCE (fabricated Request whose .session is served by a public Request subclass), no second app pass. A startup detector (hybrid_dispatch: server-level middleware audit + route-level signature bind) falls a route or the whole server back to the in-process replay bridge wherever it cannot prove direct == a real HTTP request (custom middleware, unsupported handler shapes). All 32 in-tree tools (finance 5 + workplace 27) dispatch DIRECT, zero replay; adding a custom middleware flips that server to replay automatically (verified). 37/37 acceptance checks pass against pristine origin/main handler code. Uses only public FastAPI/Starlette/pydantic surface for dispatch (no solve_dependencies, body_field, or dependant). If adopted, this synthesizes the equivalent of #2002's @gym_tool registrations from the existing routes, so the ~861-line migration across 16 tool files could revert. Signed-off-by: Codex <adasif@nvidia.com> Signed-off-by: Codex <codex@openai.com>
A live detector sweep across all 97 resources servers (71 built in-process, 45 tool routes) found 13 routes classifying REPLAY: newton_bench's 12 /run_experiment_* (the factory rewrites __signature__ with the real per-module body model but not __annotations__, and bind_route read __annotations__ via get_type_hints) and openenv's non-MCP /step (body: dict, which FastAPI passes through unvalidated). Fix bind_route's annotation resolution to match FastAPI's own: the inspect.signature annotation (which honors __signature__) wins; deferred string annotations fall back to get_type_hints; unresolvable strings refuse loudly. Add the dict-body pass-through shape. Re-verified: full 97-server sweep now 45/45 DIRECT (0 replay, no other verdict changed); behavioral parity door-vs-direct on both fixed servers (newton m0_gravity: 422 names the real model fields mass1/mass2/distance, proving the correct model is bound; openenv /step: byte-identical bodies); original acceptance suite 37/37. The replay fallback stays: this sweep is exactly how 13 quiet exceptions were found once already. Signed-off-by: Codex <adasif@nvidia.com> Signed-off-by: Codex <codex@openai.com>
bxyu-nvidia
pushed a commit
that referenced
this pull request
Jul 22, 2026
> **Draft / RFC.** Consolidates the exploration in #2002 and #2053 into one tracked module + a small framework hook, **removes #1682's decorator API** (`@gym_tool` / `MCPResourcesServer`) so there is exactly one user-facing MCP mechanism, then hardened by a full review pass (details below). Branched off `main`. ## What this is Serve a resources server's **existing** FastAPI tool routes over MCP with **zero handler changes** and **one opt-in config field** — no code change at all for a typed-route server: ```yaml resources_servers: my_server: entrypoint: app.py expose_tools_over_mcp: true # the only addition — set per instance in the config ``` `run_webserver` installs the `/mcp` endpoint automatically after the app is built (and imports the MCP SDK only for servers that opt in). Handlers keep their `request: Request` param and `request.session[...]` reads exactly as written. The only in-tree code a server ever needs is for shapes the config can't express, and it is one override: `mcp_tools(harvested, catchall)` — return the harvested typed routes (the default), filter one out to exclude a harness-only route (e.g. `/end_session`), or append catch-all-backed tools (`harvested + [catchall.tool(name, input_schema, description)]`) for dispatcher servers. A dropped route is never advertised, never callable over MCP, never shape-checked, and unchanged over HTTP. This PR also **removes the decorator-based MCP API** that #1682 added (`@gym_tool`, `MCPResourcesServer`, `MCPSessionError`, the token contextvar and header middleware): tools no longer need decorators or MCP-specific signatures, so the old mechanism is superseded rather than parallel. The wire contract survives (`MCPServerMetadata`, the `X-NeMo-Gym-Session-Token` header, the token salt, the reserved-name set) — `claude_code_agent` on `main` consumes it unchanged. `example_mcp_weather` is rewritten onto auto-exposure: a plain typed `POST /get_weather` route plus the yaml flag, with tests driving the MCP door (and the wrapped `/seed_session` + `/verify`) through the real engine mount, and the plain-HTTP tool route on the stock app. ## How to review this (~30 min reading order) 1. `nemo_gym/mcp_auto_exposure.py` module docstring — the contract in one screen. 2. `bind_route` — the detector. The accept/refuse table below is what it enforces. 3. `call_direct` — dispatch parity with the plain HTTP route (threadpool for sync handlers, body bytes, session cookie, `response_model` filtering, error-text parity). 4. `harvest_tools` → `install_auto_exposure` — tool map, startup guards, session token, `/seed_session` wrap, `/mcp` mount. 5. `base_resources_server.py` diff — the `expose_tools_over_mcp` config field, the two overridables (`mcp_tools(harvested, catchall)` — what to expose; `mcp_allowed_tools_for_session(seed_body)` — per-rollout narrowing), and the removal of the #1682 decorator API (the file drops from 315 lines on `main` to 150; it had grown to 356 on this branch before the removal commit). 6. `server_utils.py` — the 6-line activation point (plus a 4-line session-middleware idempotency guard). 7. `tests/unit_tests/test_mcp_auto_exposure.py` — 45 tests double as the spec; each guard and parity shape has a named test. ## What the detector accepts vs refuses **Accepts (dispatches identically to the plain HTTP route):** async and sync handlers (sync runs via `run_in_threadpool`, exactly like FastAPI); `body: Model`; `body: dict` / `dict[str, Any]`; raw-body catch-alls reading `await request.json()`; handlers taking both a body model *and* a `request` they read raw; `str` path params; `response_model=` filtering (decorator kwarg or return annotation). **Refuses loudly at startup (`ValueError` naming the route and reason):** non-Gym middleware; `Depends`/`Security`; `*args/**kwargs`; multiple body models; union/optional body params; defaulted query params; unresolvable annotations; tool names outside `^[A-Za-z0-9_-]+$` (e.g. nested routes); tool names (from `mcp_tools()`) colliding with `verify`/`seed_session`/`aggregate_metrics`/`mcp`; duplicate tool names; multiple parameterized catch-all routes (auto-exposure cannot tell which backs the tools); a server that already serves `/mcp` (a hand-rolled MCP mount would be shadowed); a missing `/seed_session` or `/verify`. These rules apply to route-handler signatures only — inner tool functions may use `**kwargs` freely, and no in-tree server trips any refusal. No silent fallback exists: a shape either dispatches provably like HTTP or the server does not start. One soft case: a dispatcher whose `mcp_tools()` override ignores its catch-all logs a warning (the catch-all-backed tools stay HTTP-only). ## Session & token `/seed_session` gains an additive `"mcp"` key (`MCPServerMetadata` — the same shape `claude_code_agent` on `main` already consumes): the `/mcp` URL plus a signed per-rollout token in `X-NeMo-Gym-Session-Token`. The token payload is always `{"sid", "tools"}` — the session id plus this rollout's allow-list from `mcp_allowed_tools_for_session(seed_body)` (`null` = unrestricted) — signed with `URLSafeSerializer` (untimed; expiry was cut as consumer-less scope). The header/salt/metadata constants live in `base_resources_server` — the wire contract retained from #1682's scheme after its decorator API was removed. The token is verified on every call (no cache). `tools/list` works tokenless and advertises the full exposed set; a token carrying an allow-list narrows both `tools/list` and `tools/call` for that session. Known limitation, deliberately out of scope: the signing secret derivation (`class___config-name`) is pre-existing `main` behavior (the session-middleware secret) — hardening it is a separate change. ## Verify-time tool-name normalization (install-time, gated) MCP-native agents record trajectory tool calls namespaced (`mcp__<server>__<tool>`); verifiers know bare names — found live when MCP rollouts scored 0.0 on perfect trajectories. For **flag-on servers only**, installation wraps the `/verify` route's current endpoint (whatever handler it holds — servers that re-register `/verify` are covered by construction): names are normalized for scoring on a deep copy, then the model's original names are restored in the echoed response (matched by `call_id`). Flag-off servers — every existing benchmark — keep verify byte-for-byte, so no baselines move; persisted rollout artifacts always keep the names the model actually emitted. ## Diff shape (+2,136 / −631, 11 files) | Area | Change | |---|---| | `nemo_gym/mcp_auto_exposure.py` | **new, one file, 841 lines** — detector, direct dispatcher, harvest + guards, token, `/seed_session` + `/verify` wraps, `/mcp` mount | | `nemo_gym/base_resources_server.py` | +48/−207: adds the `expose_tools_over_mcp` config field, the `normalize_tool_name` helper, and the `mcp_tools()` / `mcp_allowed_tools_for_session()` overridables; **removes** `@gym_tool`, `MCPResourcesServer`, `MCPSessionError`, and the header middleware (their unit tests go too: `test_base_resources_server.py` −277) | | `nemo_gym/server_utils.py` | +10: `run_webserver` activates auto-exposure for opted-in servers (lazy MCP import); `setup_session_middleware` is now idempotent | | `tests/unit_tests/test_mcp_auto_exposure.py` | **new — 45 self-contained tests** (TestClient, synthetic servers, ~3 s); +37 in `test_server_utils.py`; docs section in `fern/.../mcp-resources-server.mdx` | | `resources_servers/` | `example_mcp_weather` rewritten onto auto-exposure (plain typed route + yaml flag; 6 tests: the MCP door — plus the wrapped `/seed_session` and `/verify` — through the real engine mount, and the plain-HTTP tool route on the stock app). **Every other server: byte-identical to `main`.** Enablement stays per-server follow-up: pure config for typed-route servers; a small `mcp_tools()` override for catch-all dispatchers. | MCP engine: the official SDK's **public low-level** `Server` — no private-attr access. ## Review provenance This branch already went through a 7-reviewer + devil's-advocate review pass (59 confirmed findings) followed by a fix pass with adversarial re-verification: all 55 code findings are fixed here with tests (silently-wrong detector shapes, refuse-loudly gaps, an unbounded token cache — the token is now verified on every call with no cache, and expiry was later cut as consumer-less scope — event-loop blocking for sync handlers, provenance-preserving verify normalization), and a docstring-accuracy audit (77 reviewed, 3 corrected). The remaining follow-ups are listed under "For discussion". ## Verified - **Reproducible from this PR:** `pytest tests/unit_tests/test_mcp_auto_exposure.py` → **45/45** (~3 s; covers every refusal, parity shape, token path, exclusion semantics, and the normalization gating/provenance — including a re-registered-`/verify` regression test). - **Live e2e at this HEAD (`3891dd65`):** `workplace_assistant` MCP door (`claude_code_agent`) — **5/5 `reward: 1.0`**, all 8 distinct tools namespaced; HTTP door (`simple_agent`) — **5/5** on the second run, 4/5 on the first (the failing rollout omitted the `status: "Lead"` search filter — a trajectory-verified policy flake, not an engine failure; the HTTP door runs the stock route code). `example_mcp_weather` (`claude_code_agent`) — **5/5 `reward: 1.0`** at the removal commit `4d6d569c`; the one commit since is docs plus a comment reword. Earlier demo (below): both doors 5/5 tasks `reward: 1.0`, ~30 s wall for 5 concurrent rollouts, MCP-door trajectories persist the namespaced names the model emitted. n=5 single runs of a stochastic policy — a smoke demonstration that both transports work end to end, not a statistical parity claim. - **Development-time only (local, gitignored — not reproducible from this PR):** a 44-check live acceptance harness exercising six representative in-tree servers (finance, workplace, aviary, newton_bench, openenv, ns_tools) against pristine `origin/main` handlers, covering the factory-`__signature__`, `dict`-body, and raw-body/`PlainTextResponse` shapes; 44/44 as of 2026-07-17 against a pre-refactor revision of the engine (development-time evidence; the harness predates the `mcp_tools()` API). Other in-tree servers were covered by a static route sweep, not this live harness. ## Runnable e2e — both doors through `gym eval run` The full 5-task `workplace_assistant` example dataset, driven end to end by Gym. Because `resources_servers/` ships unchanged, the demo uses a small user-side entrypoint (`app_mcp.py`, shown in the docs added by this PR) that subclasses the stock server and overrides `mcp_tools()` to append the 27 catch-all-backed tools; the flag itself is plain yaml: Compose config (`workplace_2059.yaml`) — MCP door via `claude_code_agent` (already on `main`; it reads the `mcp` key `/seed_session` returns and wires the Claude Code binary itself), HTTP door via `simple_agent`: ```yaml workplace_assistant: resources_servers: workplace_assistant: entrypoint: app_mcp.py # thin subclass overriding mcp_tools(); see the docs page domain: agent expose_tools_over_mcp: true workplace_claude: responses_api_agents: claude_code_agent: entrypoint: app.py resources_server: {type: resources_servers, name: workplace_assistant} model: ${anthropic_model_name} anthropic_api_key: ${anthropic_api_key} # env.yaml; any Anthropic-format endpoint anthropic_base_url: ${anthropic_base_url} # optional gateway; CLI appends /v1/messages policy_model: responses_api_models: inference_provider: entrypoint: app.py base_url: ${policy_base_url} # any OpenAI-format endpoint api_key: ${policy_api_key} model: ${policy_model_name} workplace_simple: responses_api_agents: simple_agent: entrypoint: app.py max_steps: 12 resources_server: {type: resources_servers, name: workplace_assistant} model_server: {type: responses_api_models, name: policy_model} ``` ```bash gym env start --config workplace_2059.yaml # (a) MCP door — Claude Code discovers and calls the tools over /mcp with the per-rollout token gym eval run --no-serve --agent workplace_claude \ --input resources_servers/workplace_assistant/data/example.jsonl \ --output results/workplace_claude_rollouts.jsonl # (b) HTTP door — the same tools as plain POST /<tool_name> gym eval run --no-serve --agent workplace_simple \ --input resources_servers/workplace_assistant/data/example.jsonl \ --output results/workplace_simple_rollouts.jsonl ``` Tip: if your provider's OpenAI *Responses-API* emulation is incomplete (a common gateway gap on multi-turn tool exchanges), use `inference_provider` (chat-completions + Gym's converter) rather than `openai_model`. ## Enablement (deliberately none in this PR) This PR ships the engine plus the one in-tree example (`example_mcp_weather`) — every other server under `resources_servers/` is byte-identical to `main` (we cannot properly test each env here, per review). Enabling a server is per-server follow-up work: set `expose_tools_over_mcp: true` in its instance config (typed-route servers need no code at all), plus a small `mcp_tools()` override for catch-all dispatcher servers — the docs added in this PR walk through workplace_assistant end to end. During development, a fleet exercise ran `claude_code_agent` over `/mcp` against 12 servers enabled this way (workplace_assistant 27 tools, math_advanced_calculations, finance_sec_search, indirect_prompt_injection with per-task narrowing, the examples, and others), all dispatching correctly; those results are development-time evidence, not shipped code. **Deliberately not exposed even as follow-up (each needs design work first):** aviary (its `/step`/`/close` bodies carry an `env_id` that can address other rollouts' environments — needs session-derived lookup), genrm_compare (its only route is a harness-side batch API), and the gymnasium family (reward accrues from `/reset`/`/step`, which a model could drive to reroll its own episode). The 74 verify-only benchmarks have no tool routes to expose. ## For discussion - Flag is **opt-in (default off)** — auto-exposing every route isn't always wanted (aviary's `/step` carries `env_id`; exposing it lets a model address other rollouts' envs). - Follow-ups deliberately not in this PR: session-binding aviary's `env_id` so it can be exposed safely; an MCP-safe reward design for the gymnasium family; and hardening the pre-existing token-signing secret derivation shared with #1682. - **#1682's decorator API is removed in this PR** rather than left as a parallel mechanism: auto-exposure reads the routes authors already wrote, so `@gym_tool` had no remaining role, and keeping both would have forced every author to choose. `example_mcp_weather` (the only in-tree consumer) is migrated here; #2002's dual-registration approach is superseded. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Codex <adasif@nvidia.com> Signed-off-by: Codex <codex@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OlegSudakov
pushed a commit
to OlegSudakov/Gym
that referenced
this pull request
Aug 7, 2026
> **Draft / RFC.** Consolidates the exploration in NVIDIA-NeMo#2002 and NVIDIA-NeMo#2053 into one tracked module + a small framework hook, **removes NVIDIA-NeMo#1682's decorator API** (`@gym_tool` / `MCPResourcesServer`) so there is exactly one user-facing MCP mechanism, then hardened by a full review pass (details below). Branched off `main`. ## What this is Serve a resources server's **existing** FastAPI tool routes over MCP with **zero handler changes** and **one opt-in config field** — no code change at all for a typed-route server: ```yaml resources_servers: my_server: entrypoint: app.py expose_tools_over_mcp: true # the only addition — set per instance in the config ``` `run_webserver` installs the `/mcp` endpoint automatically after the app is built (and imports the MCP SDK only for servers that opt in). Handlers keep their `request: Request` param and `request.session[...]` reads exactly as written. The only in-tree code a server ever needs is for shapes the config can't express, and it is one override: `mcp_tools(harvested, catchall)` — return the harvested typed routes (the default), filter one out to exclude a harness-only route (e.g. `/end_session`), or append catch-all-backed tools (`harvested + [catchall.tool(name, input_schema, description)]`) for dispatcher servers. A dropped route is never advertised, never callable over MCP, never shape-checked, and unchanged over HTTP. This PR also **removes the decorator-based MCP API** that NVIDIA-NeMo#1682 added (`@gym_tool`, `MCPResourcesServer`, `MCPSessionError`, the token contextvar and header middleware): tools no longer need decorators or MCP-specific signatures, so the old mechanism is superseded rather than parallel. The wire contract survives (`MCPServerMetadata`, the `X-NeMo-Gym-Session-Token` header, the token salt, the reserved-name set) — `claude_code_agent` on `main` consumes it unchanged. `example_mcp_weather` is rewritten onto auto-exposure: a plain typed `POST /get_weather` route plus the yaml flag, with tests driving the MCP door (and the wrapped `/seed_session` + `/verify`) through the real engine mount, and the plain-HTTP tool route on the stock app. ## How to review this (~30 min reading order) 1. `nemo_gym/mcp_auto_exposure.py` module docstring — the contract in one screen. 2. `bind_route` — the detector. The accept/refuse table below is what it enforces. 3. `call_direct` — dispatch parity with the plain HTTP route (threadpool for sync handlers, body bytes, session cookie, `response_model` filtering, error-text parity). 4. `harvest_tools` → `install_auto_exposure` — tool map, startup guards, session token, `/seed_session` wrap, `/mcp` mount. 5. `base_resources_server.py` diff — the `expose_tools_over_mcp` config field, the two overridables (`mcp_tools(harvested, catchall)` — what to expose; `mcp_allowed_tools_for_session(seed_body)` — per-rollout narrowing), and the removal of the NVIDIA-NeMo#1682 decorator API (the file drops from 315 lines on `main` to 150; it had grown to 356 on this branch before the removal commit). 6. `server_utils.py` — the 6-line activation point (plus a 4-line session-middleware idempotency guard). 7. `tests/unit_tests/test_mcp_auto_exposure.py` — 45 tests double as the spec; each guard and parity shape has a named test. ## What the detector accepts vs refuses **Accepts (dispatches identically to the plain HTTP route):** async and sync handlers (sync runs via `run_in_threadpool`, exactly like FastAPI); `body: Model`; `body: dict` / `dict[str, Any]`; raw-body catch-alls reading `await request.json()`; handlers taking both a body model *and* a `request` they read raw; `str` path params; `response_model=` filtering (decorator kwarg or return annotation). **Refuses loudly at startup (`ValueError` naming the route and reason):** non-Gym middleware; `Depends`/`Security`; `*args/**kwargs`; multiple body models; union/optional body params; defaulted query params; unresolvable annotations; tool names outside `^[A-Za-z0-9_-]+$` (e.g. nested routes); tool names (from `mcp_tools()`) colliding with `verify`/`seed_session`/`aggregate_metrics`/`mcp`; duplicate tool names; multiple parameterized catch-all routes (auto-exposure cannot tell which backs the tools); a server that already serves `/mcp` (a hand-rolled MCP mount would be shadowed); a missing `/seed_session` or `/verify`. These rules apply to route-handler signatures only — inner tool functions may use `**kwargs` freely, and no in-tree server trips any refusal. No silent fallback exists: a shape either dispatches provably like HTTP or the server does not start. One soft case: a dispatcher whose `mcp_tools()` override ignores its catch-all logs a warning (the catch-all-backed tools stay HTTP-only). ## Session & token `/seed_session` gains an additive `"mcp"` key (`MCPServerMetadata` — the same shape `claude_code_agent` on `main` already consumes): the `/mcp` URL plus a signed per-rollout token in `X-NeMo-Gym-Session-Token`. The token payload is always `{"sid", "tools"}` — the session id plus this rollout's allow-list from `mcp_allowed_tools_for_session(seed_body)` (`null` = unrestricted) — signed with `URLSafeSerializer` (untimed; expiry was cut as consumer-less scope). The header/salt/metadata constants live in `base_resources_server` — the wire contract retained from NVIDIA-NeMo#1682's scheme after its decorator API was removed. The token is verified on every call (no cache). `tools/list` works tokenless and advertises the full exposed set; a token carrying an allow-list narrows both `tools/list` and `tools/call` for that session. Known limitation, deliberately out of scope: the signing secret derivation (`class___config-name`) is pre-existing `main` behavior (the session-middleware secret) — hardening it is a separate change. ## Verify-time tool-name normalization (install-time, gated) MCP-native agents record trajectory tool calls namespaced (`mcp__<server>__<tool>`); verifiers know bare names — found live when MCP rollouts scored 0.0 on perfect trajectories. For **flag-on servers only**, installation wraps the `/verify` route's current endpoint (whatever handler it holds — servers that re-register `/verify` are covered by construction): names are normalized for scoring on a deep copy, then the model's original names are restored in the echoed response (matched by `call_id`). Flag-off servers — every existing benchmark — keep verify byte-for-byte, so no baselines move; persisted rollout artifacts always keep the names the model actually emitted. ## Diff shape (+2,136 / −631, 11 files) | Area | Change | |---|---| | `nemo_gym/mcp_auto_exposure.py` | **new, one file, 841 lines** — detector, direct dispatcher, harvest + guards, token, `/seed_session` + `/verify` wraps, `/mcp` mount | | `nemo_gym/base_resources_server.py` | +48/−207: adds the `expose_tools_over_mcp` config field, the `normalize_tool_name` helper, and the `mcp_tools()` / `mcp_allowed_tools_for_session()` overridables; **removes** `@gym_tool`, `MCPResourcesServer`, `MCPSessionError`, and the header middleware (their unit tests go too: `test_base_resources_server.py` −277) | | `nemo_gym/server_utils.py` | +10: `run_webserver` activates auto-exposure for opted-in servers (lazy MCP import); `setup_session_middleware` is now idempotent | | `tests/unit_tests/test_mcp_auto_exposure.py` | **new — 45 self-contained tests** (TestClient, synthetic servers, ~3 s); +37 in `test_server_utils.py`; docs section in `fern/.../mcp-resources-server.mdx` | | `resources_servers/` | `example_mcp_weather` rewritten onto auto-exposure (plain typed route + yaml flag; 6 tests: the MCP door — plus the wrapped `/seed_session` and `/verify` — through the real engine mount, and the plain-HTTP tool route on the stock app). **Every other server: byte-identical to `main`.** Enablement stays per-server follow-up: pure config for typed-route servers; a small `mcp_tools()` override for catch-all dispatchers. | MCP engine: the official SDK's **public low-level** `Server` — no private-attr access. ## Review provenance This branch already went through a 7-reviewer + devil's-advocate review pass (59 confirmed findings) followed by a fix pass with adversarial re-verification: all 55 code findings are fixed here with tests (silently-wrong detector shapes, refuse-loudly gaps, an unbounded token cache — the token is now verified on every call with no cache, and expiry was later cut as consumer-less scope — event-loop blocking for sync handlers, provenance-preserving verify normalization), and a docstring-accuracy audit (77 reviewed, 3 corrected). The remaining follow-ups are listed under "For discussion". ## Verified - **Reproducible from this PR:** `pytest tests/unit_tests/test_mcp_auto_exposure.py` → **45/45** (~3 s; covers every refusal, parity shape, token path, exclusion semantics, and the normalization gating/provenance — including a re-registered-`/verify` regression test). - **Live e2e at this HEAD (`3891dd65`):** `workplace_assistant` MCP door (`claude_code_agent`) — **5/5 `reward: 1.0`**, all 8 distinct tools namespaced; HTTP door (`simple_agent`) — **5/5** on the second run, 4/5 on the first (the failing rollout omitted the `status: "Lead"` search filter — a trajectory-verified policy flake, not an engine failure; the HTTP door runs the stock route code). `example_mcp_weather` (`claude_code_agent`) — **5/5 `reward: 1.0`** at the removal commit `4d6d569c`; the one commit since is docs plus a comment reword. Earlier demo (below): both doors 5/5 tasks `reward: 1.0`, ~30 s wall for 5 concurrent rollouts, MCP-door trajectories persist the namespaced names the model emitted. n=5 single runs of a stochastic policy — a smoke demonstration that both transports work end to end, not a statistical parity claim. - **Development-time only (local, gitignored — not reproducible from this PR):** a 44-check live acceptance harness exercising six representative in-tree servers (finance, workplace, aviary, newton_bench, openenv, ns_tools) against pristine `origin/main` handlers, covering the factory-`__signature__`, `dict`-body, and raw-body/`PlainTextResponse` shapes; 44/44 as of 2026-07-17 against a pre-refactor revision of the engine (development-time evidence; the harness predates the `mcp_tools()` API). Other in-tree servers were covered by a static route sweep, not this live harness. ## Runnable e2e — both doors through `gym eval run` The full 5-task `workplace_assistant` example dataset, driven end to end by Gym. Because `resources_servers/` ships unchanged, the demo uses a small user-side entrypoint (`app_mcp.py`, shown in the docs added by this PR) that subclasses the stock server and overrides `mcp_tools()` to append the 27 catch-all-backed tools; the flag itself is plain yaml: Compose config (`workplace_2059.yaml`) — MCP door via `claude_code_agent` (already on `main`; it reads the `mcp` key `/seed_session` returns and wires the Claude Code binary itself), HTTP door via `simple_agent`: ```yaml workplace_assistant: resources_servers: workplace_assistant: entrypoint: app_mcp.py # thin subclass overriding mcp_tools(); see the docs page domain: agent expose_tools_over_mcp: true workplace_claude: responses_api_agents: claude_code_agent: entrypoint: app.py resources_server: {type: resources_servers, name: workplace_assistant} model: ${anthropic_model_name} anthropic_api_key: ${anthropic_api_key} # env.yaml; any Anthropic-format endpoint anthropic_base_url: ${anthropic_base_url} # optional gateway; CLI appends /v1/messages policy_model: responses_api_models: inference_provider: entrypoint: app.py base_url: ${policy_base_url} # any OpenAI-format endpoint api_key: ${policy_api_key} model: ${policy_model_name} workplace_simple: responses_api_agents: simple_agent: entrypoint: app.py max_steps: 12 resources_server: {type: resources_servers, name: workplace_assistant} model_server: {type: responses_api_models, name: policy_model} ``` ```bash gym env start --config workplace_2059.yaml # (a) MCP door — Claude Code discovers and calls the tools over /mcp with the per-rollout token gym eval run --no-serve --agent workplace_claude \ --input resources_servers/workplace_assistant/data/example.jsonl \ --output results/workplace_claude_rollouts.jsonl # (b) HTTP door — the same tools as plain POST /<tool_name> gym eval run --no-serve --agent workplace_simple \ --input resources_servers/workplace_assistant/data/example.jsonl \ --output results/workplace_simple_rollouts.jsonl ``` Tip: if your provider's OpenAI *Responses-API* emulation is incomplete (a common gateway gap on multi-turn tool exchanges), use `inference_provider` (chat-completions + Gym's converter) rather than `openai_model`. ## Enablement (deliberately none in this PR) This PR ships the engine plus the one in-tree example (`example_mcp_weather`) — every other server under `resources_servers/` is byte-identical to `main` (we cannot properly test each env here, per review). Enabling a server is per-server follow-up work: set `expose_tools_over_mcp: true` in its instance config (typed-route servers need no code at all), plus a small `mcp_tools()` override for catch-all dispatcher servers — the docs added in this PR walk through workplace_assistant end to end. During development, a fleet exercise ran `claude_code_agent` over `/mcp` against 12 servers enabled this way (workplace_assistant 27 tools, math_advanced_calculations, finance_sec_search, indirect_prompt_injection with per-task narrowing, the examples, and others), all dispatching correctly; those results are development-time evidence, not shipped code. **Deliberately not exposed even as follow-up (each needs design work first):** aviary (its `/step`/`/close` bodies carry an `env_id` that can address other rollouts' environments — needs session-derived lookup), genrm_compare (its only route is a harness-side batch API), and the gymnasium family (reward accrues from `/reset`/`/step`, which a model could drive to reroll its own episode). The 74 verify-only benchmarks have no tool routes to expose. ## For discussion - Flag is **opt-in (default off)** — auto-exposing every route isn't always wanted (aviary's `/step` carries `env_id`; exposing it lets a model address other rollouts' envs). - Follow-ups deliberately not in this PR: session-binding aviary's `env_id` so it can be exposed safely; an MCP-safe reward design for the gymnasium family; and hardening the pre-existing token-signing secret derivation shared with NVIDIA-NeMo#1682. - **NVIDIA-NeMo#1682's decorator API is removed in this PR** rather than left as a parallel mechanism: auto-exposure reads the routes authors already wrote, so `@gym_tool` had no remaining role, and keeping both would have forced every author to choose. `example_mcp_weather` (the only in-tree consumer) is migrated here; NVIDIA-NeMo#2002's dual-registration approach is superseded. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Codex <adasif@nvidia.com> Signed-off-by: Codex <codex@openai.com> Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A working proof-of-concept for an alternative way to serve resources-server tools over MCP: auto-expose every existing FastAPI tool route as an MCP tool, with zero decorator changes and byte-identical handlers. It's the counterpoint to #2002's
@gym_tooldesign, built so we can compare the two approaches on concrete, runnable code.@gym_tool)main@gym_tool/input_schema)The mechanism (replay bridge)
A frozen
request: Requesthandler can only be invoked correctly by the HTTP machinery itself. So an MCPtools/callis served by re-issuing it as an internal in-process HTTP request through the app's own ASGI stack: verify the signed session token → mint the SessionMiddleware cookie for that session id → replayPOST /<tool>through the full stack (session populated, body validated, unmodified handler runs) → map the HTTP response to an MCP result. MCP-side engine is the official SDK's public low-levelServer— no private attrs.Verified
37/37live acceptance checks pass against pristineorigin/mainhandler code imported directly fromresources_servers/— finance (typed routes) + workplace (27-tool dispatcher via themcp_tool_inventoryoverride). Proves R1 (no decorator), R2 (handlers run over MCP withrequest.sessionworking, sharing state with the HTTP door), R3 (schemas harvested + dispatcher override + plumbing routes), R4 (HTTP door byte-identical bar two additive deltas), plus per-sessionallowed_toolsfiltering and cross-session isolation.python prototypes/mcp_auto_exposure/run_checks.py # -> 37/37 (install fhaviary for +7 aviary checks)Trade-offs on the table (documented, not hidden)
add_session_idBaseHTTPMiddleware(~191 µs), which the HTTP door also pays. Negligible under real tools; real for MB payloads / extreme rates./step//closeare harness plumbing, not model tools, yet get auto-exposed;/stepcarriesenv_id, so an exposedstepcould address other rollouts' envs. Argues for opt-in-per-server + a plumbing-route declaration (mcp_toolless_catchall_pathsis a first cut).route.body_fieldis a semi-internal FastAPI attribute (fails loudly if unresolvable).gym_tools from routes, no replay) is being explored separately.🤖 Generated with Claude Code