From 3783728e2d16cc2178b5edf03d1cf3d716b850f7 Mon Sep 17 00:00:00 2001 From: monkopedia-coder Date: Tue, 15 Sep 2026 00:20:07 -0400 Subject: [PATCH] Give the two untested CI gates a self-test, and mechanise the rule (#705) (#759) * Give the two untested CI gates a self-test, and mechanise the rule (#705) `scripts/lib/gate-filters.sh` opens with the rule, deliberately as the first thing a new gate author reads: every `scripts/check-*.sh` MUST ship a matching `scripts/tests/-test.sh` and MUST have it wired into CI -- "that requirement, not this file, is the part that catches the bug". It was a comment, and on main it was false for two of the nine gates. check-apk-distribution.sh is the one that matters. release.yml runs it as "Staged FOSS APK contains no Google/Firebase code"; it is the whole of #548's answer, and a violation would be invisible until someone decompiled a shipped APK. It had never been observed going red. Being invoked in CI is not the same as being known to work -- #678's step-order gate ran on every PR for months while exiting 1 with zero output on the two inputs it existed to catch, and #686's backfill exited 0 after a failed secret push. Every case in both harnesses is a demonstrated control, red case first. scripts/tests/check-apk-distribution-test.sh (42 cases). FIXTURES, NOT REAL APKs: the gate reads an APK as a zip of files and greps the bytes, so a zip of text files exercises every path it has -- the 1MB size floor, the dex-presence check and the com/rousecontext control included -- in about two seconds with no SDK. Red: firebase and gms DEX class references separately; each of the three google-services string resources on its own; a dot-form provider in the manifest; res/raw/firebase_common_keep.xml; --expect google over a clean APK (the double-staging case release.yml's shared output path makes possible); firebase without gms; a resources-only marker not satisfying --expect google; and the five preflight refusals (missing, too small, not a zip, no dex, not this app). Green: a clean fixture, and -- pinned because it is the judgement most likely to be "tidied" later -- the four dot-form GMS strings a genuinely clean FOSS build carries in its DEX, alongside the same string in the manifest asserted RED, so the slash-vs-dot asymmetry is measured rather than assumed. Part 5 mutates the gate so a pattern stops matching its own sample, and requires the gate's own anti-vacuity self-test to fire (#579's failure mode). scripts/tests/check-zombie-screens-test.sh (36 cases). #657 already found this gate printing "OK: no zombie screens" and exiting 0 over a tree containing one, with a stubbed-failing `awk`, so that red case is pinned rather than described: the real gate is run with a PATH-shimmed failing `awk` and must fail by name, and a reconstruction of the pre-#657 shape must still produce the lie on the same tree -- otherwise the first assertion could be passing for an unrelated reason. Also pinned: self-reference does not rescue a screen, nor a test-only reference, nor a non-Kotlin file, nor a substring of the name; the exemption list works and covers only the file it names. Three residues are asserted rather than left implicit -- one live composable vouches for its whole file, a PUBLIC @Preview composable is NOT exempt (only `private fun` is dropped), and the gate reports OK when its screens directory does not exist, which is green-by-absence and is filed as #758. scripts/check-gate-selftests.sh then makes the rule mechanical: every check-*.sh has a harness, and every harness is named in a workflow. The second half matters more than it looks -- a harness nobody runs is a file, not a gate. Discovery is `git ls-files`, membership is tested against the TRACKED list (an untracked harness satisfies `[[ -f ]]` locally and does not exist in CI's checkout), and zero gates, zero harnesses or zero workflows is a FAILURE rather than a vacuous pass. The predicate is name-based for a content-based class -- the objection #658 raises about the shellcheck gate's `*.sh` -- and the header says so explicitly instead of inheriting it silently: the convention is uniform across all nine gates, proximity enforces it, and the miss direction is safe (a gate not named check-* escapes the check; it cannot cause a false red). backfill-relay-secrets.sh is deliberately outside the gate set, and its harness is still covered by the wiring half -- nothing forces it to HAVE a self-test, but having one means it must be RUN. Its own harness (27 cases) covers both gaps, both together, the untracked- harness case, and every way the gate could look at an empty set and call it clean -- no gates, no harnesses, no workflows, not a work tree, no such directory. A coverage check is the easiest kind to make vacuous (#629 proved comparisons happened, not that they could detect anything). All three harnesses read the gate's output with `grep` over a FILE, never `printf ... | grep -q`. That pipeline folds three outcomes into one answer -- no match, grep could not answer, and grep matching EARLY and `pipefail` reporting the writer's SIGPIPE for `!` to invert into "no match" -- and the last is payload-size dependent and tracked as a live defect in #725. With no pipe there is no writer to kill. Four steps added to ci.yml's lint job, next to the five self-tests already there. release.yml is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YYSPuh9bZEJkrzvnEpY1QK * TEMPORARY: a gate with no self-test, to watch #705's check go red in CI Reverted in the next commit. `scripts/check-gate-selftests.sh` is itself a gate, and a gate whose red case has only ever been exercised against fixtures in its own harness is exactly what #705 is about. This makes the real CI step fail on the real tree, once, so the link is citable. Expected: the "Every gate ships a wired self-test (#705)" step in the lint job exits 1 with 1 gate(s) ship with no self-test: scripts/check-fixture-no-harness.sh -> scripts/tests/check-fixture-no-harness-test.sh Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YYSPuh9bZEJkrzvnEpY1QK * Revert the temporary no-harness fixture gate The point is made: run 34925372964, job Lint, step "Every gate ships a wired self-test (#705)" exited 1 on the real tree with 1 gate(s) ship with no self-test: scripts/check-fixture-no-harness.sh -> scripts/tests/check-fixture-no-harness-test.sh and the two new self-test steps ahead of it passed on the same run. The coverage gate is now a demonstrated red, not an assumed one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YYSPuh9bZEJkrzvnEpY1QK --------- Co-authored-by: Monkopedia Co-authored-by: Claude Opus 5 --- .github/workflows/ci.yml | 26 + scripts/check-gate-selftests.sh | 200 ++++++++ scripts/tests/check-apk-distribution-test.sh | 473 +++++++++++++++++++ scripts/tests/check-gate-selftests-test.sh | 323 +++++++++++++ scripts/tests/check-zombie-screens-test.sh | 467 ++++++++++++++++++ 5 files changed, 1489 insertions(+) create mode 100755 scripts/check-gate-selftests.sh create mode 100755 scripts/tests/check-apk-distribution-test.sh create mode 100755 scripts/tests/check-gate-selftests-test.sh create mode 100755 scripts/tests/check-zombie-screens-test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 763de662..51b67446 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,6 +150,32 @@ jobs: # test stubs `gh` and `gcloud`, so it never reaches GitHub or the VPS. - name: "backfill-relay-secrets self-test (#686)" run: bash scripts/tests/backfill-relay-secrets-test.sh + # The FOSS-purity gate. release.yml runs it as "Staged FOSS APK contains + # no Google/Firebase code" -- the whole of #548's answer -- and until #705 + # it had never been observed going red. Fixtures, not real APKs: the gate + # reads an APK as a zip of files and greps the bytes, so a zip of text + # files exercises every path it has, in a second, with no SDK. + - name: "APK distribution gate self-test (#705, #548)" + run: bash scripts/tests/check-apk-distribution-test.sh + # #657 measured this gate printing "OK: no zombie screens" and exiting 0 + # over a tree containing one, with a stubbed-failing `awk`. That red case + # is pinned here, alongside a reconstruction of the pre-#657 shape that + # must still produce the lie -- so the pin cannot pass for an unrelated + # reason. + - name: "Zombie-screens gate self-test (#705, #657)" + run: bash scripts/tests/check-zombie-screens-test.sh + # The rule the two steps above satisfy, made mechanical: every + # scripts/check-*.sh has a scripts/tests/-test.sh, and every + # self-test is invoked by a workflow. It was a comment in + # scripts/lib/gate-filters.sh, and on main it was false. + - name: "Every gate ships a wired self-test (#705)" + run: bash scripts/check-gate-selftests.sh + # ...and the coverage gate is itself a gate, so it has its own harness. + # A coverage check is the easiest kind to make vacuous -- discover + # nothing, compare nothing, report clean (#629) -- so every way it could + # look at an empty set is a red case in here. + - name: "Gate self-test coverage gate self-test (#705)" + run: bash scripts/tests/check-gate-selftests-test.sh - uses: actions/setup-java@v4 with: distribution: temurin diff --git a/scripts/check-gate-selftests.sh b/scripts/check-gate-selftests.sh new file mode 100755 index 00000000..71739c27 --- /dev/null +++ b/scripts/check-gate-selftests.sh @@ -0,0 +1,200 @@ +#!/usr/bin/env bash +# Fails if a CI gate ships without a self-test, or if a self-test is not wired +# into a workflow (#705). +# +# WHY THIS EXISTS +# --------------- +# The rule was already written down. `scripts/lib/gate-filters.sh` puts it +# first, in a block headed READ THIS FIRST IF YOU ARE ADDING A GATE: +# +# Every `scripts/check-*.sh` MUST ship a matching +# `scripts/tests/-test.sh` and MUST have that self-test wired into CI. +# That requirement -- not this file -- is the part that catches the bug. +# +# It was a comment, and on `main` it was false: `check-apk-distribution.sh` -- +# the FOSS-purity gate `release.yml` runs as the whole of #548's answer -- and +# `check-zombie-screens.sh` had no harness at all. Both were fixed in the change +# that added this file; this is what stops the tenth gate arriving without one. +# +# The argument for mechanising it is the repo's own history. A gate that has +# only ever been observed passing is indistinguishable from one that cannot +# fail, and this tree has shipped that four times: +# +# #577/#579 a post-filter silently swallowing real violations, reporting clean +# #657 a stubbed `awk` making check-zombie-screens.sh print OK over a zombie +# #678 check-roborazzi-step-order.sh exiting 1 with zero output -- a crash +# reading as a finding -- on the two inputs it existed to catch +# #686 backfill-relay-secrets.sh exiting 0 after a failed secret push +# +# Every one was caught by a self-test, or by a human noticing what a self-test +# would have caught. None was caught by the gate itself running in CI. +# +# WHAT COUNTS AS A GATE -- a name-based predicate, stated rather than inherited +# --------------------------------------------------------------------------- +# The set is `git ls-files 'scripts/check-*.sh'`: a NAME-based predicate for +# what is really a content-based class, which is the objection #658 raises +# against the shellcheck gate's `*.sh`. It is accepted here deliberately, and +# the reasons are worth stating so the next reader does not have to guess: +# +# * the convention is real and uniform -- all nine gates on `main` when this +# landed are named `check-.sh`, and each is invoked from a workflow +# under that name; +# * proximity enforces it -- a new gate is written next to nine files that +# follow the convention, and its CI step is added next to theirs; +# * the failure direction is safe. A script that IS a gate but is not named +# `check-*` escapes this check; it does not cause a false red. The cost of +# the miss is the status quo ante, not a broken build. +# +# `scripts/backfill-relay-secrets.sh` is deliberately OUTSIDE the set: it is not +# a gate, it writes the production relay secrets. It happens to have a harness +# (#686, after it exited 0 over a failed push) and that harness is wired into +# CI, so the second half of this check covers it -- which is the right split. +# Being outside the gate set means nothing forces it to HAVE a self-test; having +# one means the self-test must be RUN. +# +# WHAT IS CHECKED +# --------------- +# 1. every `scripts/check-*.sh` has `scripts/tests/-test.sh`; +# 2. every `scripts/tests/*-test.sh` is named in a `.github/workflows/*.yml`. +# +# (2) is the half that matters more than it looks. A harness nobody runs is a +# file, not a gate, and it rots silently: the gate it covers keeps passing CI +# while the thing that would notice a lie sits unexecuted. It also covers this +# script's own harness, and every harness for a non-`check-` subject. +# +# HOW IT FAILS +# ------------ +# Loudly, by name, and never green by absence: zero gates, zero harnesses or +# zero workflow files is a FAILURE, because each of those is a way for this +# check to look at nothing and call it clean -- the #547/#579 shape it exists to +# prevent. Discovery is `git ls-files`, so it refuses to run outside a work +# tree for the same reason `check-shell-lint.sh` does. + +set -euo pipefail + +root=${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)} + +if [[ ! -d "$root" ]]; then + echo "FAIL: no such directory: $root" >&2 + exit 1 +fi +cd "$root" + +if ! git rev-parse --git-dir >/dev/null 2>&1; then + echo "FAIL: $root is not a git work tree." >&2 + echo " Discovery is 'git ls-files', so outside a work tree this gate" >&2 + echo " would find nothing and 'pass'. It fails instead." >&2 + exit 1 +fi + +# list_tracked -- tracked files matching a pathspec, into +# `tracked`. +# +# `git ls-files` is read with its status CHECKED, and the emptiness test happens +# BEFORE the split. Both matter. A process substitution reports no status at +# all, so a failed `git` would look like an empty repo (the #657 shape); and +# `mapfile -t a <<<""` yields a ONE-element array holding the empty string, +# which would sail past a `${#a[@]} -gt 0` test and then be used as a filename. +tracked=() +list_tracked() { + local what=$1 raw status + shift + set +e + raw=$(git ls-files -- "$@") + status=$? + set -e + if [[ "$status" -ne 0 ]]; then + echo "FAIL: 'git ls-files -- $*' exited $status looking for $what." >&2 + echo " Nothing was discovered, so this is a failure and not a pass." >&2 + exit 1 + fi + if [[ -z "$raw" ]]; then + echo "FAIL: no $what found (pathspec: $*)." >&2 + echo " An empty discovery would make every check below vacuously" >&2 + echo " true, so it is reported as a failure." >&2 + exit 1 + fi + mapfile -t tracked <<<"$raw" +} + +list_tracked 'CI gates' 'scripts/check-*.sh' +gates=("${tracked[@]}") +list_tracked 'gate self-tests' 'scripts/tests/*-test.sh' +harnesses=("${tracked[@]}") +list_tracked 'workflow files' '.github/workflows/*.yml' '.github/workflows/*.yaml' +workflows=("${tracked[@]}") + +echo "Checking gate self-test coverage" +echo " gates : ${#gates[@]} (scripts/check-*.sh)" +echo " self-tests : ${#harnesses[@]} (scripts/tests/*-test.sh)" +echo " workflows : ${#workflows[@]}" +echo + +# Membership is tested against the TRACKED list, not the filesystem: an +# untracked harness satisfies `[[ -f ]]` locally and does not exist in CI's +# checkout, which is the one place this check's answer matters. Same trade +# `check-shell-lint.sh` makes, and the same reason `git add` before running +# either of them locally is worth the habit. +harness_list=$'\n' +for harness in "${harnesses[@]}"; do + harness_list+="$harness"$'\n' +done + +missing_harness=() +for gate in "${gates[@]}"; do + name=$(basename "$gate" .sh) + harness="scripts/tests/$name-test.sh" + if [[ "$harness_list" == *$'\n'"$harness"$'\n'* ]]; then + printf ' %-10s %-40s %s\n' 'HAS' "$name" "$harness" + else + printf ' %-10s %-40s %s\n' 'NONE' "$name" "(expected $harness)" + missing_harness+=("$gate -> $harness") + fi +done +echo + +# Which workflow, if any, runs this harness. The search is for the harness's +# own path, which is how every wired step in this repo spells it +# (`run: bash scripts/tests/-test.sh`). +unwired=() +for harness in "${harnesses[@]}"; do + set +e + hits=$(grep -lF -e "$harness" -- "${workflows[@]}") + status=$? + set -e + if [[ "$status" -gt 1 ]]; then + echo "FAIL: grep exited $status looking for '$harness' in the workflow files." >&2 + echo " The search did not complete; this is a failure, not a pass." >&2 + exit 1 + fi + if [[ "$status" -eq 0 ]]; then + printf ' %-10s %-40s %s\n' 'WIRED' "$(basename "$harness")" "${hits//$'\n'/ }" + else + printf ' %-10s %-40s %s\n' 'UNWIRED' "$(basename "$harness")" '(no workflow runs it)' + unwired+=("$harness") + fi +done +echo + +if [[ "${#missing_harness[@]}" -gt 0 || "${#unwired[@]}" -gt 0 ]]; then + echo "GATE SELF-TEST CHECK FAILED (issue #705)" >&2 + echo >&2 + if [[ "${#missing_harness[@]}" -gt 0 ]]; then + echo " ${#missing_harness[@]} gate(s) ship with no self-test:" >&2 + printf ' %s\n' "${missing_harness[@]}" >&2 + echo >&2 + fi + if [[ "${#unwired[@]}" -gt 0 ]]; then + echo " ${#unwired[@]} self-test(s) are not invoked by any workflow:" >&2 + printf ' %s\n' "${unwired[@]}" >&2 + echo >&2 + fi + echo " A gate that has only ever been observed passing is indistinguishable" >&2 + echo " from one that cannot fail. Write the harness -- red case FIRST, so the" >&2 + echo " gate is demonstrated capable of failing -- and add a step that runs it." >&2 + echo " scripts/tests/check-zombie-screens-test.sh is a short example; see also" >&2 + echo " the rule in scripts/lib/gate-filters.sh." >&2 + exit 1 +fi + +echo "OK: all ${#gates[@]} gate(s) have a self-test, and all ${#harnesses[@]} self-test(s) are wired into a workflow." diff --git a/scripts/tests/check-apk-distribution-test.sh b/scripts/tests/check-apk-distribution-test.sh new file mode 100755 index 00000000..d9d09d36 --- /dev/null +++ b/scripts/tests/check-apk-distribution-test.sh @@ -0,0 +1,473 @@ +#!/usr/bin/env bash +# Regression tests for scripts/check-apk-distribution.sh (#705, #548). +# +# WHY THIS FILE EXISTS +# -------------------- +# `check-apk-distribution.sh` is the FOSS-purity gate: release.yml runs it as +# "Staged FOSS APK contains no Google/Firebase code", and it is the whole of +# #548's answer. Until this file, it had never been observed going RED. That is +# the distinction half this repo's gate work is about -- #678's step-order gate +# ran on every PR for months while exiting 1 with zero output on the two inputs +# it existed to catch, and #686's backfill exited 0 after a failed push. A gate +# that has only ever been seen passing is indistinguishable from one that +# cannot fail, and this particular one guards a property (no Firebase in the +# F-Droid artifact) whose violation would be invisible until somebody +# decompiled a shipped APK. +# +# So every case below is a demonstrated control in BOTH directions: a fixture +# that CONTAINS what the gate exists to catch, asserted red and asserted to NAME +# what it found; and a clean fixture asserted green. +# +# NO REAL APK IS BUILT OR DOWNLOADED. The gate reads an APK as a zip of files +# and greps the bytes, so a zip of text files exercises every path it has -- +# including the size floor, the dex-presence check, and the `com/rousecontext` +# control. Building a real APK here would make this suite need a JDK, an SDK +# and four minutes, and would test Gradle rather than the gate. + +set -euo pipefail + +repo_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +gate=$repo_root/scripts/check-apk-distribution.sh + +sandbox=$(mktemp -d) +trap 'rm -rf "$sandbox"' EXIT + +failures=0 +out_file=$sandbox/gate-output.txt +stage_dir=$sandbox/stage +run_status=0 +run_ok=0 +has_verdict= +has_status=0 + +if ! command -v zip >/dev/null 2>&1; then + echo "FAIL: 'zip' is not installed; the fixture APKs cannot be built." >&2 + echo " Skipping would make this suite green by absence, so it fails." >&2 + exit 1 +fi + +# --------------------------------------------------------------------------- +# Reading the gate's output. +# +# Deliberately `grep` over a FILE, never `printf ... | grep -q` (#716/#725). +# That pipeline has three outcomes the caller cannot tell apart: grep found +# nothing; grep never answered; or grep exited early ON A MATCH and the writer +# took SIGPIPE, which `pipefail` then reports as the pipeline's status and `!` +# inverts into "no match". The last is payload-size dependent -- measured clean +# at 4.4KB and failing 15116 times in 20000 at 32KB -- and this gate's failure +# output grows with the number of markers it found, which is precisely the case +# these assertions run on. With no pipe there is no writer to kill, so the shape +# cannot arise; grep's own status is read, and a status above 1 (it could not +# answer) is reported as its own thing rather than folded into "absent". +# --------------------------------------------------------------------------- +out_has() { + set +e + grep -qF -e "$1" "$out_file" + has_status=$? + set -e + case "$has_status" in + 0) has_verdict=yes ;; + 1) has_verdict=no ;; + *) has_verdict=broken ;; + esac +} + +fail_case() { + echo "FAIL: $*" + sed 's/^/ /' "$out_file" + failures=$((failures + 1)) +} + +# --------------------------------------------------------------------------- +# Fixture APKs. +# +# `stage_clean` lays out the minimum the gate demands of ANY artifact before it +# will render a verdict: a top-level classes.dex, app code inside it, and a +# megabyte of bulk. Individual cases then add the markers they are about. +# +# The padding is stored (`zip -0`) rather than deflated because the gate's size +# floor reads the FILE, and a megabyte of one repeated byte compresses to +# nothing. It is a run of 'A': no pattern in the gate matches it, so it cannot +# accidentally become the thing a case is measuring. +# --------------------------------------------------------------------------- +stage_clean() { + rm -rf "$stage_dir" + mkdir -p "$stage_dir/res/raw" "$stage_dir/assets" "$stage_dir/META-INF" + # DEX type descriptors: slash-form, `L...;`-wrapped, as the real thing is. + printf '%s\n' \ + 'Lcom/rousecontext/app/MainActivity;' \ + 'Lcom/rousecontext/app/ui/AppNavigation;' \ + 'Lcom/rousecontext/core/mcp/McpServer;' > "$stage_dir/classes.dex" + printf '%s\n' '' \ + > "$stage_dir/AndroidManifest.xml" + printf '%s\n' 'app_name' 'rouse_context_label' > "$stage_dir/resources.arsc" + printf '%s\n' 'MANIFEST-VERSION: 1.0' > "$stage_dir/META-INF/MANIFEST.MF" + head -c 1100000 /dev/zero | tr '\0' 'A' > "$stage_dir/assets/pad.bin" +} + +# seal -> the path of the fixture APK, in $apk +seal() { + apk=$sandbox/$1 + rm -f "$apk" + ( cd "$stage_dir" && zip -q -0 -r -X "$apk" . ) +} + +# --------------------------------------------------------------------------- +# run