forked from HAL9000/cleveragents-core
fix(plan-lifecycle): align rollback contract with spec
- update PlanLifecycleService documentation for rollback phase/state semantics\n- add Behave scenarios covering checkpoint error propagation and supporting steps\n- adjust mock helpers to raise configured exceptions and keep scenario count accurate\n\nISSUES CLOSED: #3677
This commit is contained in:
@@ -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
|
||||
# ───────────────────────────────────────────────────────────
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user