ci(merge-queue): event-driven reconciles for checks, priority labels, and reviews - #10571
Merged
Conversation
PR Summary by QodoCI: trigger merge-queue reconcile on check_suite completion
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. PR-controlled review workflow
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 182cc39 |
…secret-less bridge workflow
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
davidfirst
changed the title
|
Code review by qodo was updated up to the latest commit 4284257 |
Comment on lines
+16
to
+17
| pull_request_review: | ||
| types: [submitted, dismissed] |
There was a problem hiding this comment.
1. Review ping too broad 🐞 Bug ➹ Performance
merge-queue-review-ping triggers on every pull_request_review submission (including comment-only reviews) because it doesn’t filter on github.event.review.state, so it can cause unnecessary merge-queue reconciles unrelated to approval/dismissal changes. This increases GitHub/CircleCI API traffic because each reconcile run re-derives master state and scans open PRs.
Agent Prompt
## Issue description
`merge-queue-review-ping` runs for `pull_request_review: submitted` regardless of the review’s state. That means comment-only reviews (and other non-approval submissions) can trigger `workflow_run` → `merge-queue` reconciles even though the stated intent is to react to approvals/dismissals.
## Issue Context
The downstream `merge-queue` workflow listens for successful completions of `merge-queue-review-ping` and then runs `.github/scripts/merge-queue.js`, which calls GitHub + CircleCI APIs each time.
## Fix Focus Areas
- .github/workflows/merge-queue-review-ping.yml[15-29]
### Suggested change
Tighten the `ping` job condition to only succeed when the review event can actually affect merge eligibility, e.g.:
- keep `dismissed` (review dismissal can change approval requirements)
- for `submitted`, require `github.event.review.state == 'approved'`
Example:
```yaml
if: >-
github.repository == 'teambit/bit' &&
github.event.pull_request.auto_merge != null &&
(
github.event.action == 'dismissed' ||
(github.event.action == 'submitted' && github.event.review.state == 'approved')
)
```
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit f74619e |
GiladShoham
approved these changes
Aug 5, 2026
davidfirst
enabled auto-merge (squash)
August 5, 2026 18:24
…st-bit_merge reconcile isn't a no-op
…teambit/bit into merge-queue-check-suite-trigger
Comment on lines
+265
to
+268
| await sleep(delayMs); | ||
| const fresh = await githubRequest('GET', `/repos/${OWNER}/${REPO}/pulls/${pullRequest.number}`); | ||
| if (fresh.mergeable === null) continue; | ||
| pullRequest.mergeable = fresh.mergeable ? 'MERGEABLE' : 'CONFLICTING'; |
There was a problem hiding this comment.
1. Refresh aborts whole reconcile 🐞 Bug ☼ Reliability
refreshUnknownMergeability() awaits githubRequest() without error isolation, so any transient REST failure will throw and abort the entire reconcile run before winner selection/status updates. This introduces a new single-point failure on the settled-master path, leaving gate statuses and queue handoff stale until the next event/cron run.
Agent Prompt
### Issue description
`refreshUnknownMergeability()` makes per-PR REST calls via `githubRequest()` (which throws on non-2xx) but does not catch errors. A single API error can abort the entire reconcile run, contrary to the script’s existing pattern of isolating per-PR failures.
### Issue Context
This polling runs when `masterState.settled` and happens before winner selection and any status posting, so a thrown error prevents the reconcile from making progress.
### Fix Focus Areas
- .github/scripts/merge-queue.js[258-275]
- .github/scripts/merge-queue.js[558-561]
### Suggested fix
- Wrap the REST refresh logic in a `try/catch` per PR (or even per attempt).
- On error: log once with PR number + attempt, leave `mergeable` as `UNKNOWN`, and continue to the next entry so the reconcile can still post statuses/update dashboard.
- (Optional) Track refresh failures and set `process.exitCode = 1` at end if you want visibility without breaking the run mid-way.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+264
to
+266
| for (let attempt = 0; attempt < maxAttempts && pullRequest.mergeable === 'UNKNOWN'; attempt += 1) { | ||
| await sleep(delayMs); | ||
| const fresh = await githubRequest('GET', `/repos/${OWNER}/${REPO}/pulls/${pullRequest.number}`); |
There was a problem hiding this comment.
2. Sequential polling slows reconcile 🐞 Bug ➹ Performance
refreshUnknownMergeability() adds fixed 5s sleeps in a sequential retry loop for each queued PR with mergeable=UNKNOWN, delaying winner selection and status updates and consuming a non-trivial portion of the job’s fixed 10-minute runtime budget. Under larger queues or prolonged UNKNOWN windows, this increases the risk of delayed handoffs or hitting the workflow timeout before completing the reconcile loop.
Agent Prompt
### Issue description
`refreshUnknownMergeability()` uses sequential polling with a fixed 5s sleep per attempt. This can add substantial wall-clock delay before any reconcile actions occur.
### Issue Context
The reconcile job is configured with `timeout-minutes: 10`, so added waiting time directly reduces headroom for the rest of the reconcile (posting statuses for all PRs, dashboard updates, etc.).
### Fix Focus Areas
- .github/scripts/merge-queue.js[258-275]
- .github/workflows/merge-queue.yml[75-96]
### Suggested fix
- Remove the unconditional initial sleep: try an immediate REST read first, then sleep only if still `null`/UNKNOWN.
- Add a global deadline/budget for refresh (e.g., stop refreshing after X seconds total).
- Consider bounded concurrency (e.g., refresh up to N PRs at a time) to prevent worst-case `O(queue_length)` wall time.
- Keep logging when UNKNOWN resolves, but also log when it remains UNKNOWN after the budget so it’s diagnosable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit fc54f9c |
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
2 participants
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.
GitHub throttles scheduled workflows far beyond the
*/5spec (20m+ gaps observed today), so any queue transition that relied on the cron left green PRs sitting idle. This makes the remaining cron-only transitions event-driven; the cron stays as a last-resort safety net.check_suite: completed— a queued PR's CI finishing fired no event before; now the green→turn handoff happens in seconds. Master suites are filtered out (push/repository_dispatchalready cover master's transitions); fork-PR suites still pass. No self-recursion: the workflow's own runs complete their suites viaGITHUB_TOKEN, whose events never trigger workflows.pull_request_target: labeled/unlabeled— applyingmerge-queue:prioritynow reorders the queue immediately (filtered in the job condition to exactly that label on queued PRs).workflow_runon a new secret-lessmerge-queue-review-pingworkflow — an approval landing on a queued PR un-sticks a demoted winner right away. Reviews can't trigger the reconcile directly:pull_request_reviewexecutes the PR merge commit's workflow copy, which must never seeCIRCLE_TOKEN; the no-op bridge absorbs the untrusted context and its completion lands on master's trusted copy.