From 35ebacb888989cb7dfef419a63bb0155ca692c1a Mon Sep 17 00:00:00 2001 From: taarikashenafi Date: Mon, 22 Jun 2026 16:06:54 -0500 Subject: [PATCH 1/6] Add group_by option to consolidate scanner issues --- .github/actions/file/README.md | 27 +++++- .github/actions/file/action.yml | 32 ++++--- .github/actions/file/src/generateIssueBody.ts | 18 +++- .github/actions/file/src/index.ts | 30 ++++--- .github/actions/file/src/openIssue.ts | 15 ++-- .github/actions/file/src/reopenIssue.ts | 6 +- .github/actions/file/src/types.d.ts | 2 + .../file/src/updateFilingsWithNewFindings.ts | 45 +++++++--- .github/actions/file/tests/dryRun.test.ts | 37 +++++++- .../file/tests/generateIssueBody.test.ts | 15 ++++ .github/actions/file/tests/openIssue.test.ts | 24 ++++-- .../actions/file/tests/reopenIssue.test.ts | 14 +-- .../updateFilingsWithNewFindings.test.ts | 86 +++++++++++++++++++ README.md | 40 +++++---- action.yml | 5 ++ 15 files changed, 313 insertions(+), 83 deletions(-) create mode 100644 .github/actions/file/tests/updateFilingsWithNewFindings.test.ts diff --git a/.github/actions/file/README.md b/.github/actions/file/README.md index 7680cce4..fd956e46 100644 --- a/.github/actions/file/README.md +++ b/.github/actions/file/README.md @@ -11,6 +11,7 @@ Files GitHub issues to track potential accessibility gaps. **Required** Path to a JSON file containing the list of potential accessibility gaps. The path can be absolute or relative to the working directory (which defaults to `GITHUB_WORKSPACE`). For example: `findings.json`. The file should contain a JSON array of finding objects. For example: + ```json [] ``` @@ -28,15 +29,31 @@ The file should contain a JSON array of finding objects. For example: **Optional** Path to a JSON file containing cached filings from previous runs. The path can be absolute or relative to the working directory (which defaults to `GITHUB_WORKSPACE`). Without this, duplicate issues may be filed. For example: `cached-filings.json`. The file should contain a JSON array of filing objects. For example: + ```json [ { "findings": [], - "issue": {"id":1,"nodeId":"SXNzdWU6MQ==","url":"https://github.com/github/docs/issues/123","title":"Accessibility issue: 1"} + "issue": { + "id": 1, + "nodeId": "SXNzdWU6MQ==", + "url": "https://github.com/github/docs/issues/123", + "title": "Accessibility issue: 1" + } } ] ``` +#### `group_by` + +**Optional** How to consolidate findings into issues. One of: + +- `finding` (default): one issue per individual violation — current behavior, unchanged. +- `rule`: one issue per rule (`ruleId`/`scannerType`), aggregating every occurrence across all scanned URLs. +- `rule+url`: one issue per rule per scanned URL. + +When grouping, each additional occurrence is appended to the single "umbrella" issue body as a checklist item under an **Occurrences** section rather than spawning a new issue. This is the preferred mechanism for consolidating issues over `open_grouped_issues`. + ### Outputs #### `filings_file` @@ -44,11 +61,17 @@ The file should contain a JSON array of filing objects. For example: Absolute path to a JSON file containing the list of issues filed (and their associated finding(s)). The action writes this file to a temporary directory and returns the absolute path. For example: `$RUNNER_TEMP/filings-.json`. The file will contain a JSON array of filing objects. For example: + ```json [ { "findings": [], - "issue": {"id":1,"nodeId":"SXNzdWU6MQ==","url":"https://github.com/github/docs/issues/123","title":"Accessibility issue: 1"} + "issue": { + "id": 1, + "nodeId": "SXNzdWU6MQ==", + "url": "https://github.com/github/docs/issues/123", + "title": "Accessibility issue: 1" + } } ] ``` diff --git a/.github/actions/file/action.yml b/.github/actions/file/action.yml index 7d76af7a..714bf096 100644 --- a/.github/actions/file/action.yml +++ b/.github/actions/file/action.yml @@ -1,21 +1,21 @@ -name: "File" -description: "Files GitHub issues to track potential accessibility gaps." +name: 'File' +description: 'Files GitHub issues to track potential accessibility gaps.' inputs: findings_file: - description: "Path to a JSON file containing the list of potential accessibility gaps" + description: 'Path to a JSON file containing the list of potential accessibility gaps' required: true repository: - description: "Repository (with owner) to file issues in" + description: 'Repository (with owner) to file issues in' required: true token: description: "Token with fine-grained permission 'issues: write'" required: true base_url: - description: "Optional base URL to pass into Octokit for the GitHub API (for example, `https://YOUR_HOSTNAME/api/v3` for GitHub Enterprise Server)" + description: 'Optional base URL to pass into Octokit for the GitHub API (for example, `https://YOUR_HOSTNAME/api/v3` for GitHub Enterprise Server)' required: false cached_filings_file: - description: "Path to a JSON file containing cached filings from previous runs. Without this, duplicate issues may be filed." + description: 'Path to a JSON file containing cached filings from previous runs. Without this, duplicate issues may be filed.' required: false screenshot_repository: description: "Repository (with owner) where screenshots are stored on the gh-cache branch. Defaults to the 'repository' input if not set. Required if issues are open in a different repo to construct proper screenshot URLs." @@ -23,20 +23,24 @@ inputs: open_grouped_issues: description: "In the 'file' step, also open grouped issues which link to all issues with the same root cause" required: false - default: "false" + default: 'false' + group_by: + description: "How to group findings into issues: 'finding' (one issue per violation, default), 'rule' (one issue per rule), or 'rule+url' (one issue per rule per scanned URL)." + required: false + default: 'finding' dry_run: - description: "When true, log the issues that would be filed without opening, closing, or reopening any issues." + description: 'When true, log the issues that would be filed without opening, closing, or reopening any issues.' required: false - default: "false" + default: 'false' outputs: filings_file: - description: "Path to a JSON file containing the list of issues filed (and their associated finding(s))" + description: 'Path to a JSON file containing the list of issues filed (and their associated finding(s))' runs: - using: "node24" - main: "bootstrap.js" + using: 'node24' + main: 'bootstrap.js' branding: - icon: "compass" - color: "blue" + icon: 'compass' + color: 'blue' diff --git a/.github/actions/file/src/generateIssueBody.ts b/.github/actions/file/src/generateIssueBody.ts index 18e25d31..0f6cc307 100644 --- a/.github/actions/file/src/generateIssueBody.ts +++ b/.github/actions/file/src/generateIssueBody.ts @@ -1,6 +1,9 @@ import type {Finding} from './types.d.js' -export function generateIssueBody(finding: Finding, screenshotRepo: string): string { +export function generateIssueBody(findingOrFindings: Finding | Finding[], screenshotRepo: string): string { + const findings = Array.isArray(findingOrFindings) ? findingOrFindings : [findingOrFindings] + const finding = findings[0] + const solutionLong = finding.solutionLong ?.split('\n') .map((line: string) => @@ -18,6 +21,17 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str ` } + // When this issue groups multiple findings, list each occurrence as a checklist item. + let occurrencesSection = '' + if (findings.length > 1) { + const items = findings.map(f => `- [ ] ${f.html ? `\`${f.html}\` on ${f.url}` : f.url}`).join('\n') + occurrencesSection = ` +## Occurrences (${findings.length}) + +${items} +` + } + const acceptanceCriteria = `## Acceptance Criteria - [ ] The specific violation reported in this issue is no longer reproducible. - [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization. @@ -30,7 +44,7 @@ An accessibility scan ${finding.html ? `flagged the element \`${finding.html}\`` ${screenshotSection ?? ''} To fix this, ${finding.solutionShort}. ${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''} - +${occurrencesSection} ${acceptanceCriteria} ` diff --git a/.github/actions/file/src/index.ts b/.github/actions/file/src/index.ts index fd16b68d..6d95dfd6 100644 --- a/.github/actions/file/src/index.ts +++ b/.github/actions/file/src/index.ts @@ -1,4 +1,12 @@ -import type {Finding, ResolvedFiling, RepeatedFiling, FindingGroupIssue, Filing, IssueResponse} from './types.d.js' +import type { + Finding, + ResolvedFiling, + RepeatedFiling, + FindingGroupIssue, + Filing, + IssueResponse, + GroupBy, +} from './types.d.js' import fs from 'node:fs' import path from 'node:path' import process from 'node:process' @@ -29,6 +37,13 @@ export default async function () { ? JSON.parse(fs.readFileSync(cachedFilingsFile, 'utf8')) : [] const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues') + const groupByInput = core.getInput('group_by') || 'finding' + const validGroupByValues: GroupBy[] = ['finding', 'rule', 'rule+url'] + if (!validGroupByValues.includes(groupByInput as GroupBy)) { + core.setFailed(`Invalid 'group_by' value: '${groupByInput}'. Must be one of: ${validGroupByValues.join(', ')}.`) + return + } + const groupBy = groupByInput as GroupBy const dryRun = core.getBooleanInput('dry_run') core.debug(`Input: 'findings_file: ${findingsFile}'`) core.debug(`Input: 'repository: ${repoWithOwner}'`) @@ -36,6 +51,7 @@ export default async function () { core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`) core.debug(`Input: 'cached_filings_file: ${cachedFilingsFile}'`) core.debug(`Input: 'open_grouped_issues: ${shouldOpenGroupedIssues}'`) + core.debug(`Input: 'group_by: ${groupBy}'`) core.debug(`Input: 'dry_run: ${dryRun}'`) const octokit = new OctokitWithThrottling({ @@ -58,7 +74,7 @@ export default async function () { }, }, }) - const filings = updateFilingsWithNewFindings(cachedFilings, findings) + const filings = updateFilingsWithNewFindings(cachedFilings, findings, groupBy) // Track new issues for grouping const newIssuesByProblemShort: Record = {} @@ -91,7 +107,7 @@ export default async function () { filing.issue.state = 'closed' } else if (isNewFiling(filing)) { // Open a new issue for the filing - response = await openIssue(octokit, repoWithOwner, filing.findings[0], screenshotRepo) + response = await openIssue(octokit, repoWithOwner, filing.findings, screenshotRepo) ;(filing as Filing).issue = {state: 'open'} as Issue // Track for grouping @@ -107,13 +123,7 @@ 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, - ) + response = await reopenIssue(octokit, new Issue(filing.issue), filing.findings, repoWithOwner, screenshotRepo) filing.issue.state = 'reopened' } if (response?.data && filing.issue) { diff --git a/.github/actions/file/src/openIssue.ts b/.github/actions/file/src/openIssue.ts index 937f06cf..d25f23d8 100644 --- a/.github/actions/file/src/openIssue.ts +++ b/.github/actions/file/src/openIssue.ts @@ -17,22 +17,25 @@ function truncateWithEllipsis(text: string, maxLength: number): string { return text.length > maxLength ? text.slice(0, maxLength - 1) + '…' : text } -export async function openIssue(octokit: Octokit, repoWithOwner: string, finding: Finding, screenshotRepo?: string) { +export async function openIssue(octokit: Octokit, repoWithOwner: string, findings: Finding[], screenshotRepo?: string) { const owner = repoWithOwner.split('/')[0] const repo = repoWithOwner.split('/')[1] + const primary = findings[0] - const labels = [`${finding.scannerType}-scanning-issue`] + const labels = [`${primary.scannerType}-scanning-issue`] // Only include a ruleId label when it's defined - if (finding.ruleId) { - labels.push(`${finding.scannerType} rule: ${finding.ruleId}`) + if (primary.ruleId) { + labels.push(`${primary.scannerType} rule: ${primary.ruleId}`) } + const count = findings.length + const titleSuffix = count > 1 ? ` (${count} occurrences)` : ` on ${new URL(primary.url).pathname}` const title = truncateWithEllipsis( - `Accessibility issue: ${finding.problemShort[0].toUpperCase() + finding.problemShort.slice(1)} on ${new URL(finding.url).pathname}`, + `Accessibility issue: ${primary.problemShort[0].toUpperCase() + primary.problemShort.slice(1)}${titleSuffix}`, GITHUB_ISSUE_TITLE_MAX_LENGTH, ) - const body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner) + const body = generateIssueBody(findings, screenshotRepo ?? repoWithOwner) return octokit.request(`POST /repos/${owner}/${repo}/issues`, { owner, diff --git a/.github/actions/file/src/reopenIssue.ts b/.github/actions/file/src/reopenIssue.ts index 329c6952..b37c8ca0 100644 --- a/.github/actions/file/src/reopenIssue.ts +++ b/.github/actions/file/src/reopenIssue.ts @@ -6,13 +6,13 @@ import {generateIssueBody} from './generateIssueBody.js' export async function reopenIssue( octokit: Octokit, {owner, repository, issueNumber}: Issue, - finding?: Finding, + findings?: Finding[], repoWithOwner?: string, screenshotRepo?: string, ) { let body: string | undefined - if (finding && repoWithOwner) { - body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner) + if (findings && findings.length > 0 && repoWithOwner) { + body = generateIssueBody(findings, screenshotRepo ?? repoWithOwner) } return octokit.request(`PATCH /repos/${owner}/${repository}/issues/${issueNumber}`, { diff --git a/.github/actions/file/src/types.d.ts b/.github/actions/file/src/types.d.ts index ee91bc67..c7bf8041 100644 --- a/.github/actions/file/src/types.d.ts +++ b/.github/actions/file/src/types.d.ts @@ -1,3 +1,5 @@ +export type GroupBy = 'finding' | 'rule' | 'rule+url' + export type Finding = { scannerType: string ruleId?: string diff --git a/.github/actions/file/src/updateFilingsWithNewFindings.ts b/.github/actions/file/src/updateFilingsWithNewFindings.ts index eee1c6ae..6e258f87 100644 --- a/.github/actions/file/src/updateFilingsWithNewFindings.ts +++ b/.github/actions/file/src/updateFilingsWithNewFindings.ts @@ -1,25 +1,42 @@ -import type {Finding, ResolvedFiling, NewFiling, RepeatedFiling, Filing} from './types.d.js' +import type {Finding, ResolvedFiling, NewFiling, RepeatedFiling, Filing, GroupBy} from './types.d.js' function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string { return filing.issue.url } -function getFindingKey(finding: Finding): string { - if (finding.ruleId && finding.html) { - return `${finding.url};${finding.ruleId};${finding.html}` +/** + * Computes the dedup key for a finding based on the grouping mode. + * - 'finding' (default): one filing per individual violation (URL + rule + element). + * - 'rule': one filing per rule, aggregating every occurrence across all URLs. + * - 'rule+url': one filing per rule per scanned URL. + */ +function getFindingKey(finding: Finding, groupBy: GroupBy): string { + const rule = finding.ruleId ?? `${finding.scannerType};${finding.problemUrl}` + + switch (groupBy) { + case 'rule': + return rule + case 'rule+url': + return `${finding.url};${rule}` + case 'finding': + default: + if (finding.ruleId && finding.html) { + return `${finding.url};${finding.ruleId};${finding.html}` + } + return `${finding.url};${finding.scannerType};${finding.problemUrl}` } - return `${finding.url};${finding.scannerType};${finding.problemUrl}` } export function updateFilingsWithNewFindings( filings: (ResolvedFiling | RepeatedFiling)[], findings: Finding[], + groupBy: GroupBy = 'finding', ): Filing[] { const filingKeys: { [key: string]: ResolvedFiling | RepeatedFiling } = {} const findingKeys: {[key: string]: string} = {} - const newFilings: NewFiling[] = [] + const newFilingKeys: {[key: string]: NewFiling} = {} // Create maps for filing and finding data from previous runs, for quick lookups for (const filing of filings) { @@ -29,21 +46,25 @@ export function updateFilingsWithNewFindings( findings: [], } for (const finding of filing.findings) { - findingKeys[getFindingKey(finding)] = getFilingKey(filing) + findingKeys[getFindingKey(finding, groupBy)] = getFilingKey(filing) } } for (const finding of findings) { - const filingKey = findingKeys[getFindingKey(finding)] + const key = getFindingKey(finding, groupBy) + const filingKey = findingKeys[key] if (filingKey) { - // This finding already has an associated filing; add it to that filing's findings + // This finding already maps to an existing issue; append it to that filing ;(filingKeys[filingKey] as RepeatedFiling).findings.push(finding) + } else if (newFilingKeys[key]) { + // A new filing for this group already exists this run; append to it + newFilingKeys[key].findings.push(finding) } else { - // This finding is new; create a new entry with no associated issue yet - newFilings.push({findings: [finding]}) + // First occurrence of this group with no existing issue; start a new filing + newFilingKeys[key] = {findings: [finding]} } } const updatedFilings = Object.values(filingKeys) - return [...updatedFilings, ...newFilings] + return [...updatedFilings, ...Object.values(newFilingKeys)] } diff --git a/.github/actions/file/tests/dryRun.test.ts b/.github/actions/file/tests/dryRun.test.ts index ce8470fd..c8567d22 100644 --- a/.github/actions/file/tests/dryRun.test.ts +++ b/.github/actions/file/tests/dryRun.test.ts @@ -12,6 +12,7 @@ vi.mock('../src/closeIssue.js', () => ({closeIssue: (...args: unknown[]) => clos const inputs: Record = {} const infoLines: string[] = [] const outputs: Record = {} +const failedMessages: string[] = [] vi.mock('@actions/core', () => ({ getInput: (name: string) => inputs[name] ?? '', getBooleanInput: (name: string) => (inputs[name] ?? 'false') === 'true', @@ -23,7 +24,9 @@ vi.mock('@actions/core', () => ({ }, debug: () => {}, warning: () => {}, - setFailed: () => {}, + setFailed: (msg: string) => { + failedMessages.push(msg) + }, })) // --- Mock fs: feed findings/cached filings in, swallow the output write --- @@ -91,6 +94,7 @@ describe('file action — dry_run', () => { beforeEach(() => { vi.clearAllMocks() infoLines.length = 0 + failedMessages.length = 0 for (const k of Object.keys(inputs)) delete inputs[k] for (const k of Object.keys(outputs)) delete outputs[k] vi.spyOn(console, 'table').mockImplementation(() => {}) @@ -192,4 +196,35 @@ describe('file action — dry_run', () => { expect(reopenIssue).toHaveBeenCalled() expect(closeIssue).toHaveBeenCalled() }) + + it("group_by 'rule' collapses multiple same-rule findings into a single OPEN", async () => { + // Three brand-new color-contrast findings across two URLs, no cached filings. + const ccA1 = {...finding, url: 'https://example.com/a', html: '1'} + const ccA2 = {...finding, url: 'https://example.com/a', html: '2'} + const ccB1 = {...finding, url: 'https://example.com/b', html: '3'} + files['/tmp/findings.json'] = JSON.stringify([ccA1, ccA2, ccB1]) + files['/tmp/cached.json'] = JSON.stringify([]) + inputs.findings_file = '/tmp/findings.json' + inputs.cached_filings_file = '/tmp/cached.json' + inputs.repository = 'org/repo' + inputs.token = 'fake-token' + inputs.dry_run = 'true' + inputs.group_by = 'rule' + + await runFileAction() + + expect(vi.mocked(console.table)).toHaveBeenCalledWith(expect.objectContaining({open: 1})) + }) + + it('fails fast on an invalid group_by value', async () => { + setup() + inputs.group_by = 'bogus' + + await runFileAction() + + expect(failedMessages.join('\n')).toContain("Invalid 'group_by' value: 'bogus'") + expect(openIssue).not.toHaveBeenCalled() + expect(reopenIssue).not.toHaveBeenCalled() + expect(closeIssue).not.toHaveBeenCalled() + }) }) diff --git a/.github/actions/file/tests/generateIssueBody.test.ts b/.github/actions/file/tests/generateIssueBody.test.ts index 167ee5f8..d3ad8765 100644 --- a/.github/actions/file/tests/generateIssueBody.test.ts +++ b/.github/actions/file/tests/generateIssueBody.test.ts @@ -76,4 +76,19 @@ describe('generateIssueBody', () => { expect(body).toContain(`found an issue on ${findingWithEmptyOptionalFields.url}`) expect(body).not.toContain('flagged the element') }) + + it('omits the Occurrences section for a single finding', () => { + const body = generateIssueBody(baseFinding, 'github/accessibility-scanner') + + expect(body).not.toContain('## Occurrences') + }) + + it('renders an Occurrences checklist when given multiple findings', () => { + const second = {...baseFinding, url: 'https://example.com/other', html: 'Link'} + const body = generateIssueBody([baseFinding, second], 'github/accessibility-scanner') + + expect(body).toContain('## Occurrences (2)') + expect(body).toContain(`- [ ] \`${baseFinding.html}\` on ${baseFinding.url}`) + expect(body).toContain(`- [ ] \`${second.html}\` on ${second.url}`) + }) }) diff --git a/.github/actions/file/tests/openIssue.test.ts b/.github/actions/file/tests/openIssue.test.ts index 77a184c3..0aaa526b 100644 --- a/.github/actions/file/tests/openIssue.test.ts +++ b/.github/actions/file/tests/openIssue.test.ts @@ -28,21 +28,21 @@ function mockOctokit() { describe('openIssue', () => { it('passes screenshotRepo to generateIssueBody when provided', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/filing-repo', baseFinding, 'org/workflow-repo') + await openIssue(octokit, 'org/filing-repo', [baseFinding], 'org/workflow-repo') - expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/workflow-repo') + expect(generateIssueBody).toHaveBeenCalledWith([baseFinding], 'org/workflow-repo') }) it('falls back to repoWithOwner when screenshotRepo is not provided', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/filing-repo', baseFinding) + await openIssue(octokit, 'org/filing-repo', [baseFinding]) - expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/filing-repo') + expect(generateIssueBody).toHaveBeenCalledWith([baseFinding], 'org/filing-repo') }) it('posts to the correct filing repo, not the screenshot repo', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/filing-repo', baseFinding, 'org/workflow-repo') + await openIssue(octokit, 'org/filing-repo', [baseFinding], 'org/workflow-repo') expect(octokit.request).toHaveBeenCalledWith( 'POST /repos/org/filing-repo/issues', @@ -55,7 +55,7 @@ describe('openIssue', () => { it('includes the correct labels based on the finding', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/repo', baseFinding) + await openIssue(octokit, 'org/repo', [baseFinding]) expect(octokit.request).toHaveBeenCalledWith( expect.any(String), @@ -71,10 +71,20 @@ describe('openIssue', () => { ...baseFinding, problemShort: 'a'.repeat(300), } - await openIssue(octokit, 'org/repo', longFinding) + await openIssue(octokit, 'org/repo', [longFinding]) const callArgs = octokit.request.mock.calls[0][1] expect(callArgs.title.length).toBeLessThanOrEqual(256) expect(callArgs.title).toMatch(/…$/) }) + + it('includes an occurrence count in the title when grouping multiple findings', async () => { + const octokit = mockOctokit() + const second = {...baseFinding, url: 'https://example.com/other', html: 'Another'} + await openIssue(octokit, 'org/repo', [baseFinding, second]) + + const callArgs = octokit.request.mock.calls[0][1] + expect(callArgs.title).toContain('(2 occurrences)') + expect(generateIssueBody).toHaveBeenCalledWith([baseFinding, second], 'org/repo') + }) }) diff --git a/.github/actions/file/tests/reopenIssue.test.ts b/.github/actions/file/tests/reopenIssue.test.ts index f5b34ef8..a739a3f2 100644 --- a/.github/actions/file/tests/reopenIssue.test.ts +++ b/.github/actions/file/tests/reopenIssue.test.ts @@ -41,16 +41,16 @@ describe('reopenIssue', () => { it('passes screenshotRepo to generateIssueBody when provided', async () => { const octokit = mockOctokit() - await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo') + await reopenIssue(octokit, testIssue, [baseFinding], 'org/filing-repo', 'org/workflow-repo') - expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/workflow-repo') + expect(generateIssueBody).toHaveBeenCalledWith([baseFinding], 'org/workflow-repo') }) it('falls back to repoWithOwner when screenshotRepo is not provided', async () => { const octokit = mockOctokit() - await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo') + await reopenIssue(octokit, testIssue, [baseFinding], 'org/filing-repo') - expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/filing-repo') + expect(generateIssueBody).toHaveBeenCalledWith([baseFinding], 'org/filing-repo') }) it('does not generate a body when finding is not provided', async () => { @@ -66,14 +66,14 @@ describe('reopenIssue', () => { it('does not generate a body when repoWithOwner is not provided', async () => { const octokit = mockOctokit() - await reopenIssue(octokit, testIssue, baseFinding) + await reopenIssue(octokit, testIssue, [baseFinding]) expect(generateIssueBody).not.toHaveBeenCalled() }) it('sends PATCH to the correct issue URL with state open', async () => { const octokit = mockOctokit() - await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo') + await reopenIssue(octokit, testIssue, [baseFinding], 'org/filing-repo', 'org/workflow-repo') expect(octokit.request).toHaveBeenCalledWith( 'PATCH /repos/org/filing-repo/issues/7', @@ -86,7 +86,7 @@ describe('reopenIssue', () => { it('includes generated body when finding and repoWithOwner are provided', async () => { const octokit = mockOctokit() - await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo') + await reopenIssue(octokit, testIssue, [baseFinding], 'org/filing-repo', 'org/workflow-repo') expect(octokit.request).toHaveBeenCalledWith( expect.any(String), diff --git a/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts b/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts new file mode 100644 index 00000000..336ba504 --- /dev/null +++ b/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts @@ -0,0 +1,86 @@ +import {describe, it, expect} from 'vitest' +import {updateFilingsWithNewFindings} from '../src/updateFilingsWithNewFindings.ts' + +const cc = (url: string, html: string) => ({ + scannerType: 'axe', + ruleId: 'color-contrast', + url, + html, + problemShort: 'elements must meet minimum color contrast ratio thresholds', + problemUrl: 'https://dequeuniversity.com/rules/axe/4.10/color-contrast', + solutionShort: 'ensure sufficient contrast', +}) + +describe('updateFilingsWithNewFindings — group_by', () => { + const findings = [ + cc('https://example.com/a', '1'), + cc('https://example.com/a', '2'), + cc('https://example.com/b', '3'), + ] + + it("defaults to 'finding': one filing per individual violation", () => { + const result = updateFilingsWithNewFindings([], findings) + expect(result).toHaveLength(3) + for (const filing of result) expect(filing.findings).toHaveLength(1) + }) + + it("'rule': collapses all occurrences of a rule into a single filing", () => { + const result = updateFilingsWithNewFindings([], findings, 'rule') + expect(result).toHaveLength(1) + expect(result[0].findings).toHaveLength(3) + }) + + it("'rule+url': one filing per rule per URL", () => { + const result = updateFilingsWithNewFindings([], findings, 'rule+url') + expect(result).toHaveLength(2) + const counts = result.map(f => f.findings.length).sort() + expect(counts).toEqual([1, 2]) // 2 on /a, 1 on /b + }) + + it("'rule': appends new occurrences to an existing cached filing instead of opening a new issue", () => { + const cached = [ + { + issue: { + id: 1, + nodeId: 'N1', + url: 'https://github.com/org/repo/issues/1', + title: 'color-contrast', + }, + findings: [cc('https://example.com/a', '1')], + }, + ] + const result = updateFilingsWithNewFindings(cached, findings, 'rule') + // No brand-new filing; all three findings attach to the cached issue. + expect(result).toHaveLength(1) + expect(result[0].issue?.url).toBe('https://github.com/org/repo/issues/1') + expect(result[0].findings).toHaveLength(3) + }) + + it("keeps distinct rules separate under 'rule'", () => { + const mixed = [ + cc('https://example.com/a', '1'), + {...cc('https://example.com/a', '

x

'), ruleId: 'heading-order'}, + ] + const result = updateFilingsWithNewFindings([], mixed, 'rule') + expect(result).toHaveLength(2) + }) + + it("'finding' (default) preserves the original 1:1 behavior with cached filings", () => { + const cached = [ + { + issue: { + id: 1, + nodeId: 'N1', + url: 'https://github.com/org/repo/issues/1', + title: 'color-contrast', + }, + findings: [cc('https://example.com/a', '1')], + }, + ] + const result = updateFilingsWithNewFindings(cached, findings) + // One repeated filing (issues/1) plus two brand-new filings. + expect(result).toHaveLength(3) + const repeated = result.find(f => f.issue?.url === 'https://github.com/org/repo/issues/1') + expect(repeated?.findings).toHaveLength(1) + }) +}) diff --git a/README.md b/README.md index a20261e7..5330f799 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ jobs: # skip_copilot_assignment: false # Optional: Set to true to skip assigning issues to GitHub Copilot (or if you don't have GitHub Copilot) # include_screenshots: false # Optional: Set to true to capture screenshots and include links to them in filed issues # open_grouped_issues: false # Optional: Set to true to open an issue grouping individual issues per violation + # group_by: finding # Optional: 'finding' (default, one issue per violation), 'rule' (one per rule), or 'rule+url' (one per rule per URL) # dry_run: false # Optional: Set to true to scan and log what would be filed without creating/closing issues or writing the cache # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option # color_scheme: light # Optional: Playwright color scheme configuration option @@ -115,25 +116,26 @@ Trigger the workflow manually or automatically based on your configuration. The ## Action inputs -| Input | Required | Description | Example | -| ------------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | -| `urls` | No\* | Newline-delimited list of URLs to scan. Required unless `url_configs` is provided. | `https://primer.style`
`https://primer.style/octicons` | -| `repository` | Yes | Repository (with owner) for issues and PRs | `primer/primer-docs` | -| `token` | Yes | PAT with write permissions (see above) | `${{ secrets.GH_TOKEN }}` | -| `cache_key` | Yes | Key for caching results across runs
Allowed: `A-Za-z0-9._/-` | `cached_results-primer.style-main.json` | -| `base_url` | No | GitHub API base URL used by Octokit. Set this for GitHub Enterprise Server (format: `https://HOSTNAME/api/v3`). Defaults to `https://api.github.com` | `https://ghe.example.com/api/v3` | -| `login_url` | No | If scanned pages require authentication, the URL of the login page | `https://github.com/login` | -| `username` | No | If scanned pages require authentication, the username to use for login | `some-user` | -| `password` | No | If scanned pages require authentication, the password to use for login | `${{ secrets.PASSWORD }}` | -| `auth_context` | No | If scanned pages require authentication, a stringified JSON object containing username, password, cookies, and/or localStorage from an authenticated session | `{"username":"some-user","password":"***","cookies":[...]}` | -| `skip_copilot_assignment` | No | Whether to skip assigning filed issues to GitHub Copilot. Set to `true` if you don't have GitHub Copilot or prefer to handle issues manually | `true` | -| `include_screenshots` | No | Whether to capture screenshots of scanned pages and include links to them in filed issues. Screenshots are stored on the `gh-cache` branch of the repository running the workflow. Default: `false` | `true` | -| `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | -| `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | -| `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | -| `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | -| `dry_run` | No | When `true`, scan and log the issues that _would_ be filed without opening, closing, reopening, or assigning any issues — and without writing to the `gh-cache` branch. Useful for safely previewing results. Default: `false` | `true` | -| `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have an optional `excludeSelectors` field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]'` | +| Input | Required | Description | Example | +| ------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | +| `urls` | No\* | Newline-delimited list of URLs to scan. Required unless `url_configs` is provided. | `https://primer.style`
`https://primer.style/octicons` | +| `repository` | Yes | Repository (with owner) for issues and PRs | `primer/primer-docs` | +| `token` | Yes | PAT with write permissions (see above) | `${{ secrets.GH_TOKEN }}` | +| `cache_key` | Yes | Key for caching results across runs
Allowed: `A-Za-z0-9._/-` | `cached_results-primer.style-main.json` | +| `base_url` | No | GitHub API base URL used by Octokit. Set this for GitHub Enterprise Server (format: `https://HOSTNAME/api/v3`). Defaults to `https://api.github.com` | `https://ghe.example.com/api/v3` | +| `login_url` | No | If scanned pages require authentication, the URL of the login page | `https://github.com/login` | +| `username` | No | If scanned pages require authentication, the username to use for login | `some-user` | +| `password` | No | If scanned pages require authentication, the password to use for login | `${{ secrets.PASSWORD }}` | +| `auth_context` | No | If scanned pages require authentication, a stringified JSON object containing username, password, cookies, and/or localStorage from an authenticated session | `{"username":"some-user","password":"***","cookies":[...]}` | +| `skip_copilot_assignment` | No | Whether to skip assigning filed issues to GitHub Copilot. Set to `true` if you don't have GitHub Copilot or prefer to handle issues manually | `true` | +| `include_screenshots` | No | Whether to capture screenshots of scanned pages and include links to them in filed issues. Screenshots are stored on the `gh-cache` branch of the repository running the workflow. Default: `false` | `true` | +| `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | +| `group_by` | No | How to consolidate findings into issues: `finding` (default, one issue per individual violation), `rule` (one issue per rule, aggregating every occurrence across all scanned URLs), or `rule+url` (one issue per rule per scanned URL). Preferred over `open_grouped_issues` for consolidation. Default: `finding` | `rule` | +| `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | +| `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | +| `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | +| `dry_run` | No | When `true`, scan and log the issues that _would_ be filed without opening, closing, reopening, or assigning any issues — and without writing to the `gh-cache` branch. Useful for safely previewing results. Default: `false` | `true` | +| `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have an optional `excludeSelectors` field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]'` | --- diff --git a/action.yml b/action.yml index 1b852a5c..1ace330d 100644 --- a/action.yml +++ b/action.yml @@ -45,6 +45,10 @@ inputs: description: "In the 'file' step, also open grouped issues which link to all issues with the same problem" required: false default: 'false' + group_by: + description: "How to group findings into issues: 'finding' (one issue per violation, default), 'rule' (one issue per rule), or 'rule+url' (one issue per rule per scanned URL)." + required: false + default: 'finding' reduced_motion: description: 'Playwright reducedMotion setting: https://playwright.dev/docs/api/class-browser#browser-new-page-option-reduced-motion' required: false @@ -133,6 +137,7 @@ runs: cached_filings_file: ${{ steps.normalize_cache.outputs.cached_filings_file }} screenshot_repository: ${{ github.repository }} open_grouped_issues: ${{ inputs.open_grouped_issues }} + group_by: ${{ inputs.group_by }} dry_run: ${{ inputs.dry_run }} - if: ${{ steps.file.outputs.filings_file }} name: Get issues from filings From c1cf4d01fcf5a1ef464bcc12fea4db2441bbfee3 Mon Sep 17 00:00:00 2001 From: taarikashenafi Date: Mon, 22 Jun 2026 16:20:25 -0500 Subject: [PATCH 2/6] Remove comments --- .github/actions/file/src/generateIssueBody.ts | 1 - .../actions/file/src/updateFilingsWithNewFindings.ts | 10 +--------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/.github/actions/file/src/generateIssueBody.ts b/.github/actions/file/src/generateIssueBody.ts index 0f6cc307..91e10e69 100644 --- a/.github/actions/file/src/generateIssueBody.ts +++ b/.github/actions/file/src/generateIssueBody.ts @@ -21,7 +21,6 @@ export function generateIssueBody(findingOrFindings: Finding | Finding[], screen ` } - // When this issue groups multiple findings, list each occurrence as a checklist item. let occurrencesSection = '' if (findings.length > 1) { const items = findings.map(f => `- [ ] ${f.html ? `\`${f.html}\` on ${f.url}` : f.url}`).join('\n') diff --git a/.github/actions/file/src/updateFilingsWithNewFindings.ts b/.github/actions/file/src/updateFilingsWithNewFindings.ts index 6e258f87..42a712e0 100644 --- a/.github/actions/file/src/updateFilingsWithNewFindings.ts +++ b/.github/actions/file/src/updateFilingsWithNewFindings.ts @@ -4,12 +4,6 @@ function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string { return filing.issue.url } -/** - * Computes the dedup key for a finding based on the grouping mode. - * - 'finding' (default): one filing per individual violation (URL + rule + element). - * - 'rule': one filing per rule, aggregating every occurrence across all URLs. - * - 'rule+url': one filing per rule per scanned URL. - */ function getFindingKey(finding: Finding, groupBy: GroupBy): string { const rule = finding.ruleId ?? `${finding.scannerType};${finding.problemUrl}` @@ -54,13 +48,11 @@ export function updateFilingsWithNewFindings( const key = getFindingKey(finding, groupBy) const filingKey = findingKeys[key] if (filingKey) { - // This finding already maps to an existing issue; append it to that filing + // This finding already has an associated filing; add it to that filing's findings ;(filingKeys[filingKey] as RepeatedFiling).findings.push(finding) } else if (newFilingKeys[key]) { - // A new filing for this group already exists this run; append to it newFilingKeys[key].findings.push(finding) } else { - // First occurrence of this group with no existing issue; start a new filing newFilingKeys[key] = {findings: [finding]} } } From b61d801e89af74b2b463887076fcad972c26fca8 Mon Sep 17 00:00:00 2001 From: Taarik <147209483+taarikashenafi@users.noreply.github.com> Date: Tue, 23 Jun 2026 17:38:54 -0500 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Joyce Zhu --- .github/actions/file/src/generateIssueBody.ts | 2 +- .github/actions/file/src/updateFilingsWithNewFindings.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/file/src/generateIssueBody.ts b/.github/actions/file/src/generateIssueBody.ts index 91e10e69..27746d00 100644 --- a/.github/actions/file/src/generateIssueBody.ts +++ b/.github/actions/file/src/generateIssueBody.ts @@ -25,7 +25,7 @@ export function generateIssueBody(findingOrFindings: Finding | Finding[], screen if (findings.length > 1) { const items = findings.map(f => `- [ ] ${f.html ? `\`${f.html}\` on ${f.url}` : f.url}`).join('\n') occurrencesSection = ` -## Occurrences (${findings.length}) +## ${findings.length} Other Occurrences: ${items} ` diff --git a/.github/actions/file/src/updateFilingsWithNewFindings.ts b/.github/actions/file/src/updateFilingsWithNewFindings.ts index 42a712e0..910c3c3a 100644 --- a/.github/actions/file/src/updateFilingsWithNewFindings.ts +++ b/.github/actions/file/src/updateFilingsWithNewFindings.ts @@ -5,7 +5,7 @@ function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string { } function getFindingKey(finding: Finding, groupBy: GroupBy): string { - const rule = finding.ruleId ?? `${finding.scannerType};${finding.problemUrl}` + const rule = finding.ruleId ? `${finding.scannerType};${finding.ruleId}` : `${finding.scannerType};${finding.problemUrl}` switch (groupBy) { case 'rule': From 8997171e6374db454020ed40d0cdbdfacd6b37a1 Mon Sep 17 00:00:00 2001 From: taarikashenafi Date: Tue, 23 Jun 2026 17:53:32 -0500 Subject: [PATCH 4/6] Address review feedback: scannerType-safe keys, GroupBy source of truth, naming, docs --- .github/actions/file/src/generateIssueBody.ts | 4 +-- .github/actions/file/src/groupBy.ts | 7 +++++ .github/actions/file/src/index.ts | 18 +++-------- .github/actions/file/src/reopenIssue.ts | 2 +- .github/actions/file/src/types.d.ts | 2 -- .../file/src/updateFilingsWithNewFindings.ts | 7 +++-- .../file/tests/generateIssueBody.test.ts | 4 +-- .../updateFilingsWithNewFindings.test.ts | 31 ++++++++++++++----- README.md | 2 +- 9 files changed, 46 insertions(+), 31 deletions(-) create mode 100644 .github/actions/file/src/groupBy.ts diff --git a/.github/actions/file/src/generateIssueBody.ts b/.github/actions/file/src/generateIssueBody.ts index 27746d00..56299368 100644 --- a/.github/actions/file/src/generateIssueBody.ts +++ b/.github/actions/file/src/generateIssueBody.ts @@ -1,7 +1,7 @@ import type {Finding} from './types.d.js' -export function generateIssueBody(findingOrFindings: Finding | Finding[], screenshotRepo: string): string { - const findings = Array.isArray(findingOrFindings) ? findingOrFindings : [findingOrFindings] +export function generateIssueBody(occurrences: Finding | Finding[], screenshotRepo: string): string { + const findings = Array.isArray(occurrences) ? occurrences : [occurrences] const finding = findings[0] const solutionLong = finding.solutionLong diff --git a/.github/actions/file/src/groupBy.ts b/.github/actions/file/src/groupBy.ts new file mode 100644 index 00000000..35d2c7b5 --- /dev/null +++ b/.github/actions/file/src/groupBy.ts @@ -0,0 +1,7 @@ +export const GROUP_BY_VALUES = ['finding', 'rule', 'rule+url'] as const + +export type GroupBy = (typeof GROUP_BY_VALUES)[number] + +export function isGroupBy(value: string): value is GroupBy { + return (GROUP_BY_VALUES as readonly string[]).includes(value) +} diff --git a/.github/actions/file/src/index.ts b/.github/actions/file/src/index.ts index 6d95dfd6..c72a3812 100644 --- a/.github/actions/file/src/index.ts +++ b/.github/actions/file/src/index.ts @@ -1,12 +1,4 @@ -import type { - Finding, - ResolvedFiling, - RepeatedFiling, - FindingGroupIssue, - Filing, - IssueResponse, - GroupBy, -} from './types.d.js' +import type {Finding, ResolvedFiling, RepeatedFiling, FindingGroupIssue, Filing, IssueResponse} from './types.d.js' import fs from 'node:fs' import path from 'node:path' import process from 'node:process' @@ -21,6 +13,7 @@ import {isResolvedFiling} from './isResolvedFiling.js' import {openIssue} from './openIssue.js' import {reopenIssue} from './reopenIssue.js' import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js' +import {GROUP_BY_VALUES, isGroupBy} from './groupBy.js' import {OctokitResponse} from '@octokit/types' const OctokitWithThrottling = Octokit.plugin(throttling) @@ -38,12 +31,11 @@ export default async function () { : [] const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues') const groupByInput = core.getInput('group_by') || 'finding' - const validGroupByValues: GroupBy[] = ['finding', 'rule', 'rule+url'] - if (!validGroupByValues.includes(groupByInput as GroupBy)) { - core.setFailed(`Invalid 'group_by' value: '${groupByInput}'. Must be one of: ${validGroupByValues.join(', ')}.`) + if (!isGroupBy(groupByInput)) { + core.setFailed(`Invalid 'group_by' value: '${groupByInput}'. Must be one of: ${GROUP_BY_VALUES.join(', ')}.`) return } - const groupBy = groupByInput as GroupBy + const groupBy = groupByInput const dryRun = core.getBooleanInput('dry_run') core.debug(`Input: 'findings_file: ${findingsFile}'`) core.debug(`Input: 'repository: ${repoWithOwner}'`) diff --git a/.github/actions/file/src/reopenIssue.ts b/.github/actions/file/src/reopenIssue.ts index b37c8ca0..10705f30 100644 --- a/.github/actions/file/src/reopenIssue.ts +++ b/.github/actions/file/src/reopenIssue.ts @@ -11,7 +11,7 @@ export async function reopenIssue( screenshotRepo?: string, ) { let body: string | undefined - if (findings && findings.length > 0 && repoWithOwner) { + if (findings?.length && repoWithOwner) { body = generateIssueBody(findings, screenshotRepo ?? repoWithOwner) } diff --git a/.github/actions/file/src/types.d.ts b/.github/actions/file/src/types.d.ts index c7bf8041..ee91bc67 100644 --- a/.github/actions/file/src/types.d.ts +++ b/.github/actions/file/src/types.d.ts @@ -1,5 +1,3 @@ -export type GroupBy = 'finding' | 'rule' | 'rule+url' - export type Finding = { scannerType: string ruleId?: string diff --git a/.github/actions/file/src/updateFilingsWithNewFindings.ts b/.github/actions/file/src/updateFilingsWithNewFindings.ts index 910c3c3a..9c00be5a 100644 --- a/.github/actions/file/src/updateFilingsWithNewFindings.ts +++ b/.github/actions/file/src/updateFilingsWithNewFindings.ts @@ -1,11 +1,14 @@ -import type {Finding, ResolvedFiling, NewFiling, RepeatedFiling, Filing, GroupBy} from './types.d.js' +import type {Finding, ResolvedFiling, NewFiling, RepeatedFiling, Filing} from './types.d.js' +import type {GroupBy} from './groupBy.js' function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string { return filing.issue.url } function getFindingKey(finding: Finding, groupBy: GroupBy): string { - const rule = finding.ruleId ? `${finding.scannerType};${finding.ruleId}` : `${finding.scannerType};${finding.problemUrl}` + const rule = finding.ruleId + ? `${finding.scannerType};${finding.ruleId}` + : `${finding.scannerType};${finding.problemUrl}` switch (groupBy) { case 'rule': diff --git a/.github/actions/file/tests/generateIssueBody.test.ts b/.github/actions/file/tests/generateIssueBody.test.ts index d3ad8765..1bf0d4c2 100644 --- a/.github/actions/file/tests/generateIssueBody.test.ts +++ b/.github/actions/file/tests/generateIssueBody.test.ts @@ -80,14 +80,14 @@ describe('generateIssueBody', () => { it('omits the Occurrences section for a single finding', () => { const body = generateIssueBody(baseFinding, 'github/accessibility-scanner') - expect(body).not.toContain('## Occurrences') + expect(body).not.toContain('Other Occurrences') }) it('renders an Occurrences checklist when given multiple findings', () => { const second = {...baseFinding, url: 'https://example.com/other', html: 'Link'} const body = generateIssueBody([baseFinding, second], 'github/accessibility-scanner') - expect(body).toContain('## Occurrences (2)') + expect(body).toContain('## 2 Other Occurrences:') expect(body).toContain(`- [ ] \`${baseFinding.html}\` on ${baseFinding.url}`) expect(body).toContain(`- [ ] \`${second.html}\` on ${second.url}`) }) diff --git a/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts b/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts index 336ba504..008e186b 100644 --- a/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts +++ b/.github/actions/file/tests/updateFilingsWithNewFindings.test.ts @@ -1,7 +1,7 @@ import {describe, it, expect} from 'vitest' import {updateFilingsWithNewFindings} from '../src/updateFilingsWithNewFindings.ts' -const cc = (url: string, html: string) => ({ +const colorContrastFinding = (url: string, html: string) => ({ scannerType: 'axe', ruleId: 'color-contrast', url, @@ -13,9 +13,9 @@ const cc = (url: string, html: string) => ({ describe('updateFilingsWithNewFindings — group_by', () => { const findings = [ - cc('https://example.com/a', '1'), - cc('https://example.com/a', '2'), - cc('https://example.com/b', '3'), + colorContrastFinding('https://example.com/a', '1'), + colorContrastFinding('https://example.com/a', '2'), + colorContrastFinding('https://example.com/b', '3'), ] it("defaults to 'finding': one filing per individual violation", () => { @@ -46,7 +46,7 @@ describe('updateFilingsWithNewFindings — group_by', () => { url: 'https://github.com/org/repo/issues/1', title: 'color-contrast', }, - findings: [cc('https://example.com/a', '1')], + findings: [colorContrastFinding('https://example.com/a', '1')], }, ] const result = updateFilingsWithNewFindings(cached, findings, 'rule') @@ -58,13 +58,28 @@ describe('updateFilingsWithNewFindings — group_by', () => { it("keeps distinct rules separate under 'rule'", () => { const mixed = [ - cc('https://example.com/a', '1'), - {...cc('https://example.com/a', '

x

'), ruleId: 'heading-order'}, + colorContrastFinding('https://example.com/a', '1'), + {...colorContrastFinding('https://example.com/a', '

x

'), ruleId: 'heading-order'}, ] const result = updateFilingsWithNewFindings([], mixed, 'rule') expect(result).toHaveLength(2) }) + it("'rule': does not merge findings from different scanners that share a ruleId", () => { + const a = { + ...colorContrastFinding('https://example.com/a', '1'), + scannerType: 'axe', + ruleId: 'duplicate-id', + } + const b = { + ...colorContrastFinding('https://example.com/a', '2'), + scannerType: 'reflow', + ruleId: 'duplicate-id', + } + const result = updateFilingsWithNewFindings([], [a, b], 'rule') + expect(result).toHaveLength(2) + }) + it("'finding' (default) preserves the original 1:1 behavior with cached filings", () => { const cached = [ { @@ -74,7 +89,7 @@ describe('updateFilingsWithNewFindings — group_by', () => { url: 'https://github.com/org/repo/issues/1', title: 'color-contrast', }, - findings: [cc('https://example.com/a', '1')], + findings: [colorContrastFinding('https://example.com/a', '1')], }, ] const result = updateFilingsWithNewFindings(cached, findings) diff --git a/README.md b/README.md index 5330f799..d4ff9862 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Trigger the workflow manually or automatically based on your configuration. The | `skip_copilot_assignment` | No | Whether to skip assigning filed issues to GitHub Copilot. Set to `true` if you don't have GitHub Copilot or prefer to handle issues manually | `true` | | `include_screenshots` | No | Whether to capture screenshots of scanned pages and include links to them in filed issues. Screenshots are stored on the `gh-cache` branch of the repository running the workflow. Default: `false` | `true` | | `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | -| `group_by` | No | How to consolidate findings into issues: `finding` (default, one issue per individual violation), `rule` (one issue per rule, aggregating every occurrence across all scanned URLs), or `rule+url` (one issue per rule per scanned URL). Preferred over `open_grouped_issues` for consolidation. Default: `finding` | `rule` | +| `group_by` | No | How to consolidate findings into issues: `finding` (one issue per individual violation), `rule` (one issue per rule, aggregating every occurrence across all scanned URLs), or `rule+url` (one issue per rule per scanned URL). Preferred over `open_grouped_issues` for consolidation. Default: `finding` | `rule` | | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | | `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | From 63eef9aa25e2edffb1de5edb6823430d5fb70640 Mon Sep 17 00:00:00 2001 From: Taarik <147209483+taarikashenafi@users.noreply.github.com> Date: Wed, 24 Jun 2026 10:24:36 -0500 Subject: [PATCH 5/6] Update README.md Co-authored-by: Joyce Zhu --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d4ff9862..38ea1e85 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Trigger the workflow manually or automatically based on your configuration. The | `skip_copilot_assignment` | No | Whether to skip assigning filed issues to GitHub Copilot. Set to `true` if you don't have GitHub Copilot or prefer to handle issues manually | `true` | | `include_screenshots` | No | Whether to capture screenshots of scanned pages and include links to them in filed issues. Screenshots are stored on the `gh-cache` branch of the repository running the workflow. Default: `false` | `true` | | `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | -| `group_by` | No | How to consolidate findings into issues: `finding` (one issue per individual violation), `rule` (one issue per rule, aggregating every occurrence across all scanned URLs), or `rule+url` (one issue per rule per scanned URL). Preferred over `open_grouped_issues` for consolidation. Default: `finding` | `rule` | +| `group_by` | No | How to consolidate findings when filing issues: `finding` (one issue per individual violation), `rule` (one issue per rule, aggregating every occurrence across all scanned URLs), or `rule+url` (one issue per rule per scanned URL). Preferred over `open_grouped_issues` for consolidation. Default: `finding` | | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | | `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | From f9a77a46d896dd3f1d3ac5ddc09afee4e1a6b5ff Mon Sep 17 00:00:00 2001 From: taarikashenafi Date: Thu, 25 Jun 2026 18:26:21 -0500 Subject: [PATCH 6/6] Fix openIssue array signature and category tests after merge --- .github/actions/file/src/openIssue.ts | 4 ++-- .github/actions/file/tests/openIssue.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/actions/file/src/openIssue.ts b/.github/actions/file/src/openIssue.ts index ed74a8f8..67128d08 100644 --- a/.github/actions/file/src/openIssue.ts +++ b/.github/actions/file/src/openIssue.ts @@ -28,8 +28,8 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding labels.push(`${primary.scannerType} rule: ${primary.ruleId}`) } // Flag non-WCAG findings so they can be filtered or triaged separately - if (finding.category && finding.category !== 'wcag') { - labels.push(finding.category) + if (primary.category && primary.category !== 'wcag') { + labels.push(primary.category) } const count = findings.length diff --git a/.github/actions/file/tests/openIssue.test.ts b/.github/actions/file/tests/openIssue.test.ts index a376666f..890f8438 100644 --- a/.github/actions/file/tests/openIssue.test.ts +++ b/.github/actions/file/tests/openIssue.test.ts @@ -67,7 +67,7 @@ describe('openIssue', () => { it('adds a category label for non-WCAG findings', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/repo', {...baseFinding, category: 'best-practice'}) + await openIssue(octokit, 'org/repo', [{...baseFinding, category: 'best-practice'}]) expect(octokit.request).toHaveBeenCalledWith( expect.any(String), @@ -79,7 +79,7 @@ describe('openIssue', () => { it('does not add a category label for WCAG findings', async () => { const octokit = mockOctokit() - await openIssue(octokit, 'org/repo', {...baseFinding, category: 'wcag'}) + await openIssue(octokit, 'org/repo', [{...baseFinding, category: 'wcag'}]) const labels = octokit.request.mock.calls[0][1].labels expect(labels).not.toContain('wcag')