diff --git a/.changeset/memory-4117-persistence-guidance.md b/.changeset/memory-4117-persistence-guidance.md new file mode 100644 index 0000000000..ea0414d2d4 --- /dev/null +++ b/.changeset/memory-4117-persistence-guidance.md @@ -0,0 +1,5 @@ +--- +"@modelcontextprotocol/server-memory": patch +--- + +Warn on stderr at startup when the memory file is inside a `node_modules` directory, where reinstalling or updating the server, or clearing the npx cache, can delete it, and recommend in the README setting `MEMORY_FILE_PATH` to an absolute path you own. The default path is unchanged. diff --git a/src/memory/README.md b/src/memory/README.md index f56252a06c..975debd12e 100644 --- a/src/memory/README.md +++ b/src/memory/README.md @@ -240,10 +240,20 @@ On Windows, use: } ``` -- `MEMORY_FILE_PATH`: Path to the memory storage JSONL file (default: `memory.jsonl` in the server directory) +- `MEMORY_FILE_PATH`: Path to the memory storage JSONL file (default: `memory.jsonl` in the server directory). A leading `~` expands to your home directory; a relative path resolves against the server directory. Set it to an absolute path you own; see [Choosing where the graph is stored](#choosing-where-the-graph-is-stored). When `MEMORY_FILE_PATH` is not set and the server directory still holds a legacy `memory.json` but no `memory.jsonl`, the server renames `memory.json` to `memory.jsonl` at startup. If that rename fails, the server exits with an error naming both paths and the cause, rather than starting on an empty graph; fix the cause (a permission problem, say) and restart, or set `MEMORY_FILE_PATH` to choose a file explicitly. +#### Choosing where the graph is stored + +**Set `MEMORY_FILE_PATH` to an absolute path you own**, such as `~/memory.jsonl`, as in the configuration above. The default `memory.jsonl` sits in the server's own directory, next to its code. Under `npx` or an npm install, that directory is inside a `node_modules` tree, which npm manages as its own: reinstalling or updating the server, or clearing the npx cache, can delete the file and the knowledge graph in it. A relative `MEMORY_FILE_PATH` resolves against that same directory, so it has the same problem. + +- When the memory file it resolves is inside a `node_modules` directory, the server prints a warning on stderr at startup naming the file; most clients show a server's stderr in its log. Move the file to the new path when you set the variable, or the server starts on an empty graph. +- The directory must already exist: the server creates the memory file, not the directories above it. +- The default stays where it is so that existing graphs keep loading. The [Docker](#docker) setup above keeps it in a named volume mounted over `/app/dist`, so the graph there survives restarts and draws no warning. + +Saves are atomic. Every write tool writes the whole graph to a temporary file in the same directory and renames it over the memory file, so a server stopped or killed mid-save leaves the previous graph intact rather than a half-written file. The file's permission bits are kept across the rename. Writes from several processes are serialized by a lock, described next. + #### Sharing one memory file between clients Each MCP client starts its own server process, so two clients using the same `MEMORY_FILE_PATH` (or both using the default) are two processes writing one file. Every write tool takes an exclusive lock file, `.lock`, next to the memory file while it reads, changes and saves the graph, so writes from different processes take turns and none is lost. The lock file exists only while a write is in progress, so the directory holding the memory file must be writable (it already must be, for the atomic save). diff --git a/src/memory/__tests__/entry-point.test.ts b/src/memory/__tests__/entry-point.test.ts index 5593f0d64b..2dba5e8e20 100644 --- a/src/memory/__tests__/entry-point.test.ts +++ b/src/memory/__tests__/entry-point.test.ts @@ -1,5 +1,6 @@ // Tests for the memory server's startup path (#4854): main() run in-process -// over an in-memory transport, so its MEMORY_FILE_PATH handling is covered, +// over an in-memory transport, so its MEMORY_FILE_PATH handling and its +// warning for a graph file inside node_modules (#4117) are covered, // and isMainModule(), the guard that keeps importing index.ts from starting a // stdio server. The bin itself is exercised over real stdio by // stdio-smoke.test.ts. @@ -52,11 +53,34 @@ describe("main", () => { expect(errorSpy).toHaveBeenCalledWith( "Knowledge Graph MCP Server running on stdio", ); + // A file outside node_modules draws no location warning (#4117). + expect(errorSpy).not.toHaveBeenCalledWith( + expect.stringContaining("node_modules"), + ); } finally { await client.close(); await server.close(); } }); + + it("warns on stderr when the graph file is inside node_modules (#4117)", async () => { + const installDir = path.join(dir, "node_modules", "server-memory", "dist"); + await fs.mkdir(installDir, { recursive: true }); + const graphPath = path.join(installDir, "memory.jsonl"); + process.env.MEMORY_FILE_PATH = graphPath; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const server = await main(InMemoryTransport.createLinkedPair()[1]); + try { + expect(errorSpy).toHaveBeenCalledWith( + expect.stringMatching( + /^WARNING: the memory file .*memory\.jsonl is inside a node_modules directory/, + ), + ); + } finally { + await server.close(); + } + }); }); describe("isMainModule", () => { diff --git a/src/memory/__tests__/file-path.test.ts b/src/memory/__tests__/file-path.test.ts index 7ba32b2c40..8633ef3f5f 100644 --- a/src/memory/__tests__/file-path.test.ts +++ b/src/memory/__tests__/file-path.test.ts @@ -1,5 +1,6 @@ // Tests for how the memory server picks its graph file: MEMORY_FILE_PATH -// handling, the memory.json -> memory.jsonl migration, and "~" expansion. +// handling, the memory.json -> memory.jsonl migration, "~" expansion, and the +// startup warning for a graph file npm could delete (#4117). // // Every test runs ensureMemoryFilePath against its own temporary directory. // Vitest runs test files in parallel, so files written beside the source are @@ -14,6 +15,7 @@ import { ensureMemoryFilePath, defaultMemoryPath, expandHome, + memoryFileLocationWarning, } from "../index.js"; describe("ensureMemoryFilePath", () => { @@ -292,3 +294,53 @@ describe("expandHome", () => { ); }); }); + +describe("memoryFileLocationWarning", () => { + it("warns about a file inside an npx or npm install's node_modules", () => { + const installed = path.join( + os.homedir(), + ".npm", + "_npx", + "abc123", + "node_modules", + "@modelcontextprotocol", + "server-memory", + "dist", + "memory.jsonl", + ); + const warning = memoryFileLocationWarning(installed); + expect(warning).toContain(installed); + expect(warning).toContain("node_modules"); + expect(warning).toContain("MEMORY_FILE_PATH"); + }); + + it("warns about a Windows path inside node_modules on any platform", () => { + expect( + memoryFileLocationWarning( + "C:\\Users\\me\\AppData\\npm\\node_modules\\@modelcontextprotocol\\server-memory\\dist\\memory.jsonl", + ), + ).toContain("node_modules"); + }); + + it("does not warn about a file outside node_modules", () => { + expect( + memoryFileLocationWarning(path.join(os.homedir(), "memory.jsonl")), + ).toBeUndefined(); + }); + + it("does not warn about a directory whose name only contains node_modules", () => { + expect( + memoryFileLocationWarning( + path.join(os.homedir(), "my_node_modules_notes", "memory.jsonl"), + ), + ).toBeUndefined(); + }); + + it("does not warn about the Docker image's /app/dist, which the setup mounts as a volume", () => { + expect( + memoryFileLocationWarning( + path.join(path.sep, "app", "dist", "memory.jsonl"), + ), + ).toBeUndefined(); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index 4892ee85a2..7ea250c8fa 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -107,6 +107,32 @@ async function pathExists(filePath: string): Promise { } } +// The startup warning for a memory file inside a node_modules tree, or +// undefined for any other path. npx and npm install the server under +// node_modules, so the default memory.jsonl beside dist/index.js lands there, +// and npm treats that tree as its own: a reinstall, an update or a cleared npx +// cache can delete the file and the graph in it (#4117). The default is not +// moved, since that would strand every existing graph; the warning says how to +// choose a durable path instead. Only node_modules is matched, not the server's +// directory as such: the Docker setup mounts a volume over /app/dist, and a +// source checkout's dist survives a rebuild, so there the default is durable. +export function memoryFileLocationWarning( + memoryFilePath: string, +): string | undefined { + // Both separators, so a Windows path is matched on any platform. + const segments = path.resolve(memoryFilePath).split(/[\\/]/); + if (!segments.includes("node_modules")) { + return undefined; + } + return ( + `WARNING: the memory file ${memoryFilePath} is inside a node_modules directory, ` + + "which npm manages: reinstalling or updating the server, or clearing the npx cache, " + + "can delete it and the knowledge graph it holds. Set MEMORY_FILE_PATH to an absolute " + + "path outside node_modules, in a directory that exists (for example ~/memory.jsonl), " + + "and move this file there to keep the graph." + ); +} + // We are storing our memory using entities, relations, and observations in a graph structure export interface Entity { name: string; @@ -1241,6 +1267,10 @@ export function registerKnowledgeGraphSubscriptions( // including one that stops the transport from starting, is reported on stderr. export async function main(transport?: Transport): Promise { const memoryFilePath = await ensureMemoryFilePath(); + const locationWarning = memoryFileLocationWarning(memoryFilePath); + if (locationWarning) { + console.error(locationWarning); + } const handle = serveStdio(({ era }) => createServer(memoryFilePath, era), { transport, // serveStdio reports a transport that fails, including one that cannot