[PROBE — DO NOT MERGE] ersatztv#672 experiment A: is a pull_request workflow definition head-resolved? #693
@@ -1,59 +1,13 @@
|
||||
name: Review verdict
|
||||
|
||||
# ersatztv#622 — server-side H10 enforcement.
|
||||
# ersatztv#672 PROBE — THROWAWAY. This is a deliberately rewritten copy of the real
|
||||
# review-verdict.yml, used to answer one question empirically: does Gitea resolve a
|
||||
# `pull_request` workflow DEFINITION from the PR's own head commit?
|
||||
#
|
||||
# THE INVARIANT. `review-verdict/h10` is a REQUIRED status check on `main`, and a Gitea commit
|
||||
# status belongs to exactly ONE sha. So a commit that did not exist when a verdict was written can
|
||||
# never inherit that verdict: push a new head and the required context is simply absent, which
|
||||
# Gitea's merge-requirement check treats as not-passing. `merge_when_checks_succeed` therefore
|
||||
# refuses to fire until someone re-reviews THAT head. This is what closes the ersatztv#622 hole,
|
||||
# where the PreToolUse hook proved conditions (b) and (c) against the head at SCHEDULING time and
|
||||
# Gitea then merged whatever head happened to be green minutes later.
|
||||
#
|
||||
# WHAT THIS WORKFLOW DOES — and, importantly, does NOT do. It does NOT decide whether code was
|
||||
# reviewed; only a human/agent review does that, via `scripts/post-review-verdict.sh`, which writes
|
||||
# the `review-verdict/h10` status directly. This workflow only handles the two EXEMPT classes that
|
||||
# would otherwise deadlock, and marks everything else `pending` so the PR shows an explicit,
|
||||
# actionable blocking reason instead of a silently-missing check:
|
||||
#
|
||||
# 1. Bot-authored PRs (Renovate). Renovate uses `platformAutomerge: true` — i.e. Gitea's OWN
|
||||
# auto-merge — to land patch bumps unattended. A required verdict context with no exemption
|
||||
# would stall every dependency PR forever waiting on a human verdict.
|
||||
# 2. Docs-only PRs, mirroring the merge-consent hook's existing docs-only carve-out.
|
||||
#
|
||||
# BOTH exemptions are void when the PR touches a PROTECTED path (see PROTECTED below): the gate,
|
||||
# the CI definition, the git hooks, the scripts they call, and the CI toolchain image. A PR that
|
||||
# weakens the merge gate must never be able to exempt itself from the merge gate — that is the one
|
||||
# self-referential failure worth spending an explicit rule on. Note this also (deliberately) means
|
||||
# Renovate's `docker/ci/Dockerfile` base bumps need a real verdict; those already require the
|
||||
# manual publish-then-pin two-step (docs/ci-cd.md -> "CI toolchain image"), so unattended merge was
|
||||
# never correct for them anyway.
|
||||
#
|
||||
# WHY ITS OWN FILE, not a job in pr-checks.yml: that workflow sets `cancel-in-progress: true`, so a
|
||||
# superseding push cancels its runs. A cancelled run here would leave an EXEMPT PR with no success
|
||||
# status and no further pushes to re-trigger it — Renovate would stall silently. This workflow
|
||||
# therefore takes no cancelling concurrency group.
|
||||
#
|
||||
# This job's OWN status context ("Review verdict / Set review-verdict status (pull_request)") is
|
||||
# NOT the required check and is not what gates merges — `review-verdict/h10`, the status it POSTS,
|
||||
# is. Keeping them distinct is deliberate: a workflow cannot be allowed to satisfy the gate merely
|
||||
# by running successfully.
|
||||
#
|
||||
# THE CHANGED-FILE ENUMERATION IS NOT INLINE HERE (ersatztv#649). It lives in
|
||||
# `scripts/pr-changed-files.sh`, the single implementation this job and the advisory hook
|
||||
# `.claude/hooks/pretooluse-merge-consent.sh` both call. It used to be written twice, and drifted in
|
||||
# the dangerous direction: four rounds of ersatztv#643 hardening landed on the ADVISORY copy (whose
|
||||
# failure mode is a human prompt) and never reached THIS one (whose failure mode is a `success`
|
||||
# write to a required status with nobody in the loop). See
|
||||
# `docs/decisions/records/ci/shared-pr-file-enumeration.md`.
|
||||
#
|
||||
# WHY THE CHECKOUT TAKES THE PR'S **BASE** REF, NEVER THE HEAD. This job judges the PR, so the PR
|
||||
# must not be able to supply the code that judges it. Checking out `head.sha` would let a PR edit
|
||||
# `scripts/pr-changed-files.sh` to return an empty list and exempt itself — the `PROTECTED` list
|
||||
# below would flag the edit, but only if the enumeration that feeds it were trustworthy, and it
|
||||
# would be the PR's own. `base.sha` is the commit the PR merges INTO: already on `main`, already
|
||||
# reviewed. `persist-credentials: false` because nothing here pushes, and a checkout that leaves a
|
||||
# token in `.git/config` hands it to every script the job runs.
|
||||
# It does NOT post `review-verdict/h10`. It posts a distinct probe context instead, so the
|
||||
# experiment can never forge a green on the real required check. Read the result as:
|
||||
# probe context present + h10 absent -> the definition came from the HEAD (vulnerable)
|
||||
# h10 present + probe context absent -> the definition came from the BASE (not vulnerable)
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
@@ -66,182 +20,23 @@ defaults:
|
||||
jobs:
|
||||
set-verdict-status:
|
||||
name: Set review-verdict status
|
||||
runs-on: small # a few API calls; keep it off the build runners
|
||||
runs-on: small
|
||||
steps:
|
||||
# BASE, not head — see the header. `fetch-depth: 1` is enough: nothing here reads history,
|
||||
# only the working tree's `scripts/`.
|
||||
- name: Checkout the PR's BASE ref
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
ref: ${{ github.event.pull_request.base.sha }}
|
||||
fetch-depth: 1
|
||||
persist-credentials: false
|
||||
- name: Announce which definition is running
|
||||
run: echo "ETV672-PROBE-MARKER — the HEAD-supplied workflow definition is executing"
|
||||
|
||||
# FLOOR ONLY — never `--expect` in this workflow. `--expect` pins an exact version and fails
|
||||
# when it drifts, which is right for `script-tests` (advisory) and catastrophic here: this job
|
||||
# writes `review-verdict/h10`, a REQUIRED check on `main`, so a pin would turn any jq bump on
|
||||
# the runner into a repo-wide merge deadlock. Asserting the 1.6 floor is what the gates below
|
||||
# are written against; see docs/ci-cd.md -> "The jq contract".
|
||||
#
|
||||
# A hard failure here is correct and fails CLOSED: the job dies, no `review-verdict/h10` is
|
||||
# posted, and an absent required check blocks the merge. Guarded on presence because a PR
|
||||
# whose BASE predates ersatztv#658 has no such script, and "the base is old" is not a jq
|
||||
# problem — that case is handled as an enumeration failure below, with an actionable status.
|
||||
- name: jq preflight (floor only)
|
||||
run: |
|
||||
set -euo pipefail
|
||||
if [ -x ./scripts/jq-preflight.sh ]; then
|
||||
./scripts/jq-preflight.sh
|
||||
else
|
||||
echo "::warning::The PR's base ref has no scripts/jq-preflight.sh; skipping the version assertion. The enumeration step below will fail closed on its own."
|
||||
fi
|
||||
|
||||
- name: Classify the PR and post the review-verdict status
|
||||
- name: Post the probe status (never the real h10 context)
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||
BASE_URL: ${{ github.server_url }}/api/v1
|
||||
# `scripts/pr-changed-files.sh` reads GITEA_BASE_URL (not BASE_URL) and takes owner/repo as
|
||||
# two SEPARATE arguments (not one `owner/repo` string). Getting either wrong is silent, not
|
||||
# loud: the script would fall back to its hardcoded LAN default and enumerate the wrong
|
||||
# repo, or a wrong host that answers, rather than erroring. A value already ending in
|
||||
# /api/v1 is used as-is by the script.
|
||||
GITEA_BASE_URL: ${{ github.server_url }}/api/v1
|
||||
# BOTH names, same value, on purpose. The script's precedence is
|
||||
# `ETV_GITEA_URL` > `GITEA_BASE_URL` > a hardcoded LAN default (and `ETV_GITEA_TOKEN` >
|
||||
# `GITEA_TOKEN`), because its other caller is a developer Mac using the ETV_* convention.
|
||||
# Setting only the GITEA_* names would leave this job's explicit configuration NON-
|
||||
# authoritative: a runner that happened to export a stale ETV_GITEA_URL would silently
|
||||
# enumerate a different Gitea instance and post the verdict here from a diff read there.
|
||||
# Cheap to make deterministic; leave both set even though only one is read.
|
||||
ETV_GITEA_URL: ${{ github.server_url }}/api/v1
|
||||
ETV_GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||
REPO: ${{ github.repository }}
|
||||
PR: ${{ github.event.pull_request.number }}
|
||||
SHA: ${{ github.event.pull_request.head.sha }}
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
AUTHOR: ${{ github.event.pull_request.user.login }}
|
||||
PR_URL: ${{ github.event.pull_request.html_url }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
CONTEXT="review-verdict/h10"
|
||||
# Accounts whose PRs may merge without a human verdict. Renovate only — keep this list
|
||||
# minimal and explicit; every entry is an account that can land code unreviewed.
|
||||
BOTS="renovate"
|
||||
# Paths where NEITHER exemption applies, because a change here can alter the gate itself,
|
||||
# what CI runs, or what the hooks enforce.
|
||||
PROTECTED='^(\.claude/|\.gitea/|\.husky/|scripts/|docker/ci/)'
|
||||
# Docs-only: prose and decision records. Deliberately narrower than the hook's pattern,
|
||||
# which also lets .claude/.gitea/.husky through — that carve-out is safe there only
|
||||
# because it falls through to a HUMAN PROMPT, whereas here it would post a green status
|
||||
# with nobody in the loop.
|
||||
DOCS_ONLY='^(docs/|[^/]*\.md$)'
|
||||
|
||||
if [ -z "${GITEA_TOKEN:-}" ]; then
|
||||
echo "::error::No GITEA_TOKEN available, so the ${CONTEXT} status cannot be written. An exempt (bot/docs-only) PR will stall until this is fixed; a normal PR is unaffected — post its verdict with scripts/post-review-verdict.sh."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
gh() { curl -sf -H "Authorization: token $GITEA_TOKEN" "$@"; }
|
||||
|
||||
# --- Already decided for THIS sha? Never overwrite a real verdict. -------------------
|
||||
# A human/agent verdict for this exact head may already exist (the reviewer ran the
|
||||
# script before this workflow finished, or a rerun). Re-posting `pending` over it would
|
||||
# un-approve a reviewed head and stall the PR.
|
||||
#
|
||||
# Read the COMBINED endpoint, not `/statuses/{sha}`: the latter returns one row per
|
||||
# status POST (not per context) and pages at 50, so a head with a few CI reruns can push
|
||||
# an earlier verdict off the first page. Missing it here is NOT harmless — we would post
|
||||
# `pending` (or worse, an exemption `success`) over a real human verdict. The combined
|
||||
# endpoint returns latest-per-context, which is both what we mean and ~11 rows.
|
||||
#
|
||||
# An unreadable/unparseable response must NOT be read as "no verdict exists": fail the
|
||||
# job WITHOUT posting anything, so a transient API error can never overwrite a verdict.
|
||||
statusjson=$(gh "$BASE_URL/repos/$REPO/commits/$SHA/status?limit=100") || statusjson=""
|
||||
# The empty case is checked EXPLICITLY, not left to jq's exit status: `jq -e` over empty
|
||||
# input exits 4 on jq >= 1.7 but 0 on jq 1.6 (ersatztv#647), and the RUNNER SHIPS 1.6 —
|
||||
# so on a transient API error this guard passed, `existing` came back "", and the job
|
||||
# went on to post `pending` (or an exemption `success`) over a possibly-existing human
|
||||
# verdict. Exactly what the paragraph above says must never happen. The sibling fix in
|
||||
# .claude/hooks/pretooluse-merge-consent.sh (ersatztv#643) missed this copy: that hook
|
||||
# runs on a dev Mac with jq 1.8, this workflow runs where the bug is live.
|
||||
if [ -z "${statusjson//[[:space:]]/}" ] || ! printf '%s' "$statusjson" | jq -e '.statuses | type == "array"' >/dev/null 2>&1; then
|
||||
echo "::error::Could not read existing commit statuses for ${SHA:0:7}. Refusing to post anything rather than risk overwriting an existing verdict."
|
||||
exit 1
|
||||
fi
|
||||
existing=$(printf '%s' "$statusjson" \
|
||||
| jq -r --arg c "$CONTEXT" '[.statuses[] | select(.context == $c)] | first | .status // ""')
|
||||
if [ "$existing" = "success" ] || [ "$existing" = "failure" ]; then
|
||||
echo "${CONTEXT} is already '${existing}' on ${SHA:0:7} — leaving the existing verdict alone."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# --- Changed files: the SHARED enumeration, or no exemption. -------------------------
|
||||
# `scripts/pr-changed-files.sh` (from the BASE checkout) owns every guard this job used to
|
||||
# carry inline and six it did not: CR/LF rejection, `..` rejection, a closed `.status`
|
||||
# allow-list, `previous_filename` validated on EVERY row rather than only `renamed` ones,
|
||||
# termination only on a validated EMPTY page rather than a merely short one, and head-sha
|
||||
# binding across the paging round-trips. ersatztv#649.
|
||||
#
|
||||
# READ THE EXIT STATUS, NEVER THE STDOUT OF A FAILED RUN. exit 0 means "complete and bound
|
||||
# to $SHA"; anything else means "could not tell" and stdout is meaningless. That the
|
||||
# script happens to print nothing on its failure paths is redundancy, not contract —
|
||||
# `files` is therefore cleared explicitly rather than trusted to be empty. stderr is left
|
||||
# attached to the job log on purpose: its diagnostic is the only thing that distinguishes
|
||||
# a force-push mid-enumeration from a dead API.
|
||||
ENUM=./scripts/pr-changed-files.sh
|
||||
files=""
|
||||
complete=no
|
||||
enum_error=""
|
||||
if [ ! -x "$ENUM" ]; then
|
||||
# Only reachable for a PR whose BASE predates ersatztv#658. Fail closed with a readable
|
||||
# status rather than an absent one, so the PR shows why instead of stalling silently.
|
||||
enum_error="the PR's base ref (${BASE_SHA:0:7}) has no executable ${ENUM}"
|
||||
elif files=$("$ENUM" "${REPO%%/*}" "${REPO#*/}" "$PR" "$SHA"); then
|
||||
complete=yes
|
||||
else
|
||||
files=""
|
||||
enum_error="scripts/pr-changed-files.sh could not enumerate PR #${PR} at ${SHA:0:7} exhaustively (see the step log)"
|
||||
fi
|
||||
|
||||
files=$(printf '%s\n' "$files" | grep -v '^$' || true)
|
||||
count=$(printf '%s\n' "$files" | grep -c . || true)
|
||||
echo "Changed files (${count}, complete=${complete}):"
|
||||
printf '%s\n' "$files" | sed 's/^/ /'
|
||||
|
||||
exempt=no
|
||||
reason=""
|
||||
if [ "$complete" != yes ]; then
|
||||
reason="${enum_error} — no exemption"
|
||||
elif [ "${count:-0}" -eq 0 ]; then
|
||||
reason="no changed files could be read from the API — no exemption"
|
||||
elif printf '%s\n' "$files" | grep -qE "$PROTECTED"; then
|
||||
reason="touches a protected path (gate/CI/hooks/scripts/ci-image) — exemptions do not apply"
|
||||
elif printf '%s\n' "$BOTS" | tr ' ' '\n' | grep -qxF "$AUTHOR"; then
|
||||
exempt=yes
|
||||
reason="authored by the '$AUTHOR' bot account and touches no protected path"
|
||||
elif ! printf '%s\n' "$files" | grep -qvE "$DOCS_ONLY"; then
|
||||
exempt=yes
|
||||
reason="docs-only change (no code, no protected path)"
|
||||
else
|
||||
reason="awaiting an H10 review verdict for head ${SHA:0:7}"
|
||||
fi
|
||||
|
||||
if [ "$exempt" = yes ]; then
|
||||
state=success
|
||||
desc="Exempt: $reason"
|
||||
else
|
||||
state=pending
|
||||
desc="Awaiting review verdict for ${SHA:0:7}"
|
||||
fi
|
||||
echo "Decision: state=${state} — ${reason}"
|
||||
|
||||
payload=$(jq -n --arg s "$state" --arg c "$CONTEXT" --arg d "$desc" --arg u "$PR_URL" \
|
||||
'{state:$s, context:$c, description:$d, target_url:$u}')
|
||||
gh -X POST -H 'Content-Type: application/json' -d "$payload" \
|
||||
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null
|
||||
echo "Posted ${CONTEXT}=${state} on ${SHA:0:7}."
|
||||
|
||||
if [ "$state" = "pending" ]; then
|
||||
echo "::notice::This PR needs an H10 review verdict for head ${SHA:0:7} before it can merge. After reviewing, run: scripts/post-review-verdict.sh ${PR} MERGEABLE"
|
||||
fi
|
||||
payload=$(jq -n --arg u "$PR_URL" \
|
||||
'{state:"success", context:"review-verdict/h10-PROBE672", description:"ersatztv#672 probe: posted by a HEAD-supplied workflow definition", target_url:$u}')
|
||||
curl -sf -H "Authorization: token $GITEA_TOKEN" -X POST \
|
||||
-H 'Content-Type: application/json' -d "$payload" \
|
||||
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null
|
||||
echo "Posted review-verdict/h10-PROBE672 on ${SHA:0:7}."
|
||||
|
||||
Reference in New Issue
Block a user