From 2d519182ca9aec5af7d643b4534ff02167994f7c Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Tue, 28 Apr 2026 09:09:16 +0000 Subject: [PATCH 1/3] fix(cli): log suppressed facade dispatch exceptions in session create Replace the silent contextlib.suppress(Exception) block in the session create command with a try/except that logs at WARNING level with exc_info. Previously, any exception raised by _facade_dispatch() during session creation was silently discarded with no logging, making it impossible to diagnose failures in the facade layer (e.g. A2A bootstrap unavailable, programming errors, infrastructure issues). The fix keeps session creation non-fatal (the session is already persisted before the facade call) while ensuring the exception is visible in logs: try: _facade_dispatch(session.create, {...}) except Exception as _exc: _log.warning( session_create_facade_dispatch_failed, extra={"session_id": ..., "error": str(_exc)}, exc_info=True, ) Also includes the TDD regression test from issue #10414 (PR #10749) with @tdd_expected_fail removed, since the fix is now applied. The test verifies that a WARNING log entry is emitted when _facade_dispatch() raises and that session creation still exits 0. ISSUES CLOSED: #10433 --- ...session_create_suppress_exception_steps.py | 151 ++++++++++++++++++ ..._session_create_suppress_exception.feature | 25 +++ src/cleveragents/cli/commands/session.py | 10 +- 3 files changed, 183 insertions(+), 3 deletions(-) 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..a41c535e0 --- /dev/null +++ b/features/steps/tdd_session_create_suppress_exception_steps.py @@ -0,0 +1,151 @@ +"""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 + +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 = __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`` + 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) -- 2.52.0 From 08422b4dedb9f7de09a8989a436a6b549aaa4d9e Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Wed, 29 Apr 2026 18:50:22 +0000 Subject: [PATCH 2/3] fix(cli): log suppressed facade dispatch exceptions in session create Replace __import__("datetime") with proper top-level import per project Python import rules. This resolves the CI lint failure blocking PR #10898. Closes #10433 Refs: #10414 #10898 --- .../steps/tdd_session_create_suppress_exception_steps.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/features/steps/tdd_session_create_suppress_exception_steps.py b/features/steps/tdd_session_create_suppress_exception_steps.py index a41c535e0..1044d19db 100644 --- a/features/steps/tdd_session_create_suppress_exception_steps.py +++ b/features/steps/tdd_session_create_suppress_exception_steps.py @@ -14,6 +14,7 @@ recorded but the session creation remains non-fatal. from __future__ import annotations +from datetime import datetime import logging from unittest.mock import MagicMock, patch @@ -36,8 +37,8 @@ def _make_mock_session() -> MagicMock: 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) + 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 -- 2.52.0 From d4ebb482e10c72dd031a48c6baa31dc1bb669f49 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 5 May 2026 16:01:30 +0000 Subject: [PATCH 3/3] style(tests): fix ruff format violation in TDD step file Collapse multi-line @then decorator onto a single line in tdd_session_create_suppress_exception_steps.py to satisfy ruff format --check (the CI lint job runs both ruff check and ruff format --check). --- features/steps/tdd_session_create_suppress_exception_steps.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/features/steps/tdd_session_create_suppress_exception_steps.py b/features/steps/tdd_session_create_suppress_exception_steps.py index 1044d19db..25c2176a4 100644 --- a/features/steps/tdd_session_create_suppress_exception_steps.py +++ b/features/steps/tdd_session_create_suppress_exception_steps.py @@ -114,9 +114,7 @@ class _CapturingHandler(logging.Handler): self.records.append(record) -@then( - "a WARNING log entry should have been emitted for the facade dispatch failure" -) +@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. -- 2.52.0