diff --git a/client/pr_demo.mp4 b/client/pr_demo.mp4 deleted file mode 100644 index b6ae523d5..000000000 Binary files a/client/pr_demo.mp4 and /dev/null differ diff --git a/client/src/App.tsx b/client/src/App.tsx index 6c9ae3331..04794df05 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -22,6 +22,8 @@ import { SESSION_KEYS, getServerSpecificKey } from "./lib/constants"; import { AuthDebuggerState, EMPTY_DEBUGGER_STATE } from "./lib/auth-types"; import { OAuthStateMachine } from "./lib/oauth-state-machine"; import { cacheToolOutputSchemas } from "./utils/schemaUtils"; +import { cleanParams } from "./utils/paramUtils"; +import type { JsonSchemaType } from "./utils/jsonUtils"; import React, { Suspense, useCallback, @@ -777,12 +779,18 @@ const App = () => { lastToolCallOriginTabRef.current = currentTabRef.current; try { + // Find the tool schema to clean parameters properly + const tool = tools.find((t) => t.name === name); + const cleanedParams = tool?.inputSchema + ? cleanParams(params, tool.inputSchema as JsonSchemaType) + : params; + const response = await sendMCPRequest( { method: "tools/call" as const, params: { name, - arguments: params, + arguments: cleanedParams, _meta: { progressToken: progressTokenRef.current++, }, @@ -793,6 +801,8 @@ const App = () => { ); setToolResult(response); + // Clear any validation errors since tool execution completed + setErrors((prev) => ({ ...prev, tools: null })); } catch (e) { const toolResult: CompatibilityCallToolResult = { content: [ @@ -804,6 +814,8 @@ const App = () => { isError: true, }; setToolResult(toolResult); + // Clear validation errors - tool execution errors are shown in ToolResults + setErrors((prev) => ({ ...prev, tools: null })); } }; diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 31979f0da..0123296af 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -131,14 +131,15 @@ const ToolsTab = ({
{selectedTool.description}
@@ -184,16 +185,27 @@ const ToolsTab = ({ id={key} name={key} placeholder={prop.description} - value={(params[key] as string) ?? ""} - onChange={(e) => - setParams({ - ...params, - [key]: - e.target.value === "" - ? undefined - : e.target.value, - }) + value={ + params[key] === undefined + ? "" + : String(params[key]) } + onChange={(e) => { + const value = e.target.value; + if (value === "") { + // Field cleared - set to undefined + setParams({ + ...params, + [key]: undefined, + }); + } else { + // Field has value - keep as string + setParams({ + ...params, + [key]: value, + }); + } + }} className="mt-1" /> ) : prop.type === "object" || prop.type === "array" ? ( @@ -227,13 +239,35 @@ const ToolsTab = ({ id={key} name={key} placeholder={prop.description} - value={(params[key] as string) ?? ""} + value={ + params[key] === undefined + ? "" + : String(params[key]) + } onChange={(e) => { const value = e.target.value; - setParams({ - ...params, - [key]: value === "" ? "" : Number(value), - }); + if (value === "") { + // Field cleared - set to undefined + setParams({ + ...params, + [key]: undefined, + }); + } else { + // Field has value - try to convert to number, but store input either way + const num = Number(value); + if (!isNaN(num)) { + setParams({ + ...params, + [key]: num, + }); + } else { + // Store invalid input as string - let server validate + setParams({ + ...params, + [key]: value, + }); + } + } }} className="mt-1" /> diff --git a/client/src/components/__tests__/ToolsTab.test.tsx b/client/src/components/__tests__/ToolsTab.test.tsx index 290605dd8..a50f62df3 100644 --- a/client/src/components/__tests__/ToolsTab.test.tsx +++ b/client/src/components/__tests__/ToolsTab.test.tsx @@ -644,6 +644,7 @@ describe("ToolsTab", () => { description: "Tool with JSON parameters", inputSchema: { type: "object" as const, + required: ["config", "data"], // Make them required so they render as form fields properties: { config: { type: "object" as const, @@ -693,15 +694,16 @@ describe("ToolsTab", () => { callTool: mockCallTool, }); - // Find JSON editor textareas + // Find JSON editor textareas (should have one for each required field: config and data) const textareas = screen.getAllByRole("textbox"); + expect(textareas.length).toBe(2); - // Enter valid JSON in the first textarea + // Enter valid JSON in each textarea fireEvent.change(textareas[0], { - target: { - value: - '{ "config": { "setting": "value" }, "data": ["item1", "item2"] }', - }, + target: { value: '{ "setting": "value" }' }, + }); + fireEvent.change(textareas[1], { + target: { value: '["item1", "item2"]' }, }); // Wait for debounced updates @@ -799,9 +801,16 @@ describe("ToolsTab", () => { }); const textareas = screen.getAllByRole("textbox"); + expect(textareas.length).toBe(2); + + // Clear both textareas (empty JSON should be valid) + fireEvent.change(textareas[0], { target: { value: "{}" } }); + fireEvent.change(textareas[1], { target: { value: "[]" } }); - // Clear the textarea (empty JSON should be valid) - fireEvent.change(textareas[0], { target: { value: "" } }); + // Wait for debounced updates + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 350)); + }); // Try to run the tool const runButton = screen.getByRole("button", { name: /run tool/i }); diff --git a/client/src/utils/__tests__/paramUtils.test.ts b/client/src/utils/__tests__/paramUtils.test.ts new file mode 100644 index 000000000..3dfd53828 --- /dev/null +++ b/client/src/utils/__tests__/paramUtils.test.ts @@ -0,0 +1,208 @@ +import { cleanParams } from "../paramUtils"; +import type { JsonSchemaType } from "../jsonUtils"; + +describe("cleanParams", () => { + it("should preserve required fields even when undefined", () => { + const schema: JsonSchemaType = { + type: "object", + required: ["requiredString", "requiredNumber"], + properties: { + requiredString: { type: "string" }, + requiredNumber: { type: "number" }, + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + }, + }; + + const params = { + requiredString: undefined, + requiredNumber: undefined, + optionalString: undefined, + optionalNumber: undefined, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + requiredString: undefined, + requiredNumber: undefined, + // optionalString and optionalNumber should be omitted + }); + }); + + it("should omit optional fields with empty strings", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + }, + }; + + const params = { + optionalString: "", + optionalNumber: "", + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({}); + }); + + it("should omit optional fields with undefined values", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + }, + }; + + const params = { + optionalString: undefined, + optionalNumber: undefined, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({}); + }); + + it("should omit optional fields with null values", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + }, + }; + + const params = { + optionalString: null, + optionalNumber: null, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({}); + }); + + it("should preserve optional fields with meaningful values", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + optionalBoolean: { type: "boolean" }, + }, + }; + + const params = { + optionalString: "hello", + optionalNumber: 42, + optionalBoolean: false, // false is a meaningful value + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + optionalString: "hello", + optionalNumber: 42, + optionalBoolean: false, + }); + }); + + it("should handle mixed required and optional fields", () => { + const schema: JsonSchemaType = { + type: "object", + required: ["requiredField"], + properties: { + requiredField: { type: "string" }, + optionalWithValue: { type: "string" }, + optionalEmpty: { type: "string" }, + optionalUndefined: { type: "number" }, + }, + }; + + const params = { + requiredField: "", + optionalWithValue: "test", + optionalEmpty: "", + optionalUndefined: undefined, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + requiredField: "", + optionalWithValue: "test", + }); + }); + + it("should handle schema without required array", () => { + const schema: JsonSchemaType = { + type: "object", + properties: { + field1: { type: "string" }, + field2: { type: "number" }, + }, + }; + + const params = { + field1: "", + field2: undefined, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({}); + }); + + it("should preserve zero values for numbers", () => { + const schema: JsonSchemaType = { + type: "object", + required: [], + properties: { + optionalNumber: { type: "number" }, + }, + }; + + const params = { + optionalNumber: 0, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + optionalNumber: 0, + }); + }); + + it("should handle the new undefined-first approach (no empty strings)", () => { + const schema: JsonSchemaType = { + type: "object", + required: ["requiredField"], + properties: { + requiredField: { type: "string" }, + optionalField: { type: "string" }, + }, + }; + + // New behavior: cleared fields are undefined, never empty strings + const params = { + requiredField: undefined, // cleared required field + optionalField: undefined, // cleared optional field + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + requiredField: undefined, // required field preserved as undefined + // optionalField omitted entirely + }); + }); +}); diff --git a/client/src/utils/paramUtils.ts b/client/src/utils/paramUtils.ts new file mode 100644 index 000000000..dfc418023 --- /dev/null +++ b/client/src/utils/paramUtils.ts @@ -0,0 +1,34 @@ +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. + * + * @param params - The parameters object to clean + * @param schema - The JSON schema defining which fields are required + * @returns Cleaned parameters object with optional empty fields omitted + */ +export function cleanParams( + params: Record