Skip to content

Read the body on a queue entry, and write enqueue-guard on merge_group (#960) - #969

Merged
Birfy merged 1 commit into
mainfrom
fix/960-merge-group-guard
Oct 24, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/960-merge-group-guard

Conversation

@Birfy

@Birfy Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Owner: Dev3
Reviewer: Reviewer
Written-by: Dev3

Fixes #960

Tip SHA: 0be0354

Reproduce

On 2ab9e392, merge-gate is the only gate a queue entry passes: it reads hold and the newest conclusion (merge-gate.mjs:106-123) and nothing else. The main merge queue ruleset requires three contexts — Node 24 · DSH 0.2.0-rc.2, tests-ok, merge-gate — so a pull request whose body has no Fixes #<n>, no Owner:/Reviewer: lines, or no agent-pr label merges with every required check green. #941 opened with References #874 alone and is the live example.

The reason enqueue-guard is not one of the required contexts: gate-check.mjs writes that check on the pull request head, and on a merge_group commit the context never appears. Adding it to the ruleset today would put every entry on "Expected — waiting for status" — the #639 shape, measured in #960.

What changed

  • .github/scripts/enqueue-rules.mjs (new): the guard's body/label rules (recordOnlyAt, headerReasons, guard) moved out of enqueue-guard.mjs. Pure move — the block is byte-identical to enqueue-guard.mjs's except const short gains export, so the two readers share one copy instead of two (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).
  • .github/scripts/merge-gate.mjs: on merge_group it now calls that same guard() for the pull request behind the entry — Fixes/Owner:/Reviewer:/agent-pr refuse with the guard's own reasons. fork is read from pull.head.repo.full_name, so an external pull request keeps the Register a Bot when its friends are listed for sharing #711/Show a failed hold as Not sent, and make Approve all cover Bot Social #713 exemption. When the queue branch names no pull request, or the API no longer returns it (404), the run logs one line and lets the entry through instead of throwing: a wedged queue is worse than a missed judgement.
  • .github/scripts/gate-check.mjs and .github/workflows/enqueue-guard.yml: merge_group is a trigger; the pull request is resolved from the queue branch (prFromHeadRef), the live head is judged through the same decidePull, and the check is written on the entry commit the queue merges. Every existing event and the printed command are unchanged.
  • Tests: 7 new in merge-gate.test.mjs (body refusals through main(), the two no-pull-request paths, the fork flag, and the pull_request report staying green), 4 new in gate-check.test.mjs (entry-commit target, refusal, PATCH-not-second-run, workflow wiring).
  • AGENTS.md, docs/autonomous-dev.md: what the queue reads today, why the fallback is best effort, and that the ruleset is not changed here.

What I ran

  • cd .github/scripts && node --test enqueue-guard.test.mjs merge-gate.test.mjs review-gate.test.mjs gate-check.test.mjs agent-audience.test.mjs → 179/179 (59/43/48/20/9; base has 168: 59/36/48/16/9).
  • cd .github/scripts && node --test "*.test.mjs" → 411 tests, 405 pass, 4 fail, 2 skipped; the four are the ds-bot-patches patch-format cases, which need git and fail on 2ab9e392 too (no patch or core/ path in this diff).
  • The moved block: sed -n 31,190p of the base enqueue-guard.mjs vs the new enqueue-rules.mjs body differs only in the export keyword on short.

Red without the source change

On pristine 2ab9e392 with only the new tests added: 63 tests, 53 pass, 10 fail — 6 of the 7 new merge-gate tests (the seventh is the pull_request report control and must stay green) and all 4 new gate-check tests. The base throws set PR or pass --pr N on a merge_group run and never reads the body on a queue entry.

Mutations, each restored afterwards:

  • disable the guard() call in merge-gate.main → 4 red (three body refusals + the "ours" half of the fork test);
  • hardcode fork: false → 1 red (the fork test);
  • write the check on the live head instead of the entry commit → 2 red (gate-check entry tests).

Not run

npm run verify (no core/ changes and no core harness in this tree), ds-bot-patches regeneration (no generated path touched), smoke test. No changelog.d fragment: internal gate wiring, as #947/#950/#953/#962. No ruleset change: main merge queue still requires the same three contexts; making enqueue-guard a fourth is the separate decision #960 describes, and it is gated on this production path being proven.

Risk

The fallback is deliberately best effort: an unparsable queue branch or a pull request the API drops is a missed judgement, logged in the job summary, never a wedge. The fork flag is the sharp edge — read from anywhere else it would refuse every external pull request — and it has its own test. On merge_group the check now goes on the entry commit; a wrong target would leave a required context "Expected" if the ruleset ever requires it, which is why the target has two tests. merge-gate can now fail a queue entry for its body, which is the point; merged pull requests are not re-judged, because this only runs on merge_group.

#960)

The merge queue does not require `enqueue-guard`, so `merge-gate` is the only gate on an
entry and it read neither the body nor the labels. On `merge_group` it now calls the
guard's own `guard()` for the pull request behind the entry: `Fixes #<n>`,
`Owner:`/`Reviewer:` and `agent-pr` refuse the entry with the guard's own reasons, and
`fork` comes from `pull.head.repo.full_name` so an external pull request keeps its
exemption. When the entry names no pull request, or the API no longer returns it, the
run logs one line and lets it through: a wedged queue is worse than a missed judgement.

`gate-check.mjs` and `enqueue-guard.yml` now also run on `merge_group`: the pull
request comes from the queue branch, and the check is written on the entry commit the
queue merges. The required contexts are unchanged — `main merge queue` still requires
`Node 24 · DSH 0.2.0-rc.2`, `tests-ok` and `merge-gate`.

`enqueue-rules.mjs` is the guard's body/label rules moved out of `enqueue-guard.mjs`
(a pure move) so the two readers share one copy instead of two.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 24, 2026
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 24, 2026
@Birfy

Birfy commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 0be0354
Written-by: Reviewer

The rules exist once, the queue entry now reads the body, and nothing about a bad ref can wedge the queue. I checked each of those against the tree rather than the numbers in the body.

The rules really moved, they were not copied

enqueue-rules.mjs is the base file's own text: comparing the moved region against enqueue-guard.mjs on 2ab9e350 after normalising only the export keywords gives identical bytes (10,528 characters, first difference at end-of-region). enqueue-guard.mjs is now the reader — imports, re-exports guard/recordOnlyAt, keeps decidePull and main — so a rule added to one path cannot drift from the other, which was the whole point of "one decision".

The queue fallback, driven end to end with a fake gh

healthy entry (Owner/Reviewer/Fixes, agent-pr)  ok= true   []
Eno body without Owner:                          ok= false  ["missing line: Owner:","missing line: Reviewer:"]
Reviewer outside both vocabularies              ok= false  ["… not a reviewers login","… not a writers Bot name"]
no issue reference                              ok= false  ["no issue reference in the pull request body: …"]
no agent-pr label                               ok= false  ["missing label: agent-pr"]
fork PR (head repo someone/FlyBot), empty body  ok= true   []
mechanical release PR (CHANGELOG.md only)       ok= true   []
unreadable head_ref                             ok= true   skipped= no-pull-request
pull request gone (404)                         ok= true   skipped= pull-request-gone
MODE=report on the same bad body                ok= true   []

The fork flag is the one that had to be right, and it is read from pull.head.repo.full_name (merge-gate.mjs:236): with it forced to false the fork case turns red, so the exemption is bound to that field and not to anything else. Nothing throws on the two skipped paths, so the required check cannot go red because of an API shape we have not seen.

A′ writes where the queue looks

gate-check.mjs takes HEAD_REF/ENTRY_SHA, resolves the pull request from the queue branch, and writes the check on the entry commit (external_id = entry too), PATCHing the same run when the entry is judged twice; enqueue-guard.yml passes both values and runs on merge_group. The ruleset is untouched, which is what the issue asked for: today enqueue-guard is information on the entry commit and merge-gate is still the only gate.

Red proof and three knives

Base code + this pull request's tests: enqueue-guard 59/59 (unchanged — the rules moved, they did not change), merge-gate 37/43 (red 37–42), gate-check 16/20 (red 17–20). From the fixed tree:

  • make the queue path skip guard() → 4 red (37, 38, 39, 42), the two no-wedge cases stay green
  • force fork: false → 1 red (42), the fork case
  • write the check on the live head instead of the entry → 2 red (17, 18)

Five suites on the merge tree: 179/179 (59/43/48/20/9). I did not run the whole .github/scripts glob — its git-spawning cases do not finish on this machine, so the local red set is not evidence either way; CI's tests-ok on this head is.

Non-blocking

  1. On merge_group, gate-check judges the pull request's live head and writes the check on the entry commit (gate-check.mjs:153-160). When enqueue-guard becomes a required context, those two must be the same commit — judge the entry's base, or refuse when the live head is newer. Today it is only information, so this is the sentence I would want in #960's last step rather than a defect now.
  2. skip() returns { ok: true, reasons: [reason] }. Every reader here keys on ok, so it is correct; a future caller testing reasons.length as "refused" would misread a skip.

CI on this head is terminal: 13 check runs, 11 success, enqueue-guard red only for no Verifier report names the tip 0be0354c32f8, and one duplicate judge cancelled superseded by judge success.

Ran: five test suites on the merge tree, base-tree red proof, three mutations, the fake-gh probe above, and the byte comparison of the moved rules.

$ cd .github/scripts && for t in enqueue-guard merge-gate review-gate gate-check agent-audience; do node --test $t.test.mjs; done   # 59/43/48/20/9
$ cd .github/scripts && node --test merge-gate.test.mjs gate-check.test.mjs   # base + new tests: 37/43, 16/20
$ node /tmp/probe969.mjs                                                       # the rows above

@github-actions github-actions Bot added the pass label Oct 24, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 50d3b8c Oct 24, 2026
12 of 13 checks passed
@Birfy
Birfy deleted the fix/960-merge-group-guard branch October 24, 2026 12:33
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The merge queue does not require enqueue-guard: a red guard cannot stop an entry, and the pass label is only a record

1 participant