Add automation pipeline (merge gate, CI, PyPI release)#5
Conversation
Merge gate, pipeline status, CI failure Claude, nightly release gate, PyPI release workflow, and repo-specific gate-config for plugin lifecycle scope.
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? |
📝 WalkthroughWalkthroughThis PR adds a comprehensive GitHub Actions automation suite for PraisonAI-Plugins: CI testing, bot-opened PR review chaining, merge-gate quiescence evaluation and auto-merge, CI-failure detection with Claude fix triggers, pipeline status label syncing, and release-gate/PyPI publish automation, each with helper scripts and self-tests, plus minor unused-import cleanups. ChangesCI and Merge Automation Pipeline
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant ReviewChain as pr-review-chain.js
participant MergeGate as merge-gate.js
participant PipelineStatus as pipeline-status.js
participant ClaudeGate as claude-merge-gate.yml
participant GitHub
PR->>ReviewChain: advanceReviewChain
ReviewChain->>GitHub: post Copilot / `@claude` FINAL comments
PipelineStatus->>MergeGate: evaluatePipelineQuiescent
MergeGate-->>PipelineStatus: ready, reasons
PipelineStatus->>GitHub: sync stage/blocker labels
PipelineStatus->>ClaudeGate: dispatchMergeGateForOldestReady
ClaudeGate->>GitHub: post MERGE_GATE_VERDICT
ClaudeGate->>GitHub: merge PR when approved and ready
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Unblocks CI introduced by the automation pipeline workflow.
![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 suite of GitHub Actions automation scripts under .github/scripts/ to manage PR review chains, CI failure responses, merge gates, pipeline status syncing, and release preflights. The reviewer feedback highlights several opportunities to improve robustness by using optional chaining (?.) when accessing properties of potentially undefined objects (such as c.user or GraphQL query results) to prevent runtime TypeError crashes. Additionally, a minor copy-paste leftover in an error message within release-gate.js was identified for cleanup.
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.
|
|
||
| function isFinalClaudeTriggerComment(c) { | ||
| const body = (c.body || '').toLowerCase(); | ||
| if (!AUTO_ACTORS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined (e.g., when a user account is deleted or for certain system actors). Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!AUTO_ACTORS.includes(c.user.login)) return false; | |
| if (!AUTO_ACTORS.includes(c.user?.login)) return false; |
| function hasRecentClaudeTrigger(comments, minutes = 35) { | ||
| const cutoff = Date.now() - minutes * 60 * 1000; | ||
| return comments.some((c) => { | ||
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined. Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user.login)) return false; | |
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user?.login)) return false; |
| } | ||
|
|
||
| function isConflictRebaseTriggerComment(c) { | ||
| if (!AUTO_ACTORS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined. Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!AUTO_ACTORS.includes(c.user.login)) return false; | |
| if (!AUTO_ACTORS.includes(c.user?.login)) return false; |
| }); | ||
| if (claudeRepliedAfterFinal) return false; | ||
| const claudeSinceHead = comments.some((c) => { | ||
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined. Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user.login)) return false; | |
| if (!CLAUDE_TRIGGER_LOGINS.includes(c.user?.login)) return false; |
| const prGql = result.repository.pullRequest; | ||
| const status = (prGql?.mergeStateStatus || '').toUpperCase(); | ||
| if (status && status !== 'UNKNOWN') { | ||
| return { | ||
| status, | ||
| isDraft: prGql.isDraft, | ||
| headRepo: prGql.headRef?.repository?.nameWithOwner, | ||
| headSha: prGql.headRefOid, | ||
| maintainerCanModify: prGql.maintainerCanModify === true, | ||
| }; |
There was a problem hiding this comment.
Accessing properties of result.repository and prGql directly without optional chaining can throw a TypeError if the GraphQL query fails, returns partial errors, or if the pull request cannot be found. Using optional chaining (result?.repository?.pullRequest and prGql?.isDraft, etc.) makes the code more robust and prevents unhandled exceptions.
| const prGql = result.repository.pullRequest; | |
| const status = (prGql?.mergeStateStatus || '').toUpperCase(); | |
| if (status && status !== 'UNKNOWN') { | |
| return { | |
| status, | |
| isDraft: prGql.isDraft, | |
| headRepo: prGql.headRef?.repository?.nameWithOwner, | |
| headSha: prGql.headRefOid, | |
| maintainerCanModify: prGql.maintainerCanModify === true, | |
| }; | |
| const prGql = result?.repository?.pullRequest; | |
| const status = (prGql?.mergeStateStatus || '').toUpperCase(); | |
| if (status && status !== 'UNKNOWN') { | |
| return { | |
| status, | |
| isDraft: prGql?.isDraft, | |
| headRepo: prGql?.headRef?.repository?.nameWithOwner, | |
| headSha: prGql?.headRefOid, | |
| maintainerCanModify: prGql?.maintainerCanModify === true, | |
| }; | |
| } |
| headRepo: pr.head.repo?.full_name, | ||
| headSha: pr.head.sha, |
There was a problem hiding this comment.
If pr.head is null or undefined (e.g., if the head branch or fork repository was deleted), accessing pr.head.sha directly will throw a TypeError. Using optional chaining pr.head?.sha ensures safe property access.
| headRepo: pr.head.repo?.full_name, | |
| headSha: pr.head.sha, | |
| headRepo: pr.head?.repo?.full_name, | |
| headSha: pr.head?.sha, |
| function hasCiFixCommentForSha(comments, headSha) { | ||
| const marker = shortSha(headSha).toLowerCase(); | ||
| return comments.some((c) => { | ||
| if (!AUTO_ACTORS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined. Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!AUTO_ACTORS.includes(c.user.login)) return false; | |
| if (!AUTO_ACTORS.includes(c.user?.login)) return false; |
| const cutoff = Date.now() - COOLDOWN_MS; | ||
| const marker = shortSha(headSha).toLowerCase(); | ||
| return comments.some((c) => { | ||
| if (!AUTO_ACTORS.includes(c.user.login)) return false; |
There was a problem hiding this comment.
Directly accessing c.user.login without optional chaining can cause a TypeError if c.user is null or undefined. Using optional chaining c.user?.login ensures defensive programming and prevents the script from crashing.
| if (!AUTO_ACTORS.includes(c.user.login)) return false; | |
| if (!AUTO_ACTORS.includes(c.user?.login)) return false; |
| const path = require('path'); | ||
| const toml = fs.readFileSync(path.join(root, 'pyproject.toml'), 'utf8'); | ||
| const match = toml.match(/^version\s*=\s*"([^"]+)"/m); | ||
| if (!match) throw new Error('Could not read aiui version from pyproject.toml'); |
There was a problem hiding this comment.
The error message mentions 'aiui version', which appears to be a copy-paste leftover from the PraisonAIUI repository. It should be updated to refer to the correct package or simply 'version' to avoid confusion.
| if (!match) throw new Error('Could not read aiui version from pyproject.toml'); | |
| if (!match) throw new Error('Could not read version from pyproject.toml'); |
Greptile SummaryThis PR ports the PraisonAIUI automation pipeline to the Plugins repo, adding a merge gate, CI failure → Claude trigger, nightly PyPI release gate, pipeline status label sync, a unified CI workflow, and a customised review chain. Three Python plugin files receive minor cleanup (unused import removals).
Confidence Score: 3/5Safe to review but two concrete runtime defects need fixing before the pipeline is fully operational. The sync-secrets workflow will always fail because GITHUB_TOKEN cannot dispatch workflows in a foreign repository. Separately, in pypi-release.yml the git tag is stamped before the rebase, so when a concurrent commit lands on main the tag ends up referencing an orphaned commit rather than the version actually shipped.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
PR_OPEN([PR opened or synchronized]) --> CR[CodeRabbit posts summary]
PR_OPEN --> BOT_PR{Bot PR?}
BOT_PR -- Yes --> KICK[bot-pr-review-chain kicks CodeRabbit and Qodo]
BOT_PR -- No --> WAIT_CR[Wait for CodeRabbit or Qodo]
CR --> COPILOT[Trigger copilot via PAT as MervinPraison]
KICK --> CR
COPILOT --> POLL[Poll for Copilot response 30s x 20 attempts]
POLL -- responded --> CLAUDE_FINAL[Post at-claude FINAL]
POLL -- timeout --> CLAUDE_FINAL
SCHEDULE([Scheduled or pull_request_target sync]) --> RECOVERY[claude-review-recovery stale FINAL check]
RECOVERY -- stale or missing --> CLAUDE_FINAL
CLAUDE_FINAL --> PIPELINE_LABEL[Sync pipeline labels]
PIPELINE_LABEL --> MERGE_GATE{pipeline/merge-ready?}
MERGE_GATE -- Yes --> AUTO_MERGE[claude-merge-gate auto-merge PR]
MERGE_GATE -- No --> BLOCKED[blocked:ci or conflict or manual-review or cooldown]
CI_FAIL([CI fails on PR]) --> CI_FIX[ci-failure-claude posts at-claude CI fix comment]
CI_GREEN([CI green on main]) --> NIGHTLY[nightly-release-gate preflight]
NIGHTLY -- pass --> PYPI[pypi-release bump then publish then commit then tag]
NIGHTLY -- fail --> SKIP[Skip release]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
PR_OPEN([PR opened or synchronized]) --> CR[CodeRabbit posts summary]
PR_OPEN --> BOT_PR{Bot PR?}
BOT_PR -- Yes --> KICK[bot-pr-review-chain kicks CodeRabbit and Qodo]
BOT_PR -- No --> WAIT_CR[Wait for CodeRabbit or Qodo]
CR --> COPILOT[Trigger copilot via PAT as MervinPraison]
KICK --> CR
COPILOT --> POLL[Poll for Copilot response 30s x 20 attempts]
POLL -- responded --> CLAUDE_FINAL[Post at-claude FINAL]
POLL -- timeout --> CLAUDE_FINAL
SCHEDULE([Scheduled or pull_request_target sync]) --> RECOVERY[claude-review-recovery stale FINAL check]
RECOVERY -- stale or missing --> CLAUDE_FINAL
CLAUDE_FINAL --> PIPELINE_LABEL[Sync pipeline labels]
PIPELINE_LABEL --> MERGE_GATE{pipeline/merge-ready?}
MERGE_GATE -- Yes --> AUTO_MERGE[claude-merge-gate auto-merge PR]
MERGE_GATE -- No --> BLOCKED[blocked:ci or conflict or manual-review or cooldown]
CI_FAIL([CI fails on PR]) --> CI_FIX[ci-failure-claude posts at-claude CI fix comment]
CI_GREEN([CI green on main]) --> NIGHTLY[nightly-release-gate preflight]
NIGHTLY -- pass --> PYPI[pypi-release bump then publish then commit then tag]
NIGHTLY -- fail --> SKIP[Skip release]
Reviews (2): Last reviewed commit: "Harden automation scripts: optional chai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| git tag -f "${TAG}" | ||
| git pull --rebase origin main | ||
| git push origin main | ||
| git push --tags -f |
There was a problem hiding this comment.
git push --tags -f force-pushes every local tag, so if a tag like v0.3.128 was already pushed (e.g., by a previous run that failed mid-way), this will silently move it to a different commit. A lost tag makes it impossible to reproduce that build from source. Using --force-with-lease or omitting -f and letting the push fail on duplicate tags is safer; the job can then exit with a clear error rather than corrupting tag history.
| git push --tags -f | |
| git push --tags |
| core.setOutput('triggered', 'true'); | ||
| core.setOutput('pr_number', context.issue.number.toString()); | ||
|
|
||
| # ================================================================ | ||
| # FALLBACK: Claude trigger for cases where Copilot chain fails/skips | ||
| # Ensures Claude always runs after sufficient review time | ||
| # ================================================================ | ||
| claude-fallback-timeout: | ||
| if: | | ||
| github.event_name == 'pull_request' && | ||
| github.event.action == 'opened' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 20 | ||
| permissions: | ||
| issues: write | ||
| pull-requests: write | ||
| steps: | ||
| - name: Wait for review window (15 min) | ||
| run: sleep 900 | ||
|
|
||
| - name: Check if Claude already triggered | ||
| id: check_claude | ||
| uses: actions/github-script@v7 | ||
| env: | ||
| TRIGGER_LOGINS: ${{ env.CLAUDE_TRIGGER_LOGINS }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| with: | ||
| github-token: ${{ env.GH_TOKEN }} | ||
| script: | | ||
| const pr = parseInt(process.env.PR_NUMBER, 10); | ||
| const triggerLogins = JSON.parse(process.env.TRIGGER_LOGINS); | ||
| const comments = await github.rest.issues.listComments({ | ||
| issue_number: pr, | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo | ||
| }); | ||
| const alreadyPosted = comments.data.some(c => | ||
| c.body.includes('@claude') && triggerLogins.includes(c.user.login) | ||
| ); | ||
| core.setOutput('already_triggered', alreadyPosted ? 'true' : 'false'); | ||
| core.setOutput('comments_count', comments.data.length.toString()); | ||
|
|
||
| const botReviews = comments.data.filter(c => | ||
| ['coderabbitai[bot]', 'qodo-code-review[bot]', 'gemini-code-assist[bot]', | ||
| 'copilot-swe-agent', 'Copilot', 'greptile-apps[bot]'].some(bot => | ||
| c.user.login.toLowerCase().includes(bot.toLowerCase()) | ||
| ) | ||
| ); | ||
| core.setOutput('review_count', botReviews.length.toString()); | ||
|
|
||
| - name: Aggregate reviews and trigger Claude | ||
| if: steps.check_claude.outputs.already_triggered != 'true' | ||
| uses: actions/github-script@v7 | ||
| env: | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| with: | ||
| github-token: ${{ env.GH_TOKEN }} | ||
| script: | | ||
| const pr = parseInt(process.env.PR_NUMBER, 10); | ||
| const comments = await github.rest.issues.listComments({issue_number: pr, owner: context.repo.owner, repo: context.repo.repo, per_page: 100}); | ||
| const reviews = {coderabbit: [], qodo: [], gemini: [], copilot: [], greptile: [], others: []}; | ||
| comments.data.forEach(c => { | ||
| const login = c.user.login.toLowerCase(); | ||
| const body = c.body.substring(0, 500); | ||
| if (login.includes('coderabbit')) reviews.coderabbit.push(body); | ||
| else if (login.includes('qodo')) reviews.qodo.push(body); | ||
| else if (login.includes('gemini')) reviews.gemini.push(body); | ||
| else if (login.includes('copilot')) reviews.copilot.push(body); | ||
| else if (login.includes('greptile')) reviews.greptile.push(body); | ||
| else if (c.user.type === 'Bot') reviews.others.push(body); | ||
| }); | ||
| const summaryParts = []; | ||
| if (reviews.coderabbit.length) summaryParts.push('CodeRabbit: ' + reviews.coderabbit.length + ' comments'); | ||
| if (reviews.qodo.length) summaryParts.push('Qodo: ' + reviews.qodo.length + ' comments'); | ||
| if (reviews.gemini.length) summaryParts.push('Gemini: ' + reviews.gemini.length + ' comments'); | ||
| if (reviews.copilot.length) summaryParts.push('Copilot: ' + reviews.copilot.length + ' comments'); | ||
| if (reviews.greptile.length) summaryParts.push('Greptile: ' + reviews.greptile.length + ' comments'); | ||
| const reviewSummary = summaryParts.length ? summaryParts.join(' | ') : 'No bot reviews detected'; | ||
| const scope = process.env.CLAUDE_UI_SCOPE; | ||
| const phases = process.env.CLAUDE_FINAL_PHASES_COMPACT; | ||
| const claudeBody = '@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI-Plugins (not a fork), you are able to make modifications to this branch and push directly. ' + scope + ' Read ALL prior reviewer comments. ' + reviewSummary + '.\n\n' + | ||
| 'Review Context: ' + comments.data.length + ' total comments, ' + | ||
| (reviews.coderabbit.length + reviews.qodo.length + reviews.gemini.length + reviews.copilot.length + reviews.greptile.length) + ' bot reviews.\n\n' + | ||
| phases; | ||
| await github.rest.issues.createComment({issue_number: pr, owner: context.repo.owner, repo: context.repo.repo, body: claudeBody}); | ||
| console.log('Claude fallback triggered on PR #' + pr + ' with ' + reviewSummary); | ||
|
|
||
| # ================================================================ | ||
| # Claude FINAL review — polls for Copilot's response, then triggers | ||
| # This runs IN-BAND under the PAT context (MervinPraison), avoiding |
There was a problem hiding this comment.
claude-fallback-timeout blocks a runner for 15 minutes per PR open event
The job runs an unconditional sleep 900 on a GitHub-hosted runner, consuming 15 minutes of billing time per PR regardless of whether the Claude fallback is ultimately needed. There is also no concurrency group on this job, so a burst of PR-open events could leave many sleeping runners idle simultaneously. The existing post-missing-claude-final scheduled job serves the same purpose without holding a runner slot.
| skip: true, | ||
| reason: 'head pushed soon after FINAL (wait for CI / batched fixes)', | ||
| }; | ||
| } | ||
| const windowStart = Date.now() - STALE_FINAL_RECOVERY_WINDOW_MS; | ||
| const finalsInWindow = countFinalTriggersSince(comments, windowStart); | ||
| if (finalsInWindow >= STALE_FINAL_MAX_PER_WINDOW) { | ||
| return { | ||
| skip: true, | ||
| reason: `stale-FINAL capped (${finalsInWindow} FINAL triggers in last hour)`, | ||
| }; |
There was a problem hiding this comment.
hasRecentCiFixComment 12-hour cooldown applies across all SHAs
The function returns true (suppress new CI-fix comment) if any CI-fix comment by an auto-actor exists within the last 12 hours, even when the comment was for a completely different commit SHA. Because hasCiFixCommentForSha (checked first in shouldSkipCiFix) already handles the exact-SHA case, the broad time-based branch here silences assistance for subsequent pushes that fix one failing test but introduce a new one. Tying the cooldown to the head SHA rather than wall-clock time alone would ensure each distinct commit can receive at most one automated CI-fix comment.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI-Plugins (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on PraisonAI-Plugins (src/praisonai_plugins, tests). Do not expand into praisonaiagents or the monorepo unless the issue explicitly requires it. Read ALL prior reviewer comments. CodeRabbit: 1 comments | Qodo: 1 comments | Greptile: 1 comments. Review Context: 3 total comments, 3 bot reviews. Phase 1: Review per AGENTS.md (lazy imports, backward compat, import-time <200ms; SDK/product value gate — genuine plugin lifecycle value, correct plugin vs SDK vs Tools layering, no scope creep) |
|
Claude finished @MervinPraison's task —— View job FINAL Architecture Review — PR #5Todo
Phase 1 — Layer & scope reviewCorrect layering. The Python changes are pure unused-import cleanups ( Phase 2 — Fixes pushed to this branch (commit
|
| File | Fix | Reviewer |
|---|---|---|
merge-gate.js |
c.user?.login in 4 predicates + optional chaining on GraphQL result?.repository?.pullRequest and REST pr.head?.sha/pr.head?.repo |
Gemini (P1) |
ci-failure-claude.js |
c.user?.login in hasCiFixCommentForSha / hasRecentCiFixComment |
Gemini (P1) |
release-gate.js |
"aiui version" → "version" in error message |
Gemini (P2) |
bot-pr-review-chain.js |
qodoKickPosted now matches body.trim() === '/review' (exact) instead of substring, avoiding false positives |
Greptile (P2) |
These prevent TypeError crashes when c.user/pr.head/GraphQL results are null (deleted accounts, system actors, deleted head branch/fork, partial GraphQL errors).
Deliberately NOT changed (with reasoning)
hasRecentCiFixComment12h wall-clock cooldown (Greptile P2): the exact-SHA path (hasCiFixCommentForSha) is checked first inshouldSkipCiFix; the time branch is an intentional rate-limiter guarding against comment spam when HEAD churns rapidly. Removing it changes throttling semantics, so I left it as-is by design rather than silently altering behavior..github/workflows/*items — I cannot modify workflow files (GitHub App permission). The following remain valid manual follow-ups:pypi-release.yml: publish runs before the version-bump push → on push failure, PyPI is live but repopyproject.tomlstays stale and the dedupe gate blocks future retries. Recommend commit+push version bump first, then build+publish.pypi-release.yml:git push --tags -f→ prefer--force-with-leaseor drop-fto avoid rewriting immutable tag history.auto-pr-comment.yml:claude-fallback-timeoutsleep 900holds a runner 15 min/PR with no concurrency group; the scheduledpost-missing-claude-finaljob already covers this.
Phase 3 — Verdict
Approve-ready for the script/Python layer once CI (ruff + pytest 3.10–3.12 + selftests) passes on 4b0b0bd. The three workflow-YAML items above are non-blocking operational rough edges but should be addressed by a maintainer before the pipeline runs on production traffic (the PyPI publish-before-push ordering being the most important).
Note: I could not execute node .github/scripts/*-selftest.js locally (node not in allowed tools); all edits are mechanical ?./exact-match changes and will be validated by the CI selftest step.
--- • branch infra/automation-pipeline
Apply reviewer feedback (Gemini, Greptile): - merge-gate.js / ci-failure-claude.js: use c.user?.login and optional chaining on GraphQL/REST merge-state to avoid TypeError when user/head objects are null (deleted accounts, system actors, partial GraphQL errors, deleted head branch/fork). - release-gate.js: fix copy-paste "aiui version" error message. - bot-pr-review-chain.js: qodoKickPosted now matches full body trim() === '/review' instead of broad substring, preventing false positives from unrelated comments mentioning "/review". Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/auto-pr-comment.yml (1)
49-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSkip Copilot for bot-authored PRs in the generic Copilot trigger jobs.
The new bot PR path posts Claude directly, but
copilot-after-coderabbitandcopilot-after-qodostill match bot-authored PRs and can post@copilotbefore/alongside the direct Claude path. Add an author check before posting Copilot so bot PRs stay on the intended CodeRabbit/Qodo → Claude route.Proposed fix
const comments = await github.rest.issues.listComments({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo }); + const { data: pull } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.issue.number, + }); + if (pull.user.type === 'Bot') { + console.log('Bot PR — direct Claude path handles review chain'); + core.setOutput('triggered', 'false'); + core.setOutput('pr_number', context.issue.number.toString()); + return; + }Apply the same guard in the Qodo-triggered Copilot step before posting
@copilot.Also applies to: 93-109, 671-725
🤖 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 49 - 66, The generic Copilot trigger jobs still fire for bot-authored PRs, which can cause `@copilot` to be posted alongside the direct Claude path. Add an author-based guard in the Copilot posting logic for the `copilot-after-coderabbit` and `copilot-after-qodo` workflows, using the existing trigger steps as the entry points, so bot PRs are skipped and only the intended CodeRabbit/Qodo → Claude route runs.
🟡 Minor comments (7)
.github/scripts/pr-review-chain.js-176-179 (1)
176-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon’t collapse the optional reviewer wait to zero by default.
priorReviewersReady()defaults to waiting for optional Qodo/Gemini signals, but Line 178 overrides an omitted value with0, so the main orchestrator never waits unless callers remember to pass a timeout.🐛 Proposed fix
const chainOpts = { prCreatedAt: pr.created_at, - optionalWaitMs: options.optionalWaitMs ?? 0, + optionalWaitMs: options.optionalWaitMs, };🤖 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/pr-review-chain.js around lines 176 - 179, The main orchestrator in pr-review-chain.js is forcing optionalWaitMs to 0 when it is omitted, which disables the default waiting behavior in priorReviewersReady(). Update chainOpts so optionalWaitMs is only set when explicitly provided by options, and otherwise let priorReviewersReady() use its own default wait logic..github/scripts/pr-review-chain.js-236-241 (1)
236-241: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPaginate pull reviews like issue comments.
Only the first 100 PR reviews are considered. Busy PRs can miss later Copilot/Qodo/Gemini review signals and block or mis-order the chain.
🐛 Proposed fix
- const { data: reviews } = await github.rest.pulls.listReviews({ - owner, - repo, - pull_number: prNumber, - per_page: 100, - }); + let reviews; + if (typeof github.paginate === 'function') { + reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, + repo, + pull_number: prNumber, + per_page: 100, + }); + } else { + const { data } = await github.rest.pulls.listReviews({ + owner, + repo, + pull_number: prNumber, + per_page: 100, + }); + reviews = data; + } return { comments, reviews };🤖 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/pr-review-chain.js around lines 236 - 241, The pull review fetch in `github.rest.pulls.listReviews` only reads the first page, so later review signals can be missed on busy PRs. Update the review-loading logic in the PR chain script to paginate through all review pages, similar to how issue comments are handled, and aggregate every review before downstream processing. Keep the change localized around the existing review retrieval flow so the chain ordering and signal detection use the full set of reviews..github/scripts/bot-pr-review-chain.js-85-92 (1)
85-92: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSearch all open PRs when resolving issue branches.
findOpenPrForIssue()only inspects the first 30 open PRs. An older matching bot PR can be missed and returnno_preven though it exists.🐛 Proposed fix
- const { data: prs } = await github.rest.pulls.list({ - owner, - repo, - state: 'open', - sort: 'created', - direction: 'desc', - per_page: 30, - }); + let prs; + if (typeof github.paginate === 'function') { + prs = await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + sort: 'created', + direction: 'desc', + per_page: 100, + }); + } else { + const { data } = await github.rest.pulls.list({ + owner, + repo, + state: 'open', + sort: 'created', + direction: 'desc', + per_page: 100, + }); + prs = data; + }🤖 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/bot-pr-review-chain.js around lines 85 - 92, The issue is that findOpenPrForIssue only checks the first 30 open pull requests, so older matching bot PRs can be skipped and incorrectly return no_pr. Update the PR lookup in bot-pr-review-chain.js to page through all open PRs from github.rest.pulls.list instead of relying on a single per_page batch, and keep iterating until all pages are searched. Use the findOpenPrForIssue logic as the entry point and preserve the existing matching behavior while expanding the search scope..github/scripts/release-gate.js-64-75 (1)
64-75: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the run update time for the UTC-day gate.
created_atis the workflow start time, so a successful run that finishes after midnight UTC won't be counted for that day.updated_atfits this check better.Proposed fix
return runs.data.workflow_runs.some( - (r) => r.conclusion === 'success' && new Date(r.created_at) >= dayStart + (r) => r.conclusion === 'success' && new Date(r.updated_at || r.created_at) >= dayStart );🤖 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/release-gate.js around lines 64 - 75, The UTC-day success check in hasSuccessfulReleaseToday currently uses the workflow run creation time, which can miss runs that complete after midnight UTC. Update the timestamp comparison in hasSuccessfulReleaseToday to use the run update time instead of created_at, keeping the rest of the listWorkflowRuns and success/conclusion logic the same..github/scripts/ci-failure-claude.js-19-20 (1)
19-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the CI-fix SHA marker formatting.
ciFixShaMarker()currently returnsci failed on head \` without the closing backtick, so any caller comparing against the generated marker will not match the comment text built on Line 126.Proposed fix
function ciFixShaMarker(headSha) { - return `ci failed on head \`${shortSha(headSha)}`.toLowerCase(); + return `ci failed on head \`${shortSha(headSha)}\``.toLowerCase(); }🤖 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/ci-failure-claude.js around lines 19 - 20, The ciFixShaMarker() output is missing the closing backtick around the SHA, so the generated marker text won’t match the comment body used elsewhere. Update ciFixShaMarker() to return the full quoted marker string with matching backticks and keep the same shortSha(headSha) formatting so any caller comparing against it will match correctly..github/scripts/pipeline-status.js-143-145 (1)
143-145: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winOnly remove labels managed by this workflow.
Filtering by
pipeline/removes any future/manual label in that namespace. UseALL_PIPELINE_LABELSso sync only mutates labels this script owns.Proposed fix
const current = ctx.labels.filter((l) => l.startsWith(PIPELINE_PREFIX)); const desired = new Set(all); - const toRemove = current.filter((l) => !desired.has(l) && l !== 'pipeline/merged'); + const managed = new Set(ALL_PIPELINE_LABELS); + const toRemove = current.filter( + (l) => managed.has(l) && !desired.has(l) && l !== 'pipeline/merged' + );🤖 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 143 - 145, The label cleanup logic in the pipeline status sync is too broad because it filters by the pipeline/ prefix instead of only the labels this workflow owns. Update the label removal flow in pipeline-status.js to use ALL_PIPELINE_LABELS as the source of truth, and compute toRemove only from that managed set while preserving pipeline/merged. Keep the existing current and desired set comparison, but restrict removals to labels declared in ALL_PIPELINE_LABELS so manual or future namespace labels are not touched..github/scripts/ci-failure-claude.js-238-243 (1)
238-243: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse workflow job IDs here, not check-run IDs.
mapFailedChecks()forwardscheck_runs[].idfromchecks.listForRef(), butdownloadJobLogsForWorkflowRun({ job_id })expects a workflow job id. That makes the log download fail and leaves failure extraction empty; fetch the matching job ids from the workflow-run jobs endpoint before callingfetchJobFailureSummary().🤖 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/ci-failure-claude.js around lines 238 - 243, The failure summary loop is passing check-run IDs into fetchJobFailureSummary(), but downloadJobLogsForWorkflowRun({ job_id }) needs workflow job IDs instead. Update the failed-check mapping flow around mapFailedChecks(), fetchJobFailureSummary(), and the failedRuns iteration to look up the matching workflow-run job IDs from the jobs endpoint before building failureSummaries. Then pass those job IDs into fetchJobFailureSummary() instead of check.id so log downloads and failure extraction work correctly.
🧹 Nitpick comments (3)
.github/workflows/auto-pr-comment.yml (1)
534-534: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable checkout credential persistence in recovery/sweep jobs.
These jobs only need workspace files, not git credentials. Match the existing hardened checkout usage to avoid leaving tokens in
.git/config.Proposed fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: falseAlso applies to: 614-614
🤖 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 534, The recovery/sweep jobs still use actions/checkout without the hardened credential setting, so update those checkout steps to match the existing secure pattern by disabling credential persistence. Locate the checkout usages in the recovery/sweep job sections and add the same persist-credentials=false behavior used elsewhere so workspace files are checked out without leaving git tokens in .git/config.Source: Linters/SAST tools
.github/scripts/release-gate.js (1)
140-154: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid shell-building the path-filtered git diff.
PACKAGE_PATHSis config-controlled, but this still leaves correctness and command parsing dependent on shell escaping. UseexecFileSyncwith argv.Proposed fix
- const { execSync } = require('child_process'); + const { execFileSync, execSync } = require('child_process'); ... - changed = execSync( - `git diff --name-only ${lastTag} HEAD -- ${PACKAGE_PATHS.join(' ')}`, - { encoding: 'utf8' } - ).trim(); + changed = execFileSync( + 'git', + ['diff', '--name-only', lastTag, 'HEAD', '--', ...PACKAGE_PATHS], + { encoding: 'utf8' } + ).trim();🤖 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/release-gate.js around lines 140 - 154, The git diff command in release-gate.js is being built as a shell string, which makes command parsing depend on escaping even though PACKAGE_PATHS is config-controlled. Update the logic around execSync in the lastTag/changed calculation to use execFileSync with explicit argv instead of interpolating the diff command, and keep the same path filtering behavior by passing lastTag, HEAD, and the package paths as separate arguments.Source: Linters/SAST tools
.github/workflows/ci.yml (1)
18-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout.The
actions/checkout@v4action persists theGITHUB_TOKENcredential in.git/configby default. Since this workflow only runs lint and tests (no push-back), there is no need for persisted credentials. zizmor flagged this as an artipacked risk.🔒️ Recommended fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false🤖 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/ci.yml at line 18, Set the actions/checkout@v4 step to disable credential persistence by adding persist-credentials: false, since this CI workflow only runs lint and tests and does not need the GITHUB_TOKEN stored in .git/config. Update the checkout step in the ci workflow so the action no longer leaves reusable credentials behind.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.
Inline comments:
In @.github/scripts/ci-failure-claude.js:
- Around line 203-206: The fork-permission check in the CI-failure automation is
too permissive because it allows non-base-repo PRs when maintainer edits are
enabled. Update the logic in the PR head repo guard that uses
pr.head.repo?.full_name, baseRepo, and pr.maintainer_can_modify so the workflow
only proceeds when the PR head repo matches the base repo, and skip all other
cases. Keep the existing skip response shape and reason handling in the same
ci-failure-claude.js flow.
- Around line 248-259: The label/comment sequence in `addLabels` and
`createComment` can strand `claude-ci-fix-pending` if the comment fails after
the label is added. Update the flow around `github.rest.issues.addLabels` and
`github.rest.issues.createComment` so the pending label is only left in place
when the comment is successfully created, and if comment creation throws, remove
the label or otherwise roll back the added state before exiting. Keep the fix
localized to the comment-posting path in `ci-failure-claude.js` so future runs
don’t skip forever on a stale pending label.
In @.github/scripts/pr-review-chain.js:
- Around line 167-169: The timeout fallback in advanceReviewChain() is
incorrectly using copilotTriggered(comments) alone to bypass Copilot, which can
cause an immediate Claude FINAL before Copilot has actually timed out. Update
the logic around the allowCopilotTimeout check so it only skips Copilot after a
real elapsed timeout has occurred, using the existing review-chain state and
timeout tracking near advanceReviewChain() and the timeout fallback setup in the
related block. Ensure copilotSkipped is set only when the timeout condition is
truly met, not just because a Copilot trigger comment exists.
- Around line 180-186: The Copilot trigger in pr-review-chain.js only checks
options.skipCopilot, so the REVIEW_CHAIN_SKIP_COPILOT setting is not honored
before posting `@copilot`. Update the Copilot gating in the main flow around
maybeTriggerCopilot to also respect the environment-based skip flag (the same
source used by downstream gating), and ensure the final copilot object reflects
a skipped state whenever either skip path is enabled.
In @.github/workflows/auto-pr-comment.yml:
- Around line 621-666: The scheduled/manual sweep in the workflow script runs
ps.syncPipelineLabels() without guaranteeing the pipeline labels exist first,
which can fail on fresh repos or after label deletion. Update the sweep loop to
call ps.ensurePipelineLabels() before ps.syncPipelineLabels(), using the
existing ps helper in the same block where PRs are processed so missing
pipeline/* labels are created before syncing.
In @.github/workflows/ci-failure-claude.yml:
- Around line 28-31: The workflow permissions block is missing repository
contents access, which can break actions/checkout under restricted tokens.
Update the permissions in the ci-failure-claude workflow to include contents:
read alongside the existing pull-requests, issues, and actions scopes so the
checkout step can access the repository contents. Make the same permission
change wherever the workflow defines this permissions block, including the
duplicated section later in the file.
In @.github/workflows/ci.yml:
- Around line 25-28: The CI install step is referencing an undefined `dev` extra
in the package install command, which should be corrected to match
`pyproject.toml`. Update the `Install dependencies` step in the workflow so it
either installs from a real optional-dependencies group after defining `dev` in
`pyproject.toml`, or remove the `.[dev]` suffix entirely since `ruff` and
`pytest-timeout` are already installed separately. Use the `pip install -e`
command in that workflow block as the locator for the change.
In @.github/workflows/claude-merge-gate.yml:
- Line 61: The workflow’s uses of actions/checkout@v4 are persisting the GitHub
token in git config, which is unsafe around PR code. Update each checkout step
in the merge gate workflow to set persist-credentials to false, and only provide
credentials explicitly in the later git fetch path if needed. Use the
actions/checkout invocations as the anchor points for the change.
- Around line 485-487: The merge call in the workflow is missing the approved
head SHA, leaving a TOCTOU gap between the earlier check and
github.rest.pulls.merge. Update the merge request to pass the validated
scanHeadSha as the sha argument alongside owner, repo, pull_number, and
merge_method so only the assessed commit can be merged.
- Around line 63-69: The GitHub App token requests in the workflow are scoped
only by owner, which gives access to all repositories in the installation.
Update each `actions/create-github-app-token@v1` step in the workflow by adding
the repository-scoping input alongside `owner`, using the same `repositories`
value for every token generation block so the token is limited to
`github.event.repository.name`.
- Around line 311-313: The fallback BLOCK step is guarded by an `if:` that still
inherits GitHub Actions’ implicit success check, so it won’t run when
`steps.assess` fails. Update the conditional on the `fallback_verdict` step in
`claude-merge-gate.yml` to include `always()` alongside `steps.assess.outcome !=
'success'` so the BLOCK comment posts even after assessment failures.
In @.github/workflows/pipeline-status-sync.yml:
- Around line 10-14: The workflow permissions block is missing repository
contents access, which can break actions/checkout in this job. Update the
permissions in pipeline-status-sync workflow to include contents: read alongside
the existing pull-requests, issues, actions, and id-token scopes, and make the
same change in the other permissions block referenced by the comment so checkout
has the required access.
- Around line 24-30: The GitHub App token is currently scoped only to the owner,
which makes it usable across all repositories in that installation. Update the
Generate GitHub App token step in pipeline-status-sync to include the
repositories input alongside owner, using github.event.repository.name, so the
token is limited to this repository before it is consumed by later workflow
steps.
In @.github/workflows/pypi-release.yml:
- Around line 83-112: The version bump logic in the versions step and the
pyproject.toml update step are using different matching rules, so the write path
can silently fail when formatting varies. Update the Bump pyproject.toml step to
use the same whitespace-tolerant matcher as the version-reading logic, and apply
the replacement based on that match instead of a fixed string. Keep the behavior
aligned with the existing versions output generation so current_version and
new_version remain consistent.
- Around line 27-31: The release workflow currently ignores the preflight
trigger SHA and instead checks out and rebases from main, which can publish an
ungated artifact. Update the release job in the workflow to honor
inputs.trigger_sha by either checking out that exact SHA directly or validating
it against origin/main before proceeding, and make the gate explicit in the
release flow so the publish step cannot continue on a mismatched commit.
- Around line 114-135: The release flow in the Publish to PyPI and Commit, tag,
and release steps should push the versioned VCS state before running uv publish.
Reorder the workflow so the commit, tag, and pushes happen first, then publish
to PyPI only after the git tag and GitHub release are safely created, using the
VERSION/TAG logic already in place. Also remove force-moving tag operations in
the release step by avoiding git tag -f and git push --tags -f, and use
non-forced tag creation/push so release tags remain immutable.
In @.github/workflows/sync-secrets-from-praisonai.yml:
- Around line 14-21: The sync-secrets workflow is using secrets.GITHUB_TOKEN to
call gh workflow run against MervinPraison/PraisonAI, but that token cannot
trigger workflows in another repository. Update the job in
sync-secrets-from-praisonai.yml to use a repository secret containing a PAT or
GitHub App token with access to MervinPraison/PraisonAI, and make sure the token
has the required repo and workflow scopes before invoking gh workflow run.
---
Outside diff comments:
In @.github/workflows/auto-pr-comment.yml:
- Around line 49-66: The generic Copilot trigger jobs still fire for
bot-authored PRs, which can cause `@copilot` to be posted alongside the direct
Claude path. Add an author-based guard in the Copilot posting logic for the
`copilot-after-coderabbit` and `copilot-after-qodo` workflows, using the
existing trigger steps as the entry points, so bot PRs are skipped and only the
intended CodeRabbit/Qodo → Claude route runs.
---
Minor comments:
In @.github/scripts/bot-pr-review-chain.js:
- Around line 85-92: The issue is that findOpenPrForIssue only checks the first
30 open pull requests, so older matching bot PRs can be skipped and incorrectly
return no_pr. Update the PR lookup in bot-pr-review-chain.js to page through all
open PRs from github.rest.pulls.list instead of relying on a single per_page
batch, and keep iterating until all pages are searched. Use the
findOpenPrForIssue logic as the entry point and preserve the existing matching
behavior while expanding the search scope.
In @.github/scripts/ci-failure-claude.js:
- Around line 19-20: The ciFixShaMarker() output is missing the closing backtick
around the SHA, so the generated marker text won’t match the comment body used
elsewhere. Update ciFixShaMarker() to return the full quoted marker string with
matching backticks and keep the same shortSha(headSha) formatting so any caller
comparing against it will match correctly.
- Around line 238-243: The failure summary loop is passing check-run IDs into
fetchJobFailureSummary(), but downloadJobLogsForWorkflowRun({ job_id }) needs
workflow job IDs instead. Update the failed-check mapping flow around
mapFailedChecks(), fetchJobFailureSummary(), and the failedRuns iteration to
look up the matching workflow-run job IDs from the jobs endpoint before building
failureSummaries. Then pass those job IDs into fetchJobFailureSummary() instead
of check.id so log downloads and failure extraction work correctly.
In @.github/scripts/pipeline-status.js:
- Around line 143-145: The label cleanup logic in the pipeline status sync is
too broad because it filters by the pipeline/ prefix instead of only the labels
this workflow owns. Update the label removal flow in pipeline-status.js to use
ALL_PIPELINE_LABELS as the source of truth, and compute toRemove only from that
managed set while preserving pipeline/merged. Keep the existing current and
desired set comparison, but restrict removals to labels declared in
ALL_PIPELINE_LABELS so manual or future namespace labels are not touched.
In @.github/scripts/pr-review-chain.js:
- Around line 176-179: The main orchestrator in pr-review-chain.js is forcing
optionalWaitMs to 0 when it is omitted, which disables the default waiting
behavior in priorReviewersReady(). Update chainOpts so optionalWaitMs is only
set when explicitly provided by options, and otherwise let priorReviewersReady()
use its own default wait logic.
- Around line 236-241: The pull review fetch in `github.rest.pulls.listReviews`
only reads the first page, so later review signals can be missed on busy PRs.
Update the review-loading logic in the PR chain script to paginate through all
review pages, similar to how issue comments are handled, and aggregate every
review before downstream processing. Keep the change localized around the
existing review retrieval flow so the chain ordering and signal detection use
the full set of reviews.
In @.github/scripts/release-gate.js:
- Around line 64-75: The UTC-day success check in hasSuccessfulReleaseToday
currently uses the workflow run creation time, which can miss runs that complete
after midnight UTC. Update the timestamp comparison in hasSuccessfulReleaseToday
to use the run update time instead of created_at, keeping the rest of the
listWorkflowRuns and success/conclusion logic the same.
---
Nitpick comments:
In @.github/scripts/release-gate.js:
- Around line 140-154: The git diff command in release-gate.js is being built as
a shell string, which makes command parsing depend on escaping even though
PACKAGE_PATHS is config-controlled. Update the logic around execSync in the
lastTag/changed calculation to use execFileSync with explicit argv instead of
interpolating the diff command, and keep the same path filtering behavior by
passing lastTag, HEAD, and the package paths as separate arguments.
In @.github/workflows/auto-pr-comment.yml:
- Line 534: The recovery/sweep jobs still use actions/checkout without the
hardened credential setting, so update those checkout steps to match the
existing secure pattern by disabling credential persistence. Locate the checkout
usages in the recovery/sweep job sections and add the same
persist-credentials=false behavior used elsewhere so workspace files are checked
out without leaving git tokens in .git/config.
In @.github/workflows/ci.yml:
- Line 18: Set the actions/checkout@v4 step to disable credential persistence by
adding persist-credentials: false, since this CI workflow only runs lint and
tests and does not need the GITHUB_TOKEN stored in .git/config. Update the
checkout step in the ci workflow so the action no longer leaves reusable
credentials behind.
🪄 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: 52e51d6a-6d26-4c29-b91e-91a94ddd979b
📒 Files selected for processing (22)
.github/scripts/bot-pr-review-chain.js.github/scripts/ci-failure-claude-selftest.js.github/scripts/ci-failure-claude.js.github/scripts/gate-config.js.github/scripts/merge-gate-selftest.js.github/scripts/merge-gate.js.github/scripts/pipeline-status-selftest.js.github/scripts/pipeline-status.js.github/scripts/pr-review-chain.js.github/scripts/release-gate-selftest.js.github/scripts/release-gate.js.github/workflows/auto-pr-comment.yml.github/workflows/ci-failure-claude.yml.github/workflows/ci.yml.github/workflows/claude-merge-gate.yml.github/workflows/nightly-release-gate.yml.github/workflows/pipeline-status-sync.yml.github/workflows/pypi-release.yml.github/workflows/sync-secrets-from-praisonai.ymlsrc/praisonai_plugins/integrations/slack_integration.pysrc/praisonai_plugins/observability/gateway_forensics.pysrc/praisonai_plugins/policies/strict_policy.py
| const headRepo = pr.head.repo?.full_name; | ||
| if (headRepo && headRepo !== baseRepo && !pr.maintainer_can_modify) { | ||
| return { skipped: true, reason: 'fork without maintainer edits' }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep CI-fix automation internal-only.
This permits fork PRs when maintainer_can_modify is true, but the workflow is documented as internal-only and posts a privileged @claude CI-fix trigger. Require the PR head repo to be the base repo instead.
Proposed fix
const headRepo = pr.head.repo?.full_name;
- if (headRepo && headRepo !== baseRepo && !pr.maintainer_can_modify) {
- return { skipped: true, reason: 'fork without maintainer edits' };
+ if (headRepo !== baseRepo) {
+ return { skipped: true, reason: 'external fork' };
}📝 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.
| const headRepo = pr.head.repo?.full_name; | |
| if (headRepo && headRepo !== baseRepo && !pr.maintainer_can_modify) { | |
| return { skipped: true, reason: 'fork without maintainer edits' }; | |
| } | |
| const headRepo = pr.head.repo?.full_name; | |
| if (headRepo !== baseRepo) { | |
| return { skipped: true, reason: 'external fork' }; | |
| } |
🤖 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/ci-failure-claude.js around lines 203 - 206, The
fork-permission check in the CI-failure automation is too permissive because it
allows non-base-repo PRs when maintainer edits are enabled. Update the logic in
the PR head repo guard that uses pr.head.repo?.full_name, baseRepo, and
pr.maintainer_can_modify so the workflow only proceeds when the PR head repo
matches the base repo, and skip all other cases. Keep the existing skip response
shape and reason handling in the same ci-failure-claude.js flow.
| await github.rest.issues.addLabels({ | ||
| owner, | ||
| repo, | ||
| issue_number: prNumber, | ||
| labels: [CI_FIX_LABEL], | ||
| }); | ||
| await github.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: prNumber, | ||
| body, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid stranding claude-ci-fix-pending when comment creation fails.
If addLabels() succeeds but createComment() fails, later runs skip forever on the pending label while CI is still red.
Proposed fix
await github.rest.issues.addLabels({
owner,
repo,
issue_number: prNumber,
labels: [CI_FIX_LABEL],
});
- await github.rest.issues.createComment({
- owner,
- repo,
- issue_number: prNumber,
- body,
- });
+ try {
+ await github.rest.issues.createComment({
+ owner,
+ repo,
+ issue_number: prNumber,
+ body,
+ });
+ } catch (err) {
+ try {
+ await github.rest.issues.removeLabel({
+ owner,
+ repo,
+ issue_number: prNumber,
+ name: CI_FIX_LABEL,
+ });
+ } catch (removeErr) {
+ if (removeErr.status !== 404) core?.warning?.(`Failed to roll back ${CI_FIX_LABEL}: ${removeErr.message}`);
+ }
+ throw err;
+ }📝 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.
| await github.rest.issues.addLabels({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| labels: [CI_FIX_LABEL], | |
| }); | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| body, | |
| }); | |
| await github.rest.issues.addLabels({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| labels: [CI_FIX_LABEL], | |
| }); | |
| try { | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| body, | |
| }); | |
| } catch (err) { | |
| try { | |
| await github.rest.issues.removeLabel({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| name: CI_FIX_LABEL, | |
| }); | |
| } catch (removeErr) { | |
| if (removeErr.status !== 404) core?.warning?.(`Failed to roll back ${CI_FIX_LABEL}: ${removeErr.message}`); | |
| } | |
| 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/ci-failure-claude.js around lines 248 - 259, The
label/comment sequence in `addLabels` and `createComment` can strand
`claude-ci-fix-pending` if the comment fails after the label is added. Update
the flow around `github.rest.issues.addLabels` and
`github.rest.issues.createComment` so the pending label is only left in place
when the comment is successfully created, and if comment creation throws, remove
the label or otherwise roll back the added state before exiting. Keep the fix
localized to the comment-posting path in `ci-failure-claude.js` so future runs
don’t skip forever on a stale pending label.
| if (options.allowCopilotTimeout && copilotTriggered(comments)) { | ||
| return { ready: true, reason: 'copilot timeout fallback', copilotSkipped: true }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require an actual elapsed timeout before bypassing Copilot.
Line 167 treats any existing @copilot trigger as a timeout. Since Line 196 enables timeout fallback by default, advanceReviewChain() can post Claude FINAL immediately after posting Copilot, before Copilot responds.
🐛 Proposed fix
- if (options.allowCopilotTimeout && copilotTriggered(comments)) {
- return { ready: true, reason: 'copilot timeout fallback', copilotSkipped: true };
+ if (options.allowCopilotTimeout) {
+ const trigger = copilotTriggerComment(comments);
+ const triggerTime = new Date(trigger?.created_at || '').getTime();
+ const timeoutMs = options.copilotTimeoutMs ?? 30 * 60 * 1000;
+ if (Number.isFinite(triggerTime) && Date.now() - triggerTime >= timeoutMs) {
+ return { ready: true, reason: 'copilot timeout fallback', copilotSkipped: true };
+ }
} allowCopilotTimeout: options.allowCopilotTimeout !== false,
allowStaleRepost: options.allowStaleRepost === true,
skipCopilot: options.skipCopilot,
+ copilotTimeoutMs: options.copilotTimeoutMs,
});Also applies to: 193-199
🤖 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/pr-review-chain.js around lines 167 - 169, The timeout
fallback in advanceReviewChain() is incorrectly using copilotTriggered(comments)
alone to bypass Copilot, which can cause an immediate Claude FINAL before
Copilot has actually timed out. Update the logic around the allowCopilotTimeout
check so it only skips Copilot after a real elapsed timeout has occurred, using
the existing review-chain state and timeout tracking near advanceReviewChain()
and the timeout fallback setup in the related block. Ensure copilotSkipped is
set only when the timeout condition is truly met, not just because a Copilot
trigger comment exists.
| const copilot = options.skipCopilot | ||
| ? { triggered: false, reason: 'skipped' } | ||
| : await maybeTriggerCopilot(github, owner, repo, prNumber, core, chainOpts); | ||
| const claude = await maybeTriggerClaudeFinal( | ||
| github, owner, repo, prNumber, finalBody, core, | ||
| { ...options, ...chainOpts } | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor REVIEW_CHAIN_SKIP_COPILOT before posting Copilot.
Line 180 checks only options.skipCopilot, so REVIEW_CHAIN_SKIP_COPILOT=1 still posts an @copilot comment even though downstream gating treats Copilot as skipped.
🐛 Proposed fix
- const copilot = options.skipCopilot
+ const skipCopilot = isSkipCopilot(options);
+ const copilot = skipCopilot
? { triggered: false, reason: 'skipped' }
: await maybeTriggerCopilot(github, owner, repo, prNumber, core, chainOpts);
const claude = await maybeTriggerClaudeFinal(
github, owner, repo, prNumber, finalBody, core,
- { ...options, ...chainOpts }
+ { ...options, ...chainOpts, skipCopilot }
);📝 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.
| const copilot = options.skipCopilot | |
| ? { triggered: false, reason: 'skipped' } | |
| : await maybeTriggerCopilot(github, owner, repo, prNumber, core, chainOpts); | |
| const claude = await maybeTriggerClaudeFinal( | |
| github, owner, repo, prNumber, finalBody, core, | |
| { ...options, ...chainOpts } | |
| ); | |
| const skipCopilot = isSkipCopilot(options); | |
| const copilot = skipCopilot | |
| ? { triggered: false, reason: 'skipped' } | |
| : await maybeTriggerCopilot(github, owner, repo, prNumber, core, chainOpts); | |
| const claude = await maybeTriggerClaudeFinal( | |
| github, owner, repo, prNumber, finalBody, core, | |
| { ...options, ...chainOpts, skipCopilot } | |
| ); |
🤖 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/pr-review-chain.js around lines 180 - 186, The Copilot
trigger in pr-review-chain.js only checks options.skipCopilot, so the
REVIEW_CHAIN_SKIP_COPILOT setting is not honored before posting `@copilot`. Update
the Copilot gating in the main flow around maybeTriggerCopilot to also respect
the environment-based skip flag (the same source used by downstream gating), and
ensure the final copilot object reflects a skipped state whenever either skip
path is enabled.
| const path = require('path'); | ||
| const mergeGate = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/merge-gate.js')); | ||
| const chain = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pr-review-chain.js')); | ||
| const ps = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pipeline-status.js')); | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
| const prs = await github.paginate(github.rest.pulls.list, { | ||
| owner, repo, state: 'open', per_page: 100, | ||
| }); | ||
| console.log(`Triggered CodeRabbit for bot PR #${pr}`); | ||
| let posted = 0; | ||
| for (const pr of prs) { | ||
| if (pr.draft) continue; | ||
| const comments = await mergeGate.listAllComments(github, owner, repo, pr.number); | ||
| const headPushedAt = (await mergeGate.getHeadCommitDate(github, owner, repo, pr.number)) || pr.updated_at; | ||
| const hasFinal = mergeGate.hasFinalClaudeReviewTrigger(comments); | ||
| const isStale = mergeGate.isStaleFinalAfterPush(comments, headPushedAt); | ||
|
|
||
| // 2. Trigger Qodo review | ||
| if (hasFinal && !isStale) continue; | ||
| if (mergeGate.hasRecentClaudeTrigger(comments, 10)) { | ||
| core.info(`PR #${pr.number}: @claude within 10min — skip`); | ||
| continue; | ||
| } | ||
| if (await mergeGate.hasInProgressClaudeAssistant(github, owner, repo, pr.number)) { | ||
| core.info(`PR #${pr.number}: Claude in progress — skip`); | ||
| continue; | ||
| } | ||
| if (hasFinal && isStale) { | ||
| const gate = mergeGate.shouldSkipStaleFinalRecovery(comments, headPushedAt); | ||
| if (gate.skip) { | ||
| core.info(`PR #${pr.number}: skip stale — ${gate.reason}`); | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| const opts = hasFinal && isStale | ||
| ? { allowStaleRepost: true, skipCopilot: true, optionalWaitMs: 0 } | ||
| : { allowCopilotTimeout: true, optionalWaitMs: 0 }; | ||
| const result = await chain.maybeTriggerClaudeFinal( | ||
| github, owner, repo, pr.number, | ||
| mergeGate.FINAL_CLAUDE_REVIEW_BODY, core, opts | ||
| ); | ||
| if (result.posted) { | ||
| posted++; | ||
| core.info(`Posted Claude FINAL on PR #${pr.number}`); | ||
| } | ||
| await ps.syncPipelineLabels(github, owner, repo, pr.number, core); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Create pipeline labels before the scheduled/manual sweep syncs them.
This sweep calls ps.syncPipelineLabels(...) without first calling ps.ensurePipelineLabels(...). On a fresh repo or after labels are deleted, syncPipelineLabels can fail when adding missing pipeline/* labels.
Proposed fix
const owner = context.repo.owner;
const repo = context.repo.repo;
+ await ps.ensurePipelineLabels(github, owner, repo, core);
const prs = await github.paginate(github.rest.pulls.list, {
owner, repo, state: 'open', per_page: 100,
});📝 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.
| const path = require('path'); | |
| const mergeGate = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/merge-gate.js')); | |
| const chain = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pr-review-chain.js')); | |
| const ps = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pipeline-status.js')); | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const prs = await github.paginate(github.rest.pulls.list, { | |
| owner, repo, state: 'open', per_page: 100, | |
| }); | |
| console.log(`Triggered CodeRabbit for bot PR #${pr}`); | |
| let posted = 0; | |
| for (const pr of prs) { | |
| if (pr.draft) continue; | |
| const comments = await mergeGate.listAllComments(github, owner, repo, pr.number); | |
| const headPushedAt = (await mergeGate.getHeadCommitDate(github, owner, repo, pr.number)) || pr.updated_at; | |
| const hasFinal = mergeGate.hasFinalClaudeReviewTrigger(comments); | |
| const isStale = mergeGate.isStaleFinalAfterPush(comments, headPushedAt); | |
| // 2. Trigger Qodo review | |
| if (hasFinal && !isStale) continue; | |
| if (mergeGate.hasRecentClaudeTrigger(comments, 10)) { | |
| core.info(`PR #${pr.number}: @claude within 10min — skip`); | |
| continue; | |
| } | |
| if (await mergeGate.hasInProgressClaudeAssistant(github, owner, repo, pr.number)) { | |
| core.info(`PR #${pr.number}: Claude in progress — skip`); | |
| continue; | |
| } | |
| if (hasFinal && isStale) { | |
| const gate = mergeGate.shouldSkipStaleFinalRecovery(comments, headPushedAt); | |
| if (gate.skip) { | |
| core.info(`PR #${pr.number}: skip stale — ${gate.reason}`); | |
| continue; | |
| } | |
| } | |
| const opts = hasFinal && isStale | |
| ? { allowStaleRepost: true, skipCopilot: true, optionalWaitMs: 0 } | |
| : { allowCopilotTimeout: true, optionalWaitMs: 0 }; | |
| const result = await chain.maybeTriggerClaudeFinal( | |
| github, owner, repo, pr.number, | |
| mergeGate.FINAL_CLAUDE_REVIEW_BODY, core, opts | |
| ); | |
| if (result.posted) { | |
| posted++; | |
| core.info(`Posted Claude FINAL on PR #${pr.number}`); | |
| } | |
| await ps.syncPipelineLabels(github, owner, repo, pr.number, core); | |
| const path = require('path'); | |
| const mergeGate = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/merge-gate.js')); | |
| const chain = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pr-review-chain.js')); | |
| const ps = require(path.join(process.env.GITHUB_WORKSPACE, '.github/scripts/pipeline-status.js')); | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| await ps.ensurePipelineLabels(github, owner, repo, core); | |
| const prs = await github.paginate(github.rest.pulls.list, { | |
| owner, repo, state: 'open', per_page: 100, | |
| }); | |
| let posted = 0; | |
| for (const pr of prs) { | |
| if (pr.draft) continue; | |
| const comments = await mergeGate.listAllComments(github, owner, repo, pr.number); | |
| const headPushedAt = (await mergeGate.getHeadCommitDate(github, owner, repo, pr.number)) || pr.updated_at; | |
| const hasFinal = mergeGate.hasFinalClaudeReviewTrigger(comments); | |
| const isStale = mergeGate.isStaleFinalAfterPush(comments, headPushedAt); | |
| if (hasFinal && !isStale) continue; | |
| if (mergeGate.hasRecentClaudeTrigger(comments, 10)) { | |
| core.info(`PR #${pr.number}: `@claude` within 10min — skip`); | |
| continue; | |
| } | |
| if (await mergeGate.hasInProgressClaudeAssistant(github, owner, repo, pr.number)) { | |
| core.info(`PR #${pr.number}: Claude in progress — skip`); | |
| continue; | |
| } | |
| if (hasFinal && isStale) { | |
| const gate = mergeGate.shouldSkipStaleFinalRecovery(comments, headPushedAt); | |
| if (gate.skip) { | |
| core.info(`PR #${pr.number}: skip stale — ${gate.reason}`); | |
| continue; | |
| } | |
| } | |
| const opts = hasFinal && isStale | |
| ? { allowStaleRepost: true, skipCopilot: true, optionalWaitMs: 0 } | |
| : { allowCopilotTimeout: true, optionalWaitMs: 0 }; | |
| const result = await chain.maybeTriggerClaudeFinal( | |
| github, owner, repo, pr.number, | |
| mergeGate.FINAL_CLAUDE_REVIEW_BODY, core, opts | |
| ); | |
| if (result.posted) { | |
| posted++; | |
| core.info(`Posted Claude FINAL on PR #${pr.number}`); | |
| } | |
| await ps.syncPipelineLabels(github, owner, repo, pr.number, core); |
🤖 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 621 - 666, The
scheduled/manual sweep in the workflow script runs ps.syncPipelineLabels()
without guaranteeing the pipeline labels exist first, which can fail on fresh
repos or after label deletion. Update the sweep loop to call
ps.ensurePipelineLabels() before ps.syncPipelineLabels(), using the existing ps
helper in the same block where PRs are processed so missing pipeline/* labels
are created before syncing.
| - name: Generate GitHub App token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@v1 | ||
| with: | ||
| app-id: ${{ secrets.CLAUDE_APP_ID }} | ||
| private-key: ${{ secrets.CLAUDE_APP_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' .github/workflows/pipeline-status-sync.ymlRepository: MervinPraison/PraisonAI-Plugins
Length of output: 1348
🌐 Web query:
actions/create-github-app-token@v1 repositories input owner installation token scope default documentation
💡 Result:
In the actions/create-github-app-token action, the owner and repositories inputs work together to define the scope of the generated installation access token [1][2]. Their behavior is as follows: - owner: An optional input specifying the owner of the GitHub App installation. If left empty, it defaults to the current repository owner [1][3]. - repositories: An optional input accepting a comma or newline-separated list of repositories. If left unset, it defaults to the current repository [1][3]. The resulting token scope is determined by the combination of these inputs [1][2]: - If both owner and repositories are empty: The token is scoped only to the current repository [1][2]. - If owner is explicitly set and repositories is empty: The token is scoped to all repositories within that owner's installation [1][2]. - If repositories is set: The token is scoped specifically to those listed repositories [1][4]. Note that if the owner is unset, the repositories input defaults to the current repository, and if the owner is set, the repositories input defaults to all repositories within that owner's installation [1][5]. The token will always inherit all permissions configured for the GitHub App installation [4][2].
Citations:
- 1: https://github.com/actions/create-github-app-token/blob/main/README.md
- 2: https://github.com/actions/create-github-app-token/blob/34c66235f6ccc38eb01b7aed860890bdf40db33c/README.md
- 3: https://github.com/actions/create-github-app-token/blob/main/action.yml
- 4: https://github.com/actions/create-github-app-token/tree/v1.9.2
- 5: Create a token for all repositories in the current owner's installation actions/create-github-app-token#377
Scope the GitHub App token to this repository. Passing only owner leaves the installation token valid for every repository in that owner’s installation. Add repositories: ${{ github.event.repository.name }} before using it here.
🧰 Tools
🪛 zizmor (1.26.1)
[error] 30-30: dangerous use of GitHub App tokens (github-app): token granted access to all repositories for this owner's app installation
(github-app)
[error] 26-26: dangerous use of GitHub App tokens (github-app): app token inherits blanket installation permissions
(github-app)
🤖 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 24 - 30, The GitHub
App token is currently scoped only to the owner, which makes it usable across
all repositories in that installation. Update the Generate GitHub App token step
in pipeline-status-sync to include the repositories input alongside owner, using
github.event.repository.name, so the token is limited to this repository before
it is consumed by later workflow steps.
Source: Linters/SAST tools
| trigger_sha: | ||
| description: 'SHA that triggered this release' | ||
| required: false | ||
| default: '' | ||
| type: string |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files .github/workflows/*.yml .github/workflows/*.yaml | sed 's#^`#-` #'
printf '\n== pypi-release workflow outline ==\n'
wc -l .github/workflows/pypi-release.yml
cat -n .github/workflows/pypi-release.yml | sed -n '1,260p'
printf '\n== nightly-release-gate workflow outline ==\n'
if [ -f .github/workflows/nightly-release-gate.yml ]; then
wc -l .github/workflows/nightly-release-gate.yml
cat -n .github/workflows/nightly-release-gate.yml | sed -n '1,260p'
fiRepository: MervinPraison/PraisonAI-Plugins
Length of output: 11382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow references to trigger_sha ==\n'
rg -n "trigger_sha|rebase|origin/main|checkout@v4|pypi release|nightly" .github/workflows -S
printf '\n== relevant sections around lines in pypi-release ==\n'
sed -n '1,220p' .github/workflows/pypi-release.ymlRepository: MervinPraison/PraisonAI-Plugins
Length of output: 7623
Honor trigger_sha before publishing. The gate workflow passes the preflight SHA, but this job ignores it, checks out main, and later rebases from origin/main. If main moves between preflight and release, PyPI can get an artifact that was never gated. Fail fast when inputs.trigger_sha doesn’t match origin/main, or check out that SHA directly.
🤖 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/pypi-release.yml around lines 27 - 31, The release
workflow currently ignores the preflight trigger SHA and instead checks out and
rebases from main, which can publish an ungated artifact. Update the release job
in the workflow to honor inputs.trigger_sha by either checking out that exact
SHA directly or validating it against origin/main before proceeding, and make
the gate explicit in the release flow so the publish step cannot continue on a
mismatched commit.
| content = Path("pyproject.toml").read_text() | ||
| match = re.search(r'^version\s*=\s*"([^"]+)"', content, re.MULTILINE) | ||
| if not match: | ||
| sys.exit("Could not read version from pyproject.toml") | ||
| current = match.group(1) | ||
| maj, min_, patch = map(int, current.split(".")) | ||
| if override: | ||
| new_version = override | ||
| elif bump == "major": | ||
| new_version = f"{maj + 1}.0.0" | ||
| elif bump == "minor": | ||
| new_version = f"{maj}.{min_ + 1}.0" | ||
| else: | ||
| new_version = f"{maj}.{min_}.{patch + 1}" | ||
| with open(os.environ["GITHUB_OUTPUT"], "a") as fh: | ||
| fh.write(f"current_version={current}\nnew_version={new_version}\n") | ||
| PY | ||
|
|
||
| - name: Bump pyproject.toml | ||
| if: inputs.dry_run != true | ||
| env: | ||
| CURRENT: ${{ steps.versions.outputs.current_version }} | ||
| NEW_VERSION: ${{ steps.versions.outputs.new_version }} | ||
| run: | | ||
| python -c " | ||
| from pathlib import Path | ||
| import os | ||
| p = Path('pyproject.toml') | ||
| p.write_text(p.read_text().replace(f'version = \"{os.environ[\"CURRENT\"]}\"', f'version = \"{os.environ[\"NEW_VERSION\"]}\"', 1)) | ||
| " |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the same matcher for reading and writing the version.
The read regex accepts valid whitespace variants, but the write step only replaces version = "...". If formatting changes, the bump silently no-ops.
Proposed fix
- p.write_text(p.read_text().replace(f'version = \"{os.environ[\"CURRENT\"]}\"', f'version = \"{os.environ[\"NEW_VERSION\"]}\"', 1))
+ import re, sys
+ content = p.read_text()
+ updated, count = re.subn(
+ r'^version\s*=\s*"[^"]+"',
+ f'version = "{os.environ["NEW_VERSION"]}"',
+ content,
+ count=1,
+ flags=re.MULTILINE,
+ )
+ if count != 1:
+ sys.exit("Could not update version in pyproject.toml")
+ p.write_text(updated)📝 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.
| content = Path("pyproject.toml").read_text() | |
| match = re.search(r'^version\s*=\s*"([^"]+)"', content, re.MULTILINE) | |
| if not match: | |
| sys.exit("Could not read version from pyproject.toml") | |
| current = match.group(1) | |
| maj, min_, patch = map(int, current.split(".")) | |
| if override: | |
| new_version = override | |
| elif bump == "major": | |
| new_version = f"{maj + 1}.0.0" | |
| elif bump == "minor": | |
| new_version = f"{maj}.{min_ + 1}.0" | |
| else: | |
| new_version = f"{maj}.{min_}.{patch + 1}" | |
| with open(os.environ["GITHUB_OUTPUT"], "a") as fh: | |
| fh.write(f"current_version={current}\nnew_version={new_version}\n") | |
| PY | |
| - name: Bump pyproject.toml | |
| if: inputs.dry_run != true | |
| env: | |
| CURRENT: ${{ steps.versions.outputs.current_version }} | |
| NEW_VERSION: ${{ steps.versions.outputs.new_version }} | |
| run: | | |
| python -c " | |
| from pathlib import Path | |
| import os | |
| p = Path('pyproject.toml') | |
| p.write_text(p.read_text().replace(f'version = \"{os.environ[\"CURRENT\"]}\"', f'version = \"{os.environ[\"NEW_VERSION\"]}\"', 1)) | |
| " | |
| - name: Bump pyproject.toml | |
| if: inputs.dry_run != true | |
| env: | |
| CURRENT: ${{ steps.versions.outputs.current_version }} | |
| NEW_VERSION: ${{ steps.versions.outputs.new_version }} | |
| run: | | |
| python -c " | |
| from pathlib import Path | |
| import os | |
| import re, sys | |
| p = Path('pyproject.toml') | |
| content = p.read_text() | |
| updated, count = re.subn( | |
| r'^version\s*=\s*\"[^\"]+\"', | |
| f'version = \"{os.environ[\"NEW_VERSION\"]}\"', | |
| content, | |
| count=1, | |
| flags=re.MULTILINE, | |
| ) | |
| if count != 1: | |
| sys.exit(\"Could not update version in pyproject.toml\") | |
| p.write_text(updated) | |
| " |
🤖 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/pypi-release.yml around lines 83 - 112, The version bump
logic in the versions step and the pyproject.toml update step are using
different matching rules, so the write path can silently fail when formatting
varies. Update the Bump pyproject.toml step to use the same whitespace-tolerant
matcher as the version-reading logic, and apply the replacement based on that
match instead of a fixed string. Keep the behavior aligned with the existing
versions output generation so current_version and new_version remain consistent.
| - name: Publish to PyPI | ||
| if: inputs.dry_run != true | ||
| env: | ||
| UV_PUBLISH_TOKEN: ${{ secrets.PYPI_TOKEN || secrets.PYPI_API_TOKEN }} | ||
| run: | | ||
| uv lock | ||
| uv build | ||
| uv publish | ||
|
|
||
| - name: Commit, tag, and release | ||
| if: inputs.dry_run != true | ||
| env: | ||
| VERSION: ${{ steps.versions.outputs.new_version }} | ||
| run: | | ||
| TAG="v${VERSION}" | ||
| git add pyproject.toml uv.lock | ||
| git commit -m "Release ${TAG}" || exit 0 | ||
| git tag -f "${TAG}" | ||
| git pull --rebase origin main | ||
| git push origin main | ||
| git push --tags -f | ||
| gh release create "${TAG}" --title "praisonai-plugins ${TAG}" --notes "Release ${TAG}" --latest |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Push immutable VCS state before publishing to PyPI.
uv publish runs before the release commit/tag is pushed, so a later git or GitHub release failure leaves an immutable PyPI artifact without matching VCS state. Also avoid git tag -f and git push --tags -f, which can move existing release tags.
Safer shape
- - name: Publish to PyPI
+ - name: Build distribution
if: inputs.dry_run != true
env:
UV_PUBLISH_TOKEN: ${{ secrets.PYPI_TOKEN || secrets.PYPI_API_TOKEN }}
run: |
uv lock
uv build
- uv publish
...
- git commit -m "Release ${TAG}" || exit 0
- git tag -f "${TAG}"
- git pull --rebase origin main
- git push origin main
- git push --tags -f
+ git diff --quiet -- pyproject.toml uv.lock && {
+ echo "No release metadata changes to commit"
+ exit 1
+ }
+ git commit -m "Release ${TAG}"
+ git tag "${TAG}"
+ git push origin HEAD:main
+ git push origin "${TAG}"
gh release create "${TAG}" --title "praisonai-plugins ${TAG}" --notes "Release ${TAG}" --latest
+
+ - name: Publish to PyPI
+ if: inputs.dry_run != true
+ env:
+ UV_PUBLISH_TOKEN: ${{ secrets.PYPI_TOKEN || secrets.PYPI_API_TOKEN }}
+ run: uv publish🧰 Tools
🪛 zizmor (1.26.1)
[info] 121-121: prefer trusted publishing for authentication (use-trusted-publishing): this command
(use-trusted-publishing)
🤖 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/pypi-release.yml around lines 114 - 135, The release flow
in the Publish to PyPI and Commit, tag, and release steps should push the
versioned VCS state before running uv publish. Reorder the workflow so the
commit, tag, and pushes happen first, then publish to PyPI only after the git
tag and GitHub release are safely created, using the VERSION/TAG logic already
in place. Also remove force-moving tag operations in the release step by
avoiding git tag -f and git push --tags -f, and use non-forced tag creation/push
so release tags remain immutable.
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| gh workflow run sync-secrets-to-aiui.yml \ | ||
| --repo MervinPraison/PraisonAI \ | ||
| --ref main \ | ||
| -f target_repo=MervinPraison/PraisonAI-Plugins | ||
| echo "Triggered secret sync → MervinPraison/PraisonAI-Plugins" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
GITHUB_TOKEN cannot trigger workflows in a different repository.
The default secrets.GITHUB_TOKEN is scoped to MervinPraison/PraisonAI-Plugins only. The gh workflow run --repo MervinPraison/PraisonAI command requires a token with access to that repository. This workflow will fail with a permissions error.
Use a PAT or GitHub App token stored as a repository secret:
🔧 Proposed fix
- name: Trigger PraisonAI secret sync
env:
- GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ GH_TOKEN: ${{ secrets.PRAISONAI_PAT }}
run: |
gh workflow run sync-secrets-to-aiui.yml \
--repo MervinPraison/PraisonAI \
--ref main \
-f target_repo=MervinPraison/PraisonAI-Plugins
echo "Triggered secret sync → MervinPraison/PraisonAI-Plugins"Ensure the PAT has repo and workflow scopes for MervinPraison/PraisonAI.
📝 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.
| env: | |
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| run: | | |
| gh workflow run sync-secrets-to-aiui.yml \ | |
| --repo MervinPraison/PraisonAI \ | |
| --ref main \ | |
| -f target_repo=MervinPraison/PraisonAI-Plugins | |
| echo "Triggered secret sync → MervinPraison/PraisonAI-Plugins" | |
| env: | |
| GH_TOKEN: ${{ secrets.PRAISONAI_PAT }} | |
| run: | | |
| gh workflow run sync-secrets-to-aiui.yml \ | |
| --repo MervinPraison/PraisonAI \ | |
| --ref main \ | |
| -f target_repo=MervinPraison/PraisonAI-Plugins | |
| echo "Triggered secret sync → MervinPraison/PraisonAI-Plugins" |
🤖 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/sync-secrets-from-praisonai.yml around lines 14 - 21, The
sync-secrets workflow is using secrets.GITHUB_TOKEN to call gh workflow run
against MervinPraison/PraisonAI, but that token cannot trigger workflows in
another repository. Update the job in sync-secrets-from-praisonai.yml to use a
repository secret containing a PAT or GitHub App token with access to
MervinPraison/PraisonAI, and make sure the token has the required repo and
workflow scopes before invoking gh workflow run.
1 participant
Summary
gate-config.jsscoped to plugin lifecycle (src/praisonai_plugins/,praisonai.plugins,praisonai.sandbox)CIworkflow (ruff + pytest 3.10–3.12) andpypi-release.ymlforpraisonai-pluginsauto-pr-comment.ymlFINAL @claude scope for Plugins vs Tools vs SDK layeringsync-secrets-from-praisonai.ymlhub workflow triggerTest plan
.github/scripts/*-selftest.jspass locallypipeline/merge-readylabel flow on a test PRSummary by CodeRabbit
New Features
Bug Fixes
Chores