From 0d8ef4c81c7683f8bbec90d100605f3062066fd5 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 30 Aug 2026 10:32:53 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(870):=20an=20empty=20timeline=20page=20?= =?UTF-8?q?is=20not=20exhaustion=20=E2=80=94=20the=20walk=20reads=20to=20i?= =?UTF-8?q?ts=20cap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `count_pr_mutations` in `review-verdict.yml` treated an empty page past page 1 as proof it had reached the end of the PR timeline. Gitea does not mean that. `ListIssueCommentsAndTimeline` applies the LIMIT/OFFSET in `FindComments` at the DATABASE level and filters AFTERWARDS, dropping every `CommentTypeCode` row and every inaccessible cross-reference into a nil slice that serializes as bare `null`. A page whose 50 rows are all inline review comments is therefore byte-identical to a page past the end, while later pages still hold events — and rows are ordered ASCENDING, so the events a fence looks for are the furthest from page 1. Fifty inline comments, which a PR author can create on their own PR, truncated both walks at the same place: both counts agreed, the sha comparison agreed, and an ABA force-push (`H1 -> H2 -> H1`) yielded an exemption `success` over a diff no single head justified. The walk no longer infers the end at all. Past page 1 an empty page is SKIPPED; the loop reads every page up to its pre-existing 20-page cap and the trust decision moves to after the loop — `rt_ok=yes` only when the LAST page read came back empty. That is the contract the sibling status walk already stated for itself ("the 20th request has to be the EMPTY terminator"), so the two now agree instead of one silently claiming coverage it did not perform. An empty FIRST page and any unreadable shape still end the walk untrusted. "Require K consecutive empty pages" was rejected: 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. Three measurements at Gitea 1.27.1 rule out the cheaper fixes, and are recorded because each is the kind of thing a later reader would otherwise re-propose: - `X-Total-Count` on this endpoint is the POST-FILTER LENGTH OF THE PAGE, not a total: `?limit=1` returns `X-Total-Count: 1` on a PR whose timeline holds 14 rows, and a page past the end returns 0. `/activities/feeds` DOES return a true total (5739), so this is a property of the handler, not the server. - `limit` clamps to 50 here (`?limit=200` returned 50 on #303/#237/#91/#202). - The endpoint's only query parameters are `since`, `before`, `page`, `limit` — no row-type filter, so the paged set and the serialized set cannot be made to agree. A `since`-bounded walk was considered and not taken on its own: it narrows the attack to the run's own window but leaves the identical ambiguity inside it, and costs the "a real page was actually read" property that an anomalous empty page 1 provides. Residual, stated rather than left in the issue: the walk confirms offsets 0..949 and requires its last page to be empty, so hiding an event now needs a timeline over 1000 rows AND a fully-filtered block at offsets 950..999 — ~1000 rows instead of 50, and the same over-long -history residual the status walk already accepts. An over-cap timeline fails CLOSED. Cost: a fixed 20 requests per walk instead of ~2, measured at 2.6s against the live instance. Tests: a new stub knob `STUB_TIMELINE_FILTERED_PAGES` serves a filtered intermediate page — the shape the double could not previously produce. Three new tests reproduce the defeat (head axis over both empty shapes, and its base-axis twin) and go RED against the shipped predecessor, granting `state: success` on the hidden ABA. A positive control keeps the walk from simply refusing every filtered page, and `test_a_timeline_that_FILLS_the_page_cap_is_untrusted` guards the new post-loop clause — making that trust unconditional reddens it. Full suite: 1351 passed, 2 skipped. Fixes #870 Refs: #803, #706, #664, #751 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/review-verdict.yml | 187 ++++++++++-------- docs/ci-cd.md | 19 +- docs/decisions/README.md | 2 +- .../ci/verdict-write-retarget-fence.md | 56 ++++-- docs/remote-state-inventory.md | 2 +- scripts/tests/test_pr_changed_files.py | 133 +++++++++++++ 6 files changed, 295 insertions(+), 104 deletions(-) diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index ea2ce0408..0681550a9 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -987,7 +987,7 @@ jobs: rt_count=0 hp_count=0 rt_ok=no - local page=1 raw n m p bad kind total=0 ptotal=0 + local page=1 raw n m p bad kind total=0 ptotal=0 empty=no while [ "$page" -le 20 ]; do raw=$(gh "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || return 0 if [ -z "${raw//[[:space:]]/}" ]; then return 0; fi @@ -1014,93 +1014,122 @@ jobs: # workflow once at jq 1.6 (ersatztv#647). A `case` over `jq -r 'type'` cannot be read # two ways. Anything that is neither `null` nor `array` is still "cannot tell". kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind="" - # `null` counts as exhaustion only from page 2 ON. THE INVARIANT, not a figure that - # rots: a real PR's timeline always carries at least one event on page 1 (it is created - # by a push, which is itself an event), so a `null` FIRST page is anomalous rather than - # empty. Spot-checked across #752/#753/#749/#739/#717, all non-empty; the counts - # themselves are deliberately not recorded here because timelines grow and an earlier - # version of this comment cited five numbers of which three were stale within days. - # Trusting a zero count from an anomalous first page would - # mean trusting that no retarget happened on the strength of a response we cannot - # explain. Requiring one real page keeps the property the walk is for: something was - # actually read. A PR that somehow has an empty first page falls through to `rt_ok=no`, - # which withholds the exemption and asks for a human verdict — the safe direction. - # This narrows rather than closes the general concern: a wrong `null` on page 3 is still - # read as exhaustion, and no bounded number of round-trips can rule that out. - # - # AND THAT WRONG `null` IS CONSTRUCTIBLE, NOT HYPOTHETICAL (established 2026-08-28 by - # cross-family review of ersatztv#803, from the v1.27.1 source; tracked - # as ersatztv#870, because it predates #803 and defeats BOTH axes identically). The handler pages at the - # DATABASE level and filters AFTERWARDS: `FindComments` applies the LIMIT/OFFSET, then + # AN EMPTY PAGE IS NOT EXHAUSTION, AND THIS WALK NO LONGER TREATS IT AS ONE + # (ersatztv#870). Gitea pages this endpoint at the DATABASE level and filters + # AFTERWARDS: `FindComments` applies the LIMIT/OFFSET, then # `ListIssueCommentsAndTimeline` drops every `CommentTypeCode` row and every - # cross-reference the reader cannot access, appending survivors to a `var apiComments - # []*api.TimelineComment` — a NIL slice. So a page whose rows are ALL inline-code - # comments serializes as bare `null` while later pages still hold events, and rows are - # ordered ASCENDING, so the events a fence cares about are the ones furthest from page - # 1. Fifty inline review comments, which a PR author can create on their own PR, are - # enough to truncate both walks at the same place; both counts then agree, the sha - # comparison agrees, and the ABA goes unseen. + # cross-reference the reader cannot access, appending the survivors to a + # `var apiComments []*api.TimelineComment` — a NIL slice, which serializes as bare + # `null`. So a page whose rows are ALL inline-code comments comes back empty WHILE + # LATER PAGES STILL HOLD EVENTS, and rows are ordered ASCENDING, so the events a fence + # looks for are the ones furthest from page 1. Fifty inline review comments, which a + # PR author can create on their own PR, were enough to truncate both walks at the same + # place: both counts then agreed, the sha comparison agreed, and the ABA went unseen. # - # So DO NOT read the fence below as closing the head ABA outright. What it closes is - # the ABA on a PR whose timeline has no such truncating block — which is every PR this - # repo has, but is not a property anyone enforces. Stated here rather than left in the - # issue because the whole of #803 is that a contract asserting more than its code does - # is worse than no contract. + # THE FIX IS TO STOP INFERRING THE END AT ALL, not to count empty pages. This loop + # already carries a 20-page cap, and the sibling status walk below already states the + # contract this one was breaking — "the 20th request has to be the EMPTY terminator + # for the walk to be validated". So the walk now reads EVERY page up to that cap and + # decides afterwards: past page 1 an empty page is SKIPPED, never believed. 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. + # + # MEASURED AT GITEA 1.27.1 ON THIS INSTANCE (2026-08-30), because the cheaper + # alternatives are ruled out by measurement rather than by reasoning: + # * `X-Total-Count` on this endpoint is the POST-FILTER LENGTH OF THE PAGE, not a + # total. `?limit=1` returns `X-Total-Count: 1` on a PR whose timeline holds 14 + # rows, `?limit=5` returns 5, and a page past the end returns 0. It therefore + # carries exactly what the body already carries and cannot derive a page count. + # (`/activities/feeds` DOES return a true total — 5739 — so this is a property of + # this handler, not of the server, and a future Gitea could change it.) + # * `limit` clamps to 50 here: `?limit=200` returned 50 on #303, #237, #91 and #202. + # * The endpoint's ONLY query parameters are `since`, `before`, `page` and `limit` + # (read from the live swagger, `issueGetCommentsAndTimeline`). There is no + # row-type filter, so the paged set and the serialized set cannot be made to agree. + # A `since`-bounded walk was considered and NOT taken on its own: it narrows the attack + # to the run's own window but leaves the identical ambiguity inside it, and it would + # cost the "a real page was actually read" property below, which depends on an empty + # page 1 being anomalous. The cost of reading to the cap is 20 requests per walk, + # measured at 2.6s against this instance. + # + # WHAT REMAINS UNCOVERED, stated because #803's whole point is that a contract must + # not assert more than its code does: the walk confirms offsets 0..949 and requires + # the last page it reads to be empty, so an event can still be hidden by a timeline + # that carries more than 1000 rows AND presents a fully-filtered block at offsets + # 950..999. That costs ~1000 timeline rows instead of 50, and it is the same + # over-long-history residual the status walk already accepts and documents. + # + # Read the type as a VALUE rather than through `jq -e` — see the `kind` note above. + empty=no case "$kind" in - null) - if [ "$page" -gt 1 ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; fi - return 0 ;; + null) empty=yes ;; array) ;; *) return 0 ;; esac - n=$(printf '%s' "$raw" | jq -r 'length') - case "$n" in ''|*[!0-9]*) return 0 ;; esac - # AN EMPTY **ARRAY** FIRST PAGE IS AS ANOMALOUS AS A `null` ONE (fixed 2026-08-28, - # ersatztv#803 review). The `null` arm above requires `page > 1` and the comment - # above it says "a PR that somehow has an empty first page falls through to - # `rt_ok=no`" — but this arm trusted `[]` on ANY page, so the code contradicted its - # own stated rule and the sentence describing the safe behaviour was false for one of - # the two empty shapes. A `[]` page 1 yielded `rt_ok=yes` over a ZERO count, which is - # a fail-OPEN: it certifies "nothing was retargeted and nothing was pushed" from a - # response that cannot be explained, and a docs-only PR is then exempted on it. The - # two shapes now take the SAME rule, which is also what stops a serializer change - # from silently switching the walk between a trusted and an untrusted terminator. - if [ "$n" -eq 0 ]; then - if [ "$page" -gt 1 ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; fi - return 0 + if [ "$empty" = no ]; then + n=$(printf '%s' "$raw" | jq -r 'length') + case "$n" in ''|*[!0-9]*) return 0 ;; esac + # `[]` AND `null` TAKE THE SAME RULE (ersatztv#803 review). `/issues/{n}/comments` + # really does return `[]` when empty while this endpoint returns `null`, so both + # shapes are live on this server; treating them differently once let a `[]` page 1 + # certify a zero count. Keeping them identical is also what stops a serializer + # change from silently switching the walk between a trusted and an untrusted + # terminator. + if [ "$n" -eq 0 ]; then empty=yes; fi + fi + if [ "$empty" = yes ]; then + # AN EMPTY PAGE 1 IS ANOMALOUS AND ENDS THE WALK UNTRUSTED. THE INVARIANT, not a + # figure that rots: a real PR's timeline always carries at least one event on page 1 + # (the PR is created by a push, which is itself an event). Spot-checked across + # #752/#753/#749/#739/#717, all non-empty; the counts themselves are deliberately + # not recorded here because timelines grow and an earlier version of this comment + # cited five numbers of which three were stale within days. Trusting a zero count + # from an anomalous first page would mean trusting that no retarget happened on the + # strength of a response we cannot explain, so it falls through to `rt_ok=no`, which + # withholds the exemption and asks for a human verdict — the safe direction. + if [ "$page" -eq 1 ]; then return 0; fi + else + # VALIDATE THE FIELD THE SECURITY DECISION READS, on every row (ersatztv#803 review). + # Both tallies below select on `.type`; a row without a string `.type` matches neither + # and is silently treated as an irrelevant event. That is the same + # validate-what-you-consume rule `pr-changed-files.sh` already applies to every row it + # extracts, and the reason is the same: a shape we cannot read is not evidence that + # nothing happened. Every timeline row measured on this instance carries a string + # `.type`, so this rejects only shapes the server has not been observed to produce. + # + # COUNTED, NOT `jq -e`, and that is this function's own standing rule rather than a + # style preference. The `kind` read above says it in full: `jq -e` reports the + # truthiness of its LAST OUTPUT, which is the exit-status subtlety that already bit + # this workflow at jq 1.6 (ersatztv#647), and the runner still ships 1.6. A count read + # as a VALUE and then range-checked in shell cannot be read two ways, and it matches + # every other tally here. (The first draft of this guard used `jq -e`, safely by + # accident — the array is known non-empty by the check above, so the empty-input + # divergence could not fire — but "safe because of a property three lines up" is + # exactly the reasoning the rule exists to retire.) + bad=$(printf '%s' "$raw" | jq -r '[.[] | select(type != "object" or (.type | type) != "string")] | length') + case "$bad" in ''|*[!0-9]*) return 0 ;; esac + if [ "$bad" -ne 0 ]; then return 0; fi + m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length') + case "$m" in ''|*[!0-9]*) return 0 ;; esac + # Counted in the SAME pass over the SAME page, so the head axis costs no extra round + # trip and cannot be certified by a different set of pages than the base axis. A + # non-numeric length here abandons BOTH counts, exactly as it does above — a page whose + # push tally is unreadable has not established the retarget tally either. + p=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "pull_push")] | length') + case "$p" in ''|*[!0-9]*) return 0 ;; esac + total=$(( total + m )) + ptotal=$(( ptotal + p )) fi - # VALIDATE THE FIELD THE SECURITY DECISION READS, on every row (ersatztv#803 review). - # Both tallies below select on `.type`; a row without a string `.type` matches neither - # and is silently treated as an irrelevant event. That is the same - # validate-what-you-consume rule `pr-changed-files.sh` already applies to every row it - # extracts, and the reason is the same: a shape we cannot read is not evidence that - # nothing happened. Every timeline row measured on this instance carries a string - # `.type`, so this rejects only shapes the server has not been observed to produce. - # - # COUNTED, NOT `jq -e`, and that is this function's own standing rule rather than a - # style preference. The `kind` read above says it in full: `jq -e` reports the - # truthiness of its LAST OUTPUT, which is the exit-status subtlety that already bit - # this workflow at jq 1.6 (ersatztv#647), and the runner still ships 1.6. A count read - # as a VALUE and then range-checked in shell cannot be read two ways, and it matches - # every other tally here. (The first draft of this guard used `jq -e`, safely by - # accident — the array is known non-empty by the check above, so the empty-input - # divergence could not fire — but "safe because of a property three lines up" is - # exactly the reasoning the rule exists to retire.) - bad=$(printf '%s' "$raw" | jq -r '[.[] | select(type != "object" or (.type | type) != "string")] | length') - case "$bad" in ''|*[!0-9]*) return 0 ;; esac - if [ "$bad" -ne 0 ]; then return 0; fi - m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length') - case "$m" in ''|*[!0-9]*) return 0 ;; esac - # Counted in the SAME pass over the SAME page, so the head axis costs no extra round - # trip and cannot be certified by a different set of pages than the base axis. A - # non-numeric length here abandons BOTH counts, exactly as it does above — a page whose - # push tally is unreadable has not established the retarget tally either. - p=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "pull_push")] | length') - case "$p" in ''|*[!0-9]*) return 0 ;; esac - total=$(( total + m )) - ptotal=$(( ptotal + p )) page=$(( page + 1 )) done + # THE CAP IS THE TERMINATOR, AND THE LAST PAGE READ MUST BE EMPTY (ersatztv#870). + # Falling out of the loop on a NON-empty page 20 means the timeline may extend past the + # offsets this walk covered, so it is "could not establish" and leaves `rt_ok=no` — the + # same treatment an unreadable page gets, and the same rule the status walk states for + # itself. That is also why the confirmable bound is 950 rows and not 1000: the 20th + # request has to come back empty for the walk to be validated. + if [ "$empty" = yes ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; fi return 0 } # PAGE THE PER-POST STATUS HISTORY TO A VALIDATED TERMINATOR (ersatztv#763). diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 3735e296a..aa36f0bf0 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -2021,12 +2021,19 @@ Two things kept it invisible, and both are worth generalising: The type is now read as a value (`case` over `jq -r 'type'`) rather than through `jq -e`, whose exit-status semantics already bit this workflow once at jq 1.6, and both `null` and `[]` terminate the walk. The regression test is parameterised over both shapes because both are live on this server. -An empty page — `null` OR `[]`, which take the SAME rule since ersatztv#803 — is accepted as -exhaustion only from **page 2 on** — 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 timelines grow and such figures go stale within days), so an empty first -page is ANOMALOUS rather than EXHAUSTED, and the walk should not certify "no retarget happened" from a -response it cannot explain. +An empty page — `null` OR `[]`, which take the SAME rule since ersatztv#803 — is **no longer accepted +as exhaustion at all** (ersatztv#870, 2026-08-30). Gitea pages this endpoint at the database level and +filters afterwards, so a page whose 50 rows are all inline-code comments serializes as exactly the +value the walk used to trust as the end of the list, while later pages still hold events — 50 comments, +which a PR author can create on their own PR, bought silence on both axes. The walk now SKIPS an empty +page past page 1, reads every page up to its 20-page cap, and trusts the counts only when the LAST +page it read came back empty — the same rule the status walk above already stated for itself. 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 +timelines grow and such figures go stale within days), so an empty first page is ANOMALOUS rather than +EXHAUSTED, and the walk should not certify "no retarget happened" from a response it cannot explain. +`X-Total-Count` cannot shortcut this: on this endpoint it is the post-filter length of the PAGE, not a +total (`?limit=1` returns 1 on a 14-row timeline), unlike `/activities/feeds`, which does return one. **The same nil-slice shape bites `/commits/{sha}/status`** — a third instance. A head with no statuses yet returns `{"state":"pending","total_count":0,"statuses":null}` (measured on PR #739's head). `read_existing_verdict` diff --git a/docs/decisions/README.md b/docs/decisions/README.md index cb8b53bb3..89f9f3509 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 paging reached a validated EMPTY page; an untrusted count (unreadable page, non-array body, non-numeric length, page cap hit) 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 past page 1 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); 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 e25883e71..8f577d8df 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 paging reached a validated EMPTY page; an untrusted count (unreadable page, non-array body, non-numeric length, page cap hit) 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 past page 1 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); 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 on a validated empty page, which is a page of 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); `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 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`' --- @@ -233,7 +233,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, and the second is the one to read carefully. +**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. 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 @@ -248,24 +249,45 @@ to every row it extracts since #643, and the walk whose counts gate the write di split the file list across heads. That is #803's text and settling it needs its own analysis; #849 did not attempt one. -2. **The walk's terminator is defeatable, so neither axis is closed outright** — pre-existing, found - by cross-family review of #803 from the v1.27.1 source, and tracked as ersatztv#870 because fixing - it is a redesign of a walk both fences share. `ListIssueCommentsAndTimeline` pages at the DATABASE - level (`FindComments` applies the LIMIT/OFFSET) and filters AFTERWARDS, dropping every +2. **The walk's terminator WAS defeatable; closed 2026-08-30 by ersatztv#870, with a narrower + residual in its place.** `ListIssueCommentsAndTimeline` pages at the DATABASE level + (`FindComments` applies the LIMIT/OFFSET) and filters AFTERWARDS, dropping every `CommentTypeCode` row and every inaccessible cross-reference into a `var apiComments []*api.TimelineComment` — a NIL slice, which serializes as bare `null`. So a page whose rows are - all inline-code comments returns the exact value this walk trusts as exhaustion, while later pages - still hold events; and rows are ordered ASCENDING, so the events a fence cares about are the ones - furthest from page 1. Fifty inline review comments — which a PR author can create on their own - PR — truncate both walks at the same place, both counts then agree, the sha comparison agrees, and - the ABA is unseen. + all inline-code comments returned the exact value this walk trusted as exhaustion, while later + pages still held events; and rows are ordered ASCENDING, so the events a fence cares about are + the ones furthest from page 1. Fifty inline review comments — which a PR author can create on + their own PR — truncated both walks at the same place, both counts then agreed, the sha + comparison agreed, and the ABA was unseen. - What the fence narrows is therefore the ABA on a PR whose timeline carries no such truncating - block, which is every PR this repo has but is not a property anyone enforces. Narrows, not closes, - even for those PRs: residual 1 above applies to them too. Said here rather - than left in the issue because the whole of #803 is that a contract asserting more than its code - does is worse than no contract — and a fence advertised as closing the head ABA would have been - the fourth such contract, added by the change that removed three. + **The walk no longer infers the end at all.** An empty page past page 1 is SKIPPED, not believed; + the walk reads every page up to its existing 20-page cap and decides afterwards, trusting the + counts only when the LAST page it read came back empty. That is the rule the sibling status walk + already stated for itself, so the two now agree rather than one silently claiming coverage it did + not perform. "Require K consecutive empty pages" was rejected: K is arbitrary and only moves the + bar to 50K filtered rows. + + Three measurements at Gitea 1.27.1 (2026-08-30) rule out the cheaper fixes, and are recorded + because each is the kind of thing a later reader would otherwise re-propose. `X-Total-Count` on + this endpoint is the POST-FILTER LENGTH OF THE PAGE, not a total — `?limit=1` returns + `X-Total-Count: 1` on a PR whose timeline holds 14 rows, and a page past the end returns 0 — so it + carries exactly what the body already carries and cannot derive a page count. (`/activities/feeds` + DOES return a true total, 5739, so this is a property of the handler, not the server, and a later + Gitea could change it.) `limit` clamps to 50 here (`?limit=200` returned 50 on #303/#237/#91/#202). + And the endpoint's only query parameters are `since`, `before`, `page` and `limit` — there is no + row-type filter, so the paged set and the serialized set cannot be made to agree. A + `since`-bounded walk was considered and not taken on its own: it narrows the attack to the run's + 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 is + 20 requests per walk, measured at 2.6s. + + **What is left.** The walk confirms offsets 0..949 and requires the last page it reads to be + empty, so an event can still be hidden by a timeline carrying more than 1000 rows that ALSO + presents a fully-filtered block at offsets 950..999. That is ~1000 timeline rows instead of 50, + and it is the same over-long-history residual the status walk already accepts and documents — an + over-cap timeline fails CLOSED (`rt_ok=no`, exemption withheld), it does not pass silently. Said + here rather than left in the issue because the whole of #803 is that a contract asserting more + than its code does is worse than no contract. ## The status history is paged too (2026-08-28, ersatztv#763) diff --git a/docs/remote-state-inventory.md b/docs/remote-state-inventory.md index 3d56944ba..24327e3bb 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 (**#870**, pre-existing and equally affecting the base axis). 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 was defeatable by a page of filtered rows until **#870** closed it (2026-08-30): the walk no longer reads an empty page as the end of the list, but reads to its 20-page cap and requires the last page to be empty, so hiding an event now costs ~1000 timeline rows instead of 50 and an over-cap timeline fails closed. 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 a6892352b..879843c3a 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -1087,6 +1087,22 @@ if "/timeline" in url: # than simply corrected: `/issues/{n}/comments` really does return `[]` when empty, so both # shapes are live on this server and the job must accept either. pages = int(os.environ.get("STUB_TIMELINE_PAGES", "1")) + # A FILTERED INTERMEDIATE PAGE (ersatztv#870). This is the shape the double could not produce + # before, and it is not "a page that happens to be empty": the endpoint pages at the DATABASE + # level and filters AFTERWARDS, so a page whose 50 rows are all `CommentTypeCode` serializes as + # the SAME `null` a page past the end does, while LATER pages still hold events. Serving it from + # the terminator env var rather than a literal is deliberate — the two shapes are + # indistinguishable to the walk on the wire, which is the entire defect, so a double that made + # them distinguishable would test a server we do not have. + # + # It exits BEFORE the last-real-page branch, so it does not touch `timeline_reads.txt`: that + # counter means "real pages served", and a filtered page served no rows. + filtered = { + int(x) for x in os.environ.get("STUB_TIMELINE_FILTERED_PAGES", "").split(",") if x.strip() + } + if page in filtered: + print(os.environ.get("STUB_TIMELINE_TERMINATOR", "null")) + sys.exit(0) if page > pages: print(os.environ.get("STUB_TIMELINE_TERMINATOR", "null")) sys.exit(0) @@ -1770,6 +1786,7 @@ def _run_classify( timeline_mode: str = "none", push_mode: str = "none", timeline_pages: int = 1, + timeline_filtered_pages: str = "", history_mode: str = "none", history_creator: str = "timothy", history_extra: list | None = None, @@ -1815,6 +1832,7 @@ def _run_classify( env["STUB_TIMELINE_MODE"] = timeline_mode env["STUB_PUSH_MODE"] = push_mode env["STUB_TIMELINE_PAGES"] = str(timeline_pages) + env["STUB_TIMELINE_FILTERED_PAGES"] = timeline_filtered_pages env["STUB_TIMELINE_TERMINATOR"] = timeline_terminator env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape env["STUB_HISTORY_MODE"] = history_mode @@ -3530,6 +3548,121 @@ def test_positive_control_a_MULTI_PAGE_quiet_timeline_still_exempts(tmp_path): ) +# THE FILTERED-INTERMEDIATE-PAGE DEFEAT (ersatztv#870), on both axes. +# +# The walk used to read an empty page past page 1 as proof of exhaustion. Gitea does not mean that: +# `ListIssueCommentsAndTimeline` applies the LIMIT/OFFSET in `FindComments` and drops +# `CommentTypeCode` rows and inaccessible cross-references AFTERWARDS, into a nil slice that +# serializes as bare `null`. So a page of 50 inline review comments — which a PR author can create on +# their own PR — comes back looking exactly like the end of the list while later pages still hold +# events, and rows are ASCENDING, so the events a fence looks for are the furthest from page 1. +# +# Both of these fixtures are the same construction with a different event on the far page, because +# the defeat is not axis-specific: one walk publishes both counts. Covering only the head axis would +# have repeated `process.fix-one-path-then-check-its-twin`. + + +@pytest.mark.parametrize("terminator", ["null", "[]"]) +def test_a_FILTERED_intermediate_page_does_not_END_the_walk_head_axis(tmp_path, terminator): + """A page that is empty because it was FILTERED must not certify the count. Head axis. + + Page 1 is full filler, page 2 comes back empty, page 3 carries the pushes. A walk that stops at + page 2 certifies a zero push count on both reads, so the two agree, the sha comparison agrees, + and the ABA is exempted — which is precisely the attack: 50 inline comments buy silence. + + Parameterised over both empty shapes because the walk has to treat them identically here; the + `null`-only fixture would leave the `[]` path free to keep the old early return. + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="none", + push_mode="moves:0,2", + timeline_pages=3, + timeline_filtered_pages="2", + timeline_terminator=terminator, + ) + assert posted is None, ( + f"an exemption was granted over an ABA hidden behind a filtered page 2 (`{terminator}`) — " + f"the walk is reading a filtered page as the end of the timeline. Log:\n{r.stdout[-900:]}" + ) + assert "head branch was pushed while this job was classifying" in r.stdout, ( + f"nothing was posted, but NOT via the head fence. Log:\n{r.stdout[-900:]}" + ) + + +def test_a_FILTERED_intermediate_page_does_not_END_the_walk_base_axis(tmp_path): + """The twin: the same construction hides a RETARGET rather than a push. + + Same page layout, retargets on the far page. The base axis is the older of the two fences and + reads the same `rt_ok`/`rt_count` pair, so a fix that only reached the head arm would leave the + original #706 fence defeated by a construction its own test suite never built. + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="moves:0,1", + timeline_pages=3, + timeline_filtered_pages="2", + ) + assert posted is None, ( + f"an exemption was granted over a retarget hidden behind a filtered page 2. Log:\n{r.stdout[-900:]}" + ) + assert "retargeted while this job was classifying" in r.stdout, ( + f"nothing was posted, but NOT via the base fence. Log:\n{r.stdout[-900:]}" + ) + + +def test_positive_control_a_FILTERED_intermediate_page_on_a_QUIET_pr_still_exempts(tmp_path): + """Without this, the two tests above pass against a walk that refuses every filtered page. + + Refusing outright would be safe and useless: every PR carrying inline review comments would lose + its exemption. The walk must SKIP the empty page and keep counting, which is only visible when a + quiet PR with the same page layout still gets its `success`. + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="none", + push_mode="stable:2", + timeline_pages=3, + timeline_filtered_pages="2", + ) + assert posted is not None and posted["state"] == "success", ( + f"a quiet PR lost its exemption because a page of its timeline was filtered. Log:\n{r.stdout[-900:]}" + ) + + +def test_a_timeline_that_FILLS_the_page_cap_is_untrusted(tmp_path): + """The cap is not exhaustion, and the LAST page read has to be the empty one. + + Now that an empty page no longer ends the walk, the trust decision moved to after the loop — and + the whole of it rests on that decision still being conditional. Making it unconditional (a bare + `rt_ok=yes` once the loop finishes) would certify a partial count over an over-long timeline, + which is the failure the sibling status walk already refuses by the same rule. 25 real pages + means the walk never sees an empty one, so it must report "could not establish" and withhold. + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="none", + push_mode="stable:0", + timeline_pages=25, + ) + # NOT `posted is None`. An untrusted count on the exemption path does not merely stay silent — + # since ersatztv#849 it replaces the unknown state with the sticky unverified-write sentinel, so + # something IS posted and the assertion has to discriminate on the STATE. Asserting silence here + # passed a `pending` sentinel off as a failure and would have gone red on correct behaviour. + assert posted is None or posted["state"] != "success", ( + "an exemption was granted over a timeline that never terminated inside the page cap — the " + f"post-loop trust decision is unconditional. Log:\n{r.stdout[-900:]}" + ) + assert "Could not establish a trusted retarget/push count" in r.stdout, ( + "no exemption was granted, but NOT because the count was untrusted — this test is not " + f"measuring the page cap. Log:\n{r.stdout[-900:]}" + ) + + @pytest.mark.parametrize("terminator", ["null", "[]"]) def test_an_EMPTY_FIRST_page_is_untrusted_WHICHEVER_empty_shape_it_is(tmp_path, terminator): """Both empty shapes must fall through to `rt_ok=no`, and only one of them did. -- 2.47.3 From f34801b51fe35d50b30f28def4b8fa6d03cd5386 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 30 Aug 2026 11:13:11 +0200 Subject: [PATCH 2/4] =?UTF-8?q?fix(870):=20review=20round=20=E2=80=94=20th?= =?UTF-8?q?e=20fix=20NARROWS=20the=20defeat,=20and=20the=20prose=20said=20?= =?UTF-8?q?"closed"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent cold reviews (Codex GPT-5.6 cross-family; a fresh Opus agent in an isolated worktree, which executed the walk against synthetic page shapes). Both landed on the same substantive defect, and it was in the prose, not the code. THE OVERCLAIM. The fix moves the unprovable inference from page 2 to the page cap; it does not remove it. An empty page 20 is still trusted, for the same reason an empty page 2 used to be — this endpoint cannot distinguish a filtered page from the end of the list at ANY offset. Six sentences across three docs and the workflow said otherwise, including "no longer accepted as exhaustion at all", "STOP INFERRING THE END AT ALL", and — in the paragraph that describes the case where it fails OPEN — "an over-cap timeline fails CLOSED ... it does not pass silently". All rewritten to NARROWED, not closed, with the residual stated where a reader meets the claim. THE PRICE, IN ONE UNIT. "50 rows instead of 1000" compared an authored-comment count against a total-row count and flattered the change. Before: ~100 timeline rows (a non-empty page 1, then a 50-row filtered block at a 50-aligned offset). After: over 1000, with the block pinned to offsets 950..999. About 10x, and the attacker authors 50 inline comments either way. THE PARITY CLAIM WAS FALSE. "The two walks now agree" — they do not. `page_statuses` still returns on its FIRST empty page; what the two share is only the cap rule. Worse, the diff asserted parity it had not established: whether `/statuses/{sha}` is subject to the same post-pagination filtering was never checked. Measured instead of asserted (Gitea 1.27.1, 2026-08-30): that endpoint returns a TRUE `X-Total-Count` (105 at both `?limit=1` and `?limit=50`) where `/issues/{n}/timeline` returns the post-filter page length. Consistent with counting before filtering — evidence, not proof, since no filtering predicate has been exhibited either way. Filed as #893 rather than resolved here. CODE CHANGES, both answering the amplification this fix introduced (~2 requests per walk to a fixed 20, three walks per exempt run): - Each page is now bounded `--connect-timeout 5 --max-time 15`. `gh` is bare `curl -sf` with no timeout, and the THIRD call site runs after the exemption `success` is posted, so a hang leaves the green standing with no repair attempted. Bounded at this call rather than inside `gh`, so no other call site changes behaviour. - Each page is retried once, mirroring `page_statuses` and its stated reason — "one transient blip would cost that head its exemption until a human clears it by hand". That walk retries; this one did not, and it just took 10x the round trips. - The four jq reads carry `2>/dev/null || x=""`, as `page_statuses`'s do. Unreachable today thanks to the ordering above them, which is exactly the "safe because of a property three lines up" reasoning the adjacent comment says it is retiring. STALE CONTRACT TEXT swept: the function's own header still said "trusted ONLY when paging reached a validated EMPTY page" and listed a short page as untrusted (false before and after — a short page is tallied and the walk continues); the record's `mechanics:` field still described the old terminator and omitted the new fixtures. TESTS (+3, and one reviewer claim refuted): - `test_a_FILTERED_page_before_a_FULL_cap_page_is_still_untrusted` — filtered page 19, full page 20. This kills the "trust if ANY page was empty" mutant, which the Opus review proved survived the previous round's suite; every earlier fixture ends on an empty page, so a latching flag and a correct one agree there. - `test_a_SINGLE_transient_blip_does_not_cost_the_exemption`, with a `flaky-first` stub mode. The existing `transport-error`/`unreadable` modes fail EVERY attempt, so a retrying walk and a non-retrying one refuse identically and neither could measure this. - `test_the_timeline_read_is_TIME_BOUNDED` — structural; a hang cannot be provoked through a stub that answers instantly. Each was mutated and witnessed red. Codex claimed the per-iteration `empty=no` reset was unguarded and that deleting it would leave all four new tests green; deleting it turns THREE of them red — a stale `yes` also suppresses the tally on every later non-empty page. The Opus review mutated the same line independently and got the same red. Recorded because a wrong mechanism claim in a review is as executable as a right one. Full suite: 1354 passed, 2 skipped. Fixes #870 Refs: #803, #706, #664, #751, #893 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/review-verdict.yml | 123 ++++++++++++++---- docs/ci-cd.md | 18 ++- docs/decisions/README.md | 2 +- .../ci/verdict-write-retarget-fence.md | 79 ++++++++--- docs/remote-state-inventory.md | 2 +- scripts/tests/test_pr_changed_files.py | 94 ++++++++++++- 6 files changed, 261 insertions(+), 57 deletions(-) diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index 0681550a9..02f87e038 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -917,10 +917,13 @@ jobs: # Completeness is a guard, not an assumption (`ci.verdict-write-retarget-fence`; this # cited `ci.paged-endpoint-completeness` until 2026-08-06, a key that has never existed as # a record — resolve decisions through the catalog, never through a key or path quoted in a - # comment). The count is - # trusted ONLY when paging reached a validated EMPTY page. A short page, a non-array body, a - # non-numeric length or the page cap all leave `rt_ok=no`, and an untrusted count is treated - # below as "cannot tell" rather than as zero. + # 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 + # 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. # THE SAME WALK CARRIES A SECOND AXIS: HEAD MUTATION (ersatztv#803/#664). # # The base fence above closes `main -> S -> main`. The HEAD aliases the same way and was @@ -987,10 +990,39 @@ jobs: rt_count=0 hp_count=0 rt_ok=no - local page=1 raw n m p bad kind total=0 ptotal=0 empty=no + local page=1 raw n m p bad kind try total=0 ptotal=0 empty=no while [ "$page" -le 20 ]; do - raw=$(gh "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || return 0 - if [ -z "${raw//[[:space:]]/}" ]; then return 0; fi + # BOUNDED AND RETRIED ONCE, because this walk got 10x longer (ersatztv#870 review). + # Both halves answer the same amplification and neither is decoration. + # + # BOUNDED: `gh` is bare `curl -sf` with no timeout of its own, and the THIRD call site + # runs AFTER the exemption `success` is already posted — a hang there leaves the green + # standing with no repair attempted. That exposure predates this change, but going + # from ~2 requests per walk to a fixed 20 multiplies the chances of hitting it. The + # bound is applied HERE rather than inside `gh` so every other call site keeps the + # behaviour it was reviewed with: widening a shared helper changes callers nobody + # re-examined. 15s is ~100x the measured per-request latency, so it bounds a hang + # without reddening a slow-but-working instance. + # + # RETRIED: `page_statuses` retries each page once and states why — "one transient blip + # would cost that head its exemption until a human clears it by hand". That argument + # applies here with 10x the force after this change, because `rt_ok=no` on the + # exemption path writes the sticky sentinel too. This walk had no retry when it made + # ~2 requests; at 20 it needs the sibling's. Same shape deliberately: same `for try`, + # same one-second sleep BETWEEN attempts only, same "an outage still refuses". + raw="" + kind="" + # `if`/`then`, never `cmd && var=yes`: under `set -e` a bare `A && B` whose test fails + # is itself a failing command, and as the last command of a loop body it would kill + # the job. (Carried from `page_statuses`, where the same trap was found.) + for try in 1 2; do + raw=$(gh --connect-timeout 5 --max-time 15 "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || raw="" + if [ -n "${raw//[[:space:]]/}" ]; then + kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind="" + fi + if [ "$kind" = array ] || [ "$kind" = null ]; then break; fi + if [ "$try" -eq 1 ]; then sleep 1; fi + done # A PAGE PAST THE END IS JSON `null` HERE, NOT `[]` (ersatztv#751). Measured on this # instance at Gitea 1.27.1 (page 1 non-empty, page 2 exhausted): page 2 of PR #752's timeline is the # four bytes `null`, while `/issues/{n}/comments` with no comments returns `[]`. The @@ -1013,7 +1045,6 @@ jobs: # false" through it means relying on the same exit-status subtlety that already bit this # workflow once at jq 1.6 (ersatztv#647). A `case` over `jq -r 'type'` cannot be read # two ways. Anything that is neither `null` nor `array` is still "cannot tell". - kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind="" # AN EMPTY PAGE IS NOT EXHAUSTION, AND THIS WALK NO LONGER TREATS IT AS ONE # (ersatztv#870). Gitea pages this endpoint at the DATABASE level and filters # AFTERWARDS: `FindComments` applies the LIMIT/OFFSET, then @@ -1026,14 +1057,24 @@ jobs: # PR author can create on their own PR, were enough to truncate both walks at the same # place: both counts then agreed, the sha comparison agreed, and the ABA went unseen. # - # THE FIX IS TO STOP INFERRING THE END AT ALL, not to count empty pages. This loop - # already carries a 20-page cap, and the sibling status walk below already states the - # contract this one was breaking — "the 20th request has to be the EMPTY terminator - # for the walk to be validated". So the walk now reads EVERY page up to that cap and - # decides afterwards: past page 1 an empty page is SKIPPED, never believed. 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. + # THE FIX MOVES THE INFERENCE TO THE CAP; IT DOES NOT REMOVE IT, and saying so + # precisely matters more here than a tidy claim. This loop already carries a 20-page + # cap, and the sibling status walk below already states the CAP half of the contract + # this one was breaking — "the 20th request has to be the EMPTY terminator for the + # walk to be validated". Only the cap half: `page_statuses` still returns on its FIRST + # empty page, so the two walks do NOT now share a termination rule and saying they do + # would be a parity claim nobody established (ersatztv#893 tracks whether that walk + # needs the same change; `/statuses/{sha}` returns a TRUE `X-Total-Count`, which this + # endpoint does not, so its terminator may well mean what it says). So the walk now + # reads EVERY page up to that cap and decides afterwards: + # 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 + # 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. # # MEASURED AT GITEA 1.27.1 ON THIS INSTANCE (2026-08-30), because the cheaper # alternatives are ruled out by measurement rather than by reasoning: @@ -1053,12 +1094,31 @@ jobs: # page 1 being anomalous. The cost of reading to the cap is 20 requests per walk, # measured at 2.6s against this instance. # - # WHAT REMAINS UNCOVERED, stated because #803's whole point is that a contract must - # not assert more than its code does: the walk confirms offsets 0..949 and requires - # the last page it reads to be empty, so an event can still be hidden by a timeline - # that carries more than 1000 rows AND presents a fully-filtered block at offsets - # 950..999. That costs ~1000 timeline rows instead of 50, and it is the same - # over-long-history residual the status walk already accepts and documents. + # 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 + # 0..949 and page 20 covers 950..999, so an event is still hidden by a timeline of + # more than 1000 rows that ALSO presents a fully-filtered block at offsets 950..999: + # 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. + # + # 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 + # `/statuses/{sha}` does not filter after paging. Only this endpoint lets a page-20 + # response be empty while later pages hold rows. A timeline that fills the cap with + # 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 + # 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). # # Read the type as a VALUE rather than through `jq -e` — see the `kind` note above. empty=no @@ -1068,7 +1128,7 @@ jobs: *) return 0 ;; esac if [ "$empty" = no ]; then - n=$(printf '%s' "$raw" | jq -r 'length') + n=$(printf '%s' "$raw" | jq -r 'length' 2>/dev/null) || n="" case "$n" in ''|*[!0-9]*) return 0 ;; esac # `[]` AND `null` TAKE THE SAME RULE (ersatztv#803 review). `/issues/{n}/comments` # really does return `[]` when empty while this endpoint returns `null`, so both @@ -1090,6 +1150,15 @@ jobs: # withholds the exemption and asks for a human verdict — the safe direction. if [ "$page" -eq 1 ]; then return 0; fi else + # EVERY jq READ BELOW IS GUARDED `2>/dev/null || x=""`, matching `page_statuses` + # (added ersatztv#870 review). Today none of them can error — the `array` gate above + # and the `bad -eq 0` ordering see to that — but "safe because of a property three + # lines up" is exactly the reasoning the `jq -e` note below says it is retiring, and + # an unguarded jq error dies under `set -e`. At the POST-WRITE call site that death + # lands after the exemption `success` is on the head and before any repair, which is + # the shape this file already fixed once for `.creator`. The `case` validation on + # each value turns an empty string into the existing untrusted path. + # # VALIDATE THE FIELD THE SECURITY DECISION READS, on every row (ersatztv#803 review). # Both tallies below select on `.type`; a row without a string `.type` matches neither # and is silently treated as an irrelevant event. That is the same @@ -1107,16 +1176,16 @@ jobs: # accident — the array is known non-empty by the check above, so the empty-input # divergence could not fire — but "safe because of a property three lines up" is # exactly the reasoning the rule exists to retire.) - bad=$(printf '%s' "$raw" | jq -r '[.[] | select(type != "object" or (.type | type) != "string")] | length') + bad=$(printf '%s' "$raw" | jq -r '[.[] | select(type != "object" or (.type | type) != "string")] | length' 2>/dev/null) || bad="" case "$bad" in ''|*[!0-9]*) return 0 ;; esac if [ "$bad" -ne 0 ]; then return 0; fi - m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length') + m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length' 2>/dev/null) || m="" case "$m" in ''|*[!0-9]*) return 0 ;; esac # Counted in the SAME pass over the SAME page, so the head axis costs no extra round # trip and cannot be certified by a different set of pages than the base axis. A # non-numeric length here abandons BOTH counts, exactly as it does above — a page whose # push tally is unreadable has not established the retarget tally either. - p=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "pull_push")] | length') + p=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "pull_push")] | length' 2>/dev/null) || p="" case "$p" in ''|*[!0-9]*) return 0 ;; esac total=$(( total + m )) ptotal=$(( ptotal + p )) @@ -1852,7 +1921,7 @@ jobs: # nothing to buy by writing a value this run already knows was computed against a base the # PR no longer targets. One rule, one direction, nothing to reason about per state. # - # An UNTRUSTED count on either side (`rt_ok=no`: paging never reached a validated empty + # An UNTRUSTED count on either side (`rt_ok=no`: the walk did not reach its cap on an empty # page, a page was unreadable, the cap was hit) is NOT treated as "no retarget". It blocks # the exemption `success` only, and lets `pending` through — because a `success` written on a # count we could not verify is exactly the forged-green outcome this fence exists to diff --git a/docs/ci-cd.md b/docs/ci-cd.md index aa36f0bf0..1ca41f8ad 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -2019,15 +2019,21 @@ Two things kept it invisible, and both are worth generalising: fidelity, that claim is a test assertion and needs re-measuring like any other. The type is now read as a value (`case` over `jq -r 'type'`) rather than through `jq -e`, whose -exit-status semantics already bit this workflow once at jq 1.6, and both `null` and `[]` terminate the -walk. The regression test is parameterised over both shapes because both are live on this server. -An empty page — `null` OR `[]`, which take the SAME rule since ersatztv#803 — is **no longer accepted -as exhaustion at all** (ersatztv#870, 2026-08-30). Gitea pages this endpoint at the database level and +exit-status semantics already bit this workflow once at jq 1.6, and both `null` and `[]` are read as +the same EMPTY shape. The regression test is parameterised over both because both are live on this +server. An empty page — `null` OR `[]`, which take the SAME rule since ersatztv#803 — is **no longer +accepted as exhaustion before the page cap** (ersatztv#870, 2026-08-30). Gitea pages this endpoint at the database level and filters afterwards, so a page whose 50 rows are all inline-code comments serializes as exactly the value the walk used to trust as the end of the list, while later pages still hold events — 50 comments, which a PR author can create on their own PR, bought silence on both axes. The walk now SKIPS an empty -page past page 1, reads every page up to its 20-page cap, and trusts the counts only when the LAST -page it read came back empty — the same rule the status walk above already stated for itself. An empty +page before the cap, reads every page up to its 20-page cap, and trusts the counts only when the LAST +page it read came back empty — the same rule the status walk above states for the CAP, though not for termination: that walk still returns on its first empty page, and whether it needs the same change is tracked at ersatztv#893 rather than assumed (measured 2026-08-30, `/statuses/{sha}` returns a TRUE `X-Total-Count`, which this endpoint does not). **That +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 +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 timelines grow and such figures go stale within days), so an empty first page is ANOMALOUS rather than diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 89f9f3509..4f34ae8a3 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 past page 1 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); 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 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.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 8f577d8df..60de9ad9a 100644 --- a/docs/decisions/records/ci/verdict-write-retarget-fence.md +++ b/docs/decisions/records/ci/verdict-write-retarget-fence.md @@ -5,9 +5,9 @@ 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 past page 1 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); 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 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.' 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 on a validated empty page, which is a page of 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); `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 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`' +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`' --- `ci.exemption-provenance` closed three routes into the exemption path and left one residual it named: @@ -94,9 +94,13 @@ marker, so the exemption returned two events later instead of one. Only a re-pos 3. **No vocabulary tripwire.** If an upgrade renames `change_target_branch` or drops it, both counts read `0`, compare equal, are "trusted", and the protection evaporates silently. Accepted (an analogue of the jq `--expect` pin needs a live fixture PR), recorded so the silence is chosen. -4. **A timeline over the 20-page cap can never be exempted** — `rt_ok` stays `no` on every run, so only - a human verdict clears it and comment-flooding becomes a fail-closed denial of exemption. - Negligible at 1000 events; the log says so rather than promising a later run will fix it. +4. **A timeline over the 20-page cap that is READABLE to the cap can never be exempted** — `rt_ok` + stays `no` on every run, so only a human verdict clears it and comment-flooding becomes a + fail-closed denial of exemption. Negligible at 1000 events; the log says so rather than promising + a later run will fix it. **Narrowed 2026-08-30 (#870):** "over the cap" is not one state. A + timeline whose page 20 comes back EMPTY — which a filtered block at offsets 950..999 produces — is + trusted rather than refused, so the sentence above was already false for that shape before #870 + and is stated exactly now. See residual 2. **CORRECTION, 2026-08-06 (ersatztv#751).** Residual 4 above described as a narrow edge case what was in fact the universal behaviour: `rt_ok` stayed `no` on **every** pull request, not only over-cap ones, @@ -121,7 +125,8 @@ Two reasons it read as deliberate rather than broken, both worth carrying forwar The type is now read as a value (`case` over `jq -r 'type'`) rather than through `jq -e`, whose exit-status semantics already bit this workflow at jq 1.6 (`ci.jq-version-contract`), and both `null` -and `[]` terminate the walk. `test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates` +and `[]` are read as the same EMPTY shape (which since #870 ends the walk only at page 1 or at the +page cap, not in between). `test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates` is parameterised over both shapes and asserts the POSTED STATUS rather than the log line — on the real probe run the log said `Decision: state=success` and the job still posted nothing, so the decision and the write are separate events and only the write is what a merge reads. @@ -249,8 +254,7 @@ because what it leaves behind still bounds the fence. split the file list across heads. That is #803's text and settling it needs its own analysis; #849 did not attempt one. -2. **The walk's terminator WAS defeatable; closed 2026-08-30 by ersatztv#870, with a narrower - residual in its place.** `ListIssueCommentsAndTimeline` pages at the DATABASE level +2. **The walk's terminator is defeatable — NARROWED by ersatztv#870 on 2026-08-30, not closed.** `ListIssueCommentsAndTimeline` pages at the DATABASE level (`FindComments` applies the LIMIT/OFFSET) and filters AFTERWARDS, dropping every `CommentTypeCode` row and every inaccessible cross-reference into a `var apiComments []*api.TimelineComment` — a NIL slice, which serializes as bare `null`. So a page whose rows are @@ -260,11 +264,23 @@ because what it leaves behind still bounds the fence. their own PR — truncated both walks at the same place, both counts then agreed, the sha comparison agreed, and the ABA was unseen. - **The walk no longer infers the end at all.** An empty page past page 1 is SKIPPED, not believed; - the walk reads every page up to its existing 20-page cap and decides afterwards, trusting the - counts only when the LAST page it read came back empty. That is the rule the sibling status walk - already stated for itself, so the two now agree rather than one silently claiming coverage it did - not perform. "Require K consecutive empty pages" was rejected: K is arbitrary and only moves the + **The walk no longer infers the end from an empty page BEFORE its cap.** Such a page is SKIPPED + rather than believed; the walk reads every page up to its existing 20-page cap and decides + afterwards, trusting the counts only when the LAST page it read came back empty. One empty page + is therefore still trusted — the terminator at the cap — which is what keeps this a narrowing + rather than a fix. + + **The two walks do NOT now agree, and an earlier draft of this record said they did.** What they + share is only the cap treatment — "the 20th request has to be the EMPTY terminator". Their + TERMINATION rules are now opposite: `page_statuses` still returns on the FIRST empty page, and + this walk changed away from that. Whether it should follow is not settled here, because the + defect that forced this change has not been shown to exist on `/statuses/{sha}`: measured + 2026-08-30, that endpoint returns a TRUE `X-Total-Count` (105 on a 105-row head, at both + `?limit=1` and `?limit=50`), where `/issues/{n}/timeline` returns the post-filter page length. + A true total is consistent with counting before filtering and so with a terminator that means + what it says — evidence, not proof, and the reason it is only evidence is that no filtering + predicate on that endpoint has been exhibited either way. Tracked rather than asserted: + ersatztv#893. "Require K consecutive empty pages" was rejected: K is arbitrary and only moves the bar to 50K filtered rows. Three measurements at Gitea 1.27.1 (2026-08-30) rule out the cheaper fixes, and are recorded @@ -281,13 +297,36 @@ because what it leaves behind still bounds the fence. actually read" property that an anomalous empty page 1 provides. The cost of reading to the cap is 20 requests per walk, measured at 2.6s. - **What is left.** The walk confirms offsets 0..949 and requires the last page it reads to be - empty, so an event can still be hidden by a timeline carrying more than 1000 rows that ALSO - presents a fully-filtered block at offsets 950..999. That is ~1000 timeline rows instead of 50, - and it is the same over-long-history residual the status walk already accepts and documents — an - over-cap timeline fails CLOSED (`rt_ok=no`, exemption withheld), it does not pass silently. Said - here rather than left in the issue because the whole of #803 is that a contract asserting more - than its code does is worse than no contract. + **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 + offsets 950..999 returns an empty page 20, the walk never requests page 21, and it 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. 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. + + 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 + unprovable reason it used to trust one at page 2. And it is not "the same over-long-history + residual the status walk accepts": on the evidence above `/statuses/{sha}` is not known to filter + after paging, so there a non-empty page 20 is the only over-cap shape yet exhibited and it fails + closed. Only this endpoint lets a + page-20 response be empty while later pages hold rows. A timeline that fills the cap with + READABLE pages does fail closed here too; one that hides the 20th behind a filtered block does + not. + + No terminator AVAILABLE on this endpoint does better, and that is measured rather than assumed — + see the three measurements above. It is not a proof that none could exist. Closing it needs a monotonic key not served by a filtered, paged + endpoint, and none is known (#870 option 3). Deliberately NOT pinned by a test: a test asserting + that the exemption IS granted over this construction would encode a security hole as expected + behaviour and redden as a regression the day someone closes it. Said here rather than left in + the issue because the whole of #803 is that a contract asserting more than its code does is + worse than no contract. ## The status history is paged too (2026-08-28, ersatztv#763) diff --git a/docs/remote-state-inventory.md b/docs/remote-state-inventory.md index 24327e3bb..a27be2a54 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 was defeatable by a page of filtered rows until **#870** closed it (2026-08-30): the walk no longer reads an empty page as the end of the list, but reads to its 20-page cap and requires the last page to be empty, so hiding an event now costs ~1000 timeline rows instead of 50 and an over-cap timeline fails closed. 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 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/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 879843c3a..dee5e31bf 100644 --- a/scripts/tests/test_pr_changed_files.py +++ b/scripts/tests/test_pr_changed_files.py @@ -1012,6 +1012,17 @@ 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": + # 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. + rctr = out / "timeline_requests.txt" + rseen = int(rctr.read_text()) if rctr.exists() else 0 + rctr.write_text(str(rseen + 1)) + if rseen == 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 # zero retarget count from this, so the exemption is withheld. @@ -1095,8 +1106,11 @@ if "/timeline" in url: # indistinguishable to the walk on the wire, which is the entire defect, so a double that made # them distinguishable would test a server we do not have. # - # It exits BEFORE the last-real-page branch, so it does not touch `timeline_reads.txt`: that - # counter means "real pages served", and a filtered page served no rows. + # It exits BEFORE the last-real-page branch, so it does not touch `timeline_reads.txt`. Be exact + # about what that counter is, because the comment above it is not: it increments ONLY on the + # last-real-page branch, so it counts WALKS (one bump each, at the walk's last real page), not + # pages served. A filtered page must not bump it either way, or the `moves:`/`stable:` sequences + # would advance a walk early and address the wrong element. filtered = { int(x) for x in os.environ.get("STUB_TIMELINE_FILTERED_PAGES", "").split(",") if x.strip() } @@ -1787,6 +1801,7 @@ def _run_classify( push_mode: str = "none", timeline_pages: int = 1, timeline_filtered_pages: str = "", + timeline_flaky_first: bool = False, history_mode: str = "none", history_creator: str = "timothy", history_extra: list | None = None, @@ -1833,6 +1848,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_TERMINATOR"] = timeline_terminator env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape env["STUB_HISTORY_MODE"] = history_mode @@ -3663,6 +3679,80 @@ def test_a_timeline_that_FILLS_the_page_cap_is_untrusted(tmp_path): ) +def test_a_FILTERED_page_before_a_FULL_cap_page_is_still_untrusted(tmp_path): + """A filtered page must not survive as trust when the walk then runs into the cap. + + Page 19 comes back empty (filtered), page 20 is full. The walk must finish `empty=no` and refuse. + This is what makes the per-iteration `empty=no` reset load-bearing in the direction the other + fixtures cannot see: they all END on an empty page, so a stale flag and a correct one agree + there. Here they disagree — without the reset the flag is still `yes` from page 19 when the loop + falls out, and line `if [ "$empty" = yes ]` would certify a walk that never reached a terminator. + + (The reset is ALSO covered from the other side: deleting it makes the three filtered-page tests + above go red, because a stale `yes` suppresses the tally on every later non-empty page and the + hidden events stop being counted. Measured, not assumed — an early review round predicted those + three would stay green.) + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="none", + push_mode="stable:0", + timeline_pages=25, + timeline_filtered_pages="19", + ) + assert posted is None or posted["state"] != "success", ( + "an exemption was granted over a walk that hit the page cap on a NON-empty page after a " + f"filtered one — the empty flag is stale. Log:\n{r.stdout[-900:]}" + ) + assert "Could not establish a trusted retarget/push count" in r.stdout, ( + f"nothing was exempted, but NOT because the count was untrusted. Log:\n{r.stdout[-900:]}" + ) + + +def test_a_SINGLE_transient_blip_does_not_cost_the_exemption(tmp_path): + """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, + so a blip is 10x likelier — and `rt_ok=no` on the exemption path writes the STICKY sentinel, + which costs that head its exemption until a human clears it by hand. That is a regression this + change would otherwise have shipped. + + `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. + """ + posted, r = _run_classify( + tmp_path, + _emitting("docs/a.md"), + timeline_mode="none", + push_mode="stable:0", + timeline_flaky_first=True, + ) + 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:]}" + ) + + +def test_the_timeline_read_is_TIME_BOUNDED(tmp_path): + """`gh` is bare `curl -sf` with no timeout, and this walk now issues 20 requests instead of ~2. + + The third call site runs AFTER the exemption `success` is posted, so a hang there leaves the + green standing with no repair attempted. The bound is applied at THIS call rather than inside + `gh`, so the other call sites keep the behaviour they were reviewed with. Asserted structurally + because a hang cannot be provoked through the stub — the stub answers instantly by construction. + """ + body = _classify_step()["run"] + line = [ln for ln in body.splitlines() if "/timeline?limit=50&page=" in ln] + assert len(line) == 1, f"expected exactly one timeline read, found {len(line)}: {line}" + assert "--max-time" in line[0], ( + "the timeline read carries no --max-time; a hung request on the POST-WRITE walk leaves an " + f"exemption success standing with no repair attempted. Line: {line[0]}" + ) + assert "--connect-timeout" in line[0], f"the timeline read carries no --connect-timeout. Line: {line[0]}" + + @pytest.mark.parametrize("terminator", ["null", "[]"]) def test_an_EMPTY_FIRST_page_is_untrusted_WHICHEVER_empty_shape_it_is(tmp_path, terminator): """Both empty shapes must fall through to `rt_ok=no`, and only one of them did. -- 2.47.3 From 425f6b4a3146bf6e8cb6f5c9610d60dfde5f9d0b Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 30 Aug 2026 11:37:50 +0200 Subject: [PATCH 3/4] fix(870): state BOTH cost bounds, not just the healthy one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Executing the shipped walk against synthetic page sequences showed the cost sentence was telling half the truth: "20 requests per walk, measured at 2.6s" is the healthy path, but the per-page retry added in the previous commit makes the worst case 40 requests and 19 one-second sleeps. A reader sizing the change against the job's budget would have sized it against the better half. Measured by execution (the walk extracted from the YAML and run against scripted responses): a healthy walk is exactly 20 requests; an empty page 1 short-circuits at 1; a persistent failure stops at 2; a single blip costs 21. The retry's kind/raw correspondence holds on every path — a non-array first attempt followed by a failed second leaves the walk untrusted rather than parsing a stale body. Refs: #870 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/review-verdict.yml | 7 +++++-- docs/decisions/records/ci/verdict-write-retarget-fence.md | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.gitea/workflows/review-verdict.yml b/.gitea/workflows/review-verdict.yml index 02f87e038..3a4fc3674 100644 --- a/.gitea/workflows/review-verdict.yml +++ b/.gitea/workflows/review-verdict.yml @@ -1091,8 +1091,11 @@ jobs: # A `since`-bounded walk was considered and NOT taken on its own: it narrows the attack # to the run's own window but leaves the identical ambiguity inside it, and it would # cost the "a real page was actually read" property below, which depends on an empty - # page 1 being anomalous. The cost of reading to the cap is 20 requests per walk, - # measured at 2.6s against this instance. + # 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. # # 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 diff --git a/docs/decisions/records/ci/verdict-write-retarget-fence.md b/docs/decisions/records/ci/verdict-write-retarget-fence.md index 60de9ad9a..66a5be82d 100644 --- a/docs/decisions/records/ci/verdict-write-retarget-fence.md +++ b/docs/decisions/records/ci/verdict-write-retarget-fence.md @@ -294,8 +294,10 @@ because what it leaves behind still bounds the fence. row-type filter, so the paged set and the serialized set cannot be made to agree. A `since`-bounded walk was considered and not taken on its own: it narrows the attack to the run's 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 is - 20 requests per walk, measured at 2.6s. + 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. **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 -- 2.47.3 From eb9cc6cb317d9db75682f5f21a808607ef42a7d2 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 30 Aug 2026 12:17:00 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(870):=20round-2=20review=20=E2=80=94=20?= =?UTF-8?q?cost=20arithmetic=20was=20wrong=20twice,=20and=20one=20fixture?= =?UTF-8?q?=20proved=20nothing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold re-review of the fix commit (Opus, isolated worktree; the cross-family arm was unavailable, Codex having hit its usage quota mid-session). No Blocker, no High on the code — it executed the walk and every mutant of it. Every finding was in prose, cost accounting, or a test that did not discriminate. All are addressed here. COST ARITHMETIC, WRONG TWICE, BOTH NOW MEASURED RATHER THAN REASONED. - Worst-case sleeps: the comment said 19, reasoning that the last page could not sleep. It can. Executing the walk against scripted responses gives 40 requests and 20 sleeps. - Wall clock: the comment said ~335s, on the premise that only the last page can pay two timeouts. 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 continues. The pessimum is 20x(15+1+15) = 620s per walk, ~21min across the exempt path's three, and the job has no `timeout-minutes`. Stated rather than tuned, because the direction is the justification: the predecessor had no timeout at all. And it is bounded HERE only — `page_statuses` is still unbounded and runs in the same post-POST window, so the "a hang leaves the green standing" mode remains reachable there. TWO COSTS THE CHANGE DID NOT STATE, now measured on this machine: - The suite roughly doubled. `test_pr_changed_files.py`: 202s -> 474s (286 -> 294 tests, `-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; it is dated there rather than left to rot. - The exemption `success` is live from its POST until the post-write repair, and the walk in between went from ~2 requests to 20. The comment documenting that window was written when it was two round trips. A FIXTURE THAT PROVED NOTHING, caught by running the mutant it claimed to kill. The retry's coverage of a non-array BODY was pinned by a fixture serving a 502 HTML page — but jq fails on that, `|| kind=""` fires, and it takes the same path as a transport error. Narrowing the retry to `if [ -n "$kind" ]` passed it. The shape that discriminates is a well-formed JSON OBJECT, which is what this Gitea actually returns on an error (measured: `?since=NOTATIME` returns an object). Under the narrowing an error object ends the retry and is then refused by the `case`, costing the exemption. Fixture corrected, mutant now red on `[body]` and green on `[transport]` — which is the point of parameterising over both. PROSE, the same class the previous round removed six times: - The workflow stated the attacker's cost two ways 45 lines apart ("50 rows before, ~1000 now" against "50 comments either way"). Neither was right: the 50-row filtered block is unchanged, 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. Stated once now, in one unit, in all five places. - The record's residual intro said the second residual "was closed by #870" directly above a heading reading "NARROWED ... not closed". - "No terminator on this endpoint can do better" dropped both of the record's hedges. It is not a proof that none could exist: `?limit=49&page=21` reaches offsets 980..1028 with page edges that do not coincide with a 50-row walk's. Strictly better, still not a terminator. - Retired vocabulary swept: two sites still described trust as "reached a validated empty page", and one listed "a non-array body" as untrusted three lines from the paragraph explaining that a bare `null` IS the accepted empty page. - The retry's break set and the `case` arms are two enumerations of {array, null} whose agreement is what makes the stale-`kind` path harmless. Nothing said they must stay in sync; now it does. DELIBERATELY NOT DONE: no `test_MUTATION_*`-named fixture for the new clauses. The convention is for clauses disarmed in place via `_run_classify(mutate=...)`; these are page-LAYOUT properties, covered by behavioural fixtures that were each mutation-witnessed red (7 of 7, independently re-measured by the reviewer). Naming them for the convention would not add coverage. Full suite green. Fixes #870 Refs: #803, #706, #664, #751, #893 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) --- .gitea/workflows/review-verdict.yml | 68 ++++++++++++++----- docs/ci-cd.md | 10 +-- docs/decisions/README.md | 2 +- .../ci/verdict-write-retarget-fence.md | 37 +++++++--- .../testing/mutation-claims-are-executed.md | 2 +- docs/remote-state-inventory.md | 2 +- scripts/tests/test_pr_changed_files.py | 43 ++++++++++-- 7 files changed, 121 insertions(+), 43 deletions(-) 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:]}" ) -- 2.47.3