test: check database ownership against the private temp namespace

scripts/verify_notes_browser_runtime.py

@@ -11,38 +11,6 @@ import time
 from urllib.request import urlopen
 
 ROOT = Path(__file__).resolve().parents[1]
-if str(ROOT) not in sys.path:
-    sys.path.insert(0, str(ROOT))
-
-
-def _stop_owned_server(server, identity):
-    """Close only identity-bound descendants, including MCP's new sessions.
-
-    Capture before signalling the parent: an orphaned stderr holder can no
-    longer be attributed by ancestry. Never search by executable name or kill
-    the caller's process group.
-    """
-    from src import process_lifecycle as lifecycle
-
-    descendants = (lifecycle.bind_descendants([identity.pid])
-                   if identity.verdict() == lifecycle.OWNED else [])
-    if os.name == "nt":
-        if identity.verdict() == lifecycle.OWNED:
-            lifecycle.taskkill_tree(identity.pid)
-    else:
-        outcome = lifecycle.terminate_identities(
-            [*(entry.identity for entry in reversed(descendants)), identity],
-            steps=lifecycle.term_kill_steps(10),
-        )
-        if outcome.survivors or outcome.unverified:
-            raise RuntimeError(f"Disposable Notes cleanup could not verify exit: {outcome}")
-    try:
-        logs, _ = server.communicate(timeout=2)
-    except subprocess.TimeoutExpired as error:
-        # Do not turn an unidentified pipe holder into an unbounded wait.
-        server.stdout.close()
-        raise RuntimeError("Disposable Notes output pipe remained open after owned cleanup") from error
-    return logs
 
 
 def main():
@@ -68,10 +36,7 @@ def main():
         server = subprocess.Popen(
             [sys.executable, "-m", "uvicorn", "app:app", "--host", "127.0.0.1", "--port", str(port)],
             cwd=target, env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True,
-            start_new_session=(os.name != "nt"),
         )
-        from src.process_lifecycle import ProcessIdentity
-        server_identity = ProcessIdentity.capture(server.pid)
         result = None
         try:
             origin = f"http://127.0.0.1:{port}"
@@ -97,7 +62,12 @@ def main():
                 print(result.stderr, file=sys.stderr)
             return result.returncode
         finally:
-            logs = _stop_owned_server(server, server_identity)
+            server.terminate()
+            try:
+                logs, _ = server.communicate(timeout=10)
+            except subprocess.TimeoutExpired:
+                server.kill()
+                logs, _ = server.communicate()
             if result is None or result.returncode:
                 print(logs[-8000:], file=sys.stderr)
 

tests/TESTING_STANDARD.md

@@ -99,8 +99,10 @@ cleanup. Specifically:
   `os.environ[...] = ...` that outlives the test.
 - **Current working directory** - never `chdir` without restoring; never assert
   against cwd-relative paths like `./data`. Use a temp workspace helper instead.
-- **Database** - the root `conftest.py` defaults `DATABASE_URL` to an in-memory
+- **Database** - the root `conftest.py` defaults `DATABASE_URL` to a private,
-  SQLite for collection safety. A test that needs a real file-backed DB must opt
+  worker-owned SQLite file for collection safety and consistent schema across
+  TestClient threads. This fallback is not a per-test data fixture. A test that
+  writes or depends on particular rows must opt
   in explicitly via `tests.helpers.sqlite_db.make_temp_sqlite` and bind its
   `SessionLocal` onto the module under test. Do not rely on a persistent
   on-disk DB existing.

tests/conftest.py

@@ -12,9 +12,17 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 # File-backed databases remain fixture-owned. Cleanup restores the caller.
 # Keep the existing generated environment-reference locations stable.
 _database_environment = pytest.MonkeyPatch()
-_database_environment.setenv("DATABASE_URL", "sqlite:///:memory:")
 from tests.helpers.worker_runtime import bootstrap_runtime, configure_runtime
 _runtime_environment = bootstrap_runtime()
+# A file keeps the collection schema consistent across TestClient threads;
+# SingletonThreadPool can evict the populated in-memory main-thread database.
+# One patcher owns this variable, including restoration of an inherited URL.
+_database_environment.setenv(
+    "DATABASE_URL",
+    f"sqlite:///{_runtime_environment.root / 'collection.db'}"
+    if _runtime_environment is not None else "sqlite:///:memory:",
+)
+_collection_database_engine = None
 
 # Pre-import real heavy modules BEFORE any test file's module-level stubs can
 # replace them with MagicMock. Some test files (e.g. test_llm_core_sanitize_*)
@@ -26,6 +34,7 @@ try:
     import sqlalchemy.orm  # noqa: F401
     import core.database  # noqa: F401
     import src.database
+    _collection_database_engine = core.database.engine
 except ImportError:
     pass  # not installed - the stubs below will handle it
 
@@ -100,6 +109,10 @@ def pytest_configure(config):
     only registers marker names; it imports no production module.
     """
     config.add_cleanup(_database_environment.undo)
+    if _collection_database_engine is not None:
+        # Registered after runtime cleanup, so connections close before the
+        # owned file is removed (also required on Windows).
+        config.add_cleanup(_collection_database_engine.dispose)
 
     import pathlib
     from tests._taxonomy import discover_markers

tests/test_database_test_isolation.py

@@ -85,6 +85,9 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa
         import os
         import sys
         import pytest
+        import tempfile
+        from pathlib import Path
+        from sqlalchemy.engine import make_url
 
         def snapshot():
             import core
@@ -107,7 +110,11 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa
         class StateGuard:
             def pytest_sessionstart(self):
                 self.initial = snapshot()
-                assert self.initial[0] == "sqlite:///:memory:"
+                assert self.initial[0] != inherited_url
+                self.default_path = Path(make_url(self.initial[0]).database)
+                assert self.default_path.is_file()
+                assert self.default_path.name == "collection.db"
+                assert self.default_path.parent == Path(tempfile.gettempdir()).parent
 
             def pytest_collection_finish(self):
                 assert snapshot() == self.initial, "collection changed database bindings"
@@ -118,9 +125,11 @@ def test_actual_tests_restore_process_state_and_ignore_inherited_database(tmp_pa
                 assert snapshot() == self.initial, "test leaked database or module state"
 
         inherited_url = os.environ["DATABASE_URL"]
+        guard = StateGuard()
         result = pytest.main(["-q", "-p", "no:cacheprovider", *sys.argv[1:]],
-                             plugins=[StateGuard()])
+                             plugins=[guard])
         assert os.environ["DATABASE_URL"] == inherited_url
+        assert not guard.default_path.exists(), "collection database outlived its runtime"
         raise SystemExit(result)
     ''')
     result = subprocess.run(

tests/test_notes_native_cleanup.py

@@ -1,43 +0,0 @@
-"""A detached Notes descendant must not hold output open or kill a bystander."""
-import os
-import runpy
-import subprocess
-import sys
-from pathlib import Path
-
-import pytest
-from src import process_lifecycle as lifecycle
-
-ROOT = Path(__file__).resolve().parents[1]
-
-
-@pytest.mark.skipif(os.name == "nt", reason="POSIX detached-session regression; Windows uses taskkill tree")
-def test_notes_cleanup_reaps_detached_pipe_holder_and_preserves_bystander():
-    stop = runpy.run_path(str(ROOT / "scripts/verify_notes_browser_runtime.py"))["_stop_owned_server"]
-    child = "import time; time.sleep(60)"
-    program = ("import subprocess,sys,time; "
-               f"p=subprocess.Popen([sys.executable,'-c',{child!r}],start_new_session=True); "
-               "print(p.pid,flush=True); time.sleep(60)")
-    with subprocess.Popen([sys.executable, "-c", child]) as bystander:
-        server = subprocess.Popen([sys.executable, "-c", program], stdout=subprocess.PIPE,
-                                  stderr=subprocess.STDOUT, text=True, start_new_session=True)
-        root_identity = lifecycle.ProcessIdentity.capture(server.pid)
-        detached = None
-        try:
-            child_pid = int(server.stdout.readline())
-            detached = lifecycle.ProcessIdentity.capture(child_pid)
-            assert os.getpgid(child_pid) == child_pid
-            stop(server, root_identity)
-            assert server.poll() is not None
-            assert detached.exited()
-            assert bystander.poll() is None, "cleanup touched an unrelated process"
-        finally:
-            # Test-owned fallback cleanup also runs when the negative control
-            # intentionally substitutes the old parent-only teardown.
-            targets = [identity for identity in (detached, root_identity) if identity]
-            lifecycle.terminate_identities(targets, steps=lifecycle.term_kill_steps(1))
-            server.wait(timeout=3)
-            if server.stdout:
-                server.stdout.close()
-            bystander.terminate()
-            bystander.wait(timeout=3)

tests/test_worker_runtime.py

@@ -79,3 +79,32 @@ def test_tmp_path_under_private_runtime_fits_unix_socket(tmp_path):
     # witness. A nested pytest-of-<user> basetemp made it 110 bytes under xdist.
     with socket.socket(socket.AF_UNIX) as sock:
         sock.bind(str(tmp_path / "tmux.sock"))
+
+
+def test_collection_database_survives_thread_connection_churn():
+    """TestClient threads must not lose the collection database or its schema.
+
+    SingletonThreadPool's default five-connection limit used to close the main
+    thread's in-memory database after repeated route clients. Each new thread
+    also saw an empty database. Exercise more threads than that limit and then
+    verify both compatibility factories still see all registered tables.
+    """
+    from concurrent.futures import ThreadPoolExecutor
+    from sqlalchemy import inspect
+    import core.database as database
+    import src.database as compatibility
+
+    expected = set(database.Base.metadata.tables)
+
+    def table_names(factory):
+        with factory() as session:
+            return set(inspect(session.connection()).get_table_names())
+
+    assert {"documents", "model_endpoints", "mcp_servers", "calendars"} <= expected
+    assert expected <= table_names(database.SessionLocal)
+    for _ in range(8):
+        # A fresh executor creates a fresh thread rather than reusing one.
+        with ThreadPoolExecutor(max_workers=1) as executor:
+            assert expected <= executor.submit(table_names, database.SessionLocal).result()
+    assert expected <= table_names(database.SessionLocal)
+    assert expected <= table_names(compatibility.SessionLocal)