diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index 3a4fc3674..cb70ce633 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -919,8 +919,9 @@ jobs: # a record — resolve decisions through the catalog, never through a key or path quoted in a # comment). The count is trusted ONLY when the walk read EVERY page up to its 20-page cap # AND the LAST page came back empty (ersatztv#870 — reaching an empty page earlier is no - # longer sufficient, and no longer ends the walk). A page that is unreadable, a non-array - # body, a non-numeric length, an empty FIRST page, or a cap reached on a NON-empty page all + # longer sufficient, and no longer ends the walk). A page that is unreadable, a body that + # is neither an array NOR `null` (a bare `null` is this endpoint's empty page and is + # accepted), a non-numeric length, an empty FIRST page, or a cap reached on a NON-empty page all # leave `rt_ok=no`, and an untrusted count is treated below as "cannot tell" rather than as # zero. A SHORT page is not in that list and never was: it is tallied and the walk carries # on — the sentence here said otherwise until 2026-08-30. @@ -983,9 +984,9 @@ jobs: # paragraph below the fix that removed it. # # ONE WALK, TWO COUNTERS, ONE TRUST FLAG. The two axes share `rt_ok` because they share the - # pages it certifies: a walk that never reached a validated empty page has not established - # EITHER count, and giving them separate flags would invite a future edit to trust one over - # pages that did not certify the other. + # pages it certifies: a walk that did not read to its cap and finish on an empty page has + # not established EITHER count, and giving them separate flags would invite a future edit to + # trust one over pages that did not certify the other. count_pr_mutations() { rt_count=0 hp_count=0 @@ -1020,6 +1021,10 @@ jobs: if [ -n "${raw//[[:space:]]/}" ]; then kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind="" fi + # THIS SET MUST MATCH THE `case` ARMS BELOW. Their agreement is load-bearing rather + # than tidy: breaking only on shapes the `case` accepts is what stops a stale `kind` + # from attempt 1 being judged against a `raw` that attempt 2 cleared. Widen one + # without the other and that correspondence is gone. if [ "$kind" = array ] || [ "$kind" = null ]; then break; fi if [ "$try" -eq 1 ]; then sleep 1; fi done @@ -1070,8 +1075,8 @@ jobs: # an empty page BEFORE the cap is SKIPPED, never believed. The ONE empty page still # trusted is the page-20 terminator, and it is trusted for the same unprovable reason # the old page-2 terminator was — the endpoint cannot distinguish a filtered page from - # the end of the list, at ANY offset. What changes is the price: 50 attacker-authored - # rows bought silence before, and ~1000 buy it now. A "require K consecutive empty + # the end of the list, at ANY offset. What changes is the price — see the note on + # the residual below for the one statement of it this file makes. A "require K consecutive empty # pages" rule was rejected because K is arbitrary and only moves the bar to 50K # filtered rows; the cap is not arbitrary — it is the coverage this walk already # claimed to perform. @@ -1093,9 +1098,26 @@ jobs: # cost the "a real page was actually read" property below, which depends on an empty # page 1 being anomalous. COST, both bounds, because only one of them is the common # case: a healthy walk is exactly 20 requests, measured at 2.6s against this - # instance; the worst case with the per-page retry below is 40 requests and 19 - # one-second sleeps, a page sleeping only when its first attempt yielded neither - # an array nor null. + # instance; the worst case with the per-page retry below is 40 requests and 20 + # one-second sleeps — every page blipping once and recovering — which is 20s of + # sleep on top. MEASURED by executing this function against scripted responses, + # not counted by eye: the first draft of this sentence said 19, reasoning that + # the last page could not sleep. It can. + # + # WALL-CLOCK, which is what the timeout is actually for. An earlier version of this + # comment reasoned that only the last page can pay two timeouts, since a page whose + # BOTH attempts fail ends the walk, and put the bound at ~335s. That bounds TIMEOUTS, + # not elapsed time: attempt 1 can burn its full 15s and attempt 2 still SUCCEED + # slowly, so the page costs ~31s and the walk carries on. The pessimum is therefore + # 20 x (15 + 1 + 15) = 620s per walk — ~21min on the exempt path's three walks — and + # the job carries no `timeout-minutes` to catch it. + # + # Stated anyway rather than tuned down, because the DIRECTION is what justifies it: + # before this, `gh` carried no timeout at all and one hung connection had no bound + # whatsoever. Bounded-and-slow replaces unbounded — for THIS call site only. + # `page_statuses` still has no `--max-time`, walks up to 20 pages x 2 attempts, and + # runs in the same post-POST window ahead of the third walk here, so the "a hang + # leaves the green standing" mode is still reachable through the sibling. # # WHAT REMAINS UNCOVERED — NOT CLOSED, NARROWED — stated because #803's whole point # is that a contract must not assert more than its code does. Pages 1-19 cover offsets @@ -1104,11 +1126,14 @@ jobs: # the walk reads an empty page 20, never requests page 21, and publishes a too-low # count with `rt_ok=yes`. Both re-reads share the blind spot, so the counts agree and # the exemption is granted — the SAME construction as before, at the cap instead of at - # page 2, at roughly 10x the price. State that price in ONE unit or it flatters the - # change: BEFORE, the timeline needed ~100 rows (a non-empty page 1, then a 50-row - # filtered block at a 50-aligned offset); AFTER it needs more than 1000, with the - # block pinned to offsets 950..999. The number of COMMENTS the attacker authors is 50 - # either way — "50 vs 1000" compares an authored-comment count with a total-row count. + # page 2, at roughly 10x the price. State that price ONCE and in one unit, because + # two earlier drafts of this comment stated it two ways 45 lines apart. The FILTERED + # BLOCK is 50 rows in both cases — that is the part which must be inline review + # comments. What grew is the timeline the attacker has to build around it: ~100 rows + # before (a non-empty page 1, then a 50-row block at a 50-aligned offset) against more + # than 1000 now, with the block pinned to offsets 950..999. On their own PR the padding + # is theirs to supply too, so "50 comments either way" was wrong: it counted only the + # block and ignored the ~950 rows that now have to exist beneath it. # # This is NOT the same residual as the status walk's, and equating them would # understate it: there a non-empty page 20 is the only over-cap shape, because @@ -1117,8 +1142,12 @@ jobs: # READABLE pages does fail closed; one that fills it and hides the 20th behind a # filtered block does not. # - # No terminator on this endpoint can do better — that is measured, not assumed. See - # the measurements above: there is no total to page against, no row-type filter to + # No terminator AVAILABLE on this endpoint does better — measured, not assumed, and + # NOT a proof that none could exist. A different page SIZE, for instance, shifts the + # boundaries without closing anything: `?limit=49&page=21` reaches offsets 980..1028, + # past this walk's horizon of 999, and its page edges do not coincide with a 50-row + # walk's, so a block that exactly fills one walk's terminator page does not fill the + # other's. Strictly better, still not a terminator. See the measurements above: there is no total to page against, no row-type filter to # make the paged and serialized sets agree, and a `since` bound leaves the identical # ambiguity inside its window. Closing this needs a monotonic key that is not served # by a filtered, paged endpoint, and none is known (ersatztv#870 option 3). @@ -2439,7 +2468,10 @@ jobs: # WHAT THAT BUYS IS TRANSIENT INSTEAD OF PERMANENT, not "the green never stands" — which is # what this comment claimed until round-2 review. The `success` is live between its POST # and the repair below, including the timeline round trips in between, so branch protection - # or an already-scheduled auto-merge can observe it. Closing THAT window needs a + # or an already-scheduled auto-merge can observe it. THAT WINDOW GREW WITH #870 and the + # sentence is dated accordingly: the walk in between is now 20 requests rather than ~2, + # about +2.3s on a healthy run at the measured 0.13s per request, and bounded by the + # pessimum above rather than by anything tighter. Closing THAT window needs a # compare-and-set or serialization the API does not offer; what is removed is the case # where no event remained to correct it. # diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 1ca41f8ad..a1404815b 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -1698,8 +1698,9 @@ both ends, which is how the forged exemption was obtained in the first place. Ab a PR, because every retarget fires `edited` — the event that makes one run abstain has already queued its successor. -If the count can't be established (unreadable timeline, paging that never reached a validated empty -page), only the exemption `success` is withheld; `pending` still posts, since it blocks the merge +If the count can't be established (unreadable timeline, an empty FIRST page, or paging that reached +the 20-page cap on a NON-empty one — since #870 an empty page before the cap no longer ends the walk), +only the exemption `success` is withheld; `pending` still posts, since it blocks the merge immediately and withholding it would strand ordinary PRs whenever the timeline is unreadable. That is the right trade but not a free one — a generic `pending` still masks a rejection landing in its own write window. What made that durable is gone: since ersatztv#849 the post-write check runs after @@ -2031,8 +2032,9 @@ page it read came back empty — the same rule the status walk above states for moves the inference to the cap rather than removing it, and the residual is real**: pages 1-19 cover offsets 0..949, so a timeline of more than 1000 rows whose offsets 950..999 are all filtered still returns an empty page 20, and the walk publishes a too-low count as trusted. Both re-reads share the -blind spot, so this is the same defeat at about 10x the price — a timeline over 1000 rows rather -than ~100, the attacker authoring 50 inline comments either way — not a closed hole. It is NOT the status walk's over-cap residual: `/statuses/{sha}` does not +blind spot, so this is the same defeat at about 10x the price — the 50-row filtered block is the +same, but the timeline it must sit in grows from ~100 rows to over 1000, and on their own PR the +padding is the attacker's to supply — not a closed hole. It is NOT the status walk's over-cap residual: `/statuses/{sha}` does not filter after paging, so only here can a page-20 response be empty while later pages hold rows. An empty FIRST page still ends the walk untrusted: every real PR's first page carries events (spot-checked non-empty across #752/#753/#749/#739/#717; the counts are deliberately not recorded here because diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 4f34ae8a3..631ebdbbc 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -70,7 +70,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ci.toolchain-image-publish-is-a-dispatch` | A `push` trigger reachable from any ref other than `main` — that is `branches:` AND `tags:`, judged by the ref class it admits and never by which keyword is present — executes ref-supplied YAML, because Gitea resolves a `push` workflow's definition from the pushed ref — so `ci-image.yml` is `branches: [main]`, and publishing a toolchain image from a feature branch is a deliberate `workflow_dispatch` on that branch rather than a side effect of pushing. Be precise about what a `branches:` filter buys: it is loaded from the pushed ref like the rest of the file, so a branch that deletes it re-enables the route — this removes the DRIVE-BY case, and is not a boundary against a writer who intends to run their own YAML. The self-referencing trigger path `.gitea/workflows/ci-image.yml` came out of BOTH its own `paths:` and `ci-image-pin`'s `expected` in the same change — a DECIDED tradeoff, not a necessity: keeping it is workable via the branch dispatch, but prices every edit to that file, comments included, at a ~2GB publish plus a five-pin bump, redone after every rebase. The cost is stated, not assumed away — a change to HOW the image is built that lives only in `ci-image.yml` no longer republishes, and the ONLY remedy is to make it alongside a `docker/ci/**` edit: publishing after the merge and then pinning cannot work, because `expected` is the last `docker/ci` commit and would reject that pin. This closes the push route INTO THIS FILE, not the class: `docker-build.yml` remains reachable from an arbitrary ref by a `v*` tag push and by `pull_request`, and four workflows carry an unrestricted `workflow_dispatch`. The DISPATCH THIRD of that is settled — #853 probed it and ACCEPTED it (`ci.workflow-dispatch-ref-unrestricted`): no ref restriction exists at Gitea 1.27.1, and restricting it would close nothing anyway, because the head-resolved `pull_request:` route runs attacker-authored YAML that reaches every secret in the store. The `v*` tag push and `pull_request:` rows are NOT settled and remain open in #885. Do not re-derive any of this. | 2026-08-27 | [link](records/ci/toolchain-image-publish-is-a-dispatch.md) | | `ci.ui-e2e-harness` | The UI-interactive E2E flows run as headless Playwright specs (`web/e2e/*.spec.ts`, driven by `scripts/e2e-ui.sh`) in a **second step of the existing advisory `functional-e2e` job**, never their own job; the browser is `chromium-headless-shell` **baked into the CI toolchain image** (`docker/ci/Dockerfile`, `PLAYWRIGHT_VERSION` kept equal to `web/package.json`'s EXACT `@playwright/test` pin), never installed per run; specs are `serial` with `retries: 0` and assert only contracts the curl harness structurally cannot reach. | 2026-07-25 | [link](records/ci/ui-e2e-harness.md) | | `ci.verdict-unverified-write-sentinel` | The `review-verdict/h10` job runs its post-write race check after EVERY status write, not only an exemption `success`, and where no mark exists to run it against, the write ITSELF becomes the sentinel — so every status this job writes is either verified against the history or marked as unverifiable: a GENERIC `pending` masks a rejection landing in its own write window exactly as a `success` does, and because it carries no marker a later run re-derives it into an exemption with the human row now below THAT run's high-water mark — the damage arrives one event later, not never. Where a write cannot be checked at all the job writes a SECOND sentinel, `UNVERIFIED_DESC`, distinct from the repair sentinel and never interchangeable with it: the repair sentinel ASSERTS that a verdict existed and was buried, which only the arm that actually COUNTED such a row may claim, while every "could not check" arm — an unreadable or over-cap history, an impossible empty history, an unusable count — states only what it established. Both are STICKY (the classification refuses to grant an exemption over either, and re-writes its own description verbatim, so each is a FIXED POINT a later run cannot re-derive into `success`); only the unverified one is RECONCILABLE. Reconciliation is what bounds the stall that got the #742 attempt withdrawn: a later run pages `/statuses/{sha}` IN FULL and, ONLY IF that history contains the sentinel's own row, either finds a `Review-verdict:` row underneath the sentinel — an established fact, so it UPGRADES to the repair sentinel, clearable only by a human — or finds none, resolving the uncertainty and clearing it so the PR is classified normally. The reconciliation is sound because the two endpoints differ: a verdict this job masked is invisible on the COMBINED endpoint (latest row per context, which is the sentinel) and still present in the per-POST history. It is deliberately BROADER than the inheritance test — no `H10_REVIEWERS` membership, no `(base: …)` match — because over-counting upgrades to a stall a human clears in one command while MISSING one re-exempts a head carrying a buried rejection. ONLY a COMPLETE walk CONTAINING THE CURRENT SENTINEL ROW — matched by the `id` the combined read reported, not merely by a row with the same description — may clear it. Description alone is satisfied by an OLDER identical sentinel, which is exactly what a fixed point produces: with an earlier sentinel and a buried verdict below the current one, a read carrying only the earlier row satisfies the test, the sentinel clears, and the verdict ends up below the fresh mark. Where the server omits `id` there is nothing to match on and the check degrades to the description, which is the pre-existing behaviour rather than a new hole. The witness is not optional decoration: `ex_unverified` means the combined endpoint just returned that row, and `/statuses/{sha}` keeps one row per POST, so a complete history WITHOUT it — an empty one included — contradicts a write that demonstrably happened. `page_statuses` accepts an empty page 1 as complete (correct for a first-run mark), which is what made that shape reachable, and clearing on it exempted a head whose sentinel may have been sitting on a rejection. This is NOT the withdrawn currency witness: that asked whether ANY row sat above the mark, which an unrelated row satisfied, and it made one anomaly permanent — this asks for a SPECIFIC row already known to exist. Failing it costs one run in the transient case; it is NOT bounded in general, because a history past the 20-page cap can never be walked completely, so such a head needs a human verdict and no later run will clear it. When NO high-water mark can be established the write is withheld BEFORE the POST rather than posted and repaired, since the defect is known in advance and publishing a green to take it back opens a window branch protection — and an already-scheduled auto-merge — can see. EVERY re-derivable state is downgraded there, not only `success`: an earlier draft restricted it to the exemption on the grounds that a sticky generic `pending` "withholds nothing, since an unreviewed PR is blocked already", which analysed the wrong PR — the damaging case is one that IS exemptible and got the generic `pending` only from a transient enumeration failure, whose unmarked description the next run re-derives into the exemption with the human row below its own mark. `$REPAIR_DESC` is the one exemption, being the stronger fact and not re-derivable. SEPARATELY, the retarget count is re-taken AFTER the POST on the exemption path, which closes the PERMANENT forged green `ci.verdict-write-retarget-fence` recorded as its residual: a retarget landing after the final pre-write count leaves a stale `success` with the `edited` event already consumed by a successor that short-circuited, so nothing remains to reclassify. Re-counting afterwards means a retarget later than the re-check necessarily queues a successor that STARTS after the stale `success` exists, and a machine-written `success` is re-derived rather than inherited. The RETARGET axis only: a push after the POST moves the head, so the status no longer gates that PR, while a retarget changes the effective diff with the sha unchanged. An untrusted post-POST count repairs too — reaching that point with `success` means both earlier counts were trusted, so a third unreadable read is a fresh failure and "I cannot tell whether the base moved" must not resolve to leaving a green. FINALLY, every path that cannot establish what the head carries REPLACES the unknown state instead of merely declining to write — the combined read (retried once first), all four page-2 completeness refusals, and the fence branch that cannot trust its retarget count while holding a derived `success`. That last one is reached only AFTER the classification declined to inherit the row the head carries, so posting nothing left the declined row current; its message used to say the context "stays absent", which is true only of a head that had none. The two OBSERVED-mutation arms take the same decision for the same reason, scoped to a head that actually carries a declined row: they still refuse to post their CLASSIFICATION, which was computed against a base or head the PR may no longer have, but a row this run declined must not stay authoritative for the whole window until a successor finishes — and for a PR's FIRST push no successor is queued at all. Every element and every consumed field of the combined response is TYPE-CHECKED before extraction, and a schema failure routes to the same replacement rather than dying: `.statuses` being an array was checked and its ELEMENTS were not, so one scalar made `select(.context == $c)` hard-error and `set -e` took the step down before any path could mark the head. The path-predicate failure replaces too. And the write helper RETURNS a status: its first version ended the failure arm with a successful `echo`, so it reported 0 after both POSTs failed and the fence caller exited 0 as though the head had been marked. Declining protects a real verdict and leaves a FORGED one — an off-list `success` is the row #742 exists to revoke, revocation happens by re-deriving it, and an unreadable read is the one thing that stops it while the job goes red on a status branch protection does not read. Nothing is destroyed: the per-POST history keeps the masked verdict and the next run's reconciliation upgrades to the repair sentinel, telling the reviewer to re-post rather than silently un-approving them. The page-2 refusals were briefly excluded on the reasoning that the probe fires when NO row for this context was on page 1, so there is no green of any provenance to leave standing — self-contradictory, since the ONLY reason page 2 is read is that the row MAY be beyond page 1. WRITING THE SENTINEL AND FAILING THE JOB ARE SEPARATE DECISIONS: the read refusals were already non-zero exits on `main` and stay red, while the fence branch exited 0 there and still does, because an unreadable timeline is an ordinary hiccup and reddening every one of them is noise this file elsewhere refuses to add. The repair also has a FLOOR — it may never write a description weaker than the one this run decided, or a transient post-write read rewrites a correct `$REPAIR_DESC` carry-forward with the machine-clearable sentinel — and it is skipped entirely when it would rewrite what is already there. | 2026-08-29 | [link](records/ci/verdict-unverified-write-sentinel.md) | -| `ci.verdict-write-retarget-fence` | The `review-verdict/h10` job counts BOTH `change_target_branch` AND `pull_push` events on the PR's issue timeline at run start and again immediately before its POST, and does not post its CLASSIFICATION if EITHER count moved. Since #849 it does not merely abstain either: when the head carries a row this run did not inherit — and that row is neither the repair sentinel nor an allow-listed reviewer's verdict — the arm REPLACES it with the unverified-write sentinel, because abstention is a handoff only when there is nothing to hand off (`ci.verdict-unverified-write-sentinel`). The COUNT is the key on both axes because the underlying VALUE is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption, and a force-push `H1 -> H2 -> H1` leaves `.head.sha` equal at both ends while the middle pages of `scripts/pr-changed-files.sh`'s enumeration came from `H2` (#803/#664) — while an event count is monotonic and cannot alias. ONE walk certifies BOTH counts, so an unreadable page abandons both; the two tallies are separate so the diagnostic names the axis that actually moved. Every push is counted and `is_force_push` is deliberately NOT read: an ordinary push also invalidates a mid-flight enumeration, and an `H1 -> H2 -> H1` restoration can have its second push non-forced when `H1` is an ancestor. The head fence does NOT abstain on its own triggering push — established FROM THE v1.27.1 SOURCE, since that would be a permanent stall rather than a fence: the push comment is created BEFORE the synchronize notification is emitted, so a run always sees its own causative event, and retries add no new event. The 26-102s margin measured across 20 triggered pairs on PRs #802/#834/#761 corroborates it and is a lower bound (the job runs a checkout first; 69s end to end on run 2385) — it is NOT the basis of the claim, which an earlier draft said it was. This NARROWS the residual rather than resolving it, and what CLOSED the permanent case is a separate mechanism recorded at `ci.verdict-unverified-write-sentinel`: the retarget count is re-taken AFTER the POST, so a retarget between the final pre-write count and the POST — which used to leave a PERMANENT forged green, the successor having consumed the `edited` event and exited before the stale run posted last — is now caught by the writing run itself, and one landing after the re-check necessarily queues a successor that starts with the stale `success` already visible and re-derivable (corrected 2026-08-27, closed 2026-08-29, #849). Abstaining is a handoff, not a stall, and that is the property the design rests on — for a run that ABSTAINS; it says nothing about one that already passed its final count and then posts (see the residual, corrected #849): every retarget fires `edited`, which is in this workflow's `types:`, so the event that makes a run abstain has already queued a successor whose window opens after it; the induction terminates when retargeting stops — but ONLY over runs that ABSTAIN. A run that already passed its final count is outside it: it writes whenever it gets there, so the run that writes LAST is not necessarily the one that classified last. That was the permanent residual until #849 gave the writing run a post-POST re-count of its own (`ci.verdict-unverified-write-sentinel`), which puts such a run back inside the induction — it either withdraws its own stale write or leaves a green a guaranteed successor re-derives. `updated_at` was REJECTED as the key because it also moves for comments and labels, which fire none of this workflow's `types:` — a run could abstain with no successor coming, which is a real stall. The count is trusted only when the walk read EVERY page up to its 20-page cap and the LAST page came back empty — an empty page BEFORE the cap is SKIPPED rather than read as exhaustion, since Gitea pages this endpoint before it filters and a fully-filtered page is byte-identical to the end of the list (#870) — which NARROWS that defeat about 10x rather than closing it — the timeline needed ~100 rows before and needs over 1000 now, the attacker authoring 50 inline comments either way — the page-20 terminator still being trusted for the same unprovable reason; an untrusted count (unreadable page, non-array body, non-numeric length, empty FIRST page, a cap reached on a non-empty page) blocks the exemption `success` ONLY and still lets `pending` through, because `pending` blocks the merge immediately while withholding it would strand ordinary PRs whenever the timeline is unreadable — the right trade, but NOT a free one ("for no safety gain" retracted 2026-08-27): a GENERIC `pending` masks a rejection landing in its own write window just as a `success` does. What made that DURABLE — post-write verification skipping it, so a later run re-derived it into an exemption `success` — is closed since 2026-08-29: the check now runs after EVERY write (`ci.verdict-unverified-write-sentinel`), so such a write is repaired to the sticky repair sentinel and cannot be re-derived. The masking itself is still a cost, and a write the mark cannot cover is still unverified; that is what the second sentinel is for. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting ANY status — every write since #849, not only an exemption `success` — the job re-reads `/statuses/{sha}` IN FULL (PAGED since #763) and, if a human `Review-verdict:` row appeared with an id ABOVE a high-water mark taken just before the POST, overwrites its own status with `pending` and logs an error. The repair is `pending`, NEVER a copy of the human's state, since re-posting their `failure` under the machine credential would attribute a human verdict to the job; its description is a SENTINEL that the classification refuses to grant an exemption over AND re-writes verbatim on every later run, so the block is a FIXED POINT rather than decaying — writing the generic `pending` description there instead erases the marker and the exemption simply returns one event later. The mark is captured BEFORE the last-moment re-read, not merely before the POST — a later mark leaves a multi-round-trip blind gap in which a verdict is neither seen by the re-read nor repaired afterwards. The id comparison is load-bearing: a mere presence test would fire forever on a base-mismatched verdict that `read_existing_verdict` deliberately declines to honour, deadlocking that PR's exemption permanently. Finally, a run whose last-moment re-read finds a sentinel it did not see at its FIRST read ABSTAINS instead of posting: that can only mean an overlapping run repaired a raced verdict mid-flight, and this run's `success` — frozen at classification time, with the human row below its own mark, so neither the fence nor the post-write check would catch it — would otherwise bury the rejection. That is the one path in this design that failed toward SUCCESS rather than `pending`. The post-write check counts TWO row shapes above the mark, not one — a human `Review-verdict:` row AND a machine sentinel — because with two overlapping runs the human row can sit BELOW the second run's mark while the first masks it and only then writes the sentinel, leaving the second to post its own `success` on top; counting the sentinel converges both runs on the fixed point instead. BOTH `/statuses/{sha}` reads PAGE to a validated EMPTY page since #763 — `[]` on this endpoint, a THIRD terminator shape distinct from the timeline's bare `null` and the combined endpoint's `{"statuses": null}`, so each terminator is MEASURED per endpoint — never terminating on a SHORT page, retrying each page once, under a 20-page cap that validates at most 950 rows (the 20th request must be the empty terminator); correctness does not depend on the measured cap of 50, because any page size pages correctly. WHAT THE PAGING BUYS IS NOT WHAT #763 CLAIMED: under the DESC default page 1 already held the true maximum id AND every row newer than the mark, so a single-page read missed a raced verdict only if more than 50 rows were created INSIDE the write window — not merely on a head over 50 rows. What removed #761's stall is retiring the probe, not the walk; the walk's value is that the gate's one fail-toward-SUCCESS path no longer depends on an UNDOCUMENTED ordering the server honours only coarsely (page 1 came back `114,112,113,111,110`). This RETIRED #751's conservative page-2 probe, which treated "there are rows I did not read" as "assume raced" and so repaired every head that outgrew one page: it fired on Renovate PR #761, reporting a human verdict as overwritten when none existed and then, the sentinel being sticky, refusing to re-exempt that head on every later run. The two directions are NOT symmetric. POST-WRITE, uncertainty fails CLOSED — an unreadable history, an over-cap history, or a count that is not a number all repair to `pending`; previously an unreadable history warned and left the exemption green while the page-2 probe repaired on the same uncertainty, one check disagreeing with itself. PRE-WRITE, a PARTIAL list still yields a mark, because the mark gates the post-write check entirely and refusing one SKIPS that check, which is itself the fail-open; this rests on the DESC DEFAULT — the newest row, carrying the maximum id, is on page 1, so a walk that fails later still saw it — while a VALIDATED empty history is NOT abandoned (it yields a mark of 0, correct for a first run, since every later row is newer) — what abandons the mark is a read that both FAILED and returned nothing — which since 2026-08-29 WITHHOLDS the exemption before the POST and marks the head with the reconcilable sentinel, rather than posting a green nothing can check (#849) — and a NON-EMPTY history carrying no numeric id is reported unusable rather than collapsed to 0. BOTH id comparisons are NUMERIC-ONLY: jq orders strings above every number, so one `"id": "99999"` inflates the mark until nothing looks newer, and `.id > $since` reads any string id as newer than any mark — making a PRE-EXISTING base-mismatched verdict look raced on every run, a permanent per-sha stall (the twin was live on `main`). An EMPTY post-write history is REJECTED: the walk terminates on an empty page, which is correct before the write and impossible after it (one row per POST), and a well-formed "no statuses exist" is not retried — so accepting it would conclude `raced=0` from a list that cannot be real, silently. That is NOT the withdrawn currency witness, which asked whether ANY row sat above the mark and was satisfied by an unrelated newer row; this asks only whether the list is EMPTY, a state no unrelated row can produce. The `::error::` now names its own cause, of which there are THREE — a verdict actually FOUND, a read that could not be COMPLETED, and a read that completed but returned an IMPOSSIBLE answer (the third is not a variety of the second) — while WHICH sentinel description is written is itself part of the answer since #849: only an arm that actually COUNTED a verdict row may write the repair sentinel, and every "could not check" arm writes the reconcilable one (`ci.verdict-unverified-write-sentinel`). Both are fixed points the classification recognises. `.creator` is TYPE-TESTED before it is indexed: `.creator != null and .creator.login` hard-errors on a non-object creator, jq exits 5, and under `set -e` that took the step down AFTER the exemption was posted and BEFORE the repair — a forged green reported as an infrastructure error. | 2026-08-03 | [link](records/ci/verdict-write-retarget-fence.md) | +| `ci.verdict-write-retarget-fence` | The `review-verdict/h10` job counts BOTH `change_target_branch` AND `pull_push` events on the PR's issue timeline at run start and again immediately before its POST, and does not post its CLASSIFICATION if EITHER count moved. Since #849 it does not merely abstain either: when the head carries a row this run did not inherit — and that row is neither the repair sentinel nor an allow-listed reviewer's verdict — the arm REPLACES it with the unverified-write sentinel, because abstention is a handoff only when there is nothing to hand off (`ci.verdict-unverified-write-sentinel`). The COUNT is the key on both axes because the underlying VALUE is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption, and a force-push `H1 -> H2 -> H1` leaves `.head.sha` equal at both ends while the middle pages of `scripts/pr-changed-files.sh`'s enumeration came from `H2` (#803/#664) — while an event count is monotonic and cannot alias. ONE walk certifies BOTH counts, so an unreadable page abandons both; the two tallies are separate so the diagnostic names the axis that actually moved. Every push is counted and `is_force_push` is deliberately NOT read: an ordinary push also invalidates a mid-flight enumeration, and an `H1 -> H2 -> H1` restoration can have its second push non-forced when `H1` is an ancestor. The head fence does NOT abstain on its own triggering push — established FROM THE v1.27.1 SOURCE, since that would be a permanent stall rather than a fence: the push comment is created BEFORE the synchronize notification is emitted, so a run always sees its own causative event, and retries add no new event. The 26-102s margin measured across 20 triggered pairs on PRs #802/#834/#761 corroborates it and is a lower bound (the job runs a checkout first; 69s end to end on run 2385) — it is NOT the basis of the claim, which an earlier draft said it was. This NARROWS the residual rather than resolving it, and what CLOSED the permanent case is a separate mechanism recorded at `ci.verdict-unverified-write-sentinel`: the retarget count is re-taken AFTER the POST, so a retarget between the final pre-write count and the POST — which used to leave a PERMANENT forged green, the successor having consumed the `edited` event and exited before the stale run posted last — is now caught by the writing run itself, and one landing after the re-check necessarily queues a successor that starts with the stale `success` already visible and re-derivable (corrected 2026-08-27, closed 2026-08-29, #849). Abstaining is a handoff, not a stall, and that is the property the design rests on — for a run that ABSTAINS; it says nothing about one that already passed its final count and then posts (see the residual, corrected #849): every retarget fires `edited`, which is in this workflow's `types:`, so the event that makes a run abstain has already queued a successor whose window opens after it; the induction terminates when retargeting stops — but ONLY over runs that ABSTAIN. A run that already passed its final count is outside it: it writes whenever it gets there, so the run that writes LAST is not necessarily the one that classified last. That was the permanent residual until #849 gave the writing run a post-POST re-count of its own (`ci.verdict-unverified-write-sentinel`), which puts such a run back inside the induction — it either withdraws its own stale write or leaves a green a guaranteed successor re-derives. `updated_at` was REJECTED as the key because it also moves for comments and labels, which fire none of this workflow's `types:` — a run could abstain with no successor coming, which is a real stall. The count is trusted only when the walk read EVERY page up to its 20-page cap and the LAST page came back empty — an empty page BEFORE the cap is SKIPPED rather than read as exhaustion, since Gitea pages this endpoint before it filters and a fully-filtered page is byte-identical to the end of the list (#870) — which NARROWS that defeat about 10x rather than closing it — the 50-row filtered block is unchanged, but the timeline it must sit in grows from ~100 rows to over 1000 — the page-20 terminator still being trusted for the same unprovable reason; an untrusted count (unreadable page, non-array body, non-numeric length, empty FIRST page, a cap reached on a non-empty page) blocks the exemption `success` ONLY and still lets `pending` through, because `pending` blocks the merge immediately while withholding it would strand ordinary PRs whenever the timeline is unreadable — the right trade, but NOT a free one ("for no safety gain" retracted 2026-08-27): a GENERIC `pending` masks a rejection landing in its own write window just as a `success` does. What made that DURABLE — post-write verification skipping it, so a later run re-derived it into an exemption `success` — is closed since 2026-08-29: the check now runs after EVERY write (`ci.verdict-unverified-write-sentinel`), so such a write is repaired to the sticky repair sentinel and cannot be re-derived. The masking itself is still a cost, and a write the mark cannot cover is still unverified; that is what the second sentinel is for. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting ANY status — every write since #849, not only an exemption `success` — the job re-reads `/statuses/{sha}` IN FULL (PAGED since #763) and, if a human `Review-verdict:` row appeared with an id ABOVE a high-water mark taken just before the POST, overwrites its own status with `pending` and logs an error. The repair is `pending`, NEVER a copy of the human's state, since re-posting their `failure` under the machine credential would attribute a human verdict to the job; its description is a SENTINEL that the classification refuses to grant an exemption over AND re-writes verbatim on every later run, so the block is a FIXED POINT rather than decaying — writing the generic `pending` description there instead erases the marker and the exemption simply returns one event later. The mark is captured BEFORE the last-moment re-read, not merely before the POST — a later mark leaves a multi-round-trip blind gap in which a verdict is neither seen by the re-read nor repaired afterwards. The id comparison is load-bearing: a mere presence test would fire forever on a base-mismatched verdict that `read_existing_verdict` deliberately declines to honour, deadlocking that PR's exemption permanently. Finally, a run whose last-moment re-read finds a sentinel it did not see at its FIRST read ABSTAINS instead of posting: that can only mean an overlapping run repaired a raced verdict mid-flight, and this run's `success` — frozen at classification time, with the human row below its own mark, so neither the fence nor the post-write check would catch it — would otherwise bury the rejection. That is the one path in this design that failed toward SUCCESS rather than `pending`. The post-write check counts TWO row shapes above the mark, not one — a human `Review-verdict:` row AND a machine sentinel — because with two overlapping runs the human row can sit BELOW the second run's mark while the first masks it and only then writes the sentinel, leaving the second to post its own `success` on top; counting the sentinel converges both runs on the fixed point instead. BOTH `/statuses/{sha}` reads PAGE to a validated EMPTY page since #763 — `[]` on this endpoint, a THIRD terminator shape distinct from the timeline's bare `null` and the combined endpoint's `{"statuses": null}`, so each terminator is MEASURED per endpoint — never terminating on a SHORT page, retrying each page once, under a 20-page cap that validates at most 950 rows (the 20th request must be the empty terminator); correctness does not depend on the measured cap of 50, because any page size pages correctly. WHAT THE PAGING BUYS IS NOT WHAT #763 CLAIMED: under the DESC default page 1 already held the true maximum id AND every row newer than the mark, so a single-page read missed a raced verdict only if more than 50 rows were created INSIDE the write window — not merely on a head over 50 rows. What removed #761's stall is retiring the probe, not the walk; the walk's value is that the gate's one fail-toward-SUCCESS path no longer depends on an UNDOCUMENTED ordering the server honours only coarsely (page 1 came back `114,112,113,111,110`). This RETIRED #751's conservative page-2 probe, which treated "there are rows I did not read" as "assume raced" and so repaired every head that outgrew one page: it fired on Renovate PR #761, reporting a human verdict as overwritten when none existed and then, the sentinel being sticky, refusing to re-exempt that head on every later run. The two directions are NOT symmetric. POST-WRITE, uncertainty fails CLOSED — an unreadable history, an over-cap history, or a count that is not a number all repair to `pending`; previously an unreadable history warned and left the exemption green while the page-2 probe repaired on the same uncertainty, one check disagreeing with itself. PRE-WRITE, a PARTIAL list still yields a mark, because the mark gates the post-write check entirely and refusing one SKIPS that check, which is itself the fail-open; this rests on the DESC DEFAULT — the newest row, carrying the maximum id, is on page 1, so a walk that fails later still saw it — while a VALIDATED empty history is NOT abandoned (it yields a mark of 0, correct for a first run, since every later row is newer) — what abandons the mark is a read that both FAILED and returned nothing — which since 2026-08-29 WITHHOLDS the exemption before the POST and marks the head with the reconcilable sentinel, rather than posting a green nothing can check (#849) — and a NON-EMPTY history carrying no numeric id is reported unusable rather than collapsed to 0. BOTH id comparisons are NUMERIC-ONLY: jq orders strings above every number, so one `"id": "99999"` inflates the mark until nothing looks newer, and `.id > $since` reads any string id as newer than any mark — making a PRE-EXISTING base-mismatched verdict look raced on every run, a permanent per-sha stall (the twin was live on `main`). An EMPTY post-write history is REJECTED: the walk terminates on an empty page, which is correct before the write and impossible after it (one row per POST), and a well-formed "no statuses exist" is not retried — so accepting it would conclude `raced=0` from a list that cannot be real, silently. That is NOT the withdrawn currency witness, which asked whether ANY row sat above the mark and was satisfied by an unrelated newer row; this asks only whether the list is EMPTY, a state no unrelated row can produce. The `::error::` now names its own cause, of which there are THREE — a verdict actually FOUND, a read that could not be COMPLETED, and a read that completed but returned an IMPOSSIBLE answer (the third is not a variety of the second) — while WHICH sentinel description is written is itself part of the answer since #849: only an arm that actually COUNTED a verdict row may write the repair sentinel, and every "could not check" arm writes the reconcilable one (`ci.verdict-unverified-write-sentinel`). Both are fixed points the classification recognises. `.creator` is TYPE-TESTED before it is indexed: `.creator != null and .creator.login` hard-errors on a non-object creator, jq exits 5, and under `set -e` that took the step down AFTER the exemption was posted and BEFORE the repair — a forged green reported as an infrastructure error. | 2026-08-03 | [link](records/ci/verdict-write-retarget-fence.md) | | `ci.verify-locally-ci-confirms` | Treat the local build/verify/review pass as the decision point and CI as confirmation — don't idle waiting on a run you have no reason to doubt. | 2026-07-21 | [link](records/ci/verify-locally-ci-confirms.md) | | `ci.web-test-per-test-timeouts` | Give heavy-render web tests an explicit per-test vitest timeout (e.g. 15s); never raise the global default to fix one slow test. | 2026-07-21 | [link](records/ci/web-test-per-test-timeouts.md) | | `ci.workflow-dispatch-ref-unrestricted` | Gitea 1.27.1 offers NO mechanism to restrict `workflow_dispatch` by ref, and has no protected-environment concept at all — PROBED across the REST API, the loaded config and the CLI, not assumed (the WEB UI was not swept; the body says why that is acceptable here and where it would matter). The dispatch body schema `CreateActionWorkflowDispatch` makes `ref` a required free-form string with no allow-list or pattern field; zero of the 308 documented API paths contain "environment", and Actions secrets exist only at org/repo/user scope with no per-ref or per-environment gate; `/api/v1/settings/actions` 404s; the config file the running server actually loads (`/etc/gitea/app.ini`, named by its own `--config`) sets only `ENABLED` and `DEFAULT_ACTIONS_URL` under `[actions]`; and the `gitea` CLI exposes exactly ONE Actions subcommand, `gitea actions generate-runner-token`, which registers a runner and restricts nothing. Treat the VERSION, not the `stale-after` date, as the real trigger to re-probe: an upgrade past 1.27.1 invalidates every capability claim here the day it lands, months before the date fires. The four unrestricted dispatches (`ci-image.yml`, `docker-build.yml`, `dependency-scan.yml`, `renovate.yml`) are therefore ACCEPTED — but the operative reason is NOT "repository write access is the boundary", which is the argument to avoid because it is unfalsifiable and it hides the real route. The operative reason is that **dispatch is not the cheapest route to ANY of it**: `docker-build.yml` triggers on `pull_request:`, which Gitea resolves from the PR HEAD, so that route executes ATTACKER-AUTHORED YAML — and such YAML can name any secret in the repo store, not merely the ones the committed workflows happen to reference (`ci.gate-trigger-base-resolved`, verbatim: "any PR-added workflow can reference `RENOVATE_TOKEN`, a `write:repository` bot PAT in the same store"). Label that step honestly: it is INFERRED from the repo-scoped secret model plus that record, NOT measured here, because the measurement would print a live credential into a run log. That generalizing step is what makes the argument cover all four rather than just the registry pair: `renovate.yml`'s `RENOVATE_TOKEN`/`GH_COM_TOKEN` are reachable from a PR without dispatching `renovate.yml` at all, and `dependency-scan.yml` references no `secrets.` whatever — which corrects #853's own table row for it. On the registry credential as it stands, SIX jobs in `docker-build.yml` hold `REGISTRY_PASSWORD` and run on the PR route (`toolchain-preflight`, `test`, `migrations`, `functional-e2e`, `api-docs`, `format`), two of them — `test` and `migrations` — branch-protection required contexts per `.gitea/required-status-contexts.json`; carry that as the INVARIANT "every job on the PR route that NAMES `secrets.REGISTRY_PASSWORD`", never as the six-name list, because a remediation scoped to a stale list misses whatever lands next. Resist the tempting "every `container:` job" — `toolchain-preflight` is deliberately container-free and takes the credential through `ETV_REGISTRY_AUTH`, so that predicate names five of the six and reproduces on day one the exact staleness it was written to prevent. "Deliberate act" throughout carries `ci.toolchain-image-publish-is-a-dispatch`'s sense — an act OUTSIDE the ordinary contribution flow, not a raw step count: opening a PR costs zero such acts and a dispatch costs one. Restricting dispatch would therefore close the more visible route and change nothing. The residuals worth tracking are the PR route AND the `v*` tag push — a single act, explicitly outside `release.main-direct-push-disabled` — both in #885, not dispatch. | 2026-08-30 | [link](records/ci/workflow-dispatch-ref-unrestricted.md) | diff --git a/docs/decisions/records/ci/verdict-write-retarget-fence.md b/docs/decisions/records/ci/verdict-write-retarget-fence.md index 66a5be82d..12f7bd11d 100644 --- a/docs/decisions/records/ci/verdict-write-retarget-fence.md +++ b/docs/decisions/records/ci/verdict-write-retarget-fence.md @@ -5,7 +5,7 @@ status: active since: '2026-08-03' supersedes: none superseded-by: none -rule: 'The `review-verdict/h10` job counts BOTH `change_target_branch` AND `pull_push` events on the PR''s issue timeline at run start and again immediately before its POST, and does not post its CLASSIFICATION if EITHER count moved. Since #849 it does not merely abstain either: when the head carries a row this run did not inherit — and that row is neither the repair sentinel nor an allow-listed reviewer''s verdict — the arm REPLACES it with the unverified-write sentinel, because abstention is a handoff only when there is nothing to hand off (`ci.verdict-unverified-write-sentinel`). The COUNT is the key on both axes because the underlying VALUE is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption, and a force-push `H1 -> H2 -> H1` leaves `.head.sha` equal at both ends while the middle pages of `scripts/pr-changed-files.sh`''s enumeration came from `H2` (#803/#664) — while an event count is monotonic and cannot alias. ONE walk certifies BOTH counts, so an unreadable page abandons both; the two tallies are separate so the diagnostic names the axis that actually moved. Every push is counted and `is_force_push` is deliberately NOT read: an ordinary push also invalidates a mid-flight enumeration, and an `H1 -> H2 -> H1` restoration can have its second push non-forced when `H1` is an ancestor. The head fence does NOT abstain on its own triggering push — established FROM THE v1.27.1 SOURCE, since that would be a permanent stall rather than a fence: the push comment is created BEFORE the synchronize notification is emitted, so a run always sees its own causative event, and retries add no new event. The 26-102s margin measured across 20 triggered pairs on PRs #802/#834/#761 corroborates it and is a lower bound (the job runs a checkout first; 69s end to end on run 2385) — it is NOT the basis of the claim, which an earlier draft said it was. This NARROWS the residual rather than resolving it, and what CLOSED the permanent case is a separate mechanism recorded at `ci.verdict-unverified-write-sentinel`: the retarget count is re-taken AFTER the POST, so a retarget between the final pre-write count and the POST — which used to leave a PERMANENT forged green, the successor having consumed the `edited` event and exited before the stale run posted last — is now caught by the writing run itself, and one landing after the re-check necessarily queues a successor that starts with the stale `success` already visible and re-derivable (corrected 2026-08-27, closed 2026-08-29, #849). Abstaining is a handoff, not a stall, and that is the property the design rests on — for a run that ABSTAINS; it says nothing about one that already passed its final count and then posts (see the residual, corrected #849): every retarget fires `edited`, which is in this workflow''s `types:`, so the event that makes a run abstain has already queued a successor whose window opens after it; the induction terminates when retargeting stops — but ONLY over runs that ABSTAIN. A run that already passed its final count is outside it: it writes whenever it gets there, so the run that writes LAST is not necessarily the one that classified last. That was the permanent residual until #849 gave the writing run a post-POST re-count of its own (`ci.verdict-unverified-write-sentinel`), which puts such a run back inside the induction — it either withdraws its own stale write or leaves a green a guaranteed successor re-derives. `updated_at` was REJECTED as the key because it also moves for comments and labels, which fire none of this workflow''s `types:` — a run could abstain with no successor coming, which is a real stall. The count is trusted only when the walk read EVERY page up to its 20-page cap and the LAST page came back empty — an empty page BEFORE the cap is SKIPPED rather than read as exhaustion, since Gitea pages this endpoint before it filters and a fully-filtered page is byte-identical to the end of the list (#870) — which NARROWS that defeat about 10x rather than closing it — the timeline needed ~100 rows before and needs over 1000 now, the attacker authoring 50 inline comments either way — the page-20 terminator still being trusted for the same unprovable reason; an untrusted count (unreadable page, non-array body, non-numeric length, empty FIRST page, a cap reached on a non-empty page) blocks the exemption `success` ONLY and still lets `pending` through, because `pending` blocks the merge immediately while withholding it would strand ordinary PRs whenever the timeline is unreadable — the right trade, but NOT a free one ("for no safety gain" retracted 2026-08-27): a GENERIC `pending` masks a rejection landing in its own write window just as a `success` does. What made that DURABLE — post-write verification skipping it, so a later run re-derived it into an exemption `success` — is closed since 2026-08-29: the check now runs after EVERY write (`ci.verdict-unverified-write-sentinel`), so such a write is repaired to the sticky repair sentinel and cannot be re-derived. The masking itself is still a cost, and a write the mark cannot cover is still unverified; that is what the second sentinel is for. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting ANY status — every write since #849, not only an exemption `success` — the job re-reads `/statuses/{sha}` IN FULL (PAGED since #763) and, if a human `Review-verdict:` row appeared with an id ABOVE a high-water mark taken just before the POST, overwrites its own status with `pending` and logs an error. The repair is `pending`, NEVER a copy of the human''s state, since re-posting their `failure` under the machine credential would attribute a human verdict to the job; its description is a SENTINEL that the classification refuses to grant an exemption over AND re-writes verbatim on every later run, so the block is a FIXED POINT rather than decaying — writing the generic `pending` description there instead erases the marker and the exemption simply returns one event later. The mark is captured BEFORE the last-moment re-read, not merely before the POST — a later mark leaves a multi-round-trip blind gap in which a verdict is neither seen by the re-read nor repaired afterwards. The id comparison is load-bearing: a mere presence test would fire forever on a base-mismatched verdict that `read_existing_verdict` deliberately declines to honour, deadlocking that PR''s exemption permanently. Finally, a run whose last-moment re-read finds a sentinel it did not see at its FIRST read ABSTAINS instead of posting: that can only mean an overlapping run repaired a raced verdict mid-flight, and this run''s `success` — frozen at classification time, with the human row below its own mark, so neither the fence nor the post-write check would catch it — would otherwise bury the rejection. That is the one path in this design that failed toward SUCCESS rather than `pending`. The post-write check counts TWO row shapes above the mark, not one — a human `Review-verdict:` row AND a machine sentinel — because with two overlapping runs the human row can sit BELOW the second run''s mark while the first masks it and only then writes the sentinel, leaving the second to post its own `success` on top; counting the sentinel converges both runs on the fixed point instead. BOTH `/statuses/{sha}` reads PAGE to a validated EMPTY page since #763 — `[]` on this endpoint, a THIRD terminator shape distinct from the timeline''s bare `null` and the combined endpoint''s `{"statuses": null}`, so each terminator is MEASURED per endpoint — never terminating on a SHORT page, retrying each page once, under a 20-page cap that validates at most 950 rows (the 20th request must be the empty terminator); correctness does not depend on the measured cap of 50, because any page size pages correctly. WHAT THE PAGING BUYS IS NOT WHAT #763 CLAIMED: under the DESC default page 1 already held the true maximum id AND every row newer than the mark, so a single-page read missed a raced verdict only if more than 50 rows were created INSIDE the write window — not merely on a head over 50 rows. What removed #761''s stall is retiring the probe, not the walk; the walk''s value is that the gate''s one fail-toward-SUCCESS path no longer depends on an UNDOCUMENTED ordering the server honours only coarsely (page 1 came back `114,112,113,111,110`). This RETIRED #751''s conservative page-2 probe, which treated "there are rows I did not read" as "assume raced" and so repaired every head that outgrew one page: it fired on Renovate PR #761, reporting a human verdict as overwritten when none existed and then, the sentinel being sticky, refusing to re-exempt that head on every later run. The two directions are NOT symmetric. POST-WRITE, uncertainty fails CLOSED — an unreadable history, an over-cap history, or a count that is not a number all repair to `pending`; previously an unreadable history warned and left the exemption green while the page-2 probe repaired on the same uncertainty, one check disagreeing with itself. PRE-WRITE, a PARTIAL list still yields a mark, because the mark gates the post-write check entirely and refusing one SKIPS that check, which is itself the fail-open; this rests on the DESC DEFAULT — the newest row, carrying the maximum id, is on page 1, so a walk that fails later still saw it — while a VALIDATED empty history is NOT abandoned (it yields a mark of 0, correct for a first run, since every later row is newer) — what abandons the mark is a read that both FAILED and returned nothing — which since 2026-08-29 WITHHOLDS the exemption before the POST and marks the head with the reconcilable sentinel, rather than posting a green nothing can check (#849) — and a NON-EMPTY history carrying no numeric id is reported unusable rather than collapsed to 0. BOTH id comparisons are NUMERIC-ONLY: jq orders strings above every number, so one `"id": "99999"` inflates the mark until nothing looks newer, and `.id > $since` reads any string id as newer than any mark — making a PRE-EXISTING base-mismatched verdict look raced on every run, a permanent per-sha stall (the twin was live on `main`). An EMPTY post-write history is REJECTED: the walk terminates on an empty page, which is correct before the write and impossible after it (one row per POST), and a well-formed "no statuses exist" is not retried — so accepting it would conclude `raced=0` from a list that cannot be real, silently. That is NOT the withdrawn currency witness, which asked whether ANY row sat above the mark and was satisfied by an unrelated newer row; this asks only whether the list is EMPTY, a state no unrelated row can produce. The `::error::` now names its own cause, of which there are THREE — a verdict actually FOUND, a read that could not be COMPLETED, and a read that completed but returned an IMPOSSIBLE answer (the third is not a variety of the second) — while WHICH sentinel description is written is itself part of the answer since #849: only an arm that actually COUNTED a verdict row may write the repair sentinel, and every "could not check" arm writes the reconcilable one (`ci.verdict-unverified-write-sentinel`). Both are fixed points the classification recognises. `.creator` is TYPE-TESTED before it is indexed: `.creator != null and .creator.login` hard-errors on a non-object creator, jq exits 5, and under `set -e` that took the step down AFTER the exemption was posted and BEFORE the repair — a forged green reported as an infrastructure error.' +rule: 'The `review-verdict/h10` job counts BOTH `change_target_branch` AND `pull_push` events on the PR''s issue timeline at run start and again immediately before its POST, and does not post its CLASSIFICATION if EITHER count moved. Since #849 it does not merely abstain either: when the head carries a row this run did not inherit — and that row is neither the repair sentinel nor an allow-listed reviewer''s verdict — the arm REPLACES it with the unverified-write sentinel, because abstention is a handoff only when there is nothing to hand off (`ci.verdict-unverified-write-sentinel`). The COUNT is the key on both axes because the underlying VALUE is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption, and a force-push `H1 -> H2 -> H1` leaves `.head.sha` equal at both ends while the middle pages of `scripts/pr-changed-files.sh`''s enumeration came from `H2` (#803/#664) — while an event count is monotonic and cannot alias. ONE walk certifies BOTH counts, so an unreadable page abandons both; the two tallies are separate so the diagnostic names the axis that actually moved. Every push is counted and `is_force_push` is deliberately NOT read: an ordinary push also invalidates a mid-flight enumeration, and an `H1 -> H2 -> H1` restoration can have its second push non-forced when `H1` is an ancestor. The head fence does NOT abstain on its own triggering push — established FROM THE v1.27.1 SOURCE, since that would be a permanent stall rather than a fence: the push comment is created BEFORE the synchronize notification is emitted, so a run always sees its own causative event, and retries add no new event. The 26-102s margin measured across 20 triggered pairs on PRs #802/#834/#761 corroborates it and is a lower bound (the job runs a checkout first; 69s end to end on run 2385) — it is NOT the basis of the claim, which an earlier draft said it was. This NARROWS the residual rather than resolving it, and what CLOSED the permanent case is a separate mechanism recorded at `ci.verdict-unverified-write-sentinel`: the retarget count is re-taken AFTER the POST, so a retarget between the final pre-write count and the POST — which used to leave a PERMANENT forged green, the successor having consumed the `edited` event and exited before the stale run posted last — is now caught by the writing run itself, and one landing after the re-check necessarily queues a successor that starts with the stale `success` already visible and re-derivable (corrected 2026-08-27, closed 2026-08-29, #849). Abstaining is a handoff, not a stall, and that is the property the design rests on — for a run that ABSTAINS; it says nothing about one that already passed its final count and then posts (see the residual, corrected #849): every retarget fires `edited`, which is in this workflow''s `types:`, so the event that makes a run abstain has already queued a successor whose window opens after it; the induction terminates when retargeting stops — but ONLY over runs that ABSTAIN. A run that already passed its final count is outside it: it writes whenever it gets there, so the run that writes LAST is not necessarily the one that classified last. That was the permanent residual until #849 gave the writing run a post-POST re-count of its own (`ci.verdict-unverified-write-sentinel`), which puts such a run back inside the induction — it either withdraws its own stale write or leaves a green a guaranteed successor re-derives. `updated_at` was REJECTED as the key because it also moves for comments and labels, which fire none of this workflow''s `types:` — a run could abstain with no successor coming, which is a real stall. The count is trusted only when the walk read EVERY page up to its 20-page cap and the LAST page came back empty — an empty page BEFORE the cap is SKIPPED rather than read as exhaustion, since Gitea pages this endpoint before it filters and a fully-filtered page is byte-identical to the end of the list (#870) — which NARROWS that defeat about 10x rather than closing it — the 50-row filtered block is unchanged, but the timeline it must sit in grows from ~100 rows to over 1000 — the page-20 terminator still being trusted for the same unprovable reason; an untrusted count (unreadable page, non-array body, non-numeric length, empty FIRST page, a cap reached on a non-empty page) blocks the exemption `success` ONLY and still lets `pending` through, because `pending` blocks the merge immediately while withholding it would strand ordinary PRs whenever the timeline is unreadable — the right trade, but NOT a free one ("for no safety gain" retracted 2026-08-27): a GENERIC `pending` masks a rejection landing in its own write window just as a `success` does. What made that DURABLE — post-write verification skipping it, so a later run re-derived it into an exemption `success` — is closed since 2026-08-29: the check now runs after EVERY write (`ci.verdict-unverified-write-sentinel`), so such a write is repaired to the sticky repair sentinel and cannot be re-derived. The masking itself is still a cost, and a write the mark cannot cover is still unverified; that is what the second sentinel is for. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting ANY status — every write since #849, not only an exemption `success` — the job re-reads `/statuses/{sha}` IN FULL (PAGED since #763) and, if a human `Review-verdict:` row appeared with an id ABOVE a high-water mark taken just before the POST, overwrites its own status with `pending` and logs an error. The repair is `pending`, NEVER a copy of the human''s state, since re-posting their `failure` under the machine credential would attribute a human verdict to the job; its description is a SENTINEL that the classification refuses to grant an exemption over AND re-writes verbatim on every later run, so the block is a FIXED POINT rather than decaying — writing the generic `pending` description there instead erases the marker and the exemption simply returns one event later. The mark is captured BEFORE the last-moment re-read, not merely before the POST — a later mark leaves a multi-round-trip blind gap in which a verdict is neither seen by the re-read nor repaired afterwards. The id comparison is load-bearing: a mere presence test would fire forever on a base-mismatched verdict that `read_existing_verdict` deliberately declines to honour, deadlocking that PR''s exemption permanently. Finally, a run whose last-moment re-read finds a sentinel it did not see at its FIRST read ABSTAINS instead of posting: that can only mean an overlapping run repaired a raced verdict mid-flight, and this run''s `success` — frozen at classification time, with the human row below its own mark, so neither the fence nor the post-write check would catch it — would otherwise bury the rejection. That is the one path in this design that failed toward SUCCESS rather than `pending`. The post-write check counts TWO row shapes above the mark, not one — a human `Review-verdict:` row AND a machine sentinel — because with two overlapping runs the human row can sit BELOW the second run''s mark while the first masks it and only then writes the sentinel, leaving the second to post its own `success` on top; counting the sentinel converges both runs on the fixed point instead. BOTH `/statuses/{sha}` reads PAGE to a validated EMPTY page since #763 — `[]` on this endpoint, a THIRD terminator shape distinct from the timeline''s bare `null` and the combined endpoint''s `{"statuses": null}`, so each terminator is MEASURED per endpoint — never terminating on a SHORT page, retrying each page once, under a 20-page cap that validates at most 950 rows (the 20th request must be the empty terminator); correctness does not depend on the measured cap of 50, because any page size pages correctly. WHAT THE PAGING BUYS IS NOT WHAT #763 CLAIMED: under the DESC default page 1 already held the true maximum id AND every row newer than the mark, so a single-page read missed a raced verdict only if more than 50 rows were created INSIDE the write window — not merely on a head over 50 rows. What removed #761''s stall is retiring the probe, not the walk; the walk''s value is that the gate''s one fail-toward-SUCCESS path no longer depends on an UNDOCUMENTED ordering the server honours only coarsely (page 1 came back `114,112,113,111,110`). This RETIRED #751''s conservative page-2 probe, which treated "there are rows I did not read" as "assume raced" and so repaired every head that outgrew one page: it fired on Renovate PR #761, reporting a human verdict as overwritten when none existed and then, the sentinel being sticky, refusing to re-exempt that head on every later run. The two directions are NOT symmetric. POST-WRITE, uncertainty fails CLOSED — an unreadable history, an over-cap history, or a count that is not a number all repair to `pending`; previously an unreadable history warned and left the exemption green while the page-2 probe repaired on the same uncertainty, one check disagreeing with itself. PRE-WRITE, a PARTIAL list still yields a mark, because the mark gates the post-write check entirely and refusing one SKIPS that check, which is itself the fail-open; this rests on the DESC DEFAULT — the newest row, carrying the maximum id, is on page 1, so a walk that fails later still saw it — while a VALIDATED empty history is NOT abandoned (it yields a mark of 0, correct for a first run, since every later row is newer) — what abandons the mark is a read that both FAILED and returned nothing — which since 2026-08-29 WITHHOLDS the exemption before the POST and marks the head with the reconcilable sentinel, rather than posting a green nothing can check (#849) — and a NON-EMPTY history carrying no numeric id is reported unusable rather than collapsed to 0. BOTH id comparisons are NUMERIC-ONLY: jq orders strings above every number, so one `"id": "99999"` inflates the mark until nothing looks newer, and `.id > $since` reads any string id as newer than any mark — making a PRE-EXISTING base-mismatched verdict look raced on every run, a permanent per-sha stall (the twin was live on `main`). An EMPTY post-write history is REJECTED: the walk terminates on an empty page, which is correct before the write and impossible after it (one row per POST), and a well-formed "no statuses exist" is not retried — so accepting it would conclude `raced=0` from a list that cannot be real, silently. That is NOT the withdrawn currency witness, which asked whether ANY row sat above the mark and was satisfied by an unrelated newer row; this asks only whether the list is EMPTY, a state no unrelated row can produce. The `::error::` now names its own cause, of which there are THREE — a verdict actually FOUND, a read that could not be COMPLETED, and a read that completed but returned an IMPOSSIBLE answer (the third is not a variety of the second) — while WHICH sentinel description is written is itself part of the answer since #849: only an arm that actually COUNTED a verdict row may write the repair sentinel, and every "could not check" arm writes the reconcilable one (`ci.verdict-unverified-write-sentinel`). Both are fixed points the classification recognises. `.creator` is TYPE-TESTED before it is indexed: `.creator != null and .creator.login` hard-errors on a non-object creator, jq exits 5, and under `set -e` that took the step down AFTER the exemption was posted and BEFORE the repair — a forged green reported as an infrastructure error.' signals: 'stale review-verdict run overwrites a fresher one, retarget ABA against the docs-only classifier, head ABA force-push H1 H2 H1 during paging, pull_push timeline event count, force-push during changed-file enumeration, docs-only exemption from a file list spanning two heads, concurrency group does not serialize pull_request_target, gitea auto-cancel push vs pull_request_target, forged exemption restored after reclassification, human BLOCKED silently turned green, post-write status verification, change_target_branch timeline count, why does my PR post no verdict status after a retarget · paths: `.gitea/workflows/review-verdict.yml`, `scripts/tests/test_pr_changed_files.py` · issues: #706, #698, #672, #663, #622, #803, #664, #849' mechanics: '`count_pr_mutations()` (named `count_retargets()` until #803 put the head axis on the same walk) pages `GET /repos/{repo}/issues/{pr}/timeline?limit=50&page=N` (cap 20) setting `rt_count` (`change_target_branch`), `hp_count` (`pull_push`) and the SHARED trust flag `rt_ok`, trusted only when the walk read EVERY page up to the cap AND the LAST page came back empty (#870 — an empty page before the cap is skipped, not believed), an empty page being EITHER `null` (what this endpoint really returns past the end) or `[]` — an `array`-only type gate read the real terminator as unreadable and withheld every exemption (#751); each page bounded `--connect-timeout 5 --max-time 15` and retried once, as `page_statuses` is and for its reason, since the walk went from ~2 requests to a fixed 20 (#870); `retargets_before`/`pushes_before`/`retargets_before_ok` captured before enumeration, re-counted immediately before the POST, with a SEPARATE fence arm and diagnostic per axis (the base arm is evaluated first); the run log line is `Mutation fence: N retarget event(s) and M push event(s) ... (trusted=...)`, renamed from `Retarget fence: ...` by #803; `page_statuses()` pages `GET /repos/{repo}/statuses/{sha}?limit=50&page=N` (cap 20) setting `ph_rows`/`ph_ok`, trusted only on a validated EMPTY ARRAY page (this endpoint''s terminator, measured 2026-08-28 on PR #761''s 114-row head: pages 1-2 return 50, page 3 returns 14, page 4 is `[]`), retrying each page once with a 1s backoff; the #870 fixtures are `test_a_FILTERED_intermediate_page_does_not_END_the_walk_head_axis` (both empty shapes) and `..._base_axis`, `test_positive_control_a_FILTERED_intermediate_page_on_a_QUIET_pr_still_exempts`, `test_a_FILTERED_page_before_a_FULL_cap_page_is_still_untrusted`, `test_a_timeline_that_FILLS_the_page_cap_is_untrusted`, `test_a_SINGLE_transient_blip_does_not_cost_the_exemption` and `test_the_timeline_read_is_TIME_BOUNDED`; the server default `created_unix DESC` is KEPT, so a row inserted MID-WALK can be missed (it lands at position 0, on a page already read) — accepted and bounded, since a row arriving after this job''s POST is not one the job overwrote and wins on the combined endpoint anyway. `sort=highestindex` (index ASC, measured) closes that gap and was WITHDRAWN because ASC puts the OLDEST rows on page 1, which inverts the partial-mark fallback into a spurious-STICKY-repair engine — the #761 failure, re-introduced to close a smaller gap; `max_id_before` is the max over NUMERIC ids only in the paged list (jq orders strings above every number, so one `"id": "99999"` would otherwise pass the numeric gate and inflate the mark until nothing looked newer than it — a fail-open); this endpoint is a BARE ARRAY, unlike the combined `/commits/{sha}/status` object; repair POST is `pending`; tests `test_a_HEAD_ABA_DURING_the_run_posts_NOTHING`, `test_the_head_fence_holds_under_EITHER_terminator_shape`, `test_the_head_arm_DEFERS_to_the_trust_guard_when_the_second_count_fails`, `test_a_push_landing_BEYOND_page_1_is_still_counted`, `test_positive_control_a_MULTI_PAGE_quiet_timeline_still_exempts`, `test_an_EMPTY_FIRST_page_is_untrusted_WHICHEVER_empty_shape_it_is`, `test_a_timeline_row_with_NO_READABLE_TYPE_is_not_counted_as_nothing`, `test_a_SINGLE_head_push_during_the_run_also_posts_NOTHING`, `test_a_PR_PUSHED_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt`, `test_the_head_and_base_axes_are_counted_SEPARATELY`, `test_the_head_fence_reports_the_OBSERVED_counts_in_its_notice`, `test_a_PROTECTED_pr_pushed_mid_run_ALSO_posts_nothing`, `test_a_RETARGET_DURING_the_run_posts_NOTHING`, `test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt`, `test_an_UNTRUSTED_retarget_count_withholds_the_EXEMPTION`, `test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH`, `test_a_human_verdict_landing_AFTER_the_POST_is_repaired_to_pending`, `test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair`, `test_a_status_history_RUNNING_PAST_PAGE_1_is_PAGED_and_the_exemption_STANDS`, `test_a_raced_verdict_on_PAGE_2_is_detected_and_repaired`, `test_a_verdict_BEYOND_A_SHORT_PAGE_is_still_found`, `test_a_PRE_WRITE_paging_failure_still_yields_a_usable_high_water_mark`, `test_a_TRANSIENT_page_failure_is_retried_and_the_walk_still_completes`, `test_a_STRING_id_cannot_inflate_the_high_water_mark`, `test_a_MALFORMED_creator_row_does_not_kill_the_job_after_the_green_is_posted`, `test_a_STRING_id_on_a_PRE_EXISTING_row_is_not_read_as_raced`, `test_a_partial_mark_is_SAFE_because_the_newest_rows_are_on_page_1`, `test_an_EMPTY_post_write_history_is_not_evidence_that_nothing_raced`, `test_a_NULL_page_terminates_the_walk_as_an_empty_one`, `test_the_walk_does_not_request_a_SORT_order`' --- @@ -238,8 +238,8 @@ produces. And no row was validated before `.type` was selected on, so a page of readable `.type` certified a zero count; `pr-changed-files.sh` has applied validate-what-you-consume to every row it extracts since #643, and the walk whose counts gate the write did not. -**Not closed by this.** Two residuals; the second was closed by #870 and is kept here, rewritten, -because what it leaves behind still bounds the fence. +**Not closed by this.** Two residuals. The second was NARROWED by #870, not closed, and is kept +here rewritten because what it leaves behind still bounds the fence. 1. The residual named in "What is NOT closed" item 1 applies to the head axis with the base axis's remedy DELIBERATELY WITHHELD: a push landing between the final pre-write count and the POST is @@ -296,8 +296,22 @@ because what it leaves behind still bounds the fence. own window but leaves the identical ambiguity inside it, and it would cost the "a real page was actually read" property that an anomalous empty page 1 provides. The cost of reading to the cap has TWO bounds and both belong here: a healthy walk is exactly 20 requests, measured at 2.6s; - the worst case, with the per-page retry added in the same change, is 40 requests plus 19 - one-second sleeps — a page sleeps only when its first attempt yielded neither array nor null. + the worst case, with the per-page retry added in the same change, is 40 requests plus 20 + one-second sleeps (every page blipping once and recovering) — both MEASURED by executing the walk + against scripted responses, after a first draft said 19 by reasoning that the last page could not + sleep. In WALL-CLOCK the pessimum is 20 x (15 + 1 + 15) = 620s per walk, because a page whose + first attempt burns its timeout and whose second succeeds slowly still continues the walk; a + second draft said ~335s by bounding timeouts rather than elapsed time. The job has no + `timeout-minutes`. This is still strictly better than the predecessor, which had no timeout at + all — but only here: `page_statuses` remains unbounded and runs in the same post-POST window. + + TWO COSTS THIS ALSO PAYS, stated because neither is visible from the diff. The exemption + `success` is live between its POST and the post-write repair, and the walk in between is now + 20 requests rather than ~2 — about +2.3s on a healthy run at the measured 0.13s/request. + And the test suite roughly doubled: `test_pr_changed_files.py` went 202s -> 474s (286 -> 294 + tests, same machine, `-p no:randomly`, 528383cf3 vs this branch), because every fence fixture + now drives 20 stub requests per walk instead of 2. `testing.mutation-claims-are-executed` + carried a `~4min script-tests` figure measured before this and is dated accordingly. **What is left — NARROWED, NOT CLOSED.** Pages 1-19 cover offsets 0..949 and page 20 covers 950..999, so a timeline of more than 1000 rows that ALSO presents a fully-filtered block at @@ -305,12 +319,13 @@ because what it leaves behind still bounds the fence. too-low count with `rt_ok=yes`. Both re-reads share the blind spot, so the counts agree and the exemption is granted. That is the SAME construction as before, relocated from page 2 to the cap. - The price comparison, stated in one unit so it is not read as better than it is: BEFORE, the - timeline needed ~100 rows total — a non-empty page 1, then a 50-row filtered block at a - 50-aligned offset. AFTER, it needs more than 1000, with the filtered block pinned to offsets - 950..999. So the cost rose by roughly 10x, not 20x; "50 comments instead of 1000 rows" compares - an authored-comment count against a total-row count and flatters the change. The number of - comments the attacker must author is 50 in both cases. The inference moved; it did not go away. + The price, stated once and in one unit, because two earlier drafts stated it two ways: the + FILTERED BLOCK is 50 rows in both cases — that is the part which must be inline review comments. + What grew is the timeline it has to sit in: ~100 rows before (a non-empty page 1, then a 50-row + block at a 50-aligned offset) against more than 1000 now, with the block pinned to offsets + 950..999. Roughly 10x. "50 comments either way" was wrong: on their own PR the ~950 rows of + padding are the attacker's to supply too, so they are part of the price. The inference moved; it + did not go away. Two things this must NOT be described as, both of which an earlier draft of this record said. It is not "closed": the walk still trusts one empty page, the terminator at the cap, for the same diff --git a/docs/decisions/records/testing/mutation-claims-are-executed.md b/docs/decisions/records/testing/mutation-claims-are-executed.md index ff6209bc2..20cf5d5fe 100644 --- a/docs/decisions/records/testing/mutation-claims-are-executed.md +++ b/docs/decisions/records/testing/mutation-claims-are-executed.md @@ -7,7 +7,7 @@ supersedes: none superseded-by: none rule: 'A `MUTATION` row in `docs/guard-inventory.md` is not a statement that someone once witnessed a red. It carries a DECLARED clause mutation in `scripts/tests/mutation_manifest.py`, and `scripts/tests/test_mutation_harness.py` applies that mutation to an isolated copy of the repository on every run and requires the row''s OWN named test to go red. The manifest and the MUTATION rows are compared for SET EQUALITY in both directions, so a row cannot claim the grade without a mutation and a mutation cannot outlive the grade it justifies. EXIT STATUS IS NOT THE VERDICT: each entry also declares the DIAGNOSTIC its red must carry, matched against pytest''s exception output alone, because pytest reports a crashing test exactly as it reports a detecting one and a red for an unrelated reason is evidence about nothing. WHERE THE GUARD IS ITSELF A TEST, `target` may differ from `guard` and the exact-once check applies to the declared TARGET. Two shapes are admissible and the choice is not free. Where the guard''s assertion IS the check — a completeness comparison against a Markdown inventory — the mutation goes into the guarded ARTIFACT, per `testing.guard-ships-with-mutation-proof`''s checker-guard exception, because mutating such a checker''s own POPULATION demonstrates a false POSITIVE while proving nothing about the detection the row claims. Where the guard is a test module wrapping a separately mutable DETECTOR or helper, the clause may be in that detector, since disarming it is a real clause disarm and the module''s own assertion is what notices. THE MUTATION IS DECLARED, NEVER INFERRED: a harness that guessed which clause of a 90-line hook is the guard would manufacture the confident-but-empty coverage this exists to prevent, which is why `testing.guard-ships-with-mutation-proof` rejected a generic runner. Where a proof test already names its clause in source, the manifest reuses THAT string, so a retarget in either place is caught by the other. COARSENESS IS RECORDED, NOT HIDDEN: each entry is graded `CLAUSE` or `DETECTOR`, and a `DETECTOR` entry — one whose detector accumulates faults from independent arms, so disarming any single arm leaves its proof test green — must CARRY the finer mutation that survived, which is re-run every time and required to keep surviving. Guards that are not graded `MUTATION` each carry a STATED reason in that same manifest, keyed on the guard and compared for SET EQUALITY against the inventory''s `GUARD` rows in both directions — so a new guard cannot arrive without someone writing what a proof would need, and a reason cannot outlive the row it is about. Keying the reason on the row''s GRADE instead is tautological (a new guard inherits one and nobody looks at it) and a pinned COUNT moves only on net change; both were tried and are rejected. The sandbox is a real git repository built from `git ls-files` with working-tree content, never a filesystem walk.' signals: 'mutation harness · declared clause not inferred · MUTATION row is executed · set equality manifest vs inventory · CLAUSE vs DETECTOR granularity · surviving finer mutation is re-run · pytest exit code 1 is the only red that counts · every other exit status is rejected · positive control before any mutation · sandbox from git ls-files · paths: `scripts/tests/test_mutation_harness.py`, `scripts/tests/mutation_manifest.py`, `scripts/tests/mutation_harness_lib.py`, `docs/guard-inventory.md` · issues: #790, #775, #774, #778, #806' -mechanics: 'The harness builds ONE sandbox per session — tracked files only, from `git ls-files -s`, in an environment with every `GIT_*` variable stripped so an exported `GIT_DIR`/`GIT_COMMON_DIR`/`GIT_CONFIG_*` cannot point its `init`/`add`/`commit` at the real repository — and resets it between mutations with `git reset --hard` TO A BASELINE COMMIT RECORDED OUTSIDE THE REPOSITORY, plus `git clean -qffdx`. Both halves matter: bare `git reset --hard` resets to whatever HEAD is, so a proof test that COMMITS inside the sandbox moves HEAD onto a commit carrying the mutant and every later reset restores it faithfully; and a baseline held as a REF inside the sandbox is one more thing a proof can move. `core.hooksPath`, `commit.gpgsign` and `core.worktree` are pinned on every git invocation for the same reason. The `-ff` removes a nested repository a proof may have left. It runs the full set of named proof tests UNMUTATED first, inside the fixture rather than as a separate test so ordering is a dependency and not a convention. Only pytest exit code 1 counts as red; every other status is rejected. That is what closes the two ways a proof ref goes stale, and they are measured rather than assumed: with an explicit `file.py::function` node id a missing file and a missing function both exit 4, while 5 needs a collection that succeeded and selected nothing. The sandbox skips the `ErsatzTV-macOS` gitlink and recreates the `.claude/skills/jellyfin` symlink as a symlink. Cost: ~14s, against a ~4min `script-tests` suite.' +mechanics: 'The harness builds ONE sandbox per session — tracked files only, from `git ls-files -s`, in an environment with every `GIT_*` variable stripped so an exported `GIT_DIR`/`GIT_COMMON_DIR`/`GIT_CONFIG_*` cannot point its `init`/`add`/`commit` at the real repository — and resets it between mutations with `git reset --hard` TO A BASELINE COMMIT RECORDED OUTSIDE THE REPOSITORY, plus `git clean -qffdx`. Both halves matter: bare `git reset --hard` resets to whatever HEAD is, so a proof test that COMMITS inside the sandbox moves HEAD onto a commit carrying the mutant and every later reset restores it faithfully; and a baseline held as a REF inside the sandbox is one more thing a proof can move. `core.hooksPath`, `commit.gpgsign` and `core.worktree` are pinned on every git invocation for the same reason. The `-ff` removes a nested repository a proof may have left. It runs the full set of named proof tests UNMUTATED first, inside the fixture rather than as a separate test so ordering is a dependency and not a convention. Only pytest exit code 1 counts as red; every other status is rejected. That is what closes the two ways a proof ref goes stale, and they are measured rather than assumed: with an explicit `file.py::function` node id a missing file and a missing function both exit 4, while 5 needs a collection that succeeded and selected nothing. The sandbox skips the `ErsatzTV-macOS` gitlink and recreates the `.claude/skills/jellyfin` symlink as a symlink. Cost: ~14s. The ~4min `script-tests` suite it was measured against is PRE-#870 and no longer holds: that change made `count_pr_mutations` read to its 20-page cap, and `test_pr_changed_files.py` went from 202s to 474s locally (286 -> 294 tests, same machine, `-p no:randomly`, 528383cf3 vs the #870 branch), so the job is roughly double. Re-measure on a runner before quoting a job figure.' --- **What #775 left open, in its own words: "nothing checks that a row claiming `MUTATION` is telling diff --git a/docs/remote-state-inventory.md b/docs/remote-state-inventory.md index a27be2a54..76f1dab54 100644 --- a/docs/remote-state-inventory.md +++ b/docs/remote-state-inventory.md @@ -97,7 +97,7 @@ classifications differ; otherwise the strictest applies and the Note names the e | Site | Class | Note | |---|---|---| | `scripts/post-review-verdict.sh` — commit-status write | `PINNED` | Re-reads the PR and compares **both** `.head.sha` and `.base.ref` immediately before the POST, and `die`s (exit 1, no status written) on a mismatch **or on a field it cannot read**. That last clause is load-bearing: guarding both comparisons with `[ -n "$x" ] &&` alone makes a well-formed 2xx body that merely omits the field a no-op, and the status is posted having confirmed nothing. Regression-tested against the real predecessor (#778), since the redundant-looking `-z` arm alone mutates green. Closes #706 and #632 for this path by read-compare-refuse, not by CAS: Gitea's status API offers no conditional write. Residual closed on the write ORDER since #792: the status is written first and the comment second, so a refusal can no longer leave a verdict comment with no status behind it — the only reachable half-state is a status with no comment, which the merge hook resolves as `ask` (`release.verdict-writes-status-before-comment`). Since #845 the status is also READ BACK after the POST and refused unless it is the write just made, and — for a `success` only, mirroring the gate's own asymmetry — unless its `.creator.login` is on the allow-list derived from the gate (a `failure` is inherited from any attributable account, so requiring membership there would refuse a rejection the gate honours) — the same read-compare-refuse shape, applied to WHOSE verdict it is rather than to which commit it covers. That read is check-then-act too and is graded with this row rather than separately: it can only withhold this tool's success report, never authorize anything, and the residual it leaves — a non-inheritable status standing with no comment beside it — is the `ask` half-state above. Two properties of that read are recorded because neither is visible in the response. `GET /commits/{sha}/status` PAGES and its `total_count` reports the PAGE rather than the total (measured Gitea 1.27.1, 2026-08-29 — `?limit=2` on a 15-context head returned 2 rows and `total_count: 2`), so a truncated body is indistinguishable from a complete one. That looked like a risk of the verdict row falling off a page and is NOT, because of the second property: the endpoint selects each context's MAX row id and orders those DESCENDING before paginating (same measurement — ids `[17,19,...,41,43]`, and `?limit=2` returns 41 and 43, the two highest). The row this tool has just POSTed is the newest on the head, so it sorts first and page 1 holds it at any page size. `?limit=100` (the instance clamps `limit` to `MAX_RESPONSE_ITEMS`, measured at 50) is kept as insurance against a burst of concurrent status writes and against that ordering changing, and has no observable effect otherwise — stated so it is not mistaken for an untested guard. A row it cannot find is therefore refused with the row count it read and no stated cause; an earlier version of this row named truncation as the candidate cause, before the ordering was measured. | -| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases — "any movement fails" would be stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias — a force-push `H1 -> H2 -> H1` during pagination — leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and NOTHING inside this script can see it. **Fenced at the enforced caller since #803** (2026-08-28): the same timeline walk now also counts `pull_push`, so a head that moved at all during classification withholds the write — subject to that walk's own terminator, which is defeatable by a page of filtered rows — **NARROWED, not closed, by #870** (2026-08-30): the walk no longer reads an empty page BEFORE the cap as the end of the list, but reads to its 20-page cap and requires the last page to be empty, so the same defeat now needs a timeline of more than 1000 rows rather than ~100, with the filtered block pinned to offsets 950..999 — about 10x the price, the attacker authoring 50 inline comments either way. A cap filled with READABLE pages fails closed; a cap hidden behind a filtered block still certifies a too-low count. The grade stays `UNSAFE-KNOWN` for the SCRIPT, because the script's own guarantee is unchanged and one-way — a caller that does not run the fence inherits the alias. Enumeration bound to an immutable tree remains unavailable: re-probed at 1.27.1 on 2026-08-28, `compare/{base}...{head}` still returns no `files` key. | +| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases — "any movement fails" would be stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias — a force-push `H1 -> H2 -> H1` during pagination — leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and NOTHING inside this script can see it. **Fenced at the enforced caller since #803** (2026-08-28): the same timeline walk now also counts `pull_push`, so a head that moved at all during classification withholds the write — subject to that walk's own terminator, which is defeatable by a page of filtered rows — **NARROWED, not closed, by #870** (2026-08-30): the walk no longer reads an empty page BEFORE the cap as the end of the list, but reads to its 20-page cap and requires the last page to be empty, so the same defeat now needs a timeline of more than 1000 rows rather than ~100, with the same 50-row filtered block pinned to offsets 950..999 — about 10x the price, the padding beneath the block being the attacker's to supply on their own PR. A cap filled with READABLE pages fails closed; a cap hidden behind a filtered block still certifies a too-low count. The grade stays `UNSAFE-KNOWN` for the SCRIPT, because the script's own guarantee is unchanged and one-way — a caller that does not run the fence inherits the alias. Enumeration bound to an immutable tree remains unavailable: re-probed at 1.27.1 on 2026-08-28, `compare/{base}...{head}` still returns no `files` key. | | `scripts/select-queue.sh` — issue list, then per-issue `/dependencies` | `UNSAFE-KNOWN` | The open-issue list (labels, milestone, priority) is snapshotted once; per-candidate dependency reads happen seconds later and never re-read the issue's own labels, so an issue claimed `in-progress` in that gap still appears on the shortlist. Accepted: the script authorizes **no write**. The real gate is the four-way claim check in `process.parallel-session-claim`, which runs after selection and re-reads live state by construction. Tightening this would move a check that must be adversarial into a tool that is advisory. | | `scripts/ci-detect-already-validated.sh` — prior-head combined status | `UNSAFE-KNOWN` | Reads the PR head's status and emits `skip=true`, with nothing re-checking before the consuming job runs. Accepted and narrow: the skip elides only **re-running** test/migrations on a tree already validated; the `build` job still builds and pushes unconditionally, so no image ever ships from unvalidated source. | | `scripts/issue-qualification-audit.sh` — paged issue list, then a report | `UNSAFE-KNOWN` | Pages the open-issue list and reports which issues lack a `priority:` label, so like every paged read here its pages can straddle a change and the report can name a state no single instant held. Graded to match `select-queue.sh` rather than `N/A`: the two run the same shape, and the reason offered for accepting `select-queue.sh` — it authorizes no write — cannot simultaneously be the reason this one is out of the class. Accepted on the same terms: it is advisory, session-end, human-read, and the labels it prompts for are applied by hand afterwards. | diff --git a/scripts/tests/test_pr_changed_files.py b/scripts/tests/test_pr_changed_files.py index dee5e31bf..54655b4c3 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -1012,16 +1012,34 @@ if "/timeline" in url: for part in url.split("?", 1)[-1].split("&"): if part.startswith("page="): page = int(part.split("=", 1)[1]) - if os.environ.get("STUB_TIMELINE_FLAKY_FIRST") == "1": + flaky = os.environ.get("STUB_TIMELINE_FLAKY_FIRST", "") + if flaky: # ONE TRANSIENT BLIP, on the very first timeline request of the run (ersatztv#870 review). # This is the only arrangement that separates a walk that retries from one that does not: # `transport-error` and `unreadable` fail EVERY attempt, so a retry changes nothing and both # implementations refuse identically. Counting REQUESTS, not pages, because the retry is # per-request — `timeline_reads.txt` bumps once per walk and cannot see a second attempt. + # + # TWO BLIP SHAPES, because they reach DIFFERENT arms of the retry and only one was pinned. + # `transport` exits non-zero with no body, so `raw` is empty and `kind` is never computed. + # `body` returns 200 with a well-formed JSON OBJECT — which is what this Gitea really sends + # on an error, measured: `?since=NOTATIME` returns an object, not an array — so `jq -r type` + # SUCCEEDS and sets `kind=object`. + # + # That difference is the whole point of the second shape. An unreadable body (a 502 HTML + # page) would NOT do: jq fails on it, `|| kind=""` fires, and the walk takes the same path + # as `transport`. Only a parseable non-array body distinguishes "break out of the retry on + # anything readable" from "break only on a shape the `case` below accepts", and that + # narrowing is a real fail-open — an error object would be read as the walk's terminator + # verdict instead of being retried. The first draft of this fixture used the 502 page and + # the mutant passed it. rctr = out / "timeline_requests.txt" rseen = int(rctr.read_text()) if rctr.exists() else 0 rctr.write_text(str(rseen + 1)) if rseen == 0: + if flaky == "body": + print(json.dumps({"message": "internal server error", "url": "http://gitea"})) + sys.exit(0) sys.exit(22) if mode == "empty-first-page": # A terminator on PAGE 1 — no real page ever read (ersatztv#751). The job must NOT certify a @@ -1801,7 +1819,7 @@ def _run_classify( push_mode: str = "none", timeline_pages: int = 1, timeline_filtered_pages: str = "", - timeline_flaky_first: bool = False, + timeline_flaky_first: str = "", history_mode: str = "none", history_creator: str = "timothy", history_extra: list | None = None, @@ -1848,7 +1866,7 @@ def _run_classify( env["STUB_PUSH_MODE"] = push_mode env["STUB_TIMELINE_PAGES"] = str(timeline_pages) env["STUB_TIMELINE_FILTERED_PAGES"] = timeline_filtered_pages - env["STUB_TIMELINE_FLAKY_FIRST"] = "1" if timeline_flaky_first else "" + env["STUB_TIMELINE_FLAKY_FIRST"] = timeline_flaky_first env["STUB_TIMELINE_TERMINATOR"] = timeline_terminator env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape env["STUB_HISTORY_MODE"] = history_mode @@ -3710,7 +3728,8 @@ def test_a_FILTERED_page_before_a_FULL_cap_page_is_still_untrusted(tmp_path): ) -def test_a_SINGLE_transient_blip_does_not_cost_the_exemption(tmp_path): +@pytest.mark.parametrize("blip", ["transport", "body"]) +def test_a_SINGLE_transient_blip_does_not_cost_the_exemption(tmp_path, blip): """The walk retries each page once, as `page_statuses` does and for its stated reason. This walk had no retry while it made ~2 requests per call. At a fixed 20 it makes 10x as many, @@ -3721,17 +3740,27 @@ def test_a_SINGLE_transient_blip_does_not_cost_the_exemption(tmp_path): `transport-error` and `unreadable` cannot measure this: they fail every attempt, so a retrying walk and a non-retrying one refuse identically. Only a blip that clears on the second attempt separates them. + + BOTH SHAPES, because they reach different arms and only one was pinned. `transport` leaves `raw` + empty. `body` returns a well-formed JSON OBJECT — what this Gitea actually sends on an error + (measured: `?since=NOTATIME` returns an object) — so `jq -r type` succeeds and `kind=object`. + + That is what kills the narrowing `if [ -n "$kind" ]; then break; fi`, under which an error object + ends the retry and is then refused by the `case`, costing the exemption. An UNREADABLE body does + not kill it: jq fails, `kind=""`, and the walk takes `transport`'s path. The first version of + this fixture used a 502 HTML page for exactly that reason and the mutant passed it — measured, + not assumed. """ posted, r = _run_classify( tmp_path, _emitting("docs/a.md"), timeline_mode="none", push_mode="stable:0", - timeline_flaky_first=True, + timeline_flaky_first=blip, ) assert posted is not None and posted["state"] == "success", ( - "a docs-only PR lost its exemption to ONE transient timeline blip — the walk does not " - f"retry. Log:\n{r.stdout[-900:]}" + f"a docs-only PR lost its exemption to ONE transient timeline blip ({blip}) — the walk does " + f"not retry this shape. Log:\n{r.stdout[-900:]}" )