Commit Graph
4 Commits
Author SHA1 Message Date
timothy fe3d29276a fix(706): count a raced SENTINEL, not only a raced human verdict
PR Gates / decisions lifecycle (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 20s
PR Gates / Docs update reminder (pull_request) Successful in 21s
Review verdict / Set review-verdict status (pull_request_target) Successful in 7s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m29s
review-verdict/h10 Review-verdict: MERGEABLE @ fe3d292 (base: main)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m54s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 15m3s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 15m49s
Round-5 cold review: the post-write check counted only human `Review-verdict:`
rows above the high-water mark, which is not sufficient under the run overlap
this branch measured.

Sequence, all inside that regime, runs A and B on the same exempt-classified
sha: the human BLOCKED lands BELOW A's mark (so A cannot see it), B masks it
with an exemption `success`, and only afterwards writes the sentinel. A then
finds nothing human above its mark, does not repair, and posts its own `success`
on top of the sentinel. The human rejection is permanently green and every later
run re-derives it — the repair race failing toward SUCCESS, while the record
states it fails toward `pending`.

The filter now counts two row shapes above the mark: a human verdict (non-null
creator, `Review-verdict:` description) OR a machine sentinel (null creator,
description exactly $REPAIR_DESC). A then repairs and both runs converge on the
fixed point.

It cannot false-fire: a pre-existing sentinel would have been seen at the FIRST
read and forced the pending path, and this block only runs after a `success`, so
a sentinel above the mark can only have been written mid-flight by another run.

Mutation-verified on both halves independently — dropping the sentinel
alternation reddens the new test; dropping the human half reddens the original
race-2 test — so neither can be removed without a test noticing.

Refs #706
2026-08-03 22:22:09 +02:00
timothy 5077408528 fix(706): abstain when a repair sentinel appears mid-run
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
PR Gates / Docs update reminder (pull_request) Successful in 12s
PR Gates / decisions lifecycle (pull_request) Successful in 18s
review-verdict/h10 Awaiting review verdict for 5077408
Review verdict / Set review-verdict status (pull_request_target) Successful in 10s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m46s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m5s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 16s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 20m47s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 23m42s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Round-3 cold review: `ex_repair` was recomputed by the last-moment re-read but
never consulted after it, so the POST wrote the `$state` frozen at
classification time. A stale overlapping run therefore posted its `success`
straight over a sentinel another run had just written — burying a human
rejection with no repair (the human row sits below the stale run's own
high-water mark) and no log entry.

This is the one path in the design that failed toward SUCCESS rather than
`pending`, so it was not covered by the recorded residual, and it is reachable
through exactly the run overlap this branch measured live (probe PR #722: the
older run finished 20s after the newer one started).

The guard is exact rather than conservative: a sentinel present at the FIRST
read forces `state=pending`, so `success` together with `ex_repair=yes` at
re-read time can only mean the sentinel arrived mid-run. Abstaining is then
strictly correct and, unlike the retarget fence, needs no successor run — the
sentinel row is already `pending` and already carries the re-post instruction.

Mutation-verified three ways: removing the guard reddens the new mid-run test
while its positive control stays green; making it unconditional on `ex_repair`
reddens the fixed-point test and the positive control, proving the condition is
precisely scoped and not merely present.

Also strengthens the mark-ordering test to pin the status-history FETCH as well
as its initialisation, closing the refactor evasion review flagged; sliding the
fetch past the re-read now reddens it.

Refs #706
2026-08-03 22:06:42 +02:00
timothy 63040296f4 fix(706): make the repair sentinel a fixed point, not a two-event delay
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 13s
PR Gates / Docs update reminder (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 19s
review-verdict/h10 Awaiting review verdict for 6304029
Review verdict / Set review-verdict status (pull_request_target) Successful in 35s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m44s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 18s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 15s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 16m0s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m6s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Round-2 cold review found the round-1 sentinel self-clobbering: the branch
refused the exemption but fell through to the shared else, which posts the
GENERIC "Awaiting review verdict" description — erasing the very marker the
refusal depends on. The next run saw an ordinary machine `pending`, re-derived
it, and posted `success`, burying the human rejection two events after the
repair instead of one.

The single-hop test passed throughout, and the positive control asserting that
an ordinary machine `pending` DOES re-derive was itself the proof of the second
hop. Durability is a fixed point, and only a chain can assert a fixed point, so
the new test runs the job twice and feeds run N's own posted description in as
run N+1's existing status.

Keyed on `ex_repair` alone rather than on the exempt path: the fact recorded is
"a human verdict was lost on this sha", a property of the sha rather than of
this run's classification.

Verified by mutation — restoring the defect turns the chained test RED while the
single-hop test stays GREEN, which is exactly why the chain was needed.

Refs #706
2026-08-03 21:58:06 +02:00
timothy 8f6d4f4432 fix(706,707,711): fence the review-verdict write on the timeline retarget count
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s
PR Gates / Docs update reminder (pull_request) Successful in 15s
PR Gates / decisions lifecycle (pull_request) Successful in 23s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 23s
review-verdict/h10 Awaiting review verdict for 8f6d4f4
Review verdict / Set review-verdict status (pull_request_target) Successful in 20s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m29s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m26s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m56s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 20m59s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 22m41s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Three related defects in the `review-verdict/h10` gate, all surfaced by the
cross-family review of PR #705.

#706 race 1 — a stale run could overwrite a fresher verdict, permanently. The
race was reproduced live rather than reasoned about (Gitea 1.25.4): with every
other workflow stripped, probe PR #722 showed run 7520 (`opened`) finishing 20s
AFTER run 7521 (`synchronize`) started. `pull_request_target` runs for one PR
genuinely overlap, older finishing last.

The issue proposed serializing with a non-cancelling concurrency group. That is
REFUTED by measurement: with the group active, runs 7528/7529 still overlapped
and 7528 ended 36s after 7529 began. A first probe appeared to show the group
working — a negative control with no `concurrency:` key at all showed the same
cancellations, revealing Gitea auto-cancels superseded `push` runs on its own
and the probe had measured that, not the group. The auto-cancel does not extend
to `pull_request_target`.

The fix leaves the runs unserialized and instead makes an overtaken run decline
to write: count `change_target_branch` events on the PR timeline at start and
again just before the POST, and post nothing if the count moved. The COUNT is
the key because the branch NAME is ABA-vulnerable (`main -> S -> main` reads
`main` at both ends — how #698 route 1 forged its exemption). Abstaining is a
handoff, not a stall: every retarget fires `edited`, so the event that makes a
run abstain has already queued its successor. `updated_at` was rejected as the
key precisely because it moves for comments/labels, which queue nothing.

#706 race 2 — a human BLOCKED landing in the unclosable window between the
pre-POST re-read and the POST was silently turned green. After an exemption
`success` the job now re-reads the per-POST history and repairs its own status
to `pending` if a human verdict appeared above a high-water mark taken just
before the write. The repair is `pending`, never a copy of the human's state.
The id comparison is load-bearing: a presence test would fire forever on a
base-mismatched verdict and deadlock that PR's exemption.

#707 — `pr-changed-files.sh` bound `.base.ref` and `.head.sha` across the
enumeration but never `.base.sha`, so an ordinary advance of `main` mid-paging
could drop a code path from an offset-paged diff and leave a complete-looking
docs-only list. Now bound from the JSON already fetched (no new round trips).

#711 — `.codex/` added to PROTECTED. It mirrors `.claude/hooks/` byte for byte,
including the merge-consent hook, so the "a PR that can weaken the gate cannot
exempt itself" rule had an incomplete path list. Latent today (untracked), live
the moment anyone tracks it.

Residuals are stated, not implied: a retarget inside the final round-trip, and
the repair being itself a read-then-write. Gitea's status API has no
compare-and-set, so neither reaches zero; both now fail toward `pending`.

Tests: 398 pass in scripts/tests. Each new guard was mutation-checked — the
fence's motion comparison, the untrusted-count gate, the repair POST and the id
high-water mark were each neutered in turn and the intended test went red while
its positive control stayed green.

fixes #706
fixes #707
fixes #711

Decisions-Edit: yes
2026-08-03 21:41:16 +02:00