Skip to content

ci: cancel superseded pull request builds - #418

Merged
veprbl merged 3 commits into
masterfrom
claude/ci-pr-concurrency
Sep 15, 2026
Merged

veprbl merged 3 commits into
masterfrom
claude/ci-pr-concurrency

Conversation

@wdconinc

Copy link
Copy Markdown
Contributor

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

build-push.yml sets concurrency.cancel-in-progress: false unconditionally, so every push to a pull request queues a full build behind the previous one instead of replacing it. Each build occupies runners for hours across three architectures, so a PR updated a few times in succession holds several already-obsolete builds in the queue at once.

-  cancel-in-progress: false
+  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

Builds on master, on tags, and on the 6-hourly schedule keep cancellation disabled — they publish images and populate the caches that PR builds draw from, so they should always run to completion. The existing concurrency.group already separates PR runs from those, so this only changes PR behaviour.

Why no build work is lost

The Spack buildcache mirrors are configured with autopush in mirrors.yaml.in:

mirrors:
  eicweb:
    autopush: true
  ghcr:
    autopush: true

Packages are therefore pushed to the mirrors as they finish, not at the end of the job. A cancelled build leaves every package it completed available to the run that supersedes it, which picks them up from the buildcache instead of rebuilding.

Verification

actionlint clean; the expression parses and evaluates to the string "${{ github.event_name == 'pull_request' }}" as intended. The real check is the first PR that gets a second push after this merges — the superseded run should show as Cancelled rather than queued.

What is the urgency of this PR?

  • High (please describe reason below)
  • Medium
  • Low

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Optimization (issue #__)
  • Updated documentation
  • other: __

Please check if any of the following apply

  • This PR introduces breaking changes. Please describe changes users need to make below.
  • This PR changes default behavior. Please describe changes below.
  • AI was used in preparing this PR. Please describe usage below.

Behavior change: pushing to a PR now cancels that PR's in-flight build. If you were relying on a superseded run finishing — for example to collect its capybara artifacts — that run will now stop. Reverting is a one-line change back to false.

AI usage: the setting was flagged during a review of the CI configuration with Claude Code, and this PR was prepared with it. The autopush behaviour above was confirmed against mirrors.yaml.in in this repository; the decision to scope cancellation to pull requests was the maintainer's.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B6394UjZvzHPVygCvjRc9n


Generated by Claude Code

Every push to a pull request queued a full build behind the previous one
rather than replacing it. Each build occupies runners for hours across three
architectures, so a PR updated a few times held several obsolete builds in the
queue at once.

Cancel in-progress runs for pull_request events only. Builds on master, tags
and the schedule keep cancel-in-progress disabled: they publish images and
populate the caches that pull request builds draw from, so they should always
run to completion.

No build work is lost to a cancellation. The Spack buildcache mirrors are
configured with autopush (mirrors.yaml.in), so packages are pushed as they
finish rather than at the end of the job, and a cancelled build leaves its
completed packages available to the run that supersedes it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B6394UjZvzHPVygCvjRc9n
Copilot AI lite review requested due to automatic review settings September 14, 2026 16:11

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.

🟢 Approval recommended

The behavioral change is small and correctly scoped to PR events, with only a minor comment-wording accuracy nit noted.

Pull request overview

This PR optimizes CI resource usage by enabling GitHub Actions concurrency cancellation only for pull request runs, so new pushes supersede (cancel) in-flight PR builds instead of queueing behind them, while keeping non-PR runs (e.g., master and scheduled runs) non-cancellable to preserve publishing and cache population behavior.

Changes:

  • Set concurrency.cancel-in-progress to ${{ github.event_name == 'pull_request' }} so PR updates cancel superseded runs.
  • Add explanatory comments documenting the rationale (runner utilization and Spack buildcache autopush behavior).
File summaries
File Description
.github/workflows/build-push.yml Adjusts workflow-level concurrency cancellation behavior to supersede PR builds while preserving non-PR runs.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/build-push.yml Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 14, 2026 16:20
@wdconinc
wdconinc enabled auto-merge (squash) September 14, 2026 16:20

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.

🟢 Approval recommended

The change is low-risk and correctly scopes cancellation to pull request runs, with only a minor documentation wording nit noted.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

.github/workflows/build-push.yml:26

  • The comment says this workflow runs across "three architectures", but the matrix defines two architectures (amd64, arm64) plus an amd64 microarchitecture variant (amd64/v3). Consider calling these "target platforms" (or listing them) to avoid implying three distinct CPU architectures.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

Capybara summary for PR 418

@wdconinc
wdconinc requested review from a team and veprbl September 14, 2026 22:13
Comment thread .github/workflows/build-push.yml Outdated
Co-authored-by: Dmitry Kalinkin <dkalinkin1@bnl.gov>

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.

🟢 Approval recommended

The change is narrowly scoped to workflow concurrency behavior and appears correctly implemented, with only a minor PR-description mismatch noted.

Review details

Suppressed comments (1)

.github/workflows/build-push.yml:23

  • The PR description says cancellation remains disabled for runs "on tags", but this workflow currently only triggers on pushes to the master branch (no tag trigger under on: push:). Either remove the tag mention from the PR description or add an explicit on.push.tags: trigger if tag builds are intended.
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@veprbl veprbl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay

@veprbl
veprbl disabled auto-merge September 15, 2026 02:09
@veprbl
veprbl merged commit 595c4a0 into master Sep 15, 2026
91 of 92 checks passed
@veprbl
veprbl deleted the claude/ci-pr-concurrency branch September 15, 2026 02:09
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.

4 participants