From 15a3728ae9e53c66576ff2ac91363c6ea33e5516 Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Wed, 9 Sep 2026 08:57:34 +0200 Subject: [PATCH] feat: bake shellcheck and add a client-side pre-push lint gate v1.8.14 made shell lint a RELEASE gate (scripts/lint-shell.sh, shared by lint.yml and the new lint-gate job that resolve-versions depends on), and that script correctly exits 2 when shellcheck is absent -- "a gate that cannot run must not pass". Measured on v1.8.14 on 2026-09-09 by three routes (command -v, dpkg -l, a filesystem search): shellcheck was NOT IN THE IMAGE AT ALL. So the gate could not be run by a developer in any container, only in CI, and the loop stayed write-shell -> push -> wait for CI -> discover. That is the loop the gate was added to shorten, after v1.8.14's first attempt burned ~46 min on a tree whose lint had already been red for 24 hours. shellcheck 0.10.0-1 added to the Dockerfile.base apt block: ~39 MB installed (Installed-Size 40112 KB), measured to pull ZERO additional packages under --no-install-recommends because libc6/libffi8/libgmp10 are already present. NOTE this forces one full base rebuild -- base-decide hashes Dockerfile.base + rootfs/, so unlike a scripts/ change it cannot reuse the existing base- layer. hooks/pre-push is opt-in per clone (git config core.hooksPath hooks), bypassable with --no-verify, and execs scripts/lint-shell.sh rather than reimplementing it -- one copy, because a duplicated check that drifts is the failure this repo keeps paying for. Matches the idiom skillset/ and myconfigs/ already use. WHY THIS REPO HAD NO HOOKS, since it was reported as drift and is not: a peer asked tor-ms22 for core.hooksPath per clone on the premise that unset meant the gates were unverified there. Measured: pi-devbox unset, skillset hooks, myconfigs common/hooks, pi-toolkit unset -- but `git ls-files | grep -i hook` is EMPTY in both pi-devbox and pi-toolkit, so there was nothing to point at on any machine and unset was the only correct value. This closes the real half for pi-devbox; pi-toolkit still ships none. Verified, expected result written down before each check: * refusal paths -- shellcheck absent => rc 2 with the remedy named; linter missing => rc 2. Never waved through on the assumption CI will catch it. * the hook is IN the scan set -- "Checking 14 shell file(s)" with it present, 13 with it moved aside, so the extensionless file is found by the shebang half of the linter's two-signal union. This check exists because the first attempt was ambiguous: a planted `[ $UNSET_VAR = "x" ]` was not reported, which could equally have meant "not scanned" or "below -S error". It was the latter. A count that moves is unambiguous; a clean run is not. * it catches the REAL v1.8.14 defect -- planting `echo 'the fleet\'s thing'` in hooks/pre-push yields SC1073/SC1072 at severity error, rc=1. And the gate earned its keep inside this commit: the first version of the smoke-test assertion carried a comment beginning "# shellcheck is a GATE DEPENDENCY", and a comment whose first word is the tool's name is parsed as a DIRECTIVE, not a comment. The new gate failed it with SC1073/SC1072 before the push -- same family as the v1.8.14 apostrophe, a line that reads as prose to a human and as syntax to the parser. --- CHANGELOG.md | 83 +++++++++++++++++++++++++++++++++++++++++++ Dockerfile.base | 22 ++++++++++++ hooks/pre-push | 64 +++++++++++++++++++++++++++++++++ scripts/smoke-test.sh | 12 +++++++ 4 files changed, 181 insertions(+) create mode 100755 hooks/pre-push diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ad23e9..14bca59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,89 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). --- +## Unreleased + +**`shellcheck` is now in the image, because the release gate it depends on could +not be run by anyone.** v1.8.14 made shell lint a release gate: `scripts/lint-shell.sh` +became the single source of truth for `lint.yml` and a new `lint-gate` job that +`resolve-versions` depends on, and it deliberately exits 2 when `shellcheck` is +absent — *a gate that cannot run must not pass*. Measured on v1.8.14 on +2026-09-09, by three routes (`command -v`, `dpkg -l`, a filesystem search): +**`shellcheck` was not in the image at all.** So `bash scripts/lint-shell.sh` +exited 2 in every devbox container, and the only place the gate could ever run +was CI. The developer loop was therefore write-shell → push → wait for CI → +discover — which is the loop the gate was added to shorten, after v1.8.14's first +attempt burned ~46 minutes on a tree whose lint had already been red for 24 +hours. Added to the `apt-get` block in `Dockerfile.base`: `shellcheck 0.10.0-1`, +~39 MB installed (`Installed-Size` 40112 KB), and measured to pull **zero** +additional packages under `--no-install-recommends` because its three deps +(`libc6`, `libffi8`, `libgmp10`) are already present. **This forces one full base +rebuild** — `base-decide` hashes `Dockerfile.base` + `rootfs/`, so unlike a +`scripts/` change it cannot reuse the existing `base-` layer. + +**A client-side pre-push lint gate: `hooks/pre-push`.** Opt-in per clone with +`git config core.hooksPath hooks`, bypass with `git push --no-verify`, matching +the idiom the `skillset` and `myconfigs` repos already use. It is a thin wrapper +that `exec`s `scripts/lint-shell.sh` — the same script CI runs, one copy, because +a duplicated check that drifts is the failure this repo keeps paying for (the +`pi-extensions` skill mirror sat 9579 B behind for weeks; the shell-lint logic +was extracted to one file for exactly this reason). + +> **Why this repo had no hooks at all, which is worth stating because it was +> reported as drift and is not.** A fleet peer asked `tor-ms22` to report +> `git config core.hooksPath` per clone on the premise that an unset value meant +> "no secret-scan and no shell-lint hook locally", leaving the drift/secret gates +> unverified. Measured: `pi-devbox` **unset**, `skillset` `hooks`, `myconfigs` +> `common/hooks`, `pi-toolkit` **unset**. But `git ls-files | grep -i hook` is +> **empty** in both `pi-devbox` and `pi-toolkit` — neither repo tracked a single +> hook file, so there was nothing for `core.hooksPath` to point at on any machine +> and unset was the only correct value. The two repos that do ship hooks were +> already wired correctly. This entry closes the real half of that gap for +> `pi-devbox`; `pi-toolkit` still ships none. + +**The hook is verified to catch the defect that motivated it, not merely to +exist.** Three measurements, each with the expected result written down first: + +- **Refusal paths.** With `shellcheck` absent (the state of every container built + before this change) the hook exits **2** and names the remedy; with + `scripts/lint-shell.sh` missing it also exits **2**. It never waves a push + through on the assumption that CI will catch it. +- **The hook is actually in the scan set.** `lint-shell.sh` reports `Checking 14 + shell file(s)` with `hooks/pre-push` present and **13** with it moved aside — so + the extensionless file is discovered by the shebang half of the linter's + two-signal union, rather than being silently skipped. This check exists because + the first attempt at it was ambiguous: a planted `[ $UNSET_VAR = "x" ]` was not + reported, which could equally have meant "file not scanned" or "defect below + `-S error`". It was the latter. A count that moves is unambiguous; a clean run + is not. +- **It catches the real v1.8.14 defect.** Planting the exact failing shape — an + apostrophe inside a single-quoted string, `echo 'the fleet\'s thing'` — in + `hooks/pre-push` produces `SC1073`/`SC1072` at severity **error** and `rc=1`. + That is the defect that closed a string, truncated an `exec_test` body, sent its + tail to the runner's shell, and cost a 46-minute build. + +**The gate earned its keep inside the commit that added it.** The first version of +the `smoke-test.sh` assertion above carried a comment beginning `# shellcheck is a +GATE DEPENDENCY…`. A comment whose first word is the tool's name is parsed as a +**shellcheck directive**, not a comment, so the new gate immediately failed with +`SC1073`/`SC1072` at severity error — on the change that introduced it. Same family +as the v1.8.14 apostrophe: a line that reads as prose to a human and as syntax to +the parser. Before this change that defect would have been discovered in CI. + +**Also queued, not yet pinned:** the `mempalace-toolkit` owed-set derivation now +honours a requester withdrawing its *own* ask (`isWithdrawn`, RFC 003 §3.3 +clause 4, with `scripts/test-owed-withdrawal.sh`) — toolkit commit **`e2b060a`**, +which is the minimum revision for the behaviour. This image still pins `e45f6b4`. +Until an image bakes `e2b060a` or later, a sender must assume its withdrawal has +no effect on the recipient's mailbox — measured cost of the gap: a withdrawn +v1.8.13 rollout ask was still being reported as owed on `tor-ms22` 41 hours later, +for a release that device never installed. The same commit also anchors the +derivation's `mine` query at the newest end (`order: "desc"`); with the previous +default `asc` + `limit: 100`, a device passing 100 authored events would have its +recent replies fall out of the join window and see answered asks resurface. + +--- + ## v1.8.14 — 2026-09-08 > **First release attempt failed; fixed in this same entry.** The `smoke` and diff --git a/Dockerfile.base b/Dockerfile.base index b6b2c1b..6f66c85 100644 --- a/Dockerfile.base +++ b/Dockerfile.base @@ -101,6 +101,27 @@ ENV DEBIAN_FRONTEND=noninteractive # container: `ss` lands at /usr/bin/ss, `ip` at /usr/sbin/ip # (both already on the developer PATH), and `portcheck --all` # then correctly identifies the socat listener on 8765. +# shellcheck — shell linter. Added 2026-09-09 to close a CAPABILITY gap, not +# a style preference. `scripts/lint-shell.sh` is the release +# GATE (the `lint-gate` job that `resolve-versions` depends +# on), and it correctly refuses to pass when shellcheck is +# missing — "a gate that cannot run must not pass". Measured on +# v1.8.14: shellcheck was absent from this image by all three +# routes (PATH, dpkg, filesystem), so `bash +# scripts/lint-shell.sh` exited 2 in EVERY devbox container and +# no developer could run the release gate locally at all. The +# loop was therefore write-shell → push → wait for CI → discover, +# which is the loop the gate was added to shorten: v1.8.14's +# first attempt burned ~46 min on a tree whose lint had already +# been red for 24 h. This is also what makes a client-side +# pre-push hook possible (see hooks/pre-push); without the +# binary that hook would refuse every push. ~39 MB installed +# (Installed-Size 40112 KB, shellcheck 0.10.0-1) and measured +# to pull ZERO additional packages under +# --no-install-recommends: its deps (libc6, libffi8, libgmp10) +# are already present. NOTE this file feeds the base-decide +# hash (Dockerfile.base + rootfs/), so adding it forces one +# full base rebuild. RUN apt-get update && \ apt-get upgrade -y --no-install-recommends && \ apt-get install -y --no-install-recommends \ @@ -120,6 +141,7 @@ RUN apt-get update && \ make \ patch \ diffutils \ + shellcheck \ git-crypt \ age \ file \ diff --git a/hooks/pre-push b/hooks/pre-push new file mode 100755 index 0000000..e91f5ec --- /dev/null +++ b/hooks/pre-push @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Pre-push gate for pi-devbox: shellcheck every shell script before it leaves +# this clone. Thin wrapper — all logic lives in scripts/lint-shell.sh, which is +# the SAME script the CI release gate runs. One copy, not two: a duplicated +# check that drifts is the failure this repo keeps paying for. +# +# Install per clone: git config core.hooksPath hooks +# Bypass this gate: git push --no-verify (a guard, not a wall) +# +# WHY THIS HOOK EXISTS +# v1.8.14's first release attempt died at scripts/smoke-test.sh:770 after +# build-base had already spent ~46 minutes. shellcheck had ALREADY caught the +# defect — SC2289 at severity error, on the very push that introduced it — and +# the lint job stayed red for 24 hours, unread, across three runs. The fix at +# the time was to gate the release on the same script (the `lint-gate` job). +# This hook is the cheaper end of that: the same finding, before the push, +# in seconds rather than after a 40 s CI gate or a 46 min build. +# +# WHY IT COULD NOT EXIST UNTIL NOW +# Measured on v1.8.14 (2026-09-09): shellcheck was absent from the devbox +# image by all three routes — PATH, dpkg and a filesystem search. So +# lint-shell.sh exited 2 in every container, and a hook calling it would have +# refused EVERY push rather than gating anything. `shellcheck` was added to +# Dockerfile.base in the same change that added this file; on an image built +# before that, enable this hook and you will simply be told the gate cannot +# run. That is the correct behaviour, but it is not a working hook — so do not +# set core.hooksPath on a container older than the release that bakes it. +# +# NOTE ON SCOPE: this lints the WORKING TREE, not the exact commit range being +# pushed. That is deliberate and matches what the CI gate does to the tagged +# tree. It means a defect you have staged-but-not-committed is also reported, +# which is noisy in the safe direction. +set -euo pipefail + +HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "$HOOK_DIR/.." && pwd)" +LINTER="$REPO_ROOT/scripts/lint-shell.sh" +tag="[lint-shell]" + +# Same rule the gate itself applies, applied one level up: a missing check is +# not a pass. If the script is gone, the push is refused rather than waved +# through on the assumption that CI will catch it. +if [ ! -r "$LINTER" ]; then + echo "$tag refusing the push: $LINTER is missing, so the gate cannot" >&2 + echo "$tag run. A gate that cannot run must not pass." >&2 + exit 2 +fi + +# Point the message at the actual remedy when the binary is absent, because the +# linter's own message ("install it or run this in CI") is written for a CI +# runner and is misleading inside a container the developer cannot apt-install +# into persistently. +if ! command -v shellcheck >/dev/null 2>&1; then + echo "$tag refusing the push: shellcheck is not installed, so the gate" >&2 + echo "$tag cannot run. A gate that cannot run must not pass." >&2 + echo "$tag" >&2 + echo "$tag This container predates the image that bakes shellcheck." >&2 + echo "$tag Either recreate onto an image that has it, or unset the hook:" >&2 + echo "$tag git config --unset core.hooksPath" >&2 + echo "$tag To push this once without the gate: git push --no-verify" >&2 + exit 2 +fi + +exec bash "$LINTER" "$REPO_ROOT" diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 5b879c5..c8cc75a 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -105,6 +105,18 @@ else run "node" "node --version" fi run "git" "git --version" +# NOTE: the shellcheck binary is a GATE DEPENDENCY, not a convenience. +# scripts/lint-shell.sh is the release gate (the lint-gate job resolve-versions +# depends on) and it exits 2 when the binary is missing, by design — "a gate that +# cannot run must not pass". Measured on v1.8.14: it was absent from the image, so +# that gate could not be run by a developer in ANY container, only in CI. +# Asserted here so its absence fails a build instead of being discovered by a hook +# that then refuses every push (hooks/pre-push). +# +# This comment must not BEGIN with the tool's name: a line starting with +# `# shellcheck` is parsed as a DIRECTIVE, not a comment (SC1073/SC1072). The +# gate added in this same change caught that here, before the push. +run "shellcheck (lint gate dependency)" "shellcheck --version | grep -qE '^version: [0-9]'" run "aws" "aws --version" run "uv" "uv --version" run "nvim" "nvim --version"