fix(filesystem): include error reason + enumerate failed dirs at startup (#4152)

src/filesystem/__tests__/startup-validation.test.ts

@@ -97,4 +97,73 @@ describe('Startup Directory Validation', () => {
     // Should still start with the valid directory
     expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
   });
+
+  // Regression coverage for #4152 — warnings about inaccessible startup
+  // directories must include the underlying error reason so hosts can
+  // surface an actionable diagnostic to end users (the original report
+  // saw a silent transport close because nothing identified the bad path
+  // beyond a generic "Cannot access directory" line).
+  it('should include the underlying error reason in the per-directory warning (#4152)', async () => {
+    const nonExistentDir = path.join(testDir, 'non-existent-dir-with-reason');
+
+    const result = await spawnServer([nonExistentDir, accessibleDir]);
+
+    // Should still skip + continue with the valid directory.
+    expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
+    // Diagnostic must name the directory AND the OS-level error reason.
+    expect(result.stderr).toContain(nonExistentDir);
+    expect(result.stderr).toMatch(/ENOENT|no such file or directory/i);
+  });
+
+  // Regression coverage for #4152 — permission-denied (EACCES) directories
+  // must surface the OS reason in the warning the same way ENOENT does. The
+  // original report only mentioned non-existent dirs, but the underlying
+  // try/catch handles every fs.stat failure and hosts on Windows / locked
+  // mounts hit EACCES just as often. Skipped on POSIX root (no chmod 0o000
+  // restriction) and on Windows (chmod has no effect on directory access).
+  const skipChmodEacces = process.platform === 'win32' || (typeof process.getuid === 'function' && process.getuid() === 0);
+  (skipChmodEacces ? it.skip : it)('should include EACCES reason for permission-denied directory (#4152)', async () => {
+    const lockedDir = path.join(testDir, 'locked-dir');
+    await fs.mkdir(lockedDir);
+    // Drop *all* perms on the parent so fs.stat on a child path returns EACCES.
+    const parent = path.join(testDir, 'no-traverse');
+    await fs.mkdir(parent);
+    const child = path.join(parent, 'inner');
+    await fs.mkdir(child);
+    await fs.chmod(parent, 0o000);
+
+    try {
+      const result = await spawnServer([child, accessibleDir]);
+      // Should still continue with the accessible one.
+      expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio');
+      expect(result.stderr).toContain(child);
+      expect(result.stderr).toMatch(/EACCES|permission denied/i);
+    } finally {
+      // Restore perms so afterEach() can remove the tree.
+      await fs.chmod(parent, 0o700).catch(() => {});
+    }
+  });
+
+  // Regression coverage for #4152 — when ALL startup directories fail, the
+  // aggregate exit error must enumerate each offending path AFTER the
+  // "Error:" marker (not only as scattered per-directory warnings) so a
+  // host capturing only the final fatal line can show users every entry of
+  // allowed_directories that needs fixing.
+  it('should list every offending directory inside the aggregate startup error (#4152)', async () => {
+    const bad1 = path.join(testDir, 'aggregate-fail-1');
+    const bad2 = path.join(testDir, 'aggregate-fail-2');
+
+    const result = await spawnServer([bad1, bad2]);
+
+    expect(result.exitCode).toBe(1);
+
+    // Slice from the "Error:" marker to end-of-stderr — the aggregate
+    // message itself must contain both paths, not only the per-directory
+    // warnings that precede it.
+    const errIdx = result.stderr.indexOf('Error:');
+    expect(errIdx, 'aggregate Error: line missing').toBeGreaterThanOrEqual(0);
+    const aggregate = result.stderr.slice(errIdx);
+    expect(aggregate).toContain(bad1);
+    expect(aggregate).toContain(bad2);
+  });
 });

src/filesystem/index.ts

@@ -66,24 +66,40 @@ let allowedDirectories = (await Promise.all(
   })
 )).flat();
 
-// Filter to only accessible directories, warn about inaccessible ones
+// Filter to only accessible directories, warn about inaccessible ones.
+// Track failed entries with their reason so the aggregate fatal error
+// (when EVERY directory is unusable) can enumerate them — hosts using the
+// stdio transport often surface only the final error line, so per-path
+// warnings are not enough. See issue #4152.
 const accessibleDirectories: string[] = [];
+const failedEntries: { dir: string; reason: string }[] = [];
 for (const dir of allowedDirectories) {
   try {
     const stats = await fs.stat(dir);
     if (stats.isDirectory()) {
       accessibleDirectories.push(dir);
     } else {
+      const reason = "not a directory";
       console.error(`Warning: ${dir} is not a directory, skipping`);
+      failedEntries.push({ dir, reason });
     }
   } catch (error) {
-    console.error(`Warning: Cannot access directory ${dir}, skipping`);
+    const reason = error instanceof Error ? error.message : String(error);
+    console.error(`Warning: Cannot access directory ${dir}, skipping (${reason})`);
+    failedEntries.push({ dir, reason });
   }
 }
 
-// Exit only if ALL paths are inaccessible (and some were specified)
+// Exit only if ALL paths are inaccessible (and some were specified). The
+// aggregate error enumerates every failed entry so the host can show the
+// user which allowed_directories items to fix (#4152).
 if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) {
-  console.error("Error: None of the specified directories are accessible");
+  const enumerated = failedEntries
+    .map(({ dir, reason }) => `  - ${dir} (${reason})`)
+    .join("\n");
+  console.error(
+    `Error: None of the specified directories are accessible:\n${enumerated}`,
+  );
   process.exit(1);
 }