Merge pull request 'chore(process): #303 H10 — review-verdict merge-gate (latest commit must be reviewed)' (#306) from ci/303-h10-review-verdict-gate into main
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / decisions.md append-only (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m33s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 10m54s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m16s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / decisions.md append-only (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m33s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 10m54s
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 5m16s
This commit was merged in pull request #306.
This commit is contained in:
@@ -1,11 +1,16 @@
|
||||
#!/usr/bin/env bash
|
||||
# PreToolUse / mcp__gitea__pull_request_write — derive merge consent from STATE instead of
|
||||
# trusting the agent's judgment (ersatztv#303 H6). A PR merge is the one irreversible op; allow it
|
||||
# only when BOTH are true:
|
||||
# trusting the agent's judgment (ersatztv#303 H6 + H10). A PR merge is the one irreversible op; allow it
|
||||
# only when ALL are true:
|
||||
# (a) the PR's CI combined status is green, AND
|
||||
# (b) every checkbox in the linked issue's "## Done-when" section is ticked.
|
||||
# (b) every checkbox in the linked issue's "## Done-when" section is ticked, AND
|
||||
# (c) a review-verdict comment on the PR references the CURRENT head sha (H10) — proving the
|
||||
# LATEST commit was reviewed, not a stale earlier diff (the ersatztv#242 failure mode:
|
||||
# "re-review the fix commit, not just the initial PR diff").
|
||||
# The "## Done-when" issue-body checklist is the convention (docs/decisions.md, CLAUDE.md Task
|
||||
# Completion Protocol). One box is "adversarial review passed"; the others are per-issue.
|
||||
# The H10 review-verdict convention: after reviewing a PR (or its latest fix commit), post a PR
|
||||
# comment carrying a line `Review-verdict: <MERGEABLE|APPROVED|BLOCKED|NOT-MERGEABLE> @ <head-sha>`.
|
||||
#
|
||||
# Decision policy — a CONSENT gate, so it does NOT fail silently open:
|
||||
# - state derivable and NOT satisfied -> deny (actionable reason)
|
||||
@@ -97,5 +102,62 @@ if [ "$mwcs" != "true" ]; then
|
||||
esac
|
||||
fi
|
||||
|
||||
# Both derivable and satisfied -> allow.
|
||||
# --- (c) Review-verdict freshness (ersatztv#303 H10): a review-verdict comment must reference the
|
||||
# CURRENT head sha, so the latest commit is proven-reviewed (ersatztv#242: re-review the fix
|
||||
# commit, not just the initial diff). Graceful adoption mirrors (b): a verdict comment that
|
||||
# references head must be positive -> allow; one that exists only for an OLDER commit -> deny
|
||||
# (the stale-review failure mode); NO verdict comment at all -> ask (convention not yet used).
|
||||
[ -n "$sha" ] || decide ask "H10 merge gate: could not resolve PR #$pr head sha to verify a review verdict. Confirm the review covered the latest commit before merging."
|
||||
short=${sha:0:7}
|
||||
comments=$(gq "repos/$owner/$repo/issues/$pr/comments?limit=100")
|
||||
if [ -z "$comments" ]; then
|
||||
decide ask "H10 merge gate: could not fetch PR #$pr comments to verify a head-referencing review verdict ($short). Confirm the adversarial/Codex review covered the latest commit before merging."
|
||||
fi
|
||||
# Verdict lines across all comment bodies: a real verdict line STARTS with the marker (after optional
|
||||
# leading whitespace). Anchoring to line-start is deliberate — it rejects a comment that merely QUOTES
|
||||
# the positive template mid-sentence (an instruction "please post: Review-verdict: MERGEABLE @ <sha>",
|
||||
# or the gate's own suggestion text echoed back), which would otherwise self-approve the merge.
|
||||
verdicts=$(printf '%s' "$comments" | jq -r '.[].body // empty' 2>/dev/null | grep -iE '^[[:space:]]*review-verdict:' || true)
|
||||
if [ -z "$verdicts" ]; then
|
||||
decide ask "H10 merge gate: no 'Review-verdict:' comment found on PR #$pr referencing head $short. Post the adversarial/Codex verdict (e.g. 'Review-verdict: MERGEABLE @ $short'), or confirm the review covered the latest commit and approve."
|
||||
fi
|
||||
# Classify each verdict line by the sha it references (its "@ <sha>" field) and its verdict word.
|
||||
# A line references the CURRENT head iff head BEGINS WITH that sha token AND the token is >=7 chars
|
||||
# (git short-sha prefix semantics) — NOT a loose substring test: an older sha that merely contains
|
||||
# the head prefix, or the head prefix appearing in an unrelated URL on the line, must NOT count
|
||||
# (adversarial false-opens). The verdict token must sit right after the marker on the same line.
|
||||
head_pos=0; head_neg=0; stale=0
|
||||
while IFS= read -r line; do
|
||||
[ -n "$line" ] || continue
|
||||
# The sha the line references: the hex token in its "@ <sha>" field (>=7 chars), lowercased.
|
||||
ref=$(printf '%s' "$line" | grep -ioE '@[[:space:]]*[0-9a-f]{7,40}' | head -1 \
|
||||
| grep -oiE '[0-9a-f]{7,40}' | tr 'A-F' 'a-f' || true)
|
||||
is_pos=0
|
||||
# Positive iff the line's OWN leading verdict word (right after the line-start marker) is positive —
|
||||
# anchored so a second, later `review-verdict: mergeable` substring on a BLOCKED line can't flip it.
|
||||
if printf '%s' "$line" | grep -iqE '^[[:space:]]*review-verdict:[[:space:]]*(mergeable|approved|lgtm)'; then is_pos=1; fi
|
||||
[ -z "$ref" ] && continue # marker present but no @<sha> -> falls through to the final ask
|
||||
case "$sha" in
|
||||
"$ref"*) if [ "$is_pos" = 1 ]; then head_pos=1; else head_neg=1; fi ;;
|
||||
*) stale=1 ;;
|
||||
esac
|
||||
done <<VERDICTS
|
||||
$verdicts
|
||||
VERDICTS
|
||||
|
||||
# A negative verdict on head wins over a positive one (a later BLOCKED retracts an earlier MERGEABLE
|
||||
# on the SAME head; and if the head were fixed the sha would change, so this can't wrongly block).
|
||||
if [ "$head_neg" = 1 ]; then
|
||||
decide deny "H10 merge gate: BLOCKED — a review verdict for the current head ($short) is negative (BLOCKED/NOT-MERGEABLE). Resolve the findings and post a fresh 'Review-verdict: MERGEABLE @ $short' before merging PR #$pr."
|
||||
fi
|
||||
if [ "$head_pos" = 1 ]; then
|
||||
decide allow "" # a positive verdict references the current head -> (c) satisfied
|
||||
fi
|
||||
if [ "$stale" = 1 ]; then
|
||||
decide deny "H10 merge gate: BLOCKED — a review-verdict comment references an older commit, not the current head ($short). The latest commit(s) are unreviewed (ersatztv#242: re-review the fix commit, not just the initial diff). Re-review the head and post 'Review-verdict: MERGEABLE @ $short'."
|
||||
fi
|
||||
# Marker(s) exist but reference no sha at all -> ask (don't mislabel as a stale older-commit review).
|
||||
decide ask "H10 merge gate: a 'Review-verdict:' comment on PR #$pr references no commit sha. Post one referencing the current head ($short) — e.g. 'Review-verdict: MERGEABLE @ $short' — or confirm the review covered the latest commit and approve."
|
||||
|
||||
# All derivable and satisfied -> allow.
|
||||
decide allow ""
|
||||
|
||||
@@ -82,8 +82,8 @@ docker build -f docker/Dockerfile -t ersatztv:dev .
|
||||
|
||||
Every task that closes a Gitea issue MUST complete ALL of these before it is considered done. Use `/done <issue>` to run through this automatically.
|
||||
|
||||
**Merge-consent is derived from state, not asserted (`## Done-when` convention — ersatztv#303 H6).** Any issue whose PR will merge to `main` should carry a `## Done-when` section in its **issue body** — a checklist of completion criteria (always include an "adversarial review passed" box; add per-issue criteria like tests-green, docs-updated, live-E2E). Two hooks derive merge-consent from it so a premature merge is blocked *by construction*, not by memory:
|
||||
- `pretooluse-merge-consent.sh` (Claude PreToolUse on the Gitea merge tool) — **allows** a merge only when the PR's CI is green **and** every `## Done-when` box on the linked issue (`fixes #N`) is ticked; **denies** on an unticked box or red CI; **asks** (falls back to a human prompt) when it can't derive state (no linked issue, no `## Done-when` section, no creds, Gitea down).
|
||||
**Merge-consent is derived from state, not asserted (`## Done-when` convention — ersatztv#303 H6 + H10).** Any issue whose PR will merge to `main` should carry a `## Done-when` section in its **issue body** — a checklist of completion criteria (always include an "adversarial review passed" box; add per-issue criteria like tests-green, docs-updated, live-E2E). Two hooks derive merge-consent from it so a premature merge is blocked *by construction*, not by memory:
|
||||
- `pretooluse-merge-consent.sh` (Claude PreToolUse on the Gitea merge tool) — **allows** a merge only when the PR's CI is green **and** every `## Done-when` box on the linked issue (`fixes #N`) is ticked **and** a `Review-verdict:` comment references the PR's *current head sha* (**H10**); **denies** on an unticked box, red CI, or a stale/negative review verdict; **asks** (falls back to a human prompt) when it can't derive state (no linked issue, no `## Done-when` section, no `Review-verdict:` comment yet, no creds, Gitea down). **The H10 review-verdict convention**: after an adversarial/Codex review of a PR (or its latest fix commit), post a PR comment with a line `Review-verdict: <MERGEABLE|APPROVED|BLOCKED> @ <head-sha>` — this proves the *latest* commit was reviewed, not a stale earlier diff (ersatztv#242).
|
||||
- `.husky/pre-push` → `prepush-donewhen.sh` — a fail-open backstop that blocks a direct `git push origin main` whose commits `fix #N` an issue with unticked boxes.
|
||||
|
||||
Both need Gitea read creds in the env to enforce (**`ETV_GITEA_BASICAUTH=user:pass`** or `ETV_GITEA_TOKEN`; `ETV_GITEA_URL` overrides the base). Without them the merge hook asks and the push backstop is a no-op — the gate degrades to today's manual confirmation, never a silent pass. Docs-only PRs/pushes are exempt.
|
||||
|
||||
@@ -74,6 +74,7 @@ keep append-only from accreting stale, contradictory, or unreadably-large histor
|
||||
- [2026-07-12 — Merge-consent derived from state via a `## Done-when` issue checklist (#303 H6)](#2026-07-12--merge-consent-derived-from-state-via-a--done-when-issue-checklist-303-h6)
|
||||
- [2026-07-12 — Blocking CI gate for API-contract artifacts (#303 H4/H5)](#2026-07-12--blocking-ci-gate-for-api-contract-artifacts-303-h45)
|
||||
- [2026-07-12 — decisions.md is append-only, enforced; root-screenshot guard (#303 H9/H3)](#2026-07-12--decisionsmd-is-append-only-enforced-root-screenshot-guard-303-h9h3)
|
||||
- [2026-07-12 — Review-verdict merge-gate: latest commit must be reviewed (#303 H10)](#2026-07-12--review-verdict-merge-gate-latest-commit-must-be-reviewed-303-h10)
|
||||
|
||||
---
|
||||
|
||||
@@ -1367,3 +1368,44 @@ Companion guard **H3**: the Husky `pre-commit` hook refuses a staged **root-leve
|
||||
review/debug screenshot dropped at the repo root) — belt-and-suspenders with the `.gitignore` rule, so
|
||||
a forced `git add -f` still can't land one. Nested `*.png` (real assets) are unaffected. Rationale for
|
||||
both: the methodology review (#303) — make the process rules derivations/hooks, not prose to remember.
|
||||
|
||||
---
|
||||
|
||||
## 2026-07-12 — Review-verdict merge-gate: latest commit must be reviewed (#303 H10)
|
||||
|
||||
**A PR may not merge until a `Review-verdict:` comment on it references the PR's CURRENT head sha** —
|
||||
so the *latest* commit is proven-reviewed, not a stale earlier diff. This mechanizes the ersatztv#242
|
||||
lesson ("re-review the fix commit, not just the initial PR diff": a review of an earlier revision does
|
||||
not license merging a head that carries un-reviewed follow-up commits). It folds into the existing H6
|
||||
`pretooluse-merge-consent.sh` as condition (c), reusing its PR fetch, docs-only exemption, and
|
||||
Gitea-auth-from-env (no second hook → no detection drift, per the #303 methodology review).
|
||||
|
||||
Convention: after reviewing a PR (or its latest fix commit), post a PR **comment** (issue-style, not a
|
||||
Gitea formal-review body — the gate reads `issues/{pr}/comments`) whose line **starts with** the marker:
|
||||
`Review-verdict: <MERGEABLE|APPROVED|BLOCKED|NOT-MERGEABLE> @ <head-sha>` (short ≥7-char or full sha).
|
||||
The gate counts a line as a verdict only when the marker is at line-start (after optional indent) —
|
||||
a comment that merely *quotes* the template mid-sentence (an instruction "please post: Review-verdict:
|
||||
MERGEABLE @ …", or the gate's own suggestion text echoed back) does **not** self-approve the merge
|
||||
(adversarial re-review false-open, folded pre-merge). It then classifies each verdict line by the sha
|
||||
in its `@ <sha>` field, matched to the head by **git short-sha prefix semantics** (head *begins with*
|
||||
the token, token ≥7 chars) — NOT a loose substring test, so an older sha that merely contains the head
|
||||
prefix, or the head prefix appearing in an unrelated URL on the line, does not count:
|
||||
- a MERGEABLE/APPROVED/LGTM verdict whose `@ <sha>` is the current head → **allow**;
|
||||
- a **negative** verdict (BLOCKED/NOT-MERGEABLE) *on the head* → **deny**, and it *wins over* a positive
|
||||
one on the same head (a later BLOCKED retracts an earlier MERGEABLE; to retract, re-review head and
|
||||
post BLOCKED @ head). Staleness is **symmetric on purpose**: a negative for an *older* commit is stale
|
||||
exactly like a positive for an older commit, and does NOT override a fresh head-positive — otherwise a
|
||||
pre-fix `BLOCKED @ oldsha` would block forever even after the fix changes the sha and earns a fresh
|
||||
`MERGEABLE @ head` (the normal flow). So a genuine block must reference head, per the convention;
|
||||
- verdict comment(s) exist but reference only *older* commits → **deny** — the stale-review case #242
|
||||
targets;
|
||||
- a `Review-verdict:` marker with **no `@ <sha>`** at all → **ask** (a lazy/quoted marker; not
|
||||
mislabelled as stale);
|
||||
- no `Review-verdict:` comment at all → **ask** (graceful adoption, mirrors H6's "no Done-when →
|
||||
ask": surface, don't hard-block a PR that hasn't adopted the convention yet);
|
||||
- comments unfetchable / head sha unresolvable → **ask**.
|
||||
|
||||
Scope: the Claude PreToolUse gate on the Gitea merge tool only. A direct `git push origin main` has no
|
||||
PR comments to check, so the `.husky/pre-push` backstop is not extended for H10 (the merge tool is the
|
||||
real merge path; docs-only PRs remain exempt via H6's file-set exemption). Rationale, as with the whole
|
||||
Wave-1/2/3 hook set: make the process rule a derivation/hook, not prose to remember (#303).
|
||||
|
||||
Reference in New Issue
Block a user