230 Commits

Include renames

Author	SHA1	Message	Date
timothy	64d80e5d28	docs(786): ci-image.yml has no pull_request TRIGGER; it is not an if:-gated job ... Self-review of the scope-gap paragraph added one commit earlier. It said both publish jobs carry `if: github.event_name != 'pull_request'`. Measured: only `docker-build.yml::build` does. `ci-image.yml` declares triggers `push`, `schedule` and `workflow_dispatch` — no `pull_request` at all — so its job is never CREATED on a PR rather than being created and skipped. The conclusion the paragraph draws is unchanged (PR CI cannot exercise the job-level env scope), but the two mechanisms are worth keeping straight: an `if:`-skipped job is still dispatched as a runner task, which is the behaviour #535 was about, while a job whose workflow has no matching trigger produces no task at all. Refs #786	2026-08-28 22:14:44 +02:00
timothy	1ab50236c3	fix(786): share the last three detectors with their proofs, and close the round-4 wording items ... Round four returned MERGEABLE with seven LOW/NIT items. All seven are fixed rather than shipped, because three of them are the same defect class this branch has been closing all along. THREE MORE PROOFS RE-IMPLEMENTED THE COMPARISON THEY NAMED. The previous commit extracted `duplicate_row_faults` and `env_shadow_faults` for exactly this reason and left the set-equality and Kind comparisons inline, so `test_MUTATION_a_guard_job_with_its_row_DELETED_is_reported`, `…_a_PHANTOM_row_is_reported` and `…_a_row_whose_KIND_disagrees_is_reported` still rebuilt `declared - listed` and the Kind check themselves. Review demonstrated it by flattening the live assertion and watching the file stay green at 50 passed. `coverage_faults(declared, listed) -> (missing, phantom)` and `kind_faults(roles, listed)` are now named detectors that the live checks and all three proofs call, and each proof gained a negative control. Witnessed after the change: coverage_faults disarmed -> deleted-row proof REDDENED, phantom-row proof REDDENED kind_faults disarmed -> kind-disagreement proof REDDENED Worth recording that the system was never fail-open here: the manifest harness already caught the same disarm ("the named test still PASSED with the clause mutated"). The in-file proofs were weaker than their names, not absent. THE ENV-MERGE EVIDENCE WAS STEP-SCOPE USED FOR A JOB-SCOPE CLAIM. `build`'s Smoke step shows the runner LAYERS step-level `env:` over workflow-level, and what this branch newly relies on is job-level layering. Measured: on origin/main no job anywhere declared both a job-level `env:` and a consumed workflow-level var, so the job scope had no in-repo witness. The gap is now named, with the exposure bounded — the two publish jobs are `if: github.event_name != 'pull_request'`, so PR CI cannot exercise it and a non-layering runner would fail loudly on the first push to `main` trying to publish `":<sha>"`, not silently. Four wording fixes, each measured: * the record still enumerated "exactly three jobs are `none`" while three other sites say a list in prose is a second copy that rots — the fourth site now agrees with them. * "the remaining `docker-build.yml` jobs" over-scoped: six remain after `test`/`migrations`, and the table has four, because `toolchain-preflight` and `scan` carry the mark/assert protocol voluntarily without being in the enforced scope. Said so. * "on every push to `main`" ignored the Smoke step's own `docs_only` gate. * `docs/ci-cd.md`'s excluded-token list omitted `tar`, which the source comment includes; and the `scan` Proof cell disclosed the two pytest files' `NONE` but not the cited script row's, two cells from a sibling that discloses it — so that row now states it claims no proof at all. Suite: 1178 passed, 2 skipped (the production-log guard deselected; it is the shared-$HOME flake, green in isolation — #809/#822). ruff 0.12.11 clean over all 47 tracked files; decisions-validate OK; doc-narrative 0 warnings. Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	f45b0593ca	docs(786): reconcile the three remaining statements of the Proof: NONE rule ... The rule was hedged in `docs/guard-inventory.md` when review found two inline rows citing a pytest despite it. The record's `rule:` frontmatter, the record body, and the checker's module docstring kept the universal wording — the same fix-one-site-leave-the-paraphrase pattern this branch has now hit four times, which is why the sweep was by the RETRACTED words rather than the new ones. All three now say the same thing: an inline row carries `Proof: NONE` unless a pytest genuinely covers part of it, which `ci-image-pin` and `set-verdict-status` do, and each names which part — because a reference covering a fraction must not read as covering the whole. Refs #786	2026-08-28 22:14:44 +02:00
timothy	003a13574d	test(786): catch a marker SMEARED onto the wrong job, which no key-based assertion can see ... `test_the_marker_is_read_from_the_PARSED_yaml_not_a_text_search` claimed to guard the parser against regressing to a grep. It asserted only that every tracked job appears as a KEY, so a reader returning the right keys with wrong VALUES satisfied it — the claim was stronger than the code. Cold review found it; the same round found the sibling test making the same over-claim, and that one is now fixed by driving the real reader. Renamed to what it proves (`test_every_tracked_job_APPEARS_in_the_parse`), with the grep-regression claim moved to `test_a_commented_out_marker_does_NOT_count`, which actually drives the reader. Added the property a completeness check structurally cannot express: ATTRIBUTION. A marker belongs to the job whose `env:` block contains it. A line-oriented reader that attached each `CI_JOB_ROLE:` to the nearest preceding job header would smear one job's marker onto a neighbour declaring none, and both jobs still have keys, so nothing phrased over keys can see it. The fixture is two jobs, only the second marked. Witnessed, and the control matters: the smearing reader used to demonstrate it is byte-identical to the real `declared_roles()` on the current tree — faithful on the real tree? True REDDENED: the reader attributed a marker to the wrong job — `first` declares none and must read as None, `second` declares one and must read as its value — so it satisfies every population and completeness assertion in the file, and only the new fixture separates them. Refs #786	2026-08-28 22:14:44 +02:00
timothy	e855a79999	fix(786,789): make the new proofs exercise the shipped detectors, and rename advisory -> report-only ... Round three, reviewed twice in parallel (Codex gpt-5.6 and a cold Claude brief). Both converged independently on the same two MEDIUM findings, which are the branch's own defect class reproduced by its most recent commits: a test asserting a guarantee it does not provide. 1. THE PROOFS DID NOT EXERCISE THE CODE THEY NAMED. `test_MUTATION_a_DUPLICATE_row_is_reported` rebuilt the comparison with its own `len(set(...))`, and `test_MUTATION_a_SHADOWING_env_key_is_ reported` built a synthetic dict and intersected it by hand. Both stayed GREEN with the live check disarmed — a behavioural test wearing a MUTATION name, which is exactly what `testing.guard-ships-with-mutation-proof` excludes. `duplicate_row_faults(rows)` and `env_shadow_faults(docs)` are now named detectors; the live test and its proof both call them. Verified by reproducing the reviewers' own demonstrations against the fix: duplicate detector neutered -> REDDENED "the DETECTOR did not report a second row for ..." shadow detector neutered -> REDDENED "the DETECTOR did not report a shadowing job-level key" 2. `test_a_commented_out_marker_does_NOT_count` CLAIMED TO GUARD A TEXT-SCAN REGRESSION AND DID NOT. It re-implemented the read inline and never called `declared_roles`. One reviewer built a comment-blind grep reader reproducing all 17 current values byte-identically and ran every non-parametrised test in the file: nothing went red. `declared_roles(paths=None)` now takes an injectable population, so the test drives the REAL reader over a fixture, asserts the fault function reports it, and pins that a live marker in the same shape is still read (so the assertion cannot be satisfied by a reader returning None for everything). Verified: installing the grep reader now reddens it. RENAMED `advisory` -> `report-only`. docs/ci-cd.md already calls `functional-e2e` an advisory job in a DIFFERENT sense — not a required context, though it can certainly fail — while this branch declares that job `guard`. A reader would infer the wrong marker for exactly the job whose marker is `guard`. The value is now unambiguous, its Kind is `REPORT-ONLY`, and the manifest's declared `expect` string moved in lockstep with the message it matches. The rename also caught its own adjacent defect: `REPORT-ONLY` contains a hyphen, which the row regex's `[A-Z]+` Kind group did not match, so the docs-reminder row stopped parsing. That surfaced as a loud red naming the job — the fail-closed property documented one commit earlier, working. Widened to `[A-Z-]+`, matching the `BEHAVIOUR-ONLY` convention in the sibling table. Nine further accuracy findings, each measured before fixing: * "would therefore flag every job" — false. git/jq/python3/curl flag 4 of 8. And the REASON was wrong too: `small` is git-only and provisions Python via actions/setup-python, while `build` runs on stock ubuntu-latest, so "the bare runner has them" conflated two lanes. They are excluded because they do not DISCRIMINATE. * "four lines from the container: block" / "in one diff hunk" — false. Measured 6, 58, 6, 7, 7; `migrations` has the whole `services:` block in between. The property the guard rests on is INDEPENDENCE, not proximity, and the docs now say so. * "the merge semantics never have to be relied upon" — too strong. The check proves no NAME COLLISION; it does not prove the runner merges. That evidence is in-repo and live and is now cited: `build`'s Smoke step declares its own step-level `env:` and consumes workflow-level `${IMAGE}` on the same line, on every push to main. * "every malformed row fails closed" — over-broad. A malformed DUPLICATE is invisible: its job is already satisfied by the well-formed row. Stated. * The `scan` Blocks cell said "a marked job's run: body"; the ban's scope is DELIMITER_BAN_JOBS (the required contexts PLUS `build`), which is wider, and `scan` carries no `if:` so it runs on PRs too. * The functional-e2e row credited `e2e-local.sh` (TOOLING, only boots the instance) for curl assertions that live in `e2e-functional.sh` (GUARD). * `report-only` was defined nowhere; now defined as a verdict the job cannot enforce, with the limit stated (nothing detects a `guard` job whose checks are all continue-on-error). * "inline rows carry Proof: NONE" was stated universally while two rows cite a pytest; hedged, and each such row names which PART it covers. * "Exactly three jobs are `none`" was an unchecked prose enumeration in three sites, in a branch that deleted one for that reason. Removed; the markers are the source. * Plus: the ci-cd tool list was short by `dotnet-ef` and `playwright`; docs/README said only a `guard` job needs a row; "all three checks fail together" read as the opposite of "go blind"; and #774's unrealised consequence is now subjunctive rather than past tense. Suite: 1178 passed, 2 skipped, 1 known flake (test_hook_fire_log's production-log guard, which compares mtimes under the SHARED ~/.cache/ersatztv/hook-fire/ — four session logs were written during the window; 78 passed in isolation. ersatztv#809/#822, evidence posted there). Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	2cc97743c0	docs(786): record the parser's measured fail-closed property ... `_ROW`'s `[^|]*` Blocks cell means a raw pipe inside it — a code span such as `grep ... | cut ...` — stops the row matching. An unmatched row is an ABSENT row, which the set equality reports as a guard job with no entry, so the failure is a loud red rather than a silently skipped row. Measured rather than reasoned: injecting a pipe into one Blocks cell reddens test_the_inventory_covers_exactly_the_guard_JOBS_that_exist naming that job. Recorded because a regex over a Markdown table invites the opposite assumption, and the next person to widen a cell should know which way it breaks. Refs #786	2026-08-28 22:14:44 +02:00
timothy	ae4bf8b662	fix(786,789): guard against env shadowing, and close the weaker proof shapes ... Second cold re-review (Claude, cold brief, isolated worktree) of the tree at 0944877bf. Its BLOCKER (the retracted rule surviving in four sites) and its duplicate-row finding were already fixed in d8a507a3b, which it did not see. Six further findings remain and are fixed here. JOB-LEVEL env COULD SHADOW A MEMORY-CRITICAL WORKFLOW-LEVEL ONE. Raised as an unmeasured assumption, and it is the one worth acting on: before this branch NO job in docker-build.yml declared `env:` at all, and that workflow's workflow-level `env:` carries UseSharedCompilation, DOTNET_CLI_USE_MSBUILD_SERVER and MSBUILDDISABLENODEREUSE — what keeps a 7.8 GB VBCSCompiler off a RAM-oversubscribed runner (#406). Actions merges the scopes with the more specific winning, so a same-named job-level key replaces the workflow value and the loss is INVISIBLE: the job still runs, just with the shared compiler back on. Rather than measure the merge once and write the answer in prose, the reliance is removed: test_no_job_env_key_SHADOWS_a_workflow_level_one asserts no job-level key collides with a workflow-level one, in any tracked workflow. Measured clean today across all six. Witnessed red by planting `UseSharedCompilation: true` on `test`: docker-build.yml::test declares job-level ['UseSharedCompilation'], which also exists at workflow level. The job value WINS and the workflow value is silently lost. TWO PROOFS WERE WEAKER THAN THEIR TWINS. * test_MUTATION_a_guard_job_with_its_row_DELETED_is_reported had no negative control, so a PRE-EXISTING missing row satisfied every iteration. Its phantom twin got one in c00126b82 and this one did not — the same asymmetry, one commit apart. It now pins `declared - listed` to empty first and asserts the deletion is reported as EXACTLY that job. * test_a_commented_out_marker_does_NOT_count asserted that PyYAML ignores comments — a fact this suite does not own and that no change here could break. It now writes a workflow to tmp_path, drives the real `role_faults`, and asserts the commented text IS present in the file, which is the actual guard: a reader that ever became a text scan would find it. A PROSE COUNT SHIPPED FIVE LINES ABOVE THE ONE 0944877bf DELETED. "Only three jobs are `none` —" enumerated the workflow by hand, unasserted, in the same file where the previous commit replaced a count with an invariant for exactly that reason. Removed: which jobs are `none` is read off the markers, and the table is the only enumeration a check keeps honest. Three accuracy fixes, each measured: * docs-reminder: "all three check steps carry continue-on-error" is off by one — there are TWO check steps plus a `Set up Python` between them, all three with the flag. The substantive claim (no finding can fail the job) was right. * The two REQUIRED-context strings dropped the ` (pull_request)` suffix that .gitea/required-status-contexts.json carries, so a reader copying one into branch protection would get a context that never reports. Both now match the file exactly, verified against it. * The `_FULL_LINE_COMMENT` comment read as though stripping cured a live red. It did not: measured, ZERO toolchain tokens appear in the raw `run:` bodies of all three bare-runner jobs, comment lines included. It is preventive, and now says so — along with why it cannot manufacture a match (`.sub` on a `^…$` match leaves the newline, so no lines are joined). The re-review independently re-derived the verdict/artifact classification across all 17 jobs and would not reclassify any, and confirmed the three earlier de-vacuuming fixes hold under execution. Suite: 1179 passed, 2 skipped. ruff 0.12.11 clean; decisions-validate OK. Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	402032db98	fix(786,789): sweep the retracted rule out of three sites, and reject duplicate table rows ... Cross-family cold re-review (Codex/gpt-5.6) returned BLOCKED with four findings. All verified independently before fixing. 1. HIGH — THE RETRACTED RULE WAS STILL STATED AS ACTIVE IN THREE PLACES. The previous commit replaced the guard/none rule in docs/guard-inventory.md and flipped `test`/`migrations` to `guard`, but left the superseded wording in test_workflow_job_guards.py's module docstring, docs/ci-cd.md, and the decision record's `rule:` — from which the generated catalog (and MemPalace) copied it faithfully. Those three explicitly called `docker-build.yml::test` `none`, which is the opposite of what the workflow now declares. A maintainer following the ACTIVE decision record could downgrade both required jobs, delete their rows, and every structural check would stay green, because the checkers read the markers and the markers would agree with the stale prose. This is `process.enumerate-clauses-to-close-a-sweep` failing on my own retraction: I fixed the site I was looking at and left the paraphrases. All three now state the verdict/artifact rule, and each carries the rejected reading explicitly so it does not come back. Swept by grepping the RETRACTED words, not the new ones — the only hits left are the deliberate rejected-alternative notes. Same commit: docs/ci-cd.md claimed "every job in every workflow declares two things", which its own table contradicts — `CI_EXECUTION_CLASS` is docker-build.yml only. 2. MEDIUM — A DUPLICATE ROW WAS SILENTLY COLLAPSED. `inventory_jobs()` built a dict, so two rows for one job overwrote each other, last-one-wins, and set equality still held both ways. The realistic arrival is a merge conflict leaving an ADVISORY and a GUARD row for the same job: the registry contradicts itself and its checker reports green. Rows are now kept as a list and counted against distinct keys by `test_no_job_has_TWO_rows`, with a mutation proof carrying its own negative control. Witnessed: planting a contradictory duplicate row for `pr-checks.yml::ci-image-pin` in the real file reddens two named tests; restoring it returns 48 passed. 3. LOW — the toolchain-preflight row said it blocks "every downstream job". Measured: NOTHING `needs:` it (only `build` has a `needs:`, on test/migrations/scan), and it is not a required context. Corrected to what it actually blocks — a merge, through the combined commit status the consent gate reads (#598) — and why it must not consume the image it checks (#772). 4. LOW — the predicate's documented residue was partial. Now enumerated in both directions: false positives (trailing comment, echo string, heredoc body — stripping is line-based, not shell-aware) and false negatives (absolute path, script indirection, a `#` heredoc line that a later command strips and executes). One of those claims was WRONG as first written and is corrected to what execution shows: variable indirection is invisible ONLY when the assignment is not in the same run body. `$SDK_CMD build` with `SDK_CMD` from `env:` is missed; `SDK_CMD=dotnet` written in the same body IS caught, because the assignment line carries the bare token. Re-review confirmed the four earlier findings are genuinely fixed: the classification is consistent across all 17 jobs under the verdict/artifact rule with no job it would reclassify; both repaired positive controls redden on an emptied population; the corrected functional-e2e claim is true and the old wording has no matches; the adjacency residual is accurately disclosed and not overstated; the summary counts assertion holds at (43, 6, 21, 21, 6, 16). ruff 0.12.11 clean; decisions-validate OK; targeted suites 145 passed. Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	0c3625d2e2	fix(786,789): reclassify test/migrations, de-vacuum two controls, correct a refuted claim ... Cold adversarial review of b7e5c35fb returned BLOCKED with four substantive findings. All four are real and all four are fixed here; each fix is demonstrated rather than asserted. 1. THE guard/none LINE EXCLUDED BOTH REQUIRED CONTEXTS. The rule was "a guard enforces an invariant about the REPOSITORY; a job exercising the PRODUCT is `none`". It reads as principled and put `test` and `migrations` outside the table — the two required Actions contexts on `main` per .gitea/required-status-contexts.json, i.e. exactly the jobs where a failure to fire is fail-OPEN against branch protection, and exactly the gap #786 exists to close. It also could not survive its own examples: `test` runs ToolCatalogTests.cs and web/src/api/*.guard.test.ts, the structural guards this document's own scope-limit item 2 names. Replaced with a line that can be restated: a `guard` job's output is a VERDICT; a `none` job's output is an ARTIFACT. A `none` job can still go red — that is an error in producing the thing, not a finding about the repo. `test`, `migrations` and `functional-e2e` flip to `guard` and gain rows; `none` is now exactly the two image publishers plus renovate. The rejected rule is recorded so it is not re-adopted. 2. TWO POSITIVE CONTROLS WERE VACUOUS — both of them ones this commit series added, while the pre-existing sibling it copied was not. `len(_MUTANTS) == 2 * len(population)` is `2N == 2N`, a tautology at N=0, so an emptied population satisfied it while the parametrised proofs collected zero cases. Both now assert non-emptiness FIRST. Demonstrated after the fix, by degrading each population to empty: role mutation set -> REDDENED: "declared_roles() is EMPTY ..." declaration mutation set -> REDDENED: "the workflow parsed to ZERO jobs ..." phantom-row proof -> REDDENED: "the table parsed to nothing ..." 3. A MECHANISM CLAIM IN A DECISION RECORD WAS REFUTED BY EXECUTION. Four sites (the test docstring, docs/ci-cd.md, the record's `rule:`, and the generated catalog) said `functional-e2e` reaches dotnet only through scripts/e2e-local.sh and so is invisible to the step scan. It is not: that job runs `dotnet restore`, `npm ci`, `npm run build` and `dotnet build` in its own YAML, and the derivation detects it. Measured: the blind-spot set (declared toolchain minus detected-from-own-steps) is EMPTY today, all five jobs detected. Reworded to say that, and the converse is still not asserted because a future job could be written behind a script. 4. THE ADJACENCY RESIDUAL WAS UNDERSTATED. All three checks fail together under one edit that drops the container block, flips the marker AND moves the invocation into a script — the review verified it. Now disclosed in the code and in docs/ci-cd.md, with what actually bounds it: the failure MODE, not its likelihood. The job then dies on a missing binary, loudly, where #774's original defect sent a REQUIRED check green on the bare runner. A silent pass traded for a noisy crash, not a hole closed. Also from the review: the docs-reminder row said the job "never fails" (its Checkout step can); the phantom-row proof could not fail (it supplied its own phantom, so it now pins the unmutated set to zero phantoms first); and the scope-limit paragraph was reworded from draft history into a rejected-alternative note. Review findings 6 and 7 (regex false positives on comments, false negative on `dotnet-ef`) were already fixed in 75747fbca, which the reviewer did not see. Suite: 1161 passed, 2 skipped. ruff 0.12.11 clean; decisions-validate OK. NOTE for whoever sees a red `test_hook_fire_log.py::test_the_suite_does_not_write_to_the_ PRODUCTION_log` locally: it snapshots mtimes under ~/.cache/ersatztv/hook-fire/, which is shared across worktrees and sessions, so a second Claude session running concurrently reddens it. Seen once here with three session logs written during the window; green in isolation, both before and after this change. That is ersatztv#809 / #822, not this diff. Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	f420649fa2	fix(786,789): harden the step-text predicate and sharpen the guard/none rule ... Self-review findings on the previous commit, all measured before fixing. THE PREDICATE HAD BOTH FAILURE DIRECTIONS. Measured against constructed inputs: full-line comment -> ['dotnet'] false POSITIVE echo string / heredoc -> ['dotnet'] false POSITIVE dotnet-ef migrations -> [] false NEGATIVE playwright install -> [] false NEGATIVE The false positive is the dangerous one: a bare-runner job whose COMMENT merely mentions dotnet would redden the suite on a correct tree, and a correct guard going red on a correct tree is the fastest route to that guard being deleted. Full-line comments are now stripped before matching. `dotnet-ef` and `playwright` join the tool list, and the alternation is sorted longest-first so the match does not depend on backtracking out of the `dotnet` prefix. The residue is stated in the code rather than papered over: a TRAILING `# ...` comment, an `echo "run dotnet build"` and a heredoc body all still match. Parsing those means parsing shell, which `testing.fixing-a-parser-bug-introduces-the-next-one` prices at three or more rounds. It is tolerable only because of its DIRECTION — a loud red naming the job and the token, with an obvious remedy — never a silent pass. Verified the real workflow's classification is unchanged and exact after the change: needs-toolchain derives to {test, migrations, functional-e2e, api-docs, format}, equal to the declared set, zero violations. THE GUARD/NONE RULE WAS UNDER-SPECIFIED. "Purpose, not whether a step can fail" does not decide `docker-build.yml::test`, which blocks the merge as a required context. Sharpened everywhere it is stated: a `guard` job enforces an invariant about the REPOSITORY or its ARTIFACTS; a job that builds or exercises the PRODUCT is `none` even when it is a required check. No marker value changed — the rule now explains the classifications that were already there. Also: a row for test_workflow_job_guards.py in the file-populations table, and the scope-limit preamble de-narrated (it recounted an earlier draft; the operative warning — an unlisted class reads as covered — is kept, the autobiography dropped). Refs #786 Refs #789	2026-08-28 22:14:44 +02:00
timothy	20a6fed165	fix(786,789): workflows declare their own per-job metadata; guard populations derive from it ... together. #786 asked for inline workflow-job guards to be enumerated using "the same reviewed-registry-cross-checked-both-ways shape TOOLCHAIN_JOBS uses"; #789 said that exact shape is the defect and must be deleted. Building #786 as written would have constructed the artifact #789 removes. Both now derive from a marker the workflow declares about itself: env.CI_EXECUTION_CLASS toolchain | bare-runner (docker-build.yml) env.CI_JOB_ROLE guard | advisory | none (every tracked workflow) Missing or unknown values are hard failures in both checkers — a marker scheme whose absent value defaults to a safe class stops applying the moment a job is added and nobody notices. it: set equality between two DERIVED sets is blind to a member leaving both at once, so a job losing its container: block leaves the declared and pinned sets balanced. The population needs an anchor that does not move with the block. The marker is that anchor and is better placed — claim and reality in one diff hunk. Its cost is adjacency, and is paid rather than argued away: a marker four lines from the container: block can be deleted with it, where a distant literal could not. So the file derives the requirement a THIRD time, from each job's own step bodies, independent of both marker and block. That is also the only check that sees the failure #789 filed — a .NET step MOVED into a bare-runner job, where no set changes at all. It is a necessary condition only (functional-e2e reaches dotnet via scripts/e2e-local.sh), and says so; the converse is deliberately not asserted. test_workflow_job_guards.py asserts set equality both ways against a new "Workflow-job guards" table in docs/guard-inventory.md. The marker records the job's PURPOSE, not whether a step can fail: docker-build.yml::build runs a smoke test that fails the job and is still `none`. Read the other way, every job is a guard and the table distinguishes nothing. The four jobs #786 named as having no dropped-step guard each get a recorded decision in that section. None is a required context, so a drop there cannot send a required check green; build's residual was already self-documented and is now indexed. ci-step-ran.sh is deliberately not widened — its scope is derived from the required contexts (#787) and test_ci_dropped_step_guard.py argues the ban belongs where a drop is consequential. Two corrections to the issues, both measured rather than assumed: * #789's "editing docker-build.yml re-points ci-image-pin's expected" is false. expected="$(git log -1 --format=%H -- docker/ci)" — the pathspec is docker/ci only. Verified: docker-build.yml was last touched by HEAD and expected still resolves to 32747a0, matching the pin. No publish-then-pin two-step was needed. * #786's "15 jobs across 6 workflows" is 17. Proofs, all executed: three real mutations of docker-build.yml witnessed red (container block removed, marker removed, dotnet step planted in a bare-runner job); the two new checkers ship parametrised mutants over every job; the declared clause in mutation_manifest.py is retargeted to the surviving comparison, and the new checker's entry mutates the guarded ARTIFACT (a renamed table row) per the checker-guard exception. test_guard_inventory.py's row parsing is scoped to its own section so the second table cannot be read as phantom guard files. Suite: 1161 passed, 2 skipped (was 1083) — scripts/tests, ruff 0.12.11 clean. Refs #786 Refs #789	2026-08-28 22:14:28 +02: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	11287a54ba	fix(763): satisfy ruff — E741 and formatting on the touched test file ... CI's `Script lint and tests` job went red. Cause: I never ran ruff locally, which this repo's Python convention requires after any .py change. - E741 twice: `l` as a comprehension variable in the sort-order guard. - `ruff format --check`: the file was correctly formatted on `main`; my edits broke it. One of them left a docstring line at column 0, which `ruff format` then "corrected" by over-indenting the rest of the paragraph — repaired at the source rather than accepting that rewrite. Verified the way CI does: local ruff is the pinned 0.12.11, and both `ruff check` and `ruff format --check` run under bash over the full tracked population (`git ls-files -z '*.py' '*.pyi' '*.ipynb'`, 46 files) are clean. The population is counted, not assumed — an empty glob would pass vacuously, which is the failure `scripts/tests` guards against elsewhere. `scripts/tests` 1097 passed, 2 skipped after the reformat. refs #763 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 18:51:32 +02: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	4164efbc3e	fix(763): round 8 — a fixture must pose the scenario it narrates ... A fifth cold review confirmed the gate's behaviour is correct and proof-backed, and blocked on three non-behavioural items. All three fixed; none touches the shipped logic. MEDIUM — the round-7 fixture narrated a raced human verdict it did not construct. `null-page1-after-post` appended the row unconditionally, so it also joined the PRE-write read and lifted the high-water mark above itself; removing it changed nothing. The reviewer's suggested fix was to gate the append on the post-write read. Measured after gating: still inert, because page 1 answers `null` before any row reaches the wire. So the row is gone rather than gated, and the prose now describes what the fixture actually poses: a response asserting an empty history for a sha this job wrote to must not be accepted as proof that nothing raced. Whether a verdict really raced is not modelled and does not need to be — the response is not evidence either way. A row the test cannot observe is decoration that reads as coverage, which is the same class this branch has now been blocked on five times. LOW — the comment claimed the predecessor "at least produced a `::warning::`". Half false, measured against `origin/main`: its `jq -e 'type == "array"'` gate ACCEPTED `[]` silently and warned only on `null`. What is actually new is that the paged walk reports such a read as a SUCCESS. LOW — when the empty clause fired it set `ph_ok=no`, so the log said "could not be read completely" beside a walk that completed on a validated terminator. The answer was impossible, not unreadable, and an operator holding a sticky sentinel needs to know which. It now carries its own `raced_why`, asserted by the test. Both clauses mutation-proved: disarming the empty check, and reverting to the borrowed wording, each redden the named test. Verification: `scripts/tests` 1097 passed, 2 skipped; decisions_validate and build_decisions_catalog --check exit 0. refs #763 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 18:18:37 +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	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	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	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	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	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	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	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	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	eb5c74b4b2	fix(780): reconcile the ruff gate with #784, merged in parallel (#814) ... #780 and #784 were green separately and red together: the gate landed on a base that predated scripts/check-doc-narrative.py, so nothing ever ran ruff over it. - RUF100 x2 on `# noqa: BLE001` — BLE is not in this repo's select, so those directives suppress nothing. Enabling BLE instead was measured and rejected: 6 further sites in decisions_validate.py, whose broad catches are deliberate. The non-enabled code is dropped; S110 and both comments stay. - scripts/tests/test_check_doc_narrative.py was not ruff-formatted. Verified with the shipped invocation: 35 files, all checks passed, all formatted; suite 807 passed / 2 skipped. refs #780, refs #784 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>	2026-08-22 09:34:03 +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
timothyandClaude Opus 5	7347a08979	fix(778): the 200-not-an-array arm had the same lying reason, one branch earlier ... Local review returned MERGEABLE — no Blocker, High or Medium, both sentinels verified pinned by mutation, no regressions. Its one Low is taken rather than deferred, because it is a one-line fix and because deferring it would leave exactly the shape this PR exists to document. A 200 whose body is NOT an array never reaches the classifier: the array gate diverts it, `bp_code` stays 200, and the generic ask then reported "HTTP '200' — Gitea unreachable, or these credentials lack the repo-admin scope" about a read that plainly succeeded. That is the identical defect the previous commit fixed for the throw-inside-the-classifier arm, one branch earlier — fixed where it was noticed, left in its twin. The previous commit's message even generalised the pattern ("a sentinel that doubles as an HTTP code makes a decision state a cause that did not happen") while covering only one of the two arms it applies to. The test is now parametrised over all three shapes that reach an unusable 200 — UNPARSEABLE-RULES, GARBAGE, EMPTY — and reverting the new sentinel reddens the two that the first fix missed. Also finishes the de-indent the previous commit claimed: that comment block went from 19 leading spaces to 6 while its siblings use 2, so the claim was true of the direction and not of the result. 729 tests green. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 22:50:23 +02:00
timothyandClaude Opus 5	3f0af38509	fix(778): pin the nomatch sentinel — it shipped unpinned, exactly like the guard before it ... Local review of the previous commit returned BLOCKED. Both findings taken; nothing pushed to CI while this was iterating. MEDIUM — the `nomatch` fix was entirely unpinned. Reverting both sites to `bp_code=404` left the suite 33/33 green, because no fixture ever emitted an HTTP 404 on the LIST read: the codes exercised were 000, 403, 500 and 200, and the old `NOT-FOUND` mode had been repurposed to return 200 with `[]`. So the defect that commit describes could be reintroduced silently — in a PR whose subject is unfalsifiable guards, one round after being blocked for precisely that shape. There is now a `LIST-404` fixture and a test; reverting the sentinel reddens two tests. The same class, one arm over and found while fixing it: a 200 whose `branch_name` is a number makes the classifier throw (`//` fires only on null/false), and that was mapped to `bp_code=000`, reporting "HTTP '000' — Gitea unreachable" about a read that plainly succeeded. It gets its own `unreadable-rules` sentinel and message, with a fixture and a test — reverting it reddens. The pattern across both: a sentinel that doubles as an HTTP code makes a decision state a cause that did not happen. The decision was safe each time; only the reason lied. MEDIUM — four comments still described 404-as-a-finding as live, contradicting the hook comment added in the same commit. The worst said a 404 means "this branch is entirely unprotected" in a test whose fixture now returns 200 with `[]`, which would have talked the next reader into re-adding the deny. Renamed that mode `EMPTY-LIST` so it says what it is. Also: the hook quoted a reason string it no longer emits; a stray over-indented line survived the de-indent; `bp_called` was write-only after its reader was removed. 727 tests green. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 22:09:16 +02:00
timothyandClaude Opus 5	7d81f85b38	fix(778): make the by-name pin falsifiable — it was filtering on the property it asserted ... Confirmation pass returned BLOCKED, and its lead finding is the one worth having. The test forbidding the by-name lookup recorded URLs from INSIDE the `endswith("/branch_protections")` branch, so the only URLs it could ever record were ones that already satisfied the assertion. A by-name request was invisible to the very test written to forbid it. Cold review proved it by reintroducing the lookup in the hook: the suite stayed 33/33 green. That is the filter-on-the-asserted-property defect this PR's sibling record exists to describe, committed inside the guard against it — and the commit message had called the twin "pinned so it cannot come back". The recorder now sees every branch-protection URL whatever its shape. Re-verified by the same mutation: reintroducing a by-name call reddens exactly the two tests that forbid it. Also from that pass: - an HTTP 404 on the LIST read reached the "the full rule list was read and none matches" deny — a claim about a read that never happened. Gitea answers 404 for a repo that is absent or invisible to the credential, so the classifier's own verdict is now the sentinel `nomatch` and HTTP failures reach the ask; - two comment blocks still described the deleted by-name endpoint as live, one of them asserting the classifier "is never reached at all"; - the decision record still documented `branch_protections/{base}` and its 404 semantics as the mechanism, in the record this PR authored — now rewritten to the list endpoint, with why reading the LIST is the load-bearing choice; - seven assertions on a string the hook no longer emits, and three test names/docstrings describing the removed 404 flow; - an unused fixture helper, and 79 lines left over-indented by the removed nesting. 724 tests green. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 21:29:09 +02:00
timothyandClaude Opus 5	f3863c3720	fix(778): delete the by-name lookup — the precedence check was guarding the dead path ... Final review returned MERGEABLE with no Blocker and no High. Its one Medium is taken, and it is my own recurring trap for the third time in this PR: fix one path, then check its TWIN. The hook looked a rule up by NAME first and enumerated the rule list only on a 404. But `branch_protections/{name}` is an exact DB lookup — `GetProtectedBranchRuleByName` — which performs no matching and knows nothing about precedence. A 200 from it means "a rule with this NAME exists and lists this context", never "this context is required on this branch". So the precedence argument added last round guarded the 404 path while the 200 path granted without it — and since this repo's rule IS named `main`, the by-name lookup always returns 200. The hardened code was dead and the unhardened code was live. Given a rule `main` requiring review-verdict/h10 and a rule `m*` with better Priority that does not, Gitea applies `m*`; the by-name hit on `main` saw h10 and granted anyway. Fixed by DELETING the twin rather than documenting it: one fetch of the full list, one classifier, one argument, no second path to keep in step. Two things fall out for free — the ref no longer reaches a URL segment, so the percent-encoding hazard is removed by construction rather than escaped (its test is replaced by one asserting no ref reaches the URL at all), and every case the classifier already covered now applies to the live path instead of an unreachable one. Verified against the live Gitea: the classifier returns `exact` -> rule `main` -> enable_status_check=true, h10 present. A new test pins that the precedence check runs even when an exactly-named rule exists, and asserts the by-name endpoint is never requested, so the split cannot come back silently. The grant string now states what was actually established — read from the full rule list, matched with Gitea's own plain-vs-glob split, refusing wherever precedence or folding is not derivable — rather than the stronger "confirmed required" it claimed while consulting a single named rule. 724 tests green. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 21:07:09 +02:00
timothyandClaude Opus 5	c7bea972d6	fix(778): close the last three residuals — ambiguous exact rules, non-ASCII folds, one unpinned test ... Confirmation review returned MERGEABLE at 356cddbb5, having independently re-executed every claim (the "2 -> 6 dead-classifier" figure exact, the "20 tests" trap figure exactly reproducible, the arm reorder pinned, live branch protection confirming the dated claim). Its three residuals are taken here. M1 — the backslash test hit the undecidable arm but asserted only `"ask" in reason`, which a CRASHED classifier also satisfies. Its two siblings got `"could govern it"` one commit earlier and this one did not. Under the dead-classifier mutation it was the single green test of that arm; now red. L1 — with two plain rules differing only in case, `first` picked LIST order while Gitea picks by Priority. Given `MAIN` requiring review-verdict/h10 and `main` not, the hook could inspect the rule that requires it and auto-grant on a base where Gitea enforces the other — the same defect as the arm order, one level down. Two fold-equal rules are now undecidable. L2 — the same standard, applied where I had waived it. The backslash paragraph rejects "nearly unreachable" as a standard for the arm that issues a DENY, and two paragraphs up the ASCII-only fold was accepted on exactly those grounds: rule `ünstable` and base `Ünstable` fold equal under Gitea's EqualFold and not under `ascii_downcase`, landing on `none` -> deny with a false stated cause. A non-ASCII rule name or base is now undecidable rather than fold-compared. The non-ASCII test is `explode | any(. > 127)`, not a `\uXXXX` regex: the first attempt was a character-class regex whose backslashes are ambiguous through a single-quoted shell string into jq, and a standalone probe showed it classifying plain `main` as non-ASCII. Codepoints have no escaping layer to get wrong. Both new arms are mutation-proven individually; an earlier attempt at those proofs used perl substitutions that silently matched nothing and reported green, which is why they were redone in python with an explicit assert on the target. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 20:23:32 +02:00
timothyandClaude Opus 5	356cddbb59	fix(778): pin the arm order, and correct a wrong explanation I had just written ... Two self-found defects while pre-empting the confirmation round's own questions. THE REORDER WAS UNPINNED. Swapping the classifier arms back to exact-first left all 29 tests green, so the previous commit's central change was invisible to the suite — an unproven change shipping under a green run. The missing fixture is the one that distinguishes the orders: a list holding BOTH an exactly-named rule that requires `review-verdict/h10` AND a glob rule that could also govern the base and does not. Exact-first inspects the rule that requires h10 and auto-grants; undecidable-first asks. Now mutation-proven in both directions. I HAD MISDIAGNOSED THE TRAP, and asserted the wrong cause in a comment. A three-line repro disproves "an EXIT trap suppresses output" — it does not. The real mechanism is that this file already owns its EXIT trap: `scripts/hook-fire-log.sh` installs `trap 'etv_hook_fire_end "$?"' EXIT` (#776), and in capture mode that handler is what REPLAYS the decision JSON to stdout. A second `trap ... EXIT` silently replaces it, so the decision is captured and never emitted. The trap slot is a single shared resource and the sourced library claimed it first. That second one is the shape this whole PR is about, committed against my own work: an explanation that fits the symptom, is written down as fact, and is wrong. It would have told the next person the construct is unusable rather than that the slot is taken. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 19:54:47 +02:00
timothyandClaude Opus 5	d8ff6fa229	fix(778): ask before trusting an exact rule, and make the tests tell classified from crashed ... Ninth cold review: MERGEABLE, no Blocker, no High. Its three Mediums taken anyway, because each was a one-line fix retiring the last "asserted rather than verified" surface in the file whose whole subject is that shape. M1 (backslash missing from the metacharacter class) was already closed in cd1b28637 — found independently while stress-testing the superset claim, in the window the reviewer was working against the previous head. M2 — the `exact` arm was not decidable. Gitea picks the governing rule with GetFirstMatched over a list sorted by Priority and THEN by plain-name-ness, so a glob rule with a better Priority outranks an exactly-named one. Preferring `exact` would inspect a rule Gitea might not be applying: if the exact rule requires `review-verdict/h10` and a higher-priority glob rule does not, the gate auto-grants on a base where the check is not enforced. `undecidable` is now evaluated FIRST, which makes the classifier sound without knowing the precedence rules at all — the only claim this code is entitled to make about somebody else's resolver. M3 — four tests could not distinguish "classified correctly" from "classifier crashed", because a dead classifier lands on the generic could-not-read ask and they asserted only `"ask" in reason`. Measured with the reviewer's method rather than argued: injecting `error(...)` at the head of the jq program left 2 of them red; the strengthened assertions leave 6. The glob arms now pin the text unique to the undecidable ask, and the decidable arms assert the rule was HONOURED rather than referred to a human. Also: the module docstring still described a three-arm contract after this change added a fourth; the inventory row still summarised the old two-way behaviour; the ASCII-only case fold is now stated as a deliberate under-match rather than as parity with EqualFold; the live-config claim is dated; and one jq call rejoined the file's `|| true` discipline. A `trap ... EXIT` for temp-file cleanup was tried and REVERTED: it suppressed the hook's decision output entirely and took 20 tests red. A gate that prints nothing is the one outcome it must never produce, so the tidier construct loses to the one that works, with the reason recorded where the next person will try it. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 19:47:06 +02:00
timothyandClaude Opus 5	cd1b286378	fix(778): backslash is a metacharacter too, or the none arm's proof does not hold ... Self-found while stress-testing the superset claim I introduced one commit earlier — the crux the new classifier's safety rests on. `none` authorises a DENY on the stated grounds that nothing can possibly govern this base, so its premise must hold unconditionally, not usually. The superset was "literal prefix + .* + literal suffix", which is sound for every glob dialect EXCEPT one case: gobwas/glob reads `\{` as a LITERAL brace, so a rule `a\{b` governs the base `a{b`, while a superset treating `\` as an ordinary character builds `a\.*b`, misses, and denies a base that is in fact protected. Verified before and after: with `\` outside the metacharacter class the classifier answered `none` for that pair; with it inside, `undecidable` -> ask. 18 adversarial rule/base pairs (brace alternation, negated and ranged classes, `**`, leading and trailing metacharacters, unicode, empty alternation) all answer `undecidable`, never `none`, so no dialect-matching case falls through the deny arm. Git ref rules make this nearly unreachable — a branch name may not contain `*`, `?`, `[` or `\` — but `{` IS legal in one, and "nearly unreachable" is not the standard for the arm that issues a deny. Checked with `git check-ref-format` rather than assumed. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 18:59:36 +02:00
timothyandClaude Opus 5	456042ee51	fix(778): stop claiming parity with Gitea's glob matcher — classify, or ask ... Eighth cold review: no Blocker, no High. Three Medium, two Low, one Nit. MEDIUM — the substantive one. The glob fallback asserted it matched rules "the same way Gitea applies them", and it does not. Gitea compiles a rule name with gobwas/glob and a `/` separator: its `*` does NOT cross a slash, `?`/`[…]`/`{a,b}` are wildcards, and a plain name is folded case-insensitively. Mine used `.*` for `*` and escaped the rest. The divergence has a false-OPEN direction — `release/*` does not govern `release/26/hotfix` in Gitea, but `release/.*` matched it here, which would auto-grant a scheduled merge on a base where the check is not required. That is #622's hole, reached through the block written to close it, via exactly the failure this PR records: a claim about an external system asserted rather than verified. Reimplementing somebody else's glob dialect would be a second copy of a parser, which this repo has already withdrawn a change for. So the classification is three-way and each arm is safe WITHOUT knowing the dialect: an exact non-glob name folded case-insensitively is decidable; a glob rule that could govern the base is UNDECIDABLE and asks; and "could" is tested with a provable superset of any glob dialect — literal prefix, `.*`, literal suffix — so if even that cannot match, no dialect can. Over-matching would grant on unestablished protection; under-matching would deny with a false cause. Asking is the only answer honest in both directions, and it is rare: this repo's rule is the plain name `main`. MEDIUM — a count that was wrong the moment it was written ("46 of the 69 rows are N/A"; it is 44). It was added by the same commit that demoted two rows. That is the FOURTH stale number in this change, in the deliverable whose own record argues against hand-maintained counts. Removed rather than corrected, with the reason stated. MEDIUM — `migration-smoke.sh` still said "Same shape" as `security-scan.sh`, whose note had just been rewritten to the opposite conclusion, so the backreference had silently inverted. It is the same pull-then-run over a mutable tag and deserves the grade MORE, not less: `security-scan.sh` boots a throwaway container and authorizes nothing, while this gates a production stack recreation. Regraded. LOW/NIT: an `end <= start` guard that `str.index(…, start)` makes unreachable by construction is replaced with the reachable failure it was describing; a docstring still named a fixture from an earlier draft; a reflowed `#707.` was line-initial. Two mutations were ineffective on the first attempt — one left the `decide ask` continuation in place, the other had no test covering case-folding. Both redone; each arm now reddens a named test. Decisions-Edit: yes refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 18:52:18 +02:00
timothyandClaude Opus 5	7db4101b24	fix(778): the escape emitted TWO backslashes, turning a false-open into a false DENY ... Seventh cold review. One High, one Medium, three Low, seven Nit — all in the two newest commits, which is where every round of this PR has found its defects. HIGH, and it is my own fix from the previous commit. In jq source `"\\\\"` decodes to TWO backslashes, so escaping produced `\\.` — "a literal backslash, then any character" — instead of an escaped dot. Every rule name containing a metacharacter became UNMATCHABLE, and a rule named `a[b` crashed jq outright (swallowed by `|| true`). Verified: `release/26.*` no longer matched base `release/26.4`, so the fallback found nothing and hard-DENIED with the stated cause "has NO branch protection at all" — converting a false-open into a false deny, which the block's own comment calls the worse outcome. One character: `"\\" + .c`. Correct across 14 rule/base pairs. WHY MY TEST MISSED IT, which is the transferable part: it asserted only the NEGATIVE direction (`mai.` must not match `main`). A rule matched literally and a rule made unmatchable both fail to match the wrong base, so the assertion passed for the wrong reason. Only a rule that SHOULD match separates them, and there was no positive control. There is now — plus a char-class case — and both go red against the over-escaped version. That also needed a base containing a dot: a rule cannot carry a metacharacter and still match `main`, so the first attempt at the positive control was unsatisfiable by construction. MEDIUM — four live claims that the population "derives from the filesystem", left standing by the commit that replaced that mechanism: the guard's own docstring 45 lines above a comment shouting the opposite, the inventory heading 21 lines under "Every git-tracked file", the docs/README entry, and — worst — the record's `mechanics:` frontmatter, which is the copy the catalog and MemPalace mirror, so discovery would have returned the superseded lesson. All corrected. LOW/NIT: the URL-encoding test grepped the source for `@uri` (it now asserts the URL actually requested, and reddens when the encoding is removed); the hoist comment said "every path below" without noting the docs-only enumeration above it (bounded — that path is a passthrough to a human prompt, never a grant); a now-unreachable guard is annotated rather than left reading as live; `issue-qualification-audit.sh` was `N/A` while `select-queue.sh` was `UNSAFE-KNOWN` on the same argument, and `security-scan.sh` claimed "one step" for a pull-then-run over a mutable tag — both regraded; the `PINNED` definition now says what separates its second shape from an `N/A` "one step" row (the identifier's immutability, not the step count); the section parser raises a message naming both required headings instead of a bare ValueError; and the record's body is rewrapped. Decisions-Edit: yes refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 18:21:04 +02:00
timothyandClaude Opus 5	01de6cbd8b	fix(778): * is the only wildcard — a rule name's regex metacharacters were live ... Found by attacking my own glob fallback from the previous commit before the reviewer got to it, which is the round where this PR's defects have landed every time. The 404 fallback matched a branch-protection rule by substituting `*` into a raw regex, leaving every other metacharacter active. Verified directly: a rule named `main.x` matched the base `mainax`, and `a+b` matched `aab`. The direction is the one that matters — a spurious match to some OTHER rule that happens to require `review-verdict/h10` reports this base as protected when nothing governs it, so a consent gate answers yes on evidence about a different branch. Each literal segment is now escaped before the pattern is assembled, so the wildcard survives and nothing else does. Verified across 11 rule/base pairs: metacharacters are literal, `*` still spans, exact and non-matches unaffected. The regression test needed two goes to stop being vacuous, both times for the same reason the rest of this PR keeps hitting: the stub never 404'd for the new mode, so the run denied earlier via the by-name lookup and never reached the fallback at all. It now goes red against the unescaped predecessor. refs #778 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-19 18:01:42 +02:00