Playwright E2E is a silently-broken gate: non-required + main runs cancelled = never conclusively green

[avrabe]

Friction / CI-hygiene finding (found while dogfooding)

While triaging why my PR showed a red Playwright E2E check, I found it has
been red across multiple consecutive merges (#432, #434, …) and nobody
noticed. Two compounding reasons make it a non-functional gate:

1. Not a required status check. repos/.../branches/main/protection
   reports required_status_checks.contexts: [] — so a red Playwright never
   blocks a merge.
2. main runs are concurrency-cancelled. gh run list --workflow=ci.yml
   shows the push/main Playwright runs almost all cancelled (superseded by
   the next push). So main never gets a conclusive Playwright result either.

Net effect: a real serve/dashboard regression in any Playwright-covered view
would be invisible. In this case the failures were benign (E2E tests that
hardcoded queries which outgrew the dogfood dataset — fixed in #435 / REQ-171),
but the next red could be a genuine regression and would look identical.

What I verified

• 3 tests failed reproducibly through the CI retry (not flake): two
  diagram-viewer graph cases + graph.spec.ts polygon shapes. Reproduced
  locally in Chromium and root-caused to dataset growth past the 200-node
  render budget / heavy full-graph render. Fix in fix(playwright): bound graph E2E node count so dataset growth doesn't trip render budget (REQ-171) #435.

Suggested fixes (hygiene, separate from #435)

• Make main's Playwright run conclusive: either exempt the Playwright job
  from cancel-in-progress concurrency, or run it on a schedule against main,
  so there's always a known-good/known-bad signal.
• Decide its gate status explicitly: either promote Playwright E2E to a
  required check (now that fix(playwright): bound graph E2E node count so dataset growth doesn't trip render budget (REQ-171) #435 makes it green), or mark it advisory in the
  workflow with a comment so a red result isn't ambiguously ignored.
• Surface red E2E: a notification (or a status badge) when E2E goes red on
  main, so it can't rot silently again.

Found by an AI agent dogfooding rivet.

[avrabe]

Triage pass — premise confirmed; the body is well-researched and the three suggested fixes are clearly separable, but they're stated as "options" rather than testable done-statements, and they're a mix of workflow edits and a policy call (promote-to-required vs. mark-advisory). Could you lift the intended slice into an explicit ## Acceptance Criteria block so a single focused PR can pick it up? Sketch:

## Acceptance Criteria
- [ ] `.github/workflows/ci.yml` — the Playwright E2E job is exempted from the `concurrency.cancel-in-progress` cancellation on `push` to `main` (e.g. via a job-level `concurrency:` override or a separate `playwright-main.yml` scheduled/push-on-main workflow), so every `main` push produces a conclusive Playwright result (verified by inspecting the next 3 main runs after merge)
- [ ] Policy decision encoded — pick ONE and state in the PR description:
   - [ ] (a) Playwright E2E is added to `repos/.../branches/main/protection.required_status_checks.contexts` (now that #435 makes it green), OR
   - [ ] (b) The workflow job is renamed / annotated `playwright-e2e (advisory)` with a comment in the workflow YAML so a red result isn't ambiguously ignored
- [ ] (Optional, scope-flag if doing in this PR) A failure-notification path (e.g. a `failure()` step that opens or labels an issue, or a status badge in `README.md`) so a future red-on-main can't rot silently as it did between #432 and #434
- [ ] Commit trailer: `Trace: skip` (this is `ci` work) per AGENTS.md "Exempt Commits"; `rivet validate` PASS unchanged

If items (a) and (b) want different PRs (one's an infra-permissions change, the other is a workflow edit), say so and I'll split.

Process side-rail (same as the other open triage threads today): https://pulseengine.eu/blog/ is still HTTP 503 (the site's TLS cert is expired — exactly the second finding on #420), so the binding workflow guidance can't be consulted and no PR will land from this run regardless. Comment is the AC ask only.

Not implemented this run.

---

Generated by Claude Code — issue-triage agent run 2026-06-03.

---

Generated by Claude Code

[avrabe]

Correcting my own earlier framing after verifying against the workflow config (ci.yml):

The concurrency config is NOT the naive culprit I implied. It already does the right thing for push:

concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}   # false on push to main

So cancel-in-progress is false for main pushes — they shouldn't be actively cancelled.

The actual mechanism (verified via gh run list --branch main --event push — every recent main run is cancelled): with cancel-in-progress: false, concurrent runs in a group queue rather than cancel, BUT GitHub keeps only the most recent queued run per group — a newer queued run cancels the older queued one. Because PRs are merged roughly once per loop iteration and the self-hosted runners drain slower than that, each merge's main run supersedes the previous still-queued one before it completes. Net: main rarely gets a conclusive run.

Severity is lower than I first said: every change is fully gated on its PR (Playwright included) before merge, so a regression is caught pre-merge. The residual gap is only post-merge main-on-main validation (e.g. two PRs that each pass alone but interact) — real but uncommon, not "any regression is invisible".

Targeted fix (maintainer's design call on cadence): give main a conclusive E2E signal outside the merge-cancel race — either a dedicated scheduled workflow running Playwright against main (e.g. every few hours / nightly), or a separate concurrency group for the post-merge E2E run so it isn't superseded by the next merge. A schedule is the simplest and also dodges the self-hosted-runner saturation.

Found + corrected by an AI agent dogfooding rivet.

[avrabe]

Triaged from the hourly loop — still valid, and I can refine the root cause of part 1.

Part 1 (main runs never conclusive) — confirmed still broken, but not for the reason it looks like. The concurrency block that should protect main is already in place (added in #258, 2026-05-10, before this issue):

concurrency:
  group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

So cancel-in-progress is false for push. Yet gh run list --workflow=ci.yml --branch=main still shows the last ~6 main runs (2026-06-04) all conclusion=cancelled. Why, with cancel-in-progress=false?

Because all main pushes share one group key — CI-refs/heads/main (on push, head_ref is empty so it falls back to github.ref). GitHub's rule: with cancel-in-progress: false, the running job finishes, but any pending/queued runs in the group are superseded — when merges land faster than CI completes (as in a PR train), each newly-queued main run cancels the previous queued one. So main rarely reaches a conclusive Playwright result.

Surgical fix: key push runs per-SHA so they never share a cancellable group:

  group: ${{ github.workflow }}-${{ github.head_ref || github.sha }}

On PRs head_ref still groups per source branch (cancel-in-progress=true → supersede stale PR runs, the desired behavior); on push, the github.sha fallback gives every main commit its own group → never cancelled. One-line change, no policy judgment.

Part 2 (required-check status) remains an explicit maintainer call — promoting Playwright to a required check vs. marking it advisory-in-workflow. I've left that for you; the part-1 fix above is the pure-hygiene half that makes main conclusive regardless of which you choose.

(Separately confirmed the original failures were the dataset-growth ones fixed in #435, not a live regression — but as you noted, the next red would look identical, which is the actual risk.)

[avrabe]

Implemented the pure-hygiene half in PR #481. Confirmed the root cause from last comment: the push fallback was github.ref, so every main commit shared one concurrency group (<workflow>-refs/heads/main) and GitHub superseded queued main runs in a merge train even with cancel-in-progress: false. Fix keys push runs by github.sha so each main commit gets its own group and stays conclusive; PRs are unchanged (head_ref still supersedes stale PR runs). Applied to ci.yml and benchmarks.yml. Part 2 (promote Playwright to a required check) is left as your policy call.

[avrabe]

Update from the loop — main is now conclusive (the per-sha concurrency fix #481 merged), which immediately surfaced the lone red gate: a real Playwright failure.

Found + fixed (PR #491, merged): the failing test was rendering-invariants.spec.ts:195 "/graph?variant=minimal-ci IS scoped". The dogfood dataset grew past 200 requirement nodes, so the unscoped /graph?types=requirement now exceeds the render budget and returns the "above node budget" placeholder (0 rendered nodes) — making fullNodes(0) >= scopedNodes(1) fail. Lifted the budget with &limit=2000 on both fetches (the pattern graph.spec.ts already uses). Verified locally with the full suite: 399 passed / 1 failed → now 0 failures.

The recurring pattern (worth a durable fix): this is the third time a graph E2E test broke purely from dataset growth — #435 (graph.spec node budget), the diagram-viewer focused-subgraph fix, and now #491. Root cause: these size-sensitive graph tests render against the live, growing dogfood corpus (the repo's own artifacts/), so every batch of new artifacts eventually trips a node-budget/timeout threshold and someone band-aids another limit=2000/focused-subgraph.

The durable fix is to point the size-sensitive graph/diagram tests at a small fixed fixture project (a checked-in tests/playwright/fixtures/<n>-artifacts/ served on a side port) instead of the repo's own corpus — then node counts are deterministic and the tests stop rotting as rivet's own artifact set grows. That's a bigger change than this PR; flagging it here rather than opening yet another issue. The other half of this issue (promote Playwright to a required check now that it's green) remains your policy call.