Files
cleveragents-core/features/steps/project_create_persist_steps.py
brent.edwards 26de6bb7de
CI / lint (push) Successful in 15s
CI / build (push) Successful in 17s
CI / quality (push) Successful in 18s
CI / typecheck (push) Successful in 38s
CI / benchmark-regression (push) Has been skipped
CI / security (push) Successful in 50s
CI / unit_tests (push) Successful in 2m13s
CI / docker (push) Successful in 41s
CI / integration_tests (push) Successful in 3m11s
CI / coverage (push) Successful in 4m30s
CI / benchmark-publish (push) Successful in 16m39s
fix(project): persist project to database during creation (#591)
## Summary

Fix `agents project create` not persisting projects to the database. `NamespacedProjectRepository.create()` called `session.flush()` but never `session.commit()`, so projects were invisible to subsequent `agents project list` invocations that open a separate session.

## Changes

**Production fix** (`src/cleveragents/infrastructure/database/repositories.py`):
- Replace `session.flush()` with `session.commit()` in `NamespacedProjectRepository.create()`
- Add `finally: session.close()` guard for proper session lifecycle

**Tests & benchmarks**:
- 4 Behave BDD regression scenarios (`features/project_create_persist.feature`)
- Robot Framework integration smoke tests (`robot/project_create_persist.robot`)
- ASV benchmarks for create-then-list round-trip (`benchmarks/project_create_persist_bench.py`)

## Review feedback addressed

- **H3**: Added `finally: session.close()` to `create()` method
- **M1**: Removed redundant `session.flush()` before `session.commit()`
- **M2**: Updated stale TDD "expected to fail" comments — this PR includes the fix
- **M4**: Rewrote CHANGELOG entry to describe the fix, not just tests
- **F1**: Updated Robot documentation (Suite Setup/Teardown kept — needed for `${PYTHON}`)
- **F2**: Fixed bare assertion `"my-app"` to `"local/my-app"` in namespace scenario
- **F3**: Removed redundant `Base.metadata.create_all()` from `_make_fresh_repo()`

## Process

- Single squashed commit, rebased onto `master` (no merge commits)
- Prescribed commit message from issue #589 metadata

ISSUES CLOSED: #589

Reviewed-on: #591
Reviewed-by: Rui Hu <rui.hu@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
2026-03-09 06:59:15 +00:00

185 lines
7.1 KiB
Python

"""Step definitions for project_create_persist.feature (bug #589).
Regression tests verifying that ``agents project create`` commits data
to the database so that ``agents project list`` shows previously created
projects.
Design rationale
~~~~~~~~~~~~~~~~
The bug was that ``NamespacedProjectRepository.create()`` called
``session.flush()`` but never ``session.commit()``. Each CLI invocation
obtains a **new** session from the factory, so flushed-but-uncommitted
rows were invisible to subsequent invocations.
To exercise the fix we use a **file-based** SQLite database and patch
``_get_namespaced_project_repo`` so that every CLI invocation receives a
*fresh* ``NamespacedProjectRepository`` backed by a **new session
factory** pointing at the same file. This mirrors real CLI behaviour
where each ``agents project …`` invocation gets its own session.
"""
from __future__ import annotations
import os
import shutil
import tempfile
from typing import Any
from unittest.mock import patch
from behave import given, then, when
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker
from sqlalchemy.pool import NullPool
from typer.testing import CliRunner
from cleveragents.cli.commands.project import app as project_app
from cleveragents.infrastructure.database.models import Base
from cleveragents.infrastructure.database.repositories import (
NamespacedProjectRepository,
)
# Patch target - the module-level DI helper that returns the project repo.
_PATCH_PROJECT_REPO = "cleveragents.cli.commands.project._get_namespaced_project_repo"
# The --invariant flag is never passed in these scenarios, so
# _store_project_extras is effectively a no-op. We patch it defensively
# to avoid side-effects from DI look-ups in the real implementation.
_PATCH_STORE_EXTRAS = "cleveragents.cli.commands.project._store_project_extras"
runner = CliRunner()
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_fresh_repo(db_path: str) -> NamespacedProjectRepository:
"""Build a *new* repo with its own engine/session for *db_path*.
Each call simulates a separate CLI invocation: a new engine and session
factory are created, so uncommitted data from a prior invocation is not
visible. ``NullPool`` is used so that SQLite file handles are released
immediately when the engine is no longer referenced.
Schema is pre-created once during the Background step, so we skip
``Base.metadata.create_all()`` here to avoid redundant DDL.
"""
engine = create_engine(f"sqlite:///{db_path}", echo=False, poolclass=NullPool)
factory = sessionmaker(bind=engine, expire_on_commit=False)
return NamespacedProjectRepository(session_factory=factory)
def _cleanup_persist_db(context: Any) -> None:
"""Remove the temporary directory created for the test."""
shutil.rmtree(context.persist_tmpdir, ignore_errors=True)
# ---------------------------------------------------------------------------
# Background
# ---------------------------------------------------------------------------
@given("a fresh project-persist database is initialised")
def step_init_persist_db(context: Any) -> None:
"""Create a temp dir with a fresh SQLite file for persistence tests."""
context.persist_tmpdir = tempfile.mkdtemp(prefix="persist_589_")
context.persist_db_path = os.path.join(context.persist_tmpdir, "test.db")
# Pre-create schema so every fresh repo sees the tables.
engine = create_engine(
f"sqlite:///{context.persist_db_path}",
echo=False,
poolclass=NullPool,
)
Base.metadata.create_all(engine)
engine.dispose()
# Initialise result holders.
context.persist_create_results = [] # type: ignore[attr-defined]
context.persist_list_result = None
context.persist_dup_result = None
context.add_cleanup(_cleanup_persist_db, context)
# ---------------------------------------------------------------------------
# When steps - create
# ---------------------------------------------------------------------------
@when('I create a project named "{name}" via the persist CLI')
def step_create_project_persist(context: Any, name: str) -> None:
"""Invoke ``project create <name>`` with a fresh repo per call."""
repo = _make_fresh_repo(context.persist_db_path)
with (
patch(_PATCH_PROJECT_REPO, return_value=repo),
patch(_PATCH_STORE_EXTRAS),
):
result = runner.invoke(project_app, ["create", name])
assert result.exit_code == 0, (
f"project create '{name}' should exit 0 but got "
f"{result.exit_code}. output:\n{result.output}"
)
context.persist_create_results.append(result)
@when('I attempt to create a duplicate project named "{name}" via the persist CLI')
def step_create_duplicate_persist(context: Any, name: str) -> None:
"""Invoke ``project create`` again for the same name (expect error)."""
repo = _make_fresh_repo(context.persist_db_path)
with (
patch(_PATCH_PROJECT_REPO, return_value=repo),
patch(_PATCH_STORE_EXTRAS),
):
result = runner.invoke(project_app, ["create", name])
context.persist_dup_result = result
# ---------------------------------------------------------------------------
# When steps - list
# ---------------------------------------------------------------------------
@when("I list projects via the persist CLI")
def step_list_projects_persist(context: Any) -> None:
"""Invoke ``project list`` with a fresh repo pointing at the same DB."""
repo = _make_fresh_repo(context.persist_db_path)
with patch(_PATCH_PROJECT_REPO, return_value=repo):
result = runner.invoke(project_app, ["list"])
assert result.exit_code == 0, (
f"project list should exit 0 but got {result.exit_code}. "
f"output:\n{result.output}"
)
context.persist_list_result = result
# ---------------------------------------------------------------------------
# Then steps
# ---------------------------------------------------------------------------
@then('the persist project list should contain "{name}"')
def step_persist_list_contains(context: Any, name: str) -> None:
"""Assert that the list output includes the given project name."""
result = context.persist_list_result
assert result is not None, "project list was not invoked"
output = result.output
assert name in output, (
f"Expected '{name}' in project list output but got:\n{output}"
)
@then('the persist duplicate creation should fail with "{text}"')
def step_persist_dup_fails(context: Any, text: str) -> None:
"""Assert that the duplicate creation produced an error containing *text*."""
result = context.persist_dup_result
assert result is not None, "duplicate create was not invoked"
assert result.exit_code != 0, (
f"Duplicate create should fail (exit_code != 0) but got "
f"{result.exit_code}. output:\n{result.output}"
)
output = result.output
assert text.lower() in output.lower(), (
f"Expected '{text}' in duplicate-create output but got:\n{output}"
)