[pull] main from modelcontextprotocol:main

src/everything/resources/subscriptions.ts

@@ -99,10 +99,9 @@ export const setSubscriptionHandlers = (server: McpServer) => {
 /**
  * Sends simulated resource update notifications to the subscribed client.
  *
- * 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.
+ * Iterates the URIs in `subscriptions` and emits a
+ * `notifications/resources/updated` notification for each URI the given
+ * sessionId is subscribed to.
  *
  * @param {McpServer} server - The server instance used to send notifications.
  * @param {string | undefined} sessionId - The session ID of the client to check for subscriptions.
@@ -122,8 +121,6 @@ const sendSimulatedResourceUpdates = async (
         method: "notifications/resources/updated",
         params: { uri },
       });
-    } else {
-      subscribers.delete(sessionId); // subscriber has disconnected
     }
   }
 };

src/everything/resources/templates.ts

@@ -127,31 +127,26 @@ export const blobResourceUri = (resourceId: number) =>
   new URL(`${blobUriBase}/${resourceId}`);
 
 /**
- * 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.
+ * Parses the resource identifier from the provided variables and validates
+ * that it is a positive integer.
  *
- * @param {URL} uri - The URI of the resource to be parsed.
- * @param {Record<string, unknown>} 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.
+ * 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<string, unknown>} 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.
  */
 const parseResourceId = (uri: URL, variables: Record<string, unknown>) => {
-  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);
-    }
+  const idxStr = String((variables as any).resourceId ?? "");
+  const idx = Number(idxStr);
+  if (Number.isFinite(idx) && Number.isInteger(idx) && idx > 0) {
+    return idx;
   }
+  throw new Error(`Unknown resource: ${uri.toString()}`);
 };
 
 /**

src/filesystem/__tests__/directory-tree.mcp-sdk.test.ts

@@ -0,0 +1,57 @@
+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);
+  });
+});

src/filesystem/__tests__/lib.test.ts

@@ -65,8 +65,10 @@ describe('Lib Functions', () => {
       });
 
       it('handles negative numbers', () => {
-        // Negative numbers will result in NaN for the log calculation
-        expect(formatSize(-1024)).toContain('NaN');
+        // 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');
         expect(formatSize(-0)).toBe('0 B');
       });
 
@@ -308,6 +310,33 @@ 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();
+      });
     });
 
   });
@@ -416,6 +445,8 @@ 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 () => {
@@ -510,6 +541,30 @@ 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' }

src/filesystem/__tests__/structured-content.test.ts

@@ -22,7 +22,9 @@ describe('structuredContent schema compliance', () => {
 
   beforeEach(async () => {
     // Create a temp directory for testing
-    testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-test-'));
+    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);
 
     // Create test files
     await fs.writeFile(path.join(testDir, 'test.txt'), 'test content');

src/filesystem/lib.ts

@@ -43,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));
 
@@ -168,6 +168,7 @@ 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');
@@ -178,6 +179,13 @@ 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;
     }
@@ -266,6 +274,7 @@ 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');
@@ -276,6 +285,13 @@ 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;