diff --git a/src/filesystem/__tests__/roots-merge.test.ts b/src/filesystem/__tests__/roots-merge.test.ts new file mode 100644 index 0000000000..108b0e1585 --- /dev/null +++ b/src/filesystem/__tests__/roots-merge.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, realpathSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; + +describe('updateAllowedDirectoriesFromRoots merge behaviour', () => { + let cliDir: string; + let mcpDir1: string; + let mcpDir2: string; + + beforeEach(() => { + cliDir = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-cli-'))); + mcpDir1 = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-roots1-'))); + mcpDir2 = realpathSync(mkdtempSync(join(tmpdir(), 'mcp-roots2-'))); + }); + + afterEach(() => { + rmSync(cliDir, { recursive: true, force: true }); + rmSync(mcpDir1, { recursive: true, force: true }); + rmSync(mcpDir2, { recursive: true, force: true }); + }); + + it('should preserve CLI dirs after init with roots', () => { + // Simulates the snapshot: cliAllowedDirectories = [cliDir] + const cliAllowedDirectories = [cliDir]; + const validatedRootDirs = [mcpDir1]; + + const merged = new Set([...cliAllowedDirectories, ...validatedRootDirs]); + const allowedDirectories = [...merged]; + + expect(allowedDirectories).toContain(cliDir); + expect(allowedDirectories).toContain(mcpDir1); + }); + + it('should replace MCP roots on roots/list_changed without accumulating', () => { + // Simulates: CLI dir + first MCP root + const cliAllowedDirectories = [cliDir]; + const firstMcpRoots = [mcpDir1]; + + let merged = new Set([...cliAllowedDirectories, ...firstMcpRoots]); + let allowedDirectories = [...merged]; + + expect(allowedDirectories).toContain(cliDir); + expect(allowedDirectories).toContain(mcpDir1); + expect(allowedDirectories).toHaveLength(2); + + // Simulates: roots/list_changed with new root (mcpDir2 replaces mcpDir1) + const secondMcpRoots = [mcpDir2]; + merged = new Set([...cliAllowedDirectories, ...secondMcpRoots]); + allowedDirectories = [...merged]; + + expect(allowedDirectories).toContain(cliDir); + expect(allowedDirectories).toContain(mcpDir2); + expect(allowedDirectories).not.toContain(mcpDir1); + expect(allowedDirectories).toHaveLength(2); + }); + + it('should not accumulate stale roots across multiple updates', () => { + const cliAllowedDirectories = [cliDir]; + let allowedDirectories: string[] = []; + + for (let i = 0; i < 5; i++) { + const tmpDir = realpathSync(mkdtempSync(join(tmpdir(), `mcp-root-${i}-`))); + const merged = new Set([...cliAllowedDirectories, tmpDir]); + allowedDirectories = [...merged]; + rmSync(tmpDir, { recursive: true, force: true }); + } + + // After each iteration, only CLI dir + latest root should be present + expect(allowedDirectories).toHaveLength(2); + expect(allowedDirectories).toContain(cliDir); + }); +}); diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 7b67e63e58..cd55b8974e 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -89,6 +89,10 @@ if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) { allowedDirectories = accessibleDirectories; +// Save CLI-provided directories separately so dynamic MCP roots updates +// can replace the previous MCP roots without losing CLI dirs or accumulating stale ones. +const cliAllowedDirectories = [...allowedDirectories]; + // Initialize the global allowedDirectories in lib.ts setAllowedDirectories(allowedDirectories); @@ -706,7 +710,10 @@ server.registerTool( async function updateAllowedDirectoriesFromRoots(requestedRoots: Root[]) { const validatedRootDirs = await getValidRootDirectories(requestedRoots); if (validatedRootDirs.length > 0) { - allowedDirectories = [...validatedRootDirs]; + // Merge CLI-provided directories with current MCP roots, + // replacing any previous MCP roots on each update. + const merged = new Set([...cliAllowedDirectories, ...validatedRootDirs]); + allowedDirectories = [...merged]; setAllowedDirectories(allowedDirectories); // Update the global state in lib.ts console.error(`Updated allowed directories from MCP roots: ${validatedRootDirs.length} valid directories`); } else { @@ -714,7 +721,7 @@ async function updateAllowedDirectoriesFromRoots(requestedRoots: Root[]) { } } -// Handles dynamic roots updates during runtime, when client sends "roots/list_changed" notification, server fetches the updated roots and replaces all allowed directories with the new roots. +// Handles dynamic roots updates during runtime, when client sends "roots/list_changed" notification, server fetches the updated roots and merges them with CLI-provided directories (replacing any previous MCP roots). server.server.setNotificationHandler(RootsListChangedNotificationSchema, async () => { try { // Request the updated roots list from the client