From 21a46b97d8f35734fa23165ee2b94d1268ddb8d3 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 4 Jun 2026 01:30:24 +0000 Subject: [PATCH] refactor(backend): hoist guest_home to BottlePlan base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review feedback (review #132): guest_home shouldn't be buried inside workspace_plan / read from a hardcoded literal in each provision module. It's a cross-cutting bottle property — the backend's prepare step knows it, and every downstream consumer (contrib providers, git provisioning, gitconfig path) should read it from one place. - Adds guest_home: str to BottlePlan base dataclass. - Both backends' prepare steps populate plan.guest_home. - contrib/{claude,codex}/agent_provider.py read plan.guest_home (was plan.workspace_plan.guest_home). - bot_bottle/backend/docker/provision/git.py reads plan.guest_home for the gitconfig destination (was hardcoded "/home/node"). - bot_bottle/backend/smolmachines/provision/git.py drops the _GUEST_HOME / _guest_home() helpers and reads plan.guest_home. - Tests that construct BottlePlan subclasses directly pass guest_home="/home/node" explicitly. --- bot_bottle/backend/__init__.py | 1 + bot_bottle/backend/docker/prepare.py | 1 + bot_bottle/backend/docker/provision/git.py | 2 +- bot_bottle/backend/smolmachines/prepare.py | 1 + bot_bottle/backend/smolmachines/provision/git.py | 10 +--------- bot_bottle/contrib/claude/agent_provider.py | 4 ++-- bot_bottle/contrib/codex/agent_provider.py | 4 ++-- tests/unit/test_compose.py | 1 + tests/unit/test_contrib_claude_provider.py | 1 + tests/unit/test_contrib_codex_provider.py | 1 + tests/unit/test_docker_launch_teardown.py | 1 + tests/unit/test_docker_provision_git_user.py | 1 + tests/unit/test_plan_print_parity.py | 2 ++ tests/unit/test_smolmachines_provision.py | 1 + 14 files changed, 17 insertions(+), 14 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 0af4a84..cc408b7 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -76,6 +76,7 @@ class BottlePlan(ABC): spec: BottleSpec stage_dir: Path + guest_home: str git_gate_plan: GitGatePlan egress_plan: EgressPlan supervise_plan: SupervisePlan | None diff --git a/bot_bottle/backend/docker/prepare.py b/bot_bottle/backend/docker/prepare.py index ed16d79..b9c5df5 100644 --- a/bot_bottle/backend/docker/prepare.py +++ b/bot_bottle/backend/docker/prepare.py @@ -233,6 +233,7 @@ def resolve_plan( return DockerBottlePlan( spec=spec, stage_dir=stage_dir, + guest_home=guest_home, slug=slug, container_name=container_name, container_name_pinned=container_name_pinned, diff --git a/bot_bottle/backend/docker/provision/git.py b/bot_bottle/backend/docker/provision/git.py index d8d7da0..8d39ade 100644 --- a/bot_bottle/backend/docker/provision/git.py +++ b/bot_bottle/backend/docker/provision/git.py @@ -57,7 +57,7 @@ def _provision_git_gate_config(plan: DockerBottlePlan, bottle: Bottle) -> None: manifest_bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name) if not manifest_bottle.git: return - container_gitconfig = "/home/node/.gitconfig" + container_gitconfig = f"{plan.guest_home}/.gitconfig" content = git_gate_render_gitconfig(manifest_bottle.git, GIT_GATE_HOSTNAME) config_file = plan.stage_dir / "agent_gitconfig" diff --git a/bot_bottle/backend/smolmachines/prepare.py b/bot_bottle/backend/smolmachines/prepare.py index 765aa9f..b4c7690 100644 --- a/bot_bottle/backend/smolmachines/prepare.py +++ b/bot_bottle/backend/smolmachines/prepare.py @@ -172,6 +172,7 @@ def resolve_plan( return SmolmachinesBottlePlan( spec=spec, stage_dir=stage_dir, + guest_home=guest_home, slug=slug, bundle_subnet=subnet, bundle_gateway=gateway, diff --git a/bot_bottle/backend/smolmachines/provision/git.py b/bot_bottle/backend/smolmachines/provision/git.py index da55f73..0f5ebb8 100644 --- a/bot_bottle/backend/smolmachines/provision/git.py +++ b/bot_bottle/backend/smolmachines/provision/git.py @@ -36,14 +36,6 @@ from ... import Bottle from ..bottle_plan import SmolmachinesBottlePlan -# `node` is the agent user from the repo Dockerfile. -_GUEST_HOME = "/home/node" - - -def _guest_home() -> str: - return _GUEST_HOME - - def provision_git(plan: SmolmachinesBottlePlan, bottle: Bottle) -> None: """Set up git inside the guest. Runs all three subcases; each no-ops when its condition isn't met.""" @@ -92,7 +84,7 @@ def _provision_git_gate_config( manifest_bottle.git, plan.agent_git_gate_host, scheme="http", ) - guest_gitconfig = f"{_guest_home()}/.gitconfig" + guest_gitconfig = f"{plan.guest_home}/.gitconfig" # Stage the file under the plan's stage_dir so cp_in # has a stable host path. The plan's stage_dir is cleaned up # by start.py's session-end teardown. diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index 6d9dba9..cd8ff14 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -123,7 +123,7 @@ class ClaudeAgentProvider(AgentProvider): agent = plan.spec.manifest.agents[plan.spec.agent_name] if not agent.skills: return - skills_dir = _skills_dir(plan.workspace_plan.guest_home) + skills_dir = _skills_dir(plan.guest_home) bottle.exec(f"mkdir -p {skills_dir}", user="root") for name in agent.skills: src = host_skill_dir(name) @@ -143,7 +143,7 @@ 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.""" - prompt_path = _prompt_path(plan.workspace_plan.guest_home) + prompt_path = _prompt_path(plan.guest_home) bottle.cp_in(str(plan.prompt_file), prompt_path) bottle.exec( f"chown node:node {prompt_path} && chmod 600 {prompt_path}", diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 160e86c..9fb92f8 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -168,7 +168,7 @@ class CodexAgentProvider(AgentProvider): agent = plan.spec.manifest.agents[plan.spec.agent_name] if not agent.skills: return - skills_dir = _skills_dir(plan.workspace_plan.guest_home) + skills_dir = _skills_dir(plan.guest_home) bottle.exec(f"mkdir -p {skills_dir}", user="root") for name in agent.skills: src = host_skill_dir(name) @@ -188,7 +188,7 @@ 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.""" - prompt_path = _prompt_path(plan.workspace_plan.guest_home) + prompt_path = _prompt_path(plan.guest_home) bottle.cp_in(str(plan.prompt_file), prompt_path) bottle.exec( f"chown node:node {prompt_path} && chmod 600 {prompt_path}", diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index b566dec..b147c3e 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -164,6 +164,7 @@ def _plan( spec = _spec(supervise=supervise, with_git=with_git, with_egress=with_egress) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=STAGE, slug=SLUG, diff --git a/tests/unit/test_contrib_claude_provider.py b/tests/unit/test_contrib_claude_provider.py index d45b893..a79fada 100644 --- a/tests/unit/test_contrib_claude_provider.py +++ b/tests/unit/test_contrib_claude_provider.py @@ -78,6 +78,7 @@ def _plan( current_config_dir=Path("/tmp/current-config"), ) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=Path("/tmp/stage"), slug="demo-abc12", diff --git a/tests/unit/test_contrib_codex_provider.py b/tests/unit/test_contrib_codex_provider.py index e8caf1e..c229ea8 100644 --- a/tests/unit/test_contrib_codex_provider.py +++ b/tests/unit/test_contrib_codex_provider.py @@ -78,6 +78,7 @@ def _plan( current_config_dir=Path("/tmp/current-config"), ) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=Path("/tmp/stage"), slug="demo-abc12", diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py index 8e669d0..73b3b1e 100644 --- a/tests/unit/test_docker_launch_teardown.py +++ b/tests/unit/test_docker_launch_teardown.py @@ -44,6 +44,7 @@ def _plan(tmp: str) -> DockerBottlePlan: identity="test-teardown-00001", ) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=stage, git_gate_plan=GitGatePlan( diff --git a/tests/unit/test_docker_provision_git_user.py b/tests/unit/test_docker_provision_git_user.py index 2b0ea85..c83dd92 100644 --- a/tests/unit/test_docker_provision_git_user.py +++ b/tests/unit/test_docker_provision_git_user.py @@ -40,6 +40,7 @@ def _plan(*, git_user: dict | None = None, copy_cwd=copy_cwd, user_cwd=user_cwd, ) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=stage_dir or Path("/tmp/stage"), slug="demo-abc12", diff --git a/tests/unit/test_plan_print_parity.py b/tests/unit/test_plan_print_parity.py index 91a0afd..10328f4 100644 --- a/tests/unit/test_plan_print_parity.py +++ b/tests/unit/test_plan_print_parity.py @@ -103,6 +103,7 @@ def _proxy_plan(tmp: str) -> PipelockProxyPlan: def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan: stage = Path(tmp) return DockerBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=stage, git_gate_plan=_git_gate_plan(tmp), @@ -128,6 +129,7 @@ def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan: def _smolmachines_plan(spec: BottleSpec, tmp: str) -> SmolmachinesBottlePlan: stage = Path(tmp) return SmolmachinesBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=stage, git_gate_plan=_git_gate_plan(tmp), diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index b80f76e..c48aa01 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -120,6 +120,7 @@ def _plan( current_config_dir=Path("/tmp/current-config"), ) return SmolmachinesBottlePlan( + guest_home="/home/node", spec=spec, stage_dir=stage_dir or Path("/tmp/stage"), slug="demo-abc12",