Skip to content

fix: CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits - #125

Merged
fderuiter merged 12 commits into
mainfrom
copilot/ci-cd-pipeline-evolution
Apr 15, 2026
Merged

fix: CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits#125
fderuiter merged 12 commits into
mainfrom
copilot/ci-cd-pipeline-evolution

Conversation

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

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.yml split into build_and_teste2e (4-shard matrix) → merge_reportssecurity_scan
  • Each shard uploads a blob-report-{N} artifact; merge_reports merges all into a single HTML report via npx playwright merge-reports
  • playwright.config.ts: reporter set to [['blob'], ['line']] in CI, 'html' locally

CodeQL SAST (Ticket 20.B)

  • New .github/workflows/codeql.yml running CodeQL security-and-quality queries on JS/TS on every push/PR
  • security_scan CI job hard-fails if Math.random() appears anywhere under src/app/domain/randomization-engine/
  • Fixed 3 actual violations found during scan:
    • Seed fallback in randomization-algorithm.tscrypto.getRandomValues(new Uint32Array(4))
    • Two callback correlation IDs in randomization-engine.facade.tscrypto.randomUUID()

Bundle Budgets (Ticket 20.C)

Tightened angular.json production budgets to enforce ~baseline + 100 KB headroom:

{ "type": "initial", "maximumWarning": "1.85MB", "maximumError": "1.93MB" }

(Previously: warning 1.5MB / error 2.5MB)

Accessibility Audits (Ticket 20.D)

  • Added @axe-core/playwright dev dependency
  • tests_e2e/a11y.ts: checkA11y(page) runs axe with wcag2a/aa/wcag21a/aa tags, throws on any critical or serious violation with offending element + WCAG rule in the error message
  • tests_e2e/a11y.spec.ts: 4 E2E tests covering landing page, about page, generator wizard, and post-generation results grid

Permissions hardening

All CI jobs now carry explicit permissions: contents: read to satisfy least-privilege GITHUB_TOKEN requirements (flagged by CodeQL actions scan).

@fderuiter
fderuiter requested review from Copilot and fderuiter April 14, 2026 21:10
Copilot AI changed the title [WIP] Update CI/CD pipeline for security hardening and performance budgets CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits Apr 14, 2026
@fderuiter

Copy link
Copy Markdown
Owner

@copilot Please comprehensively fix:


Root cause: this is a CodeQL configuration conflict, not a code defect.

The job failed with:

CodeQL analyses from advanced configurations cannot be processed when the default setup is enabled

So your repo currently has GitHub Code Scanning default setup enabled and this PR/run is also uploading results from an advanced CodeQL workflow (.github/workflows/codeql.yml at ref ce18ca034143cbcc076c5aa67818f1fcf2a78d5f), which GitHub rejects.

Solution (pick one approach)

Option A (recommended): Keep advanced workflow, disable default setup

  1. Go to Security → Code security and analysis → CodeQL analysis in the repo.
  2. Disable Default setup.
  3. Keep .github/workflows/codeql.yml as the single source of CodeQL runs.

Option B: Keep default setup, remove/disable advanced workflow

If you prefer GitHub-managed setup:

  • Remove or disable .github/workflows/codeql.yml, or gate it so it does not run.
  • Let default setup handle scans.

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 it

The 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.

@fderuiter fderuiter changed the title CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits fix: CI/CD Pipeline Evolution: Playwright Sharding, CodeQL SAST, Bundle Budgets & a11y Audits Apr 14, 2026

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please comprehensively fix:


...

Removed .github/workflows/codeql.yml (commit 81ce2fc) — the repo's existing default setup will continue handling CodeQL scanning without the conflict. The custom security_scan job in ci.yml remains in place as the PRNG enforcement gate for the randomization domain.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 existing Math.random() usages with crypto.*.
  • 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.

Comment thread tests_e2e/a11y.spec.ts
Comment thread tests_e2e/a11y.spec.ts Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread src/environments/version.ts
Comment thread src/app/domain/randomization-engine/core/randomization-algorithm.ts
Comment thread angular.json
@fderuiter
fderuiter marked this pull request as ready for review April 14, 2026 21:16
@fderuiter
fderuiter requested a review from Copilot April 14, 2026 21:16
@fderuiter

Copy link
Copy Markdown
Owner

@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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml

Copilot AI commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Applied all review feedback in commit 19faffb:

  • a11y.spec.ts: Replaced all waitForLoadState('networkidle') with deterministic element waits — toBeVisible() on the page heading for landing/about, and waitFor({ state: 'visible' }) on the "Complex (Multi-strata)" preset button for the generator and results-grid tests (matching the pattern used in navigation.spec.ts).
  • ci.yml: Replaced npx wait-on (not declared in package.json) with a shell curl loop (60 × 2-second retries) that exits 1 on timeout.
  • randomization-algorithm.ts: Extracted the inline seed expression into an exported generateCryptoSeed() helper function.
  • randomization-engine.worker.ts: Removed its duplicate local generateCryptoSeed() and now imports the shared one from ../core/randomization-algorithm.

The angular.json warning-threshold increase (1.5 MB → 1.85 MB) is intentional — the actual initial bundle sits near 1.75 MB, so the old warning threshold was permanently tripped. The error threshold was tightened (2.5 MB → 1.93 MB) to give only ~80 KB of headroom above the real baseline.

@fderuiter

Copy link
Copy Markdown
Owner

@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 button-name rule fails when a <button> has no discernible accessible name (no visible text, and no aria-label, aria-labelledby, or equivalent). This often happens with icon-only buttons.

What to change in the app code

Find the button(s) on /generator and/or in the results grid UI that are icon-only or otherwise unlabeled, and add one of:

Option A: aria-label (quickest for icon-only buttons)

<button type="button" aria-label="Add arm">
  <svg aria-hidden="true" ...></svg>
</button>

Option B: visible text (best when appropriate)

<button type="button">
  Add arm
</button>

Option C: aria-labelledby if the label exists elsewhere

<span id="add-arm-label" class="sr-only">Add arm</span>
<button type="button" aria-labelledby="add-arm-label">
  <svg aria-hidden="true" ...></svg>
</button>

If you’re using Angular Material (common cause)

If the button is a mat-icon-button, Angular Material supports this pattern:

<button mat-icon-button type="button" aria-label="Remove stratum">
  <mat-icon aria-hidden="true">delete</mat-icon>
</button>

Key points:

  • The icon should be aria-hidden="true" so it doesn’t become the (often empty/incorrect) accessible name.
  • The button must have the accessible name (aria-label is usually ideal for icon buttons).

Why this will resolve the CI failure

Your CI workflow runs Playwright and calls checkA11y(page) (WCAG 2.1 AA tags) from tests_e2e/a11y.spec.ts. That helper explicitly throws if any “critical” or “serious” violations exist. Once every button on those pages has a discernible accessible name, axe will stop reporting button-name as CRITICAL, and both failing tests should pass.

Relevant failing tests (ref 19faffbd8a0e09d5695208a74f3323ee076768f1):

Minimal acceptance criteria for the PR

  • All icon-only buttons in the generator wizard + results grid have one of: visible text, aria-label, or aria-labelledby.
  • Re-run CI: the two a11y tests stop reporting button-name violations and the job exits with code 0.

@fderuiter

Copy link
Copy Markdown
Owner

@copilot Please fix comprehensively.

Use this fix path for job 71482230009 (ref 95612d65d503de812e416aecbd5ffdbdf648917c):

What is failing

All failures are the same deterministic axe rule:

  • SERIOUS: color-contrast
  • Triggered in:
    • tests_e2e/a11y.spec.ts:9 (Landing)
    • tests_e2e/a11y.spec.ts:15 (About)
    • tests_e2e/a11y.spec.ts:21 (Generator)
    • tests_e2e/a11y.spec.ts:28 (Results grid)
  • Error is thrown by tests_e2e/a11y.ts:32 after violations are collected.

Because it fails across multiple pages, this is almost certainly a shared theme/token contrast issue (not per-page markup).


Solution to implement

Update 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 patching

If 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 CI

The same color-contrast violation appears on Landing, About, Generator, and Results in all retries, which indicates shared styles. Fixing global tokens and shared text classes will remove all 4 failing a11y checks in one pass.


Verify before pushing

npx playwright test tests_e2e/a11y.spec.ts --project=chromium

If one violation remains, inspect the generated artifact referenced in logs:

  • test-results/.../error-context.md
  • Playwright trace zip (attached in job)

and patch the specific selector reported there.

Copilot AI commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please fix comprehensively.

Use this fix path for job 71482230009 (ref 95612d65d503de812e416aecbd5ffdbdf648917c):
...

Fixed in commit 3656acb. The SERIOUS: color-contrast failures appeared on all 4 pages because the shared footer used text-gray-400 (≈2.5:1 contrast) and text-gray-500 (≈4.45:1, just below the 4.5:1 WCAG AA threshold for small text) on a white background.

Changes across 11 files:

  • app.ts footer (root cause for all 4 tests): text-gray-400/text-gray-500text-gray-600 (~6.5:1 on white)
  • landing.component.ts: text-indigo-500 on bg-indigo-50 (~3.75:1) → text-indigo-700 (~8.9:1)
  • config-form.component.html: all text-gray-500 labels/descriptions and text-gray-400 icon buttons → text-gray-600
  • results-grid.component.html: stratum cell values, (Size: …) suffix, *** BLINDED ***, mobile card dt labels → text-gray-600
  • generator.component.ts, tag-input, block-preview, zero-state, balance-verification, schema-analytics-dashboard: remaining text-gray-400/text-gray-500 text elements → text-gray-600

All changes are purely Tailwind class upgrades — no logic touched.

@fderuiter
fderuiter requested a review from Copilot April 15, 2026 17:21
@fderuiter

Copy link
Copy Markdown
Owner

@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 71499372180:

  • tests_e2e/a11y.spec.ts:9 Landing page
  • tests_e2e/a11y.spec.ts:15 About page
  • tests_e2e/a11y.spec.ts:21 Generator page
  • tests_e2e/a11y.spec.ts:28 Results grid

Failure is thrown in:

  • tests_e2e/a11y.ts:32 (the helper that throws when violations are found)

Ref for this job: 3656acba7d0c94345ba4f6c8ade70935353690af

Solution (what to change)

1) Fix low-contrast design tokens/styles used across pages

Because 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 #9ca3af on white, replace it for any body/help/label copy.

2) Prioritize these components (common contrast offenders)

  • Secondary/help text (small, .text-muted, placeholders)
  • Links in About/Landing cards
  • Button variants (ghost/outline/secondary)
  • Input border + placeholder + disabled states
  • Table/grid text in results screen

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 artifact

Open the uploaded artifact path from the run:

  • test-results/.../error-context.md
    This file lists exact failing nodes/selectors and computed colors. Fix those selectors first for fastest green CI.

4) Keep strict a11y gate (don’t weaken test)

Do not suppress the color-contrast rule in a11y.ts; the failures are legitimate and consistent across retries.


If you want, I can next help you map likely files to edit (e.g., globals.css, theme tokens, button/input components) once you share the stylesheet/component paths.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
Comment thread src/environments/version.ts

Copilot AI commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@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.
...

Fixed in commit cefe565. The CI log for job 71499372180 showed exactly two violations in the Results grid test:

[SERIOUS] color-contrast — four low-contrast elements on white:

  • text-emerald-600 (#059669, ~3.18:1) in block-preview.component.ts "Valid" badges and app.ts footer → upgraded to text-emerald-700 (#047857, ~4.55:1 ✓)
  • text-amber-600 (#d97706, ~3.07:1) in schema-analytics-dashboard.component.ts "(blinded)" span → upgraded to text-amber-700 (#b45309, ~4.74:1 ✓)

[CRITICAL] labelapp-tag-input inner <input> loses its placeholder when tags are present, leaving no programmatic label. Added [attr.aria-label]="placeholder" to the <input> in tag-input.component.ts so the accessible name is always set regardless of tag state.

Also proactively upgraded the same text-emerald-600/text-amber-600 classes in monte-carlo-modal.component.ts to prevent the same violations when that modal is opened.

@fderuiter

Copy link
Copy Markdown
Owner

@copilot Please comprehensively fix.

Cause of failure (job 71504004287) is clear from the logs:

  • Failing test: tests_e2e/a11y.spec.ts:15
  • Error: [SERIOUS] definition-list: Ensure <dl> elements are structured correctly
  • Thrown from: tests_e2e/a11y.ts:32
  • Commit/ref for this job: cefe565f337747399200d87dec1a5441138d0b43

So this is not a flaky infra issue—the About page has invalid <dl> markup.

What to fix

On the About page, make sure every definition list follows valid structure:

  • <dl> contains one or more pairs/groups of:
    • <dt> (term)
    • <dd> (definition)
  • No non-<dt>/<dd> elements as direct children of <dl> (except script/template).
  • Don’t wrap <dt>/<dd> in arbitrary containers directly under <dl> unless the wrapper itself is semantically valid for this rule in axe (often safest: no wrapper).

Recommended code change

If 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 dt/dd with CSS grid/flex on the <dl> itself rather than inserting <div> wrappers.

Where to inspect

  • About page component/template (likely under src/...about...).
  • The failing test location:
    • tests_e2e/a11y.spec.ts:15
    • tests_e2e/a11y.ts:32

Quick validation

After patching:

  1. Run local app and run only a11y spec:
    npx playwright test tests_e2e/a11y.spec.ts
  2. Confirm no definition-list violation on About page.
  3. Push fix; CI should pass (other tests already passed in this job).

Copilot AI commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please comprehensively fix.

Cause of failure (job 71504004287) is clear from the logs:
...

Fixed in commit f66dc26. The tech stack <dl> on the About page had <div> wrappers containing <span> elements (not <dt>/<dd>), which violates the definition-list rule. The <div> grouping element inside a <dl> must only contain <dt> and <dd> children.

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 <dl> (lines 51–67) already had the correct <div><dt> + <dd> structure, so no change was needed there.

@fderuiter
fderuiter merged commit 66644a5 into main Apr 15, 2026
10 checks passed
@fderuiter
fderuiter deleted the copilot/ci-cd-pipeline-evolution branch April 15, 2026 17:54
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.28.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI/CD Pipeline Evolution: Security Hardening, Sharding, & Performance Budgets

3 participants