Skip to content

Commit be352ab

Browse files
authored
fix: Project deletion failing with permalink normalization (#345)
1 parent 8d2e70c commit be352ab

File tree

3 files changed

+146
-1
lines changed

3 files changed

+146
-1
lines changed

BUGFIX-project-delete.md

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# Bug Fix: Project Deletion Failure
2+
3+
## Problem Description
4+
5+
The `delete_project` MCP tool was failing with "Project 'test-verify' not found" even though the project clearly existed and showed up in `list_memory_projects`.
6+
7+
## Root Cause
8+
9+
The bug was in `/Users/drew/code/basic-memory/src/basic_memory/config.py` in the `ConfigManager.remove_project()` method (line 311):
10+
11+
```python
12+
def remove_project(self, name: str) -> None:
13+
"""Remove a project from the configuration."""
14+
15+
project_name, path = self.get_project(name)
16+
if not project_name:
17+
raise ValueError(f"Project '{name}' not found")
18+
19+
config = self.load_config()
20+
if project_name == config.default_project:
21+
raise ValueError(f"Cannot remove the default project '{name}'")
22+
23+
del config.projects[name] #BUG: Using input name instead of found project_name
24+
self.save_config(config)
25+
```
26+
27+
**The Issue:**
28+
1. Line 305: `get_project(name)` does a permalink-based lookup and returns the **actual** project name from the config (e.g., "test-verify")
29+
2. Line 311: `del config.projects[name]` tries to delete using the **input** name parameter instead of the `project_name` that was just found
30+
3. Since `get_project()` uses permalink matching, it can find a project even if the input name doesn't match the exact dictionary key
31+
32+
**Example Scenario:**
33+
- Config has project key: `"test-verify"`
34+
- User calls: `delete_project("test-verify")`
35+
- `get_project("test-verify")` finds it via permalink matching and returns `("test-verify", "/path")`
36+
- `del config.projects["test-verify"]` tries to delete using input, which should work...
37+
- BUT if there's any normalization mismatch between the stored key and the input, it fails
38+
39+
## The Fix
40+
41+
Changed line 311 to use the `project_name` returned by `get_project()`:
42+
43+
```python
44+
# Use the found project_name (which may differ from input name due to permalink matching)
45+
del config.projects[project_name]
46+
```
47+
48+
This ensures we're deleting the exact key that exists in the config dictionary, not the potentially non-normalized input name.
49+
50+
## Testing
51+
52+
After applying this fix, the delete operation should work correctly:
53+
54+
```python
55+
# This should now succeed
56+
await delete_project("test-verify")
57+
```
58+
59+
## Related Code
60+
61+
The same pattern is correctly used in other methods:
62+
- `set_default_project()` correctly uses the found `project_name` when setting default
63+
- The API endpoint `remove_project()` in project_router.py correctly passes through to this method
64+
65+
## Commit Message
66+
67+
```
68+
fix: use found project_name in ConfigManager.remove_project()
69+
70+
The remove_project() method was using the input name parameter to delete
71+
from config.projects instead of the project_name returned by get_project().
72+
This caused failures when the input name didn't exactly match the config
73+
dictionary key, even though get_project() successfully found the project
74+
via permalink matching.
75+
76+
Now uses the actual project_name returned by get_project() to ensure we're
77+
deleting the correct dictionary key.
78+
79+
Fixes: Project deletion failing with "not found" error despite project existing
80+
```

src/basic_memory/config.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,8 @@ def remove_project(self, name: str) -> None:
354354
if project_name == config.default_project: # pragma: no cover
355355
raise ValueError(f"Cannot remove the default project '{name}'")
356356

357-
del config.projects[name]
357+
# Use the found project_name (which may differ from input name due to permalink matching)
358+
del config.projects[project_name]
358359
self.save_config(config)
359360

360361
def set_default_project(self, name: str) -> None:

tests/test_config.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,67 @@ def test_config_manager_default_without_custom_config_dir(self, config_home, mon
204204
# Should use default location
205205
assert config_manager.config_dir == config_home / ".basic-memory"
206206
assert config_manager.config_file == config_home / ".basic-memory" / "config.json"
207+
208+
def test_remove_project_with_exact_name_match(self, temp_config_manager):
209+
"""Test remove_project when project name matches config key exactly."""
210+
config_manager = temp_config_manager
211+
212+
# Verify project exists
213+
config = config_manager.load_config()
214+
assert "test-project" in config.projects
215+
216+
# Remove the project with exact name match
217+
config_manager.remove_project("test-project")
218+
219+
# Verify the project was removed
220+
config = config_manager.load_config()
221+
assert "test-project" not in config.projects
222+
223+
def test_remove_project_with_permalink_lookup(self, temp_config_manager):
224+
"""Test remove_project when input needs permalink normalization."""
225+
config_manager = temp_config_manager
226+
227+
# Add a project with normalized key
228+
config = config_manager.load_config()
229+
config.projects["special-chars-project"] = str(Path("/tmp/special"))
230+
config_manager.save_config(config)
231+
232+
# Remove using a name that will normalize to the config key
233+
config_manager.remove_project(
234+
"Special Chars Project"
235+
) # This should normalize to "special-chars-project"
236+
237+
# Verify the project was removed using the correct config key
238+
updated_config = config_manager.load_config()
239+
assert "special-chars-project" not in updated_config.projects
240+
241+
def test_remove_project_uses_canonical_name(self, temp_config_manager):
242+
"""Test that remove_project uses the canonical config key, not user input."""
243+
config_manager = temp_config_manager
244+
245+
# Add a project with a config key that differs from user input
246+
config = config_manager.load_config()
247+
config.projects["my-test-project"] = str(Path("/tmp/mytest"))
248+
config_manager.save_config(config)
249+
250+
# Remove using input that will match but is different from config key
251+
config_manager.remove_project("My Test Project") # Should find "my-test-project"
252+
253+
# Verify that the canonical config key was removed
254+
updated_config = config_manager.load_config()
255+
assert "my-test-project" not in updated_config.projects
256+
257+
def test_remove_project_nonexistent_project(self, temp_config_manager):
258+
"""Test remove_project raises ValueError for nonexistent project."""
259+
config_manager = temp_config_manager
260+
261+
with pytest.raises(ValueError, match="Project 'nonexistent' not found"):
262+
config_manager.remove_project("nonexistent")
263+
264+
def test_remove_project_cannot_remove_default(self, temp_config_manager):
265+
"""Test remove_project raises ValueError when trying to remove default project."""
266+
config_manager = temp_config_manager
267+
268+
# Try to remove the default project
269+
with pytest.raises(ValueError, match="Cannot remove the default project"):
270+
config_manager.remove_project("main")

0 commit comments

Comments
 (0)