fix(803,664): fence the HEAD alias on the PR timeline's pull_push count
`scripts/pr-changed-files.sh` binds its paged enumeration by re-reading `.base.ref`,
`.base.sha` and `.head.sha` after paging and comparing them to what it expected. That
is a comparison of a value against itself, so it detects movement still in effect at
the end and is blind to an alias. The BASE alias (`main -> S -> main`) was fenced by
#706's monotonic `change_target_branch` count. The HEAD alias was fenced by nothing:
a force-push `H1 -> H2 -> H1` across the paging round trips leaves the final `.head.sha`
comparison equal while the middle pages came from `H2`, so a mixed file list can produce
a docs-only exemption `success` no single head ever justified.
Two issues found this independently — #664 from the #649 cold review, #803 from #778's
read-then-write inventory — which is the argument for fixing it rather than documenting
it a third time.
Measured on the live instance before choosing an option:
* Gitea is 1.27.1. `GET /compare/{base}...{head}` still returns `total_commits` and
`commits` and NO `files` key (probed over a 12-commit range), so pinning the diff to
two shas remains unavailable. This independently confirms #747's re-dating of the
same claim from 1.25.4.
* The timeline records every push as a `pull_push` event carrying
`{"is_force_push": bool, "commit_ids": [...]}`. PR #802 has eighteen (all false),
PR #761 five (three true). An ABA is necessarily two pushes, so the count moves by
two where the sha moves by zero.
* PR #761 really went `8798a1d -> 830a407 -> 8798a1d`. The alias shape occurs in
ordinary force-push-and-revert; no attacker is required.
* The fence does not abstain on its own trigger. Across 20 triggered
(push -> `pull_request_target` run) pairs on PRs #802/#834/#761 the `pull_push` event
predates its own run's `started_at` by 26-102s. That margin is runner queue latency,
so it is a deployment property, not an API guarantee, and is recorded as such.
So `count_retargets` becomes `count_pr_mutations`: one walk, two tallies, one shared
trust flag, a separate fence arm and diagnostic per axis. `is_force_push` is deliberately
not read — an ordinary push also invalidates a mid-flight enumeration, and an ABA's
restoring push can be non-forced when `H1` is an ancestor.
The advisory hook gets the other half of #803: `$sha` was captured once at the top and
every later check (CI status, H10 status, verdict comments) addressed it, so a push
landing across the enumeration was checked against the commit it replaced. `.head.sha`
is now re-read at the SAME hoist and off the SAME response as the base re-read, so the
two axes cannot describe different instants; a moved head denies, an unreadable one asks.
#803's floor — three contracts that asserted more than the code did — lands too:
`pr-changed-files.sh`'s `exit 0` contract line, its head re-read comment, and
`review-verdict.yml`'s call-site claim that `exit 0` means "complete and bound to $SHA".
Mutation-proved rather than asserted. Deleting the head arm reddens 4 tests; keying on
the count's VALUE instead of its MOVEMENT reddens the settled-history test (every PR has
a non-zero push count, so that mutation would withhold every exemption — the #751 shape);
folding both event types into one tally reddens the separation test. An earlier draft of
that separation test claimed to catch the folding mutation and stayed GREEN under it —
it posed only the base-moves direction — so it now poses both, and states which one is
unobservable and why.
Verification: 1109 passed, 2 skipped (`PYTHONPATH=. python3 -m pytest scripts/tests -q`);
ruff check + format clean; decisions-validate OK; YAML and `bash -n` clean on every
touched shell body. No `.cs` touched, so the BOM/format gate does not apply.
fixes #803
fixes #664
Decisions-Edit: yes
This commit is contained in:
@@ -212,6 +212,43 @@ fi
|
||||
# From here on both names are the freshly-confirmed base; they are equal by the check above.
|
||||
base_ref=$base_now
|
||||
live_base=$base_now
|
||||
|
||||
# THE HEAD IS RE-READ AT THE SAME HOIST, FROM THE SAME RESPONSE (ersatztv#803).
|
||||
#
|
||||
# `$sha` comes from the PR snapshot at the top of this hook, and until 2026-08-28 every later check
|
||||
# consumed that captured value: the CI combined status, the `review-verdict/h10` status, and the
|
||||
# verdict-comment classification were all evaluated against `/commits/$sha/status` and `--head $sha`.
|
||||
# A push landing in the gap — which includes the docs-only enumeration's up-to-forty round trips —
|
||||
# was therefore checked against the commit it had just replaced, and the hook would report "a
|
||||
# positive Review-verdict references the current head" about a head that was no longer current.
|
||||
#
|
||||
# This is the SAME defect the base had until #778 hoisted the re-read above, and it is fixed the same
|
||||
# way rather than a different way. Reading `.head.sha` off `$prjson_now` — the response the base
|
||||
# check already fetched — costs NO extra round trip, and it keeps the two axes on ONE snapshot, so
|
||||
# they cannot disagree about which moment they describe. Two separate reads would answer about two
|
||||
# different instants while reading as one check.
|
||||
#
|
||||
# DENY, not ask, and for the same reason the `stale` verdict class denies: a head that moved means
|
||||
# the verdict this hook is about to accept covers an OLDER commit, which is a state we have
|
||||
# positively established rather than failed to establish. An UNREADABLE `.head.sha` is the different
|
||||
# case and asks.
|
||||
#
|
||||
# WHAT THIS DOES NOT CLOSE, said here rather than left to be inferred. A push landing after this
|
||||
# check still passes, exactly as a retarget does — the file's rule against a second re-read applies
|
||||
# unchanged (see the branch-protection block below), because two reads only move the window rather
|
||||
# than closing it. That residual is bounded server-side and this hook is not what bounds it: the new
|
||||
# head has no `review-verdict/h10` status, and that context is REQUIRED on `main`, so Gitea refuses
|
||||
# the merge (#622). The hook's job here is to stop CLAIMING a head is reviewed when it can see that
|
||||
# it is not — an advisory gate that states something false is worse than one that asks.
|
||||
if [ -n "$sha" ]; then
|
||||
sha_now=$(printf '%s' "$prjson_now" | jq -r '.head.sha // ""' 2>/dev/null || true)
|
||||
if [ -z "$sha_now" ]; then
|
||||
decide ask "H10 merge gate: PR #$pr reports no head commit (.head.sha) on re-read, so whether the review verdict still covers the current head could not be confirmed. Check the PR, then merge."
|
||||
fi
|
||||
if [ "$sha_now" != "$sha" ]; then
|
||||
decide deny "H6/H10 merge gate: BLOCKED — PR #$pr's head moved from ${sha:0:7} to ${sha_now:0:7} while this gate was evaluating. Every check formed against ${sha:0:7} — the changed-file enumeration, the CI status and the review verdict — describes a commit that is no longer the one being merged (ersatztv#803). Re-review the current head and run: scripts/post-review-verdict.sh $pr MERGEABLE"
|
||||
fi
|
||||
fi
|
||||
if [ -n "$sha" ]; then
|
||||
# This is the THIRD read of this endpoint in a worst-case hook run (the ordinary-CI branch and the
|
||||
# scheduled-auto-merge branch each do their own). Sharing one snapshot would close a narrow
|
||||
|
||||
@@ -621,7 +621,7 @@ jobs:
|
||||
fi
|
||||
}
|
||||
|
||||
# --- The retarget fence (ersatztv#706 race 1) ----------------------------------------
|
||||
# --- 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
|
||||
@@ -665,10 +665,63 @@ jobs:
|
||||
# trusted ONLY when paging reached a validated EMPTY page. A short page, a non-array body, a
|
||||
# non-numeric length or the page cap all leave `rt_ok=no`, and an untrusted count is treated
|
||||
# below as "cannot tell" rather than as zero.
|
||||
count_retargets() {
|
||||
# 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,
|
||||
# three 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, so it was MEASURED rather than argued. 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: 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 between 26s and 102s, and the job then spends
|
||||
# a checkout and several steps before reaching the count below. That margin is the runner's
|
||||
# queue latency, so this is a property of the DEPLOYMENT rather than of the API contract —
|
||||
# recorded as such in `ci.verdict-write-retarget-fence`, together with what a violation
|
||||
# would look like (an exempt PR that never gets a status, with the head-fence notice in
|
||||
# every run's log).
|
||||
#
|
||||
# The termination argument transfers unchanged from the base axis: a push 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. Abstention hands off.
|
||||
#
|
||||
# ONE WALK, TWO COUNTERS, ONE TRUST FLAG. The two axes share `rt_ok` because they share the
|
||||
# pages it certifies: a walk that never reached a validated 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 kind total=0
|
||||
local page=1 raw n m p kind total=0 ptotal=0
|
||||
while [ "$page" -le 20 ]; do
|
||||
raw=$(gh "$BASE_URL/repos/$REPO/issues/$PR/timeline?limit=50&page=${page}") || return 0
|
||||
if [ -z "${raw//[[:space:]]/}" ]; then return 0; fi
|
||||
@@ -710,17 +763,24 @@ jobs:
|
||||
# read as exhaustion, and no bounded number of round-trips can rule that out.
|
||||
case "$kind" in
|
||||
null)
|
||||
if [ "$page" -gt 1 ]; then rt_ok=yes; rt_count=$total; fi
|
||||
if [ "$page" -gt 1 ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; fi
|
||||
return 0 ;;
|
||||
array) ;;
|
||||
*) return 0 ;;
|
||||
esac
|
||||
n=$(printf '%s' "$raw" | jq -r 'length')
|
||||
case "$n" in ''|*[!0-9]*) return 0 ;; esac
|
||||
if [ "$n" -eq 0 ]; then rt_ok=yes; rt_count=$total; return 0; fi
|
||||
if [ "$n" -eq 0 ]; then rt_ok=yes; rt_count=$total; hp_count=$ptotal; return 0; fi
|
||||
m=$(printf '%s' "$raw" | jq -r '[.[] | select(.type == "change_target_branch")] | length')
|
||||
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')
|
||||
case "$p" in ''|*[!0-9]*) return 0 ;; esac
|
||||
total=$(( total + m ))
|
||||
ptotal=$(( ptotal + p ))
|
||||
page=$(( page + 1 ))
|
||||
done
|
||||
return 0
|
||||
@@ -757,13 +817,13 @@ jobs:
|
||||
# (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_retargets` above and this walk deliberately do NOT share a terminator.
|
||||
# `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_retargets`. A head nothing has posted to
|
||||
# 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
|
||||
@@ -791,7 +851,7 @@ jobs:
|
||||
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_retargets`: `jq -e` reports the truthiness of its last output, so it cannot
|
||||
# `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
|
||||
@@ -804,7 +864,7 @@ jobs:
|
||||
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_retargets` had one, the timeline
|
||||
# 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
|
||||
@@ -836,10 +896,11 @@ jobs:
|
||||
}
|
||||
|
||||
ex_repair=no
|
||||
count_retargets
|
||||
count_pr_mutations
|
||||
retargets_before=$rt_count
|
||||
pushes_before=$hp_count
|
||||
retargets_before_ok=$rt_ok
|
||||
echo "Retarget fence: ${retargets_before} retarget event(s) observed before classifying (trusted=${retargets_before_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
|
||||
@@ -968,12 +1029,27 @@ jobs:
|
||||
# termination only on a validated EMPTY page rather than a merely short one, and head-sha
|
||||
# binding across the paging round-trips. ersatztv#649.
|
||||
#
|
||||
# READ THE EXIT STATUS, NEVER THE STDOUT OF A FAILED RUN. exit 0 means "complete and bound
|
||||
# to $SHA"; anything else means "could not tell" and stdout is meaningless. That the
|
||||
# script happens to print nothing on its failure paths is redundancy, not contract —
|
||||
# `files` is therefore cleared explicitly rather than trusted to be empty. stderr is left
|
||||
# attached to the job log on purpose: its diagnostic is the only thing that distinguishes
|
||||
# a force-push mid-enumeration from a dead API.
|
||||
# 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
|
||||
@@ -1223,10 +1299,11 @@ jobs:
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# THE FENCE ITSELF (ersatztv#706 race 1). Re-count the retargets as late as possible and
|
||||
# refuse to write anything if the PR was retargeted since this run began. See the long note
|
||||
# at `count_retargets` for why this is a handoff rather than a stall, and why the count is
|
||||
# the only key that survives an ABA.
|
||||
# THE 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
|
||||
@@ -1246,13 +1323,27 @@ jobs:
|
||||
# `success` with that rejection below the new mark. Letting it through is still right — the
|
||||
# alternative strands every PR whenever the timeline is unreadable — but the trade is
|
||||
# "immediate block, later re-derivation risk", not "free".
|
||||
count_retargets
|
||||
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 posts NOTHING. The retarget fired an 'edited' event, so a successor run is already queued and will write the authoritative status for ${SHA:0:7}."
|
||||
exit 0
|
||||
fi
|
||||
# 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 posts NOTHING. The push fired a 'synchronize' event, so a successor run is already queued and will write the authoritative status for whatever head is current."
|
||||
exit 0
|
||||
fi
|
||||
if { [ "$retargets_before_ok" != yes ] || [ "$rt_ok" != yes ]; } && [ "$state" = "success" ]; then
|
||||
echo "::error::Could not establish a trusted retarget count for PR #${PR} (before=${retargets_before_ok}, after=${rt_ok}), so an exemption 'success' cannot be shown to have been computed against the PR's current base. Posting nothing; ${CONTEXT} stays absent, which blocks the merge. NOTE a later run only helps if the cause was transient — a PR whose timeline exceeds the page cap will fail this way on every run, and needs a human verdict."
|
||||
echo "::error::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. Posting nothing; ${CONTEXT} stays absent, which blocks the merge. NOTE a later run only helps if the cause was transient — a PR whose timeline exceeds the page cap will fail this way on every run, and needs a human verdict."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
|
||||
+13
-2
@@ -190,7 +190,11 @@ dropped-step shape. What shows each read unit was exercised is a different obser
|
||||
they are not interchangeable:
|
||||
|
||||
- **`issues: read`** — both runs logged `Retarget fence: 0 retarget event(s) … (trusted=yes)`, which
|
||||
requires `count_retargets` to have walked `/issues/{n}/timeline`.
|
||||
requires the timeline walk to have read `/issues/{n}/timeline`. That log line and the function
|
||||
behind it were renamed by ersatztv#803, which added the head axis to the same walk: the line now
|
||||
reads `Mutation fence: N retarget event(s) and M push event(s) … (trusted=…)` and the function is
|
||||
`count_pr_mutations`. The observation above is left as it was recorded — it is what those runs
|
||||
printed — but grep for the new strings when reproducing it.
|
||||
- **`pull-requests: read`** — the positive control posted `=success`, and a `success` is reachable
|
||||
only through `complete=yes`, which requires `scripts/pr-changed-files.sh` to have paged
|
||||
`/pulls/{n}` and `/pulls/{n}/files` to a validated end. The fence and the start marker do **not**
|
||||
@@ -1645,6 +1649,12 @@ measured rather than assumed. Gitea 1.25.4 auto-cancels superseded `push` runs o
|
||||
statusless with nothing left to re-trigger it. Full measurements and the two surviving residuals:
|
||||
`ci.verdict-write-retarget-fence`.
|
||||
|
||||
Since ersatztv#803 that fence counts **two** event types on the one timeline walk:
|
||||
`change_target_branch` for the base alias (`main -> S -> main`) and `pull_push` for the HEAD alias
|
||||
(a force-push `H1 -> H2 -> H1` spanning `pr-changed-files.sh`'s paging, which leaves every sha
|
||||
comparison equal). Either count moving withholds the write. The run log line is
|
||||
`Mutation fence: N retarget event(s) and M push event(s) … (trusted=…)`.
|
||||
|
||||
The `push`-supersession half of that claim stays **1.25.4-dated on purpose** (ersatztv#747,
|
||||
2026-08-28). It is not unobservable, but it is no longer reproducible ON DEMAND. No workflow triggers
|
||||
on a push to a *non-`main`* branch — `docker-build.yml` filters its `push` trigger to `main` plus `v*`
|
||||
@@ -1867,7 +1877,8 @@ assumption this bug falsifies. The strict test reads the raw scalar, and must ne
|
||||
|
||||
⚠️ **A page past the end of `/issues/{n}/timeline` is JSON `null`, not `[]`** — and this instance is
|
||||
not consistent between endpoints (`/issues/{n}/comments` returns `[]` when empty). The retarget
|
||||
fence's `count_retargets` gated on `type == "array"`, so it read the real terminator as *unreadable*:
|
||||
fence's timeline walk (`count_retargets`, renamed `count_pr_mutations` by #803) gated on
|
||||
`type == "array"`, so it read the real terminator as *unreadable*:
|
||||
the walk never reached a validated empty page, `rt_ok` was never `yes` for **any** PR, and the fence
|
||||
therefore withheld **every** exemption `success`. Renovate and docs-only PRs got no status at all —
|
||||
the same user-visible outcome as the dropped step above, by a completely unrelated route. So fixing
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -119,8 +119,11 @@ shipped `review-verdict.yml`, so the gate's own shape is what was measured.
|
||||
- `code: read` -> job failure at the POST, `curl` exit 22 (HTTP >=400 under `-f`), NO status written.
|
||||
|
||||
What each observation licenses, kept separate. `issues: read` was exercised: both runs logged
|
||||
`Retarget fence: 0 retarget event(s) ... (trusted=yes)`, which requires `count_retargets` to have
|
||||
walked `/issues/{n}/timeline`. `pull-requests: read` was exercised: the positive control posted
|
||||
`Retarget fence: 0 retarget event(s) ... (trusted=yes)`, which requires the timeline walk to have
|
||||
read `/issues/{n}/timeline`. (That line and `count_retargets` were renamed by #803, which put the
|
||||
head axis on the same walk: `Mutation fence: N retarget event(s) and M push event(s) ...` and
|
||||
`count_pr_mutations`. The quoted string is what those runs actually printed and is kept verbatim;
|
||||
reproduce against the current names.) `pull-requests: read` was exercised: the positive control posted
|
||||
`=success`, reachable only through `complete=yes`, which requires `scripts/pr-changed-files.sh` to
|
||||
have paged `/pulls/{n}` and `/pulls/{n}/files` to a validated end — the fence and the start marker do
|
||||
NOT show this, the `success` does. Both reached the classify step's start marker, so neither was the
|
||||
|
||||
@@ -78,9 +78,20 @@ in each caller:
|
||||
would otherwise yield a list belonging to no single commit. **This detects ONE-WAY movement only.**
|
||||
An A→B→A force-push round trip restores the expected sha, so the binding holds while the pages came
|
||||
from two different states — see #664. Closing that needs a commit-pinned files endpoint (Gitea has
|
||||
none) or a local diff, not a tighter check here; the guarantee is stated narrowly rather than left
|
||||
none — re-probed at 1.27.1 on 2026-08-28: `compare/{base}...{head}` still returns no `files` key)
|
||||
or a local diff, not a tighter check here; the guarantee is stated narrowly rather than left
|
||||
to read as complete.
|
||||
|
||||
**Fenced at the ENFORCED caller since 2026-08-28 (#803), and only there.** The script's contract is
|
||||
unchanged and still one-way, because nothing checkable inside it can do better. What changed is that
|
||||
`review-verdict.yml` — the caller whose match posts a green required status with nobody in the loop —
|
||||
now refuses to write if the PR timeline's `pull_push` count moved while it classified, a monotonic
|
||||
key an alias cannot defeat (`ci.verdict-write-retarget-fence`). The advisory hook is deliberately not
|
||||
given that fence: its failure mode is a human prompt, and it pays for the gap differently, by
|
||||
re-reading `.head.sha` off the same response as its base re-read and DENYING if the head moved. So
|
||||
the mechanism stays shared and single while the two callers keep buying different amounts of
|
||||
protection with it — the same mechanism-not-policy split this record is about.
|
||||
|
||||
**Base-ref checkout — binds the SCRIPTS to the base, not the workflow itself.** `review-verdict.yml`
|
||||
checks out the PR's BASE ref (`ref: ${{ github.event.pull_request.base.sha }}`,
|
||||
`persist-credentials: false`), never the head, so the *scripts the job executes* — above all
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -63,6 +63,26 @@ follow that no hook-side fix could deliver:
|
||||
- it cannot be satisfied by a workflow merely running successfully, since the required context is the
|
||||
status the workflow *posts*, never the workflow's own job status.
|
||||
|
||||
**Read those three as scoped to the head that has NO status, which is what "by construction" buys and
|
||||
all it buys** (qualified 2026-08-28, ersatztv#803, under `process.check-and-use-pins-a-version`). The
|
||||
sha binding makes it impossible for a *new* commit to inherit an *old* verdict. It says nothing about
|
||||
how a head ACQUIRES a status, and one of the two ways is machine-written: the exempt-class auto-pass
|
||||
in `review-verdict.yml`. That path derives "docs-only" from a paged file enumeration, and a paged
|
||||
enumeration is a read-then-write over remote state with a real window — up to forty round trips, over
|
||||
which the PR's base or head can move and move back. So the honest form of the invariant is:
|
||||
|
||||
> a head cannot inherit a verdict written for another commit; a head CAN be given an exemption
|
||||
> `success` computed over a window in which it was not the only head.
|
||||
|
||||
That window is fenced, not eliminated (`ci.verdict-write-retarget-fence`): the job refuses to write if
|
||||
the timeline's `change_target_branch` or `pull_push` count moved while it classified, which is what
|
||||
catches the `main -> S -> main` and `H1 -> H2 -> H1` aliases that every value comparison reads as
|
||||
"unchanged". Two residuals survive it, both recorded there rather than restated here — a mutation
|
||||
landing between the final pre-write count and the POST (#849), and the fact that the head-fence's
|
||||
freedom from self-triggering rests on a measured 26-102s deployment margin rather than an API
|
||||
guarantee. Neither is reachable through the sha binding this section is about; both are reachable
|
||||
through the exemption write, which is why the scope sentence matters more than the residuals do.
|
||||
|
||||
The comment convention of `release.review-verdict-gate` is unchanged and still load-bearing: it is what a
|
||||
human reads, and it remains the hook's condition (c). `post-review-verdict.sh` writes both from one
|
||||
command so they cannot drift, and re-reads the head after posting the comment — if a commit landed in
|
||||
|
||||
@@ -65,7 +65,7 @@ classifications differ; otherwise the strictest applies and the Note names the e
|
||||
|
||||
| Site | Class | Note |
|
||||
|---|---|---|
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` — head-sha reads (CI status, H10 status, verdict comments) | `UNSAFE-KNOWN` | Every comparison uses the **full** 40-char `.head.sha` (`${sha:0:7}` appears only in human-readable strings), which is the right identifier — but it is captured ONCE from the PR snapshot at the top of the hook, and is stale in two distinct ways. FIRST, within the run: every later check (CI status, H10 status, verdict comments) is evaluated against that captured sha, so a push landing mid-run is checked against the commit it replaced. This is the same defect that WAS live for `$base_ref` until it was re-read before the branch-protection lookup; the sha is not re-read, and closing it symmetrically is tracked in **#803**. SECOND, after the decision: the hook returns `allow` and a separate call performs the merge. "No async window" was the second overclaim cold review removed from this file. Both are bounded the same way — a head the verdict does not cover cannot inherit the sha-bound required status, so the server refuses it — and neither is bounded by anything in this hook. The merge API accepts an optional `head_commit_id`, which would make the call a true CAS; a PreToolUse hook cannot ADD that argument, only refuse without it, and requiring it changes every merge call's shape — tracked as follow-up rather than smuggled in here. Accepted meanwhile because the window is one tool call wide with no scheduler in it, and the server-side per-sha required check still refuses an unreviewed head. |
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` — head-sha reads (CI status, H10 status, verdict comments) | `UNSAFE-KNOWN` | Every comparison uses the **full** 40-char `.head.sha` (`${sha:0:7}` appears only in human-readable strings), which is the right identifier — but it is captured ONCE from the PR snapshot at the top of the hook. It WAS stale in two distinct ways; the first is closed. FIRST, within the run: every later check (CI status, H10 status, verdict comments) was evaluated against the captured sha, so a push landing mid-run — including across the docs-only enumeration's up-to-forty round trips — was checked against the commit it replaced. **Closed 2026-08-28 (#803)**, symmetrically with the base and at the same hoist: `.head.sha` is re-read from `prjson_now`, the response the base re-read already fetches, so the two axes cost one round trip between them and cannot describe two different instants; a moved head DENIES, matching the `stale` verdict class, while an unreadable one asks. SECOND, after the decision: the hook returns `allow` and a separate call performs the merge. "No async window" was the second overclaim cold review removed from this file. The one that remains is bounded the same way it always was — a head the verdict does not cover cannot inherit the sha-bound required status, so the server refuses it — and is not bounded by anything in this hook. Note the re-read narrows the window rather than erasing it: a push landing after the hoist still passes, and the hook deliberately does not re-read a second time (two reads move the window rather than closing it, the same rule the branch-protection block states). The merge API accepts an optional `head_commit_id`, which would make the call a true CAS; a PreToolUse hook cannot ADD that argument, only refuse without it, and requiring it changes every merge call's shape — tracked as follow-up rather than smuggled in here. Accepted meanwhile because the window is one tool call wide with no scheduler in it, and the server-side per-sha required check still refuses an unreviewed head. |
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` — scheduled auto-merge (`merge_when_checks_succeed`) | `UNSAFE-KNOWN` | **Preflight, not a pin** — graded down by cold review, which was right: the hook's own comment concedes the branch-protection read pins nothing, so calling it `PINNED` contradicted this file's definition. What the hook proves is a snapshot; Gitea merges later; and since #778 it also verifies that `review-verdict/h10` is a required check, reading the repo's **full rule list** (never the by-name endpoint, which does no matching and knows nothing about precedence) — nothing can govern the base → deny, unreadable → ask, and a **glob rule that could govern it → ask**, because the hook deliberately does not reimplement Gitea's glob dialect. It also asks when two rule names fold equal, or when either name is non-ASCII, since Gitea's `EqualFold` is Unicode-aware and its rule precedence is not derivable here. That converts an unobserved assumption into an observed precondition and detects drift, but an admin can still weaken the protection *after* the read. Accepted, and the earlier wording here was **circular** — it said the residual was "closed one layer down" by the very branch protection an admin may have removed. It is not closed; it is BOUNDED, and the bound is a trust assumption that should be stated rather than dressed as a mechanism: everything on this path assumes repo-admin branch-protection config is not hostile. If protection is present at preflight and removed afterwards, an unreviewed head can merge, and nothing in this repo would detect it. What the check does buy is that the far commoner case — protection already weakened when the merge is attempted — stops being silent. A PreToolUse hook cannot add `head_commit_id` to the merge call, so it can never convert its own grant into a CAS — it can only refuse, which is what it now does. |
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` — base-retarget detection | `UNSAFE-KNOWN` | `recorded_base` parsed from the H10 status description is compared against the PR's live `.base.ref` (#632), and it deliberately compares the base **ref** rather than `base.sha` — the tip moves on every unrelated merge, so comparing it would deadlock every open PR. The residual is the same ABA the enumerator has: a name can be retargeted away and back, and the comparison cannot see it. Accepted because the alternative that CAN see it is the monotonic event count, which lives in the workflow that writes the enforced status rather than in this advisory hook. |
|
||||
| `.claude/hooks/pretooluse-merge-consent.sh` — `## Done-when` issue-body read | `UNSAFE-KNOWN` | The issue body carries an `updated_at` that is not used, so a box unticked between the read and the merge is invisible. Accepted: the only actor who can edit the issue is the one requesting the merge, so this is a self-inflicted race with no adversary and no silent-failure mode. |
|
||||
@@ -97,7 +97,7 @@ classifications differ; otherwise the strictest applies and the Note names the e
|
||||
| Site | Class | Note |
|
||||
|---|---|---|
|
||||
| `scripts/post-review-verdict.sh` — commit-status write | `PINNED` | Re-reads the PR and compares **both** `.head.sha` and `.base.ref` immediately before the POST, and `die`s (exit 1, no status written) on a mismatch **or on a field it cannot read**. That last clause is new: both comparisons were guarded by `[ -n "$x" ] &&`, so a well-formed 2xx body that merely omitted the field made the check a no-op and the status was posted having confirmed nothing — found by cold review on #778 and regression-tested against the real predecessor, since the redundant `-z` arm alone mutates green. Closes #706 and #632 for this path by read-compare-refuse, not by CAS: Gitea's status API offers no conditional write. Residual closed on the write ORDER since #792: the status is written first and the comment second, so a refusal can no longer leave a verdict comment with no status behind it — the only reachable half-state is a status with no comment, which the merge hook resolves as `ask` (`release.verdict-writes-status-before-comment`). |
|
||||
| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix, graded honestly after cold review: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases; this row previously said "any movement fails", which was stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias is not covered by anything — a force-push `H1 -> H2 -> H1` during pagination leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and no counter moves. That residual is real, unfenced, and stated here rather than papered over; closing it needs a monotonic head-mutation fence or enumeration bound to an immutable tree, neither of which exists today — tracked in **#803**, which also carries the three older contracts that still assert more than this row does. |
|
||||
| `scripts/pr-changed-files.sh` — paged file enumeration | `UNSAFE-KNOWN` | #707's fix, graded honestly after cold review: `.base.ref`, `.base.sha` and `.head.sha` are captured before paging and re-checked after, and any *observed* movement fails the whole enumeration closed rather than emitting a short list. But before-and-after equality is **ABA-vulnerable** — a `main → scratch → main` retarget during paging can return the same ref and, if nothing merged meanwhile, the same base sha, while the pages in between were diffed against the scratch base. The script's own comment says it narrows rather than erases; this row previously said "any movement fails", which was stronger than the code. Accepted here because the enumerator cannot close it alone, but be exact about what the caller-side fence does and does not cover: `ci.verdict-write-retarget-fence` counts `change_target_branch` events, so it catches the BASE alias and **nothing else**. A HEAD alias — a force-push `H1 -> H2 -> H1` during pagination — leaves the final `.head.sha` comparison equal while the middle pages were enumerated against `H2`, and NOTHING inside this script can see it. **Fenced at the enforced caller since #803** (2026-08-28): the same timeline walk now also counts `pull_push`, so a head that moved at all during classification withholds the write. The grade stays `UNSAFE-KNOWN` for the SCRIPT, because the script's own guarantee is unchanged and one-way — a caller that does not run the fence inherits the alias. Enumeration bound to an immutable tree remains unavailable: re-probed at 1.27.1 on 2026-08-28, `compare/{base}...{head}` still returns no `files` key. |
|
||||
| `scripts/select-queue.sh` — issue list, then per-issue `/dependencies` | `UNSAFE-KNOWN` | The open-issue list (labels, milestone, priority) is snapshotted once; per-candidate dependency reads happen seconds later and never re-read the issue's own labels, so an issue claimed `in-progress` in that gap still appears on the shortlist. Accepted: the script authorizes **no write**. The real gate is the four-way claim check in `process.parallel-session-claim`, which runs after selection and re-reads live state by construction. Tightening this would move a check that must be adversarial into a tool that is advisory. |
|
||||
| `scripts/ci-detect-already-validated.sh` — prior-head combined status | `UNSAFE-KNOWN` | Reads the PR head's status and emits `skip=true`, with nothing re-checking before the consuming job runs. Accepted and narrow: the skip elides only **re-running** test/migrations on a tree already validated; the `build` job still builds and pushes unconditionally, so no image ever ships from unvalidated source. |
|
||||
| `scripts/issue-qualification-audit.sh` — paged issue list, then a report | `UNSAFE-KNOWN` | Pages the open-issue list and reports which issues lack a `priority:` label, so like every paged read here its pages can straddle a change and the report can name a state no single instant held. Graded to match `select-queue.sh` rather than `N/A`: the two run the same shape, and the reason offered for accepting `select-queue.sh` — it authorizes no write — cannot simultaneously be the reason this one is out of the class. Accepted on the same terms: it is advisory, session-end, human-read, and the labels it prompts for are applied by hand afterwards. |
|
||||
@@ -144,9 +144,9 @@ classifications differ; otherwise the strictest applies and the Note names the e
|
||||
|
||||
| Site | Class | Note |
|
||||
|---|---|---|
|
||||
| `.gitea/workflows/review-verdict.yml` — status read → status POST | `UNSAFE-KNOWN` | The residual this whole class reduces to. Gitea's status API has no ETag, no If-Match and no expected-previous-state, so read and write cannot be made one operation. Narrowed twice rather than claimed closed: a monotonic `change_target_branch` **event-count** fence refuses to write if the count moved (`ci.verdict-write-retarget-fence`; the count is used because the branch *name* is ABA-vulnerable), and a high-water-mark re-read repairs a `success` posted over a human verdict back to `pending`. The file states the residual window explicitly rather than asserting safety. |
|
||||
| `.gitea/workflows/review-verdict.yml` — status read → status POST | `UNSAFE-KNOWN` | The residual this whole class reduces to. Gitea's status API has no ETag, no If-Match and no expected-previous-state, so read and write cannot be made one operation. Narrowed rather than claimed closed: a monotonic **event-count** fence refuses to write if the PR timeline's `change_target_branch` OR `pull_push` count moved (`ci.verdict-write-retarget-fence`; counts are used because the branch *name* and the head *sha* are both ABA-vulnerable — the head axis added by #803), and a high-water-mark re-read repairs a `success` posted over a human verdict back to `pending`. The file states the residual window explicitly rather than asserting safety. |
|
||||
| `.gitea/workflows/review-verdict.yml` — base-ref checkout | `PINNED` | `ref: ${{ github.event.pull_request.base.sha }}` — a full sha from the fixed event payload, so the PR head cannot supply the workflow definition that judges it. |
|
||||
| `.gitea/workflows/review-verdict.yml` — changed-file enumeration | `UNSAFE-KNOWN` | Delegates to `scripts/pr-changed-files.sh` with the head sha and expected base, and therefore **inherits that row's residual, not a pin** — this row said "inherits that row's pins" while the row it points at was being graded down, which is exactly the stale-cross-reference a multi-round edit produces. Accepted on better terms than the enumerator alone — this is the one caller that also runs the monotonic `change_target_branch` event-count fence (`ci.verdict-write-retarget-fence`) — but only for the BASE axis. The HEAD alias described in the enumerator's row (`H1 -> H2 -> H1` during pagination) is unfenced here too, and this row previously implied the fence covered it. |
|
||||
| `.gitea/workflows/review-verdict.yml` — changed-file enumeration | `UNSAFE-KNOWN` | Delegates to `scripts/pr-changed-files.sh` with the head sha and expected base, and therefore **inherits that row's residual, not a pin** — this row said "inherits that row's pins" while the row it points at was being graded down, which is exactly the stale-cross-reference a multi-round edit produces. Accepted on better terms than the enumerator alone — this is the one caller that also runs the monotonic event-count fence (`ci.verdict-write-retarget-fence`), and since #803 that fence covers BOTH axes: `change_target_branch` for the base alias and `pull_push` for the HEAD alias described in the enumerator's row (`H1 -> H2 -> H1` during pagination). This row said the head alias was unfenced here, which was true until 2026-08-28. What remains is the residual the fence shares with the base axis — a mutation landing between the final pre-write count and the POST (#849) — not an unwatched axis. |
|
||||
| `.gitea/workflows/docker-build.yml` — CI toolchain image | `UNSAFE-KNOWN` | Graded down by cold review, correctly: this file's own definition of `PINNED` names an image **digest**, and `ersatztv-ci:<short-sha>` is a mutable **tag**. A registry tag can be repointed after `ci-image-pin` verifies it and before a job pulls it, and a 7-hex short sha is additionally collision-prone. Accepted rather than fixed here because the exposure needs write access to our own LAN registry — i.e. an attacker already inside the trust boundary — and two controls bound it: jobs never consume `:latest`, and `pr-checks.yml`'s `ci-image-pin` fails the build if the tag drifts from the last commit touching `docker/ci/`. Consuming `image@sha256:…` is the real fix and is the natural companion to **#772**, which already covers the availability half of this tag's weakness. |
|
||||
| `.gitea/workflows/docker-build.yml` — release smoke pull | `UNSAFE-KNOWN` | Pulls `${IMAGE}:${SMOKE_SHORT_SHA}`, the tag this same job pushed moments earlier. The first draft called that `PINNED` on the strength of the job's `concurrency` group; cold review showed the group is **per-ref**, so a branch build and a tag build of the same commit sit in DIFFERENT groups and can publish the same `:<short-sha>` — the smoke step can therefore pull the other run's image. Accepted rather than fixed here because the fix is the same one-line change as the row above (pull `image@sha256:…`, propagated from the push step) and belongs with it; until then no registry row in this file claims to be pinned. |
|
||||
| `.gitea/workflows/docker-build.yml` — `api-docs` / `format` base fetch | `UNSAFE-KNOWN` | Fetches the live base tip to diff generated artifacts, with nothing pinning it. Graded up from `N/A` by cold review, which was right that "advisory" was too quick: these are not branch-protection-required contexts, but the merge-consent hook reads Gitea's **combined** status, and a combined state that is not `success` blocks the auto-grant — so a wrong answer here does participate in merge consent. Accepted because the failure direction is benign: a base that advanced mid-job makes a generated artifact look stale and FAILS the job, costing a re-run, rather than passing something it should not. |
|
||||
|
||||
@@ -20,8 +20,23 @@
|
||||
# CONTRACT
|
||||
# Usage: pr-changed-files.sh <owner> <repo> <pr> <expected-head-sha> <expected-base-ref>
|
||||
# stdout: newline-delimited paths, BOTH sides of every rename, no blank lines. May be empty.
|
||||
# exit 0 the enumeration is COMPLETE and bound to <expected-head-sha> AND <expected-base-ref>.
|
||||
# stdout is authoritative.
|
||||
# exit 0 the enumeration is COMPLETE, and both <expected-head-sha> and <expected-base-ref> were
|
||||
# observed unchanged at BOTH ends of it. stdout is authoritative.
|
||||
#
|
||||
# READ THAT AS THE ONE-WAY GUARANTEE IT IS (ersatztv#803/#664). This line said "bound to
|
||||
# <expected-head-sha>" until 2026-08-28, which claims more than the code can do. Every
|
||||
# binding below compares a value against ITSELF, so it detects movement that is still in
|
||||
# effect at the end and is blind to an ALIAS: `H1 -> H2 -> H1` across the paging round
|
||||
# trips, or `main -> S -> main`, restores the expected value while the middle pages were
|
||||
# enumerated against the other one. Exit 0 means "no movement was OBSERVABLE from here",
|
||||
# not "this list belongs to one head".
|
||||
#
|
||||
# A caller needing the stronger property must fence it with a MONOTONIC key, because a
|
||||
# count cannot alias where a value can. The ENFORCED caller
|
||||
# (`.gitea/workflows/review-verdict.yml`) does exactly that on both axes — it counts
|
||||
# `change_target_branch` and `pull_push` timeline events before and after, and writes
|
||||
# nothing if either moved (`ci.verdict-write-retarget-fence`). The advisory hook does not,
|
||||
# and does not need to: its failure mode is a human prompt, not a green status.
|
||||
# exit 1 the enumeration could NOT be completed or verified. stdout is meaningless — the caller
|
||||
# MUST fail closed (withhold any exemption). A diagnostic goes to stderr.
|
||||
# exit 2 usage error.
|
||||
@@ -45,8 +60,14 @@
|
||||
# {head}` returns `total_commits`/`commits` and NO `files` (measured on 1.25.4, re-confirmed on
|
||||
# 1.27.1 2026-08-28 ersatztv#747). Separately, and NOT re-probed since 1.25.4: a `--depth=1` fetch of
|
||||
# the two shas has no merge base, so a three-dot diff is impossible while a two-dot one over-reports
|
||||
# every commit `main` gained since the branch point (measured, #698). The remainder is covered one level up
|
||||
# instead, by the workflow reclassifying on `edited` rather than trusting a machine-written success.
|
||||
# every commit `main` gained since the branch point (measured, #698). Independently re-probed again on
|
||||
# 1.27.1, 2026-08-28 (ersatztv#803): still `total_commits`/`commits` only, over a 12-commit range.
|
||||
#
|
||||
# The remainder is covered one level up instead, and since ersatztv#803 that cover is explicit on BOTH
|
||||
# axes rather than the base alone: the enforced caller fences its WRITE on the monotonic count of
|
||||
# `change_target_branch` AND `pull_push` timeline events (`ci.verdict-write-retarget-fence`) — which
|
||||
# is the thing an ALIAS cannot defeat — in addition to reclassifying on `edited` rather than trusting
|
||||
# a machine-written success.
|
||||
#
|
||||
# AUTH/TRANSPORT is caller-supplied via env, because the two callers authenticate differently:
|
||||
# ETV_GITEA_TOKEN | GITEA_TOKEN -> `Authorization: token`
|
||||
@@ -240,10 +261,16 @@ if [ "$complete" != yes ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Bind the enumeration to ONE head. Paging is several round-trips; a force-push between them means
|
||||
# page 1 came from head A and page 2 from head B, so the assembled list belongs to no single commit —
|
||||
# B's code page can be skipped entirely while B's docs page reads as a clean short tail. Re-read the
|
||||
# head and refuse if it moved.
|
||||
# Re-read the head and refuse if it MOVED. Paging is several round-trips; a force-push between them
|
||||
# means page 1 came from head A and page 2 from head B, so the assembled list belongs to no single
|
||||
# commit — B's code page can be skipped entirely while B's docs page reads as a clean short tail.
|
||||
#
|
||||
# ONE-WAY ONLY. This said "Bind the enumeration to ONE head" until 2026-08-28, which is the overclaim
|
||||
# ersatztv#664 was filed against and ersatztv#803 carried: `H1 -> H2 -> H1` across the paging window
|
||||
# passes the comparison below, because the value re-read is the value expected, while pages 1 and 2
|
||||
# came from different trees. Nothing checkable HERE closes that — the check would have to compare
|
||||
# against something the API does not offer (see the `compare/` probe above) — so it lives at the
|
||||
# caller as a monotonic event count instead (`ci.verdict-write-retarget-fence`).
|
||||
prjson=$(gq "repos/$owner/$repo/pulls/$pr")
|
||||
if [ -z "${prjson//[[:space:]]/}" ]; then
|
||||
echo "pr-changed-files: could not re-read PR head to bind the enumeration — failing closed" >&2
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
"""Tests for the head re-read in `.claude/hooks/pretooluse-merge-consent.sh` (ersatztv#803).
|
||||
|
||||
`$sha` is captured once from the PR snapshot at the top of the hook, and every later check — the CI
|
||||
combined status, the `review-verdict/h10` status, and the verdict-comment classification — is
|
||||
addressed by it. Between the capture and those checks sits the docs-only enumeration, up to forty
|
||||
round trips. A push landing in that gap was therefore checked against the commit it had just
|
||||
replaced, and the hook would report "a positive Review-verdict references the current head" about a
|
||||
head that was no longer current.
|
||||
|
||||
The fix is deliberately the SAME one the base got in #778, at the SAME hoist and off the SAME
|
||||
response, so the two axes cannot describe two different instants. These tests pin that it fires, that
|
||||
it names both shas, that it does not fire on a quiet run, and that an unreadable head asks rather than
|
||||
denying.
|
||||
|
||||
WHAT IS NOT CLAIMED HERE. The hook cannot detect an ABA — a push away and back leaves `.head.sha`
|
||||
equal at both of its reads, exactly as it does inside `scripts/pr-changed-files.sh`. That case belongs
|
||||
to the monotonic `pull_push` count in `.gitea/workflows/review-verdict.yml`
|
||||
(`ci.verdict-write-retarget-fence`), and is covered in `test_pr_changed_files.py`. No test here
|
||||
implies the hook closes it.
|
||||
|
||||
Observable contract: the hook exits 0 with EMPTY stdout when it has no opinion (passthrough to normal
|
||||
permissioning), and emits a JSON `permissionDecision` otherwise.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
HOOK = REPO_ROOT / ".claude" / "hooks" / "pretooluse-merge-consent.sh"
|
||||
|
||||
SHA = "a9e3e23abf337980ca4c05854f5b1e210099d08b"
|
||||
MOVED = "b7c4d19ffe2210873bb9a0417c6e3f2280551ac4"
|
||||
|
||||
# NOT docs-only, on purpose: the docs-only exemption short-circuits the gate long before the hoist,
|
||||
# so a docs PR would never reach the head check and every test here would pass without exercising it.
|
||||
#
|
||||
# THE MOVE IS SERVED FROM THE SECOND `/pulls/{n}` READ ONWARD, not at a hardcoded read index. The
|
||||
# hook's own two reads are not adjacent — `scripts/pr-changed-files.sh` makes its own in between, in a
|
||||
# separate process sharing this shim — so counting to a specific read would pin an implementation
|
||||
# detail that any refactor of the enumerator would silently break. "Moved once, early, and stayed
|
||||
# moved" is both the realistic shape and the one that needs no such knowledge.
|
||||
CURL_SHIM = r"""#!/usr/bin/env python3
|
||||
import json, os, sys, pathlib, urllib.parse
|
||||
|
||||
state = pathlib.Path(os.environ["STUB_DIR"])
|
||||
args = sys.argv[1:]
|
||||
url = [a for a in args if a.startswith("http")][-1]
|
||||
|
||||
if "/pulls/" in url and "/files" in url:
|
||||
q = urllib.parse.parse_qs(urllib.parse.urlparse(url).query)
|
||||
page = int(q.get("page", ["1"])[0])
|
||||
if page == 1:
|
||||
print(json.dumps([{"filename": "ErsatzTV/Program.cs", "status": "modified"}]))
|
||||
else:
|
||||
print("[]")
|
||||
sys.exit(0)
|
||||
|
||||
if "/status" in url:
|
||||
print(json.dumps({"state": "success", "statuses": [
|
||||
{"context": "review-verdict/h10", "status": "success",
|
||||
"description": "Review-verdict: MERGEABLE @ a9e3e23 (base: main)"}]}))
|
||||
sys.exit(0)
|
||||
|
||||
if "/pulls/" in url:
|
||||
ctr = state / "pr_reads"
|
||||
seen = int(ctr.read_text()) if ctr.exists() else 0
|
||||
ctr.write_text(str(seen + 1))
|
||||
mode = (state / "head_mode").read_text().strip()
|
||||
sha = os.environ["STUB_SHA"]
|
||||
if seen >= 1:
|
||||
if mode == "moved":
|
||||
sha = os.environ["STUB_MOVED"]
|
||||
elif mode == "unreadable":
|
||||
sha = ""
|
||||
body = {"base": {"ref": "main"}, "body": "fixes #1"}
|
||||
if sha:
|
||||
body["head"] = {"sha": sha}
|
||||
print(json.dumps(body))
|
||||
sys.exit(0)
|
||||
|
||||
print("{}")
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hook(tmp_path):
|
||||
bindir = tmp_path / "bin"
|
||||
bindir.mkdir()
|
||||
curl = bindir / "curl"
|
||||
curl.write_text(CURL_SHIM)
|
||||
curl.chmod(0o755)
|
||||
state = tmp_path / "state"
|
||||
state.mkdir()
|
||||
(state / "head_mode").write_text("stable")
|
||||
|
||||
env = dict(os.environ)
|
||||
env["PATH"] = f"{bindir}{os.pathsep}{env['PATH']}"
|
||||
env["STUB_DIR"] = str(state)
|
||||
env["STUB_SHA"] = SHA
|
||||
env["STUB_MOVED"] = MOVED
|
||||
env["ETV_GITEA_TOKEN"] = "stub" # noqa: S105 - deliberately fake; the real credential comes from the environment
|
||||
env["ETV_GITEA_URL"] = "http://gitea.example"
|
||||
env.pop("ETV_GITEA_BASICAUTH", None)
|
||||
|
||||
class Handle:
|
||||
def set_head_mode(self, mode):
|
||||
"""'stable' | 'moved' | 'unreadable' — applied from the SECOND /pulls read onward."""
|
||||
(state / "head_mode").write_text(mode)
|
||||
|
||||
def decision(self, mwcs=False):
|
||||
payload = {"tool_input": {"method": "merge", "owner": "timothy", "repo": "ersatztv", "pull_number": 42}}
|
||||
if mwcs:
|
||||
payload["tool_input"]["merge_when_checks_succeed"] = True
|
||||
r = subprocess.run(["bash", str(HOOK)], input=json.dumps(payload), env=env, capture_output=True, text=True)
|
||||
assert r.returncode == 0, r.stderr
|
||||
return json.loads(r.stdout) if r.stdout.strip() else None
|
||||
|
||||
def reason(self, mwcs=False):
|
||||
d = self.decision(mwcs=mwcs)
|
||||
return "" if d is None else json.dumps(d)
|
||||
|
||||
return Handle()
|
||||
|
||||
|
||||
def test_a_head_that_MOVED_mid_run_denies(hook):
|
||||
hook.set_head_mode("moved")
|
||||
reason = hook.reason()
|
||||
assert "deny" in reason, "the hook kept its opinion about a head that was replaced while it was evaluating"
|
||||
assert SHA[:7] in reason and MOVED[:7] in reason, (
|
||||
"the deny must name BOTH shas; 'the head moved' is not something a reader can act on"
|
||||
)
|
||||
assert "803" in reason, "the deny should cite the issue that explains the window"
|
||||
|
||||
|
||||
def test_positive_control_a_QUIET_head_does_not_trigger_the_head_deny(hook):
|
||||
"""Without this, the test above passes against a hook that denies on every path.
|
||||
|
||||
It very nearly would: this PR is deliberately non-docs, so the gate runs to completion, and the
|
||||
assertion is not "no decision" but "not a decision caused by the HEAD arm".
|
||||
"""
|
||||
hook.set_head_mode("stable")
|
||||
reason = hook.reason()
|
||||
assert "head moved from" not in reason, f"the head arm fired on a run whose head never moved: {reason}"
|
||||
|
||||
|
||||
def test_an_UNREADABLE_head_on_re_read_ASKS_rather_than_denying(hook):
|
||||
"""The two cases are not the same and must not collapse into one decision.
|
||||
|
||||
A head that MOVED is a state we positively established — the verdict covers an older commit,
|
||||
which is what the `stale` verdict class denies for. A head we could not READ is the absence of
|
||||
evidence, and this gate's standing rule is that uncertainty asks. Collapsing them would block
|
||||
merges on a transport blip; collapsing them the other way would grant on one.
|
||||
"""
|
||||
hook.set_head_mode("unreadable")
|
||||
reason = hook.reason()
|
||||
assert "ask" in reason, f"an unreadable head did not fall through to a human: {reason}"
|
||||
assert "deny" not in reason, f"an unreadable head was treated as a moved one: {reason}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mwcs", [False, True], ids=["immediate", "scheduled"])
|
||||
def test_the_head_deny_covers_BOTH_merge_paths(hook, mwcs):
|
||||
"""The twin-miss shape, tested because this exact hook has already been bitten by it.
|
||||
|
||||
The base re-read first landed INSIDE the scheduled-auto-merge branch only, and cold review
|
||||
found the consequence with this repo's own fixture: scheduled+retarget denied while
|
||||
immediate+retarget auto-GRANTED. The head check is placed at the same hoist precisely so it
|
||||
sits ABOVE the point where the two paths diverge — but "it is above the split" is a claim about
|
||||
the source, and the thing worth pinning is the OUTCOME on both paths.
|
||||
"""
|
||||
hook.set_head_mode("moved")
|
||||
reason = hook.reason(mwcs=mwcs)
|
||||
assert "deny" in reason, (
|
||||
f"a head that moved mid-run was not denied on the {'scheduled' if mwcs else 'immediate'} merge path: {reason}"
|
||||
)
|
||||
assert MOVED[:7] in reason, f"the deny does not name the new head: {reason}"
|
||||
@@ -995,12 +995,32 @@ if "/timeline" in url:
|
||||
# before the POST. This is the race-1 window: the PR was retargeted while the job classified.
|
||||
a, b = mode.split(":", 1)[1].split(",")
|
||||
n_before, n_after = int(a), int(b)
|
||||
# THE PUSH AXIS IS INDEPENDENT OF THE RETARGET AXIS (ersatztv#803/#664), and it has to be, or the
|
||||
# head-fence tests could not distinguish which fence fired. Both counts come off the SAME page —
|
||||
# the job makes one walk and tallies two `.type` values — so the stub serves them from one
|
||||
# response, but the two `moves:`/`stable:` knobs are separate.
|
||||
pmode = os.environ.get("STUB_PUSH_MODE", "none")
|
||||
p_before, p_after = 0, 0
|
||||
if pmode.startswith("stable:"):
|
||||
p_before = p_after = int(pmode.split(":", 1)[1])
|
||||
elif pmode.startswith("moves:"):
|
||||
a, b = pmode.split(":", 1)[1].split(",")
|
||||
p_before, p_after = int(a), int(b)
|
||||
ctr = out / "timeline_reads.txt"
|
||||
seen = int(ctr.read_text()) if ctr.exists() else 0
|
||||
ctr.write_text(str(seen + 1))
|
||||
n = n_before if seen == 0 else n_after
|
||||
np = p_before if seen == 0 else p_after
|
||||
# `is_force_push` is emitted because the real endpoint emits it (measured on PR #761), NOT
|
||||
# because the job reads it — the fence counts rows by `.type` and never parses the body. A stub
|
||||
# that omitted it would leave a reader thinking the field is unused by contract rather than by
|
||||
# choice; one that made the count DEPEND on it would test a job we did not write.
|
||||
print(json.dumps([{"id": 1000 + i, "type": "change_target_branch",
|
||||
"old_ref": "main", "new_ref": "scratch"} for i in range(n)]
|
||||
+ [{"id": 1500 + i, "type": "pull_push",
|
||||
"body": json.dumps({"is_force_push": True,
|
||||
"commit_ids": ["a" * 40, "b" * 40]})}
|
||||
for i in range(np)]
|
||||
+ [{"id": 900, "type": "comment"}]))
|
||||
sys.exit(0)
|
||||
|
||||
@@ -1490,6 +1510,7 @@ def _run_classify(
|
||||
status_creator: str | None = "timothy",
|
||||
status_desc: str = "Review-verdict: MERGEABLE @ a9e3e23 (base: main)",
|
||||
timeline_mode: str = "none",
|
||||
push_mode: str = "none",
|
||||
history_mode: str = "none",
|
||||
history_creator: str = "timothy",
|
||||
midrun_creator: str = "timothy",
|
||||
@@ -1530,6 +1551,7 @@ def _run_classify(
|
||||
env["STUB_STATUS_CREATOR"] = status_creator or ""
|
||||
env["STUB_STATUS_DESC"] = status_desc
|
||||
env["STUB_TIMELINE_MODE"] = timeline_mode
|
||||
env["STUB_PUSH_MODE"] = push_mode
|
||||
env["STUB_TIMELINE_TERMINATOR"] = timeline_terminator
|
||||
env["STUB_STATUS_EMPTY_SHAPE"] = status_empty_shape
|
||||
env["STUB_HISTORY_MODE"] = history_mode
|
||||
@@ -2751,6 +2773,136 @@ def test_positive_control_a_QUIET_run_still_posts_its_exemption(tmp_path):
|
||||
)
|
||||
|
||||
|
||||
def test_a_HEAD_ABA_DURING_the_run_posts_NOTHING(tmp_path):
|
||||
"""THE ersatztv#803/#664 test: `H1 -> H2 -> H1` across the enumeration.
|
||||
|
||||
This is the case no sha comparison can see, and the reason the fence keys on a COUNT. The head
|
||||
is force-pushed away and back while the job classifies, so `$SHA` still equals `.head.sha` at
|
||||
both ends — `pr-changed-files.sh` exits 0 and reports a complete, bound enumeration — while the
|
||||
middle pages were served from `H2`. A mixed file list can then produce a docs-only exemption
|
||||
that no single head ever justified.
|
||||
|
||||
TWO push events, not one, because that is what the ABA costs: away, and back. The retarget axis
|
||||
is held at `none` deliberately, so a pass here cannot be the BASE fence firing.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none", push_mode="moves:0,2")
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert posted is None, (
|
||||
"the job posted an exemption for a head that was force-pushed away and back underneath it "
|
||||
f"— the head-mutation fence did not fire. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
# The DISCRIMINATOR, and specifically the head one: the retarget arm also ends in "posted
|
||||
# nothing", so without this the test would pass against a job whose base fence fired for the
|
||||
# wrong reason.
|
||||
assert "head branch was pushed while this job was classifying" in r.stdout, (
|
||||
f"nothing was posted, but NOT via the head fence. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
assert "retargeted while this job was classifying" not in r.stdout, (
|
||||
"the BASE fence fired on a run whose base never moved, so this test is not measuring the "
|
||||
f"head axis. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_SINGLE_head_push_during_the_run_also_posts_NOTHING(tmp_path):
|
||||
"""One push is the ordinary one-way move, and the fence covers it too.
|
||||
|
||||
`pr-changed-files.sh` already fails closed on this via its own sha comparison, so the exemption
|
||||
was withheld before #803 — but through a DIFFERENT mechanism, one that reports an enumeration
|
||||
error rather than a handoff. Asserting the fence arm here pins that the head axis is not
|
||||
narrowed to "two or more pushes" by some future edit reasoning that one-way is already covered.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none", push_mode="moves:0,1")
|
||||
assert posted is None, f"an exemption was posted for a PR pushed mid-classification. Log:\n{r.stdout[-900:]}"
|
||||
assert "head branch was pushed while this job was classifying" in r.stdout, (
|
||||
f"nothing was posted, but NOT via the head fence. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_PR_PUSHED_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_path):
|
||||
"""The head fence keys on MOTION during this run, never on "has ever been pushed".
|
||||
|
||||
This is the test that would go red on the most plausible wrong implementation. EVERY pull
|
||||
request has a non-zero `pull_push` count — it is created by a push, and PR #802 carries
|
||||
eighteen — so a fence keying on the count being non-zero, rather than on it CHANGING, would
|
||||
withhold the exemption from every PR that has ever existed. That is not a subtle regression: it
|
||||
is the #751 shape, where a fence shipped and silently refused every exemption on the instance.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none", push_mode="stable:18")
|
||||
assert posted is not None and posted["state"] == "success", (
|
||||
"a PR with a settled, non-zero push history was refused its exemption — the head fence is "
|
||||
f"testing the count's VALUE instead of its MOVEMENT. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
# Each row: (timeline_mode, push_mode, arm expected to fire, arm that must stay silent).
|
||||
# BOTH DIRECTIONS ARE POSED, because they fail to different mutations and only one of them is
|
||||
# reachable at all. The arms are evaluated base-first, so a tally that folds PUSHES into the retarget
|
||||
# total is caught only by the push-moves row (the base arm fires and pre-empts the head message);
|
||||
# folding retargets into the push total is NOT observable from here for the same ordering reason,
|
||||
# and is deliberately not claimed to be. Stating that is the point — an earlier version of this test
|
||||
# asserted the base-moves row alone while its docstring claimed it caught the folding mutation, and
|
||||
# it stayed GREEN under exactly that mutation.
|
||||
_AXIS_ROWS = [
|
||||
(
|
||||
"moves:0,1",
|
||||
"stable:18",
|
||||
"retargeted while this job was classifying",
|
||||
"head branch was pushed while this job was classifying",
|
||||
),
|
||||
(
|
||||
"stable:2",
|
||||
"moves:0,2",
|
||||
"head branch was pushed while this job was classifying",
|
||||
"retargeted while this job was classifying",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tl_mode,push_mode,expected,silent", _AXIS_ROWS)
|
||||
def test_the_head_and_base_axes_are_counted_SEPARATELY(tmp_path, tl_mode, push_mode, expected, silent):
|
||||
"""One walk, two tallies, and neither may be read for the other.
|
||||
|
||||
The cheapest wrong implementation reuses one tally for both axes; it passes every single-axis
|
||||
test above, because with only one axis in motion a shared counter still moves. What distinguishes
|
||||
it is holding one axis STABLE AND NON-ZERO while the other moves, then demanding that the arm
|
||||
which reports is the one that actually moved.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode=tl_mode, push_mode=push_mode)
|
||||
assert posted is None, f"neither fence fired. Log:\n{r.stdout[-900:]}"
|
||||
assert expected in r.stdout, f"the moving axis was not the one reported. Log:\n{r.stdout[-900:]}"
|
||||
assert silent not in r.stdout, (
|
||||
f"a STABLE axis was reported as having moved — the two counts are sharing a tally. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
def test_the_head_fence_reports_the_OBSERVED_counts_in_its_notice(tmp_path):
|
||||
"""The notice must name the numbers it fenced on.
|
||||
|
||||
A fence whose diagnostic says only "something moved" cannot be triaged from a run log, and this
|
||||
workflow's history is full of runs that posted nothing for reasons nobody could reconstruct
|
||||
afterwards (#751 went unnoticed for exactly that reason). Pin the before/after pair.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="none", push_mode="moves:3,5")
|
||||
assert posted is None, f"the head fence did not fire. Log:\n{r.stdout[-900:]}"
|
||||
assert "(3 -> 5 push events)" in r.stdout, (
|
||||
f"the head fence notice does not report the counts it fenced on. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_PROTECTED_pr_pushed_mid_run_ALSO_posts_nothing(tmp_path):
|
||||
"""The fence covers `pending` as well as the exemption `success`.
|
||||
|
||||
Same rule as the base axis, and the same reason it is one rule rather than a
|
||||
reason-about-it-per-state condition: the successor run is guaranteed either way, so there is
|
||||
nothing to buy by writing a value this run already knows was computed across two heads.
|
||||
"""
|
||||
posted, r = _run_classify(tmp_path, _emitting("ErsatzTV/Program.cs"), timeline_mode="none", push_mode="moves:0,2")
|
||||
assert posted is None, (
|
||||
"a run that saw its head move still wrote a status for a non-exempt PR — the fence is "
|
||||
f"scoped to `success` only. Log:\n{r.stdout[-900:]}"
|
||||
)
|
||||
|
||||
|
||||
def test_a_PR_retargeted_BEFORE_the_run_but_QUIET_during_it_is_STILL_exempt(tmp_path):
|
||||
"""The fence keys on MOTION during this run, never on "has ever been retargeted".
|
||||
|
||||
@@ -2977,7 +3129,9 @@ def test_an_UNTRUSTED_count_withholds_the_exemption_BY_THAT_BRANCH(tmp_path):
|
||||
posted, r = _run_classify(tmp_path, _emitting("docs/a.md"), timeline_mode="unreadable")
|
||||
assert posted is None
|
||||
assert r.returncode == 0, f"the job died rather than declining cleanly: {r.stderr[-800:]}"
|
||||
assert "Could not establish a trusted retarget count" in r.stdout, (
|
||||
# The wording covers BOTH axes since ersatztv#803 — one walk certifies one trust flag, so an
|
||||
# unreadable page abandons the push count and the retarget count together.
|
||||
assert "Could not establish a trusted retarget/push count" in r.stdout, (
|
||||
f"nothing posted, but not via the untrusted-count branch. Log:\n{r.stdout[-800:]}"
|
||||
)
|
||||
|
||||
@@ -3453,7 +3607,8 @@ def test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates(tmp_p
|
||||
from posting anything.
|
||||
|
||||
A page past the end of `/issues/{n}/timeline` is the JSON value `null` on this instance, not `[]`.
|
||||
`count_retargets` gated on `type == "array"` and so treated the real terminator as unreadable: the
|
||||
`count_pr_mutations` (named `count_retargets` then) gated on `type == "array"` and so treated
|
||||
the real terminator as unreadable: the
|
||||
walk never reached a validated empty page, `rt_ok` was never `yes` for ANY pull request, and the
|
||||
fence therefore withheld every exemption `success`. Renovate and docs-only PRs got NO status —
|
||||
the same user-visible outcome as #751, by an unrelated route.
|
||||
@@ -3475,7 +3630,7 @@ def test_the_fence_TRUSTS_the_count_and_POSTS_when_the_timeline_terminates(tmp_p
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert posted is not None, (
|
||||
f"a docs-only PR whose timeline terminates with `{terminator}` got NO status at all. The "
|
||||
"fence could not establish a trusted retarget count, so it withheld the exemption — which "
|
||||
"fence could not establish a trusted retarget/push count, so it withheld the exemption — which "
|
||||
"leaves the required review-verdict/h10 absent and the PR unmergeable with no bypass "
|
||||
f"(ersatztv#751).\n{r.stdout[-1200:]}"
|
||||
)
|
||||
@@ -3728,7 +3883,7 @@ def test_an_UNREADABLE_page_2_refuses_to_conclude_that_no_verdict_exists(tmp_pat
|
||||
"""The two refuse branches of the completeness probe, which cold review found untested.
|
||||
|
||||
Worth a test rather than trusting the shape: this file's history is two consecutive guards that
|
||||
were UNREACHABLE — the `count_retargets` type gate that never saw a real terminator, and a
|
||||
were UNREACHABLE — the timeline walk's type gate that never saw a real terminator, and a
|
||||
full-page check written against a limit of 100 on a server that caps at 50. An unexercised branch
|
||||
here has a track record.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user