Files
ersatztv/scripts/tests
timothyandClaude Opus 5 2bdb6c44e4 fix(751): a stray expression delimiter in a COMMENT killed the verdict gate
`review-verdict.yml`'s classify step stopped executing on 2026-08-03 and the job
reported `success` anyway, so `review-verdict/h10` — the branch-protection-required
status — was posted by nothing but a human hand for three days, and both exemption
classes (Renovate-manifest, docs-only) silently stopped working.

The cause is one token in prose. The #706 note explaining why a concurrency group
does not work here quoted a `concurrency:` snippet containing a PR-number expression
as an ILLUSTRATION, inside a shell comment. A shell comment is not inert there: the
runner scans the whole `run:` scalar for the expression opener before bash sees it,
and one occurrence makes it rewrite the ENTIRE body into a single `format(...)` call.
That rewrite is all-or-nothing, so a payload that does not parse — `pr number` does
not — fails the interpolation of the whole scalar, and the runner then DROPS THE STEP
AND CONCLUDES THE JOB GREEN. The prose documenting a fix disabled the fix.

`git blame`/`git log -S` put the line in 8f6d4f443 (2026-08-03), which dates the
outage precisely rather than "present in run 1832, not bisected further back".

Three changes, deliberately different in kind:

* The prose no longer writes the delimiters. It names the expression instead.
* The classifier writes a start marker and a new `if: always()` step FAILS THE JOB
  when it is missing. This is the half that generalises: the delimiter was one bug in
  one comment, but a dropped step concluding `success` is what made it cost three
  days behind a green tick. It asserts execution STARTED, never completed — the
  classifier has several legitimate `exit 0` abstention paths.
* Two static guards in scripts/tests/test_pr_changed_files.py: no expression
  delimiter in ANY `run:` body of the gate file (absolute, because a dropped step
  here is a dead merge gate and its bodies are ~700 lines of prose), and repo-wide,
  every expression payload must name a real context or function (permissive, because
  other workflows interpolate into `run:` legitimately). The second catches the class
  — a payload that cannot evaluate, wherever it appears.

Note every pre-existing workflow-shape test reads `_code_lines()`, which strips
comments. That is correct for what it was for, but it encodes the assumption this bug
falsifies: inside a `run:` scalar a comment CAN change behaviour. The new strict test
reads the raw scalar for that reason and must never adopt `_code_lines`.

Blast radius, audited: PR #739 (docs-only) merged 2026-08-05 with ZERO commit
statuses on its head, and got in only because admin force-merge was still enabled.
#743 disabled that on 2026-08-06, so the workaround that absorbed this bug is gone —
the next docs-only or Renovate-manifest PR would be permanently stuck. The two
Renovate PRs in the window escaped by timing, merging minutes before the bad commit.
Non-exempt PRs were unaffected throughout: humans posted their verdicts by hand.

Verification: all six mutations red, restored tree green — reintroducing the exact
defect (strict + general tests), deleting the guard step, deleting only the marker
write, weakening `if: always()`, turning the guard's `exit 1` into `exit 0`, and
putting a delimiter in the guard's own body. 129 passed on the fixed tree.

Still to establish, and NOT claimed here: that the runner executes a LATER step after
dropping an earlier one. The #751 evidence cannot say — the classifier was the job's
last step, so there was never a subsequent step to observe. A scratch-base probe with
a negative control answers it next; if the runner drops the rest of the steps too,
this guard is inert and the body has to move into `scripts/`.

Refs #751

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-10 19:22:50 +02:00
..