Skip to content

Add WCAG 2.1 AA accessibility gate for test-app routes (GSA/sam-ui-elements#708) - #2

Closed
erikrozi wants to merge 2 commits into
masterfrom
devin/1792635793-wcag-a11y-gate
Closed

erikrozi wants to merge 2 commits into
masterfrom
devin/1792635793-wcag-a11y-gate

Conversation

@erikrozi

@erikrozi erikrozi commented Oct 22, 2026 •

Copy link
Copy Markdown

Task prompt

Repository: COG-GTM/sam-ui-elements (a fork of GSA/sam-ui-elements; Angular 21, Node 24; has a Playwright smoke test in .github/workflows/e2e.yml and a test-app).

Implement upstream issue GSA/sam-ui-elements#708: add a WCAG 2.1 AA accessibility gate. Mirror the proven pattern in GSA/sam-styles and GSA/ngx-uswds (an axe-core-backed Playwright config, `playwright.a11y.config.{mjs,ts}`, run by `npm run test:a11y`, with a committed violations baseline so the gate ratchets rather than red-walling day one). Read those two public repos for the exact config, baseline file format, and CI step, and read GSA/angular-upgrade-dashboard (dashboard/ source) to see which committed files it reads to report "WCAG 2.1 AA enforced" so this repo publishes the same signal.

Steps: add the axe-core and Playwright dev dependencies the pattern needs (pin to versions already used by sam-styles/ngx-uswds), add the a11y config and spec that scans the test-app pages or stories, run it, commit the baseline of current violations, add the CI workflow step, and document the gate in CONTRIBUTING.md or README. Run the full gate locally and confirm it passes against the committed baseline.

Headline number: number of distinct WCAG 2.1 AA violations found in the baseline (by rule id, with counts), and how many pages/stories are scanned. Do not fix the violations in this PR; list them in the "Decisions for the repository owners" section grouped by rule.

Description

Adds the WCAG 2.1 AA accessibility gate requested in upstream GSA#708, mirroring the playwright.a11y.config.{mjs,ts} pattern already running in GSA/sam-styles (tests/accessibility/storybook-a11y.spec.mjs) and GSA/ngx-uswds (tests/accessibility/storybook-a11y.spec.ts):

  • test-app/playwright.a11y.config.ts extends the existing test-app/playwright.config.ts (same ng serve webServer), points at test-app/e2e/accessibility/, and runs serially with one worker and a 600 s timeout, exactly as the two reference configs do.
  • test-app/e2e/accessibility/test-app-a11y.spec.ts uses @axe-core/playwright with the wcag2a/wcag2aa/wcag21a/wcag21aa tags. This repo has no Storybook, so instead of walking index.json it reads the route table from test-app/src/app/app.module.ts and scans every declared route (the gallery-route convention from AGENTS.md). Violation fingerprints are <route> | <rule id> | <selector> with Angular's ng-tns-*/_ngcontent-*/_nghost-* noise stripped (taken from the ngx-uswds spec), diffed against the committed baseline. New violations fail unconditionally, including in update mode; resolved violations also fail until removed from the baseline with UPDATE_A11Y_BASELINE=1 npm run test:a11y, so the baseline can only shrink. The smoke config (test-app/playwright.config.ts) ignores e2e/accessibility/** so test:e2e does not re-run the scan under its 30 s budget. A sanity test proves axe reports a known contrast failure so a green run can never mean "axe did nothing".
  • test-app/e2e/accessibility/wcag-2.1-aa-baseline.json — the committed baseline of current violations (empty; see headline number).
  • npm run test:a11y at the root (delegates to test-app) and in test-app/package.json; the dashboard's a11y cell is a declared boolean that its maintainers set "during config review" from the presence of a playwright.a11y.config.* plus a test:a11y script (dashboard/lib.mjs resolveA11y), so both are now present at the same names the other two repos use.
  • .github/workflows/e2e.yml runs the gate right after the smoke test, as ci.yaml in ngx-uswds and test.yml in sam-styles do.
  • Docs: AGENTS.md (Testing + CI workflows), CONTRIBUTING.md reviewer checklist, test-app/README.md.

Dependency: @axe-core/playwright 4.13.0 (the exact pin ngx-uswds uses; sam-styles uses ^4.13.0), which brings axe-core 4.13.0. @playwright/test stays at the already-installed 1.63.0 (the same version ngx-uswds pins). The lockfile diff is limited to those two entries.

Motivation and Context

Upstream issue: GSA#708 (sub-issue of the pipeline modernization epic GSA#562). The angular-upgrade-dashboard quality grid currently reports this library's accessibility gate as not enforced because only a Playwright smoke test exists.

Type of Change (Select One and Apply Label)

  • Bug fix (non-breaking change which fixes an issue) → Apply bugfix label
  • New feature (non-breaking change which adds functionality) → Apply enhancement label
  • Breaking change (fix or feature that would cause existing functionality to change) → Apply breaking label
  • Documentation / configuration update → Apply maintenance label

Headline number

Distinct WCAG 2.1 AA violations in the committed baseline: 0 rule ids, 0 fingerprints. Routes scanned: 0 -> 3 (/, /datepicker, /tabs). Per route axe reported: / 13 rules passed / 0 violations / 0 incomplete; /datepicker 19 / 0 / 0; /tabs 21 / 0 / 0.

Before: no axe run anywhere in the repo, no test:a11y script, no a11y config (dashboard: not enforced). After: three routes scanned on every PR against a committed baseline (dashboard pattern: WCAG 2.1 AA enforced).

Evidence

  • Gate config — test-app/playwright.a11y.config.ts:11-14 (testDir: "./e2e/accessibility", timeout: 600_000, fullyParallel: false, workers: 1), same shape as sam-styles playwright.a11y.config.mjs and ngx-uswds playwright.a11y.config.ts.
  • Rule set — test-app/e2e/accessibility/test-app-a11y.spec.ts:17 (WCAG_21_AA_TAGS).
  • Route discovery from app.module.ts — test-app/e2e/accessibility/test-app-a11y.spec.ts:41-48; asserted non-empty at :93-96.
  • Angular selector normalization — test-app/e2e/accessibility/test-app-a11y.spec.ts:55-62; fingerprint format :64-72.
  • Axe sanity test — test-app/e2e/accessibility/test-app-a11y.spec.ts:75-87.
  • Render wait before scanning — test-app/e2e/accessibility/test-app-a11y.spec.ts:104-108 (app-root h1 visible; every gallery route has one).
  • Baseline read/compare and the two ratchet assertions — test-app/e2e/accessibility/test-app-a11y.spec.ts:123-148; new-violation assertion runs before the update-mode write (:135-143), so UPDATE_A11Y_BASELINE=1 can only drop resolved entries.
  • Smoke config excludes the a11y spec — test-app/playwright.config.ts:6 (testIgnore: "**/accessibility/**"); the a11y config clears it at test-app/playwright.a11y.config.ts:12. npx playwright test --list: smoke = 7 tests in 3 files, a11y = 2 tests in 1 file.
  • Baseline file — test-app/e2e/accessibility/wcag-2.1-aa-baseline.json:1 ([]).
  • Scripts — package.json:35 (test:a11y → npm --prefix test-app run test:a11y), test-app/package.json:12 (playwright test --config=playwright.a11y.config.ts).
  • Dependency pin — test-app/package.json:52, test-app/package-lock.json:44,1332-1344,9684-9693.
  • CI step — .github/workflows/e2e.yml:38-40 ("Enforce WCAG 2.1 AA on test-app routes").
  • Docs — AGENTS.md:26, AGENTS.md:87, CONTRIBUTING.md:16-19, test-app/README.md:25-27.
  • Negative test (not committed): appended <input type="text" id="a11y-probe" /> and a #aaa-on-white paragraph to test-app/src/app/home/home.component.html, ran npm run test:a11y, got New WCAG 2.1 A/AA violations ... ["/ | color-contrast | p", "/ | label | #a11y-probe"] and exit code 1; reverted the file. Repeated with UPDATE_A11Y_BASELINE=1 npm run test:a11y: still 1 failed on / | label | #a11y-probe and the baseline stayed [].

How to Test

  1. npm ci && npm ci --prefix test-app && (cd test-app && npx playwright install --with-deps chromium)
  2. npm run test:a11y
  3. Optional: add an unlabeled <input> to test-app/src/app/home/home.component.html and rerun — the gate must fail with / | label | ... listed as a new violation.

Expected result: 2 passed — the axe sanity test and the route scan, with zero new and zero resolved violations against test-app/e2e/accessibility/wcag-2.1-aa-baseline.json.

Commands run and results

UPDATE_A11Y_BASELINE=1 npm run test:a11y   # generated baseline -> [] ; 2 passed (10.1s)
npm run test:a11y                          # 2 passed (2.9s) against the committed baseline
npm run format:check                       # All matched files use Prettier code style!
npm run lint:baseline                      # root: 44 warnings (baseline 44), 0 errors
npm --prefix test-app run lint:baseline    # test-app: 0 warnings (baseline 0), 0 errors
npm --prefix test-app test                 # 176 files, 1936 tests passed; lines 95.80%
npm run coverage:check                     # Coverage gate passed (lines 95.80% >= floor 95.67%)
cd test-app && npm run build               # Output location: test-app/dist
# after review fixes (d49ce3a):
npm run test:a11y                          # 2 passed (2.4s)
UPDATE_A11Y_BASELINE=1 npm run test:a11y   # with injected unlabeled input: 1 failed (new violation), baseline unchanged []
npx playwright test --list                 # smoke: 7 tests in 3 files (a11y spec excluded)
npm run format:check                       # All matched files use Prettier code style!

Screenshots (if appropriate)

N/A — no UI change; the gate runs headless in Playwright.

Decisions for the repository owners

  • The failing DAST (medium/high gate) check is pre-existing on master, not caused by this PR. ZAP flags Vulnerable JS Library [10003] for moment.js 2.30.1 (CVE-2026-17495, GHSA-4p3w-j4w9-5jqw, published 2026-09-29, patched in 2.31.0) in the production main-*.js. Root cause: test-app/package-lock.json has moment 2.31.0 (bumped in Bump the patch group across 1 directory with 14 updates GSA/sam-ui-elements#765), but library source under root src/ resolves moment from the root node_modules, and the root package-lock.json still pins 2.30.1; moment is not in test-app/dedupe-packages.ts, so the root copy is what gets bundled. A workflow_dispatch of security.yml on unmodified master fails with the same alert: https://github.com/COG-GTM/sam-ui-elements/actions/runs/39713948937. This PR adds no runtime dependency (git diff --stat origin/master touches no test-app/src, angular.json, or runtime deps). Fix in a separate PR: bump moment to 2.31.0 in the root lockfile (and/or add moment to dedupe-packages.ts so the test-app copy wins).

  • No violations to triage. The three current test-app routes produce zero WCAG 2.1 A/AA violations under axe-core 4.13.0, so the baseline is [] and there is nothing to list by rule. The ratchet is therefore already at its floor: any future violation fails the gate outright.

  • Scan surface is only what test-app renders. This repo has no Storybook; the gate covers the home page, sam-datepicker-v2, and sam-tabs-next (the gallery routes from Fix inactive tab content rendering on top of active tab in sam-tabs-next GSA/sam-ui-elements#665/Fix sam-datepicker-v2 outside-click close and the tabindex/aria-hidden restore leak GSA/sam-ui-elements#666). The ~190 other library components are unscanned until they get a gallery route. Decide whether to (a) add gallery routes component by component as they are touched, which the spec picks up automatically, or (b) add a dedicated a11y gallery route that renders many components at once.

  • Static states only. Each route is scanned once on load; the datepicker calendar popup, tab switches, and other interactive states are not scanned. ngx-uswds and sam-styles have the same limitation.

  • Rule set. The gate uses the four WCAG 2.0/2.1 A/AA axe tags only (best-practice rules such as heading-order and region are excluded), matching both reference repos. Widening to wcag22aa or best-practice would be a separate decision.

  • Dashboard flag. angular-upgrade-dashboard's a11y cell is a hand-maintained boolean in dashboard/lib.mjs (a11y: false // no axe/WCAG gate yet for sam-ui-elements); once this lands upstream, that line needs flipping in that repo.

  • Branch naming. The PR template asks for gh-<number>-<slug>; this branch is devin/1792635793-wcag-a11y-gate.

Checklist

  • Branch name follows convention (e.g. gh-<number>-<slug>)
  • PR title starts with a verb in the imperative mood
  • I have self-reviewed my own code
  • format:check passes (npm run format:check)
  • lint passes (npm run lint)
  • build passes (cd test-app && npm run build)
  • Tests pass and coverage is reported (cd test-app && npm test)
  • If this change requires a documentation update, I have updated it accordingly
  • If there are dependent changes, they have been merged and published in downstream modules

Link to Devin session: https://devin-gtm.devinenterprise.com/sessions/b254a19e762f4c6d88c91a92c3b940db
Open in Devin Desktop: https://devin-gtm.devinenterprise.com/desktop/session/b254a19e762f4c6d88c91a92c3b940db?variant=devin
Requested by: @erikrozi


Devin Review

Status Commit
⚪ Not started —

Run Devin Review

Devin Review (Staging)


Devin Review

Mirror the playwright.a11y.config pattern from GSA/sam-styles and
GSA/ngx-uswds: an axe-core-backed Playwright spec scans every route
declared in test-app/src/app/app.module.ts with the wcag2a/wcag2aa/
wcag21a/wcag21aa tags and diffs violation fingerprints against a
committed baseline (currently empty). New violations fail CI; resolved
violations must be removed from the baseline. Runs as npm run test:a11y
and as a step in e2e.yml.

Co-Authored-By: Erik Rozi <erik.rozi@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from Devin Bot

Repository: COG-GTM/sam-ui-elements (a fork of GSA/sam-ui-elements; Angular 21, Node 24; has a Playwright smoke test in .github/workflows/e2e.yml and a test-app).

Implement upstream issue GSA/sam-ui-elements#708: add a WCAG 2.1 AA accessibility gate. Mirror the proven pattern in GSA/sam-styles and GSA/ngx-uswds (an axe-core-backed Playwright config, playwright.a11y.config.{mjs,ts}, run by npm run test:a11y, with a committed violations baseline so the gate ratchets rather than red-walling day one). Read those two public repos for the exact config, baseline file format, and CI step, and read GSA/angular-upgrade-dashboard (dashboard/ source) to see which committed files it reads to report "WCAG 2.1 AA enforced" so this repo publishes the same signal.

Steps: add the axe-core and Playwright dev dependencies the pattern needs (pin to versions already used by sam-styles/ngx-uswds), add the a11y config and spec that scans the test-app pages or stories, run it, commit the baseline of current violations, add the CI workflow step, and document the gate in CONTRIBUTING.md or README. Run the full gate locally and confirm it passes against the committed baseline.

Headline number: number of distinct WCAG 2.1 AA violations found in the baseline (by rule id, with counts), and how many pages/stories are scanned. Do not fix the violations in this PR; list them in the "Decisions for the repository owners" section grouped by rule.

Ground rules for this task:

  • Work only in the fork named above (the COG-GTM org). Never open a PR, issue, or comment against the upstream GSA/* repository, and never push to the fork's default branch. Create a branch, commit, and open a PR against the fork's default branch. Do not merge.
  • The PR description must contain this entire task text verbatim under a "#``# Task prompt" heading (fenced code block), then: a short summary, a "#``# Evidence" section with file:line references for every behavior you changed or verified, one headline number (before -> a... (1082 chars truncated...)

devin-ai-integration[bot]

This comment was marked as resolved.

…rom smoke config

- UPDATE_A11Y_BASELINE=1 now only drops resolved fingerprints; new violations
  still fail, so the committed baseline can only shrink.
- playwright.config.ts ignores e2e/accessibility/** so test:e2e no longer runs
  the route scan under its 30 s timeout; playwright.a11y.config.ts clears the
  ignore for the dedicated run.

Co-Authored-By: Erik Rozi <erik.rozi@cognition.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant