Skip to content

Commit a4a3b1b

Browse files
jope-bmclaude
andauthored
fix: handle missing 'name' key in memory JSON import (#241)
Signed-off-by: Joe P <joe@basicmemory.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: jope-bm <jope-bm@users.noreply.github.com>
1 parent 6361574 commit a4a3b1b

11 files changed

Lines changed: 183 additions & 110 deletions

File tree

src/basic_memory/cli/commands/import_memory_json.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ def memory_json(
7676
Panel(
7777
f"[green]Import complete![/green]\n\n"
7878
f"Created {result.entities} entities\n"
79-
f"Added {result.relations} relations",
79+
f"Added {result.relations} relations\n"
80+
f"Skipped {result.skipped_entities} entities\n",
8081
expand=False,
8182
)
8283
)

src/basic_memory/importers/memory_json_importer.py

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ async def import_data(
3232
# First pass - collect all relations by source entity
3333
entity_relations: Dict[str, List[Relation]] = {}
3434
entities: Dict[str, Dict[str, Any]] = {}
35+
skipped_entities: int = 0
3536

3637
# Ensure the base path exists
3738
base_path = config.home # pragma: no cover
@@ -42,7 +43,13 @@ async def import_data(
4243
for line in source_data:
4344
data = line
4445
if data["type"] == "entity":
45-
entities[data["name"]] = data
46+
# Handle different possible name keys
47+
entity_name = data.get("name") or data.get("entityName") or data.get("id")
48+
if not entity_name:
49+
logger.warning(f"Entity missing name field: {data}")
50+
skipped_entities += 1
51+
continue
52+
entities[entity_name] = data
4653
elif data["type"] == "relation":
4754
# Store relation with its source entity
4855
source = data.get("from") or data.get("from_id")
@@ -58,25 +65,31 @@ async def import_data(
5865
# Second pass - create and write entities
5966
entities_created = 0
6067
for name, entity_data in entities.items():
68+
# Get entity type with fallback
69+
entity_type = entity_data.get("entityType") or entity_data.get("type") or "entity"
70+
6171
# Ensure entity type directory exists
62-
entity_type_dir = base_path / entity_data["entityType"]
72+
entity_type_dir = base_path / entity_type
6373
entity_type_dir.mkdir(parents=True, exist_ok=True)
6474

75+
# Get observations with fallback to empty list
76+
observations = entity_data.get("observations", [])
77+
6578
entity = EntityMarkdown(
6679
frontmatter=EntityFrontmatter(
6780
metadata={
68-
"type": entity_data["entityType"],
81+
"type": entity_type,
6982
"title": name,
70-
"permalink": f"{entity_data['entityType']}/{name}",
83+
"permalink": f"{entity_type}/{name}",
7184
}
7285
),
7386
content=f"# {name}\n",
74-
observations=[Observation(content=obs) for obs in entity_data["observations"]],
87+
observations=[Observation(content=obs) for obs in observations],
7588
relations=entity_relations.get(name, []),
7689
)
7790

7891
# Write entity file
79-
file_path = base_path / f"{entity_data['entityType']}/{name}.md"
92+
file_path = base_path / f"{entity_type}/{name}.md"
8093
await self.write_entity(entity, file_path)
8194
entities_created += 1
8295

@@ -87,6 +100,7 @@ async def import_data(
87100
success=True,
88101
entities=entities_created,
89102
relations=relations_count,
103+
skipped_entities=skipped_entities,
90104
)
91105

92106
except Exception as e: # pragma: no cover

src/basic_memory/mcp/tools/read_note.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ async def read_note(
6868

6969
# Get the file via REST API - first try direct permalink lookup
7070
entity_path = memory_url_path(identifier)
71-
71+
7272
# Validate path to prevent path traversal attacks
7373
project_path = active_project.home
7474
if not validate_project_path(entity_path, project_path):

src/basic_memory/schemas/importer.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,4 @@ class EntityImportResult(ImportResult):
3232

3333
entities: int = 0
3434
relations: int = 0
35+
skipped_entities: int = 0

src/basic_memory/utils.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,23 +220,23 @@ def validate_project_path(path: str, project_path: Path) -> bool:
220220
# Allow empty strings as they resolve to the project root
221221
if not path:
222222
return True
223-
223+
224224
# Check for obvious path traversal patterns first
225225
if ".." in path or "~" in path:
226226
return False
227-
227+
228228
# Check for Windows-style path traversal (even on Unix systems)
229229
if "\\.." in path or path.startswith("\\"):
230230
return False
231-
231+
232232
# Block absolute paths (Unix-style starting with / or Windows-style with drive letters)
233233
if path.startswith("/") or (len(path) >= 2 and path[1] == ":"):
234234
return False
235-
235+
236236
# Block paths with control characters (but allow whitespace that will be stripped)
237-
if path.strip() and any(ord(c) < 32 and c not in [' ', '\t'] for c in path):
237+
if path.strip() and any(ord(c) < 32 and c not in [" ", "\t"] for c in path):
238238
return False
239-
239+
240240
try:
241241
resolved = (project_path / path).resolve()
242242
return resolved.is_relative_to(project_path.resolve())

tests/cli/test_import_memory_json.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,43 @@ def test_import_json_command_handle_old_format(tmp_path):
113113
result = runner.invoke(import_app, ["memory-json", str(json_file)])
114114
assert result.exit_code == 0
115115
assert "Import complete" in result.output
116+
117+
118+
def test_import_json_command_missing_name_key(tmp_path):
119+
"""Test handling JSON with missing 'name' key using 'id' instead."""
120+
# Create JSON with id instead of name (common in Knowledge Graph Memory Server)
121+
data_with_id = [
122+
{
123+
"type": "entity",
124+
"id": "test_entity_id",
125+
"entityType": "test",
126+
"observations": ["Test observation with id"],
127+
},
128+
{
129+
"type": "entity",
130+
"entityName": "test_entity_2",
131+
"entityType": "test",
132+
"observations": ["Test observation with entityName"],
133+
},
134+
{
135+
"type": "entity",
136+
"name": "test_entity_title",
137+
"entityType": "test",
138+
"observations": ["Test observation with name"],
139+
},
140+
]
141+
142+
json_file = tmp_path / "missing_name.json"
143+
with open(json_file, "w", encoding="utf-8") as f:
144+
for item in data_with_id:
145+
f.write(json.dumps(item) + "\n")
146+
147+
# Set up test environment
148+
monkeypatch = pytest.MonkeyPatch()
149+
monkeypatch.setenv("HOME", str(tmp_path))
150+
151+
# Run import - should not fail even without 'name' key
152+
result = runner.invoke(import_app, ["memory-json", str(json_file)])
153+
assert result.exit_code == 0
154+
assert "Import complete" in result.output
155+
assert "Created 3 entities" in result.output

tests/mcp/test_tool_move_note.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -650,7 +650,7 @@ async def test_move_note_allows_safe_paths(self, client):
650650
assert isinstance(result, str)
651651
# Should NOT contain security error message
652652
assert "Security Validation Error" not in result
653-
653+
654654
# If it fails, it should be for other reasons like "already exists" or API errors
655655
if "Move Failed" in result:
656656
assert "paths must stay within project boundaries" not in result
@@ -672,7 +672,7 @@ async def test_move_note_security_logging(self, client, caplog):
672672
)
673673

674674
assert "# Move Failed - Security Validation Error" in result
675-
675+
676676
# Check that security violation was logged
677677
# Note: This test may need adjustment based on the actual logging setup
678678
# The security validation should generate a warning log entry
@@ -710,7 +710,7 @@ async def test_move_note_current_directory_references_security(self, client):
710710
# Test current directory references (should be safe)
711711
safe_paths = [
712712
"./notes/file.md",
713-
"folder/./file.md",
713+
"folder/./file.md",
714714
"./folder/subfolder/file.md",
715715
]
716716

0 commit comments

Comments
 (0)