fix(ci): the check job installs the tools its image does not ship #147

Closed
claude-bot-andresmgsl wants to merge 2 commits from build/144-ubuntu-latest-tooling into main

Closes #144. Implements the ruling @andres gave in 4398: option Bubuntu-latest keeps meaning the slim act image, and rig's own workflow equips itself with what that image does not ship.

Why B, in one table

Measured 2026-08-01 by streaming ghcr layer blobs through gzip -dc | tar -t rather than pulling, so these are the images' own numbers:

image on the wire extracted shellcheck
act-22.04 — the default 0.55 GB 2.2 GB no
runner-22.04 0.55 GB 2.2 GB no — same slim class, not a middle
full-22.04 18.67 GB 54.52 GB yes (/usr/bin/shellcheck, 0.8.0)

The panel started 3/3 on mapping ubuntu-latest to full-22.04 and reversed on the extracted figure: a box-class ci tenant has ~34–40 GB free, so that mapping is not expensive parity, it is a default that cannot land. apt-get install -y shellcheck costs 7 s and yields the same ShellCheck 0.8.0 the parity image carries, so parity buys no newer tool either.

What is in the change

  1. .github/workflows/ci.ymlcheck installs shellcheck only when absent. The command -v short-circuit means GitHub pays nothing; the sudo is a no-op on the act path (jobs run as uid 0 there, and that image has no runner account) and load-bearing on GitHub's. The comment says exactly that, so nobody deletes it as dead weight. The check contract — globstar, the comm coverage guard, shellcheck -x — is untouched.
  2. DEFAULT_LABELS — the measurement table above now lives beside it, with the rule it implies: on this forge a workflow must not assume tools from the image.
  3. ubuntu-latest-full ships in the default map. A mapping pulls nothing until a job matches it, so it costs a box that never asks exactly nothing — and it has to ship now, because Forgejo freezes labels at registration and a label absent then cannot be added without re-registering.
  4. Stale-default warning on plain converge, with one deliberate change from the terms agreed on the issue — see below.
  5. test/cli.sh — 16 new checks; changelog.d/144.md.

One deliberate deviation from the agreed terms

The thread agreed the warning should fire when .rig-labels ≠ the current default. Implemented literally, that warns on every converge of any runner the operator deliberately gave --labels — which is precisely why the original code only spoke when LABELS_EXPLICIT. Nothing on disk distinguishes "registered under an older rig" from "the operator chose this map" except the string itself.

So the warning matches against SUPERSEDED_DEFAULTS — the exact maps rig has ever shipped as its default, appended to and never edited. Drift is recognised; an operator's own map stays silent forever. The message also says plainly that nothing is broken: under this ruling a runner on the old map still runs jobs, and still runs them green. Re-registering buys the new labels, nothing else.

A second defect this uncovered — and why it is fixed here

With the shellcheck cause fixed, check was still red, on two checks that had never run on this forge:

FAIL: forgejo-runner: a plain release number passes validation — output missing 'must run as root'
FAIL: forgejo-runner: a leading v is stripped before that check   — output missing 'must run as root'

Both prove a good --version got through validation by asserting the next gate down, must run as root — a gate that only exists for a non-root caller. forgejo-runner runs job containers as uid 0, where GitHub's runner is the unprivileged runner user, so rig reaches the token prompt instead. #136's class, in a place its sweep did not reach.

Verified pre-existing: unmodified main fails the same two checks in the same image. They are fixed here rather than filed away because #144's acceptance criterion is a green check, which is unreachable while they fail. Both gates are equally good evidence that validation is behind us, so either is accepted — and exit 1 is still required, so a validation refusal (exit 2) still fails the check, proven by driving a bad pin.

Evidence

The whole check job, run inside ghcr.io/catthehacker/ubuntu:act-22.04 as uid 0 — the exact image and user a real Forgejo job gets — on this branch at 0daa2d0:

uid=0  (this is how forgejo-runner runs jobs)
step shellcheck:      exit 0 (version: 0.8.0)
step cli tests:       765 passed, 0 failed
step release tests:    51 passed, 0 failed
step drill tests:      84 passed, 0 failed

And on the host, where the non-root blocks also run: cli 817/0, release 51/0, drill 84/0, install-lifecycle 25/0, shellcheck -x clean over 34 files.

Must-fail, both halves:

  • Revert the ci.yml step and check returns to shellcheck: command not found, exitcode 127 — the original defect.
  • Revert the two version checks and they fail again as root, which is how they were found.

Not in this PR

  • The live drill drive stays #129's, including a probe of ubuntu-latest-full (which needs a host bigger than this box class) and the non-root edge logged in 4396.
  • A coverage hole worth its own issue, @andres — say the word and I will file it. test/cli.sh runs 765 checks as root against 817 as a normal user: 52 assertions sit behind if [ "$(id -u)" -ne 0 ] and are silently skipped on this forge, not failed. Every one is a "refuses non-root" guard — bootstrap, coolify, db, users, tenant, runner. So CI on Forgejo is materially weaker than on GitHub and says nothing about it. Sibling of #136, out of scope here.

@andres — yours to review and merge; I will not merge or close anything.

Closes #144. Implements the ruling @andres gave in [4398](https://forgejo.heavyduty.builders/heavy-duty/rig/issues/144#issuecomment-4398): **option B** — `ubuntu-latest` keeps meaning the slim act image, and rig's own workflow equips itself with what that image does not ship. ## Why B, in one table Measured 2026-08-01 by streaming ghcr layer blobs through `gzip -dc | tar -t` rather than pulling, so these are the images' own numbers: | image | on the wire | extracted | `shellcheck` | |---|---|---|---| | `act-22.04` — the default | 0.55 GB | 2.2 GB | no | | `runner-22.04` | 0.55 GB | 2.2 GB | no — same slim class, not a middle | | `full-22.04` | 18.67 GB | **54.52 GB** | yes (`/usr/bin/shellcheck`, 0.8.0) | The panel started 3/3 on mapping `ubuntu-latest` to `full-22.04` and reversed on the extracted figure: a box-class ci tenant has ~34–40 GB free, so that mapping is not expensive parity, it is a default that cannot land. `apt-get install -y shellcheck` costs **7 s** and yields the same ShellCheck **0.8.0** the parity image carries, so parity buys no newer tool either. ## What is in the change 1. **`.github/workflows/ci.yml`** — `check` installs `shellcheck` only when absent. The `command -v` short-circuit means GitHub pays nothing; the `sudo` is a no-op on the act path (jobs run as uid 0 there, and that image has no `runner` account) and load-bearing on GitHub's. The comment says exactly that, so nobody deletes it as dead weight. The check contract — `globstar`, the `comm` coverage guard, `shellcheck -x` — is untouched. 2. **`DEFAULT_LABELS`** — the measurement table above now lives beside it, with the rule it implies: *on this forge a workflow must not assume tools from the image.* 3. **`ubuntu-latest-full` ships in the default map.** A mapping pulls nothing until a job matches it, so it costs a box that never asks exactly nothing — and it has to ship *now*, because Forgejo freezes labels at registration and a label absent then cannot be added without re-registering. 4. **Stale-default warning on plain converge**, with one deliberate change from the terms agreed on the issue — see below. 5. **`test/cli.sh`** — 16 new checks; **`changelog.d/144.md`**. ## One deliberate deviation from the agreed terms The thread agreed the warning should fire when `.rig-labels` ≠ the current default. Implemented literally, that warns on **every** converge of any runner the operator deliberately gave `--labels` — which is precisely why the original code only spoke when `LABELS_EXPLICIT`. Nothing on disk distinguishes "registered under an older rig" from "the operator chose this map" except the string itself. So the warning matches against `SUPERSEDED_DEFAULTS` — the exact maps rig has ever shipped as its default, appended to and never edited. Drift is recognised; an operator's own map stays silent forever. The message also says plainly that **nothing is broken**: under this ruling a runner on the old map still runs jobs, and still runs them green. Re-registering buys the new labels, nothing else. ## A second defect this uncovered — and why it is fixed here With the shellcheck cause fixed, `check` was **still red**, on two checks that had never run on this forge: ``` FAIL: forgejo-runner: a plain release number passes validation — output missing 'must run as root' FAIL: forgejo-runner: a leading v is stripped before that check — output missing 'must run as root' ``` Both prove a good `--version` got through validation by asserting the *next* gate down, `must run as root` — a gate that only exists for a non-root caller. **forgejo-runner runs job containers as uid 0**, where GitHub's runner is the unprivileged `runner` user, so rig reaches the token prompt instead. #136's class, in a place its sweep did not reach. Verified pre-existing: unmodified `main` fails the same two checks in the same image. They are fixed here rather than filed away because #144's acceptance criterion is a **green** `check`, which is unreachable while they fail. Both gates are equally good evidence that validation is behind us, so either is accepted — and `exit 1` is still required, so a validation *refusal* (exit 2) still fails the check, proven by driving a bad pin. ## Evidence The whole `check` job, run inside `ghcr.io/catthehacker/ubuntu:act-22.04` as uid 0 — the exact image and user a real Forgejo job gets — on this branch at `0daa2d0`: ``` uid=0 (this is how forgejo-runner runs jobs) step shellcheck: exit 0 (version: 0.8.0) step cli tests: 765 passed, 0 failed step release tests: 51 passed, 0 failed step drill tests: 84 passed, 0 failed ``` And on the host, where the non-root blocks also run: `cli 817/0`, `release 51/0`, `drill 84/0`, `install-lifecycle 25/0`, `shellcheck -x` clean over 34 files. **Must-fail, both halves:** - Revert the `ci.yml` step and `check` returns to `shellcheck: command not found`, exitcode 127 — the original defect. - Revert the two version checks and they fail again as root, which is how they were found. ## Not in this PR - **The live drill drive stays #129's**, including a probe of `ubuntu-latest-full` (which needs a host bigger than this box class) and the non-root edge logged in [4396](https://forgejo.heavyduty.builders/heavy-duty/rig/issues/144#issuecomment-4396). - **A coverage hole worth its own issue, @andres — say the word and I will file it.** `test/cli.sh` runs **765** checks as root against **817** as a normal user: 52 assertions sit behind `if [ "$(id -u)" -ne 0 ]` and are *silently skipped* on this forge, not failed. Every one is a "refuses non-root" guard — `bootstrap`, `coolify`, `db`, `users`, `tenant`, `runner`. So CI on Forgejo is materially weaker than on GitHub and says nothing about it. Sibling of #136, out of scope here. @andres — yours to review and merge; I will not merge or close anything.
claude-bot-andresmgsl added 2 commits 2026-08-01 21:27:56 +00:00
rig's own `ci / check` failed 9 times out of 27 ci.yml tasks on the first
Forgejo runner, every one on `shellcheck: command not found`. `ubuntu-latest`
maps to catthehacker's SLIM act image, and workflows written for GitHub
reasonably assume GitHub's tool surface.

Measured before choosing (2026-08-01, streaming ghcr layer blobs rather than
pulling): the parity image is 18.67 GB on the wire and 54.52 GB extracted,
against a box-class ci tenant with ~34-40 GB free. There is no cheap middle —
runner-22.04 is the same slim class, tool for tool, and ships no shellcheck
either. `apt-get install -y shellcheck` costs 7s and yields the same
ShellCheck 0.8.0 that full-22.04 carries.

So the slim default stays and the workflow equips itself. The `command -v`
short-circuit keeps either forge from paying for the other; the sudo is a
no-op on the act path and load-bearing on GitHub's.

`ubuntu-latest-full` ships in the default map beside it: a mapping pulls
nothing until a job matches it, and Forgejo freezes labels at registration,
so a label absent then cannot be added without re-registering.

A plain converge now warns when a runner carries a SUPERSEDED default —
matched against the exact strings rig has shipped, so a map the operator
chose stays silent. The message says plainly that nothing is broken.

Refs #144
test(cli): the pin-validation checks read the gate below, whoever runs them
Some checks failed
ci / check (pull_request) Successful in 1m7s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 7s
0daa2d0095
Two checks proved a good --version got through validation by asserting the
NEXT gate down, 'must run as root'. That gate only exists for a non-root
caller. The Forgejo runner runs job containers as uid 0 — GitHub's runner is
the unprivileged 'runner' user — so on this forge both checks failed, and
'ci / check' stayed red after the shellcheck cause was fixed. #136's class,
in a place its sweep did not reach.

Both gates are equally good evidence that validation is behind us, so accept
either, and keep requiring exit 1 so a validation refusal (exit 2) still
fails the check.

Refs #144

Closing this as the duplicate per @andres’s explicit tie-break instruction on #144. !146 survives: it was the first claimed build, now includes the same retired-default matcher, has the narrower patch, and is green on the relevant checks. No merge is being performed; useful review findings from this branch are already incorporated into !146.

Closing this as the duplicate per @andres’s explicit tie-break instruction on #144. !146 survives: it was the first claimed build, now includes the same retired-default matcher, has the narrower patch, and is green on the relevant checks. No merge is being performed; useful review findings from this branch are already incorporated into !146.
Some checks failed
ci / check (pull_request) Successful in 1m7s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 7s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
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#147
No description provided.