-
Notifications
You must be signed in to change notification settings - Fork 68
infra: add Mergify serial merge queue for dev branch #335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
TFT444
wants to merge
7
commits into
dev
Choose a base branch
from
infra/334-mergify-queue
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+294
−0
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d6d4de4
infra: add Mergify serial merge queue for dev branch
TFT444 2e7def4
docs: fix merge-queue.md review findings
TFT444 a9ebc99
fix(mergify): remove invalid speculative_checks, add queue_conditions…
TFT444 6d9ab9c
fix(mergify): address review feedback from ritiksah141
TFT444 447bb6e
fix(mergify): gate CodeQL checks in merge_conditions
TFT444 ddb1839
fix(mergify): complete check coverage, harden queue, fix dismiss policy
TFT444 7dd637b
fix(mergify): set changes_requested: false, document inactive-reviewe…
TFT444 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| # .mergify.yml | ||
| # Merge-queue configuration for the OpenShield dev branch. | ||
| # | ||
| # This file is inactive until the Mergify GitHub App is installed and | ||
| # authorized for this repository. GitHub Actions remains the CI system. | ||
| # Mergify reads pull-request state and operates the queue; it does not | ||
| # run CI, replace existing workflows, or bypass GitHub branch protection. | ||
| # | ||
| # Related: https://github.com/openshield-org/openshield/issues/334 | ||
|
|
||
| # Limit speculative checks globally so at most one PR is tested at a time. | ||
| # This enforces serial behaviour across the queue. | ||
| merge_queue: | ||
| max_parallel_checks: 1 | ||
|
|
||
| queue_rules: | ||
| - name: default | ||
| # Restrict queue entry to PRs targeting dev, including manual | ||
| # @mergifyio queue commands. Without this, a maintainer could | ||
| # manually queue a PR targeting any branch. | ||
| queue_conditions: | ||
| - base=dev | ||
|
|
||
| # One PR merged per operation. Combined with max_parallel_checks: 1 | ||
| # above, this gives a fully serial merge queue. | ||
| batch_size: 1 | ||
|
|
||
| # Dequeue a PR whose required checks have not posted within this | ||
| # window instead of stalling the entire queue indefinitely. The | ||
| # default ("auto") applies no timeout until enough queue history has | ||
| # been gathered, so a stuck check on the very first queued PR would | ||
| # block every subsequent PR forever. | ||
| checks_timeout: 2 hours | ||
|
|
||
| # These conditions must hold at the moment Mergify merges the PR. | ||
| # Mergify re-evaluates them after rebasing onto the current dev HEAD. | ||
| # | ||
| # CI Summary aggregates all jobs inside ci.yml, including the Astro | ||
| # website build and rendered-site verification added by PR #329 | ||
| # (now on dev). CodeQL runs in a separate workflow (codeql.yml) and | ||
| # is NOT included in CI Summary, so it is gated explicitly below. | ||
| # Build site is gated explicitly for belt-and-braces coverage: it | ||
| # runs unconditionally on every dev-targeted PR (no paths filter) | ||
| # so it cannot stall the queue. | ||
| # | ||
| # External Semgrep app checks (Semgrep OSS, semgrep-cloud-platform/scan) | ||
| # are deliberately not gated: their coverage duplicates the SAST | ||
| # (Semgrep) job already gated through CI Summary, and third-party app | ||
| # checks can vanish if the app is uninstalled or its plan changes, | ||
| # which would stall the queue permanently. | ||
| merge_conditions: | ||
| - check-success=CI Summary | ||
| - check-success=DCO sign-off | ||
| - check-success=dependency-review | ||
| - check-success=CodeQL | ||
| - check-success=Analyze (python) | ||
| - check-success=Analyze (javascript) | ||
| - check-success=Build site | ||
| # terraform-plan only runs for PRs touching infra/terraform/**. An | ||
| # unconditional check-success would stall every non-terraform PR | ||
| # forever. This condition passes when the PR touches no terraform | ||
| # files; otherwise the check must be green. | ||
| - or: | ||
| - -files~=^infra/terraform/ | ||
| - check-success=Terraform fmt / validate / plan | ||
| - "#approved-reviews-by>=1" | ||
| - "#changes-requested-reviews-by=0" | ||
| - "#review-threads-unresolved=0" | ||
| - -draft | ||
| - -label=blocked | ||
|
|
||
| pull_request_rules: | ||
| # Dismiss stale approvals when a new commit is pushed to a dev-targeted PR. | ||
| # CHANGES_REQUESTED reviews are intentionally NOT dismissed (changes_requested | ||
| # is explicitly set to false). A reviewer's objection must survive the | ||
| # contributor's push and be cleared only by the reviewer re-reviewing, or | ||
| # by a maintainer manually dismissing it via GitHub after verifying the fix | ||
| # (see the inactive-reviewer process in docs/merge-queue.md). | ||
| # Auto-dismissing CHANGES_REQUESTED reviews would allow any push followed | ||
| # by a third-party approval to silently erase a blocking concern without | ||
| # the original reviewer ever checking the fix. | ||
| # | ||
| # Reviewers: use inline conversation threads for every blocking concern. | ||
| # Threads survive this dismissal and continue to block via | ||
| # #review-threads-unresolved=0, giving an independent gate beyond the | ||
| # review verdict itself. | ||
| - name: dismiss stale approvals on new push | ||
| conditions: | ||
| - base=dev | ||
| actions: | ||
| dismiss_reviews: | ||
| approved: true | ||
| changes_requested: false | ||
| message: "A new commit was pushed. Please re-review and re-approve before this PR can merge." | ||
|
|
||
| # Queue an eligible PR automatically once all conditions are met. | ||
| # Entry conditions mirror merge_conditions so only fully green PRs | ||
| # enter the queue. A PR admitted while a check is still pending | ||
| # occupies the single queue slot and stalls every PR behind it. | ||
| - name: add to merge queue when eligible | ||
| conditions: | ||
| - base=dev | ||
| - check-success=CI Summary | ||
| - check-success=DCO sign-off | ||
| - check-success=dependency-review | ||
| - check-success=CodeQL | ||
| - check-success=Analyze (python) | ||
| - check-success=Analyze (javascript) | ||
| - check-success=Build site | ||
| - or: | ||
| - -files~=^infra/terraform/ | ||
| - check-success=Terraform fmt / validate / plan | ||
| - "#approved-reviews-by>=1" | ||
| - "#changes-requested-reviews-by=0" | ||
| - "#review-threads-unresolved=0" | ||
| - -draft | ||
| - -label=blocked | ||
| actions: | ||
| queue: | ||
| name: default | ||
|
|
||
| # Apply awaiting-author when a reviewer has requested changes. | ||
| - name: label awaiting-author when changes are requested | ||
| conditions: | ||
| - base=dev | ||
| - "#changes-requested-reviews-by>0" | ||
| actions: | ||
| label: | ||
| add: | ||
| - awaiting-author | ||
|
|
||
| # Remove awaiting-author once all change requests are resolved. | ||
| - name: remove awaiting-author when no changes are requested | ||
| conditions: | ||
| - base=dev | ||
| - label=awaiting-author | ||
| - "#changes-requested-reviews-by=0" | ||
| actions: | ||
| label: | ||
| remove: | ||
| - awaiting-author |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| # Merge Queue | ||
|
|
||
| OpenShield uses Mergify to run a serial merge queue on the `dev` branch. | ||
| GitHub Actions remains the CI system. Mergify reads pull-request state, | ||
| waits for all conditions to be met, rebases each PR onto the current `dev` | ||
| HEAD, and merges it only after CI passes on the updated state. | ||
|
|
||
| This configuration is inactive until the Mergify GitHub App is installed | ||
| and authorized for this repository. The `.mergify.yml` file has no effect | ||
| before installation. | ||
|
|
||
| ## Eligibility | ||
|
|
||
| The queue applies only to pull requests that target the `dev` branch. | ||
| PRs targeting any other branch are not affected by this configuration. | ||
|
|
||
| A pull request enters the queue automatically when all of the following | ||
| are true: | ||
|
|
||
| - The PR targets the `dev` branch | ||
| - `CI Summary` check is successful (aggregates all jobs in `ci.yml`, | ||
| including the Astro website build and rendered-site verification) | ||
| - `DCO sign-off` check is successful | ||
| - `dependency-review` check is successful | ||
| - `CodeQL` check is successful | ||
| - `Analyze (python)` check is successful | ||
| - `Analyze (javascript)` check is successful | ||
| - `Build site` check is successful | ||
| - `Terraform fmt / validate / plan` check is successful, or the PR does | ||
| not touch any files under `infra/terraform/` (the check only runs for | ||
| terraform-touching PRs, so a plain `check-success` condition would stall | ||
| every other PR) | ||
| - At least one approving review exists and is current for the latest push | ||
| - No active `CHANGES_REQUESTED` review exists | ||
| - All review conversations are resolved | ||
| - The pull request is not a draft | ||
| - The `blocked` label is not applied | ||
|
|
||
| Mergify processes one pull request at a time. It rebases the queued PR onto | ||
| the latest `dev` and runs CI again before merging, so the branch is always | ||
| tested against what is actually on `dev` at merge time. | ||
|
|
||
| `CI Summary` covers all jobs inside `ci.yml`. `CodeQL` and `Build site` run | ||
| in separate workflows (`codeql.yml` and `website.yml`) and are not part of | ||
| `CI Summary`, which is why they are listed explicitly above. The external | ||
| Semgrep app checks are deliberately not gated: their coverage duplicates the | ||
| `SAST (Semgrep)` job already in `CI Summary`, and gating third-party app | ||
| checks would stall the queue if the app is ever uninstalled or its plan | ||
| changes. | ||
|
|
||
| ## Keeping a PR out of the queue | ||
|
|
||
| To prevent a PR from entering the queue while it is still in progress, either: | ||
|
|
||
| - Open it as a **draft**. Mergify will not queue it until you mark it ready | ||
| for review. | ||
| - Apply the **`blocked` label**. This removes the PR from queue eligibility | ||
| and also prevents it from merging even if it is already in the queue. | ||
| Remove the label when the PR is ready to proceed. | ||
|
|
||
| ## Declaring dependencies | ||
|
|
||
| If your PR depends on another PR or issue merging first, declare it in the | ||
| pull request description: | ||
|
|
||
| ``` | ||
| Depends-On: #123 | ||
| ``` | ||
|
|
||
| Use one `Depends-On:` line per dependency. The PR can enter the queue | ||
| immediately, but Mergify holds it there until every declared dependency has | ||
| merged. Once all dependencies are satisfied, Mergify re-evaluates the PR | ||
| against the latest `dev` state and proceeds. | ||
|
|
||
| Leave the placeholder as `none` when there are no dependencies: | ||
|
|
||
| ``` | ||
| Depends-On: none | ||
| ``` | ||
|
|
||
| Do not delete the section. It makes dependency state visible to reviewers. | ||
|
|
||
| If a dependency PR is closed without merging, Mergify will hold your PR | ||
| indefinitely. To unblock it, edit the PR description and remove or replace | ||
| the `Depends-On:` line for that closed PR, then update the branch to | ||
| trigger re-evaluation. | ||
|
|
||
| ## awaiting-author label and inactive-author process | ||
|
|
||
| Mergify applies `awaiting-author` automatically when a reviewer submits a | ||
| `CHANGES_REQUESTED` review. No manual step is needed. | ||
|
|
||
| If the author remains inactive after the label is applied: | ||
|
|
||
| 1. After five working days, a maintainer notes the inactivity in the PR | ||
| and may take over the branch, open a replacement PR, or remove a | ||
| dependency that review confirms is unnecessary. | ||
| 2. The decision is recorded in the PR before changing ownership or | ||
| dependency state. | ||
|
|
||
| ## Approval freshness | ||
|
|
||
| When a contributor pushes a new commit to a dev-targeted PR, all existing | ||
| approvals are dismissed automatically. `CHANGES_REQUESTED` reviews are NOT | ||
| dismissed: a reviewer's objection survives the push and continues to block | ||
| `#changes-requested-reviews-by=0` until the reviewer themselves re-reviews | ||
| and clears it, or a maintainer manually dismisses it via GitHub (see | ||
| inactive-reviewer process below). | ||
|
|
||
| This means every approval in `merge_conditions` always reflects the code | ||
| that will actually land on `dev`, and no blocking concern can be erased by | ||
| a push followed by a third-party approval. | ||
|
|
||
| If your PR gets rebased while waiting in the queue, expect your approval to | ||
| be dismissed and the PR to return to "needs review" state before it can | ||
| re-enter the queue. | ||
|
|
||
| **For reviewers:** use inline conversation threads for every blocking | ||
| concern, not just the review summary body. Inline threads survive approval | ||
| dismissal and block independently via `#review-threads-unresolved=0`, so | ||
| your concern is protected even if another reviewer later approves. | ||
|
|
||
| ## Inactive-reviewer process | ||
|
|
||
| If a reviewer left `CHANGES_REQUESTED` and has not responded after the | ||
| contributor pushed a fix: | ||
|
|
||
| 1. After three working days with no response, the contributor tags the | ||
| reviewer and a maintainer in the PR comments. | ||
| 2. If there is still no response after two more working days, a maintainer | ||
| reads the original review, verifies that the fix addresses the concern, | ||
| and manually dismisses the stale review via GitHub with a comment | ||
| recording what was checked and why the fix is accepted. | ||
| 3. The decision is recorded in the PR before any dismissal so the reasoning | ||
| is auditable. | ||
|
|
||
| ## GitHub branch protection | ||
|
|
||
| Mergify does not replace or weaken GitHub branch protection. It operates | ||
| on top of it. Any protection rules set directly in GitHub repository | ||
| settings remain the source of truth and are enforced independently of | ||
| Mergify. | ||
|
|
||
| ## Pausing the queue | ||
|
|
||
| If the queue merges a PR that bypasses a protection or produces unexpected | ||
| behavior, a maintainer pauses queue operation immediately and records the | ||
| incident in issue #334 before resuming. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: the merge queue currently requires
CI Summary, but it does not require the separateBuild sitecheck introduced by.github/workflows/website.yml.The existing
CI Summarycovers the older website script test, not the Astro build and rendered-site verification. As a result, a PR could satisfy all configured queue conditions while the website build is failing.Please either add
Build siteas an explicit merge condition or make the website build part ofCI Summary. Please also add validation confirming that a failing website build prevents queue entry or merge.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was working on it the pr was drift also commeted not to do review now anyway i have fix everything i believe now can u re - review it please. Thank u