From 7262b2d788c5e178320b5bfe23b2a943b49a873a Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 6 Sep 2026 02:06:27 +0100 Subject: [PATCH 01/11] infra: add Mergify serial merge queue for dev branch Adds .mergify.yml with a conservative serial queue configuration for the dev branch. One PR at a time, no batch or parallel queues, no automatic conflict resolution, no bypass of required checks or reviews. A PR enters the queue only when CI Summary, DCO sign-off, and dependency-review all pass, at least one current approval exists, no CHANGES_REQUESTED review is active, all review conversations are resolved, the PR is not a draft, and all Depends-On dependencies have merged. Mergify automatically labels awaiting-author when changes are requested and removes it when change requests are resolved. Also adds docs/merge-queue.md with author guidance on declaring dependencies, the inactive-author process, and queue pausing. Updates the PR template with a Dependencies section. Closes #334 Signed-off-by: Tanvir Farhad --- .github/PULL_REQUEST_TEMPLATE.md | 5 ++ .mergify.yml | 69 ++++++++++++++++++++++++++++ docs/merge-queue.md | 79 ++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 .mergify.yml create mode 100644 docs/merge-queue.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e615bcac..325d49c3 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -25,6 +25,11 @@ ## Related issue Closes # +## Dependencies + + +Depends-On: none + ## Checklist - [ ] Every commit includes a DCO `Signed-off-by` trailer (`git commit -s`; see `docs/dco.md`) - [ ] My code follows the rule template in CONTRIBUTING.md diff --git a/.mergify.yml b/.mergify.yml new file mode 100644 index 00000000..b88318aa --- /dev/null +++ b/.mergify.yml @@ -0,0 +1,69 @@ +# .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 + +queue_rules: + - name: default + # Serial queue: one pull request processed at a time. + # No batch merging, no parallel queue, no automatic conflict resolution. + speculative_checks: 1 + batch_size: 1 + + # These conditions must hold at the moment Mergify merges the PR. + # Mergify re-evaluates them after rebasing onto the current dev HEAD. + merge_conditions: + - check-success=CI Summary + - check-success=DCO sign-off + - check-success=dependency-review + - "#approved-reviews-by>=1" + - "#changes-requested-reviews-by=0" + - "#review-threads-unresolved=0" + - -draft + +pull_request_rules: + # Queue an eligible PR automatically once all conditions are met. + # The PR is rebased onto the latest dev HEAD before merge so CI runs + # against the actual post-merge state. + - name: add to merge queue when eligible + conditions: + - base=dev + - check-success=CI Summary + - check-success=DCO sign-off + - check-success=dependency-review + - "#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. + # This makes blocked PRs visible in the issue tracker without any + # manual triage step. + - 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 diff --git a/docs/merge-queue.md b/docs/merge-queue.md new file mode 100644 index 00000000..07503111 --- /dev/null +++ b/docs/merge-queue.md @@ -0,0 +1,79 @@ +# 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. + +## Eligibility + +A pull request enters the queue automatically when all of the following +are true: + +- `CI Summary` check is successful (all GitHub Actions CI jobs passed) +- `DCO sign-off` check is successful +- `dependency-review` check is successful +- 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 +- All declared dependencies have merged (see below) + +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. + +## 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. Mergify holds the PR in the queue +until every declared dependency is merged, then re-evaluates it against the +latest `dev` state. + +Replace the placeholder in the PR template with `none` when there are no +dependencies: + +``` +Depends-On: none +``` + +Do not delete the section. It makes dependency state visible to reviewers. + +## awaiting-author label + +Mergify applies the `awaiting-author` label automatically when a reviewer +submits a `CHANGES_REQUESTED` review. The label is removed automatically +when all change requests are resolved. + +Maintainers use this label to identify PRs that are blocked on the author +rather than on review availability. + +## Inactive-author process + +1. A maintainer applies `awaiting-author` after required changes are + requested and the author is inactive for several days. +2. After five working days of inactivity, a maintainer may take over the + branch, open a replacement PR, or remove a dependency that review + confirms is unnecessary. +3. The decision is recorded in the PR before changing ownership or + dependency state. + +## 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. From 00ebfb507299b5fca34c183d880e677c5719554b Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 6 Sep 2026 02:17:39 +0100 Subject: [PATCH 02/11] docs: fix merge-queue.md review findings - State explicitly that the queue only applies to PRs targeting dev - Add app-not-installed caveat at the top - Merge awaiting-author and inactive-author sections to remove the contradiction where step 1 implied manual label application (Mergify applies it automatically on CHANGES_REQUESTED) - Add escape hatch for closed dependency PRs - Fix 'replace the placeholder' wording (default is already none, authors leave it as-is) Signed-off-by: Tanvir Farhad --- docs/merge-queue.md | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 07503111..e441ae80 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -5,11 +5,19 @@ 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 (all GitHub Actions CI jobs passed) - `DCO sign-off` check is successful - `dependency-review` check is successful @@ -37,8 +45,7 @@ Use one `Depends-On:` line per dependency. Mergify holds the PR in the queue until every declared dependency is merged, then re-evaluates it against the latest `dev` state. -Replace the placeholder in the PR template with `none` when there are no -dependencies: +Leave the placeholder as `none` when there are no dependencies: ``` Depends-On: none @@ -46,23 +53,22 @@ Depends-On: none Do not delete the section. It makes dependency state visible to reviewers. -## awaiting-author label +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. -Mergify applies the `awaiting-author` label automatically when a reviewer -submits a `CHANGES_REQUESTED` review. The label is removed automatically -when all change requests are resolved. +## awaiting-author label and inactive-author process -Maintainers use this label to identify PRs that are blocked on the author -rather than on review availability. +Mergify applies `awaiting-author` automatically when a reviewer submits a +`CHANGES_REQUESTED` review. No manual step is needed. -## Inactive-author process +If the author remains inactive after the label is applied: -1. A maintainer applies `awaiting-author` after required changes are - requested and the author is inactive for several days. -2. After five working days of inactivity, a maintainer may take over the - branch, open a replacement PR, or remove a dependency that review - confirms is unnecessary. -3. The decision is recorded in the PR before changing ownership or +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. ## GitHub branch protection From 66c5c164b67214517a1c392f78b6e876ab98aa85 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 6 Sep 2026 02:29:30 +0100 Subject: [PATCH 03/11] fix(mergify): remove invalid speculative_checks, add queue_conditions and dismiss_reviews - Remove speculative_checks (not a valid queue_rules field per Mergify docs) - Add top-level merge_queue.max_parallel_checks: 1 for serial guarantee - Add queue_conditions: [base=dev] so manual @mergifyio queue obeys the same branch restriction as the auto-queue rule - Add dismiss_reviews rule to invalidate stale approvals on each new push, ensuring merge_conditions approval reflects the latest code - Update docs/merge-queue.md with Approval freshness section explaining the dismiss behaviour and its effect on rebased PRs Signed-off-by: Tanvir Farhad --- .mergify.yml | 33 ++++++++++++++++++++++++++++----- docs/merge-queue.md | 14 ++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index b88318aa..f0536edc 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -8,11 +8,21 @@ # # 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 - # Serial queue: one pull request processed at a time. - # No batch merging, no parallel queue, no automatic conflict resolution. - speculative_checks: 1 + # 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 # These conditions must hold at the moment Mergify merges the PR. @@ -27,6 +37,21 @@ queue_rules: - -draft pull_request_rules: + # Dismiss approvals when a new commit is pushed to a dev-targeted PR. + # This ensures the approval in merge_conditions always reflects the + # latest push and cannot be satisfied by a pre-push review. + # Note: this rule fires on any push, including bot-created rebases. + # Reviewers must re-approve after each push before the PR can re-enter + # the queue. + - name: dismiss stale approvals on new push + conditions: + - base=dev + - "#approved-reviews-by>=1" + actions: + dismiss_reviews: + approved: true + message: "A new commit was pushed. Please re-review before this PR can merge." + # Queue an eligible PR automatically once all conditions are met. # The PR is rebased onto the latest dev HEAD before merge so CI runs # against the actual post-merge state. @@ -46,8 +71,6 @@ pull_request_rules: name: default # Apply awaiting-author when a reviewer has requested changes. - # This makes blocked PRs visible in the issue tracker without any - # manual triage step. - name: label awaiting-author when changes are requested conditions: - base=dev diff --git a/docs/merge-queue.md b/docs/merge-queue.md index e441ae80..92b4e196 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -71,6 +71,20 @@ If the author remains inactive after the label is applied: 2. The decision is recorded in the PR before changing ownership or dependency state. +## Approval freshness + +Approvals are dismissed automatically when a new commit is pushed to a +pull request targeting `dev`. This includes both author-pushed commits and +bot-created rebases (for example, when Mergify rebases your PR onto the +latest `dev` HEAD). + +After each dismissal, at least one reviewer must re-approve before Mergify +can queue or merge the PR. This means the approval in `merge_conditions` +always reflects the state of the actual code that will land on `dev`. + +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. + ## GitHub branch protection Mergify does not replace or weaken GitHub branch protection. It operates From efdc492fb4c2b5d0232fc9dc81a1e9991aca4543 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 6 Sep 2026 03:16:06 +0100 Subject: [PATCH 04/11] fix(mergify): address review feedback from ritiksah141 - Add -label=blocked to merge_conditions so a PR blocked after queuing cannot still merge through - Add comment in merge_conditions explaining CI Summary covers the Astro website build once PR #329 merges, satisfying the website gate concern - Fix docs/merge-queue.md: Depends-On is enforced at merge time (hold in queue), not at queue-entry time; reword bullet accordingly - Add 'Keeping a PR out of the queue' section documenting draft and blocked-label as intentional hold mechanisms - Update CI Summary note in eligibility section to reference #329 Signed-off-by: Tanvir Farhad --- .mergify.yml | 7 +++++++ docs/merge-queue.md | 23 +++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index f0536edc..cd0a17c5 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -27,6 +27,12 @@ queue_rules: # These conditions must hold at the moment Mergify merges the PR. # Mergify re-evaluates them after rebasing onto the current dev HEAD. + # + # CI Summary covers all required GitHub Actions jobs. Once PR #329 merges, + # it will also include the Astro website build and rendered-site verification + # ("Website (Astro build + verification)"), so no separate check-success + # condition is needed for the website gate. If maintainers later want an + # explicit condition, add "check-success=Build site" after #329 has merged. merge_conditions: - check-success=CI Summary - check-success=DCO sign-off @@ -35,6 +41,7 @@ queue_rules: - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" - -draft + - -label=blocked pull_request_rules: # Dismiss approvals when a new commit is pushed to a dev-targeted PR. diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 92b4e196..44c651a5 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -26,12 +26,26 @@ are true: - All review conversations are resolved - The pull request is not a draft - The `blocked` label is not applied -- All declared dependencies have merged (see below) 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 required GitHub Actions jobs. Once PR #329 merges, +it will also include the Astro website build and rendered-site verification, +so the website gate is satisfied through `CI Summary` with no separate +condition needed. + +## 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 @@ -41,9 +55,10 @@ pull request description: Depends-On: #123 ``` -Use one `Depends-On:` line per dependency. Mergify holds the PR in the queue -until every declared dependency is merged, then re-evaluates it against the -latest `dev` state. +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: From 53702ca87dff2c280f6cc690a0057befe0c429b6 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 6 Sep 2026 11:20:13 +0100 Subject: [PATCH 05/11] fix(mergify): gate CodeQL checks in merge_conditions CodeQL runs in codeql.yml, separate from ci.yml, so it is not covered by CI Summary. Without explicit gating a PR with a failing CodeQL scan could merge once Mergify is the only gate on dev. Add check-success=CodeQL, Analyze (python), and Analyze (javascript) to merge_conditions. All three run unconditionally on every dev-targeted PR so they carry no stall risk. Update docs/merge-queue.md eligibility section to list all required checks accurately and clarify the CI Summary vs CodeQL workflow split. Signed-off-by: Tanvir Farhad --- .mergify.yml | 12 +++++++----- docs/merge-queue.md | 13 ++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index cd0a17c5..669d6fa3 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -28,15 +28,17 @@ queue_rules: # These conditions must hold at the moment Mergify merges the PR. # Mergify re-evaluates them after rebasing onto the current dev HEAD. # - # CI Summary covers all required GitHub Actions jobs. Once PR #329 merges, - # it will also include the Astro website build and rendered-site verification - # ("Website (Astro build + verification)"), so no separate check-success - # condition is needed for the website gate. If maintainers later want an - # explicit condition, add "check-success=Build site" after #329 has merged. + # CI Summary aggregates the jobs inside ci.yml. Once PR #329 merges it + # also covers the Astro website build and rendered-site verification. + # CodeQL runs in a separate workflow (codeql.yml) and is NOT included in + # CI Summary, so it is gated explicitly below. 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) - "#approved-reviews-by>=1" - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 44c651a5..507e043b 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -18,9 +18,12 @@ 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 (all GitHub Actions CI jobs passed) +- `CI Summary` check is successful (aggregates all jobs in `ci.yml`) - `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 - At least one approving review exists and is current for the latest push - No active `CHANGES_REQUESTED` review exists - All review conversations are resolved @@ -31,10 +34,10 @@ 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 required GitHub Actions jobs. Once PR #329 merges, -it will also include the Astro website build and rendered-site verification, -so the website gate is satisfied through `CI Summary` with no separate -condition needed. +`CI Summary` covers all jobs inside `ci.yml`. Once PR #329 merges, it will +also include the Astro website build and rendered-site verification. CodeQL +runs in a separate workflow (`codeql.yml`) and is not part of `CI Summary`, +which is why it is listed explicitly above. ## Keeping a PR out of the queue From 98815faab7d921f010d32371dee3452f3285b986 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 7 Sep 2026 01:09:55 +0100 Subject: [PATCH 06/11] fix(mergify): complete check coverage, harden queue, fix dismiss policy Change 1 - Fix dismiss_reviews rule: - Remove #approved-reviews-by>=1 precondition so a stale CHANGES_REQUESTED review is cleared on the author's fix push even when zero approvals exist at that moment (offline-reviewer deadlock scenario) - Explicitly set changes_requested: true (was an implicit default, now declared to avoid hidden-default policy) - Update rule name and comments to describe the real behavior Change 2 - Complete check coverage and queue hardening: - Add check-success=Build site (website.yml runs unconditionally on every dev-targeted PR since PR #329 merged; no stall risk) - Add conditional terraform gate: passes when PR touches no infra/terraform/ files, otherwise requires check-success=Terraform fmt / validate / plan - Mirror all merge_conditions into the auto-queue entry conditions so only fully green PRs enter the single queue slot - Add checks_timeout: 2 hours so a check that never posts dequeues the stuck PR instead of blocking the entire queue indefinitely from day one - Add Semgrep non-gating rationale comment - Update CI Summary comment from future tense to present tense (PR #329 is now on dev) - Update docs/merge-queue.md: full check list, terraform conditional explained, Approval freshness section covers both approved and changes_requested dismissal, Semgrep decision documented Signed-off-by: Tanvir Farhad --- .mergify.yml | 72 +++++++++++++++++++++++++++++++++++---------- docs/merge-queue.md | 40 ++++++++++++++++--------- 2 files changed, 83 insertions(+), 29 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index 669d6fa3..aea34eda 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -25,13 +25,29 @@ queue_rules: # 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 the jobs inside ci.yml. Once PR #329 merges it - # also covers the Astro website build and rendered-site verification. - # CodeQL runs in a separate workflow (codeql.yml) and is NOT included in - # CI Summary, so it is gated explicitly below. + # 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 @@ -39,6 +55,14 @@ queue_rules: - 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" @@ -46,30 +70,48 @@ queue_rules: - -label=blocked pull_request_rules: - # Dismiss approvals when a new commit is pushed to a dev-targeted PR. - # This ensures the approval in merge_conditions always reflects the - # latest push and cannot be satisfied by a pre-push review. - # Note: this rule fires on any push, including bot-created rebases. - # Reviewers must re-approve after each push before the PR can re-enter - # the queue. - - name: dismiss stale approvals on new push + # Dismiss all stale reviews (both approvals and changes-requested) on + # every push to a dev-targeted PR. The approval precondition is + # intentionally absent: if it were present, a reviewer who requested + # changes and then went offline would leave a stale CHANGES_REQUESTED + # verdict that survives the author's fix push (zero approvals at that + # moment), permanently blocking #changes-requested-reviews-by=0 with + # no way to clear it except pushing again after approvals exist. + # Without the precondition, every author push clears all stale verdicts + # regardless of the current approval count. + # + # Dismissing a review does not resolve its comment threads. Unresolved + # threads continue to block via #review-threads-unresolved=0 until a + # human resolves them, and the reviewer can re-request changes after + # re-reviewing. This rule fires on author pushes and on Mergify's own + # queue rebases: a queued PR whose base moves is ejected and needs a + # fresh approval before it re-enters the queue. + - name: dismiss stale reviews on new push conditions: - base=dev - - "#approved-reviews-by>=1" actions: dismiss_reviews: approved: true - message: "A new commit was pushed. Please re-review before this PR can merge." + changes_requested: true + message: "A new commit was pushed. All reviews were dismissed. Please re-review before this PR can merge." # Queue an eligible PR automatically once all conditions are met. - # The PR is rebased onto the latest dev HEAD before merge so CI runs - # against the actual post-merge state. + # 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" diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 507e043b..4b1974fc 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -18,12 +18,18 @@ 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`) +- `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 @@ -34,10 +40,13 @@ 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`. Once PR #329 merges, it will -also include the Astro website build and rendered-site verification. CodeQL -runs in a separate workflow (`codeql.yml`) and is not part of `CI Summary`, -which is why it is listed explicitly above. +`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 @@ -91,17 +100,20 @@ If the author remains inactive after the label is applied: ## Approval freshness -Approvals are dismissed automatically when a new commit is pushed to a -pull request targeting `dev`. This includes both author-pushed commits and -bot-created rebases (for example, when Mergify rebases your PR onto the -latest `dev` HEAD). +Every push to a dev-targeted PR dismisses all existing reviews, both +approvals and `CHANGES_REQUESTED` verdicts. This applies to author-pushed +commits and to Mergify's own queue rebases (when Mergify rebases your PR +onto the latest `dev` HEAD before merging). After each dismissal, at least one reviewer must re-approve before Mergify -can queue or merge the PR. This means the approval in `merge_conditions` -always reflects the state of the actual code that will land on `dev`. - -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. +can queue or merge the PR. Dismissing a review does not resolve its comment +threads: unresolved threads continue to block via the +`#review-threads-unresolved=0` condition until a human resolves them, and a +reviewer can re-request changes after re-reviewing. + +If your PR gets rebased while waiting in the queue, expect all reviews to be +dismissed and the PR to return to "needs review" state before it can +re-enter the queue. ## GitHub branch protection From c889d410c25384397093d9b8c9fff30096c3d005 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 7 Sep 2026 18:25:00 +0100 Subject: [PATCH 07/11] fix(mergify): set changes_requested: false, document inactive-reviewer process CHANGES_REQUESTED reviews now survive contributor pushes. Auto-dismissing them would allow any push followed by a third-party approval to silently erase a blocking concern without the original reviewer seeing the fix. Since maintainers (Tanvir, Ritik) are consistently available, the right escape hatch for a truly stale blocking review is a conscious maintainer decision via GitHub, not automatic erasure. - Set changes_requested: false explicitly in dismiss_reviews action - Update rule name and comment to describe real behaviour - Add inline-thread guidance: reviewers must use threads for blocking concerns so #review-threads-unresolved=0 protects them independently - Add inactive-reviewer process to docs/merge-queue.md: 3+2 working day escalation path ending in maintainer-verified manual dismissal with an auditable comment in the PR Signed-off-by: Tanvir Farhad --- .mergify.yml | 34 ++++++++++++++++------------------ docs/merge-queue.md | 45 ++++++++++++++++++++++++++++++++------------- 2 files changed, 48 insertions(+), 31 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index aea34eda..cf563371 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -70,30 +70,28 @@ queue_rules: - -label=blocked pull_request_rules: - # Dismiss all stale reviews (both approvals and changes-requested) on - # every push to a dev-targeted PR. The approval precondition is - # intentionally absent: if it were present, a reviewer who requested - # changes and then went offline would leave a stale CHANGES_REQUESTED - # verdict that survives the author's fix push (zero approvals at that - # moment), permanently blocking #changes-requested-reviews-by=0 with - # no way to clear it except pushing again after approvals exist. - # Without the precondition, every author push clears all stale verdicts - # regardless of the current approval count. + # 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. # - # Dismissing a review does not resolve its comment threads. Unresolved - # threads continue to block via #review-threads-unresolved=0 until a - # human resolves them, and the reviewer can re-request changes after - # re-reviewing. This rule fires on author pushes and on Mergify's own - # queue rebases: a queued PR whose base moves is ejected and needs a - # fresh approval before it re-enters the queue. - - name: dismiss stale reviews on new push + # 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: true - message: "A new commit was pushed. All reviews were dismissed. Please re-review before this PR can merge." + 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 diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 4b1974fc..307238b7 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -100,21 +100,40 @@ If the author remains inactive after the label is applied: ## Approval freshness -Every push to a dev-targeted PR dismisses all existing reviews, both -approvals and `CHANGES_REQUESTED` verdicts. This applies to author-pushed -commits and to Mergify's own queue rebases (when Mergify rebases your PR -onto the latest `dev` HEAD before merging). - -After each dismissal, at least one reviewer must re-approve before Mergify -can queue or merge the PR. Dismissing a review does not resolve its comment -threads: unresolved threads continue to block via the -`#review-threads-unresolved=0` condition until a human resolves them, and a -reviewer can re-request changes after re-reviewing. - -If your PR gets rebased while waiting in the queue, expect all reviews to be -dismissed and the PR to return to "needs review" state before it can +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 From c66d098e3597cc4a2dea68b3a04a2f9606f1148b Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 14 Sep 2026 14:10:54 +0100 Subject: [PATCH 08/11] fix(mergify): require 2 approvals for governance-policy file changes GOVERNANCE.md requires project lead + one additional maintainer for changes to governance policy. Add an or-condition to both merge_conditions and queue entry conditions so PRs touching GOVERNANCE.md, .mergify.yml, or docs/merge-queue.md require at least 2 approvals before merging. Document the governance-policy gate and 3 validation scenarios in docs/merge-queue.md: - Blocking review with no inline thread, followed by unrelated push - Push after approval invalidates the stale approval - Governance-policy change with only one approval does not auto-merge Signed-off-by: Tanvir Farhad --- .mergify.yml | 10 +++++++ docs/merge-queue.md | 69 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/.mergify.yml b/.mergify.yml index cf563371..e04977ae 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -64,6 +64,13 @@ queue_rules: - -files~=^infra/terraform/ - check-success=Terraform fmt / validate / plan - "#approved-reviews-by>=1" + # Governance-policy files (GOVERNANCE.md, .mergify.yml, docs/merge-queue.md) + # require project lead + one additional maintainer per GOVERNANCE.md §5. + # This condition enforces that: either the PR does not touch governance + # files (one approval already satisfied above), or it has two approvals. + - or: + - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - "#approved-reviews-by>=2" - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" - -draft @@ -111,6 +118,9 @@ pull_request_rules: - -files~=^infra/terraform/ - check-success=Terraform fmt / validate / plan - "#approved-reviews-by>=1" + - or: + - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - "#approved-reviews-by>=2" - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" - -draft diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 307238b7..5c0ccfe8 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -31,6 +31,9 @@ are true: 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 +- If the PR touches `GOVERNANCE.md`, `.mergify.yml`, or `docs/merge-queue.md`: + at least two approving reviews (project lead + one additional maintainer, + per GOVERNANCE.md §5) - No active `CHANGES_REQUESTED` review exists - All review conversations are resolved - The pull request is not a draft @@ -134,6 +137,72 @@ contributor pushed a fix: 3. The decision is recorded in the PR before any dismissal so the reasoning is auditable. +## Governance-policy changes + +Changes to `GOVERNANCE.md`, `.mergify.yml`, or `docs/merge-queue.md` require +two approvals before they can merge through the queue. This matches the +project-lead plus one additional maintainer requirement in GOVERNANCE.md §5. + +The queue enforces this via the condition: + +``` +or: + - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - "#approved-reviews-by>=2" +``` + +For PRs that do not touch these files the left side is true and one approval +is sufficient. For PRs that do touch them, the left side is false, so two +approvals are required. + +Governance-policy PRs may not auto-merge with only one approval regardless of +how many other checks pass. They follow the same queue path as all other PRs; +no separate manual merge step is needed once two approvals are in place. + +### Validation scenarios + +The following scenarios describe expected queue behavior. They can be verified +against a Mergify dry-run or by inspecting queue state on a real PR. + +**Scenario 1: Blocking review with no inline thread, followed by unrelated push** + +A reviewer submits `CHANGES_REQUESTED` with the concern in the review summary +body only (no inline thread). A contributor then pushes a fix commit. + +Expected behavior: +- The `CHANGES_REQUESTED` review survives the push (`changes_requested: false` + in `dismiss_reviews` means only `APPROVED` reviews are dismissed). +- The `#changes-requested-reviews-by=0` condition remains unsatisfied. +- The PR cannot enter the queue or merge until the original reviewer re-reviews + and clears the `CHANGES_REQUESTED` verdict, or a maintainer dismisses it + following the inactive-reviewer process. +- A third party approving after the push does not clear the block. + +**Scenario 2: Push after approval invalidates the stale approval** + +A reviewer approves a PR. The contributor pushes one more commit. + +Expected behavior: +- Mergify dismisses the approval automatically (the `dismiss stale approvals + on new push` rule, `approved: true`). +- The PR exits the queue if it was already in it. +- `#approved-reviews-by>=1` becomes unsatisfied; the PR needs a fresh approval + before it can re-enter the queue. + +**Scenario 3: Governance-policy change with only one approval does not auto-merge** + +A PR modifies `.mergify.yml` or `GOVERNANCE.md` and receives exactly one +approving review, with all CI checks green. + +Expected behavior: +- The `or: [-files~=..., "#approved-reviews-by>=2"]` condition evaluates the + left side as false (the PR does touch governance files). +- The right side (`#approved-reviews-by>=2`) is false because only one approval + exists. +- The whole `or` condition is false; the PR does not enter the queue. +- A second approval from another maintainer or the project lead satisfies the + condition and allows the PR to proceed through the normal queue path. + ## GitHub branch protection Mergify does not replace or weaken GitHub branch protection. It operates From 911ab8c7d12d8adfc1e2b4070c89c355776facdd Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Wed, 16 Sep 2026 00:37:07 +0100 Subject: [PATCH 09/11] fix(mergify): correct phantom check names and guard dismiss rule against bot rebases - Remove check-success=Build site (no such check exists; frontend and website jobs are already aggregated inside CI Summary) - Remove check-success=CodeQL (workflow-level name, not a check run; Analyze (python) and Analyze (javascript) already gate both matrix jobs) - Add sender!=mergify[bot] to dismiss stale approvals rule so Mergify's own queue-rebase commits do not strip approvals mid-flight and cause an infinite dequeue/re-approve loop Signed-off-by: Tanvir Farhad --- .mergify.yml | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index e04977ae..39ecc2a0 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -35,13 +35,12 @@ queue_rules: # 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. + # CI Summary aggregates all jobs inside ci.yml, including the frontend + # build (Frontend (lint + build)) and website script tests (Website + # (script tests)) added by PR #329. CodeQL runs in a separate workflow + # (codeql.yml) and is NOT included in CI Summary; it posts two check + # runs named "Analyze (python)" and "Analyze (javascript)", both listed + # below. # # External Semgrep app checks (Semgrep OSS, semgrep-cloud-platform/scan) # are deliberately not gated: their coverage duplicates the SAST @@ -52,10 +51,8 @@ queue_rules: - 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 @@ -94,6 +91,9 @@ pull_request_rules: - name: dismiss stale approvals on new push conditions: - base=dev + # Exclude Mergify's own queue-rebase commits so they don't strip + # approvals mid-flight and cause an infinite dequeue/re-approve cycle. + - sender!=mergify[bot] actions: dismiss_reviews: approved: true @@ -110,10 +110,8 @@ pull_request_rules: - 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 From 4e4efe837cc55f2c463d0432ce76d9cd7a3066e7 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sat, 19 Sep 2026 12:53:06 +0100 Subject: [PATCH 10/11] fix(mergify): require project lead approval for governance paths and add MAINTAINERS.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add MAINTAINERS.md to the governed-path regex in both merge_conditions and the auto-queue rule; that file names the role-holders referenced by GOVERNANCE.md §5 and must carry the same protection - Require approved-reviews-by=Vishnu2707 alongside #approved-reviews-by>=2 so any two accounts can no longer satisfy the governance gate without the project lead; previously any two maintainers could approve a governance change without the project lead being involved - Remove stale CodeQL and Build site entries from docs/merge-queue.md eligibility list; those checks were removed from the config in an earlier commit and the docs were not updated at the same time - Correct the CI Summary explanation paragraph to describe what is actually gated: the Analyze (python) and Analyze (javascript) CodeQL job checks, not a workflow-level CodeQL check Signed-off-by: Tanvir Farhad --- .mergify.yml | 19 ++++++++++++------- docs/merge-queue.md | 22 ++++++++++------------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index 39ecc2a0..7b6be632 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -61,13 +61,16 @@ queue_rules: - -files~=^infra/terraform/ - check-success=Terraform fmt / validate / plan - "#approved-reviews-by>=1" - # Governance-policy files (GOVERNANCE.md, .mergify.yml, docs/merge-queue.md) - # require project lead + one additional maintainer per GOVERNANCE.md §5. + # Governance-policy files require project lead (@Vishnu2707) + one + # additional maintainer per GOVERNANCE.md §5. MAINTAINERS.md is included + # because it names the role-holders referenced by that policy. # This condition enforces that: either the PR does not touch governance - # files (one approval already satisfied above), or it has two approvals. + # files, or it has two approvals AND the project lead has approved. - or: - - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ - - "#approved-reviews-by>=2" + - -files~=^(GOVERNANCE\.md|MAINTAINERS\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - and: + - "#approved-reviews-by>=2" + - approved-reviews-by=Vishnu2707 - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" - -draft @@ -117,8 +120,10 @@ pull_request_rules: - check-success=Terraform fmt / validate / plan - "#approved-reviews-by>=1" - or: - - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ - - "#approved-reviews-by>=2" + - -files~=^(GOVERNANCE\.md|MAINTAINERS\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - and: + - "#approved-reviews-by>=2" + - approved-reviews-by=Vishnu2707 - "#changes-requested-reviews-by=0" - "#review-threads-unresolved=0" - -draft diff --git a/docs/merge-queue.md b/docs/merge-queue.md index 5c0ccfe8..bc1237e4 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -22,18 +22,16 @@ are true: 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 -- If the PR touches `GOVERNANCE.md`, `.mergify.yml`, or `docs/merge-queue.md`: - at least two approving reviews (project lead + one additional maintainer, - per GOVERNANCE.md §5) +- If the PR touches `GOVERNANCE.md`, `MAINTAINERS.md`, `.mergify.yml`, or + `docs/merge-queue.md`: at least two approving reviews, and the project + lead (`@Vishnu2707`) must be one of the approvers (per GOVERNANCE.md §5) - No active `CHANGES_REQUESTED` review exists - All review conversations are resolved - The pull request is not a draft @@ -43,13 +41,13 @@ 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. +`CI Summary` covers all jobs inside `ci.yml`. `Analyze (python)` and +`Analyze (javascript)` are the CodeQL per-job checks from `codeql.yml`; +they are gated separately because `codeql.yml` is a different workflow and +is not included in `CI Summary`. 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 From 2239867a9c5599a6d472ad53e91ed09c7804b80a Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 20 Sep 2026 00:58:50 +0100 Subject: [PATCH 11/11] docs: update governance-policy section to match actual Mergify condition Add MAINTAINERS.md to the governed path set, update the code block to show the nested and condition requiring approved-reviews-by=Vishnu2707 alongside two approvals, and fix Scenario 3 to describe the actual enforcement logic. Signed-off-by: Tanvir Farhad --- docs/merge-queue.md | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/docs/merge-queue.md b/docs/merge-queue.md index bc1237e4..e62d5f77 100644 --- a/docs/merge-queue.md +++ b/docs/merge-queue.md @@ -137,25 +137,32 @@ contributor pushed a fix: ## Governance-policy changes -Changes to `GOVERNANCE.md`, `.mergify.yml`, or `docs/merge-queue.md` require -two approvals before they can merge through the queue. This matches the -project-lead plus one additional maintainer requirement in GOVERNANCE.md §5. +Changes to `GOVERNANCE.md`, `MAINTAINERS.md`, `.mergify.yml`, or +`docs/merge-queue.md` require two approvals before they can merge through the +queue, and the project lead (`@Vishnu2707`) must be one of the two approvers. +This matches the project-lead plus one additional maintainer requirement in +GOVERNANCE.md §5. `MAINTAINERS.md` is included because it names the +role-holders referenced by that policy. The queue enforces this via the condition: ``` or: - - -files~=^(GOVERNANCE\.md|\.mergify\.yml|docs/merge-queue\.md)$ - - "#approved-reviews-by>=2" + - -files~=^(GOVERNANCE\.md|MAINTAINERS\.md|\.mergify\.yml|docs/merge-queue\.md)$ + - and: + - "#approved-reviews-by>=2" + - approved-reviews-by=Vishnu2707 ``` For PRs that do not touch these files the left side is true and one approval -is sufficient. For PRs that do touch them, the left side is false, so two -approvals are required. +is sufficient. For PRs that do touch them, the left side is false, so both +conditions on the right must be satisfied: at least two approvals, and +`@Vishnu2707` must be one of the approvers. -Governance-policy PRs may not auto-merge with only one approval regardless of -how many other checks pass. They follow the same queue path as all other PRs; -no separate manual merge step is needed once two approvals are in place. +Governance-policy PRs may not auto-merge with only one approval, or with two +approvals that do not include the project lead, regardless of how many other +checks pass. They follow the same queue path as all other PRs; no separate +manual merge step is needed once the two-approval-plus-lead condition is met. ### Validation scenarios @@ -193,13 +200,13 @@ A PR modifies `.mergify.yml` or `GOVERNANCE.md` and receives exactly one approving review, with all CI checks green. Expected behavior: -- The `or: [-files~=..., "#approved-reviews-by>=2"]` condition evaluates the - left side as false (the PR does touch governance files). -- The right side (`#approved-reviews-by>=2`) is false because only one approval - exists. +- The left side of the `or` condition is false (the PR touches governance files). +- The right side requires both `#approved-reviews-by>=2` and + `approved-reviews-by=Vishnu2707`. With only one approval, neither sub-condition + is satisfied. - The whole `or` condition is false; the PR does not enter the queue. -- A second approval from another maintainer or the project lead satisfies the - condition and allows the PR to proceed through the normal queue path. +- The PR can only proceed once it has at least two approvals and `@Vishnu2707` + is one of the approvers. ## GitHub branch protection