diff --git a/features/actor_remove_impact_computation.feature b/features/actor_remove_impact_computation.feature new file mode 100644 index 000000000..c6db07d2f --- /dev/null +++ b/features/actor_remove_impact_computation.feature @@ -0,0 +1,23 @@ +Feature: agents actor remove Impact section shows real counts + As a user removing an actor + I want the Impact panel to show real counts of affected sessions, plans, and actions + So that I can verify the actual impact before and after removal + + Background: + Given an actor CLI runner + + Scenario: Impact panel shows non-zero counts when actor is referenced + Given an actor "local/my-actor" referenced by 2 sessions, 1 active plan, and 3 actions + When I run actor remove for "local/my-actor" + Then the actor remove should succeed + And the Impact panel should show "2 affected" for Sessions + And the Impact panel should show "1 affected" for Active Plans + And the Impact panel should show "3" for Actions Referencing + + Scenario: Impact panel shows zero counts when actor has no references + Given an actor "local/unused-actor" with no references + When I run actor remove for "local/unused-actor" + Then the actor remove should succeed + And the Impact panel should show "0 affected" for Sessions + And the Impact panel should show "0 affected" for Active Plans + And the Impact panel should show "0" for Actions Referencing diff --git a/features/steps/actor_cli_steps.py b/features/steps/actor_cli_steps.py index ec098370f..66f6e5231 100644 --- a/features/steps/actor_cli_steps.py +++ b/features/steps/actor_cli_steps.py @@ -947,10 +947,14 @@ def step_impl(context): @when("I run actor remove successfully") def step_impl(context): - with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services: + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_get_services, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): mock_actor_service = MagicMock() mock_actor_registry = MagicMock() mock_get_services.return_value = (mock_actor_service, mock_actor_registry) + mock_impact.return_value = (0, 0, 0) context.result = context.runner.invoke(actor_app, ["remove", "local/removable"]) context.mock_actor_registry = mock_actor_registry @@ -958,11 +962,15 @@ def step_impl(context): @when("I run actor remove and it fails validation") def step_impl(context): - with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services: + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_get_services, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): mock_actor_service = MagicMock() mock_actor_registry = MagicMock() mock_actor_registry.remove_actor.side_effect = ValidationError("invalid") mock_get_services.return_value = (mock_actor_service, mock_actor_registry) + mock_impact.return_value = (0, 0, 0) context.result = context.runner.invoke(actor_app, ["remove", "local/removable"]) context.mock_actor_registry = mock_actor_registry @@ -971,9 +979,13 @@ def step_impl(context): @when("I run actor remove via service path") def step_impl(context): - with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services: + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_get_services, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): actor_service = MagicMock() mock_get_services.return_value = (actor_service, None) + mock_impact.return_value = (0, 0, 0) context.result = context.runner.invoke(actor_app, ["remove", "local/removable"]) diff --git a/features/steps/actor_cli_yaml_steps.py b/features/steps/actor_cli_yaml_steps.py index 37c7e42a0..43e9bb6eb 100644 --- a/features/steps/actor_cli_yaml_steps.py +++ b/features/steps/actor_cli_yaml_steps.py @@ -297,9 +297,13 @@ def step_add_yaml_valid(context: Any) -> None: @when("I run actor remove with namespaced name") def step_remove_namespaced(context: Any) -> None: - with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_svc, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): mock_registry = MagicMock() mock_svc.return_value = (MagicMock(), mock_registry) + mock_impact.return_value = (0, 0, 0) context.result = context.runner.invoke( actor_app, ["remove", "local/my-custom-actor"] ) diff --git a/features/steps/actor_remove_impact_steps.py b/features/steps/actor_remove_impact_steps.py new file mode 100644 index 000000000..043ed04d8 --- /dev/null +++ b/features/steps/actor_remove_impact_steps.py @@ -0,0 +1,106 @@ +# pyright: reportRedeclaration=false +"""Step definitions for actor remove impact computation feature.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from typer.testing import CliRunner # noqa: F401 - used via context.runner + +from cleveragents.cli.commands.actor import app as actor_app +from cleveragents.domain.models.core.actor import Actor + + +def _make_actor( + *, + name: str = "local/test-actor", + provider: str = "test-provider", + model: str = "test-model", +) -> Actor: + return Actor( + id=1, + name=name, + provider=provider, + model=model, + config_blob={}, + config_hash="abcdef12", + graph_descriptor=None, + unsafe=False, + is_default=False, + is_built_in=False, + ) + + +@given( + 'an actor "{actor_name}" referenced by {sessions:d} sessions, ' + "{plans:d} active plan, and {actions:d} actions" +) +def step_actor_with_references( + context: Any, actor_name: str, sessions: int, plans: int, actions: int +) -> None: + """Set up context with an actor that has known reference counts.""" + context.actor_name = actor_name + context.expected_sessions = sessions + context.expected_plans = plans + context.expected_actions = actions + + +@given('an actor "{actor_name}" with no references') +def step_actor_no_references(context: Any, actor_name: str) -> None: + """Set up context with an actor that has no references.""" + context.actor_name = actor_name + context.expected_sessions = 0 + context.expected_plans = 0 + context.expected_actions = 0 + + +@when('I run actor remove for "{actor_name}"') +def step_run_actor_remove(context: Any, actor_name: str) -> None: + """Invoke the actor remove command with mocked services and impact counts.""" + sessions = getattr(context, "expected_sessions", 0) + plans = getattr(context, "expected_plans", 0) + actions = getattr(context, "expected_actions", 0) + + mock_actor = _make_actor(name=actor_name) + + with ( + patch("cleveragents.cli.commands.actor._get_services") as mock_get_services, + patch("cleveragents.cli.commands.actor._compute_actor_impact") as mock_impact, + ): + mock_registry = MagicMock() + mock_registry.get_actor.return_value = mock_actor + mock_get_services.return_value = (MagicMock(), mock_registry) + mock_impact.return_value = (sessions, plans, actions) + + context.result = context.runner.invoke(actor_app, ["remove", actor_name]) + context.mock_impact = mock_impact + context.mock_registry = mock_registry + + +@then('the Impact panel should show "{text}" for Sessions') +def step_impact_sessions(context: Any, text: str) -> None: + """Assert the Sessions line in the Impact panel shows the expected text.""" + output = context.result.output + assert text in output, ( + f"Expected '{text}' in output for Sessions, but got:\n{output}" + ) + + +@then('the Impact panel should show "{text}" for Active Plans') +def step_impact_plans(context: Any, text: str) -> None: + """Assert the Active Plans line in the Impact panel shows the expected text.""" + output = context.result.output + assert text in output, ( + f"Expected '{text}' in output for Active Plans, but got:\n{output}" + ) + + +@then('the Impact panel should show "{text}" for Actions Referencing') +def step_impact_actions(context: Any, text: str) -> None: + """Assert the Actions Referencing line in the Impact panel shows the expected text.""" + output = context.result.output + assert text in output, ( + f"Expected '{text}' in output for Actions Referencing, but got:\n{output}" + ) diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 8f4156644..dc01f6136 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -209,6 +209,73 @@ def _get_services(): return actor_service, actor_registry +def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: + """Compute the real impact counts for removing an actor. + + Queries the database to count: + - Sessions that reference the actor via ``actor_name`` + - Active plans (queued or processing) that reference the actor via + ``strategy_actor`` or ``execution_actor`` + - Actions configured to use the actor via any actor field + + Args: + actor_name: The namespaced actor name (e.g. ``local/my-actor``). + + Returns: + A 3-tuple of ``(session_count, active_plan_count, action_count)``. + Returns ``(0, 0, 0)`` if any query fails, to avoid blocking removal. + """ + container = get_container() + + # --- Sessions referencing this actor --- + session_count = 0 + try: + session_service = container.session_service() + sessions = session_service.list() + session_count = sum(1 for s in sessions if s.actor_name == actor_name) + except Exception: # pragma: no cover - defensive; DB may be unavailable + pass + + # --- Active lifecycle plans referencing this actor --- + active_plan_count = 0 + try: + uow = container.unit_of_work() + with uow.transaction() as ctx: + plans = ctx.lifecycle_plans.list_plans(limit=10000) + active_states = {"queued", "processing"} + active_plan_count = sum( + 1 + for p in plans + if p.processing_state in active_states + and (p.strategy_actor == actor_name or p.execution_actor == actor_name) + ) + except Exception: # pragma: no cover - defensive; DB may be unavailable + pass + + # --- Actions configured to use this actor --- + action_count = 0 + try: + lifecycle_service = container.plan_lifecycle_service() + actions = lifecycle_service.list_actions() + action_count = sum( + 1 + for a in actions + if actor_name + in ( + a.strategy_actor, + a.execution_actor, + getattr(a, "review_actor", None), + getattr(a, "apply_actor", None), + getattr(a, "estimation_actor", None), + getattr(a, "invariant_actor", None), + ) + ) + except Exception: # pragma: no cover - defensive; DB may be unavailable + pass + + return session_count, active_plan_count, action_count + + def _load_config(config_path: Path | None) -> dict[str, Any] | None: """Load a JSON or YAML config file if provided.""" @@ -672,6 +739,10 @@ def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> actor_provider = "unknown" actor_model = "unknown" + # Compute impact counts before removal so the display reflects + # what was actually affected at the time of removal. + session_count, active_plan_count, action_count = _compute_actor_impact(name) + # Perform the removal if registry: registry.remove_actor(name) @@ -687,14 +758,11 @@ def remove(name: Annotated[str, typer.Argument(help="Actor name to remove")]) -> actor_panel = Panel(actor_info, title="Actor Removed", border_style="green") console.print(actor_panel) - # Display Impact panel - # Note: Computing actual impact requires deep integration - # with session/plan subsystems - # For now, display panels with conservative estimates (0 affected) + # Display Impact panel with real computed counts impact_info = ( - "[yellow]Sessions:[/yellow] 0 affected\n" - "[yellow]Active Plans:[/yellow] 0 affected\n" - "[yellow]Actions Referencing:[/yellow] 0" + f"[yellow]Sessions:[/yellow] {session_count} affected\n" + f"[yellow]Active Plans:[/yellow] {active_plan_count} affected\n" + f"[yellow]Actions Referencing:[/yellow] {action_count}" ) impact_panel = Panel(impact_info, title="Impact", border_style="yellow") console.print(impact_panel)