From 67b282bd2360ca1a27ecf7bc7e156b2fa4ca26ac Mon Sep 17 00:00:00 2001 From: Test User Date: Sun, 19 Apr 2026 02:45:34 +0000 Subject: [PATCH 1/4] fix(error-handling): log exceptions in _compute_actor_impact instead of silently swallowing Replace three bare 'except Exception: pass' blocks in _compute_actor_impact() with proper exception handling that logs at WARNING level with exception type and message for diagnostics. The function still returns (0, 0, 0) on failure (graceful degradation) but failures are now visible in logs. Also adds BDD scenarios covering the error paths (DB unavailable -> warning logged, counts return 0) and removes the pragma: no cover annotations from the exception handlers. ISSUES CLOSED: #8434 --- ...ctor_compute_impact_error_handling.feature | 49 ++++ ...tor_compute_impact_error_handling_steps.py | 216 ++++++++++++++++++ src/cleveragents/cli/commands/actor.py | 35 ++- 3 files changed, 291 insertions(+), 9 deletions(-) create mode 100644 features/actor_compute_impact_error_handling.feature create mode 100644 features/steps/actor_compute_impact_error_handling_steps.py diff --git a/features/actor_compute_impact_error_handling.feature b/features/actor_compute_impact_error_handling.feature new file mode 100644 index 000000000..b8fe97078 --- /dev/null +++ b/features/actor_compute_impact_error_handling.feature @@ -0,0 +1,49 @@ +Feature: _compute_actor_impact logs exceptions instead of silently swallowing them + As an operator debugging actor removal issues + I want exceptions in _compute_actor_impact to be logged at WARNING level + So that I can diagnose database or service failures without silent data loss + + Background: + Given an actor CLI runner + + @mock_only + Scenario: Session query failure is logged at WARNING level and returns zero count + Given the session service raises an exception when listing sessions + When I call _compute_actor_impact for actor "local/my-actor" + Then the session count should be 0 + And a WARNING log message should contain "Failed to query session count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: Active plan query failure is logged at WARNING level and returns zero count + Given the unit of work raises an exception when listing plans + When I call _compute_actor_impact for actor "local/my-actor" + Then the active plan count should be 0 + And a WARNING log message should contain "Failed to query active plan count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: Action query failure is logged at WARNING level and returns zero count + Given the plan lifecycle service raises an exception when listing actions + When I call _compute_actor_impact for actor "local/my-actor" + Then the action count should be 0 + And a WARNING log message should contain "Failed to query action count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: All three queries succeed and return correct counts + Given the session service returns 2 sessions for actor "local/my-actor" + And the unit of work returns 1 active plan for actor "local/my-actor" + And the plan lifecycle service returns 3 actions for actor "local/my-actor" + When I call _compute_actor_impact for actor "local/my-actor" + Then the session count should be 2 + And the active plan count should be 1 + And the action count should be 3 + And no WARNING log messages should be emitted + + @mock_only + Scenario: Exception log message includes exception type and message for diagnostics + Given the session service raises a RuntimeError with message "DB connection refused" + When I call _compute_actor_impact for actor "local/my-actor" + Then a WARNING log message should contain "RuntimeError" + And a WARNING log message should contain "DB connection refused" diff --git a/features/steps/actor_compute_impact_error_handling_steps.py b/features/steps/actor_compute_impact_error_handling_steps.py new file mode 100644 index 000000000..2024b0c19 --- /dev/null +++ b/features/steps/actor_compute_impact_error_handling_steps.py @@ -0,0 +1,216 @@ +# pyrightconfig: reportRedeclaration=false +"""Step definitions for _compute_actor_impact error handling feature.""" + +from __future__ import annotations + +import logging +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 + + +@given("the session service raises an exception when listing sessions") +def step_session_service_raises(context: Any) -> None: + """Configure session service to raise an exception.""" + context.session_raises = RuntimeError("DB unavailable") + context.plan_raises = None + context.action_raises = None + context.session_count_override = None + context.plan_count_override = None + context.action_count_override = None + + +@given("the unit of work raises an exception when listing plans") +def step_uow_raises(context: Any) -> None: + """Configure unit of work to raise an exception.""" + context.session_raises = None + context.plan_raises = RuntimeError("DB unavailable") + context.action_raises = None + context.session_count_override = 0 + context.plan_count_override = None + context.action_count_override = None + + +@given("the plan lifecycle service raises an exception when listing actions") +def step_lifecycle_service_raises(context: Any) -> None: + """Configure plan lifecycle service to raise an exception.""" + context.session_raises = None + context.plan_raises = None + context.action_raises = RuntimeError("DB unavailable") + context.session_count_override = 0 + context.plan_count_override = 0 + context.action_count_override = None + + +@given('the session service returns {count:d} sessions for actor "{actor_name}"') +def step_session_service_returns(context: Any, count: int, actor_name: str) -> None: + """Configure session service to return a specific count.""" + context.session_raises = None + context.plan_raises = None + context.action_raises = None + context.session_count_override = count + context.plan_count_override = None + context.action_count_override = None + + +@given('the unit of work returns {count:d} active plan for actor "{actor_name}"') +def step_uow_returns_plan(context: Any, count: int, actor_name: str) -> None: + """Configure unit of work to return a specific active plan count.""" + context.plan_count_override = count + + +@given('the plan lifecycle service returns {count:d} actions for actor "{actor_name}"') +def step_lifecycle_returns_actions(context: Any, count: int, actor_name: str) -> None: + """Configure plan lifecycle service to return a specific action count.""" + context.action_count_override = count + + +@given('the session service raises a RuntimeError with message "{message}"') +def step_session_raises_with_message(context: Any, message: str) -> None: + """Configure session service to raise a RuntimeError with a specific message.""" + context.session_raises = RuntimeError(message) + context.plan_raises = None + context.action_raises = None + context.session_count_override = None + context.plan_count_override = None + context.action_count_override = None + + +@when('I call _compute_actor_impact for actor "{actor_name}"') +def step_call_compute_actor_impact(context: Any, actor_name: str) -> None: + """Call _compute_actor_impact with mocked services and capture log output.""" + from cleveragents.cli.commands.actor import _compute_actor_impact + + session_raises = getattr(context, "session_raises", None) + plan_raises = getattr(context, "plan_raises", None) + action_raises = getattr(context, "action_raises", None) + session_count_override = getattr(context, "session_count_override", 0) + plan_count_override = getattr(context, "plan_count_override", 0) + action_count_override = getattr(context, "action_count_override", 0) + + # Build mock container + mock_container = MagicMock() + + # Configure session service + if session_raises is not None: + mock_container.session_service.return_value.list.side_effect = session_raises + else: + count = session_count_override or 0 + mock_sessions = [MagicMock(actor_name=actor_name) for _ in range(count)] + mock_container.session_service.return_value.list.return_value = mock_sessions + + # Configure unit of work / plans + if plan_raises is not None: + mock_uow = MagicMock() + mock_uow.transaction.side_effect = plan_raises + mock_container.unit_of_work.return_value = mock_uow + else: + count = plan_count_override or 0 + mock_plans = [] + for _ in range(count): + p = MagicMock() + p.processing_state = "queued" + p.strategy_actor = actor_name + p.execution_actor = None + mock_plans.append(p) + mock_ctx = MagicMock() + mock_ctx.lifecycle_plans.list_plans.return_value = mock_plans + mock_uow = MagicMock() + mock_uow.transaction.return_value.__enter__ = MagicMock(return_value=mock_ctx) + mock_uow.transaction.return_value.__exit__ = MagicMock(return_value=False) + mock_container.unit_of_work.return_value = mock_uow + + # Configure plan lifecycle service / actions + if action_raises is not None: + mock_container.plan_lifecycle_service.return_value.list_actions.side_effect = ( + action_raises + ) + else: + count = action_count_override or 0 + mock_actions = [] + for _ in range(count): + a = MagicMock() + a.strategy_actor = actor_name + a.execution_actor = None + mock_actions.append(a) + mock_container.plan_lifecycle_service.return_value.list_actions.return_value = ( + mock_actions + ) + + # Capture log records + log_records: list[logging.LogRecord] = [] + + class CapturingHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + log_records.append(record) + + handler = CapturingHandler() + actor_logger = logging.getLogger("cleveragents.cli.commands.actor") + actor_logger.addHandler(handler) + original_level = actor_logger.level + actor_logger.setLevel(logging.WARNING) + + try: + with patch( + "cleveragents.cli.commands.actor.get_container", + return_value=mock_container, + ): + context.impact_result = _compute_actor_impact(actor_name) + finally: + actor_logger.removeHandler(handler) + actor_logger.setLevel(original_level) + + context.log_records = log_records + + +@then("the session count should be {count:d}") +def step_session_count(context: Any, count: int) -> None: + """Assert the session count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[0] + assert actual == count, f"Expected session count {count}, got {actual}" + + +@then("the active plan count should be {count:d}") +def step_active_plan_count(context: Any, count: int) -> None: + """Assert the active plan count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[1] + assert actual == count, f"Expected active plan count {count}, got {actual}" + + +@then("the action count should be {count:d}") +def step_action_count(context: Any, count: int) -> None: + """Assert the action count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[2] + assert actual == count, f"Expected action count {count}, got {actual}" + + +@then('a WARNING log message should contain "{text}"') +def step_warning_log_contains(context: Any, text: str) -> None: + """Assert that at least one WARNING log message contains the given text.""" + warning_messages = [ + r.getMessage() + for r in context.log_records + if r.levelno >= logging.WARNING + ] + assert any(text in msg for msg in warning_messages), ( + f"Expected a WARNING log message containing {text!r}, " + f"but got: {warning_messages}" + ) + + +@then("no WARNING log messages should be emitted") +def step_no_warning_logs(context: Any) -> None: + """Assert that no WARNING log messages were emitted.""" + warning_messages = [ + r.getMessage() + for r in context.log_records + if r.levelno >= logging.WARNING + ] + assert not warning_messages, ( + f"Expected no WARNING log messages, but got: {warning_messages}" + ) diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 1fe10c78e..0c491af0d 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import json +import logging from pathlib import Path from typing import Annotated, Any, cast @@ -48,6 +49,7 @@ app = typer.Typer( ) ) console = Console() +_logger = logging.getLogger(__name__) # Reusable --format option description _FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)" @@ -246,9 +248,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query session count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) # --- Active lifecycle plans referencing this actor --- active_plan_count = 0 @@ -268,9 +275,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query active plan count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) # --- Actions configured to use this actor --- action_count = 0 @@ -295,9 +307,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query action count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) return session_count, active_plan_count, action_count -- 2.52.0 From 341d912c9a6613ea5938ff48c8c3ecc07da05060 Mon Sep 17 00:00:00 2001 From: Test User Date: Sun, 19 Apr 2026 08:12:33 +0000 Subject: [PATCH 2/4] fix(test): add missing 'an actor CLI runner' step definition The actor_compute_impact_error_handling.feature file references the 'Given an actor CLI runner' step in its Background section, but the step definition was missing from the step file. This caused the tests to fail with an undefined step error. Added the missing step definition that initializes a CliRunner context for testing. Also removed the unused noqa comment from the CliRunner import since the import is now used in the step definition. --- .../steps/actor_compute_impact_error_handling_steps.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/features/steps/actor_compute_impact_error_handling_steps.py b/features/steps/actor_compute_impact_error_handling_steps.py index 2024b0c19..e4234b94e 100644 --- a/features/steps/actor_compute_impact_error_handling_steps.py +++ b/features/steps/actor_compute_impact_error_handling_steps.py @@ -8,7 +8,13 @@ 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 typer.testing import CliRunner + + +@given("an actor CLI runner") +def step_actor_cli_runner(context: Any) -> None: + """Initialize an actor CLI runner for testing.""" + context.runner = CliRunner() @given("the session service raises an exception when listing sessions") -- 2.52.0 From 86c6d5e40ef1eba750bfc7ef5b572a4453b01994 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Fri, 24 Apr 2026 05:12:57 +0000 Subject: [PATCH 3/4] fix(error-handling): log exceptions in _compute_actor_impact instead of silently swallowing --- .../steps/actor_compute_impact_error_handling_steps.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/features/steps/actor_compute_impact_error_handling_steps.py b/features/steps/actor_compute_impact_error_handling_steps.py index e4234b94e..306bacc2d 100644 --- a/features/steps/actor_compute_impact_error_handling_steps.py +++ b/features/steps/actor_compute_impact_error_handling_steps.py @@ -8,13 +8,6 @@ from typing import Any from unittest.mock import MagicMock, patch from behave import given, then, when -from typer.testing import CliRunner - - -@given("an actor CLI runner") -def step_actor_cli_runner(context: Any) -> None: - """Initialize an actor CLI runner for testing.""" - context.runner = CliRunner() @given("the session service raises an exception when listing sessions") -- 2.52.0 From eb454d842168ebf5ce39b42ea31d6b58f53bb99d Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Fri, 24 Apr 2026 15:02:24 +0000 Subject: [PATCH 4/4] style(test): apply ruff format to actor_compute_impact_error_handling_steps.py --- .../steps/actor_compute_impact_error_handling_steps.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/features/steps/actor_compute_impact_error_handling_steps.py b/features/steps/actor_compute_impact_error_handling_steps.py index 306bacc2d..05ca2e4f6 100644 --- a/features/steps/actor_compute_impact_error_handling_steps.py +++ b/features/steps/actor_compute_impact_error_handling_steps.py @@ -192,9 +192,7 @@ def step_action_count(context: Any, count: int) -> None: def step_warning_log_contains(context: Any, text: str) -> None: """Assert that at least one WARNING log message contains the given text.""" warning_messages = [ - r.getMessage() - for r in context.log_records - if r.levelno >= logging.WARNING + r.getMessage() for r in context.log_records if r.levelno >= logging.WARNING ] assert any(text in msg for msg in warning_messages), ( f"Expected a WARNING log message containing {text!r}, " @@ -206,9 +204,7 @@ def step_warning_log_contains(context: Any, text: str) -> None: def step_no_warning_logs(context: Any) -> None: """Assert that no WARNING log messages were emitted.""" warning_messages = [ - r.getMessage() - for r in context.log_records - if r.levelno >= logging.WARNING + r.getMessage() for r in context.log_records if r.levelno >= logging.WARNING ] assert not warning_messages, ( f"Expected no WARNING log messages, but got: {warning_messages}" -- 2.52.0