Skip to content

Commit f7a85f5

Browse files
Add group_by option to the accessibility scanner (#239)
Tracking issue with context: github/accessibility#10753 Based on feedback from `@joseinthearena`: with many open scanner issues across a handful of rules (e.g. color-contrast ×11, heading-order ×6) there was no way to consolidate. The `gh-cache` branch maps each finding's fingerprint → issue URL 1:1, so re-routing N findings to a single "umbrella" issue meant editing `gh-cache` JSON by hand — brittle, and a typo silently breaks dedup. This adds a `group_by` input that controls how findings are consolidated into issues: `finding` (default, current behavior — one issue per violation), `rule` (one issue per rule), or `rule+url` (one issue per rule per scanned URL). When grouping, each duplicate occurrence is appended to the single issue as a checklist item rather than spawning a new issue. Because the default is `finding`, existing workflows behave exactly as before. ## Changes - **`action.yml` (root)** — New `group_by` input (default `finding`); forwarded to the `file` step - **`.github/actions/file/action.yml`** — New `group_by` input declared (default `finding`) - **`file/src/types.d.ts`** — New `GroupBy` type (`'finding' | 'rule' | 'rule+url'`) - **`file/src/updateFilingsWithNewFindings.ts`** — The 1:1 dedup behavior lived entirely in `getFindingKey`; it's now group-aware, computing a coarser key for `rule` / `rule+url` so multiple findings collapse into a single filing carrying multiple findings (a shape the pipeline already supported). New findings in the same group are also de-duplicated into one new filing - **`file/src/index.ts`** — Reads and validates `group_by` (fails fast via `setFailed` on an invalid value), threads it into `updateFilingsWithNewFindings`, and passes each filing's full findings array to the open/reopen helpers - **`file/src/generateIssueBody.ts` / `openIssue.ts` / `reopenIssue.ts`** — When a filing groups multiple findings, each occurrence is rendered as a checklist item under an **Occurrences** section, and the issue title reflects the occurrence count; single-finding output is unchanged - **`README.md`** — `group_by` input documented in the inputs table and getting-started example - **`.github/actions/file/README.md`** — `group_by` input documented ### Test Updates - Added `file/tests/updateFilingsWithNewFindings.test.ts` — asserts `finding` yields one filing per violation (default), `rule` collapses all occurrences of a rule into a single filing, `rule+url` yields one filing per rule per URL, new occurrences attach to an existing cached filing instead of opening a new issue, and distinct rules stay separate - Updated `file/tests/openIssue.test.ts`, `generateIssueBody.test.ts`, and `reopenIssue.test.ts` for the findings-array signature and the **Occurrences** checklist rendering - Updated `file/tests/dryRun.test.ts` to cover grouped dry-run behavior and invalid-`group_by` handling ## Usage ```yaml - uses: github/accessibility-scanner@v3 with: urls: | https://example.com repository: owner/repo token: ${{ secrets.GH_TOKEN }} cache_key: cached_results.json group_by: rule # 'finding' (default, one issue per violation), 'rule' (one per rule), or 'rule+url' (one per rule per scanned URL) ``` When `group_by` is not provided, the scanner behaves exactly as before (one issue per finding). ## Notes - kept the existing `open_grouped_issues` option as-is instead of removing it. It's similar but not the same (it still files every issue and adds a separate tracking issue, while `group_by` files one combined issue). Removing it would be a breaking change, so I left it for a follow-up — happy to go either way. - For a grouped issue, the title shows the occurrence count, e.g. `… (3 occurrences)`, since the findings can span multiple URLs.
2 parents 8d2bbf0 + f9a77a4 commit f7a85f5

15 files changed

Lines changed: 307 additions & 65 deletions

.github/actions/file/README.md

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Files GitHub issues to track potential accessibility gaps.
1111
**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`.
1212

1313
The file should contain a JSON array of finding objects. For example:
14+
1415
```json
1516
[]
1617
```
@@ -28,27 +29,49 @@ The file should contain a JSON array of finding objects. For example:
2829
**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`.
2930

3031
The file should contain a JSON array of filing objects. For example:
32+
3133
```json
3234
[
3335
{
3436
"findings": [],
35-
"issue": {"id":1,"nodeId":"SXNzdWU6MQ==","url":"https://github.com/github/docs/issues/123","title":"Accessibility issue: 1"}
37+
"issue": {
38+
"id": 1,
39+
"nodeId": "SXNzdWU6MQ==",
40+
"url": "https://github.com/github/docs/issues/123",
41+
"title": "Accessibility issue: 1"
42+
}
3643
}
3744
]
3845
```
3946

47+
#### `group_by`
48+
49+
**Optional** How to consolidate findings into issues. One of:
50+
51+
- `finding` (default): one issue per individual violation — current behavior, unchanged.
52+
- `rule`: one issue per rule (`ruleId`/`scannerType`), aggregating every occurrence across all scanned URLs.
53+
- `rule+url`: one issue per rule per scanned URL.
54+
55+
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`.
56+
4057
### Outputs
4158

4259
#### `filings_file`
4360

4461
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-<uuid>.json`.
4562

4663
The file will contain a JSON array of filing objects. For example:
64+
4765
```json
4866
[
4967
{
5068
"findings": [],
51-
"issue": {"id":1,"nodeId":"SXNzdWU6MQ==","url":"https://github.com/github/docs/issues/123","title":"Accessibility issue: 1"}
69+
"issue": {
70+
"id": 1,
71+
"nodeId": "SXNzdWU6MQ==",
72+
"url": "https://github.com/github/docs/issues/123",
73+
"title": "Accessibility issue: 1"
74+
}
5275
}
5376
]
5477
```

.github/actions/file/action.yml

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,54 @@
1-
name: "File"
2-
description: "Files GitHub issues to track potential accessibility gaps."
1+
name: 'File'
2+
description: 'Files GitHub issues to track potential accessibility gaps.'
33

44
inputs:
55
findings_file:
6-
description: "Path to a JSON file containing the list of potential accessibility gaps"
6+
description: 'Path to a JSON file containing the list of potential accessibility gaps'
77
required: true
88
repository:
9-
description: "Repository (with owner) to file issues in"
9+
description: 'Repository (with owner) to file issues in'
1010
required: true
1111
token:
1212
description: "Token with fine-grained permission 'issues: write'"
1313
required: true
1414
base_url:
15-
description: "Optional base URL to pass into Octokit for the GitHub API (for example, `https://YOUR_HOSTNAME/api/v3` for GitHub Enterprise Server)"
15+
description: 'Optional base URL to pass into Octokit for the GitHub API (for example, `https://YOUR_HOSTNAME/api/v3` for GitHub Enterprise Server)'
1616
required: false
1717
cached_filings_file:
18-
description: "Path to a JSON file containing cached filings from previous runs. Without this, duplicate issues may be filed."
18+
description: 'Path to a JSON file containing cached filings from previous runs. Without this, duplicate issues may be filed.'
1919
required: false
2020
screenshot_repository:
2121
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."
2222
required: false
2323
open_grouped_issues:
2424
description: "In the 'file' step, also open grouped issues which link to all issues with the same root cause"
2525
required: false
26-
default: "false"
26+
default: 'false'
27+
group_by:
28+
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)."
29+
required: false
30+
default: 'finding'
2731
dry_run:
28-
description: "When true, log the issues that would be filed without opening, closing, or reopening any issues."
32+
description: 'When true, log the issues that would be filed without opening, closing, or reopening any issues.'
2933
required: false
30-
default: "false"
34+
default: 'false'
3135
file_best_practice_issues:
32-
description: "File issues for best-practice findings (accessibility recommendations that are not hard WCAG failures). Disabling only suppresses new issues; existing ones are left untouched."
36+
description: 'File issues for best-practice findings (accessibility recommendations that are not hard WCAG failures). Disabling only suppresses new issues; existing ones are left untouched.'
3337
required: false
34-
default: "true"
38+
default: 'true'
3539
file_experimental_issues:
36-
description: "File issues for experimental findings (checks that are not yet stable). Disabling only suppresses new issues; existing ones are left untouched."
40+
description: 'File issues for experimental findings (checks that are not yet stable). Disabling only suppresses new issues; existing ones are left untouched.'
3741
required: false
38-
default: "true"
42+
default: 'true'
3943

4044
outputs:
4145
filings_file:
42-
description: "Path to a JSON file containing the list of issues filed (and their associated finding(s))"
46+
description: 'Path to a JSON file containing the list of issues filed (and their associated finding(s))'
4347

4448
runs:
45-
using: "node24"
46-
main: "bootstrap.js"
49+
using: 'node24'
50+
main: 'bootstrap.js'
4751

4852
branding:
49-
icon: "compass"
50-
color: "blue"
53+
icon: 'compass'
54+
color: 'blue'

.github/actions/file/src/generateIssueBody.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import type {Finding} from './types.d.js'
22

3-
export function generateIssueBody(finding: Finding, screenshotRepo: string): string {
3+
export function generateIssueBody(occurrences: Finding | Finding[], screenshotRepo: string): string {
4+
const findings = Array.isArray(occurrences) ? occurrences : [occurrences]
5+
const finding = findings[0]
6+
47
const solutionLong = finding.solutionLong
58
?.split('\n')
69
.map((line: string) =>
@@ -18,6 +21,16 @@ export function generateIssueBody(finding: Finding, screenshotRepo: string): str
1821
`
1922
}
2023

24+
let occurrencesSection = ''
25+
if (findings.length > 1) {
26+
const items = findings.map(f => `- [ ] ${f.html ? `\`${f.html}\` on ${f.url}` : f.url}`).join('\n')
27+
occurrencesSection = `
28+
## ${findings.length} Other Occurrences:
29+
30+
${items}
31+
`
32+
}
33+
2134
const categoryNotice =
2235
finding.category && finding.category !== 'wcag'
2336
? `> [!NOTE]\n> This is ${
@@ -42,7 +55,7 @@ An accessibility scan ${finding.html ? `flagged the element \`${finding.html}\``
4255
${screenshotSection ?? ''}
4356
To fix this, ${finding.solutionShort}.
4457
${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''}
45-
58+
${occurrencesSection}
4659
${acceptanceCriteria}
4760
`
4861

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export const GROUP_BY_VALUES = ['finding', 'rule', 'rule+url'] as const
2+
3+
export type GroupBy = (typeof GROUP_BY_VALUES)[number]
4+
5+
export function isGroupBy(value: string): value is GroupBy {
6+
return (GROUP_BY_VALUES as readonly string[]).includes(value)
7+
}

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {getWontfixIssueNumbers, shouldReopenIssue, WONTFIX_LABEL} from './should
1414
import {openIssue} from './openIssue.js'
1515
import {reopenIssue} from './reopenIssue.js'
1616
import {updateFilingsWithNewFindings} from './updateFilingsWithNewFindings.js'
17+
import {GROUP_BY_VALUES, isGroupBy} from './groupBy.js'
1718
import {OctokitResponse} from '@octokit/types'
1819
const OctokitWithThrottling = Octokit.plugin(throttling)
1920

@@ -36,6 +37,12 @@ export default async function () {
3637
? JSON.parse(fs.readFileSync(cachedFilingsFile, 'utf8'))
3738
: []
3839
const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues')
40+
const groupByInput = core.getInput('group_by') || 'finding'
41+
if (!isGroupBy(groupByInput)) {
42+
core.setFailed(`Invalid 'group_by' value: '${groupByInput}'. Must be one of: ${GROUP_BY_VALUES.join(', ')}.`)
43+
return
44+
}
45+
const groupBy = groupByInput
3946
const dryRun = core.getBooleanInput('dry_run')
4047
const fileBestPracticeIssues = getBooleanInputWithDefault('file_best_practice_issues', true)
4148
const fileExperimentalIssues = getBooleanInputWithDefault('file_experimental_issues', true)
@@ -45,6 +52,7 @@ export default async function () {
4552
core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`)
4653
core.debug(`Input: 'cached_filings_file: ${cachedFilingsFile}'`)
4754
core.debug(`Input: 'open_grouped_issues: ${shouldOpenGroupedIssues}'`)
55+
core.debug(`Input: 'group_by: ${groupBy}'`)
4856
core.debug(`Input: 'dry_run: ${dryRun}'`)
4957
core.debug(`Input: 'file_best_practice_issues: ${fileBestPracticeIssues}'`)
5058
core.debug(`Input: 'file_experimental_issues: ${fileExperimentalIssues}'`)
@@ -69,7 +77,7 @@ export default async function () {
6977
},
7078
},
7179
})
72-
const filings = updateFilingsWithNewFindings(cachedFilings, findings)
80+
const filings = updateFilingsWithNewFindings(cachedFilings, findings, groupBy)
7381

7482
// Suppressed new filings are kept out of the cache
7583
const suppressedFilings = new Set<Filing>()
@@ -131,7 +139,7 @@ export default async function () {
131139
filing.issue.state = 'closed'
132140
} else if (isNewFiling(filing)) {
133141
// Open a new issue for the filing
134-
response = await openIssue(octokit, repoWithOwner, filing.findings[0], screenshotRepo)
142+
response = await openIssue(octokit, repoWithOwner, filing.findings, screenshotRepo)
135143
;(filing as Filing).issue = {state: 'open'} as Issue
136144

137145
// Track for grouping
@@ -151,8 +159,8 @@ export default async function () {
151159
// The developer intentionally closed this issue and labeled it 'wontfix', so leave it closed
152160
core.info(`Skipping reopen of issue labeled '${WONTFIX_LABEL}': ${filing.issue.url}`)
153161
} else {
154-
// Reopen the filing's issue and update the body with the latest finding
155-
response = await reopenIssue(octokit, issue, filing.findings[0], repoWithOwner, screenshotRepo)
162+
// Reopen the filing's issue and update the body with the latest finding(s)
163+
response = await reopenIssue(octokit, issue, filing.findings, repoWithOwner, screenshotRepo)
156164
filing.issue.state = 'reopened'
157165
}
158166
}

.github/actions/file/src/openIssue.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,29 @@ function truncateWithEllipsis(text: string, maxLength: number): string {
1717
return text.length > maxLength ? text.slice(0, maxLength - 1) + '…' : text
1818
}
1919

20-
export async function openIssue(octokit: Octokit, repoWithOwner: string, finding: Finding, screenshotRepo?: string) {
20+
export async function openIssue(octokit: Octokit, repoWithOwner: string, findings: Finding[], screenshotRepo?: string) {
2121
const owner = repoWithOwner.split('/')[0]
2222
const repo = repoWithOwner.split('/')[1]
23+
const primary = findings[0]
2324

24-
const labels = [`${finding.scannerType}-scanning-issue`]
25+
const labels = [`${primary.scannerType}-scanning-issue`]
2526
// Only include a ruleId label when it's defined
26-
if (finding.ruleId) {
27-
labels.push(`${finding.scannerType} rule: ${finding.ruleId}`)
27+
if (primary.ruleId) {
28+
labels.push(`${primary.scannerType} rule: ${primary.ruleId}`)
2829
}
2930
// Flag non-WCAG findings so they can be filtered or triaged separately
30-
if (finding.category && finding.category !== 'wcag') {
31-
labels.push(finding.category)
31+
if (primary.category && primary.category !== 'wcag') {
32+
labels.push(primary.category)
3233
}
3334

35+
const count = findings.length
36+
const titleSuffix = count > 1 ? ` (${count} occurrences)` : ` on ${new URL(primary.url).pathname}`
3437
const title = truncateWithEllipsis(
35-
`Accessibility issue: ${finding.problemShort[0].toUpperCase() + finding.problemShort.slice(1)} on ${new URL(finding.url).pathname}`,
38+
`Accessibility issue: ${primary.problemShort[0].toUpperCase() + primary.problemShort.slice(1)}${titleSuffix}`,
3639
GITHUB_ISSUE_TITLE_MAX_LENGTH,
3740
)
3841

39-
const body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner)
42+
const body = generateIssueBody(findings, screenshotRepo ?? repoWithOwner)
4043

4144
return octokit.request(`POST /repos/${owner}/${repo}/issues`, {
4245
owner,

.github/actions/file/src/reopenIssue.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import {generateIssueBody} from './generateIssueBody.js'
66
export async function reopenIssue(
77
octokit: Octokit,
88
{owner, repository, issueNumber}: Issue,
9-
finding?: Finding,
9+
findings?: Finding[],
1010
repoWithOwner?: string,
1111
screenshotRepo?: string,
1212
) {
1313
let body: string | undefined
14-
if (finding && repoWithOwner) {
15-
body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner)
14+
if (findings?.length && repoWithOwner) {
15+
body = generateIssueBody(findings, screenshotRepo ?? repoWithOwner)
1616
}
1717

1818
return octokit.request(`PATCH /repos/${owner}/${repository}/issues/${issueNumber}`, {
Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,39 @@
11
import type {Finding, ResolvedFiling, NewFiling, RepeatedFiling, Filing} from './types.d.js'
2+
import type {GroupBy} from './groupBy.js'
23

34
function getFilingKey(filing: ResolvedFiling | RepeatedFiling): string {
45
return filing.issue.url
56
}
67

7-
function getFindingKey(finding: Finding): string {
8-
if (finding.ruleId && finding.html) {
9-
return `${finding.url};${finding.ruleId};${finding.html}`
8+
function getFindingKey(finding: Finding, groupBy: GroupBy): string {
9+
const rule = finding.ruleId
10+
? `${finding.scannerType};${finding.ruleId}`
11+
: `${finding.scannerType};${finding.problemUrl}`
12+
13+
switch (groupBy) {
14+
case 'rule':
15+
return rule
16+
case 'rule+url':
17+
return `${finding.url};${rule}`
18+
case 'finding':
19+
default:
20+
if (finding.ruleId && finding.html) {
21+
return `${finding.url};${finding.ruleId};${finding.html}`
22+
}
23+
return `${finding.url};${finding.scannerType};${finding.problemUrl}`
1024
}
11-
return `${finding.url};${finding.scannerType};${finding.problemUrl}`
1225
}
1326

1427
export function updateFilingsWithNewFindings(
1528
filings: (ResolvedFiling | RepeatedFiling)[],
1629
findings: Finding[],
30+
groupBy: GroupBy = 'finding',
1731
): Filing[] {
1832
const filingKeys: {
1933
[key: string]: ResolvedFiling | RepeatedFiling
2034
} = {}
2135
const findingKeys: {[key: string]: string} = {}
22-
const newFilings: NewFiling[] = []
36+
const newFilingKeys: {[key: string]: NewFiling} = {}
2337

2438
// Create maps for filing and finding data from previous runs, for quick lookups
2539
for (const filing of filings) {
@@ -29,21 +43,23 @@ export function updateFilingsWithNewFindings(
2943
findings: [],
3044
}
3145
for (const finding of filing.findings) {
32-
findingKeys[getFindingKey(finding)] = getFilingKey(filing)
46+
findingKeys[getFindingKey(finding, groupBy)] = getFilingKey(filing)
3347
}
3448
}
3549

3650
for (const finding of findings) {
37-
const filingKey = findingKeys[getFindingKey(finding)]
51+
const key = getFindingKey(finding, groupBy)
52+
const filingKey = findingKeys[key]
3853
if (filingKey) {
3954
// This finding already has an associated filing; add it to that filing's findings
4055
;(filingKeys[filingKey] as RepeatedFiling).findings.push(finding)
56+
} else if (newFilingKeys[key]) {
57+
newFilingKeys[key].findings.push(finding)
4158
} else {
42-
// This finding is new; create a new entry with no associated issue yet
43-
newFilings.push({findings: [finding]})
59+
newFilingKeys[key] = {findings: [finding]}
4460
}
4561
}
4662

4763
const updatedFilings = Object.values(filingKeys)
48-
return [...updatedFilings, ...newFilings]
64+
return [...updatedFilings, ...Object.values(newFilingKeys)]
4965
}

0 commit comments

Comments
 (0)