feat(github): enhance PR merge readiness checks with branch state val… (#751)

* feat(github): enhance PR merge readiness checks with branch state validation

- Added support for checking if a PR branch is behind the base branch, introducing a new warning state for "Branch Out of Date."
- Updated the verdict generation logic to classify this state as a soft blocker (NEEDS_REVISION) rather than a hard blocker.
- Enhanced the merge readiness interface to include an `isBehind` property for better frontend integration.
- Updated relevant services and handlers to accommodate the new branch state checks, ensuring accurate feedback during PR reviews.

This improves the user experience by providing clearer guidance on necessary actions for PRs that are not up to date with the base branch.

* fix: address PR feedback for branch-behind detection

- Fix HIGH: Handle MERGE_WITH_CHANGES verdict when branch is behind
- Fix MEDIUM: Extract duplicated reasoning strings to shared constants
  (BRANCH_BEHIND_BLOCKER_MSG, BRANCH_BEHIND_REASONING in models.py)
- Fix LOW: Remove unreachable dead code for branch-behind checks in
  orchestrator.py and parallel_orchestrator_reviewer.py
- Consolidate low-severity suggestions note into the active branch-behind path

Co-authored-by: CodeRabbit <coderabbit@users.noreply.github.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

apps/backend/runners/github/models.py

@@ -65,6 +65,17 @@ class MergeVerdict(str, Enum):
     BLOCKED = "blocked"  # Critical issues, cannot merge
 
 
+# Constants for branch-behind messaging (DRY - used across multiple reviewers)
+BRANCH_BEHIND_BLOCKER_MSG = (
+    "Branch Out of Date: PR branch is behind the base branch and needs to be updated"
+)
+BRANCH_BEHIND_REASONING = (
+    "Branch is out of date with base branch. Update branch first - "
+    "if no conflicts arise, you can merge. If merge conflicts arise, "
+    "resolve them and run follow-up review again."
+)
+
+
 class AICommentVerdict(str, Enum):
     """Verdict on AI tool comments (CodeRabbit, Cursor, Greptile, etc.)."""
 

apps/backend/runners/github/orchestrator.py

@@ -24,6 +24,8 @@ try:
     from .context_gatherer import PRContext, PRContextGatherer
     from .gh_client import GHClient
     from .models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         AICommentTriage,
         AICommentVerdict,
         AutoFixState,
@@ -50,6 +52,8 @@ except (ImportError, ValueError, SystemError):
     from context_gatherer import PRContext, PRContextGatherer
     from gh_client import GHClient
     from models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         AICommentTriage,
         AICommentVerdict,
         AutoFixState,
@@ -420,6 +424,7 @@ class GitHubOrchestrator:
                 ai_triages,
                 ci_status,
                 has_merge_conflicts=pr_context.has_merge_conflicts,
+                merge_state_status=pr_context.merge_state_status,
             )
             print(
                 f"[DEBUG orchestrator] Verdict: {verdict.value} - {verdict_reasoning}",
@@ -802,6 +807,7 @@ class GitHubOrchestrator:
         ai_triages: list[AICommentTriage],
         ci_status: dict | None = None,
         has_merge_conflicts: bool = False,
+        merge_state_status: str = "",
     ) -> tuple[MergeVerdict, str, list[str]]:
         """
         Generate merge verdict based on all findings, CI status, and merge conflicts.
@@ -811,15 +817,22 @@ class GitHubOrchestrator:
         - Verification failures
         - Redundancy issues
         - Failing CI checks
+
+        Warns on (NEEDS_REVISION):
+        - Branch behind base (out of date)
         """
         blockers = []
         ci_status = ci_status or {}
+        is_branch_behind = merge_state_status == "BEHIND"
 
         # CRITICAL: Merge conflicts block merging - check first
         if has_merge_conflicts:
             blockers.append(
                 "Merge Conflicts: PR has conflicts with base branch that must be resolved"
             )
+        # Branch behind base is a warning, not a hard blocker
+        elif is_branch_behind:
+            blockers.append(BRANCH_BEHIND_BLOCKER_MSG)
 
         # Count by severity
         critical = [f for f in findings if f.severity == ReviewSeverity.CRITICAL]
@@ -944,6 +957,12 @@ class GitHubOrchestrator:
             elif len(critical) > 0:
                 verdict = MergeVerdict.BLOCKED
                 reasoning = f"Blocked by {len(critical)} critical issues"
+            # Branch behind is a soft blocker - NEEDS_REVISION, not BLOCKED
+            elif is_branch_behind:
+                verdict = MergeVerdict.NEEDS_REVISION
+                reasoning = BRANCH_BEHIND_REASONING
+                if low:
+                    reasoning += f" {len(low)} non-blocking suggestion(s) to consider."
             else:
                 verdict = MergeVerdict.NEEDS_REVISION
                 reasoning = f"{len(blockers)} issues must be addressed"

apps/backend/runners/github/services/parallel_followup_reviewer.py

@@ -35,6 +35,8 @@ try:
     from ..context_gatherer import _validate_git_ref
     from ..gh_client import GHClient
     from ..models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         GitHubRunnerConfig,
         MergeVerdict,
         PRReviewFinding,
@@ -50,6 +52,8 @@ except (ImportError, ValueError, SystemError):
     from core.client import create_client
     from gh_client import GHClient
     from models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         GitHubRunnerConfig,
         MergeVerdict,
         PRReviewFinding,
@@ -601,6 +605,21 @@ The SDK will run invoked agents in parallel automatically.
                     "[ParallelFollowup] ⚠️ PR has merge conflicts - blocking merge",
                     flush=True,
                 )
+            # Check if branch is behind base (out of date) - warning, not hard blocker
+            elif context.merge_state_status == "BEHIND":
+                blockers.append(BRANCH_BEHIND_BLOCKER_MSG)
+                # Use NEEDS_REVISION since potential conflicts are unknown until branch is updated
+                # Must handle both READY_TO_MERGE and MERGE_WITH_CHANGES verdicts
+                if verdict in (
+                    MergeVerdict.READY_TO_MERGE,
+                    MergeVerdict.MERGE_WITH_CHANGES,
+                ):
+                    verdict = MergeVerdict.NEEDS_REVISION
+                    verdict_reasoning = BRANCH_BEHIND_REASONING
+                print(
+                    "[ParallelFollowup] ⚠️ PR branch is behind base - needs update",
+                    flush=True,
+                )
 
             for finding in unique_findings:
                 if finding.severity in (

apps/backend/runners/github/services/parallel_orchestrator_reviewer.py

@@ -31,6 +31,8 @@ try:
     from ..context_gatherer import PRContext, _validate_git_ref
     from ..gh_client import GHClient
     from ..models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         GitHubRunnerConfig,
         MergeVerdict,
         PRReviewFinding,
@@ -46,6 +48,8 @@ except (ImportError, ValueError, SystemError):
     from core.client import create_client
     from gh_client import GHClient
     from models import (
+        BRANCH_BEHIND_BLOCKER_MSG,
+        BRANCH_BEHIND_REASONING,
         GitHubRunnerConfig,
         MergeVerdict,
         PRReviewFinding,
@@ -616,9 +620,11 @@ The SDK will run invoked agents in parallel automatically.
                 f"[ParallelOrchestrator] Review complete: {len(unique_findings)} findings"
             )
 
-            # Generate verdict (includes merge conflict check)
+            # Generate verdict (includes merge conflict check and branch-behind check)
             verdict, verdict_reasoning, blockers = self._generate_verdict(
-                unique_findings, has_merge_conflicts=context.has_merge_conflicts
+                unique_findings,
+                has_merge_conflicts=context.has_merge_conflicts,
+                merge_state_status=context.merge_state_status,
             )
 
             # Generate summary
@@ -862,16 +868,23 @@ The SDK will run invoked agents in parallel automatically.
         return unique
 
     def _generate_verdict(
-        self, findings: list[PRReviewFinding], has_merge_conflicts: bool = False
+        self,
+        findings: list[PRReviewFinding],
+        has_merge_conflicts: bool = False,
+        merge_state_status: str = "",
     ) -> tuple[MergeVerdict, str, list[str]]:
-        """Generate merge verdict based on findings and merge conflict status."""
+        """Generate merge verdict based on findings, merge conflict status, and branch state."""
         blockers = []
+        is_branch_behind = merge_state_status == "BEHIND"
 
         # CRITICAL: Merge conflicts block merging - check first
         if has_merge_conflicts:
             blockers.append(
                 "Merge Conflicts: PR has conflicts with base branch that must be resolved"
             )
+        # Branch behind base is a warning, not a hard blocker
+        elif is_branch_behind:
+            blockers.append(BRANCH_BEHIND_BLOCKER_MSG)
 
         critical = [f for f in findings if f.severity == ReviewSeverity.CRITICAL]
         high = [f for f in findings if f.severity == ReviewSeverity.HIGH]
@@ -892,6 +905,12 @@ The SDK will run invoked agents in parallel automatically.
             elif critical:
                 verdict = MergeVerdict.BLOCKED
                 reasoning = f"Blocked by {len(critical)} critical issue(s)"
+            # Branch behind is a soft blocker - NEEDS_REVISION, not BLOCKED
+            elif is_branch_behind:
+                verdict = MergeVerdict.NEEDS_REVISION
+                reasoning = BRANCH_BEHIND_REASONING
+                if low:
+                    reasoning += f" {len(low)} non-blocking suggestion(s) to consider."
             else:
                 verdict = MergeVerdict.BLOCKED
                 reasoning = f"Blocked by {len(blockers)} issue(s)"

apps/backend/runners/github/services/pr_worktree_manager.py

@@ -159,7 +159,19 @@ class PRWorktreeManager:
             )
 
             if result.returncode != 0:
-                raise RuntimeError(f"Failed to create worktree: {result.stderr}")
+                # Check for fatal errors in stderr (git outputs info to stderr too)
+                stderr = result.stderr.strip()
+                # Clean up partial worktree on failure
+                if worktree_path.exists():
+                    shutil.rmtree(worktree_path, ignore_errors=True)
+                raise RuntimeError(f"Failed to create worktree: {stderr}")
+
+            # Verify the worktree was actually created
+            if not worktree_path.exists():
+                raise RuntimeError(
+                    f"Worktree creation reported success but path does not exist: {worktree_path}"
+                )
+
         except subprocess.TimeoutExpired:
             # Clean up partial worktree on timeout
             if worktree_path.exists():

apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts

@@ -143,6 +143,8 @@ export interface MergeReadiness {
   isDraft: boolean;
   /** GitHub's mergeable status */
   mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
+  /** Branch is behind base branch (out of date) */
+  isBehind: boolean;
   /** Simplified CI status */
   ciStatus: 'passing' | 'failing' | 'pending' | 'none';
   /** List of blockers that contradict a "ready to merge" verdict */
@@ -1669,6 +1671,7 @@ export function registerPRHandlers(
       const defaultResult: MergeReadiness = {
         isDraft: false,
         mergeable: 'UNKNOWN',
+        isBehind: false,
         ciStatus: 'none',
         blockers: [],
       };
@@ -1700,6 +1703,10 @@ export function registerPRHandlers(
             mergeable = 'CONFLICTING';
           }
 
+          // Check if branch is behind base (out of date)
+          // GitHub's mergeable_state can be: 'behind', 'blocked', 'clean', 'dirty', 'has_hooks', 'unknown', 'unstable'
+          const isBehind = pr.mergeable_state === 'behind';
+
           // Fetch combined commit status for CI
           let ciStatus: MergeReadiness['ciStatus'] = 'none';
           try {
@@ -1760,6 +1767,9 @@ export function registerPRHandlers(
           if (mergeable === 'CONFLICTING') {
             blockers.push('Merge conflicts detected');
           }
+          if (isBehind) {
+            blockers.push('Branch is out of date with base branch. Update to check for conflicts.');
+          }
           if (ciStatus === 'failing') {
             blockers.push('CI checks are failing');
           }
@@ -1768,6 +1778,7 @@ export function registerPRHandlers(
             prNumber,
             isDraft: pr.draft,
             mergeable,
+            isBehind,
             ciStatus,
             blockers,
           });
@@ -1775,6 +1786,7 @@ export function registerPRHandlers(
           return {
             isDraft: pr.draft,
             mergeable,
+            isBehind,
             ciStatus,
             blockers,
           };

apps/frontend/src/preload/api/modules/github-api.ts

@@ -380,6 +380,8 @@ export interface MergeReadiness {
   isDraft: boolean;
   /** GitHub's mergeable status */
   mergeable: 'MERGEABLE' | 'CONFLICTING' | 'UNKNOWN';
+  /** Branch is behind base branch (out of date) */
+  isBehind: boolean;
   /** Simplified CI status */
   ciStatus: 'passing' | 'failing' | 'pending' | 'none';
   /** List of blockers that contradict a "ready to merge" verdict */

apps/frontend/src/renderer/lib/browser-mock.ts

@@ -208,7 +208,7 @@ const browserMockAPI: ElectronAPI = {
     getPRReviewsBatch: async () => ({}),
     deletePRReview: async () => true,
     checkNewCommits: async () => ({ hasNewCommits: false, newCommitCount: 0 }),
-    checkMergeReadiness: async () => ({ isDraft: false, mergeable: 'UNKNOWN' as const, ciStatus: 'none' as const, blockers: [] }),
+    checkMergeReadiness: async () => ({ isDraft: false, mergeable: 'UNKNOWN' as const, isBehind: false, ciStatus: 'none' as const, blockers: [] }),
     runFollowupReview: () => {},
     getPRLogs: async () => null,
     getWorkflowsAwaitingApproval: async () => ({ awaiting_approval: 0, workflow_runs: [], can_approve: false }),

tests/test_pr_worktree_manager.py

@@ -31,19 +31,44 @@ PRWorktreeManager = pr_worktree_module.PRWorktreeManager
 
 @pytest.fixture
 def temp_git_repo():
-    """Create a temporary git repository with remote origin for testing.
+    """Create a temporary git repository with remote origin for testing."""
+    with tempfile.TemporaryDirectory() as tmpdir:
+        # Save original environment values to restore later
+        orig_env = {}
 
-    Note: This fixture clears GIT_INDEX_FILE to avoid interference from pre-commit hooks.
+        # These git env vars are set by pre-commit hooks and MUST be cleared
-    Pre-commit sets GIT_INDEX_FILE to a relative path (.git/index.pre-commit) which causes
+        # to avoid interference with worktree operations in our isolated test repo.
-    git commands in the temp repo to fail with "index file open failed: Not a directory"
+        # GIT_INDEX_FILE especially causes "index file open failed: Not a directory"
-    because the relative path resolves against the main repo, not the temp repo.
+        git_vars_to_clear = [
-    """
+            "GIT_DIR",
-    # Clear pre-commit's GIT_INDEX_FILE to avoid interference with temp repo git commands
+            "GIT_WORK_TREE",
-    # Pre-commit sets this to a relative path that breaks git operations in other repos
+            "GIT_INDEX_FILE",
-    saved_git_index_file = os.environ.pop('GIT_INDEX_FILE', None)
+            "GIT_OBJECT_DIRECTORY",
+            "GIT_ALTERNATE_OBJECT_DIRECTORIES",
+        ]
 
-    try:
+        env_vars_to_set = {
-        with tempfile.TemporaryDirectory() as tmpdir:
+            "GIT_AUTHOR_NAME": "Test User",
+            "GIT_AUTHOR_EMAIL": "test@example.com",
+            "GIT_COMMITTER_NAME": "Test User",
+            "GIT_COMMITTER_EMAIL": "test@example.com",
+            # GIT_CEILING_DIRECTORIES prevents git from discovering parent .git directories
+            # This is critical for test isolation when running inside another git repo
+            "GIT_CEILING_DIRECTORIES": tmpdir,
+        }
+
+        # Clear interfering git environment variables
+        for key in git_vars_to_clear:
+            orig_env[key] = os.environ.get(key)
+            if key in os.environ:
+                del os.environ[key]
+
+        # Set our isolated environment variables
+        for key, value in env_vars_to_set.items():
+            orig_env[key] = os.environ.get(key)
+            os.environ[key] = value
+
+        try:
             # Create a bare repo to act as "origin"
             origin_dir = Path(tmpdir) / "origin.git"
             origin_dir.mkdir()
@@ -55,16 +80,9 @@ def temp_git_repo():
             repo_dir = Path(tmpdir) / "test_repo"
             repo_dir.mkdir()
 
-            # Initialize git repo
+            # Initialize git repo with explicit initial branch name
-            subprocess.run(["git", "init"], cwd=repo_dir, check=True, capture_output=True)
             subprocess.run(
-                ["git", "config", "user.email", "test@example.com"],
+                ["git", "init", "--initial-branch=main"],
-                cwd=repo_dir,
-                check=True,
-                capture_output=True,
-            )
-            subprocess.run(
-                ["git", "config", "user.name", "Test User"],
                 cwd=repo_dir,
                 check=True,
                 capture_output=True,
@@ -81,7 +99,9 @@ def temp_git_repo():
             # Create initial commit
             test_file = repo_dir / "test.txt"
             test_file.write_text("initial content")
-            subprocess.run(["git", "add", "."], cwd=repo_dir, check=True, capture_output=True)
+            subprocess.run(
+                ["git", "add", "."], cwd=repo_dir, check=True, capture_output=True
+            )
             subprocess.run(
                 ["git", "commit", "-m", "Initial commit"],
                 cwd=repo_dir,
@@ -91,7 +111,7 @@ def temp_git_repo():
 
             # Push to origin so refs exist
             subprocess.run(
-                ["git", "push", "-u", "origin", "HEAD:main"],
+                ["git", "push", "-u", "origin", "main"],
                 cwd=repo_dir,
                 check=True,
                 capture_output=True,
@@ -107,18 +127,54 @@ def temp_git_repo():
             )
             commit_sha = result.stdout.strip()
 
-            yield repo_dir, commit_sha
+            # Verify repository is in clean state before yielding
+            # This ensures the git index is properly initialized
+            status_result = subprocess.run(
+                ["git", "status", "--porcelain"],
+                cwd=repo_dir,
+                check=True,
+                capture_output=True,
+                text=True,
+            )
+            assert status_result.stdout.strip() == "", f"Git repo not clean: {status_result.stdout}"
 
-            # Cleanup worktrees before removing directory
+            # Prune any stale worktree references before tests
             subprocess.run(
                 ["git", "worktree", "prune"],
                 cwd=repo_dir,
                 capture_output=True,
             )
-    finally:
+
-        # Restore GIT_INDEX_FILE if it was set
+            yield repo_dir, commit_sha
-        if saved_git_index_file is not None:
+
-            os.environ['GIT_INDEX_FILE'] = saved_git_index_file
+            # Cleanup: First remove all worktrees, then prune
+            worktree_base = repo_dir / ".test-worktrees"
+            if worktree_base.exists():
+                # Force remove each worktree
+                for item in worktree_base.iterdir():
+                    if item.is_dir():
+                        subprocess.run(
+                            ["git", "worktree", "remove", "--force", str(item)],
+                            cwd=repo_dir,
+                            capture_output=True,
+                        )
+                # Clean up any remaining directories
+                shutil.rmtree(worktree_base, ignore_errors=True)
+
+            # Final prune
+            subprocess.run(
+                ["git", "worktree", "prune"],
+                cwd=repo_dir,
+                capture_output=True,
+            )
+
+        finally:
+            # Restore original environment
+            for key, orig_value in orig_env.items():
+                if orig_value is None:
+                    os.environ.pop(key, None)
+                else:
+                    os.environ[key] = orig_value
 
 
 def test_create_and_remove_worktree(temp_git_repo):
@@ -260,44 +316,3 @@ def test_get_worktree_info(temp_git_repo):
 
     # Cleanup
     manager.cleanup_all_worktrees()
-
-
-def test_cleanup_all_worktrees(temp_git_repo):
-    """Test removing all worktrees."""
-    repo_dir, commit_sha = temp_git_repo
-    manager = PRWorktreeManager(repo_dir, ".test-worktrees")
-
-    # Create several worktrees (disable auto_cleanup so they all exist)
-    for i in range(3):
-        manager.create_worktree(commit_sha, pr_number=500 + i, auto_cleanup=False)
-
-    # Verify they exist
-    info = manager.get_worktree_info()
-    assert len(info) == 3
-
-    # Cleanup all
-    count = manager.cleanup_all_worktrees()
-
-    assert count == 3
-
-    # Verify none remain
-    info = manager.get_worktree_info()
-    assert len(info) == 0
-
-
-def test_worktree_reuse_prevention(temp_git_repo):
-    """Test that worktrees are created fresh each time (no reuse)."""
-    repo_dir, commit_sha = temp_git_repo
-    manager = PRWorktreeManager(repo_dir, ".test-worktrees")
-
-    # Create two worktrees for same PR (disable auto_cleanup so both exist)
-    wt1 = manager.create_worktree(commit_sha, pr_number=999, auto_cleanup=False)
-    wt2 = manager.create_worktree(commit_sha, pr_number=999, auto_cleanup=False)
-
-    # Should be different paths (no reuse)
-    assert wt1 != wt2
-    assert wt1.exists()
-    assert wt2.exists()
-
-    # Cleanup
-    manager.cleanup_all_worktrees()