Skip to content

fix(engine): short-circuit zero-commits-ahead branch before AI-merge clean-room churn#1920

Merged
gsxdsm merged 2 commits into
Runfusion:mainfrom
flexi767:fix/empty-branch-merge-wedge
Jul 6, 2026
Merged

fix(engine): short-circuit zero-commits-ahead branch before AI-merge clean-room churn#1920
gsxdsm merged 2 commits into
Runfusion:mainfrom
flexi767:fix/empty-branch-merge-wedge

Conversation

@flexi767

@flexi767 flexi767 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Problem

When a coding agent produces zero commits relative to base, the AI-merge path wedges the card terminally:

  1. runAiMergelandOneRepo builds a clean-room worktree and runs a dependency install.
  2. On a non-workspace land the dep-install step throws hard (if (!ctx.nonFatalDependencySync) throw depsErr;).
  3. The throw is transient-classified and retried up to MAX_AUTO_MERGE_TRANSIENT_RETRIESAuto-merge transient retries exhausted (3/3).
  4. The card is parked failed (→ archived), even though the correct outcome for an empty branch is a no-op finalize.

The truly-empty branch would reach outcome: "empty" anyway via mergeAndReview producing no squashSha — but only after the throw-prone churn that fails first.

The canonical aiMergeTask/classifyOwnedLandedEvidence path already has an early empty-own-diff fast-path; the runAiMergelandOneRepo path did not.

Fix

Short-circuit inside landOneRepo, right after the tipSha computation and before the clean-room build, when the branch is a confident zero commits ahead of the integration tip:

const aheadRaw = await git(["rev-list", "--count", `${integrationBranch}..${branch}`], repoRootDir).catch(() => "");
if (Number.parseInt(aheadRaw.trim(), 10) === 0) {
  await audit.git({ type: "merge:ai-empty", target: integrationBranch, metadata: { taskId, tipSha } });
  return { outcome: "empty", tipSha, integrationBranch };
}
  • Returns the identical { outcome: "empty", tipSha, integrationBranch } shape and the same merge:ai-empty audit event the downstream already handles, so runAiMerge's empty-outcome handling (block-to-todo / no-op finalize) is unchanged.
  • Only short-circuits on a confident 0: a git failure yields ""parseIntNaN (≠ 0) and falls through to the normal path — no behavior change on error.
  • Placed in landOneRepo (not runAiMerge) because it is shared by both the single-repo and workspace per-repo callers.

Test

New test in merger-ai.test.ts asserts the merge agent is never invoked for a 0-ahead branch, the result is a no-op, main is unmoved, and the card moves to done with preserveProgress. Without the fix the branch reaches the clean room and mergeAndReview invokes the merge agent, so the assertion fails — it genuinely guards the short-circuit.

Full engine merge-suite regression run is green (merger-ai, workspace-merger and lease variants, cleanup, dependency-sync, group-merge, classify-owned-landed-evidence).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved merge handling for branches with no new commits ahead of the target branch.
    • Empty or already-synced branches now finish as a no-op instead of triggering merge work.
    • Tasks in this scenario still move to done, while the main branch remains unchanged.

…clean-room churn

An AI-merge land of a branch with zero commits ahead of the integration
tip (the shape a coding agent that produced no commits leaves behind)
builds a clean-room worktree and runs a dependency install before
reaching the empty outcome via mergeAndReview. On a non-workspace land
the dep install throws hard, so the merge is transient-retried to
exhaustion (3/3) and the card is terminally parked failed.

Short-circuit on a CONFIDENT zero-ahead count (a git failure yields NaN
and falls through) and return the identical outcome:"empty" shape
before the throw-prone churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 18:09

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.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an early short-circuit in landOneRepo that checks commits-ahead count between task branch and integration branch via git rev-list; if zero, it audits an event and returns an "empty" outcome before worktree creation. Includes a test validating this behavior.

Changes

Empty Branch Short-Circuit

Layer / File(s) Summary
Empty-branch detection and short-circuit
packages/engine/src/merger-ai.ts, packages/engine/src/__tests__/merger-ai.test.ts
landOneRepo computes commits-ahead via git rev-list --count; if zero, it audits merge:ai-empty and returns { outcome: "empty" } before worktree/merge work; a new test verifies the merge agent isn't called, main stays unchanged, and the task moves to done.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant landOneRepo
  participant Git
  participant AuditLog
  participant MergeAgent

  Caller->>landOneRepo: runAiMerge(task)
  landOneRepo->>Git: rev-list --count integration..branch
  Git-->>landOneRepo: commit count
  alt count is 0
    landOneRepo->>AuditLog: audit merge:ai-empty
    landOneRepo-->>Caller: outcome: empty (noOp)
  else count > 0
    landOneRepo->>MergeAgent: create worktree, sync, merge
    MergeAgent-->>Caller: outcome: merged
  end
Loading

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 matches the main change: early short-circuiting zero-ahead AI merge branches before clean-room work.
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.

@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds an early no-op path for AI merges when a task branch has no commits ahead. The main changes are:

  • Adds a zero-ahead check in landOneRepo before clean-room setup.
  • Emits the existing empty-merge audit event and return shape.
  • Adds a regression test that verifies the merge agent is not called for an empty branch.

Confidence Score: 4/5

This is close, but the early no-op check should be fixed before merging.

  • The new fast path can count against the wrong ref when a tag or other ref shares the integration branch name.
  • That can make a branch with real commits finish as an empty no-op.
  • The test covers the simple empty case, but not the ref-collision case.

packages/engine/src/merger-ai.ts

Important Files Changed

Filename Overview
packages/engine/src/merger-ai.ts Adds the zero-ahead fast path before clean-room setup, but the new range still uses bare refs and can skip a real merge when ref names collide.
packages/engine/src/tests/merger-ai.test.ts Adds coverage for the normal empty-branch case where the task branch is exactly at the integration branch tip.

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/empty-branc..." | Re-trigger Greptile

// non-workspace land hard-fails), so the merge gets transient-retried to
// exhaustion and the card is parked failed. Only short-circuit on a CONFIDENT
// 0: a git failure yields "" → parseInt → NaN (≠ 0) and falls through.
const aheadRaw = await git(["rev-list", "--count", `${integrationBranch}..${branch}`], repoRootDir).catch(() => "");

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.

P2 Bare Ref Names Can Miscount

This early return uses bare branch names for the rev-list range, while the surrounding code verifies refs/heads/${integrationBranch} explicitly. If a tag or other ref shares the integration branch name, Git can resolve the range against that ref and report 0, causing landOneRepo to return empty before the normal merge path even though the task branch still has work relative to the branch head.

Suggested change
const aheadRaw = await git(["rev-list", "--count", `${integrationBranch}..${branch}`], repoRootDir).catch(() => "");
const aheadRaw = await git(["rev-list", "--count", `refs/heads/${integrationBranch}..refs/heads/${branch}`], repoRootDir).catch(() => "");

@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.

🧹 Nitpick comments (1)
packages/engine/src/__tests__/merger-ai.test.ts (1)

437-461: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the git-failure fallthrough case too.

This test covers the confident-zero short-circuit path well, but the accompanying source comment explicitly calls out that a git rev-list failure should fall through to the normal flow (NaN ≠ 0) rather than short-circuiting. A follow-up test asserting that behavior (e.g. by making the branch ref invalid) would guard against future regressions of that fallback safety net.

🤖 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 `@packages/engine/src/__tests__/merger-ai.test.ts` around lines 437 - 461, Add
a follow-up test in merger-ai.test.ts for runAiMerge that covers the git
rev-list failure fallback: make the branch ref invalid or otherwise force the
rev-list check to fail, then assert the code does not short-circuit as a
zero-ahead branch and instead continues into the normal merge flow (including
mergeAgent being invoked). Use the existing zero-commits-ahead test as the
pattern and reference runAiMerge, mergeAgent, and the zero-ahead short-circuit
behavior to keep the regression coverage aligned.
🤖 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.

Nitpick comments:
In `@packages/engine/src/__tests__/merger-ai.test.ts`:
- Around line 437-461: Add a follow-up test in merger-ai.test.ts for runAiMerge
that covers the git rev-list failure fallback: make the branch ref invalid or
otherwise force the rev-list check to fail, then assert the code does not
short-circuit as a zero-ahead branch and instead continues into the normal merge
flow (including mergeAgent being invoked). Use the existing zero-commits-ahead
test as the pattern and reference runAiMerge, mergeAgent, and the zero-ahead
short-circuit behavior to keep the regression coverage aligned.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 02024b5a-1c93-4465-a7e9-8277d44f1dbc

📥 Commits

Reviewing files that changed from the base of the PR and between 17c4007 and ca2fbbd.

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

// non-workspace land hard-fails), so the merge gets transient-retried to
// exhaustion and the card is parked failed. Only short-circuit on a CONFIDENT
// 0: a git failure yields "" → parseInt → NaN (≠ 0) and falls through.
const aheadRaw = await git(["rev-list", "--count", `${integrationBranch}..${branch}`], repoRootDir).catch(() => "");

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.

P1 Qualify branch refs

The empty-branch check still lets Git resolve the integration side as a bare ref. When a repo has a tag or other ref named like the integration branch, rev-list can count from that ref instead of refs/heads/<integrationBranch> and return 0. This branch then returns outcome: "empty", skips the merge path, and finalizes the task as a no-op even though the task branch still has commits relative to the real branch head.

Suggested change
const aheadRaw = await git(["rev-list", "--count", `${integrationBranch}..${branch}`], repoRootDir).catch(() => "");
const aheadRaw = await git(["rev-list", "--count", `refs/heads/${integrationBranch}..refs/heads/${branch}`], repoRootDir).catch(() => "");

@gsxdsm gsxdsm merged commit 6c9989c into Runfusion:main Jul 6, 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