diff --git a/package-lock.json b/package-lock.json index 1845571736..632d3404ad 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3866,7 +3866,8 @@ "@modelcontextprotocol/sdk": "^1.30.0", "diff": "^8.0.3", "glob": "^13.0.6", - "minimatch": "^10.0.1" + "minimatch": "^10.0.1", + "zod": "^4.0.0" }, "bin": { "mcp-server-filesystem": "dist/index.js" @@ -3886,7 +3887,8 @@ "version": "0.6.3", "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@modelcontextprotocol/sdk": "^1.30.0" + "@modelcontextprotocol/sdk": "^1.30.0", + "zod": "^4.0.0" }, "bin": { "mcp-server-memory": "dist/index.js" @@ -3906,7 +3908,8 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2" + "yargs": "^17.7.2", + "zod": "^4.0.0" }, "bin": { "mcp-server-sequential-thinking": "dist/index.js" diff --git a/src/filesystem/package.json b/src/filesystem/package.json index 139c4f00b4..3c288a2673 100644 --- a/src/filesystem/package.json +++ b/src/filesystem/package.json @@ -28,7 +28,8 @@ "@modelcontextprotocol/sdk": "^1.30.0", "diff": "^8.0.3", "glob": "^13.0.6", - "minimatch": "^10.0.1" + "minimatch": "^10.0.1", + "zod": "^4.0.0" }, "devDependencies": { "@types/diff": "^5.0.9", diff --git a/src/memory/README.md b/src/memory/README.md index 18851aedaa..de7be2c060 100644 --- a/src/memory/README.md +++ b/src/memory/README.md @@ -87,7 +87,7 @@ Example: - Remove entities and their relations - Input: `entityNames` (string[]) - Cascading deletion of associated relations - - Silent operation if entity doesn't exist + - No error if an entity doesn't exist; the response reports which names were not found - **delete_observations** - Remove specific observations from entities @@ -95,7 +95,7 @@ Example: - Each object contains: - `entityName` (string): Target entity - `observations` (string[]): Observations to remove - - Silent operation if observation doesn't exist + - No error if an observation doesn't exist; the response reports how many were deleted - **delete_relations** - Remove specific relations from the graph @@ -104,7 +104,7 @@ Example: - `from` (string): Source entity name - `to` (string): Target entity name - `relationType` (string): Relationship type - - Silent operation if relation doesn't exist + - No error if a relation doesn't exist; the response reports how many were deleted - **read_graph** - Read the entire knowledge graph diff --git a/src/memory/__tests__/delete-reporting.test.ts b/src/memory/__tests__/delete-reporting.test.ts new file mode 100644 index 0000000000..944c30204f --- /dev/null +++ b/src/memory/__tests__/delete-reporting.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { KnowledgeGraphManager, Entity, Relation } from '../index.js'; + +/** + * The delete tools stay silent when a target is absent, which the README + * documents. What they must not do is report a deletion that did not happen: + * an agent that mistypes a name is told its memory is clean while the data is + * still on disk, and nothing in the response contradicts that. + */ +describe('delete reporting', () => { + let manager: KnowledgeGraphManager; + let testFilePath: string; + + const entities: Entity[] = [ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp', 'likes tea'] }, + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]; + const relations: Relation[] = [{ from: 'Alice', to: 'Bob', relationType: 'works_with' }]; + + beforeEach(async () => { + testFilePath = path.join( + path.dirname(fileURLToPath(import.meta.url)), + `test-delete-reporting-${Date.now()}-${Math.random().toString(16).slice(2)}.jsonl` + ); + manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities(entities); + await manager.createRelations(relations); + }); + + afterEach(async () => { + try { + await fs.unlink(testFilePath); + } catch { + // the file is gone already + } + }); + + describe('deleteEntities', () => { + it('reports which names matched and which did not', async () => { + const result = await manager.deleteEntities(['Alice', 'Alise']); + expect(result).toEqual({ deleted: ['Alice'], notFound: ['Alise'] }); + }); + + it('reports nothing deleted when no name matches', async () => { + const result = await manager.deleteEntities(['Nobody']); + expect(result).toEqual({ deleted: [], notFound: ['Nobody'] }); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(2); + }); + + it('still deletes the entity and its relations', async () => { + await manager.deleteEntities(['Alice']); + + const graph = await manager.readGraph(); + expect(graph.entities.map(e => e.name)).toEqual(['Bob']); + expect(graph.relations).toHaveLength(0); + }); + }); + + describe('deleteObservations', () => { + it('counts only the observations that were present', async () => { + const result = await manager.deleteObservations([ + { entityName: 'Alice', observations: ['likes tea', 'never said this'] }, + ]); + expect(result).toEqual({ deletedCount: 1, missingEntities: [] }); + }); + + it('names an entity that does not exist', async () => { + const result = await manager.deleteObservations([ + { entityName: 'Carol', observations: ['anything'] }, + ]); + expect(result).toEqual({ deletedCount: 0, missingEntities: ['Carol'] }); + }); + }); + + describe('deleteRelations', () => { + it('counts only the relations that matched', async () => { + const result = await manager.deleteRelations([ + { from: 'Alice', to: 'Bob', relationType: 'works_with' }, + { from: 'Alice', to: 'Bob', relationType: 'never_existed' }, + ]); + expect(result).toEqual({ deletedCount: 1 }); + }); + + it('reports nothing deleted when the relation type is wrong', async () => { + const result = await manager.deleteRelations([ + { from: 'Alice', to: 'Bob', relationType: 'manages' }, + ]); + expect(result).toEqual({ deletedCount: 0 }); + + const graph = await manager.readGraph(); + expect(graph.relations).toHaveLength(1); + }); + }); +}); diff --git a/src/memory/__tests__/file-path.test.ts b/src/memory/__tests__/file-path.test.ts index d1a16e4600..fe5fdeb0db 100644 --- a/src/memory/__tests__/file-path.test.ts +++ b/src/memory/__tests__/file-path.test.ts @@ -1,8 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { promises as fs } from 'fs'; import path from 'path'; +import os from 'os'; import { fileURLToPath } from 'url'; -import { ensureMemoryFilePath, defaultMemoryPath } from '../index.js'; +import { ensureMemoryFilePath, defaultMemoryPath, expandHome } from '../index.js'; describe('ensureMemoryFilePath', () => { const testDir = path.dirname(fileURLToPath(import.meta.url)); @@ -72,6 +73,15 @@ describe('ensureMemoryFilePath', () => { expect(path.isAbsolute(result)).toBe(true); } }); + + it('should expand a leading "~/" to the home directory', async () => { + process.env.MEMORY_FILE_PATH = '~/custom-memory.jsonl'; + + const result = await ensureMemoryFilePath(); + + expect(result).toBe(path.join(os.homedir(), 'custom-memory.jsonl')); + expect(path.isAbsolute(result)).toBe(true); + }); }); describe('without MEMORY_FILE_PATH environment variable', () => { @@ -154,3 +164,29 @@ describe('ensureMemoryFilePath', () => { }); }); }); + +describe('expandHome', () => { + it('expands a bare "~" to the home directory', () => { + expect(expandHome('~')).toBe(os.homedir()); + }); + + it('expands a leading "~/" to the home directory', () => { + expect(expandHome('~/notes/memory.jsonl')).toBe( + path.join(os.homedir(), 'notes/memory.jsonl') + ); + }); + + it('leaves a "~" not followed by a separator unchanged', () => { + expect(expandHome('~backup.jsonl')).toBe('~backup.jsonl'); + }); + + it('leaves absolute paths unchanged', () => { + expect(expandHome('/var/data/memory.jsonl')).toBe('/var/data/memory.jsonl'); + }); + + it('leaves relative paths unchanged', () => { + expect(expandHome(path.join('data', 'memory.jsonl'))).toBe( + path.join('data', 'memory.jsonl') + ); + }); +}); diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index 61e823a2a6..7d05a0c053 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -59,6 +59,20 @@ describe('KnowledgeGraphManager', () => { const newEntities = await manager.createEntities([]); expect(newEntities).toHaveLength(0); }); + + it('should ignore duplicate entity names within a single batch', async () => { + const entities: Entity[] = [ + { name: 'Alice', entityType: 'person', observations: ['first'] }, + { name: 'Alice', entityType: 'person', observations: ['second'] }, + ]; + + const newEntities = await manager.createEntities(entities); + expect(newEntities).toHaveLength(1); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(1); + expect(graph.entities[0].name).toBe('Alice'); + }); }); describe('createRelations', () => { @@ -135,6 +149,24 @@ describe('KnowledgeGraphManager', () => { const newRelations = await manager.createRelations([]); expect(newRelations).toHaveLength(0); }); + + it('should skip duplicate relations within a single batch', async () => { + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: [] }, + { name: 'Bob', entityType: 'person', observations: [] }, + ]); + + const relations: Relation[] = [ + { from: 'Alice', to: 'Bob', relationType: 'knows' }, + { from: 'Alice', to: 'Bob', relationType: 'knows' }, + ]; + + const newRelations = await manager.createRelations(relations); + expect(newRelations).toHaveLength(1); + + const graph = await manager.readGraph(); + expect(graph.relations).toHaveLength(1); + }); }); describe('addObservations', () => { @@ -458,6 +490,76 @@ describe('KnowledgeGraphManager', () => { expect(JSON.parse(lines[1])).toHaveProperty('type', 'relation'); }); + it('should write a trailing newline to produce valid JSONL', async () => { + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['test'] }, + ]); + + const fileContent = await fs.readFile(testFilePath, 'utf-8'); + expect(fileContent.endsWith('\n')).toBe(true); + }); + + it('should produce a file where every line is individually valid JSON', async () => { + // This test catches the bug where saveGraph wrote lines.join("\n") + // without a trailing newline. When the file was later appended to + // (e.g. by a concurrent process or external tool), the last JSON + // object and the new first JSON object ended up on the same line, + // producing invalid JSONL like: + // {"type":"entity","name":"Alice"}{"type":"relation","from":"Alice",...} + // which fails with: "Unexpected non-whitespace character after JSON + // at position N" + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['test'] }, + { name: 'Bob', entityType: 'person', observations: [] }, + ]); + await manager.createRelations([ + { from: 'Alice', to: 'Bob', relationType: 'knows' }, + ]); + + const fileContent = await fs.readFile(testFilePath, 'utf-8'); + const allLines = fileContent.split('\n'); + + // Every non-empty line must be valid JSON on its own + for (const line of allLines) { + if (line.trim() === '') continue; + expect(() => JSON.parse(line)).not.toThrow(); + } + }); + + it('should not corrupt JSONL when content is appended to the file externally', async () => { + // Simulate the real-world corruption scenario: + // 1. saveGraph writes entities to the file + // 2. An external process appends a new JSON line to the file + // 3. loadGraph must still parse the file without errors + // + // Without a trailing newline on step 1, the appended content in + // step 2 lands on the same line as the last entity, producing + // invalid JSONL that breaks loadGraph. + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['original'] }, + ]); + + // Simulate an external append (e.g. another process, a script, or + // a crash-recovery replay). This is what triggers the bug: without + // a trailing newline, this JSON object concatenates onto line 1. + const externalLine = JSON.stringify({ + type: 'entity', + name: 'External', + entityType: 'person', + observations: ['appended externally'], + }); + await fs.appendFile(testFilePath, externalLine + '\n'); + + // A new manager instance forces a fresh loadGraph from disk + const manager2 = new KnowledgeGraphManager(testFilePath); + const graph = await manager2.readGraph(); + + // Both entities must load without a JSON parse error + expect(graph.entities).toHaveLength(2); + expect(graph.entities.map(e => e.name)).toContain('Alice'); + expect(graph.entities.map(e => e.name)).toContain('External'); + }); + it('should strip type field from entities when loading from file', async () => { // Create entities and relations (these get saved with type field) await manager.createEntities([ @@ -547,4 +649,126 @@ describe('KnowledgeGraphManager', () => { expect(result.relations[0]).not.toHaveProperty('type'); }); }); + + describe('loadGraph validation', () => { + it('skips corrupt entities instead of crashing search', async () => { + const lines = [ + JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }), + JSON.stringify({ type: 'entity', name: 'Broken', observations: ['missing entityType'] }), + JSON.stringify({ type: 'entity', name: 'BadObs', entityType: 'person', observations: ['ok', null] }), + ]; + await fs.writeFile(testFilePath, lines.join('\n') + '\n'); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(1); + expect(graph.entities[0].name).toBe('Alice'); + + // searchNodes must not throw even though the file contains corrupt entries + const result = await manager.searchNodes('Acme'); + expect(result.entities).toHaveLength(1); + expect(result.entities[0].name).toBe('Alice'); + }); + + it('skips corrupt relations', async () => { + const lines = [ + JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: [] }), + JSON.stringify({ type: 'relation', from: 'Alice', to: 'Bob' }), // missing relationType + JSON.stringify({ type: 'relation', from: 'Alice', to: 'Bob', relationType: 'knows' }), + ]; + await fs.writeFile(testFilePath, lines.join('\n') + '\n'); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(1); + expect(graph.relations).toHaveLength(1); + expect(graph.relations[0].relationType).toBe('knows'); + }); + + it('skips malformed JSON lines', async () => { + const lines = [ + JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: [] }), + '{this is not valid json', + JSON.stringify({ type: 'entity', name: 'Bob', entityType: 'person', observations: [] }), + ]; + await fs.writeFile(testFilePath, lines.join('\n') + '\n'); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(2); + expect(graph.entities.map(e => e.name)).toEqual(['Alice', 'Bob']); + }); + }); + + describe('concurrent mutations', () => { + // Regression test for #1819: concurrent tool calls each independently + // load the graph, mutate their own copy, and write it back. Without + // serialization, whichever write lands last silently discards the + // other's changes. All mutations below are fired without awaiting each + // other first, simulating multiple tool calls landing close together. + + it('should not lose entities created concurrently', async () => { + const batch1: Entity[] = Array.from({ length: 10 }, (_, i) => ({ + name: `batch1-entity-${i}`, + entityType: 'test', + observations: [], + })); + const batch2: Entity[] = Array.from({ length: 10 }, (_, i) => ({ + name: `batch2-entity-${i}`, + entityType: 'test', + observations: [], + })); + + // Fire both concurrently instead of awaiting sequentially. + await Promise.all([ + manager.createEntities(batch1), + manager.createEntities(batch2), + ]); + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(20); + expect(graph.entities.map(e => e.name).sort()).toEqual( + [...batch1, ...batch2].map(e => e.name).sort() + ); + }); + + it('should not lose relations created concurrently with entity creation', async () => { + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: [] }, + { name: 'Bob', entityType: 'person', observations: [] }, + { name: 'Carol', entityType: 'person', observations: [] }, + ]); + + await Promise.all([ + manager.createRelations([{ from: 'Alice', to: 'Bob', relationType: 'knows' }]), + manager.createRelations([{ from: 'Bob', to: 'Carol', relationType: 'knows' }]), + manager.addObservations([ + { entityName: 'Alice', contents: ['likes coffee'] }, + ]), + ]); + + const graph = await manager.readGraph(); + expect(graph.relations).toHaveLength(2); + expect(graph.entities.find(e => e.name === 'Alice')?.observations).toContain('likes coffee'); + }); + + it('should keep the file valid JSONL after many concurrent mutations', async () => { + const operations = Array.from({ length: 25 }, (_, i) => + manager.createEntities([ + { name: `stress-entity-${i}`, entityType: 'test', observations: [] }, + ]) + ); + + await Promise.all(operations); + + const raw = await fs.readFile(testFilePath, 'utf-8'); + const lines = raw.split('\n').filter(line => line.trim() !== ''); + + // Every line must parse as valid JSON; a corrupted interleaved write + // would produce a truncated or malformed line here. + for (const line of lines) { + expect(() => JSON.parse(line)).not.toThrow(); + } + + const graph = await manager.readGraph(); + expect(graph.entities).toHaveLength(25); + }); + }); }); diff --git a/src/memory/__tests__/search-nodes-schema.test.ts b/src/memory/__tests__/search-nodes-schema.test.ts new file mode 100644 index 0000000000..c03f384ebb --- /dev/null +++ b/src/memory/__tests__/search-nodes-schema.test.ts @@ -0,0 +1,25 @@ +import { describe, it, expect } from 'vitest'; +import { SearchNodesQuerySchema, SEARCH_QUERY_MAX_LENGTH } from '../index.js'; + +describe('search_nodes input schema', () => { + it('should accept a normal query', () => { + expect(SearchNodesQuerySchema.safeParse('Alice').success).toBe(true); + expect(SearchNodesQuerySchema.safeParse('works at Acme Corp').success).toBe(true); + }); + + it('should accept a query at exactly the max length', () => { + const atLimit = 'a'.repeat(SEARCH_QUERY_MAX_LENGTH); + expect(SearchNodesQuerySchema.safeParse(atLimit).success).toBe(true); + }); + + it('should reject a query longer than the max length', () => { + const oversized = 'a'.repeat(SEARCH_QUERY_MAX_LENGTH + 1); + const result = SearchNodesQuerySchema.safeParse(oversized); + expect(result.success).toBe(false); + }); + + it('should still reject non-string input', () => { + expect(SearchNodesQuerySchema.safeParse(42).success).toBe(false); + expect(SearchNodesQuerySchema.safeParse(null).success).toBe(false); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index 3f1179dd66..d9f814877b 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -6,6 +6,7 @@ import { SubscribeRequestSchema, UnsubscribeRequestSchema } from "@modelcontextp import { z } from "zod"; import { promises as fs } from 'fs'; import path from 'path'; +import os from 'os'; import { randomBytes } from 'crypto'; import { fileURLToPath } from 'url'; import { SERVER_VERSION } from './version.js'; @@ -13,13 +14,27 @@ import { SERVER_VERSION } from './version.js'; // Define memory file path using environment variable with fallback export const defaultMemoryPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'memory.jsonl'); +// Expand a leading "~" to the user's home directory. MCP clients pass +// MEMORY_FILE_PATH from JSON config, where no shell performs tilde expansion, +// so an unexpanded "~" would otherwise be treated as a relative path and +// joined onto the package directory. Mirrors the helper of the same name in +// the filesystem server (src/filesystem/path-utils.ts). +export function expandHome(filepath: string): string { + if (filepath.startsWith('~/') || filepath === '~') { + return path.join(os.homedir(), filepath.slice(1)); + } + return filepath; +} + // Handle backward compatibility: migrate memory.json to memory.jsonl if needed export async function ensureMemoryFilePath(): Promise { if (process.env.MEMORY_FILE_PATH) { - // Custom path provided, use it as-is (with absolute path resolution) - return path.isAbsolute(process.env.MEMORY_FILE_PATH) - ? process.env.MEMORY_FILE_PATH - : path.join(path.dirname(fileURLToPath(import.meta.url)), process.env.MEMORY_FILE_PATH); + // Custom path provided. Expand a leading "~" first, then resolve relative + // paths against the package directory (absolute paths are used as-is). + const customPath = expandHome(process.env.MEMORY_FILE_PATH); + return path.isAbsolute(customPath) + ? customPath + : path.join(path.dirname(fileURLToPath(import.meta.url)), customPath); } // No custom path set, check for backward compatibility migration @@ -71,28 +86,71 @@ export interface KnowledgeGraph { export class KnowledgeGraphManager { constructor(private memoryFilePath: string) {} + // Serializes all read-modify-write graph mutations behind a single queue. + // Without this, concurrent tool calls (e.g. multiple mutations dispatched + // from one LLM turn) each independently load the graph, mutate their own + // copy, and write it back — so whichever write lands last silently + // overwrites the other's changes, and interleaved writes to the same file + // can corrupt it outright. See #1819. + private mutationQueue: Promise = Promise.resolve(); + + private async withLock(operation: () => Promise): Promise { + const result = this.mutationQueue.then(operation, operation); + // Always resolve the queue itself, even if this operation failed, so a + // single failed mutation doesn't permanently wedge every call after it. + // The failure still propagates normally to whoever awaited `result`. + this.mutationQueue = result.then( + () => undefined, + () => undefined, + ); + return result; + } + private async loadGraph(): Promise { try { const data = await fs.readFile(this.memoryFilePath, "utf-8"); const lines = data.split("\n").filter(line => line.trim() !== ""); - return lines.reduce((graph: KnowledgeGraph, line) => { - const item = JSON.parse(line); - if (item.type === "entity") { - graph.entities.push({ - name: item.name, - entityType: item.entityType, - observations: item.observations - }); + const graph: KnowledgeGraph = { entities: [], relations: [] }; + + for (const line of lines) { + let item: unknown; + try { + item = JSON.parse(line); + } catch { + console.error("Skipping malformed line in memory file"); + continue; + } + + if (typeof item !== "object" || item === null) { + console.error("Skipping non-object line in memory file"); + continue; } - if (item.type === "relation") { - graph.relations.push({ - from: item.from, - to: item.to, - relationType: item.relationType - }); + + const record = item as Record; + if (record.type === "entity") { + const parsed = EntitySchema.safeParse(item); + if (parsed.success) { + graph.entities.push(parsed.data); + } else { + console.error( + "Skipping invalid entity in memory file:", + parsed.error.issues.map(issue => `${issue.path.join(".")}: ${issue.message}`).join(", ") + ); + } + } else if (record.type === "relation") { + const parsed = RelationSchema.safeParse(item); + if (parsed.success) { + graph.relations.push(parsed.data); + } else { + console.error( + "Skipping invalid relation in memory file:", + parsed.error.issues.map(issue => `${issue.path.join(".")}: ${issue.message}`).join(", ") + ); + } } - return graph; - }, { entities: [], relations: [] }); + } + + return graph; } catch (error) { if (error instanceof Error && 'code' in error && (error as any).code === "ENOENT") { return { entities: [], relations: [] }; @@ -132,7 +190,7 @@ export class KnowledgeGraphManager { ); try { - await fs.writeFile(tempFilePath, lines.join("\n")); + await fs.writeFile(tempFilePath, lines.join("\n") + "\n"); await fs.rename(tempFilePath, this.memoryFilePath); } catch (error) { // Never leave a stray temp file behind on failure. @@ -142,77 +200,110 @@ export class KnowledgeGraphManager { } async createEntities(entities: Entity[]): Promise { - const graph = await this.loadGraph(); - const newEntities = entities.filter(e => !graph.entities.some(existingEntity => existingEntity.name === e.name)); - graph.entities.push(...newEntities); - await this.saveGraph(graph); - return newEntities; + return this.withLock(async () => { + const graph = await this.loadGraph(); + const newEntities = entities.filter((e, index) => + !graph.entities.some(existingEntity => existingEntity.name === e.name) && + // Also skip duplicates appearing earlier in this same batch + !entities.slice(0, index).some(earlier => earlier.name === e.name) + ); + graph.entities.push(...newEntities); + await this.saveGraph(graph); + return newEntities; + }); } async createRelations(relations: Relation[]): Promise { - const graph = await this.loadGraph(); - const entityNames = new Set(graph.entities.map(e => e.name)); + return this.withLock(async () => { + const graph = await this.loadGraph(); + const entityNames = new Set(graph.entities.map(e => e.name)); - relations.forEach(r => { - if (!entityNames.has(r.from)) { - throw new Error(`Entity with name ${r.from} not found`); - } - if (!entityNames.has(r.to)) { - throw new Error(`Entity with name ${r.to} not found`); - } + relations.forEach(r => { + if (!entityNames.has(r.from)) { + throw new Error(`Entity with name ${r.from} not found`); + } + if (!entityNames.has(r.to)) { + throw new Error(`Entity with name ${r.to} not found`); + } + }); + + const isSameRelation = (a: Relation, b: Relation) => + a.from === b.from && + a.to === b.to && + a.relationType === b.relationType; + const newRelations = relations.filter((r, index) => + !graph.relations.some(existingRelation => isSameRelation(existingRelation, r)) && + // Also skip duplicates appearing earlier in this same batch + !relations.slice(0, index).some(earlier => isSameRelation(earlier, r)) + ); + graph.relations.push(...newRelations); + await this.saveGraph(graph); + return newRelations; }); - - const newRelations = relations.filter(r => !graph.relations.some(existingRelation => - existingRelation.from === r.from && - existingRelation.to === r.to && - existingRelation.relationType === r.relationType - )); - graph.relations.push(...newRelations); - await this.saveGraph(graph); - return newRelations; } async addObservations(observations: { entityName: string; contents: string[] }[]): Promise<{ entityName: string; addedObservations: string[] }[]> { - const graph = await this.loadGraph(); - const results = observations.map(o => { - const entity = graph.entities.find(e => e.name === o.entityName); - if (!entity) { - throw new Error(`Entity with name ${o.entityName} not found`); - } - const newObservations = o.contents.filter(content => !entity.observations.includes(content)); - entity.observations.push(...newObservations); - return { entityName: o.entityName, addedObservations: newObservations }; + return this.withLock(async () => { + const graph = await this.loadGraph(); + const results = observations.map(o => { + const entity = graph.entities.find(e => e.name === o.entityName); + if (!entity) { + throw new Error(`Entity with name ${o.entityName} not found`); + } + const newObservations = o.contents.filter(content => !entity.observations.includes(content)); + entity.observations.push(...newObservations); + return { entityName: o.entityName, addedObservations: newObservations }; + }); + await this.saveGraph(graph); + return results; }); - await this.saveGraph(graph); - return results; } - async deleteEntities(entityNames: string[]): Promise { - const graph = await this.loadGraph(); - graph.entities = graph.entities.filter(e => !entityNames.includes(e.name)); - graph.relations = graph.relations.filter(r => !entityNames.includes(r.from) && !entityNames.includes(r.to)); - await this.saveGraph(graph); + async deleteEntities(entityNames: string[]): Promise<{ deleted: string[]; notFound: string[] }> { + return this.withLock(async () => { + const graph = await this.loadGraph(); + const present = new Set(graph.entities.map(e => e.name)); + const deleted = entityNames.filter(name => present.has(name)); + const notFound = entityNames.filter(name => !present.has(name)); + graph.entities = graph.entities.filter(e => !entityNames.includes(e.name)); + graph.relations = graph.relations.filter(r => !entityNames.includes(r.from) && !entityNames.includes(r.to)); + await this.saveGraph(graph); + return { deleted, notFound }; + }); } - async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise { - const graph = await this.loadGraph(); - deletions.forEach(d => { - const entity = graph.entities.find(e => e.name === d.entityName); - if (entity) { - entity.observations = entity.observations.filter(o => !d.observations.includes(o)); - } + async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise<{ deletedCount: number; missingEntities: string[] }> { + return this.withLock(async () => { + const graph = await this.loadGraph(); + let deletedCount = 0; + const missingEntities: string[] = []; + deletions.forEach(d => { + const entity = graph.entities.find(e => e.name === d.entityName); + if (entity) { + const before = entity.observations.length; + entity.observations = entity.observations.filter(o => !d.observations.includes(o)); + deletedCount += before - entity.observations.length; + } else { + missingEntities.push(d.entityName); + } + }); + await this.saveGraph(graph); + return { deletedCount, missingEntities }; }); - await this.saveGraph(graph); } - async deleteRelations(relations: Relation[]): Promise { - const graph = await this.loadGraph(); - graph.relations = graph.relations.filter(r => !relations.some(delRelation => - r.from === delRelation.from && - r.to === delRelation.to && - r.relationType === delRelation.relationType - )); - await this.saveGraph(graph); + async deleteRelations(relations: Relation[]): Promise<{ deletedCount: number }> { + return this.withLock(async () => { + const graph = await this.loadGraph(); + const before = graph.relations.length; + graph.relations = graph.relations.filter(r => !relations.some(delRelation => + r.from === delRelation.from && + r.to === delRelation.to && + r.relationType === delRelation.relationType + )); + await this.saveGraph(graph); + return { deletedCount: before - graph.relations.length }; + }); } async readGraph(): Promise { @@ -421,11 +512,14 @@ server.registerTool( } }, async ({ entityNames }) => { - await knowledgeGraphManager.deleteEntities(entityNames); + const { deleted, notFound } = await knowledgeGraphManager.deleteEntities(entityNames); notifyGraphUpdated(); + const message = notFound.length === 0 + ? "Entities deleted successfully" + : `Deleted ${deleted.length} of ${entityNames.length} entities. Not found: ${notFound.join(", ")}`; return { - content: [{ type: "text" as const, text: "Entities deleted successfully" }], - structuredContent: { success: true, message: "Entities deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } ); @@ -454,11 +548,16 @@ server.registerTool( } }, async ({ deletions }) => { - await knowledgeGraphManager.deleteObservations(deletions); + const { deletedCount, missingEntities } = await knowledgeGraphManager.deleteObservations(deletions); notifyGraphUpdated(); + const requested = deletions.reduce((total, d) => total + d.observations.length, 0); + const message = deletedCount === requested + ? "Observations deleted successfully" + : `Deleted ${deletedCount} of ${requested} observations.` + + (missingEntities.length ? ` Entities not found: ${missingEntities.join(", ")}` : ""); return { - content: [{ type: "text" as const, text: "Observations deleted successfully" }], - structuredContent: { success: true, message: "Observations deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } ); @@ -484,11 +583,14 @@ server.registerTool( } }, async ({ relations }) => { - await knowledgeGraphManager.deleteRelations(relations); + const { deletedCount } = await knowledgeGraphManager.deleteRelations(relations); notifyGraphUpdated(); + const message = deletedCount === relations.length + ? "Relations deleted successfully" + : `Deleted ${deletedCount} of ${relations.length} relations. The rest matched nothing.`; return { - content: [{ type: "text" as const, text: "Relations deleted successfully" }], - structuredContent: { success: true, message: "Relations deleted successfully" } + content: [{ type: "text" as const, text: message }], + structuredContent: { success: true, message } }; } ); @@ -520,6 +622,13 @@ server.registerTool( } ); +export const SEARCH_QUERY_MAX_LENGTH = 2048; + +export const SearchNodesQuerySchema = z + .string() + .max(SEARCH_QUERY_MAX_LENGTH) + .describe("The search query to match against entity names, types, and observation content"); + // Register search_nodes tool server.registerTool( "search_nodes", @@ -527,7 +636,7 @@ server.registerTool( title: "Search Nodes", description: "Search for nodes in the knowledge graph based on a query", inputSchema: { - query: z.string().describe("The search query to match against entity names, types, and observation content") + query: SearchNodesQuerySchema }, outputSchema: { entities: z.array(EntitySchema), diff --git a/src/memory/package.json b/src/memory/package.json index 4fdcd3f9a4..ea10979f1b 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -25,7 +25,8 @@ "test": "vitest run --coverage" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.30.0" + "@modelcontextprotocol/sdk": "^1.30.0", + "zod": "^4.0.0" }, "devDependencies": { "@types/node": "^22", diff --git a/src/sequentialthinking/package.json b/src/sequentialthinking/package.json index 03fbb2b361..a668a47fd9 100644 --- a/src/sequentialthinking/package.json +++ b/src/sequentialthinking/package.json @@ -27,7 +27,8 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2" + "yargs": "^17.7.2", + "zod": "^4.0.0" }, "devDependencies": { "@types/node": "^22", @@ -37,4 +38,4 @@ "typescript": "^5.3.3", "vitest": "^4.1.8" } -} \ No newline at end of file +}