From 6efbd4f7d24650a99db073d70dd34cd5f0650802 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Thu, 2 Apr 2026 09:27:00 +0000 Subject: [PATCH] feat(events): enrich PLAN_CANCELLED event with progress and resource cleanup context Enriches the PLAN_CANCELLED domain event emitted by PlanLifecycleService.cancel_plan() with additional context fields to improve audit trail quality (SEC7 Audit Logging). Progress context fields added to event details: - cancelled_phase: the plan phase at the moment of cancellation (e.g. 'strategize') - cancelled_processing_state: the processing state at cancellation (e.g. 'queued') - last_completed_step: index of the last successfully completed step (-1 if none) - subplan_count: number of spawned subplans at cancellation time Resource cleanup context fields added to event details: - sandbox_refs: list of active sandbox reference IDs requiring cleanup - changeset_id: ID of any in-progress changeset that needs cleanup - resources_pending_cleanup: count of sandbox refs pending cleanup Existing fields (reason, project_names) are preserved for backward compatibility. Progress context is captured before the plan state is mutated to CANCELLED so the event accurately reflects the state at the moment of cancellation. BDD tests added in features/plan_cancelled_enrichment.feature covering: - All new progress context fields present in event details - All new resource cleanup context fields present in event details - Cancellation reason field (explicit and default empty string) - Accurate field values (phase, step, sandbox refs count) - Backward compatibility of existing fields Closes #717 --- features/plan_cancelled_enrichment.feature | 122 +++++++++ .../steps/plan_cancelled_enrichment_steps.py | 252 ++++++++++++++++++ .../services/plan_lifecycle_service.py | 33 ++- 3 files changed, 399 insertions(+), 8 deletions(-) create mode 100644 features/plan_cancelled_enrichment.feature create mode 100644 features/steps/plan_cancelled_enrichment_steps.py 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 84572bc9b..ca9867b35 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -1540,6 +1540,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() @@ -1551,16 +1560,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: @@ -1570,10 +1591,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) -- 2.52.0