ci(release): the milestone gate is blind to a revert, vouches for a no-op, and calls HEAD the release

[marevol]

This PR closes two of the four ways a milestone's "MERGED" claim goes wrong,
and states the other two in the gate's own output rather than leaving a reader
to infer completeness. The gate asks an ancestry question; every failure is
about content, and the two coincide only in the first row.

class	instance	ancestry alone	this PR
merged, never reached main	#245	catches	unchanged
reached main, then reverted	#259, #261	blind — reports LANDED	refused
merges cleanly, moves no bytes	#277	blind — and vouches for it	refused
removed later with no revert trailer	—	blind	declared unchecked
HEAD is main plus a smuggled commit	R9-P3	blind — names the smuggled SHA as verified	disclaimed, owner named

The last row is not a content question at all: it is the gate answering a true
question about the wrong tree. It is not enforced here because
check-release-tag.sh owns it and this script is a documented local pre-flight
run from a branch — but that owner's implementation is not on main: #259
added it, #276 reverted it, #277's re-land is one of the no-ops this PR now
refuses. The gate says so out loud.

---

Commit 1 — reverts

check-milestone-landed.sh asks one question: is this PR's merge commit an
ancestor of the tree being released? That detects a PR that NEVER REACHED main
-- #245, stranded on a branch whose base was squash-merged first. It is
structurally blind to the other way the same claim goes wrong: a PR that
REACHED MAIN AND WAS TAKEN BACK OUT. A reverted PR's merge commit stays an
ancestor forever, so the ancestry test reports it landed while the tree has
none of its lines.

Both end in the identical false statement -- the milestone says the release
contains a change the tree does not have -- and only one of them was checked.

This repository contains a live instance right now. PR #276 reverted #259
(90c96f0) and #261 (d0118fc):

git merge-base --is-ancestor 90c96f0 HEAD   -> exit 0   "LANDED"
git merge-base --is-ancestor d0118fc HEAD   -> exit 0   "LANDED"

grep -c 'is-ancestor|not on main' check-release-tag.sh
    at 90c96f0: 3      in the tree: 0
grep -c mariadb src/recotem/training/search.py
    at d0118fc: 5      in the tree: 0

Not a 2.1.0 problem: both carry milestone 2.2.0, so the 2.1.0 check never looks
at them. But on the day 2.2.0 is cut, this gate would green-light a release
missing exactly the content it was written to catch. That is #245's failure
one turn of the crank later, and the reason to fix it before the milestone that
contains the instance comes up rather than after.

The check

Each PR that passes the ancestry test is now also checked for a revert that
still stands, keyed on the canonical This reverts commit <40-hex> trailer --
a record git revert and GitHub's own Revert button both write, so it is
produced by the tooling rather than by a convention anyone has to remember.
--fixed-strings keeps the SHA out of the regex engine.

Two things it deliberately does NOT do:

• A revert of a revert is not reported. "We put it back" leaves the content
  in the tree, and a gate that fails a healthy release is a gate that gets
  switched off. Depth is one un-revert, documented at the function: anything
  deeper is rare enough that the existing waiver is a better answer than more
  recursion, and a loud wrong answer an operator clears by hand beats a silent
  one.
• A revert is not a stranding and does not get the stranding remedy. Being
  told to cherry-pick a change someone deliberately backed out is the wrong
  instruction. The message offers both real options and names the usual one
  first: re-land it and record the replacement, or move the PR off the
  milestone -- which for a deliberate revert is the honest record and clears
  the check on its own.

The relanded-prs.tsv waiver now serves both failure modes, and gained the
matching condition: a replacement clears the original only if its own merge
commit is an ancestor and has not itself been reverted. Without that second
half a reverted re-land would keep clearing the original, so the waiver would
outlive the change it vouches for. The file's header says all of this, and says
not to add a row for a deliberate revert.

Guard

tests/unit/test_check_milestone_landed.py, 15 -> 24 tests, driving the real
script against synthetic repositories with a fake gh, as the existing ones do.

test_ancestry_alone_cannot_see_a_revert is the positive control on the
defect: it asserts a reverted commit is still an ancestor and that the content
really is gone, so nothing below can pass for the wrong reason.

Two run against this repository's own history rather than a fixture, using
#259/#261/#276 -- the shape as someone really made it, not one written to be
caught. They skip rather than fail where the history is absent (a shallow
clone), because "cannot see" and "not present" are different states of
knowledge.

Mutation-tested, each mutation syntax-checked before the run:

revert detection disabled (the shipped, blind behaviour)  -> 5 failed
waiver stops checking whether the replacement was reverted-> 1 failed
depth-1 un-revert dropped (revert-of-a-revert reports)    -> 1 failed
trailer match dropped (any commit in range counts)        -> 8 failed
restored                                                 -> 24 passed

The first row is the one that matters: it is the behaviour on main today, and
it takes the live-history test with it.

2891 passed (+9), 4 deselected; ruff check and ruff format --check clean;
bash -n clean.

Commit 2 — no-op merges, and the declared boundary

P3 found a third class, and it is sharper than the revert one: a PR can merge
cleanly and change nothing, and after merging the gate does not merely fail to
notice -- it ASSERTS the milestone is complete on the strength of it.

class                              instance   ancestry alone
---------------------------------  ---------  -------------------------
merged, never reached main         #245       catches
reached main, then reverted        #259 #261  blind: reports LANDED
merges cleanly, moves no bytes     #277       blind: VOUCHES for it

PR #277 is titled "re-land of #259" and is stacked on #276, which reverted
#259. Its branch therefore reverts #259, reverts #261, then restores #259 --
and the first and third cancel. Measured by merging it:

git merge --no-ff pr/277        -> "Merge made by the 'ort' strategy"
git diff --stat origin/main HEAD-> (empty)
check-release-tag.sh            -> 305 lines, not #259's 555
'is-ancestor|not on main' hits  -> 0, not 3

A PR named for re-landing #259 restores none of it, merges cleanly, and becomes
an ancestor.

It also opens a hole in the revert check in the first commit of this PR, which
is why the two ship together: record that no-op in relanded-prs.tsv as the
re-land of #259 and the waiver clears #259 on the strength of a PR that
restored nothing. The gate would not just miss the gap, it would certify it
closed.

What this adds

contributes_nothing -- the commit's diff against its first parent is empty --
applied in two places: the main loop (a milestone PR that carried nothing is
reported, with its own remedy) and the waiver (a no-op can never clear the PR
it is recorded against). First parent is the right comparison for both shapes
here: for a squash merge it is "what did this add to main", and for a true
merge it is the same question asked of the mainline.

False-positive rate, counted rather than predicted. Over all 329
first-parent commits on main -- 4 of them true merges, 1 a root -- the number
with an empty first-parent diff is ZERO. A gate that cries wolf on ordinary
refactoring is a gate someone switches off, so this is pinned by a test that
re-counts it against the live history and fails if the number ever moves.

What this deliberately does NOT add, and why the output now says so

A fourth shape exists: a later commit removes the change with no revert
trailer -- a rewrite, a refactor, a hand-edit. That is a CONTENT question and
content is materially harder than reachability. The cheap forms do not survive
contact with a real repository: re-applying each PR's diff to see whether it is
a no-op flags every file another PR has legitimately touched since, and
hunk-grepping flags every reformat. It is not attempted.

The two classes taken here were taken precisely because they are exact -- "is
there a revert trailer naming this commit" and "is this commit's diff empty"
are both decidable with no heuristic and no threshold.

So the success message stops implying a completeness the gate does not have.
It was a bare "is an ancestor of ", which reads as "the milestone is
complete". It now enumerates the three questions asked and names the one that
is not, in the gate's own output where a reader will actually meet it:

OK: every merged PR in milestone '2.1.0' is in the tree at <sha>.
  Checked, per PR: the merge commit is an ancestor; no revert of it still
  stands; the merge is not an empty no-op.
  NOT checked: whether a later commit removed the change WITHOUT a
  'This reverts commit' trailer -- a rewrite or a refactor that drops a
  change silently is invisible here. [...]

A gate that announces its own boundary is worth more than one a reader infers
completeness from.

Guard

tests/unit/test_check_milestone_landed.py, 24 -> 30 tests. One builds #277's
actual shape (revert then restore, merged) rather than asserting it, with
fixture self-checks that the merge really is both a no-op and an ancestor. One
is the negative control -- an ordinary change must stay green. One pins the
false-positive count against the live history. One asserts the success message
declares what it does not check.

Mutation-tested, each mutant bash -n-checked first:

M1 revert detection disabled (behaviour on main today)   -> 6 failed
M2 waiver stops checking the replacement for a revert    -> 1 failed
M3 depth-1 un-revert dropped                             -> 1 failed
M4 revert trailer match dropped                          -> 9 failed
M5 no-op detection disabled                              -> 2 failed
M6 waiver stops checking the replacement for a no-op     -> 1 failed
M7 emptiness test inverted to always-true (over-broad)   -> 9 failed
M8 success message stops declaring what is NOT checked   -> 1 failed
restored                                                 -> 30 passed

M7 matters as much as M5: it shows the check is bounded in both directions, so
a future edit cannot quietly turn it into something that flags real work.

2897 passed (+6), 4 deselected; ruff check and ruff format --check clean;
bash -n clean.

Commit 3 — HEAD is not the release tree

R9-P3 found a fourth class, and it is not about a PR's content being missing --
it is this gate answering a true question about the wrong tree.

Every question here is asked of git rev-parse HEAD. "Every milestone PR is an
ancestor of HEAD" stays true when HEAD is main plus something smuggled on top:
the extra commit is not any PR's merge commit, and nothing here looks for
commits that no PR explains. P3 demonstrated it on a real off-main commit
carrying a marker -- the script exited 0 and printed the smuggled SHA in its own
success line as though it were the release.

Reproduced independently here before writing this, on a commit built on top of
7871f9f and not on main:

smuggled HEAD                     9ffec607d8b5
is it on main?                    NO
milestone PRs checked             13
milestone PRs NOT ancestors of it 0

All thirteen pass. The claim the success line makes is true, and much narrower
than it reads.

Why the rule is not enforced here

1. It belongs to the tag guard. check-release-tag.sh runs on the tag,
   where HEAD is always the tagged commit, and fix(release): close six gate holes, and refuse a tag that is not on main #259 implemented it there:
   shallow-clone refusal first (a shallow clone answers ancestry wrongly
   rather than failing), then origin/main else refs/heads/main, then
   merge-base --is-ancestor HEAD <main>. Two owners for one rule is how a
   rule ends up with none.
2. This script is documented as a local pre-flight, run from a branch before
   tagging -- see Usage in the header. A hard "HEAD must be on main" check would
   make its own documented workflow fail.

So this does not add the check. It stops the output implying one.

What it does add

The success line was already enumerating what it checks and one thing it does
not; it now names the second, and names the owner:

NOT checked, 2 of 2: that <sha> is on main. Everything above is asked of
HEAD, and 'every milestone PR is an ancestor of HEAD' stays true when HEAD
is main plus a commit no PR explains. This line is not a statement that the
release is main. check-release-tag.sh owns that rule -- and it is NOT on
main today: #259 added it, #276 reverted it, and #277's re-land is an empty
no-op. It returns with #277's rebuild.

The last sentence is the part that matters and is why this is not merely a
docstring. The protection for P3's class existed and was reverted. #259
added it, #276 backed it out, and #277's re-land is one of the no-ops this same
PR now refuses -- so today NEITHER script checks that the released commit is on
main. Verified: the current check-release-tag.sh makes no ancestry call at
all, and #259's version has the check at line 455. Naming an open hole in the
gate's own output beats leaving a reader to infer completeness from a success
message, which is the whole argument of the commit before this one.

Guard

One test, test_success_message_does_not_present_head_as_main, asserting both
the disclaimer and the owner's name -- the second because a disclaimer that does
not say who does check reads as "nobody does" rather than "someone else does".

And a correction to the test added in the previous commit. Numbering the
disclaimers "1 of 2" and "2 of 2" silently weakened
test_success_message_states_what_is_not_checked, which asserted the shared
substring "NOT checked": with two disclaimers present, deleting the first left
the second satisfying the assertion. Caught by the mutation matrix -- M8 went
from 1 failed to 31 passed -- not by reading. The test now asserts each
disclaimer by its distinguishing marker, and M8 was widened to delete the whole
block rather than one line of it. This is the second species of green-but-empty
guard, introduced and caught inside one PR.

Mutation matrix, all nine dead, each mutant bash -n-checked first:

M1 revert detection disabled (behaviour on main today)  -> 6 failed
M2 waiver stops checking the replacement for a revert   -> 1 failed
M3 depth-1 un-revert dropped                            -> 1 failed
M4 revert trailer match dropped                         -> 10 failed
M5 no-op detection disabled                             -> 2 failed
M6 waiver stops checking the replacement for a no-op    -> 1 failed
M7 emptiness test inverted to always-true (over-broad)  -> 10 failed
M8 the "silently removed" disclaimer deleted            -> 1 failed
M9 the "HEAD is not main" disclaimer deleted            -> 2 failed
restored                                                -> 31 passed

2898 passed (+1), 4 deselected; ruff check and ruff format --check clean;
bash -n clean.

---

Update — brought onto current main

This branch was written when main was 100 commits younger. Rebasing it changed
three things about what it says, and one thing about what the repository has to
record.

.github/relanded-prs.tsv gains two rows, because the new check fires

Run against milestone 2.2.0 on the rebased branch, the gate refuses the release
and names the live pair:

::error::2 PR(s) in milestone '2.2.0' reached main
  and were REVERTED, so v2.2.0 would publish without them:
    #261  d0118fc6  reverted by 504905c7
    #259  90c96f08  reverted by 504905c7

That is the finding working, not a regression. Both changes did come back:
#259 as #297 and #261 as #280, each merged onto current main. The rows record
that, and the waiver re-checks each replacement itself — ancestor, not
reverted, not an empty diff — rather than taking the row's word for it:

2 PR(s) cleared by a recorded re-land:
  #261 -> re-landed by #280 (b2315222)
  #259 -> re-landed by #297 (774745e7)
OK: every merged PR in milestone '2.2.0' is in the tree at <sha>.

Without the rows, 2.2.0 would have been green-lit as containing two changes
whose every line had been removed and restored under other numbers.

#277 is closed, not merged

The header and the contributes_nothing docstring described it in the present
tense as a no-op that merges cleanly and carries nothing — true when this was
written, when #277 was open. It was closed unmerged, so the no-op class now has
no merged instance in this history. The check stays, because nothing but
somebody noticing stopped that merge, and both comments now say which of the
two it is instead of pointing at a PR that is not live.

The "329 first-parent commits" figure is dropped, both places

Same reason #424 gave for dropping the milestone count it replaced: main only
grows, so a number written into a comment is exact the day it is typed and
misleading after the next merge. The claim it supported — zero commits in this
history have an empty first-parent diff — is re-measured on every run of
test_no_commit_in_this_repositorys_history_is_a_false_positive, which is the
copy that cannot go stale.

Conflict resolution

tests/unit/test_check_milestone_landed.py conflicted at the end of the file:
this branch appended its 16 new tests there and #362 appended two of its own
(test_a_full_page_of_results_is_refused_not_checked_partially,
test_one_short_of_the_cap_is_still_checked). Both blocks kept; no test from
either side dropped or edited. check-milestone-landed.sh auto-merged —
#424 touched only the PR_LIMIT comment, which this branch does not.

Verified on the merged tree

pytest tests            3218 passed, 1 skipped, 4 deselected
ruff check / format     clean
bash -n                 clean
milestone gate, 2.2.0   exit 0, both re-lands reported by name