From 18ce19763999dcf7697b00c86c3a427f01eb2919 Mon Sep 17 00:00:00 2001 From: eeshsaxena <139802361+eeshsaxena@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:28:51 +0530 Subject: [PATCH 1/7] fix(filesystem): make move_file fail instead of overwriting the destination (#4630) The move_file tool description and README both state the operation fails if the destination already exists, but the handler called fs.rename directly, which silently overwrites the destination. Since this server ships no delete tool, move_file effectively provided unadvertised, irreversible file deletion: moving any file onto an existing target destroyed the target. Add a moveFile helper in lib.ts that rejects when the destination already exists (checked with lstat, so an existing symlink is detected rather than followed) and only renames when the target is free, and use it from the move_file handler. Behaviour now matches the documented contract. Fixes #4628 --- src/filesystem/__tests__/lib.test.ts | 24 ++++++++++++++++++++++++ src/filesystem/index.ts | 3 ++- src/filesystem/lib.ts | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index 1da741f0b0..a3db2697f4 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -14,6 +14,7 @@ import { getFileStats, readFileContent, writeFileContent, + moveFile, // Search & filtering functions searchFilesWithValidation, // File editing functions @@ -339,6 +340,29 @@ describe('Lib Functions', () => { }); }); + describe('moveFile', () => { + it('moves the file when the destination does not exist', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename.mockResolvedValueOnce(undefined); + + await moveFile('/test/source.txt', '/test/dest.txt'); + + expect(mockFs.rename).toHaveBeenCalledWith('/test/source.txt', '/test/dest.txt'); + }); + + it('fails without overwriting when the destination already exists', async () => { + // lstat resolving means the destination is occupied. + mockFs.lstat.mockResolvedValueOnce({} as any); + + await expect(moveFile('/test/source.txt', '/test/dest.txt')).rejects.toThrow( + 'Destination already exists' + ); + + expect(mockFs.rename).not.toHaveBeenCalled(); + }); + }); + }); describe('Search & Filtering Functions', () => { diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 234605bb13..51ac523a66 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -21,6 +21,7 @@ import { getFileStats, readFileContent, writeFileContent, + moveFile, searchFilesWithValidation, applyFileEdits, tailFile, @@ -631,7 +632,7 @@ server.registerTool( async (args: z.infer) => { const validSourcePath = await validatePath(args.source); const validDestPath = await validatePath(args.destination); - await fs.rename(validSourcePath, validDestPath); + await moveFile(validSourcePath, validDestPath); const text = `Successfully moved ${args.source} to ${args.destination}`; const contentBlock = { type: "text" as const, text }; return { diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index a1c6f04b67..3d0b49e6cb 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -193,6 +193,25 @@ export async function writeFileContent(filePath: string, content: string): Promi } +export async function moveFile(sourcePath: string, destinationPath: string): Promise { + // The move_file tool contract (and README) state the operation fails if the + // destination already exists. fs.rename would silently overwrite it, which is + // a data-loss bug, so reject up front when anything - file, directory, or + // symlink - occupies the target. lstat is used so an existing symlink at the + // destination is detected rather than followed. + try { + await fs.lstat(destinationPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + await fs.rename(sourcePath, destinationPath); + return; + } + throw error; + } + throw new Error(`Destination already exists: ${destinationPath}`); +} + + // File Editing Functions interface FileEdit { oldText: string; From 642a9112e7ae8081c87fa1e2f34f14f5c3e42ab1 Mon Sep 17 00:00:00 2001 From: Teddy100x Date: Sat, 29 Aug 2026 00:00:33 +0700 Subject: [PATCH 2/7] fix(memory): write the knowledge graph atomically (#4642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(memory): write the knowledge graph atomically `saveGraph()` wrote the graph straight to the memory file: await fs.writeFile(this.memoryFilePath, lines.join("\n")); `fs.writeFile` opens the target with `'w'`, which truncates it before any new bytes are written. If the process is interrupted between truncation and completion — SIGKILL, container or host stop, OOM kill, power loss — the memory file is left empty or half-written. That file is the only persistence layer for the knowledge graph, so the window is small but the loss is total and unrecoverable. The fix writes to a temporary file in the same directory and renames it over the target. `rename(2)` is atomic on POSIX filesystems: a reader sees either the complete old file or the complete new one, never an intermediate state. Keeping the temp file in the same directory ensures the rename stays on one filesystem, since cross-device renames fail with EXDEV. On failure the temp file is removed so no strays accumulate. Tests added in `__tests__/atomic-save.test.ts`: - the live memory file is never opened for truncating writes - a committed graph survives a write that fails midway - no temporary files remain after a successful write - the temporary file is cleaned up when the rename fails - graph contents still round-trip correctly across reloads The first and fourth fail against the previous implementation. Full suite: 55 passed (50 existing, 5 new). `tsc --noEmit` clean. Fixes #4614 * fix(memory): use randomBytes for temp filename to avoid concurrent collision --------- Co-authored-by: teddiesloco --- src/memory/__tests__/atomic-save.test.ts | 133 +++++++++++++++++++++++ src/memory/index.ts | 25 ++++- 2 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 src/memory/__tests__/atomic-save.test.ts diff --git a/src/memory/__tests__/atomic-save.test.ts b/src/memory/__tests__/atomic-save.test.ts new file mode 100644 index 0000000000..8c702b5f7e --- /dev/null +++ b/src/memory/__tests__/atomic-save.test.ts @@ -0,0 +1,133 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { KnowledgeGraphManager, Entity } from '../index.js'; + +/** + * Regression tests for durable persistence of the knowledge graph. + * + * saveGraph() previously called fs.writeFile() directly on the memory file. + * fs.writeFile opens the target with 'w', which truncates it before any new + * bytes are written. If the process dies between truncation and completion + * (SIGKILL, container stop, OOM kill, power loss) the memory file — the sole + * persistence layer for the graph — is left empty or half-written, and the + * accumulated memory is unrecoverable. + * + * The fix writes to a temporary file in the same directory and then renames + * it over the target. rename(2) is atomic on POSIX filesystems: a reader + * either sees the complete old file or the complete new one, never a + * truncated intermediate state. + */ +describe('KnowledgeGraphManager persistence durability', () => { + let testDir: string; + let testFilePath: string; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-memory-atomic-')); + testFilePath = path.join(testDir, 'memory.jsonl'); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it('never truncates the live memory file in place', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + ]); + + // Any write that targets the live file directly is destructive: it + // truncates committed data before the replacement is durable. + const writeFileSpy = vi.spyOn(fs, 'writeFile'); + const openSpy = vi.spyOn(fs, 'open'); + + await manager.createEntities([ + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]); + + const destructiveTargets = [ + ...writeFileSpy.mock.calls.map(call => call[0]), + ...openSpy.mock.calls.map(call => call[0]), + ].filter(target => target === testFilePath); + + expect(destructiveTargets).toEqual([]); + }); + + it('leaves the committed graph intact when the write fails midway', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + ]); + + const before = await fs.readFile(testFilePath, 'utf-8'); + expect(before).toContain('Alice'); + + // Simulate the process being interrupted while persisting the next write. + vi.spyOn(fs, 'writeFile').mockImplementationOnce(async () => { + throw new Error('ENOSPC: simulated interruption'); + }); + + await expect( + manager.createEntities([ + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]) + ).rejects.toThrow(); + + vi.restoreAllMocks(); + + // The previously committed graph must survive untouched. + expect(await fs.readFile(testFilePath, 'utf-8')).toBe(before); + + const graph = await new KnowledgeGraphManager(testFilePath).readGraph(); + expect(graph.entities.map(e => e.name)).toEqual(['Alice']); + }); + + it('does not leave temporary files behind after a successful write', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + ]); + + expect(await fs.readdir(testDir)).toEqual(['memory.jsonl']); + }); + + it('cleans up the temporary file when the write fails', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + ]); + + vi.spyOn(fs, 'rename').mockImplementationOnce(async () => { + throw new Error('EXDEV: simulated rename failure'); + }); + + await expect( + manager.createEntities([ + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]) + ).rejects.toThrow(); + + vi.restoreAllMocks(); + expect(await fs.readdir(testDir)).toEqual(['memory.jsonl']); + }); + + it('still persists graph contents correctly across reloads', async () => { + const manager = new KnowledgeGraphManager(testFilePath); + await manager.createEntities([ + { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }, + { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, + ]); + await manager.createRelations([ + { from: 'Alice', to: 'Bob', relationType: 'knows' }, + ]); + + const reloaded = await new KnowledgeGraphManager(testFilePath).readGraph(); + expect(reloaded.entities.map(e => e.name).sort()).toEqual(['Alice', 'Bob']); + expect(reloaded.relations).toEqual([ + { from: 'Alice', to: 'Bob', relationType: 'knows' }, + ]); + }); +}); diff --git a/src/memory/index.ts b/src/memory/index.ts index 9865c5318e..b8704a9515 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 { randomBytes } from 'crypto'; import { fileURLToPath } from 'url'; // Define memory file path using environment variable with fallback @@ -114,7 +115,29 @@ export class KnowledgeGraphManager { relationType: r.relationType })), ]; - await fs.writeFile(this.memoryFilePath, lines.join("\n")); + + // Write to a temporary file in the same directory, then rename it over + // the target. fs.writeFile would truncate the memory file before writing, + // so an interruption (SIGKILL, container stop, OOM, power loss) would + // leave the only copy of the graph truncated and unrecoverable. + // rename(2) is atomic on POSIX filesystems: readers see either the + // complete old file or the complete new one, never a partial state. + // The temp file is kept in the same directory so the rename stays on one + // filesystem — renaming across mount points fails with EXDEV. + const directory = path.dirname(this.memoryFilePath); + const tempFilePath = path.join( + directory, + `${path.basename(this.memoryFilePath)}.${randomBytes(16).toString('hex')}.tmp` + ); + + try { + await fs.writeFile(tempFilePath, lines.join("\n")); + await fs.rename(tempFilePath, this.memoryFilePath); + } catch (error) { + // Never leave a stray temp file behind on failure. + await fs.unlink(tempFilePath).catch(() => {}); + throw error; + } } async createEntities(entities: Entity[]): Promise { From cf05b3769492ff94fbb25dad1a16c7703cd718c8 Mon Sep 17 00:00:00 2001 From: KXH Date: Sat, 29 Aug 2026 01:01:08 +0800 Subject: [PATCH 3/7] fix(filesystem): resolve Unicode-equivalent paths (#4638) Signed-off-by: KXH --- .../__tests__/unicode-paths.test.ts | 49 +++++++++++++++++ src/filesystem/lib.ts | 53 ++++++++++++++++--- 2 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 src/filesystem/__tests__/unicode-paths.test.ts diff --git a/src/filesystem/__tests__/unicode-paths.test.ts b/src/filesystem/__tests__/unicode-paths.test.ts new file mode 100644 index 0000000000..2d28ac21f9 --- /dev/null +++ b/src/filesystem/__tests__/unicode-paths.test.ts @@ -0,0 +1,49 @@ +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { setAllowedDirectories, validatePath } from '../lib.js'; + +describe('Unicode-equivalent filesystem paths', () => { + let testDirectory: string; + + beforeEach(async () => { + testDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-unicode-paths-')); + setAllowedDirectories([testDirectory]); + }); + + afterEach(async () => { + setAllowedDirectories([]); + await fs.rm(testDirectory, { recursive: true, force: true }); + }); + + it('resolves an existing decomposed path from a composed request', async () => { + const onDiskDirectory = 'de\u0301marche'; + const onDiskFile = 're\u0301sume\u0301.txt'; + await fs.mkdir(path.join(testDirectory, onDiskDirectory)); + await fs.writeFile(path.join(testDirectory, onDiskDirectory, onDiskFile), 'content'); + + const resolved = await validatePath(path.join(testDirectory, 'd\u00e9marche', 'r\u00e9sum\u00e9.txt')); + + expect(resolved).toBe(await fs.realpath(path.join(testDirectory, onDiskDirectory, onDiskFile))); + }); + + it('preserves a new basename after resolving a Unicode-equivalent parent', async () => { + const onDiskDirectory = 'de\u0301marche'; + await fs.mkdir(path.join(testDirectory, onDiskDirectory)); + + const resolved = await validatePath(path.join(testDirectory, 'd\u00e9marche', 'new.txt')); + + expect(resolved).toBe(path.join(await fs.realpath(path.join(testDirectory, onDiskDirectory)), 'new.txt')); + }); + + it('rejects ambiguous canonically equivalent entries', async () => { + const composed = 'caf\u00e9'; + const decomposed = 'cafe\u0301'; + await fs.mkdir(path.join(testDirectory, composed)); + await fs.mkdir(path.join(testDirectory, decomposed)); + + await expect(validatePath(path.join(testDirectory, 'cafe\u0341', 'file.txt'))) + .rejects.toThrow('Ambiguous Unicode path component'); + }); +}); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 3d0b49e6cb..f1164c3198 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -96,6 +96,46 @@ function resolveRelativePathAgainstAllowedDirectories(relativePath: string): str } // Security & Validation Functions +async function resolveUnicodeEquivalentPath(absolutePath: string): Promise { + const allowedDirectory = [...allowedDirectories] + .sort((left, right) => right.length - left.length) + .find(directory => isPathWithinAllowedDirectories(normalizePath(absolutePath), [directory])); + + if (!allowedDirectory) { + return absolutePath; + } + + let currentPath = await fs.realpath(allowedDirectory); + const relativeParts = path.relative(allowedDirectory, absolutePath).split(path.sep).filter(Boolean); + + for (let index = 0; index < relativeParts.length; index++) { + const requestedPart = relativeParts[index]; + const entries = (await fs.readdir(currentPath)) ?? []; + const exactMatch = entries.find(entry => entry === requestedPart); + const equivalentMatches = exactMatch + ? [exactMatch] + : entries.filter(entry => entry.normalize('NFC') === requestedPart.normalize('NFC')); + + if (equivalentMatches.length > 1) { + throw new Error(`Ambiguous Unicode path component: ${requestedPart}`); + } + + if (equivalentMatches.length === 0) { + if (index === relativeParts.length - 1) { + return path.join(currentPath, requestedPart); + } + throw new Error(`Parent directory does not exist: ${path.join(currentPath, requestedPart)}`); + } + + currentPath = await fs.realpath(path.join(currentPath, equivalentMatches[0])); + if (!isPathWithinAllowedDirectories(normalizePath(currentPath), allowedDirectories)) { + throw new Error(`Access denied - symlink target outside allowed directories: ${currentPath} not in ${allowedDirectories.join(', ')}`); + } + } + + return currentPath; +} + export async function validatePath(requestedPath: string): Promise { const expandedPath = expandHome(requestedPath); const absolute = path.isAbsolute(expandedPath) @@ -123,16 +163,13 @@ export async function validatePath(requestedPath: string): Promise { // Security: For new files that don't exist yet, verify parent directory // This ensures we can't create files in unauthorized locations if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - const parentDir = path.dirname(absolute); try { - const realParentPath = await fs.realpath(parentDir); - const normalizedParent = normalizePath(realParentPath); - if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) { - throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`); + return await resolveUnicodeEquivalentPath(absolute); + } catch (resolutionError) { + if ((resolutionError as NodeJS.ErrnoException).code === 'ENOENT') { + throw new Error(`Parent directory does not exist: ${path.dirname(absolute)}`); } - return absolute; - } catch { - throw new Error(`Parent directory does not exist: ${parentDir}`); + throw resolutionError; } } throw error; From 11845d51cbb42cba9695566f9134f5bd43d1216a Mon Sep 17 00:00:00 2001 From: Excelius <57819425+Excelius-Wang@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:01:16 +0800 Subject: [PATCH 4/7] fix(filesystem): preserve UTF-8 across head and tail chunks (#4667) --- src/filesystem/__tests__/lib.test.ts | 48 ++++++++++++++++++++++++++++ src/filesystem/lib.ts | 44 ++++++++++--------------- 2 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index a3db2697f4..5c19e54779 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -27,6 +27,23 @@ import { vi.mock('fs/promises'); const mockFs = fs as any; +function createMockFileHandle(content: Buffer) { + return { + read: vi.fn( + async (buffer: Buffer, offset: number, length: number, position: number) => { + const bytesRead = content.copy( + buffer, + offset, + position, + Math.min(position + length, content.length), + ); + return { bytesRead, buffer }; + }, + ), + close: vi.fn().mockResolvedValue(undefined), + }; +} + describe('Lib Functions', () => { beforeEach(() => { vi.clearAllMocks(); @@ -722,6 +739,23 @@ describe('Lib Functions', () => { expect(mockFileHandle.close).toHaveBeenCalled(); }); + it('preserves UTF-8 characters split across chunk boundaries', async () => { + const content = Buffer.concat([ + Buffer.from('discard\n'), + Buffer.from('界'), + Buffer.alloc(1017, 'a'), + Buffer.from('\nlast'), + ]); + const mockFileHandle = createMockFileHandle(content); + + mockFs.stat.mockResolvedValue({ size: content.length } as any); + mockFs.open.mockResolvedValue(mockFileHandle); + + const result = await tailFile('/test/file.txt', 2); + + expect(result).toBe(`界${'a'.repeat(1017)}\nlast`); + }); + it('handles read errors gracefully', async () => { mockFs.stat.mockResolvedValue({ size: 100 } as any); @@ -778,6 +812,20 @@ describe('Lib Functions', () => { expect(mockFileHandle.close).toHaveBeenCalled(); }); + it('preserves UTF-8 characters split across chunk boundaries', async () => { + const content = Buffer.concat([ + Buffer.alloc(1023, 'a'), + Buffer.from('界\nsecond'), + ]); + const mockFileHandle = createMockFileHandle(content); + + mockFs.open.mockResolvedValue(mockFileHandle); + + const result = await headFile('/test/file.txt', 1); + + expect(result).toBe(`${'a'.repeat(1023)}界`); + }); + it('handles files with leftover content', async () => { const mockFileHandle = { read: vi.fn(), diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index f1164c3198..d01e35f706 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -2,6 +2,7 @@ import fs from "fs/promises"; import path from "path"; import os from 'os'; import { randomBytes } from 'crypto'; +import { StringDecoder } from 'string_decoder'; import { diffLines, createTwoFilesPatch } from 'diff'; import { minimatch } from 'minimatch'; import { normalizePath, expandHome } from './path-utils.js'; @@ -364,42 +365,28 @@ export async function tailFile(filePath: string, numLines: number): Promise 0 && linesFound < numLines) { + while (position > 0 && newlinesFound < numLines) { const size = Math.min(CHUNK_SIZE, position); position -= size; const { bytesRead } = await fileHandle.read(chunk, 0, size, position); if (!bytesRead) break; - - // Get the chunk as a string and prepend any remaining text from previous iteration - const readData = chunk.slice(0, bytesRead).toString('utf-8'); - const chunkText = readData + remainingText; - - // Split by newlines and count - const chunkLines = normalizeLineEndings(chunkText).split('\n'); - - // If this isn't the end of the file, the first line is likely incomplete - // Save it to prepend to the next chunk - if (position > 0) { - remainingText = chunkLines[0]; - chunkLines.shift(); // Remove the first (incomplete) line - } - - // Add lines to our result (up to the number we need) - for (let i = chunkLines.length - 1; i >= 0 && linesFound < numLines; i--) { - lines.unshift(chunkLines[i]); - linesFound++; + + const readData = Buffer.from(chunk.subarray(0, bytesRead)); + chunks.unshift(readData); + for (const byte of readData) { + if (byte === 0x0a) newlinesFound++; } } - - return lines.join('\n'); + + const text = normalizeLineEndings(Buffer.concat(chunks).toString('utf-8')); + return text.split('\n').slice(-numLines).join('\n'); } finally { await fileHandle.close(); } @@ -413,13 +400,14 @@ export async function headFile(filePath: string, numLines: number): Promise 0 && lines.length < numLines) { From 40a8de87ed31a307c2a85cae21a78da9b7c1caf8 Mon Sep 17 00:00:00 2001 From: karl4c Date: Fri, 28 Aug 2026 13:01:24 -0400 Subject: [PATCH 5/7] fix(sequentialthinking): read server version from package.json (#4576) * fix(sequentialthinking): read server version from package.json Replace the hardcoded 0.2.0 so initialize serverInfo tracks the package version (0.6.2 in-tree; CalVer after release), matching the approach used for memory server version sync. Skip the dist-layout assertion when dist/ is absent so CI's test-before-build job (and local runs without prepare) still pass. * test(sequentialthinking): smoke stdio initialize serverInfo.version Spawn the built server over stdio and assert getServerVersion() matches package.json (and is not the old hardcoded 0.2.0). Skip when dist/ is absent so test-before-build CI still passes. --- .../__tests__/server-version.test.ts | 57 +++++++++++++++++++ src/sequentialthinking/index.ts | 3 +- src/sequentialthinking/version.ts | 33 +++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 src/sequentialthinking/__tests__/server-version.test.ts create mode 100644 src/sequentialthinking/version.ts diff --git a/src/sequentialthinking/__tests__/server-version.test.ts b/src/sequentialthinking/__tests__/server-version.test.ts new file mode 100644 index 0000000000..c7d7f5cb39 --- /dev/null +++ b/src/sequentialthinking/__tests__/server-version.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { createRequire } from 'node:module'; +import { existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; +import { resolvePackageVersion, SERVER_VERSION } from '../version.js'; + +const packageJson = createRequire(import.meta.url)('../package.json') as { version: string }; +const packageRoot = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); +const distVersionPath = path.join(packageRoot, 'dist', 'version.js'); +const distIndexPath = path.join(packageRoot, 'dist', 'index.js'); + +describe('server version', () => { + it('uses package.json version instead of a hardcoded string', () => { + expect(SERVER_VERSION).toBe(packageJson.version); + expect(resolvePackageVersion()).toBe(packageJson.version); + expect(SERVER_VERSION).not.toBe('0.2.0'); + }); + + // CI runs `npm test` before the dedicated build job. `npm ci` usually + // materializes dist/ via prepare, but that is not guaranteed (e.g. local + // `rm -rf dist && npm test`, or install with --ignore-scripts). + it.skipIf(!existsSync(distVersionPath))( + 'resolves package.json from the dist layout after build', + async () => { + const distModule = (await import(pathToFileURL(distVersionPath).href)) as { + SERVER_VERSION: string; + }; + expect(distModule.SERVER_VERSION).toBe(packageJson.version); + }, + ); + + it.skipIf(!existsSync(distIndexPath))( + 'stdio initialize reports package.json version in serverInfo', + async () => { + const transport = new StdioClientTransport({ + command: process.execPath, + args: [distIndexPath], + cwd: packageRoot, + stderr: 'pipe', + }); + const client = new Client({ name: 'version-smoke', version: '0.0.0' }); + + try { + await client.connect(transport); + const serverInfo = client.getServerVersion(); + expect(serverInfo?.name).toBe('sequential-thinking-server'); + expect(serverInfo?.version).toBe(packageJson.version); + expect(serverInfo?.version).not.toBe('0.2.0'); + } finally { + await client.close(); + } + }, + ); +}); diff --git a/src/sequentialthinking/index.ts b/src/sequentialthinking/index.ts index 217845bb3d..54c42df199 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -4,6 +4,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { z } from "zod"; import { SequentialThinkingServer } from './lib.js'; +import { SERVER_VERSION } from './version.js'; /** Safe boolean coercion that correctly handles string "false" */ const coercedBoolean = z.preprocess((val) => { @@ -17,7 +18,7 @@ const coercedBoolean = z.preprocess((val) => { const server = new McpServer({ name: "sequential-thinking-server", - version: "0.2.0", + version: SERVER_VERSION, }); const thinkingServer = new SequentialThinkingServer(); diff --git a/src/sequentialthinking/version.ts b/src/sequentialthinking/version.ts new file mode 100644 index 0000000000..1ab7d8f2fd --- /dev/null +++ b/src/sequentialthinking/version.ts @@ -0,0 +1,33 @@ +import { createRequire } from 'node:module'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * Resolve this package's version from package.json. + * + * Works both from source (`src/sequentialthinking/`) and from the published + * layout (`dist/`), where package.json lives one directory up. + */ +export function resolvePackageVersion(): string { + const require = createRequire(import.meta.url); + const moduleDir = path.dirname(fileURLToPath(import.meta.url)); + const candidates = [ + path.join(moduleDir, 'package.json'), + path.join(moduleDir, '..', 'package.json'), + ]; + + for (const candidate of candidates) { + try { + const pkg = require(candidate) as { version?: string }; + if (pkg.version) { + return pkg.version; + } + } catch { + // Try the next candidate when running from dist/ or source. + } + } + + throw new Error('Could not locate package.json for server version'); +} + +export const SERVER_VERSION = resolvePackageVersion(); From d6402efee6cc6446b23adbcb63051f9dc8612be5 Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Fri, 28 Aug 2026 11:57:34 -0600 Subject: [PATCH 6/7] fix(sequentialthinking): restore nextThoughtNeeded in the advertised inputSchema required array (#4652) * fix(sequentialthinking): restore nextThoughtNeeded in the advertised inputSchema required array commit 1cdf806d (#3533) wrapped nextThoughtNeeded in a z.preprocess-based coercedBoolean to fix a real footgun (string "false" coercing to true). zod's toJSONSchema(..., { io: "input" }) treats a z.preprocess()-wrapped field's input type as unknown, so it silently drops that field from the emitted required array, even though it carries no .optional(). A client that builds its call arguments from the advertised schema then omits nextThoughtNeeded and gets a -32602 Invalid params from the runtime validator, which still requires it. Rebuild coercedBoolean as a transform on an explicit z.union([z.boolean(), z.string()]) instead of z.preprocess. The union gives toJSONSchema a concrete input type to report, so nextThoughtNeeded stays in required, while parse behavior (including the case-insensitive string coercion #3533 added) is unchanged. Fixes #4651 * test(sequentialthinking): pin nextThoughtNeeded in required and string coercion Runs against the built server so it checks the schema the SDK emits. Skips when dist/ is absent, matching server-version.test.ts. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018Wo28CHPXyM3DHoLWvKnCK --------- Co-authored-by: olaservo Co-authored-by: Claude Fable 5 --- .../__tests__/input-schema.test.ts | 66 +++++++++++++++++++ src/sequentialthinking/index.ts | 16 ++--- 2 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 src/sequentialthinking/__tests__/input-schema.test.ts diff --git a/src/sequentialthinking/__tests__/input-schema.test.ts b/src/sequentialthinking/__tests__/input-schema.test.ts new file mode 100644 index 0000000000..4ff7be663c --- /dev/null +++ b/src/sequentialthinking/__tests__/input-schema.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { existsSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; + +const packageRoot = path.join(path.dirname(fileURLToPath(import.meta.url)), '..'); +const distIndexPath = path.join(packageRoot, 'dist', 'index.js'); + +// Regression coverage for #4651: nextThoughtNeeded must stay in the advertised +// `required` array, and string coercion must keep accepting "True"/"FALSE" +// while rejecting anything else. Runs against the built server so it checks +// the schema the SDK actually emits, not the zod object. +describe.skipIf(!existsSync(distIndexPath))('sequentialthinking input schema', () => { + let client: Client; + + beforeAll(async () => { + const transport = new StdioClientTransport({ + command: process.execPath, + args: [distIndexPath], + cwd: packageRoot, + stderr: 'pipe', + }); + client = new Client({ name: 'input-schema-test', version: '0.0.0' }); + await client.connect(transport); + }); + + afterAll(async () => { + await client?.close(); + }); + + it('advertises nextThoughtNeeded as required', async () => { + const { tools } = await client.listTools(); + const tool = tools.find(t => t.name === 'sequentialthinking'); + expect(tool).toBeDefined(); + expect(tool!.inputSchema.required).toEqual( + expect.arrayContaining(['thought', 'nextThoughtNeeded', 'thoughtNumber', 'totalThoughts']) + ); + }); + + it('rejects a call that omits nextThoughtNeeded', async () => { + const result = await client.callTool({ + name: 'sequentialthinking', + arguments: { thought: 't', thoughtNumber: 1, totalThoughts: 1 }, + }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/nextThoughtNeeded/); + }); + + it.each(['True', 'FALSE', 'true', 'false'])('accepts the string %s', async (value) => { + const result = await client.callTool({ + name: 'sequentialthinking', + arguments: { thought: 't', nextThoughtNeeded: value, thoughtNumber: 1, totalThoughts: 1 }, + }); + expect(result.isError).toBeFalsy(); + }); + + it.each(['yes', '', '1'])('rejects the string %j', async (value) => { + const result = await client.callTool({ + name: 'sequentialthinking', + arguments: { thought: 't', nextThoughtNeeded: value, thoughtNumber: 1, totalThoughts: 1 }, + }); + expect(result.isError).toBe(true); + }); +}); diff --git a/src/sequentialthinking/index.ts b/src/sequentialthinking/index.ts index 54c42df199..1ae09d1db8 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -6,15 +6,15 @@ import { z } from "zod"; import { SequentialThinkingServer } from './lib.js'; import { SERVER_VERSION } from './version.js'; -/** Safe boolean coercion that correctly handles string "false" */ -const coercedBoolean = z.preprocess((val) => { +/** Safe boolean coercion that correctly handles string "false". A union+transform, + * not z.preprocess (whose input type is `unknown`), so toJSONSchema keeps this required. */ +const coercedBoolean = z.union([z.boolean(), z.string()]).transform((val, ctx) => { if (typeof val === "boolean") return val; - if (typeof val === "string") { - if (val.toLowerCase() === "true") return true; - if (val.toLowerCase() === "false") return false; - } - return val; -}, z.boolean()); + if (val.toLowerCase() === "true") return true; + if (val.toLowerCase() === "false") return false; + ctx.addIssue({ code: "custom", message: `Expected boolean or "true"/"false" string, received "${val}"` }); + return z.NEVER; +}); const server = new McpServer({ name: "sequential-thinking-server", From cda92bdaacd558192fedf1a60d2bb27510792388 Mon Sep 17 00:00:00 2001 From: Riley Date: Fri, 28 Aug 2026 18:58:19 +0100 Subject: [PATCH 7/7] fix(filesystem): create_directory doesn't create parent directories (#4631) validatePath()'s ENOENT fallback checked only the immediate parent directory before allowing a new path through. Creating a path with more than one missing level (e.g. a/b/c when none of a, b, c exist) made the immediate parent check fail with "Parent directory does not exist", even though fs.mkdir was already called with recursive: true one level up in the create_directory handler and an existing allowed ancestor was available further up the tree. Walk up through missing ancestors until an existing one is found (or the filesystem root is reached), verifying that ancestor still resolves inside an allowed directory via realpath. The top-level lexical containment check already ran against the full target path, so this only extends how far the existence/symlink check climbs. Fixes #4629 Co-authored-by: Riley <313779891+rileybuilds@users.noreply.github.com> Co-authored-by: olaservo Co-authored-by: Claude Fable 5 --- src/filesystem/__tests__/lib.test.ts | 39 +++++++++++++------ .../__tests__/nested-parents.test.ts | 35 +++++++++++++++++ src/filesystem/lib.ts | 8 ++-- 3 files changed, 66 insertions(+), 16 deletions(-) create mode 100644 src/filesystem/__tests__/nested-parents.test.ts diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index 5c19e54779..eeee46b99d 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -208,19 +208,34 @@ describe('Lib Functions', () => { expect(result).toBe(path.resolve(newFilePath)); }); - it('rejects when parent directory does not exist', async () => { + it('walks past multiple missing ancestors to an existing allowed directory', async () => { + // e.g. create_directory('/home/user/nonexistent/nested/newfile.txt') when + // neither 'nonexistent' nor 'nonexistent/nested' exist yet, but '/home/user' + // (an allowed directory) does. Regression test for #4629. + const newFilePath = process.platform === 'win32' ? 'C:\\Users\\test\\nonexistent\\nested\\newfile.txt' : '/home/user/nonexistent/nested/newfile.txt'; + + const enoentError = new Error('ENOENT') as NodeJS.ErrnoException; + enoentError.code = 'ENOENT'; + + // The path itself is missing. The resolver then realpaths the allowed + // directory ('/home/user'), which falls through to the beforeEach base + // implementation and echoes the path back as-is. readdir is mocked to + // undefined, so every component below it counts as missing. + mockFs.realpath.mockRejectedValueOnce(enoentError); + + const result = await validatePath(newFilePath); + expect(result).toBe(path.resolve(newFilePath)); + }); + + it('rejects when no ancestor directory exists at all', async () => { const newFilePath = process.platform === 'win32' ? 'C:\\Users\\test\\nonexistent\\newfile.txt' : '/home/user/nonexistent/newfile.txt'; - - // Create errors with the ENOENT code - const enoentError1 = new Error('ENOENT') as NodeJS.ErrnoException; - enoentError1.code = 'ENOENT'; - const enoentError2 = new Error('ENOENT') as NodeJS.ErrnoException; - enoentError2.code = 'ENOENT'; - - mockFs.realpath - .mockRejectedValueOnce(enoentError1) - .mockRejectedValueOnce(enoentError2); - + + const enoentError = new Error('ENOENT') as NodeJS.ErrnoException; + enoentError.code = 'ENOENT'; + + // Every ancestor, all the way up to the filesystem root, is missing. + mockFs.realpath.mockRejectedValue(enoentError); + await expect(validatePath(newFilePath)) .rejects.toThrow('Parent directory does not exist'); }); diff --git a/src/filesystem/__tests__/nested-parents.test.ts b/src/filesystem/__tests__/nested-parents.test.ts new file mode 100644 index 0000000000..854dc4405f --- /dev/null +++ b/src/filesystem/__tests__/nested-parents.test.ts @@ -0,0 +1,35 @@ +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { setAllowedDirectories, validatePath } from '../lib.js'; + +// Regression coverage for #4629: validatePath must accept a path whose +// ancestors are missing several levels deep, so create_directory can mkdir -p. +describe('validatePath with multiple missing ancestors', () => { + let allowedDir: string; + let outsideDir: string; + + beforeEach(async () => { + allowedDir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-nested-allowed-'))); + outsideDir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-nested-outside-'))); + setAllowedDirectories([allowedDir]); + }); + + afterEach(async () => { + setAllowedDirectories([]); + await fs.rm(allowedDir, { recursive: true, force: true }); + await fs.rm(outsideDir, { recursive: true, force: true }); + }); + + it('returns the full path when several ancestors do not exist', async () => { + const requested = path.join(allowedDir, 'a', 'b', 'c'); + await expect(validatePath(requested)).resolves.toBe(requested); + }); + + it('rejects when the nearest existing ancestor is a symlink out of the allowed tree', async () => { + await fs.symlink(outsideDir, path.join(allowedDir, 'link'), 'junction'); + await expect(validatePath(path.join(allowedDir, 'link', 'a', 'b'))) + .rejects.toThrow('Access denied'); + }); +}); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index d01e35f706..f2371a9a01 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -122,10 +122,10 @@ async function resolveUnicodeEquivalentPath(absolutePath: string): Promise