Skip to content

fix(memory): atomic graph persistence and validate entity existence in create_relations - #4656

Open
AbhiPra24 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AbhiPra24:fix/memory-atomic-write-and-relations
Open

fix(memory): atomic graph persistence and validate entity existence in create_relations#4656
AbhiPra24 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AbhiPra24:fix/memory-atomic-write-and-relations

Conversation

@AbhiPra24

@AbhiPra24 AbhiPra24 commented Aug 17, 2026

Copy link
Copy Markdown

Summary

Fixes #4614 and #4457.

1. Atomic Persistence in saveGraph() (Fixes #4614)

  • Problem: saveGraph() directly called fs.writeFile(this.memoryFilePath, ...), which truncates the file immediately upon opening. If the process is terminated mid-write, the memory file becomes corrupted or empty.
  • Fix: Write serialized lines to a temporary file in the same directory (${this.memoryFilePath}.${randomBytes(16).toString('hex')}.tmp), followed by an atomic fs.rename over this.memoryFilePath. The temporary file is cleaned up if an error occurs.

2. Referential Integrity in createRelations() (Fixes #4457)

  • Problem: createRelations() only checked for duplicate relations, accepting dangling relations between non-existent entities without validation.
  • Fix: Validates that all referenced from and to entities exist in graph.entities prior to mutating the graph, throwing Entity with name <name> not found to match addObservations().

Verification

  • Added regression tests in src/memory/__tests__/knowledge-graph.test.ts.
  • npm run build and npm test passed (56/56 tests green).

cc @olaservo for review when available. Thank you!

@Selven81

Copy link
Copy Markdown

Thanks for this. I took the explicit "utf-8" on the write — it doesn't change
behaviour, but it makes the assumption visible, and none of the other PRs spell
it out.

On the relation validation: it's good work, but it addresses #4457, which is a
separate concern from persistence. Bundling it here makes both harder to review
in isolation — which I think is exactly why you opened #4696. I said as much in
the summary.

#4614 (comment)

@olaservo

Copy link
Copy Markdown
Member

The atomic-write half of this landed via #4642. The createRelations entity validation is a separate behaviour change (#4457); if you rebase that part alone onto main as its own PR it can be reviewed on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants