Compare commits

..
28 Commits
Author SHA1 Message Date
timothy 3a6174c953 Merge pull request 'docs(release): record the v26.14.0 release notes' (#739) from release/v26.14.0-notes into main
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Has been skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build CI Toolchain Image / Build & push CI image (push) Successful in 2m13s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m17s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 23m17s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m17s
Reviewed-on: #739
2026-08-05 06:13:22 +00:00
timothy 5fa672e2e5 docs(release): record the v26.14.0 release notes
Adds the v26.14.0 row to the release table. This is the commit the v26.14.0 tag
will be pushed onto, matching how v26.13.0 was cut (#716).

Contents since v26.13.0: the #726 bitmap-subtitle -readrate starvation fix
(headline), the #674/#688 decisions-validator PyYAML cross-check, the
#706/#707/#711 review-verdict raced-sentinel fix, and two Renovate bumps.

Replaces the branch behind PR #738, which never received a pull_request CI run
across six trigger attempts (two pushes, a force-push after rebase, a
close/reopen, a body edit, and a spaced push on an idle queue). A
workflow_dispatch run did complete green but writes no commit statuses, so it
cannot satisfy the required checks. Opening a fresh PR produces an `opened`
event rather than a synchronize, which is a different path.

No [skip ci] token -- this branch's merge commit is the v26.14.0 tag target.
2026-08-05 00:44:00 +02:00
timothy a2b3a56d93 Merge pull request 'fix(674,688): cross-check decision frontmatter against PyYAML; split the ceiling calibration claim' (#725) from fix/674-688-decisions-validator into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 16m55s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 22m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 22m23s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 11m16s
2026-08-04 21:43:10 +00:00
timothy 772277e255 Merge pull request 'fix(726): let a lagging realtime input catch up so a sparse bitmap-subtitle stream can't pin it below realtime' (#737) from fix/726-readrate-catchup into main
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 35s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 36s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 29s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 13m12s
2026-08-04 21:24:11 +00:00
timothyandClaude Opus 5 efc34a3481 fix(688): pin p95's inclusivity; drop a stale ratio and hedge the gap width
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 20s
PR Gates / Docs update reminder (pull_request) Successful in 32s
Review verdict / Set review-verdict status (pull_request_target) Successful in 11s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m34s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m42s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 19s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m10s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 22m46s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
PR Gates / decisions lifecycle (pull_request) Successful in 14s
review-verdict/h10 Review-verdict: MERGEABLE @ efc34a3 (base: main)
Round 7's second reviewer returned MERGEABLE on the previous head after re-measuring
every figure and running a 48-mutant battery — and reported ZERO wrong or unverified
numbers, which ends this branch's five-commit streak of them. It also independently
confirmed the round-6 adjudication: at `f394d6ce`, the sha the record cites, the #620-era
distribution really is n=167, min 2, median 26, p90 52, next value 83. All five figures
correct as written.

This commit clears its four non-blocking items.

- `marks_tail`'s UPPER inclusivity was the last meaningful surviving mutant: `ceiling <=
  p95` mutated to `<` survived the whole suite. Notice-only rather than blocking, but an
  unpinned boundary is how a documented claim quietly stops being true — the same defect
  the previous commit fixed for the coarse band. Both ends now pinned; verified the
  mutant fails.
- "the largest by ~1.6x" was TRUE at `f394d6ce` (230/147 = 1.56) and is stale today
  (230/198 = 1.16). Unlike the consolidation table two paragraphs down, that sentence was
  never scoped to a sha — so rather than re-pin a number that will rot again, it now just
  says "the longest", which stays true however the tail moves.
- The validator docstring asserted the 60->81 gap flatly; a 70-line record existed as
  recently as `8f6d4f443^`, so the gap's WIDTH is more volatile than that implied. Hedged
  to say it is the shape as measured today, not a constant. Nothing asserts it either way.
- Rewrapped a mid-sentence line break left by the previous commit.

Three surviving mutants are accepted and left: the crosscheck's not-a-mapping branch is
unreachable from any fixture, the None -> "" normalisation only matters for an explicit
YAML null no record has, and `_frontmatter_block` returning "" instead of None is a
downstream no-op.

Verification: 432 scripts/tests pass; ruff at baseline parity (47, and `ruff format
--check` at parity 9/9); validator exits 0 with no drift notice; corpus at p90=60, 18/183,
calibrated.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 23:07:15 +02:00
timothyandClaude Opus 5 57ad5efb3f fix(726): quote the decision record's rule: so PyYAML doesn't truncate it
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 19s
Review verdict / Set review-verdict status (pull_request_target) Successful in 6s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m48s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m27s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 25s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 22s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m52s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 23m52s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
review-verdict/h10 Review-verdict: MERGEABLE @ 57ad5ef (base: main)
`PR Gates / Script tests (pytest)` went red on
test_frontmatter_reader_matches_pyyaml_on_every_real_record. The rule: value was
an unquoted YAML plain scalar containing " #350's exclusion", and an unquoted
" #" starts a YAML comment -- PyYAML truncated the whole rule at "(mirroring",
so the catalog row and the mirrored MemPalace drawer carried half a sentence.

The hand-rolled frontmatter reader used by scripts/decisions_validate.py does NOT
tokenize comments, so it read the full line and reported OK; only the script-tests
job, which cross-checks the two parsers against every real record, can see this
class of defect. That is exactly what it exists for.

Fixed by single-quoting the scalar (doubling the internal apostrophe in
"image''s") and dropping the possessive from "#350's exclusion" so the token is
plain "#350". Verified both ways: PyYAML now returns the full sentence ending
"...race ahead.", and reverting the quoting reproduces the red, so the fix is
what makes the test pass rather than the test being insensitive.

Follow-up commit rather than an amend -- 56afa4652 is already pushed.

Refs #726

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 22:59:10 +02:00
timothyandClaude Opus 5 0ff9671393 fix(688): pin the minority band's constants and inclusivity; two prose corrections
Review round 7. Its adjudication of the round-6 dispute went the branch's way — measured
at `f394d6ce`, the sha the record actually cites, the #620-era distribution is n=167,
min 2, median 26, p90 52. Round 6 had measured `fefd11dff` (p90 57), a different tree.
The number stays as written.

BLOCKING FINDING: the 2%/25% constants and their inclusive boundaries were not pinned at
all. Mutating 0.02 -> 0.03, 0.25 -> 0.30, or either `<=` to `<` passed all eight
calibration tests. Those are not free parameters — they ARE the documented CI-red
thresholds, so a silent shift would quietly falsify the 38/718 figures in
docs.corpus-size-signal and docs/ci-cd.md (a strict cap reds after 37 long additions, a
strict floor after 717 short ones).

test_the_minority_band_BOUNDARIES_are_exactly_where_documented pins all four. It uses
100-record fixtures so k over the ceiling IS k%, and both 2/100 and 25/100 are exactly
representable and compare equal to the constants — true boundary cases, not near-misses.
Verified by mutation: all four now fail it.

PROSE
- corpus-size-signal said what stays blocking is "what routine growth cannot break",
  immediately before explaining that 38 routine additions break it. Now "what no SINGLE
  ordinary addition can break", which is what is actually true.
- docs/ci-cd.md said the fine claim is "never asserted"; it is never asserted AGAINST THE
  LIVE CORPUS, and IS asserted on synthetic distributions the tests own. Corrected — the
  distinction is the whole design.

Correction to an earlier commit message in this branch (9d2b30dc3, already pushed, so
recorded here rather than rewritten): it said the cross-check was clean on "all 183 real
records". 183 is the active keyed-record count; the cross-check scans the record WINGS —
195 files at that commit, 190 of them carrying frontmatter. The check was clean; the
figure named the wrong population.

Verification: 432 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with
no drift notice; corpus at p90=60, 18/183, calibrated; new record still 60 lines.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 22:52:03 +02:00
timothyandClaude Opus 5 56afa4652d fix(726): let a lagging realtime input catch up so a sparse stream can't pin it
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 10s
PR Gates / Docs update reminder (pull_request) Successful in 25s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 24s
Review verdict / Set review-verdict status (pull_request_target) Successful in 10s
PR Gates / decisions lifecycle (pull_request) Successful in 29s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m1s
PR Gates / Script tests (pytest) (pull_request) Failing after 1m53s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 19m4s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m36s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 24m58s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
-readrate throttles an INPUT and paces it off whichever of its streams is
furthest behind. An embedded bitmap subtitle (PGS/DVD) is read through the same
-i as the video -- SubtitleInputFile carries the video's path and ComplexFilter
resolves it to a stream specifier on that input, so CommandGenerator never emits
a second -i for it. Being sparse, that subtitle stream falls further behind every
second and drags video throughput down with it: measured 0.53x realtime against
the 1.0x a live client consumes at, which drains the client buffer until it
stalls. FFmpeg names the culprit itself at -loglevel warning:

  [sist#0:3/dvd_subtitle] Resumed reading at pts 10.400 with rate 6.000
                          after a lag of 0.922s   (then 1.24, 1.56, ... 3.80)

Add -readrate_catchup (6.0) to realtime video/audio inputs, capability-gated
through FFmpegKnownOption.HasOption exactly as -readrate_initial_burst is, so a
binary without it silently keeps today's behavior instead of failing to start.
The option first shipped in ffmpeg 8.0, which is NEWER than 7.1 -- hence runtime
detection rather than a version assumption. Still images and concat inputs are
excluded, mirroring #350.

Measured on prod (QSV, -threads 1, dvd_subtitle -> overlay), 45s steady-state
window after a 6s settle, replaying the captured production command line:

  baseline 1.05        0.533x  (x3 runs)
  + catchup 2.0        0.711x
  + catchup 6.0        1.067x  (x2 runs)
  + catchup 20.0       1.067x
  no subtitle overlay  1.067x  (control)

Baseline reproduces the reported 0.53x and the control the reported 1.07x, so
the harness is validated on both sides. Reproduces on software libx264 too
(0.533x -> 1.067x), as expected for an input-pacing option. Raising the base
-readrate is not an alternative and was measured: 2.0 -> 0.62x, 3.0 -> 0.80x,
4.0 -> 0.80x, 6.0 -> 0.89x -- it asymptotes below realtime because the rate
ceiling was never the binding constraint.

On #529 (readrate was incidentally bounding QSV hardware-frame allocation): the
20.0-vs-6.0 row is why 6.0 was chosen, NOT evidence about allocation -- it is a
steady-state throughput number, not a count of frames in flight. Nor is the bound
safe because read rate is allocation-irrelevant: #529 measured that it is not (at
extra_hw_frames=0, 1.05 without a burst exits 0 while 1.05+burst hits ENOMEM).
Read rate changes how fast frames enter the graph, not how deep its queues are,
and #529's failure only appeared with NO pool headroom. The 64-frame floor now
guarantees headroom, so the load-bearing measurement is row 5 of that truth
table -- no -readrate at all with 64 frames -> 14 segments, exit 0 -- and a 6x
ceiling is strictly less aggressive than no throttle. Reinforcing it,
-readrate_initial_burst 8 has read flat out at the start of every playout item
since #350, so an unbounded read here is not new. A 240s QSV soak at
QsvExtraHardwareFrames=64 across 60 segment boundaries corroborates: 1.043x
sustained, zero "Cannot allocate memory" / "Could not open encoder", RSS 166MB
vs 156MB at baseline -- corroborates rather than demonstrates, since it stayed
largely caught up.

Catchup does NOT subsume the #350 burst; measured time-to-first-segment:
-readrate alone 3.71s, +burst 0.72s, +catchup alone 3.65s, both 0.67s. They fix
orthogonal metrics.

The regression test is built on a BITMAP subtitle deliberately -- a text subtitle
is fetched by the libass filter outside the demuxer, so the same assertions would
pass vacuously while the bug is fully present. It asserts on the "[0:0][0:2]overlay"
label, which is the mechanism: subtitle stream 2 resolving onto the video's input.
Every new test was mutation-checked, each producing exactly its own expected red.

Fixes #726

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 22:50:09 +02:00
timothyandClaude Opus 5 2ff52d4236 fix(688): make the n oracle dynamic; correct a tense that asserted false history
Review round 6: one MERGEABLE with non-blocking prose, one NOT-MERGEABLE with a real
test defect. Both addressed.

THE n PIN DID NOT PIN ANYTHING. `assert n == 10` was checked against a fixture holding
exactly ten records, so a mutation returning a constant 10 for EVERY input satisfied it —
while changing the live denominator from 183 to 10, which is precisely the production
defect the test was added to close. A single hardcoded count cannot tell "counts the
input" from "returns this number". Now a dynamic oracle at two distinct cardinalities;
verified the constant-n mutation fails it.

"MOVED p90 by 21 lines" asserted a history I had not measured. 21 is TODAY's gap (60 ->
81). The actual #672 event was smaller — at that tree p90 was 60 with the next value 83,
so the 62-line record moved p90 to 62 and reddened CI with a 2-line move. The capability
claim is what matters and is true at both refs; the past tense was not. Changed to "can
move" in the two places that asserted it, which also makes all four sites agree with
docs/ci-cd.md and the validator docstring, both of which already said "could".

A REVIEW FINDING I REJECTED, having measured it. Round 6 called "p90 52" wrong for the
#620-era distribution, measuring 57. That measurement is at `fefd11dff`; the record cites
`f394d6ce`, and at THAT sha p90 is exactly 52 (n=167, min 2, median 26). The number is
correct as written and is unchanged. Recording the disagreement rather than silently
keeping it: the reviewer measured a different tree than the one the claim names.

Also corrected in this branch's own commit message trail: `b24c51ab5` said origin/main
has three 59-line records; it has four 59s and two 60s (HEAD: four and three). The claim
that survives, and the only one the code and docs now make, is that NOTHING sits between
61 and 80 at either ref — verified independently at both.

Cosmetics from the same round: a dangling modifier in ceiling_calibration's docstring, a
test_decisions_lib assertion message that said "field(s) differ" when faults can now also
be rejections, and a sentence in corpus-size-signal that named the replacement test
without saying what it asserts.

Verification: 431 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with
no drift notice; corpus at p90=60, 18/183, calibrated.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 22:35:00 +02:00
timothyandClaude Opus 5 b24c51ab51 fix(688): stop enumerating multiplicities, pin n and the keyless filter, de-couple the vacuity floors
Review round 5. One reviewer returned MERGEABLE with prose findings; the other found four
more, two of them real test gaps. Both are addressed here.

THE MULTIPLICITIES WERE WRONG AGAIN — fourth commit running. The "measured" sequence
59, 59, 60, 60 -> 81 is measured nowhere: origin/main has 59, 59, 59, 60, 60 and HEAD has
four 59s and three 60s. I had even tagged it `(measured)` in a canonical decision record.

So this stops enumerating them. All four sites now state only the load-bearing, stable
fact: the lengths climb to the ceiling and then jump STRAIGHT to 81 with nothing in
between, so one record moves p90 by 21 lines. The multiplicities change with every record
added; the gap is the point. This is the same "fix the boundary, not the site" move the
tests got three rounds ago, applied to prose that had failed four times.

TEST GAPS
- Deleting the over-tight test removed the only pin on CeilingCalibration.n: a mutation
  returning n=1 passed all 19 relevant tests while printing a wrong denominator in the
  drift notice. Pinned.
- The `if r.key` filter was load-bearing in production and unpinned: main() passes the
  UNFILTERED list (194 entries, 11 keyless, one a 106-line "Records formerly in this file"
  scaffolding block), while every test handed the function a pre-filtered list — oracle and
  production agreed only by accident. Pinned.
- The --record-ceiling 0 arm's claim that it "cannot go vacuous for any non-empty corpus"
  was FALSE: an empty record body is validator-valid and record_prose_lines returns 0, so a
  corpus of empty-bodied records has no offender at 0. Now -1, which makes the claim true.
- The three `len(recs) > 100` vacuity floors were themselves growth-coupled — 83 legitimate
  retirements would red them even with the ceiling still calibrated, which is the #688 class
  in the guard rather than the assertion. Lowered to >20 where a floor is meaningful, and to
  plain non-empty on the derived-ceiling test, whose derivations need nothing more.
- test_main_FEEDS_the_crosscheck now compares against `set(record_wing_files())` instead of
  a hardcoded basename, killing the same mutation with zero corpus dependence.

PROSE
- "ordinary growth cannot cross it — NOT immune" contradicted itself in four places. Now:
  no SINGLE ordinary addition can cross it; this is measured headroom, not immunity.
- "trimming or archiving 15" blurred two different denominators. Trimming leaves 3/183 =
  1.64%; archiving leaves 3/168 = 1.79% because the denominator moves too. Both verified,
  both under the floor, now stated separately.

Verification: 431 scripts/tests pass; ruff at baseline parity (47 — a 121-char docstring
line briefly took it to 48 and is rewrapped); validator exits 0 with no drift notice;
corpus at p90=60, 18/183, calibrated; record still 60 lines.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 22:11:25 +02:00
timothyandClaude Opus 5 c56dfdd539 fix(688): delete the last over-tight test, pin the crosscheck's INPUT, fix 4 prose defects
Review round 4, both reviewers. Both report the code path SOUND and the #688 coupling
class analytically gone (rows proved, not merely observed green); one caught 28 of 30
mutations. What blocked was one over-tight test, two unpinned mutations, and prose —
including two defects the PREVIOUS commit introduced while claiming to fix numbers.

TESTS
- Deleted test_adding_ordinary_records_cannot_RED_the_blocking_property. It appended two
  long records to the LIVE corpus and asserted flags_minority on the RESULT, so it crossed
  the cap two records before production does (56/221 vs 54/219) — a test named "cannot RED
  the blocking property" being a tighter tripwire than the property. Fourth instance of the
  #688 defect in this change. Deleted rather than tuned: both its jobs are already covered
  off live data (the synthetic v4/v5 contrast, and the deliberate live guard at the
  production threshold).
- test_main_FEEDS_the_crosscheck_the_REAL_wing_files closes a mutation hole found by
  review: replacing `pyyaml_frontmatter_faults(record_wing_files())` with `...([])` in
  main() left the ENTIRE suite green. Both existing wiring tests monkeypatch the function,
  so they pinned that its RETURN reaches errs, never that its ARGUMENT is the corpus —
  the '#609 marker that printed OK while doing nothing' defect one level up, which is the
  exact thing the new record indicts. Verified: the mutation now fails this test.
- test_main_actually_REPORTS_... went vacuous whenever the ceiling legitimately goes green
  (`False is False` passes with the whole warning branch deleted). Added an arm at
  --record-ceiling 0, which no non-empty corpus can make vacuous.
- Pinned two surviving mutations: ceiling_calibration's n_over boundary (it recomputes the
  count, so oversized_records' exclusivity test does not cover it — `>` vs `>=` differs by
  the 3 records sitting exactly on the ceiling) and p95's quantile (the 95/5 fixture cannot
  tell 0.95 from 0.99).

PROSE — two of these were introduced by the previous commit, whose stated job was fixing
numbers. That is the pattern worth naming, not the individual typos.
- "so ONE new record could move p90 lines" — the previous commit deleted the magnitude and
  left the sentence ungrammatical. Now "by 21 lines".
- It also introduced a THIRD variant of the sequence it was correcting ("60, 60, 60") and
  missed a FOURTH site in ci-cd.md still saying "twenty lines". All four sites now read the
  measured 59, 59, 60, 60 -> 81, and 21 lines.
- 59- and 60-line records were described as "above the ceiling"; they are at or below it.
- "routine growth cannot cross it" overstated the bound: it is deliberately less sensitive,
  not immune. Reworded, and the THIRD and tightest arm is now documented wherever the other
  two appear: consolidating 15 of the 18 offenders drops below the 2% floor (verified:
  3/183 = 1.64%). That is in real tension with test_oversized_records_can_go_green and is
  stated as accepted — at 3/183 the constant genuinely is mis-calibrated — with the remedy
  named: a consolidation PR that large should re-derive the ceiling in the same change.
- Corrected a docstring that called the 999-ceiling failure "silently deleting the
  assertion"; it would go red, not silent.

Verification: 430 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with
no drift notice; corpus at p90=60, 18/183, calibrated. The two new claims were measured,
not assumed: the empty-list mutation fails the new test, and 15 consolidations reaches
1.64%.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 21:47:24 +02:00
timothyandClaude Opus 5 0db56c3ebd fix(688): remove the last live-corpus tripwires and three fabricated/wrong numbers
Review round 3, both reviewers, NOT-MERGEABLE. Nothing needed rework — the code path
was found sound and mutation-sensitive (a 17-mutation battery caught every mutation with
the semantically correct test). What was left were tripwires and prose.

TRIPWIRES
- `assert need > 20` was the tightest live-corpus assertion left in the blocking job:
  it reds after 16 over-ceiling additions, while `flags_minority` — the property #688
  exists to protect — survives to 36. An arbitrary threshold on a live order statistic is
  the ratchet wearing a different hat. Removed; the measured headroom lives in prose,
  where being out of date costs a doc fix rather than someone else's red build. This also
  removes an unbounded `while` loop that HUNG the suite rather than failing it when the
  ratio could not reach the cap.
- test_main_reports_ceiling_drift hardcoded ceiling 999, which is not guaranteed above
  p95: ten valid 1000-line records make 999 calibrated and silently delete the test's only
  assertion. Now derived as max+1, off the tail by definition.
- test_main_actually_REPORTS_the_ceiling_and_the_trend required >=1 over-ceiling record.
  The ceiling is ALLOWED to go green (test_oversized_records_can_go_green says so), so
  that would red the blocking job the day someone consolidates the last offender —
  punishing exactly the work the warning asks for. Restated as an IFF.
- test_no_budget_flag_means_no_retirement_warning asserted no bare "RETIRED" in stderr; a
  legitimate stale record whose TITLE contains the word reds it. Matched precisely now.
- Added the >100-record vacuity guard its siblings carry to the derived-ceiling test.

NUMBERS — all three were mine, and two are the failure mode this repo calls worse than
no note at all (a confident claim that was never measured):
- "the lengths above the ceiling ran 60, 61, 62, 63 then jumped to 81" is FABRICATED. No
  record of 61, 62 or 63 lines exists at origin/main, at the #672 sha, or at the #706 sha.
  Measured, the sequence is 59, 59, 60, 60 then 81 — a 21-line jump, so the conclusion was
  if anything understated. Corrected in all three places it was repeated, including the
  canonical v4 row of docs.corpus-size-signal.
- The crosscheck record called `decisions-guard` a REQUIRED check — introduced by the
  previous commit in the sentence rewritten to fix an overclaim. Verified against Gitea
  branch protection: `main` requires exactly `Build & test (.NET)`, `EF migration
  integrity` and `review-verdict/h10`. NEITHER script-tests NOR decisions-guard is
  required; the record now says so.
- docs.corpus-size-signal said 37 additions "to reach" the cap two paragraphs above 38
  "below the cap" — a same-document numeric inconsistency of exactly the class this change
  set out to remove. Both now state 38 to BREACH, noting 37 lands on 0.25 and passes.
- Also: the old bound's accepted range is 39..229 (not 43..229 — 43 is the NEW bound's
  lower edge); "95% over the ceiling" was 100%; `oversized_records` said the #620
  distribution began at 0 lines where the record itself says 2.

Verification: 428 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with
no drift notice; corpus at p90=60, 18/183 over the ceiling, record trimmed to 60 lines so
main ships calibrated.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 21:26:52 +02:00
timothyandClaude Opus 5 b3a8826281 fix(688): stop asserting live-corpus order statistics anywhere in the suite
Review round 2 (both reviewers, independently) found the round-1 fix incomplete: the
live-corpus coupling survived in two more tests. This is the THIRD instance of one
defect class in this change, so the fix is to remove the coupling rather than patch
another site.

BLOCKER — test_adding_ordinary_records still asserted live order statistics. The
`if before.marks_tail:` guard made the PRECONDITION conditional but left the
CONCLUSION (`assert not after.marks_tail`) an assertion about the live p90. Verified:
appending 16 ordinary 30-line records — nothing long, nothing unusual — makes both
sides true and fires it, reddening the blocking job for an unrelated author. Exactly
what #688 exists to abolish.

The v4-vs-v5 contrast moved to test_v4_would_have_reddened_where_v5_holds, built on a
distribution the test OWNS, reproducing the shape that matters (a sparse gap just above
the ceiling). The real-corpus test now asserts only the robust claims: the additions
were counted, v5 holds, and the measured headroom.

Same treatment for the "bad ceiling" teeth test, which hard-coded that 200/229/230 stay
rejected on the live corpus — three new 200+ line records flip it. Teeth now demonstrated
synthetically; the only live-corpus assertion left is that today's ceiling is accepted,
which needs 38 over-ceiling or 718 short additions to break.

The IFF drift test could lose its quiet branch: one 61-line record makes BOTH the 60 and
999 ceilings drift, at which point an UNCONDITIONAL notice would pass. Both ceilings are
now DERIVED — p90 itself (always calibrated, since p90 <= p90 <= p95) and max+1 (always
off the tail) — so each branch is guaranteed by construction, and the test asserts it
exercised both.

Added the missing regression test for the typed-mapping-key TypeError: removing `key=str`
now fails a test instead of only a manual probe.

Corrected against measurement: the v5 row of the record's own version table still stated
the REJECTED first-draft bound (`0 < f < 1/3`) — the canonical artefact contradicting both
the code and its own next paragraph; accepted range is 43..180, not "roughly 45..150";
breaching the cap takes 38 additions, not 37 (37 lands exactly on 0.25, which passes under
`<=`); "5x headroom below the floor" was inverted; ci-cd.md said four versions "all
ratcheted" when v1 was vacuous and v2 accepted an absurd ceiling; and the crosscheck record
overstated protection — script-tests is NOT a required check, so "no broken record has
reached main" is procedural, not structural.

Evidence the coupling is actually gone: mid-fix the corpus sat at marks_tail=False (an
edit pushed this branch's own record to 61 lines, moving p90) and the suite stayed fully
green. Under the old assertions that state reddened CI. The record is trimmed back to 60
so main ships calibrated and no drift notice nags.

Verification: 428 scripts/tests pass; ruff at baseline parity (47); validator exits 0 with
no drift notice; corpus at p90=60, 18/183 over the ceiling.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 21:02:01 +02:00
timothyandClaude Opus 5 80818aa294 fix(674,688): address independent review — restore the coarse bound's teeth
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 33s
PR Gates / Docs update reminder (pull_request) Successful in 31s
PR Gates / decisions lifecycle (pull_request) Successful in 41s
Review verdict / Set review-verdict status (pull_request_target) Successful in 12s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m14s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m32s
PR Gates / Script tests (pytest) (pull_request) Successful in 2m8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m53s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 20m30s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 23m19s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Two independent cold reviews (one cross-family) agreed on the top two findings.

1. The #688 fix was defeated by its own complement test. test_main_is_QUIET_about_
   drift asserted the drift notice was ABSENT while running main() over the LIVE
   corpus — whose failure condition is bit-for-bit v4's assertion, in the same
   blocking job, three functions down. p90 sat exactly on 60, so one over-ceiling
   record would have reddened it. Replaced with an IFF test that uses
   ceiling_calibration as its oracle, so it asserts the WIRING rather than the
   corpus's current state, plus a guard that at least one branch fires.

2. The coarse bound was nearly unfalsifiable. `0 < fraction_over < 1/3` accepted
   EVERY ceiling from 39 to 229 on the real corpus — including the ceiling of 200
   my own docstring offered as the case it catches, because one 230-line record
   keeps the count nonzero. That claim was simply false and is corrected. The floor
   is now a FRACTION (2%) and the cap 25%, which rejects 200/229/230 and 20, and
   accepts roughly 45..150. Headroom measured, not estimated: 37 consecutive
   over-ceiling additions, against ONE record to break v4.

3. yaml.safe_load raises a bare ValueError, not a YAMLError, on a well-shaped but
   impossible date (stale-after: 2026-06-31), which escaped as a traceback and
   killed the validator on any machine with PyYAML. The except is now deliberately
   broad, with a test.

4. PyYAML returns TYPED mapping keys, so a stray `1: x` made sorted(set|set) raise
   TypeError. Sorted with key=str.

5. The headroom prose was arithmetically wrong (~42/~40 where the real values are
   63/64; each addition moves numerator AND denominator) and the record counts were
   stale. Corrected against measurement.

6. test_adding_ordinary_records passed identically with its two additions removed.
   It now asserts the additions were counted, and that they break the v4 property
   while leaving v5 satisfied — guarded by `if`, never asserted, since whether v4
   currently holds is a fact about the live distribution and asserting it would
   rebuild the ratchet.

Also recorded honestly in docs.frontmatter-pyyaml-crosscheck: decisions-guard
installs no PyYAML, so in CI the cross-check always skips and script-tests already
caught both hazards — the CI delta is close to zero and the real fix is the local
loop plus the tool/suite agreement. And the new record was trimmed 64 -> 58 prose
lines: at 64 it moved p90 to 64 by itself, i.e. this PR would have reddened the old
blocking job. That is now cited in the test as the live demonstration.

Verification: 426 scripts/tests pass; ruff at baseline parity (47); validator exits
0; corpus back to p90=60, 18/183 over the ceiling, marks_tail and flags_minority
both true.

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 20:37:27 +02:00
timothyandClaude Opus 5 9d2b30dc3b fix(674,688): cross-check frontmatter against PyYAML; split the ceiling calibration claim
Two defects in scripts/decisions_validate.py, fixed together because they share the
validator and its pytest suite.

#674 — the validator reported OK on frontmatter PyYAML rejects. The hand-rolled reader
is deliberately dependency-free (decisions-guard and the Husky hooks install nothing),
so it cannot see a bare apostrophe closing a single-quoted scalar. Hit twice in one
session by two independent agents. `pyyaml_frontmatter_faults()` now cross-checks the
parse against PyYAML whenever PyYAML is importable, and is SKIPPED with a ::notice::
when it is not — the read path stays dependency-free.

The two known hazards fail differently and the fix covers both: the apostrophe makes
PyYAML reject the document, while an unquoted ` #` parses fine and silently TRUNCATES
the value. So the check compares parsed results key by key rather than try/except-ing
the load, which is also what makes it generalize past the two known characters. PyYAML
wrote these files, so on disagreement it is authoritative and the file is the defect.
The comparison has one implementation, called by the validator and by the existing
test_decisions_lib agreement test, so the tool and the suite cannot drift.

#688 — test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY asserted p90 <= 60 <= p95 in
the BLOCKING script-tests job. p90 sat exactly on the ceiling and the distribution above
it is sparse, so one ordinary record moved p90 by twenty lines and reddened CI for
whoever wrote it; it reproduced twice live (#672, #706) and both times the only in-scope
remedy was trimming the new record to fit the constant.

v5 splits the claim by robustness instead of hunting for a better single assertion. The
blocking test now asserts only the coarse, non-ratcheting property (the ceiling flags a
nonempty proper minority, 0 < fraction_over < 1/3); the fine tail-boundary claim is
measured every run and REPORTED as a ::notice::, on the same reasoning stale_records
already uses — a constant going out of date is the passage of corpus growth, not a
defect in the commit under test. The fine property is still asserted, against synthetic
distributions the test owns. The ceiling stays 60.

Verification: 424 scripts/tests pass; ruff at baseline parity (47 before and after);
the cross-check is clean on all 183 real records; a positive control pins that
record_wing_faults alone still reports both hazard files as clean, so the new red cannot
pass for the wrong reason; and a test demonstrates that appending #672's 62-line and
#687's 107-line records to the real corpus does not red the blocking property.

Docs: new record docs.frontmatter-pyyaml-crosscheck, docs.corpus-size-signal updated for
the v5 split, catalog regenerated, docs/ci-cd.md updated for both.

fixes #674
fixes #688

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 20:15:16 +02:00
timothy aa79ec59c9 Merge pull request 'fix(706,707,711): fence the review-verdict write on the timeline retarget count' (#723) from fix/706-verdict-status-serialization into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 17s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 30s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 9m31s
2026-08-03 21:01:12 +00:00
timothy fe3d29276a fix(706): count a raced SENTINEL, not only a raced human verdict
PR Gates / decisions lifecycle (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 20s
PR Gates / Docs update reminder (pull_request) Successful in 21s
Review verdict / Set review-verdict status (pull_request_target) Successful in 7s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m29s
review-verdict/h10 Review-verdict: MERGEABLE @ fe3d292 (base: main)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m54s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 15m3s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 15m49s
Round-5 cold review: the post-write check counted only human `Review-verdict:`
rows above the high-water mark, which is not sufficient under the run overlap
this branch measured.

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

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

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

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

Refs #706
2026-08-03 22:22:09 +02:00
timothy a8bbd74a64 fix(706): never replace a sentinel with a non-sentinel
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 15s
PR Gates / decisions lifecycle (pull_request) Successful in 20s
review-verdict/h10 Awaiting review verdict for a8bbd74
Review verdict / Set review-verdict status (pull_request_target) Successful in 8s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m31s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m52s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 12s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 6s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m39s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 20m22s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Round-4 cold review: the mid-run sentinel guard tested `state = success`, which
is one branch too narrow. A run can reach the POST on `state=pending` carrying
the GENERIC description — most realistically after a transient enumeration
failure (`complete != yes`) — and such a run passed the success-only guard,
passed the fence, and overwrote the sentinel with ordinary text. The next run
then saw no sentinel, re-derived, and posted `success`: the same buried human
rejection as the round-2 defect, reached in two steps instead of one.

The guard now compares the DESCRIPTION rather than the state:

    if [ "$ex_repair" = yes ] && [ "$desc" != "$REPAIR_DESC" ]

"Never replace a sentinel with a non-sentinel." This is strictly more general
and exactly as precise, because the carry-forward branch guarantees that a
sentinel seen at the FIRST read already sets `desc` to the sentinel — so the
guard cannot fire on the ordinary repaired-head path and the fixed point stays
intact.

It also makes the code match the decision record, which already stated the
general property ("a run whose last-moment re-read finds a sentinel it did not
see at its FIRST read ABSTAINS instead of posting") while the code implemented
only the success case. Of the two, the code was the one that had to move.

Mutation-verified on both clauses independently: reverting to the success-only
condition reddens the new pending-path test; dropping the description check
reddens the fixed-point test and the sentinel-from-the-start control.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

fixes #706
fixes #707
fixes #711

Decisions-Edit: yes
2026-08-03 21:41:16 +02:00
timothy 7cd71327b0 Merge pull request 'chore(deps): update dependency cliwrap to 3.10.4' (#717) from renovate/cliwrap-3.x into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 18s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 31s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 11m9s
Reviewed-on: #717
2026-08-03 19:28:23 +00:00
timothy 53e6be8390 Merge branch 'main' into renovate/cliwrap-3.x
PR Gates / Docs update reminder (pull_request) Successful in 22s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 19s
review-verdict/h10 Exempt: authored by the 'renovate' bot account, touches no protected path, and changes only dependency manifests
Review verdict / Set review-verdict status (pull_request_target) Successful in 24s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m14s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 58s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m6s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 9m3s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 16m7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 19m56s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
PR Gates / decisions lifecycle (pull_request) Successful in 16s
2026-08-03 18:50:54 +00:00
timothy b99812eb8b Merge pull request 'chore(deps): update dependency jetbrains.resharper.globaltools to 2025.3.5' (#718) from renovate/jetbrains.resharper.globaltools-2025.x into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 41s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 45s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 45s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 13m2s
Reviewed-on: #718
2026-08-03 18:50:39 +00:00
timothy 03662dcdfd Merge branch 'main' into renovate/cliwrap-3.x
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 51s
PR Gates / Docs update reminder (pull_request) Successful in 47s
review-verdict/h10 Exempt: authored by the 'renovate' bot account, touches no protected path, and changes only dependency manifests
PR Gates / Script tests (pytest) (pull_request) Successful in 1m15s
Review verdict / Set review-verdict status (pull_request_target) Successful in 24s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m37s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m8s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 13s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 20m31s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
2026-08-03 18:13:53 +00:00
timothy aaa4e869b8 Merge branch 'main' into renovate/jetbrains.resharper.globaltools-2025.x
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 24s
PR Gates / Docs update reminder (pull_request) Successful in 20s
PR Gates / decisions lifecycle (pull_request) Successful in 32s
review-verdict/h10 Exempt: authored by the 'renovate' bot account, touches no protected path, and changes only dependency manifests
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m39s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m4s
Review verdict / Set review-verdict status (pull_request_target) Successful in 2m7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 18m27s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 23m8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 24m23s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
2026-08-03 18:12:59 +00:00
renovate 95f36d1c45 chore(deps): update dependency jetbrains.resharper.globaltools to 2025.3.5
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 15s
review-verdict/h10 Exempt: authored by the 'renovate' bot account, touches no protected path, and changes only dependency manifests
PR Gates / decisions lifecycle (pull_request) Successful in 24s
Review verdict / Set review-verdict status (pull_request_target) Successful in 24s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m3s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m42s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 9s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 20m8s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 20m48s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
2026-08-03 17:02:21 +00:00
renovate ff8b0bf984 chore(deps): update dependency cliwrap to 3.10.4
PR Gates / decisions lifecycle (pull_request) Successful in 30s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 42s
PR Gates / Docs update reminder (pull_request) Successful in 49s
review-verdict/h10 Exempt: authored by the 'renovate' bot account, touches no protected path, and changes only dependency manifests
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m0s
Review verdict / Set review-verdict status (pull_request_target) Successful in 30s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m1s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 32s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 21m52s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 25m25s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 28m5s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
2026-08-03 17:02:11 +00:00
24 changed files with 2144 additions and 82 deletions
+1 -1
View File
@@ -3,7 +3,7 @@
"isRoot": true,
"tools": {
"jetbrains.resharper.globaltools": {
"version": "2025.3.4.1",
"version": "2025.3.5",
"commands": [
"jb"
],
+315 -2
View File
@@ -183,6 +183,14 @@ jobs:
set -euo pipefail
CONTEXT="review-verdict/h10"
# The description this job writes when it repairs its own raced exemption (#706 race 2).
# It is a SENTINEL, not just a message: `read_existing_verdict` recognises it, and the
# classification below refuses to post `success` over it. Without that, the repair lasted
# exactly one event — the next run saw a machine-written `pending`, re-derived it, and
# posted `success` again, with its own freshly-taken high-water mark now ABOVE the human
# row, so the post-write check stayed silent and the rejection went green a second time.
# Found by cold review. Refusing here can only ever withhold an exemption, never grant one.
REPAIR_DESC="Human verdict raced this exemption write — re-post the verdict"
# Accounts whose PRs may merge without a human verdict. Renovate only — keep this list
# minimal and explicit; every entry is an account that can land code unreviewed.
BOTS="renovate"
@@ -218,7 +226,23 @@ jobs:
BOT_MANIFESTS='^(Directory\.Packages\.props|\.config/dotnet-tools\.json)$'
# Paths where NEITHER exemption applies, because a change here can alter the gate itself,
# what CI runs, or what the hooks enforce.
PROTECTED='^(\.claude/|\.gitea/|\.husky/|scripts/|docker/ci/)'
#
# `.codex/` is listed alongside `.claude/` (ersatztv#711). `.codex/hooks/` is a
# byte-identical mirror of `.claude/hooks/` — including `pretooluse-merge-consent.sh` —
# generated as the Codex-side port of the same enforcement hooks. Without it the rule "a PR
# that can weaken the gate must not exempt itself from the gate" was expressed as a path
# list that had gone incomplete: editing `.claude/hooks/pretooluse-merge-consent.sh`
# correctly voided the exemption while editing its `.codex/` twin did not. Today that is
# LATENT rather than live — `.codex/` is untracked and gitignored, and a PR cannot touch a
# path that is not in the repo — but it becomes live the moment anyone tracks it, which is
# the natural instinct given `.claude/` is tracked. Listed now because the cost is one
# alternation and the failure mode is silent.
#
# The list stays ENUMERATIVE rather than derived (e.g. "any dotted top-level directory
# containing executable hooks"). A derived rule has to be evaluated against the PR's own
# file list, which is the very thing being classified — more moving parts inside a security
# predicate, to remove a maintenance burden that is one line per new tooling directory.
PROTECTED='^(\.claude/|\.codex/|\.gitea/|\.husky/|scripts/|docker/ci/)'
# Docs-only: prose and decision records. Deliberately narrower than the hook's pattern,
# which also lets .claude/.gitea/.husky through — that carve-out is safe there only
# because it falls through to a HUMAN PROMPT, whereas here it would post a green status
@@ -330,6 +354,10 @@ jobs:
# no `(base: …)`, and re-deriving over one would un-approve a genuinely reviewed head. Only
# a base that is PRESENT and DIFFERENT is rejected, which is exactly the escape above.
ex_human=no
ex_repair=no
case "$ex_desc" in
"$REPAIR_DESC"*) ex_repair=yes ;;
esac
case "$ex_desc" in
"Review-verdict:"*)
if [ -n "$ex_creator" ]; then ex_human=yes; fi
@@ -374,6 +402,72 @@ jobs:
fi
}
# --- The retarget fence (ersatztv#706 race 1) ----------------------------------------
# THE PROBLEM THIS SOLVES. Two `pull_request_target` runs for one PR overlap, and the OLDER
# one can finish LAST — so a run that classified against a base the PR no longer targets can
# post its stale answer over a fresher run's correct one, permanently. Measured on this
# instance rather than assumed: probe PR #722 run 7520 (`opened`) ran to completion 20s AFTER
# run 7521 (`synchronize`) had started.
#
# WHY NOT A CONCURRENCY GROUP, which is the obvious answer and what #706 proposed. It does
# not work here, also measured: with `concurrency: {group: …-${{ pr number }},
# cancel-in-progress: false}` active on an identical probe, runs 7528 and 7529 still ran
# CONCURRENTLY and 7528 ended 36s after 7529 began. Gitea 1.25.4 does auto-cancel superseded
# `push` runs on a branch — a negative control with no `concurrency:` key at all showed that —
# but that behaviour does NOT extend to `pull_request_target`. `cancel-in-progress: true` is
# deliberately untried: cancellation is the one thing this workflow's own header refuses,
# because a cancelled run leaves an EXEMPT PR statusless with nothing left to re-trigger it.
#
# WHY A COUNTER AND NOT THE BRANCH NAME. The attack is an ABA: `main → S → main`. Every
# name-based check reads `main` at both ends and passes, which is exactly how route 1 got a
# forged exemption. Gitea's issue timeline records each retarget as a `change_target_branch`
# event with `old_ref`/`new_ref`; the COUNT of those events is monotonic and cannot alias.
# Verified on the real route-1 reproduction (PR #703: two events, `main → probe698/base-S` at
# 18:17:29 and back at 18:18:31) with a negative control (PR #717, never retargeted: zero).
#
# WHY ABSTAINING IS NOT A STALL — the property the whole design rests on. A retarget always
# fires `edited`, which is in this workflow's `types:` (see the header). So the very event
# that makes this run abstain has already queued a successor whose window opens after it.
# Abstention hands off; it does not drop the PR. The induction terminates when retargeting
# stops, and the last run has a clean window and writes the final answer. This is why the
# fence does not reintroduce the statusless-exempt-PR failure that rules out cancellation:
# it never stops a run from RUNNING, only from WRITING state it knows is stale.
#
# `updated_at` was considered as the key and rejected: it moves for comments and labels,
# which fire none of this workflow's `types:`, so a run could abstain with no successor
# coming — a real stall. The retarget count moves only for the mutation that actually
# invalidates a classification, and that mutation always brings its own re-run.
#
# Completeness is a guard, not an assumption (`ci.paged-endpoint-completeness`): 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.
count_retargets() {
rt_count=0
rt_ok=no
local page=1 raw n m total=0
while [ "$page" -le 20 ]; do
raw=$(gh "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || return 0
if [ -z "${raw//[[:space:]]/}" ] || ! printf '%s' "$raw" | jq -e 'type == "array"' >/dev/null 2>&1; then
return 0
fi
n=$(printf '%s' "$raw" | jq -r 'length')
case "$n" in ''|*[!0-9]*) return 0 ;; esac
if [ "$n" -eq 0 ]; then rt_ok=yes; rt_count=$total; return 0; fi
m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length')
case "$m" in ''|*[!0-9]*) return 0 ;; esac
total=$(( total + m ))
page=$(( page + 1 ))
done
return 0
}
ex_repair=no
count_retargets
retargets_before=$rt_count
retargets_before_ok=$rt_ok
echo "Retarget fence: ${retargets_before} retarget event(s) observed before classifying (trusted=${retargets_before_ok})."
# --- Whose verdict is it? (ersatztv#698 route 3) -------------------------------------
# This short-circuit used to exit on ANY existing `success`, which made an exemption this job
# wrote indistinguishable from a verdict a human wrote. That is what let a forged exemption
@@ -508,33 +602,252 @@ jobs:
fi
fi
if [ "$ex_repair" = yes ]; then
# A previous run of this job already repaired a raced exemption on this sha, which means a
# human verdict was written for it and then lost. Re-granting the exemption would bury that
# rejection again. The PR needs a real verdict; only a human can clear this.
exempt=no
reason="a human verdict raced a previous exemption write on this head and was overwritten — this head needs a re-posted verdict, not another exemption"
fi
if [ "$exempt" = yes ]; then
state=success
desc="Exempt: $reason"
elif [ "$ex_repair" = yes ]; then
# CARRY THE SENTINEL FORWARD. This branch exists because the first version of it did not,
# and cold review reproduced the consequence: refusing the exemption but posting the
# GENERIC pending description overwrote the very sentinel the refusal depends on, so the
# next run saw an ordinary machine `pending`, re-derived it, and posted `success` — burying
# the human rejection two events after the repair instead of one. The block has to be a
# FIXED POINT: what this branch writes must be what re-triggers this branch.
#
# It is keyed on `ex_repair` alone rather than on the exempt path, so the marker also
# survives runs where the PR was not exemptible anyway — the fact being recorded is "a
# human verdict was lost on this sha", which is a property of the sha, not of this run's
# classification.
state=pending
desc="$REPAIR_DESC"
else
state=pending
desc="Awaiting review verdict for ${SHA:0:7}"
fi
echo "Decision: state=${state} — ${reason}"
# HIGH-WATER MARK for the post-write verification (ersatztv#706 race 2). Taken FIRST — before
# the re-read below, before the fence, before the POST — and the ORDER IS THE POINT.
#
# An earlier version captured it just before the POST, "as late as possible". Cold review
# caught that as a High: everything between the re-read and a late mark is a blind gap. A
# human verdict landing there is invisible to the re-read (which already happened) AND
# excluded from the post-write check (its id is BELOW a mark taken afterwards), so it is
# silently overwritten with no repair. That gap spans the entire retarget re-count — up to 20
# timeline round-trips — so it was far wider than the one-round-trip residual being claimed.
#
# Taking the mark first closes the read side completely: any row newer than the mark is caught
# either by the re-read (abstain, post nothing) or by the post-write check (repair). There is
# no false-fire cost to being early, because the test is `id > mark` — rows already present
# when the mark is taken are below it and stay invisible either way.
#
# Presence alone would be the wrong test, and wrong in the direction that breaks the gate: the
# short-circuit deliberately does NOT stop for a human verdict whose recorded base does not
# match this PR's (`ex_human` is reset to `no` — see `read_existing_verdict`). Such a row stays
# in the history forever, so a presence test would fire on EVERY later run of that PR,
# downgrade every exemption to `pending`, and deadlock it permanently.
#
# `max` over an empty array is `null`, hence `// 0`. `.id? // 0` rather than `.id`: a bare
# `.[].id` hard-errors under `set -e` if the array ever holds a non-object, which would kill
# the job before it posts and strand an ordinary PR with no status at all.
max_id_before=-1
hist_before=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=100") || hist_before=""
if [ -n "${hist_before//[[:space:]]/}" ] && printf '%s' "$hist_before" | jq -e 'type == "array"' >/dev/null 2>&1; then
mark=$(printf '%s' "$hist_before" | jq -r '[.[] | .id? // 0] | max // 0' 2>/dev/null || true)
case "$mark" in
''|*[!0-9]*)
# SKIP the check rather than treat everything as raced. A mark of 0 would make every
# pre-existing human row look newer than the mark and repair every exemption away.
echo "::warning::Status high-water mark for ${SHA:0:7} was not numeric ('${mark}'); the post-write race check will be skipped."
max_id_before=-1 ;;
*) max_id_before=$mark ;;
esac
else
# Not fatal: the POST below is still correct, only the after-the-fact verification is
# weakened. Recorded so a silent degradation is visible in the log.
echo "::warning::Could not establish a status high-water mark for ${SHA:0:7}; the post-write race check will be skipped."
max_id_before=-1
fi
# LAST-MOMENT RE-READ (ersatztv#706). Classification takes several API round-trips, and a
# reviewer can post a verdict during them — most dangerously a `failure`, which this job would
# then overwrite with an exemption `success`, turning an explicit human rejection green. The
# first read cannot see that; this one can. It NARROWS the window, it does not close it: there
# is no compare-and-set on Gitea's status API, so a verdict landing between this read and the
# POST below is still lost. Said plainly here rather than left as an implied guarantee.
# POST below is still lost — which is what the post-write repair below is for.
read_existing_verdict
if [ "$ex_human" = yes ]; then
echo "::notice::A human verdict ('${ex_state}' by '${ex_creator}') landed on ${SHA:0:7} while this job was classifying — leaving it alone and posting nothing."
exit 0
fi
# A SENTINEL THAT APPEARED MID-RUN (ersatztv#706, round-3 review). The re-read above recomputes
# `ex_repair`, and until this guard existed nothing downstream read it: the POST writes the
# `$state` frozen at classification time, so a STALE OVERLAPPING RUN would post its `success`
# straight over a sentinel another run had just written — burying a human rejection, with no
# repair (the human row is below this run's mark) and no log. That fails toward SUCCESS, so it
# was not covered by the "repair fails toward pending" residual; it is the exact outcome this
# whole change exists to prevent, reached through the run overlap this branch itself measured.
#
# THE RULE IS "NEVER REPLACE A SENTINEL WITH A NON-SENTINEL", not "never overwrite it with a
# success". A first draft of this guard tested `state = success`, which is one branch too
# narrow: a run can reach the POST on `state=pending` carrying the GENERIC description — most
# realistically after a transient enumeration failure (`complete != yes`) — and that run
# passes a success-only guard, passes the fence, and overwrites the sentinel with ordinary
# text. The next run then sees no sentinel, re-derives, and posts `success`: the same buried
# human rejection as before, reached in two steps instead of one.
#
# Comparing the DESCRIPTION rather than the state is exactly as precise and strictly more
# general. A sentinel present at the FIRST read forces `desc="$REPAIR_DESC"` (the carry-forward
# branch in the decision above), so this guard cannot fire on the ordinary repaired-head path
# and the fixed point is intact. Any other description alongside `ex_repair=yes` means the
# sentinel arrived DURING this run, whatever this run concluded.
#
# Abstaining is strictly correct here and, unlike the retarget fence, needs no successor run:
# the sentinel row is already `pending` and already carries the re-post instruction.
if [ "$ex_repair" = yes ] && [ "$desc" != "$REPAIR_DESC" ]; then
echo "::notice::A repair sentinel was written on ${SHA:0:7} while this job was classifying, meaning a human verdict was raced and repaired by another run. This run's exemption is stale — posting NOTHING and leaving the sentinel standing."
exit 0
fi
# THE FENCE ITSELF (ersatztv#706 race 1). Re-count the retargets as late as possible and
# refuse to write anything if the PR was retargeted since this run began. See the long note
# at `count_retargets` for why this is a handoff rather than a stall, and why the count is
# the only key that survives an ABA.
#
# The refusal covers `pending` as well as `success`, not just the dangerous write. A stale
# `pending` over a fresh `success` is only a stall rather than a forged green, so gating it
# is not strictly required — but the successor run is guaranteed either way, so there is
# 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
# 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: `pending` cannot turn a rejection
# or an unreviewed head green, so withholding it would strand PRs for no safety gain, while
# a `success` written on a count we could not verify is exactly the forged-green outcome
# this fence exists to prevent.
count_retargets
if [ "$retargets_before_ok" = yes ] && [ "$rt_ok" = yes ] && [ "$rt_count" -ne "$retargets_before" ]; then
echo "::notice::PR #${PR} was retargeted while this job was classifying (${retargets_before} -> ${rt_count} retarget events). This run's classification was computed against a base the PR may no longer target, so it posts NOTHING. The retarget fired an 'edited' event, so a successor run is already queued and will write the authoritative status for ${SHA:0:7}."
exit 0
fi
if { [ "$retargets_before_ok" != yes ] || [ "$rt_ok" != yes ]; } && [ "$state" = "success" ]; then
echo "::error::Could not establish a trusted retarget count for PR #${PR} (before=${retargets_before_ok}, after=${rt_ok}), so an exemption 'success' cannot be shown to have been computed against the PR's current base. Posting nothing; ${CONTEXT} stays absent, which blocks the merge. NOTE a later run only helps if the cause was transient — a PR whose timeline exceeds the page cap will fail this way on every run, and needs a human verdict."
exit 0
fi
payload=$(jq -n --arg s "$state" --arg c "$CONTEXT" --arg d "$desc" --arg u "$PR_URL" \
'{state:$s, context:$c, description:$d, target_url:$u}')
gh -X POST -H 'Content-Type: application/json' -d "$payload" \
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null
echo "Posted ${CONTEXT}=${state} on ${SHA:0:7}."
# --- POST-POST VERIFICATION (ersatztv#706 race 2) ------------------------------------
# The last-moment re-read above narrows the window between reading and writing; it cannot
# close it, because Gitea's status API has no conditional write (no ETag, no If-Match, no
# expected-previous-state), so there is no compare-and-set to make the read and the POST one
# operation. A human `failure` landing in that remaining gap is overwritten by the POST above
# — turning an explicit human REJECTION green, which is the worst outcome this gate can
# produce and strictly worse than any stall.
#
# So verify AFTERWARDS and repair in the safe direction. This runs ONLY on the `success`
# path, and that restriction is the point rather than an optimisation: `pending` cannot
# turn a rejection green — it already blocks the merge — so the only write that can cause
# the damage is the exemption `success`.
#
# WHY A DIFFERENT ENDPOINT. Everywhere else this job reads the COMBINED endpoint
# (`/commits/{sha}/status`), which returns the LATEST status per context — and that is now
# OUR success, with the human's row buried underneath it. The combined view is therefore
# structurally incapable of showing the thing being looked for. `/statuses/{sha}` returns one
# row per POST instead. Measured on this instance, the two really do differ in shape as well
# as content: the combined endpoint returns an OBJECT with a `.statuses` array (12 rows on a
# live head), `/statuses/{sha}` a BARE ARRAY (24 rows on the same head) — hence the different
# `type == "array"` guard here.
#
# No claim is made about the order rows come back in, because the check does not depend on
# it: it selects by id against the high-water mark rather than inspecting the top of the
# list. A verdict older than the mark is invisible to it no matter where it sits.
#
# The repair is `pending`, NEVER a copy of the human's state. Re-posting their `failure`
# would attribute a human verdict to this job — the exact provenance confusion the
# `creator`-based short-circuit above exists to prevent, and it would be written with the
# machine credential, so it would read as machine-derived to every later run. `pending`
# asserts nothing about the review: it blocks the merge and asks for a real verdict, which
# is true and safe regardless of which way the human ruled. The reviewer is told to re-post.
#
# A read failure here does NOT fail the job: the status is already posted, so `exit 1` would
# change nothing about the gate's state while turning a routine API hiccup into a red run.
# It is reported loudly and left alone — the residual is the read/POST gap either way.
if [ "$state" = "success" ] && [ "$max_id_before" -ge 0 ]; then
post_hist=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=100") || post_hist=""
if [ -z "${post_hist//[[:space:]]/}" ] || ! printf '%s' "$post_hist" | jq -e 'type == "array"' >/dev/null 2>&1; then
echo "::warning::Could not re-read the status history for ${SHA:0:7} after posting, so a human verdict landing during the write window would not be detected. The exemption ${CONTEXT}=success stands."
else
# `.id > $since` is what confines this to the write window. Our OWN row is excluded twice
# over — it carries `creator: null` (an Actions-token POST, measured; see the provenance
# note above) and its description is `Exempt: …`, not `Review-verdict:` — so the count is
# of human verdicts that did not exist when the mark was taken.
# TWO row shapes count as "something raced this write", not one (round-5 review).
#
# (a) a HUMAN verdict — non-null creator, `Review-verdict:` description;
# (b) a machine SENTINEL — null creator, description exactly `$REPAIR_DESC`.
#
# (b) is not decoration. With two overlapping runs A and B, the human row can land BELOW
# A's mark (so (a) cannot see it) while B masks it with an exemption success and only
# afterwards writes the sentinel. A then finds nothing human above its mark, does not
# repair, and posts its own success ON TOP of the sentinel — a permanent forged green over
# a human rejection, which is precisely the outcome this whole change exists to prevent.
# Counting the sentinel closes it: A repairs, and both runs converge on the fixed point.
#
# It cannot false-fire. A sentinel that already existed would have been seen at the FIRST
# read, forcing the pending path, and this block only runs after a `success` — so a
# sentinel ABOVE the mark can only have been written by another run mid-flight.
raced=$(printf '%s' "$post_hist" | jq -r --arg c "$CONTEXT" --argjson since "$max_id_before" --arg rd "$REPAIR_DESC" \
'[.[] | select(type == "object")
| select(.context? == $c)
| select((.id? // 0) > $since)
| select(
((.creator != null and .creator.login != null and .creator.login != "")
and (((.description // "") | startswith("Review-verdict:"))))
or ((.creator == null) and ((.description // "") == $rd))
)] | length')
case "$raced" in
''|*[!0-9]*)
echo "::warning::Post-write verification for ${SHA:0:7} returned '${raced}' instead of a count; not acting on it."
;;
*)
if [ "$raced" -gt 0 ]; then
# The last-moment re-read found no human verdict, so any row present now was
# written during the window and has just been masked by the exemption above.
echo "::error::A human ${CONTEXT} verdict landed on ${SHA:0:7} while this job was writing its exemption, and was overwritten. Downgrading to 'pending' so an explicit human decision cannot be silently green. Re-post it with: scripts/post-review-verdict.sh ${PR} <VERDICT>"
repair=$(jq -n --arg c "$CONTEXT" --arg u "$PR_URL" --arg d "$REPAIR_DESC" \
'{state:"pending", context:$c, description:$d, target_url:$u}')
# A failure HERE leaves the forged green standing, so it is retried once and then
# screams. `set -e` would otherwise kill the job silently, after the success was
# written and with nothing left to re-attempt.
if ! gh -X POST -H 'Content-Type: application/json' -d "$repair" \
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
if ! gh -X POST -H 'Content-Type: application/json' -d "$repair" \
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
echo "::error::COULD NOT REPAIR ${CONTEXT} on ${SHA:0:7}. An exemption 'success' is standing on a head whose human verdict was overwritten. Post the verdict again immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
exit 1
fi
fi
echo "Repaired ${CONTEXT} to pending on ${SHA:0:7}."
state=pending
fi
;;
esac
fi
fi
if [ "$state" = "pending" ]; then
echo "::notice::This PR needs an H10 review verdict for head ${SHA:0:7} before it can merge. After reviewing, run: scripts/post-review-verdict.sh ${PR} MERGEABLE"
fi
+2 -2
View File
@@ -52,7 +52,7 @@ docker build -f docker/Dockerfile -t ersatztv:dev .
- Test with **NUnit** + Shouldly + NSubstitute (the existing `*.Tests` projects); xUnit is **not** used here
- **Dependencies use Central Package Management**: versions live in the repo-root `Directory.Packages.props`; csproj reference packages by name only. Add/upgrade by editing the central `<PackageVersion>` — never put `Version=` back on a `<PackageReference>` (trips `NU1008`). See `docs/ci-cd.md` → Dependency management.
- **DB migrations target BOTH providers**: a `TvContext` model change needs a migration in `ErsatzTV.Infrastructure.Sqlite` **and** `ErsatzTV.Infrastructure.MySql` — run `scripts/add-migration.sh <Name>` (does both). CI's `migrations` job enforces model-drift + apply-to-fresh-DB per provider. See `docs/ci-cd.md` → Migration integrity.
- **Renovate** is live (`.gitea/workflows/renovate.yml`, weekly + `workflow_dispatch`): opens dependency-update + OSV vuln-fix PRs and a Dependency Dashboard issue; patch bumps to test/dev-only packages auto-merge once `Build & test` passes, the rest are manual. Their `review-verdict/h10` required check is auto-passed **only when BOTH hold**: the PR touches none of `.claude/`/`.gitea/`/`.husky/`/`scripts/`/`docker/ci/`, **and** every changed path is a dependency manifest (`Directory.Packages.props`, `.config/dotnet-tools.json`) — ersatztv#698. A bot ACCOUNT does not attribute the CODE at a head, so identity alone is no longer sufficient; a Renovate PR touching a `.csproj` or a source file is not blocked, it just needs a real verdict. Cross-repo rollout: server-management#484. See `docs/ci-cd.md` → Dependency management.
- **Renovate** is live (`.gitea/workflows/renovate.yml`, weekly + `workflow_dispatch`): opens dependency-update + OSV vuln-fix PRs and a Dependency Dashboard issue; patch bumps to test/dev-only packages auto-merge once `Build & test` passes, the rest are manual. Their `review-verdict/h10` required check is auto-passed **only when BOTH hold**: the PR touches none of `.claude/`/`.codex/`/`.gitea/`/`.husky/`/`scripts/`/`docker/ci/`, **and** every changed path is a dependency manifest (`Directory.Packages.props`, `.config/dotnet-tools.json`) — ersatztv#698. A bot ACCOUNT does not attribute the CODE at a head, so identity alone is no longer sufficient; a Renovate PR touching a `.csproj` or a source file is not blocked, it just needs a real verdict. Cross-repo rollout: server-management#484. See `docs/ci-cd.md` → Dependency management.
- **Versioning**: release tags are `vYY.<release-seq>.<patch>` (year · sequential release-within-year · patch) — inherited from upstream, **not** year.month. `v26.3.1` = our infra rebuild of upstream 26.3.0 (no app changes); `v26.4.0` is reserved for the first release with app changes. Never `[skip ci]` a commit you'll tag (it suppresses the release build). Full policy: `docs/ci-cd.md` → Versioning & releases.
- Backlog tracked via [Gitea Issues](http://192.168.1.95:3000/timothy/ersatztv/issues)
@@ -84,7 +84,7 @@ Every task that closes a Gitea issue MUST complete ALL of these before it is con
**Merge-consent is derived from state, not asserted (`## Done-when` convention — ersatztv#303 H6 + H10).** Any issue whose PR will merge to `main` should carry a `## Done-when` section in its **issue body** — a checklist of completion criteria (always include an "adversarial review passed" box; add per-issue criteria like tests-green, docs-updated, live-E2E). Two hooks derive merge-consent from it so a premature merge is blocked *by construction*, not by memory:
- `pretooluse-merge-consent.sh` (Claude PreToolUse on the Gitea merge tool) — **auto-grants** a merge (emits `permissionDecision: allow`, so **no** redundant mechanical prompt fires) only when the PR's CI is green **and** every `## Done-when` box on the linked issue (`fixes #N`) is ticked **and** a `Review-verdict:` comment references the PR's *current head sha* (**H10**); **denies** on an unticked box, red CI, or a stale/negative review verdict; **asks** (falls back to a human prompt) when it can't derive state (no linked issue, no `## Done-when` section, no `Review-verdict:` comment yet, no creds, Gitea down). On the auto-grant (satisfied) path the derived state **is** the consent — do not also ask conversationally to merge; a separate human confirmation is warranted only when the gate **asks** (ersatztv#314). **The H10 review-verdict convention**: after an adversarial/Codex review of a PR (or its latest fix commit), run **`scripts/post-review-verdict.sh <pr> <MERGEABLE|APPROVED|BLOCKED|NOT-MERGEABLE> [note]`** — it posts both the `Review-verdict: … @ <head-sha>` comment and the sha-bound `review-verdict/h10` commit status, proving the *latest* commit was reviewed rather than a stale earlier diff (ersatztv#242). Do not hand-write the comment: the **status** is the required check branch protection enforces, and a comment alone leaves it absent.
- **The gate is enforced server-side, per sha (ersatztv#622).** `review-verdict/h10` is a required status check on `main`. Because a commit status belongs to one sha, a commit pushed *after* an auto-merge is scheduled clears it and blocks the merge — closing the hole where `merge_when_checks_succeed` froze consent at scheduling time and Gitea later merged an unreviewed head. Renovate-authored and docs-only PRs are auto-passed by `.gitea/workflows/review-verdict.yml`, **except** when they touch `.claude/`, `.gitea/`, `.husky/`, `scripts/` or `docker/ci/`. See `docs/ci-cd.md` → Review-verdict gate.
- **The gate is enforced server-side, per sha (ersatztv#622).** `review-verdict/h10` is a required status check on `main`. Because a commit status belongs to one sha, a commit pushed *after* an auto-merge is scheduled clears it and blocks the merge — closing the hole where `merge_when_checks_succeed` froze consent at scheduling time and Gitea later merged an unreviewed head. Renovate-authored and docs-only PRs are auto-passed by `.gitea/workflows/review-verdict.yml`, **except** when they touch `.claude/`, `.codex/`, `.gitea/`, `.husky/`, `scripts/` or `docker/ci/`. See `docs/ci-cd.md` → Review-verdict gate.
- `.husky/pre-push``prepush-donewhen.sh` — a fail-open backstop that blocks a direct `git push origin main` whose commits `fix #N` an issue with unticked boxes.
Both need Gitea read creds in the env to enforce (**`ETV_GITEA_BASICAUTH=user:pass`** or `ETV_GITEA_TOKEN`; `ETV_GITEA_URL` overrides the base). Without them the merge hook asks and the push backstop is a no-op — the gate degrades to today's manual confirmation, never a silent pass. Docs-only PRs/pushes are exempt.
+1 -1
View File
@@ -6,7 +6,7 @@
<ItemGroup>
<PackageVersion Include="AsyncFixer" Version="2.1.0" />
<PackageVersion Include="Blurhash.SkiaSharp" Version="2.0.0" />
<PackageVersion Include="CliWrap" Version="3.10.3" />
<PackageVersion Include="CliWrap" Version="3.10.4" />
<PackageVersion Include="coverlet.collector" Version="6.0.4" />
<PackageVersion Include="Dapper" Version="2.1.79" />
<PackageVersion Include="Destructurama.Attributed" Version="5.2.0" />
@@ -593,7 +593,97 @@ public class PipelineBuilderBaseTests
command.ShouldNotContain("-readrate_initial_burst");
}
private string BuildRealtimeCommand(IFFmpegCapabilities capabilities, bool stillImage = false)
[Test]
public void Realtime_Input_Should_Catch_Up_When_Option_Is_Supported()
{
string command = BuildRealtimeCommand(new CatchupCapableFFmpegCapabilities());
// -readrate paces an input off its furthest-behind stream, so a sparse stream sharing the
// input pins throughput below realtime; catchup lets it recover (ersatztv#726). anchor on
// the input path so this can't be satisfied by some other input carrying the option
// this overlaps Bitmap_Subtitle_Burn_In_... by design: that one pins the #726 MECHANISM on a
// bitmap pipeline, this one pins the plain no-subtitle shape plus the uniqueness guard below
command.ShouldContain("-readrate 1.05 -readrate_initial_burst 8 -readrate_catchup 6.0 -i /tmp/whatever.mkv");
Regex.Matches(command, Regex.Escape("-readrate_catchup 6.0")).Count.ShouldBe(1);
}
[Test]
public void Realtime_Input_Should_Not_Catch_Up_A_Still_Image()
{
// mirrors the burst's still-image exclusion (ersatztv#350): the video input takes no readrate
// at all, so catchup would only reach the separate audio input and run it ahead of a graph
// that the realtime filter is already pacing. pinned so the divergence can't reappear silently
string command = BuildRealtimeCommand(new CatchupCapableFFmpegCapabilities(), stillImage: true);
// the positive anchor keeps this from passing vacuously if the helper ever stops
// producing a realtime audio input at all
command.ShouldContain("-readrate 1.05");
command.ShouldNotContain("-readrate_catchup");
}
[Test]
public void Realtime_Input_Should_Not_Catch_Up_When_Option_Is_Unsupported()
{
// an older binary silently keeps today's behavior rather than failing to start
string command = BuildRealtimeCommand(new BurstCapableFFmpegCapabilities());
// the positive anchor keeps this from passing vacuously if the helper ever stops
// producing a realtime input at all
command.ShouldContain("-readrate 1.05");
command.ShouldNotContain("-readrate_catchup");
}
[Test]
public void Concat_Should_Never_Catch_Up()
{
// concat reads already-written segments from the running segmenter at a flat 1.0; it has no
// sparse stream to lag on, and letting it catch up would gallop through the segments
var concatInputFile = new ConcatInputFile("http://localhost:8080/ffmpeg/concat/1", new FrameSize(1920, 1080));
var builder = new SoftwarePipelineBuilder(
new CatchupCapableFFmpegCapabilities(),
HardwareAccelerationMode.None,
None,
None,
None,
None,
concatInputFile,
Option<GraphicsEngineInput>.None,
"",
"",
_logger);
FFmpegPipeline result = builder.Concat(concatInputFile, FFmpegState.Concat(false, "Some Channel"));
string command = PrintCommand(None, None, None, concatInputFile, None, result);
command.ShouldContain("-readrate 1.0");
command.ShouldNotContain("-readrate_catchup");
}
[Test]
public void Bitmap_Subtitle_Burn_In_Should_Catch_Up_On_The_Shared_Video_Input()
{
// THE #726 regression test. an embedded bitmap subtitle is read through the SAME -i as the
// video (SubtitleInputFile carries the video's path and resolves to a stream specifier on
// that input), and being sparse it drags that input's pacing down to ~0.53x realtime.
// this must be built on a BITMAP subtitle: a text subtitle is fetched by the libass filter
// outside the demuxer, so the same assertions would pass vacuously while the bug is present.
string command = BuildRealtimeCommand(new CatchupCapableFFmpegCapabilities(), imageSubtitle: true);
// the mechanism itself: subtitle stream 2 resolves onto input 0 -- the VIDEO's input -- so it
// is read through the throttled demuxer that catchup is being applied to. if the subtitle
// ever moves to an input of its own this label changes and the test fails, which is the point
command.ShouldContain("[0:0][0:2]overlay");
// ...so the catchup has to be on that input
command.ShouldContain("-readrate 1.05 -readrate_initial_burst 8 -readrate_catchup 6.0 -i /tmp/whatever.mkv");
}
private string BuildRealtimeCommand(
IFFmpegCapabilities capabilities,
bool stillImage = false,
bool imageSubtitle = false)
{
var videoInputFile = new VideoInputFile(
"/tmp/whatever.mkv",
@@ -676,13 +766,22 @@ public class PipelineBuilderBaseTests
AudioFilter.None,
Option<double>.None));
// an embedded bitmap subtitle carries the VIDEO's path, which is how it ends up sharing the
// video's single throttled -i rather than getting one of its own (ersatztv#726)
Option<SubtitleInputFile> subtitleInputFile = imageSubtitle
? new SubtitleInputFile(
"/tmp/whatever.mkv",
new List<MediaStream> { new(2, "dvdsub", StreamKind.Subtitle) },
SubtitleMethod.Burn)
: Option<SubtitleInputFile>.None;
var builder = new SoftwarePipelineBuilder(
capabilities,
HardwareAccelerationMode.None,
videoInputFile,
audioInputFile,
None,
None,
subtitleInputFile,
None,
Option<GraphicsEngineInput>.None,
"",
@@ -735,4 +834,19 @@ public class PipelineBuilderBaseTests
new System.Collections.Generic.HashSet<string>(),
new System.Collections.Generic.HashSet<string> { FFmpegKnownOption.ReadrateInitialBurst.Name },
new System.Collections.Generic.HashSet<string>());
// a binary new enough for -readrate_catchup also has -readrate_initial_burst, so this models a
// real ffmpeg rather than an impossible catchup-without-burst one
public class CatchupCapableFFmpegCapabilities() : FFmpegCapabilities(
string.Empty,
new System.Collections.Generic.HashSet<string>(),
new System.Collections.Generic.HashSet<string>(),
new System.Collections.Generic.HashSet<string>(),
new System.Collections.Generic.HashSet<string>(),
new System.Collections.Generic.HashSet<string>
{
FFmpegKnownOption.ReadrateInitialBurst.Name,
FFmpegKnownOption.ReadrateCatchup.Name
},
new System.Collections.Generic.HashSet<string>());
}
@@ -13,8 +13,15 @@ public record FFmpegKnownOption
// ffmpeg 6.1+; lets a readrate-throttled input read flat out for an initial window
public static FFmpegKnownOption ReadrateInitialBurst => new("readrate_initial_burst");
// ffmpeg 8.0+ (added 2025-02-15 in 6232f416b, first released in 8.0); lets a readrate-throttled
// input read faster than its readrate *while it is behind*, so a sparse stream sharing that
// input cannot pin throughput below realtime (ersatztv#726). verified present in 8.1.2, the
// pinned base image — note this is NEWER than 7.1, so it is detected at runtime, never assumed
public static FFmpegKnownOption ReadrateCatchup => new("readrate_catchup");
public static IList<string> AllOptions =>
[
ReadrateInitialBurst.Name
ReadrateInitialBurst.Name,
ReadrateCatchup.Name
];
}
@@ -3,10 +3,11 @@ using ErsatzTV.FFmpeg.Environment;
namespace ErsatzTV.FFmpeg.InputOption;
public class ReadrateInputOption(double readRate, Option<int> initialBurstSeconds) : IInputOption
public class ReadrateInputOption(double readRate, Option<int> initialBurstSeconds, Option<double> catchupReadRate)
: IInputOption
{
public ReadrateInputOption(double readRate)
: this(readRate, Option<int>.None)
: this(readRate, Option<int>.None, Option<double>.None)
{
}
@@ -30,6 +31,17 @@ public class ReadrateInputOption(double readRate, Option<int> initialBurstSecond
result.Add(burst.ToString(CultureInfo.InvariantCulture));
}
// -readrate paces the WHOLE input off its furthest-behind stream, so one sparse stream
// (an embedded PGS/DVD bitmap subtitle feeding the overlay) drags the video down with it
// and output collapses to ~0.53x realtime. catchup lets a lagging input read faster until
// it is level again; it is a ceiling that only applies WHILE behind, never a target, so
// caught-up input still paces at readRate and cannot race ahead (ersatztv#726)
foreach (double catchup in catchupReadRate)
{
result.Add("-readrate_catchup");
result.Add(catchup.ToString("0.0####", CultureInfo.InvariantCulture));
}
return result.ToArray();
}
@@ -22,6 +22,14 @@ public abstract class PipelineBuilderBase : IPipelineBuilder
// an operator who raises that setting above 2 gets less of the benefit (ersatztv#350)
private const int InitialBurstSeconds = OutputFormatHls.SegmentSeconds * 2;
// how fast a LAGGING realtime input may read until it is level again. measured on the #726
// repro (embedded dvd_subtitle -> overlay, QSV encode): 1.05 alone sustains 0.53x, catchup 2.0
// reaches 0.711x, and 6.0 restores the full 1.067x that the same pipeline achieves with no
// subtitle at all. 20.0 also measures 1.067x — i.e. the value is not a throughput dial above
// the point where the input catches up, so 6.0 is chosen as the smallest measured-sufficient
// ceiling rather than the largest that works (ersatztv#726)
private const double CatchupReadRate = 6.0;
private readonly Option<AudioInputFile> _audioInputFile;
private readonly Option<ConcatInputFile> _concatInputFile;
private readonly IFFmpegCapabilities _ffmpegCapabilities;
@@ -871,8 +879,26 @@ public abstract class PipelineBuilderBase : IPipelineBuilder
? InitialBurstSeconds
: Option<int>.None;
_audioInputFile.Iter(a => a.AddOption(new ReadrateInputOption(readRate, initialBurstSeconds)));
videoInputFile.AddOption(new ReadrateInputOption(readRate, initialBurstSeconds));
// -readrate paces an input off its furthest-behind stream. an embedded bitmap subtitle is
// read through the SAME -i as the video (its SubtitleInputFile carries the video's path and
// resolves to a stream specifier on that input), and being sparse it falls further behind
// every second, dragging video throughput to ~0.53x — well under the 1.0x a live client
// consumes at. catchup lets the lagging input recover instead of pinning the whole process.
// applied to every realtime input, not just subtitle pipelines: it is inert unless an input
// is actually behind, and any sparse stream can cause this (ersatztv#726).
//
// a still image is excluded for the SAME reason the burst above excludes it: its video input
// takes no readrate at all, so this would reach only the separate audio input and let it run
// ahead of the video, which is exactly what #350 declined. for a non-still-image item both
// inputs carry identical options, so the symmetry is preserved there. and an image-based
// subtitle always rides the video path, so this shape cannot suffer the starvation anyway
Option<double> catchupReadRate =
!isStillImage && _ffmpegCapabilities.HasOption(FFmpegKnownOption.ReadrateCatchup)
? CatchupReadRate
: Option<double>.None;
_audioInputFile.Iter(a => a.AddOption(new ReadrateInputOption(readRate, initialBurstSeconds, catchupReadRate)));
videoInputFile.AddOption(new ReadrateInputOption(readRate, initialBurstSeconds, catchupReadRate));
}
protected static void SetStillImageLoop(
+54 -9
View File
@@ -40,6 +40,7 @@ Upstream's final release was **`v26.3.0`** (archived). Our line continues from t
| `v26.11.0` | **QSV profiles decode via VA-API**`QsvPreferNativeDecoder`, default **on**, fixes ~50% channel cold-start failures on Intel (#498); unified logo/on-screen bug via a shared watermark preset (#67). Media-scanner resilience: Jellyfin mixed-content libraries (#489), music-video scan correctness (#488/#494/#497), remote-stream probing before ffmpeg (#473/#480); weighted-distribution SPA (#404). **First release deployed to `jazz`** (server-management#633). |
| `v26.12.0` | **`ErsatzTV.Mcp` MCP server** — read + cautious-write over `/api/v1`, `ERSATZTV_ALLOW_WRITES`-gated (#58). **External channel-logo URLs download + cache at save time** (#525), with the on-screen bug now rendered for external-URL logos (#502). HLS cold-start hardening: burst-read the first segments so start isn't `-readrate`-bound (#350) and floor QSV extra hardware frames so an unthrottled read can't exhaust the pool (#529); remote graphics-engine image fetches bounded — timeout, size cap, decode cap, redirects, pooling (#511). Decision-lifecycle tooling + parallel-orientation startup rewrite (#520/#521); CI `docker build` lane rebalance (#508). |
| `v26.13.0` | **RuleBuilder maturation** — arbitrary-depth group nesting (#436), inline smart-query authoring in Channel Builder (#437), DB-sourced facet typeahead + relative-date operators + validation (#434/#435/#438), and an artist typeahead covering music-video/song credits with `album_artist` no longer 404ing (#578). **Per-channel On Now/Next transient overlay** (#74/#570) and **per-schedule clock-boundary padding** (#392); in-browser channel preview (#60); Auto-Tune per-source weight steppers + exclude/add-untagged (#440). Library-browse pickers now resolve by search instead of a 100-row window, closing several silent at-cap truncations (#644/#650/#651/#634). Correctness: one watermark resolver for all four attachment points, incl. `MiddleCenter` (#503/#510); QSV HDR tonemaps through OpenCL because `vpp_qsv=tonemap` is a silent no-op (#505); `LibraryFolder` unique index + concurrent-insert tolerance (#491); per-library music-video identity with soft trash (#496); Jellyfin Album/Track music-video projection (#177); metadata-collection dedup (#500); accented facet values via a registered Unicode fold on SQLite (#668); `WorkAheadSlots` atomic slot claim, never a negative count (#536/#539); on-demand guide rebuild on thaw (#68). Process/CI: the H10 review-verdict gate became a sha-bound **required** commit status and was hardened through its false-open chain (#622/#629/#632/#648/#649/#672/#698), the decision corpus split to one YAML-frontmatter record per file (#610/#620), and headless Playwright UI-E2E flows landed (#445/#533). Five dual-provider migrations. |
| `v26.14.0` | **Live TV no longer starves on embedded bitmap subtitles**`-readrate` paces an input off its *furthest-behind* stream, and a PGS/DVD subtitle read through the video's own `-i` is sparse enough to drag the whole process to **0.53x realtime** against the 1.0x a client consumes, draining the buffer until the channel stalls. Fixed with a capability-gated `-readrate_catchup` (ffmpeg 8.0+) on realtime inputs, keeping `-readrate` on the frame-producing path so the `ffmpeg.qsv-extra-hw-frames-floor` bound is untouched; measured 0.533x → 1.067x on QSV and software, with a 240s QSV soak clean of allocation errors (#726). Affects items carrying an embedded bitmap subtitle matching the channel's subtitle mode — 3,182 of 24,646 media versions on prod, and a property of the *item*, not the channel, which is why the stall presented as random. Process/CI: the H10 review-verdict gate's repair sentinel became a fixed point and its write is now fenced on the timeline retarget count, closing a raced-sentinel false-open (#706/#707/#711). **The decisions validator now cross-checks its dependency-free frontmatter parse against PyYAML** and reports both the truncating unquoted `` #`` and the scalar-closing bare apostrophe as errors, so a record whose `rule:` silently halves under PyYAML fails the local gate instead of CI (#674/#688) — the ceiling-calibration claim was also split so the suite pins what the derivation MEANS rather than live-corpus order statistics. Dependencies: CliWrap 3.10.4, JetBrains.ReSharper.GlobalTools 2025.3.5. |
**Before cutting a release — sweep `docs/decisions.md` + `docs/decisions/`** (ersatztv#521, supersedes
the ersatztv#303 H9 append-only ritual). Supersession/retirement is now a same-PR act (add the new
@@ -47,7 +48,12 @@ active record, relocate the predecessor to `docs/decisions/archive/` with recipr
`supersedes`/`superseded-by` links), not a release-boundary batch job — most of the old "consolidate"
step is now continuous. The release boundary is instead where you:
1. Run `PYTHONPATH=. python3 scripts/decisions_validate.py` — confirms lifecycle metadata is
well-formed and every `supersedes`/`superseded-by` link resolves both ways.
well-formed and every `supersedes`/`superseded-by` link resolves both ways. Since **ersatztv#674**
it also cross-checks its dependency-free frontmatter parse against **PyYAML when PyYAML is
importable**, failing on any file PyYAML rejects (a bare apostrophe in a single-quoted value) or
reads differently (an unquoted ` #`, which YAML truncates as a comment). Where PyYAML is absent —
the `decisions-guard` job, the Husky hooks — the cross-check is **skipped with a `::notice::`**
and every other check still runs; the read path stays dependency-free.
2. Confirm every record already classified `superseded`/`retired` actually lives under
`docs/decisions/archive/` (the validator fails this, but eyeball it at the boundary too).
3. Regenerate the active catalog: `PYTHONPATH=. python3 scripts/build_decisions_catalog.py` and
@@ -56,10 +62,24 @@ step is now continuous. The release boundary is instead where you:
- a **per-record prose ceiling** (`decisions_validate.py --record-ceiling <n>`, default **60**)
— a **non-blocking `::warning::`** naming every record over it. This is the actionable signal:
it points at a file. The 60 is derived from the distribution, not picked as a round number.
A test pins what that derivation MEANS rather than any particular numbers: the ceiling must sit
between the **90th and 95th percentile** of record lengths, i.e. at the tail boundary. Stated
as percentiles it is scale-free, so ordinary corpus growth cannot ratchet it — it fires only
when the ceiling genuinely stops marking the tail and should be re-derived.
Its **calibration is guarded in two pieces of different robustness** (ersatztv#688), because
four earlier single-assertion versions all failed — the first two by being vacuous or
accepting an absurd ceiling, the last two by ratcheting:
- **blocking** (`script-tests`) — only the coarse property that the ceiling flags a
**meaningful minority** of records (`0.02 <= fraction_over <= 0.25`). One record moves a
fraction by at most 1/N, so no SINGLE ordinary addition can cross it. This is measured
headroom, not immunity: from today's 18/183 it takes 38 consecutive over-ceiling additions to
breach the cap, 718 short ones to dilute below the floor, or — the tightest arm —
consolidating 15 of the 18 offenders away. The floor is
a fraction rather than "at least one record", which would accept any ceiling up to 229 on the
live corpus; as a fraction the accepted range is 43..180.
- **reported, never asserted against the LIVE corpus** — the fine claim that the ceiling sits
between the **90th and 95th percentile**, i.e. at the tail boundary. `main()` prints a
`::notice::` when it drifts; the tests assert it only on distributions they own.
It is an order statistic over a sparse distribution, so a single new record could move p90 by
21 lines and red the blocking job for whoever wrote it; a ceiling going out of date is
the passage of corpus growth, not a defect in the commit under test, so it is treated like
`stale-after`. Re-derive the constant when the notice says so.
- the **aggregate prose total**, printed every run as an unthresholded `::notice::` **trend**.
It has no pass/fail. A total over a monotonically growing corpus can only ratchet: the old
4800→5600 budget went quiet at 5228 after #610 changed the metric and was back over at 5658
@@ -791,7 +811,7 @@ and comparing tips would invalidate every open verdict on every unrelated merge
status as `success` for **Renovate-authored** PRs (it uses `platformAutomerge: true`, so a required
verdict with no exemption would stall every dependency bump) and for **docs-only** PRs, and as
`pending` for everything else so the block has a visible reason. Both exemptions are **void when the
PR touches `.claude/`, `.gitea/`, `.husky/`, `scripts/` or `docker/ci/`** — a PR that can weaken the
PR touches `.claude/`, `.codex/`, `.gitea/`, `.husky/`, `scripts/` or `docker/ci/`** — a PR that can weaken the
gate must not be able to exempt itself from the gate. That includes Renovate's `docker/ci` base
bumps, which already need the manual publish-then-pin two-step anyway.
@@ -837,9 +857,34 @@ the PR's live base does not match it, checked both before and after paging (ersa
the head sha — a PR opened into `main` and retargeted mid-run was granted a docs-only exemption while
its diff against `main` carried a C# file. The workflow passes the base from the `pull_request_target`
payload, which a retarget cannot rewrite, and `edited` is in `types:` so a retarget reclassifies.
`edited` gives **detection, not atomicity**: runs are not serialized, so a stale run can still post
`success` after the reclassifying run posts `pending`. The `main → scratch → main` ABA transition is
narrowed and observable, not closed — see the residual in `ci.exemption-provenance`.
`edited` gives **detection, not atomicity**: runs are not serialized, so a stale run could still post
`success` after the reclassifying run posted `pending`.
**That residual is now fenced (ersatztv#706).** Runs are still not serialized — instead a run that was
overtaken *declines to write*. The job counts `change_target_branch` events on the PR's issue timeline
at start and again immediately before its POST, and posts **nothing** if the count moved. The count is
the key precisely because the branch *name* is ABA-vulnerable: `main → scratch → main` reads `main` at
both ends, which is how the forged exemption was obtained in the first place. Abstaining never strands
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 `pending` cannot turn an
unreviewed head green and withholding it would strand ordinary PRs for nothing. **If an exempt PR is
unexpectedly missing its status after a retarget, this is why** — the job log names the counts.
Worth knowing before reaching for the obvious alternative: **a concurrency group does not work here**,
measured rather than assumed. Gitea 1.25.4 auto-cancels superseded `push` runs on a branch, but *not*
`pull_request_target` runs — two runs for one PR genuinely overlap, and adding
`concurrency: {…, cancel-in-progress: false}` changed nothing (probe runs still overlapped by 36s).
`cancel-in-progress: true` is deliberately untried, because a cancelled run leaves an exempt PR
statusless with nothing left to re-trigger it. Full measurements and the two surviving residuals:
`ci.verdict-write-retarget-fence`.
Separately, after posting an exemption `success` the job re-reads the per-POST status history and, if
a human `Review-verdict:` row appeared during the write window, overwrites its own status with
`pending` and logs an error — so a human `BLOCKED` can never be silently turned green. The repair is
`pending`, never a copy of the human's verdict, which would attribute a human decision to the job.
Three properties of this workflow are security-relevant and are **structurally** asserted by tests in
`scripts/tests/test_pr_changed_files.py` — those tests pin the workflow's shape, which is not the same
+7 -4
View File
@@ -43,7 +43,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `ci.decisions-lifecycle-flake` | When `decisions lifecycle` is the **only** red job, do not investigate and do not create a new run to clear it — no rebase, no `--amend`, no no-op push; the operator reruns that single job from the Gitea UI. | 2026-07-21 | [link](records/ci/decisions-lifecycle-flake.md) |
| `ci.docs-only-detect-shallow-safe` | The docs-only detect script must diff against `FETCH_HEAD` (always resolves after `git fetch`, even shallow) using a two-dot tree diff — not `origin/<base>` with three-dot — because a `fetch-depth: 1` shallow clone has no remote-tracking ref and no merge-base, which silently fails the original detect into `docs_only=false` (full matrix, no functional error). A CI-behavior change must be verified by measuring the effect (job durations), not just a green check. | 2026-07-17 | [link](records/ci/docs-only-detect-shallow-safe.md) |
| `ci.docs-only-skip-steps` | A docs-only change must still run every required job (`test`, `migrations`) so their commit-status contexts always report; each heavy job runs `scripts/ci-detect-docs-only.sh` first and gates its real STEPS on `if: steps.detect.outputs.docs_only != 'true'`, never `if:`-skips the whole job (an `if:`-skipped job reports `skipped`, not `success`, which branch protection may never unblock on). Detection biases toward running more on any doubt. | 2026-07-17 | [link](records/ci/docs-only-skip-steps.md) |
| `ci.exemption-provenance` | The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR's live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR's immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires only for a status POSITIVELY identified as a human verdict for THIS base, meaning a non-null `.creator.login` AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR's — tested by requiring the description to END with the exact literal `(base: <base>)` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow's `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one (residual, #706). Path predicates are evaluated by COUNTING with `grep -c`, never `\| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`. | 2026-07-29 | [link](records/ci/exemption-provenance.md) |
| `ci.exemption-provenance` | The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR's live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR's immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires only for a status POSITIVELY identified as a human verdict for THIS base, meaning a non-null `.creator.login` AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR's — tested by requiring the description to END with the exact literal `(base: <base>)` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow's `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one. That residual is now FENCED rather than merely tracked — the job refuses to write at all if the PR's timeline retarget COUNT moved while it was classifying (`ci.verdict-write-retarget-fence`, #706) — leaving only the sub-round-trip window that no API without compare-and-set can close. The PROTECTED path list additionally covers `.codex/` (#711), which mirrors `.claude/hooks/` byte for byte including the merge-consent hook — latent while that directory is untracked, live the moment it is tracked; the list stays ENUMERATIVE rather than derived, because a derived rule would have to be evaluated against the very file list being classified. Path predicates are evaluated by COUNTING with `grep -c`, never `\| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`. | 2026-07-29 | [link](records/ci/exemption-provenance.md) |
| `ci.format-gate-folder-mode` | The blocking `format` CI job (and matching pre-commit hook) runs `dotnet format whitespace . --folder --include <files>` instead of loading the full MSBuild/Roslyn solution, cutting the gate from ~480s to ~0.5s with unchanged whitespace/charset coverage. | 2026-07-19 | [link](records/ci/format-gate-folder-mode.md) |
| `ci.functional-e2e-harness` | The `functional-e2e` CI job boots the PR's own code from source via `dotnet run` (`scripts/e2e-local.sh`) and runs deterministic assertions (`scripts/e2e-functional.sh`) as an advisory (non-blocking) job, not a `build` dependency or required check. Originally curl-only; since #445 the same job carries a second, headless-browser step for the contracts curl cannot express — see `ci.ui-e2e-harness`. | 2026-07-16 | [link](records/ci/functional-e2e-harness.md) |
| `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class: Gitea injects a write-capable `GITEA_TOKEN` into EVERY job, so any ref-resolved workflow — and a collaborator's own API token, since branch protection binds the context and not its issuer — can still forge `review-verdict/h10`. Tracked in #697; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) |
@@ -58,9 +58,10 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `ci.root-screenshot-guard` | The Husky `pre-commit` hook refuses a staged root-level `*.png` (belt-and-suspenders with the `.gitignore` rule); nested `*.png` real assets are unaffected. | 2026-07-12 | [link](records/ci/root-screenshot-guard.md) |
| `ci.runner-placement` | No persistent Roslyn compiler server survives a CI build (`UseSharedCompilation=false` etc., runner env + Dockerfile `ENV`); every `services:` container gets its own explicit `--memory`/`--memory-swap`/`--cpus` cap (it does not inherit the job container's). | 2026-07-17 | [link](records/ci/runner-placement.md) |
| `ci.script-tests-job` | The `scripts/tests/` pytest suite runs on every PR as a dedicated `script-tests` job in `pr-checks.yml` (`runs-on: small`, `setup-python` + `pip install pytest`, `PYTHONPATH=. python3 -m pytest scripts/tests -q`), unconditionally rather than behind a `scripts/**` path filter, and **never as a step inside `decisions-guard`** — a job whose reds a standing rule instructs sessions to ignore must never host a gate whose reds are real. Any new CI gate must be reachable by a failure that is unambiguously attributable to it. | 2026-07-26 | [link](records/ci/script-tests-job.md) |
| `ci.shared-pr-file-enumeration` | A PR's complete set of changed file paths is computed by exactly one implementation, `scripts/pr-changed-files.sh`, called by both `.claude/hooks/pretooluse-merge-consent.sh` (advisory — a failure falls through to a human prompt) and `.gitea/workflows/review-verdict.yml` (enforced — a failure must fail closed, because a match here posts the branch-protection-required `review-verdict/h10` status with nobody in the loop). The script owns exhaustiveness (pagination, rename/path validation, head-sha binding, and base-ref binding — see `ci.exemption-provenance`) and returns exit 0 only for a verified-complete list; it does NOT classify paths — each caller keeps its own docs-only allow-list, and the two allow-lists differ on purpose and stay separate. | 2026-07-26 | [link](records/ci/shared-pr-file-enumeration.md) |
| `ci.shared-pr-file-enumeration` | A PR's complete set of changed file paths is computed by exactly one implementation, `scripts/pr-changed-files.sh`, called by both `.claude/hooks/pretooluse-merge-consent.sh` (advisory — a failure falls through to a human prompt) and `.gitea/workflows/review-verdict.yml` (enforced — a failure must fail closed, because a match here posts the branch-protection-required `review-verdict/h10` status with nobody in the loop). The script owns exhaustiveness (pagination, rename/path validation, head-sha binding, base-ref binding — see `ci.exemption-provenance` — and base-TIP binding, #707: the ref answers "did this PR RETARGET", the tip answers "did the base ADVANCE mid-enumeration", and only the second can see `/pulls/{n}/files` recomputing each offset-paged page against a moved base and dropping a path out of an already-consumed range; both ends of the window are bound, and an advance BEFORE the window is deliberately not an error, or ordinary churn on `main` would fail every open PR) and returns exit 0 only for a verified-complete list; it does NOT classify paths — each caller keeps its own docs-only allow-list, and the two allow-lists differ on purpose and stay separate. | 2026-07-26 | [link](records/ci/shared-pr-file-enumeration.md) |
| `ci.small-lane-git-only` | `runs-on: small` is defined by what a job does (git-only), not its usual runtime; the two `docker build` jobs (docker-build.yml, ci-image.yml) move to `ubuntu-latest` because their worst-case memory, not median runtime, was pinning the small lane's per-slot cap. | 2026-07-20 | [link](records/ci/small-lane-git-only.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-write-retarget-fence` | The `review-verdict/h10` job counts `change_target_branch` events on the PR's issue timeline at run start and again immediately before its POST, and writes NOTHING if the count moved. The COUNT is the key because the branch NAME is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption — while the event count is monotonic and cannot alias. Abstaining is a handoff, not a stall, and that is the property the design rests on: 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 and the last run writes the final answer. `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` cannot turn an unreviewed head green while withholding it would strand ordinary PRs for no safety gain. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting an exemption `success` the job re-reads `/statuses/{sha}` 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. | 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) |
| `concurrency.diff-scalar-fanout` | The frozen Block optimistic-concurrency recipe (api-conventions §7a) fans out to Collection/Playout×2/MultiCollection/RerunCollection, keeping a guard-returned `PreconditionFailedError` out of any handler's generic `catch(Exception)`→422 mapping, and preserving each aggregate's existing `SaveChangesAsync() > 0` gate semantics under the new unconditional `Version++`. | 2026-07-11 | [link](records/concurrency/diff-scalar-fanout.md) |
@@ -71,10 +72,11 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `concurrency.replace-all-contract` | Replace-all aggregate PUTs carry a uniform plain `int Version` concurrency token (EF `.IsConcurrencyToken()`), checked pre-save and enforced by the EF UPDATE guard, returning 412 (not 409) on a stale `If-Match`. | 2026-07-11 | [link](records/concurrency/replace-all-contract.md) |
| `concurrency.schedule-item-child-identity` | `PUT /api/schedules/{id}/items` reconciles by an optional round-tripped child `Id` (null/absent/0 ⇒ new item), never by array position, so fill-group/shuffle state follows the logical item across reorders; an unknown or duplicate id is rejected 422 (checked after the §7a `CheckVersion`, so 412 precedes 422). | 2026-07-11 | [link](records/concurrency/schedule-item-child-identity.md) |
| `docs.convention-docs-session-start` | Docs-first, not source-first: conventions (api-conventions, spa-conventions, e2e-local, blazor-route-parity, domain-model, decisions, README) are read from docs, not reverse-engineered from code, via `docs/README.md`'s task-signal map — only the sections it points to for the task at hand, not the whole set. Each doc is updated in the same PR that changes what it documents, replacing deferred/follow-up doc updates. | 2026-07-07 | [link](records/docs/convention-docs-session-start.md) |
| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) |
| `docs.corpus-size-signal` | The corpus's size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling's CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a MEANINGFUL MINORITY of records (`0.02 <= fraction_over <= 0.25`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`' treatment rather than a red in the blocking `script-tests` job. | 2026-07-26 | [link](records/docs/corpus-size-signal.md) |
| `docs.decision-lifecycle` | every decision `##` record (active or archived) carries a 5-field metadata block (`key`, `status`, `since`, `supersedes`, `superseded-by`) checked by `scripts/decisions_validate.py`; a record is never deleted or line-edited to reverse a call — it is moved to `docs/decisions/archive/` with `status: superseded`/`retired` and a reciprocal `superseded-by`/`supersedes` key pair to its replacement. | 2026-07-21 | [link](records/docs/decision-lifecycle.md) |
| `docs.decision-one-file-per-record` | Each decision record is its own file at `docs/decisions/records/<area>/<topic>.md` (archived ones at `docs/decisions/archive/<area>/<topic>.md`) with YAML frontmatter; the filename IS the key, so one-active-record-per-key is a filesystem property rather than a validator check, and supersession is a `git mv`. | 2026-07-25 | [link](records/docs/decision-one-file-per-record.md) |
| `docs.decision-optional-provenance` | Decision records gain two OPTIONAL fields — `stale-after: YYYY-MM-DD` on the metadata line and a `**Sources:**` line in the metadata block; the Open Knowledge Format (OKF) itself is NOT adopted as the record format. | 2026-07-25 | [link](records/docs/decision-optional-provenance.md) |
| `docs.frontmatter-pyyaml-crosscheck` | `decisions_validate.py` runs `pyyaml_frontmatter_faults()` over every record-wing file: it loads the frontmatter with PyYAML and reports an ERROR when PyYAML rejects the document OR when any key's value differs from what the dependency-free `dl._read_frontmatter` read. PyYAML is the WRITER of these files (`migrate_decisions_split.render_record` emits them with `yaml.safe_dump`), so on any disagreement PyYAML is authoritative and the defect is in the FILE, not in either parser. The check is strictly additive: when PyYAML is not importable it is SKIPPED and `main()` says so with a `::notice::`, never silently — the read path stays dependency-free because `decisions-guard`, the Husky hooks and contributor machines install nothing. The comparison has exactly ONE implementation, called by both the validator and `test_frontmatter_reader_matches_pyyaml_on_every_real_record`, so the suite and the tool cannot drift on what "matches PyYAML" means. | 2026-08-04 | [link](records/docs/frontmatter-pyyaml-crosscheck.md) |
| `docs.record-wing-parse-guard` | `decisions_validate.py` asserts, per PATH, that every `*.md` under `docs/decisions/records/**` and `docs/decisions/archive/**` parses to exactly one record carrying a `key` — an ERROR, not a warning, since a file in the record wings that is not a record is a mistake by definition. A file sitting DIRECTLY in `archive/` is exempt only when it actually looks like a #610 stripped index — exactly one keyless record with a known generated heading — never merely by living there. The one other exemption, `archive/README.md`, is by exact RELATIVE PATH; nothing is ever exempt by BASENAME, since that would exempt the same filename in the active wing too. `_read_frontmatter` is deliberately NOT extended to accept YAML block scalars: every record value goes on ONE line, and the structural check is what makes that limitation loud instead of silent. | 2026-07-26 | [link](records/docs/record-wing-parse-guard.md) |
| `docs.tracker-comment-retrofit` | When the knowledge exporter flags an over-cap tracker issue and excludes it from ingestion, triage its comments instead of assuming a retrofit is owed — and for each decision-shaped item check the **worked issue first**, because a tracker session comment is by construction a précis of the fuller closing record posted on the issue it narrates. Applied to #237 (111 comments) this yielded **zero** records, so server-management#642's "a fact found only in a #237 comment" retrieval row has no valid subject and its interim target (an already-migrated record) is permanent. | 2026-07-21 | [link](records/docs/tracker-comment-retrofit.md) |
| `ffmpeg.external-logo-graphics-engine` | External-URL channel logos pass through to the graphics engine like any other watermark source; `WatermarkSelector` must never gate them on `File.Exists` (always false for a URL) and never route them through the ffmpeg-native overlay shortcut. | 2026-07-20 | [link](records/ffmpeg/external-logo-graphics-engine.md) |
@@ -82,6 +84,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `ffmpeg.qsv-decode-encode-split` | QSV decode is decoupled from QSV encode via a single `FFmpegProfile.QsvPreferNativeDecoder` bool (default ON, Linux-only), so a QSV encode profile can decode with the more tolerant native VA-API decoder instead of the QSV decoder, mirroring Jellyfin's hybrid decode/encode toggle instead of a general decode-family enum. | 2026-07-20 | [link](records/ffmpeg/qsv-decode-encode-split.md) |
| `ffmpeg.qsv-extra-hw-frames-floor` | a QSV upload never emits `extra_hw_frames` below `FFmpegState.MinimumQsvExtraHardwareFrames` (64); a stored `0` or negative value is treated as "no pool configured" rather than honored literally, because with no headroom any unthrottled read exhausts the pool and the transcode writes nothing at all. | 2026-07-21 | [link](records/ffmpeg/qsv-extra-hw-frames-floor.md) |
| `ffmpeg.qsv-hdr-tonemap-opencl` | the QSV pipeline never emits `vpp_qsv=tonemap=1`, which is a SILENT no-op on pre-Gen11 Intel graphics; HDR is tonemapped on the GPU via `hwupload=derive_device=vaapi``scale_vaapi``hwmap=derive_device=opencl``tonemap_opencl` when a VA-API device exists, the frames are still in software, and `tonemap_opencl` is available, and by the software `TonemapFilter` otherwise. The scale runs BEFORE the tonemap, and any hardware filter on the path forces the output to be re-tagged bt709. | 2026-07-26 | [link](records/ffmpeg/qsv-hdr-tonemap-opencl.md) |
| `ffmpeg.readrate-catchup-sparse-streams` | a realtime video/audio input also gets `-readrate_catchup` (6.0) when the binary supports it — but NOT a still-image input (mirroring the #350 exclusion) and NOT a concat input, which keep at most bare `-readrate` (a still image's video input takes none at all). Reason: `-readrate` paces the whole input off its furthest-behind stream, so a sparse stream sharing that input (an embedded PGS/DVD bitmap subtitle feeding the overlay) otherwise pins output at ~0.53x realtime. Catchup is a ceiling that applies only WHILE an input is behind, never a target, so it does not let a caught-up input race ahead. | 2026-08-04 | [link](records/ffmpeg/readrate-catchup-sparse-streams.md) |
| `ffmpeg.remote-image-fetcher-bounded` | remote graphics-engine images are fetched through `IRemoteImageFetcher` with a pooled `HttpClientFactory` client, a body-covering deadline, a wire-transfer size cap, and a decoder-enforced `DecoderOptions.MaxFrames` bound re-verified post-decode — never cached, re-fetched per element init. | 2026-07-20 | [link](records/ffmpeg/remote-image-fetcher-bounded.md) |
| `ffmpeg.watermark-resolution-unified` | Every watermark `WatermarkSelector` resolves goes through one shared `ResolveWatermark` — the playout-item, channel and global precedence levels AND the deco path, for all three `ChannelWatermarkImageSource` values. An unresolvable watermark (missing file, un-migrated external URL, or no logo artwork) resolves to no on-screen bug plus a warning, never a dead path or a URL handed downstream; the one deliberate exception is a playout-item `Custom` with a blank image, which still falls THROUGH to channel/global. The generated-initials fallback is therefore off everywhere, including the deco path where it demonstrably rendered. Watermarks built OUTSIDE the selector (the song-progress overlay, #653) are not covered and remain unchecked. | 2026-07-26 | [link](records/ffmpeg/watermark-resolution-unified.md) |
| `ffmpeg.work-ahead-slot-atomic` | `workAheadSegmenterLimit` is enforced by a single compare-exchange claim on a shared `WorkAheadSlots` pool taken by the *caller* of `Transcode`, which then passes ownership in and gets the release in `Transcode`'s `finally` — never a `Volatile.Read` compare in one place and an `Interlocked.Increment` in another. | 2026-07-21 | [link](records/ffmpeg/work-ahead-slot-atomic.md) |
@@ -124,7 +127,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
| `release.prepush-clean-worktree-guard` | A fail-open pre-push hook blocks a push when any file in the branch's diff vs `origin/main` also has uncommitted working-tree or index changes, since a stale-index commit (e.g. `git reset --soft` + `git add` over an edited-but-unstaged fix) can silently push, CI-test, and get reviewed a different tree than the one on disk. Scope is precise to pushed-diff files; escape hatch `ETV_ALLOW_DIRTY_PUSH=1`. | 2026-07-17 | [link](records/release/prepush-clean-worktree-guard.md) |
| `release.promotion-floating-prod` | Prod tracks the floating `:prod` image reference; a tag build's immutable `:<version>` image is scanned first, then promotion happens via a separate manual `DeployStack`, with daily auto-update only as a fallback — tag with enough runway before 03:00 to avoid an unscanned promotion. | 2026-07-13 | [link](records/release/promotion-floating-prod.md) |
| `release.review-verdict-gate` | A PR may not merge until a `Review-verdict: <MERGEABLE\|APPROVED\|BLOCKED\|NOT-MERGEABLE> @ <head-sha>` comment references the PR's current head sha (short-sha prefix match against the verdict's OWN `@ <sha>` field, marker at COLUMN 0 (no indent, so indented code blocks cannot self-approve), whole-word verdict token, fenced code blocks stripped with markdown fence-length semantics, negative wins over positive on the same head); folds into the H6 merge-consent hook as condition (c). The grammar lives in ONE tested place, `scripts/check-review-verdict.sh`#629 found three false-opens that survived because it was implemented inline and untested while this record described stricter behaviour than the code had. | 2026-07-12 | [link](records/release/review-verdict-gate.md) |
| `release.verdict-status-check` | The H10 review verdict is written as a `review-verdict/h10` Gitea **commit status** on the exact reviewed sha by `scripts/post-review-verdict.sh`, and that context is a REQUIRED status check on `main`. Because a status belongs to one sha, a later commit cannot inherit it, so Gitea's own `merge_when_checks_succeed` refuses to merge a head no one reviewed. The PreToolUse hook additionally refuses to SCHEDULE an auto-merge unless that status is already green on head. A `pull_request_target` workflow auto-passes the two exempt classes (Renovate-authored, docs-only) unless the PR touches a protected path (`.claude/`, `.gitea/`, `.husky/`, `scripts/`, `docker/ci/`). This extends — does not supersede — `release.review-verdict-gate` (#303 H10), whose comment convention remains the human-readable artifact and the hook's condition (c). | 2026-07-25 | [link](records/release/verdict-status-check.md) |
| `release.verdict-status-check` | The H10 review verdict is written as a `review-verdict/h10` Gitea **commit status** on the exact reviewed sha by `scripts/post-review-verdict.sh`, and that context is a REQUIRED status check on `main`. Because a status belongs to one sha, a later commit cannot inherit it, so Gitea's own `merge_when_checks_succeed` refuses to merge a head no one reviewed. The PreToolUse hook additionally refuses to SCHEDULE an auto-merge unless that status is already green on head. A `pull_request_target` workflow auto-passes the two exempt classes (Renovate-authored, docs-only) unless the PR touches a protected path (`.claude/`, `.codex/`, `.gitea/`, `.husky/`, `scripts/`, `docker/ci/`). This extends — does not supersede — `release.review-verdict-gate` (#303 H10), whose comment convention remains the human-readable artifact and the hook's condition (c). | 2026-07-25 | [link](records/release/verdict-status-check.md) |
| `rulebuilder.relative-date-macros` | The visual rule builder's `inLast`/`notInLast` date operators compile to/parse from the pre-existing `CustomMultiFieldQueryParser` macros `released_inthelast`/`released_notinthelast` and `added_inthelast`/`added_notinthelast`, value form `"<n> day\|week\|month\|year"`; there is no backend change. | 2026-07-23 | [link](records/rulebuilder/relative-date-macros.md) |
| `scan.collections-scan-status` | `GET /api/v1/media-sources/collections-scan-status` reports a family-global (not per-source), boolean-only active-scan set read from `IEntityLocker`; the SPA reconciles authoritatively against it (with a grace-tick helper) instead of a fixed client-side timeout. | 2026-07-12 | [link](records/scan/collections-scan-status.md) |
| `scan.getoraddfolder-db-lookup` | `ILibraryRepository.GetOrAddFolder` resolves the existing folder via a DB query on `(LibraryPathId, Path)`, not the caller's `LibraryPath.LibraryFolders` in-memory navigation, since that navigation is only eager-loaded on the local scan path and is null on remote (Jellyfin) callers. | 2026-07-20 | [link](records/scan/getoraddfolder-db-lookup.md) |
@@ -5,7 +5,7 @@ status: active
since: '2026-07-29'
supersedes: none
superseded-by: none
rule: 'The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR''s live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR''s immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires only for a status POSITIVELY identified as a human verdict for THIS base, meaning a non-null `.creator.login` AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR''s — tested by requiring the description to END with the exact literal `(base: <base>)` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow''s `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one (residual, #706). Path predicates are evaluated by COUNTING with `grep -c`, never `| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`.'
rule: 'The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR''s live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR''s immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires only for a status POSITIVELY identified as a human verdict for THIS base, meaning a non-null `.creator.login` AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR''s — tested by requiring the description to END with the exact literal `(base: <base>)` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow''s `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one. That residual is now FENCED rather than merely tracked — the job refuses to write at all if the PR''s timeline retarget COUNT moved while it was classifying (`ci.verdict-write-retarget-fence`, #706) — leaving only the sub-round-trip window that no API without compare-and-set can close. The PROTECTED path list additionally covers `.codex/` (#711), which mirrors `.claude/hooks/` byte for byte including the merge-consent hook — latent while that directory is untracked, live the moment it is tracked; the list stays ENUMERATIVE rather than derived, because a derived rule would have to be evaluated against the very file list being classified. Path predicates are evaluated by COUNTING with `grep -c`, never `| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`.'
signals: 'forged review-verdict exemption, retarget race against the docs-only classifier, PR base changed mid-run, hijacked Renovate branch, bot exemption on a code change, machine-written success inherited as a verdict, status creator null vs user, never overwrite a human verdict, exemption chain skips docs-only for bots, why is my Renovate PR asking for a verdict, base ref binding on pr-changed-files.sh · paths: `.gitea/workflows/review-verdict.yml`, `scripts/pr-changed-files.sh`, `.claude/hooks/pretooluse-merge-consent.sh`, `scripts/tests/test_pr_changed_files.py` · issues: #698, #697, #672, #663, #649, #632'
mechanics: '`scripts/pr-changed-files.sh <owner> <repo> <pr> <expected-head-sha> <expected-base-ref>` (5 args; a 4-arg call exits 2); workflow env `BASE_REF: ${{ github.event.pull_request.base.ref }}`; `BOT_MANIFESTS` anchored allow-list; short-circuit requires `.creator.login` non-null AND description matching `^Review-verdict:`; `types: [opened, reopened, synchronize, ready_for_review, edited]`'
---
@@ -36,6 +36,17 @@ than described as fixed. `edited` and re-derivation remain one fix — `edited`
on the existing `success`; re-derivation alone never gets a second run — but together they are
mitigation, not a guarantee.
**Resolved 2026-08-03 (#706), and worth recording that the guarantee finally came from somewhere else
entirely.** The missing piece was never ordering: `ci.verdict-write-retarget-fence` leaves the runs as
unserialized as they ever were and instead makes a run that was overtaken decline to write, keyed on
the timeline's monotonic retarget COUNT — the one signal the `main → scratch → main` ABA cannot make
look unchanged. Measurement is what redirected it: `pull_request_target` runs were confirmed to
overlap live (probe PR #722, the older run finishing 20s after the newer one started), and a
non-cancelling concurrency group — the fix this record's residual implied and #706 proposed — was
measured doing nothing at all. The paragraph above stands as written; only its last sentence is
overtaken, and the sub-round-trip window it describes survives, because Gitea's status API has no
compare-and-set.
**Route 2 — a bot ACCOUNT does not attribute the CODE.** `pull_request.user.login` is the PR's
immutable *creator*; its head is not. Push application code onto an open Renovate branch and the PR is
still "authored by renovate", touches no protected path, and was exempted. Checking the *pusher* fixes
@@ -5,7 +5,7 @@ status: active
since: '2026-07-26'
supersedes: none
superseded-by: none
rule: 'A PR''s complete set of changed file paths is computed by exactly one implementation, `scripts/pr-changed-files.sh`, called by both `.claude/hooks/pretooluse-merge-consent.sh` (advisory — a failure falls through to a human prompt) and `.gitea/workflows/review-verdict.yml` (enforced — a failure must fail closed, because a match here posts the branch-protection-required `review-verdict/h10` status with nobody in the loop). The script owns exhaustiveness (pagination, rename/path validation, head-sha binding, and base-ref binding — see `ci.exemption-provenance`) and returns exit 0 only for a verified-complete list; it does NOT classify paths — each caller keeps its own docs-only allow-list, and the two allow-lists differ on purpose and stay separate.'
rule: 'A PR''s complete set of changed file paths is computed by exactly one implementation, `scripts/pr-changed-files.sh`, called by both `.claude/hooks/pretooluse-merge-consent.sh` (advisory — a failure falls through to a human prompt) and `.gitea/workflows/review-verdict.yml` (enforced — a failure must fail closed, because a match here posts the branch-protection-required `review-verdict/h10` status with nobody in the loop). The script owns exhaustiveness (pagination, rename/path validation, head-sha binding, base-ref binding — see `ci.exemption-provenance` — and base-TIP binding, #707: the ref answers "did this PR RETARGET", the tip answers "did the base ADVANCE mid-enumeration", and only the second can see `/pulls/{n}/files` recomputing each offset-paged page against a moved base and dropping a path out of an already-consumed range; both ends of the window are bound, and an advance BEFORE the window is deliberately not an error, or ordinary churn on `main` would fail every open PR) and returns exit 0 only for a verified-complete list; it does NOT classify paths — each caller keeps its own docs-only allow-list, and the two allow-lists differ on purpose and stay separate.'
signals: 'duplicated PR file enumeration, enforced gate weaker than advisory hook, docs-only allow-list drift, shared mechanism not shared policy, pr-changed-files.sh, checkout base ref not PR head, gate judging its own PR, exhaustiveness bug in a security predicate · paths: `scripts/pr-changed-files.sh`, `.claude/hooks/pretooluse-merge-consent.sh`, `.gitea/workflows/review-verdict.yml` · issues: #643, #648, #649'
mechanics: '`scripts/pr-changed-files.sh <owner> <repo> <pr> <expected-head-sha> <expected-base-ref>` -> stdout newline-delimited paths, exit 0 only if complete and bound to BOTH the given sha and the given base branch; the 5th argument is REQUIRED and a 4-arg call exits 2 (`ci.exemption-provenance`); callers: `.claude/hooks/pretooluse-merge-consent.sh`, `.gitea/workflows/review-verdict.yml`'
---
@@ -0,0 +1,72 @@
---
key: ci.verdict-write-retarget-fence
title: '2026-08-03 — the review-verdict job fences its write on the PR timeline''s retarget COUNT, and verifies the exemption write afterwards (#706)'
status: active
since: '2026-08-03'
supersedes: none
superseded-by: none
rule: 'The `review-verdict/h10` job counts `change_target_branch` events on the PR''s issue timeline at run start and again immediately before its POST, and writes NOTHING if the count moved. The COUNT is the key because the branch NAME is ABA-vulnerable — `main -> S -> main` reads `main` at both ends, which is how #698 route 1 obtained a forged exemption — while the event count is monotonic and cannot alias. Abstaining is a handoff, not a stall, and that is the property the design rests on: 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 and the last run writes the final answer. `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` cannot turn an unreviewed head green while withholding it would strand ordinary PRs for no safety gain. SEPARATELY, and for the human-verdict race the fence does nothing about: after posting an exemption `success` the job re-reads `/statuses/{sha}` 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.'
signals: 'stale review-verdict run overwrites a fresher one, retarget ABA against the docs-only classifier, 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'
mechanics: '`count_retargets()` pages `GET /repos/{repo}/issues/{pr}/timeline?limit=50&page=N` (cap 20) setting `rt_count`/`rt_ok`, trusted only on a validated empty page; `retargets_before`/`retargets_before_ok` captured before enumeration, re-counted immediately before the POST; `max_id_before` from `GET /repos/{repo}/statuses/{sha}` (a BARE ARRAY, unlike the combined `/commits/{sha}/status` object); repair POST is `pending`; tests `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`'
---
`ci.exemption-provenance` closed three routes into the exemption path and left one residual it named:
status writes are not serialized, so a stale run can post over a fresher one. This record resolves it,
**narrowing** that record rather than superseding it.
## Measured, not reasoned (Gitea 1.25.4, 2026-08-03)
- **`pull_request_target` runs for one PR overlap, older finishing last.** Probe PR #722: run 7520
(`opened`) completed at 18:30:42, twenty seconds *after* run 7521 (`synchronize`) began. Race 1's
mechanism, observed rather than argued.
- **A non-cancelling concurrency group — #706's own proposal — does nothing.** With it active, runs
7528/7529 still overlapped; 7528 ended 36s after 7529 started. Refuted, not declined.
- **The control that saved it.** A first probe *with* a group showed cancellations, which looked like
confirmation. The identical workflow with **no `concurrency:` key at all** cancelled the same way:
Gitea auto-cancels superseded **`push`** runs by itself, and that does not extend to
`pull_request_target`. Without the control, a no-op would have shipped as a solution.
- **`cancel-in-progress: true` is deliberately untried** — cancellation is precisely what this
workflow's header refuses, since a cancelled run leaves an exempt PR statusless with nothing to
re-trigger it.
## Why the count, and why abstaining is safe
The timeline records each retarget as a `change_target_branch` event. Verified on the route-1
reproduction PR #703 (exactly two: `main → probe698/base-S` and back) against PR #717 as a
zero-control. The branch *name* aliases under `main → S → main`; the count cannot.
The standing objection to refuse-on-motion is that it strands the PR — fatal for `updated_at`,
harmless here, and not by degree: a retarget **always** fires `edited`, so the abstaining run is
guaranteed a successor. It defers rather than declines.
## What cold review caught (both easy to reintroduce)
**The mark must be taken BEFORE the last-moment re-read, not merely before the POST.** "As late as
possible" is the safer-sounding instinct and is the opposite: a verdict landing between the re-read
and a late mark is invisible to the re-read (already done) *and* excluded from the post-write check
(id below a mark taken afterwards) — a gap spanning the whole retarget re-count, while the change
claimed one round-trip. Early costs nothing, since `id > mark` hides pre-existing rows either way.
Pinned structurally, as an order not an output: with the mark late the job still posts and still
repairs in every scenario a stub can pose, and only the arithmetic silently changes.
**The repair must be a FIXED POINT or it merely decays more slowly.** A repaired status is a machine
`pending`, indistinguishable to the next run — which re-derived it and posted `success` again. The
description is now a sentinel no exemption is granted over *and* is re-written verbatim by every later
run: the first attempt refused the exemption but wrote the GENERIC pending text, erasing its own
marker, so the exemption returned two events later instead of one. Only a re-posted verdict clears it.
## What is NOT closed
1. A retarget between the final timeline read and the POST. Gitea's status API has no conditional
write, so without compare-and-set this cannot reach zero. The magnitude changed: a *permanent*
forged green became a *transient* one of about one round-trip, and that retarget still fires
`edited`, so a later run re-derives it.
2. The repair is itself a read-then-write and can be raced; it fails toward `pending`. A transport
failure on its POST is retried once then fails the job loudly. A human re-posting a BASE-MISMATCHED
verdict after a repair does bury the sentinel — that needs a user credential, so it is #697's.
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.
@@ -5,8 +5,8 @@ status: active
since: '2026-07-26'
supersedes: none
superseded-by: none
rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one.'
signals: 'aggregate active-corpus budget, schedule a consolidation warning, permanently red ratchet, per-record ceiling, 60-line prose ceiling, generated catalog counted in budget, corpus consolidation has no owner, size is not redundancy · paths: `scripts/decisions_validate.py`, `scripts/tests/test_decisions_validate.py`, `docs/ci-cd.md` · issues: #620, #610, #603, #542, #520'
rule: 'The corpus''s size signal is a per-record prose ceiling (`decisions_validate.py --record-ceiling`, default 60, chosen at a natural gap in the distribution), reported as a NON-BLOCKING `::warning::` naming each record over it. The aggregate prose total is still printed every run but carries NO threshold — it is a `::notice::` trend only — because a total over a monotonically growing corpus can only ratchet, and the generated catalog (`docs/decisions/README.md`) is no longer counted at all since it gains one row per record and cannot be consolidated away. Being listed by the ceiling is an invitation to check for REDUNDANCY, never an instruction to cut: a long record that is all distinct findings is a legitimate decline, and should be recorded as one. The ceiling''s CALIBRATION is guarded in two pieces of different robustness (#688): the blocking test asserts only the coarse, non-ratcheting property that the ceiling flags a MEANINGFUL MINORITY of records (`0.02 <= fraction_over <= 0.25`), while the fine claim — that it sits between p90 and p95 — is REPORTED by `main()` as a `::notice::` and never asserted against the live corpus. A ceiling drifting out of date is the passage of corpus growth, not a defect in the commit under test, so it gets `stale_records`'' treatment rather than a red in the blocking `script-tests` job.'
signals: 'aggregate active-corpus budget, schedule a consolidation warning, permanently red ratchet, per-record ceiling, 60-line prose ceiling, generated catalog counted in budget, corpus consolidation has no owner, size is not redundancy, ceiling calibration reddens script-tests, adding a record fails CI on length, p90 sits on the ceiling, tail-boundary drift notice · paths: `scripts/decisions_validate.py`, `scripts/tests/test_decisions_validate.py`, `docs/ci-cd.md` · issues: #688, #620, #610, #603, #542, #520'
mechanics: '`scripts/decisions_validate.py` -> `oversized_records` / `_budget_total`; `docs/ci-cd.md` -> "`decisions-guard` job"'
---
@@ -46,19 +46,49 @@ meant to prevent exactly that could not see it: `max(under) <= 60 < min(over)` i
construction** of the two lists it builds, and passes on a distribution with no gap at all. A
rationale-guarding test that cannot fail is worse than none, because it launders the claim.
It is replaced by `test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY_of_the_distribution`, which
states the property directly and scale-free: **the ceiling sits between the 90th and 95th percentile
of record lengths** — that is what "marks the start of the tail" means — and reads the value from
`RECORD_CEILING_DEFAULT` so test and CLI cannot drift.
It is replaced by `test_real_corpus_ceiling_flags_a_nonempty_proper_minority`, which asserts that the
ceiling flags a meaningful minority of records (`0.02 <= fraction_over <= 0.25`) and reads the value
from `RECORD_CEILING_DEFAULT`, so test and CLI cannot drift.
Getting there took four versions, and the failures are the useful part:
Getting there took five versions, and the failures are the useful part:
| | assertion | why it failed |
|---|---|---|
| v1 | `max(under) <= 60 < min(over)` | true **by construction** of those two lists |
| v2 | a minimum gap WIDTH | a ceiling of 200 also sits in a wide gap — it passed |
| v3 | 2-12% fraction band + "clear air" above | **hostage to an unrelated record**: one ordinary 62-line addition reddened it with the ceiling correctly placed, and the only remedy was to RAISE the ceiling — this very treadmill, as a hard failure in what #631 makes a blocking job. The fraction band had the same coupling more slowly (12 more long records breached it), and `0 <= headroom` was vacuous. |
| v4 | `p90 <= ceiling <= p95` | percentiles move WITH the corpus, so routine growth cannot ratchet it; it fires only when the ceiling genuinely stops marking the tail |
| v4 | `p90 <= ceiling <= p95` | percentiles move with the corpus, but an order statistic over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then jump straight to 81 with NOTHING in between (measured; the multiplicities move with every record added, the gap is the point), so ONE record can move p90 by twenty-one lines (that is today's gap; the #672 event moved it less and still reddened CI). It reddened the blocking job twice live (#672, #706), and both times the only in-scope remedy was to trim the new record to fit the constant — the v3 ratchet, pointed at record authors |
| v5 | coarse `0.02 <= fraction_over <= 0.25` asserted; fine `p90 <= ceiling <= p95` REPORTED | splits the claim by robustness instead of hunting for a better single assertion (#688) |
**v5 is not a fifth attempt at the same shape — it stops trying.** Four versions failed because they
all asserted, in the blocking job, a property of a corpus the commit under test does not control.
The fine claim is genuinely useful and genuinely fragile, so it is now measured on every run and
printed as a `::notice::` — the same treatment `stale_records` gets, and for the same stated reason:
a constant going out of date is the passage of time, not a defect in this change. What stays
blocking is only what no SINGLE ordinary addition can break — each record moves a fraction by at
most 1/N, so from **18/183** over the ceiling it takes **38** consecutive over-ceiling additions to
BREACH the 25% cap (37 lands exactly on 0.25, which still passes), against **one** record to break v4.
**The floor is a fraction, not `> 0`, and review is why.** The first draft of v5 asserted only
`0 < fraction_over < 1/3`, which measured against the real corpus accepted **every ceiling from 39
to 229** — including the ceiling of 200 the draft itself offered as the case it catches, because a
single 230-line record keeps the count nonzero. A bound that a deliberately absurd value satisfies
is not a guard. At a 2% floor and a 25% cap the accepted range is **43..180** (measured, contiguous):
a ceiling of 200 flags 0.5% of records and is rejected, a ceiling of 20 flags 60% and is rejected,
and today's 9.8% sits about 5x ABOVE the floor and 38 over-ceiling additions below the cap.
**Three arms, and the tightest is CONSOLIDATION** — stated because it is the easy one to forget.
Breaching the cap takes 38 over-ceiling additions; diluting below the floor takes 718 short ones;
but taking **15** of today's 18 over-ceiling records out of the over-set also drops below it — trimming them to <=60 leaves 3/183 = 1.64%, archiving them leaves 3/168 = 1.79%, since archiving moves the denominator too. That
is a real tension with `test_oversized_records_can_go_green`, and it is accepted rather than papered
over: at 3/183 the constant genuinely IS mis-calibrated, so the red is the signal working. A
consolidation PR big enough to reach it should re-derive the ceiling in the same change.
The honest cost, stated rather than buried: **nothing now forces a re-derivation.** The ceiling can
drift while only a notice complains. That is accepted on the same reasoning this record already
applies to its two "keep listed" consolidation candidates — the warning names it on every run, which
tracks it better than a red that gets trimmed around, and a red an author can only clear by editing
an unrelated constant is not enforcement, it is a toll.
Two rules came out of that sequence, and they outlive this metric:
**a guard test must depend only on the thing it guards**, and
@@ -68,7 +98,7 @@ is restated as a self-referential fact** for the same reason: the warning report
numbers on every run, and a number frozen in prose is one edit away from being a lie.
**Size is a proxy for the thing we actually care about, and the proxy is demonstrably wrong.** Of
the records over the ceiling, the largest by ~1.6x
the records over the ceiling, the longest —
`scan.libraryfolder-unique-identity`, 230 lines — is a dozen-odd **distinct** hard-won traps (MySQL
`utf8mb4_bin` PAD SPACE, create-the-composite-index-before-dropping-its-predecessor, clearing the
connection pool per MySQL fixture, lazy hash healing that must never abort a scan…). Shortening it
@@ -0,0 +1,72 @@
---
key: docs.frontmatter-pyyaml-crosscheck
title: '2026-08-04 — `decisions_validate.py` cross-checks its dependency-free frontmatter parse against PyYAML whenever PyYAML is importable (#674)'
status: active
since: '2026-08-04'
supersedes: none
superseded-by: none
rule: '`decisions_validate.py` runs `pyyaml_frontmatter_faults()` over every record-wing file: it loads the frontmatter with PyYAML and reports an ERROR when PyYAML rejects the document OR when any key''s value differs from what the dependency-free `dl._read_frontmatter` read. PyYAML is the WRITER of these files (`migrate_decisions_split.render_record` emits them with `yaml.safe_dump`), so on any disagreement PyYAML is authoritative and the defect is in the FILE, not in either parser. The check is strictly additive: when PyYAML is not importable it is SKIPPED and `main()` says so with a `::notice::`, never silently — the read path stays dependency-free because `decisions-guard`, the Husky hooks and contributor machines install nothing. The comparison has exactly ONE implementation, called by both the validator and `test_frontmatter_reader_matches_pyyaml_on_every_real_record`, so the suite and the tool cannot drift on what "matches PyYAML" means.'
signals: 'validator reports OK on a broken record, bare apostrophe in single-quoted frontmatter, unquoted hash truncates a value, hand-rolled frontmatter parser, PyYAML rejects the file but decisions-validate passes, dependency-free read path, frontmatter cross-check skipped · paths: `scripts/decisions_validate.py`, `scripts/decisions_lib.py`, `scripts/tests/test_decisions_validate.py`, `scripts/tests/test_decisions_lib.py` · issues: #674, #578, #651, #621'
mechanics: '`scripts/decisions_validate.py` -> `pyyaml_frontmatter_faults` / `_frontmatter_block`, wired into `main()` alongside `record_wing_faults`'
---
The validator read ordinary English prose in a `rule:` field and reported **OK** on a file PyYAML
refuses to parse. It was hit **twice in one session by two independent agents** on unrelated
branches (#578, #651), which is what makes it a guard rather than a note: it is not an exotic edge
case, it is what happens when anyone writes `SQLite's LOWER()` into a single-quoted scalar.
**Why the hand parser exists, and why it stays.** `dl._read_frontmatter` is deliberately
dependency-free — it runs in `decisions-guard`, in the Husky hooks, and on every contributor
machine, none of which install anything. Requiring PyYAML there once made the validator crash with
`ModuleNotFoundError` on the very records the split had just written. So the fix could not be
"import yaml in the reader". It is a second, optional opinion layered on top.
**The two known hazards fail DIFFERENTLY, and that shaped the fix.**
| input | dependency-free reader | PyYAML |
|---|---|---|
| `rule: 'SQLite's LOWER()'` | `SQLite's LOWER()` | **`ParserError`** — the bare apostrophe closes the scalar early |
| `rule: use --flag #2` | `use --flag #2` | `use --flag`` #` starts a comment, **silently truncating** |
A `try/except` would have caught only the first row. The second produces no exception at all: a
valid record whose `rule` has quietly lost its tail — the `parse-to-WRONG` case
`docs.record-wing-parse-guard` explicitly names as the gap its structural check cannot see. So the
cross-check compares the parsed **result** key by key, and reports a rejection and a mismatch as two
distinct faults with different remedies. The `except` around the load is deliberately broad, not
`yaml.YAMLError`: PyYAML's timestamp constructor raises a bare `ValueError` on an impossible date
(`stale-after: 2026-06-31`), and an additive check must never be the reason the validator can't run.
**That is also what makes it general.** #674 asked for a fix that catches the *next* character class
rather than enumerating hazards one at a time. Comparing against the writer's own library is that:
any construct where the two parsers disagree surfaces as a diff, with nobody having to predict it.
**Direction is the load-bearing part.** PyYAML is not a second opinion of equal standing — it WROTE
these files, so when the two disagree the on-disk bytes mean what PyYAML says, the record is corrupt
and the permissive reader is the one hiding it. That is what turns an ambiguous "parsers differ"
report into an actionable "this record is silently wrong".
**A skip is announced, not silent.** When PyYAML is absent the check does not run, which is correct
on the dependency-free path — but `main()` prints a `::notice::` saying so. A check that reports
success while doing nothing is the defect this corpus keeps re-learning (#603's `stale-after` that
never fired, #609's marker that exempted everything while printing OK), and adding a quiet skip
while fixing a quiet pass would have reintroduced it one level up.
**One implementation, two callers.** The comparison already existed — in the test suite only, which
is exactly why the validator could disagree with `scripts/tests` about the same file. Rather than
leaving a second copy, `test_frontmatter_reader_matches_pyyaml_on_every_real_record` now delegates
to `pyyaml_frontmatter_faults`, so the tool and the suite cannot drift on the definition.
**What this buys, stated precisely, because it is less than it looks.** In CI `decisions-guard`
installs nothing, so the validator there always takes the skip path, by design; and `script-tests`
already went red on both hazards before this change, and an advisory red still blocks the merge gate
(#598). So **no broken record has reached `main` and the CI delta here is close to zero** — though
procedurally, not structurally: branch protection on `main` requires exactly three contexts (`Build
& test (.NET)`, `EF migration integrity`, `review-verdict/h10`), and NEITHER `script-tests` nor
`decisions-guard` is among them. What this fixes is the case #674 described: the LOCAL loop, where
the validator is the tool an agent reaches for directly and it printed OK on a corrupt file — plus
the tool/suite disagreement, now impossible.
**Limits, and the positive control.** It does not catch a mis-parse both parsers agree on — strong,
not total, the same qualification `record_wing_faults` carries. The suite pins that
`record_wing_faults` ALONE still reports both hazard files as clean; without that, the cross-check
could be deleted and the tests would stay green while the guard vanished.
@@ -58,7 +58,8 @@ mechanics: '`SetRealtimeInput` readrate-burst option; `FFmpegKnownOption.HasOpti
session" flag through `FFmpegState`; that complexity was not judged worth a bounded peak.
- **Still images are excluded.** Their video input is paced by the realtime *filter* and takes no
readrate at all, so a burst would only run the audio input ahead of the video for songs and offline
filler, with no cold-start gain to show for it.
filler, with no cold-start gain to show for it. (`-readrate_catchup` mirrors this exclusion for the
same reason — `ffmpeg.readrate-catchup-sparse-streams`, #726.)
- **Non-HLS realtime outputs (`TransportStream`, HLS-Direct) burst too**, since
`FFmpegPlaybackSettingsCalculator` makes them unconditionally realtime. That is untested by the
benchmark, which was segmenter-only; it is kept because the same first-read throttle delays those
@@ -0,0 +1,71 @@
---
key: ffmpeg.readrate-catchup-sparse-streams
title: 2026-08-04 — a realtime input gets `-readrate_catchup`, because `-readrate` paces off its furthest-behind stream (#726)
status: active
since: '2026-08-04'
supersedes: none
superseded-by: none
rule: 'a realtime video/audio input also gets `-readrate_catchup` (6.0) when the binary supports it — but NOT a still-image input (mirroring the #350 exclusion) and NOT a concat input, which keep at most bare `-readrate` (a still image''s video input takes none at all). Reason: `-readrate` paces the whole input off its furthest-behind stream, so a sparse stream sharing that input (an embedded PGS/DVD bitmap subtitle feeding the overlay) otherwise pins output at ~0.53x realtime. Catchup is a ceiling that applies only WHILE an input is behind, never a target, so it does not let a caught-up input race ahead.'
signals: 'readrate, readrate_catchup, sparse stream, bitmap subtitle, PGS, DVD subtitle, dvdsub, pgssub, overlay burn-in, Live TV buffering/stalling, "Resumed reading at pts N with rate R after a lag of Ns" · paths: `PipelineBuilderBase.SetRealtimeInput`, `ReadrateInputOption`, `FFmpegKnownOption` · issues: #726, #350, #529'
mechanics: '`PipelineBuilderBase.CatchupReadRate` (6.0); `ReadrateInputOption` catchup arg; `FFmpegKnownOption.ReadrateCatchup` capability gate'
---
- **`-readrate` throttles an input, not a stream, and it paces off whichever stream is furthest
behind.** An embedded bitmap subtitle is read through the *same* `-i` as the video —
`SubtitleInputFile` carries the video's path and `ComplexFilter` resolves it to a stream specifier
on that input, and `CommandGenerator` never emits a second `-i` for it. Being sparse, the subtitle
stream falls further behind every second and drags the video down with it. FFmpeg says so itself at
`-loglevel warning`: `[sist#0:3/dvd_subtitle] Resumed reading at pts 10.400 with rate 6.000 after a
lag of 0.922s`, repeating with the lag growing 0.9→3.8 s while `pts` stays pinned (no new packet).
- **Measured on prod (QSV, `-threads 1`, `dvd_subtitle`→overlay), 45 s steady-state window after a
6 s settle:**
| variant | throughput |
|---|---|
| `-readrate 1.05` (baseline) | **0.533x** (×3 runs) |
| `+ -readrate_catchup 2.0` | 0.711x |
| `+ -readrate_catchup 6.0` | **1.067x** (×2 runs) |
| `+ -readrate_catchup 20.0` | 1.067x |
| no subtitle overlay (control) | 1.067x |
A live client consumes at 1.0x, so 0.53x drains its buffer until it stalls — the reported symptom.
- **`20.0` measuring the same as `6.0` is why 6.0 was chosen** — above the catch-up point the value
is not a throughput dial, so there is nothing to buy by going higher. It is **not** evidence about
allocation: that is a steady-state throughput number, not a count of frames in flight.
- **Why this does not reopen `ffmpeg.qsv-extra-hw-frames-floor` (#529).** Not because catchup is
brief (a permanently GPU-bound channel lags forever, so 6x is a standing licence), and **not**
because read rate is allocation-irrelevant — #529 measured that it is not (at `extra_hw_frames=0`,
`1.05` without a burst exits 0 while `1.05`+burst hits ENOMEM). Read rate changes how fast frames
enter the graph, not how deep its queues are, and #529 showed that only bites when the pool has
**no headroom**. The 64-frame floor now guarantees headroom, so the load-bearing measurement is
row 5 of that truth table — **no `-readrate` at all with 64 frames → 14 segments, exit 0** — and a
6x ceiling is strictly less aggressive than no throttle. Reinforcing it, `-readrate_initial_burst 8`
has read *flat out* at the start of every playout item since #350, so an unbounded read here is not
new. A 240 s QSV soak (64 frames, 60 segment boundaries) adds 1.043x sustained with **zero**
`Cannot allocate memory` — but it stayed largely caught-up, so it corroborates rather than proves;
the argument above is what carries the decision.
- **Not QSV-specific:** reproduces on libx264 too (0.533x → 1.067x), as expected for an input-pacing
option upstream of any encoder or filter choice.
- **Raising the base `-readrate` is not an alternative, and was measured:** 2.0→0.62x, 3.0→0.80x,
4.0→0.80x, 6.0→0.89x. It asymptotes *below* realtime, because the rate ceiling was never the
binding constraint. Recorded so it is not re-proposed.
- **Catchup does NOT subsume the #350 burst; they fix orthogonal metrics.** Measured
time-to-first-segment: `-readrate` alone 3.71 s, `+burst` **0.72 s**, `+catchup` alone **3.65 s**,
both 0.67 s. Catchup buys nothing at cold start (no accumulated lag at t=0 to recover) and the
burst buys nothing for throughput (the 0.533x baseline already had it), so removing the burst on
the theory that catchup replaces it would regress tune-in ~5x.
- **Applied to realtime video/audio inputs generally, not only subtitle pipelines** — it is inert
unless an input is behind, and any sparse stream can trigger this, so gating it on "has a bitmap
subtitle" would fix the site instead of the boundary. Two deliberate exclusions, both test-pinned:
`ConcatInputFile` (reads already-written segments at a flat 1.0, nothing sparse to lag on) and
**still images**, mirroring #350 — their video input takes no readrate at all, so catchup would
reach only the separate audio input and break the pacing symmetry #350 declined to break. An
image-based subtitle always rides the *video* path, so that shape cannot starve this way anyway.
- **Capability-gated via `FFmpegKnownOption.HasOption`**, the same fail-safe posture as
`-readrate_initial_burst`: detection parses `ffmpeg -h long`, so a binary without the option
silently keeps today's behavior instead of failing to start.
**Accepted residual:** the affected population is items carrying an embedded bitmap subtitle matching
the channel's subtitle mode — 3,182 of 24,646 media versions (12.9%) on prod. It is a property of the
*item*, not the channel, which is why the stall presented as random: a channel plays one episode fine
and stalls on the next.
@@ -5,7 +5,7 @@ status: active
since: '2026-07-25'
supersedes: none
superseded-by: none
rule: 'The H10 review verdict is written as a `review-verdict/h10` Gitea **commit status** on the exact reviewed sha by `scripts/post-review-verdict.sh`, and that context is a REQUIRED status check on `main`. Because a status belongs to one sha, a later commit cannot inherit it, so Gitea''s own `merge_when_checks_succeed` refuses to merge a head no one reviewed. The PreToolUse hook additionally refuses to SCHEDULE an auto-merge unless that status is already green on head. A `pull_request_target` workflow auto-passes the two exempt classes (Renovate-authored, docs-only) unless the PR touches a protected path (`.claude/`, `.gitea/`, `.husky/`, `scripts/`, `docker/ci/`). This extends — does not supersede — `release.review-verdict-gate` (#303 H10), whose comment convention remains the human-readable artifact and the hook''s condition (c).'
rule: 'The H10 review verdict is written as a `review-verdict/h10` Gitea **commit status** on the exact reviewed sha by `scripts/post-review-verdict.sh`, and that context is a REQUIRED status check on `main`. Because a status belongs to one sha, a later commit cannot inherit it, so Gitea''s own `merge_when_checks_succeed` refuses to merge a head no one reviewed. The PreToolUse hook additionally refuses to SCHEDULE an auto-merge unless that status is already green on head. A `pull_request_target` workflow auto-passes the two exempt classes (Renovate-authored, docs-only) unless the PR touches a protected path (`.claude/`, `.codex/`, `.gitea/`, `.husky/`, `scripts/`, `docker/ci/`). This extends — does not supersede — `release.review-verdict-gate` (#303 H10), whose comment convention remains the human-readable artifact and the hook''s condition (c).'
signals: 'merge_when_checks_succeed freezes consent, auto-merge merges an unreviewed head, verdict bound to sha, review-verdict/h10 required check, post-review-verdict.sh, Renovate platformAutomerge exemption · paths: `scripts/post-review-verdict.sh`, `.gitea/workflows/review-verdict.yml`, `.claude/hooks/pretooluse-merge-consent.sh` · issues: #622, #303 (H6/H10), #242, #619'
mechanics: '`scripts/tests/test_post_review_verdict.py` (incl. a TOCTOU head-moved case and cross-checks against the hook''s own condition-(c) regexes); branch protection `status_check_contexts` on `main`'
---
+211 -2
View File
@@ -17,6 +17,7 @@ import subprocess
import sys
from datetime import date
from pathlib import Path
from typing import NamedTuple
import scripts.decisions_lib as dl # noqa: E402 (run with PYTHONPATH=. or as module)
@@ -52,6 +53,18 @@ _STALE_AFTER_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$")
# guards the calibration claim asserts against the SAME value the CLI uses and the two cannot drift.
RECORD_CEILING_DEFAULT = 60
# The coarse, non-ratcheting calibration bound (#688): the ceiling must flag a MEANINGFUL MINORITY
# of records. Below the floor it is parked among the outliers and names almost nobody; above the cap
# it is cutting into the bulk rather than marking a tail. See `ceiling_calibration` for why the fine
# percentile claim is reported instead of asserted.
#
# The floor is NOT "at least one record" — that was the first draft and it was nearly unfalsifiable:
# measured on the live corpus it accepted every ceiling from 39 to 229, including the ceiling of 200
# this module's own docstring offered as the case it catches (one 230-line record keeps the count
# nonzero). A 2% floor rejects 200/229/230 and still leaves ~5x headroom below today's 9.8%.
CEILING_MINORITY_MIN = 0.02
CEILING_MINORITY_MAX = 0.25
def _parse_stale_after(value: str | None) -> date | None:
"""`stale-after` as a date, or None if absent, empty, or malformed.
@@ -541,6 +554,102 @@ def record_wing_faults(records_dir: Path | None = None, archive_dir: Path | None
return faults
def _frontmatter_block(text: str) -> str | None:
"""The raw YAML between the opening `---` and the next `---`, or None if there isn't one."""
if not dl.has_frontmatter(text):
return None
lines = text.splitlines()
end = next((i for i, ln in enumerate(lines[1:], start=1) if ln.rstrip() == "---"), None)
if end is None:
return None
return "\n".join(lines[1:end])
def pyyaml_frontmatter_faults(files) -> tuple[list[str], bool]:
"""Faults where PyYAML disagrees with the dependency-free reader. Returns (faults, ran).
#674: the two known hazards are a bare apostrophe inside a single-quoted scalar
(`rule: 'SQLite's LOWER()'`) and an unquoted ` #` (`rule: use --flag #2`). Before this check the
validator reported OK on both, because `dl._read_frontmatter` is a hand parser that cannot see
either. It was hit TWICE in one session by two independent agents, which is what makes it worth
a guard rather than a note.
The two hazards fail DIFFERENTLY, and catching only the first would have missed half of it:
* the apostrophe makes PyYAML **reject** the document outright (`ParserError`);
* the unquoted ` #` parses fine and **silently truncates** the value — PyYAML reads
`use --flag`, the hand parser reads `use --flag #2`. No exception, a wrong value.
So this compares the parsed RESULT and does not merely try/except the load. That is also why it
generalizes past the two known characters, which is the property #674 asked for: any future
construct where the writer's library and our reader disagree shows up as a diff, without anyone
enumerating it first.
DIRECTION MATTERS: PyYAML is the WRITER (`migrate_decisions_split.render_record` emits these
files with `yaml.safe_dump`), so it is the authority on what the on-disk bytes mean. The hand
reader is the permissive one, and a disagreement is a defect in the FILE, not in either parser.
`ran` is False when PyYAML is not importable. The read path is deliberately dependency-free
`decisions-guard`, the Husky hooks and every contributor machine install nothing so this check
is strictly additive: it must never be the reason the validator cannot run. main() announces the
skip rather than passing quietly, because a check that reports success while doing nothing is
the exact defect class this corpus keeps re-learning (#603's `stale-after`, #609's marker).
"""
try:
import yaml # pyright: ignore[reportMissingImports]
except Exception:
return [], False
faults: list[str] = []
for p in files:
try:
text = p.read_text(encoding="utf-8")
except Exception: # noqa: S112 (record_wing_faults already reports the unreadable file by name)
continue
block = _frontmatter_block(text)
if block is None:
continue # no/unterminated frontmatter: reported by record_wing_faults
try:
theirs_raw = yaml.safe_load(block)
except Exception as exc:
# DELIBERATELY broad. `yaml.YAMLError` alone is too narrow: PyYAML's timestamp
# constructor raises a BARE ValueError for a well-shaped but impossible date
# (`stale-after: 2026-06-31` -> "day is out of range for month"), which would escape as
# a traceback. This check is meant to be strictly additive — it must never be the
# reason the validator cannot run, so every failure to load becomes a reported fault.
first = str(exc).splitlines()[0] if str(exc).strip() else exc.__class__.__name__
faults.append(
f"{p}: PyYAML REJECTS this frontmatter, though the dependency-free reader accepted "
f"it ({exc.__class__.__name__}: {first}). PyYAML is what WROTE these files, so its "
f"verdict is authoritative. Quote the offending value and, inside single quotes, "
f"double any literal apostrophe — as `yaml.safe_dump` does. Usual causes: a bare "
f"apostrophe inside a single-quoted value, an unquoted `: ` or leading backtick, or "
f"an impossible date."
)
continue
if theirs_raw is None:
theirs_raw = {}
if not isinstance(theirs_raw, dict):
faults.append(f"{p}: frontmatter parses as {type(theirs_raw).__name__}, not a mapping.")
continue
mine = dl._read_frontmatter(block)
if mine is None:
continue # the reader bailed: reported by record_wing_faults as parse-to-0
theirs = {k: ("" if v is None else str(v)) for k, v in theirs_raw.items()}
# `key=str`: PyYAML returns TYPED mapping keys, so a stray `1: x` yields an int key while the
# hand reader yields "1", and sorting that mixed set raises TypeError — an uncaught traceback
# replacing what `_unknown_frontmatter_keys` used to report as an actionable error.
for k in sorted(set(mine) | set(theirs), key=str):
if mine.get(k) != theirs.get(k):
faults.append(
f"{p}: frontmatter key {k!r} means different things to the two parsers — "
f"reader={mine.get(k)!r} but PyYAML={theirs.get(k)!r}. PyYAML wrote this file, "
f"so its reading is the real value and the record is silently corrupt. Common "
f"cause: an unquoted ` #`, which YAML treats as a comment and truncates there."
)
return faults, True
def _unknown_frontmatter_keys(path: Path) -> set[str]:
"""Frontmatter keys outside the known schema. Empty on any read/parse failure (reported elsewhere)."""
try:
@@ -629,7 +738,7 @@ def oversized_records(records, ceiling: int) -> list[tuple[str, int]]:
act on instead of asserting that "the corpus" is too big.
The ceiling sits at a natural gap in the real distribution rather than a round number: at #620
the records run 0..59 prose lines (median 26, p90 52) and then jump straight to 83, with
the records ran 2..59 prose lines (median 26, p90 52) and then jumped straight to 83, with
nothing in between. 60 separates the bulk from the tail without splitting a cluster.
IMPORTANT a prompt for judgement, not a target. Length is a PROXY for "grown past what a
@@ -642,6 +751,79 @@ def oversized_records(records, ceiling: int) -> list[tuple[str, int]]:
return sorted([kv for kv in out if kv[1] > ceiling], key=lambda kv: -kv[1])
class CeilingCalibration(NamedTuple):
n: int
p90: int
p95: int
n_over: int
fraction_over: float
marks_tail: bool # the FINE claim: p90 <= ceiling <= p95
flags_minority: bool # the COARSE claim: MINORITY_MIN <= fraction_over <= MINORITY_MAX
def ceiling_calibration(records, ceiling: int) -> CeilingCalibration:
"""How well `ceiling` still marks the start of the corpus's tail (#688).
Two claims of DIFFERENT robustness, deliberately separated, because conflating them is what
made the previous guard a ratchet:
`marks_tail` `p90 <= ceiling <= p95`. Correct as a definition of "start of the tail", but an
order statistic over a SPARSE distribution is a STEP function: the lengths climb to the ceiling
and then jump straight to 81 with nothing in between AS MEASURED TODAY (the gap's width moves
with the corpus this is the shape, not a constant), so ONE new record could move p90 by 21
lines, and the only remedy the assertion admitted was to raise the ceiling. It is real signal,
but it is signal about the CONSTANT drifting, not a defect in the commit under test the same
shape as `stale_records`, and it is reported the same way: a notice, never a failure.
`flags_minority` `CEILING_MINORITY_MIN <= fraction_over <= CEILING_MINORITY_MAX`. Deliberately
coarse, and what the blocking test asserts. Each added record moves a fraction by at most 1/N, so
NO SINGLE ordinary addition can cross it this is measured headroom, not immunity. From a live
18/183 (9.8%), BREACHING the 25% cap takes 38
consecutive over-ceiling additions (37 lands exactly on 0.25, which still passes under `<=`),
or 718 short ones to dilute below the floor against ONE record to break `marks_tail`.
The THIRD arm is the tightest and is stated here because it is the easy one to forget:
CONSOLIDATION. Taking 15 of today's 18 over-ceiling records out of the over-set drops below the
2% floor trimming them to <=60 leaves 3/183 = 1.64%, archiving them outright leaves 3/168 =
1.79% (the denominator moves too); either way, under the floor. That is a real tension with
`test_oversized_records_can_go_green` the ceiling is allowed to go green and it is accepted
rather than papered over: at 3/183 the
constant genuinely IS mis-calibrated and saying so is the signal working. A consolidation PR
large enough to hit it should re-derive the ceiling in the same change.
It still catches genuine mis-calibration in both directions, measured on the real corpus: a
ceiling of 20 flags 60% of records (cutting into the bulk, so every author learns to ignore it),
and a ceiling of 200 flags 0.5% one record which is below the floor and rejected. Note that
"flags NOBODY" is the wrong way to state the upper failure: at a ceiling of 200 the count is
still nonzero because one 230-line record exists, which is exactly why the floor is a fraction
and not `> 0`.
Why not simply re-derive the constant instead: re-deriving fixes the instance and keeps the
mechanism. The v3 fraction band and the v4 percentile containment both failed the same way, one
faster than the other, and picking a new number would queue up the fifth version.
"""
lengths = sorted(record_prose_lines(r) for r in records if r.key)
n = len(lengths)
if n == 0:
return CeilingCalibration(0, 0, 0, 0, 0.0, False, False)
def pct(q: float) -> int:
return lengths[min(int(n * q), n - 1)]
n_over = sum(1 for v in lengths if v > ceiling)
frac = n_over / n
p90, p95 = pct(0.90), pct(0.95)
return CeilingCalibration(
n=n,
p90=p90,
p95=p95,
n_over=n_over,
fraction_over=frac,
marks_tail=p90 <= ceiling <= p95,
flags_minority=CEILING_MINORITY_MIN <= frac <= CEILING_MINORITY_MAX,
)
def _catalog_ok() -> bool:
try:
import scripts.build_decisions_catalog as bc # pyright: ignore[reportMissingImports]
@@ -684,6 +866,19 @@ def main(argv=None) -> int:
archive_records += dl.parse_file(f)
removed, rewritten, demoted = _diff_findings(args.base, args.head) if args.base and args.head else ([], [], [])
oversized = oversized_records(records, args.record_ceiling)
# #674: cross-check the dependency-free reader against the library that WROTE these files.
# Strictly additive — absent PyYAML skips the check (and SAYS so) rather than failing the run.
yaml_faults, yaml_ran = pyyaml_frontmatter_faults(record_wing_files())
if not yaml_ran:
print(
"::notice::decisions-validate: PyYAML is not importable, so the frontmatter cross-check "
"was SKIPPED — every other check ran. This is the expected state on the dependency-free "
"read path (decisions-guard, the Husky hooks); CI's script-tests job runs it with PyYAML "
"present.",
file=sys.stderr,
)
errs = validate(
records,
archive_keys=_archive_keys(),
@@ -692,7 +887,7 @@ def main(argv=None) -> int:
rewritten=rewritten,
archive_records=archive_records,
demoted=demoted,
wing_faults=record_wing_faults(),
wing_faults=record_wing_faults() + yaml_faults,
)
# Aggregate: an unthresholded TREND, not a gate (#620). Printed every run so the number stays
@@ -723,6 +918,20 @@ def main(argv=None) -> int:
file=sys.stderr,
)
# Ceiling calibration drift (#688): a NOTICE, never a failure. The ceiling drifting away from
# the tail boundary is the passage of corpus growth, not a defect in the commit under test — the
# same reasoning `stale_records` is built on. Asserting it in the blocking `script-tests` job
# made the next author of a substantial record pay for an unrelated constant going out of date.
cal = ceiling_calibration(records, args.record_ceiling)
if cal.n and not cal.marks_tail:
print(
f"::notice::decisions-validate: the {args.record_ceiling}-line ceiling has drifted from "
f"the tail boundary of the distribution (p90={cal.p90}, p95={cal.p95}, "
f"{cal.n_over}/{cal.n} records over it). Re-derive it when convenient — this is a "
f"maintenance signal about the constant, not a problem with this change.",
file=sys.stderr,
)
stale = stale_records(records, date.today())
if stale:
listed = "; ".join(f"{h} (stale-after {d})" for h, d in stale)
+34
View File
@@ -121,6 +121,27 @@ if [ -z "$base_before" ] || [ "$base_before" != "$expected_base" ]; then
echo "pr-changed-files: PR #$pr targets '${base_before:-<unreadable>}', not the expected '$expected_base' — the diff would be computed against a different base, failing closed" >&2
exit 1
fi
# Also capture the base's TIP at this same read (ersatztv#707). This costs no extra round trip —
# `prjson_before` is already fetched above for the `.base.ref` check. It answers a DIFFERENT
# question than that check does, and the two are not interchangeable:
# * `.base.ref` (above) answers "did this PR RETARGET to a different branch" — comparing branch
# NAMES is deliberate there (ersatztv#698 route 1 / ersatztv#632), because comparing tip shas
# for that purpose would self-deadlock: `main` advancing on every unrelated merge would fail
# every open enumeration even though the PR still targets the same branch it always did.
# * `.base.sha` (here) answers "did `$expected_base` ADVANCE while THIS enumeration was running."
# `/pulls/{n}/files` diffs against the base's LIVE tip and is offset-paged over several round
# trips; if `main` gains a commit mid-enumeration, Gitea recomputes each subsequent page against
# the new tip independently, so rows can drop out of the result entirely (a file `main` no longer
# differs on) while later rows shift into offset ranges already consumed on the old tip. The
# result reads as a complete, ordinary list — `.base.ref` never changed, `.head.sha` never
# changed, page count and termination all look normal — while silently omitting a page's worth of
# changed paths, including possibly the only code file in the diff. This is a narrower, additional
# check layered on top of the ref check, not a replacement for it.
base_sha_before=$(printf '%s' "$prjson_before" | jq -r '.base.sha // ""' 2>/dev/null || true)
if [ -z "$base_sha_before" ]; then
echo "pr-changed-files: could not read PR #$pr's base tip sha before enumerating — failing closed" >&2
exit 1
fi
PAGE_SIZE=50
MAX_PAGES=40 # 2000 files; beyond this we refuse rather than guess
@@ -240,5 +261,18 @@ if [ -z "$base_after" ] || [ "$base_after" != "$expected_base" ]; then
exit 1
fi
# Same window, the tip-advance question this time (ersatztv#707; see the comment at
# `base_sha_before` above for why this is a DIFFERENT check from `.base.ref`, not a duplicate of
# it). `prjson` is already fetched above to bind the head sha, so this is the same re-read, not a
# new round trip. `$expected_base`'s branch name can be unchanged across the whole enumeration
# while its TIP moved partway through — the exact #707 window: no retarget, no head movement,
# nothing the ref check or the head-sha check can see, yet later pages were diffed against a base
# earlier pages never saw.
base_sha_after=$(printf '%s' "$prjson" | jq -r '.base.sha // ""' 2>/dev/null || true)
if [ -z "$base_sha_after" ] || [ "$base_sha_after" != "$base_sha_before" ]; then
echo "pr-changed-files: base '$expected_base' advanced during enumeration (${base_sha_before:0:7} -> ${base_sha_after:0:7}) — later pages may have been diffed against a base earlier pages were not, failing closed" >&2
exit 1
fi
printf '%s\n' "$files" | grep -v '^$' || true
exit 0
+11 -14
View File
@@ -71,23 +71,20 @@ def test_frontmatter_reader_matches_pyyaml_on_every_real_record():
made the validator crash with ModuleNotFoundError once the corpus was migrated.) A hand parser
is only safe if it provably matches the library that WROTE the files, so this compares the two
across every record rather than on a sample.
Since #674 the comparison itself lives in `decisions_validate.pyyaml_frontmatter_faults`, which
the VALIDATOR now runs too before that it existed only here, so `decisions_validate.py`
happily reported OK on a record PyYAML rejects. This test delegates to that one implementation
rather than keeping a second copy of the comparison, so the suite and the validator cannot
drift apart and agree on what "matches PyYAML" means.
"""
yaml = pytest.importorskip("yaml")
pytest.importorskip("yaml")
import scripts.decisions_validate as dv
files = [p for p in dl.RECORDS_DIR.rglob("*.md")] + [p for p in dl.ARCHIVE_DIR.rglob("*.md")]
files = [f for f in files if dl.has_frontmatter(f.read_text(encoding="utf-8"))]
assert len(files) > 100, f"only {len(files)} frontmatter files found — test would be near-vacuous"
diffs = []
for f in files:
lines = f.read_text(encoding="utf-8").splitlines()
end = next(i for i, ln in enumerate(lines[1:], start=1) if ln.rstrip() == "---")
block = "\n".join(lines[1:end])
mine = dl._read_frontmatter(block)
theirs = yaml.safe_load(block) or {}
theirs = {k: ("" if v is None else str(v)) for k, v in theirs.items()}
if mine != theirs:
for k in set(mine or {}) | set(theirs):
if (mine or {}).get(k) != theirs.get(k):
diffs.append(f"{f.name}:{k}\n mine ={(mine or {}).get(k)!r}\n pyyaml={theirs.get(k)!r}")
assert not diffs, f"{len(diffs)} field(s) differ from PyYAML:\n" + "\n".join(diffs[:5])
faults, ran = dv.pyyaml_frontmatter_faults(files)
assert ran, "PyYAML is importable here, so the comparison must have actually run"
assert not faults, f"{len(faults)} frontmatter fault(s) vs PyYAML:\n" + "\n".join(faults[:5])
+488 -24
View File
@@ -1002,39 +1002,423 @@ def test_budget_total_excludes_the_generated_catalog(tmp_path, monkeypatch):
assert total < 100, f"the 500-line generated catalog leaked into the total ({total})"
def test_real_corpus_ceiling_sits_at_the_TAIL_BOUNDARY_of_the_distribution():
"""Guards the calibration claim. This is the FOURTH version; the failures are the lesson.
def test_real_corpus_ceiling_flags_a_nonempty_proper_minority():
"""Guards the calibration claim. This is the FIFTH version; the failures are the lesson.
v1 `max(under) <= 60 < min(over)` true by construction of those two lists.
v2 a minimum gap WIDTH but a ceiling of 200 also sits in a wide gap, so it passed.
v3 a 2-12% fraction band plus "clear air" measured against `min(over)` the nearest
record ABOVE the ceiling. That made the test a hostage to an unrelated record: one
ordinary 62-line addition reddened it with the ceiling correctly placed, and the only
remedy the assertion admitted was to RAISE the ceiling. That is the ratchet this whole
change abolishes, reinstated as a hard failure in what #631 makes a blocking CI job.
The fraction band had the same coupling more slowly (12 more long records breached it),
and `0 <= headroom` was vacuous `max(under)` is by construction <= ceiling.
remedy the assertion admitted was to RAISE the ceiling.
v4 `p90 <= ceiling <= p95`. Scale-free and correct AS A DEFINITION, but an order statistic
over a SPARSE distribution is a STEP function. The lengths climb to the ceiling and then
jump STRAIGHT to 81 with nothing between, so ONE new record can move p90 by 21 lines and
reddened the BLOCKING `script-tests` job for whoever happened to write it. It reproduced
twice live (#672, #706) and both times the only in-scope remedy was to trim the new
record to fit the constant the ratchet pointed at record authors, which is precisely
what the v3 note says this whole design abolishes.
v4 states the property directly and scale-free: **the ceiling marks the start of the tail**,
i.e. it sits between the 90th and 95th percentile of record lengths. Percentiles move WITH the
corpus, so routine growth cannot ratchet this; it fires only when the ceiling genuinely stops
marking the tail boundary, which is exactly when it should be re-derived.
v5 SPLITS the claim by robustness instead of hunting for a better single assertion:
* the COARSE property the ceiling flags a meaningful minority is asserted HERE,
blocking. One record moves a fraction by at most 1/N, so no SINGLE ordinary addition can
cross it measured headroom, not immunity (38 over-ceiling additions, 718 short ones, or
consolidating 15 of the 18 offenders would each reach a bound).
* the FINE property `p90 <= ceiling <= p95` is now REPORTED by `main()` as a notice.
It is real signal about the CONSTANT drifting out of date, which is the passage of corpus
growth rather than a defect in the commit under test. That is the same reasoning
`stale_records` is built on, and it gets the same treatment.
Note what did NOT change: the ceiling is still 60, and the fine claim is still measured on
every run. v5 moves where each claim is enforced, it does not stop making them.
"""
recs = [r for r in dl.all_active_records() if r.key]
assert len(recs) > 100, f"corpus looks empty ({len(recs)}) — this check would be vacuous"
# A low floor on purpose: this guards against a VACUOUS scan, not against corpus shrinkage.
# At >100 it would red after ~83 legitimate retirements even with the ceiling still calibrated.
assert len(recs) > 20, f"corpus looks empty ({len(recs)}) — this check would be vacuous"
ceiling = dv.RECORD_CEILING_DEFAULT # the value the CLI actually uses; cannot drift from here
lengths = sorted(dv.record_prose_lines(r) for r in recs)
p90 = lengths[int(len(lengths) * 0.90)]
p95 = lengths[int(len(lengths) * 0.95)]
ceiling = dv.RECORD_CEILING_DEFAULT # the value the CLI actually uses; cannot drift from here
cal = dv.ceiling_calibration(recs, ceiling)
assert p90 <= ceiling <= p95, (
f"the ceiling ({ceiling}) no longer marks the tail boundary: p90={p90}, p95={p95}. "
f"Below p90 it cuts into the bulk and every author will learn to ignore it; above p95 it is "
f"parked among the outliers and signals nothing. Re-derive it from the distribution."
assert cal.flags_minority, (
f"the ceiling ({ceiling}) no longer flags a nonempty proper minority of records: "
f"{cal.n_over}/{cal.n} = {cal.fraction_over:.1%} are over it. At 0% it names nobody and "
f"signals nothing; above {dv.CEILING_MINORITY_MAX:.0%} it is cutting into the bulk of the "
f"corpus rather than marking its tail. Re-derive it from the distribution."
)
def test_ceiling_calibration_detects_drift_in_BOTH_directions():
"""The fine claim is asserted here, on a distribution the test OWNS.
This is the point of the v5 split: the property is still pinned, but against synthetic data
instead of the live corpus, so it cannot be reddened by someone else's record landing.
"""
# 100 records: 95 of 20 lines, 5 of 200. Index 90 lands in the short block and index 95 in the
# long one, so p90 == 20 and p95 == 200 — a wide, unambiguous tail boundary to aim at.
recs = [_rec_body(f"a.s{i}", 20) for i in range(95)] + [_rec_body(f"a.l{i}", 200) for i in range(5)]
assert [dv.ceiling_calibration(recs, 60).p90, dv.ceiling_calibration(recs, 60).p95] == [20, 200]
assert dv.ceiling_calibration(recs, 60).marks_tail, "60 sits between p90=20 and p95=200"
assert not dv.ceiling_calibration(recs, 10).marks_tail, "below p90 it cuts into the bulk"
assert not dv.ceiling_calibration(recs, 999).marks_tail, "above p95 it is parked among outliers"
# BOTH ends of `marks_tail` are inclusive. Review found the upper one unpinned — `ceiling <= p95`
# mutated to `<` survived the whole suite. It is notice-only rather than blocking, but an
# unpinned boundary is how a documented claim quietly stops being true.
assert dv.ceiling_calibration(recs, 20).marks_tail, "p90 itself must satisfy the lower bound"
assert dv.ceiling_calibration(recs, 200).marks_tail, "p95 itself must satisfy the upper bound"
assert not dv.ceiling_calibration(recs, 201).marks_tail, "one line above p95 must not"
# and the coarse property separates the same two failure modes
assert not dv.ceiling_calibration(recs, 999).flags_minority, "a ceiling nobody is over signals nothing"
assert not dv.ceiling_calibration(recs, 10).flags_minority, "100% over the ceiling is not a tail"
assert dv.ceiling_calibration(recs, 60).flags_minority
def test_the_coarse_bound_REJECTS_a_badly_placed_ceiling():
"""The blocking property must have teeth.
Review's strongest finding on the first draft: a floor of `fraction_over > 0` was nearly
unfalsifiable measured on the live corpus it accepted every ceiling from 39 to 229, including
the ceiling of 200 the docstring itself offered as the case it catches, because one 230-line
record keeps the count nonzero. A FRACTION floor is what restores the teeth.
The rejections are pinned on a SYNTHETIC distribution: asserting that a specific absurd ceiling
stays rejected by the live corpus is itself growth-coupled (three new 200+ line records flip the
200 arm). Only the acceptance of today's ceiling is checked against live data.
"""
# The TEETH are demonstrated on an owned distribution, for the reason in
# `test_v4_would_have_reddened_where_v5_holds`: an assertion that a specific absurd ceiling is
# rejected by the LIVE corpus is itself growth-coupled (review found that three new 200+ line
# records would flip the 200 arm). 100 records of 30 lines and one of 230 — an outlier-only
# tail, which is precisely the shape a badly-placed ceiling fails to distinguish.
synthetic = [_rec_body(f"a.s{i}", 30) for i in range(100)] + [_rec_body("a.outlier", 230)]
for bad in (200, 229, 230):
cal = dv.ceiling_calibration(synthetic, bad)
assert not cal.flags_minority, (
f"a ceiling of {bad} flags only {cal.n_over}/{cal.n} records and must be rejected, got {cal}"
)
assert not dv.ceiling_calibration(synthetic, 10).flags_minority, "a ceiling of 10 cuts into the bulk"
# The only claim made against the LIVE corpus is the robust one: today's ceiling is accepted.
# Reaching a bound takes 38 consecutive over-ceiling additions, 718 short ones by dilution, or
# consolidating 15 of the 18 offenders — the tightest arm, and the one worth remembering.
recs = [r for r in dl.all_active_records() if r.key]
assert len(recs) > 20, "corpus looks empty — this check would be vacuous"
assert dv.ceiling_calibration(recs, dv.RECORD_CEILING_DEFAULT).flags_minority
def test_ceiling_calibration_is_empty_safe():
"""A vacuous corpus must report both claims FALSE, never a passing default."""
cal = dv.ceiling_calibration([], 60)
assert cal.n == 0 and not cal.marks_tail and not cal.flags_minority
def test_the_minority_band_BOUNDARIES_are_exactly_where_documented():
"""Pins both constants AND both inclusivities, which review found entirely unmutated.
Mutating `0.02 -> 0.03`, `0.25 -> 0.30`, or either `<=` to `<` passed all eight calibration
tests. These are not free parameters they ARE the documented CI-red thresholds, so a silent
shift changes them (a strict cap reds after 37 long additions instead of 38; a strict floor
after 717 short ones instead of 718), quietly falsifying the numbers in `docs.corpus-size-signal`
and `docs/ci-cd.md`.
100-record fixtures make the fraction exact and readable: k over the ceiling IS k%. Both
`2/100` and `25/100` are exactly representable and compare equal to the module constants, so
these are true boundary cases rather than near-misses.
"""
def corpus(n_over: int, total: int = 100):
return [_rec_body(f"a.o{i}", 61) for i in range(n_over)] + [
_rec_body(f"b.u{i}", 10) for i in range(total - n_over)
]
# The bounds are INCLUSIVE — exactly on either edge still passes.
assert dv.ceiling_calibration(corpus(2), 60).flags_minority, "the 2% floor must be inclusive"
assert dv.ceiling_calibration(corpus(25), 60).flags_minority, "the 25% cap must be inclusive"
# ...and one record beyond either edge does not.
assert not dv.ceiling_calibration(corpus(1), 60).flags_minority, "1% is below the floor"
assert not dv.ceiling_calibration(corpus(26), 60).flags_minority, "26% is above the cap"
# The constants themselves, so a change has to be deliberate and visible in the diff.
assert (dv.CEILING_MINORITY_MIN, dv.CEILING_MINORITY_MAX) == (0.02, 0.25)
# `test_adding_ordinary_records_cannot_RED_the_blocking_property` used to live here. It appended two
# long synthetic records to the LIVE corpus and asserted `flags_minority` on the result — which
# crosses the 25% cap TWO records before the production bound does (56/221 vs 54/219), making the
# test named "cannot RED the blocking property" a tighter tripwire than the property it guarded.
# That is the #688 defect in miniature, and the fourth instance found in this change.
#
# Deleted rather than tuned, because both of its jobs are covered without touching live data:
# `test_v4_would_have_reddened_where_v5_holds` demonstrates the v4/v5 contrast on an owned
# distribution, and `test_real_corpus_ceiling_flags_a_nonempty_proper_minority` is the deliberate
# live guard — at the production threshold rather than two records inside it.
def test_ceiling_calibration_IGNORES_keyless_records_and_counts_the_rest():
"""`n` and the `if r.key` filter, both of which review found unpinned.
`main()` passes the UNFILTERED record list, so the filter is load-bearing in production while
every live-corpus test hands this function a pre-filtered list the oracle and production's
input agreed only by accident. The corpus really does carry keyless entries (the generated
"Records formerly in this file" scaffolding, one of them 106 lines), and counting them would
drag p90/p95 around with content that is not a record.
`n` itself lost its only pin when the over-tight live test was deleted: a mutation returning
`n=1` passed everything, which would print a wrong denominator in the drift notice.
The oracle is DYNAMIC and runs at two distinct cardinalities on purpose. The first attempt
asserted `n == 10` against a ten-record fixture, and review killed it: a mutation returning a
constant 10 for every input satisfied it while changing the live denominator from 183 to 10
preserving the exact production defect the test claims to close. A single hardcoded count
cannot distinguish "counts the input" from "returns this number".
"""
for size in (7, 13):
recs = [_rec_body(f"a.s{i}", 10) for i in range(size)]
assert dv.ceiling_calibration(recs, 60).n == size, f"n must count the {size} keyed records given"
recs = [_rec_body(f"a.s{i}", 10) for i in range(9)] + [_rec_body("b.long", 500)]
keyless = _rec(key=None, heading="Records formerly in this file", body="\n".join("x" for _ in range(500)))
assert dv.ceiling_calibration(recs + [keyless], 60) == dv.ceiling_calibration(recs, 60)
def test_ceiling_calibration_counts_over_the_ceiling_EXCLUSIVELY():
"""`n_over` is recomputed inside `ceiling_calibration`, so its boundary needs its own pin.
`oversized_records` has an exclusivity test; this counter does not share its code. Flipping
`>` to `>=` here would silently shift the fraction by the number of records sitting exactly ON
the ceiling (3 in the live corpus), and the mutation survived the whole suite.
"""
recs = [_rec_body("a.under", 59), _rec_body("b.exact", 60), _rec_body("c.over", 61)]
assert dv.ceiling_calibration(recs, 60).n_over == 1
def test_ceiling_calibration_uses_the_95th_percentile_not_a_higher_one():
"""Pins p95's quantile. The synthetic 95/5 fixture cannot tell 0.95 from 0.99, so a mutation
widening the upper quantile survived the whole suite."""
# 100 records: indices 0..89 = 10, 90..94 = 50, 95..98 = 90, 99 = 900.
recs = (
[_rec_body(f"a.s{i}", 10) for i in range(90)]
+ [_rec_body(f"b.m{i}", 50) for i in range(5)]
+ [_rec_body(f"c.h{i}", 90) for i in range(4)]
+ [_rec_body("d.max", 900)]
)
cal = dv.ceiling_calibration(recs, 60)
assert (cal.p90, cal.p95) == (50, 90), f"p95 must read index 95, not a higher quantile: {cal}"
def test_v4_would_have_reddened_where_v5_holds():
"""The v4-vs-v5 contrast, on a distribution the test OWNS rather than the live corpus.
THIRD TIME for this defect class in one change, which is why the fix is to remove the coupling
rather than patch the instance. Round 1 of review caught it in the drift test; round 2 caught it
here, in what looked like a safe `if before.marks_tail:` guard the GUARD was conditional but
the CONCLUSION was still an assertion about live order statistics, and appending 16 ordinary
30-line records (nothing long, nothing unusual) makes `after.marks_tail` true again and fires it:
extra= 0 before(marks=True) after(marks=False) -> reds: False
extra=16 before(marks=True) after(marks=True) -> reds: True
Nothing about this demonstration needs the real corpus. The synthetic base reproduces the shape
that matters a sparse gap immediately above the ceiling, which is what #688 measured on
`main` (nothing at all between 60 and 81) so two over-ceiling additions advance p90 off the
ceiling and break v4, while v5 is untouched.
"""
base = (
[_rec_body(f"a.s{i}", 30) for i in range(90)] # the bulk
+ [_rec_body("a.edge", 60)] # sits exactly ON the ceiling, as main does today
+ [_rec_body(f"a.l{i}", 112) for i in range(9)] # the tail, across a sparse gap
)
before = dv.ceiling_calibration(base, 60)
assert (before.p90, before.p95) == (60, 112), before
assert before.marks_tail and before.flags_minority, before
after = dv.ceiling_calibration(base + [_rec_body("new.a", 107), _rec_body("new.b", 107)], 60)
assert not after.marks_tail, f"v4 must break on these additions, or the contrast is empty: {after}"
assert after.flags_minority, f"v5 must survive what broke v4: {after}"
# --- #674: the validator cross-checks its own parse against PyYAML ------------------------------
_HAZARDS = {
# PyYAML REJECTS: the bare apostrophe closes the single-quoted scalar early.
"apostrophe": "rule: 'SQLite's LOWER() folds ASCII only'",
# PyYAML ACCEPTS but reads a DIFFERENT value: ` #` starts a comment, truncating the rule.
"unquoted-hash": "rule: use --flag #2 for this",
}
def _wing_with(tmp_path: Path, frontmatter_line: str) -> tuple[Path, Path]:
"""A record wing containing one file whose frontmatter carries `frontmatter_line`."""
records = tmp_path / "records" / "ci"
records.mkdir(parents=True)
(records / "a.md").write_text(
"---\n"
"key: ci.a\n"
"title: 'T'\n"
"status: active\n"
"since: '2026-01-01'\n"
"supersedes: none\n"
"superseded-by: none\n"
f"{frontmatter_line}\n"
"signals: 's'\n"
"---\n\nprose.\n"
)
archive = tmp_path / "archive"
archive.mkdir(parents=True)
return records, archive
@pytest.mark.parametrize("hazard", sorted(_HAZARDS))
def test_pyyaml_crosscheck_catches_frontmatter_the_hand_reader_accepts(tmp_path, hazard):
"""#674, both shapes. Hit twice in one session by two independent agents (#578, #651)."""
pytest.importorskip("yaml")
records, _ = _wing_with(tmp_path, _HAZARDS[hazard])
faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md")))
assert ran, "PyYAML is installed here, so the cross-check must have run"
assert faults, f"the {hazard} hazard slipped through the cross-check"
@pytest.mark.parametrize("hazard", sorted(_HAZARDS))
def test_the_hand_reader_really_IS_blind_to_these(tmp_path, hazard):
"""The positive control: pin the MECHANISM, so this suite cannot pass for the wrong reason.
If `record_wing_faults` ever started catching these on its own, the cross-check above could be
deleted and the tests would stay green while the guard vanished. Asserting that the pre-#674
machinery reports these files as CLEAN is what makes the cross-check's red meaningful — and it
is the exact state #674 was filed about: `decisions_validate.py` printed OK on input the
writer's own library rejects.
"""
records, archive = _wing_with(tmp_path, _HAZARDS[hazard])
assert dv.record_wing_faults(records, archive) == [], (
"the structural guard now catches this by itself — re-derive whether the PyYAML "
"cross-check is still the thing closing this gap"
)
def test_crosscheck_REPORTS_an_impossible_date_instead_of_crashing(tmp_path):
"""PyYAML raises a bare `ValueError`, not a `YAMLError`, for a well-shaped impossible date.
`stale-after: 2026-06-31` (June has 30 days) escaped an `except yaml.YAMLError` as a traceback,
killing the validator on any machine with PyYAML including the Husky pre-commit hook. A check
documented as "strictly additive, must never be the reason the validator cannot run" must
REPORT this, so the except is deliberately broad.
"""
pytest.importorskip("yaml")
records, _ = _wing_with(tmp_path, "stale-after: 2026-06-31")
faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md")))
assert ran
assert faults and "REJECTS" in faults[0], faults
assert "ValueError" in faults[0], f"the fault must name the real exception: {faults[0]}"
def test_crosscheck_survives_a_TYPED_mapping_key(tmp_path):
"""PyYAML returns typed keys, so a stray `1: x` yields int 1 where the reader yields "1".
Sorting that mixed set raised `TypeError` an uncaught traceback replacing what
`_unknown_frontmatter_keys` used to report as an actionable error. Removing the `key=str` sort
key restores the crash, and without this test every other test here stays green.
"""
pytest.importorskip("yaml")
records, _ = _wing_with(tmp_path, "1: stray")
faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md")))
assert ran
assert faults, "a typed mapping key must be reported, not swallowed"
assert any("1" in f for f in faults), faults
def test_pyyaml_crosscheck_is_clean_on_the_REAL_corpus():
"""No false positives. A cross-check that flags correct records would be reverted within a day."""
pytest.importorskip("yaml")
files = dv.record_wing_files()
assert len(files) > 100, f"only {len(files)} wing files found — this check would be near-vacuous"
faults, ran = dv.pyyaml_frontmatter_faults(files)
assert ran
assert faults == [], "the cross-check disagrees with the live corpus:\n" + "\n".join(faults[:5])
def test_crosscheck_skips_cleanly_when_pyyaml_is_absent(tmp_path, monkeypatch):
"""The read path stays dependency-free (#674's second Done-when box).
`decisions-guard`, the Husky hooks and every contributor machine install nothing, so an absent
PyYAML must SKIP the cross-check rather than fault or crash while every other check runs.
"""
records, _ = _wing_with(tmp_path, _HAZARDS["apostrophe"])
import builtins
real_import = builtins.__import__
def no_yaml(name, *a, **kw):
if name == "yaml":
raise ImportError("no yaml here")
return real_import(name, *a, **kw)
monkeypatch.setattr(builtins, "__import__", no_yaml)
faults, ran = dv.pyyaml_frontmatter_faults(sorted(records.rglob("*.md")))
assert ran is False, "an absent PyYAML must report that the check did not run"
assert faults == [], "a skipped check must not manufacture faults"
def test_main_ANNOUNCES_a_skipped_crosscheck(capsys, monkeypatch):
"""A skipped check that says nothing is the '#603 stale-after' defect: reports success, does
nothing. The skip is legitimate; staying quiet about it is not."""
monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", lambda files: ([], False))
assert dv.main([]) == 0
err = capsys.readouterr().err
assert "cross-check" in err and "SKIPPED" in err, err
def test_main_FEEDS_the_crosscheck_the_REAL_wing_files(monkeypatch, capsys):
"""Pins the cross-check's INPUT, not just that its output is consumed.
Mutation testing found this hole: replacing `pyyaml_frontmatter_faults(record_wing_files())`
with `pyyaml_frontmatter_faults([])` in main() left the ENTIRE suite green exit 0, no skip
notice, every other test passing. The two wiring tests monkeypatch the function itself, so they
prove the return value reaches `errs`; nothing proved the argument was the corpus. That is the
'#609 marker that printed OK while doing nothing' defect one level up, which is the exact thing
this record indicts and the sibling of `test_main_actually_CALLS_the_wing_scan`.
"""
seen: list[list] = []
def spy(files):
seen.append(list(files))
return [], True
monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", spy)
dv.main([])
assert seen, "main() never called the cross-check at all"
assert len(seen[0]) > 100, f"main() passed only {len(seen[0])} file(s) — not the real wings"
assert set(seen[0]) == set(dv.record_wing_files()), (
"main() passed a file list that is not record_wing_files() — the cross-check is not seeing "
"the corpus it is supposed to check"
)
def test_main_FAILS_when_the_crosscheck_reports_a_fault(capsys, monkeypatch):
"""Wiring test: the faults must reach the exit code, not just be computed.
Without this, `wing_faults=record_wing_faults() + yaml_faults` could drop the second term and
every other test here would still pass.
"""
monkeypatch.setattr(dv, "pyyaml_frontmatter_faults", lambda files: (["x.md: bogus fault"], True))
assert dv.main([]) == 1
assert "bogus fault" in capsys.readouterr().err
def test_retired_budget_flag_says_it_is_ignored(capsys):
"""A retired flag must announce itself, not no-op silently.
@@ -1046,24 +1430,104 @@ def test_retired_budget_flag_says_it_is_ignored(capsys):
def test_no_budget_flag_means_no_retirement_warning(capsys):
# Match the retirement notice specifically, not a bare "RETIRED": a legitimate record whose
# TITLE contains that word and whose `stale-after` has passed gets printed by the stale notice,
# which would red this on an unrelated corpus change.
dv.main([])
assert "RETIRED" not in capsys.readouterr().err
assert "is RETIRED and was IGNORED" not in capsys.readouterr().err
def test_main_reports_ceiling_drift_as_a_NOTICE_and_still_exits_0(capsys):
"""The fine claim's live wiring (#688): the drift notice must fire, and must NOT turn the run
red the entire point of the v5 split.
The ceiling is DERIVED as one line above the longest record, so it is off the tail boundary by
definition. A hardcoded 999 looked safe and was not: review showed ten valid 1000-line records
would put p95 at 1000, making 999 calibrated so the notice would stop firing and this test
would go RED, for a corpus change that is nobody's defect.
"""
longest = max(dv.record_prose_lines(r) for r in dl.all_active_records() if r.key)
assert dv.main(["--record-ceiling", str(longest + 1)]) == 0
err = capsys.readouterr().err
drift = [ln for ln in err.splitlines() if "drifted from the tail boundary" in ln]
assert len(drift) == 1, err
assert drift[0].startswith("::notice::"), f"drift must be a notice, not a warning: {drift[0]}"
def test_main_reports_drift_IFF_the_ceiling_is_off_the_tail_boundary(capsys):
"""The complement of the test above — asserting the WIRING, not the corpus's current state.
The obvious way to write this is `dv.main([]); assert "drifted" not in err`, and that is a trap
review caught: `main()` emits the notice exactly when `p90 <= 60 <= p95` is false over the LIVE
corpus, so such a test fails under precisely the condition #688 exists to stop failing — it
would move v4's assertion three functions down and leave it in the same blocking job. Today p90
sits exactly ON the ceiling, so ONE new over-ceiling record would have reddened it.
So the oracle is `ceiling_calibration` itself: whatever the corpus currently looks like, the
notice must be present iff the fine claim is false. The 999 case pins that at least one branch
is genuinely exercised, so this cannot pass by never firing.
"""
recs = [r for r in dl.all_active_records() if r.key]
# Non-empty is all the derivations below need; a higher floor would itself be a growth tripwire.
assert recs, "corpus is empty — the derived ceilings need at least one record"
lengths = sorted(dv.record_prose_lines(r) for r in recs)
# Both ceilings are DERIVED so each branch is guaranteed by construction, not by luck. Review
# caught the earlier version relying on the live 60/999 pair: once one 61-line record lands,
# BOTH of those drift, and an UNCONDITIONAL notice would have passed the test.
# * p90 itself is always calibrated — `p90 <= p90 <= p95` holds for any distribution.
# * one line above the longest record is always off the tail — it exceeds p95 by definition.
quiet_ceiling = lengths[min(int(len(lengths) * 0.90), len(lengths) - 1)]
drift_ceiling = lengths[-1] + 1
expectations = []
for ceiling in (quiet_ceiling, drift_ceiling):
expected = not dv.ceiling_calibration(recs, ceiling).marks_tail
dv.main(["--record-ceiling", str(ceiling)])
err = capsys.readouterr().err
assert ("drifted from the tail boundary" in err) is expected, (
f"ceiling {ceiling}: expected drift notice={expected}, got the opposite"
)
expectations.append(expected)
assert expectations == [False, True], (
f"the two derived ceilings must exercise BOTH branches, got {expectations} — otherwise an "
f"unconditional notice (or none at all) would pass this test"
)
def test_main_actually_REPORTS_the_ceiling_and_the_trend(capsys):
"""The new signal's live wiring was untested: `if oversized:` -> `if False:`, or bumping the
default ceiling to 999999, left every test green while main() reported nothing. Only the pure
function `oversized_records()` was covered so the replacement signal could silently do
nothing, which is the exact defect this change exists to retire."""
nothing, which is the exact defect this change exists to retire.
Stated as an IFF against the live offender list rather than `assert over` (#688): the ceiling
is ALLOWED to go green `test_oversized_records_can_go_green` says so explicitly so a bare
precondition that the corpus still has an offender would red the blocking job the day someone
consolidates the last one, punishing exactly the work the warning asks for."""
dv.main([])
err = capsys.readouterr().err
assert "prose lines across" in err, "the aggregate trend notice must always print"
assert "exceed the" in err and "prose ceiling" in err, "the per-record ceiling warning must print"
over = [r.key for r in dl.all_active_records()
if r.key and dv.record_prose_lines(r) > dv.RECORD_CEILING_DEFAULT]
assert over, "precondition: the live corpus has at least one over-ceiling record"
assert any(k in err for k in over), "the warning must NAME the offending records"
warned = "exceed the" in err and "prose ceiling" in err
assert warned is bool(over), f"ceiling warning printed={warned} but {len(over)} record(s) are over it"
if over:
assert any(k in err for k in over), "the warning must NAME the offending records"
# The IFF above is only non-vacuous while an offender exists: once the corpus is legitimately
# consolidated to zero, `False is False` passes even if main()'s whole `if oversized:` branch
# were deleted. So force the branch with a ceiling nothing can sit under. It is -1, not 0:
# an empty record body is validator-valid and `record_prose_lines` returns 0, so a corpus of
# empty-bodied records has no offender at 0. Below zero the arm cannot go vacuous at all.
dv.main(["--record-ceiling", "-1"])
forced = capsys.readouterr().err
assert "exceed the" in forced and "prose ceiling" in forced, (
"at a ceiling of -1 every record is over it — the warning branch must fire"
)
def test_trend_notice_reports_record_prose_and_scaffolding_separately(capsys):
+13 -1
View File
@@ -70,8 +70,20 @@ if "/pulls/" in url:
# how this stub failed when the argument was added: the eight failures were all positive cases.
# Fail-closed, so not dangerous, but it would have made the advisory hook prompt on every
# docs-only PR.
#
# `.base.sha` joined it for the same reason one release later (ersatztv#707), and the symptom
# repeated almost exactly: NINE failures, every one a positive control, because the enumeration
# now binds the base's TIP across the paging window and an absent tip fails closed. Worth stating
# as a standing property of this stub rather than a second anecdote — it serves the fields the
# SHARED enumeration reads, so every new binding the script learns must be modelled here too, and
# the tell is always a wave of positive cases going red at once.
base_sha = os.environ.get("STUB_BASE_SHA", "b" * 40)
alt_base = state / "base_sha_after.txt"
if alt_base.exists() and nread >= 1:
base_sha = alt_base.read_text().strip()
print(json.dumps({"head": {"sha": shas[0]},
"base": {"ref": os.environ.get("STUB_BASE", "main")},
"base": {"ref": os.environ.get("STUB_BASE", "main"),
"sha": base_sha},
"body": "no linked issue here"}))
sys.exit(0)
+571 -2
View File
@@ -80,7 +80,21 @@ if "/pulls/" in url:
if base_after.exists() and n >= 1: # retargeted mid-enumeration
base = base_after.read_text().strip()
print(json.dumps({"head": {"sha": sha}, "base": {"ref": base}}))
# The base's TIP sha (ersatztv#707) — independent of the branch NAME above. Defaults to a fixed
# value so an unmoved base is the default for every existing test that never touches this.
base_sha = os.environ.get("STUB_BASE_SHA", "aaaa000000000000000000000000000000000a")
base_sha_before = state / "pr_base_sha_before.txt"
base_sha_after = state / "pr_base_sha_after.txt"
if base_sha_before.exists() and n == 0:
base_sha = base_sha_before.read_text().strip()
if base_sha_after.exists() and n >= 1:
base_sha = base_sha_after.read_text().strip()
base_obj = {"ref": base}
if base_sha != "MISSING":
base_obj["sha"] = base_sha
print(json.dumps({"head": {"sha": sha}, "base": base_obj}))
sys.exit(0)
print("{}")
@@ -125,6 +139,21 @@ def enumerate_files(tmp_path):
"""The PR is retargeted to `ref` between the files pages and the binding re-read."""
(state / "pr_base_after.txt").write_text(ref)
def base_sha_is_already(self, sha):
"""The base tip sits at `sha` for the WHOLE run — i.e. it advanced before this
enumeration started, which is outside the window the #707 guard polices."""
(state / "pr_base_sha_before.txt").write_text(sha)
(state / "pr_base_sha_after.txt").write_text(sha)
def base_sha_moves_to(self, sha):
"""The base branch's TIP advances to `sha` between the files pages and the binding
re-read, with the branch NAME unchanged (ersatztv#707)."""
(state / "pr_base_sha_after.txt").write_text(sha)
def base_sha_missing_after(self):
"""The base tip sha is absent/unparseable on the post-paging re-read."""
(state / "pr_base_sha_after.txt").write_text("MISSING")
def run(self, expected_sha=SHA, args=("timothy", "ersatztv", "42"),
expected_base="main"):
argv = ["bash", str(SCRIPT), *args, expected_sha]
@@ -343,6 +372,78 @@ def test_positive_control_an_unmoved_base_still_enumerates(enumerate_files):
assert enumerate_files.paths() == ["docs/a.md", "ErsatzTV/Program.cs"]
def test_a_BASE_ADVANCE_DURING_enumeration_fails_closed(enumerate_files):
"""ersatztv#707. `/pulls/{n}/files` diffs EACH PAGE against the base's LIVE tip; if `main`
advances between the pre-paging read and the post-paging re-read, later pages can be diffed
against a base earlier pages never saw. Rows can drop out of the result entirely (a file `main`
no longer differs on) while later rows shift into offset ranges already consumed against the old
tip. `.base.ref` never changes here that is the entire point of the defect and of this being an
ADDITIONAL guard layered on top of the existing retarget check, not a reclassification of it."""
enumerate_files.set_pages(_rows(["docs/a.md", "ErsatzTV/Program.cs"]))
enumerate_files.base_sha_moves_to("cccc000000000000000000000000000000000c")
r = enumerate_files.run()
assert r.returncode != 0, f"expected fail-closed, got {r.returncode}: stdout={r.stdout!r}"
assert not r.stdout.strip(), f"stdout must be meaningless on a failed enumeration: {r.stdout!r}"
assert "base" in r.stderr and "advanced" in r.stderr, (
f"stderr should name the base-advance failure distinctly, got: {r.stderr!r}")
assert re.search(r"[0-9a-f]{7}", r.stderr), (
f"stderr should name both short shas involved, got: {r.stderr!r}")
def test_positive_control_an_UNMOVED_base_sha_still_enumerates_across_pages(enumerate_files):
"""Without this, the fail-closed tests around it could be passing only because the new guard
broke the ordinary path outright rather than because it correctly distinguishes movement from
no movement. Deliberately multi-page, to prove the guard survives several round trips."""
enumerate_files.set_pages(_rows([f"docs/f{i}.md" for i in range(50)]),
_rows(["ErsatzTV/Program.cs"]))
paths = enumerate_files.paths()
assert len(paths) == 51
assert "ErsatzTV/Program.cs" in paths
def test_the_707_WINDOW_base_ref_unchanged_but_TIP_advanced_still_fails(enumerate_files):
"""The exact #707 window, and the centerpiece of this suite. The base BRANCH NAME is unchanged
throughout (`main` -> `main`), so the pre-existing `.base.ref` retarget guard sees nothing to
object to by construction, since a mere advance is not a retarget. Only the branch's TIP moved.
A test that passes here proves the NEW sha-based guard is what fired, not the old ref guard,
which cannot see this case at all."""
enumerate_files.set_pages(_rows(["docs/a.md"]), _rows(["ErsatzTV/Program.cs"]))
enumerate_files.base_sha_moves_to("dddd000000000000000000000000000000000d")
r = enumerate_files.run() # expected_base="main" throughout; ref never moves
assert r.returncode != 0, (
"base ref stayed 'main' but its tip advanced mid-enumeration; must still fail closed "
f"(got {r.returncode}, stdout={r.stdout!r}, stderr={r.stderr!r})")
def test_a_base_that_ADVANCED_BEFORE_the_enumeration_STILL_SUCCEEDS(enumerate_files):
"""ersatztv#707, the deadlock this guard must NOT cause — and the reason the check is scoped to
the window rather than to the base being "current".
`main` advances constantly. If the guard compared the base tip against anything other than what
it observed at the START of THIS enumeration, every open PR would fail closed on every unrelated
merge to `main` losing its exemption for reasons that have nothing to do with it. Here the base
already sits at a tip different from the suite's default before the first page is requested, and
then holds still: that is an ordinary, healthy PR and it must enumerate normally.
Note this is deliberately NOT the same test as the unmoved-base control: that one pins "nothing
happened", this one pins "something happened, but OUTSIDE the window, so it is none of our
business."
"""
enumerate_files.base_sha_is_already("eeee000000000000000000000000000000000e")
enumerate_files.set_pages(_rows(["docs/a.md", "docs/b.md"]), [])
assert enumerate_files.paths() == ["docs/a.md", "docs/b.md"], (
"a PR whose base advanced BEFORE this enumeration began was failed closed; the guard is "
"comparing against a stale expectation instead of the tip it actually started from")
def test_an_UNREADABLE_base_sha_on_the_AFTER_read_fails_closed(enumerate_files):
"""Consistent with every other guard in this script: an absent/unparseable field is never read
as 'no movement' it is fail-closed, same as the head-sha and base-ref binding above."""
enumerate_files.set_pages(_rows(["docs/a.md"]))
enumerate_files.base_sha_missing_after()
assert enumerate_files.fails_closed()
def test_the_base_comparator_is_the_BRANCH_NAME_never_the_TIP_SHA():
"""`.base.ref` is compared, never `.base.sha` — matching `post-review-verdict.sh` (ersatztv#632).
@@ -789,9 +890,100 @@ if "-X" in args and args[args.index("-X") + 1] == "POST":
payload = args[args.index("-d") + 1]
(out / "posted.json").write_text(payload)
(out / "posted_url.txt").write_text(url)
# EVERY post is also appended (ersatztv#706 race 2): the repair path POSTs a second time, and a
# test that only ever saw the last write could not tell "posted success, then repaired it to
# pending" from "posted pending once".
#
# Re-serialized COMPACT rather than appended verbatim: `jq -n` pretty-prints by default, so the
# payloads arrive spanning several lines and a raw append would produce a file that is not JSONL
# at all. (Found the honest way — the first draft of this recorder broke both race-2 tests.)
with (out / "posted_all.jsonl").open("a") as fh:
fh.write(json.dumps(json.loads(payload)) + "\n")
print("{}")
sys.exit(0)
# The PR timeline, which the retarget fence counts (ersatztv#706 race 1). Checked BEFORE the status
# branch: the fence's URL does not contain "/status", but keeping the order explicit means a later
# edit to either pattern cannot silently reroute one endpoint into the other's handler.
#
# Real shapes, measured on this instance and deliberately mirrored, because the job's guards are
# type-sensitive: this endpoint returns a BARE ARRAY of events, each with a `type`; a retarget is
# `change_target_branch` (confirmed on PR #703, the route-1 reproduction, which carries exactly two;
# and on PR #717, never retargeted, which carries none).
if "/timeline" in url:
mode = os.environ.get("STUB_TIMELINE_MODE", "none")
page = 1
for part in url.split("?", 1)[-1].split("&"):
if part.startswith("page="):
page = int(part.split("=", 1)[1])
if mode == "unreadable":
print("<html>502 Bad Gateway</html>")
sys.exit(0)
if mode == "transport-error":
sys.exit(22)
# Page 2+ is always the validated EMPTY page that terminates the walk. Without a real terminator
# the job would page to its cap and refuse to trust the count, which is a different branch from
# the one most of these tests mean to exercise.
if page > 1:
print("[]")
sys.exit(0)
n_before, n_after = 0, 0
if mode.startswith("stable:"):
n_before = n_after = int(mode.split(":", 1)[1])
elif mode.startswith("moves:"):
# "moves:A,B" — A retarget events on the fence's FIRST count, B on the re-count taken just
# before the POST. This is the race-1 window: the PR was retargeted while the job classified.
a, b = mode.split(":", 1)[1].split(",")
n_before, n_after = int(a), int(b)
ctr = out / "timeline_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
n = n_before if seen == 0 else n_after
print(json.dumps([{"id": 1000 + i, "type": "change_target_branch",
"old_ref": "main", "new_ref": "scratch"} for i in range(n)]
+ [{"id": 900, "type": "comment"}]))
sys.exit(0)
# The per-POST status HISTORY (`/statuses/{sha}`), read by the race-2 post-write verification. It is
# a BARE ARRAY, one row per POST — a different shape AND a different meaning from the combined
# `/commits/{sha}/status` below, which returns an object carrying the LATEST row per context. Both
# shapes were measured on the same live head (24 rows vs 12). Modelling them identically would have
# made the post-write check untestable, since the thing it looks for is precisely a row the combined
# endpoint no longer shows.
if "/statuses/" in url:
mode = os.environ.get("STUB_HISTORY_MODE", "none")
rows = []
if mode.startswith("human-after-post"):
# The raced verdict: absent when the high-water mark is taken, present afterwards. Its id is
# ABOVE the mark, which is what makes it detectable.
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
rows = [{"id": 5000, "context": "review-verdict/h10", "status": "failure",
"creator": {"login": "timothy"},
"description": "Review-verdict: BLOCKED @ a9e3e23 (base: main)"}]
elif mode == "sentinel-after-post":
# Another overlapping run repaired mid-flight: its SENTINEL lands above this run's mark, while
# the human row it records sits BELOW the mark and is therefore invisible here (round 5).
ctr = out / "history_reads.txt"
seen = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(seen + 1))
if seen > 0:
rows = [{"id": 6000, "context": "review-verdict/h10", "status": "pending",
"creator": None,
"description": "Human verdict raced this exemption write — re-post the verdict"}]
elif mode == "stale-human-already-present":
# A human verdict that was ALREADY in the history before this run — e.g. one whose recorded
# base did not match, which `read_existing_verdict` deliberately declines to honour. It must
# NOT be mistaken for a raced write, or every later run of that PR would repair its own
# exemption to pending forever.
rows = [{"id": 10, "context": "review-verdict/h10", "status": "success",
"creator": {"login": "timothy"},
"description": "Review-verdict: MERGEABLE @ a9e3e23 (base: other)"}]
print(json.dumps(rows))
sys.exit(0)
if "/status" in url:
# Configurable. Hardcoding "no verdict yet" left the ersatztv#647 emptiness guard and the
# never-overwrite short-circuit unreachable: neither could be made to fire, so mutations
@@ -813,6 +1005,19 @@ if "/status" in url:
"creator": {"login": "timothy"},
"description": "Review-verdict: BLOCKED @ a9e3e23 (base: main)"}]}))
sys.exit(0)
if mode.startswith("sentinel-appears-on-read:"):
# A repair sentinel written by ANOTHER, overlapping run between this job's first read and its
# last-moment re-read (ersatztv#706 round 3). Creator is null: the sentinel is machine-written.
nth = int(mode.split(":", 1)[1])
ctr = out / "status_reads.txt"
n = int(ctr.read_text()) if ctr.exists() else 0
ctr.write_text(str(n + 1))
if n + 1 < nth:
print(json.dumps({"statuses": []})); sys.exit(0)
print(json.dumps({"statuses": [
{"context": "review-verdict/h10", "status": "pending", "creator": None,
"description": "Human verdict raced this exemption write — re-post the verdict"}]}))
sys.exit(0)
if mode == "transport-error":
# Real `gh()` is `curl -sf`: an HTTP error exits 22 with EMPTY stdout.
sys.exit(22)
@@ -849,11 +1054,13 @@ print("{}")
def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy",
status_mode: str = "none", jq16: bool = False,
status_creator: str | None = "timothy",
status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)"):
status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
timeline_mode: str = "none", history_mode: str = "none"):
"""Execute the workflow's classify `run:` block with a stubbed enumeration script.
Returns the status payload the job POSTed, or None if it posted nothing.
"""
tmp_path.mkdir(parents=True, exist_ok=True) # the chained sentinel test passes sub-paths
bindir = tmp_path / "bin"; bindir.mkdir()
curl = bindir / "curl"; curl.write_text(WORKFLOW_STUB_CURL); curl.chmod(0o755)
if jq16:
@@ -873,6 +1080,8 @@ def _run_classify(tmp_path, enum_stub: str | None, author: str = "timothy",
env["STUB_STATUS_MODE"] = status_mode
env["STUB_STATUS_CREATOR"] = status_creator or ""
env["STUB_STATUS_DESC"] = status_desc
env["STUB_TIMELINE_MODE"] = timeline_mode
env["STUB_HISTORY_MODE"] = history_mode
env.update({
"GITEA_TOKEN": "stub",
"BASE_URL": "http://gitea.example/api/v1",
@@ -1542,3 +1751,363 @@ def test_a_branch_name_containing_a_PAREN_cannot_truncate_into_the_current_base(
"a verdict recorded against branch 'main)evil' was inherited by a PR targeting 'main' — the "
f"base value is being truncated at the first ')'. Log:\n{r.stdout[-600:]}")
assert posted["state"] == "pending"
# --- ersatztv#706: the retarget fence (race 1) and the post-write repair (race 2) -----------------
#
# Race 1 was reproduced LIVE before any of this was written, because the fix turns on which Gitea
# behaviours are real rather than on what the docs imply. Measured on Gitea 1.25.4:
# * `pull_request_target` runs for one PR are NOT auto-cancelled — probe PR #722, run 7520
# (`opened`) ran to completion 20s AFTER run 7521 (`synchronize`) started. The older run finished
# LAST, which is exactly the mechanism by which a stale classification overwrites a fresh one.
# * A NON-CANCELLING concurrency group does not serialize them either: with the group active, runs
# 7528 and 7529 still overlapped and 7528 ended 36s after 7529 began. #706's headline proposal is
# therefore refuted, not merely unattractive.
# * Gitea DOES auto-cancel superseded `push` runs on a branch — a control workflow carrying no
# `concurrency:` key at all showed that — which is why the concurrency-group probe was confounded
# until the control separated the two.
# The fence keys on the timeline's `change_target_branch` COUNT because the branch NAME is
# ABA-vulnerable (`main → S → main` reads `main` at both ends), while the count is monotonic.
def _posted_sequence(tmp_path):
"""Every status POST the job made, in order — not just the last one."""
f = tmp_path / "posted_all.jsonl"
return [json.loads(l) for l in f.read_text().splitlines() if l.strip()] if f.exists() else []
def test_a_RETARGET_DURING_the_run_posts_NOTHING(tmp_path):
"""THE race-1 test. A docs-only PR that would otherwise be exempted is retargeted while the job
classifies, so this run's answer describes a base the PR may no longer target. It must write
nothing at all and leave the field to the successor run the retarget's `edited` event queues."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="moves:0,1")
assert r.returncode == 0, r.stderr
assert posted is None, (
"a stale run posted its classification after the PR was retargeted underneath it — the "
f"retarget fence did not fire. Log:\n{r.stdout[-900:]}")
# Assert the DISCRIMINATOR, not just the outcome: several unrelated failures also end in "posted
# nothing", so the state alone cannot tell a working fence from a broken job.
assert "retargeted while this job was classifying" in r.stdout, (
f"nothing was posted, but NOT via the retarget fence. Log:\n{r.stdout[-900:]}")
def test_positive_control_a_QUIET_run_still_posts_its_exemption(tmp_path):
"""Without this, the test above passes against a job that simply never posts."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none")
assert posted is not None and posted["state"] == "success", (
f"an undisturbed docs-only PR lost its exemption. Log:\n{r.stdout[-900:]}")
def test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_path):
"""The fence keys on MOTION during this run, never on "has ever been retargeted".
A PR legitimately retargeted once, long before this run, carries a permanently non-zero event
count. Keying on the count being non-zero rather than on it CHANGING would deadlock that PR's
exemption forever, which is a worse failure than the race being fixed.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="stable:2")
assert posted is not None and posted["state"] == "success", (
"a PR with a settled, non-zero retarget history was refused its exemption — the fence is "
f"testing the count's VALUE instead of its MOVEMENT. Log:\n{r.stdout[-900:]}")
@pytest.mark.parametrize("mode", ["unreadable", "transport-error"])
def test_an_UNTRUSTED_retarget_count_withholds_the_EXEMPTION(tmp_path, mode):
"""A `success` that cannot be shown to describe the PR's current base must not be written. An
absent required check blocks the merge, which is the safe direction."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode=mode)
assert posted is None, (
"an exemption success was posted even though the retarget count could not be established "
f"({mode}). Log:\n{r.stdout[-900:]}")
@pytest.mark.parametrize("mode", ["unreadable", "transport-error"])
def test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH(tmp_path, mode):
"""The other half, and the one that keeps a timeline outage from taking every PR down with it.
`pending` cannot turn an unreviewed head green it is what blocks the merge so withholding it
buys no safety and would strand ordinary PRs with no status at all. Only the exemption is gated.
"""
posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode=mode)
assert posted is not None and posted["state"] == "pending", (
"an ordinary PR was left with NO status because the timeline was unreadable; only the "
f"exemption success should be gated on the count. Log:\n{r.stdout[-900:]}")
def test_a_human_verdict_landing_AFTER_the_POST_is_repaired_to_pending(tmp_path):
"""Race 2's remainder. The pre-POST re-read cannot see a verdict that lands between it and the
write, and there is no compare-and-set to make the two one operation. So the write is verified
AFTERWARDS and repaired in the safe direction: silently greening an explicit human BLOCKED is the
worst outcome this gate can produce, and strictly worse than a stall.
The repair is `pending`, never a copy of the human's `failure` — re-posting their state under the
machine credential would attribute a human verdict to this job.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="human-after-post")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"expected the exemption write followed by a repair; the post-write verification did not "
f"fire. Posts: {seq}. Log:\n{r.stdout[-900:]}")
assert seq[0]["state"] == "success"
assert seq[1]["state"] == "pending", f"the raced exemption was not repaired: {seq}"
assert posted is not None and posted["state"] == "pending"
def test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair(tmp_path):
"""The false-positive guard, and the reason the check compares ids instead of asking "does a human
verdict exist".
`read_existing_verdict` deliberately declines to honour a human verdict whose recorded base does
not match this PR's — so such a row sits in the history forever. A presence test would fire on it
on EVERY later run, repair each exemption to `pending`, and permanently deadlock a PR that had one
mismatched verdict once. Only rows newer than the high-water mark count.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"),
history_mode="stale-human-already-present")
seq = _posted_sequence(tmp_path)
assert len(seq) == 1, (
"an old, already-present human verdict row was mistaken for one that raced this run's write, "
f"so the exemption was repaired away. Posts: {seq}. Log:\n{r.stdout[-900:]}")
assert posted is not None and posted["state"] == "success"
# --- ersatztv#711: the PROTECTED list must cover every mirror of the enforcement hooks -----------
def test_a_CODEX_hook_copy_is_a_PROTECTED_path(tmp_path):
"""`.codex/hooks/` is a byte-identical mirror of `.claude/hooks/`, including
`pretooluse-merge-consent.sh`. Editing the `.claude/` copy correctly voids both exemptions;
before this, editing its `.codex/` twin did not the rule "a PR that can weaken the gate must
not exempt itself from the gate" was written as a path list and the list had gone incomplete.
Latent rather than live today (`.codex/` is untracked and gitignored, and a PR cannot touch a
path that is not in the repo), which is exactly why it needs a test: it becomes live silently,
the moment anyone tracks the directory.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md", ".codex/hooks/pretooluse-merge-consent.sh"))
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a PR editing a .codex/ copy of the merge-consent hook was granted the docs-only exemption "
f"(desc={posted['description']!r})")
# Assert the DISCRIMINATOR: a non-exempt outcome is reachable by several routes, so the state
# alone cannot show the PROTECTED branch is what rejected it.
assert "protected" in r.stdout.lower(), (
f"rejected, but NOT by the protected-path branch. Decision log:\n{r.stdout[-800:]}")
def test_BOTH_hook_directories_are_in_the_PROTECTED_pattern():
"""A structural pin over the list itself. The behavioural test above proves `.codex/` is covered
today; this one states the INVARIANT the two hook directories are mirrors, so any future edit
that drops one while keeping the other is the bug, not a simplification."""
src = _code_lines(WORKFLOW)
protected = [l for l in src.splitlines() if l.strip().startswith("PROTECTED=")]
assert len(protected) == 1, f"expected exactly one PROTECTED definition, found {protected}"
for d in (r"\.claude/", r"\.codex/"):
assert d in protected[0], (
f"{d} is missing from PROTECTED ({protected[0].strip()!r}) — a directory carrying a copy "
"of the enforcement hooks can exempt itself from the gate it enforces")
# --- ersatztv#706, round 2: findings from the cold adversarial review ----------------------------
def test_the_high_water_MARK_is_captured_BEFORE_the_last_moment_re_read():
"""The High finding of round 2, pinned as the ORDERING property it actually is.
The mark was originally taken "as late as possible", just before the POST. That leaves everything
between the last-moment re-read and the mark as a blind gap: a human verdict landing there is
invisible to the re-read (already done) and excluded from the post-write check (its id is BELOW a
mark taken afterwards), so it is overwritten with no repair. The gap spanned the whole retarget
re-count up to 20 timeline round-trips not the single round-trip that was being claimed.
Taking the mark FIRST closes the read side: any row newer than the mark is caught either by the
re-read (abstain) or by the post-write check (repair). This is asserted structurally because the
defect is an order, not an output with the mark late the job still posts and still repairs in
every scenario a stub can pose; only the id arithmetic silently changes.
"""
src = _classify_step()["run"]
# Assert on BOTH the init and the FETCH: keying only on the init line would let a refactor that
# splits initialisation from the read slide the actual round-trip back past the re-read while this
# test stayed green (round-3 review nit).
mark = max(src.index("max_id_before=-1"), src.index('hist_before=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA'))
# The LAST-MOMENT re-read is the second bare `read_existing_verdict` call.
calls = [i for i in range(len(src)) if src.startswith("read_existing_verdict\n", i)]
assert len(calls) >= 2, f"expected two read_existing_verdict call sites, found {len(calls)}"
assert mark < calls[-1], (
"the high-water mark is captured AFTER the last-moment re-read, reopening the blind window "
"in which a human verdict is neither seen by the re-read nor repaired by the post-write check")
def test_a_previously_REPAIRED_head_is_never_re_exempted(tmp_path):
"""The Medium finding of round 2: the repair used to last exactly one event.
After a repair, the status is a machine-written `pending` indistinguishable, to the next run,
from an ordinary one. That run re-derived it, posted `success`, and took a fresh high-water mark
ABOVE the human row, so the post-write check stayed silent and the human's rejection went green
again one event later. The repair description is now a sentinel the classification recognises.
"""
posted, r = _run_classify(
tmp_path, _emitting("docs/a.md"),
status_mode="existing:pending", status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict")
assert posted is not None, f"the job posted nothing: {r.stderr[-2000:]}"
assert posted["state"] == "pending", (
"a head whose human verdict was previously overwritten was granted a FRESH exemption, burying "
f"the rejection again (desc={posted['description']!r})")
assert "raced a previous exemption" in r.stdout, (
f"rejected, but not via the repair-sentinel branch. Log:\n{r.stdout[-800:]}")
def test_positive_control_an_ORDINARY_machine_pending_is_still_re_derived(tmp_path):
"""Without this, the test above would pass against a job that had stopped exempting anything with
a pre-existing `pending`. An ordinary machine `pending` no sentinel must still re-derive to
`success` for a docs-only PR."""
posted, r = _run_classify(
tmp_path, _emitting("docs/a.md"),
status_mode="existing:pending", status_creator=None,
status_desc="Awaiting review verdict for a9e3e23")
assert posted is not None and posted["state"] == "success", (
f"an ordinary pending was not re-derived to an exemption. Log:\n{r.stdout[-800:]}")
def test_the_fence_gates_PENDING_TOO_not_only_the_exemption(tmp_path):
"""The fence deliberately refuses to write `pending` as well as `success` when the PR was
retargeted mid-run. Untested, a success-only-fence mutant would have passed the whole suite.
A stale `pending` is only a stall rather than a forged green, so gating it is not strictly
required but the successor run is guaranteed either way, so there is nothing to buy by writing a
value computed against a base the PR may no longer target.
"""
posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode="moves:0,1")
assert posted is None, (
"the fence let a stale `pending` through; it is gating only the exemption path "
f"(desc={posted['description']!r})" if posted else "")
assert "retargeted while this job was classifying" in r.stdout
def test_an_UNTRUSTED_count_withholds_the_exemption_BY_THAT_BRANCH(tmp_path):
"""Round 2 test-gap: the existing untrusted-count test asserted only "posted nothing", which a
crash also produces. Assert the discriminator and a clean exit."""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="unreadable")
assert posted is None
assert r.returncode == 0, f"the job died rather than declining cleanly: {r.stderr[-800:]}"
assert "Could not establish a trusted retarget count" in r.stdout, (
f"nothing posted, but not via the untrusted-count branch. Log:\n{r.stdout[-800:]}")
def test_the_repair_sentinel_is_a_FIXED_POINT_across_consecutive_runs(tmp_path):
"""Round-3 finding: durability is a fixed point, and only a CHAIN can assert a fixed point.
The first version of the sentinel refused the exemption but posted the GENERIC pending
description, erasing the marker it depends on. The next run then saw an ordinary machine
`pending`, re-derived it, and posted `success` burying the human rejection two events after the
repair instead of one. The single-hop test passed throughout, and the positive control asserting
that an ordinary machine `pending` DOES re-derive was itself the proof of the second hop.
So chain two runs: feed run N's posted description in as run N+1's existing status. The property
is that the sentinel branch's own output re-triggers the sentinel branch, forever.
"""
first, r1 = _run_classify(
tmp_path / "run1", _emitting("docs/a.md"),
status_mode="existing:pending", status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict")
assert first is not None and first["state"] == "pending", f"run 1: {r1.stdout[-600:]}"
second, r2 = _run_classify(
tmp_path / "run2", _emitting("docs/a.md"),
status_mode="existing:pending", status_creator=None,
status_desc=first["description"]) # <-- the chain
assert second is not None, f"run 2 posted nothing: {r2.stderr[-800:]}"
assert second["state"] == "pending", (
"the repair decayed: run 1's own posted description did not re-trigger the sentinel, so run 2 "
f"re-derived the exemption and buried the human verdict again (run1 desc={first['description']!r}, "
f"run2 state={second['state']}, run2 desc={second['description']!r})")
assert second["description"] == first["description"], (
"the sentinel is not a fixed point — run 2 wrote a different description than run 1, so run 3 "
f"would not recognise it ({first['description']!r} -> {second['description']!r})")
def test_a_sentinel_APPEARING_MID_RUN_stops_a_stale_run_overwriting_it(tmp_path):
"""Round-3 finding, and the only one in this series that failed toward SUCCESS.
Two runs overlap for the same sha the regime this branch measured live (probe PR #722: the older
run finished 20s after the newer one started). Run B catches a raced human BLOCKED and repairs to
the sentinel. Run A is still in flight: its FIRST read predates all of it, so it classified
`success`; its high-water mark was taken after the human row, so the post-write check stays silent;
and the retarget fence sees nothing. Until this guard, A posted its stale `success` straight over
the sentinel burying the human rejection with no repair and no log.
The re-read recomputes `ex_repair`; the bug was that nothing downstream consulted it.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"),
status_mode="sentinel-appears-on-read:2")
assert r.returncode == 0, r.stderr
assert posted is None, (
"a stale run posted its exemption over a repair sentinel written mid-run, burying the human "
f"verdict it records (posted={posted})")
assert "repair sentinel was written" in r.stdout, (
f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}")
def test_positive_control_a_sentinel_present_from_the_START_still_posts_pending(tmp_path):
"""Without this, the guard above could be an unconditional abstain whenever a sentinel exists.
A sentinel present at the FIRST read is the ordinary repaired-head case: it must still POST
`pending` carrying the sentinel forward (the fixed point), not abstain. That difference is exactly
what makes the mid-run guard's condition exact rather than conservative.
"""
posted, r = _run_classify(
tmp_path, _emitting("docs/a.md"),
status_mode="existing:pending", status_creator=None,
status_desc="Human verdict raced this exemption write — re-post the verdict")
assert posted is not None, f"the repaired head stopped posting entirely: {r.stdout[-800:]}"
assert posted["state"] == "pending"
assert posted["description"] == "Human verdict raced this exemption write — re-post the verdict"
def test_a_PENDING_path_run_also_refuses_to_clobber_a_mid_run_sentinel(tmp_path):
"""Round-4 finding: the guard's first form tested `state = success`, one branch too narrow.
A run can reach the POST on `pending` carrying the GENERIC description most realistically after a
transient enumeration failure and a success-only guard waves it through. It then overwrites the
sentinel with ordinary text, the next run sees no sentinel, re-derives, and posts `success`: the
same buried human rejection, two steps instead of one.
Hence the rule is "never replace a sentinel with a non-sentinel", compared on the DESCRIPTION. That
is strictly more general and exactly as precise, because the carry-forward branch guarantees a
first-read sentinel already sets `desc` to the sentinel.
"""
posted, r = _run_classify(
tmp_path,
"#!/usr/bin/env bash\n" + DOCS_ONLY + "exit 1\n", # enumeration fails -> pending, generic desc
status_mode="sentinel-appears-on-read:2")
assert r.returncode == 0, r.stderr
assert posted is None, (
"a pending-path run overwrote a repair sentinel written mid-run with the generic description; "
f"the next run would re-derive the exemption and bury the human verdict (posted={posted})")
assert "repair sentinel was written" in r.stdout, (
f"nothing posted, but not via the mid-run sentinel guard. Log:\n{r.stdout[-800:]}")
def test_a_SENTINEL_landing_above_the_mark_also_triggers_the_repair(tmp_path):
"""Round-5 finding: the post-write filter counted only HUMAN rows, and that is not sufficient.
With two overlapping runs A and B, the human BLOCKED can land BELOW A's high-water mark — so A
cannot see it while B masks it with an exemption `success` and only afterwards writes the
sentinel. A then finds nothing human above its mark, does not repair, and posts its own `success`
on top of the sentinel: a PERMANENT forged green over a human rejection, and the repair race
failing toward success rather than pending, which the decision record explicitly promises it does
not do.
A sentinel above the mark can only have been written by another run mid-flight a pre-existing one
would have been seen at the first read and forced the pending path so counting it cannot
false-fire, and it converges both runs on the fixed point.
"""
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), history_mode="sentinel-after-post")
assert r.returncode == 0, r.stderr
seq = _posted_sequence(tmp_path)
assert len(seq) == 2, (
"the run posted its exemption on top of a sentinel written by an overlapping run and did not "
f"repair, leaving a human rejection permanently green. Posts: {seq}\n{r.stdout[-800:]}")
assert seq[0]["state"] == "success"
assert seq[1]["state"] == "pending"
assert seq[1]["description"] == "Human verdict raced this exemption write — re-post the verdict", (
f"repaired, but not to the sentinel, so the fixed point is broken: {seq[1]}")