diff --git a/.github/workflows/merge-queue-triage.yml b/.github/workflows/merge-queue-triage.yml index e85b6455812..f5f16f4343e 100644 --- a/.github/workflows/merge-queue-triage.yml +++ b/.github/workflows/merge-queue-triage.yml @@ -554,11 +554,19 @@ jobs: * construction. One deterministic break therefore ejects everyone * behind it, and the raw victim count measures QUEUE DEPTH. * - * ⚠️ The test is full-sha equality in the strict direction: one - * victim's base == another victim's head. ⛔ Never a base PREFIX and - * ⛔ never time ordering — both would fuse genuinely independent hits - * into one stack, trading this limb's false positive for a false - * negative, which is strictly worse. The count exists to be true. + * ⚠️ The test is full-sha equality in the strict direction, walked + * base to head: from a victim's base, through every merge_group run + * in `stackEdges` whose head is that sha (green, non-victim groups + * included), until the chain reaches another victim's head. A green + * group between two victims is NOT a break in the stack: its + * affected-only selection may simply never have run the failing + * package, so it carries the broken tree without failing on it. + * Walking only victim-to-victim read one deterministic break as two + * independent hits (#26265). ⛔ Never a base PREFIX and ⛔ never time + * ordering — both would fuse genuinely independent hits into one + * stack, trading this limb's false positive for a false negative, + * which is strictly worse. The count exists to be true. The index is + * data, not git, so the walk keeps a visited set and ends on a loop. * * A victim whose runs are not in the index (a run older than the * window, or a ref that is not a queue ref) is left as its own group: @@ -582,6 +590,14 @@ jobs: } } + // head sha -> the bases of EVERY indexed run that produced it, + // victim or not: the edges the walk below steps back along. + const basesOf = new Map(); + for (const e of stackEdges.values()) { + if (!basesOf.has(e.head)) basesOf.set(e.head, new Set()); + basesOf.get(e.head).add(e.base); + } + const inherited = new Set(); let unresolved = 0; for (const [pr, runs] of byPr) { @@ -590,10 +606,18 @@ jobs: const e = stackEdges.get(String(id)); if (!e) continue; resolved = true; - for (const upstream of producers.get(e.base) ?? []) { - if (upstream === pr) continue; - inherited.add(pr); - union(pr, upstream); + const visited = new Set(); + const frontier = [e.base]; + while (frontier.length > 0) { + const sha = frontier.pop(); + if (visited.has(sha)) continue; + visited.add(sha); + for (const upstream of producers.get(sha) ?? []) { + if (upstream === pr) continue; + inherited.add(pr); + union(pr, upstream); + } + for (const b of basesOf.get(sha) ?? []) frontier.push(b); } } if (!resolved) unresolved++; @@ -839,8 +863,9 @@ jobs: ? [ "⚠️ **Queue depth is not evidence.** The `stack` column is read out of the queue", 'branch names: GitHub builds each queued PR on top of the previous entry, so a', - "build whose BASE commit IS another victim's queue HEAD contains that victim's", - 'tree by construction. A single deterministic break therefore ejects every PR', + "build whose BASE chain reaches another victim's queue HEAD (directly, or through", + "green groups queued between them) contains that victim's tree by construction.", + 'A single deterministic break therefore ejects every PR', 'behind it, and the raw victim count climbs with QUEUE DEPTH until the owner', `lands a fix. Start with the ${independent === 1 ? 'root' : 'roots'} above; an ` + '`inherited` row is a bystander until shown otherwise.', @@ -867,6 +892,10 @@ jobs: 'opposite diagnoses), decide the cause, and close this issue with the fix or with', 'the reason it is not one.', '', + 'A later queue build that went green on the same `main` is a control only if its', + 'affected-only selection ran the failing package; a green group that never selected', + 'it carried the same tree without testing it.', + '', `Last refreshed by queue build [${run.id}](${run.html_url}) (PR #${prNumber}).`, '', '---', diff --git a/scripts/check-merge-queue-triage-outcome.mjs b/scripts/check-merge-queue-triage-outcome.mjs index c1f664deb15..4241678b5d6 100644 --- a/scripts/check-merge-queue-triage-outcome.mjs +++ b/scripts/check-merge-queue-triage-outcome.mjs @@ -122,8 +122,10 @@ // announcement, create an anchor without having established one does not exist, // revert the anchor lookup to the `open` + `finding` selection that made a // GRADED anchor invisible, stop recognising a closed-as-duplicate chain, drop -// the link a regression anchor owes the anchor it regressed from, -// and move the redelivery guard below the aggregation -- and requires the +// the link a regression anchor owes the anchor it regressed from, stop the +// stack walk at its first hop so a green group between two victims splits +// their stack again, and move the redelivery guard below the aggregation -- +// and requires the // battery to go RED for each, naming the scenario it expects. Each mutation // also asserts its own anchor was PRESENT before substituting: a mutation that // silently matched nothing would leave the battery green and read exactly like @@ -567,13 +569,14 @@ const sightingComment = (key, pr, runId) => * * `base` is the sha the queue branch NAME carries (the commit this build was * stacked on); `head` is the commit the build actually produced. Limb ②'s stack - * test is `one victim's base === another victim's head`, so a fixture that let - * these two default to the same shape could not tell the test from a tautology. + * test walks `base === head` from a victim's base, through any indexed run, to + * another victim's head, so a fixture that let these two default to the same + * shape could not tell the test from a tautology. */ -const queueRun = (id, pr, base, head) => ({ +const queueRun = (id, pr, base, head, conclusion = 'failure') => ({ id, workflow_id: WORKFLOW_ID, - conclusion: 'failure', + conclusion, head_branch: `gh-readonly-queue/main/pr-${pr}-${base}`, head_sha: head, }); @@ -596,6 +599,24 @@ const INCIDENT = { head12855: pad('1a24e9778'), }; +/** + * The #26247 anchor's chain, as the card that filed this work recorded it: one + * deterministic break (#26064's queued commit) and a second victim (#26198) + * stacked on it THROUGH two green, non-victim groups (#26158, #26210), whose + * affected-only Test Core never selected the failing package. + * + * Same padding caveat as INCIDENT: the record abbreviates, so the four recorded + * shas are right-padded, and the two the record does not carry (main before + * the queue, #26198's own head) are declared synthetic. + */ +const CHAIN = { + mainTip: pad('5c0ffee'), // synthetic: main before the queue + head26064: pad('17f21e8d'), // #26064's victim head — and #26158's base + head26158: pad('745c276b'), // #26158's GREEN group head — and #26210's base + head26210: pad('7122b0db'), // #26210's GREEN group head — and #26198's base + head26198: pad('d0d0cafe'), // synthetic: #26198's own queue head +}; + /** * The anchor issue an earlier pair already produced. * @@ -1078,6 +1099,52 @@ function scenarios(root) { ]; }, }, + { + id: 'A18', + name: 'TRANSITIVE STACK: a victim stacked on another THROUGH green, non-victim groups is ONE independent hit', + // The #26247 reading, re-scored. A ← B ← C ← D with only A and D red: + // D's base is C's head, C's base is B's head, B's base is A's head. B and + // C are not victims (their affected-only selection never ran the failing + // package), so a stack test that unions only through VICTIM runs never + // sees the chain and reports two independent hits for one break. + world: () => base({ + runId: 37500000004, + prNumber: 26198, + headSha: CHAIN.head26198, + headBranch: `gh-readonly-queue/main/pr-26198-${CHAIN.head26210}`, + repoComments: [sightingComment(KEY_A, 26064, 37500000001)], + queueRuns: [ + queueRun(37500000001, 26064, CHAIN.mainTip, CHAIN.head26064), + queueRun(37500000002, 26158, CHAIN.head26064, CHAIN.head26158, 'success'), + queueRun(37500000003, 26210, CHAIN.head26158, CHAIN.head26210, 'success'), + // The index is data, not git: a loop in it (here behind main's tip, + // where no victim lives) must END the walk, not hang the job, and + // must fuse nothing it reaches. + queueRun(37500000005, 26001, pad('c1c1e'), CHAIN.mainTip, 'success'), + queueRun(37500000006, 26002, CHAIN.mainTip, pad('c1c1e'), 'success'), + ], + }), + check(r, t) { + const body = r.calls.issueCreate[0]?.body ?? ''; + return [ + t(r.calls.issueCreate.length === 1, `one anchor is filed, got ${r.calls.issueCreate.length}`), + t(/\*\*2 pull requests\*\*/.test(body), + `the raw victim count is 2 — the green groups are NOT victims and are not listed, got: ${JSON.stringify(body.slice(0, 240))}`), + t(/\*\*1 independent hit\*\*/.test(body), + `and the chain through the green groups scores ONE independent hit, got: ${JSON.stringify(body.slice(0, 240))}`), + t(/\|\s*#26064\s*\|\s*S1 · root\s*\|/.test(body), + `#26064 — the queued commit that broke — is the stack ROOT, got: ${JSON.stringify(body)}`), + t(/\|\s*#26198\s*\|\s*S1 · inherited\s*\|/.test(body), + '#26198 is marked INHERITED — it was built on #26064 two green groups later'), + t(!/#2615[8]|#2621[0]|#2600[12]/.test(body), + 'no green group is promoted to a victim row'), + t(!/could not be placed in a stack/.test(body), + 'both victims WERE placed, so the "unresolved" hedge does not appear'), + t(postedBody(r).includes('同属 **1 条投机栈**'), + "the victim's own triage comment carries the same reading as the anchor"), + ]; + }, + }, { id: 'A3', name: 'NEGATIVE: two ejections with DIFFERENT keys produce no anchor at all', @@ -1659,8 +1726,8 @@ const MUTATIONS = [ { id: 'M16', 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', - from: ' for (const upstream of producers.get(e.base) ?? []) {', - to: ' for (const upstream of [...producers.values()].flatMap((s) => [...s])) {', + from: ' for (const upstream of producers.get(sha) ?? []) {', + to: ' for (const upstream of [...producers.values()].flatMap((s) => [...s])) {', // The mirror of M15, and the reason both exist. M15 removes the evidence and // the count goes UP; M16 stops requiring it and the count goes DOWN. A stack // rule can be wrong in either direction, and a battery that only catches one @@ -1679,6 +1746,18 @@ const MUTATIONS = [ // does not simply red the battery. keepGreen: ['A1'], }, + { + id: 'M29', + 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', + from: ' for (const b of basesOf.get(sha) ?? []) frontier.push(b);', + to: '', + expect: ['A18'], + // ⚠️ A12 must NOT move: its stack is victim-on-victim, one hop each, so it is + // the control that proves A18's red comes from the walk and not from the + // stack test as a whole. A13 holds the other direction: a stopped walk can + // only fuse less, never invent a stack. + keepGreen: ['A12', 'A13'], + }, { id: 'M17', what: 'the anchor lookup goes back to selecting `open` + `finding`, so a GRADED anchor is invisible and every further ejection files another one',