From df1091113cfd645dcd48e3ab2b6fb5813bd20b3b Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 4 Jun 2026 01:12:09 +0000 Subject: [PATCH] refactor(agent_provider): drop GUEST_HOME default, backend drives guest_home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review feedback (review #130): the GUEST_HOME = '/home/node' default in agent_provider.py was driving the wrong direction — the agent provider shouldn't ship its own opinion about the guest home, the backend should. - Removes the GUEST_HOME constant. - Makes guest_home a required kwarg on AgentProvider.provision_plan and the agent_provision_plan shim (no default). - Drops module-level _SKILLS_DIR / _PROMPT_PATH constants from contrib/{claude,codex}/agent_provider.py; both providers now derive the in-guest paths from plan.workspace_plan.guest_home at call time, which the backend's prepare step populated. - Updates tests/unit/test_agent_provider.py callers to pass guest_home explicitly. The backend prepare paths already pass it; no production-code call sites changed. --- bot_bottle/agent_provider.py | 6 ++-- bot_bottle/contrib/claude/agent_provider.py | 25 ++++++++++------ bot_bottle/contrib/codex/agent_provider.py | 32 +++++++++++++-------- tests/unit/test_agent_provider.py | 10 +++++++ 4 files changed, 48 insertions(+), 25 deletions(-) diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index 2887a30..4a5e01a 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -41,8 +41,6 @@ PROVIDER_TEMPLATES = frozenset({PROVIDER_CLAUDE, PROVIDER_CODEX}) CODEX_HOST_CREDENTIAL_HOSTS = ("api.openai.com", "chatgpt.com") PromptMode = Literal["append_file", "read_prompt_file"] -GUEST_HOME = "/home/node" - @dataclass(frozen=True) class AgentProviderRuntime: @@ -131,7 +129,7 @@ class AgentProvider(ABC): *, dockerfile: str, state_dir: Path, - guest_home: str = GUEST_HOME, + guest_home: str, guest_env: dict[str, str] | None = None, auth_token: str = "", forward_host_credentials: bool = False, @@ -201,7 +199,7 @@ def agent_provision_plan( template: str, dockerfile: str, state_dir: Path, - guest_home: str = GUEST_HOME, + guest_home: str, guest_env: dict[str, str] | None = None, auth_token: str = "", forward_host_credentials: bool = False, diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index 81504a8..6d9dba9 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -15,7 +15,6 @@ from pathlib import Path from typing import TYPE_CHECKING from ...agent_provider import ( - GUEST_HOME, AgentProvider, AgentProviderRuntime, AgentProvisionFile, @@ -32,8 +31,14 @@ 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" + + +def _skills_dir(guest_home: str) -> str: + return f"{guest_home}/.claude/skills" + + +def _prompt_path(guest_home: str) -> str: + return f"{guest_home}/.bot-bottle-prompt.txt" _RUNTIME = AgentProviderRuntime( template="claude", @@ -57,7 +62,7 @@ class ClaudeAgentProvider(AgentProvider): *, dockerfile: str, state_dir: Path, - guest_home: str = GUEST_HOME, + guest_home: str, guest_env: dict[str, str] | None = None, auth_token: str = "", forward_host_credentials: bool = False, @@ -118,7 +123,8 @@ class ClaudeAgentProvider(AgentProvider): agent = plan.spec.manifest.agents[plan.spec.agent_name] if not agent.skills: return - bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root") + skills_dir = _skills_dir(plan.workspace_plan.guest_home) + 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): @@ -126,7 +132,7 @@ class ClaudeAgentProvider(AgentProvider): f"skill {name!r} disappeared from host between " f"validation and copy at {src}." ) - dst = f"{_SKILLS_DIR}/{name}" + 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}/") @@ -137,13 +143,14 @@ class ClaudeAgentProvider(AgentProvider): 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) + prompt_path = _prompt_path(plan.workspace_plan.guest_home) + bottle.cp_in(str(plan.prompt_file), prompt_path) bottle.exec( - f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}", + 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 + return prompt_path if agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: """Apply the claude-side declarative provision steps from diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 53ed227..160e86c 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -16,7 +16,6 @@ from typing import TYPE_CHECKING from ...agent_provider import ( CODEX_HOST_CREDENTIAL_HOSTS, - GUEST_HOME, AgentProvider, AgentProviderRuntime, AgentProvisionCommand, @@ -36,11 +35,18 @@ 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" + + +def _skills_dir(guest_home: str) -> str: + # 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, + # change here. + return f"{guest_home}/.claude/skills" + + +def _prompt_path(guest_home: str) -> str: + return f"{guest_home}/.bot-bottle-prompt.txt" _RUNTIME = AgentProviderRuntime( template="codex", @@ -64,7 +70,7 @@ class CodexAgentProvider(AgentProvider): *, dockerfile: str, state_dir: Path, - guest_home: str = GUEST_HOME, + guest_home: str, guest_env: dict[str, str] | None = None, auth_token: str = "", forward_host_credentials: bool = False, @@ -162,7 +168,8 @@ class CodexAgentProvider(AgentProvider): agent = plan.spec.manifest.agents[plan.spec.agent_name] if not agent.skills: return - bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root") + skills_dir = _skills_dir(plan.workspace_plan.guest_home) + 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): @@ -170,7 +177,7 @@ class CodexAgentProvider(AgentProvider): f"skill {name!r} disappeared from host between " f"validation and copy at {src}." ) - dst = f"{_SKILLS_DIR}/{name}" + 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}/") @@ -181,13 +188,14 @@ class CodexAgentProvider(AgentProvider): 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) + prompt_path = _prompt_path(plan.workspace_plan.guest_home) + bottle.cp_in(str(plan.prompt_file), prompt_path) bottle.exec( - f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}", + 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 + return prompt_path if agent.prompt else None def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None: """Apply the codex-side declarative provision steps from diff --git a/tests/unit/test_agent_provider.py b/tests/unit/test_agent_provider.py index 216e3f2..dd5a211 100644 --- a/tests/unit/test_agent_provider.py +++ b/tests/unit/test_agent_provider.py @@ -27,6 +27,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_codex_plan_declares_home_state(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="/tmp/Dockerfile.codex", state_dir=Path(tmp), @@ -51,6 +52,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_codex_trusts_requested_project_path(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp), @@ -68,6 +70,7 @@ class TestAgentProviderRuntime(unittest.TestCase): "tokens": {"access_token": _jwt(2000000000)}, })) plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp), @@ -87,6 +90,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_claude_with_auth_token_injects_provider_route_and_placeholder(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: plan = agent_provision_plan( + guest_home="/home/node", template="claude", dockerfile="/tmp/Dockerfile.claude", state_dir=Path(tmp), @@ -109,6 +113,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_claude_trusts_requested_project_path(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: agent_provision_plan( + guest_home="/home/node", template="claude", dockerfile="", state_dir=Path(tmp), @@ -127,6 +132,7 @@ class TestAgentProviderRuntime(unittest.TestCase): "tokens": {"access_token": _jwt(2000000000)}, })) plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp), @@ -143,6 +149,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_codex_without_forward_host_credentials_has_passthrough_egress_routes(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp), @@ -160,6 +167,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_claude_without_auth_token_has_passthrough_egress_route(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: plan = agent_provision_plan( + guest_home="/home/node", template="claude", dockerfile="", state_dir=Path(tmp), @@ -183,6 +191,7 @@ class TestAgentProviderRuntime(unittest.TestCase): "tokens": {"access_token": access}, })) plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp), @@ -197,6 +206,7 @@ class TestAgentProviderRuntime(unittest.TestCase): def test_codex_without_forward_host_credentials_has_empty_provisioned_env(self): with tempfile.TemporaryDirectory(prefix="bb-provider.") as tmp: plan = agent_provision_plan( + guest_home="/home/node", template="codex", dockerfile="", state_dir=Path(tmp),