test(e2e): add a Codex row that calls a local MCP tool on a first and a resumed turn (#49817)

* test(e2e): add a Codex row that calls a local MCP tool on a first and a resumed turn

* test(e2e): assert the codex mcp row got each value back from the mcp tool call itself

* test(e2e): check the codex mcp row exits cleanly on both turns

* test(e2e): drop the unused content type from the codex mcp result model

tests/e2e/AGENTS.md

@@ -50,7 +50,7 @@ Each subdirectory under `tests/e2e/` is one suite, scoped to an endpoint family
 - `gateway/` - proxy configuration only (`litellm-config.yml`). No tests
 - `claude_code/` - the Claude Code compatibility matrix: drives the real `claude` CLI (and HTTP probes) against a proxy for each feature x provider cell, reporting tagged-union outcomes via the `compat_result` fixture. Ships its own driver/builder/publisher, covered by `tests/e2e_harness/claude_code/`. The HTTP probes ride the shared transport (`ProxyClient.count_tokens` / `ProxyClient.messages`). The CLI-driving path stays bespoke. `pr_gate_version_resolver.py` only forwards to `cli_harness/npm_version_resolver.py`, because the Buildkite runner images still run that path and install @latest without it
 - `cli_harness/` - no tests, only the plumbing every suite that drives a real coding-agent CLI shares: `npm_version_resolver.py` picks the CLI release to install (the newest npm release at least three days old, run by CI as `python tests/e2e/cli_harness/npm_version_resolver.py <npm package>`, stdlib only), `lockdown.py` gives the CLI subprocess an allowlisted env and a fresh empty HOME so it never sees the runner's credentials or dotfiles, and `rate_limiter.py` is the cross-process token bucket per upstream that keeps parallel CLI runs under one request rate. Covered by `tests/e2e_harness/cli_harness/`. Code only one CLI suite needs stays in that suite
-- `codex/` - the Codex compatibility suite: drives the real `codex exec --json` CLI through the gateway, one class per row (the command round trip, a file edit with the patch tool, a long session that follows a chain of files one command at a time, and auto-compaction, where Codex reads a random value, compacts through the gateway and is asked for the value after the file is deleted), each row parametrized over `SETUPS` in `setups.py`, one `pytest.param` per setup (GPT on Bedrock Mantle, Bedrock Runtime, Azure OpenAI and OpenAI, plus Claude on Bedrock). A row with one setup broken by a known bug parametrizes over `with_expected_failure(<setup id>, reason=...)`, which is `SETUPS` with a strict xfail on that one setup. The `setup` fixture registers that setup's deployment under a fresh name via `/model/new` and the `codex` fixture makes a key scoped to it, and both are deleted on teardown. `harness.py` gives each test a throwaway HOME and CODEX_HOME whose `config.toml` points a custom `responses` provider at the gateway and has Codex read its model list from the gateway's `/v1/models` (`model_catalog_url` on the provider), with the sandbox on (`workspace-write`) and approvals off, and runs the CLI under `cli_harness`'s env lockdown and rate limiter. `run.py` reads the JSONL events into a `CodexRun`: the commands with their output, the patched files, the last agent message as `answer`, the usage and the errors, where an error's `status_code` is what lets the suite's 5xx rerun rule catch a turn Codex gave up on. `codex exec --json` prints nothing when Codex compacts, so `rollout.py` reads the compactions from the thread's rollout file under CODEX_HOME/sessions, and a compaction is remote when its replacement history kept the provider's `compaction` item. `exec` and `resume` take `compact_after_tokens`, which forces Codex's remote compaction (`compaction_trigger` items over `/v1/responses`, the path Codex otherwise only takes for OpenAI and Azure) on the gateway provider and compacts once the context grows past that many tokens. A test passes on a random value the model can only learn through the gateway, never on the model's wording or the exact command it typed. It needs `codex` on PATH and fails without it. The Bedrock Runtime setup is explicitly skipped until stage IAM permits `bedrock:InvokeModel` on its default project. The other setups still run, and skipped Runtime cases do not qualify that route. Covered by `tests/e2e_harness/codex/`
+- `codex/` - the Codex compatibility suite: drives the real `codex exec --json` CLI through the gateway, one class per row (the command round trip, a file edit with the patch tool, a long session that follows a chain of files one command at a time, auto-compaction, where Codex reads a random value, compacts through the gateway and is asked for the value after the file is deleted, and an MCP tool, where Codex calls the tool of `mcp_server.py`, a stdio MCP server the test hands it with `add_mcp_server`, on a first and a resumed turn), each row parametrized over `SETUPS` in `setups.py`, one `pytest.param` per setup (GPT on Bedrock Mantle, Bedrock Runtime, Azure OpenAI and OpenAI, plus Claude on Bedrock). A row with one setup broken by a known bug parametrizes over `with_expected_failure(<setup id>, reason=...)`, which is `SETUPS` with a strict xfail on that one setup. The `setup` fixture registers that setup's deployment under a fresh name via `/model/new` and the `codex` fixture makes a key scoped to it, and both are deleted on teardown. `harness.py` gives each test a throwaway HOME and CODEX_HOME whose `config.toml` points a custom `responses` provider at the gateway and has Codex read its model list from the gateway's `/v1/models` (`model_catalog_url` on the provider), with the sandbox on (`workspace-write`) and approvals off, and runs the CLI under `cli_harness`'s env lockdown and rate limiter. `run.py` reads the JSONL events into a `CodexRun`: the commands with their output, the patched files, the MCP tool calls with the text they returned, the last agent message as `answer`, the usage and the errors, where an error's `status_code` is what lets the suite's 5xx rerun rule catch a turn Codex gave up on. `codex exec --json` prints nothing when Codex compacts, so `rollout.py` reads the compactions from the thread's rollout file under CODEX_HOME/sessions, and a compaction is remote when its replacement history kept the provider's `compaction` item. `exec` and `resume` take `compact_after_tokens`, which forces Codex's remote compaction (`compaction_trigger` items over `/v1/responses`, the path Codex otherwise only takes for OpenAI and Azure) on the gateway provider and compacts once the context grows past that many tokens. A test passes on a random value the model can only learn through the gateway, never on the model's wording or the exact command it typed. It needs `codex` on PATH and fails without it. Covered by `tests/e2e_harness/codex/`
 - `ui/` - the Admin UI browser suite: Playwright in TypeScript, driving the dashboard served by a live proxy on port 4000 (seeded postgres + mock LLM upstream, see its `run_e2e.sh`). It is a self-contained npm package with its own lockfile and does not use the Python harness, pytest markers, or the shared transport. The Python rules in this file (typed models, `Result` unions, basedpyright zero-error gate) do not apply inside it. Its only Python file, `fixtures/mock_llm_server/server.py`, is excluded from the e2e basedpyright gate via the root `pyrightconfig.json`. Its specs follow the dashboard's rule against testing what shadcn and Base UI already guarantee, in `ui/litellm-dashboard/AGENTS.md`
 
 ## MCP suite: real Datadog only
@@ -315,6 +315,8 @@ other.<area>.<case>.<assertion>
 ```
 
 ## Hard Rules
+- no comments in code unless they are truly necessary
+
 - no unit tests of a product feature under `tests/e2e`, and no mock tests or monkeypatching of code anywhere in it: a product feature is proven end to end against a live proxy, never with a unit test. if a contributor asks you to write an end to end test, do NOT stage a unit test with it. If you find a product gap, call it out in the PR description. the harness's own plumbing is tested outside the suite, in `tests/e2e_harness/` (mirroring this folder's layout), because the Buildkite e2e run copies `tests/e2e/` into the runner image and runs every test in it, so a harness test in here would count as a product test in the nightly numbers. those tests run without a proxy and take their inputs as arguments or env vars (setting an env var through pytest's `monkeypatch` fixture is fine, patching a function, class, or module is not), and no coverage-registry or compat-matrix cell rests on them. judge a change inside one of them by that standard, not as a misplaced product test
 
 - use model management endpoints to create new models for a test. this could be in a conftest / inline for each test. ask the user what they want.

tests/e2e/codex/harness.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import json
 import subprocess
 import tempfile
 from collections.abc import Generator, Mapping
@@ -37,6 +38,13 @@ model_catalog_url = "{gateway}/v1/models"
 """
 
 
+@dataclass(frozen=True, slots=True)
+class McpServer:
+    command: str
+    args: tuple[str, ...]
+    env: Mapping[str, str]
+
+
 @dataclass(frozen=True, slots=True)
 class CodexHarness:
     home: Path
@@ -57,6 +65,11 @@ class CodexHarness:
             _ = (workspace / name).write_text(content)
         return workspace
 
+    @step("Give Codex the {name} MCP server")
+    def add_mcp_server(self, name: str, server: McpServer) -> None:
+        with (self.codex_home / "config.toml").open("a") as config:
+            _ = config.write(_mcp_server_table(name, server))
+
     @step('Run Codex on {model} through the proxy with the prompt "{prompt}"')
     def exec(self, prompt: str, *, model: str, cwd: Path, compact_after_tokens: int | None = None) -> CodexRun:
         return self._run(("exec",), (*_compaction_flags(compact_after_tokens), "--model", model, "--", prompt), cwd)
@@ -92,6 +105,17 @@ class CodexHarness:
         return codex_run(exit_code=completed.returncode, stdout=completed.stdout, stderr=completed.stderr)
 
 
+def _mcp_server_table(name: str, server: McpServer) -> str:
+    env: Final = ", ".join(f"{json.dumps(key)} = {json.dumps(value)}" for key, value in server.env.items())
+    return (
+        f"\n[mcp_servers.{json.dumps(name)}]\n"
+        f"command = {json.dumps(server.command)}\n"
+        f"args = {json.dumps(server.args)}\n"
+        f"env = {{ {env} }}\n"
+        'default_tools_approval_mode = "approve"\n'
+    )
+
+
 def _compaction_flags(after_tokens: int | None) -> tuple[str, ...]:
     match after_tokens:
         case None:

tests/e2e/codex/mcp_server.py

@@ -0,0 +1,19 @@
+from __future__ import annotations
+
+import os
+from pathlib import Path
+from typing import Final
+
+from mcp.server.mcpserver import MCPServer
+
+server: Final = MCPServer("vault")
+
+
+@server.tool()
+def read_secret() -> str:
+    """Return the current secret value."""
+    return Path(os.environ["SECRET_FILE"]).read_text()
+
+
+if __name__ == "__main__":
+    server.run()

tests/e2e/codex/run.py

@@ -54,6 +54,22 @@ class _FileChange(_Frozen):
     status: Literal["in_progress", "completed", "failed"]
 
 
+class _McpContentBlock(_Frozen):
+    text: str | None = None
+
+
+class _McpResult(_Frozen):
+    content: tuple[_McpContentBlock, ...]
+
+
+class _McpToolCall(_Frozen):
+    type: Literal["mcp_tool_call"]
+    server: str
+    tool: str
+    result: _McpResult | None
+    status: Literal["in_progress", "completed", "failed"]
+
+
 class _ErrorItem(_Frozen):
     type: Literal["error"]
     message: str
@@ -79,9 +95,12 @@ _Item = Annotated[
     Annotated[_AgentMessage, Tag("agent_message")]
     | Annotated[_CommandExecution, Tag("command_execution")]
     | Annotated[_FileChange, Tag("file_change")]
+    | Annotated[_McpToolCall, Tag("mcp_tool_call")]
     | Annotated[_ErrorItem, Tag("error")]
     | Annotated[_Unmodelled, Tag("unmodelled")],
-    Discriminator(_known_or_unmodelled(frozenset({"agent_message", "command_execution", "file_change", "error"}))),
+    Discriminator(
+        _known_or_unmodelled(frozenset({"agent_message", "command_execution", "file_change", "mcp_tool_call", "error"}))
+    ),
 ]
 
 
@@ -131,6 +150,13 @@ class Command:
     exit_code: int | None
 
 
+@dataclass(frozen=True, slots=True)
+class McpCall:
+    server: str
+    tool: str
+    output: str
+
+
 @dataclass(frozen=True, slots=True)
 class CodexError:
     message: str
@@ -143,6 +169,7 @@ class CodexRun:
     thread_id: str | None
     commands: tuple[Command, ...]
     patched_files: tuple[Path, ...]
+    mcp_calls: tuple[McpCall, ...]
     answer: str
     usage: TokenUsage | None
     errors: tuple[CodexError, ...]
@@ -164,6 +191,9 @@ def codex_run(*, exit_code: int, stdout: str, stderr: str) -> CodexRun:
             if isinstance(item, _CommandExecution)
         ),
         patched_files=tuple(Path(change.path) for change in chain.from_iterable(patch.changes for patch in patches)),
+        mcp_calls=tuple(
+            _mcp_call(item) for item in items if isinstance(item, _McpToolCall) and item.status == "completed"
+        ),
         answer=answers[-1] if answers else "",
         usage=usages[-1] if usages else None,
         errors=tuple(_errors(events)),
@@ -171,6 +201,11 @@ def codex_run(*, exit_code: int, stdout: str, stderr: str) -> CodexRun:
     )
 
 
+def _mcp_call(item: _McpToolCall) -> McpCall:
+    blocks: Final = item.result.content if item.result else ()
+    return McpCall(server=item.server, tool=item.tool, output="".join(block.text or "" for block in blocks))
+
+
 def _errors(events: tuple[_Event, ...]) -> Iterator[CodexError]:
     for event in events:
         match event:

tests/e2e/codex/test_mcp_tool_e2e.py

@@ -0,0 +1,47 @@
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+from typing import Final
+
+import pytest
+from codex.harness import CodexHarness, McpServer
+from codex.setups import SETUPS, ModelSetup
+from e2e_config import unique_marker
+
+pytestmark = pytest.mark.e2e
+
+MCP_SERVER: Final = Path(__file__).with_name("mcp_server.py")
+
+
+@pytest.mark.parametrize("deployment", SETUPS)
+class TestCodexMcpTool:
+    def test_calls_a_local_mcp_tool_again_on_a_resumed_turn(self, codex: CodexHarness, setup: ModelSetup) -> None:
+        secret: Final = codex.home / "secret.txt"
+        vault: Final = McpServer(command=sys.executable, args=(str(MCP_SERVER),), env={"SECRET_FILE": str(secret)})
+        codex.add_mcp_server("vault", vault)
+        workspace: Final = codex.new_workspace(files={})
+
+        first_marker: Final = unique_marker()
+        _ = secret.write_text(first_marker)
+        first: Final = codex.exec(
+            "Call the read_secret tool of the vault MCP server, then reply with exactly what it returned.",
+            model=setup.model,
+            cwd=workspace,
+        )
+        assert first.exit_code == 0, first
+        assert first.thread_id is not None, first
+        assert any(first_marker in call.output for call in first.mcp_calls), first
+        assert first_marker in first.answer, first
+
+        second_marker: Final = unique_marker()
+        _ = secret.write_text(second_marker)
+        second: Final = codex.resume(
+            first.thread_id,
+            "The secret has changed. Call the read_secret tool again, then reply with exactly what it returned.",
+            model=setup.model,
+            cwd=workspace,
+        )
+        assert second.exit_code == 0, second
+        assert any(second_marker in call.output for call in second.mcp_calls), second
+        assert second_marker in second.answer, second

tests/e2e_harness/codex/test_codex_harness.py

@@ -0,0 +1,35 @@
+from __future__ import annotations
+
+import time
+import tomllib
+from pathlib import Path
+from typing import Final
+
+from cli_harness.rate_limiter import RateBucket, RateLimiter
+from codex.harness import McpServer, codex_harness
+
+
+def test_an_added_mcp_server_is_a_table_codex_reads_next_to_the_gateway_provider(tmp_path: Path) -> None:
+    server: Final = McpServer(
+        command='/opt/py "3.14"/bin/python',
+        args=("C:\\tools\\mcp_server.py", "--flag"),
+        env={"SECRET_FILE": "/tmp/a dir/secret.txt", "OTHER": "x"},
+    )
+    with codex_harness(
+        gateway="http://127.0.0.1:4000",
+        api_key="unused",
+        rate_bucket=RateBucket.OPENAI,
+        limiter=RateLimiter({}, tmp_path, time.monotonic, time.sleep),
+    ) as codex:
+        codex.add_mcp_server("vault", server)
+        config: Final = tomllib.loads((codex.codex_home / "config.toml").read_text())
+
+    assert config["model_provider"] == "litellm"
+    assert config["mcp_servers"] == {
+        "vault": {
+            "command": server.command,
+            "args": list(server.args),
+            "env": dict(server.env),
+            "default_tools_approval_mode": "approve",
+        }
+    }

tests/e2e_harness/codex/test_codex_run.py

@@ -8,7 +8,7 @@ from collections.abc import Mapping
 from pathlib import Path
 from typing import Final
 
-from codex.run import Command, codex_run
+from codex.run import Command, McpCall, codex_run
 
 E2E_PYTEST_INI: Final = Path(__file__).resolve().parents[2] / "e2e" / "pytest.ini"
 
@@ -60,6 +60,30 @@ class TestCodexRun:
 
         assert run.patched_files == (Path("/w/app.py"),)
 
+    def test_only_an_mcp_tool_call_that_completed_counts_and_keeps_the_text_the_tool_returned(self) -> None:
+        def call(status: str, result: Mapping[str, object] | None) -> str:
+            item: Final = {
+                "id": status,
+                "type": "mcp_tool_call",
+                "server": "vault",
+                "tool": "read_secret",
+                "arguments": {},
+                "result": result,
+                "error": None if result else {"message": "tool call failed"},
+                "status": status,
+            }
+            return _line({"type": "item.completed", "item": item})
+
+        returned: Final = {
+            "content": [{"type": "text", "text": "marker-7f3a91"}],
+            "structured_content": {"result": "marker-7f3a91"},
+        }
+        run: Final = codex_run(
+            exit_code=0, stdout="\n".join((call("completed", returned), call("failed", None))), stderr=""
+        )
+
+        assert run.mcp_calls == (McpCall(server="vault", tool="read_secret", output="marker-7f3a91"),)
+
     def test_a_turn_that_gave_up_on_a_5xx_is_rerun_by_the_suites_network_rule(self) -> None:
         failed: Final = _line(
             {"type": "turn.failed", "error": {"message": "exceeded retry limit, last status: 502 Bad Gateway"}}