ci: introduce deterministic native merge queue gate + PR queue visibility#22294
Conversation
BrianCLong
added
the
codex
ChatGPT Codex Connector
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the branch protection configuration for the main branch, specifically modifying the encoding of an existing check name and adding 'merge-queue / merge-queue-critical' to the required status checks. Feedback highlights a critical concern regarding a potential circular dependency: if the newly added check only runs during the merge group phase, it will prevent pull requests from being queued as the requirement cannot be met beforehand.
| "test (20.x)", | ||
| "Workflow Validity Check" | ||
| "Workflow Validity Check", | ||
| "merge-queue / merge-queue-critical" |
There was a problem hiding this comment.
Adding merge-queue / merge-queue-critical as a required status check will likely block pull requests from entering the merge queue. GitHub's branch protection requires all mandatory status checks to pass before a pull request can be added to the queue. If this job is only triggered by the merge_group event (as suggested by the workflow name and PR description), it will not have run yet when a developer attempts to queue the PR. This creates a circular dependency where the PR cannot enter the queue because the check hasn't passed, and the check cannot run because the PR isn't in the queue. To resolve this, ensure the job (or a job with the same name) also triggers on standard pull_request events (e.g., opened, synchronize), or consider if this check should be required only for the final merge and not for queue entry.
WalkthroughGitHub Actions workflows and branch protection configuration were updated to restructure merge queue automation. Triggers were modified from scheduled/automated to manual or event-based dispatches. The core merge queue validation job was refactored with new visibility checks and concurrency settings, while auxiliary autopilot and feeder workflows were disabled. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccec0c3c8a
ℹ️ 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".
| merge_group: | ||
| types: [checks_requested] | ||
| pull_request: | ||
| types: [enqueued, dequeued, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
The pull_request trigger omits opened, so a newly opened non-draft PR targeting main will not execute this workflow until a later action (like synchronize) occurs. Since this commit also adds merge-queue / merge-queue-critical to required checks, those PRs can sit in an "Expected" state with no reported context and be blocked from merge queue/merge unless another triggering action happens.
Useful? React with 👍 / 👎.
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
| group: merge-queue-main | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
Workflow-level concurrency is now a single fixed group for both pull_request and merge_group runs with cancel-in-progress: false, so every queue-visibility comment run serializes with merge-group validation. In busy repos this can delay merge-queue-critical execution behind non-critical PR events and reduce merge throughput; the concurrency gate should be scoped to the merge-group job (or keyed by event) instead of the whole workflow.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/merge-train-autopilot.yml (2)
5-17: Consider removing stale workflow inputs.The
batch_size,max_queue, anddry_runinputs are defined but no longer used since the autopilot logic was replaced with informational output. Removing them avoids confusion for operators who might expect these parameters to have an effect.🧹 Suggested cleanup
on: workflow_dispatch: - inputs: - batch_size: - description: 'Number of PRs to enqueue' - required: true - default: '10' - max_queue: - description: 'Halt if merge queue depth >= N' - required: true - default: '50' - dry_run: - description: 'Dry run mode' - type: boolean - default: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/merge-train-autopilot.yml around lines 5 - 17, Remove the stale workflow inputs batch_size, max_queue, and dry_run from the inputs block of the merge-train-autopilot workflow since they are no longer used; delete the corresponding input entries named batch_size, max_queue, and dry_run and also remove any remaining references to these input names elsewhere in the workflow (e.g., uses of inputs.batch_size, inputs.max_queue, inputs.dry_run or environment variables set from them) to avoid confusion for operators.
29-38: Checkout step is unnecessary for no-op workflow.Since the workflow only echoes informational messages, the
actions/checkoutstep (withfetch-depth: 0andfetch-tags: true) adds unnecessary execution time. Similarly,pull-requests: writepermission is unused.🧹 Suggested cleanup
permissions: contents: read - pull-requests: write + pull-requests: read ... jobs: autopilot: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - fetch-tags: true - - name: Run Autopilot run: | echo "Autopilot now manual-only to prevent unsupervised queue churn." echo "Use GitHub native merge queue for routine sequencing."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/merge-train-autopilot.yml around lines 29 - 38, Remove the unnecessary checkout step and unused permission in this no-op workflow: delete the actions/checkout@v4 step (and its with keys fetch-depth and fetch-tags) since the job only echoes messages, and remove the unused repository permission "pull-requests: write" from the workflow permissions block; keep only the "Run Autopilot" step that prints the informational messages (references: the actions/checkout step, its fetch-depth/fetch-tags keys, the "Run Autopilot" step, and the pull-requests permission)..github/workflows/merge-queue-feeder.yml (1)
6-8: Consider reducingpull-requests: writetoread.Since the workflow now only echoes informational messages without modifying any PRs, the
pull-requests: writepermission is no longer needed. Reducing toread(or removing entirely) follows least-privilege principles.🔧 Suggested permission reduction
permissions: contents: read - pull-requests: write + pull-requests: read🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/merge-queue-feeder.yml around lines 6 - 8, Change the workflow permissions in the permissions: block by replacing the "pull-requests: write" entry with "pull-requests: read" (or remove the pull-requests key entirely) so the workflow only has read access; update the permissions stanza that currently contains "contents: read" and "pull-requests: write" to instead list "contents: read" and "pull-requests: read" to follow least-privilege principles..github/workflows/merge-queue.yml (1)
14-16: Static concurrency group may cause unintended serialization.Using a fixed
group: merge-queue-mainfor bothpull_requestandmerge_groupevents means PR visibility comments and merge-queue validations compete for the same concurrency slot. Withcancel-in-progress: false, a long-running validation could delay queue visibility updates.Consider event-specific concurrency groups:
♻️ Suggested fix for event-specific concurrency
concurrency: - group: merge-queue-main + group: ${{ github.event_name == 'merge_group' && 'merge-queue-main' || format('queue-visibility-{0}', github.event.pull_request.number) }} cancel-in-progress: falseThis serializes merge-queue validations while allowing independent queue-visibility updates per PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/merge-queue.yml around lines 14 - 16, The static concurrency block uses the fixed keys "concurrency", "group: merge-queue-main", and "cancel-in-progress", which serializes unrelated events (pull_request vs merge_group); change the "group" to be event-specific so visibility updates and merge validations don't compete (e.g., use an expression like group: merge-queue-${{ github.event_name }} or include the PR identifier such as group: merge-queue-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}), keep "cancel-in-progress" as needed, and apply this updated concurrency stanza so pull_request and merge_group workflows use distinct concurrency groups.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/merge-queue.yml:
- Around line 125-129: The workflow step "Critical validation suite" uses pnpm
run lint/typecheck/test with --if-present which can silently skip those critical
checks; update the job to either remove the --if-present flags so failures occur
when scripts are missing (run pnpm run lint, pnpm run typecheck, pnpm run test)
or add a preceding verification step that asserts the package.json contains the
expected scripts ("lint", "typecheck", "test") (e.g. a small shell/node check
that exits non-zero if any script key is missing) before running the pnpm
commands so the merge gate fails explicitly if scripts are absent or misspelled.
- Around line 6-8: The pull_request trigger's types array contains invalid
activity types "enqueued" and "dequeued" which will break the workflow; edit the
pull_request section (the pull_request: types: [...] array) to remove "enqueued"
and "dequeued" and keep only valid types such as "synchronize", "reopened", and
"ready_for_review" (or any other valid pull_request events you need), and if you
intended to use merge queue signals, use the merge_group event with its
supported "checks_requested" activity instead of adding non‑existent
enqueued/dequeued types.
---
Nitpick comments:
In @.github/workflows/merge-queue-feeder.yml:
- Around line 6-8: Change the workflow permissions in the permissions: block by
replacing the "pull-requests: write" entry with "pull-requests: read" (or remove
the pull-requests key entirely) so the workflow only has read access; update the
permissions stanza that currently contains "contents: read" and "pull-requests:
write" to instead list "contents: read" and "pull-requests: read" to follow
least-privilege principles.
In @.github/workflows/merge-queue.yml:
- Around line 14-16: The static concurrency block uses the fixed keys
"concurrency", "group: merge-queue-main", and "cancel-in-progress", which
serializes unrelated events (pull_request vs merge_group); change the "group" to
be event-specific so visibility updates and merge validations don't compete
(e.g., use an expression like group: merge-queue-${{ github.event_name }} or
include the PR identifier such as group: merge-queue-${{ github.event_name
}}-${{ github.event.pull_request.number || github.ref }}), keep
"cancel-in-progress" as needed, and apply this updated concurrency stanza so
pull_request and merge_group workflows use distinct concurrency groups.
In @.github/workflows/merge-train-autopilot.yml:
- Around line 5-17: Remove the stale workflow inputs batch_size, max_queue, and
dry_run from the inputs block of the merge-train-autopilot workflow since they
are no longer used; delete the corresponding input entries named batch_size,
max_queue, and dry_run and also remove any remaining references to these input
names elsewhere in the workflow (e.g., uses of inputs.batch_size,
inputs.max_queue, inputs.dry_run or environment variables set from them) to
avoid confusion for operators.
- Around line 29-38: Remove the unnecessary checkout step and unused permission
in this no-op workflow: delete the actions/checkout@v4 step (and its with keys
fetch-depth and fetch-tags) since the job only echoes messages, and remove the
unused repository permission "pull-requests: write" from the workflow
permissions block; keep only the "Run Autopilot" step that prints the
informational messages (references: the actions/checkout step, its
fetch-depth/fetch-tags keys, the "Run Autopilot" step, and the pull-requests
permission).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a4cb46c-3a91-4da0-891b-b46a9f55586a
📒 Files selected for processing (5)
.github/protection/main.required-checks.json.github/workflows/merge-group-heavy.yml.github/workflows/merge-queue-feeder.yml.github/workflows/merge-queue.yml.github/workflows/merge-train-autopilot.yml
| pull_request: | ||
| types: [enqueued, dequeued, synchronize, reopened, ready_for_review] | ||
| branches: [main] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the valid activity types for GitHub Actions pull_request event trigger?
💡 Result:
The valid activity types (specified using the types key) for the GitHub Actions pull_request event trigger are: - assigned - unassigned - review_requested - review_request_removed - labeled - unlabeled - opened - edited - closed - reopened - synchronize - ready_for_review - locked - unlocked - auto_merge_disabled - auto_merge_enabled By default, workflows trigger on opened, synchronize, and reopened if no types are specified. These activity types correspond to the action values in the pull_request webhook payload, which GitHub Actions uses to trigger workflows. The full list is derived from official GitHub documentation on webhook events and confirmed by examples in events-that-trigger-workflows and blog posts referencing the docs. For the most up-to-date list as of 2026, refer to https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_request and https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request.
Citations:
- 1: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 2: https://frontside.com/blog/2020-05-26-github-actions-pull_request/
- 3: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 4: https://docs.github.com/en/enterprise-server@2.21/developers/webhooks-and-events/events/github-event-types
- 5: https://docs.github.com/en/webhooks/webhook-events-and-payloads
🌐 Web query:
What are the valid activity types for GitHub Actions merge_group event trigger?
💡 Result:
The valid activity type for the GitHub Actions merge_group event trigger is checks_requested. Unlike many other events, the merge_group event does not support multiple activity types that can be filtered with the 'types' keyword. It triggers specifically when checks are requested for a merge group in a merge queue, with GITHUB_SHA set to the merge group SHA and GITHUB_REF to the merge group ref.
Citations:
- 1: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 2: https://docs.github.com/en/actions/reference/events-that-trigger-workflows
- 3: https://www.magicbell.com/workflows/github/merge-group-checks-requested
- 4: https://github.blog/changelog/2022-08-18-merge-group-webhook-event-and-github-actions-workflow-trigger
- 5: https://github.apidog.io/schema-699783
enqueued and dequeued are not valid pull_request activity types.
The enqueued and dequeued events are not valid pull_request trigger types. Valid pull_request types include: opened, closed, synchronize, reopened, ready_for_review, labeled, unlabeled, assigned, unassigned, review_requested, etc.
This means lines 7-8 contain invalid syntax that will cause the workflow to fail.
Remove the invalid event types from the pull_request trigger:
pull_request:
- types: [enqueued, dequeued, synchronize, reopened, ready_for_review]
+ types: [synchronize, reopened, ready_for_review]
branches: [main]Note: The merge_group event trigger (used for GitHub merge queues) only supports the checks_requested activity type and does not support enqueued or dequeued filtering. These event types do not exist in GitHub Actions workflow triggers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/merge-queue.yml around lines 6 - 8, The pull_request
trigger's types array contains invalid activity types "enqueued" and "dequeued"
which will break the workflow; edit the pull_request section (the pull_request:
types: [...] array) to remove "enqueued" and "dequeued" and keep only valid
types such as "synchronize", "reopened", and "ready_for_review" (or any other
valid pull_request events you need), and if you intended to use merge queue
signals, use the merge_group event with its supported "checks_requested"
activity instead of adding non‑existent enqueued/dequeued types.
| - name: Critical validation suite | ||
| run: | | ||
| pnpm run lint --if-present | ||
| pnpm run typecheck --if-present | ||
| pnpm run test --if-present |
There was a problem hiding this comment.
Using --if-present may silently skip critical validations.
If lint, typecheck, or test scripts are missing or misspelled in package.json, --if-present causes pnpm to silently skip them without failure. For a merge-queue gate, this could allow broken code to merge.
Consider failing explicitly if expected scripts are missing, or add verification that the scripts ran.
🛡️ Suggested verification approach
- name: Critical validation suite
run: |
- pnpm run lint --if-present
- pnpm run typecheck --if-present
- pnpm run test --if-present
+ pnpm run lint
+ pnpm run typecheck
+ pnpm run testOr if some scripts are genuinely optional, add explicit checks:
- name: Critical validation suite
run: |
+ # Verify critical scripts exist
+ jq -e '.scripts.lint and .scripts.typecheck and .scripts.test' package.json || { echo "Missing required scripts"; exit 1; }
pnpm run lint --if-present
pnpm run typecheck --if-present
pnpm run test --if-present📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Critical validation suite | |
| run: | | |
| pnpm run lint --if-present | |
| pnpm run typecheck --if-present | |
| pnpm run test --if-present | |
| - name: Critical validation suite | |
| run: | | |
| pnpm run lint | |
| pnpm run typecheck | |
| pnpm run test |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/merge-queue.yml around lines 125 - 129, The workflow step
"Critical validation suite" uses pnpm run lint/typecheck/test with --if-present
which can silently skip those critical checks; update the job to either remove
the --if-present flags so failures occur when scripts are missing (run pnpm run
lint, pnpm run typecheck, pnpm run test) or add a preceding verification step
that asserts the package.json contains the expected scripts ("lint",
"typecheck", "test") (e.g. a small shell/node check that exits non-zero if any
script key is missing) before running the pnpm commands so the merge gate fails
explicitly if scripts are absent or misspelled.
|
Closing duplicate — superseded by #22298 |
…lity (rebase on main)
BrianCLong
force-pushed
the
codex/implement-merge-queue-system-in-ci/cd-ud4wxk
branch
from
ccec0c3 to
07c928a
Compare
BrianCLong
deleted the
codex/implement-merge-queue-system-in-ci/cd-ud4wxk
branch
1 participant
Motivation
main.Description
.github/workflows/merge-queue.ymlthat:merge_groupforchecks_requestedand topull_requestqueue lifecycle events, publishes a persistent PR comment with queue position/depth/reason, and runs an integrated validation jobmerge-queue-criticalthat performsinstall,lint,typecheck, andteston the merge candidate.merge-queue-main) withcancel-in-progress: falseto serialize queue validations and avoid superseding in-flight validations..github/workflows/merge-group-heavy.yml,.github/workflows/merge-queue-feeder.yml, and.github/workflows/merge-train-autopilot.ymltoworkflow_dispatch/no-op operator guidance..github/protection/main.required-checks.jsonasmerge-queue / merge-queue-criticalso the integrated validation is required before main merges.Testing
ruby -ryamland all modified workflow files loaded successfully (ok .github/workflows/*.yml). — Success..github/protection/main.required-checks.jsonwithjqto ensure valid JSON structure. — Success.workflow_dispatchormerge_grouptriggers as intended. — Success.Codex Task
Summary by CodeRabbit