Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 5s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 21s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 10m21s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m56s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 7m2s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Failing after 1m35s
`count_pr_mutations` treated an empty page past page 1 as proof it had reached the end of the PR timeline. Gitea does not mean that: `ListIssueCommentsAndTimeline` applies the LIMIT/OFFSET in `FindComments` at the DATABASE level and filters AFTERWARDS, dropping `CommentTypeCode` rows and inaccessible cross-references into a nil slice that serializes as bare `null`. A page of 50 inline review comments is byte-identical to a page past the end while later pages still hold events, and rows are ASCENDING, so the events a fence looks for are the furthest from page 1. Fifty comments, which a PR author can create on their own PR, truncated both walks at the same place: both counts agreed, the sha comparison agreed, and an ABA force-push yielded an exemption `success` over a diff no single head justified. The walk no longer infers the end from an empty page BEFORE its cap. Such a page is skipped; the loop reads every page to its 20-page cap and trusts the counts only when the LAST page came back empty. An empty FIRST page and any unreadable shape still end the walk untrusted. NARROWED, NOT CLOSED, and the docs say so in one unit: the page-20 terminator is still trusted for the same unprovable reason, so the defeat now costs a timeline of over 1000 rows rather than ~100, with the same 50-row filtered block pinned to offsets 950..999. Measured at Gitea 1.27.1, ruling out the cheaper fixes: `X-Total-Count` on this endpoint is the post-filter length of the PAGE, not a total (`?limit=1` returns 1 on a 14-row timeline), while `/activities/feeds` returns a true total; `limit` clamps to 50; the only query params are `since`, `before`, `page`, `limit`, so the paged and serialized sets cannot be made to agree. Also: each page bounded `--connect-timeout 5 --max-time 15` and retried once, mirroring `page_statuses`, because the walk went from ~2 requests to a fixed 20 and the third call site runs after the exemption `success` is posted. Costs stated rather than hidden — worst case 40 requests and 20 sleeps, wall-clock pessimum 620s per walk, and the suite roughly doubled (202s -> 474s). Seven tests, each mutation-witnessed red; three reproduce the defeat against the shipped predecessor. Two independent cold reviews plus a re-review of the fix: no Blocker or High in the code. Their real finding was prose claiming the hole was closed, and cost arithmetic wrong twice. One reviewer claim was refuted by execution. Fixes #870 Refs: #803, #706, #664, #751, #893 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
2557 lines
200 KiB
YAML
2557 lines
200 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
|
|
# Explicit token scope (ersatztv#748), so the owner-level Actions default can be set to
|
|
# Restricted (server-management#714) without this job losing the ability to write the gate.
|
|
#
|
|
# DECLARING THIS IS EXHAUSTIVE — it is not additive. From the moment it merges, this job holds
|
|
# EXACTLY these units even while the owner default is still `permissive`. A missing unit does not
|
|
# degrade gracefully: the `curl -sf` dies under `set -euo pipefail` and `review-verdict/h10` is
|
|
# never posted, which blocks EVERY merge to `main` — including the PR that would repair it, and
|
|
# `block_admin_merge_override: true` (ersatztv#743) means there is no force-merge escape. Change
|
|
# this block only with the scratch-base-branch probe in `docs/ci-cd.md`; this workflow is never
|
|
# exercised by its own PR.
|
|
#
|
|
# Why `code: write` and not something narrower: Gitea has NO `statuses` scope (GitHub-only, and
|
|
# explicitly unsupported). `POST /repos/{o}/{r}/statuses/{sha}` is gated by
|
|
# `reqRepoWriter(unit.TypeCode)`, so status-write IS Code-write — see `ci.actions-credential-scoping`.
|
|
# `contents: write` would also work but additionally grants `releases: write`, which is not used.
|
|
permissions:
|
|
code: write # POST /statuses/{sha} (the gate) + the /commits/{sha}/status reads
|
|
issues: read # GET /issues/{n}/timeline — the retarget+push count fence (#706, head axis #803)
|
|
pull-requests: read # scripts/pr-changed-files.sh -> GET /pulls/{n}/files (#698 exemption path)
|
|
env:
|
|
CI_JOB_ROLE: guard
|
|
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
|
|
|
|
# NEVER WRITE AN EXPRESSION DELIMITER ANYWHERE IN THIS BODY, INCLUDING IN A COMMENT
|
|
# (ersatztv#751). A shell comment is not inert here. The runner scans this whole `run:`
|
|
# scalar for the expression opener before bash ever sees it, and one occurrence makes it
|
|
# rewrite the ENTIRE body into a single `format(...)` call so the result can be spliced
|
|
# back in. That rewrite is all-or-nothing: if the payload does not parse as an expression
|
|
# the interpolation of the whole scalar fails, and — the part that made this expensive —
|
|
# the runner DROPS THE STEP AND REPORTS THE JOB GREEN.
|
|
#
|
|
# That is exactly how this gate spent 2026-08-03 to 2026-08-06 dead: the #706 note above,
|
|
# explaining why a concurrency group does not work, quoted a `concurrency:` snippet
|
|
# containing a PR-number expression as ILLUSTRATION. `pr number` is not a valid
|
|
# expression, so this step never ran, `review-verdict/h10` was never posted by anything
|
|
# but a human hand, and both exemption classes silently stopped working while every run
|
|
# reported success. The prose documenting a fix disabled the fix.
|
|
#
|
|
# Two things now stop a recurrence, and they are deliberately different in kind: the
|
|
# start-marker below turns a dropped step into a RED job instead of a green one, and
|
|
# `test_the_verdict_workflow_has_NO_expression_delimiter_in_any_run_body` in
|
|
# scripts/tests/test_pr_changed_files.py rejects the delimiter at review time. Note that
|
|
# the older workflow-shape tests all scan comment-STRIPPED text (`_code_lines`) precisely
|
|
# so prose cannot redden them — which makes every one of them structurally blind to this
|
|
# class. The new test reads the raw scalar for that reason. To describe an expression in
|
|
# prose here, name it (`a github.event.pull_request.number expression`); do not quote the
|
|
# delimiters.
|
|
#
|
|
# THE START MARKER (ersatztv#751). Written before anything else can fail, and read by the
|
|
# `Assert the classifier actually executed` step below. Its absence means this step did
|
|
# not begin — the silent-green failure mode — and that is now a job failure. It records
|
|
# only that execution STARTED: every `exit 0` abstention path below is a legitimate
|
|
# outcome, so completion is deliberately not what is asserted.
|
|
# KEYED ON THE RUN, not a fixed name. Measured on this instance, `RUNNER_TEMP` is `/tmp` —
|
|
# not a private per-job directory — so a fixed name lives at a path other jobs can also
|
|
# write. The `small` lane starts a container per job today, which makes the file fresh in
|
|
# practice, but that is a property of the lane rather than a guarantee, and a STALE marker
|
|
# would satisfy the guard below on a run whose step was dropped: a silent PASS, the exact
|
|
# failure mode this guard exists to remove. Including the run id and attempt means a marker
|
|
# from any other run cannot answer for this one. If the runner does not export them the path
|
|
# degrades to a fixed name, which is no worse than having no key at all.
|
|
RAN_MARKER="${RUNNER_TEMP:-${GITHUB_WORKSPACE:-/tmp}}/h10-classifier-started-${GITHUB_RUN_ID:-norunid}-${GITHUB_RUN_ATTEMPT:-1}"
|
|
: > "$RAN_MARKER"
|
|
|
|
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"
|
|
# THE SECOND SENTINEL, and the two are NOT interchangeable (ersatztv#849). `REPAIR_DESC`
|
|
# asserts a fact — a human verdict existed and this job's write buried it. Most of the
|
|
# paths that must withhold an exemption cannot assert that: they are the ones where the
|
|
# status history could not be READ, so what happened in the write window is unknown. Using
|
|
# `REPAIR_DESC` for those made the job state something it had not established, which is why
|
|
# its own `::error::` had to be softened to "no verdict was necessarily overwritten" — a
|
|
# message contradicting the description beside it.
|
|
#
|
|
# STICKY, exactly like `REPAIR_DESC`: `read_existing_verdict` recognises it and the
|
|
# classification refuses to grant an exemption over it. That is the property #742's
|
|
# withdrawn attempt lacked — it wrote a GENERIC `pending`, which is precisely what a later
|
|
# run re-derives into `success`, so it moved which run posted the forged green rather than
|
|
# stopping it.
|
|
#
|
|
# UNLIKE `REPAIR_DESC` IT IS RECONCILABLE, and that is the whole reason for a second state
|
|
# rather than reusing the first. `REPAIR_DESC` can only be cleared by a human re-posting,
|
|
# because the fact it records (a verdict was lost) stays true forever. "I could not read
|
|
# the history" does not: a later run that CAN read it either finds the buried verdict — in
|
|
# which case this upgrades to `REPAIR_DESC` — or finds none, in which case the uncertainty
|
|
# is genuinely resolved and the sentinel is cleared. That reconciliation is what makes the
|
|
# stall bounded instead of permanent (see RECONCILE below), and it is sound because
|
|
# `/statuses/{sha}` returns one row per POST: a verdict this job masked on the COMBINED
|
|
# endpoint is still in the per-POST history for a later run to find.
|
|
# "STATUS", NOT "EXEMPTION". This is written on paths that grant no exemption at all — an
|
|
# ordinary non-exempt `pending` with no mark, a page-2 read refusal on a head that carries
|
|
# nothing, an unreadable combined read — and the description is the operator-facing text of
|
|
# a branch-protection-required check. Telling someone whose plain unreviewed PR is waiting
|
|
# that an "exemption write" could not be verified names a thing that never happened.
|
|
#
|
|
# RENAMING IS A ONE-RUN TRANSITION, not a breaking change: a run that reads a head carrying
|
|
# the old text does not recognise it as a sentinel, re-derives it, and writes the new one.
|
|
# Nothing in production carries either string yet, so the transition is free here.
|
|
UNVERIFIED_DESC="Status write could not be verified — re-post the verdict"
|
|
# THE ONE `raced_why` VALUE THAT IS ALSO A PREDICATE. The others are prose the log prints;
|
|
# this one is compared twice, so a drift in either copy silently downgrades the human
|
|
# `::error::` — the one message that tells a reviewer their verdict was buried — to the
|
|
# generic "could not verify" text. Named for the same reason the two descriptions beside it
|
|
# are named.
|
|
RACED_BY_HUMAN="a human verdict landed in the write window"
|
|
# WHOSE verdict may be INHERITED (ersatztv#742). Space-separated logins, compared exactly.
|
|
#
|
|
# The test this replaces was "the status has a non-null `.creator.login`", which only proves
|
|
# it was POSTed with SOME user credential. This repo stores credentials for accounts that are
|
|
# not reviewers: `RENOVATE_TOKEN` is a `write:repository` PAT of the `renovate` bot, it
|
|
# CANNOT be scoped down the way `#697` scoped the registry credential (Renovate genuinely
|
|
# needs repo write to push branches and open PRs), and secrets are a per-repo STORE rather
|
|
# than a per-workflow grant — so a status POSTed with it reads back `creator: renovate`,
|
|
# non-null, and would be inherited as a human verdict.
|
|
#
|
|
# CONDITIONAL VOICE ON PURPOSE. The asymmetry it rests on IS measured (a user credential
|
|
# posts with a real creator, an Actions job with `creator: null`), but it has not been
|
|
# measured FOR THIS TOKEN: every status row on all six Renovate-authored PR heads in this
|
|
# repo carries `creator: null` — `renovate` has never posted a commit status here. So the
|
|
# route is a sound inference from a measurement, not an observed event, and an earlier
|
|
# draft's past tense ("was inherited") described something nobody saw.
|
|
#
|
|
# Membership keeps the POSITIVE direction the description test already uses: short-circuit
|
|
# only on something identified as a REVIEWER's verdict; every other creator, recognised or
|
|
# not, is RE-DERIVED, exactly as an Actions-token status already is.
|
|
#
|
|
# A LITERAL in this base-resolved definition, deliberately — not a repo `vars:` entry and not
|
|
# a secret. Those are settable without a PR, i.e. without this gate; editing this file takes
|
|
# a PR to `main`, and `.gitea/` is on the PROTECTED path list below, so the change cannot
|
|
# ride a docs-only exemption either.
|
|
#
|
|
# NO GLOB METACHARACTERS: the membership loop below word-splits this value, so pathname
|
|
# expansion would apply to an entry containing `*`, `?` or `[`. Machine-checked by
|
|
# `test_pr_changed_files.py::test_the_H10_REVIEWERS_list_is_glob_free_and_non_empty`.
|
|
#
|
|
# What this is NOT: authentication. An allow-listed collaborator's own write-scoped token can
|
|
# still POST a verdict they did not form, and branch protection binds the CONTEXT name, not
|
|
# its issuer. It removes the accounts that are not reviewers at all — provenance, not
|
|
# identity proof (`ci.exemption-provenance`).
|
|
H10_REVIEWERS="timothy"
|
|
# 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.
|
|
# `CLAUDE.md` and `AGENTS.md` are listed even though they are `.md` files, because they are
|
|
# not prose about the project — they are the documents that DEFINE the completion protocol,
|
|
# the merge-consent convention and the H10 rule itself. `.claude/` being protected while the
|
|
# file that specifies what `.claude/` enforces was docs-only-exempt is the same
|
|
# self-exemption the header rules out, one directory over. Found by cold review
|
|
# (ersatztv#751): driving the real classify body with a lone `CLAUDE.md` change produced
|
|
# `review-verdict/h10=success`, "Exempt: docs-only change (no code, no protected path)".
|
|
#
|
|
# It is fixed HERE rather than deferred because this change is what makes it reachable
|
|
# again: no exemption `success` was writable at all while the classify step was dropped, so
|
|
# restoring the exemptions restores this hole with them. `README.md` is deliberately NOT
|
|
# listed — it is ordinary prose and carries no enforcement.
|
|
PROTECTED='^(\.claude/|\.codex/|\.gitea/|\.husky/|scripts/|docker/ci/|CLAUDE\.md$|AGENTS\.md$)'
|
|
# 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" "$@"; }
|
|
|
|
# WRITE A STICKY `pending` SENTINEL, retrying once. Factored out because three call sites
|
|
# now need it (ersatztv#849) and each one runs at a moment where a silent failure is the
|
|
# worst outcome: an unreadable combined read, the post-write repair, and the post-POST
|
|
# retarget re-check. A failure returns non-zero rather than exiting, so every caller can
|
|
# say what was left standing — `set -e` would otherwise kill the step mid-repair, after a
|
|
# green was written and with nothing left to re-attempt.
|
|
#
|
|
# `>/dev/null 2>&1` on the POST: `gh` is `curl -sf`, whose stderr on an HTTP error is
|
|
# noise, and the caller's own message is the diagnostic.
|
|
repair_status_to() { # $1 = the sentinel description to write
|
|
local body
|
|
body=$(jq -n --arg c "$CONTEXT" --arg u "$PR_URL" --arg d "$1" \
|
|
'{state:"pending", context:$c, description:$d, target_url:$u}')
|
|
if gh -X POST -H 'Content-Type: application/json' -d "$body" \
|
|
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
|
|
return 0
|
|
fi
|
|
if gh -X POST -H 'Content-Type: application/json' -d "$body" \
|
|
"$BASE_URL/repos/$REPO/statuses/$SHA" >/dev/null 2>&1; then
|
|
return 0
|
|
fi
|
|
return 1
|
|
}
|
|
|
|
# EVERY "this job cannot establish what is on the head" EXIT GOES THROUGH HERE
|
|
# (ersatztv#849). Declining to write protects a real verdict and leaves a FORGED one — an
|
|
# off-list `success` is the row #742 exists to revoke, revocation happens by re-deriving
|
|
# it, and an unreadable read is the one thing that stops it while this job goes red on a
|
|
# status branch protection does not read.
|
|
#
|
|
# Factored rather than repeated because the four page-2 refusals below used to be the
|
|
# exception, on the reasoning that the probe "fires when NO row for this context was on
|
|
# page 1, so there is no green of any provenance to leave standing". That is
|
|
# self-contradictory: the ONLY reason page 2 is read at all is that the row MAY be beyond
|
|
# page 1, which the probe's own message says. Whatever a future reader concludes about
|
|
# reachability, the two branches must not resolve the same uncertainty in opposite
|
|
# directions.
|
|
#
|
|
# ACCEPTED COST, stated rather than discovered later: a head carrying more CONTEXTS than
|
|
# the server-wide page cap (measured 50) stalls on every run, because the probe refuses
|
|
# every time. Measured 2026-08-29, this repo puts 8 contexts on a `main` head. That case
|
|
# already stalled before this change — with an ABSENT required check, which reads as "not
|
|
# reviewed yet" — so what changes is that the stall now says why.
|
|
# THE RETURN VALUE IS THE WHOLE POINT, and the first version did not have one: the `else`
|
|
# branch ended with a successful `echo`, so the function returned 0 after BOTH POSTs failed
|
|
# and its fence caller took the `exit 0` beside it as though the head had been marked. An
|
|
# explicit `return` per arm, and every caller acts on it.
|
|
replace_unknown_state() { # $1 = the ::error:: naming what could not be established
|
|
echo "::error::$1"
|
|
if repair_status_to "$UNVERIFIED_DESC"; then
|
|
echo "::error::Replaced ${CONTEXT} on ${SHA:0:7} with the unverified-write sentinel rather than leaving the status it carried standing. A later run will either clear it, or — if it finds a verdict underneath — ask you to re-post that verdict. To settle it now: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
return 0
|
|
fi
|
|
echo "::error::COULD NOT WRITE THE UNVERIFIED SENTINEL to ${SHA:0:7}. Whatever ${CONTEXT} this head carries is standing unread. Post a verdict immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
return 1
|
|
}
|
|
# WRITING THE SENTINEL AND FAILING THE JOB ARE SEPARATE DECISIONS, and only the read paths
|
|
# take both. `read_existing_verdict`'s refusals already exited non-zero on `main`, so
|
|
# keeping them red changes nothing about how often this job is red. The FENCE's untrusted
|
|
# branch exited 0 there — an unreadable timeline is an ordinary API hiccup, and turning
|
|
# every one of them into a red run is the noise this file elsewhere refuses to add. It
|
|
# calls `replace_unknown_state` and returns cleanly.
|
|
replace_unknown_and_die() {
|
|
# The write may itself fail; either way this path exits non-zero, so the return value is
|
|
# deliberately not branched on here — it is `replace_unknown_state` that reports which
|
|
# happened, and the two messages differ.
|
|
replace_unknown_state "$1" || true
|
|
exit 1
|
|
}
|
|
|
|
# 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_repair and the
|
|
# two provenance flags ex_attributable / ex_human (see the flag split below, ersatztv#742).
|
|
# 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". Until ersatztv#849 the job simply died WITHOUT posting; it now REPLACES the
|
|
# unknown state with the sticky sentinel first, because declining to write protects a real
|
|
# verdict and leaves a FORGED one — and this job's own red status is not a required check.
|
|
#
|
|
# 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 rv try
|
|
# `.statuses` IS `null`, NOT `[]`, ON A HEAD WITH NO STATUSES YET — the same nil-slice
|
|
# serialization as the timeline terminator, found by cold review of the fix for that one
|
|
# (ersatztv#751). Measured on this instance: PR #739's head 5fa672e2 returns
|
|
# `{"state":"pending","total_count":0,"statuses":null}`. An `array`-only gate read that as
|
|
# unreadable and took the `exit 1` below, so the job posted NOTHING — fail-closed, but the
|
|
# user-visible outcome is exactly the one this issue is about: an exempt PR left with no
|
|
# status and, since ersatztv#743, no bypass. Fixing only the timeline site would have left
|
|
# the identical bug one function away.
|
|
#
|
|
# `total_count` is required to agree, so `null` is accepted only as the empty case and not
|
|
# as a body that merely lost its array. Anything else is still "cannot tell" and still
|
|
# refuses to post, which is what protects an existing verdict from a transient API error.
|
|
# WHETHER PAGE 1 IS THE WHOLE LIST CANNOT BE DECIDED FROM PAGE 1 (ersatztv#751). Two
|
|
# plausible guards were tried and both are no-ops HERE, which is why this ended up as an
|
|
# extra round-trip instead of an arithmetic test:
|
|
#
|
|
# * `.statuses | length` vs `.total_count` — `total_count` is the count for the PAGE
|
|
# RETURNED, not for the commit. Measured at 1.27.1 on 3aed43c6 (6 contexts):
|
|
# `?limit=1` gives `len=1, total_count=1`, `?limit=3` gives `len=3, total_count=3`.
|
|
# Equal by construction, so the check proves nothing.
|
|
# * "refuse when the page came back FULL at the requested limit of 100" — this instance
|
|
# caps `limit` at the server-wide `MAX_RESPONSE_ITEMS`, measured at 50
|
|
# (`/issues?limit=100` returns 50). A response can therefore never contain 100 rows and
|
|
# the comparison was DEAD CODE. The repo already knew this and said so in
|
|
# `scripts/pr-changed-files.sh`, two test files and `ci.script-tests-job`; the guard was
|
|
# written anyway. Hardcoding the cap instead would re-break the day it is reconfigured.
|
|
#
|
|
# So ask the server, and only when it matters. Completeness is needed ONLY to justify the
|
|
# conclusion "no verdict exists on this head" — if the row IS on page 1 there is nothing
|
|
# further to learn, since this endpoint returns the latest status per CONTEXT and the same
|
|
# context cannot recur on a later page. When the row is absent, page 2 is read: any rows
|
|
# there mean the list is longer than one page and the verdict could be sitting beyond it,
|
|
# so this refuses rather than concluding absence. Cap-independent by construction.
|
|
# RETRIED ONCE (ersatztv#849 route 5), for the same reason `page_statuses` retries and
|
|
# with the same honesty about what that buys: a second immediate request absorbs a
|
|
# momentary blip, not an outage. It matters more here than it did before, because the
|
|
# branch below no longer merely declines to write — it now REPLACES the head's status,
|
|
# so a transient failure that used to cost nothing would cost a reviewed PR its verdict.
|
|
# The retry is what keeps that cost attached to a persistent failure only.
|
|
#
|
|
# The loop re-derives the validity triple on each attempt rather than only the body:
|
|
# a 502 HTML page and a schema-corrupt JSON body are both "cannot tell", and retrying
|
|
# one but not the other would make the direction depend on how the failure arrived.
|
|
json=""
|
|
st_kind=""
|
|
st_total="x"
|
|
st_ok=no
|
|
for try in 1 2; do
|
|
json=$(gh "$BASE_URL/repos/$REPO/commits/$SHA/status?limit=100") || json=""
|
|
st_kind=$(printf '%s' "$json" | jq -r '.statuses | type' 2>/dev/null) || st_kind=""
|
|
# NUMBER, not `jq -r` text: `jq -r` renders the JSON number 0 and the JSON string "0"
|
|
# identically, so a schema-corrupted `"total_count": "0"` would satisfy a string compare
|
|
# (cold review reproduced this). Requiring the type as well pins the accept path to a real
|
|
# numeric zero.
|
|
st_total=$(printf '%s' "$json" | jq -r 'if (.total_count | type) == "number" then (.total_count | tostring) else "x" end' 2>/dev/null) || st_total="x"
|
|
st_ok=no
|
|
case "$st_kind" in
|
|
array) st_ok=yes ;;
|
|
null) if [ "$st_total" = "0" ]; then st_ok=yes; fi ;;
|
|
esac
|
|
if [ -n "${json//[[:space:]]/}" ] && [ "$st_ok" = yes ]; then break; fi
|
|
# Only BETWEEN attempts — sleeping after the last one delays a failure nobody is
|
|
# waiting on.
|
|
if [ "$try" -eq 1 ]; then sleep 1; fi
|
|
done
|
|
if [ -z "${json//[[:space:]]/}" ] || [ "$st_ok" != yes ]; then
|
|
# REPLACE THE UNKNOWN STATE, DO NOT JUST DECLINE TO WRITE (ersatztv#849 route 5).
|
|
#
|
|
# "Refusing to post anything" protects a verdict this job cannot see — which is right
|
|
# when the head carries a REAL verdict, and exactly wrong when it carries a FORGED one.
|
|
# An off-list credential that has already POSTed `review-verdict/h10=success` is the
|
|
# status #742 exists to revoke; revocation happens by re-deriving the row, which is the
|
|
# one thing an unreadable read stops. The job then went red — and its own job status is
|
|
# NOT a required check, so branch protection still saw the green. Uncertainty resolved
|
|
# toward SUCCESS, on the single path where that is never acceptable.
|
|
#
|
|
# So the unknown state is durably replaced with the sticky unverified sentinel. This is
|
|
# NOT the same trade as overwriting a verdict blindly: `/statuses/{sha}` keeps one row
|
|
# per POST, so a genuine verdict masked here is still in the history, and the
|
|
# RECONCILE step on the next run finds it and upgrades to the repair sentinel — which
|
|
# tells the reviewer to re-post rather than silently un-approving them.
|
|
#
|
|
# THE COST, stated rather than implied: a persistent combined-endpoint failure on a
|
|
# genuinely approved PR costs that head its verdict until a human re-posts it. That is
|
|
# a stall, clearable in one command, and it is the direction this whole job resolves
|
|
# uncertainty in everywhere else.
|
|
#
|
|
replace_unknown_and_die "Could not read existing commit statuses for ${SHA:0:7} (.statuses was '${st_kind:-unparseable}', total_count '${st_total}') after a retry, so any ${CONTEXT} already on this head — including one posted by an account this gate does not accept verdicts from — can neither be read nor re-derived."
|
|
fi
|
|
# `// []` so the null case cannot hard-error here under `set -e` once it is accepted above.
|
|
# TYPE-SAFE, AND A SCHEMA FAILURE REPLACES RATHER THAN DYING (ersatztv#849 round 3).
|
|
# `.statuses` being an array was checked; its ELEMENTS were not. A single scalar in that
|
|
# array makes `select(.context == ...)` hard-error, jq exits 5, and under `set -e` this
|
|
# unguarded assignment took the step down — BEFORE any of the replacement paths below,
|
|
# so a `success` already on the head stayed authoritative while the job merely went red
|
|
# on a status branch protection does not read. Same rule the post-write filter already
|
|
# applies to its own rows: drop what cannot be read, then judge what is left.
|
|
row=$(printf '%s' "$json" | jq -r --arg c "$CONTEXT" '[(.statuses // [])[] | select(type == "object") | select(.context? == $c)] | first // {}') || row=""
|
|
if [ -z "${row//[[:space:]]/}" ]; then
|
|
replace_unknown_and_die "The commit statuses for ${SHA:0:7} parsed as an array but could not be read row by row, so any ${CONTEXT} on this head can neither be read nor re-derived."
|
|
fi
|
|
# NO UNREADABLE ELEMENT MAY BE DROPPED SILENTLY, whether or not a target row was found.
|
|
# `select(type == "object")` above exists so a malformed element cannot CRASH the read —
|
|
# that part is right, and it is all it does. What it must not become is a licence to
|
|
# judge the head over the rows that happened to survive it.
|
|
#
|
|
# Two ways that went wrong, both measured. When the filter drops EVERY element,
|
|
# `first // {}` yields `{}`, every `ex_*` reads empty, and the job concludes NO VERDICT
|
|
# EXISTS, so a docs-only PR walks to the exemption. And when it drops one element beside
|
|
# a row it DID read, that element still cannot be shown to be a different context — so it
|
|
# may be a mangled rendering of this head's own rejection, and the one-row-per-context
|
|
# invariant that would rule that out is exactly what a schema-corrupt response has
|
|
# already broken. `origin/main` raised jq error 5 on such a body and `set -e` aborted
|
|
# before any POST; both shapes were inputs on which this job posted a green that `main`
|
|
# did not.
|
|
#
|
|
# So the rule is unconditional: any unreadable element, or a failure to count them,
|
|
# refuses. The cost is a stall on a head the registry serves badly — the correct
|
|
# direction for a required check, which must withhold a green rather than grant one.
|
|
unreadable=$(printf '%s' "$json" | jq -r '[(.statuses // [])[] | select(type != "object" or ((.context | type) != "string"))] | length' 2>/dev/null) || unreadable=""
|
|
case "$unreadable" in ''|*[!0-9]*) unreadable=1 ;; esac
|
|
# ANY UNREADABLE ELEMENT, whether or not a target row was also found. Round 8 scoped this
|
|
# to "no readable row was found", on the reasoning that a malformed row BESIDE one we did
|
|
# read is noise. That reasoning does not hold: an element whose `.context` cannot be read
|
|
# cannot be shown to be a different context, so it may be a mangled rendering of this
|
|
# head's own rejection — and the one-row-per-context invariant, which is the only thing
|
|
# that would rule it out, is exactly what a schema-corrupt response has already broken.
|
|
#
|
|
# The branch's own positive-control fixture encoded the failing case: a scalar beside an
|
|
# off-list `success`. It re-derived the off-list row and POSTed an exemption, where
|
|
# `origin/main` errored on the scalar and posted nothing.
|
|
#
|
|
# A failure of the count itself is treated the same way — `unreadable=1` above — because
|
|
# "I could not count what I could not read" is the same answer.
|
|
#
|
|
# THE COST IS A STALL on any head carrying a malformed element, and that is the correct
|
|
# direction for a required check: it withholds a green rather than granting one.
|
|
if [ "$unreadable" -gt 0 ]; then
|
|
replace_unknown_and_die "${unreadable} element(s) of the commit statuses for ${SHA:0:7} could not be read, and an element whose context is unreadable cannot be shown to be a different context — so neither 'no verdict exists' nor 'the row I found is the only one' is a conclusion this job may draw."
|
|
fi
|
|
# THE COMPLETENESS PROBE, run only when page 1 shows no verdict — see the note above. An
|
|
# unreadable or unexpected page 2 is treated as "cannot tell" and refuses, the same
|
|
# direction as every other unreadable case here: concluding "no verdict exists" is what
|
|
# licenses posting an exemption over one, so it is the conclusion that must be earned.
|
|
if [ "$(printf '%s' "$row" | jq -r '.context // ""')" = "" ]; then
|
|
more=$(gh "$BASE_URL/repos/$REPO/commits/$SHA/status?limit=100&page=2") || more=""
|
|
if [ -z "${more//[[:space:]]/}" ]; then
|
|
replace_unknown_and_die "Could not read page 2 of the commit statuses for ${SHA:0:7}, so 'no verdict exists' cannot be established."
|
|
fi
|
|
more_kind=$(printf '%s' "$more" | jq -r '.statuses | type' 2>/dev/null) || more_kind=""
|
|
more_len=$(printf '%s' "$more" | jq -r '(.statuses // []) | length' 2>/dev/null) || more_len=""
|
|
case "$more_kind" in
|
|
null) ;;
|
|
array)
|
|
case "$more_len" in
|
|
''|*[!0-9]*)
|
|
replace_unknown_and_die "Page 2 of the commit statuses for ${SHA:0:7} had a non-numeric length, so it cannot be concluded that no verdict exists." ;;
|
|
0) ;;
|
|
*)
|
|
replace_unknown_and_die "${CONTEXT} was not on page 1 of the statuses for ${SHA:0:7}, but page 2 carries ${more_len} more row(s) — the list is longer than one page, so a verdict of ANY provenance may be sitting beyond it where this job cannot read it." ;;
|
|
esac ;;
|
|
*)
|
|
replace_unknown_and_die "Page 2 of the commit statuses for ${SHA:0:7} was '${more_kind:-unparseable}', so it cannot be concluded that no verdict exists." ;;
|
|
esac
|
|
fi
|
|
# THE ROW ID, so "did this row change" can be asked about IDENTITY rather than about
|
|
# TEXT (ersatztv#849). Two sentinel POSTs are byte-identical by design — that is what
|
|
# makes them fixed points — so a mid-run REPLACEMENT of one sentinel by another is
|
|
# invisible to a state/creator/description comparison, and the run then overwrites a row
|
|
# another run had just written. The same blind spot applies to any replacement whose
|
|
# triple happens to match.
|
|
#
|
|
# MEASURED 2026-08-29 on this instance (Gitea 1.27.1), because the rest of this job reads
|
|
# ids only from `/statuses/{sha}`: the COMBINED endpoint's rows carry `id` too — head
|
|
# 736649b3 returned 8 rows keyed id/context/status/creator/description/created_at/
|
|
# updated_at/url, ids 14..30 ascending. `// ""` so a server that ever stopped sending it
|
|
# degrades to the pre-existing text comparison rather than to a false "changed".
|
|
# A WRONG TYPE IS NOT AN ABSENT VALUE (ersatztv#849 round 4). Round 3 type-tested these
|
|
# four fields and resolved a failure to `""` — which for `.creator` means "no creator",
|
|
# i.e. unattributable, i.e. RE-DERIVE. That turned a corrupt row into an exemption
|
|
# `success` where `origin/main` had died before writing anything: measured, a head
|
|
# carrying `h10=failure` with `"creator": 7` posts `Exempt: docs-only change` here and
|
|
# posted NOTHING on main. The rationale was transplanted from #763, whose site is the
|
|
# POST-WRITE filter — there, dying leaves a green already published, so dropping the row
|
|
# is the safe direction; HERE the alternative is dying BEFORE any write, which is
|
|
# fail-closed. The deferral rationale did not transfer.
|
|
#
|
|
# So the three outcomes are kept distinct: a value of the right type, a legitimately
|
|
# ABSENT one (`null` — an unset `.description`, the machine `creator`, and every field of
|
|
# the `{}` no-verdict row), and a type the schema does not allow, which is unknown state
|
|
# and takes the same route an unreadable ELEMENT already takes two lines up.
|
|
#
|
|
# SOH (0x01), and the byte matters: `$'\000…'` is the EMPTY STRING in bash, so
|
|
# "correcting" this to NUL would make every legitimately-absent field compare equal to the
|
|
# token and send every clean head down the fail-closed route — the gate would stall every
|
|
# PR. A control character is what makes collision impossible; NUL is the one that cannot
|
|
# be used.
|
|
SCHEMA_FAULT=$'\001schema-fault'
|
|
ex_id=$(printf '%s' "$row" | jq -r --arg f "$SCHEMA_FAULT" 'if (.id | type) == "number" then (.id | tostring) elif (.id | type) == "null" then "" else $f end')
|
|
ex_state=$(printf '%s' "$row" | jq -r --arg f "$SCHEMA_FAULT" 'if (.status | type) == "string" then .status elif (.status | type) == "null" then "" else $f end')
|
|
# `.creator.login` HARD-ERRORS on any non-object creator — the same defect #763 fixed in
|
|
# the post-write filter, still live on this read. `(.creator | type)` short-circuits it,
|
|
# so the step is not killed; the wrong TYPE then takes the fault route above rather than
|
|
# reading as "no creator", which is what made it re-derivable. (An earlier version of
|
|
# this comment described that re-derivation as the intended behaviour — it was the
|
|
# regression the block above records.)
|
|
ex_creator=$(printf '%s' "$row" | jq -r --arg f "$SCHEMA_FAULT" 'if (.creator | type) == "object" then (.creator.login // "") elif (.creator | type) == "null" then "" else $f end')
|
|
ex_desc=$(printf '%s' "$row" | jq -r --arg f "$SCHEMA_FAULT" 'if (.description | type) == "string" then .description elif (.description | type) == "null" then "" else $f end')
|
|
for _f in "$ex_id" "$ex_state" "$ex_creator" "$ex_desc"; do
|
|
if [ "$_f" = "$SCHEMA_FAULT" ]; then
|
|
replace_unknown_and_die "A field of the ${CONTEXT} row on ${SHA:0:7} carries a type the schema does not allow, so this row can neither be read nor judged — and reading it as an absent value would make it unattributable, which is a licence to re-derive it."
|
|
fi
|
|
done
|
|
# 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.
|
|
# TWO FLAGS, BECAUSE THIS FUNCTION ANSWERS TWO DIFFERENT QUESTIONS (ersatztv#742).
|
|
# It is called TWICE, and the callers want opposite widths:
|
|
#
|
|
# ex_attributable — "is this an attributable verdict for THIS base, whoever wrote it?"
|
|
# Used by the LAST-MOMENT RE-READ, which asks "did a reviewer post a
|
|
# verdict while we were classifying". Narrowing THAT does not make it
|
|
# stricter, it makes the job stop abstaining and post its exemption
|
|
# success straight over the row.
|
|
# ex_human — ex_attributable AND the creator is in `$H10_REVIEWERS`. Used by the
|
|
# INHERITANCE short-circuit, where narrowing is safe because an
|
|
# unrecognised account falls through to re-derivation.
|
|
#
|
|
# GATING ONE FLAG NARROWS BOTH SITES, which is the mistake this split exists to prevent,
|
|
# and the post-write repair does NOT cover it: that repair is skipped entirely whenever
|
|
# the high-water mark could not be established (`max_id_before=-1`). Pinned by
|
|
# `test_the_MID_RUN_abstain_still_fires_for_a_NON_allowlisted_creator`.
|
|
ex_human=no
|
|
ex_attributable=no
|
|
ex_repair=no
|
|
ex_unverified=no
|
|
# A SENTINEL IS A `pending` STATE WEARING THAT DESCRIPTION, never the text alone. Both
|
|
# flags were set from the description by itself, which is the same mistake the removed
|
|
# "this job's own output" exclusion was removed FOR — a description is not provenance,
|
|
# and it is not state either. A `success` carrying `$REPAIR_DESC` verbatim, from a machine
|
|
# or off-list account, therefore read as a sentinel: the mid-run guard exited on it and
|
|
# the mark's already-there test matched it, so a green stood on an unreviewed head with
|
|
# no successor guaranteed. Both sentinels this job writes are `pending` by construction,
|
|
# so requiring it costs nothing and closes the impersonation.
|
|
case "$ex_desc" in
|
|
"$REPAIR_DESC"*) if [ "$ex_state" = pending ]; then ex_repair=yes; fi ;;
|
|
esac
|
|
# THE TWO SENTINELS ARE TESTED SEPARATELY, never as one "is it a sentinel" flag. They
|
|
# carry different facts and clear by different means (see UNVERIFIED_DESC above), and a
|
|
# single flag would let the reconciliation below clear the one that must never be cleared
|
|
# by anything but a human.
|
|
case "$ex_desc" in
|
|
"$UNVERIFIED_DESC"*) if [ "$ex_state" = pending ]; then ex_unverified=yes; fi ;;
|
|
esac
|
|
case "$ex_desc" in
|
|
"Review-verdict:"*)
|
|
# ATTRIBUTABLE, not yet "inheritable" — the allow-list is applied below, after the
|
|
# base check, so the two flags cannot diverge on the base.
|
|
if [ -n "$ex_creator" ]; then ex_attributable=yes; fi
|
|
;;
|
|
esac
|
|
if [ "$ex_attributable" = 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_attributable=no
|
|
echo "${CONTEXT} on ${SHA:0:7} is an attributable 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
|
|
# DERIVED LAST, so the base check above governs both flags and cannot be bypassed by
|
|
# membership.
|
|
#
|
|
# THE INVARIANT IS THE QUOTING, NOT THE BRACKET FORM. The tempting rule — "never
|
|
# `[[ == ]]`, it makes the RHS a PATTERN" — is FALSE as usually written: probed on bash
|
|
# 5.3, `[[ "$rv" == "$ex_creator" ]]` with a QUOTED right-hand side is a literal
|
|
# comparison and does NOT match `ex_creator='*'`. Only an UNQUOTED RHS globs. So the rule
|
|
# is: keep both sides quoted. POSIX `[ = ]` is used because it cannot glob at all.
|
|
#
|
|
# Never `grep -xF` either: it would read a newline-bearing `$ex_creator` as SEVERAL
|
|
# patterns, any one of which could match an allow-listed line.
|
|
if [ "$ex_attributable" = yes ]; then
|
|
for rv in $H10_REVIEWERS; do
|
|
if [ "$rv" = "$ex_creator" ]; then ex_human=yes; break; fi
|
|
done
|
|
fi
|
|
}
|
|
|
|
# --- The PR-mutation fence (ersatztv#706 race 1; head axis ersatztv#803/#664) --------
|
|
# THE PROBLEM THIS NARROWS — not solves, and the difference is #849's. 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 a `concurrency:` key grouping by PR number and
|
|
# `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.verdict-write-retarget-fence`; this
|
|
# cited `ci.paged-endpoint-completeness` until 2026-08-06, a key that has never existed as
|
|
# a record — resolve decisions through the catalog, never through a key or path quoted in a
|
|
# comment). The count is trusted ONLY when the walk read EVERY page up to its 20-page cap
|
|
# AND the LAST page came back empty (ersatztv#870 — reaching an empty page earlier is no
|
|
# longer sufficient, and no longer ends the walk). A page that is unreadable, a body that
|
|
# is neither an array NOR `null` (a bare `null` is this endpoint's empty page and is
|
|
# accepted), a non-numeric length, an empty FIRST page, or a cap reached on a NON-empty page all
|
|
# leave `rt_ok=no`, and an untrusted count is treated below as "cannot tell" rather than as
|
|
# zero. A SHORT page is not in that list and never was: it is tallied and the walk carries
|
|
# on — the sentence here said otherwise until 2026-08-30.
|
|
# THE SAME WALK CARRIES A SECOND AXIS: HEAD MUTATION (ersatztv#803/#664).
|
|
#
|
|
# The base fence above closes `main -> S -> main`. The HEAD aliases the same way and was
|
|
# fenced by nothing: a force-push `H1 -> H2 -> H1` during `pr-changed-files.sh`'s paging
|
|
# leaves its final `.head.sha` comparison equal while the middle pages were enumerated
|
|
# against `H2`, so a mixed file list can produce a docs-only exemption no single head ever
|
|
# justified. That script cannot close it alone — the binding it performs is a post-hoc
|
|
# comparison of one value against itself, which is the definition of ABA-vulnerable — and
|
|
# pinning the diff to two shas is still not available: re-measured on THIS instance at
|
|
# Gitea 1.27.1 on 2026-08-28, `GET /compare/{base}...{head}` returns `total_commits` and
|
|
# `commits` and NO `files` key (probed over a 12-commit range). So the fix has to be a
|
|
# caller-side monotonic counter, exactly as it was for the base.
|
|
#
|
|
# `pull_push` IS THAT COUNTER, and it is the same shape of key as `change_target_branch`:
|
|
# the timeline records EVERY push to the PR branch as one `pull_push` event whose body is
|
|
# `{"is_force_push": bool, "commit_ids": [...]}`. Measured on this instance at 1.27.1
|
|
# (2026-08-28): PR #802 carries eighteen, all `is_force_push: false`; PR #761 carries five,
|
|
# four of them `true`. An `H1 -> H2 -> H1` round trip is necessarily TWO pushes, so the
|
|
# count moves by two where the sha moves by zero. The shape is not hypothetical — PR #761
|
|
# really did go `8798a1d -> 830a407 -> 8798a1d` (26 minutes apart, so no enumeration
|
|
# straddled it, but the ALIAS is what the sha check cannot see whatever the interval).
|
|
#
|
|
# WHY THE COUNT AND NOT `is_force_push`. Filtering to force-pushes only would be the
|
|
# tempting narrowing and it is wrong twice over: an ordinary non-force push also invalidates
|
|
# a mid-flight enumeration (it moves the head, and `pr-changed-files.sh`'s own one-way check
|
|
# exists precisely for that), and a `H1 -> H2 -> H1` restoration can be performed with the
|
|
# SECOND push non-forced when `H1` is an ancestor. Every push is counted; the boolean is
|
|
# deliberately not read, and the body is never parsed — a counter that had to parse
|
|
# `commit_ids` would fail closed on any body shape change, which is a worse trade than
|
|
# counting rows by `.type` alone.
|
|
#
|
|
# WHY THIS DOES NOT ABSTAIN ON ITS OWN TRIGGER — the objection that would make this a
|
|
# permanent stall rather than a fence. A `synchronize` run is caused by a push, and if that
|
|
# push's timeline event were created AFTER the run started, `count_before` would miss it,
|
|
# `count_after` would see it, and every such run would abstain forever.
|
|
#
|
|
# It is created first, and that is an ORDERING PROPERTY OF THE SOURCE rather than a lucky
|
|
# margin: in Gitea v1.27.1 the push comment is written BEFORE the synchronize notification
|
|
# is emitted, so a run always sees its own causative event, and retries and re-runs add no
|
|
# new event. The measurement corroborates rather than carries the claim — across 20
|
|
# triggered (push -> `pull_request_target` run) pairs on PRs #802, #834 and #761 the
|
|
# `pull_push` event predates its own run's `started_at` by 26-102s, itself a lower bound
|
|
# since the job runs a checkout and several steps before counting (69s end to end on run
|
|
# 2385). An earlier version of this comment rested the claim on that margin alone and
|
|
# called it a deployment property; see `ci.verdict-write-retarget-fence`, which also names
|
|
# the one window this does NOT cover — a PR's FIRST push is recorded about a second after
|
|
# creation and fires no `synchronize`, so an `opened` run landing inside it would abstain
|
|
# with no successor queued.
|
|
#
|
|
# The termination argument transfers from the base axis with ONE exception, which is the
|
|
# window named just above and must not be dropped here: a push to an ALREADY-OPEN PR fires
|
|
# `synchronize`, which is in this workflow's `types:` (see the header), so the very event
|
|
# that makes this run abstain has already queued a successor and abstention hands off. A
|
|
# PR's FIRST push does not — it is recorded about a second after creation and fires no
|
|
# `synchronize` — so an `opened` run abstaining on it has no successor coming. "Transfers
|
|
# unchanged" was the wording here until 2026-08-28 and was the overclaim, regenerated one
|
|
# paragraph below the fix that removed it.
|
|
#
|
|
# ONE WALK, TWO COUNTERS, ONE TRUST FLAG. The two axes share `rt_ok` because they share the
|
|
# pages it certifies: a walk that did not read to its cap and finish on an empty page has
|
|
# not established EITHER count, and giving them separate flags would invite a future edit to
|
|
# trust one over pages that did not certify the other.
|
|
count_pr_mutations() {
|
|
rt_count=0
|
|
hp_count=0
|
|
rt_ok=no
|
|
local page=1 raw n m p bad kind try total=0 ptotal=0 empty=no
|
|
while [ "$page" -le 20 ]; do
|
|
# BOUNDED AND RETRIED ONCE, because this walk got 10x longer (ersatztv#870 review).
|
|
# Both halves answer the same amplification and neither is decoration.
|
|
#
|
|
# BOUNDED: `gh` is bare `curl -sf` with no timeout of its own, and the THIRD call site
|
|
# runs AFTER the exemption `success` is already posted — a hang there leaves the green
|
|
# standing with no repair attempted. That exposure predates this change, but going
|
|
# from ~2 requests per walk to a fixed 20 multiplies the chances of hitting it. The
|
|
# bound is applied HERE rather than inside `gh` so every other call site keeps the
|
|
# behaviour it was reviewed with: widening a shared helper changes callers nobody
|
|
# re-examined. 15s is ~100x the measured per-request latency, so it bounds a hang
|
|
# without reddening a slow-but-working instance.
|
|
#
|
|
# RETRIED: `page_statuses` retries each page once and states why — "one transient blip
|
|
# would cost that head its exemption until a human clears it by hand". That argument
|
|
# applies here with 10x the force after this change, because `rt_ok=no` on the
|
|
# exemption path writes the sticky sentinel too. This walk had no retry when it made
|
|
# ~2 requests; at 20 it needs the sibling's. Same shape deliberately: same `for try`,
|
|
# same one-second sleep BETWEEN attempts only, same "an outage still refuses".
|
|
raw=""
|
|
kind=""
|
|
# `if`/`then`, never `cmd && var=yes`: under `set -e` a bare `A && B` whose test fails
|
|
# is itself a failing command, and as the last command of a loop body it would kill
|
|
# the job. (Carried from `page_statuses`, where the same trap was found.)
|
|
for try in 1 2; do
|
|
raw=$(gh --connect-timeout 5 --max-time 15 "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || raw=""
|
|
if [ -n "${raw//[[:space:]]/}" ]; then
|
|
kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind=""
|
|
fi
|
|
# THIS SET MUST MATCH THE `case` ARMS BELOW. Their agreement is load-bearing rather
|
|
# than tidy: breaking only on shapes the `case` accepts is what stops a stale `kind`
|
|
# from attempt 1 being judged against a `raw` that attempt 2 cleared. Widen one
|
|
# without the other and that correspondence is gone.
|
|
if [ "$kind" = array ] || [ "$kind" = null ]; then break; fi
|
|
if [ "$try" -eq 1 ]; then sleep 1; fi
|
|
done
|
|
# A PAGE PAST THE END IS JSON `null` HERE, NOT `[]` (ersatztv#751). Measured on this
|
|
# instance at Gitea 1.27.1 (page 1 non-empty, page 2 exhausted): page 2 of PR #752's timeline is the
|
|
# four bytes `null`, while `/issues/{n}/comments` with no comments returns `[]`. The
|
|
# instance is NOT consistent between endpoints, so the terminator has to be read from
|
|
# what this endpoint actually does rather than from the shape a list endpoint "should"
|
|
# return.
|
|
#
|
|
# This mattered far more than it looks. An `array`-only type gate treated `null` as
|
|
# unreadable, so the walk NEVER reached a validated empty page, `rt_ok` was never `yes`
|
|
# for ANY pull request, and the fence below therefore withheld every exemption
|
|
# `success` — permanently. Renovate and docs-only PRs got no status at all rather than
|
|
# an auto-pass, which is the same user-visible outcome as ersatztv#751 arriving by a
|
|
# completely different route. It was invisible because it shipped in the SAME commit
|
|
# (8f6d4f443) that stopped this step from executing at all, so the fence had never once
|
|
# run in production, and because the test double asserted the wrong shape — it printed
|
|
# `[]` for a page past the end while claiming to mirror measured reality.
|
|
#
|
|
# Read the type as a VALUE rather than through `jq -e`. `jq -e` reports the truthiness
|
|
# of the last output, so distinguishing "the body is null" from "the predicate is
|
|
# false" through it means relying on the same exit-status subtlety that already bit this
|
|
# workflow once at jq 1.6 (ersatztv#647). A `case` over `jq -r 'type'` cannot be read
|
|
# two ways. Anything that is neither `null` nor `array` is still "cannot tell".
|
|
# AN EMPTY PAGE IS NOT EXHAUSTION, AND THIS WALK NO LONGER TREATS IT AS ONE
|
|
# (ersatztv#870). Gitea pages this endpoint at the DATABASE level and filters
|
|
# AFTERWARDS: `FindComments` applies the LIMIT/OFFSET, then
|
|
# `ListIssueCommentsAndTimeline` drops every `CommentTypeCode` row and every
|
|
# cross-reference the reader cannot access, appending the survivors to a
|
|
# `var apiComments []*api.TimelineComment` — a NIL slice, which serializes as bare
|
|
# `null`. So a page whose rows are ALL inline-code comments comes back empty WHILE
|
|
# LATER PAGES STILL HOLD EVENTS, and rows are ordered ASCENDING, so the events a fence
|
|
# looks for are the ones furthest from page 1. Fifty inline review comments, which a
|
|
# PR author can create on their own PR, were enough to truncate both walks at the same
|
|
# place: both counts then agreed, the sha comparison agreed, and the ABA went unseen.
|
|
#
|
|
# THE FIX MOVES THE INFERENCE TO THE CAP; IT DOES NOT REMOVE IT, and saying so
|
|
# precisely matters more here than a tidy claim. This loop already carries a 20-page
|
|
# cap, and the sibling status walk below already states the CAP half of the contract
|
|
# this one was breaking — "the 20th request has to be the EMPTY terminator for the
|
|
# walk to be validated". Only the cap half: `page_statuses` still returns on its FIRST
|
|
# empty page, so the two walks do NOT now share a termination rule and saying they do
|
|
# would be a parity claim nobody established (ersatztv#893 tracks whether that walk
|
|
# needs the same change; `/statuses/{sha}` returns a TRUE `X-Total-Count`, which this
|
|
# endpoint does not, so its terminator may well mean what it says). So the walk now
|
|
# reads EVERY page up to that cap and decides afterwards:
|
|
# an empty page BEFORE the cap is SKIPPED, never believed. The ONE empty page still
|
|
# trusted is the page-20 terminator, and it is trusted for the same unprovable reason
|
|
# the old page-2 terminator was — the endpoint cannot distinguish a filtered page from
|
|
# the end of the list, at ANY offset. What changes is the price — see the note on
|
|
# the residual below for the one statement of it this file makes. A "require K consecutive empty
|
|
# pages" rule was rejected because K is arbitrary and only moves the bar to 50K
|
|
# filtered rows; the cap is not arbitrary — it is the coverage this walk already
|
|
# claimed to perform.
|
|
#
|
|
# MEASURED AT GITEA 1.27.1 ON THIS INSTANCE (2026-08-30), because the cheaper
|
|
# alternatives are ruled out by measurement rather than by reasoning:
|
|
# * `X-Total-Count` on this endpoint is the POST-FILTER LENGTH OF THE PAGE, not a
|
|
# total. `?limit=1` returns `X-Total-Count: 1` on a PR whose timeline holds 14
|
|
# rows, `?limit=5` returns 5, and a page past the end returns 0. It therefore
|
|
# carries exactly what the body already carries and cannot derive a page count.
|
|
# (`/activities/feeds` DOES return a true total — 5739 — so this is a property of
|
|
# this handler, not of the server, and a future Gitea could change it.)
|
|
# * `limit` clamps to 50 here: `?limit=200` returned 50 on #303, #237, #91 and #202.
|
|
# * The endpoint's ONLY query parameters are `since`, `before`, `page` and `limit`
|
|
# (read from the live swagger, `issueGetCommentsAndTimeline`). There is no
|
|
# row-type filter, so the paged set and the serialized set cannot be made to agree.
|
|
# A `since`-bounded walk was considered and NOT taken on its own: it narrows the attack
|
|
# to the run's own window but leaves the identical ambiguity inside it, and it would
|
|
# cost the "a real page was actually read" property below, which depends on an empty
|
|
# page 1 being anomalous. COST, both bounds, because only one of them is the common
|
|
# case: a healthy walk is exactly 20 requests, measured at 2.6s against this
|
|
# instance; the worst case with the per-page retry below is 40 requests and 20
|
|
# one-second sleeps — every page blipping once and recovering — which is 20s of
|
|
# sleep on top. MEASURED by executing this function against scripted responses,
|
|
# not counted by eye: the first draft of this sentence said 19, reasoning that
|
|
# the last page could not sleep. It can.
|
|
#
|
|
# WALL-CLOCK, which is what the timeout is actually for. An earlier version of this
|
|
# comment reasoned that only the last page can pay two timeouts, since a page whose
|
|
# BOTH attempts fail ends the walk, and put the bound at ~335s. That bounds TIMEOUTS,
|
|
# not elapsed time: attempt 1 can burn its full 15s and attempt 2 still SUCCEED
|
|
# slowly, so the page costs ~31s and the walk carries on. The pessimum is therefore
|
|
# 20 x (15 + 1 + 15) = 620s per walk — ~21min on the exempt path's three walks — and
|
|
# the job carries no `timeout-minutes` to catch it.
|
|
#
|
|
# Stated anyway rather than tuned down, because the DIRECTION is what justifies it:
|
|
# before this, `gh` carried no timeout at all and one hung connection had no bound
|
|
# whatsoever. Bounded-and-slow replaces unbounded — for THIS call site only.
|
|
# `page_statuses` still has no `--max-time`, walks up to 20 pages x 2 attempts, and
|
|
# runs in the same post-POST window ahead of the third walk here, so the "a hang
|
|
# leaves the green standing" mode is still reachable through the sibling.
|
|
#
|
|
# WHAT REMAINS UNCOVERED — NOT CLOSED, NARROWED — stated because #803's whole point
|
|
# is that a contract must not assert more than its code does. Pages 1-19 cover offsets
|
|
# 0..949 and page 20 covers 950..999, so an event is still hidden by a timeline of
|
|
# more than 1000 rows that ALSO presents a fully-filtered block at offsets 950..999:
|
|
# the walk reads an empty page 20, never requests page 21, and publishes a too-low
|
|
# count with `rt_ok=yes`. Both re-reads share the blind spot, so the counts agree and
|
|
# the exemption is granted — the SAME construction as before, at the cap instead of at
|
|
# page 2, at roughly 10x the price. State that price ONCE and in one unit, because
|
|
# two earlier drafts of this comment stated it two ways 45 lines apart. The FILTERED
|
|
# BLOCK is 50 rows in both cases — that is the part which must be inline review
|
|
# comments. What grew is the timeline the attacker has to build around it: ~100 rows
|
|
# before (a non-empty page 1, then a 50-row block at a 50-aligned offset) against more
|
|
# than 1000 now, with the block pinned to offsets 950..999. On their own PR the padding
|
|
# is theirs to supply too, so "50 comments either way" was wrong: it counted only the
|
|
# block and ignored the ~950 rows that now have to exist beneath it.
|
|
#
|
|
# This is NOT the same residual as the status walk's, and equating them would
|
|
# understate it: there a non-empty page 20 is the only over-cap shape, because
|
|
# `/statuses/{sha}` does not filter after paging. Only this endpoint lets a page-20
|
|
# response be empty while later pages hold rows. A timeline that fills the cap with
|
|
# READABLE pages does fail closed; one that fills it and hides the 20th behind a
|
|
# filtered block does not.
|
|
#
|
|
# No terminator AVAILABLE on this endpoint does better — measured, not assumed, and
|
|
# NOT a proof that none could exist. A different page SIZE, for instance, shifts the
|
|
# boundaries without closing anything: `?limit=49&page=21` reaches offsets 980..1028,
|
|
# past this walk's horizon of 999, and its page edges do not coincide with a 50-row
|
|
# walk's, so a block that exactly fills one walk's terminator page does not fill the
|
|
# other's. Strictly better, still not a terminator. See the measurements above: there is no total to page against, no row-type filter to
|
|
# make the paged and serialized sets agree, and a `since` bound leaves the identical
|
|
# ambiguity inside its window. Closing this needs a monotonic key that is not served
|
|
# by a filtered, paged endpoint, and none is known (ersatztv#870 option 3).
|
|
#
|
|
# Read the type as a VALUE rather than through `jq -e` — see the `kind` note above.
|
|
empty=no
|
|
case "$kind" in
|
|
null) empty=yes ;;
|
|
array) ;;
|
|
*) return 0 ;;
|
|
esac
|
|
if [ "$empty" = no ]; then
|
|
n=$(printf '%s' "$raw" | jq -r 'length' 2>/dev/null) || n=""
|
|
case "$n" in ''|*[!0-9]*) return 0 ;; esac
|
|
# `[]` AND `null` TAKE THE SAME RULE (ersatztv#803 review). `/issues/{n}/comments`
|
|
# really does return `[]` when empty while this endpoint returns `null`, so both
|
|
# shapes are live on this server; treating them differently once let a `[]` page 1
|
|
# certify a zero count. Keeping them identical is also what stops a serializer
|
|
# change from silently switching the walk between a trusted and an untrusted
|
|
# terminator.
|
|
if [ "$n" -eq 0 ]; then empty=yes; fi
|
|
fi
|
|
if [ "$empty" = yes ]; then
|
|
# AN EMPTY PAGE 1 IS ANOMALOUS AND ENDS THE WALK UNTRUSTED. THE INVARIANT, not a
|
|
# figure that rots: a real PR's timeline always carries at least one event on page 1
|
|
# (the PR is created by a push, which is itself an event). Spot-checked across
|
|
# #752/#753/#749/#739/#717, all non-empty; the counts themselves are deliberately
|
|
# not recorded here because timelines grow and an earlier version of this comment
|
|
# cited five numbers of which three were stale within days. Trusting a zero count
|
|
# from an anomalous first page would mean trusting that no retarget happened on the
|
|
# strength of a response we cannot explain, so it falls through to `rt_ok=no`, which
|
|
# withholds the exemption and asks for a human verdict — the safe direction.
|
|
if [ "$page" -eq 1 ]; then return 0; fi
|
|
else
|
|
# EVERY jq READ BELOW IS GUARDED `2>/dev/null || x=""`, matching `page_statuses`
|
|
# (added ersatztv#870 review). Today none of them can error — the `array` gate above
|
|
# and the `bad -eq 0` ordering see to that — but "safe because of a property three
|
|
# lines up" is exactly the reasoning the `jq -e` note below says it is retiring, and
|
|
# an unguarded jq error dies under `set -e`. At the POST-WRITE call site that death
|
|
# lands after the exemption `success` is on the head and before any repair, which is
|
|
# the shape this file already fixed once for `.creator`. The `case` validation on
|
|
# each value turns an empty string into the existing untrusted path.
|
|
#
|
|
# VALIDATE THE FIELD THE SECURITY DECISION READS, on every row (ersatztv#803 review).
|
|
# Both tallies below select on `.type`; a row without a string `.type` matches neither
|
|
# and is silently treated as an irrelevant event. That is the same
|
|
# validate-what-you-consume rule `pr-changed-files.sh` already applies to every row it
|
|
# extracts, and the reason is the same: a shape we cannot read is not evidence that
|
|
# nothing happened. Every timeline row measured on this instance carries a string
|
|
# `.type`, so this rejects only shapes the server has not been observed to produce.
|
|
#
|
|
# COUNTED, NOT `jq -e`, and that is this function's own standing rule rather than a
|
|
# style preference. The `kind` read above says it in full: `jq -e` reports the
|
|
# truthiness of its LAST OUTPUT, which is the exit-status subtlety that already bit
|
|
# this workflow at jq 1.6 (ersatztv#647), and the runner still ships 1.6. A count read
|
|
# as a VALUE and then range-checked in shell cannot be read two ways, and it matches
|
|
# every other tally here. (The first draft of this guard used `jq -e`, safely by
|
|
# accident — the array is known non-empty by the check above, so the empty-input
|
|
# divergence could not fire — but "safe because of a property three lines up" is
|
|
# exactly the reasoning the rule exists to retire.)
|
|
bad=$(printf '%s' "$raw" | jq -r '[.[] | select(type != "object" or (.type | type) != "string")] | length' 2>/dev/null) || bad=""
|
|
case "$bad" in ''|*[!0-9]*) return 0 ;; esac
|
|
if [ "$bad" -ne 0 ]; then return 0; fi
|
|
m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length' 2>/dev/null) || m=""
|
|
case "$m" in ''|*[!0-9]*) return 0 ;; esac
|
|
# Counted in the SAME pass over the SAME page, so the head axis costs no extra round
|
|
# trip and cannot be certified by a different set of pages than the base axis. A
|
|
# non-numeric length here abandons BOTH counts, exactly as it does above — a page whose
|
|
# push tally is unreadable has not established the retarget tally either.
|
|
p=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "pull_push")] | length' 2>/dev/null) || p=""
|
|
case "$p" in ''|*[!0-9]*) return 0 ;; esac
|
|
total=$(( total + m ))
|
|
ptotal=$(( ptotal + p ))
|
|
fi
|
|
page=$(( page + 1 ))
|
|
done
|
|
# THE CAP IS THE TERMINATOR, AND THE LAST PAGE READ MUST BE EMPTY (ersatztv#870).
|
|
# Falling out of the loop on a NON-empty page 20 means the timeline may extend past the
|
|
# offsets this walk covered, so it is "could not establish" and leaves `rt_ok=no` — the
|
|
# same treatment an unreadable page gets, and the same rule the status walk states for
|
|
# itself. That is also why the confirmable bound is 950 rows and not 1000: the 20th
|
|
# request has to come back empty for the walk to be validated.
|
|
if [ "$empty" = yes ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; fi
|
|
return 0
|
|
}
|
|
# PAGE THE PER-POST STATUS HISTORY TO A VALIDATED TERMINATOR (ersatztv#763).
|
|
#
|
|
# Both callers below want the WHOLE history, and a single `?limit=100` read cannot give it:
|
|
# `limit` clamps to the server-wide `MAX_RESPONSE_ITEMS`, measured at 50.
|
|
#
|
|
# BE PRECISE ABOUT WHAT THAT COST, because the issue and an earlier version of this comment
|
|
# both overstated it. Under the server default (`created_unix DESC`) page 1 holds the NEWEST
|
|
# rows, and ids are monotonic with `created_at` (measured). So page 1 already carried the
|
|
# true maximum id, AND every row newer than the mark — which is the only thing the
|
|
# post-write check selects on. A single-page read therefore missed a raced verdict only if
|
|
# more than 50 rows were created INSIDE the write window, not merely "on a head with more
|
|
# than 50 rows". What actually removed #761's stall is deleting #751's page-2 "assume raced"
|
|
# probe, further down; this walk is not what fixed it.
|
|
#
|
|
# THE WALK IS STILL WORTH HAVING, for a different reason than the issue gives: it removes
|
|
# the gate's one fail-toward-SUCCESS path from depending on an UNDOCUMENTED ordering that
|
|
# the server honours only coarsely. Page 1 came back `114,112,113,111,110` — newest-first,
|
|
# but not strictly, and nothing in the API contract promises even that. Selecting by id over
|
|
# a list known to be complete does not care how the server ordered it.
|
|
#
|
|
# CORRECTNESS DOES NOT DEPEND ON THE NUMBER 50. The walk stops at a validated EMPTY page, so
|
|
# whatever page size the server actually applies is paged correctly; `limit=50` only asks
|
|
# for what this instance will really give. The distinction matters because #751 shipped a
|
|
# completeness check written against a hardcoded `limit=100` that was DEAD CODE precisely
|
|
# because the cap is 50. A guard against a hardcoded cap is the thing to avoid; a request
|
|
# that names one, whose correctness does not turn on it, is not the same mistake.
|
|
#
|
|
# THE TERMINATOR IS MEASURED PER ENDPOINT — this server is not consistent between them and
|
|
# guessing has been wrong twice. Measured at 1.27.1 on 2026-08-28 against PR #761's head
|
|
# (114 rows): pages 1 and 2 return 50, page 3 returns 14, page 4 is the two bytes `[]`.
|
|
# `/issues/{n}/timeline` returns bare `null` past the end and `/commits/{sha}/status`
|
|
# returns `{"statuses": null}` — three distinct empty shapes on one instance, which is why
|
|
# `count_pr_mutations` above and this walk deliberately do NOT share a terminator.
|
|
#
|
|
# A SHORT PAGE IS NOT THE END. Page 3 above carried 14 rows and was still followed by a real
|
|
# page 4, and more fundamentally a short page is indistinguishable from a truncated
|
|
# response. Only an empty page is evidence, which costs exactly one extra round-trip.
|
|
#
|
|
# AN EMPTY PAGE 1 IS LEGITIMATE HERE, unlike `count_pr_mutations`. A head nothing has posted to
|
|
# yet genuinely has no statuses (and a bogus sha returns `[]` too — measured), so an empty
|
|
# first page is an ordinary answer rather than the anomaly it is on a PR timeline. It yields
|
|
# `ph_ok=yes` over zero rows. Copying the timeline walk's "one real page required" rule here
|
|
# would withhold the mark on every first run.
|
|
#
|
|
# RETRY ONCE, THEN REFUSE. `ph_ok=no` is consumed below as "could not establish", which on
|
|
# the post-write path repairs to the STICKY sentinel — so one transient blip would cost that
|
|
# head its exemption until a human clears it by hand. A single retry absorbs the blip;
|
|
# anything persistent still refuses, because on that path uncertainty must never resolve to
|
|
# green. The retry covers a non-array body too (a 502 HTML page reads exactly like one).
|
|
page_statuses() {
|
|
ph_rows='[]'
|
|
ph_ok=no
|
|
local page=1 raw kind n acc='[]' try
|
|
# EVERY exit publishes what WAS read, not just the complete ones. `ph_ok` alone says
|
|
# whether the list is whole; a caller that can still use a partial list must be able to
|
|
# reach it, and the high-water mark below is exactly such a caller — see the fail-open it
|
|
# otherwise creates (ersatztv#763, round 2).
|
|
while [ "$page" -le 20 ]; do
|
|
raw=""
|
|
kind=""
|
|
# `if`/`then`, never `cmd && var=yes`: under `set -e` a bare `A && B` whose test fails is
|
|
# itself a failing command, and as the last command of a loop body it would kill the job.
|
|
for try in 1 2; do
|
|
raw=$(gh "$BASE_URL/repos/$REPO/statuses/$SHA?limit=50&page=${page}") || raw=""
|
|
if [ -n "${raw//[[:space:]]/}" ]; then
|
|
# Read the type as a VALUE, not through `jq -e`, for the reason recorded at
|
|
# `count_pr_mutations`: `jq -e` reports the truthiness of its last output, so it cannot
|
|
# separate "the body is null" from "the predicate is false".
|
|
kind=$(printf '%s' "$raw" | jq -r 'type' 2>/dev/null) || kind=""
|
|
fi
|
|
if [ "$kind" = array ] || [ "$kind" = null ]; then break; fi
|
|
# A SECOND IMMEDIATE REQUEST BARELY COUNTS AS A RETRY. One second does not survive a
|
|
# Gitea restart either, and saying so is the point: this absorbs a momentary blip, not
|
|
# an outage. An outage still lands on `ph_ok=no`, which is the safe direction.
|
|
# Only BETWEEN attempts — sleeping after the last one delays a failure nobody is
|
|
# waiting on, once per walk and so twice per job run.
|
|
if [ "$try" -eq 1 ]; then sleep 1; fi
|
|
done
|
|
# `null` COUNTS AS AN EMPTY PAGE, though this endpoint returns `[]` today. An
|
|
# array-only gate is the exact shape of #751: `count_pr_mutations` had one, the timeline
|
|
# really did return `null` past the end, so the walk never reached a validated empty
|
|
# page and the fence withheld EVERY exemption. Re-adopting that narrowing on a second
|
|
# endpoint would be worse, because this walk's failure is the STICKY sentinel — every
|
|
# exempt PR would need a human verdict, per head. It cannot misread `[]` either.
|
|
#
|
|
# It is NOT free, though, and the cost lands on one caller: an empty page 1 accepted as
|
|
# "complete, zero rows" is correct BEFORE the write and impossible AFTER it. The
|
|
# post-write caller therefore rejects an empty result itself, below — this walk stays
|
|
# caller-agnostic because the pre-write caller genuinely needs the empty answer.
|
|
case "$kind" in
|
|
null) ph_ok=yes; ph_rows=$acc; return 0 ;;
|
|
array) ;;
|
|
*) ph_rows=$acc; return 0 ;;
|
|
esac
|
|
n=$(printf '%s' "$raw" | jq -r 'length' 2>/dev/null) || n=""
|
|
case "$n" in ''|*[!0-9]*) ph_rows=$acc; return 0 ;; esac
|
|
if [ "$n" -eq 0 ]; then ph_ok=yes; ph_rows=$acc; return 0; fi
|
|
acc=$(printf '%s\n%s' "$acc" "$raw" | jq -s -c 'add' 2>/dev/null) || acc=""
|
|
if [ -z "$acc" ]; then return 0; fi
|
|
ph_rows=$acc
|
|
page=$(( page + 1 ))
|
|
done
|
|
# THE PAGE CAP IS NOT EXHAUSTION. Falling out of the loop leaves `ph_ok=no`, so an
|
|
# over-long history is "could not establish" rather than a silent partial list — the same
|
|
# treatment as an unreadable page, and for the same reason. The bound is 950 rows, not
|
|
# 1000: the 20th request has to be the EMPTY terminator for the walk to be validated, so
|
|
# only 19 full pages can be confirmed.
|
|
return 0
|
|
}
|
|
|
|
ex_repair=no
|
|
ex_unverified=no
|
|
count_pr_mutations
|
|
retargets_before=$rt_count
|
|
pushes_before=$hp_count
|
|
retargets_before_ok=$rt_ok
|
|
echo "Mutation fence: ${retargets_before} retarget event(s) and ${pushes_before} push 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 collaborator's own token, the
|
|
# `GITEA_TOKEN` injected into every job, `RENOVATE_TOKEN`, 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. (`#697`'s
|
|
# registry credential can no longer post a status at all — see `ci.actions-credential-scoping`
|
|
# — but that closes only one route; the others above still work.)
|
|
#
|
|
# 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.
|
|
#
|
|
# SINCE ersatztv#742 the creator must additionally be in `$H10_REVIEWERS`, not merely
|
|
# non-null. "Non-null" was satisfied by ANY account's credential, `RENOVATE_TOKEN` included —
|
|
# a `write:repository` PAT that cannot be scoped down the way `#697` scoped the registry
|
|
# credential, and reachable from any workflow because secrets are a per-repo store.
|
|
#
|
|
# What this STILL does not claim: the test asks "was this POSTed by an account we accept
|
|
# verdicts from", NOT "by someone who actually reviewed it". `ETV_STATUS_AUTH`'s registry
|
|
# credential can no longer POST a status at all (#697, fixed by scoping it off
|
|
# `write:repository` — see `ci.actions-credential-scoping`), and `RENOVATE_TOKEN` is now
|
|
# re-derived rather than inherited; but an ALLOW-LISTED collaborator's own token can still
|
|
# POST a success with an attacker-chosen `Review-verdict:` description, which this guard then
|
|
# preserves. That is provenance, not authentication — branch protection binds the context
|
|
# NAME, not its issuer, and Gitea offers no way to restrict who may post a context.
|
|
#
|
|
# THE ALLOW-LIST APPLIES TO `success` ONLY — the two states are not symmetric, and treating
|
|
# them alike is a FAIL-OPEN (ersatztv#742). Trace the symmetric version on an off-list
|
|
# `failure`: the head carries
|
|
# `h10=failure` from `alice`, a real reviewer left off the literal. `ex_human=no`, so the
|
|
# short-circuit declines; the PR is docs-only, so the classifier grants an exemption; the
|
|
# re-read triple is unchanged, so the mid-run abstain declines; alice's row predates the
|
|
# high-water mark, so the post-write repair cannot see it. The job posts `success` over an
|
|
# explicit human rejection — reached by the very misconfiguration the allow-list's own
|
|
# diagnostic is about.
|
|
#
|
|
# Splitting by state costs NOTHING that #742 bought. Inheriting a `failure` cannot green
|
|
# anything: the worst an off-list or forged `failure` achieves is a STALL, and a stall is
|
|
# clearable in one command by any human — `post-review-verdict.sh` POSTs unconditionally,
|
|
# with no read-modify-write, so there is nothing to be locked out of. Inheriting a `success`
|
|
# is the durable forgery #742 exists to stop. So `success` requires membership and `failure`
|
|
# requires only attributability.
|
|
#
|
|
# "ONLY ATTRIBUTABILITY" IS STILL A REQUIREMENT, not a synonym for "any failure": a `failure`
|
|
# whose description is not `Review-verdict:`-shaped, or whose creator is null, or which
|
|
# records a DIFFERENT base, is re-derived like any other unattributable row. Widening this to
|
|
# a bare `[ "$ex_state" = failure ]` would be safe by the argument above, and is deliberately
|
|
# NOT done — it would quietly opt `failure` out of the base binding (`ci.exemption-provenance`
|
|
# input 1), which exists because a verdict earned against another base is not a verdict for
|
|
# this diff. None of the excluded shapes is producible by `post-review-verdict.sh`.
|
|
read_existing_verdict
|
|
if [ "$ex_human" = yes ] && [ "$ex_state" = "success" ]; then
|
|
echo "${CONTEXT} is already 'success' on ${SHA:0:7}, written by '${ex_creator}', an allow-listed reviewer — leaving it alone."
|
|
exit 0
|
|
fi
|
|
if [ "$ex_attributable" = yes ] && [ "$ex_state" = "failure" ]; then
|
|
echo "${CONTEXT} is already 'failure' on ${SHA:0:7}, written by '${ex_creator}' — an ATTRIBUTABLE rejection for this base is left alone whichever account recorded it, because honouring it can only ever withhold an exemption, never grant one."
|
|
exit 0
|
|
fi
|
|
# SNAPSHOT THE ROW WE DECIDED AGAINST (ersatztv#742). The last-moment re-read below asks
|
|
# "did a verdict land WHILE WE WERE CLASSIFYING", and before #742 it got that for free: the
|
|
# two reads computed an IDENTICAL predicate, so anything that fired at the second and not
|
|
# the first must have arrived in between. Splitting `ex_human` from `ex_attributable` broke
|
|
# that — a pre-existing row this job declined to INHERIT is still attributable, so the
|
|
# re-read would abstain on the very row the classification just decided to overwrite, leave
|
|
# the forged success standing untouched, and post nothing. Ever. (Measured, not reasoned:
|
|
# the inheritance tests went red on exactly that.)
|
|
#
|
|
# So "changed" is made explicit instead of implied. Compare the whole row we saw, not just
|
|
# its presence: a REPLACEMENT (same context, new state/creator/description) is as much a
|
|
# mid-run event as an arrival, and the combined endpoint gives one row per context, so a
|
|
# newer POST replaces rather than adds.
|
|
#
|
|
# MEASURED on this instance 2026-08-26, not assumed: two POSTs of context `probe742/combined-newest` to one sha, `pending`
|
|
# then `success`. `/commits/{sha}/status` returned ONE row for that context, the newest
|
|
# (`success`); `/statuses/{sha}` returned BOTH. Reproduce with two POSTs to
|
|
# `/repos/{o}/{r}/statuses/{sha}` and a read of each endpoint. `main` depended on none of
|
|
# this — presence alone sufficed there — so the claim became load-bearing with the
|
|
# changed-row comparison, and is measured rather than inherited.
|
|
#
|
|
# ONE ORDERING CHANGES BEHAVIOUR AND IT IS AN IMPROVEMENT, recorded so it is not read later
|
|
# as an accident: an attributable row in a state OTHER than `success`/`failure` — a
|
|
# `pending`- or `error`-state row whose description happens to start `Review-verdict:`. On
|
|
# `main` neither short-circuit fired for it (they test the state) and the old unconditional
|
|
# abstain then fired at the re-read, so the job posted NOTHING, on that run and on every
|
|
# later one: a permanent stall with no diagnostic. The changed-row comparison removes it —
|
|
# the triple is unchanged, so the job falls through and posts.
|
|
pre_state=$ex_state
|
|
pre_creator=$ex_creator
|
|
pre_desc=$ex_desc
|
|
# THE ID IS PART OF THE SNAPSHOT (ersatztv#849, round 2). The triple cannot see a
|
|
# REPLACEMENT whose text matches, and the two sentinels are byte-identical by design, so
|
|
# "another run replaced this row while we classified" was invisible for exactly the rows
|
|
# where it matters most. Where the server does not send an id both sides are empty and the
|
|
# comparison falls back to the triple, which is the pre-existing behaviour.
|
|
pre_id=$ex_id
|
|
if [ -n "$ex_state" ]; then
|
|
# STATE-NEUTRAL WORDING. Control also reaches here for an ALLOW-LISTED creator whose row is
|
|
# in some state other than `success`/`failure`, so this must not assert the row is
|
|
# unattributable.
|
|
#
|
|
# THE ALLOW-LIST IS NAMED HERE ON PURPOSE. If a reviewer is ever added to the repo and
|
|
# forgotten from `H10_REVIEWERS`, their verdicts are re-derived rather than honoured, and
|
|
# this line — creator, allow-list and description together — is what says so. A separate
|
|
# `::warning::` annotation was tried and WITHDRAWN: it duplicated all three values from
|
|
# this line, its only delta was a severity whose rendering could not be probed, and it
|
|
# produced three defects across three review rounds (a message that read "from 'timothy',
|
|
# which is NOT in H10_REVIEWERS ('timothy')", an unescaped API value interpolated into a
|
|
# workflow command, and a test that asserted its absence on a path that exits two guards
|
|
# earlier and so could never fail). One line that is always right beats two where the
|
|
# second needs a guard to stop contradicting the first.
|
|
echo "${CONTEXT} is '${ex_state}' on ${SHA:0:7} and is not being inherited as a reviewer verdict for this base (creator='${ex_creator:-null}', allow-list='${H10_REVIEWERS}', description='${ex_desc}') — re-deriving it from the PR's current state. If '${ex_creator:-that account}' is a reviewer, add them to H10_REVIEWERS in .gitea/workflows/review-verdict.yml."
|
|
fi
|
|
|
|
# --- RECONCILE AN UNVERIFIED WRITE (ersatztv#849) ------------------------------------
|
|
# This is what makes the unverified sentinel a BOUNDED stall rather than the permanent one
|
|
# that got #742's attempt withdrawn. The sentinel records "a write on this head could not
|
|
# be checked against the status history", and unlike the repair sentinel that fact expires:
|
|
# a run that CAN read the history settles the question either way.
|
|
#
|
|
# WHY THE PER-POST HISTORY ANSWERS IT AND THE COMBINED ENDPOINT CANNOT. The combined
|
|
# endpoint returns the latest row per context, and the latest row IS the sentinel — the
|
|
# verdict this job may have masked is underneath it and structurally invisible there.
|
|
# `/statuses/{sha}` returns one row per POST, so a masked verdict is still in that list.
|
|
# This is the same asymmetry the post-write check relies on, used for the opposite purpose.
|
|
#
|
|
# DELIBERATELY BROADER THAN THE INHERITANCE TEST, and broader than the base binding too: no
|
|
# `$H10_REVIEWERS` membership, no `(base: …)` match. The two errors are not symmetric.
|
|
# Counting a row that is not really a verdict for this base upgrades to the repair sentinel
|
|
# — a stall a human clears with one command. MISSING one clears the sentinel and lets a
|
|
# later run exempt a head that carries a buried human rejection, which is the permanent
|
|
# forged green this whole issue is about. Over-counting is the affordable error.
|
|
#
|
|
# ONLY A COMPLETE WALK MAY CLEAR IT. `ph_ok != yes` means the history is still unreadable,
|
|
# so the question the sentinel asks is still open and it is carried forward unchanged by the
|
|
# classification below. An unreadable reconciliation must never resolve to "nothing buried".
|
|
if [ "$ex_unverified" = yes ]; then
|
|
echo "${CONTEXT} on ${SHA:0:7} carries the unverified-write sentinel from an earlier run — reconciling it against the per-POST status history."
|
|
page_statuses
|
|
# THE WITNESS: THE SENTINEL'S OWN ROW MUST BE IN THE HISTORY (ersatztv#849, round 2).
|
|
# `ex_unverified=yes` means the COMBINED endpoint just returned the sentinel for this
|
|
# sha, and `/statuses/{sha}` keeps one row per POST — so a complete history that does
|
|
# NOT contain it, empty ones included, is a response that cannot be true. The post-write
|
|
# check already refuses an empty history for exactly this reason; reconciliation stands
|
|
# in the same position and had no such guard, so a single anomalous `[]` cleared the
|
|
# sentinel and let the run exempt a head carrying a buried rejection. `page_statuses`
|
|
# deliberately accepts an empty page 1 as complete (correct for a first-run mark), which
|
|
# is what made the shape reachable.
|
|
#
|
|
# THIS IS NOT THE WITHDRAWN CURRENCY WITNESS. That asked "is there ANY row above the
|
|
# mark", which an unrelated newer row satisfied, and it converted one anomaly into a
|
|
# PERMANENT sentinel. This asks for a SPECIFIC row already known to exist, and failing it
|
|
# carries the sentinel forward for THIS run only — the next run retries.
|
|
# THE WITNESS IS THE CURRENT ROW'S ID, not merely a row with the right text
|
|
# (ersatztv#849 round 3). Matching on description alone is satisfied by an OLDER
|
|
# identical sentinel, which is precisely what a fixed point produces: with S1 and a
|
|
# buried human verdict below the CURRENT S2, a read carrying only S1 satisfies the
|
|
# witness, `buried` sees nothing, the sentinel clears, and the verdict ends up below the
|
|
# fresh mark and invisible. The record already described this as identifying a SPECIFIC
|
|
# row; the code did not.
|
|
#
|
|
# `$ex_id` COMES FROM THE COMBINED READ, so it names the row that is current right now.
|
|
# When the server omits it — not observed on this instance, where every row carries one —
|
|
# there is nothing to match on and the check degrades to the description, which is the
|
|
# pre-round-3 behaviour rather than a new hole.
|
|
if [ -n "$ex_id" ]; then
|
|
witness=$(printf '%s' "$ph_rows" | jq -r --arg c "$CONTEXT" --argjson wid "$ex_id" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select(((.id | numbers) // -1) == $wid)] | length') || witness=""
|
|
else
|
|
witness=$(printf '%s' "$ph_rows" | jq -r --arg c "$CONTEXT" --arg ud "$UNVERIFIED_DESC" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select(((.description | type) == "string") and (.description == $ud))] | length') || witness=""
|
|
fi
|
|
case "$witness" in ''|*[!0-9]*) witness=0 ;; esac
|
|
if [ "$ph_ok" != yes ] || [ "$witness" -eq 0 ]; then
|
|
echo "::warning::Could not read a status history for ${SHA:0:7} that is both complete and contains the sentinel this head carries (complete=${ph_ok}, sentinel rows found=${witness}), so the unverified write from an earlier run still cannot be reconciled. Carrying the sentinel forward; ${CONTEXT} stays pending and no exemption is granted."
|
|
else
|
|
# `.description` IS TYPE-TESTED BEFORE `startswith`, exactly as the post-write filter
|
|
# does it and for the same reason: a row whose description is a number makes
|
|
# `startswith` hard-error, jq exits 5, and the count comes back unusable. Dropping the
|
|
# malformed row keeps a well-formed verdict beside it countable.
|
|
buried=$(printf '%s' "$ph_rows" | jq -r --arg c "$CONTEXT" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select(((.creator | type) == "object") and ((.creator.login // "") != ""))
|
|
| select(((.description | type) == "string")
|
|
and (.description | startswith("Review-verdict:")))] | length') || buried=""
|
|
case "$buried" in
|
|
''|*[!0-9]*)
|
|
# AN UNUSABLE COUNT IS NOT ZERO. Same direction as every other "cannot tell" on
|
|
# this path: the sentinel stands.
|
|
echo "::warning::The reconciliation count for ${SHA:0:7} came back '${buried}' instead of a number, so the unverified write still cannot be reconciled. Carrying the sentinel forward."
|
|
;;
|
|
0)
|
|
ex_unverified=no
|
|
echo "Reconciled: the complete status history for ${SHA:0:7} carries no verdict row, so the earlier unverified write masked nothing. Clearing the sentinel and classifying normally."
|
|
;;
|
|
*)
|
|
# UPGRADE, never clear. A verdict IS in the history and the sentinel is sitting on
|
|
# top of it, so the earlier unverified write did mask a human decision. That is now
|
|
# an established fact rather than an open question, which is exactly the repair
|
|
# sentinel's meaning — and unlike this one, it may only be cleared by a human.
|
|
ex_unverified=no
|
|
ex_repair=yes
|
|
echo "::error::Reconciled: the status history for ${SHA:0:7} carries ${buried} verdict row(s) underneath an unverified exemption write, so a human verdict was masked. Upgrading to the repair sentinel. Re-post it with: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
;;
|
|
esac
|
|
fi
|
|
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. Anything but exit 0 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.
|
|
#
|
|
# WHAT exit 0 ACTUALLY ASSERTS (corrected 2026-08-28, ersatztv#803/#664). It said "complete
|
|
# and bound to $SHA". The first half is right; the second claims more than the script can
|
|
# do, and this job is the caller for which the difference has consequences — a match here
|
|
# posts a green required status with nobody in the loop. The script's head check compares
|
|
# `.head.sha` against `$SHA` after paging, which catches every one-way move and NO alias:
|
|
# `H1 -> H2 -> H1` restores the expected value while the middle pages came from `H2`. So
|
|
# exit 0 means "complete, with no head or base movement OBSERVABLE from inside the
|
|
# enumeration".
|
|
#
|
|
# The alias is fenced HERE instead, by the monotonic `pull_push` count in
|
|
# `count_pr_mutations` — the write is withheld if the head branch was pushed at all while
|
|
# this job was classifying. That is why the fence and this call site must be read together:
|
|
# neither is sufficient alone, and the enumerator is the half that CANNOT be made
|
|
# sufficient (`ci.verdict-write-retarget-fence`).
|
|
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]*)
|
|
# REPLACES, for the reason every other refusal here does (ersatztv#849 round 3): this
|
|
# run has already DECLINED to inherit whatever the head carries, so exiting without
|
|
# writing leaves that row authoritative. The old message said no status "will be
|
|
# written", which was true and beside the point — the question is what is standing.
|
|
replace_unknown_and_die "A path predicate returned '${v}' instead of a count for ${SHA:0:7}, so the classifier is not operating and this run cannot say what belongs on this head." ;;
|
|
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 [ "$ex_unverified" = yes ]; then
|
|
# Reached only when the reconciliation above could not read the history. Granting an
|
|
# exemption now would be granting it over a write nobody has ever been able to check.
|
|
exempt=no
|
|
reason="an earlier write on this head could not be verified against the status history, and this run could not reconcile it — no exemption until that history can be read"
|
|
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"
|
|
elif [ "$ex_unverified" = yes ]; then
|
|
# CARRY THIS SENTINEL FORWARD TOO, and for the same fixed-point reason: writing the
|
|
# generic description here would erase the marker the refusal above depends on, and the
|
|
# next run would see an ordinary machine `pending` and re-derive it into `success`. That
|
|
# is precisely the defect that got #742's generic-`pending` attempt withdrawn.
|
|
#
|
|
# ORDERED AFTER `ex_repair`, never before it. The reconciliation can turn this state into
|
|
# that one but never the reverse, so when both are somehow set the stronger fact — a
|
|
# verdict was definitely lost, clearable only by a human — must be the one written.
|
|
state=pending
|
|
desc="$UNVERIFIED_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 verdict whose recorded base does not match this
|
|
# PR's (`ex_attributable` is cleared, and `ex_human` is derived from it afterwards, so both
|
|
# flags go with it — 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
|
|
# PAGED (ersatztv#763). This was one `?limit=100` read, so on a head carrying more rows than
|
|
# the clamp the "maximum" was only the maximum of page 1. That direction is safe — a mark
|
|
# that is too low makes pre-existing rows look newer and repairs an exemption that nobody
|
|
# raced — but it is a stall with no diagnosis attached, and it is the same read the
|
|
# post-write check depends on being complete.
|
|
page_statuses
|
|
# A PARTIAL LIST STILL YIELDS A USABLE MARK, and refusing one is a FAIL-OPEN (ersatztv#763).
|
|
# This depends on the DESC default above: the newest row — the one carrying the maximum id,
|
|
# ids being monotonic with `created_at` (measured) — is on page 1, so a walk that fails
|
|
# later still saw it. Under an ASCENDING sort this reasoning inverts and the fallback
|
|
# becomes a spurious-repair engine; see the withdrawal note at `page_statuses`.
|
|
# Skipping the mark skips the post-write race check entirely, so a human rejection landing
|
|
# in the write window is neither detected nor repaired and the exemption stands green over
|
|
# it. Before paging existed only a failure of the single page-1 request could reach that;
|
|
# requiring a COMPLETE walk here would newly route a page-2 hiccup, an over-cap history or
|
|
# one malformed id on a later page into the same hole — turning a paging fix into a wider
|
|
# fail-open than the bug it closes.
|
|
#
|
|
# A mark taken over fewer rows can only be LOWER than the true maximum, and low is the safe
|
|
# direction: it makes the post-write check MORE eager (it may repair over a pre-existing
|
|
# row), never blinder. That is the same trade the pre-paging code made every time, since a
|
|
# single page was all it ever read.
|
|
ph_len=$(printf '%s' "$ph_rows" | jq -r 'if type == "array" then length else 0 end' 2>/dev/null) || ph_len=""
|
|
case "$ph_len" in ''|*[!0-9]*) ph_len=0 ;; esac
|
|
if [ "$ph_ok" != yes ] && [ "$ph_len" -gt 0 ]; then
|
|
echo "::warning::Could not page the whole status history for ${SHA:0:7}; taking the high-water mark over the ${ph_len} row(s) that were read. The mark may be below the true maximum, which makes the post-write race check more eager, never blinder."
|
|
fi
|
|
if [ "$ph_ok" = yes ] || [ "$ph_len" -gt 0 ]; then
|
|
# NUMBERS ONLY. jq orders strings above every number, so a single `"id": "12"` from a
|
|
# schema-corrupt row becomes the max, passes the `*[!0-9]*` gate as `12`, and — with a
|
|
# larger string — silently inflates the mark until nothing looks newer than it. That is a
|
|
# fail-open, so ids that are not numbers are excluded rather than coerced.
|
|
# AN EMPTY HISTORY AND AN UNUSABLE ONE ARE DIFFERENT ANSWERS. A head nothing has posted
|
|
# to legitimately has no rows, and its mark is 0 — every later row is newer, which is
|
|
# correct. But a NON-empty history in which no row carries a numeric id is a schema we
|
|
# cannot read, and collapsing that to 0 makes every pre-existing row look newer than the
|
|
# mark and repairs the exemption away. So the two are reported separately and only the
|
|
# first yields a number; `none` falls through the numeric gate below and skips the check.
|
|
mark=$(printf '%s' "$ph_rows" | jq -r 'if length == 0 then 0 else ([.[] | select(type == "object") | .id | numbers] | max // "none") end' 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
|
|
# NO MARK MEANS NO POST-WRITE CHECK AT ALL, which is why the write itself changes below
|
|
# rather than this branch merely warning (ersatztv#849 route 1). Without a mark a verdict
|
|
# landing in the write window is neither detected nor repaired.
|
|
#
|
|
# THE #742 ATTEMPT AT THIS WAS WITHDRAWN and the reasons are what shaped the fix: it wrote
|
|
# a GENERIC `pending`, which is exactly what a later run re-derives into `success`, so it
|
|
# moved which run posted the forged green rather than stopping it; and this workflow
|
|
# triggers only on `pull_request_target` types — no `schedule`, no `workflow_dispatch` —
|
|
# so a transient failure on a PR's last event left an exempt PR stalled with no retry.
|
|
# Both are addressed by the sentinel being STICKY (a later run cannot re-derive it) and
|
|
# RECONCILABLE (a later run that can read the history clears it without a human).
|
|
echo "::warning::Could not establish a status high-water mark for ${SHA:0:7}; nothing can verify this run's write against the status history."
|
|
max_id_before=-1
|
|
fi
|
|
|
|
# MARK ANY WRITE NOTHING CAN VERIFY (ersatztv#849 route 1). The heading said "withhold the
|
|
# EXEMPTION" while the block downgrades every re-derivable state, which is narrower than
|
|
# what it does.
|
|
#
|
|
# EVERY RE-DERIVABLE WRITE IS DOWNGRADED, not only `success` (corrected in round 2). An
|
|
# earlier version restricted this to the exemption, reasoning that a sticky generic
|
|
# `pending` "withholds nothing, since an unreviewed PR is blocked already". That analysed
|
|
# the wrong PR. The damaging case is a PR that IS exemptible and only got the generic
|
|
# `pending` from a transient enumeration failure: the generic description carries no
|
|
# marker, the post-write check below does not run without a mark, so a human verdict
|
|
# landing in the write window is buried and the NEXT run re-derives that `pending` into
|
|
# the exemption with the human row below its own mark. That is route 2's damage reached
|
|
# through route 1's condition.
|
|
#
|
|
# `$REPAIR_DESC` IS EXEMPT because it is the stronger fact and is not re-derivable — the
|
|
# classification refuses to exempt over it. Downgrading it here would lose a verdict this
|
|
# head is known to have lost.
|
|
#
|
|
# POSITIONED AFTER THE MARK, NOT WITH THE CLASSIFICATION. `$max_id_before` does not exist
|
|
# up there and referencing it early is an unbound variable under `set -u` — a job that
|
|
# dies before posting anything, on every PR.
|
|
#
|
|
# NOT POST-THEN-REPAIR, which is what happens when the mark DOES exist and the check then
|
|
# fires. Here the defect is known BEFORE the write, so there is no reason to publish a
|
|
# green and take it back: a repair leaves a window in which branch protection can see the
|
|
# exemption, and an already-scheduled auto-merge can fire inside it.
|
|
if [ "$max_id_before" -lt 0 ] && [ "$desc" != "$REPAIR_DESC" ]; then
|
|
echo "::error::No status high-water mark could be established for ${SHA:0:7}, so nothing can check this run's '${state}' write for a human verdict landing in the write window. Writing the unverified-write sentinel instead of a status a later run would re-derive. A later run will either clear it, or — if it finds a verdict underneath — ask you to re-post that verdict. To settle it now: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
state=pending
|
|
desc="$UNVERIFIED_DESC"
|
|
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.
|
|
#
|
|
# `ex_attributable`, NOT `ex_human` (ersatztv#742). The allow-list narrows INHERITANCE; this
|
|
# site asks the opposite question and must stay broad, for the same reason the post-write
|
|
# raced check does. Narrowing it would make the job stop abstaining and post its exemption
|
|
# over the row — and the post-write repair does not cover that, because it is skipped
|
|
# whenever the high-water mark could not be established.
|
|
read_existing_verdict
|
|
# `$row_replaced` — "the row changed", computed ONCE so both mid-run guards ask the same
|
|
# question. The id is evidence of a REPLACEMENT only when BOTH reads supplied one: if one
|
|
# response omits `id` and the other includes it, a bare inequality reports a replacement
|
|
# that did not happen, and this guard's reaction to that is to abstain — leaving a row the
|
|
# classification had already declined to inherit. One-sided absence therefore falls back to
|
|
# the triple, which is exactly the pre-existing behaviour.
|
|
row_replaced=no
|
|
if [ "$ex_state" != "$pre_state" ] || [ "$ex_creator" != "$pre_creator" ] || [ "$ex_desc" != "$pre_desc" ]; then
|
|
row_replaced=yes
|
|
fi
|
|
if [ -n "$ex_id" ] && [ -n "$pre_id" ] && [ "$ex_id" != "$pre_id" ]; then
|
|
row_replaced=yes
|
|
fi
|
|
if [ "$ex_attributable" = yes ] && [ "$row_replaced" = yes ]; then
|
|
echo "::notice::An attributable 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 SAME RULE FOR THE UNVERIFIED SENTINEL (ersatztv#849), with one clause the repair
|
|
# guard above does not need.
|
|
#
|
|
# `$row_replaced` IS LOAD-BEARING HERE, and leaving it out deadlocks the PR. The
|
|
# repair guard gets its "arrived DURING this run" test for free: a repair sentinel present
|
|
# at the FIRST read forces `desc="$REPAIR_DESC"`, so the guard cannot fire on a
|
|
# pre-existing one. That is NOT true here — the reconciliation above deliberately CLEARS a
|
|
# pre-existing unverified sentinel and lets this run overwrite it, so a presence-only test
|
|
# would abstain on the very row this run just decided to replace, on this run and on every
|
|
# later one. Comparing against the snapshot is what separates "already there, and being
|
|
# replaced on purpose" from "written by an overlapping run while we classified".
|
|
#
|
|
# A run whose own write IS a sentinel is exempt from the guard: replacing a sentinel with a
|
|
# sentinel loses nothing, and the repair sentinel outranks this one.
|
|
if [ "$ex_unverified" = yes ] && [ "$row_replaced" = yes ] \
|
|
&& [ "$desc" != "$REPAIR_DESC" ] && [ "$desc" != "$UNVERIFIED_DESC" ]; then
|
|
echo "::notice::An unverified-write sentinel was written on ${SHA:0:7} while this job was classifying, so another run posted something it could not check. This run would overwrite that record with an unmarked status a later run could re-derive into an exemption — posting NOTHING and leaving the sentinel standing."
|
|
exit 0
|
|
fi
|
|
|
|
# THE FENCE ITSELF (ersatztv#706 race 1; head axis ersatztv#803/#664). Re-count BOTH
|
|
# mutation axes as late as possible and refuse to write anything if either moved since this
|
|
# run began — the PR was retargeted, or its head branch was pushed. See the long note at
|
|
# `count_pr_mutations` for why this is a handoff rather than a stall, and why a COUNT is the
|
|
# only key that survives an ABA on either axis.
|
|
#
|
|
# 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`: the walk did not reach its cap on an 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 — because a `success` written on a
|
|
# count we could not verify is exactly the forged-green outcome this fence exists to
|
|
# prevent, while `pending` blocks the merge immediately.
|
|
#
|
|
# "FOR NO SAFETY GAIN" WAS TOO STRONG and is retracted (ersatztv#742 review, #849). A
|
|
# GENERIC `pending` is not inert: it masks a rejection landing in its own write window. What
|
|
# made that DURABLE — post-write verification skipping it, so a later run re-derived it into
|
|
# an exemption `success` with the rejection below the new mark — is closed: the check now
|
|
# runs after every write, and a write no high-water mark can cover becomes the sentinel
|
|
# instead of a re-derivable description. Letting `pending` through here is still right — the
|
|
# alternative strands every PR whenever the timeline is unreadable — but the masking itself
|
|
# remains a cost, so the trade is "immediate block, one repaired write", not "free".
|
|
# ABSTAINING IS A HANDOFF ONLY WHEN THERE IS NOTHING TO HAND OFF (ersatztv#849 round 3).
|
|
# The two arms below are right not to write their CLASSIFICATION — it was computed against
|
|
# a base or a head the PR may no longer have, and the mutation that invalidated it has
|
|
# already queued a successor. But when this run DECLINED to inherit a row the head carries,
|
|
# posting nothing leaves that row authoritative for the whole window until the successor
|
|
# finishes, and the successor is not guaranteed in the one case the head-arm's own message
|
|
# names: a PR's FIRST push fires no `synchronize`.
|
|
#
|
|
# So the arms mark the head instead of merely leaving it. The sentinel is not a
|
|
# classification — it asserts nothing about the diff, only that this head carries something
|
|
# unverified — so writing it does not reintroduce what the fence exists to prevent.
|
|
# Scoped to `ex_state` being non-empty — the CURRENT row, since that is what the write
|
|
# would replace: on a head that carries nothing, abstaining leaves nothing.
|
|
mark_declined_row_if_any() { # $1 = the ::notice:: this arm already emitted
|
|
local rv mark_desc
|
|
# DECIDED ON THE ROW THIS POST ACTUALLY REPLACES — `$ex_*`, the LAST read — and on that
|
|
# ALONE. Two rounds got this wrong in opposite directions. First the refusals read only
|
|
# `$pre_*` while the write replaced `$ex_*`, so a reviewer's verdict arriving between the
|
|
# reads was buried by the refusal written to protect it. Then they read BOTH and either
|
|
# could veto, which is the mirror defect: an opening row that has since been REPLACED by a
|
|
# machine `success` still vetoed, so the arm left that success gating the head. A veto
|
|
# earned by a row that is no longer there protects nothing.
|
|
#
|
|
# `$pre_*` keeps exactly one job, below: it can only make this write STRONGER, never
|
|
# suppress it.
|
|
if [ -z "$ex_state" ]; then return 0; fi
|
|
# NO EXCLUSION FOR "THIS JOB'S OWN ORDINARY OUTPUT", and the attempt is recorded because
|
|
# it is the tempting one. A prefix test on the generic description would keep the write
|
|
# off the commonest head — but A DESCRIPTION IS NOT PROVENANCE. Any workflow holding
|
|
# `code: write` can POST a `creator: null` row, and any repository writer's user
|
|
# credential can POST one with a creator; either may choose `pending` and any description
|
|
# it likes, including this job's. The exclusion would therefore be spoofable by exactly
|
|
# the writers it must not trust: mask a human `failure` with a lookalike `pending`, and
|
|
# the arm declines to mark, the successor re-derives it as ordinary machine output, and
|
|
# the rejection ends up below that run's high-water mark where nothing can see it.
|
|
# There is no issuer field that separates this job's row from another workflow's, so text
|
|
# cannot establish ownership. The cost of marking an ordinary head is one paged walk and a
|
|
# sentinel the next run CLEARS OR UPGRADES — not "reconciles away", which overstates it:
|
|
# the reconciliation is deliberately broad, so a history carrying any `Review-verdict:`
|
|
# row, off-list or for another base, upgrades to the human-only sentinel instead, and an
|
|
# over-cap or unreadable history retains it (see the record's residuals).
|
|
# NEVER WEAKEN THE REPAIR SENTINEL. `replace_unknown_state` writes the machine-clearable
|
|
# one, and a head carrying `$REPAIR_DESC` has a non-empty `pre_state`, so the bare scope
|
|
# test fired on it — inverting the ordering this same change added a floor to protect at
|
|
# the repair site. One mechanism, three writers, and only two had the rule.
|
|
# NO SEPARATE REPAIR VETO. There was one, and it was unreachable: an `$ex_desc` of
|
|
# `$REPAIR_DESC` with a different `$desc` is caught by the mid-run sentinel guard long
|
|
# before an arm runs, and when `$desc` IS `$REPAIR_DESC` the promotion below writes the
|
|
# same string, so the veto could only ever suppress a no-op. Rather than keep a guard no
|
|
# fixture can reach — and rather than delete it on the strength of a check three hundred
|
|
# lines away — the invariant it stood for is enforced HERE, where it is local and
|
|
# provable: pick the strongest description any snapshot shows, then decline to write what
|
|
# is already there.
|
|
# NEVER BURY A ROW AN ALLOW-LISTED REVIEWER WROTE. "Declined" is decided against THIS
|
|
# event's `$BASE_REF`, so a genuine verdict recorded for another base is declined here and
|
|
# is still the right answer for the base it names — the successor run for that base
|
|
# short-circuits on it. Burying it costs a manual re-post on an ordinary
|
|
# retarget-onto-the-reviewed-base flow. Membership is tested on the raw creator rather
|
|
# than on `ex_human`, which the base check has already cleared: the question here is who
|
|
# wrote the row, not whether it governs this diff. An off-list row is exactly what this
|
|
# marking exists for, and is left to it.
|
|
for rv in $H10_REVIEWERS; do
|
|
if [ "$rv" = "$ex_creator" ]; then return 0; fi
|
|
done
|
|
# THE STRONGEST FACT ANY SNAPSHOT SHOWS. If the opening read, the current read, or this
|
|
# run's own decision carried the repair sentinel, the mark carries it too: a human-only
|
|
# marker must never be replaced by the machine-clearable one, and this is the only write
|
|
# in the helper that may be stronger than the reconcilable sentinel. It can only ever
|
|
# withhold an exemption.
|
|
mark_desc="$UNVERIFIED_DESC"
|
|
# `$desc` ALONE, because the classification has already collected the others. Round 8
|
|
# promoted on `$pre_desc` and `$ex_desc` as well, and both are unreachable as a sole
|
|
# cause: a repair sentinel at the FIRST read sets `ex_repair`, which forces
|
|
# `desc="$REPAIR_DESC"`; one arriving mid-run is caught by the sentinel guard unless this
|
|
# run is itself writing that string, which is the same condition. Keeping them meant two
|
|
# clauses no fixture could ever distinguish — the shape this branch has spent several
|
|
# rounds either proving or declaring, and here the honest answer is that they are
|
|
# redundant rather than unprovable. `$desc` needs no state test: it is this run''s own
|
|
# decision, and it is `pending` whenever it is that string.
|
|
if [ "$desc" = "$REPAIR_DESC" ]; then mark_desc="$REPAIR_DESC"; fi
|
|
# A MARK THAT WOULD WRITE WHAT IS ALREADY THERE IS NOT A MARK. This is what the deleted
|
|
# veto was really doing on every path that could reach it.
|
|
# THE STATE IS PART OF "ALREADY THERE". Matching text over a `success` does not mean the
|
|
# pending write is present — it means something is impersonating it.
|
|
if [ "$mark_desc" = "$ex_desc" ] && [ "$ex_state" = pending ]; then return 0; fi
|
|
if ! repair_status_to "$mark_desc"; then
|
|
echo "::error::COULD NOT MARK ${CONTEXT} on ${SHA:0:7}. $1 The head carries a ${CONTEXT} that this run did not inherit and it is still authoritative. Post a verdict immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
return 1
|
|
fi
|
|
echo "::error::$1 This head carries a ${CONTEXT} that this run did not inherit, so leaving it alone would keep it authoritative until a successor finishes — and for a PR's FIRST push no successor is queued at all. Marked it with '${mark_desc}'."
|
|
return 0
|
|
}
|
|
count_pr_mutations
|
|
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 does NOT post that classification. The retarget fired an 'edited' event, so a successor run is already queued and will write the authoritative status for ${SHA:0:7}."
|
|
mark_declined_row_if_any "PR #${PR} was retargeted while this job was classifying." || exit 1
|
|
exit 0
|
|
fi
|
|
# THE HEAD AXIS (ersatztv#803/#664). Separate arm, separate message, same rule — see the
|
|
# long note at `count_pr_mutations`. A push during this window means the file enumeration
|
|
# this run classified may have been paged across two different heads, and `.head.sha`
|
|
# equality at both ends cannot see that when the head was restored. The count can.
|
|
#
|
|
# THIS ARM IS NOT REDUNDANT WITH `pr-changed-files.sh`'s OWN CHECK, which is the reading to
|
|
# guard against: that check compares the head to `$SHA` and so catches every one-way move.
|
|
# What it cannot catch is the move that comes BACK. This arm is for the ABA specifically,
|
|
# and it is why the enumerator's contract now says "detects one-way movement" rather than
|
|
# "bound to one head".
|
|
if [ "$retargets_before_ok" = yes ] && [ "$rt_ok" = yes ] && [ "$hp_count" -ne "$pushes_before" ]; then
|
|
echo "::notice::PR #${PR}'s head branch was pushed while this job was classifying (${pushes_before} -> ${hp_count} push events). The file enumeration behind this run's classification may have been paged across more than one head, and a force-push that RESTORED ${SHA:0:7} would leave every sha comparison equal, so this run does NOT post that classification. A push to an OPEN PR fires 'synchronize', so a successor run is normally already queued and will write the authoritative status for whatever head is current — with one exception worth knowing before you conclude this PR is stuck: a PR's FIRST push is recorded about a second after creation and fires no 'synchronize', so an 'opened' run that abstained on it has no successor coming and the PR needs a re-trigger: a push, or a title/body/base edit. A label or a comment fires none of this workflow's triggers and will NOT re-trigger it."
|
|
mark_declined_row_if_any "PR #${PR}'s head branch was pushed while this job was classifying." || exit 1
|
|
exit 0
|
|
fi
|
|
if { [ "$retargets_before_ok" != yes ] || [ "$rt_ok" != yes ]; } && [ "$state" = "success" ]; then
|
|
# ABSTAINING HERE WAS A FAIL-OPEN WHEN THE HEAD ALREADY CARRIED A ROW (ersatztv#849,
|
|
# round 2). This branch is reached only after the classification DECLINED to inherit
|
|
# whatever `review-verdict/h10` the head carries — that is why it is re-deriving — so
|
|
# posting nothing leaves the declined row current. The old message said the context
|
|
# "stays absent", which is true only for a head that had none; on a head carrying a
|
|
# machine or off-list `success` it is the opposite of what happens, and no retarget or
|
|
# push need have occurred, so no successor run is guaranteed either.
|
|
#
|
|
# The sentinel is the right write rather than the exemption: it states what was actually
|
|
# established (this run could not verify its own classification), blocks the merge, and
|
|
# is cleared by the next run that can read the timeline — so a transient outage costs one
|
|
# event rather than a human verdict.
|
|
if replace_unknown_state "Could not establish a trusted retarget/push 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, nor at a single head. 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."; then
|
|
exit 0
|
|
fi
|
|
# THE SENTINEL WRITE FAILED, so nothing marked this head and whatever it carries is still
|
|
# authoritative. Exiting 0 here would report an abstention that did not happen.
|
|
exit 1
|
|
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 FOR EVERY WRITE, not
|
|
# only the exemption `success` (ersatztv#849 route 2). It was `success`-only, on the claim
|
|
# that "`pending` cannot turn a rejection green, so the only write that can cause the
|
|
# damage is the exemption `success`". That claim is false, and the counter-example is
|
|
# ordinary: a docs-only PR hits a transient enumeration failure, so the run writes the
|
|
# GENERIC `pending` — which masks a rejection landing in its own write window exactly as a
|
|
# `success` would, and got no verification because of the state test. The next run then
|
|
# sees an ordinary machine `pending`, re-derives it into an exemption `success`, and the
|
|
# human's row is now BELOW that run's high-water mark and invisible. The damage arrives one
|
|
# event later rather than immediately, which is not the same thing as not arriving.
|
|
#
|
|
# THE COST IS ONE EXTRA WALK OF `/statuses/{sha}` ON EVERY RUN, which is two requests on an
|
|
# ordinary head. Gating it on `success` to save that is what the paragraph above is about.
|
|
#
|
|
# 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.
|
|
#
|
|
# ORDER IS NOT RELIED ON *WITHIN* THE RESULT — the check selects by id against the
|
|
# high-water mark rather than inspecting the top of the list, so a verdict older than the
|
|
# mark is invisible to it no matter where it sits. The read is now the whole list rather
|
|
# than one clamped page.
|
|
#
|
|
# "THE WHOLE LIST" MEANS WHAT WAS PRESENT WHEN EACH PAGE WAS FETCHED, not a snapshot. These
|
|
# are independent offset-paginated GETs with no snapshot token, so a row INSERTED MID-WALK
|
|
# can be missed: under the server default (`created_unix DESC`, measured) a new row lands at
|
|
# position 0 — on a page already read — while everything else shifts down, so the walk
|
|
# re-reads a duplicate and never sees the newcomer. That residual is ACCEPTED, and it is
|
|
# bounded: a row arriving after this job's POST is not one this job overwrote, and being
|
|
# newest it wins on the combined endpoint that branch protection actually reads.
|
|
#
|
|
# `sort=highestindex` WAS TRIED AND WITHDRAWN. It returns `index` ASCENDING (measured), which
|
|
# does close the mid-walk-insert gap — an insert appends at the end, onto a page not yet
|
|
# fetched. But ASC puts the OLDEST rows on page 1, and that inverts the safety of the
|
|
# partial-mark fallback below: an incomplete walk would then take its maximum over the
|
|
# oldest rows, leaving every pre-existing row above the mark and read as "raced" — the
|
|
# spurious STICKY repair this change exists to stop. Under the server default the newest row
|
|
# is on page 1 by construction, so a partial mark is at or very near the true maximum.
|
|
# Closing a bounded gap by re-opening the one that caused #761 is the wrong trade.
|
|
#
|
|
# That matters more here than anywhere else in this job, because this is the ONE path whose
|
|
# failure direction is toward SUCCESS: missing a raced human `failure` leaves a forged green
|
|
# standing over a rejection. Everything below therefore resolves uncertainty to `pending`.
|
|
#
|
|
# 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 REPAIRED (ersatztv#763) — the old treatment left it alone, which
|
|
# is the wrong direction on this path; see below.
|
|
if [ "$max_id_before" -ge 0 ]; then
|
|
page_statuses
|
|
# AN EMPTY HISTORY IS IMPOSSIBLE HERE, so it is not evidence. This job has just POSTed to
|
|
# this sha, and `/statuses/{sha}` returns one row per POST, so a well-formed response
|
|
# saying "no statuses exist" contradicts a write that succeeded. Accepting it yields
|
|
# `raced=0` — a green concluded from a list that cannot be real — on the one path whose
|
|
# failure direction is toward SUCCESS, and silently, since the walk itself reports
|
|
# success. The predecessor was not better in the way it first appears: its
|
|
# `jq -e 'type == "array"'` gate ACCEPTED `[]` and, of the two empty shapes, warned only
|
|
# on `null` (an empty body and a non-array object warned too), so the `[]` half
|
|
# of this fail-open was already unlogged on `main`. What is new is that the paged walk
|
|
# additionally reports such a read as a SUCCESS, which is why the rejection lives here.
|
|
#
|
|
# THIS IS NOT THE WITHDRAWN CURRENCY WITNESS, and the difference is what makes it safe.
|
|
# That mechanism asked "is there ANY row above the mark", which an unrelated newer row
|
|
# satisfied while the rejection stayed hidden, and which fired on schema-valid staleness.
|
|
# This asks only whether the list is EMPTY — a state no unrelated row can produce and no
|
|
# ordering can disguise. It carries neither defect.
|
|
ph_len_after=$(printf '%s' "$ph_rows" | jq -r 'if type == "array" then length else 0 end' 2>/dev/null) || ph_len_after=0
|
|
case "$ph_len_after" in ''|*[!0-9]*) ph_len_after=0 ;; esac
|
|
# WHY the repair fired, so the log can stop asserting something it does not know. The
|
|
# sentinel DESCRIPTION stays generic on purpose — the classification recognises it as a
|
|
# fixed point, so its wording is load-bearing — but the `::error::` beside it need not be.
|
|
#
|
|
# THE EMPTY CASE GETS ITS OWN REASON rather than borrowing the unreadable one. Routing it
|
|
# through "could not be read completely" would print that beside a walk that DID complete,
|
|
# on a validated terminator — the answer was impossible, not unreadable, and an operator
|
|
# reading a sticky sentinel needs to know which.
|
|
# WHICH SENTINEL THE REPAIR WRITES IS PART OF THE ANSWER, not a detail (ersatztv#849).
|
|
# `REPAIR_DESC` asserts that a human verdict existed and was buried; only the arm that
|
|
# actually COUNTED such a row may assert it. Every "I could not check" arm writes the
|
|
# unverified sentinel instead — which blocks the merge just as hard, is just as sticky
|
|
# against a later re-derivation, and unlike the repair sentinel can be reconciled away by
|
|
# the next run instead of requiring a human. Before this the uncertainty arms wrote
|
|
# `REPAIR_DESC` and then had to soften their own message to "no verdict was necessarily
|
|
# overwritten", i.e. the description and the log contradicted each other.
|
|
# `raced_why` IS OPERATOR-FACING, so its initial value is a sentence rather than the bare
|
|
# token `human`. The skip below prints it, and a log line reading "(human)" says nothing
|
|
# to whoever has to act on it.
|
|
raced_why="$RACED_BY_HUMAN"
|
|
repair_desc="$REPAIR_DESC"
|
|
if [ "$ph_ok" = yes ] && [ "$ph_len_after" -eq 0 ]; then
|
|
echo "::warning::The status history for ${SHA:0:7} came back empty after this job posted to it, which cannot be true, so a raced verdict could not be ruled out. Repairing ${CONTEXT} to pending."
|
|
raced_why="the status history came back empty after this job posted to it"
|
|
repair_desc="$UNVERIFIED_DESC"
|
|
raced=1
|
|
elif [ "$ph_ok" != yes ]; then
|
|
# FAIL CLOSED, AND THAT IS A BEHAVIOUR CHANGE (ersatztv#763). This used to warn and leave
|
|
# the exemption green, while #751's page-2 probe (since deleted) repaired on
|
|
# exactly the same uncertainty — the two halves of one check disagreed about which way
|
|
# "I could not read it" resolves. On the one path whose failure direction is toward
|
|
# SUCCESS it has to be `pending`: an unreadable history is not evidence that nothing
|
|
# raced the write.
|
|
#
|
|
# This is affordable ONLY because the walk above is real paging. Under the old probe the
|
|
# common trigger was an ordinary head carrying more than 50 rows, so failing closed then
|
|
# would have cost the exemption on every busy PR. Now "could not establish" means a
|
|
# genuine API failure that survived a retry, or a history past the page cap (950 rows,
|
|
# since the twentieth request must be the empty terminator).
|
|
echo "::warning::Could not establish a complete status history for ${SHA:0:7} after posting, so a human verdict landing during the write window could not be ruled out. Repairing ${CONTEXT} to pending rather than leaving an unverified write standing."
|
|
raced_why="the status history could not be read completely"
|
|
repair_desc="$UNVERIFIED_DESC"
|
|
raced=1
|
|
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`.
|
|
#
|
|
# (a) IS DELIBERATELY BROADER THAN THE INHERITANCE TEST (ersatztv#742), and the reason
|
|
# is NOT the obvious one. "Narrowing it would let a rejection go green" does not hold:
|
|
# a genuine reviewer is on `$H10_REVIEWERS` by construction, so membership here would
|
|
# still count every real verdict, and an off-list row is one this job deliberately
|
|
# overwrote a few lines earlier. That reason is stated here only to be refused, because
|
|
# it is the one that looks obviously right.
|
|
#
|
|
# THE REASON is the misconfiguration case, which is the only way an off-list row is
|
|
# ever a real verdict: a second human account added to the repo and forgotten from the
|
|
# literal. It is the last net on THIS path — not the last net overall, which an earlier
|
|
# comment claimed: a rejection already on the head sits below the mark and is invisible
|
|
# here, and the `pending` route above bypasses this block entirely (ersatztv#849). The
|
|
# two errors within its reach are not
|
|
# symmetric — repairing to `pending` is recoverable by re-posting the verdict, while a
|
|
# wrongly-standing `success` on a head nobody reviewed is not. So it counts any
|
|
# attributable row.
|
|
#
|
|
# THE COST, stated rather than implied: any `Review-verdict:`-shaped row from any
|
|
# account landing inside the write window triggers the STICKY `$REPAIR_DESC` sentinel,
|
|
# and that head loses its exemption until a human runs `post-review-verdict.sh`. The
|
|
# window is the gap between the last-moment re-read and the POST, so this is narrow, but
|
|
# it is a permanent per-sha loss and not an inconvenience.
|
|
#
|
|
# (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 — so a sentinel ABOVE the mark can only have been
|
|
# written by another run mid-flight. (This block runs after EVERY write since #849; the
|
|
# argument never depended on the state, only on what a sentinel at the FIRST read
|
|
# forces, and `$own` excludes this run's own row.)
|
|
# `.creator` IS TYPE-TESTED BEFORE IT IS INDEXED, and the substitution is guarded.
|
|
# `.creator != null and .creator.login` hard-errors ("Cannot index number with string")
|
|
# on a row whose `creator` is any non-object, jq exits 5, and under `set -e` the
|
|
# assignment takes the step down — AFTER the exemption `success` is already posted and
|
|
# with the repair never attempted, leaving a forged green on a head that may carry a
|
|
# rejection. Reproduced (ersatztv#763). `(.creator | type) == "object"` short-circuits jq's
|
|
# `and` before the index, so a malformed row is dropped from the count instead of
|
|
# killing the job, and a well-formed verdict beside it is still counted.
|
|
# `$own` EXCLUDES THIS JOB'S OWN ROW FROM THE SENTINEL ARM (ersatztv#849). The human
|
|
# arm never needed it — an Actions-token POST records `creator: null`, measured — but
|
|
# the sentinel arm matches on a NULL creator by design, so once this block started
|
|
# running for every write it began matching the write it is checking: a run taking the
|
|
# carry-forward path POSTs `$REPAIR_DESC`, then found "a sentinel above the mark",
|
|
# then repaired to the identical description. Harmless in state, wrong in the log, and
|
|
# a second POST on every repaired head.
|
|
#
|
|
# EXCLUDING BY DESCRIPTION, NOT BY ID, because the id of the row just written is not
|
|
# knowable here — the POST response is discarded and a re-read cannot distinguish our
|
|
# row from an identical one. Excluding by description also drops another run that wrote
|
|
# the SAME sentinel, and that is correct rather than merely tolerable: both runs
|
|
# converged on the same fixed point, so there is nothing to repair.
|
|
raced=$(printf '%s' "$ph_rows" | jq -r --arg c "$CONTEXT" --argjson since "$max_id_before" --arg rd "$REPAIR_DESC" --arg own "$desc" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select(((.id | numbers) // 0) > $since)
|
|
| select(
|
|
(((.creator | type) == "object") and ((.creator.login // "") != "")
|
|
and ((.description | type) == "string")
|
|
and (.description | startswith("Review-verdict:")))
|
|
or ((.creator == null) and ((.description // "") == $rd)
|
|
and ((.description // "") != $own))
|
|
)] | length') || raced=""
|
|
# NO "CURRENCY WITNESS" HERE, AND THAT IS A DELIBERATE WITHDRAWAL (ersatztv#763).
|
|
# A draft of this change also asserted that the post-write read must show at least one
|
|
# row above the mark, on the grounds that reaching a validated empty page proves the
|
|
# walk finished but not that it saw the POST just made. Two independent defects came out
|
|
# of that one mechanism, which is the signal to remove it rather than patch it twice:
|
|
#
|
|
# * it did not witness what it claimed. Counting ANY row above the mark accepts an
|
|
# unrelated newer row, so a stale-but-valid snapshot carrying that row and not the
|
|
# human rejection passed the check and left the exemption green — the exact
|
|
# fail-toward-SUCCESS it was added to prevent.
|
|
# * it converted a single transient anomaly into a PERMANENT sentinel. A schema-valid
|
|
# stale read is not retried (only transport failures and non-array bodies are), so
|
|
# one such response cost that head its exemption until a human cleared it.
|
|
#
|
|
# The hazard it addressed is also not one this deployment plausibly has: Gitea here is a
|
|
# single instance with no read replicas, so "the read does not reflect the write" has no
|
|
# mechanism behind it. Removing it restores exactly the pre-change exposure on this
|
|
# path — a non-regression — while the paging above closes the hole #763 is actually
|
|
# about. If a real instance of a stale read ever appears, it needs a witness that
|
|
# identifies THIS job's own row, plus a re-read before repairing.
|
|
fi
|
|
case "$raced" in
|
|
''|*[!0-9]*)
|
|
# FAILS CLOSED (ersatztv#763). "Not acting on it" left the exemption green on a head whose
|
|
# write window could not be inspected at all — the same uncertainty every other branch
|
|
# here resolves to `pending`, resolved the opposite way purely because it arrived as a
|
|
# malformed count rather than a failed read.
|
|
#
|
|
# NO FIXTURE REACHES THIS BRANCH, and that is stated rather than implied. With the
|
|
# type-safe filter above, `raced` is a number for every input the stub can pose, so
|
|
# this arm is defense-in-depth against a future jq or schema change and is proved only
|
|
# in combination (disarming it alone reddens nothing — measured, not assumed).
|
|
echo "::warning::Post-write verification for ${SHA:0:7} returned '${raced}' instead of a count, so a raced verdict could not be ruled out; repairing ${CONTEXT} to pending."
|
|
raced_why="the post-write verification did not return a count"
|
|
repair_desc="$UNVERIFIED_DESC"
|
|
raced=1
|
|
;;
|
|
esac
|
|
|
|
# THE OTHER RUN'S UNVERIFIED SENTINEL (ersatztv#849), the exact analogue of the repair
|
|
# sentinel already counted above and reachable the same way: two runs overlap, B writes
|
|
# the unverified sentinel because IT could not check its own write, and A — whose mark was
|
|
# taken earlier — then posts straight over it. A's count above finds nothing, because the
|
|
# unverified sentinel is neither a `Review-verdict:` row nor `$REPAIR_DESC`, so A leaves an
|
|
# unmarked status where a run had recorded that something on this head is unchecked. A
|
|
# later run re-derives that into an exemption.
|
|
#
|
|
# PROBED ONLY WHEN NOTHING STRONGER RACED. If a human row or a repair sentinel is above
|
|
# the mark, that fact already forces `REPAIR_DESC`, which outranks this one; asking would
|
|
# cost a jq call and could only weaken the answer.
|
|
#
|
|
# It cannot false-fire on a sentinel this run itself is replacing: one present at the
|
|
# FIRST read is either cleared by the reconciliation (and then it is BELOW the mark, which
|
|
# is taken afterwards) or carried forward as this run's own description. Either way it is
|
|
# not a row above the mark that this run did not expect.
|
|
if [ "$raced" -eq 0 ]; then
|
|
raced_unverified=$(printf '%s' "$ph_rows" | jq -r --arg c "$CONTEXT" --argjson since "$max_id_before" --arg ud "$UNVERIFIED_DESC" --arg own "$desc" \
|
|
'[.[] | select(type == "object")
|
|
| select(.context? == $c)
|
|
| select(((.id | numbers) // 0) > $since)
|
|
| select((.creator == null) and ((.description // "") == $ud)
|
|
and ((.description // "") != $own))] | length') || raced_unverified=""
|
|
case "$raced_unverified" in
|
|
''|*[!0-9]*)
|
|
echo "::warning::The unverified-sentinel probe for ${SHA:0:7} returned '${raced_unverified}' instead of a count, so it could not be ruled out that this write replaced another run's sentinel; repairing ${CONTEXT} to pending."
|
|
raced_why="the unverified-sentinel probe did not return a count"
|
|
repair_desc="$UNVERIFIED_DESC"
|
|
raced=1 ;;
|
|
0) ;;
|
|
*)
|
|
raced_why="another run recorded an unverified write on this head and this run posted over it"
|
|
repair_desc="$UNVERIFIED_DESC"
|
|
raced=1 ;;
|
|
esac
|
|
fi
|
|
# NEVER WRITE A WEAKER DESCRIPTION THAN THIS RUN ALREADY DECIDED (ersatztv#849, round
|
|
# 2). `repair_desc` is set to the reconcilable sentinel by every "could not check" arm,
|
|
# and since this block started running for EVERY write it also runs after a
|
|
# carry-forward write of `$REPAIR_DESC`. One transient post-write read then rewrote that
|
|
# head with the strictly weaker, machine-clearable sentinel — the exact reverse of the
|
|
# ordering rule the classification chain states, and the input to a two-step green if
|
|
# anything later cleared it wrongly.
|
|
if [ "$desc" = "$REPAIR_DESC" ]; then repair_desc="$REPAIR_DESC"; fi
|
|
# NORMALISE FIRST, THEN ACT ONCE. Setting `raced=1` inside a `case` arm and expecting the
|
|
# repair to happen in a SIBLING arm does not work — the case has already dispatched. That
|
|
# is why the unusable-count branch above sets the flag and the decision lives out here,
|
|
# where every path that concluded "raced" reaches the same single writer.
|
|
# A REPAIR THAT WOULD WRITE WHAT IS ALREADY THERE IS NOT A REPAIR. Reachable once the
|
|
# floor above pins `repair_desc` to this run's own description: without this the job
|
|
# POSTs a duplicate row and logs "Repaired …" for a change that did not happen.
|
|
if [ "$raced" -gt 0 ] && [ "$repair_desc" = "$desc" ]; then
|
|
# THE HUMAN CASE STILL SCREAMS. Skipping the POST is right — the head already carries
|
|
# the strongest marker this job writes — but the `::error::` below is the only place a
|
|
# reviewer is told their verdict was buried and given the command to re-post it, and an
|
|
# early `raced=0` used to swallow it. What is skipped is the WRITE, not the report.
|
|
if [ "$raced_why" = "$RACED_BY_HUMAN" ]; then
|
|
echo "::error::A human ${CONTEXT} verdict landed on ${SHA:0:7} while this job was writing its '${state}' status, and was overwritten. ${CONTEXT} already carries the strongest marker this job writes, so nothing is re-posted — but the verdict itself still needs re-posting: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
else
|
|
echo "::notice::Post-write verification for ${SHA:0:7} could not clear the write window (${raced_why}), and the repair would write the description this run already POSTed, so nothing is re-posted. If another run has written since, its row stands — a convergence on its answer, not a silent loss: both sentinels block the merge."
|
|
fi
|
|
raced=0
|
|
fi
|
|
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 this job's write.
|
|
# DO NOT ASSERT AN OVERWRITE THAT MAY NOT HAVE HAPPENED (ersatztv#763). This
|
|
# message used to be unconditional, so when the old page-2 probe repaired on mere
|
|
# uncertainty it reported a human verdict as overwritten when none existed — which
|
|
# is what made the live PR #761 stall undiagnosable from the status alone.
|
|
#
|
|
# STATE-NEUTRAL WORDING since ersatztv#849: this block now also runs after a `pending`
|
|
# write, so a message naming "this exemption write" would be wrong on the very path
|
|
# that was added, and wrong in the direction of understating what happened.
|
|
if [ "$raced_why" = "$RACED_BY_HUMAN" ]; then
|
|
echo "::error::A human ${CONTEXT} verdict landed on ${SHA:0:7} while this job was writing its '${state}' status, and was overwritten. Downgrading to 'pending' so an explicit human decision cannot be silently green, and marking the head so a later run cannot re-derive it. Re-post it with: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
else
|
|
echo "::error::Could not verify that no human ${CONTEXT} verdict raced this job's '${state}' write on ${SHA:0:7} — ${raced_why}. Downgrading to 'pending' rather than leaving an unverified write standing; no verdict was necessarily overwritten. Clear it with: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
fi
|
|
# A failure HERE leaves the unverified write standing, so `repair_status_to` retries
|
|
# once and this screams if both attempts fail. `set -e` would otherwise kill the job
|
|
# silently, after the write and with nothing left to re-attempt.
|
|
if ! repair_status_to "$repair_desc"; then
|
|
echo "::error::COULD NOT REPAIR ${CONTEXT} on ${SHA:0:7}. A '${state}' status this job could not verify is standing on a head whose write window could not be cleared (${raced_why}). Post a verdict immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
exit 1
|
|
fi
|
|
echo "Repaired ${CONTEXT} to pending on ${SHA:0:7} (${repair_desc})."
|
|
state=pending
|
|
desc="$repair_desc"
|
|
fi
|
|
fi
|
|
|
|
# --- THE POST-POST RETARGET RE-CHECK (ersatztv#849 route 3) --------------------------
|
|
# The fence before the POST narrows "writing while overtaken". It does nothing about being
|
|
# overtaken AFTER writing, and that second window is the one that leaves a PERMANENT forged
|
|
# green rather than a transient one:
|
|
#
|
|
# `main` carries a real `failure` for head H. The PR is retargeted to a scratch base where
|
|
# H is docs-only; run S classifies, derives `success`, and passes its final fence check.
|
|
# The PR is retargeted BACK to `main` while S is paused before its POST. The successor run
|
|
# M — fired by that retarget — sees the base-matching `failure`, short-circuits, and posts
|
|
# nothing. S then resumes and posts its stale `success`. The rejection predates S's
|
|
# high-water mark, so the post-write check above cannot see it, and NO EVENT REMAINS to
|
|
# reclassify. `ci.verdict-write-retarget-fence` described the residual as the
|
|
# sub-round-trip window no API without compare-and-set can close; this is wider than that.
|
|
#
|
|
# RE-COUNTING AFTER THE POST closes the induction. A retarget landing after this check
|
|
# necessarily queues a successor that STARTS after the stale `success` already exists — and
|
|
# a machine-written `success` is re-derived, not inherited, so that successor changes the
|
|
# answer. A retarget landing before it is caught here.
|
|
#
|
|
# WHAT THAT BUYS IS TRANSIENT INSTEAD OF PERMANENT, not "the green never stands" — which is
|
|
# what this comment claimed until round-2 review. The `success` is live between its POST
|
|
# and the repair below, including the timeline round trips in between, so branch protection
|
|
# or an already-scheduled auto-merge can observe it. THAT WINDOW GREW WITH #870 and the
|
|
# sentence is dated accordingly: the walk in between is now 20 requests rather than ~2,
|
|
# about +2.3s on a healthy run at the measured 0.13s per request, and bounded by the
|
|
# pessimum above rather than by anything tighter. Closing THAT window needs a
|
|
# compare-and-set or serialization the API does not offer; what is removed is the case
|
|
# where no event remained to correct it.
|
|
#
|
|
# ONLY WHEN AN EXEMPTION `success` IS WHAT STANDS. A `pending` cannot be a forged green, and
|
|
# `$state` is re-read after the repair above precisely so a write already downgraded is not
|
|
# re-examined. This also bounds the cost: the third timeline walk is paid by exempt PRs only.
|
|
#
|
|
# THE RETARGET AXIS ONLY — the push axis is deliberately NOT re-checked here, and that is
|
|
# not an oversight. A push after the POST moves the head, so this status is no longer on the
|
|
# PR's head and cannot gate its merge; a retarget changes the effective DIFF while the sha
|
|
# stays, which is the whole reason the status remains authoritative for a diff it no longer
|
|
# describes. Fencing pushes here would instead punish the ordinary case — a contributor
|
|
# pushing right after the run — by stranding a sentinel on a sha that later becomes the head
|
|
# again after a revert.
|
|
#
|
|
# AN UNTRUSTED COUNT REPAIRS TOO. The pre-POST fence already refuses to post `success` on an
|
|
# untrusted count, so reaching here with `success` means both earlier counts were trusted; a
|
|
# third read that cannot be trusted is a fresh failure, and "I cannot tell whether the base
|
|
# moved" must not resolve to leaving a green. It resolves to the RECONCILABLE sentinel, so a
|
|
# transient timeline failure costs the exemption only until the next run.
|
|
if [ "$state" = "success" ]; then
|
|
count_pr_mutations
|
|
if [ "$rt_ok" != yes ] || [ "$rt_count" -ne "$retargets_before" ]; then
|
|
echo "::error::PR #${PR} was retargeted, or its retarget count became unreadable, AFTER this job posted ${CONTEXT}=success on ${SHA:0:7} (${retargets_before} -> ${rt_count}, trusted=${rt_ok}). The exemption was computed against a base the PR may no longer target, and a status is per-sha, so it would stand over a diff it does not describe. Replacing it with the unverified-write sentinel."
|
|
if ! repair_status_to "$UNVERIFIED_DESC"; then
|
|
echo "::error::COULD NOT REPLACE ${CONTEXT} on ${SHA:0:7} after a post-write retarget. An exemption 'success' is standing on a head whose base may have changed. Post a verdict immediately: scripts/post-review-verdict.sh ${PR} <VERDICT>"
|
|
exit 1
|
|
fi
|
|
echo "Replaced ${CONTEXT} with the unverified-write sentinel on ${SHA:0:7}."
|
|
state=pending
|
|
desc="$UNVERIFIED_DESC"
|
|
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
|
|
|
|
# THE SILENT-GREEN HALF OF ersatztv#751, WHICH IS THE ACTUAL DEFECT. The stray expression
|
|
# delimiter was one bug in one comment; this step exists because of how the runner REPORTED it.
|
|
# A step it declines to interpolate is dropped and the job concludes `success`, so the gate was
|
|
# dead for three days behind a green tick. Nothing surfaced it: the workflow's own status
|
|
# context said success, and `review-verdict/h10` was merely ABSENT — which reads as "not
|
|
# reviewed yet" on a normal PR and is indistinguishable from the correct pending state.
|
|
#
|
|
# This turns that class of failure — dropped for ANY reason, not only an interpolation failure
|
|
# — into a red job. It asserts execution STARTED, never that it completed: the classifier has
|
|
# several legitimate `exit 0` abstention paths (a human verdict landed mid-run, a mid-run
|
|
# retarget, a sentinel appearing) and treating those as failures would redden ordinary PRs.
|
|
#
|
|
# `if: always()` so it also runs when the classifier failed on purpose — its fail-closed paths
|
|
# exit non-zero, the job is already red, and this step then finds the marker and stays quiet
|
|
# rather than piling a misleading second error on top.
|
|
#
|
|
# ITS OWN BODY MUST STAY EXPRESSION-FREE. A guard that can be dropped by the very mechanism it
|
|
# guards against is worse than none, because its absence is also silent. No delimiters here, no
|
|
# illustrative snippets, and `always()` is written bare as `if:` requires — the static test in
|
|
# scripts/tests/test_pr_changed_files.py holds this to it.
|
|
#
|
|
# THAT A LATER STEP STILL RUNS AFTER AN EARLIER ONE IS DROPPED IS MEASURED, not assumed — it is
|
|
# the premise this guard stands on, and the #751 report could not settle it because the
|
|
# classifier was the job's last step, leaving nothing subsequent to observe. Established on this
|
|
# instance by a scratch-base probe with a negative control (Gitea 1.27.1, 2026-08-06): run 1863
|
|
# dropped the classifier on a reintroduced bad payload, logged `evaluating expression 'always()'
|
|
# -> true`, ran THIS step, and the job concluded `failure`. Run 1866 is the positive control —
|
|
# the classifier ran, posted its exemption, and this step found the marker at
|
|
# `/tmp/h10-classifier-started-1866-1`, confirming the run-keyed path resolves.
|
|
#
|
|
# Had the runner dropped the remaining steps too, this guard could not work and the body would
|
|
# have had to move into `scripts/`, where a one-line `run:` makes the class unreachable. It does
|
|
# not, so it stays here. Re-measure if the runner is upgraded: this is the one assumption whose
|
|
# failure is silent again.
|
|
- name: Assert the classifier actually executed
|
|
if: always()
|
|
run: |
|
|
set -euo pipefail
|
|
marker="${RUNNER_TEMP:-${GITHUB_WORKSPACE:-/tmp}}/h10-classifier-started-${GITHUB_RUN_ID:-norunid}-${GITHUB_RUN_ATTEMPT:-1}"
|
|
if [ ! -f "$marker" ]; then
|
|
echo "::error::The classify step did not execute AT ALL — no start marker at ${marker}. The runner dropped it (an interpolation failure over the run: body does this and still reports the job green; see ersatztv#751) or it was skipped. review-verdict/h10 has NOT been posted for this head, so exempt PRs (Renovate, docs-only) are silently unmergeable. Failing the job so this is visible instead of green."
|
|
exit 1
|
|
fi
|
|
echo "The classify step executed (start marker present at ${marker})."
|