fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites

[groksrc]

Summary

Follow-up to #741 / #743. Several modules were hardcoding Path.home() / ".basic-memory" when resolving Basic Memory state, which ignored BASIC_MEMORY_CONFIG_DIR and caused split state when users isolated config and the database under a redirected data directory:

• utils.setup_logging — log file
• ignore_utils.get_bmignore_path — .bmignore
• sync.watch_service.WatchService.status_path — watch-status.json
• services.project_service.get_system_status — watch-status.json read
• cli.commands.cloud.rclone_commands.get_project_bisync_state — bisync state dir

Route each call site through resolve_data_dir() (introduced in #743) or the already-threaded config where one is available, so all of these follow the same override.

Fixes #742.

Stacked on #743. Merge #743 first. GitHub's base branch for this PR is fix/fastembed-cache-default-path; GitHub will auto-retarget to main after #743 lands.

What changed

• src/basic_memory/utils.py — setup_logging() now uses resolve_data_dir() / log_filename via a deferred import (avoids the config.py → utils.py cycle).
• src/basic_memory/ignore_utils.py — get_bmignore_path() uses resolve_data_dir() / ".bmignore".
• src/basic_memory/sync/watch_service.py — WatchService.status_path uses app_config.data_dir_path / WATCH_STATUS_JSON.
• src/basic_memory/services/project_service.py — get_system_status() uses self.config_manager.config.data_dir_path / WATCH_STATUS_JSON.
• src/basic_memory/cli/commands/cloud/rclone_commands.py — get_project_bisync_state() uses resolve_data_dir() / "bisync-state" / project_name.

Test plan

• 5 new regression tests, one per call site, all named *_honors_basic_memory_config_dir for easy grep:
  • test_setup_logging_honors_basic_memory_config_dir
  • test_get_bmignore_path_honors_basic_memory_config_dir (+ default-path coverage)
  • test_watch_service_status_path_honors_basic_memory_config_dir
  • test_get_system_status_reads_watch_status_from_config_dir
  • test_get_project_bisync_state_honors_basic_memory_config_dir
• Updated existing test_setup_logging_* Windows/POSIX tests and test_get_project_bisync_state to clear BASIC_MEMORY_CONFIG_DIR explicitly, so their behavior doesn't depend on ambient env state.
• 225 tests pass in directly affected files.
• 700 tests pass across tests/sync, tests/services, tests/utils, tests/test_config.py (broader sanity check).
• Ruff lint + format + ty typecheck all green.

🤖 Generated with Claude Code

[github-actions]

Claude finished @groksrc's task in 2m 35s —— View job

---

PR Review: fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites

• Gather context and read changed files
• Review source code changes
• Review test coverage
• Verify checklist items
• Post final review

---

Summary

This is a clean, well-scoped follow-up to #743. Each of the five hardcoded Path.home() / ".basic-memory" call sites has been corrected with a minimal, targeted change. The approach is consistent across all sites: use resolve_data_dir() when no config object is in scope, or use app_config.data_dir_path / config_manager.config.data_dir_path when one is already threaded through.

Overall: LGTM with minor feedback below.

---

Code Quality Checklist

Item	Status
Follows Basic Memory coding conventions	✅
Python 3.12+ type annotations and async patterns	✅
FastAPI / Typer conventions followed	✅
100-char line length maintained	✅
No commented-out code	✅
Unit tests for changed call sites	✅ 5 new regression tests
Edge-case coverage (env var absent)	✅ (test_get_bmignore_path_defaults_under_home, existing tests now explicitly clear the var)
100% test coverage maintained	⚠️ see below
Documentation / docstrings updated	⚠️ two stale docstrings (see below)
Preserves local-first architecture	✅
No hardcoded secrets	✅
Windows path compatibility	✅ (Path operations are platform-agnostic; per-process log file naming already handled)
No emojis/unicode in console/log output	✅

---

Findings

1. Stale docstring — utils.py:265

The setup_logging docstring was not updated alongside the implementation:

# utils.py:265 — still says the old path
log_to_file: Write to ~/.basic-memory/basic-memory.log with rotation

Should be <basic-memory data dir>/basic-memory.log (or reference BASIC_MEMORY_CONFIG_DIR). Fix this →

2. Stale docstring — ignore_utils.py:183

load_gitignore_patterns still references the old path:

# ignore_utils.py:183
1. ~/.basic-memory/.bmignore (user's global ignore patterns)

Since get_bmignore_path() was updated, this docstring should read <basic-memory data dir>/.bmignore. Fix this →

3. # pragma: no cover may now be removable — project_service.py:1142–1146

if watch_status_path.exists():
    try:  # pragma: no cover
        watch_status = json.loads(  # pragma: no cover
            watch_status_path.read_text(encoding="utf-8")
        )
    except Exception:  # pragma: no cover
        pass

The new test test_get_system_status_reads_watch_status_from_config_dir creates the watch-status file and calls get_system_status(), so this try block is now exercised. If coverage is confirming these lines are hit, removing the # pragma: no cover annotations would keep the "100% coverage" claim accurate and self-consistent. Worth checking whether removing them keeps coverage at 100%.

4. Minor — remaining ~/.basic-memory references in bisync_commands.py docstrings (lines 48–60, 104)

These are comments only (the underlying calls to get_bmignore_path() are now correct), so there's no functional impact. Worth a follow-up cleanup pass but not blocking.

---

Correctness Notes

• Circular import handled correctly. config.py imports from utils.py, so the deferred from basic_memory.config import resolve_data_dir inside setup_logging is the right approach. Top-level imports in ignore_utils.py and rclone_commands.py are safe because config.py doesn't import those modules.
• WatchService.status_path evaluated at init-time. Correct — the path is resolved once when the service starts, which is the intended behavior.
• data_dir_path property calls resolve_data_dir() dynamically. Confirmed — no caching concern; the env var is read fresh on each property access, so monkeypatching in tests works reliably.
• Test for get_system_status uses @pytest.mark.asyncio on a synchronous method. Fine — async tests can call sync methods, and the project_service fixture is async.

[github-actions]

Claude finished @groksrc's task in 2m 47s —— View job

---

PR Review: fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites

• Gather context
• Read changed source files
• Check previous findings status
• Post final review

---

What changed in the latest commit (99af4ea)

The new commit makes test_setup_logging_honors_basic_memory_config_dir platform-agnostic:

• Removed monkeypatch.setattr(utils.os, "name", "posix") — no longer forces POSIX branch
• Changed assertion from an exact filename match (basic-memory.log) to directory + prefix + suffix checks, which works on both Windows (per-process basic-memory-<pid>.log) and POSIX (shared basic-memory.log)

This is the correct fix — the original test was broken on Windows because it hardcoded the POSIX filename while forcing os.name.

---

Code Quality Checklist

Item	Status
Follows Basic Memory coding conventions	✅
Python 3.12+ type annotations and async patterns	✅
FastAPI / Typer conventions followed	✅
100-char line length maintained	✅
No commented-out code	✅
Unit tests for all 5 changed call sites	✅
Edge-case coverage (env var absent)	✅
100% test coverage claim is accurate	⚠️ # pragma: no cover on now-exercised lines (see below)
Documentation / docstrings updated	⚠️ Two stale docstrings (see below)
Preserves local-first architecture	✅
No hardcoded secrets	✅
Windows path compatibility	✅ (latest commit fixed the test)
No emojis/unicode in console/log output	✅

---

Outstanding Findings (3 items, carried from previous reviews — not yet addressed)

1. Stale docstring — utils.py:265

# Current (stale)
log_to_file: Write to ~/.basic-memory/basic-memory.log with rotation

# Should be
log_to_file: Write to <basic-memory data dir>/basic-memory.log with rotation (honors BASIC_MEMORY_CONFIG_DIR)

Fix this →

2. Stale docstring — ignore_utils.py:183

# Current (stale)
1. ~/.basic-memory/.bmignore (user's global ignore patterns)

# Should be
1. <basic-memory data dir>/.bmignore (user's global ignore patterns, honors BASIC_MEMORY_CONFIG_DIR)

Fix this →

3. # pragma: no cover on now-exercised lines — project_service.py:1141–1147

if watch_status_path.exists():
    try:  # pragma: no cover            ← now exercised — remove this
        watch_status = json.loads(  # pragma: no cover   ← now exercised — remove this
            watch_status_path.read_text(encoding="utf-8")
        )
    except Exception:  # pragma: no cover   ← keep — still hard to test
        pass

test_get_system_status_reads_watch_status_from_config_dir creates the watch-status file and calls get_system_status(), so the try: / json.loads / .read_text lines are exercised. Remove # pragma: no cover from those two lines only; leave it on except Exception:.

Fix this →

---

Note: I attempted to push these three fixes directly but lacked write access to the branch (403). The changes are minimal and mechanical — any of the "Fix this →" links above will apply them via Claude Code.

Overall: The five functional changes are correct and well-tested. The latest commit correctly resolves the Windows test-compatibility issue. Three minor cleanup items remain before merge.

[github-actions]

Claude finished @groksrc's task in 58s —— View job

---

PR Review: fix(core): honor BASIC_MEMORY_CONFIG_DIR across remaining call sites

• Gather context
• Read files with outstanding findings
• Check previous findings status
• Post final review

---

What changed in the latest commit (9d07d24)

The new commit directly addresses all three findings carried across the previous two review rounds:

Finding	Status
Stale docstring utils.py:265	✅ Fixed — now reads <basic-memory data dir>/basic-memory.log with rotation (honors BASIC_MEMORY_CONFIG_DIR)
Stale docstring ignore_utils.py:183	✅ Fixed — now reads <basic-memory data dir>/.bmignore (user's global ignore patterns, honors BASIC_MEMORY_CONFIG_DIR)
# pragma: no cover on now-exercised lines project_service.py:1142–1143	✅ Fixed — pragmas removed from try: and json.loads lines; except Exception: retains its annotation

---

Code Quality Checklist

Item	Status
Follows Basic Memory coding conventions	✅
Python 3.12+ type annotations and async patterns	✅
FastAPI / Typer conventions followed	✅
100-char line length maintained	✅
No commented-out code	✅
Unit tests for all 5 changed call sites	✅
Edge-case coverage (env var absent)	✅
100% test coverage claim is accurate	✅ (pragmas cleaned up)
Documentation / docstrings updated	✅ (both stale docstrings corrected)
Preserves local-first architecture	✅
No hardcoded secrets	✅
Windows path compatibility	✅
No emojis/unicode in console/log output	✅

---

Summary

All previous findings resolved. The PR is clean:

• Five hardcoded Path.home() / ".basic-memory" call sites now correctly route through resolve_data_dir() or the threaded config object
• Five regression tests (all named *_honors_basic_memory_config_dir) guard each fix
• Deferred import in setup_logging correctly handles the config.py → utils.py circular-import constraint
• Two docstrings updated to reflect the new behavior
• # pragma: no cover cleaned up to match actual coverage

LGTM — ready to merge.