4 Commits

Include renames

Author	SHA1	Message	Date
timothyandClaude Opus 5	e3a19227c1	fix(845): two independent cold reviews — the guard did not walk the same tree, and CLAUDE.md said the status is not written ... Two cold reviewers, independently briefed and worktree-isolated. One returned BLOCKED on the first two below; the other returned MERGEABLE having not examined them. No arbitration was needed — both are verifiable, and I reproduced each before acting. BLOCKING 1 — the CI guard parsed a different population than the library. `test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty` read `_classify_step()["run"]`, one step's body. `scripts/lib/h10-reviewers.sh` greps the WHOLE YAML and refuses on anything but exactly one match. So a second anchored `H10_REVIEWERS=` in ANY other `run:` block was invisible to the guard while making the writer refuse every verdict — and `main` is PR-only with `review-verdict/h10` required, so that is a repo-wide merge outage. Reproduced: injecting one into the last step left the guard green and reddened 31 writer tests whose diagnostics said nothing about a second assignment. The guard now reads the whole file, so it walks the same tree as its subject; the header and the decision record no longer claim coverage that did not exist. BLOCKING 2 — `CLAUDE.md` said an off-list verdict "fails loudly at your terminal instead of being written". The status IS written; only the comment is withheld. The script, `ci-cd.md` and the record all said so correctly, and the most-read doc contradicted them. It now also says what the other docs did not: the green left standing satisfies the server-side required check, because branch protection binds the context NAME and not its issuer. Four clauses were shipped UNWITNESSED, found by the reviewer's own disarm matrix and each now reddening a named test: the `.statuses` array-TYPE test, the read-back GET's explicit `|| die`, `?limit=100`, and the `set -f` restoration. The first two passed only because a downstream check produced the same non-zero exit — the shape this very file already documents at the `-z`/`-n` arms. They now assert their own diagnostic. `?limit=100` was unobservable because the shim discarded the query string; the shim now models the measured server (default 30, clamped to 50, `total_count` reporting the page). Also fixed, all reproduced first: - `[0]` over same-context rows had no uniqueness guard. Unreachable today, but the alternative is judging one row's author while another is the one standing. - The row count in the not-found refusal came out BLANK on a 200 with an empty body: jq exits 0 with empty output, so the `||` fallback was dead exactly where the count matters. Tests the value now, not jq's exit. - The split pinned globbing but not IFS. A caller with `IFS=-` made `renovate` read as a member of `renovate-bot`. Unreachable from this script (bash resets IFS), pinned because the header invites reuse. - The `set -u` rationale was inherited and is WRONG. Both reviewers measured it: the shell EXITS, in both the `if ! f` and the `f; rc=$?` shapes, on bash 5.3 and 3.2. Nothing here depended on it; the sentinel now rests on the two grounds that were measured. - The `?limit=100` margin was overstated. This instance clamps to MAX_RESPONSE_ITEMS = 50, so the headroom is ~3x over today's 8-15 contexts, not 6x, and it shrinks as checks are added. - The snapshot boundary now names its DIRECTION in all three places: it is a false ACCEPT, and the likely edit (ADDING a reviewer) is exactly the one it mis-handles. - The overwrite refusal told the operator to re-review when a gate re-derive needs no review. A note on the measurements, since two of my own disarms were inert and briefly read as "no test covers this": a mutation must be checked to actually mutate. The glob-class disarm still excluded `*`, and the IFS disarm matched two sites and applied to neither. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW	2026-08-30 03:38:47 +02:00
timothyandClaude Opus 5	afe61e80aa	fix(845): the verdict writer checks that the gate will honour what it just posted ... `.gitea/workflows/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). `scripts/post-review-verdict.sh` writes those verdicts with whatever account owns `ETV_GITEA_TOKEN`/`ETV_GITEA_BASICAUTH` and never asked whose it was. The two values were coupled with nothing asserting the coupling, and the failure mode was the silent one: the status really is written, the tool really does report success, and then the next `pull_request_target` event re-derives it and posts over it. And again. The PR deadlocks and the only diagnostic is a `::warning::` inside a workflow run nobody is reading. The script now READS THE STATUS BACK after posting it, identifies its own write by state and description, and refuses — before the comment, so the surviving half-state is the documented `ask` one — unless the recorded creator is on the allow-list. Measured after the write rather than probed before it: a pre-flight `GET /user` tests what the credential claims to be, this tests what Gitea recorded as the author of this status, which is the value the gate reads. It also needs no scope beyond the repo access the POST already required. The allow-list is not restated. `scripts/lib/h10-reviewers.sh` derives it from the workflow's own literal, so there is ONE declaration rather than two held together by a parity test — the shape `testing.guard-derives-population-from-source` records six failed rounds against. It is a parse rather than a shared declaration both sides source, and that is not a preference: the gate runs against a checkout of the PR's BASE sha, so a PR whose base predates a new `scripts/lib/` 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 to `main` including the PR that would repair it, with no admin override (#743). A fallback list for that case reintroduces the second copy. So the derivation runs on the side that can fail safely and the gate's body is untouched. Fail-closed on every branch of the derivation: 0 or >1 anchored assignments, an entry that is not a plain login, an empty list, or an unreadable file all refuse BEFORE any network call. The membership query answers member / not-a-member / cannot-tell as three distinct codes rather than two, so a caller cannot fold "cannot tell" into either. The safety DIRECTION is unchanged and the commit does not claim otherwise: a non-allow-listed verdict was always a stall, never a fail-open. What changes is where the signal lands — at the terminal that made the mistake, instead of one PR event later. Residual, stated rather than fixed: the check runs after the POST, so a refused verdict leaves a non-inheritable status standing with no comment beside it. That is the `ask` half-state `release.verdict-writes-status-before-comment` already designates as the safe one, and the gate re-derives it on the next PR event. A second corrective write is the sticky-sentinel mechanism #849 is separately designing; inventing a parallel one here would be two mechanisms for one invariant. `scripts/post-review-verdict.sh` moves BEHAVIOUR-ONLY -> MUTATION in the guard inventory, which the manifest's own UNDECLARED note called "the most valuable upgrade on this list". The declared clause is in the GATE, not the script: 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. Disarming the comparison instead would prove the second and say nothing about the first, which is the half that was missing. Verification: full `scripts/tests` suite green (1239 passed, 2 skipped); the declared mutation executes and reddens its named proof with the declared diagnostic; four clauses disarmed by hand and each confirmed to redden its own test. fixes #845 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW	2026-08-30 03:38:47 +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	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