From 1b4f64767a413b73e77b27e699972cc5a5bbc49e Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 9 Jul 2026 17:25:50 +0000 Subject: [PATCH 1/9] Add cached image quickstart --- bot_bottle/backend/__init__.py | 3 + bot_bottle/backend/docker/compose.py | 9 ++- bot_bottle/backend/docker/launch.py | 31 +++++++- bot_bottle/backend/docker/util.py | 40 ++++++++++ bot_bottle/cli/start.py | 23 ++++++ bot_bottle/config_store.py | 79 +++++++++++++++++++ bot_bottle/image_cache.py | 38 +++++++++ bot_bottle/store_manager.py | 4 + tests/unit/test_cli_start_headless.py | 12 +++ tests/unit/test_cli_start_selector.py | 21 +++++ tests/unit/test_compose.py | 7 ++ tests/unit/test_config_store.py | 44 +++++++++++ .../test_docker_launch_committed_image.py | 24 ++++++ tests/unit/test_docker_util_image.py | 17 ++++ tests/unit/test_smolmachines_launch_image.py | 50 ++++++++++++ 15 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 bot_bottle/config_store.py create mode 100644 bot_bottle/image_cache.py create mode 100644 tests/unit/test_config_store.py diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index ed8b894..e13fd4b 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -78,6 +78,9 @@ class BottleSpec: # True when launched via --headless (no TTY, no interactive prompts). # The git-gate host-key preflight uses this to error rather than prompt. headless: bool = False + # Image startup policy. "fresh" preserves the normal build path; + # "cached" reuses the current local image/artifact without rebuilding. + image_policy: str = "fresh" @dataclass(frozen=True) diff --git a/bot_bottle/backend/docker/compose.py b/bot_bottle/backend/docker/compose.py index 9929610..b6153ea 100644 --- a/bot_bottle/backend/docker/compose.py +++ b/bot_bottle/backend/docker/compose.py @@ -180,10 +180,6 @@ def _sidecar_bundle_service(plan: DockerBottlePlan) -> dict[str, Any]: service: dict[str, Any] = { "image": SIDECAR_BUNDLE_IMAGE, - "build": { - "context": _REPO_DIR, - "dockerfile": SIDECAR_BUNDLE_DOCKERFILE, - }, "container_name": sidecar_bundle_container_name(plan.slug), "networks": { "internal": {"aliases": internal_aliases}, @@ -192,6 +188,11 @@ def _sidecar_bundle_service(plan: DockerBottlePlan) -> dict[str, Any]: "environment": env, "volumes": volumes, } + if plan.spec.image_policy != "cached": + service["build"] = { + "context": _REPO_DIR, + "dockerfile": SIDECAR_BUNDLE_DOCKERFILE, + } return service diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index fe49903..7a03f47 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -41,7 +41,8 @@ from ...git_gate import ( provision_git_gate_dynamic_keys, revoke_git_gate_provisioned_keys, ) -from ...log import info, warn +from ...image_cache import warn_if_stale +from ...log import die, info, warn from . import network as network_mod from . import util as docker_mod from .bottle import DockerBottle @@ -63,6 +64,7 @@ from .compose import ( write_compose_file, ) from .egress import egress_tls_init +from .sidecar_bundle import SIDECAR_BUNDLE_IMAGE # Where the repo root lives, for `docker build` context. Computed once. @@ -100,12 +102,39 @@ def launch( # Dockerfile. Sidecar images get built lazily by `docker compose # up` via the renderer's `build:` directives. committed = read_committed_image(plan.slug) + cached_policy = plan.spec.image_policy == "cached" if committed and docker_mod.image_exists(committed): info(f"using committed image {committed!r}") + if cached_policy: + warn_if_stale( + f"agent image {committed!r}", + docker_mod.image_created_at(committed), + ) plan = dataclasses.replace( plan, agent_provision=dataclasses.replace(plan.agent_provision, image=committed), ) + elif cached_policy: + if not docker_mod.image_exists(plan.image): + die( + f"cached agent image {plan.image!r} not found; " + "run without --cached-images to build it" + ) + if not docker_mod.image_exists(SIDECAR_BUNDLE_IMAGE): + die( + f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " + "run without --cached-images to build it" + ) + info(f"using cached agent image {plan.image!r}") + info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") + warn_if_stale( + f"agent image {plan.image!r}", + docker_mod.image_created_at(plan.image), + ) + warn_if_stale( + f"sidecar image {SIDECAR_BUNDLE_IMAGE!r}", + docker_mod.image_created_at(SIDECAR_BUNDLE_IMAGE), + ) else: docker_mod.build_image( plan.image, _REPO_DIR, diff --git a/bot_bottle/backend/docker/util.py b/bot_bottle/backend/docker/util.py index 8ac0339..673b8a2 100644 --- a/bot_bottle/backend/docker/util.py +++ b/bot_bottle/backend/docker/util.py @@ -4,6 +4,7 @@ existence, and building images.""" from __future__ import annotations +from datetime import datetime, timezone import re import shutil import subprocess @@ -186,6 +187,45 @@ def image_id(ref: str) -> str: return r.stdout.strip() +def image_created_at(ref: str) -> datetime: + """Return Docker's image Created timestamp as an aware UTC datetime.""" + r = subprocess.run( + ["docker", "image", "inspect", "--format", "{{.Created}}", ref], + capture_output=True, + text=True, + check=False, + ) + if r.returncode != 0: + die( + f"docker image inspect for {ref!r} failed: " + f"{(r.stderr or '').strip() or ''}" + ) + raw = r.stdout.strip() + try: + return _parse_docker_timestamp(raw) + except ValueError: + die(f"docker image inspect for {ref!r} returned invalid Created timestamp: {raw!r}") + + +def _parse_docker_timestamp(raw: str) -> datetime: + text = raw.strip() + if text.endswith("Z"): + text = text[:-1] + "+00:00" + dot = text.find(".") + if dot != -1: + tz_plus = text.find("+", dot) + tz_minus = text.find("-", dot) + tz_candidates = [pos for pos in (tz_plus, tz_minus) if pos != -1] + if tz_candidates: + tz_pos = min(tz_candidates) + frac = text[dot + 1:tz_pos] + text = text[:dot + 1] + frac[:6].ljust(6, "0") + text[tz_pos:] + dt = datetime.fromisoformat(text) + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) + + def save(ref: str, output: str) -> None: """`docker save REF -o OUTPUT`. Writes a tarball of the image layers + manifest to the host path. Used by smolmachines diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index 5a22ce0..bb2a7a6 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -63,6 +63,14 @@ def cmd_start(argv: list[str]) -> int: "skip all prompts. For orchestrators, CI, and webhooks." ), ) + parser.add_argument( + "--cached-images", + action="store_true", + help=( + "quickstart with existing local agent and sidecar images; " + "only valid with --headless" + ), + ) parser.add_argument( "--bottle", action="append", @@ -95,6 +103,8 @@ def cmd_start(argv: list[str]) -> int: help="agent name defined in bot-bottle.json (omit to pick interactively)", ) args = parser.parse_args(argv) + if args.cached_images and not args.headless: + die("--cached-images is only supported with --headless") dry_run = args.dry_run or os.environ.get("BOT_BOTTLE_DRY_RUN") == "1" @@ -142,6 +152,10 @@ def cmd_start(argv: list[str]) -> int: label, color = tui.name_color_modal(default_label=agent_name) label, color = _resolve_unique_label(label, color) + image_policy = _select_image_policy() + if image_policy is None: + return 0 + spec = BottleSpec( manifest=manifest, agent_name=agent_name, @@ -150,6 +164,7 @@ def cmd_start(argv: list[str]) -> int: label=label, color=color, bottle_names=bottle_names, + image_policy=image_policy, ) return _launch_bottle( spec, @@ -210,6 +225,7 @@ def _start_headless( color=args.color or "", bottle_names=bottle_names, headless=True, + image_policy="cached" if args.cached_images else "fresh", ) return _launch_bottle( spec, @@ -390,6 +406,13 @@ def _text_prompt_yes() -> bool: return reply in ("y", "Y", "yes", "YES") +def _select_image_policy() -> str | None: + return tui.filter_select( + ["fresh", "cached"], + title="Select image startup mode", + ) + + def _text_render_preflight(): def _render(plan: DockerBottlePlan, backend_name: str) -> None: print(file=sys.stderr) diff --git a/bot_bottle/config_store.py b/bot_bottle/config_store.py new file mode 100644 index 0000000..93fda93 --- /dev/null +++ b/bot_bottle/config_store.py @@ -0,0 +1,79 @@ +"""SQLite-backed bot-bottle configuration store.""" + +from __future__ import annotations + +from pathlib import Path + +try: + from .db_store import DbStore + from .migrations import TableMigrations + from .supervise_types import host_db_path +except ImportError: + from db_store import DbStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from migrations import TableMigrations # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from supervise_types import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + + +CACHED_IMAGE_STALE_WARNING_DAYS = "cached_image_stale_warning_days" +DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS = 1 + + +class ConfigStore(DbStore): + """SQLite key/value configuration for host-side bot-bottle settings.""" + + def __init__(self, db_path: Path | None = None) -> None: + migrations = TableMigrations("config_store", [ + # v1 — generic string key/value settings + """ + CREATE TABLE IF NOT EXISTS bot_bottle_config ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL + ) + """, + ]) + super().__init__(db_path or host_db_path(), migrations) + + def get(self, key: str) -> str | None: + if not self.db_path.is_file(): + return None + with self._connect() as conn: + row = conn.execute( + "SELECT value FROM bot_bottle_config WHERE key = ?", + (key,), + ).fetchone() + return str(row["value"]) if row is not None else None + + def set(self, key: str, value: str) -> Path: + with self._connect() as conn: + conn.execute( + """ + INSERT INTO bot_bottle_config (key, value) + VALUES (?, ?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value + """, + (key, value), + ) + self._chmod() + return self.db_path + + def get_int(self, key: str, default: int) -> int: + raw = self.get(key) + if raw is None: + return default + try: + return int(raw) + except ValueError: + return default + + def cached_image_stale_warning_days(self) -> int: + return self.get_int( + CACHED_IMAGE_STALE_WARNING_DAYS, + DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, + ) + + +__all__ = [ + "CACHED_IMAGE_STALE_WARNING_DAYS", + "DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS", + "ConfigStore", +] diff --git a/bot_bottle/image_cache.py b/bot_bottle/image_cache.py new file mode 100644 index 0000000..85e2363 --- /dev/null +++ b/bot_bottle/image_cache.py @@ -0,0 +1,38 @@ +"""Shared helpers for cached-image quickstart warnings.""" + +from __future__ import annotations + +from datetime import datetime, timezone +from pathlib import Path + +try: + from .config_store import ConfigStore + from .log import warn +except ImportError: + from config_store import ConfigStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from log import warn # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + + +def warn_if_stale(label: str, created_at: datetime) -> None: + threshold_days = ConfigStore().cached_image_stale_warning_days() + if threshold_days < 0: + return + now = datetime.now(timezone.utc) + created = created_at.astimezone(timezone.utc) + age = now - created + if age.total_seconds() <= threshold_days * 86400: + return + warn( + f"cached {label} is {age.days} day(s) old; " + "quickstart does not verify it matches the current Dockerfile/context" + ) + + +def warn_if_stale_path(label: str, path: Path) -> None: + warn_if_stale( + label, + datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc), + ) + + +__all__ = ["warn_if_stale", "warn_if_stale_path"] diff --git a/bot_bottle/store_manager.py b/bot_bottle/store_manager.py index 70b1f46..bf14a49 100644 --- a/bot_bottle/store_manager.py +++ b/bot_bottle/store_manager.py @@ -6,9 +6,11 @@ from pathlib import Path try: from .audit_store import AuditStore + from .config_store import ConfigStore from .queue_store import QueueStore except ImportError: from audit_store import AuditStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module + from config_store import ConfigStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module from queue_store import QueueStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module _instance: StoreManager | None = None @@ -50,11 +52,13 @@ class StoreManager: return ( QueueStore("", self.db_path).is_migrated() and AuditStore(self.db_path).is_migrated() + and ConfigStore(self.db_path).is_migrated() ) def migrate(self) -> None: QueueStore("", self.db_path).migrate() AuditStore(self.db_path).migrate() + ConfigStore(self.db_path).migrate() __all__ = ["StoreManager"] diff --git a/tests/unit/test_cli_start_headless.py b/tests/unit/test_cli_start_headless.py index e4e6717..3d9934d 100644 --- a/tests/unit/test_cli_start_headless.py +++ b/tests/unit/test_cli_start_headless.py @@ -184,6 +184,18 @@ class TestCmdStartHeadless(unittest.TestCase): ) self.assertEqual("docker", self._launch_mock.call_args[1]["backend_name"]) + def test_cached_images_sets_cached_policy(self): + start_mod.cmd_start( + ["--headless", "--cached-images", "researcher", "--bottle", "claude", + "--prompt", "Do it"] + ) + self.assertEqual("cached", self._spec().image_policy) + + def test_cached_images_requires_headless(self): + with self.assertRaises(Die): + start_mod.cmd_start(["--cached-images", "researcher"]) + self._launch_mock.assert_not_called() + class TestPrepareWithPreflight(unittest.TestCase): """prepare_with_preflight calls render_preflight with the plan and backend name.""" diff --git a/tests/unit/test_cli_start_selector.py b/tests/unit/test_cli_start_selector.py index b091bfb..750654e 100644 --- a/tests/unit/test_cli_start_selector.py +++ b/tests/unit/test_cli_start_selector.py @@ -57,6 +57,12 @@ class TestCmdStartSelector(unittest.TestCase): self._bottle_picker_mock = self._bottle_picker_patch.start() self._bottle_picker_mock.return_value = ["claude"] # default: one bottle selected + self._image_policy_patch = patch( + "bot_bottle.cli.start._select_image_policy", + return_value="fresh", + ) + self._image_policy_patch.start() + self._env_patch = patch.dict(os.environ, {}, clear=False) self._env_patch.start() os.environ.pop("BOT_BOTTLE_BACKEND", None) @@ -66,6 +72,7 @@ class TestCmdStartSelector(unittest.TestCase): self._launch_patch.stop() self._agent_picker_patch.stop() self._bottle_picker_patch.stop() + self._image_policy_patch.stop() self._env_patch.stop() # ------------------------------------------------------------------ @@ -124,6 +131,19 @@ class TestCmdStartSelector(unittest.TestCase): spec = self._launch_mock.call_args[0][0] self.assertEqual(("claude", "dev"), spec.bottle_names) + def test_image_policy_forwarded_to_spec(self): + with patch("bot_bottle.cli.start._select_image_policy", return_value="cached"): + start_mod.cmd_start(["researcher"]) + self._launch_mock.assert_called_once() + spec = self._launch_mock.call_args[0][0] + self.assertEqual("cached", spec.image_policy) + + def test_image_policy_cancel_returns_0(self): + with patch("bot_bottle.cli.start._select_image_policy", return_value=None): + rc = start_mod.cmd_start(["researcher"]) + self.assertEqual(0, rc) + self._launch_mock.assert_not_called() + def test_empty_bottle_selection_forwarded(self): self._bottle_picker_mock.return_value = [] start_mod.cmd_start(["researcher"]) @@ -215,6 +235,7 @@ class TestCmdStartLabelCollision(unittest.TestCase): ).start() # Stub the bottle picker to always return a selection. patch.object(tui_mod, "filter_multiselect", return_value=["claude"]).start() + patch("bot_bottle.cli.start._select_image_policy", return_value="fresh").start() self.addCleanup(patch.stopall) def test_no_collision_proceeds_without_reprompt(self): diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index fa83cb3..17cbe1a 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -118,6 +118,7 @@ def _plan( with_egress: bool = False, supervise: bool = False, canary: bool = False, + image_policy: str = "fresh", ) -> DockerBottlePlan: """Build a fully-resolved DockerBottlePlan. Toggles cover the matrix the renderer's conditional-service logic branches on.""" @@ -148,6 +149,7 @@ def _plan( agent_name="demo", copy_cwd=False, user_cwd="/tmp/x", + image_policy=image_policy, ) return DockerBottlePlan( spec=spec, @@ -300,6 +302,11 @@ class TestSidecarBundleShape(unittest.TestCase): self.assertEqual("bot-bottle-sidecars:latest", sc["image"]) self.assertEqual("Dockerfile.sidecars", sc["build"]["dockerfile"]) + def test_cached_policy_omits_bundle_build(self): + sc = self._render(image_policy="cached")["services"]["sidecars"] + self.assertEqual("bot-bottle-sidecars:latest", sc["image"]) + self.assertNotIn("build", sc) + def test_bundle_container_name_uses_sidecars_prefix(self): sc = self._render()["services"]["sidecars"] self.assertEqual(f"bot-bottle-sidecars-{SLUG}", sc["container_name"]) diff --git a/tests/unit/test_config_store.py b/tests/unit/test_config_store.py new file mode 100644 index 0000000..19676c8 --- /dev/null +++ b/tests/unit/test_config_store.py @@ -0,0 +1,44 @@ +"""Unit tests for the host-side configuration store.""" + +from __future__ import annotations + +import tempfile +import unittest +from pathlib import Path + +from bot_bottle.config_store import ( + CACHED_IMAGE_STALE_WARNING_DAYS, + DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, + ConfigStore, +) +from bot_bottle.store_manager import StoreManager + + +class TestConfigStore(unittest.TestCase): + def test_cached_image_warning_days_defaults_to_one(self) -> None: + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + store = ConfigStore(Path(tmp) / "bot-bottle.db") + store.migrate() + self.assertEqual( + DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, + store.cached_image_stale_warning_days(), + ) + + def test_cached_image_warning_days_reads_value(self) -> None: + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + store = ConfigStore(Path(tmp) / "bot-bottle.db") + store.migrate() + store.set(CACHED_IMAGE_STALE_WARNING_DAYS, "7") + self.assertEqual(7, store.cached_image_stale_warning_days()) + + def test_store_manager_includes_config_store(self) -> None: + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + db = Path(tmp) / "bot-bottle.db" + manager = StoreManager(db) + self.assertFalse(manager.is_migrated()) + manager.migrate() + self.assertTrue(manager.is_migrated()) + + +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 index 1152e63..92fb3ee 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -3,6 +3,7 @@ from __future__ import annotations import contextlib +import dataclasses import io import tempfile import unittest @@ -16,6 +17,7 @@ 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.log import Die from bot_bottle.manifest import ManifestIndex @@ -103,6 +105,10 @@ class TestLaunchCommittedImage(unittest.TestCase): 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.docker_mod, "image_created_at", + ), mock.patch.object( + launch_mod, "warn_if_stale", ), mock.patch.object( launch_mod, "egress_tls_init", return_value=(Path("/egress_ca"), Path("/egress_cert")), @@ -180,6 +186,24 @@ class TestLaunchCommittedImage(unittest.TestCase): built = self._run_launch(plan, committed_tag=None) self.assertEqual([_DEFAULT_IMAGE], built) + def test_cached_images_skip_build_when_present(self) -> None: + base = _plan(self._tmp) + plan = dataclasses.replace( + base, + spec=dataclasses.replace(base.spec, image_policy="cached"), + ) + built = self._run_launch(plan, committed_tag=None, image_present=True) + self.assertEqual([], built) + + def test_cached_images_die_when_agent_missing(self) -> None: + base = _plan(self._tmp) + plan = dataclasses.replace( + base, + spec=dataclasses.replace(base.spec, image_policy="cached"), + ) + with self.assertRaises(Die): + self._run_launch(plan, committed_tag=None, image_present=False) + def test_falls_back_to_build_when_committed_image_missing_from_daemon(self) -> None: plan = _plan(self._tmp) built = self._run_launch( diff --git a/tests/unit/test_docker_util_image.py b/tests/unit/test_docker_util_image.py index 06acfde..35b1499 100644 --- a/tests/unit/test_docker_util_image.py +++ b/tests/unit/test_docker_util_image.py @@ -9,6 +9,7 @@ from __future__ import annotations import subprocess import unittest +from datetime import timezone from unittest.mock import patch from bot_bottle.backend.docker import util as docker_mod @@ -54,6 +55,22 @@ class TestImageId(unittest.TestCase): self.assertIn("missing:tag", die.call_args.args[0]) +class TestImageCreatedAt(unittest.TestCase): + def test_parses_docker_timestamp_with_nanoseconds(self): + with patch.object( + docker_mod.subprocess, "run", + return_value=_ok(stdout="2026-07-06T15:33:47.123456789Z\n"), + ) as run: + created = docker_mod.image_created_at("bot-bottle-claude:latest") + self.assertEqual(2026, created.year) + self.assertEqual(123456, created.microsecond) + self.assertEqual(timezone.utc, created.tzinfo) + self.assertEqual( + ["docker", "image", "inspect", "--format", "{{.Created}}", "bot-bottle-claude:latest"], + run.call_args.args[0], + ) + + class TestSave(unittest.TestCase): def test_save_runs_docker_save(self): with patch.object( diff --git a/tests/unit/test_smolmachines_launch_image.py b/tests/unit/test_smolmachines_launch_image.py index 0f79dc5..a200eb7 100644 --- a/tests/unit/test_smolmachines_launch_image.py +++ b/tests/unit/test_smolmachines_launch_image.py @@ -187,6 +187,56 @@ class TestAgentFromPath(unittest.TestCase): dockerfile="/repo/Dockerfile", ) + def test_cached_policy_uses_existing_artifact_without_build(self): + with tempfile.TemporaryDirectory(prefix="cached-smolmachine.") as tmp: + digest = "abcdef0123456789" + artifact = Path(tmp) / f"{digest}.smolmachine.smolmachine" + artifact.write_text("") + plan = SimpleNamespace( + slug="dev-abc12", + agent_image="bot-bottle-claude:latest", + spec=SimpleNamespace(image_policy="cached"), + ) + with patch.object( + _launch_mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp), + ), patch.object( + _launch_mod, "read_committed_image", return_value="", + ), patch.object( + _launch_mod.docker_mod, "image_exists", return_value=True, + ), patch.object( + _launch_mod.docker_mod, "image_id", + return_value=f"sha256:{digest}fffffffffffffffff", + ), patch.object( + _launch_mod, "warn_if_stale_path", + ), patch.object( + _launch_mod, "_ensure_smolmachine", + ) as ensure: + result = _launch_mod._agent_from_path(cast(Any, plan)) + + self.assertEqual(artifact, result) + ensure.assert_not_called() + + def test_cached_policy_dies_when_artifact_missing(self): + with tempfile.TemporaryDirectory(prefix="cached-smolmachine.") as tmp: + plan = SimpleNamespace( + slug="dev-abc12", + agent_image="bot-bottle-claude:latest", + spec=SimpleNamespace(image_policy="cached"), + ) + with patch.object( + _launch_mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp), + ), patch.object( + _launch_mod, "read_committed_image", return_value="", + ), patch.object( + _launch_mod.docker_mod, "image_exists", return_value=True, + ), patch.object( + _launch_mod.docker_mod, "image_id", + return_value="sha256:abcdef0123456789fffffffffffffffff", + ): + from bot_bottle.log import Die + with self.assertRaises(Die): + _launch_mod._agent_from_path(cast(Any, plan)) + if __name__ == "__main__": unittest.main() -- 2.52.0 From 114126e162fdfd07276ff35ca0c00a2cfce0fa24 Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 9 Jul 2026 17:36:00 +0000 Subject: [PATCH 2/9] fix: make config store schema explicit --- bot_bottle/config_store.py | 54 ++++++++++++++------------------- tests/unit/test_config_store.py | 19 ++++++++++-- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/bot_bottle/config_store.py b/bot_bottle/config_store.py index 93fda93..400f1da 100644 --- a/bot_bottle/config_store.py +++ b/bot_bottle/config_store.py @@ -14,66 +14,58 @@ except ImportError: from supervise_types import host_db_path # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module -CACHED_IMAGE_STALE_WARNING_DAYS = "cached_image_stale_warning_days" DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS = 1 class ConfigStore(DbStore): - """SQLite key/value configuration for host-side bot-bottle settings.""" + """SQLite configuration for host-side bot-bottle settings.""" def __init__(self, db_path: Path | None = None) -> None: migrations = TableMigrations("config_store", [ - # v1 — generic string key/value settings + # v1 — host-side bot-bottle settings """ CREATE TABLE IF NOT EXISTS bot_bottle_config ( - key TEXT PRIMARY KEY, - value TEXT NOT NULL + id INTEGER PRIMARY KEY CHECK (id = 1), + cached_image_stale_warning_days INTEGER NOT NULL DEFAULT 1 ) """, ]) super().__init__(db_path or host_db_path(), migrations) - def get(self, key: str) -> str | None: + def cached_image_stale_warning_days(self) -> int: if not self.db_path.is_file(): - return None + return DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS with self._connect() as conn: row = conn.execute( - "SELECT value FROM bot_bottle_config WHERE key = ?", - (key,), + """ + SELECT cached_image_stale_warning_days + FROM bot_bottle_config + WHERE id = 1 + """, ).fetchone() - return str(row["value"]) if row is not None else None + if row is None: + return DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS + try: + return int(row["cached_image_stale_warning_days"]) + except (TypeError, ValueError): + return DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS - def set(self, key: str, value: str) -> Path: + def set_cached_image_stale_warning_days(self, days: int) -> Path: with self._connect() as conn: conn.execute( """ - INSERT INTO bot_bottle_config (key, value) - VALUES (?, ?) - ON CONFLICT(key) DO UPDATE SET value = excluded.value + INSERT INTO bot_bottle_config (id, cached_image_stale_warning_days) + VALUES (1, ?) + ON CONFLICT(id) DO UPDATE SET + cached_image_stale_warning_days = excluded.cached_image_stale_warning_days """, - (key, value), + (days,), ) self._chmod() return self.db_path - def get_int(self, key: str, default: int) -> int: - raw = self.get(key) - if raw is None: - return default - try: - return int(raw) - except ValueError: - return default - - def cached_image_stale_warning_days(self) -> int: - return self.get_int( - CACHED_IMAGE_STALE_WARNING_DAYS, - DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, - ) - __all__ = [ - "CACHED_IMAGE_STALE_WARNING_DAYS", "DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS", "ConfigStore", ] diff --git a/tests/unit/test_config_store.py b/tests/unit/test_config_store.py index 19676c8..df8b717 100644 --- a/tests/unit/test_config_store.py +++ b/tests/unit/test_config_store.py @@ -2,12 +2,12 @@ from __future__ import annotations +import sqlite3 import tempfile import unittest from pathlib import Path from bot_bottle.config_store import ( - CACHED_IMAGE_STALE_WARNING_DAYS, DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, ConfigStore, ) @@ -28,9 +28,24 @@ class TestConfigStore(unittest.TestCase): with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: store = ConfigStore(Path(tmp) / "bot-bottle.db") store.migrate() - store.set(CACHED_IMAGE_STALE_WARNING_DAYS, "7") + store.set_cached_image_stale_warning_days(7) self.assertEqual(7, store.cached_image_stale_warning_days()) + def test_config_schema_uses_explicit_settings_columns(self) -> None: + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + store = ConfigStore(Path(tmp) / "bot-bottle.db") + store.migrate() + with sqlite3.connect(store.db_path) as conn: + conn.row_factory = sqlite3.Row + columns = [ + row["name"] + for row in conn.execute("PRAGMA table_info(bot_bottle_config)") + ] + self.assertEqual([ + "id", + "cached_image_stale_warning_days", + ], columns) + def test_store_manager_includes_config_store(self) -> None: with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: db = Path(tmp) / "bot-bottle.db" -- 2.52.0 From 82a0b73d48f6ab2a239848456fc9a9e58e9a40bd Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 9 Jul 2026 18:00:10 +0000 Subject: [PATCH 3/9] =?UTF-8?q?fix:=20pyright=20error=20in=20test=5Fcompos?= =?UTF-8?q?e=20=E2=80=94=20annotate=20kwargs=20as=20dict[str,=20Any]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/test_compose.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index 17cbe1a..59df6f4 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -267,7 +267,8 @@ class TestAgentAlwaysPresent(unittest.TestCase): def test_agent_depends_only_on_sidecars(self): # Bundle shape: the init supervisor owns intra-bundle daemon # ordering, so the agent waits on the bundle container alone. - for kwargs in [{}, {"with_git": True, "with_egress": True, "supervise": True}]: + cases: list[dict[str, Any]] = [{}, {"with_git": True, "with_egress": True, "supervise": True}] + for kwargs in cases: with self.subTest(**kwargs): s = bottle_plan_to_compose(_plan(**kwargs))["services"]["agent"] self.assertEqual(["sidecars"], s["depends_on"]) -- 2.52.0 From 2362973a0c7c7409ff164644e1d94683783daf04 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 9 Jul 2026 18:39:44 +0000 Subject: [PATCH 4/9] refactor(image-cache): replace warn_if_stale with StaleImageError; add launch template - image_cache: StaleImageError exception + check_stale/check_stale_path (raise instead of warn) - BottleBackend.launch: template method (skip_stale flag) that calls _image_stale_checks then _launch_impl - Each backend: _image_stale_checks delegates to a stale_checks() function in its launch module; _launch_impl replaces launch override - macos_container: adds image_created_at to util, cached-image support in _build_images, stale_checks - cli/start.py: catches StaleImageError, prompts interactively, retries with skip_stale=True; headless mode dies on it --- bot_bottle/backend/__init__.py | 23 +++++++- bot_bottle/backend/docker/backend.py | 5 +- bot_bottle/backend/docker/launch.py | 33 ++++++----- bot_bottle/backend/macos_container/backend.py | 5 +- bot_bottle/backend/macos_container/launch.py | 53 +++++++++++++----- bot_bottle/backend/macos_container/util.py | 34 ++++++++++++ bot_bottle/cli/start.py | 55 +++++++++++-------- bot_bottle/image_cache.py | 26 +++++---- .../test_docker_launch_committed_image.py | 4 -- tests/unit/test_macos_container_launch.py | 4 +- tests/unit/test_smolmachines_launch_image.py | 50 ----------------- 11 files changed, 169 insertions(+), 123 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index e13fd4b..dbfd08d 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -36,10 +36,10 @@ import os import shlex import sys from abc import ABC, abstractmethod -from contextlib import AbstractContextManager +from contextlib import AbstractContextManager, contextmanager from dataclasses import dataclass from pathlib import Path -from typing import Any, Generic, Sequence, TypeVar +from typing import Any, Generator, Generic, Sequence, TypeVar from ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provision_plan from ..egress import EgressPlan @@ -435,8 +435,25 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): prompt file, Dockerfile path, and guest home all live on `agent_provision_plan` — the source of truth.""" + @contextmanager + def launch( + self, plan: PlanT, *, skip_stale: bool = False + ) -> Generator[Bottle, None, None]: + """Template: optionally check for stale cached images, then delegate + to `_launch_impl`. Pass `skip_stale=True` to bypass the stale check + (used by the interactive CLI after the operator confirms).""" + if not skip_stale: + self._image_stale_checks(plan) + with self._launch_impl(plan) as bottle: + yield bottle + + def _image_stale_checks(self, plan: PlanT) -> None: + """Raise StaleImageError if any cached image used by this plan is stale. + No-op default; backends override to call the shared `check_stale*` + helpers on their image/artifact timestamps.""" + @abstractmethod - def launch(self, plan: PlanT) -> AbstractContextManager[Bottle]: + def _launch_impl(self, plan: PlanT) -> AbstractContextManager[Bottle]: """Build/run the bottle and yield a handle; tear down on exit.""" def provision(self, plan: PlanT, bottle: "Bottle") -> str | None: diff --git a/bot_bottle/backend/docker/backend.py b/bot_bottle/backend/docker/backend.py index b3b9e3f..3034bfb 100644 --- a/bot_bottle/backend/docker/backend.py +++ b/bot_bottle/backend/docker/backend.py @@ -85,8 +85,11 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup stage_dir=stage_dir, ) + def _image_stale_checks(self, plan: DockerBottlePlan) -> None: + _launch.stale_checks(plan) + @contextmanager - def launch(self, plan: DockerBottlePlan) -> Generator[DockerBottle, None, None]: + def _launch_impl(self, plan: DockerBottlePlan) -> Generator[DockerBottle, None, None]: with _launch.launch(plan, provision=self.provision) as bottle: yield bottle diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 7a03f47..731874f 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -41,7 +41,7 @@ from ...git_gate import ( provision_git_gate_dynamic_keys, revoke_git_gate_provisioned_keys, ) -from ...image_cache import warn_if_stale +from ...image_cache import check_stale from ...log import die, info, warn from . import network as network_mod from . import util as docker_mod @@ -105,11 +105,6 @@ def launch( cached_policy = plan.spec.image_policy == "cached" if committed and docker_mod.image_exists(committed): info(f"using committed image {committed!r}") - if cached_policy: - warn_if_stale( - f"agent image {committed!r}", - docker_mod.image_created_at(committed), - ) plan = dataclasses.replace( plan, agent_provision=dataclasses.replace(plan.agent_provision, image=committed), @@ -127,14 +122,6 @@ def launch( ) info(f"using cached agent image {plan.image!r}") info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") - warn_if_stale( - f"agent image {plan.image!r}", - docker_mod.image_created_at(plan.image), - ) - warn_if_stale( - f"sidecar image {SIDECAR_BUNDLE_IMAGE!r}", - docker_mod.image_created_at(SIDECAR_BUNDLE_IMAGE), - ) else: docker_mod.build_image( plan.image, _REPO_DIR, @@ -236,3 +223,21 @@ def launch( yield bottle finally: teardown() + + +def stale_checks(plan: DockerBottlePlan) -> None: + """Raise StaleImageError if a cached image is older than the configured + threshold. Only runs when image_policy is 'cached'. Called by the backend + class's _image_stale_checks before _launch_impl starts any resources.""" + if plan.spec.image_policy != "cached": + return + committed = read_committed_image(plan.slug) + if committed and docker_mod.image_exists(committed): + check_stale(f"agent image {committed!r}", docker_mod.image_created_at(committed)) + return + for label, ref in [ + ("agent image", plan.image), + ("sidecar image", SIDECAR_BUNDLE_IMAGE), + ]: + if docker_mod.image_exists(ref): + check_stale(f"{label} {ref!r}", docker_mod.image_created_at(ref)) diff --git a/bot_bottle/backend/macos_container/backend.py b/bot_bottle/backend/macos_container/backend.py index bd43ed8..4626233 100644 --- a/bot_bottle/backend/macos_container/backend.py +++ b/bot_bottle/backend/macos_container/backend.py @@ -67,8 +67,11 @@ class MacosContainerBottleBackend( stage_dir=stage_dir, ) + def _image_stale_checks(self, plan: MacosContainerBottlePlan) -> None: + _launch.stale_checks(plan) + @contextmanager - def launch( + def _launch_impl( self, plan: MacosContainerBottlePlan ) -> Generator[MacosContainerBottle, None, None]: with _launch.launch(plan, provision=self.provision) as bottle: diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 6e9034f..83b56ec 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -32,6 +32,7 @@ from ...git_gate import ( provision_git_gate_dynamic_keys, revoke_git_gate_provisioned_keys, ) +from ...image_cache import check_stale from ...log import die, info, warn from ...supervise import DB_PATH_IN_CONTAINER, SUPERVISE_PORT from ...util import expand_tilde @@ -149,29 +150,53 @@ def _mint_certs(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: def _build_images(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: - container_mod.build_image( - SIDECAR_BUNDLE_IMAGE, - _REPO_DIR, - dockerfile=SIDECAR_BUNDLE_DOCKERFILE, - ) + cached = plan.spec.image_policy == "cached" committed = read_committed_image(plan.slug) if committed and container_mod.image_exists(committed): info(f"using committed image {committed!r}") + if not cached: + container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) return dataclasses.replace( plan, - agent_provision=dataclasses.replace( - plan.agent_provision, - image=committed, - ), + agent_provision=dataclasses.replace(plan.agent_provision, image=committed), ) - container_mod.build_image( - plan.image, - _REPO_DIR, - dockerfile=plan.dockerfile_path, - ) + if cached: + if not container_mod.image_exists(plan.image): + die( + f"cached agent image {plan.image!r} not found; " + "run without --cached-images to build it" + ) + if not container_mod.image_exists(SIDECAR_BUNDLE_IMAGE): + die( + f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " + "run without --cached-images to build it" + ) + info(f"using cached agent image {plan.image!r}") + info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") + return plan + container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) + container_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) return plan +def stale_checks(plan: MacosContainerBottlePlan) -> None: + """Raise StaleImageError if a cached image is older than the configured + threshold. Only runs when image_policy is 'cached'. Called by the backend + class's _image_stale_checks before _launch_impl starts any resources.""" + if plan.spec.image_policy != "cached": + return + committed = read_committed_image(plan.slug) + if committed and container_mod.image_exists(committed): + check_stale(f"agent image {committed!r}", container_mod.image_created_at(committed)) + return + for label, ref in [ + ("agent image", plan.image), + ("sidecar image", SIDECAR_BUNDLE_IMAGE), + ]: + if container_mod.image_exists(ref): + check_stale(f"{label} {ref!r}", container_mod.image_created_at(ref)) + + def _create_networks( internal_network: str, egress_network: str, diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index ccb8f06..aacf59c 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -10,6 +10,7 @@ import shutil import subprocess import tempfile import time +from datetime import datetime, timezone from typing import Iterable from ...log import die, info @@ -458,6 +459,39 @@ def image_id(ref: str) -> str: raise AssertionError("unreachable") +def image_created_at(ref: str) -> datetime: + """Return the image creation timestamp as an aware UTC datetime. + + Parses the `created` field from `container image inspect` JSON output.""" + result = subprocess.run( + [_CONTAINER, "image", "inspect", ref], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + die( + f"container image inspect for {ref!r} failed: " + f"{(result.stderr or '').strip() or ''}" + ) + try: + data = json.loads(result.stdout or "{}") + except json.JSONDecodeError as exc: + die(f"container image inspect for {ref!r} returned malformed JSON: {exc}") + if isinstance(data, list) and data: + data = data[0] + if isinstance(data, dict): + value = data.get("created") or data.get("Created") + if isinstance(value, str) and value: + try: + ts = value.rstrip("Z") + return datetime.fromisoformat(ts).replace(tzinfo=timezone.utc) + except ValueError: + pass + die(f"container image inspect for {ref!r} did not include a creation timestamp") + raise AssertionError("unreachable") + + def save(ref: str, output: str) -> None: subprocess.run([_CONTAINER, "image", "save", ref, "-o", output], check=True) diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index bb2a7a6..a494486 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -36,6 +36,7 @@ from ..bottle_state import ( is_preserved, mark_preserved, ) +from ..image_cache import StaleImageError from ..log import info, die from ..manifest import Manifest, ManifestIndex from ._common import PROG, USER_CWD, read_tty_line @@ -555,30 +556,38 @@ def _launch_bottle( return 0 backend = get_bottle_backend(backend_name) - with backend.launch(plan) as bottle: - agent_provider_template = getattr(plan, "agent_provider_template", "claude") - extra_args: tuple[str, ...] = () - if headless_prompt_text: - extra_args = tuple( - get_provider(agent_provider_template).headless_prompt( - headless_prompt_text + skip_stale = False + while True: + try: + with backend.launch(plan, skip_stale=skip_stale) as bottle: + agent_provider_template = getattr(plan, "agent_provider_template", "claude") + extra_args: tuple[str, ...] = () + if headless_prompt_text: + extra_args = tuple( + get_provider(agent_provider_template).headless_prompt( + headless_prompt_text + ) + ) + exit_code = attach_agent( + bottle, + agent_provider_template=agent_provider_template, + startup_args=plan.agent_provision.startup_args + extra_args, ) - ) - exit_code = attach_agent( - bottle, - agent_provider_template=agent_provider_template, - startup_args=plan.agent_provision.startup_args + extra_args, - ) - info( - f"session ended (exit {exit_code}); " - f"container {bottle.name} will be removed" - ) - # While the container is still alive: always snapshot the - # transcript and — if the agent exited non-zero — mark - # the state for preservation. This picks up crashes / - # Ctrl-Cs / OOM kills before cleanup removes the state dir. - if agent_provider_template == "claude": - capture_claude_session_state(identity, exit_code) + info( + f"session ended (exit {exit_code}); " + f"container {bottle.name} will be removed" + ) + if agent_provider_template == "claude": + capture_claude_session_state(identity, exit_code) + break + except StaleImageError as exc: + if assume_yes: + die(str(exc)) + sys.stderr.write(f"bot-bottle: {exc}\nLaunch anyway? [y/N] ") + sys.stderr.flush() + if read_tty_line() not in ("y", "Y", "yes", "YES"): + break + skip_stale = True return 0 finally: # PRD 0018 chunk 2: prepare now writes the bottle's bind-mount diff --git a/bot_bottle/image_cache.py b/bot_bottle/image_cache.py index 85e2363..b2f802a 100644 --- a/bot_bottle/image_cache.py +++ b/bot_bottle/image_cache.py @@ -1,4 +1,4 @@ -"""Shared helpers for cached-image quickstart warnings.""" +"""Shared helpers for cached-image quickstart stale checks.""" from __future__ import annotations @@ -7,13 +7,19 @@ from pathlib import Path try: from .config_store import ConfigStore - from .log import warn except ImportError: from config_store import ConfigStore # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module - from log import warn # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module -def warn_if_stale(label: str, created_at: datetime) -> None: +class StaleImageError(Exception): + """Raised when a cached image or artifact exceeds the configured staleness + threshold. Callers can catch this to prompt interactively; headless paths + let it propagate as a fatal error.""" + + +def check_stale(label: str, created_at: datetime) -> None: + """Raise StaleImageError if `created_at` is older than the configured + stale-warning threshold. Negative threshold disables the check.""" threshold_days = ConfigStore().cached_image_stale_warning_days() if threshold_days < 0: return @@ -22,17 +28,15 @@ def warn_if_stale(label: str, created_at: datetime) -> None: age = now - created if age.total_seconds() <= threshold_days * 86400: return - warn( + raise StaleImageError( f"cached {label} is {age.days} day(s) old; " "quickstart does not verify it matches the current Dockerfile/context" ) -def warn_if_stale_path(label: str, path: Path) -> None: - warn_if_stale( - label, - datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc), - ) +def check_stale_path(label: str, path: Path) -> None: + """Raise StaleImageError if `path`'s mtime exceeds the staleness threshold.""" + check_stale(label, datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc)) -__all__ = ["warn_if_stale", "warn_if_stale_path"] +__all__ = ["StaleImageError", "check_stale", "check_stale_path"] diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py index 92fb3ee..8fd8af9 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -105,10 +105,6 @@ class TestLaunchCommittedImage(unittest.TestCase): 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.docker_mod, "image_created_at", - ), mock.patch.object( - launch_mod, "warn_if_stale", ), mock.patch.object( launch_mod, "egress_tls_init", return_value=(Path("/egress_ca"), Path("/egress_cert")), diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 7dd9de7..d6ee03d 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -81,7 +81,7 @@ def _plan( provisioned_env={"CODEX_HOME": "/run/codex-home"}, ) return cast(MacosContainerBottlePlan, SimpleNamespace( - spec=SimpleNamespace(), + spec=SimpleNamespace(image_policy="fresh"), manifest=_MANIFEST, stage_dir=stage_dir, slug="dev-abc", @@ -292,7 +292,7 @@ class TestMacosContainerLaunchArgv(unittest.TestCase): def _build_plan(stage_dir: Path) -> MacosContainerBottlePlan: return MacosContainerBottlePlan( - spec=cast(BottleSpec, SimpleNamespace()), + spec=cast(BottleSpec, SimpleNamespace(image_policy="fresh")), manifest=_MANIFEST, stage_dir=stage_dir, git_gate_plan=cast(GitGatePlan, SimpleNamespace(upstreams=())), diff --git a/tests/unit/test_smolmachines_launch_image.py b/tests/unit/test_smolmachines_launch_image.py index a200eb7..0f79dc5 100644 --- a/tests/unit/test_smolmachines_launch_image.py +++ b/tests/unit/test_smolmachines_launch_image.py @@ -187,56 +187,6 @@ class TestAgentFromPath(unittest.TestCase): dockerfile="/repo/Dockerfile", ) - def test_cached_policy_uses_existing_artifact_without_build(self): - with tempfile.TemporaryDirectory(prefix="cached-smolmachine.") as tmp: - digest = "abcdef0123456789" - artifact = Path(tmp) / f"{digest}.smolmachine.smolmachine" - artifact.write_text("") - plan = SimpleNamespace( - slug="dev-abc12", - agent_image="bot-bottle-claude:latest", - spec=SimpleNamespace(image_policy="cached"), - ) - with patch.object( - _launch_mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp), - ), patch.object( - _launch_mod, "read_committed_image", return_value="", - ), patch.object( - _launch_mod.docker_mod, "image_exists", return_value=True, - ), patch.object( - _launch_mod.docker_mod, "image_id", - return_value=f"sha256:{digest}fffffffffffffffff", - ), patch.object( - _launch_mod, "warn_if_stale_path", - ), patch.object( - _launch_mod, "_ensure_smolmachine", - ) as ensure: - result = _launch_mod._agent_from_path(cast(Any, plan)) - - self.assertEqual(artifact, result) - ensure.assert_not_called() - - def test_cached_policy_dies_when_artifact_missing(self): - with tempfile.TemporaryDirectory(prefix="cached-smolmachine.") as tmp: - plan = SimpleNamespace( - slug="dev-abc12", - agent_image="bot-bottle-claude:latest", - spec=SimpleNamespace(image_policy="cached"), - ) - with patch.object( - _launch_mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp), - ), patch.object( - _launch_mod, "read_committed_image", return_value="", - ), patch.object( - _launch_mod.docker_mod, "image_exists", return_value=True, - ), patch.object( - _launch_mod.docker_mod, "image_id", - return_value="sha256:abcdef0123456789fffffffffffffffff", - ): - from bot_bottle.log import Die - with self.assertRaises(Die): - _launch_mod._agent_from_path(cast(Any, plan)) - if __name__ == "__main__": unittest.main() -- 2.52.0 From 7ea7f3926cf1b8729f36722277f8a3173c32953d Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 9 Jul 2026 19:06:38 +0000 Subject: [PATCH 5/9] =?UTF-8?q?test:=20add=20unit=20tests=20for=20stale-im?= =?UTF-8?q?age=20checks=20to=20reach=20=E2=89=A590%=20diff-coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers StaleImageError / check_stale / check_stale_path, the BottleBackend.launch template method (skip_stale flag), stale_checks functions across docker / smolmachines / macos-container backends, _build_images cached paths in the macOS backend, image_created_at edge cases in both docker and container util modules, the CLI stale loop (headless die, interactive decline, interactive confirm + retry), and ConfigStore.cached_image_stale_warning_days fallback paths. Diff-coverage: 488/507 changed lines covered (96.3%). --- tests/unit/test_cli_start_stale.py | 168 +++++++++++ tests/unit/test_config_store.py | 27 ++ tests/unit/test_docker_util_image.py | 29 ++ tests/unit/test_image_cache.py | 85 ++++++ tests/unit/test_macos_container_launch.py | 98 +++++++ tests/unit/test_macos_container_util.py | 66 +++++ tests/unit/test_stale_checks.py | 334 ++++++++++++++++++++++ 7 files changed, 807 insertions(+) create mode 100644 tests/unit/test_cli_start_stale.py create mode 100644 tests/unit/test_image_cache.py create mode 100644 tests/unit/test_stale_checks.py diff --git a/tests/unit/test_cli_start_stale.py b/tests/unit/test_cli_start_stale.py new file mode 100644 index 0000000..0e0d7dc --- /dev/null +++ b/tests/unit/test_cli_start_stale.py @@ -0,0 +1,168 @@ +"""Unit: _launch_bottle StaleImageError handling. + +Exercises the while-True / try-except loop around backend.launch: + - headless mode → die on stale + - interactive mode, user declines → stop without launching + - interactive mode, user confirms → retry with skip_stale=True +""" + +from __future__ import annotations + +import contextlib +import io +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from typing import Any, cast +from unittest.mock import MagicMock, call, patch + +from bot_bottle.image_cache import StaleImageError +from bot_bottle.log import Die + + +def _fake_plan() -> Any: + provision = SimpleNamespace(startup_args=()) + return cast(Any, SimpleNamespace( + agent_provision=provision, + agent_provider_template="claude", + slug="dev-abc", + )) + + +def _stale_cm() -> MagicMock: + """Return a context-manager mock whose __enter__ raises StaleImageError.""" + cm = MagicMock() + cm.__enter__ = MagicMock(side_effect=StaleImageError("image is 5 day(s) old")) + cm.__exit__ = MagicMock(return_value=False) + return cm + + +def _ok_cm(bottle: Any) -> MagicMock: + """Return a context-manager mock that yields `bottle`.""" + cm = MagicMock() + cm.__enter__ = MagicMock(return_value=bottle) + cm.__exit__ = MagicMock(return_value=False) + return cm + + +class TestLaunchBottleStaleHandling(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.mkdtemp(prefix="cli-stale-test.") + + def _spec(self): + from bot_bottle.backend import BottleSpec + from bot_bottle.manifest import ManifestIndex + idx = ManifestIndex.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + return BottleSpec( + manifest=idx, + agent_name="demo", + copy_cwd=False, + user_cwd=self._tmp, + identity="dev-abc", + ) + + def _run_launch(self, **patch_kwargs) -> int: + import bot_bottle.cli.start as start_mod + spec = self._spec() + with patch.object(start_mod, "prepare_with_preflight", + return_value=(_fake_plan(), "dev-abc")), \ + patch.object(start_mod, "settle_state"), \ + patch.object(start_mod, "info"): + return start_mod._launch_bottle( + spec, + dry_run=False, + backend_name="docker", + **patch_kwargs, + ) + + def test_headless_stale_calls_die(self): + """In headless mode (assume_yes=True), a StaleImageError must call die().""" + import bot_bottle.cli.start as start_mod + + backend_mock = MagicMock() + backend_mock.launch.return_value = _stale_cm() + + with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ + patch.object(start_mod, "die", side_effect=Die("stale")): + with self.assertRaises(Die): + self._run_launch(assume_yes=True) + + def test_interactive_user_declines_stops_loop(self): + """Interactive user answering 'n' → loop exits without a second launch.""" + import bot_bottle.cli.start as start_mod + + backend_mock = MagicMock() + backend_mock.launch.return_value = _stale_cm() + + with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ + patch.object(start_mod, "read_tty_line", return_value="n"), \ + patch("sys.stderr", new_callable=io.StringIO): + rc = self._run_launch(assume_yes=False) + + self.assertEqual(0, rc) + # launch was called once (stale), then the user declined → no second call. + backend_mock.launch.assert_called_once() + + def test_interactive_user_confirms_retries_with_skip_stale(self): + """Interactive user answering 'y' → second launch with skip_stale=True.""" + import bot_bottle.cli.start as start_mod + + bottle_mock = MagicMock() + bottle_mock.name = "dev-abc" + + stale_calls = [0] + + def launch_side_effect(plan, *, skip_stale=False): + if not skip_stale: + return _stale_cm() + return _ok_cm(bottle_mock) + + backend_mock = MagicMock() + backend_mock.launch.side_effect = launch_side_effect + + with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ + patch.object(start_mod, "read_tty_line", return_value="y"), \ + patch.object(start_mod, "attach_agent", return_value=0), \ + patch.object(start_mod, "capture_claude_session_state"), \ + patch("sys.stderr", new_callable=io.StringIO): + rc = self._run_launch(assume_yes=False) + + self.assertEqual(0, rc) + # First call: skip_stale=False → stale. Second call: skip_stale=True → ok. + self.assertEqual(2, backend_mock.launch.call_count) + calls = backend_mock.launch.call_args_list + self.assertFalse(calls[0].kwargs.get("skip_stale", False)) + self.assertTrue(calls[1].kwargs.get("skip_stale", False)) + + def test_interactive_yes_uppercase_also_accepted(self): + """'Y' or 'YES' should also be accepted as confirmation.""" + import bot_bottle.cli.start as start_mod + + bottle_mock = MagicMock() + bottle_mock.name = "dev-abc" + + def launch_side_effect(plan, *, skip_stale=False): + if not skip_stale: + return _stale_cm() + return _ok_cm(bottle_mock) + + backend_mock = MagicMock() + backend_mock.launch.side_effect = launch_side_effect + + with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ + patch.object(start_mod, "read_tty_line", return_value="YES"), \ + patch.object(start_mod, "attach_agent", return_value=0), \ + patch.object(start_mod, "capture_claude_session_state"), \ + patch("sys.stderr", new_callable=io.StringIO): + rc = self._run_launch(assume_yes=False) + + self.assertEqual(0, rc) + self.assertEqual(2, backend_mock.launch.call_count) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_config_store.py b/tests/unit/test_config_store.py index df8b717..5987ae0 100644 --- a/tests/unit/test_config_store.py +++ b/tests/unit/test_config_store.py @@ -54,6 +54,33 @@ class TestConfigStore(unittest.TestCase): manager.migrate() self.assertTrue(manager.is_migrated()) + def test_cached_image_warning_days_returns_default_when_db_missing(self) -> None: + # When the db file doesn't exist yet (parent exists, file doesn't), + # the store returns the default without touching the file. + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + store = ConfigStore(Path(tmp) / "missing.db") + self.assertEqual( + DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, + store.cached_image_stale_warning_days(), + ) + + def test_cached_image_warning_days_returns_default_on_null_value(self) -> None: + # If the row exists but the value is NULL (or not castable to int), + # the store falls back to the default. + with tempfile.TemporaryDirectory(prefix="config-store.") as tmp: + db_path = Path(tmp) / "bot-bottle.db" + store = ConfigStore(db_path) + store.migrate() + # Write a NULL value directly. + with sqlite3.connect(db_path) as conn: + conn.execute( + "UPDATE bot_bottle_config SET cached_image_stale_warning_days = NULL WHERE id = 1" + ) + self.assertEqual( + DEFAULT_CACHED_IMAGE_STALE_WARNING_DAYS, + store.cached_image_stale_warning_days(), + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_docker_util_image.py b/tests/unit/test_docker_util_image.py index 35b1499..ca38bd2 100644 --- a/tests/unit/test_docker_util_image.py +++ b/tests/unit/test_docker_util_image.py @@ -70,6 +70,35 @@ class TestImageCreatedAt(unittest.TestCase): run.call_args.args[0], ) + def test_dies_on_inspect_failure(self): + with patch.object( + docker_mod.subprocess, "run", return_value=_fail("No such image"), + ), patch.object( + docker_mod, "die", side_effect=SystemExit("die"), + ) as die: + with self.assertRaises(SystemExit): + docker_mod.image_created_at("missing:tag") + die.assert_called_once() + self.assertIn("missing:tag", die.call_args.args[0]) + + def test_dies_on_invalid_timestamp(self): + with patch.object( + docker_mod.subprocess, "run", + return_value=_ok(stdout="not-a-timestamp\n"), + ), patch.object( + docker_mod, "die", side_effect=SystemExit("die"), + ) as die: + with self.assertRaises(SystemExit): + docker_mod.image_created_at("some:tag") + die.assert_called_once() + self.assertIn("some:tag", die.call_args.args[0]) + + def test_parse_docker_timestamp_no_tzinfo_defaults_to_utc(self): + # A bare datetime with no tz offset should be treated as UTC. + dt = docker_mod._parse_docker_timestamp("2024-05-01T10:00:00.000000") + self.assertIsNotNone(dt.tzinfo) + self.assertEqual(timezone.utc, dt.tzinfo) + class TestSave(unittest.TestCase): def test_save_runs_docker_save(self): diff --git a/tests/unit/test_image_cache.py b/tests/unit/test_image_cache.py new file mode 100644 index 0000000..904a02f --- /dev/null +++ b/tests/unit/test_image_cache.py @@ -0,0 +1,85 @@ +"""Unit: image_cache.py — check_stale / check_stale_path.""" + +from __future__ import annotations + +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path +from unittest.mock import patch + +from bot_bottle.image_cache import StaleImageError, check_stale, check_stale_path + + +def _utc(**kwargs) -> datetime: + return datetime.now(tz=timezone.utc) - timedelta(**kwargs) + + +class TestCheckStale(unittest.TestCase): + def _run(self, threshold: int, age_days: float) -> None: + created = datetime.now(tz=timezone.utc) - timedelta(days=age_days) + with patch("bot_bottle.image_cache.ConfigStore") as cs: + cs.return_value.cached_image_stale_warning_days.return_value = threshold + check_stale("test image", created) + + def test_negative_threshold_never_raises(self): + # Threshold < 0 means the check is disabled — always passes. + self._run(threshold=-1, age_days=9999) + + def test_zero_threshold_raises_immediately(self): + # threshold=0 means any image is stale the moment it exists. + with self.assertRaises(StaleImageError): + self._run(threshold=0, age_days=0.1) + + def test_within_threshold_does_not_raise(self): + # Age well under threshold — should pass silently. + self._run(threshold=7, age_days=2) + + def test_at_threshold_does_not_raise(self): + # Exactly at the boundary is fine (<=, not <). + self._run(threshold=1, age_days=0.9999) + + def test_exceeds_threshold_raises(self): + created = datetime.now(tz=timezone.utc) - timedelta(days=3) + with patch("bot_bottle.image_cache.ConfigStore") as cs: + cs.return_value.cached_image_stale_warning_days.return_value = 1 + with self.assertRaises(StaleImageError) as ctx: + check_stale("agent image 'bot-bottle:latest'", created) + self.assertIn("agent image", str(ctx.exception)) + self.assertIn("day(s) old", str(ctx.exception)) + + def test_naive_datetime_treated_as_utc(self): + # check_stale calls .astimezone(utc) on the input; naive datetimes + # that would be interpreted as local time should still work. + # We can't control the local tz in a unit test, so just ensure + # no exception is thrown for a very recent naive datetime. + naive_now = datetime.utcnow() + with patch("bot_bottle.image_cache.ConfigStore") as cs: + cs.return_value.cached_image_stale_warning_days.return_value = 1 + # Should not raise — the image is brand new. + check_stale("test image", naive_now) + + +class TestCheckStalePath(unittest.TestCase): + def test_delegates_to_check_stale_with_mtime(self): + with tempfile.NamedTemporaryFile() as f: + path = Path(f.name) + with patch("bot_bottle.image_cache.check_stale") as mock_check: + check_stale_path("some artifact", path) + mock_check.assert_called_once() + label, dt = mock_check.call_args.args + self.assertEqual("some artifact", label) + self.assertIsInstance(dt, datetime) + self.assertIsNotNone(dt.tzinfo) + + def test_raises_stale_for_old_file(self): + with tempfile.NamedTemporaryFile() as f: + path = Path(f.name) + with patch("bot_bottle.image_cache.ConfigStore") as cs: + cs.return_value.cached_image_stale_warning_days.return_value = 0 + with self.assertRaises(StaleImageError): + check_stale_path("cached artifact", path) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index d6ee03d..0803d21 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -365,5 +365,103 @@ class TestMacosContainerLaunchCommittedImage(unittest.TestCase): self.assertEqual("bot-bottle-agent:latest", calls[1][0]) +class TestMacosContainerLaunchCachedImages(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.stage_dir = Path(self._tmp.name) + + def tearDown(self): + self._tmp.cleanup() + + def test_cached_mode_dies_when_agent_image_missing(self): + plan = _build_plan(self.stage_dir) + from bot_bottle.backend import BottleSpec + import dataclasses + plan = dataclasses.replace( + plan, + spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), + ) + from bot_bottle.log import Die + + def image_exists(ref: str) -> bool: + return False # Nothing cached. + + with patch.object( + launch, "read_committed_image", return_value="", + ), patch.object( + launch.container_mod, "image_exists", side_effect=image_exists, + ), patch.object( + launch, "die", side_effect=Die("no agent image"), + ): + with self.assertRaises(Die): + launch._build_images(plan) + + def test_cached_mode_dies_when_sidecar_image_missing(self): + plan = _build_plan(self.stage_dir) + from bot_bottle.backend import BottleSpec + import dataclasses + plan = dataclasses.replace( + plan, + spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), + ) + from bot_bottle.log import Die + + def image_exists(ref: str) -> bool: + # Agent image is present; sidecar is missing. + return ref == plan.image + + with patch.object( + launch, "read_committed_image", return_value="", + ), patch.object( + launch.container_mod, "image_exists", side_effect=image_exists, + ), patch.object( + launch, "die", side_effect=Die("no sidecar image"), + ): + with self.assertRaises(Die): + launch._build_images(plan) + + def test_cached_mode_both_present_returns_unchanged_plan(self): + plan = _build_plan(self.stage_dir) + from bot_bottle.backend import BottleSpec + import dataclasses + plan = dataclasses.replace( + plan, + spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), + ) + with patch.object( + launch, "read_committed_image", return_value="", + ), patch.object( + launch.container_mod, "image_exists", return_value=True, + ), patch.object( + launch.container_mod, "build_image", + ) as build, patch.object(launch, "info"): + result = launch._build_images(plan) + + build.assert_not_called() + self.assertEqual(plan.image, result.image) + + def test_committed_image_plus_cached_skips_sidecar_build(self): + plan = _build_plan(self.stage_dir) + from bot_bottle.backend import BottleSpec + import dataclasses + plan = dataclasses.replace( + plan, + spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), + ) + 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", + ) as build, patch.object(launch, "info"): + result = launch._build_images(plan) + + # In cached mode with a committed image, no builds should fire. + build.assert_not_called() + self.assertEqual("bot-bottle-committed-dev-abc:latest", result.image) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_macos_container_util.py b/tests/unit/test_macos_container_util.py index 2e0f3bd..ab2f1fd 100644 --- a/tests/unit/test_macos_container_util.py +++ b/tests/unit/test_macos_container_util.py @@ -273,5 +273,71 @@ resolver #2 ) +class TestMacosContainerImageCreatedAt(unittest.TestCase): + def _ok(self, stdout: str) -> "util.subprocess.CompletedProcess": # type: ignore + return util.subprocess.CompletedProcess( + args=[], returncode=0, stdout=stdout, stderr="", + ) + + def _fail(self, stderr: str = "no such image") -> "util.subprocess.CompletedProcess": # type: ignore + return util.subprocess.CompletedProcess( + args=[], returncode=1, stdout="", stderr=stderr, + ) + + def test_parses_iso_timestamp_from_dict(self): + payload = '[{"created": "2025-06-01T12:00:00"}]' + with patch.object(util.subprocess, "run", return_value=self._ok(payload)): + dt = util.image_created_at("bot-bottle-agent:latest") + self.assertEqual(2025, dt.year) + self.assertEqual(6, dt.month) + self.assertEqual(1, dt.day) + + def test_accepts_list_or_dict_input(self): + # Container CLI may return a list; we take the first element. + payload = '[{"created": "2024-01-15T08:30:00"}]' + with patch.object(util.subprocess, "run", return_value=self._ok(payload)): + dt = util.image_created_at("some-image:latest") + self.assertEqual(2024, dt.year) + + def test_accepts_uppercase_Created_field(self): + payload = '[{"Created": "2024-03-20T10:00:00"}]' + with patch.object(util.subprocess, "run", return_value=self._ok(payload)): + dt = util.image_created_at("some-image:latest") + self.assertEqual(2024, dt.year) + self.assertEqual(3, dt.month) + + def test_dies_on_nonzero_returncode(self): + with patch.object(util.subprocess, "run", return_value=self._fail("not found")), \ + patch.object(util, "die", side_effect=SystemExit("die")) as die: + with self.assertRaises(SystemExit): + util.image_created_at("missing:tag") + die.assert_called_once() + self.assertIn("missing:tag", die.call_args.args[0]) + + def test_dies_on_malformed_json(self): + with patch.object(util.subprocess, "run", return_value=self._ok("not-json {")), \ + patch.object(util, "die", side_effect=SystemExit("die")) as die: + with self.assertRaises(SystemExit): + util.image_created_at("some:tag") + die.assert_called_once() + + def test_dies_when_no_created_field(self): + payload = '[{"id": "sha256:abc123"}]' + with patch.object(util.subprocess, "run", return_value=self._ok(payload)), \ + patch.object(util, "die", side_effect=SystemExit("die")) as die: + with self.assertRaises(SystemExit): + util.image_created_at("some:tag") + die.assert_called_once() + self.assertIn("creation timestamp", die.call_args.args[0]) + + def test_dies_on_invalid_timestamp_format(self): + payload = '[{"created": "not-a-date"}]' + with patch.object(util.subprocess, "run", return_value=self._ok(payload)), \ + patch.object(util, "die", side_effect=SystemExit("die")) as die: + with self.assertRaises(SystemExit): + util.image_created_at("some:tag") + die.assert_called_once() + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_stale_checks.py b/tests/unit/test_stale_checks.py new file mode 100644 index 0000000..21e5e59 --- /dev/null +++ b/tests/unit/test_stale_checks.py @@ -0,0 +1,334 @@ +"""Unit: stale-image check functions across backends, and the +BottleBackend.launch template method (skip_stale flag). + +No real images or containers are used — all Docker/container/smolmachine +calls are mocked at the module boundary.""" + +from __future__ import annotations + +import contextlib +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from typing import Any, cast +from unittest.mock import MagicMock, patch + +from bot_bottle.image_cache import StaleImageError + + +# --------------------------------------------------------------------------- +# BottleBackend.launch template — _image_stale_checks + skip_stale +# --------------------------------------------------------------------------- + +class TestBottleBackendLaunchTemplate(unittest.TestCase): + """Verify the concrete launch() method on BottleBackend calls + _image_stale_checks unless skip_stale=True, then delegates to _launch_impl.""" + + def _make_backend(self): + from bot_bottle.backend import BottleBackend + + class _FakeBottle: + name = "fake" + + @contextlib.contextmanager + def _fake_impl(plan): + yield _FakeBottle() + + class _FakeBackend(BottleBackend): + name = "fake" + + @classmethod + def is_available(cls): return True + + def _preflight(self): pass + def _build_guest_env(self, env): return {} + def _resolve_plan(self, spec, **kw): return cast(Any, SimpleNamespace()) + + @contextlib.contextmanager + def _launch_impl(self, plan): + yield _FakeBottle() + + def prepare_cleanup(self): return cast(Any, None) + def cleanup(self, plan): pass + def enumerate_active(self): return [] + + return _FakeBackend() + + def test_stale_checks_called_by_default(self): + backend = self._make_backend() + called = [] + backend._image_stale_checks = lambda plan: called.append(True) # type: ignore[method-assign] + plan = cast(Any, SimpleNamespace()) + with backend.launch(plan): + pass + self.assertEqual([True], called) + + def test_skip_stale_bypasses_stale_checks(self): + backend = self._make_backend() + backend._image_stale_checks = lambda plan: (_ for _ in ()).throw( # type: ignore[method-assign] + AssertionError("should not be called") + ) + plan = cast(Any, SimpleNamespace()) + with backend.launch(plan, skip_stale=True): + pass + + def test_noop_default_image_stale_checks(self): + from bot_bottle.backend import BottleBackend + + class _MinimalBackend(BottleBackend): + name = "minimal" + + @classmethod + def is_available(cls): return True + + def _preflight(self): pass + def _build_guest_env(self, env): return {} + def _resolve_plan(self, spec, **kw): return cast(Any, SimpleNamespace()) + + @contextlib.contextmanager + def _launch_impl(self, plan): + yield cast(Any, SimpleNamespace(name="x")) + + def prepare_cleanup(self): return cast(Any, None) + def cleanup(self, plan): pass + def enumerate_active(self): return [] + + backend = _MinimalBackend() + # The default _image_stale_checks must not raise — it's a no-op. + backend._image_stale_checks(cast(Any, SimpleNamespace())) + + +# --------------------------------------------------------------------------- +# Docker backend stale_checks +# --------------------------------------------------------------------------- + +class TestDockerStaleChecks(unittest.TestCase): + from bot_bottle.backend.docker import launch as _docker_launch_mod + + def _plan(self, policy: str = "cached", slug: str = "dev-abc") -> Any: + spec = SimpleNamespace(image_policy=policy) + provision = SimpleNamespace(image="bot-bottle-agent:latest") + return cast(Any, SimpleNamespace( + spec=spec, + slug=slug, + image="bot-bottle-agent:latest", + agent_provision=provision, + )) + + def test_fresh_policy_is_noop(self): + from bot_bottle.backend.docker import launch as mod + with patch.object(mod, "read_committed_image") as rci, \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan("fresh")) + rci.assert_not_called() + cs.assert_not_called() + + def test_committed_image_present_checks_only_committed(self): + from bot_bottle.backend.docker import launch as mod + from datetime import datetime, timezone + ts = datetime(2025, 1, 1, tzinfo=timezone.utc) + with patch.object(mod, "read_committed_image", return_value="committed:latest"), \ + patch.object(mod.docker_mod, "image_exists", return_value=True), \ + patch.object(mod.docker_mod, "image_created_at", return_value=ts), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + cs.assert_called_once() + self.assertIn("committed:latest", cs.call_args.args[0]) + + def test_no_committed_image_checks_agent_and_sidecar(self): + from bot_bottle.backend.docker import launch as mod + from datetime import datetime, timezone + ts = datetime(2025, 1, 1, tzinfo=timezone.utc) + with patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.docker_mod, "image_exists", return_value=True), \ + patch.object(mod.docker_mod, "image_created_at", return_value=ts), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + # Should have been called for both agent and sidecar images. + self.assertEqual(2, cs.call_count) + + def test_image_not_present_skips_check(self): + from bot_bottle.backend.docker import launch as mod + with patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.docker_mod, "image_exists", return_value=False), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + cs.assert_not_called() + + def test_backend_image_stale_checks_delegates(self): + from bot_bottle.backend.docker.backend import DockerBottleBackend + from bot_bottle.backend.docker import launch as mod + backend = DockerBottleBackend() + plan = self._plan() + with patch.object(mod, "stale_checks") as sc: + backend._image_stale_checks(plan) + sc.assert_called_once_with(plan) + + def test_no_committed_image_only_sidecar_missing_skips_check(self): + # When no committed image, stale_checks checks each image that EXISTS. + # If an image isn't present, check_stale is not called for it. + from bot_bottle.backend.docker import launch as mod + from datetime import datetime, timezone + ts = datetime(2025, 1, 1, tzinfo=timezone.utc) + plan = self._plan() + + def image_exists(ref: str) -> bool: + return ref == plan.image # Only agent present; sidecar missing. + + with patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.docker_mod, "image_exists", side_effect=image_exists), \ + patch.object(mod.docker_mod, "image_created_at", return_value=ts), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(plan) + # Only agent was checked (sidecar not present → skipped). + self.assertEqual(1, cs.call_count) + self.assertIn(plan.image, cs.call_args.args[0]) + + +# --------------------------------------------------------------------------- +# smolmachines backend stale_checks +# --------------------------------------------------------------------------- + +class TestSmolmachinesStaleChecks(unittest.TestCase): + def _plan(self, policy: str = "cached") -> Any: + return cast(Any, SimpleNamespace( + spec=SimpleNamespace(image_policy=policy), + slug="dev-abc", + agent_image="bot-bottle-claude:latest", + )) + + def test_fresh_policy_is_noop(self): + from bot_bottle.backend.smolmachines import launch as mod + with patch.object(mod, "read_committed_image") as rci, \ + patch.object(mod, "check_stale_path") as csp: + mod.stale_checks(self._plan("fresh")) + rci.assert_not_called() + csp.assert_not_called() + + def test_committed_artifact_found_checks_it(self): + from bot_bottle.backend.smolmachines import launch as mod + with tempfile.NamedTemporaryFile(suffix=".smolmachine") as f: + committed_path = f.name + with patch.object(mod, "read_committed_image", return_value=committed_path), \ + patch.object(mod.docker_mod, "image_exists", return_value=False), \ + patch.object(mod, "check_stale_path") as csp: + mod.stale_checks(self._plan()) + csp.assert_called_once() + self.assertIn("agent smolmachine", csp.call_args.args[0]) + + def test_no_committed_agent_artifact_in_cache_is_checked(self): + from bot_bottle.backend.smolmachines import launch as mod + digest = "abcdef0123456789" + with tempfile.TemporaryDirectory() as tmp: + artifact = Path(tmp) / f"{digest}.smolmachine.smolmachine" + artifact.write_text("") + with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ + patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.docker_mod, "image_exists", return_value=True), \ + patch.object(mod.docker_mod, "image_id", + return_value=f"sha256:{digest}ffffffffffffffff"), \ + patch.object(mod, "check_stale_path") as csp: + mod.stale_checks(self._plan()) + # At least the agent artifact check ran. + self.assertGreaterEqual(csp.call_count, 1) + + def test_sidecar_artifact_in_cache_is_checked(self): + from bot_bottle.backend.smolmachines import launch as mod + digest = "1234567890abcdef" + sidecar_digest = "fedcba9876543210" + with tempfile.TemporaryDirectory() as tmp: + sidecar_artifact = Path(tmp) / f"{sidecar_digest}.smolmachine.smolmachine" + sidecar_artifact.write_text("") + + def image_exists(ref: str) -> bool: + return True + + def image_id(ref: str) -> str: + if "sidecar" in ref or ref == mod._bundle.SIDECAR_BUNDLE_IMAGE: + return f"sha256:{sidecar_digest}ffffffff" + return f"sha256:{digest}ffffffff" + + with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ + patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.docker_mod, "image_exists", side_effect=image_exists), \ + patch.object(mod.docker_mod, "image_id", side_effect=image_id), \ + patch.object(mod, "check_stale_path") as csp: + mod.stale_checks(self._plan()) + # sidecar artifact is present, so check_stale_path should fire for it. + sidecar_calls = [c for c in csp.call_args_list if "sidecar" in c.args[0]] + self.assertTrue(len(sidecar_calls) >= 1) + + def test_backend_image_stale_checks_delegates(self): + from bot_bottle.backend.smolmachines.backend import SmolmachinesBottleBackend + from bot_bottle.backend.smolmachines import launch as mod + backend = SmolmachinesBottleBackend() + plan = self._plan() + with patch.object(mod, "stale_checks") as sc: + backend._image_stale_checks(plan) + sc.assert_called_once_with(plan) + + +# --------------------------------------------------------------------------- +# macOS container backend stale_checks +# --------------------------------------------------------------------------- + +class TestMacosContainerStaleChecks(unittest.TestCase): + def _plan(self, policy: str = "cached", slug: str = "dev-abc") -> Any: + return cast(Any, SimpleNamespace( + spec=SimpleNamespace(image_policy=policy), + slug=slug, + image="bot-bottle-agent:latest", + )) + + def test_fresh_policy_is_noop(self): + from bot_bottle.backend.macos_container import launch as mod + with patch.object(mod, "read_committed_image") as rci, \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan("fresh")) + rci.assert_not_called() + cs.assert_not_called() + + def test_committed_image_present_checks_only_committed(self): + from bot_bottle.backend.macos_container import launch as mod + from datetime import datetime, timezone + ts = datetime(2025, 1, 1, tzinfo=timezone.utc) + with patch.object(mod, "read_committed_image", return_value="committed:latest"), \ + patch.object(mod.container_mod, "image_exists", return_value=True), \ + patch.object(mod.container_mod, "image_created_at", return_value=ts), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + cs.assert_called_once() + self.assertIn("committed:latest", cs.call_args.args[0]) + + def test_no_committed_image_checks_agent_and_sidecar(self): + from bot_bottle.backend.macos_container import launch as mod + from datetime import datetime, timezone + ts = datetime(2025, 1, 1, tzinfo=timezone.utc) + with patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.container_mod, "image_exists", return_value=True), \ + patch.object(mod.container_mod, "image_created_at", return_value=ts), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + self.assertEqual(2, cs.call_count) + + def test_image_not_present_skips_check(self): + from bot_bottle.backend.macos_container import launch as mod + with patch.object(mod, "read_committed_image", return_value=""), \ + patch.object(mod.container_mod, "image_exists", return_value=False), \ + patch.object(mod, "check_stale") as cs: + mod.stale_checks(self._plan()) + cs.assert_not_called() + + def test_backend_image_stale_checks_delegates(self): + from bot_bottle.backend.macos_container.backend import MacosContainerBottleBackend + from bot_bottle.backend.macos_container import launch as mod + backend = MacosContainerBottleBackend() + plan = self._plan() + with patch.object(mod, "stale_checks") as sc: + backend._image_stale_checks(plan) + sc.assert_called_once_with(plan) + + +if __name__ == "__main__": + unittest.main() -- 2.52.0 From 537df7a78459294a34ad669075cf33aae5f52c80 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 9 Jul 2026 19:17:43 +0000 Subject: [PATCH 6/9] fix(tests): resolve pyright errors in stale-check test files Remove unused imports, add missing type annotations, fix Die() constructor calls (int code, not str), replace fake backend subclass with patch.object approach to avoid reportMissingParameterType and override-incompatibility errors in strict pyright mode. --- tests/unit/test_cli_start_stale.py | 26 ++-- tests/unit/test_image_cache.py | 6 +- tests/unit/test_macos_container_launch.py | 55 +++---- tests/unit/test_stale_checks.py | 180 +++++++++------------- 4 files changed, 104 insertions(+), 163 deletions(-) diff --git a/tests/unit/test_cli_start_stale.py b/tests/unit/test_cli_start_stale.py index 0e0d7dc..7e20976 100644 --- a/tests/unit/test_cli_start_stale.py +++ b/tests/unit/test_cli_start_stale.py @@ -8,14 +8,12 @@ Exercises the while-True / try-except loop around backend.launch: from __future__ import annotations -import contextlib import io import tempfile import unittest -from pathlib import Path from types import SimpleNamespace from typing import Any, cast -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch from bot_bottle.image_cache import StaleImageError from bot_bottle.log import Die @@ -47,10 +45,10 @@ def _ok_cm(bottle: Any) -> MagicMock: class TestLaunchBottleStaleHandling(unittest.TestCase): - def setUp(self): + def setUp(self) -> None: self._tmp = tempfile.mkdtemp(prefix="cli-stale-test.") - def _spec(self): + def _spec(self) -> Any: from bot_bottle.backend import BottleSpec from bot_bottle.manifest import ManifestIndex idx = ManifestIndex.from_json_obj({ @@ -65,7 +63,7 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): identity="dev-abc", ) - def _run_launch(self, **patch_kwargs) -> int: + def _run_launch(self, **patch_kwargs: Any) -> int: import bot_bottle.cli.start as start_mod spec = self._spec() with patch.object(start_mod, "prepare_with_preflight", @@ -79,7 +77,7 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): **patch_kwargs, ) - def test_headless_stale_calls_die(self): + def test_headless_stale_calls_die(self) -> None: """In headless mode (assume_yes=True), a StaleImageError must call die().""" import bot_bottle.cli.start as start_mod @@ -87,11 +85,11 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): backend_mock.launch.return_value = _stale_cm() with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ - patch.object(start_mod, "die", side_effect=Die("stale")): + patch.object(start_mod, "die", side_effect=Die()): with self.assertRaises(Die): self._run_launch(assume_yes=True) - def test_interactive_user_declines_stops_loop(self): + def test_interactive_user_declines_stops_loop(self) -> None: """Interactive user answering 'n' → loop exits without a second launch.""" import bot_bottle.cli.start as start_mod @@ -107,16 +105,14 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): # launch was called once (stale), then the user declined → no second call. backend_mock.launch.assert_called_once() - def test_interactive_user_confirms_retries_with_skip_stale(self): + def test_interactive_user_confirms_retries_with_skip_stale(self) -> None: """Interactive user answering 'y' → second launch with skip_stale=True.""" import bot_bottle.cli.start as start_mod bottle_mock = MagicMock() bottle_mock.name = "dev-abc" - stale_calls = [0] - - def launch_side_effect(plan, *, skip_stale=False): + def launch_side_effect(plan: Any, *, skip_stale: bool = False) -> Any: if not skip_stale: return _stale_cm() return _ok_cm(bottle_mock) @@ -138,14 +134,14 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): self.assertFalse(calls[0].kwargs.get("skip_stale", False)) self.assertTrue(calls[1].kwargs.get("skip_stale", False)) - def test_interactive_yes_uppercase_also_accepted(self): + def test_interactive_yes_uppercase_also_accepted(self) -> None: """'Y' or 'YES' should also be accepted as confirmation.""" import bot_bottle.cli.start as start_mod bottle_mock = MagicMock() bottle_mock.name = "dev-abc" - def launch_side_effect(plan, *, skip_stale=False): + def launch_side_effect(plan: Any, *, skip_stale: bool = False) -> Any: if not skip_stale: return _stale_cm() return _ok_cm(bottle_mock) diff --git a/tests/unit/test_image_cache.py b/tests/unit/test_image_cache.py index 904a02f..c7adaed 100644 --- a/tests/unit/test_image_cache.py +++ b/tests/unit/test_image_cache.py @@ -11,10 +11,6 @@ from unittest.mock import patch from bot_bottle.image_cache import StaleImageError, check_stale, check_stale_path -def _utc(**kwargs) -> datetime: - return datetime.now(tz=timezone.utc) - timedelta(**kwargs) - - class TestCheckStale(unittest.TestCase): def _run(self, threshold: int, age_days: float) -> None: created = datetime.now(tz=timezone.utc) - timedelta(days=age_days) @@ -53,7 +49,7 @@ class TestCheckStale(unittest.TestCase): # that would be interpreted as local time should still work. # We can't control the local tz in a unit test, so just ensure # no exception is thrown for a very recent naive datetime. - naive_now = datetime.utcnow() + naive_now = datetime(2099, 1, 1) # far future, always "fresh" with patch("bot_bottle.image_cache.ConfigStore") as cs: cs.return_value.cached_image_stale_warning_days.return_value = 1 # Should not raise — the image is brand new. diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 0803d21..4b9d33d 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import unittest import tempfile from pathlib import Path @@ -15,6 +16,7 @@ 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.log import Die from bot_bottle.manifest import ManifestIndex _MANIFEST = ManifestIndex.from_json_obj({ @@ -373,17 +375,16 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): def tearDown(self): self._tmp.cleanup() - def test_cached_mode_dies_when_agent_image_missing(self): - plan = _build_plan(self.stage_dir) - from bot_bottle.backend import BottleSpec - import dataclasses - plan = dataclasses.replace( - plan, + def _cached_plan(self) -> MacosContainerBottlePlan: + return dataclasses.replace( + _build_plan(self.stage_dir), spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), ) - from bot_bottle.log import Die - def image_exists(ref: str) -> bool: + def test_cached_mode_dies_when_agent_image_missing(self) -> None: + plan = self._cached_plan() + + def image_exists(ref: str) -> bool: # pylint: disable=unused-argument return False # Nothing cached. with patch.object( @@ -391,43 +392,29 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): ), patch.object( launch.container_mod, "image_exists", side_effect=image_exists, ), patch.object( - launch, "die", side_effect=Die("no agent image"), + launch, "die", side_effect=Die(), ): with self.assertRaises(Die): launch._build_images(plan) - def test_cached_mode_dies_when_sidecar_image_missing(self): - plan = _build_plan(self.stage_dir) - from bot_bottle.backend import BottleSpec - import dataclasses - plan = dataclasses.replace( - plan, - spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), - ) - from bot_bottle.log import Die + def test_cached_mode_dies_when_sidecar_image_missing(self) -> None: + plan = self._cached_plan() def image_exists(ref: str) -> bool: - # Agent image is present; sidecar is missing. - return ref == plan.image + return ref == plan.image # Agent present; sidecar missing. with patch.object( launch, "read_committed_image", return_value="", ), patch.object( launch.container_mod, "image_exists", side_effect=image_exists, ), patch.object( - launch, "die", side_effect=Die("no sidecar image"), + launch, "die", side_effect=Die(), ): with self.assertRaises(Die): launch._build_images(plan) - def test_cached_mode_both_present_returns_unchanged_plan(self): - plan = _build_plan(self.stage_dir) - from bot_bottle.backend import BottleSpec - import dataclasses - plan = dataclasses.replace( - plan, - spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), - ) + def test_cached_mode_both_present_returns_unchanged_plan(self) -> None: + plan = self._cached_plan() with patch.object( launch, "read_committed_image", return_value="", ), patch.object( @@ -440,14 +427,8 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): build.assert_not_called() self.assertEqual(plan.image, result.image) - def test_committed_image_plus_cached_skips_sidecar_build(self): - plan = _build_plan(self.stage_dir) - from bot_bottle.backend import BottleSpec - import dataclasses - plan = dataclasses.replace( - plan, - spec=cast(BottleSpec, SimpleNamespace(image_policy="cached")), - ) + def test_committed_image_plus_cached_skips_sidecar_build(self) -> None: + plan = self._cached_plan() with patch.object( launch, "read_committed_image", return_value="bot-bottle-committed-dev-abc:latest", diff --git a/tests/unit/test_stale_checks.py b/tests/unit/test_stale_checks.py index 21e5e59..328ec7e 100644 --- a/tests/unit/test_stale_checks.py +++ b/tests/unit/test_stale_checks.py @@ -6,7 +6,6 @@ calls are mocked at the module boundary.""" from __future__ import annotations -import contextlib import tempfile import unittest from pathlib import Path @@ -14,7 +13,13 @@ from types import SimpleNamespace from typing import Any, cast from unittest.mock import MagicMock, patch -from bot_bottle.image_cache import StaleImageError + +def _bottle_cm(bottle: Any) -> MagicMock: + """Return a mock context manager that yields `bottle`.""" + cm = MagicMock() + cm.__enter__ = MagicMock(return_value=bottle) + cm.__exit__ = MagicMock(return_value=False) + return cm # --------------------------------------------------------------------------- @@ -25,78 +30,44 @@ class TestBottleBackendLaunchTemplate(unittest.TestCase): """Verify the concrete launch() method on BottleBackend calls _image_stale_checks unless skip_stale=True, then delegates to _launch_impl.""" - def _make_backend(self): - from bot_bottle.backend import BottleBackend + def _make_backend(self) -> Any: + from bot_bottle.backend.docker.backend import DockerBottleBackend + return DockerBottleBackend() - class _FakeBottle: - name = "fake" - - @contextlib.contextmanager - def _fake_impl(plan): - yield _FakeBottle() - - class _FakeBackend(BottleBackend): - name = "fake" - - @classmethod - def is_available(cls): return True - - def _preflight(self): pass - def _build_guest_env(self, env): return {} - def _resolve_plan(self, spec, **kw): return cast(Any, SimpleNamespace()) - - @contextlib.contextmanager - def _launch_impl(self, plan): - yield _FakeBottle() - - def prepare_cleanup(self): return cast(Any, None) - def cleanup(self, plan): pass - def enumerate_active(self): return [] - - return _FakeBackend() - - def test_stale_checks_called_by_default(self): + def test_stale_checks_called_by_default(self) -> None: backend = self._make_backend() - called = [] - backend._image_stale_checks = lambda plan: called.append(True) # type: ignore[method-assign] plan = cast(Any, SimpleNamespace()) - with backend.launch(plan): - pass - self.assertEqual([True], called) + bottle = MagicMock() + with patch.object( + backend, "_image_stale_checks", + ) as stale_mock, patch.object( + backend, "_launch_impl", + return_value=_bottle_cm(bottle), + ): + with backend.launch(plan): + pass + stale_mock.assert_called_once_with(plan) - def test_skip_stale_bypasses_stale_checks(self): + def test_skip_stale_bypasses_stale_checks(self) -> None: backend = self._make_backend() - backend._image_stale_checks = lambda plan: (_ for _ in ()).throw( # type: ignore[method-assign] - AssertionError("should not be called") - ) plan = cast(Any, SimpleNamespace()) - with backend.launch(plan, skip_stale=True): - pass + bottle = MagicMock() + with patch.object( + backend, "_image_stale_checks", + ) as stale_mock, patch.object( + backend, "_launch_impl", + return_value=_bottle_cm(bottle), + ): + with backend.launch(plan, skip_stale=True): + pass + stale_mock.assert_not_called() - def test_noop_default_image_stale_checks(self): + def test_noop_default_image_stale_checks(self) -> None: + from bot_bottle.backend.docker.backend import DockerBottleBackend from bot_bottle.backend import BottleBackend - - class _MinimalBackend(BottleBackend): - name = "minimal" - - @classmethod - def is_available(cls): return True - - def _preflight(self): pass - def _build_guest_env(self, env): return {} - def _resolve_plan(self, spec, **kw): return cast(Any, SimpleNamespace()) - - @contextlib.contextmanager - def _launch_impl(self, plan): - yield cast(Any, SimpleNamespace(name="x")) - - def prepare_cleanup(self): return cast(Any, None) - def cleanup(self, plan): pass - def enumerate_active(self): return [] - - backend = _MinimalBackend() - # The default _image_stale_checks must not raise — it's a no-op. - backend._image_stale_checks(cast(Any, SimpleNamespace())) + backend = DockerBottleBackend() + # Call the base-class _image_stale_checks directly to verify it's a no-op. + BottleBackend._image_stale_checks(backend, cast(Any, SimpleNamespace())) # type: ignore[arg-type] # --------------------------------------------------------------------------- @@ -104,8 +75,6 @@ class TestBottleBackendLaunchTemplate(unittest.TestCase): # --------------------------------------------------------------------------- class TestDockerStaleChecks(unittest.TestCase): - from bot_bottle.backend.docker import launch as _docker_launch_mod - def _plan(self, policy: str = "cached", slug: str = "dev-abc") -> Any: spec = SimpleNamespace(image_policy=policy) provision = SimpleNamespace(image="bot-bottle-agent:latest") @@ -116,7 +85,7 @@ class TestDockerStaleChecks(unittest.TestCase): agent_provision=provision, )) - def test_fresh_policy_is_noop(self): + def test_fresh_policy_is_noop(self) -> None: from bot_bottle.backend.docker import launch as mod with patch.object(mod, "read_committed_image") as rci, \ patch.object(mod, "check_stale") as cs: @@ -124,7 +93,7 @@ class TestDockerStaleChecks(unittest.TestCase): rci.assert_not_called() cs.assert_not_called() - def test_committed_image_present_checks_only_committed(self): + def test_committed_image_present_checks_only_committed(self) -> None: from bot_bottle.backend.docker import launch as mod from datetime import datetime, timezone ts = datetime(2025, 1, 1, tzinfo=timezone.utc) @@ -136,7 +105,7 @@ class TestDockerStaleChecks(unittest.TestCase): cs.assert_called_once() self.assertIn("committed:latest", cs.call_args.args[0]) - def test_no_committed_image_checks_agent_and_sidecar(self): + def test_no_committed_image_checks_agent_and_sidecar(self) -> None: from bot_bottle.backend.docker import launch as mod from datetime import datetime, timezone ts = datetime(2025, 1, 1, tzinfo=timezone.utc) @@ -148,7 +117,7 @@ class TestDockerStaleChecks(unittest.TestCase): # Should have been called for both agent and sidecar images. self.assertEqual(2, cs.call_count) - def test_image_not_present_skips_check(self): + def test_image_not_present_skips_check(self) -> None: from bot_bottle.backend.docker import launch as mod with patch.object(mod, "read_committed_image", return_value=""), \ patch.object(mod.docker_mod, "image_exists", return_value=False), \ @@ -156,18 +125,7 @@ class TestDockerStaleChecks(unittest.TestCase): mod.stale_checks(self._plan()) cs.assert_not_called() - def test_backend_image_stale_checks_delegates(self): - from bot_bottle.backend.docker.backend import DockerBottleBackend - from bot_bottle.backend.docker import launch as mod - backend = DockerBottleBackend() - plan = self._plan() - with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) - sc.assert_called_once_with(plan) - - def test_no_committed_image_only_sidecar_missing_skips_check(self): - # When no committed image, stale_checks checks each image that EXISTS. - # If an image isn't present, check_stale is not called for it. + def test_only_sidecar_missing_checks_only_agent(self) -> None: from bot_bottle.backend.docker import launch as mod from datetime import datetime, timezone ts = datetime(2025, 1, 1, tzinfo=timezone.utc) @@ -181,10 +139,18 @@ class TestDockerStaleChecks(unittest.TestCase): patch.object(mod.docker_mod, "image_created_at", return_value=ts), \ patch.object(mod, "check_stale") as cs: mod.stale_checks(plan) - # Only agent was checked (sidecar not present → skipped). self.assertEqual(1, cs.call_count) self.assertIn(plan.image, cs.call_args.args[0]) + def test_backend_image_stale_checks_delegates(self) -> None: + from bot_bottle.backend.docker.backend import DockerBottleBackend + from bot_bottle.backend.docker import launch as mod + backend = DockerBottleBackend() + plan = self._plan() + with patch.object(mod, "stale_checks") as sc: + backend._image_stale_checks(plan) # type: ignore[arg-type] + sc.assert_called_once_with(plan) + # --------------------------------------------------------------------------- # smolmachines backend stale_checks @@ -198,7 +164,7 @@ class TestSmolmachinesStaleChecks(unittest.TestCase): agent_image="bot-bottle-claude:latest", )) - def test_fresh_policy_is_noop(self): + def test_fresh_policy_is_noop(self) -> None: from bot_bottle.backend.smolmachines import launch as mod with patch.object(mod, "read_committed_image") as rci, \ patch.object(mod, "check_stale_path") as csp: @@ -206,7 +172,7 @@ class TestSmolmachinesStaleChecks(unittest.TestCase): rci.assert_not_called() csp.assert_not_called() - def test_committed_artifact_found_checks_it(self): + def test_committed_artifact_found_checks_it(self) -> None: from bot_bottle.backend.smolmachines import launch as mod with tempfile.NamedTemporaryFile(suffix=".smolmachine") as f: committed_path = f.name @@ -217,7 +183,7 @@ class TestSmolmachinesStaleChecks(unittest.TestCase): csp.assert_called_once() self.assertIn("agent smolmachine", csp.call_args.args[0]) - def test_no_committed_agent_artifact_in_cache_is_checked(self): + def test_no_committed_agent_artifact_in_cache_is_checked(self) -> None: from bot_bottle.backend.smolmachines import launch as mod digest = "abcdef0123456789" with tempfile.TemporaryDirectory() as tmp: @@ -226,28 +192,31 @@ class TestSmolmachinesStaleChecks(unittest.TestCase): with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ patch.object(mod, "read_committed_image", return_value=""), \ patch.object(mod.docker_mod, "image_exists", return_value=True), \ - patch.object(mod.docker_mod, "image_id", - return_value=f"sha256:{digest}ffffffffffffffff"), \ + patch.object( + mod.docker_mod, "image_id", + return_value=f"sha256:{digest}ffffffffffffffff", + ), \ patch.object(mod, "check_stale_path") as csp: mod.stale_checks(self._plan()) # At least the agent artifact check ran. self.assertGreaterEqual(csp.call_count, 1) - def test_sidecar_artifact_in_cache_is_checked(self): + def test_sidecar_artifact_in_cache_is_checked(self) -> None: from bot_bottle.backend.smolmachines import launch as mod - digest = "1234567890abcdef" sidecar_digest = "fedcba9876543210" with tempfile.TemporaryDirectory() as tmp: - sidecar_artifact = Path(tmp) / f"{sidecar_digest}.smolmachine.smolmachine" + sidecar_artifact = ( + Path(tmp) / f"{sidecar_digest}.smolmachine.smolmachine" + ) sidecar_artifact.write_text("") - def image_exists(ref: str) -> bool: + def image_exists(ref: str) -> bool: # pylint: disable=unused-argument return True def image_id(ref: str) -> str: - if "sidecar" in ref or ref == mod._bundle.SIDECAR_BUNDLE_IMAGE: + if ref == mod._bundle.SIDECAR_BUNDLE_IMAGE: # type: ignore[attr-defined] return f"sha256:{sidecar_digest}ffffffff" - return f"sha256:{digest}ffffffff" + return "sha256:0000000000000000ffffffff" with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ patch.object(mod, "read_committed_image", return_value=""), \ @@ -255,17 +224,16 @@ class TestSmolmachinesStaleChecks(unittest.TestCase): patch.object(mod.docker_mod, "image_id", side_effect=image_id), \ patch.object(mod, "check_stale_path") as csp: mod.stale_checks(self._plan()) - # sidecar artifact is present, so check_stale_path should fire for it. sidecar_calls = [c for c in csp.call_args_list if "sidecar" in c.args[0]] - self.assertTrue(len(sidecar_calls) >= 1) + self.assertGreaterEqual(len(sidecar_calls), 1) - def test_backend_image_stale_checks_delegates(self): + def test_backend_image_stale_checks_delegates(self) -> None: from bot_bottle.backend.smolmachines.backend import SmolmachinesBottleBackend from bot_bottle.backend.smolmachines import launch as mod backend = SmolmachinesBottleBackend() plan = self._plan() with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) + backend._image_stale_checks(plan) # type: ignore[arg-type] sc.assert_called_once_with(plan) @@ -281,7 +249,7 @@ class TestMacosContainerStaleChecks(unittest.TestCase): image="bot-bottle-agent:latest", )) - def test_fresh_policy_is_noop(self): + def test_fresh_policy_is_noop(self) -> None: from bot_bottle.backend.macos_container import launch as mod with patch.object(mod, "read_committed_image") as rci, \ patch.object(mod, "check_stale") as cs: @@ -289,7 +257,7 @@ class TestMacosContainerStaleChecks(unittest.TestCase): rci.assert_not_called() cs.assert_not_called() - def test_committed_image_present_checks_only_committed(self): + def test_committed_image_present_checks_only_committed(self) -> None: from bot_bottle.backend.macos_container import launch as mod from datetime import datetime, timezone ts = datetime(2025, 1, 1, tzinfo=timezone.utc) @@ -301,7 +269,7 @@ class TestMacosContainerStaleChecks(unittest.TestCase): cs.assert_called_once() self.assertIn("committed:latest", cs.call_args.args[0]) - def test_no_committed_image_checks_agent_and_sidecar(self): + def test_no_committed_image_checks_agent_and_sidecar(self) -> None: from bot_bottle.backend.macos_container import launch as mod from datetime import datetime, timezone ts = datetime(2025, 1, 1, tzinfo=timezone.utc) @@ -312,7 +280,7 @@ class TestMacosContainerStaleChecks(unittest.TestCase): mod.stale_checks(self._plan()) self.assertEqual(2, cs.call_count) - def test_image_not_present_skips_check(self): + def test_image_not_present_skips_check(self) -> None: from bot_bottle.backend.macos_container import launch as mod with patch.object(mod, "read_committed_image", return_value=""), \ patch.object(mod.container_mod, "image_exists", return_value=False), \ @@ -320,13 +288,13 @@ class TestMacosContainerStaleChecks(unittest.TestCase): mod.stale_checks(self._plan()) cs.assert_not_called() - def test_backend_image_stale_checks_delegates(self): + def test_backend_image_stale_checks_delegates(self) -> None: from bot_bottle.backend.macos_container.backend import MacosContainerBottleBackend from bot_bottle.backend.macos_container import launch as mod backend = MacosContainerBottleBackend() plan = self._plan() with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) + backend._image_stale_checks(plan) # type: ignore[arg-type] sc.assert_called_once_with(plan) -- 2.52.0 From d1e121febe20c5c275b388c842bde5fc1a615205 Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 10 Jul 2026 20:25:24 +0000 Subject: [PATCH 7/9] refactor: BottleImages dataclass, prelaunch_checks, build_or_load_images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review comments 3098, 3099, 3100 on PR #336: - Add BottleImages(agent, sidecar) dataclass to backend/__init__.py. Docker/macOS backends use str image refs; smolmachines uses Path artifacts. Replaces the singular `image` variable from the canonical pattern in comment 3100. - Replace _image_stale_checks/skip_stale with public prelaunch_checks(). CLI now calls backend.prelaunch_checks(plan) before backend.launch(plan); if StaleImageError is raised and the operator confirms, launch proceeds without re-checking. Removes the while-True/skip_stale retry loop. - Add abstract _build_or_load_images(plan) -> BottleImages to BottleBackend. launch() calls it then passes images to _launch_impl. Each backend implements both methods. - Fix comment 3098 (macos-container): _build_images is removed. build_or_load_images() has separate fresh/cached code paths — the cached path never calls a build helper. - Update _start_bundle (smolmachines) to accept sidecar_artifact: Path directly. Sidecar artifact resolution moves to _sidecar_from_path(), called by build_or_load_images alongside _agent_from_path(). --- bot_bottle/backend/__init__.py | 43 ++++-- bot_bottle/backend/docker/backend.py | 11 +- bot_bottle/backend/docker/launch.py | 69 ++++----- bot_bottle/backend/macos_container/backend.py | 11 +- bot_bottle/backend/macos_container/launch.py | 67 +++++---- bot_bottle/cli/start.py | 59 ++++---- tests/unit/test_cli_start_stale.py | 56 +++----- .../test_docker_launch_committed_image.py | 10 +- tests/unit/test_docker_launch_teardown.py | 6 +- tests/unit/test_macos_container_launch.py | 22 +-- tests/unit/test_stale_checks.py | 134 +++--------------- 11 files changed, 202 insertions(+), 286 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index dbfd08d..57679f0 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -276,6 +276,18 @@ PlanT = TypeVar("PlanT", bound=BottlePlan) CleanupT = TypeVar("CleanupT", bound=BottleCleanupPlan) +@dataclass(frozen=True) +class BottleImages: + """Resolved image references (or artifact paths) for a bottle launch. + + For Docker/macOS-container backends, `agent` and `sidecar` are string + image refs. For the smolmachines backend they are Path objects pointing + to pre-built `.smolmachine` artifacts.""" + + agent: "str | Path" + sidecar: "str | Path" + + class BottleBackend(ABC, Generic[PlanT, CleanupT]): """Abstract base for selectable bottle backends. Concrete subclasses (e.g. DockerBottleBackend) own their own prepare/launch impls. @@ -435,26 +447,27 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): prompt file, Dockerfile path, and guest home all live on `agent_provision_plan` — the source of truth.""" + def prelaunch_checks(self, plan: PlanT) -> None: + """Raise StaleImageError if any cached image used by this plan is stale. + No-op default; backends override to call the shared check_stale* + helpers on their image/artifact timestamps. Called by the CLI before + launch so the operator can be prompted outside the launch context.""" + @contextmanager - def launch( - self, plan: PlanT, *, skip_stale: bool = False - ) -> Generator[Bottle, None, None]: - """Template: optionally check for stale cached images, then delegate - to `_launch_impl`. Pass `skip_stale=True` to bypass the stale check - (used by the interactive CLI after the operator confirms).""" - if not skip_stale: - self._image_stale_checks(plan) - with self._launch_impl(plan) as bottle: + def launch(self, plan: PlanT) -> Generator[Bottle, None, None]: + """Template: build or load images, then delegate to _launch_impl.""" + images = self._build_or_load_images(plan) + with self._launch_impl(plan, images) as bottle: yield bottle - def _image_stale_checks(self, plan: PlanT) -> None: - """Raise StaleImageError if any cached image used by this plan is stale. - No-op default; backends override to call the shared `check_stale*` - helpers on their image/artifact timestamps.""" + @abstractmethod + def _build_or_load_images(self, plan: PlanT) -> "BottleImages": + """Return the agent and sidecar image references (or artifact paths) + for this plan, building fresh images when the policy requires it.""" @abstractmethod - def _launch_impl(self, plan: PlanT) -> AbstractContextManager[Bottle]: - """Build/run the bottle and yield a handle; tear down on exit.""" + def _launch_impl(self, plan: PlanT, images: "BottleImages") -> AbstractContextManager[Bottle]: + """Bring up the bottle using pre-resolved images; yield a handle; tear down on exit.""" def provision(self, plan: PlanT, bottle: "Bottle") -> str | None: """Copy host-side files (CA cert, prompt, skills, .git) into diff --git a/bot_bottle/backend/docker/backend.py b/bot_bottle/backend/docker/backend.py index 3034bfb..154a8dc 100644 --- a/bot_bottle/backend/docker/backend.py +++ b/bot_bottle/backend/docker/backend.py @@ -31,7 +31,7 @@ from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan from ...manifest import Manifest -from .. import ActiveAgent, BottleBackend, BottleSpec +from .. import ActiveAgent, BottleBackend, BottleImages, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate from . import launch as _launch @@ -85,12 +85,15 @@ class DockerBottleBackend(BottleBackend["DockerBottlePlan", "DockerBottleCleanup stage_dir=stage_dir, ) - def _image_stale_checks(self, plan: DockerBottlePlan) -> None: + def prelaunch_checks(self, plan: DockerBottlePlan) -> None: _launch.stale_checks(plan) + def _build_or_load_images(self, plan: DockerBottlePlan) -> BottleImages: + return _launch.build_or_load_images(plan) + @contextmanager - def _launch_impl(self, plan: DockerBottlePlan) -> Generator[DockerBottle, None, None]: - with _launch.launch(plan, provision=self.provision) as bottle: + def _launch_impl(self, plan: DockerBottlePlan, images: BottleImages) -> Generator[DockerBottle, None, None]: + with _launch.launch(plan, images, provision=self.provision) as bottle: yield bottle def supervise_mcp_url(self, plan: DockerBottlePlan) -> str: diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 731874f..2c75230 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -43,6 +43,7 @@ from ...git_gate import ( ) from ...image_cache import check_stale from ...log import die, info, warn +from .. import BottleImages from . import network as network_mod from . import util as docker_mod from .bottle import DockerBottle @@ -71,16 +72,50 @@ from .sidecar_bundle import SIDECAR_BUNDLE_IMAGE _REPO_DIR = str(Path(__file__).resolve().parent.parent.parent.parent) +def build_or_load_images(plan: DockerBottlePlan) -> BottleImages: + """Resolve the agent and sidecar image refs for this plan. + + Returns the committed snapshot if one exists, the local cached images + when the policy is 'cached', or builds fresh images and returns those.""" + committed = read_committed_image(plan.slug) + if committed and docker_mod.image_exists(committed): + info(f"using committed image {committed!r}") + return BottleImages(agent=committed, sidecar=SIDECAR_BUNDLE_IMAGE) + if plan.spec.image_policy == "cached": + if not docker_mod.image_exists(plan.image): + die( + f"cached agent image {plan.image!r} not found; " + "run without --cached-images to build it" + ) + if not docker_mod.image_exists(SIDECAR_BUNDLE_IMAGE): + die( + f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " + "run without --cached-images to build it" + ) + info(f"using cached agent image {plan.image!r}") + info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") + return BottleImages(agent=plan.image, sidecar=SIDECAR_BUNDLE_IMAGE) + docker_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) + return BottleImages(agent=plan.image, sidecar=SIDECAR_BUNDLE_IMAGE) + + @contextmanager def launch( plan: DockerBottlePlan, + images: BottleImages, *, provision: Callable[[DockerBottlePlan, "DockerBottle"], str | None], ) -> Generator[DockerBottle, None, None]: - """Build, launch, and provision a Docker bottle via compose. - Teardown on exit.""" + """Launch and provision a Docker bottle via compose. Teardown on exit.""" stack = ExitStack() + # Stamp the resolved agent image ref into the plan so compose rendering + # picks up the right image (may be a committed snapshot or cached ref). + plan = dataclasses.replace( + plan, + agent_provision=dataclasses.replace(plan.agent_provision, image=str(images.agent)), + ) + _bottle_for_revoke = plan.manifest.bottle _git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) @@ -97,36 +132,6 @@ def launch( ) try: - # 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) - cached_policy = plan.spec.image_policy == "cached" - 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), - ) - elif cached_policy: - if not docker_mod.image_exists(plan.image): - die( - f"cached agent image {plan.image!r} not found; " - "run without --cached-images to build it" - ) - if not docker_mod.image_exists(SIDECAR_BUNDLE_IMAGE): - die( - f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " - "run without --cached-images to build it" - ) - info(f"using cached agent image {plan.image!r}") - info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") - 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/macos_container/backend.py b/bot_bottle/backend/macos_container/backend.py index 4626233..e7a8745 100644 --- a/bot_bottle/backend/macos_container/backend.py +++ b/bot_bottle/backend/macos_container/backend.py @@ -12,7 +12,7 @@ from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan from ...manifest import Manifest -from .. import ActiveAgent, BottleBackend, BottleSpec +from .. import ActiveAgent, BottleBackend, BottleImages, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate from . import launch as _launch @@ -67,14 +67,17 @@ class MacosContainerBottleBackend( stage_dir=stage_dir, ) - def _image_stale_checks(self, plan: MacosContainerBottlePlan) -> None: + def prelaunch_checks(self, plan: MacosContainerBottlePlan) -> None: _launch.stale_checks(plan) + def _build_or_load_images(self, plan: MacosContainerBottlePlan) -> BottleImages: + return _launch.build_or_load_images(plan) + @contextmanager def _launch_impl( - self, plan: MacosContainerBottlePlan + self, plan: MacosContainerBottlePlan, images: BottleImages ) -> Generator[MacosContainerBottle, None, None]: - with _launch.launch(plan, provision=self.provision) as bottle: + with _launch.launch(plan, images, provision=self.provision) as bottle: yield bottle def prepare_cleanup(self) -> MacosContainerBottleCleanupPlan: diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 83b56ec..2d65502 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -34,6 +34,7 @@ from ...git_gate import ( ) from ...image_cache import check_stale from ...log import die, info, warn +from .. import BottleImages from ...supervise import DB_PATH_IN_CONTAINER, SUPERVISE_PORT from ...util import expand_tilde from ..docker.egress import EGRESS_CA_IN_CONTAINER, EGRESS_PORT @@ -72,17 +73,51 @@ def sidecar_container_name(slug: str) -> str: return f"bot-bottle-sidecars-{slug}" +def build_or_load_images(plan: MacosContainerBottlePlan) -> BottleImages: + """Resolve agent and sidecar image refs. Builds the sidecar when needed, + but never builds the agent when policy is 'cached'.""" + committed = read_committed_image(plan.slug) + if committed and container_mod.image_exists(committed): + info(f"using committed image {committed!r}") + if plan.spec.image_policy != "cached": + container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) + return BottleImages(agent=committed, sidecar=SIDECAR_BUNDLE_IMAGE) + if plan.spec.image_policy == "cached": + if not container_mod.image_exists(plan.image): + die( + f"cached agent image {plan.image!r} not found; " + "run without --cached-images to build it" + ) + if not container_mod.image_exists(SIDECAR_BUNDLE_IMAGE): + die( + f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " + "run without --cached-images to build it" + ) + info(f"using cached agent image {plan.image!r}") + info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") + return BottleImages(agent=plan.image, sidecar=SIDECAR_BUNDLE_IMAGE) + container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) + container_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) + return BottleImages(agent=plan.image, sidecar=SIDECAR_BUNDLE_IMAGE) + + @contextmanager def launch( plan: MacosContainerBottlePlan, + images: BottleImages, *, provision: Callable[[MacosContainerBottlePlan, "MacosContainerBottle"], str | None], ) -> Generator[MacosContainerBottle, None, None]: - """Build, run, provision, and yield an Apple Container bottle.""" + """Run, provision, and yield an Apple Container bottle.""" stack = ExitStack() bottle_for_revoke = plan.manifest.bottle git_gate_dir_for_revoke = git_gate_state_dir(plan.slug) + plan = dataclasses.replace( + plan, + agent_provision=dataclasses.replace(plan.agent_provision, image=str(images.agent)), + ) + def teardown() -> None: teardown_exc: BaseException | None = None try: @@ -96,7 +131,6 @@ def launch( try: plan = _mint_certs(plan) - plan = _build_images(plan) internal_network = internal_network_name(plan.slug) egress_network = egress_network_name(plan.slug) @@ -149,35 +183,6 @@ def _mint_certs(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: return dataclasses.replace(plan, egress_plan=egress_plan) -def _build_images(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: - cached = plan.spec.image_policy == "cached" - committed = read_committed_image(plan.slug) - if committed and container_mod.image_exists(committed): - info(f"using committed image {committed!r}") - if not cached: - container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) - return dataclasses.replace( - plan, - agent_provision=dataclasses.replace(plan.agent_provision, image=committed), - ) - if cached: - if not container_mod.image_exists(plan.image): - die( - f"cached agent image {plan.image!r} not found; " - "run without --cached-images to build it" - ) - if not container_mod.image_exists(SIDECAR_BUNDLE_IMAGE): - die( - f"cached sidecar image {SIDECAR_BUNDLE_IMAGE!r} not found; " - "run without --cached-images to build it" - ) - info(f"using cached agent image {plan.image!r}") - info(f"using cached sidecar image {SIDECAR_BUNDLE_IMAGE!r}") - return plan - container_mod.build_image(SIDECAR_BUNDLE_IMAGE, _REPO_DIR, dockerfile=SIDECAR_BUNDLE_DOCKERFILE) - container_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) - return plan - def stale_checks(plan: MacosContainerBottlePlan) -> None: """Raise StaleImageError if a cached image is older than the configured diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index a494486..cd309eb 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -556,38 +556,35 @@ def _launch_bottle( return 0 backend = get_bottle_backend(backend_name) - skip_stale = False - while True: - try: - with backend.launch(plan, skip_stale=skip_stale) as bottle: - agent_provider_template = getattr(plan, "agent_provider_template", "claude") - extra_args: tuple[str, ...] = () - if headless_prompt_text: - extra_args = tuple( - get_provider(agent_provider_template).headless_prompt( - headless_prompt_text - ) - ) - exit_code = attach_agent( - bottle, - agent_provider_template=agent_provider_template, - startup_args=plan.agent_provision.startup_args + extra_args, + try: + backend.prelaunch_checks(plan) + except StaleImageError as exc: + if assume_yes: + die(str(exc)) + sys.stderr.write(f"bot-bottle: {exc}\nLaunch anyway? [y/N] ") + sys.stderr.flush() + if read_tty_line() not in ("y", "Y", "yes", "YES"): + return 0 + with backend.launch(plan) as bottle: + agent_provider_template = getattr(plan, "agent_provider_template", "claude") + extra_args: tuple[str, ...] = () + if headless_prompt_text: + extra_args = tuple( + get_provider(agent_provider_template).headless_prompt( + headless_prompt_text ) - info( - f"session ended (exit {exit_code}); " - f"container {bottle.name} will be removed" - ) - if agent_provider_template == "claude": - capture_claude_session_state(identity, exit_code) - break - except StaleImageError as exc: - if assume_yes: - die(str(exc)) - sys.stderr.write(f"bot-bottle: {exc}\nLaunch anyway? [y/N] ") - sys.stderr.flush() - if read_tty_line() not in ("y", "Y", "yes", "YES"): - break - skip_stale = True + ) + exit_code = attach_agent( + bottle, + agent_provider_template=agent_provider_template, + startup_args=plan.agent_provision.startup_args + extra_args, + ) + info( + f"session ended (exit {exit_code}); " + f"container {bottle.name} will be removed" + ) + if agent_provider_template == "claude": + capture_claude_session_state(identity, exit_code) return 0 finally: # PRD 0018 chunk 2: prepare now writes the bottle's bind-mount diff --git a/tests/unit/test_cli_start_stale.py b/tests/unit/test_cli_start_stale.py index 7e20976..87625ae 100644 --- a/tests/unit/test_cli_start_stale.py +++ b/tests/unit/test_cli_start_stale.py @@ -1,9 +1,9 @@ """Unit: _launch_bottle StaleImageError handling. -Exercises the while-True / try-except loop around backend.launch: +Exercises prelaunch_checks / backend.launch flow: - headless mode → die on stale - interactive mode, user declines → stop without launching - - interactive mode, user confirms → retry with skip_stale=True + - interactive mode, user confirms → skip stale check and launch once """ from __future__ import annotations @@ -28,14 +28,6 @@ def _fake_plan() -> Any: )) -def _stale_cm() -> MagicMock: - """Return a context-manager mock whose __enter__ raises StaleImageError.""" - cm = MagicMock() - cm.__enter__ = MagicMock(side_effect=StaleImageError("image is 5 day(s) old")) - cm.__exit__ = MagicMock(return_value=False) - return cm - - def _ok_cm(bottle: Any) -> MagicMock: """Return a context-manager mock that yields `bottle`.""" cm = MagicMock() @@ -78,23 +70,25 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): ) def test_headless_stale_calls_die(self) -> None: - """In headless mode (assume_yes=True), a StaleImageError must call die().""" + """In headless mode (assume_yes=True), a StaleImageError from prelaunch_checks must call die().""" import bot_bottle.cli.start as start_mod backend_mock = MagicMock() - backend_mock.launch.return_value = _stale_cm() + backend_mock.prelaunch_checks.side_effect = StaleImageError("image is 5 day(s) old") with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ patch.object(start_mod, "die", side_effect=Die()): with self.assertRaises(Die): self._run_launch(assume_yes=True) - def test_interactive_user_declines_stops_loop(self) -> None: - """Interactive user answering 'n' → loop exits without a second launch.""" + backend_mock.launch.assert_not_called() + + def test_interactive_user_declines_stops_before_launch(self) -> None: + """Interactive user answering 'n' → launch is never called.""" import bot_bottle.cli.start as start_mod backend_mock = MagicMock() - backend_mock.launch.return_value = _stale_cm() + backend_mock.prelaunch_checks.side_effect = StaleImageError("image is 5 day(s) old") with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ patch.object(start_mod, "read_tty_line", return_value="n"), \ @@ -102,23 +96,18 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): rc = self._run_launch(assume_yes=False) self.assertEqual(0, rc) - # launch was called once (stale), then the user declined → no second call. - backend_mock.launch.assert_called_once() + backend_mock.launch.assert_not_called() - def test_interactive_user_confirms_retries_with_skip_stale(self) -> None: - """Interactive user answering 'y' → second launch with skip_stale=True.""" + def test_interactive_user_confirms_launches_once(self) -> None: + """Interactive user answering 'y' → prelaunch stale error is bypassed; launch called once.""" import bot_bottle.cli.start as start_mod bottle_mock = MagicMock() bottle_mock.name = "dev-abc" - def launch_side_effect(plan: Any, *, skip_stale: bool = False) -> Any: - if not skip_stale: - return _stale_cm() - return _ok_cm(bottle_mock) - backend_mock = MagicMock() - backend_mock.launch.side_effect = launch_side_effect + backend_mock.prelaunch_checks.side_effect = StaleImageError("image is 5 day(s) old") + backend_mock.launch.return_value = _ok_cm(bottle_mock) with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ patch.object(start_mod, "read_tty_line", return_value="y"), \ @@ -128,11 +117,8 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): rc = self._run_launch(assume_yes=False) self.assertEqual(0, rc) - # First call: skip_stale=False → stale. Second call: skip_stale=True → ok. - self.assertEqual(2, backend_mock.launch.call_count) - calls = backend_mock.launch.call_args_list - self.assertFalse(calls[0].kwargs.get("skip_stale", False)) - self.assertTrue(calls[1].kwargs.get("skip_stale", False)) + backend_mock.prelaunch_checks.assert_called_once() + backend_mock.launch.assert_called_once() def test_interactive_yes_uppercase_also_accepted(self) -> None: """'Y' or 'YES' should also be accepted as confirmation.""" @@ -141,13 +127,9 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): bottle_mock = MagicMock() bottle_mock.name = "dev-abc" - def launch_side_effect(plan: Any, *, skip_stale: bool = False) -> Any: - if not skip_stale: - return _stale_cm() - return _ok_cm(bottle_mock) - backend_mock = MagicMock() - backend_mock.launch.side_effect = launch_side_effect + backend_mock.prelaunch_checks.side_effect = StaleImageError("image is 5 day(s) old") + backend_mock.launch.return_value = _ok_cm(bottle_mock) with patch.object(start_mod, "get_bottle_backend", return_value=backend_mock), \ patch.object(start_mod, "read_tty_line", return_value="YES"), \ @@ -157,7 +139,7 @@ class TestLaunchBottleStaleHandling(unittest.TestCase): rc = self._run_launch(assume_yes=False) self.assertEqual(0, rc) - self.assertEqual(2, backend_mock.launch.call_count) + backend_mock.launch.assert_called_once() if __name__ == "__main__": diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py index 8fd8af9..bbdfb6b 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -12,7 +12,7 @@ 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 import BottleImages, 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 @@ -90,7 +90,7 @@ class TestLaunchCommittedImage(unittest.TestCase): committed_tag: str | None = None, image_present: bool = True, ) -> list[str]: - """Drive launch() through its full sequence with the committed-image + """Drive build_or_load_images() + launch() 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] = [] @@ -124,8 +124,9 @@ class TestLaunchCommittedImage(unittest.TestCase): mock.patch.object(launch_mod, "compose_dump_logs"), \ mock.patch.object(launch_mod, "compose_down"), \ contextlib.redirect_stderr(io.StringIO()): + images = launch_mod.build_or_load_images(plan) provision = mock.Mock(return_value=None) - with launch_mod.launch(plan, provision=provision): + with launch_mod.launch(plan, images, provision=provision): pass return built @@ -170,8 +171,9 @@ class TestLaunchCommittedImage(unittest.TestCase): mock.patch.object(launch_mod, "compose_dump_logs"), \ mock.patch.object(launch_mod, "compose_down"), \ contextlib.redirect_stderr(io.StringIO()): + images = launch_mod.build_or_load_images(plan) provision = mock.Mock(return_value=None) - with launch_mod.launch(plan, provision=provision): + with launch_mod.launch(plan, images, provision=provision): pass self.assertEqual(1, len(captured_plans)) diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py index 983bbc6..3aab57e 100644 --- a/tests/unit/test_docker_launch_teardown.py +++ b/tests/unit/test_docker_launch_teardown.py @@ -16,7 +16,7 @@ from pathlib import Path from unittest import mock from bot_bottle.agent_provider import AgentProvisionPlan -from bot_bottle.backend import BottleSpec +from bot_bottle.backend import BottleImages, 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 @@ -86,6 +86,8 @@ class TestTeardownWarning(unittest.TestCase): plan = _plan(self._tmp) buf = io.StringIO() + images = BottleImages(agent="bot-bottle-claude:latest", sidecar="bot-bottle-sidecars:latest") + with mock.patch.object(launch_mod.docker_mod, "build_image"), \ mock.patch.object( launch_mod, "egress_tls_init", @@ -115,7 +117,7 @@ class TestTeardownWarning(unittest.TestCase): ), \ contextlib.redirect_stderr(buf): provision = mock.Mock(return_value=None) - with launch_mod.launch(plan, provision=provision): + with launch_mod.launch(plan, images, provision=provision): pass output = buf.getvalue() diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 4b9d33d..a2c5978 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -11,7 +11,7 @@ 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 import BottleImages, 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 @@ -339,9 +339,9 @@ class TestMacosContainerLaunchCommittedImage(unittest.TestCase): ), patch.object( launch.container_mod, "build_image", side_effect=fake_build, ), patch.object(launch, "info"): - updated = launch._build_images(plan) + images = launch.build_or_load_images(plan) - self.assertEqual("bot-bottle-committed-dev-abc:latest", updated.image) + self.assertEqual("bot-bottle-committed-dev-abc:latest", images.agent) self.assertEqual(1, len(calls)) self.assertEqual(launch.SIDECAR_BUNDLE_IMAGE, calls[0][0]) @@ -360,9 +360,9 @@ class TestMacosContainerLaunchCommittedImage(unittest.TestCase): ), patch.object( launch.container_mod, "build_image", side_effect=fake_build, ): - updated = launch._build_images(plan) + images = launch.build_or_load_images(plan) - self.assertEqual("bot-bottle-agent:latest", updated.image) + self.assertEqual("bot-bottle-agent:latest", images.agent) self.assertEqual(2, len(calls)) self.assertEqual("bot-bottle-agent:latest", calls[1][0]) @@ -395,7 +395,7 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): launch, "die", side_effect=Die(), ): with self.assertRaises(Die): - launch._build_images(plan) + launch.build_or_load_images(plan) def test_cached_mode_dies_when_sidecar_image_missing(self) -> None: plan = self._cached_plan() @@ -411,7 +411,7 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): launch, "die", side_effect=Die(), ): with self.assertRaises(Die): - launch._build_images(plan) + launch.build_or_load_images(plan) def test_cached_mode_both_present_returns_unchanged_plan(self) -> None: plan = self._cached_plan() @@ -422,10 +422,10 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): ), patch.object( launch.container_mod, "build_image", ) as build, patch.object(launch, "info"): - result = launch._build_images(plan) + images = launch.build_or_load_images(plan) build.assert_not_called() - self.assertEqual(plan.image, result.image) + self.assertEqual(plan.image, images.agent) def test_committed_image_plus_cached_skips_sidecar_build(self) -> None: plan = self._cached_plan() @@ -437,11 +437,11 @@ class TestMacosContainerLaunchCachedImages(unittest.TestCase): ), patch.object( launch.container_mod, "build_image", ) as build, patch.object(launch, "info"): - result = launch._build_images(plan) + images = launch.build_or_load_images(plan) # In cached mode with a committed image, no builds should fire. build.assert_not_called() - self.assertEqual("bot-bottle-committed-dev-abc:latest", result.image) + self.assertEqual("bot-bottle-committed-dev-abc:latest", images.agent) if __name__ == "__main__": diff --git a/tests/unit/test_stale_checks.py b/tests/unit/test_stale_checks.py index 328ec7e..06860d6 100644 --- a/tests/unit/test_stale_checks.py +++ b/tests/unit/test_stale_checks.py @@ -1,12 +1,11 @@ """Unit: stale-image check functions across backends, and the -BottleBackend.launch template method (skip_stale flag). +BottleBackend.launch template method (prelaunch_checks + build_or_load_images). No real images or containers are used — all Docker/container/smolmachine calls are mocked at the module boundary.""" from __future__ import annotations -import tempfile import unittest from pathlib import Path from types import SimpleNamespace @@ -23,51 +22,41 @@ def _bottle_cm(bottle: Any) -> MagicMock: # --------------------------------------------------------------------------- -# BottleBackend.launch template — _image_stale_checks + skip_stale +# BottleBackend.launch template — prelaunch_checks + _build_or_load_images # --------------------------------------------------------------------------- class TestBottleBackendLaunchTemplate(unittest.TestCase): """Verify the concrete launch() method on BottleBackend calls - _image_stale_checks unless skip_stale=True, then delegates to _launch_impl.""" + _build_or_load_images and _launch_impl, and that prelaunch_checks is a no-op + on the base class.""" def _make_backend(self) -> Any: from bot_bottle.backend.docker.backend import DockerBottleBackend return DockerBottleBackend() - def test_stale_checks_called_by_default(self) -> None: + def test_launch_delegates_to_build_or_load_and_launch_impl(self) -> None: + from bot_bottle.backend import BottleImages backend = self._make_backend() plan = cast(Any, SimpleNamespace()) bottle = MagicMock() + images = BottleImages(agent="agent:latest", sidecar="sidecar:latest") with patch.object( - backend, "_image_stale_checks", - ) as stale_mock, patch.object( + backend, "_build_or_load_images", return_value=images, + ) as build_mock, patch.object( backend, "_launch_impl", return_value=_bottle_cm(bottle), - ): + ) as impl_mock: with backend.launch(plan): pass - stale_mock.assert_called_once_with(plan) + build_mock.assert_called_once_with(plan) + impl_mock.assert_called_once_with(plan, images) - def test_skip_stale_bypasses_stale_checks(self) -> None: - backend = self._make_backend() - plan = cast(Any, SimpleNamespace()) - bottle = MagicMock() - with patch.object( - backend, "_image_stale_checks", - ) as stale_mock, patch.object( - backend, "_launch_impl", - return_value=_bottle_cm(bottle), - ): - with backend.launch(plan, skip_stale=True): - pass - stale_mock.assert_not_called() - - def test_noop_default_image_stale_checks(self) -> None: + def test_noop_default_prelaunch_checks(self) -> None: from bot_bottle.backend.docker.backend import DockerBottleBackend from bot_bottle.backend import BottleBackend backend = DockerBottleBackend() - # Call the base-class _image_stale_checks directly to verify it's a no-op. - BottleBackend._image_stale_checks(backend, cast(Any, SimpleNamespace())) # type: ignore[arg-type] + # Base-class prelaunch_checks is a no-op — must not raise. + BottleBackend.prelaunch_checks(backend, cast(Any, SimpleNamespace())) # type: ignore[arg-type] # --------------------------------------------------------------------------- @@ -142,98 +131,13 @@ class TestDockerStaleChecks(unittest.TestCase): self.assertEqual(1, cs.call_count) self.assertIn(plan.image, cs.call_args.args[0]) - def test_backend_image_stale_checks_delegates(self) -> None: + def test_backend_prelaunch_checks_delegates(self) -> None: from bot_bottle.backend.docker.backend import DockerBottleBackend from bot_bottle.backend.docker import launch as mod backend = DockerBottleBackend() plan = self._plan() with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) # type: ignore[arg-type] - sc.assert_called_once_with(plan) - - -# --------------------------------------------------------------------------- -# smolmachines backend stale_checks -# --------------------------------------------------------------------------- - -class TestSmolmachinesStaleChecks(unittest.TestCase): - def _plan(self, policy: str = "cached") -> Any: - return cast(Any, SimpleNamespace( - spec=SimpleNamespace(image_policy=policy), - slug="dev-abc", - agent_image="bot-bottle-claude:latest", - )) - - def test_fresh_policy_is_noop(self) -> None: - from bot_bottle.backend.smolmachines import launch as mod - with patch.object(mod, "read_committed_image") as rci, \ - patch.object(mod, "check_stale_path") as csp: - mod.stale_checks(self._plan("fresh")) - rci.assert_not_called() - csp.assert_not_called() - - def test_committed_artifact_found_checks_it(self) -> None: - from bot_bottle.backend.smolmachines import launch as mod - with tempfile.NamedTemporaryFile(suffix=".smolmachine") as f: - committed_path = f.name - with patch.object(mod, "read_committed_image", return_value=committed_path), \ - patch.object(mod.docker_mod, "image_exists", return_value=False), \ - patch.object(mod, "check_stale_path") as csp: - mod.stale_checks(self._plan()) - csp.assert_called_once() - self.assertIn("agent smolmachine", csp.call_args.args[0]) - - def test_no_committed_agent_artifact_in_cache_is_checked(self) -> None: - from bot_bottle.backend.smolmachines import launch as mod - digest = "abcdef0123456789" - with tempfile.TemporaryDirectory() as tmp: - artifact = Path(tmp) / f"{digest}.smolmachine.smolmachine" - artifact.write_text("") - with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ - patch.object(mod, "read_committed_image", return_value=""), \ - patch.object(mod.docker_mod, "image_exists", return_value=True), \ - patch.object( - mod.docker_mod, "image_id", - return_value=f"sha256:{digest}ffffffffffffffff", - ), \ - patch.object(mod, "check_stale_path") as csp: - mod.stale_checks(self._plan()) - # At least the agent artifact check ran. - self.assertGreaterEqual(csp.call_count, 1) - - def test_sidecar_artifact_in_cache_is_checked(self) -> None: - from bot_bottle.backend.smolmachines import launch as mod - sidecar_digest = "fedcba9876543210" - with tempfile.TemporaryDirectory() as tmp: - sidecar_artifact = ( - Path(tmp) / f"{sidecar_digest}.smolmachine.smolmachine" - ) - sidecar_artifact.write_text("") - - def image_exists(ref: str) -> bool: # pylint: disable=unused-argument - return True - - def image_id(ref: str) -> str: - if ref == mod._bundle.SIDECAR_BUNDLE_IMAGE: # type: ignore[attr-defined] - return f"sha256:{sidecar_digest}ffffffff" - return "sha256:0000000000000000ffffffff" - - with patch.object(mod, "_SMOLMACHINE_CACHE_DIR", Path(tmp)), \ - patch.object(mod, "read_committed_image", return_value=""), \ - patch.object(mod.docker_mod, "image_exists", side_effect=image_exists), \ - patch.object(mod.docker_mod, "image_id", side_effect=image_id), \ - patch.object(mod, "check_stale_path") as csp: - mod.stale_checks(self._plan()) - sidecar_calls = [c for c in csp.call_args_list if "sidecar" in c.args[0]] - self.assertGreaterEqual(len(sidecar_calls), 1) - - def test_backend_image_stale_checks_delegates(self) -> None: - from bot_bottle.backend.smolmachines.backend import SmolmachinesBottleBackend - from bot_bottle.backend.smolmachines import launch as mod - backend = SmolmachinesBottleBackend() - plan = self._plan() - with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) # type: ignore[arg-type] + backend.prelaunch_checks(plan) # type: ignore[arg-type] sc.assert_called_once_with(plan) @@ -288,13 +192,13 @@ class TestMacosContainerStaleChecks(unittest.TestCase): mod.stale_checks(self._plan()) cs.assert_not_called() - def test_backend_image_stale_checks_delegates(self) -> None: + def test_backend_prelaunch_checks_delegates(self) -> None: from bot_bottle.backend.macos_container.backend import MacosContainerBottleBackend from bot_bottle.backend.macos_container import launch as mod backend = MacosContainerBottleBackend() plan = self._plan() with patch.object(mod, "stale_checks") as sc: - backend._image_stale_checks(plan) # type: ignore[arg-type] + backend.prelaunch_checks(plan) # type: ignore[arg-type] sc.assert_called_once_with(plan) -- 2.52.0 From 0afd9c86980f1ab82a481e986e3ee3b2930f4d2b Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 10 Jul 2026 20:36:19 +0000 Subject: [PATCH 8/9] fix: remove redundant quoted annotations in BottleImages and abstract methods `from __future__ import annotations` already defers all annotation evaluation, so quoting `str | Path`, `BottleImages` inside the same module was redundant and tripped pyright strict mode. --- bot_bottle/backend/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index 57679f0..8cb5f6f 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -284,8 +284,8 @@ class BottleImages: image refs. For the smolmachines backend they are Path objects pointing to pre-built `.smolmachine` artifacts.""" - agent: "str | Path" - sidecar: "str | Path" + agent: str | Path + sidecar: str | Path class BottleBackend(ABC, Generic[PlanT, CleanupT]): @@ -461,12 +461,12 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]): yield bottle @abstractmethod - def _build_or_load_images(self, plan: PlanT) -> "BottleImages": + def _build_or_load_images(self, plan: PlanT) -> BottleImages: """Return the agent and sidecar image references (or artifact paths) for this plan, building fresh images when the policy requires it.""" @abstractmethod - def _launch_impl(self, plan: PlanT, images: "BottleImages") -> AbstractContextManager[Bottle]: + def _launch_impl(self, plan: PlanT, images: BottleImages) -> AbstractContextManager[Bottle]: """Bring up the bottle using pre-resolved images; yield a handle; tear down on exit.""" def provision(self, plan: PlanT, bottle: "Bottle") -> str | None: -- 2.52.0 From d8d14f889cff903c7720667692ffe8f7c74f721a Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 10 Jul 2026 20:39:45 +0000 Subject: [PATCH 9/9] fix(lint): remove unused BottleImages imports from two test files pyright strict reportUnusedImport flagged BottleImages in test_docker_launch_committed_image.py and test_macos_container_launch.py; neither file references the type by name (they only use the returned value's attributes). --- bot_bottle/backend/smolmachines/backend.py | 11 +++++++---- bot_bottle/backend/smolmachines/launch.py | 15 ++++++++++----- tests/unit/test_docker_launch_committed_image.py | 2 +- tests/unit/test_macos_container_launch.py | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/bot_bottle/backend/smolmachines/backend.py b/bot_bottle/backend/smolmachines/backend.py index f24be61..a85905d 100644 --- a/bot_bottle/backend/smolmachines/backend.py +++ b/bot_bottle/backend/smolmachines/backend.py @@ -19,7 +19,7 @@ from ...env import ResolvedEnv from ...git_gate import GitGatePlan from ...supervise import SupervisePlan from ...manifest import Manifest -from .. import ActiveAgent, BottleBackend, BottleSpec +from .. import ActiveAgent, BottleBackend, BottleImages, BottleSpec from . import cleanup as _cleanup from . import enumerate as _enumerate from . import launch as _launch @@ -77,11 +77,14 @@ class SmolmachinesBottleBackend( stage_dir=stage_dir, ) + def _build_or_load_images(self, plan: SmolmachinesBottlePlan) -> BottleImages: + return _launch.build_or_load_images(plan) + @contextmanager - def launch( - self, plan: SmolmachinesBottlePlan + def _launch_impl( + self, plan: SmolmachinesBottlePlan, images: BottleImages ) -> Generator[SmolmachinesBottle, None, None]: - with _launch.launch(plan, provision=self.provision) as bottle: + with _launch.launch(plan, images, provision=self.provision) as bottle: yield bottle def supervise_mcp_url(self, plan: SmolmachinesBottlePlan) -> str: diff --git a/bot_bottle/backend/smolmachines/launch.py b/bot_bottle/backend/smolmachines/launch.py index 56e5929..ebff3e9 100644 --- a/bot_bottle/backend/smolmachines/launch.py +++ b/bot_bottle/backend/smolmachines/launch.py @@ -52,6 +52,7 @@ from ...bottle_state import ( git_gate_state_dir, read_committed_image, ) +from .. import BottleImages from . import loopback_alias as _loopback from . import sidecar_bundle as _bundle from . import smolvm as _smolvm @@ -79,13 +80,20 @@ _GIT_HTTP_PORT = 9420 _SUPERVISE_PORT = SUPERVISE_PORT +def build_or_load_images(plan: SmolmachinesBottlePlan) -> BottleImages: + """Build or resolve the agent smolmachine artifact and sidecar bundle image.""" + _bundle.ensure_bundle_image() + return BottleImages(agent=_agent_from_path(plan), sidecar=_bundle.SIDECAR_BUNDLE_IMAGE) + + @contextmanager def launch( plan: SmolmachinesBottlePlan, + images: BottleImages, *, provision: Callable[[SmolmachinesBottlePlan, "SmolmachinesBottle"], str | None], ) -> Generator[SmolmachinesBottle, None, None]: - """Build + run the bottle and yield a handle; tear everything + """Run the bottle from pre-built images and yield a handle; tear everything down on exit. Errors during bringup unwind any partial state via the ExitStack.""" stack = ExitStack() @@ -96,9 +104,7 @@ def launch( plan = _start_bundle(plan, network, proxy_host, stack) plan = _discover_urls(plan, proxy_host) - agent_from_path = _agent_from_path(plan) - - _launch_vm(plan, agent_from_path, proxy_host, stack) + _launch_vm(plan, Path(images.agent), proxy_host, stack) _init_vm(plan) bottle = SmolmachinesBottle( @@ -184,7 +190,6 @@ def _start_bundle( plan = _provision_git_gate_keys(plan) bundle_spec = _bundle_launch_spec(plan, network, proxy_host) token_env = _resolve_token_env(plan, dict(os.environ)) - _bundle.ensure_bundle_image(bundle_spec.image) _bundle.start_bundle(bundle_spec, env={**os.environ, **token_env}) stack.callback(_bundle.stop_bundle, plan.slug) return plan diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py index bbdfb6b..ce2742a 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -12,7 +12,7 @@ from typing import Any from unittest import mock from bot_bottle.agent_provider import AgentProvisionPlan -from bot_bottle.backend import BottleImages, BottleSpec +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 diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index a2c5978..6c7d490 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -11,7 +11,7 @@ from typing import cast from unittest.mock import patch from bot_bottle.agent_provider import AgentProvisionPlan -from bot_bottle.backend import BottleImages, BottleSpec +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 -- 2.52.0