fix: remove CLI backend assumptions and add docker fallback prompt
test / stage-firecracker-inputs (pull_request) Successful in 3s
tracker-policy-pr / check-pr (pull_request) Successful in 12s
lint / lint (push) Successful in 42s
test / integration-docker (pull_request) Successful in 33s
test / unit (pull_request) Successful in 37s
test / build-infra (pull_request) Successful in 3m47s
test / integration-firecracker (pull_request) Successful in 1m39s
test / coverage (pull_request) Failing after 1m27s
test / publish-infra (pull_request) Has been skipped
test / stage-firecracker-inputs (pull_request) Successful in 3s
tracker-policy-pr / check-pr (pull_request) Successful in 12s
lint / lint (push) Successful in 42s
test / integration-docker (pull_request) Successful in 33s
test / unit (pull_request) Successful in 37s
test / build-infra (pull_request) Successful in 3m47s
test / integration-firecracker (pull_request) Successful in 1m39s
test / coverage (pull_request) Failing after 1m27s
test / publish-infra (pull_request) Has been skipped
- Remove hardcoded --backend=macos-container flag reference in firecracker/util.py require_firecracker() error message - Remove --backend flag from cli.py start; backend selection now driven exclusively by BOT_BOTTLE_BACKEND env var or auto-selection - Skip unavailable backends in cli.py cleanup (fixes crash on Linux when macos-container.prepare_cleanup calls require_container()) - Add two-tier auto-selection: VM backend first (macos-container on macOS, firecracker on Linux+KVM); fall back to docker with a security warning and interactive i/d/q prompt; exit if docker also unavailable and print VM install instructions Closes #344
This commit is contained in:
@@ -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 ..agent_provider import AgentProvisionPlan, get_provider, build_agent_provision_plan
|
||||||
from ..egress import EgressPlan
|
from ..egress import EgressPlan
|
||||||
from ..git_gate import GitGatePlan
|
from ..git_gate import GitGatePlan
|
||||||
from ..log import die, info
|
from ..log import die, info, warn
|
||||||
from ..manifest import Manifest, ManifestIndex
|
from ..manifest import Manifest, ManifestIndex
|
||||||
from ..supervise import SupervisePlan
|
from ..supervise import SupervisePlan
|
||||||
from ..util import expand_tilde
|
from ..util import expand_tilde
|
||||||
@@ -652,15 +652,15 @@ def get_bottle_backend(
|
|||||||
"""Resolve the bottle backend.
|
"""Resolve the bottle backend.
|
||||||
|
|
||||||
`name` precedence:
|
`name` precedence:
|
||||||
1. explicit arg (CLI `--backend=<name>` passes through here)
|
1. explicit arg (e.g. resume passes the recorded backend name)
|
||||||
2. BOT_BOTTLE_BACKEND env var
|
2. BOT_BOTTLE_BACKEND env var
|
||||||
3. `macos-container` on compatible macOS hosts
|
3. auto-selection: VM backend first, docker fallback with prompt
|
||||||
4. `firecracker` on KVM-capable Linux hosts
|
|
||||||
5. default `docker`
|
|
||||||
|
|
||||||
Dies with a pointer at the known backends if the chosen name
|
Dies with a pointer at the known backends if the chosen name
|
||||||
isn't implemented."""
|
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()
|
backends = _get_backends()
|
||||||
if resolved not in backends:
|
if resolved not in backends:
|
||||||
known = ", ".join(sorted(backends))
|
known = ", ".join(sorted(backends))
|
||||||
@@ -668,7 +668,42 @@ def get_bottle_backend(
|
|||||||
return backends[resolved]
|
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"):
|
if has_backend("macos-container"):
|
||||||
return "macos-container"
|
return "macos-container"
|
||||||
# A KVM-capable Linux host defaults to firecracker even when the
|
# A KVM-capable Linux host defaults to firecracker even when the
|
||||||
@@ -678,7 +713,32 @@ def _default_backend_name() -> str:
|
|||||||
from .firecracker import FirecrackerBottleBackend
|
from .firecracker import FirecrackerBottleBackend
|
||||||
if FirecrackerBottleBackend.is_host_capable():
|
if FirecrackerBottleBackend.is_host_capable():
|
||||||
return "firecracker"
|
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, ...]:
|
def known_backend_names() -> tuple[str, ...]:
|
||||||
|
|||||||
@@ -88,7 +88,7 @@ def require_firecracker() -> None:
|
|||||||
booting a VM without it."""
|
booting a VM without it."""
|
||||||
if not is_linux():
|
if not is_linux():
|
||||||
die("firecracker backend is only supported on Linux (KVM). "
|
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:
|
if shutil.which("firecracker") is None:
|
||||||
info("Firecracker is required but was not found on PATH.")
|
info("Firecracker is required but was not found on PATH.")
|
||||||
info("Install: https://github.com/firecracker-microvm/firecracker/releases")
|
info("Install: https://github.com/firecracker-microvm/firecracker/releases")
|
||||||
|
|||||||
@@ -21,16 +21,20 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import sys
|
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 ..log import info
|
||||||
from ._common import read_tty_line
|
from ._common import read_tty_line
|
||||||
|
|
||||||
|
|
||||||
def cmd_cleanup(_argv: list[str]) -> int:
|
def cmd_cleanup(_argv: list[str]) -> int:
|
||||||
# Order: stable backend iteration so the y/N output is
|
# 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 = [
|
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]
|
prepared = [(name, b, b.prepare_cleanup()) for name, b in plans]
|
||||||
|
|
||||||
|
|||||||
+1
-15
@@ -27,7 +27,6 @@ from ..backend import (
|
|||||||
BottleSpec,
|
BottleSpec,
|
||||||
enumerate_active_agents,
|
enumerate_active_agents,
|
||||||
get_bottle_backend,
|
get_bottle_backend,
|
||||||
known_backend_names,
|
|
||||||
)
|
)
|
||||||
from ..backend.docker import util as docker_mod
|
from ..backend.docker import util as docker_mod
|
||||||
from ..backend.docker.bottle_plan import DockerBottlePlan
|
from ..backend.docker.bottle_plan import DockerBottlePlan
|
||||||
@@ -57,15 +56,6 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
"into a cached layer."
|
"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(
|
parser.add_argument(
|
||||||
"--headless",
|
"--headless",
|
||||||
action="store_true",
|
action="store_true",
|
||||||
@@ -115,11 +105,10 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
os.environ["BOT_BOTTLE_NO_CACHE"] = "1"
|
os.environ["BOT_BOTTLE_NO_CACHE"] = "1"
|
||||||
|
|
||||||
manifest = ManifestIndex.resolve(USER_CWD)
|
manifest = ManifestIndex.resolve(USER_CWD)
|
||||||
backend_name: str | None = args.backend
|
|
||||||
|
|
||||||
if args.headless:
|
if args.headless:
|
||||||
return _start_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
|
agent_name: str | None = args.name
|
||||||
@@ -170,7 +159,6 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
return _launch_bottle(
|
return _launch_bottle(
|
||||||
spec,
|
spec,
|
||||||
dry_run=dry_run,
|
dry_run=dry_run,
|
||||||
backend_name=backend_name,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -182,7 +170,6 @@ def _start_headless(
|
|||||||
args: argparse.Namespace,
|
args: argparse.Namespace,
|
||||||
*,
|
*,
|
||||||
dry_run: bool,
|
dry_run: bool,
|
||||||
backend_name: str | None,
|
|
||||||
) -> int:
|
) -> int:
|
||||||
"""Non-interactive launch path for orchestrators / CI / webhooks.
|
"""Non-interactive launch path for orchestrators / CI / webhooks.
|
||||||
|
|
||||||
@@ -230,7 +217,6 @@ def _start_headless(
|
|||||||
return _launch_bottle(
|
return _launch_bottle(
|
||||||
spec,
|
spec,
|
||||||
dry_run=dry_run,
|
dry_run=dry_run,
|
||||||
backend_name=backend_name,
|
|
||||||
assume_yes=True,
|
assume_yes=True,
|
||||||
headless_prompt_text=prompt,
|
headless_prompt_text=prompt,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -57,14 +57,16 @@ class TestGetBottleBackend(unittest.TestCase):
|
|||||||
return self._available
|
return self._available
|
||||||
|
|
||||||
# No macOS container and the host can't run firecracker (no
|
# 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), \
|
with patch.dict(os.environ, {}, clear=True), \
|
||||||
patch.object(backend_mod.FirecrackerBottleBackend,
|
patch.object(backend_mod.FirecrackerBottleBackend,
|
||||||
"is_host_capable", classmethod(lambda cls: False)), \
|
"is_host_capable", classmethod(lambda cls: False)), \
|
||||||
patch.object(backend_mod, "_backends", {
|
patch.object(backend_mod, "_backends", {
|
||||||
"macos-container": _FakeBackend("macos-container", False),
|
"macos-container": _FakeBackend("macos-container", False),
|
||||||
"docker": _FakeBackend("docker", True),
|
"docker": _FakeBackend("docker", True),
|
||||||
}):
|
}), \
|
||||||
|
patch.object(backend_mod, "_read_tty_line", return_value="d"):
|
||||||
b = get_bottle_backend()
|
b = get_bottle_backend()
|
||||||
self.assertEqual("docker", b.name)
|
self.assertEqual("docker", b.name)
|
||||||
|
|
||||||
@@ -96,6 +98,49 @@ class TestGetBottleBackend(unittest.TestCase):
|
|||||||
with self.assertRaises(SystemExit):
|
with self.assertRaises(SystemExit):
|
||||||
get_bottle_backend("nonexistent")
|
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):
|
class TestKnownBackendNames(unittest.TestCase):
|
||||||
def test_returns_backends_sorted(self):
|
def test_returns_backends_sorted(self):
|
||||||
|
|||||||
@@ -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`,
|
Asserts cmd_cleanup queries each available backend's `prepare_cleanup`,
|
||||||
combines the y/N output, and runs each backend's `cleanup` when
|
combines the y/N output, and runs each backend's `cleanup` when the
|
||||||
the operator confirms. Mocks the backends and stdin."""
|
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
|
from __future__ import annotations
|
||||||
|
|
||||||
@@ -21,7 +23,7 @@ def _make_backend(empty: bool = True):
|
|||||||
|
|
||||||
|
|
||||||
class TestCmdCleanup(unittest.TestCase):
|
class TestCmdCleanup(unittest.TestCase):
|
||||||
def test_iterates_every_backend(self):
|
def test_iterates_every_available_backend(self):
|
||||||
docker, docker_plan = _make_backend(empty=False)
|
docker, docker_plan = _make_backend(empty=False)
|
||||||
fc, fc_plan = _make_backend(empty=False)
|
fc, fc_plan = _make_backend(empty=False)
|
||||||
backends_by_name = {"docker": docker, "firecracker": fc}
|
backends_by_name = {"docker": docker, "firecracker": fc}
|
||||||
@@ -32,6 +34,8 @@ class TestCmdCleanup(unittest.TestCase):
|
|||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "get_bottle_backend",
|
cmd, "get_bottle_backend",
|
||||||
side_effect=lambda name: backends_by_name[name], # type: ignore
|
side_effect=lambda name: backends_by_name[name], # type: ignore
|
||||||
|
), patch.object(
|
||||||
|
cmd, "has_backend", return_value=True,
|
||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "_prompt_yes", return_value=True,
|
cmd, "_prompt_yes", return_value=True,
|
||||||
):
|
):
|
||||||
@@ -42,6 +46,32 @@ class TestCmdCleanup(unittest.TestCase):
|
|||||||
docker.cleanup.assert_called_once_with(docker_plan)
|
docker.cleanup.assert_called_once_with(docker_plan)
|
||||||
fc.cleanup.assert_called_once_with(fc_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):
|
def test_short_circuits_when_all_empty(self):
|
||||||
docker, _ = _make_backend(empty=True)
|
docker, _ = _make_backend(empty=True)
|
||||||
fc, _ = _make_backend(empty=True)
|
fc, _ = _make_backend(empty=True)
|
||||||
@@ -53,6 +83,8 @@ class TestCmdCleanup(unittest.TestCase):
|
|||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "get_bottle_backend",
|
cmd, "get_bottle_backend",
|
||||||
side_effect=lambda name: backends_by_name[name], # type: ignore
|
side_effect=lambda name: backends_by_name[name], # type: ignore
|
||||||
|
), patch.object(
|
||||||
|
cmd, "has_backend", return_value=True,
|
||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "_prompt_yes",
|
cmd, "_prompt_yes",
|
||||||
) as prompt:
|
) as prompt:
|
||||||
@@ -72,6 +104,8 @@ class TestCmdCleanup(unittest.TestCase):
|
|||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "get_bottle_backend",
|
cmd, "get_bottle_backend",
|
||||||
side_effect=lambda name: backends_by_name[name], # type: ignore
|
side_effect=lambda name: backends_by_name[name], # type: ignore
|
||||||
|
), patch.object(
|
||||||
|
cmd, "has_backend", return_value=True,
|
||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "_prompt_yes", return_value=False,
|
cmd, "_prompt_yes", return_value=False,
|
||||||
):
|
):
|
||||||
@@ -92,6 +126,8 @@ class TestCmdCleanup(unittest.TestCase):
|
|||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "get_bottle_backend",
|
cmd, "get_bottle_backend",
|
||||||
side_effect=lambda name: backends_by_name[name], # type: ignore
|
side_effect=lambda name: backends_by_name[name], # type: ignore
|
||||||
|
), patch.object(
|
||||||
|
cmd, "has_backend", return_value=True,
|
||||||
), patch.object(
|
), patch.object(
|
||||||
cmd, "_prompt_yes", return_value=True,
|
cmd, "_prompt_yes", return_value=True,
|
||||||
):
|
):
|
||||||
|
|||||||
@@ -1,61 +1,29 @@
|
|||||||
"""Unit: `cli.py start --backend=<name>` 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 `--backend` flag has been removed from `cli.py start`; backend
|
||||||
the fallback, and that the choices are pulled from the backend
|
selection is driven by BOT_BOTTLE_BACKEND or auto-selection only.
|
||||||
registry (so adding a backend lights up in argparse without code
|
`get_bottle_backend` still accepts an explicit name so that `resume`
|
||||||
edits)."""
|
(which records the backend from a prior session) and the `backend`
|
||||||
|
sub-command can pass one directly."""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import argparse
|
|
||||||
import os
|
import os
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
|
||||||
from bot_bottle.backend import known_backend_names
|
class TestBackendResolutionPriority(unittest.TestCase):
|
||||||
|
def test_explicit_name_overrides_env_var(self):
|
||||||
|
|
||||||
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.
|
|
||||||
from bot_bottle.backend import get_bottle_backend
|
from bot_bottle.backend import get_bottle_backend
|
||||||
with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}):
|
with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}):
|
||||||
self.assertEqual("docker", get_bottle_backend("docker").name)
|
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__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
@@ -175,14 +175,6 @@ class TestCmdStartHeadless(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertEqual("researcher-2", self._spec().label)
|
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):
|
class TestPrepareWithPreflight(unittest.TestCase):
|
||||||
|
|||||||
@@ -82,7 +82,7 @@ class TestCmdStartSelector(unittest.TestCase):
|
|||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
|
|
||||||
def test_explicit_agent_skips_agent_picker(self):
|
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.assertEqual(0, rc)
|
||||||
self._agent_picker_mock.assert_not_called()
|
self._agent_picker_mock.assert_not_called()
|
||||||
self._bottle_picker_mock.assert_called_once()
|
self._bottle_picker_mock.assert_called_once()
|
||||||
@@ -100,7 +100,7 @@ class TestCmdStartSelector(unittest.TestCase):
|
|||||||
|
|
||||||
def test_agent_absent_shows_agent_picker(self):
|
def test_agent_absent_shows_agent_picker(self):
|
||||||
self._agent_picker_mock.return_value = "researcher"
|
self._agent_picker_mock.return_value = "researcher"
|
||||||
rc = start_mod.cmd_start(["--backend=docker"])
|
rc = start_mod.cmd_start([])
|
||||||
self.assertEqual(0, rc)
|
self.assertEqual(0, rc)
|
||||||
self._agent_picker_mock.assert_called_once()
|
self._agent_picker_mock.assert_called_once()
|
||||||
call_kwargs = self._agent_picker_mock.call_args
|
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):
|
def test_agent_picker_cancel_skips_bottle_picker(self):
|
||||||
self._agent_picker_mock.return_value = None
|
self._agent_picker_mock.return_value = None
|
||||||
rc = start_mod.cmd_start(["--backend=docker"])
|
rc = start_mod.cmd_start([])
|
||||||
self.assertEqual(0, rc)
|
self.assertEqual(0, rc)
|
||||||
self._bottle_picker_mock.assert_not_called()
|
self._bottle_picker_mock.assert_not_called()
|
||||||
self._launch_mock.assert_not_called()
|
self._launch_mock.assert_not_called()
|
||||||
@@ -168,16 +168,6 @@ class TestCmdStartSelector(unittest.TestCase):
|
|||||||
# Backend wiring
|
# 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):
|
def test_bot_bottle_backend_env_skips_backend_picker(self):
|
||||||
os.environ["BOT_BOTTLE_BACKEND"] = "docker"
|
os.environ["BOT_BOTTLE_BACKEND"] = "docker"
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user