diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index a4fd6c52..6e916eba 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -18,10 +18,6 @@ jobs: (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) || (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) || (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude'))) - ) && - ( - github.actor == 'ihrpr' || - github.actor == 'olaservo' ) runs-on: ubuntu-latest permissions: @@ -29,6 +25,7 @@ jobs: pull-requests: read issues: read id-token: write + actions: read steps: - name: Checkout repository uses: actions/checkout@v4 @@ -41,24 +38,16 @@ jobs: with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - # Optional: Specify model (defaults to Claude Sonnet 4, uncomment for Claude Opus 4) - # model: "claude-opus-4-20250514" + # Allow Claude to read CI results on PRs + additional_permissions: | + actions: read - # Optional: Customize the trigger phrase (default: @claude) - # trigger_phrase: "/claude" + # Trigger when assigned to an issue + assignee_trigger: "claude" - # Optional: Trigger when specific user is assigned to an issue - # assignee_trigger: "claude-bot" + # Allow Claude to run bash + # This should be safe given the repo is already public + allowed_tools: "Bash" - # Optional: Allow Claude to run specific commands - # allowed_tools: "Bash(npm install),Bash(npm run build),Bash(npm run test:*),Bash(npm run lint:*)" - - # Optional: Add custom instructions for Claude to customize its behavior for your project - # custom_instructions: | - # Follow our coding standards - # Ensure all new code has tests - # Use TypeScript for new files - - # Optional: Custom environment variables for Claude - # claude_env: | - # NODE_ENV: test + custom_instructions: | + If posting a comment to GitHub, give a concise summary of the comment at the top and put all the details in a
block. 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/Dockerfile b/Dockerfile index f36fb8bb..d66091d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build stage -FROM node:24-slim AS builder +FROM node:current-alpine3.22 AS builder # Set working directory WORKDIR /app @@ -49,4 +49,4 @@ ENV SERVER_PORT=6277 EXPOSE ${CLIENT_PORT} ${SERVER_PORT} # Use ENTRYPOINT with CMD for arguments -ENTRYPOINT ["npm", "start"] \ No newline at end of file +ENTRYPOINT ["npm", "start"] diff --git a/README.md b/README.md index 1f6cb822..a0716556 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,14 @@ npx @modelcontextprotocol/inspector The server will start up and the UI will be accessible at `http://localhost:6274`. +### Docker Container + +You can also start it in a Docker container with the following command: + +```bash +docker run --rm --network host -p 6274:6274 -p 6277:6277 ghcr.io/modelcontextprotocol/inspector:latest +``` + ### From an MCP server repository To inspect an MCP server implementation, there's no need to clone this repo. Instead, use `npx`. For example, if your server is built at `build/index.js`: @@ -387,6 +395,9 @@ npx @modelcontextprotocol/inspector --cli node build/index.js --method tools/lis # Call a specific tool npx @modelcontextprotocol/inspector --cli node build/index.js --method tools/call --tool-name mytool --tool-arg key=value --tool-arg another=value2 +# Call a tool with JSON arguments +npx @modelcontextprotocol/inspector --cli node build/index.js --method tools/call --tool-name mytool --tool-arg 'options={"format": "json", "max_tokens": 100}' + # List available resources npx @modelcontextprotocol/inspector --cli node build/index.js --method resources/list diff --git a/cli/package.json b/cli/package.json index 2f0f53b6..e274bbdf 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 { diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index 7518f814..624583dc 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -877,4 +877,129 @@ describe("useConnection", () => { }); }); }); + + describe("MCP_PROXY_FULL_ADDRESS Configuration", () => { + beforeEach(() => { + jest.clearAllMocks(); + // Reset the mock transport objects + mockSSETransport.url = undefined; + mockSSETransport.options = undefined; + mockStreamableHTTPTransport.url = undefined; + mockStreamableHTTPTransport.options = undefined; + }); + + test("sends proxyFullAddress query parameter for stdio transport when configured", async () => { + const propsWithProxyFullAddress = { + ...defaultProps, + transportType: "stdio" as const, + command: "test-command", + args: "test-args", + env: {}, + config: { + ...DEFAULT_INSPECTOR_CONFIG, + MCP_PROXY_FULL_ADDRESS: { + ...DEFAULT_INSPECTOR_CONFIG.MCP_PROXY_FULL_ADDRESS, + value: "https://example.com/inspector/mcp_proxy", + }, + }, + }; + + const { result } = renderHook(() => + useConnection(propsWithProxyFullAddress), + ); + + await act(async () => { + await result.current.connect(); + }); + + // Check that the URL contains the proxyFullAddress parameter + expect(mockSSETransport.url?.searchParams.get("proxyFullAddress")).toBe( + "https://example.com/inspector/mcp_proxy", + ); + }); + + test("sends proxyFullAddress query parameter for sse transport when configured", async () => { + const propsWithProxyFullAddress = { + ...defaultProps, + transportType: "sse" as const, + sseUrl: "http://localhost:8080", + config: { + ...DEFAULT_INSPECTOR_CONFIG, + MCP_PROXY_FULL_ADDRESS: { + ...DEFAULT_INSPECTOR_CONFIG.MCP_PROXY_FULL_ADDRESS, + value: "https://example.com/inspector/mcp_proxy", + }, + }, + }; + + const { result } = renderHook(() => + useConnection(propsWithProxyFullAddress), + ); + + await act(async () => { + await result.current.connect(); + }); + + // Check that the URL contains the proxyFullAddress parameter + expect(mockSSETransport.url?.searchParams.get("proxyFullAddress")).toBe( + "https://example.com/inspector/mcp_proxy", + ); + }); + + test("does not send proxyFullAddress parameter when MCP_PROXY_FULL_ADDRESS is empty", async () => { + const propsWithEmptyProxy = { + ...defaultProps, + transportType: "stdio" as const, + command: "test-command", + args: "test-args", + env: {}, + config: { + ...DEFAULT_INSPECTOR_CONFIG, + MCP_PROXY_FULL_ADDRESS: { + ...DEFAULT_INSPECTOR_CONFIG.MCP_PROXY_FULL_ADDRESS, + value: "", + }, + }, + }; + + const { result } = renderHook(() => useConnection(propsWithEmptyProxy)); + + await act(async () => { + await result.current.connect(); + }); + + // Check that the URL does not contain the proxyFullAddress parameter + expect( + mockSSETransport.url?.searchParams.get("proxyFullAddress"), + ).toBeNull(); + }); + + test("does not send proxyFullAddress parameter for streamable-http transport", async () => { + const propsWithStreamableHttp = { + ...defaultProps, + transportType: "streamable-http" as const, + sseUrl: "http://localhost:8080", + config: { + ...DEFAULT_INSPECTOR_CONFIG, + MCP_PROXY_FULL_ADDRESS: { + ...DEFAULT_INSPECTOR_CONFIG.MCP_PROXY_FULL_ADDRESS, + value: "https://example.com/inspector/mcp_proxy", + }, + }, + }; + + const { result } = renderHook(() => + useConnection(propsWithStreamableHttp), + ); + + await act(async () => { + await result.current.connect(); + }); + + // Check that streamable-http transport doesn't get proxyFullAddress parameter + expect( + mockStreamableHTTPTransport.url?.searchParams.get("proxyFullAddress"), + ).toBeNull(); + }); + }); }); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 8c44d51b..a07b67dc 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -409,11 +409,20 @@ export function useConnection({ let mcpProxyServerUrl; switch (transportType) { - case "stdio": + case "stdio": { mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/stdio`); mcpProxyServerUrl.searchParams.append("command", command); mcpProxyServerUrl.searchParams.append("args", args); mcpProxyServerUrl.searchParams.append("env", JSON.stringify(env)); + + const proxyFullAddress = config.MCP_PROXY_FULL_ADDRESS + .value as string; + if (proxyFullAddress) { + mcpProxyServerUrl.searchParams.append( + "proxyFullAddress", + proxyFullAddress, + ); + } transportOptions = { authProvider: serverAuthProvider, eventSourceInit: { @@ -431,10 +440,20 @@ export function useConnection({ }, }; break; + } - case "sse": + case "sse": { mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/sse`); mcpProxyServerUrl.searchParams.append("url", sseUrl); + + const proxyFullAddressSSE = config.MCP_PROXY_FULL_ADDRESS + .value as string; + if (proxyFullAddressSSE) { + mcpProxyServerUrl.searchParams.append( + "proxyFullAddress", + proxyFullAddressSSE, + ); + } transportOptions = { eventSourceInit: { fetch: ( @@ -451,6 +470,7 @@ export function useConnection({ }, }; break; + } case "streamable-http": mcpProxyServerUrl = new URL(`${getMCPProxyAddress(config)}/mcp`); diff --git a/client/src/utils/jsonUtils.ts b/client/src/utils/jsonUtils.ts index 60f5b1d6..08de50ba 100644 --- a/client/src/utils/jsonUtils.ts +++ b/client/src/utils/jsonUtils.ts @@ -63,12 +63,22 @@ export type DataType = | "array" | "null"; +/** + * Determines the specific data type of a JSON value + * @param value The JSON value to analyze + * @returns The specific data type including "array" and "null" as distinct types + */ export function getDataType(value: JsonValue): DataType { if (Array.isArray(value)) return "array"; if (value === null) return "null"; return typeof value; } +/** + * Attempts to parse a string as JSON, only for objects and arrays + * @param str The string to parse + * @returns Object with success boolean and either parsed data or original string + */ export function tryParseJson(str: string): { success: boolean; data: JsonValue; diff --git a/client/src/utils/oauthUtils.ts b/client/src/utils/oauthUtils.ts index 83de080b..9b51179c 100644 --- a/client/src/utils/oauthUtils.ts +++ b/client/src/utils/oauthUtils.ts @@ -24,6 +24,11 @@ type CallbackParams = error_uri: string | null; }; +/** + * Parses OAuth 2.1 callback parameters from a URL search string + * @param location The URL search string (e.g., "?code=abc123" or "?error=access_denied") + * @returns Parsed callback parameters with success/error information + */ export const parseOAuthCallbackParams = (location: string): CallbackParams => { const params = new URLSearchParams(location); @@ -62,6 +67,11 @@ export const generateOAuthState = () => { ); }; +/** + * Generates a human-readable error description from OAuth callback error parameters + * @param params OAuth error callback parameters containing error details + * @returns Formatted multiline error message with error code, description, and optional URI + */ export const generateOAuthErrorDescription = ( params: Extract, ): string => { diff --git a/server/src/index.ts b/server/src/index.ts index a30c1845..c0fb3797 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -378,7 +378,6 @@ app.get( let serverTransport: Transport | undefined; try { serverTransport = await createTransport(req); - console.log("Created server transport"); } catch (error) { if (error instanceof SseError && error.code === 401) { console.error( @@ -391,11 +390,16 @@ app.get( throw error; } - const webAppTransport = new SSEServerTransport("/message", res); + const proxyFullAddress = (req.query.proxyFullAddress as string) || ""; + const prefix = proxyFullAddress || ""; + const endpoint = `${prefix}/message`; + + const webAppTransport = new SSEServerTransport(endpoint, res); + webAppTransports.set(webAppTransport.sessionId, webAppTransport); console.log("Created client transport"); - webAppTransports.set(webAppTransport.sessionId, webAppTransport); serverTransports.set(webAppTransport.sessionId, serverTransport); + console.log("Created server transport"); await webAppTransport.start(); @@ -484,7 +488,7 @@ app.get( async (req, res) => { try { console.log( - "New SSE connection request. NOTE: The sse transport is deprecated and has been replaced by StreamableHttp", + "New SSE connection request. NOTE: The SSE transport is deprecated and has been replaced by StreamableHttp", ); let serverTransport: Transport | undefined; try { @@ -511,9 +515,14 @@ app.get( } if (serverTransport) { - const webAppTransport = new SSEServerTransport("/message", res); + const proxyFullAddress = (req.query.proxyFullAddress as string) || ""; + const prefix = proxyFullAddress || ""; + const endpoint = `${prefix}/message`; + + const webAppTransport = new SSEServerTransport(endpoint, res); webAppTransports.set(webAppTransport.sessionId, webAppTransport); console.log("Created client transport"); + serverTransports.set(webAppTransport.sessionId, serverTransport!); console.log("Created server transport");