From 6645bf71fa4f444c3f7d0750ec8ec137c86ee27e Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 16:56:16 +0800 Subject: [PATCH 01/12] feat(memory): add file locking to support multi-instance - Add proper-lockfile dependency for cross-process file locking - Wrap all write operations (create/delete entities/relations/observations) in withLock() to ensure atomic read-modify-write - Add chaos test validating lock consistency under 10k concurrent writes --- package-lock.json | 57 ++++++++- src/memory/__tests__/knowledge-graph.test.ts | 46 +++++++ src/memory/index.ts | 128 ++++++++++++------- src/memory/package.json | 6 +- 4 files changed, 190 insertions(+), 47 deletions(-) diff --git a/package-lock.json b/package-lock.json index 36333dcead..d2744fb7c1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1120,6 +1120,16 @@ "undici-types": "~6.20.0" } }, + "node_modules/@types/proper-lockfile": { + "version": "4.1.4", + "resolved": "https://registry.npmjs.org/@types/proper-lockfile/-/proper-lockfile-4.1.4.tgz", + "integrity": "sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/retry": "*" + } + }, "node_modules/@types/qs": { "version": "6.9.17", "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.17.tgz", @@ -1132,6 +1142,13 @@ "integrity": "sha512-hKormJbkJqzQGhziax5PItDUTMAM9uE2XXQmM37dyd4hVM+5aVl7oVxMVUiVQn2oCQFN/LKCZdvSM0pFRqbSmQ==", "dev": true }, + "node_modules/@types/retry": { + "version": "0.12.5", + "resolved": "https://registry.npmjs.org/@types/retry/-/retry-0.12.5.tgz", + "integrity": "sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/send": { "version": "0.17.4", "resolved": "https://registry.npmjs.org/@types/send/-/send-0.17.4.tgz", @@ -1877,6 +1894,7 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", + "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -2113,6 +2131,12 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/graceful-fs": { + "version": "4.2.11", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", + "integrity": "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==", + "license": "ISC" + }, "node_modules/has-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", @@ -2727,6 +2751,23 @@ "integrity": "sha512-3ouUOpQhtgrbOa17J7+uxOTpITYWaGP7/AhoR3+A+/1e9skrzelGi/dXzEYyvbxubEF6Wn2ypscTKiKJFFn1ag==", "license": "MIT" }, + "node_modules/proper-lockfile": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/proper-lockfile/-/proper-lockfile-4.1.2.tgz", + "integrity": "sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==", + "license": "MIT", + "dependencies": { + "graceful-fs": "^4.2.4", + "retry": "^0.12.0", + "signal-exit": "^3.0.2" + } + }, + "node_modules/proper-lockfile/node_modules/signal-exit": { + "version": "3.0.7", + "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", + "integrity": "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==", + "license": "ISC" + }, "node_modules/proxy-addr": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", @@ -2873,6 +2914,15 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/retry": { + "version": "0.12.0", + "resolved": "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz", + "integrity": "sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==", + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, "node_modules/rollup": { "version": "4.52.5", "resolved": "https://registry.npmjs.org/rollup/-/rollup-4.52.5.tgz", @@ -3436,6 +3486,7 @@ "integrity": "sha512-j3lYzGC3P+B5Yfy/pfKNgVEg4+UtcIJcVRt2cDjIOmhLourAqPqf8P7acgxeiSgUB7E3p2P8/3gNIgDLpwzs4g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -3519,6 +3570,7 @@ "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", @@ -3688,6 +3740,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -3921,13 +3974,15 @@ "version": "0.6.3", "license": "MIT", "dependencies": { - "@modelcontextprotocol/sdk": "^1.25.2" + "@modelcontextprotocol/sdk": "^1.25.2", + "proper-lockfile": "^4.1.2" }, "bin": { "mcp-server-memory": "dist/index.js" }, "devDependencies": { "@types/node": "^22", + "@types/proper-lockfile": "^4.1.4", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", "typescript": "^5.6.2", diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index 7edab5e42c..867b168b97 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -480,4 +480,50 @@ describe('KnowledgeGraphManager', () => { expect(result.relations[0]).not.toHaveProperty('type'); }); }); + + describe('saveGraph locking', () => { + it('should guarantee consistency: succeeded operations must be in file', async () => { + const chaosManager = new KnowledgeGraphManager(testFilePath, { retries: 100, minTimeout: 10, maxTimeout: 50 }); + const totalOperations = 10000; + const promises: Promise[] = []; + + for (let i = 0; i < totalOperations; i++) { + const randomName = `Entity_${Math.random().toString(36).substring(2)}`; + promises.push( + chaosManager.createEntities([ + { name: randomName, entityType: 'test', observations: [] } + ]) + ); + } + + const results = await Promise.allSettled(promises); + + const succeeded = results.filter(r => r.status === 'fulfilled') as PromiseFulfilledResult[]; + const failed = results.filter(r => r.status === 'rejected') as PromiseRejectedResult[]; + + // Collect succeeded entity names + const succeededNames = new Set( + succeeded.flatMap(r => r.value.map(e => e.name)) + ); + + // Read file + const graph = await chaosManager.readGraph(); + const fileNames = new Set(graph.entities.map(e => e.name)); + + // Verify: succeeded entities must be in file + succeededNames.forEach(name => { + expect(fileNames.has(name)).toBe(true); + }); + + // File entity count should equal succeeded count + expect(graph.entities.length).toBe(succeededNames.size); + + // Failed operations should contain correct error message + failed.forEach(f => { + expect(f.reason.message).toContain('Lock operation failed:'); + }); + + console.log(`Chaos test: ${succeeded.length} succeeded, ${failed.length} failed`); + }, 60000); + }); }); diff --git a/src/memory/index.ts b/src/memory/index.ts index 600a7edcc8..8d5f85596a 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -5,6 +5,7 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import { z } from "zod"; import { promises as fs } from 'fs'; import path from 'path'; +import lockfile from 'proper-lockfile'; import { fileURLToPath } from 'url'; // Define memory file path using environment variable with fallback @@ -65,8 +66,35 @@ export interface KnowledgeGraph { } // The KnowledgeGraphManager class contains all operations to interact with the knowledge graph +// Default: 50 + 100 + 200 + 400 + 800 + 1000*55 ≈ 56.5s max wait time +const DEFAULT_LOCK_RETRIES = { + retries: 60, + minTimeout: 50, + maxTimeout: 1000, +}; + export class KnowledgeGraphManager { - constructor(private memoryFilePath: string) {} + private lockRetries: object; + + constructor(private memoryFilePath: string, lockRetries: object = DEFAULT_LOCK_RETRIES) { + this.lockRetries = lockRetries; + } + + private async withLock(fn: () => Promise): Promise { + return lockfile.lock(this.memoryFilePath, { + stale: 10000, + retries: this.lockRetries, + realpath: false, + }) + .then(async (release) => { + const result = await fn(); + await release(); + return result; + }) + .catch((e) => { + throw new Error(`Lock operation failed: ${e.message}`); + }); + } private async loadGraph(): Promise { try { @@ -117,66 +145,78 @@ 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 => !graph.entities.some(existingEntity => existingEntity.name === e.name)); + graph.entities.push(...newEntities); + await this.saveGraph(graph); + return newEntities; + }); } async createRelations(relations: Relation[]): Promise { - const graph = await this.loadGraph(); - 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; + return this.withLock(async () => { + const graph = await this.loadGraph(); + 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); + return this.withLock(async () => { + 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 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)); - } + return this.withLock(async () => { + 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)); + } + }); + await this.saveGraph(graph); }); - 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); + return this.withLock(async () => { + 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 readGraph(): Promise { diff --git a/src/memory/package.json b/src/memory/package.json index 29b6605250..629d4fc52f 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -25,13 +25,15 @@ "test": "vitest run --coverage" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.25.2" + "@modelcontextprotocol/sdk": "^1.25.2", + "proper-lockfile": "^4.1.2" }, "devDependencies": { "@types/node": "^22", + "@types/proper-lockfile": "^4.1.4", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", "typescript": "^5.6.2", "vitest": "^2.1.8" } -} \ No newline at end of file +} From 6764412202655373958c7308a3badaecaaa5f313 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 18:13:52 +0800 Subject: [PATCH 02/12] test(memory): add multi-process lock test to validate cross-process file locking Spawns 5 worker processes each writing 2000 entities to verify that proper-lockfile correctly prevents data loss across separate processes. This simulates the real-world scenario where multiple MCP clients spawn separate mcp-server-memory instances via stdio. --- .../__tests__/multi-process-lock.test.ts | 133 ++++++++++++++++++ src/memory/__tests__/multi-process-worker.ts | 59 ++++++++ 2 files changed, 192 insertions(+) create mode 100644 src/memory/__tests__/multi-process-lock.test.ts create mode 100644 src/memory/__tests__/multi-process-worker.ts diff --git a/src/memory/__tests__/multi-process-lock.test.ts b/src/memory/__tests__/multi-process-lock.test.ts new file mode 100644 index 0000000000..b3a8ef91be --- /dev/null +++ b/src/memory/__tests__/multi-process-lock.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { spawn } from 'child_process'; +import { KnowledgeGraphManager } from '../index.js'; + +/** + * Multi-process lock test. + * + * This test validates that proper-lockfile correctly prevents data loss + * when multiple processes (simulating multiple MCP server instances) + * concurrently write to the same memory file. + * + * This is the critical scenario that in-memory locks (like PR #3060) cannot handle, + * because each MCP server instance spawned via stdio runs as a separate process. + */ +describe('Multi-process file locking', () => { + let testFilePath: string; + + beforeEach(async () => { + testFilePath = path.join( + path.dirname(fileURLToPath(import.meta.url)), + `test-multi-process-${Date.now()}.jsonl` + ); + // Create empty file for locking (proper-lockfile requires file to exist) + await fs.writeFile(testFilePath, ''); + }); + + afterEach(async () => { + try { + await fs.unlink(testFilePath); + } catch { + // Ignore errors if file doesn't exist + } + // Clean up lock file if exists + try { + await fs.unlink(`${testFilePath}.lock`); + } catch { + // Ignore + } + }); + + it('should guarantee consistency: succeeded operations must be in file (5 processes x 2000 writes)', async () => { + const NUM_PROCESSES = 5; + const WRITES_PER_PROCESS = 2000; + + const workerPath = path.join( + path.dirname(fileURLToPath(import.meta.url)), + 'multi-process-worker.ts' + ); + + // Spawn all worker processes in parallel + const workerPromises: Promise<{ workerId: string; succeededNames: string[]; failed: number }>[] = []; + + for (let i = 0; i < NUM_PROCESSES; i++) { + workerPromises.push( + new Promise((resolve, reject) => { + const child = spawn('npx', ['tsx', workerPath, testFilePath, String(i), String(WRITES_PER_PROCESS)], { + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code !== 0) { + reject(new Error(`Worker ${i} exited with code ${code}: ${stderr}`)); + return; + } + try { + const result = JSON.parse(stdout.trim()); + resolve(result); + } catch (e) { + reject(new Error(`Worker ${i} output parse error: ${stdout}`)); + } + }); + + child.on('error', (err) => { + reject(new Error(`Worker ${i} spawn error: ${err.message}`)); + }); + }) + ); + } + + // Wait for all workers to complete + const results = await Promise.all(workerPromises); + + // Collect all succeeded entity names + const succeededNames = new Set( + results.flatMap(r => r.succeededNames) + ); + + const totalFailed = results.reduce((sum, r) => sum + r.failed, 0); + + console.log(`\n=== Multi-process Lock Test Results ===`); + console.log(`Processes: ${NUM_PROCESSES}`); + console.log(`Writes per process: ${WRITES_PER_PROCESS}`); + console.log(`Total succeeded: ${succeededNames.size}`); + console.log(`Total failed: ${totalFailed}`); + + // Read the final file + const manager = new KnowledgeGraphManager(testFilePath); + const graph = await manager.readGraph(); + const fileNames = new Set(graph.entities.map(e => e.name)); + + console.log(`Entities in file: ${graph.entities.length}`); + + // Verify: succeeded entities must be in file + succeededNames.forEach(name => { + expect(fileNames.has(name)).toBe(true); + }); + + // File entity count should equal succeeded count + expect(graph.entities.length).toBe(succeededNames.size); + + // Log per-worker stats + console.log(`\nPer-worker breakdown:`); + for (const r of results) { + console.log(` Worker ${r.workerId}: ${r.succeededNames.length} succeeded, ${r.failed} failed`); + } + + console.log(`\n✓ File integrity verified: all ${succeededNames.size} succeeded writes are in the file`); + }, 300000); // 5 minute timeout for 10k writes +}); diff --git a/src/memory/__tests__/multi-process-worker.ts b/src/memory/__tests__/multi-process-worker.ts new file mode 100644 index 0000000000..582351c224 --- /dev/null +++ b/src/memory/__tests__/multi-process-worker.ts @@ -0,0 +1,59 @@ +/** + * Worker script for multi-process lock testing. + * Spawned by multi-process-lock.test.ts to simulate multiple MCP server instances. + * + * Usage: npx tsx multi-process-worker.ts + */ + +import { KnowledgeGraphManager } from '../index.js'; + +const [,, memoryFilePath, workerId, numWritesStr] = process.argv; + +if (!memoryFilePath || !workerId || !numWritesStr) { + console.error('Usage: npx tsx multi-process-worker.ts '); + process.exit(1); +} + +const numWrites = parseInt(numWritesStr, 10); + +async function main() { + // Low retry count to speed up test - failures are expected under heavy contention + const manager = new KnowledgeGraphManager(memoryFilePath, { + retries: { + retries: 10, + minTimeout: 10, + factor: 1.5, + maxTimeout: 50, + }, + }); + + const succeededNames: string[] = []; + let failed = 0; + + for (let i = 0; i < numWrites; i++) { + const entityName = `Worker${workerId}_Entity${i}`; + try { + const created = await manager.createEntities([ + { + name: entityName, + entityType: 'test', + observations: [`Created by worker ${workerId}`], + }, + ]); + // Only count as succeeded if entity was actually created + if (created.length > 0) { + succeededNames.push(entityName); + } + } catch { + failed++; + } + } + + // Output result as JSON for parent process to parse + console.log(JSON.stringify({ workerId, succeededNames, failed })); +} + +main().catch((error) => { + console.error(`Worker ${workerId} crashed:`, error); + process.exit(1); +}); From 5fd0be34aa3536ffe2ef52be1cd0a476274cacca Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 18:14:02 +0800 Subject: [PATCH 03/12] refactor(memory): optimize lock config for NFS compatibility - Increase stale timeout to 60s (safe for NFS acregmax) - Add 10s update heartbeat for lock freshness - Refactor lockOptions to use spread for easier customization - Update test to match new lockOptions structure --- src/memory/__tests__/knowledge-graph.test.ts | 2 +- src/memory/index.ts | 38 +++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index 867b168b97..31ef996362 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -483,7 +483,7 @@ describe('KnowledgeGraphManager', () => { describe('saveGraph locking', () => { it('should guarantee consistency: succeeded operations must be in file', async () => { - const chaosManager = new KnowledgeGraphManager(testFilePath, { retries: 100, minTimeout: 10, maxTimeout: 50 }); + const chaosManager = new KnowledgeGraphManager(testFilePath, { retries: { retries: 100, minTimeout: 10, maxTimeout: 50 } }); const totalOperations = 10000; const promises: Promise[] = []; diff --git a/src/memory/index.ts b/src/memory/index.ts index 8d5f85596a..e09b4ec4b1 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -66,26 +66,38 @@ export interface KnowledgeGraph { } // The KnowledgeGraphManager class contains all operations to interact with the knowledge graph -// Default: 50 + 100 + 200 + 400 + 800 + 1000*55 ≈ 56.5s max wait time -const DEFAULT_LOCK_RETRIES = { - retries: 60, - minTimeout: 50, - maxTimeout: 1000, +// Lock configuration optimized for both local disk and NFS/network file systems +// +// For NFS: stale must be > acregmax (typically 60s) to avoid false stale detection +// due to attribute caching. Setting stale=60000ms ensures that even if another +// process sees a 50s-old cached mtime, it won't incorrectly assume the lock is stale. +// +// For local disk: These settings work perfectly - the longer stale timeout just means +// a slightly longer wait if a process actually crashes (60s vs 10s), which is acceptable. +// +// Retry strategy: minTimeout=50ms allows fast acquisition on local disk, +// exponential backoff handles NFS latency. +// Max wait: 50 + 100 + 200 + 400 + 800 + 1600 + 3200 + 6400 + 12800 + 25600 ≈ 51s +const DEFAULT_LOCK_OPTIONS = { + stale: 60000, // 60s - safe for NFS acregmax (default 60s) + update: 10000, // 10s heartbeat - ensures lock freshness even with NFS cache delays + retries: { + retries: 10, + minTimeout: 50, + factor: 2, + }, + realpath: false, }; export class KnowledgeGraphManager { - private lockRetries: object; + private lockOptions: object; - constructor(private memoryFilePath: string, lockRetries: object = DEFAULT_LOCK_RETRIES) { - this.lockRetries = lockRetries; + constructor(private memoryFilePath: string, lockOptions: object = {}) { + this.lockOptions = { ...DEFAULT_LOCK_OPTIONS, ...lockOptions }; } private async withLock(fn: () => Promise): Promise { - return lockfile.lock(this.memoryFilePath, { - stale: 10000, - retries: this.lockRetries, - realpath: false, - }) + return lockfile.lock(this.memoryFilePath, this.lockOptions) .then(async (release) => { const result = await fn(); await release(); From fc12005bfff96d895ee5ecb551cea2b2962751b9 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 18:22:26 +0800 Subject: [PATCH 04/12] test(memory): merge multi-process-worker into lock test - Move worker logic into multi-process-lock.test.ts to make the test self-contained - Remove separate worker script - Add tsx to devDependencies to support running the worker process - Fix vitest import issue in worker process --- package-lock.json | 528 ++++++++++++++++++ .../__tests__/multi-process-lock.test.ts | 304 ++++++---- src/memory/__tests__/multi-process-worker.ts | 59 -- src/memory/package.json | 1 + 4 files changed, 721 insertions(+), 171 deletions(-) delete mode 100644 src/memory/__tests__/multi-process-worker.ts diff --git a/package-lock.json b/package-lock.json index d2744fb7c1..cd936ad3b7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -378,6 +378,23 @@ "node": ">=12" } }, + "node_modules/@esbuild/netbsd-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/netbsd-arm64/-/netbsd-arm64-0.27.2.tgz", + "integrity": "sha512-Kj6DiBlwXrPsCRDeRvGAUb/LNrBASrfqAIok+xB0LxK8CHqxZ037viF13ugfsIpePH93mX7xfJp97cyDuTZ3cw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=18" + } + }, "node_modules/@esbuild/netbsd-x64": { "version": "0.21.5", "resolved": "https://registry.npmjs.org/@esbuild/netbsd-x64/-/netbsd-x64-0.21.5.tgz", @@ -395,6 +412,23 @@ "node": ">=12" } }, + "node_modules/@esbuild/openbsd-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/openbsd-arm64/-/openbsd-arm64-0.27.2.tgz", + "integrity": "sha512-DNIHH2BPQ5551A7oSHD0CKbwIA/Ox7+78/AWkbS5QoRzaqlev2uFayfSxq68EkonB+IKjiuxBFoV8ESJy8bOHA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=18" + } + }, "node_modules/@esbuild/openbsd-x64": { "version": "0.21.5", "resolved": "https://registry.npmjs.org/@esbuild/openbsd-x64/-/openbsd-x64-0.21.5.tgz", @@ -412,6 +446,23 @@ "node": ">=12" } }, + "node_modules/@esbuild/openharmony-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/openharmony-arm64/-/openharmony-arm64-0.27.2.tgz", + "integrity": "sha512-LRBbCmiU51IXfeXk59csuX/aSaToeG7w48nMwA6049Y4J4+VbWALAuXcs+qcD04rHDuSCSRKdmY63sruDS5qag==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openharmony" + ], + "engines": { + "node": ">=18" + } + }, "node_modules/@esbuild/sunos-x64": { "version": "0.21.5", "resolved": "https://registry.npmjs.org/@esbuild/sunos-x64/-/sunos-x64-0.21.5.tgz", @@ -2099,6 +2150,19 @@ "node": ">= 0.4" } }, + "node_modules/get-tsconfig": { + "version": "4.13.1", + "resolved": "https://registry.npmjs.org/get-tsconfig/-/get-tsconfig-4.13.1.tgz", + "integrity": "sha512-EoY1N2xCn44xU6750Sx7OjOIT59FkmstNc3X6y5xpz7D5cBtZRe/3pSlTkDJgqsOk3WwZPkWfonhhUJfttQo3w==", + "dev": true, + "license": "MIT", + "dependencies": { + "resolve-pkg-maps": "^1.0.0" + }, + "funding": { + "url": "https://github.com/privatenumber/get-tsconfig?sponsor=1" + } + }, "node_modules/glob": { "version": "10.5.0", "resolved": "https://registry.npmjs.org/glob/-/glob-10.5.0.tgz", @@ -2914,6 +2978,16 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/resolve-pkg-maps": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/resolve-pkg-maps/-/resolve-pkg-maps-1.0.0.tgz", + "integrity": "sha512-seS2Tj26TBVOC2NIc2rOe2y2ZO7efxITtLZcGSOnHHNOQ7CkiUBfw0Iw2ck6xkIhPwLhKNLS8BO+hEpngQlqzw==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/privatenumber/resolve-pkg-maps?sponsor=1" + } + }, "node_modules/retry": { "version": "0.12.0", "resolved": "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz", @@ -3423,6 +3497,459 @@ "node": ">=0.6" } }, + "node_modules/tsx": { + "version": "4.21.0", + "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.21.0.tgz", + "integrity": "sha512-5C1sg4USs1lfG0GFb2RLXsdpXqBSEhAaA/0kPL01wxzpMqLILNxIxIOKiILz+cdg/pLnOUxFYOR5yhHU666wbw==", + "dev": true, + "license": "MIT", + "dependencies": { + "esbuild": "~0.27.0", + "get-tsconfig": "^4.7.5" + }, + "bin": { + "tsx": "dist/cli.mjs" + }, + "engines": { + "node": ">=18.0.0" + }, + "optionalDependencies": { + "fsevents": "~2.3.3" + } + }, + "node_modules/tsx/node_modules/@esbuild/aix-ppc64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.27.2.tgz", + "integrity": "sha512-GZMB+a0mOMZs4MpDbj8RJp4cw+w1WV5NYD6xzgvzUJ5Ek2jerwfO2eADyI6ExDSUED+1X8aMbegahsJi+8mgpw==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "aix" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/android-arm": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/android-arm/-/android-arm-0.27.2.tgz", + "integrity": "sha512-DVNI8jlPa7Ujbr1yjU2PfUSRtAUZPG9I1RwW4F4xFB1Imiu2on0ADiI/c3td+KmDtVKNbi+nffGDQMfcIMkwIA==", + "cpu": [ + "arm" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/android-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/android-arm64/-/android-arm64-0.27.2.tgz", + "integrity": "sha512-pvz8ZZ7ot/RBphf8fv60ljmaoydPU12VuXHImtAs0XhLLw+EXBi2BLe3OYSBslR4rryHvweW5gmkKFwTiFy6KA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/android-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/android-x64/-/android-x64-0.27.2.tgz", + "integrity": "sha512-z8Ank4Byh4TJJOh4wpz8g2vDy75zFL0TlZlkUkEwYXuPSgX8yzep596n6mT7905kA9uHZsf/o2OJZubl2l3M7A==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "android" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/darwin-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/darwin-arm64/-/darwin-arm64-0.27.2.tgz", + "integrity": "sha512-davCD2Zc80nzDVRwXTcQP/28fiJbcOwvdolL0sOiOsbwBa72kegmVU0Wrh1MYrbuCL98Omp5dVhQFWRKR2ZAlg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/darwin-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/darwin-x64/-/darwin-x64-0.27.2.tgz", + "integrity": "sha512-ZxtijOmlQCBWGwbVmwOF/UCzuGIbUkqB1faQRf5akQmxRJ1ujusWsb3CVfk/9iZKr2L5SMU5wPBi1UWbvL+VQA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/freebsd-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/freebsd-arm64/-/freebsd-arm64-0.27.2.tgz", + "integrity": "sha512-lS/9CN+rgqQ9czogxlMcBMGd+l8Q3Nj1MFQwBZJyoEKI50XGxwuzznYdwcav6lpOGv5BqaZXqvBSiB/kJ5op+g==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/freebsd-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/freebsd-x64/-/freebsd-x64-0.27.2.tgz", + "integrity": "sha512-tAfqtNYb4YgPnJlEFu4c212HYjQWSO/w/h/lQaBK7RbwGIkBOuNKQI9tqWzx7Wtp7bTPaGC6MJvWI608P3wXYA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-arm": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-arm/-/linux-arm-0.27.2.tgz", + "integrity": "sha512-vWfq4GaIMP9AIe4yj1ZUW18RDhx6EPQKjwe7n8BbIecFtCQG4CfHGaHuh7fdfq+y3LIA2vGS/o9ZBGVxIDi9hw==", + "cpu": [ + "arm" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-arm64/-/linux-arm64-0.27.2.tgz", + "integrity": "sha512-hYxN8pr66NsCCiRFkHUAsxylNOcAQaxSSkHMMjcpx0si13t1LHFphxJZUiGwojB1a/Hd5OiPIqDdXONia6bhTw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-ia32": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-ia32/-/linux-ia32-0.27.2.tgz", + "integrity": "sha512-MJt5BRRSScPDwG2hLelYhAAKh9imjHK5+NE/tvnRLbIqUWa+0E9N4WNMjmp/kXXPHZGqPLxggwVhz7QP8CTR8w==", + "cpu": [ + "ia32" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-loong64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-loong64/-/linux-loong64-0.27.2.tgz", + "integrity": "sha512-lugyF1atnAT463aO6KPshVCJK5NgRnU4yb3FUumyVz+cGvZbontBgzeGFO1nF+dPueHD367a2ZXe1NtUkAjOtg==", + "cpu": [ + "loong64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-mips64el": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-mips64el/-/linux-mips64el-0.27.2.tgz", + "integrity": "sha512-nlP2I6ArEBewvJ2gjrrkESEZkB5mIoaTswuqNFRv/WYd+ATtUpe9Y09RnJvgvdag7he0OWgEZWhviS1OTOKixw==", + "cpu": [ + "mips64el" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-ppc64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-ppc64/-/linux-ppc64-0.27.2.tgz", + "integrity": "sha512-C92gnpey7tUQONqg1n6dKVbx3vphKtTHJaNG2Ok9lGwbZil6DrfyecMsp9CrmXGQJmZ7iiVXvvZH6Ml5hL6XdQ==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-riscv64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-riscv64/-/linux-riscv64-0.27.2.tgz", + "integrity": "sha512-B5BOmojNtUyN8AXlK0QJyvjEZkWwy/FKvakkTDCziX95AowLZKR6aCDhG7LeF7uMCXEJqwa8Bejz5LTPYm8AvA==", + "cpu": [ + "riscv64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-s390x": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-s390x/-/linux-s390x-0.27.2.tgz", + "integrity": "sha512-p4bm9+wsPwup5Z8f4EpfN63qNagQ47Ua2znaqGH6bqLlmJ4bx97Y9JdqxgGZ6Y8xVTixUnEkoKSHcpRlDnNr5w==", + "cpu": [ + "s390x" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/linux-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/linux-x64/-/linux-x64-0.27.2.tgz", + "integrity": "sha512-uwp2Tip5aPmH+NRUwTcfLb+W32WXjpFejTIOWZFw/v7/KnpCDKG66u4DLcurQpiYTiYwQ9B7KOeMJvLCu/OvbA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/netbsd-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/netbsd-x64/-/netbsd-x64-0.27.2.tgz", + "integrity": "sha512-HwGDZ0VLVBY3Y+Nw0JexZy9o/nUAWq9MlV7cahpaXKW6TOzfVno3y3/M8Ga8u8Yr7GldLOov27xiCnqRZf0tCA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/openbsd-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/openbsd-x64/-/openbsd-x64-0.27.2.tgz", + "integrity": "sha512-/it7w9Nb7+0KFIzjalNJVR5bOzA9Vay+yIPLVHfIQYG/j+j9VTH84aNB8ExGKPU4AzfaEvN9/V4HV+F+vo8OEg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/sunos-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/sunos-x64/-/sunos-x64-0.27.2.tgz", + "integrity": "sha512-kMtx1yqJHTmqaqHPAzKCAkDaKsffmXkPHThSfRwZGyuqyIeBvf08KSsYXl+abf5HDAPMJIPnbBfXvP2ZC2TfHg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "sunos" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/win32-arm64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/win32-arm64/-/win32-arm64-0.27.2.tgz", + "integrity": "sha512-Yaf78O/B3Kkh+nKABUF++bvJv5Ijoy9AN1ww904rOXZFLWVc5OLOfL56W+C8F9xn5JQZa3UX6m+IktJnIb1Jjg==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/win32-ia32": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/win32-ia32/-/win32-ia32-0.27.2.tgz", + "integrity": "sha512-Iuws0kxo4yusk7sw70Xa2E2imZU5HoixzxfGCdxwBdhiDgt9vX9VUCBhqcwY7/uh//78A1hMkkROMJq9l27oLQ==", + "cpu": [ + "ia32" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/@esbuild/win32-x64": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/@esbuild/win32-x64/-/win32-x64-0.27.2.tgz", + "integrity": "sha512-sRdU18mcKf7F+YgheI/zGf5alZatMUTKj/jNS6l744f9u3WFu4v7twcUI9vu4mknF4Y9aDlblIie0IM+5xxaqQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/tsx/node_modules/esbuild": { + "version": "0.27.2", + "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.27.2.tgz", + "integrity": "sha512-HyNQImnsOC7X9PMNaCIeAm4ISCQXs5a5YasTXVliKv4uuBo1dKrG0A+uQS8M5eXjVMnLg3WgXaKvprHlFJQffw==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "bin": { + "esbuild": "bin/esbuild" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "@esbuild/aix-ppc64": "0.27.2", + "@esbuild/android-arm": "0.27.2", + "@esbuild/android-arm64": "0.27.2", + "@esbuild/android-x64": "0.27.2", + "@esbuild/darwin-arm64": "0.27.2", + "@esbuild/darwin-x64": "0.27.2", + "@esbuild/freebsd-arm64": "0.27.2", + "@esbuild/freebsd-x64": "0.27.2", + "@esbuild/linux-arm": "0.27.2", + "@esbuild/linux-arm64": "0.27.2", + "@esbuild/linux-ia32": "0.27.2", + "@esbuild/linux-loong64": "0.27.2", + "@esbuild/linux-mips64el": "0.27.2", + "@esbuild/linux-ppc64": "0.27.2", + "@esbuild/linux-riscv64": "0.27.2", + "@esbuild/linux-s390x": "0.27.2", + "@esbuild/linux-x64": "0.27.2", + "@esbuild/netbsd-arm64": "0.27.2", + "@esbuild/netbsd-x64": "0.27.2", + "@esbuild/openbsd-arm64": "0.27.2", + "@esbuild/openbsd-x64": "0.27.2", + "@esbuild/openharmony-arm64": "0.27.2", + "@esbuild/sunos-x64": "0.27.2", + "@esbuild/win32-arm64": "0.27.2", + "@esbuild/win32-ia32": "0.27.2", + "@esbuild/win32-x64": "0.27.2" + } + }, "node_modules/type-is": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/type-is/-/type-is-2.0.1.tgz", @@ -3985,6 +4512,7 @@ "@types/proper-lockfile": "^4.1.4", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", + "tsx": "^4.21.0", "typescript": "^5.6.2", "vitest": "^2.1.8" } diff --git a/src/memory/__tests__/multi-process-lock.test.ts b/src/memory/__tests__/multi-process-lock.test.ts index b3a8ef91be..85d294f97b 100644 --- a/src/memory/__tests__/multi-process-lock.test.ts +++ b/src/memory/__tests__/multi-process-lock.test.ts @@ -1,133 +1,213 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +/// import { promises as fs } from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; import { spawn } from 'child_process'; import { KnowledgeGraphManager } from '../index.js'; -/** - * Multi-process lock test. - * - * This test validates that proper-lockfile correctly prevents data loss - * when multiple processes (simulating multiple MCP server instances) - * concurrently write to the same memory file. - * - * This is the critical scenario that in-memory locks (like PR #3060) cannot handle, - * because each MCP server instance spawned via stdio runs as a separate process. - */ -describe('Multi-process file locking', () => { - let testFilePath: string; - - beforeEach(async () => { - testFilePath = path.join( - path.dirname(fileURLToPath(import.meta.url)), - `test-multi-process-${Date.now()}.jsonl` - ); - // Create empty file for locking (proper-lockfile requires file to exist) - await fs.writeFile(testFilePath, ''); - }); +// Check if running in worker mode +const isWorker = process.argv.includes('--worker'); - afterEach(async () => { - try { - await fs.unlink(testFilePath); - } catch { - // Ignore errors if file doesn't exist - } - // Clean up lock file if exists - try { - await fs.unlink(`${testFilePath}.lock`); - } catch { - // Ignore - } +if (isWorker) { + runWorker().catch((error) => { + console.error(`Worker crashed:`, error); + process.exit(1); }); +} else { + // Main Test Suite + describe('Multi-process file locking', () => { + let testFilePath: string; + const currentFilePath = fileURLToPath(import.meta.url); + + beforeEach(async () => { + testFilePath = path.join( + path.dirname(currentFilePath), + `test-multi-process-${Date.now()}.jsonl` + ); + // Create empty file for locking (proper-lockfile requires file to exist) + await fs.writeFile(testFilePath, ''); + }); - it('should guarantee consistency: succeeded operations must be in file (5 processes x 2000 writes)', async () => { - const NUM_PROCESSES = 5; - const WRITES_PER_PROCESS = 2000; - - const workerPath = path.join( - path.dirname(fileURLToPath(import.meta.url)), - 'multi-process-worker.ts' - ); - - // Spawn all worker processes in parallel - const workerPromises: Promise<{ workerId: string; succeededNames: string[]; failed: number }>[] = []; - - for (let i = 0; i < NUM_PROCESSES; i++) { - workerPromises.push( - new Promise((resolve, reject) => { - const child = spawn('npx', ['tsx', workerPath, testFilePath, String(i), String(WRITES_PER_PROCESS)], { - stdio: ['ignore', 'pipe', 'pipe'], - }); - - let stdout = ''; - let stderr = ''; - - child.stdout.on('data', (data) => { - stdout += data.toString(); - }); - - child.stderr.on('data', (data) => { - stderr += data.toString(); - }); - - child.on('close', (code) => { - if (code !== 0) { - reject(new Error(`Worker ${i} exited with code ${code}: ${stderr}`)); - return; - } - try { - const result = JSON.parse(stdout.trim()); - resolve(result); - } catch (e) { - reject(new Error(`Worker ${i} output parse error: ${stdout}`)); - } - }); - - child.on('error', (err) => { - reject(new Error(`Worker ${i} spawn error: ${err.message}`)); - }); - }) + afterEach(async () => { + try { + await fs.unlink(testFilePath); + } catch { + // Ignore errors if file doesn't exist + } + // Clean up lock file if exists + try { + await fs.unlink(`${testFilePath}.lock`); + } catch { + // Ignore + } + }); + + it('should guarantee consistency: succeeded operations must be in file (5 processes x 2000 writes)', async () => { + const NUM_PROCESSES = 5; + const WRITES_PER_PROCESS = 2000; + + // Spawn all worker processes in parallel + const workerPromises: Promise<{ workerId: string; succeededNames: string[]; failed: number }>[] = []; + + for (let i = 0; i < NUM_PROCESSES; i++) { + workerPromises.push( + new Promise((resolve, reject) => { + // Spawn self with --worker flag + const child = spawn('npx', ['tsx', currentFilePath, '--worker', testFilePath, String(i), String(WRITES_PER_PROCESS)], { + stdio: ['ignore', 'pipe', 'pipe'], + env: { ...process.env }, + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code !== 0) { + reject(new Error(`Worker ${i} exited with code ${code}: ${stderr}`)); + return; + } + try { + // Parse stdout, looking for the JSON object + // In case there are other logs, try to find the last line that looks like JSON + const lines = stdout.trim().split('\n'); + let result: any = null; + for (let j = lines.length - 1; j >= 0; j--) { + try { + const parsed = JSON.parse(lines[j]); + if (parsed && typeof parsed === 'object' && parsed.workerId !== undefined) { + result = parsed; + break; + } + } catch { + continue; + } + } + + if (!result) { + // If simple parse fails, try parsing the whole trimmed string + try { + result = JSON.parse(stdout.trim()); + } catch { + // ignore + } + } + + if (!result) { + reject(new Error(`Worker ${i} output parse error: ${stdout}`)); + return; + } + + resolve(result); + } catch (e: any) { + reject(new Error(`Worker ${i} output parse error: ${stdout}. Error: ${e.message}`)); + } + }); + + child.on('error', (err) => { + reject(new Error(`Worker ${i} spawn error: ${err.message}`)); + }); + }) + ); + } + + // Wait for all workers to complete + const results = await Promise.all(workerPromises); + + // Collect all succeeded entity names + const succeededNames = new Set( + results.flatMap(r => r.succeededNames) ); - } - // Wait for all workers to complete - const results = await Promise.all(workerPromises); + const totalFailed = results.reduce((sum, r) => sum + r.failed, 0); - // Collect all succeeded entity names - const succeededNames = new Set( - results.flatMap(r => r.succeededNames) - ); + console.log(`\n=== Multi-process Lock Test Results ===`); + console.log(`Processes: ${NUM_PROCESSES}`); + console.log(`Writes per process: ${WRITES_PER_PROCESS}`); + console.log(`Total succeeded: ${succeededNames.size}`); + console.log(`Total failed: ${totalFailed}`); - const totalFailed = results.reduce((sum, r) => sum + r.failed, 0); + // Read the final file + const manager = new KnowledgeGraphManager(testFilePath); + const graph = await manager.readGraph(); + const fileNames = new Set(graph.entities.map(e => e.name)); - console.log(`\n=== Multi-process Lock Test Results ===`); - console.log(`Processes: ${NUM_PROCESSES}`); - console.log(`Writes per process: ${WRITES_PER_PROCESS}`); - console.log(`Total succeeded: ${succeededNames.size}`); - console.log(`Total failed: ${totalFailed}`); + console.log(`Entities in file: ${graph.entities.length}`); - // Read the final file - const manager = new KnowledgeGraphManager(testFilePath); - const graph = await manager.readGraph(); - const fileNames = new Set(graph.entities.map(e => e.name)); + // Verify: succeeded entities must be in file + succeededNames.forEach(name => { + expect(fileNames.has(name)).toBe(true); + }); - console.log(`Entities in file: ${graph.entities.length}`); + // File entity count should equal succeeded count + expect(graph.entities.length).toBe(succeededNames.size); - // Verify: succeeded entities must be in file - succeededNames.forEach(name => { - expect(fileNames.has(name)).toBe(true); - }); + // Log per-worker stats + console.log(`\nPer-worker breakdown:`); + for (const r of results) { + console.log(` Worker ${r.workerId}: ${r.succeededNames.length} succeeded, ${r.failed} failed`); + } - // File entity count should equal succeeded count - expect(graph.entities.length).toBe(succeededNames.size); + console.log(`\n✓ File integrity verified: all ${succeededNames.size} succeeded writes are in the file`); + }, 300000); // 5 minute timeout for 10k writes + }); +} - // Log per-worker stats - console.log(`\nPer-worker breakdown:`); - for (const r of results) { - console.log(` Worker ${r.workerId}: ${r.succeededNames.length} succeeded, ${r.failed} failed`); +/** + * Worker Logic + */ +async function runWorker() { + const workerFlagIndex = process.argv.indexOf('--worker'); + const memoryFilePath = process.argv[workerFlagIndex + 1]; + const workerId = process.argv[workerFlagIndex + 2]; + const numWritesStr = process.argv[workerFlagIndex + 3]; + + if (!memoryFilePath || !workerId || !numWritesStr) { + console.error('Usage: npx tsx multi-process-lock.test.ts --worker '); + process.exit(1); + } + + const numWrites = parseInt(numWritesStr, 10); + + // Low retry count to speed up test - failures are expected under heavy contention + const manager = new KnowledgeGraphManager(memoryFilePath, { + retries: { + retries: 10, + minTimeout: 10, + factor: 1.5, + maxTimeout: 50, + }, + }); + + const succeededNames: string[] = []; + let failed = 0; + + for (let i = 0; i < numWrites; i++) { + const entityName = `Worker${workerId}_Entity${i}`; + try { + const created = await manager.createEntities([ + { + name: entityName, + entityType: 'test', + observations: [`Created by worker ${workerId}`], + }, + ]); + // Only count as succeeded if entity was actually created + if (created.length > 0) { + succeededNames.push(entityName); + } + } catch { + failed++; } + } - console.log(`\n✓ File integrity verified: all ${succeededNames.size} succeeded writes are in the file`); - }, 300000); // 5 minute timeout for 10k writes -}); + // Output result as JSON for parent process to parse + console.log(JSON.stringify({ workerId, succeededNames, failed })); +} diff --git a/src/memory/__tests__/multi-process-worker.ts b/src/memory/__tests__/multi-process-worker.ts deleted file mode 100644 index 582351c224..0000000000 --- a/src/memory/__tests__/multi-process-worker.ts +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Worker script for multi-process lock testing. - * Spawned by multi-process-lock.test.ts to simulate multiple MCP server instances. - * - * Usage: npx tsx multi-process-worker.ts - */ - -import { KnowledgeGraphManager } from '../index.js'; - -const [,, memoryFilePath, workerId, numWritesStr] = process.argv; - -if (!memoryFilePath || !workerId || !numWritesStr) { - console.error('Usage: npx tsx multi-process-worker.ts '); - process.exit(1); -} - -const numWrites = parseInt(numWritesStr, 10); - -async function main() { - // Low retry count to speed up test - failures are expected under heavy contention - const manager = new KnowledgeGraphManager(memoryFilePath, { - retries: { - retries: 10, - minTimeout: 10, - factor: 1.5, - maxTimeout: 50, - }, - }); - - const succeededNames: string[] = []; - let failed = 0; - - for (let i = 0; i < numWrites; i++) { - const entityName = `Worker${workerId}_Entity${i}`; - try { - const created = await manager.createEntities([ - { - name: entityName, - entityType: 'test', - observations: [`Created by worker ${workerId}`], - }, - ]); - // Only count as succeeded if entity was actually created - if (created.length > 0) { - succeededNames.push(entityName); - } - } catch { - failed++; - } - } - - // Output result as JSON for parent process to parse - console.log(JSON.stringify({ workerId, succeededNames, failed })); -} - -main().catch((error) => { - console.error(`Worker ${workerId} crashed:`, error); - process.exit(1); -}); diff --git a/src/memory/package.json b/src/memory/package.json index 629d4fc52f..13a5a19045 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -33,6 +33,7 @@ "@types/proper-lockfile": "^4.1.4", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", + "tsx": "^4.21.0", "typescript": "^5.6.2", "vitest": "^2.1.8" } From a2a568be3b6e1d1c46e8c1a510f21dac56c0ba32 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 20:45:07 +0800 Subject: [PATCH 05/12] refactor(test): rename chaosManager to stressTestManager for clarity --- src/memory/__tests__/knowledge-graph.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index 31ef996362..b70487d453 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -483,14 +483,14 @@ describe('KnowledgeGraphManager', () => { describe('saveGraph locking', () => { it('should guarantee consistency: succeeded operations must be in file', async () => { - const chaosManager = new KnowledgeGraphManager(testFilePath, { retries: { retries: 100, minTimeout: 10, maxTimeout: 50 } }); + const stressTestManager = new KnowledgeGraphManager(testFilePath, { retries: { retries: 100, minTimeout: 10, maxTimeout: 50 } }); const totalOperations = 10000; const promises: Promise[] = []; for (let i = 0; i < totalOperations; i++) { const randomName = `Entity_${Math.random().toString(36).substring(2)}`; promises.push( - chaosManager.createEntities([ + stressTestManager.createEntities([ { name: randomName, entityType: 'test', observations: [] } ]) ); @@ -507,7 +507,7 @@ describe('KnowledgeGraphManager', () => { ); // Read file - const graph = await chaosManager.readGraph(); + const graph = await stressTestManager.readGraph(); const fileNames = new Set(graph.entities.map(e => e.name)); // Verify: succeeded entities must be in file @@ -523,7 +523,7 @@ describe('KnowledgeGraphManager', () => { expect(f.reason.message).toContain('Lock operation failed:'); }); - console.log(`Chaos test: ${succeeded.length} succeeded, ${failed.length} failed`); + console.log(`Stress test: ${succeeded.length} succeeded, ${failed.length} failed`); }, 60000); }); }); From 9f2f95b23633a30677f6225216bc6e4907430889 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 20:56:58 +0800 Subject: [PATCH 06/12] feat(memory): use atomic rename for safer file writes --- src/memory/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/memory/index.ts b/src/memory/index.ts index e09b4ec4b1..6d8b56f30c 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -153,7 +153,10 @@ export class KnowledgeGraphManager { relationType: r.relationType })), ]; - await fs.writeFile(this.memoryFilePath, lines.join("\n")); + // Atomic write: write to temp file, then rename + const tmpPath = `${this.memoryFilePath}.tmp`; + await fs.writeFile(tmpPath, lines.join("\n")); + await fs.rename(tmpPath, this.memoryFilePath); } async createEntities(entities: Entity[]): Promise { From e3be6b3fd814e7ea58a45ff06ea89065e60fe5a9 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 21:33:43 +0800 Subject: [PATCH 07/12] refactor(memory): use write-file-atomic for atomic file writes Replace manual temp file + rename implementation with write-file-atomic package, which provides more robust atomic write handling including: - Automatic concurrent write serialization - Better temp file naming with murmur hash - Automatic cleanup on errors - Worker thread support Disabled fsync for better performance. --- package-lock.json | 25 ++++++++++++++++++++++++- src/memory/index.ts | 6 ++---- src/memory/package.json | 3 ++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index cd936ad3b7..e59e1ba2fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2288,6 +2288,15 @@ "integrity": "sha512-XXOFtyqDjNDAQxVfYxuF7g9Il/IbWmmlQg2MYKOH8ExIT1qg6xc4zyS3HaEEATgs1btfzxq15ciUiY7gjSXRGQ==", "license": "MIT" }, + "node_modules/imurmurhash": { + "version": "0.1.4", + "resolved": "https://registry.npmjs.org/imurmurhash/-/imurmurhash-0.1.4.tgz", + "integrity": "sha512-JmXMZ6wuvDmLiHEml9ykzqO6lwFbof0GG4IkcGaENdCRDDmMVnny7s5HsIgHCbaq0w2MyPhDqkhTUgS2LU2PHA==", + "license": "MIT", + "engines": { + "node": ">=0.8.19" + } + }, "node_modules/inflight": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", @@ -4229,6 +4238,19 @@ "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", "integrity": "sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==" }, + "node_modules/write-file-atomic": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/write-file-atomic/-/write-file-atomic-7.0.0.tgz", + "integrity": "sha512-YnlPC6JqnZl6aO4uRc+dx5PHguiR9S6WeoLtpxNT9wIG+BDya7ZNE1q7KOjVgaA73hKhKLpVPgJ5QA9THQ5BRg==", + "license": "ISC", + "dependencies": { + "imurmurhash": "^0.1.4", + "signal-exit": "^4.0.1" + }, + "engines": { + "node": "^20.17.0 || >=22.9.0" + } + }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", @@ -4502,7 +4524,8 @@ "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.25.2", - "proper-lockfile": "^4.1.2" + "proper-lockfile": "^4.1.2", + "write-file-atomic": "^7.0.0" }, "bin": { "mcp-server-memory": "dist/index.js" diff --git a/src/memory/index.ts b/src/memory/index.ts index 6d8b56f30c..eed6a9f687 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -7,6 +7,7 @@ import { promises as fs } from 'fs'; import path from 'path'; import lockfile from 'proper-lockfile'; import { fileURLToPath } from 'url'; +import writeFileAtomic from 'write-file-atomic'; // Define memory file path using environment variable with fallback export const defaultMemoryPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'memory.jsonl'); @@ -153,10 +154,7 @@ export class KnowledgeGraphManager { relationType: r.relationType })), ]; - // Atomic write: write to temp file, then rename - const tmpPath = `${this.memoryFilePath}.tmp`; - await fs.writeFile(tmpPath, lines.join("\n")); - await fs.rename(tmpPath, this.memoryFilePath); + await writeFileAtomic(this.memoryFilePath, lines.join("\n"), { fsync: false }); } async createEntities(entities: Entity[]): Promise { diff --git a/src/memory/package.json b/src/memory/package.json index 13a5a19045..adc239e531 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -26,7 +26,8 @@ }, "dependencies": { "@modelcontextprotocol/sdk": "^1.25.2", - "proper-lockfile": "^4.1.2" + "proper-lockfile": "^4.1.2", + "write-file-atomic": "^7.0.0" }, "devDependencies": { "@types/node": "^22", From 7c54ecc4fc03de19153fb56ad59f18ed8b08f789 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 21:40:37 +0800 Subject: [PATCH 08/12] fix(memory): add @types/write-file-atomic and improve error handling - Add missing @types/write-file-atomic to fix TypeScript build error - Refactor withLock to use try/finally for clearer control flow - Fix incorrect comment about proper-lockfile requirements --- package-lock.json | 11 +++++++++ .../__tests__/multi-process-lock.test.ts | 2 +- src/memory/index.ts | 23 ++++++++++++------- src/memory/package.json | 1 + 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index e59e1ba2fd..48361190ea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1221,6 +1221,16 @@ "@types/node": "*" } }, + "node_modules/@types/write-file-atomic": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/@types/write-file-atomic/-/write-file-atomic-4.0.3.tgz", + "integrity": "sha512-qdo+vZRchyJIHNeuI1nrpsLw+hnkgqP/8mlaN6Wle/NKhydHmUN9l4p3ZE8yP90AJNJW4uB8HQhedb4f1vNayQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@types/yargs": { "version": "17.0.33", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.33.tgz", @@ -4533,6 +4543,7 @@ "devDependencies": { "@types/node": "^22", "@types/proper-lockfile": "^4.1.4", + "@types/write-file-atomic": "^4.0.3", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", "tsx": "^4.21.0", diff --git a/src/memory/__tests__/multi-process-lock.test.ts b/src/memory/__tests__/multi-process-lock.test.ts index 85d294f97b..d9d665d609 100644 --- a/src/memory/__tests__/multi-process-lock.test.ts +++ b/src/memory/__tests__/multi-process-lock.test.ts @@ -24,7 +24,7 @@ if (isWorker) { path.dirname(currentFilePath), `test-multi-process-${Date.now()}.jsonl` ); - // Create empty file for locking (proper-lockfile requires file to exist) + // Create empty file for testing await fs.writeFile(testFilePath, ''); }); diff --git a/src/memory/index.ts b/src/memory/index.ts index eed6a9f687..86e5be91d0 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -98,15 +98,22 @@ export class KnowledgeGraphManager { } private async withLock(fn: () => Promise): Promise { - return lockfile.lock(this.memoryFilePath, this.lockOptions) - .then(async (release) => { - const result = await fn(); + let release: (() => Promise) | undefined; + try { + release = await lockfile.lock(this.memoryFilePath, this.lockOptions); + } catch (e) { + throw new Error(`Failed to acquire lock: ${e instanceof Error ? e.message : String(e)}`); + } + + try { + return await fn(); + } finally { + try { await release(); - return result; - }) - .catch((e) => { - throw new Error(`Lock operation failed: ${e.message}`); - }); + } catch (e) { + throw new Error(`Failed to release lock: ${e instanceof Error ? e.message : String(e)}`); + } + } } private async loadGraph(): Promise { diff --git a/src/memory/package.json b/src/memory/package.json index adc239e531..10fe101921 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -32,6 +32,7 @@ "devDependencies": { "@types/node": "^22", "@types/proper-lockfile": "^4.1.4", + "@types/write-file-atomic": "^4.0.3", "@vitest/coverage-v8": "^2.1.8", "shx": "^0.3.4", "tsx": "^4.21.0", From 69c65544fa0ac6904cb22a7b75f0adac0e936946 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 21:45:50 +0800 Subject: [PATCH 09/12] refactor(memory): simplify withLock using .catch() and distinguish lock vs business errors --- src/memory/index.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/memory/index.ts b/src/memory/index.ts index 86e5be91d0..751ba82df3 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -98,21 +98,17 @@ export class KnowledgeGraphManager { } private async withLock(fn: () => Promise): Promise { - let release: (() => Promise) | undefined; - try { - release = await lockfile.lock(this.memoryFilePath, this.lockOptions); - } catch (e) { - throw new Error(`Failed to acquire lock: ${e instanceof Error ? e.message : String(e)}`); - } + const release = await lockfile.lock(this.memoryFilePath, this.lockOptions) + .catch((e: unknown) => { + throw new Error(`Failed to acquire memory file lock: ${e instanceof Error ? e.message : String(e)}`); + }); try { return await fn(); } finally { - try { - await release(); - } catch (e) { - throw new Error(`Failed to release lock: ${e instanceof Error ? e.message : String(e)}`); - } + await release().catch((e: unknown) => { + throw new Error(`Failed to release memory file lock: ${e instanceof Error ? e.message : String(e)}`); + }); } } From 556f8a4eb994b223c93110801eb0e5f8d5fe3b58 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 21:48:30 +0800 Subject: [PATCH 10/12] fix(memory): correct lock cleanup and error message assertion - Use rmdir instead of unlink for lock directory cleanup - Update error message assertion to match new format --- src/memory/__tests__/knowledge-graph.test.ts | 2 +- src/memory/__tests__/multi-process-lock.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index b70487d453..3aff05ac86 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -520,7 +520,7 @@ describe('KnowledgeGraphManager', () => { // Failed operations should contain correct error message failed.forEach(f => { - expect(f.reason.message).toContain('Lock operation failed:'); + expect(f.reason.message).toContain('Failed to acquire'); }); console.log(`Stress test: ${succeeded.length} succeeded, ${failed.length} failed`); diff --git a/src/memory/__tests__/multi-process-lock.test.ts b/src/memory/__tests__/multi-process-lock.test.ts index d9d665d609..4964ab56ee 100644 --- a/src/memory/__tests__/multi-process-lock.test.ts +++ b/src/memory/__tests__/multi-process-lock.test.ts @@ -34,9 +34,9 @@ if (isWorker) { } catch { // Ignore errors if file doesn't exist } - // Clean up lock file if exists + // Clean up lock directory if exists (proper-lockfile creates a directory, not a file) try { - await fs.unlink(`${testFilePath}.lock`); + await fs.rmdir(`${testFilePath}.lock`); } catch { // Ignore } From ffd7c534334683044460a3ca7325006adc725932 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Tue, 3 Feb 2026 22:04:56 +0800 Subject: [PATCH 11/12] docs(memory): explain lock-free read strategy in KnowledgeGraphManager --- src/memory/index.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/memory/index.ts b/src/memory/index.ts index 751ba82df3..c8e0e673c3 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -236,6 +236,14 @@ export class KnowledgeGraphManager { } async readGraph(): Promise { + // We intentionally do not use a read lock here. + // 1. Performance: Read locks would serialize all reads, significantly degrading performance + // for read-heavy workloads (like LLM context retrieval). + // 2. Deadlock risk: Read-write locks increase deadlock probability. + // 3. Atomicity: write-file-atomic ensures we either read the old file or the new file, + // never a partial write. + // 4. Optimistic concurrency: In the rare case of a race condition (ENOENT/ESTALE), + // it's better for the client to retry than to block all readers. return this.loadGraph(); } From 280f6fba4460e4e873073a14981632420a4bab88 Mon Sep 17 00:00:00 2001 From: xz-dev Date: Thu, 5 Feb 2026 10:40:36 +0800 Subject: [PATCH 12/12] fix(memory): correct NFS cache terminology and extend retry timeout - Change acregmax to acdirmax (lockfile is a directory, not a file) - Increase minTimeout from 50ms to 60ms so total retry time (~61s) exceeds stale timeout (60s) - Add note about custom acdirmax configuration --- src/memory/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/memory/index.ts b/src/memory/index.ts index c8e0e673c3..0b7813e4b8 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -69,22 +69,24 @@ export interface KnowledgeGraph { // The KnowledgeGraphManager class contains all operations to interact with the knowledge graph // Lock configuration optimized for both local disk and NFS/network file systems // -// For NFS: stale must be > acregmax (typically 60s) to avoid false stale detection +// For NFS: stale must be > acdirmax (typically 60s) to avoid false stale detection // due to attribute caching. Setting stale=60000ms ensures that even if another // process sees a 50s-old cached mtime, it won't incorrectly assume the lock is stale. +// Note: If your NFS has non-default acdirmax, set stale >= acdirmax via lockOptions in constructor. // // For local disk: These settings work perfectly - the longer stale timeout just means // a slightly longer wait if a process actually crashes (60s vs 10s), which is acceptable. // -// Retry strategy: minTimeout=50ms allows fast acquisition on local disk, +// Retry strategy: minTimeout=60ms allows fast acquisition on local disk, // exponential backoff handles NFS latency. -// Max wait: 50 + 100 + 200 + 400 + 800 + 1600 + 3200 + 6400 + 12800 + 25600 ≈ 51s +// Max wait: 60 + 120 + 240 + 480 + 960 + 1920 + 3840 + 7680 + 15360 + 30720 ≈ 61s +// This exceeds the stale timeout (60s) to ensure we wait long enough for NFS acdirmax cache const DEFAULT_LOCK_OPTIONS = { - stale: 60000, // 60s - safe for NFS acregmax (default 60s) + stale: 60000, // 60s - safe for NFS acdirmax (default 60s) update: 10000, // 10s heartbeat - ensures lock freshness even with NFS cache delays retries: { retries: 10, - minTimeout: 50, + minTimeout: 60, factor: 2, }, realpath: false,