From 4e3ca2289897f67e1775640353856a43b46f4405 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Thu, 22 Oct 2026 16:46:49 +0800 Subject: [PATCH 1/3] Refuse a pull request with no issue reference or no agent-pr label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #711/#712/#713 entered the review path with no issue and no `agent-pr` label and nothing refused them: the enqueue guard only read `hold`, the conclusions and the diff. It now refuses a body with no `Fixes #`/`Closes #` line and one without the label, with the two wordings the issue fixes. The exemption is `isMechanicalRelease` over the changed files, not `result.exempt` — that flag is also true for T0/T1, and routine work still names its issue. Refs #718 --- .github/scripts/enqueue-guard.mjs | 28 +++++++++++++++---- .github/scripts/enqueue-guard.test.mjs | 38 +++++++++++++++++++++++--- AGENTS.md | 1 + 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/.github/scripts/enqueue-guard.mjs b/.github/scripts/enqueue-guard.mjs index 541f674c0..ed1e47b3c 100644 --- a/.github/scripts/enqueue-guard.mjs +++ b/.github/scripts/enqueue-guard.mjs @@ -11,7 +11,9 @@ // // So this is the step immediately before that call. It refuses — exit 1, the reason printed — when // the pull request carries `hold`, when the newest conclusion at its head is not a PASS, or when that -// conclusion is itself marked "record only — do not merge". It reuses `judge({ mode: 'queue' })`, so +// conclusion is itself marked "record only — do not merge". It also refuses a pull request that names +// no issue in its body or carries no `agent-pr` label (#NNN; a mechanical release PR is exempt), so +// "one issue, one pull request" is checked rather than remembered. It reuses `judge({ mode: 'queue' })`, so // "may this enter the queue" keeps one definition: a rule added to the merge gate is enforced here // without being restated, unless someone deliberately changes both. // @@ -21,7 +23,7 @@ // routine's path rather than left to a check that reports after the queue is reached. import { execFileSync } from 'node:child_process' import { fileURLToPath } from 'node:url' -import { judge } from './merge-gate.mjs' +import { isMechanicalRelease, judge } from './merge-gate.mjs' import { loadPolicy, parseReport } from './review-gate.mjs' const SHORT = 12 @@ -34,7 +36,14 @@ const SHORT = 12 // pull request out` — must not freeze itself (#687). A bullet or a quote line is allowed, but nothing // may follow the marker on it except its other language half. const RECORD_ONLY = /^[\s>*+-]*(?:record only\s*[—–-]+\s*do not (?:merge|queue)(?:\s*[//]\s*仅记录[、,,]?\s*禁止入队)?|仅记录[、,,]?\s*禁止入队)\s*[.。]?\s*$/im +// AGENTS.md → Claim: a pull request says `Fixes #N` in its body, and one issue carries one pull request. +// #718: a pull request with no issue behind it, or with no `agent-pr` label, is not the routine's work at +// all — #711/#712/#713 arrived with neither and nothing refused them, because the guard only looked at +// `hold`, the conclusions and the diff. The reference is a line of its own, so prose about "fixes" does +// not pass; the spellings accepted are exactly `Fixes #` and `Closes #`, behind any `#`, `>` or `-`. +const ISSUE_REF = /^[\s>#-]*(?:fixes|closes)\s*#\d+/im const short = (s) => String(s ?? '').slice(0, SHORT) +const labelNames = (labels) => (labels ?? []).map((l) => String(typeof l === 'string' ? l : l?.name ?? '').toLowerCase()) // The newest conclusion that names this head, if it carries the record-only marker. Only a body that // parses as a conclusion counts, so ordinary prose quoting the phrase is no marker; only the newest @@ -52,16 +61,24 @@ export function recordOnlyAt(reports, head) { } // The decision, without the network: `result` is merge-gate's `judge` output for this head. -export function guard({ pr, head, labels, result, reports }) { +export function guard({ pr, head, labels, body, mechanicalRelease, result, reports }) { const quiet = recordOnlyAt(reports, result.head ?? head) const reasons = [...result.reasons] if (quiet) reasons.push('the newest conclusion at this head says "record only — do not merge"') - const ok = result.ok && !quiet + // A mechanical release PR carries neither an issue nor the label by design — it is the routine's own + // daily output under the standing approval (AGENTS.md 6) — so it is the one exemption. `result.exempt` + // cannot be used for this: it is true for T0/T1 as well, and routine work still names its issue. + if (!mechanicalRelease) { + if (!ISSUE_REF.test(String(body ?? ''))) reasons.push('no issue reference in the pull request body: add "Fixes #"') + if (!labelNames(labels).includes('agent-pr')) reasons.push('missing label: agent-pr') + } + const ok = result.ok && !quiet && reasons.length === 0 return { ...result, ok, reasons, recordOnly: Boolean(quiet), + mechanicalRelease: Boolean(mechanicalRelease), labels: labels ?? [], // One loud sentence: a refusal says what to do ("do not enqueue") and why, so it cannot be read // as a warning that some other step may override. @@ -106,7 +123,8 @@ export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prO // The live head is judged, never a cached one: a push that moved the tip voids the PASS the caller // believes it has, and `--sha` lets a caller insist on the head it read a moment ago. const result = judge({ labels, files, head, author: pull.user?.login, reports, policy: loadPolicy(policyPath), expectedHead: arg('--sha'), mode: 'queue' }) - const decision = guard({ pr, head, labels, result, reports }) + const mechanicalRelease = isMechanicalRelease(files.map((f) => f.filename)) + const decision = guard({ pr, head, labels, body: pull.body, mechanicalRelease, result, reports }) console.log(`enqueue-guard: PR #${pr} head ${short(decision.head)}, labels [${labels.join(', ')}]`) console.log(decision.ok diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index 033ccdd5a..c75036836 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -29,7 +29,7 @@ const report = ({ tip = HEAD, verdict = 'PASS', extra = [] } = {}) => const comment = (body = report(), at = '2026-10-21T10:31:00Z') => ({ user: { login: 'reviewer-agent' }, body, created_at: at, author_association: 'MEMBER' }) -const fakeGh = ({ comments = [], reviews = [], labels = [], files = [], head = HEAD, author = 'opener' } = {}) => { +const fakeGh = ({ body = 'Fixes #602\n', comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, author = 'opener' } = {}) => { const calls = [] const gh = (...args) => { const path = args[args.length - 1] @@ -37,7 +37,7 @@ const fakeGh = ({ comments = [], reviews = [], labels = [], files = [], head = H if (path.includes('/files')) return files.map((f) => JSON.stringify(typeof f === 'string' ? { filename: f } : f)).join('\n') if (path.includes('/comments')) return comments.map((c) => JSON.stringify(c)).join('\n') if (path.includes('/reviews')) return reviews.map((r) => JSON.stringify(r)).join('\n') - if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: head }, user: { login: author }, labels: labels.map((name) => ({ name })) }) + if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: head }, user: { login: author }, body, labels: labels.map((name) => ({ name })) }) // Anything else — `gh pr merge` above all — is the guard doing more than checking. throw new Error('fake gh: unexpected ' + path) } @@ -53,7 +53,7 @@ const run = (fixture = {}) => main({ }) test('#602: a held pull request is not enqueued, whatever a PASS at its tip says', () => { - const r = run({ labels: ['hold'], comments: [comment()] }) + const r = run({ labels: ['hold', 'agent-pr'], comments: [comment()] }) assert.equal(r.ok, false) assert.equal(r.held, true) assert.match(r.reasons.join('; '), /`hold` label/) @@ -150,7 +150,7 @@ test('#602: a T0/T1 pull request still needs no PASS', () => { }) test('#602: hold refuses a T0/T1 pull request too', () => { - const r = run({ files: [TEST_FILE], labels: ['hold'], comments: [] }) + const r = run({ files: [TEST_FILE], labels: ['hold', 'agent-pr'], comments: [] }) assert.equal(r.ok, false) assert.equal(r.held, true) }) @@ -169,6 +169,36 @@ test('#602: the self-gate case (author Birfy, PASS by Birfy) is allowed', () => assert.equal(r.ok, true, r.reasons.join('; ')) }) +// #718: a pull request with no issue behind it, or with no `agent-pr` label, is refused before the +// queue: #711/#712/#713 arrived with neither and the guard said nothing, because it only read `hold`, +// the conclusions and the diff. The release PR is the one exception — it has neither by design. +test('#718: a body that names no issue is not enqueued', () => { + const r = run({ body: '## Summary\n\nTidy up the dock.', comments: [comment()] }) + assert.equal(r.ok, false) + assert.match(r.reasons.join('; '), /no issue reference in the pull request body: add "Fixes #"/) + assert.match(r.message, /^do not enqueue #566: /) +}) + +test('#718: a pull request without the agent-pr label is not enqueued', () => { + const r = run({ body: 'Fixes #566\n\nthe detail', labels: [], comments: [comment()] }) + assert.equal(r.ok, false) + assert.ok(r.reasons.includes('missing label: agent-pr'), r.reasons.join('; ')) +}) + +test('#718: a Closes reference and the agent-pr label let it through', () => { + const gh = fakeGh({ body: 'Closes #714\n\nthe detail', labels: ['agent-pr'], files: [HOST_FILE], comments: [comment()] }) + const r = main({ gh, env: { REVIEW_POLICY: POLICY, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '566' }, repo: 'ThinkFlowLab/FlyBot', pr: 566 }) + assert.equal(r.ok, true, r.reasons.join('; ')) + assert.equal(r.command, `gh pr merge 566 --auto --squash --match-head-commit ${HEAD}`) +}) + +test('#718: a mechanical release pull request needs neither an issue nor the label', () => { + const r = run({ body: 'Version 0.2.1', labels: [], files: ['changelog.d/999-x.changed.md'], comments: [] }) + assert.equal(r.ok, true, r.reasons.join('; ')) + assert.equal(r.exempt, true) + assert.equal(r.mechanicalRelease, true) +}) + // The guard is only in the path if the instructions that tell an agent to enqueue name it; otherwise // the next agent runs `gh pr merge` from memory and #602 is back. test('#602: the instructions that enqueue point at the guard', () => { diff --git a/AGENTS.md b/AGENTS.md index 93288d1e1..e64c18e76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,6 +144,7 @@ What this buys is presence and a name from a closed list, not truth: with one sh - **No implementer writes the Verdict.** On the API side self-review cannot be told apart mechanically — every pull request is opened by the shared `Birfy` account, and `selfReview: ["Birfy"]` in [`review-policy.json`](.github/agent/review-policy.json) makes a PASS always look like Birfy judging Birfy — so what the team holds is the role split: the implementer Bot does not write the Verdict, the Bot that writes the Verdict neither changes the tip nor re-judges its own verdict, and `selfReview` stays as it is because clearing it would leave every pull request without a conclusion. - **`hold` is human-only.** Only a person sets the label and only a person lifts it; automation refuses a held pull request and never removes the label itself. That is the one human input left on the merge path. - **The label awaiting-approval is retired.** A PASS at the tip goes straight to the queue through the enqueue guard (#687); no pull request waits for an approval click and the label is not set again. +- **A pull request enters the queue with its issue and its `agent-pr` label.** The enqueue guard also refuses one whose body names no issue (`Fixes #` / `Closes #`) and one without the `agent-pr` label (#718), so "one issue, one pull request" is checked rather than remembered; a mechanical release PR is the one exemption, because it carries neither by design. Four rules decide what a queue entry must pass. The mechanical judge runs where the merge actually happens: [`merge-gate.yml`](.github/workflows/merge-gate.yml) fails the `merge-gate` job on a `merge_group` entry, and [`merge-gate.mjs`](.github/scripts/merge-gate.mjs) does the judging. From 7abac4811a890f1ee67ac48d5ff51e0f21a61dd1 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Thu, 22 Oct 2026 16:50:53 +0800 Subject: [PATCH 2/3] Exempt a fork from the issue and agent-pr rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules only hold where the branch is ours: a fork's head cannot be pushed by us, and the owner keeps their own workflow there (#711/#713). The test is `head.repo.full_name` against the repository being guarded, so it does not overlap `result.exempt`, which is also true for T0/T1 — routine work on our own branch still names its issue. Refs #718 --- .github/scripts/enqueue-guard.mjs | 20 ++++++++++++++------ .github/scripts/enqueue-guard.test.mjs | 13 +++++++++++-- AGENTS.md | 2 +- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/.github/scripts/enqueue-guard.mjs b/.github/scripts/enqueue-guard.mjs index ed1e47b3c..e737b0f84 100644 --- a/.github/scripts/enqueue-guard.mjs +++ b/.github/scripts/enqueue-guard.mjs @@ -41,6 +41,8 @@ const RECORD_ONLY = /^[\s>*+-]*(?:record only\s*[—–-]+\s*do not (?:merge|que // all — #711/#712/#713 arrived with neither and nothing refused them, because the guard only looked at // `hold`, the conclusions and the diff. The reference is a line of its own, so prose about "fixes" does // not pass; the spellings accepted are exactly `Fixes #` and `Closes #`, behind any `#`, `>` or `-`. +// The rules only hold where the branch is ours: a fork is exempt, because we cannot push to its head +// and the owner keeps their own workflow there (#711/#713). const ISSUE_REF = /^[\s>#-]*(?:fixes|closes)\s*#\d+/im const short = (s) => String(s ?? '').slice(0, SHORT) const labelNames = (labels) => (labels ?? []).map((l) => String(typeof l === 'string' ? l : l?.name ?? '').toLowerCase()) @@ -61,14 +63,16 @@ export function recordOnlyAt(reports, head) { } // The decision, without the network: `result` is merge-gate's `judge` output for this head. -export function guard({ pr, head, labels, body, mechanicalRelease, result, reports }) { +export function guard({ pr, head, labels, body, mechanicalRelease, fork, result, reports }) { const quiet = recordOnlyAt(reports, result.head ?? head) const reasons = [...result.reasons] if (quiet) reasons.push('the newest conclusion at this head says "record only — do not merge"') - // A mechanical release PR carries neither an issue nor the label by design — it is the routine's own - // daily output under the standing approval (AGENTS.md 6) — so it is the one exemption. `result.exempt` - // cannot be used for this: it is true for T0/T1 as well, and routine work still names its issue. - if (!mechanicalRelease) { + // Two exemptions, both about what the pull request is rather than how risky it is. A mechanical release + // PR carries neither an issue nor the label by design — it is the routine's own daily output under the + // standing approval (AGENTS.md 6). A fork is not ours: its head cannot be pushed by us, and the owner runs + // their own workflow there (#711/#713). `result.exempt` cannot stand in for either: it is true for T0/T1 + // as well, and routine work still names its issue. + if (!mechanicalRelease && !fork) { if (!ISSUE_REF.test(String(body ?? ''))) reasons.push('no issue reference in the pull request body: add "Fixes #"') if (!labelNames(labels).includes('agent-pr')) reasons.push('missing label: agent-pr') } @@ -79,6 +83,7 @@ export function guard({ pr, head, labels, body, mechanicalRelease, result, repor reasons, recordOnly: Boolean(quiet), mechanicalRelease: Boolean(mechanicalRelease), + fork: Boolean(fork), labels: labels ?? [], // One loud sentence: a refusal says what to do ("do not enqueue") and why, so it cannot be read // as a warning that some other step may override. @@ -124,7 +129,10 @@ export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prO // believes it has, and `--sha` lets a caller insist on the head it read a moment ago. const result = judge({ labels, files, head, author: pull.user?.login, reports, policy: loadPolicy(policyPath), expectedHead: arg('--sha'), mode: 'queue' }) const mechanicalRelease = isMechanicalRelease(files.map((f) => f.filename)) - const decision = guard({ pr, head, labels, body: pull.body, mechanicalRelease, result, reports }) + // The branch this repository owns, not a contributor's: `head.repo` is null when the fork was deleted, + // which is not ours either. + const fork = pull.head?.repo?.full_name !== repo + const decision = guard({ pr, head, labels, body: pull.body, mechanicalRelease, fork, result, reports }) console.log(`enqueue-guard: PR #${pr} head ${short(decision.head)}, labels [${labels.join(', ')}]`) console.log(decision.ok diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index c75036836..cac033ba9 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -29,7 +29,7 @@ const report = ({ tip = HEAD, verdict = 'PASS', extra = [] } = {}) => const comment = (body = report(), at = '2026-10-21T10:31:00Z') => ({ user: { login: 'reviewer-agent' }, body, created_at: at, author_association: 'MEMBER' }) -const fakeGh = ({ body = 'Fixes #602\n', comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, author = 'opener' } = {}) => { +const fakeGh = ({ body = 'Fixes #602\n', comments = [], reviews = [], labels = ['agent-pr'], files = [], head = HEAD, author = 'opener', headRepo = 'ThinkFlowLab/FlyBot' } = {}) => { const calls = [] const gh = (...args) => { const path = args[args.length - 1] @@ -37,7 +37,7 @@ const fakeGh = ({ body = 'Fixes #602\n', comments = [], reviews = [], labels = [ if (path.includes('/files')) return files.map((f) => JSON.stringify(typeof f === 'string' ? { filename: f } : f)).join('\n') if (path.includes('/comments')) return comments.map((c) => JSON.stringify(c)).join('\n') if (path.includes('/reviews')) return reviews.map((r) => JSON.stringify(r)).join('\n') - if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: head }, user: { login: author }, body, labels: labels.map((name) => ({ name })) }) + if (/\/pulls\/\d+$/.test(path)) return JSON.stringify({ head: { sha: head, repo: { full_name: headRepo } }, user: { login: author }, body, labels: labels.map((name) => ({ name })) }) // Anything else — `gh pr merge` above all — is the guard doing more than checking. throw new Error('fake gh: unexpected ' + path) } @@ -199,6 +199,15 @@ test('#718: a mechanical release pull request needs neither an issue nor the lab assert.equal(r.mechanicalRelease, true) }) +test('#718: a fork is exempt — its head is not ours to ask for an issue', () => { + const r = run({ body: '## Summary\n\nTidy up the dock.', labels: [], files: [TEST_FILE], headRepo: 'HugoXK/FlyBot', comments: [] }) + assert.equal(r.ok, true, r.reasons.join('; ')) + assert.equal(r.fork, true) + // The same pull request on a branch this repository owns is refused, so the exemption is the fork and + // not a missing fixture field. + assert.equal(run({ body: '## Summary\n\nTidy up the dock.', labels: [], files: [TEST_FILE], comments: [] }).ok, false) +}) + // The guard is only in the path if the instructions that tell an agent to enqueue name it; otherwise // the next agent runs `gh pr merge` from memory and #602 is back. test('#602: the instructions that enqueue point at the guard', () => { diff --git a/AGENTS.md b/AGENTS.md index e64c18e76..de3194a25 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,7 +144,7 @@ What this buys is presence and a name from a closed list, not truth: with one sh - **No implementer writes the Verdict.** On the API side self-review cannot be told apart mechanically — every pull request is opened by the shared `Birfy` account, and `selfReview: ["Birfy"]` in [`review-policy.json`](.github/agent/review-policy.json) makes a PASS always look like Birfy judging Birfy — so what the team holds is the role split: the implementer Bot does not write the Verdict, the Bot that writes the Verdict neither changes the tip nor re-judges its own verdict, and `selfReview` stays as it is because clearing it would leave every pull request without a conclusion. - **`hold` is human-only.** Only a person sets the label and only a person lifts it; automation refuses a held pull request and never removes the label itself. That is the one human input left on the merge path. - **The label awaiting-approval is retired.** A PASS at the tip goes straight to the queue through the enqueue guard (#687); no pull request waits for an approval click and the label is not set again. -- **A pull request enters the queue with its issue and its `agent-pr` label.** The enqueue guard also refuses one whose body names no issue (`Fixes #` / `Closes #`) and one without the `agent-pr` label (#718), so "one issue, one pull request" is checked rather than remembered; a mechanical release PR is the one exemption, because it carries neither by design. +- **A pull request enters the queue with its issue and its `agent-pr` label.** On a branch this repository owns, the enqueue guard also refuses one whose body names no issue (`Fixes #` / `Closes #`) and one without the `agent-pr` label (#718), so "one issue, one pull request" is checked rather than remembered. Two exemptions: a mechanical release PR, which carries neither by design, and a **fork** (`pull.head.repo.full_name` is not this repository), whose head we cannot push to and where the contributor keeps their own workflow. Four rules decide what a queue entry must pass. The mechanical judge runs where the merge actually happens: [`merge-gate.yml`](.github/workflows/merge-gate.yml) fails the `merge-gate` job on a `merge_group` entry, and [`merge-gate.mjs`](.github/scripts/merge-gate.mjs) does the judging. From d70f6183ea98aaedbc0b32f6d25ccabdc17244e9 Mon Sep 17 00:00:00 2001 From: Danyang Chen Date: Thu, 22 Oct 2026 17:14:19 +0800 Subject: [PATCH 3/3] Compare the repository name case-insensitively in the fork test The repository name in the API paths is case-insensitive, so `--repo thinkflowlab/flybot` made our own branch read as a fork and the guard answered ALLOW with the two new rules disabled. Both sides are lower-cased now, and a case pins the lower-case spelling: the refusal must still fire for our own branch, and a fork must stay exempt under either spelling. Also replaces the `#NNN` placeholder left in the header comment with #718. Refs #718 --- .github/scripts/enqueue-guard.mjs | 8 +++++--- .github/scripts/enqueue-guard.test.mjs | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/.github/scripts/enqueue-guard.mjs b/.github/scripts/enqueue-guard.mjs index e737b0f84..1012d1050 100644 --- a/.github/scripts/enqueue-guard.mjs +++ b/.github/scripts/enqueue-guard.mjs @@ -12,7 +12,8 @@ // So this is the step immediately before that call. It refuses — exit 1, the reason printed — when // the pull request carries `hold`, when the newest conclusion at its head is not a PASS, or when that // conclusion is itself marked "record only — do not merge". It also refuses a pull request that names -// no issue in its body or carries no `agent-pr` label (#NNN; a mechanical release PR is exempt), so +// no issue in its body or carries no `agent-pr` label (#718; a mechanical release PR and a fork are +// exempt), so // "one issue, one pull request" is checked rather than remembered. It reuses `judge({ mode: 'queue' })`, so // "may this enter the queue" keeps one definition: a rule added to the merge gate is enforced here // without being restated, unless someone deliberately changes both. @@ -130,8 +131,9 @@ export function main({ gh = realGh, env = process.env, repo: repoOption, pr: prO const result = judge({ labels, files, head, author: pull.user?.login, reports, policy: loadPolicy(policyPath), expectedHead: arg('--sha'), mode: 'queue' }) const mechanicalRelease = isMechanicalRelease(files.map((f) => f.filename)) // The branch this repository owns, not a contributor's: `head.repo` is null when the fork was deleted, - // which is not ours either. - const fork = pull.head?.repo?.full_name !== repo + // which is not ours either. Both sides are lower-cased because the repository name in the API paths is + // case-insensitive: `--repo thinkflowlab/flybot` used to read our own branch as a fork and print ALLOW. + const fork = String(pull.head?.repo?.full_name ?? '').toLowerCase() !== String(repo).toLowerCase() const decision = guard({ pr, head, labels, body: pull.body, mechanicalRelease, fork, result, reports }) console.log(`enqueue-guard: PR #${pr} head ${short(decision.head)}, labels [${labels.join(', ')}]`) diff --git a/.github/scripts/enqueue-guard.test.mjs b/.github/scripts/enqueue-guard.test.mjs index cac033ba9..f2838ef78 100644 --- a/.github/scripts/enqueue-guard.test.mjs +++ b/.github/scripts/enqueue-guard.test.mjs @@ -47,8 +47,8 @@ const fakeGh = ({ body = 'Fixes #602\n', comments = [], reviews = [], labels = [ const run = (fixture = {}) => main({ gh: fakeGh({ files: [HOST_FILE], ...fixture }), - env: { REVIEW_POLICY: POLICY, GH_REPO: 'ThinkFlowLab/FlyBot', PR: '566' }, - repo: 'ThinkFlowLab/FlyBot', + env: { REVIEW_POLICY: POLICY, GH_REPO: fixture.repo ?? 'ThinkFlowLab/FlyBot', PR: '566' }, + repo: fixture.repo ?? 'ThinkFlowLab/FlyBot', pr: 566, }) @@ -208,6 +208,24 @@ test('#718: a fork is exempt — its head is not ours to ask for an issue', () = assert.equal(run({ body: '## Summary\n\nTidy up the dock.', labels: [], files: [TEST_FILE], comments: [] }).ok, false) }) +// The repository name in the API paths is case-insensitive, so the ownership test is too: spelling `--repo` +// in lower case used to turn our own branch into a "fork" and let both rules through (#718 INC). +test('#718: a lower-case --repo spelling does not turn our own branch into a fork', () => { + const r = run({ body: '## Summary', labels: [], files: [TEST_FILE], repo: 'thinkflowlab/flybot', comments: [] }) + assert.equal(r.fork, false) + assert.equal(r.ok, false) + assert.match(r.reasons.join('; '), /no issue reference/) + assert.match(r.reasons.join('; '), /missing label: agent-pr/) +}) + +test('#718: a fork stays exempt under any --repo spelling', () => { + for (const repo of ['ThinkFlowLab/FlyBot', 'thinkflowlab/flybot']) { + const r = run({ body: '## Summary', labels: [], files: [TEST_FILE], headRepo: 'HugoXK/FlyBot', repo, comments: [] }) + assert.equal(r.fork, true, repo) + assert.equal(r.ok, true, repo + ' → ' + r.reasons.join('; ')) + } +}) + // The guard is only in the path if the instructions that tell an agent to enqueue name it; otherwise // the next agent runs `gh pr merge` from memory and #602 is back. test('#602: the instructions that enqueue point at the guard', () => {