Skip to content

Commit 0848009

Browse files
phernandezclaude
andcommitted
fix: Improve project management validation and idempotency
Refines the idempotent project creation behavior and adds validation for project deletion to prevent user errors. Changes: - Project creation now properly validates path matching for true idempotency - Same name + same path: Returns 200 OK (idempotent) - Same name + different path: Returns 400 with clear error message - Project deletion now prevents removing the default project - Returns 400 with helpful error listing alternative projects - Prevents accidental deletion of the only/default project This fixes issues encountered by tenant user_01K7GCGSPZ4A63H3QA5M9AMT23 who experienced 32 errors (15 delete failures, 12 create failures) due to inadequate validation. Tests: - Added 6 new comprehensive tests for validation logic - All 24 tests pass with 97% coverage on project_router - No regressions detected Fixes #173 (tenant errors) Related: #175 (delete validation), #176 (set default validation) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 256c851 commit 0848009

2 files changed

Lines changed: 201 additions & 15 deletions

File tree

src/basic_memory/api/routers/project_router.py

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,35 @@ async def add_project(
193193
Returns:
194194
Response confirming the project was added
195195
"""
196+
# Check if project already exists before attempting to add
197+
existing_project = await project_service.get_project(project_data.name)
198+
if existing_project:
199+
# Project exists - check if paths match for true idempotency
200+
# Normalize paths for comparison (resolve symlinks, etc.)
201+
from pathlib import Path
202+
203+
requested_path = Path(project_data.path).resolve()
204+
existing_path = Path(existing_project.path).resolve()
205+
206+
if requested_path == existing_path:
207+
# Same name, same path - return 200 OK (idempotent)
208+
return ProjectStatusResponse( # pyright: ignore [reportCallIssue]
209+
message=f"Project '{project_data.name}' already exists",
210+
status="success",
211+
default=existing_project.is_default or False,
212+
new_project=ProjectItem(
213+
name=existing_project.name,
214+
path=existing_project.path,
215+
is_default=existing_project.is_default or False,
216+
),
217+
)
218+
else:
219+
# Same name, different path - this is an error
220+
raise HTTPException(
221+
status_code=400,
222+
detail=f"Project '{project_data.name}' already exists with different path. Existing: {existing_project.path}, Requested: {project_data.path}",
223+
)
224+
196225
try: # pragma: no cover
197226
# The service layer now handles cloud mode validation and path sanitization
198227
await project_service.add_project(
@@ -208,21 +237,6 @@ async def add_project(
208237
),
209238
)
210239
except ValueError as e: # pragma: no cover
211-
# If project already exists, return 200 OK with existing project info
212-
error_msg = str(e)
213-
if "already exists" in error_msg.lower():
214-
existing_project = await project_service.get_project(project_data.name)
215-
if existing_project:
216-
return ProjectStatusResponse( # pyright: ignore [reportCallIssue]
217-
message=f"Project '{project_data.name}' already exists",
218-
status="success",
219-
default=existing_project.is_default or False,
220-
new_project=ProjectItem(
221-
name=existing_project.name,
222-
path=existing_project.path,
223-
is_default=existing_project.is_default or False,
224-
),
225-
)
226240
raise HTTPException(status_code=400, detail=str(e))
227241

228242

@@ -247,6 +261,19 @@ async def remove_project(
247261
status_code=404, detail=f"Project: '{name}' does not exist"
248262
) # pragma: no cover
249263

264+
# Check if trying to delete the default project
265+
if name == project_service.default_project:
266+
available_projects = await project_service.list_projects()
267+
other_projects = [p.name for p in available_projects if p.name != name]
268+
detail = f"Cannot delete default project '{name}'. "
269+
if other_projects:
270+
detail += (
271+
f"Set another project as default first. Available: {', '.join(other_projects)}"
272+
)
273+
else:
274+
detail += "This is the only project in your configuration."
275+
raise HTTPException(status_code=400, detail=detail)
276+
250277
await project_service.remove_project(name)
251278

252279
return ProjectStatusResponse(

tests/api/test_project_router.py

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,3 +474,162 @@ async def test_sync_project_endpoint_not_found(client):
474474

475475
# Should return 404
476476
assert response.status_code == 404
477+
478+
479+
@pytest.mark.asyncio
480+
async def test_remove_default_project_fails(test_config, client, project_service):
481+
"""Test that removing the default project returns an error."""
482+
# Get the current default project
483+
default_project_name = project_service.default_project
484+
485+
# Try to remove the default project
486+
response = await client.delete(f"/projects/{default_project_name}")
487+
488+
# Should return 400 with helpful error message
489+
assert response.status_code == 400
490+
data = response.json()
491+
assert "detail" in data
492+
assert "Cannot delete default project" in data["detail"]
493+
assert default_project_name in data["detail"]
494+
495+
496+
@pytest.mark.asyncio
497+
async def test_remove_default_project_with_alternatives(test_config, client, project_service):
498+
"""Test that error message includes alternative projects when trying to delete default."""
499+
# Get the current default project
500+
default_project_name = project_service.default_project
501+
502+
# Create another project so there are alternatives
503+
test_project_name = "test-alternative-project"
504+
await project_service.add_project(test_project_name, "/tmp/test-alternative")
505+
506+
try:
507+
# Try to remove the default project
508+
response = await client.delete(f"/projects/{default_project_name}")
509+
510+
# Should return 400 with helpful error message including alternatives
511+
assert response.status_code == 400
512+
data = response.json()
513+
assert "detail" in data
514+
assert "Cannot delete default project" in data["detail"]
515+
assert "Set another project as default first" in data["detail"]
516+
assert test_project_name in data["detail"]
517+
518+
finally:
519+
# Clean up
520+
try:
521+
await project_service.remove_project(test_project_name)
522+
except Exception:
523+
pass
524+
525+
526+
@pytest.mark.asyncio
527+
async def test_remove_non_default_project_succeeds(test_config, client, project_service):
528+
"""Test that removing a non-default project succeeds."""
529+
# Create a test project to remove
530+
test_project_name = "test-remove-non-default"
531+
await project_service.add_project(test_project_name, "/tmp/test-remove-non-default")
532+
533+
# Verify it's not the default
534+
assert project_service.default_project != test_project_name
535+
536+
# Remove the project
537+
response = await client.delete(f"/projects/{test_project_name}")
538+
539+
# Should succeed
540+
assert response.status_code == 200
541+
data = response.json()
542+
assert data["status"] == "success"
543+
544+
# Verify project is removed
545+
removed_project = await project_service.get_project(test_project_name)
546+
assert removed_project is None
547+
548+
549+
@pytest.mark.asyncio
550+
async def test_set_nonexistent_project_as_default_fails(test_config, client, project_service):
551+
"""Test that setting a non-existent project as default returns 404."""
552+
# Try to set a project that doesn't exist as default
553+
response = await client.put("/projects/nonexistent-project/default")
554+
555+
# Should return 404
556+
assert response.status_code == 404
557+
data = response.json()
558+
assert "detail" in data
559+
assert "does not exist" in data["detail"]
560+
561+
562+
@pytest.mark.asyncio
563+
async def test_create_project_idempotent_same_path(test_config, client, project_service):
564+
"""Test that creating a project with same name and same path is idempotent."""
565+
# Create a project
566+
test_project_name = "test-idempotent"
567+
test_project_path = "/tmp/test-idempotent"
568+
569+
response1 = await client.post(
570+
"/projects/projects",
571+
json={"name": test_project_name, "path": test_project_path, "set_default": False},
572+
)
573+
574+
# Should succeed
575+
assert response1.status_code == 200
576+
data1 = response1.json()
577+
assert data1["status"] == "success"
578+
assert data1["new_project"]["name"] == test_project_name
579+
580+
# Try to create the same project again with same name and path
581+
response2 = await client.post(
582+
"/projects/projects",
583+
json={"name": test_project_name, "path": test_project_path, "set_default": False},
584+
)
585+
586+
# Should also succeed (idempotent)
587+
assert response2.status_code == 200
588+
data2 = response2.json()
589+
assert data2["status"] == "success"
590+
assert "already exists" in data2["message"]
591+
assert data2["new_project"]["name"] == test_project_name
592+
assert data2["new_project"]["path"] == test_project_path
593+
594+
# Clean up
595+
try:
596+
await project_service.remove_project(test_project_name)
597+
except Exception:
598+
pass
599+
600+
601+
@pytest.mark.asyncio
602+
async def test_create_project_fails_different_path(test_config, client, project_service):
603+
"""Test that creating a project with same name but different path fails."""
604+
# Create a project
605+
test_project_name = "test-path-conflict"
606+
test_project_path1 = "/tmp/test-path-conflict-1"
607+
608+
response1 = await client.post(
609+
"/projects/projects",
610+
json={"name": test_project_name, "path": test_project_path1, "set_default": False},
611+
)
612+
613+
# Should succeed
614+
assert response1.status_code == 200
615+
616+
# Try to create the same project with different path
617+
test_project_path2 = "/tmp/test-path-conflict-2"
618+
response2 = await client.post(
619+
"/projects/projects",
620+
json={"name": test_project_name, "path": test_project_path2, "set_default": False},
621+
)
622+
623+
# Should fail with 400
624+
assert response2.status_code == 400
625+
data2 = response2.json()
626+
assert "detail" in data2
627+
assert "already exists with different path" in data2["detail"]
628+
assert test_project_path1 in data2["detail"]
629+
assert test_project_path2 in data2["detail"]
630+
631+
# Clean up
632+
try:
633+
await project_service.remove_project(test_project_name)
634+
except Exception:
635+
pass

0 commit comments

Comments
 (0)