Skip to content

fix(ci): restore per-commit CI runs on main - #1536

Merged
CatherineSue merged 1 commit into
mainfrom
catherinesue/restore-main-per-commit-ci
May 25, 2026
Merged

CatherineSue merged 1 commit into
mainfrom
catherinesue/restore-main-per-commit-ci

Conversation

@CatherineSue

@CatherineSue CatherineSue commented May 25, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

#1530 rewrote the concurrency block in pr-test-rust.yml and pr-test-mlx.yml and, alongside the new PR-number-based group key, dropped the event_name == 'pull_request' guard on cancel-in-progress, making it unconditionally true.

For push-to-main, the group key resolves to gateway-tests-main / mlx-tests-main — every commit on main shares one group. With cancel-in-progress: true, when commit B lands while commit A's CI is still running, B joins A's group and kills A's run mid-flight.

This breaks smg's invariant that every commit on main runs CI to completion. Without per-commit CI, a git bisect against a regression on main can land on a commit whose CI was cancelled and therefore has no signal.

The PR-number-based group key for PR events and the new cancel-merged-pr-tests.yml workflow added by #1530 are unaffected by this fix — they only target PR runs.

Solution

Restore the event_name == 'pull_request' guard on cancel-in-progress, keeping the PR-number-based group key from #1530:

concurrency:
  group: gateway-tests-${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || github.ref_name }}
  cancel-in-progress: ${{ github.event_name == 'pull_request' }}

Net behavior after this PR:

  • PR runs — still cancelled on new pushes within the same PR (preserves the intent of ci: cancel stale PR test runs #1530).
  • Main push runs — each commit's CI run completes independently (preserves the per-commit bisection invariant).
  • workflow_dispatch on main — shares the main push group; neither cancels the other.

Changes

  • .github/workflows/pr-test-rust.yml — change cancel-in-progress: true back to cancel-in-progress: ${{ github.event_name == 'pull_request' }}.
  • .github/workflows/pr-test-mlx.yml — same change.

Test Plan

Local checks:

pre-commit run --files .github/workflows/pr-test-rust.yml .github/workflows/pr-test-mlx.yml
git diff --cached --check

Both pass; YAML parses, no whitespace issues.

No Rust or Python sources are touched in this PR, so cargo fmt / cargo clippy are not applicable.

Behavioral verification once merged (observable on main):

  • Land a no-op commit on main while the prior commit's PR Test (SMG) is in progress; the prior run should continue to completion (after this PR) instead of being cancelled (before this PR).
  • On an open PR, push two consecutive commits — the older run should still be cancelled when the newer one starts (intent of ci: cancel stale PR test runs #1530 preserved).
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

  • Chores
    • Optimized GitHub Actions workflow concurrency settings to improve CI/CD efficiency for pull requests.

Review Change Stack

PR #1530 changed the concurrency block in pr-test-rust.yml and
pr-test-mlx.yml so that `cancel-in-progress` is unconditionally true,
including for push events on main. Because all main pushes share one
concurrency group (`gateway-tests-main` / `mlx-tests-main`), a new
commit landing on main now cancels the in-progress CI run for the
previous commit, breaking the project's invariant that every commit
on main runs CI to completion (so a failure can be bisected to a
specific commit).

Re-introduce the `event_name == 'pull_request'` guard on
`cancel-in-progress` while keeping the new PR-number-keyed group. Net
effect: PR runs still cancel on push (the intent of #1530), main push
runs do not cancel each other (the prior behavior).

Signed-off-by: Chang Su <8605658+CatherineSue@users.noreply.github.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@github-actions github-actions Bot added the ci CI/CD configuration changes label May 25, 2026
@coderabbitai

coderabbitai Bot commented May 25, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8baa30bd-1a49-4afd-87cd-68e926f20755

📥 Commits

Reviewing files that changed from the base of the PR and between 9341651 and c40bfc9.

📒 Files selected for processing (2)
  • .github/workflows/pr-test-mlx.yml
  • .github/workflows/pr-test-rust.yml

📝 Walkthrough

Walkthrough

This PR updates two GitHub Actions test workflows to conditionally cancel in-progress runs only for pull request events. The cancel-in-progress setting in .github/workflows/pr-test-mlx.yml and .github/workflows/pr-test-rust.yml now uses an event-type expression instead of always being true.

Changes

Workflow concurrency settings

Layer / File(s) Summary
Conditional cancellation in PR test workflows
.github/workflows/pr-test-mlx.yml, .github/workflows/pr-test-rust.yml
concurrency.cancel-in-progress is changed from unconditional true to ${{ github.event_name == 'pull_request' }}, allowing in-progress runs to continue when triggered by non-pull-request events.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • lightseekorg/smg#1530: Both PRs modify the same GitHub Actions workflow concurrency settings in pr-test-mlx.yml and pr-test-rust.yml (notably concurrency.group and cancel-in-progress behavior), so the changes overlap at the workflow level.
  • lightseekorg/smg#1175: The main PR's change to make cancel-in-progress conditional for pull_request events directly overlaps with the retrieved PR's corresponding update in .github/workflows/pr-test-rust.yml (and related workflow gating changes), so they are code-level related.
  • lightseekorg/smg#678: Both PRs modify the GitHub Actions workflow concurrency behavior in .github/workflows/pr-test-rust.yml, changing concurrency.cancel-in-progress to be conditional on pull_request events (${{ github.event_name == 'pull_request' }}) rather than always true.

Suggested labels

ci

Suggested reviewers

  • key4ng
  • slin1237
  • XinyueZhang369

Poem

🐰 Workflows once wild, now gently refined,
Pull requests alone shall keep in-progress confined,
Other events flow free, no cancellation bound,
Efficiency whispers—listen to the sound! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: restoring per-commit CI runs on main by fixing the concurrency configuration in CI workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch catherinesue/restore-main-per-commit-ci

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good. The cancel-in-progress change correctly preserves per-commit CI on main while still cancelling superseded PR runs. Clean and consistent across both workflow files.

@CatherineSue
CatherineSue merged commit fa6014c into main May 25, 2026
26 of 37 checks passed
@CatherineSue
CatherineSue deleted the catherinesue/restore-main-per-commit-ci branch May 25, 2026 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD configuration changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant