From e2e65575d69229e1786fe93b45ef7d858aa5d449 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Wed, 15 Apr 2026 22:55:25 -0500 Subject: [PATCH] fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites (#744) Signed-off-by: Drew Cain Co-authored-by: Claude Opus 4.6 (1M context) --- .../cli/commands/cloud/rclone_commands.py | 6 ++- src/basic_memory/ignore_utils.py | 11 +++-- src/basic_memory/services/project_service.py | 8 ++-- src/basic_memory/sync/watch_service.py | 2 +- src/basic_memory/utils.py | 9 +++- tests/cli/test_ignore_utils.py | 17 ++++++++ tests/services/test_project_service.py | 21 ++++++++++ tests/sync/test_watch_service.py | 21 +++++++++- tests/test_rclone_commands.py | 11 ++++- tests/utils/test_setup_logging.py | 42 +++++++++++++++++++ 10 files changed, 134 insertions(+), 14 deletions(-) diff --git a/src/basic_memory/cli/commands/cloud/rclone_commands.py b/src/basic_memory/cli/commands/cloud/rclone_commands.py index 09cb16ce..463b43f1 100644 --- a/src/basic_memory/cli/commands/cloud/rclone_commands.py +++ b/src/basic_memory/cli/commands/cloud/rclone_commands.py @@ -20,6 +20,7 @@ from loguru import logger from rich.console import Console from basic_memory.cli.commands.cloud.rclone_installer import is_rclone_installed +from basic_memory.config import resolve_data_dir from basic_memory.utils import normalize_project_path console = Console() @@ -138,13 +139,16 @@ def get_bmignore_filter_path() -> Path: def get_project_bisync_state(project_name: str) -> Path: """Get path to project's bisync state directory. + Honors ``BASIC_MEMORY_CONFIG_DIR`` so isolated instances each keep their + own bisync state alongside their config. + Args: project_name: Name of the project Returns: Path to bisync state directory for this project """ - return Path.home() / ".basic-memory" / "bisync-state" / project_name + return resolve_data_dir() / "bisync-state" / project_name def bisync_initialized(project_name: str) -> bool: diff --git a/src/basic_memory/ignore_utils.py b/src/basic_memory/ignore_utils.py index e68fc5a7..df43f3ed 100644 --- a/src/basic_memory/ignore_utils.py +++ b/src/basic_memory/ignore_utils.py @@ -4,6 +4,8 @@ import fnmatch from pathlib import Path from typing import Set +from basic_memory.config import resolve_data_dir + # Common directories and patterns to ignore by default # These are used as fallback if .bmignore doesn't exist @@ -61,9 +63,11 @@ def get_bmignore_path() -> Path: """Get path to .bmignore file. Returns: - Path to ~/.basic-memory/.bmignore + Path to /.bmignore, honoring + ``BASIC_MEMORY_CONFIG_DIR`` so isolated instances each keep their + own ignore file. """ - return Path.home() / ".basic-memory" / ".bmignore" + return resolve_data_dir() / ".bmignore" def create_default_bmignore() -> None: @@ -176,7 +180,8 @@ def load_gitignore_patterns(base_path: Path, use_gitignore: bool = True) -> Set[ """Load gitignore patterns from .gitignore file and .bmignore. Combines patterns from: - 1. ~/.basic-memory/.bmignore (user's global ignore patterns) + 1. /.bmignore (user's global ignore patterns, honors + BASIC_MEMORY_CONFIG_DIR) 2. {base_path}/.gitignore (project-specific patterns, if use_gitignore=True) Args: diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index 6ede7df2..e01d43b5 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -1137,12 +1137,10 @@ class ProjectService: # Get watch service status if available watch_status = None - watch_status_path = Path.home() / ".basic-memory" / WATCH_STATUS_JSON + watch_status_path = self.config_manager.config.data_dir_path / WATCH_STATUS_JSON if watch_status_path.exists(): - try: # pragma: no cover - watch_status = json.loads( # pragma: no cover - watch_status_path.read_text(encoding="utf-8") - ) + try: + watch_status = json.loads(watch_status_path.read_text(encoding="utf-8")) except Exception: # pragma: no cover pass diff --git a/src/basic_memory/sync/watch_service.py b/src/basic_memory/sync/watch_service.py index 4a7d29b7..549fe4a2 100644 --- a/src/basic_memory/sync/watch_service.py +++ b/src/basic_memory/sync/watch_service.py @@ -89,7 +89,7 @@ class WatchService: self.app_config = app_config self.project_repository = project_repository self.state = WatchServiceState() - self.status_path = Path.home() / ".basic-memory" / WATCH_STATUS_JSON + self.status_path = app_config.data_dir_path / WATCH_STATUS_JSON self.status_path.parent.mkdir(parents=True, exist_ok=True) self._ignore_patterns_cache: dict[Path, Set[str]] = {} self._sync_service_factory = sync_service_factory diff --git a/src/basic_memory/utils.py b/src/basic_memory/utils.py index 1dceb745..74b02896 100644 --- a/src/basic_memory/utils.py +++ b/src/basic_memory/utils.py @@ -262,7 +262,8 @@ def setup_logging( Args: log_level: DEBUG, INFO, WARNING, ERROR - log_to_file: Write to ~/.basic-memory/basic-memory.log with rotation + log_to_file: Write to /basic-memory.log with rotation + (honors BASIC_MEMORY_CONFIG_DIR) log_to_stdout: Write to stderr (for Docker/cloud deployments) structured_context: Bind tenant_id, fly_region, etc. for cloud observability """ @@ -281,7 +282,11 @@ def setup_logging( # Why: multiple basic-memory processes can share the same log directory at once. # Outcome: use per-process log files on Windows so log rotation stays local. log_filename = f"basic-memory-{os.getpid()}.log" if os.name == "nt" else "basic-memory.log" - log_path = Path.home() / ".basic-memory" / log_filename + # Deferred import: basic_memory.config imports from this module at load time, + # so resolving the data dir via a top-level import would cycle. + from basic_memory.config import resolve_data_dir + + log_path = resolve_data_dir() / log_filename log_path.parent.mkdir(parents=True, exist_ok=True) if os.name == "nt": _cleanup_windows_log_files(log_path.parent, log_path.name) diff --git a/tests/cli/test_ignore_utils.py b/tests/cli/test_ignore_utils.py index b6470b6c..c86146e4 100644 --- a/tests/cli/test_ignore_utils.py +++ b/tests/cli/test_ignore_utils.py @@ -5,12 +5,29 @@ from pathlib import Path from basic_memory.ignore_utils import ( DEFAULT_IGNORE_PATTERNS, + get_bmignore_path, load_gitignore_patterns, should_ignore_path, filter_files, ) +def test_get_bmignore_path_honors_basic_memory_config_dir(tmp_path, monkeypatch): + """Regression guard for #742: .bmignore must follow BASIC_MEMORY_CONFIG_DIR.""" + custom_dir = tmp_path / "instance-y" / "state" + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir)) + + assert get_bmignore_path() == custom_dir / ".bmignore" + + +def test_get_bmignore_path_defaults_under_home(tmp_path, monkeypatch): + """Without BASIC_MEMORY_CONFIG_DIR, .bmignore lives under ~/.basic-memory.""" + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) + monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path)) + + assert get_bmignore_path() == tmp_path / ".basic-memory" / ".bmignore" + + def test_load_default_patterns_only(): """Test loading default patterns when no .gitignore exists.""" with tempfile.TemporaryDirectory() as temp_dir: diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index c4c0cb26..57063f18 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -126,6 +126,27 @@ async def test_get_system_status(project_service: ProjectService): assert status.database_size +@pytest.mark.asyncio +async def test_get_system_status_reads_watch_status_from_config_dir( + project_service: ProjectService, tmp_path, monkeypatch +): + """Regression guard for #742: watch-status.json is read from the configured + data dir, not hardcoded to ~/.basic-memory.""" + import json as _json + from basic_memory.config import WATCH_STATUS_JSON + + custom_dir = tmp_path / "instance-v" / "state" + custom_dir.mkdir(parents=True) + (custom_dir / WATCH_STATUS_JSON).write_text( + _json.dumps({"running": True, "error_count": 7}), encoding="utf-8" + ) + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir)) + + status = project_service.get_system_status() + + assert status.watch_status == {"running": True, "error_count": 7} + + @pytest.mark.asyncio async def test_get_statistics(project_service: ProjectService, test_graph, test_project): """Test getting statistics.""" diff --git a/tests/sync/test_watch_service.py b/tests/sync/test_watch_service.py index ee659a6f..5ea4b237 100644 --- a/tests/sync/test_watch_service.py +++ b/tests/sync/test_watch_service.py @@ -3,12 +3,14 @@ import asyncio import json from pathlib import Path +from unittest.mock import MagicMock import pytest from watchfiles import Change +from basic_memory.config import BasicMemoryConfig, WATCH_STATUS_JSON from basic_memory.models.project import Project -from basic_memory.sync.watch_service import WatchServiceState +from basic_memory.sync.watch_service import WatchService, WatchServiceState async def create_test_file(path: Path, content: str = "test content") -> None: @@ -25,6 +27,23 @@ def test_watch_service_init(watch_service, project_config): assert watch_service.status_path.parent.exists() +def test_watch_service_status_path_honors_basic_memory_config_dir(tmp_path, monkeypatch): + """Regression guard for #742: watch-status.json follows BASIC_MEMORY_CONFIG_DIR. + + WatchService previously hardcoded ``Path.home() / ".basic-memory"`` which + split state across instances running under an isolated config dir. Ensure + the status path now lives under the configured data dir. + """ + custom_dir = tmp_path / "instance-z" / "state" + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir)) + + app_config = BasicMemoryConfig(projects={"main": {"path": str(tmp_path / "project")}}) + service = WatchService(app_config=app_config, project_repository=MagicMock()) + + assert service.status_path == custom_dir / WATCH_STATUS_JSON + assert service.status_path.parent.exists() + + def test_state_add_event(): """Test adding events to state.""" state = WatchServiceState() diff --git a/tests/test_rclone_commands.py b/tests/test_rclone_commands.py index 9b1c7c4e..83d9d412 100644 --- a/tests/test_rclone_commands.py +++ b/tests/test_rclone_commands.py @@ -83,12 +83,21 @@ def test_get_project_remote_strips_app_data_prefix(): assert get_project_remote(project, "my-bucket") == "basic-memory-cloud:my-bucket/research" -def test_get_project_bisync_state(): +def test_get_project_bisync_state(monkeypatch): + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) state_path = get_project_bisync_state("research") expected = Path.home() / ".basic-memory" / "bisync-state" / "research" assert state_path == expected +def test_get_project_bisync_state_honors_basic_memory_config_dir(tmp_path, monkeypatch): + """Regression guard for #742: bisync state dir follows BASIC_MEMORY_CONFIG_DIR.""" + custom_dir = tmp_path / "instance-w" / "state" + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir)) + + assert get_project_bisync_state("research") == custom_dir / "bisync-state" / "research" + + def test_bisync_initialized_false_when_not_exists(tmp_path, monkeypatch): monkeypatch.setattr( "basic_memory.cli.commands.cloud.rclone_commands.get_project_bisync_state", diff --git a/tests/utils/test_setup_logging.py b/tests/utils/test_setup_logging.py index 316c29b4..9bf02a8a 100644 --- a/tests/utils/test_setup_logging.py +++ b/tests/utils/test_setup_logging.py @@ -2,6 +2,7 @@ import os import sys +from pathlib import Path from basic_memory import utils @@ -11,6 +12,7 @@ def test_setup_logging_uses_shared_log_file_off_windows(monkeypatch, tmp_path) - added_sinks: list[str] = [] monkeypatch.setenv("BASIC_MEMORY_ENV", "dev") + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) monkeypatch.setattr(utils.os, "name", "posix") monkeypatch.setattr(utils.Path, "home", lambda: tmp_path) monkeypatch.setattr(utils.logger, "remove", lambda *args, **kwargs: None) @@ -32,6 +34,7 @@ def test_setup_logging_uses_per_process_log_file_on_windows(monkeypatch, tmp_pat added_sinks: list[str] = [] monkeypatch.setenv("BASIC_MEMORY_ENV", "dev") + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) monkeypatch.setattr(utils.os, "name", "nt") monkeypatch.setattr(utils.os, "getpid", lambda: 4242) monkeypatch.setattr(utils.Path, "home", lambda: tmp_path) @@ -63,6 +66,7 @@ def test_setup_logging_trims_stale_windows_pid_logs(monkeypatch, tmp_path) -> No stale_logs.append(log_path) monkeypatch.setenv("BASIC_MEMORY_ENV", "dev") + monkeypatch.delenv("BASIC_MEMORY_CONFIG_DIR", raising=False) monkeypatch.setattr(utils.os, "name", "nt") monkeypatch.setattr(utils.os, "getpid", lambda: 4242) monkeypatch.setattr(utils.Path, "home", lambda: tmp_path) @@ -82,6 +86,44 @@ def test_setup_logging_trims_stale_windows_pid_logs(monkeypatch, tmp_path) -> No ] +def test_setup_logging_honors_basic_memory_config_dir(monkeypatch, tmp_path) -> None: + """Regression guard for #742: log path must follow BASIC_MEMORY_CONFIG_DIR. + + Prior to #742 the log path was hardcoded to ``~/.basic-memory/``, which + split state across instances when users set BASIC_MEMORY_CONFIG_DIR to + isolate config and the database elsewhere. + + Asserts on the log *directory* rather than the exact filename because + Windows uses a per-process ``basic-memory-.log`` while POSIX + shares a single ``basic-memory.log``. The thing this regression guard + cares about is that the log lives under the redirected config dir, + not at ``Path.home() / ".basic-memory"``. Patching ``utils.os.name`` + to force one branch would break ``Path(str)`` dispatch on the other + platform, so we stay platform-agnostic. + """ + added_sinks: list[str] = [] + + custom_dir = tmp_path / "instance-x" / "state" + monkeypatch.setenv("BASIC_MEMORY_ENV", "dev") + monkeypatch.setenv("BASIC_MEMORY_CONFIG_DIR", str(custom_dir)) + monkeypatch.setattr(utils.logger, "remove", lambda *args, **kwargs: None) + monkeypatch.setattr( + utils.logger, + "add", + lambda sink, **kwargs: added_sinks.append(str(sink)), + ) + monkeypatch.setattr(utils.telemetry, "get_logfire_handler", lambda: None) + monkeypatch.setattr(utils.telemetry, "pop_telemetry_warnings", lambda: []) + + utils.setup_logging(log_to_file=True) + + assert len(added_sinks) == 1 + log_path = Path(added_sinks[0]) + assert log_path.parent == custom_dir + assert log_path.name.startswith("basic-memory") + assert log_path.suffix == ".log" + + def test_setup_logging_test_env_uses_stderr_only(monkeypatch) -> None: """Test mode should add one stderr sink and return before other branches run.""" added_sinks: list[object] = []