diff --git a/features/steps/tdd_session_create_suppress_exception_steps.py b/features/steps/tdd_session_create_suppress_exception_steps.py new file mode 100644 index 000000000..25c2176a4 --- /dev/null +++ b/features/steps/tdd_session_create_suppress_exception_steps.py @@ -0,0 +1,150 @@ +"""Step definitions for tdd_session_create_suppress_exception.feature. + +This test captures bug #10414: ``session create`` in +``src/cleveragents/cli/commands/session.py`` used +``contextlib.suppress(Exception)`` to silently discard ALL exceptions raised +by ``_facade_dispatch()`` without any logging. Programming errors, +unexpected failures, and infrastructure issues in the facade dispatch were +completely invisible. + +The fix replaces the suppress block with a ``try/except Exception`` that +calls ``_log.warning(..., exc_info=True)`` so that the exception is +recorded but the session creation remains non-fatal. +""" + +from __future__ import annotations + +from datetime import datetime +import logging +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands import session as session_mod +from cleveragents.cli.commands.session import app as session_app +from cleveragents.domain.models.core.session import Session + +# A distinctive error message to confirm the right exception was raised. +_DISPATCH_ERROR_MESSAGE = "facade dispatch failed for tdd_10414" + + +def _make_mock_session() -> MagicMock: + """Build a minimal mock Session object sufficient for the create command.""" + mock_session = MagicMock(spec=Session) + mock_session.session_id = "01JTEST000000000000000000000" + mock_session.actor_name = None + mock_session.namespace = "default" + mock_session.message_count = 0 + mock_session.created_at = datetime(2026, 1, 1, 0, 0, 0) + mock_session.updated_at = datetime(2026, 1, 1, 0, 0, 0) + return mock_session + + +@given("a session create command with a mocked session service") +def step_given_mocked_session_service(context: Context) -> None: + """Set up a CLI runner with a mocked session service. + + The mock service returns a valid session so that the create command + proceeds past the service call and reaches the ``_facade_dispatch`` + try/except block. + """ + context.runner = CliRunner() + + mock_service = MagicMock() + mock_service.create.return_value = _make_mock_session() + + # Patch the module-level service so the CLI uses our mock. + context._orig_service = session_mod._service + session_mod._service = mock_service + + def _cleanup() -> None: + session_mod._service = context._orig_service + + context.add_cleanup(_cleanup) + + +@given("the facade dispatch is patched to raise a RuntimeError") +def step_given_facade_dispatch_raises(context: Context) -> None: + """Patch ``_facade_dispatch`` so it always raises a RuntimeError. + + This simulates a real failure in the facade layer (e.g. the A2A + bootstrap is unavailable) and exercises the ``try/except`` block in + the ``create`` command. + """ + context._facade_patcher = patch( + "cleveragents.cli.commands.session._facade_dispatch", + side_effect=RuntimeError(_DISPATCH_ERROR_MESSAGE), + ) + context._facade_patcher.start() + + def _cleanup() -> None: + context._facade_patcher.stop() + + context.add_cleanup(_cleanup) + + +@when("I invoke the session create command via the CLI runner") +def step_when_invoke_create(context: Context) -> None: + """Invoke ``session create`` and capture log records during the call.""" + # Capture WARNING-level log records from the session module logger. + session_logger = logging.getLogger("cleveragents.cli.commands.session") + handler = _CapturingHandler() + handler.setLevel(logging.WARNING) + session_logger.addHandler(handler) + session_logger.setLevel(logging.WARNING) + + try: + context.result = context.runner.invoke(session_app, ["create"]) + finally: + session_logger.removeHandler(handler) + + context.captured_log_records = handler.records + + +class _CapturingHandler(logging.Handler): + """A logging handler that stores all emitted records in a list.""" + + def __init__(self) -> None: + super().__init__() + self.records: list[logging.LogRecord] = [] + + def emit(self, record: logging.LogRecord) -> None: + self.records.append(record) + + +@then("a WARNING log entry should have been emitted for the facade dispatch failure") +def step_then_warning_logged(context: Context) -> None: + """Assert that a WARNING log entry was emitted when the facade dispatch raised. + + The fix replaces ``contextlib.suppress(Exception)`` with a + ``try/except Exception`` block that calls ``_log.warning(..., exc_info=True)``. + This assertion verifies the fix is in place. + """ + # The session create command should still succeed (non-fatal). + assert context.result.exit_code == 0, ( + f"Expected session create to exit 0 even when facade dispatch fails, " + f"but got exit code {context.result.exit_code}.\n" + f"Output:\n{context.result.output}" + ) + + # Assert that a WARNING log record was emitted. + warning_records = [ + r for r in context.captured_log_records if r.levelno >= logging.WARNING + ] + assert warning_records, ( + f"Bug #10414: No WARNING log record was emitted when _facade_dispatch() " + f"raised a RuntimeError inside the try/except block. " + f"Captured records: {context.captured_log_records}" + ) + + # Assert the log record references the facade dispatch failure. + all_messages = " ".join(r.getMessage() for r in warning_records) + assert _DISPATCH_ERROR_MESSAGE in all_messages or any( + r.exc_info is not None for r in warning_records + ), ( + f"Bug #10414: WARNING log record was found but did not contain the " + f"expected error message {_DISPATCH_ERROR_MESSAGE!r} or exc_info. " + f"Log messages: {all_messages!r}" + ) diff --git a/features/tdd_session_create_suppress_exception.feature b/features/tdd_session_create_suppress_exception.feature new file mode 100644 index 000000000..2b15bc127 --- /dev/null +++ b/features/tdd_session_create_suppress_exception.feature @@ -0,0 +1,25 @@ +@tdd_issue @tdd_issue_10414 +Feature: TDD Issue #10414 — session create silently suppresses facade dispatch exceptions without logging + As a developer debugging a session creation failure + I want exceptions from _facade_dispatch() to be logged at WARNING level + So that I can diagnose why the facade layer failed without silent data loss + + The ``session create`` command in + ``src/cleveragents/cli/commands/session.py`` previously used + ``contextlib.suppress(Exception)`` to silently discard ALL exceptions + raised by ``_facade_dispatch()``. There was no logging call before or + after the suppress block, making it impossible to diagnose failures in + the facade layer. + + The fix replaces the suppress block with a ``try/except Exception`` that + calls ``_log.warning(..., exc_info=True)`` so that the exception is + recorded but the session creation remains non-fatal. + + See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. + + @tdd_issue @tdd_issue_10414 + Scenario: Bug #10414 — session create logs a warning when facade dispatch raises + Given a session create command with a mocked session service + And the facade dispatch is patched to raise a RuntimeError + When I invoke the session create command via the CLI runner + Then a WARNING log entry should have been emitted for the facade dispatch failure diff --git a/src/cleveragents/cli/commands/session.py b/src/cleveragents/cli/commands/session.py index 30202e6da..27a9f1c99 100644 --- a/src/cleveragents/cli/commands/session.py +++ b/src/cleveragents/cli/commands/session.py @@ -213,13 +213,17 @@ def create( # Notify the facade layer for A2A protocol bookkeeping. # Pass session_id so the facade handler acknowledges the already- # persisted session instead of creating a duplicate (#1141). - import contextlib - - with contextlib.suppress(Exception): + try: _facade_dispatch( "session.create", {"actor_name": actor or "", "session_id": session.session_id}, ) + except Exception as _exc: + _log.warning( + "session_create_facade_dispatch_failed", + extra={"session_id": session.session_id, "error": str(_exc)}, + exc_info=True, + ) data = _session_summary_dict(session)