Skip to content

ci: key main concurrency group per-sha so main runs stay conclusive (#436)#481

Merged
avrabe merged 1 commit into
mainfrom
fix/436-main-runs-conclusive
Jun 5, 2026
Merged

ci: key main concurrency group per-sha so main runs stay conclusive (#436)#481
avrabe merged 1 commit into
mainfrom
fix/436-main-runs-conclusive

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented Jun 5, 2026

Closes the pure-hygiene half of #436.

Root cause (verified)

#436 reported Playwright E2E red/cancelled across consecutive main merges with nobody noticing. The concurrency block looked correct (cancel-in-progress is false on push), yet gh run list --branch=main showed the last several main runs cancelled. Why?

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

On push, head_ref is empty, so every main commit shares one group: <workflow>-refs/heads/main. With cancel-in-progress: false the running job finishes, but GitHub still supersedes queued runs in the group. In a PR-merge train, each new main push cancels the previous queued main run — so main rarely reaches a conclusive Playwright/benchmark result. A genuine regression would look identical to the benign dataset-growth failures (#435).

Fix

Fall back to github.sha (not github.ref) on push:

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

Every main commit now gets its own group and is never superseded. PRs are unchangedhead_ref still groups per source branch, so cancel-in-progress keeps superseding stale PR runs (the desired behavior). Applied to both workflows that shared the pattern: ci.yml and benchmarks.yml.

No run:-context change, so no injection surface — head_ref/sha are only used in the concurrency-group key.

Out of scope (your call)

Whether to promote Playwright E2E to a required status check (part 2 of #436) is a separate policy decision left to the maintainer.

🤖 Generated with Claude Code

…436)

Issue #436: Playwright E2E (and Criterion benchmarks) were red/cancelled across
consecutive main merges and nobody noticed, because main never reached a
conclusive result. Root cause (verified): the concurrency group fell back to
`github.ref` on push, so every main commit shared one group
(`<workflow>-refs/heads/main`). With `cancel-in-progress` false the *running*
job completes, but GitHub still supersedes *queued* runs in the group — so in a
PR-merge train each new main push cancelled the previous queued run, and main
rarely produced a conclusive Playwright/benchmark result. A real regression in
any covered view would have looked identical to the benign dataset-growth
failures (#435).

Fix: fall back to `github.sha` (not `github.ref`) on push, so every main commit
gets its own concurrency group and is never superseded. PRs are unchanged —
`head_ref` still groups per source branch, so cancel-in-progress keeps
superseding stale PR runs. Applied to both workflows that shared the pattern
(ci.yml, benchmarks.yml).

This is the pure-hygiene half of #436; whether to promote Playwright to a
required status check remains a separate maintainer decision.
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@avrabe avrabe merged commit 55769be into main Jun 5, 2026
21 of 39 checks passed
@avrabe avrabe deleted the fix/436-main-runs-conclusive branch June 5, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant