From 227fa7abd14af67802bf40a0800209f4e5eec1a0 Mon Sep 17 00:00:00 2001 From: go165 <196723798+go165@users.noreply.github.com> Date: Sun, 14 Jun 2026 02:18:25 +0800 Subject: [PATCH] fix(filesystem): describe tool parameters --- src/filesystem/__tests__/tool-schema.test.ts | 61 ++++++++++++++ src/filesystem/index.ts | 87 ++++++++++++-------- 2 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 src/filesystem/__tests__/tool-schema.test.ts diff --git a/src/filesystem/__tests__/tool-schema.test.ts b/src/filesystem/__tests__/tool-schema.test.ts new file mode 100644 index 0000000000..cfd7fc82f3 --- /dev/null +++ b/src/filesystem/__tests__/tool-schema.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; + +describe('tool schemas', () => { + let client: Client; + let transport: StdioClientTransport; + let testDir: string; + + beforeEach(async () => { + testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-schema-test-')); + + const serverPath = path.resolve(__dirname, '../dist/index.js'); + transport = new StdioClientTransport({ + command: 'node', + args: [serverPath, testDir], + }); + + client = new Client({ + name: 'test-client', + version: '1.0.0', + }, { + capabilities: {} + }); + + await client.connect(transport); + }); + + afterEach(async () => { + await client?.close(); + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it('describes every required tool parameter', async () => { + const { tools } = await client.listTools(); + const missingDescriptions: string[] = []; + + for (const tool of tools) { + const required = tool.inputSchema.required ?? []; + const properties = tool.inputSchema.properties ?? {}; + + for (const propertyName of required) { + const property = properties[propertyName]; + if (!property || typeof property !== 'object' || !('description' in property)) { + missingDescriptions.push(`${tool.name}.${propertyName}`); + continue; + } + + const description = property.description; + if (typeof description !== 'string' || description.trim() === '') { + missingDescriptions.push(`${tool.name}.${propertyName}`); + } + } + } + + expect(missingDescriptions).toEqual([]); + }); +}); diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts index 7b67e63e58..0145185222 100644 --- a/src/filesystem/index.ts +++ b/src/filesystem/index.ts @@ -93,14 +93,33 @@ allowedDirectories = accessibleDirectories; setAllowedDirectories(allowedDirectories); // Schema definitions +const FilePathDescription = + "Path to a file within the allowed directories. Absolute paths are accepted; relative paths are resolved against the first allowed directory."; +const DirectoryPathDescription = + "Path to a directory within the allowed directories. Absolute paths are accepted; relative paths are resolved against the first allowed directory."; +const FileOrDirectoryPathDescription = + "Path to a file or directory within the allowed directories. Absolute paths are accepted; relative paths are resolved against the first allowed directory."; +const WriteContentDescription = + "Complete text content to write to the file. Existing file contents are overwritten."; +const EditOperationsDescription = + "Ordered list of exact text replacements to apply to the file."; +const SourcePathDescription = + "Existing file or directory path to move. Must be within the allowed directories."; +const DestinationPathDescription = + "Destination path for the moved file or directory. Must be within the allowed directories and must not already exist."; +const SearchPatternDescription = + "Glob-style pattern to match file and directory names, such as '*.ts' for the current directory or '**/*.ts' for all subdirectories."; +const ExcludePatternsDescription = + "Optional glob-style patterns to exclude from traversal, such as ['node_modules', '**/.git/**']."; + const ReadTextFileArgsSchema = z.object({ - path: z.string(), + path: z.string().describe(FilePathDescription), tail: z.number().optional().describe('If provided, returns only the last N lines of the file'), head: z.number().optional().describe('If provided, returns only the first N lines of the file') }); const ReadMediaFileArgsSchema = z.object({ - path: z.string() + path: z.string().describe(FilePathDescription) }); const ReadMultipleFilesArgsSchema = z.object({ @@ -111,8 +130,8 @@ const ReadMultipleFilesArgsSchema = z.object({ }); const WriteFileArgsSchema = z.object({ - path: z.string(), - content: z.string(), + path: z.string().describe(FilePathDescription), + content: z.string().describe(WriteContentDescription), }); const EditOperation = z.object({ @@ -121,42 +140,42 @@ const EditOperation = z.object({ }); const EditFileArgsSchema = z.object({ - path: z.string(), - edits: z.array(EditOperation), + path: z.string().describe(FilePathDescription), + edits: z.array(EditOperation).describe(EditOperationsDescription), dryRun: z.boolean().default(false).describe('Preview changes using git-style diff format') }); const CreateDirectoryArgsSchema = z.object({ - path: z.string(), + path: z.string().describe(DirectoryPathDescription), }); const ListDirectoryArgsSchema = z.object({ - path: z.string(), + path: z.string().describe(DirectoryPathDescription), }); const ListDirectoryWithSizesArgsSchema = z.object({ - path: z.string(), + path: z.string().describe(DirectoryPathDescription), sortBy: z.enum(['name', 'size']).optional().default('name').describe('Sort entries by name or size'), }); const DirectoryTreeArgsSchema = z.object({ - path: z.string(), - excludePatterns: z.array(z.string()).optional().default([]) + path: z.string().describe(DirectoryPathDescription), + excludePatterns: z.array(z.string()).optional().default([]).describe(ExcludePatternsDescription) }); const MoveFileArgsSchema = z.object({ - source: z.string(), - destination: z.string(), + source: z.string().describe(SourcePathDescription), + destination: z.string().describe(DestinationPathDescription), }); const SearchFilesArgsSchema = z.object({ - path: z.string(), - pattern: z.string(), - excludePatterns: z.array(z.string()).optional().default([]) + path: z.string().describe(DirectoryPathDescription), + pattern: z.string().describe(SearchPatternDescription), + excludePatterns: z.array(z.string()).optional().default([]).describe(ExcludePatternsDescription) }); const GetFileInfoArgsSchema = z.object({ - path: z.string(), + path: z.string().describe(FileOrDirectoryPathDescription), }); // Server setup @@ -235,7 +254,7 @@ server.registerTool( "the last N lines of a file. Operates on the file as text regardless of extension. " + "Only works within allowed directories.", inputSchema: { - path: z.string(), + path: z.string().describe(FilePathDescription), tail: z.number().optional().describe("If provided, returns only the last N lines of the file"), head: z.number().optional().describe("If provided, returns only the first N lines of the file") }, @@ -253,7 +272,7 @@ server.registerTool( "Read an image or audio file. Returns the base64 encoded data and MIME type. " + "Only works within allowed directories.", inputSchema: { - path: z.string() + path: z.string().describe(FilePathDescription) }, outputSchema: { content: z.array(z.object({ @@ -345,8 +364,8 @@ server.registerTool( "Use with caution as it will overwrite existing files without warning. " + "Handles text content with proper encoding. Only works within allowed directories.", inputSchema: { - path: z.string(), - content: z.string() + path: z.string().describe(FilePathDescription), + content: z.string().describe(WriteContentDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: false, idempotentHint: true, destructiveHint: true } @@ -371,11 +390,11 @@ server.registerTool( "with new content. Returns a git-style diff showing the changes made. " + "Only works within allowed directories.", inputSchema: { - path: z.string(), + path: z.string().describe(FilePathDescription), edits: z.array(z.object({ oldText: z.string().describe("Text to search for - must match exactly"), newText: z.string().describe("Text to replace with") - })), + })).describe(EditOperationsDescription), dryRun: z.boolean().default(false).describe("Preview changes using git-style diff format") }, outputSchema: { content: z.string() }, @@ -401,7 +420,7 @@ server.registerTool( "this operation will succeed silently. Perfect for setting up directory " + "structures for projects or ensuring required paths exist. Only works within allowed directories.", inputSchema: { - path: z.string() + path: z.string().describe(DirectoryPathDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: false, idempotentHint: true, destructiveHint: false } @@ -427,7 +446,7 @@ server.registerTool( "prefixes. This tool is essential for understanding directory structure and " + "finding specific files within a directory. Only works within allowed directories.", inputSchema: { - path: z.string() + path: z.string().describe(DirectoryPathDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: true } @@ -455,7 +474,7 @@ server.registerTool( "prefixes. This tool is useful for understanding directory structure and " + "finding specific files within a directory. Only works within allowed directories.", inputSchema: { - path: z.string(), + path: z.string().describe(DirectoryPathDescription), sortBy: z.enum(["name", "size"]).optional().default("name").describe("Sort entries by name or size") }, outputSchema: { content: z.string() }, @@ -534,8 +553,8 @@ server.registerTool( "Files have no children array, while directories always have a children array (which may be empty). " + "The output is formatted with 2-space indentation for readability. Only works within allowed directories.", inputSchema: { - path: z.string(), - excludePatterns: z.array(z.string()).optional().default([]) + path: z.string().describe(DirectoryPathDescription), + excludePatterns: z.array(z.string()).optional().default([]).describe(ExcludePatternsDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: true } @@ -604,8 +623,8 @@ server.registerTool( "operation will fail. Works across different directories and can be used " + "for simple renaming within the same directory. Both source and destination must be within allowed directories.", inputSchema: { - source: z.string(), - destination: z.string() + source: z.string().describe(SourcePathDescription), + destination: z.string().describe(DestinationPathDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: false, idempotentHint: false, destructiveHint: true } @@ -634,9 +653,9 @@ server.registerTool( "Returns full paths to all matching items. Great for finding files when you don't know their exact location. " + "Only searches within allowed directories.", inputSchema: { - path: z.string(), - pattern: z.string(), - excludePatterns: z.array(z.string()).optional().default([]) + path: z.string().describe(DirectoryPathDescription), + pattern: z.string().describe(SearchPatternDescription), + excludePatterns: z.array(z.string()).optional().default([]).describe(ExcludePatternsDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: true } @@ -662,7 +681,7 @@ server.registerTool( "and type. This tool is perfect for understanding file characteristics " + "without reading the actual content. Only works within allowed directories.", inputSchema: { - path: z.string() + path: z.string().describe(FileOrDirectoryPathDescription) }, outputSchema: { content: z.string() }, annotations: { readOnlyHint: true }