From 5f473eb842db11eb48499db13931b844de9265d4 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 08:42:03 +0000 Subject: [PATCH] fix(smolmachines): stop VM before pack commit, with confirm prompt smolvm pack create --from-vm requires the VM to be stopped. Add machine_is_running() to smolvm.py (via machine ls --json state field), and add the same confirm-stop flow to SmolmachinesFreezer that was originally designed for macos-container: if running, prompt the user, stop the VM, then pack. Already-stopped VMs are packed directly. --- bot_bottle/backend/smolmachines/freezer.py | 32 +++++++++++++-- bot_bottle/backend/smolmachines/smolvm.py | 16 ++++++++ tests/unit/test_backend_freezer.py | 47 ++++++++++++++++++++-- 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/bot_bottle/backend/smolmachines/freezer.py b/bot_bottle/backend/smolmachines/freezer.py index b0ef0a6..003228f 100644 --- a/bot_bottle/backend/smolmachines/freezer.py +++ b/bot_bottle/backend/smolmachines/freezer.py @@ -2,15 +2,31 @@ from __future__ import annotations +import sys + from .. import ActiveAgent -from ..freeze import Freezer -from .smolvm import pack_create_from_vm +from ..freeze import CommitCancelled, Freezer +from .smolvm import machine_is_running, machine_stop, pack_create_from_vm from ...bottle_state import bottle_state_dir from ...log import info +def _read_tty_line() -> str: + """Read one line from /dev/tty, falling back to stdin.""" + try: + with open("/dev/tty", "r", encoding="utf-8") as tty: + return tty.readline().rstrip("\n") + except OSError: + return sys.stdin.readline().rstrip("\n") + + class SmolmachinesFreezer(Freezer): - """Freezes a smolmachines bottle via `smolvm pack create --from-vm`.""" + """Freezes a smolmachines bottle via `smolvm pack create --from-vm`. + + `smolvm pack create --from-vm` requires the VM to be stopped first. + If the VM is running the freezer prompts the user to confirm the stop + before proceeding. The VM remains stopped after commit; use + `./cli.py resume` to restart.""" backend_name = "smolmachines" @@ -18,6 +34,16 @@ class SmolmachinesFreezer(Freezer): machine = f"bot-bottle-{agent.slug}" output = bottle_state_dir(agent.slug) / "committed-smolmachine" output.parent.mkdir(parents=True, exist_ok=True) + if machine_is_running(machine): + sys.stderr.write( + f"bot-bottle: bottle {agent.slug!r} is running; " + "commit will stop it. Continue? [y/N] " + ) + sys.stderr.flush() + reply = _read_tty_line().strip().lower() + if reply not in ("y", "yes"): + raise CommitCancelled + machine_stop(machine) pack_create_from_vm(machine, output) artifact = output.with_name(f"{output.name}.smolmachine") return str(artifact) diff --git a/bot_bottle/backend/smolmachines/smolvm.py b/bot_bottle/backend/smolmachines/smolvm.py index 6d326ba..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 @@ -153,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/tests/unit/test_backend_freezer.py b/tests/unit/test_backend_freezer.py index b0b9371..ca8c3db 100644 --- a/tests/unit/test_backend_freezer.py +++ b/tests/unit/test_backend_freezer.py @@ -183,16 +183,21 @@ class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase): def tearDown(self): self._teardown_fake_home() - def test_packs_vm_and_records_artifact(self): - slug = "dev-abc12" + 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_packs_stopped_vm_directly(self): + slug = "dev-abc12" + self._write_meta(slug) freezer = SmolmachinesFreezer() agent = _make_agent(slug, "smolmachines") - with patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack, \ + with patch("bot_bottle.backend.smolmachines.freezer.machine_is_running", + return_value=False), \ + patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack, \ patch("bot_bottle.backend.freeze.info"), \ patch("bot_bottle.backend.smolmachines.freezer.info"): freezer.commit(agent) @@ -203,6 +208,42 @@ class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase): self.assertEqual(expected_artifact, bottle_state.read_committed_image(slug)) self.assertTrue(bottle_state.is_preserved(slug)) + def test_stops_running_vm_on_yes(self): + slug = "dev-abc12" + self._write_meta(slug) + freezer = SmolmachinesFreezer() + agent = _make_agent(slug, "smolmachines") + + with patch("bot_bottle.backend.smolmachines.freezer.machine_is_running", + return_value=True), \ + patch("bot_bottle.backend.smolmachines.freezer._read_tty_line", return_value="y"), \ + patch("bot_bottle.backend.smolmachines.freezer.machine_stop") as mock_stop, \ + patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack, \ + patch("bot_bottle.backend.freeze.info"), \ + patch("bot_bottle.backend.smolmachines.freezer.info"): + freezer.commit(agent) + + mock_stop.assert_called_once_with(f"bot-bottle-{slug}") + mock_pack.assert_called_once() + + def test_raises_commit_cancelled_on_no(self): + slug = "dev-abc12" + self._write_meta(slug) + freezer = SmolmachinesFreezer() + agent = _make_agent(slug, "smolmachines") + + with patch("bot_bottle.backend.smolmachines.freezer.machine_is_running", + return_value=True), \ + patch("bot_bottle.backend.smolmachines.freezer._read_tty_line", return_value="n"), \ + patch("bot_bottle.backend.smolmachines.freezer.machine_stop") as mock_stop, \ + patch("bot_bottle.backend.smolmachines.freezer.pack_create_from_vm") as mock_pack: + from bot_bottle.backend.freeze import CommitCancelled + with self.assertRaises(CommitCancelled): + freezer.commit(agent) + + mock_stop.assert_not_called() + mock_pack.assert_not_called() + if __name__ == "__main__": unittest.main()