Skip to content

fix(filesystem): describe tool parameters #4317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
go165 wants to merge 1 commit into
base: main
Choose a base branch
from
+114 −34
Open

fix(filesystem): describe tool parameters #4317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions src/filesystem/__tests__/tool-schema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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([]);
});
});
87 changes: 53 additions & 34 deletions src/filesystem/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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({
Expand All @@ -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
Expand Down Expand Up @@ -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")
},
Expand All @@ -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({
Expand Down Expand Up @@ -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 }
Expand All @@ -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() },
Expand All @@ -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 }
Expand All @@ -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 }
Expand Down Expand Up @@ -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() },
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand All @@ -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 }
Expand Down
Loading