fix(plan): only cleanup worktree sandbox on execute failure, not success #10872

New issue

Closed

opened 2026-04-27 14:29:31 +00:00 by hamza.khyari · 0 comments

hamza.khyari commented

2026-04-27 14:29:31 +00:00

Member

Summary

The finally block in execute_plan() unconditionally calls sandbox_obj.cleanup() which deletes the worktree branch (cleveragents/plan-<id>). When plan apply runs, the branch is gone — no merge happens. The plan reaches applied state but the LLM's file changes are silently discarded.

Metadata

Commit Message: fix(plan): only cleanup worktree sandbox on execute failure, not success
Branch: bugfix/sandbox-cleanup-on-failure-only

Root Cause

The finally block at plan.py:2973 was added to fix M4 (cleanup sandboxes on execute failure) but runs unconditionally — on both success AND failure. On success, the worktree branch must survive until plan apply merges it.

finally:
    for _sinfo in sandbox_infos:
        _sinfo.sandbox_obj.cleanup()  # Deletes branch + worktree

After this runs, _apply_sandbox_changes() can't find the branch (git rev-parse --verify refs/heads/cleveragents/plan-<id> fails) and falls through to the flat copy path, which also finds nothing.

Impact

All scenarios with large projects (30+ files) where the LLM generates valid output but the apply silently discards it
The plan reaches applied terminal state with NO file changes
No error message — completely silent data loss

Fix

Replace the unconditional finally cleanup with a conditional check: only cleanup when the plan is NOT in execute/complete state.

finally:
    execute_succeeded = (
        final_plan is not None
        and final_plan.phase.value == "execute"
        and final_plan.state.value == "complete"
    )
    if not execute_succeeded:
        for _sinfo in sandbox_infos:
            _sinfo.sandbox_obj.cleanup()

The plan apply command handles cleanup after merge (removes worktree + deletes branch in the Sandbox Cleanup panel).

Subtasks

Replace unconditional finally cleanup with conditional check on plan state
Verify scenario-4 (large project) applies file changes
Verify M1 E2E passes (no regression)
Remove temp debug code from llm_actors.py and plan_executor.py

Definition of Done

Worktree branch survives after successful execute
plan apply merges the LLM's file changes
Worktree is cleaned up on execute failure
All 15 scenarios pass
M1 E2E passes

Spec Reference

spec: Sandbox Cleanup — worktree removed after apply, not after execute

## Summary The `finally` block in `execute_plan()` unconditionally calls `sandbox_obj.cleanup()` which deletes the worktree branch (`cleveragents/plan-<id>`). When `plan apply` runs, the branch is gone — no merge happens. The plan reaches `applied` state but the LLM's file changes are silently discarded. ## Metadata - **Commit Message**: `fix(plan): only cleanup worktree sandbox on execute failure, not success` - **Branch**: `bugfix/sandbox-cleanup-on-failure-only` ## Root Cause The `finally` block at `plan.py:2973` was added to fix M4 (cleanup sandboxes on execute failure) but runs unconditionally — on both success AND failure. On success, the worktree branch must survive until `plan apply` merges it. ```python finally: for _sinfo in sandbox_infos: _sinfo.sandbox_obj.cleanup() # Deletes branch + worktree ``` After this runs, `_apply_sandbox_changes()` can't find the branch (`git rev-parse --verify refs/heads/cleveragents/plan-<id>` fails) and falls through to the flat copy path, which also finds nothing. ## Impact - All scenarios with large projects (30+ files) where the LLM generates valid output but the apply silently discards it - The plan reaches `applied` terminal state with NO file changes - No error message — completely silent data loss ## Fix Replace the unconditional `finally` cleanup with a conditional check: only cleanup when the plan is NOT in `execute/complete` state. ```python finally: execute_succeeded = ( final_plan is not None and final_plan.phase.value == "execute" and final_plan.state.value == "complete" ) if not execute_succeeded: for _sinfo in sandbox_infos: _sinfo.sandbox_obj.cleanup() ``` The `plan apply` command handles cleanup after merge (removes worktree + deletes branch in the Sandbox Cleanup panel). ## Subtasks - [ ] Replace unconditional `finally` cleanup with conditional check on plan state - [ ] Verify scenario-4 (large project) applies file changes - [ ] Verify M1 E2E passes (no regression) - [ ] Remove temp debug code from llm_actors.py and plan_executor.py ## Definition of Done - Worktree branch survives after successful execute - `plan apply` merges the LLM's file changes - Worktree is cleaned up on execute failure - All 15 scenarios pass - M1 E2E passes ## Spec Reference - spec: Sandbox Cleanup — worktree removed after apply, not after execute

Rows
Columns