From 947ac0b08aeaf09f48c5454c508fd81c7cfae21f Mon Sep 17 00:00:00 2001 From: CleverThis Date: Mon, 11 May 2026 17:53:02 +0000 Subject: [PATCH 1/2] fix: preserve stale branch with pending changes for plan apply When cleanup_stable() was re-invoked during plan execute recovery, it unconditionally destroyed the worktree branch even if it contained committed work waiting for plan apply to merge, causing plan apply to find zero artifacts. Fix: add allow_destroy_with_changes parameter (default False) to cleanup_stale(). When False, inspect the branch diff against HEAD before destroying and preserve branches with insertions or modifications so that plan apply retains access to the work products. Only truly empty or deletions-only branches are treated as stale. - git_worktree.py: Add allow_destroy_with_changes param with diff inspection - plan.py: Pass True explicitly in _cleanup_sandbox_for_plan (plan cancel) --- src/cleveragents/cli/commands/plan.py | 4 +- .../infrastructure/sandbox/git_worktree.py | 74 ++++++++++++++++++- 2 files changed, 74 insertions(+), 4 deletions(-) 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..bebc910fd 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, -- 2.52.0 From f7c297c620172ed46ee6b8c85a62e6c8d6020980 Mon Sep 17 00:00:00 2001 From: HAL 9000 Date: Mon, 11 May 2026 19:38:17 +0000 Subject: [PATCH 2/2] fix(lint): replace EN DASH with hyphen-minus in comment --- src/cleveragents/infrastructure/sandbox/git_worktree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cleveragents/infrastructure/sandbox/git_worktree.py b/src/cleveragents/infrastructure/sandbox/git_worktree.py index bebc910fd..b59cbb7d1 100644 --- a/src/cleveragents/infrastructure/sandbox/git_worktree.py +++ b/src/cleveragents/infrastructure/sandbox/git_worktree.py @@ -245,7 +245,7 @@ class GitWorktreeSandbox: ) return False - # Branch has only deletions or is empty – safe to clean up. + # 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, -- 2.52.0