cleveragents/cleveragents-core

Fork 3

UAT: CheckpointManager.rollback_to() silently skips rollback when sandbox_path is absent from checkpoint metadata — NoSandbox checkpoints are unrestorable #4885

New issue

Open

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

HAL9000 commented

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

Owner

Bug Report

Feature Area: Sandbox and Checkpoint — checkpoint rollback
Severity: Medium — rollback silently no-ops for NoSandbox checkpoints, giving false confidence that rollback succeeded

What Was Tested

Code-level analysis of:

src/cleveragents/infrastructure/sandbox/checkpoint.py — CheckpointManager.rollback_to()
src/cleveragents/application/services/plan_executor.py — PlanExecutor._try_create_checkpoint()

Expected Behavior (from spec)

When CheckpointManager.rollback_to(checkpoint) is called, it should restore the sandbox to the captured state. If rollback is not possible (e.g., no physical sandbox), it should raise an error or return a clear failure indicator.

Actual Behavior (from code)

CheckpointManager.rollback_to() (checkpoint.py:176) requires sandbox_path in the checkpoint's metadata dict:

sandbox_path = checkpoint.metadata.get("sandbox_path")
if not sandbox_path or not os.path.isdir(sandbox_path):
    self._logger.warning(
        "Rollback skipped: sandbox path unknown or missing",
        checkpoint_id=checkpoint.checkpoint_id,
    )
    return False  # ← silently returns False

PlanExecutor._try_create_checkpoint() (plan_executor.py:579) only stores sandbox_path in metadata when sandbox.context is not None:

meta = dict(metadata or {})
if sandbox.context is not None:
    meta["sandbox_path"] = sandbox.context.sandbox_path
return self._checkpoint_manager.create_checkpoint(
    sandbox=sandbox,
    plan_id=plan_id,
    phase=phase,
    metadata=meta,
)

NoSandbox.context always returns None (it has no physical path). Therefore:

Checkpoints created for NoSandbox plans never have sandbox_path in metadata
rollback_to() silently returns False for all such checkpoints
PlanExecutor._try_rollback_to_last_checkpoint() logs nothing and returns False
The caller (error handler in _run_execute_with_stub) proceeds as if rollback succeeded

Impact

For plans using NoSandbox (resources with sandbox_strategy=none):

Checkpoints are created (metadata-only snapshots)
On failure, rollback is attempted
Rollback silently fails with return False
The caller receives False but does not distinguish "rollback not applicable" from "rollback failed"
No error is surfaced to the user

This is a silent failure that violates the principle of least surprise. The spec requires that rollback failures be surfaced.

Code Location

CheckpointManager.rollback_to() — src/cleveragents/infrastructure/sandbox/checkpoint.py:176
PlanExecutor._try_create_checkpoint() — src/cleveragents/application/services/plan_executor.py:579
PlanExecutor._try_rollback_to_last_checkpoint() — src/cleveragents/application/services/plan_executor.py:624
NoSandbox — src/cleveragents/infrastructure/sandbox/no_sandbox.py

Steps to Reproduce

Register a resource with sandbox_strategy=none (uses NoSandbox)
Configure CheckpointManager on PlanExecutor
Execute a plan that fails partway through
Observe: _try_rollback_to_last_checkpoint() returns False with no error logged
The plan error handler proceeds as if rollback was attempted successfully

Fix Direction

Two options:

Option A (preferred): Distinguish "rollback not applicable" from "rollback failed":

Add a RollbackOutcome enum: SUCCEEDED, SKIPPED_NO_PATH, FAILED
Return SKIPPED_NO_PATH for metadata-only checkpoints (not an error)
Return FAILED for actual rollback failures (should be logged as error)

Option B: Log a clear warning when sandbox_path is missing:

Change the return False to log at WARNING level: "Rollback not possible: checkpoint has no physical sandbox path (NoSandbox strategy)"
This makes the silent skip visible in logs

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

## Bug Report **Feature Area:** Sandbox and Checkpoint — checkpoint rollback **Severity:** Medium — rollback silently no-ops for `NoSandbox` checkpoints, giving false confidence that rollback succeeded --- ## What Was Tested Code-level analysis of: - `src/cleveragents/infrastructure/sandbox/checkpoint.py` — `CheckpointManager.rollback_to()` - `src/cleveragents/application/services/plan_executor.py` — `PlanExecutor._try_create_checkpoint()` ## Expected Behavior (from spec) When `CheckpointManager.rollback_to(checkpoint)` is called, it should restore the sandbox to the captured state. If rollback is not possible (e.g., no physical sandbox), it should raise an error or return a clear failure indicator. ## Actual Behavior (from code) **`CheckpointManager.rollback_to()` (checkpoint.py:176)** requires `sandbox_path` in the checkpoint's metadata dict: ```python sandbox_path = checkpoint.metadata.get("sandbox_path") if not sandbox_path or not os.path.isdir(sandbox_path): self._logger.warning( "Rollback skipped: sandbox path unknown or missing", checkpoint_id=checkpoint.checkpoint_id, ) return False # ← silently returns False ``` **`PlanExecutor._try_create_checkpoint()` (plan_executor.py:579)** only stores `sandbox_path` in metadata when `sandbox.context is not None`: ```python meta = dict(metadata or {}) if sandbox.context is not None: meta["sandbox_path"] = sandbox.context.sandbox_path return self._checkpoint_manager.create_checkpoint( sandbox=sandbox, plan_id=plan_id, phase=phase, metadata=meta, ) ``` **`NoSandbox.context`** always returns `None` (it has no physical path). Therefore: 1. Checkpoints created for `NoSandbox` plans never have `sandbox_path` in metadata 2. `rollback_to()` silently returns `False` for all such checkpoints 3. `PlanExecutor._try_rollback_to_last_checkpoint()` logs nothing and returns `False` 4. The caller (error handler in `_run_execute_with_stub`) proceeds as if rollback succeeded ## Impact For plans using `NoSandbox` (resources with `sandbox_strategy=none`): - Checkpoints are created (metadata-only snapshots) - On failure, rollback is attempted - Rollback silently fails with `return False` - The caller receives `False` but does not distinguish "rollback not applicable" from "rollback failed" - No error is surfaced to the user This is a silent failure that violates the principle of least surprise. The spec requires that rollback failures be surfaced. ## Code Location - `CheckpointManager.rollback_to()` — `src/cleveragents/infrastructure/sandbox/checkpoint.py:176` - `PlanExecutor._try_create_checkpoint()` — `src/cleveragents/application/services/plan_executor.py:579` - `PlanExecutor._try_rollback_to_last_checkpoint()` — `src/cleveragents/application/services/plan_executor.py:624` - `NoSandbox` — `src/cleveragents/infrastructure/sandbox/no_sandbox.py` ## Steps to Reproduce 1. Register a resource with `sandbox_strategy=none` (uses `NoSandbox`) 2. Configure `CheckpointManager` on `PlanExecutor` 3. Execute a plan that fails partway through 4. Observe: `_try_rollback_to_last_checkpoint()` returns `False` with no error logged 5. The plan error handler proceeds as if rollback was attempted successfully ## Fix Direction Two options: **Option A (preferred)**: Distinguish "rollback not applicable" from "rollback failed": - Add a `RollbackOutcome` enum: `SUCCEEDED`, `SKIPPED_NO_PATH`, `FAILED` - Return `SKIPPED_NO_PATH` for metadata-only checkpoints (not an error) - Return `FAILED` for actual rollback failures (should be logged as error) **Option B**: Log a clear warning when `sandbox_path` is missing: - Change the `return False` to log at `WARNING` level: "Rollback not possible: checkpoint has no physical sandbox path (NoSandbox strategy)" - This makes the silent skip visible in logs --- **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:14:51 +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

freemo removed the

State

Unverified

label

2026-04-08 23:43:11 +00:00

HAL9000 added the

State

Unverified

label

2026-04-09 00:29:20 +00:00

HAL9000 commented

2026-04-09 03:02:37 +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:43 +00:00

HAL9000 added this to the v3.5.0 milestone