timothyandClaude Opus 5
d4c600149d
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 16s
PR Gates / Docs update reminder (pull_request) Successful in 19s
PR Gates / decisions lifecycle (pull_request) Successful in 22s
review-verdict/h10 Awaiting review verdict for d4c6001
Review verdict / Set review-verdict status (pull_request_target) Successful in 13s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m23s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 22s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m38s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m30s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 18m18s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Round-3 cross-family review returned BLOCKED with 3 Mediums. The first was serious
and self-inflicted.
THE PROTECTED GUARD WAS A NO-OP. Round 3's `count_matching` / `count_not_matching`
helpers were defined AFTER the classification chain that calls them, so
`count_matching` was `command not found` on every run, `$( )` yielded an empty string,
`[ "" -gt 0 ]` errored, and the `elif` was simply skipped — the protected-path check
never executed at all. Confirmed by direct execution before fixing.
Three "protected path" tests stayed GREEN throughout, because a protected path is also
not a manifest and not docs-only, so the job still reached `pending` down a different
route. Asserting the STATE could not distinguish a working guard from a dead one. The
mutation battery missed it too: I had mutated the predicates, not their reachability.
Fixed three ways:
* helpers are defined immediately after `gh()`, before any use;
* the three counts are evaluated ONCE at TOP LEVEL and validated numeric, because
`exit 1` inside `$( )` leaves only the subshell and, with the substitution sitting
in a conditional, `set -e` never fires either — so a grep error had been silently
reading as "no match". A non-numeric result now aborts with nothing posted, and an
absent required check blocks the merge;
* the helpers return a non-numeric sentinel instead of trying to `exit`.
Verified: an invalid regex now exits 2 and posts NOTHING (previously it classified and
posted). Renaming the helper at its definition turns six tests red.
TESTS, aimed at the failure mode rather than the symptom:
* assert the DISCRIMINATOR (the job's `Decision:` reason line), not the outcome —
when several branches yield the same verdict, the verdict cannot tell you which ran.
A first draft of this test asserted the status description and failed against a
WORKING guard, because for `pending` the description is constant;
* a cheap stderr sweep for `command not found` / `integer expression expected` /
`unbound variable` across four representative PR shapes. Each of those makes an `if`
condition merely false while the job exits 0 and posts a plausible status, so this
catches a whole family of silently-skipped guards.
DOCS. The record and ci-cd.md still PRESCRIBED the here-string that round 3 removed —
following them would have reintroduced the temp-storage failure. Both now prescribe
counting, define-before-use, top-level evaluation and numeric validation. The workflow's
measurement paragraph still said the npm manifests "are included" three lines above the
note saying they are excluded; corrected.
379 tests pass.
Refs: #698
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>