cleveragents/cleveragents-core

Fork 3

UAT: CheckpointService.rollback_to_checkpoint() uses plan.sandbox_refs[0] as filesystem path but ExecuteStubActor stores non-path strings there #4883

New issue

Open

opened 2026-04-08 20:13:58 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-08 20:13:58 +00:00

Owner

Bug Report

Feature Area: Sandbox and Checkpoint — rollback on failure
Severity: Medium — rollback always fails silently when using the stub executor, leaving plans in an inconsistent state after errors

What Was Tested

Code-level analysis of the rollback path:

src/cleveragents/application/services/checkpoint_service.py — rollback_to_checkpoint(), _resolve_sandbox_path()
src/cleveragents/application/services/plan_executor.py — ExecuteStubActor.execute(), _run_execute_with_stub()

Expected Behavior (from spec)

When a plan execution fails, the checkpoint service should roll back the sandbox to the last checkpoint. The _resolve_sandbox_path() method should return a valid filesystem path to the sandbox directory.

Actual Behavior (from code)

CheckpointService._resolve_sandbox_path() (checkpoint_service.py:718) resolves the sandbox path from the plan's sandbox_refs list:

if plan.processing_state == ProcessingState.APPLIED:
    raise BusinessRuleViolation("Cannot rollback: plan is already applied")
if not plan.sandbox_refs:
    raise BusinessRuleViolation(
        "Cannot rollback: sandbox is missing for this plan"
    )
return str(plan.sandbox_refs[0])  # ← used as filesystem path

ExecuteStubActor.execute() (plan_executor.py:208) populates sandbox_refs with the sandbox_root string passed in:

if sandbox_root is not None:
    sandbox_refs.append(sandbox_root)

When sandbox_root is None (the default in most test and production configurations), sandbox_refs is empty — so _resolve_sandbox_path() raises BusinessRuleViolation("Cannot rollback: sandbox is missing").

When sandbox_root IS provided, it is a filesystem path — but _validate_sandbox() then checks:

git_dir = path / ".git"
if not git_dir.exists():
    raise BusinessRuleViolation(
        f"Cannot rollback: sandbox is not a git repository: {sandbox_path}"
    )

A copy_on_write or overlay sandbox root is not a git repository, so this check fails for non-git sandbox strategies.

Impact

Stub executor with no sandbox_root: All rollback attempts raise BusinessRuleViolation("sandbox is missing") — rollback silently fails (caught by _try_rollback_to_last_checkpoint())
Non-git sandbox strategies: Rollback fails with "not a git repository" even for valid sandbox paths
The _validate_sandbox() check is git-specific but is applied to all sandbox strategies

Code Location

CheckpointService._resolve_sandbox_path() — src/cleveragents/application/services/checkpoint_service.py:718
CheckpointService._validate_sandbox() — src/cleveragents/application/services/checkpoint_service.py:757
ExecuteStubActor.execute() — src/cleveragents/application/services/plan_executor.py:208
PlanExecutor._try_rollback_to_last_checkpoint() — src/cleveragents/application/services/plan_executor.py:624

Steps to Reproduce

Create a plan with a copy_on_write sandbox strategy
Execute the plan — execution fails partway through
Observe: _try_rollback_to_last_checkpoint() is called but silently returns False
The sandbox is left in a partially-modified state with no rollback

Fix Direction

_validate_sandbox() should be strategy-aware: only check for .git when the sandbox strategy is git_worktree
CheckpointService should store the sandbox strategy alongside the sandbox path so it can apply the correct validation
ExecuteStubActor should always populate sandbox_refs when a sandbox is active, even for non-git strategies

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Bug Report **Feature Area:** Sandbox and Checkpoint — rollback on failure **Severity:** Medium — rollback always fails silently when using the stub executor, leaving plans in an inconsistent state after errors --- ## What Was Tested Code-level analysis of the rollback path: - `src/cleveragents/application/services/checkpoint_service.py` — `rollback_to_checkpoint()`, `_resolve_sandbox_path()` - `src/cleveragents/application/services/plan_executor.py` — `ExecuteStubActor.execute()`, `_run_execute_with_stub()` ## Expected Behavior (from spec) When a plan execution fails, the checkpoint service should roll back the sandbox to the last checkpoint. The `_resolve_sandbox_path()` method should return a valid filesystem path to the sandbox directory. ## Actual Behavior (from code) **`CheckpointService._resolve_sandbox_path()` (checkpoint_service.py:718)** resolves the sandbox path from the plan's `sandbox_refs` list: ```python if plan.processing_state == ProcessingState.APPLIED: raise BusinessRuleViolation("Cannot rollback: plan is already applied") if not plan.sandbox_refs: raise BusinessRuleViolation( "Cannot rollback: sandbox is missing for this plan" ) return str(plan.sandbox_refs[0]) # ← used as filesystem path ``` **`ExecuteStubActor.execute()` (plan_executor.py:208)** populates `sandbox_refs` with the `sandbox_root` string passed in: ```python if sandbox_root is not None: sandbox_refs.append(sandbox_root) ``` When `sandbox_root` is `None` (the default in most test and production configurations), `sandbox_refs` is empty — so `_resolve_sandbox_path()` raises `BusinessRuleViolation("Cannot rollback: sandbox is missing")`. When `sandbox_root` IS provided, it is a filesystem path — but `_validate_sandbox()` then checks: ```python git_dir = path / ".git" if not git_dir.exists(): raise BusinessRuleViolation( f"Cannot rollback: sandbox is not a git repository: {sandbox_path}" ) ``` A `copy_on_write` or `overlay` sandbox root is not a git repository, so this check fails for non-git sandbox strategies. ## Impact 1. **Stub executor with no sandbox_root**: All rollback attempts raise `BusinessRuleViolation("sandbox is missing")` — rollback silently fails (caught by `_try_rollback_to_last_checkpoint()`) 2. **Non-git sandbox strategies**: Rollback fails with "not a git repository" even for valid sandbox paths 3. **The `_validate_sandbox()` check is git-specific** but is applied to all sandbox strategies ## Code Location - `CheckpointService._resolve_sandbox_path()` — `src/cleveragents/application/services/checkpoint_service.py:718` - `CheckpointService._validate_sandbox()` — `src/cleveragents/application/services/checkpoint_service.py:757` - `ExecuteStubActor.execute()` — `src/cleveragents/application/services/plan_executor.py:208` - `PlanExecutor._try_rollback_to_last_checkpoint()` — `src/cleveragents/application/services/plan_executor.py:624` ## Steps to Reproduce 1. Create a plan with a `copy_on_write` sandbox strategy 2. Execute the plan — execution fails partway through 3. Observe: `_try_rollback_to_last_checkpoint()` is called but silently returns `False` 4. The sandbox is left in a partially-modified state with no rollback ## Fix Direction 1. `_validate_sandbox()` should be strategy-aware: only check for `.git` when the sandbox strategy is `git_worktree` 2. `CheckpointService` should store the sandbox strategy alongside the sandbox path so it can apply the correct validation 3. `ExecuteStubActor` should always populate `sandbox_refs` when a sandbox is active, even for non-git strategies --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 referenced this issue

2026-04-08 20:14:46 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799

HAL9000 referenced this issue

2026-04-08 20:16:04 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799

HAL9000 referenced this issue

2026-04-08 20:22:25 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799

HAL9000 added the

labels

2026-04-08 22:59:24 +00:00

HAL9000 commented

2026-04-09 03:02:38 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Medium — spec compliance bug identified by UAT testing
Story Points: 3 (M) — targeted fix to align implementation with spec
MoSCoW: Must Have — spec compliance is required for correct system behavior

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Medium — spec compliance bug identified by UAT testing - **Story Points**: 3 (M) — targeted fix to align implementation with spec - **MoSCoW**: Must Have — spec compliance is required for correct system behavior --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner

HAL9000 added

and removed

labels

2026-04-09 03:02:44 +00:00

HAL9000 added this to the v3.5.0 milestone