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

timothy commented 2026-08-30 12:58:52 +02:00

Owner

Copy Link

Copy Source

Closes the two #787 cold-review residuals in the merge-consent gate. #859 was filed as a wrong stated cause; it turned out to be masking a live false-open in the gate.

fixes #858
fixes #859

The false-open (#859, and not what the issue described)

Gitea reports a glob branch-protection rule with an empty branch_name — the canonical name lives only in rule_name. Measured 2026-08-30 on a scratch repo against 1.27.1, creating one rule of each kind:

{"branch_name":"",     "rule_name":"release/*"}   <- GLOB rule
{"branch_name":"main", "rule_name":"main"}        <- plain rule

jq's // fires on null and false but not on "". So (.branch_name // .rule_name // "") resolved every glob rule to the empty string — a name with no metacharacters — and the glob test, which is the entire basis of the classifier's undecidable-first ordering, never saw it. Measured on the predecessor:

payload (base main)	old	new
glob m* (no h10) + plain main (h10)	exact → AUTO-GRANT	undecidable → ask
glob m* alone	none → DENY "none matches"	undecidable → ask

The first row is #622's hole reached through the ordering written to close it: the gate reads the plain rule's contexts and arms a scheduled merge, while Gitea — ordering by Priority then plain-name-ness — may be applying m*, on which review-verdict/h10 is not required.

An earlier draft of this PR called that shape unreachable, having probed 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.

What changed

• scripts/lib/branch-rule-classifier.jq — a name is a non-empty string. Each field resolves to a NAME, a SKIP (absent/null/empty — fall through), or POISON (present, wrong type — poison the list, whichever field carries it). A rule with no usable name is a new unreadable verdict instead of feeding none, whose whole authority is "the full rule list was read and none matches". The short-circuit is structural, not merely first in the chain: jq binds as eagerly, so the flat form still evaluated offs/nonascii on the bad name and died before reaching the arm meant to prevent that.
• .claude/hooks/pretooluse-merge-consent.sh — an unnamed-rule sentinel with its own reason (a first draft reused the catch-all's sentinel and was measured to be a no-op: deleting the arm left the whole suite green). branch_protections is fetched once per run instead of twice. verdict_script resolves from $repo_root.
• ETV_HOOK_FIRE_LIB is self-located — the finding that mattered most. A first draft exempted it 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, and a decoy hook-fire-log.sh in an env-var-named tree grants the merge outright. Classify a path by how it is CONSUMED, never by what it is called. This hook's copy is fixed; the other 12 are #891 (priority: high + security), which records the reachable case — husky launches the prepush hooks by relative path, so there the two roots genuinely diverge.
• scripts/check-required-contexts.sh — an array-type gate (a JSON object previously printed nomatch, a positive claim about server config from a body that is not the shape it consumes). Its no-op unreadable) arm was deleted rather than kept as decoration.
• Docs — new record process.hook-resolves-inputs-from-repo-root; both inline sites cite it instead of arguing it twice. docs/remote-state-inventory.md's row for the second read updated.

Verification

• Full scripts/tests: 1377 passed, 2 skipped on the rebased tree.
• 11 declared mutants, 11 detected, disjoint reddened sets — including reverting each fix to its real predecessor.
• Both env-var tests ship a negative control: the passing outcome is also what an inert decoy produces, so the same decoy is re-run as genuine $repo_root and must flip the decision.
• Live Gitea probed for the glob payload shape on a scratch repo, which was deleted afterwards.
• Classifier executed on a 27-payload matrix across jq 1.8.2 and jq 1.6 (the CI floor) with byte-identical results.

Review history — worth reading before approving

Four rounds. Every round found defects the previous round's fixes introduced, which is why the count is high rather than a sign the change is thrashing:

• R1 (Codex): the telemetry exemption is false — a sourced path is code. Also: my "not known to be reachable" claim was wrong.
• R2 (Claude, worktree-isolated): the round-1 fix treated a present-but-malformed branch_name as absent, so {"branch_name":42,"rule_name":"main"} answered exact and auto-granted where the predecessor threw and asked.
• R3 (Claude, worktree-isolated): the round-2 fix left a comment asserting the opposite of the line its own commit changed; an operator reason naming a cause that did not happen; and an invariant stated field-agnostically that held for one field only.
• R4 (Claude, worktree-isolated): Blocker and High empty. One Medium — round 3 corrected a stale test count in the hook and left the identical sentence with the identical wrong number in the test file, on a line this branch added. Sweeping the class found two more (pre-existing, and made worse by this branch's own new tests); a fourth, in an unrelated file, is filed as #895 with a candidate detector rather than swept here.
• R5 (bounded, prose delta only): CLEAN.

Deferred

• #891 — the other 12 hooks' sourced fire-log path. Deliberately not swept here: byte-identical copies are what make them safe to reason about, and diverging one is the defect extracting the shared classifier was meant to end.
• #876 — three of its known sites are comments in this file; its line numbers will be stale after this merges, and it should re-derive them by grep.

Closes the two `#787` cold-review residuals in the merge-consent gate. **#859 was filed as a wrong *stated cause*; it turned out to be masking a live false-open in the gate.** fixes #858 fixes #859 ## The false-open (#859, and not what the issue described) Gitea reports a **glob** branch-protection rule with an **empty `branch_name`** — the canonical name lives only in `rule_name`. Measured 2026-08-30 on a scratch repo against 1.27.1, creating one rule of each kind: ``` {"branch_name":"", "rule_name":"release/*"} <- GLOB rule {"branch_name":"main", "rule_name":"main"} <- plain rule ``` jq's `//` fires on null and false but **not** on `""`. So `(.branch_name // .rule_name // "")` resolved every glob rule to the empty string — a name with no metacharacters — and the glob test, which is the entire basis of the classifier's undecidable-first ordering, never saw it. Measured on the predecessor: | payload (base `main`) | old | new | |---|---|---| | glob `m*` (no h10) + plain `main` (h10) | **`exact`** → AUTO-GRANT | `undecidable` → ask | | glob `m*` alone | `none` → DENY "none matches" | `undecidable` → ask | The first row is #622's hole reached through the ordering written to close it: the gate reads the plain rule's contexts and arms a scheduled merge, while Gitea — ordering by Priority then plain-name-ness — may be applying `m*`, on which `review-verdict/h10` is not required. An earlier draft of this PR called that shape unreachable, having probed *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. ## What changed - **`scripts/lib/branch-rule-classifier.jq`** — a name is a non-empty string. Each field resolves to a NAME, a SKIP (absent/null/empty — fall through), or POISON (present, wrong type — poison the list, whichever field carries it). A rule with no usable name is a new `unreadable` verdict instead of feeding `none`, whose whole authority is "the full rule list was read and none matches". The short-circuit is **structural**, not merely first in the chain: jq binds `as` eagerly, so the flat form still evaluated `offs`/`nonascii` on the bad name and died before reaching the arm meant to prevent that. - **`.claude/hooks/pretooluse-merge-consent.sh`** — an `unnamed-rule` sentinel with its own reason (a first draft reused the catch-all's sentinel and was *measured* to be a no-op: deleting the arm left the whole suite green). `branch_protections` is fetched **once per run** instead of twice. `verdict_script` resolves from `$repo_root`. - **`ETV_HOOK_FIRE_LIB` is self-located** — the finding that mattered most. A first draft exempted it 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, and a decoy `hook-fire-log.sh` in an env-var-named tree **grants the merge outright**. Classify a path by how it is CONSUMED, never by what it is called. This hook's copy is fixed; the other 12 are **#891** (`priority: high` + `security`), which records the reachable case — husky launches the prepush hooks by *relative* path, so there the two roots genuinely diverge. - **`scripts/check-required-contexts.sh`** — an array-type gate (a JSON object previously printed `nomatch`, a positive claim about server config from a body that is not the shape it consumes). Its no-op `unreadable)` arm was deleted rather than kept as decoration. - **Docs** — new record `process.hook-resolves-inputs-from-repo-root`; both inline sites cite it instead of arguing it twice. `docs/remote-state-inventory.md`'s row for the second read updated. ## Verification - Full `scripts/tests`: **1377 passed, 2 skipped** on the rebased tree. - **11 declared mutants, 11 detected**, disjoint reddened sets — including reverting each fix to its real predecessor. - Both env-var tests ship a **negative control**: the passing outcome is also what an inert decoy produces, so the same decoy is re-run as genuine `$repo_root` and must flip the decision. - Live Gitea probed for the glob payload shape on a scratch repo, which was deleted afterwards. - Classifier executed on a 27-payload matrix across **jq 1.8.2 and jq 1.6** (the CI floor) with byte-identical results. ## Review history — worth reading before approving Four rounds. **Every round found defects the previous round's fixes introduced**, which is why the count is high rather than a sign the change is thrashing: - **R1** (Codex): the telemetry exemption is false — a sourced path is code. Also: my "not known to be reachable" claim was wrong. - **R2** (Claude, worktree-isolated): the round-1 fix treated a *present-but-malformed* `branch_name` as absent, so `{"branch_name":42,"rule_name":"main"}` answered `exact` and auto-granted where the predecessor threw and asked. - **R3** (Claude, worktree-isolated): the round-2 fix left a comment asserting the opposite of the line its own commit changed; an operator reason naming a cause that did not happen; and an invariant stated field-agnostically that held for one field only. - **R4** (Claude, worktree-isolated): **Blocker and High empty.** One Medium — round 3 corrected a stale test count in the hook and left the *identical sentence with the identical wrong number* in the test file, on a line this branch added. Sweeping the class found two more (pre-existing, and made worse by this branch's own new tests); a fourth, in an unrelated file, is filed as **#895** with a candidate detector rather than swept here. - **R5** (bounded, prose delta only): **CLEAN.** ## Deferred - **#891** — the other 12 hooks' sourced fire-log path. Deliberately not swept here: byte-identical copies are what make them safe to reason about, and diverging one is the defect extracting the shared classifier was meant to end. - **#876** — three of its known sites are comments in this file; its line numbers will be stale after this merges, and it should re-derive them by grep.

timothy added 3 commits 2026-08-30 12:58:53 +02:00

fix(858,859): a rule the classifier cannot read is not a rule that matches nothing ... 43a2d64703

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

fix(858,859): review round 3 — a comment that contradicted its own commit, and a reason that named the wrong cause ... 0281acc3b7

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.

fix(858,859): review round 4 — the corrected sentence had a twin, and its twin had two more ... e4a8b6ffb4

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.

timothy commented 2026-08-30 12:59:33 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ e4a8b6f

Four cold rounds (1 Codex cross-family, 3 Claude worktree-isolated) plus a bounded prose check. R4: Blocker+High empty, one Medium (a corrected sentence whose twin survived in the same diff) and two Lows — all fixed. R5: CLEAN. Local gate: 1377 passed/2 skipped; 11 declared mutants, 11 detected, disjoint reddened sets; ruff clean; catalog in sync. Every round found defects the previous round's fixes introduced, so each fix ships a guard that reddens on revert.

Review-verdict: MERGEABLE @ e4a8b6f Four cold rounds (1 Codex cross-family, 3 Claude worktree-isolated) plus a bounded prose check. R4: Blocker+High empty, one Medium (a corrected sentence whose twin survived in the same diff) and two Lows — all fixed. R5: CLEAN. Local gate: 1377 passed/2 skipped; 11 declared mutants, 11 detected, disjoint reddened sets; ruff clean; catalog in sync. Every round found defects the previous round's fixes introduced, so each fix ships a guard that reddens on revert.

timothy merged commit cf5f42edf9 into main 2026-08-30 13:33:51 +02:00

timothy deleted branch fix/858-859-merge-consent-hook-residuals 2026-08-30 13:33:55 +02:00

timothy referenced this pull request 2026-08-30 13:34:46 +02:00

Merge-gate branch-protection read: a malformed rule states a cause that did not happen, and the endpoint is now fetched twice #859

timothy referenced this pull request 2026-08-30 13:35:14 +02:00

`pretooluse-merge-consent.sh` resolves its verdict classifier from `$CLAUDE_PROJECT_DIR` — an env var as a security input #858

timothy referenced this pull request 2026-08-30 16:56:57 +02:00

docs(release): record the v26.15.0 release notes #898

timothy referenced this pull request 2026-09-03 18:58:02 +02:00

`docs.no-session-narrative` reaches hooks and code comments, but nothing has ever swept them — 4 known sites #876

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#897