test(pg-js): assert what main's twenty required checks are wired to

[rubenhensen]

Closes #222. Part of encryption4all/postguard#247 (workstream C); the sibling half landed as encryption4all/postguard#316 (pg-core/tests/ci_wiring.rs).

The problem

main pins twenty status checks by display name, across seven of the eight workflow files, and nothing in this repo read any of those names:

Integration complete   Envelope compatibility   API surface       Conventional Commit
Build & test           Lint, typecheck & build  Unit tests        Baked URLs resolve
Image builds (PR, no push)   Svelte Check       Unit Tests        Lint
E2E Tests              Build (amd64)            Build (arm64)     Node examples
pg-dotnet              Canary scope is complete
nginx config test (outlook-addon)               nginx config test (website)

Two corrections to where those live, measured rather than assumed, both now recorded in CLAUDE.md. Classic protection here requires no status checks at all — only one approving review, with enforce_admins: false — so all twenty come from the ruleset main (id 14326269); read them from rules/branches/main, not from the protection endpoint. And that ruleset is bypassable, unlike the sibling repo's: the developers team holds bypass_mode: always, so for anyone on that team all twenty are advisory. Which is the argument for a guard rather than against one — a test runs on its own merits whether or not the enforcement behind it binds.

Branch protection matches a required check by its name. So renaming a job does not break its gate, it disarms it — the required context is simply never produced, and a check that never reports is not a check. Two variants are just as quiet: a required job that gains an if: reports skipped, which counts as passing; and a workflow that gains a paths: filter stops reporting on the PRs that miss it.

Integration complete had the same problem one level down. Its coverage is exactly its needs: list, and the only thing saying so was a comment in the workflow — "Adding a lane above? Add it to needs here too, or it is not covered by the required check no matter how loudly it fails." A lane added and not covered runs, reports, and counts for nothing.

What this adds

packages/pg-js/tests/ci-wiring.test.ts reads every file in .github/workflows/ and asserts, from the required name inwards rather than from a job id outwards:

• exactly one job produces each of the twenty required contexts — one producer catches a rename, exactly one catches the collision ci: qualify both nginx config check names with their app #156 fixed, where two jobs called nginx config test were indistinguishable to branch protection
• no job carrying a required context can skip itself on a pull request (job-level if: against an allowlist), and no workflow carrying one path-filters itself off a PR
• Integration complete's needs equals every other job in integration.yml, derived from the file — so a new lane fails here until it is covered
• that aggregate still keeps if: always(), still compares the lane results against success, and still exits non-zero when one does not. That last pair is one regex rather than two assertions on purpose: separately, each passes on a step the other has already gutted, because the empty-results guard supplies an exit 1 all by itself
• edited is still a pull_request type in integration.yml and pr-title.yml, where a verdict depends on the merge base or the title
• api-surface still runs pnpm api:gate, envelope-compat still runs both directions and the append-only fixture check, and both still check out with fetch-depth: 0
• every release trigger is app-scoped, never a bare v* — the tag namespace here is shared with changesets and the pre-monorepo v2.3.3 tags
• the two published image names are still the literals ops deploys rather than ${{ github.repository }}, and the website image is still built on a PR but pushed and tagged only from main

Matrix names are expanded, both spellings: Build (amd64)/Build (arm64) are required contexts that exist only after expansion.

Placement

This is the load-bearing decision, and it differs from the sibling repo's. There, ci_wiring.rs had to dodge a path filter. Here no workflow is path-filtered — every one says so in its header comment, deliberately — so "which lane is unconditional" answers "all of them", and the real question is which lane the guard is a hostage of.

It lives in packages/pg-js, whose vitest suite runs in three lanes (Node 22/Node 24 via pnpm -r test, Bun 1.3.14, Deno 2.8.0). That puts it in a different file from six of the seven workflows that carry a required context, so a job renamed or deleted in website.yml, outlook-addon.yml, tb-addon.yml, pr-title.yml, examples.yml or sdk-canary.yml cannot skip its own guard — a guard under apps/website would be run by Svelte Check's own workflow, which is exactly the file whose renames it is supposed to notice. No single lane deletion silences it either.

Stated rather than papered over: dropping all three SDK lanes from the aggregate's needs in one change would leave this file red in jobs nothing requires. It still fails loudly — silent drift is what the map is about, and that is not silent — and Envelope compatibility and API surface, required in their own right, would still block.

Verification

38 drifts were applied to the workflows one at a time and each confirmed red: ten renames (including a case-only one, a matrix-derived context, a name: stripped entirely, and Integration complete moved onto a single lane), six skip/filter drifts, six against the aggregate's coverage and its ability to fail, six against the merge-base gates, seven against release and publish wiring, a new job in delivery.yml colliding with website.yml's Lint, and two against the reader itself, which throw by name rather than passing vacuously.

Four were caught only because of how the assertions are written, and would have passed inspection: deleting the aggregator and moving Integration complete onto a single lane (caught by reading the required name first, not the job id), and two that slipped the first draft — a lane loop that logs without exiting, and if false; then in place of the success comparison. Separately, "contains success" and "contains exit 1" each pass on a step the other has already gutted, because the empty-results guard supplies an exit 1 by itself; hence the single regex. Three semantics-preserving rewrites — reformatting integration.yml to 2-space and 8-space indent, and to CRLF — were confirmed still green: the reader detects each file's indent rather than assuming it, and throws when it finds no jobs, so a shape change it cannot follow fails loudly instead of asserting nothing.

What this does not cover

That the ruleset still lists those twenty contexts, and who may bypass them, lives in GitHub's API — gh api repos/encryption4all/postguard-js/rules/branches/main. No test runner can call it. REQUIRED_CONTEXTS is this side of that link; rename a job and the context together.

No source or app behaviour changes, so no changeset.

[dobby-coder]

Thanks for the detailed writeup, Ruben — I'm on it. I'll review the CI wiring changes here (matching status checks, if:/paths: gating, and Integration complete's needs: coverage) against main's required checks and follow up with a review shortly.

[dobby-coder]

Reviewed at 4ca1d32. The instrument is good, and the mutation-verification standard behind it is the right one — so I held the guard to its own standard. I re-ran the suite (14/14, and 344/344 for the whole pg-js package at HEAD), then mutated the workflows against it.

Two of its assertions fail open. A semantics-preserving YAML reformat silences each of them while all 14 tests stay green. For a file whose entire thesis is that silent drift is the enemy, that is worth closing before merge. Both are reproduced below, and both suggested fixes are verified rather than reasoned about: applied to the working tree, red on the mutation, green on the real workflows, and green on a legitimate reformat of the same key.

Every mutation below was applied alone, with the suite re-run and the tree restored afterwards:

mutation	wanted	observed
outlook-addon.yml tags: rewritten as a block sequence with a bare v*	red	14/14 green
the same bare v* in flow form, tags: ['v*'] (control)	red	red
website.yml pull_request: branches-ignore: [main]	red	14/14 green, 7 required contexts silenced
continue-on-error: true on website.yml's svelte-check	red	14/14 green
needs: [pg-manual] added to sdk-canary.yml's scope	red	14/14 green
integration.yml needs: as a block sequence (no semantic change)	green	red, and misdiagnosed as missing coverage
tb-addon.yml tags: ["tb-addon-v*"], quote style only	green	red

The first two are the blockers. The rest are non-blocking notes, inline.

Two things I checked and found already right, so they need no action: the reader's .replace(/\r\n/g, '\n') on line 249 closes the CRLF trap that bit the sibling pg-core/tests/ci_wiring.rs in encryption4all/postguard#316, so this repo needs no .gitattributes for it (I confirmed there is none). And the types: reader on line 529 and the needs: reader both fail closed on a block sequence rather than open — loud, not silent.

---

Outside the diff: the PR description undercounts what the file already claims.

The description is not part of the diff, so this cannot be an inline comment. At HEAD the code and CLAUDE.md say seven of the eight workflow files (test file line 4, CLAUDE.md line 140), six of the seven (line 43, CLAUDE.md line 164), and 38 drifts. The description still says "across six workflow files", "a different file from four of the six workflows it reads", and "37 drifts were applied".

I counted independently: seven of the eight files produce required contexts — every one but delivery.yml — and the guard sits in a different file from six of those seven, since examples.yml and sdk-canary.yml run no pg-js vitest either. So the description understates the guard's own reach, and the placement argument is stronger than the version written there. It is a PATCH on the body — no branch push, no CI rerun, no dismissed approval.

[dobby-coder]

Fails open on the one YAML spelling it does not read. tags: is matched only as a flow list, so rewriting a trigger as a block sequence leaves tags undefined and if (!tags) continue skips the whole loop in silence — which is exactly the drift this test's title names.

Verified by mutation on outlook-addon.yml:

    push:
        branches: [main]
        tags:
            - 'v*'

All 14 tests stayed green with a bare v* release trigger in place. The control — the same bare pattern in flow form, tags: ['v*'] — correctly goes red with "expected 'v*' to match /^[a-z][a-z0-9-]*-v*$/". A block sequence is the commoner of the two spellings, and a yamlfmt pass would introduce it without touching a single pattern.

The suggestion below reads both spellings and asserts the pattern list is non-empty, so a tags: this reader cannot parse fails loudly instead of skipping. Verified with it applied: real workflows 14/14 green, the block-style bare v* red, and a legitimate block-sequence reformat (- 'outlook-addin-v*') still green. tsc --noEmit passes.

Suggested change

	const tags = push?.match(/tags:\s*\[(.*)\]/)?.[1];
	if (!tags) continue;
	for (const pattern of tags.split(',').map(unquote)) {
	if (push === null || !/^\s*tags:/m.test(push)) continue;
	const pushLines = push.split('\n');
	const at = pushLines.findIndex((line) => /^\s*tags:/.test(line));
	const flow = pushLines[at].match(/tags:\s*\[(.*)\]/)?.[1];
	const patterns = (
	flow === undefined
	? blockUnder(pushLines, at)
	.filter((line) => line.trim().startsWith('- '))
	.map((line) => line.trim().slice(2))
	: flow.split(',')
	)
	.map(unquote)
	.filter((entry) => entry !== '');
	expect(
	patterns,
	`${candidate.file} declares a \`tags:\` trigger whose patterns this reader could not ` +
	'read, so the assertion below would have skipped it in silence',
	).not.toHaveLength(0);
	for (const pattern of patterns) {

[dobby-coder]

branches-ignore: walks straight past this guard. Line 443 correctly covers both paths: and paths-ignore:, but the branches check on this line only fires for /^\s*branches:/m, which does not match branches-ignore:. So a pull_request: branches-ignore: [main] skips the guard entirely — and that is the same never-reports outcome as the paths filter the test already refuses, for the same reason.

Verified by mutation on website.yml:

    pull_request:
        branches-ignore: [main]

All 14 tests stayed green while seven required contexts went silent: Svelte Check, Unit Tests, Lint, E2E Tests, Build (amd64), Build (arm64) and nginx config test (website).

Suggested fix verified: red on the mutation with the message below, green on the real workflows, tsc --noEmit clean.

Suggested change

	if (/^\s*branches:/m.test(block ?? '')) {
	expect(
	block,
	`${file}'s \`pull_request:\` trigger declares a \`branches-ignore:\` filter, which ` +
	'silences it the same way a paths filter does: the workflow never runs for the PRs ' +
	'it excludes, so its required contexts never report on them.',
	).not.toMatch(/^\s*branches-ignore:/m);
	if (/^\s*branches:/m.test(block ?? '')) {

[dobby-coder]

Non-blocking. The skip vector this test covers is job-level if:, but continue-on-error: true produces the same gate-that-cannot-fail and is unasserted.

Verified by mutation: adding continue-on-error: true under website.yml's svelte-check keeps all 14 tests green, while Svelte Check can then report success on a failing run. Worth one more assertion in this loop — a required job should not carry continue-on-error: true, and for the two merge-base gates below, neither should the step running the command.

[dobby-coder]

Non-blocking, and latent rather than live. requiredJobs() is checked for its own if:, but not for a needs: on a job that is itself skipped on a pull request — which skips the required job transitively, and a skipped check counts as passing.

The ingredients are in this repo: sdk-canary.yml's pg-manual and pg-dotnet both carry if: github.event_name != 'pull_request', and Canary scope is complete sits in the same file. Verified by mutation: adding needs: [pg-manual] to that job keeps all 14 tests green while the required context never reports on a PR.

Nothing is wrong today — scope has no needs:. The suggestion is to extend the allowlist check across a required job's needs closure, or at minimum assert that required jobs have no needs: outside the aggregate.

[dobby-coder]

Non-blocking, and it fails closed — but about the wrong thing. needs: is parsed as a flow list only, so a block-sequence needs: reads back as [] rather than throwing.

Verified by mutation: reformatting the aggregate's needs: [node, envelope-compat, api-surface, bun, deno] into a block sequence — semantically identical — fails the aggregate covers every lane in the file with "A lane in this file but not in that list runs, reports, and counts for nothing", i.e. it reports a pure reformat as missing coverage.

No gate is disarmed, so this is a message-quality point rather than a hole. But it does sit against the promise on lines 75-76 that a shape change the reader cannot follow "fails loudly instead of quietly asserting nothing" — here it fails loudly about something that is not true. Either read both spellings, or throw a "this reader no longer understands needs:" error when the key is present and yields nothing.

[dobby-coder]

Nit. toContain("tags: ['tb-addon-v*']") pins one exact YAML spelling — quote style and inter-token spacing included — in a file that otherwise goes out of its way to read shape-tolerantly.

Verified: changing tb-addon.yml to tags: ["tb-addon-v*"], a quote-style change and nothing else, turns this red with "tb-addon.yml no longer builds on its own release tags", which is not what happened. A yamlfmt or prettier pass over the workflows would do exactly that.

every release trigger is app-scoped above already parses the pattern; asserting the parsed pattern equals tb-addon-v* buys the same guarantee without the brittleness. Worth noting the interaction with the blocker above: this literal is currently the only thing that would keep the block-sequence hole from applying to tb-addon.yml too.

[dobby-coder]

Nit. everyRequiredContextHasExactlyOneJob names nothing in this file — the test is titled in prose, it('every context the ruleset requires is produced by exactly one job'). Same pointer on line 685.

It reads like a carry-over from the sibling pg-core/tests/ci_wiring.rs, where tests are fn names; here a reader following the pointer greps and finds only these two comments. Quoting the it(...) title instead would match what line 25 already does for the aggregate covers every lane in the file.

[dobby-coder]

Posted a REQUEST_CHANGES review on #223 — verified everything against HEAD (4ca1d32) by actually mutating the workflows rather than reasoning about them.

Two real fail-open bugs, both with tested one-click suggestion fixes:

• The tags: check only handles flow-list YAML (['v*']) — a semantics-preserving switch to block-sequence form (a plain yamlfmt pass) leaves outlook-addon.yml's bare v* trigger completely unnoticed, 14/14 green.
• branches-ignore: isn't covered by the filter guard the way paths-ignore: is — applying it to website.yml silently drops 7 required contexts, also 14/14 green.

Also flagged 5 non-blocking nits (unasserted continue-on-error, needs: transitive-skip gap, a couple of brittle checks, and two comments referencing a function that doesn't exist in the file), plus one out-of-diff note in the review body: the PR description's numbers ("six workflow files", "four of the six", 37 drifts) are stale — the code at HEAD actually covers seven of eight files, six of seven, 38 drifts.

Two things checked out fine as-is: the CRLF handling and the types:/needs: readers already fail closed. Full suite's green (344/344).