CHANGELOG.md

@@ -6,7 +6,10 @@
   help content that varies for main-screen, slash-command, reference, and
   shell prompt modes. Updated Behave and Robot coverage for help-panel
   rendering and mode switching. (#1013)
-
+- Added direct BDD coverage for `_fast_init_or_upgrade` early-return behavior,
+  template-copy/fallback delegation paths, and the existing-empty-DB branch in
+  `features/fast_init_upgrade.feature`. Uses race-safe temp-path allocation
+  (`mkstemp`/`mkdtemp`) throughout new fast-init test steps. (#733)
 - Expanded the TUI slash command overlay catalog to include 67 commands across
   14 groups, aligned with the specification command reference for session,
   persona, scope, plan, project, registry/config, context, and utility flows.

features/fast_init_upgrade.feature

@@ -0,0 +1,59 @@
+Feature: Fast init_or_upgrade early-return behavior
+  As the test harness
+  I need _fast_init_or_upgrade to skip Alembic migrations for existing databases
+  So that parallel test execution does not hang on redundant migrations
+
+  Background:
+    Given the fast-init template-DB patch is installed
+
+  @mock_only
+  Scenario: Non-empty database with matching prefix skips original migration
+    Given a non-empty SQLite database file with a matching scenario prefix
+    When I call init_or_upgrade on the fast-init database
+    Then the original init_or_upgrade should not have been invoked
+    And the database file content should be unchanged
+
+  @mock_only
+  Scenario: Template is copied for a new database with matching prefix
+    Given a non-existent SQLite database path with a matching scenario prefix
+    When I call init_or_upgrade on the fast-init database
+    Then the database file should be a copy of the template
+    And the original init_or_upgrade should not have been invoked
+
+  @mock_only
+  Scenario: Existing empty database with matching prefix copies template
+    Given an empty SQLite database file with a matching scenario prefix
+    When I call init_or_upgrade on the fast-init database
+    Then the database file should be a copy of the template
+    And the original init_or_upgrade should not have been invoked
+
+  @mock_only
+  Scenario: Non-matching prefix delegates to original init_or_upgrade
+    Given a SQLite database URL whose filename has a non-matching prefix
+    When I call init_or_upgrade on the fast-init database
+    Then the original init_or_upgrade should have been invoked exactly once
+
+  @mock_only
+  Scenario: In-memory SQLite delegates to original init_or_upgrade
+    Given an in-memory SQLite database URL for fast-init testing
+    When I call init_or_upgrade on the fast-init database
+    Then the original init_or_upgrade should have been invoked exactly once
+
+  @mock_only
+  Scenario: Non-SQLite URL delegates to original init_or_upgrade
+    Given a non-SQLite database URL for fast-init testing
+    When I call init_or_upgrade on the fast-init database
+    Then the original init_or_upgrade should have been invoked exactly once
+
+  @mock_only
+  Scenario: Bare sqlite:// URL delegates to original init_or_upgrade
+    Given a bare sqlite:// database URL for fast-init testing
+    When I call init_or_upgrade on the fast-init database
+    Then the original init_or_upgrade should have been invoked exactly once
+
+  @mock_only
+  Scenario: Delegation forwards runner instance and keyword arguments
+    Given a non-SQLite database URL for fast-init testing
+    When I call init_or_upgrade with keyword arguments on the fast-init database
+    Then the original init_or_upgrade should have been invoked exactly once
+    And the original should have received the runner instance and keyword arguments

features/mocks/fast_init_test_helpers.py

@@ -0,0 +1,95 @@
+"""Test helpers for exercising ``_fast_init_or_upgrade`` closure logic.
+
+This module provides utilities that safely swap the
+``_original_init_or_upgrade`` reference captured inside the
+``_fast_init_or_upgrade`` closure (installed by
+``features.environment._install_template_db_patch``) with a
+``unittest.mock.MagicMock`` so that step definitions can assert
+whether the original migration runner was invoked.
+
+All helpers live in ``features/mocks/`` per the project's mock-placement
+rule.
+"""
+
+from __future__ import annotations
+
+import contextlib
+import types
+from collections.abc import Generator
+from typing import Any
+from unittest.mock import MagicMock
+
+
+def _find_closure_cell(
+    func: types.FunctionType,
+    var_name: str,
+) -> tuple[int, types.CellType] | None:
+    """Locate a closure cell by free-variable name.
+
+    Args:
+        func: The function whose closure to inspect.
+        var_name: The free-variable name to find.
+
+    Returns:
+        A ``(index, cell)`` tuple if found, otherwise ``None``.
+
+    Raises:
+        TypeError: If *func* is not a function with a closure.
+    """
+    if not isinstance(func, types.FunctionType):
+        raise TypeError(f"Expected a function, got {type(func).__name__}")
+    freevars: tuple[str, ...] = func.__code__.co_freevars
+    closure: tuple[types.CellType, ...] | None = func.__closure__
+    if closure is None:
+        return None
+    for idx, name in enumerate(freevars):
+        if name == var_name:
+            return idx, closure[idx]
+    return None
+
+
+@contextlib.contextmanager
+def patch_original_init_or_upgrade() -> Generator[MagicMock]:
+    """Context manager that replaces ``_original_init_or_upgrade`` inside
+    the ``_fast_init_or_upgrade`` closure with a :class:`~unittest.mock.MagicMock`.
+
+    Yields the mock so callers can assert on call counts and arguments.
+    Restores the original cell contents on exit.
+
+    Raises:
+        RuntimeError: If the fast-init patch is not installed on
+            ``MigrationRunner.init_or_upgrade`` (e.g. template DB missing).
+
+    Example usage in a step definition::
+
+        with patch_original_init_or_upgrade() as mock_original:
+            runner.init_or_upgrade()
+            assert mock_original.called
+    """
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+
+    fast_fn: Any = MigrationRunner.init_or_upgrade
+    if not isinstance(fast_fn, types.FunctionType):
+        raise RuntimeError(
+            "MigrationRunner.init_or_upgrade is not a plain function — "
+            "the _fast_init_or_upgrade patch does not appear to be installed."
+        )
+
+    result = _find_closure_cell(fast_fn, "_original_init_or_upgrade")
+    if result is None:
+        raise RuntimeError(
+            "Cannot find '_original_init_or_upgrade' in the closure of "
+            f"{fast_fn.__qualname__}. Is the template-DB patch installed?"
+        )
+
+    _idx, cell = result
+    cell_ref: Any = cell
+    saved: Any = cell_ref.cell_contents
+    mock = MagicMock(name="_original_init_or_upgrade")
+    cell_ref.cell_contents = mock
+    try:
+        yield mock
+    finally:
+        cell_ref.cell_contents = saved

features/steps/fast_init_upgrade_steps.py

@@ -0,0 +1,284 @@
+"""Step definitions for fast_init_upgrade.feature.
+
+Exercises the ``_fast_init_or_upgrade`` closure installed by
+``features.environment._install_template_db_patch`` — specifically the
+early-return path for non-empty databases, the template-copy path for
+new databases, and the fallback delegation to the original
+``MigrationRunner.init_or_upgrade``.
+"""
+
+from __future__ import annotations
+
+import contextlib
+import filecmp
+import os
+import shutil
+import tempfile
+import types
+from pathlib import Path
+from typing import Any
+
+from behave import given, then, when
+from behave.runner import Context
+
+# ---------------------------------------------------------------------------
+# Internal helpers
+# ---------------------------------------------------------------------------
+
+
+def _register_path_cleanup(context: Context, path: str) -> None:
+    """Schedule *path* (and its SQLite sidecar files) for removal."""
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+
+    def _remove() -> None:
+        for suffix in ("", "-journal", "-wal", "-shm"):
+            with contextlib.suppress(OSError):
+                os.unlink(path + suffix)
+
+    context._cleanup_handlers.append(_remove)
+
+
+def _register_directory_cleanup(context: Context, path: str) -> None:
+    """Schedule directory *path* for recursive removal."""
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+
+    def _remove() -> None:
+        with contextlib.suppress(OSError):
+            shutil.rmtree(path)
+
+    context._cleanup_handlers.append(_remove)
+
+
+# ---------------------------------------------------------------------------
+# Background
+# ---------------------------------------------------------------------------
+
+
+@given("the fast-init template-DB patch is installed")
+def step_verify_patch_installed(context: Context) -> None:
+    """Verify that ``_install_template_db_patch`` has been applied."""
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+
+    fast_fn: Any = MigrationRunner.init_or_upgrade
+    if not isinstance(fast_fn, types.FunctionType):
+        raise AssertionError(
+            "MigrationRunner.init_or_upgrade is not a plain function — "
+            "the _fast_init_or_upgrade patch does not appear to be installed. "
+            "Ensure CLEVERAGENTS_TEMPLATE_DB is set and the template exists."
+        )
+
+    freevars: tuple[str, ...] = fast_fn.__code__.co_freevars
+    if "_original_init_or_upgrade" not in freevars:
+        raise AssertionError(
+            "MigrationRunner.init_or_upgrade does not capture "
+            "'_original_init_or_upgrade' — the template-DB patch is not "
+            "installed as expected."
+        )
+
+    # Store template path for later steps.
+    template_db: str | None = os.environ.get("CLEVERAGENTS_TEMPLATE_DB")
+    if not template_db or not Path(template_db).is_file():
+        raise AssertionError(
+            "CLEVERAGENTS_TEMPLATE_DB is not set or the file does not exist."
+        )
+    context.fast_init_template_path = template_db
+
+
+# ---------------------------------------------------------------------------
+# Given — non-empty DB with matching prefix
+# ---------------------------------------------------------------------------
+
+
+@given("a non-empty SQLite database file with a matching scenario prefix")
+def step_create_nonempty_db(context: Context) -> None:
+    """Create a temporary non-empty file whose name starts with a matching prefix."""
+    fd, path = tempfile.mkstemp(suffix=".db", prefix="test_fastinit_")
+    os.write(fd, b"non-empty-content-to-simulate-migrated-db")
+    os.close(fd)
+    context.fast_init_db_path = path
+    context.fast_init_db_url = f"sqlite:///{path}"
+    context.fast_init_original_content = Path(path).read_bytes()
+    _register_path_cleanup(context, path)
+
+
+# ---------------------------------------------------------------------------
+# Given — template-copy path (new DB)
+# ---------------------------------------------------------------------------
+
+
+@given("a non-existent SQLite database path with a matching scenario prefix")
+def step_create_nonexistent_db_path(context: Context) -> None:
+    """Generate a path for a non-existent file with a matching prefix."""
+    temp_dir = tempfile.mkdtemp(prefix="test_fastinit_new_")
+    path = str(Path(temp_dir) / "test_fastinit.db")
+    context.fast_init_db_path = path
+    context.fast_init_db_url = f"sqlite:///{path}"
+    _register_path_cleanup(context, path)
+    _register_directory_cleanup(context, temp_dir)
+
+
+@given("an empty SQLite database file with a matching scenario prefix")
+def step_create_empty_db(context: Context) -> None:
+    """Create an existing zero-byte DB file with a matching prefix."""
+    fd, path = tempfile.mkstemp(suffix=".db", prefix="test_fastinit_empty_")
+    os.close(fd)
+    context.fast_init_db_path = path
+    context.fast_init_db_url = f"sqlite:///{path}"
+    _register_path_cleanup(context, path)
+
+
+# ---------------------------------------------------------------------------
+# Given — fallback paths (non-matching / in-memory / non-SQLite)
+# ---------------------------------------------------------------------------
+
+
+@given("a SQLite database URL whose filename has a non-matching prefix")
+def step_create_nonmatching_prefix_url(context: Context) -> None:
+    """Create a URL whose filename prefix is not in _SCENARIO_DB_PREFIXES."""
+    fd, path = tempfile.mkstemp(suffix=".db", prefix="nomatch_fastinit_")
+    os.close(fd)
+    context.fast_init_db_path = path
+    context.fast_init_db_url = f"sqlite:///{path}"
+    _register_path_cleanup(context, path)
+
+
+@given("an in-memory SQLite database URL for fast-init testing")
+def step_create_inmemory_url(context: Context) -> None:
+    """Set up an in-memory SQLite URL."""
+    context.fast_init_db_path = None
+    context.fast_init_db_url = "sqlite:///:memory:"
+
+
+@given("a non-SQLite database URL for fast-init testing")
+def step_create_nonsqlite_url(context: Context) -> None:
+    """Set up a non-SQLite URL (PostgreSQL placeholder)."""
+    context.fast_init_db_path = None
+    context.fast_init_db_url = "postgresql://fake_user:fake_pass@localhost/testdb"
+
+
+@given("a bare sqlite:// database URL for fast-init testing")
+def step_create_bare_sqlite_url(context: Context) -> None:
+    """Set up a bare ``sqlite://`` URL (no path, no ``:memory:``)."""
+    context.fast_init_db_path = None
+    context.fast_init_db_url = "sqlite://"
+
+
+# ---------------------------------------------------------------------------
+# When
+# ---------------------------------------------------------------------------
+
+
+@when("I call init_or_upgrade on the fast-init database")
+def step_call_init_or_upgrade(context: Context) -> None:
+    """Invoke ``MigrationRunner.init_or_upgrade`` via the fast-init patch."""
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+    from features.mocks.fast_init_test_helpers import (
+        patch_original_init_or_upgrade,
+    )
+
+    runner = MigrationRunner(database_url=context.fast_init_db_url)
+
+    with patch_original_init_or_upgrade() as mock_original:
+        runner.init_or_upgrade()
+        context.fast_init_mock_called = mock_original.called
+        context.fast_init_mock_call_count = mock_original.call_count
+        context.fast_init_mock_call_args = mock_original.call_args
+        context.fast_init_runner = runner
+
+
+@when("I call init_or_upgrade with keyword arguments on the fast-init database")
+def step_call_init_or_upgrade_with_kwargs(context: Context) -> None:
+    """Invoke ``init_or_upgrade`` with keyword arguments to verify forwarding."""
+    from cleveragents.infrastructure.database.migration_runner import (
+        MigrationRunner,
+    )
+    from features.mocks.fast_init_test_helpers import (
+        patch_original_init_or_upgrade,
+    )
+
+    runner = MigrationRunner(database_url=context.fast_init_db_url)
+
+    with patch_original_init_or_upgrade() as mock_original:
+        runner.init_or_upgrade(require_confirmation=True)
+        context.fast_init_mock_called = mock_original.called
+        context.fast_init_mock_call_count = mock_original.call_count
+        context.fast_init_mock_call_args = mock_original.call_args
+        context.fast_init_runner = runner
+
+
+# ---------------------------------------------------------------------------
+# Then — original invocation assertions
+# ---------------------------------------------------------------------------
+
+
+@then("the original init_or_upgrade should not have been invoked")
+def step_assert_original_not_called(context: Context) -> None:
+    """Assert the mocked original was never called."""
+    assert not context.fast_init_mock_called, (
+        "Expected _original_init_or_upgrade NOT to be called, "
+        f"but it was called {context.fast_init_mock_call_count} time(s)."
+    )
+
+
+@then("the original init_or_upgrade should have been invoked exactly once")
+def step_assert_original_called_once(context: Context) -> None:
+    """Assert the mocked original was called exactly once."""
+    assert context.fast_init_mock_call_count == 1, (
+        "Expected _original_init_or_upgrade to be called exactly once, "
+        f"but it was called {context.fast_init_mock_call_count} time(s)."
+    )
+
+
+@then("the original should have received the runner instance and keyword arguments")
+def step_assert_original_received_runner_and_kwargs(context: Context) -> None:
+    """Assert the mock was called with the correct runner and kwargs."""
+    call_args = context.fast_init_mock_call_args
+    assert call_args is not None, (
+        "Expected _original_init_or_upgrade to have been called, but call_args is None."
+    )
+    positional = call_args.args
+    assert len(positional) >= 1, (
+        "Expected _original_init_or_upgrade to receive the runner as "
+        f"first positional argument, but got {len(positional)} positional args."
+    )
+    assert positional[0] is context.fast_init_runner, (
+        "Expected the first positional argument to be the MigrationRunner "
+        "instance, but a different object was passed."
+    )
+    assert call_args.kwargs.get("require_confirmation") is True, (
+        "Expected keyword argument 'require_confirmation=True' to be "
+        f"forwarded, but got kwargs={call_args.kwargs}."
+    )
+
+
+# ---------------------------------------------------------------------------
+# Then — file-content assertions
+# ---------------------------------------------------------------------------
+
+
+@then("the database file content should be unchanged")
+def step_assert_file_unchanged(context: Context) -> None:
+    """Assert the DB file still has its original content."""
+    current_content = Path(context.fast_init_db_path).read_bytes()
+    assert current_content == context.fast_init_original_content, (
+        "Database file content changed unexpectedly."
+    )
+
+
+@then("the database file should be a copy of the template")
+def step_assert_file_is_template_copy(context: Context) -> None:
+    """Assert the DB file was created as a copy of the template."""
+    db_path = Path(context.fast_init_db_path)
+    assert db_path.exists(), f"Database file was not created at {db_path}"
+    assert db_path.stat().st_size > 0, "Database file is empty"
+    assert filecmp.cmp(
+        str(db_path),
+        context.fast_init_template_path,
+        shallow=False,
+    ), "Database file does not match the template"