diff --git a/.github/scripts/issue-gate.js b/.github/scripts/issue-gate.js index c9bbb668..d0ff0d04 100644 --- a/.github/scripts/issue-gate.js +++ b/.github/scripts/issue-gate.js @@ -33,22 +33,36 @@ const isBot = (account) => Boolean(account) && account.type === 'Bot'; * Why this pull request is exempt from the gate, or null if it is not. * * Single source of truth: every caller that acts on a pull request runs this. - * The stale-draft sweep previously re-listed these checks and silently lost the - * bot case. */ -const exemptReason = (pr) => { +const exemptReason = async ({ github, owner, repo, pr }) => { if (isBot(pr.user)) return 'author is a bot'; - if (WRITE_ACCESS.includes(pr.author_association)) { - return `author_association is ${pr.author_association}`; - } if (hasLabel(pr, EXEMPT_LABEL)) return `carries the ${EXEMPT_LABEL} label`; + + const username = pr.user && pr.user.login; + if (!username) return null; + + const permission = await repoPermission({ github, owner, repo, username }); + if (WRITE_PERMISSIONS.includes(permission)) { + return `author has ${permission} permission`; + } return null; }; -// Write access to the repository. CONTRIBUTOR is deliberately absent: GitHub uses -// it for "has previously committed to the repository", which describes every -// returning outside contributor, not a maintainer. Do not add it. -const WRITE_ACCESS = ['OWNER', 'MEMBER', 'COLLABORATOR']; +// Repo roles that skip the gate. `read` / `triage` do not. +const WRITE_PERMISSIONS = ['admin', 'maintain', 'write']; + +/** Highest repo permission for `username`, or null if they are not a collaborator. */ +async function repoPermission({ github, owner, repo, username }) { + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, repo, username, + }); + return data.permission; + } catch (err) { + if (err && err.status === 404) return null; + throw err; + } +} const CLOSING_ISSUES = ` query($owner: String!, $repo: String!, $number: Int!) { @@ -78,7 +92,7 @@ const CLOSING_ISSUES = ` async function checkGate({ github, owner, repo, pr }) { if (pr.state !== 'open') return { passed: true, skipped: 'pull request is not open' }; if (pr.draft) return { passed: true, skipped: 'pull request is a draft' }; - const exempt = exemptReason(pr); + const exempt = await exemptReason({ github, owner, repo, pr }); if (exempt) return { passed: true, skipped: exempt }; const data = await github.graphql(CLOSING_ISSUES, { owner, repo, number: pr.number }); @@ -251,7 +265,7 @@ async function runSweep({ github, core, context, dryRun }) { // Stale drafts. The gate skips drafts entirely, so they never carry the label; // this pass keys off inactivity and applies the shared exemptions itself. for (const pr of prs.filter((p) => p.draft)) { - const exempt = exemptReason(pr); + const exempt = await exemptReason({ github, owner, repo, pr }); if (exempt) { core.info(`#${pr.number}: leaving stale draft alone — ${exempt}`); continue; diff --git a/.github/scripts/issue-gate.test.js b/.github/scripts/issue-gate.test.js index 695df3a2..29989cdc 100644 --- a/.github/scripts/issue-gate.test.js +++ b/.github/scripts/issue-gate.test.js @@ -12,12 +12,18 @@ const { const pull = (over = {}) => ({ number: 1, state: 'open', draft: false, - user: { type: 'User' }, author_association: 'NONE', labels: [], + user: { type: 'User', login: 'alice' }, labels: [], ...over, }); +const notCollaborator = () => { + const err = new Error('Not Found'); + err.status = 404; + throw err; +}; + // `linked` is the list of issues GitHub resolves as closing references. -const stub = (linked) => ({ +const stub = (linked, permission) => ({ graphql: async () => ({ repository: { pullRequest: { closingIssuesReferences: { nodes: linked.map((i) => ({ @@ -26,10 +32,18 @@ const stub = (linked) => ({ })), } } }, }), + rest: { + repos: { + getCollaboratorPermissionLevel: async () => { + if (!permission) return notCollaborator(); + return { data: { permission } }; + }, + }, + }, }); -const run = (linked, over) => - checkGate({ github: stub(linked), owner: 'o', repo: 'r', pr: pull(over) }); +const run = (linked, over, permission) => + checkGate({ github: stub(linked, permission), owner: 'o', repo: 'r', pr: pull(over) }); const cases = [ ['no linked issue fails', () => run([]), (r) => r.passed === false], @@ -45,17 +59,19 @@ const cases = [ (r) => r.passed === true && r.issue === 8], // Exemptions. - ['maintainer skips', () => run([], { author_association: 'MEMBER' }), (r) => r.passed === true], - ['collaborator skips', () => run([], { author_association: 'COLLABORATOR' }), (r) => r.passed === true], + ['write permission skips', () => run([], {}, 'write'), (r) => r.passed === true], + ['maintain permission skips', () => run([], {}, 'maintain'), (r) => r.passed === true], ['bot skips', () => run([], { user: { type: 'Bot' } }), (r) => r.passed === true], ['draft skips', () => run([], { draft: true }), (r) => r.passed === true], [`${EXEMPT_LABEL} skips`, () => run([], { labels: [{ name: EXEMPT_LABEL }] }), (r) => r.passed === true], - // Regression guard: GitHub hands CONTRIBUTOR to anyone who has previously - // committed, i.e. every returning outside contributor. It must stay gated. - ['CONTRIBUTOR is still gated', - () => run([], { author_association: 'CONTRIBUTOR' }), + ['triage permission is still gated', () => run([], {}, 'triage'), (r) => r.passed === false], + ['MEMBER association without write is still gated', + () => run([], { author_association: 'MEMBER' }), (r) => r.passed === false], + ['CONTRIBUTOR with write skips', + () => run([], { author_association: 'CONTRIBUTOR' }, 'write'), + (r) => r.passed === true], ]; // --- findNotices: only the bot's own notices count ------------------------- @@ -81,12 +97,12 @@ const noticeCases = [ // --- runSweep: the stale-draft pass must honour every exemption ------------ const draft = (over) => ({ number: 9, draft: true, state: 'open', labels: [], - user: { type: 'User' }, author_association: 'NONE', + user: { type: 'User', login: 'alice' }, updated_at: new Date(Date.now() - 400 * 86400_000).toISOString(), ...over, }); -async function sweepClosed(pr) { +async function sweepClosed(pr, permission) { const closed = []; const github = { paginate: async (route) => (route === 'pulls' ? [pr] : []), @@ -96,6 +112,12 @@ async function sweepClosed(pr) { update: async ({ pull_number }) => closed.push(pull_number), }, issues: { listComments: 'comments', createComment: async () => {} }, + repos: { + getCollaboratorPermissionLevel: async () => { + if (!permission) return notCollaborator(); + return { data: { permission } }; + }, + }, }, }; await runSweep({ @@ -108,7 +130,7 @@ async function sweepClosed(pr) { const sweepCases = [ ['stale draft from an outside author closes', draft({}), 1], ['stale draft from a bot is left alone', draft({ user: { type: 'Bot' } }), 0], - ['stale draft from a maintainer is left alone', draft({ author_association: 'MEMBER' }), 0], + ['stale draft from a writer is left alone', draft({}), 0, 'write'], [`stale draft with ${EXEMPT_LABEL} is left alone`, draft({ labels: [{ name: EXEMPT_LABEL }] }), 0], ['recent draft is left alone', draft({ updated_at: new Date().toISOString() }), 0], ]; @@ -120,8 +142,8 @@ const sweepCases = [ if (got === want) console.log(` ok ${name}`); else { failed++; console.log(` FAIL ${name} -> ${got} notices, wanted ${want}`); } } - for (const [name, pr, want] of sweepCases) { - const got = (await sweepClosed(pr)).length; + for (const [name, pr, want, permission] of sweepCases) { + const got = (await sweepClosed(pr, permission)).length; if (got === want) console.log(` ok ${name}`); else { failed++; console.log(` FAIL ${name} -> closed ${got}, wanted ${want}`); } }