Files
ersatztv/scripts/ci-step-ran.sh
T
timothy 0cf355e494 fix(772,792): address both cold reviews — an unverified pin is no longer a green job
Two independent reviewers (one cross-family) converged on the same defect, and it was the
important one: the preflight WARNED and exited 0 on every answer that was not 200 or 404,
so a missing `curl`, a moved registry or a DNS change would have left it green forever —
"the check could not run" presenting as "the pin is fine", in a script whose own header
disclaimed exactly that. Unknown answers are now retried (3x, 5s) and then FAIL, with
wording kept distinct from the deleted case because the two send an operator to different
places.

Also from the reviews:

  * An absent secret does not arrive as an unset variable. `${{ secrets.X }}:${{ secrets.Y }}`
    interpolates to ":", a perfectly non-empty and perfectly useless credential, and the
    tests covered only the unset shape. Both halves are now required, and the parametrised
    test drives the production shape.
  * HTTP 200 is not a manifest. A proxy or a login page answers 200 too, so the body is
    fetched and matched for `schemaVersion` (a shell `case`, so no jq dependency and no
    pipeline that can inject).
  * The curl stub ignored `-u` and answered 200 regardless, so deleting the real `-u` would
    have left the suite green while the live registry rejected every request. It now 401s an
    unauthenticated read, as the registry does.
  * The mutation's declared diagnostic changed with the script: now that unknown fails too,
    the exit code no longer separates "deleted" from "could not check", so the proof turns on
    the message and `expect` says so.
  * docs/ci-cd.md: `scan` is no longer the only `docker-build.yml` job on the small lane, so
    the tag-push exclusivity claim and the lane membership were both false. Fixed.
  * "Immutable" was overstated: `ci-image.yml` tags `rev-parse --short HEAD`, so a dispatch or
    a weekly no-cache run at the same HEAD republishes that tag from a rebuilt image. Stated,
    along with what the rebuild recovery does NOT restore (mutable bases and apt, so equivalent
    rather than bit-identical).
  * The recovery recipe left you in a worktree checked out at the pin commit — where the
    verify script does not exist, and where the workflow carries the pre-bump pin. It now
    keeps `$repo`, returns, and removes the worktree. It also needed BuildKit's `http = true`
    caveat: the container driver does not inherit the daemon's insecure-registries.
  * The root cause carries its evidentiary limit and its reproduction commands, and says what
    to conclude if a pin vanishes after server-management#842 lands (refuted, not re-applied).
  * The `ci.required-job-step-execution-markers` carve-out named one container-free job; there
    are two now, and the membership is what rots.
  * The decision record's `''` YAML escapes leaked into rendered prose; "status, no comment ->
    ask" is qualified (a prior positive verdict for the SAME head still satisfies condition
    (c)); "exits 1" is "exits non-zero" (usage exits 2, jq its own status, signals 128+n).

refs #772
refs #792
Decisions-Edit: yes
2026-08-22 23:25:26 +02:00

258 lines
16 KiB
Bash
Executable File

#!/usr/bin/env bash
# Per-step execution markers for the two REQUIRED docker-build.yml jobs (ersatztv#756).
#
# WHY THIS EXISTS. A `run:` body the runner declines to interpolate is DROPPED, and the job still
# concludes `success` (ersatztv#751, `ci.workflow-run-body-no-expressions`). In
# `review-verdict.yml` that is fail-CLOSED — the required `review-verdict/h10` is simply absent and
# the merge is blocked. In `docker-build.yml` it is fail-OPEN: `Build & test (.NET)` and
# `EF migration integrity (SQLite + MySql)` are the other two required contexts on `main`, so a
# dropped step there sends a required check GREEN having done no work. #751 guarded the safe
# direction because that is where the live bug was, not because these were checked.
#
# WHY PER STEP, NOT PER JOB, which is what #756 proposed. A marker written by the job's FIRST step
# only proves the job started. The dangerous drop is not step 1 — it is `Test`, or the migration
# replay: the job runs everything around them, reports green, and nothing ran that anyone cared
# about. A guard that cannot see the fail-open case it was built for is the "guard that never
# executed" failure one level up. So every consequential step marks itself and a trailing guard
# asserts the whole expected SET.
#
# THAT GUARD CARRIES NO `if:` — unlike the #751 one, which uses `if: always()` because its job has a
# single real step. These jobs have a dozen, and a genuine early failure legitimately skips every
# later step, so `always()` would print a false "these steps never executed" on top of every ordinary
# red build. The default `success()` is the wanted condition: the guard is skipped only when an
# earlier step FAILED, which already fails the job, so guard-skipped implies job-red and every green
# path runs the guard.
#
# WHY A SCRIPT AND NOT AN INLINE BODY, unlike the #751 guard. Two reasons, and the second is the
# load-bearing one:
#
# * The path literal exists ONCE. The #751 guard carries it twice (write + assert) and its tests
# spend real effort proving the two copies agree, because a divergence reddens every run and
# then gets deleted as broken. Here they cannot diverge.
# * A one-line `run: scripts/ci-step-ran.sh …` CANNOT CONTAIN AN EXPRESSION DELIMITER, so the
# mechanism this guards against cannot drop the guard itself. #751's own record names this as
# the stronger construction ("the body would have had to move into scripts/, where a one-line
# run: makes the class unreachable") and settled for inline only because the measurement showed
# it was not required there.
#
# WHY A SCRIPT IS ACCEPTABLE HERE THOUGH IT WOULD NOT BE IN review-verdict.yml. That workflow
# checks out the PR's BASE precisely so a PR cannot supply the code that judges it. `docker-build.yml`
# is head-resolved by design — a PR already supplies every test this job runs — so calling a script
# from the head adds no authority a PR did not already have. This is a CORRECTNESS gate against
# silent no-ops, not a security gate against a hostile PR; that job belongs to `review-verdict/h10`.
# Do not copy this reasoning back into the gate workflow.
#
# THE MARKER FILE IS KEYED ON THE RUN, and BE PRECISE ABOUT WHY — the obvious justification is a
# #751 measurement that does NOT transfer to these jobs, and saying so is the point. #751 measured
# `RUNNER_TEMP` to be `/tmp` and called it "not a private per-job directory"; that was taken on
# `review-verdict.yml`, which runs WITHOUT a `container:`. `test` and `migrations` run INSIDE the CI
# toolchain image, so their `/tmp` is the job container's own and starts empty. That follows from
# `container:`, NOT from a measurement: the build-lane probe confirmed only that `RUNNER_TEMP` is
# `/tmp` here (the marker landed at `/tmp/etv-ci-steps-ran-test-1910-1`) — it says nothing about the
# directory being private or empty, and an earlier draft of this comment cited it as though it did.
# The fresh container is what actually rules out a stale marker here; the keying is defence in depth.
#
# It is kept because container-per-job is a property of how the lane is configured today, not a
# guarantee, and a STALE marker is the one failure that makes this guard PASS on a run whose step was
# dropped — a silent success, i.e. the exact thing being removed. Cheap insurance against a lane
# change nobody would think to re-check this against.
set -euo pipefail
usage() {
cat >&2 <<'EOF'
usage:
ci-step-ran.sh mark <key>
Record that this step began executing. Call it as the step's FIRST act, before
anything in the body can fail.
ci-step-ran.sh assert --always <key>... [--gated <key>...]
Fail unless every expected key was marked. --always keys are always required.
--gated keys are required only when the job's skip gates did NOT fire, read from
ETV_DOCS_ONLY / ETV_REVALIDATE_SKIP so this mirrors the steps' own `if:`.
EOF
exit 2
}
# NO SILENT FALLBACK FOR THE RUN IDENTITY — found by cold review. The first version defaulted to
# `nojob`/`norunid`/`1`, and those are REUSABLE: with `GITHUB_RUN_ID` unset, every run on the host
# would share ONE marker file, so a leftover from any earlier run would satisfy the guard on a run
# whose step was dropped. A silent PASS — the exact failure the keying exists to remove, reintroduced
# by the code meant to implement it.
#
# THE TWO HALVES ARE TREATED DIFFERENTLY, ON EVIDENCE, because the blast radii differ and this is a
# REQUIRED check — a wrong refusal deadlocks every merge, so strictness is not free:
#
# * `GITHUB_JOB` and `GITHUB_RUN_ID` are MEASURED present on this runner (#756's build-lane probe
# wrote `/tmp/etv-ci-steps-ran-test-1910-1`; `test` is the job id and 1910 is the real API run
# id). Absence would mean the runner changed under us, so refusing is safe AND correct.
# * `GITHUB_RUN_ATTEMPT` is measured present TOO, as of ersatztv#756's own PR run — but note how,
# because the first two attempts to settle it were both bad. Grepping a job log for the variable
# NAME proves nothing (logs do not dump the environment). Inferring it from the ABSENCE of this
# script's "not set" warning proves nothing either, because that warning goes to stderr and
# whether step stderr reaches a job log here was itself never established. So the script was made
# to REPORT its resolved identity on stdout, where capture is not in question, and the answer was
# then simply read off run 1916: `Marker identity: job=test run=1916 attempt=1 (from the runner)`
# and the same for `migrations`. Both required jobs, on the lane that matters.
#
# That measurement is what promoted it from warn-and-default to REQUIRED, which is why the residual
# this comment used to describe — a rerun inheriting attempt 1's markers — no longer exists FOR THE
# CONTAINER JOBS. If a future runner stops exporting any of the three, every job reddens with a
# message naming the variable; that is loud, instantly diagnosable, and the correct direction for a
# required check.
#
# EXCEPT ON A LANE WITH NO `container:` (ersatztv#767, extended by #772). `scan` and
# `toolchain-preflight` both run on `small` with no container, so RUNNER_TEMP is the shared host /tmp
# and the keying below is the ONLY thing separating runs, not defence in depth on top of a fresh
# filesystem. There the rerun residual DOES still exist: a single-job rerun that does not increment
# GITHUB_RUN_ATTEMPT would find the previous attempt's marker file. See the carve-out in
# `ci.required-job-step-execution-markers`; do not read the paragraph above as covering those jobs.
# The list is the property to keep current — a THIRD container-free job added without appearing here
# inherits a reassurance that was never checked for it.
marker_path() {
local missing=""
[ -n "${GITHUB_JOB:-}" ] || missing="$missing GITHUB_JOB"
[ -n "${GITHUB_RUN_ID:-}" ] || missing="$missing GITHUB_RUN_ID"
[ -n "${GITHUB_RUN_ATTEMPT:-}" ] || missing="$missing GITHUB_RUN_ATTEMPT"
if [ -n "$missing" ]; then
# NOTHING IS PRINTED TO STDOUT HERE, and that is load-bearing rather than style: this
# function's stdout IS its return value (it is always called inside `$( )`), so a notice
# printed here is captured INTO the path. An earlier revision did exactly that and both
# sub-commands then failed on a nonexistent directory. Caught by
# test_a_degraded_run_IDENTITY_*, which is why that test asserts on the exit status and on
# the absence of any marker file rather than only on the message.
echo "::error::ci-step-ran.sh cannot identify this run —${missing} not set. The marker path would fall back to a name other runs also use, and a stale marker would make the dropped-step guard PASS on a run whose step never executed (ersatztv#756). Refusing rather than degrading to a reusable name." >&2
exit 3
fi
printf '%s/etv-ci-steps-ran-%s-%s-%s' \
"${RUNNER_TEMP:-${GITHUB_WORKSPACE:-/tmp}}" \
"$GITHUB_JOB" "$GITHUB_RUN_ID" "$GITHUB_RUN_ATTEMPT"
}
cmd_mark() {
[ "$#" -eq 1 ] && [ -n "$1" ] || usage
# Appended, never truncated: every step in the job shares one file, and a `>` here would erase
# its predecessors and make the guard red on every run.
#
# A failure to write is NOT swallowed. The step is running under `bash -e`, so a non-zero here
# fails the step and reddens the job — which is the same direction the guard would take a moment
# later, but with a message pointing at the real cause instead of at a missing marker.
local target
# NOT `>> "$(marker_path)"`: the refusal above `exit`s a SUBSHELL there, and bash discards a
# command substitution's exit status when it is only part of a redirection — the write would go
# to an empty path and the error would read as a redirection failure rather than the real cause.
target="$(marker_path)" || exit $?
printf '%s\n' "$1" >> "$target"
}
cmd_assert() {
local -a always=() gated=()
local bucket=""
while [ "$#" -gt 0 ]; do
case "$1" in
--always) bucket=always ;;
--gated) bucket=gated ;;
-*) usage ;;
*)
case "$bucket" in
always) always+=("$1") ;;
gated) gated+=("$1") ;;
*) usage ;;
esac ;;
esac
shift
done
# ANTI-VACUITY, at runtime rather than only in the test suite. An `assert` called with no
# expectations passes unconditionally and reports "every expected step executed" — a guard that
# proves nothing while looking like it proved everything. Refuse instead.
if [ "${#always[@]}" -eq 0 ] && [ "${#gated[@]}" -eq 0 ]; then
echo "::error::ci-step-ran.sh assert was called with no expected keys, so it would pass unconditionally. This is a workflow bug, not a build failure." >&2
exit 2
fi
# The skip gates, mirroring the `if:` every gated step carries:
# steps.detect.outputs.docs_only != 'true' && steps.revalidate.outputs.skip != 'true'
# Anything other than the exact string `true` means the step was expected to run — including the
# EMPTY string, which is what these read as when the detect step itself was dropped. That
# direction is deliberate: a dropped detect step must widen what is required, never narrow it.
local skipped=no
if [ "${ETV_DOCS_ONLY:-}" = "true" ] || [ "${ETV_REVALIDATE_SKIP:-}" = "true" ]; then
skipped=yes
fi
local marker attempt_used
# `|| exit $?` because `set -e` does NOT fire on a failing command substitution in an assignment;
# without it a degraded identity would leave `marker` empty and every key would read as missing —
# fail-closed by luck, with a misleading message.
marker="$(marker_path)" || exit $?
# Read the attempt back OFF THE RESOLVED PATH rather than from the environment. It reports what
# the path was actually keyed on, so a future change to how the path is built cannot silently
# disagree with the line that documents it.
attempt_used="${marker##*-}"
# `${arr[@]+"${arr[@]}"}` rather than a bare `"${arr[@]}"`: under `set -u` bash 3.2 (the system
# bash on the Macs this suite also runs on) treats expanding an EMPTY array as an unbound
# variable and aborts. The CI image ships bash 5, where it is fine — which is exactly the kind of
# difference that makes a guard pass locally and die on the runner, or the reverse.
local -a expected=(${always[@]+"${always[@]}"})
if [ "$skipped" = no ]; then
expected+=(${gated[@]+"${gated[@]}"})
else
echo "Skip gate fired (docs_only='${ETV_DOCS_ONLY:-}', already_validated='${ETV_REVALIDATE_SKIP:-}') — the gated steps were not expected to run."
fi
# RE-CHECKED AFTER GATING, not only on argv — found by cold review, which reproduced it:
# `ETV_DOCS_ONLY=true … assert --always --gated foo` printed "All 0 expected step(s) executed"
# and exited 0. The argv check above cannot see that, because the set is emptied by the gate, not
# by the caller. Unreachable with today's argv (both jobs pass `--always detect revalidate`), but
# it directly contradicted the comment above it, and a guard that reports proving everything
# while proving nothing is the failure this whole file exists to remove.
if [ "${#expected[@]}" -eq 0 ]; then
echo "::error::ci-step-ran.sh assert ended up with NO expected keys after the skip gate, so it would pass unconditionally. This is a workflow bug, not a build failure." >&2
exit 2
fi
local -a missing=()
local key
for key in "${expected[@]}"; do
# `grep -qxF` over a FILE, never a pipeline: `grep -q` exits at its first match and would
# SIGPIPE a producer, which under `set -o pipefail` inverts the result for large inputs
# (ersatztv#698). Reading the file directly has no producer to kill. `-x` so a key cannot be
# satisfied by another key that contains it, `-F` so a key is never read as a pattern.
if ! grep -qxF "$key" "$marker" 2>/dev/null; then
missing+=("$key")
fi
done
if [ "${#missing[@]}" -gt 0 ]; then
echo "::error::These steps of job '${GITHUB_JOB:-?}' never executed: ${missing[*]}. The runner DROPPED them (an interpolation failure over a run: body does this and still reports the job GREEN — ersatztv#751/#756) or their \`if:\` no longer matches the guard's expectations. This job is a REQUIRED check, so a green here would mean a required context passed having done no work. Failing the job so it is visible."
if [ -f "$marker" ]; then
echo "Marker file ${marker} recorded:"
sed 's/^/ /' "$marker"
else
echo "There is no marker file at ${marker} at all — not one step of this job executed."
fi
exit 1
fi
# The resolved identity, on stdout, every run. This is what turns "is GITHUB_RUN_ATTEMPT
# exported here?" from an inference into something a reader just looks up — and it is why the
# variable is still WARN-and-default rather than REFUSE: `GITHUB_JOB` and `GITHUB_RUN_ID` have
# positive evidence (the probe's marker filename), this one does not yet, and refusing on an
# unestablished variable would redden a REQUIRED check. Promote it once a run has printed
# `attempt=<n> (from the runner)`.
# Kept after the promotion, though all three components are now required and the line can no
# longer report anything but the runner's own values. It is the standing evidence: this is the
# line that settled whether GITHUB_RUN_ATTEMPT is exported, and it is what a future reader checks
# first if the keying is ever doubted again.
echo "Marker identity: job=${GITHUB_JOB} run=${GITHUB_RUN_ID} attempt=${attempt_used} (from the runner)"
echo "All ${#expected[@]} expected step(s) executed: ${expected[*]}"
}
[ "$#" -ge 1 ] || usage
sub="$1"
shift
case "$sub" in
mark) cmd_mark "$@" ;;
assert) cmd_assert "$@" ;;
*) usage ;;
esac