diff --git a/.github/actions/file/src/index.ts b/.github/actions/file/src/index.ts index fd16b68..832a701 100644 --- a/.github/actions/file/src/index.ts +++ b/.github/actions/file/src/index.ts @@ -10,6 +10,7 @@ import {closeIssue} from './closeIssue.js' import {isNewFiling} from './isNewFiling.js' import {isRepeatedFiling} from './isRepeatedFiling.js' import {isResolvedFiling} from './isResolvedFiling.js' +import {getWontfixIssueNumbers, shouldReopenIssue, WONTFIX_LABEL} from './shouldReopenIssue.js' import {openIssue} from './openIssue.js' import {reopenIssue} from './reopenIssue.js' import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js' @@ -60,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() + 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 = {} const trackingIssueUrls: Record = {} @@ -106,15 +118,15 @@ export default async function () { }) } } else if (isRepeatedFiling(filing)) { - // Reopen the filing's issue (if necessary) and update the body with the latest finding - response = await reopenIssue( - octokit, - new Issue(filing.issue), - filing.findings[0], - repoWithOwner, - screenshotRepo, - ) - filing.issue.state = 'reopened' + const issue = new Issue(filing.issue) + 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 { + // Reopen the filing's issue and update the body with the latest finding + response = await reopenIssue(octokit, issue, filing.findings[0], repoWithOwner, screenshotRepo) + filing.issue.state = 'reopened' + } } if (response?.data && filing.issue) { // Update the filing with the latest issue data diff --git a/.github/actions/file/src/shouldReopenIssue.ts b/.github/actions/file/src/shouldReopenIssue.ts new file mode 100644 index 0000000..d431ed9 --- /dev/null +++ b/.github/actions/file/src/shouldReopenIssue.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> { + const wontfixIssueNumbers = new Set() + 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): boolean { + return !wontfixIssueNumbers.has(issue.issueNumber) +} diff --git a/.github/actions/file/tests/dryRun.test.ts b/.github/actions/file/tests/dryRun.test.ts index ce8470f..f5fdf2b 100644 --- a/.github/actions/file/tests/dryRun.test.ts +++ b/.github/actions/file/tests/dryRun.test.ts @@ -185,6 +185,8 @@ describe('file action — dry_run', () => { openIssue.mockResolvedValue(resp) reopenIssue.mockResolvedValue(resp) closeIssue.mockResolvedValue(resp) + // the wontfix-label check issues a GET before reopening; return no labels so the reopen proceeds + octokitRequest.mockResolvedValue({data: {labels: []}}) await runFileAction() diff --git a/.github/actions/file/tests/shouldReopenIssue.test.ts b/.github/actions/file/tests/shouldReopenIssue.test.ts new file mode 100644 index 0000000..7eedc13 --- /dev/null +++ b/.github/actions/file/tests/shouldReopenIssue.test.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>) { + 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) + }) +}) diff --git a/.github/actions/file/tests/wontfixReopen.test.ts b/.github/actions/file/tests/wontfixReopen.test.ts new file mode 100644 index 0000000..b938d2b --- /dev/null +++ b/.github/actions/file/tests/wontfixReopen.test.ts @@ -0,0 +1,135 @@ +import {describe, it, expect, vi, beforeEach, afterEach} from 'vitest' + +const openIssue = vi.fn() +const reopenIssue = vi.fn() +const closeIssue = vi.fn() +vi.mock('../src/openIssue.js', () => ({openIssue: (...args: unknown[]) => openIssue(...args)})) +vi.mock('../src/reopenIssue.js', () => ({reopenIssue: (...args: unknown[]) => reopenIssue(...args)})) +vi.mock('../src/closeIssue.js', () => ({closeIssue: (...args: unknown[]) => closeIssue(...args)})) + +const inputs: Record = {} +const infoLines: string[] = [] +const warnLines: string[] = [] +const outputs: Record = {} +vi.mock('@actions/core', () => ({ + getInput: (name: string) => inputs[name] ?? '', + getBooleanInput: (name: string) => (inputs[name] ?? 'false') === 'true', + setOutput: (name: string, value: string) => { + outputs[name] = value + }, + info: (msg: string) => { + infoLines.push(msg) + }, + debug: () => {}, + warning: (msg: string) => { + warnLines.push(msg) + }, + setFailed: () => {}, +})) + +// Feed findings/cached filings in +const files: Record = {} +vi.mock('node:fs', () => ({ + default: { + readFileSync: (p: string) => files[p], + writeFileSync: (p: string, data: string) => { + files[p] = data + }, + }, +})) + +// 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: { + plugin: () => + class { + request = octokitRequest + }, + }, +})) +vi.mock('@octokit/plugin-throttling', () => ({throttling: {}})) + +import runFileAction from '../src/index.ts' + +const wontfixFinding = { + scannerType: 'axe', + ruleId: 'color-contrast', + url: 'https://example.com/page', + html: 'Low contrast', + problemShort: 'elements must meet minimum color contrast ratio thresholds', + problemUrl: 'https://dequeuniversity.com/rules/axe/4.10/color-contrast', + solutionShort: 'ensure sufficient contrast', +} +const normalFinding = {...wontfixFinding, ruleId: 'heading-order', html: '

Skipped

'} + +// Both cached filings' findings reappear this run, so both are repeated +const wontfixCached = { + issue: {id: 1, nodeId: 'N1', url: 'https://github.com/org/repo/issues/1', title: 'wontfix'}, + findings: [wontfixFinding], +} +const normalCached = { + issue: {id: 3, nodeId: 'N3', url: 'https://github.com/org/repo/issues/3', title: 'normal'}, + findings: [normalFinding], +} + +function setup() { + files['/tmp/findings.json'] = JSON.stringify([wontfixFinding, normalFinding]) + files['/tmp/cached.json'] = JSON.stringify([wontfixCached, normalCached]) + inputs.findings_file = '/tmp/findings.json' + inputs.cached_filings_file = '/tmp/cached.json' + inputs.repository = 'org/repo' + inputs.token = 'fake-token' + // Single up-front fetch: only issue 1 is closed and labeled wontfix + octokitRequest.mockImplementation((route: string) => + route.includes('GET /repos/org/repo/issues') ? Promise.resolve({data: [{number: 1}]}) : Promise.resolve({data: {}}), + ) +} + +describe('file action — wontfix label', () => { + beforeEach(() => { + vi.clearAllMocks() + infoLines.length = 0 + warnLines.length = 0 + for (const k of Object.keys(inputs)) delete inputs[k] + for (const k of Object.keys(outputs)) delete outputs[k] + }) + afterEach(() => { + vi.restoreAllMocks() + }) + + it('reopens the unlabeled issue but not the one labeled wontfix', async () => { + setup() + + await runFileAction() + + expect(reopenIssue).toHaveBeenCalledTimes(1) + const reopenedIssue = reopenIssue.mock.calls[0][1] as {url: string} + expect(reopenedIssue.url).toBe('https://github.com/org/repo/issues/3') + expect(openIssue).not.toHaveBeenCalled() + expect(closeIssue).not.toHaveBeenCalled() + }) + + it('logs that it skipped the wontfix issue', async () => { + setup() + + await runFileAction() + + expect(infoLines.join('\n')).toContain( + "Skipping reopen of issue labeled 'wontfix': https://github.com/org/repo/issues/1", + ) + }) + + it('reopens as usual (and warns) when the label check fails', async () => { + setup() + // 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 fetch 'wontfix' issues") + }) +}) diff --git a/README.md b/README.md index a20261e..2814517 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,14 @@ If your login flow is more complex—if it requires two-factor authentication, s --- +## Keeping an issue closed with `wontfix` + +When the scanner files an issue for an accessibility finding and that same finding turns up again on a later run, it reopens closed issues so the problem doesn't get lost. Sometimes, though, you may want a closed issue to _stay_ closed -- for example, if you've decided not to act on a particular finding, or if you're already tracking the work outside of GitHub issues. + +To stop the scanner from reopening a closed issue, add the **`wontfix`** label to it. On its next run, the scanner sees the label and skips reopening the issue, leaving it closed. + +--- + ## Configuring GitHub Copilot The a11y scanner leverages GitHub Copilot coding agent, which can be configured with custom instructions: