mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites (#744)
Signed-off-by: Drew Cain <groksrc@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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 <basic-memory data dir>/.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. <basic-memory data dir>/.bmignore (user's global ignore patterns, honors
|
||||
BASIC_MEMORY_CONFIG_DIR)
|
||||
2. {base_path}/.gitignore (project-specific patterns, if use_gitignore=True)
|
||||
|
||||
Args:
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 data dir>/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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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-<pid>.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] = []
|
||||
|
||||
Reference in New Issue
Block a user