Batch wontfix lookups into a single set check

Author: kzhou314

.github/actions/file/src/index.ts

@@ -10,7 +10,7 @@ import {closeIssue} from './closeIssue.js'
 import {isNewFiling} from './isNewFiling.js'
 import {isRepeatedFiling} from './isRepeatedFiling.js'
 import {isResolvedFiling} from './isResolvedFiling.js'
-import {isWontfixIssue, WONTFIX_LABEL} from './isWontfixIssue.js'
+import {getWontfixIssueNumbers, shouldReopenIssue, WONTFIX_LABEL} from './shouldReopenIssue.js'
 import {openIssue} from './openIssue.js'
 import {reopenIssue} from './reopenIssue.js'
 import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js'
@@ -61,6 +61,17 @@ export default async function () {
   })
   const filings = updateFilingsWithNewFindings(cachedFilings, findings)
 
+  // Fetch closed wontfix issues once up front; a failed fetch reopens as usual
+  let wontfixIssueNumbers = new Set<number>()
+  if (!dryRun) {
+    try {
+      const [owner, repository] = repoWithOwner.split('/')
+      wontfixIssueNumbers = await getWontfixIssueNumbers(octokit, {owner, repository})
+    } catch (error) {
+      core.warning(`Could not fetch '${WONTFIX_LABEL}' issues; proceeding with reopen as usual: ${error}`)
+    }
+  }
+
   // Track new issues for grouping
   const newIssuesByProblemShort: Record<string, FindingGroupIssue[]> = {}
   const trackingIssueUrls: Record<string, string> = {}
@@ -108,14 +119,7 @@ export default async function () {
           }
         } else if (isRepeatedFiling(filing)) {
           const issue = new Issue(filing.issue)
-          let isWontfix = false
-          try {
-            isWontfix = await isWontfixIssue(octokit, issue)
-          } catch (error) {
-            // A failed label check shouldn't abort the run, so reopen as usual
-            core.warning(`Could not check labels for ${filing.issue.url}; proceeding with reopen: ${error}`)
-          }
-          if (isWontfix) {
+          if (!shouldReopenIssue(issue, wontfixIssueNumbers)) {
             // The developer intentionally closed this issue and labeled it 'wontfix', so leave it closed
             core.info(`Skipping reopen of issue labeled '${WONTFIX_LABEL}': ${filing.issue.url}`)
           } else {

.github/actions/file/src/isWontfixIssue.ts

@@ -0,0 +1,40 @@
+import type {Octokit} from '@octokit/core'
+import type {Issue} from './Issue.js'
+
+/** Issues with this label are intentionally closed and should not be reopened. */
+export const WONTFIX_LABEL = 'wontfix'
+
+// Fetch every closed wontfix issue once so the per-filing check is a set lookup
+export async function getWontfixIssueNumbers(
+  octokit: Octokit,
+  {owner, repository}: {owner: string; repository: string},
+): Promise<Set<number>> {
+  const wontfixIssueNumbers = new Set<number>()
+  const perPage = 100
+  for (let page = 1; ; page++) {
+    const response = await octokit.request(`GET /repos/${owner}/${repository}/issues`, {
+      owner,
+      repo: repository,
+      state: 'closed',
+      labels: WONTFIX_LABEL,
+      per_page: perPage,
+      page,
+    })
+    const issues = (response.data as Array<{number: number; pull_request?: unknown}>) ?? []
+    for (const issue of issues) {
+      // The issues endpoint also returns pull requests; skip them
+      if (!issue.pull_request) {
+        wontfixIssueNumbers.add(issue.number)
+      }
+    }
+    if (issues.length < perPage) {
+      break
+    }
+  }
+  return wontfixIssueNumbers
+}
+
+// The single place to decide whether a repeated filing's issue should reopen
+export function shouldReopenIssue(issue: Issue, wontfixIssueNumbers: Set<number>): boolean {
+  return !wontfixIssueNumbers.has(issue.issueNumber)
+}

.github/actions/file/src/shouldReopenIssue.ts

@@ -0,0 +1,90 @@
+import {describe, it, expect, vi, beforeEach} from 'vitest'
+
+import {getWontfixIssueNumbers, shouldReopenIssue, WONTFIX_LABEL} from '../src/shouldReopenIssue.ts'
+import {Issue} from '../src/Issue.ts'
+
+function issueAt(issueNumber: number): Issue {
+  return new Issue({
+    id: issueNumber,
+    nodeId: `node-${issueNumber}`,
+    url: `https://github.com/org/filing-repo/issues/${issueNumber}`,
+    title: `Accessibility issue ${issueNumber}`,
+    state: 'closed',
+  })
+}
+
+// `pages` is consumed one response per request call, in order.
+function mockOctokit(pages: Array<Array<{number: number; pull_request?: unknown}>>) {
+  const request = vi.fn()
+  for (const page of pages) {
+    request.mockResolvedValueOnce({data: page})
+  }
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  return {request} as any
+}
+
+describe('getWontfixIssueNumbers', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('returns the numbers of closed wontfix issues as a set', async () => {
+    const octokit = mockOctokit([[{number: 1}, {number: 5}, {number: 9}]])
+
+    const result = await getWontfixIssueNumbers(octokit, {owner: 'org', repository: 'repo'})
+
+    expect(result).toEqual(new Set([1, 5, 9]))
+  })
+
+  it('requests closed issues filtered by the wontfix label', async () => {
+    const octokit = mockOctokit([[]])
+
+    await getWontfixIssueNumbers(octokit, {owner: 'org', repository: 'repo'})
+
+    expect(octokit.request).toHaveBeenCalledWith(
+      'GET /repos/org/repo/issues',
+      expect.objectContaining({state: 'closed', labels: WONTFIX_LABEL}),
+    )
+  })
+
+  it('returns an empty set when no issues are labeled wontfix', async () => {
+    const octokit = mockOctokit([[]])
+
+    const result = await getWontfixIssueNumbers(octokit, {owner: 'org', repository: 'repo'})
+
+    expect(result.size).toBe(0)
+  })
+
+  it('paginates until a short page is returned', async () => {
+    const firstPage = Array.from({length: 100}, (_, i) => ({number: i + 1}))
+    const octokit = mockOctokit([firstPage, [{number: 101}]])
+
+    const result = await getWontfixIssueNumbers(octokit, {owner: 'org', repository: 'repo'})
+
+    expect(octokit.request).toHaveBeenCalledTimes(2)
+    expect(result.has(1)).toBe(true)
+    expect(result.has(101)).toBe(true)
+  })
+
+  it('ignores pull requests returned by the issues endpoint', async () => {
+    const octokit = mockOctokit([[{number: 2}, {number: 3, pull_request: {url: 'https://example.com/pull/3'}}]])
+
+    const result = await getWontfixIssueNumbers(octokit, {owner: 'org', repository: 'repo'})
+
+    expect(result).toEqual(new Set([2]))
+  })
+})
+
+describe('shouldReopenIssue', () => {
+  it('returns false when the issue is in the wontfix set', () => {
+    expect(shouldReopenIssue(issueAt(7), new Set([7]))).toBe(false)
+  })
+
+  it('returns true when the issue is not in the wontfix set', () => {
+    expect(shouldReopenIssue(issueAt(7), new Set([1, 2, 3]))).toBe(true)
+  })
+
+  it('returns true when the wontfix set is empty', () => {
+    expect(shouldReopenIssue(issueAt(7), new Set())).toBe(true)
+  })
+})

.github/actions/file/tests/isWontfixIssue.test.ts

@@ -38,7 +38,8 @@ vi.mock('node:fs', () => ({
   },
 }))
 
-// Stub Octokit: `request` serves the GET that isWontfixIssue makes
+// Stub Octokit: `request` serves the list of closed `wontfix` issues that
+// getWontfixIssueNumbers fetches once up front.
 const octokitRequest = vi.fn()
 vi.mock('@octokit/core', () => ({
   Octokit: {
@@ -80,11 +81,9 @@ function setup() {
   inputs.cached_filings_file = '/tmp/cached.json'
   inputs.repository = 'org/repo'
   inputs.token = 'fake-token'
-  // GET issue: issue 1 is labeled wontfix, issue 3 is not
+  // Single up-front fetch: only issue 1 is closed and labeled wontfix
   octokitRequest.mockImplementation((route: string) =>
-    route.includes('/issues/1')
-      ? Promise.resolve({data: {labels: [{name: 'wontfix'}]}})
-      : Promise.resolve({data: {labels: []}}),
+    route.includes('GET /repos/org/repo/issues') ? Promise.resolve({data: [{number: 1}]}) : Promise.resolve({data: {}}),
   )
 }
 
@@ -124,13 +123,13 @@ describe('file action — wontfix label', () => {
 
   it('reopens as usual (and warns) when the label check fails', async () => {
     setup()
-    // The label-check GET fails for every issue (e.g. transient API error)
+    // The up-front wontfix fetch fails (e.g. transient API error)
     octokitRequest.mockRejectedValue(new Error('boom'))
 
     await runFileAction()
 
     // Both repeated filings should still be reopened rather than aborting the run
     expect(reopenIssue).toHaveBeenCalledTimes(2)
-    expect(warnLines.join('\n')).toContain('Could not check labels for')
+    expect(warnLines.join('\n')).toContain("Could not fetch 'wontfix' issues")
   })
 })