refactor(bottles): split factory into prepare + launch phases
test / run tests/run_tests.py (pull_request) Successful in 15s
test / run tests/run_tests.py (pull_request) Successful in 15s
The Docker factory had absorbed live container ops but left the
host-side prep (image-name resolution, container-name collision
retry, pipelock yaml generation, env_resolve writes, host
validation) in cli/start.py. That kept ~half the Docker-specific
logic outside the abstraction.
Split the factory into two phases:
prepare_docker_bottle(spec, stage_dir=...) -> DockerBottlePlan
Resolves names, validates skills/SSH, writes scratch files.
No Docker resources created yet.
launch_docker_bottle(plan) -> ContextManager[Bottle]
Builds image, creates networks, boots pipelock, runs the
agent container, provisions files. Teardown on exit.
DockerBottleSpec shrinks to intent-only inputs (manifest, agent
name, --cwd flag, user_cwd, forward_oauth_token). The CLI no longer
references docker_mod, pipelock, skills, ssh, or env_resolve.
get_bottle_factory becomes get_bottle_platform returning a
BottlePlatform with .prepare and .launch — one selectable thing per
platform.
The Bottle handle now remembers the in-container prompt path and
adds --append-system-prompt-file to claude's argv when present, so
the CLI no longer needs to know the path.
cmd_start: ~148 lines down from 229. Tests pass; dry-run output
byte-identical.
This commit is contained in:
+28
-120
@@ -11,27 +11,17 @@ import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from .. import docker as docker_mod
|
||||
from .. import pipelock
|
||||
from .. import skills as skills_mod
|
||||
from .. import ssh as ssh_mod
|
||||
from ..bottles import get_bottle_factory
|
||||
from ..bottles.docker import (
|
||||
DockerBottleSpec,
|
||||
container_prompt_path,
|
||||
docker_runtime_label,
|
||||
)
|
||||
from ..env_resolve import env_resolve
|
||||
from ..log import die, info
|
||||
from ..bottles import get_bottle_platform
|
||||
from ..bottles.docker import DockerBottlePlan, DockerBottleSpec
|
||||
from ..log import info
|
||||
from ..manifest import Manifest
|
||||
from ._common import PROG, USER_CWD, read_tty_line
|
||||
|
||||
|
||||
def show_plan(spec: DockerBottleSpec, *, remote_control: bool) -> None:
|
||||
"""Render the y/N preflight summary to stderr. Pure presentation;
|
||||
reads manifest-backed fields off `spec` and probes the Docker
|
||||
runtime label. `remote_control` is the only field not already on
|
||||
the spec — it's a claude CLI flag, not a bottle property."""
|
||||
def show_plan(plan: DockerBottlePlan, *, remote_control: bool) -> None:
|
||||
"""Render the y/N preflight summary to stderr. Reads everything off
|
||||
the plan; pure presentation."""
|
||||
spec = plan.spec
|
||||
manifest = spec.manifest
|
||||
agent = manifest.agents[spec.agent_name]
|
||||
bottle = manifest.bottle_for(spec.agent_name)
|
||||
@@ -41,28 +31,28 @@ def show_plan(spec: DockerBottleSpec, *, remote_control: bool) -> None:
|
||||
env_names.append("CLAUDE_CODE_OAUTH_TOKEN")
|
||||
|
||||
ssh_hosts = [e.Host for e in bottle.ssh]
|
||||
allowlist_summary = pipelock.pipelock_allowlist_summary(manifest, agent.bottle)
|
||||
prompt_first_line = agent.prompt.splitlines()[0] if agent.prompt else ""
|
||||
runtime_label = "runsc (gVisor)" if plan.use_runsc else "runc (default)"
|
||||
|
||||
print(file=sys.stderr)
|
||||
info(f"agent : {spec.agent_name}")
|
||||
info(f"image : {spec.image}")
|
||||
if spec.derived_image:
|
||||
info(f"image : {plan.image}")
|
||||
if plan.derived_image:
|
||||
info(
|
||||
f"cwd : {spec.user_cwd} -> /home/node/workspace "
|
||||
f"(derived: {spec.derived_image})"
|
||||
f"(derived: {plan.derived_image})"
|
||||
)
|
||||
info(f"container : {spec.container_name}")
|
||||
info(f"stage dir : {spec.stage_dir}")
|
||||
info(f"container : {plan.container_name}")
|
||||
info(f"stage dir : {plan.stage_dir}")
|
||||
info("env (names only): " + (", ".join(env_names) if env_names else "(none)"))
|
||||
info("skills : " + (" ".join(agent.skills) if agent.skills else "(none)"))
|
||||
info(f"docker runtime : {docker_runtime_label()}")
|
||||
info(f"docker runtime : {runtime_label}")
|
||||
info(f"bottle : {agent.bottle}")
|
||||
if ssh_hosts:
|
||||
info(f" ssh hosts : {', '.join(ssh_hosts)}")
|
||||
else:
|
||||
info(" ssh hosts : (none)")
|
||||
info(f" egress : {allowlist_summary}")
|
||||
info(f" egress : {plan.allowlist_summary}")
|
||||
info(
|
||||
f"prompt : {len(agent.prompt)} chars; "
|
||||
f"first line: {prompt_first_line or '(empty)'}"
|
||||
@@ -81,97 +71,20 @@ def cmd_start(argv: list[str]) -> int:
|
||||
|
||||
dry_run = args.dry_run or os.environ.get("CLAUDE_BOTTLE_DRY_RUN") == "1"
|
||||
|
||||
name = args.name
|
||||
slug = docker_mod.slugify(name)
|
||||
|
||||
image = os.environ.get("CLAUDE_BOTTLE_IMAGE", "claude-bottle:latest")
|
||||
default_container = f"claude-bottle-{slug}"
|
||||
pinned_container = os.environ.get("CLAUDE_BOTTLE_CONTAINER", "")
|
||||
|
||||
runtime_image = image
|
||||
derived_image = ""
|
||||
if args.cwd:
|
||||
derived_image = os.environ.get("CLAUDE_BOTTLE_DERIVED_IMAGE", f"claude-bottle:cwd-{slug}")
|
||||
runtime_image = derived_image
|
||||
|
||||
docker_mod.require_docker()
|
||||
manifest = Manifest.resolve(USER_CWD)
|
||||
manifest.require_agent(name)
|
||||
agent = manifest.agents[name]
|
||||
bottle_name = agent.bottle
|
||||
bottle = manifest.bottle_for(name)
|
||||
|
||||
container = pinned_container or default_container
|
||||
suffix = 2
|
||||
if pinned_container:
|
||||
if docker_mod.container_exists(container):
|
||||
die(
|
||||
f"container '{container}' already exists "
|
||||
f"(pinned via CLAUDE_BOTTLE_CONTAINER). "
|
||||
f"Remove it with 'docker rm -f {container}' or unset the override."
|
||||
)
|
||||
else:
|
||||
while docker_mod.container_exists(container):
|
||||
container = f"{default_container}-{suffix}"
|
||||
suffix += 1
|
||||
if suffix > 100:
|
||||
die(
|
||||
f"could not find a free container name after "
|
||||
f"{default_container}-99; clean up old containers with "
|
||||
f"'docker rm -f <name>'"
|
||||
)
|
||||
|
||||
# CLAUDE_BOTTLE_OAUTH_TOKEN → CLAUDE_CODE_OAUTH_TOKEN forwarding.
|
||||
# Host-side token is always forwarded so every container can authenticate.
|
||||
forward_oauth_token = bool(os.environ.get("CLAUDE_BOTTLE_OAUTH_TOKEN"))
|
||||
|
||||
if agent.skills:
|
||||
skills_mod.skills_validate_all(list(agent.skills))
|
||||
|
||||
if bottle.ssh:
|
||||
ssh_mod.ssh_validate_entries(bottle.ssh)
|
||||
spec = DockerBottleSpec(
|
||||
manifest=manifest,
|
||||
agent_name=args.name,
|
||||
copy_cwd=args.cwd,
|
||||
user_cwd=USER_CWD,
|
||||
forward_oauth_token=bool(os.environ.get("CLAUDE_BOTTLE_OAUTH_TOKEN")),
|
||||
)
|
||||
|
||||
stage_dir = Path(tempfile.mkdtemp(prefix="claude-bottle-stage."))
|
||||
env_file = stage_dir / "agent.env"
|
||||
args_file = stage_dir / "docker-args"
|
||||
prompt_file = stage_dir / "prompt.txt"
|
||||
pipelock_yaml_filename = "pipelock.yaml"
|
||||
pipelock_yaml = stage_dir / pipelock_yaml_filename
|
||||
env_file.write_text("")
|
||||
env_file.chmod(0o600)
|
||||
args_file.write_text("")
|
||||
prompt_file.write_text("")
|
||||
prompt_file.chmod(0o600)
|
||||
|
||||
try:
|
||||
pipelock.pipelock_write_yaml(manifest, bottle_name, pipelock_yaml)
|
||||
|
||||
env_resolve(manifest, name, env_file, args_file)
|
||||
|
||||
prompt_content = agent.prompt
|
||||
prompt_file.write_text(prompt_content)
|
||||
|
||||
spec = DockerBottleSpec(
|
||||
agent_name=name,
|
||||
slug=slug,
|
||||
manifest=manifest,
|
||||
container_name=container,
|
||||
container_name_pinned=bool(pinned_container),
|
||||
image=image,
|
||||
derived_image=derived_image,
|
||||
runtime_image=runtime_image,
|
||||
user_cwd=USER_CWD,
|
||||
copy_cwd_git=bool(args.cwd and Path(USER_CWD, ".git").is_dir()),
|
||||
stage_dir=stage_dir,
|
||||
prompt_file=prompt_file,
|
||||
env_file=env_file,
|
||||
args_file=args_file,
|
||||
pipelock_yaml_path=pipelock_yaml,
|
||||
pipelock_yaml_filename=pipelock_yaml_filename,
|
||||
forward_oauth_token=forward_oauth_token,
|
||||
)
|
||||
|
||||
show_plan(spec, remote_control=args.remote_control)
|
||||
platform = get_bottle_platform()
|
||||
plan = platform.prepare(spec, stage_dir=stage_dir)
|
||||
show_plan(plan, remote_control=args.remote_control)
|
||||
|
||||
if dry_run:
|
||||
info("dry-run requested; not starting container.")
|
||||
@@ -184,8 +97,7 @@ def cmd_start(argv: list[str]) -> int:
|
||||
info("aborted by user")
|
||||
return 0
|
||||
|
||||
factory = get_bottle_factory()
|
||||
with factory(spec) as bottle_handle:
|
||||
with platform.launch(plan) as bottle:
|
||||
info(
|
||||
"attaching interactive claude session "
|
||||
"(Ctrl-D or 'exit' to leave; container will be removed)"
|
||||
@@ -193,12 +105,8 @@ def cmd_start(argv: list[str]) -> int:
|
||||
claude_args = ["--dangerously-skip-permissions"]
|
||||
if args.remote_control:
|
||||
claude_args.append("--remote-control")
|
||||
if prompt_content:
|
||||
claude_args.extend(
|
||||
["--append-system-prompt-file", container_prompt_path()]
|
||||
)
|
||||
bottle_handle.exec_claude(claude_args, tty=True)
|
||||
info(f"session ended; container {bottle_handle.name} will be removed")
|
||||
bottle.exec_claude(claude_args, tty=True)
|
||||
info(f"session ended; container {bottle.name} will be removed")
|
||||
return 0
|
||||
finally:
|
||||
shutil.rmtree(stage_dir, ignore_errors=True)
|
||||
|
||||
Reference in New Issue
Block a user