diff --git a/docs/acp.md b/docs/acp.md index 45493ea97..bec648541 100644 --- a/docs/acp.md +++ b/docs/acp.md @@ -12,7 +12,7 @@ Two-layer pattern (mirrors A2A Gateway): | Platform Bridge | `BandACPServerAdapter` | `ACPClientAdapter` | **Server**: Editor -> ACP -> `ACPServer` -> `BandACPServerAdapter` -> Band REST/WS -> Peers -**Client**: Band room message -> `ACPClientAdapter` -> stdio subprocess **or** TCP connection (Codex, Claude Code, Cursor, GitHub Copilot, etc.) +**Client**: Band room message -> `ACPClientAdapter` -> its room-owned stdio subprocess (Codex, Claude Code, Cursor, GitHub Copilot, etc.) ## Key Files @@ -20,7 +20,7 @@ Two-layer pattern (mirrors A2A Gateway): |------|---------| | `src/band/integrations/acp/server.py` | `ACPServer` — handles ACP JSON-RPC methods, does not subclass `acp.Agent`; `run_acp_server` — runs it with `use_unstable_protocol` (required for `session/fork`, `session/resume`, `session/close`) | | `src/band/integrations/acp/server_adapter.py` | `BandACPServerAdapter` — REST client, room/session mapping | -| `src/band/integrations/acp/client_adapter.py` | `ACPClientAdapter` — drives a remote ACP agent over stdio-spawn or TCP-connect | +| `src/band/integrations/acp/client_adapter.py` | `ACPClientAdapter` — drives a room-owned ACP agent over stdio | | `src/band/integrations/acp/client_runtime.py` | `ACPRuntime` (transport-agnostic) + `ACPCollectingClient` (session_update parsing / coalescing / collapse / live sink), `tcp_spawn_process` (TCP connect seam) | | `src/band/integrations/acp/room_emitter.py` | `RoomTurnEmitter` — posts a turn's chunks to the room in causal order; `turn_replied_in_room` (text-fallback suppression) | | `src/band/adapters/copilot_acp.py` | `CopilotACPAdapter` — thin `ACPClientAdapter` for the GitHub Copilot CLI | @@ -108,21 +108,16 @@ acp = ["agent-client-protocol"] Install with: `pip install band-sdk[acp]` or `uv add band-sdk[acp]` -## Client transports (stdio / TCP) - -`ACPClientAdapter` selects a transport at construction; both flow through `ACPRuntime`'s -injectable `spawn_process` seam, so the runtime and downstream code are transport-agnostic. - -- **stdio** (default): pass `command=[...]` to spawn the agent as a subprocess - (`acp.spawn_agent_process`). -- **TCP**: pass `host=` + `port=` to connect to an already-running ACP server - (`tcp_spawn_process` → `asyncio.open_connection` → `acp.connect_to_agent`). Use for an - ACP agent in a remote/containerized environment. -- Exactly one of `{command, (host, port)}` is required (validated in `__init__`). -- Advanced: inject a custom `spawn_process` (e.g. `docker exec -i … copilot --acp`, ssh, - or a fake in tests). Tests inject a fake through this seam rather than patching module - globals (see `tests/integrations/acp/conftest.py::FakeSpawn` / the `make_acp_transport` - fixture). +## Client workspace isolation + +`ACPClientAdapter` creates an isolated `./.band-workspaces/` directory for +each Band room by default. Pass `workspace_for_room` only to select a different +absolute workspace policy. It lazily starts one stdio agent process per room and +stops that process when the room is cleaned up. TCP and custom transport injection are +rejected because they cannot prove that a remote process belongs to only one room. +The assigned working directory is not an operating-system sandbox; configure the agent's +sandbox policy separately when that boundary is required. A custom resolver must assign a +different workspace to every live room, and the adapter requires a non-empty stdio command. ## GitHub Copilot CLI backend diff --git a/docs/adapters/codex.md b/docs/adapters/codex.md index 0b02d57ab..a8db7ce64 100644 --- a/docs/adapters/codex.md +++ b/docs/adapters/codex.md @@ -1,6 +1,6 @@ # Codex Adapter -[OpenAI Codex](https://openai.com/codex) is a coding agent runtime that can inspect files, edit files, run commands, and manage approval workflows. The Band Codex adapter connects a Codex process to Band rooms over stdio or WebSocket so it can take part in conversations as a coding collaborator. +[OpenAI Codex](https://openai.com/codex) is a coding agent runtime that can inspect files, edit files, run commands, and manage approval workflows. The Band Codex adapter connects one room-owned Codex process to each Band room over stdio. Use this adapter when you want an OpenAI-powered coding agent with configurable sandboxing, approval commands, command/file-change telemetry, reasoning visibility, and task lifecycle events. Use the [Claude SDK adapter](claude_sdk.md) for Claude Code based coding agents, the [Anthropic adapter](anthropic.md) for direct Claude API chat/tool agents, or the [LangGraph adapter](langgraph.md) for custom graph workflows. @@ -26,47 +26,31 @@ You need two credentials or auth contexts: - A Band platform API key for `Agent.create(api_key=...)`. - Codex authentication for the Codex process. Use `codex login`, or set `OPENAI_API_KEY` if that is how your Codex environment is configured. -For `transport="ws"`, start the Codex app server separately: - -```bash -codex app-server --listen ws://127.0.0.1:8765 -``` - Credentials for Band can also be loaded from `agent_config.yaml` with `Agent.from_config("my_agent", adapter=adapter)`. ## Quick Start ```python -import asyncio -import os - from band import Agent from band.adapters.codex import CodexAdapter, CodexAdapterConfig adapter = CodexAdapter( - config=CodexAdapterConfig( - cwd=os.getcwd(), - model="gpt-5.5", - ), + config=CodexAdapterConfig(model="gpt-5.5"), ) - agent = Agent.create( adapter=adapter, agent_id="your-agent-uuid", api_key="your-band-api-key", - ws_url="wss://app.band.ai/api/v1/socket/websocket", - rest_url="https://app.band.ai", ) - -asyncio.run(agent.run()) +assert adapter.config.model == "gpt-5.5" ``` ## Where Parameters Go Codex has three setup layers: -- `CodexAdapterConfig(...)` configures the Codex runtime: transport, model, working directory, sandbox, approval behavior, prompts, context injection, and streaming/telemetry detail. -- `CodexAdapter(...)` wraps that runtime config for Band and adds adapter-level settings: feature flags, custom tools, history conversion, and advanced client injection. +- `CodexAdapterConfig(...)` configures the Codex runtime: a room workspace resolver, model, sandbox, approval behavior, prompts, context injection, and streaming/telemetry detail. +- `CodexAdapter(...)` wraps that runtime config for Band and adds adapter-level settings: feature flags, custom tools, and history conversion. - `Agent.create(...)` connects the configured adapter to Band. Use it for the Band agent identity, Band API key, platform URLs, session settings, contact-event handling, callbacks, and preprocessing. Codex authentication is handled by `codex login`, `OPENAI_API_KEY`, or the Codex process environment. `Agent.create(api_key=...)` is only the Band platform key. @@ -104,7 +88,10 @@ from band.core.types import Emit from band.adapters.codex import CodexAdapter, CodexAdapterConfig adapter = CodexAdapter( - config=CodexAdapterConfig(cwd="/repo", sandbox="workspace-write"), + config=CodexAdapterConfig( + workspace_for_room=lambda room_id: f"/workspaces/{room_id}", + sandbox="workspace-write", + ), emit=Emit.TOOL_CALLS | Emit.TASK_EVENTS, ) ``` @@ -115,12 +102,11 @@ Pass these to `CodexAdapterConfig(...)`: | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `transport` | `"stdio" \| "ws"` | `"stdio"` | How the adapter connects to Codex. Use `"stdio"` to spawn a process, or `"ws"` to connect to `codex app-server`. | +| `workspace_for_room` | `Callable[[str], str] | None` | `None` | Optional override for a room workspace. By default, the adapter creates `./.band-workspaces/`. | | `model` | `str \| None` | `None` | Model to use. When unset, the adapter asks Codex for visible models and uses the first visible model, or the adapter default if discovery fails or returns no usable model. | | `reasoning_effort` | `"none" \| "minimal" \| "low" \| "medium" \| "high" \| "xhigh" \| None` | `None` | Reasoning effort for models that support it. | | `reasoning_summary` | `"auto" \| "concise" \| "detailed" \| "none" \| None` | `None` | How Codex summarizes reasoning in responses. | | `personality` | `"friendly" \| "pragmatic" \| "none"` | `"pragmatic"` | Codex response style. | -| `cwd` | `str \| None` | `None` | Working directory for Codex sessions. | | `turn_timeout_s` | `float` | `180.0` | Maximum seconds to wait for one Codex turn. | ### Safety, Sandbox, and Approvals @@ -170,7 +156,6 @@ Pass these to `CodexAdapterConfig(...)`: |-----------|------|---------|-------------| | `codex_command` | `tuple[str, ...] \| None` | `None` | Custom command used to launch Codex for stdio transport. | | `codex_env` | `dict[str, str] \| None` | `None` | Extra environment variables for the Codex process. | -| `codex_ws_url` | `str` | `"ws://127.0.0.1:8765"` | WebSocket URL for `transport="ws"`. | | `experimental_api` | `bool` | `True` | Use experimental Codex API features. | | `enable_self_config_tools` | `bool` | `False` | Expose tools that let Codex change its own model and reasoning settings. Use only in trusted rooms. | | `additional_dynamic_tools` | `list[dict]` | `[]` | Extra dynamic tool schemas registered with the Codex client. | @@ -187,7 +172,6 @@ Pass these directly to `CodexAdapter(...)`: | `capabilities` | `Capability \| Iterable[Capability] \| None` | none | Optional Band tool categories exposed to the model. Opt-in: omitted, defaults to empty. | | `additional_tools` | `list[CustomToolDef] \| None` | `None` | Custom tools as `(PydanticModel, callable)` tuples. | | `history_converter` | `CodexHistoryConverter \| None` | auto | Advanced escape hatch for replacing the default history/thread-metadata converter. | -| `client_factory` | callable | `None` | Test/advanced injection point for a custom Codex client. | ## Feature flags: Capabilities and Emit diff --git a/examples/acp/clients/bridge_architecture.py b/examples/acp/clients/bridge_architecture.py index 0ad428918..3f8a20997 100644 --- a/examples/acp/clients/bridge_architecture.py +++ b/examples/acp/clients/bridge_architecture.py @@ -53,7 +53,7 @@ from band import Agent, configure_logging from band.adapters import ACPClientAdapter -from band.integrations.acp.client_profiles import CursorACPClientProfile +from band.integrations.acp.client_profiles import resolve_acp_client_profile configure_logging( level=logging.INFO, @@ -83,15 +83,12 @@ async def main() -> None: settings = Settings() command = shlex.split(settings.acp_agent_command) - cwd = settings.acp_agent_cwd auth_method = settings.acp_auth_method or None inject_band_tools = settings.acp_inject_band_tools - profile_name = settings.acp_client_profile.strip().lower() - profile = CursorACPClientProfile() if profile_name == "cursor" else None + profile = resolve_acp_client_profile(settings.acp_client_profile) adapter = ACPClientAdapter( command=command, - cwd=cwd, inject_band_tools=inject_band_tools, auth_method=auth_method, profile=profile, diff --git a/examples/acp/clients/cursor.py b/examples/acp/clients/cursor.py index bc6160b20..4a6091d78 100644 --- a/examples/acp/clients/cursor.py +++ b/examples/acp/clients/cursor.py @@ -53,8 +53,6 @@ import os from dotenv import load_dotenv -from pydantic_settings import BaseSettings, SettingsConfigDict - from band import Agent, configure_logging from band.adapters import ACPClientAdapter from band.integrations.acp.client_profiles import CursorACPClientProfile @@ -70,18 +68,8 @@ logger = logging.getLogger(__name__) -class Settings(BaseSettings): - model_config = SettingsConfigDict( - extra="ignore", case_sensitive=False, env_ignore_empty=True - ) - - acp_agent_cwd: str = "." - - async def main() -> None: load_dotenv() - settings = Settings() - cwd = settings.acp_agent_cwd # Cursor authentication environment — passed to the subprocess, so left as # a direct os.getenv pair rather than a Settings field. @@ -98,7 +86,6 @@ async def main() -> None: # - Band tools are injected through a local localhost-only MCP server adapter = ACPClientAdapter( command=[os.path.expanduser("~/.local/bin/agent"), "acp"], - cwd=cwd, env=cursor_env or None, inject_band_tools=True, auth_method="cursor_login", diff --git a/examples/acp/clients/generic.py b/examples/acp/clients/generic.py index 614161682..80f7a2420 100644 --- a/examples/acp/clients/generic.py +++ b/examples/acp/clients/generic.py @@ -77,13 +77,9 @@ async def main() -> None: # Command to spawn the remote ACP agent acp_command = shlex.split(settings.acp_agent_command) - # Working directory for ACP sessions - acp_cwd = settings.acp_agent_cwd - # Create adapter pointing to remote ACP agent adapter = ACPClientAdapter( command=acp_command, - cwd=acp_cwd, ) logger.info( diff --git a/examples/acp/clients/rich_streaming.py b/examples/acp/clients/rich_streaming.py index 3f843e9ff..2586e3517 100644 --- a/examples/acp/clients/rich_streaming.py +++ b/examples/acp/clients/rich_streaming.py @@ -89,13 +89,9 @@ async def main() -> None: # Command to spawn the remote ACP agent acp_command = shlex.split(settings.acp_agent_command) - # Working directory for ACP sessions - acp_cwd = settings.acp_agent_cwd - # Create adapter pointing to remote ACP agent adapter = ACPClientAdapter( command=acp_command, - cwd=acp_cwd, ) logger.info("Starting ACP client bridge with rich streaming...") diff --git a/examples/codex/01_basic_agent.py b/examples/codex/01_basic_agent.py index 37f01f086..1033c2b39 100644 --- a/examples/codex/01_basic_agent.py +++ b/examples/codex/01_basic_agent.py @@ -13,17 +13,14 @@ Prerequisites: 1. OAuth login: codex login -2. For stdio mode (default), no extra process is needed. -3. For ws mode, start app-server separately: - codex app-server --listen ws://127.0.0.1:8765 +2. The adapter starts one local stdio process for each Band room. Run: uv run examples/codex/01_basic_agent.py Optional env overrides: AGENT_KEY=darter - CODEX_TRANSPORT=stdio|ws - CODEX_WS_URL=ws://127.0.0.1:8765 + CODEX_WORKSPACE_ROOT=.band-workspaces CODEX_ROLE=coding|planner|reviewer CODEX_MODEL=gpt-5.5 CODEX_APPROVAL_MODE=manual|auto_accept|auto_decline @@ -84,7 +81,7 @@ async def main() -> None: "Role '%s' specified but no prompt file at %s", codex_role, prompt_file ) - # transport/codex_ws_url/model/cwd/approval_policy/approval_mode/ + # model/approval_policy/approval_mode/ # emit_turn_task_markers all self-source from CODEX_* env vars (see module # docstring) when omitted here. adapter = CodexAdapter( diff --git a/examples/codex/02_tom_agent.py b/examples/codex/02_tom_agent.py index 1e93f4949..1b28bfd42 100644 --- a/examples/codex/02_tom_agent.py +++ b/examples/codex/02_tom_agent.py @@ -57,7 +57,7 @@ async def main() -> None: load_dotenv() - # cwd/model self-source from CODEX_CWD/CODEX_MODEL when omitted here. + # model self-sources from CODEX_MODEL when omitted here. adapter = CodexAdapter( config=CodexAdapterConfig( transport="stdio", diff --git a/examples/codex/03_jerry_agent.py b/examples/codex/03_jerry_agent.py index ee1cd8b1a..c78ed720a 100644 --- a/examples/codex/03_jerry_agent.py +++ b/examples/codex/03_jerry_agent.py @@ -57,7 +57,7 @@ async def main() -> None: load_dotenv() - # cwd/model self-source from CODEX_CWD/CODEX_MODEL when omitted here. + # model self-sources from CODEX_MODEL when omitted here. adapter = CodexAdapter( config=CodexAdapterConfig( transport="stdio", diff --git a/examples/docker_demo/agents/dev/main.py b/examples/docker_demo/agents/dev/main.py index 971d891c7..e8adb90be 100644 --- a/examples/docker_demo/agents/dev/main.py +++ b/examples/docker_demo/agents/dev/main.py @@ -82,7 +82,9 @@ async def main() -> None: config = DevConfig() adapter = CodexAdapter( config=CodexAdapterConfig( - model=config.model, approval_policy="never", custom_section=build_persona() + model=config.model, + approval_policy="never", + custom_section=build_persona(), ), # Emit tool_call/tool_result and reasoning to the room, keeping the default # per-turn task markers but excluding usage events. Codex's Band tools diff --git a/examples/run_agent.py b/examples/run_agent.py index fb37065f0..8f978ebd3 100644 --- a/examples/run_agent.py +++ b/examples/run_agent.py @@ -32,8 +32,7 @@ uv run python examples/run_agent.py --example crewai uv run python examples/run_agent.py --example crewai --streaming # Show tool calls uv run python examples/run_agent.py --example codex - uv run python examples/run_agent.py --example codex --agent darter --codex-transport stdio - uv run python examples/run_agent.py --example codex --agent darter --codex-transport ws --codex-ws-url ws://127.0.0.1:8765 + uv run python examples/run_agent.py --example codex --agent darter uv run python examples/run_agent.py --example a2a --a2a-url http://localhost:10000 # A2A bridge uv run python examples/run_agent.py --example a2a_gateway # A2A Gateway (exposes peers) uv run python examples/run_agent.py --example a2a_gateway --gateway-port 8080 # Custom port @@ -446,7 +445,6 @@ async def run_codex_agent( api_key: str, custom_section: str, codex_transport: str, - codex_ws_url: str, codex_model: str | None, codex_personality: str, codex_approval_policy: str, @@ -458,20 +456,20 @@ async def run_codex_agent( logger: logging.Logger, ) -> None: """Run the Codex app-server adapter.""" + from band import create_room_workspace_resolver # noqa: PLC0415 -- only load the adapters extra when this example is the one selected to run from band.adapters import CodexAdapter # noqa: PLC0415 -- only load the adapters extra when this example is the one selected to run from band.adapters.codex import CodexAdapterConfig # noqa: PLC0415 -- only load the codex extra when this example is the one selected to run adapter = CodexAdapter( config=CodexAdapterConfig( transport=codex_transport, # type: ignore[arg-type] # str from CLI args, validated at runtime - cwd=codex_cwd, + workspace_for_room=create_room_workspace_resolver(codex_cwd), model=codex_model, personality=codex_personality, # type: ignore[arg-type] # str from CLI args, validated at runtime approval_policy=codex_approval_policy, approval_mode=codex_approval_mode, # type: ignore[arg-type] # str from CLI args, validated at runtime sandbox=codex_sandbox, reasoning_effort=codex_reasoning_effort, # type: ignore[arg-type] # str from CLI args, validated at runtime - codex_ws_url=codex_ws_url, custom_section=custom_section, include_base_instructions=True, emit_turn_task_markers=codex_turn_task_markers, @@ -482,7 +480,7 @@ async def run_codex_agent( ) logger.info( - "Starting Codex agent (transport=%s, model=%s, cwd=%s)", + "Starting Codex agent (transport=%s, model=%s, workspace_root=%s)", codex_transport, codex_model or "auto", codex_cwd, @@ -775,7 +773,6 @@ async def main() -> None: uv run python examples/run_agent.py --example codex # Codex app-server adapter uv run python examples/run_agent.py --example codex --agent darter # Run Codex as darter agent uv run python examples/run_agent.py --example codex --codex-transport stdio - uv run python examples/run_agent.py --example codex --codex-transport ws --codex-ws-url ws://127.0.0.1:8765 uv run python examples/run_agent.py --example a2a # A2A bridge (default: localhost:10000) uv run python examples/run_agent.py --example a2a --debug # A2A with debug logging (context_id tracing) uv run python examples/run_agent.py --example a2a --a2a-url http://remote:8080 # A2A with custom URL @@ -859,15 +856,10 @@ async def main() -> None: ) parser.add_argument( "--codex-transport", - choices=["stdio", "ws"], + choices=["stdio"], default="stdio", help="Codex transport mode (default: stdio)", ) - parser.add_argument( - "--codex-ws-url", - default=os.getenv("CODEX_WS_URL", "ws://127.0.0.1:8765"), - help="Codex WebSocket URL when --codex-transport=ws", - ) parser.add_argument( "--codex-role", default=None, @@ -887,7 +879,7 @@ async def main() -> None: parser.add_argument( "--codex-cwd", default=os.getcwd(), - help="Working directory given to Codex app-server (default: current directory)", + help="Root directory for per-room Codex workspaces (default: current directory)", ) parser.add_argument( "--codex-reasoning-effort", @@ -1105,7 +1097,6 @@ async def main() -> None: api_key=api_key, custom_section=codex_custom, codex_transport=args.codex_transport, - codex_ws_url=args.codex_ws_url, codex_model=args.codex_model, codex_personality=args.codex_personality, codex_approval_policy=args.codex_approval_policy, diff --git a/src/band/__init__.py b/src/band/__init__.py index a69faeba9..03791448b 100644 --- a/src/band/__init__.py +++ b/src/band/__init__.py @@ -72,6 +72,7 @@ async def handle_event(ctx: ExecutionContext, event: PlatformEvent): # Platform layer from .platform import BandLink, PlatformEvent +from .workspaces import create_room_workspace_resolver # Runtime layer from .runtime import ( @@ -132,6 +133,7 @@ async def handle_event(ctx: ExecutionContext, event: PlatformEvent): # Platform "BandLink", "PlatformEvent", + "create_room_workspace_resolver", # Runtime - Core "AgentRuntime", "RoomPresence", diff --git a/src/band/adapters/codex.py b/src/band/adapters/codex.py index 9ad53f155..87c03e9af 100644 --- a/src/band/adapters/codex.py +++ b/src/band/adapters/codex.py @@ -5,10 +5,10 @@ import asyncio import json import logging -import os import time as _time from collections import OrderedDict -from dataclasses import dataclass +from contextvars import ContextVar +from dataclasses import dataclass, field as dataclass_field from datetime import datetime, timezone from typing import ClassVar, Any, Callable, Literal, NamedTuple, Protocol @@ -20,6 +20,7 @@ from band.converters.helpers import build_replay_messages from band.core.protocols import AgentToolsProtocol from band.core.simple_adapter import SimpleAdapter +from band.workspaces import claim_room_workspace, resolve_room_workspace from band.core.types import ( AgentInput, Capability, @@ -32,7 +33,6 @@ from band.integrations.codex import ( CodexJsonRpcError, CodexStdioClient, - CodexWebSocketClient, RpcEvent, ) from band.integrations.codex.types import ( @@ -244,6 +244,19 @@ class TurnResult: saw_send_message_tool: bool = False +@dataclass +class RoomCodexClient: + """The process and protocol state owned by one Band room.""" + + workspace: str + client: CodexClientProtocol | None = None + initialized: bool = False + selected_model: str | None = None + reasoning_effort: str | None = None + reasoning_summary: str | None = None + rpc_lock: asyncio.Lock = dataclass_field(default_factory=asyncio.Lock) + + class CodexAdapterConfig(BaseSettings): """Runtime configuration for Codex adapter sessions. @@ -279,6 +292,7 @@ class CodexAdapterConfig(BaseSettings): extra="forbid", env_ignore_empty=True, populate_by_name=True, + arbitrary_types_allowed=True, ) transport: TransportKind = "stdio" @@ -287,7 +301,8 @@ class CodexAdapterConfig(BaseSettings): Literal["none", "minimal", "low", "medium", "high", "xhigh"] | None ) = None reasoning_summary: Literal["auto", "concise", "detailed", "none"] | None = None - cwd: str = Field(default_factory=os.getcwd) + cwd: str | None = None + workspace_for_room: Callable[[str], str] | None = Field(default=None, exclude=True) approval_policy: str = "never" personality: Literal["friendly", "pragmatic", "none"] = "pragmatic" sandbox: str | None = None @@ -417,14 +432,27 @@ def __init__( self._custom_tools: list[CustomToolDef] = list(additional_tools or []) if self.config.enable_self_config_tools: self._custom_tools.extend(self._build_self_config_tools()) - self._client_factory = client_factory - self._client: CodexClientProtocol | None = None - self._initialized = False - self._selected_model: str | None = None + if self.config.cwd is not None: + raise ValueError( + "cwd is not supported; use workspace_for_room or the default" + ) + if self.config.transport != "stdio": + raise ValueError( + "only stdio Codex transport guarantees room process isolation" + ) + if client_factory is not None: + raise ValueError( + "custom Codex clients cannot guarantee room process isolation" + ) + self._room_clients: dict[str, RoomCodexClient] = {} + self._workspace_rooms: dict[str, str] = {} + self._active_room: ContextVar[str | None] = ContextVar( + "codex_active_room", default=None + ) self._system_prompt: str = "" self._room_threads: dict[str, str] = {} self._prompt_injected_rooms: set[str] = set() - self._task_titles_by_id: OrderedDict[str, str] = OrderedDict() + self._room_task_titles: dict[str, OrderedDict[str, str]] = {} self._max_task_titles: int = _MAX_TASK_TITLES self._pending_approvals: dict[str, dict[str, PendingApproval]] = {} self._raw_history_by_room: dict[str, list[dict[str, Any]]] = {} @@ -445,7 +473,81 @@ def __init__( # up to ``approval_wait_timeout_s`` (300s default). Approval resolution # commands (/approve, /decline) are handled *outside* this lock in # ``on_message`` so they can unblock a waiting turn. - self._rpc_lock = asyncio.Lock() + + def _room_client(self, room_id: str) -> RoomCodexClient: + room = self._room_clients.get(room_id) + if room is None: + workspace = resolve_room_workspace(room_id, self.config.workspace_for_room) + claim_room_workspace(room_id, workspace, self._workspace_rooms) + room = RoomCodexClient(workspace=workspace) + self._room_clients[room_id] = room + return room + + def _active_client_state(self) -> RoomCodexClient | None: + room_id = self._active_room.get() + return self._room_clients.get(room_id) if room_id is not None else None + + def _require_active_client_state(self) -> RoomCodexClient: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex operation requires a room context") + return state + + @property + def _task_titles_by_id(self) -> OrderedDict[str, str]: + room_id = self._active_room.get() + if room_id is None: + raise RuntimeError("Codex task state requires a room context") + return self._room_task_titles.setdefault(room_id, OrderedDict()) + + @property + def _client(self) -> CodexClientProtocol | None: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + return state.client + + @_client.setter + def _client(self, value: CodexClientProtocol | None) -> None: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + state.client = value + + @property + def _initialized(self) -> bool: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + return state.initialized + + @_initialized.setter + def _initialized(self, value: bool) -> None: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + state.initialized = value + + @property + def _selected_model(self) -> str | None: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + return state.selected_model + + @_selected_model.setter + def _selected_model(self, value: str | None) -> None: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + state.selected_model = value + + @property + def _rpc_lock(self) -> asyncio.Lock: + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client requires a room context") + return state.rpc_lock def _build_self_config_tools(self) -> list[CustomToolDef]: """Build custom tools that let Codex change its own model/reasoning. @@ -460,7 +562,6 @@ def _build_self_config_tools(self) -> list[CustomToolDef]: def _handle_set_model(inp: SetModelInput) -> str: if not adapter._rpc_lock.locked(): raise RuntimeError("_handle_set_model must run under _rpc_lock") - adapter.config.model = inp.model adapter._selected_model = inp.model return f"Model changed to {inp.model} for subsequent turns." @@ -474,7 +575,7 @@ def _handle_set_reasoning(inp: SetReasoningInput) -> str: f"Invalid reasoning effort '{inp.effort}'. " f"Valid: {', '.join(sorted(_REASONING_EFFORTS))}." ) - adapter.config.reasoning_effort = inp.effort # type: ignore[assignment] # Literal narrowed by Pydantic validation + adapter._require_active_client_state().reasoning_effort = inp.effort parts.append(f"effort={inp.effort}") if inp.summary is not None: if inp.summary not in _REASONING_SUMMARIES: @@ -482,7 +583,7 @@ def _handle_set_reasoning(inp: SetReasoningInput) -> str: f"Invalid reasoning summary '{inp.summary}'. " f"Valid: {', '.join(sorted(_REASONING_SUMMARIES))}." ) - adapter.config.reasoning_summary = inp.summary # type: ignore[assignment] # Literal narrowed by Pydantic validation + adapter._require_active_client_state().reasoning_summary = inp.summary parts.append(f"summary={inp.summary}") if not parts: return ( @@ -499,8 +600,6 @@ def _handle_set_reasoning(inp: SetReasoningInput) -> str: async def on_started(self, agent_name: str, agent_description: str) -> None: await super().on_started(agent_name, agent_description) self._build_system_prompt() - async with self._rpc_lock: - await self._ensure_client_ready() self._log_startup_config(agent_name) def _log_startup_config(self, agent_name: str) -> None: @@ -528,7 +627,7 @@ def _log_startup_config(self, agent_name: str) -> None: "diffs=%s, token_usage=%s, structured_errors=%s", agent_name, self.config.transport, - self._selected_model or self.config.model or "auto", + self.config.model or "auto", self.config.sandbox or "default", self.config.approval_mode, Emit.TOOL_CALLS in self.features.emit, @@ -564,6 +663,8 @@ async def on_message( is_session_bootstrap: bool, room_id: str, ) -> None: + self._room_client(room_id) + self._active_room.set(room_id) command = self._extract_local_command(msg.content) if command is not None and command[0] in { "approve", @@ -963,14 +1064,11 @@ async def _process_turn_events( # Also drop token-usage entries keyed by the dead thread # ids; otherwise they leak until the last-room teardown # because on_cleanup can no longer resolve their keys. - stale_rooms = list(self._room_threads.keys()) - stale_threads = list(self._room_threads.values()) - self._room_threads.clear() - self._raw_history_by_room.clear() - for stale_thread in stale_threads: + stale_thread = self._room_threads.pop(room_id, None) + self._raw_history_by_room.pop(room_id, None) + if stale_thread: self._token_usage.pop(stale_thread, None) - for stale_room in stale_rooms: - self._clear_pending_approvals_for_room(stale_room) + self._clear_pending_approvals_for_room(room_id) break if event.method == "turn/completed": @@ -1017,11 +1115,10 @@ async def _process_turn_events( return result async def on_cleanup(self, room_id: str) -> None: - # NOTE: _rpc_lock is adapter-wide, so cleanup for room B blocks if - # room A holds the lock during a pending manual approval (up to - # approval_wait_timeout_s). This is a known limitation of the single- - # client architecture — the lock serializes all turn processing and - # cleanup across rooms. + room = self._room_clients.get(room_id) + if room is None: + return + self._active_room.set(room_id) async with self._rpc_lock: thread_id = self._room_threads.pop(room_id, None) if thread_id: @@ -1033,9 +1130,10 @@ async def on_cleanup(self, room_id: str) -> None: self._approval_audit.pop(room_id, None) self._session_approved.pop(room_id, None) self._sandbox_overrides.pop(room_id, None) - if self._room_threads: - return + self._room_task_titles.pop(room_id, None) if self._client is None: + self._room_clients.pop(room_id, None) + self._workspace_rooms.pop(room.workspace, None) return try: close_coro = self._client.close() @@ -1055,24 +1153,27 @@ async def on_cleanup(self, room_id: str) -> None: self._client = None self._initialized = False self._selected_model = None - self._task_titles_by_id.clear() - # Defensive: wipe all pending approvals globally on last-room - # teardown. Per-room cleanup already resolves futures to - # "decline" via _clear_pending_approvals_for_room, so this - # catches any leaked entries from rooms whose cleanup failed. - self._pending_approvals.clear() - self._token_usage.clear() - self._approval_audit.clear() - self._session_approved.clear() - self._sandbox_overrides.clear() + self._room_clients.pop(room_id, None) + self._workspace_rooms.pop(room.workspace, None) + + async def cleanup_all(self) -> None: + """Close every room-owned Codex process during agent shutdown.""" + await asyncio.gather( + *(self.on_cleanup(room_id) for room_id in list(self._room_clients)) + ) async def _ensure_client_ready(self) -> None: if self._client is None: self._client = self._build_client(self.config) - if not self._initialized: - await self._client.connect() - await self._client.initialize( + client = self._client + if client is None: + raise RuntimeError("Codex client was not created") + if self._initialized: + return + try: + await client.connect() + await client.initialize( client_name=self.config.client_name, client_title=self.config.client_title, client_version=self.config.client_version, @@ -1080,17 +1181,25 @@ async def _ensure_client_ready(self) -> None: ) self._selected_model = await self._select_model() self._initialized = True + except Exception: + if self._client is client: + self._client = None + try: + await client.close() + except Exception: + logger.debug( + "Failed to close unsuccessfully initialized Codex client", + exc_info=True, + ) + raise def _build_client(self, config: CodexAdapterConfig) -> CodexClientProtocol: - if self._client_factory is not None: - return self._client_factory(config) - - if config.transport == "ws": - return CodexWebSocketClient(ws_url=config.codex_ws_url) - + state = self._active_client_state() + if state is None: + raise RuntimeError("Codex client creation requires a room context") return CodexStdioClient( command=config.codex_command, - cwd=config.cwd, + cwd=state.workspace, env=config.codex_env, ) @@ -1175,7 +1284,7 @@ async def _ensure_thread( dynamic_tools = self._build_dynamic_tools(tools) start_params: dict[str, Any] = { "model": self._selected_model, - "cwd": self.config.cwd, + "cwd": self._room_client(room_id).workspace, "approvalPolicy": self.config.approval_policy, "personality": self.config.personality, "dynamicTools": dynamic_tools, @@ -2622,7 +2731,7 @@ async def _handle_local_command( ) return True - self.config.model = model_arg + # Model selection belongs to the room's process, never adapter-wide config. self._selected_model = model_arg await tools.send_message( f"Model override set to `{model_arg}` for subsequent turns.", @@ -2647,7 +2756,7 @@ async def _handle_local_command( mentions=mention, ) return True - self.config.reasoning_effort = effort_arg # type: ignore[assignment] # Literal narrowed by Pydantic validation + self._require_active_client_state().reasoning_effort = effort_arg await tools.send_message( f"Reasoning effort set to `{effort_arg}` for subsequent turns.", mentions=mention, @@ -2927,14 +3036,15 @@ def _build_system_prompt(self) -> None: def _apply_turn_overrides( self, params: dict[str, Any], *, room_id: str | None = None ) -> None: + state = self._require_active_client_state() params["model"] = self._selected_model - params["cwd"] = self.config.cwd + params["cwd"] = state.workspace params["approvalPolicy"] = self.config.approval_policy params["personality"] = self.config.personality - if self.config.reasoning_effort is not None: - params["effort"] = self.config.reasoning_effort - if self.config.reasoning_summary is not None: - params["summary"] = self.config.reasoning_summary + if state.reasoning_effort or self.config.reasoning_effort: + params["effort"] = state.reasoning_effort or self.config.reasoning_effort + if state.reasoning_summary or self.config.reasoning_summary: + params["summary"] = state.reasoning_summary or self.config.reasoning_summary self._apply_turn_sandbox(params, room_id=room_id) async def _start_turn(self, params: dict[str, Any]) -> dict[str, Any]: diff --git a/src/band/integrations/acp/__init__.py b/src/band/integrations/acp/__init__.py index 9519a9f97..9df090190 100644 --- a/src/band/integrations/acp/__init__.py +++ b/src/band/integrations/acp/__init__.py @@ -33,7 +33,9 @@ from band import Agent from band.integrations.acp import ACPClientAdapter - adapter = ACPClientAdapter(command="codex", cwd="/workspace") + adapter = ACPClientAdapter( + command="codex", + ) agent = Agent.create(adapter=adapter, agent_id="...", api_key="...") await agent.run() """ diff --git a/src/band/integrations/acp/client_adapter.py b/src/band/integrations/acp/client_adapter.py index 57666a0e3..3bb0ebe84 100644 --- a/src/band/integrations/acp/client_adapter.py +++ b/src/band/integrations/acp/client_adapter.py @@ -4,7 +4,6 @@ import asyncio import logging -import os import shutil from collections.abc import Callable from typing import Any, ClassVar @@ -29,11 +28,11 @@ from band.integrations.acp.client_runtime import ( ACPConnectionProtocol, ACPRuntime, + MCPTransportKind, PermissionHandler, allow_permission, cancel_permission, select_allow_option_id, - tcp_spawn_process, ) from band.integrations.acp.client_types import ( ACPClientSessionState, @@ -41,10 +40,12 @@ ) from band.integrations.mcp.backends import ( BandMCPBackend, + BandMCPBackendKind, create_band_mcp_backend, ) from band.integrations.acp.room_emitter import RoomTurnEmitter from band.integrations.acp.types import ACPToolCall +from band.workspaces import claim_room_workspace, resolve_room_workspace from band.runtime.prompts import render_system_prompt from band.runtime.custom_tools import CustomToolDef, get_custom_tool_name from band.runtime.formatters import messages_before @@ -61,6 +62,7 @@ logger = logging.getLogger(__name__) LocalMcpServerConfig = HttpMcpServer | SseMcpServer +DEFAULT_BAND_MCP_BACKEND_KIND: BandMCPBackendKind = "http" # Prefixes the change-triggered roster/contacts updates injected into a # prompt, so the model reads them as platform state, not as the requester @@ -108,6 +110,7 @@ def new_message_marker() -> str: # stdio and TCP are the built-in transports; injecting one (e.g. docker exec / ssh, # or a fake in tests) is the supported extension point. SpawnProcess = Callable[..., object] +WorkspaceResolver = Callable[[str], str] def _resolve_launcher(command: list[str]) -> list[str]: @@ -144,6 +147,7 @@ def __init__( command: str | list[str] | None = None, env: dict[str, str] | None = None, cwd: str | None = None, + workspace_for_room: WorkspaceResolver | None = None, mcp_servers: list[dict[str, Any]] | None = None, additional_tools: list[CustomToolDef] | None = None, inject_band_tools: bool = True, @@ -163,10 +167,23 @@ def __init__( history_converter=ACPClientHistoryConverter(), **features, ) - self._host, self._port = self._resolve_transport(command, host, port) - self._command = self._shape_command(command, self._host) + if cwd is not None: + raise ValueError( + "cwd is not supported; use workspace_for_room or the default" + ) + if host is not None or port is not None: + raise ValueError( + "TCP ACP transport cannot guarantee room process isolation" + ) + if spawn_process is not None: + raise ValueError( + "custom ACP transports cannot guarantee room process isolation" + ) + if not command: + raise ValueError("ACP stdio transport requires a command") + self._command = [command] if isinstance(command, str) else list(command) self._env = env - self._cwd = os.path.abspath(cwd or ".") + self._workspace_for_room = workspace_for_room self._mcp_servers = list(mcp_servers or []) self._custom_tools: list[CustomToolDef] = list(additional_tools or []) self._tool_definitions, self._own_tool_names = self._registered_tools() @@ -174,7 +191,9 @@ def __init__( self._auth_method = auth_method self._profile = profile self._custom_section = custom_section - self._runtime = self._build_runtime(spawn_process) + self._runtimes: dict[str, ACPRuntime] = {} + self._room_workspaces: dict[str, str] = {} + self._workspace_rooms: dict[str, str] = {} self._room_to_session: dict[str, str] = {} self._room_tools: dict[str, AgentToolsProtocol] = {} @@ -196,21 +215,6 @@ def apply_effective_features(self, features: AdapterFeatures) -> None: super().apply_effective_features(features) self._tool_definitions, self._own_tool_names = self._registered_tools() - @staticmethod - def _shape_command(command: str | list[str] | None, host: str | None) -> list[str]: - """The subprocess command for stdio, or an empty command for TCP. - - stdio spawns a subprocess from ``command``; TCP dials an - already-running ACP server at ``host``/port instead. ``host`` is - passed explicitly (not read off ``self``) so this stays checkable - independent of ``__init__``'s statement order. - """ - if host is not None: - return [] - # _resolve_transport guarantees command is set when host is None. - assert command is not None - return [command] if isinstance(command, str) else list(command) - def _registered_tools(self) -> tuple[list[ToolDefinition], frozenset[str]]: """The tools this adapter registers on the loopback MCP server. @@ -249,22 +253,7 @@ def _registered_tools(self) -> tuple[list[ToolDefinition], frozenset[str]]: ) return definitions, names - @staticmethod - def _select_transport( - spawn_process: SpawnProcess | None, host: str | None, port: int | None - ) -> SpawnProcess: - """An explicit ``spawn_process`` wins (advanced/custom transports and - tests); otherwise acp's subprocess spawner (stdio) or a connect-only - seam closed over host/port (TCP; see ``tcp_spawn_process``). ``host``/ - ``port`` are explicit (not read off ``self``), matching - ``_shape_command``.""" - if spawn_process is not None: - return spawn_process - if host is not None and port is not None: - return tcp_spawn_process(host, port) - return spawn_agent_process - - def _build_runtime(self, spawn_process: SpawnProcess | None) -> ACPRuntime: + def _build_runtime(self) -> ACPRuntime: return ACPRuntime( command=_resolve_launcher(self._command), env=self._env, @@ -273,9 +262,23 @@ def _build_runtime(self, spawn_process: SpawnProcess | None) -> ACPRuntime: profile=self._profile, canonicalize_tool_name=self._canonical_tool_name, ), - spawn_process=self._select_transport(spawn_process, self._host, self._port), + spawn_process=spawn_agent_process, ) + def _workspace(self, room_id: str) -> str: + return resolve_room_workspace(room_id, self._workspace_for_room) + + async def _runtime_for(self, room_id: str) -> ACPRuntime: + async with self._session_lock: + runtime = self._runtimes.get(room_id) + if runtime is None: + workspace = self._workspace(room_id) + claim_room_workspace(room_id, workspace, self._workspace_rooms) + runtime = self._build_runtime() + self._runtimes[room_id] = runtime + self._room_workspaces[room_id] = workspace + return runtime + async def on_started(self, agent_name: str, agent_description: str) -> None: await super().on_started(agent_name, agent_description) # The other end of cleanup_all(final=True)'s _stopped: Agent.start() @@ -284,10 +287,6 @@ async def on_started(self, agent_name: str, agent_description: str) -> None: # the backend must be startable again too. async with self._mcp_backend_lock: self._stopped = False - await self._spawn_process() - - async def _spawn_process(self) -> None: - await self._runtime.start(respawn=False) async def on_message( self, @@ -300,7 +299,8 @@ async def on_message( is_session_bootstrap: bool, room_id: str, ) -> None: - await self._ensure_connection() + runtime = await self._runtime_for(room_id) + await self._ensure_connection(runtime) if self._inject_band_tools: async with self._session_lock: @@ -310,7 +310,7 @@ async def on_message( await self._load_persisted_session(room_id, history) session_id, created = await self._get_or_create_session(room_id) - self._runtime.reset_session(session_id) + runtime.reset_session(session_id) # A just-created session holds no remote context (a restored one does), # so seed it with the Band room's transcript. On bootstrap the converter @@ -348,18 +348,18 @@ async def on_message( session_id=session_id, room_id=room_id, ) as emitter: - self._runtime.set_permission_handler( + runtime.set_permission_handler( session_id, self._make_permission_handler(emitter, room_id), ) - await self._runtime.prompt( + await runtime.prompt( session_id=session_id, prompt_text=prompt_text, on_chunk=emitter.emit, ) except Exception as e: logger.exception("ACP agent error: %s", e) - await self.stop() + await self.on_cleanup(room_id) await tools.send_event( content=f"ACP agent error: {e}", message_type="error", @@ -412,33 +412,6 @@ async def handler( return handler - @staticmethod - def _resolve_transport( - command: str | list[str] | None, - host: str | None, - port: int | None, - ) -> tuple[str | None, int | None]: - """Validate exactly one transport is configured; return (host, port) for TCP. - - stdio spawns a subprocess from ``command``; TCP connects to an - already-running ACP server at ``host``/``port``. The two are mutually - exclusive and one is required. - """ - # An empty command ("" or []) is not a usable stdio transport — treat it as - # absent so it fails the "one is required" check below with a clear error, - # rather than slipping through to crash at spawn time. - has_command = bool(command) - has_tcp = host is not None or port is not None - if has_command and has_tcp: - raise ValueError( - "Provide either command (stdio) or host+port (TCP), not both" - ) - if not has_command and not has_tcp: - raise ValueError("Provide either command (stdio) or host+port (TCP)") - if has_tcp and (host is None or port is None): - raise ValueError("TCP transport requires both host and port") - return (host, port) if has_tcp else (None, None) - def _build_system_context(self, room_id: str, msg: PlatformMessage) -> str: agent_name = self.agent_name or "Agent" agent_desc = self.agent_description or "An AI assistant" @@ -473,10 +446,9 @@ def _build_system_context(self, room_id: str, msg: PlatformMessage) -> str: return f"[System Context]\n{system_prompt}\n{room_context}" def _build_local_mcp_server_config( - self, - local_server: LocalMCPServer, + self, local_server: LocalMCPServer, transport: MCPTransportKind ) -> LocalMcpServerConfig: - if self._runtime._agent_mcp_transport == "sse": + if transport == "sse": return SseMcpServer( type="sse", name=BAND_MCP_SERVER_NAME, @@ -535,7 +507,7 @@ async def _ensure_band_mcp_backend(self) -> BandMCPBackend: self._band_mcp_backend = None if self._band_mcp_backend is None: backend = await create_band_mcp_backend( - kind=self._runtime._agent_mcp_transport, + kind=DEFAULT_BAND_MCP_BACKEND_KIND, tool_definitions=self._tool_definitions, get_tools=self._room_tools.get, additional_tools=self._custom_tools, @@ -543,13 +515,16 @@ async def _ensure_band_mcp_backend(self) -> BandMCPBackend: self._band_mcp_backend = backend return self._band_mcp_backend - async def _get_or_start_band_mcp_server(self) -> LocalMcpServerConfig: + async def _get_or_start_band_mcp_server(self, room_id: str) -> LocalMcpServerConfig: backend = await self._ensure_band_mcp_backend() local_server = backend.local_server if local_server is None: raise RuntimeError("ACP MCP backend did not create a local server") - return self._build_local_mcp_server_config(local_server) + runtime = await self._runtime_for(room_id) + return self._build_local_mcp_server_config( + local_server, runtime._agent_mcp_transport + ) async def _get_or_create_session(self, room_id: str) -> tuple[str, bool]: """This room's ACP session id, plus whether it was created just now. @@ -560,14 +535,14 @@ async def _get_or_create_session(self, room_id: str) -> tuple[str, bool]: if room_id in self._room_to_session: return self._room_to_session[room_id], False + runtime = await self._runtime_for(room_id) + mcp_servers = await self._session_mcp_servers(room_id) async with self._session_lock: if room_id in self._room_to_session: return self._room_to_session[room_id], False - mcp_servers = await self._session_mcp_servers() - - session_id = await self._runtime.create_session( - cwd=self._cwd, + session_id = await runtime.create_session( + cwd=self._room_workspaces[room_id], mcp_servers=mcp_servers, ) self._room_to_session[room_id] = session_id @@ -579,11 +554,11 @@ async def _get_or_create_session(self, room_id: str) -> tuple[str, bool]: ) return session_id, True - async def _session_mcp_servers(self) -> list[object]: + async def _session_mcp_servers(self, room_id: str) -> list[object]: """The MCP configuration supplied when creating or loading a session.""" mcp_servers: list[object] = list(self._mcp_servers) if self._inject_band_tools: - mcp_servers.append(await self._get_or_start_band_mcp_server()) + mcp_servers.append(await self._get_or_start_band_mcp_server(room_id)) return mcp_servers def _claim_session_bootstrap(self, session_id: str) -> bool: @@ -666,15 +641,26 @@ async def on_cleanup(self, room_id: str) -> None: self._room_tools.pop(room_id, None) if session_id: self._bootstrapped_sessions.discard(session_id) + runtime = self._runtimes.pop(room_id, None) + workspace = self._room_workspaces.pop(room_id, None) + if workspace is not None: + self._workspace_rooms.pop(workspace, None) + + if runtime is not None: + await runtime.stop() logger.debug("Cleaned up ACP client resources for room %s", room_id) + @staticmethod + async def _stop_runtimes(runtimes: list[ACPRuntime]) -> None: + await asyncio.gather(*(runtime.stop() for runtime in runtimes)) + async def cleanup_all(self, *, final: bool = True) -> None: """Adapter-wide teardown — the hook ``Agent.stop()`` invokes on shutdown. - The ACP subprocess / TCP connection and the local Band MCP server are started - adapter-wide in ``on_started`` (not per room), so releasing them belongs here, - not in per-room ``on_cleanup``. Idempotent — safe to call again from ``stop()``. + Room-owned ACP subprocesses are released by ``on_cleanup``; this method + releases every remaining runtime and the shared local Band MCP server. + Idempotent — safe to call again from ``stop()``. ``final`` distinguishes real process shutdown (the default: no future turn can arrive, so a still-parked one must fail rather than start resources @@ -688,6 +674,10 @@ async def cleanup_all(self, *, final: bool = True) -> None: self._room_to_session.clear() self._room_tools.clear() self._bootstrapped_sessions.clear() + runtimes = list(self._runtimes.values()) + self._runtimes.clear() + self._room_workspaces.clear() + self._workspace_rooms.clear() async with self._mcp_backend_lock: backend = self._band_mcp_backend self._band_mcp_backend = None @@ -703,7 +693,7 @@ async def cleanup_all(self, *, final: bool = True) -> None: # None and start a fresh backend while this one is mid-teardown. if backend is not None: await backend.stop() - await self._runtime.stop() + await self._stop_runtimes(runtimes) logger.info("ACP client adapter stopped") async def stop(self) -> None: @@ -730,10 +720,11 @@ async def _load_persisted_session( if session_id is None: return - loaded = await self._runtime.load_session( - cwd=self._cwd, + runtime = await self._runtime_for(room_id) + loaded = await runtime.load_session( + cwd=self._room_workspaces[room_id], session_id=session_id, - mcp_servers=await self._session_mcp_servers(), + mcp_servers=await self._session_mcp_servers(room_id), ) if not loaded: logger.info( @@ -778,7 +769,7 @@ async def _fetch_replay( raw = messages_before(context.get("data") or [], msg.id) return build_replay_messages([m for m in raw if m.get("id") != msg.id]) - async def _ensure_connection(self) -> ACPConnectionProtocol: - return await self._runtime.ensure_connection( + async def _ensure_connection(self, runtime: ACPRuntime) -> ACPConnectionProtocol: + return await runtime.ensure_connection( can_respawn=bool(self.agent_name), ) diff --git a/src/band/integrations/acp/client_profiles.py b/src/band/integrations/acp/client_profiles.py index 8e909b9fa..d8765cf9e 100644 --- a/src/band/integrations/acp/client_profiles.py +++ b/src/band/integrations/acp/client_profiles.py @@ -108,3 +108,16 @@ async def ext_notification( ] return [] + + +CURSOR_PROFILE_NAME = "cursor" + + +def resolve_acp_client_profile(profile_name: str) -> ACPClientProfile | None: + """Map a configured profile name to a runtime-specific ACP client profile.""" + normalized = profile_name.strip().lower() + if not normalized: + return None + if normalized == CURSOR_PROFILE_NAME: + return CursorACPClientProfile() + return None diff --git a/src/band/workspaces.py b/src/band/workspaces.py new file mode 100644 index 000000000..64d0a5d11 --- /dev/null +++ b/src/band/workspaces.py @@ -0,0 +1,54 @@ +"""Workspace selection for room-owned local coding agents.""" + +from __future__ import annotations + +import os +from collections.abc import Callable +from pathlib import Path + +WorkspaceResolver = Callable[[str], str] + +_DEFAULT_WORKSPACE_DIRECTORY = ".band-workspaces" + + +def resolve_room_workspace( + room_id: str, workspace_for_room: WorkspaceResolver | None +) -> str: + """Return a room's absolute workspace, creating the safe default on demand.""" + if workspace_for_room is not None: + workspace = workspace_for_room(room_id) + if not isinstance(workspace, str) or not os.path.isabs(workspace): + raise ValueError("workspace_for_room must return an absolute path") + return os.path.realpath(workspace) + + return create_room_workspace_resolver(Path.cwd() / _DEFAULT_WORKSPACE_DIRECTORY)( + room_id + ) + + +def claim_room_workspace( + room_id: str, + workspace: str, + workspace_rooms: dict[str, str], +) -> None: + """Record one room as the live owner of a resolved workspace path.""" + owner = workspace_rooms.get(workspace) + if owner is not None and owner != room_id: + raise ValueError( + f"workspace_for_room assigned {workspace!r} to both {owner!r} and {room_id!r}" + ) + workspace_rooms[workspace] = room_id + + +def create_room_workspace_resolver(root: str | Path) -> WorkspaceResolver: + """Build a resolver that creates an isolated child workspace per room.""" + workspace_root = Path(root).expanduser().resolve() + + def workspace_for_room(room_id: str) -> str: + workspace = (workspace_root / room_id).resolve() + if workspace_root not in workspace.parents: + raise ValueError("room id cannot escape the workspace root") + workspace.mkdir(parents=True, exist_ok=True) + return str(workspace) + + return workspace_for_room diff --git a/tests/adapters/test_codex_adapter.py b/tests/adapters/test_codex_adapter.py index b01b97dd4..a14c3f7e8 100644 --- a/tests/adapters/test_codex_adapter.py +++ b/tests/adapters/test_codex_adapter.py @@ -204,6 +204,37 @@ async def close(self) -> None: return None +def patch_codex_client(adapter: CodexAdapter, client: FakeCodexClient) -> None: + def _build(_config: CodexAdapterConfig) -> FakeCodexClient: + return client + + adapter._build_client = _build # type: ignore[method-assign] + + +def make_codex_adapter( + client: FakeCodexClient, + config: CodexAdapterConfig | None = None, + **kwargs: Any, +) -> CodexAdapter: + adapter = CodexAdapter(config=config or CodexAdapterConfig(), **kwargs) + patch_codex_client(adapter, client) + return adapter + + +def wire_codex_room( + adapter: CodexAdapter, + client: FakeCodexClient, + room_id: str = "room-1", + *, + initialized: bool = True, +) -> None: + adapter._room_client(room_id) + adapter._active_room.set(room_id) + adapter._client = client # type: ignore[assignment] + if initialized: + adapter._initialized = True + + def _event_notification(method: str, params: dict[str, Any]) -> RpcEvent: return RpcEvent( kind="notification", @@ -274,11 +305,7 @@ async def run_codex_turn( test states only the events it scripts and the outcome it asserts. """ client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=config or CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: client, - **adapter_kwargs, - ) + adapter = make_codex_adapter(client, config=config, **adapter_kwargs) room_tools = tools if tools is not None else ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -333,10 +360,7 @@ async def test_bootstrap_starts_thread_and_sends_fallback_message(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -379,9 +403,8 @@ async def test_system_prompt_retry_after_turn_start_failure(self) -> None: ), turn_start_error_once=True, ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", model="gpt-5.5"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(model="gpt-5.5") ) tools = ToolSchemaFakeTools() @@ -429,10 +452,7 @@ async def test_tool_call_request_is_dispatched_and_responded(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -499,10 +519,7 @@ async def execute_tool_call_structured( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = SendMessageFailureTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -529,10 +546,7 @@ async def test_resume_failure_falls_back_to_thread_start(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Not found"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -561,12 +575,8 @@ async def test_approval_request_auto_decline(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="auto_decline", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="auto_decline") ) tools = ToolSchemaFakeTools() @@ -606,13 +616,11 @@ async def send_message( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - approval_mode="auto_decline", - approval_text_notifications=True, + approval_mode="auto_decline", approval_text_notifications=True ), - client_factory=lambda _config: fake_client, ) tools = FailingNotifyTools() @@ -651,12 +659,8 @@ async def send_message( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="manual", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = FailingNotifyTools() @@ -680,10 +684,7 @@ async def send_message( @pytest.mark.asyncio async def test_cleanup_closes_client_when_last_room_removed(self) -> None: fake_client = FakeCodexClient(events=[_turn_completed()]) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -705,10 +706,7 @@ async def test_cleanup_closes_client_when_last_room_removed(self) -> None: async def test_cleanup_idempotent(self) -> None: """Calling on_cleanup twice for the same room should not raise.""" fake_client = FakeCodexClient(events=[_turn_completed()]) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -728,44 +726,40 @@ async def test_cleanup_idempotent(self) -> None: await adapter.on_cleanup("room-1") @pytest.mark.asyncio - async def test_cleanup_multi_room_keeps_client_until_last(self) -> None: - """Client stays open until the last room is cleaned up.""" - events_room1 = [_turn_completed()] - events_room2 = [_turn_completed("turn-2")] - fake_client = FakeCodexClient(events=events_room1 + events_room2) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + async def test_cleanup_multi_room_closes_each_room_client(self) -> None: + """Each room owns its Codex client; cleanup closes only that room's client.""" + clients = { + "room-1": FakeCodexClient(events=[_turn_completed()]), + "room-2": FakeCodexClient(events=[_turn_completed("turn-2")]), + } + adapter = CodexAdapter(config=CodexAdapterConfig()) + + def _build(_config: CodexAdapterConfig) -> FakeCodexClient: + room_id = adapter._active_room.get() + assert room_id is not None + return clients[room_id] + + adapter._build_client = _build # type: ignore[method-assign] tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") - await adapter.on_message( - make_platform_message(room_id="room-1"), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-1", - ) - await adapter.on_message( - make_platform_message(room_id="room-2"), - tools, - CodexSessionState(), - participants_msg=None, - contacts_msg=None, - is_session_bootstrap=True, - room_id="room-2", - ) + for room_id in ("room-1", "room-2"): + await adapter.on_message( + make_platform_message(room_id=room_id), + tools, + CodexSessionState(), + participants_msg=None, + contacts_msg=None, + is_session_bootstrap=True, + room_id=room_id, + ) - # Cleaning up room-1 should NOT close the client (room-2 still active) await adapter.on_cleanup("room-1") - assert fake_client.closed is False + assert clients["room-1"].closed is True + assert clients["room-2"].closed is False - # Cleaning up room-2 should close the client (last room) await adapter.on_cleanup("room-2") - assert fake_client.closed is True + assert clients["room-2"].closed is True @pytest.mark.asyncio async def test_forwards_raw_codex_task_events(self) -> None: @@ -781,10 +775,7 @@ async def test_forwards_raw_codex_task_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -828,12 +819,8 @@ async def test_can_disable_synthetic_turn_task_markers(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_task_markers=False, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_task_markers=False) ) tools = ToolSchemaFakeTools() @@ -871,12 +858,8 @@ async def test_raw_task_event_without_explicit_task_id_does_not_emit_uuid( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_task_markers=False, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_task_markers=False) ) tools = ToolSchemaFakeTools() @@ -905,10 +888,7 @@ async def test_raw_task_event_without_explicit_task_id_does_not_emit_uuid( @pytest.mark.asyncio async def test_status_command_returns_state_without_starting_turn(self) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -932,10 +912,7 @@ async def test_status_command_returns_state_without_starting_turn(self) -> None: @pytest.mark.asyncio async def test_model_command_sets_override_without_starting_turn(self) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -952,7 +929,7 @@ async def test_model_command_sets_override_without_starting_turn(self) -> None: methods = [method for method, _ in fake_client.requests] assert "turn/start" not in methods assert "thread/start" not in methods - assert adapter.config.model == "gpt-5.5-codex" + assert adapter._selected_model == "gpt-5.5-codex" assert len(tools.messages_sent) == 1 assert ( "Model override set to `gpt-5.5-codex`" in tools.messages_sent[0]["content"] @@ -961,10 +938,7 @@ async def test_model_command_sets_override_without_starting_turn(self) -> None: @pytest.mark.asyncio async def test_models_alias_lists_models_without_starting_turn(self) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1001,13 +975,11 @@ async def test_reasoning_effort_passed_in_turn_overrides(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - reasoning_effort="high", - reasoning_summary="concise", + reasoning_effort="high", reasoning_summary="concise" ), - client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1042,10 +1014,7 @@ async def test_reasoning_effort_omitted_when_none(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -1066,10 +1035,7 @@ async def test_reasoning_effort_omitted_when_none(self) -> None: @pytest.mark.asyncio async def test_reasoning_command_sets_effort(self) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -1081,17 +1047,15 @@ async def test_reasoning_command_sets_effort(self) -> None: is_session_bootstrap=True, room_id="room-1", ) - assert adapter.config.reasoning_effort == "high" + room = adapter._room_clients["room-1"] + assert room.reasoning_effort == "high" assert len(tools.messages_sent) == 1 assert "Reasoning effort set to `high`" in tools.messages_sent[0]["content"] @pytest.mark.asyncio async def test_reasoning_command_rejects_invalid_effort(self) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -1123,9 +1087,8 @@ async def test_self_config_tools_registered_when_enabled(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", enable_self_config_tools=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(enable_self_config_tools=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1164,9 +1127,8 @@ async def test_self_config_tools_not_registered_when_disabled(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", enable_self_config_tools=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(enable_self_config_tools=False) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1203,9 +1165,8 @@ async def test_setmodel_tool_changes_model(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", enable_self_config_tools=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(enable_self_config_tools=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1218,7 +1179,6 @@ async def test_setmodel_tool_changes_model(self) -> None: is_session_bootstrap=True, room_id="room-1", ) - assert adapter.config.model == "o3" assert adapter._selected_model == "o3" # Verify the tool response was sent back tool_responses = [ @@ -1245,9 +1205,8 @@ async def test_setreasoning_tool_changes_effort(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", enable_self_config_tools=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(enable_self_config_tools=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1260,8 +1219,9 @@ async def test_setreasoning_tool_changes_effort(self) -> None: is_session_bootstrap=True, room_id="room-1", ) - assert adapter.config.reasoning_effort == "xhigh" - assert adapter.config.reasoning_summary == "detailed" + room = adapter._room_clients["room-1"] + assert room.reasoning_effort == "xhigh" + assert room.reasoning_summary == "detailed" @pytest.mark.asyncio async def test_setreasoning_tool_rejects_invalid_effort(self) -> None: @@ -1278,9 +1238,8 @@ async def test_setreasoning_tool_rejects_invalid_effort(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", enable_self_config_tools=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(enable_self_config_tools=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -1309,12 +1268,8 @@ async def test_setreasoning_tool_rejects_invalid_effort(self) -> None: async def test_sandbox_alias_is_normalized_for_thread_and_turn(self) -> None: events = [_turn_completed()] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - sandbox="dangerFullAccess", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(sandbox="dangerFullAccess") ) tools = ToolSchemaFakeTools() @@ -1346,12 +1301,8 @@ async def test_sandbox_alias_is_normalized_for_thread_and_turn(self) -> None: async def test_external_sandbox_alias_uses_sandbox_policy(self) -> None: events = [_turn_completed()] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - sandbox="external-sandbox", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(sandbox="external-sandbox") ) tools = ToolSchemaFakeTools() @@ -1391,10 +1342,7 @@ async def test_transport_closed_event_aborts_turn(self) -> None: ) ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1424,10 +1372,7 @@ async def test_transport_closed_resets_client_state(self) -> None: ) ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1458,10 +1403,7 @@ async def test_transport_closed_clears_per_room_state(self) -> None: ) ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1499,10 +1441,7 @@ async def test_transport_closed_drains_token_usage_for_dead_threads( ) ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1532,9 +1471,8 @@ async def test_turn_timeout_sends_interrupt_and_clean_error(self) -> None: """When recv_event times out, the adapter sends turn/interrupt and reports cleanly.""" # No events means FakeCodexClient raises asyncio.TimeoutError immediately. fake_client = FakeCodexClient(events=[]) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", turn_timeout_s=0.01), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(turn_timeout_s=0.01) ) tools = ToolSchemaFakeTools() @@ -1585,10 +1523,7 @@ async def test_item_completed_text_overrides_accumulated_deltas(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1621,7 +1556,7 @@ def get_weather(inp: WeatherInput) -> str: custom_tools: list[CustomToolDef] = [(WeatherInput, get_weather)] adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), + config=CodexAdapterConfig(), additional_tools=custom_tools, ) @@ -1665,10 +1600,8 @@ async def calculate(inp: CalculatorInput) -> str: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - additional_tools=custom_tools, - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), additional_tools=custom_tools ) tools = ToolSchemaFakeTools() @@ -1709,10 +1642,8 @@ async def test_execution_reporting_emits_tool_call_and_result_events(self) -> No _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -1752,10 +1683,8 @@ async def test_send_room_file_tool_call_event_redacts_content(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -1793,11 +1722,7 @@ async def test_execution_reporting_silenced_with_explicit_empty_emit(self) -> No _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=(), - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(), emit=()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -1844,10 +1769,10 @@ async def fail_func(inp: FailInput) -> str: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), + adapter = make_codex_adapter( + fake_client, + config=CodexAdapterConfig(), additional_tools=custom_tools, - client_factory=lambda _config: fake_client, emit=Emit.TOOL_CALLS, ) tools = ToolSchemaFakeTools() @@ -1896,10 +1821,8 @@ async def test_execution_reporting_emitted_for_platform_output_tools( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -1960,10 +1883,8 @@ async def test_item_completed_mcpToolCall_send_room_file_redacts_content( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2007,10 +1928,8 @@ async def test_item_completed_commandExecution_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2063,10 +1982,8 @@ async def test_item_completed_fileChange_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2111,10 +2028,8 @@ async def test_item_completed_fileChange_missing_changes_is_safe(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2153,10 +2068,8 @@ async def test_item_completed_imageView_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2200,10 +2113,8 @@ async def test_item_completed_collabAgentToolCall_emits_tool_events(self) -> Non _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2251,10 +2162,7 @@ async def test_item_completed_collabAgentToolCall_non_text_list_result_preserves _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -2293,10 +2201,8 @@ async def test_item_completed_mcpToolCall_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2353,10 +2259,7 @@ async def test_item_completed_mcpToolCall_non_text_list_result_preserves_data( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -2396,10 +2299,8 @@ async def test_item_completed_dynamicToolCall_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2456,10 +2357,7 @@ async def test_item_completed_dynamicToolCall_non_text_list_result_falls_back_to _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -2498,10 +2396,8 @@ async def test_item_completed_reasoning_emits_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.THOUGHTS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.THOUGHTS ) tools = ToolSchemaFakeTools() @@ -2541,10 +2437,8 @@ async def test_item_completed_dict_summary_text_emits_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit={Emit.THOUGHTS}, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit={Emit.THOUGHTS} ) tools = ToolSchemaFakeTools() @@ -2601,10 +2495,8 @@ async def test_item_completed_empty_reasoning_summary_skips_thought(self) -> Non _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit={Emit.THOUGHTS}, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit={Emit.THOUGHTS} ) tools = ToolSchemaFakeTools() @@ -2637,10 +2529,8 @@ async def test_item_completed_empty_plan_text_skips_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit={Emit.THOUGHTS}, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit={Emit.THOUGHTS} ) tools = ToolSchemaFakeTools() @@ -2686,10 +2576,8 @@ async def test_item_completed_skipped_when_reporting_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TASK_EVENTS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TASK_EVENTS ) tools = ToolSchemaFakeTools() @@ -2740,10 +2628,8 @@ async def test_item_completed_agentMessage_still_sets_final_text(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2782,10 +2668,8 @@ async def test_item_completed_webSearch_emits_tool_events(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2826,10 +2710,7 @@ async def test_item_completed_webSearch_non_text_list_action_preserves_data( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -2866,10 +2747,8 @@ async def test_item_completed_metadata_includes_codex_ids(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - emit=Emit.TOOL_CALLS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS ) tools = ToolSchemaFakeTools() @@ -2901,10 +2780,7 @@ async def test_history_injected_on_resume_failure(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Thread expired"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -2961,10 +2837,7 @@ async def test_history_not_injected_on_successful_resume(self) -> None: """Resume succeeds, no history injection.""" events = [_turn_completed()] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -3010,12 +2883,9 @@ async def test_history_not_injected_when_disabled(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Thread expired"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - inject_history_on_resume_failure=False, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, + config=CodexAdapterConfig(inject_history_on_resume_failure=False), ) tools = ToolSchemaFakeTools() @@ -3054,10 +2924,7 @@ async def test_history_filters_non_text_messages(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Not found"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -3134,9 +3001,8 @@ async def test_history_respects_max_messages(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Not found"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", max_history_messages=3), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(max_history_messages=3) ) tools = ToolSchemaFakeTools() @@ -3193,10 +3059,7 @@ async def test_history_cleared_after_injection(self) -> None: events=events, resume_error=CodexJsonRpcError(code=-32002, message="Not found"), ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Codex Agent", "A coding agent") @@ -3240,10 +3103,7 @@ async def test_auto_selected_model_error_propagates_without_retry(self) -> None: ] }, ) - adapter = CodexAdapter( - config=CodexAdapterConfig(model=None), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(model=None)) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "An agent") @@ -3274,11 +3134,11 @@ async def test_model_selection_uses_first_visible_model(self) -> None: ] }, ) - adapter = CodexAdapter( - config=CodexAdapterConfig(model=None), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(model=None)) await adapter.on_started("Agent", "An agent") + adapter._room_client("room-1") + adapter._active_room.set("room-1") + await adapter._ensure_client_ready() assert adapter._selected_model == "gpt-5.4-mini" @@ -3298,9 +3158,8 @@ async def test_explicit_model_error_propagates_without_fallback(self) -> None: ] }, ) - adapter = CodexAdapter( - config=CodexAdapterConfig(model="unavailable-test-model"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(model="unavailable-test-model") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "An agent") @@ -3324,11 +3183,11 @@ async def test_explicit_model_error_propagates_without_fallback(self) -> None: async def test_model_selection_uses_default_when_model_list_empty(self) -> None: """Auto-selection uses the adapter default when Codex returns no visible models.""" fake_client = FakeCodexClient(model_list_result={"data": []}) - adapter = CodexAdapter( - config=CodexAdapterConfig(model=None), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(model=None)) await adapter.on_started("Agent", "An agent") + adapter._room_client("room-1") + adapter._active_room.set("room-1") + await adapter._ensure_client_ready() assert adapter._selected_model == "gpt-5.5" @@ -3351,11 +3210,11 @@ async def request( ) fake_client = ModelListFailsClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(model=None), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(model=None)) await adapter.on_started("Agent", "An agent") + adapter._room_client("room-1") + adapter._active_room.set("room-1") + await adapter._ensure_client_ready() assert adapter._selected_model == "gpt-5.5" @@ -3368,10 +3227,7 @@ async def test_non_model_error_propagates(self) -> None: ), turn_start_error_once=False, ) - adapter = CodexAdapter( - config=CodexAdapterConfig(model=None), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig(model=None)) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "An agent") @@ -3393,14 +3249,11 @@ async def test_startup_config_logged( ) -> None: """Startup emits a redacted config summary log line.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="stdio", - model="gpt-5.5", - sandbox="workspace-write", - approval_mode="manual", + model="gpt-5.5", sandbox="workspace-write", approval_mode="manual" ), - client_factory=lambda _config: fake_client, ) with caplog.at_level("INFO", logger="band.adapters.codex"): @@ -3432,10 +3285,7 @@ async def test_codex_error_emits_event_unconditionally(self) -> None: ), ], ) - adapter = CodexAdapter( - config=CodexAdapterConfig(), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "An agent") @@ -3457,9 +3307,9 @@ async def test_codex_error_emits_event_unconditionally(self) -> None: @pytest.mark.asyncio async def test_cleanup_before_start(self) -> None: """Calling on_cleanup on a freshly constructed adapter should not raise.""" - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="stdio"), - client_factory=lambda _config: FakeCodexClient(), + adapter = make_codex_adapter( + FakeCodexClient(), + config=CodexAdapterConfig(), ) # No on_started called — cleanup should be safe (idempotent) await adapter.on_cleanup("room-x") @@ -3468,10 +3318,7 @@ async def test_cleanup_before_start(self) -> None: async def test_cleanup_clears_pending_approvals(self) -> None: """on_cleanup should evict all pending approvals for the given room.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="stdio"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) await adapter.on_started("Bot", "desc") # Manually inject a pending approval for room-1 @@ -3490,9 +3337,8 @@ async def test_cleanup_clears_pending_approvals(self) -> None: }, )(), } - # Also register a room thread so the client isn't closed + wire_codex_room(adapter, fake_client, "room-1") adapter._room_threads["room-1"] = "thr-1" - adapter._room_threads["room-2"] = "thr-2" await adapter.on_cleanup("room-1") @@ -3525,10 +3371,7 @@ async def execute_tool_call_structured( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ValidationErrorTools() await adapter.on_started("Bot", "desc") @@ -3580,9 +3423,8 @@ async def test_structured_error_from_error_event(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(structured_errors=True) ) tools = ToolSchemaFakeTools() @@ -3628,9 +3470,8 @@ async def test_structured_error_from_failed_turn(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(structured_errors=True) ) tools = ToolSchemaFakeTools() @@ -3669,9 +3510,8 @@ async def test_structured_errors_disabled_falls_back_to_plain_text(self) -> None _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(structured_errors=False) ) tools = ToolSchemaFakeTools() @@ -3711,9 +3551,8 @@ async def test_approve_session_auto_approves_subsequent_requests(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=first_events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", approval_mode="manual"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -3765,12 +3604,8 @@ async def test_approval_audit_trail_emitted(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="auto_decline", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="auto_decline") ) tools = ToolSchemaFakeTools() @@ -3798,10 +3633,7 @@ async def test_approval_audit_trail_emitted(self) -> None: async def test_sandbox_command_changes_mode(self) -> None: """The /sandbox command sets a per-room override, not mutating global config.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -3823,10 +3655,7 @@ async def test_sandbox_command_changes_mode(self) -> None: async def test_sandbox_command_is_per_room(self) -> None: """Sandbox override in one room does not affect other rooms.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) await adapter.on_started("Agent", "A coding agent") adapter._sandbox_overrides["room-1"] = "read-only" @@ -3838,10 +3667,7 @@ async def test_sandbox_command_is_per_room(self) -> None: async def test_sandbox_danger_full_access_requires_confirm_flag(self) -> None: """Escalating to danger-full-access without --confirm shows a prompt.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -3865,10 +3691,7 @@ async def test_sandbox_escalation_to_danger_full_access_logs_warning( ) -> None: """Escalating to danger-full-access with --confirm logs a warning.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") with caplog.at_level(logging.WARNING, logger="band.adapters.codex"): @@ -3892,10 +3715,7 @@ async def test_sandbox_escalation_to_danger_full_access_logs_warning( async def test_sandbox_command_rejects_invalid_mode(self) -> None: """The /sandbox command rejects invalid modes.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -3916,10 +3736,7 @@ async def test_sandbox_command_rejects_invalid_mode(self) -> None: async def test_permissions_command_shows_state(self) -> None: """/permissions shows current effective permissions.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -3961,9 +3778,8 @@ async def test_plan_steps_forwarded(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_plan_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_plan_events=True) ) tools = ToolSchemaFakeTools() @@ -4007,9 +3823,8 @@ async def test_plan_steps_not_forwarded_when_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_plan_events=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_plan_events=False) ) tools = ToolSchemaFakeTools() @@ -4038,12 +3853,8 @@ async def test_turn_lifecycle_events_emitted(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_lifecycle_events=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_lifecycle_events=True) ) tools = ToolSchemaFakeTools() @@ -4078,10 +3889,7 @@ async def test_threads_command_lists_mappings(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4122,10 +3930,7 @@ async def test_thread_archive_clears_mapping(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4170,9 +3975,8 @@ async def test_reasoning_delta_streamed_as_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_reasoning_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_reasoning_events=True) ) tools = ToolSchemaFakeTools() @@ -4207,9 +4011,8 @@ async def test_reasoning_delta_ignored_when_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_reasoning_events=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_reasoning_events=False) ) tools = ToolSchemaFakeTools() @@ -4240,9 +4043,8 @@ async def test_plan_delta_streamed_as_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_plan_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_plan_events=True) ) tools = ToolSchemaFakeTools() @@ -4288,9 +4090,8 @@ async def test_commentary_phase_streamed_as_thought(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_commentary_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_commentary_events=True) ) tools = ToolSchemaFakeTools() @@ -4339,9 +4140,8 @@ async def test_commentary_excluded_from_final_text_when_streaming_enabled( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_commentary_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_commentary_events=True) ) tools = ToolSchemaFakeTools() @@ -4385,9 +4185,8 @@ async def test_commentary_included_in_final_text_when_streaming_disabled( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_commentary_events=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_commentary_events=False) ) tools = ToolSchemaFakeTools() @@ -4427,12 +4226,8 @@ async def test_diff_event_forwarded(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_diff_events=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_diff_events=True) ) tools = ToolSchemaFakeTools() @@ -4469,13 +4264,8 @@ async def test_diff_event_requires_task_events_emit(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_diff_events=True, - ), - client_factory=lambda _config: fake_client, - emit=(), + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_diff_events=True), emit=() ) tools = ToolSchemaFakeTools() @@ -4515,9 +4305,8 @@ async def test_token_usage_tracked_and_emitted(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", emit_token_usage_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_token_usage_events=True) ) tools = ToolSchemaFakeTools() @@ -4559,9 +4348,8 @@ async def test_token_usage_ignored_when_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", emit_token_usage_events=False), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_token_usage_events=False) ) tools = ToolSchemaFakeTools() @@ -4608,10 +4396,7 @@ async def test_usage_command_shows_token_usage(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4837,12 +4622,8 @@ async def test_session_auto_approves_matching_command_binary(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="manual", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4881,12 +4662,8 @@ async def test_session_does_not_auto_approve_different_command(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="auto_decline", - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="auto_decline") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4922,13 +4699,11 @@ async def test_session_binary_granularity_approves_same_binary(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - approval_mode="manual", - session_approval_granularity="binary", + approval_mode="manual", session_approval_granularity="binary" ), - client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -4974,10 +4749,7 @@ async def test_on_cleanup_removes_per_room_token_usage(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5006,9 +4778,9 @@ async def test_on_cleanup_removes_per_room_token_usage(self) -> None: class TestAuditCap: def test_audit_trail_capped_at_limit(self) -> None: """Approval audit trail is capped at max_approval_audit_per_room.""" - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", max_approval_audit_per_room=5), - client_factory=lambda _config: FakeCodexClient(), + adapter = make_codex_adapter( + FakeCodexClient(), + config=CodexAdapterConfig(max_approval_audit_per_room=5), ) for i in range(10): adapter._record_approval_audit( @@ -5026,9 +4798,9 @@ def test_audit_trail_capped_at_limit(self) -> None: def test_session_approved_capped_at_limit(self) -> None: """Session approvals evict LRU when max_session_approved_per_room is hit.""" - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", max_session_approved_per_room=3), - client_factory=lambda _config: FakeCodexClient(), + adapter = make_codex_adapter( + FakeCodexClient(), + config=CodexAdapterConfig(max_session_approved_per_room=3), ) for i in range(5): adapter._record_session_approval("room-1", f"commandExecution:cmd{i}") @@ -5041,9 +4813,9 @@ def test_session_approved_capped_at_limit(self) -> None: def test_session_approval_reinsert_moves_to_end(self) -> None: """Re-approving an existing key moves it to the most-recent slot.""" - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", max_session_approved_per_room=3), - client_factory=lambda _config: FakeCodexClient(), + adapter = make_codex_adapter( + FakeCodexClient(), + config=CodexAdapterConfig(max_session_approved_per_room=3), ) adapter._record_session_approval("room-1", "commandExecution:a") adapter._record_session_approval("room-1", "commandExecution:b") @@ -5068,12 +4840,8 @@ class TestReviewFixes: async def test_sandbox_command_blocked_when_sandbox_policy_set(self) -> None: """/sandbox is rejected when sandbox_policy is configured.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - sandbox_policy={"type": "readOnly"}, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(sandbox_policy={"type": "readOnly"}) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5097,10 +4865,7 @@ async def test_thread_archive_clears_raw_history(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5167,12 +4932,8 @@ async def recv_event(self, timeout_s: float | None = None) -> RpcEvent: raise ConnectionError("transport died") fake_client = BrokenClient() - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - fallback_send_agent_text=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(fallback_send_agent_text=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5208,9 +4969,8 @@ async def test_approve_session_sends_accept_for_session_decision(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", approval_mode="manual"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5260,9 +5020,8 @@ async def test_session_auto_approval_sends_accept_for_session(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", approval_mode="manual"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5305,9 +5064,8 @@ async def test_network_context_included_in_approval_metadata(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", approval_mode="manual"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5357,12 +5115,8 @@ async def test_turn_started_lifecycle_event_emitted(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_lifecycle_events=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_lifecycle_events=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5400,12 +5154,8 @@ async def test_context_compaction_event_emitted(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_lifecycle_events=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_lifecycle_events=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5438,12 +5188,8 @@ async def test_context_compaction_ignored_when_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_lifecycle_events=False, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_lifecycle_events=False) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5625,9 +5371,8 @@ async def test_approve_session_rejects_empty_session_key( _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", approval_mode="manual"), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(approval_mode="manual") ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5697,10 +5442,7 @@ async def test_token_usage_event_skipped_when_total_is_zero(self) -> None: """ fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5744,7 +5486,7 @@ class TestSessionApprovalKeying: def test_file_change_session_key_requires_paths(self) -> None: """/approve-session must refuse fileChange requests with no paths.""" - adapter = CodexAdapter(config=CodexAdapterConfig(transport="ws")) + adapter = CodexAdapter(config=CodexAdapterConfig()) key = adapter._session_approval_key( "item/fileChange/requestApproval", {"reason": "something vague"}, @@ -5753,7 +5495,7 @@ def test_file_change_session_key_requires_paths(self) -> None: def test_file_change_session_key_uses_paths_when_present(self) -> None: """fileChange session key includes sorted path list for stable matching.""" - adapter = CodexAdapter(config=CodexAdapterConfig(transport="ws")) + adapter = CodexAdapter(config=CodexAdapterConfig()) key1 = adapter._session_approval_key( "item/fileChange/requestApproval", {"changes": [{"path": "b.py"}, {"path": "a.py"}]}, @@ -5767,7 +5509,7 @@ def test_file_change_session_key_uses_paths_when_present(self) -> None: def test_file_change_session_key_handles_top_level_paths(self) -> None: """fileChange session key also picks up top-level path/paths fields.""" - adapter = CodexAdapter(config=CodexAdapterConfig(transport="ws")) + adapter = CodexAdapter(config=CodexAdapterConfig()) key = adapter._session_approval_key( "item/fileChange/requestApproval", {"paths": ["src/foo.py", "src/bar.py"]}, @@ -5777,7 +5519,7 @@ def test_file_change_session_key_handles_top_level_paths(self) -> None: def test_unknown_approval_method_returns_empty_key(self) -> None: """Session-level approval refuses unknown methods rather than bucketing them.""" - adapter = CodexAdapter(config=CodexAdapterConfig(transport="ws")) + adapter = CodexAdapter(config=CodexAdapterConfig()) assert adapter._session_approval_key("item/unknown/requestApproval", {}) == "" @pytest.mark.asyncio @@ -5791,14 +5533,13 @@ async def test_approve_session_refused_for_fileChange_without_paths(self) -> Non ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", approval_mode="manual", approval_wait_timeout_s=0.05, approval_timeout_decision="decline", ), - client_factory=lambda _config: fake_client, ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -5865,7 +5606,7 @@ class TestApprovalAuditRecording: def test_record_approval_audit_returns_entry(self) -> None: """_record_approval_audit returns the entry it appended.""" - adapter = CodexAdapter(config=CodexAdapterConfig(transport="ws")) + adapter = CodexAdapter(config=CodexAdapterConfig()) entry = adapter._record_approval_audit( room_id="room-1", request_id="req-1", @@ -5944,10 +5685,7 @@ class TestSlashCommandCoverage: async def test_thread_info_with_no_mapping(self) -> None: """/thread info reports gracefully when the room has no thread yet.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -5980,12 +5718,8 @@ async def test_thread_info_includes_thread_and_usage(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_token_usage_events=True, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_token_usage_events=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -6019,10 +5753,7 @@ async def test_thread_info_includes_thread_and_usage(self) -> None: async def test_permissions_reflects_sandbox_override(self) -> None: """/permissions reports the per-room sandbox override once set.""" fake_client = FakeCodexClient() - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") await adapter.on_message( @@ -6062,9 +5793,8 @@ async def test_error_event_with_non_dict_error_field(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", structured_errors=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(structured_errors=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -6089,10 +5819,7 @@ async def test_turn_completed_without_items_key(self) -> None: ), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _config: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -6117,9 +5844,8 @@ async def test_turn_plan_updated_with_garbage_steps(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", stream_plan_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(stream_plan_events=True) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -6146,13 +5872,11 @@ async def test_pending_approvals_cleared_on_room_cleanup(self) -> None: not that the natural approval timeout fired first. """ fake_client = FakeCodexClient(events=[]) - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - approval_mode="manual", - approval_wait_timeout_s=30.0, + approval_mode="manual", approval_wait_timeout_s=30.0 ), - client_factory=lambda _config: fake_client, ) await adapter.on_started("Agent", "A coding agent") @@ -6160,6 +5884,7 @@ async def test_pending_approvals_cleared_on_room_cleanup(self) -> None: loop = asyncio.get_running_loop() approval_future: asyncio.Future[str] = loop.create_future() + wire_codex_room(adapter, fake_client, "room-1") adapter._room_threads["room-1"] = "thr-1" adapter._pending_approvals["room-1"] = { "token-1": PendingApproval( @@ -6190,12 +5915,8 @@ async def test_no_lifecycle_events_when_disabled(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - emit_turn_lifecycle_events=False, - ), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_turn_lifecycle_events=False) ) tools = ToolSchemaFakeTools() await adapter.on_started("Agent", "A coding agent") @@ -6356,9 +6077,8 @@ async def test_multibyte_diff_respects_byte_budget(self) -> None: _turn_completed(), ] fake_client = FakeCodexClient(events=events) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws", emit_diff_events=True), - client_factory=lambda _config: fake_client, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(emit_diff_events=True) ) tools = ToolSchemaFakeTools() @@ -6444,13 +6164,11 @@ async def test_warns_when_both_channels_enabled( self, caplog: pytest.LogCaptureFixture ) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - emit_turn_task_markers=True, - emit_turn_lifecycle_events=True, + emit_turn_task_markers=True, emit_turn_lifecycle_events=True ), - client_factory=lambda _config: fake_client, ) with caplog.at_level(logging.WARNING, logger="band.adapters.codex"): await adapter.on_started("Agent", "A coding agent") @@ -6463,13 +6181,11 @@ async def test_no_warning_when_only_one_channel_enabled( self, caplog: pytest.LogCaptureFixture ) -> None: fake_client = FakeCodexClient() - adapter = CodexAdapter( + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig( - transport="ws", - emit_turn_task_markers=True, - emit_turn_lifecycle_events=False, + emit_turn_task_markers=True, emit_turn_lifecycle_events=False ), - client_factory=lambda _config: fake_client, ) with caplog.at_level(logging.WARNING, logger="band.adapters.codex"): await adapter.on_started("Agent", "A coding agent") diff --git a/tests/adapters/test_copilot_acp_adapter.py b/tests/adapters/test_copilot_acp_adapter.py index 6433f2315..71037be7d 100644 --- a/tests/adapters/test_copilot_acp_adapter.py +++ b/tests/adapters/test_copilot_acp_adapter.py @@ -29,12 +29,10 @@ def test_is_acp_client_adapter(self) -> None: def test_defaults_to_stdio_copilot_command(self) -> None: adapter = CopilotACPAdapter() assert adapter._command == list(DEFAULT_COPILOT_COMMAND) - assert adapter._host is None - assert adapter._port is None def test_no_config_equivalent_to_default_config(self) -> None: a, b = CopilotACPAdapter(), CopilotACPAdapter(CopilotACPAdapterConfig()) - for attr in ("_command", "_host", "_port", "_env", "_inject_band_tools"): + for attr in ("_command", "_env", "_inject_band_tools"): assert getattr(a, attr) == getattr(b, attr) def test_custom_command_is_forwarded(self) -> None: @@ -48,7 +46,7 @@ def test_no_profile_uses_default_noop(self) -> None: # collecting client the runtime builds falls back to the no-op profile. adapter = CopilotACPAdapter() assert adapter._profile is None - client = adapter._runtime._client_factory() + client = adapter._build_runtime()._client_factory() assert isinstance(client._profile, NoopACPClientProfile) def test_github_token_injected_into_stdio_env(self) -> None: @@ -109,48 +107,14 @@ def _echo(text: str) -> str: class TestCopilotACPAdapterTcpTransport: - def test_host_port_selects_tcp_and_empty_command(self) -> None: - adapter = CopilotACPAdapter(CopilotACPAdapterConfig(host="10.0.0.5", port=8080)) - assert adapter._host == "10.0.0.5" - assert adapter._port == 8080 - assert adapter._command == [] - - def test_tcp_does_not_inject_env(self) -> None: - # Over TCP the already-running server carries its own environment; neither - # the token nor a general env is smuggled through (they'd be ignored anyway). - adapter = CopilotACPAdapter( - CopilotACPAdapterConfig( - host="10.0.0.5", - port=8080, - github_token="ghp_x", - env={"COPILOT_GITHUB_TOKEN": "tok"}, - ) - ) - assert adapter._env is None - - def test_tcp_with_auth_warns_it_is_ignored( - self, caplog: pytest.LogCaptureFixture - ) -> None: - # Symmetric with the loud command+TCP error: dropping auth over TCP is - # surfaced, not silent, so a caller can't believe auth is configured. - with caplog.at_level(logging.WARNING, logger="band.adapters.copilot_acp"): - CopilotACPAdapter( - CopilotACPAdapterConfig( - host="10.0.0.5", port=8080, github_token="ghp_x" - ) - ) - assert any("ignored over TCP" in r.message for r in caplog.records) - - def test_tcp_without_auth_does_not_warn( - self, caplog: pytest.LogCaptureFixture - ) -> None: - with caplog.at_level(logging.WARNING, logger="band.adapters.copilot_acp"): + def test_tcp_config_is_rejected(self) -> None: + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): CopilotACPAdapter(CopilotACPAdapterConfig(host="10.0.0.5", port=8080)) - assert not any("ignored over TCP" in r.message for r in caplog.records) def test_custom_command_with_tcp_is_rejected(self) -> None: - # A non-default command AND host/port is a misconfiguration; fail loudly - # rather than silently dropping the command. with pytest.raises(ValueError, match="not both"): CopilotACPAdapter( CopilotACPAdapterConfig( @@ -158,7 +122,18 @@ def test_custom_command_with_tcp_is_rejected(self) -> None: ) ) - def test_default_command_with_tcp_is_allowed(self) -> None: - # The default command is not "set" for exclusivity purposes — TCP is fine. - adapter = CopilotACPAdapter(CopilotACPAdapterConfig(host="10.0.0.5", port=8080)) - assert adapter._host == "10.0.0.5" + def test_tcp_with_auth_warns_before_rejection( + self, caplog: pytest.LogCaptureFixture + ) -> None: + # CopilotACPAdapter warns about ignored auth before the base adapter rejects TCP. + with caplog.at_level(logging.WARNING, logger="band.adapters.copilot_acp"): + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): + CopilotACPAdapter( + CopilotACPAdapterConfig( + host="10.0.0.5", port=8080, github_token="ghp_x" + ) + ) + assert any("ignored over TCP" in r.message for r in caplog.records) diff --git a/tests/adapters/test_room_workspace_isolation.py b/tests/adapters/test_room_workspace_isolation.py new file mode 100644 index 000000000..b1bcf8bca --- /dev/null +++ b/tests/adapters/test_room_workspace_isolation.py @@ -0,0 +1,96 @@ +"""Room workspace isolation for coding-agent adapters.""" + +from __future__ import annotations + +from pathlib import Path +from typing import cast + +import pytest + +from band.adapters.codex import CodexAdapter, CodexAdapterConfig, CodexSessionState +from band.core.protocols import AgentToolsProtocol +from band.integrations.acp.client_adapter import ACPClientAdapter +from band.testing import FakeAgentTools +from band.workspaces import resolve_room_workspace + + +def test_default_workspace_is_created_per_room( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + monkeypatch.chdir(tmp_path) + + first = resolve_room_workspace("room-a", None) + second = resolve_room_workspace("room-b", None) + + assert [first, second] == [ + str(tmp_path / ".band-workspaces" / "room-a"), + str(tmp_path / ".band-workspaces" / "room-b"), + ] + assert Path(first).is_dir() + assert Path(second).is_dir() + + +@pytest.mark.asyncio +async def test_adapters_reject_a_custom_workspace_shared_by_live_rooms() -> None: + def resolver(_room_id: str) -> str: + return "/workspace" + + codex = CodexAdapter(CodexAdapterConfig(workspace_for_room=resolver)) + acp = ACPClientAdapter(command="codex", workspace_for_room=resolver) + + codex._room_client("room-a") + await acp._runtime_for("room-a") + + with pytest.raises(ValueError, match="both 'room-a' and 'room-b'"): + codex._room_client("room-b") + with pytest.raises(ValueError, match="both 'room-a' and 'room-b'"): + await acp._runtime_for("room-b") + + +def test_codex_rejects_the_former_shared_cwd_option() -> None: + with pytest.raises(ValueError, match="workspace_for_room or the default"): + CodexAdapter(CodexAdapterConfig(cwd="/workspace")) + + +@pytest.mark.asyncio +async def test_codex_starts_each_thread_in_its_room_workspace() -> None: + class Client: + def __init__(self) -> None: + self.params: dict[str, object] | None = None + + async def request( + self, method: str, params: dict[str, object] + ) -> dict[str, object]: + assert method == "thread/start" + self.params = params + return {"thread": {"id": "thread"}} + + def workspace_for_room(room_id: str) -> str: + return f"/workspace/{room_id}" + + adapter = CodexAdapter( + CodexAdapterConfig( + model="gpt-5.5", + workspace_for_room=workspace_for_room, + ) + ) + tools = cast(AgentToolsProtocol, FakeAgentTools()) + clients: list[Client] = [] + for room_id in ("room-a", "room-b"): + adapter._room_client(room_id) + adapter._active_room.set(room_id) + client = Client() + adapter._client = client # type: ignore[assignment] + adapter._selected_model = "gpt-5.5" + clients.append(client) + await adapter._ensure_thread( + room_id=room_id, + history=CodexSessionState(), + tools=tools, + is_session_bootstrap=False, + ) + + assert [client.params["cwd"] for client in clients if client.params] == [ + resolve_room_workspace("room-a", workspace_for_room), + resolve_room_workspace("room-b", workspace_for_room), + ] diff --git a/tests/integrations/acp/acp_toolkit/__init__.py b/tests/integrations/acp/acp_toolkit/__init__.py index 10d2f9d8e..c4ef7b275 100644 --- a/tests/integrations/acp/acp_toolkit/__init__.py +++ b/tests/integrations/acp/acp_toolkit/__init__.py @@ -30,6 +30,7 @@ RoomActivity, TranscriptTools, acp_adapter, + inject_acp_spawn, live_line, make_acp_connection, ) @@ -43,6 +44,7 @@ "RoomActivity", "TranscriptTools", "acp_adapter", + "inject_acp_spawn", "live_line", "make_acp_connection", ] diff --git a/tests/integrations/acp/acp_toolkit/harness.py b/tests/integrations/acp/acp_toolkit/harness.py index 1b914eda0..e4d1ca5f3 100644 --- a/tests/integrations/acp/acp_toolkit/harness.py +++ b/tests/integrations/acp/acp_toolkit/harness.py @@ -17,8 +17,9 @@ from acp.agent.connection import AgentSideConnection from band.core.types import PlatformMessage -from band.integrations.acp.client_adapter import ACPClientAdapter -from band.integrations.acp.client_types import ACPClientSessionState +from band.integrations.acp.client_adapter import ACPClientAdapter, _resolve_launcher +from band.integrations.acp.client_runtime import ACPRuntime +from band.integrations.acp.client_types import ACPClientSessionState, BandACPClient from band.integrations.acp.types import ToolCallRoomEvent, ToolResultRoomEvent from band.testing import FakeAgentTools @@ -104,7 +105,7 @@ def make_acp_connection(*, http: bool = True, sse: bool = False) -> AsyncMock: class FakeSpawn: """A fake ``spawn_process`` seam: records calls (spy) and yields a scripted conn. - Drop-in for the injectable ``spawn_process`` on ``ACPClientAdapter``/``ACPRuntime`` + Drop-in for the injectable ``spawn_process`` on ``ACPRuntime`` so tests exercise the real transport seam by dependency injection instead of patching module globals. The instance *is* the callable and returns an async context manager, matching the runtime's contract: @@ -131,6 +132,26 @@ def last_kwargs(self) -> dict[str, Any]: return self.calls[-1][1] +def inject_acp_spawn( + adapter: ACPClientAdapter, spawn: FakeSpawn | Callable[..., Any] +) -> None: + """Patch ``adapter._build_runtime`` so each room runtime uses ``spawn``.""" + + def _build_runtime() -> ACPRuntime: + return ACPRuntime( + command=_resolve_launcher(adapter._command), + env=adapter._env, + auth_method=adapter._auth_method, + client_factory=lambda: BandACPClient( + profile=adapter._profile, + canonicalize_tool_name=adapter._canonical_tool_name, + ), + spawn_process=spawn, + ) + + adapter._build_runtime = _build_runtime # type: ignore[method-assign] + + @dataclass class Reply: """A readable view of what the adapter posted back for one turn.""" @@ -267,10 +288,10 @@ async def acp_adapter( """ adapter = ACPClientAdapter( command="fake-agent", # ignored — the injected transport pairs us with agent - spawn_process=_pair_in_process(agent), inject_band_tools=inject_band_tools, **adapter_kwargs, ) + inject_acp_spawn(adapter, _pair_in_process(agent)) await adapter.on_started("Fake Agent", "in-process fake") try: yield AcpSession(adapter, agent) diff --git a/tests/integrations/acp/test_client_adapter.py b/tests/integrations/acp/test_client_adapter.py index 8e912665e..b9ae7ef10 100644 --- a/tests/integrations/acp/test_client_adapter.py +++ b/tests/integrations/acp/test_client_adapter.py @@ -3,7 +3,6 @@ from __future__ import annotations import asyncio -import os from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -22,8 +21,11 @@ from band.integrations.acp.types import ACPToolCall, ACPToolResult, CollectedChunk from band.testing import FakeAgentTools +from tests.integrations.acp.acp_toolkit.harness import inject_acp_spawn from tests.integrations.acp.conftest import make_platform_message +_MOCK_ROOM = "room-123" + def permission_events(tools: FakeAgentTools) -> list[dict[str, object]]: """The permission tool_call/tool_result events the handler posted to the room.""" @@ -65,34 +67,26 @@ def test_init_list_command(self) -> None: def test_init_default_values(self) -> None: """Should initialize with default values.""" adapter = ACPClientAdapter(command="codex") - assert adapter._cwd == os.path.abspath(".") assert adapter._env is None assert adapter._mcp_servers == [] - assert adapter._runtime._conn is None - assert adapter._runtime._client is None + assert adapter._runtimes == {} + assert adapter._room_workspaces == {} assert adapter._room_to_session == {} assert adapter._room_tools == {} assert adapter._band_mcp_backend is None - def test_init_codex_acp_uses_absolute_default_cwd(self) -> None: - """Should normalize codex-acp default cwd to an absolute path.""" - adapter = ACPClientAdapter(command="codex-acp") - assert adapter._cwd == os.path.abspath(".") - - def test_init_npx_codex_acp_uses_absolute_default_cwd(self) -> None: - """Should normalize npx codex-acp default cwd to an absolute path.""" - adapter = ACPClientAdapter(command=["npx", "@zed-industries/codex-acp"]) - assert adapter._cwd == os.path.abspath(".") + def test_init_cwd_is_rejected(self) -> None: + """Per-room workspaces replaced the adapter-wide cwd knob.""" + with pytest.raises(ValueError, match="cwd is not supported"): + ACPClientAdapter(command="codex", cwd="/workspace") def test_init_with_custom_values(self) -> None: """Should accept custom configuration.""" adapter = ACPClientAdapter( command="codex", env={"API_KEY": "test"}, - cwd="/workspace", mcp_servers=[{"type": "stdio", "command": "server"}], ) - assert adapter._cwd == os.path.abspath("/workspace") assert adapter._env == {"API_KEY": "test"} assert len(adapter._mcp_servers) == 1 @@ -101,64 +95,74 @@ def test_init_sets_history_converter(self) -> None: adapter = ACPClientAdapter(command="codex") assert adapter.history_converter is not None - def test_init_resolves_custom_cwd_to_absolute_path(self) -> None: - """Should normalize explicit cwd values to absolute paths.""" - adapter = ACPClientAdapter(command="codex", cwd="examples") - assert adapter._cwd == os.path.abspath("examples") - class TestACPClientAdapterTransport: - """Tests for stdio-vs-TCP transport selection and validation.""" + """Tests for stdio transport validation and rejected legacy knobs.""" - def test_tcp_construction_sets_host_port_and_empty_command(self) -> None: - """TCP transport records host/port and spawns no subprocess command.""" - adapter = ACPClientAdapter(host="10.0.0.5", port=8080) - assert adapter._host == "10.0.0.5" - assert adapter._port == 8080 - assert adapter._command == [] - - def test_stdio_construction_leaves_host_port_unset(self) -> None: + def test_stdio_construction(self) -> None: adapter = ACPClientAdapter(command="copilot") - assert adapter._host is None - assert adapter._port is None assert adapter._command == ["copilot"] - def test_requires_a_transport(self) -> None: - """Neither command nor host/port is a misconfiguration.""" - with pytest.raises(ValueError, match="command .*or host"): + def test_requires_command(self) -> None: + with pytest.raises(ValueError, match="ACP stdio transport requires a command"): ACPClientAdapter() def test_empty_command_is_rejected(self) -> None: - """An empty command is not a usable transport (would crash at spawn).""" - with pytest.raises(ValueError, match="command .*or host"): + with pytest.raises(ValueError, match="ACP stdio transport requires a command"): ACPClientAdapter(command=[]) - with pytest.raises(ValueError, match="command .*or host"): + with pytest.raises(ValueError, match="ACP stdio transport requires a command"): ACPClientAdapter(command="") - def test_rejects_command_and_tcp_together(self) -> None: - with pytest.raises(ValueError, match="not both"): - ACPClientAdapter(command="copilot", host="10.0.0.5", port=8080) + def test_rejects_tcp_transport(self) -> None: + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): + ACPClientAdapter(host="10.0.0.5", port=8080) - def test_tcp_requires_both_host_and_port(self) -> None: - with pytest.raises(ValueError, match="both host and port"): + def test_rejects_partial_tcp_config(self) -> None: + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): ACPClientAdapter(host="10.0.0.5") - with pytest.raises(ValueError, match="both host and port"): + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): ACPClientAdapter(port=8080) + def test_rejects_command_and_tcp_together(self) -> None: + with pytest.raises( + ValueError, + match="TCP ACP transport cannot guarantee room process isolation", + ): + ACPClientAdapter(command="copilot", host="10.0.0.5", port=8080) + + def test_rejects_spawn_process_constructor(self) -> None: + with pytest.raises( + ValueError, + match="custom ACP transports cannot guarantee room process isolation", + ): + ACPClientAdapter(command="codex", spawn_process=object()) + @pytest.mark.asyncio - async def test_injected_spawn_process_wins_over_defaults( + async def test_injected_spawn_used_on_connection_start( self, make_acp_transport ) -> None: - """An explicit spawn_process is used even for a TCP-configured adapter.""" + """FakeSpawn patched onto _build_runtime is used when a room connects.""" transport = make_acp_transport() - adapter = ACPClientAdapter(host="10.0.0.5", port=8080, spawn_process=transport) - - await adapter.on_started("Copilot", "Copilot over TCP") - - assert adapter._runtime._conn is transport.conn - # TCP still forwards no positional command. + with patch( + "band.integrations.acp.client_adapter.shutil.which", return_value=None + ): + adapter = ACPClientAdapter(command="codex") + inject_acp_spawn(adapter, transport) + await adapter.on_started("Codex", "Codex bridge") + runtime = await adapter._runtime_for("room-1") + await runtime.start() + assert runtime._conn is transport.conn args, _ = transport.last_call - assert args == () + assert args == ("codex",) class TestACPClientAdapterShutdown: @@ -172,16 +176,18 @@ class TestACPClientAdapterShutdown: async def test_cleanup_all_tears_down_the_transport( self, make_acp_transport ) -> None: - adapter = ACPClientAdapter( - command="codex", spawn_process=make_acp_transport(), inject_band_tools=False - ) + transport = make_acp_transport() + adapter = ACPClientAdapter(command="codex", inject_band_tools=False) + inject_acp_spawn(adapter, transport) await adapter.on_started("Codex", "bridge") - assert adapter._runtime._ctx is not None # transport is up + runtime = await adapter._runtime_for("room-1") + await runtime.start() + assert runtime._ctx is not None # transport is up await adapter.cleanup_all() # the hook Agent.stop() calls on graceful shutdown - assert adapter._runtime._ctx is None # ...and released - assert adapter._runtime._conn is None + assert runtime._ctx is None # ...and released + assert runtime._conn is None @pytest.mark.asyncio async def test_restart_after_a_full_stop_allows_backend_creation( @@ -193,8 +199,11 @@ async def test_restart_after_a_full_stop_allows_backend_creation( connection self-heals unconditionally; the MCP backend must too, or a perfectly healthy restarted adapter can never call a Band tool again.""" transport = make_acp_transport() - adapter = ACPClientAdapter(command="codex", spawn_process=transport) + adapter = ACPClientAdapter(command="codex") + inject_acp_spawn(adapter, transport) await adapter.on_started("Codex", "bridge") + runtime = await adapter._runtime_for("room-1") + await runtime.start() await adapter.cleanup_all() # Agent.stop(), final=True @@ -222,7 +231,7 @@ async def test_get_or_start_band_mcp_server_returns_http_config(self) -> None: "band.integrations.acp.client_adapter.create_band_mcp_backend", new=AsyncMock(return_value=backend), ): - server = await adapter._get_or_start_band_mcp_server() + server = await adapter._get_or_start_band_mcp_server("room-1") assert server.name == "band" assert server.url == "http://127.0.0.1:50000/mcp" @@ -235,7 +244,11 @@ async def test_get_or_start_band_mcp_server_returns_http_config(self) -> None: async def test_get_or_start_band_mcp_server_returns_sse_config(self) -> None: """Should expose shared SSE when the ACP agent only supports SSE MCP.""" adapter = ACPClientAdapter(command="codex") - adapter._runtime._agent_mcp_transport = "sse" + runtime = adapter._build_runtime() + runtime._agent_mcp_transport = "sse" + adapter._runtimes["room-1"] = runtime + adapter._room_workspaces["room-1"] = "/tmp/room-1" + adapter._workspace_rooms["/tmp/room-1"] = "room-1" mock_server = MagicMock(sse_url="http://127.0.0.1:50000/sse") backend = MagicMock(local_server=mock_server) @@ -243,7 +256,7 @@ async def test_get_or_start_band_mcp_server_returns_sse_config(self) -> None: "band.integrations.acp.client_adapter.create_band_mcp_backend", new=AsyncMock(return_value=backend), ): - server = await adapter._get_or_start_band_mcp_server() + server = await adapter._get_or_start_band_mcp_server("room-1") assert server.name == "band" assert server.url == "http://127.0.0.1:50000/sse" @@ -263,8 +276,8 @@ async def test_get_or_start_band_mcp_server_reuses_shared_server(self) -> None: "band.integrations.acp.client_adapter.create_band_mcp_backend", new=AsyncMock(return_value=backend), ) as mock_create_backend: - first = await adapter._get_or_start_band_mcp_server() - second = await adapter._get_or_start_band_mcp_server() + first = await adapter._get_or_start_band_mcp_server("room-1") + second = await adapter._get_or_start_band_mcp_server("room-1") assert first.url == second.url mock_create_backend.assert_awaited_once() @@ -285,8 +298,8 @@ async def slow_create(**kwargs: object) -> MagicMock: new=AsyncMock(side_effect=slow_create), ) as mock_create_backend: await asyncio.gather( - adapter._get_or_start_band_mcp_server(), - adapter._get_or_start_band_mcp_server(), + adapter._get_or_start_band_mcp_server("room-1"), + adapter._get_or_start_band_mcp_server("room-2"), ) mock_create_backend.assert_awaited_once() @@ -401,7 +414,7 @@ async def _registered_tool_names(self, adapter: ACPClientAdapter) -> set[str]: "band.integrations.acp.client_adapter.create_band_mcp_backend", new=AsyncMock(return_value=backend), ) as mock_create_backend: - await adapter._get_or_start_band_mcp_server() + await adapter._get_or_start_band_mcp_server("room-1") return { d.name for d in mock_create_backend.await_args.kwargs["tool_definitions"] } @@ -468,34 +481,34 @@ def test_build_system_context_defers_to_external_mcp_tool_schema(self) -> None: class TestACPClientAdapterOnStarted: - """Tests for ACPClientAdapter.on_started(). + """Tests for ACPClientAdapter.on_started() and lazy room runtime connection. - These inject a :class:`FakeSpawn` transport (the ``make_acp_transport`` fixture) - through the adapter's ``spawn_process`` seam rather than patching module globals, - so the real ACPRuntime start path runs against a scripted connection. + Spawn/transport tests inject :class:`FakeSpawn` via ``inject_acp_spawn`` and + start the room runtime explicitly — ``on_started`` no longer spawns. """ @pytest.mark.asyncio async def test_on_started_spawns_process(self, make_acp_transport) -> None: """Should spawn ACP process and initialize connection.""" transport = make_acp_transport() - adapter = ACPClientAdapter(command="codex", spawn_process=transport) - + adapter = ACPClientAdapter(command="codex") + inject_acp_spawn(adapter, transport) await adapter.on_started("Codex Bridge", "Bridge to Codex") + runtime = await adapter._runtime_for("room-1") + await runtime.start() - assert adapter._runtime._conn is transport.conn + assert runtime._conn is transport.conn transport.conn.initialize.assert_awaited_once_with(protocol_version=1) @pytest.mark.asyncio async def test_on_started_uses_large_stdio_limit(self, make_acp_transport) -> None: """Should raise the stdio reader limit for large ACP JSON frames.""" transport = make_acp_transport() - adapter = ACPClientAdapter( - command=["npx", "@zed-industries/codex-acp"], - spawn_process=transport, - ) - + adapter = ACPClientAdapter(command=["npx", "@zed-industries/codex-acp"]) + inject_acp_spawn(adapter, transport) await adapter.on_started("Codex Bridge", "Bridge to Codex") + runtime = await adapter._runtime_for("room-1") + await runtime.start() assert transport.last_kwargs["transport_kwargs"] == {"limit": 16 * 1024 * 1024} @@ -512,21 +525,20 @@ async def test_on_started_forwards_command_positionally( with patch( "band.integrations.acp.client_adapter.shutil.which", return_value=None ): - adapter = ACPClientAdapter( - command=["npx", "@zed-industries/codex-acp"], - spawn_process=transport, - ) - - await adapter.on_started("Codex Bridge", "Bridge to Codex") + adapter = ACPClientAdapter(command=["npx", "@zed-industries/codex-acp"]) + inject_acp_spawn(adapter, transport) + await adapter.on_started("Codex Bridge", "Bridge to Codex") + runtime = await adapter._runtime_for("room-1") + await runtime.start() - # spawn(client, *command, ...) — command splatted as positional args. - args, _ = transport.last_call - assert args == ("npx", "@zed-industries/codex-acp") + # spawn(client, *command, ...) — command splatted as positional args. + args, _ = transport.last_call + assert args == ("npx", "@zed-industries/codex-acp") @pytest.mark.asyncio async def test_on_started_stores_agent_info(self, make_acp_transport) -> None: """Should store agent name and description.""" - adapter = ACPClientAdapter(command="codex", spawn_process=make_acp_transport()) + adapter = ACPClientAdapter(command="codex") await adapter.on_started("Test Agent", "A test agent") @@ -538,50 +550,49 @@ async def test_on_started_prefers_http_mcp_when_supported( self, make_acp_transport ) -> None: """Should select HTTP MCP when the ACP agent advertises it.""" - adapter = ACPClientAdapter( - command="codex", - spawn_process=make_acp_transport(http=True, sse=True), - ) - + adapter = ACPClientAdapter(command="codex") + inject_acp_spawn(adapter, make_acp_transport(http=True, sse=True)) await adapter.on_started("Test Agent", "A test agent") + runtime = await adapter._runtime_for("room-1") + await runtime.start() - assert adapter._runtime._agent_mcp_transport == "http" + assert runtime._agent_mcp_transport == "http" @pytest.mark.asyncio async def test_on_started_uses_sse_mcp_when_http_missing( self, make_acp_transport ) -> None: """Should fall back to SSE MCP when that's all the ACP agent supports.""" - adapter = ACPClientAdapter( - command="codex", - spawn_process=make_acp_transport(http=False, sse=True), - ) - + adapter = ACPClientAdapter(command="codex") + inject_acp_spawn(adapter, make_acp_transport(http=False, sse=True)) await adapter.on_started("Test Agent", "A test agent") + runtime = await adapter._runtime_for("room-1") + await runtime.start() - assert adapter._runtime._agent_mcp_transport == "sse" + assert runtime._agent_mcp_transport == "sse" class TestACPClientAdapterOnMessage: """Tests for ACPClientAdapter.on_message().""" @pytest.fixture - def adapter_with_mocks(self) -> ACPClientAdapter: - """Create adapter with mocked ACP connection.""" + async def adapter_with_mocks(self) -> ACPClientAdapter: + """Create adapter with mocked ACP connection for one room.""" adapter = ACPClientAdapter(command="codex", inject_band_tools=False) + runtime = await adapter._runtime_for(_MOCK_ROOM) - # Mock ACP connection - adapter._runtime._conn = AsyncMock() + runtime._conn = AsyncMock() mock_session = MagicMock() mock_session.session_id = "acp-session-123" - adapter._runtime._conn.new_session = AsyncMock(return_value=mock_session) - adapter._runtime._conn.prompt = AsyncMock() - - # Mock client with response text - adapter._runtime._client = BandACPClient() + runtime._conn.new_session = AsyncMock(return_value=mock_session) + runtime._conn.prompt = AsyncMock() + runtime._client = BandACPClient() return adapter + def _runtime(self, adapter: ACPClientAdapter): + return adapter._runtimes[_MOCK_ROOM] + @pytest.mark.asyncio async def test_on_message_creates_session( self, adapter_with_mocks: ACPClientAdapter @@ -600,7 +611,7 @@ async def test_on_message_creates_session( room_id="room-123", ) - adapter_with_mocks._runtime._conn.new_session.assert_called_once() + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.new_session.assert_called_once() assert adapter_with_mocks._room_to_session["room-123"] == "acp-session-123" @pytest.mark.asyncio @@ -622,7 +633,7 @@ async def test_on_message_reuses_session( room_id="room-123", ) - adapter_with_mocks._runtime._conn.new_session.assert_not_called() + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.new_session.assert_not_called() @pytest.mark.asyncio async def test_on_message_sends_prompt( @@ -642,8 +653,10 @@ async def test_on_message_sends_prompt( room_id="room-123", ) - adapter_with_mocks._runtime._conn.prompt.assert_called_once() - call_kwargs = adapter_with_mocks._runtime._conn.prompt.call_args.kwargs + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt.assert_called_once() + call_kwargs = adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._conn.prompt.call_args.kwargs assert call_kwargs["session_id"] == "acp-session-123" @pytest.mark.asyncio @@ -677,8 +690,8 @@ async def test_on_message_bootstrap_rehydrates( tools = FakeAgentTools() msg = make_platform_message("Hello", room_id="room-123") - adapter_with_mocks._runtime._agent_supports_session_load = True - adapter_with_mocks._runtime._conn.load_session = AsyncMock( + adapter_with_mocks._runtimes[_MOCK_ROOM]._agent_supports_session_load = True + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.load_session = AsyncMock( return_value=object() ) history = ACPClientSessionState(room_to_session={"room-123": "session-abc"}) @@ -694,7 +707,9 @@ async def test_on_message_bootstrap_rehydrates( ) assert adapter_with_mocks._room_to_session["room-123"] == "session-abc" - adapter_with_mocks._runtime._conn.load_session.assert_awaited_once() + adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._conn.load_session.assert_awaited_once() @pytest.mark.asyncio async def test_on_message_creates_new_session_when_persisted_session_cannot_load( @@ -703,21 +718,23 @@ async def test_on_message_creates_new_session_when_persisted_session_cannot_load """A rebooted ephemeral ACP agent creates a session before prompting.""" stale_session = "stale-session" fresh_session = MagicMock(session_id="fresh-session") - adapter_with_mocks._runtime._conn.new_session = AsyncMock( + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.new_session = AsyncMock( return_value=fresh_session ) - adapter_with_mocks._runtime._agent_supports_session_load = True - adapter_with_mocks._runtime._conn.load_session = AsyncMock(return_value=None) + adapter_with_mocks._runtimes[_MOCK_ROOM]._agent_supports_session_load = True + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.load_session = AsyncMock( + return_value=None + ) async def prompt_new_session(**kwargs): session_id = kwargs["session_id"] # Stream the reply through the live sink the adapter registers for the # turn (as a real agent would), not a direct buffer poke. - await adapter_with_mocks._runtime._client.session_update( + await adapter_with_mocks._runtimes[_MOCK_ROOM]._client.session_update( session_id, update_agent_message_text("Recovered reply") ) - adapter_with_mocks._runtime._conn.prompt = AsyncMock( + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( side_effect=prompt_new_session ) tools = FakeAgentTools() @@ -734,9 +751,13 @@ async def prompt_new_session(**kwargs): ) assert adapter_with_mocks._room_to_session["room-123"] == "fresh-session" - adapter_with_mocks._runtime._conn.new_session.assert_awaited_once() - adapter_with_mocks._runtime._conn.load_session.assert_awaited_once() - prompt_calls = adapter_with_mocks._runtime._conn.prompt.call_args_list + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.new_session.assert_awaited_once() + adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._conn.load_session.assert_awaited_once() + prompt_calls = adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._conn.prompt.call_args_list assert [call.kwargs["session_id"] for call in prompt_calls] == ["fresh-session"] assert "[System Context]" in prompt_calls[0].kwargs["prompt"][0].text assert tools.messages_sent[0]["content"] == "Recovered reply" @@ -746,7 +767,7 @@ async def test_on_message_error_sends_error_event( self, adapter_with_mocks: ACPClientAdapter ) -> None: """Should send error event when ACP agent fails.""" - adapter_with_mocks._runtime._conn.prompt = AsyncMock( + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( side_effect=RuntimeError("Agent crashed") ) @@ -790,22 +811,23 @@ class TestACPClientAdapterPermissionHandler: """Tests for bidirectional permission proxying.""" @pytest.fixture - def adapter_with_mocks(self) -> ACPClientAdapter: - """Create adapter with mocked ACP connection.""" + async def adapter_with_mocks(self) -> ACPClientAdapter: + """Create adapter with mocked ACP connection for one room.""" adapter = ACPClientAdapter(command="codex", inject_band_tools=False) + runtime = await adapter._runtime_for(_MOCK_ROOM) - # Mock ACP connection - adapter._runtime._conn = AsyncMock() + runtime._conn = AsyncMock() mock_session = MagicMock() mock_session.session_id = "acp-session-123" - adapter._runtime._conn.new_session = AsyncMock(return_value=mock_session) - adapter._runtime._conn.prompt = AsyncMock() - - # Mock client with response text - adapter._runtime._client = BandACPClient() + runtime._conn.new_session = AsyncMock(return_value=mock_session) + runtime._conn.prompt = AsyncMock() + runtime._client = BandACPClient() return adapter + def _runtime(self, adapter: ACPClientAdapter): + return adapter._runtimes[_MOCK_ROOM] + @pytest.mark.asyncio async def test_permission_handler_wired_on_message( self, adapter_with_mocks: ACPClientAdapter @@ -825,7 +847,10 @@ async def test_permission_handler_wired_on_message( ) # Permission handler should have been set for this session - assert len(adapter_with_mocks._runtime._client._permission_handlers) > 0 + assert ( + len(adapter_with_mocks._runtimes[_MOCK_ROOM]._client._permission_handlers) + > 0 + ) @pytest.mark.asyncio async def test_permission_handler_skips_pair_for_approved_band_send_message( @@ -847,7 +872,9 @@ async def mock_prompt(**kwargs): tool_call.title = "band_send_message" tool_call.tool_call_id = "tc-perm-1" - result = await adapter_with_mocks._runtime._client.request_permission( + result = await adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._client.request_permission( options=[ {"optionId": "allow-once", "name": "Allow", "kind": "allow_once"} ], @@ -858,7 +885,9 @@ async def mock_prompt(**kwargs): "outcome": {"outcome": "selected", "optionId": "allow-once"} } - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -889,7 +918,9 @@ async def mock_prompt(**kwargs): tool_call.title = "write_file" tool_call.tool_call_id = "tc-perm-1" - result = await adapter_with_mocks._runtime._client.request_permission( + result = await adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._client.request_permission( options=[ {"optionId": "allow-once", "name": "Allow", "kind": "allow_once"} ], @@ -901,7 +932,9 @@ async def mock_prompt(**kwargs): "outcome": {"outcome": "selected", "optionId": "allow-once"} } - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -930,7 +963,9 @@ async def mock_prompt(**kwargs): tool_call.title = "read_file" tool_call.tool_call_id = "tc-read" - result = await adapter_with_mocks._runtime._client.request_permission( + result = await adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._client.request_permission( options=[ {"optionId": "p-once", "name": "Allow once", "kind": "allow_once"}, {"optionId": "p-rej", "name": "Reject", "kind": "reject_once"}, @@ -940,7 +975,9 @@ async def mock_prompt(**kwargs): ) captured_result.update(result) - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -972,7 +1009,9 @@ async def mock_prompt(**kwargs): tool_call.tool_call_id = "tc-danger" tool_call.raw_input = {"path": "/tmp/important"} - result = await adapter_with_mocks._runtime._client.request_permission( + result = await adapter_with_mocks._runtimes[ + _MOCK_ROOM + ]._client.request_permission( options=[ {"optionId": "p-rej", "name": "Reject", "kind": "reject_once"}, ], @@ -981,7 +1020,9 @@ async def mock_prompt(**kwargs): ) captured_result.update(result) - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -1023,7 +1064,7 @@ async def mock_prompt(**kwargs): tool_call = MagicMock() tool_call.title = "band-band_send_event" tool_call.tool_call_id = "tc-band" - await adapter_with_mocks._runtime._client.request_permission( + await adapter_with_mocks._runtimes[_MOCK_ROOM]._client.request_permission( options=[ {"optionId": "p-rej", "name": "Reject", "kind": "reject_once"}, ], @@ -1031,7 +1072,9 @@ async def mock_prompt(**kwargs): tool_call=tool_call, ) - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -1065,13 +1108,15 @@ async def mock_prompt(**kwargs): tool_call.name = "bash" tool_call.tool_call_id = "tc-bash" - await adapter_with_mocks._runtime._client.request_permission( + await adapter_with_mocks._runtimes[_MOCK_ROOM]._client.request_permission( options={}, session_id="acp-session-123", tool_call=tool_call, ) - adapter_with_mocks._runtime._conn.prompt = AsyncMock(side_effect=mock_prompt) + adapter_with_mocks._runtimes[_MOCK_ROOM]._conn.prompt = AsyncMock( + side_effect=mock_prompt + ) await adapter_with_mocks.on_message( msg, @@ -1135,13 +1180,17 @@ class TestACPClientAdapterStop: async def test_stop_closes_connection(self) -> None: """Should close ACP connection gracefully.""" adapter = ACPClientAdapter(command="codex") + runtime = adapter._build_runtime() mock_ctx = MagicMock() mock_ctx.__aexit__ = AsyncMock(return_value=None) - adapter._runtime._ctx = mock_ctx - adapter._runtime._conn = AsyncMock() - adapter._runtime._client = BandACPClient() - adapter._room_to_session["room-123"] = "session-123" - adapter._room_tools["room-123"] = MagicMock() + runtime._ctx = mock_ctx + runtime._conn = AsyncMock() + runtime._client = BandACPClient() + adapter._runtimes[_MOCK_ROOM] = runtime + adapter._room_workspaces[_MOCK_ROOM] = "/tmp/room-123" + adapter._workspace_rooms["/tmp/room-123"] = _MOCK_ROOM + adapter._room_to_session[_MOCK_ROOM] = "session-123" + adapter._room_tools[_MOCK_ROOM] = MagicMock() local_server = MagicMock() local_server.stop = AsyncMock() backend = MagicMock(local_server=local_server) @@ -1153,9 +1202,9 @@ async def test_stop_closes_connection(self) -> None: mock_ctx.__aexit__.assert_called_once() backend.stop.assert_awaited_once() - assert adapter._runtime._ctx is None - assert adapter._runtime._conn is None - assert adapter._runtime._client is None + assert runtime._ctx is None + assert runtime._conn is None + assert runtime._client is None assert adapter._room_to_session == {} assert adapter._room_tools == {} assert adapter._band_mcp_backend is None @@ -1179,14 +1228,16 @@ async def test_stop_no_connection(self) -> None: async def test_stop_handles_exit_error(self) -> None: """Should handle errors during shutdown.""" adapter = ACPClientAdapter(command="codex") - adapter._runtime._ctx = AsyncMock() - adapter._runtime._ctx.__aexit__ = AsyncMock( - side_effect=RuntimeError("Cleanup error") - ) + runtime = adapter._build_runtime() + runtime._ctx = AsyncMock() + runtime._ctx.__aexit__ = AsyncMock(side_effect=RuntimeError("Cleanup error")) + adapter._runtimes[_MOCK_ROOM] = runtime + adapter._room_workspaces[_MOCK_ROOM] = "/tmp/room-123" + adapter._workspace_rooms["/tmp/room-123"] = _MOCK_ROOM # Should not raise await adapter.stop() - assert adapter._runtime._ctx is None + assert runtime._ctx is None class TestACPCollectingClientCursorProfileExtensions: @@ -1295,18 +1346,17 @@ class TestACPClientAdapterDeadConnectionRecovery: async def test_prompt_error_clears_connection(self) -> None: """Should stop connection on prompt error so next message respawns.""" adapter = ACPClientAdapter(command="codex", inject_band_tools=False) - adapter._runtime._conn = AsyncMock() - adapter._runtime._conn.prompt = AsyncMock( - side_effect=RuntimeError("Process died") - ) + runtime = await adapter._runtime_for("room-1") + runtime._conn = AsyncMock() + runtime._conn.prompt = AsyncMock(side_effect=RuntimeError("Process died")) mock_session = MagicMock() mock_session.session_id = "sess-1" - adapter._runtime._conn.new_session = AsyncMock(return_value=mock_session) - adapter._runtime._client = BandACPClient() + runtime._conn.new_session = AsyncMock(return_value=mock_session) + runtime._client = BandACPClient() mock_ctx = MagicMock() mock_ctx.__aexit__ = AsyncMock(return_value=None) - adapter._runtime._ctx = mock_ctx + runtime._ctx = mock_ctx tools = FakeAgentTools() msg = make_platform_message("Hello", room_id="room-1") @@ -1322,8 +1372,8 @@ async def test_prompt_error_clears_connection(self) -> None: ) # Connection should be cleared after error - assert adapter._runtime._conn is None - assert adapter._runtime._ctx is None + assert runtime._conn is None + assert runtime._ctx is None # Error event should be sent error_events = events_of_type(tools, "error") diff --git a/tests/integrations/codex/test_adapter_e2e.py b/tests/integrations/codex/test_adapter_e2e.py index d66457b15..75f567d37 100644 --- a/tests/integrations/codex/test_adapter_e2e.py +++ b/tests/integrations/codex/test_adapter_e2e.py @@ -161,6 +161,23 @@ async def close(self) -> None: return None +def patch_codex_client(adapter: CodexAdapter, client: _FakeCodexClient) -> None: + def _build(_config: CodexAdapterConfig) -> _FakeCodexClient: + return client + + adapter._build_client = _build # type: ignore[method-assign] + + +def make_codex_adapter( + client: _FakeCodexClient, + config: CodexAdapterConfig | None = None, + **kwargs: Any, +) -> CodexAdapter: + adapter = CodexAdapter(config=config or CodexAdapterConfig(), **kwargs) + patch_codex_client(adapter, client) + return adapter + + def _notify(method: str, params: dict[str, Any]) -> RpcEvent: return RpcEvent( kind="notification", @@ -199,10 +216,7 @@ async def test_on_event_uses_converter_history_to_resume_thread() -> None: ) ] ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client, - ) + adapter = make_codex_adapter(fake_client, config=CodexAdapterConfig()) await adapter.on_started("Codex Agent", "Integration test agent") raw_history = [ @@ -255,13 +269,9 @@ async def test_manual_approval_resolved_by_out_of_band_approve_command() -> None ), ] ) - adapter = CodexAdapter( - config=CodexAdapterConfig( - transport="ws", - approval_mode="manual", - approval_wait_timeout_s=30.0, - ), - client_factory=lambda _cfg: fake_client, + adapter = make_codex_adapter( + fake_client, + config=CodexAdapterConfig(approval_mode="manual", approval_wait_timeout_s=30.0), ) await adapter.on_started("Codex Agent", "Integration test agent") @@ -319,10 +329,7 @@ async def test_restart_rehydrates_mapping_from_previous_task_events() -> None: ) ] ) - adapter_first = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client_first, - ) + adapter_first = make_codex_adapter(fake_client_first, config=CodexAdapterConfig()) await adapter_first.on_started("Codex Agent", "Integration test agent") await adapter_first.on_event( _agent_input( @@ -351,10 +358,7 @@ async def test_restart_rehydrates_mapping_from_previous_task_events() -> None: ) ] ) - adapter_second = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client_second, - ) + adapter_second = make_codex_adapter(fake_client_second, config=CodexAdapterConfig()) await adapter_second.on_started("Codex Agent", "Integration test agent") await adapter_second.on_event( _agent_input( @@ -392,10 +396,7 @@ async def test_resume_failure_injects_conversation_history() -> None: ) ] ) - adapter_first = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client_first, - ) + adapter_first = make_codex_adapter(fake_client_first, config=CodexAdapterConfig()) await adapter_first.on_started("Codex Agent", "Integration test agent") await adapter_first.on_event( _agent_input( @@ -441,10 +442,7 @@ async def test_resume_failure_injects_conversation_history() -> None: ], resume_error=CodexJsonRpcError(code=-32002, message="Thread expired"), ) - adapter_second = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client_second, - ) + adapter_second = make_codex_adapter(fake_client_second, config=CodexAdapterConfig()) await adapter_second.on_started("Codex Agent", "Integration test agent") await adapter_second.on_event( _agent_input( @@ -541,10 +539,8 @@ async def test_item_completed_forwards_internal_operations() -> None: ), ] ) - adapter = CodexAdapter( - config=CodexAdapterConfig(transport="ws"), - client_factory=lambda _cfg: fake_client, - emit=Emit.TOOL_CALLS | Emit.THOUGHTS, + adapter = make_codex_adapter( + fake_client, config=CodexAdapterConfig(), emit=Emit.TOOL_CALLS | Emit.THOUGHTS ) await adapter.on_started("Codex Agent", "Integration test agent") await adapter.on_event(