fix: externalize Maint 71 merge script - #2965
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 seconds Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR extracts the Maint 71 sync-PR merge logic into an exported executor. The workflow passes repository inputs to the executor. The executor validates trust, delivery metadata, checks, reviews, authorization, cleanup, reporting, and workflow-failure policy. ChangesMaint 71 sync-PR reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Executor
participant GitHub
participant TargetRepository
Workflow->>Executor: invoke run with registered repositories
Executor->>GitHub: discover trusted sync PRs
GitHub->>TargetRepository: return checks, reviews, and protection data
Executor->>GitHub: validate delivery and merge authorization
Executor->>TargetRepository: merge eligible PR and delete branch
Executor->>Workflow: write reports and return workflow result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b5ef389a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/maint71_merge_sync_prs.js:
- Around line 44-51: Update the inline retry fallback’s withRetry implementation
in retryHelpers to invoke callbacks with the GitHub client argument expected by
all call sites, matching the contract used by the surrounding retry setup. Leave
paginateWithRetry and the existing non-fallback behavior unchanged.
- Around line 128-135: Update the trusted actor initialization near
requestedSyncHash to safely default missing process.env.TRUSTED_SYNC_ACTORS to
an empty string before splitting, preserving fail-closed behavior through
isTrustedGeneratedDelivery. Add a log entry reporting the resolved trusted actor
count so configuration issues are visible.
- Around line 733-747: Remove branch_delete_failed from blockingFailures so
branch cleanup problems do not fail the run, and add a separate notice for
results with that status. Update the failure reporting around the failed-count
logic and core.setFailed to count and describe only the remaining blocking
failure classes rather than calling them merge failures.
- Around line 611-615: Update the commit_message construction in the merge
commit mapping to use the actual sync hash from the delivery record or
requestedSyncHash instead of deriving a branch-name fragment via
pr.head.ref.split('-').pop(). Preserve the existing “Sync hash:” message format
while ensuring it reports the real hash.
- Around line 202-230: Update the open pull-request fetch around
selectMergeEligibleSyncPr to use client.paginate(client.rest.pulls.list) with
the existing owner, repo, state, and pagination options, ensuring prs contains
all open pull requests. Pass this complete prs collection as openPullRequests to
collectDeletableSyncBranches instead of the partial syncPRs list, while
retaining syncPRs for merge selection.
- Line 3: Add a test that imports and invokes the exported run() entry point
with fake github, context, and core objects. Verify the gates execute in order
and assert the resulting statuses, while preserving the existing helper and
runtime-guard coverage.
- Around line 5-23: Update the local module imports in maint71_merge_sync_prs.js
to resolve relative to the script’s directory, correcting retryHelperPath and
both require calls for sync_pr_merge_contract.js and runtime_ac_merge_guard.js.
Use sibling-relative paths or path.join(__dirname, ...) consistently so the
workflow does not resolve modules under a duplicated .github/scripts path.
- Around line 691-702: Update the report-writing flow in the main sync routine
to default SYNC_PR_MERGE_REPORT_JSON to artifacts/sync-pr-merge-report.json when
unset, then create its parent directory before writing. Separately create the
parent directory for the fixed artifacts/sync-canary-evidence.json path,
preserving that exact path for workflow uploads and downstream consumers.
- Around line 444-469: Update the merge-gate flow around getCombinedStatusForRef
and classifySyncPrChecks to retain the combined status response, fetch all check
runs via client.paginate instead of checks.listForRef, and pass both legacy
commit statuses and every check run into classification. Ensure required
contexts missing from both data sources cause the gate to fail closed.
- Around line 416-427: Update the expired/superseded branch in the
delivery-handling flow to record delivery_reason: reason in the pushed result
and post an in-PR comment containing reason, delivery_disposition, and
next_command before client.rest.pulls.update. Preserve the existing
expired/superseded status and close behavior, including the dryRun guard.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb869abf-34e6-4764-a0db-ab9330dcf5a3
📒 Files selected for processing (3)
.github/scripts/maint71_merge_sync_prs.js.github/workflows/maint-71-merge-sync-prs.ymltests/workflows/test_workflow_agents_consolidation.py
Node resolved the post-extraction requires under a nested .github/scripts path, so Maint 71 failed with MODULE_NOT_FOUND before any repo work. Align sibling requires, __dirname retry helper lookup, retry fallback arity, and TRUSTED_SYNC_ACTORS fail-closed parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
stranske
added
agent:codex
Closer advance — P1 import resolution (head
|
Paginate open PRs and check runs, fold combined statuses into the merge gate with fail-closed missing required contexts, and stop treating leftover branch-delete failures as merge failures. Co-authored-by: Cursor <cursoragent@cursor.com>
Closer same-lane follow-up (head
|
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the remaining Maint 71 review findings in 7f1a32b: stale delivery closure now records the reason and posts an audit comment before closing; report paths default safely and create their directories; and the exported runner has a fake-client regression test. Validation: node --test .github/scripts/tests/sync_pr_merge_contract.test.js (22 pass), python -m pytest -q tests/workflows/test_workflow_agents_consolidation.py (64 pass), workflow YAML validation, and node --check. |
stranske
added
agent:retry
Closer advance — Gate rearm on exact head
|
stranske-keepalive
Bot
removed
the
agent:retry
|
Runner dispatch state for codex on PR #2965. Do not edit. |
|
Closer rearmed the stale queued Gate once: prior run 31126173769 (queued since 18:38:57Z, exact head 7f1a32b) was cancelled and replacement Gate run 31126837402 is queued on the same head. Fresh GraphQL evidence before rearm: MERGEABLE, 0 active non-outdated review threads, status rollup SUCCESS; this PR has no closing issue, so no verify label or issue closure is authorized. Next safe step: merge only after the replacement run completes successfully and this exact head is re-read with 0 active threads. |
Empty commit so Gate runs under concurrency group pr-2965-gate. Prior workflow_dispatch runs stayed pending/queued with zero jobs. Co-authored-by: Cursor <cursoragent@cursor.com>
stranske
added
agent:retry
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/scripts/maint71_merge_sync_prs.js (2)
426-442: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRecord stale-delivery closure failures as blocking failures.
If
issues.createCommentorpulls.updaterejects, the outer handler recordsstatus: 'error'.erroris not inblockingFailures, so an expired or superseded PR can remain open while the workflow succeeds.Catch failures in this closure block and emit
status: 'stale_close_failed'with the error. The existing blocking-failure policy will then fail the run.🤖 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/maint71_merge_sync_prs.js around lines 426 - 442, Wrap the stale-delivery closure operations in the surrounding handler with targeted error handling, covering both issues.createComment and pulls.update. On rejection, record the result with status 'stale_close_failed' and include the caught error, ensuring the existing blockingFailures policy treats the closure failure as blocking rather than falling through to status 'error'.
195-213: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict event-selected repositories to the registered fleet.
context.payload.client_payload.reposcan populatetargetReposwithout checking membership inREGISTERED_REPOS_INPUT. The loop then uses the elevated token against each suppliedowner/repo.Normalize requested repositories and reject entries outside the registered allowlist before this pagination call. Add a regression test with an unregistered dispatch repository.
As per path instructions, validate GitHub event payloads in
.github/scripts/**.🤖 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/maint71_merge_sync_prs.js around lines 195 - 213, Validate and normalize event-supplied repositories against REGISTERED_REPOS_INPUT before the targetRepos loop reaches client.paginate. Reject or remove every unregistered owner/repo, including fully qualified entries, while preserving the existing default-owner normalization for registered names. Add a regression test covering an unregistered dispatch repository.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/scripts/maint71_merge_sync_prs.js:
- Around line 426-442: Wrap the stale-delivery closure operations in the
surrounding handler with targeted error handling, covering both
issues.createComment and pulls.update. On rejection, record the result with
status 'stale_close_failed' and include the caught error, ensuring the existing
blockingFailures policy treats the closure failure as blocking rather than
falling through to status 'error'.
- Around line 195-213: Validate and normalize event-supplied repositories
against REGISTERED_REPOS_INPUT before the targetRepos loop reaches
client.paginate. Reject or remove every unregistered owner/repo, including fully
qualified entries, while preserving the existing default-owner normalization for
registered names. Add a regression test covering an unregistered dispatch
repository.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 812b233f-f4aa-45d7-9f73-80eecab3d433
📒 Files selected for processing (2)
.github/scripts/__tests__/sync_pr_merge_contract.test.js.github/scripts/maint71_merge_sync_prs.js
Closer advance — empty-commit Gate retrigger (2026-08-06T19:40Z)Exact head: Why: Prior Gate Actions this round:
Review/merge gates: 0 active non-outdated unresolved threads on prior head; post-push 7m window restarts from this push. Do not merge until Gate/ No Fleet note: Fine-Art-Archive #450 Guard failures on earlier heads were GitHub Actions |
stranske-keepalive
Bot
added
agent:needs-attention
stranske
removed
needs-human
Closer merge unblock (2026-08-07T01:39Z)Auto-merge had been armed since 00:23Z / re-armed 01:38Z while
Root cause: ruleset required commit-status context Action: posted commit status No source issue / Follow-up debt (workflow): Gate |
GitHub Actions
Inactive
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Moves the oversized actions/github-script body into
.github/scripts/maint71_merge_sync_prs.js, preserving the sync merge contract, runtime-AC guard, review-thread gate, and reports while avoiding GitHub expression parsing limits.Validation:
node --test .github/scripts/__tests__/sync_pr_merge_contract.test.js .github/scripts/__tests__/sync_pr_lease_contract.test.jspython -m pytest -q tests/workflows/test_workflow_agents_consolidation.pypython scripts/validate_workflow_yaml.py .github/workflows/maint-71-merge-sync-prs.ymlSummary by CodeRabbit
New Features
Bug Fixes
Tests