diff --git a/bot_bottle/_provision_apply.py b/bot_bottle/_provision_apply.py deleted file mode 100644 index be8210d..0000000 --- a/bot_bottle/_provision_apply.py +++ /dev/null @@ -1,117 +0,0 @@ -"""Shared apply primitives used by every `AgentProvider` (PRD 0050). - -The skills/prompt/provision loops here are backend-agnostic — they -only touch the `Bottle.exec`/`Bottle.cp_in` primitives that every -backend implements. Each provider's `provision_skills` / -`provision_prompt` / `provision` defaults delegate here so the -business logic lives in exactly one place.""" - -from __future__ import annotations - -import os -import shlex -from typing import TYPE_CHECKING - -from .agent_provider import GUEST_HOME -from .log import die, info - - -if TYPE_CHECKING: - from .backend import Bottle, BottlePlan - - -# In-guest paths agents look at. These are baked into the bot-bottle -# Dockerfile and the smolmachines guest image; no env knob exposed, -# per the PRD 0050 issue feedback. -SKILLS_DIR = f"{GUEST_HOME}/.claude/skills" -PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt" - - -def apply_skills(plan: "BottlePlan", bottle: "Bottle") -> None: - """Copy each named skill tree from the host into the guest. - No-op when the agent has no skills. - - `cp_in` lands files as root in every backend; chown back to - node:node so the agent can read its own skills.""" - from .backend.util import host_skill_dir - - agent = plan.spec.manifest.agents[plan.spec.agent_name] - if not agent.skills: - return - - bottle.exec(f"mkdir -p {SKILLS_DIR}", user="root") - - for name in agent.skills: - src = host_skill_dir(name) - if not os.path.isdir(src): - die( - f"skill {name!r} disappeared from host between " - f"validation and copy at {src}." - ) - dst = f"{SKILLS_DIR}/{name}" - info(f"copying skill {name} into {bottle.name}:{dst}") - bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root") - bottle.cp_in(f"{src}/.", f"{dst}/") - bottle.exec(f"chown -R node:node {dst}", user="root") - - -def apply_prompt(plan: "BottlePlan", bottle: "Bottle") -> str | None: - """Copy the prompt file into the guest, fix ownership/mode. - Returns the in-guest path iff the agent has a non-empty prompt - (drives `--append-system-prompt-file`); the file is copied either - way so the path always exists.""" - bottle.cp_in(str(plan.prompt_file), PROMPT_PATH) - bottle.exec( - f"chown node:node {PROMPT_PATH} && chmod 600 {PROMPT_PATH}", - user="root", - ) - agent = plan.spec.manifest.agents[plan.spec.agent_name] - return PROMPT_PATH if agent.prompt else None - - -def apply_provision(plan: "BottlePlan", bottle: "Bottle") -> None: - """Apply the declarative `dirs`/`pre_copy`/`files`/`verify` steps - from the provider's `AgentProvisionPlan`. This is the loop that - used to live in - `backend//provision/provider_auth.py:provision_provider_auth` - pre-PRD-0050.""" - provision = plan.agent_provision - for d in provision.dirs: - path = shlex.quote(d.guest_path) - _exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}") - _exec( - bottle, - f"chown {shlex.quote(d.owner)} {path}", - f"could not chown {d.guest_path}", - ) - _exec( - bottle, - f"chmod {shlex.quote(d.mode)} {path}", - f"could not chmod {d.guest_path}", - ) - for command in provision.pre_copy: - _exec(bottle, shlex.join(command.argv), command.error) - for f in provision.files: - bottle.cp_in(str(f.host_path), f.guest_path) - path = shlex.quote(f.guest_path) - _exec( - bottle, - f"chown {shlex.quote(f.owner)} {path}", - f"could not chown {f.guest_path}", - ) - _exec( - bottle, - f"chmod {shlex.quote(f.mode)} {path}", - f"could not chmod {f.guest_path}", - ) - for command in provision.verify: - _exec(bottle, shlex.join(command.argv), command.error) - - -def _exec(bottle: "Bottle", script: str, error: str) -> None: - result = bottle.exec(script, user="root") - if result.returncode != 0: - detail = (result.stderr or result.stdout).strip() - if detail: - detail = f": {detail}" - die(f"agent provider provisioning: {error}{detail}") diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index a323635..2887a30 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -142,35 +142,27 @@ class AgentProvider(ABC): Backends call this during `prepare` and consume the result as before.""" + @abstractmethod def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: - """Copy each of the agent's named skills from - ~/.claude/skills// on the host into - /home/node/.claude/skills// in the guest. No-op when - the agent has no skills. - - Default implementation matches the legacy backend-side - modules; providers override only if the in-guest layout - differs.""" - from . import _provision_apply - _provision_apply.apply_skills(plan, bottle) + """Copy each of the agent's named skills from the host into + the guest. No-op when the agent has no skills. The in-guest + layout is provider-specific (claude-code's + `~/.claude/skills/` today; future providers may differ).""" + @abstractmethod def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None: """Copy the prompt file into the guest, fix ownership/mode, and return the in-guest path iff the agent has a non-empty prompt (drives the `--append-system-prompt-file` flag). The file is copied either way so the path always exists.""" - from . import _provision_apply - return _provision_apply.apply_prompt(plan, bottle) + @abstractmethod def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: - """Apply the declarative `dirs`/`pre_copy`/`files`/`verify` - steps from `plan.agent_provision`. Default implementation - works for any provider that produces a standard plan; was - called `provision_provider_auth` on `BottleBackend` before - PRD 0050.""" - from . import _provision_apply - _provision_apply.apply_provision(plan, bottle) + """Apply the provider's declarative + `dirs`/`pre_copy`/`files`/`verify` steps from + `plan.agent_provision`. Was called `provision_provider_auth` + on `BottleBackend` before PRD 0050.""" @abstractmethod def provision_supervise_mcp( diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index f197b30..81504a8 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -9,6 +9,8 @@ sidecar in claude-code's user config (PRD 0013).""" from __future__ import annotations import json +import os +import shlex from pathlib import Path from typing import TYPE_CHECKING @@ -20,7 +22,7 @@ from ...agent_provider import ( AgentProvisionPlan, ) from ...egress import EgressRoute -from ...log import info, warn +from ...log import die, info, warn if TYPE_CHECKING: @@ -30,6 +32,8 @@ if TYPE_CHECKING: _REPO_ROOT = Path(__file__).resolve().parents[3] _SUPERVISE_MCP_NAME = "supervise" +_SKILLS_DIR = f"{GUEST_HOME}/.claude/skills" +_PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt" _RUNTIME = AgentProviderRuntime( template="claude", @@ -105,6 +109,79 @@ class ClaudeAgentProvider(AgentProvider): hidden_env_names=hidden_env_names, ) + def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: + """Copy each named skill tree from `~/.claude/skills//` + on the host into the guest's claude-code skills dir. No-op + when the agent has no skills.""" + from ...backend.util import host_skill_dir + + agent = plan.spec.manifest.agents[plan.spec.agent_name] + if not agent.skills: + return + bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root") + for name in agent.skills: + src = host_skill_dir(name) + if not os.path.isdir(src): + die( + f"skill {name!r} disappeared from host between " + f"validation and copy at {src}." + ) + dst = f"{_SKILLS_DIR}/{name}" + info(f"copying skill {name} into {bottle.name}:{dst}") + bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root") + bottle.cp_in(f"{src}/.", f"{dst}/") + bottle.exec(f"chown -R node:node {dst}", user="root") + + def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None: + """Copy the prompt file into the guest, fix ownership/mode. + Returns the in-guest path iff the agent has a non-empty + prompt (drives `--append-system-prompt-file`); the file is + copied either way so the path always exists.""" + bottle.cp_in(str(plan.prompt_file), _PROMPT_PATH) + bottle.exec( + f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}", + user="root", + ) + agent = plan.spec.manifest.agents[plan.spec.agent_name] + return _PROMPT_PATH if agent.prompt else None + + def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: + """Apply the claude-side declarative provision steps from + `plan.agent_provision` — today that's the `claude.json` + trust-marker file. Hot-replace this with a richer flow as + claude-code's harness shape evolves.""" + provision = plan.agent_provision + for d in provision.dirs: + path = shlex.quote(d.guest_path) + _exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}") + _exec( + bottle, + f"chown {shlex.quote(d.owner)} {path}", + f"could not chown {d.guest_path}", + ) + _exec( + bottle, + f"chmod {shlex.quote(d.mode)} {path}", + f"could not chmod {d.guest_path}", + ) + for command in provision.pre_copy: + _exec(bottle, shlex.join(command.argv), command.error) + for f in provision.files: + bottle.cp_in(str(f.host_path), f.guest_path) + path = shlex.quote(f.guest_path) + _exec( + bottle, + f"chown {shlex.quote(f.owner)} {path}", + f"could not chown {f.guest_path}", + ) + _exec( + bottle, + f"chmod {shlex.quote(f.mode)} {path}", + f"could not chmod {f.guest_path}", + ) + for command in provision.verify: + _exec(bottle, shlex.join(command.argv), command.error) + def provision_supervise_mcp( self, plan: "BottlePlan", @@ -131,3 +208,12 @@ class ClaudeAgentProvider(AgentProvider): f"register manually with: " f"claude mcp add --scope user --transport http supervise {supervise_url}" ) + + +def _exec(bottle: "Bottle", script: str, error: str) -> None: + result = bottle.exec(script, user="root") + if result.returncode != 0: + detail = (result.stderr or result.stdout).strip() + if detail: + detail = f": {detail}" + die(f"agent provider provisioning: {error}{detail}") diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 1dd84cb..53ed227 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -10,6 +10,7 @@ invocation that registers the supervise sidecar in Codex's from __future__ import annotations import os +import shlex from pathlib import Path from typing import TYPE_CHECKING @@ -25,7 +26,7 @@ from ...agent_provider import ( ) from ...codex_auth import codex_host_access_token, write_codex_dummy_auth_file from ...egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute -from ...log import info, warn +from ...log import die, info, warn if TYPE_CHECKING: @@ -35,6 +36,11 @@ if TYPE_CHECKING: _REPO_ROOT = Path(__file__).resolve().parents[3] _SUPERVISE_MCP_NAME = "supervise" +# Codex agents still read skills from the claude-code convention +# (~/.claude/skills/) — the bot-bottle-codex image follows the same +# layout. If Codex grows native skill discovery later, override here. +_SKILLS_DIR = f"{GUEST_HOME}/.claude/skills" +_PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt" _RUNTIME = AgentProviderRuntime( template="codex", @@ -147,6 +153,79 @@ class CodexAgentProvider(AgentProvider): provisioned_env=provisioned_env, ) + def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None: + """Copy each named skill tree from `~/.claude/skills//` + on the host into the guest. No-op when the agent has no + skills.""" + from ...backend.util import host_skill_dir + + agent = plan.spec.manifest.agents[plan.spec.agent_name] + if not agent.skills: + return + bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root") + for name in agent.skills: + src = host_skill_dir(name) + if not os.path.isdir(src): + die( + f"skill {name!r} disappeared from host between " + f"validation and copy at {src}." + ) + dst = f"{_SKILLS_DIR}/{name}" + info(f"copying skill {name} into {bottle.name}:{dst}") + bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root") + bottle.cp_in(f"{src}/.", f"{dst}/") + bottle.exec(f"chown -R node:node {dst}", user="root") + + def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None: + """Copy the prompt file into the guest, fix ownership/mode. + Codex reads it via the agent's `Read and follow the + instructions in .` bootstrap (see `prompt_args`); the + file is copied either way so the path always exists.""" + bottle.cp_in(str(plan.prompt_file), _PROMPT_PATH) + bottle.exec( + f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}", + user="root", + ) + agent = plan.spec.manifest.agents[plan.spec.agent_name] + return _PROMPT_PATH if agent.prompt else None + + def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: + """Apply the codex-side declarative provision steps from + `plan.agent_provision`: the `~/.codex/` dir + config.toml + trust marker, plus the dummy-auth.json drop + `codex login + status` verify when host-credential forwarding is on.""" + provision = plan.agent_provision + for d in provision.dirs: + path = shlex.quote(d.guest_path) + _exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}") + _exec( + bottle, + f"chown {shlex.quote(d.owner)} {path}", + f"could not chown {d.guest_path}", + ) + _exec( + bottle, + f"chmod {shlex.quote(d.mode)} {path}", + f"could not chmod {d.guest_path}", + ) + for command in provision.pre_copy: + _exec(bottle, shlex.join(command.argv), command.error) + for f in provision.files: + bottle.cp_in(str(f.host_path), f.guest_path) + path = shlex.quote(f.guest_path) + _exec( + bottle, + f"chown {shlex.quote(f.owner)} {path}", + f"could not chown {f.guest_path}", + ) + _exec( + bottle, + f"chmod {shlex.quote(f.mode)} {path}", + f"could not chmod {f.guest_path}", + ) + for command in provision.verify: + _exec(bottle, shlex.join(command.argv), command.error) + def provision_supervise_mcp( self, plan: "BottlePlan", @@ -173,3 +252,12 @@ class CodexAgentProvider(AgentProvider): f"register manually with: " f"codex mcp add --transport http supervise {supervise_url}" ) + + +def _exec(bottle: "Bottle", script: str, error: str) -> None: + result = bottle.exec(script, user="root") + if result.returncode != 0: + detail = (result.stderr or result.stdout).strip() + if detail: + detail = f": {detail}" + die(f"agent provider provisioning: {error}{detail}") diff --git a/docs/prds/0050-agent-provider-contrib.md b/docs/prds/0050-agent-provider-contrib.md index d7af307..a14b340 100644 --- a/docs/prds/0050-agent-provider-contrib.md +++ b/docs/prds/0050-agent-provider-contrib.md @@ -370,12 +370,15 @@ Each chunk is one commit on the PR; the PR ships as one cut. symmetric with `claude mcp add` (no `--scope user`; Codex writes `~/.codex/config.toml` by default). Failure logs a warning; the bottle still works without the entry. -2. **Default methods on `AgentProvider`.** The base ABC's - `provision_skills` / `provision_prompt` / `provision` delegate - to a small `bot_bottle/_provision_apply.py` helper. Concrete - subclasses don't override the defaults today; the helper exists - so a future provider that legitimately needs a different layout - can stay declarative. +2. **Each provider owns its apply steps end-to-end.** The base + ABC declares `provision_skills` / `provision_prompt` / + `provision` as abstract; each concrete provider implements its + own copy loop. No shared `_provision_apply.py`. The apply + sequences look similar today, but Claude and Codex harnesses + diverge over time (codex already grew a dummy-auth dance + a + `codex login status` verify with no Claude analogue) and the + "shared because both happen to call cp_in then chown" coupling + would just rot. Duplication is intentional. 3. **Env knobs removed.** `BOT_BOTTLE_CONTAINER_HOME`, `BOT_BOTTLE_GUEST_HOME`, `BOT_BOTTLE_CONTAINER_SKILLS_DIR`, and `BOT_BOTTLE_GUEST_SKILLS_DIR` are gone; `/home/node` is hardcoded diff --git a/tests/unit/test_contrib_claude_provider.py b/tests/unit/test_contrib_claude_provider.py new file mode 100644 index 0000000..d45b893 --- /dev/null +++ b/tests/unit/test_contrib_claude_provider.py @@ -0,0 +1,302 @@ +"""Unit: ClaudeAgentProvider provisioning (PRD 0050, contrib/claude). + +Each provider owns its own in-guest provisioning end-to-end — +skills copy, prompt copy, declarative dirs/files/pre_copy/verify +apply, and supervise MCP registration. The Claude / Codex paths +intentionally don't share a helper module: harness changes on +either side are expected to diverge the implementations.""" + +from __future__ import annotations + +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +from bot_bottle.agent_provider import ( + AgentProvisionCommand, + AgentProvisionDir, + AgentProvisionFile, + AgentProvisionPlan, +) +from bot_bottle.backend import Bottle, BottleSpec, ExecResult +from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan +from bot_bottle.contrib.claude.agent_provider import ClaudeAgentProvider +from bot_bottle.egress import EgressPlan +from bot_bottle.git_gate import GitGatePlan +from bot_bottle.manifest import Manifest +from bot_bottle.pipelock import PipelockProxyPlan +from bot_bottle.supervise import SupervisePlan +from bot_bottle.workspace import workspace_plan + + +_URL = "http://supervise:9100/" + + +def _make_bottle(exec_result: ExecResult | None = None) -> MagicMock: + bottle = MagicMock(spec=Bottle) + bottle.name = "bot-bottle-demo-abc12" + bottle.exec.return_value = ( + exec_result if exec_result is not None + else ExecResult(returncode=0, stdout="", stderr="") + ) + return bottle + + +def _exec_scripts(bottle: MagicMock) -> list[str]: + return [c.args[0] for c in bottle.exec.call_args_list] + + +def _plan( + *, + agent_prompt: str = "", + skills: list[str] | None = None, + agent_provision: AgentProvisionPlan | None = None, + supervise: bool = False, +) -> DockerBottlePlan: + bottle_json: dict = {"agent_provider": {"template": "claude"}} + if supervise: + bottle_json["supervise"] = True + manifest = Manifest.from_json_obj({ + "bottles": {"dev": bottle_json}, + "agents": { + "demo": { + "skills": list(skills or []), + "prompt": agent_prompt, + "bottle": "dev", + }, + }, + }) + spec = BottleSpec( + manifest=manifest, agent_name="demo", + copy_cwd=False, user_cwd="/tmp/x", + ) + supervise_plan = None + if supervise: + supervise_plan = SupervisePlan( + slug="demo-abc12", + queue_dir=Path("/tmp/queue"), + current_config_dir=Path("/tmp/current-config"), + ) + return DockerBottlePlan( + spec=spec, + stage_dir=Path("/tmp/stage"), + slug="demo-abc12", + container_name="bot-bottle-demo-abc12", + container_name_pinned=False, + image="bot-bottle-claude:latest", + derived_image="", + runtime_image="bot-bottle-claude:latest", + dockerfile_path="", + env_file=Path("/tmp/agent.env"), + forwarded_env={}, + prompt_file=Path("/tmp/state/demo-abc12/agent/prompt.txt"), + proxy_plan=PipelockProxyPlan( + yaml_path=Path("/tmp/pipelock.yaml"), slug="demo-abc12", + ), + git_gate_plan=GitGatePlan( + slug="demo-abc12", + entrypoint_script=Path("/tmp/git-gate-entrypoint.sh"), + hook_script=Path("/tmp/git-gate-hook"), + access_hook_script=Path("/tmp/git-gate-access-hook"), + upstreams=(), + ), + egress_plan=EgressPlan( + slug="demo-abc12", + routes_path=Path("/tmp/routes.yaml"), + routes=(), + token_env_map={}, + ), + supervise_plan=supervise_plan, + use_runsc=False, + agent_provision=agent_provision or AgentProvisionPlan( + template="claude", command="claude", prompt_mode="append_file", + image="", dockerfile="", guest_env={}, + ), + workspace_plan=workspace_plan(spec, guest_home="/home/node"), + ) + + +class TestClaudeProvisionPrompt(unittest.TestCase): + def test_cp_uses_bottle_cp_in(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision_prompt(_plan(), bottle) + bottle.cp_in.assert_called_once_with( + "/tmp/state/demo-abc12/agent/prompt.txt", + "/home/node/.bot-bottle-prompt.txt", + ) + + def test_returns_path_when_agent_has_prompt(self): + bottle = _make_bottle() + r = ClaudeAgentProvider().provision_prompt( + _plan(agent_prompt="You are helpful."), bottle, + ) + self.assertEqual("/home/node/.bot-bottle-prompt.txt", r) + + def test_returns_none_when_agent_has_no_prompt(self): + bottle = _make_bottle() + r = ClaudeAgentProvider().provision_prompt(_plan(agent_prompt=""), bottle) + self.assertIsNone(r) + bottle.cp_in.assert_called_once() + + def test_chowns_to_node_after_copy(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision_prompt(_plan(), bottle) + scripts = _exec_scripts(bottle) + self.assertTrue( + any("chown node:node" in s + and "/home/node/.bot-bottle-prompt.txt" in s + for s in scripts) + ) + self.assertTrue( + any("chmod 600" in s + and "/home/node/.bot-bottle-prompt.txt" in s + for s in scripts) + ) + + +class TestClaudeProvisionSkills(unittest.TestCase): + def test_noop_when_agent_has_no_skills(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision_skills(_plan(skills=[]), bottle) + bottle.cp_in.assert_not_called() + bottle.exec.assert_not_called() + + def test_mkdir_plus_cp_per_skill(self): + bottle = _make_bottle() + with patch( + "bot_bottle.backend.util.host_skill_dir", + side_effect=lambda n: f"/host/skills/{n}", + ), patch( + "bot_bottle.contrib.claude.agent_provider.os.path.isdir", + return_value=True, + ): + ClaudeAgentProvider().provision_skills( + _plan(skills=["init-prd", "verify"]), bottle, + ) + scripts = _exec_scripts(bottle) + self.assertTrue( + any("mkdir -p" in s and "/home/node/.claude/skills" in s + for s in scripts) + ) + cp_targets = {c.args[1] for c in bottle.cp_in.call_args_list} + self.assertEqual({ + "/home/node/.claude/skills/init-prd/", + "/home/node/.claude/skills/verify/", + }, cp_targets) + self.assertEqual( + 2, sum(1 for s in scripts if "chown -R node:node" in s), + ) + + def test_missing_skill_dies(self): + bottle = _make_bottle() + with patch( + "bot_bottle.backend.util.host_skill_dir", + side_effect=lambda n: f"/host/skills/{n}", + ), patch( + "bot_bottle.contrib.claude.agent_provider.os.path.isdir", + return_value=False, + ): + with self.assertRaises(SystemExit): + ClaudeAgentProvider().provision_skills( + _plan(skills=["init-prd"]), bottle, + ) + + +class TestClaudeProvision(unittest.TestCase): + """The declarative dirs/files/pre_copy/verify apply loop for + the claude.json trust marker.""" + + def test_noop_on_empty_provision_plan(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision(_plan(), bottle) + bottle.cp_in.assert_not_called() + bottle.exec.assert_not_called() + + def test_copies_files_and_chowns(self): + provision = AgentProvisionPlan( + template="claude", command="claude", prompt_mode="append_file", + image="", dockerfile="", guest_env={}, + files=(AgentProvisionFile( + Path("/tmp/claude.json"), "/home/node/.claude.json", + ),), + ) + bottle = _make_bottle() + ClaudeAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) + bottle.cp_in.assert_called_once_with( + "/tmp/claude.json", "/home/node/.claude.json", + ) + scripts = _exec_scripts(bottle) + self.assertTrue( + any("chown" in s and "/home/node/.claude.json" in s for s in scripts) + ) + self.assertTrue( + any("chmod" in s and "/home/node/.claude.json" in s for s in scripts) + ) + + def test_dies_when_file_chown_fails(self): + provision = AgentProvisionPlan( + template="claude", command="claude", prompt_mode="append_file", + image="", dockerfile="", guest_env={}, + files=(AgentProvisionFile( + Path("/tmp/claude.json"), "/home/node/.claude.json", + ),), + ) + bottle = _make_bottle( + exec_result=ExecResult(1, "", "chown: no such file\n"), + ) + with self.assertRaises(SystemExit): + ClaudeAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) + + def test_runs_verify_commands(self): + provision = AgentProvisionPlan( + template="claude", command="claude", prompt_mode="append_file", + image="", dockerfile="", guest_env={}, + verify=(AgentProvisionCommand( + ("/usr/bin/true",), "verify failed", + ),), + ) + bottle = _make_bottle() + ClaudeAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) + scripts = _exec_scripts(bottle) + self.assertTrue(any("/usr/bin/true" in s for s in scripts)) + + +class TestClaudeSuperviseMcp(unittest.TestCase): + def test_noop_when_supervise_disabled(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision_supervise_mcp( + _plan(supervise=False), bottle, _URL, + ) + bottle.exec.assert_not_called() + + def test_runs_claude_mcp_add_as_node(self): + bottle = _make_bottle() + ClaudeAgentProvider().provision_supervise_mcp( + _plan(supervise=True), bottle, _URL, + ) + bottle.exec.assert_called_once() + script = bottle.exec.call_args.args[0] + self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) + self.assertIn("claude mcp add", script) + self.assertIn("--scope user", script) + self.assertIn("--transport http", script) + self.assertIn("supervise", script) + self.assertIn(_URL, script) + + def test_logs_warning_on_failure_but_does_not_raise(self): + bottle = _make_bottle( + exec_result=ExecResult(returncode=1, stdout="", stderr="boom"), + ) + ClaudeAgentProvider().provision_supervise_mcp( + _plan(supervise=True), bottle, _URL, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_provision_apply.py b/tests/unit/test_contrib_codex_provider.py similarity index 55% rename from tests/unit/test_provision_apply.py rename to tests/unit/test_contrib_codex_provider.py index 9a4f675..e8caf1e 100644 --- a/tests/unit/test_provision_apply.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -1,11 +1,10 @@ -"""Unit: shared provision-apply helpers (PRD 0050). +"""Unit: CodexAgentProvider provisioning (PRD 0050, contrib/codex). -Covers `bot_bottle._provision_apply.apply_skills` / -`apply_prompt` / `apply_provision` — the backend-agnostic helpers -that AgentProvider's default `provision_skills` / `provision_prompt` -/ `provision` dispatch through. The same suite covered the -docker / smolmachines `provision/{skills,prompt,provider_auth}.py` -modules before they were deleted.""" +The Codex provider owns its own skills / prompt / provision / +supervise-mcp end-to-end — symmetric with the claude provider but +not sharing a helper module, since codex's apply steps include +the dummy-auth dance and a `codex login status` verify that have +no claude equivalent.""" from __future__ import annotations @@ -13,14 +12,6 @@ import unittest from pathlib import Path from unittest.mock import MagicMock, patch -from bot_bottle import _provision_apply -from bot_bottle._provision_apply import ( - PROMPT_PATH, - SKILLS_DIR, - apply_prompt, - apply_provision, - apply_skills, -) from bot_bottle.agent_provider import ( AgentProvisionCommand, AgentProvisionDir, @@ -29,13 +20,18 @@ from bot_bottle.agent_provider import ( ) from bot_bottle.backend import Bottle, BottleSpec, ExecResult from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan +from bot_bottle.contrib.codex.agent_provider import CodexAgentProvider from bot_bottle.egress import EgressPlan from bot_bottle.git_gate import GitGatePlan from bot_bottle.manifest import Manifest from bot_bottle.pipelock import PipelockProxyPlan +from bot_bottle.supervise import SupervisePlan from bot_bottle.workspace import workspace_plan +_URL = "http://supervise:9100/" + + def _make_bottle(exec_result: ExecResult | None = None) -> MagicMock: bottle = MagicMock(spec=Bottle) bottle.name = "bot-bottle-demo-abc12" @@ -55,9 +51,13 @@ def _plan( agent_prompt: str = "", skills: list[str] | None = None, agent_provision: AgentProvisionPlan | None = None, + supervise: bool = False, ) -> DockerBottlePlan: + bottle_json: dict = {"agent_provider": {"template": "codex"}} + if supervise: + bottle_json["supervise"] = True manifest = Manifest.from_json_obj({ - "bottles": {"dev": {}}, + "bottles": {"dev": bottle_json}, "agents": { "demo": { "skills": list(skills or []), @@ -67,27 +67,31 @@ def _plan( }, }) spec = BottleSpec( - manifest=manifest, - agent_name="demo", - copy_cwd=False, - user_cwd="/tmp/x", + manifest=manifest, agent_name="demo", + copy_cwd=False, user_cwd="/tmp/x", ) + supervise_plan = None + if supervise: + supervise_plan = SupervisePlan( + slug="demo-abc12", + queue_dir=Path("/tmp/queue"), + current_config_dir=Path("/tmp/current-config"), + ) return DockerBottlePlan( spec=spec, stage_dir=Path("/tmp/stage"), slug="demo-abc12", container_name="bot-bottle-demo-abc12", container_name_pinned=False, - image="bot-bottle-claude:latest", + image="bot-bottle-codex:latest", derived_image="", - runtime_image="bot-bottle-claude:latest", + runtime_image="bot-bottle-codex:latest", dockerfile_path="", env_file=Path("/tmp/agent.env"), forwarded_env={}, prompt_file=Path("/tmp/state/demo-abc12/agent/prompt.txt"), proxy_plan=PipelockProxyPlan( - yaml_path=Path("/tmp/pipelock.yaml"), - slug="demo-abc12", + yaml_path=Path("/tmp/pipelock.yaml"), slug="demo-abc12", ), git_gate_plan=GitGatePlan( slug="demo-abc12", @@ -102,105 +106,81 @@ def _plan( routes=(), token_env_map={}, ), - supervise_plan=None, + supervise_plan=supervise_plan, use_runsc=False, agent_provision=agent_provision or AgentProvisionPlan( - template="claude", - command="claude", - prompt_mode="append_file", - image="", - dockerfile="", - guest_env={}, + template="codex", command="codex", prompt_mode="read_prompt_file", + image="", dockerfile="", guest_env={}, ), workspace_plan=workspace_plan(spec, guest_home="/home/node"), ) -class TestApplyPrompt(unittest.TestCase): - def test_cp_uses_bottle_cp_in(self): +class TestCodexProvisionPrompt(unittest.TestCase): + def test_cp_uses_bottle_cp_in_and_chowns(self): bottle = _make_bottle() - apply_prompt(_plan(), bottle) + r = CodexAgentProvider().provision_prompt( + _plan(agent_prompt="hello"), bottle, + ) + self.assertEqual("/home/node/.bot-bottle-prompt.txt", r) bottle.cp_in.assert_called_once_with( "/tmp/state/demo-abc12/agent/prompt.txt", - PROMPT_PATH, + "/home/node/.bot-bottle-prompt.txt", + ) + scripts = _exec_scripts(bottle) + self.assertTrue( + any("chown node:node" in s + and "/home/node/.bot-bottle-prompt.txt" in s + for s in scripts) ) - - def test_returns_path_when_agent_has_prompt(self): - bottle = _make_bottle() - r = apply_prompt(_plan(agent_prompt="You are a helpful assistant."), bottle) - self.assertEqual(PROMPT_PATH, r) def test_returns_none_when_agent_has_no_prompt(self): bottle = _make_bottle() - r = apply_prompt(_plan(agent_prompt=""), bottle) + r = CodexAgentProvider().provision_prompt(_plan(agent_prompt=""), bottle) self.assertIsNone(r) bottle.cp_in.assert_called_once() - def test_chowns_to_node_after_copy(self): - bottle = _make_bottle() - apply_prompt(_plan(), bottle) - scripts = _exec_scripts(bottle) - self.assertTrue(any("chown node:node" in s and PROMPT_PATH in s for s in scripts)) - self.assertTrue(any("chmod 600" in s and PROMPT_PATH in s for s in scripts)) - -class TestApplySkills(unittest.TestCase): +class TestCodexProvisionSkills(unittest.TestCase): def test_noop_when_agent_has_no_skills(self): bottle = _make_bottle() - apply_skills(_plan(skills=[]), bottle) + CodexAgentProvider().provision_skills(_plan(skills=[]), bottle) bottle.cp_in.assert_not_called() bottle.exec.assert_not_called() def test_mkdir_plus_cp_per_skill(self): - bottle = _make_bottle() - with patch.object( - _provision_apply, "host_skill_dir", create=True, - side_effect=lambda n: f"/host/skills/{n}", - ) if False else patch( - "bot_bottle.backend.util.host_skill_dir", - side_effect=lambda n: f"/host/skills/{n}", - ), patch("bot_bottle._provision_apply.os.path.isdir", return_value=True): - apply_skills(_plan(skills=["init-prd", "verify"]), bottle) - scripts = _exec_scripts(bottle) - self.assertTrue(any("mkdir -p" in s and SKILLS_DIR in s for s in scripts)) - cp_targets = {c.args[1] for c in bottle.cp_in.call_args_list} - self.assertEqual({ - f"{SKILLS_DIR}/init-prd/", - f"{SKILLS_DIR}/verify/", - }, cp_targets) - self.assertEqual( - 2, - sum(1 for s in scripts if "chown -R node:node" in s), - ) - - def test_missing_skill_dies(self): bottle = _make_bottle() with patch( "bot_bottle.backend.util.host_skill_dir", side_effect=lambda n: f"/host/skills/{n}", - ), patch("bot_bottle._provision_apply.os.path.isdir", return_value=False): - with self.assertRaises(SystemExit): - apply_skills(_plan(skills=["init-prd"]), bottle) + ), patch( + "bot_bottle.contrib.codex.agent_provider.os.path.isdir", + return_value=True, + ): + CodexAgentProvider().provision_skills( + _plan(skills=["init-prd"]), bottle, + ) + scripts = _exec_scripts(bottle) + self.assertTrue( + any("mkdir -p" in s and "/home/node/.claude/skills" in s + for s in scripts) + ) + bottle.cp_in.assert_called_once() + self.assertEqual( + "/home/node/.claude/skills/init-prd/", + bottle.cp_in.call_args.args[1], + ) -class TestApplyProvision(unittest.TestCase): - """The `dirs` / `pre_copy` / `files` / `verify` apply loop that - used to live in `provision_provider_auth`.""" +class TestCodexProvision(unittest.TestCase): + """Codex's declarative provision step: ~/.codex/ dir + config.toml + + (optional) dummy-auth.json + `codex login status` verify.""" - def test_noop_on_empty_provision_plan(self): - bottle = _make_bottle() - apply_provision(_plan(), bottle) - bottle.cp_in.assert_not_called() - bottle.exec.assert_not_called() - - def test_codex_provision_creates_dir_and_copies_config(self): + def test_creates_dir_and_copies_config(self): provision = AgentProvisionPlan( - template="codex", - command="codex", + template="codex", command="codex", prompt_mode="read_prompt_file", - image="bot-bottle-codex:latest", - dockerfile="", - guest_env={}, + image="", dockerfile="", guest_env={}, dirs=(AgentProvisionDir("/home/node/.codex"),), files=(AgentProvisionFile( Path("/tmp/codex-config.toml"), @@ -208,7 +188,9 @@ class TestApplyProvision(unittest.TestCase): ),), ) bottle = _make_bottle() - apply_provision(_plan(agent_provision=provision), bottle) + CodexAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) bottle.cp_in.assert_called_once_with( "/tmp/codex-config.toml", "/home/node/.codex/config.toml", @@ -220,12 +202,9 @@ class TestApplyProvision(unittest.TestCase): def test_runs_pre_copy_then_verify(self): provision = AgentProvisionPlan( - template="codex", - command="codex", + template="codex", command="codex", prompt_mode="read_prompt_file", - image="", - dockerfile="", - guest_env={}, + image="", dockerfile="", guest_env={}, pre_copy=(AgentProvisionCommand( ("find", "/home/node/.codex", "-name", "*.sqlite", "-delete"), "could not reset runtime db files", @@ -236,24 +215,55 @@ class TestApplyProvision(unittest.TestCase): ),), ) bottle = _make_bottle() - apply_provision(_plan(agent_provision=provision), bottle) + CodexAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) scripts = _exec_scripts(bottle) self.assertTrue(any("find" in s and "-delete" in s for s in scripts)) self.assertTrue(any("runuser" in s and "codex login status" in s for s in scripts)) def test_dies_when_dir_creation_fails(self): provision = AgentProvisionPlan( - template="codex", - command="codex", + template="codex", command="codex", prompt_mode="read_prompt_file", - image="", - dockerfile="", - guest_env={}, + image="", dockerfile="", guest_env={}, dirs=(AgentProvisionDir("/home/node/.codex"),), ) bottle = _make_bottle(exec_result=ExecResult(1, "", "mkdir: nope\n")) with self.assertRaises(SystemExit): - apply_provision(_plan(agent_provision=provision), bottle) + CodexAgentProvider().provision( + _plan(agent_provision=provision), bottle, + ) + + +class TestCodexSuperviseMcp(unittest.TestCase): + def test_noop_when_supervise_disabled(self): + bottle = _make_bottle() + CodexAgentProvider().provision_supervise_mcp( + _plan(supervise=False), bottle, _URL, + ) + bottle.exec.assert_not_called() + + def test_runs_codex_mcp_add_as_node(self): + bottle = _make_bottle() + CodexAgentProvider().provision_supervise_mcp( + _plan(supervise=True), bottle, _URL, + ) + bottle.exec.assert_called_once() + script = bottle.exec.call_args.args[0] + self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) + self.assertIn("codex mcp add", script) + self.assertIn("--transport http", script) + self.assertIn("supervise", script) + self.assertIn(_URL, script) + + def test_logs_warning_on_failure_but_does_not_raise(self): + bottle = _make_bottle( + exec_result=ExecResult(returncode=1, stdout="", stderr="boom"), + ) + CodexAgentProvider().provision_supervise_mcp( + _plan(supervise=True), bottle, _URL, + ) if __name__ == "__main__": diff --git a/tests/unit/test_contrib_supervise_mcp.py b/tests/unit/test_contrib_supervise_mcp.py deleted file mode 100644 index 8a9b25e..0000000 --- a/tests/unit/test_contrib_supervise_mcp.py +++ /dev/null @@ -1,163 +0,0 @@ -"""Unit: contrib supervise MCP registration (PRD 0050). - -Each provider plugin's `provision_supervise_mcp` runs the -provider's own CLI (`claude mcp add` / `codex mcp add`) inside the -agent guest to register the per-bottle supervise sidecar in the -provider's user config. The previous claude-only `provision_supervise` -modules under backend/{docker,smolmachines}/provision/supervise.py -covered this behavior pre-PRD-0050.""" - -from __future__ import annotations - -import unittest -from pathlib import Path -from unittest.mock import MagicMock - -from bot_bottle.agent_provider import AgentProvisionPlan -from bot_bottle.backend import Bottle, BottleSpec, ExecResult -from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan -from bot_bottle.contrib.claude.agent_provider import ClaudeAgentProvider -from bot_bottle.contrib.codex.agent_provider import CodexAgentProvider -from bot_bottle.egress import EgressPlan -from bot_bottle.git_gate import GitGatePlan -from bot_bottle.manifest import Manifest -from bot_bottle.pipelock import PipelockProxyPlan -from bot_bottle.supervise import SupervisePlan -from bot_bottle.workspace import workspace_plan - - -def _make_bottle(exec_result: ExecResult | None = None) -> MagicMock: - bottle = MagicMock(spec=Bottle) - bottle.name = "bot-bottle-demo-abc12" - bottle.exec.return_value = ( - exec_result if exec_result is not None - else ExecResult(returncode=0, stdout="", stderr="") - ) - return bottle - - -def _plan(*, supervise: bool, template: str = "claude") -> DockerBottlePlan: - manifest = Manifest.from_json_obj({ - "bottles": { - "dev": {"agent_provider": {"template": template}, - **({"supervise": True} if supervise else {})}, - }, - "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, - }) - spec = BottleSpec( - manifest=manifest, agent_name="demo", - copy_cwd=False, user_cwd="/tmp/x", - ) - supervise_plan = None - if supervise: - supervise_plan = SupervisePlan( - slug="demo-abc12", - queue_dir=Path("/tmp/queue"), - current_config_dir=Path("/tmp/current-config"), - ) - return DockerBottlePlan( - spec=spec, - stage_dir=Path("/tmp/stage"), - slug="demo-abc12", - container_name="bot-bottle-demo-abc12", - container_name_pinned=False, - image="bot-bottle-claude:latest", - derived_image="", - runtime_image="bot-bottle-claude:latest", - dockerfile_path="", - env_file=Path("/tmp/agent.env"), - forwarded_env={}, - prompt_file=Path("/tmp/prompt.txt"), - proxy_plan=PipelockProxyPlan( - yaml_path=Path("/tmp/pipelock.yaml"), slug="demo-abc12", - ), - git_gate_plan=GitGatePlan( - slug="demo-abc12", - entrypoint_script=Path("/tmp/git-gate-entrypoint.sh"), - hook_script=Path("/tmp/git-gate-hook"), - access_hook_script=Path("/tmp/git-gate-access-hook"), - upstreams=(), - ), - egress_plan=EgressPlan( - slug="demo-abc12", - routes_path=Path("/tmp/routes.yaml"), - routes=(), - token_env_map={}, - ), - supervise_plan=supervise_plan, - use_runsc=False, - agent_provision=AgentProvisionPlan( - template=template, command=template, - prompt_mode="append_file" if template == "claude" else "read_prompt_file", - image="", dockerfile="", guest_env={}, - ), - workspace_plan=workspace_plan(spec, guest_home="/home/node"), - ) - - -_URL = "http://supervise:9100/" - - -class TestClaudeSuperviseMcp(unittest.TestCase): - def test_noop_when_supervise_disabled(self): - bottle = _make_bottle() - ClaudeAgentProvider().provision_supervise_mcp( - _plan(supervise=False), bottle, _URL, - ) - bottle.exec.assert_not_called() - - def test_runs_claude_mcp_add_as_node(self): - bottle = _make_bottle() - ClaudeAgentProvider().provision_supervise_mcp( - _plan(supervise=True), bottle, _URL, - ) - bottle.exec.assert_called_once() - script = bottle.exec.call_args.args[0] - self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) - self.assertIn("claude mcp add", script) - self.assertIn("--scope user", script) - self.assertIn("--transport http", script) - self.assertIn("supervise", script) - self.assertIn(_URL, script) - - def test_logs_warning_on_failure_but_does_not_raise(self): - bottle = _make_bottle( - exec_result=ExecResult(returncode=1, stdout="", stderr="boom"), - ) - ClaudeAgentProvider().provision_supervise_mcp( - _plan(supervise=True), bottle, _URL, - ) - - -class TestCodexSuperviseMcp(unittest.TestCase): - def test_noop_when_supervise_disabled(self): - bottle = _make_bottle() - CodexAgentProvider().provision_supervise_mcp( - _plan(supervise=False, template="codex"), bottle, _URL, - ) - bottle.exec.assert_not_called() - - def test_runs_codex_mcp_add_as_node(self): - bottle = _make_bottle() - CodexAgentProvider().provision_supervise_mcp( - _plan(supervise=True, template="codex"), bottle, _URL, - ) - bottle.exec.assert_called_once() - script = bottle.exec.call_args.args[0] - self.assertEqual("node", bottle.exec.call_args.kwargs.get("user")) - self.assertIn("codex mcp add", script) - self.assertIn("--transport http", script) - self.assertIn("supervise", script) - self.assertIn(_URL, script) - - def test_logs_warning_on_failure_but_does_not_raise(self): - bottle = _make_bottle( - exec_result=ExecResult(returncode=1, stdout="", stderr="boom"), - ) - CodexAgentProvider().provision_supervise_mcp( - _plan(supervise=True, template="codex"), bottle, _URL, - ) - - -if __name__ == "__main__": - unittest.main()