e4a8b6ffb4bd702e7cbbdb472a7665bf9c71b225
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
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.
|
||
|
|
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 |
||
|
|
5d955000f3 |
fix(845): the verdict writer checks that the gate will honour what it just posted (#889)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 10s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 31s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 15m44s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 10m51s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 8m27s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 1m46s
`review-verdict.yml` inherits an existing `review-verdict/h10=success` only from a status whose `.creator.login` is on its `H10_REVIEWERS` allow-list (#742). `post-review-verdict.sh` wrote those verdicts with whatever account owned the credential in the environment and never asked whose it was. Two coupled values, nothing asserting the coupling, and the failure was the silent kind: the status is written, the tool reports success, and the next `pull_request_target` event re-derives it and posts over it. The PR stalls with no visible cause. The writer now READS ITS OWN STATUS BACK, identifies that write by state and description, and refuses — before the verdict comment, so the surviving half-state is the documented `ask` one — unless the recorded creator is allow-listed. Measured after the write rather than probed before it: that tests what Gitea recorded as the author, which is the value the gate reads, and needs no scope beyond the repo access the POST already required. Membership is required for a `success` ONLY, mirroring the gate's own asymmetry: a `failure` is inherited from any attributable account, so requiring it there would refuse a verdict the gate honours and leave an off-list reviewer no supported way to record a rejection. The allow-list is DERIVED from the gate's own literal by the new `scripts/lib/h10-reviewers.sh` — one declaration, not two plus a parity test. It is a parse rather than a shared declaration both sides source because the gate runs against a checkout of the PR's BASE sha: a PR whose base predates such a file would not have it, and a missing `source` under `set -euo pipefail` kills the job, which posts no `review-verdict/h10` at all and blocks every merge including its own repair (#743). `scripts/post-review-verdict.sh` moves BEHAVIOUR-ONLY -> MUTATION in the guard inventory, which the manifest's own note called "the most valuable upgrade on this list". The declared clause lives in the GATE: rewriting `H10_REVIEWERS` while the posting account stays fixed reddens the accept path only if the writer reads the list live AND the comparison gates the outcome. Two defects were caught by probing the live instance rather than re-reading the code. Reading `.state` instead of `.status` per row would have refused EVERY verdict — a repo-wide deadlock, shipped green, because the test shim replayed the POST payload as the read-back body and so agreed with the parser by construction. Then a `(.status // .state)` fallback added as defensiveness recreated #845 exactly: the writer would accept a shape the gate cannot read and report success. Nine independent cold review rounds, all worktree-isolated, one cross-family (GPT-5.6 via Codex). Round 8 caught the most important one: a `set -u` "correction" made mid-branch had inverted a TRUE statement in live merge-gate code, because the probe used a plain `$UNSET` while the validator uses `${#arr[@]}` — different shapes, different behaviour. Withdrawn wholesale; both libraries are byte-identical to `main` again. Verification: full `scripts/tests` suite green (1278 passed, 2 skipped); the declared mutation executes every run and reddens its named proof with the manifest's `expect` string; every clause disarmed individually and confirmed to redden its own named test; live probes against Gitea 1.27.1 for the row shape, the description round-trip, the paging order and the required-check list. Docs: `ci.exemption-provenance` records the coupling as asserted rather than as a tracked residual, plus `docs/ci-cd.md`, `CLAUDE.md`, `docs/guard-inventory.md`, `docs/remote-state-inventory.md`, `ci.script-tests-job` and the `script-tests` population comment in `pr-checks.yml`. Deferred: the refused-verdict residual (a non-inheritable status left standing with no comment) is the `ask` half-state `release.verdict-writes-status-before-comment` designates as safe; a second corrective write is the sticky-sentinel mechanism #849 is separately designing. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
761e575836 |
fix(787): derive the dropped-step guard's scope, and reconcile its snapshot against the server (#861)
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 21s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m45s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m22s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m56s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m33s
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be> |
||
|
|
469d19852c |
fix(788): one declarative H10 verdict vocabulary, derived by both sides (#846)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m34s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m50s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m22s
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> |