diff --git a/features/plan_cancelled_enrichment.feature b/features/plan_cancelled_enrichment.feature new file mode 100644 index 000000000..459577551 --- /dev/null +++ b/features/plan_cancelled_enrichment.feature @@ -0,0 +1,122 @@ +Feature: Enriched PLAN_CANCELLED event payload + As an audit log consumer + I want the PLAN_CANCELLED domain event to include progress and resource cleanup context + So that I can understand what state the plan was in when it was cancelled + and what resources need cleanup + + Background: + Given I have a plan lifecycle service for cancelled enrichment tests + + # --------------------------------------------------------------- + # Core enrichment: progress context fields + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event includes phase at cancellation + Given an action "local/enrich-cancel-phase" exists for enrichment tests + And a plan created from "local/enrich-cancel-phase" for enrichment tests + When I cancel the plan for enrichment tests with reason "user requested" + Then the PLAN_CANCELLED event details should include "cancelled_phase" + + Scenario: PLAN_CANCELLED event includes processing state at cancellation + Given an action "local/enrich-cancel-state" exists for enrichment tests + And a plan created from "local/enrich-cancel-state" for enrichment tests + When I cancel the plan for enrichment tests with reason "timeout" + Then the PLAN_CANCELLED event details should include "cancelled_processing_state" + + Scenario: PLAN_CANCELLED event includes last completed step + Given an action "local/enrich-cancel-step" exists for enrichment tests + And a plan created from "local/enrich-cancel-step" for enrichment tests + When I cancel the plan for enrichment tests with reason "budget exceeded" + Then the PLAN_CANCELLED event details should include "last_completed_step" + + Scenario: PLAN_CANCELLED event includes subplan count + Given an action "local/enrich-cancel-subplan" exists for enrichment tests + And a plan created from "local/enrich-cancel-subplan" for enrichment tests + When I cancel the plan for enrichment tests with reason "manual" + Then the PLAN_CANCELLED event details should include "subplan_count" + + # --------------------------------------------------------------- + # Core enrichment: resource cleanup context fields + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event includes sandbox refs for cleanup + Given an action "local/enrich-cancel-sandbox" exists for enrichment tests + And a plan created from "local/enrich-cancel-sandbox" for enrichment tests + When I cancel the plan for enrichment tests with reason "resource limit" + Then the PLAN_CANCELLED event details should include "sandbox_refs" + + Scenario: PLAN_CANCELLED event includes changeset id for cleanup + Given an action "local/enrich-cancel-changeset" exists for enrichment tests + And a plan created from "local/enrich-cancel-changeset" for enrichment tests + When I cancel the plan for enrichment tests with reason "rollback" + Then the PLAN_CANCELLED event details should include "changeset_id" + + Scenario: PLAN_CANCELLED event includes resources pending cleanup count + Given an action "local/enrich-cancel-resources" exists for enrichment tests + And a plan created from "local/enrich-cancel-resources" for enrichment tests + When I cancel the plan for enrichment tests with reason "cleanup test" + Then the PLAN_CANCELLED event details should include "resources_pending_cleanup" + + # --------------------------------------------------------------- + # Cancellation reason field + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event includes cancellation reason + Given an action "local/enrich-cancel-reason" exists for enrichment tests + And a plan created from "local/enrich-cancel-reason" for enrichment tests + When I cancel the plan for enrichment tests with reason "explicit reason" + Then the PLAN_CANCELLED event details should have reason "explicit reason" + + Scenario: PLAN_CANCELLED event reason defaults to empty string when not provided + Given an action "local/enrich-cancel-no-reason" exists for enrichment tests + And a plan created from "local/enrich-cancel-no-reason" for enrichment tests + When I cancel the plan for enrichment tests without a reason + Then the PLAN_CANCELLED event reason field should be empty string + + # --------------------------------------------------------------- + # Progress context accuracy + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event captures phase accurately as Strategize + Given an action "local/enrich-cancel-strategize" exists for enrichment tests + And a plan created from "local/enrich-cancel-strategize" for enrichment tests + When I cancel the plan for enrichment tests with reason "cancelled in strategize" + Then the PLAN_CANCELLED event details should have cancelled_phase "strategize" + + Scenario: PLAN_CANCELLED event captures last_completed_step value accurately + Given an action "local/enrich-cancel-step-val" exists for enrichment tests + And a plan created from "local/enrich-cancel-step-val" for enrichment tests + When I cancel the plan for enrichment tests with reason "step check" + Then the PLAN_CANCELLED event details should have last_completed_step -1 + + # --------------------------------------------------------------- + # Resource cleanup context accuracy + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event captures sandbox refs accurately when present + Given an action "local/enrich-cancel-sandbox-val" exists for enrichment tests + And a plan with sandbox refs created from "local/enrich-cancel-sandbox-val" + When I cancel the plan for enrichment tests with reason "sandbox cleanup" + Then the PLAN_CANCELLED event details should have sandbox_refs count 2 + + Scenario: PLAN_CANCELLED event captures resources_pending_cleanup count accurately + Given an action "local/enrich-cancel-pending" exists for enrichment tests + And a plan with sandbox refs created from "local/enrich-cancel-pending" + When I cancel the plan for enrichment tests with reason "pending cleanup" + Then the PLAN_CANCELLED event details should have resources_pending_cleanup 2 + + # --------------------------------------------------------------- + # Backward compatibility: existing fields still present + # --------------------------------------------------------------- + + Scenario: PLAN_CANCELLED event still includes project_names field + Given an action "local/enrich-cancel-compat" exists for enrichment tests + And a plan created from "local/enrich-cancel-compat" for enrichment tests + When I cancel the plan for enrichment tests with reason "compat check" + Then the PLAN_CANCELLED event details should include "project_names" + + Scenario: PLAN_CANCELLED event still includes reason field + Given an action "local/enrich-cancel-compat-reason" exists for enrichment tests + And a plan created from "local/enrich-cancel-compat-reason" for enrichment tests + When I cancel the plan for enrichment tests with reason "compat reason" + Then the PLAN_CANCELLED event details should include "reason" diff --git a/features/steps/plan_cancelled_enrichment_steps.py b/features/steps/plan_cancelled_enrichment_steps.py new file mode 100644 index 000000000..fb16e434d --- /dev/null +++ b/features/steps/plan_cancelled_enrichment_steps.py @@ -0,0 +1,252 @@ +"""Step definitions for plan_cancelled_enrichment.feature. + +Tests that the PLAN_CANCELLED domain event is enriched with: +- Progress context: cancelled_phase, cancelled_processing_state, + last_completed_step, subplan_count +- Resource cleanup context: sandbox_refs, changeset_id, + resources_pending_cleanup +- Cancellation reason (existing field, verified still present) +- Backward-compatible fields: project_names, reason + +Related: Forgejo issue #717 +""" + +from __future__ import annotations + +from unittest.mock import patch + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.plan_lifecycle_service import ( + PlanLifecycleService, +) +from cleveragents.config.settings import Settings +from cleveragents.domain.models.core.plan import ProjectLink +from cleveragents.infrastructure.events.types import EventType + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +class _RecordingBus: + """Minimal event bus that records every emitted event.""" + + def __init__(self) -> None: + self.events: list = [] + + def emit(self, event) -> None: + self.events.append(event) + + def subscribe(self, event_type, handler) -> None: + pass + + +def _get_cancelled_event(context: Context): + """Return the first PLAN_CANCELLED event from the recording bus.""" + events = [ + e + for e in context.recording_bus.events + if e.event_type == EventType.PLAN_CANCELLED + ] + assert events, ( + f"No PLAN_CANCELLED event found. " + f"All events: {[e.event_type for e in context.recording_bus.events]}" + ) + return events[0] + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("I have a plan lifecycle service for cancelled enrichment tests") +def step_create_service_for_enrichment(context: Context) -> None: + """Create a PlanLifecycleService with a recording event bus.""" + Settings._instance = None + settings = Settings() + context.recording_bus = _RecordingBus() + context.service = PlanLifecycleService( + settings=settings, + event_bus=context.recording_bus, + ) + context.error = None + context.plan = None + + +# --------------------------------------------------------------------------- +# Given: action and plan creation +# --------------------------------------------------------------------------- + + +@given('an action "{action_name}" exists for enrichment tests') +def step_create_action_for_enrichment(context: Context, action_name: str) -> None: + """Create an action with the given namespaced name.""" + context.action = context.service.create_action( + name=action_name, + description=f"Action {action_name} for enrichment tests", + definition_of_done="Tests pass", + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + ) + + +@given('a plan created from "{action_name}" for enrichment tests') +def step_create_plan_for_enrichment(context: Context, action_name: str) -> None: + """Create a plan from the given action.""" + context.plan = context.service.use_action( + action_name=action_name, + project_links=[ProjectLink(project_name="test-project")], + ) + + +@given('a plan with sandbox refs created from "{action_name}"') +def step_create_plan_with_sandbox_refs(context: Context, action_name: str) -> None: + """Create a plan and inject sandbox refs to simulate active sandboxes.""" + plan = context.service.use_action( + action_name=action_name, + project_links=[ProjectLink(project_name="test-project")], + ) + # Inject sandbox refs directly (model is frozen, use model_copy) + updated_plan = plan.model_copy( + update={"sandbox_refs": ["sandbox-001", "sandbox-002"]} + ) + # Update the in-memory store so cancel_plan retrieves the updated plan + context.service._plans[updated_plan.identity.plan_id] = updated_plan + context.plan = updated_plan + + +# --------------------------------------------------------------------------- +# When: cancel the plan +# --------------------------------------------------------------------------- + + +@when('I cancel the plan for enrichment tests with reason "{reason}"') +def step_cancel_plan_with_reason(context: Context, reason: str) -> None: + """Cancel the plan with the given reason.""" + context.error = None + try: + with patch( + "cleveragents.application.services.cleanup_service" + ".stop_all_active_containers", + return_value=[], + ): + context.plan = context.service.cancel_plan( + context.plan.identity.plan_id, + reason=reason, + ) + except Exception as exc: + context.error = exc + + +@when("I cancel the plan for enrichment tests without a reason") +def step_cancel_plan_without_reason(context: Context) -> None: + """Cancel the plan without providing a reason.""" + context.error = None + try: + with patch( + "cleveragents.application.services.cleanup_service" + ".stop_all_active_containers", + return_value=[], + ): + context.plan = context.service.cancel_plan( + context.plan.identity.plan_id, + ) + except Exception as exc: + context.error = exc + + +# --------------------------------------------------------------------------- +# Then: assertions on event details +# --------------------------------------------------------------------------- + + +@then('the PLAN_CANCELLED event details should include "{field}"') +def step_event_details_include_field(context: Context, field: str) -> None: + """Assert that the PLAN_CANCELLED event details dict contains the field.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + assert field in event.details, ( + f"Expected field '{field}' in PLAN_CANCELLED event details. " + f"Actual details keys: {list(event.details.keys())}" + ) + + +@then('the PLAN_CANCELLED event details should have reason "{expected_reason}"') +def step_event_details_reason(context: Context, expected_reason: str) -> None: + """Assert the reason field in the event details matches expected value.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + actual = event.details.get("reason") + assert actual == expected_reason, ( + f"Expected reason '{expected_reason}', got '{actual}'" + ) + + +@then("the PLAN_CANCELLED event reason field should be empty string") +def step_event_details_reason_empty(context: Context) -> None: + """Assert the reason field in the event details is an empty string.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + actual = event.details.get("reason") + assert actual == "", f"Expected empty string reason, got '{actual}'" + + +@then('the PLAN_CANCELLED event details should have cancelled_phase "{expected_phase}"') +def step_event_details_cancelled_phase(context: Context, expected_phase: str) -> None: + """Assert the cancelled_phase field matches the expected phase string.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + actual = event.details.get("cancelled_phase") + assert actual == expected_phase, ( + f"Expected cancelled_phase '{expected_phase}', got '{actual}'" + ) + + +@then( + "the PLAN_CANCELLED event details should have last_completed_step {expected_step:d}" +) +def step_event_details_last_completed_step( + context: Context, expected_step: int +) -> None: + """Assert the last_completed_step field matches the expected value.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + actual = event.details.get("last_completed_step") + assert actual == expected_step, ( + f"Expected last_completed_step {expected_step}, got {actual}" + ) + + +@then( + "the PLAN_CANCELLED event details should have sandbox_refs count {expected_count:d}" +) +def step_event_details_sandbox_refs_count( + context: Context, expected_count: int +) -> None: + """Assert the sandbox_refs list has the expected number of entries.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + sandbox_refs = event.details.get("sandbox_refs", []) + actual_count = len(sandbox_refs) + assert actual_count == expected_count, ( + f"Expected {expected_count} sandbox_refs, got {actual_count}: {sandbox_refs}" + ) + + +@then( + "the PLAN_CANCELLED event details should have resources_pending_cleanup" + " {expected_count:d}" +) +def step_event_details_resources_pending_cleanup( + context: Context, expected_count: int +) -> None: + """Assert the resources_pending_cleanup count matches expected value.""" + assert context.error is None, f"Unexpected error: {context.error}" + event = _get_cancelled_event(context) + actual = event.details.get("resources_pending_cleanup") + assert actual == expected_count, ( + f"Expected resources_pending_cleanup {expected_count}, got {actual}" + ) diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index d669f6a98..c5f02211d 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -1737,6 +1737,15 @@ class PlanLifecycleService: f"Plan {plan_id} is already in terminal state and cannot be cancelled" ) + # Capture progress and resource context *before* mutating the plan + # so the event reflects the state at the moment of cancellation. + cancelled_phase: str = str(plan.phase) + cancelled_processing_state: str = str(plan.processing_state) + last_completed_step: int = plan.last_completed_step + sandbox_refs: list[str] = list(plan.sandbox_refs) + changeset_id: str | None = plan.changeset_id + subplan_count: int = len(plan.subplan_statuses) + plan.processing_state = ProcessingState.CANCELLED plan.error_message = reason plan.timestamps.updated_at = datetime.now() @@ -1748,16 +1757,28 @@ class PlanLifecycleService: project_names = [ link.project_name for link in (plan.project_links or []) ] + # Build enriched details: progress context + resource cleanup + # context as required by issue #717 / SEC7 audit logging spec. + event_details: dict[str, object] = { + "reason": reason or "", + "project_names": project_names, + # Progress context — where the plan was when cancelled + "cancelled_phase": cancelled_phase, + "cancelled_processing_state": cancelled_processing_state, + "last_completed_step": last_completed_step, + "subplan_count": subplan_count, + # Resource cleanup context — what needs / needed cleanup + "sandbox_refs": sandbox_refs, + "changeset_id": changeset_id, + "resources_pending_cleanup": len(sandbox_refs), + } self.event_bus.emit( DomainEvent( event_type=EventType.PLAN_CANCELLED, plan_id=plan_id, actor_name=plan.created_by, project_name=project_names[0] if project_names else None, - details={ - "reason": reason or "", - "project_names": project_names, - }, + details=event_details, ) ) except Exception: @@ -1767,10 +1788,6 @@ class PlanLifecycleService: plan_id=plan_id, exc_info=True, ) - # COV-3: Spec says plan_cancelled should also include - # "resources released", but that data is not available at - # this point -- downstream cleanup happens in separate - # services. Tracked for future enhancement. # F1-r6 fix: wire container cleanup to plan cancellation hook. self._cleanup_devcontainers(plan_id)