Compare commits

...

1 Commits

Author SHA1 Message Date
hamza.khyari a9c820610a fix(plan): clean up stale worktree branch before re-creating sandbox
CI / build (pull_request) Successful in 3m45s
CI / coverage (pull_request) Successful in 13m29s
CI / helm (pull_request) Successful in 55s
CI / push-validation (pull_request) Successful in 21s
CI / security (pull_request) Successful in 4m22s
CI / e2e_tests (pull_request) Successful in 6m47s
CI / lint (pull_request) Successful in 3m44s
CI / typecheck (pull_request) Successful in 4m15s
CI / quality (pull_request) Successful in 4m20s
CI / integration_tests (pull_request) Successful in 8m4s
CI / unit_tests (pull_request) Successful in 7m3s
CI / docker (pull_request) Successful in 1m29s
CI / status-check (pull_request) Successful in 3s
When a user re-executes a plan (e.g. after a failed attempt or after
reviewing the diff), _create_sandbox_for_plan crashes with 'fatal: a
branch named cleveragents/plan-<id> already exists' because the
worktree branch from the previous execute was never cleaned up.

Add GitWorktreeSandbox.cleanup_stale() classmethod in the
infrastructure layer that idempotently removes stale worktree
directories and branches.  Each git command has explicit error
handling with logging and fallback (manual rmtree if worktree remove
fails).  _create_sandbox_for_plan in the CLI layer delegates to
cleanup_stale before calling sandbox.create().

ISSUES CLOSED: #7271
2026-04-20 13:22:44 +00:00
5 changed files with 273 additions and 0 deletions
+6
View File
@@ -7,6 +7,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Fixed
- **Stale worktree branch cleanup on re-execute** (#7271): Re-executing a plan
after a failed attempt or diff review crashed with `fatal: a branch named
'cleveragents/plan-<id>' already exists`. Added `GitWorktreeSandbox.cleanup_stale()`
classmethod in the infrastructure layer that idempotently removes stale worktree
directories and branches before creating a fresh sandbox.
- **`plan apply` merge conflict cleanup** (#7250): When `git merge` fails due to
a conflict during `plan apply`, the apply command now reads the actual conflict
detail from `CalledProcessError.stdout` (git writes conflict info to stdout, not
@@ -0,0 +1,22 @@
@sandbox-reexecute-cleanup
Feature: Stale worktree branch cleanup before re-execute (#7271)
Verifies that re-executing a plan cleans up the stale worktree
branch from the previous execution before creating a fresh sandbox.
Scenario: cleanup_stale removes existing branch and worktree for srec
Given a temp git repo with a worktree branch for plan "01TESTREEXEC000000000000" for srec
When I call cleanup_stale for plan "01TESTREEXEC000000000000" for srec
Then the branch "cleveragents/plan-01TESTREEXEC000000000000" should not exist for srec
And the worktree directory should not exist for srec
Scenario: cleanup_stale is idempotent when no branch exists for srec
Given a temp git repo without any worktree branches for srec
When I call cleanup_stale for plan "01TESTNOEXIST00000000000" for srec
Then cleanup_stale should return False for srec
Scenario: create succeeds after cleanup_stale removes stale branch for srec
Given a temp git repo with a worktree branch for plan "01TESTRECREATE0000000000" for srec
When I call cleanup_stale for plan "01TESTRECREATE0000000000" for srec
And I create a fresh sandbox for plan "01TESTRECREATE0000000000" for srec
Then the fresh sandbox should be a directory for srec
And the branch "cleveragents/plan-01TESTRECREATE0000000000" should exist for srec
@@ -0,0 +1,132 @@
"""Steps for sandbox_reexecute_cleanup.feature."""
from __future__ import annotations
import os
import shutil
import subprocess
import tempfile
from pathlib import Path
from behave import given, then, when
def _git(args: list[str], cwd: str) -> subprocess.CompletedProcess[str]:
return subprocess.run(
["git", *args],
cwd=cwd,
capture_output=True,
text=True,
check=True,
timeout=10,
)
@given('a temp git repo with a worktree branch for plan "{plan_id}" for srec')
def step_create_repo_with_branch(context: object, plan_id: str) -> None:
d = tempfile.mkdtemp(prefix="srec-")
context.add_cleanup(shutil.rmtree, d, True)
_git(["init", "-q", "-b", "main"], d)
_git(["config", "user.name", "T"], d)
_git(["config", "user.email", "t@t"], d)
_git(["config", "commit.gpgsign", "false"], d)
Path(d, "file.py").write_text("content\n")
_git(["add", "."], d)
_git(["commit", "-q", "-m", "init"], d)
# Create a worktree branch (simulating a previous execute)
branch = f"cleveragents/plan-{plan_id}"
wt_dir = tempfile.mkdtemp(prefix="srec-wt-")
context.add_cleanup(shutil.rmtree, wt_dir, True)
_git(["worktree", "add", "-b", branch, wt_dir, "HEAD"], d)
context.srec_repo = d
context.srec_wt_dir = wt_dir
context.srec_branch = branch
@given("a temp git repo without any worktree branches for srec")
def step_create_clean_repo(context: object) -> None:
d = tempfile.mkdtemp(prefix="srec-clean-")
context.add_cleanup(shutil.rmtree, d, True)
_git(["init", "-q", "-b", "main"], d)
_git(["config", "user.name", "T"], d)
_git(["config", "user.email", "t@t"], d)
_git(["config", "commit.gpgsign", "false"], d)
Path(d, "file.py").write_text("content\n")
_git(["add", "."], d)
_git(["commit", "-q", "-m", "init"], d)
context.srec_repo = d
@when('I call cleanup_stale for plan "{plan_id}" for srec')
def step_call_cleanup_stale(context: object, plan_id: str) -> None:
from cleveragents.infrastructure.sandbox.git_worktree import (
GitWorktreeSandbox,
)
context.srec_cleanup_result = GitWorktreeSandbox.cleanup_stale(
context.srec_repo,
plan_id,
)
@when('I create a fresh sandbox for plan "{plan_id}" for srec')
def step_create_fresh_sandbox(context: object, plan_id: str) -> None:
from cleveragents.infrastructure.sandbox.git_worktree import (
GitWorktreeSandbox,
)
sandbox = GitWorktreeSandbox(
resource_id="res-srec-test",
original_path=context.srec_repo,
)
ctx = sandbox.create(plan_id)
context.srec_fresh_sandbox = ctx.sandbox_path
context.srec_fresh_sandbox_obj = sandbox
context.add_cleanup(sandbox.cleanup)
@then('the branch "{branch_name}" should not exist for srec')
def step_branch_not_exists(context: object, branch_name: str) -> None:
result = subprocess.run(
["git", "rev-parse", "--verify", f"refs/heads/{branch_name}"],
cwd=context.srec_repo,
capture_output=True,
check=False,
timeout=10,
)
assert result.returncode != 0, f"Branch {branch_name} still exists"
@then('the branch "{branch_name}" should exist for srec')
def step_branch_exists(context: object, branch_name: str) -> None:
result = subprocess.run(
["git", "rev-parse", "--verify", f"refs/heads/{branch_name}"],
cwd=context.srec_repo,
capture_output=True,
check=False,
timeout=10,
)
assert result.returncode == 0, f"Branch {branch_name} does not exist"
@then("the worktree directory should not exist for srec")
def step_worktree_gone(context: object) -> None:
assert not os.path.exists(context.srec_wt_dir), (
f"Worktree directory still exists: {context.srec_wt_dir}"
)
@then("cleanup_stale should return False for srec")
def step_cleanup_returned_false(context: object) -> None:
assert context.srec_cleanup_result is False, (
f"Expected False but got {context.srec_cleanup_result}"
)
@then("the fresh sandbox should be a directory for srec")
def step_fresh_sandbox_is_dir(context: object) -> None:
assert os.path.isdir(context.srec_fresh_sandbox), (
f"Fresh sandbox is not a directory: {context.srec_fresh_sandbox}"
)
+18
View File
@@ -1349,6 +1349,20 @@ def _get_lifecycle_service():
return container.plan_lifecycle_service()
def _cleanup_stale_sandbox(repo_path: str, plan_id: str) -> None:
"""Remove a stale worktree branch and directory for a plan.
Delegates to :meth:`GitWorktreeSandbox.cleanup_stale` in the
infrastructure layer. Silently does nothing if no stale branch
exists.
"""
from cleveragents.infrastructure.sandbox.git_worktree import (
GitWorktreeSandbox,
)
GitWorktreeSandbox.cleanup_stale(repo_path, plan_id)
def _create_sandbox_for_plan(
plan_id: str,
service: PlanLifecycleService,
@@ -1391,6 +1405,10 @@ def _create_sandbox_for_plan(
and resource.location
and os.path.isdir(os.path.join(resource.location, ".git"))
):
_cleanup_stale_sandbox(
resource.location,
plan_id,
)
sandbox = GitWorktreeSandbox(
resource_id=resource.resource_id,
original_path=resource.location,
@@ -163,6 +163,101 @@ class GitWorktreeSandbox:
"""Context after creation, ``None`` before ``create``."""
return self._context
# -- class helpers -------------------------------------------------------
@classmethod
def cleanup_stale(cls, repo_path: str, plan_id: str) -> bool:
"""Remove a stale worktree branch left by a previous execution.
If a previous ``create()`` left behind a branch
``cleveragents/plan-<plan_id>``, this method removes the
worktree directory and deletes the branch so that a fresh
sandbox can be created.
Idempotent does nothing if no stale branch exists.
Args:
repo_path: Absolute path to the git repository root.
plan_id: The plan ULID whose stale branch should be removed.
Returns:
``True`` if a stale branch was found and cleaned up,
``False`` if no stale branch existed.
"""
branch_name = f"cleveragents/plan-{plan_id}"
# Check if the branch exists
try:
_run_git(
["rev-parse", "--verify", f"refs/heads/{branch_name}"],
cwd=repo_path,
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired):
return False # No stale branch — nothing to clean
logger.info(
"Cleaning up stale sandbox branch: branch=%s repo=%s",
branch_name,
repo_path,
)
# Find and remove the worktree directory
try:
wt_result = _run_git(
["worktree", "list", "--porcelain"],
cwd=repo_path,
)
for wt_block in wt_result.stdout.split("\n\n"):
if f"branch refs/heads/{branch_name}" in wt_block:
for line in wt_block.splitlines():
if line.startswith("worktree "):
wt_path = line.split("worktree ", 1)[1]
try:
_run_git(
["worktree", "remove", "--force", wt_path],
cwd=repo_path,
)
except (
subprocess.CalledProcessError,
subprocess.TimeoutExpired,
):
logger.warning(
"git worktree remove failed; "
"removing directory manually: %s",
wt_path,
)
shutil.rmtree(wt_path, ignore_errors=True)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired):
logger.warning(
"Failed to list worktrees for stale cleanup: %s",
branch_name,
)
# Delete the branch
try:
_run_git(
["branch", "-D", branch_name],
cwd=repo_path,
)
except (subprocess.CalledProcessError, subprocess.TimeoutExpired):
logger.warning(
"Failed to delete stale branch %s; it may need manual cleanup",
branch_name,
)
# Prune stale worktree references
with contextlib.suppress(
subprocess.CalledProcessError,
subprocess.TimeoutExpired,
):
_run_git(["worktree", "prune"], cwd=repo_path)
logger.info(
"Stale sandbox branch cleaned up: branch=%s",
branch_name,
)
return True
# -- protocol methods ----------------------------------------------------
def create(self, plan_id: str) -> SandboxContext: