fix: merge gate scan every 10 min after PR is merge-ready#2379
Conversation
Replace 4x-daily schedule with */10 cron so merge-ready PRs are picked up within ~10 minutes. Do not block on 35min @claude cooldown once FINAL review is complete on current HEAD. Disable cancel-in-progress so assess jobs are not aborted by the next scan. Co-authored-by: Cursor <cursoragent@cursor.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe merge-gate cooldown now checks for a verdict on the head SHA before applying the recent ChangesMerge-gate cooldown, dispatch, and workflow cadence
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![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 introduces a mechanism to skip the 35-minute cooldown gate if a final Claude review has been completed on the current HEAD, adding a check in merge-gate.js and a corresponding test in merge-gate-selftest.js. The reviewer identified a critical issue where the implementation checks for the presence of the trigger comment (finalClaudeCompletedOnSha) rather than an actual completed verdict, which would prematurely bypass the cooldown. The feedback suggests using findMergeGateVerdict instead to ensure a verdict has been posted, and provides updated test assertions to match this corrected behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt); | ||
| if (!finalDone) reasons.push('recent @claude within 35min'); |
There was a problem hiding this comment.
Using finalClaudeCompletedOnSha to skip the 35-minute cooldown is problematic because finalClaudeCompletedOnSha only checks if the trigger comment (e.g., @claude You are the FINAL architecture reviewer.) has been posted and is not stale. It does not check if Claude has actually completed its review and posted a verdict.
As a result, the 35-minute cooldown is bypassed immediately after the trigger comment is posted (even before Claude starts or finishes its run). This defeats the purpose of the cooldown, which is to prevent concurrent/rapid triggers or race conditions while Claude is running or queued.
To correctly skip the cooldown only when Claude has actually completed its review and posted a verdict, you should check if a verdict comment exists on the current HEAD using findMergeGateVerdict.
| const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt); | |
| if (!finalDone) reasons.push('recent @claude within 35min'); | |
| const finalDone = findMergeGateVerdict(ctx.comments, null, ctx.headPushedAt) !== null; | |
| if (!finalDone) reasons.push('recent @claude within 35min'); |
| const recentFinalOnHead = [ | ||
| { | ||
| user: { login: 'MervinPraison' }, | ||
| body: '@claude You are the FINAL architecture reviewer.', | ||
| created_at: '2026-06-27T10:00:00Z', | ||
| }, | ||
| ]; | ||
| assert( | ||
| 'final on head skips cooldown gate', | ||
| mg.finalClaudeCompletedOnSha(recentFinalOnHead, '2026-06-27T09:55:00Z') | ||
| ); |
There was a problem hiding this comment.
The test name 'final on head skips cooldown gate' is misleading because finalClaudeCompletedOnSha only checks if the trigger comment is present and not stale, rather than verifying if the cooldown gate is skipped.
Since we should use findMergeGateVerdict to determine if the final review is actually complete (and thus safe to skip the cooldown), we should update this test to assert the correct behavior of findMergeGateVerdict or the actual cooldown skipping logic.
const recentFinalOnHead = [
{
user: { login: 'MervinPraison' },
body: '@claude You are the FINAL architecture reviewer.',
created_at: '2026-06-27T10:00:00Z',
},
];
assert(
'final trigger on head does not skip cooldown without verdict',
mg.findMergeGateVerdict(recentFinalOnHead, null, '2026-06-27T09:55:00Z') === null
);
const recentVerdictOnHead = [
...recentFinalOnHead,
{
user: { login: 'github-actions[bot]' },
body: 'MERGE_GATE_VERDICT: APPROVE',
created_at: '2026-06-27T10:05:00Z',
},
];
assert(
'final verdict on head skips cooldown gate',
mg.findMergeGateVerdict(recentVerdictOnHead, null, '2026-06-27T09:55:00Z') === 'APPROVE'
);![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/merge-gate.js:
- Around line 605-607: The cooldown check in merge-gate.js is skipping too early
when finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt) is true. Update
the logic around hasRecentClaudeTrigger and finalClaudeCompletedOnSha so the
“recent `@claude` within 35min” reason is only suppressed when the latest FINAL
completion on HEAD is at least as recent as the latest qualifying `@claude`
trigger; otherwise keep the cooldown reason and let hasInProgressClaudeAssistant
handle any newer run.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7a84a9b-858a-4021-9d1c-2359f4471cd5
📒 Files selected for processing (3)
.github/scripts/merge-gate-selftest.js.github/scripts/merge-gate.js.github/workflows/claude-merge-gate.yml
| if (!skipRecentClaudeCooldown && hasRecentClaudeTrigger(ctx.comments, 35)) { | ||
| reasons.push('recent @claude within 35min'); | ||
| const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt); | ||
| if (!finalDone) reasons.push('recent @claude within 35min'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't bypass cooldown for newer non-FINAL @claude comments.
finalClaudeCompletedOnSha(...) only proves that some FINAL review completed on the current HEAD. It does not prove that the FINAL is newer than the most recent @claude trigger. If someone posts another @claude comment after that FINAL, this branch suppresses the 35-minute guard before hasInProgressClaudeAssistant(...) can see the new run, so the PR can be treated as merge-ready while a fresh Claude review is still being kicked off.
Only skip this reason when the latest qualifying FINAL completion on HEAD is at least as new as the latest qualifying @claude trigger; otherwise keep recent @claude within 35min.
Suggested direction
- if (!skipRecentClaudeCooldown && hasRecentClaudeTrigger(ctx.comments, 35)) {
- const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt);
- if (!finalDone) reasons.push('recent `@claude` within 35min');
- }
+ if (!skipRecentClaudeCooldown && hasRecentClaudeTrigger(ctx.comments, 35)) {
+ const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt);
+ const latestClaudeTriggerAt = getLatestClaudeTriggerTime(ctx.comments);
+ const latestFinalOnHeadAt = getLatestFinalCompletionOnHeadTime(ctx.comments, ctx.headPushedAt);
+ if (!finalDone || latestFinalOnHeadAt < latestClaudeTriggerAt) {
+ reasons.push('recent `@claude` within 35min');
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/merge-gate.js around lines 605 - 607, The cooldown check in
merge-gate.js is skipping too early when finalClaudeCompletedOnSha(ctx.comments,
ctx.headPushedAt) is true. Update the logic around hasRecentClaudeTrigger and
finalClaudeCompletedOnSha so the “recent `@claude` within 35min” reason is only
suppressed when the latest FINAL completion on HEAD is at least as recent as the
latest qualifying `@claude` trigger; otherwise keep the cooldown reason and let
hasInProgressClaudeAssistant handle any newer run.
Greptile SummaryThis PR updates the merge-gate workflow timing and readiness checks.
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: require verdict on HEAD before skip..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Remove redundant post-push buffer (CI green on HEAD is the gate) - MAX_CANDIDATES=1 to avoid parallel merge race on main - Retry merge up to 3x when base branch was modified - Pipeline status sync dispatches merge gate for oldest merge-ready PR - Align pipeline sync cron to every 10 minutes Co-authored-by: Cursor <cursoragent@cursor.com>
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
MervinPraison
added
pipeline/blocked:ci
|
Claude finished @MervinPraison's task —— View job FINAL Architecture Review — PR #2379
Scope noteThis PR only touches CI infrastructure ( Valid issue fixed (flagged by Gemini, CodeRabbit, and Greptile — same root cause)
The cooldown bypass used
Fix: skip the cooldown only when a real - const finalDone = finalClaudeCompletedOnSha(ctx.comments, ctx.headPushedAt);
- if (!finalDone) reasons.push('recent @claude within 35min');
+ const verdictOnHead = findMergeGateVerdict(ctx.comments, null, ctx.headPushedAt) !== null;
+ if (!verdictOnHead) reasons.push('recent @claude within 35min');Self-test updated: the misleading Files modified
Skipped
VerdictMERGE_GATE_VERDICT: APPROVE — the one valid blocking issue (premature cooldown bypass) is resolved; changes are minimal, backward-compatible, and make the gate strictly more conservative.
|
The cooldown bypass used finalClaudeCompletedOnSha which only checks the FINAL trigger comment is present and not stale — not that Claude actually finished and posted a verdict. This let the gate skip the 35-min cooldown before the FINAL run was even visible. Switch to findMergeGateVerdict so the cooldown is only skipped once a real MERGE_GATE_VERDICT exists on HEAD. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
MervinPraison
added
the
pipeline/blocked:cooldown
|
Claude finished @MervinPraison's task —— View job FINAL Architecture Review — PR #2379
Scope noteThis PR touches only CI infrastructure ( Valid issue — already FIXED on HEAD ✅All three bots (Gemini 🔴, CodeRabbit 🟠, Greptile P1) flagged the same root cause at The fix (commit if (!skipRecentClaudeCooldown && hasRecentClaudeTrigger(ctx.comments, 35)) {
const verdictOnHead = findMergeGateVerdict(ctx.comments, null, ctx.headPushedAt) !== null;
if (!verdictOnHead) reasons.push('recent @claude within 35min');
}Why this resolves every reviewer's concern (traced via
Self-test coverage ✅
Plus existing assertions at lines 44-49 exercise the Skipped
VerdictMERGE_GATE_VERDICT: APPROVE — the one valid blocking issue (premature cooldown bypass) is already resolved on HEAD; changes are minimal, backward-compatible, and make the gate strictly more conservative.
|
MervinPraison
added
pipeline/blocked:cooldown
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/pipeline-status.js:
- Around line 158-165: Refresh the label check immediately before dispatch in
syncPipelineLabels so cand.labels is not based only on the stale loadPrContext
snapshot. Re-read the current PR labels from GitHub right before deciding to
trigger the merge-gate workflow, and use that fresh state to confirm
claude-merge-gate-active is still absent. Update the candidate handling around
the dispatch path so concurrent runs cannot enqueue a duplicate merge-gate
workflow for the same PR.
In @.github/workflows/claude-merge-gate.yml:
- Around line 456-475: The merge retry path in the pull-request merge loop needs
to re-check that the current PR head still matches the scanned SHA before
attempting the second merge. After the 8s backoff and the call to
mergeGate.evaluatePipelineQuiescent, rerun the same scanned-SHA guard used in
the initial approval path (the check comparing check.headSha and scanHeadSha)
inside this retry branch, and abort/remove labels if it no longer matches. Keep
this logic in the merge attempt loop around github.rest.pulls.merge and
mergeGate.evaluatePipelineQuiescent so retries cannot merge a different commit.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32227927-33b2-4698-85d8-e2085bbee809
📒 Files selected for processing (4)
.github/scripts/merge-gate.js.github/scripts/pipeline-status.js.github/workflows/claude-merge-gate.yml.github/workflows/pipeline-status-sync.yml
| return { | ||
| synced: true, | ||
| stage, | ||
| blockers, | ||
| ready: evalResult.ready, | ||
| reasons: evalResult.reasons, | ||
| labels: ctx.labels, | ||
| createdAt: new Date(ctx.pr.created_at).getTime(), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Refresh the active-label state immediately before dispatch.
cand.labels comes from syncPipelineLabels()'s earlier loadPrContext() snapshot. If another run adds claude-merge-gate-active before this loop reaches the candidate, we still dispatch a second merge-gate workflow for the same PR.
Suggested fix
async function dispatchMergeGateForOldestReady(github, owner, repo, readyCandidates, core) {
if (!readyCandidates.length) return 0;
readyCandidates.sort((a, b) => a.createdAt - b.createdAt);
for (const cand of readyCandidates) {
- if ((cand.labels || []).includes('claude-merge-gate-active')) {
+ const { data: labels } = await github.rest.issues.listLabelsOnIssue({
+ owner,
+ repo,
+ issue_number: cand.prNumber,
+ per_page: 100,
+ });
+ if (labels.some((label) => label.name === 'claude-merge-gate-active')) {
core?.info?.(`Skip dispatch PR #${cand.prNumber}: merge gate already active`);
continue;
}
await github.rest.repos.createDispatchEvent({Also applies to: 169-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/pipeline-status.js around lines 158 - 165, Refresh the label
check immediately before dispatch in syncPipelineLabels so cand.labels is not
based only on the stale loadPrContext snapshot. Re-read the current PR labels
from GitHub right before deciding to trigger the merge-gate workflow, and use
that fresh state to confirm claude-merge-gate-active is still absent. Update the
candidate handling around the dispatch path so concurrent runs cannot enqueue a
duplicate merge-gate workflow for the same PR.
| for (let attempt = 1; attempt <= 3; attempt++) { | ||
| try { | ||
| await github.rest.pulls.merge({ | ||
| owner, repo, pull_number: prNumber, merge_method: method, | ||
| }); | ||
| merged = true; | ||
| break; | ||
| } catch (e) { | ||
| const msg = (e.message || '').toLowerCase(); | ||
| if (attempt < 3 && msg.includes('base branch was modified')) { | ||
| core.info(`Merge attempt ${attempt} failed (base moved), retrying in 8s...`); | ||
| await sleep(8000); | ||
| const recheck = await mergeGate.evaluatePipelineQuiescent( | ||
| github, owner, repo, prNumber, core, { forMergeStep: true } | ||
| ); | ||
| if (!recheck.ready) { | ||
| core.info(`Skip merge retry: ${recheck.reasons.join(', ')}`); | ||
| await removeLabels(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Re-apply the scanned SHA guard on merge retries.
After the 8s backoff, evaluatePipelineQuiescent() only proves the current HEAD is mergeable. It does not prove this workflow's APPROVE verdict was produced for that SHA. If the PR head changes during the wait, attempt 2 can merge a different commit because the original check.headSha !== scanHeadSha guard is never rerun inside the retry path.
Suggested fix
const recheck = await mergeGate.evaluatePipelineQuiescent(
github, owner, repo, prNumber, core, { forMergeStep: true }
);
if (!recheck.ready) {
core.info(`Skip merge retry: ${recheck.reasons.join(', ')}`);
await removeLabels();
return;
}
+ if (recheck.headSha !== scanHeadSha) {
+ core.info(`Skip merge retry: HEAD moved ${scanHeadSha.slice(0, 7)} → ${recheck.headSha.slice(0, 7)}`);
+ await removeLabels();
+ return;
+ }
continue;📝 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.
| for (let attempt = 1; attempt <= 3; attempt++) { | |
| try { | |
| await github.rest.pulls.merge({ | |
| owner, repo, pull_number: prNumber, merge_method: method, | |
| }); | |
| merged = true; | |
| break; | |
| } catch (e) { | |
| const msg = (e.message || '').toLowerCase(); | |
| if (attempt < 3 && msg.includes('base branch was modified')) { | |
| core.info(`Merge attempt ${attempt} failed (base moved), retrying in 8s...`); | |
| await sleep(8000); | |
| const recheck = await mergeGate.evaluatePipelineQuiescent( | |
| github, owner, repo, prNumber, core, { forMergeStep: true } | |
| ); | |
| if (!recheck.ready) { | |
| core.info(`Skip merge retry: ${recheck.reasons.join(', ')}`); | |
| await removeLabels(); | |
| return; | |
| } | |
| for (let attempt = 1; attempt <= 3; attempt++) { | |
| try { | |
| await github.rest.pulls.merge({ | |
| owner, repo, pull_number: prNumber, merge_method: method, | |
| }); | |
| merged = true; | |
| break; | |
| } catch (e) { | |
| const msg = (e.message || '').toLowerCase(); | |
| if (attempt < 3 && msg.includes('base branch was modified')) { | |
| core.info(`Merge attempt ${attempt} failed (base moved), retrying in 8s...`); | |
| await sleep(8000); | |
| const recheck = await mergeGate.evaluatePipelineQuiescent( | |
| github, owner, repo, prNumber, core, { forMergeStep: true } | |
| ); | |
| if (!recheck.ready) { | |
| core.info(`Skip merge retry: ${recheck.reasons.join(', ')}`); | |
| await removeLabels(); | |
| return; | |
| } | |
| if (recheck.headSha !== scanHeadSha) { | |
| core.info(`Skip merge retry: HEAD moved ${scanHeadSha.slice(0, 7)} → ${recheck.headSha.slice(0, 7)}`); | |
| await removeLabels(); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/claude-merge-gate.yml around lines 456 - 475, The merge
retry path in the pull-request merge loop needs to re-check that the current PR
head still matches the scanned SHA before attempting the second merge. After the
8s backoff and the call to mergeGate.evaluatePipelineQuiescent, rerun the same
scanned-SHA guard used in the initial approval path (the check comparing
check.headSha and scanHeadSha) inside this retry branch, and abort/remove labels
if it no longer matches. Keep this logic in the merge attempt loop around
github.rest.pulls.merge and mergeGate.evaluatePipelineQuiescent so retries
cannot merge a different commit.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/merge-gate.js (1)
609-614: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the post-push buffer before trusting empty check results.
allChecksGreenOnSha()explicitly allowsruns.length === 0, so after this removal a freshly pushed SHA can be treated as CI-green before GitHub has even created its check runs. That makes the gate eligible for dispatch/merge too early.Suggested fix
+ const headAgeMs = Date.now() - new Date(ctx.headPushedAt).getTime(); + if (headAgeMs < 5 * 60 * 1000) { + reasons.push('post-push buffer (<5min)'); + } + const checksOk = await allChecksGreenOnSha(github, owner, repo, ctx.headSha, core); if (!checksOk) reasons.push('CI not green on HEAD');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/merge-gate.js around lines 609 - 614, The merge gate currently trusts allChecksGreenOnSha() even when it returns an empty result set, which can let a just-pushed HEAD look CI-green too early. Reintroduce the post-push buffer logic in merge-gate.js around the checksOk decision, using the existing ctx.headSha / prNumber flow so empty runs from allChecksGreenOnSha() do not immediately allow dispatch or merge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/scripts/merge-gate.js:
- Around line 609-614: The merge gate currently trusts allChecksGreenOnSha()
even when it returns an empty result set, which can let a just-pushed HEAD look
CI-green too early. Reintroduce the post-push buffer logic in merge-gate.js
around the checksOk decision, using the existing ctx.headSha / prNumber flow so
empty runs from allChecksGreenOnSha() do not immediately allow dispatch or
merge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4e8a299-4b50-4b80-8552-0dc3d39b0e4d
📒 Files selected for processing (2)
.github/scripts/merge-gate-selftest.js.github/scripts/merge-gate.js
MervinPraison
removed
the
pipeline/blocked:cooldown
1 participant
Summary
@claudecooldown whenfinalClaudeCompletedOnShais already true (FINAL done on HEAD — no need to wait after the trigger that finished review)cancel-in-progress: falseso an in-flight assess/merge is not killed by the next scanExpected timing after merge-ready
Test plan
node .github/scripts/merge-gate-selftest.js(new cooldown test passes)Made with Cursor
Summary by CodeRabbit
Bug Fixes
Chores