diff --git a/src/memory/__tests__/cross-process-lock.test.ts b/src/memory/__tests__/cross-process-lock.test.ts new file mode 100644 index 0000000000..0d8765d4a0 --- /dev/null +++ b/src/memory/__tests__/cross-process-lock.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { KnowledgeGraphManager } from '../index.js'; + +/** + * Regression tests for cross-process exclusion (#1819, #3286). + * + * The in-process mutation queue (#4555) serialises one instance's mutations, + * but the server is stdio-only, so every client is its own process with its + * own queue. Two instances on one file are a faithful stand-in: nothing but + * the sidecar lock file separates them, exactly as with two processes. + */ +describe('KnowledgeGraphManager cross-process exclusion', () => { + let testDir: string; + let testFilePath: string; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-memory-xproc-')); + testFilePath = path.join(testDir, 'memory.jsonl'); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + const entity = (name: string) => ({ name, entityType: 'thing', observations: [] as string[] }); + + it('keeps every entity when two instances write the same file concurrently', async () => { + const a = new KnowledgeGraphManager(testFilePath); + const b = new KnowledgeGraphManager(testFilePath); + await Promise.all( + Array.from({ length: 20 }, (_, i) => (i % 2 ? b : a).createEntities([entity(`e${i}`)])), + ); + const names = (await a.readGraph()).entities.map(e => e.name).sort(); + expect(names).toEqual(Array.from({ length: 20 }, (_, i) => `e${i}`).sort()); + }); + + it('removes its lock file after each mutation', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([entity('x')]); + await expect(fs.stat(`${testFilePath}.lock`)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('reclaims a lock left behind by a crashed process', async () => { + await fs.mkdir(testDir, { recursive: true }); + await fs.writeFile(`${testFilePath}.lock`, ''); + const old = new Date(Date.now() - 60_000); + await fs.utimes(`${testFilePath}.lock`, old, old); + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([entity('x')]); + expect((await manager.readGraph()).entities).toHaveLength(1); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index d9f814877b..214fd2f5ad 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -95,7 +95,8 @@ export class KnowledgeGraphManager { private mutationQueue: Promise = Promise.resolve(); private async withLock(operation: () => Promise): Promise { - const result = this.mutationQueue.then(operation, operation); + const guarded = () => this.withFileLock(operation); + const result = this.mutationQueue.then(guarded, guarded); // 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`. @@ -106,6 +107,56 @@ export class KnowledgeGraphManager { return result; } + // Cross-process exclusion. The queue above serialises this instance's + // mutations, but the server is stdio-only, so every client is its own + // process: two editor windows or two git worktrees sharing MEMORY_FILE_PATH + // each run their own queue and still overwrite each other's load→mutate→save + // (#1819, #3286). Measured on this commit: 20 writes split across two + // processes keep exactly 10 — each process serialises its own half and the + // last one to write the file wins. A sidecar lock file created with O_EXCL is + // atomic on POSIX and Windows and needs no dependency. A lock older than + // LOCK_STALE_MS is treated as left behind by a crashed process. + private static readonly LOCK_STALE_MS = 30_000; + private static readonly LOCK_RETRY_MS = 15; + private static readonly LOCK_TIMEOUT_MS = 10_000; + + private get lockFilePath(): string { + return `${this.memoryFilePath}.lock`; + } + + private async withFileLock(operation: () => Promise): Promise { + await fs.mkdir(path.dirname(this.memoryFilePath), { recursive: true }).catch(() => {}); + const deadline = Date.now() + KnowledgeGraphManager.LOCK_TIMEOUT_MS; + for (;;) { + let handle: fs.FileHandle; + try { + handle = await fs.open(this.lockFilePath, 'wx'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + try { + const stat = await fs.stat(this.lockFilePath); + if (Date.now() - stat.mtimeMs > KnowledgeGraphManager.LOCK_STALE_MS) { + await fs.unlink(this.lockFilePath).catch(() => {}); + continue; + } + } catch { + continue; // the holder released between open and stat; retry now + } + if (Date.now() > deadline) { + throw new Error(`Timed out waiting for lock ${this.lockFilePath}`); + } + await new Promise(resolve => setTimeout(resolve, KnowledgeGraphManager.LOCK_RETRY_MS)); + continue; + } + try { + return await operation(); + } finally { + await handle.close().catch(() => {}); + await fs.unlink(this.lockFilePath).catch(() => {}); + } + } + } + private async loadGraph(): Promise { try { const data = await fs.readFile(this.memoryFilePath, "utf-8");