From 6517e9845f953a9f4f21745cf839b0bd9366b2b4 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Thu, 13 Nov 2025 09:12:26 -0600 Subject: [PATCH] fix: Use platform-native path separators in config.json (#429) Signed-off-by: Drew Cain Co-authored-by: Claude --- src/basic_memory/config.py | 8 +- tests/api/test_project_router.py | 16 +-- tests/services/test_project_service.py | 48 +++---- .../test_project_service_operations.py | 20 +-- tests/test_config.py | 132 +++++++++++++++--- 5 files changed, 160 insertions(+), 64 deletions(-) diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index 764fe926..357b86f6 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -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) diff --git a/tests/api/test_project_router.py b/tests/api/test_project_router.py index d7c33e59..c9c9044f 100644 --- a/tests/api/test_project_router.py +++ b/tests/api/test_project_router.py @@ -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 diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index 4b1cc8f3..cfd3045c 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -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 diff --git a/tests/services/test_project_service_operations.py b/tests/services/test_project_service_operations.py index a0786694..ff36b541 100644 --- a/tests/services/test_project_service_operations.py +++ b/tests/services/test_project_service_operations.py @@ -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 diff --git a/tests/test_config.py b/tests/test_config.py index 818611b2..430360bf 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -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