From 8bbf1e9dc9c52e3cd7930f3c6111a6130b837f7b Mon Sep 17 00:00:00 2001 From: David Ahmann Date: Tue, 10 Mar 2026 12:00:29 -0400 Subject: [PATCH] filesystem: emit structured startup errors (#3512) --- .../__tests__/startup-validation.test.ts | 48 +++++++++++++++++++ src/filesystem/index.ts | 28 +++++++++-- src/filesystem/roots-utils.ts | 38 +++++++++++++-- src/filesystem/startup-errors.ts | 29 +++++++++++ 4 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 src/filesystem/startup-errors.ts diff --git a/src/filesystem/__tests__/startup-validation.test.ts b/src/filesystem/__tests__/startup-validation.test.ts index 3be283df74..2860d54431 100644 --- a/src/filesystem/__tests__/startup-validation.test.ts +++ b/src/filesystem/__tests__/startup-validation.test.ts @@ -6,6 +6,15 @@ import * as os from 'os'; const SERVER_PATH = path.join(__dirname, '..', 'dist', 'index.js'); +type StartupValidationEvent = { + kind: 'filesystem_startup_validation'; + code: string; + source: 'argv' | 'roots'; + message: string; + path?: string; + paths?: string[]; +}; + /** * Spawns the filesystem server with given arguments and returns exit info */ @@ -36,6 +45,21 @@ async function spawnServer(args: string[], timeoutMs = 2000): Promise<{ exitCode }); } +function getStructuredEvents(stderr: string): StartupValidationEvent[] { + return stderr + .split('\n') + .map((line) => line.trim()) + .filter(Boolean) + .flatMap((line) => { + try { + const parsed = JSON.parse(line) as StartupValidationEvent; + return parsed.kind === 'filesystem_startup_validation' ? [parsed] : []; + } catch { + return []; + } + }); +} + describe('Startup Directory Validation', () => { let testDir: string; let accessibleDir: string; @@ -64,10 +88,18 @@ describe('Startup Directory Validation', () => { const nonExistentDir = path.join(testDir, 'non-existent-dir-12345'); const result = await spawnServer([nonExistentDir, accessibleDir]); + const events = getStructuredEvents(result.stderr); // Should warn about inaccessible directory expect(result.stderr).toContain('Warning: Cannot access directory'); expect(result.stderr).toContain(nonExistentDir); + expect(events).toContainEqual({ + kind: 'filesystem_startup_validation', + code: 'argv_path_inaccessible', + source: 'argv', + path: nonExistentDir, + message: `Warning: Cannot access directory ${nonExistentDir}, skipping`, + }); // Should still start successfully expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); @@ -78,10 +110,18 @@ describe('Startup Directory Validation', () => { const nonExistent2 = path.join(testDir, 'non-existent-2'); const result = await spawnServer([nonExistent1, nonExistent2]); + const events = getStructuredEvents(result.stderr); // Should exit with error expect(result.exitCode).toBe(1); expect(result.stderr).toContain('Error: None of the specified directories are accessible'); + expect(events).toContainEqual({ + kind: 'filesystem_startup_validation', + code: 'argv_no_accessible_directories', + source: 'argv', + paths: [nonExistent1, nonExistent2], + message: 'Error: None of the specified directories are accessible', + }); }); it('should warn when path is not a directory', async () => { @@ -89,10 +129,18 @@ describe('Startup Directory Validation', () => { await fs.writeFile(filePath, 'content'); const result = await spawnServer([filePath, accessibleDir]); + const events = getStructuredEvents(result.stderr); // Should warn about non-directory expect(result.stderr).toContain('Warning:'); expect(result.stderr).toContain('not a directory'); + expect(events).toContainEqual({ + kind: 'filesystem_startup_validation', + code: 'argv_path_not_directory', + source: 'argv', + path: filePath, + message: `Warning: ${filePath} is not a directory, skipping`, + }); // Should still start with the valid directory expect(result.stderr).toContain('Secure MCP Filesystem Server running on stdio'); diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 7b67e63e58..5fdbf1c6c8 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -14,6 +14,7 @@ import { z } from "zod"; import { minimatch } from "minimatch"; import { normalizePath, expandHome } from './path-utils.js'; import { getValidRootDirectories } from './roots-utils.js'; +import { emitStartupValidationEvent } from './startup-errors.js'; import { // Function imports formatSize, @@ -74,16 +75,37 @@ for (const dir of allowedDirectories) { if (stats.isDirectory()) { accessibleDirectories.push(dir); } else { - console.error(`Warning: ${dir} is not a directory, skipping`); + const message = `Warning: ${dir} is not a directory, skipping`; + console.error(message); + emitStartupValidationEvent({ + code: 'argv_path_not_directory', + source: 'argv', + path: dir, + message, + }); } } catch (error) { - console.error(`Warning: Cannot access directory ${dir}, skipping`); + const message = `Warning: Cannot access directory ${dir}, skipping`; + console.error(message); + emitStartupValidationEvent({ + code: 'argv_path_inaccessible', + source: 'argv', + path: dir, + message, + }); } } // Exit only if ALL paths are inaccessible (and some were specified) if (accessibleDirectories.length === 0 && allowedDirectories.length > 0) { - console.error("Error: None of the specified directories are accessible"); + const message = "Error: None of the specified directories are accessible"; + console.error(message); + emitStartupValidationEvent({ + code: 'argv_no_accessible_directories', + source: 'argv', + paths: allowedDirectories, + message, + }); process.exit(1); } diff --git a/src/filesystem/roots-utils.ts b/src/filesystem/roots-utils.ts index 5e26bb246b..c2dcf79f68 100644 --- a/src/filesystem/roots-utils.ts +++ b/src/filesystem/roots-utils.ts @@ -4,6 +4,7 @@ import os from 'os'; import { normalizePath } from './path-utils.js'; import type { Root } from '@modelcontextprotocol/sdk/types.js'; import { fileURLToPath } from "url"; +import { emitStartupValidationEvent } from './startup-errors.js'; /** * Converts a root URI to a normalized directory path with basic security validation. @@ -57,7 +58,18 @@ export async function getValidRootDirectories( for (const requestedRoot of requestedRoots) { const resolvedPath = await parseRootUri(requestedRoot.uri); if (!resolvedPath) { - console.error(formatDirectoryError(requestedRoot.uri, undefined, 'invalid path or inaccessible')); + const message = formatDirectoryError( + requestedRoot.uri, + undefined, + 'invalid path or inaccessible', + ); + console.error(message); + emitStartupValidationEvent({ + code: 'root_invalid_or_inaccessible', + source: 'roots', + path: requestedRoot.uri, + message, + }); continue; } @@ -66,12 +78,30 @@ export async function getValidRootDirectories( if (stats.isDirectory()) { validatedDirectories.push(resolvedPath); } else { - console.error(formatDirectoryError(resolvedPath, undefined, 'non-directory root')); + const message = formatDirectoryError( + resolvedPath, + undefined, + 'non-directory root', + ); + console.error(message); + emitStartupValidationEvent({ + code: 'root_not_directory', + source: 'roots', + path: resolvedPath, + message, + }); } } catch (error) { - console.error(formatDirectoryError(resolvedPath, error)); + const message = formatDirectoryError(resolvedPath, error); + console.error(message); + emitStartupValidationEvent({ + code: 'root_validation_error', + source: 'roots', + path: resolvedPath, + message, + }); } } return validatedDirectories; -} \ No newline at end of file +} diff --git a/src/filesystem/startup-errors.ts b/src/filesystem/startup-errors.ts new file mode 100644 index 0000000000..78ce5c3249 --- /dev/null +++ b/src/filesystem/startup-errors.ts @@ -0,0 +1,29 @@ +export type StartupValidationCode = + | 'argv_path_not_directory' + | 'argv_path_inaccessible' + | 'argv_no_accessible_directories' + | 'root_invalid_or_inaccessible' + | 'root_not_directory' + | 'root_validation_error'; + +export type StartupValidationSource = 'argv' | 'roots'; + +type StartupValidationEvent = { + kind: 'filesystem_startup_validation'; + code: StartupValidationCode; + source: StartupValidationSource; + message: string; + path?: string; + paths?: string[]; +}; + +export function emitStartupValidationEvent( + event: Omit, +): void { + console.error( + JSON.stringify({ + kind: 'filesystem_startup_validation', + ...event, + }), + ); +}