Skip to content

Fix merge blockers lost during concurrent rebuilds - #2346

Merged
gsxdsm merged 4 commits into
Runfusion:mainfrom
flexi767:codex/fix-merge-review-blocker-rebuild
Jul 22, 2026
Merged

Fix merge blockers lost during concurrent rebuilds#2346
gsxdsm merged 4 commits into
Runfusion:mainfrom
flexi767:codex/fix-merge-review-blocker-rebuild

Conversation

@flexi767

@flexi767 flexi767 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Preserve blocking merge-review reasons when main advances and the clean-room squash must be rebuilt.
  • Recover the latest unresolved blocking reason from task history when a later merge retry starts.
  • Require reviewers to validate prior blockers against the complete resulting tree, not only a smaller residual diff.
  • Add regression coverage for both concurrent-main rebuilds and durable retry recovery.

Why

A corrective clean-room squash can be approved and then discarded when main advances before landing. The rebuild previously reset the reviewer context, allowing a later, smaller squash to be approved and the task to be finalized as Done without rechecking the original correctness blocker.

Impact

Tasks with unresolved blocking review findings can no longer become Done merely because a concurrent rebuild or later retry loses that review context.

Validation

  • pnpm --filter @fusion/engine exec vitest run src/__tests__/merger-ai.test.ts — 45 passed
  • pnpm --filter @fusion/engine typecheck
  • ESLint on the changed merger source files
  • Changeset format check

Summary by CodeRabbit

  • Bug Fixes

    • Merge and review blockers now remain active across rebuilds and retry attempts.
    • Previous blocking reasons are preserved alongside newly identified issues.
    • Empty corrective rebuilds are reviewed before being accepted as complete.
    • Tasks can no longer be finalized solely because a rebuilt diff is smaller when unresolved blockers remain.
  • Documentation

    • Updated release notes to describe the improved blocker behavior.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@flexi767, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c86cf74-bdb2-45fa-8083-9465023d0a73

📥 Commits

Reviewing files that changed from the base of the PR and between 9145c71 and 2838122.

📒 Files selected for processing (2)
  • packages/engine/src/__tests__/merger-ai.test.ts
  • packages/engine/src/merger-ai.ts
📝 Walkthrough

Walkthrough

Merge-review blocker reasons now persist across concurrent-main rebuilds, later retries, and empty corrective rebuilds. The merge flow recovers prior blockers, carries them through review outcomes, strengthens corrective prompts, and adds regression coverage.

Changes

Merge blocker persistence

Layer / File(s) Summary
Recover and seed blocking reasons
packages/engine/src/merger-ai.ts
Task history is scanned for unresolved blocking reasons, and repository landing initializes review state from the current task snapshot.
Propagate reasons through merge exits
packages/engine/src/merger-ai.ts
mergeAndReview accepts initial reasons and returns updated reasons with the squash result across empty, approval, blocking, and advisory paths.
Strengthen prompts and regression coverage
packages/engine/src/merger-ai-prompts.ts, packages/engine/src/__tests__/merger-ai.test.ts, .changeset/merge-review-blockers-survive-rebuilds.md
Corrective prompts require checking the complete resulting tree, while tests cover rebuilds, retries, empty corrective merges, and accumulated blockers; a patch changeset documents the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TaskStore
  participant landOneRepo
  participant mergeAndReview
  participant ReviewAgent
  TaskStore-->>landOneRepo: task history with blocker reasons
  landOneRepo->>mergeAndReview: initialPriorReasons
  mergeAndReview->>ReviewAgent: prompt with prior reasons
  ReviewAgent-->>mergeAndReview: approval or new rejection reason
  mergeAndReview-->>landOneRepo: squashSha and accumulated priorReasons
Loading

Possibly related PRs

  • Runfusion/Fusion#2160: Updates AI merge review reason generation and retryability, overlapping with this PR’s blocker persistence changes.

Suggested reviewers: gsxdsm

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: merge blockers no longer disappear during concurrent rebuilds.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@flexi767

Copy link
Copy Markdown
Contributor Author

The implementation is complete and the current checks are green. Marking this ready for maintainer review.

@flexi767
flexi767 marked this pull request as ready for review July 21, 2026 16:38
Copilot AI review requested due to automatic review settings July 21, 2026 16:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR keeps unresolved merge-review blockers active across rebuilds and retries. The main changes are:

  • Recovers unresolved blockers from task history.
  • Carries earlier blockers through corrective review passes and concurrent-main rebuilds.
  • Reviews zero-ahead and empty rebuilds when blockers remain.
  • Checks prior blockers against the complete resulting tree.
  • Adds tests for blocker persistence and retry recovery.

Confidence Score: 5/5

This looks safe to merge.

  • The updated flow preserves blockers through each previously reported retry and rebuild path.
  • Empty and zero-ahead results cannot bypass review while durable blockers remain.
  • The tests cover the fixed lifecycle paths and accumulated blocker recovery.

Important Files Changed

Filename Overview
packages/engine/src/merger-ai.ts Preserves unresolved blockers across corrective passes, empty merges, zero-ahead retries, and concurrent rebuilds.
packages/engine/src/merger-ai-prompts.ts Directs reviewers to validate earlier blockers against the complete resulting tree.
packages/engine/src/tests/merger-ai.test.ts Adds tests for concurrent rebuilds, durable recovery, empty retries, and accumulated blockers.
.changeset/merge-review-blockers-survive-rebuilds.md Adds the patch release note for persistent merge-review blockers.

Reviews (3): Last reviewed commit: "test(engine): assert empty corrective me..." | Re-trigger Greptile

Comment thread packages/engine/src/merger-ai.ts Outdated
Comment thread packages/engine/src/merger-ai.ts Outdated
Comment thread packages/engine/src/merger-ai.ts
Comment thread packages/engine/src/merger-ai.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@packages/engine/src/__tests__/merger-ai.test.ts`:
- Around line 298-316: The test “reviews an empty corrective rebuild before
accepting it as a no-op” should verify the full no-op invariant: assert
mergeAgent was called twice, capture the integration tip or result before the
corrective pass, and confirm it remains unchanged after the approved empty
rebuild. Keep the existing review-agent assertions and blocker verification
intact.
- Around line 306-307: Replace the plain technical-decision comment in the
corrective-pass test near the merger test callback with a dated
FNXC:MergeReviewBlockers yyyy-MM-dd-hh:mm: comment, preserving and updating the
rationale to accurately explain why the clean-room tree remains at the tip.
🪄 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 Plus

Run ID: 49b881f5-3e84-407f-8a42-0952681a1d6c

📥 Commits

Reviewing files that changed from the base of the PR and between 29da3de and 9145c71.

📒 Files selected for processing (2)
  • packages/engine/src/__tests__/merger-ai.test.ts
  • packages/engine/src/merger-ai.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/engine/src/merger-ai.ts

Comment thread packages/engine/src/__tests__/merger-ai.test.ts
Comment thread packages/engine/src/__tests__/merger-ai.test.ts Outdated
@gsxdsm
gsxdsm merged commit 8f7f527 into Runfusion:main Jul 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants