Files
temp/features/steps/session_list_error_steps.py
T
brent.edwards e732c32981 fix(cli): handle missing database in session list command
Register PersistentSessionService in the DI Container so that
'agents session list' (and all other session subcommands) no longer
throw AttributeError due to a missing 'db' provider.

Changes:
- Add _build_session_service() factory and session_service provider to
  Container, with targeted table creation for session/session_messages
  only (avoids bypassing Alembic for the full schema).
- Add auto_commit parameter to SessionRepository and
  SessionMessageRepository; when True each method commits and closes
  its own database session, preventing resource leaks in CLI context.
- Rewrite _get_session_service() to resolve via container.session_service()
  with module-level caching.
- Add (DatabaseError, AttributeError) error handling with logging to all
  seven session subcommands (list, create, show, delete, export, import,
  tell).
- Remove @tdd_expected_fail tags from all session test files so they run
  as proper regression tests.

ISSUES CLOSED: #554, #570, #680
2026-03-12 16:16:41 +00:00

256 lines
9.4 KiB
Python

"""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 logging
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 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.config.settings import Settings
from cleveragents.infrastructure.database.models import Base
from cleveragents.infrastructure.database.repositories import (
SessionMessageRepository,
SessionRepository,
)
runner = CliRunner()
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _suppress_structlog_stdout() -> tuple[int, bool]:
"""Prevent structlog debug lines from contaminating CLI stdout.
See ``tdd_session_shared_steps._suppress_structlog_stdout`` for the
full rationale.
"""
import structlog
root = logging.getLogger()
prev_level = root.level
root.setLevel(logging.WARNING)
prev_config = structlog.get_config()
prev_cache: bool = prev_config.get("cache_logger_on_first_use", True) # type: ignore[assignment]
structlog.configure(
logger_factory=structlog.stdlib.LoggerFactory(),
wrapper_class=structlog.stdlib.BoundLogger,
cache_logger_on_first_use=False,
)
return prev_level, prev_cache
def _restore_structlog(prev_level: int, prev_cache: bool) -> None:
"""Undo the suppression applied by :func:`_suppress_structlog_stdout`."""
import structlog
logging.getLogger().setLevel(prev_level)
structlog.configure(cache_logger_on_first_use=prev_cache)
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()
Settings._instance = None # type: ignore[attr-defined]
# Suppress structlog debug output so CliRunner captures clean output.
prev_level, prev_cache = _suppress_structlog_stdout()
context.sle_log_prev = (prev_level, prev_cache)
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()
Settings._instance = None # type: ignore[attr-defined]
# Restore structlog / logging state.
prev_level, prev_cache = context.sle_log_prev
_restore_structlog(prev_level, prev_cache)
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 = sessionmaker(bind=engine, expire_on_commit=False)
repo = SessionRepository(session_factory=factory, auto_commit=True)
msg_repo = SessionMessageRepository(session_factory=factory, auto_commit=True)
svc = PersistentSessionService(repo, msg_repo)
svc.create(actor_name="openai/gpt-4")
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]}"
)