fix(sync): persist Maint 71 delivery handoffs - #2899
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: 48 minutes 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 (6)
📝 WalkthroughWalkthroughThe change adds delivery handoff validation, deduplication, retention, campaign persistence, marker serialization, and workflow transfer between Maint 71 and Maint 82. Tests cover merge behavior, state persistence, event payloads, and campaign consumption. ChangesDelivery handoff lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Maint71 as Maint 71 merge workflow
participant GitHub as repository event
participant Maint82 as Maint 82 campaign workflow
participant Campaign as runCampaign
Maint71->>GitHub: Send handoff_records
GitHub->>Maint82: Provide delivery_handoff_records
Maint82->>Campaign: Pass deliveryHandoffRecords
Campaign-->>Maint82: Return campaign state and handoff count
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Runner dispatch state for autofix on PR #2899. Do not edit. |
|
Runner dispatch state for codex on PR #2899. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/sync_dependency_campaign.js:
- Around line 352-371: Update normalizeDeliveryHandoff in
.github/scripts/sync_dependency_campaign.js at lines 352-371 to require
non-empty disposition, blocker_owner, and next_command, add normalized check and
review state fields, and reject incomplete records. Update the valid handoff
fixture in .github/scripts/__tests__/sync_dependency_campaign.test.js at lines
26-49 with check and review state values, and replace the empty next_command
expectation with coverage asserting incomplete records are rejected.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 362c5459-9cb9-47ea-b69c-f7bb022d5121
📒 Files selected for processing (5)
.github/scripts/__tests__/sync_dependency_campaign.test.js.github/scripts/sync_dependency_campaign.js.github/workflows/maint-71-merge-sync-prs.yml.github/workflows/maint-82-sync-dependency-campaign.ymltests/workflows/test_maint82_sync_campaign_contract.py
There was a problem hiding this comment.
Pull request overview
This PR extends the sync/dependency campaign pipeline so Maint 71 can emit machine-readable “delivery handoff” records via repository_dispatch, and Maint 82 can ingest and persist those records in the campaign’s durable marker state. This supports the broader goal (Issue #2881) of making handoffs durable and avoiding repeated “rediscovery” work across runs.
Changes:
- Dispatch Maint 71 merge/reconcile results (handoff records) to the Maint 82 campaign workflow via
repository_dispatch. - Teach Maint 82’s workflow runner to consume
delivery_handoff_recordsfrom the dispatch payload and pass them into the campaign script. - Add a durable-state merge path + tests for persisting Maint 71 handoffs in
sync_dependency_campaign.js.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/workflows/test_maint82_sync_campaign_contract.py |
Adds contract assertions proving Maint 82 consumes delivery handoffs and Maint 71 dispatches them. |
.github/workflows/maint-82-sync-dependency-campaign.yml |
Reads delivery_handoff_records from repository_dispatch payload and passes them into runCampaign, plus adds a summary line. |
.github/workflows/maint-71-merge-sync-prs.yml |
Dispatches handoff records to the campaign workflow after producing the merge report. |
.github/scripts/sync_dependency_campaign.js |
Adds delivery-handoff schema/constants, normalization + merge logic, and persists handoffs into durable marker state. |
.github/scripts/__tests__/sync_dependency_campaign.test.js |
Adds unit coverage for handoff merge behavior and durable marker persistence. |
| stats: { | ||
| ...buildStats(items, discoveredItems, options), | ||
| delivery_handoffs_observed: deliveryHandoffs.length, | ||
| }, |
| if (!dryRun && report.handoff_records.length > 0) { | ||
| await withRetry((client) => client.rest.repos.createDispatchEvent({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| event_type: 'sync-dependabot-campaign', | ||
| client_payload: { | ||
| repos: targetRepos.join(','), | ||
| delivery_handoff_records: report.handoff_records, | ||
| }, | ||
| })); | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92106ebc6e
ℹ️ 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".
| repo: context.repo.repo, | ||
| event_type: 'sync-dependabot-campaign', | ||
| client_payload: { | ||
| repos: targetRepos.join(','), |
There was a problem hiding this comment.
Keep handoff-triggered campaign scans fleet-wide
When Maint 71 is run with its supported repos filter, this forwards only those target repositories to Maint 82. mergeCampaignState() treats every previously active item absent from the resulting discovery set as stale, without distinguishing repositories that were not scanned, so a targeted Maint 71 run for repo B can stale repo A's live queue entries and remove campaign:needs-local-codex. Pass the full registered set for this refresh, or make partial-scan scope explicit to the state merger.
Useful? React with 👍 / 👎.
| event_type: 'sync-dependabot-campaign', | ||
| client_payload: { | ||
| repos: targetRepos.join(','), | ||
| delivery_handoff_records: report.handoff_records, |
There was a problem hiding this comment.
Emit terminal handoffs after successful merges
For every successful auto-merge, report.handoff_records is built from the pre-merge deliveryContext: buildDeliveryHandoff() prefers delivery_disposition over the result status, so the newly persisted record still says disposition: current and next_command: merge-current-delivery even though the PR has already merged. Maint 82 therefore durably publishes a stale command to handoff consumers; update or omit the record after terminal merge/close outcomes before dispatching it.
Useful? React with 👍 / 👎.
🤖 Bot Comment Handler
The agent has been assigned to this PR to address the bot review comments. Instructions for agent
The bot comment handler workflow has prepared context in the artifacts. |
Require full restart fields, count observed handoffs per run, emit terminal merge dispositions, and keep campaign refresh fleet-wide. Co-authored-by: Cursor <cursoragent@cursor.com>
Closer review recovery (handoff contract)Addressed the five unresolved review threads on head
Validation: Node suites 49 passed ( |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
Closer verifier disposition (Provider Comparison Report)Durable report (comment 5154432709) is CONCERNS (openai + anthropic). Direct current- Substantive / still open (follow-up required):
Overstated / false when judged only on #2899's diff (cumulative #2898+#2899 on main):
#2881 stays OPEN. Opening a bounded follow-up PR for the substantive gaps above. |
Closes #2881
Automated Status Summary
Scope
Generated PR ownership is split across overlapping controllers. Maint 71 filters only
sync/workflows-heads today (.github/workflows/maint-71-merge-sync-prs.yml:284), while Maint 82 and.github/scripts/sync_dependency_campaign.js:493-530separately decide which exceptions are claimable by Codex. Dev-tool-sync PRs therefore follow a different merge path, and unchanged exception state can continue to produce automation work.This is a current efficiency defect: repeated observation of the same blocker consumes agent runs without changing repository state. One repository-owned reconciliation contract should decide merge/close disposition, and agent handoff should occur only when the exception fingerprint changes.
Tasks
.github/workflows/maint-71-merge-sync-prs.ymlto reconcile bothsync/workflows-anddeps/sync-dev-versions-PRs through the same contract..github/scripts/sync_pr_merge_contract.js: current, awaiting-checks, review-blocked, repo-local-failure, shared-source-failure, superseded, expired, and owner-decision..github/workflows/maint-82-sync-dependency-auto-pilot.ymlconsume those records and own durable campaign state rather than recomputing merge disposition independently..github/scripts/sync_dependency_campaign.jsto include only state that should trigger new work; preserveupdated_at, claim generation, and lease history.campaign:needs-local-codexonly for new or materially changed actionable exception fingerprints, and remove it after terminal disposition or successful claim completion.docs/ops/SYNC_DEPENDENCY_CAMPAIGN.md; mark local watcher changes as an operator follow-up, not repository implementation.Acceptance criteria
.github/scripts/__tests__/sync_dependency_campaign.test.jsproves the same exception observed on three consecutive runs creates one claim generation and one agent handoff..github/scripts/__tests__/sync_pr_merge_contract.test.jsapplies identical merge/review/check rules to sync and dev-tool-sync fixtures.updated_at, verify the campaign test detects duplicate handoffs, then revert and verify it passes.python scripts/dev_check.py --action testand workflow validation successfully.Summary by CodeRabbit
New Features
Tests