diff --git a/.gitignore b/.gitignore index 6ce55639..eefb0a79 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ client/tsconfig.app.tsbuildinfo client/tsconfig.node.tsbuildinfo cli/build test-output +tool-test-output # symlinked by `npm run link:sdk`: sdk client/playwright-report/ diff --git a/cli/package.json b/cli/package.json index 6bb55253..f29586e7 100644 --- a/cli/package.json +++ b/cli/package.json @@ -17,7 +17,9 @@ "scripts": { "build": "tsc", "postbuild": "node scripts/make-executable.js", - "test": "node scripts/cli-tests.js" + "test": "node scripts/cli-tests.js && node scripts/cli-tool-tests.js", + "test:cli": "node scripts/cli-tests.js", + "test:cli-tools": "node scripts/cli-tool-tests.js" }, "devDependencies": {}, "dependencies": { diff --git a/cli/scripts/cli-tests.js b/cli/scripts/cli-tests.js index f714cb6a..554a5262 100755 --- a/cli/scripts/cli-tests.js +++ b/cli/scripts/cli-tests.js @@ -37,9 +37,6 @@ console.log(`${colors.BLUE}- Environment variables (-e)${colors.NC}`); console.log(`${colors.BLUE}- Config file (--config)${colors.NC}`); console.log(`${colors.BLUE}- Server selection (--server)${colors.NC}`); console.log(`${colors.BLUE}- Method selection (--method)${colors.NC}`); -console.log( - `${colors.BLUE}- Tool-related options (--tool-name, --tool-arg)${colors.NC}`, -); console.log(`${colors.BLUE}- Resource-related options (--uri)${colors.NC}`); console.log( `${colors.BLUE}- Prompt-related options (--prompt-name, --prompt-args)${colors.NC}`, @@ -533,65 +530,6 @@ async function runTests() { "tools/list", ); - console.log( - `\n${colors.YELLOW}=== Running Tool-Related Tests ===${colors.NC}`, - ); - - // Test 12: CLI mode with tool call - await runBasicTest( - "tool_call", - TEST_CMD, - ...TEST_ARGS, - "--cli", - "--method", - "tools/call", - "--tool-name", - "echo", - "--tool-arg", - "message=Hello", - ); - - // Test 13: CLI mode with tool call but missing tool name (should fail) - await runErrorTest( - "missing_tool_name", - TEST_CMD, - ...TEST_ARGS, - "--cli", - "--method", - "tools/call", - "--tool-arg", - "message=Hello", - ); - - // Test 14: CLI mode with tool call but invalid tool args format (should fail) - await runErrorTest( - "invalid_tool_args", - TEST_CMD, - ...TEST_ARGS, - "--cli", - "--method", - "tools/call", - "--tool-name", - "echo", - "--tool-arg", - "invalid_format", - ); - - // Test 15: CLI mode with multiple tool args - await runBasicTest( - "multiple_tool_args", - TEST_CMD, - ...TEST_ARGS, - "--cli", - "--method", - "tools/call", - "--tool-name", - "add", - "--tool-arg", - "a=1", - "b=2", - ); - console.log( `\n${colors.YELLOW}=== Running Resource-Related Tests ===${colors.NC}`, ); diff --git a/cli/scripts/cli-tool-tests.js b/cli/scripts/cli-tool-tests.js new file mode 100644 index 00000000..b06aea94 --- /dev/null +++ b/cli/scripts/cli-tool-tests.js @@ -0,0 +1,614 @@ +#!/usr/bin/env node + +// Colors for output +const colors = { + GREEN: "\x1b[32m", + YELLOW: "\x1b[33m", + RED: "\x1b[31m", + BLUE: "\x1b[34m", + ORANGE: "\x1b[33m", + NC: "\x1b[0m", // No Color +}; + +import fs from "fs"; +import path from "path"; +import { spawn } from "child_process"; +import os from "os"; +import { fileURLToPath } from "url"; + +// Get directory paths with ESM compatibility +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +// Track test results +let PASSED_TESTS = 0; +let FAILED_TESTS = 0; +let SKIPPED_TESTS = 0; +let TOTAL_TESTS = 0; + +console.log(`${colors.YELLOW}=== MCP Inspector CLI Tool Tests ===${colors.NC}`); +console.log( + `${colors.BLUE}This script tests the MCP Inspector CLI's tool-related functionality:${colors.NC}`, +); +console.log(`${colors.BLUE}- Tool discovery and listing${colors.NC}`); +console.log( + `${colors.BLUE}- JSON argument parsing (strings, numbers, booleans, objects, arrays)${colors.NC}`, +); +console.log(`${colors.BLUE}- Tool schema validation${colors.NC}`); +console.log( + `${colors.BLUE}- Tool execution with various argument types${colors.NC}`, +); +console.log( + `${colors.BLUE}- Error handling for invalid tools and arguments${colors.NC}`, +); +console.log(`\n`); + +// Get directory paths +const SCRIPTS_DIR = __dirname; +const PROJECT_ROOT = path.join(SCRIPTS_DIR, "../../"); +const BUILD_DIR = path.resolve(SCRIPTS_DIR, "../build"); + +// Define the test server command using npx +const TEST_CMD = "npx"; +const TEST_ARGS = ["@modelcontextprotocol/server-everything"]; + +// Create output directory for test results +const OUTPUT_DIR = path.join(SCRIPTS_DIR, "tool-test-output"); +if (!fs.existsSync(OUTPUT_DIR)) { + fs.mkdirSync(OUTPUT_DIR, { recursive: true }); +} + +// Create a temporary directory for test files +const TEMP_DIR = path.join(os.tmpdir(), "mcp-inspector-tool-tests"); +fs.mkdirSync(TEMP_DIR, { recursive: true }); + +// Track servers for cleanup +let runningServers = []; + +process.on("exit", () => { + try { + fs.rmSync(TEMP_DIR, { recursive: true, force: true }); + } catch (err) { + console.error( + `${colors.RED}Failed to remove temp directory: ${err.message}${colors.NC}`, + ); + } + + runningServers.forEach((server) => { + try { + process.kill(-server.pid); + } catch (e) {} + }); +}); + +process.on("SIGINT", () => { + runningServers.forEach((server) => { + try { + process.kill(-server.pid); + } catch (e) {} + }); + process.exit(1); +}); + +// Function to run a basic test +async function runBasicTest(testName, ...args) { + const outputFile = path.join( + OUTPUT_DIR, + `${testName.replace(/\//g, "_")}.log`, + ); + + console.log(`\n${colors.YELLOW}Testing: ${testName}${colors.NC}`); + TOTAL_TESTS++; + + // Run the command and capture output + console.log( + `${colors.BLUE}Command: node ${BUILD_DIR}/cli.js ${args.join(" ")}${colors.NC}`, + ); + + try { + // Create a write stream for the output file + const outputStream = fs.createWriteStream(outputFile); + + // Spawn the process + return new Promise((resolve) => { + const child = spawn("node", [path.join(BUILD_DIR, "cli.js"), ...args], { + stdio: ["ignore", "pipe", "pipe"], + }); + + const timeout = setTimeout(() => { + console.log(`${colors.YELLOW}Test timed out: ${testName}${colors.NC}`); + child.kill(); + }, 10000); + + // Pipe stdout and stderr to the output file + child.stdout.pipe(outputStream); + child.stderr.pipe(outputStream); + + // Also capture output for display + let output = ""; + child.stdout.on("data", (data) => { + output += data.toString(); + }); + child.stderr.on("data", (data) => { + output += data.toString(); + }); + + child.on("close", (code) => { + clearTimeout(timeout); + outputStream.end(); + + if (code === 0) { + console.log(`${colors.GREEN}✓ Test passed: ${testName}${colors.NC}`); + console.log(`${colors.BLUE}First few lines of output:${colors.NC}`); + const firstFewLines = output + .split("\n") + .slice(0, 5) + .map((line) => ` ${line}`) + .join("\n"); + console.log(firstFewLines); + PASSED_TESTS++; + resolve(true); + } else { + console.log(`${colors.RED}✗ Test failed: ${testName}${colors.NC}`); + console.log(`${colors.RED}Error output:${colors.NC}`); + console.log( + output + .split("\n") + .map((line) => ` ${line}`) + .join("\n"), + ); + FAILED_TESTS++; + + // Stop after any error is encountered + console.log( + `${colors.YELLOW}Stopping tests due to error. Please validate and fix before continuing.${colors.NC}`, + ); + process.exit(1); + } + }); + }); + } catch (error) { + console.error( + `${colors.RED}Error running test: ${error.message}${colors.NC}`, + ); + FAILED_TESTS++; + process.exit(1); + } +} + +// Function to run an error test (expected to fail) +async function runErrorTest(testName, ...args) { + const outputFile = path.join( + OUTPUT_DIR, + `${testName.replace(/\//g, "_")}.log`, + ); + + console.log(`\n${colors.YELLOW}Testing error case: ${testName}${colors.NC}`); + TOTAL_TESTS++; + + // Run the command and capture output + console.log( + `${colors.BLUE}Command: node ${BUILD_DIR}/cli.js ${args.join(" ")}${colors.NC}`, + ); + + try { + // Create a write stream for the output file + const outputStream = fs.createWriteStream(outputFile); + + // Spawn the process + return new Promise((resolve) => { + const child = spawn("node", [path.join(BUILD_DIR, "cli.js"), ...args], { + stdio: ["ignore", "pipe", "pipe"], + }); + + const timeout = setTimeout(() => { + console.log( + `${colors.YELLOW}Error test timed out: ${testName}${colors.NC}`, + ); + child.kill(); + }, 10000); + + // Pipe stdout and stderr to the output file + child.stdout.pipe(outputStream); + child.stderr.pipe(outputStream); + + // Also capture output for display + let output = ""; + child.stdout.on("data", (data) => { + output += data.toString(); + }); + child.stderr.on("data", (data) => { + output += data.toString(); + }); + + child.on("close", (code) => { + clearTimeout(timeout); + outputStream.end(); + + // For error tests, we expect a non-zero exit code + if (code !== 0) { + console.log( + `${colors.GREEN}✓ Error test passed: ${testName}${colors.NC}`, + ); + console.log(`${colors.BLUE}Error output (expected):${colors.NC}`); + const firstFewLines = output + .split("\n") + .slice(0, 5) + .map((line) => ` ${line}`) + .join("\n"); + console.log(firstFewLines); + PASSED_TESTS++; + resolve(true); + } else { + console.log( + `${colors.RED}✗ Error test failed: ${testName} (expected error but got success)${colors.NC}`, + ); + console.log(`${colors.RED}Output:${colors.NC}`); + console.log( + output + .split("\n") + .map((line) => ` ${line}`) + .join("\n"), + ); + FAILED_TESTS++; + + // Stop after any error is encountered + console.log( + `${colors.YELLOW}Stopping tests due to error. Please validate and fix before continuing.${colors.NC}`, + ); + process.exit(1); + } + }); + }); + } catch (error) { + console.error( + `${colors.RED}Error running test: ${error.message}${colors.NC}`, + ); + FAILED_TESTS++; + process.exit(1); + } +} + +// Run all tests +async function runTests() { + console.log( + `\n${colors.YELLOW}=== Running Tool Discovery Tests ===${colors.NC}`, + ); + + // Test 1: List available tools + await runBasicTest( + "tool_discovery_list", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/list", + ); + + console.log( + `\n${colors.YELLOW}=== Running JSON Argument Parsing Tests ===${colors.NC}`, + ); + + // Test 2: String arguments (backward compatibility) + await runBasicTest( + "json_args_string", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=hello world", + ); + + // Test 3: Number arguments + await runBasicTest( + "json_args_number_integer", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "add", + "--tool-arg", + "a=42", + "b=58", + ); + + // Test 4: Number arguments with decimals (using add tool with decimal numbers) + await runBasicTest( + "json_args_number_decimal", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "add", + "--tool-arg", + "a=19.99", + "b=20.01", + ); + + // Test 5: Boolean arguments - true + await runBasicTest( + "json_args_boolean_true", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "annotatedMessage", + "--tool-arg", + "messageType=success", + "includeImage=true", + ); + + // Test 6: Boolean arguments - false + await runBasicTest( + "json_args_boolean_false", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "annotatedMessage", + "--tool-arg", + "messageType=error", + "includeImage=false", + ); + + // Test 7: Null arguments (using echo with string "null") + await runBasicTest( + "json_args_null", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + 'message="null"', + ); + + // Test 14: Multiple arguments with mixed types (using add tool) + await runBasicTest( + "json_args_multiple_mixed", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "add", + "--tool-arg", + "a=42.5", + "b=57.5", + ); + + console.log( + `\n${colors.YELLOW}=== Running JSON Parsing Edge Cases ===${colors.NC}`, + ); + + // Test 15: Invalid JSON should fall back to string + await runBasicTest( + "json_args_invalid_fallback", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message={invalid json}", + ); + + // Test 16: Empty string value + await runBasicTest( + "json_args_empty_value", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + 'message=""', + ); + + // Test 17: Special characters in strings + await runBasicTest( + "json_args_special_chars", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + 'message="C:\\\\Users\\\\test"', + ); + + // Test 18: Unicode characters + await runBasicTest( + "json_args_unicode", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + 'message="🚀🎉✨"', + ); + + // Test 19: Arguments with equals signs in values + await runBasicTest( + "json_args_equals_in_value", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=2+2=4", + ); + + // Test 20: Base64-like strings + await runBasicTest( + "json_args_base64_like", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0=", + ); + + console.log( + `\n${colors.YELLOW}=== Running Tool Error Handling Tests ===${colors.NC}`, + ); + + // Test 21: Non-existent tool + await runErrorTest( + "tool_error_nonexistent", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "nonexistent_tool", + "--tool-arg", + "message=test", + ); + + // Test 22: Missing tool name + await runErrorTest( + "tool_error_missing_name", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-arg", + "message=test", + ); + + // Test 23: Invalid tool argument format + await runErrorTest( + "tool_error_invalid_arg_format", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "invalid_format_no_equals", + ); + + console.log( + `\n${colors.YELLOW}=== Running Prompt JSON Argument Tests ===${colors.NC}`, + ); + + // Test 24: Prompt with JSON arguments + await runBasicTest( + "prompt_json_args_mixed", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "prompts/get", + "--prompt-name", + "complex_prompt", + "--prompt-args", + "temperature=0.7", + 'style="concise"', + 'options={"format":"json","max_tokens":100}', + ); + + // Test 25: Prompt with simple arguments + await runBasicTest( + "prompt_json_args_simple", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "prompts/get", + "--prompt-name", + "simple_prompt", + "--prompt-args", + "name=test", + "count=5", + ); + + console.log( + `\n${colors.YELLOW}=== Running Backward Compatibility Tests ===${colors.NC}`, + ); + + // Test 26: Ensure existing string-only usage still works + await runBasicTest( + "backward_compatibility_strings", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "echo", + "--tool-arg", + "message=hello", + ); + + // Test 27: Multiple string arguments (existing pattern) - using add tool + await runBasicTest( + "backward_compatibility_multiple_strings", + TEST_CMD, + ...TEST_ARGS, + "--cli", + "--method", + "tools/call", + "--tool-name", + "add", + "--tool-arg", + "a=10", + "b=20", + ); + + // Print test summary + console.log(`\n${colors.YELLOW}=== Test Summary ===${colors.NC}`); + console.log(`${colors.GREEN}Passed: ${PASSED_TESTS}${colors.NC}`); + console.log(`${colors.RED}Failed: ${FAILED_TESTS}${colors.NC}`); + console.log(`${colors.ORANGE}Skipped: ${SKIPPED_TESTS}${colors.NC}`); + console.log(`Total: ${TOTAL_TESTS}`); + console.log( + `${colors.BLUE}Detailed logs saved to: ${OUTPUT_DIR}${colors.NC}`, + ); + + console.log(`\n${colors.GREEN}All tool tests completed!${colors.NC}`); +} + +// Run all tests +runTests().catch((error) => { + console.error( + `${colors.RED}Tests failed with error: ${error.message}${colors.NC}`, + ); + process.exit(1); +}); diff --git a/cli/src/client/prompts.ts b/cli/src/client/prompts.ts index 0b237496..87014617 100644 --- a/cli/src/client/prompts.ts +++ b/cli/src/client/prompts.ts @@ -1,6 +1,16 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { McpResponse } from "./types.js"; +// JSON value type matching the client utils +type JsonValue = + | string + | number + | boolean + | null + | undefined + | JsonValue[] + | { [key: string]: JsonValue }; + // List available prompts export async function listPrompts(client: Client): Promise { try { @@ -17,12 +27,26 @@ export async function listPrompts(client: Client): Promise { export async function getPrompt( client: Client, name: string, - args?: Record, + args?: Record, ): Promise { try { + // Convert all arguments to strings for prompt arguments + const stringArgs: Record = {}; + if (args) { + for (const [key, value] of Object.entries(args)) { + if (typeof value === "string") { + stringArgs[key] = value; + } else if (value === null || value === undefined) { + stringArgs[key] = String(value); + } else { + stringArgs[key] = JSON.stringify(value); + } + } + } + const response = await client.getPrompt({ name, - arguments: args || {}, + arguments: stringArgs, }); return response; diff --git a/cli/src/client/tools.ts b/cli/src/client/tools.ts index acdb4871..9da0b4d9 100644 --- a/cli/src/client/tools.ts +++ b/cli/src/client/tools.ts @@ -2,6 +2,16 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { Tool } from "@modelcontextprotocol/sdk/types.js"; import { McpResponse } from "./types.js"; +// JSON value type matching the client utils +type JsonValue = + | string + | number + | boolean + | null + | undefined + | JsonValue[] + | { [key: string]: JsonValue }; + type JsonSchemaType = { type: "string" | "number" | "integer" | "boolean" | "array" | "object"; description?: string; @@ -20,7 +30,10 @@ export async function listTools(client: Client): Promise { } } -function convertParameterValue(value: string, schema: JsonSchemaType): unknown { +function convertParameterValue( + value: string, + schema: JsonSchemaType, +): JsonValue { if (!value) { return value; } @@ -35,7 +48,7 @@ function convertParameterValue(value: string, schema: JsonSchemaType): unknown { if (schema.type === "object" || schema.type === "array") { try { - return JSON.parse(value); + return JSON.parse(value) as JsonValue; } catch (error) { return value; } @@ -47,8 +60,8 @@ function convertParameterValue(value: string, schema: JsonSchemaType): unknown { function convertParameters( tool: Tool, params: Record, -): Record { - const result: Record = {}; +): Record { + const result: Record = {}; const properties = tool.inputSchema.properties || {}; for (const [key, value] of Object.entries(params)) { @@ -68,18 +81,29 @@ function convertParameters( export async function callTool( client: Client, name: string, - args: Record, + args: Record, ): Promise { try { const toolsResponse = await listTools(client); const tools = toolsResponse.tools as Tool[]; const tool = tools.find((t) => t.name === name); - let convertedArgs: Record = args; + let convertedArgs: Record = args; if (tool) { - // Convert parameters based on the tool's schema - convertedArgs = convertParameters(tool, args); + // Convert parameters based on the tool's schema, but only for string values + // since we now accept pre-parsed values from the CLI + const stringArgs: Record = {}; + for (const [key, value] of Object.entries(args)) { + if (typeof value === "string") { + stringArgs[key] = value; + } + } + + if (Object.keys(stringArgs).length > 0) { + const convertedStringArgs = convertParameters(tool, stringArgs); + convertedArgs = { ...args, ...convertedStringArgs }; + } } const response = await client.callTool({ diff --git a/cli/src/index.ts b/cli/src/index.ts index 2b0c4f53..563160bc 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -20,15 +20,25 @@ import { import { handleError } from "./error-handler.js"; import { createTransport, TransportOptions } from "./transport.js"; +// JSON value type for CLI arguments +type JsonValue = + | string + | number + | boolean + | null + | undefined + | JsonValue[] + | { [key: string]: JsonValue }; + type Args = { target: string[]; method?: string; promptName?: string; - promptArgs?: Record; + promptArgs?: Record; uri?: string; logLevel?: LogLevel; toolName?: string; - toolArg?: Record; + toolArg?: Record; transport?: "sse" | "stdio" | "http"; }; @@ -162,8 +172,8 @@ async function callMethod(args: Args): Promise { function parseKeyValuePair( value: string, - previous: Record = {}, -): Record { + previous: Record = {}, +): Record { const parts = value.split("="); const key = parts[0]; const val = parts.slice(1).join("="); @@ -174,7 +184,16 @@ function parseKeyValuePair( ); } - return { ...previous, [key as string]: val }; + // Try to parse as JSON first + let parsedValue: JsonValue; + try { + parsedValue = JSON.parse(val) as JsonValue; + } catch { + // If JSON parsing fails, keep as string + parsedValue = val; + } + + return { ...previous, [key as string]: parsedValue }; } function parseArgs(): Args {