cleveragents/cleveragents-core

Fork 3

UAT: GitWorktreeSandbox.get_path() allows path resolution in ROLLED_BACK status and silently transitions to ACTIVE — enables writes after rollback #5473

New issue

Open

opened 2026-04-09 06:56:46 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 06:56:46 +00:00

Owner

Bug Report

Feature Area: git-worktree-sandbox — sandbox path management
Severity: Priority/Backlog — potential data integrity issue; rolled-back sandbox can be silently reactivated

What Was Tested

Code-level analysis of GitWorktreeSandbox.get_path() in src/cleveragents/infrastructure/sandbox/git_worktree.py against the spec's sandbox lifecycle model.

Expected Behavior (from spec)

Per the SandboxStatus transition graph in src/cleveragents/infrastructure/sandbox/protocol.py (lines 76–84):

ROLLED_BACK ──► ACTIVE  (re-use after rollback is allowed)
ROLLED_BACK ──► CLEANED_UP

The spec allows re-use after rollback. However, the transition back to ACTIVE should be explicit — the caller should know they are re-using a rolled-back sandbox.

Actual Behavior

GitWorktreeSandbox.get_path() (lines 303–321) silently transitions the sandbox from ROLLED_BACK to ACTIVE when a path is requested:

def get_path(self, resource_path: str) -> str:
    if self._status not in (
        SandboxStatus.CREATED,
        SandboxStatus.ACTIVE,
        SandboxStatus.ROLLED_BACK,  # ← ROLLED_BACK is allowed
    ):
        raise SandboxStateError(...)

    if ".." in resource_path.split("/"):
        raise ValueError(...)

    if self._status in (SandboxStatus.CREATED, SandboxStatus.ROLLED_BACK):
        self._status = SandboxStatus.ACTIVE  # ← Silent state transition!
    ...

Problems:

Silent state transition: Calling get_path() on a ROLLED_BACK sandbox silently transitions it to ACTIVE without any explicit re-activation call. This means any code that calls get_path() after a rollback (e.g., for inspection purposes) will inadvertently re-activate the sandbox.
No re-initialization after rollback: After rollback(), the worktree is reset to base_commit (via git reset --hard). But get_path() doesn't verify that the worktree is in a clean state before re-activating. If the rollback partially failed, the sandbox may be in an inconsistent state.
Inconsistency with SandboxStatus.assert_transition(): The commit() method calls SandboxStatus.assert_transition(self._status, SandboxStatus.COMMITTED) before proceeding. But get_path() performs the ROLLED_BACK → ACTIVE transition without calling assert_transition(), bypassing the validation.

Code Location

Bug: src/cleveragents/infrastructure/sandbox/git_worktree.py, get_path() (lines 303–321)
Protocol definition: src/cleveragents/infrastructure/sandbox/protocol.py, SandboxStatus.valid_transitions() (line 104)

Impact

Code that calls get_path() for read-only inspection after a rollback will inadvertently re-activate the sandbox
The sandbox can be written to after a rollback without any explicit re-activation, which may be unexpected
If a rollback partially failed (worktree in inconsistent state), the sandbox can be re-activated and written to, potentially corrupting the worktree

Fix Required

Option 1: Add an explicit reactivate() method that transitions ROLLED_BACK → ACTIVE with proper validation, and remove the silent transition from get_path().

Option 2: Add a read-only get_path() mode that does not transition state, and only allow writes via a separate get_write_path() method that validates the state.

Option 3: Document the behavior clearly in the docstring and add a SandboxStatus.assert_transition() call in get_path() when transitioning from ROLLED_BACK.

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

## Bug Report **Feature Area**: git-worktree-sandbox — sandbox path management **Severity**: Priority/Backlog — potential data integrity issue; rolled-back sandbox can be silently reactivated ## What Was Tested Code-level analysis of `GitWorktreeSandbox.get_path()` in `src/cleveragents/infrastructure/sandbox/git_worktree.py` against the spec's sandbox lifecycle model. ## Expected Behavior (from spec) Per the `SandboxStatus` transition graph in `src/cleveragents/infrastructure/sandbox/protocol.py` (lines 76–84): ``` ROLLED_BACK ──► ACTIVE (re-use after rollback is allowed) ROLLED_BACK ──► CLEANED_UP ``` The spec allows re-use after rollback. However, the transition back to `ACTIVE` should be **explicit** — the caller should know they are re-using a rolled-back sandbox. ## Actual Behavior `GitWorktreeSandbox.get_path()` (lines 303–321) silently transitions the sandbox from `ROLLED_BACK` to `ACTIVE` when a path is requested: ```python def get_path(self, resource_path: str) -> str: if self._status not in ( SandboxStatus.CREATED, SandboxStatus.ACTIVE, SandboxStatus.ROLLED_BACK, # ← ROLLED_BACK is allowed ): raise SandboxStateError(...) if ".." in resource_path.split("/"): raise ValueError(...) if self._status in (SandboxStatus.CREATED, SandboxStatus.ROLLED_BACK): self._status = SandboxStatus.ACTIVE # ← Silent state transition! ... ``` **Problems:** 1. **Silent state transition**: Calling `get_path()` on a `ROLLED_BACK` sandbox silently transitions it to `ACTIVE` without any explicit re-activation call. This means any code that calls `get_path()` after a rollback (e.g., for inspection purposes) will inadvertently re-activate the sandbox. 2. **No re-initialization after rollback**: After `rollback()`, the worktree is reset to `base_commit` (via `git reset --hard`). But `get_path()` doesn't verify that the worktree is in a clean state before re-activating. If the rollback partially failed, the sandbox may be in an inconsistent state. 3. **Inconsistency with `SandboxStatus.assert_transition()`**: The `commit()` method calls `SandboxStatus.assert_transition(self._status, SandboxStatus.COMMITTED)` before proceeding. But `get_path()` performs the `ROLLED_BACK → ACTIVE` transition without calling `assert_transition()`, bypassing the validation. ## Code Location - **Bug**: `src/cleveragents/infrastructure/sandbox/git_worktree.py`, `get_path()` (lines 303–321) - **Protocol definition**: `src/cleveragents/infrastructure/sandbox/protocol.py`, `SandboxStatus.valid_transitions()` (line 104) ## Impact - Code that calls `get_path()` for read-only inspection after a rollback will inadvertently re-activate the sandbox - The sandbox can be written to after a rollback without any explicit re-activation, which may be unexpected - If a rollback partially failed (worktree in inconsistent state), the sandbox can be re-activated and written to, potentially corrupting the worktree ## Fix Required Option 1: **Add an explicit `reactivate()` method** that transitions `ROLLED_BACK → ACTIVE` with proper validation, and remove the silent transition from `get_path()`. Option 2: **Add a read-only `get_path()` mode** that does not transition state, and only allow writes via a separate `get_write_path()` method that validates the state. Option 3: **Document the behavior** clearly in the docstring and add a `SandboxStatus.assert_transition()` call in `get_path()` when transitioning from `ROLLED_BACK`. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 06:57:08 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 06:59:18 +00:00

HAL9000 added

and removed

labels

2026-04-09 07:00:16 +00:00

HAL9000 commented

2026-04-09 07:00:43 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

Label compliance fix applied: - Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 added a new dependency

2026-04-09 07:00:45 +00:00

#4973 EPIC: Checkpoint & Rollback System — Plan State Persistence

HAL9000 referenced this issue

2026-04-09 07:01:00 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 26) #5296

HAL9000 referenced this issue

2026-04-09 07:13:39 +00:00

[AUTO-EPIC] Epic Planning Health Report (Cycle 11) #5298

HAL9000 referenced this issue

2026-04-09 08:20:03 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 1) #5369