fix: pipeline status labels and reliable merge-gate dispatch#2343
Conversation
Filter fork sync PRs in resolvePrNumberFromWorkflowRun, dispatch merge gate explicitly from claude.yml after PR runs, and sync pipeline/* stage/blocker labels via auto-pr-comment, merge-gate scan, and 15-min sync workflow. 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? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds pipeline-status label syncing, merge-gate PR-number resolution, self-tests, and workflow steps that sync labels or dispatch merge-gate events. ChangesPipeline status and merge-gate automation
Sequence Diagram(s)PR-triggered label sync and merge-gate dispatch sequenceDiagram
participant ClaudeResponse as claude.yml claude-response job
participant PipelineStatus as pipeline-status.js
participant MergeGate as merge-gate.js
participant GitHubAPI as GitHub REST API
ClaudeResponse->>PipelineStatus: ensurePipelineLabels(...) and syncPipelineLabels(...)
PipelineStatus->>GitHubAPI: list labels and update PR labels
ClaudeResponse->>MergeGate: evaluate pipeline readiness
MergeGate->>GitHubAPI: inspect PR and workflow state
ClaudeResponse->>GitHubAPI: createDispatchEvent("claude-merge-gate", { pr_number })
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 pipeline status synchronization mechanism to manage GitHub labels representing PR stages and blockers, along with corresponding self-tests. It also refactors the merge gate logic to extract helper functions for resolving PR numbers from linked pull requests and head branches. The review feedback suggests several optimizations and robustness improvements: prioritizing checking linked PRs over head branches to reduce API calls, wrapping label creation in a try-catch block to handle existing labels gracefully, and avoiding unnecessary pagination when fetching open pull requests if the limit is small.
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.
| async function resolvePrNumberFromWorkflowRun(github, owner, repo, workflowRun) { | ||
| const fromBranch = await resolvePrNumberFromHeadBranch( | ||
| github, owner, repo, workflowRun.head_branch | ||
| ); | ||
| if (fromBranch) return fromBranch; | ||
|
|
||
| const fromLinked = await resolvePrNumberFromLinkedPullRequests( | ||
| github, owner, repo, workflowRun.pull_requests | ||
| ); | ||
| if (fromLinked) return fromLinked; | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Checking the head branch first via resolvePrNumberFromHeadBranch always triggers a pulls.list API call. However, if the workflow run already has linked pull requests (workflowRun.pull_requests), we can validate them first using resolvePrNumberFromLinkedPullRequests. This is both more accurate (since it uses the explicit association from GitHub) and more efficient (as it avoids the list API call entirely when a valid linked PR is found).
| async function resolvePrNumberFromWorkflowRun(github, owner, repo, workflowRun) { | |
| const fromBranch = await resolvePrNumberFromHeadBranch( | |
| github, owner, repo, workflowRun.head_branch | |
| ); | |
| if (fromBranch) return fromBranch; | |
| const fromLinked = await resolvePrNumberFromLinkedPullRequests( | |
| github, owner, repo, workflowRun.pull_requests | |
| ); | |
| if (fromLinked) return fromLinked; | |
| return null; | |
| } | |
| async function resolvePrNumberFromWorkflowRun(github, owner, repo, workflowRun) { | |
| const fromLinked = await resolvePrNumberFromLinkedPullRequests( | |
| github, owner, repo, workflowRun.pull_requests | |
| ); | |
| if (fromLinked) return fromLinked; | |
| const fromBranch = await resolvePrNumberFromHeadBranch( | |
| github, owner, repo, workflowRun.head_branch | |
| ); | |
| if (fromBranch) return fromBranch; | |
| return null; | |
| } |
| async function ensurePipelineLabels(github, owner, repo, core) { | ||
| const { data: existing } = await github.rest.issues.listLabelsForRepo({ | ||
| owner, | ||
| repo, | ||
| per_page: 100, | ||
| }); | ||
| const names = new Set(existing.map((l) => l.name)); | ||
| for (const spec of LABEL_SPECS) { | ||
| if (names.has(spec.name)) continue; | ||
| await github.rest.issues.createLabel({ | ||
| owner, | ||
| repo, | ||
| name: spec.name, | ||
| color: spec.color, | ||
| description: spec.description, | ||
| }); | ||
| core?.info?.(`Created label ${spec.name}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
If a repository has more than 100 labels, or if there is a race condition where multiple workflow runs attempt to sync labels concurrently, createLabel will throw a 422 Unprocessable Entity error because the label already exists. Wrapping the createLabel call in a try/catch block to gracefully handle 422 or already_exists errors prevents the entire sync process from crashing.
async function ensurePipelineLabels(github, owner, repo, core) {
const { data: existing } = await github.rest.issues.listLabelsForRepo({
owner,
repo,
per_page: 100,
});
const names = new Set(existing.map((l) => l.name));
for (const spec of LABEL_SPECS) {
if (names.has(spec.name)) continue;
try {
await github.rest.issues.createLabel({
owner,
repo,
name: spec.name,
color: spec.color,
description: spec.description,
});
core?.info?.('Created label ' + spec.name);
} catch (err) {
if (err.status === 422 || err.message?.includes('already_exists')) {
core?.info?.('Label ' + spec.name + ' already exists');
} else {
throw err;
}
}
}
}| async function syncOpenPullRequests(github, owner, repo, options, core) { | ||
| const { maxPrs = 20 } = options || {}; | ||
| await ensurePipelineLabels(github, owner, repo, core); | ||
| const prs = await github.paginate(github.rest.pulls.list, { | ||
| owner, | ||
| repo, | ||
| state: 'open', | ||
| per_page: 100, | ||
| }); | ||
| let synced = 0; | ||
| for (const pr of prs) { | ||
| if (synced >= maxPrs) break; | ||
| if (pr.draft) continue; | ||
| if (pr.head?.repo?.full_name && pr.head.repo.full_name !== `${owner}/${repo}`) continue; | ||
| await syncPipelineLabels(github, owner, repo, pr.number, core); | ||
| synced += 1; | ||
| } | ||
| core?.info?.(`Pipeline label sync complete (${synced} PR(s))`); | ||
| return synced; | ||
| } |
There was a problem hiding this comment.
Using github.paginate will fetch all open pull requests in the repository. If a repository has hundreds of open PRs, this consumes significant API rate limits and causes unnecessary overhead, especially since the sync is capped at maxPrs (which defaults to 20). If maxPrs is less than or equal to 100, we can fetch just the first page using github.rest.pulls.list to optimize performance.
async function syncOpenPullRequests(github, owner, repo, options, core) {
const { maxPrs = 20 } = options || {};
await ensurePipelineLabels(github, owner, repo, core);
let prs = [];
if (maxPrs <= 100) {
const { data } = await github.rest.pulls.list({
owner,
repo,
state: 'open',
per_page: 100,
});
prs = data;
} else {
prs = await github.paginate(github.rest.pulls.list, {
owner,
repo,
state: 'open',
per_page: 100,
});
}
let synced = 0;
for (const pr of prs) {
if (synced >= maxPrs) break;
if (pr.draft) continue;
if (pr.head?.repo?.full_name && pr.head.repo.full_name !== `${owner}/${repo}`) continue;
await syncPipelineLabels(github, owner, repo, pr.number, core);
synced += 1;
}
core?.info?.('Pipeline label sync complete (' + synced + ' PR(s))');
return synced;
}
Greptile SummaryThis PR updates the review pipeline automation and status reporting. The main changes are:
Confidence Score: 4/5This is close, but the merge-gate dispatch path should be fixed before merging.
.github/workflows/claude-merge-gate.yml Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
.github/scripts/merge-gate.js (1)
308-313: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
base.repo.namefallback never matches theowner/repocomparison.
link?.base?.repo?.nameis the bare repo name (e.g.PraisonAI), which can never equal${owner}/${repo}(MervinPraison/PraisonAI). The fallback only ever yields a false negative; thebaseUrlcheck is the effective secondary path. Consider dropping the fallback or comparing it againstrepoalone to make intent clear.🤖 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 308 - 313, The fallback in isInternalPullRequestLink is comparing link?.base?.repo?.name against owner/repo, but repo.name is only the bare repository name and can never match that full string. Update the logic in isInternalPullRequestLink to either remove the repo.name fallback or compare it only to repo, while keeping the baseUrl.endsWith(/repos/${owner}/${repo}) check as the secondary path so the intent is clear..github/scripts/pipeline-status.js (1)
93-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
ensurePipelineLabelsonly reads the first 100 labels and doesn't guardcreateLabelagainst duplicates.
listLabelsForRepois called withper_page: 100and no pagination. If the repo has more than 100 labels and an existingpipeline/*label falls outside the first page,names.has(...)misses it andcreateLabelthrows a 422 (already_exists), aborting the whole sync. Usegithub.paginateor tolerate the 422.♻️ Proposed hardening
- const { data: existing } = await github.rest.issues.listLabelsForRepo({ - owner, - repo, - per_page: 100, - }); + const existing = await github.paginate(github.rest.issues.listLabelsForRepo, { + owner, + repo, + per_page: 100, + }); const names = new Set(existing.map((l) => l.name)); for (const spec of LABEL_SPECS) { if (names.has(spec.name)) continue; - await github.rest.issues.createLabel({ - owner, - repo, - name: spec.name, - color: spec.color, - description: spec.description, - }); - core?.info?.(`Created label ${spec.name}`); + try { + await github.rest.issues.createLabel({ + owner, repo, name: spec.name, color: spec.color, description: spec.description, + }); + core?.info?.(`Created label ${spec.name}`); + } catch (err) { + if (err.status !== 422) throw err; + } }🤖 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 93 - 111, ensurePipelineLabels currently only checks the first page of labels and can fail when an existing pipeline label is beyond the initial 100 results. Update ensurePipelineLabels to paginate through all labels (for example via github.paginate around issues.listLabelsForRepo) before building the names set, and also make the createLabel call resilient to a duplicate-existing-label 422 so the sync continues if another process created the label first..github/workflows/pipeline-status-sync.yml (1)
6-7: 🚀 Performance & Scalability | 🔵 TrivialCron cadence vs. API cost.
Running
syncOpenPullRequestswithmaxPrs: 30every 15 minutes means each cycle fans out into many REST/GraphQL calls per PR (loadPrContext+evaluatePipelineQuiescent+ label add/remove). On a busy repo this can pressure the secondary rate limit forGH_TOKEN. Consider a longer interval (e.g.*/30) or confirm the per-cycle call budget stays well under limits, since the PR-, scan-, and scheduled-paths now all sync the same labels.Also applies to: 29-31
🤖 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/pipeline-status-sync.yml around lines 6 - 7, The scheduled run in the pipeline status sync workflow is too aggressive for the amount of GitHub API traffic generated by syncOpenPullRequests and related label updates. Update the cron cadence in the workflow schedule to a longer interval, or otherwise adjust the syncOpenPullRequests/maxPrs budget so the combined load from loadPrContext, evaluatePipelineQuiescent, and label add/remove stays comfortably within GH_TOKEN rate limits..github/workflows/auto-pr-comment.yml (2)
219-219: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueUnpinned action references flagged by policy.
zizmor reports these
actions/checkout@v4/actions/github-script@v7refs as unpinned under the blanket pin-to-hash policy. The rest of this file uses the same unpinned@vNstyle, so pinning only the new lines would be inconsistent — if the policy is intended to be enforced, pin repo-wide; otherwise this is informational.Also applies to: 335-335, 362-362, 459-459, 589-589, 676-676
🤖 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/auto-pr-comment.yml at line 219, The workflow uses unpinned GitHub Actions references, and the policy check flags the new actions/checkout and actions/github-script entries as inconsistent with the required pin-to-hash standard. Update the action references in this workflow consistently by pinning all uses of actions/checkout and actions/github-script to immutable commit SHAs, or, if that policy is not meant to be enforced here, keep the versioned refs uniform across the file. Use the existing action usages in the workflow to locate all affected steps, including the repeated checkout and github-script invocations.Source: Linters/SAST tools
334-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffIdentical pipeline-sync step duplicated across four jobs.
This same 6-line
require(...pipeline-status.js)+ensurePipelineLabels+syncPipelineLabelsblock now appears in four jobs (and again inclaude.yml). Consider extracting a local composite action (e.g..github/actions/sync-pipeline-labels) to keep token/permission handling and the script path consistent in one place.Also applies to: 458-470, 588-598, 673-684
🤖 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/auto-pr-comment.yml around lines 334 - 346, The pipeline label sync logic is duplicated across multiple workflow jobs, so extract the repeated actions/github-script block that loads pipeline-status.js and calls ensurePipelineLabels/syncPipelineLabels into a reusable local composite action such as .github/actions/sync-pipeline-labels. Update each job to invoke that action instead of inlining the script, keeping PR_NUMBER, github-token, and permission handling consistent in one place..github/workflows/claude.yml (1)
361-366: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPass
pr_numberviaenv, not inline template expansion.
parseInt('${{ steps.check_fork.outputs.pr_number }}', 10)interpolates the value directly into the script body (flagged by zizmortemplate-injection). Althoughpr_numberis currently set fromString(prNumber)and is numeric, the othergithub-scriptsteps in this repo pass values throughenv:and readprocess.env. Mirror that pattern here for consistency and to harden against future changes.🔒 Suggested change (both steps)
- name: Sync pipeline status labels for PR if: steps.check_fork.outputs.is_pr == 'true' uses: actions/github-script@v7 env: PR_NUMBER: ${{ steps.check_fork.outputs.pr_number }} with: github-token: ${{ secrets.GH_TOKEN }} script: | const path = require('path'); const ps = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pipeline-status.js')); const prNumber = parseInt(process.env.PR_NUMBER, 10); await ps.ensurePipelineLabels(github, context.repo.owner, context.repo.repo, core); await ps.syncPipelineLabels(github, context.repo.owner, context.repo.repo, prNumber, core);Apply the same
env: PR_NUMBER+process.env.PR_NUMBERapproach to the dispatch step at Line 378.Also applies to: 373-378
🤖 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.yml around lines 361 - 366, The github-script step is still interpolating pr_number directly into the script body, which triggers template-injection risk and is inconsistent with the repo’s other steps. Update the pipeline status label sync block to pass PR_NUMBER through env and read it with process.env.PR_NUMBER inside the script, using the existing ps.ensurePipelineLabels and ps.syncPipelineLabels flow. Apply the same env-based pattern to the dispatch step as well, so both github-script invocations avoid inline template expansion.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/scripts/merge-gate.js:
- Around line 308-313: The fallback in isInternalPullRequestLink is comparing
link?.base?.repo?.name against owner/repo, but repo.name is only the bare
repository name and can never match that full string. Update the logic in
isInternalPullRequestLink to either remove the repo.name fallback or compare it
only to repo, while keeping the baseUrl.endsWith(/repos/${owner}/${repo}) check
as the secondary path so the intent is clear.
In @.github/scripts/pipeline-status.js:
- Around line 93-111: ensurePipelineLabels currently only checks the first page
of labels and can fail when an existing pipeline label is beyond the initial 100
results. Update ensurePipelineLabels to paginate through all labels (for example
via github.paginate around issues.listLabelsForRepo) before building the names
set, and also make the createLabel call resilient to a duplicate-existing-label
422 so the sync continues if another process created the label first.
In @.github/workflows/auto-pr-comment.yml:
- Line 219: The workflow uses unpinned GitHub Actions references, and the policy
check flags the new actions/checkout and actions/github-script entries as
inconsistent with the required pin-to-hash standard. Update the action
references in this workflow consistently by pinning all uses of actions/checkout
and actions/github-script to immutable commit SHAs, or, if that policy is not
meant to be enforced here, keep the versioned refs uniform across the file. Use
the existing action usages in the workflow to locate all affected steps,
including the repeated checkout and github-script invocations.
- Around line 334-346: The pipeline label sync logic is duplicated across
multiple workflow jobs, so extract the repeated actions/github-script block that
loads pipeline-status.js and calls ensurePipelineLabels/syncPipelineLabels into
a reusable local composite action such as .github/actions/sync-pipeline-labels.
Update each job to invoke that action instead of inlining the script, keeping
PR_NUMBER, github-token, and permission handling consistent in one place.
In @.github/workflows/claude.yml:
- Around line 361-366: The github-script step is still interpolating pr_number
directly into the script body, which triggers template-injection risk and is
inconsistent with the repo’s other steps. Update the pipeline status label sync
block to pass PR_NUMBER through env and read it with process.env.PR_NUMBER
inside the script, using the existing ps.ensurePipelineLabels and
ps.syncPipelineLabels flow. Apply the same env-based pattern to the dispatch
step as well, so both github-script invocations avoid inline template expansion.
In @.github/workflows/pipeline-status-sync.yml:
- Around line 6-7: The scheduled run in the pipeline status sync workflow is too
aggressive for the amount of GitHub API traffic generated by
syncOpenPullRequests and related label updates. Update the cron cadence in the
workflow schedule to a longer interval, or otherwise adjust the
syncOpenPullRequests/maxPrs budget so the combined load from loadPrContext,
evaluatePipelineQuiescent, and label add/remove stays comfortably within
GH_TOKEN rate limits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91a7cb4f-502f-44ee-aa33-965c2aa47cc4
📒 Files selected for processing (8)
.github/scripts/merge-gate-selftest.js.github/scripts/merge-gate.js.github/scripts/pipeline-status-selftest.js.github/scripts/pipeline-status.js.github/workflows/auto-pr-comment.yml.github/workflows/claude-merge-gate.yml.github/workflows/claude.yml.github/workflows/pipeline-status-sync.yml
|
@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 |
|
Claude finished @MervinPraison's task —— View job FINAL Architecture Review — PR #2343Scope note: This PR is CI/automation only ( Fixes applied (pushed to
|
- Reorder resolvePrNumberFromWorkflowRun to check linked PRs before head branch (Gemini): more accurate and avoids a pulls.list call - Drop unreachable base.repo.name fallback in isInternalPullRequestLink (CodeRabbit): bare repo name can never match owner/repo - Paginate labels and tolerate 422 in ensurePipelineLabels (Gemini/CodeRabbit) - Skip full pagination when maxPrs <= 100 in syncOpenPullRequests (Gemini) 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
pipeline/blocked:ci
|
Claude finished @MervinPraison's task —— View job FINAL Architecture Review — PR #2343Scope: CI/automation only (
Verification — all editable (
|
1 participant
Summary
resolvePrNumberFromWorkflowRunignores fork sync PR links (fixes 404 on [pull] main from MervinPraison:main Milkmange/PraisonAI#21) and validates internal PRs before returningclaude.ymldispatchesclaude-merge-gatewithpr_numberafter PR@clauderuns when pipeline is readypipeline/*stage + blocker labels synced fromevaluatePipelineQuiescentviapipeline-status.jspipeline-status-sync.yml(every 15 min)Test plan
node .github/scripts/merge-gate-selftest.jsnode .github/scripts/pipeline-status-selftest.jspipeline/merge-ready,pipeline/blocked:manual-review)Made with Cursor
Summary by CodeRabbit
New Features
pipeline/*stage andpipeline/blocked:*labels aligned for open pull requests.Bug Fixes