From e847d51a7182d4d9cc74e5c58e08885f19598c76 Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 27 Jul 2026 00:43:12 -0400 Subject: [PATCH] feat: add a one-shot `test` cycle to the macOS install harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test` chains up -> run -> status -> down, which is the loop you actually want when verifying a clean install. Three things make it more than a convenience wrapper: * It refuses to start against an existing account. A reused home is not a clean install, so testing one silently would defeat the harness. * It tears the account down from an EXIT/INT trap armed the moment the account exists, so a failed run — or a Ctrl-C mid-install — still leaves the machine clean. BB_TEST_KEEP=1 opts out to poke at a failure. * Its verdict is stricter than the installer's. install.sh exits 0 when it finishes but `doctor` reports unmet prerequisites, so "the installer succeeded" is not a useful assertion; `test` fails if the install fails, if bot-bottle never reached the new user's PATH, or if doctor is unhappy. That meant giving cmd_status a real exit status instead of swallowing doctor's. Also fixes two bugs in `run`'s installer staging, by removing the staging entirely and feeding install.sh in on stdin: * `mktemp /tmp/bb-install.XXXXXX.sh` did not do what it looks like. BSD mktemp only substitutes trailing Xs, so every run wrote the *same* predictable path, as root, mode 644, in a world-writable directory. * The cleanup only ran on the normal and failure returns, so an interrupted run leaked the file. Piping on stdin sidesteps both: root opens the redirect before sudo drops privileges, so the mode-700 home that motivated the staging is a non-issue, there is no file to leak, and `sh -s` matches the documented `curl … | sh` shape more closely than executing a staged copy did. The command-level `exit 1`s become `return 1` so the steps compose under the trap, and the "next, run this" hints are suppressed inside `test`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WEfZZhakx13bxTfXcZCoS5 --- .../testing-clean-install-on-macos.md | 29 +++++ scripts/macos-install-test.sh | 120 +++++++++++++++--- 2 files changed, 131 insertions(+), 18 deletions(-) diff --git a/docs/research/testing-clean-install-on-macos.md b/docs/research/testing-clean-install-on-macos.md index 7c3e76b5..f9e47842 100644 --- a/docs/research/testing-clean-install-on-macos.md +++ b/docs/research/testing-clean-install-on-macos.md @@ -213,6 +213,35 @@ headlessly (so a PR is verifiable before it lands) and lets the installer run the footprint analysis above — the reset is just user deletion because everything `install.sh` writes is user-home-local. +`test` chains `up → run → status → down` into the one-shot cycle you normally +want: + +```sh +sudo ./scripts/macos-install-test.sh test +``` + +It refuses to start against an existing account (a reused home is not a clean +install), and it tears the account down from an `EXIT`/`INT` trap armed the +moment the account exists, so a failed or Ctrl-C'd run still leaves the machine +clean. Its verdict is deliberately stricter than the installer's own: note that +`install.sh` exits **0** when it finishes but `doctor` reports unmet +prerequisites, so "the installer succeeded" is not the assertion — `test` fails +if the install fails, if `bot-bottle` never reached the new user's `PATH`, or if +`doctor` is unhappy. `BB_TEST_KEEP=1` skips the teardown to poke at a failure. + +### What a fresh account actually inherits + +Expect the first honest run on a developer Mac to fail at the *Python* gate, +and expect that to be correct. A new account's `PATH` is just `/etc/paths` +(`/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin`), +which notably does **not** include `/opt/homebrew/bin`. Homebrew's `shellenv` +line lives in the *installing* user's `~/.zprofile` and is not inherited, so a +throwaway user resolves `python3` to `/usr/bin/python3` — the Command Line +Tools stub, still **3.9.6** on macOS 26 — and `install.sh` correctly dies on its +`3.11+` requirement. Your own shell resolving `python3` to a 3.14 Homebrew +build says nothing about what a new user sees; that gap is exactly what this +harness exists to expose. + ## Sources - [Apple Containers on macOS: technical comparison with Docker — The New Stack](https://thenewstack.io/apple-containers-on-macos-a-technical-comparison-with-docker/) diff --git a/scripts/macos-install-test.sh b/scripts/macos-install-test.sh index 37c318e9..0ec94dde 100755 --- a/scripts/macos-install-test.sh +++ b/scripts/macos-install-test.sh @@ -20,19 +20,27 @@ # docs/research/testing-clean-install-on-macos.md. # # Usage: +# sudo ./scripts/macos-install-test.sh test # up -> run -> status -> down # sudo ./scripts/macos-install-test.sh up # create the throwaway user # sudo ./scripts/macos-install-test.sh run # run install.sh (+doctor) as it # ./scripts/macos-install-test.sh status # user present? backend ready? # sudo ./scripts/macos-install-test.sh down # delete user + home (the reset) # sudo ./scripts/macos-install-test.sh deep-reset # ALSO uninstall host `container` # -# A full clean cycle is: sudo ... up && sudo ... run && sudo ... down +# `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. # # Config via env: # BB_TEST_USER account short name (default: bbtest) # BB_TEST_FULLNAME account full name (default: "bot-bottle install test") # 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 # BOT_BOTTLE_INSTALL_SPEC passed through to install.sh (pip / git spec) # # Notes: @@ -58,6 +66,10 @@ ADMIN="${BB_TEST_ADMIN:-1}" _SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" _REPO_ROOT="$(cd "$_SCRIPT_DIR/.." && pwd)" +# Set by `test`, which chains the steps itself and so suppresses the +# "here's the next command to run" hints the individual steps print. +IN_TEST=0 + # --- guards ---------------------------------------------------------- require_macos() { [ "$(uname -s)" = "Darwin" ] \ @@ -76,6 +88,16 @@ user_exists() { dscl . -read "/Users/$USER_NAME" >/dev/null 2>&1; } # Run a shell snippet as the throwaway user in a fresh login shell. run_as_user() { sudo -u "$USER_NAME" -i sh -c "$1"; } +# `bot-bottle doctor` as the throwaway user. Non-zero when the entry point +# never made it onto that user's PATH, or when doctor itself is unhappy. +doctor_as_user() { + if ! run_as_user 'command -v bot-bottle >/dev/null 2>&1'; then + echo " bot-bottle is not on PATH for $USER_NAME" + return 1 + fi + run_as_user 'bot-bottle doctor' +} + # --- commands -------------------------------------------------------- cmd_up() { require_macos @@ -91,14 +113,18 @@ cmd_up() { # irrelevant to a headless test account. sysadminctl -addUser "$USER_NAME" -fullName "$FULL_NAME" "${admin_flag[@]}" || true # sysadminctl exits 0 regardless of outcome, so confirm the account landed. - user_exists || { echo "error: failed to create $USER_NAME" >&2; exit 1; } - echo "created $USER_NAME (admin=$ADMIN). Install into it with: sudo $0 run" + user_exists || { echo "error: failed to create $USER_NAME" >&2; return 1; } + if [ "$IN_TEST" = 1 ]; then + echo "created $USER_NAME (admin=$ADMIN)" + else + echo "created $USER_NAME (admin=$ADMIN). Install into it with: sudo $0 run" + fi } cmd_run() { require_macos require_root run - user_exists || { echo "error: $USER_NAME does not exist; run 'sudo $0 up' first" >&2; exit 1; } + user_exists || { echo "error: $USER_NAME does not exist; run 'sudo $0 up' first" >&2; return 1; } local spec_env="" [ -n "${BOT_BOTTLE_INSTALL_SPEC:-}" ] \ @@ -109,27 +135,28 @@ cmd_run() { run_as_user "curl -fsSL '$BB_TEST_INSTALL_URL' | ${spec_env}sh" else # Test THIS checkout's install.sh, not the published one, so a PR is - # verifiable before it lands. Stage it world-readable in /tmp because - # the throwaway user can't read the tester's (mode 700) home. - local staged - staged="$(mktemp /tmp/bb-install.XXXXXX.sh)" - cp "$_REPO_ROOT/install.sh" "$staged" - chmod 644 "$staged" - run_as_user "${spec_env}sh '$staged'" || { rm -f "$staged"; exit 1; } - rm -f "$staged" + # verifiable before it lands. Feed it in on stdin rather than staging a + # copy somewhere the throwaway user can read: the redirect is opened by + # root before sudo drops privileges, so the tester's mode-700 home is a + # non-issue, there's no temp file to leak if the run is interrupted, and + # `sh -s` is the same shape as the documented `curl … | sh` install. + run_as_user "${spec_env}sh -s" < "$_REPO_ROOT/install.sh" fi - echo "== install.sh runs 'doctor' itself; re-check anytime with: $0 status ==" + [ "$IN_TEST" = 1 ] \ + || echo "== install.sh runs 'doctor' itself; re-check anytime with: $0 status ==" } +# Informational, with one teeth-bearing case: when it can actually reach +# doctor (root, account present) its exit status is doctor's, so `test` and +# any other caller can use it as the post-install assertion. cmd_status() { require_macos + local rc=0 if user_exists; then echo "user: $USER_NAME present" if [ "$(id -u)" -eq 0 ]; then echo "doctor (as $USER_NAME):" - run_as_user 'command -v bot-bottle >/dev/null 2>&1 \ - && bot-bottle doctor \ - || echo " bot-bottle not installed for this user yet"' || true + doctor_as_user || rc=1 else echo " (re-run under sudo to run 'bot-bottle doctor' as $USER_NAME)" fi @@ -141,6 +168,7 @@ cmd_status() { else echo "backend: apple 'container' NOT on PATH (host prerequisite; install once)" fi + return "$rc" } cmd_down() { @@ -158,11 +186,66 @@ cmd_down() { echo "error: $USER_NAME still present after delete." >&2 echo " - grant Terminal Full Disk Access (System Settings > Privacy & Security), or" >&2 echo " - it may hold the last Secure Token (won't happen while another admin exists)" >&2 - exit 1 + return 1 fi echo "removed $USER_NAME and its home — install surface is clean." } +# Teardown half of `test`, installed as an EXIT trap the moment the account +# exists so that a failure — or a Ctrl-C — still leaves the machine clean. +_test_teardown() { + local rc=$? + trap - EXIT INT TERM + if [ "${BB_TEST_KEEP:-0}" = "1" ]; then + echo + echo "== [4/4] down: SKIPPED (BB_TEST_KEEP=1) ==" + echo " $USER_NAME is still around; remove it with: sudo $0 down" + exit "$rc" + fi + echo + echo "== [4/4] down ==" + cmd_down || rc=1 + if [ "$rc" -eq 0 ]; then + echo + echo "PASS: a brand-new user can install bot-bottle and pass doctor." + else + echo + echo "FAIL: see above (the throwaway account was torn down regardless)." >&2 + fi + exit "$rc" +} + +cmd_test() { + require_macos + require_root test + # A pre-existing account means a pre-existing home, which is the one thing + # this harness exists to rule out. Don't silently test a dirty install. + if user_exists; then + echo "error: $USER_NAME already exists, so this would not be a clean install." >&2 + echo " reset first: sudo $0 down" >&2 + return 1 + fi + IN_TEST=1 + + echo "== [1/4] up ==" + cmd_up + trap _test_teardown EXIT INT TERM + + echo + echo "== [2/4] run ==" + cmd_run + + echo + echo "== [3/4] status ==" + # install.sh exits 0 even when doctor reports unmet prerequisites, so the + # install succeeding is not the verdict — this is. + cmd_status || { + echo "error: doctor is unhappy for a freshly installed user (see above)." >&2 + echo " re-run with BB_TEST_KEEP=1 to keep $USER_NAME around and dig in." >&2 + return 1 + } +} + cmd_deep_reset() { require_macos require_root deep-reset @@ -186,10 +269,11 @@ cmd_deep_reset() { } case "${1:-}" in + test) cmd_test ;; up) cmd_up ;; run) cmd_run ;; status) cmd_status ;; down) cmd_down ;; deep-reset) cmd_deep_reset ;; - *) echo "usage: $0 {up|run|status|down|deep-reset}" >&2 ; exit 2 ;; + *) echo "usage: $0 {test|up|run|status|down|deep-reset}" >&2 ; exit 2 ;; esac