infra: add Mergify serial merge queue for dev branch

[TFT444]

What does this PR do?

Introduces a conservative Mergify serial merge queue for the dev branch, as specified in #334.

Type of change

• New scan rule
• Remediation playbook
• Bug fix
• Dashboard/front-end work
• API endpoint
• Documentation
• Compliance mapping

Changes

.mergify.yml (new)

• Top-level merge_queue.max_parallel_checks: 1 enforces serial operation globally
• queue_rules.queue_conditions: [base=dev] restricts queue entry for both automatic rules and manual @mergifyio queue commands
• queue_rules.batch_size: 1 merges one PR at a time
• Merge conditions: CI Summary + DCO sign-off + dependency-review all pass, at least one current approval, no CHANGES_REQUESTED, all threads resolved, not draft
• dismiss_reviews rule invalidates approvals after each push so merge_conditions always reflects the latest code
• Auto-labels awaiting-author when changes are requested; removes it when resolved
• File is inert until the Mergify GitHub App is installed and authorized

.github/PULL_REQUEST_TEMPLATE.md (modified)

• Adds a ## Dependencies section with Depends-On: none placeholder

docs/merge-queue.md (new)

• Explains eligibility conditions and base=dev scope
• Explains Depends-On syntax and how Mergify holds PRs until dependencies merge
• Documents approval freshness behaviour (dismissal on push, including bot rebases)
• Documents awaiting-author label lifecycle
• Documents the five-working-day inactive-author process
• Notes that GitHub branch protection remains the source of truth

Pre-activation requirement

The awaiting-author label has been created in the repository. The Mergify GitHub App must be installed and authorized (limited to this repository only) by an organization owner before the queue becomes active. This PR only prepares the configuration.

Branch protection note

The dev branch has no GitHub branch protection rules configured. Mergify will act as the primary merge gate once the app is installed. If branch protection rules are added later, Mergify will respect them and will not bypass them.

Related issue

Related: #334

Dependencies

Depends-On: none

Checklist

• Every commit includes a DCO Signed-off-by trailer
• No tokens, secrets, or credentials added
• No existing CI workflows modified
• No unrelated files modified
• YAML syntax validated
• awaiting-author label created in repository

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[TFT444]

Still under processing so no need for the review I will ask for it.

Thank You

[parthrohit22]

I reviewed the Mergify configuration, PR template updates, and merge-queue documentation.

The configuration is generally clear and conservative. However, the merge queue currently requires CI Summary but does not explicitly require the separate website Build site workflow. This could allow a PR to merge while the Astro build or rendered-site verification is failing.

Please connect the website build to the required merge gate before approval.

[parthrohit22]

Blocking: the merge queue currently requires CI Summary, but it does not require the separate Build site check introduced by .github/workflows/website.yml.

The existing CI Summary covers 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 site as an explicit merge condition or make the website build part of CI Summary. Please also add validation confirming that a failing website build prevents queue entry or merge.

[TFT444]

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

[TFT444]

@ritiksah141 @parthrohit22 now do the review please it ready.

[parthrohit22]

I re-reviewed the latest head and the changes made after the previous review.

The earlier merge-safety concern has been addressed. Website validation is now included in the OpenShield CI Summary, queue execution is restricted to the dev branch, speculative checks are limited to one at a time, and approvals are dismissed when new commits are pushed.

The approval-freshness behaviour is also documented clearly in the merge-queue documentation.

I found no remaining blocking issues in the current revision.

Approving this PR, subject to the outstanding review thread being resolved and the required CI checks passing.

[ritiksah141]

Thanks for the fixes in the last two commits. Removing the invalid speculative_checks key and adding queue_conditions plus the dismiss_reviews rule were the right moves, and the configuration is otherwise technically sound. I reviewed the full diff end to end, validated every key and action against the current Mergify documentation, and cross-checked the referenced CI checks, labels, and sign-offs. One unresolved item blocks approval, and there are a few smaller fixes.

Blocking: the website build gate raised by @parthrohit22 is not addressed

The earlier change request asked for the Astro website build to be part of the merge gate. The fix commit addresses configuration validity only. The merge conditions still gate solely on CI Summary, DCO sign-off, and dependency-review, and neither the config nor the docs answer the concern.

Note that adding check-success=Build site as-is would be unsafe. That check only exists once PR #329 merges its new website.yml. Until then no PR can ever produce it, and Mergify waits forever for a missing check, so the queue would deadlock on dev.

There is a clean path, because #329 already folds the Astro build into CI Summary: its ci.yml change turns the website job into "Website (Astro build + verification)" running npm run check (build, CMS configuration, and rendered-site verification, run for both CMS variants), and that job stays in the CI Summary needs list. That covers everything Build site verifies on pull requests. So the second option from the earlier review, making the website build part of CI Summary, is satisfied as soon as #329 merges.

Please resolve the thread with one of:

1. Add a short note in .mergify.yml and docs/merge-queue.md stating that Astro build and rendered-site verification are gated through CI Summary once #329 merges, and commit to a follow-up that adds an explicit check-success=Build site condition after #329 lands, if the maintainers still want the belt-and-braces check. Or:
2. Add check-success=Build site in this PR now, together with a Depends-On: #329 declaration in the PR description, so this change can only take effect after the check exists.

Either way, the earlier review needs a substantive answer in this PR.

Requested changes

1. Add -label=blocked to merge_conditions. It is present in the auto-queue conditions, but a PR that receives the blocked label after it is already queued would still merge.

2. Fix the eligibility bullet "All declared dependencies have merged" in docs/merge-queue.md. Mergify enforces Depends-On at merge time by holding the queued PR, not at queue entry. This bullet currently contradicts the dependencies section, which describes the hold correctly.

3. Add a short note in docs/merge-queue.md explaining how an author can keep an eligible PR out of the queue on purpose: open it as a draft, or apply the blocked label. Right now blocked appears only as an eligibility condition with no explanation of its purpose.

Policy question for the maintainers

CodeQL runs on every PR but is not in merge_conditions. Since dev has no GitHub branch protection, Mergify will be the only merge gate once the app is installed. Please decide whether check-success=CodeQL should be required as well. Not a blocker on this PR, but it should be a conscious choice.

Confirmed correct, no action needed

• YAML parses cleanly and every key is valid against the current Mergify documentation: merge_queue.max_parallel_checks, queue_rules.queue_conditions, batch_size, the check-success, #approved-reviews-by, #changes-requested-reviews-by, #review-threads-unresolved, -draft, and -label conditions, and the dismiss_reviews, label, and queue actions.
• max_parallel_checks: 1 enforces serial processing, and serial mode still tests each PR against the merged dev state before merging, which is what #334 asks for.
• Depends-On is natively supported by Mergify, the bare #123 syntax in the template is valid, and Depends-On: none is safely ignored.
• All three gated checks run unconditionally on every PR targeting dev, so the conditions as configured today cannot stall the queue.
• The awaiting-author and blocked labels exist, all commits are signed off, and all CI checks on this PR pass.
• The approval freshness behavior is strict: because dismiss_reviews fires on every push, including Mergify's own queue rebases, a queued PR whose base moves is ejected and needs re-approval before it re-enters. In an active repo this means roughly two approval cycles per PR. The docs disclose this honestly and #334 requires approval freshness, so it is acceptable, but the maintainers should accept this operational cost knowingly.

[TFT444]

@

Thanks for the fixes in the last two commits. Removing the invalid speculative_checks key and adding queue_conditions plus the dismiss_reviews rule were the right moves, and the configuration is otherwise technically sound. I reviewed the full diff end to end, validated every key and action against the current Mergify documentation, and cross-checked the referenced CI checks, labels, and sign-offs. One unresolved item blocks approval, and there are a few smaller fixes.

Blocking: the website build gate raised by @parthrohit22 is not addressed

The earlier change request asked for the Astro website build to be part of the merge gate. The fix commit addresses configuration validity only. The merge conditions still gate solely on CI Summary, DCO sign-off, and dependency-review, and neither the config nor the docs answer the concern.

Note that adding check-success=Build site as-is would be unsafe. That check only exists once PR #329 merges its new website.yml. Until then no PR can ever produce it, and Mergify waits forever for a missing check, so the queue would deadlock on dev.

There is a clean path, because #329 already folds the Astro build into CI Summary: its ci.yml change turns the website job into "Website (Astro build + verification)" running npm run check (build, CMS configuration, and rendered-site verification, run for both CMS variants), and that job stays in the CI Summary needs list. That covers everything Build site verifies on pull requests. So the second option from the earlier review, making the website build part of CI Summary, is satisfied as soon as #329 merges.

Please resolve the thread with one of:

1. Add a short note in .mergify.yml and docs/merge-queue.md stating that Astro build and rendered-site verification are gated through CI Summary once website: migrate to Astro + Decap CMS with GitHub Pages pipeline #329 merges, and commit to a follow-up that adds an explicit check-success=Build site condition after website: migrate to Astro + Decap CMS with GitHub Pages pipeline #329 lands, if the maintainers still want the belt-and-braces check. Or:
2. Add check-success=Build site in this PR now, together with a Depends-On: #329 declaration in the PR description, so this change can only take effect after the check exists.

Either way, the earlier review needs a substantive answer in this PR.

Requested changes

1. Add -label=blocked to merge_conditions. It is present in the auto-queue conditions, but a PR that receives the blocked label after it is already queued would still merge.
2. Fix the eligibility bullet "All declared dependencies have merged" in docs/merge-queue.md. Mergify enforces Depends-On at merge time by holding the queued PR, not at queue entry. This bullet currently contradicts the dependencies section, which describes the hold correctly.
3. Add a short note in docs/merge-queue.md explaining how an author can keep an eligible PR out of the queue on purpose: open it as a draft, or apply the blocked label. Right now blocked appears only as an eligibility condition with no explanation of its purpose.

Policy question for the maintainers

CodeQL runs on every PR but is not in merge_conditions. Since dev has no GitHub branch protection, Mergify will be the only merge gate once the app is installed. Please decide whether check-success=CodeQL should be required as well. Not a blocker on this PR, but it should be a conscious choice.

Confirmed correct, no action needed

• YAML parses cleanly and every key is valid against the current Mergify documentation: merge_queue.max_parallel_checks, queue_rules.queue_conditions, batch_size, the check-success, #approved-reviews-by, #changes-requested-reviews-by, #review-threads-unresolved, -draft, and -label conditions, and the dismiss_reviews, label, and queue actions.
• max_parallel_checks: 1 enforces serial processing, and serial mode still tests each PR against the merged dev state before merging, which is what infra: adopt Mergify daily merge queue and PR dependency workflow #334 asks for.
• Depends-On is natively supported by Mergify, the bare #123 syntax in the template is valid, and Depends-On: none is safely ignored.
• All three gated checks run unconditionally on every PR targeting dev, so the conditions as configured today cannot stall the queue.
• The awaiting-author and blocked labels exist, all commits are signed off, and all CI checks on this PR pass.
• The approval freshness behavior is strict: because dismiss_reviews fires on every push, including Mergify's own queue rebases, a queued PR whose base moves is ejected and needs re-approval before it re-enters. In an active repo this means roughly two approval cycles per PR. The docs disclose this honestly and infra: adopt Mergify daily merge queue and PR dependency workflow #334 requires approval freshness, so it is acceptable, but the maintainers should accept this operational cost knowingly.

@ritiksah141 Re-review, please. All of them fixed

[ritiksah141]

Thanks for the latest commit. The earlier items are all addressed: -label=blocked is now in merge_conditions, the Depends-On timing wording is fixed, the opt-out section is added, and the website gate coordination with #329 is documented in both the config and docs/merge-queue.md.

One change is still required before approval: merge_conditions must list every required check, not just the three it has today.

Requested change: gate all required checks in merge_conditions

dev has no GitHub branch protection, so once the Mergify app is installed this file is the only merge gate. Any check that is not listed in merge_conditions can fail and the queue will still merge. Today that gap is CodeQL:

• CI Summary only aggregates the jobs inside ci.yml. CodeQL runs in a separate workflow, codeql.yml, so it is not covered by CI Summary.
• A PR that introduces a vulnerability CodeQL classifies as Error level, or Critical/High security severity, gets a failing CodeQL check, but nothing in the current configuration looks at it, so the queue would merge it.

Please add these conditions to merge_conditions:

- check-success=CodeQL
- check-success=Analyze (python)
- check-success=Analyze (javascript)

Why all three:

• CodeQL is the code scanning check run. It is the one that goes red when serious findings are reported, so it is the condition that actually blocks vulnerable code.
• Analyze (python) and Analyze (javascript) are the workflow jobs themselves. Gating on them guarantees the analysis ran and succeeded on the PR instead of silently not producing results.

All three checks run unconditionally on every PR targeting dev, so they carry no stall risk of the kind discussed for Build site.

Also update the eligibility section of docs/merge-queue.md to list the required checks accurately. The current text says CI Summary covers all GitHub Actions CI jobs, which stops being accurate once CodeQL is gated separately.

Notes, no action needed

• terraform-plan cannot be gated this way. It only runs for PRs that touch infra/terraform/**, so an unconditional check-success condition would stall every other PR forever. Leave it out.
• The external Semgrep checks do not need gating. Semgrep already runs inside ci.yml and is covered by CI Summary. Gating the third-party app checks would add fragility without extra coverage.
• Optional hardening to consider: set checks_timeout on the queue rule, for example checks_timeout: 2 hours, so a check that never posts cannot stall the queue indefinitely as the gated list grows. Your call.

Once merge_conditions covers the CodeQL checks and the docs are updated, this is ready to approve from my side.

[TFT444]

@ritiksah141 pushed a new commit (447bb6e) that adds check-success=CodeQL, Analyze (python), and Analyze (javascript) to merge_conditions. The docs eligibility section is updated too, so it accurately reflects that CodeQL is separate from CI Summary.

All the items from your last review should be covered now. Would appreciate another look when you get a chance!

[ritiksah141]

The CodeQL gating in the latest commit is correct: all three checks are in merge_conditions, the docs eligibility list is accurate, and terraform-plan and the external Semgrep checks were correctly left out of a plain check-success gate. Two further changes are needed to make the configuration robust: the review-dismissal policy must be made explicit and order-independent, and check coverage plus queue hardening must be completed.

Update since this review was first posted: PR #329 has merged into dev (4706319). The Build site check now exists and runs on every PR targeting dev with no paths filter, and CI Summary already includes the Astro website job. That changes the website gating picture and is folded into Change 2 below.

Change 1: make the push-dismissal policy explicit and complete

The current dismiss_reviews rule has two problems.

First, it only fires when #approved-reviews-by>=1 at push time. Consider the common flow: a reviewer requests changes, goes offline for weeks, the author pushes fixes (zero approvals at that moment, so the rule does not fire and the stale review survives), and then two other reviewers approve. The PR is now hard-blocked forever by #changes-requested-reviews-by=0 despite being fixed and doubly approved. The stale verdict is only cleared if another push happens after approvals exist, which nobody would know to do.

Second, the rule silently dismisses changes-requested reviews even today. In Mergify's dismiss_reviews action, changes_requested defaults to true, so the current config already dismisses both review types whenever it fires, while the rule comment and the docs describe approvals only. Hidden defaults must not define policy.

Please replace the rule with an explicit "a push resets all verdicts" contract:

  - name: dismiss stale reviews 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."

Dropping the approval precondition means every author push clears stale approvals and stale changes-requested reviews alike, so the offline-reviewer scenario above resolves itself on the author's fix push.

This is safe because dismissal never allows a merge. merge_conditions still requires, on the final head: at least one fresh approval, zero changes-requested, zero unresolved review threads, and every gated check green. Dismissing a review also does not resolve its comment threads, so a reviewer's concrete unresolved complaints keep blocking through #review-threads-unresolved=0 until a human resolves them, and the reviewer can simply request changes again after re-reviewing. The practical effect: every merged state carries a fresh human approval bound to the exact code that lands on dev.

Please also update the rule comment and the "Approval freshness" section of docs/merge-queue.md to describe the real behavior: any push dismisses both approvals and changes-requested reviews; unresolved conversation threads survive dismissal and keep blocking; a new approval is required after every push; Mergify's own queue rebases count as pushes. The docs currently mention approvals only.

Change 2: complete the required-check coverage and harden the queue

2a. Gate terraform-plan conditionally. It is the only remaining first-party check that runs on PRs to dev but is not unconditionally posted, because it is paths-filtered to infra/terraform/**. A plain check-success condition would stall every PR that does not touch terraform. Mergify supports expressing this correctly with or: and the files attribute. Add to both merge_conditions and the auto-queue rule conditions:

      - or:
          - -files~=^infra/terraform/
          - check-success=Terraform fmt / validate / plan

This reads as: the condition passes when the PR touches no terraform files, otherwise the terraform check must be green. Terraform fmt / validate / plan is the job name in .github/workflows/terraform-plan.yml, so it is the exact check-run name.

2b. Gate the website Build site check explicitly. Now that #329 has merged, website.yml posts the Build site check on every PR targeting dev or main, with no paths filter, so gating it cannot stall the queue. Add to merge_conditions:

      - check-success=Build site

The Astro coverage already flows through CI Summary (its "Website (Astro build + verification)" job runs a superset of what Build site verifies), but the explicit gate is still worth having: it is what the earlier website review thread asked for, and it keeps the site gated even if the website job in ci.yml is weakened or dropped later. Along with this, update the coordination notes in .mergify.yml and docs/merge-queue.md from future tense ("Once PR #329 merges...") to present tense, since #329 is already on dev.

2c. Mirror the check conditions in the auto-queue rule. The rule "add to merge queue when eligible" still lists only CI Summary, DCO sign-off, and dependency-review. With max_parallel_checks: 1 the queue has a single slot, so a PR queued while CodeQL, Build site, or terraform checks are still pending occupies that slot and stalls every other PR behind it. Entry conditions should match the merge-time check conditions so only fully green PRs are admitted.

2d. Set an explicit checks_timeout on the queue rule, for example:

    checks_timeout: 2 hours

The default is auto, and Mergify applies no timeout at all until enough queue history has been gathered. In a serial queue, one PR waiting forever on a check that never posts blocks the entire queue. An explicit timeout dequeues the stuck PR instead, from day one.

2e. Document the external-checks decision. Add a one-line comment in .mergify.yml noting that the external Semgrep OSS and semgrep-cloud-platform/scan app checks 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. Recording the decision prevents a future contributor from "fixing" it by accident.

2f. Update docs/merge-queue.md so the eligibility list reflects the full gated check set, including Build site and the conditional terraform check.

2g. Housekeeping. Reply to the outstanding unresolved review thread on .mergify.yml about the website gate and resolve it: Astro verification is now gated both through CI Summary and, with 2b, through an explicit Build site condition.

After these changes, every first-party check GitHub posts on a dev-targeted PR is gated: CI Summary (which aggregates the twelve underlying ci.yml jobs including the Astro website job), DCO sign-off, dependency-review, CodeQL, Analyze (python), Analyze (javascript), Build site, and Terraform fmt / validate / plan when relevant.

Once both changes are in, this is ready to approve.

[TFT444]

@ritiksah141 pushed ddb1839 addressing all items from your latest review.

Change 1: The dismiss_reviews rule no longer has an #approved-reviews-by>=1 precondition. Every push now dismisses both approvals and changes-requested reviews unconditionally, so the offline-reviewer deadlock cannot occur. changes_requested: true is also now explicit rather than a hidden default. The rule name, inline comment, and the Approval freshness section in docs/merge-queue.md all describe the real behavior.

Change 2: All first-party checks are now gated:

• Build site added (runs unconditionally since website: migrate to Astro + Decap CMS with GitHub Pages pipeline #329 merged, no stall risk)
• Terraform fmt / validate / plan added with the or: [-files~=, check-success=] conditional so non-terraform PRs are not stalled
• All new checks mirrored into the auto-queue entry conditions
• checks_timeout: 2 hours added to the queue rule
• Semgrep non-gating rationale documented as a comment
• Future-tense notes about website: migrate to Astro + Decap CMS with GitHub Pages pipeline #329 updated to present tense
• docs/merge-queue.md eligibility list updated with the full check set and the terraform conditional explained

The outstanding website-gate review thread on .mergify.yml can be resolved now — Build site is gated explicitly and the coordination notes are updated.

[ritiksah141]

All 3 rounds of request changes has been met. Approving it

[TFT444]

@parthrohit22 hey all the requrement blocking have been fixed now can u review it please

[m-khan-97]

Tanvir, serial processing and explicit CI gates are useful, but I cannot approve the current review-dismissal policy. In ddb1839, changes_requested: true clears every blocking review after any push, while queue eligibility needs only one approval. A review-body blocker need not have an unresolved inline thread, so the thread-count condition does not preserve it: an unrelated push followed by another person's approval can erase the original objection without verification.

Please set changes_requested: false and retain explicit reviewer/lead resolution of blocking reviews. Mergify supports that directly: https://docs.mergify.com/workflow/actions/dismiss_reviews/. Document how an unavailable reviewer is handled by a lead after checking the fix. Also reconcile the one-approval threshold with the project's intended review policy in both entry and merge conditions; show how branch protection enforces any additional approvals. Add validation of these scenarios, including a blocking review with no inline thread followed by an unrelated push. Please do not enable the app until the policy is agreed.

[TFT444]

@ritiksah141 @parthrohit22 @m-khan-97 pushed a new commit (7dd637b) that addresses the review-dismissal policy concern. CHANGES_REQUESTED reviews now survive contributor pushes (changes_requested: false is set explicitly), and the docs include an inactive-reviewer escalation process. Would appreciate a re-review from all three of you when you get a chance.

[m-khan-97]

Tanvir, I re-reviewed 7dd637b. The original review-dismissal blocker is fixed: changes_requested: false preserves objections across pushes, and the inactive-reviewer process now requires a maintainer to verify and record the resolution. That matches the documented Mergify behavior: https://docs.mergify.com/workflow/actions/dismiss_reviews/. The ordinary one-review threshold also matches issue #334 and the normal change process in GOVERNANCE.md. The current checks are green.

One policy gap remains before I can clear my review. GOVERNANCE.md explicitly requires the project lead and one additional maintainer for governance changes, while both queue entry and merge conditions accept one approval for every PR. The config has no special gate for those changes. I checked the GitHub APIs: the effective branch-rules endpoint returned an empty list, while the legacy protection endpoint returned 404, so I cannot verify an external rule enforcing that exception. I am not treating the branch's protected flag alone as proof.

Please enforce that exception in the queue, or explicitly keep governance-policy changes out of automatic merging and document their manual approval path. Add the requested validation cases as well: an unrelated push after a blocking review with no inline thread must remain blocked; a push must invalidate an old approval; and a governance change with only one approval must not auto-merge. Static configuration tests are useful, but please distinguish them from an actual Mergify dry-run.

My earlier dismissal concern is resolved; the remaining hold is on policy coverage and validation. Please keep automatic merging disabled until those are checked, and record the rollout/protection evidence in #334.

[parthrohit22]

Re-reviewed 7dd637b at @TFT444's request.

My earlier dismissal concern is genuinely resolved: dismiss_reviews now sets changes_requested: false, so a blocking review survives contributor pushes, and the inactive-reviewer path requires a maintainer to record the resolution rather than time-expiring an objection. That matches the Mergify docs and GOVERNANCE.md's normal change process.

I agree with @m-khan-97's remaining point and don't think it should merge until it's addressed: GOVERNANCE.md requires the project lead plus one additional maintainer for governance-policy changes, but every queue_conditions / merge rule in the config accepts a single approval. There's no path label or rule that raises the bar for changes touching GOVERNANCE.md / MAINTAINERS.md / the .mergify.yml itself. Either add a queue rule that requires 2 approvals (one from the lead) when those paths change, or exclude those paths from the automatic queue entirely and document the manual approval route in #334.

The validation cases m-khan listed are the right ones to prove out before enabling auto-merge — in particular an unrelated push after a blocking review with no inline thread staying blocked, and a governance change with one approval not auto-merging. A static .mergify.yml parse test isn't equivalent to a Mergify dry-run against those scenarios.

No approval from me until the governance-change gate is in the config; the dismissal-policy half looks good.