From b6abb7fb85a1e6f68e4235774ad8fbd12620ddc0 Mon Sep 17 00:00:00 2001 From: FlyBot Dev-3 Date: Tue, 3 Nov 2026 15:35:39 +0000 Subject: [PATCH 1/3] test(gate): pin a ref-based ruleset lookup and a signable list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2314: the gate-ruleset tests drive the live shape — an active ruleset named "main merge queue" on refs/heads/main that requires the Node 24 context, tests-ok and merge-gate and carries the merge_queue rule — and expect --check to name its id, --apply to update it in place, and --apply to refuse a downgrading body. policy-sign's refusals must list the comments that can sign the tip. --- .github/scripts/gate-ruleset.test.mjs | 81 +++++++++++++++++++++++++++ .github/scripts/policy-sign.test.mjs | 44 +++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/.github/scripts/gate-ruleset.test.mjs b/.github/scripts/gate-ruleset.test.mjs index 306da49ba..2c0cde5fb 100644 --- a/.github/scripts/gate-ruleset.test.mjs +++ b/.github/scripts/gate-ruleset.test.mjs @@ -2,6 +2,9 @@ // that would quietly turn it back into a report. import { test } from 'node:test' import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { BODY_PATH, REQUIRED_CONTEXTS, RULESET_NAME, bodyFor, main, problems, render, summarize } from './gate-ruleset.mjs' const REPO = 'Birfy/FlyBot' @@ -193,3 +196,81 @@ test('#812: the printed report names one problem per line, and nothing when it i const good = render({ ok: true, mode: 'check', problems: [], summary: summarize(goodRow()) }) assert.match(good, /OK — main requires the checks and nobody can bypass them/) }) + +// #2314: the ruleset is identified by what it governs, not by its display name. On a11983c5 the only +// active ruleset is `main merge queue` (id 25173372): it requires the Node 24 context, `tests-ok` and +// `merge-gate`, and it carries the `merge_queue` rule. These are the two responses the issue quotes. +const LIVE_ID = 25173372 +const liveRules = () => [ + { type: 'deletion' }, + { type: 'non_fast_forward' }, + { + type: 'required_status_checks', + parameters: { + strict_required_status_checks_policy: false, + required_status_checks: [ + { context: 'ubuntu-latest · Node 24 · DSH 0.2.0-rc.2' }, + { context: 'tests-ok' }, + { context: 'merge-gate' }, + ], + }, + }, + { + type: 'pull_request', + parameters: { + required_approving_review_count: 0, dismiss_stale_reviews_on_push: false, require_code_owner_review: false, + require_last_push_approval: false, required_review_thread_resolution: false, + }, + }, + { type: 'merge_queue', parameters: { merge_method: 'SQUASH', max_entries_to_merge: 1 } }, +] +const liveRow = (name = 'main merge queue') => ({ + id: LIVE_ID, name, target: 'branch', enforcement: 'active', + bypass_actors: [], current_user_can_bypass: 'never', + conditions: { ref_name: { include: ['refs/heads/main'], exclude: [] } }, + rules: liveRules(), +}) + +test('#2314: --check finds the gate by the ref it governs, not by its display name', () => { + const { state, gh, get } = fakeRepo({ rulesets: [liveRow()] }) + const r = main({ mode: 'check', repo: REPO, gh, get }) + assert.equal(state.writes.length, 0, 'check is still read-only') + assert.equal(r.id, LIVE_ID, 'the ruleset the check reads is the live one, named by its id') + const why = r.problems.join('\n') + assert.doesNotMatch(why, /unprotected/, 'main is gated under another name, not unprotected') + assert.match(why, /merge-gate/, 'a context the live gate requires and the file lacks is drift') + assert.match(why, /Node 24|ubuntu-latest/, 'the live Node 24 context is drift too') + assert.match(why, /enqueue-guard|policy-sign/, 'a context the file wants and the live gate lacks is drift as well') + assert.equal(r.ok, false, 'drift in either direction is not OK') +}) + +test('#2314: --apply updates the ruleset that governs main even under another name', (t) => { + // The body that converges the live ruleset: same rules, only the display name still differs. + const body = liveRow('main merge gate') + delete body.id + delete body.current_user_can_bypass + const dir = mkdtempSync(join(tmpdir(), 'gate-ruleset-body-')) + t.after(() => rmSync(dir, { recursive: true, force: true })) + const bodyPath = join(dir, 'body.json') + writeFileSync(bodyPath, JSON.stringify(body)) + const { state, gh, get } = fakeRepo({ rulesets: [liveRow()] }) + const r = main({ mode: 'apply', repo: REPO, gh, get, bodyPath }) + assert.equal(r.action, 'update', 'a differently named ruleset on refs/heads/main is the one to update') + assert.equal(r.id, LIVE_ID) + assert.equal(state.writes.filter((w) => w.method === 'POST').length, 0, 'no second ruleset is created') + const put = state.writes.find((w) => w.method === 'PUT') + assert.ok(put, 'the live ruleset is updated in place') + assert.match(put.path, new RegExp(`/rulesets/${LIVE_ID}$`)) +}) + +test('#2314: --apply refuses a body that would drop a live-required context or the merge queue', () => { + // The repository's own body file: no `merge-gate`, no Node 24 context, and no `merge_queue` rule. + const { state, gh, get } = fakeRepo({ rulesets: [liveRow()] }) + const r = main({ mode: 'apply', repo: REPO, gh, get }) + assert.equal(r.ok, false, 'a downgrade is not a convergence') + const why = r.problems.join('\n') + assert.match(why, /merge-gate/, 'the live verdict gate it would drop is named') + assert.match(why, /Node 24|ubuntu-latest/, 'and the live Node 24 context') + assert.match(why, /merge_queue/, 'and the merge-queue rule it would remove') + assert.equal(state.writes.length, 0, 'nothing is written when the body is a downgrade') +}) diff --git a/.github/scripts/policy-sign.test.mjs b/.github/scripts/policy-sign.test.mjs index 9a5e793f9..56f9a14f7 100644 --- a/.github/scripts/policy-sign.test.mjs +++ b/.github/scripts/policy-sign.test.mjs @@ -254,3 +254,47 @@ test('#2060: a list answer is still paged and flattened', async t => { assert.equal(stub.calls.filter(call => call.path.endsWith('/files')).length, 2, 'a full page asks for the next one') assert.equal(pull.body, 'Fixes #1') }) + +// #2314 Part 2 (P-507): a refusal never said which comment would sign the tip, although `judge()` +// already has the comments and the three signability criteria. It now returns `signable` — at most +// three `{ id, login }`, in comment order — and `main()` prints one line per candidate. +const signable = (id, tip = HEAD) => ({ id, body: `ok at ${tip}`, user: { login: 'Birfy', type: 'User' } }) + +test('#2314: the missing-signature refusal lists the comments that can sign this tip', () => { + const comments = [ + { id: 41, body: `ok at ${HEAD}`, user: { login: 'HugoXK', type: 'User' } }, // the pull request's own author + { id: 42, body: `ok at ${HEAD}`, user: { login: 'Birfy', type: 'User' } }, + { id: 43, body: `ok at ${OTHER_TIP}`, user: { login: 'Birfy', type: 'User' } }, // an older tip + { id: 44, body: `ok at ${HEAD}`, user: { login: 'helper[bot]', type: 'Bot' } }, // a bot account + { id: 45, body: 'ok to change the gate', user: { login: 'Dev3', type: 'User' } }, // names no tip + signable(46), signable(47), signable(48), + ] + const verdict = judge({ files: [GATE_FILE], body: 'Fixes #1', ...PR, head: HEAD, comments, policy }) + assert.equal(verdict.ok, false) + assert.deepEqual(verdict.signable, [ + { id: 42, login: 'Birfy' }, + { id: 46, login: 'Birfy' }, + { id: 47, login: 'Birfy' }, + ], 'at most three, in comment order, and only the comments that can sign this tip') +}) + +test('#2314: a signature for another tip lists the comments that can sign this tip', () => { + const comments = [ + { id: 42, body: `ok at ${OTHER_TIP}`, user: { login: 'Birfy', type: 'User' } }, + signable(77), + ] + const verdict = judge({ files: [GATE_FILE], body: 'Fixes #1\n\nPolicy-sign: 42', ...PR, head: HEAD, comments, policy }) + assert.equal(verdict.ok, false) + assert.match(verdict.reason, /does not name this tip/) + assert.deepEqual(verdict.signable, [{ id: 77, login: 'Birfy' }]) +}) + +test('#2314: main prints one line per signable comment', async () => { + const run = await runMain({ + '/pulls/2060/files': [{ filename: GATE_FILE }], + '/issues/2060/comments': [signable(42)], + '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/pulls/2060': pullAnswer('Fixes #1\n\nTightens the gate.'), + }) + assert.equal(run.code, 1) + assert.match(run.out, /policy-sign: comment 42 by Birfy signs this tip — add `Policy-sign: 42` to the body/) +}) From 31ed7c3ada5d3ac2f22f485f96f0c673b099ddbd Mon Sep 17 00:00:00 2001 From: FlyBot Dev-3 Date: Tue, 3 Nov 2026 15:51:26 +0000 Subject: [PATCH 2/3] test(gate): pin rollback and the refusal-before-request rule --rollback resolves through the same name-based lookup as --check, so a differently named ruleset on refs/heads/main reads as nothing to roll back and the gate stays. The downgrade refusal also has to be decided before the first write: the fake now counts every gh call, not only the writes it records. --- .github/scripts/gate-ruleset.test.mjs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/scripts/gate-ruleset.test.mjs b/.github/scripts/gate-ruleset.test.mjs index 2c0cde5fb..19323fb4c 100644 --- a/.github/scripts/gate-ruleset.test.mjs +++ b/.github/scripts/gate-ruleset.test.mjs @@ -266,11 +266,27 @@ test('#2314: --apply updates the ruleset that governs main even under another na test('#2314: --apply refuses a body that would drop a live-required context or the merge queue', () => { // The repository's own body file: no `merge-gate`, no Node 24 context, and no `merge_queue` rule. const { state, gh, get } = fakeRepo({ rulesets: [liveRow()] }) - const r = main({ mode: 'apply', repo: REPO, gh, get }) + let ghCalls = 0 + const counting = (args, input) => { ghCalls += 1; return gh(args, input) } + const r = main({ mode: 'apply', repo: REPO, gh: counting, get }) assert.equal(r.ok, false, 'a downgrade is not a convergence') const why = r.problems.join('\n') assert.match(why, /merge-gate/, 'the live verdict gate it would drop is named') assert.match(why, /Node 24|ubuntu-latest/, 'and the live Node 24 context') assert.match(why, /merge_queue/, 'and the merge-queue rule it would remove') + assert.equal(ghCalls, 0, 'the refusal is decided before the first write request, not after one') assert.equal(state.writes.length, 0, 'nothing is written when the body is a downgrade') }) + +test('#2314: --rollback deletes the ruleset that governs main, whatever it is called', () => { + // Only the live `main merge queue` exists: a name-based lookup finds nothing, so the old code + // reported "nothing to roll back" and left the gate in place. + const { state, gh, get } = fakeRepo({ rulesets: [liveRow()] }) + const r = main({ mode: 'rollback', repo: REPO, gh, get }) + assert.equal(r.action, 'delete', 'a differently named ruleset that governs main is the one to roll back') + assert.equal(r.id, LIVE_ID) + const deletes = state.writes.filter((w) => w.method === 'DELETE') + assert.equal(deletes.length, 1) + assert.match(deletes[0].path, new RegExp(`/rulesets/${LIVE_ID}$`)) + assert.equal(r.ok, true, r.problems.join('; ')) +}) From 7c9418426bbaa0174aef501c8088049a938052a6 Mon Sep 17 00:00:00 2001 From: FlyBot Dev-3 Date: Tue, 3 Nov 2026 15:53:34 +0000 Subject: [PATCH 3/3] fix(gate): find main's ruleset by the ref it governs, not by its name The live ruleset is called "main merge queue"; the tool looked for "main merge gate", so --check reported main as unprotected, --apply would have created a second active ruleset, and the repository's own body file would have dropped merge-gate, the Node 24 context and the merge_queue rule if it had been applied. Rows are now matched by target branch plus refs/heads/main, drift is reported in both directions, --apply updates that row and refuses a downgrading body before it sends a request, and --rollback deletes what it matched. policy-sign also lists the comments that can sign the tip when it refuses for a missing or stale signature, using the criteria judge() already applied. Fixes #2314 --- .github/scripts/gate-ruleset.mjs | 74 ++++++++++++++++++------ .github/scripts/policy-sign.mjs | 39 +++++++++++-- changelog.d/2314-ruleset-by-ref.fixed.md | 1 + 3 files changed, 92 insertions(+), 22 deletions(-) create mode 100644 changelog.d/2314-ruleset-by-ref.fixed.md diff --git a/.github/scripts/gate-ruleset.mjs b/.github/scripts/gate-ruleset.mjs index 1d245beee..065f9b147 100644 --- a/.github/scripts/gate-ruleset.mjs +++ b/.github/scripts/gate-ruleset.mjs @@ -1,8 +1,13 @@ // The ruleset that makes the check a gate (#812). // +// A ruleset is identified by what it governs, never by its display name: the live one is called +// `main merge queue` while this file is called `main merge gate`, and a name-based lookup read that as +// "main is unprotected" (#2314). `.github/agent/merge-gate-ruleset.json` is the ruleset this file +// converges to. +// // The check run is only half of #812: `enqueue-guard.yml` writes it, and a ruleset on `main` has to -// require it, or the refusal is a red badge a merge endpoint never reads. `.github/agent/ -// merge-gate-ruleset.json` is that ruleset verbatim — branch ruleset, `enforcement: active`, +// require it, or the refusal is a red badge a merge endpoint never reads. That file is the ruleset +// verbatim — branch ruleset, `enforcement: active`, // `bypass_actors: []`, `refs/heads/main`, `tests-ok`, `enqueue-guard` and `policy-sign` required, // `strict_required_status_checks_policy: false` (main moves on every merge, and a strict rule would make // every open pull request rebase before it could merge). @@ -62,8 +67,12 @@ export function summarize(row) { // Everything that makes the gate real, as problems: an empty list is "main is gated". Each one is a // separate line because they fail for different reasons and are fixed in different places. +// +// Drift is read in both directions: a context this file wants and the ruleset lacks, and a context the +// ruleset requires and this file does not carry. The second is the downgrade `--apply` must refuse, and +// on the live ruleset it is the only evidence of the drift a person can see without writing anything. export function problems(row) { - if (!row) return [`no ruleset named "${RULESET_NAME}" on ${BASE_REF}: main is unprotected`] + if (!row) return [`no active ruleset governs ${BASE_REF}: main is unprotected`] const s = summarize(row) const out = [] if (s.enforcement !== 'active') out.push(`the ruleset is "${s.enforcement || 'missing'}", not active`) @@ -72,6 +81,7 @@ export function problems(row) { if (s.canBypass !== 'never') out.push(`current_user_can_bypass is "${s.canBypass ?? 'not reported'}", not "never"`) if (s.strict !== false) out.push(`strict_required_status_checks_policy is ${JSON.stringify(s.strict)}, not false: every open pull request would have to rebase first`) for (const context of REQUIRED_CONTEXTS) if (!s.contexts.includes(context)) out.push(`the required checks do not include "${context}" (they are ${s.contexts.join(', ') || 'none'})`) + for (const context of s.contexts) if (!REQUIRED_CONTEXTS.includes(context)) out.push(`the ruleset also requires "${context}", which the body file does not carry: applying it would drop that context`) return out } @@ -81,8 +91,29 @@ export function bodyFor(path = BODY_PATH) { return loadBody(path) } -export function findByRow(rows, name = RULESET_NAME) { - return (rows ?? []).find((row) => String(row?.name ?? '') === name) ?? null +// The rulesets that govern `main`: a branch ruleset whose `conditions.ref_name.include` names +// `refs/heads/main`. The list endpoint reports no `conditions`, so each row is read in full — which is +// what `--check` needs anyway for `current_user_can_bypass`. +export function findGoverning(repo, get = realGet) { + return (listRulesets(repo, get) ?? []) + .map((row) => (row?.id === undefined ? null : get(`repos/${repo}/rulesets/${row.id}`))) + .filter((row) => String(row?.target ?? '') === 'branch' && (row?.conditions?.ref_name?.include ?? []).includes(BASE_REF)) +} + +// What a body would remove from the ruleset it is applied to: every context the live ruleset requires +// and the body does not carry, and the `merge_queue` rule. The body file in this repository lacks +// `merge-gate`, the Node 24 context and the queue rule, so applying it to the live `main merge queue` +// would weaken the gate it is supposed to describe. +export function downgrade(live, body) { + const s = summarize(live) + const wanted = summarize({ rules: body?.rules ?? [] }) + const out = [] + for (const context of s.contexts) { + if (!wanted.contexts.includes(context)) out.push(`the body drops "${context}", which the live ruleset requires: refusing to apply a downgrade`) + } + const hasQueue = (rules) => (rules ?? []).some((rule) => rule?.type === 'merge_queue') + if (hasQueue(live?.rules) && !hasQueue(body?.rules)) out.push('the body drops the merge_queue rule, which the live ruleset requires: refusing to apply a downgrade') + return out } // `gh`/`get` are injectable so a test drives every mode without the network (`gh(args, input)`, the way @@ -91,28 +122,31 @@ export function main({ mode = 'check', repo, gh = realGh, get = realGet, bodyPat if (!repo) throw new Error('set GH_REPO or pass --repo owner/name') if (mode === 'apply') { const body = bodyFor(bodyPath) - const existing = findByRow(listRulesets(repo, get)) + const [existing] = findGoverning(repo, get) + // Decided before the first request: a downgrade that is discovered after the PUT is already written. + const refused = existing ? downgrade(existing, body) : [] + if (refused.length) { + return { ok: false, mode, action: 'refused', id: existing.id, name: existing.name ?? '', summary: summarize(existing), problems: refused } + } const saved = existing // PUT, not PATCH: the rulesets API answers 404 to a PATCH on a repository ruleset. ? JSON.parse(gh(['api', '-X', 'PUT', `repos/${repo}/rulesets/${existing.id}`, '--input', '-'], JSON.stringify(body))) : JSON.parse(gh(['api', '-X', 'POST', `repos/${repo}/rulesets`, '--input', '-'], JSON.stringify(body))) const readBack = get(`repos/${repo}/rulesets/${saved.id}`) const found = problems(readBack) - return { ok: found.length === 0, mode, action: existing ? 'update' : 'create', id: saved.id, summary: summarize(readBack), problems: found } + return { ok: found.length === 0, mode, action: existing ? 'update' : 'create', id: saved.id, name: readBack?.name ?? body.name ?? '', summary: summarize(readBack), problems: found } } if (mode === 'rollback') { - const existing = findByRow(listRulesets(repo, get)) + const [existing] = findGoverning(repo, get) if (!existing) return { ok: true, mode, action: 'none', problems: [] } gh(['api', '-X', 'DELETE', `repos/${repo}/rulesets/${existing.id}`]) - const left = findByRow(listRulesets(repo, get)) - return { ok: !left, mode, action: 'delete', id: existing.id, problems: left ? [`the ruleset ${existing.id} is still listed after the delete`] : [] } + const left = findGoverning(repo, get)[0] + return { ok: !left, mode, action: 'delete', id: existing.id, name: existing.name ?? '', problems: left ? [`the ruleset ${left.id} is still listed after the delete`] : [] } } - const rows = listRulesets(repo, get) - const hit = findByRow(rows) + const [hit] = findGoverning(repo, get) if (!hit) return { ok: false, mode: 'check', problems: problems(null), summary: null } - const row = get(`repos/${repo}/rulesets/${hit.id}`) - const found = problems(row) - return { ok: found.length === 0, mode: 'check', id: hit.id, summary: summarize(row), problems: found } + const found = problems(hit) + return { ok: found.length === 0, mode: 'check', id: hit.id, summary: summarize(hit), problems: found } } export function listRulesets(repo, get = realGet) { @@ -121,8 +155,14 @@ export function listRulesets(repo, get = realGet) { export function render(result) { const lines = [] - if (result.mode === 'apply') lines.push(`gate-ruleset: ${result.action}d "${RULESET_NAME}" (id ${result.id})`) - else if (result.mode === 'rollback') lines.push(result.action === 'delete' ? `gate-ruleset: deleted "${RULESET_NAME}" (id ${result.id})` : `gate-ruleset: no ruleset named "${RULESET_NAME}" to roll back`) + const label = result.name || result.summary?.name || RULESET_NAME + if (result.mode === 'apply') { + lines.push(result.action === 'refused' + ? `gate-ruleset: refused to write "${label}" (id ${result.id})` + : `gate-ruleset: ${result.action}d "${label}" (id ${result.id})`) + } else if (result.mode === 'rollback') { + lines.push(result.action === 'delete' ? `gate-ruleset: deleted "${label}" (id ${result.id})` : `gate-ruleset: nothing governs ${BASE_REF} to roll back`) + } if (result.summary) { const s = result.summary lines.push(`gate-ruleset: ${s.enforcement}, ${s.target} ${s.refs.join(', ')}, bypass [${s.bypass.join(', ')}], can-bypass ${s.canBypass ?? 'not reported'}, strict ${JSON.stringify(s.strict)}, requires [${s.contexts.join(', ')}], approvals ${s.approvals ?? '—'}`) diff --git a/.github/scripts/policy-sign.mjs b/.github/scripts/policy-sign.mjs index 3c7c45556..6e4048d9a 100644 --- a/.github/scripts/policy-sign.mjs +++ b/.github/scripts/policy-sign.mjs @@ -49,14 +49,42 @@ export function parseSignature(body = '') { return url === null ? value : url[1] } +// A comment signs a tip when it names it. Twelve characters are enough: the comment quotes a sha a +// person pasted, and the comparison is a prefix of the tip, the way the gate reads a verdict. +function namesTip(comment, tip) { + const tokens = String(comment?.body ?? '').toLowerCase().match(/[0-9a-f]{12,40}/g) ?? [] + return tokens.some(value => tip.startsWith(value)) +} + +// The three signability criteria, in one place: the refusal refuses with them, and lists the comments +// that would satisfy them (#2314). A gate change needs another person, not the author and not a bot, +// and the comment has to name the tip it signs. +function canSign(comment, { author, tip }) { + if (tip === '') return false + const login = String(comment?.user?.login ?? '') + if (login !== '' && login === String(author)) return false + if (comment?.user?.type !== 'User') return false + return namesTip(comment, tip) +} + +// At most three: a refusal is a remedy, not a list of every comment on the pull request. +export function signable(comments = [], { author = '', tip = '', limit = 3 } = {}) { + return comments + .filter(comment => canSign(comment, { author, tip })) + .slice(0, limit) + .map(comment => ({ id: comment.id, login: String(comment.user?.login ?? '') })) +} + export function judge({ files = [], body = '', author = '', comments = [], head = '', policy = loadPolicy() }) { const touched = gateFiles(files, policy) if (touched.length === 0) return { ok: true, reason: 'no gate file changed, so no signature is needed' } const named = touched.length === 1 ? touched[0] : `${touched.length} gate files (${touched.join(', ')})` + const tip = String(head ?? '').trim().toLowerCase() + const candidates = signable(comments, { author, tip }) const token = parseSignature(body) if (token === undefined) { - return { ok: false, reason: `a gate file changed (${named}), and the body carries no \`Policy-sign:\` line naming a comment` } + return { ok: false, reason: `a gate file changed (${named}), and the body carries no \`Policy-sign:\` line naming a comment`, signable: candidates } } const comment = comments.find(entry => String(entry?.id ?? '') === String(token)) if (comment === undefined) { @@ -71,13 +99,11 @@ export function judge({ files = [], body = '', author = '', comments = [], head } // The comment has to name the tip it signs. A comment reviewed one change set; without this, the // author can sign a harmless tip and then push the change the signature was supposed to cover. - const tip = String(head ?? '').trim().toLowerCase() if (tip === '') { return { ok: false, reason: `\`Policy-sign:\` names comment ${token}, but this run carries no head sha to bind it to — the check fails closed` } } - const tokens = String(comment.body ?? '').toLowerCase().match(/[0-9a-f]{12,40}/g) ?? [] - if (!tokens.some(value => tip.startsWith(value))) { - return { ok: false, reason: `\`Policy-sign:\` names comment ${token}, which does not name this tip ${String(head).trim()} — a signature binds the tip it was written for, so a new tip needs a new signature` } + if (!namesTip(comment, tip)) { + return { ok: false, reason: `\`Policy-sign:\` names comment ${token}, which does not name this tip ${String(head).trim()} — a signature binds the tip it was written for, so a new tip needs a new signature`, signable: candidates } } return { ok: true, reason: `signed by ${login} in comment ${token}`, signer: login } } @@ -149,6 +175,9 @@ export async function main(argv = process.argv.slice(2), env = process.env) { return 0 } process.stdout.write(`policy-sign refused (#${number}): ${verdict.reason}\n`) + for (const candidate of verdict.signable ?? []) { + process.stdout.write(`policy-sign: comment ${candidate.id} by ${candidate.login} signs this tip — add \`Policy-sign: ${candidate.id}\` to the body\n`) + } process.stdout.write(`::error::${verdict.reason}\n`) return 1 } diff --git a/changelog.d/2314-ruleset-by-ref.fixed.md b/changelog.d/2314-ruleset-by-ref.fixed.md new file mode 100644 index 000000000..f2c34b91a --- /dev/null +++ b/changelog.d/2314-ruleset-by-ref.fixed.md @@ -0,0 +1 @@ +`gate-ruleset` now finds `main`'s ruleset by the ref it governs instead of by its display name, so the live `main merge queue` reads as gated rather than as "main is unprotected": `--check` names the live id and reports drift in both directions (a context the file wants and the ruleset lacks, and a context the ruleset requires and the file does not carry), `--apply` updates that ruleset instead of creating a second one and refuses a body that would drop a live-required context or the `merge_queue` rule before it sends the request, and `--rollback` deletes the same rows it matched. `policy-sign` also names the comments that can sign the tip when it refuses for a missing or stale signature ([#2314](https://github.com/ThinkFlowLab/FlyBot/issues/2314)).