Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions .github/actions/file/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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<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> = {}
Expand Down Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions .github/actions/file/src/shouldReopenIssue.ts
Original file line number Diff line number Diff line change
@@ -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)
}
2 changes: 2 additions & 0 deletions .github/actions/file/tests/dryRun.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
90 changes: 90 additions & 0 deletions .github/actions/file/tests/shouldReopenIssue.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
135 changes: 135 additions & 0 deletions .github/actions/file/tests/wontfixReopen.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> = {}
const infoLines: string[] = []
const warnLines: string[] = []
const outputs: Record<string, string> = {}
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<string, string> = {}
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: '<span>Low contrast</span>',
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: '<h3>Skipped</h3>'}

// 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")
})
})
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading