diff --git a/features/plan_lifecycle_rollback.feature b/features/plan_lifecycle_rollback.feature index fd91b8cfc9..d167443aba 100644 --- a/features/plan_lifecycle_rollback.feature +++ b/features/plan_lifecycle_rollback.feature @@ -83,6 +83,24 @@ Feature: PlanLifecycleService rollback_plan method When plr-I call rollback_plan with the plan id and checkpoint id Then plr-the rollback should succeed + # ─────────────────────────────────────────────────────────── + # CheckpointService exception propagation + # ─────────────────────────────────────────────────────────── + + Scenario: rollback_plan propagates ResourceNotFoundError from checkpoint service + Given plr-a plan lifecycle service with a checkpoint service that raises ResourceNotFoundError + And plr-a plan in Execute phase with a sandbox + When plr-I call rollback_plan with the plan id and checkpoint id + Then plr-a ResourceNotFoundError should be raised + And plr-the checkpoint service selective_rollback should have been called + + Scenario: rollback_plan propagates BusinessRuleViolation from checkpoint service + Given plr-a plan lifecycle service with a checkpoint service that raises BusinessRuleViolation + And plr-a plan in Execute phase with a sandbox + When plr-I call rollback_plan with the plan id and checkpoint id + Then plr-a BusinessRuleViolation should be raised + And plr-the checkpoint service selective_rollback should have been called + # ─────────────────────────────────────────────────────────── # NotFoundError propagation # ─────────────────────────────────────────────────────────── diff --git a/features/steps/plan_lifecycle_rollback_steps.py b/features/steps/plan_lifecycle_rollback_steps.py index e03abac002..6895a7b85f 100644 --- a/features/steps/plan_lifecycle_rollback_steps.py +++ b/features/steps/plan_lifecycle_rollback_steps.py @@ -20,7 +20,12 @@ from cleveragents.application.services.plan_lifecycle_service import ( PlanLifecycleService, ) from cleveragents.config.settings import Settings -from cleveragents.core.exceptions import NotFoundError, PlanError +from cleveragents.core.exceptions import ( + BusinessRuleViolation, + NotFoundError, + PlanError, + ResourceNotFoundError, +) from cleveragents.domain.models.core.checkpoint import RollbackResult from cleveragents.domain.models.core.plan import ( PlanPhase, @@ -45,14 +50,19 @@ _NONEXISTENT_PLAN_ID = "01NONEXISTENT0000000000000" def _make_mock_checkpoint_service( plan_id: str = _PLAN_ID, checkpoint_id: str = _CHECKPOINT_ID, + *, + side_effect: Exception | None = None, ) -> MagicMock: """Build a mock CheckpointService that returns a valid RollbackResult.""" mock_svc = MagicMock() - mock_svc.selective_rollback.return_value = RollbackResult( - restored_files_count=3, - changed_paths=["src/foo.py", "src/bar.py", "tests/test_foo.py"], - from_checkpoint_id=checkpoint_id, - ) + if side_effect is not None: + mock_svc.selective_rollback.side_effect = side_effect + else: + mock_svc.selective_rollback.return_value = RollbackResult( + restored_files_count=3, + changed_paths=["src/foo.py", "src/bar.py", "tests/test_foo.py"], + from_checkpoint_id=checkpoint_id, + ) return mock_svc @@ -154,6 +164,38 @@ def step_plr_service_with_checkpoint_no_event_bus(context: Context) -> None: context.plr_checkpoint_id = _CHECKPOINT_ID +@given( + "plr-a plan lifecycle service with a checkpoint service that raises ResourceNotFoundError" +) +def step_plr_service_checkpoint_raises_resource_not_found(context: Context) -> None: + """Set up a service whose checkpoint service raises ResourceNotFoundError.""" + context.plr_mock_checkpoint_svc = _make_mock_checkpoint_service( + side_effect=ResourceNotFoundError( + "Checkpoint not found", + resource_type="checkpoint", + resource_id=_CHECKPOINT_ID, + ) + ) + context.plr_service = _create_service( + checkpoint_service=context.plr_mock_checkpoint_svc, + ) + context.plr_checkpoint_id = _CHECKPOINT_ID + + +@given( + "plr-a plan lifecycle service with a checkpoint service that raises BusinessRuleViolation" +) +def step_plr_service_checkpoint_raises_business_rule(context: Context) -> None: + """Set up a service whose checkpoint service raises BusinessRuleViolation.""" + context.plr_mock_checkpoint_svc = _make_mock_checkpoint_service( + side_effect=BusinessRuleViolation("Checkpoint rollback failed"), + ) + context.plr_service = _create_service( + checkpoint_service=context.plr_mock_checkpoint_svc, + ) + context.plr_checkpoint_id = _CHECKPOINT_ID + + @given("plr-a plan in Execute phase with a sandbox") def step_plr_plan_in_execute_with_sandbox(context: Context) -> None: """Create a plan in Execute/PROCESSING state.""" @@ -334,6 +376,30 @@ def step_plr_not_found_error_raised(context: Context) -> None: ) +@then("plr-a ResourceNotFoundError should be raised") +def step_plr_resource_not_found_error_raised(context: Context) -> None: + """Assert that a ResourceNotFoundError was raised.""" + assert context.plr_raised_error is not None, ( + "Expected a ResourceNotFoundError to be raised but no error was raised." + ) + assert isinstance(context.plr_raised_error, ResourceNotFoundError), ( + f"Expected ResourceNotFoundError, got {type(context.plr_raised_error)}: " + f"{context.plr_raised_error}" + ) + + +@then("plr-a BusinessRuleViolation should be raised") +def step_plr_business_rule_violation_raised(context: Context) -> None: + """Assert that a BusinessRuleViolation was raised.""" + assert context.plr_raised_error is not None, ( + "Expected a BusinessRuleViolation to be raised but no error was raised." + ) + assert isinstance(context.plr_raised_error, BusinessRuleViolation), ( + f"Expected BusinessRuleViolation, got {type(context.plr_raised_error)}: " + f"{context.plr_raised_error}" + ) + + @then("plr-a PLAN_ROLLED_BACK domain event should have been emitted") def step_plr_plan_rolled_back_event_emitted(context: Context) -> None: """Assert that a PLAN_ROLLED_BACK event was emitted.""" diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index abccfe2e9a..b7e876858f 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -18,7 +18,7 @@ Terminal outcomes live in Apply's processing state: ``applied``, | ``use_action(...)`` | Action (available) | Strategize/QUEUED| | ``execute_plan(...)`` | Strategize/COMPLETE | Execute/QUEUED | | ``apply_plan(...)`` | Execute/COMPLETE | Apply/QUEUED | -| ``rollback_plan(...)`` | Execute (any state) | Execute/QUEUED | +| ``rollback_plan(...)`` | Any (non-terminal) | *(unchanged)* | ## Apply Terminal Outcomes @@ -2149,7 +2149,10 @@ class PlanLifecycleService: current_state=plan.processing_state.value, ) - # Delegate to CheckpointService for the actual sandbox rollback + # Delegate to CheckpointService for the actual sandbox rollback. + # Rollback restores sandbox state but does not mutate the plan's + # phase or processing_state; callers observe the pre-rollback + # lifecycle metadata. result = self.checkpoint_service.selective_rollback(plan_id, checkpoint_id) self._logger.info(