chore(ci): CTC-1231 — Mergify merge_protections, queue entry becomes platform-enforced - #4089
Merged
Conversation
…platform-enforced Adds two merge_protections rules so the queue-entry contract holds even for a PR merged by hand outside Mergify: zero unresolved review threads, and a Conventional-Commit-shaped title (Dependabot/Mergify-train/release-please exempt). The auto-queue rule itself (pull_request_rules + queue_conditions gating on queue:ready/threads/checks) already shipped via CTL-2266 (#4079) today — this PR doesn't touch queue_rules/pull_request_rules. Validated with `mergify config validate`; `mergify config simulate` only previews pull_request_rules/queue_rules, not merge_protections, so the two new rules were verified by regex/polarity testing against real PR titles, authors, and branches instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 792c459d49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…po's five valid scopes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
merge_protectionsrules to.mergify.yml, extending the catalyst-cloud config-as-code adoption (Mergify feature-suite evaluation, "Merge Protections rules — ADOPT NOW") to this repo: two checks that hold even for a PR merged by hand, outside the Mergify queue.#review-threads-unresolved=0, active on every non-draft PR againstmain.type(scope): ..., active on every non-draft PR againstmainexcept automation:dependabot[bot], Mergify's own queue-train/dashboard-config branches (head~=^mergify/), and release-please (head~=^release-please--— it opens PRs under a human GitHub identity in this repo, not a bot login, so it can't be author-exempted).This does not touch
queue_rules/pull_request_rules. The auto-queue rule CTC-1231 also asks for (label=queue:ready+#review-threads-unresolved=0+ the required-check set →queue) already shipped today via CTL-2266 (#4079) — re-checked, unchanged, still correct.Not required yet. A failing active
merge_protectionsrule surfaces as a non-requiredMergify Merge Protectionscheck — this PR does not mark it required in themain branch protectionruleset, so nothing is newly blocking. That flip is Ryan's call, same restraint #4079 took with branch protection.Note on
.mergify.yml's own path exclusion: this repo'squeue_conditions/merge_conditionsalready hard-exclude-files~=^\.mergify\.yml$, so this PR (and any future.mergify.ymlchange) can never enter the queue regardless of labeling — hand-merge only, by design, already in place before this PR.Explicitly not in this PR (per the sibling catalyst-cloud evaluation and the dispatch instructions): CI Insights auto-retry, any Test Insights config.
Validation
mergify config validatemergify config simulate— scope limitation, documentedmergify config simulate <PR_URL>only previewspull_request_rules/queue_rulesevaluation. Ran it against #4066 (real Dependabot PR) to confirm the pre-existing queue rule is unaffected by this diff — it evaluated correctly (all conditions still resolve as before). It never surfaced the newmerge_protectionsrules on any PR — that's a CLI/product scope limit (I found no documented way to previewmerge_protectionsoutside the dashboard), not a config defect.In place of
simulatefor the new rules, I verified both directly:#review-threads-unresolved=0attribute the existingqueue_conditionsalready gates on; unchanged in this diff, and I didn't need to re-derive its correctness.gh pr listhistory for this repo, both polarities:feat(meta): CTL-2266 — ...,perf(ci): CTC-1229 — ...,ci(runner-image): CTC-1228 — ...,chore(deps): Bump ...(Dependabot),ci: Bump actions/checkout from 4 to 7(Dependabot, no scope),chore: release main(release-please).CTC-746: A ticket's agent session should still show its plan after th(a real, old, merged PR using a pre-convention title shape — historical, not currently open, so nothing bricks).Local gate
No unified
bun run check-equivalent here; ranmergify config validate(pass) andmake lint/make testlocally.make lint'strunk checkcurrently fails on a pre-existingfmt/prettier formatting complaint about.mergify.yml— reproduced identically against the unmodifiedorigin/maincopy of the same file, so it predates this PR and isn't something this diff introduced or should fix (touching those pre-existing lines was explicitly avoided to keep the live, already-proven queue config untouched).make testsurfaced one unrelated pre-existing failure (catalyst-config.test.sh, a config-fingerprint test with no relation to.mergify.yml) before my owntimeout 180truncated the (very large) suite; nothing in this diff touches that test or its dependencies.Linear
CTC-1231 stays in Implement — dispatched with
--phase implement; not moving it to In Review myself per the dispatch instructions (report only, don't merge).