ci: Add pa11y in verify pipeline #79

.pa11yci.json is in pa11y-ci 5's config lookup order (.pa11yci, .pa11yci.cjs, .pa11yci.js, .pa11yci.json), with chromeLaunchConfig correctly placed in defaults (a whole-run option that per-URL objects cannot override). WCAG2AA with the default threshold 0 is a strict, sensible baseline.
Browser wiring works: Bun blocks puppeteer's postinstall download, but PUPPETEER_EXECUTABLE_PATH exported in the accessibility step is honored by puppeteer's configuration layer (pa11y depends on full puppeteer ^25.9.0, not -core), so the system Chromium from apk add chromium is used. --no-sandbox is required for Chrome as root in the container — acceptable for CI. Note for local runs the needed variable is PUPPETEER_EXECUTABLE_PATH (a prior comment saying PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH applied to pa11y was incorrect — that variable is for the Playwright e2e suite).
Sitemap-driven discovery is the right call over the earlier hardcoded URL list: site resolves to https://martials.no on astro build because the build entry calls ensureProcessNodeEnv('production') before astro.config.ts evaluates, so loadEnv picks up .env.production; the custom sitemap-index.xml.ts route emits a urlset whose <loc> values are built with new URL(path, site.href) — no double-slash paths.
contact.astro: moving the submit button inside the <form> fixes a real functional bug — a type="submit" outside the form never submits. The honeypot restyle (display:none + aria-hidden + tabindex="-1") is harmless: display:none inputs are still submitted, so Formspree's _gotcha keeps working.
Lockfile/nix churn is coherent: bun.lock changes are confined to pa11y's dependency tree plus bun re-hoisting (root commander → 14.0.3 with svgo/commander nested; entities/undici re-hoisted with both versions retained — not a true downgrade), and the bun.nix bumps (astro 7.3.4, @astrojs/node 11.1.6, etc.) sync entries that were already stale versus package.json/bun.lock on main. Worth a line in the PR description, but not scope creep.
The a11y step now fails loudly with a diagnostic if the server never becomes ready, and includes the Node ≥ 22.13 sanity check pa11y 10 requires — addressing the earlier inline findings.

Non-blocking: see the three comments — harden the sitemap origin rewrite, add --disable-dev-shm-usage for Chrome in Docker, and document the a11y step's reliance on workspace state from the verify step.

*🤖 Review by opencode (opencode/mimo-v2.6-flash-free) — verdict: approve

# Review Summary **Verdict: approve** — no blocking issues; three non-blocking `style` comments below. **What's solid:** - `.pa11yci.json` is in pa11y-ci 5's config lookup order (`.pa11yci`, `.pa11yci.cjs`, `.pa11yci.js`, `.pa11yci.json`), with `chromeLaunchConfig` correctly placed in `defaults` (a whole-run option that per-URL objects cannot override). `WCAG2AA` with the default threshold 0 is a strict, sensible baseline. - Browser wiring works: Bun blocks puppeteer's postinstall download, but `PUPPETEER_EXECUTABLE_PATH` exported in the accessibility step is honored by puppeteer's configuration layer (pa11y depends on full `puppeteer` ^25.9.0, not `-core`), so the system Chromium from `apk add chromium` is used. `--no-sandbox` is required for Chrome as root in the container — acceptable for CI. Note for local runs the needed variable is `PUPPETEER_EXECUTABLE_PATH` (a prior comment saying `PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH` applied to pa11y was incorrect — that variable is for the Playwright e2e suite). - Sitemap-driven discovery is the right call over the earlier hardcoded URL list: `site` resolves to `https://martials.no` on `astro build` because the build entry calls `ensureProcessNodeEnv('production')` before `astro.config.ts` evaluates, so `loadEnv` picks up `.env.production`; the custom `sitemap-index.xml.ts` route emits a urlset whose `<loc>` values are built with `new URL(path, site.href)` — no double-slash paths. - `contact.astro`: moving the submit button inside the `<form>` fixes a real functional bug — a `type="submit"` outside the form never submits. The honeypot restyle (`display:none` + `aria-hidden` + `tabindex="-1"`) is harmless: `display:none` inputs are still submitted, so Formspree's `_gotcha` keeps working. - Lockfile/nix churn is coherent: `bun.lock` changes are confined to pa11y's dependency tree plus bun re-hoisting (root `commander` → 14.0.3 with `svgo/commander` nested; `entities`/`undici` re-hoisted with both versions retained — not a true downgrade), and the `bun.nix` bumps (astro 7.3.4, @astrojs/node 11.1.6, etc.) sync entries that were already stale versus `package.json`/`bun.lock` on main. Worth a line in the PR description, but not scope creep. - The a11y step now fails loudly with a diagnostic if the server never becomes ready, and includes the Node ≥ 22.13 sanity check pa11y 10 requires — addressing the earlier inline findings. **Non-blocking:** see the three comments — harden the sitemap origin rewrite, add `--disable-dev-shm-usage` for Chrome in Docker, and document the a11y step's reliance on workspace state from the verify step. --- *🤖 Review by opencode (opencode/mimo-v2.6-flash-free) — verdict: **approve**

clank-bot reviewed

2026-10-09 22:07:36 +00:00

clank-bot left a comment

Code review findings from opencode.

Code review findings from opencode.

.pa11yci.cjs Outdated

					
				@ -0,0 +6,4 @@

				      args: ["--no-sandbox"],

				    },

				  },

				  urls: [

[style] The hardcoded URL list (14 routes) will silently miss any newly added page. Consider a short comment above `urls` reminding contributors to update it when adding routes, since the sitemap option can't be reused here (it contains production URLs, not 127.0.0.1).

[style] The hardcoded URL list (14 routes) will silently miss any newly added page. Consider a short comment above `urls` reminding contributors to update it when adding routes, since the sitemap option can't be reused here (it contains production URLs, not 127.0.0.1).

Replaced by sitemap

martin marked this conversation as resolved

.woodpecker/verify.yml Outdated

					
				@ -8,3 +8,3 @@

				    image: docker.io/oven/bun:1.4.2-alpine

				    commands:

				      - apk add --no-cache chromium

				      - apk add --no-cache chromium nodejs

[style] pa11y 10 requires Node.js >= 22.13 (even-numbered releases). `apk add nodejs` is unpinned, so a base-image bump could silently install an unsupported Node. Verify/pin (e.g. `node --version` sanity check), or drop `nodejs` entirely and use `bun -e` for the readiness probe.

[style] pa11y 10 requires Node.js >= 22.13 (even-numbered releases). `apk add nodejs` is unpinned, so a base-image bump could silently install an unsupported Node. Verify/pin (e.g. `node --version` sanity check), or drop `nodejs` entirely and use `bun -e` for the readiness probe.

It is a runtime dependency

martin marked this conversation as resolved

.woodpecker/verify.yml

					
				@ -15,0 +23,4 @@

				          fi

				          sleep 1

				        done

				        exit 1

[style] If the server never becomes ready, this exits 1 with no message, and a crashed server burns the full 30s with output swallowed. Add a diagnostic, e.g. `echo "server did not become ready" >&2` before `exit 1`, and consider dumping the background server's output on failure for debuggability.

[style] If the server never becomes ready, this exits 1 with no message, and a crashed server burns the full 30s with output swallowed. Add a diagnostic, e.g. `echo "server did not become ready" >&2` before `exit 1`, and consider dumping the background server's output on failure for debuggability.

package.json Outdated

					
				@ -10,6 +10,7 @@

				    "astro": "bunx --bun astro",

				    "type-check": "bunx --bun astro check",

				    "test:e2e": "bunx playwright test",

				    "test:a11y": "pa11y-ci",

[style] Confirm `pa11y-ci` actually runs under Node (via its `#!/usr/bin/env node` shebang) and not Bun — puppeteer under Bun is not officially supported by pa11y 10. Also note local `bun run test:a11y` will fail unless `PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH` is set, since Bun blocks puppeteer's Chrome download postinstall. Worth a line in AGENTS.md (whose Testing section still claims there is no test suite).

[style] Confirm `pa11y-ci` actually runs under Node (via its `#!/usr/bin/env node` shebang) and not Bun — puppeteer under Bun is not officially supported by pa11y 10. Also note local `bun run test:a11y` will fail unless `PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH` is set, since Bun blocks puppeteer's Chrome download postinstall. Worth a line in AGENTS.md (whose Testing section still claims there is no test suite).

package.json

[style] This PR also bumps astro 7.2.10->7.3.4, @astrojs/node 11.1.5->11.1.6, tinyclip, verkit, find-proc and re-hoists `undici` from 8.10.2 down to 7.30.0 (via bun.lock/bun.nix). Unrelated dependency churn mixed with the pa11y addition makes regressions harder to bisect — consider splitting dependency bumps into their own PR, or at least calling them out in the PR description.

[style] This PR also bumps astro 7.2.10->7.3.4, @astrojs/node 11.1.5->11.1.6, tinyclip, verkit, find-proc and re-hoists `undici` from 8.10.2 down to 7.30.0 (via bun.lock/bun.nix). Unrelated dependency churn mixed with the pa11y addition makes regressions harder to bisect — consider splitting dependency bumps into their own PR, or at least calling them out in the PR description.

martin added 1 commit

2026-10-09 22:10:43 +00:00

ci: Refactor pa11y

ci/woodpecker/push/bun-verify Pipeline was successful

Details

ci/woodpecker/pr/verify Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

89e4f3f5e1

martin added 1 commit

2026-10-10 11:29:45 +00:00

ci: Improve pa11y d2a7d61ea5

martin added 1 commit

2026-10-10 11:33:56 +00:00

ci: Fix parse error

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/push/bun-verify Pipeline was successful

Details

ci/woodpecker/pr/verify Pipeline was successful

Details

083314dbc5

clank-bot reviewed

2026-10-10 11:34:24 +00:00

clank-bot left a comment

Code review findings from opencode.

Code review findings from opencode.

.pa11yci.json

					
				@ -0,0 +2,4 @@

				  "defaults": {

				    "standard": "WCAG2AA",

				    "chromeLaunchConfig": {

				      "args": ["--no-sandbox", "--disable-dev-shm-usage"]

[style] Consider adding `--disable-dev-shm-usage` alongside `--no-sandbox`: Docker/Alpine containers default to a 64MB `/dev/shm`, which can crash Chrome mid-run on heavier pages (a common source of flaky pa11y runs in CI). The `--no-sandbox` + `--disable-dev-shm-usage` combination in `playwright.config.ts` applies only to Playwright's e2e launch and does not carry over to pa11y's puppeteer launch here.

[style] Consider adding `--disable-dev-shm-usage` alongside `--no-sandbox`: Docker/Alpine containers default to a 64MB `/dev/shm`, which can crash Chrome mid-run on heavier pages (a common source of flaky pa11y runs in CI). The `--no-sandbox` + `--disable-dev-shm-usage` combination in `playwright.config.ts` applies only to Playwright's e2e launch and does not carry over to pa11y's puppeteer launch here.

.woodpecker/verify.yml

					
				@ -15,0 +21,4 @@

				      - apk add --no-cache chromium nodejs

				      - node -e 'const [major, minor] = process.versions.node.split(".").map(Number); if (!((major === 22 && minor >= 13) || major >= 24)) { console.error("Pa11y needs Node.js 22.13+ or 24+; found " + process.version); process.exit(1); }'

				      - export PUPPETEER_EXECUTABLE_PATH="$(command -v chromium-browser || command -v chromium)"

				      - |

[style] This step only works because the shared Woodpecker workspace carries `dist/` and `node_modules/` produced by the `verify` step — no `bun install` or `bun run build` happens here. `depends_on` documents the ordering but not that filesystem coupling; a one-line comment would prevent a confusing failure if the steps are ever reordered, split across pipelines, or run individually.

[style] This step only works because the shared Woodpecker workspace carries `dist/` and `node_modules/` produced by the `verify` step — no `bun install` or `bun run build` happens here. `depends_on` documents the ordering but not that filesystem coupling; a one-line comment would prevent a confusing failure if the steps are ever reordered, split across pipelines, or run individually.

package.json

					
				@ -10,6 +10,7 @@

				    "astro": "bunx --bun astro",

				    "type-check": "bunx --bun astro check",

				    "test:e2e": "bunx playwright test",

				    "test:a11y": "pa11y-ci --sitemap http://127.0.0.1:4323/sitemap-index.xml --sitemap-find 'https?://[^/]*' --sitemap-replace http://127.0.0.1:4323",

[style] The origin-rewrite pattern is coupled to the build-time `site` value: it covers `martials.no` (what CI builds actually produce) but its `localhost:4321` alternative is dead code — `astro.config.ts` falls back to `http://localhost:3000` when no env is loaded, and `sitemap-index.xml.ts`'s own `?? localhost:4321` fallback never fires because `site` is always defined. Today CI works because `ensureProcessNodeEnv('production')` runs before config load, but if `site` ever resolves to another origin (staging URL, config change) the rewrite silently no-ops and pa11y-ci targets the wrong host — failing loudly against a dead `localhost:3000`, or in the worst case testing the live production site. An origin-agnostic rewrite would be robust either way: `--sitemap-find 'https?://[^/]*' --sitemap-replace http://127.0.0.1:4323`.

[style] The origin-rewrite pattern is coupled to the build-time `site` value: it covers `martials.no` (what CI builds actually produce) but its `localhost:4321` alternative is dead code — `astro.config.ts` falls back to `http://localhost:3000` when no env is loaded, and `sitemap-index.xml.ts`'s own `?? localhost:4321` fallback never fires because `site` is always defined. Today CI works because `ensureProcessNodeEnv('production')` runs before config load, but if `site` ever resolves to another origin (staging URL, config change) the rewrite silently no-ops and pa11y-ci targets the wrong host — failing loudly against a dead `localhost:3000`, or in the worst case testing the live production site. An origin-agnostic rewrite would be robust either way: `--sitemap-find 'https?://[^/]*' --sitemap-replace http://127.0.0.1:4323`.