refactor(backend): hoist guest_home to BottlePlan base
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.
This commit was merged in pull request #180.
This commit is contained in:
@@ -76,6 +76,7 @@ class BottlePlan(ABC):
|
|||||||
|
|
||||||
spec: BottleSpec
|
spec: BottleSpec
|
||||||
stage_dir: Path
|
stage_dir: Path
|
||||||
|
guest_home: str
|
||||||
git_gate_plan: GitGatePlan
|
git_gate_plan: GitGatePlan
|
||||||
egress_plan: EgressPlan
|
egress_plan: EgressPlan
|
||||||
supervise_plan: SupervisePlan | None
|
supervise_plan: SupervisePlan | None
|
||||||
|
|||||||
@@ -233,6 +233,7 @@ def resolve_plan(
|
|||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage_dir,
|
stage_dir=stage_dir,
|
||||||
|
guest_home=guest_home,
|
||||||
slug=slug,
|
slug=slug,
|
||||||
container_name=container_name,
|
container_name=container_name,
|
||||||
container_name_pinned=container_name_pinned,
|
container_name_pinned=container_name_pinned,
|
||||||
|
|||||||
@@ -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)
|
manifest_bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name)
|
||||||
if not manifest_bottle.git:
|
if not manifest_bottle.git:
|
||||||
return
|
return
|
||||||
container_gitconfig = "/home/node/.gitconfig"
|
container_gitconfig = f"{plan.guest_home}/.gitconfig"
|
||||||
|
|
||||||
content = git_gate_render_gitconfig(manifest_bottle.git, GIT_GATE_HOSTNAME)
|
content = git_gate_render_gitconfig(manifest_bottle.git, GIT_GATE_HOSTNAME)
|
||||||
config_file = plan.stage_dir / "agent_gitconfig"
|
config_file = plan.stage_dir / "agent_gitconfig"
|
||||||
|
|||||||
@@ -172,6 +172,7 @@ def resolve_plan(
|
|||||||
return SmolmachinesBottlePlan(
|
return SmolmachinesBottlePlan(
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage_dir,
|
stage_dir=stage_dir,
|
||||||
|
guest_home=guest_home,
|
||||||
slug=slug,
|
slug=slug,
|
||||||
bundle_subnet=subnet,
|
bundle_subnet=subnet,
|
||||||
bundle_gateway=gateway,
|
bundle_gateway=gateway,
|
||||||
|
|||||||
@@ -36,14 +36,6 @@ from ... import Bottle
|
|||||||
from ..bottle_plan import SmolmachinesBottlePlan
|
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:
|
def provision_git(plan: SmolmachinesBottlePlan, bottle: Bottle) -> None:
|
||||||
"""Set up git inside the guest. Runs all three subcases; each
|
"""Set up git inside the guest. Runs all three subcases; each
|
||||||
no-ops when its condition isn't met."""
|
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",
|
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
|
# 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
|
# has a stable host path. The plan's stage_dir is cleaned up
|
||||||
# by start.py's session-end teardown.
|
# by start.py's session-end teardown.
|
||||||
|
|||||||
@@ -123,7 +123,7 @@ class ClaudeAgentProvider(AgentProvider):
|
|||||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||||
if not agent.skills:
|
if not agent.skills:
|
||||||
return
|
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")
|
bottle.exec(f"mkdir -p {skills_dir}", user="root")
|
||||||
for name in agent.skills:
|
for name in agent.skills:
|
||||||
src = host_skill_dir(name)
|
src = host_skill_dir(name)
|
||||||
@@ -143,7 +143,7 @@ class ClaudeAgentProvider(AgentProvider):
|
|||||||
Returns the in-guest path iff the agent has a non-empty
|
Returns the in-guest path iff the agent has a non-empty
|
||||||
prompt (drives `--append-system-prompt-file`); the file is
|
prompt (drives `--append-system-prompt-file`); the file is
|
||||||
copied either way so the path always exists."""
|
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.cp_in(str(plan.prompt_file), prompt_path)
|
||||||
bottle.exec(
|
bottle.exec(
|
||||||
f"chown node:node {prompt_path} && chmod 600 {prompt_path}",
|
f"chown node:node {prompt_path} && chmod 600 {prompt_path}",
|
||||||
|
|||||||
@@ -168,7 +168,7 @@ class CodexAgentProvider(AgentProvider):
|
|||||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||||
if not agent.skills:
|
if not agent.skills:
|
||||||
return
|
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")
|
bottle.exec(f"mkdir -p {skills_dir}", user="root")
|
||||||
for name in agent.skills:
|
for name in agent.skills:
|
||||||
src = host_skill_dir(name)
|
src = host_skill_dir(name)
|
||||||
@@ -188,7 +188,7 @@ class CodexAgentProvider(AgentProvider):
|
|||||||
Codex reads it via the agent's `Read and follow the
|
Codex reads it via the agent's `Read and follow the
|
||||||
instructions in <path>.` bootstrap (see `prompt_args`); the
|
instructions in <path>.` bootstrap (see `prompt_args`); the
|
||||||
file is copied either way so the path always exists."""
|
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.cp_in(str(plan.prompt_file), prompt_path)
|
||||||
bottle.exec(
|
bottle.exec(
|
||||||
f"chown node:node {prompt_path} && chmod 600 {prompt_path}",
|
f"chown node:node {prompt_path} && chmod 600 {prompt_path}",
|
||||||
|
|||||||
@@ -164,6 +164,7 @@ def _plan(
|
|||||||
|
|
||||||
spec = _spec(supervise=supervise, with_git=with_git, with_egress=with_egress)
|
spec = _spec(supervise=supervise, with_git=with_git, with_egress=with_egress)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=STAGE,
|
stage_dir=STAGE,
|
||||||
slug=SLUG,
|
slug=SLUG,
|
||||||
|
|||||||
@@ -78,6 +78,7 @@ def _plan(
|
|||||||
current_config_dir=Path("/tmp/current-config"),
|
current_config_dir=Path("/tmp/current-config"),
|
||||||
)
|
)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=Path("/tmp/stage"),
|
stage_dir=Path("/tmp/stage"),
|
||||||
slug="demo-abc12",
|
slug="demo-abc12",
|
||||||
|
|||||||
@@ -78,6 +78,7 @@ def _plan(
|
|||||||
current_config_dir=Path("/tmp/current-config"),
|
current_config_dir=Path("/tmp/current-config"),
|
||||||
)
|
)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=Path("/tmp/stage"),
|
stage_dir=Path("/tmp/stage"),
|
||||||
slug="demo-abc12",
|
slug="demo-abc12",
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ def _plan(tmp: str) -> DockerBottlePlan:
|
|||||||
identity="test-teardown-00001",
|
identity="test-teardown-00001",
|
||||||
)
|
)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage,
|
stage_dir=stage,
|
||||||
git_gate_plan=GitGatePlan(
|
git_gate_plan=GitGatePlan(
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ def _plan(*, git_user: dict | None = None,
|
|||||||
copy_cwd=copy_cwd, user_cwd=user_cwd,
|
copy_cwd=copy_cwd, user_cwd=user_cwd,
|
||||||
)
|
)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage_dir or Path("/tmp/stage"),
|
stage_dir=stage_dir or Path("/tmp/stage"),
|
||||||
slug="demo-abc12",
|
slug="demo-abc12",
|
||||||
|
|||||||
@@ -103,6 +103,7 @@ def _proxy_plan(tmp: str) -> PipelockProxyPlan:
|
|||||||
def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan:
|
def _docker_plan(spec: BottleSpec, tmp: str) -> DockerBottlePlan:
|
||||||
stage = Path(tmp)
|
stage = Path(tmp)
|
||||||
return DockerBottlePlan(
|
return DockerBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage,
|
stage_dir=stage,
|
||||||
git_gate_plan=_git_gate_plan(tmp),
|
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:
|
def _smolmachines_plan(spec: BottleSpec, tmp: str) -> SmolmachinesBottlePlan:
|
||||||
stage = Path(tmp)
|
stage = Path(tmp)
|
||||||
return SmolmachinesBottlePlan(
|
return SmolmachinesBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage,
|
stage_dir=stage,
|
||||||
git_gate_plan=_git_gate_plan(tmp),
|
git_gate_plan=_git_gate_plan(tmp),
|
||||||
|
|||||||
@@ -120,6 +120,7 @@ def _plan(
|
|||||||
current_config_dir=Path("/tmp/current-config"),
|
current_config_dir=Path("/tmp/current-config"),
|
||||||
)
|
)
|
||||||
return SmolmachinesBottlePlan(
|
return SmolmachinesBottlePlan(
|
||||||
|
guest_home="/home/node",
|
||||||
spec=spec,
|
spec=spec,
|
||||||
stage_dir=stage_dir or Path("/tmp/stage"),
|
stage_dir=stage_dir or Path("/tmp/stage"),
|
||||||
slug="demo-abc12",
|
slug="demo-abc12",
|
||||||
|
|||||||
Reference in New Issue
Block a user