diff --git a/.github/scripts/assign.js b/.github/scripts/assign.js new file mode 100644 index 00000000..b105fb30 --- /dev/null +++ b/.github/scripts/assign.js @@ -0,0 +1,86 @@ +// Handles "/assign" and "/unassign" comments on issues (see .github/workflows/assign.yaml). +// It only compares the comment text with fixed strings: nothing from the comment is ever run. + +const MESSAGES = { + assigned: 'Assigned to you. Open a PR within a week or comment if you need more time.', + alreadyYours: 'This one is already assigned to you. Open a PR within a week or comment if you need more time.', + taken: (owners) => + `Thanks for offering to help! ${owners} is already working on this one. ` + + 'Have a look at the issues labeled `good first issue` or `help wanted` for something free, ' + + 'or comment here if you would like to team up.', + unassigned: 'Unassigned you. Thanks for letting us know! The issue is free for someone else now.', + notYours: (owners) => + `Only the person working on this (${owners}) can use \`/unassign\` here. ` + + 'If something is stuck, a maintainer can help.', + notAssigned: 'Nobody is assigned to this issue, so there is nothing to unassign. Comment `/assign` to take it.', + closed: 'This issue is closed, so there is nothing to pick up here. Thanks for offering!', + failed: 'Sorry, I could not assign you. A maintainer will take a look.', +}; + +/** Returns "assign", "unassign" or null. The whole comment must be the command. */ +function parseCommand(body) { + const text = String(body || '').trim(); + if (text === '/assign') return 'assign'; + if (text === '/unassign') return 'unassign'; + return null; +} + +function mentionAll(logins) { + return logins.map((login) => `@${login}`).join(', '); +} + +/** + * Decides what to do, without touching GitHub. + * Returns { action: "assign" | "unassign" | "none", reply: string | null }. + */ +function decide({ command, commenter, assignees, issueState }) { + const owners = assignees.map((a) => a.toLowerCase()); + const me = commenter.toLowerCase(); + if (issueState !== 'open') return { action: 'none', reply: MESSAGES.closed }; + if (command === 'assign') { + if (owners.includes(me)) return { action: 'none', reply: MESSAGES.alreadyYours }; + if (assignees.length > 0) return { action: 'none', reply: MESSAGES.taken(mentionAll(assignees)) }; + return { action: 'assign', reply: MESSAGES.assigned }; + } + if (command === 'unassign') { + if (owners.includes(me)) return { action: 'unassign', reply: MESSAGES.unassigned }; + if (assignees.length > 0) return { action: 'none', reply: MESSAGES.notYours(mentionAll(assignees)) }; + return { action: 'none', reply: MESSAGES.notAssigned }; + } + return { action: 'none', reply: null }; +} + +/** Entry point for actions/github-script: `await require('./.github/scripts/assign.js')({ github, context, core })`. */ +async function run({ github, context, core }) { + const { issue, comment } = context.payload; + if (!issue || !comment) return core.info('Not an issue comment.'); + if (issue.pull_request) return core.info('Comment is on a pull request: ignored.'); + if (comment.user.type === 'Bot') return core.info('Comment is from a bot: ignored.'); + const command = parseCommand(comment.body); + if (!command) return core.info('Not a command.'); + + const { owner, repo } = context.repo; + const issue_number = issue.number; + const commenter = comment.user.login; + // The payload is a snapshot from when the comment was made; read the issue now so two quick claims see each other. + const { data: current } = await github.rest.issues.get({ owner, repo, issue_number }); + const assignees = (current.assignees || []).map((a) => a.login); + const { action, reply } = decide({ command, commenter, assignees, issueState: current.state }); + let text = reply; + + if (action === 'assign') { + const { data } = await github.rest.issues.addAssignees({ owner, repo, issue_number, assignees: [commenter] }); + // GitHub silently drops people it can't assign, so check that it worked. + const now = (data.assignees || []).map((a) => a.login.toLowerCase()); + if (!now.includes(commenter.toLowerCase())) text = MESSAGES.failed; + } else if (action === 'unassign') { + await github.rest.issues.removeAssignees({ owner, repo, issue_number, assignees: [commenter] }); + } + core.info(`/${command} by ${commenter} on #${issue_number}: ${action}`); + if (text) await github.rest.issues.createComment({ owner, repo, issue_number, body: `@${commenter} ${text}` }); +} + +module.exports = run; +module.exports.parseCommand = parseCommand; +module.exports.decide = decide; +module.exports.MESSAGES = MESSAGES; diff --git a/.github/scripts/assign.test.js b/.github/scripts/assign.test.js new file mode 100644 index 00000000..e6f00198 --- /dev/null +++ b/.github/scripts/assign.test.js @@ -0,0 +1,145 @@ +// Run with: node --test ".github/scripts/*.test.js" +const test = require('node:test'); +const assert = require('node:assert/strict'); +const run = require('./assign.js'); +const { parseCommand, decide, MESSAGES } = run; + +// `live` is the issue as GitHub has it now, which can differ from the comment's payload. +function fakeGithub({ assignable = true, live = null } = {}) { + const calls = []; + const github = { + rest: { + issues: { + get: async () => ({ data: live }), + addAssignees: async (args) => { + calls.push(['addAssignees', args]); + return { data: { assignees: assignable ? args.assignees.map((login) => ({ login })) : [] } }; + }, + removeAssignees: async (args) => calls.push(['removeAssignees', args]), + createComment: async (args) => calls.push(['createComment', args]), + }, + }, + }; + return { github, calls }; +} + +const core = { info: () => {} }; + +// Runs the handler. GitHub's live issue matches the comment's payload unless the fake was given `live`. +function runWith(github, ctx) { + const { state, assignees } = ctx.payload.issue; + const get = github.rest.issues.get; + github.rest.issues.get = async (args) => ({ data: (await get(args)).data || { state, assignees } }); + return run({ github, context: ctx, core }); +} + +function context({ body, login = 'maya-rao', assignees = [], state = 'open', pr = false, type = 'User' }) { + return { + repo: { owner: 'existence-master', repo: 'Sentient' }, + payload: { + issue: { + number: 42, + state, + assignees: assignees.map((a) => ({ login: a })), + ...(pr ? { pull_request: { url: 'https://api.github.com/x' } } : {}), + }, + comment: { body, user: { login, type } }, + }, + }; +} + +test('only an exact command counts', () => { + assert.equal(parseCommand('/assign'), 'assign'); + assert.equal(parseCommand(' /assign\n'), 'assign'); + assert.equal(parseCommand('/unassign'), 'unassign'); + assert.equal(parseCommand('/assign please'), null); + assert.equal(parseCommand('I will /assign myself'), null); + assert.equal(parseCommand('/assign @someone'), null); + assert.equal(parseCommand(''), null); + assert.equal(parseCommand(undefined), null); +}); + +test('decide covers every case', () => { + const free = { commenter: 'maya', assignees: [], issueState: 'open' }; + assert.equal(decide({ ...free, command: 'assign' }).action, 'assign'); + assert.equal(decide({ ...free, command: 'assign', assignees: ['Maya'] }).action, 'none'); + assert.equal(decide({ ...free, command: 'assign', assignees: ['Maya'] }).reply, MESSAGES.alreadyYours); + const taken = decide({ ...free, command: 'assign', assignees: ['ravi'] }); + assert.equal(taken.action, 'none'); + assert.match(taken.reply, /@ravi is already working on this/); + assert.equal(decide({ ...free, command: 'unassign', assignees: ['maya'] }).action, 'unassign'); + assert.match(decide({ ...free, command: 'unassign', assignees: ['ravi'] }).reply, /Only the person working on this \(@ravi\)/); + assert.equal(decide({ ...free, command: 'unassign' }).reply, MESSAGES.notAssigned); + assert.equal(decide({ ...free, command: 'assign', issueState: 'closed' }).reply, MESSAGES.closed); +}); + +test('/assign on a free issue assigns the commenter and says so', async () => { + const { github, calls } = fakeGithub(); + await runWith(github, context({ body: '/assign' })); + assert.deepEqual(calls[0], ['addAssignees', { owner: 'existence-master', repo: 'Sentient', issue_number: 42, assignees: ['maya-rao'] }]); + assert.equal(calls[1][0], 'createComment'); + assert.equal(calls[1][1].body, `@maya-rao ${MESSAGES.assigned}`); + assert.equal(calls.length, 2); +}); + +test('/assign on a taken issue only replies with who has it', async () => { + const { github, calls } = fakeGithub(); + await runWith(github, context({ body: '/assign', assignees: ['ravi'] })); + assert.equal(calls.length, 1); + assert.equal(calls[0][0], 'createComment'); + assert.match(calls[0][1].body, /^@maya-rao Thanks for offering to help! @ravi is already working on this one/); +}); + +test('/unassign by the assignee removes them', async () => { + const { github, calls } = fakeGithub(); + await runWith(github, context({ body: '/unassign', assignees: ['maya-rao'] })); + assert.deepEqual(calls[0], ['removeAssignees', { owner: 'existence-master', repo: 'Sentient', issue_number: 42, assignees: ['maya-rao'] }]); + assert.equal(calls[1][1].body, `@maya-rao ${MESSAGES.unassigned}`); +}); + +test('/unassign by someone else changes nothing', async () => { + const { github, calls } = fakeGithub(); + await runWith(github, context({ body: '/unassign', assignees: ['ravi'] })); + assert.equal(calls.length, 1); + assert.equal(calls[0][0], 'createComment'); +}); + +test('comments on pull requests, from bots, and other text are ignored', async () => { + for (const ctx of [ + context({ body: '/assign', pr: true }), + context({ body: '/assign', type: 'Bot', login: 'coderabbitai[bot]' }), + context({ body: 'looks good, /assign me later' }), + ]) { + const { github, calls } = fakeGithub(); + await runWith(github, ctx); + assert.equal(calls.length, 0); + } +}); + +test('a silently dropped assignment is reported, not claimed', async () => { + const { github, calls } = fakeGithub({ assignable: false }); + await runWith(github, context({ body: '/assign' })); + assert.equal(calls[1][1].body, `@maya-rao ${MESSAGES.failed}`); +}); + +test('two quick /assign comments: the second sees the first claim', async () => { + // Both payloads say the issue is free, but by the time the second job runs GitHub has the first claimer. + const { github, calls } = fakeGithub({ live: { state: 'open', assignees: [{ login: 'ravi' }] } }); + await runWith(github, context({ body: '/assign' })); + assert.equal(calls.length, 1); + assert.match(calls[0][1].body, /@ravi is already working on this one/); +}); + +test('an issue closed since the comment is not assigned', async () => { + const { github, calls } = fakeGithub({ live: { state: 'closed', assignees: [] } }); + await runWith(github, context({ body: '/assign' })); + assert.deepEqual(calls.map((c) => c[0]), ['createComment']); + assert.equal(calls[0][1].body, `@maya-rao ${MESSAGES.closed}`); +}); + +test('the comment text never reaches the API or a reply', async () => { + const { github, calls } = fakeGithub(); + const body = '/assign\n$(curl evil) ${{ secrets.X }}'; + await runWith(github, context({ body })); + assert.equal(calls.length, 0); +}); diff --git a/.github/scripts/stale.js b/.github/scripts/stale.js new file mode 100644 index 00000000..9ffabd2c --- /dev/null +++ b/.github/scripts/stale.js @@ -0,0 +1,308 @@ +// Daily housekeeping for contributor pull requests and issue claims (see .github/workflows/stale.yaml). +// +// Pull requests (not maintainers', not drafts) whose latest review asks for changes, with no new commit or +// comment from the author since: +// 7 days -> label "waiting on author" and one friendly nudge. Only one nudge per PR, ever: a PR that has had a +// nudge comment or the label (now or before) only gets the label back, quietly. +// 21 days -> close with a kind note (only if the last nudge or label is at least 7 days old) and unassign the +// author from the issues the PR closes +// author pushes or comments -> the label comes off +// Issues claimed with /assign by someone who isn't a maintainer, with no open PR from them and no comment from +// them since: +// 14 days -> a friendly ping +// 21 days -> unassigned with a note (only if the ping went out at least 7 days earlier) +// +// With dryRun every decision is logged and nothing on GitHub changes. + +const MAINTAINERS = ['itsskofficial', 'kabeer2004', 'dependabot', 'dependabot[bot]']; +const BOT = 'github-actions[bot]'; +const LABEL = { name: 'waiting on author', color: 'fef2c0', description: 'A maintainer asked for changes; waiting for the author' }; +const DAY = 24 * 60 * 60 * 1000; +const LIMITS = { nudge: 7, close: 21, claimPing: 14, claimDrop: 21, grace: 7 }; +const MARK = { + nudge: '', + close: '', + claimPing: '', + claimDrop: '', +}; + +const MESSAGES = { + nudge: (author) => + `Hi @${author}, just a friendly check-in. A maintainer asked for a few changes here about a week ago. ` + + 'Whenever you are ready, push an update or reply with any questions, and we are happy to help. ' + + 'If you no longer have time for it, that is completely fine too, just let us know.\n\n' + MARK.nudge, + close: (author, issues) => + `Hi @${author}, we are closing this for now because it has been waiting on changes for three weeks. ` + + 'Thank you for the work you put into it! You can reopen it any time, or comment here and we will reopen it ' + + 'for you.' + + (issues.length + ? ` We have unassigned you from ${issues.map((n) => `#${n}`).join(', ')} so someone else can pick it up; ` + + 'comment `/assign` there if you want it back while it is still free.' + : '') + + '\n\n' + MARK.close, + claimPing: (user) => + `Hi @${user}, checking in on this one. Are you still working on it? A quick comment or a draft PR is ` + + 'perfect. If you have moved on, comment `/unassign` so someone else can pick it up. No pressure either way.' + + '\n\n' + MARK.claimPing, + claimDrop: (user) => + `Hi @${user}, we have unassigned you because we have not heard back in a while, so others can pick this up. ` + + 'Thanks for your interest! If you want it back, comment `/assign` while it is still free.' + + '\n\n' + MARK.claimDrop, +}; + +const lower = (s) => String(s || '').toLowerCase(); +const isMaintainer = (login) => MAINTAINERS.includes(lower(login)); +const isBot = (user) => !user || user.type === 'Bot' || lower(user.login).endsWith('[bot]'); +const time = (s) => (s ? new Date(s).getTime() : 0); +const days = (ms) => Math.floor((ms / DAY) * 10) / 10; +const dateOnly = (ms) => new Date(ms).toISOString().slice(0, 10); +const actorOf = (e) => lower((e.actor && e.actor.login) || (e.user && e.user.login)); + +/** The latest review that approved, dismissed or asked for changes, from someone other than the author or a bot. */ +function latestDecisiveReview(reviews, author) { + const decisive = reviews + .filter((r) => ['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED'].includes(r.state)) + .filter((r) => !isBot(r.user) && lower(r.user.login) !== lower(author)) + .sort((a, b) => time(a.submitted_at) - time(b.submitted_at)); + return decisive[decisive.length - 1] || null; +} + +/** The last time the author pushed or commented, from the PR timeline. Any new commit or a reopen counts. */ +function lastAuthorActivity(timeline, author) { + const me = lower(author); + let last = 0; + for (const e of timeline) { + let at = 0; + if (e.event === 'committed') at = time((e.committer && e.committer.date) || (e.author && e.author.date)); + else if (e.event === 'head_ref_force_pushed' && actorOf(e) === me) at = time(e.created_at); + else if (e.event === 'reopened') at = time(e.created_at); // reopened by anyone: it is active again + else if (e.event === 'commented' && actorOf(e) === me) at = time(e.created_at); + else if (e.event === 'reviewed' && actorOf(e) === me) at = time(e.submitted_at); + else if (e.event === 'line-commented') { + for (const c of e.comments || []) if (lower(c.user && c.user.login) === me) at = Math.max(at, time(c.created_at)); + } + last = Math.max(last, at); + } + return last; +} + +/** When the bot last posted a comment carrying `mark` after `since` (0 if never). */ +function botCommentAfter(timeline, mark, since, mention) { + let at = 0; + for (const e of timeline) { + if (e.event !== 'commented' || actorOf(e) !== BOT) continue; + const body = String(e.body || ''); + if (!body.includes(mark) || (mention && !lower(body).includes(`@${lower(mention)}`))) continue; + if (time(e.created_at) > since) at = Math.max(at, time(e.created_at)); + } + return at; +} + +/** + * Earlier warnings on a PR. `nudged`: the bot's nudge comment or the label was ever there (so no second nudge). + * `warnedAt`: the latest nudge comment or labeling, which the close waits a week after. + */ +function prWarnings(timeline) { + let nudged = false; + let warnedAt = 0; + for (const e of timeline) { + const isNudge = e.event === 'commented' && actorOf(e) === BOT && String(e.body || '').includes(MARK.nudge); + const isLabel = e.event === 'labeled' && lower(e.label && e.label.name) === LABEL.name; + if (isNudge || isLabel) { + nudged = true; + warnedAt = Math.max(warnedAt, time(e.created_at)); + } + } + return { nudged, warnedAt }; +} + +/** Whether a PR counts as waiting on its author, and since when. */ +function prWaiting({ author, reviews, timeline }) { + const review = latestDecisiveReview(reviews, author); + if (!review || review.state !== 'CHANGES_REQUESTED') return { waiting: false, since: 0, reviewer: null }; + const since = time(review.submitted_at); + const waiting = lastAuthorActivity(timeline, author) <= since; + return { waiting, since, reviewer: review.user.login }; +} + +/** + * Pure decision for one PR. Returns { action, note }. Actions: none, unlabel, wait, nudge, label, close. + * `nudged` and `warnedAt` come from prWarnings; a PR that has the label now also counts as nudged. + */ +function decidePr({ waiting, since, nudged, warnedAt, hasLabel, now }) { + if (!waiting) return hasLabel ? { action: 'unlabel', note: 'author is active again' } : { action: 'none', note: '' }; + const waited = days(now - since); + if (now - since < LIMITS.nudge * DAY) { + return { action: 'wait', note: `waiting ${waited} days; nudge due ${dateOnly(since + LIMITS.nudge * DAY)}` }; + } + if (!nudged && !hasLabel) return { action: 'nudge', note: `waiting ${waited} days` }; + // Already nudged once: never comment again. Close a week after the last warning, at the earliest. + const lastWarning = warnedAt || now; + const closeAt = Math.max(since + LIMITS.close * DAY, lastWarning + LIMITS.grace * DAY); + if (now >= closeAt) return { action: 'close', note: `waiting ${waited} days, warned ${dateOnly(lastWarning)}` }; + if (!hasLabel) return { action: 'label', note: `waiting ${waited} days, nudged before; label only, no comment` }; + return { action: 'wait', note: `waiting ${waited} days, already nudged; close due ${dateOnly(closeAt)}` }; +} + +/** Pure decision for one claim. Actions: none, wait, ping, unassign. */ +function decideClaim({ viaCommand, hasOpenPr, lastActive, pingedAt, now }) { + if (!viaCommand) return { action: 'none', note: 'not claimed with /assign' }; + if (hasOpenPr) return { action: 'none', note: 'has an open pull request' }; + const quiet = (now - lastActive) / DAY; + if (!pingedAt) { + if (quiet >= LIMITS.claimPing) return { action: 'ping', note: `quiet ${days(now - lastActive)} days` }; + return { action: 'wait', note: `quiet ${days(now - lastActive)} days; ping due ${dateOnly(lastActive + LIMITS.claimPing * DAY)}` }; + } + const dropAt = Math.max(lastActive + LIMITS.claimDrop * DAY, pingedAt + LIMITS.grace * DAY); + if (now >= dropAt) return { action: 'unassign', note: `quiet ${days(now - lastActive)} days, pinged ${dateOnly(pingedAt)}` }; + return { action: 'wait', note: `quiet ${days(now - lastActive)} days, pinged; unassign due ${dateOnly(dropAt)}` }; +} + +/** Facts about one assignee of an issue, from the issue timeline. */ +function claimFacts(timeline, user) { + const me = lower(user); + let assignedAt = 0; + let viaCommand = false; + let lastComment = 0; + let hasOpenPr = false; + for (const e of timeline) { + if (e.event === 'assigned' && lower(e.assignee && e.assignee.login) === me && time(e.created_at) >= assignedAt) { + assignedAt = time(e.created_at); + viaCommand = actorOf(e) === BOT; + } else if (e.event === 'commented' && actorOf(e) === me) { + lastComment = Math.max(lastComment, time(e.created_at)); + } else if (e.event === 'cross-referenced') { + const src = e.source && e.source.issue; + if (src && src.pull_request && src.state === 'open' && lower(src.user && src.user.login) === me) hasOpenPr = true; + } + } + const lastActive = Math.max(assignedAt, lastComment); + return { viaCommand, hasOpenPr, lastActive, pingedAt: botCommentAfter(timeline, MARK.claimPing, lastActive, user) }; +} + +async function linkedIssues(github, owner, repo, number) { + const q = `query($owner:String!,$repo:String!,$n:Int!){repository(owner:$owner,name:$repo){pullRequest(number:$n){ + closingIssuesReferences(first:20){nodes{number assignees(first:20){nodes{login}}}}}}}`; + const res = await github.graphql(q, { owner, repo, n: number }); + return res.repository.pullRequest.closingIssuesReferences.nodes.map((i) => ({ + number: i.number, + assignees: i.assignees.nodes.map((a) => a.login), + })); +} + +/** Entry point for actions/github-script. */ +async function run({ github, context, core, dryRun = true, now = Date.now() }) { + const { owner, repo } = context.repo; + const rows = []; + const say = (kind, number, who, action, note) => { + rows.push([kind, `#${number}`, who, action, note]); + core.info(`${dryRun && !['none', 'wait'].includes(action) ? '[dry run] would ' : ''}${action} ${kind} #${number} (${who}): ${note}`); + }; + const write = async (fn) => { + if (!dryRun) await fn(); + }; + core.info(dryRun ? 'Dry run: nothing on GitHub will change.' : 'Live run.'); + const timelineOf = (issue_number) => + github.paginate(github.rest.issues.listEventsForTimeline, { owner, repo, issue_number, per_page: 100 }); + + let labelReady = false; + const ensureLabel = async () => { + if (labelReady || dryRun) return; + try { + await github.rest.issues.createLabel({ owner, repo, ...LABEL }); + } catch (err) { + if (err.status !== 422) throw err; // 422: it already exists + } + labelReady = true; + }; + + const pulls = await github.paginate(github.rest.pulls.list, { owner, repo, state: 'open', per_page: 100 }); + for (const pr of pulls) { + const author = pr.user.login; + if (isMaintainer(author) || isBot(pr.user)) continue; + if (pr.draft) { + say('PR', pr.number, author, 'none', 'draft'); + continue; + } + const [reviews, timeline] = await Promise.all([ + github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number: pr.number, per_page: 100 }), + timelineOf(pr.number), + ]); + const state = prWaiting({ author, reviews, timeline }); + const hasLabel = (pr.labels || []).some((l) => lower(l.name) === LABEL.name); + const { action, note } = decidePr({ ...state, ...prWarnings(timeline), hasLabel, now }); + let reason = note || 'no open request for changes'; + if (state.reviewer) { + const asked = `changes asked by ${state.reviewer} on ${dateOnly(state.since)}`; + reason = state.waiting ? `${note} (${asked})` : `author pushed or replied since ${asked}`; + } + say('PR', pr.number, author, action, reason); + const issue_number = pr.number; + if (action === 'unlabel') { + await write(() => github.rest.issues.removeLabel({ owner, repo, issue_number, name: LABEL.name })); + } else if (action === 'nudge') { + await write(async () => { + await ensureLabel(); + await github.rest.issues.addLabels({ owner, repo, issue_number, labels: [LABEL.name] }); + await github.rest.issues.createComment({ owner, repo, issue_number, body: MESSAGES.nudge(author) }); + }); + } else if (action === 'label') { + await write(async () => { + await ensureLabel(); + await github.rest.issues.addLabels({ owner, repo, issue_number, labels: [LABEL.name] }); + }); + } else if (action === 'close') { + const issues = (await linkedIssues(github, owner, repo, pr.number)).filter((i) => + i.assignees.some((a) => lower(a) === lower(author)), + ); + if (issues.length) core.info(` linked issues ${author} would be unassigned from: ${issues.map((i) => `#${i.number}`).join(', ')}`); + await write(async () => { + for (const i of issues) { + await github.rest.issues.removeAssignees({ owner, repo, issue_number: i.number, assignees: [author] }); + } + await github.rest.issues.createComment({ owner, repo, issue_number, body: MESSAGES.close(author, issues.map((i) => i.number)) }); + await github.rest.pulls.update({ owner, repo, pull_number: pr.number, state: 'closed' }); + }); + } + } + + const assigned = await github.paginate(github.rest.issues.listForRepo, { owner, repo, state: 'open', assignee: '*', per_page: 100 }); + for (const issue of assigned) { + if (issue.pull_request) continue; + const people = (issue.assignees || []).filter((a) => !isMaintainer(a.login) && !isBot(a)); + if (!people.length) continue; + const timeline = await timelineOf(issue.number); + for (const person of people) { + const user = person.login; + const { action, note } = decideClaim({ ...claimFacts(timeline, user), now }); + say('issue', issue.number, user, action, note); + const issue_number = issue.number; + if (action === 'ping') { + await write(() => github.rest.issues.createComment({ owner, repo, issue_number, body: MESSAGES.claimPing(user) })); + } else if (action === 'unassign') { + await write(async () => { + await github.rest.issues.removeAssignees({ owner, repo, issue_number, assignees: [user] }); + await github.rest.issues.createComment({ owner, repo, issue_number, body: MESSAGES.claimDrop(user) }); + }); + } + } + } + + if (core.summary) { + await core.summary + .addHeading(dryRun ? 'Stale check (dry run, nothing changed)' : 'Stale check') + .addTable([ + [{ data: 'Kind', header: true }, { data: 'Number', header: true }, { data: 'Who', header: true }, + { data: 'Action', header: true }, { data: 'Why', header: true }], + ...rows, + ]) + .write(); + } + return rows; +} + +module.exports = run; +Object.assign(module.exports, { + MAINTAINERS, BOT, LABEL, LIMITS, MARK, MESSAGES, + latestDecisiveReview, lastAuthorActivity, botCommentAfter, prWarnings, prWaiting, decidePr, decideClaim, claimFacts, +}); diff --git a/.github/scripts/stale.test.js b/.github/scripts/stale.test.js new file mode 100644 index 00000000..f9f3c14f --- /dev/null +++ b/.github/scripts/stale.test.js @@ -0,0 +1,240 @@ +// Run with: node --test ".github/scripts/*.test.js" +const test = require('node:test'); +const assert = require('node:assert/strict'); +const run = require('./stale.js'); +const { BOT, MARK, LABEL, prWaiting, prWarnings, decidePr, decideClaim, claimFacts } = run; + +const DAY = 24 * 60 * 60 * 1000; +const T0 = Date.parse('2026-10-01T12:00:00Z'); +const at = (d) => new Date(T0 + d * DAY).toISOString(); +const user = (login) => ({ login, type: login.endsWith('[bot]') ? 'Bot' : 'User' }); +const review = (login, state, d) => ({ user: user(login), state, submitted_at: at(d) }); +const commit = (d) => ({ event: 'committed', committer: { date: at(d) }, author: { date: at(d) } }); +const comment = (login, d, body = 'hi') => ({ event: 'commented', actor: user(login), user: user(login), created_at: at(d), body }); + +test('waiting starts at the latest changes-requested review and ignores bot reviews after it', () => { + const reviews = [review('itsskofficial', 'CHANGES_REQUESTED', 1), review('coderabbitai[bot]', 'COMMENTED', 2)]; + const s = prWaiting({ author: 'maya', reviews, timeline: [commit(0), comment('maya', 0.5)] }); + assert.deepEqual(s, { waiting: true, since: Date.parse(at(1)), reviewer: 'itsskofficial' }); +}); + +test('a commit, a force push, a comment or a review reply from the author stops the wait', () => { + const reviews = [review('itsskofficial', 'CHANGES_REQUESTED', 1)]; + for (const event of [ + commit(2), + { event: 'head_ref_force_pushed', actor: user('maya'), created_at: at(2) }, + comment('maya', 2), + { event: 'reviewed', user: user('maya'), state: 'commented', submitted_at: at(2) }, + { event: 'line-commented', comments: [{ user: user('maya'), created_at: at(2) }] }, + ]) { + assert.equal(prWaiting({ author: 'maya', reviews, timeline: [commit(0), event] }).waiting, false, event.event); + } + // Someone else commenting does not count. + assert.equal(prWaiting({ author: 'maya', reviews, timeline: [commit(0), comment('ravi', 2)] }).waiting, true); +}); + +test('reopening a closed PR restarts it, so it is not closed again the next day', () => { + const reviews = [review('itsskofficial', 'CHANGES_REQUESTED', 0)]; + const timeline = [ + commit(-1), + comment(BOT, 7, `Hi @maya + +${MARK.nudge}`), + comment(BOT, 21, `Hi @maya + +${MARK.close}`), + { event: 'reopened', actor: user('maya'), created_at: at(25) }, + ]; + assert.equal(prWaiting({ author: 'maya', reviews, timeline }).waiting, false); +}); + +test('an approval or dismissal after the request means not waiting', () => { + for (const state of ['APPROVED', 'DISMISSED']) { + const reviews = [review('itsskofficial', 'CHANGES_REQUESTED', 1), review('Kabeer2004', state, 2)]; + assert.equal(prWaiting({ author: 'maya', reviews, timeline: [commit(0)] }).waiting, false); + } + assert.equal(prWaiting({ author: 'maya', reviews: [review('coderabbitai[bot]', 'COMMENTED', 1)], timeline: [] }).waiting, false); +}); + +test('PR timeline: wait, nudge at 7 days, close at 21 only after a week of warning', () => { + const since = T0; + const base = { waiting: true, since, nudged: false, warnedAt: 0, hasLabel: false }; + assert.equal(decidePr({ ...base, now: T0 + 6.9 * DAY }).action, 'wait'); + assert.match(decidePr({ ...base, now: T0 + 6.9 * DAY }).note, /nudge due 2026-10-08/); + assert.equal(decidePr({ ...base, now: T0 + 7 * DAY }).action, 'nudge'); + assert.equal(decidePr({ ...base, now: T0 + 30 * DAY }).action, 'nudge'); // never closed without a warning + const nudged = { ...base, nudged: true, warnedAt: T0 + 7 * DAY, hasLabel: true }; + assert.equal(decidePr({ ...nudged, now: T0 + 20 * DAY }).action, 'wait'); + assert.equal(decidePr({ ...nudged, now: T0 + 21 * DAY }).action, 'close'); + const lateNudge = { ...nudged, warnedAt: T0 + 20 * DAY }; + assert.equal(decidePr({ ...lateNudge, now: T0 + 22 * DAY }).action, 'wait'); + assert.equal(decidePr({ ...lateNudge, now: T0 + 27 * DAY }).action, 'close'); +}); + +test('only one nudge per PR, ever', () => { + const base = { waiting: true, since: T0, hasLabel: false, now: T0 + 8 * DAY }; + // Nudged in an earlier round (before the author pushed and got a new review): the label comes back, no comment. + assert.equal(decidePr({ ...base, nudged: true, warnedAt: T0 - 30 * DAY }).action, 'label'); + // Labeled already (by a maintainer or an earlier run) without the bot's comment: no comment either. + assert.equal(decidePr({ ...base, nudged: true, warnedAt: T0 + 7.5 * DAY, hasLabel: true }).action, 'wait'); + assert.equal(decidePr({ ...base, nudged: false, warnedAt: 0, hasLabel: true }).action, 'wait'); + // A label with no timeline event never leads straight to a close. + assert.equal(decidePr({ ...base, nudged: false, warnedAt: 0, hasLabel: true, now: T0 + 40 * DAY }).action, 'wait'); +}); + +test('warnings come from the nudge comment and the label, whenever they happened', () => { + const labeled = { event: 'labeled', actor: user('itsskofficial'), label: { name: LABEL.name }, created_at: at(3) }; + const other = { event: 'labeled', actor: user('itsskofficial'), label: { name: 'area: tasks' }, created_at: at(4) }; + assert.deepEqual(prWarnings([other]), { nudged: false, warnedAt: 0 }); + assert.deepEqual(prWarnings([labeled, other]), { nudged: true, warnedAt: Date.parse(at(3)) }); + const nudge = comment(BOT, 5, `Hi @maya ${MARK.nudge}`); + assert.deepEqual(prWarnings([labeled, nudge]), { nudged: true, warnedAt: Date.parse(at(5)) }); + // Someone quoting the marker does not count. + assert.deepEqual(prWarnings([comment('maya', 6, MARK.nudge)]), { nudged: false, warnedAt: 0 }); +}); + +test('the label comes off when the author is active again', () => { + const idle = { waiting: false, since: 0, nudged: true, warnedAt: 0, now: T0 }; + assert.equal(decidePr({ ...idle, hasLabel: true }).action, 'unlabel'); + assert.equal(decidePr({ ...idle, hasLabel: false }).action, 'none'); +}); + +test('claims: only /assign claims, ping at 14 days, unassign at 21 after a week of warning', () => { + const base = { viaCommand: true, hasOpenPr: false, lastActive: T0, pingedAt: 0 }; + assert.equal(decideClaim({ ...base, viaCommand: false, now: T0 + 40 * DAY }).action, 'none'); + assert.equal(decideClaim({ ...base, hasOpenPr: true, now: T0 + 40 * DAY }).action, 'none'); + assert.equal(decideClaim({ ...base, now: T0 + 13 * DAY }).action, 'wait'); + assert.equal(decideClaim({ ...base, now: T0 + 14 * DAY }).action, 'ping'); + const pinged = { ...base, pingedAt: T0 + 14 * DAY }; + assert.equal(decideClaim({ ...pinged, now: T0 + 20 * DAY }).action, 'wait'); + assert.equal(decideClaim({ ...pinged, now: T0 + 21 * DAY }).action, 'unassign'); +}); + +test('claim facts read the issue timeline', () => { + const assigned = (actor, d) => ({ event: 'assigned', actor: user(actor), assignee: user('maya'), created_at: at(d) }); + const viaBot = claimFacts([assigned(BOT, 0), comment('maya', 3)], 'maya'); + assert.equal(viaBot.viaCommand, true); + assert.equal(viaBot.lastActive, Date.parse(at(3))); + assert.equal(claimFacts([assigned('itsskofficial', 0)], 'maya').viaCommand, false); + const pr = { event: 'cross-referenced', source: { issue: { pull_request: {}, state: 'open', user: user('maya') } } }; + assert.equal(claimFacts([assigned(BOT, 0), pr], 'maya').hasOpenPr, true); + const closedPr = { event: 'cross-referenced', source: { issue: { pull_request: {}, state: 'closed', user: user('maya') } } }; + assert.equal(claimFacts([assigned(BOT, 0), closedPr], 'maya').hasOpenPr, false); + const ping = comment(BOT, 15, `Hi @maya, checking in\n\n${MARK.claimPing}`); + assert.equal(claimFacts([assigned(BOT, 0), ping], 'maya').pingedAt, Date.parse(at(15))); + // A ping from before her last comment no longer counts. + assert.equal(claimFacts([assigned(BOT, 0), ping, comment('maya', 16)], 'maya').pingedAt, 0); +}); + +// A fake GitHub with one PR per situation and one claimed issue. +function fakeGithub() { + const writes = []; + const pulls = [ + { number: 1, user: user('maya'), draft: false, labels: [] }, // waiting 8 days: nudge + { number: 2, user: user('itsskofficial'), draft: false, labels: [] }, // maintainer: skipped + { number: 3, user: user('ravi'), draft: true, labels: [] }, // draft: skipped + { number: 4, user: user('ravi'), draft: false, labels: [{ name: LABEL.name }] }, // pushed since: unlabel + { number: 5, user: user('dependabot[bot]'), draft: false, labels: [] }, // bot: skipped + { number: 6, user: user('lena'), draft: false, labels: [{ name: LABEL.name }] }, // nudged long ago: close + { number: 7, user: user('kai'), draft: false, labels: [{ name: LABEL.name }] }, // labeled by hand: no nudge + { number: 8, user: user('noor'), draft: false, labels: [] }, // nudged in an earlier round: label only + ]; + const reviews = { + 1: [review('itsskofficial', 'CHANGES_REQUESTED', 0)], + 2: [review('Kabeer2004', 'CHANGES_REQUESTED', 0)], + 3: [review('itsskofficial', 'CHANGES_REQUESTED', 0)], + 4: [review('itsskofficial', 'CHANGES_REQUESTED', 0)], + 5: [review('itsskofficial', 'CHANGES_REQUESTED', 0)], + 6: [review('itsskofficial', 'CHANGES_REQUESTED', -14)], + 7: [review('itsskofficial', 'CHANGES_REQUESTED', 0)], + 8: [review('itsskofficial', 'CHANGES_REQUESTED', -40), review('itsskofficial', 'CHANGES_REQUESTED', 0)], + }; + const timelines = { + 1: [commit(-1)], + 4: [commit(-1), commit(2)], + 6: [commit(-15), comment(BOT, -7, `Hi @lena\n\n${MARK.nudge}`)], + 7: [commit(-1), { event: 'labeled', actor: user('itsskofficial'), label: { name: LABEL.name }, created_at: at(1) }], + 8: [commit(-41), comment(BOT, -33, `Hi @noor ${MARK.nudge}`), commit(-20)], + 10: [{ event: 'assigned', actor: user(BOT), assignee: user('omar'), created_at: at(-7) }], + 11: [{ event: 'assigned', actor: user('itsskofficial'), assignee: user('omar'), created_at: at(-30) }], + }; + const issues = [ + { number: 10, assignees: [user('omar')] }, // claimed with /assign 15 days ago: ping + { number: 11, assignees: [user('omar')] }, // assigned by a maintainer: left alone + { number: 12, assignees: [user('Kabeer2004')] }, // maintainer: skipped + { number: 6, assignees: [user('lena')], pull_request: {} }, // a PR in the issues list: skipped + ]; + const record = (name) => async (args) => writes.push([name, args]); + const github = { + paginate: async (fn, args) => fn(args), + graphql: async () => ({ repository: { pullRequest: { closingIssuesReferences: { nodes: [ + { number: 20, assignees: { nodes: [{ login: 'lena' }] } }, + { number: 21, assignees: { nodes: [{ login: 'someone-else' }] } }, + ] } } } }), + rest: { + pulls: { + list: async () => pulls, + listReviews: async ({ pull_number }) => reviews[pull_number] || [], + update: record('pulls.update'), + }, + issues: { + listEventsForTimeline: async ({ issue_number }) => timelines[issue_number] || [], + listForRepo: async () => issues, + createLabel: record('createLabel'), + addLabels: record('addLabels'), + removeLabel: record('removeLabel'), + removeAssignees: record('removeAssignees'), + createComment: record('createComment'), + }, + }, + }; + return { github, writes }; +} + +const context = { repo: { owner: 'existence-master', repo: 'Sentient' } }; +const quietCore = { info: () => {} }; +const NOW = T0 + 8 * DAY; + +test('a dry run decides everything and changes nothing', async () => { + const { github, writes } = fakeGithub(); + const lines = []; + const rows = await run({ github, context, core: { info: (l) => lines.push(l) }, dryRun: true, now: NOW }); + assert.deepEqual(writes, []); + const actions = Object.fromEntries(rows.map((r) => [`${r[0]} ${r[1]}`, r[3]])); + assert.deepEqual(actions, { + 'PR #1': 'nudge', + 'PR #3': 'none', + 'PR #4': 'unlabel', + 'PR #6': 'close', + 'PR #7': 'wait', + 'PR #8': 'label', + 'issue #10': 'ping', + 'issue #11': 'none', + }); + assert.ok(lines.some((l) => l.startsWith('[dry run] would nudge PR #1 (maya)'))); +}); + +test('a live run labels, nudges once, closes, unassigns and pings', async () => { + const { github, writes } = fakeGithub(); + await run({ github, context, core: quietCore, dryRun: false, now: T0 + 14 * DAY }); + const names = writes.map((w) => `${w[0]} ${w[1].issue_number || w[1].pull_number}`); + assert.deepEqual(names, [ + 'createLabel undefined', + 'addLabels 1', + 'createComment 1', + 'removeLabel 4', + 'removeAssignees 20', + 'createComment 6', + 'pulls.update 6', + 'addLabels 8', + 'createComment 10', + ]); + const nudge = writes.find((w) => w[0] === 'createComment' && w[1].issue_number === 1)[1].body; + assert.match(nudge, /^Hi @maya, just a friendly check-in/); + assert.ok(nudge.includes(MARK.nudge)); + const close = writes.find((w) => w[0] === 'createComment' && w[1].issue_number === 6)[1].body; + assert.match(close, /You can reopen it any time/); + assert.match(close, /unassigned you from #20/); + assert.deepEqual(writes.find((w) => w[0] === 'removeAssignees')[1].assignees, ['lena']); + assert.match(writes.at(-1)[1].body, /^Hi @omar, checking in on this one/); +}); diff --git a/.github/workflows/assign.yaml b/.github/workflows/assign.yaml new file mode 100644 index 00000000..41be4556 --- /dev/null +++ b/.github/workflows/assign.yaml @@ -0,0 +1,34 @@ +# "/assign" and "/unassign" comments on issues. The logic is in .github/scripts/assign.js (tested by +# .github/scripts/assign.test.js). It reads the comment only to compare it with those two words and never runs it. +name: Claim issues + +on: + issue_comment: + types: [created] + +permissions: + issues: write + +jobs: + claim: + # Issues only (a PR comment also fires issue_comment), and only comments that are exactly a command (allowing a + # trailing space or line break), so other comments never queue here and can't push a waiting command out. + if: >- + github.event.issue.pull_request == null && + contains(fromJSON('["/assign", "/assign ", "/assign\n", "/assign\r\n", "/unassign", "/unassign ", "/unassign\n", "/unassign\r\n"]'), github.event.comment.body) + # Per job, so only runs that pass the condition above share the queue. + concurrency: + group: claim-${{ github.event.issue.number }} + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # issue_comment always runs on the default branch, so this is our own script, never a contributor's. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + sparse-checkout: .github/scripts + persist-credentials: false + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const run = require('./.github/scripts/assign.js'); + await run({ github, context, core }); diff --git a/.github/workflows/bot-scripts.yaml b/.github/workflows/bot-scripts.yaml new file mode 100644 index 00000000..814c3ed8 --- /dev/null +++ b/.github/workflows/bot-scripts.yaml @@ -0,0 +1,26 @@ +# Unit tests for the repository bots in .github/scripts (plain Node, no packages, no network). +name: Bot scripts + +on: + pull_request: + paths: + - .github/scripts/** + - .github/workflows/assign.yaml + - .github/workflows/stale.yaml + - .github/workflows/bot-scripts.yaml + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-node@v6 + with: + node-version: 22 + - run: node --test ".github/scripts/*.test.js" diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml new file mode 100644 index 00000000..4ba92a3f --- /dev/null +++ b/.github/workflows/stale.yaml @@ -0,0 +1,50 @@ +# Daily check of contributor pull requests waiting on their author and of quiet issue claims. The rules are in +# .github/scripts/stale.js and CONTRIBUTING.md ("If things go quiet"). +# +# Scheduled runs act for real. A manual run is a dry run (it only logs what it would do) unless dry_run is unticked. +# To pause the scheduled runs without a code change, set the repository variable STALE_BOT_DRY_RUN to "true" +# (Settings > Secrets and variables > Actions > Variables). A pull request that changes the bot gets a dry run +# against the real repository as a preview. Each PR gets at most one nudge comment, ever (see stale.js). +name: Stale pull requests and claims + +on: + schedule: + - cron: "23 6 * * *" + workflow_dispatch: + inputs: + dry_run: + description: Only log what would happen, change nothing + type: boolean + default: true + pull_request: + paths: + - .github/workflows/stale.yaml + - .github/scripts/stale.js + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: stale-bot-${{ github.event_name }} + cancel-in-progress: false + +jobs: + stale: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + sparse-checkout: .github/scripts + persist-credentials: false + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + LIVE: >- + ${{ (github.event_name == 'workflow_dispatch' && !inputs.dry_run) || + (github.event_name == 'schedule' && vars.STALE_BOT_DRY_RUN != 'true') }} + with: + script: | + const run = require('./.github/scripts/stale.js'); + await run({ github, context, core, dryRun: process.env.LIVE !== 'true' }); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e4cb1921..6c6deaf4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,9 +34,10 @@ engine on its own and how to capture screenshots, is in [docs/DEVELOPING.md](doc We use **GitHub flow**: -1. **Find or open an issue.** Look for [`good first issue`](../../labels/good%20first%20issue) and - [`help wanted`](../../labels/help%20wanted). For anything bigger than a small fix, comment on the issue first - so we can agree on the approach and avoid duplicate work. Small fixes and docs need no issue. +1. **Find or open an issue, then claim it.** Look for [`good first issue`](../../labels/good%20first%20issue) and + [`help wanted`](../../labels/help%20wanted). Comment `/assign` on a free issue and a bot assigns it to you, so + nobody else starts the same work. For anything bigger than a small fix, also say how you plan to do it so we + can agree on the approach. Small fixes and docs need no issue. 2. **Fork, then branch from `main`.** Name the branch after what it does, for example `fix/telegram-long-messages` or `feat/outlook-integration`. 3. **Keep the pull request small and focused.** One change per PR is much faster to review. @@ -47,6 +48,32 @@ We use **GitHub flow**: There are no long-lived `development` or `staging` branches. `main` is always the latest working version, and releases are tags on `main`. +### Claiming an issue + +Comment on the issue with just one line: + +- `/assign` assigns the issue to you, if nobody has it yet. If someone does, the bot tells you who, so you can + pick another issue or offer to team up. +- `/unassign` hands it back, if it is yours. No explanation needed, and thanks for saying so. + +Try to open a pull request (a draft is fine) within a week. If you need longer, just comment on the issue. + +### If things go quiet + +Life happens, and that's fine. To keep issues moving for everyone, a bot checks once a day: + +- **A claimed issue with no pull request and no comment from you for 14 days** gets a friendly ping. After 21 + days (and at least a week after the ping), you are unassigned so someone else can pick it up. You can + `/assign` it again any time while it is free. +- **A pull request where we asked for changes** and nothing has happened for 7 days gets the + `waiting on author` label and one gentle reminder (only ever one per pull request). Pushing a commit or + replying removes the label. After 21 days (and at least a week after the reminder) we close it with a + thank-you, and unassign you from its issue. You can reopen it whenever you have time, or comment and we will + reopen it for you. + +Draft pull requests are never touched, and the clock only runs while we are waiting on you, never while you +are waiting on us. + ### PR titles Use [Conventional Commits](https://www.conventionalcommits.org/) with an area, because the title becomes the