From 6e73cc4d86cec1eec947207dd18a431c29eb7f05 Mon Sep 17 00:00:00 2001 From: codex Date: Tue, 23 Jun 2026 03:40:03 +0000 Subject: [PATCH] feat: support smolmachines bottle commit --- bot_bottle/backend/smolmachines/launch.py | 41 +++++++--- bot_bottle/backend/smolmachines/smolvm.py | 10 +++ bot_bottle/cli/commit.py | 70 ++++++++++++----- docs/prds/prd-new-commit-bottle-state.md | 77 ++++++++++++------- tests/unit/test_cli_commit.py | 53 ++++++++++--- .../test_docker_launch_committed_image.py | 45 +++++------ tests/unit/test_smolmachines_launch_image.py | 43 +++++++++++ tests/unit/test_smolmachines_smolvm.py | 20 +++++ 8 files changed, 266 insertions(+), 93 deletions(-) diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index d1a8eda..2bde131 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) @@ -386,6 +383,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..6d326ba 100644 --- a/bot_bottle/backend/smolmachines/smolvm.py +++ b/bot_bottle/backend/smolmachines/smolvm.py @@ -94,6 +94,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 --------------------------------------------------- diff --git a/bot_bottle/cli/commit.py b/bot_bottle/cli/commit.py index ed1a2c0..cab3135 100644 --- a/bot_bottle/cli/commit.py +++ b/bot_bottle/cli/commit.py @@ -1,20 +1,21 @@ -"""commit: freeze a running Docker bottle's container state to a local image. +"""commit: freeze a running bottle's state to a resumable artifact. -Runs `docker commit ` on the active agent -container and stores the image tag in per-bottle state so the next -`./cli.py resume ` boots from that snapshot instead of -rebuilding from the Dockerfile. - -Only the Docker backend is supported. Smolmachines VMs have no -container-level commit API in the current smolvm CLI surface. +Docker bottles are committed to a local Docker 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 pathlib import Path from ..backend import enumerate_active_agents from ..backend.docker.util import commit_container +from ..backend.smolmachines.smolvm import pack_create_from_vm +from ..bottle_state import bottle_state_dir from ..bottle_state import mark_preserved, read_metadata, write_committed_image from ..log import die, info from ._common import PROG @@ -23,6 +24,7 @@ from . import tui _COMMITTED_IMAGE_PREFIX = "bot-bottle-committed-" _DOCKER_BACKENDS = {"docker", ""} +_SMOLMACHINES_BACKEND = "smolmachines" def _committed_image_tag(slug: str) -> str: @@ -33,6 +35,19 @@ def _agent_container_name(slug: str) -> str: return f"bot-bottle-{slug}" +def _agent_machine_name(slug: str) -> str: + return f"bot-bottle-{slug}" + + +def _committed_smolmachine_output(slug: str) -> Path: + return bottle_state_dir(slug) / "committed-smolmachine" + + +def _committed_smolmachine_artifact(slug: str) -> Path: + output = _committed_smolmachine_output(slug) + return output.with_name(f"{output.name}.smolmachine") + + def cmd_commit(argv: list[str]) -> int: parser = argparse.ArgumentParser(prog=f"{PROG} commit", add_help=True) parser.add_argument( @@ -58,18 +73,33 @@ def cmd_commit(argv: list[str]) -> int: metadata = read_metadata(slug) backend = metadata.backend if metadata else "" - if backend not in _DOCKER_BACKENDS: + if backend in _DOCKER_BACKENDS: + container = _agent_container_name(slug) + image_tag = _committed_image_tag(slug) + + commit_container(container, image_tag) + write_committed_image(slug, image_tag) + mark_preserved(slug) + info(f"to resume from this snapshot: ./cli.py resume {slug}") + info(f"to export for migration: docker save {image_tag} -o {slug}.tar") + return 0 + + if backend == _SMOLMACHINES_BACKEND: + machine = _agent_machine_name(slug) + output = _committed_smolmachine_output(slug) + output.parent.mkdir(parents=True, exist_ok=True) + pack_create_from_vm(machine, output) + artifact = _committed_smolmachine_artifact(slug) + write_committed_image(slug, str(artifact)) + mark_preserved(slug) + info(f"to resume from this snapshot: ./cli.py resume {slug}") + info(f"to export for migration: cp {artifact} {slug}.smolmachine") + return 0 + + if backend: die( - f"commit is only supported for the docker backend; " + f"commit is only supported for the docker and smolmachines backends; " f"bottle {slug!r} uses {backend!r}" ) - - container = _agent_container_name(slug) - image_tag = _committed_image_tag(slug) - - commit_container(container, image_tag) - write_committed_image(slug, image_tag) - mark_preserved(slug) - info(f"to resume from this snapshot: ./cli.py resume {slug}") - info(f"to export for migration: docker save {image_tag} -o {slug}.tar") - return 0 + die(f"commit cannot determine the backend for bottle {slug!r}") + return 1 diff --git a/docs/prds/prd-new-commit-bottle-state.md b/docs/prds/prd-new-commit-bottle-state.md index 055bc8d..93f6ef1 100644 --- a/docs/prds/prd-new-commit-bottle-state.md +++ b/docs/prds/prd-new-commit-bottle-state.md @@ -7,10 +7,11 @@ ## Summary -Add a `commit` CLI command that freezes a running Docker bottle's -container state to a named Docker image. Operators can then resume the -bottle from that exact filesystem snapshot, or export the image with -`docker save` to migrate work to a different host. +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 @@ -29,30 +30,29 @@ snapshot before a planned host reboot or hardware migration. ## Goals / Success Criteria -- `./cli.py commit []` takes a snapshot of the running Docker - agent container and stores it as a local Docker image. +- `./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 image tag is stored in per-bottle state so that the next - `./cli.py resume ` automatically uses the committed image instead - of rebuilding from the Dockerfile. +- 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 `docker save` hint is printed so operators know how to export the - image for migration. -- The command errors clearly on non-Docker backends (smolmachines does - not expose a container-level commit API in its current CLI surface). +- A backend-specific export hint is printed so operators know how to + migrate the snapshot. +- The command errors clearly on unsupported backends. ## Non-goals -- Smolmachines or macOS-container backend support. +- 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 -### Image tag +### Docker image tag `bot-bottle-committed-:latest` — namespaced under `bot-bottle-` to match existing image naming conventions; `committed` distinguishes it @@ -68,13 +68,15 @@ directory: ~/.bot-bottle/state// metadata.json Dockerfile (capability-block override; optional) - committed-image (committed image tag; 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. +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 @@ -83,14 +85,15 @@ helpers, matching the existing `per_bottle_dockerfile` pattern. ``` 1. Resolve slug (arg or interactive picker from `enumerate_active_agents`). -2. Check metadata: if `backend` is set and is not `docker`, die with a - clear "not supported" error. -3. Derive container name: `bot-bottle-` (matches the agent - provision plan's `instance_name` convention). -4. Run `docker commit bot-bottle-committed-:latest`. -5. Write the image tag to `~/.bot-bottle/state//committed-image`. +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 `docker save` export example. +7. Print the resume hint and a backend-specific export example. ### Resume from committed image @@ -120,6 +123,22 @@ 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 @@ -127,10 +146,14 @@ to a normal Dockerfile build, matching the pre-commit behavior. 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 and the "unsupported - backend" error path, mocking `enumerate_active_agents` and - `commit_container`. +- 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_cli_commit.py b/tests/unit/test_cli_commit.py index 4930882..38eb7d5 100644 --- a/tests/unit/test_cli_commit.py +++ b/tests/unit/test_cli_commit.py @@ -5,9 +5,15 @@ from __future__ import annotations import tempfile import unittest from pathlib import Path -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch -from bot_bottle.cli.commit import cmd_commit, _committed_image_tag, _agent_container_name +from bot_bottle.cli.commit import ( + cmd_commit, + _agent_container_name, + _committed_image_tag, + _committed_smolmachine_artifact, + _committed_smolmachine_output, +) from bot_bottle import supervise from bot_bottle import bottle_state @@ -41,6 +47,16 @@ class TestCommitHelpers(unittest.TestCase): _agent_container_name("dev-abc12"), ) + def test_committed_smolmachine_paths(self): + output = _committed_smolmachine_output("dev-abc12") + artifact = _committed_smolmachine_artifact("dev-abc12") + self.assertTrue(str(output).endswith( + "/.bot-bottle/state/dev-abc12/committed-smolmachine" + )) + self.assertTrue(str(artifact).endswith( + "/.bot-bottle/state/dev-abc12/committed-smolmachine.smolmachine" + )) + class TestCmdCommitSlugArg(_FakeHomeMixin, unittest.TestCase): """cmd_commit with an explicit slug bypasses the TUI picker.""" @@ -117,14 +133,14 @@ class TestCmdCommitSlugArg(_FakeHomeMixin, unittest.TestCase): mock_commit.assert_called_once() -class TestCmdCommitNonDockerBackend(_FakeHomeMixin, unittest.TestCase): +class TestCmdCommitSmolmachinesBackend(_FakeHomeMixin, unittest.TestCase): def setUp(self): self._setup_fake_home() def tearDown(self): self._teardown_fake_home() - def test_dies_for_smolmachines_backend(self): + def test_packs_smolmachines_bottle(self): slug = "dev-abc12" bottle_state.write_metadata(bottle_state.BottleMetadata( identity=slug, agent_name="dev", cwd="", copy_cwd=False, @@ -132,13 +148,30 @@ class TestCmdCommitNonDockerBackend(_FakeHomeMixin, unittest.TestCase): )) with patch( - "bot_bottle.cli.commit.die", side_effect=SystemExit("die"), - ) as mock_die: - with self.assertRaises(SystemExit): - cmd_commit([slug]) + "bot_bottle.cli.commit.pack_create_from_vm", + ) as mock_pack, patch( + "bot_bottle.cli.commit.info", + ): + rc = cmd_commit([slug]) - mock_die.assert_called_once() - self.assertIn("smolmachines", mock_die.call_args.args[0]) + self.assertEqual(0, rc) + mock_pack.assert_called_once_with( + f"bot-bottle-{slug}", + _committed_smolmachine_output(slug), + ) + self.assertEqual( + str(_committed_smolmachine_artifact(slug)), + bottle_state.read_committed_image(slug), + ) + self.assertTrue(bottle_state.is_preserved(slug)) + + +class TestCmdCommitUnsupportedBackend(_FakeHomeMixin, unittest.TestCase): + def setUp(self): + self._setup_fake_home() + + def tearDown(self): + self._teardown_fake_home() def test_dies_for_macos_container_backend(self): slug = "dev-abc12" diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py index 76ca613..be253d7 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -7,6 +7,7 @@ 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 @@ -73,36 +74,28 @@ def _plan(tmp: str) -> DockerBottlePlan: ) -def _std_mocks(test, plan): - """Context manager providing the standard launch-step mocks needed to - get through the non-image parts of `launch()` without real Docker.""" - return mock.patch.multiple( - launch_mod, - egress_tls_init=mock.DEFAULT, - network_mod=mock.DEFAULT, - bottle_plan_to_compose=mock.DEFAULT, - write_compose_file=mock.DEFAULT, - compose_up=mock.DEFAULT, - compose_dump_logs=mock.DEFAULT, - compose_down=mock.DEFAULT, - ) - - class TestLaunchCommittedImage(unittest.TestCase): - def setUp(self): + def setUp(self) -> None: self._tmp = tempfile.mkdtemp(prefix="launch-committed-test.") - def tearDown(self): + def tearDown(self) -> None: import shutil shutil.rmtree(self._tmp, ignore_errors=True) - def _run_launch(self, plan, *, committed_tag=None, image_present=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 = [] + built: list[str] = [] - def fake_build(image, ctx, *, dockerfile=""): + def fake_build(image: str, ctx: str, *, dockerfile: str = "") -> None: + del ctx, dockerfile built.append(image) with mock.patch.object( @@ -136,19 +129,19 @@ class TestLaunchCommittedImage(unittest.TestCase): return built - def test_skips_build_when_committed_image_present(self): + 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): + 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 = [] + captured_plans: list[DockerBottlePlan] = [] - def fake_compose(p): + def fake_compose(p: DockerBottlePlan) -> dict[str, Any]: captured_plans.append(p) return {"services": {"agent": {}}} @@ -183,12 +176,12 @@ class TestLaunchCommittedImage(unittest.TestCase): 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): + 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): + 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, 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.