fix(harness): install the branch under test, and stop mangling snippets
prd-number-check / require-numbered-prds (pull_request) Successful in 9s
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / unit (pull_request) Successful in 1m3s
test / integration-docker (pull_request) Successful in 1m2s
test / image-input-builds (pull_request) Successful in 1m6s
test / coverage (pull_request) Failing after 20s
lint / lint (push) Successful in 3m30s

Two harness bugs, both of which made the last run lie about what it verified.

The run installed main. install.sh's default spec is the repo's default
branch, so `run` piped *this checkout's* installer into the throwaway user and
then had it install a package that does not contain the branch's changes —
which is why the doctor PermissionError fix appeared not to work: it was never
in the code under test. The clone line said so plainly (commit 420184b, not
this branch's HEAD) and I read past it. The spec now pins to the current
branch, over https because the throwaway user has neither our SSH key nor read
access to this mode-700 checkout. Since what it clones is whatever the remote
has, a dirty tree or an unpushed branch now prints a warning rather than
quietly testing something other than what is on disk.

The doctor probe also died with "sh: -c: line 1: syntax error: unexpected end
of file". `sudo -i` joins its argv into one string for the login shell's -c, so
an argument's quoting is not preserved and a newline terminates the command.
The multi-line snippet added in the previous commit could not survive that.
run_as_user now feeds snippets on stdin to `sh -s`, which is the same trick
install.sh already arrives by, and is immune to the joining. The install spec
moves into the stream as an export for the same reason — as an argument it had
the identical latent quoting bug, unnoticed only because the default spec has
no shell metacharacters in it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WEfZZhakx13bxTfXcZCoS5
This commit is contained in:
2026-07-27 09:32:20 -04:00
parent 5c12c6bf4e
commit 7461802b62
+56 -11
View File
@@ -63,6 +63,8 @@ set -euo pipefail
USER_NAME="${BB_TEST_USER:-bbtest}"
FULL_NAME="${BB_TEST_FULLNAME:-bot-bottle install test}"
ADMIN="${BB_TEST_ADMIN:-1}"
# https, not the ssh `origin`: the throwaway user has no key of ours.
BB_TEST_REPO_URL="${BB_TEST_REPO_URL:-https://gitea.dideric.is/didericis/bot-bottle.git}"
_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
_REPO_ROOT="$(cd "$_SCRIPT_DIR/.." && pwd)"
@@ -87,7 +89,13 @@ require_root() {
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"; }
#
# The snippet goes in on stdin, never as an argument. `sudo -i` joins its argv
# into a single string and hands that to the login shell's -c, so quoting in an
# argument is not preserved and a newline ends the command outright ("sh: -c:
# line 1: syntax error: unexpected end of file"). Feeding `sh -s` from stdin
# sidesteps the joining entirely and lets snippets be multi-line.
run_as_user() { printf '%s\n' "$1" | sudo -u "$USER_NAME" -i sh -s; }
# `bot-bottle doctor` as the throwaway user. Non-zero when no entry point was
# installed at all, or when doctor itself is unhappy.
@@ -100,14 +108,15 @@ doctor_as_user() {
# 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 '
for bb in "$HOME/.local/bin/bot-bottle" "$HOME/.bot-bottle/venv/bin/bot-bottle"; do
if [ -x "$bb" ]; then
command -v bot-bottle >/dev/null 2>&1 \
|| echo " (not on PATH — running $bb directly, as install.sh advises)"
for bb in bot-bottle "$HOME/.local/bin/bot-bottle" "$HOME/.bot-bottle/venv/bin/bot-bottle"; do
if command -v "$bb" >/dev/null 2>&1; then
case "$bb" in
bot-bottle) : ;;
*) echo " (not on PATH — running $bb directly, as install.sh advises)" ;;
esac
exec "$bb" doctor
fi
done
command -v bot-bottle >/dev/null 2>&1 && exec bot-bottle doctor
echo " no bot-bottle entry point found for this user" >&2
exit 1
'
@@ -136,18 +145,48 @@ cmd_up() {
fi
}
# The package spec to install. install.sh's own default is the repo's DEFAULT
# BRANCH, which would mean piping this checkout's installer into the throwaway
# user and then installing main's code — verifying the PR's install.sh against
# a package that doesn't contain the PR. Pin to the branch under test instead.
#
# The branch has to be pushed: the throwaway user clones over https and cannot
# read this checkout (mode-700 home, and no SSH key for the ssh remote), so
# what it gets is whatever the remote has. Say so when that differs from here.
resolve_install_spec() {
if [ -n "${BOT_BOTTLE_INSTALL_SPEC:-}" ]; then
echo "$BOT_BOTTLE_INSTALL_SPEC"
return 0
fi
local branch
branch="$(git -C "$_REPO_ROOT" rev-parse --abbrev-ref HEAD 2>/dev/null)" || branch=""
if [ -z "$branch" ] || [ "$branch" = "HEAD" ]; then
echo "note: not on a named branch; installing the repo default" >&2
echo "git+${BB_TEST_REPO_URL}"
return 0
fi
if [ -n "$(git -C "$_REPO_ROOT" status --porcelain 2>/dev/null)" ]; then
echo "warning: working tree is dirty — the throwaway user installs" >&2
echo " $branch as pushed, not what is on disk here." >&2
elif ! git -C "$_REPO_ROOT" diff --quiet "@{upstream}" 2>/dev/null; then
echo "warning: $branch differs from its upstream — push first, or the" >&2
echo " throwaway user installs stale code." >&2
fi
echo "git+${BB_TEST_REPO_URL}@${branch}"
}
cmd_run() {
require_macos
require_root run
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:-}" ] \
&& spec_env="BOT_BOTTLE_INSTALL_SPEC='$BOT_BOTTLE_INSTALL_SPEC' "
local spec
spec="$(resolve_install_spec)"
echo "== installing bot-bottle as $USER_NAME =="
echo "== spec: $spec =="
if [ -n "${BB_TEST_INSTALL_URL:-}" ]; then
run_as_user "curl -fsSL '$BB_TEST_INSTALL_URL' | ${spec_env}sh"
run_as_user "BOT_BOTTLE_INSTALL_SPEC='$spec' curl -fsSL '$BB_TEST_INSTALL_URL' | sh"
else
# Test THIS checkout's install.sh, not the published one, so a PR is
# verifiable before it lands. Feed it in on stdin rather than staging a
@@ -155,7 +194,13 @@ cmd_run() {
# 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"
#
# The spec is prepended to the stream as an export rather than passed
# as an argument, for the same argv-joining reason as run_as_user.
{
printf "export BOT_BOTTLE_INSTALL_SPEC='%s'\n" "$spec"
cat "$_REPO_ROOT/install.sh"
} | sudo -u "$USER_NAME" -i sh -s
fi
[ "$IN_TEST" = 1 ] \
|| echo "== install.sh runs 'doctor' itself; re-check anytime with: $0 status =="