test/cli.sh is not hermetic — 13 checks fail on any box that has a Forgejo runner installed #136

Closed
opened 2026-07-31 16:47:31 +00:00 by claude-bot-andresmgsl · 0 comments

Context

test/cli.sh is not hermetic: 13 checks fail on any machine that has a Forgejo runner installed, which is exactly a machine that has been drilled or used as a ci-box. Measured 2026-07-31 on a box where rig forgejo-runner install had really been run — unmodified origin/main, so this is not any branch's doing:

773 passed, 13 failed

Two independent groups, both reading real host state instead of a fixture:

1. Seven bootstrap --undo: checks

commands/bootstrap-undo.sh:46 refuses to undo while a Forgejo runner exists — correct behaviour, and the guard is wanted. But the test never turns it off, so it fires against the real machine:

rig-bootstrap: ERROR: a Forgejo runner is installed — run 'rig forgejo-runner remove' first

The production code already ships the escape hatch, and says so, bootstrap-undo.sh:33:

RIG_FORGEJO_RUNNER_DIR mirrors RIG_RUNNER_DIR above so tests can point this at a fixture.

test/cli.sh's undo() helper sets RIG_ROLE_MARKER and RIG_RUNNER_DIR — but not RIG_FORGEJO_RUNNER_DIR. So it falls through to the host scan of /home/*/forgejo-runner/.runner, /root/forgejo-runner/.runner and /etc/systemd/system/forgejo-runner.service. Adding the one variable turns the refusal into tailnet join removed; role marker removed — driven, not reasoned.

2. Six ci-box: checks

docs/templates/ci-box/install.sh:82 early-exits when the binary is already there:

if [ -x "$BIN" ]; then    # BIN=/usr/local/bin/forgejo-runner
  exit 0
fi

Also correct behaviour — an install that finds the binary present has nothing to do. But the test stubs curl and install and then drives the real script, so on a box with a real /usr/local/bin/forgejo-runner the script correctly does nothing and the checks see no install. Parking the binary recovers all six; restoring it breaks them again.

Why it matters beyond the annoyance

The suite is the gate. A gate that passes only on machines that have never run the thing it gates is a gate you cannot trust on the machine that matters — and the failure is loud but misattributed: it reads as "this branch broke 13 tests", which is how it consumed a review round on !130.

It is also the same class the panel required to be fixed in test/drill.sh during that round (a fixture copied from a scratch path). This is that bug, older and larger.

Spec

Point both groups at fixtures instead of the host. No production behaviour changes; both guards stay exactly as they are.

  • undo() gains RIG_FORGEJO_RUNNER_DIR="$UNDO_FIX/fjrunner", mirroring the RIG_RUNNER_DIR already there. The knob exists for this.
  • The ci-box harness points BIN at a writable fixture path rather than letting the script default to /usr/local/bin/forgejo-runner, so a real binary on the host is irrelevant. If the script has no knob for it, add one in the same spirit as RIG_FORGEJO_RUNNER_DIR — a test-only override the production default ignores.

Tasks

  • Set RIG_FORGEJO_RUNNER_DIR in test/cli.sh's undo() helper
  • Make the ci-box checks independent of /usr/local/bin/forgejo-runner
  • Assert hermeticity itself: a check that fails if either group can see host state
  • changelog.d/<n>.md only if production files change; a test-only fix needs none

Acceptance criteria

  • bash test/cli.sh passes on a box with a Forgejo runner installed and a real /usr/local/bin/forgejo-runner
  • It still passes on a box without one
  • bootstrap-undo.sh's guard is unchanged — the refusal still fires for real operators
  • ci-box/install.sh's early-exit is unchanged
  • CI's shellcheck -x step is clean

Test plan

  • Run the suite on this box, which has both a real runner and the binary: it must go from 773/13 to fully green.
  • Must fail: remove the new RIG_FORGEJO_RUNNER_DIR and the seven undo checks go red again on such a box.
  • Confirm the two production guards still refuse when driven for real, so the fix has not disarmed them.

Dependencies

None. Independent of #129, #133 and #135; test-only.


@andres — filed under your standing instruction. The 13 failures are reproducible on this box right now, on unmodified main.

## Context `test/cli.sh` is not hermetic: 13 checks fail on any machine that has a Forgejo runner installed, which is exactly a machine that has been drilled or used as a ci-box. Measured 2026-07-31 on a box where `rig forgejo-runner install` had really been run — **unmodified `origin/main`**, so this is not any branch's doing: ``` 773 passed, 13 failed ``` Two independent groups, both reading real host state instead of a fixture: ### 1. Seven `bootstrap --undo:` checks `commands/bootstrap-undo.sh:46` refuses to undo while a Forgejo runner exists — correct behaviour, and the guard is wanted. But the test never turns it off, so it fires against the real machine: ``` rig-bootstrap: ERROR: a Forgejo runner is installed — run 'rig forgejo-runner remove' first ``` The production code already ships the escape hatch, and says so, `bootstrap-undo.sh:33`: > `RIG_FORGEJO_RUNNER_DIR` mirrors `RIG_RUNNER_DIR` above so tests can point this at a fixture. `test/cli.sh`'s `undo()` helper sets `RIG_ROLE_MARKER` and `RIG_RUNNER_DIR` — but **not** `RIG_FORGEJO_RUNNER_DIR`. So it falls through to the host scan of `/home/*/forgejo-runner/.runner`, `/root/forgejo-runner/.runner` and `/etc/systemd/system/forgejo-runner.service`. Adding the one variable turns the refusal into `tailnet join removed; role marker removed` — driven, not reasoned. ### 2. Six `ci-box:` checks `docs/templates/ci-box/install.sh:82` early-exits when the binary is already there: ```bash if [ -x "$BIN" ]; then # BIN=/usr/local/bin/forgejo-runner exit 0 fi ``` Also correct behaviour — an install that finds the binary present has nothing to do. But the test stubs `curl` and `install` and then drives the real script, so on a box with a real `/usr/local/bin/forgejo-runner` the script correctly does nothing and the checks see no install. Parking the binary recovers all six; restoring it breaks them again. ## Why it matters beyond the annoyance The suite is the gate. A gate that passes only on machines that have never run the thing it gates is a gate you cannot trust on the machine that matters — and the failure is loud but misattributed: it reads as "this branch broke 13 tests", which is how it consumed a review round on !130. It is also the same class the panel required to be fixed in `test/drill.sh` during that round (a fixture copied from a scratch path). This is that bug, older and larger. ## Spec Point both groups at fixtures instead of the host. No production behaviour changes; both guards stay exactly as they are. - `undo()` gains `RIG_FORGEJO_RUNNER_DIR="$UNDO_FIX/fjrunner"`, mirroring the `RIG_RUNNER_DIR` already there. The knob exists for this. - The `ci-box` harness points `BIN` at a writable fixture path rather than letting the script default to `/usr/local/bin/forgejo-runner`, so a real binary on the host is irrelevant. If the script has no knob for it, add one in the same spirit as `RIG_FORGEJO_RUNNER_DIR` — a test-only override the production default ignores. ## Tasks - [ ] Set `RIG_FORGEJO_RUNNER_DIR` in `test/cli.sh`'s `undo()` helper - [ ] Make the `ci-box` checks independent of `/usr/local/bin/forgejo-runner` - [ ] Assert hermeticity itself: a check that fails if either group can see host state - [ ] `changelog.d/<n>.md` only if production files change; a test-only fix needs none ## Acceptance criteria - [ ] `bash test/cli.sh` passes on a box **with** a Forgejo runner installed and a real `/usr/local/bin/forgejo-runner` - [ ] It still passes on a box without one - [ ] `bootstrap-undo.sh`'s guard is unchanged — the refusal still fires for real operators - [ ] `ci-box/install.sh`'s early-exit is unchanged - [ ] CI's `shellcheck -x` step is clean ## Test plan - Run the suite on this box, which has both a real runner and the binary: it must go from 773/13 to fully green. - **Must fail:** remove the new `RIG_FORGEJO_RUNNER_DIR` and the seven undo checks go red again on such a box. - Confirm the two production guards still refuse when driven for real, so the fix has not disarmed them. ## Dependencies None. Independent of #129, #133 and #135; test-only. --- @andres — filed under your standing instruction. The 13 failures are reproducible on this box right now, on unmodified `main`.
claude-bot-andresmgsl added the
bug
scope:labels
claimed
labels 2026-07-31 16:47:32 +00:00
claude-bot-andresmgsl self-assigned this 2026-07-31 16:47:33 +00:00
Sign in to join this conversation.
No milestone
No project
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#136
No description provided.