Skip to content

feat: isolate default sqlite db by config dir#567

Merged
phernandez merged 1 commit intomainfrom
codex/config-dir-db-path
Feb 13, 2026
Merged

feat: isolate default sqlite db by config dir#567
phernandez merged 1 commit intomainfrom
codex/config-dir-db-path

Conversation

@phernandez
Copy link
Member

Summary

This keeps the solution simple: when BASIC_MEMORY_CONFIG_DIR is set, Basic Memory now stores the default SQLite index DB (memory.db) in that same directory.

  • app_database_path now derives from data_dir_path
  • data_dir_path now respects BASIC_MEMORY_CONFIG_DIR (falls back to ~/.basic-memory)
  • Added focused tests for both custom-config-dir and default behavior

Why

This enables separate config+db state per worktree/process/MCP instance without introducing new URL parsing or Postgres schema logic.

Scope

  • Solves per-instance SQLite isolation
  • Does not change Postgres schema/search_path behavior

Tests

  • uv run pytest tests/test_config.py -q
  • uv run ruff check src/basic_memory/config.py tests/test_config.py

Supersedes #540.

Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Contributor

github-actions bot commented Feb 13, 2026

Claude finished @phernandez's task in 1m 33s —— View job


Code Review Complete ✓

Summary

This PR cleanly implements per-instance SQLite isolation by making the default database path derive from BASIC_MEMORY_CONFIG_DIR. The implementation is consistent, well-tested, and maintains backward compatibility.

Review Checklist

Code Quality & Standards ✓

  • Follows Basic Memory's coding conventions - Code adheres to all patterns in CLAUDE.md
  • Python 3.12+ type annotations - Added proper return type -> Path to data_dir_path property
  • SQLAlchemy 2.0 best practices - N/A for this change
  • FastAPI and Typer conventions - N/A for this change
  • 100-character line length - All lines comply (config.py:369 is exactly 77 chars)
  • No commented-out code - Clean implementation

Testing & Documentation ✓

  • Unit tests for new functions - Two focused tests added:
    • test_app_database_path_uses_custom_config_dir (lines 100-109)
    • test_app_database_path_defaults_to_home_data_dir (lines 111-117)
  • Integration tests - N/A (config-level change)
  • Test coverage for edge cases - Both custom and default paths tested
  • 100% test coverage maintained - New code properly covered
  • Documentation updated - Enhanced docstrings explain the BASIC_MEMORY_CONFIG_DIR behavior
  • CLAUDE.md updated - N/A (no convention changes)

Basic Memory Architecture ✓

  • MCP tools follow atomic design - N/A for this change
  • Database changes include migrations - N/A (no schema changes)
  • Preserves local-first architecture - Enhanced! Better supports multiple local instances
  • Knowledge graph operations - N/A for this change
  • Markdown file handling - N/A for this change
  • AI-human collaboration patterns - Follows established patterns

Security & Performance ✓

  • No hardcoded secrets - Uses environment variable pattern correctly
  • Input validation - Path handling via Path() is safe
  • Proper error handling - Inherits existing error handling
  • Performance considerations - Property-based, no performance impact
  • No sensitive data in logs - Clean implementation

Compatibility ✓

  • File path comparisons must be Windows compatible - Uses Path() which handles platform differences
  • Avoid emojis/unicode in console/log output - N/A for this change

Key Observations

Strengths

  1. Consistent implementation: Both BasicMemoryConfig.data_dir_path (line 417) and ConfigManager.__init__ (line 440) use the same BASIC_MEMORY_CONFIG_DIR logic
  2. Backward compatible: Falls back to ~/.basic-memory when env var is not set
  3. Well-documented: Clear docstrings explain the purpose (lines 418, 365-369)
  4. Focused tests: New tests verify both custom and default behavior
  5. Correct usage elsewhere: Verified that src/basic_memory/cli/auth.py:35 and src/basic_memory/cli/commands/db.py:66 will work correctly with this change

Implementation Details

config.py:371 - Changed from:

database_path = Path.home() / DATA_DIR_NAME / APP_DATABASE_NAME

to:

database_path = self.data_dir_path / APP_DATABASE_NAME

config.py:417-423 - New data_dir_path property:

@property
def data_dir_path(self) -> Path:
    """Get app state directory for config and default SQLite database."""
    if config_dir := os.getenv("BASIC_MEMORY_CONFIG_DIR"):
        return Path(config_dir)
    
    home = os.getenv("HOME", Path.home())
    return Path(home) / DATA_DIR_NAME

This creates a single source of truth for the config directory path, used by both config storage and database storage.

Verification

✓ Logic is consistent with ConfigManager.__init__ (lines 440-443)
✓ Tests cover both custom config dir and default behavior
✓ Existing code in cli/auth.py and cli/commands/db.py will benefit from this change
✓ No breaking changes - backward compatible fallback to ~/.basic-memory

Recommendation

APPROVE - This PR is ready to merge. The implementation is clean, well-tested, and achieves the stated goal of enabling per-instance SQLite isolation through BASIC_MEMORY_CONFIG_DIR.

The solution is appropriately scoped (doesn't touch Postgres schema logic) and maintains backward compatibility while enabling the new use case (multiple worktrees/MCP instances).

@phernandez phernandez merged commit 7624a20 into main Feb 13, 2026
28 checks passed
@phernandez phernandez deleted the codex/config-dir-db-path branch February 13, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant