Round-five review confirmed the decision record is coherent with no third
survivor of the empty reading, and returned three LOW findings. All are in
prose I wrote in the last two commits.
- The comment defending `(IsAbstract && !IsSealed)` cited
AlternateScheduleSelectorTests as an in-repo static-fixture witness. That
class IS static, but it merely NESTS its [TestFixture]es and declares no test
of its own, so it would fail the sibling "declares no runnable test" assertion
rather than demonstrating the point. The rule is right and the witness was
wrong, which is the worse of the two failures because a wrong example is what
a reader checks the rule against. No witness is cited now, and why is stated.
- A mis-bound `because` in `rule:`: "assigning a null and calling SaveChanges
SUCCEEDS ... because only the HTTP request records normalize with `?? []`".
The `?? []` clause explains how a null could REACH the entity; what makes the
save succeed is the column being nullable. A right observation with a wrong
cause attached. Split into the two claims.
- `signals:` carried the literal token `paths:` twice, an artifact of appending
the #823 path list to the existing one. It degrades the field the discovery
surface parses.
Also recorded from that review, and NOT changed: `MonthsOfYear ?? AllDaysOfMonth()`
survives the selector fixture and no date can kill it -- 1..31 contains every
valid month, so it is an EQUIVALENT mutant there rather than a coverage gap.
Its non-equivalent twin at the DTO boundary is pinned per-dimension by
RecurrenceLimitsMapperNullTests. Left alone deliberately: chasing an equivalent
mutant with a contrived date would buy nothing and cost the fixture's
readability.
Local gate: ErsatzTV.Tests 2091 passed / 6 skipped, Core.Tests 697/1 -- 0
failures. Format clean, no BOM. decisions_validate OK.
Refs #823
Refs #824
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Round-four review. One HIGH, again in the decision record, and the previous
commit message asserted this exact class was cleared. It was not.
THE HIGH, and the reason it recurred.
A second sentence still described the rejected reading: "The guard form is
`?? []` into a local rather than this record's Optional(x).Flatten(), a STATED
deviation". The shipped guard is `?? AllDaysOfWeek()`. That sentence is the one
that dictates guard FORM to the next implementer, so it would have taught the
`[]` reading the same record spends a paragraph calling data corruption -- and
it had already propagated into docs/decisions/README.md, the mandated entry
point, which carries `rule:` verbatim.
The mechanism, not the sentence, is the defect. I swept with a regex keyed on
"null" plus a reading word; this sentence talks about guard FORM and contains
neither, so it could not match. That is grepping the retracted WORDING instead
of sweeping the CONCEPT, which is exactly what this corpus warns about -- and
three rounds in a row have now found a defect introduced by the previous
round's targeted string edit. So the fix is not another targeted edit: the
whole `rule:` field was split into its 39 sentences and read back one by one
against the code. Everything below came out of that pass rather than a grep.
Its secondary damage is worth recording because it is the shape of a rationale
that outlives its claim: the deviation was justified by ".ToList() allocates
for nothing", which is now BOTH irrelevant to the choice AND false about the
shipped code, since AllDaysOfMonth()/AllMonthsOfYear() are themselves
Enumerable.Range(...).ToList() on exactly the null path it describes.
- The opening sentence of `rule:` prescribed Optional(x).Flatten() as THE
read-site form. It is the sentence most likely to be read in isolation, and
it is wrong for six of the eight columns. It now separates the universal half
(a LOCAL, never assigned back) from the half that is not (the substituted
value), and names where each applies.
- `signals:` had never been touched, so roughly 60% of `rule:` was unreachable
by the discovery surface built for it -- no AlternateScheduleSelector, no
mapper, no "unrestricted", and its paths: list named none of the files this
work touched. It also advertised "Optional Flatten hoisted local" as the
form, which is precisely what the six do NOT use.
- The body prose was still entirely about SongMetadata while `rule:` had grown
a whole second subject. Added the two results that contradicted the prior
reasoning, in prose, where a reader meets them.
A REAL BUG in my own guard, not just prose:
fixture.IsAbstract.ShouldBeFalse(...)
A C# `static class` compiles to `abstract sealed`, and NUnit runs tests
declared in one -- this repo already has such a fixture
(AlternateScheduleSelectorTests is `public static class`). So the check I added
one commit ago to reject an un-runnable fixture would have falsely reddened a
perfectly good static one. Now rejects an abstract BASE (abstract and NOT
sealed), which is the case NUnit actually cannot instantiate.
A SURVIVING MUTANT the added controls did not kill:
AnyDate was 2024-03-06. With a day <= 12 a CROSS-WIRED substitution survives
the whole fixture -- `DaysOfMonth ?? AllMonthsOfYear()` hands back 1..12, which
still contains day 6, so every assertion passes while the guard substitutes the
wrong set. Moved to 2024-03-20, still a Wednesday in March, outside 1..12.
Measured both ways rather than reasoned: the cross-wire mutant passes the old
fixture and FAILS 2 of 11 on the new one.
Also re-witnessed, because I had modified that file and never re-proved it:
restoring `??=` in LuceneSearchIndex reddens the LUCENE fixture (1 red, 1
green) -- the exact mirror of the Elastic mutation. Extracting
ThrowOnWarningLogger did not cost #701 its proof, and the two fixtures are
independently load-bearing in both directions.
The record is now 73 prose lines, over the 60-line WARNING ceiling. Stated
rather than trimmed: it is 42nd of 42 records over that line, and the added
content is distinct findings (a second subject, a migration analysis and three
residuals), not redundancy against a sibling.
Local gate: ErsatzTV.Tests 2091 passed / 6 skipped (the three fixtures' MySQL
halves), Core.Tests 697/1 -- 0 failures. Format clean, no BOM. decisions
validate OK.
Refs #823
Refs #824
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Round-three review findings. One HIGH, and it was in the durable artifact
rather than the code.
THE HIGH: the record stated the shipped reading and its inverse.
The semantic reversal (empty -> unrestricted) rewrote the residual and the
write-half of `media.nullable-primitive-collection-mutation` but left the
ORIGINAL reasoning standing two sentences earlier: "A null reads as EMPTY, so
the item matches nothing"; "the REJECTED alternative was the All*() set";
"SKIPPING the row is the conservative repair". The shipped code is
`?? AllDaysOfWeek()` -- precisely the alternative that passage calls rejected.
The previous commit then inserted residual (1), which reasons entirely FROM
the All*() reading, two sentences after the sentence denying it.
That is worse than a stale comment. A session resolving this key -- or reading
the MemPalace mirror, which carries `rule:` verbatim -- would have been told to
write the guard the other way, i.e. talked into the `[]` reading that the same
record elsewhere argues is data corruption one save later. Replaced the whole
passage, then swept the record for every other mention of the empty reading
rather than trusting the one replacement: the only survivor is the new sentence
that records EMPTY as the rejected alternative, which is the direction that
stops it being re-adopted.
THE MEDIUM: one arrangement did not close the hole it claimed to.
The discriminating control added last commit nulls DaysOfWeek against a
restrictive MonthsOfYear. It excludes "any NULL matches unconditionally" only
for that dimension. The review supplied the surviving mutant --
`if (item.MonthsOfYear is null) { return item; }` ahead of the checks -- and
traced it green through all nine tests. Verified by EXECUTION, not by reading:
applied to the previous fixture it passes; applied now it FAILS 1 of 11. Each
of the three dimensions is now nulled against a restriction on a different
dimension.
The rest, all from the same round:
- The coverage guard's test detection listed attribute TYPES, and each list
falsely reddened whatever it omitted: TestAttribute alone missed [TestCase],
and the three-type replacement missed [Theory]. Now decided by NUnit's own
ITestBuilder/ISimpleTestBuilder interfaces, which cannot fall behind the
vocabulary. It also dropped BindingFlags.Static (GetMethods() defaults to
including it), which would have falsely reddened a static test method.
- The same guard accepted an ABSTRACT fixture -- NUnit never instantiates one.
The indexer population already filtered IsAbstract; the fixture side now
mirrors it.
- The record's `mechanics:` still described the old `[Test]`-only clause, in
the same file the change edited.
- An <inheritdoc> made the ProgramScheduleAlternate empty-case test inherit a
docstring written from the PlayoutTemplate test's viewpoint.
Two more mutations executed:
- `if (item.MonthsOfYear is null) return item;` -> 1 red, 10 green. This is the
mutant that survived the previous head; it no longer does.
- an abstract type named in the covered set -> coverage guard red.
Local gate: ErsatzTV.Tests 2091 passed / 6 skipped (the three fixtures' MySQL
halves, skipping visibly without ETV_TEST_MYSQL_CONNECTION), Core.Tests 697/1
-- 0 failures. Format clean, no BOM on the touched set. decisions_validate OK.
Refs #823
Refs #824
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Follow-up commit (the branch is pushed, so not an amend). Two more cold
reviews landed on the previous head; both reported 0 Blocker and 0 High, and
these are their Mediums and Lows. Each fix carries its own witnessed mutation.
1. The selector fixture could not tell the fix from a much broader one.
Every null test set a NULL and expected the item SELECTED, so all of them
pass equally under "NULL means unrestricted" and under "any NULL makes this
item match unconditionally" -- a refactor short-circuiting the whole date
check on any null kept them green. Added the discriminating control: a NULL
DaysOfWeek paired with MonthsOfYear = [1] against a MARCH date must be None.
Only the narrow reading passes.
2. A_Null_Item_Does_Not_Disturb_Selection_Of_A_Later_Item never measured its
own docstring. The nulled item was unrestricted and at Index 0, so it always
won and the second item was never evaluated -- the stated invariant ("a null
on the first item must not decide the second") went unmeasured while the
test passed. Split into two: one where the nulled item genuinely does not
match, which measures that the loop CONTINUES; and one that pins the
index-order win separately.
3. The empty-preservation control existed for one of two identical mappers.
The anti-mutant test for "empty or null becomes All*" covered only
Playouts.Mapper; Scheduling.Mapper is a byte-identical triple in another
file and had none, so a defensive edit to it alone would have rewritten a
deliberately-empty user selection to 1..31 with the suite green. That is the
one-helper-two-callers shape this repo has been bitten by. Added the
matching test.
4. The coverage guard's [Test] clause did not check what its message claimed.
GetMethods() without BindingFlags returns INHERITED methods, so a fixture
that merely subclasses another satisfied it while driving the wrong indexer
-- and Values.Distinct() cannot catch that, since the two Types differ. It
also matched TestAttribute alone, so a future fixture written as [TestCase]
would have falsely reddened, and it accepted an [Explicit]/[Ignore]d fixture
that never runs, which is the "wired is not running" failure the guard
exists to prevent. Now DeclaredOnly, the full test-method vocabulary, and
Explicit/Ignore rejected at both method and fixture level.
5. Three residuals recorded on media.nullable-primitive-collection-mutation
that the previous head asserted nothing about:
- the LOUDNESS change, worst for an all-three-NULL ProgramScheduleAlternate,
which now matches unconditionally and shadows the default schedule where
it previously threw. Unreachable today, and a choice over an unreachable
state rather than a measured requirement -- said plainly.
- the normalization is ONE-WAY and WHOLE-LIST: both PUT paths are full
replaces, so editing any row persists All*() over EVERY NULL row in that
playout, and afterwards "the operator selected all 31" and "this is a
legacy row" are indistinguishable. An ordinary user action closes that
door.
- the WRITE side disagrees with the READ side about what ABSENCE means: an
omitted daysOfWeek normalizes to [] ("never applies") while a NULL column
reads as unrestricted, so an API client gets HTTP 200 and a row that
silently never fires. Filed as #880 rather than folded in here, because a
client omitting a field on a write is a different question from what a
legacy NULL meant.
Two more mutations executed, both witnessed:
- DaysOfWeek guard disarmed in Scheduling.Mapper -> 1 red, 3 green.
- A fixture with no DECLARED test named in the covered set -> coverage red.
Local gate (MySQL lane armed): ErsatzTV.Tests 2097 passed / 0 skipped,
Core.Tests 695/1 -- 0 failures. Format clean, no BOM on the touched set with
the population count asserted. decisions_validate OK.
Refs #823
Refs #824
Refs #880
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
Both issues are #701 deferrals, and they land together because both rewrite
the same decision record.
#823 -- can a null reach one of the six collection-valued scalar columns?
MEASURED against a real TvContext on BOTH providers (SQLite, and MySQL 8.4
on an ephemeral server), because the reasoning available beforehand pointed
the wrong way. The two converters differ on their read side --
IntCollectionValueConverter maps null-or-blank to Array.Empty<int>(), while
EnumCollectionJsonValueConverter would dereference the result of
JsonConvert.DeserializeObject -- so the expectation was that a NULL row
behaves differently per column. NEITHER RUNS: EF does not invoke a value
converter for a NULL column at all. All six materialize as CLR null, the
int converter's null-to-empty branch is dead on this path, and unguarded
each .Contains in AlternateScheduleSelector throws NullReferenceException.
A NULL reads as UNRESTRICTED -- the All*() sets -- not as empty. This is
the whole semantic question and the first draft got it backwards. It is
decided by the one NULL reachable WITHOUT any code writing one: Sqlite's
20240113140741_Add_PlayoutTemplate_DaysOfMonth adds the column with
nullable:true and NO defaultValue, so a PlayoutTemplate row inserted before
it holds NULL and by construction had no day-of-month restriction. Reading
that as empty INVERTS the row's meaning and silently stops the template
applying at all. All*() preserves it, and is how "no restriction recorded"
is already represented (GetPlayoutAlternateSchedulesHandler,
PreviewBlockPlayoutHandler). What does NOT decide it, and was wrongly cited
in the first draft: the API request records normalize an omitted field with
`?? []`, but that is a client omitting a field on a WRITE and says nothing
about what a legacy database NULL meant.
Two read sites, not one. Guarding only the selector would have left the
entity->DTO mappers unguarded, and those feed the SPA: PlayoutScheduleEditors
spreads the collection (`[...template.daysOfMonth]` -> TypeError on a JSON
null) and playoutTemplateCalendar's appliesToDate -- an exact port of
GetScheduleForDate -- calls .includes on it. Both mappers now substitute the
SAME defaults, so the preview agrees with what is actually scheduled. Neither
guard is assigned back onto the entity, which is the
media.nullable-primitive-collection-mutation mechanism.
Reachability, stated precisely rather than overclaimed. All six are
nullable:true on both providers, but a nullable column does not produce a
NULL row: five of the six were present at CreateTable, so a NULL there still
needs code to write one, and on MySQL there is NO code-path-free NULL for any
of the six. The write path ACCEPTS a null (SaveChanges succeeds, stores SQL
NULL) but no caller supplies one today -- every production construction of the
two commands goes through the request records. That is a property of the code,
not a live caller; claiming otherwise would be the banned "it's AsNoTracking
today" argument pointed the other way.
#824 -- ElasticSearchIndex.UpdateSong had no regression test
Issue option 1 (a non-network transport) shipped, and needed no new package:
Elastic.Transport.InMemoryRequestInvoker is public in the pinned version and
ElasticsearchClientSettings(NodePool, IRequestInvoker) accepts it, injected
into the private _client the way #701 injects the Lucene IndexWriter.
UpdateItems never runs `_client ??= CreateClient()`, so the injected instance
is the one used.
Two traps there are load-bearing, both measured: the canned response must
carry an `X-Elastic-Product: Elasticsearch` header or the client's product
check throws UnsupportedProductException INTO UpdateSong's catch, and an empty
body fails to deserialize the same way. Either turns the fixture into a green
measurement of the error path -- which is how it first failed here, caught by
the ThrowOnWarningLogger. The document id is asserted as the LAST PATH SEGMENT,
not by substring: the index name carries digits, so ShouldContain would stop
discriminating for a song whose id collided with one.
Six mutations executed, each disarming ITS OWN clause alone:
- `??=` restored in ElasticSearchIndex only -> the Elastic fixture reddens on
"metadata.Artists should be null but was []" while the LUCENE fixture stays
GREEN. The #824 hole demonstrated, not described.
- DaysOfWeek guard disarmed in the selector -> 4 red, 3 green (DaysOfMonth and
MonthsOfYear unaffected). Each clause is independently load-bearing.
- DaysOfMonth guard disarmed in Playouts.Mapper -> 1 red, 2 green.
- Elastic dropped from the covered set / mapped to the SAME fixture as Lucene /
mapped to a class with no [Test] -> SearchIndexMutationCoverageTests reddens
on each.
That coverage guard is the boundary fix the issue asked for: the covered set is
compared against an ISearchIndex population DERIVED FROM THE ASSEMBLY. Its claim
stops where the check does -- no static check can establish that a named fixture
actually DRIVES its indexer, so it forces a human to look rather than proving
coverage. ThrowOnWarningLogger moved to ErsatzTV.Tests/Support so both fixtures
share it; the Lucene fixture's assertions are otherwise untouched, since it is a
witnessed proof artifact.
No production change in ElasticSearchIndex.cs -- #824 is coverage only.
Docs: testing.md gains a "Provider-parity fixtures" section naming all THREE
opt-in-MySQL fixtures and recording that CI runs none of them (#627);
docs/README.md gains the matching task signal; guard-inventory.md's
hand-written C# guard list goes from five files to six. Scheduling/Mapper.cs
loses the UTF-8 BOM it inherited, per #311 fix-as-you-touch.
Local gate (with the MySQL lane armed): ErsatzTV.Tests 2096 passed / 0 skipped,
Core.Tests 693/1, Infrastructure.Tests 114, Architecture.Tests 7, Scanner.Tests
1504 -- 0 failures in each. scripts/tests 1228 passed / 2 skipped. dotnet format
whitespace --verify-no-changes clean; BOM check over the touched set with the
population COUNT asserted, because a bare zsh loop silently checks one
concatenated filename. decisions_validate OK.
Fixes#823Fixes#824
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
The guard asserted EXACT completeness over a population enumerated by a directory
walk, so an untracked .ts/.tsx under web/src/ entered it and failed as unregistered
on that developer's checkout while CI — which only ever checks out tracked files —
stayed green.
The glob still supplies file CONTENT; the POPULATION is now the git index, read by
web/vite-plugins/trackedSourceFiles.ts in Vite's own Node context and handed to the
app project as a virtual module. That reaches the index without admitting
@types/node to tsconfig.app.json, the obstacle that deferred this in #818.
Three mechanisms carry the proof, each added because the previous was measured
insufficient: a closed-form restatement of the shared scope predicate (sharing no
helper at any depth with what it checks); a second independent `ls-files --others`
query cross-checking the population; and real-git tests that execute the derivation
against a temp repository.
Six residuals are stated with their MEASURED fail-directions, and
testing.guard-derives-population-from-source gains a bounded exception plus the
closed-form criterion.
fixes#819
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
A force-push H1 -> H2 -> H1 spanning `pr-changed-files.sh`'s paging leaves its final
`.head.sha` comparison equal while the middle pages came from H2, so a mixed file list
could produce a docs-only exemption `success` no single head ever justified. The base
alias had been fenced since #706 by a monotonic `change_target_branch` count; the head
axis had nothing, and three contracts asserted otherwise.
`count_retargets` becomes `count_pr_mutations`: one timeline walk, two tallies, one shared
trust flag, a separate fence arm and diagnostic per axis. The advisory hook re-reads
`.head.sha` at the same hoist and off the same response as the base re-read. All three
overclaiming contracts are corrected, plus four paraphrases the first sweep missed.
Measured, not assumed: Gitea 1.27.1 still serves no `files` on `compare/{base}...{head}`;
every push is a `pull_push` event and its count cannot alias; PR #761 really went
`8798a1d -> 830a407 -> 8798a1d`; and Gitea creates the push comment BEFORE emitting the
synchronize notification, so a run cannot abstain on its own trigger.
Two pre-existing fail-opens in the shared walk were found by review and fixed: an empty
ARRAY first page was trusted on any page while the `null` arm required `page > 1`, and no
row was validated before `.type` was selected on.
NOT closed, and documented rather than overclaimed: the walk's `null` terminator is
defeatable, because Gitea pages before it filters (#870). The fence closes the ABA on a
timeline with no truncating block, not the ABA outright.
fixes#803fixes#664
Refs #870
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Closes#786 and #789, bundled because working either alone would build the artifact the other removes.
Every job in all six tracked workflows declares `env.CI_JOB_ROLE` (guard/report-only/none); the
`docker-build.yml` jobs also declare `env.CI_EXECUTION_CLASS` (toolchain/bare-runner). Both guard
populations derive from those markers; the `TOOLCHAIN_JOBS`/`BARE_RUNNER_JOBS` literals are deleted.
A missing or unrecognised marker is a hard failure in both checkers.
#789's literal had a real justification — set equality between two DERIVED sets is blind to a member
leaving both at once — so the marker is the anchor that replaces it, and the cost (proximity to the
`container:` block) is paid by a THIRD derivation from each job's own steps, which is also the only
check that sees the failure #789 filed: a .NET step moved into a bare-runner job, where no set
changes. The residual is disclosed: drop the block, flip the marker AND hide the tool behind a
script and all three go blind, bounded by the failure mode being a loud missing-binary crash.
#786's guard jobs join a machine-checked population: a new `test_workflow_job_guards.py` asserts set
equality both ways against a new "Workflow-job guards" table, and the four jobs with no dropped-step
guard each carry a recorded decision.
Two issue claims were refuted by measurement: #789's "editing docker-build.yml re-points the pin"
(the pathspec is `docker/ci` only) and #786's job count (17, not 15).
Four cold adversarial review rounds across two model families; rounds 1-3 BLOCKED, all findings
fixed and each fix demonstrated by reproducing the reviewer's own test. The recurring defect class
was prose drifting from code, including a mechanism claim in the decision record that execution
refuted. All five mutation proofs redden when their shipped detector is disarmed.
New decision record: `testing.workflow-declares-its-own-job-metadata`.
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Records `testing.verification-code-needs-its-own-proof`: the proof obligation follows the
VERDICT rather than the file, so it binds harnesses, wrappers, timeouts and checkers — not
only the files the guard population derives.
The issue asked for a stated position on whether non-guard checker scripts get mutation
proofs. The position as first written claimed `scripts/mcp_smoke.py` "cannot participate"
because driving it needs the gitignored `.mcp.json` and a cold-built language server. Cold
review refuted that by execution: it takes its config path and server name as positional
arguments. The record had failed its own headline rule on the one claim its decision rested
on, so this ships the proof instead of the exemption.
- `scripts/tests/test_mcp_smoke.py` — a hermetic stub JSON-RPC responder and six cases
pinning the defects the checker has already had, with the positive control as a fixture
the refusal tests depend on, so a node-id or `-k` selection cannot skip it.
- A declared clause in `mutation_manifest.py` targeting the unguessable request id, using
the `guard=test / target=script` shape that already exists for `mutation_harness_lib.py`.
Witnessed red: `id_init = 1` makes the pre-answer accepted at `initialize` (rc 9 -> 10),
and only that test moves.
`mcp_smoke.py` still gets no inventory row — one is rejected as a phantom (measured). The
row goes to the test file, which joins the derived population automatically.
Five cold-review rounds, four BLOCKED. Round 2 caught a `ruff format` red that would have
failed `script-tests`. Rounds 3-5 found only hand-maintained counts and uniqueness claims in
prose, three of them created by the previous round's fix; that class was deleted rather than
corrected again, per this record's own stop-and-subtract rule.
Docs updated in the same PR: `docs/README.md` task-signal map and `docs/guard-inventory.md`
(row, summary counts, scope-limit item 6).
fixes#796
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
CI's `Script lint and tests` job went red. Cause: I never ran ruff locally,
which this repo's Python convention requires after any .py change.
- E741 twice: `l` as a comprehension variable in the sort-order guard.
- `ruff format --check`: the file was correctly formatted on `main`; my edits
broke it. One of them left a docstring line at column 0, which `ruff format`
then "corrected" by over-indenting the rest of the paragraph — repaired at
the source rather than accepting that rewrite.
Verified the way CI does: local ruff is the pinned 0.12.11, and both
`ruff check` and `ruff format --check` run under bash over the full tracked
population (`git ls-files -z '*.py' '*.pyi' '*.ipynb'`, 46 files) are clean.
The population is counted, not assumed — an empty glob would pass vacuously,
which is the failure `scripts/tests` guards against elsewhere.
`scripts/tests` 1097 passed, 2 skipped after the reformat.
refs #763
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Population derived from `git ls-files`, not the issue's 9-key list (~21 claim sites).
Re-confirmed unchanged on 1.27.1: the distinct `skipped` commit-status state; `compare` serving
no `files`; no agent-side cancel route (REST route + swagger only); `branches: [main]` suppressing
the run off a non-main base.
Newly measured on four throwaway scratch bases, `main`'s rule never PATCHed: an absent required
context blocks an ORDINARY merge without needing `block_admin_merge_override` (that field governs
the FORCE path only), and `enable_bypass_allowlist` with an empty list is NOT a substitute for it.
Trap recorded: the PR API reports `mergeable: true` while such a merge is refused.
Left explicitly dated with reasons: push-supersession auto-cancel, `pull_request_target` overlap,
`--depth=1` no-merge-base, and the scope-enum/`reqRepoWriter`/403 items. Not a corpus sweep, and
`ci.actions-credential-scoping` now says so. `review-verdict.yml` untouched — #763 holds that file.
Five adversarial review rounds (21/12/9/6/2). Caveat: all same-model-family; Codex was rate-limited.
fixes#747
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
A sixth cold review found everything in round 8 clean except one line, and it
is the rule this branch keeps rediscovering: the test pinned the new
`raced_why` only by asserting the ABSENCE of the borrowed wording. Measured —
replacing the string with `zzz` left the suite green while an operator would
get `::error::… — zzz.` beside a sticky sentinel. The sibling test 330 lines
away states the rule and follows it; this one did not.
Now asserted positively, with the em-dash and full stop discriminating the
`::error::` reason from the `::warning::` text that continues ", which cannot
be true". The `zzz` mutation reddens it.
Three nits from the same review, all verified by execution rather than reading:
- the earlier fixture's row was excluded by the strict `> $since` because the
mark became its OWN id, not because it sat below the mark.
- the predecessor comment said `main` "warned only on `null`". True of the two
EMPTY shapes being contrasted; an empty body and a non-array object warned
as well. Scoped.
- `docs/ci-cd.md` and the record described the `::error::` as a two-way split
(found vs unverifiable). Round 8's whole argument is that a complete read
returning an IMPOSSIBLE answer is a third case, not a variety of the second
— which is the operator-facing point, since it decides whether to go looking
for an API failure that never happened. Both now say three.
The review re-verified, by comment-stripped diff, that round 8 changed no
executable line beyond the `raced_why` string and the if/elif restructure, and
independently reproduced both inertness measurements and the `origin/main`
predecessor behaviour.
Verification: `scripts/tests` 1097 passed, 2 skipped; decisions_validate and
build_decisions_catalog --check exit 0.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fifth cold review confirmed the gate's behaviour is correct and proof-backed,
and blocked on three non-behavioural items. All three fixed; none touches the
shipped logic.
MEDIUM — the round-7 fixture narrated a raced human verdict it did not
construct. `null-page1-after-post` appended the row unconditionally, so it also
joined the PRE-write read and lifted the high-water mark above itself; removing
it changed nothing. The reviewer's suggested fix was to gate the append on the
post-write read. Measured after gating: still inert, because page 1 answers
`null` before any row reaches the wire.
So the row is gone rather than gated, and the prose now describes what the
fixture actually poses: a response asserting an empty history for a sha this job
wrote to must not be accepted as proof that nothing raced. Whether a verdict
really raced is not modelled and does not need to be — the response is not
evidence either way. A row the test cannot observe is decoration that reads as
coverage, which is the same class this branch has now been blocked on five
times.
LOW — the comment claimed the predecessor "at least produced a `::warning::`".
Half false, measured against `origin/main`: its `jq -e 'type == "array"'` gate
ACCEPTED `[]` silently and warned only on `null`. What is actually new is that
the paged walk reports such a read as a SUCCESS.
LOW — when the empty clause fired it set `ph_ok=no`, so the log said "could not
be read completely" beside a walk that completed on a validated terminator. The
answer was impossible, not unreadable, and an operator holding a sticky sentinel
needs to know which. It now carries its own `raced_why`, asserted by the test.
Both clauses mutation-proved: disarming the empty check, and reverting to the
borrowed wording, each redden the named test.
Verification: `scripts/tests` 1097 passed, 2 skipped; decisions_validate and
build_decisions_catalog --check exit 0.
refs #763
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fourth cold review returned NOT-MERGEABLE on two Mediums. Both fixed, plus
its three Lows.
MEDIUM, and a defect this branch introduced. Tolerating a `null`/`[]` page 1 as
"complete, zero rows" is correct for the PRE-write caller — a head nothing has
posted to genuinely has no statuses — and impossible for the POST-write one,
which has just written a row to that sha. The body is well-formed, so nothing
retries it, and the walk reports success: `raced=0` concluded from a list that
cannot be real, on the one path whose failure direction is toward SUCCESS.
Worse than the code it replaced, which at least emitted a `::warning::` — a
logged fail-open had become an unlogged one. Reviewer measured both directions.
The post-write caller now rejects an empty result itself; the walk stays
caller-agnostic because the pre-write caller genuinely needs the empty answer.
This is NOT the withdrawn currency witness: that asked whether ANY row sat above
the mark, which an unrelated newer row satisfied while the rejection stayed
hidden, and it fired on schema-valid staleness. This asks only whether the list
is EMPTY — a state no unrelated row can produce and no ordering can disguise.
It carries neither defect. Proved by fixture; disarming it reddens the named
test, and the previously-uncovered `null`-at-page-1 clause is now covered too.
MEDIUM — the fourth overclaim of the same class, in the decision record body:
"Uncertainty must fail closed at both ends … Both repair now." The page-2 probe
was DELETED, not converted; it repairs nothing. It also contradicted the
record's own `rule:` ("the two directions are NOT symmetric") and the bullet
directly beneath it. Round 5 retracted this wording in `docs/ci-cd.md` only —
the sweep was by subject, not by the retracted words.
Also fixed:
- the record presented "an empty FIRST page is legitimate" as a property of
the walk; it is a property of the pre-write caller.
- `docs/ci-cd.md` called the numeric-only id comparisons a fix for mark
inflation; they are a TYPE guard, closing the string half. A corrupt but
genuinely numeric id still inflates the mark — not attacker-controllable,
since ids are server-assigned, and now stated rather than implied.
- `test_a_partial_mark_is_SAFE...`'s self-guard promised to detect that the
fallback ran; it keys on a warning emitted by a different condition, so
deleting the fallback left it green. Its sibling is what reddens; the
message now says what it actually pins.
- the order-faithful fixture appended the job's own POST after the reversal,
serving the NEWEST row on the OLDEST page — the opposite of DESC, in the one
fixture that exists to be ordering-faithful.
- "twice per walk" for the wasted sleep; it is once per walk, twice per run.
- a dead counter read in the DESC mode.
Rebased onto b16ec15d6 (the other session's #781/#799 docs work; no file
overlap, no conflicts).
Verification: `scripts/tests` 1097 passed, 2 skipped; fifteen executed mutations
across rounds 2-7; decisions_validate and build_decisions_catalog --check exit 0.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fourth cold review (Opus, isolated worktree, tests/double/docs focus)
reported no correctness bugs in shipped behaviour but two coverage defects on
exactly the two things this change advertises. Both are closed.
The partial-mark fallback's safety is a claim ABOUT THE ORDERING — page 1 holds
the newest rows, so a walk that fails later still saw the true maximum. The
fixture pinning it served ASCENDING ids, i.e. the arrangement the design calls
unsafe, and passed anyway because the raced row's id sat above even the partial
mark. It could not distinguish safe from unsafe.
The stub now HONOURS the sort parameter: order-faithful modes serve DESC by
default and ASC when the request asks. The new fixture holds a PRE-EXISTING
base-mismatched verdict at id 7055 among 60 rows. Under DESC the salvaged mark
is 7059 and that row is below it — the exemption correctly stands. Under ASC
the mark would be 7049 and that untouched row tests as NEWER, a sticky repair
on a head nothing raced. So re-adding `sort=highestindex` now reddens by
BEHAVIOUR, not only by the structural assertion added in round 5. Measured:
re-adding it reds both tests.
Most modes stay ordering-blind on purpose and now say so: they test walk
COMPLETENESS, which is order-independent, and insertion order is what lets a
fixture place a row beyond page 1.
Also fixed:
- `null` is accepted as an empty page. An array-only gate is the exact shape
of #751 — `count_retargets` had one, the timeline really did return `null`
past the end, and the fence withheld EVERY exemption from the day it
shipped. The same narrowing here is worse, because this walk's failure is
the STICKY sentinel: every exempt PR would need a hand-posted verdict, per
head. Tolerating `null` cannot misread `[]`. Proved by fixture.
- the fail-closed comment said "past the 1000-row page cap"; the bound is 950,
as the walk's own comment and both docs already said.
- the docs claimed "only a read returning no rows at all abandons the mark".
False: a VALIDATED empty history yields a mark of 0 and is not abandoned —
that is the normal first run. What abandons it is a read that both FAILED
and returned nothing. Corrected in ci-cd.md and the record `rule:`.
- a comment pointed at the page-2 probe "a few lines further down"; it was
deleted, so the deixis pointed at nothing.
- the stub claimed its logical-read counter "is only reached on a SUCCESSFUL
page-1 serve" — measured false; it counts page-1 requests, retries included.
- five `(round N)` markers removed. A round number is session chronology and
does not parse for a reader who never saw it (`docs.no-session-narrative`);
an issue number does. The four that remain predate this change.
Verification: `scripts/tests` 1096 passed, 2 skipped. Thirteen executed
mutations across rounds 2-6. The reviewer independently re-ran the earlier
matrix and confirmed it, with one correction carried here: two of those
mutations redden MORE than their named test, so "each reddening exactly its
named test" was wrong — they redden at least it.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A third cold review (Opus, isolated worktree) returned NOT MERGEABLE with one
High and three Medium. All are addressed.
HIGH — the stated motivation was wrong, and self-contradictory once round 4
landed. Under the server default (`created_unix DESC`) page 1 holds the NEWEST
rows and ids are monotonic with `created_at`, so page 1 already carried the
true maximum id AND every row newer than the mark — the only rows the
post-write check selects on. A single-page read therefore missed a raced
verdict only if more than 50 rows were created INSIDE the write window, not
merely on "a head with more than 50 rows", which the issue, the comments and
the docs all asserted. Reviewer executed an order-faithful DESC stub: a
page-1-only reader repairs identically to the full walk.
What actually removed #761's stall is retiring #751's page-2 probe, not the
paging. The walk still earns its place, for a reason now stated instead of the
false one: it stops the gate's one fail-toward-SUCCESS path depending on an
undocumented ordering the server honours only coarsely (page 1 came back
`114,112,113,111,110`). That measurement was deleted in commit 1 and is
restored, since round 4's safety argument rests on exactly it.
MEDIUM/real defect — the string-id TWIN, live on `main` and one expression
away from the fix already made: `select((.id? // 0) > $since)`. jq orders
strings above every number, so a PRE-EXISTING row with `"id": "3"` reads as
newer than any mark, is counted as having raced the write, and gets the sticky
sentinel plus a false "was overwritten" on EVERY later run — a permanent
per-sha stall no re-trigger clears. Now numeric-only, with a test.
Also fixed: a non-empty history carrying no numeric id was collapsed to a mark
of 0 (making every pre-existing row look newer); it is now reported unusable
and the check is skipped. `sleep` no longer fires after the final attempt.
Three unpinned clauses now have tests, each proved by an executed mutation:
- the page cap is a refusal, not a terminator (1050-row fixture)
- the `::error::` found-vs-unverifiable distinction (forcing `raced_why=human`
reddened nothing before)
- the walk requests no sort order — a structural guard on round 4's
withdrawal, which nothing mechanical protected. It reads request LINES, not
comments, since the withdrawal note names the parameter to explain it.
Honest scoping, not new code: the test double is ordering-blind, so the paging
tests prove WALK COMPLETENESS, not that a real raced verdict would otherwise be
missed — under DESC it would not be. The stub comment and the docstrings now
say so rather than implying the stronger claim.
Docs: `ci-cd.md` and the record's `rule:` carry the corrected reachability, the
DESC dependency of the partial-mark fallback, and both rejected alternatives
stated as rejected alternatives rather than as draft chronology
(`docs.no-session-narrative`).
Verification: `scripts/tests` 1094 passed, 2 skipped; eleven executed
mutations across rounds 2-5, each reddening exactly its named test;
decisions_validate and build_decisions_catalog --check exit 0.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 3 added `sort=highestindex` to close a mid-walk-insert gap: under the
server default (`created_unix DESC`) a row inserted while the walk is running
lands at position 0, on a page already read, so the walk never sees it.
That fix and the round-2 partial-mark fallback are incompatible. ASC puts the
OLDEST rows on page 1, so an incomplete walk takes its high-water mark over
the oldest rows — leaving every pre-existing row above the mark and read as
"raced". That is a spurious STICKY repair on a head nothing raced, which is
precisely the #761 failure this whole issue exists to remove. Under the
default DESC the newest row is on page 1 by construction and ids are monotonic
with `created_at` (measured), so a partial mark is at or very near the true
maximum and "lower is safe" actually holds.
Two defects from one mechanism again, so the mechanism goes rather than
getting patched: the sort is withdrawn and the mid-walk-insert residual is
ACCEPTED and documented. It is bounded — a row arriving after this job's POST
is not one this job overwrote, and being newest it wins on the combined
endpoint branch protection reads.
Both the code comment and the docs record the withdrawal and the reason, so
the next reader does not re-adopt it.
Verification: `scripts/tests` 1090 passed, 2 skipped; the partial-mark mutation
still reddens `test_a_PRE_WRITE_paging_failure_still_yields_a_usable_high_water_mark`;
decisions_validate and build_decisions_catalog --check both exit 0.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two independent cold reviews (Codex GPT-5.6 cross-family, and an isolated
Opus agent) converged on the same blocker, which is fixed here along with
everything else they found.
BLOCKER — the mark walk turned a fail-closed case into a fail-open. The
high-water mark gates the post-write race check entirely: `max_id_before=-1`
skips it. Before paging, only a failure of the single page-1 request could
reach that. Requiring a COMPLETE walk newly routed a page-2 hiccup, an
over-cap history, or one malformed id on a later page into the same hole, so
a human rejection racing the write was left green where `main` repaired.
A partial list now still yields a mark: it can only be LOWER than the true
maximum, which makes the check more eager, never blinder. Only a read
returning no rows at all abandons it — the pre-existing #849 gap, unchanged
and now asserted by a test so it stays visible.
WITHDRAWN — the "currency witness". It produced two defects from one
mechanism, which is the signal to remove rather than patch twice: counting
ANY row above the mark does not witness this job's write, so a stale-but-valid
snapshot carrying an unrelated newer row passed while hiding a rejection; and
a schema-valid stale read is not retried, so one such response turned a
transient anomaly into a permanent sentinel. The hazard has no mechanism here
either — Gitea is a single instance with no read replicas. Removing it
restores the pre-change exposure on that path, a non-regression.
Also fixed, each a fail-open with a fixture and an executed mutation:
- `.creator` is type-tested before indexing. `.creator.login` on a non-object
exits jq 5 and `set -e` took the step down after the green was posted and
before the repair. Reproduced by both reviewers.
- the mark is the max over NUMERIC ids only. jq orders strings above every
number, so one `"id": "99999"` passed the numeric gate and inflated the
mark until nothing looked newer.
- an unusable `raced` count now repairs instead of "not acting on it".
- `sort=highestindex` (ASC, measured) so a row inserted mid-walk appends at
the end rather than at position 0 on a page already read. An unknown sort
value silently falls back to DESC, so this is insurance, not load-bearing,
and the comment says so.
- `ph_ok`/`ph_rows` renamed off `read_existing_verdict`'s `st_ok`. No live
bug, but a name collision in a 1400-line step.
Tests the reviews showed were missing, each proved by an executed mutation:
- verdict beyond a SHORT page (a deliberately unfaithful truncated response
— against a faithful double a short page is always the last, so the rule
"terminate only on an EMPTY page" was unobservable)
- pre-write paging failure still yields a usable mark
- pre-write read returning nothing abandons the mark and says so
- a TRANSIENT page failure is retried (the retry was unproven code: every
other error mode fails on every attempt, so disarming it reddened nothing)
- a string id cannot inflate the mark
- a malformed `creator` row does not kill the job
Stub corrections, both the same class as the earlier `[]`-vs-`null` gap: it
served one flat list (so paging was unobservable) and computed its own-post id
with `max()` over mixed str/int, which raised TypeError and made the string-id
test pass because the DOUBLE crashed rather than because the mark was right.
Mutation matrix, all executed, each reddening exactly its named test: retry
disarmed; numeric-max reverted; partial-mark fallback removed; short-page
terminates; page-1-only walk; post-write fail-closed flipped open; jq
type-guard reverted. The unusable-count arm is unreachable by any fixture and
is annotated as such rather than claimed as proved.
Verification: `scripts/tests` 1090 passed, 2 skipped; decisions_validate and
build_decisions_catalog --check both exit 0; terminator, clamp, sort order and
id monotonicity all re-measured live on Gitea 1.27.1.
refs #763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`review-verdict.yml` read the per-POST status history twice with a single
`?limit=100` request. `limit` clamps to the server-wide `MAX_RESPONSE_ITEMS`
(measured 50), so on a head carrying more rows than the clamp both reads saw a
partial list. The high-water mark was only page 1's maximum, and — the direction
that matters — a raced human verdict beyond page 1 was invisible to the
post-write race check, leaving a forged green over a rejection.
Both reads now walk to a validated empty page (`[]` on this endpoint, measured
2026-08-28 against PR #761's 114-row head: pages 1-2 return 50, page 3 returns
14, page 4 is `[]`), never terminating on a short page, under a 20-page cap and
retrying each page once. Correctness does not depend on the cap value.
This retires #751's page-2 "assume raced" probe, which repaired every head that
outgrew one page. It fired on Renovate PR #761: an `::error::` claimed a human
verdict had been overwritten on a head carrying none, and the sticky sentinel
then refused re-exemption on every later run.
Two properties replace it. Uncertainty now fails closed at both ends — the
unreadable-history branch warned and left the exemption green while the page-2
probe repaired on the same uncertainty, one check disagreeing with itself; this
is affordable only because paging removed the common trigger. And the post-write
read must witness the job's own write: reaching a validated empty page proves the
walk finished, not that it saw a current list, so at least one row above the
pre-write mark must exist because the job just posted one.
The `::error::` now distinguishes a verdict actually found from an unverifiable
read. The sentinel description stays generic — the classification recognises it
as a fixed point, so its wording is load-bearing.
The stub gained faithful paging (50-row slices, `[]` past the end, one snapshot
per logical read so a counter mode cannot describe two different histories across
pages) and, separately, modelling of the job's own POST appearing in the history
— which it had never done, so in its world every ordinary run looked like a head
nothing had been posted to. `own-write-invisible` withholds exactly that detail
as the negative control for the currency witness.
Mutation-proved by execution, one clause at a time:
- walk reads page 1 only -> RUNNING_PAST_PAGE_1_is_PAGED_and_the_exemption_
STANDS, raced_verdict_on_PAGE_2_is_detected_and_repaired and both UNREADABLE
history tests go red
- currency-witness zero branch deleted -> CANNOT_SEE_OUR_OWN_WRITE red
- fail-closed flipped to fail-open -> both UNREADABLE history tests red
Verification: `scripts/tests` 1085 passed, 2 skipped; decisions_validate and
build_decisions_catalog --check both exit 0.
fixes#763
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
§5.3's verdicts rested on a single surface, which manufactured four false zeros: codex is driven
through `codex exec` inside Bash, security-guidance and ralph-loop expose no tool at all and run as
hooks (1,086 executions each), and feature-dev is used through its agents. The audit also compared
current enablement against historical usage — six of the eight plugins it called "genuinely unused"
were disabled for 16 of the 30 corpus days.
The retirement half of #781 is answered *no* on evidence: the zeros split six ways and only one is
grounds for removal. Eight plugins are kept by operator decision.
#799's observation was correct and its cause is now established. serena was `false` in settings.json
until 2026-08-14T12:31Z, when a concurrent session enabled it; its tools appear in no transcript
before 12:42:54Z. #799's session started at 12:01Z and never reloaded, so its probe correctly found
nothing while the settings file already said `true`. serena is adopted and documented as the third
code-intelligence surface.
Four review rounds, two independent cold reviewers (one cross-family); rounds 1-3 BLOCKED.
fixes#781fixes#799
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Closes the push route into ci-image.yml (#744) and ships the persist-credentials guard that was waiting on it (#835).
ci-image.yml's push trigger had no branches: filter and was path-scoped to docker/ci/** AND to the workflow file itself. Gitea resolves a push workflow's definition from the pushed ref, so any branch push touching those paths ran that branch's own YAML on a docker-capable runner holding the credential that writes ersatztv:prod and the ersatztv-ci:<sha> five container: jobs execute.
Be precise about what the filter buys: it is loaded from the pushed ref like the rest of the file, so a branch that deletes it re-enables the route. This closes the DRIVE-BY case - publication as a side effect of an ordinary push - and is not a boundary against a writer who intends to run their own YAML. The wider class is #853.
The self-reference left both paths: and ci-image-pin's expected in the same change - a decided tradeoff with both prices stated, not a necessity. Branch publishing moves to workflow_dispatch, probed live: run 2340 on this branch published ersatztv-ci:43b1e45 and left :latest unchanged.
With both mechanical blockers gone, ci-image.yml's checkout takes persist-credentials: false (16 of 16) and scripts/tests/test_workflow_persist_credentials.py holds the convention with NO exemption list - git-index population, declared clause mutation re-run every suite, guard-inventory rows.
fixes#744fixes#835
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
`review-verdict.yml` decided whether an existing `review-verdict/h10` was worth INHERITING by
testing `.creator.login != null` — satisfied by any account's credential, including the `renovate`
bot's `RENOVATE_TOKEN`, a `write:repository` PAT that cannot be scoped down the way #697 scoped the
registry credential. The test is now membership in `H10_REVIEWERS="timothy"`, a literal in the
base-resolved definition.
The design that survived 11 cold review rounds:
* `read_existing_verdict` carries TWO flags. `ex_human` (attributable AND allow-listed) gates
INHERITANCE; `ex_attributable` gates the last-moment re-read, which asks the opposite question and
must stay broad. Narrowing both — the first draft — makes the job post its exemption over a
mid-run rejection, and the post-write repair does not cover that.
* The two calls no longer compute an identical predicate, so "changed" is made explicit: the
state/creator/description triple from the first read is snapshotted and compared.
* The allow-list governs an inherited `success` ONLY. An existing `failure` inherits on
attributability alone, because inheriting a rejection can only withhold an exemption while
re-deriving one can turn it green on an exempt PR. A symmetric rule was a measured fail-open.
* The post-write raced check stays broad — not because narrowing would let a rejection go green
(a real reviewer is on the list by construction), but for the misconfiguration case.
Two mechanisms were WITHDRAWN rather than patched a third time, and both withdrawals are recorded
in `ci.exemption-provenance` so they are not re-attempted: a `::warning::` annotation that produced
three defects in three rounds, and a post-write fix whose generic `pending` would have been
re-derived anyway and which had no retry trigger.
Verified: the inheritance predicate driven against the LIVE Gitea API on a probe-named context,
both allow-list directions; every clause mutation-proven against the shipped file; `scripts/tests`
1012 passed, 2 skipped.
Follow-ups filed: #845 (post-review-verdict.sh does not check its own account is allow-listed) and
#849 (post-write verification: three routes leaving an exemption `success` over a human `failure`,
plus the retarget fence's post-POST gap, plus the prose sweep that lands with the behaviour).
fixes#742
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
The verdict words lived in two hand-written shell copies — the `case` arms of
post-review-verdict.sh (write) and the POS_RE/NEG_RE regexes of
check-review-verdict.sh (read) — held together by nothing but a comment that had
already gone stale. scripts/lib/review-verdict-vocabulary.sh now declares them
once and both sides derive; neither script enumerates a verdict word any more.
Only the WORD SET moved. The grammar stays in check-review-verdict.sh, where
every #629 false-open actually lived.
No parity test: #774 shipped one and withdrew it after six rounds, because a
regex over shell source is not a shell parser. The proof is behavioural and
graded MUTATION — the harness restores the pre-#788 hardcoded POS_RE each run and
requires it to redden.
Enforcement is a DATA dependency, not a control-flow gate. Review round 1 found a
real fail-open in the first commit: `${#arr[@]}` is nounset-safe only for a
declared-empty array, and under `set -u` that error inside a function called as
`if ! validate` skips BOTH branches — so on the reader (deliberately no `set -e`)
an explicit BLOCKED @ head classified `positive`, exit 0. Validation now sets a
sentinel on its last line and the derived views refuse without it.
Six cold review rounds; rounds 2-6 found no fail-open across differential fuzzing
(4788 / 2612 / 7560 payloads, zero divergences from origin/main's grammar),
sentinel forgery, environment poisoning, declare -p evasion on bash 5.3 and 3.2,
path/symlink resolution and probe TOCTOU. Every malformation fails closed: reader
exit 2, writer exit 1 with nothing posted.
Also corrected: CLAUDE.md and release.review-verdict-gate both enumerated the
vocabulary without LGTM, a word the code has accepted since #629.
fixes#788
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Implements the three-level field-help pattern from #734 as a shared component: field name + optional one-sentence summary → a one-short-paragraph panel behind a consistent Info-icon trigger → a future external-docs deep link (`docsHref`, built and typed; no screen passes one yet).
Adopted on FFmpegProfilesScreen (9 fields), documented as docs/spa-conventions.md §15 with decision record `spa.field-progressive-disclosure`, and mirrored into the design-system prototype.
The panel is portalled to document.body: `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant whatever its z-index, and one field's explainer rendered 12px of a 92px paragraph in every state of the Audio card.
A `::before` hover bridge was added and then WITHDRAWN — it held for a vertical descent onto the panel and failed for a diagonal one, leaving a safe sideways exit of 1.25px on an 18px icon. Hover reads the paragraph in place; the panel's interactive content is reached by pinning.
Four cold adversarial review rounds; the first three returned BLOCKED. They found five wrong copy claims across nine paragraphs and two vacuous tests in a row for the same mechanism.
Deferred with owners: #839 (placement verified by hand, not by a test) and #840 (the portal puts a docsHref link at the end of the tab order).
fixes#734
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Both search indexers opened UpdateSong with
metadata.AlbumArtists ??= [];
metadata.Artists ??= [];
Artists/AlbumArtists hold the whole list in ONE COLUMN rather than
being navigations. So unlike the same `??= []` idiom on
Genres/Tags/Artwork all around them, the property IS the column value:
assigning it on a TRACKED entity flips the entry to Modified and the
next SaveChanges writes [] over a NULL column. This is the mechanism an
adversarial review demonstrated in #691, which is why that issue's
entity-level guard was reverted in favour of guarding at the read site.
Measured rather than reasoned about, per the issue's first done-when
box. Restoring ONLY the `??= []` clause (the real predecessor lines,
not a hand-written mutant) reddens the new fixture on
`metadata.Artists should be null but was []`; a probe variant with the
first two assertions replaced by prints reports STATE=Modified and the
raw column moving from NULL to "[]". Today's two feeds are both
AsNoTracking (SearchRepository.GetItemToIndex and GetAllSongs), so no
shipped caller loses data -- but that is a property of two callers, not
of the indexer, and #691 already recorded it as a loaded gun. The
fixture pins the indexer's own contract instead.
Removing the assignment is not sufficient alone: it was load-bearing
for the four reads below it, and deleting it by itself converts a
silent write into a live throw on every untagged song. Measured by
deleting only those two lines from the real predecessor file:
NullReferenceException, thrown at the foreach (cited by symbol: a line
number in a mutant that exists in no committed tree is unreproducible
by construction). The
exception type follows the read FORM, not the field -- foreach yields
NRE, string.Join/ToList yield ArgumentNullException -- and this PR
contains two of each, which is why no single exception-name grep
characterises the class. So each site moves together with its reads:
- LuceneSearchIndex.UpdateSong / ElasticSearchIndex.UpdateSong: hoist
Optional(...).Flatten().ToList() locals and read those.
- RefreshChannelDataHandler: the Scriban context took the raw nullable
lists (the issue's second item). The shipped _song.sbntxt only does
array.join, but a custom template is free to do anything.
The population was derived from the MODEL rather than from the issue's
file list, and the obvious derivation is wrong: "the IList<string>
properties under ErsatzTV.Core/Domain" returns two of eight. It misses
the six value-converted collections (ProgramScheduleAlternate and
PlayoutTemplate each carrying DaysOfMonth, MonthsOfYear, DaysOfWeek),
declared as plain ICollection<T> and made single columns only in
Data/Configurations -- and their storage differs (comma-separated text
for the int converter, JSON for the enum one), so the shared property
is "one scalar column", not the serialization. No site applies `??=`
to any of the six, so this defect has no instance there; whether a null
can REACH one at runtime is unverified and is filed as #823 rather than
asserted either way. Only the SongMetadata pair is left NULL in
practice, by FallbackMetadataProvider. Every site touching either field
was then swept; the remaining readers were already guarded by #691.
The fixture carries two anti-vacuity guards, both witnessed:
- A POSITIVE CONTROL (`writer.NumDocs.ShouldBe(1)`). Every other
assertion says something did NOT happen, so all of them hold
vacuously if UpdateSong never runs -- and it silently stops running
if a future refactor gates UpdateItems on `_initialized`, which this
fixture bypasses by injecting the writer. Verified BOTH directions:
with that gate added the control fails `NumDocs should be 1 but was
0`, and with the control removed the whole test PASSES while the code
under test is unreachable.
- A capturing logger, because UpdateSong wraps its body in a catch that
assigns metadata.Song = null -- severing a required relationship and
cascading the metadata to Deleted. Without it the probe silently
measures the error path; on the first run it did exactly that (a bare
ILanguageCodeService substitute NPEs inside AddLanguages). The raw
column helper also fails loudly on a missing row, since ExecuteScalar
returns CLR null for both "NULL column" and "no such row".
ElasticSearchIndex has no equivalent fixture -- it needs a stubbed
transport -- so its change is by inspection against the Lucene one, and
the gap is filed as #824 rather than covered by a source-text guard.
The whitespace-only churn in ElasticSearchIndex.cs is the #311
fix-as-you-touch format gate: it scopes to whole changed FILES.
`git diff -w` over that file shows only the two hunks above.
Local gate: ErsatzTV.Tests 2006 passed / 4 pre-existing skips,
Core.Tests 685/1 skip, Infrastructure.Tests 114, Architecture.Tests 7,
Scanner.Tests 1504 -- 0 failures in each. scripts/tests 874 passed / 2
skipped. dotnet format whitespace --verify-no-changes clean on the four
touched files, no BOM on any. decisions_validate OK.
Fixes#701
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Graded a nit and not blocked on, but it is a claim about a neighbouring subsystem that is
one notch too strong: the hook decides on the combined `.state` and its filters exclude
`skipped` (#593). Left as-is it would have taught the next reader that any non-success
context denies, which is what #593 exists to correct.
refs #772
Both remaining review findings were the same shape as the one before them, and it is the
shape this repo keeps recording: a claim corrected in one place, its copy left standing
somewhere else in the tree.
* `docs/ci-cd.md` said "gates nothing" in the small-lane paragraph while the section 1441
lines below said the opposite. A red preflight lands in the PR's combined status, which
the merge gate reads (#598) — what it does not do is SKIP the jobs it diagnoses, and
that is now the sentence in both places.
* Two docstrings in the preflight's test file still described the disarmed script as
warning and exiting 0. Built the mutant and ran it: it emits an error and exits 1. The
exit code separates nothing now that an unverifiable answer fails too — the DIAGNOSTIC
is what the mutation destroys, which is what `mutation_manifest.py` already said and
the prose next to it contradicted.
Nits from the same pass: the admin-cron URL is quoted (`?` globs in zsh, the operator's
shell); the retry assertion's message quoted a threshold it does not use; the arm table
omitted the malformed-credential shape the code and tests both have; `buildx inspect` no
longer `--bootstrap`s a builder just to read its name, and an empty capture no longer
produces a noisy `buildx use ""`.
Swept the tree for the shape rather than the two reported lines: the surviving "exits 0"
and "could-not-tell" hits are other subsystems, or the concept named as a concept.
refs #772
The advisory narrative check was right about both new passages: 'an earlier draft warned and
exited 0' and 'both cold reviews found it independently' only parse to someone who saw the
session. What a cold reader needs is that warning-and-exiting-0 is the natural way to write
this check and is wrong, and what it costs — which is now what the doc says.
refs #772
The re-review's one HIGH was mine and was the obvious one to miss: the previous commit changed
the preflight so an unverifiable answer FAILS, and left a `docs/ci-cd.md` paragraph two
screens away still saying "anything else is reported as could-not-tell". That paragraph is
the one an operator reads when the job goes red, and it would have talked them into
reinstating the defect. Replaced with the full arm table, including the two rows the first
draft got wrong and why.
* "gates nothing" was false in the way this repo has recorded before (#598): the
merge-consent hook reads the COMBINED status, so a red preflight blocks the merge like
any other red job. It does not SKIP the jobs it diagnoses; that is the accurate claim,
in ci-cd.md and in the remote-state row.
* The production retry defaults were evaluated by nothing — every test overrode both
knobs. A test now drops the overrides and measures three attempts and a real pause, so
editing the default to 1/0 (which would falsify the "a blip does not redden a PR"
argument) goes red.
* `journalctl -u gitea | grep ExecuteCleanupRules` is not a reproduction: that identifier
reaches the log only through slow-query warnings, so an empty grep on a healthy host
reads as "the rule never ran" — the inverse. Replaced with the admin cron API, which
answers deterministically.
* The recovery recipe's `docker buildx use default` needs the containerd image store to
`--push` (both named hosts have it, checked today) and mutated the operator's builder
selection without restoring it.
* The stub's comment claimed both halves of real curl's transport failure mattered; only
the exit status is observable, because `|| resp=""` discards what curl printed.
* The empty-half credential refusal echoed the username; it needs no value at all. The
401/403 arm aborts the remaining pins while 404 continues — deliberate, now stated.
* `curl -u "$VAR"` puts a credential in argv, and this job runs container-free on a shared
host. NOT fixed here: it is the shape all five `scripts/` callers already use, so fixing
one site leaves the class and splits the codebase. Filed as #821 and named at the site.
refs #772
refs #792
Two independent reviewers (one cross-family) converged on the same defect, and it was the
important one: the preflight WARNED and exited 0 on every answer that was not 200 or 404,
so a missing `curl`, a moved registry or a DNS change would have left it green forever —
"the check could not run" presenting as "the pin is fine", in a script whose own header
disclaimed exactly that. Unknown answers are now retried (3x, 5s) and then FAIL, with
wording kept distinct from the deleted case because the two send an operator to different
places.
Also from the reviews:
* An absent secret does not arrive as an unset variable. `${{ secrets.X }}:${{ secrets.Y }}`
interpolates to ":", a perfectly non-empty and perfectly useless credential, and the
tests covered only the unset shape. Both halves are now required, and the parametrised
test drives the production shape.
* HTTP 200 is not a manifest. A proxy or a login page answers 200 too, so the body is
fetched and matched for `schemaVersion` (a shell `case`, so no jq dependency and no
pipeline that can inject).
* The curl stub ignored `-u` and answered 200 regardless, so deleting the real `-u` would
have left the suite green while the live registry rejected every request. It now 401s an
unauthenticated read, as the registry does.
* The mutation's declared diagnostic changed with the script: now that unknown fails too,
the exit code no longer separates "deleted" from "could not check", so the proof turns on
the message and `expect` says so.
* docs/ci-cd.md: `scan` is no longer the only `docker-build.yml` job on the small lane, so
the tag-push exclusivity claim and the lane membership were both false. Fixed.
* "Immutable" was overstated: `ci-image.yml` tags `rev-parse --short HEAD`, so a dispatch or
a weekly no-cache run at the same HEAD republishes that tag from a rebuilt image. Stated,
along with what the rebuild recovery does NOT restore (mutable bases and apt, so equivalent
rather than bit-identical).
* The recovery recipe left you in a worktree checked out at the pin commit — where the
verify script does not exist, and where the workflow carries the pre-bump pin. It now
keeps `$repo`, returns, and removes the worktree. It also needed BuildKit's `http = true`
caveat: the container driver does not inherit the daemon's insecure-registries.
* The root cause carries its evidentiary limit and its reproduction commands, and says what
to conclude if a pin vanishes after server-management#842 lands (refuted, not re-applied).
* The `ci.required-job-step-execution-markers` carve-out named one container-free job; there
are two now, and the membership is what rots.
* The decision record's `''` YAML escapes leaked into rendered prose; "status, no comment ->
ask" is qualified (a prior positive verdict for the SAME head still satisfies condition
(c)); "exits 1" is "exits non-zero" (usage exits 2, jq its own status, signals 128+n).
refs #772
refs #792
Decisions-Edit: yes
#772 — the pinned CI toolchain image can be deleted out from under us, and when it was
(2026-08-11..13) all five `container:` jobs died at image pull, both required contexts
included, with the cause buried in each job's log. Root cause is registry-side and is now
established rather than guessed: an owner-level Gitea package cleanup rule (keep_count 15,
remove_days 1, remove_pattern `.*`, keep_pattern no 7-hex sha can match) deletes a sha tag
once 15 newer versions exist, and `ExecuteCleanupRules` ran nightly through the window. The
`ersatztv` package carries the same rule's fingerprint exactly — every sha tag older than
the 15-slot window is gone, every keep_pattern tag back to 26.3.1 survives. Version deletes
leave no audit row, so the specific run cannot be replayed; that limit is stated where the
claim is made. The durable fix belongs to the registry's repo: server-management#842.
What lands here is what a consumer of someone else's registry can do:
* `toolchain-preflight`, a container-free job (a job consuming the image could not run to
report it missing) resolving every pin against the registry and failing with a message
that names the tag and the recovery. Not a `needs:` of the jobs it diagnoses — gating
five jobs behind a checkout and one curl taxes every green run to speed up a rare red
one, and they already fail fast.
* Only HTTP 404 means gone. Everything else is could-not-tell, and rejected credentials
fail rather than pass as unknown — "the check could not run" must never present as
"the pin is fine".
* A recovery path that does not need CI: rebuild the SAME tag from the commit it names
and push it. The push half was verified against this registry on 2026-08-22 with a
throwaway package (created, resolved 200, deleted).
#792 — the reported defect was the exit code, and re-measuring says that premise is false:
every no-status path already exits 1, and eight refusal modes now assert it against the real
predecessor, where they pass. The observed 0 came from the invocation, not the script. What
WAS broken is the half-state the issue describes second: the comment was written before the
status, so every refusal left `Review-verdict: MERGEABLE @ <head>` on a PR with no gating
status behind it. The two writes are now ordered status-then-comment, which makes the only
reachable half-state the safe one — a status with no comment leaves the merge hook's
condition (c) with nothing to classify, which is an `ask`. The refusals themselves are
untouched. Ordering rather than compensating deletion: an orphaned-comment cleanup needs a
Gitea call, and these refusals are usually caused by Gitea being unreachable.
Proof for the ordering is the split against origin/main's script: the 8 orphan/ordering
tests go red there, the 8 exit-code tests stay green.
fixes#772fixes#792
Refs: server-management#842
Decisions-Edit: yes
`testing.guard-derives-population-from-source` (#774) was silent on the commonest
population in our own guards — files in a directory — and every one answered with a
filesystem walk. A walk is not authoritative: it reports build output, generated
shims and editor droppings, and differs per machine. #778 measured the cost by
getting the same population wrong three times in one PR.
CONVERTED (a completeness claim over tracked files): `test_guard_inventory.py`,
`test_hook_fire_log.py`, `test_ci_image_pin_population.py` (which also gained
`*.yaml`), `test_remote_state_inventory.py` (folded onto the shared derivation), and
`test_pr_changed_files.py` (not on the issue's list — found by sweeping the whole
repo).
ASSESSED AND RECORDED, not silently skipped: `_repo_copy` takes its file list from
the index for hermeticity though it makes no completeness claim;
`test_ci_dropped_step_guard.py` has no filesystem population at all; the decisions
corpus is recorded as unexamined rather than cleared; and the SPA page-size guard is
deferred to #819 with its obstacle documented. This is not "replace every glob".
`scripts/tests/tracked_files.py` is the single derivation.
`test_guard_populations_derive_from_git.py` proves it in two measured complements:
exhaustive removal catches a hardcoded `.exists()` admit and memoisation; the call
log catches an append-only source that yields nothing on this machine — #778's
shape — which removal cannot see because it has nothing to remove.
Twelve rounds of independent cold review, alternating model families in isolated
worktrees. The production derivations were confirmed sound every round; every
blocking finding after the first was in the proofs or in prose claims about them.
Counts over growing populations were removed rather than corrected, after three
drifted.
fixes#806
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
`ci.cancelled-is-not-a-verdict` documented the run/job API, where cancelled is distinguishable. The endpoint a CI monitor actually polls — `commits/{sha}/status`, the per-sha view the merge gate reads — has no `cancelled` state and reports one as `failure`. The record now says to resolve the job-level `conclusion` before reporting a red.
The kickoff's push HARD CONSTRAINT is tightened from "a review has run" to "a CLEAN verdict, zero outstanding findings on the current tree", since #790's rounds 7 and 8 each still found a real mechanism defect and every earlier push auto-cancelled a live run.
Refs #790
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Every `MUTATION` row of `docs/guard-inventory.md` now carries a DECLARED clause mutation that is applied to an isolated copy of the repository on every suite run, with the row's own named test required to go red carrying a declared diagnostic. Manifest and MUTATION rows are compared for set equality both directions; the other 22 guards each carry a stated reason, compared the same way.
Measured rather than assumed: 12 of 13 guards admit a single-clause mutation; `instrumentation_faults` does not, and that entry carries the surviving finer mutation, re-run every suite.
Nine cold cross-family review rounds. Rounds 1, 2, 7 and 8 each found real mechanism defects — two mutations that measured nothing, an incomplete git-environment sanitisation, a reset that restored its own mutant, and a proof of that fix which was not itself isolated. All fixed and witnessed red.
fixes#790
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
#780 and #784 were green separately and red together: the gate landed on a base
that predated scripts/check-doc-narrative.py, so nothing ever ran ruff over it.
- RUF100 x2 on `# noqa: BLE001` — BLE is not in this repo's select, so those
directives suppress nothing. Enabling BLE instead was measured and rejected: 6
further sites in decisions_validate.py, whose broad catches are deliberate. The
non-enabled code is dropped; S110 and both comments stay.
- scripts/tests/test_check_doc_narrative.py was not ruff-formatted.
Verified with the shipped invocation: 35 files, all checks passed, all formatted;
suite 807 passed / 2 skipped.
refs #780, refs #784
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Python lint here was a property of the operator's laptop: the global instructions
say to run ruff, no workflow ran it, and with no committed config ruff fell back
to whichever ~/.config/ruff/ruff.toml the machine happened to have.
- ruff.toml at the root, pinned ruff==0.12.11 in the script-tests job.
- Both lint steps pass an EXPLICIT population from `git ls-files` with
`--no-force-exclude`, never `ruff check .` — an `exclude` empties a
discovery-based run into a GREEN one (top level empties both commands, [lint]
empties check, [format] empties format --check), and `ruff check .` over zero
files exits 0 with only a stderr warning. Guarded by an empty-population arm.
- Tree clean: 74 findings at 706674272, 57 fixed in code, 17 per-site noqa with
reasons inline. S105 deliberately per-site, not a directory blanket. RUF100
selected so a suppression that suppresses nothing is itself a finding.
- pyright stays ungated; reasoning in the record.
Both steps witnessed red on the runner against the shipped bodies: run 2173 job
9176 (ruff check) and run 2170 job 9163 (ruff format --check).
Docs: new record ci.python-lint-ruff-config-committed, ci.script-tests-job
cross-ref, docs/ci-cd.md (also correcting a stale ~190-tests/~10s figure to the
measured 773 tests / ~4.5 min), docs/defect-shapes-773.md §5.2 resolved.
fixes#780
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Closes the remaining three entries on #785's ranked list with clause-level mutation proofs, each
witnessed red against the real subject in place:
* the `pretooluse-worktree-guard.sh` + `posttooluse-worktree-marker.sh` PAIR — four clauses,
including the cross-file seam (a clause in the marker hook, asserted against the guard's
decision) that could not exist while the halves were tested apart;
* `.husky/pre-push:11`'s `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE` — git exports `GIT_DIR` to
`pre-push` only from a worktree, which `process.shared-tree-readonly` makes the mandated way to
work here, so the guarded case is the normal one;
* `scripts/build_decisions_catalog.py --check` — including the `__main__` wiring, which can print
"is stale" on stderr and still exit 0.
Nine ways the catalog guard can stop gating are detected, judged by executing the step's whole
`run` script rather than by matching lines out of it. Two channels are undecidable outside the
runner and are stated as uncovered rather than guessed at.
Inventory regraded to 12 MUTATION / 6 BEHAVIOUR-ONLY / 16 NONE, with a stated reason for every
remaining NONE row, verified member-for-member against the derived set.
Five cold review rounds; findings closed include production-hook-fire-log corruption, a
tautological assertion, a guard asserting on its helper rather than on the effect, and two false
greens in the workflow extractor. Follow-up: #809.
fixes#785
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Two conventions from #773's detector menu (F and G), each as a decision record plus a filled gap.
Part 1 — the deny path at the production config value. Every assertion about the API read-gating posture ran through a hand-written fake HANDED the boolean, and no test in the repo constructed ApiKeyProvider at all, so the line deriving that posture from configuration had never executed. Now covered across the matrix through the real provider: absent, true/True/TRUE, false/False, and a present-but-non-boolean value (which throws at startup — fail-closed, pinned).
Part 2 — a full-replace path asserts its complete field list. ScheduleItemResponseRoundTripTests is the release gate for the flat schedule-item DTO, and its comparer was itself a hand-copied field list: complete when written, unable to report when it stopped being. Now derived by reflection with an empty exemption set and a written-down count pin (55).
Four cold adversarial review rounds. Three returned BLOCKED, every one on a claim in a decision record that the code contradicted — the exact defect the records exist to name. The surviving rule, now written into the record: state the measurement and the code path you actually read; do not generalise from one executed case, and do not explain a mechanism you did not measure.
Residual SPA optional-field drift tracked as #807.
fixes#779
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Local review returned MERGEABLE — no Blocker, High or Medium, both sentinels verified
pinned by mutation, no regressions. Its one Low is taken rather than deferred, because
it is a one-line fix and because deferring it would leave exactly the shape this PR
exists to document.
A 200 whose body is NOT an array never reaches the classifier: the array gate diverts
it, `bp_code` stays 200, and the generic ask then reported "HTTP '200' — Gitea
unreachable, or these credentials lack the repo-admin scope" about a read that plainly
succeeded. That is the identical defect the previous commit fixed for the
throw-inside-the-classifier arm, one branch earlier — fixed where it was noticed, left
in its twin.
The previous commit's message even generalised the pattern ("a sentinel that doubles as
an HTTP code makes a decision state a cause that did not happen") while covering only
one of the two arms it applies to. The test is now parametrised over all three shapes
that reach an unusable 200 — UNPARSEABLE-RULES, GARBAGE, EMPTY — and reverting the new
sentinel reddens the two that the first fix missed.
Also finishes the de-indent the previous commit claimed: that comment block went from
19 leading spaces to 6 while its siblings use 2, so the claim was true of the direction
and not of the result.
729 tests green.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Local review of the previous commit returned BLOCKED. Both findings taken; nothing
pushed to CI while this was iterating.
MEDIUM — the `nomatch` fix was entirely unpinned. Reverting both sites to `bp_code=404`
left the suite 33/33 green, because no fixture ever emitted an HTTP 404 on the LIST
read: the codes exercised were 000, 403, 500 and 200, and the old `NOT-FOUND` mode had
been repurposed to return 200 with `[]`. So the defect that commit describes could be
reintroduced silently — in a PR whose subject is unfalsifiable guards, one round after
being blocked for precisely that shape. There is now a `LIST-404` fixture and a test;
reverting the sentinel reddens two tests.
The same class, one arm over and found while fixing it: a 200 whose `branch_name` is a
number makes the classifier throw (`//` fires only on null/false), and that was mapped
to `bp_code=000`, reporting "HTTP '000' — Gitea unreachable" about a read that plainly
succeeded. It gets its own `unreadable-rules` sentinel and message, with a fixture and
a test — reverting it reddens.
The pattern across both: a sentinel that doubles as an HTTP code makes a decision state
a cause that did not happen. The decision was safe each time; only the reason lied.
MEDIUM — four comments still described 404-as-a-finding as live, contradicting the hook
comment added in the same commit. The worst said a 404 means "this branch is entirely
unprotected" in a test whose fixture now returns 200 with `[]`, which would have talked
the next reader into re-adding the deny. Renamed that mode `EMPTY-LIST` so it says what
it is.
Also: the hook quoted a reason string it no longer emits; a stray over-indented line
survived the de-indent; `bp_called` was write-only after its reader was removed.
727 tests green.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirmation pass returned BLOCKED, and its lead finding is the one worth having.
The test forbidding the by-name lookup recorded URLs from INSIDE the
`endswith("/branch_protections")` branch, so the only URLs it could ever record were
ones that already satisfied the assertion. A by-name request was invisible to the very
test written to forbid it. Cold review proved it by reintroducing the lookup in the
hook: the suite stayed 33/33 green. That is the filter-on-the-asserted-property defect
this PR's sibling record exists to describe, committed inside the guard against it —
and the commit message had called the twin "pinned so it cannot come back".
The recorder now sees every branch-protection URL whatever its shape. Re-verified by
the same mutation: reintroducing a by-name call reddens exactly the two tests that
forbid it.
Also from that pass:
- an HTTP 404 on the LIST read reached the "the full rule list was read and none
matches" deny — a claim about a read that never happened. Gitea answers 404 for a
repo that is absent or invisible to the credential, so the classifier's own verdict
is now the sentinel `nomatch` and HTTP failures reach the ask;
- two comment blocks still described the deleted by-name endpoint as live, one of them
asserting the classifier "is never reached at all";
- the decision record still documented `branch_protections/{base}` and its 404
semantics as the mechanism, in the record this PR authored — now rewritten to the
list endpoint, with why reading the LIST is the load-bearing choice;
- seven assertions on a string the hook no longer emits, and three test
names/docstrings describing the removed 404 flow;
- an unused fixture helper, and 79 lines left over-indented by the removed nesting.
724 tests green.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Final review returned MERGEABLE with no Blocker and no High. Its one Medium is taken,
and it is my own recurring trap for the third time in this PR: fix one path, then check
its TWIN.
The hook looked a rule up by NAME first and enumerated the rule list only on a 404.
But `branch_protections/{name}` is an exact DB lookup — `GetProtectedBranchRuleByName`
— which performs no matching and knows nothing about precedence. A 200 from it means
"a rule with this NAME exists and lists this context", never "this context is required
on this branch". So the precedence argument added last round guarded the 404 path while
the 200 path granted without it — and since this repo's rule IS named `main`, the by-name
lookup always returns 200. The hardened code was dead and the unhardened code was live.
Given a rule `main` requiring review-verdict/h10 and a rule `m*` with better Priority
that does not, Gitea applies `m*`; the by-name hit on `main` saw h10 and granted anyway.
Fixed by DELETING the twin rather than documenting it: one fetch of the full list, one
classifier, one argument, no second path to keep in step. Two things fall out for free
— the ref no longer reaches a URL segment, so the percent-encoding hazard is removed by
construction rather than escaped (its test is replaced by one asserting no ref reaches
the URL at all), and every case the classifier already covered now applies to the live
path instead of an unreachable one.
Verified against the live Gitea: the classifier returns `exact` -> rule `main` ->
enable_status_check=true, h10 present. A new test pins that the precedence check runs
even when an exactly-named rule exists, and asserts the by-name endpoint is never
requested, so the split cannot come back silently.
The grant string now states what was actually established — read from the full rule
list, matched with Gitea's own plain-vs-glob split, refusing wherever precedence or
folding is not derivable — rather than the stronger "confirmed required" it claimed
while consulting a single named rule.
724 tests green.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirmation review returned MERGEABLE at 356cddbb5, having independently re-executed
every claim (the "2 -> 6 dead-classifier" figure exact, the "20 tests" trap figure
exactly reproducible, the arm reorder pinned, live branch protection confirming the
dated claim). Its three residuals are taken here.
M1 — the backslash test hit the undecidable arm but asserted only `"ask" in reason`,
which a CRASHED classifier also satisfies. Its two siblings got `"could govern it"`
one commit earlier and this one did not. Under the dead-classifier mutation it was the
single green test of that arm; now red.
L1 — with two plain rules differing only in case, `first` picked LIST order while
Gitea picks by Priority. Given `MAIN` requiring review-verdict/h10 and `main` not, the
hook could inspect the rule that requires it and auto-grant on a base where Gitea
enforces the other — the same defect as the arm order, one level down. Two fold-equal
rules are now undecidable.
L2 — the same standard, applied where I had waived it. The backslash paragraph rejects
"nearly unreachable" as a standard for the arm that issues a DENY, and two paragraphs
up the ASCII-only fold was accepted on exactly those grounds: rule `ünstable` and base
`Ünstable` fold equal under Gitea's EqualFold and not under `ascii_downcase`, landing
on `none` -> deny with a false stated cause. A non-ASCII rule name or base is now
undecidable rather than fold-compared.
The non-ASCII test is `explode | any(. > 127)`, not a `\uXXXX` regex: the first
attempt was a character-class regex whose backslashes are ambiguous through a
single-quoted shell string into jq, and a standalone probe showed it classifying plain
`main` as non-ASCII. Codepoints have no escaping layer to get wrong.
Both new arms are mutation-proven individually; an earlier attempt at those proofs used
perl substitutions that silently matched nothing and reported green, which is why they
were redone in python with an explicit assert on the target.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two self-found defects while pre-empting the confirmation round's own questions.
THE REORDER WAS UNPINNED. Swapping the classifier arms back to exact-first left all 29
tests green, so the previous commit's central change was invisible to the suite — an
unproven change shipping under a green run. The missing fixture is the one that
distinguishes the orders: a list holding BOTH an exactly-named rule that requires
`review-verdict/h10` AND a glob rule that could also govern the base and does not.
Exact-first inspects the rule that requires h10 and auto-grants; undecidable-first
asks. Now mutation-proven in both directions.
I HAD MISDIAGNOSED THE TRAP, and asserted the wrong cause in a comment. A three-line
repro disproves "an EXIT trap suppresses output" — it does not. The real mechanism is
that this file already owns its EXIT trap: `scripts/hook-fire-log.sh` installs
`trap 'etv_hook_fire_end "$?"' EXIT` (#776), and in capture mode that handler is what
REPLAYS the decision JSON to stdout. A second `trap ... EXIT` silently replaces it, so
the decision is captured and never emitted. The trap slot is a single shared resource
and the sourced library claimed it first.
That second one is the shape this whole PR is about, committed against my own work: an
explanation that fits the symptom, is written down as fact, and is wrong. It would have
told the next person the construct is unusable rather than that the slot is taken.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ninth cold review: MERGEABLE, no Blocker, no High. Its three Mediums taken anyway,
because each was a one-line fix retiring the last "asserted rather than verified"
surface in the file whose whole subject is that shape.
M1 (backslash missing from the metacharacter class) was already closed in cd1b28637 —
found independently while stress-testing the superset claim, in the window the reviewer
was working against the previous head.
M2 — the `exact` arm was not decidable. Gitea picks the governing rule with
GetFirstMatched over a list sorted by Priority and THEN by plain-name-ness, so a glob
rule with a better Priority outranks an exactly-named one. Preferring `exact` would
inspect a rule Gitea might not be applying: if the exact rule requires
`review-verdict/h10` and a higher-priority glob rule does not, the gate auto-grants on
a base where the check is not enforced. `undecidable` is now evaluated FIRST, which
makes the classifier sound without knowing the precedence rules at all — the only
claim this code is entitled to make about somebody else's resolver.
M3 — four tests could not distinguish "classified correctly" from "classifier
crashed", because a dead classifier lands on the generic could-not-read ask and they
asserted only `"ask" in reason`. Measured with the reviewer's method rather than
argued: injecting `error(...)` at the head of the jq program left 2 of them red; the
strengthened assertions leave 6. The glob arms now pin the text unique to the
undecidable ask, and the decidable arms assert the rule was HONOURED rather than
referred to a human.
Also: the module docstring still described a three-arm contract after this change added
a fourth; the inventory row still summarised the old two-way behaviour; the ASCII-only
case fold is now stated as a deliberate under-match rather than as parity with
EqualFold; the live-config claim is dated; and one jq call rejoined the file's
`|| true` discipline.
A `trap ... EXIT` for temp-file cleanup was tried and REVERTED: it suppressed the
hook's decision output entirely and took 20 tests red. A gate that prints nothing is
the one outcome it must never produce, so the tidier construct loses to the one that
works, with the reason recorded where the next person will try it.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-found while stress-testing the superset claim I introduced one commit earlier —
the crux the new classifier's safety rests on.
`none` authorises a DENY on the stated grounds that nothing can possibly govern this
base, so its premise must hold unconditionally, not usually. The superset was "literal
prefix + .* + literal suffix", which is sound for every glob dialect EXCEPT one case:
gobwas/glob reads `\{` as a LITERAL brace, so a rule `a\{b` governs the base `a{b`,
while a superset treating `\` as an ordinary character builds `a\.*b`, misses, and
denies a base that is in fact protected.
Verified before and after: with `\` outside the metacharacter class the classifier
answered `none` for that pair; with it inside, `undecidable` -> ask. 18 adversarial
rule/base pairs (brace alternation, negated and ranged classes, `**`, leading and
trailing metacharacters, unicode, empty alternation) all answer `undecidable`, never
`none`, so no dialect-matching case falls through the deny arm.
Git ref rules make this nearly unreachable — a branch name may not contain `*`, `?`,
`[` or `\` — but `{` IS legal in one, and "nearly unreachable" is not the standard for
the arm that issues a deny. Checked with `git check-ref-format` rather than assumed.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eighth cold review: no Blocker, no High. Three Medium, two Low, one Nit.
MEDIUM — the substantive one. The glob fallback asserted it matched rules "the same
way Gitea applies them", and it does not. Gitea compiles a rule name with gobwas/glob
and a `/` separator: its `*` does NOT cross a slash, `?`/`[…]`/`{a,b}` are wildcards,
and a plain name is folded case-insensitively. Mine used `.*` for `*` and escaped the
rest. The divergence has a false-OPEN direction — `release/*` does not govern
`release/26/hotfix` in Gitea, but `release/.*` matched it here, which would auto-grant
a scheduled merge on a base where the check is not required. That is #622's hole,
reached through the block written to close it, via exactly the failure this PR
records: a claim about an external system asserted rather than verified.
Reimplementing somebody else's glob dialect would be a second copy of a parser, which
this repo has already withdrawn a change for. So the classification is three-way and
each arm is safe WITHOUT knowing the dialect: an exact non-glob name folded
case-insensitively is decidable; a glob rule that could govern the base is
UNDECIDABLE and asks; and "could" is tested with a provable superset of any glob
dialect — literal prefix, `.*`, literal suffix — so if even that cannot match, no
dialect can. Over-matching would grant on unestablished protection; under-matching
would deny with a false cause. Asking is the only answer honest in both directions,
and it is rare: this repo's rule is the plain name `main`.
MEDIUM — a count that was wrong the moment it was written ("46 of the 69 rows are
N/A"; it is 44). It was added by the same commit that demoted two rows. That is the
FOURTH stale number in this change, in the deliverable whose own record argues against
hand-maintained counts. Removed rather than corrected, with the reason stated.
MEDIUM — `migration-smoke.sh` still said "Same shape" as `security-scan.sh`, whose
note had just been rewritten to the opposite conclusion, so the backreference had
silently inverted. It is the same pull-then-run over a mutable tag and deserves the
grade MORE, not less: `security-scan.sh` boots a throwaway container and authorizes
nothing, while this gates a production stack recreation. Regraded.
LOW/NIT: an `end <= start` guard that `str.index(…, start)` makes unreachable by
construction is replaced with the reachable failure it was describing; a docstring
still named a fixture from an earlier draft; a reflowed `#707.` was line-initial.
Two mutations were ineffective on the first attempt — one left the `decide ask`
continuation in place, the other had no test covering case-folding. Both redone; each
arm now reddens a named test.
Decisions-Edit: yes
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seventh cold review. One High, one Medium, three Low, seven Nit — all in the two
newest commits, which is where every round of this PR has found its defects.
HIGH, and it is my own fix from the previous commit. In jq source `"\\\\"` decodes to
TWO backslashes, so escaping produced `\\.` — "a literal backslash, then any
character" — instead of an escaped dot. Every rule name containing a metacharacter
became UNMATCHABLE, and a rule named `a[b` crashed jq outright (swallowed by
`|| true`). Verified: `release/26.*` no longer matched base `release/26.4`, so the
fallback found nothing and hard-DENIED with the stated cause "has NO branch protection
at all" — converting a false-open into a false deny, which the block's own comment
calls the worse outcome. One character: `"\\" + .c`. Correct across 14 rule/base pairs.
WHY MY TEST MISSED IT, which is the transferable part: it asserted only the NEGATIVE
direction (`mai.` must not match `main`). A rule matched literally and a rule made
unmatchable both fail to match the wrong base, so the assertion passed for the wrong
reason. Only a rule that SHOULD match separates them, and there was no positive
control. There is now — plus a char-class case — and both go red against the
over-escaped version. That also needed a base containing a dot: a rule cannot carry a
metacharacter and still match `main`, so the first attempt at the positive control was
unsatisfiable by construction.
MEDIUM — four live claims that the population "derives from the filesystem", left
standing by the commit that replaced that mechanism: the guard's own docstring 45
lines above a comment shouting the opposite, the inventory heading 21 lines under
"Every git-tracked file", the docs/README entry, and — worst — the record's
`mechanics:` frontmatter, which is the copy the catalog and MemPalace mirror, so
discovery would have returned the superseded lesson. All corrected.
LOW/NIT: the URL-encoding test grepped the source for `@uri` (it now asserts the URL
actually requested, and reddens when the encoding is removed); the hoist comment said
"every path below" without noting the docs-only enumeration above it (bounded — that
path is a passthrough to a human prompt, never a grant); a now-unreachable guard is
annotated rather than left reading as live; `issue-qualification-audit.sh` was `N/A`
while `select-queue.sh` was `UNSAFE-KNOWN` on the same argument, and
`security-scan.sh` claimed "one step" for a pull-then-run over a mutable tag — both
regraded; the `PINNED` definition now says what separates its second shape from an
`N/A` "one step" row (the identifier's immutability, not the step count); the section
parser raises a message naming both required headings instead of a bare ValueError;
and the record's body is rewrapped.
Decisions-Edit: yes
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by attacking my own glob fallback from the previous commit before the reviewer
got to it, which is the round where this PR's defects have landed every time.
The 404 fallback matched a branch-protection rule by substituting `*` into a raw
regex, leaving every other metacharacter active. Verified directly: a rule named
`main.x` matched the base `mainax`, and `a+b` matched `aab`. The direction is the one
that matters — a spurious match to some OTHER rule that happens to require
`review-verdict/h10` reports this base as protected when nothing governs it, so a
consent gate answers yes on evidence about a different branch.
Each literal segment is now escaped before the pattern is assembled, so the wildcard
survives and nothing else does. Verified across 11 rule/base pairs: metacharacters are
literal, `*` still spans, exact and non-matches unaffected.
The regression test needed two goes to stop being vacuous, both times for the same
reason the rest of this PR keeps hitting: the stub never 404'd for the new mode, so
the run denied earlier via the by-name lookup and never reached the fallback at all. It
now goes red against the unescaped predecessor.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The record said "wrong twice" and drew the lesson "execute the traversal and eyeball
it". The third instance (rglob picking up untracked `.husky/_` shims) shows that was
still the wrong generalisation: every round had executed its traversal, and every round
had an argument for why it was sufficient.
What held was changing the SOURCE, not the walk — `git ls-files` instead of the
filesystem. The index is authoritative, identical for CI and every checkout, and
excludes untracked build output by construction. So the lesson is the one
`testing.guard-derives-population-from-source` already states, one level up: ask what
the authoritative list of these things IS, and if the answer is "whatever the walk
finds", the guard is not finished however carefully the walk is written.
Decisions-Edit: yes
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sixth cold review (a different reviewer, in-repo, worktree-isolated after the
cross-family runs wedged twice on their sandbox). One High, one Medium, two Low, two
Nit. All fixed.
HIGH, and it is the third time this population has been wrong. `rglob` is recursive,
so it also enumerated `.husky/_/` — 17 husky shims generated by `npm ci` via
web/package.json's `prepare`, gitignored and untracked. The guard therefore derived 76
files against a 59-row table and was RED on every checkout that has run `npm ci`,
while staying GREEN in CI, whose `script-tests` job checks out and pip-installs but
never runs `npm ci`. A guard that fails everywhere except where it runs is the fastest
possible route to "that test is always broken, ignore it" — on the artifact whose
entire thesis is population correctness. Reproduced, then fixed at the source rather
than with a fourth traversal patch: the population now comes from `git ls-files`. The
index is authoritative, identical for CI and every checkout, and excludes untracked
build output by construction instead of by an exclusion list someone must maintain.
That is what this PR's own record says to do; the first three attempts each derived
from whatever happened to be on disk. Three tests go red against the rglob
predecessor.
MEDIUM — twin-missed, in the fix from the previous round. Round 4 re-read the base
before the branch-protection lookup, inside the scheduled branch only, leaving the
#632 retarget DETECTION still reading the top-of-hook snapshot. The reviewer
demonstrated it with this PR's own fixture: scheduled+retarget denied while
immediate+retarget AUTO-GRANTED. The re-read is now hoisted above every base-dependent
consumer, so one read serves both paths, and the duplicate is gone. Note for the
record: the hoist is the load-bearing part — once `live_base` is fresh, #632's own
comparison catches the retarget too, so the explicit deny only bites when no verdict
records a base. The tests are scoped to exactly that case, because as first written
they passed under mutation.
LOW — a 404 from `branch_protections/<ref>` does not prove the branch is unprotected.
Gitea keys that endpoint on the RULE name, so a base covered by a glob rule 404s while
being fully protected, and an unencoded ref containing `/` (`release/26.4`) 404s
because the path is malformed. Both produced a hard deny stating a specific, false
cause — and a deny blocks outright rather than prompting. The ref is percent-encoded,
and a 404 now consults the rule list before denying; an unreadable list asks.
LOW/NIT — the scope prose attached the extension restriction to `scripts/` alone while
the guard applied it everywhere (a `.py` hook would have joined the described scope and
acquired no row); `.yaml` workflows are now in scope too. The `PINNED` definition
required re-validation, which two legitimately-pinned rows do not do because their
check and use are one step over an immutable event-payload sha. Row ordering restored.
And once more, the recurring one: adding a scope TABLE to the doc made three prose
rows parse as inventory sites — the parser reading its own documentation as data, the
same defect as the UNSAFE-KNOWN check that once parsed the paragraph defining
UNSAFE-KNOWN. Row parsing is now bounded to the inventory section explicitly.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by applying round 5's own finding symmetrically. `$base_ref` was re-read before
the branch-protection lookup because using a value captured at hook start is not
checking. `$sha` is captured from the same snapshot and is never re-read, so every
later check — CI status, H10 status, verdict comments — evaluates against the commit a
mid-run push replaced.
Not fixed here: the base case was inside the code this PR introduced, while the sha
spans the pre-existing H10 logic, and opening that at round five of review is how a
scoped change stops being reviewable. The row now names both staleness windows (within
the run, and after the decision) instead of only the second, and #803 carries the fix.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fifth cold review: no Blockers, no Highs. 1 Medium, 1 Low, both fixed here. It also
confirmed the #803 deferral is sound and re-verified every count (59 files, 69 rows,
3 PINNED; guards 34/5/15, proofs 8/6/20).
MEDIUM — and it is the one worth the round. The branch-protection lookup used
`$base_ref` from the PR snapshot taken at the TOP of the hook, and everything between
is round trips (the file enumeration alone can be forty pages). A PERSISTENT retarget
in that gap needs no ABA and no force-push: the lookup names the OLD base, confirms
`review-verdict/h10` on a branch the PR no longer targets, and grants a scheduled
merge onto one that may require nothing. The guard written to enforce
"checking a stale identifier is not checking" was doing exactly that. The base is now
re-read and compared immediately before the lookup; a move denies and names both
branches. Mutation-proven.
LOW — my caveat erred in the rare direction, understating a clause instead of
overstating it. The scalar-row test's docstring called the whole `.statuses` member
validation defence-in-depth because the #632 block masks it. That block validates
`.context` and `.description` but NOT `.status`, so an object row with a numeric
status passes it and does reach the new validator — where without the clause it
becomes `vstate=7` and is reported as "the verdict is '7'" rather than as an
unreadable payload. The caveat is now scoped to the payload rather than the clause,
and the reachable case has its own test, also mutation-proven.
The inventory row now conditions the guarantee on BOTH of its preconditions — the PR
still targeting that base (fixed here) and the protection still standing (cannot be
closed here, and said so).
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourth cold review: no Blockers, 2 High / 2 Medium / 2 Low. It independently
re-derived the population (59 files, 59 sites, 69 rows, 3 PINNED) and verified every
numeric and factual claim in the diff, including the corrected confinement rationale.
THE ONE THAT STINGS. The grant reason string still said a commit pushed before Gitea
merges "will clear it and block the merge" — the exact sentence the new decision
record quotes as THE overclaim this issue exists to remove. I documented it in three
files and left it in the code a human actually reads. It now states the guarantee and
its condition: the required check was confirmed rather than assumed, and it holds
while that branch protection stands.
FIXED HERE (all in files this PR already touches):
- enable_status_check is validated as a BOOLEAN. `"true"` is not `true`, and comparing
the string to `true` produced a confident deny from a payload never understood —
the tri-state collapsing to two, the same defect as the contexts shape one line down.
- `.statuses` members are validated, not just the array (see the honest caveat below).
- the docs-reminder N/A rationale said "the job cannot fail and never reaches the
combined status", which is false — any job's status joins the combined state. The
true, narrower reason is that its fetch and diff are failure-swallowed, so the
remote read can only change the warning's wording.
- docs/README names the scripts/tests exclusion in BOTH statements.
A VACUOUS TEST, CAUGHT BY ITS OWN MUTATION PROOF. The regression case for the
`.statuses` member validation stays GREEN against the predecessor: the #632
base-retarget block runs first and already validates every member it consumes, so it
catches the payload before the scheduled branch is reached. The two guards overlap —
duplicate guards masking each other, again — which makes that finding LATENT, not
live, and my added clause defence-in-depth rather than a fix. The test now asserts the
observable contract (a decision is always emitted) and says plainly that it is not a
mutation proof of the newer clause. Shipping it as one would have been the exact
grade inflation round 2 rejected.
DEFERRED to #803, with the reason stated rather than implied: a head-ABA
(force-push H1 -> H2 -> H1 during pagination) defeats pr-changed-files.sh, and three
OLDER contracts still assert more than the new inventory rows do. That residual
predates #778 and lives in #707's mechanism; correcting another active decision
record inside a PR already at four review rounds is how a scoped change stops being
reviewable. The inventory rows are accurate today and now point at #803.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third cold review: no Blockers, 1 High / 5 Medium / 1 Low / 1 Nit. All accepted.
It independently re-derived the 59-file population and matched it against `find`,
so the traversal that was wrong in rounds 1 and 2 is now verified rather than argued.
HIGH — the ABA claim was too broad. `ci.verdict-write-retarget-fence` counts
`change_target_branch` events, so it fences the BASE axis and nothing else. A
force-push H1 -> H2 -> H1 during pagination leaves the final `.head.sha` comparison
equal while the middle pages were enumerated against H2, and no counter moves. Two
rows implied the fence covered that; both now state the head residual as real and
unfenced, with what closing it would take.
Also: the record still said the scheduled-merge residual was "closed one layer down"
by the branch protection an admin may have removed — the circular sentence that was
rewritten in the inventory last round and left standing in its twin. The hook header
still called an immediate merge "sound". Both now describe the bounded window.
`docs-reminder` was over-demoted by grouping it with `decisions-guard`: it cannot
fail its job, so it authorizes nothing and is N/A, while `decisions-guard` reaches
the combined status. Split, per this file's own rule that differing classifications
get separate rows. Over-demotion is a defect too — it makes the column noise.
The scope heading and the docs/README entry now name the `scripts/tests/` exclusion
explicitly, so nobody adds a remote-reading test executable expecting a red guard
that stays green, and a wrong limit cross-reference is corrected. The exclusion's
justification was also factually false — it claimed the only network calls were to
PATH stubs, but test_hook_fire_log.py starts a real http.server on 127.0.0.1 and
drives it with real curl. The exclusion stands on confinement, not on absence, and
now says so.
COUNTS. "4 of 68 PINNED" was wrong (3), and rewriting it as "3 of 68" went stale in
the same commit when splitting a row moved the denominator to 69 — three stale
figures in three rounds, in the record warning against exactly this. The exact
denominator is gone: three rows survive as PINNED out of roughly seventy, and the
load-bearing claim is "almost nothing is pinned", not an integer. A hand-maintained
count is a second copy of the table; guard-inventory.md gives its counts an equality
check because they ARE the point, and a rationale record should not pretend to.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second cold review: no Blockers, 4 High / 1 Medium / 2 Low. All accepted.
POPULATION, WRONG A SECOND TIME. Round 1 removed a content filter that had
omitted `git fetch`. Round 2 found the replacement traversal used non-recursive
`Path.glob`, so four nested files were still outside it — including
scripts/scripted-schedules/entrypoint.py, which calls get_context() against a live
ErsatzTV server and then drives define_content/reset_playout/build_playout off the
result. Now rglob, with scripts/tests/ as the single stated DIRECTORY-level
exclusion (a scope choice, reviewable in one line; not a predicate over content).
Population 55 -> 59, rows 63 -> 68.
The generalisation is in the record, because the deliverable made the same mistake
twice: the scope may be hand-written, but anything narrowing the POPULATION has to
be executed and its output compared against the filesystem — the members it drops
are invisible by construction. That is the #774 rule turned on the artifact meant
to enforce it.
FIVE MORE OVERCLAIMS GRADED DOWN. Both merge-consent head/base rows (the hook
returns `allow` and a separate call merges, so the window is small, not absent —
"no async window" was simply false); the release smoke pull and the ci-image verify
(the concurrency group is PER-REF, so a branch build and a tag build of one commit
can publish the same :<short-sha>); and the workflow base-fetch rows, which are not
advisory — the merge hook reads the COMBINED status, so any red context blocks the
auto-grant. Also fixed a stale cross-reference where the enumeration row still said
it "inherits that row's pins" from a row graded down in the same commit.
Four PINNED rows survive out of 68. That ratio is the honest finding.
CIRCULAR JUSTIFICATION REMOVED. The scheduled-merge row said its residual was
"closed one layer down" by the very branch protection an admin may have removed.
It is not closed, it is BOUNDED by a trust assumption, and the row now says so.
Low: jq's `//` fires on `false` as well as null, so `status_check_contexts: false`
was defaulted to [] and produced a confident deny from a shape never understood —
absent and null are now defaulted explicitly, everything else is "unknown". And the
title sentence claimed "every executable in this repo" while the guard covers four
directories; both it and the docs/README entry now say what is actually enforced.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Independent cross-family review (Codex, cold brief, read-only worktree) returned
BLOCKED with 9 findings. All 9 accepted; #5 partially, keeping one row PINNED with
its justification stated.
POPULATION (the finding that mattered most). The derivation filtered the scope by an
outbound-network token list and argued that was a scope choice rather than a
population filter. It omitted `git fetch` — this repo's most common remote read — so
prepush-rebase-check.sh, which fetches origin/main and derives a PUSH DECISION, was
structurally invisible to a guard claiming to cover "every executable that reads live
remote state", along with three others. The defence offered was that over-inclusion is
the safe direction; the filter also under-included. The content filter is gone: the
population is now all 55 files in the scoped directories, and a file that reads no
remote state carries an explicit N/A row.
OVERCLAIMS, graded down. Three rows asserted more than the code does:
- the scheduled-merge path was PINNED while the hook's own comment concedes the
branch-protection read pins nothing → UNSAFE-KNOWN, "preflight, not a pin";
- pr-changed-files.sh was PINNED and claimed "any movement fails", but
before-and-after equality is ABA-vulnerable (main → scratch → main) → UNSAFE-KNOWN,
pointing at the caller-side event-count fence that does close it;
- the CI toolchain image was PINNED on a mutable TAG, against this file's own
definition naming a digest → UNSAFE-KNOWN. The release smoke pull stays PINNED: it
pulls the tag the same concurrency-serialized job just pushed.
The guard-inventory MUTATION regrade is reverted to BEHAVIOUR-ONLY (8/6/20). The
review is right on species: the test feeds the real script an input the clause
rejects, which this table explicitly defines as behaviour-only and has already
regraded three rows for. A manually-executed disarm does not change what the test is.
TWO REAL FAIL-OPENS FIXED:
- jq `index()` on a STRING is substring search, so a status_check_contexts arriving
as "prefix-review-verdict/h10-suffix" answered yes and would auto-grant. Membership
is now exact equality over a value first proven to be an array of strings.
- post-review-verdict.sh guarded both re-read comparisons with `[ -n "$x" ] &&`, so a
2xx body that merely omitted .head.sha or .base.ref made the check a no-op and the
status was posted having confirmed nothing.
That second fix carries a lesson worth the line: the obvious mutation (disarm the new
`-z` arm) stays GREEN, because the unconditional `!=` also rejects empty — the two
overlap, exactly the duplicate-guards-mask-each-other shape. The proof is taken
against the REAL predecessor with the `-n` conjunct restored, which goes red showing
returncode=0 and a status written.
Also: 404 is now separated from 403/transport (an unprotected branch is the strongest
form of the finding; `curl -sf` collapses both to an empty string), and the positive
control asserts the decision is `allow` and that the endpoint was actually reached,
rather than the absence of one phrase.
refs #778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#773 §3 Family D is the one class its taxonomy had no bucket for: a check and the
action it authorizes are separated in time over state that can change in between,
with nothing pinning a version (#536, #622, #632, #706, #707).
The repo had already solved this twice without noticing it was one problem — a
compare-exchange claim in-process (ffmpeg.work-ahead-slot-atomic) and RFC 7232
If-Match across /api/v1 (concurrency.ifmatch-rfc7232) — and then solved it a third
time from scratch for the tooling at #706/#707. Hence a class-level record rather
than a fourth per-instance one.
What the enumeration actually found, which none of the five records predicted:
the merge-consent hook's scheduled-auto-merge path is safe only because
`review-verdict/h10` is a REQUIRED status check on main. That is branch-protection
CONFIG, it lives outside this repo, nothing compared the two, and the hook asserted
it in a comment AND in the grant reason a human reads. Switch the context off and
every word of that sentence is false while the hook keeps printing it and keeps
auto-granting. The hook now reads the branch protection and treats it as three
outcomes: present proceeds, unreadable asks, absent denies.
Two defects were caught by the new checks themselves rather than by review:
- the population test found .gitea/workflows/dependency-scan.yml absent from the
first draft of the inventory (a sixth workflow the recon slice never listed);
- self-review found the guard denying with a confident wrong reason when jq errors
one level down on a malformed contexts member, so the word is now matched
exhaustively rather than compared against "yes". Same swallow that survived the
first fix in the #632 base-change guard.
Detector D has no plausible linter, so the detector is detector A applied to an
enumerated inventory: docs/remote-state-inventory.md classifies every in-scope
executable, and scripts/tests/test_remote_state_inventory.py derives the population
from the filesystem and asserts set equality both ways.
Deferred with reasons stated in the inventory: select-queue.sh (advisory, authorizes
no write), ci-detect-already-validated.sh (skip elides re-validation only, the image
still builds), review-verdict.yml's status POST (Gitea offers no conditional write;
already fenced by #706's retarget counter).
Mutation proofs witnessed for both new guards, clause-level, not whole-file.
fixes#778
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mechanises the defect that took #776 and #793 six review rounds each: a fix's test
written to confirm the fix, not to discriminate against its absence.
testing.guard-ships-with-mutation-proof generalised from guards to fixes.
prove-fix.sh runs the selector at the commit (control, must be GREEN) and again in a
separate fresh worktree with the non-test files reverted (must be RED = pytest exit 1
exactly; 2/3/4/5/143 are refused, and --continue-on-collection-errors keeps add-a-file
fixes provable). pytest's status comes from a marker written only after it returns,
because ( cd X && pytest ); rc=$? returns the SUBSHELL's status. Opt-in by a Proves:
trailer; CI checks every commit that carries one and says out loud when a PR has none.
THE TOOL REJECTED ITS OWN AUTHOR. Three commits on the branch claimed
Proves: scripts/tests/test_prove_fix.py; the job returned UNPROVEN for all three,
because reverting the script restored a working earlier version the suite also passed.
Two had been "verified" against hand-written mutants that did not match the code that
actually shipped. The tests were rewritten until both go RED against 587edbecc — whose
script emits "red without it (pytest exit 2)", a witnessed false PROVEN.
This branch deliberately carries no Proves: trailer: the only one that would pass does
so because reverting deletes prove-fix.sh, an add-file smoke check rather than a proof
of its logic. The logic proof is a clause-level mutation that re-runs the unchanged
refusal test against a mutant and witnesses it red (graded MUTATION).
fixes#794
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
The §5.3 table was hand-assembled; deriving the population from config surfaces
seven enabled plugins it omits, including serena. The corpus was undercounted too
(964 transcripts via rglob, not 811 — a top-level glob sees 209 and manufactures
false zeros; positive control 24,762 Bash).
Two corrections: mempalace is not dead (31 calls, last seen 2026-08-14 — the gap
was a snapshot artifact), and codex is the third-heaviest tool in the corpus at
113 `codex exec` calls across 17 sessions.
The issue's framing does not survive: "retire what is enabled and never invoked"
reads a zero as uselessness, but these zeros split four ways — broken (#777),
unreachable (serena, #799), just enabled, and measured on the wrong surface.
Establishing WHY a counter is zero is a precondition for acting on it.
The one supported removal was gitea's PROJECT copy, not the "more specific" one:
server-management and homelab-docs have no .mcp.json and depend on user scope.
The dated 2026-08-13 table is kept, with the re-measurement stated against it.
refs #781
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Both C#/TS language servers and the csharp-lsp MCP server were dead; all three are
fixed and each demonstrated with a real find-all-references call in this repo.
Root causes were one shape — a config naming a path this machine does not have,
with nothing checking. None returned a wrong answer; each refused to start:
- csharp-ls: MSBuildLocator needs a dotnet root owning host/fxr; Homebrew's bin
has none, libexec does.
- typescript-language-server: the LSP workspace root is the repo root but
`typescript` lives in web/node_modules, and the plugin cannot pass a tsserver
path (v5 dropped --tsserver-path; lspServers cannot set initializationOptions).
- the csharp-lsp MCP server: .mcp.json named a dotnet install that no longer
existed, while ~/.codex/config.toml's copy of the same server had been migrated.
Both files are gitignored, so nothing could compare them.
Corrects defect-shapes-773.md §5.1: the "workflow agents must use csharp-lsp" note
names the MCP server's tools, which subagents DO reach — it was dead because the
server could not start, not because agents cannot call it. The LSP tool is the one
no subagent has been observed to resolve.
Six cold review rounds. Five false greens were found in this PR's own verification
code, each introduced by the fix for the previous one — extracted as #796.
fixes#777
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
Final review round. One blocker, and it is the table committing the failure the table exists
to prevent.
The withdrawn parity test asserted two DISJOINTNESS properties — no read-side word in both
`POS_RE` and `NEG_RE`, no write-side word in both `case` arms. The enumeration listed rows
called "read-side polarity" and "write-side polarity" and pointed them at the two guards
added in the rescue. But polarity is not disjointness, so those rows described the
REPLACEMENTS while quietly dropping the originals from the ledger. Enumerating what a
removal cost is the whole job of that table, and relabelling a lost invariant as a narrower
surviving one is precisely how the previous two removals lost something.
Both are now listed as LOST, and the two added guards moved to a separate table that says
what they actually pin. The gap is stated with its demonstration rather than asserted:
`MERGEABLE` in BOTH write-side arms leaves every polarity assertion green, because the
success arm wins — the withdrawn test failed that mutation. What the added guards DO catch
is the dangerous direction, a token meant as BLOCKED reading or posting as approval, which
writes a green `review-verdict/h10`.
Documentation only; no code changed. Review confirmed everything else clean: both new tests
load-bearing (BLOCKED added to the success arm, and LGTM moved to failure, each reddens),
fixture usage correct, ten cases collecting with no skips or collisions, and both names and
docstrings accurately disclaiming disjointness and parity.
584 script-tests pass, decisions-validate OK, inventory parses to 47 rows unchanged.
(--no-verify: pre-commit hook exceeds the tool timeout; its checks were run explicitly.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cold review of the rescue returned BLOCKED on two, both fair.
THE SUBSTANTIVE ONE: the deleted parity test checked disjointness on BOTH scripts; the
rescue covered only the read side. Review demonstrated the gap rather than asserting it —
adding `BLOCKED` to post-review-verdict.sh's SUCCESS arm produced an overlap the deleted
test caught and the rescue did not, because the rescue never executes that script. That was
a real, undisclosed loss, and it is the second time in two commits that removing something
dropped an invariant nobody enumerated. So:
test_post_review_verdict.py::test_each_verdict_word_posts_its_established_polarity
`case` takes the FIRST matching arm, so a token in both arms is not ambiguous — it resolves
to whichever comes first, exactly as `is_pos` wins on the read side. Same consequence, and
it is the one that matters: a word a reviewer means as BLOCKED posting `success` writes a
GREEN `review-verdict/h10`, the required context branch protection honours. Mutation-proved
with the exact case review cited: `BLOCKED` in the success arm -> the test names it and
reddens.
THE NAMING ONE, and it is the mistake I keep repeating: the read-side test called itself a
disjointness test and its docstring said "no word may be in both vocabularies", while it
pins the observable classification of five hardcoded tokens. For a UNIVERSAL property an
omitted token is not a vacuous pass, it is precisely the untested member — the record's own
warning. Renamed to test_each_verdict_word_retains_its_established_polarity and the
docstring now scopes itself to the five words. Both surviving tests are polarity
regressions, not disjointness and not parity.
The inventory now enumerates all seven invariants the withdrawn file asserted and says where
each went — five retired to #788, two rescued as per-script polarity. Enumerating on removal
is `process.enumerate-workaround-behaviors-before-deleting`, which this branch has now
failed twice and should stop failing.
584 script-tests pass, pyright clean, decisions-validate OK. ruff reports one S105 in
test_post_review_verdict.py:103 — PRE-EXISTING and a known false positive on a test stub
(identical on origin/main, my additions start at line 335); it is #780's territory.
(--no-verify: pre-commit hook exceeds the tool timeout; its checks were run explicitly.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cold review of the deletion caught what the deletion itself missed: the withdrawn parity
test carried a SECOND, separable invariant. `test_no_word_is_both_positive_and_negative`
had nothing to do with parsing shell — it prevented a verdict token belonging to both
vocabularies, which matters because `check-review-verdict.sh` sets `is_pos` and `is_neg`
from two INDEPENDENT `grep -iqE` calls. Deleting the file took it along, undisclosed. That
is `process.enumerate-workaround-behaviors-before-deleting`, and I did not enumerate.
Rescued BEHAVIOURALLY, which is why it survives where its parent could not: it EXECUTES the
real classifier rather than reading its source, so no shell construction can fool it. The
word list is a literal, and per `testing.guard-derives-population-from-source` that is
legitimate here — the property is PER-MEMBER ("each of these classifies as exactly one
thing"), not a completeness claim, so a word missing from the list is untested rather than
concealed. It is explicitly NOT a claim that these are the only words the scripts accept;
proving that still needs #788.
THE MUTATION RUN CORRECTED THE DOCSTRING, which had been written first — the wrong order,
and the third time this session that running a mutation contradicted something already
asserted. Adding `mergeable` to NEG_RE left the test GREEN. Reading
check-review-verdict.sh:212 explains it: `if [ "$is_pos" = 1 ]; then head_pos=1; else
head_neg=1; fi` means `is_pos` wins, so that edit has NO observable effect — NEG_RE is
shadowed by POS_RE for any overlapping word. The real direction is the reverse: adding
`blocked` to POS_RE makes `BLOCKED` classify `positive`, and the test goes red naming it.
Both mutations are now recorded in the docstring as measured, with which one is caught and
why the other has nothing to catch.
Also closed from the same review: issue #788's BODY still described the parity test as the
live interim measure with an unticked "delete it" box, while only a later comment recorded
the withdrawal. The body now carries a status banner, strikes the superseded line and ticks
the box — fixed on the issue, since a stale first bookkeeping surface is the same defect
class this branch fixed in post-review-verdict.sh.
ruff clean, pyright clean, decisions-validate OK, 579 script-tests pass.
(--no-verify: the pre-commit hook exceeds the tool timeout; its checks were run explicitly.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round six returned BLOCKED on the same file again: a column-zero `esac` inside a string
truncates the scoped match and silently drops a real arm, and a heredoc inside the block
still false-reds. Both correct. Both the sixth distinct shell construction found in six
rounds.
That is no longer a sequence of bugs, it is a result. A regex over shell source is not a
shell parser and cannot be made into one, and each round's fix was locally right while the
sequence converged on nothing. The file's own docstring told the next session not to get on
this treadmill; the honest reading is that it should not have been built.
DELETED rather than patched again. The reasoning is this change's own thesis, applied to
itself: `testing.guard-derives-population-from-source` says the answer to a missing
authoritative source is to CREATE one, never to approximate it with a predicate over text —
and detector C says two copies of one rule get deduped, not compared. The right fix was
available from the start and is #788. What I built instead was the weak detector the record
warns against, and six rounds of a reviewer falsifying its prose is the empirical proof.
A guard whose accompanying prose can be falsified every round is worse than no guard,
because by this record's own argument a guard described as sound stops being re-examined.
WHAT IS LOST, stated plainly: the duplication is real and is now UNMITIGATED. The two
vocabularies in post-review-verdict.sh and check-review-verdict.sh can drift, and only a
comment says they must not. That comment now says so explicitly, names #788 as the fix, and
no longer claims a test is holding them together.
WHAT IS KEPT: the finding itself (the duplication, the stale breadcrumb pointing at the
merge-consent hook that carries no copy), the corrected comment, #788, and a new section in
the #774 record recording this as the worked example of "a weak detector is itself the
symptom-keyed mistake" — demonstrated rather than argued.
Inventory updated: 31 guards / 4 tooling / 12 proof, 4 MUTATION / 6 BEHAVIOUR-ONLY / 21 NONE.
The withdrawal is recorded IN the inventory, since that is where a future session will look
for the guard and find it missing. Its count-parity guard verified the new numbers itself.
decisions-validate OK, 574 script-tests pass (six removed with the file).
(--no-verify: the pre-commit hook exceeds the tool timeout; its checks were run explicitly.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round five. The confirmation review closed Q1 (the unquoted-value escape is gone) and found
two things left: the file still described every loose match as a case ARM, and
`_ANY_CASE_ARM` could false-RED on a `<word>) state=` inside a heredoc, a compact comment or
an unrelated case statement.
Both had one cause, and it was not the regex. Both patterns read the WHOLE FILE when their
subject is a single `case` block. No amount of widening or narrowing fixes a wrong input —
that is the treadmill this file's own docstring told the next session not to get on, and
round five would have been the first step of it.
The scan now reads only between `case "$verdict" in` and its `esac`. That removes the entire
false-positive class at once, and it makes the "every case arm" language TRUE rather than
nearly true — the overclaim and the false red were the same defect described from two sides.
If the block cannot be located the helper REFUSES: falling back to the whole file would
silently restore the false reds, and returning empty would make every assertion vacuous.
Also from the same review: comparison is now a MULTISET rather than a set, so two arms
sharing a label cannot let an unparsed occurrence hide behind a parsed twin — the same rule
as testing.enumerating-guard-identity-not-position. And the failure message no longer offers
two causes that scoping has since eliminated.
Proven both directions: a heredoc containing `SHIP-IT) state=success ;;` leaves the suite
green; the same line inside the case block reddens it.
The limits list is updated, and records the removed class deliberately — it shows which
fixes are worth making. What remains are same-line-shape misses, which really are regex-bound;
the false-positive family was an input-scope bug wearing a regex costume.
ruff clean, pyright clean, 580 script-tests pass.
(Committed with --no-verify: the pre-commit hook exceeded the tool timeout on the previous
commit; decisions-validate and the full suite were run explicitly above.)
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-audit of the round-four fix, before its confirmation review returned. Making
`_ANY_CASE_ARM` permissive by construction closed the miss and opened the opposite failure:
`#FOO) state=bar` — a comment with no space after the hash — satisfies the loose pattern and
not the strict one, so it would be reported as an unparsed case arm on a completely correct
tree.
That direction matters as much as the miss did. A guard that reddens a correct tree gets
deleted, and then catches nothing at all — which costs more than the construction the
widening was for. Comments are now stripped before both scans, the same treatment the hook
wiring check already needed for the same reason.
Narrow: `# FOO) state=bar` with a space never matched, and the real file contains no such
line today. Fixed anyway, because "narrow" is how each of the previous four rounds started.
Proven three ways: a comment mentioning a hypothetical arm leaves the suite green; a real
unquoted `SHIP-IT) state=success ;;` arm still reddens; clean tree green.
ruff clean, pyright clean, 580 script-tests pass.
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round four, one Medium, and it lands on the defence rather than the code: the change argued
that its residue was acceptable BECAUSE it was accurately disclosed, and the disclosure was
wrong within one round.
`SHIP-IT) state=success ;;` is valid shell selecting `success`. Both extractors required the
double-quoted spelling `state="success"`, so the LOOSE one missed it too — `unparsed` stayed
empty, the vocabularies stayed equal, everything stayed green. A completeness check that
shares its subject's blind spot is not a completeness check.
The bug was structural, not about quoting. A loose counterpart must be permissive BY
CONSTRUCTION; mine was merely a little wider than the strict pattern, so the one thing it
could not see was the one thing it existed to find. It now matches `state=` with any value
form and lets the strict pattern's failures surface as a difference. Proven on three arms —
unquoted, single-quoted, and a differently-named double-quoted one — each red, clean tree
green.
The disclosure is corrected too, and this is the part worth keeping. It said "KNOWN LIMITS,
ENUMERATED", which reads as exhaustive and was false one round later. It now says the list
is NOT exhaustive, records that this very entry was the one it missed, and ends with
"whatever the next round finds. Assume this list is one short." Four rounds have each
produced another construction; claiming completeness over a regex on shell source is the
overclaim the whole change argues against.
Test renamed to test_the_strict_extractor_consumed_EVERY_case_arm_THE_LOOSE_ONE_FOUND, since
the old name asserted more than the code could deliver — and the inventory guard immediately
went red on the now-stale proof ref, which is exactly the drift it was built to catch,
catching its own author one commit after being written.
ruff clean, pyright clean, decisions-validate OK, 580 script-tests pass.
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third review round. Of the eight findings from round two, five were closed; this addresses
what remained, and the split between "fixed" and "stated" is deliberate.
FIXED — the record could not adjudicate. Its frontmatter `rule` required disarming the
guard's clause; the body added an input-mutation standard for guards that ARE tests. Two
incompatible criteria in one active record means one reviewer rejects the self-referencing
MUTATION rows on the frontmatter and another accepts them on the body. The exception is now
IN the rule with its limits: admissible only for checker-guards, only when executed and
witnessed, never a licence to grade a script-guard MUTATION for having a bad-input test,
and a file-level grade covers the clause its cited case mutates rather than every assertion
that later lands in the file.
FIXED — a matrix-templated image bypassed the cross-workflow check. `_PIN.match` requires a
literal tag, so `image: <repo>:${{ matrix.tag }}` in another workflow ran on the toolchain
image while the check reported none. Now keyed on the image REPOSITORY, so a templated tag
is reported rather than skipped — it is a fault in its own right, since nothing could then
verify which image ran. Proven: a probe workflow with exactly that construction is caught,
removed, green.
STATED, NOT PATCHED — the remaining three findings are all one shape: a regex over shell
source cannot be made complete. Each round found another construction (a case arm whose
first command is not the assignment, an indented reassignment, a basename inside `: #
... disabled`), and a fourth round would find a fifth. This repo has already paid three
rounds for exactly this class at #629, #633 and #698. So the limits are now enumerated in
the files themselves rather than left for the next reader:
- the parity extractors list the three constructions that escape them, say what they DO
catch (the realistic same-style edit on one side only), and say plainly that this is
not a proof of semantic equality between two shell programs;
- the inventory records that hook wiring is a substring test for the basename, so it
catches deletion but not deliberate disablement.
Both name the issue that removes the underlying duplication (#788), and the parity file
tells the next session NOT to invest another widening round.
Also outstanding and tracked, not silently dropped: PROOF/GUARD roles and MUTATION grades
are per FILE, so a standalone invariant added to a PROOF file inherits its classification
and a self-referencing grade does not cover clauses added later. That is clause-level
inventory, which is #790.
ruff clean, pyright clean, decisions-validate OK, 580 script-tests pass.
Refs #774
Refs #775
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-audit before re-review, and it found one. `wired_hook_files()` was added to stop hook
EXISTENCE standing in for hook WIRING — but it substring-matched the filename against the
whole husky text, and `.husky/pre-commit:7` reads
# CI where a base ref exists). Fail-open shim — see .claude/hooks/decisions-guard.sh.
one line above the real invocation. Delete line 8, keep line 7, and the hook still reads as
wired. That is mention-for-invocation, which is the exact substitution the function exists
to prevent, one line inside the fix for it. Comment lines are now stripped from the husky
hooks first; settings.json needs no stripping because JSON has no comments.
Proven both ways: with the invocation removed and the comment left, the guard names
decisions-guard.sh as unwired; clean tree stays green.
Also verified rather than assumed, since a fix round is where adjacent defects live:
- a stale SELF-referencing proof ref is still caught (the self-reference escape hatch
skips only the PROOF-row classification check, not the def-existence check);
- a reworded summary is LOUD, not vacuous — an unparsed summary fails with a message
saying so, rather than silently checking nothing.
ruff clean, pyright clean (0 errors) on the three new files. Deliberately NOT ruff-format-ed:
the pre-existing scripts/tests corpus is not formatted either, so reformatting only these
three would diverge them from every sibling and bake in a format derived from an
un-versioned config on one machine — which is the divergence #780 exists to settle.
580 script-tests pass.
Refs #774
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two independent cold reviews (Codex GPT-5.6 cross-family; Fable 5 on the patch) both
returned BLOCKED. They agreed on the counts error and the extractor hole; each found
things the other did not. Fixes, with what each was:
THE INVENTORY DID NOT COVER ITS OWN NEW GUARDS. `_SCRIPT_REF` matched `scripts/name.py`
but not `scripts/tests/*.py`, so the three guard files this change introduced had no rows
and the completeness check stayed green. A completeness guard blind to its author's new
guards is precisely the defect being legislated against. The population now globs
`scripts/tests/test_*.py` — which is how they actually run, since pr-checks.yml invokes
the directory. 32 rows -> 48.
That forced a third Kind. Once test files are in the population, every mutation proof
becomes a row wanting a proof of its own, forever. `PROOF` marks a file whose job is to
prove another guard; a scripts/tests file enforcing a repo invariant with no separate
guard behind it stays GUARD and may cite a mutation case in its own file.
HOOK EXISTENCE WAS STANDING IN FOR HOOK WIRING. Deleting a hook's registration from
.claude/settings.json left the population and the table unchanged, so the row went on
describing a guard that no longer ran — #631's shape one level down. Now derived from
settings.json plus the husky hooks.
THE SUMMARY COUNTS WERE A HAND-KEPT MIRROR AND WERE WRONG ON ARRIVAL: "28 guards, 4
tooling ... 19 have none" against a table holding 27/5/6/3/18. Both reviewers found it
independently. The prose is now parsed and asserted against the table.
TWO FALSE MUTATION GRADES, each with a concrete disarm:
- test_full_first_page_alone_does_not_end_enumeration sends 50 docs paths then one more
docs path; disarm pagination to treat a full page as final and it is still all-docs,
still exempt, still green. Re-pointed at test_protected_path_on_a_LATER_page_is_still_seen,
which does go red under that mutation.
- test_the_scan_job_runs_the_out_of_pytest_positive_control asserts only that the script
exists, is executable, is referenced and is marked; replace its logic with `exit 0` and
all four pass. ci-prove-ban-detects.sh regraded NONE.
The MUTATION column was also being applied as a curve: three rows graded MUTATION fed the
real script an input only that clause rejects, which is what the rows eight lines away are
graded BEHAVIOUR-ONLY for. Definition sharpened to *witnessed* rather than plausible, and
those regraded. 5 MUTATION / 6 BEHAVIOUR-ONLY / 21 NONE across 32 guards.
THE VOCABULARY EXTRACTOR COULD RETURN A PARTIAL SET. `[A-Z|-]` cannot match `SHIP*)`, so
adding that arm leaves the extracted set non-empty AND equal to the read side — parity
green while the gate desyncs. Emptiness checks cannot see partial degradation. A loose
counterpart now asserts the strict pattern consumed every arm; proven red on exactly that
attack and green on a clean tree. Also: each verdict pattern must be assigned once, since
the extractor unions assignments while the classifier runs the last.
Also: docker-build.yml was itself an unchecked scope mirror (now asserted to be the only
workflow with toolchain container jobs, by parsing container.image rather than grepping —
ci-image.yml names the image because it builds it); the mutant floor is an equality;
e2e-functional.sh reclassified GUARD (it exits 1 on a failed contract assertion);
design-sync-reminder.sh does block the first Stop. The doc now states all six excluded
classes instead of one.
Not done here, filed instead: workflow-owned execution-class metadata to replace
TOOLCHAIN_JOBS, a single shared verdict vocabulary, and an executable clause-level
mutation harness. Each touches a CI-gating or merge-gate path and wants its own review.
580 script-tests pass.
Refs #774
Refs #775
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#773's analysis found that the largest recorded failure family is reasoning about a
representative instead of the population (39% of process-failure records), and that the
most common is a check that never ran at all (25%). Both rules had been reinvented
repeatedly and written down nowhere.
Two decision records:
testing.guard-derives-population-from-source (#774) — a guard enumerates its population
from a machine-readable authoritative source and asserts set equality both ways. States
the boundary that keeps it honest: filtering to select the SUBJECT of a per-member
property is fine; filtering the population before a COMPLETENESS claim is the defect.
Also separates guard SCOPE (a reviewable policy choice) from guard POPULATION (always
derived).
testing.guard-ships-with-mutation-proof (#775) — disarm that clause alone and a named
test must go red. Behaviour-only coverage is graded separately, because it proves the
guard reacts, never that it is connected.
Audit findings fixed:
ci-image-pin stated an invariant it did not check. Its error text says "Every container:
job must pin ersatztv-ci:<7-char-sha>"; what it asserts is that `grep … | sort -u` yields
one DISTINCT value. Distinctness is a property of the pins present, so deleting the
container: block from `test` leaves four pins, one distinct value, and a REQUIRED context
silently running on the bare runner. test_ci_image_pin_population.py adds the population
check, keyed on a reviewed registry cross-checked both ways — set equality between two
DERIVED sets could not see this, because both sides shrink together.
The verdict vocabulary was written down twice with no cross-check — post-review-verdict.sh
(write) and check-review-verdict.sh (read). A word in one and not the other sends the
required status green while the hook still denies. Both vocabularies are now extracted
from their own source and compared as sets; a test that restated the words would just be
a third copy. The write side's comment pointing at pretooluse-merge-consent.sh was also
stale — the hook carries no copy and delegates.
Mechanical enforcement, answered explicitly for both:
No to a filter-shaped-guard lint. The token is not the defect — ToolCatalogTests filters
correctly eight lines from a completeness assertion that must not — and it would be a
string predicate over source, which this repo's record says takes 3+ rounds. Building it
would be #774 violating #774.
Yes to enforcing the bookkeeping. docs/guard-inventory.md classifies all 32 guard files;
test_guard_inventory.py derives the population from the filesystem and call sites,
asserts set equality both ways, and resolves every claimed proof ref to a real def. A new
guard cannot ship unclassified; a renamed test cannot leave a row claiming lost coverage.
What it does NOT check — whether a MUTATION claim is true — is stated, not implied.
Measured: 28 guards, 4 tooling. 6 mutation-proved, 3 behaviour-only, 19 unproven.
Every guard added here was mutation-proved by execution before being believed: neutering
pin_population_faults turned 20 of 25 red; the inventory guard was driven red three ways
(deleted row, new unclassified hook, stale proof ref) and restored green.
573 script-tests pass. Scope limit stated in the doc: inline workflow-job guards are not
in the machine-checked population.
Refs #774
Refs #775
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Analysis over all 349 closed issues (95 carry a `## Closing record`), classified by five
independent raters against a written taxonomy that permitted `NEW:<name>`, with two controls: a
blind inter-rater re-rating (12/15 agreement) and a backward-generalization sample over the 254
pre-convention issues.
Findings that change the picture #773 started from:
- The ranking reverses. Vacuous verification is the most common shape (17/69), not twin-missed
(14/69) — #773's 50-issue sample had it the other way.
- #773's central hypothesis holds and extends: twin-missed, vacuous-by-sampling and symptom-keyed
guards are one error (reasoning about a representative instead of a population), 27/69 (39%),
one detector — already reinvented six times in this repo under six names.
- The shapes predate the closing-record convention (#1, #215, #232, #403, #473), so they are not an
artifact of recent guard-building. That confound was tested and refuted, not assumed away.
- A class the taxonomy missed entirely: check-and-use races over mutable state (#536, #622, #632,
#706, #707).
- Overclaim drops to 4% as a primary cause — a modifier, not a class. Round-churn likewise: 33 of 69
records narrate >=3 review rounds, spread across every family, only 2 in the class named after it.
Part 2, measured rather than assumed: csharp-lsp cannot initialize and typescript-lsp cannot resolve
typescript, the LSP tool has 0 calls across 811 transcripts, ruff/pyright are enforced nowhere
despite the global instruction, no hook scripts are dead — but PreToolUse/PostToolUse execution
leaves no durable trace, so we cannot tell whether our own guards fire.
Names the classes where no mechanical detector is plausible rather than inventing weak ones, and
strikes one proposed tool (shellcheck) after testing showed it does not catch the case it was
proposed for.
Provenance, kept here rather than in the document because a reader never saw the earlier drafts: six
cold review rounds, worktree-isolated. Rounds 2-5 each found a real defect in the text written to fix
the round before — two fabricated quotes, a Family A/C double-count, a miscited precedent (#711
argues FOR the enumeration it was cited as rejecting), a false floor-rounding claim, and a
round-count built by pattern-matching that undercounted by half. Every one landed in new prose, never
in the text under review, which is the document's own thesis operating on its author and the reason
the final pass was whole-file rather than delta-scoped.
Spawns #774-#781 and #784, tracked in the "Defect-shape hardening" milestone.
fixes#773
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
The delimiter ban protecting `build`'s `Smoke + IPTV E2E` was enforced only by a pytest in `script-tests` — `on: pull_request`, not a required context — so nothing re-checked it on a `v*` tag push, which is exactly when the candidate image is published. A `scan` job now runs the ban test and `build` lists it in `needs:`, so a red `scan` skips `build` and no image is built.
Measured both directions without cutting a release: run 1928 (poisoned Smoke) → scan failed, `Build & push` skipped; run 1929 (control) → scan green, build ran.
The gate rests on three different KINDS of check, because each single kind was defeated in review: the ban test; an execution probe against a poisoned copy with all three `env:` tiers layered; and `scripts/ci-prove-ban-detects.sh`, which is not a test — it poisons the real checkout and vouches only for the ban test's `build` parametrisation failing. Eight review rounds; rounds 1-5 each found a real defect in the previous fix.
Refs: #767
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
A live Komodo stack literally named `ersatztv` owns the TEST channel, not prod. `DeployStack ersatztv` succeeds, looks healthy, and promotes nothing — silent and plausible. Extends the existing callout with all three stack names and the resolution rule: identify the prod stack by the container's `com.docker.compose.project.config_files` label, not by stack name.
Container labels re-verified live on jazz 2026-08-11; the verification date is scoped to what was actually re-measured, after review flagged the stamp as covering unchecked values.
The server-management half (the `komodo` skill still uses the dead `media-servers` as its worked deploy target) cannot land in this repo and is tracked as server-management#743. #720's box 2 was re-scoped to that hand-off rather than ticked as though the skill were fixed.
fixes#720
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
A `run:` body the runner declines to interpolate is dropped, and the job still
concludes `success` (#751). #751 fixed that in review-verdict.yml, where the
failure is fail-CLOSED. This closes the two places where it is fail-OPEN:
`Build & test (.NET)` and `EF migration integrity (SQLite + MySql)` are the
other two required contexts on `main`, so a dropped step there sends a required
check green having done no work.
Per-STEP markers, not per-job as proposed: a marker on the first step only
proves the job began, while the drop that costs something is `Test`, `Build` or
a migration replay. The trailing guard carries no `if:` — with a dozen steps,
`always()` would announce a false "these steps never executed" on every ordinary
red build; the default `success()` is correct because guard-skipped implies
job-red. Plus a ban on the raw `${{` opener in `test`, `migrations` and `build`,
which makes the class unreachable rather than merely caught. `build` is included
because its Smoke step runs AFTER the image is pushed.
Measured live on the build lane in both directions: probe #765 (drop caught,
sole failure in the job) and #766 (a failing continue-on-error step does not
skip the guard). 510 tests, 30 mutations killed across two harnesses, five cold
review rounds across two model families.
Residual tracked as #767: the `build` ban is review-time only, not fail-closed
on the release path.
fixes#756
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
The classify step in review-verdict.yml stopped executing on 2026-08-03 and the job
reported success anyway, so the branch-protection-required review-verdict/h10 was posted
by nothing but a human hand for three days and both exemption classes silently died.
Three independent defects, each alone sufficient:
* A ${{ }} sequence in a SHELL COMMENT. The runner scans the whole run: scalar for the
expression opener and rewrites the entire body into one format(...) call; `pr number`
does not parse, so it drops the step and concludes the job green. The prose documenting
a fix disabled the fix.
* The retarget fence never trusted its count: a page past the end of the timeline is JSON
`null`, not `[]`, so rt_ok was never yes for ANY PR and every exemption success was
withheld. Fixing the first alone would not have restored the exemptions.
* The same nil-slice shape on /commits/{sha}/status, which made read_existing_verdict
exit 1 and post nothing.
A nil Go slice serialises to `null`, so every list-shaped field on this API is suspect and
only a per-endpoint measurement settles it — timeline returns bare null, the combined
status returns {"statuses": null}, comments and pulls/{n}/files return [], and
/statuses/{sha} returns []. Four endpoints, three shapes.
The silent green is the actual defect, so a start-marker guard now fails the job when the
classifier did not execute, and two static guards reject the delimiter at review time.
CLAUDE.md and AGENTS.md became PROTECTED paths: they define the H10 rule and were
docs-only-exemptible, reachable again precisely because this restores the exemptions.
Verified by a live scratch-base probe pair with a negative control, 460 tests, and 29
mutations across six rounds. Five cold review rounds, alternating model families; none
found a path to a green review-verdict/h10 on an unreviewed head, and every one found a
defect beside the fix — including that round 2's guard was dead code against a page limit
of 100 on a server that caps at 50.
Deferred: #756 (docker-build's required jobs, where a dropped step is fail-OPEN) and #763
(paging both /statuses/{sha} reads).
fixes#751
Fifth cold review: MERGEABLE, no Blocker, no High. Four Low findings, none behavioural.
Fixing all four rather than accepting them, because two are the exact class this issue
exists to retire: text that reads as a checked reason and is not.
A VACUOUS RATIONALE, on the branch about vacuous rationales. The comment on the history
stub's `page` guard said it sits ahead of the read-counting modes "so the page-2 probe
cannot shift 'raced row appears on read N'", by analogy with the combined endpoint.
Measured: moving that guard AFTER the counter modes reddens NOTHING, because no history
mode that counts reads ever issues a page-2 request — `raced=1` on page 1 short-circuits
the probe. The real reason is the other half: page 2 must terminate for modes that
describe page 1 only, and dropping just that `print("[]")` reddens
`test_a_PRE_EXISTING_human_row_does_NOT_trigger_a_repair`. Comment now says which half is
load-bearing and which was wrong. (The COMBINED endpoint's guard genuinely is
counter-related — moving it reddens three mid-run-race tests.)
THE FALSE REPAIR IS STICKY, and the previous commit undersold it as "a stall a reviewer
can clear". It writes `$REPAIR_DESC`, which the classification refuses to grant an
exemption over and re-writes as a fixed point on every later run — so a spurious repair
removes that head's exemption PERMANENTLY, not for one run, and only a human verdict
clears it. Still the right direction against a forged green over a rejection, but it is a
per-sha loss of the exemption, and that is the argument for real paging (#763) rather
than living with this. Said in the comment now.
CORRECTING THE PREVIOUS COMMIT MESSAGE, which over-generalised: "uncertainty resolves to
a stall … never to leaving green" is true of the page-2 probe and NOT of the enclosing
path. An unreadable page 1, or a non-numeric high-water mark, still leaves the exemption
`success` standing unverified. The workflow's own comments state that correctly; the
message did not.
THIRD ROUND ON ONE REGEX, which is the documented budget for a string-matching predicate.
Assertion C started as `\w+\s*\(\)\s*\{`, gained `function\s+\w+` when review found
`function mk {` slipped it, and STILL missed the union form `function mk() {` — the
natural next spelling once the previous one is caught. Now
`^\s*(function\s+)?\w+\s*(\(\s*\))?\s*\{`, verified against all seven spellings.
THE COMPLETENESS COUNT, restored properly. Relaxing `len(bodies) >= 3` to `assert bodies`
fixed a false red but threw away the only check that the walk reached ALL run-bearing
steps: `max(len) > 5000` proves it reached the classifier and nothing about the short
ones, so a helper that silently stopped yielding them would pass an unscanned delimiter.
Now counted against the job's own step list, read directly rather than through the helper
under test — which catches a helper reading the wrong key or dropping steps, while still
tolerating a step being legitimately added or removed.
Verification: 460 green. Three mutations, each as intended — the union spelling `function
mk() {` (red, previously passed), a walk that drops the short steps (red, the property the
count guard restores), and a legitimate step deletion (PASSES, confirming the false red it
replaced stays fixed). Twenty-nine mutations across six rounds.
Refs: #751
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourth cold review round: no Blocker, no new path to a green `review-verdict/h10` on an
unreviewed head, and it independently re-measured 14 claims in the diff. It also caught
that this branch was about to revert someone else's work, and found the one remaining
place where the nil-slice/clamp lesson had not been applied.
REBASED ONTO 9881d1ff8 (#760), which landed while this was in review. The tell was the
one CLAUDE.md documents: `git diff origin/main HEAD` showed deletions I never made —
`docs/decisions/records/mcp/tool-schema-openapi-parity.md` and edits to `docs/mcp.md`.
Pushing would have reverted them. The generated catalog was regenerated rather than
trusted to the rebase, and verified to carry BOTH records.
THE FAIL-OPEN TWIN, one function further on than the last round reached.
`/statuses/{sha}?limit=100` is read twice — for the high-water mark and for the
post-write race check — and neither pages, while `limit` clamps to 50. So "no raced row
on page 1" does not establish "no race": a human `BLOCKED` landing in the write window
can sit on a page the job never reads, leaving a forged green over a rejection. This is
the ONE path in the design whose failure direction is toward SUCCESS.
Measured rather than argued: a probe head reached 33 rows after ~5 runs against a cap of
50, and the ordering is only coarsely newest-first (`33,32,31,30,28,29,27,…`), so a few
CI reruns reach it and the row's position cannot be relied on — which this workflow's own
comment already disclaimed. That comment ALSO claimed order-independence flatly; false
once the page clamps, so it now says what actually holds and what saves us.
The mitigation is conservative rather than complete: if page 1 shows no race, page 2 is
read, and any rows there — or an unreadable page 2 — count as "assume raced" and repair
to `pending`. Uncertainty resolves to a stall a reviewer can clear, never to leaving
green. Real paging of both reads, including the high-water mark, is #763.
A THIRD empty shape turned up while modelling it: `/statuses/{sha}` past the end returns
`[]`, where `/commits/{sha}/status` returns `{"statuses": null}` and the timeline returns
bare `null`. Three endpoints, three shapes, one server. The code tolerates both here
because guessing per endpoint has now been wrong twice.
MY OWN COVERAGE GAP, found by mutation rather than by reading: inverting the
unreadable-history-page-2 branch reddened NOTHING. Now tested both ways. Same class as
the two untested refuse branches the review flagged, which are also covered now.
FOUR CLAIMS RETIRED, all of the shape this issue is about — text that reads as checked
and is not:
* "18 tests fail" for the corrected-double mutation is 21 now, because rounds 3-4 added
three fence-dependent tests. Broke a number while documenting broken numbers. Both
citations now give the range and lead with the invariant.
* "measured: 4, 2, 9, 5, 10" first-page timeline events are 7, 5, 9, 6, 10 today.
Timelines grow; the figures are gone and the invariant stated instead — a PR is created
by a push, and a push is an event, so page 1 is never empty.
* The strict test's docstring said "RAW TEXT" while the test reads parsed `run:` scalars,
with a dead `raw =` assignment left behind (a new ruff F841).
* `len(bodies) >= 3` had zero slack: deleting the optional jq-preflight step reddened it
with a message asserting the classifier had not been examined, which was untrue. The
length assertion already carries the property, so the count only needs to be non-empty.
Also: dead `_workflow_expression_fields_text` removed; assertion C's regex now catches
`function foo {` as well as `foo() {`; the page-2 refusal says a human verdict clears it.
Verification: 460 green. Five more mutations as intended — deleting the history page-2
check (red), accepting an unreadable one (red, after the coverage gap was closed),
accepting a garbage page 2 in read_existing_verdict (red), hiding the marker write behind
`function mk {` (red), and the earlier twenty-one still hold. Live re-probe on this body
follows.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third review round, cut short by a transport hang after ~11h, but it had already found
the thing that mattered: the guard added last round could never fire.
`read_existing_verdict` asks for `limit=100` and refused when the page came back with
100 rows. This instance caps `limit` at the server-wide `MAX_RESPONSE_ITEMS`, MEASURED
AT 50 — `/issues?limit=100` returns 50 items. A response can therefore never carry 100
rows, so the comparison was unreachable and the hole it was written for was still open.
The sting is that the repo already knew. `scripts/pr-changed-files.sh`, two test files
and `ci.script-tests-job` all document that Gitea caps `limit` at `MAX_RESPONSE_ITEMS`
(50 in the PR #619 measurement). The review found it by grepping this codebase, not
upstream. Writing a guard against a constant the repo had already measured as wrong is
the same failure as the unfaithful test double two rounds ago: a number believed rather
than checked.
So this is now the THIRD guard for one hole, and the first two were both no-ops:
1. `.statuses | length` vs `.total_count` — `total_count` is the count for the PAGE
RETURNED, not the commit (`?limit=1` on a 6-context head gives
`len=1, total_count=1`). Equal by construction.
2. "refuse when the page is full at 100" — dead code, as above.
3. Ask the server. Completeness is needed ONLY to justify "no verdict exists on this
head", so when the row is absent from page 1 the job reads PAGE 2, and refuses if
it carries anything. Cap-independent: no reconfiguration re-breaks it, and nothing
is hardcoded that a measurement could contradict.
Measured to make sure page 2 is real rather than assumed: `?limit=3&page=2` on
3aed43c6 returns three further rows, and `page=9` returns the same `statuses: null`
terminator the timeline uses.
The probe is skipped when the row IS on page 1, because there is nothing to learn — the
combined endpoint returns the latest status per CONTEXT, so a context cannot recur on a
later page.
TEST-DOUBLE FIDELITY, again the fiddly part. The stub now honours `page`, and that guard
had to go BEFORE the read-counting modes: `appears-on-read:N` counts how many times the
job has LOOKED at the status, and the completeness probe is part of the same look, not a
further one. Letting it increment those counters shifted "the verdict appears on read N"
by one and broke three mid-run-race tests — a false red that would have been easy to
"fix" by adjusting the expected counts, which would have quietly destroyed what those
three tests measure.
Verification: 455 green. Four more mutations, each as intended — deleting the probe
(red), accepting a non-empty page 2 (red), refusing even on an EMPTY page 2 (red, the
deadlock control), and reverting the twin `statuses: null` gate (48 red). Twenty-one
mutations across four rounds.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cross-family re-review of the previous fix commit. It did NOT pass, and it was right
not to: the round that fixed the reviewers' findings introduced two of its own, both in
the tests written to close them. That is this file's recurring shape, and it is the
reason the fix commit gets re-reviewed rather than the initial diff only.
TRUNCATION (High). `read_existing_verdict` asks for 100 statuses and never checked
whether the page was full. If a head ever carried more contexts than that, an existing
`review-verdict/h10` could fall off page 1, the job would conclude no verdict exists,
and it could post an exemption `success` over a human `failure` — the worst thing this
gate can do. Six contexts exist today, so this guards a future shape, not a live bug.
BUT THE PROPOSED GUARD WAS A NO-OP, and measuring is what showed it. The review asked
for `.statuses | length` compared against `.total_count`. On this instance `total_count`
is the count for the PAGE RETURNED, not for the commit: on 3aed43c6 (6 contexts),
`?limit=1` gives `len=1, total_count=1` and `?limit=3` gives `len=3, total_count=3`.
The two are equal by construction, so that check would have read as a completeness
proof while proving nothing — and it would have been the second guard in this file to
look like a check and not be one. What IS observable is a page at the requested limit,
which means "maybe more", so that is now treated as unreadable: post nothing, leave the
required check absent. The stub mirrors the per-page `total_count` deliberately, so the
new test cannot pass for the wrong reason either.
THE TESTS THAT CLOSED THE LAST ROUND'S FINDINGS:
* The behavioural guard test — added to answer "a bare `exit 1` substring is satisfiable
by dead code" — extracted the two marker lines BY TEXT and ran them alone. That
passes even if the write is moved into a function nobody calls: the extractor finds
the text, runs it at top level, the marker appears, and the test reports the guard
proven while production writes no marker. It now executes the classify body's real
PREFIX down to and including the write, which reproduces the production control flow
instead of a reconstruction of it. Mutation: move the write into an uncalled function
-> RED (it previously passed).
* The anti-vacuity check — added to replace an over-broad assertion — hand-counted
`run:` keys with a regex that only matched an indented `run:` starting `|` or `>`. It
false-redded legal spellings (`- run: |`, a single-line `run: echo ok`) and could
count a `run: |` sitting inside a heredoc. Hand-parsing YAML to validate a YAML parse
is the wrong shape: it adds a second, worse parser whose every disagreement is a
false alarm, and a red here blocks all merges. Now asserted on CONTENT — the walk
reached >=3 bodies and one over 5000 chars.
FALSE RED, THIRD INSTANCE IN THIS FILE. The repo-wide expression test scanned raw file
text, so a delimiter in an inert top-level YAML comment redded the repo even though the
runner never evaluates it. It now scans PARSED scalars: PyYAML drops YAML comments,
while a `run:` body is itself a scalar and keeps its SHELL comments — which is exactly
the distinction that matters, since inside a `run:` scalar a comment is not inert.
Verified in both directions: an inert top-level comment passes, the same payload in a
run-body comment still reds.
Also: the `total_count` zero check now requires the JSON TYPE to be number — `jq -r`
renders `0` and `"0"` identically, so a text compare accepted a corrupted
`"total_count": "0"` as "no statuses".
Verification: 455 green. Six further mutations, each landing as intended — the uncalled
function (red), an inert YAML comment (PASSES, no false red), the same payload in a
run-body comment (red), accepting a full status page (red), comparing total_count as
text (red), and breaking the YAML walk's job key (red). Seventeen mutations across the
three rounds.
Re-probe of both live controls follows on this body; the previous probe evidence was
taken before this commit and no longer describes what would merge.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two independent cold reviews (a cross-family GPT-5.6 pass and an isolated Opus pass).
Neither found a path to a green `review-verdict/h10` on an unreviewed head. Both found
real defects BESIDE the fix, which is the failure mode this file keeps producing.
THE TWIN, and the reason not to trust "I fixed the two I could see". `GET
/commits/{sha}/status` returns `statuses: null` — not `[]` — for a head with no
statuses yet: `{"state":"pending","total_count":0,"statuses":null}`, measured on PR
#739's head. `read_existing_verdict` gated on `.statuses | type == "array"` and took
its `exit 1` path, posting NOTHING. Fail-closed, but the user-visible outcome is the
one this issue is about: an exempt PR with no status and, since #743, no bypass. Its
double printed `{"statuses": []}` at all three no-verdict sites, so that branch was
unreachable in the suite — the same unfaithful-double story as the timeline, one
function over. `null` is accepted only when `total_count` is 0, so a body that merely
lost its array is still refused and an existing verdict is still protected. Swept
`scripts/pr-changed-files.sh` too: `pulls/{n}/files` returns `[]`, unaffected. The
generalisable rule is that a nil Go slice serialises to `null`, so every list-shaped
field on this API is suspect and only a per-endpoint measurement settles it.
A GOVERNANCE SELF-EXEMPTION, reachable again precisely because this change works.
`DOCS_ONLY` matched `CLAUDE.md` and `AGENTS.md` — the documents that DEFINE the
completion protocol, the merge-consent convention and the H10 rule. Driving the real
classify body with a lone `CLAUDE.md` change produced `review-verdict/h10=success`.
Protecting `.claude/` while the file specifying what it enforces stayed exemptible is
the same self-exemption the header rules out, one directory over. Both added to
PROTECTED; `README.md` deliberately not (ordinary prose, no enforcement).
FOUR OVER-CLAIMS, corrected rather than defended:
* The repo-wide expression test does NOT catch "any payload that cannot evaluate".
It checks the HEAD TOKEN of each dotted path. `${{ github.ref == }}` and
`${{ …head.sha + }}` pass; so does a renamed output, since tokens after the first
are skipped by design. Claim corrected in the docstring, `docs/ci-cd.md` and the
record. The test is kept permissive on purpose: a red here blocks every merge.
* The strict test's anti-vacuity half banned expressions ANYWHERE outside
`with:`/`env:`, so the standard `if: ${{ always() }}` spelling and even a delimiter
in an inert top-level comment went red — a guard more dangerous than its target.
Replaced with the honest property: the YAML walk saw every `run:` body it declares.
* The `if:` assertion demanded the bare `always()` exactly; now normalised, since the
wrapped form is identical to the runner.
* `exit 1` was matched anywhere in the guard body, so an unreachable
`if false; then exit 1; fi` satisfied it while the real branch said `exit 0`. Now
required INSIDE the missing-marker branch — and the new behavioural test settles it
properly by EXECUTING the guard body both ways.
* The record asserted a repo-wide obligation to guard consequential steps. It is not
repo-wide: `docker-build.yml`'s `test`/`migrations` are also required contexts and a
dropped step there is fail-OPEN (green having done no work), strictly worse than
here. Scoped to this file and tracked as #756 rather than asserted as done.
Also: comments in both files still said it was unestablished whether a later step runs
after a drop — runs 1863/1866 established it, so they now record the measurement; a
cited test name that never existed; `kind` leaked to global scope; a mangled comment
wrap; and an already-false "one event on page 1".
Hardening of my own: `null` now counts as exhaustion only from page 2 ON. Every real
PR's first page carries events (4, 2, 9, 5, 10 across #752/#753/#749/#739/#717), so a
terminator on page 1 means no page was ever read, and certifying "no retarget" from a
response we cannot explain is the one thing the fence exists to refuse. Narrows rather
than closes it: a wrong `null` on page 3 still reads as exhaustion.
Verification: 137 in this file / 452 total green; ELEVEN mutations each red —
reintroducing the defect, deleting the guard, deleting the marker write, removing
`if: always()`, `exit 1`→`exit 0`, a delimiter in the guard body, the fence gate (20
red), the TWIN gate (70 red), dropping the governance paths, accepting a null first
page, and diverging the marker path between the two steps.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scratch-base probe found a SECOND, independent reason `review-verdict/h10` was
never posted automatically. Fixing the dropped step alone would NOT have restored the
exemptions.
`count_retargets` pages `/issues/{n}/timeline` and trusts its count only on a
validated empty page, gated on `type == "array"`. But a page past the end of that
endpoint is the JSON value `null` — measured at Gitea 1.27.1 on PR #752, four bytes —
so the real terminator read as UNREADABLE. The walk never reached a validated empty
page, `rt_ok` was never `yes` for ANY pull request, and the fence therefore withheld
EVERY exemption `success`. Renovate and docs-only PRs got no status at all: the same
user-visible outcome as the dropped step, by a completely unrelated route.
The instance is not consistent between endpoints — `/issues/{n}/comments` returns `[]`
when empty — so both shapes terminate the walk now, and the regression test is
parameterised over both. The type is read as a VALUE (`case` over `jq -r 'type'`)
rather than through `jq -e`, whose exit-status semantics already bit this workflow at
jq 1.6 (#647).
TWO REASONS THIS LOOKED DELIBERATE RATHER THAN BROKEN, both worth generalising:
* It had never run. This fence shipped in 8f6d4f443 — the same commit whose prose
comment stopped the classify step executing at all. Merging a guard and first
executing it are different events, and only the second tells you anything.
* The test double asserted the wrong shape while claiming to be measured. Its comment
read "Real shapes, measured on this instance and deliberately mirrored" and it
printed `[]` past the end, so the `array`-only gate was never exercised by the suite
either. Correcting the double and restoring the old gate turns 18 TESTS RED — every
one of them had been green for the wrong reason. A fidelity claim in a double is an
assertion and it decays like any other.
The new test asserts the POSTED STATUS, not the log: on the real probe run the log
said `Decision: state=success` and the job still posted nothing, so the decision and
the write are separate events and only the write is what a merge reads.
Also here, both found while editing this code:
* `ci.verdict-write-retarget-fence` stated this as a narrow residual ("a timeline over
the 20-page cap can never be exempted") when the behaviour was universal. Corrected
in place rather than left as a checked-looking claim that talks the next reader out
of verifying.
* The workflow cited `ci.paged-endpoint-completeness`, a key that has never existed as
a record anywhere. Repointed at the record that actually owns this walk.
Marker hardening from the probe: `RUNNER_TEMP` is `/tmp` on this runner, not a private
per-job directory, so the start marker is now keyed on the run id and attempt. The
lane starts a container per job today, which makes a fixed name fresh in practice, but
that is a property of the lane and a stale marker would make the guard PASS on a run
whose step was dropped — the exact silent pass it exists to remove.
Verification: 446 passed; M7 (revert only the type gate, keep the corrected double) →
18 red. Probe evidence in the issue.
Refs: #751
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`review-verdict.yml`'s classify step stopped executing on 2026-08-03 and the job
reported `success` anyway, so `review-verdict/h10` — the branch-protection-required
status — was posted by nothing but a human hand for three days, and both exemption
classes (Renovate-manifest, docs-only) silently stopped working.
The cause is one token in prose. The #706 note explaining why a concurrency group
does not work here quoted a `concurrency:` snippet containing a PR-number expression
as an ILLUSTRATION, inside a shell comment. A shell comment is not inert there: the
runner scans the whole `run:` scalar for the expression opener before bash sees it,
and one occurrence makes it rewrite the ENTIRE body into a single `format(...)` call.
That rewrite is all-or-nothing, so a payload that does not parse — `pr number` does
not — fails the interpolation of the whole scalar, and the runner then DROPS THE STEP
AND CONCLUDES THE JOB GREEN. The prose documenting a fix disabled the fix.
`git blame`/`git log -S` put the line in 8f6d4f443 (2026-08-03), which dates the
outage precisely rather than "present in run 1832, not bisected further back".
Three changes, deliberately different in kind:
* The prose no longer writes the delimiters. It names the expression instead.
* The classifier writes a start marker and a new `if: always()` step FAILS THE JOB
when it is missing. This is the half that generalises: the delimiter was one bug in
one comment, but a dropped step concluding `success` is what made it cost three
days behind a green tick. It asserts execution STARTED, never completed — the
classifier has several legitimate `exit 0` abstention paths.
* Two static guards in scripts/tests/test_pr_changed_files.py: no expression
delimiter in ANY `run:` body of the gate file (absolute, because a dropped step
here is a dead merge gate and its bodies are ~700 lines of prose), and repo-wide,
every expression payload must name a real context or function (permissive, because
other workflows interpolate into `run:` legitimately). The second catches the class
— a payload that cannot evaluate, wherever it appears.
Note every pre-existing workflow-shape test reads `_code_lines()`, which strips
comments. That is correct for what it was for, but it encodes the assumption this bug
falsifies: inside a `run:` scalar a comment CAN change behaviour. The new strict test
reads the raw scalar for that reason and must never adopt `_code_lines`.
Blast radius, audited: PR #739 (docs-only) merged 2026-08-05 with ZERO commit
statuses on its head, and got in only because admin force-merge was still enabled.
#743 disabled that on 2026-08-06, so the workaround that absorbed this bug is gone —
the next docs-only or Renovate-manifest PR would be permanently stuck. The two
Renovate PRs in the window escaped by timing, merging minutes before the bad commit.
Non-exempt PRs were unaffected throughout: humans posted their verdicts by hand.
Verification: all six mutations red, restored tree green — reintroducing the exact
defect (strict + general tests), deleting the guard step, deleting only the marker
write, weakening `if: always()`, turning the guard's `exit 1` into `exit 0`, and
putting a delimiter in the guard's own body. 129 passed on the fixed tree.
Still to establish, and NOT claimed here: that the runner executes a LATER step after
dropping an earlier one. The #751 evidence cannot say — the classifier was the job's
last step, so there was never a subsequent step to observe. A scratch-base probe with
a negative control answers it next; if the runner drops the rest of the steps too,
this guard is inert and the body has to move into `scripts/`.
Refs #751
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 3 returned MERGEABLE with three LOW documentation findings. Batched
before posting the verdict, since a new sha voids both the CI run and the
verdict.
- `ci-cd.md` labelled the unprobed half of the `enable_push` bullet but stated
the `block_admin_merge_override` counterfactual flatly one bullet below —
the same measured-vs-attested flattening round 2 fixed, one site over. Now
labelled, with why it was not probed (verifying it means merging an
unreviewed PR).
- `release.verdict-status-check` said "what survives is the forgery list
above". That record's job is enumerating survivors, so an unqualified "what
survives is X" reads as exhaustive — and it omitted the admin residual, which
is a SKIP route rather than a forgery one. Added.
- `CLAUDE.md` never learned the rule. It is the always-read surface, and it
still framed a direct `git push origin main` as a live path while describing
a docs-only *push* exemption for a push the server now refuses. My corpus
sweep covered `docs/` and missed the file that carries the docs-update rule.
Note on what remains unverified rather than closed: neither direction of
`block_admin_merge_override` was measured, and whether Gitea treats an ABSENT
required context as blocking (versus satisfied) is asserted by our docs but
not proven — the combined status on this PR reads `success` with
`review-verdict/h10` absent. Both belong to #747's re-verification sweep.
Verification: 441/441 script tests; decisions-validate OK.
refs #743
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 2 of review. One blocking finding, and it is the same defect class as
round 1's: a present-tense claim that this PR falsified.
`release.verdict-status-check` — the record ABOUT the h10 status check — still
said "direct pushes to `main` are server-side permitted, so the gate can be
skipped without forging anything". A reader resolving that key from the catalog
would conclude the control does not exist. Round 1 corrected `ci-cd.md` and
`ci.actions-credential-scoping` and I stopped at the two sites I had edited,
instead of sweeping the corpus by SUBJECT. Swept properly this time
(`server-side permitted`, `bypassable`, `without forging`, `push whitelist`,
`enable_push`): this was the only remaining stale site.
Test gaps the reviewer found by mutation testing, now closed. Both mutants
SURVIVED the suite as shipped — the round-1 fixes were correct but unpinned:
- dropping `|| [ -n "${_h11_local_ref:-}" ]` → an unterminated final line is
dropped. Two directions, and the dangerous one is not the obvious one: a
dropped *branch* line leaves only tag refs and grants the exemption to a push
containing a branch. Both pinned.
- dropping `[ -t 0 ] ||` → the hook hangs forever on an interactive run. Pinned
with a real pty and an explicit timeout, so a regression fails cleanly rather
than hanging a CI job. Verified the mutant is killed by exactly that test
(and that it dies via the timeout, 32s).
Also from review, non-blocking:
- `ci-cd.md:951` cited `enable_push: false` alone as what closed#743 — the
precise thing the new record says never to do, since the force-merge route
also skipped the gate with no forgery. Now cites both fields.
- `ci-cd.md` flattened measured and source-attested into one 403: only the
contents API was probed; the web editor/upload/apply-patch paths share the
predicate but were not. Separated.
- `format-as-you-touch-rebase` still said "the documented sequence" and
"always" for the release-cut behind-ness. `docs/ci-cd.md` documents the tag
step, not the release-notes-PR flow, and the frequency is attested by one
observed cut. Attributed to #719 instead.
- Documented the operator recovery path. `block_admin_merge_override: true`
removes the `force_merge` escape that used to unstick a wrongly-red required
context — that escape WAS the bypass, so it is gone by design, and the
recovery (fix the status; last resort PATCH the field, merge, set it back)
needed to be written down rather than left implicit in a residual.
Verification: 441/441 script tests; decisions-validate OK; PyYAML parses all
193 records; both mutants confirmed killed and the hook restored byte-identical.
fixes#719
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Independent review found the record repeated on the merge path exactly the
mistake it had just diagnosed on the push path.
The push argument was: a whitelist naming `timothy` closes nothing, because
`timothy` is the identity every credential already holds. The merge path had
the identical shape and went unchecked — `block_admin_merge_override` defaults
to `false`, so `CanBypassBranchProtection` returns true for a repo admin and
`POST /pulls/{n}/merge` with `force_merge: true` merges straight past a missing
or red `review-verdict/h10`. One API call, no forgery, no PATCH — cheaper than
the push route this change had just removed.
So `enable_push: false` alone did NOT make the gate load-bearing, which is
what the record's headline sentence claimed. `main` now carries both fields;
they are one control and neither is citable alone.
An admin-shaped control that exempts the only admin exempts everybody.
Other review findings addressed:
- H11's owning record (`release.format-as-you-touch-rebase`) now documents the
#719 tag-only carve-out. It is a narrowing of an existing convention, so it
amends that record rather than adding a new one — including the two details
that are easy to regress (the .husky/pre-push forwarding, without which the
exemption is dead code the unit tests still pass over; and the at-least-one-
ref guard against vacuous exemption).
- The record now states which write surfaces were enumerated and how each was
established — contents-API refusal is MEASURED here (403 `user cannot commit
to repo`), apply-patch/revert/cherry-pick are source-attested only. The
admin force-merge bypass is likewise marked source-attested, not probed:
probing it means merging an unreviewed PR.
- prepush-rebase-check.sh: process a final ref line with no trailing newline
(previously dropped, which silently reinstated the #719 block), and skip the
stdin read on a TTY so an interactive run does not hang.
- Corrected a citation the review caught: docs/ci-cd.md documents the tag step,
not a release-notes-PR flow. Cite #719 for the observed flow instead.
Also fixed a frontmatter break this round introduced: a `: ` inside the
unquoted `rule:` scalar. PyYAML rejected it while the dependency-free reader
accepted it, so only `scripts/tests` caught it.
Verification: 438/438 script tests pass; decisions-validate OK; PyYAML parses
all three touched records.
refs #743#719
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
H11 (.claude/hooks/prepush-rebase-check.sh) refuses to push a branch that is
behind origin/main. It fired on tag-only pushes too, breaking every release
cut: docs/ci-cd.md's "Cutting a release" flow lands a release-notes commit
via PR and then tags that merge commit, so the local branch is always one
commit behind origin/main at tag time. A tag push cannot revert anyone's
merged work, which is the failure H11 exists to prevent, so skip the
freshness check when every ref being pushed is under refs/tags/.
.husky/pre-push previously consumed pre-push's stdin ref lines and forwarded
them only to prepush-donewhen.sh; prepush-rebase-check.sh got none. Forward
the captured $_prepush_refs to it too, or the new logic is dead.
Guard against the vacuous-truth case explicitly required by #719: "all
pushed refs are tags" is trivially true over zero ref lines (manual run,
forgotten forwarding), which would silently disable H11 for every push.
Require at least one parsed ref line before granting the exemption.
Adds scripts/tests/test_prepush_rebase_check_tag_exemption.py using real
local git repos (bare origin + a work tree pushed one commit behind it) to
exercise git fetch/merge-base/rev-list against a genuinely-moved origin:
tag-only allowed, branch-only still blocked, mixed branch+tag still
blocked, and zero ref lines still blocked (the vacuous-truth guard).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gitea evaluates `status_check_contexts` when it MERGES a PR. A direct
`git push origin HEAD:main` never consults them, so the whole h10 gate was
skippable with no forgery — strictly cheaper than every route enumerated in
#697. `main` now carries `enable_push: false`.
Measured on this instance (Gitea 1.27.1) against a throwaway `probe-743-*`
rule rather than against `main`:
enable_push: false -> push by timothy (site admin)
REFUSED, pre-receive hook declined
enable_push_whitelist + ["timothy"] -> identical push SUCCEEDED
That second line is why this is a DISABLE and not a whitelist: #743 offered the
two as interchangeable, but the only write accounts here are `timothy` (site
admin) and `renovate`, and every credential in the threat model — agent
sessions, PATs, the injected GITEA_TOKEN — acts as `timothy`. A whitelist
naming `timothy` would have ticked the box and closed nothing.
Then demonstrated on `main` itself, per the issue's Done-when: a direct push
was refused, and a tag-only push from the same worktree succeeded (tags are
governed by `tag_protections`, which is empty). The release cut is unaffected.
What this closes: the write-only credential routes — the injected GITEA_TOKEN,
RENOVATE_TOKEN, any non-admin collaborator PAT. What it does NOT close: an
admin credential can PATCH the protection off, push, and restore it. Recorded
as an accepted residual rather than implied to be covered.
Also corrects two claims the probe contradicted, and one that the mid-session
Gitea upgrade (1.25.4 -> 1.27.1) invalidated:
- ci-cd.md and ci.actions-credential-scoping both said "a push whitelist would
close more of this class than the 1.26 upgrade". The whitelist form closes
nothing here; corrected in place.
- ci.actions-credential-scoping's rule said "do NOT add a `permissions:` key
while this instance is below Gitea 1.26.0". That precondition no longer
holds at 1.27.1, so the directive now misleads. Corrected — while noting the
consequence is still UNVERIFIED: `/api/v1/settings/actions` 404s at 1.27.1,
so whether `permissions:` binds here was not probed. The upgrade alone is
not evidence the constraint works.
- That record's 1.25.4 measurements are now dated, not current. Flagged as
such rather than silently re-pinned to a version they were never taken on.
#743's fourth box (docker-build.yml `persist-credentials: false`) is decided
in the record and deliberately not done here: two of its checkout steps run
`git fetch ... || true` feeding the changed-file skip logic, so a credential
regression would be silent rather than loud. Drop the `|| true` masking first.
fixes#743
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the credential half of #697. `REGISTRY_PASSWORD` was the admin account's
basic auth, handed to head-resolved PR code by docker-build.yml; it is now a PAT
scoped `write:package` + `read:repository`.
Verified on Gitea 1.25.4: registry push SUCCEEDED, status GET 200, status POST
REFUSED 403 (required=[write:repository]).
Does NOT close the class. Surviving routes, all recorded: RENOVATE_TOKEN (#742),
the injected GITEA_TOKEN (server-management#714), a collaborator's own token, the
`v*` tag push, and — making all of them unnecessary — direct pushes to `main`,
which are server-side permitted (#743). ci-image.yml's trigger filter was
attempted, reverted, and split out as #744.
Three cold adversarial review rounds: BLOCKED, BLOCKED, BLOCKED, then MERGEABLE.
fixes#697
Round 1 BLOCKED (1 Blocker, 4 High, 3 Medium, 2 Low); round 2 BLOCKED on the fix
(1 Blocker, 2 High, 4 Medium, 2 Low); round 3 BLOCKED on one Medium. Every
finding re-verified against the live instance before acting.
ROUND 2 — the blocker was self-inflicted and the local gate could not see it.
Adding `branches: [main]` to ci-image.yml re-points `ci-image-pin`'s `expected`
at the editing commit, staling all five `container:` pins and failing that
BLOCKING job — for a change altering zero bytes of the toolchain image.
Reproduced: expected=ed9dd6254 vs pins=32747a0. Reverted here (the commit was
amended, so no commit on the branch touches that path) and filed as #744.
That edit had also FALSIFIED its own justification: branch publishing IS
load-bearing — docs/ci-cd.md documents the rebase-recovery flow as "let
ci-image.yml publish :<short sha>, then bump the pin", which is how you satisfy
ci-image-pin from inside a PR. Reverting also keeps three trigger descriptions
true (ci-cd.md:1043, the recovery flow, pr-checks.yml's escape-hatch comment).
Also fixed:
- gate-trigger-base-resolved.md was the file round 1's fix did not touch, and
still said "no workflow route retains human provenance" — false, since a
PR-added workflow can reference RENOVATE_TOKEN. Its `rule:` also kept the
race framing, and `rule:` is what the catalog and MemPalace mirror.
- `mechanics:` claimed "independent review confirmed no CI consumption breaks".
It confirmed no such thing. Round 3 then caught the REPLACEMENT sentence
making the same class of error: only the `container:` pull is exercised by a
PR, because `build` carries `if: github.event_name != 'pull_request'` and
cache-to/cache-from live only there. Those and the base-image pull first run
on the post-merge push to main — a wrong inference reddens main, not the PR.
- A fourth surviving route was unnamed: docker-build.yml publishes :prod from a
`v*` tag push and a tag may point at any commit (tag protections are empty).
"three surviving routes" became "at least these" — a count reads as complete.
- Unmarked inferences, a "three later sections" that undercounted four, a
dangling "the two items below", and a #744 rationale that stated the pin toll
without its documented remedy.
Local gate: 432 script tests pass; `decisions_validate.py --base origin/main
--head HEAD` and `build_decisions_catalog.py --check` both exit 0; ci-image-pin
recomputed by hand and matching the pinned commit. The record is 62 prose lines
against a 60-line ceiling that is a `::warning::` by design (#520) — the blocking
constraint is the 2-25% minority band, currently 10.8%.
Refs #697, #742, #743, #744.
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`REGISTRY_USER`/`REGISTRY_PASSWORD` were the ADMIN account's basic auth, and
`docker-build.yml` triggers on `pull_request` — head-resolved — so a PR's own
code was handed instance-admin credentials. Basic auth carries no scope, so the
same secret that pushes an image administers every repo on the instance and can
POST `review-verdict/h10`, the required context that makes merge-consent derived
rather than assertable. Refs #697.
Fixed at the credential, not the triggers: patching triggers enumerates
instances of "a ref-resolved workflow obtains status-capable credentials", and
adding a new workflow file is itself a route. `REGISTRY_PASSWORD` is now a PAT
scoped `write:package` + `read:repository`.
Verified on Gitea 1.25.4, not inferred:
- registry push of a probe tag SUCCEEDED (cleaned up, confirmed 404)
- GET /commits/{sha}/status SUCCEEDED (what ci-detect-already-validated.sh does)
- POST /statuses/{sha} REFUSED, HTTP 403:
required=[write:repository], token scope=write:package,read:repository
Scope of what this closes, stated without overclaim. It closes the instance-wide
admin escalation and that credential's durable forgery route — durable because a
status POSTed with a USER credential carries a real `creator` and is inherited as
a human verdict, while an Actions job's carries `creator: null` and is re-derived.
It does NOT close the class. Three things survive it:
- `RENOVATE_TOKEN` is a `write:repository` PAT of a real bot account in the
SAME secret store, so it also posts with non-null `creator`. It cannot be
scoped down (Renovate needs repo write), and secrets are a per-repo store
that any PR-added workflow can reference. Closing this needs the provenance
check tightened to an allow-list of approved reviewers.
- Every job still receives a write-capable `GITEA_TOKEN`. `permissions:` YAML
is a no-op before Gitea 1.26.0 and no `app.ini` lever exists at any version;
only >=1.26 with the Actions default set to Restricted binds it.
Tracked in server-management#714.
- Branch protection binds the context NAME, not its issuer, so any write-scoped
personal token forges the status with genuine human provenance. Unfixable
in-repo. `h10` is a process guard, not a security boundary against push access.
Auditing the secret STORE rather than the workflow set also surfaced
`SERVERMGMT_DEPLOY_KEY`, still present though the `bump-prod-compose` job that
used it was removed in 1b5efd7b9 — an SSH deploy key to another repo, obtainable
by any PR-added workflow, with no remaining benefit.
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-4 verification returned MERGEABLE with no new defects and no
BLOCKER/HIGH/MEDIUM. These are its remaining LOW and nits.
- "No results — try a search above." asserts a search that COMPLETED and found
nothing, so it was false beside a failed request. Both empty-state messages
are now suppressed on error and the role="alert" banner is the whole message,
as that test's comment already claimed. Pinned positively and negatively so a
refactor cannot satisfy the assertion by rendering nothing at all.
- The (typeof ADDABLE_TYPE_LIST)[number] derivation is now the named
AddableKind, spelled once instead of twice: a third ingress into the searched
kinds is most likely to be written by copying one of the existing two, and the
"one list, all ingresses" property should be visible at a glance rather than
reassembled.
- The §3b lesson cited two measured test counts, which go stale against the very
suite they describe — a count taken before the helper had unit tests no longer
holds now that it does. Scoped the observation to the sha it was measured on
and replaced the counts with the invariant they were evidence for: every gate
needs at least one test that reddens when that gate ALONE is removed.
- Documented why the error banner stays conditionally mounted while the hint's
live region does not: role="alert" is the one live-region role screen readers
reliably announce on insertion, so the two are correct for opposite reasons.
The review flagged the divergence as unexplained, not as wrong.
refs #740
Verification round returned MERGEABLE with all three blocking findings resolved
by measurement. These are its four remaining items.
- DEFAULT_SEARCH_KINDS was the SECOND ingress into the searched kinds and was
not derived, so the previous commit's "enforced by the type system" claim held
for one of two paths. A non-addable kind there typechecked clean and would
have overstated the hint with every one of its rows dropped — the exact defect
the derivation exists to prevent. Now derived; verified by mutation that
adding 'Collection' to it is a compile error.
- The error path fell into the min-query guidance branch, so a valid 2-character
query that got a 500 told the user to type at least 2 characters. That branch
conflated "nothing searched yet" with "the last search failed". Newly
introduced by the previous commit's error-path reset; now gated on !error and
pinned by a test.
- The aria-live region was mounted conditionally, creating the region and its
text in one commit — which most screen readers do not announce. It is now
mounted unconditionally with the condition inside.
- The §3b lesson mis-stated where the duplicate gate lived: it was inside
runSearch, the genuine single sink, NOT at one of the callers — so the rule as
written ("put the gate in the single sink, not at each caller") described the
revision that was rejected. Reworded to the actual lesson: the gate's home is
the shared helper, and "it's the single sink" is not evidence it is the only
guard. That misreading is why #685 got this wrong twice.
Declined again, with reasons: the NaN pageSize edge (faithful to the sibling
helper), the clamp test's unpinned lower bound (same), and the registry's
disclosed same-identity substitution gap.
refs #740
Independent review round 2 returned BLOCKED on two findings, both correct.
- The helper's bound was dead code to the suite. searchLibraryBrowseItems had
zero tests, so deleting its clamp OR its gate left the whole suite green —
while the registry note claimed a caller "cannot skip the bound". That is the
previous round's finding relocated, not removed. It now has the three tests
its sibling searchLibraryPickerOptions already had (clamp, gate, compile),
plus one pinning the full-row return that is its reason to exist.
- The screen kept a second copy of the min-query check, and the two masked each
other: the 1-character boundary test passed with EITHER gate alone, so it
pinned nothing. The screen's copy is deleted; the helper is the sole gate.
Measured before/after: with the duplicate present, removing the helper's gate
left that test green; with it gone, the same removal reddens it.
- §3b contradicted itself two lines apart — the parent still said "there is no
truncation, so there is no truncation hint" above a sub-bullet mandating one.
Reworded so a hint is permitted, required only where bulk selection makes the
count actionable. Same correction to the 'search-bounded' definition.
- A failed search left results/totalMatches stale, rendering a confident
"Showing 75 of 60000 matches" beside the error banner. The catch clears them.
- Results now carry a `Results for "<query>"` heading and the guidance is keyed
to the settled query, not the live input, so rows are never shown without
saying which search produced them. `selected` persists across queries (correct
for a multi-select picker); the Add button's count keeps it discoverable.
- The hint sums pre-filter totalCount against post-filter rows, which is only
correct because every filterable kind is addable. MediaKindFilter is now
derived from ADDABLE_TYPE_LIST, making that a compile error rather than prose.
- aria-live on the hint; the #740 doc caveat no longer overstates the typeahead
rule as a mandate this screen violates.
Declined: the NaN pageSize edge (copied faithfully from the sibling helper) and
the registry's same-identity substitution gap (already disclosed in that file).
refs #740
Independent review round 2. Verdict was MERGEABLE with no blockers; this takes
the two recommended fixes plus the structural one it listed as a follow-up.
- The bound was caller discipline, not code: getLibraryBrowseItems does not
clamp pageSize, so the bound was only the constant this one call site chose
to pass, and §3b is explicit that a bound a caller can exceed is not a bound.
New searchLibraryBrowseItems in libraryBrowse.ts owns the min-query gate, the
pageSize clamp and the titleContainsQuery compile, returning full
LibraryBrowseItem rows plus totalCount (searchLibraryPickerOptions' {id,name}
shape loses the mediaType that toAddItemsRequest needs). runSearch keeps one
early return, for the spinner only, and no longer re-implements the gate.
- The min-query guidance was keyed to the LIVE input, so backspacing below the
gate after a search wiped the rendered rows and their checkmarks while
`selected` and the Add button still counted them. Keyed to results.length too.
- "Nothing left to hint at" was false: each kind is still capped at
LIBRARY_PICKER_RESULTS and totalCount was never read. This is a bulk
multi-select add, so the cap is surfaced — per-kind totalCounts are summed and
rendered as "Showing N of M matches" once it exceeds the rendered rows. The
registry note and the §3b bullet are corrected to stop claiming otherwise.
- Gate boundary tested at 1 character (§3b: inclusive endpoints, or a > for >=
slip passes the whole suite).
- The guard test's deviation loop iterates an empty list now, so it gains one
bidirectional assertion that is non-vacuous: the set carrying an `issue` field
must equal the set classified 'deviation'.
- The §3b bullet no longer reads as a conformance certificate: AddItemsDialog
still lacks the seqRef and useIsMountedRef guards §3b mandates. That defect is
PRE-EXISTING, not introduced here, and is tracked in #740.
refs #740
AddItemsDialog.runSearch was reachable with an empty query two ways — a blank
form submit, and a kind-chip click, which called it immediately — and
getLibraryBrowseItems omits a falsy `query`, so each path degraded into an
unfiltered browse of the whole media-library type (first 50 rows, per kind)
presented as the answer with nothing surfacing the truncation. All ten
ADDABLE_TYPE_LIST entries are spa-conventions §3b Class B media-library types.
The dialog is multi-select, so §3b's SearchPicker (single-select) does not fit;
it takes §3b's constraints instead:
- no request below LIBRARY_PICKER_MIN_QUERY, enforced in runSearch — the single
sink both entry paths route through, not duplicated per caller
- typed text compiled with titleContainsQuery rather than forwarded raw (a
second latent §3b violation here: the search index's default field does not
match bare title words)
- each kind bounded to LIBRARY_PICKER_RESULTS
- merged.slice(0, 50) removed — it silently dropped up to 100 of 150 fetched
rows even for a real query
Tests assert zero requests below the gate on both paths, exactly one bounded
request per kind above it (20k-row fixture), the compiled+escaped query, and
that no fetched row is dropped. Each was verified to fail with its mechanism
removed.
The pageSize registry entry moves from `deviation` to `search-bounded`. That
leaves zero deviation entries, so the anti-vacuity assertion guarding that list
is deleted deliberately, per its own instruction.
Done-when box 4 (collection-family truncation hint) has no subject: this screen
offers no collection-family type.
fixes#685
The refresh_epg_data command targeted 192.168.1.99, but Dispatcharr moved to
jazz (192.168.1.29) in #634 — it would fail with 'No such container'.
refs server-management#692
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-05 18:35:38 +02:00
272 changed files with 54166 additions and 1474 deletions
# --- Docs-only exemption: if every changed file is docs/process, skip the gate. ---
# The file list must be enumerated EXHAUSTIVELY, validated row by row, and bound to ONE head, or the
# exemption is unsafe. ALL of that now lives in scripts/pr-changed-files.sh — the single shared
# The file list must be enumerated EXHAUSTIVELY, validated row by row, and checked for head/base
# movement across the paging round trips, or the exemption is unsafe. (That check detects ONE-WAY
# movement only — this said "bound to ONE head" until 2026-08-28, ersatztv#803.) ALL of that now lives in scripts/pr-changed-files.sh — the single shared
# implementation, also called by .gitea/workflows/review-verdict.yml (ersatztv#649).
#
# Why it moved: this logic was written twice. This copy is ADVISORY (a failure produces a human
@@ -168,10 +180,81 @@ fi
# after which a later, successful status read could still auto-grant. A transient failure would then
# have produced a "merge gate: satisfied" message for a comparison that never happened. Every
# unreadable input here therefore falls through to a human (`ask`), never to silence.
decide ask "H10 merge gate: could not re-read PR #$pr to confirm it still targets '$base_ref' before checking the verdict against it. Confirm the target branch, then merge."
decide ask "H10 merge gate: PR #$pr reports no base branch (.base.ref), so the verdict cannot be checked against the branch it was formed for (ersatztv#632). Confirm the PR still targets the branch it was reviewed against before merging."
decide deny "H6/H10 merge gate: BLOCKED — PR #$pr was retargeted from '$base_ref' to '$base_now' while this gate was evaluating. Every check formed against '$base_ref', including the changed-file enumeration and the review verdict, describes a merge that is no longer the one being requested (ersatztv#632). Re-review against '$base_now' and run: scripts/post-review-verdict.sh $pr MERGEABLE"
fi
# From here on both names are the freshly-confirmed base; they are equal by the check above.
base_ref=$base_now
live_base=$base_now
# THE HEAD IS RE-READ AT THE SAME HOIST, FROM THE SAME RESPONSE (ersatztv#803).
#
# `$sha` comes from the PR snapshot at the top of this hook, and until 2026-08-28 every later check
# consumed that captured value: the CI combined status, the `review-verdict/h10` status, and the
# verdict-comment classification were all evaluated against `/commits/$sha/status` and `--head $sha`.
# A push landing in the gap — which includes the docs-only enumeration's up-to-forty round trips —
# was therefore checked against the commit it had just replaced, and the hook would report "a
# positive Review-verdict references the current head" about a head that was no longer current.
#
# This is the SAME defect the base had until #778 hoisted the re-read above, and it is fixed the same
# way rather than a different way. Reading `.head.sha` off `$prjson_now` — the response the base
# check already fetched — costs NO extra round trip, and it keeps the two axes on ONE snapshot, so
# they cannot disagree about which moment they describe. Two separate reads would answer about two
# different instants while reading as one check.
#
# DENY, not ask, and for the same reason the `stale` verdict class denies: a head that moved means
# the verdict this hook is about to accept covers an OLDER commit, which is a state we have
# positively established rather than failed to establish. An UNREADABLE `.head.sha` is the different
# case and asks.
#
# WHAT THIS DOES NOT CLOSE, said here rather than left to be inferred. A push landing after this
# check still passes, exactly as a retarget does — the file's rule against a second re-read applies
# unchanged (see the branch-protection block below), because two reads only move the window rather
# than closing it. That residual is bounded server-side and this hook is not what bounds it: the new
# head has no `review-verdict/h10` status, and that context is REQUIRED on `main`, so Gitea refuses
# the merge (#622). The hook's job here is to stop CLAIMING a head is reviewed when it can see that
# it is not — an advisory gate that states something false is worse than one that asks.
decide ask "H10 merge gate: PR #$pr reports no head commit (.head.sha) on re-read, so whether the review verdict still covers the current head could not be confirmed. Check the PR, then merge."
fi
if["$sha_now" !="$sha"];then
decide deny "H6/H10 merge gate: BLOCKED — PR #$pr's head moved from ${sha:0:7} to ${sha_now:0:7} while this gate was evaluating. Every check formed against ${sha:0:7} — the changed-file enumeration, the CI status and the review verdict — describes a commit that is no longer the one being merged (ersatztv#803). Re-review the current head and run: scripts/post-review-verdict.sh $pr MERGEABLE"
fi
# From here on `$sha` is the freshly-confirmed head; the two are equal by the check above. Mirrors
# `base_ref=$base_now` a few lines up, and is written for the same reason that one is: it makes the
# value every later check consumes the one that was just re-read, so a future edit moving a
# consumer above this point fails visibly rather than silently reading the stale capture.
sha=$sha_now
fi
if[ -n "$sha"];then
# This is the THIRD read of this endpoint in a worst-case hook run (the ordinary-CI branch and the
# scheduled-auto-merge branch each do their own). Sharing one snapshot would close a narrow
@@ -301,8 +384,19 @@ else
# status decide this. Here the fallthrough happens to land on `vstate=""` -> deny (fail-CLOSED,
# so this was never a hole), but it would have surfaced the wrong message — a "BLOCKED, no
# verdict" deny instead of the "could not read the status" ask this branch exists to give.
decide ask "H6/H10 merge gate: could not read the 'review-verdict/h10' status for PR #$pr head ${sha:0:7} (Gitea unreachable or an unexpected response). Confirm the current head is reviewed before scheduling an auto-merge."
# Validate the MEMBERS, not just the array. `.statuses | type == "array"` passes for
# `{"statuses":[1]}`, and the extraction below then errors with "Cannot index number with string"
# and exits 5 — which, under `set -e`, aborts this hook with NO JSON on stdout at all. A consent
# hook that emits nothing has violated its own contract: it neither grants, denies nor asks. Same
# one-level-down swallow as the #632 base-change guard and the branch-protection shape check
# below; the validation domain must match the CONSUMPTION domain (ersatztv#778).
if[ -z "${vjson//[[:space:]]/}"]\
|| ! printf'%s'"$vjson"\
| jq -e '(.statuses | type == "array")
and all(.statuses[]; type == "object"
and ((.context | type) == "string")
and ((.status | type) == "string"))' >/dev/null 2>&1;then
decide ask "H6/H10 merge gate: could not read the 'review-verdict/h10' status for PR #$pr head ${sha:0:7} (Gitea unreachable, or a response whose status rows are not the expected shape). Confirm the current head is reviewed before scheduling an auto-merge."
pending) decide deny "H6/H10 merge gate: BLOCKED — 'review-verdict/h10' is still pending on PR #$pr head ${sha:0:7} (no verdict posted for this commit yet). Review the current head and run: scripts/post-review-verdict.sh $pr MERGEABLE";;
*) decide deny "H6/H10 merge gate: BLOCKED — 'review-verdict/h10' is '$vstate' on PR #$pr head ${sha:0:7}. Resolve the findings, then run: scripts/post-review-verdict.sh $pr MERGEABLE";;
esac
# --- The mitigation this path RESTS on, verified instead of asserted (ersatztv#778). -----------
# Everything above proves a property of the head that exists NOW. What makes that safe under
# merge_when_checks_succeed is stated in the paragraph opening this branch: `review-verdict/h10`
# is a REQUIRED status check on the base, a commit status belongs to exactly ONE sha, so a commit
# pushed after scheduling cannot inherit it and Gitea's own gate refuses the merge.
#
# That guarantee is branch-protection CONFIG. It lives outside this repo, no code here owned it,
# and until #778 nothing compared the two — so the grant reason handed to a human cited a
# protection that could have been switched off with no signal anywhere. The comment above and the
# grant string below are claims about the past; a dated claim is not a check.
#
# This is the hook's OWN defect class (#778 / `process.check-and-use-pins-a-version`): a check
# ("a later push clears the status") authorizes an action ("arm an auto-merge that Gitea completes
# later") over state that can change in between, with nothing pinning it. The read here does not
# pin anything either — branch protection can still be edited after this call — but it converts an
# ASSUMPTION that was never observed into a precondition that is, which is the honest ceiling for
# a config whose API offers no version, ETag or conditional read.
#
# Tri-state, matching this file's idiom throughout: unreadable -> ask (a human adjudicates),
# present -> proceed, ABSENT -> deny. Absence is not a degraded read; it is #622's hole reopened,
# and the whole point of that issue is that the failure is silent from the merge caller's side.
# Belt-and-braces: `$base_ref` was proven non-empty and re-confirmed at the hoisted check above,
# so this cannot fire today. Kept because it is the precondition this block's URL depends on, and
# a future edit that moves either piece should fail loudly here rather than request a URL with an
# empty path segment.
[ -n "$base_ref"]|| decide ask "H6/H10 merge gate: could not resolve PR #$pr's base branch, so the 'review-verdict/h10' required-check protection that makes a scheduled auto-merge safe (ersatztv#622) can't be confirmed. Verify branch protection on the base, or merge immediately instead of scheduling."
# The base was re-read and confirmed unchanged above, for every path — see the hoist comment
# there. It is deliberately NOT re-read a second time here: two reads would create a window
# between them for no gain, and the hoisted check already covers the enumeration gap that made
# this necessary.
# A read failure here is NOT evidence about the branch. The deleted by-name endpoint answered 404
# for "no rule with this name", which was a finding; the LIST endpoint's 404 means the repo was not
# found or is invisible to this credential, which is a read failure. Absence is now established by
# the classifier returning `nomatch` over a list that WAS read, never by an HTTP status.
# ALWAYS enumerate the rule LIST; never look a rule up by name. The by-name endpoint
# (`branch_protections/{name}`) is an exact DB lookup — `GetProtectedBranchRuleByName` — which
# performs no matching and knows nothing about precedence, so a 200 from it means only "a rule
# with this NAME exists and lists this context", never "this context is required on this branch".
#
# It was used first, with the list consulted only on a 404, and cold review found what that left
# behind: the precedence argument below guarded the 404 path while the 200 path — the one this
# repo actually takes — granted without it. Given a rule `main` requiring `review-verdict/h10` and
# a rule `m*` with better Priority that does not, Gitea applies `m*`, and the by-name hit on
# `main` granted anyway. The hardened path was dead code and the unhardened one was live. Deleting
# the twin rather than documenting it is the point: one fetch, one classifier, one argument, and
# no second path to keep in step. The ref no longer reaches a URL segment, so it needs no
# encoding either.
bp_file=$(mktemp)|| decide ask "H6/H10 merge gate: could not allocate a temp file to read branch protection for '$base_ref'. Confirm the 'review-verdict/h10' required check manually before scheduling an auto-merge."
decide ask "H6/H10 merge gate: the shared branch-protection rule classifier is missing or unreadable at $classifier, so which rule governs '$base_ref' — and therefore whether 'review-verdict/h10' is required on it — could not be derived (ersatztv#787). Restore the file, or confirm the required checks manually."
fi
bp_verdict=$(printf'%s'"$bp_list"| jq --arg b "$base_ref" -c -f "$classifier" 2>/dev/null ||true)
case$(printf'%s'"$bp_verdict"| jq -r '.verdict // ""' 2>/dev/null ||true) in
decide ask "H6/H10 merge gate: no branch-protection rule on this repo governs '$base_ref' decidably — a GLOB rule could govern it, or two rule names fold-equal, or a name is non-ASCII. This hook deliberately does not reimplement Gitea's glob matcher, so whether 'review-verdict/h10' is required on this base cannot be derived here (ersatztv#778). Confirm it in the repo's branch-protection settings, or merge immediately instead of scheduling.";;
none)bp_code=nomatch;bp="";;
*)bp_code=unreadable-rules;bp="";;
esac
else
# A 200 whose body is NOT an array never reaches the classifier — it is diverted by the array
# gate above — so it needs the same sentinel, or the generic ask below reports
# "HTTP '200' — Gitea unreachable" about a read that plainly succeeded. Same defect as the
# throw-inside-the-classifier arm, one branch earlier; fixing only the arm where it was noticed
# is the twin-missed shape this PR is largely about.
if["$bp_code"="200"];then
bp_code=unreadable-rules
else
bp_code=${bp_code:-000}# a real transport/HTTP failure -> the ask arm below
fi
bp=""
fi
rm -f "$bp_file"
# `nomatch` is the CLASSIFIER's verdict, deliberately not an HTTP code. Reusing 404 for it made
# this deny reachable from an HTTP 404 on the list read too — repo not found, or invisible to the
# credential, which Gitea also answers 404 — and then the reason claimed "the full rule list was
# read and none matches" about a read that never happened. A transport failure must reach the ask
# below, not a deny stating a finding.
if["$bp_code"="nomatch"];then
decide deny "H6/H10 merge gate: BLOCKED — no branch-protection rule on this repo can govern '$base_ref' (the full rule list was read and none matches), so 'review-verdict/h10' is not a required check on it. A scheduled auto-merge is safe ONLY because that per-sha required check stops a commit pushed after scheduling from merging unreviewed (ersatztv#622). Restore branch protection on '$base_ref', or merge immediately (without merge_when_checks_succeed) once CI is green."
fi
# `unreadable-rules` is the CLASSIFIER failing on a 200 it could not parse — a numeric
# `branch_name` makes jq throw, and `//` does not catch it because it fires only on null/false.
# It gets its own sentinel for the same reason `nomatch` does: reporting "HTTP '000' — Gitea
# unreachable" about a successful 200 read states a cause that did not happen, which is the defect
# fixed one arm over for the deny.
if["$bp_code"="unreadable-rules"];then
decide ask "H6/H10 merge gate: this repo's branch-protection rules came back in a shape this hook could not parse, so whether 'review-verdict/h10' is required on '$base_ref' is unknown. Check the rules manually, or merge immediately instead of scheduling."
decide ask "H6/H10 merge gate: could not read this repo's branch-protection rules (HTTP '${bp_code:-none}' — Gitea unreachable, or these credentials lack the repo-admin scope that endpoint needs), so whether 'review-verdict/h10' is required on '$base_ref' is unknown. Scheduling an auto-merge is only safe while 'review-verdict/h10' is a REQUIRED check there (ersatztv#622) — confirm that manually, or merge immediately instead of scheduling."
fi
# The membership test is `any(.[]; . == …)` over a value FIRST PROVEN to be an array of strings —
# never `index()`. `index` on a STRING is substring search, so a `status_check_contexts` that
# arrived as the string "prefix-review-verdict/h10-suffix" would answer "yes" and auto-grant a
# merge on a base where no such context is required. That is a FALSE-OPEN in the gate, reachable
# from any payload shape drift, and it is the direction that matters: a false-closed costs a
# prompt, a false-open costs an unreviewed merge.
#
# Validating `$bp` as an object does not make its MEMBERS well-formed, which is the same
# one-level-down swallow that survived the first fix in the #632 base-change guard — the
# validation domain has to match the CONSUMPTION domain, not stop at the top-level type. So the
# shape is checked explicitly and anything else becomes "unknown" rather than a decision.
#
# `null` and `[]` are legitimate (an unprotected-in-practice branch) and answer "no", not
# "unknown": absent IS the finding here, not a read failure. The word is then matched
# exhaustively, because "" is not a third synonym for "no".
# `// []` defaults on FALSE as well as on null, because jq's alternative operator fires for both.
# So `"status_check_contexts": false` — a malformed shape — became `[]` and answered "no", i.e. a
# confident DENY derived from a payload that was never understood. Absent and null are defaulted
# explicitly; every other non-array is "unknown".
# `enable_status_check` is validated as a BOOLEAN before it is trusted, for the same reason the
# contexts list is: `"true"` (the string) is not `true`, and comparing it to `true` yields a
# confident "no" -> deny derived from a payload never understood. Every malformed shape on this
# endpoint has to reach the same "unknown" -> ask arm, or the tri-state is only two states.
guarded=$(printf'%s'"$bp"\
| jq -r 'def ctxs: if (has("status_check_contexts") | not) or .status_check_contexts == null
then [] else .status_check_contexts end;
if (.enable_status_check | type) != "boolean" then "unknown"
elif (ctxs | type) != "array" or any(ctxs[]; type != "string") then "unknown"
elif (.enable_status_check == true) and any(ctxs[]; . == "review-verdict/h10") then "yes"
else "no" end' 2>/dev/null ||true)
case"$guarded" in
yes) : ;;
no) decide deny "H6/H10 merge gate: BLOCKED — 'review-verdict/h10' is NOT a required status check on '$base_ref' (branch protection reports enable_status_check/status_check_contexts without it). A scheduled auto-merge is safe ONLY because that per-sha required check stops a commit pushed after scheduling from merging unreviewed (ersatztv#622); without it, arming merge_when_checks_succeed freezes consent at a head Gitea may not be the one to merge. Restore it in branch protection, or merge immediately (without merge_when_checks_succeed) once CI is green.";;
*) decide ask "H6/H10 merge gate: branch protection for '$base_ref' came back in an unexpected shape, so the 'review-verdict/h10' required check that makes a scheduled auto-merge safe (ersatztv#622) could not be confirmed either way. Check it manually, or merge immediately instead of scheduling.";;
esac
fi
# --- (c) Review-verdict freshness (ersatztv#303 H10): a review-verdict comment must reference the
@@ -359,13 +649,126 @@ case "$class" in
decide ask "H10 merge gate: unrecognized verdict classification '$class' for PR #$pr. Confirm the review covered the latest commit ($short) before merging.";;
esac
# --- (d) Guard-scope freshness (ersatztv#787): the committed mirror of `main`'s required status
# checks must still match the server. ------------------------------------------------------
# ORDERED LAST, and that is a severity argument rather than a stylistic one. Every check above
# can DENY; this one can only ever downgrade an otherwise-satisfied auto-grant to a prompt. Run
# earlier it would preempt those verdicts and report a stale guard scope at a reader whose merge
# is blocked for a completely different and more serious reason, and it would ask on payloads the
# checks above are about to reject anyway. Placed here it is also PAST the point where the two
# merge paths converge, so it covers both without duplicating anything.
# `scripts/tests/test_ci_dropped_step_guard.py` DERIVES which jobs must carry per-step execution
# markers from `.gitea/required-status-contexts.json`, because its CI job checks out with
# `persist-credentials: false` and cannot ask Gitea. That makes the snapshot the single
# hand-maintained input in the chain: a fourth required context added on the server leaves the
# snapshot — and therefore the guard's scope — silently behind, which is the whole of #787.
#
# THIS RUNS ON BOTH MERGE PATHS, deliberately, and it is placed here rather than beside the
# branch-protection read in the scheduled-auto-merge branch for that reason.
#
# WHAT IT DOES NOT COVER, said here rather than left to be discovered: a PR whose changed files are
# all docs/process — `.gitea/` included — exits at the docs-only passthrough far above, so this arm
# never runs for it. A PR that edits ONLY `.gitea/required-status-contexts.json` is docs-only BY
# CONSTRUCTION, and that is exactly the snapshot-NARROWING direction the decision record names as
# this design's residual. Excluding that path from the allow-list would not buy the protection it
# looks like it would: this arm compares the live server against the snapshot in the LOCAL CHECKOUT,
# not against the version the PR proposes, so it cannot see a narrowing that has not landed yet.
# What does hold is that the passthrough is a passthrough — a human prompt, never an auto-grant —
# which is the `.gitea/` treatment ersatztv#317 asked for. That read is inside
# `else` (mwcs = true) and never executes on an immediate merge, which is the common case; hanging
# the freshness check off it would fire it only when an auto-merge is armed. This file already
# records that exact defect one section up — the base re-read "first landed inside the
# scheduled-auto-merge branch only", and cold review found scheduled+retarget denied while
# immediate+retarget auto-GRANTED. Same shape, so it is not repeated here.
#
# It reads `main` (the branch the snapshot names), NOT `$base_ref`. That is a DIFFERENT question
# from the one the scheduled branch asks — "is review-verdict/h10 required on the base I am merging
# into" — so this is not a second copy of that classifier and the two cannot drift into disagreeing:
# they consume different fields of different rules for different decisions.
#
# ASK, NEVER DENY. Drift does not make THIS merge unsafe: Gitea enforces the live required set
# server-side, so a newly required context with no status blocks the merge on its own. What has gone
# stale is a guard's scope — a different artifact, on a different clock. Denying would state
# something false about the change in front of the reader. Every non-`match` class asks, so a
# comparison that could not be made is surfaced rather than skipped (`unknown` is not `fine`).
# ONE base for both the checker and the snapshot, and it is `$repo_root` — derived from this file's
# own location — rather than `$CLAUDE_PROJECT_DIR`. Two reasons, and the second is the load-bearing
# one. Resolving them from different roots would let the hook classify one checkout's snapshot with
# another checkout's script, mismatched halves of a comparison whose whole job is to detect a
# mismatch. And an ENV VAR is not a sound input to a security decision: a wrong value pointing at a
# tree that happens to contain an executable checker returns `match` about a different checkout
# entirely, which silently authorizes the grant. A missing path only asks, so the failure is quiet
decide ask "H6 merge gate: $ctx_snapshot is missing, unreadable, or names no \`repo\`, so the dropped-step guard's scope could not be checked against branch protection — nor could it be established whether this snapshot even describes $owner/$repo (ersatztv#787). Restore the file, or check the required checks manually."
fi
# CASE-FOLDED, because Gitea resolves owner/repo case-insensitively: verified live, both
# `/repos/timothy/ersatztv` and `/repos/TIMOTHY/ErsatzTV` answer 200. A byte-exact compare would let
# any case variant sail through every other arm and SKIP this one, so drift would go unreported with
# no ask — the gate failing open on a spelling. The hook already treats case folding as
# decision-relevant one section up, where `MAIN` vs `main` makes the governing rule undecidable.
decide ask "H6 merge gate: the required-contexts checker is missing or not executable at $ctx_script, so whether the dropped-step guard's scope still matches branch protection on 'main' could not be derived (ersatztv#787). Check it manually, or restore the script."
fi
bpf=$(mktemp)|| decide ask "H6 merge gate: could not allocate a temp file to read branch protection for the guard-scope freshness check (ersatztv#787)."
decide ask "H6 merge gate: the required status checks on 'main' no longer match .gitea/required-status-contexts.json (ersatztv#787). scripts/tests/test_ci_dropped_step_guard.py derives its marked-job scope from that snapshot, so until it is reconciled a required context may have NO dropped-step guard — a step the runner drops would conclude success and take that check green having done no work (ersatztv#756). Re-read the live list and update the snapshot in a PR (the guard will then demand markers for any newly required job, or an ACCOUNTED_ELSEWHERE entry naming what covers it). This does not make the merge in front of you unsafe — Gitea enforces the live required set server-side — so approve if you have judged it unrelated.";;
nomatch)
decide ask "H6 merge gate: no branch-protection rule governs 'main' at all, so the required status checks the dropped-step guard scopes itself to could not be confirmed (ersatztv#787). Branch protection on 'main' is what makes 'review-verdict/h10' load-bearing (ersatztv#743) — check it before merging.";;
undecidable)
decide ask "H6 merge gate: a glob branch-protection rule could govern 'main', so which rule's required contexts to compare against .gitea/required-status-contexts.json is not derivable without reimplementing Gitea's matcher (ersatztv#787). Confirm the required checks manually.";;
unreadable)
decide ask "H6 merge gate: branch protection for 'main', or .gitea/required-status-contexts.json itself, came back in a shape the required-contexts checker could not consume, so whether the dropped-step guard's scope is still current is unknown (ersatztv#787). Check the rules and the snapshot manually.";;
readfail)
decide ask "H6 merge gate: could not read branch protection for the guard-scope freshness check (HTTP '${ctx_code:-none}' — Gitea unreachable, or these credentials lack the repo-admin scope that endpoint needs), so whether .gitea/required-status-contexts.json is still current is unknown (ersatztv#787). Confirm the required checks on 'main' manually.";;
*)
decide ask "H6 merge gate: the required-contexts checker returned '${ctx_class:-nothing}', which is not a class this hook understands, so the dropped-step guard's scope could not be confirmed against branch protection (ersatztv#787).${ctx_diag:+ It said:${ctx_diag}}Check scripts/check-required-contexts.sh.";;
esac
fi# end of the guard-scope freshness arm (opened at `if [ "$ctx_repo_fold" = ... ]` above). The
# body is left unindented to match the rest of this file, which is flat throughout; the marker
# is here because the block is long enough that its extent is otherwise easy to misread.
if["$class"="positive"];then
# (a) CI + (b) all Done-when ticked + (c) positive verdict @ current head -> SATISFIED. Auto-grant.
# The reason string must not claim more than was actually checked: on the merge_when_checks_succeed
# path this hook never read the CI status at all (it is delegated to Gitea), so saying "CI green"
# there was a plain falsehood in the one message a human reads to decide whether to trust the gate.
if["$mwcs"="true"];then
decide grant "H6/H10 merge gate: satisfied — all Done-when boxes ticked, and both a positive Review-verdict comment and the 'review-verdict/h10' status cover the current head ($short). CI is gated by Gitea (merge_when_checks_succeed), and because the verdict status is bound to this sha, a commit pushed before Gitea merges will clear it and block the merge (ersatztv#622). Auto-granted."
decide grant "H6/H10 merge gate: satisfied — all Done-when boxes ticked, and both a positive Review-verdict comment and the 'review-verdict/h10' status cover the current head ($short). CI is gated by Gitea (merge_when_checks_succeed). A commit pushed before Gitea merges clears the sha-bound verdict status and is blocked by the 'review-verdict/h10' required check (ersatztv#622) — which this hook has just CONFIRMED is still required on '$base_ref' — read from the repo's full rule list and matched with Gitea's own plain-vs-glob split, refusing rather than guessing wherever precedence or folding is not derivable. That guarantee holds while that branch protection stands; if it is weakened after this check, nothing here would see it (ersatztv#778). Auto-granted."
fi
decide grant "H6/H10 merge gate: satisfied — CI green, all Done-when boxes ticked, and a positive Review-verdict references the current head ($short). Auto-granted (no separate confirmation needed)."
print('\n'.join(f'{m.upper()} {p}' for p,i in d['paths'].items() for m in i if m in('get','post')))"
```
Field lists above are the request-body property names only; consult the spec for types,
required-ness and defaults. That omission matters for the three on/off pairs: `graphics_on`/
`graphics_off`, `watermark_on`/`watermark_off` and `pre_roll_on`/`pre_roll_off` are **separate
operations, not one toggle**, and the difference is not always visible as differing property names.
`graphics_*` and `pre_roll_*` differ outright. `watermark_on` and `watermark_off` both list
`{watermark}`, but only `on` marks it **required** — `watermark_off` with an **empty** list turns
*every* scripted watermark off (`SchedulingEngine.WatermarkOff`: `watermarks.Count == 0` →
`ClearChannelWatermarkIds()`; `GraphicsOff` is the same shape). Read the schema, not this table,
before sending an `_off`.
## SQLite DB Operations
```bash
@@ -363,7 +463,7 @@ docker start ersatztv
```
- **Dispatcharr caches ErsatzTV's XMLTV.** Repointing its DB rows is not enough — it keeps serving a stale EPG full of dead `ersatztv:8409` artwork URLs (breaks Kodi artwork). Force a refresh (EPG source 9):
- **`/api/health` returns 401** (needs an API key). The Telegraf probe has no `response_string_match`, so ErsatzTV reads as **unhealthy in Grafana** — a false alarm, and **pre-existing**, not caused by the move. The container healthcheck uses the unauthenticated internal `/health` and is unaffected.
"source":"GET /repos/timothy/ersatztv/branch_protections -> the rule governing `main` -> status_check_contexts",
"why":"ersatztv#787. The committed mirror of the required status checks on `main`. It exists because the guards that make a required context trustworthy run in `pr-checks.yml::script-tests`, which checks out with persist-credentials:false and holds no Gitea credential, so it cannot ask the server. scripts/tests/test_ci_dropped_step_guard.py DERIVES its marked-job scope from `contexts` rather than repeating it as a literal, and scripts/check-required-contexts.sh compares this list against the live one wherever a credential does exist. Editing `contexts` by hand without re-reading the server is the one move that defeats both. The `repo` field exists because the merge-consent hook fires for whatever owner/repo the merge tool was called with: without it, merging a PR in another repo from an ersatztv session compares that repo's live contexts against THIS repo's mirror and reports a confident, flatly false finding about it.",
"contexts":[
"Build ErsatzTV Image / Build & test (.NET) (pull_request)",
echo "::error::git fetch of origin/${base_ref} failed, so this job cannot compute the changed-file set it derives its work from. That is a broken job, not an empty change set (ersatztv#746). Check the base branch still exists and that the runner can reach the repository."
exit 1
fi
if ! changed="$(git diff --name-only "origin/${base_ref}...HEAD")"; then
echo "::error::git diff against origin/${base_ref} failed, so the changed-file set could not be computed — do not read this as 'nothing changed' (ersatztv#746). If it reports no merge base, rebase this branch onto ${base_ref}."
exit 1
fi
echo "Changed files in this PR:"; printf '%s\n' "$changed"
if printf '%s\n' "$changed" | grep -Eq '^ErsatzTV/Controllers/Api/|^ErsatzTV\.Core/Api/'; then
echo "::error::git fetch of origin/${base_ref} failed, so this job cannot compute the changed-file set it derives its work from. That is a broken job, not an empty change set (ersatztv#746). Check the base branch still exists and that the runner can reach the repository."
exit 1
fi
if ! changed="$(git diff --name-only --diff-filter=ACM "origin/${base_ref}...HEAD" -- '*.cs')"; then
echo "::error::git diff against origin/${base_ref} failed, so the changed-file set could not be computed — do not read this as 'nothing changed' (ersatztv#746). If it reports no merge base, rebase this branch onto ${base_ref}."
exit 1
fi
echo "Changed .cs files in this PR:"; printf '%s\n' "$changed"
echo "Pins found in docker-build.yml: ${pins[*]} (${#pins[@]} distinct)"
@@ -107,10 +146,10 @@ jobs:
# in-repo remedy in that state: relax this length check in the same PR and say why. Note
# that ci-image.yml still tags with a plain `--short` (auto-scaled), so "always 7" is an
# empirical property of today's shallow clone, not an enforced invariant. Making the
# publisher emit `--short=7` is tracked as ersatztv#597. It is not blocked, just out of
# scope here: editing ci-image.yml re-points `expected` (above) at that commit, so it needs
# the branch's own publish-then-pin two-step (docs/ci-cd.md -> 'CI toolchain image') —
# ci-image.yml's push trigger has no branches: filter, so a feature branch does publish.
# publisher emit `--short=7` is tracked as ersatztv#597. That is no longer blocked by this
# job at all: since ersatztv#744, editing ci-image.yml does NOT re-point `expected`, so a
# `--short=7` change lands like any other PR. It does need a deliberate republish to take
# effect — see the note on `expected` above.
if [ "${#pins[0]}" -ne 7 ]; then
echo "::error::CI toolchain image pin ersatztv-ci:${pins[0]} is ${#pins[0]} chars, but ci-image.yml publishes 7-char tags (it tags with 'git rev-parse --short HEAD' from a fetch-depth:1 clone). A differently-sized abbreviation still resolves to the right commit, so this would pass every other check here — but NO such tag exists in the registry, and all five container: jobs would fail at image-pull time with 'manifest unknown'. Pin exactly: ersatztv-ci:${expected:0:7} (locally: git rev-parse --short=7 HEAD). See docs/ci-cd.md -> 'CI toolchain image'."
exit 1
@@ -121,7 +160,7 @@ jobs:
exit 1
fi
if [ "$pin_full" != "$expected" ]; then
echo "::error::CI toolchain image pin is stale: docker-build.yml pins ersatztv-ci:${pins[0]} ($pin_full), but docker/ci was last changed in $expected. Your jobs are testing an image that is NOT built from this PR's docker/ci. Let ci-image.yml publish the new :<sha>, then update the pin in ALL jobs to it (docs/ci-cd.md -> 'CI toolchain image')."
echo "::error::CI toolchain image pin is stale: docker-build.yml pins ersatztv-ci:${pins[0]} ($pin_full), but docker/ci was last changed in $expected. Your jobs are testing an image that is NOT built from this PR's docker/ci. Publish the new :<sha> — push this commit as branch HEAD and dispatch ci-image.yml on the branch (a branch PUSH no longer publishes, ersatztv#744) — then update the pin in ALL jobs to it (docs/ci-cd.md -> 'CI toolchain image')."
exit 1
fi
echo "Pin is current: ersatztv-ci:${pins[0]} resolves to $pin_full = docker/ci's last change."
@@ -134,16 +173,32 @@ jobs:
name:Docs update reminder
runs-on:small # seconds-long git diff; keep it off the build runners
if:github.event_name == 'pull_request'
env:
CI_JOB_ROLE:report-only
steps:
- name:Checkout
uses:actions/checkout@v4
with:
persist-credentials:false
fetch-depth:0
# `continue-on-error` for the same reason the two steps below carry it: this whole job
# is a non-blocking nudge, and an advisory red still joins the combined status the merge gate
# reads. Unmasking the fetch (ersatztv#746) makes a broken base LOUD in the log; it must not
# also make a warn-only job merge-blocking. The three jobs that genuinely gate on this diff —
# api-docs, format, decisions lifecycle — do redden on a failed fetch, which is where that
# belongs.
- name:Warn when a screen/route change skips the parity doc
echo "::error::git fetch of origin/${base_ref} failed, so this job cannot compute the changed-file set it derives its work from. That is a broken job, not an empty change set (ersatztv#746). Check the base branch still exists and that the runner can reach the repository."
exit 1
fi
if ! changed="$(git diff --name-only "origin/${base_ref}...HEAD")"; then
echo "::error::git diff against origin/${base_ref} failed, so the changed-file set could not be computed — do not read this as 'nothing changed' (ersatztv#746). If it reports no merge base, rebase this branch onto ${base_ref}."
exit 1
fi
echo "Changed files in this PR:"; printf '%s\n' "$changed"
screen_or_route=no
if printf '%s\n' "$changed" | grep -Eq '^web/src/screens/.+\.tsx$|^ErsatzTV/LegacyUiRedirects\.cs$'; then
@@ -159,6 +214,40 @@ jobs:
echo "Parity-doc reminder: nothing to flag."
fi
# ersatztv#784 — ADVISORY nudge for `docs.no-session-narrative`. Deliberately NON-BLOCKING and
# deliberately in this job rather than a gate of its own: it is a string predicate over prose,
# and `docs/defect-shapes-773.md` §4 argues that class must not be load-bearing. The script
# exits 0 on every path (asserted per argument shape in scripts/tests/test_check_doc_narrative.py,
# not only in prose), so this step cannot redden the run even on a hit; if you find yourself
# wanting it to fail, read the decision record first — it says no in as many words.
# `python3` is not guaranteed on the bare `small` lane (docs/ci-cd.md), and every other
# python-using job on it declares this. Without it a missing interpreter is exit 127 — a RED
# advisory job joining the combined status, which is the one thing this step must never be.
#
# Both steps OF THIS CHECK (setup-python + the narrative step; the parity nudge above has its
# own) carry `continue-on-error` because the SCRIPT exiting 0 is not the whole invariant:
# a setup-python download failure reddens the job just as effectively as a hit would, and an
# advisory red still joins the combined status the merge gate reads (ersatztv#598). Scope,
# stated rather than implied: this covers the two steps that exist to run the check. A failed
# `Checkout` is NOT covered and deliberately so — with no tree there is nothing to check, and
# a job that cannot run is a different failure from an advisory one that ran and disagreed.
# Measured on this runner (PR#811, run 2179): the job reports `success` and the commit status
# context is `success` with both steps green under `continue-on-error`.
- name:Set up Python
uses:actions/setup-python@v5
continue-on-error:true
with:
python-version:'3.x'
- name:Warn when a doc narrates its own revision history
continue-on-error:true
run:|
base_ref="${{ github.base_ref }}"
if ! git fetch --no-tags origin "$base_ref"; then
echo "::error::git fetch of origin/${base_ref} failed, so this job cannot compute the changed-file set it derives its work from. That is a broken job, not an empty change set (ersatztv#746). Check the base branch still exists and that the runner can reach the repository."
echo "::error::git fetch of origin/${base_ref} failed, so this job cannot compute the changed-file set it derives its work from. That is a broken job, not an empty change set (ersatztv#746). Check the base branch still exists and that the runner can reach the repository."
exit 1
fi
PYTHONPATH=. python3 scripts/decisions_validate.py --base "origin/${base_ref}" --head HEAD
@@ -83,11 +83,13 @@ main in) and re-run the local gate whenever the fetch shows movement.
Every task that closes a Gitea issue MUST complete ALL of these before it is considered done. Use `/done <issue>` to run through this automatically.
**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.
-`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|LGTM|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 credential you post with must be an account on `H10_REVIEWERS` in `.gitea/workflows/review-verdict.yml`** (`timothy` today) — since ersatztv#742 the gate inherits an existing `success` only from an allow-listed creator (an existing `failure` is left alone on a weaker attributability test, so an attributable rejection VISIBLE AT THE FIRST READ is not re-derived into a green — a rejection landing later, inside a run's own write window, is a separate and still-open route, ersatztv#849), so a verdict posted with any other account is written, reported as success by the script, and then silently re-derived on the next PR event (ersatztv#845).
- **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.
-`.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.**Since ersatztv#743 that push can no longer happen at all** (see below), so this hook is now belt-and-braces for a path the server refuses.
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.
**`main` is PR-only — there is no direct-push path any more (ersatztv#743, `release.main-direct-push-disabled`).** Branch protection carries `enable_push: false`**and**`block_admin_merge_override: true`: a direct `git push origin HEAD:main` is refused server-side at pre-receive for every account including a site admin, the contents API is refused too, and an admin cannot `force_merge` past a missing or red required context. This is what makes `review-verdict/h10` load-bearing rather than conventional — Gitea only evaluates `status_check_contexts` on the PR merge path, so before this the whole gate was skippable with no forgery. Practically: **every** change to `main` goes through a PR, including a one-line docs fix. Tag pushes are unaffected (separate mechanism), so the release cut is unchanged.
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 are exempt from the *review-verdict* gate; the direct-push exemption is moot now that direct pushes are refused outright.
**The 7 mandatory completion steps and the `## Closing record` comment template** live in the
`closing-an-issue` skill (`.claude/skills/closing-an-issue/SKILL.md`) — invoke it (or `/done`)
@@ -95,17 +97,30 @@ when finishing a task that closes an issue.
## Project Boundaries
**ersatztv OWNS**: ErsatzTV fork code (C#/.NET), channel/collection/schedule management, M3U/XMLTV generation, and the **`ersatztv` skill** — whose canonical copy is `.claude/skills/ersatztv/SKILL.md`**here**; `~/server-management/.claude/skills/ersatztv` is a symlink to it (ersatztv#617). Edit it in this repo; never fork a second copy.
**ersatztv OWNS** — *developing the fork*: the ErsatzTV fork code (C#/.NET), the `/api/v1` REST
surface, M3U/XMLTV generation, the `ErsatzTV.Mcp` server, CI and releases, and the **`ersatztv`
skill** — whose canonical copy is `.claude/skills/ersatztv/SKILL.md`**here**. Both
`~/server-management/.claude/skills/ersatztv` and `~/media-management/.claude/skills/ersatztv` are
symlinks to it (ersatztv#617, #755). Edit it in this repo; never fork a second copy.
**The split that is easy to get wrong** (ersatztv#755, `process.ersatztv-owns-code-not-operations`):
channel/collection/schedule *code* is owned here; **channel OPERATIONS against the running instance
are not**. Creating and editing channels, lineups, collections, schedules, playouts, logos and
overlays on the live ErsatzTV belong to `media-management`. Driving prod from here is in scope only
as *verification of a change this repo is shipping* (live-E2E, a release smoke test) — not as
day-to-day channel work.
**ersatztv does NOT own**:
- Channel/collection/schedule/playout **operations** against a live instance → media-management
- Jellyfin skill → server-management. `.claude/skills/jellyfin` here is a **relative symlink** to `~/server-management/.claude/skills/jellyfin` (ersatztv#617 — it had silently become a stale divergent copy). It therefore resolves only in a checkout at `~/ersatztv`, not inside a git worktree; that is inherent to the cross-repo symlink pattern server-management already uses (`beets`, `radarr`, `sonarr`, …).
**For infrastructure changes** (Docker, NFS, ports, Authelia): open an issue in `timothy/server-management`.
**For content/media sourcing questions** (what goes into channels, yt-dlp pipelines): open an issue in `timothy/media-management` once it exists; for now, `timothy/server-management`.
**For content/media sourcing questions and channel operations** (what goes into channels, yt-dlp
pipelines, editing a live channel): open an issue in `timothy/media-management`.
**For plan/audit reviews**: open `~/adversarial-reviewer` before significant architecture changes.
"description":"Extra surfaces in the QSV upload pool. Must be at least 64 when set; a smaller pool leaves no headroom for frames in flight and the transcode writes nothing at all. On update, a value equal to the one already stored is accepted unchanged, so a profile written before this validation existed stays editable.",
"format":"int32"
},
"resolutionId":{
@@ -25478,6 +25479,22 @@
"null",
"boolean"
]
},
"readRate":{
"type":[
"null",
"number"
],
"description":"Realtime pacing multiplier for the input. Unset keeps the built-in pacing (1.05, or 1.0 for a stream copy). Must be between 1.0 and 2.0 when set.",
"format":"double"
},
"readRateCatchup":{
"type":[
"null",
"number"
],
"description":"Rate a lagging realtime input may read at until it is level again. Unset keeps the built-in 6.0. Must be between 1.0 and 10.0, and GREATER than the read rate — equal is zero headroom, which is functionally no catchup.",
"format":"double"
}
}
},
@@ -26485,7 +26502,9 @@
"normalizeFramerate",
"normalizeColors",
"deinterlaceVideo",
"qsvPreferNativeDecoder"
"qsvPreferNativeDecoder",
"readRate",
"readRateCatchup"
],
"type":"object",
"properties":{
@@ -26604,6 +26623,20 @@
},
"qsvPreferNativeDecoder":{
"type":"boolean"
},
"readRate":{
"type":[
"null",
"number"
],
"format":"double"
},
"readRateCatchup":{
"type":[
"null",
"number"
],
"format":"double"
}
}
},
@@ -32016,6 +32049,7 @@
"null",
"integer"
],
"description":"Extra surfaces in the QSV upload pool. Must be at least 64 when set; a smaller pool leaves no headroom for frames in flight and the transcode writes nothing at all. On update, a value equal to the one already stored is accepted unchanged, so a profile written before this validation existed stays editable.",
"format":"int32"
},
"resolutionId":{
@@ -32103,6 +32137,22 @@
"null",
"boolean"
]
},
"readRate":{
"type":[
"null",
"number"
],
"description":"Realtime pacing multiplier for the input. Unset keeps the built-in pacing (1.05, or 1.0 for a stream copy). Must be between 1.0 and 2.0 when set.",
"format":"double"
},
"readRateCatchup":{
"type":[
"null",
"number"
],
"description":"Rate a lagging realtime input may read at until it is level again. Unset keeps the built-in 6.0. Must be between 1.0 and 10.0, and GREATER than the read rate — equal is zero headroom, which is functionally no catchup.",
// Level-2 explainer copy for the field-level progressive-disclosure pattern (ersatztv#734).
// Mirrors web/src/screens/FFmpegProfilesScreen.tsx's FIELD_HELP record; see
// docs/spa-conventions.md §15 for the contract this shape is mirroring.
constFIELD_HELP={
threadCount:
"Caps the worker threads FFmpeg uses per transcode. 0 lets FFmpeg decide, which is usually right; a low fixed value keeps one channel from starving the others on a busy host, at the cost of falling behind realtime on heavy content.",
scalingBehavior:
"Decides what happens when the source aspect ratio does not match the preferred resolution. Scale and Pad keeps the whole picture and adds bars; Crop fills the frame and cuts whatever overflows; Stretch fills it by distorting the image, which is why it is rarely what you want.",
videoBitrate:
"Target output bitrate. Too low and the encoder throws away detail on motion; too high and clients on slow links buffer. Buffer size is the companion setting — it bounds how far the encoder may deviate from this target.",
videoBufferSize:
"How much bitrate deviation the encoder may bank before it has to correct. Roughly 2x the bitrate is the usual starting point. Very small values force a near-constant bitrate and hurt quality on scene changes.",
hardwareAcceleration:
"Offloads decode and encode to the GPU. The list only offers what this FFmpeg build supports, so an unsupported kind never appears here. What nothing checks when you save is whether the device itself is present and passed through to the container — that is the mismatch that fails at playback time.",
normalizeLoudnessMode:
"Levels volume across content from different sources. Off leaves each item at its own level, so volume jumps between them; loudnorm retargets everything to one integrated loudness, which evens that out at the cost of an extra filter in the graph and a less faithful dynamic range.",
};
// Mockup of the shared `FieldHelp` primitive (web/src/components/fieldHelp.tsx). The prototype
// reproduces the two opening signals that read in a static review (tap-pins and hover); the shipped
<Rowlabel="Hardware acceleration"control={360}detail={FIELD_HELP.hardwareAcceleration}help="Requires the device to be passed through to the container.">
| Adding a ChicoryTV SPA screen | `docs/spa-conventions.md` |
| Explaining a consequential settings field in the SPA (summary → hover/tap panel → docs link) | `docs/spa-conventions.md` §15 — use the shared `FieldHelp` trigger and put the copy in the screen's own `FIELD_HELP` record; the icon, the gesture and the a11y contract are fixed |
| Graphics element / overlay work (text bug, On Now / Next, watermark-vs-`[vge]`) | `docs/graphics-elements.md`, then decisions catalog rows keyed `graphics.*` |
| Adding or changing a paged list handler (a page plus a `TotalCount`) | Resolve `api.paged-count-matches-page-query` via `docs/decisions/README.md` — for an EF-backed filtered list, count the SAME query you page, with includes appended to the page chain only; where the count and the page are separate methods, a test pins their agreement. Then `api.paging-zero-based` for the `pageNum`/`pageSize` contract |
| Auth / security-surface work | `docs/decisions/api-auth-security.md` |
| CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` |
| Proposing a new guard / CI check / regression test convention | `docs/defect-shapes-773.md` §4 (detector menu + the classes where no detector is plausible), then the rules every guard must satisfy: `docs/decisions/records/testing/guard-derives-population-from-source.md`, `…/guard-ships-with-mutation-proof.md` and `…/mutation-claims-are-executed.md` (a `MUTATION` grade carries a DECLARED clause mutation that is re-run every suite) — plus `…/verification-code-needs-its-own-proof.md`, which extends the same obligation BEYOND guards to the harness, wrapper or checker doing the checking, and says where its proof lives when the checker holds no row |
| Adding or bounding a consequential numeric config field (an FFmpeg profile tunable, a pipeline knob) | `docs/api-conventions.md` §3d — reject out of range with a 422 naming the bound and its consequence, never accept-then-rewrite; validate against the constants the renderer reads, keep the render-time clamp for pre-existing rows, and let an UNCHANGED legacy value through on update. Then `api.ffmpeg-profile-numeric-bounds` |
| Testing a surface gated by config / an env var / a credential | `docs/decisions/records/testing/deny-path-at-production-config-value.md` — cover the setting absent, at its production value, and each opt-out, and assert the DENY branch |
| Touching a full-replace write path or a hand-built request object | `docs/decisions/records/testing/full-replace-asserts-field-list.md` — derive the field list from the DTO and assert set equality; reconcile by id where child state exists. In the SPA the same rule is enforced by the type system: `docs/spa-conventions.md` §4b — build the body as `Complete<T>`, annotating BOTH the wrapper parameter and every construction site |
| Writing or editing any doc, or answering a review finding in prose | `docs/decisions/records/docs/no-session-narrative.md` — the doc records the END STATE; the path to it goes in the commit message. Apply the who-benefits test, and read the carve-out before you cut (dated measurements, stated snapshot boundaries and tested-and-rejected results stay) |
| Adding, renaming or removing a workflow JOB | `docs/ci-cd.md` → "Per-job declarations" — every job declares `env.CI_JOB_ROLE` (and, in `docker-build.yml`, `env.CI_EXECUTION_CLASS`); a missing or unknown value fails `scripts/tests/test_workflow_job_guards.py` / `…/test_ci_image_pin_population.py`, and a `guard`**or `report-only`** job also needs a row in `docs/guard-inventory.md` → "Workflow-job guards". Rationale: `docs/decisions/records/testing/workflow-declares-its-own-job-metadata.md` |
| Adding / changing / deleting a guard file | `docs/guard-inventory.md` — every guard's row is machine-checked by `scripts/tests/test_guard_inventory.py`, so a new guard must acquire a row before the suite goes green, and a row graded `MUTATION` must also acquire a declared clause in `scripts/tests/mutation_manifest.py` |
| Writing code that reads live Gitea/remote state and then acts on it | `docs/decisions/records/process/check-and-use-pins-a-version.md`, then `docs/remote-state-inventory.md` — a new executable under `scripts/` (**excluding `scripts/tests/`**), `.claude/hooks/`, `.husky/` or `.gitea/workflows/` must acquire a row there before `scripts/tests/test_remote_state_inventory.py` goes green |
| Finding every site that references a symbol (multi-site fix/sweep) | `docs/local-lsp-tooling.md` — which of the three surfaces answers, and why a delegated agent must be pointed at an MCP server (`csharp-lsp`, or `serena` after an `activate_project`) rather than the `LSP` tool, which no subagent has been observed to reach |
| Live local run / Playwright-MCP verification | `docs/e2e-local.md` + `scripts/e2e-local.sh` |
| What does a test suite cover | `docs/testing.md` |
| Writing a test whose behaviour is PROVIDER-SPECIFIC (collation, a value converter, data-migration DML) | `docs/testing.md` → "Provider-parity fixtures (opt-in MySQL)" — run one fixture body against both providers via `ETV_TEST_MYSQL_CONNECTION`; without it the MySQL arm `Assert.Ignore`s visibly, and CI does not currently run it (ersatztv#627) |
| "Why do we do X this way" / challenging a convention | **Catalog-first**: `docs/decisions/README.md` (active rows) → follow the row's link to `docs/decisions/records/<area>/<topic>.md` for full rationale. `docs/decisions/archive/<area>/` only for "what did the rule used to be." |
that file's standing kickoff for the two concurrent tracks (orientation ‖ selection). ersatztv#237
is a closed, archival historical tracker (superseded by `startup.parallel-orientation` in
`docs/decisions.md`) — not a live pointer.
- **`docs/defect-shapes-773.md`** — root-cause analysis of the recurring defect shapes across the
whole closed-issue corpus (#773): the measured class ranking, the four families they consolidate
into, the cheapest mechanical detector per class, the classes where **no** detector is plausible,
and an audit of which configured hooks/MCP servers/LSPs are actually invoked. Read it before
proposing a new guard or CI check — §4 is the detector menu, and it argues against enumerating
cases one incident at a time.
- **`docs/remote-state-inventory.md`** — every executable in `scripts/` (**excluding
`scripts/tests/`**), `.claude/hooks/`, `.husky/` and `.gitea/workflows/` that reads live remote
state and acts on that read, classified `PINNED` / `CAS` / `UNSAFE-KNOWN` / `N/A` with the window
and what bounds it. Code outside those directories — C#/TypeScript guards, `web/`, and the test
suites themselves — is out of scope, and the doc states that rather than implying coverage.
The population is derived from `git ls-files` and compared for set equality by
`scripts/tests/test_remote_state_inventory.py`, so a new script that talks to a remote service
cannot ship unclassified. Read it with `process.check-and-use-pins-a-version`; it is that record's
detector, since the class has no plausible linter (`docs/defect-shapes-773.md` §4 detector D).
- **`docs/guard-inventory.md`** — every executable guard file, what it blocks, whether it is a
`GUARD` or `TOOLING`, and whether it ships a mutation proof (`MUTATION` / `BEHAVIOUR-ONLY` /
`NONE`) with a `file::function` ref. The population is derived from the GIT INDEX (not a
filesystem walk, since ersatztv#806) and the workflow/hook call sites, and compared for set
equality by `scripts/tests/test_guard_inventory.py`,
so a new guard cannot ship unclassified and a renamed test cannot leave a row claiming coverage it
has lost. Guards implemented inline in workflow YAML are deliberately outside that population —
the doc states the limit rather than implying coverage.
- **`docs/tracker-retrofit-triage-237.md`** — audit trail for the #524 triage of ersatztv#237's 111
comments (method, per-comment classification, totals). Evidence for the
`docs.tracker-comment-retrofit` decision; read it only when triaging another over-cap tracker.
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
renovate