feat(sync): lease generated delivery attempts - #2895
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: 1 minute 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 (9)
Comment |
|
Runner dispatch state for codex on PR #2895. Do not edit. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdbf0fb859
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR introduces a “leased delivery attempt” provenance marker for generated sync PRs and begins enforcing that lease contract during automated merge, aligning sync coordination around a durable campaign issue rather than long-lived generated PRs.
Changes:
- Emit a machine-readable
sync-pr-delivery-record/v1marker (with lease expiry + desired tree hash) in PR bodies created by Maint 68 (consumer sync) and Maint 52 (dev-tool sync). - Add a new JS lease-contract module (
sync_pr_lease_contract.js) plus unit tests, and wire lease parsing/eligibility into the sync PR merge contract. - Update Maint 71 to select merge candidates via the new “merge-eligible” selection path and record delivery-contract blocks in results.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/ops/DURABLE_TRACKING_ISSUES.md | Documents the “durable issue, leased PR” model for generated sync deliveries. |
| .github/workflows/maint-71-merge-sync-prs.yml | Uses delivery-record-based merge eligibility when selecting the active sync PR to merge. |
| .github/workflows/maint-68-sync-consumer-repos.yml | Adds delivery-record marker (lease + desired tree hash) to newly created consumer sync PR bodies. |
| .github/workflows/maint-52-sync-dev-versions.yml | Adds delivery-record marker (lease + desired tree hash) to newly created dev-tool sync PR bodies. |
| .github/scripts/sync_pr_merge_contract.js | Adds selectMergeEligibleSyncPr and extends merge reporting to include delivery-contract blocks. |
| .github/scripts/sync_pr_lease_contract.js | Implements delivery-record parsing/validation and merge-eligibility checks. |
| .github/scripts/tests/sync-pr-merge-contract.test.js | Extends merge-contract tests for delivery-record gating and report summary counts. |
| .github/scripts/tests/sync_pr_lease_contract.test.js | Adds tests covering delivery-record formatting/parsing and eligibility decisions. |
|
Addressed the current delivery-lease review findings in d8f4503. Existing Maint 52 attempts now renew their marker without restarting unchanged checks; Maint 68 rebuilds and refreshes existing generated attempts; and Maint 71 compares the selected PR head tree against the leased desired tree before merge, while stale predecessors are cleaned before a delivery-contract block is reported. Validation: node --test sync-pr-merge-contract + sync_pr_lease_contract (18 passed), focused pytest delivery/body tests (12 passed), check_gate_diff_quality, dev_check --changed, and git diff --check. |
🤖 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. |
… assert
Unquoted HEAD^{tree} tripped shellcheck SC1083 in Parse/lint; the
consolidation test now expects selectMergeEligibleSyncPr after the lease
eligibility wrapper replaced the bare selectActiveSyncPr call site.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closer CI recovery on
Next: wait for fresh CI on |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
DisagreementNo major disagreements detected. Unique Insights
🔍 LangSmith Traces |
Closer disposition — verifier CONCERNS (not false-positive)Audited dual-provider Reopened source issue #2880 with a sequencing comment. Leaving this PR's |
Closes #2880
Automated Status Summary
Scope
The coordination object must be allowed to persist, but a generated PR must not. The current durable campaign already models queue fingerprints and item leases in
.github/scripts/sync_dependency_campaign.js:178-230,360-456, while Maint 68 stops when an existing consumer PR is found (.github/workflows/maint-68-sync-consumer-repos.yml:681-724). That combination leaves the opener/closer system without a bounded rule for whether an old PR should be refreshed, replaced, closed, or escalated.This is a current productivity defect: generated PRs can survive across source generations, repeatedly consume review and CI attention, and still be indistinguishable from the latest intended delivery. The durable object should be the campaign issue (currently #1836), with each generated PR acting as a leased delivery attempt that has an explicit terminal disposition.
Context for Agent
Related Issues/PRs
Tasks
.github/scripts/sync_dependency_campaign.jswith campaign issue URL, plan ID, generation, repository, desired tree hash, source commit, lease expiry, and predecessor/successor PR references..github/workflows/maint-68-sync-consumer-repos.ymland.github/workflows/maint-52-sync-dev-versions.yml..github/scripts/sync_pr_merge_contract.jsand.github/workflows/maint-71-merge-sync-prs.ymlso only the latest unexpired delivery generation is merge-eligible.merged,superseded,expired, andblocked; blocked transitions must link a durable source or repo-local issue with the exact next action..github/scripts/sync_tracker_state.jsand the campaign state used by Maint 82.docs/ops/SYNC_DEPENDENCY_CAMPAIGN.md,docs/ops/DURABLE_TRACKING_ISSUES.md, anddocs/ops/CONSUMER_REPO_MAINTENANCE.mdwith the rule “durable issue, leased PR.”Acceptance criteria
.github/scripts/__tests__/sync_dependency_campaign.test.jsproves that the durable issue survives multiple delivery generations while each PR reaches exactly one terminal disposition..github/scripts/__tests__/sync_pr_lease_contract.test.jsproves that only the newest unexpired PR whose desired tree hash matches the current plan is merge-eligible.tests/workflows/test_sync_manifest_delivery.pyproves Maint 68 updates the same current-generation PR when safe and creates a replacement only after a generation or terminal-state transition.sync_pr_lease_contract.test.jsfails merge eligibility, then revert the fixture and verify the suite passes.python scripts/dev_check.py --action testand the repository workflow-validation suite successfully.