fix(756): promote GITHUB_RUN_ATTEMPT to required — its presence is now MEASURED

PR #768's own run answered the question the identity line was added to answer:

  Marker identity: job=test       run=1916 attempt=1 (from the runner)
  Marker identity: job=migrations run=1916 attempt=1 (from the runner)

Both required jobs, on the build lane, printed to stdout where capture is not
in question. So the variable IS exported here, the warn-and-default compromise
has served its purpose, and all three identity components now REFUSE rather
than falling back to a name other runs share. The residual that compromise
left — a rerun inheriting attempt 1's markers — no longer exists.

Recording HOW this was settled, because the first two attempts were worthless
and the next reader will be tempted by both. Grepping a job log for the
variable NAME proves nothing: logs do not dump the environment. Inferring it
from the ABSENCE of the script's "not set" warning proves nothing either — that
warning goes to stderr, and whether step stderr reaches a job log here was
itself never established (the control offered for it turned out to be an
::error:: this script writes to STDOUT). Only making the script report its own
resolved identity on stdout, and then reading it, settled anything.

The docs said "not established either way", which this run falsifies — so the
prose had to change regardless; promoting was the same edit surface.

The identity line is kept, now as standing evidence rather than an open
question, and is still asserted so it cannot rot into a line nobody can trust.

Decisions-Edit: yes
Refs: #756

docs/ci-cd.md

@@ -699,15 +699,20 @@ which runs *without* a `container:`. These two jobs run **inside** the CI toolch
 rules out a stale marker here; the keying is defence in depth against a lane change nobody would
 think to re-check this against. `GITHUB_JOB` and `GITHUB_RUN_ID` are *measured* present and the
 script refuses without them rather than falling back to a name other runs share.
-`GITHUB_RUN_ATTEMPT` is **not** established either way, so it warns and defaults instead: these are
-required checks, and reddening every merge over an unverified variable is a worse trade than a
-residual needing an unset variable *and* a non-fresh container to bite. Cold review argued it *is*
-set, reasoning that the script's "not set" warning appears nowhere in the probe logs — but that needs
-step **stderr** to reach the job log, and the control offered for it (an `::error::` line in the log)
-turned out to be one this script writes to *stdout*. Rather than settle it by argument, the guard now
-prints `Marker identity: job=… run=… attempt=… (from the runner | DEFAULTED …)` on stdout every run.
-Read that line on this change's own CI run; if it says "from the runner", promote the variable to the
-refuse bucket alongside the other two.
+`GITHUB_RUN_ATTEMPT` is required too — but **how** that was established is the part worth keeping,
+because the first two attempts at it were both worthless. Grepping a job log for the variable *name*
+proves nothing: logs do not dump the environment. Inferring it from the *absence* of the 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 — the control offered for that turned out
+to be an `::error::` line this script writes to *stdout*. So the script was made to **report its
+resolved identity on stdout**, where capture is not in question, and the answer was simply read off
+this change's own run: `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, and it is why the residual
+this paragraph used to describe — a rerun inheriting attempt 1's markers — no longer exists. The
+identity line stays, as the standing evidence a future reader checks first if the keying is ever
+doubted again.
 
 **The premise was re-measured on the build lane.** The whole thing rests on the runner still executing
 a later step after dropping an earlier one. #751 established that on the `small` lane; these jobs run

docs/decisions/records/ci/required-job-step-execution-markers.md

@@ -7,7 +7,7 @@ supersedes: none
 superseded-by: none
 rule: 'A step the runner declines to interpolate is DROPPED and the job still concludes `success` (`ci.workflow-run-body-no-expressions`). In `review-verdict.yml` that is fail-CLOSED — the required status is absent and the merge is blocked. In `docker-build.yml`''s `test` and `migrations` it is fail-OPEN: those are the other two required contexts on `main`, so the check reports green having done no work. So in those two jobs every `run:` step that is not `continue-on-error: true` calls `"$GITHUB_WORKSPACE/scripts/ci-step-ran.sh" mark <key>` as its FIRST act, and the job''s LAST step calls `ci-step-ran.sh assert --always <keys> --gated <keys>`, which fails the job when an expected key was never recorded. PER STEP, not per job: a marker written by the first step only proves the job started, while the drop that costs something is `Test` or the migration replay. The guard carries NO `if:` — the default `success()` is the wanted condition, because a genuine failure in an early step legitimately skips every later one and an `always()` guard would announce a false "these steps never executed" on every ordinary red build; the invariant that makes the omission safe is that the guard is skipped only when an earlier step FAILED, which already fails the job, so guard-skipped implies job-red and every path to a green job runs the guard. Separately and independently, no `${{` OPENER may appear in any `run:` body of those two jobs OR of `build` — the drop mechanism requires the opener, so banning it makes the class unreachable rather than merely caught, and an UNCLOSED opener triggers the same rewrite as a well-formed pair. Pass values in through the step''s `env:`, which is interpolated per value. The two halves have DIFFERENT scopes on purpose: markers cover the required pair, while the ban also covers `build`, whose `Smoke + IPTV E2E` step runs AFTER the image is pushed, so a drop there publishes a release candidate that was never booted and that `DeployStack jazz-media` then promotes. `functional-e2e` is delimiter-free but deliberately excluded (advisory by declaration), and `api-docs`/`format` keep one `github.base_ref` each and gate nothing that ships.'
 signals: 'required check green but no work done, step never ran but job green, Build & test green in seconds, EF migration integrity green without replaying, missing Run Main step marker, Unable to interpolate expression format(, dropped step docker-build, ci-step-ran.sh, marker file, expression delimiter in a required job · paths: `.gitea/workflows/docker-build.yml`, `scripts/ci-step-ran.sh`, `scripts/tests/test_ci_dropped_step_guard.py` · issues: #756, #751, #684'
-mechanics: '`scripts/ci-step-ran.sh` owns the marker path so it exists ONCE and the write and the read cannot diverge. It is keyed on `GITHUB_JOB`/`GITHUB_RUN_ID` — REQUIRED, refusing rather than falling back to a reusable name — plus `GITHUB_RUN_ATTEMPT`, which is NOT established either way on this runner and therefore WARNS and defaults to 1 rather than reddening a required check over an assumption; `assert` prints `Marker identity: … attempt=… (from the runner | DEFAULTED …)` on stdout every run so the next reader settles it by reading rather than inferring. Do NOT justify the keying with #751''s "RUNNER_TEMP is /tmp, not a private per-job dir": that was measured on a job with no `container:` and does not transfer — these jobs get a fresh container, which is the primary protection, and the keying is defence in depth. Held by `scripts/tests/test_ci_dropped_step_guard.py`: static (marker set derived from the workflow equals the guard''s expectations, bucket matches each step''s `if:`, guard is last / has no `if:` / is not advisory / has no delimiter) and behavioural (the guard''s real command line executed against markers written by the steps'' real marker lines, dropping each key in turn).'
+mechanics: '`scripts/ci-step-ran.sh` owns the marker path so it exists ONCE and the write and the read cannot diverge. It is keyed on `GITHUB_JOB`/`GITHUB_RUN_ID` — REQUIRED, refusing rather than falling back to a reusable name — plus `GITHUB_RUN_ATTEMPT`. All three REFUSE rather than falling back to a reusable name. The third was warn-and-default until its presence was measured: grepping a log for the variable NAME proves nothing, and inferring it from the absence of a stderr warning proves nothing either (stderr capture was itself unestablished), so `assert` was made to print `Marker identity: job=… run=… attempt=… (from the runner)` on STDOUT and the answer was read off run 1916 for both required jobs. That line is retained as standing evidence. Do NOT justify the keying with #751''s "RUNNER_TEMP is /tmp, not a private per-job dir": that was measured on a job with no `container:` and does not transfer — these jobs get a fresh container, which is the primary protection, and the keying is defence in depth. Held by `scripts/tests/test_ci_dropped_step_guard.py`: static (marker set derived from the workflow equals the guard''s expectations, bucket matches each step''s `if:`, guard is last / has no `if:` / is not advisory / has no delimiter) and behavioural (the guard''s real command line executed against markers written by the steps'' real marker lines, dropping each key in turn).'
 ---
 
 **Why per step, when #756 proposed per job.** A job-start marker answers "did this job begin", which

scripts/ci-step-ran.sh

@@ -86,36 +86,37 @@ EOF
 #   * `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 NOT measured — it appears nowhere in that 2.9 MB job log, so its
-#     presence is an assumption either way. Requiring it would risk reddening every job over an
-#     unverified variable. It therefore WARNS and defaults, which is loud rather than silent.
+#   * `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.
 #
-# The residual after this needs BOTH conditions at once: `GITHUB_RUN_ATTEMPT` unset AND a lane that
-# stopped giving each job a fresh container (see above) — a rerun could then inherit attempt 1's
-# markers. Stated rather than papered over; if the warning is ever seen in a real run, promote it.
+# 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. 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.
 marker_path() {
-    local missing="" attempt="${GITHUB_RUN_ATTEMPT:-}"
+    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
-    if [ -z "$attempt" ]; then
-        # STDERR ONLY, AND THAT IS NOT STYLE. This function's STDOUT *is* its return value — it is
-        # always called inside `$( )` — so anything printed here is captured INTO THE PATH. An
-        # earlier version of this block echoed the notice to stdout and produced a marker path with
-        # the notice embedded in it; both sub-commands then failed on a nonexistent directory.
-        # Caught by test_a_missing_RUN_ATTEMPT_*, which is why that test asserts a clean exit rather
-        # than only the presence of the warning.
-        #
-        # The stdout half of this reporting lives in `cmd_assert`, which is free to print.
-        echo "::warning::GITHUB_RUN_ATTEMPT is not set, so this run's markers share a path with its other attempts. Harmless while each job gets a fresh container; see scripts/ci-step-ran.sh (ersatztv#756)." >&2
-        attempt=1
-    fi
     printf '%s/etv-ci-steps-ran-%s-%s-%s' \
         "${RUNNER_TEMP:-${GITHUB_WORKSPACE:-/tmp}}" \
-        "$GITHUB_JOB" "$GITHUB_RUN_ID" "$attempt"
+        "$GITHUB_JOB" "$GITHUB_RUN_ID" "$GITHUB_RUN_ATTEMPT"
 }
 
 cmd_mark() {
@@ -174,15 +175,10 @@ cmd_assert() {
     # 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 re-deriving it. `marker_path` runs in a
-    # subshell, so anything it sets cannot reach here; re-deriving would be a second copy of the
-    # defaulting rule, free to drift from the one that actually built the path.
+    # 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##*-}"
-    # The provenance is computed INLINE below rather than held in a variable. An earlier version set
-    # an `ETV_ATTEMPT_DEFAULTED` global with no `else` branch, so the same name inherited from the
-    # environment made the line report `attempt=5 (DEFAULTED — GITHUB_RUN_ATTEMPT unset)` — self
-    # contradictory. `ETV_*` is a namespace this workflow already writes into via the guard's `env:`,
-    # so that was reachable, not hypothetical.
     # `${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
@@ -233,9 +229,11 @@ cmd_assert() {
     # 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)`.
-    local provenance="from the runner"
-    if [ -z "${GITHUB_RUN_ATTEMPT:-}" ]; then provenance="DEFAULTED — GITHUB_RUN_ATTEMPT unset"; fi
-    echo "Marker identity: job=${GITHUB_JOB} run=${GITHUB_RUN_ID} attempt=${attempt_used} (${provenance})"
+    # 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[*]}"
 }
 

scripts/tests/test_ci_dropped_step_guard.py

@@ -457,7 +457,7 @@ def test_the_guard_PASSES_when_every_step_marked_itself(job, gate, tmp_path):
     # must report the REAL value and say so. A mis-derivation (`${marker#*-}` rather than `##`) or an
     # inverted provenance test would otherwise ship silently, and the operator reading this line to
     # settle the promotion question would read it wrong.
-    assert "attempt=7 (from the runner)" in r.stdout, (
+    assert f"Marker identity: job={job} run=424242 attempt=7 (from the runner)" in r.stdout, (
         f"the guard misreported its marker identity: {r.stdout!r}")
 
 
@@ -628,7 +628,7 @@ def test_a_degraded_run_IDENTITY_refuses_rather_than_sharing_a_marker_path(tmp_p
     shared path and leave the two disagreeing about where the file is.
     """
     env = _env(tmp_path)
-    for var in ("GITHUB_RUN_ID", "GITHUB_JOB"):
+    for var in ("GITHUB_RUN_ID", "GITHUB_JOB", "GITHUB_RUN_ATTEMPT"):
         degraded = {k: v for k, v in env.items() if k != var}
         for argv in (f"{SCRIPT} mark alpha", f"{SCRIPT} assert --always alpha"):
             r = _run(argv, degraded)
@@ -643,38 +643,30 @@ def test_a_degraded_run_IDENTITY_refuses_rather_than_sharing_a_marker_path(tmp_p
         "a degraded-identity `mark` still created a marker file somewhere under RUNNER_TEMP")
 
 
-def test_a_missing_RUN_ATTEMPT_warns_and_continues_rather_than_deadlocking(tmp_path):
-    """The deliberate asymmetry, and the reason it is not simply "require all three".
+def test_the_marker_identity_is_REPORTED_on_stdout_every_run(tmp_path):
+    """The line that settled `GITHUB_RUN_ATTEMPT`, kept as standing evidence.
 
-    `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`). `GITHUB_RUN_ATTEMPT` is not established either way:
-    cold review argued its absence from the probe logs proves it IS set, but that inference needs
-    step stderr to reach the job log and the control offered for it was an `::error::` this script
-    writes to STDOUT. These jobs write REQUIRED contexts, so refusing on an unverified variable would
-    risk reddening every merge in the repo to close a residual needing a second condition (a lane
-    that stopped giving each job a fresh container) to bite at all.
+    Worth recording HOW that was settled, because the first two attempts 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 a "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 — the control
+    offered for that was an `::error::` this script writes to STDOUT. So the script was made to
+    REPORT its resolved identity on stdout, where capture is not in question, and the answer was read
+    off ersatztv#756's own PR run: `Marker identity: job=test run=1916 attempt=1 (from the runner)`,
+    and the same for `migrations`. That is what promoted the variable from warn-and-default to
+    required.
 
-    This test also pins the bug that fix introduced: `marker_path`'s STDOUT *is* the path, so a
-    notice printed there lands INSIDE it. Asserting a clean exit — not merely that the warning
-    appeared — is what catches that.
+    Asserted because cold review demonstrated three mutations of this reporting — deleting the echo,
+    mis-deriving the attempt, inverting the provenance — all surviving a 50-green suite. It is a
+    documented contract (the record's `mechanics:`), and a future reader is told to trust it.
     """
-    env = {k: v for k, v in _env(tmp_path).items() if k != "GITHUB_RUN_ATTEMPT"}
-    r = _run(f"{SCRIPT} mark alpha && {SCRIPT} assert --always alpha", env)
-    assert r.returncode == 0, (
-        "a missing GITHUB_RUN_ATTEMPT reddened the job. That variable is unmeasured on this runner, "
-        f"so this would deadlock every merge over an assumption.\n{r.stdout}{r.stderr}")
-    assert "::warning::" in (r.stdout + r.stderr) and "GITHUB_RUN_ATTEMPT" in (r.stdout + r.stderr), (
-        f"it continued SILENTLY, which is the shape being avoided: {r.stdout!r} {r.stderr!r}")
-    # THE IDENTITY LINE ON STDOUT, which is the whole point of the warn-and-default compromise: the
-    # open question ("does this runner export GITHUB_RUN_ATTEMPT?") is answered by READING a run
-    # rather than by inference, and the decision to promote the variable to the refuse bucket hangs
-    # on it. Asserted because cold review showed three mutations of this reporting — deleting the
-    # echo, mis-deriving the attempt, inverting the provenance — all survived a 50-green suite. The
-    # `::warning::` goes to stderr, whose capture in a job log is exactly what is NOT established;
-    # stdout is not in question, so the stdout half is the load-bearing one.
-    assert "attempt=1 (DEFAULTED" in r.stdout, (
-        "the guard did not report a DEFAULTED attempt on stdout, so a reader cannot tell from a run "
-        f"log whether the marker path was keyed on a real attempt: {r.stdout!r}")
+    marks = [_mark_line(s) for s, _ in _marked("test")]
+    r = _run("\n".join(["set -e", *marks, _guard("test")["run"]]),
+             _env(tmp_path, GITHUB_RUN_ID="1916", GITHUB_RUN_ATTEMPT="4"))
+    assert r.returncode == 0, r.stdout + r.stderr
+    assert "Marker identity: job=test run=1916 attempt=4 (from the runner)" in r.stdout, (
+        "the guard did not report the identity its marker path was actually keyed on, so a reader "
+        f"cannot audit the keying from a run log: {r.stdout!r}")
 
 
 def test_a_skip_gate_that_empties_the_expected_set_REFUSES(tmp_path):