3bbe4f0635e26d210f02e2ad17dfe69df7d3ff35
239
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3bbe4f0635 |
fix(845): "14 of 36" was a grep LINE count, and alignment ADDS a named failure rather than replacing them
Seventh cold review. Three findings, all in prose I wrote correcting earlier prose.
HIGH — "14 of 36 did" is not a failure count. `grep -c` over pytest output counts LINES, and
`--tb=line` prints each failure twice, so 14 was 7 failures doubled; only 5 distinct tests are
involved, and with truncation disabled the figure moves the other way (31 of 36). Measured:
raw grep -c 14
of which '^E ' rows 7
of which '^/' tb=line rows 7
DISTINCT failing tests 5
A number that changes with pytest's display options is not a fact about the code, so no figure
is given at all now — which is what the same comment already said it was doing about counts.
The trap itself is recorded so one is not added back.
MEDIUM — "ONE named guard failure INSTEAD OF half the suite reddening" asserts a substitution
that does not happen, and contradicted the sentence directly above it. Measured on the aligned
tree: the guard failure appears AND the ~36 unrelated reds still appear, in the same job.
Alignment adds a diagnosis; it replaces nothing. Now "ALONGSIDE".
MEDIUM — the `set -u` sweep was incomplete, and the previous commit message's "gone repo-wide"
was false. Four files still asserted the refuted mechanism. The sharpest was
`scripts/check-review-verdict.sh`, whose comment said "see the sentinel comment in the library"
— and following that pointer landed on the retraction saying the opposite. Corrected there, in
`docs/ci-cd.md` and in `test_review_verdict_vocabulary.py`, all three already touched by this
branch. Every remaining mention in code, tests and docs is now a quotation INSIDE a retraction;
the only standing assertion left is the decision record, which is its own change and stays with
ersatztv#886 (now narrowed to exactly that).
CORRECTION to the previous commit message: it said the retracted wording was "gone repo-wide".
That was true only of the two phrases the round-6 reviewer checked, not of the mechanism. It is
true now for code, tests and docs.
Also: dropped a "for the same reason" back-reference that resolved to the wrong reason (drift,
where the actual reason is falsity), and removed the session narrative from the figure notes —
"a first attempt said 14" is autobiography; the durable form is the trap and its consequence.
Verified again on this tree: full suite 1278 passed / 2 skipped, catalog in sync,
decisions-validate OK, doc-narrative clean.
fixes #845
refs #886
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
06873a0113 |
fix(845): "no diagnostic naming a second assignment" was false — the failures DO name it
Sixth cold review. One HIGH, and it is the sentence the previous commit rewrote.
The claim, in five sites across three files, was that a second anchored `H10_REVIEWERS=`
reddened the writer suite "with no diagnostic naming a second assignment". Measured: 14 of the
36 failures carry this library's own refusal, verbatim —
E AssertionError: h10 reviewers: .../review-verdict.yml carries 2 anchored
H10_REVIEWERS assignments, not exactly 1 — refusing to guess which one the gate runs with
— which names both the count and the variable. The message predates the alignment commit that
introduced the claim, so it was false when written, not newly broken here. What is actually
missing is a failure that POINTS AT THE WORKFLOW: the reds surface under unrelated test names
(`test_positive_verdict_posts_success_on_the_resolved_head`, …) and the cause has to be
inferred from a stack of them. Aligning the populations buys ONE named guard failure instead.
All five sites now say that, and "about half the suite" replaces the earlier "the writer suite"
where it read as all of it (36 of 75).
Two nits from the same round: the aside inserted into `docs/ci-cd.md` turned an em-dash from an
introducer into a closing parenthetical and left the enumeration dangling — the hedge now
follows the sentence instead of splitting it; and "on that ground alone, which is the one that
was measured" implied a contrast that does not exist, since the retracted ground was measured
too. Trailing clause dropped.
CORRECTION to the previous commit message, which no artifact carries but which was wrong:
it said "Removing the guard reddens the new rejection test; `if true` reddens 20 tests",
pairing one mutation with two counts. Measured on the load guard: `if true` reddens 1
(the rejection test), `if false` reddens 20.
The behaviour was attacked again and held: BLOCKED end to end with the library DELETED gives
rc=0 with status and comment posted and an empty stderr, MERGEABLE on the same tree dies before
any write, and the declared mutation still reddens its named proof with the manifest's `expect`
string. The reviewer also confirmed the retracted `set -u` wording is now gone repo-wide.
fixes #845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
94cc924c36 |
fix(845): the retracted set -u mechanism survived seven lines below its own retraction — and the no-set -e case is now measured
Found by sweeping the RETRACTED WORDS repo-wide rather than re-reading the file I had edited,
which is the `stale-comments-sweep-by-subject` lesson the previous round already charged me
with once.
`scripts/lib/review-verdict-vocabulary.sh` still said "A control-flow gate can be skipped by
an abort" seven lines under the paragraph retracting exactly that mechanism — the same
self-contradiction shape as the sentinel comment in the last round, in the file that round
corrected.
MEASURING IT PROPERLY CHANGED WHAT I CHECKED. The decision record justifies the concern by
noting the READ side deliberately runs without `set -e`, and both prior reviewers had measured
the claim only under `set -euo pipefail`. If the original reading held without `set -e`, the
correction shipped last round would itself have been wrong. Measured on bash 3.2.57 and 5.3.15
across all three modes:
[set -u] in f / DEFINITELY_UNSET: unbound variable / exit=127
[set -eu] in f / DEFINITELY_UNSET: unbound variable / exit=1
[set -euo pipefail] in f / DEFINITELY_UNSET: unbound variable / exit=1
No branch taken and nothing after the gate, in every mode — including the one the record
relies on. The correction stands. The old reading holds only inside a command substitution
(`v=$(f) || v=FALLBACK` continues, fallback taken), which is not the shape gating either file.
WHAT IS NOT IN QUESTION, stated because the retraction is easy to over-read: the fail-open
that record documents is REAL — with `.*` in the positive list and the negative list written as
a scalar, an explicit `BLOCKED @ <head>` classified `positive`, exit 0. Only the attributed
mechanism is wrong: `${#name[@]}` on a scalar yields 1 rather than erroring, so no abort
occurred in that scenario at all. The sentinel is still the right design, on the ground that
survives — the caller that never runs validation.
The same refuted mechanism remains in `release.verdict-vocabulary-shared` and in
`test_review_verdict_vocabulary.py`. Correcting a decision record is its own change with its
own review surface, so it is tracked as ersatztv#886 with the full measurement, and
cross-referenced from the corrected file rather than folded in here.
fixes #845
refs #886
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
0f4bb4244d |
fix(845): the invariant that replaced a drifting count was itself false, and two of the counts survived
Fifth cold review, on the previous fix commit. Every finding is prose, and three of them are
claims that commit's own message said it had fixed. On a branch whose stated thesis is "four
prose claims were falsifiable by grep", shipping four more is the finding. Each verified here
before acting.
- "replaced in all four places" was wrong: there were FIVE occurrences of `31`. Two survived,
one of them four lines below the sentence the same commit added saying the number drifts.
- "the WHOLE writer suite reddens" is measurably false. Injecting a second anchored
`H10_REVIEWERS=` gives 36 failed / 39 passed — a large part, not all. The commit invoked
`state-the-invariant-not-the-measurement` and then wrote a claim STRONGER than the count it
removed. Now stated without a count and without "whole".
- The sentinel re-attribution retracted the environment-defeating ground in one paragraph and
still asserted it seven lines below ("the two that were measured"). The sibling library
corrected in the same commit got this right ("the ground that survives", singular). This is
`stale-comments-sweep-by-subject`: after a retraction, grep the retracted words.
- "spans THREE top-level directories" replaced a TRUE hedge with a false count, and did not
match the workflow it was supposed to be synced with (which says "at least three"). The
suite reads seven — `.claude`, `.gitea`, `.husky`, `docs`, `ErsatzTV`, `scripts`, `web` —
so this PR's own doc edits are inputs to that job. Hedge restored, with the reason.
Also from that review: the `if [ "$state" = "success" ]` body was unindented at column 0, and
"Fails CLOSED on every branch" now means every branch OF THAT LOAD, since the load is itself
conditional. Both fixed; the indent change is whitespace-only (`git diff -w` empty).
The behaviour was attacked again and held. The reviewer traced a BLOCKED verdict end to end
with `scripts/lib/h10-reviewers.sh` DELETED — rc=0, status and comment both posted, no unbound
variable and no command-not-found, because `$state` is set by the `case` well before the guard
and every reference to the library sits inside one of the two `state = success` blocks. An
APPROVAL on the same tree still refuses before writing anything. Removing the guard reddens the
new rejection test; `if true` reddens 20 tests. The declared mutation still reddens its named
proof.
fixes #845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
f54667cc5a |
fix(845): a declared security invariant had no guard, and four prose claims were falsifiable by grep
Fourth cold review, worktree-isolated, on the rebased head. Every finding reproduced before
acting; the two behavioural ones were re-verified with the reviewer's own mutations.
MEDIUM — "no environment variable selects the source file" was declared as a deliberate
security decision, in the library header AND in `docs/remote-state-inventory.md`, with no test
anywhere. The property held, but restoring the override the way a refactor naturally would
(`ETV_H10_REVIEWERS_WORKFLOW="${ETV_H10_REVIEWERS_WORKFLOW:-}"`) left the whole suite green
while an env var installed an arbitrary allow-list and an off-list account was accepted as a
reviewer. That is the class this branch removed in `28aab4632`, in a clause labelled as a
decision rather than as untested. Now pinned, and confirmed to redden under that exact mutation.
LOW (behavioural) — a BLOCKED verdict was refused when the allow-list could not be DERIVED.
Membership never applies to a `failure`, but the derivation was loaded unconditionally, so a
workflow with a broken `H10_REVIEWERS` declaration refused rejections too — the exact outcome
the success-only rule exists to prevent ("no supported way to record a rejection"), reached one
condition earlier, on the branch most likely to have broken that declaration: the one editing
it. The load is now guarded by state; both of its stated properties (after argument validation,
before the first network call) survive. A test covers the rejection AND the approval on the
same tree, so the guard was narrowed rather than removed.
Prose, all four falsifiable by `grep` or `pytest`:
- The comment claimed `review-verdict.yml` "rejected a hardcoded limit for its own read". It
does not — it sends `?limit=100` on both reads. What it rejected was using that limit as a
COMPLETENESS argument, which was dead code because the instance caps at 50. The real point
(presence vs absence) was right and is kept; a reader who greps the workflow no longer finds
the opposite of what this says.
- "31 red writer tests" had drifted to 40 INSIDE this branch as tests were added, in all four
places the previous commit said it had restated. Replaced with the invariant — the whole
writer suite reddens — per `state-the-invariant-not-the-measurement`, which is exactly the
lesson the stale number re-taught.
- The sentinel-reset comment cited a measurement that does not isolate it: removing the reset
produces a byte-identical refusal, because `load` overwrites both variables on that path. The
reset's real ground is the caller that never reaches `load`, which is what `contains`
returning 2 rests on. Re-attributed.
- `docs/ci-cd.md` still mirrored a TWO-directory input set for `script-tests` after this same
PR widened the workflow's own comment to three. Synced.
Also: `scripts/lib/review-verdict-vocabulary.sh` still shipped the `set -u` claim this branch
demonstrated to be wrong, one file over, and it was the source the new library copied it from.
Corrected there too — measured on bash 3.2.57 and 5.3.15, the shell exits outright in both the
`if ! f` and `f; rc=$?` shapes; the old reading holds only inside a command substitution. Its
sentinel keeps the ground that survives.
Nits: a stale "~10s" runtime in a sentence this branch rewrote, replaced with the reason that
does not drift; an unresolvable breadcrumb citing a private memory filename rather than a
decision key, dropped; "so only this run writes it" narrowed to what the description actually
identifies; and the paging test's docstring now says it is a POSITIVE CONTROL rather than
claiming a property pin it cannot deliver.
fixes #845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
a55f47023e |
fix(845): a cross-family review and a fix-commit re-review — the .state fallback recreated #845, and the paging model was backwards
Two more independent cold reviews: GPT-5.6 via Codex (cross-family) and a worktree-isolated Opus re-review of the previous fix commit. They converged on the paging defect from different directions. Each finding below was reproduced before acting. HIGH — the `(.status // .state)` fallback recreated the bug it was added to prevent. The gate reads `.status` and nothing else. On a response carrying only `.state`, the fallback made THIS tool accept and report success while the gate saw no recognisable verdict and re-derived it — which is #845 exactly, reached through the check meant to stop it. My comment claimed it "degrades to working"; it degrades to the bug. Removed: the writer now reads exactly what the gate reads, and a shape neither understands is refused loudly. MEDIUM — the allow-list was applied to `failure` too, which the gate deliberately does not do. `review-verdict.yml` inherits a `success` only from a member but a `failure` from any ATTRIBUTABLE account, and says so at length: inheriting a rejection can only withhold an exemption, while re-deriving one can turn it green. So refusing an off-list `BLOCKED` refused a verdict the gate honours, told the reviewer their rejection would be re-derived when it would not, and — because the comment is withheld on refusal — left a real reviewer no supported way to record a rejection at all. Membership is now required for `success` only. MEDIUM — the paging model was backwards, and so was the reasoning built on it. Gitea selects each context's MAX row id, orders those DESCENDING, and paginates that. Measured: a head with ids [17,19,...,41,43] returns 41 and 43 at `?limit=2`, and on another head the h10 row appears at `limit=9` but not `limit=8`, exactly its rank by recency. The status this script has just POSTed is the newest on the head, so it sorts FIRST and page 1 always holds it. The shim appended it LAST and sliced from the front, which manufactured a truncation the server cannot produce — so `test_a_head_carrying_more_contexts_than_the_server_cap_refuses` asserted a fiction, and the `?limit=100` "witness" was circular: observable only because the fixture was wrong in precisely the direction that made it observable. The shim now orders newest-first, that test is deleted, and `?limit=100` is kept as declared-unwitnessed insurance rather than dressed up as a tested guard. Also corrected, each verified first: - The `readback_rows` fallback was unreachable and its test never reached the branch it named. The fallback is gone; the test now asserts the message an empty body actually produces. The blank-count bug was real and was fixed by the restructure, not by the clause I credited. - "A repo-wide merge outage" overstated the guard's role. Such an edit landing WOULD be one, but `script-tests` is ADVISORY — measured, the required contexts are `Build & test`, `EF migration integrity` and `review-verdict/h10` — so the writer suite already reddens the PR. What alignment buys is a diagnostic naming the cause instead of 31 unrelated failures. Restated in all four places, and connected to the snapshot boundary, which is the route by which the outage is actually reachable. - The IFS test's docstring blamed the wrong split. `load` stores the RAW string, so its pin affects validation only; membership is the `contains` split, and only that one is witnessed. - The two `|| die` guards on already-constrained jq output are declared unreachable rather than counted as protection, and a multi-document body now refuses instead of printing "0 0 separate rows". - Why the writer may hardcode a limit where the gate deliberately refused to: the gate must prove a verdict ABSENT, which no single page can establish; this only confirms a row it just wrote is PRESENT. Presence needs no completeness argument. CORRECTION to the previous commit message: it claimed four clauses were made witnessed, including the `set -f` restoration. Measured on that predecessor, `set -f` restoration already reddened its named test at both sites, and the one genuinely unwitnessed `set -f` — the `contains`-side pathname guard — still is, correctly, since the validator excludes every metacharacter before it runs. Three, not four. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW |
||
|
|
04a3752432 |
test(845): the two identify-our-own-write arms masked each other; grammar in the library header
Caught while a cross-family review was probing the same spot, and confirmed before fixing: `test_a_status_OVERWRITTEN_...` used a fixture differing in BOTH state and description, so either arm of `[ state != ] || [ desc != ]` could be deleted with the suite still green. Each arm was individually disarmed and the suite stayed at 69 passed both times — #685's duplicate-guards-mask-each-other shape. Each arm now has a fixture differing in exactly one field, and both were confirmed to redden their own named test. The creator in each is deliberately ALLOW-LISTED, so the refusal has to come from identifying the row as not-ours rather than from its author. The script now also states WHICH arm carries the safety, matching how this file already handles its overlapping `-z`/`-n` arms: the DESCRIPTION is the identifier (verdict word, short sha, base — only this run writes it), and the STATE is defence in depth, since our own writer cannot produce our description with a different state. Also: apostrophes restored in four library header sentences that read as typos. fixes #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW |
||
|
|
e3a19227c1 |
fix(845): two independent cold reviews — the guard did not walk the same tree, and CLAUDE.md said the status is not written
Two cold reviewers, independently briefed and worktree-isolated. One returned BLOCKED on the
first two below; the other returned MERGEABLE having not examined them. No arbitration was
needed — both are verifiable, and I reproduced each before acting.
BLOCKING 1 — the CI guard parsed a different population than the library.
`test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty` read `_classify_step()["run"]`, one
step's body. `scripts/lib/h10-reviewers.sh` greps the WHOLE YAML and refuses on anything but
exactly one match. So a second anchored `H10_REVIEWERS=` in ANY other `run:` block was
invisible to the guard while making the writer refuse every verdict — and `main` is PR-only
with `review-verdict/h10` required, so that is a repo-wide merge outage. Reproduced: injecting
one into the last step left the guard green and reddened 31 writer tests whose diagnostics
said nothing about a second assignment. The guard now reads the whole file, so it walks the
same tree as its subject; the header and the decision record no longer claim coverage that
did not exist.
BLOCKING 2 — `CLAUDE.md` said an off-list verdict "fails loudly at your terminal instead of
being written". The status IS written; only the comment is withheld. The script, `ci-cd.md`
and the record all said so correctly, and the most-read doc contradicted them. It now also
says what the other docs did not: the green left standing satisfies the server-side required
check, because branch protection binds the context NAME and not its issuer.
Four clauses were shipped UNWITNESSED, found by the reviewer's own disarm matrix and each now
reddening a named test: the `.statuses` array-TYPE test, the read-back GET's explicit `|| die`,
`?limit=100`, and the `set -f` restoration. The first two passed only because a downstream
check produced the same non-zero exit — the shape this very file already documents at the
`-z`/`-n` arms. They now assert their own diagnostic. `?limit=100` was unobservable because
the shim discarded the query string; the shim now models the measured server (default 30,
clamped to 50, `total_count` reporting the page).
Also fixed, all reproduced first:
- `[0]` over same-context rows had no uniqueness guard. Unreachable today, but the
alternative is judging one row's author while another is the one standing.
- The row count in the not-found refusal came out BLANK on a 200 with an empty body: jq
exits 0 with empty output, so the `||` fallback was dead exactly where the count matters.
Tests the value now, not jq's exit.
- The split pinned globbing but not IFS. A caller with `IFS=-` made `renovate` read as a
member of `renovate-bot`. Unreachable from this script (bash resets IFS), pinned because
the header invites reuse.
- The `set -u` rationale was inherited and is WRONG. Both reviewers measured it: the shell
EXITS, in both the `if ! f` and the `f; rc=$?` shapes, on bash 5.3 and 3.2. Nothing here
depended on it; the sentinel now rests on the two grounds that were measured.
- The `?limit=100` margin was overstated. This instance clamps to MAX_RESPONSE_ITEMS = 50,
so the headroom is ~3x over today's 8-15 contexts, not 6x, and it shrinks as checks are added.
- The snapshot boundary now names its DIRECTION in all three places: it is a false ACCEPT,
and the likely edit (ADDING a reviewer) is exactly the one it mis-handles.
- The overwrite refusal told the operator to re-review when a gate re-derive needs no review.
A note on the measurements, since two of my own disarms were inert and briefly read as
"no test covers this": a mutation must be checked to actually mutate. The glob-class disarm
still excluded `*`, and the IFS disarm matched two sites and applied to neither.
fixes #845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
5f93930a18 |
test(845): the library's own clauses were untested — four of them, including the glob fail-open
Found by disarming the LIBRARY's clauses rather than the script's. The end-to-end tests only ever drive the real workflow, which is well-formed, so four clauses could be removed with the suite still green: - the plain-login validation that rejects a glob entry - the sentinel gate that makes `contains` answer CANNOT-TELL instead of NOT-A-MEMBER - the empty-list check - the restoration of the caller's `-f` setting after the split The first is the one that matters. `for x in $list` performs PATHNAME EXPANSION, so a `*` entry expands to the FILENAMES around it, and any of those matching the plain-login class would validate cleanly and enter the allow-list as a real reviewer. `review-verdict-vocabulary.sh` records exactly this happening to its own word list; the new test stocks the working directory with plausible login names so an expanding implementation loads a list instead of refusing. The sentinel one is a wrong-cause defect rather than a fail-open: without it a broken checkout answers NOT-A-MEMBER, and the script then tells the operator to fix a credential that is fine. A note on the measurement, because the first attempt was wrong. The initial disarm of the plain-login class widened it to `[!A-Za-z0-9._!@#$%^-]`, which still excludes `*` — so it mutated nothing and the "no test covers this" reading was an artifact of an inert mutation. The real disarm has to ADMIT `*` (`[!A-Za-z0-9._*-]`), and the probe now asserts that the mutant class accepts `*` before drawing any conclusion from the run. All four confirmed to redden their own named test. fixes #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW |
||
|
|
ca38f285d5 |
docs(845): cite the measurement behind the latest-per-context claim instead of asserting it
The comment stated that `/commits/{sha}/status` returns the latest row per context as a bare
fact. It was measured — a head carrying two `review-verdict/h10` rows in the `/statuses` list
returned only the newer one from the combined endpoint — so the comment now says so. An
unattributed claim about an API is the kind that quietly stops being true and talks the next
reader out of re-checking it.
refs #845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
09dcfe76fb |
fix(845): each read-back extraction dies with its own message, and the no-creator refusal stops asserting a cause
Probed, not reasoned: a `creator` arriving as a STRING rather than an object makes jq exit 5
("Cannot index string with string"). That already failed closed — the abort lands before the
comment — but with only jq's stderr, which never says a status was left standing on the head.
Each extraction now carries its own message.
The no-creator refusal said the status "is how a status posted by an ACTIONS token appears".
Three shapes reach that branch (`creator` absent, null, or an empty login) and only one of
them is an Actions token, so the message now states the fact first and offers the cause as
the likely one rather than the established one.
fixes #845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
785edbe6e6 |
fix(845): the read-back asks for a page big enough, and stops naming a cause it cannot know
Second self-review defect, again found by probing rather than reading.
`GET /commits/{sha}/status` PAGES, and it pages undetectably: measured on Gitea 1.27.1
(2026-08-29), `?limit=2` against a head carrying 15 contexts returned 2 rows AND
`total_count: 2`. The count reports the PAGE, not the total, so no field in the body
distinguishes a truncated response from a complete one. A head whose context count grew
past the server's default page size would silently stop containing the `review-verdict/h10`
row, and the read-back would refuse a verdict that is actually in force — the deadlock
direction, on the one path that must not deadlock.
The call now asks for `?limit=100` against the ~15 contexts this repo produces.
The not-found refusal no longer states a cause. Two causes reach it — the status is gone,
or the list was truncated — and the response cannot tell them apart, so asserting either
would be the #859 defect: a message that sends the reader hunting something that did not
happen. It now reports how many rows it actually read and names neither, which also makes a
future paging overflow legible instead of mysterious.
Recorded in `docs/remote-state-inventory.md` on the writer's own row, since it is a property
of that read rather than of the tool.
fixes #845
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
|
||
|
|
8c580cbfcd |
fix(845): read .status, not .state — the fixture was mirroring my POST body, not Gitea
Self-review defect, caught by probing the live instance rather than re-reading the code. A row inside `.statuses[]` serialises its state under the key `status`. `state` exists only as the AGGREGATE at the top level of the combined body. Measured on Gitea 1.27.1 (2026-08-29) against a real `review-verdict/h10` row on PR #882's head. Reading `.state` per row therefore yields "" for every well-formed response, which compares unequal to the state just posted — so the previous commit would have refused EVERY verdict with "something overwrote it". Since no PR can merge to `main` without a `review-verdict/h10` success, that is a repo-wide deadlock, shipped green. WHY THE TESTS DID NOT CATCH IT, which is the more useful half. The `curl` shim replays the status POST payload as the read-back body. A POST body legitimately uses `state` — that is the field the write API takes — so the fixture agreed with the parser BY CONSTRUCTION and the pair was self-consistent and both wrong. A fixture derived from the code under test cannot disagree with it. The shim now renames the field on replay, and a separate test feeds a VERBATIM live-Gitea body so the real shape is pinned independently of the shim. Measured, both directions: with the fixture fixed and `.state` restored, 25 tests redden; before the fixture fix, zero did. The gate reads the same endpoint and the same key (`review-verdict.yml` line 520, `ex_state=$(... jq -r '.status // ""')`), so the writer and the gate are now wrong or right together rather than independently. `.state` is kept as a fallback so a server spelling it the other way degrades to working rather than to a deadlock; it is not a second source of truth, since the value still has to equal the state this run posted. A row carrying NEITHER key now gets its own message. It is a response-SHAPE problem, not a race, and "something overwrote it" would state a cause that did not happen — the #859 defect class. fixes #845 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW |
||
|
|
afe61e80aa |
fix(845): the verdict writer checks that the gate will honour what it just posted
`.gitea/workflows/review-verdict.yml` inherits an existing `review-verdict/h10=success` only from a status whose `.creator.login` is on its `H10_REVIEWERS` allow-list (#742). `scripts/post-review-verdict.sh` writes those verdicts with whatever account owns `ETV_GITEA_TOKEN`/`ETV_GITEA_BASICAUTH` and never asked whose it was. The two values were coupled with nothing asserting the coupling, and the failure mode was the silent one: the status really is written, the tool really does report success, and then the next `pull_request_target` event re-derives it and posts over it. And again. The PR deadlocks and the only diagnostic is a `::warning::` inside a workflow run nobody is reading. The script now READS THE STATUS BACK after posting it, identifies its own write by state and description, and refuses — before the comment, so the surviving half-state is the documented `ask` one — unless the recorded creator is on the allow-list. Measured after the write rather than probed before it: a pre-flight `GET /user` tests what the credential claims to be, this tests what Gitea recorded as the author of this status, which is the value the gate reads. It also needs no scope beyond the repo access the POST already required. The allow-list is not restated. `scripts/lib/h10-reviewers.sh` derives it from the workflow's own literal, so there is ONE declaration rather than two held together by a parity test — the shape `testing.guard-derives-population-from-source` records six failed rounds against. It is a parse rather than a shared declaration both sides source, and that is not a preference: the gate runs against a checkout of the PR's BASE sha, so a PR whose base predates a new `scripts/lib/` file would not have it, and a missing `source` under `set -euo pipefail` kills the job — which posts no `review-verdict/h10` at all and blocks every merge to `main` including the PR that would repair it, with no admin override (#743). A fallback list for that case reintroduces the second copy. So the derivation runs on the side that can fail safely and the gate's body is untouched. Fail-closed on every branch of the derivation: 0 or >1 anchored assignments, an entry that is not a plain login, an empty list, or an unreadable file all refuse BEFORE any network call. The membership query answers member / not-a-member / cannot-tell as three distinct codes rather than two, so a caller cannot fold "cannot tell" into either. The safety DIRECTION is unchanged and the commit does not claim otherwise: a non-allow-listed verdict was always a stall, never a fail-open. What changes is where the signal lands — at the terminal that made the mistake, instead of one PR event later. Residual, stated rather than fixed: the check runs after the POST, so a refused verdict leaves a non-inheritable status standing with no comment beside it. That is the `ask` half-state `release.verdict-writes-status-before-comment` already designates as the safe one, and the gate re-derives it on the next PR event. A second corrective write is the sticky-sentinel mechanism #849 is separately designing; inventing a parallel one here would be two mechanisms for one invariant. `scripts/post-review-verdict.sh` moves BEHAVIOUR-ONLY -> MUTATION in the guard inventory, which the manifest's own UNDECLARED note called "the most valuable upgrade on this list". The declared clause is in the GATE, not the script: rewriting `H10_REVIEWERS` while the posting account stays fixed reddens the accept path only if the writer reads the list live AND the comparison gates the outcome. Disarming the comparison instead would prove the second and say nothing about the first, which is the half that was missing. Verification: full `scripts/tests` suite green (1239 passed, 2 skipped); the declared mutation executes and reddens its named proof with the declared diagnostic; four clauses disarmed by hand and each confirmed to redden its own test. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW |
||
|
|
94a3d13495 |
fix(836): never pass --depth to a checkout that may already be complete (#884)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 11s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 32s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 16m47s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 9m7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 8m7s
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 2m0s
`git fetch --depth=N` grafts a complete clone shallow. `scripts/ci-detect-docs-only.sh` applied a depth chosen for its three `fetch-depth: 2` consumers to `build`'s `fetch-depth: 0` checkout, so the `git describe --tags` in the next step found no reachable tag and a `|| echo v0.0.0` fallback turned that into a version: every `:latest` image shipped `InformationalVersion 0.0.0-<sha>` from 2026-07-17 (#416) until now. Both fetch sites now go through `fetch_ref`, which passes `--depth` only when the checkout is already shallow. `Compute version and tags` fails the job instead of defaulting, so no `:latest` is published rather than a mislabelled one; releases are unaffected because the tag path never calls `describe`. Ships a guard that drives the real script over real `file://` clones with a negative control, a declared clause mutation, and a decision record `ci.fetch-depth-never-grafts-a-complete-clone`. fixes #836 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
e8f80c42ce |
fix(820): derive where Complete<T> is APPLIED, not just what it means (#883)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 9s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 28s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 14m22s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 11m1s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 9m58s
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 1m58s
`Complete<T>` (#807) makes SPA full-replace bodies fail typecheck when a builder omits a schema member. Nothing checked it was APPLIED: `completeRequest.guard.test.ts` proves the type's semantics and would stay green with every annotation deleted, and `test_optional_request_members.py`'s COVERED disposition — "the builder is annotated `Complete<T>`" — was a claim about another language's source that nothing verified. Adds `completeAnnotationScan.ts` (compiler-API scanners) + `completeAnnotations.guard.test.ts`, with a synthetic-source fixture suite. Two derived populations: the `Complete<…>` annotations (SPA AST ∩ git index) and the droppable schemas (parsed from the generated `v1.d.ts`, a pass-through of the OpenAPI `required` array). It asserts a production annotation per schema dispositioned as needing one, NO annotation on the server-computed and load-bearing-omission schemas, that every `Complete<X>` resolves to a generated schema rather than a hand-written mirror, and set equality between droppable schemas and the reviewed dispositions. `test_complete_annotation_dispositions.py` cross-checks that table against the authoritative Python one and ships a declared, harness-executed mutation. Found one live defect: `playouts.ts` declared two request types as hand-written mirrors SHADOWING generated schemas of the same name, so their `Complete<>` was checking a local copy rather than the contract — the #754 mechanism wearing the annotation meant to prevent it. Eight review rounds, seven BLOCKED, two independent cold reviewers. A wrapper-signature scanner was built and REMOVED: every blocker traced to that one mechanism (obligation on the wrong population; reachability mistaken for protection, since `Complete<T>` is shallow; body discovery keyed on a parameter name, then parameter-vs-local; and finally `export function` → `export const` blinding the scanner and its cross-check together). Five defects from one mechanism, so the mechanism went rather than a sixth patch. Residuals stated in §4b, the guard-inventory row and the record: per-SCHEMA not per-site or per-wrapper; token presence not liveness; the phantom direction unchecked (#777); a second `setupFiles` entry could discharge; and plugin-level population integrity borrowed from the sibling guard. fixes #820 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
736649b3b7 |
fix(812): classify the narrative sites by who-benefits; keep the detector's reach (#882)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 22s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m18s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m33s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m52s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
b6b3520bdb |
fix(809,822): isolate the suite from the production hook-fire log by construction (#874)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 8s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 26s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m32s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m12s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m14s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m32s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
90f96c14a5 |
fix(803,664): fence the HEAD alias on the PR timeline's pull_push count (#873)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 15s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 26s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m14s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m16s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m39s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m37s
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 ->
|
||
|
|
4b3f6f6c90 |
fix(786,789): workflows declare their own per-job metadata; guard populations derive from it (#872)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m41s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m21s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m58s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m8s
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> |
||
|
|
e11d577193 |
docs(796): verification code is code under test, and the proof it was claiming an exemption from (#871)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 8s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m38s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m9s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m50s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m18s
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> |
||
|
|
609fd852c2 |
fix(763): page both /statuses/{sha} reads to a validated terminator (#868)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 20s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m45s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m10s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m2s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m9s
|
||
|
|
11287a54ba |
fix(763): satisfy ruff — E741 and formatting on the touched test file
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 10s
review-verdict/h10 Review-verdict: MERGEABLE @ 11287a5 (base: main)
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 19s
PR Gates / Docs update reminder (pull_request) Successful in 16s
Review verdict / Set review-verdict status (pull_request_target) Successful in 20s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 17s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Successful in 6m51s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m24s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m5s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 6m6s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 7s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 5s
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>
|
||
|
|
5fb9c8537a |
docs(747): re-verify the Gitea 1.25.4-pinned CI claims on 1.27.1, and measure the merge-gate semantics that were source-attested (#867)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 10s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 17s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m20s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m25s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m48s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
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> |
||
|
|
462118d98d |
fix(763): round 9 — assert the repair's reason positively, not by absence
Build ErsatzTV Image / CI toolchain image resolves (pull_request) Successful in 10s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 22s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 17s
PR Gates / Docs update reminder (pull_request) Successful in 21s
review-verdict/h10 Review-verdict: MERGEABLE @ 462118d (base: main)
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 23s
PR Gates / Script lint and tests (ruff + pytest) (pull_request) Failing after 14s
Review verdict / Set review-verdict status (pull_request_target) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Canceled after 1m31s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Canceled after 0s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Canceled after 0s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Canceled after 0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Canceled after 0s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Canceled after 0s
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>
|
||
|
|
4164efbc3e |
fix(763): round 8 — a fixture must pose the scenario it narrates
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> |
||
|
|
e6f04cc739 |
fix(763): round 7 — reject an empty post-write history, and the fourth overclaim
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
|
||
|
|
89db6022d9 |
fix(763): round 6 — measure the partial-mark fallback under the REAL ordering
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>
|
||
|
|
53ee44cbc1 |
fix(763): round 5 — correct the overstated motivation, and fix the string-id twin
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> |
||
|
|
007d2fd3df |
fix(763): round 2+3 — close the fail-opens the paging change introduced
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> |
||
|
|
4368cc8cbe |
fix(763): page both /statuses/{sha} reads to a validated terminator
`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> |
||
|
|
761e575836 |
fix(787): derive the dropped-step guard's scope, and reconcile its snapshot against the server (#861)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 21s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m45s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m22s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m56s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m33s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
8aebba4d89 |
fix(748): declare permissions: on all six workflows, and prove the declaration binds (#860)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 7s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 18s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m40s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m11s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m4s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m20s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
45b17e58e1 |
fix(744,835): ci-image.yml publishes from main only; guard persist-credentials with no exemption (#857)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 23s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m53s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m23s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m15s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m27s
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 #744 fixes #835 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
d68ce42fcb |
fix(742): inherit an h10 verdict only from an allow-listed reviewer (#850)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 12s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 27s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m47s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m55s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m0s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m25s
`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> |
||
|
|
469d19852c |
fix(788): one declarative H10 verdict vocabulary, derived by both sides (#846)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m34s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m50s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m22s
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> |
||
|
|
f2551b778e |
fix(746): drop the persisted checkout credential; unmask the base-ref fetches (#842)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 24s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 30s
Build ErsatzTV Image / Build & test (.NET) (push) Failing after 1m37s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Build & push image (amd64) (push) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m1s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
3e2c07b42f |
fix(807): SPA full-replace bodies are built as Complete<T>, with a derived guard over droppable request members (#827)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 14s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 30s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m36s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m9s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m49s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 15s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
8c72a5de9e |
docs(772): correct the twins of two claims this branch already fixed
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
|
||
|
|
30640bb780 |
fix(772,792): close the fix-round review — the doc still described the semantics the fix reversed
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 |
||
|
|
0cf355e494 |
fix(772,792): address both cold reviews — an unverified pin is no longer a green job
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
|
||
|
|
5ba442c11c |
fix(772,792): name the missing toolchain image, and stop a refusal leaving a verdict comment
#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 #772 fixes #792 Refs: server-management#842 Decisions-Edit: yes |
||
|
|
29d7a06e65 |
fix(806): guard populations over FILES derive from the git index, not a filesystem walk (#818)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 27s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m53s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 5m57s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m56s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m20s
`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> |
||
|
|
3b386649bf |
feat(790): execute every MUTATION grade instead of asserting it (#815)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 18s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m2s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m30s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m9s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
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> |
||
|
|
eb5c74b4b2 |
fix(780): reconcile the ruff gate with #784, merged in parallel (#814)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 19s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m30s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m5s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m38s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m3s
#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> |
||
|
|
d4c72697f2 |
feat(780): commit a ruff config and enforce it in CI (#813)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 28s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m29s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 5m54s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m59s
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 15s
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
|
||
|
|
6a4265d81d |
feat(784): a doc records the end state — generalize the no-session-narrative rule (#811)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 40s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m30s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m16s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m52s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 17s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
706674272c |
test(785): mutation proofs for the three unproven ranked guards (#810)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 26s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m48s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m14s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m17s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m10s
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> |
||
|
|
7347a08979 |
fix(778): the 200-not-an-array arm had the same lying reason, one branch earlier
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 31s
Build ErsatzTV Image / Delimiter ban (release path) (pull_request) Successful in 43s
PR Gates / Docs update reminder (pull_request) Successful in 48s
PR Gates / Fix proofs (Proves trailers) (pull_request) Successful in 12s
PR Gates / decisions lifecycle (pull_request) Successful in 55s
Review verdict / Set review-verdict status (pull_request_target) Successful in 16s
PR Gates / Script tests (pytest) (pull_request) Successful in 4m1s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m59s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m17s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 5m51s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 7s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 5s
review-verdict/h10 Review-verdict: MERGEABLE @ 7347a08 (base: main)
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>
|
||
|
|
3f0af38509 |
fix(778): pin the nomatch sentinel — it shipped unpinned, exactly like the guard before it
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> |