ci: Skip merge queue if pull request is up-to-date#24786
Conversation
| uses: MetaMask/github-tools/.github/actions/check-skip-merge-queue@v1 | ||
|
|
||
| check-diff: | ||
| name: Check diff |
There was a problem hiding this comment.
Also added some missing names in this PR for consistency. Can move this into a separate pull request if preferred.
bafa24f to
e8c82d9
Compare
| dedupe: | ||
| name: Dedupe | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name != 'merge_group' || needs.check-skip-merge-queue.outputs.skip-merge-queue != 'true' |
There was a problem hiding this comment.
Just to make sure I'm not missing anything. There is no risk of removing dedupe from the merge queue? I'm not remembering anything, just thinking out loud
There was a problem hiding this comment.
This is not removing it from the merge queue. With this condition, dedupe will run if:
- The run is not in the merge queue (
pull_request,push); or - The run is in the merge queue, but
skip-merge-queueis nottrue.
I don't think we can safely remove dedupe from the merge queue in pull requests that are not up-to-date.
tommasini
left a comment
There was a problem hiding this comment.
This is very cool! LGTM!
c5d9929 to
70fec24
Compare
7f12895 to
560ada5
Compare
| name: Check if pull request can skip merge queue | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| skip-merge-queue: ${{ steps.check-skip-merge-queue.outputs.up-to-date }} |
There was a problem hiding this comment.
S: The mapping is correct, but the naming is confusing. Consider renaming for clarity or document the mapping.
acffdfc to
20e7789
Compare
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This adds a new action which determines whether a pull request in the merge queue is up-to-date, meaning: - The pull request is based on the latest commit on `main`. - The pull request is the first in the merge queue. In this case, all status checks have already passed on the branch, and running in the merge queue would be redundant, meaning we can skip the merge queue checks. The same thing was implemented in the extension here: MetaMask/metamask-extension#38966 ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Introduces a merge-queue optimization to skip CI when a PR in `merge_group` is already up-to-date and first in queue. > > - Adds `check-skip-merge-queue` job (uses `MetaMask/github-tools` action) to detect up-to-date PRs > - Gates most jobs (`check-diff`, `dedupe`, `git-safe-dependencies`, `scripts`, `unit-tests`, `merge-unit-tests`, `needs_e2e_build`, `component-view-test`, `smart-e2e-selection`, `js-bundle-size-check`, `sonar-cloud`, `sonar-cloud-quality-gate-status`, `check-workflows`) with `if: github.event_name != 'merge_group' || needs.check-skip-merge-queue.outputs.skip-merge-queue != 'true'` and adds `needs: [check-skip-merge-queue]` > - Updates `merge-unit-tests`/`sonar-cloud` dependencies and conditions to respect skip logic; `check-all-jobs-pass` short-circuits when `SKIPPED` is true > - Adds human-friendly `name:` labels across jobs; no application code changes > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 20e7789. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
20e7789 to
dd033c3
Compare
|
dd033c3 to
a2116f1
Compare
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
These changes are entirely about CI pipeline optimization (merge queue skip logic) and do not touch:
No E2E tests need to run for this change. The risk is low as this only affects CI orchestration logic, not the app itself. Performance Test Selection: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
|




Description
This adds a new action which determines whether a pull request in the merge queue is up-to-date, meaning:
main.In this case, all status checks have already passed on the branch, and running in the merge queue would be redundant, meaning we can skip the merge queue checks.
The same thing was implemented in the extension here: MetaMask/metamask-extension#38966
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes CI behavior for
merge_groupevents by conditionally skipping most checks, which could let an incorrect “up-to-date” determination bypass required validations if the action/logic is wrong.Overview
Adds a new
check-skip-merge-queuejob to detect when a PR in the merge queue is already up-to-date (and first in queue) viaMetaMask/github-tools.When that flag is true, most CI jobs (lint/test/dedupe/workflow checks/bundle size and smart E2E selection) are skipped during
merge_groupruns, andcheck-all-jobs-passtreats the workflow as passed to avoid redundant merge-queue executions.Written by Cursor Bugbot for commit e04991a. This will update automatically on new commits. Configure here.