From 18dbdb828c6df7a774657b248e7f0071d69c8eab Mon Sep 17 00:00:00 2001 From: claude Date: Sat, 25 Jul 2026 23:16:50 +0000 Subject: [PATCH 1/3] feat: add quick install script and packaging (#197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give bot-bottle a real distribution path so new users can install without cloning the repo: - pyproject.toml: full project metadata, a `bot-bottle` console-script entry point (bot_bottle.cli:main), and package-data for the runtime assets (Dockerfiles, egress entrypoint, netpool defaults, macos init). Still zero runtime pip dependencies. - install.sh: POSIX, sudo-free, idempotent bootstrapper — checks Python >= 3.11, creates ~/.bot-bottle/{agents,bottles,contrib}, installs via pipx (pip --user fallback), then runs `bot-bottle doctor`. - `bot-bottle doctor`: new store-free subcommand reporting Python version, backend availability (reuses is_backend_available rather than hardcoding Docker), and config-dir presence. Exits non-zero when a hard prerequisite is unmet. - PRD prd-new-install-script and unit tests for doctor, the packaging contract, and the install script. Co-Authored-By: Claude Opus 4.8 --- bot_bottle/cli/commands/__init__.py | 3 +- bot_bottle/cli/commands/doctor.py | 81 +++++++++++++++++++ bot_bottle/cli/commands/help.py | 1 + docs/prds/prd-new-install-script.md | 119 ++++++++++++++++++++++++++++ install.sh | 79 ++++++++++++++++++ pyproject.toml | 39 ++++++++- tests/unit/test_cli_doctor.py | 77 ++++++++++++++++++ tests/unit/test_install_script.py | 58 ++++++++++++++ tests/unit/test_pyproject.py | 45 +++++++++++ 9 files changed, 500 insertions(+), 2 deletions(-) create mode 100644 bot_bottle/cli/commands/doctor.py create mode 100644 docs/prds/prd-new-install-script.md create mode 100755 install.sh create mode 100644 tests/unit/test_cli_doctor.py create mode 100644 tests/unit/test_install_script.py create mode 100644 tests/unit/test_pyproject.py diff --git a/bot_bottle/cli/commands/__init__.py b/bot_bottle/cli/commands/__init__.py index de42a33a..3adb0be3 100644 --- a/bot_bottle/cli/commands/__init__.py +++ b/bot_bottle/cli/commands/__init__.py @@ -21,6 +21,7 @@ _HANDLERS: dict[str, str] = { "backend": "backend:cmd_backend", "cleanup": "cleanup:cmd_cleanup", "commit": "commit:cmd_commit", + "doctor": "doctor:cmd_doctor", "edit": "edit:cmd_edit", "help": "help:cmd_help", "init": "init:cmd_init", @@ -53,6 +54,6 @@ COMMANDS = {name: _lazy(spec) for name, spec in _HANDLERS.items()} # gating it on the schema breaks preflight on a fresh CI runner where stdin # isn't a TTY and the migration prompt can't be answered. `help` and `login` # likewise never touch the store. -NO_MIGRATION_COMMANDS = frozenset({"backend", "help", "login"}) +NO_MIGRATION_COMMANDS = frozenset({"backend", "doctor", "help", "login"}) __all__ = ["COMMANDS", "NO_MIGRATION_COMMANDS"] diff --git a/bot_bottle/cli/commands/doctor.py b/bot_bottle/cli/commands/doctor.py new file mode 100644 index 00000000..1cc218da --- /dev/null +++ b/bot_bottle/cli/commands/doctor.py @@ -0,0 +1,81 @@ +"""`doctor` CLI command — validate host prerequisites for running +bot-bottle and report what's ready. + +Fails (non-zero exit) only on the two hard requirements: a new-enough +Python and at least one usable backend. The config directory is a soft +check — `install.sh` creates it, but a missing one only warrants a note, +not a failure, since `start` provisions what it needs on first run. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +from ...backend import is_backend_available, known_backend_names +from ..constants import PROG + +MIN_PYTHON = (3, 11) +CONFIG_DIR = ".bot-bottle" + + +def _ok(label: str, detail: str) -> None: + print(f"ok: {label}: {detail}") + + +def _warn(label: str, detail: str) -> None: + print(f"warn: {label}: {detail}") + + +def _fail(label: str, detail: str) -> None: + print(f"fail: {label}: {detail}") + + +def _check_python() -> bool: + v = sys.version_info + detail = f"{v.major}.{v.minor}.{v.micro}" + if (v.major, v.minor) >= MIN_PYTHON: + _ok("python", detail) + return True + _fail("python", f"{detail}; need {MIN_PYTHON[0]}.{MIN_PYTHON[1]} or newer") + return False + + +def _check_backends() -> bool: + """At least one backend must be available on this host. Report each + known backend so the operator sees why a missing one is missing.""" + available = [] + for name in known_backend_names(): + if is_backend_available(name): + available.append(name) + if available: + _ok("backend", f"available: {', '.join(available)}") + return True + _fail( + "backend", + "no backend available; install Apple Container (macOS), " + "Firecracker (Linux), or Docker", + ) + return False + + +def _check_config_dir() -> None: + config = Path.home() / CONFIG_DIR + if config.is_dir(): + _ok("config", str(config)) + else: + _warn("config", f"{config} does not exist yet (created on first use)") + + +def cmd_doctor(argv: list[str]) -> int: + parser = argparse.ArgumentParser( + prog=f"{PROG} doctor", + description="Check host prerequisites for running bot-bottle.", + ) + parser.parse_args(argv) + + # Hard requirements gate the exit code; the config note is advisory. + required = [_check_python(), _check_backends()] + _check_config_dir() + return 0 if all(required) else 1 diff --git a/bot_bottle/cli/commands/help.py b/bot_bottle/cli/commands/help.py index 9e163723..8c154da3 100644 --- a/bot_bottle/cli/commands/help.py +++ b/bot_bottle/cli/commands/help.py @@ -25,6 +25,7 @@ def cmd_help(argv: list[str] | None = None) -> int: w(" backend set up / check / undo a backend's host prerequisites (setup|status|teardown)\n") w(" cleanup stop and remove all active bot-bottle containers\n") w(" commit snapshot a running bottle's container state to a Docker image\n") + w(" doctor check host prerequisites (Python, backend, config dir)\n") w(" edit open an agent in vim for editing\n") w(" help show this command list\n") w(" init interactively create a new agent and add it to bot-bottle.json\n") diff --git a/docs/prds/prd-new-install-script.md b/docs/prds/prd-new-install-script.md new file mode 100644 index 00000000..54e302b9 --- /dev/null +++ b/docs/prds/prd-new-install-script.md @@ -0,0 +1,119 @@ +# PRD prd-new: Quick install script + +- **Status:** Draft +- **Author:** claude +- **Created:** 2026-07-25 +- **Issue:** #197 + +## Summary + +Add a proper Python package distribution (`pyproject.toml` with a +`bot-bottle` entry point) plus a thin `install.sh` bootstrapper, so users +can install bot-bottle with a single command instead of cloning the repo +and invoking `cli.py` directly. A new `bot-bottle doctor` subcommand +verifies host prerequisites after install. + +## Problem + +There is currently no install path for new users. The only way to run +bot-bottle is to clone the repo and invoke `./cli.py`. This blocks any +public demo: readers want `curl | sh` or `pipx install`, not a manual +clone-and-configure flow. There is also no single command that tells a +user whether their host is actually ready to run a bottle. + +## Goals / Success Criteria + +- `curl -fsSL /install.sh | sh` leaves a working `bot-bottle` + command on PATH. +- Python-native users can install with `pipx install bot-bottle` or + `uv tool install bot-bottle` (once published) — or from a local + checkout today. +- `install.sh` validates prerequisites (Python ≥ 3.11), creates the + `~/.bot-bottle/` config tree, installs the package, and runs + `bot-bottle doctor`. It never installs Docker or a VM backend silently + and never uses `sudo`. +- `install.sh` is idempotent — safe to re-run. +- `bot-bottle doctor` reports Python version, backend availability, and + config-dir presence, exiting non-zero when a hard prerequisite is unmet. +- The package keeps **zero runtime pip dependencies** (stdlib-only, + matching the existing constraint in `AGENTS.md`). + +## Non-goals + +- Bundling a Python runtime or producing a standalone binary. +- Automatic Docker / VM-backend installation. +- Plugin-architecture changes (issue #197 floats a containerized-plugin + direction; that's a separate feature). +- Publishing to a package index in this PR — the package *structure* is + the deliverable; publishing is a follow-up step. + +## Design + +### Package structure (`pyproject.toml`) + +Fill out the previously-stub `pyproject.toml` with project metadata, a +console-script entry point, and package-data for the non-Python assets the +runtime reads from inside the package: + +```toml +[project] +name = "bot-bottle" +version = "0.1.0" +requires-python = ">=3.11" +dependencies = [] + +[project.scripts] +bot-bottle = "bot_bottle.cli:main" +``` + +`bot_bottle.cli:main` already exists (the `cli.py` shim calls it), so no +refactor of the entry point is needed. `package-data` ships the container +Dockerfiles, `egress_entrypoint.sh`, the firecracker netpool defaults, and +the macos-container init script so an installed wheel can still build its +sidecar/agent images. + +### `install.sh` + +A POSIX `sh` bootstrapper that: + +1. Checks `python3` is present and ≥ 3.11; exits with a clear message + otherwise. +2. Creates `~/.bot-bottle/{agents,bottles,contrib}`. +3. Installs via `pipx` if available, else `python3 -m pip install --user`. + The spec defaults to the git URL and is overridable via + `BOT_BOTTLE_INSTALL_SPEC` (used by tests / local installs). +4. Locates the `bot-bottle` entry point (PATH or `~/.local/bin`). +5. Runs `bot-bottle doctor` and reports the result. + +It is idempotent and never calls `sudo`. + +### `bot-bottle doctor` + +A new store-free subcommand (no DB migration required) that checks and +reports: + +- **python** — interpreter version (hard requirement: ≥ 3.11). +- **backend** — at least one backend available on this host + (macos-container / firecracker / docker), reusing + `is_backend_available()` rather than hardcoding Docker, since the + default backend is now a VM backend. Hard requirement. +- **config** — whether `~/.bot-bottle/` exists (advisory only; `start` + provisions on first run). + +Exits 0 when both hard requirements pass, non-zero otherwise. + +## Testing strategy + +- Unit test `bot-bottle doctor` success/failure paths with backend + availability and Python version mocked. +- Unit test that `pyproject.toml` parses, declares the entry point and an + empty `dependencies` list, and that every `package-data` glob resolves + to a file that exists on disk (guards against drift). +- Unit test that `install.sh` is executable, POSIX-ish (`set -eu`), never + calls `sudo`, and runs `doctor` after install. + +## Open questions + +- Should `version` be derived from a git tag at build time (e.g. + `hatch-vcs`) or kept static? Static (`0.1.0`) is simpler for now. +- Publishing target (PyPI vs. a self-hosted index) is deferred. diff --git a/install.sh b/install.sh new file mode 100755 index 00000000..667b4b8c --- /dev/null +++ b/install.sh @@ -0,0 +1,79 @@ +#!/bin/sh +# bot-bottle quick installer. +# +# Usage: +# curl -fsSL https://gitea.dideric.is/didericis/bot-bottle/raw/branch/main/install.sh | sh +# +# Python-native users can skip this entirely: +# pipx install bot-bottle # from a checkout or a published index +# uv tool install bot-bottle +# +# This script is a thin bootstrapper: it checks prerequisites, installs the +# package with pipx (falling back to pip --user), creates the config dir, and +# runs `bot-bottle doctor`. It is idempotent (safe to re-run) and never uses +# sudo. It does NOT install Docker or a VM backend for you — `doctor` reports +# what's missing after install. +set -eu + +PACKAGE_SPEC="${BOT_BOTTLE_INSTALL_SPEC:-git+https://gitea.dideric.is/didericis/bot-bottle.git}" +MIN_PYTHON_MAJOR=3 +MIN_PYTHON_MINOR=11 + +say() { + printf 'bot-bottle install: %s\n' "$*" >&2 +} + +die() { + say "error: $*" + exit 1 +} + +# --- prerequisites ----------------------------------------------------------- + +command -v python3 >/dev/null 2>&1 \ + || die "python3 ${MIN_PYTHON_MAJOR}.${MIN_PYTHON_MINOR}+ is required but was not found" + +python3 - "$MIN_PYTHON_MAJOR" "$MIN_PYTHON_MINOR" <<'PY' || die "python3 ${MIN_PYTHON_MAJOR}.${MIN_PYTHON_MINOR} or newer is required" +import sys + +want = (int(sys.argv[1]), int(sys.argv[2])) +raise SystemExit(0 if sys.version_info[:2] >= want else 1) +PY + +# --- config directories ------------------------------------------------------ + +mkdir -p \ + "${HOME}/.bot-bottle/agents" \ + "${HOME}/.bot-bottle/bottles" \ + "${HOME}/.bot-bottle/contrib" + +# --- install ----------------------------------------------------------------- + +if command -v pipx >/dev/null 2>&1; then + say "installing with pipx" + pipx install --force "${PACKAGE_SPEC}" +else + say "pipx not found; installing with 'python3 -m pip install --user'" + python3 -m pip install --user --upgrade "${PACKAGE_SPEC}" +fi + +# --- locate the entry point -------------------------------------------------- + +if command -v bot-bottle >/dev/null 2>&1; then + BOT_BOTTLE_BIN="bot-bottle" +elif [ -x "${HOME}/.local/bin/bot-bottle" ]; then + BOT_BOTTLE_BIN="${HOME}/.local/bin/bot-bottle" + say "note: add ${HOME}/.local/bin to your PATH to run 'bot-bottle' directly" +else + die "bot-bottle was installed but is not on PATH; add ~/.local/bin to PATH and re-run" +fi + +# --- verify ------------------------------------------------------------------ + +say "running '${BOT_BOTTLE_BIN} doctor'" +if "${BOT_BOTTLE_BIN}" doctor; then + say "done. Run '${BOT_BOTTLE_BIN} --help' to get started." +else + say "install completed, but 'doctor' reported unmet prerequisites (see above)." + say "resolve them, then re-run '${BOT_BOTTLE_BIN} doctor'." +fi diff --git a/pyproject.toml b/pyproject.toml index 64359a16..6d600f2c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,5 +4,42 @@ build-backend = "setuptools.build_meta" [project] name = "bot-bottle" -version = "0.0.0" +version = "0.1.0" +description = "Self-hosted sandbox for running AI coding agents with egress controls" +readme = "README.md" requires-python = ">=3.11" +license = { text = "Apache-2.0" } +authors = [{ name = "didericis" }] +keywords = ["ai", "agents", "sandbox", "security", "egress"] +classifiers = [ + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3 :: Only", + "Operating System :: POSIX :: Linux", + "Operating System :: MacOS", +] +# The package itself has no runtime pip dependencies (stdlib-only); the +# only language runtime is the Python interpreter. Keep this empty. +dependencies = [] + +[project.urls] +Homepage = "https://gitea.dideric.is/didericis/bot-bottle" +Source = "https://gitea.dideric.is/didericis/bot-bottle" + +[project.scripts] +bot-bottle = "bot_bottle.cli:main" + +[tool.setuptools.packages.find] +include = ["bot_bottle*"] + +# Non-Python assets the runtime reads from inside the package (container +# build contexts, entrypoints, netpool defaults). Keep in sync with the +# files shipped under bot_bottle/; test_pyproject.py asserts they exist. +[tool.setuptools.package-data] +bot_bottle = [ + "egress_entrypoint.sh", + "contrib/claude/Dockerfile", + "contrib/codex/Dockerfile", + "contrib/pi/Dockerfile", + "backend/firecracker/netpool.defaults.env", + "backend/macos_container/nested-containers-init.sh", +] diff --git a/tests/unit/test_cli_doctor.py b/tests/unit/test_cli_doctor.py new file mode 100644 index 00000000..871398ed --- /dev/null +++ b/tests/unit/test_cli_doctor.py @@ -0,0 +1,77 @@ +"""Unit: `bot-bottle doctor` host prerequisite checks (ADR 0004). + +`doctor` is a store-free diagnostic — it must run on a fresh install +before any DB migration, and its exit code gates only the two hard +prerequisites (Python and an available backend). The config-dir check is +advisory and never affects the exit code. +""" + +from __future__ import annotations + +import io +import tempfile +import unittest +from contextlib import redirect_stdout +from pathlib import Path +from unittest.mock import patch + +from bot_bottle.cli.commands import doctor + + +def _run(argv: list[str] | None = None) -> tuple[int, str]: + buf = io.StringIO() + with redirect_stdout(buf): + code = doctor.cmd_doctor(argv or []) + return code, buf.getvalue() + + +class TestDoctor(unittest.TestCase): + def test_passes_when_python_and_backend_ok(self): + with patch.object(doctor, "known_backend_names", return_value=("docker",)), \ + patch.object(doctor, "is_backend_available", return_value=True): + code, out = _run() + self.assertEqual(0, code) + self.assertIn("ok: python", out) + self.assertIn("ok: backend: available: docker", out) + + def test_fails_when_no_backend_available(self): + with patch.object(doctor, "known_backend_names", return_value=("docker", "firecracker")), \ + patch.object(doctor, "is_backend_available", return_value=False): + code, out = _run() + self.assertEqual(1, code) + self.assertIn("fail: backend", out) + + def test_fails_when_python_too_old(self): + # Force the version gate to fail without touching the interpreter. + with patch.object(doctor, "MIN_PYTHON", (99, 0)), \ + patch.object(doctor, "known_backend_names", return_value=("docker",)), \ + patch.object(doctor, "is_backend_available", return_value=True): + code, out = _run() + self.assertEqual(1, code) + self.assertIn("fail: python", out) + + def test_missing_config_dir_is_advisory_not_fatal(self): + # A missing ~/.bot-bottle warns but must not fail. Point home at a + # fresh empty dir so the shared suite HOME (which other tests may + # populate) can't turn this into an "ok: config". + with tempfile.TemporaryDirectory() as tmp, \ + patch.object(doctor.Path, "home", return_value=Path(tmp)), \ + patch.object(doctor, "known_backend_names", return_value=("docker",)), \ + patch.object(doctor, "is_backend_available", return_value=True): + code, out = _run() + self.assertEqual(0, code) + self.assertIn("warn: config", out) + + def test_present_config_dir_reports_ok(self): + with tempfile.TemporaryDirectory() as tmp, \ + patch.object(doctor.Path, "home", return_value=Path(tmp)), \ + patch.object(doctor, "known_backend_names", return_value=("docker",)), \ + patch.object(doctor, "is_backend_available", return_value=True): + (Path(tmp) / ".bot-bottle").mkdir() + code, out = _run() + self.assertEqual(0, code) + self.assertIn("ok: config", out) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_install_script.py b/tests/unit/test_install_script.py new file mode 100644 index 00000000..48475ed8 --- /dev/null +++ b/tests/unit/test_install_script.py @@ -0,0 +1,58 @@ +"""Unit: install.sh bootstrapper contract. + +The installer is a thin, sudo-free, idempotent bootstrapper. These are +static checks on the script text (no network / no real install) so CI can +run them anywhere: it must be executable, fail-fast, never call sudo, +create the config tree, install the package, and verify with `doctor`. +""" + +from __future__ import annotations + +import os +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +INSTALL_SH = REPO_ROOT / "install.sh" + + +class TestInstallScript(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.text = INSTALL_SH.read_text() + + def test_exists_and_executable(self): + self.assertTrue(INSTALL_SH.is_file()) + self.assertTrue(os.access(INSTALL_SH, os.X_OK), "install.sh must be executable") + + def test_posix_shebang_and_failfast(self): + first = self.text.splitlines()[0] + self.assertEqual("#!/bin/sh", first) + self.assertIn("set -eu", self.text) + + def test_never_uses_sudo(self): + # Only executable lines matter; the header comment may mention sudo. + code = [ + ln for ln in self.text.splitlines() + if ln.strip() and not ln.lstrip().startswith("#") + ] + self.assertNotIn("sudo", "\n".join(code)) + + def test_creates_config_tree(self): + self.assertIn(".bot-bottle/agents", self.text) + self.assertIn(".bot-bottle/bottles", self.text) + + def test_installs_via_pipx_with_pip_fallback(self): + self.assertIn("pipx install", self.text) + self.assertIn("pip install --user", self.text) + + def test_runs_doctor_after_install(self): + self.assertIn("doctor", self.text) + + def test_install_spec_is_overridable(self): + # Tests / local installs point BOT_BOTTLE_INSTALL_SPEC at a checkout. + self.assertIn("BOT_BOTTLE_INSTALL_SPEC", self.text) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_pyproject.py b/tests/unit/test_pyproject.py new file mode 100644 index 00000000..fed76d02 --- /dev/null +++ b/tests/unit/test_pyproject.py @@ -0,0 +1,45 @@ +"""Unit: pyproject.toml packaging contract. + +Guards the install/distribution surface: the console-script entry point, +the stdlib-only (empty) dependency list, and that every package-data glob +still points at a file that exists (so an installed wheel isn't missing a +Dockerfile or entrypoint the runtime reads). +""" + +from __future__ import annotations + +import tomllib +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +PYPROJECT = REPO_ROOT / "pyproject.toml" + + +class TestPyproject(unittest.TestCase): + @classmethod + def setUpClass(cls): + with PYPROJECT.open("rb") as fh: + cls.data = tomllib.load(fh) + + def test_entry_point_targets_cli_main(self): + scripts = self.data["project"]["scripts"] + self.assertEqual("bot_bottle.cli:main", scripts["bot-bottle"]) + + def test_no_runtime_dependencies(self): + # AGENTS.md: the package has no runtime pip dependencies. + self.assertEqual([], self.data["project"]["dependencies"]) + + def test_requires_python_311(self): + self.assertEqual(">=3.11", self.data["project"]["requires-python"]) + + def test_package_data_files_exist(self): + pkg_data = self.data["tool"]["setuptools"]["package-data"]["bot_bottle"] + self.assertTrue(pkg_data, "expected package-data entries") + for rel in pkg_data: + path = REPO_ROOT / "bot_bottle" / rel + self.assertTrue(path.is_file(), f"package-data missing: {path}") + + +if __name__ == "__main__": + unittest.main() -- 2.52.0 From 10c46fc584fe205f3bfd4026d439503b52044bb6 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 00:04:35 +0000 Subject: [PATCH 2/3] fix: make installed wheel self-contained + harden install.sh prereqs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on PR #481. Self-contained wheel (review point 1): the gateway/infra/orchestrator images build from a context that must hold bot_bottle/, pyproject.toml, and the root-level Dockerfiles. Modules previously located these by walking __file__ to the repo root, so an installed wheel (package in site-packages, no repo root) passed `doctor` but failed `start`. - Add bot_bottle/resources.py: build_root() returns the repo root in a checkout (unchanged) or a staged copy from the wheel's bundled _resources/ otherwise; dockerfile()/nix_netpool_module()/ netpool_script() derive from it. - setup.py bundles the root Dockerfiles, nix module, netpool script, and pyproject.toml into bot_bottle/_resources/ at build; MANIFEST.in ships them in the sdist. - Route every _REPO_ROOT/_REPO_DIR call site (docker/macos launch, macos infra, firecracker infra_vm/infra_artifact/setup, orchestrator lifecycle/gateway) through resources. Checkout behavior is unchanged. install.sh prerequisites (review point 2): check for git when installing a git+ spec, and — before the pip fallback — that pip is usable and the interpreter isn't externally managed (PEP 668), pointing at pipx. Tests: test_resources covers checkout + staged-wheel layouts; test_wheel_install builds the wheel, installs it into an isolated venv, and asserts `doctor` runs and build_root() yields a valid context. Running `start` end-to-end still needs a Docker/KVM host (CI). Co-Authored-By: Claude Opus 4.8 --- .coveragerc | 6 + .gitignore | 3 + MANIFEST.in | 8 + bot_bottle/backend/docker/gateway.py | 12 +- bot_bottle/backend/docker/infra.py | 9 +- bot_bottle/backend/docker/launch.py | 8 +- bot_bottle/backend/docker/orchestrator.py | 9 +- .../backend/firecracker/infra_artifact.py | 7 +- bot_bottle/backend/firecracker/infra_vm.py | 9 +- bot_bottle/backend/firecracker/setup.py | 9 +- bot_bottle/backend/macos_container/gateway.py | 9 +- bot_bottle/backend/macos_container/infra.py | 9 +- bot_bottle/backend/macos_container/launch.py | 5 +- .../backend/macos_container/orchestrator.py | 8 +- bot_bottle/gateway/__init__.py | 1 - bot_bottle/resources.py | 159 ++++++++++++++++ docs/prds/prd-new-install-script.md | 59 +++++- install.sh | 35 ++++ pyproject.toml | 2 +- requirements-dev.txt | 3 + setup.py | 46 +++++ tests/unit/test_install_script.py | 15 ++ tests/unit/test_macos_nested_containers.py | 2 +- tests/unit/test_resources.py | 175 ++++++++++++++++++ tests/unit/test_wheel_install.py | 119 ++++++++++++ 25 files changed, 675 insertions(+), 52 deletions(-) create mode 100644 MANIFEST.in create mode 100644 bot_bottle/resources.py create mode 100644 setup.py create mode 100644 tests/unit/test_resources.py create mode 100644 tests/unit/test_wheel_install.py diff --git a/.coveragerc b/.coveragerc index 161fde3f..7bf4ae03 100644 --- a/.coveragerc +++ b/.coveragerc @@ -20,3 +20,9 @@ omit = bot_bottle/cli/tui.py bot_bottle/cli/init.py tests/* + # Build-time only: setuptools invokes it out-of-process to build the + # wheel/sdist (it's never imported by the running app), so in-process + # coverage can't reach it. Its one job — bundling the root resources into + # bot_bottle/_resources/ — is exercised end-to-end by test_wheel_install, + # which builds and installs a real wheel and checks the result. + setup.py diff --git a/.gitignore b/.gitignore index 03d76f4e..8c0aa223 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,9 @@ __pycache__/ *.py[cod] *$py.class *.egg-info/ +# setuptools/build_meta output (wheels, sdists, build tree) +/build/ +/dist/ .venv/ venv/ .pytest_cache/ diff --git a/MANIFEST.in b/MANIFEST.in new file mode 100644 index 00000000..a39c977c --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,8 @@ +# Root-level build resources copied into bot_bottle/_resources/ at build time +# (see setup.py). Included in the sdist so `pip install` from an sdist can +# still bundle them into the wheel. +include Dockerfile.gateway +include Dockerfile.orchestrator +include Dockerfile.orchestrator.fc +include nix/firecracker-netpool.nix +include scripts/firecracker-netpool.sh diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index 1cac5d32..612b3d77 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -10,9 +10,10 @@ from ...paths import ( ORCHESTRATOR_AUTH_JWT_ENV, host_gateway_ca_dir, ) +from ... import resources from ...gateway import ( Gateway, GatewayTransport, GATEWAY_IMAGE, GATEWAY_NAME, GATEWAY_NETWORK, - GATEWAY_DOCKERFILE, REPO_ROOT, GATEWAY_LABEL, MITMPROXY_HOME, + GATEWAY_DOCKERFILE, GATEWAY_LABEL, MITMPROXY_HOME, DEFAULT_CA_TIMEOUT_SECONDS, CA_POLL_SECONDS, GATEWAY_CA_CERT, GatewayError ) @@ -50,7 +51,9 @@ class DockerGateway(Gateway): # `address` / `stop` work on an already-running gateway without it. self._orchestrator_url = "" self._gateway_token = "" - self._build_context = build_context or REPO_ROOT + # Resolved lazily in ensure_built() so merely constructing a gateway to + # read its CA never stages a build root from an installed wheel. + self._build_context = build_context self._dockerfile = dockerfile # Ports published on the host (0.0.0.0). Used by the Firecracker # backend's dev-harness gateway so VMs can reach it via their TAP link; @@ -72,9 +75,10 @@ class DockerGateway(Gateway): forces a full rebuild (parity with `start --no-cache`).""" if self._dockerfile is None: return + context = self._build_context or resources.build_root() argv = ["docker", "build", "-t", self.image_ref, - "-f", str(self._build_context / self._dockerfile), - str(self._build_context)] + "-f", str(context / self._dockerfile), + str(context)] if os.environ.get("BOT_BOTTLE_NO_CACHE"): argv.insert(2, "--no-cache") proc = run_docker(argv) diff --git a/bot_bottle/backend/docker/infra.py b/bot_bottle/backend/docker/infra.py index 1320146a..66e26837 100644 --- a/bot_bottle/backend/docker/infra.py +++ b/bot_bottle/backend/docker/infra.py @@ -34,6 +34,7 @@ from .orchestrator import ( ORCHESTRATOR_NETWORK, ) from ...paths import bot_bottle_root +from ... import resources from ...gateway import ( GATEWAY_IMAGE, GATEWAY_NAME, @@ -50,8 +51,6 @@ from ...orchestrator.lifecycle import ( # the pair's public identity. INFRA_NAME = GATEWAY_NAME # the container agents attribute against is the gateway -_REPO_ROOT = Path(__file__).resolve().parents[3] - class DockerInfraService(InfraService): """Composes the per-host control plane + gateway as two containers. @@ -68,7 +67,7 @@ class DockerInfraService(InfraService): control_network: str = ORCHESTRATOR_NETWORK, orchestrator_image: str = ORCHESTRATOR_IMAGE, gateway_image: str = GATEWAY_IMAGE, - repo_root: Path = _REPO_ROOT, + repo_root: Path | None = None, host_root: Path | None = None, orchestrator_name: str = ORCHESTRATOR_NAME, orchestrator_label: str = ORCHESTRATOR_LABEL, @@ -79,7 +78,9 @@ class DockerInfraService(InfraService): self.control_network = control_network self.orchestrator_image = orchestrator_image self.gateway_image = gateway_image - self._repo_root = repo_root + # Build context / bind-mount source: the repo root in a checkout, a + # staged copy from the installed wheel otherwise (bot_bottle.resources). + self._repo_root = repo_root if repo_root is not None else resources.build_root() self._host_root = host_root or bot_bottle_root() self._orchestrator_name = orchestrator_name self._orchestrator_label = orchestrator_label diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 3cec0b6a..1b96f01d 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -33,7 +33,6 @@ from __future__ import annotations import dataclasses import os from contextlib import ExitStack, contextmanager -from pathlib import Path from typing import Callable, Generator from ...agent_provider import runtime_for @@ -65,10 +64,7 @@ from ...orchestrator.store.config_store import resolve_teardown_timeout from .consolidated_launch import launch_consolidated, deprovision_consolidated from .infra import INFRA_NAME from .gateway import DockerGateway - - -# Where the repo root lives, for `docker build` context. Computed once. -_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent.parent) +from ... import resources def build_or_load_images(plan: DockerBottlePlan) -> BottleImages: @@ -88,7 +84,7 @@ def build_or_load_images(plan: DockerBottlePlan) -> BottleImages: ) info(f"using cached agent image {plan.image!r}") return BottleImages(agent=plan.image) - docker_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) + docker_mod.build_image(plan.image, str(resources.build_root()), dockerfile=plan.dockerfile_path) docker_mod.verify_agent_image( plan.image, runtime_for(plan.agent_provider_template).smoke_test, ) diff --git a/bot_bottle/backend/docker/orchestrator.py b/bot_bottle/backend/docker/orchestrator.py index 6c8c45ad..6197fe87 100644 --- a/bot_bottle/backend/docker/orchestrator.py +++ b/bot_bottle/backend/docker/orchestrator.py @@ -15,6 +15,7 @@ import time from pathlib import Path from ... import log +from ... import resources from .util import run_docker from ...paths import ( ORCHESTRATOR_TOKEN_ENV, @@ -55,8 +56,6 @@ _ROOT_IN_CONTAINER = "/bot-bottle-root" _HEALTH_POLL_SECONDS = 0.25 -_REPO_ROOT = Path(__file__).resolve().parents[3] - class DockerOrchestrator(Orchestrator): """The control plane as a single fixed-name container. `ensure_built` builds @@ -71,7 +70,7 @@ class DockerOrchestrator(Orchestrator): label: str = ORCHESTRATOR_LABEL, port: int = DEFAULT_PORT, control_network: str = ORCHESTRATOR_NETWORK, - repo_root: Path = _REPO_ROOT, + repo_root: Path | None = None, host_root: Path | None = None, dockerfile: str | None = ORCHESTRATOR_DOCKERFILE, ) -> None: @@ -80,7 +79,9 @@ class DockerOrchestrator(Orchestrator): self.label = label self.port = port self.control_network = control_network - self._repo_root = repo_root + # Build context / bind-mount source: the repo root in a checkout, a + # staged copy from the installed wheel otherwise (bot_bottle.resources). + self._repo_root = repo_root if repo_root is not None else resources.build_root() self._host_root = host_root or bot_bottle_root() self._dockerfile = dockerfile diff --git a/bot_bottle/backend/firecracker/infra_artifact.py b/bot_bottle/backend/firecracker/infra_artifact.py index b5b7ae60..ba0ca075 100644 --- a/bot_bottle/backend/firecracker/infra_artifact.py +++ b/bot_bottle/backend/firecracker/infra_artifact.py @@ -37,6 +37,7 @@ import urllib.error import urllib.request from pathlib import Path +from ... import resources from ...log import die, info from . import util @@ -44,8 +45,6 @@ from . import util # scheme can't collide with a cached/published artifact of the old one. _ARTIFACT_FORMAT = "1" -_REPO_ROOT = Path(__file__).resolve().parents[3] - # The two per-plane infra VM roles. Each publishes/pulls its own rootfs artifact # from its own generic package; the Dockerfiles baked into each differ (only the # orchestrator rootfs carries buildah), so the versions are hashed separately. @@ -74,7 +73,7 @@ def local_build_requested() -> bool: def infra_artifact_version( - init_script: str, role: str, *, repo_root: Path = _REPO_ROOT, + init_script: str, role: str, *, repo_root: Path | None = None, ) -> str: """Content hash (16 hex) of everything baked into `role`'s infra rootfs: the whole shipped `bot_bottle` package, that role's Dockerfiles, and its guest @@ -89,6 +88,8 @@ def infra_artifact_version( version or a launch host could boot a stale rootfs whose code differs from its checkout. `__pycache__`/`.pyc` are the only exclusions — build artifacts, never copied.""" + if repo_root is None: + repo_root = resources.build_root() h = hashlib.sha256() h.update(f"format={_ARTIFACT_FORMAT}\nrole={role}\n".encode()) pkg = repo_root / "bot_bottle" diff --git a/bot_bottle/backend/firecracker/infra_vm.py b/bot_bottle/backend/firecracker/infra_vm.py index 9183136c..0fa03f0f 100644 --- a/bot_bottle/backend/firecracker/infra_vm.py +++ b/bot_bottle/backend/firecracker/infra_vm.py @@ -42,6 +42,7 @@ from dataclasses import dataclass from pathlib import Path from typing import Generator +from ... import resources from ...log import die, info from ..docker import util as docker_mod from . import firecracker_vm, infra_artifact, netpool, util @@ -65,7 +66,6 @@ _GUEST_GATEWAY_JWT_PATH = "/var/lib/bot-bottle/gateway-jwt" _GATEWAY_IMAGE = "bot-bottle-gateway:latest" _ORCHESTRATOR_IMAGE = "bot-bottle-orchestrator:latest" _ORCHESTRATOR_FC_IMAGE = "bot-bottle-orchestrator-fc:latest" -_REPO_ROOT = Path(__file__).resolve().parents[3] # Per-role rootfs source image + the extra free space `mke2fs` leaves for the # guest to grow into. The orchestrator keeps buildah's large build slack; the @@ -130,12 +130,13 @@ def build_infra_images_with_docker() -> None: orchestrator + buildah). The gateway VM boots the gateway image directly. The launch host uses this only in `BOT_BOTTLE_INFRA_BUILD=local` mode; `publish_infra` uses it off-host to produce the published artifacts.""" + root = str(resources.build_root()) docker_mod.build_image( - _ORCHESTRATOR_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.orchestrator") + _ORCHESTRATOR_IMAGE, root, dockerfile="Dockerfile.orchestrator") docker_mod.build_image( - _GATEWAY_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.gateway") + _GATEWAY_IMAGE, root, dockerfile="Dockerfile.gateway") docker_mod.build_image( - _ORCHESTRATOR_FC_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.orchestrator.fc") + _ORCHESTRATOR_FC_IMAGE, root, dockerfile="Dockerfile.orchestrator.fc") def build_rootfs_dir(role: str) -> Path: diff --git a/bot_bottle/backend/firecracker/setup.py b/bot_bottle/backend/firecracker/setup.py index d63dbb50..b7c93ed2 100644 --- a/bot_bottle/backend/firecracker/setup.py +++ b/bot_bottle/backend/firecracker/setup.py @@ -20,6 +20,7 @@ import subprocess import sys from pathlib import Path +from ... import resources from . import netpool from . import util @@ -42,13 +43,13 @@ def _has_systemd() -> bool: def _module_path() -> str: - """Absolute path to the importable NixOS module in this checkout.""" - return str(Path(__file__).resolve().parents[3] / "nix" / "firecracker-netpool.nix") + """Absolute path to the importable NixOS module (checkout or wheel).""" + return str(resources.nix_netpool_module()) def _script_path() -> str: - """Absolute path to the bundled bring-up script in this checkout.""" - return str(Path(__file__).resolve().parents[3] / "scripts" / "firecracker-netpool.sh") + """Absolute path to the bundled bring-up script (checkout or wheel).""" + return str(resources.netpool_script()) def _print_prereqs() -> None: diff --git a/bot_bottle/backend/macos_container/gateway.py b/bot_bottle/backend/macos_container/gateway.py index e81b8e9c..4b84193e 100644 --- a/bot_bottle/backend/macos_container/gateway.py +++ b/bot_bottle/backend/macos_container/gateway.py @@ -28,6 +28,7 @@ from ...paths import ( ORCHESTRATOR_AUTH_JWT_ENV, host_gateway_ca_dir, ) +from ... import resources from .. import util as backend_util from . import util as container_mod @@ -52,8 +53,6 @@ GATEWAY_DAEMONS = "egress,git-http,supervise" GATEWAY_IMAGE = os.environ.get("BOT_BOTTLE_GATEWAY_IMAGE", "bot-bottle-gateway:latest") -_REPO_ROOT = Path(__file__).resolve().parents[3] - def ensure_networks( network: str = GATEWAY_NETWORK, @@ -84,14 +83,16 @@ class MacosGateway(Gateway): network: str = GATEWAY_NETWORK, egress_network: str = GATEWAY_EGRESS_NETWORK, control_network: str = CONTROL_NETWORK, - repo_root: Path = _REPO_ROOT, + repo_root: Path | None = None, ) -> None: self.image_ref = image_ref self.name = name self.network = network self.egress_network = egress_network self.control_network = control_network - self._repo_root = repo_root + # Build context: the repo root in a checkout, a staged copy from the + # installed wheel otherwise (bot_bottle.resources). + self._repo_root = repo_root if repo_root is not None else resources.build_root() # Set by `connect_to_orchestrator`: the URL the daemons resolve policy # against + the pre-minted `gateway` token they present. The gateway # never mints, so it never holds the signing key (#469). diff --git a/bot_bottle/backend/macos_container/infra.py b/bot_bottle/backend/macos_container/infra.py index 2327480d..7d250cd5 100644 --- a/bot_bottle/backend/macos_container/infra.py +++ b/bot_bottle/backend/macos_container/infra.py @@ -24,6 +24,7 @@ from __future__ import annotations from pathlib import Path +from ... import resources from ...orchestrator.lifecycle import ( DEFAULT_PORT, DEFAULT_STARTUP_TIMEOUT_SECONDS, @@ -53,8 +54,6 @@ from .orchestrator import ( # still import it (probe / reprovision attribute against the gateway). INFRA_NAME = GATEWAY_NAME -_REPO_ROOT = Path(__file__).resolve().parents[3] - class MacosInfraService(InfraService): """Composes the per-host orchestrator + gateway containers. Callers use @@ -70,7 +69,7 @@ class MacosInfraService(InfraService): control_network: str = CONTROL_NETWORK, gateway_image: str = GATEWAY_IMAGE, orchestrator_image: str = ORCHESTRATOR_IMAGE, - repo_root: Path = _REPO_ROOT, + repo_root: Path | None = None, orchestrator_name: str = ORCHESTRATOR_NAME, gateway_name: str = INFRA_NAME, db_volume: str = ORCHESTRATOR_DB_VOLUME, @@ -81,7 +80,9 @@ class MacosInfraService(InfraService): self.control_network = control_network self.gateway_image = gateway_image self.orchestrator_image = orchestrator_image - self._repo_root = repo_root + # Build context / bind-mount source: the repo root in a checkout, a + # staged copy from the installed wheel otherwise (bot_bottle.resources). + self._repo_root = repo_root if repo_root is not None else resources.build_root() self._orchestrator_name = orchestrator_name self._gateway_name = gateway_name self._db_volume = db_volume diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 34793fba..e3b87fa4 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -36,7 +36,6 @@ import dataclasses import os import subprocess from contextlib import ExitStack, contextmanager -from pathlib import Path from typing import Callable, Generator from ...bottle_state import ( @@ -49,6 +48,7 @@ from ...git_gate import GitGate from ...gateway.git_gate.http_backend import DEFAULT_PORT as _GIT_HTTP_PORT from ...image_cache import check_stale from ...log import die, info, warn +from ... import resources from .. import BottleImages from ...supervisor.types import SUPERVISE_PORT from ..docker.egress import EGRESS_PORT @@ -71,7 +71,6 @@ from .consolidated_launch import ( deprovision_consolidated, ) -_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent.parent) _AGENT_SLEEP_SECONDS = "2147483647" @@ -94,7 +93,7 @@ def _agent_image(plan: MacosContainerBottlePlan) -> str: ) info(f"using cached agent image {plan.image!r}") return plan.image - container_mod.build_image(plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path) + container_mod.build_image(plan.image, str(resources.build_root()), dockerfile=plan.dockerfile_path) return plan.image diff --git a/bot_bottle/backend/macos_container/orchestrator.py b/bot_bottle/backend/macos_container/orchestrator.py index 3831e5ba..bd14d6ca 100644 --- a/bot_bottle/backend/macos_container/orchestrator.py +++ b/bot_bottle/backend/macos_container/orchestrator.py @@ -18,6 +18,7 @@ import urllib.request from pathlib import Path from ... import log +from ... import resources from ...paths import ORCHESTRATOR_TOKEN_ENV from ...orchestrator.lifecycle import ( DEFAULT_HEALTH_TIMEOUT_SECONDS, @@ -45,7 +46,6 @@ _DB_ROOT_IN_CONTAINER = "/var/lib/bot-bottle" _SRC_IN_CONTAINER = "/bot-bottle-src" _HEALTH_POLL_SECONDS = 0.25 -_REPO_ROOT = Path(__file__).resolve().parents[3] class MacosOrchestrator(Orchestrator): @@ -61,7 +61,7 @@ class MacosOrchestrator(Orchestrator): label: str = ORCHESTRATOR_LABEL, port: int = DEFAULT_PORT, control_network: str = CONTROL_NETWORK, - repo_root: Path = _REPO_ROOT, + repo_root: Path | None = None, db_volume: str = ORCHESTRATOR_DB_VOLUME, ) -> None: self.image_ref = image_ref @@ -69,7 +69,9 @@ class MacosOrchestrator(Orchestrator): self.label = label self.port = port self.control_network = control_network - self._repo_root = repo_root + # Build context / bind-mount source: the repo root in a checkout, a + # staged copy from the installed wheel otherwise (bot_bottle.resources). + self._repo_root = repo_root if repo_root is not None else resources.build_root() self._db_volume = db_volume def url(self) -> str: diff --git a/bot_bottle/gateway/__init__.py b/bot_bottle/gateway/__init__.py index 568ba7c5..27925748 100644 --- a/bot_bottle/gateway/__init__.py +++ b/bot_bottle/gateway/__init__.py @@ -62,7 +62,6 @@ GATEWAY_CA_GLOB = "mitmproxy-ca*" # that lands. Env override matches the backend's BOT_BOTTLE_GATEWAY_IMAGE. GATEWAY_IMAGE = os.environ.get("BOT_BOTTLE_GATEWAY_IMAGE", "bot-bottle-gateway:latest") GATEWAY_DOCKERFILE = "Dockerfile.gateway" -REPO_ROOT = Path(__file__).resolve().parents[2] def rotate_gateway_ca(ca_dir: Path | None = None) -> list[Path]: diff --git a/bot_bottle/resources.py b/bot_bottle/resources.py new file mode 100644 index 00000000..6fa9c671 --- /dev/null +++ b/bot_bottle/resources.py @@ -0,0 +1,159 @@ +"""Locate build-time resources whether bot-bottle runs from a source +checkout or an installed wheel. + +The gateway / infra / orchestrator images are built from a Docker (or Apple +`container`) build context that must contain the `bot_bottle` package, +`pyproject.toml`, and the root-level Dockerfiles as siblings. In a source +checkout that context is simply the repo root, one level above the package. +An installed wheel has no repo root: the same root-level files are shipped +inside the package under ``bot_bottle/_resources/`` (see ``setup.py``), and a +repo-root-shaped build context is staged on demand into the app-data dir. + +``build_root()`` is the single source of truth — it returns a directory laid +out like a repo root (has ``bot_bottle/``, ``pyproject.toml``, the +Dockerfiles, ``nix/``, ``scripts/``). Every caller that needs a build +context, a Dockerfile path, the nix netpool module, or the netpool script +derives from it, so checkout and wheel installs share one downstream path. +""" + +from __future__ import annotations + +import fcntl +import hashlib +import os +import shutil +import tempfile +from pathlib import Path + +from .paths import bot_bottle_root + +_PKG = Path(__file__).resolve().parent # …/bot_bottle +_CHECKOUT_ROOT = _PKG.parent # repo root in a checkout +_BUNDLED = _PKG / "_resources" # wheel-shipped copies + +# Root-level files bundled into the wheel under ``_resources/`` (paths are +# relative to the checkout root, and preserved verbatim under ``_resources/`` +# and in the staged build root). ``setup.py`` copies exactly this set; keep +# the two lists in sync (``test_resources`` guards that every entry exists). +BUNDLED_RESOURCES: tuple[str, ...] = ( + "pyproject.toml", + "Dockerfile.gateway", + "Dockerfile.orchestrator", + "Dockerfile.orchestrator.fc", + "nix/firecracker-netpool.nix", + "scripts/firecracker-netpool.sh", +) + +# Present at a checkout root, never in a bare installed package — the cheap +# tell for which layout we're in. +_CHECKOUT_MARKER = "Dockerfile.gateway" + + +class ResourceError(RuntimeError): + """Build resources are missing from the install (corrupt/partial wheel).""" + + +def is_source_checkout() -> bool: + """True when running from a source tree (the root Dockerfiles sit beside + the package); False from an installed wheel.""" + return (_CHECKOUT_ROOT / _CHECKOUT_MARKER).is_file() + + +def build_root() -> Path: + """A directory shaped like a repo root: ``bot_bottle/``, ``pyproject.toml``, + the root Dockerfiles, ``nix/``, and ``scripts/``. + + A checkout returns the repo root itself (no copying). An installed wheel + returns a staged copy under the app-data dir, materialized once and reused. + The stage is keyed by a digest of the installed package + bundled resources + (not the distribution version), so a force-reinstall of a newer commit that + keeps ``version = 0.1.0`` still rebuilds instead of reusing a stale tree.""" + if is_source_checkout(): + return _CHECKOUT_ROOT + return _stage_build_root() + + +def dockerfile(name: str) -> Path: + """Absolute path to a root-level Dockerfile, e.g. ``Dockerfile.gateway``.""" + return build_root() / name + + +def nix_netpool_module() -> Path: + """Absolute path to the firecracker netpool NixOS module.""" + return build_root() / "nix" / "firecracker-netpool.nix" + + +def netpool_script() -> Path: + """Absolute path to the firecracker netpool bring-up script.""" + return build_root() / "scripts" / "firecracker-netpool.sh" + + +def _content_digest() -> str: + """A 16-hex digest of the installed package + bundled resources. + + Keys the staged build root by *content*, so a force-reinstall over the same + version string (the installer defaults to a git branch + ``pipx install + --force``, and ``version`` stays ``0.1.0``) yields a different key and + re-stages, rather than reusing an old commit's tree. ``_PKG`` already + contains ``_resources``, so walking it covers both.""" + h = hashlib.sha256() + for path in sorted(_PKG.rglob("*")): + if not path.is_file() or "__pycache__" in path.parts or path.suffix == ".pyc": + continue + h.update(str(path.relative_to(_PKG)).encode()) + h.update(b"\0") + h.update(path.read_bytes()) + return h.hexdigest()[:16] + + +def _stage_build_root() -> Path: + """Materialize a repo-root-shaped build context from the installed wheel's + bundled resources, keyed by content digest. Idempotent and concurrency-safe: + a file lock serializes staging, a partial/stale tree is replaced, and the + finished tree is published with an atomic rename.""" + if not _BUNDLED.is_dir(): + raise ResourceError( + "bot-bottle build resources are missing from this install " + f"(expected {_BUNDLED}). Reinstall the package." + ) + base = bot_bottle_root() / "build-root" + base.mkdir(parents=True, exist_ok=True) + dest = base / _content_digest() + if (dest / ".complete").is_file(): + return dest + + # Serialize staging across processes: a concurrent `start` after an install + # must not race on the shared tree. The lock is held only around stage + + # atomic publish; the fast path above never blocks. + with open(base / ".stage.lock", "w", encoding="utf-8") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + if (dest / ".complete").is_file(): # another process staged while we waited + return dest + # Stage into a private temp dir on the same filesystem, then publish by + # rename — never populate a shared path other processes might read. + staging = Path(tempfile.mkdtemp(prefix=".staging-", dir=base)) + try: + # The package itself, minus caches and the bundled-resource copies, + # so the staged ``bot_bottle/`` matches a checkout's (keeps the + # firecracker infra-artifact hash stable across checkout and wheel). + shutil.copytree( + _PKG, + staging / "bot_bottle", + ignore=shutil.ignore_patterns("__pycache__", "*.pyc", "_resources"), + ) + # The bundled root files, restored to their checkout-relative layout. + for rel in BUNDLED_RESOURCES: + dst = staging / rel + dst.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(_BUNDLED / rel, dst) + (staging / ".complete").write_text("") + # Replace any partial leftover for this digest (safe: we hold the + # lock), then publish atomically. + if dest.exists(): + shutil.rmtree(dest) + os.replace(staging, dest) + staging = None # published; nothing to clean up + finally: + if staging is not None: + shutil.rmtree(staging, ignore_errors=True) + return dest diff --git a/docs/prds/prd-new-install-script.md b/docs/prds/prd-new-install-script.md index 54e302b9..87cff452 100644 --- a/docs/prds/prd-new-install-script.md +++ b/docs/prds/prd-new-install-script.md @@ -67,10 +67,54 @@ bot-bottle = "bot_bottle.cli:main" ``` `bot_bottle.cli:main` already exists (the `cli.py` shim calls it), so no -refactor of the entry point is needed. `package-data` ships the container -Dockerfiles, `egress_entrypoint.sh`, the firecracker netpool defaults, and -the macos-container init script so an installed wheel can still build its -sidecar/agent images. +refactor of the entry point is needed. `package-data` ships the non-Python +assets that live *inside* the package (`egress_entrypoint.sh`, the contrib +Dockerfiles, the firecracker netpool defaults, the macos-container init +script). + +### Self-contained wheel (build resources) + +The gateway / infra / orchestrator images are built from a Docker (or Apple +`container`) build context that must contain the `bot_bottle` package, +`pyproject.toml`, and the **root-level** Dockerfiles as siblings. Several +modules used to locate that context by walking `__file__`'s parents to the +repo root (`_REPO_ROOT = Path(__file__)…parents[N]`) and reading +`Dockerfile.gateway`, `nix/firecracker-netpool.nix`, and +`scripts/firecracker-netpool.sh` from it. In an installed wheel the package +lives in `site-packages` with no repo root above it, so those reads fail — +`doctor` passes but `start` / backend setup breaks. + +Fix: a single resolver, `bot_bottle/resources.py`. + +- `build_root()` returns a directory shaped like a repo root (has + `bot_bottle/`, `pyproject.toml`, the Dockerfiles, `nix/`, `scripts/`). + In a **checkout** it's the repo root itself — unchanged behavior. From an + **installed wheel** it stages a copy under the app-data dir, keyed by a + **content digest** of the installed package + bundled resources (not the + distribution version): the installer defaults to a git branch and + `pipx install --force` while `version` stays `0.1.0`, so a version key + would reuse a previous commit's tree — the digest key re-stages instead. + Staging is concurrency-safe: a file lock serializes it, each writer builds + into a private temp dir, and the finished tree is published with an atomic + rename (never populating a shared path another process might read). +- The root-level resources are shipped inside the wheel under + `bot_bottle/_resources/` by a `setup.py` `build_py` step (kept in sync + with `resources.BUNDLED_RESOURCES`); `MANIFEST.in` includes them in the + sdist. +- Every former `_REPO_ROOT` / `_REPO_DIR` call site now derives from + `resources`: the docker/macos agent-image launch, each backend's + `orchestrator` / `gateway` / `infra` service, firecracker `infra_vm` / + `infra_artifact` / `setup`, and the shared `gateway` build context. So + checkout and wheel installs share one downstream path. + +Verification: `test_resources` exercises both layouts — including the staged +wheel context, a re-stage when package content changes at the same version, +and a rebuild of a partial (crashed) stage. `test_wheel_install` builds the +wheel, installs it into an isolated venv, and asserts `bot-bottle doctor` +runs and `build_root()` produces a valid context; `build` is in +`requirements-dev.txt` so it runs in CI, and a build/install failure fails +the test (it does not skip). Running `start` end-to-end still needs a +Docker/KVM host (CI), not a source checkout. ### `install.sh` @@ -78,8 +122,11 @@ A POSIX `sh` bootstrapper that: 1. Checks `python3` is present and ≥ 3.11; exits with a clear message otherwise. -2. Creates `~/.bot-bottle/{agents,bottles,contrib}`. -3. Installs via `pipx` if available, else `python3 -m pip install --user`. +2. Checks `git` when installing a `git+` spec, and — when falling back to + pip — that pip is usable and the interpreter isn't externally managed + (PEP 668), pointing at pipx otherwise. +3. Creates `~/.bot-bottle/{agents,bottles,contrib}`. +4. Installs via `pipx` if available, else `python3 -m pip install --user`. The spec defaults to the git URL and is overridable via `BOT_BOTTLE_INSTALL_SPEC` (used by tests / local installs). 4. Locates the `bot-bottle` entry point (PATH or `~/.local/bin`). diff --git a/install.sh b/install.sh index 667b4b8c..0af73145 100755 --- a/install.sh +++ b/install.sh @@ -40,6 +40,41 @@ want = (int(sys.argv[1]), int(sys.argv[2])) raise SystemExit(0 if sys.version_info[:2] >= want else 1) PY +# Installing a `git+` spec (the default) shells out to git under the hood, +# whether via pipx or pip. Fail early with a clear message rather than deep +# inside the installer's output. +case "${PACKAGE_SPEC}" in + git+*|*.git) + command -v git >/dev/null 2>&1 || die \ + "git is required to install from '${PACKAGE_SPEC}'. Install git, or set "\ +"BOT_BOTTLE_INSTALL_SPEC to a non-git spec (e.g. a wheel path or a package index name)." + ;; +esac + +# The pip fallback needs a usable pip. Externally-managed interpreters +# (PEP 668, common on Debian/Ubuntu/Homebrew) reject `pip install --user`; +# pipx sidesteps that, so recommend it when pip can't be used. +if ! command -v pipx >/dev/null 2>&1; then + python3 -m pip --version >/dev/null 2>&1 || die \ + "neither pipx nor a usable 'python3 -m pip' was found. Install pipx "\ +"(recommended): 'python3 -m pip install --user pipx' or your OS package manager." + if python3 - <<'PY' +import os +import sys +import sysconfig + +# PEP 668: an EXTERNALLY-MANAGED marker in the stdlib dir means pip refuses +# to install into this interpreter without --break-system-packages. +marker = os.path.join(sysconfig.get_path("stdlib"), "EXTERNALLY-MANAGED") +raise SystemExit(0 if os.path.exists(marker) else 1) +PY + then + die "this Python is externally managed (PEP 668), so 'pip install --user' is "\ +"blocked. Install pipx and re-run: 'python3 -m pip install --user --break-system-packages pipx', "\ +"then 'pipx ensurepath'." + fi +fi + # --- config directories ------------------------------------------------------ mkdir -p \ diff --git a/pyproject.toml b/pyproject.toml index 6d600f2c..615afb68 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ include = ["bot_bottle*"] # files shipped under bot_bottle/; test_pyproject.py asserts they exist. [tool.setuptools.package-data] bot_bottle = [ - "egress_entrypoint.sh", + "gateway/egress/entrypoint.sh", "contrib/claude/Dockerfile", "contrib/codex/Dockerfile", "contrib/pi/Dockerfile", diff --git a/requirements-dev.txt b/requirements-dev.txt index 8ba2f162..9772fc41 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,3 +5,6 @@ pylint>=3.0.0 pyright>=1.1.411 coverage>=7.0.0 +# PEP 517 build front-end used by tests/unit/test_wheel_install.py to build and +# install a real wheel (proves the installed distribution is self-contained). +build>=1.0.0 diff --git a/setup.py b/setup.py new file mode 100644 index 00000000..331c9e49 --- /dev/null +++ b/setup.py @@ -0,0 +1,46 @@ +"""Build shim. Project metadata lives in ``pyproject.toml``; this only adds a +build step that copies the root-level build resources (the Dockerfiles, the +nix netpool module, the netpool script, and ``pyproject.toml``) into +``bot_bottle/_resources/`` so an installed wheel is self-contained and can +build its gateway/infra/orchestrator images without a source checkout. + +Kept in sync with ``bot_bottle.resources.BUNDLED_RESOURCES`` — the +``test_resources`` suite guards against drift between the two lists. +""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +from setuptools import setup +from setuptools.command.build_py import build_py + +_ROOT = Path(__file__).resolve().parent + +# Must match bot_bottle.resources.BUNDLED_RESOURCES (paths relative to root). +_BUNDLED_RESOURCES = ( + "pyproject.toml", + "Dockerfile.gateway", + "Dockerfile.orchestrator", + "Dockerfile.orchestrator.fc", + "nix/firecracker-netpool.nix", + "scripts/firecracker-netpool.sh", +) + + +class _BundleResources(build_py): + """Copy the root-level build resources into the built package tree so they + ship inside the wheel under ``bot_bottle/_resources/``.""" + + def run(self) -> None: + super().run() + pkg_resources = Path(self.build_lib) / "bot_bottle" / "_resources" + for rel in _BUNDLED_RESOURCES: + src = _ROOT / rel + dst = pkg_resources / rel + dst.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(src, dst) + + +setup(cmdclass={"build_py": _BundleResources}) diff --git a/tests/unit/test_install_script.py b/tests/unit/test_install_script.py index 48475ed8..3186a5cf 100644 --- a/tests/unit/test_install_script.py +++ b/tests/unit/test_install_script.py @@ -53,6 +53,21 @@ class TestInstallScript(unittest.TestCase): # Tests / local installs point BOT_BOTTLE_INSTALL_SPEC at a checkout. self.assertIn("BOT_BOTTLE_INSTALL_SPEC", self.text) + def test_requires_git_for_git_specs(self): + # A git+ / .git spec (the default) shells out to git; the script must + # gate on it rather than failing opaquely inside pipx/pip. + self.assertIn("command -v git", self.text) + self.assertIn("git+*|*.git", self.text) + + def test_checks_pip_usable_before_fallback(self): + self.assertIn("python3 -m pip --version", self.text) + + def test_detects_externally_managed_python(self): + # PEP 668: 'pip install --user' is blocked on externally-managed + # interpreters; the script must detect this and point at pipx. + self.assertIn("EXTERNALLY-MANAGED", self.text) + self.assertIn("pipx", self.text) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_macos_nested_containers.py b/tests/unit/test_macos_nested_containers.py index c6e5fdb0..731dc320 100644 --- a/tests/unit/test_macos_nested_containers.py +++ b/tests/unit/test_macos_nested_containers.py @@ -283,7 +283,7 @@ class TestBuildOrLoadImages(unittest.TestCase): images = launch_mod.build_or_load_images(plan) build.assert_called_once_with( - "agent:base", launch_mod._REPO_DIR, # pylint: disable=protected-access + "agent:base", str(launch_mod.resources.build_root()), dockerfile="/repo/Dockerfile", ) derived.assert_called_once_with("agent:base", build) diff --git a/tests/unit/test_resources.py b/tests/unit/test_resources.py new file mode 100644 index 00000000..639b2e8a --- /dev/null +++ b/tests/unit/test_resources.py @@ -0,0 +1,175 @@ +"""Unit: bot_bottle.resources — build-resource resolution for both a source +checkout and an installed wheel. + +The checkout path is what the whole test suite already runs under; the wheel +path is exercised here by faking an installed layout (a package dir with a +bundled ``_resources/`` and no sibling Dockerfiles) and asserting that +``build_root()`` stages a repo-root-shaped context. See +``test_wheel_install.py`` for the end-to-end build+install check. +""" + +from __future__ import annotations + +import tempfile +import unittest +from contextlib import contextmanager +from pathlib import Path +from unittest.mock import patch + +from bot_bottle import resources + +from tests.unit import use_bottle_root + + +class TestCheckoutMode(unittest.TestCase): + """The environment the suite runs in: a real source checkout.""" + + def test_is_source_checkout(self): + self.assertTrue(resources.is_source_checkout()) + + def test_build_root_is_repo_root(self): + root = resources.build_root() + self.assertTrue((root / "bot_bottle").is_dir()) + self.assertTrue((root / "pyproject.toml").is_file()) + self.assertTrue((root / "Dockerfile.gateway").is_file()) + + def test_resource_helpers_resolve(self): + self.assertTrue(resources.dockerfile("Dockerfile.gateway").is_file()) + self.assertTrue(resources.nix_netpool_module().is_file()) + self.assertTrue(resources.netpool_script().is_file()) + + def test_bundled_resources_all_exist_at_root(self): + # Drift guard: every path setup.py bundles must exist in the checkout. + root = resources.build_root() + for rel in resources.BUNDLED_RESOURCES: + self.assertTrue((root / rel).is_file(), f"missing bundled resource: {rel}") + + +class TestWheelMode(unittest.TestCase): + """Fake an installed wheel: a package dir with _resources/ and no + checkout Dockerfiles beside it.""" + + def _fake_install(self, tmp: Path) -> Path: + pkg = tmp / "site-packages" / "bot_bottle" + (pkg / "cli").mkdir(parents=True) + (pkg / "__init__.py").write_text("") + (pkg / "cli" / "__init__.py").write_text("# module\n") + bundled = pkg / "_resources" + for rel in resources.BUNDLED_RESOURCES: + dst = bundled / rel + dst.parent.mkdir(parents=True, exist_ok=True) + dst.write_text(f"# fake {rel}\n") + return pkg + + @contextmanager + def _wheel(self): + """Point `resources` at a fake installed wheel with an isolated + app-data dir; yields the package dir so a test can mutate it.""" + with tempfile.TemporaryDirectory() as tmpname: + tmp = Path(tmpname) + pkg = self._fake_install(tmp) + self.addCleanup(use_bottle_root(tmp / "appdata")) + with patch.object(resources, "_PKG", pkg), \ + patch.object(resources, "_BUNDLED", pkg / "_resources"), \ + patch.object(resources, "_CHECKOUT_ROOT", tmp / "no-checkout"): + yield pkg + + def test_stage_and_resolve(self): + with self._wheel(): + self.assertFalse(resources.is_source_checkout()) + root = resources.build_root() + # Staged context looks like a repo root. + self.assertTrue((root / "bot_bottle" / "__init__.py").is_file()) + self.assertTrue((root / "bot_bottle" / "cli" / "__init__.py").is_file()) + self.assertTrue((root / "pyproject.toml").is_file()) + self.assertTrue((root / "Dockerfile.gateway").is_file()) + self.assertTrue((root / "nix" / "firecracker-netpool.nix").is_file()) + self.assertTrue((root / "scripts" / "firecracker-netpool.sh").is_file()) + # The bundled-resource copies are NOT re-nested under the staged + # package (keeps it byte-identical to a checkout package). + self.assertFalse((root / "bot_bottle" / "_resources").exists()) + # Helpers resolve off the staged root. + self.assertEqual(root / "Dockerfile.gateway", + resources.dockerfile("Dockerfile.gateway")) + # Idempotent: second call returns the same completed dir. + self.assertEqual(root, resources.build_root()) + + def test_refreshes_when_content_changes_at_same_version(self): + # Regression for the stale-cache bug: `pipx install --force` of a newer + # commit keeps version 0.1.0, so keying on version would reuse the old + # tree. Keying on content must re-stage when a package file changes. + with self._wheel() as pkg: + root1 = resources.build_root() + self.assertTrue((root1 / ".complete").is_file()) + (pkg / "cli" / "__init__.py").write_text("# new commit, same version\n") + root2 = resources.build_root() + self.assertNotEqual(root1, root2) + self.assertEqual( + "# new commit, same version\n", + (root2 / "bot_bottle" / "cli" / "__init__.py").read_text(), + ) + + def test_rebuilds_when_stage_incomplete(self): + # A crash mid-stage can leave a dir without its `.complete` marker; the + # next call must rebuild it rather than trust the partial tree. + with self._wheel(): + root = resources.build_root() + (root / ".complete").unlink() + (root / "sentinel").write_text("stale") + again = resources.build_root() + self.assertEqual(root, again) # same content digest → same dir + self.assertTrue((again / ".complete").is_file()) + self.assertFalse((again / "sentinel").exists()) # rebuilt clean + + def test_reuses_peer_stage_after_lock_wait(self): + # Regression for the staging race: a caller that loses the lock must, + # once it wins, see the peer's completed tree and reuse it — never + # re-clobber a shared path. Drive it deterministically: hold the lock, + # let a worker block after its fast-path miss, publish a complete tree + # as the "peer", then release so the worker takes the reuse path. + import fcntl + import threading + import time + + with self._wheel(): + base = resources.bot_bottle_root() / "build-root" + base.mkdir(parents=True, exist_ok=True) + dest = base / resources._content_digest() # pylint: disable=protected-access + + result: dict[str, Path] = {} + with open(base / ".stage.lock", "w", encoding="utf-8") as held: + fcntl.flock(held, fcntl.LOCK_EX) + + def worker() -> None: + result["root"] = resources.build_root() + + t = threading.Thread(target=worker) + t.start() + # Let the worker miss the fast path (dest not yet complete) and + # block on the held lock, then publish a complete tree as a peer + # would have and release the lock. + time.sleep(0.3) + dest.mkdir(parents=True) + (dest / ".complete").write_text("") + fcntl.flock(held, fcntl.LOCK_UN) + t.join(timeout=10) + + self.assertEqual(dest, result["root"]) + self.assertFalse(t.is_alive()) + + def test_missing_bundle_raises(self): + with tempfile.TemporaryDirectory() as tmpname: + tmp = Path(tmpname) + pkg = tmp / "bot_bottle" + pkg.mkdir() + restore = use_bottle_root(tmp / "appdata") + self.addCleanup(restore) + with patch.object(resources, "_PKG", pkg), \ + patch.object(resources, "_BUNDLED", pkg / "_resources"), \ + patch.object(resources, "_CHECKOUT_ROOT", tmp / "no-checkout"): + with self.assertRaises(resources.ResourceError): + resources.build_root() + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_wheel_install.py b/tests/unit/test_wheel_install.py new file mode 100644 index 00000000..90065e9a --- /dev/null +++ b/tests/unit/test_wheel_install.py @@ -0,0 +1,119 @@ +"""Integration: build the wheel, install it into an isolated venv, and prove +the installed distribution is self-contained. + +This is the boundary a source-tree existence test can't reach (issue #197 +review): under an installed wheel the package lives in ``site-packages`` with +no repo root above it, so anything resolving Dockerfiles / nix / scripts from +``__file__``'s parents would break. Here we install for real and assert that +``bot-bottle doctor`` runs from the console script and that +``bot_bottle.resources`` stages a valid, repo-root-shaped build context. + +It does NOT run `start` — building images needs a Docker/KVM host (CI). It +skips cleanly when the build/venv toolchain isn't available. +""" + +from __future__ import annotations + +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +class TestWheelInstall(unittest.TestCase): + """`build` is a declared dev dependency (requirements-dev.txt), so this runs + in CI. A build/install failure is a real packaging regression and FAILS — + only genuinely-unsupported infra (no `venv`/`ensurepip`) skips.""" + + _tmp: "tempfile.TemporaryDirectory[str]" + venv_py: Path + app_root: Path + + @classmethod + def setUpClass(cls): + cls._tmp = tempfile.TemporaryDirectory( # pylint: disable=consider-using-with + prefix="bb-wheel-") + tmp = Path(cls._tmp.name) + dist = tmp / "dist" + + # A failed wheel build is exactly the regression this test guards — fail, + # don't skip. `build` is installed via requirements-dev.txt. + built = subprocess.run( + [sys.executable, "-m", "build", "--wheel", "--outdir", str(dist), str(REPO_ROOT)], + capture_output=True, text=True, check=False, + ) + if built.returncode != 0: + raise AssertionError(f"wheel build failed:\n{built.stderr[-2000:]}") + wheels = list(dist.glob("*.whl")) + if not wheels: + raise AssertionError(f"no wheel produced:\n{built.stdout[-2000:]}") + + # A missing `venv`/`ensurepip` is unsupported optional infra, not a + # packaging bug — skip only here. + venv = tmp / "venv" + made = subprocess.run([sys.executable, "-m", "venv", str(venv)], + capture_output=True, text=True, check=False) + if made.returncode != 0: + raise unittest.SkipTest(f"venv/ensurepip unavailable:\n{made.stderr[-1500:]}") + cls.venv_py = venv / "bin" / "python" + + # Installing the freshly-built wheel must succeed — fail if it doesn't. + install = subprocess.run( + [str(cls.venv_py), "-m", "pip", "install", "--quiet", str(wheels[0])], + capture_output=True, text=True, check=False, + ) + if install.returncode != 0: + raise AssertionError(f"pip install of the wheel failed:\n{install.stderr[-2000:]}") + + # Isolate the staged build root the wheel writes under the app-data dir. + cls.app_root = tmp / "appdata" + + @classmethod + def tearDownClass(cls): + cls._tmp.cleanup() + + def _run(self, tail: "list[str]") -> "subprocess.CompletedProcess[str]": + """Run the installed venv's python with `tail` appended, from a neutral + cwd so the source checkout isn't on sys.path — we must import the + *installed* package, not the repo we built from.""" + env = {"BOT_BOTTLE_ROOT": str(self.app_root), "PATH": "/usr/bin:/bin"} + return subprocess.run( + [str(self.venv_py), *tail], + capture_output=True, text=True, env=env, cwd=self._tmp.name, check=False, + ) + + def test_console_entry_point_installed(self): + # The `bot-bottle` script the wheel declares must exist in the venv. + script = self.venv_py.parent / "bot-bottle" + self.assertTrue(script.is_file(), "bot-bottle console script not installed") + + def test_doctor_runs_from_installed_package(self): + proc = self._run(["-m", "bot_bottle.cli", "doctor"]) + # doctor exits non-zero here (no backend), but it must RUN and report. + self.assertIn("python", proc.stdout) + self.assertIn(proc.returncode, (0, 1)) + + def test_installed_wheel_is_self_contained(self): + # From the installed layout (not a checkout), resources must resolve + # Dockerfiles and stage a repo-root-shaped build context. + script = ( + "import bot_bottle.resources as r\n" + "assert not r.is_source_checkout(), 'should not look like a checkout'\n" + "assert r.dockerfile('Dockerfile.gateway').is_file()\n" + "assert r.nix_netpool_module().is_file()\n" + "assert r.netpool_script().is_file()\n" + "root = r.build_root()\n" + "assert (root / 'bot_bottle' / '__init__.py').is_file(), 'no package in context'\n" + "assert (root / 'pyproject.toml').is_file(), 'no pyproject in context'\n" + "assert (root / 'Dockerfile.gateway').is_file(), 'no Dockerfile in context'\n" + "print('SELF_CONTAINED_OK')\n" + ) + proc = self._run(["-c", script]) + self.assertIn("SELF_CONTAINED_OK", proc.stdout, msg=proc.stderr) + + +if __name__ == "__main__": + unittest.main() -- 2.52.0 From beaaf847a621bef1fe3f669b0955fa04f5caaad4 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 26 Jul 2026 02:03:58 +0000 Subject: [PATCH 3/3] fix: doctor probes backend readiness; install.sh resolves user-scripts dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the third review round on PR #481. - `bot-bottle doctor` now checks `is_backend_ready()` (a full backend status() probe: daemon reachable, network pool present, KVM usable) instead of the cheap PATH-only `is_backend_available()`. A host with a stopped Docker daemon or half-configured Firecracker no longer reports `ok: backend` / exit 0 when `start` can't actually work; each not-ready backend prints its own diagnostics, and doctor passes only if at least one backend is ready. - `install.sh` resolves the pip `--user` scripts directory from the interpreter (`sysconfig.get_path("scripts", get_preferred_scheme("user"))`) instead of hardcoding `~/.local/bin`, which is wrong on a python.org macOS interpreter (`~/Library/Python//bin`). The PATH guidance now prints the actual directory. Tests: doctor tests mock `is_backend_ready` (the readiness contract) and cover the not-ready → fail path; a new install-script test drives the macOS `osx_framework_user` scheme and asserts it resolves a non-~/.local/bin directory. Co-Authored-By: Claude Opus 4.8 --- bot_bottle/cli/commands/doctor.py | 29 ++++++++++++-------- docs/prds/prd-new-install-script.md | 22 ++++++++++------ install.sh | 17 +++++++++--- tests/unit/test_cli_doctor.py | 41 ++++++++++++++++++++++------- tests/unit/test_install_script.py | 28 ++++++++++++++++++++ tests/unit/test_resources.py | 9 +++++++ 6 files changed, 113 insertions(+), 33 deletions(-) diff --git a/bot_bottle/cli/commands/doctor.py b/bot_bottle/cli/commands/doctor.py index 1cc218da..0144694d 100644 --- a/bot_bottle/cli/commands/doctor.py +++ b/bot_bottle/cli/commands/doctor.py @@ -2,7 +2,8 @@ bot-bottle and report what's ready. Fails (non-zero exit) only on the two hard requirements: a new-enough -Python and at least one usable backend. The config directory is a soft +Python and at least one backend that is *ready* (passes its full status +checks, so `start` can actually work). The config directory is a soft check — `install.sh` creates it, but a missing one only warrants a note, not a failure, since `start` provisions what it needs on first run. """ @@ -13,7 +14,7 @@ import argparse import sys from pathlib import Path -from ...backend import is_backend_available, known_backend_names +from ...backend import is_backend_ready, known_backend_names from ..constants import PROG MIN_PYTHON = (3, 11) @@ -43,19 +44,25 @@ def _check_python() -> bool: def _check_backends() -> bool: - """At least one backend must be available on this host. Report each - known backend so the operator sees why a missing one is missing.""" - available = [] + """At least one backend must be *ready* to run a bottle — i.e. pass its + full status() checks (daemon reachable, network pool present, KVM usable), + not merely have a binary on PATH. A binary-only check would report `ok` + on a host with a stopped Docker daemon or a half-configured Firecracker, + where `start` still can't work. Each not-ready backend prints its own + diagnostics (quiet=False) so the operator sees exactly what's missing.""" + ready = [] for name in known_backend_names(): - if is_backend_available(name): - available.append(name) - if available: - _ok("backend", f"available: {', '.join(available)}") + if is_backend_ready(name, quiet=False): + _ok("backend", f"{name}: ready") + ready.append(name) + else: + _warn("backend", f"{name}: not ready (see diagnostics above)") + if ready: return True _fail( "backend", - "no backend available; install Apple Container (macOS), " - "Firecracker (Linux), or Docker", + "no backend is ready to run a bottle; start Docker, or finish " + "Apple Container (macOS) / Firecracker (Linux) setup", ) return False diff --git a/docs/prds/prd-new-install-script.md b/docs/prds/prd-new-install-script.md index 87cff452..1009a111 100644 --- a/docs/prds/prd-new-install-script.md +++ b/docs/prds/prd-new-install-script.md @@ -33,7 +33,7 @@ user whether their host is actually ready to run a bottle. `bot-bottle doctor`. It never installs Docker or a VM backend silently and never uses `sudo`. - `install.sh` is idempotent — safe to re-run. -- `bot-bottle doctor` reports Python version, backend availability, and +- `bot-bottle doctor` reports Python version, backend *readiness*, and config-dir presence, exiting non-zero when a hard prerequisite is unmet. - The package keeps **zero runtime pip dependencies** (stdlib-only, matching the existing constraint in `AGENTS.md`). @@ -129,8 +129,11 @@ A POSIX `sh` bootstrapper that: 4. Installs via `pipx` if available, else `python3 -m pip install --user`. The spec defaults to the git URL and is overridable via `BOT_BOTTLE_INSTALL_SPEC` (used by tests / local installs). -4. Locates the `bot-bottle` entry point (PATH or `~/.local/bin`). -5. Runs `bot-bottle doctor` and reports the result. +5. Locates the `bot-bottle` entry point: PATH first, else the + interpreter's own user-scheme scripts dir resolved via `sysconfig` + (`~/.local/bin` on Linux, `~/Library/Python//bin` on a python.org + macOS interpreter — not hardcoded). +6. Runs `bot-bottle doctor` and reports the result. It is idempotent and never calls `sudo`. @@ -140,10 +143,12 @@ A new store-free subcommand (no DB migration required) that checks and reports: - **python** — interpreter version (hard requirement: ≥ 3.11). -- **backend** — at least one backend available on this host - (macos-container / firecracker / docker), reusing - `is_backend_available()` rather than hardcoding Docker, since the - default backend is now a VM backend. Hard requirement. +- **backend** — at least one backend *ready* on this host + (macos-container / firecracker / docker), via `is_backend_ready()` — a + full backend `status()` probe (daemon reachable, network pool present, + KVM usable), not a PATH-only check: a stopped daemon or half-configured + backend must not report `ok` when `start` can't work. Each not-ready + backend prints its own diagnostics. Hard requirement. - **config** — whether `~/.bot-bottle/` exists (advisory only; `start` provisions on first run). @@ -152,7 +157,8 @@ Exits 0 when both hard requirements pass, non-zero otherwise. ## Testing strategy - Unit test `bot-bottle doctor` success/failure paths with backend - availability and Python version mocked. + readiness (`is_backend_ready`) and Python version mocked, including the + available-but-not-ready → fail case. - Unit test that `pyproject.toml` parses, declares the entry point and an empty `dependencies` list, and that every `package-data` glob resolves to a file that exists on disk (guards against drift). diff --git a/install.sh b/install.sh index 0af73145..058c2109 100755 --- a/install.sh +++ b/install.sh @@ -94,13 +94,22 @@ fi # --- locate the entry point -------------------------------------------------- +# The pip --user scripts directory is platform-specific: ~/.local/bin on Linux, +# but ~/Library/Python//bin on a python.org macOS interpreter. Ask the +# interpreter for its own user-scheme scripts dir instead of hardcoding. +USER_SCRIPTS="$(python3 - <<'PY' +import sysconfig +print(sysconfig.get_path("scripts", sysconfig.get_preferred_scheme("user"))) +PY +)" + if command -v bot-bottle >/dev/null 2>&1; then BOT_BOTTLE_BIN="bot-bottle" -elif [ -x "${HOME}/.local/bin/bot-bottle" ]; then - BOT_BOTTLE_BIN="${HOME}/.local/bin/bot-bottle" - say "note: add ${HOME}/.local/bin to your PATH to run 'bot-bottle' directly" +elif [ -n "${USER_SCRIPTS}" ] && [ -x "${USER_SCRIPTS}/bot-bottle" ]; then + BOT_BOTTLE_BIN="${USER_SCRIPTS}/bot-bottle" + say "note: add ${USER_SCRIPTS} to your PATH to run 'bot-bottle' directly" else - die "bot-bottle was installed but is not on PATH; add ~/.local/bin to PATH and re-run" + die "bot-bottle was installed but is not on PATH; add ${USER_SCRIPTS:-your user scripts dir} to PATH and re-run" fi # --- verify ------------------------------------------------------------------ diff --git a/tests/unit/test_cli_doctor.py b/tests/unit/test_cli_doctor.py index 871398ed..7b82d0e3 100644 --- a/tests/unit/test_cli_doctor.py +++ b/tests/unit/test_cli_doctor.py @@ -2,8 +2,12 @@ `doctor` is a store-free diagnostic — it must run on a fresh install before any DB migration, and its exit code gates only the two hard -prerequisites (Python and an available backend). The config-dir check is -advisory and never affects the exit code. +prerequisites (Python and at least one *ready* backend). The config-dir +check is advisory and never affects the exit code. + +Backend readiness is probed with `is_backend_ready()` (a full status() +check), not the cheap PATH-only `is_backend_available()` — a host with a +stopped daemon or half-configured backend must not report `ok`. """ from __future__ import annotations @@ -26,26 +30,43 @@ def _run(argv: list[str] | None = None) -> tuple[int, str]: class TestDoctor(unittest.TestCase): - def test_passes_when_python_and_backend_ok(self): + def test_passes_when_python_and_backend_ready(self): with patch.object(doctor, "known_backend_names", return_value=("docker",)), \ - patch.object(doctor, "is_backend_available", return_value=True): + patch.object(doctor, "is_backend_ready", return_value=True): code, out = _run() self.assertEqual(0, code) self.assertIn("ok: python", out) - self.assertIn("ok: backend: available: docker", out) + self.assertIn("ok: backend: docker: ready", out) - def test_fails_when_no_backend_available(self): + def test_fails_when_no_backend_ready(self): + # The regression the reviewer flagged: a backend whose binary is on PATH + # but whose daemon/pool isn't ready must NOT pass. is_backend_ready is + # the full status() check, so returning False here means "not ready". with patch.object(doctor, "known_backend_names", return_value=("docker", "firecracker")), \ - patch.object(doctor, "is_backend_available", return_value=False): + patch.object(doctor, "is_backend_ready", return_value=False): code, out = _run() self.assertEqual(1, code) self.assertIn("fail: backend", out) + self.assertIn("warn: backend: docker: not ready", out) + + def test_passes_when_at_least_one_backend_ready(self): + # docker not ready, firecracker ready → overall pass, mixed report. + def ready(name: str, *, quiet: bool = False) -> bool: + del quiet + return name == "firecracker" + + with patch.object(doctor, "known_backend_names", return_value=("docker", "firecracker")), \ + patch.object(doctor, "is_backend_ready", side_effect=ready): + code, out = _run() + self.assertEqual(0, code) + self.assertIn("warn: backend: docker: not ready", out) + self.assertIn("ok: backend: firecracker: ready", out) def test_fails_when_python_too_old(self): # Force the version gate to fail without touching the interpreter. with patch.object(doctor, "MIN_PYTHON", (99, 0)), \ patch.object(doctor, "known_backend_names", return_value=("docker",)), \ - patch.object(doctor, "is_backend_available", return_value=True): + patch.object(doctor, "is_backend_ready", return_value=True): code, out = _run() self.assertEqual(1, code) self.assertIn("fail: python", out) @@ -57,7 +78,7 @@ class TestDoctor(unittest.TestCase): with tempfile.TemporaryDirectory() as tmp, \ patch.object(doctor.Path, "home", return_value=Path(tmp)), \ patch.object(doctor, "known_backend_names", return_value=("docker",)), \ - patch.object(doctor, "is_backend_available", return_value=True): + patch.object(doctor, "is_backend_ready", return_value=True): code, out = _run() self.assertEqual(0, code) self.assertIn("warn: config", out) @@ -66,7 +87,7 @@ class TestDoctor(unittest.TestCase): with tempfile.TemporaryDirectory() as tmp, \ patch.object(doctor.Path, "home", return_value=Path(tmp)), \ patch.object(doctor, "known_backend_names", return_value=("docker",)), \ - patch.object(doctor, "is_backend_available", return_value=True): + patch.object(doctor, "is_backend_ready", return_value=True): (Path(tmp) / ".bot-bottle").mkdir() code, out = _run() self.assertEqual(0, code) diff --git a/tests/unit/test_install_script.py b/tests/unit/test_install_script.py index 3186a5cf..a0a27905 100644 --- a/tests/unit/test_install_script.py +++ b/tests/unit/test_install_script.py @@ -9,6 +9,7 @@ create the config tree, install the package, and verify with `doctor`. from __future__ import annotations import os +import sysconfig import unittest from pathlib import Path @@ -68,6 +69,33 @@ class TestInstallScript(unittest.TestCase): self.assertIn("EXTERNALLY-MANAGED", self.text) self.assertIn("pipx", self.text) + def test_resolves_user_scripts_dir_not_hardcoded(self): + # The pip --user scripts dir differs by platform; the script must ask + # the interpreter (sysconfig + the preferred *user* scheme) rather than + # hardcoding Linux's ~/.local/bin (which is wrong on macOS python.org). + self.assertIn("get_preferred_scheme", self.text) + self.assertIn("sysconfig", self.text) + # No hardcoded Linux path in executable lines (a comment may mention it). + code = "\n".join( + ln for ln in self.text.splitlines() + if ln.strip() and not ln.lstrip().startswith("#") + ) + self.assertNotIn(".local/bin", code) + + def test_macos_user_scheme_is_not_dot_local_bin(self): + # The case the fix exists for: a python.org macOS interpreter uses the + # osx_framework_user scheme, whose scripts land under + # ~/Library/Python//bin — NOT ~/.local/bin. Drive the same + # sysconfig lookup install.sh uses, with a mac-like userbase, to prove + # it resolves a non-~/.local/bin directory. + self.assertIn("osx_framework_user", sysconfig.get_scheme_names()) + scripts = sysconfig.get_path( + "scripts", "osx_framework_user", + vars={"userbase": "/Users/dev/Library/Python/3.11"}, + ) + self.assertEqual("/Users/dev/Library/Python/3.11/bin", scripts) + self.assertNotIn("/.local/bin", scripts) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_resources.py b/tests/unit/test_resources.py index 639b2e8a..110353aa 100644 --- a/tests/unit/test_resources.py +++ b/tests/unit/test_resources.py @@ -109,6 +109,15 @@ class TestWheelMode(unittest.TestCase): (root2 / "bot_bottle" / "cli" / "__init__.py").read_text(), ) + def test_failed_stage_cleans_up_temp_dir(self): + # A failure mid-stage must not leave a half-written temp dir behind. + with self._wheel(): + base = resources.bot_bottle_root() / "build-root" + with patch.object(resources.shutil, "copytree", side_effect=OSError("boom")): + with self.assertRaises(OSError): + resources.build_root() + self.assertEqual([], list(base.glob(".staging-*"))) + def test_rebuilds_when_stage_incomplete(self): # A crash mid-stage can leave a dir without its `.complete` marker; the # next call must rebuild it rather than trust the partial tree. -- 2.52.0