enqueue-guard: a tip that moved without changing the change set must not void the PASS (update-branch should not cost a review) #1317

Written-by: Triage
Owner: Dev3（guard/CI 这侧；排期由 Lead 控）

The cost, with the receipts

#1304 was PASSed three times for one change set: 206fbadb (01:30:48Z), d7bd1c1d (02:13:48Z), dae08a98 (02:38:43Z). Each time the tip had moved only because main moved and someone ran PUT /pulls/1304/update-branch; the reviewer then re-read 48 files, re-ran the suites, and re-wrote the same PASS at the new head. Today that is the rule, literally:

• review-gate.mjs:332 — if (parsed.tip !== headSha) → "names 206fbad, not the tip dae08a9"; only the exact 40-character tip counts (The enqueue guard reported a PASS that had not been posted yet, and would go green again on a re-push #1112). enqueue-guard's reviewed tip and merge-gate's queue entry both read that one decide, so all three refuse together.
• AGENTS.md:34 — "a new commit voids the PASS"; AGENTS.md:203 — one conclusion per tip.

That rule exists for a good reason (a PASS must not survive a push that changes the code — #1221, #1256), but it cannot tell "the code changed" from "main was merged in and the code did not change". The second case cost a full re-review three times in a single PR, and it will recur on every PR that has to be brought up to date, which — with one PR in the queue at a time — is now most of them.

Note the case that proves the rule must stay: at d7bd1c1d an update-branch did change the code — that tree carried core/package.json at 0.2.9 while main had 0.2.10, and the correction (6432090166) voided it. Any relaxation must still catch exactly that.

The ask

A tip that moved without changing the reviewed change set does not void the PASS. Everything else stays: a PASS at a tip whose change set differs is worth nothing; an INC at the head still vetoes; one conclusion per tip is untouched; the Ran: requirement is untouched; T0/T1 and the mechanical release are untouched.

The comparison (pinned, because "patch-id" has to mean one thing here)

The guard has gh and nothing else (enqueue-guard.mjs:40 is execFileSync('gh'); the scripts do not shell out to git, and a rebased-away tip is not reachable by ref anyway). So the identity is a change-set fingerprint computed over (path → blob sha) at the PR's changed paths, not a shell git patch-id:

1. Read the PR's current changed-file list (the guard already reads it: decidePull), paginated.
2. Read the tree at the reviewed tip and at the head (GET /git/trees/<sha>?recursive=1).
3. The change set is identical iff, for every path in that list, the blob at T equals the blob at H. A path missing on either side is a difference (not a pass).

Why this is the right identity and not a weaker one: it compares the content a reviewer actually judged, path by path, and it does not care which commits carried it (a merge of main, a rebase, a cherry-pick of the same tree). It also cannot be fooled by a tip that merely looks like the old one: any byte changed in any changed file fails the equality.

Fail closed, always. An API error, a 404 on the old tip, a truncated tree (truncated: true), a missing blob, or a paginated file list the guard could not finish reading → behave exactly as today (no matching conclusion → refuse, re-review). If Dev finds a hole in this definition, say so on this issue and propose the alternative before implementing; do not quietly substitute another rule.

Where it must live (one cut, three readers)

decide in review-gate.mjs (:303, the tip compare at :332) is the single place that answers "does this tip carry a conclusion". Add the comparison there, behind an injectable hook (the signature already takes gh and repo/pr, so tests drive it with a stub and no network):

• enqueue-guard.mjs decidePull (:56) — both the printed command and the enqueue-guard check run (:812: one function, two readers) must ALLOW with the same sentence.
• merge-gate.mjs judge (:104) — the queue entry reads the same decide; a repaired entry must pass there too, and a refused one must say the same thing.
• A second copy of this rule anywhere is an incident (The fork's main has no ruleset and no branch protection, so a PASS is only advisory and an unreviewed pull request can be squash-merged #812's kill condition); gate-check.mjs must not grow its own.

New helper: .github/scripts/change-set.mjs — sameChangeSet({ gh, repo, pr, reviewed, head }), sync like the rest, no git. It is the only reader of trees; decide gets it as an injected function (default = that helper) so every existing test keeps running with a stub.

Messages (a refusal or an allow has to name the fact)

• Allow: ALLOW — Reviewer's PASS at 206fbadb still describes this head: the change set is identical (the tip moved only by merges of main). — both full shas, and the fact that the comparison was made.
• Refuse (one changed blob): today's sentence, plus one clause naming the first differing path: … the change set differs at core/package.json — the PASS at 206fbadb does not describe dae08a98.
• Refuse (could not read): … the change set could not be read (git tree of 206fbadb: 404) — never "assume unchanged".

Acceptance (with knives)

1. Repaired tip passes: a fixture PR whose head is the reviewed tip plus a merge of main, trees identical at every changed path → decide returns ok with the reviewer's PASS, and the message names both shas. (Knife: delete the comparison → test 1 red.)
2. A changed blob refuses: same fixture, one blob differing (the d7bd1c1d package.json case, reproduced as a fixture) → refused, naming the path. (Knife: make sameChangeSet always return true → test 2 red.)
3. Unreadable refuses: stub gh raising / returning truncated → refused with the read that failed; nothing passes.
4. INC still vetoes: an INC at H refuses even when H's change set equals T's (the newest conclusion at the head decides).
5. Newest matching conclusion wins: two old tips whose change sets both match H → the newer one is used; an older PASS never outranks a newer INC.
6. Three readers, one answer: a table-driven test asserting decidePull (allow + printed command), the enqueue-guard check object and judge (queue mode) agree on the same fixture; cases live in .github/scripts/fixtures/workflow-cases.json (the shared table The workflow engine: each node names its Bot and its inputs and outputs, the nodes are drawn as a flowchart, and a node missing an input is refused on the card #1272 step 2 added), not in a second table.
7. No weakening: Ran: still required, one-pass-per-tip tests unchanged, T0/T1 and mechanical-release exemptions unchanged, hold still refuses.
8. The rule text moves with the code: AGENTS.md:34 and :203 say the new sentence — a new commit voids the PASS unless it changes no file of the reviewed change set, and the guard names the sha it accepted and why. Docs change in the same PR (this is a rule change, not a code detail).
9. node --test .github/scripts/*.test.mjs and npm run verify green; node scripts/ds-bot-patches.mjs check unaffected (no core/ file changes expected — if one is needed, mirror it).

Out of scope

• Signed conclusions (A forged Written-by: Reviewer still merges: the guard cannot tell which workstation wrote a Verdict #1110/Mint one Ed25519 key per reviewing workstation (public key in the repo, private key in that Bot's secret store) #1111, owner declined keys), the Verdict: format, Attestation:, and who may review.
• Auto-merge, the merge queue's ruleset, anything that makes the guard enqueue.
• Loosening anything for T0/T1 (they already need no PASS) or for the mechanical release PR.
• Retroactively re-labelling or re-opening merged PRs; this changes only what a future entry accepts.