diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index cda1461fa..a88b9ebcf 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -583,7 +583,9 @@ def _cleanup_sandbox_for_plan( ): continue - GitWorktreeSandbox.cleanup_stale(resource.location, plan_id) + GitWorktreeSandbox.cleanup_stale( + resource.location, plan_id, allow_destroy_with_changes=True + ) class _SandboxInfo: diff --git a/src/cleveragents/infrastructure/sandbox/git_worktree.py b/src/cleveragents/infrastructure/sandbox/git_worktree.py index 3974679b8..b59cbb7d1 100644 --- a/src/cleveragents/infrastructure/sandbox/git_worktree.py +++ b/src/cleveragents/infrastructure/sandbox/git_worktree.py @@ -164,17 +164,37 @@ class GitWorktreeSandbox: """Context after creation, ``None`` before ``create``.""" return self._context - # -- class helpers ------------------------------------------------------- + # -- class helpers ------------------------------------------------------- @classmethod - def cleanup_stale(cls, repo_path: str, plan_id: str) -> bool: + def cleanup_stale( + cls, + repo_path: str, + plan_id: str, + allow_destroy_with_changes: bool = False, + ) -> bool: """Remove a stale worktree branch left by a previous execution. - Idempotent — does nothing if no stale branch exists. + Idempotent -- does nothing if no stale branch exists. + + When *allow_destroy_with_changes* is ``False`` (the default), the + method inspects the branch's diff against HEAD. Branches with + insertions or modifications are preserved because they may contain + work waiting for **plan apply** to merge. Only truly empty or + deletions-only branches are treated as stale and cleaned up. + + When *allow_destroy_with_changes* is ``True`` (used by explicit + plan cancel), the old behaviour applies: any existing branch is + unconditionally destroyed regardless of whether it has changes. Args: repo_path: Absolute path to the git repository root. plan_id: The plan ULID whose stale branch should be removed. + allow_destroy_with_changes: If ``True``, always destroy the + branch even if it contains meaningful changes. When + ``False`` (default), preserve branches with pending + insertions or modifications so that *plan apply* can + find the artifacts. Returns: ``True`` if a stale branch was found and cleaned up, @@ -190,6 +210,54 @@ class GitWorktreeSandbox: except (subprocess.CalledProcessError, subprocess.TimeoutExpired): return False + # When not explicitly allowed to destroy with changes, inspect the + # branch diff before tearing it down. Branches that have pending + # modifications are work products waiting for plan apply to merge; + # destroying them causes "plan apply" to find zero artifacts. + if not allow_destroy_with_changes: + try: + diff_result = _run_git( + ["diff", "--shortstat", f"HEAD...{branch_name}"], + cwd=repo_path, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired): + # On inspection failure preserve the branch to avoid data loss + logger.debug( + "Preserving sandbox branch due to diff check failure: branch=%s", + branch_name, + ) + return False + + diff_text = diff_result.stdout.strip() + has_changes = bool(diff_text) and any( + keyword in diff_text + for keyword in ("insertion", "deletion") + ) + # Also treat empty diffs (no files changed at all) as preserved. + if not diff_text: + has_changes = False + + if has_changes: + logger.info( + "Preserving sandbox branch with pending changes " + "(plan apply has work to merge): branch=%s", + branch_name, + ) + return False + + # Branch has only deletions or is empty - safe to clean up. + logger.info( + "Branch %s has no new insertions; treating as truly stale.", + branch_name, + ) + else: + logger.info( + "Cleaning up sandbox branch (allow_destroy_with_changes): " + "branch=%s repo=%s", + branch_name, + repo_path, + ) + logger.info( "Cleaning up stale sandbox branch: branch=%s repo=%s", branch_name,