.gitea/workflows/review-verdict.yml inherits an existing review-verdict/h10=success only from a
status whose .creator.login is on its H10_REVIEWERS allow-list (#742). scripts/post-review-verdict.sh writes those verdicts with whatever account owns ETV_GITEA_TOKEN/ETV_GITEA_BASICAUTH and never asked whose it was. Two coupled values, nothing
asserting the coupling — and the failure was the silent kind: the status really is written, the tool
really does report success, and the next pull_request_target event re-derives it and posts over it.
And again. The PR deadlocks and the only trace is a ::warning:: in a run nobody reads.
What changed
The writer now measures its own effect. After posting the status it reads it back, identifies
its own write by state and description, and refuses — before the comment, so the surviving
half-state is the documented ask one — unless the recorded creator is on the allow-list. Measured
after the write rather than probed before it: a pre-flight GET /user would test what the credential claims to be, while this tests what Gitea recorded as the author of this status, which is the value
the gate actually reads. It also needs no scope beyond the repo access the POST already required, so
it cannot become a new way to block a narrowly-scoped but legitimate reviewer token.
The allow-list is derived, not restated. New scripts/lib/h10-reviewers.sh parses the single
anchored H10_REVIEWERS="..." assignment out of the gate's own definition. One declaration, not two
held together by a parity test — the shape testing.guard-derives-population-from-source records six
failed rounds against.
Why a parse and not a shared declaration both sides source, since that would obviously be better:
the gate runs against a checkout of the PR's base sha. A PR whose base predates a new scripts/lib/ file would not have it, and a missing source under set -euo pipefail kills the job
— which posts no review-verdict/h10 at all and blocks every merge to main, including the PR
that would repair it, with no admin override (#743). A fallback list for that case reintroduces the
second copy. So the derivation runs on the side that can fail safely and the gate's body is untouched.
Fail-closed on every branch: 0 or >1 anchored assignments, a non-plain-login entry, an empty list, or
an unreadable file all refuse before any network call. The membership query answers member /
not-a-member / cannot-tell as three distinct codes, so a caller cannot fold "cannot tell" into either.
The safety direction is unchanged and this PR does not claim otherwise. A non-allow-listed verdict
was always a stall, never a fail-open. What changes is where the signal lands: at the terminal that
made the mistake, instead of one PR event later.
Two defects I shipped and then caught by probing the live instance
Both were found by asking Gitea rather than re-reading my own code, and both are recorded in the
commits because the reason they survived is the reusable part.
.status, not .state. A row inside .statuses[] serialises its state under status; state is only the top-level aggregate. Reading .state per row yields "" for every
well-formed response, compares unequal to the state just posted, and would have refused every
verdict — a repo-wide deadlock, shipped green. The tests could not catch it because the curl
shim replayed the status POST payload as the read-back body, and a POST body legitimately uses state. The fixture agreed with the parser by construction. The shim now renames the field on
replay, and a separate test pins a verbatim live-Gitea body independently of the shim.
Measured both ways: with the fixture fixed and .state restored, 25 tests redden; before the
fixture fix, zero did.
The combined-status endpoint pages, undetectably.?limit=2 against a 15-context head
returned 2 rows andtotal_count: 2 — the count reports the page, not the total, so no field
distinguishes a truncated body from a complete one. The call now asks for ?limit=100, and the
not-found refusal names no cause (absence and truncation are not separable here) and reports
the row count it read, so a future overflow is legible.
What the review rounds found
Four independent cold reviewers, all worktree-isolated and briefed with no implementation context;
one of them cross-family (GPT-5.6 via Codex). Every finding below was reproduced before it was acted
on. Where two reviewers disagreed, no arbitration was needed — the MERGEABLE one had simply not
examined the blocking items, and both were verifiable.
Round 1 — the guard did not walk the same tree, and CLAUDE.md was wrong
The CI guard did not walk the same tree as its subject. test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty parsed the classify step's run: body;
the library greps the whole YAML and refuses on anything but exactly one match. A second
anchored H10_REVIEWERS= in any other run: block was therefore invisible to the guard while
making the writer refuse every verdict — a repo-wide merge outage on a PR-only main. Reproduced:
green guard, 31 red writer tests whose diagnostics named no second assignment. The guard now reads
the whole file.
CLAUDE.md said the status is not written. It is — only the comment is withheld. It now also
says what no doc did: the green left standing satisfies the server-side required check, because
branch protection binds the context name, not its issuer.
Four clauses shipped unwitnessed, found by a reviewer's disarm matrix: the .statuses
array-TYPE test, the read-back GET's explicit || die, ?limit=100, and the set -f restoration.
The first two passed only because a downstream check produced the same non-zero exit. They now
assert their own diagnostic, and the shim models the measured server so ?limit= is observable.
Plus: [0] over same-context rows had no uniqueness guard; the row count went blank on a
200-with-empty-body (jq exits 0 with empty output, so the || fallback was dead exactly where it
mattered); the split pinned globbing but not IFS; and the set -u rationale I inherited is wrong — both reviewers measured the shell exiting outright in both shapes.
Round 2 (cross-family) — two of my corrections had themselves gone wrong
The (.status // .state) fallback recreated #845. I added it as harmless defensiveness after
fixing the .state bug. The gate reads .status and nothing else, so on a response carrying only .state the fallback made the writer accept and report success while the gate saw no recognisable
verdict and re-derived it — the exact stall, reached through the check meant to prevent it.
The allow-list was applied to failure too, which the gate deliberately does not do. It
inherits a success only from a member but a failure from any attributable account. Refusing an
off-list BLOCKED refused a verdict the gate honours, stated a falsehood in the diagnostic, and —
because the comment is withheld on refusal — left a real reviewer no supported way to record a
rejection at all.
The paging model was backwards. Gitea selects each context's max row id and orders those
descending, so a just-posted row is always on page 1. My shim appended it last and sliced from the
front, manufacturing a truncation the server cannot produce — which made one test assert a fiction
and made the ?limit=100 "witness" circular: observable only because the fixture was wrong in
precisely the direction that made it observable.
Round 3 — a correction that overshot
"A repo-wide merge outage" overstated the guard's role. Such an edit landing would be one, but script-tests is advisory — measured, the required contexts are Build & test, EF migration integrity and review-verdict/h10 — so the writer suite already reddens the PR. What alignment buys
is a diagnostic naming the cause instead of 31 unrelated failures. Restated in all four places.
Round 4 — a declared security invariant with no guard
scripts/lib/h10-reviewers.sh states, and docs/remote-state-inventory.md records as a decision,
that no environment variable selects which workflow the allow-list comes from. The property held —
but nothing pinned it, and restoring the override the way a refactor naturally would left the whole
suite green while an env var installed an arbitrary allow-list and an off-list account was accepted
as a reviewer. Now tested, and confirmed to redden under that exact mutation.
The same round found a behavioural one: a BLOCKED verdict was refused when the allow-list could not
be derived. Membership never applies to a rejection, so a broken H10_REVIEWERS declaration was
blocking the one thing the success-only rule exists to protect — on the branch most likely to have
broken it. The load is now guarded by state; every reference to the library sits inside one of the
two state = success blocks, so the failure path never touches it.
And four prose claims falsifiable by grep or pytest, including one that re-taught its own lesson:
"31 red writer tests" had drifted to 40 inside this branch as tests were added, in all four places
the previous commit said it had restated. Replaced with the invariant.
Round 5 — my corrections were themselves falsifiable by grep
The sharpest round, and aimed at this branch's own thesis. Three of the four findings were claims a
previous commit message said it had fixed: "replaced in all four places" missed two of five
occurrences; "the WHOLE writer suite reddens" is measurably false (36 of 75, not all — I invoked state-the-invariant-not-the-measurement and then wrote a stronger claim than the count I removed);
a retraction left the retracted ground asserted seven lines below it; and "spans THREE top-level
directories" replaced a true hedge with a false count (the suite reads seven, including docs/, so
this PR's own doc edits are inputs to that job). All now stated without counts, or hedged to what is
measurable.
Round 6 (self-directed) — sweeping the retracted words, not the file
Chasing the retracted wording repo-wide surfaced the same shape once more, in the file the previous
round had just corrected. It also surfaced a real question: the decision record justifies the sentinel
by noting the read side deliberately runs without set -e, and every prior measurement had used set -euo pipefail. If the original claim held there, the correction would have been wrong. Measured
across bash 3.2.57 and 5.3.15 in all three modes — set -u alone exits 127, set -eu and set -euo pipefail exit 1, no branch taken in any of them. The correction stands, including for the
case the record relies on.
Rounds 7 and 8 — a correction that inverted a true statement, and its withdrawal
Round 7 caught two more prose defects: a "14 of 36" figure that was a grep LINE count (7 failures
printed twice by --tb=line), and an "instead of" that claimed alignment REPLACES the unrelated reds
when measurement shows it adds a named failure alongside them. Both fixed; all figures removed, since
a number that changes with pytest display flags is not a fact about the code.
Round 8 found the serious one. The set -u "correction" made in rounds 5–6 had inverted a true
statement into a false one, in live merge-gate code. My probe used a plain $UNSET; the validator
uses ${#arr[@]}, and those shapes differ:
The wording on main was right all along, for exactly the expansion the validator uses and exactly
the flags its consumer runs (check-review-verdict.sh is set -uo pipefail, deliberately without -e). The repo's own test_MUTATION_..._SENTINEL_still_refuses proves it — it asserts a message
only emitted after the gate, and it passes. The version I had shipped said "there is no skip to
defend against", which invites deleting the sentinel and reopening the fail-open that file exists to
close.
Withdrawn wholesale rather than patched a third time. review-verdict-vocabulary.sh and check-review-verdict.sh are byte-identical to main again; the doc paragraph and test comment are
restored verbatim. What survives is one correctly-scoped note in the new library. #886 is closed
as invalid — I had filed it asking for a record to be "corrected" that was right.
Guard status
scripts/post-review-verdict.sh moves BEHAVIOUR-ONLY → MUTATION, which the manifest's own UNDECLARED note called "the most valuable upgrade on this list". The declared clause is in the gate, not the script: rewriting H10_REVIEWERS while the posting account stays fixed reddens the
accept path only if the writer reads the list live and the comparison gates the outcome.
Disarming the comparison instead would prove the second and say nothing about the first, which is the
half that was missing.
Verification
Full scripts/tests suite green.
The declared mutation executes every run and reddens its named proof with the declared diagnostic.
Thirteen clauses disarmed one at a time, each confirmed to redden its own named test — and two of
my own disarms were inert (a character class that still excluded *; an IFS clause matching
two sites and applied to neither), which briefly read as "no test covers this". A mutation has to
be checked to actually mutate.
Live probes against Gitea 1.27.1 for the response shape, the description round-trip, and paging.
jq expressions probed against hostile bodies (non-object array elements, creator as a string, statuses null / non-array / empty): every one fails closed.
Docs
ci.exemption-provenance now records the coupling as asserted rather than as a tracked residual
(Decisions-Edit: yes), plus docs/ci-cd.md, CLAUDE.md, docs/guard-inventory.md and docs/remote-state-inventory.md.
Not in scope
The refused-verdict residual — a non-inheritable status left standing with no comment beside it — is
deliberately not repaired here. That is the ask half-state release.verdict-writes-status-before-comment already designates as safe, and a second corrective
write is the sticky-sentinel mechanism #849 is separately designing; inventing a parallel one on
this path would be two mechanisms for one invariant.
`.gitea/workflows/review-verdict.yml` inherits an existing `review-verdict/h10=success` only from a
status whose `.creator.login` is on its `H10_REVIEWERS` allow-list (#742).
`scripts/post-review-verdict.sh` writes those verdicts with whatever account owns
`ETV_GITEA_TOKEN`/`ETV_GITEA_BASICAUTH` and never asked whose it was. Two coupled values, nothing
asserting the coupling — and the failure was the silent kind: the status really is written, the tool
really does report success, and the next `pull_request_target` event re-derives it and posts over it.
And again. The PR deadlocks and the only trace is a `::warning::` in a run nobody reads.
## What changed
**The writer now measures its own effect.** After posting the status it reads it back, identifies
its own write by state and description, and refuses — **before the comment**, so the surviving
half-state is the documented `ask` one — unless the recorded creator is on the allow-list. Measured
after the write rather than probed before it: a pre-flight `GET /user` would test what the credential
*claims* to be, while this tests what Gitea recorded as the author of this status, which is the value
the gate actually reads. It also needs no scope beyond the repo access the POST already required, so
it cannot become a new way to block a narrowly-scoped but legitimate reviewer token.
**The allow-list is derived, not restated.** New `scripts/lib/h10-reviewers.sh` parses the single
anchored `H10_REVIEWERS="..."` assignment out of the gate's own definition. One declaration, not two
held together by a parity test — the shape `testing.guard-derives-population-from-source` records six
failed rounds against.
**Why a parse and not a shared declaration both sides source**, since that would obviously be better:
the gate runs against a checkout of the PR's **base** sha. A PR whose base predates a new
`scripts/lib/` file would not have it, and a missing `source` under `set -euo pipefail` kills the job
— which posts no `review-verdict/h10` at all and blocks **every** merge to `main`, including the PR
that would repair it, with no admin override (#743). A fallback list for that case reintroduces the
second copy. So the derivation runs on the side that can fail safely and the gate's body is untouched.
Fail-closed on every branch: 0 or >1 anchored assignments, a non-plain-login entry, an empty list, or
an unreadable file all refuse **before any network call**. The membership query answers member /
not-a-member / cannot-tell as three distinct codes, so a caller cannot fold "cannot tell" into either.
**The safety direction is unchanged and this PR does not claim otherwise.** A non-allow-listed verdict
was always a stall, never a fail-open. What changes is *where the signal lands*: at the terminal that
made the mistake, instead of one PR event later.
## Two defects I shipped and then caught by probing the live instance
Both were found by asking Gitea rather than re-reading my own code, and both are recorded in the
commits because the *reason* they survived is the reusable part.
1. **`.status`, not `.state`.** A row inside `.statuses[]` serialises its state under `status`;
`state` is only the top-level aggregate. Reading `.state` per row yields `""` for every
well-formed response, compares unequal to the state just posted, and would have refused **every**
verdict — a repo-wide deadlock, shipped green. The tests could not catch it because the `curl`
shim replayed the status **POST payload** as the read-back body, and a POST body legitimately uses
`state`. The fixture agreed with the parser *by construction*. The shim now renames the field on
replay, and a separate test pins a **verbatim live-Gitea body** independently of the shim.
Measured both ways: with the fixture fixed and `.state` restored, 25 tests redden; before the
fixture fix, zero did.
2. **The combined-status endpoint pages, undetectably.** `?limit=2` against a 15-context head
returned 2 rows *and* `total_count: 2` — the count reports the page, not the total, so no field
distinguishes a truncated body from a complete one. The call now asks for `?limit=100`, and the
not-found refusal names **no** cause (absence and truncation are not separable here) and reports
the row count it read, so a future overflow is legible.
## What the review rounds found
Four independent cold reviewers, all worktree-isolated and briefed with no implementation context;
one of them cross-family (GPT-5.6 via Codex). Every finding below was reproduced before it was acted
on. Where two reviewers disagreed, no arbitration was needed — the MERGEABLE one had simply not
examined the blocking items, and both were verifiable.
**Round 1 — the guard did not walk the same tree, and CLAUDE.md was wrong**
- **The CI guard did not walk the same tree as its subject.**
`test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty` parsed the classify step's `run:` body;
the library greps the **whole** YAML and refuses on anything but exactly one match. A second
anchored `H10_REVIEWERS=` in any other `run:` block was therefore invisible to the guard while
making the writer refuse every verdict — a repo-wide merge outage on a PR-only `main`. Reproduced:
green guard, 31 red writer tests whose diagnostics named no second assignment. The guard now reads
the whole file.
- **`CLAUDE.md` said the status is not written.** It is — only the comment is withheld. It now also
says what no doc did: the green left standing satisfies the *server-side* required check, because
branch protection binds the context **name**, not its issuer.
- **Four clauses shipped unwitnessed**, found by a reviewer's disarm matrix: the `.statuses`
array-TYPE test, the read-back GET's explicit `|| die`, `?limit=100`, and the `set -f` restoration.
The first two passed only because a downstream check produced the same non-zero exit. They now
assert their own diagnostic, and the shim models the measured server so `?limit=` is observable.
- Plus: `[0]` over same-context rows had no uniqueness guard; the row count went **blank** on a
200-with-empty-body (jq exits 0 with empty output, so the `||` fallback was dead exactly where it
mattered); the split pinned globbing but not `IFS`; and the `set -u` rationale I inherited is
**wrong** — both reviewers measured the shell exiting outright in both shapes.
**Round 2 (cross-family) — two of my corrections had themselves gone wrong**
- **The `(.status // .state)` fallback recreated #845.** I added it as harmless defensiveness after
fixing the `.state` bug. The gate reads `.status` and nothing else, so on a response carrying only
`.state` the fallback made the writer accept and report success while the gate saw no recognisable
verdict and re-derived it — the exact stall, reached through the check meant to prevent it.
- **The allow-list was applied to `failure` too, which the gate deliberately does not do.** It
inherits a `success` only from a member but a `failure` from any attributable account. Refusing an
off-list `BLOCKED` refused a verdict the gate honours, stated a falsehood in the diagnostic, and —
because the comment is withheld on refusal — left a real reviewer no supported way to record a
rejection at all.
- **The paging model was backwards.** Gitea selects each context's max row id and orders those
descending, so a just-posted row is always on page 1. My shim appended it last and sliced from the
front, manufacturing a truncation the server cannot produce — which made one test assert a fiction
and made the `?limit=100` "witness" circular: observable only because the fixture was wrong in
precisely the direction that made it observable.
**Round 3 — a correction that overshot**
"A repo-wide merge outage" overstated the guard's role. Such an edit landing would be one, but
`script-tests` is **advisory** — measured, the required contexts are `Build & test`, `EF migration
integrity` and `review-verdict/h10` — so the writer suite already reddens the PR. What alignment buys
is a diagnostic naming the cause instead of 31 unrelated failures. Restated in all four places.
**Round 4 — a declared security invariant with no guard**
`scripts/lib/h10-reviewers.sh` states, and `docs/remote-state-inventory.md` records as a decision,
that no environment variable selects which workflow the allow-list comes from. The property held —
but nothing pinned it, and restoring the override the way a refactor naturally would left the whole
suite green while an env var installed an arbitrary allow-list and an off-list account was accepted
as a reviewer. Now tested, and confirmed to redden under that exact mutation.
The same round found a behavioural one: a `BLOCKED` verdict was refused when the allow-list could not
be *derived*. Membership never applies to a rejection, so a broken `H10_REVIEWERS` declaration was
blocking the one thing the `success`-only rule exists to protect — on the branch most likely to have
broken it. The load is now guarded by state; every reference to the library sits inside one of the
two `state = success` blocks, so the `failure` path never touches it.
And four prose claims falsifiable by `grep` or `pytest`, including one that re-taught its own lesson:
"31 red writer tests" had drifted to 40 *inside this branch* as tests were added, in all four places
the previous commit said it had restated. Replaced with the invariant.
**Round 5 — my corrections were themselves falsifiable by grep**
The sharpest round, and aimed at this branch's own thesis. Three of the four findings were claims a
previous commit message said it had *fixed*: "replaced in all four places" missed two of five
occurrences; "the **WHOLE** writer suite reddens" is measurably false (36 of 75, not all — I invoked
`state-the-invariant-not-the-measurement` and then wrote a *stronger* claim than the count I removed);
a retraction left the retracted ground asserted seven lines below it; and "spans **THREE** top-level
directories" replaced a true hedge with a false count (the suite reads seven, including `docs/`, so
this PR's own doc edits are inputs to that job). All now stated without counts, or hedged to what is
measurable.
**Round 6 (self-directed) — sweeping the retracted words, not the file**
Chasing the retracted wording repo-wide surfaced the same shape once more, in the file the previous
round had just corrected. It also surfaced a real question: the decision record justifies the sentinel
by noting the read side deliberately runs **without `set -e`**, and every prior measurement had used
`set -euo pipefail`. If the original claim held there, the correction would have been wrong. Measured
across bash 3.2.57 and 5.3.15 in all three modes — `set -u` alone exits 127, `set -eu` and
`set -euo pipefail` exit 1, no branch taken in any of them. The correction stands, including for the
case the record relies on.
**Rounds 7 and 8 — a correction that inverted a true statement, and its withdrawal**
Round 7 caught two more prose defects: a "14 of 36" figure that was a grep LINE count (7 failures
printed twice by `--tb=line`), and an "instead of" that claimed alignment REPLACES the unrelated reds
when measurement shows it adds a named failure alongside them. Both fixed; all figures removed, since
a number that changes with pytest display flags is not a fact about the code.
Round 8 found the serious one. The `set -u` "correction" made in rounds 5–6 had **inverted a true
statement into a false one, in live merge-gate code**. My probe used a plain `$UNSET`; the validator
uses `${#arr[@]}`, and those shapes differ:
```
shape set -u set -eu
plain $UNSET rc=127 exits rc=1 exits
unset ${#UNSET_ARR[@]} rc=0 CONTINUES rc=1 exits
scalar ${#SCALAR[@]} rc=0 CONTINUES rc=1 exits
```
The wording on `main` was right all along, for exactly the expansion the validator uses and exactly
the flags its consumer runs (`check-review-verdict.sh` is `set -uo pipefail`, deliberately without
`-e`). The repo's own `test_MUTATION_..._SENTINEL_still_refuses` proves it — it asserts a message
only emitted *after* the gate, and it passes. The version I had shipped said "there is no skip to
defend against", which invites deleting the sentinel and reopening the fail-open that file exists to
close.
**Withdrawn wholesale** rather than patched a third time. `review-verdict-vocabulary.sh` and
`check-review-verdict.sh` are byte-identical to `main` again; the doc paragraph and test comment are
restored verbatim. What survives is one correctly-scoped note in the *new* library. **#886 is closed
as invalid** — I had filed it asking for a record to be "corrected" that was right.
## Guard status
`scripts/post-review-verdict.sh` moves **BEHAVIOUR-ONLY → MUTATION**, which the manifest's own
`UNDECLARED` note called "the most valuable upgrade on this list". The declared clause is in the
**gate**, not the script: rewriting `H10_REVIEWERS` while the posting account stays fixed reddens the
accept path only if the writer reads the list live **and** the comparison gates the outcome.
Disarming the comparison instead would prove the second and say nothing about the first, which is the
half that was missing.
## Verification
- Full `scripts/tests` suite green.
- The declared mutation executes every run and reddens its named proof with the declared diagnostic.
- Thirteen clauses disarmed one at a time, each confirmed to redden its own named test — and two of
my own disarms were **inert** (a character class that still excluded `*`; an IFS clause matching
two sites and applied to neither), which briefly read as "no test covers this". A mutation has to
be checked to actually mutate.
- Live probes against Gitea 1.27.1 for the response shape, the description round-trip, and paging.
- jq expressions probed against hostile bodies (non-object array elements, `creator` as a string,
`statuses` null / non-array / empty): every one fails closed.
## Docs
`ci.exemption-provenance` now records the coupling as **asserted** rather than as a tracked residual
(`Decisions-Edit: yes`), plus `docs/ci-cd.md`, `CLAUDE.md`, `docs/guard-inventory.md` and
`docs/remote-state-inventory.md`.
## Not in scope
The refused-verdict residual — a non-inheritable status left standing with no comment beside it — is
deliberately not repaired here. That is the `ask` half-state
`release.verdict-writes-status-before-comment` already designates as safe, and a second corrective
write is the sticky-sentinel mechanism **#849** is separately designing; inventing a parallel one on
this path would be two mechanisms for one invariant.
fixes #845
`.gitea/workflows/review-verdict.yml` inherits an existing `review-verdict/h10=success`
only from a status whose `.creator.login` is on its `H10_REVIEWERS` allow-list (#742).
`scripts/post-review-verdict.sh` writes those verdicts with whatever account owns
`ETV_GITEA_TOKEN`/`ETV_GITEA_BASICAUTH` and never asked whose it was. The two values were
coupled with nothing asserting the coupling, and the failure mode was the silent one: the
status really is written, the tool really does report success, and then the next
`pull_request_target` event re-derives it and posts over it. And again. The PR deadlocks
and the only diagnostic is a `::warning::` inside a workflow run nobody is reading.
The script now READS THE STATUS BACK after posting it, identifies its own write by state
and description, and refuses — before the comment, so the surviving half-state is the
documented `ask` one — unless the recorded creator is on the allow-list. Measured after
the write rather than probed before it: a pre-flight `GET /user` tests what the credential
claims to be, this tests what Gitea recorded as the author of this status, which is the
value the gate reads. It also needs no scope beyond the repo access the POST already
required.
The allow-list is not restated. `scripts/lib/h10-reviewers.sh` derives it from the
workflow's own literal, so there is ONE declaration rather than two held together by a
parity test — the shape `testing.guard-derives-population-from-source` records six failed
rounds against. It is a parse rather than a shared declaration both sides source, and that
is not a preference: the gate runs against a checkout of the PR's BASE sha, so a PR whose
base predates a new `scripts/lib/` file would not have it, and a missing `source` under
`set -euo pipefail` kills the job — which posts no `review-verdict/h10` at all and blocks
every merge to `main` including the PR that would repair it, with no admin override (#743).
A fallback list for that case reintroduces the second copy. So the derivation runs on the
side that can fail safely and the gate's body is untouched.
Fail-closed on every branch of the derivation: 0 or >1 anchored assignments, an entry that
is not a plain login, an empty list, or an unreadable file all refuse BEFORE any network
call. The membership query answers member / not-a-member / cannot-tell as three distinct
codes rather than two, so a caller cannot fold "cannot tell" into either.
The safety DIRECTION is unchanged and the commit does not claim otherwise: a
non-allow-listed verdict was always a stall, never a fail-open. What changes is where the
signal lands — at the terminal that made the mistake, instead of one PR event later.
Residual, stated rather than fixed: the check runs after the POST, so a refused verdict
leaves a non-inheritable status standing with no comment beside it. That is the `ask`
half-state `release.verdict-writes-status-before-comment` already designates as the safe
one, and the gate re-derives it on the next PR event. A second corrective write is the
sticky-sentinel mechanism #849 is separately designing; inventing a parallel one here would
be two mechanisms for one invariant.
`scripts/post-review-verdict.sh` moves BEHAVIOUR-ONLY -> MUTATION in the guard inventory,
which the manifest's own UNDECLARED note called "the most valuable upgrade on this list".
The declared clause is in the GATE, not the script: rewriting `H10_REVIEWERS` while the
posting account stays fixed reddens the accept path only if the writer reads the list live
AND the comparison gates the outcome. Disarming the comparison instead would prove the
second and say nothing about the first, which is the half that was missing.
Verification: full `scripts/tests` suite green (1239 passed, 2 skipped); the declared
mutation executes and reddens its named proof with the declared diagnostic; four clauses
disarmed by hand and each confirmed to redden its own test.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Self-review defect, caught by probing the live instance rather than re-reading the code.
A row inside `.statuses[]` serialises its state under the key `status`. `state` exists only
as the AGGREGATE at the top level of the combined body. Measured on Gitea 1.27.1
(2026-08-29) against a real `review-verdict/h10` row on PR #882's head. Reading `.state`
per row therefore yields "" for every well-formed response, which compares unequal to the
state just posted — so the previous commit would have refused EVERY verdict with
"something overwrote it". Since no PR can merge to `main` without a `review-verdict/h10`
success, that is a repo-wide deadlock, shipped green.
WHY THE TESTS DID NOT CATCH IT, which is the more useful half. The `curl` shim replays the
status POST payload as the read-back body. A POST body legitimately uses `state` — that is
the field the write API takes — so the fixture agreed with the parser BY CONSTRUCTION and
the pair was self-consistent and both wrong. A fixture derived from the code under test
cannot disagree with it. The shim now renames the field on replay, and a separate test
feeds a VERBATIM live-Gitea body so the real shape is pinned independently of the shim.
Measured, both directions: with the fixture fixed and `.state` restored, 25 tests redden;
before the fixture fix, zero did.
The gate reads the same endpoint and the same key (`review-verdict.yml` line 520,
`ex_state=$(... jq -r '.status // ""')`), so the writer and the gate are now wrong or right
together rather than independently. `.state` is kept as a fallback so a server spelling it
the other way degrades to working rather than to a deadlock; it is not a second source of
truth, since the value still has to equal the state this run posted.
A row carrying NEITHER key now gets its own message. It is a response-SHAPE problem, not a
race, and "something overwrote it" would state a cause that did not happen — the #859
defect class.
fixes#845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Second self-review defect, again found by probing rather than reading.
`GET /commits/{sha}/status` PAGES, and it pages undetectably: measured on Gitea 1.27.1
(2026-08-29), `?limit=2` against a head carrying 15 contexts returned 2 rows AND
`total_count: 2`. The count reports the PAGE, not the total, so no field in the body
distinguishes a truncated response from a complete one. A head whose context count grew
past the server's default page size would silently stop containing the `review-verdict/h10`
row, and the read-back would refuse a verdict that is actually in force — the deadlock
direction, on the one path that must not deadlock.
The call now asks for `?limit=100` against the ~15 contexts this repo produces.
The not-found refusal no longer states a cause. Two causes reach it — the status is gone,
or the list was truncated — and the response cannot tell them apart, so asserting either
would be the #859 defect: a message that sends the reader hunting something that did not
happen. It now reports how many rows it actually read and names neither, which also makes a
future paging overflow legible instead of mysterious.
Recorded in `docs/remote-state-inventory.md` on the writer's own row, since it is a property
of that read rather than of the tool.
fixes#845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Probed, not reasoned: a `creator` arriving as a STRING rather than an object makes jq exit 5
("Cannot index string with string"). That already failed closed — the abort lands before the
comment — but with only jq's stderr, which never says a status was left standing on the head.
Each extraction now carries its own message.
The no-creator refusal said the status "is how a status posted by an ACTIONS token appears".
Three shapes reach that branch (`creator` absent, null, or an empty login) and only one of
them is an Actions token, so the message now states the fact first and offers the cause as
the likely one rather than the established one.
fixes#845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
The comment stated that `/commits/{sha}/status` returns the latest row per context as a bare
fact. It was measured — a head carrying two `review-verdict/h10` rows in the `/statuses` list
returned only the newer one from the combined endpoint — so the comment now says so. An
unattributed claim about an API is the kind that quietly stops being true and talks the next
reader out of re-checking it.
refs #845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found by disarming the LIBRARY's clauses rather than the script's. The end-to-end tests only
ever drive the real workflow, which is well-formed, so four clauses could be removed with
the suite still green:
- the plain-login validation that rejects a glob entry
- the sentinel gate that makes `contains` answer CANNOT-TELL instead of NOT-A-MEMBER
- the empty-list check
- the restoration of the caller's `-f` setting after the split
The first is the one that matters. `for x in $list` performs PATHNAME EXPANSION, so a `*`
entry expands to the FILENAMES around it, and any of those matching the plain-login class
would validate cleanly and enter the allow-list as a real reviewer.
`review-verdict-vocabulary.sh` records exactly this happening to its own word list; the new
test stocks the working directory with plausible login names so an expanding implementation
loads a list instead of refusing.
The sentinel one is a wrong-cause defect rather than a fail-open: without it a broken
checkout answers NOT-A-MEMBER, and the script then tells the operator to fix a credential
that is fine.
A note on the measurement, because the first attempt was wrong. The initial disarm of the
plain-login class widened it to `[!A-Za-z0-9._!@#$%^-]`, which still excludes `*` — so it
mutated nothing and the "no test covers this" reading was an artifact of an inert mutation.
The real disarm has to ADMIT `*` (`[!A-Za-z0-9._*-]`), and the probe now asserts that the
mutant class accepts `*` before drawing any conclusion from the run.
All four confirmed to redden their own named test.
fixes#845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Two cold reviewers, independently briefed and worktree-isolated. One returned BLOCKED on the
first two below; the other returned MERGEABLE having not examined them. No arbitration was
needed — both are verifiable, and I reproduced each before acting.
BLOCKING 1 — the CI guard parsed a different population than the library.
`test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty` read `_classify_step()["run"]`, one
step's body. `scripts/lib/h10-reviewers.sh` greps the WHOLE YAML and refuses on anything but
exactly one match. So a second anchored `H10_REVIEWERS=` in ANY other `run:` block was
invisible to the guard while making the writer refuse every verdict — and `main` is PR-only
with `review-verdict/h10` required, so that is a repo-wide merge outage. Reproduced: injecting
one into the last step left the guard green and reddened 31 writer tests whose diagnostics
said nothing about a second assignment. The guard now reads the whole file, so it walks the
same tree as its subject; the header and the decision record no longer claim coverage that
did not exist.
BLOCKING 2 — `CLAUDE.md` said an off-list verdict "fails loudly at your terminal instead of
being written". The status IS written; only the comment is withheld. The script, `ci-cd.md`
and the record all said so correctly, and the most-read doc contradicted them. It now also
says what the other docs did not: the green left standing satisfies the server-side required
check, because branch protection binds the context NAME and not its issuer.
Four clauses were shipped UNWITNESSED, found by the reviewer's own disarm matrix and each now
reddening a named test: the `.statuses` array-TYPE test, the read-back GET's explicit `|| die`,
`?limit=100`, and the `set -f` restoration. The first two passed only because a downstream
check produced the same non-zero exit — the shape this very file already documents at the
`-z`/`-n` arms. They now assert their own diagnostic. `?limit=100` was unobservable because
the shim discarded the query string; the shim now models the measured server (default 30,
clamped to 50, `total_count` reporting the page).
Also fixed, all reproduced first:
- `[0]` over same-context rows had no uniqueness guard. Unreachable today, but the
alternative is judging one row's author while another is the one standing.
- The row count in the not-found refusal came out BLANK on a 200 with an empty body: jq
exits 0 with empty output, so the `||` fallback was dead exactly where the count matters.
Tests the value now, not jq's exit.
- The split pinned globbing but not IFS. A caller with `IFS=-` made `renovate` read as a
member of `renovate-bot`. Unreachable from this script (bash resets IFS), pinned because
the header invites reuse.
- The `set -u` rationale was inherited and is WRONG. Both reviewers measured it: the shell
EXITS, in both the `if ! f` and the `f; rc=$?` shapes, on bash 5.3 and 3.2. Nothing here
depended on it; the sentinel now rests on the two grounds that were measured.
- The `?limit=100` margin was overstated. This instance clamps to MAX_RESPONSE_ITEMS = 50,
so the headroom is ~3x over today's 8-15 contexts, not 6x, and it shrinks as checks are added.
- The snapshot boundary now names its DIRECTION in all three places: it is a false ACCEPT,
and the likely edit (ADDING a reviewer) is exactly the one it mis-handles.
- The overwrite refusal told the operator to re-review when a gate re-derive needs no review.
A note on the measurements, since two of my own disarms were inert and briefly read as
"no test covers this": a mutation must be checked to actually mutate. The glob-class disarm
still excluded `*`, and the IFS disarm matched two sites and applied to neither.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Caught while a cross-family review was probing the same spot, and confirmed before fixing:
`test_a_status_OVERWRITTEN_...` used a fixture differing in BOTH state and description, so
either arm of `[ state != ] || [ desc != ]` could be deleted with the suite still green. Each
arm was individually disarmed and the suite stayed at 69 passed both times — #685's
duplicate-guards-mask-each-other shape.
Each arm now has a fixture differing in exactly one field, and both were confirmed to redden
their own named test. The creator in each is deliberately ALLOW-LISTED, so the refusal has to
come from identifying the row as not-ours rather than from its author.
The script now also states WHICH arm carries the safety, matching how this file already
handles its overlapping `-z`/`-n` arms: the DESCRIPTION is the identifier (verdict word, short
sha, base — only this run writes it), and the STATE is defence in depth, since our own writer
cannot produce our description with a different state.
Also: apostrophes restored in four library header sentences that read as typos.
fixes#845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Two more independent cold reviews: GPT-5.6 via Codex (cross-family) and a worktree-isolated
Opus re-review of the previous fix commit. They converged on the paging defect from
different directions. Each finding below was reproduced before acting.
HIGH — the `(.status // .state)` fallback recreated the bug it was added to prevent. The gate
reads `.status` and nothing else. On a response carrying only `.state`, the fallback made THIS
tool accept and report success while the gate saw no recognisable verdict and re-derived it —
which is #845 exactly, reached through the check meant to stop it. My comment claimed it
"degrades to working"; it degrades to the bug. Removed: the writer now reads exactly what the
gate reads, and a shape neither understands is refused loudly.
MEDIUM — the allow-list was applied to `failure` too, which the gate deliberately does not do.
`review-verdict.yml` inherits a `success` only from a member but a `failure` from any
ATTRIBUTABLE account, and says so at length: inheriting a rejection can only withhold an
exemption, while re-deriving one can turn it green. So refusing an off-list `BLOCKED` refused
a verdict the gate honours, told the reviewer their rejection would be re-derived when it
would not, and — because the comment is withheld on refusal — left a real reviewer no
supported way to record a rejection at all. Membership is now required for `success` only.
MEDIUM — the paging model was backwards, and so was the reasoning built on it. Gitea selects
each context's MAX row id, orders those DESCENDING, and paginates that. Measured: a head with
ids [17,19,...,41,43] returns 41 and 43 at `?limit=2`, and on another head the h10 row appears
at `limit=9` but not `limit=8`, exactly its rank by recency. The status this script has just
POSTed is the newest on the head, so it sorts FIRST and page 1 always holds it. The shim
appended it LAST and sliced from the front, which manufactured a truncation the server cannot
produce — so `test_a_head_carrying_more_contexts_than_the_server_cap_refuses` asserted a
fiction, and the `?limit=100` "witness" was circular: observable only because the fixture was
wrong in precisely the direction that made it observable. The shim now orders newest-first,
that test is deleted, and `?limit=100` is kept as declared-unwitnessed insurance rather than
dressed up as a tested guard.
Also corrected, each verified first:
- The `readback_rows` fallback was unreachable and its test never reached the branch it
named. The fallback is gone; the test now asserts the message an empty body actually
produces. The blank-count bug was real and was fixed by the restructure, not by the
clause I credited.
- "A repo-wide merge outage" overstated the guard's role. Such an edit landing WOULD be one,
but `script-tests` is ADVISORY — measured, the required contexts are `Build & test`,
`EF migration integrity` and `review-verdict/h10` — so the writer suite already reddens the
PR. What alignment buys is a diagnostic naming the cause instead of 31 unrelated failures.
Restated in all four places, and connected to the snapshot boundary, which is the route by
which the outage is actually reachable.
- The IFS test's docstring blamed the wrong split. `load` stores the RAW string, so its pin
affects validation only; membership is the `contains` split, and only that one is witnessed.
- The two `|| die` guards on already-constrained jq output are declared unreachable rather
than counted as protection, and a multi-document body now refuses instead of printing
"0 0 separate rows".
- Why the writer may hardcode a limit where the gate deliberately refused to: the gate must
prove a verdict ABSENT, which no single page can establish; this only confirms a row it
just wrote is PRESENT. Presence needs no completeness argument.
CORRECTION to the previous commit message: it claimed four clauses were made witnessed,
including the `set -f` restoration. Measured on that predecessor, `set -f` restoration already
reddened its named test at both sites, and the one genuinely unwitnessed `set -f` — the
`contains`-side pathname guard — still is, correctly, since the validator excludes every
metacharacter before it runs. Three, not four.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Fourth cold review, worktree-isolated, on the rebased head. Every finding reproduced before
acting; the two behavioural ones were re-verified with the reviewer's own mutations.
MEDIUM — "no environment variable selects the source file" was declared as a deliberate
security decision, in the library header AND in `docs/remote-state-inventory.md`, with no test
anywhere. The property held, but restoring the override the way a refactor naturally would
(`ETV_H10_REVIEWERS_WORKFLOW="${ETV_H10_REVIEWERS_WORKFLOW:-}"`) left the whole suite green
while an env var installed an arbitrary allow-list and an off-list account was accepted as a
reviewer. That is the class this branch removed in `28aab4632`, in a clause labelled as a
decision rather than as untested. Now pinned, and confirmed to redden under that exact mutation.
LOW (behavioural) — a BLOCKED verdict was refused when the allow-list could not be DERIVED.
Membership never applies to a `failure`, but the derivation was loaded unconditionally, so a
workflow with a broken `H10_REVIEWERS` declaration refused rejections too — the exact outcome
the success-only rule exists to prevent ("no supported way to record a rejection"), reached one
condition earlier, on the branch most likely to have broken that declaration: the one editing
it. The load is now guarded by state; both of its stated properties (after argument validation,
before the first network call) survive. A test covers the rejection AND the approval on the
same tree, so the guard was narrowed rather than removed.
Prose, all four falsifiable by `grep` or `pytest`:
- The comment claimed `review-verdict.yml` "rejected a hardcoded limit for its own read". It
does not — it sends `?limit=100` on both reads. What it rejected was using that limit as a
COMPLETENESS argument, which was dead code because the instance caps at 50. The real point
(presence vs absence) was right and is kept; a reader who greps the workflow no longer finds
the opposite of what this says.
- "31 red writer tests" had drifted to 40 INSIDE this branch as tests were added, in all four
places the previous commit said it had restated. Replaced with the invariant — the whole
writer suite reddens — per `state-the-invariant-not-the-measurement`, which is exactly the
lesson the stale number re-taught.
- The sentinel-reset comment cited a measurement that does not isolate it: removing the reset
produces a byte-identical refusal, because `load` overwrites both variables on that path. The
reset's real ground is the caller that never reaches `load`, which is what `contains`
returning 2 rests on. Re-attributed.
- `docs/ci-cd.md` still mirrored a TWO-directory input set for `script-tests` after this same
PR widened the workflow's own comment to three. Synced.
Also: `scripts/lib/review-verdict-vocabulary.sh` still shipped the `set -u` claim this branch
demonstrated to be wrong, one file over, and it was the source the new library copied it from.
Corrected there too — measured on bash 3.2.57 and 5.3.15, the shell exits outright in both the
`if ! f` and `f; rc=$?` shapes; the old reading holds only inside a command substitution. Its
sentinel keeps the ground that survives.
Nits: a stale "~10s" runtime in a sentence this branch rewrote, replaced with the reason that
does not drift; an unresolvable breadcrumb citing a private memory filename rather than a
decision key, dropped; "so only this run writes it" narrowed to what the description actually
identifies; and the paging test's docstring now says it is a POSITIVE CONTROL rather than
claiming a property pin it cannot deliver.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Fifth cold review, on the previous fix commit. Every finding is prose, and three of them are
claims that commit's own message said it had fixed. On a branch whose stated thesis is "four
prose claims were falsifiable by grep", shipping four more is the finding. Each verified here
before acting.
- "replaced in all four places" was wrong: there were FIVE occurrences of `31`. Two survived,
one of them four lines below the sentence the same commit added saying the number drifts.
- "the WHOLE writer suite reddens" is measurably false. Injecting a second anchored
`H10_REVIEWERS=` gives 36 failed / 39 passed — a large part, not all. The commit invoked
`state-the-invariant-not-the-measurement` and then wrote a claim STRONGER than the count it
removed. Now stated without a count and without "whole".
- The sentinel re-attribution retracted the environment-defeating ground in one paragraph and
still asserted it seven lines below ("the two that were measured"). The sibling library
corrected in the same commit got this right ("the ground that survives", singular). This is
`stale-comments-sweep-by-subject`: after a retraction, grep the retracted words.
- "spans THREE top-level directories" replaced a TRUE hedge with a false count, and did not
match the workflow it was supposed to be synced with (which says "at least three"). The
suite reads seven — `.claude`, `.gitea`, `.husky`, `docs`, `ErsatzTV`, `scripts`, `web` —
so this PR's own doc edits are inputs to that job. Hedge restored, with the reason.
Also from that review: the `if [ "$state" = "success" ]` body was unindented at column 0, and
"Fails CLOSED on every branch" now means every branch OF THAT LOAD, since the load is itself
conditional. Both fixed; the indent change is whitespace-only (`git diff -w` empty).
The behaviour was attacked again and held. The reviewer traced a BLOCKED verdict end to end
with `scripts/lib/h10-reviewers.sh` DELETED — rc=0, status and comment both posted, no unbound
variable and no command-not-found, because `$state` is set by the `case` well before the guard
and every reference to the library sits inside one of the two `state = success` blocks. An
APPROVAL on the same tree still refuses before writing anything. Removing the guard reddens the
new rejection test; `if true` reddens 20 tests. The declared mutation still reddens its named
proof.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Found by sweeping the RETRACTED WORDS repo-wide rather than re-reading the file I had edited,
which is the `stale-comments-sweep-by-subject` lesson the previous round already charged me
with once.
`scripts/lib/review-verdict-vocabulary.sh` still said "A control-flow gate can be skipped by
an abort" seven lines under the paragraph retracting exactly that mechanism — the same
self-contradiction shape as the sentinel comment in the last round, in the file that round
corrected.
MEASURING IT PROPERLY CHANGED WHAT I CHECKED. The decision record justifies the concern by
noting the READ side deliberately runs without `set -e`, and both prior reviewers had measured
the claim only under `set -euo pipefail`. If the original reading held without `set -e`, the
correction shipped last round would itself have been wrong. Measured on bash 3.2.57 and 5.3.15
across all three modes:
[set -u] in f / DEFINITELY_UNSET: unbound variable / exit=127
[set -eu] in f / DEFINITELY_UNSET: unbound variable / exit=1
[set -euo pipefail] in f / DEFINITELY_UNSET: unbound variable / exit=1
No branch taken and nothing after the gate, in every mode — including the one the record
relies on. The correction stands. The old reading holds only inside a command substitution
(`v=$(f) || v=FALLBACK` continues, fallback taken), which is not the shape gating either file.
WHAT IS NOT IN QUESTION, stated because the retraction is easy to over-read: the fail-open
that record documents is REAL — with `.*` in the positive list and the negative list written as
a scalar, an explicit `BLOCKED @ <head>` classified `positive`, exit 0. Only the attributed
mechanism is wrong: `${#name[@]}` on a scalar yields 1 rather than erroring, so no abort
occurred in that scenario at all. The sentinel is still the right design, on the ground that
survives — the caller that never runs validation.
The same refuted mechanism remains in `release.verdict-vocabulary-shared` and in
`test_review_verdict_vocabulary.py`. Correcting a decision record is its own change with its
own review surface, so it is tracked as ersatztv#886 with the full measurement, and
cross-referenced from the corrected file rather than folded in here.
fixes#845
refs #886
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Sixth cold review. One HIGH, and it is the sentence the previous commit rewrote.
The claim, in five sites across three files, was that a second anchored `H10_REVIEWERS=`
reddened the writer suite "with no diagnostic naming a second assignment". Measured: 14 of the
36 failures carry this library's own refusal, verbatim —
E AssertionError: h10 reviewers: .../review-verdict.yml carries 2 anchored
H10_REVIEWERS assignments, not exactly 1 — refusing to guess which one the gate runs with
— which names both the count and the variable. The message predates the alignment commit that
introduced the claim, so it was false when written, not newly broken here. What is actually
missing is a failure that POINTS AT THE WORKFLOW: the reds surface under unrelated test names
(`test_positive_verdict_posts_success_on_the_resolved_head`, …) and the cause has to be
inferred from a stack of them. Aligning the populations buys ONE named guard failure instead.
All five sites now say that, and "about half the suite" replaces the earlier "the writer suite"
where it read as all of it (36 of 75).
Two nits from the same round: the aside inserted into `docs/ci-cd.md` turned an em-dash from an
introducer into a closing parenthetical and left the enumeration dangling — the hedge now
follows the sentence instead of splitting it; and "on that ground alone, which is the one that
was measured" implied a contrast that does not exist, since the retracted ground was measured
too. Trailing clause dropped.
CORRECTION to the previous commit message, which no artifact carries but which was wrong:
it said "Removing the guard reddens the new rejection test; `if true` reddens 20 tests",
pairing one mutation with two counts. Measured on the load guard: `if true` reddens 1
(the rejection test), `if false` reddens 20.
The behaviour was attacked again and held: BLOCKED end to end with the library DELETED gives
rc=0 with status and comment posted and an empty stderr, MERGEABLE on the same tree dies before
any write, and the declared mutation still reddens its named proof with the manifest's `expect`
string. The reviewer also confirmed the retracted `set -u` wording is now gone repo-wide.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Seventh cold review. Three findings, all in prose I wrote correcting earlier prose.
HIGH — "14 of 36 did" is not a failure count. `grep -c` over pytest output counts LINES, and
`--tb=line` prints each failure twice, so 14 was 7 failures doubled; only 5 distinct tests are
involved, and with truncation disabled the figure moves the other way (31 of 36). Measured:
raw grep -c 14
of which '^E ' rows 7
of which '^/' tb=line rows 7
DISTINCT failing tests 5
A number that changes with pytest's display options is not a fact about the code, so no figure
is given at all now — which is what the same comment already said it was doing about counts.
The trap itself is recorded so one is not added back.
MEDIUM — "ONE named guard failure INSTEAD OF half the suite reddening" asserts a substitution
that does not happen, and contradicted the sentence directly above it. Measured on the aligned
tree: the guard failure appears AND the ~36 unrelated reds still appear, in the same job.
Alignment adds a diagnosis; it replaces nothing. Now "ALONGSIDE".
MEDIUM — the `set -u` sweep was incomplete, and the previous commit message's "gone repo-wide"
was false. Four files still asserted the refuted mechanism. The sharpest was
`scripts/check-review-verdict.sh`, whose comment said "see the sentinel comment in the library"
— and following that pointer landed on the retraction saying the opposite. Corrected there, in
`docs/ci-cd.md` and in `test_review_verdict_vocabulary.py`, all three already touched by this
branch. Every remaining mention in code, tests and docs is now a quotation INSIDE a retraction;
the only standing assertion left is the decision record, which is its own change and stays with
ersatztv#886 (now narrowed to exactly that).
CORRECTION to the previous commit message: it said the retracted wording was "gone repo-wide".
That was true only of the two phrases the round-6 reviewer checked, not of the mechanism. It is
true now for code, tests and docs.
Also: dropped a "for the same reason" back-reference that resolved to the wrong reason (drift,
where the actual reason is falsity), and removed the session narrative from the figure notes —
"a first attempt said 14" is autobiography; the durable form is the trap and its consequence.
Verified again on this tree: full suite 1278 passed / 2 skipped, catalog in sync,
decisions-validate OK, doc-narrative clean.
fixes#845
refs #886
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Self-caught while verifying my own correction, which is the point at which it should have been
caught the previous three times.
The note added last commit argues that no figure should be given for how many failures name the
second assignment, because such counts are artifacts of pytest display flags. It then offered
"7 under the default display and 31 under `--tb=long -vv`" as evidence. The 7 is reproducible.
The 31 was taken from a review report and never re-measured; counting the same way against
`--tb=long -vv` gives 62, so either 31 is a different counting or it is wrong, and I cannot tell
which. Quoting a number from a report instead of reproducing it is the exact failure the note
was written to prevent, committed inside the note.
Every figure is gone from all three sites. What survives is the MECHANISM of the trap — a
`grep -c` counts lines and `--tb=line` prints each matching failure twice, so it overstates;
raising the traceback detail moves it the other way — plus the qualitative "about half the suite
reddens", which is coarse enough to be robust. Reproducing any exact figure needs per-failure
parsing rather than grepping, and that is now what the note says instead of demonstrating it
with numbers.
Also dropped "36 of 75" from the guard comment. It reproduces today, but a note whose argument
is that counts drift as tests are added should not carry one.
No code changed. Full suite 1278 passed / 2 skipped; catalog in sync; decisions-validate OK.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Eighth cold review. It found that the correction shipped over the last two commits inverted a
TRUE statement into a false one, in live merge-gate code. Verified independently before acting,
and the reviewer is right.
WHAT I MEASURED WRONG. My probe used a PLAIN unset variable reference (`: "$UNSET"`) and I
generalised the result to `${#arr[@]}`, which is the expansion the validator actually uses.
Those shapes do not behave alike. Measured on bash 3.2.57 and 5.3.15, `if ! f` form, function
aborting on each shape:
shape set -u set -eu
plain $UNSET rc=127 exits rc=1 exits
unset ${#UNSET_ARR[@]} rc=0 CONTINUES rc=1 exits
scalar ${#SCALAR[@]} rc=0 CONTINUES rc=1 exits
For both `${#...[@]}` rows under `set -u` alone: continued-past-gate=1, a-branch-ran=0. So the
wording on `main` was correct as written — the abort takes neither branch of `if ! validate` and
execution continues past the gate — for exactly the shape the validator uses and exactly the
flag set its consumer runs: `check-review-verdict.sh` is `set -uo pipefail`, deliberately
without `-e`. The repo's own `test_MUTATION_with_the_array_assertion_disarmed_the_SENTINEL_still_refuses`
is the end-to-end proof: it asserts a message only emitted AFTER the gate, and it passes.
The concrete harm of the version being withdrawn: it had `review-verdict-vocabulary.sh` saying
"there is no skip to defend against" — an invitation to delete the sentinel and reopen the
measured fail-open that file exists to close.
WITHDRAWN WHOLESALE rather than patched a third time, per the budget-3-rounds-then-withdraw rule
this repo records for string/prose predicates. `scripts/lib/review-verdict-vocabulary.sh` and
`scripts/check-review-verdict.sh` are byte-identical to `main` again; the `docs/ci-cd.md`
paragraph and the `test_review_verdict_vocabulary.py` comment are restored verbatim. Kept from
those commits: the `docs/ci-cd.md` input-set fix (and its wrap repair), which is unrelated and
sound.
`scripts/lib/h10-reviewers.sh` keeps a note on the topic, now correctly scoped by BOTH flag set
and expansion shape: the route is real for `${#arr[@]}` under `set -u` without `-e`, which is the
vocabulary library's consumer, and is not reachable from `post-review-verdict.sh`, which runs
`set -euo pipefail` where the shell exits on every shape. So the sentinel here rests on the
data-dependency ground alone.
ersatztv#886 is CLOSED as invalid — it asked for a record to be "corrected" that is right — with
the measurement recorded on it.
CORRECTION to the two previous commit messages: their claims that the mechanism was refuted, and
that the retracted wording was "gone repo-wide" for code/tests/docs, are both withdrawn.
Reusable: a runtime claim needs a probe that exercises the SAME CONSTRUCT, not merely the same
flags. `set -u` is not one behaviour — it is one per expansion shape.
Full suite 1278 passed / 2 skipped; catalog in sync; decisions-validate OK.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Round 9 returned MERGEABLE with four LOW/NIT observations; these are the two that are
actionable in the tree.
`docs/decisions/records/ci/script-tests-job.md` still enumerated the `script-tests` input set as
TWO top-level directories after this branch raised both derived copies (`docs/ci-cd.md` and the
comment in `pr-checks.yml`) to three. Nothing was contradicted — "spans more than `scripts/`" is
a true lower bound — but CLAUDE.md says convention lookups resolve at the record, and the
`.gitea/workflows/**` member was missing from exactly the place a reader is told to look. This
branch created that gap, so it closes it. The enumeration is now explicitly a lower bound rather
than a census, since the corpus tests read `docs/` as well, and the stale present-tense "~10s" is
reframed as the figure at decision time rather than a current one.
`scripts/lib/h10-reviewers.sh`: a colon attributed the `-e` scoping and the bash-version
measurement to `review-verdict-vocabulary.sh`, which documents neither. That library documents
the MECHANISM; the scoping is stated and measured here. Reworded.
The two NITs not acted on, and why: the "CORRECTION to the two previous commit messages" in
`c03717f72` actually spans three commits, and a `rc=127` cited there is invocation-dependent
(127 under `bash -c`, 1 from a script file). Both are commit-message-only and immutable. The
same figure appeared in the ersatztv#886 closing comment, which IS editable — it now reports the
stable outcome (which arm ran, whether execution continued) instead of an exit code, with the
invocation dependence stated.
Full suite 1278 passed / 2 skipped; catalog in sync; decisions-validate OK; doc-narrative clean.
fixes#845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Nine independent cold review rounds, all worktree-isolated; one cross-family (GPT-5.6 via Codex). Round 9 returned MERGEABLE with four LOW/NIT items, two of which are fixed in this head. Full scripts/tests suite green (1278 passed, 2 skipped); the declared mutation executes each run and reddens its named proof with the manifest expect string; every clause disarmed individually and confirmed to redden its own test; live probes against Gitea 1.27.1 for the row shape, description round-trip, paging order and required-check list.
Review-verdict: MERGEABLE @ 65a36f2
Nine independent cold review rounds, all worktree-isolated; one cross-family (GPT-5.6 via Codex). Round 9 returned MERGEABLE with four LOW/NIT items, two of which are fixed in this head. Full scripts/tests suite green (1278 passed, 2 skipped); the declared mutation executes each run and reddens its named proof with the manifest expect string; every clause disarmed individually and confirmed to redden its own test; live probes against Gitea 1.27.1 for the row shape, description round-trip, paging order and required-check list.
timothy
merged commit 5d955000f3 into main2026-08-30 04:43:09 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
.gitea/workflows/review-verdict.ymlinherits an existingreview-verdict/h10=successonly from astatus whose
.creator.loginis on itsH10_REVIEWERSallow-list (#742).scripts/post-review-verdict.shwrites those verdicts with whatever account ownsETV_GITEA_TOKEN/ETV_GITEA_BASICAUTHand never asked whose it was. Two coupled values, nothingasserting the coupling — and the failure was the silent kind: the status really is written, the tool
really does report success, and the next
pull_request_targetevent re-derives it and posts over it.And again. The PR deadlocks and the only trace is a
::warning::in a run nobody reads.What changed
The writer now measures its own effect. After posting the status it reads it back, identifies
its own write by state and description, and refuses — before the comment, so the surviving
half-state is the documented
askone — unless the recorded creator is on the allow-list. Measuredafter the write rather than probed before it: a pre-flight
GET /userwould test what the credentialclaims to be, while this tests what Gitea recorded as the author of this status, which is the value
the gate actually reads. It also needs no scope beyond the repo access the POST already required, so
it cannot become a new way to block a narrowly-scoped but legitimate reviewer token.
The allow-list is derived, not restated. New
scripts/lib/h10-reviewers.shparses the singleanchored
H10_REVIEWERS="..."assignment out of the gate's own definition. One declaration, not twoheld together by a parity test — the shape
testing.guard-derives-population-from-sourcerecords sixfailed rounds against.
Why a parse and not a shared declaration both sides source, since that would obviously be better:
the gate runs against a checkout of the PR's base sha. A PR whose base predates a new
scripts/lib/file would not have it, and a missingsourceunderset -euo pipefailkills the job— which posts no
review-verdict/h10at all and blocks every merge tomain, including the PRthat would repair it, with no admin override (#743). A fallback list for that case reintroduces the
second copy. So the derivation runs on the side that can fail safely and the gate's body is untouched.
Fail-closed on every branch: 0 or >1 anchored assignments, a non-plain-login entry, an empty list, or
an unreadable file all refuse before any network call. The membership query answers member /
not-a-member / cannot-tell as three distinct codes, so a caller cannot fold "cannot tell" into either.
The safety direction is unchanged and this PR does not claim otherwise. A non-allow-listed verdict
was always a stall, never a fail-open. What changes is where the signal lands: at the terminal that
made the mistake, instead of one PR event later.
Two defects I shipped and then caught by probing the live instance
Both were found by asking Gitea rather than re-reading my own code, and both are recorded in the
commits because the reason they survived is the reusable part.
.status, not.state. A row inside.statuses[]serialises its state understatus;stateis only the top-level aggregate. Reading.stateper row yields""for everywell-formed response, compares unequal to the state just posted, and would have refused every
verdict — a repo-wide deadlock, shipped green. The tests could not catch it because the
curlshim replayed the status POST payload as the read-back body, and a POST body legitimately uses
state. The fixture agreed with the parser by construction. The shim now renames the field onreplay, and a separate test pins a verbatim live-Gitea body independently of the shim.
Measured both ways: with the fixture fixed and
.staterestored, 25 tests redden; before thefixture fix, zero did.
?limit=2against a 15-context headreturned 2 rows and
total_count: 2— the count reports the page, not the total, so no fielddistinguishes a truncated body from a complete one. The call now asks for
?limit=100, and thenot-found refusal names no cause (absence and truncation are not separable here) and reports
the row count it read, so a future overflow is legible.
What the review rounds found
Four independent cold reviewers, all worktree-isolated and briefed with no implementation context;
one of them cross-family (GPT-5.6 via Codex). Every finding below was reproduced before it was acted
on. Where two reviewers disagreed, no arbitration was needed — the MERGEABLE one had simply not
examined the blocking items, and both were verifiable.
Round 1 — the guard did not walk the same tree, and CLAUDE.md was wrong
test_the_H10_REVIEWERS_list_is_glob_free_and_non_emptyparsed the classify step'srun:body;the library greps the whole YAML and refuses on anything but exactly one match. A second
anchored
H10_REVIEWERS=in any otherrun:block was therefore invisible to the guard whilemaking the writer refuse every verdict — a repo-wide merge outage on a PR-only
main. Reproduced:green guard, 31 red writer tests whose diagnostics named no second assignment. The guard now reads
the whole file.
CLAUDE.mdsaid the status is not written. It is — only the comment is withheld. It now alsosays what no doc did: the green left standing satisfies the server-side required check, because
branch protection binds the context name, not its issuer.
.statusesarray-TYPE test, the read-back GET's explicit
|| die,?limit=100, and theset -frestoration.The first two passed only because a downstream check produced the same non-zero exit. They now
assert their own diagnostic, and the shim models the measured server so
?limit=is observable.[0]over same-context rows had no uniqueness guard; the row count went blank on a200-with-empty-body (jq exits 0 with empty output, so the
||fallback was dead exactly where itmattered); the split pinned globbing but not
IFS; and theset -urationale I inherited iswrong — both reviewers measured the shell exiting outright in both shapes.
Round 2 (cross-family) — two of my corrections had themselves gone wrong
(.status // .state)fallback recreated #845. I added it as harmless defensiveness afterfixing the
.statebug. The gate reads.statusand nothing else, so on a response carrying only.statethe fallback made the writer accept and report success while the gate saw no recognisableverdict and re-derived it — the exact stall, reached through the check meant to prevent it.
failuretoo, which the gate deliberately does not do. Itinherits a
successonly from a member but afailurefrom any attributable account. Refusing anoff-list
BLOCKEDrefused a verdict the gate honours, stated a falsehood in the diagnostic, and —because the comment is withheld on refusal — left a real reviewer no supported way to record a
rejection at all.
descending, so a just-posted row is always on page 1. My shim appended it last and sliced from the
front, manufacturing a truncation the server cannot produce — which made one test assert a fiction
and made the
?limit=100"witness" circular: observable only because the fixture was wrong inprecisely the direction that made it observable.
Round 3 — a correction that overshot
"A repo-wide merge outage" overstated the guard's role. Such an edit landing would be one, but
script-testsis advisory — measured, the required contexts areBuild & test,EF migration integrityandreview-verdict/h10— so the writer suite already reddens the PR. What alignment buysis a diagnostic naming the cause instead of 31 unrelated failures. Restated in all four places.
Round 4 — a declared security invariant with no guard
scripts/lib/h10-reviewers.shstates, anddocs/remote-state-inventory.mdrecords as a decision,that no environment variable selects which workflow the allow-list comes from. The property held —
but nothing pinned it, and restoring the override the way a refactor naturally would left the whole
suite green while an env var installed an arbitrary allow-list and an off-list account was accepted
as a reviewer. Now tested, and confirmed to redden under that exact mutation.
The same round found a behavioural one: a
BLOCKEDverdict was refused when the allow-list could notbe derived. Membership never applies to a rejection, so a broken
H10_REVIEWERSdeclaration wasblocking the one thing the
success-only rule exists to protect — on the branch most likely to havebroken it. The load is now guarded by state; every reference to the library sits inside one of the
two
state = successblocks, so thefailurepath never touches it.And four prose claims falsifiable by
greporpytest, including one that re-taught its own lesson:"31 red writer tests" had drifted to 40 inside this branch as tests were added, in all four places
the previous commit said it had restated. Replaced with the invariant.
Round 5 — my corrections were themselves falsifiable by grep
The sharpest round, and aimed at this branch's own thesis. Three of the four findings were claims a
previous commit message said it had fixed: "replaced in all four places" missed two of five
occurrences; "the WHOLE writer suite reddens" is measurably false (36 of 75, not all — I invoked
state-the-invariant-not-the-measurementand then wrote a stronger claim than the count I removed);a retraction left the retracted ground asserted seven lines below it; and "spans THREE top-level
directories" replaced a true hedge with a false count (the suite reads seven, including
docs/, sothis PR's own doc edits are inputs to that job). All now stated without counts, or hedged to what is
measurable.
Round 6 (self-directed) — sweeping the retracted words, not the file
Chasing the retracted wording repo-wide surfaced the same shape once more, in the file the previous
round had just corrected. It also surfaced a real question: the decision record justifies the sentinel
by noting the read side deliberately runs without
set -e, and every prior measurement had usedset -euo pipefail. If the original claim held there, the correction would have been wrong. Measuredacross bash 3.2.57 and 5.3.15 in all three modes —
set -ualone exits 127,set -euandset -euo pipefailexit 1, no branch taken in any of them. The correction stands, including for thecase the record relies on.
Rounds 7 and 8 — a correction that inverted a true statement, and its withdrawal
Round 7 caught two more prose defects: a "14 of 36" figure that was a grep LINE count (7 failures
printed twice by
--tb=line), and an "instead of" that claimed alignment REPLACES the unrelated redswhen measurement shows it adds a named failure alongside them. Both fixed; all figures removed, since
a number that changes with pytest display flags is not a fact about the code.
Round 8 found the serious one. The
set -u"correction" made in rounds 5–6 had inverted a truestatement into a false one, in live merge-gate code. My probe used a plain
$UNSET; the validatoruses
${#arr[@]}, and those shapes differ:The wording on
mainwas right all along, for exactly the expansion the validator uses and exactlythe flags its consumer runs (
check-review-verdict.shisset -uo pipefail, deliberately without-e). The repo's owntest_MUTATION_..._SENTINEL_still_refusesproves it — it asserts a messageonly emitted after the gate, and it passes. The version I had shipped said "there is no skip to
defend against", which invites deleting the sentinel and reopening the fail-open that file exists to
close.
Withdrawn wholesale rather than patched a third time.
review-verdict-vocabulary.shandcheck-review-verdict.share byte-identical tomainagain; the doc paragraph and test comment arerestored verbatim. What survives is one correctly-scoped note in the new library. #886 is closed
as invalid — I had filed it asking for a record to be "corrected" that was right.
Guard status
scripts/post-review-verdict.shmoves BEHAVIOUR-ONLY → MUTATION, which the manifest's ownUNDECLAREDnote called "the most valuable upgrade on this list". The declared clause is in thegate, not the script: rewriting
H10_REVIEWERSwhile the posting account stays fixed reddens theaccept path only if the writer reads the list live and the comparison gates the outcome.
Disarming the comparison instead would prove the second and say nothing about the first, which is the
half that was missing.
Verification
scripts/testssuite green.my own disarms were inert (a character class that still excluded
*; an IFS clause matchingtwo sites and applied to neither), which briefly read as "no test covers this". A mutation has to
be checked to actually mutate.
creatoras a string,statusesnull / non-array / empty): every one fails closed.Docs
ci.exemption-provenancenow records the coupling as asserted rather than as a tracked residual(
Decisions-Edit: yes), plusdocs/ci-cd.md,CLAUDE.md,docs/guard-inventory.mdanddocs/remote-state-inventory.md.Not in scope
The refused-verdict residual — a non-inheritable status left standing with no comment beside it — is
deliberately not repaired here. That is the
askhalf-staterelease.verdict-writes-status-before-commentalready designates as safe, and a second correctivewrite is the sticky-sentinel mechanism #849 is separately designing; inventing a parallel one on
this path would be two mechanisms for one invariant.
fixes #845
.status, not.state— the fixture was mirroring my POST body, not Gitea 8c580cbfcdSecond self-review defect, again found by probing rather than reading. `GET /commits/{sha}/status` PAGES, and it pages undetectably: measured on Gitea 1.27.1 (2026-08-29), `?limit=2` against a head carrying 15 contexts returned 2 rows AND `total_count: 2`. The count reports the PAGE, not the total, so no field in the body distinguishes a truncated response from a complete one. A head whose context count grew past the server's default page size would silently stop containing the `review-verdict/h10` row, and the read-back would refuse a verdict that is actually in force — the deadlock direction, on the one path that must not deadlock. The call now asks for `?limit=100` against the ~15 contexts this repo produces. The not-found refusal no longer states a cause. Two causes reach it — the status is gone, or the list was truncated — and the response cannot tell them apart, so asserting either would be the #859 defect: a message that sends the reader hunting something that did not happen. It now reports how many rows it actually read and names neither, which also makes a future paging overflow legible instead of mysterious. Recorded in `docs/remote-state-inventory.md` on the writer's own row, since it is a property of that read rather than of the tool. fixes #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWProbed, not reasoned: a `creator` arriving as a STRING rather than an object makes jq exit 5 ("Cannot index string with string"). That already failed closed — the abort lands before the comment — but with only jq's stderr, which never says a status was left standing on the head. Each extraction now carries its own message. The no-creator refusal said the status "is how a status posted by an ACTIONS token appears". Three shapes reach that branch (`creator` absent, null, or an empty login) and only one of them is an Actions token, so the message now states the fact first and offers the cause as the likely one rather than the established one. fixes #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWThe comment stated that `/commits/{sha}/status` returns the latest row per context as a bare fact. It was measured — a head carrying two `review-verdict/h10` rows in the `/statuses` list returned only the newer one from the combined endpoint — so the comment now says so. An unattributed claim about an API is the kind that quietly stops being true and talks the next reader out of re-checking it. refs #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Two cold reviewers, independently briefed and worktree-isolated. One returned BLOCKED on the first two below; the other returned MERGEABLE having not examined them. No arbitration was needed — both are verifiable, and I reproduced each before acting. BLOCKING 1 — the CI guard parsed a different population than the library. `test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty` read `_classify_step()["run"]`, one step's body. `scripts/lib/h10-reviewers.sh` greps the WHOLE YAML and refuses on anything but exactly one match. So a second anchored `H10_REVIEWERS=` in ANY other `run:` block was invisible to the guard while making the writer refuse every verdict — and `main` is PR-only with `review-verdict/h10` required, so that is a repo-wide merge outage. Reproduced: injecting one into the last step left the guard green and reddened 31 writer tests whose diagnostics said nothing about a second assignment. The guard now reads the whole file, so it walks the same tree as its subject; the header and the decision record no longer claim coverage that did not exist. BLOCKING 2 — `CLAUDE.md` said an off-list verdict "fails loudly at your terminal instead of being written". The status IS written; only the comment is withheld. The script, `ci-cd.md` and the record all said so correctly, and the most-read doc contradicted them. It now also says what the other docs did not: the green left standing satisfies the server-side required check, because branch protection binds the context NAME and not its issuer. Four clauses were shipped UNWITNESSED, found by the reviewer's own disarm matrix and each now reddening a named test: the `.statuses` array-TYPE test, the read-back GET's explicit `|| die`, `?limit=100`, and the `set -f` restoration. The first two passed only because a downstream check produced the same non-zero exit — the shape this very file already documents at the `-z`/`-n` arms. They now assert their own diagnostic. `?limit=100` was unobservable because the shim discarded the query string; the shim now models the measured server (default 30, clamped to 50, `total_count` reporting the page). Also fixed, all reproduced first: - `[0]` over same-context rows had no uniqueness guard. Unreachable today, but the alternative is judging one row's author while another is the one standing. - The row count in the not-found refusal came out BLANK on a 200 with an empty body: jq exits 0 with empty output, so the `||` fallback was dead exactly where the count matters. Tests the value now, not jq's exit. - The split pinned globbing but not IFS. A caller with `IFS=-` made `renovate` read as a member of `renovate-bot`. Unreachable from this script (bash resets IFS), pinned because the header invites reuse. - The `set -u` rationale was inherited and is WRONG. Both reviewers measured it: the shell EXITS, in both the `if ! f` and the `f; rc=$?` shapes, on bash 5.3 and 3.2. Nothing here depended on it; the sentinel now rests on the two grounds that were measured. - The `?limit=100` margin was overstated. This instance clamps to MAX_RESPONSE_ITEMS = 50, so the headroom is ~3x over today's 8-15 contexts, not 6x, and it shrinks as checks are added. - The snapshot boundary now names its DIRECTION in all three places: it is a false ACCEPT, and the likely edit (ADDING a reviewer) is exactly the one it mis-handles. - The overwrite refusal told the operator to re-review when a gate re-derive needs no review. A note on the measurements, since two of my own disarms were inert and briefly read as "no test covers this": a mutation must be checked to actually mutate. The glob-class disarm still excluded `*`, and the IFS disarm matched two sites and applied to neither. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWFourth cold review, worktree-isolated, on the rebased head. Every finding reproduced before acting; the two behavioural ones were re-verified with the reviewer's own mutations. MEDIUM — "no environment variable selects the source file" was declared as a deliberate security decision, in the library header AND in `docs/remote-state-inventory.md`, with no test anywhere. The property held, but restoring the override the way a refactor naturally would (`ETV_H10_REVIEWERS_WORKFLOW="${ETV_H10_REVIEWERS_WORKFLOW:-}"`) left the whole suite green while an env var installed an arbitrary allow-list and an off-list account was accepted as a reviewer. That is the class this branch removed in `28aab4632`, in a clause labelled as a decision rather than as untested. Now pinned, and confirmed to redden under that exact mutation. LOW (behavioural) — a BLOCKED verdict was refused when the allow-list could not be DERIVED. Membership never applies to a `failure`, but the derivation was loaded unconditionally, so a workflow with a broken `H10_REVIEWERS` declaration refused rejections too — the exact outcome the success-only rule exists to prevent ("no supported way to record a rejection"), reached one condition earlier, on the branch most likely to have broken that declaration: the one editing it. The load is now guarded by state; both of its stated properties (after argument validation, before the first network call) survive. A test covers the rejection AND the approval on the same tree, so the guard was narrowed rather than removed. Prose, all four falsifiable by `grep` or `pytest`: - The comment claimed `review-verdict.yml` "rejected a hardcoded limit for its own read". It does not — it sends `?limit=100` on both reads. What it rejected was using that limit as a COMPLETENESS argument, which was dead code because the instance caps at 50. The real point (presence vs absence) was right and is kept; a reader who greps the workflow no longer finds the opposite of what this says. - "31 red writer tests" had drifted to 40 INSIDE this branch as tests were added, in all four places the previous commit said it had restated. Replaced with the invariant — the whole writer suite reddens — per `state-the-invariant-not-the-measurement`, which is exactly the lesson the stale number re-taught. - The sentinel-reset comment cited a measurement that does not isolate it: removing the reset produces a byte-identical refusal, because `load` overwrites both variables on that path. The reset's real ground is the caller that never reaches `load`, which is what `contains` returning 2 rests on. Re-attributed. - `docs/ci-cd.md` still mirrored a TWO-directory input set for `script-tests` after this same PR widened the workflow's own comment to three. Synced. Also: `scripts/lib/review-verdict-vocabulary.sh` still shipped the `set -u` claim this branch demonstrated to be wrong, one file over, and it was the source the new library copied it from. Corrected there too — measured on bash 3.2.57 and 5.3.15, the shell exits outright in both the `if ! f` and `f; rc=$?` shapes; the old reading holds only inside a command substitution. Its sentinel keeps the ground that survives. Nits: a stale "~10s" runtime in a sentence this branch rewrote, replaced with the reason that does not drift; an unresolvable breadcrumb citing a private memory filename rather than a decision key, dropped; "so only this run writes it" narrowed to what the description actually identifies; and the paging test's docstring now says it is a POSITIVE CONTROL rather than claiming a property pin it cannot deliver. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWFifth cold review, on the previous fix commit. Every finding is prose, and three of them are claims that commit's own message said it had fixed. On a branch whose stated thesis is "four prose claims were falsifiable by grep", shipping four more is the finding. Each verified here before acting. - "replaced in all four places" was wrong: there were FIVE occurrences of `31`. Two survived, one of them four lines below the sentence the same commit added saying the number drifts. - "the WHOLE writer suite reddens" is measurably false. Injecting a second anchored `H10_REVIEWERS=` gives 36 failed / 39 passed — a large part, not all. The commit invoked `state-the-invariant-not-the-measurement` and then wrote a claim STRONGER than the count it removed. Now stated without a count and without "whole". - The sentinel re-attribution retracted the environment-defeating ground in one paragraph and still asserted it seven lines below ("the two that were measured"). The sibling library corrected in the same commit got this right ("the ground that survives", singular). This is `stale-comments-sweep-by-subject`: after a retraction, grep the retracted words. - "spans THREE top-level directories" replaced a TRUE hedge with a false count, and did not match the workflow it was supposed to be synced with (which says "at least three"). The suite reads seven — `.claude`, `.gitea`, `.husky`, `docs`, `ErsatzTV`, `scripts`, `web` — so this PR's own doc edits are inputs to that job. Hedge restored, with the reason. Also from that review: the `if [ "$state" = "success" ]` body was unindented at column 0, and "Fails CLOSED on every branch" now means every branch OF THAT LOAD, since the load is itself conditional. Both fixed; the indent change is whitespace-only (`git diff -w` empty). The behaviour was attacked again and held. The reviewer traced a BLOCKED verdict end to end with `scripts/lib/h10-reviewers.sh` DELETED — rc=0, status and comment both posted, no unbound variable and no command-not-found, because `$state` is set by the `case` well before the guard and every reference to the library sits inside one of the two `state = success` blocks. An APPROVAL on the same tree still refuses before writing anything. Removing the guard reddens the new rejection test; `if true` reddens 20 tests. The declared mutation still reddens its named proof. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWset -umechanism survived seven lines below its own retraction — and the no-set -ecase is now measured 94cc924c36Found by sweeping the RETRACTED WORDS repo-wide rather than re-reading the file I had edited, which is the `stale-comments-sweep-by-subject` lesson the previous round already charged me with once. `scripts/lib/review-verdict-vocabulary.sh` still said "A control-flow gate can be skipped by an abort" seven lines under the paragraph retracting exactly that mechanism — the same self-contradiction shape as the sentinel comment in the last round, in the file that round corrected. MEASURING IT PROPERLY CHANGED WHAT I CHECKED. The decision record justifies the concern by noting the READ side deliberately runs without `set -e`, and both prior reviewers had measured the claim only under `set -euo pipefail`. If the original reading held without `set -e`, the correction shipped last round would itself have been wrong. Measured on bash 3.2.57 and 5.3.15 across all three modes: [set -u] in f / DEFINITELY_UNSET: unbound variable / exit=127 [set -eu] in f / DEFINITELY_UNSET: unbound variable / exit=1 [set -euo pipefail] in f / DEFINITELY_UNSET: unbound variable / exit=1 No branch taken and nothing after the gate, in every mode — including the one the record relies on. The correction stands. The old reading holds only inside a command substitution (`v=$(f) || v=FALLBACK` continues, fallback taken), which is not the shape gating either file. WHAT IS NOT IN QUESTION, stated because the retraction is easy to over-read: the fail-open that record documents is REAL — with `.*` in the positive list and the negative list written as a scalar, an explicit `BLOCKED @ <head>` classified `positive`, exit 0. Only the attributed mechanism is wrong: `${#name[@]}` on a scalar yields 1 rather than erroring, so no abort occurred in that scenario at all. The sentinel is still the right design, on the ground that survives — the caller that never runs validation. The same refuted mechanism remains in `release.verdict-vocabulary-shared` and in `test_review_verdict_vocabulary.py`. Correcting a decision record is its own change with its own review surface, so it is tracked as ersatztv#886 with the full measurement, and cross-referenced from the corrected file rather than folded in here. fixes #845 refs #886 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWSixth cold review. One HIGH, and it is the sentence the previous commit rewrote. The claim, in five sites across three files, was that a second anchored `H10_REVIEWERS=` reddened the writer suite "with no diagnostic naming a second assignment". Measured: 14 of the 36 failures carry this library's own refusal, verbatim — E AssertionError: h10 reviewers: .../review-verdict.yml carries 2 anchored H10_REVIEWERS assignments, not exactly 1 — refusing to guess which one the gate runs with — which names both the count and the variable. The message predates the alignment commit that introduced the claim, so it was false when written, not newly broken here. What is actually missing is a failure that POINTS AT THE WORKFLOW: the reds surface under unrelated test names (`test_positive_verdict_posts_success_on_the_resolved_head`, …) and the cause has to be inferred from a stack of them. Aligning the populations buys ONE named guard failure instead. All five sites now say that, and "about half the suite" replaces the earlier "the writer suite" where it read as all of it (36 of 75). Two nits from the same round: the aside inserted into `docs/ci-cd.md` turned an em-dash from an introducer into a closing parenthetical and left the enumeration dangling — the hedge now follows the sentence instead of splitting it; and "on that ground alone, which is the one that was measured" implied a contrast that does not exist, since the retracted ground was measured too. Trailing clause dropped. CORRECTION to the previous commit message, which no artifact carries but which was wrong: it said "Removing the guard reddens the new rejection test; `if true` reddens 20 tests", pairing one mutation with two counts. Measured on the load guard: `if true` reddens 1 (the rejection test), `if false` reddens 20. The behaviour was attacked again and held: BLOCKED end to end with the library DELETED gives rc=0 with status and comment posted and an empty stderr, MERGEABLE on the same tree dies before any write, and the declared mutation still reddens its named proof with the manifest's `expect` string. The reviewer also confirmed the retracted `set -u` wording is now gone repo-wide. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWSeventh cold review. Three findings, all in prose I wrote correcting earlier prose. HIGH — "14 of 36 did" is not a failure count. `grep -c` over pytest output counts LINES, and `--tb=line` prints each failure twice, so 14 was 7 failures doubled; only 5 distinct tests are involved, and with truncation disabled the figure moves the other way (31 of 36). Measured: raw grep -c 14 of which '^E ' rows 7 of which '^/' tb=line rows 7 DISTINCT failing tests 5 A number that changes with pytest's display options is not a fact about the code, so no figure is given at all now — which is what the same comment already said it was doing about counts. The trap itself is recorded so one is not added back. MEDIUM — "ONE named guard failure INSTEAD OF half the suite reddening" asserts a substitution that does not happen, and contradicted the sentence directly above it. Measured on the aligned tree: the guard failure appears AND the ~36 unrelated reds still appear, in the same job. Alignment adds a diagnosis; it replaces nothing. Now "ALONGSIDE". MEDIUM — the `set -u` sweep was incomplete, and the previous commit message's "gone repo-wide" was false. Four files still asserted the refuted mechanism. The sharpest was `scripts/check-review-verdict.sh`, whose comment said "see the sentinel comment in the library" — and following that pointer landed on the retraction saying the opposite. Corrected there, in `docs/ci-cd.md` and in `test_review_verdict_vocabulary.py`, all three already touched by this branch. Every remaining mention in code, tests and docs is now a quotation INSIDE a retraction; the only standing assertion left is the decision record, which is its own change and stays with ersatztv#886 (now narrowed to exactly that). CORRECTION to the previous commit message: it said the retracted wording was "gone repo-wide". That was true only of the two phrases the round-6 reviewer checked, not of the mechanism. It is true now for code, tests and docs. Also: dropped a "for the same reason" back-reference that resolved to the wrong reason (drift, where the actual reason is falsity), and removed the session narrative from the figure notes — "a first attempt said 14" is autobiography; the durable form is the trap and its consequence. Verified again on this tree: full suite 1278 passed / 2 skipped, catalog in sync, decisions-validate OK, doc-narrative clean. fixes #845 refs #886 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWset -uretraction — the original claim was right and my probe used the wrong construct dc09eceee9Eighth cold review. It found that the correction shipped over the last two commits inverted a TRUE statement into a false one, in live merge-gate code. Verified independently before acting, and the reviewer is right. WHAT I MEASURED WRONG. My probe used a PLAIN unset variable reference (`: "$UNSET"`) and I generalised the result to `${#arr[@]}`, which is the expansion the validator actually uses. Those shapes do not behave alike. Measured on bash 3.2.57 and 5.3.15, `if ! f` form, function aborting on each shape: shape set -u set -eu plain $UNSET rc=127 exits rc=1 exits unset ${#UNSET_ARR[@]} rc=0 CONTINUES rc=1 exits scalar ${#SCALAR[@]} rc=0 CONTINUES rc=1 exits For both `${#...[@]}` rows under `set -u` alone: continued-past-gate=1, a-branch-ran=0. So the wording on `main` was correct as written — the abort takes neither branch of `if ! validate` and execution continues past the gate — for exactly the shape the validator uses and exactly the flag set its consumer runs: `check-review-verdict.sh` is `set -uo pipefail`, deliberately without `-e`. The repo's own `test_MUTATION_with_the_array_assertion_disarmed_the_SENTINEL_still_refuses` is the end-to-end proof: it asserts a message only emitted AFTER the gate, and it passes. The concrete harm of the version being withdrawn: it had `review-verdict-vocabulary.sh` saying "there is no skip to defend against" — an invitation to delete the sentinel and reopen the measured fail-open that file exists to close. WITHDRAWN WHOLESALE rather than patched a third time, per the budget-3-rounds-then-withdraw rule this repo records for string/prose predicates. `scripts/lib/review-verdict-vocabulary.sh` and `scripts/check-review-verdict.sh` are byte-identical to `main` again; the `docs/ci-cd.md` paragraph and the `test_review_verdict_vocabulary.py` comment are restored verbatim. Kept from those commits: the `docs/ci-cd.md` input-set fix (and its wrap repair), which is unrelated and sound. `scripts/lib/h10-reviewers.sh` keeps a note on the topic, now correctly scoped by BOTH flag set and expansion shape: the route is real for `${#arr[@]}` under `set -u` without `-e`, which is the vocabulary library's consumer, and is not reachable from `post-review-verdict.sh`, which runs `set -euo pipefail` where the shell exits on every shape. So the sentinel here rests on the data-dependency ground alone. ersatztv#886 is CLOSED as invalid — it asked for a record to be "corrected" that is right — with the measurement recorded on it. CORRECTION to the two previous commit messages: their claims that the mechanism was refuted, and that the retracted wording was "gone repo-wide" for code/tests/docs, are both withdrawn. Reusable: a runtime claim needs a probe that exercises the SAME CONSTRUCT, not merely the same flags. `set -u` is not one behaviour — it is one per expansion shape. Full suite 1278 passed / 2 skipped; catalog in sync; decisions-validate OK. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TWReview-verdict: MERGEABLE @
65a36f2Nine independent cold review rounds, all worktree-isolated; one cross-family (GPT-5.6 via Codex). Round 9 returned MERGEABLE with four LOW/NIT items, two of which are fixed in this head. Full scripts/tests suite green (1278 passed, 2 skipped); the declared mutation executes each run and reddens its named proof with the manifest expect string; every clause disarmed individually and confirmed to redden its own test; live probes against Gitea 1.27.1 for the row shape, description round-trip, paging order and required-check list.