fix: CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits - #125
Conversation
Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/c681cf11-2fe2-4641-ac1b-86c6312c6774 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
|
@copilot Please comprehensively fix: Root cause: this is a CodeQL configuration conflict, not a code defect. The job failed with:
So your repo currently has GitHub Code Scanning default setup enabled and this PR/run is also uploading results from an advanced CodeQL workflow ( Solution (pick one approach)Option A (recommended): Keep advanced workflow, disable default setup
Option B: Keep default setup, remove/disable advanced workflowIf you prefer GitHub-managed setup:
Code suggestion (if choosing Option B temporarily)Disable the workflow job by adding a false condition: # .github/workflows/codeql.yml
jobs:
analyze:
if: ${{ false }} # temporary disable to avoid conflict with default setup(or rename/delete the workflow file). Why this fixes itThe logs show scan queries executed and SARIF upload attempted, but processing failed at ingestion due to setup mode conflict. Once only one mode is active (default or advanced), SARIF processing succeeds. |
… setup Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/ac715a75-46d5-4cba-8bbd-16ce61f122e8 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
... Removed |
There was a problem hiding this comment.
Pull request overview
This PR hardens and modernizes the repo’s CI/CD and security posture by adding SAST (CodeQL + a targeted insecure RNG scan), improving Playwright E2E throughput via sharding and report merging, introducing automated accessibility checks, and adjusting Angular bundle budgets. It also removes Math.random() usage from the randomization domain in favor of Web Crypto APIs.
Changes:
- Add Playwright sharding (4-way) with blob report upload + merged HTML report artifact.
- Add CodeQL workflow and a CI job to block
Math.random()usage in the randomization-engine domain; replace existingMath.random()usages withcrypto.*. - Add axe-core Playwright accessibility utility + WCAG 2.1 AA E2E tests; adjust Playwright reporter behavior for CI vs local.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests_e2e/a11y.ts |
Adds a reusable axe-core audit helper used by E2E tests. |
tests_e2e/a11y.spec.ts |
Introduces WCAG 2.1 AA accessibility smoke tests across key routes and a generated-results flow. |
src/environments/version.ts |
Bumps generated app version constant to v1.25.0. |
src/app/domain/randomization-engine/randomization-engine.facade.ts |
Replaces request IDs previously derived from Math.random() with crypto.randomUUID(). |
src/app/domain/randomization-engine/core/randomization-algorithm.ts |
Replaces default seed generation from Math.random() to a Web Crypto seed. |
playwright.config.ts |
Uses blob reporter in CI (for sharded merging) and html locally. |
package.json |
Adds @axe-core/playwright dev dependency. |
package-lock.json |
Locks @axe-core/playwright and axe-core dependency tree. |
angular.json |
Updates initial bundle size budgets (warn/error thresholds). |
.github/workflows/codeql.yml |
Adds CodeQL security-and-quality scanning on PRs and main. |
.github/workflows/ci.yml |
Updates CI permissions, upgrades actions, adds Playwright sharding + merge job, and adds a Math.random() security scan job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot apply changes based on the comments in this thread |
…adiness, shared generateCryptoSeed Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/1f3c1502-f453-400b-85c7-dc828abff038 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Applied all review feedback in commit
The |
|
@copilot Please fix comprehensively: Job 71352105013 is failing due to axe-core accessibility violations in the Playwright a11y E2E tests, specifically:
Fix (recommended): add an accessible name to the offending button(s)The What to change in the app codeFind the button(s) on Option A:
|
Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/5f8c3c41-e32c-45a0-a11e-2408ef7751b3 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
|
@copilot Please fix comprehensively. Use this fix path for job 71482230009 (ref What is failingAll failures are the same deterministic axe rule:
Because it fails across multiple pages, this is almost certainly a shared theme/token contrast issue (not per-page markup). Solution to implementUpdate global text/surface color tokens so all default, secondary, muted, and helper text meet WCAG AA on light backgrounds. Suggested token changes:root {
--bg-default: #ffffff;
--bg-surface: #f8fafc;
--text-primary: #0f172a; /* very high contrast */
--text-secondary: #334155; /* safe secondary on white/light bg */
--text-muted: #475569; /* avoid low-contrast gray */
--border-subtle: #cbd5e1;
--link: #1d4ed8; /* accessible on white */
}Suggested component-level patchingIf you currently style “muted/help/meta” text with a light gray, replace it: .help-text,
.meta-text,
.table-subtext,
.placeholder-like-label {
color: var(--text-muted);
}For results grid/table specifically (one of the failing tests), ensure body text doesn’t inherit low-contrast color: .results-grid td,
.results-grid th,
.results-grid .cell-text {
color: var(--text-primary);
}
.results-grid .cell-secondary {
color: var(--text-secondary);
}Why this should clear CIThe same Verify before pushingnpx playwright test tests_e2e/a11y.spec.ts --project=chromiumIf one violation remains, inspect the generated artifact referenced in logs:
and patch the specific selector reported there. |
…requirements Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/7807ab0a-1c8c-49af-a649-251639fa4575 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
Fixed in commit Changes across 11 files:
All changes are purely Tailwind class upgrades — no logic touched. |
|
@copilot Please comprehensively fix: Root cause: the job is failing on Playwright a11y tests due to repeated WCAG 2.1 AA color-contrast violations (severity: serious), not flakiness. Failing tests from job
Failure is thrown in:
Ref for this job: Solution (what to change)1) Fix low-contrast design tokens/styles used across pagesBecause all four pages fail with the same rule, this is likely a shared text/button/link/input color token issue. Update your theme/CSS variables to meet at least 4.5:1 for normal text (and 3:1 for large text/UI components where applicable). Use a safer baseline like: :root {
--bg: #ffffff;
--text: #1f2937; /* strong dark gray on white */
--muted-text: #374151; /* avoid very light grays for body text */
--link: #1d4ed8; /* accessible blue on white */
--border: #6b7280; /* visible form boundaries */
--btn-primary-bg: #1d4ed8;
--btn-primary-text: #ffffff;
}If you currently use light gray text like 2) Prioritize these components (common contrast offenders)
Example fixes: .text-muted { color: #4b5563; } /* was too light */
input::placeholder { color: #6b7280; } /* avoid faint placeholder */
.btn-secondary {
background: #e5e7eb;
color: #111827;
border-color: #6b7280;
}3) Re-run and inspect exact nodes from Playwright artifactOpen the uploaded artifact path from the run:
4) Keep strict a11y gate (don’t weaken test)Do not suppress the If you want, I can next help you map likely files to edit (e.g., |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…put for WCAG AA Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/6f611155-06fc-4eeb-a90d-82f54004a772 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
Fixed in commit
Also proactively upgraded the same |
|
@copilot Please comprehensively fix. Cause of failure (job 71504004287) is clear from the logs:
So this is not a flaky infra issue—the About page has invalid What to fixOn the About page, make sure every definition list follows valid structure:
Recommended code changeIf current markup is like this (common failing pattern): <dl>
<div class="row">
<dt>Version</dt>
<dd>1.2.3</dd>
</div>
</dl>Change to: <dl class="about-meta">
<dt>Version</dt>
<dd>1.2.3</dd>
<dt>Build date</dt>
<dd>2026-04-15</dd>
</dl>If you need styling hooks, target Where to inspect
Quick validationAfter patching:
|
…ist a11y violation Agent-Logs-Url: https://github.com/fderuiter/Clinical-Randomization-Generator/sessions/72751eb0-65c3-4099-9aa1-3ff1bfd83f75 Co-authored-by: fderuiter <127706008+fderuiter@users.noreply.github.com>
Fixed in commit Changed from: <div class="flex items-start gap-3 ...">
<span class="bullet-dot"></span> <!-- non-dt/dd child → violation -->
<div>
<span>{{ tech.name }}</span> <!-- non-dt/dd child → violation -->
<span>{{ tech.role }}</span>
</div>
</div>To: <div class="rounded-lg ...">
<dt class="...">{{ tech.name }}</dt>
<dd class="...">{{ tech.role }}</dd>
</div>The feature cards |
|
🎉 This PR is included in version 1.28.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Scales the CI/CD pipeline for a growing clinical-grade test suite: adds parallel E2E sharding, CodeQL SAST, tighter bundle budgets, and automated WCAG 2.1 AA accessibility gates.
Playwright Sharding (Ticket 20.A)
ci.ymlsplit intobuild_and_test→e2e(4-shard matrix) →merge_reports→security_scanblob-report-{N}artifact;merge_reportsmerges all into a single HTML report vianpx playwright merge-reportsplaywright.config.ts: reporter set to[['blob'], ['line']]in CI,'html'locallyCodeQL SAST (Ticket 20.B)
.github/workflows/codeql.ymlrunning CodeQLsecurity-and-qualityqueries on JS/TS on every push/PRsecurity_scanCI job hard-fails ifMath.random()appears anywhere undersrc/app/domain/randomization-engine/randomization-algorithm.ts→crypto.getRandomValues(new Uint32Array(4))randomization-engine.facade.ts→crypto.randomUUID()Bundle Budgets (Ticket 20.C)
Tightened
angular.jsonproduction budgets to enforce ~baseline + 100 KB headroom:{ "type": "initial", "maximumWarning": "1.85MB", "maximumError": "1.93MB" }(Previously: warning
1.5MB/ error2.5MB)Accessibility Audits (Ticket 20.D)
@axe-core/playwrightdev dependencytests_e2e/a11y.ts:checkA11y(page)runs axe withwcag2a/aa/wcag21a/aatags, throws on anycriticalorseriousviolation with offending element + WCAG rule in the error messagetests_e2e/a11y.spec.ts: 4 E2E tests covering landing page, about page, generator wizard, and post-generation results gridPermissions hardening
All CI jobs now carry explicit
permissions: contents: readto satisfy least-privilege GITHUB_TOKEN requirements (flagged by CodeQL actions scan).