diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 36bbb37..06b279e 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -526,6 +526,11 @@ from .docker import DockerBottleBackend # noqa: E402 # pylint: disable=wrong-i from .macos_container import MacosContainerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position from .smolmachines import SmolmachinesBottleBackend # noqa: E402 # pylint: disable=wrong-import-position +# Freezer is imported after the backend classes for the same reason: +# Freezer.commit_slug constructs ActiveAgent, which must be fully +# defined first. +from .freeze import CommitCancelled, Freezer, get_freezer # noqa: E402 # pylint: disable=wrong-import-position + # The dict is heterogeneous: each value is a BottleBackend specialized # over its own plan type. Concrete plan types are erased here because @@ -613,9 +618,12 @@ __all__ = [ "BottleCleanupPlan", "BottlePlan", "BottleSpec", + "CommitCancelled", "ExecResult", + "Freezer", "enumerate_active_agents", "get_bottle_backend", + "get_freezer", "has_backend", "known_backend_names", ] diff --git a/bot_bottle/backend/docker/freezer.py b/bot_bottle/backend/docker/freezer.py new file mode 100644 index 0000000..5f50d96 --- /dev/null +++ b/bot_bottle/backend/docker/freezer.py @@ -0,0 +1,23 @@ +"""DockerFreezer — snapshot a Docker bottle via `docker commit`.""" + +from __future__ import annotations + +from .. import ActiveAgent +from ..freeze import Freezer +from .util import commit_container +from ...log import info + + +class DockerFreezer(Freezer): + """Freezes a Docker bottle by running `docker commit`.""" + + backend_name = "docker" + + def _freeze(self, agent: ActiveAgent) -> str: + container = f"bot-bottle-{agent.slug}" + image_tag = f"bot-bottle-committed-{agent.slug}:latest" + commit_container(container, image_tag) + return image_tag + + def _export_hint(self, slug: str, image_ref: str) -> None: + info(f"to export for migration: docker save {image_ref} -o {slug}.tar") diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index c930b36..03ab619 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -47,6 +47,7 @@ from ...bottle_state import ( bottle_state_dir, egress_state_dir, git_gate_state_dir, + read_committed_image, ) from .compose import ( bottle_plan_to_compose, @@ -91,12 +92,22 @@ def launch( ) try: - # Step 1: agent image build. Sidecar images get built lazily by - # `docker compose up` via the renderer's `build:` directives. - docker_mod.build_image( - plan.image, _REPO_DIR, - dockerfile=plan.dockerfile_path, - ) + # Step 1: agent image. Use a committed snapshot when one exists + # and is present in the local daemon; otherwise build from the + # Dockerfile. Sidecar images get built lazily by `docker compose + # up` via the renderer's `build:` directives. + committed = read_committed_image(plan.slug) + if committed and docker_mod.image_exists(committed): + info(f"using committed image {committed!r}") + plan = dataclasses.replace( + plan, + agent_provision=dataclasses.replace(plan.agent_provision, image=committed), + ) + else: + docker_mod.build_image( + plan.image, _REPO_DIR, + dockerfile=plan.dockerfile_path, + ) internal_network = network_mod.network_name_for_slug(plan.slug) egress_network = network_mod.network_egress_name_for_slug(plan.slug) diff --git a/bot_bottle/backend/docker/util.py b/bot_bottle/backend/docker/util.py index 85c4a42..8ac0339 100644 --- a/bot_bottle/backend/docker/util.py +++ b/bot_bottle/backend/docker/util.py @@ -152,6 +152,21 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: # ) +def commit_container(container_name: str, image_tag: str) -> None: + """Run `docker commit ` to snapshot the + running container's filesystem state as a local Docker image.""" + result = subprocess.run( + ["docker", "commit", container_name, image_tag], + capture_output=True, text=True, check=False, + ) + if result.returncode != 0: + die( + f"docker commit {container_name!r} → {image_tag!r} failed: " + f"{(result.stderr or '').strip() or ''}" + ) + info(f"committed {container_name!r} → {image_tag!r}") + + def image_id(ref: str) -> str: """Return the content-addressed image ID (e.g. `sha256:abcd...`) for `ref`. The smolmachines backend keys its diff --git a/bot_bottle/backend/freeze.py b/bot_bottle/backend/freeze.py new file mode 100644 index 0000000..d9a8013 --- /dev/null +++ b/bot_bottle/backend/freeze.py @@ -0,0 +1,100 @@ +"""Freezer — snapshot a running bottle to a resumable artifact. + +Follows the same pattern as BottleBackend: a shared base class with +common post-freeze steps (write committed-image path, mark preserved, +print resume hint) and backend-specific subclasses in their respective +backend directories. + +Entry points: + Freezer.commit(agent) — freeze by ActiveAgent + Freezer.commit_slug(slug) — convenience wrapper for cmd_commit + get_freezer(backend_name) — factory +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod + +from . import ActiveAgent +from ..bottle_state import mark_preserved, write_committed_image +from ..log import die, info + + +class CommitCancelled(Exception): + """Raised by Freezer._freeze when the user declines a confirmation prompt.""" + + +class Freezer(ABC): + """Freezes a running bottle to a resumable artifact. + + The base class owns the shared post-commit steps: + - write_committed_image — records the artifact path in per-bottle state + - mark_preserved — prevents teardown from removing the state dir + - resume hint — printed to stderr after the snapshot + + Subclasses implement _freeze with the backend-specific snapshot + operation and optionally override _export_hint for migration hints. + """ + + backend_name: str + + def commit(self, agent: ActiveAgent) -> None: + """Freeze the bottle for `agent` to a resumable artifact. + + Calls _freeze for the backend-specific snapshot, then writes the + committed image reference to per-bottle state and marks the bottle + preserved so the next `./cli.py resume` boots from the snapshot. + + Raises CommitCancelled if the user declines an interactive + confirmation prompt (e.g. the macos-container stop prompt). + """ + image_ref = self._freeze(agent) + write_committed_image(agent.slug, image_ref) + mark_preserved(agent.slug) + info(f"to resume from this snapshot: ./cli.py resume {agent.slug}") + self._export_hint(agent.slug, image_ref) + + @abstractmethod + def _freeze(self, agent: ActiveAgent) -> str: + """Backend-specific snapshot. Returns the image tag or artifact path + stored by write_committed_image. Raises CommitCancelled if the user + declines a stop-confirmation prompt.""" + + def _export_hint(self, slug: str, image_ref: str) -> None: + """Optionally print an export-for-migration hint after committing. + Overridden by backends that provide a meaningful export command.""" + + def commit_slug(self, slug: str) -> None: + """Convenience entry for cmd_commit when only a slug is available.""" + from ..bottle_state import read_metadata + metadata = read_metadata(slug) + agent = ActiveAgent( + backend_name=self.backend_name, + slug=slug, + agent_name=metadata.agent_name if metadata else "", + started_at=metadata.started_at if metadata else "", + services=(), + ) + self.commit(agent) + + +def get_freezer(backend_name: str) -> Freezer: + """Return the Freezer for the named backend. + + backend_name "" is treated as "docker" for backward compatibility + with state dirs written before the backend field was added.""" + resolved = backend_name or "docker" + if resolved == "docker": + from .docker.freezer import DockerFreezer + return DockerFreezer() + if resolved == "macos-container": + from .macos_container.freezer import MacosContainerFreezer + return MacosContainerFreezer() + if resolved == "smolmachines": + from .smolmachines.freezer import SmolmachinesFreezer + return SmolmachinesFreezer() + die( + f"commit is only supported for docker, macos-container, and " + f"smolmachines; backend {backend_name!r} has no freezer" + ) + raise AssertionError("unreachable") diff --git a/bot_bottle/backend/macos_container/freezer.py b/bot_bottle/backend/macos_container/freezer.py new file mode 100644 index 0000000..2fae40e --- /dev/null +++ b/bot_bottle/backend/macos_container/freezer.py @@ -0,0 +1,31 @@ +"""MacosContainerFreezer — snapshot a macOS container bottle. + +Apple Container removes containers when they stop, making stop-then-export +impossible. Instead, commit_container execs into the running container and +streams the root filesystem via tar. The bottle continues running after commit. +""" + +from __future__ import annotations + +from .. import ActiveAgent +from ..freeze import Freezer +from .util import commit_container +from ...log import info + + +class MacosContainerFreezer(Freezer): + """Freezes a macOS-container bottle via exec-tar + image rebuild.""" + + backend_name = "macos-container" + + def _freeze(self, agent: ActiveAgent) -> str: + container = f"bot-bottle-{agent.slug}" + image_tag = f"bot-bottle-committed-{agent.slug}:latest" + commit_container(container, image_tag) + return image_tag + + def _export_hint(self, slug: str, image_ref: str) -> None: + info( + f"to export for migration: " + f"container image save {image_ref} -o {slug}.tar" + ) diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 8f0d47a..8de4385 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -17,7 +17,11 @@ from contextlib import ExitStack, contextmanager from pathlib import Path from typing import Callable, Generator -from ...bottle_state import egress_state_dir, git_gate_state_dir +from ...bottle_state import ( + egress_state_dir, + git_gate_state_dir, + read_committed_image, +) from ...egress import EGRESS_ROUTES_IN_CONTAINER, egress_resolve_token_values from ...git_gate import revoke_git_gate_provisioned_keys from ...log import die, info, warn @@ -83,7 +87,7 @@ def launch( try: plan = _mint_certs(plan) - _build_images(plan) + plan = _build_images(plan) internal_network = internal_network_name(plan.slug) egress_network = egress_network_name(plan.slug) @@ -134,17 +138,28 @@ def _mint_certs(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: return dataclasses.replace(plan, egress_plan=egress_plan) -def _build_images(plan: MacosContainerBottlePlan) -> None: +def _build_images(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: container_mod.build_image( SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE, ) + committed = read_committed_image(plan.slug) + if committed and container_mod.image_exists(committed): + info(f"using committed image {committed!r}") + return dataclasses.replace( + plan, + agent_provision=dataclasses.replace( + plan.agent_provision, + image=committed, + ), + ) container_mod.build_image( plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path, ) + return plan def _create_networks( @@ -313,7 +328,6 @@ def _agent_run_argv( "container", "run", "--name", plan.container_name, "--detach", - "--rm", "--network", internal_network, ] for entry in _agent_env_entries(plan, sidecar_ip): diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index 350ba3b..52ea20d 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -8,6 +8,7 @@ import ipaddress import platform import shutil import subprocess +import tempfile import time from typing import Iterable @@ -72,6 +73,53 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: subprocess.run(args, check=True) +def commit_container(container_name: str, image_tag: str) -> None: + """Snapshot a running Apple Container as a local image. + + `container export` requires a stopped container, but Apple Container + removes containers when they stop, making stop-then-export impossible. + Instead, exec into the running container as root and stream the root + filesystem out via tar, then build a new image from that archive. + The bottle continues running after commit. + """ + with tempfile.TemporaryDirectory(prefix="bot-bottle-container-commit.") as tmp: + rootfs_tar = os.path.join(tmp, "rootfs.tar") + dockerfile = os.path.join(tmp, "Dockerfile") + with open(rootfs_tar, "wb") as tar_out: + result = subprocess.run( + [ + _CONTAINER, "exec", + "--user", "root", + container_name, + "tar", "--create", + "--exclude=./proc", + "--exclude=./sys", + "--exclude=./dev", + "--exclude=./run", + "--file=-", + "--directory=/", + ".", + ], + stdout=tar_out, + stderr=subprocess.PIPE, + check=False, + ) + if result.returncode != 0: + die( + f"container exec tar {container_name!r} failed: " + f"{(result.stderr or b'').decode().strip() or ''}" + ) + with open(dockerfile, "w", encoding="utf-8") as f: + f.write( + "FROM scratch\n" + "ADD rootfs.tar /\n" + "USER node\n" + "WORKDIR /home/node\n" + ) + build_image(image_tag, tmp, dockerfile=dockerfile) + info(f"committed {container_name!r} → {image_tag!r}") + + def _ensure_builder_dns() -> None: dns = dns_server() status = _builder_status() @@ -218,6 +266,36 @@ def container_exists(name: str) -> bool: return name in {line.strip() for line in result.stdout.splitlines()} +def container_is_running(name: str) -> bool: + """Return True if the named container is currently running. + + `container list` without `--all` lists only running containers.""" + result = subprocess.run( + [_CONTAINER, "list", "--quiet"], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + return False + return name in {line.strip() for line in result.stdout.splitlines()} + + +def stop_container(name: str) -> None: + """Stop the named container without deleting it.""" + result = subprocess.run( + [_CONTAINER, "stop", name], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + die( + f"container stop {name!r} failed: " + f"{(result.stderr or '').strip() or ''}" + ) + + def force_remove_container(name: str) -> None: if container_exists(name): subprocess.run( diff --git a/bot_bottle/backend/smolmachines/bottle.py b/bot_bottle/backend/smolmachines/bottle.py index ca3ff71..c278937 100644 --- a/bot_bottle/backend/smolmachines/bottle.py +++ b/bot_bottle/backend/smolmachines/bottle.py @@ -145,7 +145,12 @@ class SmolmachinesBottle(Bottle): script = exec_shell_script(agent_argv, self.terminal_title, self.terminal_color) if tty else None if script is None: return subprocess.run(agent_argv, check=False).returncode - return subprocess.run(["sh", "-lc", script], check=False).returncode + # Use sh -c (not -lc) so the script inherits PATH from the calling + # process. sh -l sources login-shell init files (e.g. /etc/profile) + # which may NOT include smolvm's location when it was installed via + # homebrew. The calling process (./cli.py) already has smolvm on PATH + # (provision steps succeed), so -c is sufficient. + return subprocess.run(["sh", "-c", script], check=False).returncode # smolvm/libkrun can SIGKILL an otherwise-normal exec during # early-VM provisioning. Retry once after a short settle so diff --git a/bot_bottle/backend/smolmachines/freezer.py b/bot_bottle/backend/smolmachines/freezer.py new file mode 100644 index 0000000..1315008 --- /dev/null +++ b/bot_bottle/backend/smolmachines/freezer.py @@ -0,0 +1,145 @@ +"""SmolmachinesFreezer — snapshot a smolmachines bottle. + +`smolvm pack create --from-vm` requires the VM to be stopped, and smolvm +removes VMs when stopped (same issue as Apple Container). Instead, exec +into the running VM as root to write a gzip-compressed tar of the root +filesystem to /var/tmp, then copy it to the host with `smolvm machine cp`, +build a Docker image from the archive, convert it to a smolmachine artifact +via the existing registry pipeline, and record the sidecar path. The VM +stays running throughout.""" + +from __future__ import annotations + +import tempfile +from pathlib import Path + +from .. import ActiveAgent +from ..freeze import Freezer +from ..docker import util as docker_mod +from .local_registry import crane_push_tarball, ephemeral_registry +from .smolvm import machine_cp, machine_exec, pack_create +from ...bottle_state import bottle_state_dir +from ...log import die, info + + +# Temp file written inside the VM during commit. Lives in /var/tmp +# (on-disk, unlike tmpfs /tmp) to survive for machine_cp. +_VM_COMMIT_TAR = "/var/tmp/.bot-bottle-commit.tar.gz" + + +class SmolmachinesFreezer(Freezer): + """Freezes a smolmachines bottle via exec-tar + Docker image + smolmachine pack. + + The VM is NOT stopped. We exec into the running VM to write a compressed + tar of the root filesystem to /var/tmp, copy it to the host with + machine_cp, build a Docker image (Docker's ADD decompresses .tar.gz + automatically), then run the same image→registry→pack_create pipeline + that _ensure_smolmachine uses for fresh builds.""" + + backend_name = "smolmachines" + + def _freeze(self, agent: ActiveAgent) -> str: + machine = f"bot-bottle-{agent.slug}" + image_ref = f"bot-bottle-committed-{agent.slug}:latest" + output_dir = bottle_state_dir(agent.slug) + output_dir.mkdir(parents=True, exist_ok=True) + binary = output_dir / "committed-smolmachine" + sidecar = output_dir / "committed-smolmachine.smolmachine" + _snapshot_running_vm(machine, image_ref, binary) + return str(sidecar) + + def _export_hint(self, slug: str, image_ref: str) -> None: + info(f"to export for migration: cp {image_ref} {slug}.smolmachine") + + +def _snapshot_running_vm(machine: str, image_ref: str, binary: Path) -> None: + """Exec-tar the running VM, build a Docker image, and pack to a smolmachine. + + binary: destination for the launcher (sibling .smolmachine is the artifact + that machine_create --from consumes, same convention as pack_create). + """ + with tempfile.TemporaryDirectory(prefix="bot-bottle-vm-commit.") as tmp: + tmp_path = Path(tmp) + # Use .tar.gz — Docker ADD decompresses automatically and the + # compressed archive fits in the VM's /var/tmp more easily. + rootfs_tar_gz = tmp_path / "rootfs.tar.gz" + dockerfile = tmp_path / "Dockerfile" + + _exec_tar_to_file(machine, rootfs_tar_gz) + + dockerfile.write_text( + "FROM scratch\n" + "ADD rootfs.tar.gz /\n" + "USER node\n" + "WORKDIR /home/node\n" + ) + docker_mod.build_image(image_ref, str(tmp_path), dockerfile=str(dockerfile)) + + image_tarball = binary.parent / "committed.image.tar" + docker_mod.save(image_ref, str(image_tarball)) + try: + with ephemeral_registry() as handle: + digest = docker_mod.image_id(image_ref).split(":", 1)[-1][:16] + push_ref = f"{handle.push_endpoint}/bot-bottle-committed:{digest}" + pack_ref = f"{handle.pull_endpoint}/bot-bottle-committed:{digest}" + crane_push_tarball(handle, str(image_tarball), push_ref) + pack_create(pack_ref, binary) + finally: + image_tarball.unlink(missing_ok=True) + + +def _exec_tar_to_file(machine: str, dest: Path) -> None: + """Snapshot the running VM's root filesystem to dest (.tar.gz). + + Writes a gzip-compressed tar to _VM_COMMIT_TAR inside the VM via + machine_exec (same mechanism as provisioning), then copies it to the + host with machine_cp. This avoids binary-stdout piping through the + smolvm exec channel, which does not reliably handle large binary output. + + A connectivity probe (machine_exec true) runs first so a concurrent-exec + limitation (smolvm may reject a second exec while -i -t is active) is + reported clearly rather than as a silent failure.""" + # Connectivity probe — if smolvm rejects concurrent exec while an + # interactive session is running, fail clearly here. + probe = machine_exec(machine, ["true"]) + if probe.returncode != 0: + die( + f"smolvm exec is not available for {machine!r} " + f"(exit {probe.returncode}: {probe.stderr.strip() or probe.stdout.strip() or ''}). " + f"If an interactive session is active, smolvm may not support concurrent exec." + ) + + # Create the compressed tar inside the VM. + # tar exits 1 when files change during archiving (normal for a live + # filesystem); only treat exit > 1 as fatal. + tar_result = machine_exec( + machine, + [ + "tar", "--create", "--gzip", + "--exclude=./proc", + "--exclude=./sys", + "--exclude=./dev", + "--exclude=./run", + # /tmp and /var/tmp are ephemeral. Their stale contents + # (e.g. /tmp/claude-) have uid remapped by smolvm's + # pack process, causing Claude Code to refuse to use them + # on resume. Exclude both; _init_vm recreates them with + # mkdir -p + correct ownership on every boot. + "--exclude=./tmp", + "--exclude=./var/tmp", + f"--file={_VM_COMMIT_TAR}", + "--directory=/", + ".", + ], + ) + if tar_result.returncode > 1: + die( + f"smolvm exec tar {machine!r} failed (exit {tar_result.returncode}): " + f"{tar_result.stderr.strip() or tar_result.stdout.strip() or ''}" + ) + + # Copy from VM to host, then clean up. + try: + machine_cp(f"{machine}:{_VM_COMMIT_TAR}", str(dest)) + finally: + machine_exec(machine, ["rm", "-f", _VM_COMMIT_TAR]) diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index d1a8eda..e45fbbb 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -40,8 +40,12 @@ from ..docker.git_gate import ( GIT_GATE_HOOK_IN_CONTAINER, ) from ...git_gate import revoke_git_gate_provisioned_keys -from ...log import warn -from ...bottle_state import egress_state_dir, git_gate_state_dir +from ...log import info, warn +from ...bottle_state import ( + egress_state_dir, + git_gate_state_dir, + read_committed_image, +) from . import loopback_alias as _loopback from . import sidecar_bundle as _bundle from . import smolvm as _smolvm @@ -85,14 +89,7 @@ def launch( plan = _start_bundle(plan, network, loopback_ip, stack) plan = _discover_urls(plan, loopback_ip) - # Build the agent image and pack it into a `.smolmachine` - # artifact (or hit the per-Dockerfile-digest cache). Runs - # here, not in prepare, so the docker-build output doesn't - # garble the dashboard's preflight modal. - agent_from_path = _ensure_smolmachine( - plan.agent_image, - dockerfile=plan.agent_dockerfile_path, - ) + agent_from_path = _agent_from_path(plan) _launch_vm(plan, agent_from_path, loopback_ip, stack) _init_vm(plan) @@ -279,10 +276,16 @@ def _init_vm(plan: SmolmachinesBottlePlan) -> None: All folded into one sh -c to avoid back-to-back exec calls immediately after machine_start (libkrun exec-channel race). + mkdir -p guards: when booting from a committed snapshot, /tmp and + /var/tmp are excluded from the archive (they're ephemeral and their + stale contents would have wrong uid after smolvm's uid remap). The + directories must be created before chown/chmod can set permissions. + wait_exec_ready polls until the exec channel is ready for the subsequent provision calls, replacing the empirical sleep.""" _smolvm.machine_exec(plan.machine_name, [ "sh", "-c", + "mkdir -p /tmp /var/tmp && " "chown -R node:node /home/node && " "chown root:root /tmp /var/tmp && " "chmod 1777 /tmp /var/tmp", @@ -386,6 +389,30 @@ def _resolve_token_env( return egress_resolve_token_values(plan.egress_plan.token_env_map, effective_env) +def _agent_from_path(plan: SmolmachinesBottlePlan) -> Path: + """Return the `.smolmachine` artifact used for `machine create --from`. + + Prefer a committed VM artifact when one is recorded and still + present. If the file was removed, fall back to the normal image + build + pack cache path. + """ + committed = read_committed_image(plan.slug) + if committed: + committed_path = Path(committed) + if committed_path.is_file(): + info(f"using committed smolmachine {str(committed_path)!r}") + return committed_path + + # Build the agent image and pack it into a `.smolmachine` + # artifact (or hit the per-Dockerfile-digest cache). Runs here, + # not in prepare, so the docker-build output doesn't garble the + # dashboard's preflight modal. + return _ensure_smolmachine( + plan.agent_image, + dockerfile=plan.agent_dockerfile_path, + ) + + def _ensure_smolmachine(image_ref: str, *, dockerfile: str = "") -> Path: """Build the agent docker image and convert it into a `.smolmachine` artifact, caching the result under diff --git a/bot_bottle/backend/smolmachines/smolvm.py b/bot_bottle/backend/smolmachines/smolvm.py index fc25573..d2dd280 100644 --- a/bot_bottle/backend/smolmachines/smolvm.py +++ b/bot_bottle/backend/smolmachines/smolvm.py @@ -25,6 +25,7 @@ smolvm binary.""" from __future__ import annotations +import json import shutil import subprocess import time @@ -94,6 +95,16 @@ def pack_create(image: str, output: Path) -> None: _smolvm("pack", "create", "--image", image, "-o", str(output)) +def pack_create_from_vm(name: str, output: Path) -> None: + """`smolvm pack create --from-vm -o `. + + Snapshots an existing persistent VM into a pack artifact. As + with `pack_create`, smolvm writes a launcher at `output` and the + bootable sidecar at `output.smolmachine`. + """ + _smolvm("pack", "create", "--from-vm", name, "-o", str(output)) + + # --- Machine lifecycle --------------------------------------------------- @@ -143,6 +154,21 @@ def machine_create( _smolvm(*args) +def machine_is_running(name: str) -> bool: + """Return True if the named VM is in the 'running' state.""" + result = _smolvm("machine", "ls", "--json", check=False) + if result.returncode != 0: + return False + try: + machines = json.loads(result.stdout or "[]") + except ValueError: + return False + return any( + isinstance(m, dict) and m.get("name") == name and m.get("state") == "running" + for m in machines + ) + + def machine_start(name: str) -> None: """`smolvm machine start --name NAME`.""" _smolvm("machine", "start", "--name", name) diff --git a/bot_bottle/bottle_state.py b/bot_bottle/bottle_state.py index d7cb7de..93b671a 100644 --- a/bot_bottle/bottle_state.py +++ b/bot_bottle/bottle_state.py @@ -43,6 +43,7 @@ from . import supervise as _supervise # Directory layout: ~/.bot-bottle/state//... _STATE_SUBDIR = "state" _PER_BOTTLE_DOCKERFILE_NAME = "Dockerfile" +_COMMITTED_IMAGE_NAME = "committed-image" _TRANSCRIPT_SUBDIR = "transcript" # Per-sidecar scratch subdirs. PRD 0018 chunk 2: bind-mount sources # live here so chunk 3's `docker compose up` can find them at stable @@ -179,6 +180,32 @@ def write_per_bottle_dockerfile(identity: str, content: str) -> Path: return p +def committed_image_path(identity: str) -> Path: + return bottle_state_dir(identity) / _COMMITTED_IMAGE_NAME + + +def write_committed_image(identity: str, image_tag: str) -> Path: + """Persist the committed image tag for `identity`. The next + `cli.py resume ` will boot from this image instead of + rebuilding from the Dockerfile.""" + path = committed_image_path(identity) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(image_tag.strip() + "\n") + path.chmod(0o644) + return path + + +def read_committed_image(identity: str) -> str | None: + """Return the committed image tag for `identity`, or None if no + commit has been recorded. Used by the Docker launch step to skip + the Dockerfile build when a committed snapshot exists.""" + path = committed_image_path(identity) + if not path.is_file(): + return None + tag = path.read_text().strip() + return tag or None + + def per_bottle_image_tag(identity: str) -> str: """Image tag for a rebuilt bottle. Distinct from the base bot-bottle-claude:latest so per-bottle rebuilds don't collide in @@ -314,6 +341,7 @@ __all__ = [ "bottle_state_dir", "cleanup_state", "clear_preserve_marker", + "committed_image_path", "egress_state_dir", "git_gate_state_dir", "is_preserved", @@ -323,9 +351,11 @@ __all__ = [ "per_bottle_dockerfile_path", "per_bottle_image_tag", "preserve_marker_path", + "read_committed_image", "read_metadata", "supervise_state_dir", "transcript_snapshot_dir", + "write_committed_image", "write_metadata", "write_per_bottle_dockerfile", ] diff --git a/bot_bottle/cli/__init__.py b/bot_bottle/cli/__init__.py index a0ca633..879d067 100644 --- a/bot_bottle/cli/__init__.py +++ b/bot_bottle/cli/__init__.py @@ -1,6 +1,6 @@ """Main CLI dispatcher. -Commands: cleanup, edit, info, init, list, resume, start, supervise +Commands: cleanup, commit, edit, info, init, list, resume, start, supervise """ from __future__ import annotations @@ -12,6 +12,7 @@ from ..manifest import ManifestError from ._common import PROG from . import list as _list_mod from .cleanup import cmd_cleanup +from .commit import cmd_commit from .edit import cmd_edit from .info import cmd_info from .init import cmd_init @@ -23,6 +24,7 @@ cmd_list = _list_mod.cmd_list COMMANDS = { "cleanup": cmd_cleanup, + "commit": cmd_commit, "edit": cmd_edit, "info": cmd_info, "init": cmd_init, @@ -37,6 +39,7 @@ def usage() -> None: sys.stderr.write(f"usage: {PROG} [args...]\n\n") sys.stderr.write("Commands:\n") sys.stderr.write(" cleanup stop and remove all active bot-bottle containers\n") + sys.stderr.write(" commit snapshot a running bottle's container state to a Docker image\n") sys.stderr.write(" edit open an agent in vim for editing\n") sys.stderr.write(" info print env, skills, and prompt details for a named agent\n") sys.stderr.write(" init interactively create a new agent and add it to bot-bottle.json\n") diff --git a/bot_bottle/cli/commit.py b/bot_bottle/cli/commit.py new file mode 100644 index 0000000..d724bcc --- /dev/null +++ b/bot_bottle/cli/commit.py @@ -0,0 +1,53 @@ +"""commit: freeze a running bottle's state to a resumable artifact. + +Docker bottles are committed to a local Docker image. Macos-container +bottles are exported and rebuilt as a local Apple Container image. +Smolmachines bottles are packed from the running VM into a +`.smolmachine` artifact. The resulting reference is stored in +per-bottle state so the next `./cli.py resume ` boots from the +snapshot instead of rebuilding from the Dockerfile. +""" + +from __future__ import annotations + +import argparse + +from ..backend import enumerate_active_agents +from ..backend.freeze import CommitCancelled, get_freezer +from ..bottle_state import read_metadata +from ..log import die +from ._common import PROG +from . import tui + + +def cmd_commit(argv: list[str]) -> int: + parser = argparse.ArgumentParser(prog=f"{PROG} commit", add_help=True) + parser.add_argument( + "slug", + nargs="?", + default=None, + help=( + "bottle slug from `cli.py list active` " + "(omit to pick interactively)" + ), + ) + args = parser.parse_args(argv) + + slug = args.slug + if slug is None: + active = enumerate_active_agents() + if not active: + die("no active bottles; start one with `./cli.py start`") + choices = [a.slug for a in active] + slug = tui.filter_select(choices, title="Select bottle to commit") + if slug is None: + return 0 + + metadata = read_metadata(slug) + backend = metadata.backend if metadata else "" + + try: + get_freezer(backend).commit_slug(slug) + except CommitCancelled: + return 0 + return 0 diff --git a/docs/prds/prd-new-commit-bottle-state.md b/docs/prds/prd-new-commit-bottle-state.md new file mode 100644 index 0000000..93f6ef1 --- /dev/null +++ b/docs/prds/prd-new-commit-bottle-state.md @@ -0,0 +1,159 @@ +# PRD prd-new: Commit bottle state to an image + +- **Status:** Active +- **Author:** Claude +- **Created:** 2026-06-20 +- **Issue:** #194 + +## Summary + +Add a `commit` CLI command that freezes a running bottle's state to a +resumable local artifact. Docker bottles are stored as Docker images; +smolmachines bottles are stored as `.smolmachine` artifacts. Operators +can then resume the bottle from that exact filesystem snapshot, or +export the artifact to migrate work to a different host. + +## Problem + +When a long-running agent session is interrupted — by a host reboot, a +network failure, or a planned infrastructure migration — the in-progress +container state is lost. `cli.py resume` rebuilds the agent image from +the Dockerfile and reprovi-sions the bottle, but that returns the guest +to its initial state, not to wherever the agent was mid-task. + +There is no mechanism today to capture "what's installed / configured +inside the running container right now" and make it reproducible. The +`capability-block` flow writes a new Dockerfile and marks the bottle for +resume, but that only applies when the agent itself has requested a +capability change; it doesn't help the operator who wants to take a +snapshot before a planned host reboot or hardware migration. + +## Goals / Success Criteria + +- `./cli.py commit []` takes a snapshot of the running agent and + stores it as a local artifact. +- Without a slug argument the command shows the same interactive picker + as `start` (the list of active slugs). +- The committed artifact reference is stored in per-bottle state so + that the next `./cli.py resume ` automatically uses the + snapshot instead of rebuilding from the Dockerfile. +- `mark_preserved` is called so the state dir survives the normal + session-end cleanup. +- A backend-specific export hint is printed so operators know how to + migrate the snapshot. +- The command errors clearly on unsupported backends. + +## Non-goals + +- macOS-container backend support. +- Automatic commit on agent exit. +- Image push to a remote registry. +- Storing the image tag in the manifest or sharing it between operators. + +## Design + +### Docker image tag + +`bot-bottle-committed-:latest` — namespaced under `bot-bottle-` +to match existing image naming conventions; `committed` distinguishes it +from the build-time image (`bot-bottle-claude:latest`) and the +capability-block rebuild image (`bot-bottle-rebuilt-:latest`). + +### State storage + +A new plain-text file `committed-image` is added to the per-bottle state +directory: + +``` +~/.bot-bottle/state// + metadata.json + Dockerfile (capability-block override; optional) + committed-image (committed artifact reference; optional) + transcript/ +``` + +`bottle_state.committed_image_path(identity)` returns the path. +`write_committed_image` / `read_committed_image` are the read/write +helpers, matching the existing `per_bottle_dockerfile` pattern. Docker +stores a Docker tag in this file; smolmachines stores the absolute path +to the committed `.smolmachine` artifact. + +### `commit` command + +``` +./cli.py commit [] +``` + +1. Resolve slug (arg or interactive picker from `enumerate_active_agents`). +2. Check metadata and branch by backend. +3. For Docker, derive container name `bot-bottle-` and run + `docker commit bot-bottle-committed-:latest`. +4. For smolmachines, derive machine name `bot-bottle-` and run + `smolvm pack create --from-vm -o ~/.bot-bottle/state//committed-smolmachine`. +5. Write the Docker image tag or smolmachine artifact path to + `~/.bot-bottle/state//committed-image`. +6. Call `mark_preserved()` so the state dir survives session-end. +7. Print the resume hint and a backend-specific export example. + +### Resume from committed image + +`bot_bottle/backend/docker/launch.py` already rebuilds the agent image +at the top of the `launch` context manager. The change is a check +immediately before that step: + +```python +committed = read_committed_image(plan.slug) +if committed and docker_mod.image_exists(committed): + info(f"using committed image {committed!r}") + plan = dataclasses.replace( + plan, + agent_provision=dataclasses.replace(plan.agent_provision, image=committed), + ) +else: + docker_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) +``` + +Replacing `agent_provision.image` propagates to `plan.image` (a +property) and from there to the Compose spec renderer's `_agent_service` +→ `image:` field, so the container boots from the committed snapshot. +The build step is skipped entirely when a committed image is found and +exists locally. + +If the committed image has been deleted from the local daemon (e.g. +after `docker rmi` or a `docker system prune`), the launch falls back +to a normal Dockerfile build, matching the pre-commit behavior. + +### Resume from committed smolmachine + +`bot_bottle/backend/smolmachines/launch.py` checks the committed +reference before the normal Docker build -> pack cache path: + +```python +committed = read_committed_image(plan.slug) +if committed and Path(committed).is_file(): + return Path(committed) +return _ensure_smolmachine(plan.agent_image, dockerfile=plan.agent_dockerfile_path) +``` + +The returned path is passed to `smolvm machine create --from`, so the +resumed VM boots from the committed snapshot. If the artifact has been +deleted, launch falls back to the normal build and pack flow. + +## Testing strategy + +- Unit tests for `write_committed_image` / `read_committed_image` in + `tests/unit/test_bottle_state.py`, using the existing `_FakeHomeMixin` + pattern. +- Unit tests for `commit_container` in `tests/unit/test_docker_util_image.py`, + mocking `subprocess.run` and asserting on the `docker commit` argv. +- Unit tests for `cmd_commit` argument parsing, Docker commit, + smolmachines pack, and the unsupported backend error path, mocking + `enumerate_active_agents`, `commit_container`, and + `pack_create_from_vm`. +- Unit tests for the launch-step committed-image branch: patch + `read_committed_image` to return a tag, patch `image_exists` to return + True, and assert that `build_image` is not called and `plan.image` is + overridden. +- Unit tests for the smolmachines launch-step committed-artifact branch: + patch `read_committed_image` to return an existing path and assert the + normal `_ensure_smolmachine` path is skipped. diff --git a/tests/unit/test_backend_freezer.py b/tests/unit/test_backend_freezer.py new file mode 100644 index 0000000..9ed1cec --- /dev/null +++ b/tests/unit/test_backend_freezer.py @@ -0,0 +1,216 @@ +"""Unit: Freezer class hierarchy.""" + +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from bot_bottle import supervise, bottle_state +from bot_bottle.backend import ActiveAgent +from bot_bottle.backend.freeze import get_freezer +from bot_bottle.backend.docker.freezer import DockerFreezer +from bot_bottle.backend.macos_container.freezer import MacosContainerFreezer +from bot_bottle.backend.smolmachines.freezer import SmolmachinesFreezer + + +class _FakeHomeMixin: + def _setup_fake_home(self): + self._tmp = tempfile.TemporaryDirectory(prefix="freezer-test.") + original = supervise.bot_bottle_root + + def fake_root() -> Path: + return Path(self._tmp.name) / ".bot-bottle" + + supervise.bot_bottle_root = fake_root # type: ignore[assignment] + self._restore = lambda: setattr(supervise, "bot_bottle_root", original) + + def _teardown_fake_home(self): + self._restore() + self._tmp.cleanup() + + +def _make_agent(slug: str, backend: str = "docker") -> ActiveAgent: + return ActiveAgent( + backend_name=backend, + slug=slug, + agent_name="dev", + started_at="t", + services=(), + ) + + +class TestGetFreezer(unittest.TestCase): + def test_docker(self): + self.assertIsInstance(get_freezer("docker"), DockerFreezer) + + def test_empty_backend_gives_docker(self): + self.assertIsInstance(get_freezer(""), DockerFreezer) + + def test_macos_container(self): + self.assertIsInstance(get_freezer("macos-container"), MacosContainerFreezer) + + def test_smolmachines(self): + self.assertIsInstance(get_freezer("smolmachines"), SmolmachinesFreezer) + + def test_unknown_backend_dies(self): + with patch("bot_bottle.backend.freeze.die", side_effect=SystemExit("die")): + with self.assertRaises(SystemExit): + get_freezer("unknown-backend") + + +class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase): + """The base Freezer.commit() owns the shared post-freeze steps.""" + + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_writes_committed_image_and_marks_preserved(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer = get_freezer("docker") + agent = _make_agent(slug) + + with patch.object(freezer, "_freeze", return_value="bot-bottle-committed-dev-abc12:latest"), \ + patch("bot_bottle.backend.freeze.info"): + freezer.commit(agent) + + self.assertEqual( + "bot-bottle-committed-dev-abc12:latest", + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + def test_commit_slug_passes_correct_slug_to_freeze(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer = get_freezer("docker") + captured = {} + + def capture_freeze(agent: ActiveAgent) -> str: + captured["slug"] = agent.slug + return "some-ref" + + with patch.object(freezer, "_freeze", side_effect=capture_freeze), \ + patch("bot_bottle.backend.freeze.info"): + freezer.commit_slug(slug) + + self.assertEqual(slug, captured["slug"]) + + +class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_commits_container_and_records_image(self): + slug = "dev-abc12" + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="docker", + )) + freezer = DockerFreezer() + agent = _make_agent(slug) + + with patch("bot_bottle.backend.docker.freezer.commit_container") as mock_commit, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.docker.freezer.info"): + freezer.commit(agent) + + mock_commit.assert_called_once_with( + f"bot-bottle-{slug}", + f"bot-bottle-committed-{slug}:latest", + ) + self.assertEqual( + f"bot-bottle-committed-{slug}:latest", + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + +class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def _write_meta(self, slug: str) -> None: + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="macos-container", + )) + + def test_commits_running_container_without_stopping(self): + """Commit should exec-tar the running container, not stop it.""" + slug = "dev-abc12" + self._write_meta(slug) + freezer = MacosContainerFreezer() + agent = _make_agent(slug, "macos-container") + + with patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.macos_container.freezer.info"): + freezer.commit(agent) + + mock_commit.assert_called_once_with( + f"bot-bottle-{slug}", + f"bot-bottle-committed-{slug}:latest", + ) + self.assertEqual( + f"bot-bottle-committed-{slug}:latest", + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + +class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def _write_meta(self, slug: str) -> None: + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend="smolmachines", + )) + + def test_snapshots_running_vm_without_stopping(self): + """Commit should exec-tar the running VM, not stop it.""" + slug = "dev-abc12" + self._write_meta(slug) + freezer = SmolmachinesFreezer() + agent = _make_agent(slug, "smolmachines") + + with patch("bot_bottle.backend.smolmachines.freezer._snapshot_running_vm") as mock_snap, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.smolmachines.freezer.info"): + freezer.commit(agent) + + expected_binary = bottle_state.bottle_state_dir(slug) / "committed-smolmachine" + mock_snap.assert_called_once_with( + f"bot-bottle-{slug}", + f"bot-bottle-committed-{slug}:latest", + expected_binary, + ) + expected_sidecar = str(expected_binary.with_suffix(".smolmachine")) + self.assertEqual(expected_sidecar, bottle_state.read_committed_image(slug)) + self.assertTrue(bottle_state.is_preserved(slug)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_bottle_state.py b/tests/unit/test_bottle_state.py index acda6c9..8507b56 100644 --- a/tests/unit/test_bottle_state.py +++ b/tests/unit/test_bottle_state.py @@ -277,5 +277,56 @@ class TestBottleMetadataBackend(_FakeHomeMixin, unittest.TestCase): self.assertEqual("", loaded.backend) +class TestCommittedImage(_FakeHomeMixin, unittest.TestCase): + """write_committed_image / read_committed_image round-trip.""" + + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_returns_none_when_absent(self): + self.assertIsNone(bottle_state.read_committed_image("dev")) + + def test_write_then_read_roundtrip(self): + bottle_state.write_committed_image("dev", "bot-bottle-committed-dev:latest") + self.assertEqual( + "bot-bottle-committed-dev:latest", + bottle_state.read_committed_image("dev"), + ) + + def test_strips_trailing_newline_on_read(self): + path = bottle_state.committed_image_path("dev") + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("bot-bottle-committed-dev:latest\n\n") + self.assertEqual( + "bot-bottle-committed-dev:latest", + bottle_state.read_committed_image("dev"), + ) + + def test_isolated_per_slug(self): + bottle_state.write_committed_image("dev", "bot-bottle-committed-dev:latest") + bottle_state.write_committed_image("api", "bot-bottle-committed-api:latest") + self.assertEqual( + "bot-bottle-committed-dev:latest", + bottle_state.read_committed_image("dev"), + ) + self.assertEqual( + "bot-bottle-committed-api:latest", + bottle_state.read_committed_image("api"), + ) + + def test_path_under_state_dir(self): + path = bottle_state.committed_image_path("dev") + self.assertTrue(str(path).endswith("/.bot-bottle/state/dev/committed-image")) + + def test_empty_content_returns_none(self): + path = bottle_state.committed_image_path("dev") + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(" \n") + self.assertIsNone(bottle_state.read_committed_image("dev")) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_cli_commit.py b/tests/unit/test_cli_commit.py new file mode 100644 index 0000000..c88cc9a --- /dev/null +++ b/tests/unit/test_cli_commit.py @@ -0,0 +1,143 @@ +"""Unit: cli.py commit command.""" + +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +from bot_bottle.cli.commit import cmd_commit +from bot_bottle import supervise +from bot_bottle import bottle_state +from bot_bottle.backend.freeze import CommitCancelled + + +class _FakeHomeMixin: + def _setup_fake_home(self): + self._tmp = tempfile.TemporaryDirectory(prefix="cli-commit-test.") + original = supervise.bot_bottle_root + + def fake_root() -> Path: + return Path(self._tmp.name) / ".bot-bottle" + + supervise.bot_bottle_root = fake_root # type: ignore[assignment] + self._restore = lambda: setattr(supervise, "bot_bottle_root", original) + + def _teardown_fake_home(self): + self._restore() + self._tmp.cleanup() + + +class TestCmdCommitSlugArg(_FakeHomeMixin, unittest.TestCase): + """cmd_commit with an explicit slug delegates to get_freezer.""" + + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def _write_meta(self, slug: str, backend: str) -> None: + bottle_state.write_metadata(bottle_state.BottleMetadata( + identity=slug, agent_name="dev", cwd="", copy_cwd=False, + started_at="t", backend=backend, + )) + + def test_commits_docker_bottle(self): + slug = "dev-abc12" + self._write_meta(slug, "docker") + + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer + rc = cmd_commit([slug]) + + self.assertEqual(0, rc) + mock_gf.assert_called_once_with("docker") + mock_freezer.commit_slug.assert_called_once_with(slug) + + def test_empty_backend_passed_to_get_freezer(self): + """Old state dirs without a backend field pass '' to get_freezer.""" + slug = "dev-abc12" + self._write_meta(slug, "") + + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer + rc = cmd_commit([slug]) + + self.assertEqual(0, rc) + mock_gf.assert_called_once_with("") + + def test_commits_macos_container_bottle(self): + slug = "dev-abc12" + self._write_meta(slug, "macos-container") + + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer + rc = cmd_commit([slug]) + + self.assertEqual(0, rc) + mock_gf.assert_called_once_with("macos-container") + mock_freezer.commit_slug.assert_called_once_with(slug) + + def test_commits_smolmachines_bottle(self): + slug = "dev-abc12" + self._write_meta(slug, "smolmachines") + + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_gf.return_value = mock_freezer + rc = cmd_commit([slug]) + + self.assertEqual(0, rc) + mock_gf.assert_called_once_with("smolmachines") + + def test_returns_zero_on_commit_cancelled(self): + slug = "dev-abc12" + self._write_meta(slug, "macos-container") + + with patch("bot_bottle.cli.commit.get_freezer") as mock_gf: + mock_freezer = MagicMock() + mock_freezer.commit_slug.side_effect = CommitCancelled + mock_gf.return_value = mock_freezer + rc = cmd_commit([slug]) + + self.assertEqual(0, rc) + + +class TestCmdCommitNoActiveBottles(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() + + def test_dies_when_no_active_bottles_and_no_slug(self): + with patch( + "bot_bottle.cli.commit.enumerate_active_agents", return_value=[], + ), patch( + "bot_bottle.cli.commit.die", side_effect=SystemExit("die"), + ) as mock_die: + with self.assertRaises(SystemExit): + cmd_commit([]) + + mock_die.assert_called_once() + + def test_returns_zero_when_picker_cancelled(self): + active = MagicMock() + active.slug = "dev-abc12" + with patch( + "bot_bottle.cli.commit.enumerate_active_agents", return_value=[active], + ), patch( + "bot_bottle.cli.commit.tui.filter_select", return_value=None, + ): + rc = cmd_commit([]) + + self.assertEqual(0, rc) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py new file mode 100644 index 0000000..1152e63 --- /dev/null +++ b/tests/unit/test_docker_launch_committed_image.py @@ -0,0 +1,192 @@ +"""Unit: Docker launch step uses committed image when available.""" + +from __future__ import annotations + +import contextlib +import io +import tempfile +import unittest +from pathlib import Path +from typing import Any +from unittest import mock + +from bot_bottle.agent_provider import AgentProvisionPlan +from bot_bottle.backend import BottleSpec +from bot_bottle.backend.docker import launch as launch_mod +from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan +from bot_bottle.egress import EgressPlan +from bot_bottle.git_gate import GitGatePlan +from bot_bottle.manifest import ManifestIndex + + +_SLUG = "dev-abc12" +_COMMITTED_TAG = f"bot-bottle-committed-{_SLUG}:latest" +_DEFAULT_IMAGE = "bot-bottle-claude:latest" + +_IDX = ManifestIndex.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, +}) + + +def _plan(tmp: str) -> DockerBottlePlan: + stage = Path(tmp) + spec = BottleSpec( + manifest=_IDX, + agent_name="demo", + copy_cwd=False, + user_cwd=tmp, + identity=_SLUG, + ) + return DockerBottlePlan( + spec=spec, + manifest=_IDX.load_for_agent("demo"), + stage_dir=stage, + git_gate_plan=GitGatePlan( + slug=_SLUG, + entrypoint_script=stage / "entrypoint.sh", + hook_script=stage / "hook.sh", + access_hook_script=stage / "access-hook.sh", + upstreams=(), + ), + egress_plan=EgressPlan( + slug=_SLUG, + routes_path=stage / "egress.yaml", + routes=(), + token_env_map={}, + ), + supervise_plan=None, + agent_provision=AgentProvisionPlan( + template="claude", + command="claude", + prompt_mode="append_file", + image=_DEFAULT_IMAGE, + dockerfile="", + guest_home="/home/node", + instance_name=f"bot-bottle-{_SLUG}", + prompt_file=stage / "prompt.txt", + guest_env={}, + ), + slug=_SLUG, + forwarded_env={}, + use_runsc=False, + ) + + +class TestLaunchCommittedImage(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.mkdtemp(prefix="launch-committed-test.") + + def tearDown(self) -> None: + import shutil + shutil.rmtree(self._tmp, ignore_errors=True) + + def _run_launch( + self, + plan: DockerBottlePlan, + *, + committed_tag: str | None = None, + image_present: bool = True, + ) -> list[str]: + """Drive launch() through its full sequence with the committed-image + behaviour controlled by the arguments. Returns the images that were + passed to `build_image` (empty list if it was never called).""" + built: list[str] = [] + + def fake_build(image: str, ctx: str, *, dockerfile: str = "") -> None: + del ctx, dockerfile + built.append(image) + + with mock.patch.object( + launch_mod, "read_committed_image", return_value=committed_tag, + ), mock.patch.object( + launch_mod.docker_mod, "image_exists", return_value=image_present, + ), mock.patch.object( + launch_mod.docker_mod, "build_image", side_effect=fake_build, + ), mock.patch.object( + launch_mod, "egress_tls_init", + return_value=(Path("/egress_ca"), Path("/egress_cert")), + ), mock.patch.object( + launch_mod.network_mod, "network_name_for_slug", + return_value="bb-internal", + ), mock.patch.object( + launch_mod.network_mod, "network_egress_name_for_slug", + return_value="bb-egress", + ), mock.patch.object( + launch_mod, "bottle_plan_to_compose", + return_value={"services": {"agent": {}}}, + ), mock.patch.object( + launch_mod, "write_compose_file", + return_value=Path("/tmp/compose.yml"), + ), mock.patch.object(launch_mod, "compose_up"), \ + mock.patch.object(launch_mod, "compose_dump_logs"), \ + mock.patch.object(launch_mod, "compose_down"), \ + contextlib.redirect_stderr(io.StringIO()): + provision = mock.Mock(return_value=None) + with launch_mod.launch(plan, provision=provision): + pass + + return built + + def test_skips_build_when_committed_image_present(self) -> None: + plan = _plan(self._tmp) + built = self._run_launch(plan, committed_tag=_COMMITTED_TAG, image_present=True) + self.assertEqual([], built, "build_image should not be called when committed image exists") + + def test_uses_committed_image_in_compose_spec(self) -> None: + """The compose spec renderer receives the committed image tag via + plan.image — captured here by checking what bottle_plan_to_compose + was called with.""" + plan = _plan(self._tmp) + captured_plans: list[DockerBottlePlan] = [] + + def fake_compose(p: DockerBottlePlan) -> dict[str, Any]: + captured_plans.append(p) + return {"services": {"agent": {}}} + + with mock.patch.object( + launch_mod, "read_committed_image", return_value=_COMMITTED_TAG, + ), mock.patch.object( + launch_mod.docker_mod, "image_exists", return_value=True, + ), mock.patch.object( + launch_mod.docker_mod, "build_image", + ), mock.patch.object( + launch_mod, "egress_tls_init", + return_value=(Path("/egress_ca"), Path("/egress_cert")), + ), mock.patch.object( + launch_mod.network_mod, "network_name_for_slug", + return_value="bb-internal", + ), mock.patch.object( + launch_mod.network_mod, "network_egress_name_for_slug", + return_value="bb-egress", + ), mock.patch.object( + launch_mod, "bottle_plan_to_compose", side_effect=fake_compose, + ), mock.patch.object( + launch_mod, "write_compose_file", + return_value=Path("/tmp/compose.yml"), + ), mock.patch.object(launch_mod, "compose_up"), \ + mock.patch.object(launch_mod, "compose_dump_logs"), \ + mock.patch.object(launch_mod, "compose_down"), \ + contextlib.redirect_stderr(io.StringIO()): + provision = mock.Mock(return_value=None) + with launch_mod.launch(plan, provision=provision): + pass + + self.assertEqual(1, len(captured_plans)) + self.assertEqual(_COMMITTED_TAG, captured_plans[0].image) + + def test_falls_back_to_build_when_no_committed_image(self) -> None: + plan = _plan(self._tmp) + built = self._run_launch(plan, committed_tag=None) + self.assertEqual([_DEFAULT_IMAGE], built) + + def test_falls_back_to_build_when_committed_image_missing_from_daemon(self) -> None: + plan = _plan(self._tmp) + built = self._run_launch( + plan, committed_tag=_COMMITTED_TAG, image_present=False, + ) + self.assertEqual([_DEFAULT_IMAGE], built) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_docker_util_image.py b/tests/unit/test_docker_util_image.py index 67b5124..06acfde 100644 --- a/tests/unit/test_docker_util_image.py +++ b/tests/unit/test_docker_util_image.py @@ -67,5 +67,46 @@ class TestSave(unittest.TestCase): ) +class TestCommitContainer(unittest.TestCase): + def test_runs_docker_commit(self): + with patch.object( + docker_mod.subprocess, "run", return_value=_ok(), + ) as run, patch.object(docker_mod, "info"): + docker_mod.commit_container( + "bot-bottle-dev-abc12", + "bot-bottle-committed-dev-abc12:latest", + ) + argv = run.call_args.args[0] + self.assertEqual( + [ + "docker", "commit", + "bot-bottle-dev-abc12", + "bot-bottle-committed-dev-abc12:latest", + ], + argv, + ) + + def test_dies_on_docker_commit_failure(self): + with patch.object( + docker_mod.subprocess, "run", return_value=_fail("No such container"), + ), patch.object( + docker_mod, "die", side_effect=SystemExit("die"), + ) as die: + with self.assertRaises(SystemExit): + docker_mod.commit_container("missing-container", "some:tag") + die.assert_called_once() + self.assertIn("missing-container", die.call_args.args[0]) + + def test_die_message_includes_image_tag(self): + with patch.object( + docker_mod.subprocess, "run", return_value=_fail("boom"), + ), patch.object( + docker_mod, "die", side_effect=SystemExit("die"), + ) as die: + with self.assertRaises(SystemExit): + docker_mod.commit_container("ctr", "my-tag:v1") + self.assertIn("my-tag:v1", die.call_args.args[0]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 415884a..d9ae81c 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -9,8 +9,12 @@ from types import SimpleNamespace from typing import cast from unittest.mock import patch +from bot_bottle.agent_provider import AgentProvisionPlan +from bot_bottle.backend import BottleSpec from bot_bottle.backend.macos_container import launch from bot_bottle.backend.macos_container.bottle_plan import MacosContainerBottlePlan +from bot_bottle.egress import EgressPlan +from bot_bottle.git_gate import GitGatePlan from bot_bottle.manifest import ManifestIndex _MANIFEST = ManifestIndex.from_json_obj({ @@ -261,5 +265,80 @@ class TestMacosContainerLaunchArgv(unittest.TestCase): ) +def _build_plan(stage_dir: Path) -> MacosContainerBottlePlan: + return MacosContainerBottlePlan( + spec=cast(BottleSpec, SimpleNamespace()), + manifest=_MANIFEST, + stage_dir=stage_dir, + git_gate_plan=cast(GitGatePlan, SimpleNamespace(upstreams=())), + egress_plan=cast(EgressPlan, SimpleNamespace()), + supervise_plan=None, + agent_provision=AgentProvisionPlan( + template="claude", + command="claude", + prompt_mode="append_file", + image="bot-bottle-agent:latest", + dockerfile="/repo/Dockerfile", + guest_home="/home/node", + instance_name="bot-bottle-dev-abc", + prompt_file=stage_dir / "prompt.txt", + guest_env={}, + ), + slug="dev-abc", + forwarded_env={}, + ) + + +class TestMacosContainerLaunchCommittedImage(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.stage_dir = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def test_build_images_uses_committed_image_when_present(self): + plan = _build_plan(self.stage_dir) + calls = [] + + def fake_build(image: str, context: str, *, dockerfile: str = "") -> None: + calls.append((image, context, dockerfile)) + + with patch.object( + launch, "read_committed_image", + return_value="bot-bottle-committed-dev-abc:latest", + ), patch.object( + launch.container_mod, "image_exists", return_value=True, + ), patch.object( + launch.container_mod, "build_image", side_effect=fake_build, + ), patch.object(launch, "info"): + updated = launch._build_images(plan) + + self.assertEqual("bot-bottle-committed-dev-abc:latest", updated.image) + self.assertEqual(1, len(calls)) + self.assertEqual(launch.SIDECAR_BUNDLE_IMAGE, calls[0][0]) + + def test_build_images_builds_agent_when_committed_image_missing(self): + plan = _build_plan(self.stage_dir) + calls = [] + + def fake_build(image: str, context: str, *, dockerfile: str = "") -> None: + calls.append((image, context, dockerfile)) + + with patch.object( + launch, "read_committed_image", + return_value="bot-bottle-committed-dev-abc:latest", + ), patch.object( + launch.container_mod, "image_exists", return_value=False, + ), patch.object( + launch.container_mod, "build_image", side_effect=fake_build, + ): + updated = launch._build_images(plan) + + self.assertEqual("bot-bottle-agent:latest", updated.image) + self.assertEqual(2, len(calls)) + self.assertEqual("bot-bottle-agent:latest", calls[1][0]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_macos_container_util.py b/tests/unit/test_macos_container_util.py index 9789a29..9943163 100644 --- a/tests/unit/test_macos_container_util.py +++ b/tests/unit/test_macos_container_util.py @@ -73,6 +73,53 @@ resolver #2 ) self.assertTrue(run.call_args_list[-1].kwargs["check"]) + def test_commit_container_execs_tar_and_builds_image(self): + # stderr is bytes because subprocess.run uses stderr=PIPE without text=True + completed = util.subprocess.CompletedProcess( + args=[], returncode=0, stdout=b"", stderr=b"", + ) + dockerfile_text = "" + + def fake_build_image(image_tag: str, context: str, *, dockerfile: str = "") -> None: + nonlocal dockerfile_text + with open(dockerfile, encoding="utf-8") as f: + dockerfile_text = f.read() + + with patch.object(util.subprocess, "run", return_value=completed) as run, \ + patch.object(util, "build_image", side_effect=fake_build_image) as build_image, \ + patch.object(util, "info"): + util.commit_container( + "bot-bottle-dev-abc12", + "bot-bottle-committed-dev-abc12:latest", + ) + + argv = run.call_args.args[0] + self.assertEqual("container", argv[0]) + self.assertEqual("exec", argv[1]) + self.assertIn("bot-bottle-dev-abc12", argv) + self.assertIn("tar", argv) + self.assertIn("--directory=/", argv) + build_image.assert_called_once() + self.assertEqual( + "bot-bottle-committed-dev-abc12:latest", + build_image.call_args.args[0], + ) + self.assertIn("ADD rootfs.tar /\n", dockerfile_text) + self.assertIn("USER node\n", dockerfile_text) + self.assertIn("WORKDIR /home/node\n", dockerfile_text) + + def test_commit_container_dies_on_exec_tar_failure(self): + failed = util.subprocess.CompletedProcess( + args=[], returncode=1, stdout=b"", stderr=b"No such container", + ) + with patch.object(util.subprocess, "run", return_value=failed), \ + patch.object(util, "die", side_effect=SystemExit("die")) as die: + with self.assertRaises(SystemExit): + util.commit_container("missing-container", "some:tag") + + die.assert_called_once() + self.assertIn("missing-container", die.call_args.args[0]) + def test_build_image_restarts_builder_when_dns_mismatches(self): status = util.subprocess.CompletedProcess( args=[], diff --git a/tests/unit/test_smolmachines_launch_image.py b/tests/unit/test_smolmachines_launch_image.py index add3fab..e767089 100644 --- a/tests/unit/test_smolmachines_launch_image.py +++ b/tests/unit/test_smolmachines_launch_image.py @@ -16,6 +16,8 @@ from __future__ import annotations import tempfile import unittest from pathlib import Path +from types import SimpleNamespace +from typing import Any, cast from unittest.mock import patch from bot_bottle.backend.smolmachines import launch as _launch_mod @@ -141,5 +143,46 @@ class TestEnsureSmolmachine(unittest.TestCase): self.assertTrue(str(pack_args[1]).endswith(f"{digest}.smolmachine")) +class TestAgentFromPath(unittest.TestCase): + def _plan(self) -> Any: + return cast(Any, SimpleNamespace( + slug="dev-abc12", + agent_image="bot-bottle-claude:latest", + agent_dockerfile_path="/repo/Dockerfile", + )) + + def test_uses_committed_artifact_when_present(self): + with tempfile.TemporaryDirectory(prefix="committed-smolmachine.") as tmp: + artifact = Path(tmp) / "committed-smolmachine.smolmachine" + artifact.write_text("") + with patch.object( + _launch_mod, "read_committed_image", return_value=str(artifact), + ), patch.object( + _launch_mod, "_ensure_smolmachine", + ) as ensure, patch.object( + _launch_mod, "info", + ): + result = _launch_mod._agent_from_path(self._plan()) + + self.assertEqual(artifact, result) + ensure.assert_not_called() + + def test_falls_back_when_committed_artifact_missing(self): + packed = Path("/cache/agent.smolmachine") + with patch.object( + _launch_mod, "read_committed_image", + return_value="/missing/committed.smolmachine", + ), patch.object( + _launch_mod, "_ensure_smolmachine", return_value=packed, + ) as ensure: + result = _launch_mod._agent_from_path(self._plan()) + + self.assertEqual(packed, result) + ensure.assert_called_once_with( + "bot-bottle-claude:latest", + dockerfile="/repo/Dockerfile", + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_smolmachines_smolvm.py b/tests/unit/test_smolmachines_smolvm.py index e693c57..9d17f5b 100644 --- a/tests/unit/test_smolmachines_smolvm.py +++ b/tests/unit/test_smolmachines_smolvm.py @@ -24,6 +24,7 @@ from bot_bottle.backend.smolmachines.smolvm import ( machine_start, machine_stop, pack_create, + pack_create_from_vm, wait_exec_ready, ) @@ -63,6 +64,17 @@ class TestArgvShapes(unittest.TestCase): argv, ) + def test_pack_create_from_vm_argv(self): + with self._patch_run() as m: + pack_create_from_vm("bot-bottle-dev-abc12", Path("/tmp/committed")) + argv = m.call_args.args[0] + self.assertEqual( + ["smolvm", "pack", "create", + "--from-vm", "bot-bottle-dev-abc12", + "-o", "/tmp/committed"], + argv, + ) + def test_machine_create_minimal(self): with self._patch_run() as m: machine_create("agent-xyz") @@ -193,6 +205,14 @@ class TestErrorPath(unittest.TestCase): with self.assertRaises(SmolvmError): pack_create("missing:tag", Path("/tmp/out")) + def test_pack_create_from_vm_failure_raises(self): + with patch( + "bot_bottle.backend.smolmachines.smolvm.subprocess.run", + return_value=_fail("pack failed"), + ): + with self.assertRaises(SmolvmError): + pack_create_from_vm("bot-bottle-dev-abc12", Path("/tmp/out")) + def test_exec_failure_returns_result(self): # The in-VM command's exit code is what Bottle.exec sees; # `false` exiting non-zero is not a smolvm failure.