test(core): stabilize postgres fixtures

Signed-off-by: phernandez <paul@basicmachines.co>

AGENTS.md

@@ -22,16 +22,16 @@ See the [README.md](README.md) file for a project overview.
 - Run unit tests (Postgres): `just test-unit-postgres`
 - Run integration tests (SQLite): `just test-int-sqlite`
 - Run integration tests (Postgres): `just test-int-postgres`
-- Run impacted tests: `just testmon` (pytest-testmon)
+- Run impacted tests: `just testmon` (pytest-testmon; only tests affected by changed code)
 - Run MCP smoke test: `just test-smoke`
-- Fast local loop: `just fast-check`
+- Fast local loop: `just fast-check` (default iteration flow)
 - Local consistency check: `just doctor`
 - Generate HTML coverage: `just coverage`
 - Single test: `pytest tests/path/to/test_file.py::test_function_name`
 - Run benchmarks: `pytest test-int/test_sync_performance_benchmark.py -v -m "benchmark and not slow"`
 - Lint: `just lint` or `ruff check . --fix`
-- Type check: `just typecheck` or `uv run pyright`
-- Type check (supplemental): `just typecheck-ty` or `uv run ty check src/`
+- Type check: `just typecheck` or `uv run ty check src tests test-int`
+- Type check (pyright): `just typecheck-pyright` or `uv run pyright`
 - Format: `just format` or `uv run ruff format .`
 - Run all code checks: `just check` (runs lint, format, typecheck, test)
 - Create db migration: `just migration "Your migration message"`
@@ -48,10 +48,12 @@ See the [README.md](README.md) file for a project overview.
 ### Code/Test/Verify Loop (fast path)
 
 1) **Code:** make changes.
-2) **Test:** `just fast-check` (lint/format/typecheck + impacted tests + MCP smoke).
+2) **Test:** `just fast-check` (lint/format/typecheck + pytest-testmon impacted tests for changed code).
 3) **Verify:** `just doctor` (end-to-end file ↔ DB loop in a temp project).
 4) **Full gate (when needed):** `just test` or `just check` for SQLite + Postgres.
 
+Run `just test-smoke` when you specifically need the MCP smoke flow.
+
 If testmon is “cold,” the first run may be long. Subsequent runs get much faster.
 
 ### Test Structure

justfile

@@ -62,20 +62,20 @@ test-int-postgres:
     fi
 
 # Run tests impacted by recent changes (requires pytest-testmon)
+# Pass paths or node ids after `just testmon` to limit the candidate set further.
 testmon *args:
-    BASIC_MEMORY_ENV=test uv run pytest -p pytest_mock -v --no-cov --testmon --testmon-forceselect {{args}}
+    BASIC_MEMORY_ENV=test uv run pytest -p pytest_mock -v --no-cov --testmon {{args}}
 
 # Run MCP smoke test (fast end-to-end loop)
 test-smoke:
     BASIC_MEMORY_ENV=test uv run pytest -p pytest_mock -v --no-cov -m smoke test-int/mcp/test_smoke_integration.py
 
-# Fast local loop: lint, format, typecheck, impacted tests
+# Fast local loop: lint, format, typecheck, impacted tests via pytest-testmon
 fast-check:
     just fix
     just format
     just typecheck
     just testmon
-    just test-smoke
 
 # Reset Postgres test database (drops and recreates schema)
 # Useful when Alembic migration state gets out of sync during development

pyproject.toml

@@ -71,6 +71,12 @@ addopts = "--cov=basic_memory --cov-report term-missing"
 testpaths = ["tests", "test-int"]
 asyncio_mode = "strict"
 asyncio_default_fixture_loop_scope = "function"
+filterwarnings = [
+    "ignore:The @wait_container_is_ready decorator is deprecated.*:DeprecationWarning:testcontainers\\.core\\.waiting_utils",
+    "ignore:The default datetime adapter is deprecated as of Python 3\\.12.*:DeprecationWarning:aiosqlite\\.core",
+    "ignore:codecs\\.open\\(\\) is deprecated\\. Use open\\(\\) instead\\.:DeprecationWarning:frontmatter",
+    "ignore:Parsing dates involving a day of month without a year specified is ambiguous.*:DeprecationWarning:dateparser\\.utils\\.strptime",
+]
 markers = [
     "benchmark: Performance benchmark tests (deselect with '-m \"not benchmark\"')",
     "slow: Slow-running tests (deselect with '-m \"not slow\"')",

src/basic_memory/repository/repository.py

@@ -280,9 +280,9 @@ class Repository[T: Base]:
 
                 if isinstance(entity_data, dict):
                     update_data = cast(dict[str, Any], entity_data)
-                    for key, value in update_data.items():
-                        if key in self.valid_columns:
-                            setattr(entity, key, value)
+                    for key in self.valid_columns:
+                        if key in update_data:
+                            setattr(entity, key, update_data[key])
 
                 elif isinstance(entity_data, self.Model):
                     for column in self.valid_columns:

test-int/conftest.py

@@ -57,7 +57,12 @@ import pytest
 import pytest_asyncio
 from pathlib import Path
 from sqlalchemy import text
-from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
+from sqlalchemy.ext.asyncio import (
+    AsyncEngine,
+    AsyncSession,
+    async_sessionmaker,
+    create_async_engine,
+)
 from sqlalchemy.pool import NullPool
 from testcontainers.postgres import PostgresContainer
 
@@ -118,6 +123,21 @@ def postgres_container(db_backend):
         yield postgres
 
 
+@pytest_asyncio.fixture(autouse=True)
+async def cleanup_global_db_after_test() -> AsyncGenerator[None, None]:
+    """Close any module-level DB engine created outside fixture ownership."""
+    yield
+
+    # Trigger: integration tests invoke CLI/MCP routes through the production
+    # client fallback, bypassing this file's engine_factory fixture.
+    # Why: those fallback engines live in basic_memory.db module state and can
+    # otherwise leave a non-daemon aiosqlite worker alive after pytest finishes.
+    # Outcome: every test boundary becomes a cleanup point for fallback engines.
+    from basic_memory import db
+
+    await db.shutdown_db()
+
+
 POSTGRES_EPHEMERAL_TABLES = [
     "search_vector_embeddings",
     "search_vector_chunks",
@@ -182,12 +202,36 @@ async def _reset_postgres_integration_schema(engine) -> None:
         )
 
 
+@pytest_asyncio.fixture(scope="session", loop_scope="session")
+async def postgres_engine(
+    db_backend: Literal["sqlite", "postgres"], postgres_container
+) -> AsyncGenerator[AsyncEngine | None, None]:
+    """Create the shared Postgres engine once per integration test session."""
+    if db_backend != "postgres":
+        yield None
+        return
+
+    sync_url = _resolve_postgres_sync_url(postgres_container)
+    async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
+    engine = create_async_engine(
+        async_url,
+        echo=False,
+        poolclass=NullPool,
+    )
+
+    try:
+        yield engine
+    finally:
+        await engine.dispose()
+
+
 @pytest_asyncio.fixture
 async def engine_factory(
     app_config,
     config_manager,
     db_backend: Literal["sqlite", "postgres"],
     postgres_container,
+    postgres_engine,
     tmp_path,
 ) -> AsyncGenerator[tuple, None]:
     """Create engine and session factory for the configured database backend."""
@@ -195,18 +239,17 @@ async def engine_factory(
     from basic_memory import db
 
     if db_backend == "postgres":
-        # Postgres mode using testcontainers
-        sync_url = _resolve_postgres_sync_url(postgres_container)
-        async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
+        assert postgres_engine is not None
 
-        engine = create_async_engine(
-            async_url,
-            echo=False,
-            poolclass=NullPool,
-        )
+        # Trigger: full-stack MCP/CLI tests exercise sync/indexing code that can
+        # recover from DB errors by rolling back and opening later scoped sessions.
+        # Why: one savepoint-backed connection is too brittle for that flow.
+        # Outcome: reuse the engine, but reset rows/schema before each test and
+        # let app code use normal transaction boundaries.
+        await _reset_postgres_integration_schema(postgres_engine)
 
         session_maker = async_sessionmaker(
-            bind=engine,
+            bind=postgres_engine,
             class_=AsyncSession,
             expire_on_commit=False,
             autoflush=False,
@@ -214,17 +257,17 @@ async def engine_factory(
 
         # Set module-level state to prevent MCP lifespan from re-initializing
         # This ensures get_or_create_db() sees an existing engine and skips initialization
-        db._engine = engine
+        db._engine = postgres_engine
         db._session_maker = session_maker
 
-        await _reset_postgres_integration_schema(engine)
-
-        yield engine, session_maker
-
-        # Clean up module-level state
-        await engine.dispose()
-        db._engine = None
-        db._session_maker = None
+        try:
+            yield postgres_engine, session_maker
+        finally:
+            # Clean up module-level state
+            if db._engine is postgres_engine:
+                db._engine = None
+            if db._session_maker is session_maker:
+                db._session_maker = None
 
     else:
         # SQLite: Create fresh database (fast with tmp files)

test-int/semantic/conftest.py

@@ -8,6 +8,7 @@ SearchService for each (backend, provider) combination.
 from __future__ import annotations
 
 import os
+from collections.abc import AsyncGenerator
 from dataclasses import dataclass
 from pathlib import Path
 
@@ -15,7 +16,12 @@ import pytest
 import pytest_asyncio
 from dotenv import load_dotenv
 from sqlalchemy import text
-from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
+from sqlalchemy.ext.asyncio import (
+    AsyncEngine,
+    AsyncSession,
+    async_sessionmaker,
+    create_async_engine,
+)
 from sqlalchemy.pool import NullPool
 from testcontainers.postgres import PostgresContainer
 
@@ -152,25 +158,8 @@ async def sqlite_engine_factory(tmp_path):
         yield engine, session_maker
 
 
-@pytest_asyncio.fixture
-async def postgres_engine_factory(pgvector_container):
-    """Create a Postgres engine + session factory with pgvector extension."""
-    if pgvector_container is None:
-        yield None
-        return
-
-    sync_url = pgvector_container.get_connection_url()
-    async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
-
-    engine = create_async_engine(async_url, echo=False, poolclass=NullPool)
-    session_maker = async_sessionmaker(
-        bind=engine,
-        class_=AsyncSession,
-        expire_on_commit=False,
-        autoflush=False,
-    )
-
-    # Create schema from scratch for each test
+async def _reset_postgres_semantic_schema(engine: AsyncEngine) -> None:
+    """Reset the semantic Postgres schema to a clean baseline."""
     async with engine.begin() as conn:
         await conn.execute(text("DROP TABLE IF EXISTS search_vector_embeddings CASCADE"))
         await conn.execute(text("DROP TABLE IF EXISTS search_vector_chunks CASCADE"))
@@ -182,9 +171,47 @@ async def postgres_engine_factory(pgvector_container):
         await conn.execute(CREATE_POSTGRES_SEARCH_INDEX_METADATA)
         await conn.execute(CREATE_POSTGRES_SEARCH_INDEX_PERMALINK)
 
-    yield engine, session_maker
 
-    await engine.dispose()
+@pytest_asyncio.fixture(scope="session", loop_scope="session")
+async def postgres_engine(pgvector_container) -> AsyncGenerator[AsyncEngine | None, None]:
+    """Create the shared semantic Postgres engine once per test session."""
+    if pgvector_container is None:
+        yield None
+        return
+
+    sync_url = pgvector_container.get_connection_url()
+    async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
+
+    engine = create_async_engine(async_url, echo=False, poolclass=NullPool)
+
+    try:
+        yield engine
+    finally:
+        await engine.dispose()
+
+
+@pytest_asyncio.fixture
+async def postgres_engine_factory(postgres_engine):
+    """Create a Postgres session factory isolated by schema reset."""
+    if postgres_engine is None:
+        yield None
+        return
+
+    # Trigger: semantic provider combos create and rebuild vector tables.
+    # Why: the main suite showed savepoint-bound shared connections can get
+    # poisoned by app-level rollback/recovery paths.
+    # Outcome: keep the pgvector engine warm, but reset schema per test and let
+    # repository code use normal Postgres transactions.
+    await _reset_postgres_semantic_schema(postgres_engine)
+
+    session_maker = async_sessionmaker(
+        bind=postgres_engine,
+        class_=AsyncSession,
+        expire_on_commit=False,
+        autoflush=False,
+    )
+
+    yield postgres_engine, session_maker
 
 
 # --- Embedding provider factories ---

tests/api/v2/conftest.py

@@ -14,13 +14,21 @@ from basic_memory.models import Project
 
 
 @pytest_asyncio.fixture
-async def app(test_config, engine_factory, app_config) -> FastAPI:
+async def app(test_config, engine_factory, app_config) -> AsyncGenerator[FastAPI, None]:
     """Create FastAPI test application."""
     from basic_memory.api.app import app
 
+    previous_overrides = dict(app.dependency_overrides)
     app.dependency_overrides[get_app_config] = lambda: app_config
     app.dependency_overrides[get_engine_factory] = lambda: engine_factory
-    return app
+    try:
+        yield app
+    finally:
+        # Trigger: the FastAPI app is a module-level singleton shared across tests.
+        # Why: dependency overrides that capture a per-test engine can leak into
+        # later CLI/MCP tests and create connections outside fixture ownership.
+        # Outcome: each API test leaves the shared app exactly as it found it.
+        app.dependency_overrides = previous_overrides
 
 
 @pytest_asyncio.fixture

tests/cli/conftest.py

@@ -38,13 +38,23 @@ def isolated_home(tmp_path, monkeypatch) -> Path:
 
 
 @pytest_asyncio.fixture
-async def app(app_config, project_config, engine_factory, test_config, aiolib) -> FastAPI:
+async def app(
+    app_config, project_config, engine_factory, test_config, aiolib
+) -> AsyncGenerator[FastAPI, None]:
     """Create test FastAPI application."""
     app = fastapi_app
+    previous_overrides = dict(app.dependency_overrides)
     app.dependency_overrides[get_app_config] = lambda: app_config
     app.dependency_overrides[get_project_config] = lambda: project_config
     app.dependency_overrides[get_engine_factory] = lambda: engine_factory
-    return app
+    try:
+        yield app
+    finally:
+        # Trigger: CLI tests share the module-level FastAPI app with API/MCP tests.
+        # Why: leaving per-test dependency overrides installed lets later commands
+        # talk to stale engines that no cleanup fixture owns.
+        # Outcome: keep CLI app wiring isolated to the requesting test.
+        app.dependency_overrides = previous_overrides
 
 
 @pytest_asyncio.fixture

tests/conftest.py

@@ -146,7 +146,7 @@ def _resolve_postgres_sync_url(postgres_container) -> str:
 
 
 async def _reset_postgres_test_schema(engine: AsyncEngine, async_url: str) -> None:
-    """Restore the shared Postgres schema to a clean baseline before each test."""
+    """Restore the shared Postgres schema to a clean baseline."""
     from basic_memory.models.search import (
         CREATE_POSTGRES_SEARCH_INDEX_FTS,
         CREATE_POSTGRES_SEARCH_INDEX_METADATA,
@@ -183,6 +183,29 @@ async def _reset_postgres_test_schema(engine: AsyncEngine, async_url: str) -> No
         command.stamp(_postgres_alembic_config(async_url), "head")
 
 
+@pytest_asyncio.fixture(scope="session", loop_scope="session")
+async def postgres_engine(
+    db_backend, postgres_container
+) -> AsyncGenerator[AsyncEngine | None, None]:
+    """Create the shared Postgres engine once per test session."""
+    if db_backend != "postgres":
+        yield None
+        return
+
+    sync_url = _resolve_postgres_sync_url(postgres_container)
+    async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
+    engine = create_async_engine(
+        async_url,
+        echo=False,
+        poolclass=NullPool,
+    )
+
+    try:
+        yield engine
+    finally:
+        await engine.dispose()
+
+
 @pytest.fixture
 def anyio_backend():
     return "asyncio"
@@ -194,6 +217,19 @@ def suppress_logfire_no_config_warning(monkeypatch) -> None:
     monkeypatch.setenv("LOGFIRE_IGNORE_NO_CONFIG", "1")
 
 
+@pytest_asyncio.fixture(autouse=True)
+async def cleanup_global_db_after_test() -> AsyncGenerator[None, None]:
+    """Close any module-level DB engine created outside fixture ownership."""
+    yield
+
+    # Trigger: a test exercises production fallback routing instead of the
+    # per-test engine fixture.
+    # Why: that path stores an engine in basic_memory.db module state, and
+    # a later fixture can overwrite the reference before it is disposed.
+    # Outcome: close straggler aiosqlite worker threads before the loop closes.
+    await db.shutdown_db()
+
+
 @pytest.fixture
 def project_root() -> Path:
     return Path(__file__).parent.parent
@@ -293,6 +329,7 @@ async def engine_factory(
     config_manager,
     db_backend,
     postgres_container,
+    postgres_engine,
 ) -> AsyncGenerator[tuple[AsyncEngine, async_sessionmaker[AsyncSession]], None]:
     """Engine factory for SQLite or Postgres tests.
 
@@ -306,19 +343,20 @@ async def engine_factory(
     )
 
     if db_backend == "postgres":
-        # Postgres mode using testcontainers
-        # Get async connection URL (asyncpg driver - same as production)
+        assert postgres_engine is not None
         sync_url = _resolve_postgres_sync_url(postgres_container)
         async_url = sync_url.replace("postgresql+psycopg2", "postgresql+asyncpg")
 
-        engine = create_async_engine(
-            async_url,
-            echo=False,
-            poolclass=NullPool,  # NullPool for better test isolation
-        )
+        # Trigger: Basic Memory sync/indexing paths can catch database errors,
+        # roll back, and continue through new scoped sessions.
+        # Why: binding every scoped session to one savepoint-backed connection can
+        # leave that connection in PendingRollbackError state for later sessions.
+        # Outcome: keep the session-scoped engine, but use normal Postgres
+        # transaction semantics and reset rows/schema between tests.
+        await _reset_postgres_test_schema(postgres_engine, async_url)
 
         session_maker = async_sessionmaker(
-            bind=engine,
+            bind=postgres_engine,
             class_=AsyncSession,
             expire_on_commit=False,
             autoflush=False,
@@ -328,16 +366,16 @@ async def engine_factory(
         # Some codepaths (e.g. app initialization / MCP lifespan) call db.get_or_create_db(),
         # which would otherwise create a separate engine and run migrations, conflicting with
         # our test-created schema (and causing DuplicateTableError).
-        db._engine = engine
+        db._engine = postgres_engine
         db._session_maker = session_maker
 
-        await _reset_postgres_test_schema(engine, async_url)
-
-        yield engine, session_maker
-
-        await engine.dispose()
-        db._engine = None
-        db._session_maker = None
+        try:
+            yield postgres_engine, session_maker
+        finally:
+            if db._engine is postgres_engine:
+                db._engine = None
+            if db._session_maker is session_maker:
+                db._session_maker = None
     else:
         # SQLite mode
         db_type = DatabaseType.MEMORY

tests/mcp/conftest.py

@@ -1,6 +1,7 @@
 """Tests for the MCP server implementation using FastAPI TestClient."""
 
-from typing import Any, AsyncGenerator, cast
+from collections.abc import AsyncGenerator, Generator
+from typing import Any, cast
 
 import pytest
 import pytest_asyncio
@@ -20,13 +21,23 @@ def mcp() -> FastMCP:
 
 
 @pytest.fixture(scope="function")
-def app(app_config, project_config, engine_factory, config_manager) -> FastAPI:
+def app(
+    app_config, project_config, engine_factory, config_manager
+) -> Generator[FastAPI, None, None]:
     """Create test FastAPI application."""
     app = fastapi_app
+    previous_overrides = dict(app.dependency_overrides)
     app.dependency_overrides[get_app_config] = lambda: app_config
     app.dependency_overrides[get_project_config] = lambda: project_config
     app.dependency_overrides[get_engine_factory] = lambda: engine_factory
-    return app
+    try:
+        yield app
+    finally:
+        # Trigger: the FastAPI app is a module-level singleton shared across tests.
+        # Why: stale dependency overrides can hold onto a disposed per-test engine
+        # and reopen SQLite connections during later unrelated tests.
+        # Outcome: restore the shared app's override table after each MCP test.
+        app.dependency_overrides = previous_overrides
 
 
 @pytest_asyncio.fixture(scope="function")