From 669013b1e2e091664c425ce3c6146e814af37350 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Sun, 19 Apr 2026 11:16:56 +0000 Subject: [PATCH] test(cli): add failing TDD test for session create silently suppressing facade dispatch exceptions Added a new BDD feature file: features/tdd_session_create_suppress_exception.feature Added step definitions: features/steps/tdd_session_create_suppress_exception_steps.py The test captures bug #10414: session create uses contextlib.suppress(Exception) to silently discard ALL exceptions from _facade_dispatch() without any logging The test is tagged @tdd_issue @tdd_issue_10414 @tdd_expected_fail per TDD workflow The test FAILS before the fix (no logging currently) and the @tdd_expected_fail tag inverts it to a CI pass The test will pass after the fix adds _log.warning(..., exc_info=True) inside the suppress block ISSUES CLOSED: #10414 --- ...session_create_suppress_exception_steps.py | 156 ++++++++++++++++++ ..._session_create_suppress_exception.feature | 30 ++++ 2 files changed, 186 insertions(+) create mode 100644 features/steps/tdd_session_create_suppress_exception_steps.py create mode 100644 features/tdd_session_create_suppress_exception.feature 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..ee5001be8 --- /dev/null +++ b/features/steps/tdd_session_create_suppress_exception_steps.py @@ -0,0 +1,156 @@ +"""Step definitions for tdd_session_create_suppress_exception.feature. + +This test captures bug #10414: ``session create`` in +``src/cleveragents/cli/commands/session.py`` uses +``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 are +completely invisible. + +The scenario is tagged ``@tdd_expected_fail`` so the underlying assertion +failure (confirming the bug exists) is inverted to a CI pass. Once the +fix for #10414 is merged, remove the ``@tdd_expected_fail`` tag from the +feature file and this test will run normally as a regression guard. +""" + +from __future__ import annotations + +import logging +from unittest.mock import MagicMock, patch + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] +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 = "01JTEST00000000000000000000" + mock_session.actor_name = None + mock_session.namespace = "default" + mock_session.message_count = 0 + mock_session.created_at = __import__("datetime").datetime(2026, 1, 1, 0, 0, 0) + mock_session.updated_at = __import__("datetime").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`` + suppress 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 ``contextlib.suppress`` + 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. + + Bug #10414: The ``contextlib.suppress(Exception)`` block in + ``session create`` does NOT log the suppressed exception. This + assertion therefore FAILS while the bug exists, and the + ``@tdd_expected_fail`` tag causes CI to report the scenario as passed. + + Once the fix is applied (adding ``_log.warning(..., exc_info=True)`` + inside the suppress block), this assertion will pass and the + ``@tdd_expected_fail`` tag must be removed. + """ + # 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 contextlib.suppress(). " + f"The exception was silently discarded with no logging. " + 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..76d3b9323 --- /dev/null +++ b/features/tdd_session_create_suppress_exception.feature @@ -0,0 +1,30 @@ +@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`` (lines 199–205) uses + ``contextlib.suppress(Exception)`` to silently discard ALL exceptions + raised by ``_facade_dispatch()``. There is no logging call before or + after the suppress block, making it impossible to diagnose failures in + the facade layer. + + The fix must add a ``_log.warning(...)`` call (with ``exc_info=True``) + inside the suppress block so that the exception is recorded but the + session creation remains non-fatal. + + The scenario below is tagged ``@tdd_expected_fail`` because the assertion + FAILS while the bug exists (no logging currently). Once the fix is + merged, remove ``@tdd_expected_fail`` so the scenario runs as a + regression guard. + + See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags. + + @tdd_issue @tdd_issue_10414 @tdd_expected_fail + 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 -- 2.52.0