fix(858,859): a rule the classifier cannot read is not a rule that matches nothing #897

Merged

timothy merged 3 commits from fix/858-859-merge-consent-hook-residuals into main 2026-08-30 13:33:51 +02:00

Conversation 1 Commits 3 Files Changed 10

+899 -65

3 Commits

Author	SHA1	Message	Date
timothy	e4a8b6ffb4	fix(858,859): review round 4 — the corrected sentence had a twin, and its twin had two more ... Round 4 returned NOT-MERGEABLE with Blocker and High both EMPTY: one Medium and two Lows, all prose, no behavioural defect found across four attack directions. MEDIUM — round 3 corrected "deleting it left all 57 tests green" in the hook and left the IDENTICAL sentence, with the identical wrong number, in the test file — a line this branch had added. Measured: 64, not 57. A comment fix applied to one of two sites, inside the PR whose subject is arms that state what did not happen. Fix-one-path-then-check-its-twin, one level up from the code. Sweeping the CLASS rather than the instance then found two more, both pre-existing and both made MORE wrong by this branch's new tests ("all 71 tests", "all 29 tests" — now 111 and 64). Fixed here because this PR falsified them; all three now state the invariant ("the WHOLE suite") with a parenthetical saying why the count was dropped. A fourth lives in test_ci_dropped_step_guard.py, which this PR neither touches nor falsifies — left alone and filed as ersatztv#895 with a candidate detector, since four instances of a mechanically-detectable class is the criterion for a check rather than another sweep. LOW — "THE PAYLOAD MUST BE AN ARRAY before the classifier sees it" is false on jq 1.6, the version scripts/jq-preflight.sh pins for script-tests: `jq -e` over EMPTY input exits 0 there, so an empty 200 body is not diverted and does reach the classifier. Measured on 1.6 and 1.8.2 — the class word is `unreadable` either way, so the OUTCOME is version-independent and the comment now states the outcome rather than the mechanism (ci.jq-version-contract, ersatztv#643). LOW — scripts/hook-fire-log.sh's own USAGE recipe still showed the ${CLAUDE_PROJECT_DIR:-...} form that process.hook-resolves-inputs-from-repo-root bans. That header is what a new hook copies, so leaving it would keep minting the defect after #891 sweeps the existing twelve. Corrected, with the reason and the measurement beside it. Suites: 193 passed, 2 skipped.	2026-08-30 12:38:20 +02:00
timothy	0281acc3b7	fix(858,859): review round 3 — a comment that contradicted its own commit, and a reason that named the wrong cause ... Cold re-review returned NOT-MERGEABLE with 6 findings, 3 of them introduced by the previous fix round. All measured, all fixed. H1 — the `verdict_script` block still said `ETV_HOOK_FIRE_LIB` 'still consults $CLAUDE_PROJECT_DIR ... byte-identical across all thirteen tracked hooks', in the commit that changed that very line. Three false clauses at once, and the third re-asserted the telemetry exemption the decision record documents as refuted. Exactly the second, weaker answer this file's own thesis warns about. M1 — the `unnamed-rule` reason said 'neither branch_name nor rule_name holds a non-empty string'. False for the poison case round 2 added: `{"branch_name":42,"rule_name":"main"}` has a perfectly good `rule_name`. A reason that names a cause which did not happen, inside the fix for reasons that name causes which did not happen. The message now covers both shapes, and the matching 'in both' wording is corrected in the hook and the checker. L2 — poison was order-dependent: it short-circuited on `branch_name`, so a malformed `rule_name` beside a good `branch_name` answered `exact`. The comment claimed the invariant field-agnostically. Both fields are now tested before either name is taken. Gitea 1.27.1 does not send that shape, so this pins the stated property rather than a live bug. L4 — `check-required-contexts.sh` had no array-type gate, so a JSON object reached the classifier and printed `nomatch` — 'protection removed', a positive claim about the server from a body that is not the shape it consumes. Pre-existing; the hook has had that gate since #778. Fixed here with a negative control pinning that `[]` stays a real finding. L1 — a `'"'"'` shell-quote escape had leaked from a heredoc into the jq source. L3 — 'deleting it left all 57 tests green' now reads 57 of 64; replaced with the invariant. 8 declared mutants still detected. Suites: 111 passed.	2026-08-30 11:54:17 +02:00
timothy	43a2d64703	fix(858,859): a rule the classifier cannot read is not a rule that matches nothing ... Three residuals from #787's cold review, in the merge-consent gate. What #859 described as a wrong STATED CAUSE turned out to be masking a live false-open. #859 (1) — `(.branch_name // .rule_name // "")` collapsed an unreadable rule to the empty string: a VALID name that matches nothing. That fed `none`, whose entire authority is "the full rule list was read and none matches", and on the hook's caller `none` DENIES. A rule with no usable name is now a distinct `unreadable` verdict, and the short-circuit is STRUCTURAL rather than first-in-the-chain: jq binds `as` eagerly, so the flat version still evaluated `offs`/`nonascii` on the bad name and died before reaching the arm written to prevent that. THE FALSE-OPEN, which the issue did not know about. jq's `//` fires on null and false but NOT on `""`. Measured on a scratch repo against Gitea 1.27.1: a GLOB rule comes back as `{"branch_name":"","rule_name":"release/*"}` — the canonical name is ONLY in `rule_name`. So every glob rule reached the classifier as a name with no metacharacters and the glob test never saw it. Measured on the predecessor: glob `m*` (not requiring `review-verdict/h10`) beside plain `main` (requiring it) resolved to `exact` on `main` and AUTO-GRANTED a scheduled merge, while Gitea — ordering by Priority then plain-name-ness — may be applying `m*`. That is #622's hole, reached through the undecidable-first ordering written to close it. The mirror case: a glob alone resolved to `none` and DENIED about a rule that provably governs the base. An earlier draft of this message called that shape unreachable, on a probe of THIS repo, which has exactly one plain rule. An arbitrary sample generalised into a claim about all rule shapes; the positive case had to be constructed. The fall-through is typed, not permissive. Cold review caught the first draft treating a PRESENT-but-malformed `branch_name` as absent: `{"branch_name":42,"rule_name":"main"}` answered `exact` and auto-granted where the predecessor threw and asked. A field now either supplies a name, SKIPs (absent/null/empty), or POISONs the list. #859 (2) — `branch_protections` was fetched twice on a scheduled merge. The round trip is the smaller half: it is MUTABLE config, so two reads can disagree and the two arms then decide about different repo states with neither able to notice. One cached read per run makes a run internally consistent by construction; the arms still classify independently, so the bytes are shared and never a verdict. #858 — `verdict_script` resolved from `$CLAUDE_PROJECT_DIR`, an env var selecting the code that decides whether a merge is granted, while #787's arm below bound the same class of input to `$repo_root`. Two answers to one question in one file is the state most likely to be tidied toward the weaker one. AND THE ONE THAT MATTERED MORE. The first draft exempted `ETV_HOOK_FIRE_LIB` as "telemetry, not a predicate". Cold review refuted that by execution: the line is `. `-SOURCED, so it is CODE, running before stdin is read and before `decide` exists — a decoy `hook-fire-log.sh` in an env-var-named tree printing an `allow` and exiting 0 GRANTS THE MERGE, having bypassed every check. Classify a path by how it is CONSUMED, never by what it is called. This hook's copy is self-located now; the other twelve are #891, which also records the reachable case (husky launches the prepush hooks by RELATIVE path, so there the two roots genuinely diverge — measured). Every fix ships a test witnessed RED against the real predecessor or a declared clause mutation — 8 mutants, 8 detected, disjoint reddened sets. The env-var pairs each carry a NEGATIVE CONTROL, because the passing outcome is also what an inert decoy produces: the same decoy is re-run as genuine `$repo_root` and must flip the decision. Docs: new record `process.hook-resolves-inputs-from-repo-root` (both inline sites cite it rather than arguing it twice), regenerated catalog, and the `remote-state-inventory.md` row that described the second read. fixes #858 fixes #859	2026-08-30 11:54:17 +02:00