ci(mergify): use github-review-decision for is_approved

[Kevin (kevcube)]

what

• Update is_approved anchor in .github/mergify.yml to a list containing github-review-decision=APPROVED.

why

• Updates Mergify configuration to a valid state and aligns with the schema used across Cloud Posse repositories, stopping automated blank PRs opened by Mergify across extending repos.
• Formats is_approved as a list so it expands properly under - and: *is_approved.
• Replaces explicit review count predicates with GitHub's native github-review-decision=APPROVED, delegating review enforcement to repository branch protection.

[mergify]

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

[Erik Osterman (Cloud Posse) (osterman)]

This simplifies the condition, but changes its meaning. github-review-decision=APPROVED does not guarantee zero unresolved threads or zero comment-only reviews, which the original predicate explicitly required.

Please retain those checks or clarify why the weaker semantics are necessary.

[Kevin (kevcube)]

Updated the PR title and description.

The goal here is getting the config into a valid, current state and aligned with the schema across Cloud Posse repos to stop Mergify from opening automated blank upgrade PRs in repos extending this config.

Pushed 403a01e to format is_approved as a list (- "github-review-decision=APPROVED"), so it expands properly under - and: *is_approved.

I checked repos extending this config (atmos, cloudposse/.github, github-commenter). Their default branches have protection active, so GitHub's reviewDecision: APPROVED works as expected.

[Erik Osterman (Cloud Posse) (osterman)]

The syntax is not obsolete

The description says that this change removes deprecated syntax. This is not correct.

The Mergify documentation shows these four attributes as current:

• approved-reviews-by
• changes-requested-reviews-by
• commented-reviews-by
• review-threads-unresolved

The documentation does not show a note that these attributes are obsolete. The attribute github-review-decision is also current. But it is an addition to the set of attributes. It is not a replacement for the four attributes.

This file uses the same syntax in three other rules

These three rules keep the same syntax after this change:

• The rule remove triage label if approved uses #approved-reviews-by>=1.
• The rule add triage label for new pull requests uses #approved-reviews-by=0.
• The rule close pull request waiting on feedback for 1 month uses #commented-reviews-by > 0 and #changes-requested-reviews-by > 0.

If the syntax was obsolete, this pull request must also change these three rules. It does not change them.

This is a change of behavior

The old condition tests four items. The new condition tests only the review decision from GitHub. The new condition removes three tests:

1. Unresolved review threads no longer stop a merge. The GitHub review decision does not include the status of review threads.
2. Reviews with the status COMMENTED no longer stop a merge.
3. The test for approval now depends fully on the branch protection rules.

Item 2 is possibly the reason for this change. Copilot adds a review with the status COMMENTED to almost all pull requests in this organization. The old condition #commented-reviews-by=0 then stops these pull requests permanently. This is a real problem, and you must correct it. But the description must tell the reader about the change of behavior.

Item 3 is a risk. GitHub sets reviewDecision to null when the branch has no protection rule for reviews. A null value is not equal to APPROVED. Thus the condition is_approved is never true in such a repository. This file is the default configuration for the organization. Make sure that each repository that uses this file has a branch protection rule that requires reviews.

The new value has an incorrect type

The anchor is_approved was a list. It is now a string. The rule uses the anchor in this form:

- and: *is_approved

The key and requires a list of conditions. It now receives a string. Change the value to a list:

  is_approved: &is_approved
    - "github-review-decision=APPROVED"

All other anchors in this file are lists for this reason.

This change has no effect now

There is only one reference to *is_approved in this file. The reference is in the rule mergeable. All lines of this rule have comment characters. Thus Mergify does not read the rule, and this change does not operate. The incorrect type causes a failure only when a person removes the comment characters from the rule mergeable.

Requested changes

1. Correct the description. Tell the reader that this change removes the test for unresolved review threads and the test for reviews with the status COMMENTED.
2. Change the value of is_approved to a list.
3. Confirm that each repository with this configuration has a branch protection rule that requires reviews.

References

• Mergify conditions reference
• Mergify changelog: branch-protection-review-decision
• GitHub discussion 24375: when reviewDecision is null

[Kevin (kevcube)]

Erik Osterman (Cloud Posse) (@osterman) whoops, this one was kinda slop. i was just attempting to resolve cloudposse/terraform-aws-efs#166 and cloudposse/packages#6317 but i ended up down the wrong approach. if you could resolve those PRs (approve+merge) I'd appreciate.