Files
cleveragents-core/features/steps/sandbox_cleanup_conditional_steps.py
T
HAL9001 1789f6323b
CI / benchmark-publish (pull_request) Has been skipped
CI / helm (pull_request) Successful in 33s
CI / push-validation (pull_request) Successful in 28s
CI / lint (pull_request) Successful in 1m14s
CI / quality (pull_request) Successful in 1m36s
CI / typecheck (pull_request) Successful in 1m37s
CI / build (pull_request) Successful in 1m42s
CI / security (pull_request) Successful in 1m53s
CI / e2e_tests (pull_request) Successful in 3m49s
CI / integration_tests (pull_request) Successful in 3m53s
CI / unit_tests (pull_request) Successful in 6m33s
CI / docker (pull_request) Successful in 1m29s
CI / coverage (pull_request) Successful in 11m1s
CI / status-check (pull_request) Successful in 4s
fix(plan): only cleanup worktree sandbox on execute failure, not success
Replace unconditional sandbox cleanup in execute_plan's finally block
with conditional cleanup gated on an explicit execute_succeeded flag.
On success the worktree branch survives until plan apply merges it
into the project (spec §13256-13260).

- Add execute_succeeded flag set after successful execution
- Guard finally-block cleanup with 'if not execute_succeeded'
- Remove unused _commit_exc variable in _commit_worktree_changes
- Add BDD test scenarios (sandbox_cleanup_conditional.feature)

ISSUES CLOSED: #10872
2026-04-30 10:03:08 +00:00

454 lines
16 KiB
Python

"""Steps for sandbox_cleanup_conditional.feature."""
from __future__ import annotations
from unittest.mock import MagicMock, patch
from behave import given, then, when
from typer.testing import CliRunner
from cleveragents.cli.commands.plan import app as plan_app
from cleveragents.domain.models.core.plan import PlanPhase, ProcessingState
from cleveragents.infrastructure.sandbox.git_worktree import (
GitWorktreeSandbox,
)
# ---------------------------------------------------------------------------
# Fixture helper: mock sandbox objects
# ---------------------------------------------------------------------------
def _make_mock_sandbox_infos(count: int) -> list[object]:
"""Return a list of ``_SandboxInfo``-like objects with mocked cleanup.
Since ``_SandboxInfo`` uses ``__slots__``, we cannot monkey-patch extra
attributes. Instead we pass a MagicMock as ``sandbox_obj`` whose
``cleanup`` method we can later assert on via ``info.sandbox_obj.cleanup``.
"""
from cleveragents.cli.commands.plan import _SandboxInfo
infos: list[object] = []
for i in range(count):
sandbox_obj = MagicMock(spec=GitWorktreeSandbox)
sandbox_obj.cleanup = MagicMock()
sandbox_obj.sandbox_path = f"/tmp/sandbox-{i}"
info = _SandboxInfo(
sandbox_path=sandbox_obj.sandbox_path,
sandbox_obj=sandbox_obj,
resource_location=f"/tmp/resource-{i}",
project_name=f"project-{i}",
)
infos.append(info)
return infos
# ---------------------------------------------------------------------------
# Context label dispatch helper
# ---------------------------------------------------------------------------
_LABEL_MAP = {
"scco": "scco",
"scef": "scef",
"scef-er": "scef_er",
"sccs": "sccs",
}
def _attr(context: object, label: str, suffix: str) -> object:
"""Get a context attribute by label and suffix, e.g. scco_runner."""
key = _LABEL_MAP.get(label)
if key is None:
raise ValueError(f"Unknown context_label: {label}")
return getattr(context, f"{key}_{suffix}")
# ---------------------------------------------------------------------------
# scco - sandbox cleanup conditional on execute success
# ---------------------------------------------------------------------------
@given("a mocked lifecycle service with a strat-complete plan for scco")
def step_mock_lifecycle_service_scco(context: object) -> None:
plan = MagicMock()
plan.phase = PlanPhase.STRATEGIZE
plan.state = ProcessingState.COMPLETE
plan.processing_state = ProcessingState.COMPLETE
plan.identity.plan_id = "01TESTSCCPASS0000000000"
plan.read_only = False
plan.project_links = []
plan.automation_profile = None
plan.timestamps = MagicMock()
plan.timestamps.execute_started_at = None
plan.estimation_result = None
plan.decisions = []
plan.invariants = []
mock_service = MagicMock()
# execute_plan transitions to EXECUTE/QUEUED so run_execute is called
def fake_execute(pid: str) -> MagicMock:
plan.phase = PlanPhase.EXECUTE
plan.state = ProcessingState.QUEUED
plan.processing_state = ProcessingState.QUEUED
return plan
mock_service.execute_plan.side_effect = fake_execute
mock_service.get_plan.return_value = plan
context.scco_runner = CliRunner()
context.scco_service = mock_service
context.scco_executor = MagicMock()
context.scco_plan = plan
@given("the executor completes successfully for scco")
def step_executor_succeeds_scco(context: object) -> None:
"""No side-effect — ``run_execute`` does nothing → success."""
context.scco_executor.run_execute.side_effect = None
@given("sandbox_infos contains two active sandboxes for scco")
def step_two_sandboxes_scco(context: object) -> None:
context.scco_sandbox_infos = _make_mock_sandbox_infos(2)
context.scco_sandbox_patcher = patch(
"cleveragents.cli.commands.plan._create_sandbox_for_plan",
return_value=("/tmp/sandbox-parent", context.scco_sandbox_infos),
)
context.scco_sandbox_patcher.start()
if not hasattr(context, "_cleanup_handlers"):
context._cleanup_handlers = []
context._cleanup_handlers.append(context.scco_sandbox_patcher.stop)
context.scco_executor_patcher = patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=context.scco_executor,
)
context.scco_executor_patcher.start()
context._cleanup_handlers.append(context.scco_executor_patcher.stop)
@given("a mocked lifecycle service with a strat-complete plan for sccs")
def step_mock_lifecycle_service_sccs(context: object) -> None:
plan = MagicMock()
plan.phase = PlanPhase.STRATEGIZE
plan.state = ProcessingState.COMPLETE
plan.processing_state = ProcessingState.COMPLETE
plan.identity.plan_id = "01TESTSCCSKIP000000000000"
plan.read_only = False
plan.project_links = []
plan.automation_profile = None
plan.timestamps = MagicMock()
plan.timestamps.execute_started_at = None
plan.estimation_result = None
plan.decisions = []
plan.invariants = []
mock_service = MagicMock()
# execute_plan transitions to EXECUTE/QUEUED so run_execute is called
def fake_execute(pid: str) -> MagicMock:
plan.phase = PlanPhase.EXECUTE
plan.state = ProcessingState.QUEUED
plan.processing_state = ProcessingState.QUEUED
return plan
mock_service.execute_plan.side_effect = fake_execute
mock_service.get_plan.return_value = plan
context.sccs_runner = CliRunner()
context.sccs_service = mock_service
context.sccs_executor = MagicMock()
context.sccs_plan = plan
@given("the executor completes successfully for sccs")
def step_executor_succeeds_sccs(context: object) -> None:
context.sccs_executor.run_execute.side_effect = None
@given("sandbox_infos is empty for sccs")
def step_empty_sandboxes_sccs(context: object) -> None:
context.sccs_sandbox_infos = []
context.sccs_sandbox_patcher = patch(
"cleveragents.cli.commands.plan._create_sandbox_for_plan",
return_value=(None, []),
)
context.sccs_sandbox_patcher.start()
if not hasattr(context, "_cleanup_handlers"):
context._cleanup_handlers = []
context._cleanup_handlers.append(context.sccs_sandbox_patcher.stop)
context.sccs_executor_patcher = patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=context.sccs_executor,
)
context.sccs_executor_patcher.start()
context._cleanup_handlers.append(context.sccs_executor_patcher.stop)
# ---------------------------------------------------------------------------
# scef - sandbox cleanup on execute failure
# ---------------------------------------------------------------------------
@given("a mocked lifecycle service with a strat-complete plan for scef")
def step_mock_lifecycle_service_scef(context: object) -> None:
plan = MagicMock()
plan.phase = PlanPhase.STRATEGIZE
plan.state = ProcessingState.COMPLETE
plan.processing_state = ProcessingState.COMPLETE
plan.identity.plan_id = "01TESTSCCFAIL0000000000"
plan.read_only = False
plan.project_links = []
plan.automation_profile = None
plan.timestamps = MagicMock()
plan.timestamps.execute_started_at = None
plan.estimation_result = None
plan.decisions = []
plan.invariants = []
mock_service = MagicMock()
# execute_plan transitions to EXECUTE/QUEUED so run_execute is called
def fake_execute(pid: str) -> MagicMock:
plan.phase = PlanPhase.EXECUTE
plan.state = ProcessingState.QUEUED
plan.processing_state = ProcessingState.QUEUED
return plan
mock_service.execute_plan.side_effect = fake_execute
mock_service.get_plan.return_value = plan
context.scef_runner = CliRunner()
context.scef_service = mock_service
context.scef_executor = MagicMock()
context.scef_plan = plan
@given("the executor raises RuntimeError during run_execute for scef")
def step_executor_raises_scef(context: object) -> None:
context.scef_executor.run_execute.side_effect = RuntimeError(
"Simulated execute failure"
)
@given("sandbox_infos contains two active sandboxes for scef")
def step_two_sandboxes_scef(context: object) -> None:
context.scef_sandbox_infos = _make_mock_sandbox_infos(2)
context.scef_sandbox_patcher = patch(
"cleveragents.cli.commands.plan._create_sandbox_for_plan",
return_value=("/tmp/sandbox-parent", context.scef_sandbox_infos),
)
context.scef_sandbox_patcher.start()
if not hasattr(context, "_cleanup_handlers"):
context._cleanup_handlers = []
context._cleanup_handlers.append(context.scef_sandbox_patcher.stop)
context.scef_executor_patcher = patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=context.scef_executor,
)
context.scef_executor_patcher.start()
context._cleanup_handlers.append(context.scef_executor_patcher.stop)
# ---------------------------------------------------------------------------
# scef-er - sandbox cleanup on error-recovery failure
# ---------------------------------------------------------------------------
@given("a mocked lifecycle service with an execute-errored plan for scef-er")
def step_mock_execute_errored_scefer(context: object) -> None:
plan = MagicMock()
plan.phase = PlanPhase.EXECUTE
plan.state = ProcessingState.ERRORED
plan.processing_state = ProcessingState.ERRORED
plan.identity.plan_id = "01TESTSCEFERR0000000000"
plan.error_details = {"exception_type": "ValueError"}
plan.read_only = False
plan.project_links = []
plan.automation_profile = None
plan.timestamps = MagicMock()
plan.timestamps.execute_started_at = None
mock_service = MagicMock()
mock_service.get_plan.return_value = plan
# revert_plan raises ValueError → _recover_errored_execute_plan
# converts it to typer.Abort → caught by the outer except → cleanup
mock_service.revert_plan.side_effect = ValueError("Recovery failed")
context.scef_er_runner = CliRunner()
context.scef_er_service = mock_service
context.scef_er_executor = MagicMock()
context.scef_er_plan = plan
@given("the executor raises ValueError during error recovery for scef-er")
def step_executor_raises_value_error(context: object) -> None:
"""The ValueError is already wired on service.revert_plan — nothing to do."""
@given("sandbox_infos contains two active sandboxes for scef-er")
def step_two_sandboxes_scefer(context: object) -> None:
context.scef_er_sandbox_infos = _make_mock_sandbox_infos(2)
context.scef_er_sandbox_patcher = patch(
"cleveragents.cli.commands.plan._create_sandbox_for_plan",
return_value=(
"/tmp/sandbox-parent",
context.scef_er_sandbox_infos,
),
)
context.scef_er_sandbox_patcher.start()
if not hasattr(context, "_cleanup_handlers"):
context._cleanup_handlers = []
context._cleanup_handlers.append(context.scef_er_sandbox_patcher.stop)
context.scef_er_executor_patcher = patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=context.scef_er_executor,
)
context.scef_er_executor_patcher.start()
context._cleanup_handlers.append(context.scef_er_executor_patcher.stop)
# ---------------------------------------------------------------------------
# When steps
# ---------------------------------------------------------------------------
@when('I invoke plan execute for plan "{plan_id}" for {context_label}')
def step_invoke_execute(context: object, plan_id: str, context_label: str) -> None:
"""Invoke ``plan execute`` with the appropriate mocked fixtures."""
from contextlib import ExitStack
# Map context_label to the correct attribute prefix
prefix_map = {
"scco": "scco",
"scef": "scef",
"sccs": "sccs",
"scef-er": "scef_er",
}
prefix = prefix_map.get(context_label)
if prefix is None:
raise ValueError(f"Unknown context_label: {context_label}")
runner = getattr(context, f"{prefix}_runner")
sandboxes = getattr(context, f"{prefix}_sandbox_infos")
executor = getattr(context, f"{prefix}_executor")
svc = getattr(context, f"{prefix}_service")
with ExitStack() as stack:
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=svc,
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._create_sandbox_for_plan",
return_value=("/tmp/sandbox-parent", sandboxes),
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=executor,
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._validate_plan_ulid",
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._route_sandbox_files_to_worktrees",
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._commit_worktree_changes",
)
)
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._notify_facade",
)
)
# For error-recovery scenarios the real _recover_errored_execute_plan
# must run so the error propagates. For all others we stub it out.
if context_label != "scef-er":
stack.enter_context(
patch(
"cleveragents.cli.commands.plan._recover_errored_execute_plan",
)
)
context.sandbox_result = runner.invoke(plan_app, ["execute", plan_id])
context.sandbox_output = context.sandbox_result.output
# ---------------------------------------------------------------------------
# Then steps
# ---------------------------------------------------------------------------
def _get_sandbox_infos(context: object, context_label: str) -> list[object]:
"""Return the sandbox_infos list for a given context label."""
prefix_map = {
"scco": "scco",
"scef": "scef",
"sccs": "sccs",
"scef-er": "scef_er",
}
prefix = prefix_map.get(context_label)
if prefix is None:
raise AssertionError(f"Unknown context_label: {context_label}")
return getattr(context, f"{prefix}_sandbox_infos")
@then("sandbox cleanup should have been called for {context_label}")
def step_cleanup_called(context: object, context_label: str) -> None:
"""Verify every sandbox_obj in the infos called cleanup()."""
sandboxes = _get_sandbox_infos(context, context_label)
for info in sandboxes:
assert info.sandbox_obj.cleanup.called, (
f"Expected sandbox_obj.cleanup() to be called, "
f"but it was not for sandbox {info.sandbox_path}"
)
@then("sandbox cleanup should NOT have been called for {context_label}")
def step_cleanup_not_called(context: object, context_label: str) -> None:
"""Verify no sandbox_obj cleanup() was called."""
sandboxes = _get_sandbox_infos(context, context_label)
for info in sandboxes:
assert not info.sandbox_obj.cleanup.called, (
f"Expected sandbox_obj.cleanup() NOT to be called, "
f"but it was called for sandbox {info.sandbox_path}"
)
@then("the CLI exit code should be zero for {context_label}")
def step_check_success(context: object, context_label: str) -> None:
assert context.sandbox_result.exit_code == 0, (
f"Expected zero but got {context.sandbox_result.exit_code}\n"
f"Output: {context.sandbox_output[:500]}"
)
@then("the CLI exit code should be non-zero for {context_label}")
def step_check_failure(context: object, context_label: str) -> None:
assert context.sandbox_result.exit_code != 0, (
f"Expected non-zero but got {context.sandbox_result.exit_code}"
)