From fd5c04efc986fb9539bb10fbb296b76212260ff7 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 16:51:28 +0000 Subject: [PATCH 1/3] test: cli.sh reads fixtures, not the host it happens to run on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 13 checks failed on any box with a Forgejo runner installed — which is exactly a box that has been drilled or used as a ci-box. Unmodified main, 773/13 on this machine. Seven bootstrap --undo checks fell through to the real host scan. The production code already ships the escape hatch and documents it — bootstrap-undo.sh:33, "RIG_FORGEJO_RUNNER_DIR mirrors RIG_RUNNER_DIR above so tests can point this at a fixture" — and the suite simply never set it. One check rebuilt its env by hand instead of using undo(), so it needed the same variable a second time. Six ci-box checks drove the real template installer, which correctly exits 0 when /usr/local/bin/forgejo-runner already exists. CIBOX_BIN is a test-only override in the same spirit; the production default is untouched and remains the only path the mechanism uses. Neither guard is disarmed: driven for real, undo still refuses while a runner exists, and the template installer still does nothing when the binary is present. Removing either override brings its failures straight back (7 and 6). Closes #136 Co-Authored-By: Claude Opus 5 (1M context) --- docs/templates/ci-box/install.sh | 7 ++++++- test/cli.sh | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/docs/templates/ci-box/install.sh b/docs/templates/ci-box/install.sh index 4210676..ed73aa6 100755 --- a/docs/templates/ci-box/install.sh +++ b/docs/templates/ci-box/install.sh @@ -77,7 +77,12 @@ fetch_and_verify_sha256() { printf 'checksum verified (%s)\n' "$got" } -BIN=/usr/local/bin/forgejo-runner +# CIBOX_BIN is a TEST-ONLY override, in the same spirit as bootstrap-undo.sh's +# RIG_FORGEJO_RUNNER_DIR: the production default is the only path the mechanism +# ever uses, but test/cli.sh must be able to drive this script on a box that +# already has a real runner installed. Without it the early-exit below fires +# against the host and the checksum checks silently test nothing (#136). +BIN="${CIBOX_BIN:-/usr/local/bin/forgejo-runner}" if [ -x "$BIN" ]; then exit 0 diff --git a/test/cli.sh b/test/cli.sh index 4846778..3cbf64f 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -156,8 +156,9 @@ UNDO_FIX="$(mktemp -d)" UNDO_BIN="$UNDO_FIX/bin" UNDO_MARKER="$UNDO_FIX/role" UNDO_RUNNER="$UNDO_FIX/runner" +UNDO_FJRUNNER="$UNDO_FIX/fjrunner" UNDO_CALLS="$UNDO_FIX/tailscale.calls" -mkdir -p "$UNDO_BIN" "$UNDO_RUNNER" +mkdir -p "$UNDO_BIN" "$UNDO_RUNNER" "$UNDO_FJRUNNER" cat > "$UNDO_BIN/tailscale" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$UNDO_CALLS" @@ -169,8 +170,13 @@ if [ "${1:-}" = -u ]; then printf '0\n'; else exec /usr/bin/id "$@"; fi SH chmod +x "$UNDO_BIN/tailscale" "$UNDO_BIN/id" undo() { + # RIG_FORGEJO_RUNNER_DIR is as load-bearing as RIG_RUNNER_DIR: without it + # bootstrap-undo.sh scans /home/*/forgejo-runner/.runner and the systemd unit + # on the REAL box, so these checks fail on any machine that has actually been + # drilled or used as a ci-box — which is the machine that matters (#136). env PATH="$UNDO_BIN:$PATH" UNDO_CALLS="$UNDO_CALLS" \ RIG_ROLE_MARKER="$UNDO_MARKER" RIG_RUNNER_DIR="$UNDO_RUNNER" \ + RIG_FORGEJO_RUNNER_DIR="$UNDO_FJRUNNER" \ "$ROOT/bin/rig" bootstrap --undo } undo_untouched() { @@ -196,7 +202,8 @@ rm -f "$UNDO_RUNNER/.runner" check "bootstrap --undo: failed logout is loud" \ 1 "role marker kept" env TAILSCALE_LOGOUT_FAIL=1 PATH="$UNDO_BIN:$PATH" \ UNDO_CALLS="$UNDO_CALLS" RIG_ROLE_MARKER="$UNDO_MARKER" \ - RIG_RUNNER_DIR="$UNDO_RUNNER" "$ROOT/bin/rig" bootstrap --undo + RIG_RUNNER_DIR="$UNDO_RUNNER" RIG_FORGEJO_RUNNER_DIR="$UNDO_FJRUNNER" \ + "$ROOT/bin/rig" bootstrap --undo check "bootstrap --undo: failed logout preserves the marker" 0 "" test -e "$UNDO_MARKER" : > "$UNDO_CALLS" check "bootstrap --undo: proven rig join succeeds" 0 "tailnet join removed" undo @@ -3276,7 +3283,9 @@ cibox_src_matches_install() { # shellcheck source=/dev/null . "$ROOT/commands/lib/templates.sh" template_parse_env "$dir/template.env" >/dev/null || return 2 - grep -qF "BIN=$TPL_CLI_SRC" "$dir/install.sh" + # BIN carries a test-only override (#136), so the agreement this asserts is + # with the DEFAULT — the only path the mechanism itself ever uses. + grep -qF "CIBOX_BIN:-$TPL_CLI_SRC" "$dir/install.sh" } check "ci-box: CLI_SRC is the path its install.sh installs" 0 "" cibox_src_matches_install @@ -3460,7 +3469,7 @@ chmod +x "$CBSTUB/install" cibox_run() { # cibox_run [VAR=val ...] — the REAL template install.sh, stubbed rm -f "$CBW/installed" - env PATH="$CBSTUB:$PATH" \ + env PATH="$CBSTUB:$PATH" CIBOX_BIN="$CBW/bin-under-test" \ CB_REDIRECT=https://code.forgejo.org/forgejo/runner/releases/tag/v9.9.9 \ CB_PAYLOAD="$CBW/payload" "$@" bash "$CIBOX" } -- 2.45.2 From d46697034df5f77db9baa2ae424353c08fe3ea09 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 20:15:35 +0000 Subject: [PATCH 2/3] test: pin the hermeticity, not just achieve it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codex and kimi both drove the same experiment: delete the two wiring lines this branch adds and the suite stays 786/786 on any host without a real Forgejo runner. Hermetic today, unpinned tomorrow — and #136's task list names the guard verbatim: "a check that fails if either group can see host state". Three checks assert the suite's own helpers keep their seals: undo() passes RIG_FORGEJO_RUNNER_DIR, cibox_run() passes CIBOX_BIN, and the hand-rolled undo invocation behind "failed logout is loud" passes it too — that third one being the site I missed first time round, which is exactly why it earns a check rather than a comment. They assert on the test side deliberately: the production knobs are already covered, and the regression worth catching is a deletion in the suite. Verified by re-running codex's experiment: with both wiring lines gone, the two seal checks fail where previously nothing did. Refs #136 Co-Authored-By: Claude Opus 5 (1M context) --- test/cli.sh | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/cli.sh b/test/cli.sh index 3cbf64f..6c2883b 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -3289,6 +3289,29 @@ cibox_src_matches_install() { } check "ci-box: CLI_SRC is the path its install.sh installs" 0 "" cibox_src_matches_install +# codex/kimi on !137: the two wiring lines this change exists to add could be +# deleted tomorrow and the suite stayed 786/786 on any host without a real +# Forgejo runner — hermetic today, unpinned. #136's task list names the guard +# verbatim: "a check that fails if either group can see host state". +# +# These assert on the SUITE's own helpers, not the production knobs — the knobs +# are already covered above. What must go red is a deletion on the test side, +# because that is the regression that silently reintroduces host dependence. +undo_is_sealed() { + sed -n '/^undo() {/,/^}/p' "$0" | grep -q 'RIG_FORGEJO_RUNNER_DIR=' +} +cibox_run_is_sealed() { + sed -n '/^cibox_run() {/,/^}/p' "$0" | grep -q 'CIBOX_BIN=' +} +check "hermetic: undo() seals the Forgejo-runner host scan" 0 "" undo_is_sealed +check "hermetic: cibox_run() seals the real /usr/local/bin lookup" 0 "" cibox_run_is_sealed +# The failed-logout check builds its own env rather than calling undo(), so it +# needs the same seal — and it is the one that was missed first time round. +inline_undo_is_sealed() { + grep -A 4 'failed logout is loud' "$0" | grep -q 'RIG_FORGEJO_RUNNER_DIR=' +} +check "hermetic: the hand-rolled undo invocation is sealed too" 0 "" inline_undo_is_sealed + # Registration holds a credential, so it must NOT be in the definition: a # tenant install is creds-free by contract — box auto-runs it at mint, holding # nothing. Registration is the operator's separate, out-loud act. -- 2.45.2 From 7385707770fee884c6dbebc625c47b2b753b8254 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Fri, 31 Jul 2026 20:32:08 +0000 Subject: [PATCH 3/3] test: the third hermetic pin can actually fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kimi was right that inline_undo_is_sealed matched itself: it grepped the whole file for a string that appears inside its own body, so it succeeded on every tree forever. Their suggested anchoring had the identical flaw for the identical reason — any literal this function searches for necessarily appears in this function — which I only found by running their probe and watching the pin stay green while the behavioural test went red. So it locates the real check by line number and reads only that block. head -1 takes the site at :202, never this body at :3311. Driven, since a pin nobody has watched fail is not a pin: deleting the :205 wiring now fails exactly this check, and deleting all three seals fails all three. That is the third vacuous guard I have written on this branch family; the comment says why the obvious shapes do not work, so the next person does not rediscover it a fourth time. Refs #136 Co-Authored-By: Claude Opus 5 (1M context) --- test/cli.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/cli.sh b/test/cli.sh index 6c2883b..6d4220f 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -3308,7 +3308,17 @@ check "hermetic: cibox_run() seals the real /usr/local/bin lookup" 0 "" cibox_ru # The failed-logout check builds its own env rather than calling undo(), so it # needs the same seal — and it is the one that was missed first time round. inline_undo_is_sealed() { - grep -A 4 'failed logout is loud' "$0" | grep -q 'RIG_FORGEJO_RUNNER_DIR=' + # Locate the REAL check by line number and read only its own block. Anchoring + # on a string and grepping the whole file cannot work here: any pattern this + # function searches for necessarily appears inside this function, so the + # search matches itself and can never fail. kimi caught the first version of + # that on !137; the anchored second version had the identical flaw for the + # identical reason. head -1 takes the real check (~:202), never this body. + local start end + start="$(grep -n 'failed logout is loud' "$0" | head -1 | cut -d: -f1)" + [ -n "$start" ] || return 1 + end=$((start + 5)) + sed -n "${start},${end}p" "$0" | grep -q RIG_FORGEJO_RUNNER_DIR } check "hermetic: the hand-rolled undo invocation is sealed too" 0 "" inline_undo_is_sealed -- 2.45.2