From 7ab35a5e2ac3b8c8c9cbe72a7012a13471ae9f77 Mon Sep 17 00:00:00 2001 From: didericis Date: Sun, 10 May 2026 23:00:07 -0400 Subject: [PATCH] refactor(bottles): absorb prepare/launch fns into DockerBottlePlatform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module-level prepare_docker_bottle and launch_docker_bottle were trivial delegations from the platform class. Move their bodies onto DockerBottlePlatform.prepare and .launch directly. The launch method keeps the @contextmanager decorator; isinstance narrows BottlePlan to DockerBottlePlan at the entry point. Internal helpers (_run_agent_container, _provision_container, runsc_available) stay at module scope — they're stateless and don't benefit from self. No behavior change. Tests pass; dry-run output unchanged. --- claude_bottle/bottles/docker.py | 333 +++++++++++++++----------------- 1 file changed, 160 insertions(+), 173 deletions(-) diff --git a/claude_bottle/bottles/docker.py b/claude_bottle/bottles/docker.py index c41a56b..17cd985 100644 --- a/claude_bottle/bottles/docker.py +++ b/claude_bottle/bottles/docker.py @@ -1,14 +1,14 @@ -"""Docker bottle factory. +"""Docker bottle platform. -Two phases: +DockerBottlePlatform owns the two-phase factory: - prepare_docker_bottle(spec, stage_dir=...) -> DockerBottlePlan + .prepare(spec, stage_dir=...) -> DockerBottlePlan Resolve names, validate host-side prerequisites, and write scratch files (env_file, args_file, prompt, pipelock yaml) to stage_dir. No Docker resources are created yet. Suitable to call before the y/N preflight. - launch_docker_bottle(plan) -> ContextManager[Bottle] + .launch(plan) -> ContextManager[_DockerBottle] Build the image, create networks, boot the pipelock sidecar, launch the agent container (with `--runtime=runsc` iff the daemon has gVisor registered), and copy prompt/skills/ssh/.git @@ -22,7 +22,7 @@ from __future__ import annotations import os import subprocess import sys -from contextlib import AbstractContextManager, contextmanager +from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path from typing import Iterator @@ -37,6 +37,10 @@ from ..log import die, info from . import BottlePlan, BottlePlatform, BottleSpec +# Where the repo root lives, for `docker build` context. Computed once. +_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent) + + # --- Runtime detection ----------------------------------------------------- @@ -56,8 +60,9 @@ def runsc_available() -> bool: @dataclass(frozen=True) class DockerBottlePlan(BottlePlan): - """Docker-specific resolved fields produced by prepare_docker_bottle. - Inherits `spec` and `stage_dir` from BottlePlan.""" + """Docker-specific resolved fields produced by + DockerBottlePlatform.prepare. Inherits `spec` and `stage_dir` from + BottlePlan.""" slug: str container_name: str @@ -153,166 +158,7 @@ class _DockerBottle: self._teardown() -# --- Prepare --------------------------------------------------------------- - - -def prepare_docker_bottle(spec: BottleSpec, *, stage_dir: Path) -> DockerBottlePlan: - """Resolve names, validate, write scratch files. No Docker resources - are created; the only side effects are host-side files under - stage_dir and a probe of `docker info`.""" - docker_mod.require_docker() - - manifest = spec.manifest - manifest.require_agent(spec.agent_name) - agent = manifest.agents[spec.agent_name] - bottle = manifest.bottle_for(spec.agent_name) - bottle_name = agent.bottle - - slug = docker_mod.slugify(spec.agent_name) - - image = os.environ.get("CLAUDE_BOTTLE_IMAGE", "claude-bottle:latest") - derived_image = "" - runtime_image = image - if spec.copy_cwd: - derived_image = os.environ.get( - "CLAUDE_BOTTLE_DERIVED_IMAGE", f"claude-bottle:cwd-{slug}" - ) - runtime_image = derived_image - - default_container = f"claude-bottle-{slug}" - pinned_container = os.environ.get("CLAUDE_BOTTLE_CONTAINER", "") - container_name = pinned_container or default_container - container_name_pinned = bool(pinned_container) - suffix = 2 - if container_name_pinned: - if docker_mod.container_exists(container_name): - die( - f"container '{container_name}' already exists " - f"(pinned via CLAUDE_BOTTLE_CONTAINER). " - f"Remove it with 'docker rm -f {container_name}' or unset the override." - ) - else: - while docker_mod.container_exists(container_name): - container_name = 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 '" - ) - - if agent.skills: - skills_mod.skills_validate_all(list(agent.skills)) - if bottle.ssh: - ssh_mod.ssh_validate_entries(bottle.ssh) - - 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) - - pipelock.pipelock_write_yaml(manifest, bottle_name, pipelock_yaml) - env_resolve(manifest, spec.agent_name, env_file, args_file) - prompt_file.write_text(agent.prompt) - - allowlist_summary = pipelock.pipelock_allowlist_summary(manifest, bottle_name) - use_runsc = runsc_available() - - return DockerBottlePlan( - spec=spec, - slug=slug, - container_name=container_name, - container_name_pinned=container_name_pinned, - image=image, - derived_image=derived_image, - runtime_image=runtime_image, - stage_dir=stage_dir, - env_file=env_file, - args_file=args_file, - prompt_file=prompt_file, - pipelock_yaml_path=pipelock_yaml, - pipelock_yaml_filename=pipelock_yaml_filename, - allowlist_summary=allowlist_summary, - use_runsc=use_runsc, - ) - - -# --- Launch ---------------------------------------------------------------- - - -# Where the repo root lives, for `docker build` context. Computed once. -_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent) - - -@contextmanager -def launch_docker_bottle(plan: DockerBottlePlan) -> Iterator[_DockerBottle]: - """Build, launch, and provision a Docker bottle. Teardown on exit.""" - state: dict[str, str] = { - "container": "", - "pipelock": "", - "internal_network": "", - "egress_network": "", - } - - def teardown() -> None: - try: - if state["container"] and docker_mod.container_exists(state["container"]): - subprocess.run( - ["docker", "rm", "-f", state["container"]], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ) - state["container"] = "" - if state["pipelock"]: - pipelock.pipelock_stop(plan.slug) - state["pipelock"] = "" - if state["internal_network"]: - network_mod.network_remove(state["internal_network"]) - state["internal_network"] = "" - if state["egress_network"]: - network_mod.network_remove(state["egress_network"]) - state["egress_network"] = "" - except BaseException: - # Teardown must not raise; swallow so the caller's __exit__ - # path can still propagate the original error. - pass - - try: - docker_mod.build_image(plan.image, _REPO_DIR) - if plan.derived_image: - docker_mod.build_image_with_cwd( - plan.derived_image, plan.image, plan.spec.user_cwd - ) - - state["internal_network"] = network_mod.network_create_internal(plan.slug) - state["egress_network"] = network_mod.network_create_egress(plan.slug) - state["pipelock"] = pipelock.pipelock_start( - plan.slug, - state["internal_network"], - state["egress_network"], - plan.stage_dir, - plan.pipelock_yaml_filename, - ) - - container = _run_agent_container(plan, state["internal_network"]) - state["container"] = container - - prompt_path = _provision_container(plan, container) - - bottle = _DockerBottle(container, teardown, prompt_path) - yield bottle - finally: - teardown() - - -# --- Internals ------------------------------------------------------------- +# --- Container internals --------------------------------------------------- def _run_agent_container(plan: DockerBottlePlan, internal_network: str) -> str: @@ -441,17 +287,158 @@ def _provision_container(plan: DockerBottlePlan, container: str) -> str | None: class DockerBottlePlatform(BottlePlatform): """Docker platform implementation. Selected by CLAUDE_BOTTLE_PLATFORM - (default). The methods delegate to the module-level prepare/launch - functions so the platform class itself stays a thin dispatch layer.""" + (default).""" name = "docker" - def prepare(self, spec: BottleSpec, *, stage_dir: Path) -> BottlePlan: - return prepare_docker_bottle(spec, stage_dir=stage_dir) + def prepare(self, spec: BottleSpec, *, stage_dir: Path) -> DockerBottlePlan: + """Resolve names, validate, write scratch files. No Docker + resources are created; the only side effects are host-side + files under stage_dir and a probe of `docker info`.""" + docker_mod.require_docker() - def launch(self, plan: BottlePlan) -> AbstractContextManager[_DockerBottle]: + manifest = spec.manifest + manifest.require_agent(spec.agent_name) + agent = manifest.agents[spec.agent_name] + bottle = manifest.bottle_for(spec.agent_name) + bottle_name = agent.bottle + + slug = docker_mod.slugify(spec.agent_name) + + image = os.environ.get("CLAUDE_BOTTLE_IMAGE", "claude-bottle:latest") + derived_image = "" + runtime_image = image + if spec.copy_cwd: + derived_image = os.environ.get( + "CLAUDE_BOTTLE_DERIVED_IMAGE", f"claude-bottle:cwd-{slug}" + ) + runtime_image = derived_image + + default_container = f"claude-bottle-{slug}" + pinned_container = os.environ.get("CLAUDE_BOTTLE_CONTAINER", "") + container_name = pinned_container or default_container + container_name_pinned = bool(pinned_container) + suffix = 2 + if container_name_pinned: + if docker_mod.container_exists(container_name): + die( + f"container '{container_name}' already exists " + f"(pinned via CLAUDE_BOTTLE_CONTAINER). " + f"Remove it with 'docker rm -f {container_name}' or unset the override." + ) + else: + while docker_mod.container_exists(container_name): + container_name = 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 '" + ) + + if agent.skills: + skills_mod.skills_validate_all(list(agent.skills)) + if bottle.ssh: + ssh_mod.ssh_validate_entries(bottle.ssh) + + 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) + + pipelock.pipelock_write_yaml(manifest, bottle_name, pipelock_yaml) + env_resolve(manifest, spec.agent_name, env_file, args_file) + prompt_file.write_text(agent.prompt) + + allowlist_summary = pipelock.pipelock_allowlist_summary(manifest, bottle_name) + use_runsc = runsc_available() + + return DockerBottlePlan( + spec=spec, + stage_dir=stage_dir, + slug=slug, + container_name=container_name, + container_name_pinned=container_name_pinned, + image=image, + derived_image=derived_image, + runtime_image=runtime_image, + env_file=env_file, + args_file=args_file, + prompt_file=prompt_file, + pipelock_yaml_path=pipelock_yaml, + pipelock_yaml_filename=pipelock_yaml_filename, + allowlist_summary=allowlist_summary, + use_runsc=use_runsc, + ) + + @contextmanager + def launch(self, plan: BottlePlan) -> Iterator[_DockerBottle]: + """Build, launch, and provision a Docker bottle. Teardown on exit.""" assert isinstance(plan, DockerBottlePlan), ( f"DockerBottlePlatform.launch expects DockerBottlePlan, " f"got {type(plan).__name__}" ) - return launch_docker_bottle(plan) + + state: dict[str, str] = { + "container": "", + "pipelock": "", + "internal_network": "", + "egress_network": "", + } + + def teardown() -> None: + try: + if state["container"] and docker_mod.container_exists(state["container"]): + subprocess.run( + ["docker", "rm", "-f", state["container"]], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + state["container"] = "" + if state["pipelock"]: + pipelock.pipelock_stop(plan.slug) + state["pipelock"] = "" + if state["internal_network"]: + network_mod.network_remove(state["internal_network"]) + state["internal_network"] = "" + if state["egress_network"]: + network_mod.network_remove(state["egress_network"]) + state["egress_network"] = "" + except BaseException: + # Teardown must not raise; swallow so the caller's + # __exit__ path can still propagate the original error. + pass + + try: + docker_mod.build_image(plan.image, _REPO_DIR) + if plan.derived_image: + docker_mod.build_image_with_cwd( + plan.derived_image, plan.image, plan.spec.user_cwd + ) + + state["internal_network"] = network_mod.network_create_internal(plan.slug) + state["egress_network"] = network_mod.network_create_egress(plan.slug) + state["pipelock"] = pipelock.pipelock_start( + plan.slug, + state["internal_network"], + state["egress_network"], + plan.stage_dir, + plan.pipelock_yaml_filename, + ) + + container = _run_agent_container(plan, state["internal_network"]) + state["container"] = container + + prompt_path = _provision_container(plan, container) + + bottle = _DockerBottle(container, teardown, prompt_path) + yield bottle + finally: + teardown()