mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: Use platform-native path separators in config.json (#429)
Signed-off-by: Drew Cain <groksrc@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -63,7 +63,7 @@ class BasicMemoryConfig(BaseSettings):
|
||||
|
||||
projects: Dict[str, str] = Field(
|
||||
default_factory=lambda: {
|
||||
"main": Path(os.getenv("BASIC_MEMORY_HOME", Path.home() / "basic-memory")).as_posix()
|
||||
"main": str(Path(os.getenv("BASIC_MEMORY_HOME", Path.home() / "basic-memory")))
|
||||
}
|
||||
if os.getenv("BASIC_MEMORY_HOME")
|
||||
else {},
|
||||
@@ -196,9 +196,9 @@ class BasicMemoryConfig(BaseSettings):
|
||||
"""Ensure configuration is valid after initialization."""
|
||||
# Ensure at least one project exists; if none exist then create main
|
||||
if not self.projects: # pragma: no cover
|
||||
self.projects["main"] = (
|
||||
self.projects["main"] = str(
|
||||
Path(os.getenv("BASIC_MEMORY_HOME", Path.home() / "basic-memory"))
|
||||
).as_posix()
|
||||
)
|
||||
|
||||
# Ensure default project is valid (i.e. points to an existing project)
|
||||
if self.default_project not in self.projects: # pragma: no cover
|
||||
@@ -361,7 +361,7 @@ class ConfigManager:
|
||||
|
||||
# Load config, modify it, and save it
|
||||
config = self.load_config()
|
||||
config.projects[name] = project_path.as_posix()
|
||||
config.projects[name] = str(project_path)
|
||||
self.save_config(config)
|
||||
return ProjectConfig(name=name, home=project_path)
|
||||
|
||||
|
||||
@@ -219,20 +219,20 @@ async def test_update_project_path_endpoint(test_config, client, project_service
|
||||
test_project_name = "test-update-project"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
old_path = (test_root / "old-location").as_posix()
|
||||
new_path = (test_root / "new-location").as_posix()
|
||||
old_path = test_root / "old-location"
|
||||
new_path = test_root / "new-location"
|
||||
|
||||
await project_service.add_project(test_project_name, old_path)
|
||||
await project_service.add_project(test_project_name, str(old_path))
|
||||
|
||||
try:
|
||||
# Verify initial state
|
||||
project = await project_service.get_project(test_project_name)
|
||||
assert project is not None
|
||||
assert project.path == old_path
|
||||
assert Path(project.path) == old_path
|
||||
|
||||
# Update the project path
|
||||
response = await client.patch(
|
||||
f"{project_url}/project/{test_project_name}", json={"path": new_path}
|
||||
f"{project_url}/project/{test_project_name}", json={"path": str(new_path)}
|
||||
)
|
||||
|
||||
# Verify response
|
||||
@@ -248,16 +248,16 @@ async def test_update_project_path_endpoint(test_config, client, project_service
|
||||
|
||||
# Check old project data
|
||||
assert data["old_project"]["name"] == test_project_name
|
||||
assert data["old_project"]["path"] == old_path
|
||||
assert Path(data["old_project"]["path"]) == old_path
|
||||
|
||||
# Check new project data
|
||||
assert data["new_project"]["name"] == test_project_name
|
||||
assert data["new_project"]["path"] == new_path
|
||||
assert Path(data["new_project"]["path"]) == new_path
|
||||
|
||||
# Verify project was actually updated in database
|
||||
updated_project = await project_service.get_project(test_project_name)
|
||||
assert updated_project is not None
|
||||
assert updated_project.path == new_path
|
||||
assert Path(updated_project.path) == new_path
|
||||
|
||||
finally:
|
||||
# Clean up
|
||||
|
||||
@@ -77,18 +77,18 @@ async def test_project_operations_sync_methods(
|
||||
test_project_name = f"test-project-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
test_project_path = (test_root / "test-project").as_posix()
|
||||
test_project_path = test_root / "test-project"
|
||||
|
||||
# Make sure the test directory exists
|
||||
os.makedirs(test_project_path, exist_ok=True)
|
||||
test_project_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
try:
|
||||
# Test adding a project (using ConfigManager directly)
|
||||
config_manager.add_project(test_project_name, test_project_path)
|
||||
config_manager.add_project(test_project_name, str(test_project_path))
|
||||
|
||||
# Verify it was added
|
||||
assert test_project_name in project_service.projects
|
||||
assert project_service.projects[test_project_name] == test_project_path
|
||||
assert Path(project_service.projects[test_project_name]) == test_project_path
|
||||
|
||||
# Test setting as default
|
||||
original_default = project_service.default_project
|
||||
@@ -173,24 +173,24 @@ async def test_add_project_async(project_service: ProjectService):
|
||||
test_project_name = f"test-async-project-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
test_project_path = (test_root / "test-async-project").as_posix()
|
||||
test_project_path = test_root / "test-async-project"
|
||||
|
||||
# Make sure the test directory exists
|
||||
os.makedirs(test_project_path, exist_ok=True)
|
||||
test_project_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
try:
|
||||
# Test adding a project
|
||||
await project_service.add_project(test_project_name, test_project_path)
|
||||
await project_service.add_project(test_project_name, str(test_project_path))
|
||||
|
||||
# Verify it was added to config
|
||||
assert test_project_name in project_service.projects
|
||||
assert project_service.projects[test_project_name] == test_project_path
|
||||
assert Path(project_service.projects[test_project_name]) == test_project_path
|
||||
|
||||
# Verify it was added to the database
|
||||
project = await project_service.repository.get_by_name(test_project_name)
|
||||
assert project is not None
|
||||
assert project.name == test_project_name
|
||||
assert project.path == test_project_path
|
||||
assert Path(project.path) == test_project_path
|
||||
|
||||
finally:
|
||||
# Clean up
|
||||
@@ -569,34 +569,34 @@ async def test_move_project(project_service: ProjectService):
|
||||
test_project_name = f"test-move-project-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
old_path = (test_root / "old-location").as_posix()
|
||||
new_path = (test_root / "new-location").as_posix()
|
||||
old_path = test_root / "old-location"
|
||||
new_path = test_root / "new-location"
|
||||
|
||||
# Create old directory
|
||||
os.makedirs(old_path, exist_ok=True)
|
||||
old_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
try:
|
||||
# Add project with initial path
|
||||
await project_service.add_project(test_project_name, old_path)
|
||||
await project_service.add_project(test_project_name, str(old_path))
|
||||
|
||||
# Verify initial state
|
||||
assert test_project_name in project_service.projects
|
||||
assert project_service.projects[test_project_name] == old_path
|
||||
assert Path(project_service.projects[test_project_name]) == old_path
|
||||
|
||||
project = await project_service.repository.get_by_name(test_project_name)
|
||||
assert project is not None
|
||||
assert project.path == old_path
|
||||
assert Path(project.path) == old_path
|
||||
|
||||
# Move project to new location
|
||||
await project_service.move_project(test_project_name, new_path)
|
||||
await project_service.move_project(test_project_name, str(new_path))
|
||||
|
||||
# Verify config was updated
|
||||
assert project_service.projects[test_project_name] == new_path
|
||||
assert Path(project_service.projects[test_project_name]) == new_path
|
||||
|
||||
# Verify database was updated
|
||||
updated_project = await project_service.repository.get_by_name(test_project_name)
|
||||
assert updated_project is not None
|
||||
assert updated_project.path == new_path
|
||||
assert Path(updated_project.path) == new_path
|
||||
|
||||
# Verify new directory was created
|
||||
assert os.path.exists(new_path)
|
||||
@@ -624,17 +624,17 @@ async def test_move_project_db_mismatch(project_service: ProjectService):
|
||||
test_project_name = f"test-move-mismatch-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
old_path = (test_root / "old-location").as_posix()
|
||||
new_path = (test_root / "new-location").as_posix()
|
||||
old_path = test_root / "old-location"
|
||||
new_path = test_root / "new-location"
|
||||
|
||||
# Create directories
|
||||
os.makedirs(old_path, exist_ok=True)
|
||||
old_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
config_manager = project_service.config_manager
|
||||
|
||||
try:
|
||||
# Add project to config only (not to database)
|
||||
config_manager.add_project(test_project_name, old_path)
|
||||
config_manager.add_project(test_project_name, str(old_path))
|
||||
|
||||
# Verify it's in config but not in database
|
||||
assert test_project_name in project_service.projects
|
||||
@@ -643,10 +643,10 @@ async def test_move_project_db_mismatch(project_service: ProjectService):
|
||||
|
||||
# Try to move project - should fail and restore config
|
||||
with pytest.raises(ValueError, match="not found in database"):
|
||||
await project_service.move_project(test_project_name, new_path)
|
||||
await project_service.move_project(test_project_name, str(new_path))
|
||||
|
||||
# Verify config was restored to original path
|
||||
assert project_service.projects[test_project_name] == old_path
|
||||
assert Path(project_service.projects[test_project_name]) == old_path
|
||||
|
||||
finally:
|
||||
# Clean up
|
||||
|
||||
@@ -53,18 +53,18 @@ async def test_add_project_to_config(project_service: ProjectService, config_man
|
||||
test_project_name = f"config-project-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
test_path = (test_root / "config-project").as_posix()
|
||||
test_path = test_root / "config-project"
|
||||
|
||||
# Make sure directory exists
|
||||
os.makedirs(test_path, exist_ok=True)
|
||||
test_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
try:
|
||||
# Add a project to config only (using ConfigManager directly)
|
||||
config_manager.add_project(test_project_name, test_path)
|
||||
config_manager.add_project(test_project_name, str(test_path))
|
||||
|
||||
# Verify it's in the config
|
||||
assert test_project_name in project_service.projects
|
||||
assert project_service.projects[test_project_name] == test_path
|
||||
assert Path(project_service.projects[test_project_name]) == test_path
|
||||
|
||||
finally:
|
||||
# Clean up
|
||||
@@ -79,23 +79,23 @@ async def test_update_project_path(project_service: ProjectService, config_manag
|
||||
test_project = f"path-update-test-project-{os.urandom(4).hex()}"
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
test_root = Path(temp_dir)
|
||||
original_path = (test_root / "original-path").as_posix()
|
||||
new_path = (test_root / "new-path").as_posix()
|
||||
original_path = test_root / "original-path"
|
||||
new_path = test_root / "new-path"
|
||||
|
||||
# Make sure directories exist
|
||||
os.makedirs(original_path, exist_ok=True)
|
||||
os.makedirs(new_path, exist_ok=True)
|
||||
original_path.mkdir(parents=True, exist_ok=True)
|
||||
new_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
try:
|
||||
# Add the project
|
||||
await project_service.add_project(test_project, original_path)
|
||||
await project_service.add_project(test_project, str(original_path))
|
||||
|
||||
# Mock the update_project method to avoid issues with complex DB updates
|
||||
with patch.object(project_service, "update_project"):
|
||||
# Just check if the project exists
|
||||
project = await project_service.repository.get_by_name(test_project)
|
||||
assert project is not None
|
||||
assert project.path == original_path
|
||||
assert Path(project.path) == original_path
|
||||
|
||||
# Since we mock the update_project method, we skip verifying path updates
|
||||
|
||||
|
||||
+114
-18
@@ -19,47 +19,47 @@ class TestBasicMemoryConfig:
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
# Should use the default path (home/basic-memory)
|
||||
expected_path = (config_home / "basic-memory").as_posix()
|
||||
assert config.projects["main"] == Path(expected_path).as_posix()
|
||||
expected_path = config_home / "basic-memory"
|
||||
assert Path(config.projects["main"]) == expected_path
|
||||
|
||||
def test_respects_basic_memory_home_environment_variable(self, config_home, monkeypatch):
|
||||
"""Test that config respects BASIC_MEMORY_HOME environment variable."""
|
||||
custom_path = (config_home / "app" / "data").as_posix()
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", custom_path)
|
||||
custom_path = config_home / "app" / "data"
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", str(custom_path))
|
||||
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
# Should use the custom path from environment variable
|
||||
assert config.projects["main"] == custom_path
|
||||
assert Path(config.projects["main"]) == custom_path
|
||||
|
||||
def test_model_post_init_respects_basic_memory_home_creates_main(
|
||||
self, config_home, monkeypatch
|
||||
):
|
||||
"""Test that model_post_init creates main project with BASIC_MEMORY_HOME when missing and no other projects."""
|
||||
custom_path = str(config_home / "custom" / "memory" / "path")
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", custom_path)
|
||||
custom_path = config_home / "custom" / "memory" / "path"
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", str(custom_path))
|
||||
|
||||
# Create config without main project
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
# model_post_init should have added main project with BASIC_MEMORY_HOME
|
||||
assert "main" in config.projects
|
||||
assert config.projects["main"] == Path(custom_path).as_posix()
|
||||
assert Path(config.projects["main"]) == custom_path
|
||||
|
||||
def test_model_post_init_respects_basic_memory_home_sets_non_main_default(
|
||||
self, config_home, monkeypatch
|
||||
):
|
||||
"""Test that model_post_init does not create main project with BASIC_MEMORY_HOME when another project exists."""
|
||||
custom_path = str(config_home / "custom" / "memory" / "path")
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", custom_path)
|
||||
custom_path = config_home / "custom" / "memory" / "path"
|
||||
monkeypatch.setenv("BASIC_MEMORY_HOME", str(custom_path))
|
||||
|
||||
# Create config without main project
|
||||
other_path = str(config_home / "some" / "path")
|
||||
config = BasicMemoryConfig(projects={"other": Path(other_path).as_posix()})
|
||||
other_path = config_home / "some" / "path"
|
||||
config = BasicMemoryConfig(projects={"other": str(other_path)})
|
||||
|
||||
# model_post_init should not add main project with BASIC_MEMORY_HOME
|
||||
assert "main" not in config.projects
|
||||
assert config.projects["other"] == Path(other_path).as_posix()
|
||||
assert Path(config.projects["other"]) == other_path
|
||||
|
||||
def test_model_post_init_fallback_without_basic_memory_home(self, config_home, monkeypatch):
|
||||
"""Test that model_post_init can set a non-main default when BASIC_MEMORY_HOME is not set."""
|
||||
@@ -67,12 +67,12 @@ class TestBasicMemoryConfig:
|
||||
monkeypatch.delenv("BASIC_MEMORY_HOME", raising=False)
|
||||
|
||||
# Create config without main project
|
||||
other_path = (config_home / "some" / "path").as_posix()
|
||||
config = BasicMemoryConfig(projects={"other": other_path})
|
||||
other_path = config_home / "some" / "path"
|
||||
config = BasicMemoryConfig(projects={"other": str(other_path)})
|
||||
|
||||
# model_post_init should not add main project, but "other" should now be the default
|
||||
assert "main" not in config.projects
|
||||
assert config.projects["other"] == Path(other_path).as_posix()
|
||||
assert Path(config.projects["other"]) == other_path
|
||||
|
||||
def test_basic_memory_home_with_relative_path(self, config_home, monkeypatch):
|
||||
"""Test that BASIC_MEMORY_HOME works with relative paths."""
|
||||
@@ -81,8 +81,8 @@ class TestBasicMemoryConfig:
|
||||
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
# Should use the exact value from environment variable
|
||||
assert config.projects["main"] == relative_path
|
||||
# Should normalize to platform-native path format
|
||||
assert Path(config.projects["main"]) == Path(relative_path)
|
||||
|
||||
def test_basic_memory_home_overrides_existing_main_project(self, config_home, monkeypatch):
|
||||
"""Test that BASIC_MEMORY_HOME is not used when a map is passed in the constructor."""
|
||||
@@ -382,3 +382,99 @@ class TestConfigManager:
|
||||
config = config_manager.load_config()
|
||||
assert config.cloud_projects == {}
|
||||
assert config.projects == {"main": str(temp_path / "main")}
|
||||
|
||||
|
||||
class TestPlatformNativePathSeparators:
|
||||
"""Test that config uses platform-native path separators."""
|
||||
|
||||
def test_project_paths_use_platform_native_separators_in_config(self, monkeypatch):
|
||||
"""Test that project paths use platform-native separators when created."""
|
||||
import platform
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp_path = Path(temp_dir)
|
||||
|
||||
# Set up ConfigManager with temp directory
|
||||
config_manager = ConfigManager()
|
||||
config_manager.config_dir = temp_path / "basic-memory"
|
||||
config_manager.config_file = config_manager.config_dir / "config.json"
|
||||
config_manager.config_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Create a project path
|
||||
project_path = temp_path / "my" / "project"
|
||||
project_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Add project via ConfigManager
|
||||
config = BasicMemoryConfig(projects={})
|
||||
config.projects["test-project"] = str(project_path)
|
||||
config_manager.save_config(config)
|
||||
|
||||
# Read the raw JSON file
|
||||
import json
|
||||
|
||||
config_data = json.loads(config_manager.config_file.read_text())
|
||||
|
||||
# Verify path uses platform-native separators
|
||||
saved_path = config_data["projects"]["test-project"]
|
||||
|
||||
# On Windows, should have backslashes; on Unix, forward slashes
|
||||
if platform.system() == "Windows":
|
||||
# Windows paths should contain backslashes
|
||||
assert "\\" in saved_path or ":" in saved_path # C:\\ or \\UNC
|
||||
assert "/" not in saved_path.replace(":/", "") # Exclude drive letter
|
||||
else:
|
||||
# Unix paths should use forward slashes
|
||||
assert "/" in saved_path
|
||||
# Should not force POSIX on non-Windows
|
||||
assert saved_path == str(project_path)
|
||||
|
||||
def test_add_project_uses_platform_native_separators(self, monkeypatch):
|
||||
"""Test that ConfigManager.add_project() uses platform-native separators."""
|
||||
import platform
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
temp_path = Path(temp_dir)
|
||||
|
||||
# Set up ConfigManager
|
||||
config_manager = ConfigManager()
|
||||
config_manager.config_dir = temp_path / "basic-memory"
|
||||
config_manager.config_file = config_manager.config_dir / "config.json"
|
||||
config_manager.config_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Initialize with empty projects
|
||||
initial_config = BasicMemoryConfig(projects={})
|
||||
config_manager.save_config(initial_config)
|
||||
|
||||
# Add project
|
||||
project_path = temp_path / "new" / "project"
|
||||
config_manager.add_project("new-project", str(project_path))
|
||||
|
||||
# Load and verify
|
||||
config = config_manager.load_config()
|
||||
saved_path = config.projects["new-project"]
|
||||
|
||||
# Verify platform-native separators
|
||||
if platform.system() == "Windows":
|
||||
assert "\\" in saved_path or ":" in saved_path
|
||||
else:
|
||||
assert "/" in saved_path
|
||||
assert saved_path == str(project_path)
|
||||
|
||||
def test_model_post_init_uses_platform_native_separators(self, config_home, monkeypatch):
|
||||
"""Test that model_post_init uses platform-native separators."""
|
||||
import platform
|
||||
|
||||
monkeypatch.delenv("BASIC_MEMORY_HOME", raising=False)
|
||||
|
||||
# Create config without projects (triggers model_post_init to add main)
|
||||
config = BasicMemoryConfig(projects={})
|
||||
|
||||
# Verify main project path uses platform-native separators
|
||||
main_path = config.projects["main"]
|
||||
|
||||
if platform.system() == "Windows":
|
||||
# Windows: should have backslashes or drive letter
|
||||
assert "\\" in main_path or ":" in main_path
|
||||
else:
|
||||
# Unix: should have forward slashes
|
||||
assert "/" in main_path
|
||||
|
||||
Reference in New Issue
Block a user