Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 27 additions & 0 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,13 @@ import {
GetPromptsArgsSchema,
GetRecentToolCallsArgsSchema,
WritePdfArgsSchema,
toolArgSchemas,
} from './tools/schemas.js';
import {
detectUnsupportedParams,
getSupportedParams,
buildUnsupportedParamsWarning,
} from './utils/unsupportedParams.js';
import { getConfig, setConfigValue } from './tools/config.js';
import { getUsageStats } from './tools/usage.js';
import { giveFeedbackToDesktopCommander } from './tools/feedback.js';
Expand Down Expand Up @@ -1559,6 +1565,27 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
result = await processDockerPrompt(result, name);
}

// If the caller sent parameters this tool does not support, Zod silently
// strips them. Prepend a corrective warning so the model knows they were
// ignored and which parameters are actually supported.
try {
const argSchema = toolArgSchemas[name];
if (argSchema && result && Array.isArray((result as any).content)) {
const unsupported = detectUnsupportedParams(args, argSchema);
if (unsupported.length > 0) {
const warning = buildUnsupportedParamsWarning(
name, unsupported, getSupportedParams(argSchema)
);
(result as any).content = [
{ type: "text", text: warning },
...(result as any).content,
];
}
}
} catch {
// Never let the advisory warning break an otherwise-successful call.
}

return result;
} catch (error) {
isError = true;
Expand Down
35 changes: 35 additions & 0 deletions src/tools/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,38 @@ export const TrackUiEventArgsSchema = z.object({
component: z.string().optional().default('file_preview'),
params: z.record(z.union([z.string(), z.number(), z.boolean(), z.null()])).optional().default({}),
});

/**
* Map of tool name -> argument schema, used by the dispatcher to detect and warn
* about parameters a caller sent that the tool does not support. Keep in sync
* with the tool definitions in server.ts.
*/
export const toolArgSchemas: Record<string, z.ZodTypeAny> = {
get_config: GetConfigArgsSchema,
set_config_value: SetConfigValueArgsSchema,
read_file: ReadFileArgsSchema,
read_multiple_files: ReadMultipleFilesArgsSchema,
write_file: WriteFileArgsSchema,
write_pdf: WritePdfArgsSchema,
create_directory: CreateDirectoryArgsSchema,
list_directory: ListDirectoryArgsSchema,
move_file: MoveFileArgsSchema,
start_search: StartSearchArgsSchema,
get_more_search_results: GetMoreSearchResultsArgsSchema,
stop_search: StopSearchArgsSchema,
list_searches: ListSearchesArgsSchema,
get_file_info: GetFileInfoArgsSchema,
edit_block: EditBlockArgsSchema,
start_process: StartProcessArgsSchema,
read_process_output: ReadProcessOutputArgsSchema,
interact_with_process: InteractWithProcessArgsSchema,
force_terminate: ForceTerminateArgsSchema,
list_sessions: ListSessionsArgsSchema,
list_processes: ListProcessesArgsSchema,
kill_process: KillProcessArgsSchema,
get_usage_stats: GetUsageStatsArgsSchema,
get_recent_tool_calls: GetRecentToolCallsArgsSchema,
give_feedback_to_desktop_commander: GiveFeedbackArgsSchema,
get_prompts: GetPromptsArgsSchema,
track_ui_event: TrackUiEventArgsSchema,
};
68 changes: 68 additions & 0 deletions src/utils/unsupportedParams.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Utility for detecting parameters a caller sent that a tool's Zod schema does
* not accept.
*
* Zod's default `z.object()` silently strips unknown keys, so a model that
* invents a parameter (e.g. `view_range`) gets a normal-looking response with no
* signal that its input was ignored. These helpers let the dispatcher surface a
* corrective warning instead.
*
* Detection is top-level only. That is sufficient here because none of the tool
* arg schemas use a top-level `.passthrough()`/catchall — free-form fields
* (`options`, `params`, `pdfOptions`) are named keys whose contents are
* intentionally unvalidated.
*/

/**
* Resolve a Zod schema down to the field map of its underlying object schema,
* unwrapping wrappers (effects/optional/default/nullable/branded) defensively.
* Returns null when the schema is not (and does not wrap) a plain object schema,
* in which case we cannot determine the supported parameters and must not warn.
*/
function getObjectShape(schema: any): Record<string, unknown> | null {
let current = schema;
for (let i = 0; current && current._def && i < 10; i++) {
const typeName = current._def.typeName;
if (typeName === 'ZodObject') {
const shape = typeof current.shape === 'function' ? current.shape() : current.shape;
return shape ?? null;
}
if (typeName === 'ZodEffects') current = current._def.schema;
else if (typeName === 'ZodOptional' || typeName === 'ZodNullable' || typeName === 'ZodDefault') {
current = typeof current._def.innerType === 'function' ? current._def.innerType() : current._def.innerType;
} else if (typeName === 'ZodBranded') current = current._def.type;
else return null;
}
return null;
}

/** List the parameter names a tool schema accepts (empty array if it takes none). */
export function getSupportedParams(schema: unknown): string[] {
const shape = getObjectShape(schema);
return shape ? Object.keys(shape) : [];
}

/**
* Return the names of top-level keys in `args` that the schema does not accept.
* Returns [] when args is not a plain object or the schema's params can't be
* determined (so we never warn on something we can't reason about).
*/
export function detectUnsupportedParams(args: unknown, schema: unknown): string[] {
if (!args || typeof args !== 'object' || Array.isArray(args)) return [];
const shape = getObjectShape(schema);
if (shape === null) return [];
const supported = new Set(Object.keys(shape));
return Object.keys(args as Record<string, unknown>).filter((k) => !supported.has(k));
}

/** Build the corrective warning string shown to the model. */
export function buildUnsupportedParamsWarning(
toolName: string,
unsupported: string[],
supported: string[],
): string {
const ignored = unsupported.join(', ');
const supportedList = supported.length > 0 ? supported.join(', ') : '(none)';
return `You sent parameters not supported by this tool, which were ignored: ${ignored}. `
+ `Supported parameters for ${toolName}: ${supportedList}.`;
}
147 changes: 147 additions & 0 deletions test/integration/read-file-unknown-params.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/**
* Integration test: what does read_file return when the caller sends parameters
* the schema does not accept?
*
* Drives the real MCP server over stdio (exactly like an LLM client), so every
* assertion is against the actual CallToolResult the model receives.
*
* Documents and validates behavior:
* 1. Unsupported params (e.g. view_range, foo_bar) are stripped, the read still
* succeeds (isError falsy), and a corrective warning is PREPENDED as the
* first content block, naming the ignored params and listing the supported
* ones. This is the unsupported-parameter warning feature.
* 2. Wrong type on a known param -> dispatcher-shaped isError: true.
* 3. Missing required param (path) -> dispatcher-shaped isError: true.
*/

import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';
import assert from 'assert';
import fs from 'fs/promises';
import path from 'path';
import { fileURLToPath } from 'url';

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const PROJECT_ROOT = path.resolve(__dirname, '..', '..');
const TEST_DIR = path.join(__dirname, 'test_read_file_unknown_params');
const TEST_FILE = path.join(TEST_DIR, 'numbered.txt');
const LINE_COUNT = 50;

async function callTool(client, name, args) {
return client.callTool({ name, arguments: args }, undefined, { timeout: 30000 });
}

function textOf(result) {
return result?.content?.find?.((c) => c.type === 'text')?.text ?? '';
}

async function createMcpClient() {
const transport = new StdioClientTransport({
command: process.execPath,
args: [path.join(PROJECT_ROOT, 'dist/index.js'), '--no-onboarding'],
cwd: PROJECT_ROOT,
stderr: 'pipe',
env: { ...process.env, DESKTOP_COMMANDER_DISABLE_TELEMETRY: 'true' },
});

const client = new Client(
{ name: 'desktop-commander-unknown-params-test', version: '1.0.0' },
{ capabilities: {} }
);
await client.connect(transport, { timeout: 30000 });
return client;
}

async function setup(client) {
await fs.rm(TEST_DIR, { recursive: true, force: true });
await fs.mkdir(TEST_DIR, { recursive: true });
const lines = Array.from({ length: LINE_COUNT }, (_, i) => `line-${i + 1}`);
await fs.writeFile(TEST_FILE, lines.join('\n'));

const original = await callTool(client, 'get_config', {});
const entries = original.structuredContent?.entries;
assert.ok(Array.isArray(entries), 'get_config should return structured entries');
const originalConfig = Object.fromEntries(
entries.filter((e) => e && e.editable === true).map((e) => [e.key, e.value])
);

const set = await callTool(client, 'set_config_value', {
key: 'allowedDirectories', value: [TEST_DIR], origin: 'llm',
});
assert.ok(!set.isError, 'set_config_value allowedDirectories should succeed');
return originalConfig;
}

async function teardown(client, originalConfig) {
for (const [key, value] of Object.entries(originalConfig)) {
await callTool(client, 'set_config_value', { key, value, origin: 'llm' });
}
await fs.rm(TEST_DIR, { recursive: true, force: true });
}

async function main() {
console.log('===== read_file Unknown-Parameter Behavior Integration Test =====');
const client = await createMcpClient();
const originalConfig = await setup(client);

try {
// --- Case 0: a clean call gets NO warning (guards against over-firing) ---
const clean = await callTool(client, 'read_file', { path: TEST_FILE, offset: 0, length: 3 });
const cleanFirst = (clean.content || []).find((c) => c.type === 'text')?.text || '';
assert.ok(!clean.isError, 'Case 0: valid call should succeed');
assert.ok(!/not supported|ignored/i.test(cleanFirst),
'Case 0: valid call should NOT get an unsupported-params warning');
assert.ok(/line-1\b/.test(cleanFirst), 'Case 0: valid call returns file content first');
console.log('[Case 0] PASS - clean call has no warning');

// --- Case 1: unsupported parameters are stripped, with a corrective warning ---
const unknown = await callTool(client, 'read_file', {
path: TEST_FILE,
view_range: [5, 10], // not a supported param
foo_bar: true, // clearly bogus
});
const blocks = (unknown.content || []).filter((c) => c.type === 'text').map((c) => c.text);
const firstBlock = blocks[0] || '';
const joined = blocks.join('\n');
console.log('\n[Case 1] unsupported params -> isError:', !!unknown.isError);

// 1a. The call is NOT rejected; unsupported params don't fail the read.
assert.ok(!unknown.isError, 'Case 1: unsupported params should NOT cause isError');
// 1b. A corrective warning is PREPENDED as the first content block.
assert.ok(/not supported|ignored/i.test(firstBlock),
'Case 1: first content block should be the unsupported-params warning');
// 1c. The warning names exactly the params that were ignored.
assert.ok(/view_range/.test(firstBlock) && /foo_bar/.test(firstBlock),
'Case 1: warning should name the ignored params (view_range, foo_bar)');
// 1d. The warning lists the supported params (the corrective part).
assert.ok(/path/.test(firstBlock) && /offset/.test(firstBlock) && /length/.test(firstBlock),
'Case 1: warning should list the supported params');
// 1e. The read still happened (from the start, since the params were ignored).
assert.ok(/line-1\b/.test(joined), 'Case 1: file content still returned (read from start)');
console.log('[Case 1] PASS - ignored params named + supported list returned, read still served');

// --- Case 2: wrong type on a known param -> shaped isError ---
const badType = await callTool(client, 'read_file', { path: TEST_FILE, offset: 'not-a-number' });
console.log('[Case 2] wrong type -> isError:', !!badType.isError);
assert.ok(badType.isError, 'Case 2: wrong type should return isError: true');
assert.ok(/offset/i.test(textOf(badType)), 'Case 2: error should reference offset');
console.log('[Case 2] PASS - wrong type surfaced to the model');

// --- Case 3: missing required param -> shaped isError ---
const missing = await callTool(client, 'read_file', { offset: 2 });
console.log('[Case 3] missing path -> isError:', !!missing.isError);
assert.ok(missing.isError, 'Case 3: missing path should return isError: true');
console.log('[Case 3] PASS - missing required param surfaced to the model');

console.log('\nAll assertions passed. Current read_file param behavior is documented.');
} finally {
await teardown(client, originalConfig);
await client.close();
}
}

main().catch((err) => {
console.error('\nTEST FAILED:', err && err.message ? err.message : err);
process.exit(1);
});
Loading