fix(803,664): fence the HEAD alias on the PR timeline's pull_push count #873

Merged

timothy merged 10 commits from fix/803-head-aba-fence into main 2026-08-29 01:29:48 +02:00

Conversation 1 Commits 10 Files Changed 13

+1217 -74

timothy commented 2026-08-28 23:29:54 +02:00

Owner

Copy Link

Copy Source

Closes the HEAD half of the changed-file enumeration's ABA hole, and corrects the contracts that
claimed it was already closed.

scripts/pr-changed-files.sh binds its paged enumeration by re-reading .base.ref, .base.sha and
.head.sha after paging and comparing them to what it expected. That is a comparison of a value
against itself, so it catches movement still in effect at the end and is blind to an alias. The BASE
alias (main -> S -> main) has been fenced since #706 by a monotonic change_target_branch count.
The HEAD alias was fenced by nothing: a force-push H1 -> H2 -> H1 across the paging round trips
leaves the final .head.sha comparison equal while the middle pages came from H2, so a mixed file
list can produce a docs-only exemption success that no single head ever justified.

fixes #803 — the head-ABA, plus the three stale contracts it carried and the advisory hook's
stale $sha.
fixes #664 — the same defect, filed independently from the #649 cold review. Fixed once.

What landed

• The fence. count_retargets becomes count_pr_mutations: one timeline walk, two tallies
  (change_target_branch and pull_push), one shared trust flag, a separate fence arm and
  diagnostic per axis. Either count moving withholds the write.
• The hook. .head.sha is re-read at the same hoist and off the same response as the base
  re-read, so the two axes cannot describe different instants. A moved head denies; an unreadable
  one asks.
• The contracts. pr-changed-files.sh's exit 0 line, its head re-read comment, and
  review-verdict.yml's call site all said the enumeration was bound to one head. They now say what
  the code supports. Sweeping by wording missed four paraphrases; sweeping by concept found them.
• Two pre-existing fail-opens in the shared walk, found by review: an empty ARRAY first page was
  trusted on any page while the null arm required page > 1, and no row was validated before
  .type was selected on.

Measured, not assumed

• Gitea 1.27.1: GET /compare/{base}...{head} still returns no files key, so pinning the diff to
  two shas remains unavailable (independently confirms #747's re-dating).
• The timeline records every push as pull_push with {"is_force_push", "commit_ids"}. PR feat(778): name the check-and-use race, and verify the protection the merge gate rests on (#802)
  carries eighteen (all false); PR #761 five (four true).
• PR #761 really went 8798a1d -> 830a407 -> 8798a1d. The alias occurs in ordinary
  force-push-and-revert; no attacker required.
• The self-trigger stall is refuted from the v1.27.1 source: the push comment is created before
  the synchronize notification is emitted, so a run always sees its own causative event. Corroborated
  by a 69s push→fence-count margin measured end to end on run 2385.

What this does NOT close — stated because the whole issue is about contracts that overclaim

The walk's null terminator is defeatable. ListIssueCommentsAndTimeline applies LIMIT/OFFSET in
FindComments and filters afterwards, dropping CommentTypeCode rows into a nil slice that
serializes as bare null — so a page of inline review comments reads as exhaustion while later pages
still hold events. That predates this change and defeats the BASE fence identically. Filed as #870
with the source evidence. The fence is documented as closing the ABA on a timeline with no such
truncating block, not the ABA outright.

Verification

1117 passed, 2 skipped (PYTHONPATH=. python3 -m pytest scripts/tests -q); ruff check + format
clean; decisions-validate: OK; YAML and bash -n clean on every touched shell body. No .cs
touched, so the BOM/format gate does not apply.

Ten mutations verified to redden the right tests: delete the head arm (4 red), value-not-movement (1),
fold the tallies (1), page-1-only walk (3), restore the [] fail-open (1), drop row validation (1),
count force-pushes only (2), drop the head count from the [] arm (1), drop the head arm's trust
guard (1), and read an unreadable head as unchanged (1).

Three review rounds: two independent cold reviews (cross-family Codex + an isolated Opus reviewer),
both BLOCKED, then a self-caught jq -e inside the one function whose own comment forbids it.

fixes #803
fixes #664

Closes the HEAD half of the changed-file enumeration's ABA hole, and corrects the contracts that claimed it was already closed. `scripts/pr-changed-files.sh` binds its paged enumeration by re-reading `.base.ref`, `.base.sha` and `.head.sha` after paging and comparing them to what it expected. That is a comparison of a value against itself, so it catches movement still in effect at the end and is blind to an alias. The BASE alias (`main -> S -> main`) has been fenced since #706 by a monotonic `change_target_branch` count. The HEAD alias was fenced by nothing: a force-push `H1 -> H2 -> H1` across the paging round trips leaves the final `.head.sha` comparison equal while the middle pages came from `H2`, so a mixed file list can produce a docs-only exemption `success` that no single head ever justified. **fixes #803** — the head-ABA, plus the three stale contracts it carried and the advisory hook's stale `$sha`. **fixes #664** — the same defect, filed independently from the #649 cold review. Fixed once. ## What landed - **The fence.** `count_retargets` becomes `count_pr_mutations`: one timeline walk, two tallies (`change_target_branch` and `pull_push`), one shared trust flag, a separate fence arm and diagnostic per axis. Either count moving withholds the write. - **The hook.** `.head.sha` is re-read at the same hoist and off the same response as the base re-read, so the two axes cannot describe different instants. A moved head denies; an unreadable one asks. - **The contracts.** `pr-changed-files.sh`'s `exit 0` line, its head re-read comment, and `review-verdict.yml`'s call site all said the enumeration was bound to one head. They now say what the code supports. Sweeping by wording missed four paraphrases; sweeping by concept found them. - **Two pre-existing fail-opens in the shared walk**, found by review: an empty ARRAY first page was trusted on any page while the `null` arm required `page > 1`, and no row was validated before `.type` was selected on. ## Measured, not assumed - Gitea 1.27.1: `GET /compare/{base}...{head}` still returns no `files` key, so pinning the diff to two shas remains unavailable (independently confirms #747's re-dating). - The timeline records every push as `pull_push` with `{"is_force_push", "commit_ids"}`. PR #802 carries eighteen (all false); PR #761 five (four true). - **PR #761 really went `8798a1d -> 830a407 -> 8798a1d`.** The alias occurs in ordinary force-push-and-revert; no attacker required. - **The self-trigger stall is refuted from the v1.27.1 source**: the push comment is created before the synchronize notification is emitted, so a run always sees its own causative event. Corroborated by a 69s push→fence-count margin measured end to end on run 2385. ## What this does NOT close — stated because the whole issue is about contracts that overclaim The walk's `null` terminator is defeatable. `ListIssueCommentsAndTimeline` applies LIMIT/OFFSET in `FindComments` and filters afterwards, dropping `CommentTypeCode` rows into a nil slice that serializes as bare `null` — so a page of inline review comments reads as exhaustion while later pages still hold events. That predates this change and defeats the BASE fence identically. Filed as **#870** with the source evidence. The fence is documented as closing the ABA on a timeline with no such truncating block, not the ABA outright. ## Verification `1117 passed, 2 skipped` (`PYTHONPATH=. python3 -m pytest scripts/tests -q`); ruff check + format clean; `decisions-validate: OK`; YAML and `bash -n` clean on every touched shell body. No `.cs` touched, so the BOM/format gate does not apply. Ten mutations verified to redden the right tests: delete the head arm (4 red), value-not-movement (1), fold the tallies (1), page-1-only walk (3), restore the `[]` fail-open (1), drop row validation (1), count force-pushes only (2), drop the head count from the `[]` arm (1), drop the head arm's trust guard (1), and read an unreadable head as unchanged (1). Three review rounds: two independent cold reviews (cross-family Codex + an isolated Opus reviewer), both BLOCKED, then a self-caught `jq -e` inside the one function whose own comment forbids it. fixes #803 fixes #664

timothy added 7 commits 2026-08-28 23:29:55 +02:00

fix(803,664): fence the HEAD alias on the PR timeline's pull_push count ... 50f2068be1

`scripts/pr-changed-files.sh` binds its paged enumeration by re-reading `.base.ref`,
`.base.sha` and `.head.sha` after paging and comparing them to what it expected. That
is a comparison of a value against itself, so it detects movement still in effect at
the end and is blind to an alias. The BASE alias (`main -> S -> main`) was fenced by
#706's monotonic `change_target_branch` count. The HEAD alias was fenced by nothing:
a force-push `H1 -> H2 -> H1` across the paging round trips leaves the final `.head.sha`
comparison equal while the middle pages came from `H2`, so a mixed file list can produce
a docs-only exemption `success` no single head ever justified.

Two issues found this independently — #664 from the #649 cold review, #803 from #778's
read-then-write inventory — which is the argument for fixing it rather than documenting
it a third time.

Measured on the live instance before choosing an option:

* Gitea is 1.27.1. `GET /compare/{base}...{head}` still returns `total_commits` and
  `commits` and NO `files` key (probed over a 12-commit range), so pinning the diff to
  two shas remains unavailable. This independently confirms #747's re-dating of the
  same claim from 1.25.4.
* The timeline records every push as a `pull_push` event carrying
  `{"is_force_push": bool, "commit_ids": [...]}`. PR #802 has eighteen (all false),
  PR #761 five (three true). An ABA is necessarily two pushes, so the count moves by
  two where the sha moves by zero.
* PR #761 really went `8798a1d -> 830a407 -> 8798a1d`. The alias shape occurs in
  ordinary force-push-and-revert; no attacker is required.
* The fence does not abstain on its own trigger. Across 20 triggered
  (push -> `pull_request_target` run) pairs on PRs #802/#834/#761 the `pull_push` event
  predates its own run's `started_at` by 26-102s. That margin is runner queue latency,
  so it is a deployment property, not an API guarantee, and is recorded as such.

So `count_retargets` becomes `count_pr_mutations`: one walk, two tallies, one shared
trust flag, a separate fence arm and diagnostic per axis. `is_force_push` is deliberately
not read — an ordinary push also invalidates a mid-flight enumeration, and an ABA's
restoring push can be non-forced when `H1` is an ancestor.

The advisory hook gets the other half of #803: `$sha` was captured once at the top and
every later check (CI status, H10 status, verdict comments) addressed it, so a push
landing across the enumeration was checked against the commit it replaced. `.head.sha`
is now re-read at the SAME hoist and off the SAME response as the base re-read, so the
two axes cannot describe different instants; a moved head denies, an unreadable one asks.

#803's floor — three contracts that asserted more than the code did — lands too:
`pr-changed-files.sh`'s `exit 0` contract line, its head re-read comment, and
`review-verdict.yml`'s call-site claim that `exit 0` means "complete and bound to $SHA".

Mutation-proved rather than asserted. Deleting the head arm reddens 4 tests; keying on
the count's VALUE instead of its MOVEMENT reddens the settled-history test (every PR has
a non-zero push count, so that mutation would withhold every exemption — the #751 shape);
folding both event types into one tally reddens the separation test. An earlier draft of
that separation test claimed to catch the folding mutation and stayed GREEN under it —
it posed only the base-moves direction — so it now poses both, and states which one is
unobservable and why.

Verification: 1109 passed, 2 skipped (`PYTHONPATH=. python3 -m pytest scripts/tests -q`);
ruff check + format clean; decisions-validate OK; YAML and `bash -n` clean on every
touched shell body. No `.cs` touched, so the BOM/format gate does not apply.

fixes #803
fixes #664
Decisions-Edit: yes

fix(803,664): round 2 — the guard-inventory row, two shared-walk fail-opens, and a false measurement ... b21cd79d60

Two independent cold reviews (Codex/GPT-5.6 cross-family, and an isolated Opus reviewer).
Both returned BLOCKED. Every finding below was reproduced before being acted on.

BLOCKER — the new test file had no `docs/guard-inventory.md` row, so
`test_the_inventory_covers_exactly_the_guards_that_exist` failed and took all 22
`test_mutation_harness.py` parametrisations down with it at fixture setup: the whole
`script-tests` CI context was red. Reproduced locally (1 failed, 22 errors). The row is
added, and the hand-maintained summary count with it (21 -> 22 proof files) — that
summary is itself asserted against the table, which is the second half of the same trap.

BLOCKER (Codex), VERIFIED AGAINST THE v1.27.1 SOURCE, not fixed here — the timeline
walk's `null` terminator does not prove exhaustion.
`ListIssueCommentsAndTimeline` applies LIMIT/OFFSET in `FindComments` and filters
AFTERWARDS, dropping `CommentTypeCode` rows and inaccessible xrefs into a NIL slice that
serializes as bare `null`. So a page of inline review comments — which a PR author can
create — reads as the end of the list while later pages still hold events, and rows are
ASCENDING so the events a fence wants are furthest from page 1. This predates #803 and
defeats the BASE fence identically, and closing it is a redesign of a walk both axes
share. Filed as #870 with the source evidence; the fence is now documented as closing the
ABA on a timeline with no such truncating block, NOT the ABA outright. Adding a fourth
overclaiming contract to the change that removed three is the one outcome not acceptable
here.

Two fail-opens in that shared walk, both pre-existing, both fixed:
* an empty ARRAY first page was trusted on ANY page while the `null` arm required
  `page > 1` — so `[]` on page 1 certified "nothing retargeted, nothing pushed" from a
  response the walk cannot explain. The comment three lines above already asserted the
  safe behaviour, so the code contradicted its own stated rule for one of the two empty
  shapes this server produces.
* no row was validated before `.type` was selected on, so a page of rows with no readable
  `.type` certified a zero count. `pr-changed-files.sh` has applied
  validate-what-you-consume to every row it extracts since #643.

FALSE MEASUREMENT — "PR #761 carries five, three of them `true`" is wrong; it is FOUR
true and one false (ids 20485/21341/21450/21452 true, 21451 false). Re-measured and
corrected in both copies.

The self-trigger stall is REFUTED FROM THE SOURCE rather than from my 20-sample margin:
v1.27.1 creates the push comment BEFORE emitting the synchronize notification, so a run
always sees its own causative event. The measurement is kept for the margin and corrected
— `started_at` is a LOWER bound, since the job runs a checkout and several steps before
counting; timed end to end on run 2385 (PR #868) the real margin is 69s, 16.7s of it
in-job. A second, DIFFERENT inversion route is now named: a PR's first push is recorded
~1s after creation and fires no `synchronize`, so an `opened` run landing in that window
would abstain with no successor. Unreachable at 26s+ queue latency; recorded, not fixed.

Three tests were passing for the wrong reason, all found by review, all now mutation-proved:
* the unreadable-head hook test asserted only that SOME `ask` happened, and this fixture
  asks anyway for an unrelated reason — so a mutation reading an unreadable head as
  unchanged left all 107 merge-consent tests green.
* every synthetic `pull_push` row carried `is_force_push: true`, making "every push is
  counted, the boolean is not read" untestable: narrowing the tally to force-pushes passed
  the whole suite. The flag now alternates.
* every fence test terminated on the default `null`, so deleting the head count from the
  empty-ARRAY arm left the head fence silently dead for `[]` timelines. Now parameterised.
And my own `.type` test passed for the wrong reason on first writing — the mode served
untyped rows on every page, running the walk into its page cap, which withholds anyway.

The overclaim sweep was incomplete: grepping the retracted WORDING missed four
paraphrases ("at one head" twice, "bound to ONE head" in the other caller of the same
script, "bound to BOTH the given sha" in a record's machine-readable mechanics line, which
left frontmatter contradicting its own body). The record no longer states a count.

New coverage: cross-page accumulation (the double could not serve a multi-page timeline at
all, so a page-1-only walk passed everything), both terminator shapes, the head arm's
trust guard, and row validation. Mutations verified to redden: delete the head arm (4),
value-not-movement (1), fold the tallies (1), page-1-only walk (3), restore the `[]`
fail-open (1), drop row validation (1), count force-pushes only (2), drop the head count
from the `[]` arm (1), drop the head arm's trust guard (1), and Codex's named hook
mutation (1).

Verification: 1117 passed, 2 skipped; ruff clean; decisions-validate OK; YAML + `bash -n`
clean. No `.cs` touched, so the BOM/format gate does not apply.

Refs #870
Decisions-Edit: yes

fix(803,664): round 3 — the row guard used jq -e inside the one function that forbids it ... cf9d274bf5

Self-caught while verifying round 2 against the jq contract, before the re-review returned.

`count_pr_mutations` deliberately avoids `jq -e`, and says so at its `kind` read: `jq -e`
reports the truthiness of its LAST OUTPUT, which is the exit-status subtlety that already
took this workflow down at jq 1.6 (ersatztv#647), and the runner still ships 1.6. Round 2's
new row-validation guard was written with `jq -e` — inside that function, three lines below
the comment forbidding it.

It was safe, but only by accident: the guard runs after `n -eq 0` has returned, so the
array is known non-empty and the empty-input divergence the header warns about could not
fire. "Safe because of a property three lines up" is exactly the reasoning the local rule
exists to retire, and it is one refactor away from being false.

Rewritten in the function's own idiom — count the offending rows with `jq -r`, range-check
the count in shell — which cannot be read two ways and matches every other tally here.
`bad` is declared `local` alongside the other scratch variables.

Also re-ran the round-2 mutation for this guard properly. The first attempt replaced the
block with an unindented `:`, which broke the YAML block scalar: the test went red on a
ParserError rather than on behaviour, proving nothing. A mutant that does not parse is not
a measurement. With a correctly indented no-op the guard's test reddens for the right
reason.

Verification: 1117 passed, 2 skipped; ruff clean; decisions-validate OK; YAML + `bash -n`
clean; `count_pr_mutations` now contains no `jq -e` call, only the comments explaining why.

Decisions-Edit: yes

fix(803,664): round 4 — a retraction left standing in four places, and a guard for the rule I broke ... 9454be1226

Third cold review (isolated Opus reviewer; no cross-family pass this round — Codex hit its
usage limit, stated rather than quietly skipped). It found NOTHING at Blocker or High, and
independently re-verified the green, the guard-inventory summary, jq 1.6's behaviour on the
new predicate, and the live Gitea probes. It confirmed round 2's "four true, one false"
correction. BLOCKED on one Medium and some Lows, all documentation.

MEDIUM — round 2 retracted a claim in one place and left it standing in four. The body of
`ci.verdict-write-retarget-fence` says the no-self-abstain property is established FROM THE
SOURCE and that calling it "a deployment property" was weaker than the truth. The `rule:`
frontmatter, the generated catalog, the workflow comment and `release.verdict-status-check`
all still carried the retracted framing — and the catalog is the DESIGNATED lookup surface,
so resolving the key gave the retracted version and only reading two screens in gave the
retraction. That is precisely the defect this PR's own round 2 named and fixed for the
head-binding overclaim; I swept for that retraction and not for my own. Corrected at every
site, and `release.verdict-status-check`'s residual now names the real one (#870) instead.

LOW — `docs/ci-cd.md` still said `null` is accepted as exhaustion "only from page 2 on", two
sentences after saying both shapes terminate the walk. Round 2 gave `[]` the identical
`page > 1` rule, so singling out `null` now reads as "`[]` is trusted on page 1" — the exact
fail-open round 2 closed. Round 2 edited three other hunks of that file and missed this one.

LOW — the head arm's notice promised unconditionally that a successor run is queued, while
the record names the window where that is false (a PR's first push fires no `synchronize`).
The record's stated detector for that inversion is "the head-fence notice in every run's
log" — the very line that would then be lying. The notice now carries the exception.

LOW — round 3 asserted "`count_pr_mutations` now contains no `jq -e` call" and nothing
guarded it. The rule was broken one commit after being quoted and caught only by hand, so
it now has a test, function-scoped (`jq -e` is legitimate elsewhere in both files) and
comment-stripped so the paragraphs explaining the rule cannot redden it. Mutation-proved
against the REAL predecessor — round 2's actual `jq -e` guard, restored verbatim — not a
hand-written mutant.

NITS — the `trusted-then-unreadable` comment described request-counting when the counter
counts real pages served (a mechanism nobody measured; the arrangement is produced either
way, proved by the mutant log); a dead `or mode == ...` arm re-initialising zeros; a
present-tense reference to the renamed `count_retargets`.

CORRECTED FIGURE — round 2 quoted "page-1-only walk (3 red)" from a `-k` subset. Measured
over the whole file it is 53, because a walk that never reaches a terminator leaves
`rt_ok=no` and withholds EVERY exemption — the #751 shape. The number is fragile and the
invariant is not, so: a page-1-only walk stops the fence trusting anything.

NOT MINE, and verified so rather than assumed: the full suite showed one red in
`test_hook_fire_log.py::test_the_suite_does_not_write_to_the_PRODUCTION_log`. That test
snapshots mtimes in `~/.cache/ersatztv/hook-fire/`, the log every live Claude session on
this machine writes to, and four peer sessions were running. It is already filed as #822
("any concurrent session reddens it"), passes in isolation (78 passed), and passed on the
final run here.

Verification: 1118 passed, 2 skipped; ruff clean; decisions-validate OK; YAML + `bash -n`
clean. No `.cs` touched, so the BOM/format gate does not apply.

Refs #870, #822
Decisions-Edit: yes

fix(803,664): round 5 — the guard I added in round 4 was a substring match ... f754ad65e6

Self-caught while pre-empting the round-4 review, which is still in flight.

Round 4 added `test_count_pr_mutations_uses_NO_jq_e` to pin a rule I had broken in round 2:
the timeline walk must read jq output as values, never through `jq -e`'s exit status,
because jq 1.6 (which the runner ships) diverges from >= 1.7 over empty input. The guard
tested `"jq -e" in line`.

That is a substring match standing in for a predicate, and it misses four spellings of the
same request: `jq -re`, `jq -er`, `jq  -e` (two spaces) and `jq --exit-status`. A guard a
rename of the flag walks straight through is not a guard — it is a comment that runs. Worse,
this one would have read GREEN on the exact reintroduction it exists to prevent.

The detector now parses the OPTION TOKENS after each `jq` occurrence: a short bundle
containing `e`, or the long `--exit-status`, and it stops at the first non-option token
because everything from there on is the jq PROGRAM, where an `e` is not a flag. The
false-positive direction matters as much as the false-negative one — a detector that
reddens on `select(.type != "e")` would make the rule unusable and get waived.

The detector is itself a string-matching predicate, so it now has its own parameterised
cases: five spellings that must trip it and four that must not, including a jq program
containing `e` and a `--arg e x` binding.

Mutation-proved on the evasion rather than only on the obvious case: reintroducing the guard
as `jq --exit-status` — which round 4's version accepted silently — reddens it.

Verification: 1127 passed, 2 skipped; ruff clean; decisions-validate OK; YAML parses and
the mutant parsed too (a mutant that does not parse is not a measurement).

fix(803,664): round 6 — withdraw the clever half of the jq detector; the retraction regenerated one paragraph below its own fix ... 7271fd9ab6

Fourth cold review, scoped to round 4. It found no Blocker and no High, and confirmed the
retraction sweep was complete, the ci-cd.md correction accurate, the notice string safe and
test-compatible, and the fixture edits behaviour-preserving.

THE DETECTOR IS NOW ITS THIRD VERSION, AND THE THIRD VERSION IS THE DUMBEST ONE ON PURPOSE.
Round 4 matched `"jq -e" in line`. Round 5 replaced that with a parse of the LEADING option
tokens, stopping at the first non-option because everything after is the jq program. Review
named two false negatives that reasoning bought: `jq --argjson e 1 -e '.'` (the walk stops at
the bare `e` argument) and `jq '.a' -e` (jq accepts options after the filter). That is three
defects in one string-matching predicate, which is this repo's documented signal to stop
adding cases and remove the cleverness. It now scans EVERY token after each `jq` with no
attempt to locate the program. That over-approximates, and the over-approximation is the safe
direction for a guard over one 36-line function we own: a false positive costs a rewrite, a
false negative restores the jq-1.6 divergence inside the code that writes a required status.
Verified no false positive against the real body — its six jq reads carry no `-`-prefixed
token but `-r`.

Proved on the regression review called MOST LIKELY rather than only the obvious one: every
read in that function is already `jq -r`, so the natural way `-e` returns is clustered onto
one of them. `jq -re 'length'` reddens the guard; so does `jq '.a' -e`.

THE RETRACTION REGENERATED ITSELF ONE PARAGRAPH BELOW ITS OWN FIX. Round 4 qualified the
self-trigger claim in the paragraph at the head of `count_pr_mutations` and in the notice, and
left the paragraph two lines further down asserting "the termination argument transfers
UNCHANGED from the base axis: a push fires `synchronize`" — the exact unqualified form, in the
paragraph a reader goes to for the safety property. Fixed, with the first-push window named
inline rather than by reference.

Also: the head-fence notice said the PR "needs a re-trigger (any edit will do)", which is
false — a label or a comment fires none of this workflow's `types:`, so a reader following
that advice would conclude the gate is broken. It now names a push or a title/body/base edit.

The brace-counting extraction could not distinguish "captured the function" from "stopped at a
balanced prefix", so a `jq -e` past an early depth-0 was invisible while the guard reported
success. It now pins a line from the END of the body.

Two doc nits: `docs/ci-cd.md` still singled out `null` in its tail after the edit that made
both shapes symmetric (and then read "an empty first page is anomalous rather than empty");
the record's rename note parsed ambiguously as "and then".

PROCESS NOTE, because it nearly cost the round: restoring a mutated file with
`git checkout -- <file>` restored it from the INDEX, silently discarding every uncommitted
edit of this round in that file. Caught by grepping for the edits rather than trusting the
restore. Mutation loops here back up with `cp` and restore with `cp`.

Verification: 1129 passed, 2 skipped; ruff clean; decisions-validate OK; YAML parses, and
every mutant parsed before being measured.

fix(803,664): round 7 — close the line-continuation evasion; three counts I asserted and never measured ... 7e8364501d

Fifth cold review returned MERGEABLE, having mutation-tested the guard rather than reading it
(`jq -e`, `-re`, `--exit-status` all red; the tail sentinel red on a synthetic early close;
0 detector hits against the real body; suite green including the #822 flake). These are its
Lows, plus one it prompted me to measure and get wrong a second time.

A CONSTRUCTIBLE MISS, and it was in the function under guard. The detector is per-line, so
`jq \` on one line and `-e 'length'` on the next carried a live `jq -e` past it. Not
hypothetical: the `bad=` read in that function is already long enough to invite exactly that
wrap. Continuations are now folded before scanning — at the CALL SITE, and the docstring says
so rather than letting "in ANY spelling" imply the helper handles it.

THREE COUNTS ASSERTED AND NEVER MEASURED, all wrong, all mine, all in docstrings added by the
last two rounds: "six jq reads" (five), "36-line function" (35 non-comment lines), and "four
spellings" missed by v1 (three — v1 catches `jq -er`, because `jq -e` is a substring of it).
Removed rather than corrected where the number carried no weight; measured where it did.

AND THE ONE I GOT WRONG TWICE. Correcting the above I wrote "the real body's jq reads carry no
`-`-prefixed token other than `-r`", then measured it: the function contains `-eq`, `-ne` and
`-le` — shell test operators, every one carrying an `e`. They are invisible only because none
shares a LINE with a `jq` invocation, which is a different and much narrower claim. Now stated
that way, with the measurement (five jq-bearing lines, none carrying a non-`-r` dash token) and
with what would legitimately trip it later (`if [ "$(... | jq -r .)" -eq 0 ]`) and what to do
then — split the line, do not make the detector clever again.

THE WITHDRAWN REASONING REGENERATED ITSELF IN THE TEST FILE, ~25 lines below the fix that
withdrew it: the comment over the false-positive rows still said "everything from the first
non-option onward is source", which is v2's rule, and those rows pass for a different reason
than it gives. That is the third time in this branch a retraction has reappeared at an adjacent
site; the pattern is that fixing the statement is not the same as sweeping the claim.

The tail sentinel is now STRUCTURAL — the capture must end on the closing brace — rather than
pinning `page=$(( page + 1 ))`, which reddened on a reformat of the increment for no reason
connected to the guard. A guard that cries wolf gets waived.

Nit: a fixture quoting the `kind=` line ended at `|| kind=` because of Python adjacent-literal
concatenation, so the double misquoted the line it claims to mirror.

Verification: 1129 passed, 2 skipped; ruff clean; decisions-validate OK; YAML + `bash -n`
clean. M18 (the reviewer's constructed continuation miss) reddens; every mutant parsed first.

timothy added 1 commit 2026-08-29 00:03:52 +02:00

fix(803,664): round 8 — the continuation fold joined with a space, which is not what the shell does ... c1007d6c6c

Sixth cold review, scoped to round 7 and the rebase. It verified the rebase resolution
independently (counting the guard-inventory table by hand rather than trusting the machine
check), confirmed every one of round 7's re-measured claims true, found the tail sentinel
sound, and ran the suite green at 1218. Two Lows, both real.

THE FOLD JOINED WITH A SPACE. Round 7 folded backslash continuations before scanning, to
close a `jq \` + `-e` split. It appended a space where bash appends nothing — and that is not
cosmetic. `jq -\` + `e '.'` is `jq -e '.'` to the shell and `jq - e '.'` to a space-joining
fold: two tokens, no flag, detector silent. Fixed by joining with nothing, which is a
simplification toward real shell semantics rather than another layer of cleverness.

BUT THE REVIEW'S THREE DEMONSTRATIONS WERE MIS-SPECIFIED, and checking that is what produced
the useful part. All three used an INDENTED continuation, and bash does not strip the
continuation line's leading whitespace: `echo -\` followed by an indented `e X` prints
`- e X` — the token does not re-form — and `jq -\` + indented `e` exits 3, not as `-e`.
Measured directly rather than argued. So those three were not evasions; the guard was right
to stay silent on them.

The reachable shape is a continuation whose NEXT LINE BEGINS AT COLUMN 0, and in this file
that is ordinary rather than exotic: YAML strips the `run:` block's common indentation (10
spaces), so any line written at the block's base indent executes at column 0. My first attempt
to construct it put the line at column 0 IN THE FILE, which simply broke the YAML — the
mutant did not parse, which is not a measurement. The correct mutant parses, passes `bash -n`,
forms a live `jq -e`, goes GREEN under the space-joining fold and RED under this one. So the
review's conclusion was right and its mechanism was not, and the fold is now documented by the
shape that actually reaches it.

The fold is extracted to `_fold_continuations` and carries its own cases, including the
indented split that must NOT be joined into a flag — the false-positive direction matters here
too, since inventing a token bash never forms would redden the guard on innocent code.

Also: "four spellings" appeared a second time in the test docstring, 55 lines below where
round 7 corrected it (v1 catches `jq -er`; it missed three). That is the fourth time in this
branch a retraction has stood at an adjacent site after being fixed at the first — the pattern
being that correcting the sentence is not the same as sweeping the claim.

Verification: 1223 passed, 2 skipped; ruff clean; decisions-validate OK; YAML + `bash -n`
clean. M22 (revert the fold to space-joining) lets the reachable evasion through, so the
separator is load-bearing and proved so.

timothy added 1 commit 2026-08-29 00:34:23 +02:00

fix(803,664): round 9 — drop the fold's rstrip(); it was the one way the guard could LOSE a jq ... 05d3e01fe0

Seventh cold review returned MERGEABLE and independently reproduced round 8's central result:
it verified the bash semantics from scratch (an indented continuation prints `- e X` and
`jq -\` + indented `e` exits 3; only a column-0 continuation re-forms the flag), and confirmed
by its own mutant table that reverting to the space-joining fold lets the reachable evasion
through. It also confirmed the shipped CI behaviour is untouched by rounds 7-9.

It left one Low and one surviving mutation, which are the same thing, so both close together.

`_fold_continuations` used `ln.rstrip()` to spot a continuation. Bash does not: a backslash
followed by whitespace escapes the SPACE, so the line does not continue. Tolerating it was the
ONLY route by which this fold could DESTROY a token rather than fabricate one — every other
divergence over-joins, which can at worst cost a false positive. The concrete shape, measured:
`ok=y\` with invisible trailing spaces, then a dedented `jq -e ...`, folds to `okyjq -e`, and
`\bjq\b` finds no word boundary after the `y`, so a live exit-status call reads as absent.

Dropping the `rstrip()` is a strict simplification toward the shell rather than another layer
of cleverness — the same direction as round 8 — so it does not restart the "3+ rounds on a
string-matching predicate, then WITHDRAW" clock. Verified both ways: with it gone, the
contrived shape above AND the genuinely reachable column-0 continuation both redden, and the
baseline stays green.

The reviewer also named the mutation that survived every fold case — dropping the `rstrip()` —
which was true because no case exercised trailing whitespace at all. That case now exists, and
it is the one that goes red first: I added it BEFORE the fix and watched it fail against the
shipped fold, then pass after. An assertion nobody has seen fail is not evidence.

Remaining known gaps, recorded rather than chased: a flag reaching jq through a variable
(`ef=-e; jq $ef`) is outside the detector's declared scope — indirection is not a spelling —
and the guard is function-scoped by design, so a helper CALLED by `count_pr_mutations` is not
covered. Both are documented choices; chasing either is the treadmill this branch withdrew
from.

Verification: 1224 passed, 2 skipped; ruff clean; decisions-validate OK; YAML ok. One file
changed, no shipped CI behaviour touched.

timothy added 1 commit 2026-08-29 01:02:13 +02:00

fix(803,664): round 10 — a raw docstring escaped as if it were not one ... 6c3e69adbc

Eighth cold review: MERGEABLE, one cosmetic nit, nothing else. It re-derived the
bash-exactness table itself, ran an 80k-input randomised differential (the predecessor loses
27 live `jq -e` sightings, this loses 0), and confirmed round 9 touches one test-only file and
leaves the shipped CI behaviour untouched.

The nit: my round-9 prose went into `_fold_continuations`' `r"""` docstring escaped as though
it were an ordinary string, so it rendered `ok=y\\` and `\\bjq\\b` — the latter naming a
DIFFERENT regex than the `\bjq\b` the code uses. Small, and exactly the class this branch
exists to remove: a claim that does not describe the code. Corrected, and verified by printing
`__doc__` rather than by reading the source, since the source is precisely what misled here.

No code change. Verification: 1224 passed, 2 skipped; ruff clean; decisions-validate OK.

timothy commented 2026-08-29 01:08:41 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ 6c3e69a

8 cold review rounds (cross-family Codex round 1 + 7 isolated Opus reviewers); final 3 verdicts MERGEABLE. Head-ABA fenced on the monotonic pull_push count; residual #870 documented, not overclaimed.

Review-verdict: MERGEABLE @ 6c3e69a 8 cold review rounds (cross-family Codex round 1 + 7 isolated Opus reviewers); final 3 verdicts MERGEABLE. Head-ABA fenced on the monotonic pull_push count; residual #870 documented, not overclaimed.

timothy merged commit 90f96c14a5 into main 2026-08-29 01:29:48 +02:00

timothy deleted branch fix/803-head-aba-fence 2026-08-29 01:29:49 +02:00

timothy referenced this pull request 2026-08-29 01:30:05 +02:00

Head-ABA: a force-push H1->H2->H1 during paging defeats pr-changed-files.sh, and three contracts still deny it #803

timothy referenced this pull request 2026-08-29 01:30:31 +02:00

`pr-changed-files.sh` head binding cannot see an A→B→A force-push across its paging round-trips #664

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#873