Skip to content

Commit 45ca050

Browse files
committed
ci(reaper): guard the MERGED bucket on base.ref === 'main', with its contract harness
`merged_at` says a pull request landed; it does not say WHERE. A stacked PR merged into another dev branch reports `merged_at` exactly like one that landed on `main`, and its content reaches `main` only if that base branch itself later merged -- a second hop the reaper's deliberately flat criterion cannot see. Measured in the #13503 census (2026-09-02) over 678 `copilot/*` branches: of 575 MERGED, 48 merged into something other than `main`; 43 of those have a base that itself later merged, 4 have a base that no longer exists, and 1 -- `copilot/check-action-run-status`, based on a PR that closed WITHOUT merging -- has no confirmed path to `main` at all. Unguarded, the classifier calls that one safe to delete. `reapable` now additionally requires that one of the branch's merged pull requests reports `base.ref === 'main'`. Everything else that merged lands in a new `mergedElsewhere` bucket: rendered in full with the base it merged into, never reaped. A PR record carrying no `base` is counted as NOT main -- the fail-closed direction, because the cost of guessing here is a deleted branch. The guard only NARROWS, and that is asserted rather than promised. `scripts/check-merged-branch-reaper-outcome.mjs` extracts the shipped script from the YAML with a real parser, runs it under doubles the way actions/github-script runs it, and holds four invariants over every scenario -- `reaped => the branch has a merged PR based on main`, the held bucket really is merged-and-not-into-main, the buckets partition the candidates, and the held count is rendered. Its `--self-test` deletes the guard, inverts it, renames the base constant, flips the fail-closed direction and unrecords/unrenders the bucket, and requires the battery to redden for each; it mutates the decisions the guard sits beside too (open-PR precedence, the grace window, the defensive head-ref filter, the protected short-circuit), because a guard that cost something already there would not be a narrowing. No `is-ancestor` is introduced anywhere: the base ref is read off the PR record, which is the instrument the workflow header mandates. The grace window still runs off the newest merge of ALL merged PRs, so the guard cannot shorten it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
1 parent e37456e commit 45ca050

3 files changed

Lines changed: 1135 additions & 9 deletions

File tree

‎.github/workflows/lint.yml‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2972,6 +2972,42 @@ jobs:
29722972
node scripts/check-merge-queue-triage-outcome.mjs --self-test
29732973
node scripts/check-merge-queue-triage-outcome.mjs
29742974
2975+
# Merged-branch reaper classifier contract (#13503). The third harness of
2976+
# this shape, and the one with the sharpest consequence: the classifier in
2977+
# merged-branch-reaper.yml decides which branches a deletion pass would
2978+
# take, and the maintainer's ruling of 2026-09-03 turns its dry-run list
2979+
# into an action a human releases. The workflow's own token grant
2980+
# (`contents: read`) stops it deleting anything; nothing stopped the LIST
2981+
# from being wrong.
2982+
#
2983+
# The ruling added a base-ref guard — `merged_at` says a PR landed, not
2984+
# WHERE, and 48 of the 575 merged `copilot/*` branches in the #13503
2985+
# census merged into another branch, one of them (based on a PR that
2986+
# closed unmerged) with no confirmed path to `main` at all. Read alone,
2987+
# the classifier's diff looks the same whether an edit narrows what gets
2988+
# reaped or widens it, and a dry run over a real repository prints a list,
2989+
# which is not a contract. So the contract is asserted: the shipped script
2990+
# is extracted from the YAML with a real parser (never retyped) and run
2991+
# under doubles the way actions/github-script runs it, as one
2992+
# AsyncFunction body, holding `reaped ⇒ that branch has a merged PR based
2993+
# on main` over every scenario.
2994+
#
2995+
# Its --self-test deletes the guard, inverts it, renames the base it
2996+
# compares against, flips its fail-closed direction, and unrecords and
2997+
# unrenders its bucket — and requires the battery to redden for each,
2998+
# naming the scenario that catches it. It also mutates the decisions the
2999+
# guard sits beside (open-PR precedence, the grace window, the defensive
3000+
# head-ref filter, the protected short-circuit), because a guard that cost
3001+
# something already there is not a narrowing.
3002+
#
3003+
# Invoked as `node` rather than through a `pnpm check:*` alias, same
3004+
# reason as the two steps above: see the GATE INVOCATION IDIOM note at the
3005+
# top of this file.
3006+
- name: Merged-branch reaper classifier contract
3007+
run: |
3008+
node scripts/check-merged-branch-reaper-outcome.mjs --self-test
3009+
node scripts/check-merged-branch-reaper-outcome.mjs
3010+
29753011
# Shard positive-attestation gate (#6082). ci.yml's two aggregate gates
29763012
# used to decide from one `needs.<matrix>.result` word, which cannot carry
29773013
# three shards' verdicts: run 31120902911 read the undocumented

‎.github/workflows/merged-branch-reaper.yml‎

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,44 @@ name: Merged-Branch Reaper (report-only)
2828
# `is-ancestor` proves "nothing would be lost". It does NOT prove the converse,
2929
# and the converse is what a reaper needs.
3030
#
31+
# ## MERGED is not MERGED INTO `main` — the base-ref guard (#13503)
32+
#
33+
# `merged_at` says a pull request landed. It does not say WHERE. A stacked PR
34+
# whose base is another dev branch reports `merged_at` exactly like one that
35+
# landed on `main`, and its content reaches `main` only if that base branch
36+
# itself later merged — a second hop this deliberately flat criterion cannot
37+
# see and must not start guessing at.
38+
#
39+
# Measured, not supposed (the #13503 census, 2026-09-02, over the 678
40+
# `copilot/*` branches): of 575 MERGED, 48 (8.3%) merged into something other
41+
# than `main`. Walking all 48 — 43 have a base that itself later merged to
42+
# `main`, so the content lands via a 2-hop chain; 4 have a base that no longer
43+
# exists and could not be re-verified; and 1, `copilot/check-action-run-status`
44+
# (based on a PR that closed WITHOUT merging), is a MERGED branch whose content
45+
# has no confirmed path to `main` at all. Against that last one the unguarded
46+
# criterion says "safe to delete", and it is wrong.
47+
#
48+
# So `reapable` now additionally requires that at least one of the branch's
49+
# merged PRs reports `base.ref === 'main'`. Everything else that merged goes to
50+
# `mergedElsewhere`: reported in full, never reaped — the 43 safe-by-chain ones
51+
# included. That asymmetry is the point. A reaper's two error directions do not
52+
# cost the same: an unreaped branch costs a line of noise in `ls-remote`, a
53+
# wrongly reaped one costs work that exists nowhere else. The guard buys the
54+
# removal of the one false positive with a handful of false negatives, at a
55+
# measured rate of 48 held per 575 merged.
56+
#
57+
# ⛔ THE GUARD ONLY NARROWS, and that is asserted rather than asked for on
58+
# trust. `scripts/check-merged-branch-reaper-outcome.mjs` extracts THIS script
59+
# from THIS file and drives it under doubles, holding the invariant
60+
# `reapable ⇒ that branch has a merged PR whose base is main` over every
61+
# scenario — so no later edit here can widen what is reaped without turning
62+
# that check red, and its `--self-test` deletes the guard and requires the
63+
# battery to notice.
64+
#
65+
# ⛔ It is NOT `is-ancestor` wearing a new hat. The base ref is read off the PR
66+
# RECORD, which is the instrument the section above mandates; nothing here asks
67+
# git whether a commit is reachable from anything.
68+
#
3169
# ## Why a scheduled sweep and NOT `on: pull_request` — measured, 2026-08-30
3270
#
3371
# The obvious shape is an event-driven reaper on `pull_request: [closed]`. It
@@ -100,12 +138,24 @@ name: Merged-Branch Reaper (report-only)
100138
# `check:*` family or declare why it has none. This one genuinely has none: its
101139
# single step is an API sweep run through `actions/github-script`, not a named
102140
# local verification, so there is no `check:*` script a card's file surface
103-
# could ever schedule. The `pull_request` filter below is not a verification
104-
# step either — it exists so that edits to this file exercise the sweep before
105-
# they merge, and that is not decoration: it is what produced this workflow's
106-
# first real dry-run list ("111 of 335 ... would be deleted", run 33318728567),
107-
# which is the one human look the 2026-08-28 ruling requires before deletion is
108-
# ever enabled. Removing the filter to satisfy the gate would delete that.
141+
# could ever schedule.
142+
#
143+
# ⚠️ Since #13503 there IS a gate over this file — the classifier's contract,
144+
# `scripts/check-merged-branch-reaper-outcome.mjs` — and it does NOT change the
145+
# declaration below, deliberately. That gate does not run HERE; it runs in
146+
# lint.yml beside its two sibling harnesses (the cross-repo closer, the
147+
# merge-queue triage bot), where a red blocks and where the toolchain it needs
148+
# is already installed. This marker answers "does THIS workflow's own steps
149+
# discover a family", and the answer is still no. A card touching this file
150+
# still gets that gate scheduled, because the gate names this path in its own
151+
# source, which is how `dispatch-gates` derives a family in the first place.
152+
#
153+
# The `pull_request` filter below is not a verification step either — it exists
154+
# so that edits to this file exercise the sweep before they merge, and that is
155+
# not decoration: it is what produced this workflow's first real dry-run list
156+
# ("111 of 335 ... would be deleted", run 33318728567), which is the one human
157+
# look the 2026-08-28 ruling requires before deletion is ever enabled. Removing
158+
# the filter to satisfy the gate would delete that.
109159
#
110160
# dispatch-gates: no-check-families -- the only step is an API sweep via actions/github-script; no named local check exists to run
111161

@@ -161,6 +211,10 @@ jobs:
161211
const owner = context.repo.owner;
162212
const repo = context.repo.repo;
163213
const PREFIX = 'claude/';
214+
// The one base a merge has to land on for this reaper to call a
215+
// branch reapable. See "MERGED is not MERGED INTO `main`" above:
216+
// read off the PR record, never from git reachability.
217+
const BASE_REF = 'main';
164218
const graceDays = Number(process.env.GRACE_DAYS || '7');
165219
if (!Number.isFinite(graceDays) || graceDays < 0) {
166220
core.setFailed(`grace_days must be a non-negative number, got ${process.env.GRACE_DAYS}`);
@@ -181,7 +235,7 @@ jobs:
181235
// --- 2. classify by PR state ------------------------------------------
182236
// ⛔ NO `is-ancestor` ANYWHERE. The only question asked of each branch is
183237
// "does a PR whose head ref is this branch report a merged_at?".
184-
const buckets = { reapable: [], grace: [], open: [], closedUnmerged: [], noPr: [], protectedBranch: [] };
238+
const buckets = { reapable: [], grace: [], open: [], mergedElsewhere: [], closedUnmerged: [], noPr: [], protectedBranch: [] };
185239
186240
for (const branch of candidates) {
187241
if (branch.protected) {
@@ -206,6 +260,13 @@ jobs:
206260
const merged = mine.filter((pr) => pr.merged_at);
207261
const open = mine.filter((pr) => pr.state === 'open');
208262
263+
// The base-ref guard (#13503). `merged_at` is WHETHER it landed;
264+
// `base.ref` is WHERE. A PR record with no `base` at all answers
265+
// neither, so it is counted as NOT main — the fail-closed
266+
// direction, because the cost of being wrong here is a deleted
267+
// branch.
268+
const intoMain = merged.filter((pr) => pr.base && pr.base.ref === BASE_REF);
269+
209270
if (merged.length > 0) {
210271
// Newest merge wins: a branch reused across two PRs is safe to reap
211272
// only once the LAST of them has landed and aged out of the grace
@@ -217,10 +278,30 @@ jobs:
217278
// reapable, whatever the merge says.
218279
if (open.length > 0) {
219280
buckets.open.push({ branch: branch.name, pr: open[0].number, note: 'also has a merged PR' });
281+
} else if (intoMain.length === 0) {
282+
// Merged, but never onto `main`. Held and REPORTED, never
283+
// reaped. The grace window is deliberately not consulted:
284+
// this exclusion does not age out, it is answered by a human
285+
// reading the list or by the base branch landing later.
286+
buckets.mergedElsewhere.push({
287+
branch: branch.name,
288+
pr: newest.number,
289+
merged_at: newest.merged_at,
290+
base: (newest.base && newest.base.ref) || '(no base on the PR record)',
291+
});
220292
} else if (new Date(newest.merged_at).getTime() > graceCutoff) {
221293
buckets.grace.push({ branch: branch.name, pr: newest.number, merged_at: newest.merged_at });
222294
} else {
223-
buckets.reapable.push({ branch: branch.name, pr: newest.number, merged_at: newest.merged_at });
295+
// `newest` is still the newest of ALL merged PRs, not of the
296+
// main-based ones: the grace window must not shorten because
297+
// of the guard. The guard only ever REMOVES branches from
298+
// this bucket.
299+
buckets.reapable.push({
300+
branch: branch.name,
301+
pr: newest.number,
302+
merged_at: newest.merged_at,
303+
base_pr: intoMain[0].number,
304+
});
224305
}
225306
} else if (open.length > 0) {
226307
buckets.open.push({ branch: branch.name, pr: open[0].number });
@@ -249,6 +330,7 @@ jobs:
249330
lines.push(`| MERGED, past grace | ${n(buckets.reapable)} | ${pct(buckets.reapable)}% | ✅ would delete |`);
250331
lines.push(`| MERGED, within grace | ${n(buckets.grace)} | ${pct(buckets.grace)}% | held ${graceDays}d |`);
251332
lines.push(`| OPEN PR | ${n(buckets.open)} | ${pct(buckets.open)}% | ⛔ excluded — in use |`);
333+
lines.push(`| MERGED, but not into \`${BASE_REF}\` | ${n(buckets.mergedElsewhere)} | ${pct(buckets.mergedElsewhere)}% | ⛔ excluded — base-ref guard |`);
252334
lines.push(`| CLOSED, unmerged | ${n(buckets.closedUnmerged)} | ${pct(buckets.closedUnmerged)}% | ⛔ excluded — MERGED-only policy |`);
253335
lines.push(`| NO PR at all | ${n(buckets.noPr)} | ${pct(buckets.noPr)}% | ⛔ unreachable by construction |`);
254336
lines.push(`| protected | ${n(buckets.protectedBranch)} | ${pct(buckets.protectedBranch)}% | ⛔ excluded |`);
@@ -264,6 +346,11 @@ jobs:
264346
lines.push(`**OPEN PR (${n(buckets.open)})** — still in use:`);
265347
lines.push(n(buckets.open) ? sample(buckets.open, (r) => `- \`${r.branch}\` — PR #${r.pr}${r.note ? ` (${r.note})` : ''}`) : '_none_');
266348
lines.push('');
349+
lines.push(`**MERGED, but not into \`${BASE_REF}\` (${n(buckets.mergedElsewhere)})** — the base-ref guard. \`merged_at\` says it landed, the base says not here; the content reaches \`${BASE_REF}\` only if the base branch itself merged, and that second hop is not a question this criterion can answer. Held for a human look:`);
350+
lines.push(n(buckets.mergedElsewhere)
351+
? sample(buckets.mergedElsewhere, (r) => `- \`${r.branch}\` — PR #${r.pr} merged into \`${r.base}\` at ${r.merged_at}`, 40)
352+
: '_none_');
353+
lines.push('');
267354
lines.push(`**CLOSED, unmerged (${n(buckets.closedUnmerged)})** — work that never landed; MERGED-only is the default policy:`);
268355
lines.push(n(buckets.closedUnmerged) ? sample(buckets.closedUnmerged, (r) => `- \`${r.branch}\` — PR #${r.pr}`) : '_none_');
269356
lines.push('');
@@ -278,9 +365,10 @@ jobs:
278365
const payload = { generated_at: new Date().toISOString(), grace_days: graceDays, total_branches: allBranches.length, prefix_branches: total, buckets };
279366
require('fs').writeFileSync(`${process.env.RUNNER_TEMP}/branch-reaper-report.json`, JSON.stringify(payload, null, 2));
280367
281-
core.notice(`Dry run: ${n(buckets.reapable)} of ${total} ${PREFIX} branches would be deleted. ${n(buckets.noPr)} have no PR and are unreachable by this criterion. Nothing was deleted.`);
368+
core.notice(`Dry run: ${n(buckets.reapable)} of ${total} ${PREFIX} branches would be deleted. ${n(buckets.mergedElsewhere)} merged somewhere other than \`${BASE_REF}\` and are held by the base-ref guard. ${n(buckets.noPr)} have no PR and are unreachable by this criterion. Nothing was deleted.`);
282369
core.setOutput('reapable', String(n(buckets.reapable)));
283370
core.setOutput('no_pr', String(n(buckets.noPr)));
371+
core.setOutput('merged_elsewhere', String(n(buckets.mergedElsewhere)));
284372
285373
- name: Upload the full classification
286374
# `always()`: a run whose report is missing because an earlier step died

0 commit comments

Comments
 (0)