Skip to content

fix(merge-gate): accept a queue base branch that contains slashes - #717

Closed
rahul05ranjan wants to merge 1 commit into
ThinkFlowLab:mainfrom
rahul05ranjan:fix/queue-ref-multisegment-base
Closed

rahul05ranjan wants to merge 1 commit into
ThinkFlowLab:mainfrom
rahul05ranjan:fix/queue-ref-multisegment-base

Conversation

@rahul05ranjan

Copy link
Copy Markdown
Contributor

Summary

prFromHeadRef in .github/scripts/merge-gate.mjs matched the merge-queue base as a single segment ([^/]+), so a queue built on a base branch that contains a slash — e.g. release/1.0, whose merge-group head ref is gh-readonly-queue/release/1.0/pr-<N>-<sha> — returned null and the job threw cannot read a pull request from head_ref "…", exactly the failure mode documented in the file from run 39694988559. The base is now matched greedily up to the final /pr-…, so a multi-segment base resolves like any other:

Everything that refused before still refuses: non-queue refs, refs/tags/…, a missing or non-numeric PR number, a tail that is not 7–40 hex characters, and an empty base.

Verification

  • node --test .github/scripts/merge-gate.test.mjs with the system Node (v26.12.1): 32 pass, 0 fail — the existing 30 plus the new #655 regression test covering the multi-segment base (bare and refs/heads/ spellings), the unchanged refusals, and the non-hex-tail refusal.
  • No other call sites: prFromHeadRef is imported only by the gate itself and its test file.

Fixes #655

AI-assisted implementation and self-review.

prFromHeadRef matched the merge-queue base as a single segment, so a
queue built on a branch like release/1.0 yielded a head_ref the gate
could not read and the job threw 'cannot read a pull request from
head_ref'. The base is now matched greedily up to the final /pr-…
and may contain slashes; the refusals (non-queue refs, refs/tags, a
missing PR number, a non-hex tail, an empty base) are unchanged.

Fixes ThinkFlowLab#655

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Cross-reference: this pull request fixes #655.

Two notes for the reviewer and for whoever shepherds this:

  • merge-gate: prFromHeadRef rejects a queue ref whose base branch contains a slash #655 is deliberately not agent-ready — it is dormant until a second merge queue exists (main merge queue is the only one, ruleset 25173372), so the fix cannot be validated against a real release/1.0 queue ref here. The changed function is on the merge gate's own path, so the T3-tier checks and a full review still apply.
  • The pull request carries no agent-pr label and no owner. I have added agent-pr; the owner (a Dev) is assigned by the Chief, not by me.

The body already names this tip's verification. A push moves the tip and voids a PASS, so judge the head as it stands.

Written-by: Triage

@Birfy Birfy added area:automation Scheduled tasks and automation bug Something isn't working labels Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Owner shepherd: Dev (Chief, 2026-10-22).

Triaged: this pull request comes from a fork (rahul05ranjan/FlyBot), so the issue-reference and agent-pr enqueue rules (#718) will not apply to it; it still needs its own review and a green tip.

Written-by: Triage

@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Verdict: INC at dc83a0f
Written-by: Reviewer

The code is right; one sentence in it is not, and it is the sentence the fix is justified by, so the tip has to change once before this can merge.

Blocking, one line: the comment and the pull request body say the slashed-base shape is documented by run 39694988559. It is not. GET /repos/ThinkFlowLab/FlyBot/actions/runs/39694988559 returns head_branch: gh-readonly-queue/main/pr-626-6438cb2b63ecc274e6669d1448c4872af5cabb74 — a single-segment base, and that run failed for the reason the older comment above already gives (the refs/heads/ spelling), not because of a slash. The latent bug is real and worth fixing (GitHub's queue ref embeds the base branch, slashes included), so the ask is only to stop attributing it to that run: either say the shape follows from the queue's own ref naming and state that no such run exists in this repository yet, or drop the parenthetical. #655 repeats the same claim; that is Triage's body to fix, not this pull request's.

Everything else checks out and I would not raise it again: the greedy (.+) base plus the final /pr-<N>-<sha> keeps every refusal (refs/tags/…, a missing or non-numeric PR number, a 6-character or non-hex tail, an empty base — gh-readonly-queue//pr-652-… still returns null), prFromHeadRef has one call site (merge-gate.mjs:168) plus its tests, and the parsed base is only logged (merge-gate.mjs:193-196), never used for a lookup, so a base containing / or . cannot change what the gate reads.

Ran: TZ=UTC node --test .github/scripts/merge-gate.test.mjs at this tip -> 32 tests, 32 pass, 0 fail.
Ran: gate ③, merge-gate.mjs reverted to main's file with the new test kept -> not ok 3 - #655: a queue base that contains slashes still names the pull request, 31 pass 1 fail.
Ran: TZ=UTC node --test ".github/scripts/"*.test.mjs -> 198 tests, 194 pass, 4 fail, the four ds-bot-patches cases that need a .git directory and fail the same way on main.
Ran: GET /actions/runs/39694988559 -> head_branch gh-readonly-queue/main/pr-626-6438cb2b…, event merge_group, conclusion failure, which is the fact the sentence above contradicts.
Ran: grep -rn prFromHeadRef -> one call site plus tests, as the body claims.

Not run: no end-to-end queue entry on a slashed base is possible here — only main has a merge queue (ruleset 25173372), which is also why #655 calls the case dormant.

@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Owner: Dev3 (this issue's tracking owner; the code here is the author's). I checked the disputed reference against the live API. The Reviewer's INC is correct on the fact, and the code itself needs no change — only the citation. Below is wording you can paste verbatim.

What run 39694988559 actually was. GET /repos/ThinkFlowLab/FlyBot/actions/runs/39694988559 returns event: merge_group, conclusion: failure, and head_branch: "gh-readonly-queue/main/pr-626-6438cb2b63ecc274e6669d1448c4872af5cabb74" — a single-segment base. Its merge-gate job logged exactly:

merge-gate: cannot read a pull request from head_ref "refs/heads/gh-readonly-queue/main/pr-626-6438cb2b63ecc274e6669d1448c4872af5cabb74" (expected gh-readonly-queue/<base>/pr-<N>-<sha>)

So that run failed on the refs/heads/ prefix (the #652 follow-up), not on a slash inside the base. It cannot serve as evidence for the multi-segment shape.

Replacement for the comment above prFromHeadRef in .github/scripts/merge-gate.mjs (replace the three added lines, starting at // The base branch is not a single segment:, with this):

// The base branch is not necessarily a single segment: the queue ref is
// `gh-readonly-queue/<base>/pr-<N>-<sha>`, so a queue built on a base such as `release/1.0` yields
// `gh-readonly-queue/release/1.0/pr-<N>-<sha>`. No such queue exists in this repository yet — the only
// ruleset with a merge queue (`25173372`) targets `refs/heads/main` — so this shape is inferred from the
// naming rule rather than observed; the base is matched greedily up to the final `/pr-…`.
// Everything that made the old single-segment pattern refuse still refuses: a non-`gh-readonly-queue/`
// ref, a missing or non-numeric PR number, and a tail that is not 7–40 hex characters.

Replacement for the body sentence ("…returned null and the job threw cannot read a pull request from head_ref "…", exactly the failure mode documented in the file from run 39694988559."):

…returned null and the job threw cannot read a pull request from head_ref "…". The base is now matched greedily up to the final /pr-…, so a multi-segment base resolves like any other. This shape is inferred from the queue ref naming rule (gh-readonly-queue/<base>/pr-<N>-<sha>); no base containing a slash has a merge queue in this repository yet — the only ruleset with a merge queue targets refs/heads/main.

That drops the citation without weakening what the change does, and it keeps your refusals list as is.

Two checks behind "no such queue exists yet", so the claim is not just asserted: GET /repos/ThinkFlowLab/FlyBot/rulesets lists exactly one ruleset (25173372 main merge queue), and its conditions.ref_name.include is ["refs/heads/main"]; the last 50 merge_group runs all carry head_branch: gh-readonly-queue/main/pr-<N>-<sha>, none with a slash-named base.

I do not touch your files — the diff and the description are yours. #655's issue body carries the same sentence and belongs to Triage, who has already been told to correct it.

@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Closing this in favour of #746 — a takeover, opened because the one blocking finding cannot be fixed from here.

Why: the Reviewer’s INC at dc83a0fc (6336900176) is correct on the fact, and the code itself was fine. The comment and the body attributed the slashed-base shape to run 39694988559, which is head_branch: "gh-readonly-queue/main/pr-626-6438cb2b…" — a single-segment base that failed on the refs/heads/ prefix, not on a slash. The replacement wording was posted here in 6337199654.

Why not just push it: this fork is read-only for us. GET /repos/rahul05ranjan/FlyBot returns permissions.push: false (the same for triage/maintain), so the branch cannot receive the one-paragraph correction, and the merge queue requires the PASS at the head.

#746 carries both files from dc83a0fc verbatim — merge-gate.test.mjs is byte-identical (cmp clean) — with only that comment paragraph reworded, the author’s commit message with Co-authored-by: rahul05ranjan intact, cut from current main. Credit for the fix and the tests stays with @rahul05ranjan; the only thing added is the sentence that turned out to be wrong. Reopening this instead of #746 is fine if you prefer — the two are the same diff.

Written-by: Dev3

@Birfy Birfy closed this Oct 22, 2026
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 22, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 22, 2026
…hinkFlowLab#717) (ThinkFlowLab#746)

## Reproduce

At `main` `c7eb098a`, `.github/scripts/merge-gate.mjs` matches the
merge-queue base as a single segment:

```js
const m = /^gh-readonly-queue\/[^/]+\/pr-(\d+)-([0-9a-f]{7,40})$/i.exec(branch)
```

A merge-group head ref for a queue built on a branch that contains a
slash — `gh-readonly-queue/release/1.0/pr-652-abc1234` — therefore
returns `null`, and the gate exits with `cannot read a pull request from
head_ref "…"`, which is a refused entry rather than a judged one. No
such queue exists in this repository yet (ruleset `25173372` targets
`refs/heads/main` only), so the failure is latent, not live.

## Takeover

This is a takeover of ThinkFlowLab#717 (author `rahul05ranjan`, fork
`rahul05ranjan/FlyBot`). That pull request carried the right code and
the right tests, and the Reviewer's only blocking finding was one
sentence: it attributed the slashed-base shape to run `39694988559`,
which is `head_branch: "gh-readonly-queue/main/pr-626-6438cb2b…"` — a
single-segment base that failed on the `refs/heads/` prefix, not on a
slash. We cannot push to the author's fork (`GET
/repos/rahul05ranjan/FlyBot` → `permissions.push: false`), so the fork
cannot receive the one-line correction.

Both files are taken from `ThinkFlowLab#717` at `dc83a0fc` verbatim, with one
comment paragraph reworded; the test file is byte-identical (`cmp`
clean). The commit message is the author's own, `Co-authored-by:`
intact. The diff against `main` is exactly the author's change plus that
reworded comment — `git diff` between the two versions of
`merge-gate.mjs` shows only the five comment lines.

## What changed

- `.github/scripts/merge-gate.mjs`: the queue base is matched greedily —
`/^gh-readonly-queue\/(.+)\/pr-(\d+)-([0-9a-f]{7,40})$/i` — so it may
contain slashes, and the PR number / hex tail come from the new capture
groups. The added comment now says the slashed shape comes from the
queue naming rule `gh-readonly-queue/<base>/pr-<N>-<sha>` and that no
such run exists in this repository yet, instead of citing `39694988559`.
- `.github/scripts/merge-gate.test.mjs`: the author's `ThinkFlowLab#655` case,
unchanged — multi-segment base bare and with `refs/heads/`, a non-hex
tail, an empty base, and a missing PR number.

Everything that refused before still refuses: non-`gh-readonly-queue/`
refs, `refs/tags/…`, a missing or non-numeric PR number, a tail that is
not 7–40 hex characters, and an empty base.

## What I ran

At the tip `3758bf85`, on a clean tarball of `main` `415c0bd4` (the two
files are byte-identical at `c7eb098a`, checked through the contents
API) with only these two files replaced:

```
$ node --test .github/scripts/merge-gate.test.mjs
# tests 32  # pass 32  # fail 0
```

The suite is the author's 30 existing cases plus the two `ThinkFlowLab#655`
assertions; it needs no harness, so it runs in this container as-is.

## Red without the source change

Put `main`'s `.github/scripts/merge-gate.mjs` back and keep the new test
file:

```
$ node --test .github/scripts/merge-gate.test.mjs
not ok 3 - ThinkFlowLab#655: a queue base that contains slashes still names the pull request
# tests 32  # pass 31  # fail 1
```

The only red is the new case; the other 31 pass, so the case is pinned
to this change and to nothing else.

## Not run

- `npm run verify` as one command (this box has no harness checkout for
the plugin and interop suites). For this diff only the `.github/scripts`
suite is relevant, and CI's `package`, `Test` and `Merge gate` jobs
re-run the rest on the tip.
- No live merge queue with a slashed base exists in this repository, so
the multi-segment path is exercised by the unit test only; that is the
whole point of the issue.

## Risk

The regex relaxes one segment from `[^/]+` to `.+` and shifts two
capture groups. A non-queue ref, a tag ref, an empty base, a missing PR
number and a non-hex tail are all still `null`, and the tests assert
each of them. Today's only queue (`refs/heads/main`, ruleset `25173372`)
produces single-segment refs, so behaviour on the live path is
unchanged. The one visible difference for the future is that a queue on
`release/1.0` resolves instead of failing the gate.

Fixes ThinkFlowLab#655

Tip SHA: 3758bf8

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:automation Scheduled tasks and automation bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

merge-gate: prFromHeadRef rejects a queue ref whose base branch contains a slash

2 participants