diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index bd83a1e..e1f862e 100644 --- a/bot_bottle/backend/__init__.py +++ b/bot_bottle/backend/__init__.py @@ -45,7 +45,7 @@ from typing import TYPE_CHECKING, Any, Generic, Sequence, TypeVar from ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provision_plan from ..egress import EgressPlan from ..git_gate import GitGatePlan -from ..log import die, info +from ..log import die, info, warn from ..manifest import Manifest, ManifestIndex from ..supervise import SupervisePlan from ..util import expand_tilde @@ -652,15 +652,15 @@ def get_bottle_backend( """Resolve the bottle backend. `name` precedence: - 1. explicit arg (CLI `--backend=` passes through here) + 1. explicit arg (e.g. resume passes the recorded backend name) 2. BOT_BOTTLE_BACKEND env var - 3. `macos-container` on compatible macOS hosts - 4. `firecracker` on KVM-capable Linux hosts - 5. default `docker` + 3. auto-selection: VM backend first, docker fallback with prompt Dies with a pointer at the known backends if the chosen name isn't implemented.""" - resolved = name or os.environ.get("BOT_BOTTLE_BACKEND") or _default_backend_name() + resolved = name or os.environ.get("BOT_BOTTLE_BACKEND") + if resolved is None: + resolved = _auto_select_backend() backends = _get_backends() if resolved not in backends: known = ", ".join(sorted(backends)) @@ -668,7 +668,42 @@ def get_bottle_backend( return backends[resolved] -def _default_backend_name() -> str: +def _read_tty_line() -> str: + """Read a line from /dev/tty, falling back to stdin.""" + try: + with open("/dev/tty", "r", encoding="utf-8") as tty: + return tty.readline().rstrip("\n") + except OSError: + return sys.stdin.readline().rstrip("\n") + + +def _platform_vm_suggestion() -> str: + """Platform-appropriate VM backend name for install suggestions.""" + return "macos-container" if sys.platform == "darwin" else "firecracker" + + +def _print_vm_install_instructions() -> None: + """Print platform-appropriate VM backend install instructions to stderr.""" + vm = _platform_vm_suggestion() + if vm == "macos-container": + info("Install Apple Container: https://github.com/apple/container/releases") + info("Then start the service: container system start") + else: + info("Install Firecracker: https://github.com/firecracker-microvm/firecracker/releases") + info("Configure the host: ./cli.py backend setup") + + +def _auto_select_backend() -> str: + """Tier-1 / tier-2 backend auto-selection. + + Tier 1: VM backend — macos-container on macOS when Apple Container is + installed; firecracker on KVM-capable Linux even before the binary is + present (its preflight prints an install pointer). + + Tier 2: docker, with a security warning and an interactive prompt. + When docker is also absent, prints VM install instructions and exits. + """ + # --- Tier 1: VM backend ----------------------------------------- if has_backend("macos-container"): return "macos-container" # A KVM-capable Linux host defaults to firecracker even when the @@ -678,7 +713,32 @@ def _default_backend_name() -> str: from .firecracker import FirecrackerBottleBackend if FirecrackerBottleBackend.is_host_capable(): return "firecracker" - return "docker" + + # --- Tier 2: docker fallback ------------------------------------ + if not has_backend("docker"): + info("No backend available on this host.") + _print_vm_install_instructions() + die("no backend available; install a VM backend and re-run") + + vm = _platform_vm_suggestion() + warn( + "docker is less secure than VM backends — " + "containers share the host kernel." + ) + sys.stderr.write( + f"bot-bottle: For better isolation, install the {vm!r} backend.\n" + f" [i] show {vm} install instructions and exit\n" + " [d] use docker anyway\n" + " [q] quit\n" + "bot-bottle: choice [i/d/q]: " + ) + sys.stderr.flush() + reply = _read_tty_line().strip().lower() + if reply == "d": + return "docker" + if reply == "i": + _print_vm_install_instructions() + die("not proceeding with docker; install a VM backend or set BOT_BOTTLE_BACKEND=docker") def known_backend_names() -> tuple[str, ...]: diff --git a/bot_bottle/backend/firecracker/util.py b/bot_bottle/backend/firecracker/util.py index 5fd77c1..cf30a18 100644 --- a/bot_bottle/backend/firecracker/util.py +++ b/bot_bottle/backend/firecracker/util.py @@ -88,7 +88,7 @@ def require_firecracker() -> None: booting a VM without it.""" if not is_linux(): die("firecracker backend is only supported on Linux (KVM). " - "On macOS use --backend=macos-container.") + "On macOS use the macos-container backend.") if shutil.which("firecracker") is None: info("Firecracker is required but was not found on PATH.") info("Install: https://github.com/firecracker-microvm/firecracker/releases") diff --git a/bot_bottle/cli/cleanup.py b/bot_bottle/cli/cleanup.py index bbed373..34d6de9 100644 --- a/bot_bottle/cli/cleanup.py +++ b/bot_bottle/cli/cleanup.py @@ -21,16 +21,20 @@ from __future__ import annotations import sys -from ..backend import get_bottle_backend, known_backend_names +from ..backend import get_bottle_backend, has_backend, known_backend_names from ..log import info from ._common import read_tty_line def cmd_cleanup(_argv: list[str]) -> int: # Order: stable backend iteration so the y/N output is - # deterministic across runs. + # deterministic across runs. Skip backends whose runtime + # isn't available on this host so e.g. macos-container + # doesn't error on Linux. plans = [ - (name, get_bottle_backend(name)) for name in known_backend_names() + (name, get_bottle_backend(name)) + for name in known_backend_names() + if has_backend(name) ] prepared = [(name, b, b.prepare_cleanup()) for name, b in plans] diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index 4101804..c2f57c8 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -27,7 +27,6 @@ from ..backend import ( BottleSpec, enumerate_active_agents, get_bottle_backend, - known_backend_names, ) from ..backend.docker import util as docker_mod from ..backend.docker.bottle_plan import DockerBottlePlan @@ -57,15 +56,6 @@ def cmd_start(argv: list[str]) -> int: "into a cached layer." ), ) - parser.add_argument( - "--backend", - choices=known_backend_names(), - default=None, - help=( - "backend to launch the bottle on (default: $BOT_BOTTLE_BACKEND " - "or host auto-selection). Overrides the env var when set." - ), - ) parser.add_argument( "--headless", action="store_true", @@ -115,11 +105,10 @@ def cmd_start(argv: list[str]) -> int: os.environ["BOT_BOTTLE_NO_CACHE"] = "1" manifest = ManifestIndex.resolve(USER_CWD) - backend_name: str | None = args.backend if args.headless: return _start_headless( - manifest, args, dry_run=dry_run, backend_name=backend_name + manifest, args, dry_run=dry_run ) agent_name: str | None = args.name @@ -170,7 +159,6 @@ def cmd_start(argv: list[str]) -> int: return _launch_bottle( spec, dry_run=dry_run, - backend_name=backend_name, ) @@ -182,7 +170,6 @@ def _start_headless( args: argparse.Namespace, *, dry_run: bool, - backend_name: str | None, ) -> int: """Non-interactive launch path for orchestrators / CI / webhooks. @@ -230,7 +217,6 @@ def _start_headless( return _launch_bottle( spec, dry_run=dry_run, - backend_name=backend_name, assume_yes=True, headless_prompt_text=prompt, ) diff --git a/tests/unit/test_backend_selection.py b/tests/unit/test_backend_selection.py index 658ecee..5f52979 100644 --- a/tests/unit/test_backend_selection.py +++ b/tests/unit/test_backend_selection.py @@ -57,14 +57,16 @@ class TestGetBottleBackend(unittest.TestCase): return self._available # No macOS container and the host can't run firecracker (no - # KVM / not Linux) → docker is the last resort. + # KVM / not Linux) → docker fallback with a prompt. Simulate + # the user picking "d" (use docker anyway). with patch.dict(os.environ, {}, clear=True), \ patch.object(backend_mod.FirecrackerBottleBackend, "is_host_capable", classmethod(lambda cls: False)), \ patch.object(backend_mod, "_backends", { "macos-container": _FakeBackend("macos-container", False), "docker": _FakeBackend("docker", True), - }): + }), \ + patch.object(backend_mod, "_read_tty_line", return_value="d"): b = get_bottle_backend() self.assertEqual("docker", b.name) @@ -96,6 +98,49 @@ class TestGetBottleBackend(unittest.TestCase): with self.assertRaises(SystemExit): get_bottle_backend("nonexistent") + def test_no_backend_available_dies(self): + # No VM and no docker → print install instructions and die. + class _FakeBackend: + def __init__(self, name: str, available: bool) -> None: + self.name = name + self._available = available + + def is_available(self) -> bool: + return self._available + + with patch.dict(os.environ, {}, clear=True), \ + patch.object(backend_mod.FirecrackerBottleBackend, + "is_host_capable", classmethod(lambda cls: False)), \ + patch.object(backend_mod, "_backends", { + "macos-container": _FakeBackend("macos-container", False), + "docker": _FakeBackend("docker", False), + }), \ + patch.object(backend_mod, "die", side_effect=SystemExit("die")): + with self.assertRaises(SystemExit): + get_bottle_backend() + + def test_docker_fallback_user_quits(self): + # VM unavailable, docker available, user picks "q" → die. + class _FakeBackend: + def __init__(self, name: str, available: bool) -> None: + self.name = name + self._available = available + + def is_available(self) -> bool: + return self._available + + with patch.dict(os.environ, {}, clear=True), \ + patch.object(backend_mod.FirecrackerBottleBackend, + "is_host_capable", classmethod(lambda cls: False)), \ + patch.object(backend_mod, "_backends", { + "macos-container": _FakeBackend("macos-container", False), + "docker": _FakeBackend("docker", True), + }), \ + patch.object(backend_mod, "_read_tty_line", return_value="q"), \ + patch.object(backend_mod, "die", side_effect=SystemExit("die")): + with self.assertRaises(SystemExit): + get_bottle_backend() + class TestKnownBackendNames(unittest.TestCase): def test_returns_backends_sorted(self): diff --git a/tests/unit/test_cli_cleanup_cross_backend.py b/tests/unit/test_cli_cleanup_cross_backend.py index 95b869a..e1f4be8 100644 --- a/tests/unit/test_cli_cleanup_cross_backend.py +++ b/tests/unit/test_cli_cleanup_cross_backend.py @@ -1,8 +1,10 @@ -"""Unit: `cli.py cleanup` walks every backend (issue follow-up). +"""Unit: `cli.py cleanup` walks every available backend. -Asserts cmd_cleanup queries each backend's `prepare_cleanup`, -combines the y/N output, and runs each backend's `cleanup` when -the operator confirms. Mocks the backends and stdin.""" +Asserts cmd_cleanup queries each available backend's `prepare_cleanup`, +combines the y/N output, and runs each backend's `cleanup` when the +operator confirms. Unavailable backends (e.g. macos-container on Linux) +are skipped so that `cleanup` never errors on a platform where a backend +is not installed. Mocks the backends and stdin.""" from __future__ import annotations @@ -21,7 +23,7 @@ def _make_backend(empty: bool = True): class TestCmdCleanup(unittest.TestCase): - def test_iterates_every_backend(self): + def test_iterates_every_available_backend(self): docker, docker_plan = _make_backend(empty=False) fc, fc_plan = _make_backend(empty=False) backends_by_name = {"docker": docker, "firecracker": fc} @@ -32,6 +34,8 @@ class TestCmdCleanup(unittest.TestCase): ), patch.object( cmd, "get_bottle_backend", side_effect=lambda name: backends_by_name[name], # type: ignore + ), patch.object( + cmd, "has_backend", return_value=True, ), patch.object( cmd, "_prompt_yes", return_value=True, ): @@ -42,6 +46,32 @@ class TestCmdCleanup(unittest.TestCase): docker.cleanup.assert_called_once_with(docker_plan) fc.cleanup.assert_called_once_with(fc_plan) + def test_skips_unavailable_backends(self): + # macos-container is not available on Linux — must be silently skipped. + docker, docker_plan = _make_backend(empty=False) + macos = MagicMock() + backends_by_name = {"docker": docker} + + def _has(name: str) -> bool: + return name == "docker" + + with patch.object( + cmd, "known_backend_names", + return_value=("docker", "macos-container"), + ), patch.object( + cmd, "get_bottle_backend", + side_effect=lambda name: backends_by_name[name], # type: ignore + ), patch.object( + cmd, "has_backend", side_effect=_has, + ), patch.object( + cmd, "_prompt_yes", return_value=True, + ): + self.assertEqual(0, cmd.cmd_cleanup([])) + + docker.prepare_cleanup.assert_called_once() + docker.cleanup.assert_called_once_with(docker_plan) + macos.prepare_cleanup.assert_not_called() + def test_short_circuits_when_all_empty(self): docker, _ = _make_backend(empty=True) fc, _ = _make_backend(empty=True) @@ -53,6 +83,8 @@ class TestCmdCleanup(unittest.TestCase): ), patch.object( cmd, "get_bottle_backend", side_effect=lambda name: backends_by_name[name], # type: ignore + ), patch.object( + cmd, "has_backend", return_value=True, ), patch.object( cmd, "_prompt_yes", ) as prompt: @@ -72,6 +104,8 @@ class TestCmdCleanup(unittest.TestCase): ), patch.object( cmd, "get_bottle_backend", side_effect=lambda name: backends_by_name[name], # type: ignore + ), patch.object( + cmd, "has_backend", return_value=True, ), patch.object( cmd, "_prompt_yes", return_value=False, ): @@ -92,6 +126,8 @@ class TestCmdCleanup(unittest.TestCase): ), patch.object( cmd, "get_bottle_backend", side_effect=lambda name: backends_by_name[name], # type: ignore + ), patch.object( + cmd, "has_backend", return_value=True, ), patch.object( cmd, "_prompt_yes", return_value=True, ): diff --git a/tests/unit/test_cli_start_backend_flag.py b/tests/unit/test_cli_start_backend_flag.py index ed6562a..b890271 100644 --- a/tests/unit/test_cli_start_backend_flag.py +++ b/tests/unit/test_cli_start_backend_flag.py @@ -1,61 +1,29 @@ -"""Unit: `cli.py start --backend=` flag (issue #77). +"""Unit: backend resolution priority — explicit name > env var. -Asserts that the flag wins over the env var, that the env var is -the fallback, and that the choices are pulled from the backend -registry (so adding a backend lights up in argparse without code -edits).""" +The `--backend` flag has been removed from `cli.py start`; backend +selection is driven by BOT_BOTTLE_BACKEND or auto-selection only. +`get_bottle_backend` still accepts an explicit name so that `resume` +(which records the backend from a prior session) and the `backend` +sub-command can pass one directly.""" from __future__ import annotations -import argparse import os import unittest from unittest.mock import patch -from bot_bottle.backend import known_backend_names - - -class TestStartBackendFlag(unittest.TestCase): - """The flag is wired by `cmd_start`'s argparse and threaded - through `prepare_with_preflight(backend_name=...)`. Rather than - drive the whole start flow (which builds containers), we test - the argparse shape and the resolution function separately.""" - - def _build_parser(self): - # Mirror the parser definition from `cmd_start` so this - # test doesn't have to invoke the full command. - parser = argparse.ArgumentParser(prog="cb start") - parser.add_argument( - "--backend", - choices=known_backend_names(), - default=None, - ) - parser.add_argument("name") - return parser - - def test_flag_recognized(self): - args = self._build_parser().parse_args(["--backend=firecracker", "researcher"]) - self.assertEqual("firecracker", args.backend) - self.assertEqual("researcher", args.name) - - def test_flag_default_none_means_env_or_default_backend(self): - args = self._build_parser().parse_args(["researcher"]) - self.assertIsNone(args.backend) - - def test_invalid_backend_rejected_by_argparse(self): - parser = self._build_parser() - with self.assertRaises(SystemExit): - parser.parse_args(["--backend=garbage", "researcher"]) - - def test_resolution_priority_explicit_over_env(self): - # Independent assertion that get_bottle_backend (where - # `--backend` ultimately threads to) prefers the explicit - # name over BOT_BOTTLE_BACKEND. +class TestBackendResolutionPriority(unittest.TestCase): + def test_explicit_name_overrides_env_var(self): from bot_bottle.backend import get_bottle_backend with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}): self.assertEqual("docker", get_bottle_backend("docker").name) + def test_env_var_used_when_no_explicit_name(self): + from bot_bottle.backend import get_bottle_backend + with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}): + self.assertEqual("firecracker", get_bottle_backend().name) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_cli_start_headless.py b/tests/unit/test_cli_start_headless.py index e4e6717..4cd69ec 100644 --- a/tests/unit/test_cli_start_headless.py +++ b/tests/unit/test_cli_start_headless.py @@ -175,14 +175,6 @@ class TestCmdStartHeadless(unittest.TestCase): ) self.assertEqual("researcher-2", self._spec().label) - # -- backend wiring ------------------------------------------------ - - def test_backend_flag_forwarded(self): - start_mod.cmd_start( - ["--headless", "--backend=docker", "researcher", "--bottle", "claude", - "--prompt", "Do it"] - ) - self.assertEqual("docker", self._launch_mock.call_args[1]["backend_name"]) class TestPrepareWithPreflight(unittest.TestCase): diff --git a/tests/unit/test_cli_start_selector.py b/tests/unit/test_cli_start_selector.py index ba1a6b3..7667123 100644 --- a/tests/unit/test_cli_start_selector.py +++ b/tests/unit/test_cli_start_selector.py @@ -82,7 +82,7 @@ class TestCmdStartSelector(unittest.TestCase): # ------------------------------------------------------------------ def test_explicit_agent_skips_agent_picker(self): - rc = start_mod.cmd_start(["--backend=docker", "researcher"]) + rc = start_mod.cmd_start(["researcher"]) self.assertEqual(0, rc) self._agent_picker_mock.assert_not_called() self._bottle_picker_mock.assert_called_once() @@ -100,7 +100,7 @@ class TestCmdStartSelector(unittest.TestCase): def test_agent_absent_shows_agent_picker(self): self._agent_picker_mock.return_value = "researcher" - rc = start_mod.cmd_start(["--backend=docker"]) + rc = start_mod.cmd_start([]) self.assertEqual(0, rc) self._agent_picker_mock.assert_called_once() call_kwargs = self._agent_picker_mock.call_args @@ -111,7 +111,7 @@ class TestCmdStartSelector(unittest.TestCase): def test_agent_picker_cancel_skips_bottle_picker(self): self._agent_picker_mock.return_value = None - rc = start_mod.cmd_start(["--backend=docker"]) + rc = start_mod.cmd_start([]) self.assertEqual(0, rc) self._bottle_picker_mock.assert_not_called() self._launch_mock.assert_not_called() @@ -168,16 +168,6 @@ class TestCmdStartSelector(unittest.TestCase): # Backend wiring # ------------------------------------------------------------------ - def test_explicit_backend_forwarded(self): - start_mod.cmd_start(["--backend=docker", "researcher"]) - _, kwargs = self._launch_mock.call_args - self.assertEqual("docker", kwargs["backend_name"]) - - def test_absent_backend_uses_default(self): - start_mod.cmd_start(["researcher"]) - _, kwargs = self._launch_mock.call_args - self.assertIsNone(kwargs["backend_name"]) - def test_bot_bottle_backend_env_skips_backend_picker(self): os.environ["BOT_BOTTLE_BACKEND"] = "docker" try: