From 1e9f3807a1d9e8ce1f7cde78866f5ef4e8263c6b Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Fri, 5 Jun 2026 14:39:02 -0500 Subject: [PATCH] Add author mention to stale issue bots (#46787) **Related issue:** Resolves #46790 Example live run off this branch: https://github.com/fleetdm/fleet/actions/runs/27005120262 Example stale issue with comment: https://github.com/fleetdm/fleet/issues/18421 - Added `@author` mention when marking issue as stale - Refactored so that both Fleetie and eng-initiated stale issue bots use the same core JS code - Run Fleetie-initiated workflow on a schedule ## Summary by CodeRabbit * **New Features** * Automated stale-issue workflows for engineering-initiated and Fleetie issues with configurable dry-run, max-operations, manual triggers, and scheduled runs. * **Tests** * Added shared test helpers and expanded, tightened test suites covering staleness, closing, unstale, and error/boundary behaviors. * **Refactor** * Introduced a shared stale-issue engine used by thin, author/label-based wrappers for consistent behavior and messaging. * **Chores** * Updated workflow triggers, permissions, and CI test matrix to include the new core and wrappers. --- .github/scripts/stale-eng-issues.js | 66 +++ .github/scripts/stale-eng-issues.test.js | 137 ++++++ .github/scripts/stale-fleetie-issues.js | 368 ++-------------- .github/scripts/stale-fleetie-issues.test.js | 157 +------ .github/scripts/stale-issues-core.js | 403 ++++++++++++++++++ .github/scripts/stale-test-helpers.js | 126 ++++++ .../close-stale-eng-initiated-issues.yml | 47 +- .../close-stale-fleetie-initiated-issues.yml | 8 +- .../test-stale-fleetie-issue-scripts.yml | 23 +- 9 files changed, 845 insertions(+), 490 deletions(-) create mode 100644 .github/scripts/stale-eng-issues.js create mode 100644 .github/scripts/stale-eng-issues.test.js create mode 100644 .github/scripts/stale-issues-core.js create mode 100644 .github/scripts/stale-test-helpers.js diff --git a/.github/scripts/stale-eng-issues.js b/.github/scripts/stale-eng-issues.js new file mode 100644 index 0000000000..8b918a60eb --- /dev/null +++ b/.github/scripts/stale-eng-issues.js @@ -0,0 +1,66 @@ +// Marks open engineering-initiated issues (label `~engineering-initiated`) as stale after 1y of +// inactivity, and closes them after 14 more days. Invoked by `actions/github-script` from +// `.github/workflows/close-stale-eng-initiated-issues.yml`. +// +// This is a thin wrapper over `stale-issues-core.js`: it provides label-based eligibility and the +// eng-initiated thresholds and wording. The scanning, labeling, closing, and un-staling logic all +// live in the core. The sibling `stale-fleetie-issues.js` wraps the same core with author-based +// eligibility. +// +// Why this replaced `actions/stale`: that action posts a single static stale comment with no way to +// template the issue author, so it could not @-mention the author. The core lets us build the +// comment per issue. Behavior otherwise matches the previous `actions/stale` config (365d to stale, +// 14d to close, remove-stale-when-updated). +// +// Inputs (env): +// DRY_RUN 'true' to log candidates without writing (read by the core). +// MAX_OPERATIONS Cap on API write operations per run. Default 400. `0` disables writes / dry-runs (read by the core). +// +// Exports: `async function run({ github, context, core })`. Returns a summary object for tests. + +"use strict"; + +const core_run = require("./stale-issues-core.js"); + +const STALE_DAYS = 365; +const CLOSE_DAYS = 14; +const STALE_LABEL = "stale"; +const ELIGIBLE_LABEL = "~engineering-initiated"; + +const staleMessage = (author) => + `@${author} this issue is stale because it has been open for 365 days with no activity. ` + + "Please update the issue if it is still relevant; otherwise it will be closed in 14 days."; +const CLOSE_MSG = + "This issue was closed because it has been inactive for 14 days since being marked as stale."; + +async function run({ github, context, core }) { + const result = await core_run({ + github, + context, + core, + config: { + title: "Eng-initiated stale-issue closer", + staleDays: STALE_DAYS, + closeDays: CLOSE_DAYS, + staleLabel: STALE_LABEL, + isEligible: (issue) => + (issue.labels || []).some( + (l) => + (typeof l === "string" ? l : (l && l.name) || "").toLowerCase() === + ELIGIBLE_LABEL.toLowerCase() + ), + staleMessage, + closeMessage: () => CLOSE_MSG, + ineligibleSummaryLabel: `Skipped (no ${ELIGIBLE_LABEL} label)`, + }, + }); + return result; +} + +module.exports = run; +// Exported for test boundary assertions so a future policy change surfaces in the boundary tests +// instead of silently passing on a hardcoded old value. +module.exports.STALE_DAYS = STALE_DAYS; +module.exports.CLOSE_DAYS = CLOSE_DAYS; +module.exports.ELIGIBLE_LABEL = ELIGIBLE_LABEL; +module.exports.SELF_ACTIVITY_EPSILON_MS = core_run.SELF_ACTIVITY_EPSILON_MS; diff --git a/.github/scripts/stale-eng-issues.test.js b/.github/scripts/stale-eng-issues.test.js new file mode 100644 index 0000000000..cb7807c342 --- /dev/null +++ b/.github/scripts/stale-eng-issues.test.js @@ -0,0 +1,137 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); + +const run = require('./stale-eng-issues.js'); +// Pull constants from the script so the boundary tests keep exercising the real boundary if a +// future policy change (e.g. STALE_DAYS 365 -> 730) moves it. +const { STALE_DAYS, CLOSE_DAYS, ELIGIBLE_LABEL } = run; +const { DAY_MS, daysAgoIso, makeStaleLabelEvent, makeContext, makeCore, makeGithub } = require('./stale-test-helpers.js'); + +// Eng-initiated issues qualify by carrying the `~engineering-initiated` label, so the default +// issue includes it. Tests that exercise the ineligible path drop it explicitly. +function makeIssue(overrides = {}) { + return { + number: 1, + html_url: 'https://github.com/o/r/issues/1', + user: { login: 'getvictor' }, + labels: [{ name: ELIGIBLE_LABEL }], + updated_at: daysAgoIso(STALE_DAYS + 70), + state: 'open', + pull_request: undefined, + ...overrides, + }; +} + +async function runWith({ issues, issuesByPage, dryRun = false, maxOps = 1000, eventsByIssue = {}, failOn = {} } = {}) { + process.env.DRY_RUN = dryRun ? 'true' : 'false'; + process.env.MAX_OPERATIONS = String(maxOps); + const pages = issuesByPage != null ? issuesByPage : [issues || []]; + const github = makeGithub({ issuesByPage: pages, eventsByIssue, failOn }); + const context = makeContext(); + const core = makeCore(); + const result = await run({ github, context, core }); + return { github, core, result }; +} + +// Core engine behavior (caps, error paths, pagination, un-stale epsilon, etc.) is covered through +// the Fleetie wrapper in stale-fleetie-issues.test.js. These tests pin what the eng wrapper +// configures differently: label-based eligibility, no exempt labels, the 1-year threshold, and the +// eng-specific message wording. + +test('marks an eng-initiated issue idle >1y as stale and @-mentions the author', async () => { + const { github, result } = await runWith({ + issues: [makeIssue({ number: 1, user: { login: 'getvictor' }, updated_at: daysAgoIso(STALE_DAYS + 70) })], + }); + assert.strictEqual(github._captured.createCommentCalls.length, 1); + const staleComment = github._captured.createCommentCalls[0].body; + assert.match(staleComment, /^@getvictor /, 'stale comment @-mentions the author'); + assert.ok(staleComment.includes('365 days'), 'stale comment uses the eng wording, not the Fleetie template'); + assert.strictEqual(github._captured.addLabelsCalls.length, 1); + assert.deepStrictEqual(github._captured.addLabelsCalls[0].labels, ['stale']); + assert.strictEqual(result.staled.length, 1); +}); + +test('skips issues without the ~engineering-initiated label', async () => { + const { github, result } = await runWith({ + issues: [makeIssue({ labels: [], updated_at: daysAgoIso(STALE_DAYS + 70) })], + }); + assert.strictEqual(github._captured.createCommentCalls.length, 0); + assert.strictEqual(result.skippedIneligible, 1); + assert.strictEqual(result.staled.length, 0); +}); + +test('eligible-label match is case-insensitive', async () => { + const { result } = await runWith({ + issues: [makeIssue({ labels: [{ name: ELIGIBLE_LABEL.toUpperCase() }], updated_at: daysAgoIso(STALE_DAYS + 70) })], + }); + assert.strictEqual(result.staled.length, 1); +}); + +test('does not exempt bug-labeled eng issues (unlike the Fleetie closer)', async () => { + const { result } = await runWith({ + issues: [makeIssue({ labels: [{ name: ELIGIBLE_LABEL }, { name: 'bug' }], updated_at: daysAgoIso(STALE_DAYS + 70) })], + }); + assert.strictEqual(result.skippedExempt, 0); + assert.strictEqual(result.staled.length, 1); +}); + +test('closes a stale-labeled eng issue idle >14d with no activity after labeling', async () => { + const labeledAt = Date.now() - (CLOSE_DAYS + 6) * DAY_MS; + const issue = makeIssue({ + number: 2, + labels: [{ name: ELIGIBLE_LABEL }, { name: 'stale' }], + updated_at: new Date(labeledAt).toISOString(), + }); + const { github, result } = await runWith({ + issues: [issue], + eventsByIssue: { 2: [makeStaleLabelEvent({ at: labeledAt })] }, + }); + assert.strictEqual(github._captured.createCommentCalls.length, 1, 'close comment posted'); + assert.strictEqual(github._captured.updateCalls.length, 1); + assert.strictEqual(github._captured.updateCalls[0].state, 'closed'); + assert.strictEqual(github._captured.updateCalls[0].state_reason, 'not_planned'); + assert.strictEqual(result.closed.length, 1); +}); + +test('un-stales an eng issue with activity after labeling', async () => { + const issue = makeIssue({ + number: 3, + labels: [{ name: ELIGIBLE_LABEL }, { name: 'stale' }], + updated_at: daysAgoIso(5), + }); + const { github, result } = await runWith({ + issues: [issue], + eventsByIssue: { 3: [makeStaleLabelEvent({ daysAgo: 20 })] }, + }); + assert.strictEqual(github._captured.removeLabelCalls.length, 1); + assert.strictEqual(github._captured.removeLabelCalls[0].name, 'stale'); + assert.strictEqual(github._captured.createCommentCalls.length, 0, 'un-stale writes no comment'); + assert.strictEqual(github._captured.updateCalls.length, 0, 'un-stale does not close'); + assert.strictEqual(result.unstaled.length, 1); +}); + +test('skips eng issue idle between 14 and 365 days as "not stale yet"', async () => { + const { result } = await runWith({ + issues: [makeIssue({ updated_at: daysAgoIso(100) })], + }); + assert.strictEqual(result.skippedNotStaleYet, 1); + assert.strictEqual(result.staled.length, 0); +}); + +test('stale-phase boundary: issue just past STALE_DAYS is staled', async () => { + const { result } = await runWith({ + issues: [makeIssue({ number: 200, updated_at: daysAgoIso(STALE_DAYS + 0.01) })], + }); + assert.strictEqual(result.staled.length, 1); + assert.strictEqual(result.skippedNotStaleYet, 0); +}); + +test('stale-phase boundary: issue just under STALE_DAYS is skipped as not stale yet', async () => { + const { result } = await runWith({ + issues: [makeIssue({ number: 201, updated_at: daysAgoIso(STALE_DAYS - 0.1) })], + }); + assert.strictEqual(result.staled.length, 0); + assert.strictEqual(result.skippedNotStaleYet, 1); +}); diff --git a/.github/scripts/stale-fleetie-issues.js b/.github/scripts/stale-fleetie-issues.js index d9a26fc78e..1108314527 100644 --- a/.github/scripts/stale-fleetie-issues.js +++ b/.github/scripts/stale-fleetie-issues.js @@ -1,36 +1,35 @@ -// Marks open issues authored by current/former Fleeties as stale after 2y of inactivity, and closes them -// after 14 more days. Exempts `bug`, `:product`, and `customer-*` labels. Invoked by +// Marks open issues authored by current/former Fleeties as stale after 2y of inactivity, and closes +// them after 14 more days. Exempts `bug`, `:product`, and `customer-*` labels. Invoked by // `actions/github-script` from `.github/workflows/close-stale-fleetie-initiated-issues.yml`. // -// If a stale-labeled issue receives activity (e.g. a comment) after being labeled, the close phase -// removes the stale label instead of closing, mirroring `actions/stale`'s `remove-stale-when-updated` -// behavior. Detected by comparing `updated_at` against the most recent `labeled` event for `stale`. +// This is a thin wrapper over `stale-issues-core.js`: it provides the author-based eligibility check +// (issue author is a current/former Fleetie) and the Fleetie-specific thresholds and wording. The +// scanning, labeling, closing, and un-staling logic all live in the core. The sibling +// `stale-eng-issues.js` wraps the same core with label-based eligibility. // // Inputs (env): // FLEETIE_HANDLES_FILE Path to newline-delimited lowercased GitHub usernames (built by `build-fleetie-handles.js`). -// DRY_RUN 'true' to log candidates without writing. -// MAX_OPERATIONS Cap on API write operations per run. Default 400. `0` disables writes. +// DRY_RUN 'true' to log candidates without writing (read by the core). +// MAX_OPERATIONS Cap on API write operations per run. Default 400. `0` disables writes / dry-runs (read by the core). // // Exports: `async function run({ github, context, core })`. Returns a summary object for tests. "use strict"; const fs = require("node:fs"); +const core_run = require("./stale-issues-core.js"); const STALE_DAYS = 730; const CLOSE_DAYS = 14; const STALE_LABEL = "stale"; -const STALE_MSG = - "This issue is stale because it was opened by a current or former Fleetie and has had " + + +const staleMessage = (author) => + `@${author} this issue is stale because it was opened by a current or former Fleetie and has had ` + "no activity for 2 years. Please update the issue if it is still relevant; otherwise it " + "will be closed in 14 days."; const CLOSE_MSG = "This issue was closed because it received no further activity for 14 days after being marked stale. " + "Any comment would have removed the stale label and prevented closure."; -// Tolerance for our own label+comment landing milliseconds apart. Distinguishes the bot's own -// activity bump from genuine user activity after labeling. -const SELF_ACTIVITY_EPSILON_MS = 60 * 1000; -const MS_PER_DAY = 1000 * 60 * 60 * 24; const isExempt = (name) => { const lower = (name || "").toLowerCase(); @@ -39,11 +38,6 @@ const isExempt = (name) => { ); }; -const parseMaxOps = (raw) => { - const parsed = Number.parseInt(raw ?? "", 10); - return Number.isInteger(parsed) && parsed >= 0 ? parsed : 400; -}; - async function run({ github, context, core }) { const handles = new Set( fs @@ -52,326 +46,34 @@ async function run({ github, context, core }) { .map((s) => s.trim().toLowerCase()) .filter(Boolean) ); + core.info(`Loaded ${handles.size} Fleetie handles.`); - const dryRun = String(process.env.DRY_RUN).toLowerCase() === "true"; - const maxOps = parseMaxOps(process.env.MAX_OPERATIONS); - - // All time math is in UTC-equivalent epoch milliseconds: Date.now() is UTC, and the GitHub - // REST API returns ISO 8601 strings with a `Z` suffix, so timezone and DST cannot affect the - // result. `daysSince` is a 24-hour-day count, not a calendar-day count. - const now = Date.now(); - const daysSince = (iso) => (now - new Date(iso).getTime()) / MS_PER_DAY; - - core.info( - `Loaded ${handles.size} Fleetie handles. dry_run=${dryRun}, max_operations=${maxOps}` - ); - - // Collect candidates by scanning all open issues. Two groups qualify: - // 1. Idle >= CLOSE_DAYS — feeds the stale and close phases. - // 2. Currently `stale`-labeled regardless of idle time — feeds the un-stale phase so a user - // comment on a stale issue removes the label on the next run, not 14 days later. - const candidates = []; - const iterator = github.paginate.iterator(github.rest.issues.listForRepo, { - owner: context.repo.owner, - repo: context.repo.repo, - state: "open", - sort: "updated", - direction: "asc", - per_page: 100, + const result = await core_run({ + github, + context, + core, + config: { + title: "Fleetie stale-issue closer", + staleDays: STALE_DAYS, + closeDays: CLOSE_DAYS, + staleLabel: STALE_LABEL, + isEligible: (issue) => + handles.has(((issue.user && issue.user.login) || "").toLowerCase()), + isExempt, + staleMessage, + closeMessage: () => CLOSE_MSG, + ineligibleSummaryLabel: "Skipped (non-Fleetie author)", + summaryLines: [`Fleetie handles loaded: **${handles.size}**`], + }, }); - for await (const { data } of iterator) { - for (const issue of data) { - if (issue.pull_request) continue; - const idleDays = daysSince(issue.updated_at); - const hasStaleLabel = (issue.labels || []).some( - (l) => - (typeof l === "string" ? l : (l && l.name) || "").toLowerCase() === - STALE_LABEL - ); - if (idleDays >= CLOSE_DAYS || hasStaleLabel) { - candidates.push(issue); - } - } - } - core.info( - `Collected ${candidates.length} open candidate issues (idle >= ${CLOSE_DAYS}d or stale-labeled)` - ); - const staled = []; - const closed = []; - const unstaled = []; - const errored = []; - let skippedNonFleetie = 0; - let skippedExempt = 0; - let skippedNotStaleYet = 0; - let skippedNotReadyToClose = 0; - let writes = 0; - let hitCap = false; - - const owner = context.repo.owner; - const repo = context.repo.repo; - - for (const issue of candidates) { - // Conservative pre-check: each iteration may do up to 2 writes (stale or close phases). The - // unstale path is 1 write, so this estimate is safe but slightly over-conservative on that path. - if (!dryRun && writes + 2 > maxOps) { - hitCap = true; - core.warning(`Reached max_operations=${maxOps}; stopping.`); - break; - } - - const author = (issue.user && issue.user.login - ? issue.user.login - : "" - ).toLowerCase(); - if (!handles.has(author)) { - skippedNonFleetie++; - continue; - } - - const labelNames = (issue.labels || []).map( - (l) => (typeof l === "string" ? l : l.name) || "" - ); - if (labelNames.some(isExempt)) { - skippedExempt++; - continue; - } - - const idleDays = daysSince(issue.updated_at); - const alreadyStale = labelNames.some( - (n) => n.toLowerCase() === STALE_LABEL - ); - - if (alreadyStale) { - // Determine whether there's been activity after the stale label was applied. If so, remove the - // stale label (mirroring actions/stale's remove-stale-when-updated) and skip the close. - let events; - try { - events = await github.paginate(github.rest.issues.listEvents, { - owner, - repo, - issue_number: issue.number, - per_page: 100, - }); - } catch (err) { - core.warning( - `listEvents failed for #${issue.number}: ${err.message}; skipping` - ); - errored.push({ - number: issue.number, - phase: "check-activity", - message: err.message, - }); - continue; - } - - let lastStaleLabelEvent = null; - for (let i = events.length - 1; i >= 0; i--) { - const e = events[i]; - if (e.event === "labeled" && e.label && e.label.name === STALE_LABEL) { - lastStaleLabelEvent = e; - break; - } - } - - if (!lastStaleLabelEvent) { - core.warning( - `#${issue.number}: has '${STALE_LABEL}' label but no labeling event in history; skipping` - ); - continue; - } - - const labeledAt = new Date(lastStaleLabelEvent.created_at).getTime(); - const updatedAt = new Date(issue.updated_at).getTime(); - const activityAfterLabel = - updatedAt > labeledAt + SELF_ACTIVITY_EPSILON_MS; - - if (activityAfterLabel) { - const entry = { - number: issue.number, - url: issue.html_url, - author, - idleDays, - }; - core.info( - `unstale: #${issue.number} by @${author} (activity after label)` - ); - if (dryRun) { - unstaled.push(entry); - } else { - try { - await github.rest.issues.removeLabel({ - owner, - repo, - issue_number: issue.number, - name: STALE_LABEL, - }); - writes += 1; - unstaled.push(entry); - } catch (err) { - if (err && err.status === 404) { - // Label already gone (idempotent success). - unstaled.push(entry); - } else { - core.warning( - `unstale failed for #${issue.number}: ${err.message}` - ); - errored.push({ - number: issue.number, - phase: "unstale", - message: err.message, - }); - } - } - } - continue; - } - - // Close phase: stale label present, no activity since labeling, AND >= CLOSE_DAYS idle. - // The idle gate matters because candidate collection accepts stale-labeled issues regardless - // of idle time (so the un-stale path runs promptly on activity); without this check, a - // freshly-staled issue with no activity would be closed on the very next run. - if (idleDays < CLOSE_DAYS) { - skippedNotReadyToClose++; - continue; - } - const entry = { - number: issue.number, - url: issue.html_url, - author, - idleDays, - }; - core.info( - `close: #${issue.number} by @${author}, idle ${idleDays.toFixed(1)}d` - ); - if (dryRun) { - closed.push(entry); - } else { - try { - await github.rest.issues.createComment({ - owner, - repo, - issue_number: issue.number, - body: CLOSE_MSG, - }); - await github.rest.issues.update({ - owner, - repo, - issue_number: issue.number, - state: "closed", - state_reason: "not_planned", - }); - writes += 2; - closed.push(entry); - } catch (err) { - core.warning(`close failed for #${issue.number}: ${err.message}`); - errored.push({ - number: issue.number, - phase: "close", - message: err.message, - }); - } - } - } else if (idleDays >= STALE_DAYS) { - const entry = { - number: issue.number, - url: issue.html_url, - author, - idleDays, - }; - core.info( - `stale: #${issue.number} by @${author}, idle ${idleDays.toFixed(1)}d` - ); - if (dryRun) { - staled.push(entry); - } else { - try { - await github.rest.issues.createComment({ - owner, - repo, - issue_number: issue.number, - body: STALE_MSG, - }); - await github.rest.issues.addLabels({ - owner, - repo, - issue_number: issue.number, - labels: [STALE_LABEL], - }); - writes += 2; - staled.push(entry); - } catch (err) { - core.warning(`stale failed for #${issue.number}: ${err.message}`); - errored.push({ - number: issue.number, - phase: "stale", - message: err.message, - }); - } - } - } else { - skippedNotStaleYet++; - } - } - - // Each entry is an HTML
  • so it renders correctly inside