timothy
fe3d29276a
PR Gates / decisions lifecycle (pull_request) Successful in 19s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 20s
PR Gates / Docs update reminder (pull_request) Successful in 21s
Review verdict / Set review-verdict status (pull_request_target) Successful in 7s
PR Gates / Script tests (pytest) (pull_request) Successful in 1m29s
review-verdict/h10 Review-verdict: MERGEABLE @ fe3d292 (base: main)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m54s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 15m3s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 15m49s
Round-5 cold review: the post-write check counted only human `Review-verdict:` rows above the high-water mark, which is not sufficient under the run overlap this branch measured. Sequence, all inside that regime, runs A and B on the same exempt-classified sha: the human BLOCKED lands BELOW A's mark (so A cannot see it), B masks it with an exemption `success`, and only afterwards writes the sentinel. A then finds nothing human above its mark, does not repair, and posts its own `success` on top of the sentinel. The human rejection is permanently green and every later run re-derives it — the repair race failing toward SUCCESS, while the record states it fails toward `pending`. The filter now counts two row shapes above the mark: a human verdict (non-null creator, `Review-verdict:` description) OR a machine sentinel (null creator, description exactly $REPAIR_DESC). A then repairs and both runs converge on the fixed point. It cannot false-fire: a pre-existing sentinel would have been seen at the FIRST read and forced the pending path, and this block only runs after a `success`, so a sentinel above the mark can only have been written mid-flight by another run. Mutation-verified on both halves independently — dropping the sentinel alternation reddens the new test; dropping the human half reddens the original race-2 test — so neither can be removed without a test noticing. Refs #706
854 lines
61 KiB
YAML
854 lines
61 KiB
YAML
name: Review verdict
|
|
|
|
# ersatztv#622 — server-side H10 enforcement.
|
|
#
|
|
# 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_target)")
|
|
# 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 context string carries the trigger name, so the #672 switch
|
|
# renamed it; that is safe only because it was never in branch protection's required list (which is
|
|
# the two `docker-build.yml` job contexts plus `review-verdict/h10`). Adding it there later would
|
|
# undo the distinction this paragraph exists to protect.
|
|
#
|
|
# 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.
|
|
#
|
|
# WHY THE TRIGGER IS `pull_request_target`, NOT `pull_request` (ersatztv#672). The base-ref checkout
|
|
# above binds the SCRIPTS this job runs to the base. It does not bind the job DEFINITION. Gitea
|
|
# resolves a `pull_request` workflow definition from the PR's own head commit, so a PR editing THIS
|
|
# FILE ran its own rewritten copy — which could delete the checkout above, or skip straight to
|
|
# posting `review-verdict/h10=success` for its head sha. `PROTECTED` did not help (the rewrite
|
|
# defines `PROTECTED` too) and neither did branch protection, which requires the *context* and
|
|
# carries `required_approvals: 0`, so a self-posted success satisfied it outright.
|
|
#
|
|
# Measured on this instance (Gitea 1.25.4) rather than inferred from GitHub, because the whole point
|
|
# is that the gate's authority is derived, not asserted. A scratch PR rewriting this file to post a
|
|
# distinct probe context posted exactly that context, and the real `review-verdict/h10` was never
|
|
# written at all — the base's definition never ran. Under `pull_request_target` the same rewrite was
|
|
# ignored: the BASE definition ran and posted `h10=pending`, on both `opened` and `synchronize`,
|
|
# with `secrets` still available.
|
|
#
|
|
# `pull_request_target` is normally the DANGEROUS trigger, and it is worth being explicit about why
|
|
# that reputation does not transfer here. Its footgun is running untrusted HEAD code with a
|
|
# privileged token. This job never checks out the head and never executes anything the PR supplies:
|
|
# it checks out `base.sha` and runs only scripts from that tree. The base-ref checkout is what makes
|
|
# this trigger safe, so the two must be read as one decision — reintroducing a head checkout under
|
|
# this trigger would be far worse than the bug being fixed here.
|
|
#
|
|
# `branches: [main]` IS LOAD-BEARING, not cosmetic. Base resolution means the BASE branch supplies
|
|
# the definition, so without this filter a PR opened into an attacker-pushed base branch would run
|
|
# THAT branch's rewritten gate — trading a head-supplied definition for a base-supplied one and
|
|
# closing nothing. It matters more than it looks because a commit status is repo-global per sha
|
|
# (#663): a `success` forged on a head sha under a scratch base is inherited by a later, real PR
|
|
# into `main` carrying the same head. With the filter, a PR whose base is not `main` produces no run
|
|
# and no status at all (verified the same way).
|
|
|
|
# `edited` IS LOAD-BEARING (ersatztv#698 route 1), not completeness for its own sake. Gitea fires it
|
|
# when a PR's base is retargeted, and a retarget changes the effective diff WITHOUT moving the head
|
|
# sha — so none of the other four types fire and the per-sha status stays exactly as it was. That is
|
|
# what made route 1 persist rather than merely exist: a PR was opened into `main`, retargeted to a
|
|
# scratch base while this job was in flight so the enumeration read docs-only and posted an exemption
|
|
# `success`, then retargeted BACK to `main`, where the forged success sat unchallenged on a head whose
|
|
# diff against `main` carried a C# file (reproduced as probe PR #703; `created_at == updated_at`
|
|
# afterwards proves nothing reclassified). With `edited`, the retarget back re-runs this job — and the
|
|
# short-circuit below now re-derives machine-written successes instead of inheriting them, which is
|
|
# the half that makes the re-run actually change the answer. The two are one fix; `edited` alone would
|
|
# re-run and then bail out on the existing `success`.
|
|
#
|
|
# BE PRECISE ABOUT WHAT THIS BUYS: detection, not atomicity or ordering. Runs are NOT serialized, so
|
|
# the stale run can post `success` AFTER the reclassifying run posts `pending` — restoring the forged
|
|
# state with no further event left to correct it — and an already-scheduled auto-merge can fire in the
|
|
# green window between them. The `main -> scratch -> main` ABA transition is therefore NARROWED and
|
|
# observable, not closed. Tracked as ersatztv#706; do not read this block as claiming otherwise.
|
|
on:
|
|
pull_request_target:
|
|
branches: [main]
|
|
types: [opened, reopened, synchronize, ready_for_review, edited]
|
|
|
|
defaults:
|
|
run:
|
|
shell: bash
|
|
|
|
jobs:
|
|
set-verdict-status:
|
|
name: Set review-verdict status
|
|
runs-on: small # a few API calls; keep it off the build runners
|
|
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
|
|
|
|
# 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
|
|
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 }}
|
|
# The base BRANCH the event was raised for, passed down to the enumeration so the diff it
|
|
# reads cannot silently be one against a different base (ersatztv#698 route 1). This comes
|
|
# from the `pull_request_target` event payload, which is fixed at event time and is exactly
|
|
# what a mid-run retarget cannot rewrite — the live PR object can, which is the whole bug.
|
|
# `branches: [main]` means this is always `main` today; it is threaded through as a value
|
|
# rather than hardcoded so the two stay consistent if the filter ever widens.
|
|
BASE_REF: ${{ github.event.pull_request.base.ref }}
|
|
AUTHOR: ${{ github.event.pull_request.user.login }}
|
|
PR_URL: ${{ github.event.pull_request.html_url }}
|
|
run: |
|
|
set -euo pipefail
|
|
|
|
CONTEXT="review-verdict/h10"
|
|
# The description this job writes when it repairs its own raced exemption (#706 race 2).
|
|
# It is a SENTINEL, not just a message: `read_existing_verdict` recognises it, and the
|
|
# classification below refuses to post `success` over it. Without that, the repair lasted
|
|
# exactly one event — the next run saw a machine-written `pending`, re-derived it, and
|
|
# posted `success` again, with its own freshly-taken high-water mark now ABOVE the human
|
|
# row, so the post-write check stayed silent and the rejection went green a second time.
|
|
# Found by cold review. Refusing here can only ever withhold an exemption, never grant one.
|
|
REPAIR_DESC="Human verdict raced this exemption write — re-post the verdict"
|
|
# 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"
|
|
# The bot exemption is additionally constrained by CONTENT (ersatztv#698 route 2), because
|
|
# identity alone is not attributable to whoever wrote the code. `AUTHOR` is
|
|
# `pull_request.user.login` — the PR's CREATOR, which is immutable — while the head a PR
|
|
# points at is not: force-push application code onto an open Renovate branch and the PR is
|
|
# still authored by `renovate`, still touches no protected path, and was exempted. Nothing
|
|
# in the identity check attributes the CODE to the bot.
|
|
#
|
|
# Checking the pusher instead would not fix it — a git author/committer is self-asserted
|
|
# text and forgeable. So the exemption is gated on what a dependency bump can legitimately
|
|
# BE: an unattended merge is justified only for the manifests Renovate actually edits.
|
|
#
|
|
# The set is measured, not guessed: across all 11 Renovate PRs this repo has ever had, the
|
|
# paths touched were `Directory.Packages.props` (10 of them) and `.config/dotnet-tools.json`
|
|
# (1). The npm manifests are deliberately NOT included — see the BOT_MANIFESTS note below.
|
|
#
|
|
# Deliberately EXCLUDED, with the cost stated: `*.csproj` and any source file. The one
|
|
# historical Renovate PR outside the set above is #20, which touched a `.csproj` AND two C#
|
|
# files — and received an unattended bot exemption for a source change. Under Central
|
|
# Package Management versions live in `Directory.Packages.props`, so a `.csproj` edit
|
|
# attributed to Renovate is anomalous by construction. Such a PR is not blocked, it simply
|
|
# needs a real verdict, which is the correct handling for a PR carrying source changes.
|
|
# NOTE the npm manifests are deliberately ABSENT. An earlier draft included
|
|
# `web/package.json` / `web/package-lock.json` "so a first SPA bump cannot deadlock". That was
|
|
# a self-inflicted code-execution vector for zero benefit: `renovate.json` sets
|
|
# `enabledManagers: ["nuget", "github-actions", "dockerfile"]`, so Renovate does not manage npm
|
|
# in this repo at all, while `package.json` carries `scripts` that CI EXECUTES (`npm ci`,
|
|
# `npm run build` in docker-build.yml). Exempting it would let a hijacked bot branch run
|
|
# arbitrary shell in CI while every path still "looked like a manifest". If npm is ever added
|
|
# to enabledManagers, the lockfile may be exemptible but `package.json` is not.
|
|
BOT_MANIFESTS='^(Directory\.Packages\.props|\.config/dotnet-tools\.json)$'
|
|
# Paths where NEITHER exemption applies, because a change here can alter the gate itself,
|
|
# what CI runs, or what the hooks enforce.
|
|
#
|
|
# `.codex/` is listed alongside `.claude/` (ersatztv#711). `.codex/hooks/` is a
|
|
# byte-identical mirror of `.claude/hooks/` — including `pretooluse-merge-consent.sh` —
|
|
# generated as the Codex-side port of the same enforcement hooks. Without it the rule "a PR
|
|
# that can weaken the gate must not exempt itself from the gate" was expressed as a path
|
|
# list that had gone incomplete: editing `.claude/hooks/pretooluse-merge-consent.sh`
|
|
# correctly voided the exemption while editing its `.codex/` twin did not. Today that is
|
|
# LATENT rather than live — `.codex/` is untracked and gitignored, and a PR cannot touch a
|
|
# path that is not in the repo — but it becomes live the moment anyone tracks it, which is
|
|
# the natural instinct given `.claude/` is tracked. Listed now because the cost is one
|
|
# alternation and the failure mode is silent.
|
|
#
|
|
# The list stays ENUMERATIVE rather than derived (e.g. "any dotted top-level directory
|
|
# containing executable hooks"). A derived rule has to be evaluated against the PR's own
|
|
# file list, which is the very thing being classified — more moving parts inside a security
|
|
# predicate, to remove a maintenance burden that is one line per new tooling directory.
|
|
PROTECTED='^(\.claude/|\.codex/|\.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" "$@"; }
|
|
|
|
# DEFINED HERE, BEFORE ANY USE. An earlier round defined these AFTER the classification
|
|
# chain that calls them, so `count_matching` was `command not found` on every run, the
|
|
# PROTECTED branch silently never fired, and three "protected path" tests still passed —
|
|
# they reached `pending` by another route, so the guard being dead was invisible.
|
|
#
|
|
# HOW THE PATH PREDICATES ARE EVALUATED, and why neither obvious spelling is used.
|
|
#
|
|
# `producer | grep -q…` is FORBIDDEN here: `grep -q` exits at its first match, the producer
|
|
# then takes SIGPIPE and exits 141 once the list exceeds the pipe buffer, and under
|
|
# `set -o pipefail` the pipeline is a FAILURE even though grep MATCHED — inverting the guard
|
|
# for exactly the large PRs that matter. Reproduced with `A.cs` + 1900 docs paths (171KB,
|
|
# inside the enumerator's 2000-file cap): `docs_only=yes`, status 141; and a `.gitea/` path
|
|
# made `PROTECTED` MISS. That construct predates #698 and was live on `main`.
|
|
#
|
|
# A here-string (`grep -q… <<< "$files"`) fixes the SIGPIPE but bash materialises a large
|
|
# here-string via TEMPORARY STORAGE, so it can fail when the runner's temp space is full or
|
|
# unwritable — and because these run inside `if`/`!`, that failure would flip the predicate
|
|
# the same way. Trading a buffer bug for an environmental one is not a fix.
|
|
#
|
|
# So: count with `grep -c`, which DRAINS stdin (no early exit, no SIGPIPE) over an ordinary
|
|
# pipe (no temp file), and treat grep's own exit status honestly — `grep -c` exits 1 when the
|
|
# count is zero, which is a legitimate answer, while anything >1 is a real error and must FAIL
|
|
# THE JOB rather than silently read as "no match". `set -e` would not catch these on its own
|
|
# because they sit inside command substitution in a conditional.
|
|
count_matching() { # how many lines of $2 match $1
|
|
local out st=0
|
|
out=$(printf '%s\n' "$2" | grep -cE "$1") || st=$?
|
|
# NOT `exit 1`: these run inside `$( )`, so an exit leaves only the SUBSHELL and, because
|
|
# the substitution sits in a conditional, `set -e` does not fire either — the job would sail
|
|
# on with the predicate silently reading as "no match". Emit a NON-NUMERIC sentinel instead
|
|
# and let the caller, at top level, refuse to classify.
|
|
if [ "$st" -gt 1 ]; then
|
|
echo "::error::grep failed (status ${st}) evaluating a path predicate." >&2
|
|
printf 'ERR'
|
|
return 0
|
|
fi
|
|
printf '%s' "${out:-0}"
|
|
}
|
|
count_not_matching() { # how many lines of $2 do NOT match $1
|
|
local out st=0
|
|
out=$(printf '%s\n' "$2" | grep -cvE "$1") || st=$?
|
|
if [ "$st" -gt 1 ]; then
|
|
echo "::error::grep failed (status ${st}) evaluating a path predicate." >&2
|
|
printf 'ERR'
|
|
return 0
|
|
fi
|
|
printf '%s' "${out:-0}"
|
|
}
|
|
|
|
|
|
# --- Is there already a verdict for THIS sha? ----------------------------------------
|
|
# NOTE the heading no longer says "never overwrite". It cannot promise that: the read below
|
|
# and the POST at the end of this job are not atomic, so a human verdict posted in between is
|
|
# still overwritten. The re-read immediately before the POST narrows that window; it does not
|
|
# close it. Tracked as ersatztv#706 rather than claimed as solved.
|
|
# Reads the CONTEXT row for $SHA and sets ex_state / ex_creator / ex_desc / ex_human.
|
|
# Factored into a function because it is now called TWICE — once here, and once immediately
|
|
# before the POST (see below). An unreadable/unparseable response must NOT be read as "no
|
|
# verdict exists": the job dies WITHOUT posting, so a transient API error can never overwrite
|
|
# a verdict.
|
|
#
|
|
# 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, and THE RUNNER SHIPS 1.6 (ersatztv#647) — so on a
|
|
# transient error this guard passed, the row came back "", and the job posted over a
|
|
# possibly-existing human verdict.
|
|
#
|
|
# The COMBINED endpoint is read, not `/statuses/{sha}`: the latter returns one row per 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.
|
|
read_existing_verdict() {
|
|
local json row
|
|
json=$(gh "$BASE_URL/repos/$REPO/commits/$SHA/status?limit=100") || json=""
|
|
if [ -z "${json//[[:space:]]/}" ] || ! printf '%s' "$json" | 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
|
|
row=$(printf '%s' "$json" | jq -r --arg c "$CONTEXT" '[.statuses[] | select(.context == $c)] | first // {}')
|
|
ex_state=$(printf '%s' "$row" | jq -r '.status // ""')
|
|
ex_creator=$(printf '%s' "$row" | jq -r '.creator.login // ""')
|
|
ex_desc=$(printf '%s' "$row" | jq -r '.description // ""')
|
|
# A `case` prefix test rather than grep: the description is a single short string, and this
|
|
# removes one more pipeline from a security predicate entirely. The PATTERN is a literal, so
|
|
# there is no glob-injection concern from $ex_desc.
|
|
# A human verdict also has to have been formed against THIS base (ersatztv#698, found in
|
|
# round-4 review). `post-review-verdict.sh` records the base it reviewed in the status
|
|
# description — `Review-verdict: MERGEABLE @ abc1234 (base: main)` — precisely because
|
|
# retargeting changes the effective diff without moving the head sha (ersatztv#632).
|
|
# Without this check the sha-binding is escapable through the HUMAN path rather than the
|
|
# exemption path: get a genuine `success` on head H while it targets a scratch base S with
|
|
# a benign diff, then retarget H onto `main`, where its diff contains unreviewed code. The
|
|
# status is real, its creator is real, and it was silently inherited. The merge-consent
|
|
# hook compares the base and would object, but that is advisory and covers only its own
|
|
# path — a merge through the Gitea UI or API just sees a green required check.
|
|
#
|
|
# An ABSENT base is deliberately NOT treated as a mismatch: verdicts predating #632 carry
|
|
# no `(base: …)`, and re-deriving over one would un-approve a genuinely reviewed head. Only
|
|
# a base that is PRESENT and DIFFERENT is rejected, which is exactly the escape above.
|
|
ex_human=no
|
|
ex_repair=no
|
|
case "$ex_desc" in
|
|
"$REPAIR_DESC"*) ex_repair=yes ;;
|
|
esac
|
|
case "$ex_desc" in
|
|
"Review-verdict:"*)
|
|
if [ -n "$ex_creator" ]; then ex_human=yes; fi
|
|
;;
|
|
esac
|
|
if [ "$ex_human" = yes ]; then
|
|
# COMPARE, NEVER PARSE. Two earlier attempts both extracted the base out of the
|
|
# description and both were defeated, the second in a way that looked like a fix for the
|
|
# first:
|
|
# * `${ex_desc##*"(base: "}` (LAST occurrence) let an APPENDED `(base: main)` override a
|
|
# genuine `(base: probe/scratch)`;
|
|
# * `${ex_desc#*"(base: "}` (FIRST occurrence) fixed that, but `${...%%)*}` still
|
|
# truncates at the first `)`. `main)evil` IS A VALID GIT BRANCH NAME
|
|
# (`git check-ref-format --branch 'main)evil'` succeeds), so a verdict earned while
|
|
# targeting it reads `(base: main)evil)`, truncates to exactly `main`, and is
|
|
# INHERITED after retargeting onto `main`. No forged description, no #697 needed.
|
|
# The comment here previously asserted a `)` in a branch name "mismatches — safe
|
|
# direction"; that was generalised from `feat/foo)bar` and is FALSE for any branch
|
|
# whose name starts with the target base.
|
|
#
|
|
# So extract nothing. `post-review-verdict.sh` writes the marker LAST, so require the
|
|
# description to END with the exact literal `(base: <this PR's base>)` and to contain
|
|
# exactly ONE marker — which kills the append trick without having to decide which
|
|
# occurrence is authoritative. Pure shell; no truncation exists to abuse.
|
|
#
|
|
# `${#}` arithmetic rather than a `grep -o | wc -l` pipeline; 7 is the length of
|
|
# "(base: ". An ABSENT marker is still not a mismatch (verdicts predate #632).
|
|
ex_stripped=${ex_desc//"(base: "/}
|
|
ex_markers=$(( (${#ex_desc} - ${#ex_stripped}) / 7 ))
|
|
if [ "$ex_markers" -ne 0 ]; then
|
|
ex_base_ok=no
|
|
if [ "$ex_markers" -eq 1 ]; then
|
|
case "$ex_desc" in
|
|
*"(base: $BASE_REF)") ex_base_ok=yes ;;
|
|
esac
|
|
fi
|
|
if [ "$ex_base_ok" != yes ]; then
|
|
ex_human=no
|
|
echo "${CONTEXT} on ${SHA:0:7} is a human verdict, but its recorded base does not match this PR's base '${BASE_REF}' (description: ${ex_desc}) — the reviewed diff is not this PR's diff, so it is NOT treated as a verdict for this base."
|
|
fi
|
|
fi
|
|
fi
|
|
}
|
|
|
|
# --- The retarget fence (ersatztv#706 race 1) ----------------------------------------
|
|
# THE PROBLEM THIS SOLVES. Two `pull_request_target` runs for one PR overlap, and the OLDER
|
|
# one can finish LAST — so a run that classified against a base the PR no longer targets can
|
|
# post its stale answer over a fresher run's correct one, permanently. Measured on this
|
|
# instance rather than assumed: probe PR #722 run 7520 (`opened`) ran to completion 20s AFTER
|
|
# run 7521 (`synchronize`) had started.
|
|
#
|
|
# WHY NOT A CONCURRENCY GROUP, which is the obvious answer and what #706 proposed. It does
|
|
# not work here, also measured: with `concurrency: {group: …-${{ pr number }},
|
|
# cancel-in-progress: false}` active on an identical probe, runs 7528 and 7529 still ran
|
|
# CONCURRENTLY and 7528 ended 36s after 7529 began. Gitea 1.25.4 does auto-cancel superseded
|
|
# `push` runs on a branch — a negative control with no `concurrency:` key at all showed that —
|
|
# but that behaviour does NOT extend to `pull_request_target`. `cancel-in-progress: true` is
|
|
# deliberately untried: cancellation is the one thing this workflow's own header refuses,
|
|
# because a cancelled run leaves an EXEMPT PR statusless with nothing left to re-trigger it.
|
|
#
|
|
# WHY A COUNTER AND NOT THE BRANCH NAME. The attack is an ABA: `main → S → main`. Every
|
|
# name-based check reads `main` at both ends and passes, which is exactly how route 1 got a
|
|
# forged exemption. Gitea's issue timeline records each retarget as a `change_target_branch`
|
|
# event with `old_ref`/`new_ref`; the COUNT of those events is monotonic and cannot alias.
|
|
# Verified on the real route-1 reproduction (PR #703: two events, `main → probe698/base-S` at
|
|
# 18:17:29 and back at 18:18:31) with a negative control (PR #717, never retargeted: zero).
|
|
#
|
|
# WHY ABSTAINING IS NOT A STALL — the property the whole design rests on. A retarget always
|
|
# fires `edited`, which is in this workflow's `types:` (see the header). So the very event
|
|
# that makes this run abstain has already queued a successor whose window opens after it.
|
|
# Abstention hands off; it does not drop the PR. The induction terminates when retargeting
|
|
# stops, and the last run has a clean window and writes the final answer. This is why the
|
|
# fence does not reintroduce the statusless-exempt-PR failure that rules out cancellation:
|
|
# it never stops a run from RUNNING, only from WRITING state it knows is stale.
|
|
#
|
|
# `updated_at` was considered as the key and rejected: it moves for comments and labels,
|
|
# which fire none of this workflow's `types:`, so a run could abstain with no successor
|
|
# coming — a real stall. The retarget count moves only for the mutation that actually
|
|
# invalidates a classification, and that mutation always brings its own re-run.
|
|
#
|
|
# Completeness is a guard, not an assumption (`ci.paged-endpoint-completeness`): the count is
|
|
# trusted ONLY when paging reached a validated EMPTY page. A short page, a non-array body, a
|
|
# non-numeric length or the page cap all leave `rt_ok=no`, and an untrusted count is treated
|
|
# below as "cannot tell" rather than as zero.
|
|
count_retargets() {
|
|
rt_count=0
|
|
rt_ok=no
|
|
local page=1 raw n m total=0
|
|
while [ "$page" -le 20 ]; do
|
|
raw=$(gh "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || return 0
|
|
if [ -z "${raw//[[:space:]]/}" ] || ! printf '%s' "$raw" | jq -e 'type == "array"' >/dev/null 2>&1; then
|
|
return 0
|
|
fi
|
|
n=$(printf '%s' "$raw" | jq -r 'length')
|
|
case "$n" in ''|*[!0-9]*) return 0 ;; esac
|
|
if [ "$n" -eq 0 ]; then rt_ok=yes; rt_count=$total; return 0; fi
|
|
m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length')
|
|
case "$m" in ''|*[!0-9]*) return 0 ;; esac
|
|
total=$(( total + m ))
|
|
page=$(( page + 1 ))
|
|
done
|
|
return 0
|
|
}
|
|
|
|
ex_repair=no
|
|
count_retargets
|
|
retargets_before=$rt_count
|
|
retargets_before_ok=$rt_ok
|
|
echo "Retarget fence: ${retargets_before} retarget event(s) observed before classifying (trusted=${retargets_before_ok})."
|
|
|
|
# --- Whose verdict is it? (ersatztv#698 route 3) -------------------------------------
|
|
# This short-circuit used to exit on ANY existing `success`, which made an exemption this job
|
|
# wrote indistinguishable from a verdict a human wrote. That is what let a forged exemption
|
|
# survive: obtained once — via the route-1 retarget race, a sibling workflow holding
|
|
# status-write credentials (#697), a direct API call, or inheritance across PRs by sha (#663)
|
|
# — it was thereafter accepted unchanged on every run, because the guard exited before it
|
|
# looked at the PR, the base, the author or the files.
|
|
#
|
|
# The guard still exists for its original reason: re-posting `pending` over a real human
|
|
# verdict would un-approve a reviewed head and stall the PR. So it discriminates by PROVENANCE.
|
|
#
|
|
# MEASURED on this instance (Gitea 1.25.4), on the COMBINED endpoint this job reads: a status
|
|
# POSTed with a USER credential — how `scripts/post-review-verdict.sh` writes a verdict —
|
|
# carries `.creator.login`, while one POSTed by an Actions job with the built-in `GITEA_TOKEN`
|
|
# carries `"creator": null`. A real verdict read back `creator=timothy`; this job's own
|
|
# exemption read back `creator=null`.
|
|
#
|
|
# BOTH conditions are required, and the DIRECTION of the test is the point: we short-circuit
|
|
# only on something POSITIVELY identified as a human verdict. Anything else, including anything
|
|
# we do not recognise, is RE-DERIVED. Written the other way round ("skip if it looks
|
|
# machine-written") an unrecognised shape would be trusted — the fail-open this issue is about.
|
|
#
|
|
# What this does NOT claim: the test asks "was this POSTed by a user credential", NOT "by a
|
|
# reviewer". `ETV_STATUS_AUTH` is basic auth, so head-controlled code can POST a success with a
|
|
# non-null creator AND an attacker-chosen `Review-verdict:` description, which this guard then
|
|
# preserves. That is #697 — provenance, not authentication.
|
|
read_existing_verdict
|
|
if [ "$ex_human" = yes ] && { [ "$ex_state" = "success" ] || [ "$ex_state" = "failure" ]; }; then
|
|
echo "${CONTEXT} is already '${ex_state}' on ${SHA:0:7}, written by '${ex_creator}' as a human verdict — leaving it alone."
|
|
exit 0
|
|
fi
|
|
if [ -n "$ex_state" ]; then
|
|
echo "${CONTEXT} is '${ex_state}' on ${SHA:0:7} but is NOT an attributable human verdict (creator='${ex_creator:-null}', description='${ex_desc}') — re-deriving it from the PR's current state rather than inheriting it."
|
|
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" "$BASE_REF"); 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/^/ /'
|
|
|
|
# Evaluated ONCE, at TOP LEVEL, so a failure can actually stop the job. Evaluating them
|
|
# inline inside the `if`/`elif` chain is what hid the two defects above: a bad status or a
|
|
# missing function turned into an empty string, `[ "" -gt 0 ]` errored, and the branch was
|
|
# simply skipped. A non-numeric result here is fatal and posts nothing — an absent required
|
|
# check blocks the merge, which is the correct direction.
|
|
n_protected=$(count_matching "$PROTECTED" "$files")
|
|
n_not_manifest=$(count_not_matching "$BOT_MANIFESTS" "$files")
|
|
n_not_docs=$(count_not_matching "$DOCS_ONLY" "$files")
|
|
for v in "$n_protected" "$n_not_manifest" "$n_not_docs"; do
|
|
case "$v" in
|
|
''|*[!0-9]*)
|
|
echo "::error::A path predicate returned '${v}' instead of a count — the classifier is not operating, so no ${CONTEXT} status will be written for ${SHA:0:7}."
|
|
exit 1 ;;
|
|
esac
|
|
done
|
|
|
|
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 [ "$n_protected" -gt 0 ]; then
|
|
reason="touches a protected path (gate/CI/hooks/scripts/ci-image) — exemptions do not apply"
|
|
else
|
|
# The two exemptions are evaluated as INDEPENDENT predicates rather than as a chain.
|
|
# An `elif` chain was wrong once the bot exemption gained a second condition
|
|
# (ersatztv#698 route 2): a Renovate PR that changes only `docs/` would enter the bot
|
|
# branch, fail the manifest test, and never reach the docs-only branch at all — silently
|
|
# withdrawing an exemption that the docs-only rule grants on its own merits, for any
|
|
# author. Composing the predicates and deciding afterwards keeps each rule's meaning
|
|
# independent of the order they happen to be written in.
|
|
#
|
|
# `grep -qv` asks "is there any line NOT in this allow-list", so an unrecognised path
|
|
# withholds the exemption instead of being ignored — the same closed-set direction the
|
|
# enumeration itself uses. Both are safe against an empty `$files` because `count -eq 0`
|
|
# is handled above.
|
|
# Written as `if`/`then`, never as `cmd && var=yes`: under `set -e` a bare `A && B`
|
|
# statement whose `A` fails takes the failure as the statement's own exit status and
|
|
# kills the job. That would fail closed here (no status posted, absent required check
|
|
# blocks the merge) but it would do so on the ORDINARY path — every non-bot PR — so the
|
|
# gate would look broken rather than strict. `cmd || var=yes` is safe for the same
|
|
# reason it is confusing; both are spelled out instead.
|
|
# BOTS is a short fixed literal, so it cannot reach the pipe buffer; it is still written
|
|
# with an explicit status capture so a grep error cannot read as "not a bot" by accident.
|
|
is_bot=no
|
|
bot_hits=$(printf '%s\n' "$BOTS" | tr ' ' '\n' | grep -cxF "$AUTHOR") || bot_hits=0
|
|
if [ "${bot_hits:-0}" -gt 0 ]; then is_bot=yes; fi
|
|
manifests_only=no
|
|
if [ "$n_not_manifest" -eq 0 ]; then manifests_only=yes; fi
|
|
docs_only=no
|
|
if [ "$n_not_docs" -eq 0 ]; then docs_only=yes; fi
|
|
|
|
if [ "$is_bot" = yes ] && [ "$manifests_only" = yes ]; then
|
|
exempt=yes
|
|
reason="authored by the '$AUTHOR' bot account, touches no protected path, and changes only dependency manifests"
|
|
elif [ "$docs_only" = yes ]; then
|
|
exempt=yes
|
|
reason="docs-only change (no code, no protected path)"
|
|
elif [ "$is_bot" = yes ]; then
|
|
reason="authored by the '$AUTHOR' bot account, but changes files outside the dependency-manifest set — a bot ACCOUNT does not attribute the CODE at this head (the account is the PR's immutable creator; the head is not), so this needs a real verdict"
|
|
else
|
|
reason="awaiting an H10 review verdict for head ${SHA:0:7}"
|
|
fi
|
|
fi
|
|
|
|
if [ "$ex_repair" = yes ]; then
|
|
# A previous run of this job already repaired a raced exemption on this sha, which means a
|
|
# human verdict was written for it and then lost. Re-granting the exemption would bury that
|
|
# rejection again. The PR needs a real verdict; only a human can clear this.
|
|
exempt=no
|
|
reason="a human verdict raced a previous exemption write on this head and was overwritten — this head needs a re-posted verdict, not another exemption"
|
|
fi
|
|
if [ "$exempt" = yes ]; then
|
|
state=success
|
|
desc="Exempt: $reason"
|
|
elif [ "$ex_repair" = yes ]; then
|
|
# CARRY THE SENTINEL FORWARD. This branch exists because the first version of it did not,
|
|
# and cold review reproduced the consequence: refusing the exemption but posting the
|
|
# GENERIC pending description overwrote the very sentinel the refusal depends on, so the
|
|
# next run saw an ordinary machine `pending`, re-derived it, and posted `success` — burying
|
|
# the human rejection two events after the repair instead of one. The block has to be a
|
|
# FIXED POINT: what this branch writes must be what re-triggers this branch.
|
|
#
|
|
# It is keyed on `ex_repair` alone rather than on the exempt path, so the marker also
|
|
# survives runs where the PR was not exemptible anyway — the fact being recorded is "a
|
|
# human verdict was lost on this sha", which is a property of the sha, not of this run's
|
|
# classification.
|
|
state=pending
|
|
desc="$REPAIR_DESC"
|
|
else
|
|
state=pending
|
|
desc="Awaiting review verdict for ${SHA:0:7}"
|
|
fi
|
|
echo "Decision: state=${state} — ${reason}"
|
|
|
|
# HIGH-WATER MARK for the post-write verification (ersatztv#706 race 2). Taken FIRST — before
|
|
# the re-read below, before the fence, before the POST — and the ORDER IS THE POINT.
|
|
#
|
|
# An earlier version captured it just before the POST, "as late as possible". Cold review
|
|
# caught that as a High: everything between the re-read and a late mark is a blind gap. A
|
|
# human verdict landing there is invisible to the re-read (which already happened) AND
|
|
# excluded from the post-write check (its id is BELOW a mark taken afterwards), so it is
|
|
# silently overwritten with no repair. That gap spans the entire retarget re-count — up to 20
|
|
# timeline round-trips — so it was far wider than the one-round-trip residual being claimed.
|
|
#
|
|
# Taking the mark first closes the read side completely: any row newer than the mark is caught
|
|
# either by the re-read (abstain, post nothing) or by the post-write check (repair). There is
|
|
# no false-fire cost to being early, because the test is `id > mark` — rows already present
|
|
# when the mark is taken are below it and stay invisible either way.
|
|
#
|
|
# Presence alone would be the wrong test, and wrong in the direction that breaks the gate: the
|
|
# short-circuit deliberately does NOT stop for a human verdict whose recorded base does not
|
|
# match this PR's (`ex_human` is reset to `no` — see `read_existing_verdict`). Such a row stays
|
|
# in the history forever, so a presence test would fire on EVERY later run of that PR,
|
|
# downgrade every exemption to `pending`, and deadlock it permanently.
|
|
#
|
|
# `max` over an empty array is `null`, hence `// 0`. `.id? // 0` rather than `.id`: a bare
|
|
# `.[].id` hard-errors under `set -e` if the array ever holds a non-object, which would kill
|
|
# the job before it posts and strand an ordinary PR with no status at all.
|
|
max_id_before=-1
|
|
hist_before=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=100") || hist_before=""
|
|
if [ -n "${hist_before//[[:space:]]/}" ] && printf '%s' "$hist_before" | jq -e 'type == "array"' >/dev/null 2>&1; then
|
|
mark=$(printf '%s' "$hist_before" | jq -r '[.[] | .id? // 0] | max // 0' 2>/dev/null || true)
|
|
case "$mark" in
|
|
''|*[!0-9]*)
|
|
# SKIP the check rather than treat everything as raced. A mark of 0 would make every
|
|
# pre-existing human row look newer than the mark and repair every exemption away.
|
|
echo "::warning::Status high-water mark for ${SHA:0:7} was not numeric ('${mark}'); the post-write race check will be skipped."
|
|
max_id_before=-1 ;;
|
|
*) max_id_before=$mark ;;
|
|
esac
|
|
else
|
|
# Not fatal: the POST below is still correct, only the after-the-fact verification is
|
|
# weakened. Recorded so a silent degradation is visible in the log.
|
|
echo "::warning::Could not establish a status high-water mark for ${SHA:0:7}; the post-write race check will be skipped."
|
|
max_id_before=-1
|
|
fi
|
|
|
|
# LAST-MOMENT RE-READ (ersatztv#706). Classification takes several API round-trips, and a
|
|
# reviewer can post a verdict during them — most dangerously a `failure`, which this job would
|
|
# then overwrite with an exemption `success`, turning an explicit human rejection green. The
|
|
# first read cannot see that; this one can. It NARROWS the window, it does not close it: there
|
|
# is no compare-and-set on Gitea's status API, so a verdict landing between this read and the
|
|
# POST below is still lost — which is what the post-write repair below is for.
|
|
read_existing_verdict
|
|
if [ "$ex_human" = yes ]; then
|
|
echo "::notice::A human verdict ('${ex_state}' by '${ex_creator}') landed on ${SHA:0:7} while this job was classifying — leaving it alone and posting nothing."
|
|
exit 0
|
|
fi
|
|
|
|
# A SENTINEL THAT APPEARED MID-RUN (ersatztv#706, round-3 review). The re-read above recomputes
|
|
# `ex_repair`, and until this guard existed nothing downstream read it: the POST writes the
|
|
# `$state` frozen at classification time, so a STALE OVERLAPPING RUN would post its `success`
|
|
# straight over a sentinel another run had just written — burying a human rejection, with no
|
|
# repair (the human row is below this run's mark) and no log. That fails toward SUCCESS, so it
|
|
# was not covered by the "repair fails toward pending" residual; it is the exact outcome this
|
|
# whole change exists to prevent, reached through the run overlap this branch itself measured.
|
|
#
|
|
# THE RULE IS "NEVER REPLACE A SENTINEL WITH A NON-SENTINEL", not "never overwrite it with a
|
|
# success". A first draft of this guard tested `state = success`, which is one branch too
|
|
# narrow: a run can reach the POST on `state=pending` carrying the GENERIC description — most
|
|
# realistically after a transient enumeration failure (`complete != yes`) — and that run
|
|
# passes a success-only guard, passes the fence, and overwrites the sentinel with ordinary
|
|
# text. The next run then sees no sentinel, re-derives, and posts `success`: the same buried
|
|
# human rejection as before, reached in two steps instead of one.
|
|
#
|
|
# Comparing the DESCRIPTION rather than the state is exactly as precise and strictly more
|
|
# general. A sentinel present at the FIRST read forces `desc="$REPAIR_DESC"` (the carry-forward
|
|
# branch in the decision above), so this guard cannot fire on the ordinary repaired-head path
|
|
# and the fixed point is intact. Any other description alongside `ex_repair=yes` means the
|
|
# sentinel arrived DURING this run, whatever this run concluded.
|
|
#
|
|
# Abstaining is strictly correct here and, unlike the retarget fence, needs no successor run:
|
|
# the sentinel row is already `pending` and already carries the re-post instruction.
|
|
if [ "$ex_repair" = yes ] && [ "$desc" != "$REPAIR_DESC" ]; then
|
|
echo "::notice::A repair sentinel was written on ${SHA:0:7} while this job was classifying, meaning a human verdict was raced and repaired by another run. This run's exemption is stale — posting NOTHING and leaving the sentinel standing."
|
|
exit 0
|
|
fi
|
|
|
|
# THE FENCE ITSELF (ersatztv#706 race 1). Re-count the retargets as late as possible and
|
|
# refuse to write anything if the PR was retargeted since this run began. See the long note
|
|
# at `count_retargets` for why this is a handoff rather than a stall, and why the count is
|
|
# the only key that survives an ABA.
|
|
#
|
|
# The refusal covers `pending` as well as `success`, not just the dangerous write. A stale
|
|
# `pending` over a fresh `success` is only a stall rather than a forged green, so gating it
|
|
# is not strictly required — but the successor run is guaranteed either way, so there is
|
|
# nothing to buy by writing a value this run already knows was computed against a base the
|
|
# PR no longer targets. One rule, one direction, nothing to reason about per state.
|
|
#
|
|
# An UNTRUSTED count on either side (`rt_ok=no`: paging never reached a validated empty
|
|
# page, a page was unreadable, the cap was hit) is NOT treated as "no retarget". It blocks
|
|
# the exemption `success` only, and lets `pending` through: `pending` cannot turn a rejection
|
|
# or an unreviewed head green, so withholding it would strand PRs for no safety gain, while
|
|
# a `success` written on a count we could not verify is exactly the forged-green outcome
|
|
# this fence exists to prevent.
|
|
count_retargets
|
|
if [ "$retargets_before_ok" = yes ] && [ "$rt_ok" = yes ] && [ "$rt_count" -ne "$retargets_before" ]; then
|
|
echo "::notice::PR #${PR} was retargeted while this job was classifying (${retargets_before} -> ${rt_count} retarget events). This run's classification was computed against a base the PR may no longer target, so it posts NOTHING. The retarget fired an 'edited' event, so a successor run is already queued and will write the authoritative status for ${SHA:0:7}."
|
|
exit 0
|
|
fi
|
|
if { [ "$retargets_before_ok" != yes ] || [ "$rt_ok" != yes ]; } && [ "$state" = "success" ]; then
|
|
echo "::error::Could not establish a trusted retarget count for PR #${PR} (before=${retargets_before_ok}, after=${rt_ok}), so an exemption 'success' cannot be shown to have been computed against the PR's current base. Posting nothing; ${CONTEXT} stays absent, which blocks the merge. NOTE a later run only helps if the cause was transient — a PR whose timeline exceeds the page cap will fail this way on every run, and needs a human verdict."
|
|
exit 0
|
|
fi
|
|
|
|
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}."
|
|
|
|
# --- POST-POST VERIFICATION (ersatztv#706 race 2) ------------------------------------
|
|
# The last-moment re-read above narrows the window between reading and writing; it cannot
|
|
# close it, because Gitea's status API has no conditional write (no ETag, no If-Match, no
|
|
# expected-previous-state), so there is no compare-and-set to make the read and the POST one
|
|
# operation. A human `failure` landing in that remaining gap is overwritten by the POST above
|
|
# — turning an explicit human REJECTION green, which is the worst outcome this gate can
|
|
# produce and strictly worse than any stall.
|
|
#
|
|
# So verify AFTERWARDS and repair in the safe direction. This runs ONLY on the `success`
|
|
# path, and that restriction is the point rather than an optimisation: `pending` cannot
|
|
# turn a rejection green — it already blocks the merge — so the only write that can cause
|
|
# the damage is the exemption `success`.
|
|
#
|
|
# WHY A DIFFERENT ENDPOINT. Everywhere else this job reads the COMBINED endpoint
|
|
# (`/commits/{sha}/status`), which returns the LATEST status per context — and that is now
|
|
# OUR success, with the human's row buried underneath it. The combined view is therefore
|
|
# structurally incapable of showing the thing being looked for. `/statuses/{sha}` returns one
|
|
# row per POST instead. Measured on this instance, the two really do differ in shape as well
|
|
# as content: the combined endpoint returns an OBJECT with a `.statuses` array (12 rows on a
|
|
# live head), `/statuses/{sha}` a BARE ARRAY (24 rows on the same head) — hence the different
|
|
# `type == "array"` guard here.
|
|
#
|
|
# No claim is made about the order rows come back in, because the check does not depend on
|
|
# it: it selects by id against the high-water mark rather than inspecting the top of the
|
|
# list. A verdict older than the mark is invisible to it no matter where it sits.
|
|
#
|
|
# The repair is `pending`, NEVER a copy of the human's state. Re-posting their `failure`
|
|
# would attribute a human verdict to this job — the exact provenance confusion the
|
|
# `creator`-based short-circuit above exists to prevent, and it would be written with the
|
|
# machine credential, so it would read as machine-derived to every later run. `pending`
|
|
# asserts nothing about the review: it blocks the merge and asks for a real verdict, which
|
|
# is true and safe regardless of which way the human ruled. The reviewer is told to re-post.
|
|
#
|
|
# A read failure here does NOT fail the job: the status is already posted, so `exit 1` would
|
|
# change nothing about the gate's state while turning a routine API hiccup into a red run.
|
|
# It is reported loudly and left alone — the residual is the read/POST gap either way.
|
|
if [ "$state" = "success" ] && [ "$max_id_before" -ge 0 ]; then
|
|
post_hist=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=100") || post_hist=""
|
|
if [ -z "${post_hist//[[:space:]]/}" ] || ! printf '%s' "$post_hist" | jq -e 'type == "array"' >/dev/null 2>&1; then
|
|
echo "::warning::Could not re-read the status history for ${SHA:0:7} after posting, so a human verdict landing during the write window would not be detected. The exemption ${CONTEXT}=success stands."
|
|
else
|
|
# `.id > $since` is what confines this to the write window. Our OWN row is excluded twice
|
|
# over — it carries `creator: null` (an Actions-token POST, measured; see the provenance
|
|
# note above) and its description is `Exempt: …`, not `Review-verdict:` — so the count is
|
|
# of human verdicts that did not exist when the mark was taken.
|
|
# TWO row shapes count as "something raced this write", not one (round-5 review).
|
|
#
|
|
# (a) a HUMAN verdict — non-null creator, `Review-verdict:` description;
|
|
# (b) a machine SENTINEL — null creator, description exactly `$REPAIR_DESC`.
|
|
#
|
|
# (b) is not decoration. With two overlapping runs A and B, the human row can land BELOW
|
|
# A's mark (so (a) cannot see it) while B masks it with an exemption success and only
|
|
# afterwards writes the sentinel. A then finds nothing human above its mark, does not
|
|
# repair, and posts its own success ON TOP of the sentinel — a permanent forged green over
|
|
# a human rejection, which is precisely the outcome this whole change exists to prevent.
|
|
# Counting the sentinel closes it: A repairs, and both runs converge on the fixed point.
|
|
#
|
|
# It cannot false-fire. A sentinel that already existed would have been seen at the FIRST
|
|
# read, forcing the pending path, and this block only runs after a `success` — so a
|
|
# sentinel ABOVE the mark can only have been written by another run mid-flight.
|
|
raced=$(printf '%s' "$post_hist" | jq -r --arg c "$CONTEXT" --argjson since "$max_id_before" --arg rd "$REPAIR_DESC" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select((.id? // 0) > $since)
|
|
| select(
|
|
((.creator != null and .creator.login != null and .creator.login != "")
|
|
and (((.description // "") | startswith("Review-verdict:"))))
|
|
or ((.creator == null) and ((.description // "") == $rd))
|
|
)] | length')
|
|
case "$raced" in
|
|
''|*[!0-9]*)
|
|
echo "::warning::Post-write verification for ${SHA:0:7} returned '${raced}' instead of a count; not acting on it."
|
|
;;
|
|
*)
|
|
if [ "$raced" -gt 0 ]; then
|
|
# The last-moment re-read found no human verdict, so any row present now was
|
|
# written during the window and has just been masked by the exemption above.
|
|
echo "::error::A human ${CONTEXT} verdict landed on ${SHA:0:7} while this job was writing its exemption, and was overwritten. Downgrading to 'pending' so an explicit human decision cannot be silently green. Re-post it with: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
repair=$(jq -n --arg c "$CONTEXT" --arg u "$PR_URL" --arg d "$REPAIR_DESC" \
|
|
'{state:"pending", context:$c, description:$d, target_url:$u}')
|
|
# A failure HERE leaves the forged green standing, so it is retried once and then
|
|
# screams. `set -e` would otherwise kill the job silently, after the success was
|
|
# written and with nothing left to re-attempt.
|
|
if ! gh -X POST -H 'Content-Type: application/json' -d "$repair" \
|
|
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
|
|
if ! gh -X POST -H 'Content-Type: application/json' -d "$repair" \
|
|
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
|
|
echo "::error::COULD NOT REPAIR ${CONTEXT} on ${SHA:0:7}. An exemption 'success' is standing on a head whose human verdict was overwritten. Post the verdict again immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
exit 1
|
|
fi
|
|
fi
|
|
echo "Repaired ${CONTEXT} to pending on ${SHA:0:7}."
|
|
state=pending
|
|
fi
|
|
;;
|
|
esac
|
|
fi
|
|
fi
|
|
|
|
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
|