795 Commits

Include renames

Author	SHA1	Message	Date
timothyandClaude Opus 5	f376d6cdf4	test(830): pin the ORDINARY close too — the negative test did not cover it ... Round 6 verdict was MERGEABLE with two follow-ups; both are one-liners on lines this branch just touched, so they are in rather than deferred. L11. I believed the suite covered "a normal add still closes the dialog". Review MEASURED that it did not: deleting the `onClose()` call entirely -- so a successful add leaves the picker open forever -- kept the whole suite green, 1277/1277. The new #830 test only pins the NEGATIVE direction (do not close when unmounted), so a future edit dropping the call, believing the guard had made it dead, would have shipped silently. The Song add test now asserts the dialog closes; with that line, the same deletion reddens. Both directions of the report/dismiss split are pinned. Worth naming the shape: I asserted coverage from plausibility rather than from a mutation, in the same PR whose whole subject is claims that were written down before they were measured. N12. The guards test's "exactly ONE post-await write to state THIS component owns" is still true, but it now reads as a census of `mountedRef` reads, and `submit` has two -- the success path's guarded `onClose()` is the other, which that failure-path test never reaches. Added the clause so nobody derives the guard population from that number. 1277 tests green, tsc/eslint/build clean, pytest 1228 passed, validator OK, catalog no drift. The two red CI contexts on the previous head are runner flakes, not this branch: both failed inside `Post Checkout` with `Cannot find module '/var/run/act/actions/<hash>/dist/index.js'`, their logs are timestamped 19:18 (before that head existed), this branch touches no CI or docker/ci file, and both contexts were green on its earlier heads. refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 21:38:10 +02:00
timothyandClaude Opus 5	d90326f762	fix(830): gate onClose at the exemplar — the convention contradicted its own reference ... Round 5 found that §3c and the `rule:` field now instruct readers to gate the DISMISS request, while `AddItemsDialog` -- the one site this record names as "the shape fixed here" -- called `onClose()` unguarded, with a comment arguing that was correct. So a reader following the convention wrote the gate and a reader copying the reference implementation did not. The defect is pre-existing; the CONTRADICTION is mine, introduced when round 3 withdrew H1's code but kept the convention it produced. I checked the docs against the withdrawn addTo code and did not re-check them against the exemplar that stayed. Measured at this site: submit, Escape mid-request, reopen the picker to retry, first POST returns 204 -> the stale instance's `onClose()` (`() => setPickerOpen(false)`) closes the dialog the user just reopened, discarding the selection they rebuilt. Identical mechanism to the addTo clobber. Unlike the addTo layer, the one-line gate IS sufficient here, and that difference is the point: `AddItemsDialog`'s parent has no competing closer (`onAdded` is `load`, which never touches `pickerOpen`), whereas `AddToMenu.handleAdded` closes its dialog itself. That is now stated in the record as the concrete reason one half shipped and the other went to #877. - `onAdded()` stays unguarded -- it REPORTS, and the parent's list reload must survive dismissal - `onClose()` is guarded -- it REQUESTS A DISMISSAL, and after dismissal it aims at whatever the user opened next - comment rewritten to say which is which and why, instead of defending both as "belong to the still-mounted PARENT" Pinned, and nothing pinned it before: "a late SUCCESS does not close the dialog the user reopened after dismissing (#830)". It carries an anti-vacuity check that the late response was actually processed -- `onAdded` is `load`, so a second GET of the items endpoint must have happened -- because otherwise "the dialog is still open" holds trivially. Executed: deleting the `if (mountedRef.current)` around `onClose()` reddens it alone. Also rewrapped five record body lines left ragged by earlier splices. 1277 tests green, tsc/eslint/build clean, pytest 1228 passed, validator OK, catalog no drift. refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 21:18:03 +02:00
timothyandClaude Opus 5	3717f623db	docs(830): drop a coverage claim this record's own mechanics field contradicts ... Round 4 found one clause left, and it is a good example of the thing this record is about. The one-consumer paragraph said `hooks.test.tsx` "is the ONLY thing pinning the diverted branch". Measured at the previous head, disarming `reportRef.current(message)` reddens THREE tests -- both hooks.test.tsx divert tests AND the CollectionsScreen integration test -- which is exactly what the `mechanics:` field of the same record says 58 lines earlier. So the record asserted a coverage fact and then contradicted itself. The concrete harm is not the inconsistency: a future session pruning tests reads "hooks.test.tsx is the only pin", concludes the CollectionsScreen #830 test is redundant, and deletes the only end-to-end pin of the whole path -- the one that actually drives Escape-dismissal through the real dialog. Clause dropped; the argument the paragraph needed (the hook's shape earns its own unit tests) survives without it. The clause originated in the reviewer's round-3 wording and I transcribed it without checking it against a field I had written myself two rounds earlier. Worth recording: a review finding is not exempt from verification just because it came from the reviewer. Also: - the `AddToMenu` clobber sentence now splits what was MEASURED (a late success closes a reopened dialog) from what was READ (both parents call `clearSelection()` unconditionally, so the wipe follows). On a record whose subject is over-attributing measurements, that distinction has to hold in its own prose. - §3c now carries the same "nothing diverts to those screens today" disclaimer the record's limit (2) has, so the two artifacts say the same thing - rewrapped one 141-char comment line left ragged by the previous round's splice Docs only, plus one comment rewrap. 1276 tests green, tsc/eslint/build/validator/catalog clean. refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 20:57:13 +02:00
timothyandClaude Opus 5	96a800dd61	docs(830): three sentences survived the revert of the code they described ... Round 3 verified the withdrawal itself is clean -- the eight reverted files are byte-identical to origin/main, no orphans, and the mutation gives the stated three reds -- but found docs still asserting the withdrawn change shipped. That is the stale-comment failure in its usual form: after a retraction, the retracted WORDING has to be swept, not just the code. - `hooks.ts` said "#830 removed that gate", flatly false at this head, in the hook's own doc comment right above the export -- the first thing a maintainer reads. It also carried round 2's framing ("both halves of the outcome") as the hook's purpose, when what ships carries only the failure half. Rewritten to the present tense of the shipped tree. - The `rule:` field still said the surviving surface "differs per screen", naming MediaBrowseScreen and SearchScreen as wired. They wire nothing. This one matters beyond an ordinary sentence: `rule:` is the canonical summary, it is what the catalog row shows, and it is what gets mirrored per-key into MemPalace -- so it is the version a future session retrieves WITHOUT opening the file. Now: exactly one wired screen, the Toast pair named as a CANDIDATE. - The "two limits" bullet described a failure being diverted to those same screens and announced politely. Nothing can divert there -- they receive no reporting callback. Restated as the limit the second surface will have when it is wired. - `onFailed` in a hooks.ts comment was a dangling identifier; the real prop is `onAddFailed`. Also added the caveat the reviewer asked for rather than leaving it to be discovered: this is a shared hook with exactly ONE consumer. It earns that shape (directly unit-tested, and those tests are the only thing pinning the diverted branch; prescribed by §3c; #877 queued as a second consumer) -- but #877 may land a shared reporting SURFACE instead of a per-site prop, in which case the second consumer never arrives. Accepted risk, now written down. Docs only. No code change, 1276 tests still green, tsc/eslint/validator/catalog clean. refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 20:46:30 +02:00
timothyandClaude Opus 5	5d035e3d3e	fix(830): withdraw the media/addTo extension — two rounds, two defects, one coupled gate ... Round 2 of adversarial review found that my round-1 fix introduced an adjacent defect, and it is the same mechanism both times: `onAdded?.()` and `onClose()` sat behind ONE unmount gate in the four `media/addTo/` dialogs, and those two callbacks do not mean the same thing. - Gate both (origin/main): a write that SUCCEEDS after dismissal reports nothing. Measured on `AddToCollectionDialog` -- `onAdded` called 0 times after dismissal. That was round 1's finding. - Un-gate both (my round-1 fix): a late success closes a dialog the user REOPENED to retry, and on SearchScreen/MediaBrowseScreen `clearSelection()` wipes a multi-select they rebuilt. Measured against the real `AddToMenu`. That was round 2's finding, and I introduced it. - Gate only `onClose`: still wrong on its own, because `AddToMenu.handleAdded` nulls the dialog itself. Needs three coupled edits across five files -- plus a genuine product question nobody has answered: should `clearSelection()` fire for a write the user walked away from? That is a design change, not a bug fix, and #830 never asked for it -- the issue is about `AddItemsDialog`. Two defects from one mechanism in two rounds is the signal to stop widening, so the `media/addTo/` extension is REVERTED here and moves to #877 with every measurement attached (#877 comment). What ships is the thing the issue asked for, proved: - `useDismissSafeError` + `AddItemsDialog` + `CollectionsScreen` wiring - the witnessed red is unchanged: disarming `reportRef.current(message)` reddens the integration test on `Unable to find an element with the text: Request failed with status 500` Docs now describe what is actually true rather than what I hoped: - the record says ONE A1 site is fixed and explains why the other four were withdrawn, keeping the wrong first claim visible because "one site read, four assumed" is the lesson - §3c splits the rule the round-2 defect came from: report the OUTCOME unguarded, gate the DISMISS request separately -- the earlier text lumped `onClose` in with `onAdded` and would have propagated the clobber to the next screen that adopted it - §5c no longer tells authors to wire an `onFailed` that the addTo layer does not have; it says the layer has no failure channel at all and points at #877 - the reporting prop is REQUIRED where the host has a surface (`AddItemsDialog.onAddFailed`), which is what the docs now say instead of calling it optional - `mechanics:` no longer implies the shared clause reddens one test; it reddens three, so re-running the mutation should expect three refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 20:34:40 +02:00
timothyandClaude Opus 5	06a582de50	fix(830): report SUCCESS past dismissal too — the review measured my claim false ... Adversarial review (cold, worktree-isolated) blocked the first commit on its central prose claim, correctly. I wrote "at every one of these sites SUCCESS already outlives dismissal" into three durable artifacts -- the decision record, spa-conventions §3c and the hooks.ts header -- after reading ONE site. `AddItemsDialog` does report success past dismissal and says so in a comment; I generalised from it. The review probed the other four instead and MEASURED `onAdded` called 0 times after dismissal: all four `media/addTo/` dialogs gated `onAdded?.()`/`onSaved?.()` behind their own `activeRef`, exactly like the failure path. So after the first commit those four were still asymmetric, just inverted: dismiss-then-fail loud, dismiss-then-succeed silent -- and additionally leaving the caller's selection state stale, because SearchScreen's `onAddedToSelectionTarget` never ran to clear it. The record's own advice ("add the failure counterpart") followed literally would have reproduced it. Fixes, each proved by execution: - the `activeRef` gate above `onAdded?.()`/`onSaved?.()` is removed in all four dialogs; those two statements belong to the still-mounted PARENT, which is the reasoning AddItemsDialog already had - `AddToCollectionDialog.test.tsx` covers the media/addTo half in BOTH directions. It had NO coverage before: reverting `reportFailure` to `setInlineError` in all four left the whole suite green. Restoring the success gate reddens the SUCCESS test alone; disarming `reportRef.current(message)` reddens the FAILURE test alone - the three prose sites now say what was measured, and the record keeps the wrong first version visible, because "one site read, four assumed, written down before measuring" is the finding Also from the review: - hooks.ts said "React 18"; package.json pins 19.2.7. Now "React 18+" - the "nothing better to do" comment overclaimed: diversion reaches ONE level, so Back out of a collection mid-add still drops the message. Stated, with where it would be fixed - recorded two limits rather than leaving them to be rediscovered: useIsMountedRef clears in a PASSIVE effect cleanup, leaving a narrow window where the message renders inline into a detached tree (useLayoutEffect would close it, but that hook is shared by every async caller -- #877, not a bug fix); and Toast is role="status" with one last-writer-wins slot, so it is not equivalent to CollectionsScreen's role="alert" - §5c now cross-links §3c, since that is the section a screen author reads before wiring AddToMenu - the sweep count is 67 caller-owned + ConfirmDialog's own internal <Dialog> Three other media/addTo dialogs remain unpinned; they are identical in shape to the covered one, which is a reason to expect the same behaviour, not evidence of it. Said so in the record. refs #830, #877 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 20:08:24 +02:00
timothyandClaude Opus 5	044b64ee87	fix(830): a write failure reports to a surface that outlives the dialog that started it ... `AddItemsDialog.submit` POSTed to `/api/v1/collections/{id}/items` and reported failure into a banner rendered from its OWN state. The dialog is dismissible mid-request through three paths that never consult `adding` -- Escape and a backdrop click (both `useOverlayBehavior`) and the header close button -- and the caller remounts it on `key={`add-${pickerOpen}`}`, so dismissal genuinely unmounts it. Select 12 items, Add, press Escape, the request fails: nothing surfaces, the list reloads unchanged, and the user believes 12 items were added. #740 guarded that `setError` with an is-mounted check, which was right and stays -- it made the drop deliberate rather than accidental. A deliberate drop is still a user who is told nothing. The asymmetry is the finding: SUCCESS already outlived dismissal everywhere here, because it is reported through a parent callback (`onAdded`/`onDone`, which the screens turn into a `Toast`). Only failure died with the surface. So this is not a new notification system -- it routes failure through the channel success already uses. `AddToMenu` had `onDone` and no counterpart at all. `useDismissSafeError` (`web/src/hooks.ts`) renders the message INLINE while the surface is mounted -- the better surface, since it keeps the user's selections and context -- and diverts to a caller-supplied `onFailed` once it is gone. The surviving surface belongs to the parent and differs per screen (a `role="alert"` banner on CollectionsScreen, `notice`+`Toast` on MediaBrowse/Search), so it is a prop contract rather than a rendering decision. Gating dismissal on the busy flag was considered and rejected: it traps the user behind an in-flight request with no cancel path, and would not cancel the write anyway. Applied to the A1 shape -- where the surface owns the error state and is really unmounted: AddItemsDialog plus the four `web/src/media/addTo/` dialogs, whose failures previously could not reach the screen Toast that already showed their successes. Proofs, executed rather than described: - deleting `reportRef.current(message)` alone reddens the new CollectionsScreen test on `Unable to find an element with the text: Request failed with status 500` - `hooks.test.tsx` pins both branches directly, plus that the report goes through the LATEST callback rather than the one captured on first render - `CollectionsScreen.guards.test.tsx`'s is-mounted read count moves 2 -> 1 because the catch's guard migrated into the hook (its `...actual` module mock cannot see the hook's internal `useIsMountedRef()`); removing the surviving `finally` guard takes it to 0 and reddens, so the anti-masking property that count was added for is intact Scope is stated rather than implied. A sweep of all 68 Dialog/ConfirmDialog/SlideOver call sites found three shapes; only A1 is fixed here. A2 -- error state that survives but whose render site is gated by the same condition dismissal clears, mostly delete-confirm flows -- is left open in #877 because its right answer is probably a shared surface, not twenty prop threads. fixes #830 refs #877, #740, #685 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY	2026-08-29 19:31:56 +02:00
timothy	8aeacd534a	fix(819): derive the SPA page-size guard population from the git index (#875) ... The guard asserted EXACT completeness over a population enumerated by a directory walk, so an untracked .ts/.tsx under web/src/ entered it and failed as unregistered on that developer's checkout while CI — which only ever checks out tracked files — stayed green. The glob still supplies file CONTENT; the POPULATION is now the git index, read by web/vite-plugins/trackedSourceFiles.ts in Vite's own Node context and handed to the app project as a virtual module. That reaches the index without admitting @types/node to tsconfig.app.json, the obstacle that deferred this in #818. Three mechanisms carry the proof, each added because the previous was measured insufficient: a closed-form restatement of the shared scope predicate (sharing no helper at any depth with what it checks); a second independent `ls-files --others` query cross-checking the population; and real-git tests that execute the derivation against a temp repository. Six residuals are stated with their MEASURED fail-directions, and testing.guard-derives-population-from-source gains a bounded exception plus the closed-form criterion. fixes #819 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-29 09:14:15 +00:00
timothy	b6b3520bdb	fix(809,822): isolate the suite from the production hook-fire log by construction (#874) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-29 02:32:26 +00:00
timothyandClaude Opus 5	90f96c14a5	fix(803,664): fence the HEAD alias on the PR timeline's pull_push count (#873) ... A force-push H1 -> H2 -> H1 spanning `pr-changed-files.sh`'s paging leaves its final `.head.sha` comparison equal while the middle pages came from H2, so a mixed file list could produce a docs-only exemption `success` no single head ever justified. The base alias had been fenced since #706 by a monotonic `change_target_branch` count; the head axis had nothing, and three contracts asserted otherwise. `count_retargets` becomes `count_pr_mutations`: one timeline walk, two tallies, one shared trust flag, a separate fence arm and diagnostic per axis. The advisory hook re-reads `.head.sha` at the same hoist and off the same response as the base re-read. All three overclaiming contracts are corrected, plus four paraphrases the first sweep missed. Measured, not assumed: Gitea 1.27.1 still serves no `files` on `compare/{base}...{head}`; every push is a `pull_push` event and its count cannot alias; PR #761 really went `8798a1d -> 830a407 -> 8798a1d`; and Gitea creates the push comment BEFORE emitting the synchronize notification, so a run cannot abstain on its own trigger. Two pre-existing fail-opens in the shared walk were found by review and fixed: an empty ARRAY first page was trusted on any page while the `null` arm required `page > 1`, and no row was validated before `.type` was selected on. NOT closed, and documented rather than overclaimed: the walk's `null` terminator is defeatable, because Gitea pages before it filters (#870). The fence closes the ABA on a timeline with no truncating block, not the ABA outright. fixes #803 fixes #664 Refs #870 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-28 23:29:47 +00:00
timothy	4b3f6f6c90	fix(786,789): workflows declare their own per-job metadata; guard populations derive from it (#872) ... 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>	2026-08-28 20:54:53 +00:00
timothy	e11d577193	docs(796): verification code is code under test, and the proof it was claiming an exemption from (#871) ... 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>	2026-08-28 20:06:45 +00:00
timothy	609fd852c2	fix(763): page both /statuses/{sha} reads to a validated terminator (#868)	2026-08-28 17:34:48 +00:00
timothyandClaude Opus 5	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) ... 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>	2026-08-28 16:51:13 +00:00
timothyandClaude Opus 5	462118d98d	fix(763): round 9 — assert the repair's reason positively, not by absence ... 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>	2026-08-28 18:42:22 +02:00
timothyandClaude Opus 5	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 b16ec15d6 (the other session's #781/#799 docs work; no file overlap, no conflicts). Verification: `scripts/tests` 1097 passed, 2 skipped; fifteen executed mutations across rounds 2-7; decisions_validate and build_decisions_catalog --check exit 0. refs #763 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 17:47:06 +02:00
timothyandClaude Opus 5	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>	2026-08-28 17:34:08 +02:00
timothyandClaude Opus 5	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>	2026-08-28 17:34:07 +02:00
timothyandClaude Opus 5	1eb7b41fc1	fix(763): round 4 — withdraw the ASC sort, which inverted the partial-mark fallback ... Round 3 added `sort=highestindex` to close a mid-walk-insert gap: under the server default (`created_unix DESC`) a row inserted while the walk is running lands at position 0, on a page already read, so the walk never sees it. That fix and the round-2 partial-mark fallback are incompatible. ASC puts the OLDEST rows on page 1, so an incomplete walk takes its high-water mark over the oldest rows — leaving every pre-existing row above the mark and read as "raced". That is a spurious STICKY repair on a head nothing raced, which is precisely the #761 failure this whole issue exists to remove. Under the default DESC the newest row is on page 1 by construction and ids are monotonic with `created_at` (measured), so a partial mark is at or very near the true maximum and "lower is safe" actually holds. Two defects from one mechanism again, so the mechanism goes rather than getting patched: the sort is withdrawn and the mid-walk-insert residual is ACCEPTED and documented. It is bounded — a row arriving after this job's POST is not one this job overwrote, and being newest it wins on the combined endpoint branch protection reads. Both the code comment and the docs record the withdrawal and the reason, so the next reader does not re-adopt it. Verification: `scripts/tests` 1090 passed, 2 skipped; the partial-mark mutation still reddens `test_a_PRE_WRITE_paging_failure_still_yields_a_usable_high_water_mark`; decisions_validate and build_decisions_catalog --check both exit 0. refs #763 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 17:34:07 +02:00
timothyandClaude Opus 5	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>	2026-08-28 17:34:07 +02:00
timothyandClaude Opus 5	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>	2026-08-28 17:34:07 +02:00
timothy	b16ec15d6c	docs(781,799): re-measure the tooling audit from a derived population, and adopt serena (#862) ... §5.3's verdicts rested on a single surface, which manufactured four false zeros: codex is driven through `codex exec` inside Bash, security-guidance and ralph-loop expose no tool at all and run as hooks (1,086 executions each), and feature-dev is used through its agents. The audit also compared current enablement against historical usage — six of the eight plugins it called "genuinely unused" were disabled for 16 of the 30 corpus days. The retirement half of #781 is answered *no* on evidence: the zeros split six ways and only one is grounds for removal. Eight plugins are kept by operator decision. #799's observation was correct and its cause is now established. serena was `false` in settings.json until 2026-08-14T12:31Z, when a concurrent session enabled it; its tools appear in no transcript before 12:42:54Z. #799's session started at 12:01Z and never reloaded, so its probe correctly found nothing while the settings file already said `true`. serena is adopted and documented as the third code-intelligence surface. Four review rounds, two independent cold reviewers (one cross-family); rounds 1-3 BLOCKED. fixes #781 fixes #799 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-28 14:37:34 +00:00
timothy	761e575836	fix(787): derive the dropped-step guard's scope, and reconcile its snapshot against the server (#861) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-27 22:37:02 +00:00
timothy	8aebba4d89	fix(748): declare permissions: on all six workflows, and prove the declaration binds (#860) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-27 22:02:44 +00:00
timothy	45b17e58e1	fix(744,835): ci-image.yml publishes from main only; guard persist-credentials with no exemption (#857) ... 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>	2026-08-27 20:44:44 +00:00
timothy	d68ce42fcb	fix(742): inherit an h10 verdict only from an allow-listed reviewer (#850) ... `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>	2026-08-27 02:52:02 +00:00
timothy	ed8b602445	feat(735): bound the numeric FFmpeg profile fields with a 422, and expose readrate pacing (#847) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 22:05:38 +00:00
timothy	469d19852c	fix(788): one declarative H10 verdict vocabulary, derived by both sides (#846) ... 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>	2026-08-26 20:58:09 +00:00
timothy	ba6a4b08aa	feat(732): On Now / Next gets a background box, and is on by default (#843) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 19:28:25 +00:00
timothy	f2551b778e	fix(746): drop the persisted checkout credential; unmask the base-ref fetches (#842) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 18:50:56 +00:00
timothy	40a3232d9e	feat(734): field-level progressive disclosure — shared FieldHelp trigger + panel (#841) ... Implements the three-level field-help pattern from #734 as a shared component: field name + optional one-sentence summary → a one-short-paragraph panel behind a consistent Info-icon trigger → a future external-docs deep link (`docsHref`, built and typed; no screen passes one yet). Adopted on FFmpegProfilesScreen (9 fields), documented as docs/spa-conventions.md §15 with decision record `spa.field-progressive-disclosure`, and mirrored into the design-system prototype. The panel is portalled to document.body: `.ctv-card` sets `overflow: hidden`, which clips a positioned descendant whatever its z-index, and one field's explainer rendered 12px of a 92px paragraph in every state of the Audio card. A `::before` hover bridge was added and then WITHDRAWN — it held for a vertical descent onto the panel and failed for a diagonal one, leaving a safe sideways exit of 1.25px on an 18px icon. Hover reads the paragraph in place; the panel's interactive content is reached by pinning. Four cold adversarial review rounds; the first three returned BLOCKED. They found five wrong copy claims across nine paragraphs and two vacuous tests in a row for the same mechanism. Deferred with owners: #839 (placement verified by hand, not by a test) and #840 (the portal puts a docsHref link at the end of the tab order). fixes #734 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 18:05:53 +00:00
timothy	35affecd29	docs(755): ersatztv owns the fork code, media-management owns channel operations (#838) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 17:29:16 +00:00
timothy	e8e17f3844	docs(708): record the live route-2 reproduction against PR #761 (#834)	2026-08-26 09:11:23 +00:00
timothy	08cd3a002d	fix(690,758): count the same query a paged handler pages (#833) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-26 07:39:23 +00:00
timothy	7453dd3a82	fix(721,740): align Auto-Tune proposal rows on a grid; guard AddItemsDialog's async searches (#831) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-25 21:55:23 +00:00
timothy	3e2c07b42f	fix(807): SPA full-replace bodies are built as Complete&lt;T&gt;, with a derived guard over droppable request members (#827) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-23 00:58:19 +00:00
timothy	7966e08164	fix(701): guard SongMetadata's nullable primitive collections at the read site (#826)	2026-08-22 23:24:41 +00:00
timothyandClaude Opus 5	b5dee26202	fix(701): guard SongMetadata's nullable primitive collections at the read site ... Both search indexers opened UpdateSong with metadata.AlbumArtists ??= []; metadata.Artists ??= []; Artists/AlbumArtists hold the whole list in ONE COLUMN rather than being navigations. So unlike the same `??= []` idiom on Genres/Tags/Artwork all around them, the property IS the column value: assigning it on a TRACKED entity flips the entry to Modified and the next SaveChanges writes [] over a NULL column. This is the mechanism an adversarial review demonstrated in #691, which is why that issue's entity-level guard was reverted in favour of guarding at the read site. Measured rather than reasoned about, per the issue's first done-when box. Restoring ONLY the `??= []` clause (the real predecessor lines, not a hand-written mutant) reddens the new fixture on `metadata.Artists should be null but was []`; a probe variant with the first two assertions replaced by prints reports STATE=Modified and the raw column moving from NULL to "[]". Today's two feeds are both AsNoTracking (SearchRepository.GetItemToIndex and GetAllSongs), so no shipped caller loses data -- but that is a property of two callers, not of the indexer, and #691 already recorded it as a loaded gun. The fixture pins the indexer's own contract instead. Removing the assignment is not sufficient alone: it was load-bearing for the four reads below it, and deleting it by itself converts a silent write into a live throw on every untagged song. Measured by deleting only those two lines from the real predecessor file: NullReferenceException, thrown at the foreach (cited by symbol: a line number in a mutant that exists in no committed tree is unreproducible by construction). The exception type follows the read FORM, not the field -- foreach yields NRE, string.Join/ToList yield ArgumentNullException -- and this PR contains two of each, which is why no single exception-name grep characterises the class. So each site moves together with its reads: - LuceneSearchIndex.UpdateSong / ElasticSearchIndex.UpdateSong: hoist Optional(...).Flatten().ToList() locals and read those. - RefreshChannelDataHandler: the Scriban context took the raw nullable lists (the issue's second item). The shipped _song.sbntxt only does array.join, but a custom template is free to do anything. The population was derived from the MODEL rather than from the issue's file list, and the obvious derivation is wrong: "the IList<string> properties under ErsatzTV.Core/Domain" returns two of eight. It misses the six value-converted collections (ProgramScheduleAlternate and PlayoutTemplate each carrying DaysOfMonth, MonthsOfYear, DaysOfWeek), declared as plain ICollection<T> and made single columns only in Data/Configurations -- and their storage differs (comma-separated text for the int converter, JSON for the enum one), so the shared property is "one scalar column", not the serialization. No site applies `??=` to any of the six, so this defect has no instance there; whether a null can REACH one at runtime is unverified and is filed as #823 rather than asserted either way. Only the SongMetadata pair is left NULL in practice, by FallbackMetadataProvider. Every site touching either field was then swept; the remaining readers were already guarded by #691. The fixture carries two anti-vacuity guards, both witnessed: - A POSITIVE CONTROL (`writer.NumDocs.ShouldBe(1)`). Every other assertion says something did NOT happen, so all of them hold vacuously if UpdateSong never runs -- and it silently stops running if a future refactor gates UpdateItems on `_initialized`, which this fixture bypasses by injecting the writer. Verified BOTH directions: with that gate added the control fails `NumDocs should be 1 but was 0`, and with the control removed the whole test PASSES while the code under test is unreachable. - A capturing logger, because UpdateSong wraps its body in a catch that assigns metadata.Song = null -- severing a required relationship and cascading the metadata to Deleted. Without it the probe silently measures the error path; on the first run it did exactly that (a bare ILanguageCodeService substitute NPEs inside AddLanguages). The raw column helper also fails loudly on a missing row, since ExecuteScalar returns CLR null for both "NULL column" and "no such row". ElasticSearchIndex has no equivalent fixture -- it needs a stubbed transport -- so its change is by inspection against the Lucene one, and the gap is filed as #824 rather than covered by a source-text guard. The whitespace-only churn in ElasticSearchIndex.cs is the #311 fix-as-you-touch format gate: it scopes to whole changed FILES. `git diff -w` over that file shows only the two hunks above. Local gate: ErsatzTV.Tests 2006 passed / 4 pre-existing skips, Core.Tests 685/1 skip, Infrastructure.Tests 114, Architecture.Tests 7, Scanner.Tests 1504 -- 0 failures in each. scripts/tests 874 passed / 2 skipped. dotnet format whitespace --verify-no-changes clean on the four touched files, no BOM on any. decisions_validate OK. Fixes #701 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-23 00:49:57 +02:00
timothy	78fc283627	docs(772): the merge gate decides on the COMBINED state, and skipped counts as green ... Graded a nit and not blocked on, but it is a claim about a neighbouring subsystem that is one notch too strong: the hook decides on the combined `.state` and its filters exclude `skipped` (#593). Left as-is it would have taught the next reader that any non-success context denies, which is what #593 exists to correct. refs #772	2026-08-23 00:39:41 +02:00
timothy	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	2026-08-23 00:23:44 +02:00
timothy	d304c8573e	docs(772): state the trap, not the draft history ... The advisory narrative check was right about both new passages: 'an earlier draft warned and exited 0' and 'both cold reviews found it independently' only parse to someone who saw the session. What a cold reader needs is that warning-and-exiting-0 is the natural way to write this check and is wrong, and what it costs — which is now what the doc says. refs #772	2026-08-22 23:48:52 +02:00
timothy	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	2026-08-22 23:48:20 +02:00
timothy	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	2026-08-22 23:25:26 +02:00
timothy	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	2026-08-22 22:52:56 +02:00
timothy	29d7a06e65	fix(806): guard populations over FILES derive from the git index, not a filesystem walk (#818) ... `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>	2026-08-22 16:07:58 +00:00
timothyandClaude Opus 5	3a53c50c45	docs: a cancelled job reads as failure where a monitor actually looks (#817) ... `ci.cancelled-is-not-a-verdict` documented the run/job API, where cancelled is distinguishable. The endpoint a CI monitor actually polls — `commits/{sha}/status`, the per-sha view the merge gate reads — has no `cancelled` state and reports one as `failure`. The record now says to resolve the job-level `conclusion` before reporting a red. The kickoff's push HARD CONSTRAINT is tightened from "a review has run" to "a CLEAN verdict, zero outstanding findings on the current tree", since #790's rounds 7 and 8 each still found a real mechanism defect and every earlier push auto-cancelled a live run. Refs #790 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-22 13:35:21 +00:00
timothyandClaude Opus 5	3b386649bf	feat(790): execute every MUTATION grade instead of asserting it (#815) ... 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>	2026-08-22 13:12:15 +00:00
timothy	d4c72697f2	feat(780): commit a ruff config and enforce it in CI (#813) ... Python lint here was a property of the operator's laptop: the global instructions say to run ruff, no workflow ran it, and with no committed config ruff fell back to whichever ~/.config/ruff/ruff.toml the machine happened to have. - ruff.toml at the root, pinned ruff==0.12.11 in the script-tests job. - Both lint steps pass an EXPLICIT population from `git ls-files` with `--no-force-exclude`, never `ruff check .` — an `exclude` empties a discovery-based run into a GREEN one (top level empties both commands, [lint] empties check, [format] empties format --check), and `ruff check .` over zero files exits 0 with only a stderr warning. Guarded by an empty-population arm. - Tree clean: 74 findings at 706674272, 57 fixed in code, 17 per-site noqa with reasons inline. S105 deliberately per-site, not a directory blanket. RUF100 selected so a suppression that suppresses nothing is itself a finding. - pyright stays ungated; reasoning in the record. Both steps witnessed red on the runner against the shipped bodies: run 2173 job 9176 (ruff check) and run 2170 job 9163 (ruff format --check). Docs: new record ci.python-lint-ruff-config-committed, ci.script-tests-job cross-ref, docs/ci-cd.md (also correcting a stale ~190-tests/~10s figure to the measured 773 tests / ~4.5 min), docs/defect-shapes-773.md §5.2 resolved. fixes #780 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-22 00:33:18 +00:00
timothy	6a4265d81d	feat(784): a doc records the end state — generalize the no-session-narrative rule (#811) ... Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-22 00:23:22 +00:00
timothyandClaude Opus 5	706674272c	test(785): mutation proofs for the three unproven ranked guards (#810) ... 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>	2026-08-21 20:10:01 +00:00