timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

review-verdict.yml post-write verification: three routes that leave an exemption success over a human failure #849

New Issue
Closed
opened 2026-08-27 00:25:17 +02:00 by timothy · 5 comments
timothy commented 2026-08-27 00:25:17 +02:00
Owner
Copy Link
Copy Source

Raised by a cold cross-family (Codex/GPT-5.6) review of the #742 branch. All three are PRE-EXISTING on main, not regressions from #742 — verified by diffing the branch against origin/main: the three guards involved (if [ "$state" = "success" ] && [ "$max_id_before" -ge 0 ], the two "mark could not be established" warnings, and the unreadable-post-history warning) are byte-identical to main and #742 touches none of them. They are filed rather than fixed there because #742 is a provenance change and these are the post-write verification's own residuals.

Each ends with an exemption success standing over a real human failure — the outcome the workflow's own comments call "the worst outcome this gate can produce".

1. An unreadable history read leaves an exemption green over a raced rejection

review-verdict.yml — the post-write race check is gated on [ "$max_id_before" -ge 0 ].

  • If hist_before is unreadable or its max id is non-numeric, max_id_before=-1, and the job emits a ::warning:: and skips the check entirely. A reviewer's failure landing after the last-moment re-read is then overwritten by the exemption success and never repaired.
  • Symmetrically, if the post-write history read fails, the code logs "The exemption review-verdict/h10=success stands" and does exactly that.

Both are the fail-toward-SUCCESS direction, on the one path the design says must fail toward pending. The existing comment argues a read failure "does NOT fail the job" because the status is already posted — true, but the conclusion should be a sticky pending repair, not leaving the green.

Suggested: withhold the exemption success when no high-water mark could be established, and treat an unreadable post-write history exactly like the uncertain-pagination case already does — POST the sticky REPAIR_DESC sentinel, retry, and scream if the repair fails.

2. A pending write can bury a rejection, which then goes green on a later run

The post-write race check runs only if [ "$state" = "success" ].

Trace: a docs-only run hits a transient enumeration failure, so it selects the generic pending (not the sentinel). A reviewer posts failure between the last-moment re-read and the POST. The generic pending masks it, and no post-write verification runs because the state is not success. On a later event enumeration succeeds, the ordinary machine-written pending is re-derived, and the exemption success is posted — with the buried failure now below the new high-water mark, so it is never repaired.

This is the same shape as the #706 round-3 sentinel finding (a stale run burying a rejection), reached through the pending path instead.

Suggested: run post-write race detection after every status write, not only success. When a pending write raced an attributable verdict, rewrite it with REPAIR_DESC so it becomes the sticky fixed point the classifier already refuses to grant an exemption over.

3. The retarget fence is pre-POST only, so a stale exemption can become permanent

The fence compares the timeline's retarget count taken before classification against one taken before the write. Nothing re-checks after the POST.

Trace: main carries a real failure for head H. Retarget to a scratch base where H is docs-only; run S derives success and passes its final fence check. Retarget back to main while S is paused before its POST. Successor run M sees the base-matching failure and short-circuits, posting nothing. S resumes and posts its stale success. The rejection predates S's high-water mark, so post-write verification ignores it — and no event remains to reclassify. The forged green is permanent.

ci.verdict-write-retarget-fence describes the surviving window as the sub-round-trip one that no API without compare-and-set can close. This scenario says the window is wider than that: the fence narrows writing while overtaken, but not being overtaken after writing.

Suggested: re-count retargets after the POST. If the count moved, or cannot be trusted, immediately replace the status with the sticky pending sentinel — a retarget after that post-check then necessarily queues a successor that starts after the stale success exists and can re-derive it.

Testing note

The reviewer also observed that test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH asserts only a one-run payload from an initially statusless head, and that every post-write race test derives success — which is why the state = success gate in (2) survives the suite. A chained test is the right shape (this repo has the pattern recorded as fixed-point-properties-need-chained-tests): run N with an enumeration failure plus a raced human verdict, assert the sentinel repair; feed that resulting status into run N+1 with successful docs enumeration, and assert it stays the sentinel.

Done-when

Consolidated 2026-08-29 — the merge-consent hook reads THIS list, not the revisions that were
left in comments, so the two later lists (the 01:05 revision and the 02:57 addition) are folded in
here and the original eight are superseded. The comments stay as the record of how the scope moved.

  • Every write is verified when a high-water mark exists — not only state = success
  • A distinct STICKY "unverified write" sentinel when no mark can be established, so a later run cannot re-derive it into success
  • The stall cost is either given a retry/reconciliation path, or documented and accepted as human-required — not described as a one-run cost
  • Every parse/schema failure in the post-write jq resolves to raced=1 rather than killing the job or "not acting"
  • An unreadable combined-status read cannot leave a pre-existing off-list success standing
  • The post-POST retarget re-check exists (ci.verdict-write-retarget-fence's residual 1 was a PERMANENT green, not a sub-round-trip window)
  • A CHAINED test feeds run N's real output into run N+1
  • Each fix ships with a mutation proof that it reddens
  • The fence/pending guarantee PROSE is swept by CONCEPT (not by phrase) and corrected in the same PR as the behaviour
  • Adversarial review passed
Raised by a cold cross-family (Codex/GPT-5.6) review of the #742 branch. **All three are PRE-EXISTING on `main`, not regressions from #742** — verified by diffing the branch against `origin/main`: the three guards involved (`if [ "$state" = "success" ] && [ "$max_id_before" -ge 0 ]`, the two "mark could not be established" warnings, and the unreadable-post-history warning) are byte-identical to `main` and #742 touches none of them. They are filed rather than fixed there because #742 is a provenance change and these are the post-write verification's own residuals. Each ends with an exemption `success` standing over a real human `failure` — the outcome the workflow's own comments call "the worst outcome this gate can produce". ## 1. An unreadable history read leaves an exemption green over a raced rejection `review-verdict.yml` — the post-write race check is gated on `[ "$max_id_before" -ge 0 ]`. * If `hist_before` is unreadable or its max id is non-numeric, `max_id_before=-1`, and the job emits a `::warning::` and **skips the check entirely**. A reviewer's `failure` landing after the last-moment re-read is then overwritten by the exemption `success` and never repaired. * Symmetrically, if the **post**-write history read fails, the code logs "The exemption `review-verdict/h10=success` stands" and does exactly that. Both are the fail-toward-SUCCESS direction, on the one path the design says must fail toward `pending`. The existing comment argues a read failure "does NOT fail the job" because the status is already posted — true, but the conclusion should be a sticky `pending` repair, not leaving the green. **Suggested:** withhold the exemption `success` when no high-water mark could be established, and treat an unreadable post-write history exactly like the uncertain-pagination case already does — POST the sticky `REPAIR_DESC` sentinel, retry, and scream if the repair fails. ## 2. A `pending` write can bury a rejection, which then goes green on a later run The post-write race check runs only `if [ "$state" = "success" ]`. Trace: a docs-only run hits a transient enumeration failure, so it selects the **generic `pending`** (not the sentinel). A reviewer posts `failure` between the last-moment re-read and the POST. The generic `pending` masks it, and no post-write verification runs because the state is not `success`. On a later event enumeration succeeds, the ordinary machine-written `pending` is re-derived, and the exemption `success` is posted — with the buried `failure` now **below** the new high-water mark, so it is never repaired. This is the same shape as the #706 round-3 sentinel finding (a stale run burying a rejection), reached through the `pending` path instead. **Suggested:** run post-write race detection after **every** status write, not only `success`. When a `pending` write raced an attributable verdict, rewrite it with `REPAIR_DESC` so it becomes the sticky fixed point the classifier already refuses to grant an exemption over. ## 3. The retarget fence is pre-POST only, so a stale exemption can become permanent The fence compares the timeline's retarget count taken *before* classification against one taken *before the write*. Nothing re-checks after the POST. Trace: `main` carries a real `failure` for head H. Retarget to a scratch base where H is docs-only; run S derives `success` and passes its final fence check. Retarget back to `main` while S is paused before its POST. Successor run M sees the base-matching `failure` and short-circuits, posting nothing. S resumes and posts its stale `success`. The rejection predates S's high-water mark, so post-write verification ignores it — and **no event remains** to reclassify. The forged green is permanent. `ci.verdict-write-retarget-fence` describes the surviving window as the sub-round-trip one that no API without compare-and-set can close. This scenario says the window is wider than that: the fence narrows *writing while overtaken*, but not *being overtaken after writing*. **Suggested:** re-count retargets **after** the POST. If the count moved, or cannot be trusted, immediately replace the status with the sticky `pending` sentinel — a retarget after that post-check then necessarily queues a successor that starts after the stale `success` exists and can re-derive it. ## Testing note The reviewer also observed that `test_an_UNTRUSTED_retarget_count_STILL_LETS_PENDING_THROUGH` asserts only a one-run payload from an initially statusless head, and that every post-write race test derives `success` — which is why the `state = success` gate in (2) survives the suite. A **chained** test is the right shape (this repo has the pattern recorded as `fixed-point-properties-need-chained-tests`): run N with an enumeration failure plus a raced human verdict, assert the sentinel repair; feed that resulting status into run N+1 with successful docs enumeration, and assert it stays the sentinel. ## Done-when **Consolidated 2026-08-29** — the merge-consent hook reads THIS list, not the revisions that were left in comments, so the two later lists (the 01:05 revision and the 02:57 addition) are folded in here and the original eight are superseded. The comments stay as the record of how the scope moved. - [x] Every write is verified when a high-water mark exists — not only `state = success` - [x] A distinct STICKY "unverified write" sentinel when no mark can be established, so a later run cannot re-derive it into `success` - [x] The stall cost is either given a retry/reconciliation path, or documented and accepted as human-required — not described as a one-run cost - [x] Every parse/schema failure in the post-write `jq` resolves to `raced=1` rather than killing the job or "not acting" - [x] An unreadable combined-status read cannot leave a pre-existing off-list `success` standing - [x] The post-POST retarget re-check exists (`ci.verdict-write-retarget-fence`'s residual 1 was a PERMANENT green, not a sub-round-trip window) - [x] A CHAINED test feeds run N's real output into run N+1 - [x] Each fix ships with a mutation proof that it reddens - [x] The fence/pending guarantee PROSE is swept by CONCEPT (not by phrase) and corrected in the same PR as the behaviour - [x] Adversarial review passed
timothy added the ci-cdpriority: mediumsecurity labels 2026-08-27 00:25:17 +02:00
timothy commented 2026-08-27 00:50:52 +02:00
Author
Owner
Copy Link
Copy Source

Scope narrowed — two of the three are being fixed in #742 after all

The original framing here ("all three are pre-existing, #742 touches none of them") was only textually right, and a follow-up cross-family review said so. The guards are byte-identical to main, but #742 changes their reachability: before it, an attributable success from any account short-circuited and the job wrote nothing, so the unverified-write paths were unreachable for that entire class of head. Re-deriving an off-list success is the point of #742 — and it turns "no write" into "a write", which is what reaches them.

So two of the three are fixed on that branch rather than deferred here:

  • (1) unestablished high-water mark — the exemption is now withheld instead of being posted with verification skipped.
  • (2) unreadable post-write history — now repairs to the sticky sentinel instead of logging "The exemption … stands". The repair case was hoisted out of the readable branch so both paths converge on it.

Both are mutation-proven (restoring either old behaviour reddens its test), and both cost the run its exemption on a transient API failure — the price this file already pays for the same reason in #751's page-2 rule.

Implementation note worth keeping: the withholding guard has to sit after the high-water mark is established, not with the classification. $max_id_before does not exist there, and referencing it early is an unbound variable under set -u — the step dies and every PR is stranded with no status. Found by running it, not by reading it.

What REMAINS in this issue

  • (3) success-only post-write verification — the check runs only if [ "$state" = "success" ], so a generic pending write can bury a raced rejection that a later run then re-derives to green. Still open, and reachable from #742's class too, not only from a statusless head.
  • The retarget fence's post-POST gap — the fence compares counts before the write and never after, so a run overtaken after writing can leave a stale exemption permanently.
  • The chained test for (3): run N with an enumeration failure plus a raced verdict, assert the sentinel; feed that status into run N+1 and assert it stays the sentinel.

The ## Done-when boxes for (1) and (2) can be ticked when #742 merges; the rest stand.

## Scope narrowed — two of the three are being fixed in #742 after all The original framing here ("all three are pre-existing, #742 touches none of them") was **only textually right**, and a follow-up cross-family review said so. The guards are byte-identical to `main`, but #742 changes their **reachability**: before it, an attributable `success` from *any* account short-circuited and the job wrote **nothing**, so the unverified-write paths were unreachable for that entire class of head. Re-deriving an off-list `success` is the *point* of #742 — and it turns "no write" into "a write", which is what reaches them. So two of the three are fixed on that branch rather than deferred here: - **(1) unestablished high-water mark** — the exemption is now **withheld** instead of being posted with verification skipped. - **(2) unreadable post-write history** — now **repairs to the sticky sentinel** instead of logging "The exemption … stands". The repair `case` was hoisted out of the readable branch so both paths converge on it. Both are mutation-proven (restoring either old behaviour reddens its test), and both cost the run its exemption on a transient API failure — the price this file already pays for the same reason in #751's page-2 rule. Implementation note worth keeping: the withholding guard has to sit **after** the high-water mark is established, not with the classification. `$max_id_before` does not exist there, and referencing it early is an unbound variable under `set -u` — the step dies and every PR is stranded with no status. Found by running it, not by reading it. ## What REMAINS in this issue - **(3) success-only post-write verification** — the check runs only `if [ "$state" = "success" ]`, so a generic `pending` write can bury a raced rejection that a later run then re-derives to green. Still open, and **reachable from #742's class too**, not only from a statusless head. - **The retarget fence's post-POST gap** — the fence compares counts before the write and never after, so a run overtaken *after* writing can leave a stale exemption permanently. - **The chained test** for (3): run N with an enumeration failure plus a raced verdict, assert the sentinel; feed that status into run N+1 and assert it stays the sentinel. The `## Done-when` boxes for (1) and (2) can be ticked when #742 merges; the rest stand.
timothy commented 2026-08-27 01:05:10 +02:00
Author
Owner
Copy Link
Copy Source

Scope CORRECTED again — the two "fixed in #742" boxes are un-ticked, and the issue grows two routes

I previously said routes (1) and (2) were fixed on the #742 branch. They have been withdrawn. A further cross-family round showed the fix was worse than the gap on both counts, on grounds that generalise:

  • The withheld exemption wrote a GENERIC pending — and a generic pending is precisely what a later run re-derives into success. It did not close the burial route, it moved which run greens it. Closing it properly needs a distinct sticky "unverified write" sentinel, i.e. a new state in a machine that already has three (Exempt:, the repair sentinel, generic awaiting).
  • That pending has NO RETRY PATH. review-verdict.yml triggers only on pull_request_target types — verified: no schedule, no workflow_dispatch. So a transient API failure on a PR's last event leaves an exempt PR stalled until a human nudges it. Trading a race needing BOTH a read failure AND a reviewer posting inside the write window, for a stall needing only the read failure, is not obviously the safe direction — and I had described it as a "one-run cost", which it is not.

Two lines inside a provenance change was the wrong size for this. It belongs here, designed once.

Two further routes found in the same round

(4) A malformed status row kills the verifier instead of repairing. The post-write raced=$(... jq ...) assignment is unguarded. A row with "creator": "timothy" (a string, not an object) makes .creator.login error; a numeric .description makes startswith error. Under set -euo pipefail the job exits immediately, skipping the repair entirely and leaving the exemption green. The non-numeric branch below it likewise logs "not acting on it" rather than repairing. Every parse/schema failure in that expression should resolve to raced=1.

(5) An unreadable combined-status read leaves an off-list green standing. If an off-list credential has already posted review-verdict/h10=success and the initial or last-moment combined read fails, read_existing_verdict exits without replacing it. The job goes red — but the workflow's own job result is not a required check, so branch protection still sees the forged green. This is the exact status #742 exists to revoke, surviving a read failure. Fix shape: a separate classifier-health required context, or durably replacing unknown state with a sentinel.

Why #742 still ships

Its own change is sound and unbroken after seven rounds: an existing success is inherited only from an allow-listed creator, an existing failure on a weaker attributability test. What #742 does is make these residuals reachable from a new class — before it, an attributable success from any account short-circuited and the job wrote nothing. That reachability is recorded in ci.exemption-provenance rather than left implicit, and ci.verdict-write-retarget-fence has had its false "transient forged green" claim corrected in the same PR (the successor run can finish first, consuming the edited event, after which the stale run's success lands last and nothing remains to correct it).

Done-when (revised)

  • Every write is verified when a high-water mark exists — not only state = success
  • A distinct STICKY "unverified write" sentinel when no mark can be established, so a later run cannot re-derive it into success
  • The stall cost is either given a retry/reconciliation path, or documented and accepted as human-required — not described as a one-run cost
  • Every parse/schema failure in the post-write jq resolves to raced=1 rather than killing the job or "not acting"
  • An unreadable combined-status read cannot leave a pre-existing off-list success standing
  • The post-POST retarget re-check exists (ci.verdict-write-retarget-fence's residual is now stated as a PERMANENT green, corrected in #742)
  • A CHAINED test feeds run N's real output into run N+1
  • Each fix ships with a mutation proof that it reddens
  • Adversarial review passed
## Scope CORRECTED again — the two "fixed in #742" boxes are un-ticked, and the issue grows two routes I previously said routes (1) and (2) were fixed on the #742 branch. **They have been withdrawn.** A further cross-family round showed the fix was worse than the gap on both counts, on grounds that generalise: - **The withheld exemption wrote a GENERIC `pending`** — and a generic `pending` is precisely what a later run re-derives into `success`. It did not close the burial route, it moved which run greens it. Closing it properly needs a distinct **sticky "unverified write" sentinel**, i.e. a new state in a machine that already has three (`Exempt:`, the repair sentinel, generic awaiting). - **That `pending` has NO RETRY PATH.** `review-verdict.yml` triggers only on `pull_request_target` types — verified: no `schedule`, no `workflow_dispatch`. So a transient API failure on a PR's *last* event leaves an exempt PR stalled until a human nudges it. Trading a race needing BOTH a read failure AND a reviewer posting inside the write window, for a stall needing only the read failure, is not obviously the safe direction — and I had described it as a "one-run cost", which it is not. Two lines inside a provenance change was the wrong size for this. It belongs here, designed once. ## Two further routes found in the same round **(4) A malformed status row kills the verifier instead of repairing.** The post-write `raced=$(... jq ...)` assignment is unguarded. A row with `"creator": "timothy"` (a string, not an object) makes `.creator.login` error; a numeric `.description` makes `startswith` error. Under `set -euo pipefail` the job exits immediately, **skipping the repair entirely** and leaving the exemption green. The non-numeric branch below it likewise logs "not acting on it" rather than repairing. Every parse/schema failure in that expression should resolve to `raced=1`. **(5) An unreadable combined-status read leaves an off-list green standing.** If an off-list credential has already posted `review-verdict/h10=success` and the initial or last-moment combined read fails, `read_existing_verdict` exits without replacing it. The job goes red — but the workflow's own job result is not a required check, so branch protection still sees the forged green. This is the exact status #742 exists to revoke, surviving a read failure. Fix shape: a separate classifier-health required context, or durably replacing unknown state with a sentinel. ## Why #742 still ships Its own change is sound and unbroken after seven rounds: an existing `success` is inherited only from an allow-listed creator, an existing `failure` on a weaker attributability test. What #742 does is make these residuals **reachable from a new class** — before it, an attributable `success` from any account short-circuited and the job wrote *nothing*. That reachability is recorded in `ci.exemption-provenance` rather than left implicit, and `ci.verdict-write-retarget-fence` has had its false "transient forged green" claim corrected in the same PR (the successor run can finish *first*, consuming the `edited` event, after which the stale run's `success` lands last and nothing remains to correct it). ## Done-when (revised) - [ ] Every write is verified when a high-water mark exists — not only `state = success` - [ ] A distinct STICKY "unverified write" sentinel when no mark can be established, so a later run cannot re-derive it into `success` - [ ] The stall cost is either given a retry/reconciliation path, or documented and accepted as human-required — not described as a one-run cost - [ ] Every parse/schema failure in the post-write `jq` resolves to `raced=1` rather than killing the job or "not acting" - [ ] An unreadable combined-status read cannot leave a pre-existing off-list `success` standing - [ ] The post-POST retarget re-check exists (`ci.verdict-write-retarget-fence`'s residual is now stated as a PERMANENT green, corrected in #742) - [ ] A CHAINED test feeds run N's real output into run N+1 - [ ] Each fix ships with a mutation proof that it reddens - [ ] Adversarial review passed
timothy commented 2026-08-27 02:57:01 +02:00
Author
Owner
Copy Link
Copy Source

Scope boundary for the DOCUMENTATION half — recorded so it stops re-opening

Four review rounds on #742 kept finding paraphrases of two claims that the residuals in this issue disprove:

  • "the retarget fence resolved the race" — it narrows it; a retarget between the fence's final pre-write count and the POST leaves a permanent forged green.
  • "a pending write is harmless" — a generic pending masks a rejection landing in its own write window, gets no post-write verification, and a later run re-derives it green.

Each round corrected the sites it could see and the next round found more, because the claims exist in paraphrase across a corpus written before #742 — including a record's rule: frontmatter, an induction argument ("the last run writes the final answer"), a workflow comment heading ("THE PROBLEM THIS SOLVES"), and test docstrings.

#742 has now corrected every site that is either its own text or directly about the allow-list, plus the fence/pending claims in ci.verdict-write-retarget-fence, ci.exemption-provenance, ci.actions-credential-scoping, docs/ci-cd.md, CLAUDE.md, the classify comments and the affected test docstrings. docs/decisions/README.md is regenerated from those.

What is explicitly NOT in #742's scope, and belongs here: any remaining corpus prose describing the #706 fence's or the post-write verification's guarantees. Those sentences were true-as-written for the design they document; what makes them wrong is the residual analysis in this issue, so they should be corrected by whoever closes it — at which point the corrected behaviour and the corrected prose land together, instead of prose promising a property no code yet has.

One false positive worth recording so it is not "fixed": process.check-and-use-pins-a-version's title contains "the gap is stated and fenced". That is the generic check/action version-binding rule, unrelated to the retarget fence. Leave it.

Added to Done-when

  • When the behaviour lands, sweep the corpus for the fence/pending guarantee prose by CONCEPT (not by phrase) and correct it in the same PR — ci.verdict-write-retarget-fence, ci.exemption-provenance, docs/ci-cd.md, CLAUDE.md, the classify comments, and any test whose NAME asserts a guarantee (e.g. ..._is_still_NEVER_overwritten, which is scoped in its docstring but not in its name)
## Scope boundary for the DOCUMENTATION half — recorded so it stops re-opening Four review rounds on #742 kept finding paraphrases of two claims that the residuals in this issue disprove: - **"the retarget fence resolved the race"** — it narrows it; a retarget between the fence's final pre-write count and the POST leaves a **permanent** forged green. - **"a `pending` write is harmless"** — a generic `pending` masks a rejection landing in its own write window, gets no post-write verification, and a later run re-derives it green. Each round corrected the sites it could see and the next round found more, because the claims exist in **paraphrase** across a corpus written before #742 — including a record's `rule:` frontmatter, an induction argument ("the last run writes the final answer"), a workflow comment heading ("THE PROBLEM THIS SOLVES"), and test docstrings. **#742 has now corrected every site that is either its own text or directly about the allow-list**, plus the fence/pending claims in `ci.verdict-write-retarget-fence`, `ci.exemption-provenance`, `ci.actions-credential-scoping`, `docs/ci-cd.md`, `CLAUDE.md`, the classify comments and the affected test docstrings. `docs/decisions/README.md` is regenerated from those. **What is explicitly NOT in #742's scope, and belongs here:** any remaining corpus prose describing the #706 fence's or the post-write verification's guarantees. Those sentences were true-as-written for the design they document; what makes them wrong is the residual analysis in *this* issue, so they should be corrected by whoever closes it — at which point the corrected behaviour and the corrected prose land together, instead of prose promising a property no code yet has. One false positive worth recording so it is not "fixed": `process.check-and-use-pins-a-version`'s title contains "the gap is stated and fenced". That is the generic check/action version-binding rule, unrelated to the retarget fence. Leave it. ## Added to Done-when - [ ] When the behaviour lands, sweep the corpus for the fence/pending guarantee prose by CONCEPT (not by phrase) and correct it in the same PR — `ci.verdict-write-retarget-fence`, `ci.exemption-provenance`, `docs/ci-cd.md`, `CLAUDE.md`, the classify comments, and any test whose NAME asserts a guarantee (e.g. `..._is_still_NEVER_overwritten`, which is scoped in its docstring but not in its name)
timothy added the in-progress label 2026-08-29 22:36:23 +02:00
timothy commented 2026-08-29 22:36:42 +02:00
Author
Owner
Copy Link
Copy Source

Claiming this (Claude Code / Opus 5 orchestrator session, 2026-08-29).

Pre-claim checks per process.parallel-session-claim: no open PR references #849, git ls-remote --heads origin '*849*' is empty, the three existing comments are scope corrections (the select-queue.sh CLAIM? flag fired on the word "claims" in prose, not on a pickup claim), and origin/main is freshly fetched at 736649b3b.

Scope taken = the five routes as revised in the 2026-08-27T01:05 comment, plus the documentation half scoped by the 02:57 comment.

Sibling scanned and deliberately NOT bundled: #869 also edits review-verdict.yml's comment block (the 1.25.4-dated claims at lines ~69/~633/~746). Different work — it is a re-probe/re-date exercise needing live Gitea version reads — so it stays separate. Flagging the textual adjacency so whoever takes #869 next expects a moved comment block rather than a conflict they did not cause.

Claiming this (Claude Code / Opus 5 orchestrator session, 2026-08-29). Pre-claim checks per `process.parallel-session-claim`: no open PR references #849, `git ls-remote --heads origin '*849*'` is empty, the three existing comments are scope corrections (the `select-queue.sh` `CLAIM?` flag fired on the word "claims" in prose, not on a pickup claim), and `origin/main` is freshly fetched at 736649b3b. Scope taken = the five routes as revised in the 2026-08-27T01:05 comment, plus the documentation half scoped by the 02:57 comment. **Sibling scanned and deliberately NOT bundled:** #869 also edits `review-verdict.yml`'s comment block (the 1.25.4-dated claims at lines ~69/~633/~746). Different work — it is a re-probe/re-date exercise needing live Gitea version reads — so it stays separate. Flagging the textual adjacency so whoever takes #869 next expects a moved comment block rather than a conflict they did not cause.
timothy commented 2026-08-30 07:56:27 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Shipped in PR #890 (nine commits, merged as 58681b3a7). All five routes closed. Every path that cannot establish what the head carries now REPLACES that unknown state with a sticky sentinel instead of leaving it standing; post-write verification runs after every write; the retarget count is re-taken after the POST; and a write no high-water mark can cover becomes the sentinel before the POST rather than a status a later run re-derives.

Root cause: One mistake in three costumes — uncertainty resolving toward success. The gate had a rule ("uncertainty on the write path resolves to pending") and applied it unevenly: the post-write check was gated on state = success because "a pending cannot turn a rejection green" (false — it carries no marker, so the NEXT run re-derives it); an unreadable read "declined to write" to protect a real verdict (which also protects a FORGED one); and the mark on a declined row was decided from the opening snapshot while the POST replaced the current one. The #742 attempt failed because a GENERIC pending is exactly what a later run re-derives — so the fix needs sticky (a later run cannot re-derive it) and reconcilable (a blip does not cost a head its exemption permanently), and neither the repair sentinel nor a generic pending has both.

Decisions/conventions changed: New record ci.verdict-unverified-write-sentinel. Corrected by CONCEPT, not phrase: ci.verdict-write-retarget-fence (its rule: frontmatter, the "resolves it" opener, "the fence above closes", the truncating-block claim, residual 1), ci.exemption-provenance (the post-final-count window is transient now, not permanent), docs/ci-cd.md, docs/remote-state-inventory.md, docs/guard-inventory.md, CLAUDE.md.

Reusable knowledge:

  • A mutation SWEEP finds what per-finding review cannot. 60 mutants, 40 red, 20 survivors — on a tree that had already passed three review rounds by two model families. A review reads what the diff says it does; a sweep measures what the tests pin. Dispatch it as its own job on anything gate-shaped.
  • A proof can name the right clause and measure nothing. The no-mark proof mutated the description the block writes, not its scope, on a fixture where the predecessor behaved identically. State the claim, then ask what the predecessor would do on that exact fixture.
  • A range replacement anchored on the NEXT definition deletes everything between. Round 9 removed 15 tests and added 4 while claiming two — including proofs for two earlier Blockers — and the suite went green because the tests were gone. Count ^def test_ across HEAD~1 before committing any test-file change.
  • git parses only the LAST paragraph as trailers. A blank line before Co-Authored-By demoted Decisions-Edit: yes on all nine commits; CI caught it, local runs did not.
  • Selective CI rerun exists: POST /actions/runs/{run}/jobs/{job}/rerun → 201. /actions/jobs/{id}/rerun is 404 and a whole-run rerun 400s while any job is pending.

Verification: scripts/tests/ green (1346); CI fully green on d4b36ac2 (13 success, 1 skipped, 0 red) including both required contexts and the job that executes all 229 workflow-body tests. Every reachable clause carries a test_MUTATION_… bound by a count assertion, so a reworded clause fails loudly rather than measuring the unmutated body. Two CHAINED tests feed run N's real output into run N+1, because both sentinels are fixed points. Measured on this instance (Gitea 1.27.1): /commits/{sha}/status rows carry id, which the row-identity comparison depends on.

Six clauses ship deliberately unproven and are ENUMERATED by name in the record with per-clause unreachability arguments (path-predicate failure; empty-row refusal; page 2's non-numeric length; the $witness normalisation; both unusable-count arms). The sweep independently confirmed that set is exactly right. That number went two → five → six across three rounds as the sweep widened; naming them beats a smaller number that reads better.

Review: nine rounds. Cross-family (Codex/GPT-5.6) on rounds 1–2 and 8–9; cold same-family reviewers throughout; plus the sweep. Rounds 4, 8 and 9 each found a defect introduced by the PREVIOUS round's fix — including one regression against origin/main (a malformed .creator read as "unattributable", greening a human failure that main fail-closed on). Codex was quota-limited for rounds 3–7; the sweep stood in, and that is a substitute rather than an equivalent.

Deferred: none for this issue. Adjacent and untouched: ci.verdict-write-retarget-fence's head-axis residual (#803's text, re-bound here but not re-litigated); the registry cleanup rule that evicts pinned CI images (server-management#842).

Docs updated: docs/decisions/records/ci/verdict-unverified-write-sentinel.md (new), ci/verdict-write-retarget-fence.md, ci/exemption-provenance.md, docs/decisions/README.md (regenerated), docs/ci-cd.md, docs/guard-inventory.md, docs/remote-state-inventory.md, CLAUDE.md.

Unrelated incident resolved mid-session

CI went red on a preflight, and it was not this diff: the pinned CI toolchain image ersatztv-ci:32747a0 had been evicted from the registry (HTTP 404), which kills every container: job in docker-build.yml — both required contexts included — so no PR in this repo could merge. Verified independently (manifest 404; five other tags present; the pin is the last commit to touch docker/ci, so missing, not stale). Recovered per docs/ci-cd.md → "When the pinned tag disappears": republished the same tag from the same commit, built on jazz (bumblebee was at load 14 running this PR's own CI). The repo's own preflight then reported resolves (HTTP 200). This is the ersatztv#772 failure mode; the durable fix is registry-side (server-management#842).

A second red was runner-side: Set up Python failed on a corrupt act action cache (lstat …/eslint.config.mjs: no such file or directory) before pytest ran at all. Cleared by a selective job rerun.

## Closing record **Outcome:** Shipped in PR #890 (nine commits, merged as `58681b3a7`). All five routes closed. Every path that cannot establish what the head carries now REPLACES that unknown state with a sticky sentinel instead of leaving it standing; post-write verification runs after every write; the retarget count is re-taken after the POST; and a write no high-water mark can cover becomes the sentinel before the POST rather than a status a later run re-derives. **Root cause:** One mistake in three costumes — **uncertainty resolving toward `success`**. The gate had a rule ("uncertainty on the write path resolves to `pending`") and applied it unevenly: the post-write check was gated on `state = success` because "a `pending` cannot turn a rejection green" (false — it carries no marker, so the NEXT run re-derives it); an unreadable read "declined to write" to protect a real verdict (which also protects a FORGED one); and the mark on a declined row was decided from the opening snapshot while the POST replaced the current one. The #742 attempt failed because a GENERIC `pending` is exactly what a later run re-derives — so the fix needs **sticky** (a later run cannot re-derive it) *and* **reconcilable** (a blip does not cost a head its exemption permanently), and neither the repair sentinel nor a generic `pending` has both. **Decisions/conventions changed:** New record `ci.verdict-unverified-write-sentinel`. Corrected by CONCEPT, not phrase: `ci.verdict-write-retarget-fence` (its `rule:` frontmatter, the "resolves it" opener, "the fence above closes", the truncating-block claim, residual 1), `ci.exemption-provenance` (the post-final-count window is transient now, not permanent), `docs/ci-cd.md`, `docs/remote-state-inventory.md`, `docs/guard-inventory.md`, `CLAUDE.md`. **Reusable knowledge:** - **A mutation SWEEP finds what per-finding review cannot.** 60 mutants, 40 red, **20 survivors** — on a tree that had already passed three review rounds by two model families. A review reads what the diff says it does; a sweep measures what the tests pin. Dispatch it as its own job on anything gate-shaped. - **A proof can name the right clause and measure nothing.** The no-mark proof mutated the *description* the block writes, not its *scope*, on a fixture where the predecessor behaved identically. State the claim, then ask what the predecessor would do on that exact fixture. - **A range replacement anchored on the NEXT definition deletes everything between.** Round 9 removed 15 tests and added 4 while claiming two — including proofs for two earlier Blockers — and the suite went green *because the tests were gone*. Count `^def test_` across `HEAD~1` before committing any test-file change. - **`git` parses only the LAST paragraph as trailers.** A blank line before `Co-Authored-By` demoted `Decisions-Edit: yes` on all nine commits; CI caught it, local runs did not. - **Selective CI rerun exists**: `POST /actions/runs/{run}/jobs/{job}/rerun` → 201. `/actions/jobs/{id}/rerun` is 404 and a whole-run rerun 400s while any job is pending. **Verification:** `scripts/tests/` green (1346); CI fully green on `d4b36ac2` (13 success, 1 skipped, 0 red) including both required contexts and the job that executes all 229 workflow-body tests. Every reachable clause carries a `test_MUTATION_…` bound by a count assertion, so a reworded clause fails loudly rather than measuring the unmutated body. Two CHAINED tests feed run N's real output into run N+1, because both sentinels are fixed points. Measured on this instance (Gitea 1.27.1): `/commits/{sha}/status` rows carry `id`, which the row-identity comparison depends on. **Six clauses ship deliberately unproven and are ENUMERATED by name** in the record with per-clause unreachability arguments (path-predicate failure; empty-`row` refusal; page 2's non-numeric length; the `$witness` normalisation; both unusable-count arms). The sweep independently confirmed that set is exactly right. That number went two → five → six across three rounds as the sweep widened; naming them beats a smaller number that reads better. **Review:** nine rounds. Cross-family (Codex/GPT-5.6) on rounds 1–2 and 8–9; cold same-family reviewers throughout; plus the sweep. Rounds 4, 8 and 9 each found a defect introduced by the PREVIOUS round's fix — including one regression against `origin/main` (a malformed `.creator` read as "unattributable", greening a human `failure` that `main` fail-closed on). Codex was quota-limited for rounds 3–7; the sweep stood in, and that is a substitute rather than an equivalent. **Deferred:** none for this issue. Adjacent and untouched: `ci.verdict-write-retarget-fence`'s head-axis residual (#803's text, re-bound here but not re-litigated); the registry cleanup rule that evicts pinned CI images (server-management#842). **Docs updated:** `docs/decisions/records/ci/verdict-unverified-write-sentinel.md` (new), `ci/verdict-write-retarget-fence.md`, `ci/exemption-provenance.md`, `docs/decisions/README.md` (regenerated), `docs/ci-cd.md`, `docs/guard-inventory.md`, `docs/remote-state-inventory.md`, `CLAUDE.md`. --- ### Unrelated incident resolved mid-session CI went red on a preflight, and it was **not this diff**: the pinned CI toolchain image `ersatztv-ci:32747a0` had been **evicted from the registry** (HTTP 404), which kills every `container:` job in `docker-build.yml` — both required contexts included — so no PR in this repo could merge. Verified independently (manifest 404; five other tags present; the pin *is* the last commit to touch `docker/ci`, so **missing, not stale**). Recovered per `docs/ci-cd.md` → "When the pinned tag disappears": republished the same tag from the same commit, built on jazz (bumblebee was at load 14 running this PR's own CI). The repo's own preflight then reported `resolves (HTTP 200)`. This is the ersatztv#772 failure mode; the durable fix is registry-side (server-management#842). A second red was runner-side: `Set up Python` failed on a corrupt act action cache (`lstat …/eslint.config.mjs: no such file or directory`) before pytest ran at all. Cleared by a selective job rerun.
timothy removed the in-progress label 2026-08-30 07:56:38 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels ci-cd priority: medium security
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#849