bug(plan): checkpoint rollback is simulated — does not execute real git reset #822

Closed
opened 2026-03-13 20:12:30 +00:00 by freemo · 5 comments
Owner

Metadata

  • Commit Message: fix(plan): implement real checkpoint rollback via git reset
  • Branch: bugfix/m4-checkpoint-real-rollback
  • Type: Bug
  • Priority: High
  • MoSCoW: Must have
  • Points: 5
  • Milestone: v3.5.0

Background and Context

The specification defines checkpoint rollback as a mechanism to revert the working tree to a prior checkpoint state using git reset. PR #462 (feat(sandbox): add checkpoint and rollback hooks) was merged as complete.

However, a spec-vs-code audit found that CheckpointService.rollback() in src/cleveragents/application/services/checkpoint_service.py is simulated: it returns a successful RollbackResult but does not actually execute git reset --hard (or equivalent) against the repository. The method constructs a result object with the checkpoint metadata but skips the actual filesystem/git operation.

This means:

  • plan rollback CLI command appears to succeed but leaves the working tree unchanged
  • No files are actually reverted to the checkpoint state
  • The rollback is purely a metadata operation with no side effects

Acceptance Criteria

  • CheckpointService.rollback(checkpoint_id) executes a real git reset --hard <checkpoint_commit> (or git checkout + git clean equivalent) against the project repository
  • File system state after rollback matches the checkpoint state exactly
  • Rollback respects sandbox boundaries (only resets within the plan's sandbox scope)
  • Rollback emits appropriate domain events (CheckpointRolledBack)
  • Failed rollback (dirty state, conflicts) produces meaningful error with recovery guidance
  • plan rollback CLI command produces observable file changes

Definition of Done

This issue is complete when:

  • All subtasks below are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Subtasks

  • Replace simulated rollback with real git reset --hard execution in CheckpointService.rollback()
  • Add sandbox boundary enforcement (only reset files within sandbox scope)
  • Add domain event emission for rollback operations
  • Add error handling for dirty state / conflict scenarios
  • Tests (Behave): Add scenarios verifying actual file reversion after rollback
  • Tests (Robot): Add integration test creating files, checkpointing, modifying, rolling back, and verifying original state
  • Verify coverage >=97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors
## Metadata - **Commit Message**: `fix(plan): implement real checkpoint rollback via git reset` - **Branch**: `bugfix/m4-checkpoint-real-rollback` - **Type**: Bug - **Priority**: High - **MoSCoW**: Must have - **Points**: 5 - **Milestone**: v3.5.0 ## Background and Context The specification defines checkpoint rollback as a mechanism to revert the working tree to a prior checkpoint state using `git reset`. PR #462 (`feat(sandbox): add checkpoint and rollback hooks`) was merged as complete. However, a spec-vs-code audit found that `CheckpointService.rollback()` in `src/cleveragents/application/services/checkpoint_service.py` is **simulated**: it returns a successful `RollbackResult` but does not actually execute `git reset --hard` (or equivalent) against the repository. The method constructs a result object with the checkpoint metadata but skips the actual filesystem/git operation. This means: - `plan rollback` CLI command appears to succeed but leaves the working tree unchanged - No files are actually reverted to the checkpoint state - The rollback is purely a metadata operation with no side effects ## Acceptance Criteria - [x] `CheckpointService.rollback(checkpoint_id)` executes a real `git reset --hard <checkpoint_commit>` (or `git checkout` + `git clean` equivalent) against the project repository - [x] File system state after rollback matches the checkpoint state exactly - [x] Rollback respects sandbox boundaries (only resets within the plan's sandbox scope) - [x] Rollback emits appropriate domain events (CheckpointRolledBack) - [x] Failed rollback (dirty state, conflicts) produces meaningful error with recovery guidance - [x] `plan rollback` CLI command produces observable file changes ## Definition of Done This issue is complete when: - All subtasks below are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. ## Subtasks - [x] Replace simulated rollback with real `git reset --hard` execution in `CheckpointService.rollback()` - [x] Add sandbox boundary enforcement (only reset files within sandbox scope) - [x] Add domain event emission for rollback operations - [x] Add error handling for dirty state / conflict scenarios - [x] Tests (Behave): Add scenarios verifying actual file reversion after rollback - [x] Tests (Robot): Add integration test creating files, checkpointing, modifying, rolling back, and verifying original state - [x] Verify coverage >=97% via `nox -s coverage_report` - [x] Run `nox` (all default sessions), fix any errors
freemo added this to the v3.5.0 milestone 2026-03-13 20:19:57 +00:00
freemo self-assigned this 2026-03-13 21:31:14 +00:00
freemo modified the milestone from v3.5.0 to v3.3.0 2026-03-13 22:05:23 +00:00
Author
Owner

Dependency (TDD workflow): This bug fix is blocked by TDD issue #839. Per CONTRIBUTING.md Bug Fix Workflow, the TDD test capturing the bug must be written and merged first (@tdd_expected_fail), then the fix PR removes the tag.

Blocked by: #839

**Dependency (TDD workflow):** This bug fix is blocked by TDD issue #839. Per `CONTRIBUTING.md` Bug Fix Workflow, the TDD test capturing the bug must be written and merged first (`@tdd_expected_fail`), then the fix PR removes the tag. **Blocked by:** #839
Author
Owner

PM Status — Day 36

TDD in final review. PR #929 (TDD tests for this bug) is in review. @hamza.khyari was requested for fast-track peer review on Day 34.

TDD Pipeline Status:

Stage Item Status
1. TDD Issue #839 Open, State/In Review
2. TDD PR #929 Open, mergeable, awaiting peer review
3. Bug fix PR Cannot start until TDD merges

Action items:

  • @hamza.khyari — Please complete the fast-track review of PR #929. This is 2 days past the review request. The TDD tests are solid (4 comments of thorough implementation notes, all CI passing).
  • @freemo — Once PR #929 merges and #839 is closed, proceed with the bug fix on branch bugfix/m4-checkpoint-real-rollback.

Priority: Critical. Checkpoint rollback is a core safety mechanism.


PM status comment — Day 36

## PM Status — Day 36 **TDD in final review.** PR #929 (TDD tests for this bug) is in review. @hamza.khyari was requested for fast-track peer review on Day 34. **TDD Pipeline Status:** | Stage | Item | Status | |-------|------|--------| | 1. TDD Issue | #839 | Open, State/In Review | | 2. TDD PR | #929 | Open, mergeable, awaiting peer review | | 3. Bug fix PR | — | Cannot start until TDD merges | **Action items:** - @hamza.khyari — Please complete the fast-track review of PR #929. This is 2 days past the review request. The TDD tests are solid (4 comments of thorough implementation notes, all CI passing). - @freemo — Once PR #929 merges and #839 is closed, proceed with the bug fix on branch `bugfix/m4-checkpoint-real-rollback`. **Priority**: Critical. Checkpoint rollback is a core safety mechanism. --- *PM status comment — Day 36*
Author
Owner

PM Status — Day 36

TDD in final review. PR #929 awaiting peer review from @hamza.khyari (requested Day 34).

TDD Pipeline:

Stage Item Status
TDD Issue #839 Open, In Review
TDD PR #929 Open, mergeable, awaiting review
Bug fix Blocked until TDD merges

@hamza.khyari — Please complete fast-track review of PR #929. 2 days past request.
@freemo — Once PR #929 merges, proceed with fix on bugfix/m4-checkpoint-real-rollback.


PM status comment — Day 36

## PM Status — Day 36 **TDD in final review.** PR #929 awaiting peer review from @hamza.khyari (requested Day 34). **TDD Pipeline:** | Stage | Item | Status | |-------|------|--------| | TDD Issue | #839 | Open, In Review | | TDD PR | #929 | Open, mergeable, awaiting review | | Bug fix | — | Blocked until TDD merges | @hamza.khyari — Please complete fast-track review of PR #929. 2 days past request. @freemo — Once PR #929 merges, proceed with fix on `bugfix/m4-checkpoint-real-rollback`. --- *PM status comment — Day 36*
Author
Owner

PM Escalation — Day 37

STALLED TDD PIPELINE — FIX BRANCH NOT STARTED

The TDD counterpart for this bug (issue #839) was closed on Day 36 when PR #929 was merged to master. The @tdd_expected_fail tests proving the bug exists are now on master. Per the TDD workflow in CONTRIBUTING.md, the fix can and should have started immediately.

It has been over 24 hours since the TDD test landed on master and no fix branch exists.

@freemo — This is a Priority/Critical, MoSCoW/Must Have bug. Checkpoint rollback is a core safety mechanism. The mandatory next steps are:

  1. Create branch bugfix/m4-checkpoint-real-rollback from master today.
  2. Implement the fix: CheckpointService.rollback_to_checkpoint() must execute git reset --hard to the checkpoint commit.
  3. Remove the @tdd_expected_fail tag from the tests in features/tdd_checkpoint_real_rollback.feature and robot/tdd_checkpoint_real_rollback.robot.
  4. Verify all quality gates pass.
  5. Open PR from bugfix/m4-checkpoint-real-rollback to master.

This bug is blocking M4 closure and contributing to the project's 4-5 day schedule slip. Every day the fix is delayed adds directly to milestone risk.


PM escalation — Day 37

## PM Escalation — Day 37 **STALLED TDD PIPELINE — FIX BRANCH NOT STARTED** The TDD counterpart for this bug (issue #839) was **closed on Day 36** when PR #929 was merged to `master`. The `@tdd_expected_fail` tests proving the bug exists are now on `master`. Per the TDD workflow in `CONTRIBUTING.md`, the fix can and should have started immediately. **It has been over 24 hours since the TDD test landed on `master` and no fix branch exists.** @freemo — This is a **Priority/Critical, MoSCoW/Must Have** bug. Checkpoint rollback is a core safety mechanism. The mandatory next steps are: 1. Create branch `bugfix/m4-checkpoint-real-rollback` from `master` **today**. 2. Implement the fix: `CheckpointService.rollback_to_checkpoint()` must execute `git reset --hard` to the checkpoint commit. 3. Remove the `@tdd_expected_fail` tag from the tests in `features/tdd_checkpoint_real_rollback.feature` and `robot/tdd_checkpoint_real_rollback.robot`. 4. Verify all quality gates pass. 5. Open PR from `bugfix/m4-checkpoint-real-rollback` to `master`. **This bug is blocking M4 closure and contributing to the project's 4-5 day schedule slip.** Every day the fix is delayed adds directly to milestone risk. --- *PM escalation — Day 37*
Author
Owner

Implementation Notes — Day 37

Design Decisions

  1. Git operations via subprocess: Chose subprocess.run(["git", ...], cwd=sandbox_path) over GitPython/dulwich to stay consistent with the existing codebase pattern and avoid additional dependencies. The cwd parameter enforces sandbox boundaries at the OS level.

  2. Sandbox validation: Added _validate_sandbox() as a pre-flight check before git operations. Verifies the path exists as a directory and contains a .git subdirectory. This prevents cryptic git errors if the sandbox path is stale.

  3. Changed paths computation: _git_changed_paths() runs git diff --name-only target_ref HEAD plus git ls-files --others --exclude-standard before the reset, providing accurate file counts and paths for the RollbackResult. This gives callers visibility into what was actually restored.

  4. Domain event emission: Uses the existing EventBus protocol with EventType.CHECKPOINT_RESTORED and the DomainEvent model. Event includes checkpoint_id, sandbox_ref, restored_files_count, and changed_paths. Only emits when event_bus is provided (optional constructor parameter).

  5. Backward compatibility: The _resolve_sandbox_path() method preserves both the lifecycle-service path (for production) and the in-memory fallback path (for unit tests without the lifecycle service).

Key Code Locations

  • CheckpointService.rollback_to_checkpoint()src/cleveragents/application/services/checkpoint_service.py (commit b4e475a7)
  • _git_reset_hard(), _git_clean(), _run_git() — same file, private methods
  • _resolve_sandbox_path(), _validate_sandbox() — same file, extracted guards

Existing Test Fix

The existing robot/helper_checkpoint_rollback.py::_rollback_checkpoint() used a fake sandbox path ("sandbox-123") which worked with the simulated rollback but fails with real git operations. Updated to create a temporary git workspace with a real commit history.

Quality Gates

  • Lint: All checks passed
  • Typecheck: 0 errors (Pyright)
  • Unit tests: 384 features, 10965 scenarios passed
  • Integration tests: 1549 tests passed (0 failed)
  • Coverage: 97%

Branch & PR

  • Branch: bugfix/m4-checkpoint-real-rollback
  • Commit: b4e475a7
  • PR created → transitioning to State/In Review
## Implementation Notes — Day 37 ### Design Decisions 1. **Git operations via subprocess**: Chose `subprocess.run(["git", ...], cwd=sandbox_path)` over GitPython/dulwich to stay consistent with the existing codebase pattern and avoid additional dependencies. The `cwd` parameter enforces sandbox boundaries at the OS level. 2. **Sandbox validation**: Added `_validate_sandbox()` as a pre-flight check before git operations. Verifies the path exists as a directory and contains a `.git` subdirectory. This prevents cryptic git errors if the sandbox path is stale. 3. **Changed paths computation**: `_git_changed_paths()` runs `git diff --name-only target_ref HEAD` plus `git ls-files --others --exclude-standard` before the reset, providing accurate file counts and paths for the `RollbackResult`. This gives callers visibility into what was actually restored. 4. **Domain event emission**: Uses the existing `EventBus` protocol with `EventType.CHECKPOINT_RESTORED` and the `DomainEvent` model. Event includes checkpoint_id, sandbox_ref, restored_files_count, and changed_paths. Only emits when `event_bus` is provided (optional constructor parameter). 5. **Backward compatibility**: The `_resolve_sandbox_path()` method preserves both the lifecycle-service path (for production) and the in-memory fallback path (for unit tests without the lifecycle service). ### Key Code Locations - `CheckpointService.rollback_to_checkpoint()` — `src/cleveragents/application/services/checkpoint_service.py` (commit b4e475a7) - `_git_reset_hard()`, `_git_clean()`, `_run_git()` — same file, private methods - `_resolve_sandbox_path()`, `_validate_sandbox()` — same file, extracted guards ### Existing Test Fix The existing `robot/helper_checkpoint_rollback.py::_rollback_checkpoint()` used a fake sandbox path (`"sandbox-123"`) which worked with the simulated rollback but fails with real git operations. Updated to create a temporary git workspace with a real commit history. ### Quality Gates - Lint: ✅ All checks passed - Typecheck: ✅ 0 errors (Pyright) - Unit tests: ✅ 384 features, 10965 scenarios passed - Integration tests: ✅ 1549 tests passed (0 failed) - Coverage: ✅ 97% ### Branch & PR - Branch: `bugfix/m4-checkpoint-real-rollback` - Commit: `b4e475a7` - PR created → transitioning to `State/In Review`
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#822
No description provided.