feat(events): enrich PLAN_CANCELLED event with progress and resource cleanup context (#1301)
CI / typecheck (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / build (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / helm (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / security (push) Has been cancelled
CI / unit_tests (push) Has been cancelled
CI / status-check (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
CI / typecheck (push) Has been cancelled
CI / quality (push) Has been cancelled
CI / e2e_tests (push) Has been cancelled
CI / build (push) Has been cancelled
CI / integration_tests (push) Has been cancelled
CI / helm (push) Has been cancelled
CI / benchmark-publish (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / security (push) Has been cancelled
CI / unit_tests (push) Has been cancelled
CI / status-check (push) Has been cancelled
CI / coverage (push) Has been cancelled
CI / docker (push) Has been cancelled
CI / benchmark-regression (push) Has been cancelled
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: cancelled_phase, cancelled_processing_state, last_completed_step, subplan_count Resource cleanup context fields: sandbox_refs, changeset_id, resources_pending_cleanup Existing fields (reason, project_names) preserved for backward compatibility. 15 BDD scenarios added covering all new fields, accuracy, and backward compatibility. Closes #717 Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
This commit was merged in pull request #1301.
This commit is contained in:
@@ -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"
|
||||
@@ -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}"
|
||||
)
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user