From d9edfe10590665ba773b27019b978899b2095795 Mon Sep 17 00:00:00 2001 From: tonydzi Date: Mon, 14 Sep 2026 08:18:55 -0700 Subject: [PATCH] feat(config): accept SQLite URLs in database_url (#539) Implements the SQLite half of #539, as scoped by the maintainer's triage: when database_backend is sqlite (default) and database_url is set to a sqlite+aiosqlite:// URL, app_database_path/database_path derive the DB file from it (relative = three slashes/cwd-relative, absolute = four slashes, per SQLAlchemy's own convention) instead of always using ~/.basic-memory/memory.db. WAL/pragma setup in db.py and Alembic migrations already key off this same path, so both apply to the custom file unchanged. Postgres behavior and database_url handling are untouched; search_path/schema work is out of scope here. Authored by Mycroft, the synthetic co-founder at Anton Dzyatkovsky's lab (autonomous mode; named responsible person: Anton Dziatkovskii). The test runs above were independently re-executed before submission. Assisted-by: Claude Code / claude-sonnet Machine: MacBook-Anton Account: a@ Operator: robot:connector-butcher Signed-off-by: tonydzi --- src/basic_memory/config_models.py | 53 +++++++++++++- tests/db/test_sqlite_database_url.py | 104 +++++++++++++++++++++++++++ tests/test_config.py | 67 +++++++++++++++++ 3 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 tests/db/test_sqlite_database_url.py diff --git a/src/basic_memory/config_models.py b/src/basic_memory/config_models.py index 5b0044564..010e7f469 100644 --- a/src/basic_memory/config_models.py +++ b/src/basic_memory/config_models.py @@ -11,6 +11,7 @@ from loguru import logger from pydantic import AliasChoices, BaseModel, Field, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +from sqlalchemy.engine import make_url from basic_memory.config_migrations import ( migrate_legacy_projects, @@ -942,6 +943,50 @@ def model_post_init(self, __context: Any) -> None: elif self.default_project is not None and self.default_project not in self.projects: self.default_project = next(iter(self.projects.keys())) + def _resolve_sqlite_database_url_path(self) -> Optional[Path]: + """Derive the SQLite database file path from an explicit ``database_url``. + + Returns ``None`` when ``database_url`` is unset or the backend is not + SQLite, so ``app_database_path`` falls back to the existing + ``data_dir_path / APP_DATABASE_NAME`` default (Postgres deployments + never use this; their ``database_url`` is consumed directly in db.py). + + When a SQLite ``database_url`` is set, this is the single place that + parses it, following SQLAlchemy's own convention: three slashes is a + relative path (resolved against the current working directory), four + is absolute (issue #539). This lets each project/worktree point at + its own index file — e.g. + ``BASIC_MEMORY_DATABASE_URL=sqlite+aiosqlite:///.basic-memory/memory.db`` — + instead of always sharing the global data-dir database. + """ + if self.database_backend != DatabaseBackend.SQLITE or not self.database_url: + return None + + try: + url = make_url(self.database_url) + except Exception as error: + raise ValueError( + f"Invalid database_url for sqlite backend: {self.database_url!r} ({error})" + ) from error + + if not url.drivername.startswith("sqlite"): + raise ValueError( + "database_url must use a sqlite driver (e.g. " + "'sqlite+aiosqlite:///path/to.db') when database_backend='sqlite'; " + f"got {url.drivername!r}. Set database_backend='postgres' for a " + "Postgres database_url, or unset database_url to use the default " + "SQLite path." + ) + + if not url.database: + raise ValueError( + "database_url for the sqlite backend must include a file path, e.g. " + "'sqlite+aiosqlite:///.basic-memory/memory.db' " + "(in-memory SQLite is not supported for the app database)." + ) + + return Path(url.database) + @property def app_database_path(self) -> Path: """Get the path to the app-level database. @@ -950,9 +995,13 @@ def app_database_path(self) -> Path: across all projects. Uses BASIC_MEMORY_CONFIG_DIR when set so each process/worktree can - isolate both config and database state. + isolate both config and database state. When ``database_url`` is set + to a SQLite URL, that path is used instead — see + ``_resolve_sqlite_database_url_path``. """ - database_path = self.data_dir_path / APP_DATABASE_NAME + database_path = self._resolve_sqlite_database_url_path() or ( + self.data_dir_path / APP_DATABASE_NAME + ) if not database_path.exists(): # pragma: no cover database_path.parent.mkdir(parents=True, exist_ok=True) database_path.touch() diff --git a/tests/db/test_sqlite_database_url.py b/tests/db/test_sqlite_database_url.py new file mode 100644 index 000000000..c2e60429c --- /dev/null +++ b/tests/db/test_sqlite_database_url.py @@ -0,0 +1,104 @@ +"""End-to-end coverage for issue #539's SQLite half: BASIC_MEMORY_DATABASE_URL / +config ``database_url`` accepting a ``sqlite+aiosqlite://`` URL and the engine +actually reading/writing the file it points at (not the default +~/.basic-memory/memory.db). + +Config-level URL parsing (relative/absolute, error cases) is covered in +tests/test_config.py::TestBasicMemoryConfig - this module proves the engine +layer honors config.app_database_path once it is derived from database_url, +including the WAL/pragma setup in db.py. +""" + +from pathlib import Path + +import pytest +from sqlalchemy import text + +from basic_memory.config import BasicMemoryConfig, DatabaseBackend +from basic_memory.db import DatabaseType, engine_session_factory + + +@pytest.mark.asyncio +async def test_engine_creates_sqlite_file_at_custom_relative_database_url(tmp_path, monkeypatch): + """The engine writes to the custom path derived from a relative database_url.""" + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + + config = BasicMemoryConfig( + env="test", + database_backend=DatabaseBackend.SQLITE, + database_url="sqlite+aiosqlite:///.basic-memory/memory.db", + skip_initialization_sync=True, + ) + + db_path = config.app_database_path + assert db_path == Path(".basic-memory/memory.db") + + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM, config) as ( + engine, + session_maker, + ): + async with session_maker() as session: + # Force a real connection/write so SQLite actually creates the file + # and applies the WAL pragma from _configure_sqlite_connection. + await session.execute(text("CREATE TABLE probe (id INTEGER PRIMARY KEY)")) + await session.execute(text("INSERT INTO probe (id) VALUES (1)")) + await session.commit() + + result = await session.execute(text("PRAGMA journal_mode")) + assert result.scalar() == "wal" + + resolved_db = tmp_path / ".basic-memory" / "memory.db" + assert resolved_db.exists() + assert resolved_db.stat().st_size > 0 + + # The default per-user location must never have been created. + assert not (tmp_path / ".basic-memory-home" / "memory.db").exists() + + +@pytest.mark.asyncio +async def test_engine_creates_sqlite_file_at_custom_absolute_database_url(tmp_path, monkeypatch): + """The engine writes to the custom path derived from an absolute database_url.""" + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + custom_db = tmp_path / "instance-a" / "index.db" + + config = BasicMemoryConfig( + env="test", + database_backend=DatabaseBackend.SQLITE, + database_url=f"sqlite+aiosqlite:///{custom_db}", + skip_initialization_sync=True, + ) + + db_path = config.app_database_path + assert db_path == custom_db + + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM, config) as ( + engine, + session_maker, + ): + async with session_maker() as session: + await session.execute(text("CREATE TABLE probe (id INTEGER PRIMARY KEY)")) + await session.commit() + + assert custom_db.exists() + + +@pytest.mark.asyncio +async def test_engine_uses_default_path_when_database_url_unset(tmp_path, monkeypatch): + """Unset database_url keeps the pre-existing default SQLite path unchanged.""" + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(tmp_path / "state")) + + config = BasicMemoryConfig(env="test", skip_initialization_sync=True) + + db_path = config.app_database_path + assert db_path == tmp_path / "state" / "memory.db" + + async with engine_session_factory(db_path, DatabaseType.FILESYSTEM, config) as ( + engine, + session_maker, + ): + async with session_maker() as session: + await session.execute(text("CREATE TABLE probe (id INTEGER PRIMARY KEY)")) + await session.commit() + + assert (tmp_path / "state" / "memory.db").exists() diff --git a/tests/test_config.py b/tests/test_config.py index 19caff170..7671c26e8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -284,6 +284,73 @@ def test_app_database_path_defaults_to_home_data_dir(self, config_home, monkeypa assert config.data_dir_path == config_home / ".basic-memory" assert config.app_database_path == config_home / ".basic-memory" / "memory.db" + def test_app_database_path_from_sqlite_database_url_relative(self, config_home, monkeypatch): + """A relative sqlite database_url resolves against the cwd (issue #539). + + Three slashes = relative path, per SQLAlchemy's own URL convention - + this is what lets a git worktree point BASIC_MEMORY_DATABASE_URL at a + project-local index instead of sharing ~/.basic-memory/memory.db. + """ + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + project_dir = config_home / "worktree" + project_dir.mkdir() + monkeypatch.chdir(project_dir) + + config = BasicMemoryConfig(database_url="sqlite+aiosqlite:///.basic-memory/memory.db") + + assert config.app_database_path == Path(".basic-memory/memory.db") + assert (project_dir / ".basic-memory" / "memory.db").exists() + + def test_app_database_path_from_sqlite_database_url_absolute(self, config_home, monkeypatch): + """An absolute sqlite database_url (four slashes) is used verbatim (issue #539).""" + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + custom_db = config_home / "custom-index" / "memory.db" + + config = BasicMemoryConfig(database_url=f"sqlite+aiosqlite:///{custom_db}") + + assert config.app_database_path == custom_db + assert custom_db.exists() + # The default location must not have been touched. + assert not (config_home / ".basic-memory" / "memory.db").exists() + + def test_app_database_path_ignores_database_url_for_postgres_backend( + self, config_home, monkeypatch + ): + """database_url only overrides the SQLite path when database_backend='sqlite'. + + Postgres deployments keep consuming database_url directly in db.py; this + property must not be repurposed for them (out of scope for #539's + SQLite-only half). + """ + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + config = BasicMemoryConfig( + database_backend="postgres", + database_url="postgresql+asyncpg://user:pass@localhost/db", + skip_initialization_sync=True, + ) + + assert config.app_database_path == config_home / ".basic-memory" / "memory.db" + + def test_app_database_path_rejects_non_sqlite_url_for_sqlite_backend( + self, config_home, monkeypatch + ): + """A non-sqlite database_url with the (default) sqlite backend is a clear config error.""" + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + config = BasicMemoryConfig( + database_url="postgresql+asyncpg://user:pass@localhost/db", + ) + + with pytest.raises(ValueError, match="sqlite driver"): + _ = config.app_database_path + + def test_app_database_path_rejects_sqlite_url_without_path(self, config_home, monkeypatch): + """An in-memory sqlite database_url has no file path - reject it clearly.""" + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + config = BasicMemoryConfig(database_url="sqlite+aiosqlite://") + + with pytest.raises(ValueError, match="must include a file path"): + _ = config.app_database_path + def test_semantic_embedding_cache_dir_field_stays_none_by_default( self, config_home, monkeypatch ):