Remove CLI backend assumptions and add docker fallback prompt #434
@@ -45,7 +45,8 @@ 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 ..util import read_tty_line
|
||||||
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
|
||||||
@@ -648,19 +649,26 @@ def __getattr__(name: str) -> Any:
|
|||||||
|
|
||||||
def get_bottle_backend(
|
def get_bottle_backend(
|
||||||
name: str | None = None,
|
name: str | None = None,
|
||||||
|
*,
|
||||||
|
prompt: bool = True,
|
||||||
) -> BottleBackend[Any, Any]:
|
) -> BottleBackend[Any, Any]:
|
||||||
"""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`
|
`prompt` controls whether auto-selection may block on an interactive
|
||||||
|
[i/d/q] prompt when falling back to docker. Pass `prompt=False` in
|
||||||
|
non-interactive contexts (headless launches, CI) so the call dies
|
||||||
|
with an actionable message instead of hanging.
|
||||||
|
|
||||||
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(prompt=prompt)
|
||||||
|
didericis marked this conversation as resolved
Outdated
|
|||||||
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 +676,35 @@ def get_bottle_backend(
|
|||||||
return backends[resolved]
|
return backends[resolved]
|
||||||
|
|
||||||
|
|
||||||
def _default_backend_name() -> str:
|
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(prompt: bool = True) -> 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 `prompt=False` (headless / CI), dies with an actionable message
|
||||||
|
instead of blocking on a TTY read. 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 +714,37 @@ 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"
|
||||||
|
|
||||||
|
# --- 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."
|
||||||
|
)
|
||||||
|
if not prompt:
|
||||||
|
didericis-claude
commented
Added Added `if not prompt: die(...)` here. When `prompt=False` (headless / CI) the docker-fallback prompt is bypassed entirely — `die()` emits an actionable message and exits instead of blocking on `read_tty_line()`.
|
|||||||
|
die(
|
||||||
|
f"no VM backend available; set BOT_BOTTLE_BACKEND=docker to proceed "
|
||||||
|
f"with docker, or install the {vm!r} backend."
|
||||||
|
)
|
||||||
|
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"
|
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")
|
||||||
|
|||||||
@@ -3,18 +3,10 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import os
|
import os
|
||||||
import sys
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
from ..util import read_tty_line as read_tty_line
|
||||||
|
|
||||||
PROG = "cli.py"
|
PROG = "cli.py"
|
||||||
USER_CWD = os.getcwd()
|
USER_CWD = os.getcwd()
|
||||||
REPO_DIR = str(Path(__file__).resolve().parent.parent.parent)
|
REPO_DIR = str(Path(__file__).resolve().parent.parent.parent)
|
||||||
|
|
||||||
|
|
||||||
def read_tty_line() -> str:
|
|
||||||
"""Mirror `IFS= read -r REPLY </dev/tty`. Falls 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")
|
|
||||||
|
|||||||
@@ -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]
|
||||||
|
|
||||||
|
|||||||
+7
-18
@@ -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,
|
||||||
)
|
)
|
||||||
@@ -268,15 +254,18 @@ def prepare_with_preflight(
|
|||||||
injected callable, prompt y/N via the injected callable.
|
injected callable, prompt y/N via the injected callable.
|
||||||
|
|
||||||
`backend_name` selects which backend prepares the plan
|
`backend_name` selects which backend prepares the plan
|
||||||
(`None` → `$BOT_BOTTLE_BACKEND` → host auto-selection). The CLI
|
(`None` → `$BOT_BOTTLE_BACKEND` → host auto-selection).
|
||||||
passes whatever `--backend` resolved to.
|
|
||||||
|
When `spec.headless` is True the docker-fallback prompt is suppressed:
|
||||||
|
auto-selection dies with an actionable message rather than blocking
|
||||||
|
on a TTY read (which would hang CI, webhook dispatch, and orchestrators).
|
||||||
|
|
||||||
Returns `(plan, identity)`. `plan` is None on dry-run or
|
Returns `(plan, identity)`. `plan` is None on dry-run or
|
||||||
operator-N, but `identity` is set as soon as `backend.prepare`
|
operator-N, but `identity` is set as soon as `backend.prepare`
|
||||||
returns so callers can reap the prepare-time state dir via
|
returns so callers can reap the prepare-time state dir via
|
||||||
`settle_state(identity)` in their finally — exactly the existing
|
`settle_state(identity)` in their finally — exactly the existing
|
||||||
semantics."""
|
semantics."""
|
||||||
backend = get_bottle_backend(backend_name)
|
backend = get_bottle_backend(backend_name, prompt=not spec.headless)
|
||||||
plan = backend.prepare(spec, stage_dir=stage_dir)
|
plan = backend.prepare(spec, stage_dir=stage_dir)
|
||||||
identity = _identity_from_plan(plan)
|
identity = _identity_from_plan(plan)
|
||||||
|
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import ipaddress
|
import ipaddress
|
||||||
import os
|
import os
|
||||||
|
import sys
|
||||||
|
|
||||||
|
|
||||||
def is_ip_literal(value: str) -> bool:
|
def is_ip_literal(value: str) -> bool:
|
||||||
@@ -17,6 +18,15 @@ def is_ip_literal(value: str) -> bool:
|
|||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
|
def read_tty_line() -> str:
|
||||||
|
"""Mirror `IFS= read -r REPLY </dev/tty`. Falls 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 expand_tilde(path: str) -> str:
|
def expand_tilde(path: str) -> str:
|
||||||
"""Expand a leading '~' to $HOME. Leaves paths without a leading
|
"""Expand a leading '~' to $HOME. Leaves paths without a leading
|
||||||
tilde unchanged. Falls back to the empty string if $HOME is unset
|
tilde unchanged. Falls back to the empty string if $HOME is unset
|
||||||
|
|||||||
@@ -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,145 @@ 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()
|
||||||
|
|
||||||
|
|
||||||
|
def test_docker_fallback_non_interactive_dies(self):
|
||||||
|
# prompt=False: headless/CI contexts must not block on a TTY read.
|
||||||
|
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, "die", side_effect=SystemExit("die")):
|
||||||
|
with self.assertRaises(SystemExit):
|
||||||
|
get_bottle_backend(prompt=False)
|
||||||
|
|
||||||
|
def test_docker_fallback_user_picks_install(self):
|
||||||
|
# User picks [i] → print install instructions then 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="i"), \
|
||||||
|
patch.object(backend_mod, "_print_vm_install_instructions") as mock_inst, \
|
||||||
|
patch.object(backend_mod, "die", side_effect=SystemExit("die")):
|
||||||
|
with self.assertRaises(SystemExit):
|
||||||
|
get_bottle_backend()
|
||||||
|
mock_inst.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
class TestReadTtyLine(unittest.TestCase):
|
||||||
|
"""Unit tests for the shared read_tty_line helper in bot_bottle.util."""
|
||||||
|
|
||||||
|
def test_reads_from_dev_tty(self):
|
||||||
|
from unittest.mock import mock_open
|
||||||
|
from bot_bottle.util import read_tty_line
|
||||||
|
|
||||||
|
m = mock_open(read_data="hello\n")
|
||||||
|
with patch("builtins.open", m):
|
||||||
|
result = read_tty_line()
|
||||||
|
self.assertEqual("hello", result)
|
||||||
|
m.assert_called_once_with("/dev/tty", "r", encoding="utf-8")
|
||||||
|
|
||||||
|
def test_falls_back_to_stdin_on_oserror(self):
|
||||||
|
import io
|
||||||
|
from bot_bottle.util import read_tty_line
|
||||||
|
import sys as _sys
|
||||||
|
|
||||||
|
with patch("builtins.open", side_effect=OSError("no tty")), \
|
||||||
|
patch.object(_sys, "stdin", io.StringIO("world\n")):
|
||||||
|
result = read_tty_line()
|
||||||
|
self.assertEqual("world", result)
|
||||||
|
|
||||||
|
|
||||||
|
class TestPrintVmInstallInstructions(unittest.TestCase):
|
||||||
|
"""Unit tests for _print_vm_install_instructions platform branches."""
|
||||||
|
|
||||||
|
def test_linux_prints_firecracker_instructions(self):
|
||||||
|
from bot_bottle.backend import _print_vm_install_instructions
|
||||||
|
|
||||||
|
with patch.object(backend_mod, "_platform_vm_suggestion",
|
||||||
|
return_value="firecracker"), \
|
||||||
|
patch.object(backend_mod, "info") as mock_info:
|
||||||
|
_print_vm_install_instructions()
|
||||||
|
|
||||||
|
messages = [str(c[0][0]) for c in mock_info.call_args_list]
|
||||||
|
self.assertTrue(any("Firecracker" in m for m in messages))
|
||||||
|
|
||||||
|
def test_macos_prints_apple_container_instructions(self):
|
||||||
|
from bot_bottle.backend import _print_vm_install_instructions
|
||||||
|
|
||||||
|
with patch.object(backend_mod, "_platform_vm_suggestion",
|
||||||
|
return_value="macos-container"), \
|
||||||
|
patch.object(backend_mod, "info") as mock_info:
|
||||||
|
_print_vm_install_instructions()
|
||||||
|
|
||||||
|
messages = [str(c[0][0]) for c in mock_info.call_args_list]
|
||||||
|
self.assertTrue(any("Apple Container" in m for m in messages))
|
||||||
|
|
||||||
|
|
||||||
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
This logic is undoubtably used other places, should be called from a util script/consolidate where this logic is defined
Good call — the same logic lives in
cli/_common.py::read_tty_line. Will move it tobot_bottle/util.pyso both the CLI layer and the backend can import from one place.