diff --git a/.github/workflows/readme-pr-check.yml b/.github/workflows/readme-pr-check.yml index ec5a0a2..66cb3a3 100644 --- a/.github/workflows/readme-pr-check.yml +++ b/.github/workflows/readme-pr-check.yml @@ -1,10 +1,7 @@ name: README PR Check on: - # pull_request_target, not pull_request: fork PRs get a read-only token under - # pull_request, so the label and comment calls fail with 403. This job only - # calls the API and never checks out PR code, so the write token is safe. - pull_request_target: + pull_request: types: [opened] paths: - 'README.md' @@ -13,7 +10,7 @@ on: jobs: check-readme-only: - if: github.event_name == 'pull_request_target' + if: github.event_name == 'pull_request' runs-on: ubuntu-latest permissions: contents: read diff --git a/README.md b/README.md index 9c388c8..fe5351a 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ These servers aim to demonstrate MCP features and the official SDKs. The following reference servers are now archived and can be found at [servers-archived](https://github.com/modelcontextprotocol/servers-archived). - **[AWS KB Retrieval](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/aws-kb-retrieval-server)** - Retrieval from AWS Knowledge Base using Bedrock Agent Runtime. -- **[Brave Search](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/brave-search)** - Web and local search using Brave's Search API. Has been replaced by the [official server](https://github.com/brave/brave-search-mcp-server) ([`@brave/brave-search-mcp-server`](https://www.npmjs.com/package/@brave/brave-search-mcp-server)). +- **[Brave Search](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/brave-search)** - Web and local search using Brave's Search API. Has been replaced by the [official server](https://github.com/brave/brave-search-mcp-server). - **[EverArt](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/everart)** - AI image generation using various models. - **[GitHub](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/github)** - Repository management, file operations, and GitHub API integration. - **[GitLab](https://github.com/modelcontextprotocol/servers-archived/tree/main/src/gitlab)** - GitLab API, enabling project management. diff --git a/package-lock.json b/package-lock.json index 632d340..1845571 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3866,8 +3866,7 @@ "@modelcontextprotocol/sdk": "^1.30.0", "diff": "^8.0.3", "glob": "^13.0.6", - "minimatch": "^10.0.1", - "zod": "^4.0.0" + "minimatch": "^10.0.1" }, "bin": { "mcp-server-filesystem": "dist/index.js" @@ -3887,8 +3886,7 @@ "version": "0.6.3", "license": "SEE LICENSE IN LICENSE", "dependencies": { - "@modelcontextprotocol/sdk": "^1.30.0", - "zod": "^4.0.0" + "@modelcontextprotocol/sdk": "^1.30.0" }, "bin": { "mcp-server-memory": "dist/index.js" @@ -3908,8 +3906,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2", - "zod": "^4.0.0" + "yargs": "^17.7.2" }, "bin": { "mcp-server-sequential-thinking": "dist/index.js" diff --git a/src/everything/package.json b/src/everything/package.json index 745d1b1..bab2bac 100644 --- a/src/everything/package.json +++ b/src/everything/package.json @@ -1,6 +1,6 @@ { "name": "@modelcontextprotocol/server-everything", - "version": "2.0.0", + "version": "2026.8.18", "description": "MCP server that exercises all the features of the MCP protocol", "license": "SEE LICENSE IN LICENSE", "mcpName": "io.github.modelcontextprotocol/server-everything", @@ -45,4 +45,4 @@ "typescript": "^5.6.2", "vitest": "^4.1.8" } -} +} \ No newline at end of file diff --git a/src/everything/resources/subscriptions.ts b/src/everything/resources/subscriptions.ts index 854a863..2a5e574 100644 --- a/src/everything/resources/subscriptions.ts +++ b/src/everything/resources/subscriptions.ts @@ -99,9 +99,10 @@ export const setSubscriptionHandlers = (server: McpServer) => { /** * Sends simulated resource update notifications to the subscribed client. * - * Iterates the URIs in `subscriptions` and emits a - * `notifications/resources/updated` notification for each URI the given - * sessionId is subscribed to. + * This function iterates through all resource URIs stored in the subscriptions + * and checks if the specified session ID is subscribed to them. If so, it sends + * a notification through the provided server. If the session ID is no longer valid + * (disconnected), it removes the session ID from the list of subscribers. * * @param {McpServer} server - The server instance used to send notifications. * @param {string | undefined} sessionId - The session ID of the client to check for subscriptions. @@ -121,6 +122,8 @@ const sendSimulatedResourceUpdates = async ( method: "notifications/resources/updated", params: { uri }, }); + } else { + subscribers.delete(sessionId); // subscriber has disconnected } } }; diff --git a/src/everything/resources/templates.ts b/src/everything/resources/templates.ts index 4530741..6d4903f 100644 --- a/src/everything/resources/templates.ts +++ b/src/everything/resources/templates.ts @@ -127,26 +127,31 @@ export const blobResourceUri = (resourceId: number) => new URL(`${blobUriBase}/${resourceId}`); /** - * Parses the resource identifier from the provided variables and validates - * that it is a positive integer. + * Parses the resource identifier from the provided URI and validates it + * against the given variables. Throws an error if the URI corresponds + * to an unknown resource or if the resource identifier is invalid. * - * The SDK only routes URIs that match the registered template to the - * resource handler, so by the time we get here the URI prefix is already - * known to be one of `textUriBase` / `blobUriBase`. Only the resourceId - * variable still needs validating. - * - * @param {URL} uri - The URI of the resource (used in the error message). - * @param {Record} variables - Context variables including resourceId. - * @returns {number} The parsed and validated resource identifier as a positive integer. - * @throws {Error} If the resourceId is not a finite positive integer. + * @param {URL} uri - The URI of the resource to be parsed. + * @param {Record} variables - A record containing context-specific variables that include the resourceId. + * @returns {number} The parsed and validated resource identifier as an integer. + * @throws {Error} Throws an error if the URI matches unsupported base URIs or if the resourceId is invalid. */ const parseResourceId = (uri: URL, variables: Record) => { - const idxStr = String((variables as any).resourceId ?? ""); - const idx = Number(idxStr); - if (Number.isFinite(idx) && Number.isInteger(idx) && idx > 0) { - return idx; + const uriError = `Unknown resource: ${uri.toString()}`; + if ( + uri.toString().startsWith(textUriBase) && + uri.toString().startsWith(blobUriBase) + ) { + throw new Error(uriError); + } else { + const idxStr = String((variables as any).resourceId ?? ""); + const idx = Number(idxStr); + if (Number.isFinite(idx) && Number.isInteger(idx) && idx > 0) { + return idx; + } else { + throw new Error(uriError); + } } - throw new Error(`Unknown resource: ${uri.toString()}`); }; /** diff --git a/src/fetch/Dockerfile b/src/fetch/Dockerfile index d2a4504..e81610c 100644 --- a/src/fetch/Dockerfile +++ b/src/fetch/Dockerfile @@ -11,8 +11,9 @@ ENV UV_COMPILE_BYTECODE=1 ENV UV_LINK_MODE=copy # Install the project's dependencies using the lockfile and settings -COPY uv.lock pyproject.toml ./ RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ uv sync --locked --no-install-project --no-dev --no-editable # Then, add the rest of the project source code and install it @@ -26,10 +27,6 @@ FROM python:3.12-slim-bookworm WORKDIR /app COPY --from=uv /root/.local /root/.local - -RUN if ! id -u app >/dev/null 2>&1; then \ - useradd -rUM -s /usr/sbin/nologin app; \ - fi COPY --from=uv --chown=app:app /app/.venv /app/.venv # Place executables in the environment at the front of the path diff --git a/src/fetch/pyproject.toml b/src/fetch/pyproject.toml index ee97fc8..acc9b29 100644 --- a/src/fetch/pyproject.toml +++ b/src/fetch/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-server-fetch" -version = "0.6.3" +version = "2026.8.18" description = "A Model Context Protocol server providing tools to fetch and convert web content for usage by LLMs" readme = "README.md" requires-python = ">=3.10" diff --git a/src/fetch/uv.lock b/src/fetch/uv.lock index f35a318..2d10be8 100644 --- a/src/fetch/uv.lock +++ b/src/fetch/uv.lock @@ -564,7 +564,7 @@ wheels = [ [[package]] name = "mcp-server-fetch" -version = "0.6.3" +version = "2026.8.18" source = { editable = "." } dependencies = [ { name = "httpx" }, diff --git a/src/filesystem/__tests__/directory-tree.mcp-sdk.test.ts b/src/filesystem/__tests__/directory-tree.mcp-sdk.test.ts deleted file mode 100644 index 0cbe48c..0000000 --- a/src/filesystem/__tests__/directory-tree.mcp-sdk.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import * as fs from 'fs/promises'; -import * as path from 'path'; -import * as os from 'os'; -import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; - -describe('directory_tree MCP SDK regression', () => { - let client: Client; - let transport: StdioClientTransport; - let testDir: string; - - beforeEach(async () => { - const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-tree-')); - testDir = await fs.realpath(tmp); - - await fs.writeFile(path.join(testDir, 'root.txt'), 'root'); - await fs.mkdir(path.join(testDir, 'nested')); - await fs.writeFile(path.join(testDir, 'nested', 'child.txt'), 'child'); - - const serverPath = path.resolve(__dirname, '../dist/index.js'); - transport = new StdioClientTransport({ - command: 'node', - args: [serverPath, testDir], - }); - - client = new Client( - { name: 'directory-tree-regression-test', version: '1.0.0' }, - { capabilities: {} }, - ); - - await client.connect(transport); - }); - - afterEach(async () => { - await client?.close(); - await fs.rm(testDir, { recursive: true, force: true }); - }); - - it('returns structuredContent.content as a string (not an array) when called via MCP SDK', async () => { - const result = await client.callTool({ - name: 'directory_tree', - arguments: { path: testDir }, - }); - - // Regression test for issues where structuredContent was returned as an array, - // which causes MCP SDK validation to throw -32602 (invalid structured content). - const structured = result.structuredContent as { content: unknown }; - expect(structured).toBeDefined(); - expect(typeof structured.content).toBe('string'); - expect(Array.isArray(structured.content)).toBe(false); - - const parsed = JSON.parse(structured.content as string); - expect(Array.isArray(parsed)).toBe(true); - expect(parsed.length).toBeGreaterThan(0); - }); -}); diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index 24eb18a..e0ae612 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -14,7 +14,6 @@ import { getFileStats, readFileContent, writeFileContent, - moveFile, // Search & filtering functions searchFilesWithValidation, // File editing functions @@ -27,23 +26,6 @@ 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(); @@ -83,10 +65,8 @@ describe('Lib Functions', () => { }); it('handles negative numbers', () => { - // Negative numbers should return '0 B' as file sizes cannot be negative - expect(formatSize(-1)).toBe('0 B'); - expect(formatSize(-1024)).toBe('0 B'); - expect(formatSize(-1000000)).toBe('0 B'); + // Negative numbers will result in NaN for the log calculation + expect(formatSize(-1024)).toContain('NaN'); expect(formatSize(-0)).toBe('0 B'); }); @@ -173,13 +153,6 @@ describe('Lib Functions', () => { describe('Security & Validation Functions', () => { describe('validatePath', () => { - it('rejects Windows drive paths on POSIX hosts', async () => { - if (process.platform === 'win32') return; - - await expect(validatePath('C:\\Users\\me\\notes\\file.md')) - .rejects.toThrow('Windows-style path received on a POSIX host'); - }); - // Use Windows-compatible paths for testing const allowedDirs = process.platform === 'win32' ? ['C:\\Users\\test', 'C:\\temp'] : ['/home/user', '/tmp']; @@ -215,34 +188,19 @@ describe('Lib Functions', () => { expect(result).toBe(path.resolve(newFilePath)); }); - 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 () => { + it('rejects when parent directory does not exist', async () => { const newFilePath = process.platform === 'win32' ? 'C:\\Users\\test\\nonexistent\\newfile.txt' : '/home/user/nonexistent/newfile.txt'; - - 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); - + + // 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); + await expect(validatePath(newFilePath)) .rejects.toThrow('Parent directory does not exist'); }); @@ -350,56 +308,6 @@ describe('Lib Functions', () => { expect(mockFs.writeFile).toHaveBeenCalledWith('/test/file.txt', 'new content', { encoding: "utf-8", flag: 'wx' }); }); - - it('preserves file permissions when overwriting existing file', async () => { - // First writeFile call with 'wx' flag fails because file exists - mockFs.writeFile.mockRejectedValueOnce(Object.assign(new Error('EEXIST'), { code: 'EEXIST' })); - // stat returns executable permissions - mockFs.stat.mockResolvedValueOnce({ mode: 0o100755 }); - // Second writeFile (to temp) succeeds - mockFs.writeFile.mockResolvedValueOnce(undefined); - mockFs.rename.mockResolvedValueOnce(undefined); - mockFs.chmod.mockResolvedValueOnce(undefined); - - await writeFileContent('/test/script.sh', 'new content'); - - expect(mockFs.stat).toHaveBeenCalledWith('/test/script.sh'); - expect(mockFs.chmod).toHaveBeenCalledWith('/test/script.sh', 0o755); - }); - - it('does not fail the write when chmod fails', async () => { - mockFs.writeFile.mockRejectedValueOnce(Object.assign(new Error('EEXIST'), { code: 'EEXIST' })); - mockFs.stat.mockResolvedValueOnce({ mode: 0o100755 }); - mockFs.writeFile.mockResolvedValueOnce(undefined); - mockFs.rename.mockResolvedValueOnce(undefined); - mockFs.chmod.mockRejectedValueOnce(Object.assign(new Error('EPERM'), { code: 'EPERM' })); - - await expect(writeFileContent('/test/script.sh', 'new content')).resolves.toBeUndefined(); - expect(mockFs.unlink).not.toHaveBeenCalled(); - }); - }); - - 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(); - }); }); }); @@ -508,8 +416,6 @@ describe('Lib Functions', () => { beforeEach(() => { mockFs.readFile.mockResolvedValue('line1\nline2\nline3\n'); mockFs.writeFile.mockResolvedValue(undefined); - mockFs.stat.mockResolvedValue({ mode: 0o100644 }); - mockFs.chmod.mockResolvedValue(undefined); }); it('applies simple text replacement', async () => { @@ -604,30 +510,6 @@ describe('Lib Functions', () => { ); }); - it('preserves file permissions after applying edits', async () => { - mockFs.stat.mockResolvedValue({ mode: 0o100755 }); - const edits = [ - { oldText: 'line2', newText: 'modified line2' } - ]; - - mockFs.rename.mockResolvedValueOnce(undefined); - - await applyFileEdits('/test/script.sh', edits, false); - - expect(mockFs.stat).toHaveBeenCalledWith('/test/script.sh'); - expect(mockFs.chmod).toHaveBeenCalledWith('/test/script.sh', 0o755); - }); - - it('does not restore permissions in dry run mode', async () => { - const edits = [ - { oldText: 'line2', newText: 'modified line2' } - ]; - - await applyFileEdits('/test/file.txt', edits, true); - - expect(mockFs.chmod).not.toHaveBeenCalled(); - }); - it('throws error for non-matching edits', async () => { const edits = [ { oldText: 'nonexistent line', newText: 'replacement' } @@ -761,23 +643,6 @@ 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); @@ -834,20 +699,6 @@ 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/__tests__/nested-parents.test.ts b/src/filesystem/__tests__/nested-parents.test.ts deleted file mode 100644 index 854dc44..0000000 --- a/src/filesystem/__tests__/nested-parents.test.ts +++ /dev/null @@ -1,35 +0,0 @@ -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/__tests__/structured-content.test.ts b/src/filesystem/__tests__/structured-content.test.ts index 67da16f..4605b72 100644 --- a/src/filesystem/__tests__/structured-content.test.ts +++ b/src/filesystem/__tests__/structured-content.test.ts @@ -22,9 +22,7 @@ describe('structuredContent schema compliance', () => { beforeEach(async () => { // Create a temp directory for testing - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-test-')); - // macOS temp dirs may be symlinked (/var -> /private/var); resolve for server allowlist checks. - testDir = await fs.realpath(tmpDir); + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-test-')); // Create test files await fs.writeFile(path.join(testDir, 'test.txt'), 'test content'); diff --git a/src/filesystem/__tests__/unicode-paths.test.ts b/src/filesystem/__tests__/unicode-paths.test.ts deleted file mode 100644 index 2d28ac2..0000000 --- a/src/filesystem/__tests__/unicode-paths.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -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/index.ts b/src/filesystem/index.ts index 51ac523..234605b 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -21,7 +21,6 @@ import { getFileStats, readFileContent, writeFileContent, - moveFile, searchFilesWithValidation, applyFileEdits, tailFile, @@ -632,7 +631,7 @@ server.registerTool( async (args: z.infer) => { const validSourcePath = await validatePath(args.source); const validDestPath = await validatePath(args.destination); - await moveFile(validSourcePath, validDestPath); + await fs.rename(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 6cd2165..ce4af9f 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -2,7 +2,6 @@ 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'; @@ -44,7 +43,7 @@ export interface SearchResult { // Pure Utility Functions export function formatSize(bytes: number): string { const units = ['B', 'KB', 'MB', 'GB', 'TB']; - if (bytes <= 0) return '0 B'; + if (bytes === 0) return '0 B'; const i = Math.floor(Math.log(bytes) / Math.log(1024)); @@ -97,54 +96,8 @@ 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) { - // Nothing below this point exists yet, so there are no symlinks left to - // resolve. currentPath is already realpath'd and inside an allowed - // directory; append the missing tail so create_directory can mkdir -p it. - return path.join(currentPath, ...relativeParts.slice(index)); - } - - 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); - // Do not silently reinterpret a Windows drive path as a relative POSIX path. - // This would create a literal filename such as `C:\\Users\\...` inside the - // allowed root and report success for the wrong location. - if (process.platform !== 'win32' && /^(?:[A-Za-z]:)(?:[\\/]|$)/.test(expandedPath)) { - throw new Error(`Access denied - Windows-style path received on a POSIX host: ${requestedPath}`); - } const absolute = path.isAbsolute(expandedPath) ? path.resolve(expandedPath) : resolveRelativePathAgainstAllowedDirectories(expandedPath); @@ -170,13 +123,16 @@ 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 { - return await resolveUnicodeEquivalentPath(absolute); - } catch (resolutionError) { - if ((resolutionError as NodeJS.ErrnoException).code === 'ENOENT') { - throw new Error(`Parent directory does not exist: ${path.dirname(absolute)}`); + 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(', ')}`); } - throw resolutionError; + return absolute; + } catch { + throw new Error(`Parent directory does not exist: ${parentDir}`); } } throw error; @@ -212,7 +168,6 @@ export async function writeFileContent(filePath: string, content: string): Promi // Security: Use atomic rename to prevent race conditions where symlinks // could be created between validation and write. Rename operations // replace the target file atomically and don't follow symlinks. - const origStats = await fs.stat(filePath); const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, content, 'utf-8'); @@ -223,13 +178,6 @@ export async function writeFileContent(filePath: string, content: string): Promi } catch {} throw renameError; } - // Restore original permission bits since the atomic rename replaces the - // inode and the temp file has default (0644) permissions. Mask off the - // file-type bits; POSIX leaves them unspecified for chmod. A chmod - // failure must not fail the write, which has already succeeded. - try { - await fs.chmod(filePath, origStats.mode & 0o777); - } catch {} } else { throw error; } @@ -237,25 +185,6 @@ 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; @@ -337,7 +266,6 @@ export async function applyFileEdits( // Security: Use atomic rename to prevent race conditions where symlinks // could be created between validation and write. Rename operations // replace the target file atomically and don't follow symlinks. - const origStats = await fs.stat(filePath); const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, modifiedContent, 'utf-8'); @@ -348,13 +276,6 @@ export async function applyFileEdits( } catch {} throw error; } - // Restore original permission bits since the atomic rename replaces the - // inode and the temp file has default (0644) permissions. Mask off the - // file-type bits; POSIX leaves them unspecified for chmod. A chmod - // failure must not fail the write, which has already succeeded. - try { - await fs.chmod(filePath, origStats.mode & 0o777); - } catch {} } return formattedDiff; @@ -371,28 +292,42 @@ export async function tailFile(filePath: string, numLines: number): Promise 0 && newlinesFound < numLines) { + while (position > 0 && linesFound < numLines) { const size = Math.min(CHUNK_SIZE, position); position -= size; const { bytesRead } = await fileHandle.read(chunk, 0, size, position); if (!bytesRead) break; - - const readData = Buffer.from(chunk.subarray(0, bytesRead)); - chunks.unshift(readData); - for (const byte of readData) { - if (byte === 0x0a) newlinesFound++; + + // 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 text = normalizeLineEndings(Buffer.concat(chunks).toString('utf-8')); - return text.split('\n').slice(-numLines).join('\n'); + + return lines.join('\n'); } finally { await fileHandle.close(); } @@ -406,14 +341,13 @@ export async function headFile(filePath: string, numLines: number): Promise 0 && lines.length < numLines) { diff --git a/src/filesystem/package.json b/src/filesystem/package.json index 3c288a2..139c4f0 100644 --- a/src/filesystem/package.json +++ b/src/filesystem/package.json @@ -28,8 +28,7 @@ "@modelcontextprotocol/sdk": "^1.30.0", "diff": "^8.0.3", "glob": "^13.0.6", - "minimatch": "^10.0.1", - "zod": "^4.0.0" + "minimatch": "^10.0.1" }, "devDependencies": { "@types/diff": "^5.0.9", diff --git a/src/git/Dockerfile b/src/git/Dockerfile index 4af4161..6b53886 100644 --- a/src/git/Dockerfile +++ b/src/git/Dockerfile @@ -11,8 +11,9 @@ ENV UV_COMPILE_BYTECODE=1 ENV UV_LINK_MODE=copy # Install the project's dependencies using the lockfile and settings -COPY uv.lock pyproject.toml ./ RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ uv sync --locked --no-install-project --no-dev --no-editable # Then, add the rest of the project source code and install it @@ -29,10 +30,6 @@ RUN apt-get update && apt-get install -y git git-lfs && rm -rf /var/lib/apt/list WORKDIR /app COPY --from=uv /root/.local /root/.local - -RUN if ! id -u app >/dev/null 2>&1; then \ - useradd -rUM -s /usr/sbin/nologin app; \ - fi COPY --from=uv --chown=app:app /app/.venv /app/.venv # Place executables in the environment at the front of the path diff --git a/src/git/pyproject.toml b/src/git/pyproject.toml index e2407ba..45261d7 100644 --- a/src/git/pyproject.toml +++ b/src/git/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-server-git" -version = "0.6.2" +version = "2026.8.18" description = "A Model Context Protocol server providing tools to read, search, and manipulate Git repositories programmatically via LLMs" readme = "README.md" requires-python = ">=3.10" diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index b94af84..84188d8 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -1,6 +1,6 @@ import logging from pathlib import Path -from typing import Any, Optional, Sequence +from typing import Sequence, Optional from mcp.server import Server from mcp.server.session import ServerSession from mcp.server.stdio import stdio_server @@ -157,28 +157,45 @@ def git_reset(repo: git.Repo) -> str: return "All staged changes reset" def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] = None, end_timestamp: Optional[str] = None) -> list[str]: - # Defense in depth: reject timestamps starting with '-' to prevent flag injection - if start_timestamp and start_timestamp.startswith("-"): - raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'") - if end_timestamp and end_timestamp.startswith("-"): - raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'") + if start_timestamp or end_timestamp: + # Defense in depth: reject timestamps starting with '-' to prevent flag injection + if start_timestamp and start_timestamp.startswith("-"): + raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'") + if end_timestamp and end_timestamp.startswith("-"): + raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'") + # Use git log command with date filtering + args = [] + if start_timestamp: + args.extend(['--since', start_timestamp]) + if end_timestamp: + args.extend(['--until', end_timestamp]) + args.extend(['--format=%H%n%an%n%ad%n%s%n']) - kwargs: dict[str, Any] = {"max_count": max_count} - if start_timestamp: - kwargs["since"] = start_timestamp - if end_timestamp: - kwargs["until"] = end_timestamp + log_output = repo.git.log(*args).split('\n') - commits = list(repo.iter_commits(**kwargs)) - log = [] - for commit in commits: - log.append( - f"Commit: {commit.hexsha}\n" - f"Author: {commit.author}\n" - f"Date: {commit.authored_datetime}\n" - f"Message: {commit.message}\n" - ) - return log + log = [] + # Process commits in groups of 4 (hash, author, date, message) + for i in range(0, len(log_output), 4): + if i + 3 < len(log_output) and len(log) < max_count: + log.append( + f"Commit: {log_output[i]}\n" + f"Author: {log_output[i+1]}\n" + f"Date: {log_output[i+2]}\n" + f"Message: {log_output[i+3]}\n" + ) + return log + else: + # Use existing logic for simple log without date filtering + commits = list(repo.iter_commits(max_count=max_count)) + log = [] + for commit in commits: + log.append( + f"Commit: {commit.hexsha!r}\n" + f"Author: {commit.author!r}\n" + f"Date: {commit.authored_datetime}\n" + f"Message: {commit.message!r}\n" + ) + return log def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None = None) -> str: # Defense in depth: reject names starting with '-' to prevent flag injection @@ -582,4 +599,4 @@ async def serve(repository: Path | None) -> None: options = server.create_initialization_options() async with stdio_server() as (read_stream, write_stream): - await server.run(read_stream, write_stream, options) + await server.run(read_stream, write_stream, options, raise_exceptions=True) diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index 05d5931..893195d 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -16,10 +16,8 @@ from mcp_server_git.server import ( git_create_branch, git_show, validate_repo_path, - serve, ) import shutil -import unittest.mock as mock @pytest.fixture def test_repository(tmp_path: Path): @@ -510,81 +508,3 @@ def test_git_branch_rejects_contains_flag_injection(test_repository): with pytest.raises(BadName): git_branch(test_repository, "local", not_contains="--exec=evil") - - -def test_git_log_formatting_no_repr(test_repository): - """Test that git_log does not use !r formatting (no Python object repr or quotes).""" - file_path = Path(test_repository.working_dir) / "multiline.txt" - file_path.write_text("multiline test") - test_repository.index.add(["multiline.txt"]) - test_repository.index.commit("Subject line\n\nDetailed body line 1\nDetailed body line 2") - - result = git_log(test_repository, max_count=1) - entry = result[0] - - # Verify no python repr quotes around commit hash or message - assert "Commit: '" not in entry - assert 'Commit: "' not in entry - assert " { - 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/__tests__/delete-reporting.test.ts b/src/memory/__tests__/delete-reporting.test.ts deleted file mode 100644 index 944c302..0000000 --- a/src/memory/__tests__/delete-reporting.test.ts +++ /dev/null @@ -1,99 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { promises as fs } from 'fs'; -import path from 'path'; -import { fileURLToPath } from 'url'; -import { KnowledgeGraphManager, Entity, Relation } from '../index.js'; - -/** - * The delete tools stay silent when a target is absent, which the README - * documents. What they must not do is report a deletion that did not happen: - * an agent that mistypes a name is told its memory is clean while the data is - * still on disk, and nothing in the response contradicts that. - */ -describe('delete reporting', () => { - let manager: KnowledgeGraphManager; - let testFilePath: string; - - const entities: Entity[] = [ - { name: 'Alice', entityType: 'person', observations: ['works at Acme Corp', 'likes tea'] }, - { name: 'Bob', entityType: 'person', observations: ['likes programming'] }, - ]; - const relations: Relation[] = [{ from: 'Alice', to: 'Bob', relationType: 'works_with' }]; - - beforeEach(async () => { - testFilePath = path.join( - path.dirname(fileURLToPath(import.meta.url)), - `test-delete-reporting-${Date.now()}-${Math.random().toString(16).slice(2)}.jsonl` - ); - manager = new KnowledgeGraphManager(testFilePath); - await manager.createEntities(entities); - await manager.createRelations(relations); - }); - - afterEach(async () => { - try { - await fs.unlink(testFilePath); - } catch { - // the file is gone already - } - }); - - describe('deleteEntities', () => { - it('reports which names matched and which did not', async () => { - const result = await manager.deleteEntities(['Alice', 'Alise']); - expect(result).toEqual({ deleted: ['Alice'], notFound: ['Alise'] }); - }); - - it('reports nothing deleted when no name matches', async () => { - const result = await manager.deleteEntities(['Nobody']); - expect(result).toEqual({ deleted: [], notFound: ['Nobody'] }); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(2); - }); - - it('still deletes the entity and its relations', async () => { - await manager.deleteEntities(['Alice']); - - const graph = await manager.readGraph(); - expect(graph.entities.map(e => e.name)).toEqual(['Bob']); - expect(graph.relations).toHaveLength(0); - }); - }); - - describe('deleteObservations', () => { - it('counts only the observations that were present', async () => { - const result = await manager.deleteObservations([ - { entityName: 'Alice', observations: ['likes tea', 'never said this'] }, - ]); - expect(result).toEqual({ deletedCount: 1, missingEntities: [] }); - }); - - it('names an entity that does not exist', async () => { - const result = await manager.deleteObservations([ - { entityName: 'Carol', observations: ['anything'] }, - ]); - expect(result).toEqual({ deletedCount: 0, missingEntities: ['Carol'] }); - }); - }); - - describe('deleteRelations', () => { - it('counts only the relations that matched', async () => { - const result = await manager.deleteRelations([ - { from: 'Alice', to: 'Bob', relationType: 'works_with' }, - { from: 'Alice', to: 'Bob', relationType: 'never_existed' }, - ]); - expect(result).toEqual({ deletedCount: 1 }); - }); - - it('reports nothing deleted when the relation type is wrong', async () => { - const result = await manager.deleteRelations([ - { from: 'Alice', to: 'Bob', relationType: 'manages' }, - ]); - expect(result).toEqual({ deletedCount: 0 }); - - const graph = await manager.readGraph(); - expect(graph.relations).toHaveLength(1); - }); - }); -}); diff --git a/src/memory/__tests__/file-path.test.ts b/src/memory/__tests__/file-path.test.ts index fe5fdeb..d1a16e4 100644 --- a/src/memory/__tests__/file-path.test.ts +++ b/src/memory/__tests__/file-path.test.ts @@ -1,9 +1,8 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { promises as fs } from 'fs'; import path from 'path'; -import os from 'os'; import { fileURLToPath } from 'url'; -import { ensureMemoryFilePath, defaultMemoryPath, expandHome } from '../index.js'; +import { ensureMemoryFilePath, defaultMemoryPath } from '../index.js'; describe('ensureMemoryFilePath', () => { const testDir = path.dirname(fileURLToPath(import.meta.url)); @@ -73,15 +72,6 @@ describe('ensureMemoryFilePath', () => { expect(path.isAbsolute(result)).toBe(true); } }); - - it('should expand a leading "~/" to the home directory', async () => { - process.env.MEMORY_FILE_PATH = '~/custom-memory.jsonl'; - - const result = await ensureMemoryFilePath(); - - expect(result).toBe(path.join(os.homedir(), 'custom-memory.jsonl')); - expect(path.isAbsolute(result)).toBe(true); - }); }); describe('without MEMORY_FILE_PATH environment variable', () => { @@ -164,29 +154,3 @@ describe('ensureMemoryFilePath', () => { }); }); }); - -describe('expandHome', () => { - it('expands a bare "~" to the home directory', () => { - expect(expandHome('~')).toBe(os.homedir()); - }); - - it('expands a leading "~/" to the home directory', () => { - expect(expandHome('~/notes/memory.jsonl')).toBe( - path.join(os.homedir(), 'notes/memory.jsonl') - ); - }); - - it('leaves a "~" not followed by a separator unchanged', () => { - expect(expandHome('~backup.jsonl')).toBe('~backup.jsonl'); - }); - - it('leaves absolute paths unchanged', () => { - expect(expandHome('/var/data/memory.jsonl')).toBe('/var/data/memory.jsonl'); - }); - - it('leaves relative paths unchanged', () => { - expect(expandHome(path.join('data', 'memory.jsonl'))).toBe( - path.join('data', 'memory.jsonl') - ); - }); -}); diff --git a/src/memory/__tests__/knowledge-graph.test.ts b/src/memory/__tests__/knowledge-graph.test.ts index 7d05a0c..2362424 100644 --- a/src/memory/__tests__/knowledge-graph.test.ts +++ b/src/memory/__tests__/knowledge-graph.test.ts @@ -59,20 +59,6 @@ describe('KnowledgeGraphManager', () => { const newEntities = await manager.createEntities([]); expect(newEntities).toHaveLength(0); }); - - it('should ignore duplicate entity names within a single batch', async () => { - const entities: Entity[] = [ - { name: 'Alice', entityType: 'person', observations: ['first'] }, - { name: 'Alice', entityType: 'person', observations: ['second'] }, - ]; - - const newEntities = await manager.createEntities(entities); - expect(newEntities).toHaveLength(1); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(1); - expect(graph.entities[0].name).toBe('Alice'); - }); }); describe('createRelations', () => { @@ -113,60 +99,10 @@ describe('KnowledgeGraphManager', () => { expect(graph.relations).toHaveLength(1); }); - it('should reject relations from non-existent entities', async () => { - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: [] }, - ]); - - await expect( - manager.createRelations([ - { from: 'Ghost', to: 'Alice', relationType: 'knows' }, - ]) - ).rejects.toThrow('Entity with name Ghost not found'); - - const graph = await manager.readGraph(); - expect(graph.relations).toHaveLength(0); - }); - - it('should reject relation batches that reference non-existent target entities', async () => { - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: [] }, - { name: 'Bob', entityType: 'person', observations: [] }, - ]); - - await expect( - manager.createRelations([ - { from: 'Alice', to: 'Bob', relationType: 'knows' }, - { from: 'Alice', to: 'Ghost', relationType: 'knows' }, - ]) - ).rejects.toThrow('Entity with name Ghost not found'); - - const graph = await manager.readGraph(); - expect(graph.relations).toHaveLength(0); - }); - it('should handle empty relation arrays', async () => { const newRelations = await manager.createRelations([]); expect(newRelations).toHaveLength(0); }); - - it('should skip duplicate relations within a single batch', async () => { - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: [] }, - { name: 'Bob', entityType: 'person', observations: [] }, - ]); - - const relations: Relation[] = [ - { from: 'Alice', to: 'Bob', relationType: 'knows' }, - { from: 'Alice', to: 'Bob', relationType: 'knows' }, - ]; - - const newRelations = await manager.createRelations(relations); - expect(newRelations).toHaveLength(1); - - const graph = await manager.readGraph(); - expect(graph.relations).toHaveLength(1); - }); }); describe('addObservations', () => { @@ -210,11 +146,11 @@ describe('KnowledgeGraphManager', () => { }); it('should throw error for non-existent entity', async () => { - await expect(async () => { - await manager.addObservations([ + await expect( + manager.addObservations([ { entityName: 'NonExistent', contents: ['some observation'] }, - ]); - }).rejects.toThrow('Entity with name NonExistent not found'); + ]) + ).rejects.toThrow('Entity with name NonExistent not found'); }); }); @@ -490,76 +426,6 @@ describe('KnowledgeGraphManager', () => { expect(JSON.parse(lines[1])).toHaveProperty('type', 'relation'); }); - it('should write a trailing newline to produce valid JSONL', async () => { - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: ['test'] }, - ]); - - const fileContent = await fs.readFile(testFilePath, 'utf-8'); - expect(fileContent.endsWith('\n')).toBe(true); - }); - - it('should produce a file where every line is individually valid JSON', async () => { - // This test catches the bug where saveGraph wrote lines.join("\n") - // without a trailing newline. When the file was later appended to - // (e.g. by a concurrent process or external tool), the last JSON - // object and the new first JSON object ended up on the same line, - // producing invalid JSONL like: - // {"type":"entity","name":"Alice"}{"type":"relation","from":"Alice",...} - // which fails with: "Unexpected non-whitespace character after JSON - // at position N" - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: ['test'] }, - { name: 'Bob', entityType: 'person', observations: [] }, - ]); - await manager.createRelations([ - { from: 'Alice', to: 'Bob', relationType: 'knows' }, - ]); - - const fileContent = await fs.readFile(testFilePath, 'utf-8'); - const allLines = fileContent.split('\n'); - - // Every non-empty line must be valid JSON on its own - for (const line of allLines) { - if (line.trim() === '') continue; - expect(() => JSON.parse(line)).not.toThrow(); - } - }); - - it('should not corrupt JSONL when content is appended to the file externally', async () => { - // Simulate the real-world corruption scenario: - // 1. saveGraph writes entities to the file - // 2. An external process appends a new JSON line to the file - // 3. loadGraph must still parse the file without errors - // - // Without a trailing newline on step 1, the appended content in - // step 2 lands on the same line as the last entity, producing - // invalid JSONL that breaks loadGraph. - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: ['original'] }, - ]); - - // Simulate an external append (e.g. another process, a script, or - // a crash-recovery replay). This is what triggers the bug: without - // a trailing newline, this JSON object concatenates onto line 1. - const externalLine = JSON.stringify({ - type: 'entity', - name: 'External', - entityType: 'person', - observations: ['appended externally'], - }); - await fs.appendFile(testFilePath, externalLine + '\n'); - - // A new manager instance forces a fresh loadGraph from disk - const manager2 = new KnowledgeGraphManager(testFilePath); - const graph = await manager2.readGraph(); - - // Both entities must load without a JSON parse error - expect(graph.entities).toHaveLength(2); - expect(graph.entities.map(e => e.name)).toContain('Alice'); - expect(graph.entities.map(e => e.name)).toContain('External'); - }); - it('should strip type field from entities when loading from file', async () => { // Create entities and relations (these get saved with type field) await manager.createEntities([ @@ -649,126 +515,4 @@ describe('KnowledgeGraphManager', () => { expect(result.relations[0]).not.toHaveProperty('type'); }); }); - - describe('loadGraph validation', () => { - it('skips corrupt entities instead of crashing search', async () => { - const lines = [ - JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: ['works at Acme Corp'] }), - JSON.stringify({ type: 'entity', name: 'Broken', observations: ['missing entityType'] }), - JSON.stringify({ type: 'entity', name: 'BadObs', entityType: 'person', observations: ['ok', null] }), - ]; - await fs.writeFile(testFilePath, lines.join('\n') + '\n'); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(1); - expect(graph.entities[0].name).toBe('Alice'); - - // searchNodes must not throw even though the file contains corrupt entries - const result = await manager.searchNodes('Acme'); - expect(result.entities).toHaveLength(1); - expect(result.entities[0].name).toBe('Alice'); - }); - - it('skips corrupt relations', async () => { - const lines = [ - JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: [] }), - JSON.stringify({ type: 'relation', from: 'Alice', to: 'Bob' }), // missing relationType - JSON.stringify({ type: 'relation', from: 'Alice', to: 'Bob', relationType: 'knows' }), - ]; - await fs.writeFile(testFilePath, lines.join('\n') + '\n'); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(1); - expect(graph.relations).toHaveLength(1); - expect(graph.relations[0].relationType).toBe('knows'); - }); - - it('skips malformed JSON lines', async () => { - const lines = [ - JSON.stringify({ type: 'entity', name: 'Alice', entityType: 'person', observations: [] }), - '{this is not valid json', - JSON.stringify({ type: 'entity', name: 'Bob', entityType: 'person', observations: [] }), - ]; - await fs.writeFile(testFilePath, lines.join('\n') + '\n'); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(2); - expect(graph.entities.map(e => e.name)).toEqual(['Alice', 'Bob']); - }); - }); - - describe('concurrent mutations', () => { - // Regression test for #1819: concurrent tool calls each independently - // load the graph, mutate their own copy, and write it back. Without - // serialization, whichever write lands last silently discards the - // other's changes. All mutations below are fired without awaiting each - // other first, simulating multiple tool calls landing close together. - - it('should not lose entities created concurrently', async () => { - const batch1: Entity[] = Array.from({ length: 10 }, (_, i) => ({ - name: `batch1-entity-${i}`, - entityType: 'test', - observations: [], - })); - const batch2: Entity[] = Array.from({ length: 10 }, (_, i) => ({ - name: `batch2-entity-${i}`, - entityType: 'test', - observations: [], - })); - - // Fire both concurrently instead of awaiting sequentially. - await Promise.all([ - manager.createEntities(batch1), - manager.createEntities(batch2), - ]); - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(20); - expect(graph.entities.map(e => e.name).sort()).toEqual( - [...batch1, ...batch2].map(e => e.name).sort() - ); - }); - - it('should not lose relations created concurrently with entity creation', async () => { - await manager.createEntities([ - { name: 'Alice', entityType: 'person', observations: [] }, - { name: 'Bob', entityType: 'person', observations: [] }, - { name: 'Carol', entityType: 'person', observations: [] }, - ]); - - await Promise.all([ - manager.createRelations([{ from: 'Alice', to: 'Bob', relationType: 'knows' }]), - manager.createRelations([{ from: 'Bob', to: 'Carol', relationType: 'knows' }]), - manager.addObservations([ - { entityName: 'Alice', contents: ['likes coffee'] }, - ]), - ]); - - const graph = await manager.readGraph(); - expect(graph.relations).toHaveLength(2); - expect(graph.entities.find(e => e.name === 'Alice')?.observations).toContain('likes coffee'); - }); - - it('should keep the file valid JSONL after many concurrent mutations', async () => { - const operations = Array.from({ length: 25 }, (_, i) => - manager.createEntities([ - { name: `stress-entity-${i}`, entityType: 'test', observations: [] }, - ]) - ); - - await Promise.all(operations); - - const raw = await fs.readFile(testFilePath, 'utf-8'); - const lines = raw.split('\n').filter(line => line.trim() !== ''); - - // Every line must parse as valid JSON; a corrupted interleaved write - // would produce a truncated or malformed line here. - for (const line of lines) { - expect(() => JSON.parse(line)).not.toThrow(); - } - - const graph = await manager.readGraph(); - expect(graph.entities).toHaveLength(25); - }); - }); }); diff --git a/src/memory/__tests__/search-nodes-schema.test.ts b/src/memory/__tests__/search-nodes-schema.test.ts deleted file mode 100644 index c03f384..0000000 --- a/src/memory/__tests__/search-nodes-schema.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { SearchNodesQuerySchema, SEARCH_QUERY_MAX_LENGTH } from '../index.js'; - -describe('search_nodes input schema', () => { - it('should accept a normal query', () => { - expect(SearchNodesQuerySchema.safeParse('Alice').success).toBe(true); - expect(SearchNodesQuerySchema.safeParse('works at Acme Corp').success).toBe(true); - }); - - it('should accept a query at exactly the max length', () => { - const atLimit = 'a'.repeat(SEARCH_QUERY_MAX_LENGTH); - expect(SearchNodesQuerySchema.safeParse(atLimit).success).toBe(true); - }); - - it('should reject a query longer than the max length', () => { - const oversized = 'a'.repeat(SEARCH_QUERY_MAX_LENGTH + 1); - const result = SearchNodesQuerySchema.safeParse(oversized); - expect(result.success).toBe(false); - }); - - it('should still reject non-string input', () => { - expect(SearchNodesQuerySchema.safeParse(42).success).toBe(false); - expect(SearchNodesQuerySchema.safeParse(null).success).toBe(false); - }); -}); diff --git a/src/memory/__tests__/server-version.test.ts b/src/memory/__tests__/server-version.test.ts deleted file mode 100644 index f9cf437..0000000 --- a/src/memory/__tests__/server-version.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { createRequire } from 'node:module'; -import path from 'path'; -import { fileURLToPath } from 'url'; -import { resolvePackageVersion, SERVER_VERSION } from '../version.js'; - -const packageJson = createRequire(import.meta.url)('../package.json') as { version: string }; - -describe('server version', () => { - it('uses package.json version for serverInfo', () => { - expect(SERVER_VERSION).toBe(packageJson.version); - expect(resolvePackageVersion()).toBe(packageJson.version); - }); - - it('resolves package.json from the dist layout', () => { - const distDir = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', 'dist'); - const distVersionPath = path.join(distDir, 'version.js'); - - expect(() => createRequire(distVersionPath)('./version.js')).not.toThrow(); - const distModule = createRequire(distVersionPath)('./version.js') as { - SERVER_VERSION: string; - }; - expect(distModule.SERVER_VERSION).toBe(packageJson.version); - }); -}); diff --git a/src/memory/index.ts b/src/memory/index.ts index d9f8148..9865c53 100644 --- a/src/memory/index.ts +++ b/src/memory/index.ts @@ -6,35 +6,18 @@ import { SubscribeRequestSchema, UnsubscribeRequestSchema } from "@modelcontextp import { z } from "zod"; import { promises as fs } from 'fs'; import path from 'path'; -import os from 'os'; -import { randomBytes } from 'crypto'; import { fileURLToPath } from 'url'; -import { SERVER_VERSION } from './version.js'; // Define memory file path using environment variable with fallback export const defaultMemoryPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'memory.jsonl'); -// Expand a leading "~" to the user's home directory. MCP clients pass -// MEMORY_FILE_PATH from JSON config, where no shell performs tilde expansion, -// so an unexpanded "~" would otherwise be treated as a relative path and -// joined onto the package directory. Mirrors the helper of the same name in -// the filesystem server (src/filesystem/path-utils.ts). -export function expandHome(filepath: string): string { - if (filepath.startsWith('~/') || filepath === '~') { - return path.join(os.homedir(), filepath.slice(1)); - } - return filepath; -} - // Handle backward compatibility: migrate memory.json to memory.jsonl if needed export async function ensureMemoryFilePath(): Promise { if (process.env.MEMORY_FILE_PATH) { - // Custom path provided. Expand a leading "~" first, then resolve relative - // paths against the package directory (absolute paths are used as-is). - const customPath = expandHome(process.env.MEMORY_FILE_PATH); - return path.isAbsolute(customPath) - ? customPath - : path.join(path.dirname(fileURLToPath(import.meta.url)), customPath); + // Custom path provided, use it as-is (with absolute path resolution) + return path.isAbsolute(process.env.MEMORY_FILE_PATH) + ? process.env.MEMORY_FILE_PATH + : path.join(path.dirname(fileURLToPath(import.meta.url)), process.env.MEMORY_FILE_PATH); } // No custom path set, check for backward compatibility migration @@ -86,71 +69,28 @@ export interface KnowledgeGraph { export class KnowledgeGraphManager { constructor(private memoryFilePath: string) {} - // Serializes all read-modify-write graph mutations behind a single queue. - // Without this, concurrent tool calls (e.g. multiple mutations dispatched - // from one LLM turn) each independently load the graph, mutate their own - // copy, and write it back — so whichever write lands last silently - // overwrites the other's changes, and interleaved writes to the same file - // can corrupt it outright. See #1819. - private mutationQueue: Promise = Promise.resolve(); - - private async withLock(operation: () => Promise): Promise { - const result = this.mutationQueue.then(operation, operation); - // Always resolve the queue itself, even if this operation failed, so a - // single failed mutation doesn't permanently wedge every call after it. - // The failure still propagates normally to whoever awaited `result`. - this.mutationQueue = result.then( - () => undefined, - () => undefined, - ); - return result; - } - private async loadGraph(): Promise { try { const data = await fs.readFile(this.memoryFilePath, "utf-8"); const lines = data.split("\n").filter(line => line.trim() !== ""); - const graph: KnowledgeGraph = { entities: [], relations: [] }; - - for (const line of lines) { - let item: unknown; - try { - item = JSON.parse(line); - } catch { - console.error("Skipping malformed line in memory file"); - continue; + return lines.reduce((graph: KnowledgeGraph, line) => { + const item = JSON.parse(line); + if (item.type === "entity") { + graph.entities.push({ + name: item.name, + entityType: item.entityType, + observations: item.observations + }); } - - if (typeof item !== "object" || item === null) { - console.error("Skipping non-object line in memory file"); - continue; + if (item.type === "relation") { + graph.relations.push({ + from: item.from, + to: item.to, + relationType: item.relationType + }); } - - const record = item as Record; - if (record.type === "entity") { - const parsed = EntitySchema.safeParse(item); - if (parsed.success) { - graph.entities.push(parsed.data); - } else { - console.error( - "Skipping invalid entity in memory file:", - parsed.error.issues.map(issue => `${issue.path.join(".")}: ${issue.message}`).join(", ") - ); - } - } else if (record.type === "relation") { - const parsed = RelationSchema.safeParse(item); - if (parsed.success) { - graph.relations.push(parsed.data); - } else { - console.error( - "Skipping invalid relation in memory file:", - parsed.error.issues.map(issue => `${issue.path.join(".")}: ${issue.message}`).join(", ") - ); - } - } - } - - return graph; + return graph; + }, { entities: [], relations: [] }); } catch (error) { if (error instanceof Error && 'code' in error && (error as any).code === "ENOENT") { return { entities: [], relations: [] }; @@ -174,136 +114,70 @@ export class KnowledgeGraphManager { relationType: r.relationType })), ]; - - // 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") + "\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; - } + await fs.writeFile(this.memoryFilePath, lines.join("\n")); } async createEntities(entities: Entity[]): Promise { - return this.withLock(async () => { - const graph = await this.loadGraph(); - const newEntities = entities.filter((e, index) => - !graph.entities.some(existingEntity => existingEntity.name === e.name) && - // Also skip duplicates appearing earlier in this same batch - !entities.slice(0, index).some(earlier => earlier.name === e.name) - ); - graph.entities.push(...newEntities); - await this.saveGraph(graph); - return newEntities; - }); + 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 { - return this.withLock(async () => { - const graph = await this.loadGraph(); - const entityNames = new Set(graph.entities.map(e => e.name)); - - relations.forEach(r => { - if (!entityNames.has(r.from)) { - throw new Error(`Entity with name ${r.from} not found`); - } - if (!entityNames.has(r.to)) { - throw new Error(`Entity with name ${r.to} not found`); - } - }); - - const isSameRelation = (a: Relation, b: Relation) => - a.from === b.from && - a.to === b.to && - a.relationType === b.relationType; - const newRelations = relations.filter((r, index) => - !graph.relations.some(existingRelation => isSameRelation(existingRelation, r)) && - // Also skip duplicates appearing earlier in this same batch - !relations.slice(0, index).some(earlier => isSameRelation(earlier, r)) - ); - graph.relations.push(...newRelations); - await this.saveGraph(graph); - return newRelations; - }); + const 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[] }[]> { - 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; + 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; } - async deleteEntities(entityNames: string[]): Promise<{ deleted: string[]; notFound: string[] }> { - return this.withLock(async () => { - const graph = await this.loadGraph(); - const present = new Set(graph.entities.map(e => e.name)); - const deleted = entityNames.filter(name => present.has(name)); - const notFound = entityNames.filter(name => !present.has(name)); - graph.entities = graph.entities.filter(e => !entityNames.includes(e.name)); - graph.relations = graph.relations.filter(r => !entityNames.includes(r.from) && !entityNames.includes(r.to)); - await this.saveGraph(graph); - return { deleted, notFound }; - }); + async deleteEntities(entityNames: string[]): Promise { + const graph = await this.loadGraph(); + graph.entities = graph.entities.filter(e => !entityNames.includes(e.name)); + graph.relations = graph.relations.filter(r => !entityNames.includes(r.from) && !entityNames.includes(r.to)); + await this.saveGraph(graph); } - async deleteObservations(deletions: { entityName: string; observations: string[] }[]): Promise<{ deletedCount: number; missingEntities: string[] }> { - return this.withLock(async () => { - const graph = await this.loadGraph(); - let deletedCount = 0; - const missingEntities: string[] = []; - deletions.forEach(d => { - const entity = graph.entities.find(e => e.name === d.entityName); - if (entity) { - const before = entity.observations.length; - entity.observations = entity.observations.filter(o => !d.observations.includes(o)); - deletedCount += before - entity.observations.length; - } else { - missingEntities.push(d.entityName); - } - }); - await this.saveGraph(graph); - return { deletedCount, missingEntities }; + 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)); + } }); + await this.saveGraph(graph); } - async deleteRelations(relations: Relation[]): Promise<{ deletedCount: number }> { - return this.withLock(async () => { - const graph = await this.loadGraph(); - const before = graph.relations.length; - graph.relations = graph.relations.filter(r => !relations.some(delRelation => - r.from === delRelation.from && - r.to === delRelation.to && - r.relationType === delRelation.relationType - )); - await this.saveGraph(graph); - return { deletedCount: before - graph.relations.length }; - }); + async deleteRelations(relations: Relation[]): Promise { + const graph = await this.loadGraph(); + graph.relations = graph.relations.filter(r => !relations.some(delRelation => + r.from === delRelation.from && + r.to === delRelation.to && + r.relationType === delRelation.relationType + )); + await this.saveGraph(graph); } async readGraph(): Promise { @@ -379,9 +253,10 @@ const RelationSchema = z.object({ relationType: z.string().describe("The type of the relation") }); +// The server instance and tools exposed to Claude const server = new McpServer({ name: "memory-server", - version: SERVER_VERSION, + version: "0.6.3", }); const RESOURCE_URI = "memory://knowledge-graph"; @@ -512,14 +387,11 @@ server.registerTool( } }, async ({ entityNames }) => { - const { deleted, notFound } = await knowledgeGraphManager.deleteEntities(entityNames); + await knowledgeGraphManager.deleteEntities(entityNames); notifyGraphUpdated(); - const message = notFound.length === 0 - ? "Entities deleted successfully" - : `Deleted ${deleted.length} of ${entityNames.length} entities. Not found: ${notFound.join(", ")}`; return { - content: [{ type: "text" as const, text: message }], - structuredContent: { success: true, message } + content: [{ type: "text" as const, text: "Entities deleted successfully" }], + structuredContent: { success: true, message: "Entities deleted successfully" } }; } ); @@ -548,16 +420,11 @@ server.registerTool( } }, async ({ deletions }) => { - const { deletedCount, missingEntities } = await knowledgeGraphManager.deleteObservations(deletions); + await knowledgeGraphManager.deleteObservations(deletions); notifyGraphUpdated(); - const requested = deletions.reduce((total, d) => total + d.observations.length, 0); - const message = deletedCount === requested - ? "Observations deleted successfully" - : `Deleted ${deletedCount} of ${requested} observations.` + - (missingEntities.length ? ` Entities not found: ${missingEntities.join(", ")}` : ""); return { - content: [{ type: "text" as const, text: message }], - structuredContent: { success: true, message } + content: [{ type: "text" as const, text: "Observations deleted successfully" }], + structuredContent: { success: true, message: "Observations deleted successfully" } }; } ); @@ -583,14 +450,11 @@ server.registerTool( } }, async ({ relations }) => { - const { deletedCount } = await knowledgeGraphManager.deleteRelations(relations); + await knowledgeGraphManager.deleteRelations(relations); notifyGraphUpdated(); - const message = deletedCount === relations.length - ? "Relations deleted successfully" - : `Deleted ${deletedCount} of ${relations.length} relations. The rest matched nothing.`; return { - content: [{ type: "text" as const, text: message }], - structuredContent: { success: true, message } + content: [{ type: "text" as const, text: "Relations deleted successfully" }], + structuredContent: { success: true, message: "Relations deleted successfully" } }; } ); @@ -622,13 +486,6 @@ server.registerTool( } ); -export const SEARCH_QUERY_MAX_LENGTH = 2048; - -export const SearchNodesQuerySchema = z - .string() - .max(SEARCH_QUERY_MAX_LENGTH) - .describe("The search query to match against entity names, types, and observation content"); - // Register search_nodes tool server.registerTool( "search_nodes", @@ -636,7 +493,7 @@ server.registerTool( title: "Search Nodes", description: "Search for nodes in the knowledge graph based on a query", inputSchema: { - query: SearchNodesQuerySchema + query: z.string().describe("The search query to match against entity names, types, and observation content") }, outputSchema: { entities: z.array(EntitySchema), diff --git a/src/memory/package.json b/src/memory/package.json index ea10979..4fdcd3f 100644 --- a/src/memory/package.json +++ b/src/memory/package.json @@ -25,8 +25,7 @@ "test": "vitest run --coverage" }, "dependencies": { - "@modelcontextprotocol/sdk": "^1.30.0", - "zod": "^4.0.0" + "@modelcontextprotocol/sdk": "^1.30.0" }, "devDependencies": { "@types/node": "^22", diff --git a/src/memory/version.ts b/src/memory/version.ts deleted file mode 100644 index f1df958..0000000 --- a/src/memory/version.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { createRequire } from 'node:module'; -import path from 'path'; -import { fileURLToPath } from 'url'; - -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(); diff --git a/src/sequentialthinking/__tests__/input-schema.test.ts b/src/sequentialthinking/__tests__/input-schema.test.ts deleted file mode 100644 index 4ff7be6..0000000 --- a/src/sequentialthinking/__tests__/input-schema.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -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/__tests__/server-version.test.ts b/src/sequentialthinking/__tests__/server-version.test.ts deleted file mode 100644 index c7d7f5c..0000000 --- a/src/sequentialthinking/__tests__/server-version.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -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 1ae09d1..217845b 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -4,21 +4,20 @@ 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". 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) => { +/** Safe boolean coercion that correctly handles string "false" */ +const coercedBoolean = z.preprocess((val) => { if (typeof val === "boolean") return val; - 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; -}); + if (typeof val === "string") { + if (val.toLowerCase() === "true") return true; + if (val.toLowerCase() === "false") return false; + } + return val; +}, z.boolean()); const server = new McpServer({ name: "sequential-thinking-server", - version: SERVER_VERSION, + version: "0.2.0", }); const thinkingServer = new SequentialThinkingServer(); diff --git a/src/sequentialthinking/package.json b/src/sequentialthinking/package.json index a668a47..03fbb2b 100644 --- a/src/sequentialthinking/package.json +++ b/src/sequentialthinking/package.json @@ -27,8 +27,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.30.0", "chalk": "^5.3.0", - "yargs": "^17.7.2", - "zod": "^4.0.0" + "yargs": "^17.7.2" }, "devDependencies": { "@types/node": "^22", @@ -38,4 +37,4 @@ "typescript": "^5.3.3", "vitest": "^4.1.8" } -} +} \ No newline at end of file diff --git a/src/sequentialthinking/version.ts b/src/sequentialthinking/version.ts deleted file mode 100644 index 1ab7d8f..0000000 --- a/src/sequentialthinking/version.ts +++ /dev/null @@ -1,33 +0,0 @@ -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(); diff --git a/src/time/Dockerfile b/src/time/Dockerfile index c928732..ac5f752 100644 --- a/src/time/Dockerfile +++ b/src/time/Dockerfile @@ -11,8 +11,9 @@ ENV UV_COMPILE_BYTECODE=1 ENV UV_LINK_MODE=copy # Install the project's dependencies using the lockfile and settings -COPY uv.lock pyproject.toml ./ RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ uv sync --locked --no-install-project --no-dev --no-editable # Then, add the rest of the project source code and install it @@ -26,10 +27,6 @@ FROM python:3.12-slim-bookworm WORKDIR /app COPY --from=uv /root/.local /root/.local - -RUN if ! id -u app >/dev/null 2>&1; then \ - useradd -rUM -s /usr/sbin/nologin app; \ - fi COPY --from=uv --chown=app:app /app/.venv /app/.venv # Place executables in the environment at the front of the path diff --git a/src/time/pyproject.toml b/src/time/pyproject.toml index e6400d5..490ee43 100644 --- a/src/time/pyproject.toml +++ b/src/time/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "mcp-server-time" -version = "0.6.2" +version = "2026.8.18" description = "A Model Context Protocol server providing tools for time queries and timezone conversions for LLMs" readme = "README.md" requires-python = ">=3.10" diff --git a/src/time/uv.lock b/src/time/uv.lock index 542b996..f1ae822 100644 --- a/src/time/uv.lock +++ b/src/time/uv.lock @@ -349,7 +349,7 @@ wheels = [ [[package]] name = "mcp-server-time" -version = "0.6.2" +version = "2026.8.18" source = { editable = "." } dependencies = [ { name = "mcp" },