Skip to content

fix(memory): validate knowledge graph entries when loading from disk - #4717

Open
Ethanz11-creat wants to merge 1 commit into
modelcontextprotocol:mainfrom
Ethanz11-creat:fix/memory-validate-graph-on-load
Open

fix(memory): validate knowledge graph entries when loading from disk#4717
Ethanz11-creat wants to merge 1 commit into
modelcontextprotocol:mainfrom
Ethanz11-creat:fix/memory-validate-graph-on-load

Conversation

@Ethanz11-creat

@Ethanz11-creat Ethanz11-creat commented Aug 29, 2026

Copy link
Copy Markdown

Description

Fixes #2044 by addressing the root cause rather than adding defensive checks to searchNodes.

The crash happens because loadGraph() trusts the persisted memory.jsonl file and pushes every parsed line into the in-memory graph without validating it. A corrupted or legacy entry — an entity missing entityType, an observation that is null, a relation missing relationType — reaches searchNodes and throws Cannot read properties of undefined (reading 'toLowerCase').

This change validates each line against the existing EntitySchema / RelationSchema before adding it to the graph, and skips malformed entries (including malformed JSON lines) with a warning on stderr. Tools already validate their inputs with these schemas, so this closes the last unvalidated path (the on-disk file) and guarantees the in-memory graph only ever contains well-formed entities and relations.

This follows the direction from the earlier attempt (#2054): fix the source of non-strings rather than papering over the crash in searchNodes.

Server Details

  • Server: memory
  • Changes to: knowledge graph persistence (loading)

Motivation and Context

search_nodes crashes with MCP error -32603: Cannot read properties of undefined (reading 'toLowerCase') for users whose memory file contains a legacy or hand-edited entity. See #2044.

How Has This Been Tested?

  • npm run build (tsc) passes.
  • npx vitest run passes: 58 tests across 4 files (unit level; not yet exercised through a live LLM client).
  • Added 3 tests in knowledge-graph.test.ts under loadGraph validation:
    • corrupt entities (missing entityType, null observation) are skipped and searchNodes no longer throws;
    • corrupt relations (missing relationType) are skipped;
    • malformed JSON lines are skipped.
  • Existing tests (including "strip type field when loading") still pass.

Breaking Changes

None. Valid memory files load exactly as before; only malformed entries are now skipped instead of crashing the server.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follows MCP security best practices
  • I have updated the server's README accordingly
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options

Additional context

I kept the skip-with-warning behavior minimal and scoped to loading, so no existing data is mutated and nothing is silently dropped without a stderr trace. The warning text mirrors the existing console.error style used for file migration notices.

loadGraph() trusted the persisted memory file and pushed entities and
relations without validating their fields. A corrupted or legacy entry
(e.g. an entity missing entityType, or an observation that is not a string)
would reach searchNodes and crash with "Cannot read properties of undefined
(reading 'toLowerCase')".

Validate each line against the existing EntitySchema/RelationSchema and skip
malformed entries with a warning, so the in-memory graph only ever contains
well-formed data. Malformed JSON lines are skipped as well.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Cannot read properties of undefined (reading 'toLowerCase')" Error

1 participant