From d5762fa9d8d10343d556187c31495f8fa0f7c7fe Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 23 Jun 2026 07:46:38 +0000 Subject: [PATCH] refactor(freezer): drop Bottle from commit signature Freezer._freeze only ever used bottle.name, which is always f"bot-bottle-{agent.slug}". Remove the Bottle parameter from commit() and _freeze(), derive the container name from agent.slug directly in each subclass, and delete the _NamedBottle stub that existed solely to paper over this. --- bot_bottle/backend/__init__.py | 6 +-- bot_bottle/backend/docker/freezer.py | 7 +-- bot_bottle/backend/freeze.py | 51 ++++--------------- bot_bottle/backend/macos_container/freezer.py | 11 ++-- bot_bottle/backend/smolmachines/freezer.py | 7 +-- tests/unit/test_backend_freezer.py | 45 +++++++--------- 6 files changed, 44 insertions(+), 83 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 09ca6f1..06b279e 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -526,9 +526,9 @@ 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, so the dataclass must be -# fully defined first. +# 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 diff --git a/bot_bottle/backend/docker/freezer.py b/bot_bottle/backend/docker/freezer.py index 7d67ac6..5f50d96 100644 --- a/bot_bottle/backend/docker/freezer.py +++ b/bot_bottle/backend/docker/freezer.py @@ -2,7 +2,7 @@ from __future__ import annotations -from .. import ActiveAgent, Bottle +from .. import ActiveAgent from ..freeze import Freezer from .util import commit_container from ...log import info @@ -13,9 +13,10 @@ class DockerFreezer(Freezer): backend_name = "docker" - def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + def _freeze(self, agent: ActiveAgent) -> str: + container = f"bot-bottle-{agent.slug}" image_tag = f"bot-bottle-committed-{agent.slug}:latest" - commit_container(bottle.name, image_tag) + commit_container(container, image_tag) return image_tag def _export_hint(self, slug: str, image_ref: str) -> None: diff --git a/bot_bottle/backend/freeze.py b/bot_bottle/backend/freeze.py index 12aa6ad..d9a8013 100644 --- a/bot_bottle/backend/freeze.py +++ b/bot_bottle/backend/freeze.py @@ -6,16 +6,16 @@ print resume hint) and backend-specific subclasses in their respective backend directories. Entry points: - Freezer.commit(agent, bottle) — for use within a live launch context - Freezer.commit_slug(slug) — for cmd_commit when no live Bottle exists - get_freezer(backend_name) — factory + 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, Bottle, ExecResult +from . import ActiveAgent from ..bottle_state import mark_preserved, write_committed_image from ..log import die, info @@ -38,8 +38,8 @@ class Freezer(ABC): backend_name: str - def commit(self, agent: ActiveAgent, bottle: Bottle) -> None: - """Freeze `bottle` to a resumable artifact. + 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 @@ -48,14 +48,14 @@ class Freezer(ABC): Raises CommitCancelled if the user declines an interactive confirmation prompt (e.g. the macos-container stop prompt). """ - image_ref = self._freeze(agent, bottle) + 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, bottle: Bottle) -> str: + 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.""" @@ -65,11 +65,7 @@ class Freezer(ABC): Overridden by backends that provide a meaningful export command.""" def commit_slug(self, slug: str) -> None: - """Convenience entry for cmd_commit when no live Bottle is available. - - Constructs a minimal ActiveAgent from per-bottle state and a - name-only Bottle stub, then delegates to commit(). Freezer - subclasses must not call exec / exec_agent / cp_in on the stub.""" + """Convenience entry for cmd_commit when only a slug is available.""" from ..bottle_state import read_metadata metadata = read_metadata(slug) agent = ActiveAgent( @@ -79,34 +75,7 @@ class Freezer(ABC): started_at=metadata.started_at if metadata else "", services=(), ) - bottle: Bottle = _NamedBottle(f"bot-bottle-{slug}") - self.commit(agent, bottle) - - -class _NamedBottle(Bottle): - """Name-only Bottle stub for Freezer.commit_slug. - - Only `name` is meaningful. All runtime operations raise - NotImplementedError — Freezer._freeze implementations must only - access bottle.name.""" - - def __init__(self, name: str) -> None: - self.name = name - - def agent_argv(self, argv: list[str], *, tty: bool = True) -> list[str]: - raise NotImplementedError - - def exec_agent(self, argv: list[str], *, tty: bool = True) -> int: - raise NotImplementedError - - def exec(self, script: str, *, user: str = "node") -> ExecResult: - raise NotImplementedError - - def cp_in(self, host_path: str, container_path: str) -> None: - raise NotImplementedError - - def close(self) -> None: - pass + self.commit(agent) def get_freezer(backend_name: str) -> Freezer: diff --git a/bot_bottle/backend/macos_container/freezer.py b/bot_bottle/backend/macos_container/freezer.py index f405fe2..c4dff1e 100644 --- a/bot_bottle/backend/macos_container/freezer.py +++ b/bot_bottle/backend/macos_container/freezer.py @@ -8,7 +8,7 @@ from __future__ import annotations import sys -from .. import ActiveAgent, Bottle +from .. import ActiveAgent from ..freeze import CommitCancelled, Freezer from .util import commit_container, container_is_running, stop_container from ...log import info @@ -19,9 +19,10 @@ class MacosContainerFreezer(Freezer): backend_name = "macos-container" - def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + def _freeze(self, agent: ActiveAgent) -> str: + container = f"bot-bottle-{agent.slug}" image_tag = f"bot-bottle-committed-{agent.slug}:latest" - if container_is_running(bottle.name): + if container_is_running(container): sys.stderr.write( f"bot-bottle: bottle {agent.slug!r} is running; " "commit will stop it. Continue? [y/N] " @@ -30,8 +31,8 @@ class MacosContainerFreezer(Freezer): reply = _read_tty_line().strip().lower() if reply not in ("y", "yes"): raise CommitCancelled - stop_container(bottle.name) - commit_container(bottle.name, image_tag) + stop_container(container) + commit_container(container, image_tag) return image_tag def _export_hint(self, slug: str, image_ref: str) -> None: diff --git a/bot_bottle/backend/smolmachines/freezer.py b/bot_bottle/backend/smolmachines/freezer.py index d165a21..b0ef0a6 100644 --- a/bot_bottle/backend/smolmachines/freezer.py +++ b/bot_bottle/backend/smolmachines/freezer.py @@ -2,7 +2,7 @@ from __future__ import annotations -from .. import ActiveAgent, Bottle +from .. import ActiveAgent from ..freeze import Freezer from .smolvm import pack_create_from_vm from ...bottle_state import bottle_state_dir @@ -14,10 +14,11 @@ class SmolmachinesFreezer(Freezer): backend_name = "smolmachines" - def _freeze(self, agent: ActiveAgent, bottle: Bottle) -> str: + def _freeze(self, agent: ActiveAgent) -> str: + machine = f"bot-bottle-{agent.slug}" output = bottle_state_dir(agent.slug) / "committed-smolmachine" output.parent.mkdir(parents=True, exist_ok=True) - pack_create_from_vm(bottle.name, output) + pack_create_from_vm(machine, output) artifact = output.with_name(f"{output.name}.smolmachine") return str(artifact) diff --git a/tests/unit/test_backend_freezer.py b/tests/unit/test_backend_freezer.py index 29fd64e..36865f6 100644 --- a/tests/unit/test_backend_freezer.py +++ b/tests/unit/test_backend_freezer.py @@ -5,14 +5,13 @@ 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 import supervise, bottle_state from bot_bottle.backend import ActiveAgent from bot_bottle.backend.freeze import ( CommitCancelled, Freezer, - _NamedBottle, get_freezer, ) from bot_bottle.backend.docker.freezer import DockerFreezer @@ -46,10 +45,6 @@ def _make_agent(slug: str, backend: str = "docker") -> ActiveAgent: ) -def _make_bottle(slug: str) -> _NamedBottle: - return _NamedBottle(f"bot-bottle-{slug}") - - class TestGetFreezer(unittest.TestCase): def test_docker(self): self.assertIsInstance(get_freezer("docker"), DockerFreezer) @@ -85,12 +80,11 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase): started_at="t", backend="docker", )) freezer = get_freezer("docker") - agent = _make_agent(slug, "docker") - bottle = _make_bottle(slug) + 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, bottle) + freezer.commit(agent) self.assertEqual( "bot-bottle-committed-dev-abc12:latest", @@ -98,7 +92,7 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase): ) self.assertTrue(bottle_state.is_preserved(slug)) - def test_commit_slug_builds_correct_bottle_name(self): + 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, @@ -107,15 +101,15 @@ class TestFreezerBaseCommit(_FakeHomeMixin, unittest.TestCase): freezer = get_freezer("docker") captured = {} - def capture_freeze(agent, bottle): - captured["bottle_name"] = bottle.name + def capture_freeze(agent): + 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(f"bot-bottle-{slug}", captured["bottle_name"]) + self.assertEqual(slug, captured["slug"]) class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase): @@ -125,20 +119,19 @@ class TestDockerFreezer(_FakeHomeMixin, unittest.TestCase): def tearDown(self): self._teardown_fake_home() - def test_commits_container_and_returns_image_tag(self): + 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) - bottle = _make_bottle(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"): - bottle_state.write_metadata(bottle_state.BottleMetadata( - identity=slug, agent_name="dev", cwd="", copy_cwd=False, - started_at="t", backend="docker", - )) - freezer.commit(agent, bottle) + freezer.commit(agent) mock_commit.assert_called_once_with( f"bot-bottle-{slug}", @@ -169,14 +162,13 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): self._write_meta(slug) freezer = MacosContainerFreezer() agent = _make_agent(slug, "macos-container") - bottle = _make_bottle(slug) with patch("bot_bottle.backend.macos_container.freezer.container_is_running", return_value=False), \ 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, bottle) + freezer.commit(agent) mock_commit.assert_called_once_with( f"bot-bottle-{slug}", @@ -188,7 +180,6 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): self._write_meta(slug) freezer = MacosContainerFreezer() agent = _make_agent(slug, "macos-container") - bottle = _make_bottle(slug) with patch("bot_bottle.backend.macos_container.freezer.container_is_running", return_value=True), \ @@ -198,7 +189,7 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): 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, bottle) + freezer.commit(agent) mock_stop.assert_called_once_with(f"bot-bottle-{slug}") mock_commit.assert_called_once() @@ -208,7 +199,6 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): self._write_meta(slug) freezer = MacosContainerFreezer() agent = _make_agent(slug, "macos-container") - bottle = _make_bottle(slug) with patch("bot_bottle.backend.macos_container.freezer.container_is_running", return_value=True), \ @@ -217,7 +207,7 @@ class TestMacosContainerFreezer(_FakeHomeMixin, unittest.TestCase): patch("bot_bottle.backend.macos_container.freezer.stop_container") as mock_stop, \ patch("bot_bottle.backend.macos_container.freezer.commit_container") as mock_commit: with self.assertRaises(CommitCancelled): - freezer.commit(agent, bottle) + freezer.commit(agent) mock_stop.assert_not_called() mock_commit.assert_not_called() @@ -238,12 +228,11 @@ class TestSmolmachinesFreezer(_FakeHomeMixin, unittest.TestCase): )) freezer = SmolmachinesFreezer() agent = _make_agent(slug, "smolmachines") - bottle = _make_bottle(slug) with 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, bottle) + freezer.commit(agent) expected_output = bottle_state.bottle_state_dir(slug) / "committed-smolmachine" mock_pack.assert_called_once_with(f"bot-bottle-{slug}", expected_output)