From 556217ae7b1f7241f44298d5d5e6cd40c888fea6 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 27 Jul 2026 09:45:57 -0400 Subject: [PATCH] fix(harness): judge the install separately from backend readiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first run against the branch's own code confirmed the doctor fix — no traceback, the firecracker probes now report "TAP pool: 0/8" instead of dying on PermissionError — and then failed for a reason the installer cannot fix. The Apple `container` service is per-USER. `container system status` reports an appRoot under ~/Library/Application Support/com.apple.container/, and bbtest sees "container system service: NOT running" while the creating admin's is running. A brand-new account therefore has no backend until it runs `container system start` once, doctor rightly fails, and `test` would be permanently red for host state that install.sh neither creates nor can regress. So stop treating doctor's exit code as one verdict. `test` now fails when the install is broken — no entry point, doctor crashed, or doctor could not report a usable python and config dir — and reports backend readiness as a note. BB_TEST_REQUIRE_BACKEND=1 restores the strict behaviour. A traceback is checked for explicitly rather than inferred from the exit code, because those are the same value. The PermissionError bug exited non-zero exactly like a missing prerequisite does; only the traceback distinguishes a defect from an environment fact, and that distinction is the whole point of this change. The research doc's footprint table had the service as a host-wide launchd service that survives user deletion. It does not. The state lives in the home and goes with it, so the reset is more complete than claimed — but the corollary is that a fresh account cannot run a bottle until the service is started for it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WEfZZhakx13bxTfXcZCoS5 --- .../testing-clean-install-on-macos.md | 20 ++++-- scripts/macos-install-test.sh | 62 +++++++++++++++++-- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/docs/research/testing-clean-install-on-macos.md b/docs/research/testing-clean-install-on-macos.md index 53dd1039..aebd0225 100644 --- a/docs/research/testing-clean-install-on-macos.md +++ b/docs/research/testing-clean-install-on-macos.md @@ -72,14 +72,24 @@ Grounding the teardown story in what `install.sh` and the backend create: | private venv fallback (no pipx) | `~/.bot-bottle/venv` + symlink in `~/.local/bin` ([`install.sh`](../install.sh)) | ✅ | ❌ removed with home | | PATH / token exports | shell profile (`~/.zprofile`, etc.); `BOT_BOTTLE_CLAUDE_OAUTH_TOKEN` ([`README.md:74`](../README.md)) | ✅ | ❌ removed with home | | **Apple `container` install** | `/usr/local/...` + notarized `.pkg` receipts | ❌ | ✅ **stays** | -| **Apple `container` system service** | launchd system service (`container system start`) | ❌ | ✅ **stays** | +| Apple `container` **service state** | per-user: `~/Library/Application Support/com.apple.container/` (`container system start`) | ✅ | ❌ removed with home | | **Homebrew** (if used for `container`/python) | `/opt/homebrew` | ❌ | ✅ **stays** | | Rosetta 2 (needed for image builds) | system | ❌ | ✅ **stays** | -The three bold rows are the crux: **deleting the throwaway user does not -uninstall the Apple Container runtime, its system service, Homebrew, or -Rosetta.** A VM, by contrast, wipes 100% of the above by definition — -that's its entire advantage for this task. +The bold rows are the crux: **deleting the throwaway user does not uninstall +the Apple Container runtime, Homebrew, or Rosetta.** A VM, by contrast, wipes +100% of the above by definition — that's its entire advantage for this task. + +The service row is the exception, and a live harness run corrected it: the +Apple `container` service is **per-user**, not a host-wide launchd service. +`container system status` reports an `appRoot` under +`~/Library/Application Support/com.apple.container/`, and a freshly created +account sees `container system service: NOT running` even while the creating +admin's is running. That cuts both ways — the state is genuinely removed with +the home, so the reset is *more* complete than this table first claimed, but +it also means **no brand-new user can run a bottle until they run +`container system start` once**. `doctor` correctly fails until they do, which +is why `test` judges backend readiness separately from install correctness. ## Option A — Disposable Tart VM (recommended) diff --git a/scripts/macos-install-test.sh b/scripts/macos-install-test.sh index ca1dc46d..63d53eb5 100755 --- a/scripts/macos-install-test.sh +++ b/scripts/macos-install-test.sh @@ -31,10 +31,14 @@ # `test` is the one-shot clean cycle and the command you normally want: it # refuses to start if the account already exists (a reused home is not a clean # install), and it tears the account down on the way out however it exits, so -# a failed run never leaves an orphan behind. It exits non-zero if the install -# fails, if `bot-bottle` is missing from the new user's PATH, or if `doctor` -# reports unmet prerequisites — note install.sh itself exits 0 in that last -# case, so `test` is a stricter gate than running the installer by hand. +# a failed run never leaves an orphan behind. +# +# It exits non-zero if the install fails, if no entry point was installed, if +# `doctor` crashes, or if doctor can't report a usable python and config dir. +# install.sh itself exits 0 when doctor reports unmet prerequisites, so `test` +# is the stricter gate. It does NOT fail on backend readiness, which is host +# and per-user state the installer neither creates nor can regress — see +# doctor_as_user. BB_TEST_REQUIRE_BACKEND=1 makes that fatal too. # # Config via env: # BB_TEST_USER account short name (default: bbtest) @@ -42,6 +46,8 @@ # BB_TEST_ADMIN 1=admin (reach container svc), 0=standard (default: 1) # BB_TEST_INSTALL_URL curl this install.sh instead of piping the local checkout # BB_TEST_KEEP 1=`test` skips its teardown, to poke at a failure +# BB_TEST_REQUIRE_BACKEND 1=`test` also fails when no backend is ready +# BB_TEST_REPO_URL https repo the throwaway user clones (default: this one) # BOT_BOTTLE_INSTALL_SPEC passed through to install.sh (pip / git spec) # # Notes: @@ -104,7 +110,19 @@ run_as_user() { printf '%s\n' "$1" | sudo -u "$USER_NAME" -i sh -s; } # PATH line rather than editing a shell profile, so on a fresh account the # entry point is installed and working but not on PATH. Demanding PATH here # would fail every run for a reason the installer intends. +# +# Doctor's own exit code conflates two unrelated things: whether the install +# works, and whether this user has a backend ready to run a bottle. Those need +# different verdicts here. install.sh explicitly does not install a backend, +# and the Apple `container` service is per-USER — its state lives in +# ~/Library/Application Support/com.apple.container — so a brand-new account +# never has one running, no matter how correct the install is. Failing on that +# would leave `test` permanently red for something the installer cannot fix +# and cannot regress. So: assert the install, report the backend. +# BB_TEST_REQUIRE_BACKEND=1 makes backend readiness fatal too. doctor_as_user() { + local out rc=0 bad=0 + out="$(mktemp "${TMPDIR:-/tmp}/bb-doctor.XXXXXX")" # shellcheck disable=SC2016 # $HOME/$bb must expand in the *target* user's # shell, not in this one — that's the whole point of the single quotes. run_as_user ' @@ -119,7 +137,41 @@ doctor_as_user() { done echo " no bot-bottle entry point found for this user" >&2 exit 1 - ' + ' >"$out" 2>&1 || rc=$? + cat "$out" + + # An unhandled exception is always an install/product defect, never an + # environment fact — this is exactly how the PermissionError on `ip` showed + # up, and doctor's non-zero exit alone would not have distinguished it. + if grep -q 'Traceback (most recent call last)' "$out"; then + echo " doctor crashed (traceback above) — a defect, not a missing prerequisite" >&2 + bad=1 + fi + grep -qE '^ok: +python:' "$out" \ + || { echo " doctor never reported a usable python" >&2; bad=1; } + grep -qE '^ok: +config:' "$out" \ + || { echo " doctor never reported a usable config dir" >&2; bad=1; } + if [ "$rc" -ne 0 ] && ! grep -qE '^(fail|warn): +backend' "$out"; then + echo " doctor failed for something other than backend readiness" >&2 + bad=1 + fi + + local backend_ready=1 + grep -qE '^fail: +backend' "$out" && backend_ready=0 + rm -f "$out" + + [ "$bad" -eq 0 ] || return 1 + if [ "$backend_ready" -eq 0 ]; then + if [ "${BB_TEST_REQUIRE_BACKEND:-0}" = "1" ]; then + echo " no backend is ready and BB_TEST_REQUIRE_BACKEND=1" >&2 + return 1 + fi + echo " note: install is sound; no backend ready for this user." + echo " Expected on a fresh account — the Apple 'container' service is" + echo " per-user and needs one 'container system start'. Set" + echo " BB_TEST_REQUIRE_BACKEND=1 to treat this as a failure." + fi + return 0 } # --- commands --------------------------------------------------------