fix: basic memory home env var not respected when project path is changed. (#239)

Signed-off-by: Joe P <joe@basicmemory.com>
This commit is contained in:
jope-bm
2025-07-28 14:50:47 -06:00
committed by GitHub
parent 24a1d6195d
commit 6361574a20
10 changed files with 551 additions and 30 deletions
+21 -13
View File
@@ -1,5 +1,6 @@
"""Router for project management."""
import os
from fastapi import APIRouter, HTTPException, Path, Body
from typing import Optional
@@ -32,40 +33,47 @@ async def get_project_info(
@project_router.patch("/{name}", response_model=ProjectStatusResponse)
async def update_project(
project_service: ProjectServiceDep,
project_name: str = Path(..., description="Name of the project to update"),
path: Optional[str] = Body(None, description="New path for the project"),
name: str = Path(..., description="Name of the project to update"),
path: Optional[str] = Body(None, description="New absolute path for the project"),
is_active: Optional[bool] = Body(None, description="Status of the project (active/inactive)"),
) -> ProjectStatusResponse:
"""Update a project's information in configuration and database.
Args:
project_name: The name of the project to update
path: Optional new path for the project
name: The name of the project to update
path: Optional new absolute path for the project
is_active: Optional status update for the project
Returns:
Response confirming the project was updated
"""
try: # pragma: no cover
try:
# Validate that path is absolute if provided
if path and not os.path.isabs(path):
raise HTTPException(status_code=400, detail="Path must be absolute")
# Get original project info for the response
old_project_info = ProjectItem(
name=project_name,
path=project_service.projects.get(project_name, ""),
name=name,
path=project_service.projects.get(name, ""),
)
await project_service.update_project(project_name, updated_path=path, is_active=is_active)
if path:
await project_service.move_project(name, path)
elif is_active is not None:
await project_service.update_project(name, is_active=is_active)
# Get updated project info
updated_path = path if path else project_service.projects.get(project_name, "")
updated_path = path if path else project_service.projects.get(name, "")
return ProjectStatusResponse(
message=f"Project '{project_name}' updated successfully",
message=f"Project '{name}' updated successfully",
status="success",
default=(project_name == project_service.default_project),
default=(name == project_service.default_project),
old_project=old_project_info,
new_project=ProjectItem(name=project_name, path=updated_path),
new_project=ProjectItem(name=name, path=updated_path),
)
except ValueError as e: # pragma: no cover
except ValueError as e:
raise HTTPException(status_code=400, detail=str(e))
+37
View File
@@ -23,6 +23,7 @@ from basic_memory.mcp.tools.utils import call_post
from basic_memory.schemas.project_info import ProjectStatusResponse
from basic_memory.mcp.tools.utils import call_delete
from basic_memory.mcp.tools.utils import call_put
from basic_memory.mcp.tools.utils import call_patch
from basic_memory.utils import generate_permalink
console = Console()
@@ -148,6 +149,42 @@ def synchronize_projects() -> None:
raise typer.Exit(1)
@project_app.command("move")
def move_project(
name: str = typer.Argument(..., help="Name of the project to move"),
new_path: str = typer.Argument(..., help="New absolute path for the project"),
) -> None:
"""Move a project to a new location."""
# Resolve to absolute path
resolved_path = os.path.abspath(os.path.expanduser(new_path))
try:
data = {"path": resolved_path}
project_name = generate_permalink(name)
current_project = session.get_current_project()
response = asyncio.run(call_patch(client, f"/{current_project}/project/{project_name}", json=data))
result = ProjectStatusResponse.model_validate(response.json())
console.print(f"[green]{result.message}[/green]")
# Show important file movement reminder
console.print() # Empty line for spacing
console.print(Panel(
"[bold red]IMPORTANT:[/bold red] Project configuration updated successfully.\n\n"
"[yellow]You must manually move your project files from the old location to:[/yellow]\n"
f"[cyan]{resolved_path}[/cyan]\n\n"
"[dim]Basic Memory has only updated the configuration - your files remain in their original location.[/dim]",
title="⚠️ Manual File Movement Required",
border_style="yellow",
expand=False
))
except Exception as e:
console.print(f"[red]Error moving project: {str(e)}[/red]")
raise typer.Exit(1)
@project_app.command("info")
def display_project_info(
json_output: bool = typer.Option(False, "--json", help="Output in JSON format"),
+2 -4
View File
@@ -182,10 +182,8 @@ class ConfigManager:
data = json.loads(self.config_file.read_text(encoding="utf-8"))
return BasicMemoryConfig(**data)
except Exception as e: # pragma: no cover
logger.error(f"Failed to load config: {e}")
config = BasicMemoryConfig()
self.save_config(config)
return config
logger.exception(f"Failed to load config: {e}")
raise e
else:
config = BasicMemoryConfig()
self.save_config(config)
@@ -83,3 +83,21 @@ class ProjectRepository(Repository[Project]):
await session.flush()
return target_project
return None # pragma: no cover
async def update_path(self, project_id: int, new_path: str) -> Optional[Project]:
"""Update project path.
Args:
project_id: ID of the project to update
new_path: New filesystem path for the project
Returns:
The updated project if found, None otherwise
"""
async with db.scoped_session(self.session_maker) as session:
project = await self.select_by_id(session, project_id)
if project:
project.path = new_path
await session.flush()
return project
return None
@@ -309,6 +309,47 @@ class ProjectService:
# MCP components might not be available in all contexts
logger.debug("MCP session not available, skipping session refresh")
async def move_project(self, name: str, new_path: str) -> None:
"""Move a project to a new location.
Args:
name: The name of the project to move
new_path: The new absolute path for the project
Raises:
ValueError: If the project doesn't exist or repository isn't initialized
"""
if not self.repository:
raise ValueError("Repository is required for move_project")
# Resolve to absolute path
resolved_path = os.path.abspath(os.path.expanduser(new_path))
# Validate project exists in config
if name not in self.config_manager.projects:
raise ValueError(f"Project '{name}' not found in configuration")
# Create the new directory if it doesn't exist
Path(resolved_path).mkdir(parents=True, exist_ok=True)
# Update in configuration
config = self.config_manager.load_config()
old_path = config.projects[name]
config.projects[name] = resolved_path
self.config_manager.save_config(config)
# Update in database
project = await self.repository.get_by_name(name)
if project:
await self.repository.update_path(project.id, resolved_path)
logger.info(f"Moved project '{name}' from {old_path} to {resolved_path}")
else:
logger.error(f"Project '{name}' exists in config but not in database")
# Restore the old path in config since DB update failed
config.projects[name] = old_path
self.config_manager.save_config(config)
raise ValueError(f"Project '{name}' not found in database")
async def update_project( # pragma: no cover
self, name: str, updated_path: Optional[str] = None, is_active: Optional[bool] = None
) -> None:
+13 -13
View File
@@ -146,19 +146,19 @@ def setup_logging(
# logger.remove()
# Add file handler if we are not running tests and a log file is specified
# if log_file and env != "test":
# # Setup file logger
# log_path = home_dir / log_file
# logger.add(
# str(log_path),
# level=log_level,
# rotation="10 MB",
# retention="10 days",
# backtrace=True,
# diagnose=True,
# enqueue=True,
# colorize=False,
# )
if log_file and env != "test":
# Setup file logger
log_path = home_dir / log_file
logger.add(
str(log_path),
level=log_level,
rotation="10 MB",
retention="10 days",
backtrace=True,
diagnose=True,
enqueue=True,
colorize=False,
)
# Add console logger if requested or in test mode
# if env == "test" or console:
+233
View File
@@ -159,3 +159,236 @@ async def test_set_default_project_endpoint(test_config, client, project_service
# Verify it's actually set as default
assert project_service.default_project == test_project_name
@pytest.mark.asyncio
async def test_update_project_path_endpoint(test_config, client, project_service, project_url, tmp_path):
"""Test the update project endpoint for changing project path."""
# Create a test project to update
test_project_name = "test-update-project"
old_path = str(tmp_path / "old-location")
new_path = str(tmp_path / "new-location")
await project_service.add_project(test_project_name, 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
# Update the project path
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={"path": new_path}
)
# Verify response
assert response.status_code == 200
data = response.json()
# Check response structure
assert "message" in data
assert "status" in data
assert data["status"] == "success"
assert "old_project" in data
assert "new_project" in data
# Check old project data
assert data["old_project"]["name"] == test_project_name
assert 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
# 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
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
@pytest.mark.asyncio
async def test_update_project_is_active_endpoint(test_config, client, project_service, project_url):
"""Test the update project endpoint for changing is_active status."""
# Create a test project to update
test_project_name = "test-update-active-project"
test_path = "/tmp/test-update-active"
await project_service.add_project(test_project_name, test_path)
try:
# Update the project is_active status
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={"is_active": False}
)
# Verify response
assert response.status_code == 200
data = response.json()
# Check response structure
assert "message" in data
assert "status" in data
assert data["status"] == "success"
assert f"Project '{test_project_name}' updated successfully" == data["message"]
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
@pytest.mark.asyncio
async def test_update_project_both_params_endpoint(test_config, client, project_service, project_url, tmp_path):
"""Test the update project endpoint with both path and is_active parameters."""
# Create a test project to update
test_project_name = "test-update-both-project"
old_path = str(tmp_path / "old-location")
new_path = str(tmp_path / "new-location")
await project_service.add_project(test_project_name, old_path)
try:
# Update both path and is_active (path should take precedence)
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={"path": new_path, "is_active": False}
)
# Verify response
assert response.status_code == 200
data = response.json()
# Check that path update was performed (takes precedence)
assert 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
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
@pytest.mark.asyncio
async def test_update_project_nonexistent_endpoint(client, project_url):
"""Test the update project endpoint with a nonexistent project."""
# Try to update a project that doesn't exist
response = await client.patch(
f"{project_url}/project/nonexistent-project",
json={"path": "/tmp/new-path"}
)
# Should return 400 error
assert response.status_code == 400
data = response.json()
assert "detail" in data
assert "not found in configuration" in data["detail"]
@pytest.mark.asyncio
async def test_update_project_relative_path_error_endpoint(test_config, client, project_service, project_url):
"""Test the update project endpoint with relative path (should fail)."""
# Create a test project to update
test_project_name = "test-update-relative-project"
test_path = "/tmp/test-update-relative"
await project_service.add_project(test_project_name, test_path)
try:
# Try to update with relative path
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={"path": "./relative-path"}
)
# Should return 400 error
assert response.status_code == 400
data = response.json()
assert "detail" in data
assert "Path must be absolute" in data["detail"]
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
@pytest.mark.asyncio
async def test_update_project_no_params_endpoint(test_config, client, project_service, project_url):
"""Test the update project endpoint with no parameters (should fail)."""
# Create a test project to update
test_project_name = "test-update-no-params-project"
test_path = "/tmp/test-update-no-params"
await project_service.add_project(test_project_name, test_path)
proj_info = await project_service.get_project(test_project_name)
assert proj_info.name == test_project_name
assert proj_info.path == test_path
try:
# Try to update with no parameters
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={}
)
# Should return 200 (no-op)
assert response.status_code == 200
proj_info = await project_service.get_project(test_project_name)
assert proj_info.name == test_project_name
assert proj_info.path == test_path
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
@pytest.mark.asyncio
async def test_update_project_empty_path_endpoint(test_config, client, project_service, project_url):
"""Test the update project endpoint with empty path parameter."""
# Create a test project to update
test_project_name = "test-update-empty-path-project"
test_path = "/tmp/test-update-empty-path"
await project_service.add_project(test_project_name, test_path)
try:
# Try to update with empty/null path - should be treated as no path update
response = await client.patch(
f"{project_url}/project/{test_project_name}",
json={"path": None, "is_active": True}
)
# Should succeed and perform is_active update
assert response.status_code == 200
data = response.json()
assert data["status"] == "success"
finally:
# Clean up
try:
await project_service.remove_project(test_project_name)
except Exception:
pass
+38
View File
@@ -141,3 +141,41 @@ def test_project_failure_exits_with_error(mock_run, cli_env):
assert default_result.exit_code == 1
assert "Error setting default project" in default_result.output
@patch("basic_memory.cli.commands.project.asyncio.run")
def test_project_move_command(mock_run, cli_env):
"""Test the 'project move' command with mocked API."""
# Mock the API response
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.json.return_value = {
"message": "Project 'test-project' updated successfully",
"status": "success",
"default": False,
}
mock_run.return_value = mock_response
runner = CliRunner()
result = runner.invoke(cli_app, ["project", "move", "test-project", "/new/path/to/project"])
# Verify it runs without exception
assert result.exit_code == 0
# Verify the important warning message is displayed
assert "Manual File Movement Required" in result.output
assert "You must manually move your project files" in result.output
assert "/new/path/to/project" in result.output
@patch("basic_memory.cli.commands.project.asyncio.run")
def test_project_move_command_failure(mock_run, cli_env):
"""Test the 'project move' command with API failure."""
# Mock an exception being raised
mock_run.side_effect = Exception("Project not found")
runner = CliRunner()
result = runner.invoke(cli_app, ["project", "move", "nonexistent-project", "/new/path"])
# Should exit with code 1 and show error message
assert result.exit_code == 1
assert "Error moving project" in result.output
@@ -267,3 +267,31 @@ async def test_delete_nonexistent_project(project_repository: ProjectRepository)
"""Test deleting a project that doesn't exist."""
result = await project_repository.delete(999) # Non-existent ID
assert result is False
@pytest.mark.asyncio
async def test_update_path(project_repository: ProjectRepository, sample_project: Project):
"""Test updating a project's path."""
new_path = "/new/project/path"
# Update the project path
updated_project = await project_repository.update_path(sample_project.id, new_path)
# Verify returned object
assert updated_project is not None
assert updated_project.id == sample_project.id
assert updated_project.path == new_path
assert updated_project.name == sample_project.name # Other fields unchanged
# Verify in database
found = await project_repository.find_by_id(sample_project.id)
assert found is not None
assert found.path == new_path
assert found.name == sample_project.name
@pytest.mark.asyncio
async def test_update_path_nonexistent_project(project_repository: ProjectRepository):
"""Test updating path for a project that doesn't exist."""
result = await project_repository.update_path(999, "/some/path") # Non-existent ID
assert result is None
+120
View File
@@ -535,6 +535,124 @@ async def test_synchronize_projects_normalizes_project_names(
await project_service.repository.delete(db_project.id)
@pytest.mark.asyncio
async def test_move_project(project_service: ProjectService, tmp_path):
"""Test moving a project to a new location."""
test_project_name = f"test-move-project-{os.urandom(4).hex()}"
old_path = str(tmp_path / "old-location")
new_path = str(tmp_path / "new-location")
# Create old directory
os.makedirs(old_path, exist_ok=True)
try:
# Add project with initial path
await project_service.add_project(test_project_name, old_path)
# Verify initial state
assert test_project_name in project_service.projects
assert 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
# Move project to new location
await project_service.move_project(test_project_name, new_path)
# Verify config was updated
assert 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
# Verify new directory was created
assert os.path.exists(new_path)
finally:
# Clean up
if test_project_name in project_service.projects:
await project_service.remove_project(test_project_name)
@pytest.mark.asyncio
async def test_move_project_nonexistent(project_service: ProjectService, tmp_path):
"""Test moving a project that doesn't exist."""
new_path = str(tmp_path / "new-location")
with pytest.raises(ValueError, match="not found in configuration"):
await project_service.move_project("nonexistent-project", new_path)
@pytest.mark.asyncio
async def test_move_project_db_mismatch(project_service: ProjectService, tmp_path):
"""Test moving a project that exists in config but not in database."""
test_project_name = f"test-move-mismatch-{os.urandom(4).hex()}"
old_path = str(tmp_path / "old-location")
new_path = str(tmp_path / "new-location")
# Create directories
os.makedirs(old_path, 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)
# Verify it's in config but not in database
assert test_project_name in project_service.projects
db_project = await project_service.repository.get_by_name(test_project_name)
assert db_project is None
# 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)
# Verify config was restored to original path
assert project_service.projects[test_project_name] == old_path
finally:
# Clean up
if test_project_name in project_service.projects:
config_manager.remove_project(test_project_name)
@pytest.mark.asyncio
async def test_move_project_expands_path(project_service: ProjectService, tmp_path):
"""Test that move_project expands ~ and relative paths."""
test_project_name = f"test-move-expand-{os.urandom(4).hex()}"
old_path = str(tmp_path / "old-location")
# Create old directory
os.makedirs(old_path, exist_ok=True)
try:
# Add project with initial path
await project_service.add_project(test_project_name, old_path)
# Use a relative path for the move
relative_new_path = "./new-location"
expected_absolute_path = os.path.abspath(relative_new_path)
# Move project using relative path
await project_service.move_project(test_project_name, relative_new_path)
# Verify the path was expanded to absolute
assert project_service.projects[test_project_name] == expected_absolute_path
updated_project = await project_service.repository.get_by_name(test_project_name)
assert updated_project is not None
assert updated_project.path == expected_absolute_path
finally:
# Clean up
if test_project_name in project_service.projects:
await project_service.remove_project(test_project_name)
@pytest.mark.asyncio
async def test_synchronize_projects_handles_case_sensitivity_bug(
project_service: ProjectService, tmp_path
@@ -595,3 +713,5 @@ async def test_synchronize_projects_handles_case_sensitivity_bug(
db_project = await project_service.repository.get_by_name(name)
if db_project:
await project_service.repository.delete(db_project.id)