diff --git a/tests/e2e/AGENTS.md b/tests/e2e/AGENTS.md index b4ca8bd3d50..a4c51a53098 100644 --- a/tests/e2e/AGENTS.md +++ b/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 `, 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(, 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(, 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... ``` ## 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. diff --git a/tests/e2e/codex/harness.py b/tests/e2e/codex/harness.py index f6dd3700149..df005b5b5e7 100644 --- a/tests/e2e/codex/harness.py +++ b/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: diff --git a/tests/e2e/codex/mcp_server.py b/tests/e2e/codex/mcp_server.py new file mode 100644 index 00000000000..5d35e620248 --- /dev/null +++ b/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() diff --git a/tests/e2e/codex/run.py b/tests/e2e/codex/run.py index 5195da67494..996b2172113 100644 --- a/tests/e2e/codex/run.py +++ b/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: diff --git a/tests/e2e/codex/test_mcp_tool_e2e.py b/tests/e2e/codex/test_mcp_tool_e2e.py new file mode 100644 index 00000000000..89053c47562 --- /dev/null +++ b/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 diff --git a/tests/e2e_harness/codex/test_codex_harness.py b/tests/e2e_harness/codex/test_codex_harness.py new file mode 100644 index 00000000000..520162c4093 --- /dev/null +++ b/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", + } + } diff --git a/tests/e2e_harness/codex/test_codex_run.py b/tests/e2e_harness/codex/test_codex_run.py index 415967da114..93a6580444d 100644 --- a/tests/e2e_harness/codex/test_codex_run.py +++ b/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"}}