refactor(backend): make provision_* abstract; provision lives on the base
test / run tests/run_tests.py (pull_request) Successful in 14s
test / run tests/run_tests.py (pull_request) Successful in 14s
Template Method pattern. BottleBackend.provision is now concrete and orchestrates four abstract sub-methods: provision_prompt -> str | None (only one with a meaningful return) provision_skills -> None provision_ssh -> None provision_git -> None Each is self-gating: skills/ssh/git short-circuit on empty inputs; prompt always copies (the path must exist) and returns None when the agent has no prompt content. DockerBottleBackend drops its own `provision` (inherited from the base) and now just implements the four sub-methods. Each sub-method takes `plan: BottlePlan` (matching the abstract) and asserts isinstance to narrow to DockerBottlePlan internally, same pattern as `launch`. A future fly.io backend implements the four sub-methods; provision works for it unchanged.
This commit is contained in:
@@ -122,7 +122,6 @@ class BottleBackend(ABC):
|
|||||||
def launch(self, plan: BottlePlan) -> AbstractContextManager[Bottle]:
|
def launch(self, plan: BottlePlan) -> AbstractContextManager[Bottle]:
|
||||||
"""Build/run the bottle and yield a handle; tear down on exit."""
|
"""Build/run the bottle and yield a handle; tear down on exit."""
|
||||||
|
|
||||||
@abstractmethod
|
|
||||||
def provision(self, plan: BottlePlan, target: str) -> str | None:
|
def provision(self, plan: BottlePlan, target: str) -> str | None:
|
||||||
"""Copy host-side files (prompt, skills, SSH keys, .git) into
|
"""Copy host-side files (prompt, skills, SSH keys, .git) into
|
||||||
the running bottle. Called from `launch` after the container/
|
the running bottle. Called from `launch` after the container/
|
||||||
@@ -131,7 +130,39 @@ class BottleBackend(ABC):
|
|||||||
machine id). Returns the in-container prompt path if a prompt
|
machine id). Returns the in-container prompt path if a prompt
|
||||||
was provisioned, else None — the Bottle handle uses it to
|
was provisioned, else None — the Bottle handle uses it to
|
||||||
decide whether to add --append-system-prompt-file to claude's
|
decide whether to add --append-system-prompt-file to claude's
|
||||||
argv."""
|
argv.
|
||||||
|
|
||||||
|
Default orchestration: prompt → skills → ssh → git. Subclasses
|
||||||
|
typically don't override this; they implement the four
|
||||||
|
sub-methods below."""
|
||||||
|
prompt_path = self.provision_prompt(plan, target)
|
||||||
|
self.provision_skills(plan, target)
|
||||||
|
self.provision_ssh(plan, target)
|
||||||
|
self.provision_git(plan, target)
|
||||||
|
return prompt_path
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
def provision_prompt(self, plan: BottlePlan, target: str) -> str | None:
|
||||||
|
"""Copy the prompt file into the running bottle. Returns the
|
||||||
|
in-container path iff the agent has a non-empty prompt;
|
||||||
|
callers use the return value to decide whether to add
|
||||||
|
--append-system-prompt-file to claude's argv."""
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
def provision_skills(self, plan: BottlePlan, target: str) -> None:
|
||||||
|
"""Copy the agent's named skills from the host into the
|
||||||
|
running bottle. No-op when the agent has no skills."""
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
def provision_ssh(self, plan: BottlePlan, target: str) -> None:
|
||||||
|
"""Set up SSH in the running bottle (config, agent, keys)
|
||||||
|
so the bottle can reach the manifest's declared SSH hosts.
|
||||||
|
No-op when the bottle has no SSH entries."""
|
||||||
|
|
||||||
|
@abstractmethod
|
||||||
|
def provision_git(self, plan: BottlePlan, target: str) -> None:
|
||||||
|
"""Copy the host's cwd `.git` directory into the running
|
||||||
|
bottle if the user requested --cwd. No-op otherwise."""
|
||||||
|
|
||||||
@abstractmethod
|
@abstractmethod
|
||||||
def prepare_cleanup(self) -> BottleCleanupPlan:
|
def prepare_cleanup(self) -> BottleCleanupPlan:
|
||||||
|
|||||||
@@ -258,29 +258,13 @@ class DockerBottleBackend(BottleBackend):
|
|||||||
docker_args[name_idx] = container
|
docker_args[name_idx] = container
|
||||||
info(f"name conflict; retrying as {container}")
|
info(f"name conflict; retrying as {container}")
|
||||||
|
|
||||||
def provision(self, plan: BottlePlan, target: str) -> str | None:
|
def provision_prompt(self, plan: BottlePlan, target: str) -> str | None:
|
||||||
"""Copy prompt, skills, ssh keys, and (optionally) .git into
|
|
||||||
the running container. `target` is the resolved container
|
|
||||||
name. Returns the in-container prompt path if a prompt was
|
|
||||||
provisioned, else None — the Bottle handle uses it to decide
|
|
||||||
whether to add --append-system-prompt-file to claude's argv."""
|
|
||||||
assert isinstance(plan, DockerBottlePlan), (
|
|
||||||
f"DockerBottleBackend.provision expects DockerBottlePlan, "
|
|
||||||
f"got {type(plan).__name__}"
|
|
||||||
)
|
|
||||||
container = target
|
|
||||||
|
|
||||||
prompt_path = self.provision_prompt(plan, container)
|
|
||||||
self.provision_skills(plan, container)
|
|
||||||
self.provision_ssh(plan, container)
|
|
||||||
self.provision_git(plan, container)
|
|
||||||
return prompt_path
|
|
||||||
|
|
||||||
def provision_prompt(self, plan: DockerBottlePlan, container: str) -> str | None:
|
|
||||||
"""Copy the prompt file into the container, fix ownership/mode.
|
"""Copy the prompt file into the container, fix ownership/mode.
|
||||||
Returns the in-container path if the agent has a non-empty
|
Returns the in-container path if the agent has a non-empty
|
||||||
prompt (drives --append-system-prompt-file), else None. The
|
prompt (drives --append-system-prompt-file), else None. The
|
||||||
file is copied either way so the path always exists."""
|
file is copied either way so the path always exists."""
|
||||||
|
assert isinstance(plan, DockerBottlePlan)
|
||||||
|
container = target
|
||||||
container_home = os.environ.get("CLAUDE_BOTTLE_CONTAINER_HOME", "/home/node")
|
container_home = os.environ.get("CLAUDE_BOTTLE_CONTAINER_HOME", "/home/node")
|
||||||
in_container_prompt_path = f"{container_home}/.claude-bottle-prompt.txt"
|
in_container_prompt_path = f"{container_home}/.claude-bottle-prompt.txt"
|
||||||
|
|
||||||
@@ -305,31 +289,35 @@ class DockerBottleBackend(BottleBackend):
|
|||||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||||
return in_container_prompt_path if agent.prompt else None
|
return in_container_prompt_path if agent.prompt else None
|
||||||
|
|
||||||
def provision_skills(self, plan: DockerBottlePlan, container: str) -> None:
|
def provision_skills(self, plan: BottlePlan, target: str) -> None:
|
||||||
"""Copy each of the agent's named skills from the host's
|
"""Copy each of the agent's named skills from the host's
|
||||||
~/.claude/skills/<name>/ into the container's equivalent path.
|
~/.claude/skills/<name>/ into the container's equivalent path.
|
||||||
No-op when the agent has no skills."""
|
No-op when the agent has no skills."""
|
||||||
|
assert isinstance(plan, DockerBottlePlan)
|
||||||
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_mod.skills_copy_into(container, list(agent.skills))
|
skills_mod.skills_copy_into(target, list(agent.skills))
|
||||||
|
|
||||||
def provision_ssh(self, plan: DockerBottlePlan, container: str) -> None:
|
def provision_ssh(self, plan: BottlePlan, target: str) -> None:
|
||||||
"""If the bottle has SSH entries, set up the in-container
|
"""If the bottle has SSH entries, set up the in-container
|
||||||
ssh-agent and config so node can authenticate without ever
|
ssh-agent and config so node can authenticate without ever
|
||||||
seeing the key bytes. No-op when the bottle has no SSH."""
|
seeing the key bytes. No-op when the bottle has no SSH."""
|
||||||
|
assert isinstance(plan, DockerBottlePlan)
|
||||||
bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name)
|
bottle = plan.spec.manifest.bottle_for(plan.spec.agent_name)
|
||||||
if not bottle.ssh:
|
if not bottle.ssh:
|
||||||
return
|
return
|
||||||
proxy_host_port = pipelock.pipelock_proxy_host_port(plan.slug)
|
proxy_host_port = pipelock.pipelock_proxy_host_port(plan.slug)
|
||||||
ssh_mod.ssh_setup(container, plan.stage_dir, proxy_host_port, bottle.ssh)
|
ssh_mod.ssh_setup(target, plan.stage_dir, proxy_host_port, bottle.ssh)
|
||||||
|
|
||||||
def provision_git(self, plan: DockerBottlePlan, container: str) -> None:
|
def provision_git(self, plan: BottlePlan, target: str) -> None:
|
||||||
"""If --cwd was set and the host cwd has a .git directory, copy
|
"""If --cwd was set and the host cwd has a .git directory, copy
|
||||||
it into /home/node/workspace/.git and fix ownership. No-op
|
it into /home/node/workspace/.git and fix ownership. No-op
|
||||||
otherwise."""
|
otherwise."""
|
||||||
|
assert isinstance(plan, DockerBottlePlan)
|
||||||
if not (plan.spec.copy_cwd and Path(plan.spec.user_cwd, ".git").is_dir()):
|
if not (plan.spec.copy_cwd and Path(plan.spec.user_cwd, ".git").is_dir()):
|
||||||
return
|
return
|
||||||
|
container = target
|
||||||
info(f"copying {plan.spec.user_cwd}/.git -> {container}:/home/node/workspace/.git")
|
info(f"copying {plan.spec.user_cwd}/.git -> {container}:/home/node/workspace/.git")
|
||||||
subprocess.run(
|
subprocess.run(
|
||||||
["docker", "cp", f"{plan.spec.user_cwd}/.git", f"{container}:/home/node/workspace/.git"],
|
["docker", "cp", f"{plan.spec.user_cwd}/.git", f"{container}:/home/node/workspace/.git"],
|
||||||
|
|||||||
Reference in New Issue
Block a user