From 46654c1628c5ec5d1e87d3f35f86f830696218a7 Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 10:15:20 -0500 Subject: [PATCH 1/6] feat(agents): select opencode's own tools at deploy with builtin_tools OpenCodeConfig takes builtin_tools, as claude-code's does (#437), and turns it into the config's permission: a denied tool is left out of the model's tool list. disable "*" and only keep the tools of agent-env's MCP servers, which a "*" deny would also remove. The names are opencode-ai 1.17.18's permission names. Without a selection the config and argv are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../agents/agents/opencode/README.md | 10 +- .../agents/agents/opencode/a2a_server.py | 4 + .../agents/agents/opencode/opencode_bridge.py | 66 ++++++++- .../test_opencode_bridge_builtin_tools.py | 138 ++++++++++++++++++ 4 files changed, 212 insertions(+), 6 deletions(-) create mode 100644 tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py diff --git a/src/agent_env/agents/agents/opencode/README.md b/src/agent_env/agents/agents/opencode/README.md index a630fcfea..8da5a1843 100644 --- a/src/agent_env/agents/agents/opencode/README.md +++ b/src/agent_env/agents/agents/opencode/README.md @@ -29,7 +29,7 @@ disk: | Extension | Implementation | |---|---| -| `agent-config/v1` | `OpenCodeConfig`: `model`, `effort` (the provider's `reasoningEffort`), `system_prompt`, `output_format` (a JSON schema the answer is checked against), `project_id`, `task_id`, `timeout_seconds` (1800 by default) | +| `agent-config/v1` | `OpenCodeConfig`: `model`, `effort` (the provider's `reasoningEffort`), `system_prompt`, `output_format` (a JSON schema the answer is checked against), `project_id`, `task_id`, `timeout_seconds` (1800 by default), `builtin_tools` (below) | | `mcp-config/v1` | SDK default; each server passed to the task's CLI | | `skill-config/v1` | inline and bundle skills installed as `$XDG_CONFIG_HOME/opencode/skills//`, where OpenCode finds them | | `trajectory/v1` | the task's `opencode run --format json` events, live while it runs (format `opencode-events/1`) | @@ -40,3 +40,11 @@ disk: The image is `python:3.11-slim` with Node, `pdftoppm`, the CLI and the agent, running as the `agent` user. OpenCode's state is under `/data`, its `HOME`, whatever the uid. + +A `deploy_agent` `builtin_tools` selection picks the CLI's own tools through the config's `permission`, which leaves a +denied tool out of the model's tool list rather than refusing it when called. `{"disable": "*"}` and `{"only": [...]}` +deny every tool but those named and the tools of the MCP servers agent-env gives the agent (`_*`), a list to +turn off denies those alone, and `question` is denied in every case. The names are `OpenCodeTool`'s in +`opencode_bridge.py`, the CLI's permission names: `edit` also covers `write` and `apply_patch`, which can't be selected +alone, and `read` covers the MCP resource tools. A test fails when package.json's `opencode-ai` moves, until +`OpenCodeTool` and `TOOLS_CLI_VERSION` move with it. diff --git a/src/agent_env/agents/agents/opencode/a2a_server.py b/src/agent_env/agents/agents/opencode/a2a_server.py index 6bd5b93b3..c95f34da8 100644 --- a/src/agent_env/agents/agents/opencode/a2a_server.py +++ b/src/agent_env/agents/agents/opencode/a2a_server.py @@ -67,6 +67,7 @@ from opencode_bridge import ( ERROR_MESSAGE_LIMIT, + OpenCodeTools, event_session_id, export_session, get_last_attribution_snapshot, @@ -138,6 +139,8 @@ class OpenCodeConfig(AgentConfig): project_id: str | None = None task_id: str | None = None timeout_seconds: int = Field(default=1800, gt=0) + # Which of the CLI's own tools it may use. With any selection, the tools of the MCP servers agent-env gives it stay. + builtin_tools: OpenCodeTools | None = None @field_validator("output_format") @classmethod @@ -378,6 +381,7 @@ def record(event: dict) -> None: task_id=request.config.task_id, role=request.config.role, attachment_paths=attachments, + builtin_tools=request.config.builtin_tools, event_callback=record, workspace=str(WORKSPACE_DIR), ) diff --git a/src/agent_env/agents/agents/opencode/opencode_bridge.py b/src/agent_env/agents/agents/opencode/opencode_bridge.py index 6d150bfa9..9635cdec0 100644 --- a/src/agent_env/agents/agents/opencode/opencode_bridge.py +++ b/src/agent_env/agents/agents/opencode/opencode_bridge.py @@ -27,8 +27,10 @@ from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path +from typing import Literal from jsonschema import Draft202012Validator, SchemaError, ValidationError +from pydantic import BaseModel, ConfigDict, model_validator logger = logging.getLogger(__name__) @@ -45,6 +47,32 @@ SESSIONS_ROOT = os.environ.get("OPENCODE_SESSIONS_ROOT") or os.path.join(os.environ.get("HOME", "/data"), "sessions") _CLI_TIMEOUT_S = 120 +# The CLI version OpenCodeTool names the tools of; a test holds it to package.json's opencode-ai. +TOOLS_CLI_VERSION = "1.17.18" +# The CLI's own tools, by the permission names that remove them from the model's tool list. `edit` covers `write` and +# `apply_patch` too, which the CLI offers in its place for some models, so neither can be selected alone. `question` +# is left out: it is denied on every turn, so a headless run never waits for a human. Which tools the CLI offers by +# default depends on the model and its flags (`lsp`, `websearch` and `plan_exit` are off by default), and its MCP +# resource tools come only with MCP servers, under `read`. +OpenCodeTool = Literal["bash", "edit", "glob", "grep", "lsp", "plan_exit", "read", "skill", "task", "todowrite", + "webfetch", "websearch"] + + +class OpenCodeTools(BaseModel): + """The protocol's ``BuiltinTools[OpenCodeTool]``, which the image's protocol floor doesn't have yet: the same + fields and checks, so the card's schema is the same.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + disable: Literal["*"] | list[OpenCodeTool] | None = None + only: list[OpenCodeTool] | None = None + + @model_validator(mode="after") + def _one_of(self) -> OpenCodeTools: + if (self.disable is None) == (self.only is None): + raise ValueError("builtin_tools takes exactly one of disable and only") + return self + def _base_url(environ: Mapping[str, str]) -> str: """The endpoint's chat-completions root: agent-env's LITELLM_BASE_URL has no /v1 suffix.""" @@ -196,6 +224,30 @@ def _mcp_headers(headers: Mapping[str, str] | None, role: str | None) -> dict[st return merged +def _mcp_tool_prefix(server: str) -> str: + """The prefix of the names the CLI gives a server's tools: ``_``, each part with every character + but letters, digits, ``_`` and ``-`` replaced by ``_``, as opencode-ai 1.17.18 names them.""" + return re.sub(r"[^a-zA-Z0-9_-]", "_", server) + "_" + + +def _permission(builtin_tools: OpenCodeTools | None, mcp_servers: dict[str, dict] | None) -> dict[str, str]: + """The config's ``permission``: which tools the CLI offers the model. A denied tool is left out of the model's + tool list, not only refused when called. Without a selection every tool is allowed; ``{"disable": "*"}`` and + ``{"only": [...]}`` deny every tool but those named and the tools of ``mcp_servers``, which a ``"*"`` deny would + remove too; ``{"disable": [...]}`` denies those alone. ``question`` is denied in every case, and last, since the + CLI applies the last rule that matches a tool.""" + if builtin_tools is None: + return {"*": "allow", "question": "deny"} + if isinstance(builtin_tools.disable, list): + return {"*": "allow", **{tool: "deny" for tool in builtin_tools.disable}, "question": "deny"} + return { + "*": "deny", + **{tool: "allow" for tool in builtin_tools.only or []}, + **{f"{_mcp_tool_prefix(name)}*": "allow" for name in mcp_servers or {}}, + "question": "deny", + } + + def _build_config( model: str, mcp_servers: dict[str, dict] | None, @@ -204,6 +256,7 @@ def _build_config( effort: str | None = None, environ: Mapping[str, str] = os.environ, instructions: list[str] | None = None, + builtin_tools: OpenCodeTools | None = None, ) -> dict: """Assemble the inline OpenCode config passed via OPENCODE_CONFIG_CONTENT. @@ -250,9 +303,10 @@ def _build_config( } }, "model": f"litellm/{model}", - # Headless: allow every tool, hard-deny the "ask the human" tool so a - # run can never block waiting for input. - "permission": {"*": "allow", "question": "deny"}, + # Headless: hard-deny the "ask the human" tool so a run can never block + # waiting for input; every other tool is allowed unless builtin_tools + # selects otherwise. + "permission": _permission(builtin_tools, mcp_servers), "autoupdate": False, } if instructions: @@ -363,11 +417,13 @@ async def run_opencode( role: str | None = None, attachment_paths: list[str] | None = None, effort: str | None = None, + builtin_tools: OpenCodeTools | None = None, event_callback: Callable[[dict], None] | None = None, workspace: str = "/app", ) -> OpencodeResult: """Run one `opencode run` turn in ``workspace``. ``event_callback`` gets each JSON event as the CLI prints it: - exactly the events of the returned trajectory, in order.""" + exactly the events of the returned trajectory, in order. ``builtin_tools`` selects the CLI's own tools, as + ``_permission`` describes; the tools of ``mcp_servers`` stay offered with any selection.""" model = model or DEFAULT_MODEL attribution = _attribution_headers(project_id, task_id) if attribution: @@ -385,7 +441,7 @@ async def run_opencode( ) config = _build_config(model, mcp_servers, provider_headers(os.environ, attribution), role, effort, - instructions=instructions) + instructions=instructions, builtin_tools=builtin_tools) # Resume a specific session by id (multi-turn continuity). `--continue` # does NOT replay history to the model; `-s ` does. First turn diff --git a/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py b/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py new file mode 100644 index 000000000..3bc92e4b2 --- /dev/null +++ b/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py @@ -0,0 +1,138 @@ +"""A selection of the CLI's own tools, as the config's ``permission``: ``"*"`` and ``only`` deny every tool but those +named and the tools of agent-env's MCP servers, a list to turn off denies those alone, and ``question`` is denied in +every case; with none, the config and argv are what they were. The card's agent-config takes the field, so deploy_agent +accepts a selection for opencode, and a tool the CLI doesn't have is refused. The selection's type has the protocol's +schema, and its tool names are those of the CLI version package.json pins.""" + +import asyncio +import json +from typing import Any + +import pytest +from agentenv_protocol.a2a_agent import BuiltinTools + +from agent_env.a2a_agent import A2AAgent +from agent_env.task_step.task_steps.deploy_agent import _check_builtin_tools +from tst.unit.agents.opencode.agent import AGENT_DIR, a2a_server, opencode_bridge +from tst.unit.agents.opencode.fake_cli import FakeCLI + +MODEL = "fireworks_ai/kimi-k3" +_STOP = {"type": "step_finish", "sessionID": "ses_1", "part": {"type": "step-finish", "reason": "stop"}} +_SERVERS = {"env4821": {"url": "https://gw/mcp", "headers": None}, + "peer_agents": {"url": "http://127.0.0.1:8000/ext/peer-agents/mcp", "headers": None}, + "fake-env.x": {"url": "https://gw2/mcp", "headers": None}} + + +def _permission(selection: dict | None, servers: dict | None = None) -> dict: + tools = a2a_server.OpenCodeConfig(builtin_tools=selection).builtin_tools + return opencode_bridge._build_config(MODEL, servers, {}, None, builtin_tools=tools)["permission"] + + +@pytest.fixture +def spawned(monkeypatch): + """Every spawn's argv and environment, each answered by a CLI that finishes its turn.""" + calls: list[tuple[list[str], dict]] = [] + + async def _exec(*cmd, **kwargs): + calls.append((list(cmd), kwargs["env"])) + return FakeCLI(json.dumps(_STOP).encode() + b"\n") + + monkeypatch.setattr(asyncio, "create_subprocess_exec", _exec) + monkeypatch.setattr(opencode_bridge, "_context_xdg", lambda _context: {"XDG_DATA_HOME": "/ctx/share"}) + return calls + + +def test_no_selection_leaves_the_config_and_argv_as_they_were(monkeypatch, spawned): + """The whole config and argv of a turn without a selection, as origin/main builds them.""" + monkeypatch.setenv("LITELLM_BASE_URL", "https://gateway.example") + monkeypatch.delenv("LITELLM_EXTRA_HEADERS", raising=False) + + asyncio.run(opencode_bridge.run_opencode( + prompt="go", context_id="ctx", session_id="ses_0", model="m", mcp_servers={"env": {"url": "https://gw/mcp"}}, + effort="high", attachment_paths=["/w/a.png"], workspace="/w")) + + ((argv, env),) = spawned + assert argv == ["opencode", "run", "--format", "json", "--model", "litellm/m", "--dir", "/w", "-s", "ses_0", "go", + "--file", "/w/a.png"] + assert env["OPENCODE_CONFIG_CONTENT"] == json.dumps({ + "$schema": "https://opencode.ai/config.json", + "provider": {"litellm": { + "npm": "@ai-sdk/openai-compatible", "name": "LiteLLM", + "options": {"baseURL": "https://gateway.example/v1", "apiKey": "{env:LITELLM_API_KEY}"}, + "models": {"m": {"name": "m", "modalities": {"input": ["text", "image"], "output": ["text"]}, + "options": {"additional_drop_params": ["tool_choice"], "reasoningEffort": "high"}}}, + }}, + "model": "litellm/m", + "permission": {"*": "allow", "question": "deny"}, + "autoupdate": False, + "mcp": {"env": {"type": "remote", "url": "https://gw/mcp", "enabled": True}}, + }) + + +@pytest.mark.parametrize("selection, permission", [ + ({"disable": "*"}, {"*": "deny", "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", + "question": "deny"}), + ({"only": ["read", "grep"]}, {"*": "deny", "read": "allow", "grep": "allow", "env4821_*": "allow", + "peer_agents_*": "allow", "fake-env_x_*": "allow", "question": "deny"}), + ({"only": []}, {"*": "deny", "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", + "question": "deny"}), + ({"disable": ["bash", "webfetch"]}, {"*": "allow", "bash": "deny", "webfetch": "deny", "question": "deny"}), +]) +def test_a_selection_is_the_permission_and_agent_envs_mcp_tools_stay(selection, permission): + result = _permission(selection, _SERVERS) + + assert result == permission + assert list(result)[-1] == "question" # the CLI applies the last rule that matches a tool + + +def test_without_mcp_servers_disable_all_denies_every_tool(): + assert _permission({"disable": "*"}) == {"*": "deny", "question": "deny"} + + +def test_the_selection_reaches_the_cli(spawned): + tools = a2a_server.OpenCodeConfig(builtin_tools={"disable": "*"}).builtin_tools + + asyncio.run(opencode_bridge.run_opencode(prompt="go", context_id="ctx", builtin_tools=tools)) + + ((argv, env),) = spawned + assert json.loads(env["OPENCODE_CONFIG_CONTENT"])["permission"] == {"*": "deny", "question": "deny"} + assert argv[-1] == "go" + + +@pytest.mark.parametrize("selection, match", [ + ({"only": ["write"]}, "write"), + ({"disable": ["question"]}, "question"), + ({"disable": "*", "only": ["read"]}, "exactly one"), + ({}, "exactly one"), + ({"disable": "all"}, "disable"), +]) +def test_an_invalid_selection_is_refused(selection, match): + with pytest.raises(ValueError, match=match): + a2a_server.OpenCodeConfig(builtin_tools=selection) + + +def test_the_card_takes_builtin_tools_so_deploy_agent_accepts_a_selection(oc): + extension = A2AAgent.find_extension(oc.card, A2AAgent.EXT_AGENT_CONFIG) + + assert "builtin_tools" in extension["params"]["methods"]["set"]["request"]["supported"] + _check_builtin_tools(extension, "opencode", {"disable": "*"}) + _check_builtin_tools(extension, "opencode", {"only": ["read", "edit"]}) + with pytest.raises(RuntimeError, match="can't take builtin_tools"): + _check_builtin_tools(extension, "opencode", {"disable": ["Bash"]}) + + +def test_the_selection_has_the_protocols_schema(): + def untitled(schema: Any) -> Any: + if isinstance(schema, dict): + return {k: untitled(v) for k, v in schema.items() if k not in ("title", "description")} + return [untitled(v) for v in schema] if isinstance(schema, list) else schema + + assert untitled(opencode_bridge.OpenCodeTools.model_json_schema()) == untitled( + BuiltinTools[opencode_bridge.OpenCodeTool].model_json_schema()) + + +def test_the_tool_names_are_those_of_the_cli_version_package_json_pins(): + pinned = json.loads((AGENT_DIR / "package.json").read_text())["dependencies"]["opencode-ai"] + + assert opencode_bridge.TOOLS_CLI_VERSION == pinned, ( + "the CLI version changed: record the tools the new CLI offers in OpenCodeTool, then TOOLS_CLI_VERSION") From e65b162bb0776959f8a0215601d8f3c1feb84f2d Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 10:15:25 -0500 Subject: [PATCH 2/6] feat(agents): select codex's own tools at deploy with builtin_tools CodexConfig takes builtin_tools and turns it into -c overrides on the turn's CLI. disable "*" also turns off the multi_agent_v2 collaboration tools. The MCP tools agent-env gives the agent stay. Tools that one setting switches together must be named together. apply_patch is left out: codex 0.147.0 has no setting that turns it off. Without a selection the overrides are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/agents/agents/codex/README.md | 23 ++- .../agents/agents/codex/a2a_server.py | 5 +- .../agents/agents/codex/codex_config.py | 59 ++++++- tst/unit/agents/codex/test_card_contract.py | 2 +- .../agents/codex/test_codex_builtin_tools.py | 150 ++++++++++++++++++ 5 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 tst/unit/agents/codex/test_codex_builtin_tools.py diff --git a/src/agent_env/agents/agents/codex/README.md b/src/agent_env/agents/agents/codex/README.md index 69da8dbcc..dde0dfe9e 100644 --- a/src/agent_env/agents/agents/codex/README.md +++ b/src/agent_env/agents/agents/codex/README.md @@ -26,7 +26,7 @@ Codex reads its settings from `$CODEX_HOME/config.toml`, which the agent writes: | Extension | Implementation | |---|---| -| `agent-config/v1` | `CodexConfig`: `model`, `effort` (Codex's `model_reasoning_effort`; a set without it clears it), `system_prompt` (Codex's `developer_instructions`), `output_format` (a JSON schema, as `--output-schema`), `project_id`, `task_id`, `timeout_seconds` | +| `agent-config/v1` | `CodexConfig`: `model`, `effort` (Codex's `model_reasoning_effort`; a set without it clears it), `system_prompt` (Codex's `developer_instructions`), `output_format` (a JSON schema, as `--output-schema`), `project_id`, `task_id`, `timeout_seconds`, `builtin_tools` (below) | | `mcp-config/v1` | SDK default; written to `config.toml` before each task | | `skill-config/v1` | inline and bundle skills installed as `~/.agents/skills//SKILL.md`, where Codex finds them | | `trajectory/v1` | the task's `codex exec --json` events, live while it runs (format `codex-exec-json/1`) | @@ -38,3 +38,24 @@ Codex reads its settings from `$CODEX_HOME/config.toml`, which the agent writes: The image is `python:3.11-slim` with Node, the CLIs and the agent, running as the `agent` user; Codex's state is in `/home/agent/.codex` whatever the uid. + +A `deploy_agent` `builtin_tools` selection turns the CLI's own tools off with `-c` overrides on the task's CLI alone; +config.toml stays as it is. `{"disable": "*"}` turns off every setting below, `{"only": [...]}` every one but those whose +tools it names, and a list to turn off those whose tools it names. A list names all of a setting's tools or none: + +| Tools | Override | +|---|---| +| `exec_command`, `write_stdin` | `features.shell_tool=false` | +| `update_plan` | `tools.update_plan.enabled=false` | +| `request_user_input` | `tools.experimental_request_user_input.enabled=false` | +| `view_image` | `features.view_image=false` | +| `spawn_agent`, `send_message`, `wait_agent`, `followup_task`, `interrupt_agent`, `list_agents` | `features.multi_agent_v2=false` and `features.multi_agent=false`, since the v1 tools come back behind `tool_search` otherwise | +| `create_goal`, `get_goal`, `update_goal` | `features.goals=false` | +| `web_search` | `web_search="disabled"` | + +Codex 0.147.0 can't turn off `apply_patch` with a setting: a model's entry in its catalog turns it on (every GPT model it +ships with does), so `{"disable": "*"}` still leaves it for those models, and it isn't a name a selection takes. The MCP +tools agent-env gives the agent stay with any selection, and so do the tools Codex adds for them: `tool_search`, +`list_mcp_resources`, `list_mcp_resource_templates` and `read_mcp_resource`. The names are `CodexTool`'s in +`codex_config.py`; a test fails when package.json's `@openai/codex` moves, until `CodexTool` and `TOOLS_CLI_VERSION` +move with it. diff --git a/src/agent_env/agents/agents/codex/a2a_server.py b/src/agent_env/agents/agents/codex/a2a_server.py index 87b1ccf7d..e34e15ab2 100644 --- a/src/agent_env/agents/agents/codex/a2a_server.py +++ b/src/agent_env/agents/agents/codex/a2a_server.py @@ -235,6 +235,8 @@ class CodexConfig(AgentConfig): project_id: str | None = None task_id: str | None = None timeout_seconds: int = 1800 + # Which of the CLI's own tools it may use. With any selection, the MCP tools agent-env gives it stay. + builtin_tools: codex_config.CodexTools | None = None @asynccontextmanager @@ -679,7 +681,8 @@ def _record_event(event: dict) -> None: project_id=cfg.project_id, task_id=cfg.task_id, event_callback=_record_event, - config_overrides=codex_config.role_overrides(request.mcp_servers, cfg.role), + config_overrides=[*codex_config.role_overrides(request.mcp_servers, cfg.role), + *codex_config.builtin_tools_overrides(cfg.builtin_tools)], ) except Exception as e: logger.error(f"codex CLI invocation raised: {e}", exc_info=True) diff --git a/src/agent_env/agents/agents/codex/codex_config.py b/src/agent_env/agents/agents/codex/codex_config.py index ee1fe0370..2bea330dd 100644 --- a/src/agent_env/agents/agents/codex/codex_config.py +++ b/src/agent_env/agents/agents/codex/codex_config.py @@ -14,9 +14,10 @@ import tomllib from collections.abc import Mapping from pathlib import Path -from typing import Any +from typing import Any, Literal import tomli_w +from pydantic import BaseModel, ConfigDict, model_validator logger = logging.getLogger(__name__) @@ -29,6 +30,62 @@ # certificates to trust. _TRUST_ENV = ("NODE_EXTRA_CA_CERTS", "NODE_TLS_REJECT_UNAUTHORIZED", "SSL_CERT_FILE") +# The CLI version CodexTool names the tools of; a test holds it to package.json's @openai/codex. +TOOLS_CLI_VERSION = "0.147.0" +# The CLI's own tools, by the names its model sees, that a `-c` override turns off. Some only go together, as one +# setting turns them on: the shell's two, the six collaboration tools `multi_agent_v2` brings, and the three goal tools. +# Left out: `apply_patch`, which a model's catalog entry turns on and no setting turns off, and the MCP tools +# (`tool_search`, `list_mcp_resources`, `list_mcp_resource_templates`, `read_mcp_resource`) that come with MCP servers. +CodexTool = Literal["create_goal", "exec_command", "followup_task", "get_goal", "interrupt_agent", "list_agents", + "request_user_input", "send_message", "spawn_agent", "update_goal", "update_plan", "view_image", + "wait_agent", "web_search", "write_stdin"] +# Each setting that turns some of the CLI's tools off: those tools, and the `-c` overrides that turn them off. Turning +# off `multi_agent_v2` alone would bring back the v1 collaboration tools behind `tool_search`, so both go. +_TOOL_SWITCHES: tuple[tuple[frozenset[str], tuple[str, ...]], ...] = ( + (frozenset({"exec_command", "write_stdin"}), ("features.shell_tool=false",)), + (frozenset({"update_plan"}), ("tools.update_plan.enabled=false",)), + (frozenset({"request_user_input"}), ("tools.experimental_request_user_input.enabled=false",)), + (frozenset({"view_image"}), ("features.view_image=false",)), + (frozenset({"followup_task", "interrupt_agent", "list_agents", "send_message", "spawn_agent", "wait_agent"}), + ("features.multi_agent_v2=false", "features.multi_agent=false")), + (frozenset({"create_goal", "get_goal", "update_goal"}), ("features.goals=false",)), + (frozenset({"web_search"}), ('web_search="disabled"',)), +) + + +class CodexTools(BaseModel): + """The protocol's ``BuiltinTools[CodexTool]``, which the image's protocol floor doesn't have yet: the same fields + and checks, so the card's schema is the same. A list must name all the tools of a setting or none of them.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + disable: Literal["*"] | list[CodexTool] | None = None + only: list[CodexTool] | None = None + + @model_validator(mode="after") + def _one_of(self) -> CodexTools: + if (self.disable is None) == (self.only is None): + raise ValueError("builtin_tools takes exactly one of disable and only") + named = set(self.only if self.only is not None else [] if self.disable == "*" else self.disable) + for tools, _ in _TOOL_SWITCHES: + if named & tools and not tools <= named: + raise ValueError(f"codex turns {', '.join(sorted(tools))} on and off together: name all or none") + return self + + +def builtin_tools_overrides(builtin_tools: CodexTools | None) -> list[str]: + """The `-c` overrides that turn off the CLI's own tools a selection doesn't keep; none without one. The MCP + tools agent-env gives the agent stay with any selection.""" + if builtin_tools is None: + return [] + if builtin_tools.only is not None: + off = [tools for tools, _ in _TOOL_SWITCHES if not tools <= set(builtin_tools.only)] + elif builtin_tools.disable == "*": + off = [tools for tools, _ in _TOOL_SWITCHES] + else: + off = [tools for tools, _ in _TOOL_SWITCHES if tools <= set(builtin_tools.disable)] + return [override for tools, overrides in _TOOL_SWITCHES if tools in off for override in overrides] + def _config_path() -> Path: home = os.environ.get("CODEX_HOME") or str(Path.home() / ".codex") diff --git a/tst/unit/agents/codex/test_card_contract.py b/tst/unit/agents/codex/test_card_contract.py index f5904d1bd..8cff32ee2 100644 --- a/tst/unit/agents/codex/test_card_contract.py +++ b/tst/unit/agents/codex/test_card_contract.py @@ -105,7 +105,7 @@ def test_request_and_response_contracts(codex): methods = {uri: ext["params"].get("methods", {}) for uri, ext in _extensions(codex).items()} config = methods["urn:agentenv:agent-config/v1"] assert config["set"]["request"]["supported"] == [ - "description", "effort", "model", "name", "output_format", "project_id", + "builtin_tools", "description", "effort", "model", "name", "output_format", "project_id", "role", "system_prompt", "task_id", "timeout_seconds", ] assert set(config["set"]["request"]["schema"]["properties"]) == set(config["set"]["request"]["supported"]) diff --git a/tst/unit/agents/codex/test_codex_builtin_tools.py b/tst/unit/agents/codex/test_codex_builtin_tools.py new file mode 100644 index 000000000..798f72ae9 --- /dev/null +++ b/tst/unit/agents/codex/test_codex_builtin_tools.py @@ -0,0 +1,150 @@ +"""A selection of the CLI's own tools, as `-c` overrides on the turn's CLI: ``"*"`` turns off every setting that brings +one, the `multi_agent_v2` collaboration tools included, ``only`` every setting but those it keeps, and a list to turn off +those it names; config.toml and the MCP servers agent-env gives the agent stay as they are. With no selection, the CLI +gets the overrides it got before. The card's agent-config takes the field, so deploy_agent accepts a selection for +codex, and a tool the CLI doesn't have, or part of a setting's tools, is refused. The selection's type has the +protocol's schema, and its tool names are those of the CLI version package.json pins.""" + +import asyncio +import json +import os +import tomllib +from pathlib import Path +from typing import Any + +import pytest +from agentenv_protocol.a2a_agent import BuiltinTools + +from agent_env.a2a_agent import A2AAgent +from agent_env.task_step.task_steps.deploy_agent import _check_builtin_tools +from tst.unit.agents.codex.agent import AGENT_DIR, a2a_server, codex_bridge, codex_config + +_ALL_OFF = [ + "features.shell_tool=false", + "tools.update_plan.enabled=false", + "tools.experimental_request_user_input.enabled=false", + "features.view_image=false", + "features.multi_agent_v2=false", + "features.multi_agent=false", + "features.goals=false", + 'web_search="disabled"', +] + + +def _overrides(selection: dict | None) -> list[str]: + return codex_config.builtin_tools_overrides(a2a_server.CodexConfig(builtin_tools=selection).builtin_tools) + + +@pytest.mark.parametrize("selection, overrides", [ + (None, []), + ({"disable": "*"}, _ALL_OFF), + ({"only": []}, _ALL_OFF), + ({"disable": []}, []), + ({"disable": ["web_search", "view_image"]}, ["features.view_image=false", 'web_search="disabled"']), + ({"disable": ["spawn_agent", "send_message", "wait_agent", "followup_task", "interrupt_agent", "list_agents"]}, + ["features.multi_agent_v2=false", "features.multi_agent=false"]), + ({"only": ["exec_command", "write_stdin", "update_plan"]}, + [o for o in _ALL_OFF if o not in ("features.shell_tool=false", "tools.update_plan.enabled=false")]), +]) +def test_a_selection_is_the_overrides_that_turn_tools_off(selection, overrides): + assert _overrides(selection) == overrides + + +def test_turns_get_the_selections_overrides_after_their_role_and_config_toml_stays(codex, monkeypatch): + """No selection, with no role, is no override at all, as before; a selection adds its overrides after the role's, + and config.toml, whose MCP servers give the agent agent-env's tools, is the same either way.""" + codex_home = Path(os.environ["CODEX_HOME"]) + monkeypatch.setenv("A2A_PORT", "8000") + turns = [] + + async def fake_run_codex(**kwargs): + turns.append((tomllib.loads((codex_home / "config.toml").read_text()), list(kwargs["config_overrides"]))) + return codex_bridge.CodexResult(response="ok", tool_call_count=0, session_id="t") + + monkeypatch.setattr(a2a_server, "run_codex", fake_run_codex) + + async def exercise(): + async with codex.app.router.lifespan_context(codex.app), codex.client() as client: + await client.post("/ext/mcp-config", json={"url": "https://gw/mcp", "name": "env4821"}) + await codex.send(client, "one") + assert (await client.post("/ext/agent-config", json={"builtin_tools": {"disable": "*"}})).status_code == 200 + await codex.send(client, "two") + await client.post("/ext/agent-config", json={"role": "solver", "builtin_tools": {"disable": ["web_search"]}}) + await codex.send(client, "three") + + asyncio.run(exercise()) + + (before, none), (after, all_off), (_, solver) = turns + assert none == [] + assert all_off == _ALL_OFF + assert solver == [*codex_config.role_overrides({"env4821": {}, "peer_agents": {}}, "solver"), + 'web_search="disabled"'] + assert before == after and before["features"] == {"multi_agent_v2": True} + assert set(before["mcp_servers"]) == {"env4821", "peer_agents"} + + +def test_the_overrides_reach_the_cli_argv(monkeypatch, tmp_path): + """run_codex appends each override as its own `-c`, after the turn's own settings.""" + seen = [] + + class _Stop(Exception): + pass + + async def _exec(*cmd, **kwargs): + seen.append(list(cmd)) + raise _Stop + + monkeypatch.setattr(asyncio, "create_subprocess_exec", _exec) + with pytest.raises(_Stop): + asyncio.run(codex_bridge._run_codex_once(prompt="go", thread_id=None, config_overrides=_overrides( + {"disable": "*"}))) + + (argv,) = seen + assert argv[argv.index("--dangerously-bypass-approvals-and-sandbox") + 1:] == [ + arg for override in _ALL_OFF for arg in ("-c", override)] + + +@pytest.mark.parametrize("selection, match", [ + ({"only": ["apply_patch"]}, "apply_patch"), + ({"disable": ["tool_search"]}, "tool_search"), + ({"disable": ["exec_command"]}, "on and off together"), + ({"only": ["get_goal"]}, "on and off together"), + ({"disable": "*", "only": ["update_plan"]}, "exactly one"), + ({}, "exactly one"), +]) +def test_an_invalid_selection_is_refused(selection, match): + with pytest.raises(ValueError, match=match): + a2a_server.CodexConfig(builtin_tools=selection) + + +def test_the_card_takes_builtin_tools_so_deploy_agent_accepts_a_selection(codex): + extension = A2AAgent.find_extension(codex.card, A2AAgent.EXT_AGENT_CONFIG) + + assert "builtin_tools" in extension["params"]["methods"]["set"]["request"]["supported"] + _check_builtin_tools(extension, "codex", {"disable": "*"}) + _check_builtin_tools(extension, "codex", {"only": ["update_plan"]}) + with pytest.raises(RuntimeError, match="can't take builtin_tools"): + _check_builtin_tools(extension, "codex", {"disable": ["shell"]}) + + +def test_the_selection_has_the_protocols_schema(): + def untitled(schema: Any) -> Any: + if isinstance(schema, dict): + return {k: untitled(v) for k, v in schema.items() if k not in ("title", "description")} + return [untitled(v) for v in schema] if isinstance(schema, list) else schema + + assert untitled(codex_config.CodexTools.model_json_schema()) == untitled( + BuiltinTools[codex_config.CodexTool].model_json_schema()) + + +def test_every_tool_is_turned_off_by_one_setting(): + switched = [tool for tools, _ in codex_config._TOOL_SWITCHES for tool in tools] + + assert sorted(switched) == sorted(codex_config.CodexTool.__args__) + + +def test_the_tool_names_are_those_of_the_cli_version_package_json_pins(): + pinned = json.loads((AGENT_DIR / "package.json").read_text())["dependencies"]["@openai/codex"] + + assert codex_config.TOOLS_CLI_VERSION == pinned, ( + "the CLI version changed: record the tools the new CLI offers in CodexTool, then TOOLS_CLI_VERSION") From 3c45493d15f110d87359c04249e19f8381b6c8e4 Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 10:27:02 -0500 Subject: [PATCH 3/6] fix(agents): keep opencode's path and repeat checks allowed under a tool selection With {"disable": "*"} or {"only": [...]}, the permission's "*": "deny" also denied external_directory, so a kept read or edit could no longer reach a path outside the workspace, and doom_loop. Both stay allowed, as they are without a selection. A new agent-level test checks that the agent passes the stored selection to each turn. Co-Authored-By: Claude Opus 5.5 --- .../agents/agents/opencode/README.md | 4 +- .../agents/agents/opencode/opencode_bridge.py | 12 +++++- .../test_opencode_bridge_builtin_tools.py | 43 ++++++++++++++++--- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/agent_env/agents/agents/opencode/README.md b/src/agent_env/agents/agents/opencode/README.md index 8da5a1843..0e0d04c72 100644 --- a/src/agent_env/agents/agents/opencode/README.md +++ b/src/agent_env/agents/agents/opencode/README.md @@ -44,7 +44,9 @@ state is under `/data`, its `HOME`, whatever the uid. A `deploy_agent` `builtin_tools` selection picks the CLI's own tools through the config's `permission`, which leaves a denied tool out of the model's tool list rather than refusing it when called. `{"disable": "*"}` and `{"only": [...]}` deny every tool but those named and the tools of the MCP servers agent-env gives the agent (`_*`), a list to -turn off denies those alone, and `question` is denied in every case. The names are `OpenCodeTool`'s in +turn off denies those alone, and `question` is denied in every case. The CLI's checks that aren't tools stay +allowed, as they are without a selection: `external_directory`, which a file tool asks before it touches a path outside +the workspace, and `doom_loop`. The names are `OpenCodeTool`'s in `opencode_bridge.py`, the CLI's permission names: `edit` also covers `write` and `apply_patch`, which can't be selected alone, and `read` covers the MCP resource tools. A test fails when package.json's `opencode-ai` moves, until `OpenCodeTool` and `TOOLS_CLI_VERSION` move with it. diff --git a/src/agent_env/agents/agents/opencode/opencode_bridge.py b/src/agent_env/agents/agents/opencode/opencode_bridge.py index 9635cdec0..797a0f1d5 100644 --- a/src/agent_env/agents/agents/opencode/opencode_bridge.py +++ b/src/agent_env/agents/agents/opencode/opencode_bridge.py @@ -230,18 +230,28 @@ def _mcp_tool_prefix(server: str) -> str: return re.sub(r"[^a-zA-Z0-9_-]", "_", server) + "_" +# The permissions opencode-ai 1.17.18 asks that aren't the name of a tool, and that its own defaults set. +_NON_TOOL_PERMISSIONS = ("external_directory", "doom_loop") + + def _permission(builtin_tools: OpenCodeTools | None, mcp_servers: dict[str, dict] | None) -> dict[str, str]: """The config's ``permission``: which tools the CLI offers the model. A denied tool is left out of the model's tool list, not only refused when called. Without a selection every tool is allowed; ``{"disable": "*"}`` and ``{"only": [...]}`` deny every tool but those named and the tools of ``mcp_servers``, which a ``"*"`` deny would remove too; ``{"disable": [...]}`` denies those alone. ``question`` is denied in every case, and last, since the - CLI applies the last rule that matches a tool.""" + CLI applies the last rule that matches a tool. + + A ``"*"`` deny would also deny the CLI's checks that aren't tools: ``external_directory``, which every file tool + asks before it touches a path outside the run directory, and ``doom_loop``, which it asks when the model repeats + a call. Those stay allowed, as ``"*": "allow"`` leaves them, so a kept tool works as it does without a + selection.""" if builtin_tools is None: return {"*": "allow", "question": "deny"} if isinstance(builtin_tools.disable, list): return {"*": "allow", **{tool: "deny" for tool in builtin_tools.disable}, "question": "deny"} return { "*": "deny", + **{check: "allow" for check in _NON_TOOL_PERMISSIONS}, **{tool: "allow" for tool in builtin_tools.only or []}, **{f"{_mcp_tool_prefix(name)}*": "allow" for name in mcp_servers or {}}, "question": "deny", diff --git a/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py b/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py index 3bc92e4b2..2c3a4f0f4 100644 --- a/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py +++ b/tst/unit/agents/opencode/test_opencode_bridge_builtin_tools.py @@ -14,10 +14,14 @@ from agent_env.a2a_agent import A2AAgent from agent_env.task_step.task_steps.deploy_agent import _check_builtin_tools from tst.unit.agents.opencode.agent import AGENT_DIR, a2a_server, opencode_bridge +from tst.unit.agents.opencode.conftest import result from tst.unit.agents.opencode.fake_cli import FakeCLI MODEL = "fireworks_ai/kimi-k3" _STOP = {"type": "step_finish", "sessionID": "ses_1", "part": {"type": "step-finish", "reason": "stop"}} +# What a "*" deny would otherwise deny that isn't a tool: the check a file tool makes before it touches a path outside +# the run directory, and the one made when the model repeats a call. +_CHECKS = {"external_directory": "allow", "doom_loop": "allow"} _SERVERS = {"env4821": {"url": "https://gw/mcp", "headers": None}, "peer_agents": {"url": "http://127.0.0.1:8000/ext/peer-agents/mcp", "headers": None}, "fake-env.x": {"url": "https://gw2/mcp", "headers": None}} @@ -70,11 +74,11 @@ def test_no_selection_leaves_the_config_and_argv_as_they_were(monkeypatch, spawn @pytest.mark.parametrize("selection, permission", [ - ({"disable": "*"}, {"*": "deny", "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", + ({"disable": "*"}, {"*": "deny", **_CHECKS, "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", "question": "deny"}), - ({"only": ["read", "grep"]}, {"*": "deny", "read": "allow", "grep": "allow", "env4821_*": "allow", + ({"only": ["read", "grep"]}, {"*": "deny", **_CHECKS, "read": "allow", "grep": "allow", "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", "question": "deny"}), - ({"only": []}, {"*": "deny", "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", + ({"only": []}, {"*": "deny", **_CHECKS, "env4821_*": "allow", "peer_agents_*": "allow", "fake-env_x_*": "allow", "question": "deny"}), ({"disable": ["bash", "webfetch"]}, {"*": "allow", "bash": "deny", "webfetch": "deny", "question": "deny"}), ]) @@ -86,7 +90,7 @@ def test_a_selection_is_the_permission_and_agent_envs_mcp_tools_stay(selection, def test_without_mcp_servers_disable_all_denies_every_tool(): - assert _permission({"disable": "*"}) == {"*": "deny", "question": "deny"} + assert _permission({"disable": "*"}) == {"*": "deny", **_CHECKS, "question": "deny"} def test_the_selection_reaches_the_cli(spawned): @@ -95,10 +99,39 @@ def test_the_selection_reaches_the_cli(spawned): asyncio.run(opencode_bridge.run_opencode(prompt="go", context_id="ctx", builtin_tools=tools)) ((argv, env),) = spawned - assert json.loads(env["OPENCODE_CONFIG_CONTENT"])["permission"] == {"*": "deny", "question": "deny"} + assert json.loads(env["OPENCODE_CONFIG_CONTENT"])["permission"] == {"*": "deny", **_CHECKS, "question": "deny"} assert argv[-1] == "go" +def test_turns_get_the_selection_the_agent_config_set(monkeypatch, oc): + """The agent passes the stored selection to each turn's run_opencode: none before one is set, then the one set, + whose permission keeps the tools of the MCP servers registered with the agent.""" + monkeypatch.setenv("A2A_PORT", "8000") + permissions = [] + + async def fake_run_opencode(**kwargs): + config = opencode_bridge._build_config(MODEL, kwargs["mcp_servers"], {}, None, + builtin_tools=kwargs.get("builtin_tools")) + permissions.append(config["permission"]) + return result() + + monkeypatch.setattr(a2a_server, "run_opencode", fake_run_opencode) + + async def exercise(): + async with oc.client() as client: + await client.post("/ext/mcp-config", json={"name": "env4821", "url": "https://gw/mcp"}) + await oc.send(client, "one") + assert (await client.post("/ext/agent-config", json={"builtin_tools": {"disable": "*"}})).status_code == 200 + await oc.send(client, "two") + + asyncio.run(exercise()) + + assert permissions == [ + {"*": "allow", "question": "deny"}, + {"*": "deny", **_CHECKS, "env4821_*": "allow", "peer_agents_*": "allow", "question": "deny"}, + ] + + @pytest.mark.parametrize("selection, match", [ ({"only": ["write"]}, "write"), ({"disable": ["question"]}, "question"), From 5ebe6dc98758db2f9557d13080ada6f94dfa2a58 Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 10:46:56 -0500 Subject: [PATCH 4/6] feat(agents): turn off codex's apply_patch with a catalog copy when a selection drops it No codex 0.147.0 setting removes apply_patch: the model catalog offers it to every bundled GPT model through apply_patch_tool_type. When a builtin_tools selection drops it (disable "*", a disable list naming it, or an only list without it), the agent now writes a copy of the catalog the CLI would use, from `codex debug models --bundled` or the file config.toml's model_catalog_json names, with apply_patch_tool_type null for every model, under CODEX_HOME, and points the turn's CLI at it with -c model_catalog_json. apply_patch joins CodexTool. With no selection, or one that keeps apply_patch, the overrides and config.toml are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/agents/agents/codex/README.md | 16 ++- .../agents/agents/codex/a2a_server.py | 3 +- .../agents/agents/codex/codex_config.py | 72 ++++++++++-- .../agents/codex/test_codex_builtin_tools.py | 109 ++++++++++++++++-- 4 files changed, 173 insertions(+), 27 deletions(-) diff --git a/src/agent_env/agents/agents/codex/README.md b/src/agent_env/agents/agents/codex/README.md index dde0dfe9e..118cc8806 100644 --- a/src/agent_env/agents/agents/codex/README.md +++ b/src/agent_env/agents/agents/codex/README.md @@ -53,9 +53,13 @@ tools it names, and a list to turn off those whose tools it names. A list names | `create_goal`, `get_goal`, `update_goal` | `features.goals=false` | | `web_search` | `web_search="disabled"` | -Codex 0.147.0 can't turn off `apply_patch` with a setting: a model's entry in its catalog turns it on (every GPT model it -ships with does), so `{"disable": "*"}` still leaves it for those models, and it isn't a name a selection takes. The MCP -tools agent-env gives the agent stay with any selection, and so do the tools Codex adds for them: `tool_search`, -`list_mcp_resources`, `list_mcp_resource_templates` and `read_mcp_resource`. The names are `CodexTool`'s in -`codex_config.py`; a test fails when package.json's `@openai/codex` moves, until `CodexTool` and `TOOLS_CLI_VERSION` -move with it. +No setting turns off `apply_patch`: a model's entry in Codex's model catalog offers it (every GPT model 0.147.0 ships +with does). So a selection without it (`{"disable": "*"}`, a list naming it, or an `{"only": [...]}` that doesn't) +writes a copy of the catalog the CLI would use, with `apply_patch_tool_type` null for every model, to +`$CODEX_HOME/model_catalog_without_apply_patch.json`, and points the task's CLI at it with `-c model_catalog_json=...`. +The copy is made from `codex debug models --bundled`, or from the file config.toml's own `model_catalog_json` names. + +The MCP tools agent-env gives the agent stay with any selection, and so do the tools Codex adds for them: +`tool_search`, `list_mcp_resources`, `list_mcp_resource_templates` and `read_mcp_resource`. The names are `CodexTool`'s +in `codex_config.py`; a test fails when package.json's `@openai/codex` moves, until `CodexTool` and +`TOOLS_CLI_VERSION` move with it. diff --git a/src/agent_env/agents/agents/codex/a2a_server.py b/src/agent_env/agents/agents/codex/a2a_server.py index e34e15ab2..c991fab61 100644 --- a/src/agent_env/agents/agents/codex/a2a_server.py +++ b/src/agent_env/agents/agents/codex/a2a_server.py @@ -682,7 +682,8 @@ def _record_event(event: dict) -> None: task_id=cfg.task_id, event_callback=_record_event, config_overrides=[*codex_config.role_overrides(request.mcp_servers, cfg.role), - *codex_config.builtin_tools_overrides(cfg.builtin_tools)], + *codex_config.builtin_tools_overrides(cfg.builtin_tools), + *await codex_config.catalog_overrides(cfg.builtin_tools)], ) except Exception as e: logger.error(f"codex CLI invocation raised: {e}", exc_info=True) diff --git a/src/agent_env/agents/agents/codex/codex_config.py b/src/agent_env/agents/agents/codex/codex_config.py index 2bea330dd..fdd7597c4 100644 --- a/src/agent_env/agents/agents/codex/codex_config.py +++ b/src/agent_env/agents/agents/codex/codex_config.py @@ -32,13 +32,14 @@ # The CLI version CodexTool names the tools of; a test holds it to package.json's @openai/codex. TOOLS_CLI_VERSION = "0.147.0" -# The CLI's own tools, by the names its model sees, that a `-c` override turns off. Some only go together, as one +# The CLI's own tools, by the names its model sees, that a selection can turn off. Some only go together, as one # setting turns them on: the shell's two, the six collaboration tools `multi_agent_v2` brings, and the three goal tools. -# Left out: `apply_patch`, which a model's catalog entry turns on and no setting turns off, and the MCP tools -# (`tool_search`, `list_mcp_resources`, `list_mcp_resource_templates`, `read_mcp_resource`) that come with MCP servers. -CodexTool = Literal["create_goal", "exec_command", "followup_task", "get_goal", "interrupt_agent", "list_agents", - "request_user_input", "send_message", "spawn_agent", "update_goal", "update_plan", "view_image", - "wait_agent", "web_search", "write_stdin"] +# `apply_patch` is turned off through the model catalog (`catalog_overrides`), the others by `_TOOL_SWITCHES`. Left out: +# the MCP tools (`tool_search`, `list_mcp_resources`, `list_mcp_resource_templates`, `read_mcp_resource`) that come +# with MCP servers. +CodexTool = Literal["apply_patch", "create_goal", "exec_command", "followup_task", "get_goal", "interrupt_agent", + "list_agents", "request_user_input", "send_message", "spawn_agent", "update_goal", "update_plan", + "view_image", "wait_agent", "web_search", "write_stdin"] # Each setting that turns some of the CLI's tools off: those tools, and the `-c` overrides that turn them off. Turning # off `multi_agent_v2` alone would bring back the v1 collaboration tools behind `tool_search`, so both go. _TOOL_SWITCHES: tuple[tuple[frozenset[str], tuple[str, ...]], ...] = ( @@ -87,6 +88,56 @@ def builtin_tools_overrides(builtin_tools: CodexTools | None) -> list[str]: return [override for tools, overrides in _TOOL_SWITCHES if tools in off for override in overrides] +# No setting turns off `apply_patch`: a model's entry in the CLI's model catalog offers it when its +# `apply_patch_tool_type` is set, as it is for every GPT model 0.147.0 ships with. A selection without it points the CLI, +# with `model_catalog_json`, at a copy of the catalog it would use whose models all have the field null. The copy is +# made from the installed CLI's own catalog each time, so it follows the pinned version. +CATALOG_WITHOUT_APPLY_PATCH = "model_catalog_without_apply_patch.json" +_CATALOG_TIMEOUT_S = 60 + + +def removes_apply_patch(builtin_tools: CodexTools | None) -> bool: + """Whether a selection turns `apply_patch` off; never without one.""" + if builtin_tools is None: + return False + if builtin_tools.only is not None: + return "apply_patch" not in builtin_tools.only + return builtin_tools.disable == "*" or "apply_patch" in builtin_tools.disable + + +async def _catalog() -> dict[str, Any]: + """The model catalog the CLI uses: the file config.toml's `model_catalog_json` names (relative to CODEX_HOME, as + the CLI reads it), or else the one the installed CLI ships with, from `codex debug models --bundled`.""" + configured = _read().get("model_catalog_json") + if configured: + return json.loads((_config_path().parent / Path(configured).expanduser()).read_text()) + process = await asyncio.create_subprocess_exec( + "codex", "debug", "models", "--bundled", stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE) + try: + stdout, stderr = await asyncio.wait_for(process.communicate(), _CATALOG_TIMEOUT_S) + except TimeoutError: + process.kill() + await process.wait() + raise RuntimeError(f"codex debug models gave no catalog within {_CATALOG_TIMEOUT_S}s") from None + if process.returncode != 0: + raise RuntimeError(f"codex debug models exited {process.returncode}: {stderr.decode(errors='replace')[-500:]}") + return json.loads(stdout) + + +async def catalog_overrides(builtin_tools: CodexTools | None) -> list[str]: + """The `-c` override that points the CLI at a catalog copy without `apply_patch`, when the selection turns it off; + none otherwise. Raises when the catalog can't be read, so a turn never gets the tool its selection turned off.""" + if not removes_apply_patch(builtin_tools): + return [] + catalog = await _catalog() + for model in catalog["models"]: + model["apply_patch_tool_type"] = None + path = _config_path().with_name(CATALOG_WITHOUT_APPLY_PATCH) + async with _write_lock: + _replace(path, json.dumps(catalog, ensure_ascii=False)) + return [f"model_catalog_json={json.dumps(str(path))}"] + + def _config_path() -> Path: home = os.environ.get("CODEX_HOME") or str(Path.home() / ".codex") return Path(home) / "config.toml" @@ -103,12 +154,15 @@ def _write(cfg: dict[str, Any]) -> bool: """Replace config.toml whole, never truncating it in place: a codex CLI starting in another context reads it at launch. Unchanged content is left alone. Returns whether the file was written.""" - path = _config_path() + return _replace(_config_path(), tomli_w.dumps(cfg)) + + +def _replace(path: Path, text: str) -> bool: + """Replace a file under CODEX_HOME whole, unless its content is already ``text``; see `_write`.""" path.parent.mkdir(parents=True, exist_ok=True) - text = tomli_w.dumps(cfg) if path.exists() and path.read_text() == text: return False - # It carries MCP and model headers: keep the existing file's permissions (owner-only when new). + # config.toml carries MCP and model headers: keep the existing file's permissions (owner-only when new). mode = stat.S_IMODE(path.stat().st_mode) if path.exists() else 0o600 staged = path.with_name(f".{path.name}.{os.getpid()}.tmp") with os.fdopen(os.open(staged, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode), "w") as handle: diff --git a/tst/unit/agents/codex/test_codex_builtin_tools.py b/tst/unit/agents/codex/test_codex_builtin_tools.py index 798f72ae9..02c1fbace 100644 --- a/tst/unit/agents/codex/test_codex_builtin_tools.py +++ b/tst/unit/agents/codex/test_codex_builtin_tools.py @@ -1,7 +1,8 @@ """A selection of the CLI's own tools, as `-c` overrides on the turn's CLI: ``"*"`` turns off every setting that brings one, the `multi_agent_v2` collaboration tools included, ``only`` every setting but those it keeps, and a list to turn off -those it names; config.toml and the MCP servers agent-env gives the agent stay as they are. With no selection, the CLI -gets the overrides it got before. The card's agent-config takes the field, so deploy_agent accepts a selection for +those it names; config.toml and the MCP servers agent-env gives the agent stay as they are. A selection without +`apply_patch`, which only the model catalog turns on, also points the CLI at a copy of its catalog with the tool off. With +no selection, the CLI gets the overrides it got before. The card's agent-config takes the field, so deploy_agent accepts a selection for codex, and a tool the CLI doesn't have, or part of a setting's tools, is refused. The selection's type has the protocol's schema, and its tool names are those of the CLI version package.json pins.""" @@ -31,10 +32,43 @@ ] +_CATALOG = {"models": [ + {"slug": "gpt-5.4", "apply_patch_tool_type": "freeform", "context_window": 272000}, + {"slug": "gpt-5.6-terra", "apply_patch_tool_type": "freeform", "shell_type": "shell_command"}, +]} + + def _overrides(selection: dict | None) -> list[str]: return codex_config.builtin_tools_overrides(a2a_server.CodexConfig(builtin_tools=selection).builtin_tools) +def _catalog_overrides(selection: dict | None) -> list[str]: + return asyncio.run(codex_config.catalog_overrides(a2a_server.CodexConfig(builtin_tools=selection).builtin_tools)) + + +@pytest.fixture +def cli(monkeypatch, tmp_path) -> Path: + """A `codex` on PATH whose `debug models` prints `_CATALOG` and records its arguments in the returned file, under a + CODEX_HOME of the test's own.""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path / "codex-home")) + bin_dir, calls = tmp_path / "bin", tmp_path / "codex-calls" + bin_dir.mkdir() + (tmp_path / "catalog.json").write_text(json.dumps(_CATALOG)) + script = bin_dir / "codex" + script.write_text(f'#!/bin/sh\necho "$*" >> {calls}\ncat {tmp_path / "catalog.json"}\n') + script.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") + return calls + + +def _catalog_file() -> Path: + return Path(os.environ["CODEX_HOME"]) / codex_config.CATALOG_WITHOUT_APPLY_PATCH + + +def _catalog_override() -> str: + return f"model_catalog_json={json.dumps(str(_catalog_file()))}" + + @pytest.mark.parametrize("selection, overrides", [ (None, []), ({"disable": "*"}, _ALL_OFF), @@ -50,15 +84,17 @@ def test_a_selection_is_the_overrides_that_turn_tools_off(selection, overrides): assert _overrides(selection) == overrides -def test_turns_get_the_selections_overrides_after_their_role_and_config_toml_stays(codex, monkeypatch): - """No selection, with no role, is no override at all, as before; a selection adds its overrides after the role's, - and config.toml, whose MCP servers give the agent agent-env's tools, is the same either way.""" +def test_turns_get_the_selections_overrides_after_their_role_and_config_toml_stays(codex, cli, monkeypatch): + """No selection, with no role, is no override at all, as before, and no catalog copy; a selection adds its + overrides after the role's, and config.toml, whose MCP servers give the agent agent-env's tools, is the same either + way.""" codex_home = Path(os.environ["CODEX_HOME"]) monkeypatch.setenv("A2A_PORT", "8000") turns = [] async def fake_run_codex(**kwargs): - turns.append((tomllib.loads((codex_home / "config.toml").read_text()), list(kwargs["config_overrides"]))) + turns.append((tomllib.loads((codex_home / "config.toml").read_text()), list(kwargs["config_overrides"]), + _catalog_file().exists())) return codex_bridge.CodexResult(response="ok", tool_call_count=0, session_id="t") monkeypatch.setattr(a2a_server, "run_codex", fake_run_codex) @@ -74,9 +110,9 @@ async def exercise(): asyncio.run(exercise()) - (before, none), (after, all_off), (_, solver) = turns - assert none == [] - assert all_off == _ALL_OFF + (before, none, catalog_before), (after, all_off, _), (_, solver, _) = turns + assert none == [] and not catalog_before + assert all_off == [*_ALL_OFF, _catalog_override()] assert solver == [*codex_config.role_overrides({"env4821": {}, "peer_agents": {}}, "solver"), 'web_search="disabled"'] assert before == after and before["features"] == {"multi_agent_v2": True} @@ -104,8 +140,57 @@ async def _exec(*cmd, **kwargs): arg for override in _ALL_OFF for arg in ("-c", override)] +@pytest.mark.parametrize("selection, removes", [ + (None, False), + ({"disable": []}, False), + ({"disable": ["web_search"]}, False), + ({"only": ["apply_patch"]}, False), + ({"only": ["apply_patch", "update_plan"]}, False), + ({"disable": "*"}, True), + ({"disable": ["apply_patch"]}, True), + ({"only": []}, True), + ({"only": ["update_plan"]}, True), +]) +def test_a_selection_without_apply_patch_points_the_cli_at_a_catalog_without_it(cli, selection, removes): + """The copy is the installed CLI's bundled catalog with `apply_patch_tool_type` null for every model and nothing + else changed; a selection that keeps the tool, or none, runs nothing and writes nothing.""" + overrides = _catalog_overrides(selection) + + if not removes: + assert overrides == [] and not _catalog_file().exists() and not cli.exists() + return + assert overrides == [_catalog_override()] + assert cli.read_text().split() == ["debug", "models", "--bundled"] + assert json.loads(_catalog_file().read_text()) == {"models": [ + {**model, "apply_patch_tool_type": None} for model in _CATALOG["models"]]} + + +def test_a_catalog_config_toml_names_is_the_one_copied(cli): + """A `model_catalog_json` already in config.toml, relative to CODEX_HOME as the CLI reads it, is the catalog the CLI + would use, so its copy is the one without `apply_patch`; the bundled one isn't read. config.toml stays.""" + codex_home = Path(os.environ["CODEX_HOME"]) + codex_home.mkdir() + mine = {"models": [{"slug": "house-model", "apply_patch_tool_type": "function", "priority": 1}]} + (codex_home / "house.json").write_text(json.dumps(mine)) + (codex_home / "config.toml").write_text('model_catalog_json = "house.json"\n') + + assert _catalog_overrides({"disable": "*"}) == [_catalog_override()] + assert json.loads(_catalog_file().read_text()) == {"models": [ + {"slug": "house-model", "apply_patch_tool_type": None, "priority": 1}]} + assert not cli.exists() + assert (codex_home / "config.toml").read_text() == 'model_catalog_json = "house.json"\n' + + +def test_a_catalog_the_cli_cant_give_fails_the_selection(cli, tmp_path): + """Without a catalog, the turn would get the `apply_patch` its selection turned off: it raises instead.""" + (tmp_path / "bin" / "codex").write_text("#!/bin/sh\necho 'no catalog' >&2\nexit 3\n") + + with pytest.raises(RuntimeError, match="exited 3: no catalog"): + _catalog_overrides({"disable": "*"}) + + @pytest.mark.parametrize("selection, match", [ - ({"only": ["apply_patch"]}, "apply_patch"), + ({"only": ["shell"]}, "shell"), ({"disable": ["tool_search"]}, "tool_search"), ({"disable": ["exec_command"]}, "on and off together"), ({"only": ["get_goal"]}, "on and off together"), @@ -123,6 +208,7 @@ def test_the_card_takes_builtin_tools_so_deploy_agent_accepts_a_selection(codex) assert "builtin_tools" in extension["params"]["methods"]["set"]["request"]["supported"] _check_builtin_tools(extension, "codex", {"disable": "*"}) _check_builtin_tools(extension, "codex", {"only": ["update_plan"]}) + _check_builtin_tools(extension, "codex", {"disable": ["apply_patch"]}) with pytest.raises(RuntimeError, match="can't take builtin_tools"): _check_builtin_tools(extension, "codex", {"disable": ["shell"]}) @@ -138,9 +224,10 @@ def untitled(schema: Any) -> Any: def test_every_tool_is_turned_off_by_one_setting(): + """`apply_patch` by the catalog copy, every other tool by one `-c` setting.""" switched = [tool for tools, _ in codex_config._TOOL_SWITCHES for tool in tools] - assert sorted(switched) == sorted(codex_config.CodexTool.__args__) + assert sorted([*switched, "apply_patch"]) == sorted(codex_config.CodexTool.__args__) def test_the_tool_names_are_those_of_the_cli_version_package_json_pins(): From 3f05a09ade97f4e38a849b45492cc5e104fcd53f Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 11:26:46 -0500 Subject: [PATCH 5/6] fix(agents): bound codex's catalog read by the turn's time and kill it on cancel The catalog read now counts against timeout_seconds, run_codex gets what is left, and a timed-out or cancelled read kills its `codex debug models` child. Names the stderr tail limit. Co-Authored-By: Claude Opus 5.5 --- .../agents/agents/codex/a2a_server.py | 14 +++- .../agents/agents/codex/codex_config.py | 25 ++++--- .../agents/codex/test_codex_builtin_tools.py | 74 +++++++++++++++++++ 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/src/agent_env/agents/agents/codex/a2a_server.py b/src/agent_env/agents/agents/codex/a2a_server.py index c991fab61..e9bac0983 100644 --- a/src/agent_env/agents/agents/codex/a2a_server.py +++ b/src/agent_env/agents/agents/codex/a2a_server.py @@ -19,6 +19,7 @@ import shutil import tarfile import tempfile +import time import uuid from collections.abc import Awaitable, Callable from contextlib import asynccontextmanager, suppress @@ -668,6 +669,15 @@ def _record_event(event: dict) -> None: try: await codex_config.write_mcp_servers(request.mcp_servers) + # The catalog read a selection may need counts against the turn's time limit. + started = time.monotonic() + catalog = await codex_config.catalog_overrides(cfg.builtin_tools, cfg.timeout_seconds) + remaining_s = cfg.timeout_seconds + if catalog: + remaining_s -= time.monotonic() - started + if remaining_s <= 0: + raise RuntimeError( + f"no time left of the turn's {cfg.timeout_seconds}s after the catalog read") async with _workspace_barrier.shared(): result = await run_codex( prompt=prompt, @@ -676,14 +686,14 @@ def _record_event(event: dict) -> None: effort=cfg.effort, system_prompt=cfg.system_prompt, output_format=cfg.output_format, - timeout_s=cfg.timeout_seconds, + timeout_s=remaining_s, # Forward LiteLLM attribution per subprocess. project_id=cfg.project_id, task_id=cfg.task_id, event_callback=_record_event, config_overrides=[*codex_config.role_overrides(request.mcp_servers, cfg.role), *codex_config.builtin_tools_overrides(cfg.builtin_tools), - *await codex_config.catalog_overrides(cfg.builtin_tools)], + *catalog], ) except Exception as e: logger.error(f"codex CLI invocation raised: {e}", exc_info=True) diff --git a/src/agent_env/agents/agents/codex/codex_config.py b/src/agent_env/agents/agents/codex/codex_config.py index fdd7597c4..ebb78a100 100644 --- a/src/agent_env/agents/agents/codex/codex_config.py +++ b/src/agent_env/agents/agents/codex/codex_config.py @@ -94,6 +94,8 @@ def builtin_tools_overrides(builtin_tools: CodexTools | None) -> list[str]: # made from the installed CLI's own catalog each time, so it follows the pinned version. CATALOG_WITHOUT_APPLY_PATCH = "model_catalog_without_apply_patch.json" _CATALOG_TIMEOUT_S = 60 +# How much of `codex debug models`' stderr a failed catalog read keeps in its error. +_CATALOG_STDERR_CHARS = 500 def removes_apply_patch(builtin_tools: CodexTools | None) -> bool: @@ -105,7 +107,7 @@ def removes_apply_patch(builtin_tools: CodexTools | None) -> bool: return builtin_tools.disable == "*" or "apply_patch" in builtin_tools.disable -async def _catalog() -> dict[str, Any]: +async def _catalog(timeout_s: float) -> dict[str, Any]: """The model catalog the CLI uses: the file config.toml's `model_catalog_json` names (relative to CODEX_HOME, as the CLI reads it), or else the one the installed CLI ships with, from `codex debug models --bundled`.""" configured = _read().get("model_catalog_json") @@ -114,22 +116,27 @@ async def _catalog() -> dict[str, Any]: process = await asyncio.create_subprocess_exec( "codex", "debug", "models", "--bundled", stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE) try: - stdout, stderr = await asyncio.wait_for(process.communicate(), _CATALOG_TIMEOUT_S) - except TimeoutError: - process.kill() - await process.wait() - raise RuntimeError(f"codex debug models gave no catalog within {_CATALOG_TIMEOUT_S}s") from None + stdout, stderr = await asyncio.wait_for(process.communicate(), timeout_s) + except (TimeoutError, asyncio.CancelledError) as e: + # A turn cancelled or out of time mid-read must not leave the child running. + if process.returncode is None: + process.kill() + await asyncio.shield(process.wait()) + if isinstance(e, asyncio.CancelledError): + raise + raise RuntimeError(f"codex debug models gave no catalog within {timeout_s:.0f}s") from None if process.returncode != 0: - raise RuntimeError(f"codex debug models exited {process.returncode}: {stderr.decode(errors='replace')[-500:]}") + tail = stderr.decode(errors="replace")[-_CATALOG_STDERR_CHARS:] + raise RuntimeError(f"codex debug models exited {process.returncode}: {tail}") return json.loads(stdout) -async def catalog_overrides(builtin_tools: CodexTools | None) -> list[str]: +async def catalog_overrides(builtin_tools: CodexTools | None, timeout_s: float = _CATALOG_TIMEOUT_S) -> list[str]: """The `-c` override that points the CLI at a catalog copy without `apply_patch`, when the selection turns it off; none otherwise. Raises when the catalog can't be read, so a turn never gets the tool its selection turned off.""" if not removes_apply_patch(builtin_tools): return [] - catalog = await _catalog() + catalog = await _catalog(min(timeout_s, _CATALOG_TIMEOUT_S)) for model in catalog["models"]: model["apply_patch_tool_type"] = None path = _config_path().with_name(CATALOG_WITHOUT_APPLY_PATCH) diff --git a/tst/unit/agents/codex/test_codex_builtin_tools.py b/tst/unit/agents/codex/test_codex_builtin_tools.py index 02c1fbace..755f5512c 100644 --- a/tst/unit/agents/codex/test_codex_builtin_tools.py +++ b/tst/unit/agents/codex/test_codex_builtin_tools.py @@ -189,6 +189,80 @@ def test_a_catalog_the_cli_cant_give_fails_the_selection(cli, tmp_path): _catalog_overrides({"disable": "*"}) +def _hanging_cli(tmp_path) -> Path: + """Turn the fixture's `codex` into one that records its pid, then never prints a catalog.""" + pid_file = tmp_path / "codex-pid" + (tmp_path / "bin" / "codex").write_text(f"#!/bin/sh\necho $$ > {pid_file}\nexec sleep 60\n") + return pid_file + + +def _alive(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + return True + + +def test_a_catalog_read_past_its_time_is_killed_and_fails_the_selection(cli, tmp_path): + """The read gets only the time it's given, and its child doesn't outlive it.""" + pid_file = _hanging_cli(tmp_path) + tools = a2a_server.CodexConfig(builtin_tools={"disable": "*"}).builtin_tools + + with pytest.raises(RuntimeError, match="no catalog within 1s"): + asyncio.run(codex_config.catalog_overrides(tools, timeout_s=1)) + assert not _alive(int(pid_file.read_text())) + + +def test_a_cancelled_catalog_read_kills_its_child(cli, tmp_path): + pid_file = _hanging_cli(tmp_path) + tools = a2a_server.CodexConfig(builtin_tools={"disable": "*"}).builtin_tools + + async def cancel_mid_read(): + task = asyncio.create_task(codex_config.catalog_overrides(tools)) + while not pid_file.exists(): + await asyncio.sleep(0.05) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + asyncio.run(cancel_mid_read()) + assert not _alive(int(pid_file.read_text())) + + +def test_the_catalog_read_counts_against_the_turns_time_limit(codex, cli, monkeypatch): + """The CLI gets what is left of `timeout_seconds` after the catalog read; without a read, all of it, as before.""" + monkeypatch.setenv("A2A_PORT", "8000") + timeouts = [] + real_catalog_overrides = codex_config.catalog_overrides + + async def slow_catalog_overrides(builtin_tools, timeout_s): + overrides = await real_catalog_overrides(builtin_tools, timeout_s) + if overrides: + await asyncio.sleep(0.5) + return overrides + + async def fake_run_codex(**kwargs): + timeouts.append(kwargs["timeout_s"]) + return codex_bridge.CodexResult(response="ok", tool_call_count=0, session_id="t") + + monkeypatch.setattr(codex_config, "catalog_overrides", slow_catalog_overrides) + monkeypatch.setattr(a2a_server, "run_codex", fake_run_codex) + + async def exercise(): + async with codex.app.router.lifespan_context(codex.app), codex.client() as client: + await client.post("/ext/agent-config", json={"timeout_seconds": 30}) + await codex.send(client, "one") + await client.post("/ext/agent-config", json={"builtin_tools": {"disable": "*"}}) + await codex.send(client, "two") + + asyncio.run(exercise()) + + without_read, with_read = timeouts + assert without_read == 30 + assert 29 < with_read <= 29.5 + + @pytest.mark.parametrize("selection, match", [ ({"only": ["shell"]}, "shell"), ({"disable": ["tool_search"]}, "tool_search"), From 8f0df60500b2a6a5b11404b8552dae376d6da76e Mon Sep 17 00:00:00 2001 From: Aidan Sims Date: Tue, 3 Nov 2026 13:04:09 -0500 Subject: [PATCH 6/6] ci: retrigger Integration Tests Co-Authored-By: Claude Opus 5.5