fix(memory): validate knowledge graph entries when loading from disk - #4717
Open
Ethanz11-creat wants to merge 1 commit into
Open
fix(memory): validate knowledge graph entries when loading from disk#4717Ethanz11-creat wants to merge 1 commit into
Ethanz11-creat wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2044 by addressing the root cause rather than adding defensive checks to
searchNodes.The crash happens because
loadGraph()trusts the persistedmemory.jsonlfile and pushes every parsed line into the in-memory graph without validating it. A corrupted or legacy entry — an entity missingentityType, an observation that isnull, a relation missingrelationType— reachessearchNodesand throwsCannot read properties of undefined (reading 'toLowerCase').This change validates each line against the existing
EntitySchema/RelationSchemabefore 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
Motivation and Context
search_nodescrashes withMCP 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 runpasses: 58 tests across 4 files (unit level; not yet exercised through a live LLM client).knowledge-graph.test.tsunderloadGraph validation:entityType,nullobservation) are skipped andsearchNodesno longer throws;relationType) are skipped;Breaking Changes
None. Valid memory files load exactly as before; only malformed entries are now skipped instead of crashing the server.
Types of changes
Checklist
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.errorstyle used for file migration notices.