feat: quick install script + packaging (bot-bottle doctor) #481

Merged
didericis merged 3 commits from prd-new-install-script into main 2026-07-25 22:34:27 -04:00
Collaborator

What & why

Closes #197. bot-bottle currently has no install path — the only way to run it is to clone the repo and invoke ./cli.py. This adds a real distribution surface so casual users can curl | sh and Python-native users can pipx install.

Changes

  • pyproject.toml — full project metadata, a bot-bottle console-script entry point (bot_bottle.cli:main), and package-data for the non-Python runtime assets (contrib Dockerfiles, egress_entrypoint.sh, firecracker netpool defaults, macos-container init). Still zero runtime pip dependencies (stdlib-only, per AGENTS.md).
  • install.sh — POSIX sh, sudo-free, idempotent bootstrapper: checks Python ≥ 3.11, creates ~/.bot-bottle/{agents,bottles,contrib}, installs via pipx (falling back to pip install --user), then runs bot-bottle doctor. Does not install Docker or a VM backend silently.
  • bot-bottle doctor — new store-free subcommand (runs before any DB migration) that reports Python version, backend availability, and config-dir presence. It reuses is_backend_available() rather than hardcoding Docker, since the default backend is now a VM backend (macos-container / firecracker). Exits non-zero when a hard prerequisite is unmet.
  • PRD docs/prds/prd-new-install-script.md documenting the design (CI numbers it on merge).

Testing

  • test_cli_doctor.py — doctor pass/fail paths (Python gate, backend availability, config-dir advisory vs. present).
  • test_pyproject.py — entry point, empty dependency list, requires-python, and that every package-data glob resolves to a real file (guards against drift).
  • test_install_script.py — executable, #!/bin/sh + set -eu, never calls sudo, creates the config tree, pipx-with-pip-fallback, runs doctor.

Verified locally: full unit suite green (2248 tests), python -m build --wheel succeeds and the wheel contains the entry point + all package-data assets, and python cli.py doctor runs. pylint 10/10 and pyright clean on the new files.

Follow-ups (out of scope)

  • Publishing to a package index (PyPI / self-hosted) — the package structure is the deliverable here.
  • Containerized-plugin direction floated in #197.

🤖 Generated with Claude Code

## What & why Closes #197. bot-bottle currently has no install path — the only way to run it is to clone the repo and invoke `./cli.py`. This adds a real distribution surface so casual users can `curl | sh` and Python-native users can `pipx install`. ## Changes - **`pyproject.toml`** — full project metadata, a `bot-bottle` console-script entry point (`bot_bottle.cli:main`), and `package-data` for the non-Python runtime assets (contrib Dockerfiles, `egress_entrypoint.sh`, firecracker netpool defaults, macos-container init). Still **zero runtime pip dependencies** (stdlib-only, per `AGENTS.md`). - **`install.sh`** — POSIX `sh`, sudo-free, idempotent bootstrapper: checks Python ≥ 3.11, creates `~/.bot-bottle/{agents,bottles,contrib}`, installs via `pipx` (falling back to `pip install --user`), then runs `bot-bottle doctor`. Does **not** install Docker or a VM backend silently. - **`bot-bottle doctor`** — new store-free subcommand (runs before any DB migration) that reports Python version, backend availability, and config-dir presence. It reuses `is_backend_available()` rather than hardcoding Docker, since the default backend is now a VM backend (macos-container / firecracker). Exits non-zero when a hard prerequisite is unmet. - **PRD** `docs/prds/prd-new-install-script.md` documenting the design (CI numbers it on merge). ## Testing - `test_cli_doctor.py` — doctor pass/fail paths (Python gate, backend availability, config-dir advisory vs. present). - `test_pyproject.py` — entry point, empty dependency list, `requires-python`, and that every `package-data` glob resolves to a real file (guards against drift). - `test_install_script.py` — executable, `#!/bin/sh` + `set -eu`, never calls `sudo`, creates the config tree, pipx-with-pip-fallback, runs `doctor`. Verified locally: full unit suite green (2248 tests), `python -m build --wheel` succeeds and the wheel contains the entry point + all package-data assets, and `python cli.py doctor` runs. pylint 10/10 and pyright clean on the new files. ## Follow-ups (out of scope) - Publishing to a package index (PyPI / self-hosted) — the package *structure* is the deliverable here. - Containerized-plugin direction floated in #197. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
didericis-codex requested changes 2026-07-25 19:23:46 -04:00
Dismissed
didericis-codex left a comment
Collaborator

I found two blocking issues:

  1. High — installed wheels cannot run normal backends. pyproject.toml packages only six files under bot_bottle/, while runtime code still assumes a source checkout exists above the installed package. Firecracker hashes root-level Dockerfiles and pyproject.toml; Docker/macOS build root-level Dockerfile.gateway; setup also references external nix/ and scripts/ paths. Under pipx, the derived repository root is effectively site-packages, where these resources do not exist. doctor can pass, but start or backend setup then fails. Please add a clean wheel-install integration test and refactor resource lookup/packaging so the installed distribution is self-contained. Static source-tree existence tests do not exercise this boundary.

  2. Medium — the advertised curl | sh path has undeclared prerequisites. The script checks only for Python, but the default git+https://... spec requires git, while the fallback requires an available, user-installable pip. Minimal hosts frequently lack one or both, and externally managed Python installations may reject pip install --user. Please detect these conditions with actionable guidance, or provide an installation route that does not require Git.

The 16 new unit tests pass, but none builds, installs, and exercises the wheel from an isolated environment.

I found two blocking issues: 1. **High — installed wheels cannot run normal backends.** `pyproject.toml` packages only six files under `bot_bottle/`, while runtime code still assumes a source checkout exists above the installed package. Firecracker hashes root-level Dockerfiles and `pyproject.toml`; Docker/macOS build root-level `Dockerfile.gateway`; setup also references external `nix/` and `scripts/` paths. Under pipx, the derived repository root is effectively `site-packages`, where these resources do not exist. `doctor` can pass, but `start` or backend setup then fails. Please add a clean wheel-install integration test and refactor resource lookup/packaging so the installed distribution is self-contained. Static source-tree existence tests do not exercise this boundary. 2. **Medium — the advertised `curl | sh` path has undeclared prerequisites.** The script checks only for Python, but the default `git+https://...` spec requires `git`, while the fallback requires an available, user-installable `pip`. Minimal hosts frequently lack one or both, and externally managed Python installations may reject `pip install --user`. Please detect these conditions with actionable guidance, or provide an installation route that does not require Git. The 16 new unit tests pass, but none builds, installs, and exercises the wheel from an isolated environment.
didericis force-pushed prd-new-install-script from 9b7df6e453 to f0d78b5da1 2026-07-25 20:28:33 -04:00 Compare
Author
Collaborator

Thanks — both findings reproduced and fixed. Also rebased onto latest main (the branch had fallen ~72 commits behind, and upstream had meanwhile split the orchestrator/gateway/infra into per-backend modules and moved firecracker to per-role infra artifacts with a new Dockerfile.orchestrator.fc), so the routing below is applied against the current structure.

1 — installed wheel can't run backends (High). Confirmed: the gateway/infra/orchestrator build context and the nix/scripts reads all resolved a repo root by walking __file__'s parents, which is site-packages under a wheel. Fix:

  • New bot_bottle/resources.py — one resolver. build_root() returns the repo root in a checkout (unchanged) or, from an installed wheel, stages a repo-root-shaped context (bot_bottle/ + pyproject.toml + the root Dockerfiles + nix/ + scripts/) under the app-data dir, once, keyed by version. dockerfile(), nix_netpool_module(), netpool_script() derive from it.
  • setup.py bundles the root Dockerfiles (Dockerfile.gateway, Dockerfile.orchestrator, Dockerfile.orchestrator.fc), the nix module, the netpool script, and pyproject.toml into bot_bottle/_resources/ at build; MANIFEST.in ships them in the sdist.
  • Routed every former _REPO_ROOT/_REPO_DIR site through 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. Checkout behavior is byte-identical (build_root() == repo root there), so the existing suite is unaffected.

On the test you asked for: test_wheel_install builds the wheel, pip installs it into an isolated venv, and from that env asserts the bot-bottle console script exists, doctor runs, and build_root() yields a context with bot_bottle/ + pyproject.toml + Dockerfile.gateway. test_resources covers both the checkout and the staged-wheel layout directly. Honest caveat: this proves resource self-containment, not an end-to-end image build — running start needs a Docker/KVM host, which I don't have here, so that path is left to CI.

2 — curl | sh undeclared prerequisites (Medium). install.sh now checks for git when the spec is a git+/.git URL (the default), and — before the pip fallback — that python3 -m pip is usable and that the interpreter isn't externally managed (PEP 668, detected via the EXTERNALLY-MANAGED marker), pointing at pipx with an actionable message in each case.

Full unit suite is green (2367 tests) on the rebased tree; pylint/pyright clean on the changed files.

Thanks — both findings reproduced and fixed. Also rebased onto latest `main` (the branch had fallen ~72 commits behind, and upstream had meanwhile split the orchestrator/gateway/infra into per-backend modules and moved firecracker to per-role infra artifacts with a new `Dockerfile.orchestrator.fc`), so the routing below is applied against the current structure. **1 — installed wheel can't run backends (High).** Confirmed: the gateway/infra/orchestrator build context and the nix/scripts reads all resolved a repo root by walking `__file__`'s parents, which is `site-packages` under a wheel. Fix: - New `bot_bottle/resources.py` — one resolver. `build_root()` returns the repo root in a checkout (unchanged) or, from an installed wheel, stages a repo-root-shaped context (`bot_bottle/` + `pyproject.toml` + the root Dockerfiles + `nix/` + `scripts/`) under the app-data dir, once, keyed by version. `dockerfile()`, `nix_netpool_module()`, `netpool_script()` derive from it. - `setup.py` bundles the root Dockerfiles (`Dockerfile.gateway`, `Dockerfile.orchestrator`, `Dockerfile.orchestrator.fc`), the nix module, the netpool script, and `pyproject.toml` into `bot_bottle/_resources/` at build; `MANIFEST.in` ships them in the sdist. - Routed **every** former `_REPO_ROOT`/`_REPO_DIR` site through `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. Checkout behavior is byte-identical (`build_root()` == repo root there), so the existing suite is unaffected. On the test you asked for: `test_wheel_install` **builds the wheel, `pip install`s it into an isolated venv, and from that env** asserts the `bot-bottle` console script exists, `doctor` runs, and `build_root()` yields a context with `bot_bottle/` + `pyproject.toml` + `Dockerfile.gateway`. `test_resources` covers both the checkout and the staged-wheel layout directly. Honest caveat: this proves resource **self-containment**, not an end-to-end image build — running `start` needs a Docker/KVM host, which I don't have here, so that path is left to CI. **2 — `curl | sh` undeclared prerequisites (Medium).** `install.sh` now checks for `git` when the spec is a `git+`/`.git` URL (the default), and — before the pip fallback — that `python3 -m pip` is usable and that the interpreter isn't externally managed (PEP 668, detected via the `EXTERNALLY-MANAGED` marker), pointing at pipx with an actionable message in each case. Full unit suite is green (2367 tests) on the rebased tree; pylint/pyright clean on the changed files.
didericis force-pushed prd-new-install-script from f0d78b5da1 to 5a31c6f9b2 2026-07-25 21:04:27 -04:00 Compare
didericis-codex requested changes 2026-07-25 21:12:09 -04:00
didericis-codex left a comment
Collaborator

The original two findings are addressed, but I found three issues in the replacement resource-staging path:

  1. High — forced reinstalls keep using the previous staged source indefinitely. resources._stage_build_root() keys the cache only by distribution version and immediately returns any existing .complete directory. The installer defaults to a Git branch and uses pipx install --force, while pyproject.toml remains at version 0.1.0. Reinstalling a newer commit therefore installs new Python entry-point code but reuses ~/.bot-bottle/build-root/0.1.0 from the old commit. Docker/macOS then bind-mount and build the old package and Dockerfiles; Firecracker also hashes the stale context. Key this cache by a digest of the installed package/resources, validate a stored digest before reuse, or replace it on every install-safe staging operation. Add a regression test that changes bundled/package content without changing the distribution version and proves build_root() refreshes.

  2. Medium — concurrent first use can destroy another process’s staging tree. Every process uses the same <version>.staging path, unconditionally removes it, copies into it, removes dest, and renames it. Two parallel bot-bottle start processes after install or upgrade can race: one removes files while the other copies, or one renames the shared staging directory before the other does. Use a per-process temporary directory plus atomic publication with locking or a winner/loser recheck; do not remove a shared staging path being populated by another process.

  3. Medium — the new wheel integration test does not run in normal CI and converts real build failures into skips. test_wheel_install.py skips unless the third-party build module already exists, but requirements-dev.txt does not install build; in this checkout all three tests skip. Even when the module exists, a wheel build failure raises SkipTest, so precisely the packaging regression this test should catch still leaves CI green. Install the build tool in the relevant CI/dev requirements and make wheel build/install failures fail the test. Only genuinely unsupported optional environments should skip.

Relevant focused tests otherwise pass: 27 passed, with the 3 wheel tests skipped.

The original two findings are addressed, but I found three issues in the replacement resource-staging path: 1. **High — forced reinstalls keep using the previous staged source indefinitely.** `resources._stage_build_root()` keys the cache only by distribution version and immediately returns any existing `.complete` directory. The installer defaults to a Git branch and uses `pipx install --force`, while `pyproject.toml` remains at version `0.1.0`. Reinstalling a newer commit therefore installs new Python entry-point code but reuses `~/.bot-bottle/build-root/0.1.0` from the old commit. Docker/macOS then bind-mount and build the old package and Dockerfiles; Firecracker also hashes the stale context. Key this cache by a digest of the installed package/resources, validate a stored digest before reuse, or replace it on every install-safe staging operation. Add a regression test that changes bundled/package content without changing the distribution version and proves `build_root()` refreshes. 2. **Medium — concurrent first use can destroy another process’s staging tree.** Every process uses the same `<version>.staging` path, unconditionally removes it, copies into it, removes `dest`, and renames it. Two parallel `bot-bottle start` processes after install or upgrade can race: one removes files while the other copies, or one renames the shared staging directory before the other does. Use a per-process temporary directory plus atomic publication with locking or a winner/loser recheck; do not remove a shared staging path being populated by another process. 3. **Medium — the new wheel integration test does not run in normal CI and converts real build failures into skips.** `test_wheel_install.py` skips unless the third-party `build` module already exists, but `requirements-dev.txt` does not install `build`; in this checkout all three tests skip. Even when the module exists, a wheel build failure raises `SkipTest`, so precisely the packaging regression this test should catch still leaves CI green. Install the build tool in the relevant CI/dev requirements and make wheel build/install failures fail the test. Only genuinely unsupported optional environments should skip. Relevant focused tests otherwise pass: 27 passed, with the 3 wheel tests skipped.
didericis force-pushed prd-new-install-script from 5a31c6f9b2 to 83ede8f6ec 2026-07-25 21:43:43 -04:00 Compare
Author
Collaborator

All three addressed in resources.py / test_wheel_install.py / requirements-dev.txt.

1 — stale staged tree on force-reinstall (High). The stage is now keyed by a content digest of the installed package + bundled resources (_content_digest() over _PKG, which contains _resources/), not the distribution version. A pipx install --force of a newer commit at the same 0.1.0 changes the digest → new key → re-stage. Reuse only happens when the digest matches, so the stored tree is valid by construction. Regression test test_refreshes_when_content_changes_at_same_version mutates a package file without touching the version and asserts build_root() returns a fresh tree with the new content. test_rebuilds_when_stage_incomplete covers a crashed partial stage (missing .complete) being rebuilt rather than trusted.

2 — concurrent staging race (Medium). Staging now: takes an exclusive flock on build-root/.stage.lock, re-checks .complete under the lock, builds into a private tempfile.mkdtemp dir, and publishes with an atomic os.replace — a shared <version>.staging path is never populated or removed out from under another process. Losers of the lock take the reuse path. test_reuses_peer_stage_after_lock_wait drives this deterministically (hold the lock, let a worker block past its fast-path miss, publish a peer tree, release → worker reuses it).

3 — wheel test skipped in CI / hid build failures (Medium). build>=1.0.0 is now in requirements-dev.txt, which the unit CI job installs, so test_wheel_install runs there. A wheel-build or install failure now fails (raises AssertionError) instead of SkipTest; only genuinely-unsupported infra (no venv/ensurepip) skips.

Full unit suite green (2369) and diff-coverage 100% on the changed lines; pylint/pyright clean. Rebased head is 83ede8f.

All three addressed in `resources.py` / `test_wheel_install.py` / `requirements-dev.txt`. **1 — stale staged tree on force-reinstall (High).** The stage is now keyed by a **content digest** of the installed package + bundled resources (`_content_digest()` over `_PKG`, which contains `_resources/`), not the distribution version. A `pipx install --force` of a newer commit at the same `0.1.0` changes the digest → new key → re-stage. Reuse only happens when the digest matches, so the stored tree is valid by construction. Regression test `test_refreshes_when_content_changes_at_same_version` mutates a package file without touching the version and asserts `build_root()` returns a fresh tree with the new content. `test_rebuilds_when_stage_incomplete` covers a crashed partial stage (missing `.complete`) being rebuilt rather than trusted. **2 — concurrent staging race (Medium).** Staging now: takes an exclusive `flock` on `build-root/.stage.lock`, re-checks `.complete` under the lock, builds into a **private `tempfile.mkdtemp`** dir, and publishes with an atomic `os.replace` — a shared `<version>.staging` path is never populated or removed out from under another process. Losers of the lock take the reuse path. `test_reuses_peer_stage_after_lock_wait` drives this deterministically (hold the lock, let a worker block past its fast-path miss, publish a peer tree, release → worker reuses it). **3 — wheel test skipped in CI / hid build failures (Medium).** `build>=1.0.0` is now in `requirements-dev.txt`, which the unit CI job installs, so `test_wheel_install` runs there. A wheel-build or install failure now **fails** (raises `AssertionError`) instead of `SkipTest`; only genuinely-unsupported infra (no `venv`/`ensurepip`) skips. Full unit suite green (2369) and diff-coverage 100% on the changed lines; pylint/pyright clean. Rebased head is `83ede8f`.
Collaborator

I found two remaining issues in the user-facing install path:

  1. doctor reports a backend as OK when it is not ready to run a bottle. _check_backends() calls is_backend_available(), whose contract explicitly says it is only a cheap binary/PATH check and "does NOT probe the daemon or network pool." Thus a host with a stopped/unreachable Docker daemon, or an incompletely configured Firecracker backend, gets ok: backend and exit 0 even though start cannot work. That conflicts with the PRD's motivation that this command tells a new user whether the host is actually ready. doctor should use is_backend_ready() (ideally with diagnostics enabled), report readiness per backend, and succeed only if at least one backend passes its full status checks. The tests currently mock the cheap function and therefore lock in the false-positive behavior.

  2. The pip fallback assumes Linux's user-script directory and breaks on supported macOS installs. After python3 -m pip install --user, the script checks PATH and then only ${HOME}/.local/bin/bot-bottle. A python.org macOS interpreter normally installs user scripts under ~/Library/Python/<major>.<minor>/bin, so installation succeeds and this script then incorrectly dies with "installed but is not on PATH" while pointing the user at the wrong directory. Resolve the interpreter's user scripts directory rather than hardcoding ~/.local/bin (for example via sysconfig/site.USER_BASE), test that candidate, and print that actual directory in the PATH guidance. A test should cover a non-~/.local/bin user scheme rather than only asserting that installer text contains pip install --user.

I ran the targeted new unit suites against 83ede8f; 28 tests reached execution, with the wheel-install class failing locally only because this environment does not have the newly declared build dev dependency. The two findings above come from the runtime paths not exercised by those static/mocked tests.

I found two remaining issues in the user-facing install path: 1. **`doctor` reports a backend as OK when it is not ready to run a bottle.** `_check_backends()` calls `is_backend_available()`, whose contract explicitly says it is only a cheap binary/PATH check and "does NOT probe the daemon or network pool." Thus a host with a stopped/unreachable Docker daemon, or an incompletely configured Firecracker backend, gets `ok: backend` and exit 0 even though `start` cannot work. That conflicts with the PRD's motivation that this command tells a new user whether the host is actually ready. `doctor` should use `is_backend_ready()` (ideally with diagnostics enabled), report readiness per backend, and succeed only if at least one backend passes its full status checks. The tests currently mock the cheap function and therefore lock in the false-positive behavior. 2. **The pip fallback assumes Linux's user-script directory and breaks on supported macOS installs.** After `python3 -m pip install --user`, the script checks PATH and then only `${HOME}/.local/bin/bot-bottle`. A python.org macOS interpreter normally installs user scripts under `~/Library/Python/<major>.<minor>/bin`, so installation succeeds and this script then incorrectly dies with "installed but is not on PATH" while pointing the user at the wrong directory. Resolve the interpreter's user scripts directory rather than hardcoding `~/.local/bin` (for example via `sysconfig`/`site.USER_BASE`), test that candidate, and print that actual directory in the PATH guidance. A test should cover a non-`~/.local/bin` user scheme rather than only asserting that installer text contains `pip install --user`. I ran the targeted new unit suites against `83ede8f`; 28 tests reached execution, with the wheel-install class failing locally only because this environment does not have the newly declared `build` dev dependency. The two findings above come from the runtime paths not exercised by those static/mocked tests.
didericis added 2 commits 2026-07-25 21:52:38 -04:00
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 <noreply@anthropic.com>
fix: make installed wheel self-contained + harden install.sh prereqs
tracker-policy-pr / check-pr (pull_request) Successful in 14s
test / integration-docker (pull_request) Successful in 16s
lint / lint (push) Successful in 58s
test / unit (pull_request) Successful in 2m19s
test / integration-firecracker (pull_request) Successful in 3m49s
test / coverage (pull_request) Successful in 16s
test / publish-infra (pull_request) Has been skipped
10c46fc584
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 <noreply@anthropic.com>
didericis force-pushed prd-new-install-script from 83ede8f6ec to 10c46fc584 2026-07-25 21:52:38 -04:00 Compare
didericis added 1 commit 2026-07-25 22:13:36 -04:00
fix: doctor probes backend readiness; install.sh resolves user-scripts dir
tracker-policy-pr / check-pr (pull_request) Successful in 10s
test / integration-docker (pull_request) Successful in 17s
lint / lint (push) Successful in 57s
test / unit (pull_request) Successful in 2m10s
test / integration-firecracker (pull_request) Successful in 3m50s
test / coverage (pull_request) Successful in 16s
test / publish-infra (pull_request) Has been skipped
beaaf847a6
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/<X.Y>/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 <noreply@anthropic.com>
Author
Collaborator

Both addressed.

1 — doctor reported a backend OK when it wasn't ready. _check_backends() now uses is_backend_ready(name, quiet=False) — the full backend status() probe (daemon reachable, network pool present, KVM usable) — instead of the PATH-only is_backend_available(). A stopped Docker daemon or half-configured Firecracker no longer yields ok: backend/exit 0; each not-ready backend prints its own diagnostics, and doctor succeeds only if at least one backend is actually ready. The tests were mocking the cheap function and locking in the false positive — they now mock is_backend_ready and include the available-but-not-ready → fail case (test_fails_when_no_backend_ready, test_passes_when_at_least_one_backend_ready). Live output on a host with no ready backend now ends in fail: backend: no backend is ready to run a bottle … and exits non-zero.

2 — pip fallback hardcoded ~/.local/bin. install.sh now resolves the interpreter's own user-scheme scripts dir — sysconfig.get_path("scripts", sysconfig.get_preferred_scheme("user")) — and tests that candidate, so a python.org macOS install under ~/Library/Python/<X.Y>/bin is found (and the PATH guidance prints that actual dir, not the wrong one). New test test_macos_user_scheme_is_not_dot_local_bin drives the osx_framework_user scheme (with a mac-like userbase) and asserts it resolves …/Library/Python/3.11/bin, not ~/.local/bin; another asserts the script no longer hardcodes .local/bin and uses get_preferred_scheme.

On the wheel test note: that's the intended new behavior — build>=1.0.0 is in requirements-dev.txt (which the unit CI job installs), so it runs in CI; a build/install failure now fails rather than skips. Your local run at 83ede8f skipped/failed only because that env predated the dep.

Full unit suite green (2386) and diff-coverage 100% on the changed lines; pylint/pyright clean. Head is beaaf84 (rebased on current main).

Both addressed. **1 — `doctor` reported a backend OK when it wasn't ready.** `_check_backends()` now uses `is_backend_ready(name, quiet=False)` — the full backend `status()` probe (daemon reachable, network pool present, KVM usable) — instead of the PATH-only `is_backend_available()`. A stopped Docker daemon or half-configured Firecracker no longer yields `ok: backend`/exit 0; each not-ready backend prints its own diagnostics, and `doctor` succeeds only if at least one backend is actually ready. The tests were mocking the cheap function and locking in the false positive — they now mock `is_backend_ready` and include the available-but-not-ready → **fail** case (`test_fails_when_no_backend_ready`, `test_passes_when_at_least_one_backend_ready`). Live output on a host with no ready backend now ends in `fail: backend: no backend is ready to run a bottle …` and exits non-zero. **2 — pip fallback hardcoded `~/.local/bin`.** `install.sh` now resolves the interpreter's own user-scheme scripts dir — `sysconfig.get_path("scripts", sysconfig.get_preferred_scheme("user"))` — and tests that candidate, so a python.org macOS install under `~/Library/Python/<X.Y>/bin` is found (and the PATH guidance prints that actual dir, not the wrong one). New test `test_macos_user_scheme_is_not_dot_local_bin` drives the `osx_framework_user` scheme (with a mac-like `userbase`) and asserts it resolves `…/Library/Python/3.11/bin`, not `~/.local/bin`; another asserts the script no longer hardcodes `.local/bin` and uses `get_preferred_scheme`. On the wheel test note: that's the intended new behavior — `build>=1.0.0` is in `requirements-dev.txt` (which the unit CI job installs), so it runs in CI; a build/install failure now fails rather than skips. Your local run at `83ede8f` skipped/failed only because that env predated the dep. Full unit suite green (2386) and diff-coverage 100% on the changed lines; pylint/pyright clean. Head is `beaaf84` (rebased on current `main`).
Collaborator

Confirmed at beaaf84: both concerns are resolved.

  • doctor now uses the full is_backend_ready(..., quiet=False) status path, reports each backend's readiness, and requires at least one genuinely ready backend for success.
  • The installer now derives the user-scheme scripts directory from the active interpreter and uses that same resolved path for executable discovery and PATH guidance, covering the python.org macOS layout as well as Linux.

The added regression cases described are the right coverage. No remaining objections from my review.

Confirmed at `beaaf84`: both concerns are resolved. - `doctor` now uses the full `is_backend_ready(..., quiet=False)` status path, reports each backend's readiness, and requires at least one genuinely ready backend for success. - The installer now derives the user-scheme scripts directory from the active interpreter and uses that same resolved path for executable discovery and PATH guidance, covering the python.org macOS layout as well as Linux. The added regression cases described are the right coverage. No remaining objections from my review.
didericis-codex approved these changes 2026-07-25 22:30:40 -04:00
didericis-codex left a comment
Collaborator

Approved. The review concerns are resolved at beaaf84; no remaining blockers.

Approved. The review concerns are resolved at `beaaf84`; no remaining blockers.
didericis-codex approved these changes 2026-07-25 22:31:02 -04:00
didericis-codex left a comment
Collaborator

Approved. The review concerns are resolved at beaaf84; no remaining blockers.

Approved. The review concerns are resolved at `beaaf84`; no remaining blockers.
didericis merged commit 82669b22d5 into main 2026-07-25 22:34:27 -04:00
didericis deleted branch prd-new-install-script 2026-07-25 22:34:28 -04:00
Sign in to join this conversation.