CHANGELOG.md

@@ -21,6 +21,17 @@
 - Polymorphic handler resolution with ancestor-type fallback
 - CLI: `agents resource type list` shows Inherits column; `type show` displays inheritance chain
 - Alembic migration `m6_004_resource_type_inherits` adds `inherits` column to `resource_types`
+- Added TDD regression tests for `agents session list` DI container wiring
+  error (bug #554).  `_get_session_service()` calls `container.db()` but the
+  `Container` class has no `db` provider, raising `AttributeError`.  Includes
+  10 Behave BDD scenarios (`@tdd_bug @tdd_bug_554 @tdd_expected_fail`)
+  covering empty list, empty-list format validation (JSON/YAML/plain),
+  init-then-list lifecycle, post-create list, rich/JSON/plain/YAML output
+  formats, and stderr error-path assertions.  Robot Framework integration
+  smoke tests and ASV service-layer benchmarks.  Implements
+  `@tdd_expected_fail` infrastructure (Behave `after_scenario` hook and Robot
+  listener) and migrates 18 existing TDD scenarios from `@tdd @bugNNN` to
+  `@tdd_bug @tdd_bug_NNN` convention.  (#554)
 - Added TDD regression tests for `agents session create` DI container wiring
   error (bug #570).  `_get_session_service()` calls `container.db()` but the
   `Container` class has no `db` provider, raising `AttributeError`.  Same root

benchmarks/session_list_bench.py

@@ -0,0 +1,157 @@
+"""ASV benchmarks for session list service-layer performance baseline (bug #554).
+
+Measures the cost of listing sessions through ``PersistentSessionService``
+using a file-based SQLite database.  These benchmarks construct the service
+directly (bypassing DI wiring) and therefore do **not** exercise the
+``_get_session_service()`` code path that triggers bug #554.  Their purpose
+is to establish a performance baseline for the service layer itself.
+"""
+
+from __future__ import annotations
+
+import os
+import shutil
+import sys
+import tempfile
+from pathlib import Path
+
+_SRC = str(Path(__file__).resolve().parents[1] / "src")
+if _SRC not in sys.path:
+    sys.path.insert(0, _SRC)
+
+from sqlalchemy import create_engine  # noqa: E402
+from sqlalchemy.orm import sessionmaker  # noqa: E402
+
+from cleveragents.application.services.session_service import (  # noqa: E402
+    PersistentSessionService,
+)
+from cleveragents.infrastructure.database.models import Base  # noqa: E402
+from cleveragents.infrastructure.database.repositories import (  # noqa: E402
+    SessionMessageRepository,
+    SessionRepository,
+)
+
+
+class SessionListDISuite:
+    """Benchmark session list through the service layer (direct construction).
+
+    Engine and sessionmaker are built once in ``setup()`` and reused across
+    ``time_list_empty`` iterations.  Methods that mutate state
+    (``time_list_after_create``, ``track_list_after_create_count``) use
+    per-method setup/teardown to get a fresh database each time, preventing
+    row accumulation across ASV iterations.
+
+    Does **not** exercise ``_get_session_service()`` or the DI container
+    wiring.
+    """
+
+    timeout = 30.0
+
+    # -- suite-level setup (shared engine for read-only benchmarks) ----------
+
+    def setup(self) -> None:
+        self._tmpdir = tempfile.mkdtemp(prefix="bench_sle_554_")
+        self._db_path = os.path.join(self._tmpdir, "bench.db")
+        self._engine = create_engine(
+            f"sqlite:///{self._db_path}",
+            echo=False,
+        )
+        Base.metadata.create_all(self._engine)
+        self._session_factory = sessionmaker(
+            bind=self._engine,
+            expire_on_commit=False,
+        )
+
+    def teardown(self) -> None:
+        self._engine.dispose()
+        shutil.rmtree(self._tmpdir, ignore_errors=True)
+
+    def _make_service(self) -> PersistentSessionService:
+        """Build a PersistentSessionService using the shared session factory."""
+        return PersistentSessionService(
+            session_repo=SessionRepository(
+                session_factory=self._session_factory,
+            ),
+            message_repo=SessionMessageRepository(
+                session_factory=self._session_factory,
+            ),
+        )
+
+    # -- per-method setup for mutating benchmarks ----------------------------
+
+    def _fresh_engine(self) -> None:
+        """Create an isolated engine+factory so create() doesn't accumulate."""
+        self._mut_tmpdir = tempfile.mkdtemp(prefix="bench_sle_554_mut_")
+        db_path = os.path.join(self._mut_tmpdir, "bench.db")
+        self._mut_engine = create_engine(
+            f"sqlite:///{db_path}",
+            echo=False,
+        )
+        Base.metadata.create_all(self._mut_engine)
+        self._mut_factory = sessionmaker(
+            bind=self._mut_engine,
+            expire_on_commit=False,
+        )
+
+    def _dispose_fresh(self) -> None:
+        if hasattr(self, "_mut_engine"):
+            self._mut_engine.dispose()
+        if hasattr(self, "_mut_tmpdir"):
+            shutil.rmtree(self._mut_tmpdir, ignore_errors=True)
+
+    def _make_mut_service(self) -> PersistentSessionService:
+        return PersistentSessionService(
+            session_repo=SessionRepository(session_factory=self._mut_factory),
+            message_repo=SessionMessageRepository(
+                session_factory=self._mut_factory,
+            ),
+        )
+
+    # -- ASV per-method hooks ------------------------------------------------
+
+    def setup_time_list_after_create(self) -> None:
+        self._fresh_engine()
+
+    def teardown_time_list_after_create(self) -> None:
+        self._dispose_fresh()
+
+    def setup_track_list_after_create_count(self) -> None:
+        self._fresh_engine()
+
+    def teardown_track_list_after_create_count(self) -> None:
+        self._dispose_fresh()
+
+    def setup_time_list_empty(self) -> None:
+        self._empty_svc = self._make_service()
+
+    # -- benchmarks ----------------------------------------------------------
+
+    def time_list_empty(self) -> None:
+        """List sessions when DB is empty (service-layer only, no DI)."""
+        self._empty_svc.list()
+
+    def time_list_after_create(self) -> None:
+        """Create then list (service-layer only, fresh DB per iteration).
+
+        Bypasses ``_get_session_service()`` / DI container — measures the
+        service-layer round-trip cost, not the DI wiring path.
+        """
+        svc = self._make_mut_service()
+        svc.create()
+        svc.list()
+
+    def track_list_after_create_count(self) -> int:
+        """Track session list persistence at the service layer.
+
+        Returns the count of sessions visible after a create.  Uses a
+        fresh DB per iteration so the count is always exactly 1.
+        This benchmark constructs ``PersistentSessionService`` directly,
+        bypassing the DI container — it does **not** reproduce bug #554.
+        """
+        svc = self._make_mut_service()
+        svc.create(actor_name="bench/test")
+        sessions = svc.list()
+        return len(sessions)
+
+
+SessionListDISuite.track_list_after_create_count.unit = "sessions"

features/session_list_error.feature

@@ -0,0 +1,85 @@
+# TDD tests for bug #554 — expected to fail until the DI container fix lands.
+# Once the fix is applied, remove the @tdd_expected_fail tags and verify all
+# scenarios pass.
+Feature: Session list command handles missing database gracefully
+  As a developer using the agents CLI
+  I want "agents session list" to work after a fresh init
+  So that I can view my sessions without a DI container error
+
+  Background:
+    Given a session-list-error CLI runner using the real DI path
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list returns empty list when no sessions exist
+    When I invoke session-list-error list with default format
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should contain "No sessions found"
+    And the session-list-error output should not contain "AttributeError"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list after init does not raise DI error
+    When I invoke session-list-error list with default format
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should not contain "AttributeError"
+    And the session-list-error output should not contain "INTERNAL"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list returns sessions after creation via service
+    Given a session-list-error service with a pre-populated session
+    When I invoke session-list-error list with default format
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should contain "Sessions ("
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list works with rich output format
+    Given a session-list-error service with a pre-populated session
+    When I invoke session-list-error list with format "rich"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should contain "Sessions ("
+    And the session-list-error output should not contain "AttributeError"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list works with JSON output format
+    Given a session-list-error service with a pre-populated session
+    When I invoke session-list-error list with format "json"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should be valid JSON containing "sessions"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list works with plain output format
+    Given a session-list-error service with a pre-populated session
+    When I invoke session-list-error list with format "plain"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should contain "Sessions ("
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Session list works with YAML output format
+    Given a session-list-error service with a pre-populated session
+    When I invoke session-list-error list with format "yaml"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should be valid YAML containing "sessions"
+
+  # Empty-list format scenarios (F2/F3) — exercises the empty-list code path
+  # with explicit output formats.  The production code currently bypasses
+  # --format for empty lists, so these document the expected behaviour.
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Empty session list with JSON format produces valid JSON
+    When I invoke session-list-error list with format "json"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should be valid JSON containing "sessions"
+    And the session-list-error output should not contain "AttributeError"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Empty session list with YAML format produces valid YAML
+    When I invoke session-list-error list with format "yaml"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should be valid YAML containing "sessions"
+    And the session-list-error output should not contain "AttributeError"
+
+  @tdd_bug @tdd_bug_554 @tdd_expected_fail
+  Scenario: Empty session list with plain format does not error
+    When I invoke session-list-error list with format "plain"
+    Then the session-list-error command should exit successfully
+    And the session-list-error output should contain "No sessions found"
+    And the session-list-error output should not contain "AttributeError"

features/steps/session_list_error_steps.py

@@ -0,0 +1,217 @@
+"""Step definitions for session_list_error.feature (bug #554).
+
+TDD regression tests for ``agents session list`` after ``agents init``.
+These scenarios assert the correct expected behaviour and will fail until
+the DI container fix is applied.
+
+Design rationale
+~~~~~~~~~~~~~~~~
+``_get_session_service()`` calls ``container.db()`` but the DI ``Container``
+class has no ``db`` provider, raising ``AttributeError``.
+
+We reset ``_service`` to ``None`` so the real ``_get_session_service()`` is
+exercised.  A file-based SQLite database and ``CLEVERAGENTS_DATABASE_URL``
+override ensure the commands can reach the database once the fix lands.
+
+Private API access
+~~~~~~~~~~~~~~~~~~
+This module accesses ``session_mod._service`` (module-level singleton cache)
+to force the real ``_get_session_service()`` code path during tests.  This is
+intentional: the public API (``CliRunner.invoke``) does not expose the DI
+wiring that triggers the bug, so we must bypass the cache to exercise it.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import shutil
+import tempfile
+
+import yaml
+from behave import given, then, when
+from behave.runner import Context
+from sqlalchemy import create_engine
+from sqlalchemy.orm import scoped_session, sessionmaker
+from typer.testing import CliRunner
+
+from cleveragents.application.container import reset_container
+from cleveragents.application.services.session_service import (
+    PersistentSessionService,
+)
+from cleveragents.cli.commands import session as session_mod
+from cleveragents.cli.commands.session import app as session_app
+from cleveragents.infrastructure.database.models import Base
+from cleveragents.infrastructure.database.repositories import (
+    SessionMessageRepository,
+    SessionRepository,
+)
+
+runner = CliRunner()
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _setup_real_di_path(context: Context) -> None:
+    """Prepare a temp dir with a fresh SQLite DB and override container."""
+    # Store original _service so cleanup can restore it.
+    context.sle_original_service = session_mod._service
+
+    # Reset any stale DI container singleton before configuring the env var
+    # so that cached providers don't carry over from a prior test suite (F17).
+    reset_container()
+
+    context.sle_tmpdir = tempfile.mkdtemp(prefix="session_list_err_554_")
+
+    # Register cleanup immediately after mkdtemp so the temp directory is
+    # always removed even if the rest of setup fails (F21).
+    context.add_cleanup(_cleanup_sle, context)
+
+    context.sle_db_path = os.path.join(context.sle_tmpdir, "test.db")
+    db_url = f"sqlite:///{context.sle_db_path}"
+
+    # Create schema so the DB file exists with all tables.
+    engine = create_engine(db_url, echo=False)
+    try:
+        Base.metadata.create_all(engine)
+    finally:
+        engine.dispose()
+
+    # Override the container's database_url so real DI can find the DB.
+    os.environ["CLEVERAGENTS_DATABASE_URL"] = db_url
+
+    # Reset the module-level _service so _get_session_service() is used.
+    # This direct attribute mutation is fragile — if the module's internal
+    # caching mechanism changes (e.g. lazy singleton via descriptor), this
+    # line will need to be updated.  See module docstring for rationale.
+    session_mod._service = None
+
+    context.sle_result = None
+
+
+def _cleanup_sle(context: Context) -> None:
+    """Remove temp dir, restore env, original _service, and container."""
+    session_mod._service = context.sle_original_service
+    os.environ.pop("CLEVERAGENTS_DATABASE_URL", None)
+
+    # Reset the DI container singleton to avoid polluting later scenarios.
+    reset_container()
+
+    shutil.rmtree(context.sle_tmpdir, ignore_errors=True)
+
+
+# ---------------------------------------------------------------------------
+# Background
+# ---------------------------------------------------------------------------
+
+
+@given("a session-list-error CLI runner using the real DI path")
+def step_session_list_error_runner(context: Context) -> None:
+    _setup_real_di_path(context)
+
+
+# ---------------------------------------------------------------------------
+# Given - pre-populated session
+# ---------------------------------------------------------------------------
+
+
+@given("a session-list-error service with a pre-populated session")
+def step_pre_populate_session(context: Context) -> None:
+    """Insert a session directly via the repository so list has data."""
+    db_url = f"sqlite:///{context.sle_db_path}"
+    engine = create_engine(db_url, echo=False)
+    try:
+        factory = scoped_session(
+            sessionmaker(bind=engine, expire_on_commit=False),
+        )
+        repo = SessionRepository(session_factory=factory)
+        msg_repo = SessionMessageRepository(session_factory=factory)
+        svc = PersistentSessionService(repo, msg_repo)
+        svc.create(actor_name="openai/gpt-4")
+        # Commit via the scoped session so the data is visible to later queries.
+        factory().commit()
+        factory.remove()
+    finally:
+        engine.dispose()
+
+
+# ---------------------------------------------------------------------------
+# When - list
+# ---------------------------------------------------------------------------
+
+
+@when("I invoke session-list-error list with default format")
+def step_invoke_list_default(context: Context) -> None:
+    context.sle_result = runner.invoke(session_app, ["list"])
+
+
+@when('I invoke session-list-error list with format "{fmt}"')
+def step_invoke_list_format(context: Context, fmt: str) -> None:
+    context.sle_result = runner.invoke(session_app, ["list", "--format", fmt])
+
+
+# ---------------------------------------------------------------------------
+# Then - assertions
+# ---------------------------------------------------------------------------
+
+
+@then("the session-list-error command should exit successfully")
+def step_exit_success(context: Context) -> None:
+    result = context.sle_result
+    assert result is not None, "No command was invoked"
+    assert result.exit_code == 0, (
+        f"Expected exit code 0, got {result.exit_code}.\n"
+        f"Output: {result.output}\n"
+        f"Exception: {result.exception!r}"
+    )
+
+
+@then('the session-list-error output should contain "{text}"')
+def step_output_contains(context: Context, text: str) -> None:
+    result = context.sle_result
+    assert result is not None, "No command was invoked"
+    assert text in result.output, (
+        f"Expected '{text}' in output but got:\n{result.output}"
+    )
+
+
+@then('the session-list-error output should not contain "{text}"')
+def step_output_not_contains(context: Context, text: str) -> None:
+    result = context.sle_result
+    assert result is not None, "No command was invoked"
+    assert text not in result.output, (
+        f"Did not expect '{text}' in output but found it:\n{result.output}"
+    )
+
+
+@then('the session-list-error output should be valid JSON containing "{key}"')
+def step_output_json_key(context: Context, key: str) -> None:
+    result = context.sle_result
+    assert result is not None, "No command was invoked"
+    try:
+        data = json.loads(result.output)
+    except json.JSONDecodeError as exc:
+        raise AssertionError(f"Output is not valid JSON:\n{result.output}") from exc
+    assert isinstance(data, dict), f"Expected JSON object, got {type(data)}: {data}"
+    assert key in data, f"Key '{key}' not in JSON: {data}"
+    assert isinstance(data[key], list), (
+        f"Expected '{key}' to be a list, got {type(data[key])}: {data[key]}"
+    )
+
+
+@then('the session-list-error output should be valid YAML containing "{key}"')
+def step_output_yaml_key(context: Context, key: str) -> None:
+    result = context.sle_result
+    assert result is not None, "No command was invoked"
+    try:
+        data = yaml.safe_load(result.output)
+    except yaml.YAMLError as exc:
+        raise AssertionError(f"Output is not valid YAML:\n{result.output}") from exc
+    assert isinstance(data, dict), f"Expected YAML dict, got {type(data)}: {data}"
+    assert key in data, f"Key '{key}' not in YAML: {data}"
+    assert isinstance(data[key], list), (
+        f"Expected '{key}' to be a list, got {type(data[key])}: {data[key]}"
+    )

robot/session_list_error.robot

@@ -0,0 +1,58 @@
+*** Settings ***
+Documentation     Integration smoke test for session list DI error (bug #554).
+...               TDD-style tests — expected to FAIL until the DI container fix
+...               lands.  The bug is that ``_get_session_service()`` calls
+...               ``container.db()`` but the DI container has no ``db`` provider,
+...               causing an ``AttributeError``.
+Resource          ${CURDIR}/common.resource
+Library           Process
+Library           OperatingSystem
+Suite Setup       Setup Test Environment
+Suite Teardown    Cleanup Test Environment
+
+*** Comments ***
+# NOTE: ``Should Contain`` and ``Should Not Contain`` are case-sensitive
+# by default in Robot Framework.  The assertions below rely on the exact
+# output format produced by the CLI:
+#   - "AttributeError" (Python exception class name, title-case)
+# If the production code changes its error message casing, update these
+# assertions accordingly.
+
+*** Test Cases ***
+Session List After Init Should Not Error
+    [Documentation]    After agents init, session list should exit 0 and show
+    ...                "No sessions found" rather than a DI AttributeError.
+    [Tags]    tdd_bug    tdd_bug_554    tdd_expected_fail
+    ${tmpdir}=    Evaluate    __import__('tempfile').mkdtemp(prefix='sle_554_')
+    ${init}=    Run Process    ${PYTHON}    -m    cleveragents    init    sle-test
+    ...    timeout=60s    cwd=${tmpdir}
+    Should Be Equal As Integers    ${init.rc}    0
+    ...    msg=agents init should exit 0 but got ${init.rc}. stderr: ${init.stderr}
+    ${list}=    Run Process    ${PYTHON}    -m    cleveragents    session    list
+    ...    timeout=60s    cwd=${tmpdir}
+    Should Be Equal As Integers    ${list.rc}    0
+    ...    msg=session list should exit 0 but got ${list.rc}. stderr: ${list.stderr}
+    Should Not Contain    ${list.stderr}    AttributeError
+    ...    msg=session list should not raise AttributeError in stderr: ${list.stderr}
+    Should Not Contain    ${list.stdout}    AttributeError
+    ...    msg=session list should not raise AttributeError in stdout: ${list.stdout}
+    [Teardown]    Remove Directory    ${tmpdir}    recursive=True
+
+Session List JSON Format Does Not Error
+    [Documentation]    session list --format json should exit 0 without raising
+    ...                a DI AttributeError.
+    [Tags]    tdd_bug    tdd_bug_554    tdd_expected_fail
+    ${tmpdir}=    Evaluate    __import__('tempfile').mkdtemp(prefix='sle_554_json_')
+    ${init}=    Run Process    ${PYTHON}    -m    cleveragents    init    sle-json
+    ...    timeout=60s    cwd=${tmpdir}
+    Should Be Equal As Integers    ${init.rc}    0
+    ...    msg=agents init should exit 0 but got ${init.rc}. stderr: ${init.stderr}
+    ${list}=    Run Process    ${PYTHON}    -m    cleveragents    session    list    --format    json
+    ...    timeout=60s    cwd=${tmpdir}
+    Should Be Equal As Integers    ${list.rc}    0
+    ...    msg=session list --format json should exit 0 but got ${list.rc}. stderr: ${list.stderr}
+    Should Not Contain    ${list.stderr}    AttributeError
+    ...    msg=session list --format json should not raise AttributeError in stderr: ${list.stderr}
+    Should Not Contain    ${list.stdout}    AttributeError
+    ...    msg=session list --format json should not raise AttributeError in stdout: ${list.stdout}
+    [Teardown]    Remove Directory    ${tmpdir}    recursive=True