From 0123a71da49f9fd6b2066b644e35042c3099b482 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 8 Oct 2025 08:43:11 -0700 Subject: [PATCH 1/7] Add input validation guidelines and enhance cleanParams function to preserve default values --- CLAUDE.md | 11 +++ client/src/utils/__tests__/paramUtils.test.ts | 72 +++++++++++++++++++ client/src/utils/paramUtils.ts | 12 +++- 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index ec826b1fd..1e6a37c25 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,3 +31,14 @@ The project is organized as a monorepo with workspaces: - `client/`: React frontend with Vite, TypeScript and Tailwind - `server/`: Express backend with TypeScript - `cli/`: Command-line interface for testing and invoking MCP server methods directly + +## Tool Input Validation Guidelines + +When handling tool input parameters and form fields: + +- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages +- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema + +These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. diff --git a/client/src/utils/__tests__/paramUtils.test.ts b/client/src/utils/__tests__/paramUtils.test.ts index 3dfd53828..6c853490f 100644 --- a/client/src/utils/__tests__/paramUtils.test.ts +++ b/client/src/utils/__tests__/paramUtils.test.ts @@ -205,4 +205,76 @@ describe("cleanParams", () => { // optionalField omitted entirely }); }); + + it("should preserve null values when field has default: null", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalFieldWithNullDefault: { type: "string", default: null }, + optionalFieldWithoutDefault: { type: "string" }, + }, + }; + + const params = { + optionalFieldWithNullDefault: null, + optionalFieldWithoutDefault: null, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + optionalFieldWithNullDefault: null, // preserved because default: null + // optionalFieldWithoutDefault omitted + }); + }); + + it("should preserve default values that match current value", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + fieldWithDefaultString: { type: "string", default: "defaultValue" }, + fieldWithDefaultNumber: { type: "number", default: 42 }, + fieldWithDefaultNull: { type: "string", default: null }, + fieldWithDefaultBoolean: { type: "boolean", default: false }, + }, + }; + + const params = { + fieldWithDefaultString: "defaultValue", + fieldWithDefaultNumber: 42, + fieldWithDefaultNull: null, + fieldWithDefaultBoolean: false, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + fieldWithDefaultString: "defaultValue", + fieldWithDefaultNumber: 42, + fieldWithDefaultNull: null, + fieldWithDefaultBoolean: false, + }); + }); + + it("should omit values that do not match their default", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + fieldWithDefault: { type: "string", default: "defaultValue" }, + }, + }; + + const params = { + fieldWithDefault: null, // doesn't match default + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + // fieldWithDefault omitted because value (null) doesn't match default ("defaultValue") + }); + }); }); diff --git a/client/src/utils/paramUtils.ts b/client/src/utils/paramUtils.ts index dfc418023..aa538f404 100644 --- a/client/src/utils/paramUtils.ts +++ b/client/src/utils/paramUtils.ts @@ -2,7 +2,7 @@ import type { JsonSchemaType } from "./jsonUtils"; /** * Cleans parameters by removing undefined, null, and empty string values for optional fields - * while preserving all values for required fields. + * while preserving all values for required fields and fields with explicit default values. * * @param params - The parameters object to clean * @param schema - The JSON schema defining which fields are required @@ -14,13 +14,23 @@ export function cleanParams( ): Record { const cleaned: Record = {}; const required = schema.required || []; + const properties = schema.properties || {}; for (const [key, value] of Object.entries(params)) { const isFieldRequired = required.includes(key); + const fieldSchema = properties[key] as JsonSchemaType | undefined; + + // Check if the field has an explicit default value + const hasDefault = fieldSchema && "default" in fieldSchema; + const defaultValue = hasDefault ? fieldSchema.default : undefined; if (isFieldRequired) { // Required fields: always include, even if empty string or falsy cleaned[key] = value; + } else if (hasDefault && value === defaultValue) { + // Field has a default value and current value matches it - preserve it + // This is important for cases like default: null + cleaned[key] = value; } else { // Optional fields: only include if they have meaningful values if (value !== undefined && value !== "" && value !== null) { From 89dc57c4883372c51acb4600b54404d6b0d1c562 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 12 Oct 2025 21:13:56 -0700 Subject: [PATCH 2/7] Add tool input validation guidelines to AGENTS.md --- AGENTS.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index ec826b1fd..1e6a37c25 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,3 +31,14 @@ The project is organized as a monorepo with workspaces: - `client/`: React frontend with Vite, TypeScript and Tailwind - `server/`: Express backend with TypeScript - `cli/`: Command-line interface for testing and invoking MCP server methods directly + +## Tool Input Validation Guidelines + +When handling tool input parameters and form fields: + +- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages +- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema + +These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. From 541a59f1fec0ce51aa93955d947e943a51243db6 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sat, 18 Oct 2025 21:20:33 -0700 Subject: [PATCH 3/7] Add test to explicitly reproduce issue #846 regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test verifies that tools with multiple nullable parameters (default: null) work correctly. In v0.17.0, cleanParams would incorrectly remove these null values, breaking tool execution. đŸ¤– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- client/src/utils/__tests__/paramUtils.test.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/client/src/utils/__tests__/paramUtils.test.ts b/client/src/utils/__tests__/paramUtils.test.ts index 6c853490f..1077d76b7 100644 --- a/client/src/utils/__tests__/paramUtils.test.ts +++ b/client/src/utils/__tests__/paramUtils.test.ts @@ -277,4 +277,40 @@ describe("cleanParams", () => { // fieldWithDefault omitted because value (null) doesn't match default ("defaultValue") }); }); + + it("should fix regression from issue #846 - tools with multiple null defaults", () => { + // Reproduces the exact scenario from https://github.com/modelcontextprotocol/inspector/issues/846 + // In v0.17.0, the cleanParams function would remove all null values, + // breaking tools that have parameters with explicit default: null + const schema: JsonSchemaType = { + type: "object", + required: ["requiredString"], + properties: { + optionalString: { type: ["string", "null"], default: null }, + optionalNumber: { type: ["number", "null"], default: null }, + optionalBoolean: { type: ["boolean", "null"], default: null }, + requiredString: { type: "string" }, + }, + }; + + // When a user opens the tool in Inspector, fields initialize with their defaults + const params = { + optionalString: null, // initialized to default + optionalNumber: null, // initialized to default + optionalBoolean: null, // initialized to default + requiredString: "test", + }; + + const cleaned = cleanParams(params, schema); + + // In v0.16, null defaults were preserved (working behavior) + // In v0.17.0, they were removed (regression) + // This fix restores the v0.16 behavior + expect(cleaned).toEqual({ + optionalString: null, + optionalNumber: null, + optionalBoolean: null, + requiredString: "test", + }); + }); }); From 93fc8cc97e0077b712d69978f8cf141427fa9e34 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 19 Oct 2025 05:47:33 -0700 Subject: [PATCH 4/7] Support tri-state nullable boolean --- client/src/components/ToolsTab.tsx | 6 +- .../components/__tests__/ToolsTab.test.tsx | 97 +++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 50edaf514..8776e553b 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -179,7 +179,11 @@ const ToolsTab = ({ onCheckedChange={(checked: boolean) => setParams({ ...params, - [key]: checked ? null : prop.default, + [key]: checked + ? null + : prop.type === "boolean" + ? false + : prop.default, }) } /> diff --git a/client/src/components/__tests__/ToolsTab.test.tsx b/client/src/components/__tests__/ToolsTab.test.tsx index d9237be2e..007ca4f7b 100644 --- a/client/src/components/__tests__/ToolsTab.test.tsx +++ b/client/src/components/__tests__/ToolsTab.test.tsx @@ -177,6 +177,103 @@ describe("ToolsTab", () => { }); }); + it("should support tri-state nullable boolean (null -> false -> true -> null)", async () => { + const mockCallTool = jest.fn(); + const toolWithNullableBoolean: Tool = { + name: "testTool", + description: "Tool with nullable boolean", + inputSchema: { + type: "object" as const, + properties: { + optionalBoolean: { + type: ["boolean", "null"] as const, + default: null, + }, + }, + }, + }; + + renderToolsTab({ + tools: [toolWithNullableBoolean], + selectedTool: toolWithNullableBoolean, + callTool: mockCallTool, + }); + + const nullCheckbox = screen.getByRole("checkbox", { name: /null/i }); + const runButton = screen.getByRole("button", { name: /run tool/i }); + + // State 1: Initial state should be null (input disabled) + const wrapper = screen.getByRole("toolinputwrapper"); + expect(wrapper.classList).toContain("pointer-events-none"); + expect(wrapper.classList).toContain("opacity-50"); + + // Verify tool is called with null initially + await act(async () => { + fireEvent.click(runButton); + }); + expect(mockCallTool).toHaveBeenCalledWith(toolWithNullableBoolean.name, { + optionalBoolean: null, + }); + + // State 2: Uncheck null checkbox -> should set value to false and enable input + await act(async () => { + fireEvent.click(nullCheckbox); + }); + expect(wrapper.classList).not.toContain("pointer-events-none"); + + // Clear previous calls to make assertions clearer + mockCallTool.mockClear(); + + // Verify tool can be called with false + await act(async () => { + fireEvent.click(runButton); + }); + expect(mockCallTool).toHaveBeenLastCalledWith( + toolWithNullableBoolean.name, + { + optionalBoolean: false, + }, + ); + + // State 3: Check boolean checkbox -> should set value to true + // Find the boolean checkbox within the input wrapper (to avoid ID conflict with null checkbox) + const booleanCheckbox = within(wrapper).getByRole("checkbox"); + + mockCallTool.mockClear(); + + await act(async () => { + fireEvent.click(booleanCheckbox); + }); + + // Verify tool can be called with true + await act(async () => { + fireEvent.click(runButton); + }); + expect(mockCallTool).toHaveBeenLastCalledWith( + toolWithNullableBoolean.name, + { + optionalBoolean: true, + }, + ); + + // State 4: Check null checkbox again -> should set value back to null and disable input + await act(async () => { + fireEvent.click(nullCheckbox); + }); + expect(wrapper.classList).toContain("pointer-events-none"); + + // Verify tool can be called with null again + await act(async () => { + fireEvent.click(runButton); + }); + expect(mockCallTool).toHaveBeenLastCalledWith( + toolWithNullableBoolean.name, + { + optionalBoolean: null, + }, + ); + }); + it("should disable button and change text while tool is running", async () => { // Create a promise that we can resolve later let resolvePromise: ((value: unknown) => void) | undefined; From 1a93897533824bf8c2e13ca7be3189715f676258 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 19 Oct 2025 11:00:19 -0700 Subject: [PATCH 5/7] Update AGENTS to be directed at agent --- AGENTS.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1e6a37c25..dbd07c0a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,11 +34,11 @@ The project is organized as a monorepo with workspaces: ## Tool Input Validation Guidelines -When handling tool input parameters and form fields: +When implementing or modifying tool input parameter handling in the Inspector: -- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value -- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects -- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages -- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema +- **Omit optional fields with empty values** - When processing form inputs, omit empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Preserve explicit default values** - If a field schema contains an explicit default (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Always include required fields** - Preserve required field values even when empty, allowing the MCP server to validate and return appropriate error messages +- **Defer deep validation to the server** - Implement basic field presence checking in the Inspector client, but rely on the MCP server for parameter validation according to its schema -These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. +These guidelines maintain clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. From acf30b077ab4c1ee665b1e2493a9f0a4931af831 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 19 Oct 2025 11:01:37 -0700 Subject: [PATCH 6/7] Handle tri state nullable boolean in null checkbox --- client/src/components/ToolsTab.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 8776e553b..740cc5edb 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -181,9 +181,16 @@ const ToolsTab = ({ ...params, [key]: checked ? null - : prop.type === "boolean" - ? false - : prop.default, + : prop.default !== null + ? prop.default + : prop.type === "boolean" + ? false + : prop.type === "string" + ? "" + : prop.type === "number" || + prop.type === "integer" + ? undefined + : undefined, }) } /> From 60ff2d37af710613ebf300127f25bee5a7565151 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 19 Oct 2025 17:31:31 -0700 Subject: [PATCH 7/7] Move Tool Input Validation Guidelines to Readme --- AGENTS.md | 11 ----------- README.md | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dbd07c0a9..ec826b1fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,14 +31,3 @@ The project is organized as a monorepo with workspaces: - `client/`: React frontend with Vite, TypeScript and Tailwind - `server/`: Express backend with TypeScript - `cli/`: Command-line interface for testing and invoking MCP server methods directly - -## Tool Input Validation Guidelines - -When implementing or modifying tool input parameter handling in the Inspector: - -- **Omit optional fields with empty values** - When processing form inputs, omit empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value -- **Preserve explicit default values** - If a field schema contains an explicit default (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects -- **Always include required fields** - Preserve required field values even when empty, allowing the MCP server to validate and return appropriate error messages -- **Defer deep validation to the server** - Implement basic field presence checking in the Inspector client, but rely on the MCP server for parameter validation according to its schema - -These guidelines maintain clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. diff --git a/README.md b/README.md index 290375508..d753424e6 100644 --- a/README.md +++ b/README.md @@ -456,6 +456,17 @@ npx @modelcontextprotocol/inspector --cli https://my-mcp-server.example.com --me | **Automation** | N/A | Ideal for CI/CD pipelines, batch processing, and integration with coding assistants | | **Learning MCP** | Rich visual interface helps new users understand server capabilities | Simplified commands for focused learning of specific endpoints | +## Tool Input Validation Guidelines + +When implementing or modifying tool input parameter handling in the Inspector: + +- **Omit optional fields with empty values** - When processing form inputs, omit empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Preserve explicit default values** - If a field schema contains an explicit default (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Always include required fields** - Preserve required field values even when empty, allowing the MCP server to validate and return appropriate error messages +- **Defer deep validation to the server** - Implement basic field presence checking in the Inspector client, but rely on the MCP server for parameter validation according to its schema + +These guidelines maintain clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. + ## License This project is licensed under the MIT License—see the [LICENSE](LICENSE) file for details.