From e7fe7ad6aade6f4d3b7364745e3e0bf706a8d2b4 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:05:34 +0100 Subject: [PATCH 1/2] fix(gates): composite shell -e silently killed two gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub runs a composite `shell: bash` step as `bash --noprofile --norc -e -o pipefail {0}`. The `-e` comes from the harness, and a script's own `set -uo pipefail` does not clear it. Any `x=$(grep ... )` is therefore a silent kill site whenever grep legitimately matches nothing. Two gates were dying on it, both in the case they exist to handle: required-files-check A comment-only CODEOWNERS made `grep -v ... | wc -l` exit 1, killing the step ONE LINE ABOVE the message declaring that exact file valid ("valid for solo-maintained repos per Rule 1"). The gate killed precisely the repos it was written to accept. Measured on pons-asinorum run 35782895038: the log stops after the last successful echo, with no `::error::` anywhere, and the check-run annotation carries only "Process completed with exit code 1". Because this is step 3 of the estate audit, its red SKIPS 25 downstream gates — one unguarded pipeline blanked the whole audit for every solo-maintained repo in the estate. spdx-license-check `git grep -i "SPDX-License-Identifier" | wc -l` exits 1 on a repo with zero SPDX lines, so the step hard-failed and the `::warning::` branch beneath it was UNREACHABLE — inverting the intent stated in the comment beside it ("disabled strictly to prevent total CI blockage until adoption"). Fixes: - required-files-check restores its own contract with `set +e`. The script accumulates `fail=1` to report EVERY defect and exit once at the end; `-e` means "die on the first", which is the opposite. Both count sites are additionally guarded with `|| true` so the intent survives someone re-adding `-e`. - spdx-license-check guards its `git grep` so the zero-SPDX branch can be reached at all. - formatting-check and metrics-check already used `|| true`/`|| echo 0` at every such site and are untouched; the sweep found no others. tests/composite-shell-contract.sh runs both gates under the exact CI shell against a fixture repo that is valid under every rule they state, and KILLS A MUTANT: it reinstates the unguarded pipeline, asserts both mutations actually applied, asserts the mutant parses, and requires it to die SILENTLY with no `::error::` — the CI signature. Without that last check the red could come from fixture drift and prove nothing about `-e`. Wired into code-hygiene-self-test.yml. PASS=7 FAIL=0. Refs: #787 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WRvDivYwLSeVCJUrfjic3f --- .github/workflows/code-hygiene-self-test.yml | 11 ++ actions/required-files-check/action.yml | 15 +- actions/spdx-license-check/action.yml | 7 +- tests/composite-shell-contract.sh | 156 +++++++++++++++++++ 4 files changed, 186 insertions(+), 3 deletions(-) create mode 100755 tests/composite-shell-contract.sh diff --git a/.github/workflows/code-hygiene-self-test.yml b/.github/workflows/code-hygiene-self-test.yml index 4f1efff..8f3f8e1 100644 --- a/.github/workflows/code-hygiene-self-test.yml +++ b/.github/workflows/code-hygiene-self-test.yml @@ -13,6 +13,9 @@ on: - 'actions/secrets-check/**' - 'actions/boj-cartridge-check/**' - 'actions/manifest-check/**' + - 'actions/required-files-check/**' + - 'actions/spdx-license-check/**' + - 'tests/**' - '.github/workflows/code-hygiene-self-test.yml' pull_request: paths: @@ -22,6 +25,9 @@ on: - 'actions/secrets-check/**' - 'actions/boj-cartridge-check/**' - 'actions/manifest-check/**' + - 'actions/required-files-check/**' + - 'actions/spdx-license-check/**' + - 'tests/**' - '.github/workflows/code-hygiene-self-test.yml' permissions: @@ -45,6 +51,11 @@ jobs: - run: bash actions/referencing-check/test.sh - run: bash actions/secrets-check/test.sh - run: bash actions/boj-cartridge-check/test.sh + # Proves the composite shell contract: GitHub runs `shell: bash` with + # `-e`, which a script's own `set -uo pipefail` does not clear. Two gates + # were dying silently on it. This suite kills a mutant, so a green square + # here means the contract is actually tested. + - run: bash tests/composite-shell-contract.sh - name: Create manifest controls id: fixtures shell: bash diff --git a/actions/required-files-check/action.yml b/actions/required-files-check/action.yml index f0253cd..1befebd 100755 --- a/actions/required-files-check/action.yml +++ b/actions/required-files-check/action.yml @@ -31,6 +31,17 @@ runs: # .txt licence texts # fixed names GitHub or convention dictates (CODEOWNERS, funding.yml, # NOTICE, AUTHORS, MAINTAINERS) keep their form + # GitHub runs a composite step as `bash --noprofile --norc -e -o pipefail`. + # That `-e` comes from the harness, not from here, and `set -uo pipefail` + # does NOT clear it. It directly contradicts this gate's design: the whole + # script accumulates `fail=1` so it can report EVERY defect and exit once + # at the end. Under `-e` the first non-zero command kills the step instead, + # silently, before the line that would have explained itself. + # Measured 2026-09-22 on pons-asinorum: a comment-only CODEOWNERS made + # `grep -v ... | wc -l` exit 1, which killed the step one line ABOVE the + # message declaring that exact file valid. The count sites below are also + # individually guarded, so the contract survives someone re-adding `-e`. + set +e set -uo pipefail fail=0 @@ -100,7 +111,7 @@ runs: CODEOWNERS) # Rule 1: Solo-owned repos may have comment-only CODEOWNERS # Check if file has any functional (non-comment) lines - functional_lines=$(grep -vE '^\s*(#|//|;|$)' "$f" | wc -l) + functional_lines=$(grep -cvE '^\s*(#|//|;|$)' "$f" || true) # || true: grep exits 1 on a comment-only file if [ "$functional_lines" -eq 0 ]; then # File is all comments - this is valid for solo-maintained repos per CODEOWNERS-POLICY.adoc Rule 1 echo " CODEOWNERS is comment-only (valid for solo-maintained repos per Rule 1)" @@ -114,7 +125,7 @@ runs: esac # Documents are judged on substance. - body=$(grep -vE '^\s*(#|//|;|$)' "$f" | wc -l) + body=$(grep -cvE '^\s*(#|//|;|$)' "$f" || true) # || true: grep exits 1 on an all-comment file if [ "$body" -lt 5 ]; then echo "::error::$f has $body substantive lines — a stub, not a document." fail=1 diff --git a/actions/spdx-license-check/action.yml b/actions/spdx-license-check/action.yml index ac8cd72..47e3bec 100755 --- a/actions/spdx-license-check/action.yml +++ b/actions/spdx-license-check/action.yml @@ -17,7 +17,12 @@ runs: # Check that files have SPDX lines (heuristic check on some files) # We ensure at least one SPDX line for code and one for docs is found in the repo - has_spdx=$(git grep -i "SPDX-License-Identifier" | wc -l) + # GitHub runs this step as `bash -e -o pipefail`. `git grep` exits 1 when + # it matches nothing, so without the guard below the ZERO-SPDX case — the + # only case this check exists to detect — killed the step outright, making + # the `::warning::` branch beneath unreachable and inverting the stated + # intent of the comment beside it. Measured 2026-09-22. + has_spdx=$({ git grep -i "SPDX-License-Identifier" || true; } | wc -l) if [ "$has_spdx" -eq 0 ]; then echo "::warning::No SPDX-License-Identifier lines found in the repository." # exit 1 (disabled strictly to prevent total CI blockage until adoption) diff --git a/tests/composite-shell-contract.sh b/tests/composite-shell-contract.sh new file mode 100755 index 0000000..6a0595c --- /dev/null +++ b/tests/composite-shell-contract.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# +# Composite shell contract test. +# +# GitHub runs every composite `shell: bash` step as: +# bash --noprofile --norc -e -o pipefail {0} +# +# The `-e` is supplied by the harness, not by the script, and a script's own +# `set -uo pipefail` does NOT clear it. Any `x=$(grep ... )` is therefore a +# silent kill site whenever grep legitimately matches nothing. +# +# Two gates were dying that way on 2026-09-22 (measured on pons-asinorum): +# +# required-files-check a comment-only CODEOWNERS killed the step ONE LINE +# ABOVE the message declaring that exact file valid. +# spdx-license-check a repo with zero SPDX lines killed the step, making +# the `::warning::` branch beneath it unreachable and +# inverting the intent stated in its own comment. +# +# This suite reproduces the CI shell exactly, and — critically — kills a mutant. +# A gate suite that only ever goes green proves nothing. + +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PASS=0; FAIL=0 +ok() { printf ' ok %s\n' "$1"; PASS=$((PASS+1)); } +bad() { printf ' FAIL %s\n' "$1"; FAIL=$((FAIL+1)); } + +command -v yq >/dev/null || { echo "yq is required (Y-1: gates read YAML with yq, never grep)"; exit 2; } + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +# --- the fixture: a repo that is VALID under every rule these gates state ---- +# Solo-maintained (comment-only CODEOWNERS, explicitly valid per Rule 1) and +# carrying no SPDX headers (the case spdx-license-check exists to warn about). +FIX="$WORK/fixture" +mkdir -p "$FIX/crates" +cd "$FIX" +git init -q . +: > .editorconfig +: > .gitignore +: > .gitattributes +mkdir -p .github +cat > .github/CODEOWNERS <<'EOF' +# Solo-maintained repository. +# No path-specific owners are assigned; the sole maintainer owns everything. +EOF +cat > GOVERNANCE.adoc <<'EOF' += Governance +This repository is maintained by a single owner. +Decisions are recorded in the issue tracker. +Changes land through pull requests. +Releases are tagged from main. +EOF +cat > ARCHITECTURE.adoc <<'EOF' += Architecture +The implementation lives under crates/ in this repository. +Each crate is an independent unit. +Tests live beside the code they cover. +Build orchestration is a justfile. +EOF +cat > MAINTAINERS <<'EOF' +# Maintainers +hyperpolymath is the sole maintainer of this repository. +Contact goes through the issue tracker. +Security reports follow the disclosure policy. +Releases are cut by the maintainer. +Pull requests are reviewed by the maintainer before merge. +EOF +: > mise.toml +git add -A >/dev/null 2>&1 +git -c user.email=t@example.invalid -c user.name=t commit -qm init >/dev/null 2>&1 + +# --- run a composite's script under the EXACT CI shell ---------------------- +extract() { yq -r '.runs.steps[0].run' "$ROOT/actions/$1/action.yml"; } + +# Runs a composite script under the exact CI shell. The result cannot come back +# through a command substitution — that would run this in a subshell and lose +# the exit code — so it lands in $GATE_OUT / $GATE_RC. +GATE_OUT="$WORK/gate.out" +run_gate() { #