Skip to content

Commit df6dac8

Browse files
fix(ci): merge-queue-triage stacksFor() walks the stack through green queue groups (#26294)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9KyXC7PqZzbzLfU36Sd9Xoo)_ Fixes #26265 Clause-②: no Metadata-protocol: none ⛔ **Tier H (governed: `.github/workflows/**`).** This PR stays a draft. Landing waits for an approved review from a `GOVERNED_APPROVERS` account; no seat readies it, queues it or arms auto-merge. ## What changed `stacksFor()` in `.github/workflows/merge-queue-triage.yml` used to union two victims only when one victim's queue base was another victim's queue head. A victim stacked on another victim **through green, non-victim queue groups** therefore read as an independent hit. On #26247 that turned one deterministic break (#26064's queued commit) into "2 independent hits". - **The walk.** From each victim's base, `stacksFor()` now walks base to head through every `merge_group` run already indexed in `stackEdges` (green groups included). It unions with any other victim whose head the chain reaches. The index was already in hand, so this adds no API calls. - **The two rules from before are unchanged.** Matching is still full-sha equality only (no prefixes, no time ordering). A victim whose own run is missing from the index is still counted as independent and still counts toward `unresolved`. - **Loops.** The walk keeps a `visited` set, so a loop in the index ends the walk instead of hanging the job. - **Anchor prose.** The anchor's "Queue depth is not evidence" paragraph now says the chain may pass through green groups. The reading guidance gains one line: a later queue build that went green on the same `main` is a control only if its affected-only selection ran the failing package. - **Harness** (`scripts/check-merge-queue-triage-outcome.mjs`): - New scenario **A18** re-scores the #26247 chain: victim #26064 ← green #26158 ← green #26210 ← victim #26198. Expected: 2 PRs, **1 independent hit**, #26064 as root and #26198 as inherited. It also carries a two-run loop behind main's tip, which the walk must end on and must not fuse. - New mutation **M29** stops the walk at its first hop. A18 must go red, and A12 and A13 must stay green as controls. - M16's anchor moved to the new lookup line. - `queueRun()` takes an optional `conclusion`, so green groups are spelled `success`. ## Evidence - **Red before the fix.** Commit `f6efe203` adds A18 on top of `origin/main` `be531f68`, with the workflow unchanged. `node scripts/check-merge-queue-triage-outcome.mjs` exits 1 with 4 failed assertions, all A18. The anchor reads `**2 pull requests** … **2 independent hits**`, and both rows show `independent`. - **Green after the fix** (head `4394d310`): - `node scripts/check-merge-queue-triage-outcome.mjs` exits 0: `OK (141 assertions over 35 scenarios …)`. - `node scripts/check-merge-queue-triage-outcome.mjs --self-test` exits 0: `187 assertions, 33 mutations of the shipped script each driven to red`. On main this was 34 scenarios and 32 mutations; every original scenario is unchanged and still green. ## Gates Local subset, measured at head `4394d310`: - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives **51 commands** for this diff (2 paths, +127/-19 changed lines, under the 3000-line threshold). - **All 51 ran, and all 51 exited 0.** Each exit code was captured before any pipe, with stdin set to `/dev/null`. That includes `check-merge-queue-triage-outcome` with and without `--self-test`, `check:nul-bytes`, `check:self-test-wired`, `check:workflow-step-name-quoting`, `check:pm-governed-merges` and `check:pm-dispatch-gates` (fast tier: 1899 cases, 575 s). - `dispatch-gates --ran` reports: `51 derived, 51 run, 0 NOT-MEASURED, 0 UNRUN`. That zero is derived from the recorded exit codes. - **Not measured locally:** - 7 derived families take a value from the workflow (the shard-attestation and test-completeness argvs, `--verify-required-set`) and have no local invocation. - The path-scheduled `Test Core` job and the type-check lanes run only in CI. - CI on this PR is the reading for all of these. - **No package touched.** No build or typecheck is owed, and no dogfood (the title has no `!` and there is no `(narrowing)` arm). - **Changeset:** `skip-changeset`. This changes in-repo tooling only and publishes nothing. ## Acceptance notes - **The walk also passes through landed history.** A victim's base is usually `main`'s tip, which is itself the head of the queue group that landed it, so the walk keeps going into landed groups inside the 24 h window. That fuses nothing as long as no victim's head ever landed. One case can break that. A CI run that is red only on advisory (non-required) jobs still lands. Its head then sits on `main`, and every later victim with the same key would be read as stacked on it. The old one-hop test already had this exposure for the PR directly behind such a landing; the walk widens it to the rest of the window. I'm noting it here rather than filing it: it is an inference from reading the code, and no real queue run has been seen in that state. No owner is assigned. - `stackEdges` is still not narrowed to the CI workflow, as before. Other workflows' `merge_group` runs on the same commit share its base and head, so they add the same edge twice and can never add a wrong one. ## 维护者速读(草稿) - **改了什么**:合并队列分诊工作流里判断「两个被弹出的 PR 是否同一条投机栈」的函数,现在会沿着窗口内所有队列构建(包括中间变绿、没被弹出的那几组)从 base 一路追到 head,追到另一个被弹出 PR 的 head 就算同一栈。原来只看相邻一跳。锚卡阅读指引加了一句:同一 main 上后来变绿的队列构建,只有它的 affected-only 选择真跑到了失败包,才算对照。 - **为什么改**:#26247 上,一次确定性的断裂(#26064 排队提交)被读成了「2 次独立命中」,差点被当成 flaky 交给错的人。原因是中间两组队列构建没选中失败包所以是绿的,旧算法在那里断了链。 - **风险与代价(含回滚)**:只影响锚卡与分诊评论里「独立命中」的计数和 stack 列,不改触发条件(仍是 ≥2 个 PR 就立锚),不重排、不打标签、不碰任何门禁。不多调 API。可能的偏差方向见 Acceptance notes 第一条(非必需 job 红但落地的构建会让之后同签名的弹出被算成同栈)。回滚就是 revert 本 PR。 - **席位意见**: - **你要做的**:审阅后用 GOVERNED_APPROVERS 账号批准;批准后由席位落地。 --- _Generated by [Claude Code](https://claude.ai/code/session_01TB4mXrHF2oa5RXawpKD7Rj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ab4e30a commit df6dac8

2 files changed

Lines changed: 127 additions & 19 deletions

File tree

‎.github/workflows/merge-queue-triage.yml‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -554,11 +554,19 @@ jobs:
554554
* construction. One deterministic break therefore ejects everyone
555555
* behind it, and the raw victim count measures QUEUE DEPTH.
556556
*
557-
* ⚠️ The test is full-sha equality in the strict direction: one
558-
* victim's base == another victim's head. ⛔ Never a base PREFIX and
559-
* ⛔ never time ordering — both would fuse genuinely independent hits
560-
* into one stack, trading this limb's false positive for a false
561-
* negative, which is strictly worse. The count exists to be true.
557+
* ⚠️ The test is full-sha equality in the strict direction, walked
558+
* base to head: from a victim's base, through every merge_group run
559+
* in `stackEdges` whose head is that sha (green, non-victim groups
560+
* included), until the chain reaches another victim's head. A green
561+
* group between two victims is NOT a break in the stack: its
562+
* affected-only selection may simply never have run the failing
563+
* package, so it carries the broken tree without failing on it.
564+
* Walking only victim-to-victim read one deterministic break as two
565+
* independent hits (#26265). ⛔ Never a base PREFIX and ⛔ never time
566+
* ordering — both would fuse genuinely independent hits into one
567+
* stack, trading this limb's false positive for a false negative,
568+
* which is strictly worse. The count exists to be true. The index is
569+
* data, not git, so the walk keeps a visited set and ends on a loop.
562570
*
563571
* A victim whose runs are not in the index (a run older than the
564572
* window, or a ref that is not a queue ref) is left as its own group:
@@ -582,6 +590,14 @@ jobs:
582590
}
583591
}
584592
593+
// head sha -> the bases of EVERY indexed run that produced it,
594+
// victim or not: the edges the walk below steps back along.
595+
const basesOf = new Map();
596+
for (const e of stackEdges.values()) {
597+
if (!basesOf.has(e.head)) basesOf.set(e.head, new Set());
598+
basesOf.get(e.head).add(e.base);
599+
}
600+
585601
const inherited = new Set();
586602
let unresolved = 0;
587603
for (const [pr, runs] of byPr) {
@@ -590,10 +606,18 @@ jobs:
590606
const e = stackEdges.get(String(id));
591607
if (!e) continue;
592608
resolved = true;
593-
for (const upstream of producers.get(e.base) ?? []) {
594-
if (upstream === pr) continue;
595-
inherited.add(pr);
596-
union(pr, upstream);
609+
const visited = new Set();
610+
const frontier = [e.base];
611+
while (frontier.length > 0) {
612+
const sha = frontier.pop();
613+
if (visited.has(sha)) continue;
614+
visited.add(sha);
615+
for (const upstream of producers.get(sha) ?? []) {
616+
if (upstream === pr) continue;
617+
inherited.add(pr);
618+
union(pr, upstream);
619+
}
620+
for (const b of basesOf.get(sha) ?? []) frontier.push(b);
597621
}
598622
}
599623
if (!resolved) unresolved++;
@@ -839,8 +863,9 @@ jobs:
839863
? [
840864
"⚠️ **Queue depth is not evidence.** The `stack` column is read out of the queue",
841865
'branch names: GitHub builds each queued PR on top of the previous entry, so a',
842-
"build whose BASE commit IS another victim's queue HEAD contains that victim's",
843-
'tree by construction. A single deterministic break therefore ejects every PR',
866+
"build whose BASE chain reaches another victim's queue HEAD (directly, or through",
867+
"green groups queued between them) contains that victim's tree by construction.",
868+
'A single deterministic break therefore ejects every PR',
844869
'behind it, and the raw victim count climbs with QUEUE DEPTH until the owner',
845870
`lands a fix. Start with the ${independent === 1 ? 'root' : 'roots'} above; an `
846871
+ '`inherited` row is a bystander until shown otherwise.',
@@ -867,6 +892,10 @@ jobs:
867892
'opposite diagnoses), decide the cause, and close this issue with the fix or with',
868893
'the reason it is not one.',
869894
'',
895+
'A later queue build that went green on the same `main` is a control only if its',
896+
'affected-only selection ran the failing package; a green group that never selected',
897+
'it carried the same tree without testing it.',
898+
'',
870899
`Last refreshed by queue build [${run.id}](${run.html_url}) (PR #${prNumber}).`,
871900
'',
872901
'---',

‎scripts/check-merge-queue-triage-outcome.mjs‎

Lines changed: 87 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,10 @@
122122
// announcement, create an anchor without having established one does not exist,
123123
// revert the anchor lookup to the `open` + `finding` selection that made a
124124
// GRADED anchor invisible, stop recognising a closed-as-duplicate chain, drop
125-
// the link a regression anchor owes the anchor it regressed from,
126-
// and move the redelivery guard below the aggregation -- and requires the
125+
// the link a regression anchor owes the anchor it regressed from, stop the
126+
// stack walk at its first hop so a green group between two victims splits
127+
// their stack again, and move the redelivery guard below the aggregation --
128+
// and requires the
127129
// battery to go RED for each, naming the scenario it expects. Each mutation
128130
// also asserts its own anchor was PRESENT before substituting: a mutation that
129131
// silently matched nothing would leave the battery green and read exactly like
@@ -567,13 +569,14 @@ const sightingComment = (key, pr, runId) =>
567569
*
568570
* `base` is the sha the queue branch NAME carries (the commit this build was
569571
* stacked on); `head` is the commit the build actually produced. Limb ②'s stack
570-
* test is `one victim's base === another victim's head`, so a fixture that let
571-
* these two default to the same shape could not tell the test from a tautology.
572+
* test walks `base === head` from a victim's base, through any indexed run, to
573+
* another victim's head, so a fixture that let these two default to the same
574+
* shape could not tell the test from a tautology.
572575
*/
573-
const queueRun = (id, pr, base, head) => ({
576+
const queueRun = (id, pr, base, head, conclusion = 'failure') => ({
574577
id,
575578
workflow_id: WORKFLOW_ID,
576-
conclusion: 'failure',
579+
conclusion,
577580
head_branch: `gh-readonly-queue/main/pr-${pr}-${base}`,
578581
head_sha: head,
579582
});
@@ -596,6 +599,24 @@ const INCIDENT = {
596599
head12855: pad('1a24e9778'),
597600
};
598601

602+
/**
603+
* The #26247 anchor's chain, as the card that filed this work recorded it: one
604+
* deterministic break (#26064's queued commit) and a second victim (#26198)
605+
* stacked on it THROUGH two green, non-victim groups (#26158, #26210), whose
606+
* affected-only Test Core never selected the failing package.
607+
*
608+
* Same padding caveat as INCIDENT: the record abbreviates, so the four recorded
609+
* shas are right-padded, and the two the record does not carry (main before
610+
* the queue, #26198's own head) are declared synthetic.
611+
*/
612+
const CHAIN = {
613+
mainTip: pad('5c0ffee'), // synthetic: main before the queue
614+
head26064: pad('17f21e8d'), // #26064's victim head — and #26158's base
615+
head26158: pad('745c276b'), // #26158's GREEN group head — and #26210's base
616+
head26210: pad('7122b0db'), // #26210's GREEN group head — and #26198's base
617+
head26198: pad('d0d0cafe'), // synthetic: #26198's own queue head
618+
};
619+
599620
/**
600621
* The anchor issue an earlier pair already produced.
601622
*
@@ -1078,6 +1099,52 @@ function scenarios(root) {
10781099
];
10791100
},
10801101
},
1102+
{
1103+
id: 'A18',
1104+
name: 'TRANSITIVE STACK: a victim stacked on another THROUGH green, non-victim groups is ONE independent hit',
1105+
// The #26247 reading, re-scored. A ← B ← C ← D with only A and D red:
1106+
// D's base is C's head, C's base is B's head, B's base is A's head. B and
1107+
// C are not victims (their affected-only selection never ran the failing
1108+
// package), so a stack test that unions only through VICTIM runs never
1109+
// sees the chain and reports two independent hits for one break.
1110+
world: () => base({
1111+
runId: 37500000004,
1112+
prNumber: 26198,
1113+
headSha: CHAIN.head26198,
1114+
headBranch: `gh-readonly-queue/main/pr-26198-${CHAIN.head26210}`,
1115+
repoComments: [sightingComment(KEY_A, 26064, 37500000001)],
1116+
queueRuns: [
1117+
queueRun(37500000001, 26064, CHAIN.mainTip, CHAIN.head26064),
1118+
queueRun(37500000002, 26158, CHAIN.head26064, CHAIN.head26158, 'success'),
1119+
queueRun(37500000003, 26210, CHAIN.head26158, CHAIN.head26210, 'success'),
1120+
// The index is data, not git: a loop in it (here behind main's tip,
1121+
// where no victim lives) must END the walk, not hang the job, and
1122+
// must fuse nothing it reaches.
1123+
queueRun(37500000005, 26001, pad('c1c1e'), CHAIN.mainTip, 'success'),
1124+
queueRun(37500000006, 26002, CHAIN.mainTip, pad('c1c1e'), 'success'),
1125+
],
1126+
}),
1127+
check(r, t) {
1128+
const body = r.calls.issueCreate[0]?.body ?? '';
1129+
return [
1130+
t(r.calls.issueCreate.length === 1, `one anchor is filed, got ${r.calls.issueCreate.length}`),
1131+
t(/\*\*2 pull requests\*\*/.test(body),
1132+
`the raw victim count is 2 — the green groups are NOT victims and are not listed, got: ${JSON.stringify(body.slice(0, 240))}`),
1133+
t(/\*\*1 independent hit\*\*/.test(body),
1134+
`and the chain through the green groups scores ONE independent hit, got: ${JSON.stringify(body.slice(0, 240))}`),
1135+
t(/\|\s*#26064\s*\|\s*S1 · root\s*\|/.test(body),
1136+
`#26064 — the queued commit that broke — is the stack ROOT, got: ${JSON.stringify(body)}`),
1137+
t(/\|\s*#26198\s*\|\s*S1 · inherited\s*\|/.test(body),
1138+
'#26198 is marked INHERITED — it was built on #26064 two green groups later'),
1139+
t(!/#2615[8]|#2621[0]|#2600[12]/.test(body),
1140+
'no green group is promoted to a victim row'),
1141+
t(!/could not be placed in a stack/.test(body),
1142+
'both victims WERE placed, so the "unresolved" hedge does not appear'),
1143+
t(postedBody(r).includes('同属 **1 条投机栈**'),
1144+
"the victim's own triage comment carries the same reading as the anchor"),
1145+
];
1146+
},
1147+
},
10811148
{
10821149
id: 'A3',
10831150
name: 'NEGATIVE: two ejections with DIFFERENT keys produce no anchor at all',
@@ -1659,8 +1726,8 @@ const MUTATIONS = [
16591726
{
16601727
id: 'M16',
16611728
what: 'same-stack is inferred from CO-OCCURRENCE instead of base === head, so unrelated victims fuse into one stack and a real cross-PR signal is under-reported',
1662-
from: ' for (const upstream of producers.get(e.base) ?? []) {',
1663-
to: ' for (const upstream of [...producers.values()].flatMap((s) => [...s])) {',
1729+
from: ' for (const upstream of producers.get(sha) ?? []) {',
1730+
to: ' for (const upstream of [...producers.values()].flatMap((s) => [...s])) {',
16641731
// The mirror of M15, and the reason both exist. M15 removes the evidence and
16651732
// the count goes UP; M16 stops requiring it and the count goes DOWN. A stack
16661733
// rule can be wrong in either direction, and a battery that only catches one
@@ -1679,6 +1746,18 @@ const MUTATIONS = [
16791746
// does not simply red the battery.
16801747
keepGreen: ['A1'],
16811748
},
1749+
{
1750+
id: 'M29',
1751+
what: 'the stack walk stops at the first hop, so a victim stacked on another THROUGH green, non-victim groups reads as an independent hit again',
1752+
from: ' for (const b of basesOf.get(sha) ?? []) frontier.push(b);',
1753+
to: '',
1754+
expect: ['A18'],
1755+
// ⚠️ A12 must NOT move: its stack is victim-on-victim, one hop each, so it is
1756+
// the control that proves A18's red comes from the walk and not from the
1757+
// stack test as a whole. A13 holds the other direction: a stopped walk can
1758+
// only fuse less, never invent a stack.
1759+
keepGreen: ['A12', 'A13'],
1760+
},
16821761
{
16831762
id: 'M17',
16841763
what: 'the anchor lookup goes back to selecting `open` + `finding`, so a GRADED anchor is invisible and every further ejection files another one',

0 commit comments

Comments
 (0)