From 69a88ba7753a8ce86d24251a33ebe06f908f1af0 Mon Sep 17 00:00:00 2001 From: olaservo Date: Tue, 15 Jul 2025 21:33:11 -0700 Subject: [PATCH] Omit optional fields with empty strings from tool calls --- client/src/App.tsx | 9 +- client/src/components/ToolsTab.tsx | 42 +++- client/src/utils/__tests__/paramUtils.test.ts | 184 ++++++++++++++++++ client/src/utils/paramUtils.ts | 44 +++++ 4 files changed, 268 insertions(+), 11 deletions(-) create mode 100644 client/src/utils/__tests__/paramUtils.test.ts create mode 100644 client/src/utils/paramUtils.ts diff --git a/client/src/App.tsx b/client/src/App.tsx index cff51b4ff..92d918948 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -22,6 +22,7 @@ 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 React, { Suspense, useCallback, @@ -546,12 +547,18 @@ const App = () => { const callTool = async (name: string, params: Record) => { 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 any) + : params; + const response = await sendMCPRequest( { method: "tools/call" as const, params: { name, - arguments: params, + arguments: cleanedParams, _meta: { progressToken: progressTokenRef.current++, }, diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 3c7db84e4..736492930 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -140,12 +140,22 @@ const ToolsTab = ({ name={key} placeholder={prop.description} value={(params[key] as string) ?? ""} - onChange={(e) => - setParams({ - ...params, - [key]: e.target.value, - }) - } + onChange={(e) => { + const value = e.target.value; + if (value === "" && !required) { + // Optional field cleared - set to undefined to omit from request + setParams({ + ...params, + [key]: undefined, + }); + } else { + // Field has value or is required - keep as string + setParams({ + ...params, + [key]: value, + }); + } + }} className="mt-1" /> ) : prop.type === "object" || prop.type === "array" ? ( @@ -179,10 +189,22 @@ const ToolsTab = ({ value={(params[key] as string) ?? ""} onChange={(e) => { const value = e.target.value; - setParams({ - ...params, - [key]: value === "" ? "" : Number(value), - }); + if (value === "" && !required) { + // Optional field cleared - set to undefined to omit from request + setParams({ + ...params, + [key]: undefined, + }); + } else { + // Field has value or is required - convert to number + const num = Number(value); + if (!isNaN(num) || value === "") { + setParams({ + ...params, + [key]: value === "" ? "" : num, + }); + } + } }} className="mt-1" /> diff --git a/client/src/utils/__tests__/paramUtils.test.ts b/client/src/utils/__tests__/paramUtils.test.ts new file mode 100644 index 000000000..84735a040 --- /dev/null +++ b/client/src/utils/__tests__/paramUtils.test.ts @@ -0,0 +1,184 @@ +import { cleanParams } from "../paramUtils"; +import type { JsonSchemaType } from "../jsonUtils"; + +describe("cleanParams", () => { + it("should preserve required fields even when empty", () => { + const schema: JsonSchemaType = { + type: "object", + required: ["requiredString", "requiredNumber"], + properties: { + requiredString: { type: "string" }, + requiredNumber: { type: "number" }, + optionalString: { type: "string" }, + optionalNumber: { type: "number" }, + }, + }; + + const params = { + requiredString: "", + requiredNumber: 0, + optionalString: "", + optionalNumber: undefined, + }; + + const cleaned = cleanParams(params, schema); + + expect(cleaned).toEqual({ + requiredString: "", + requiredNumber: 0, + // 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, + }); + }); +}); diff --git a/client/src/utils/paramUtils.ts b/client/src/utils/paramUtils.ts new file mode 100644 index 000000000..2a8cfcd9d --- /dev/null +++ b/client/src/utils/paramUtils.ts @@ -0,0 +1,44 @@ +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, + schema: JsonSchemaType +): Record { + const cleaned: Record = {}; + const required = schema.required || []; + + for (const [key, value] of Object.entries(params)) { + const isFieldRequired = required.includes(key); + + if (isFieldRequired) { + // Required fields: always include, even if empty string or falsy + cleaned[key] = value; + } else { + // Optional fields: only include if they have meaningful values + if (value !== undefined && value !== "" && value !== null) { + cleaned[key] = value; + } + // Empty strings, undefined, null for optional fields → omit completely + } + } + + return cleaned; +} + +/** + * Checks if a field should be set to undefined when cleared + * @param isRequired - Whether the field is required + * @param value - The current value + * @returns Whether to set the field to undefined + */ +export function shouldSetToUndefined(isRequired: boolean, value: string): boolean { + return !isRequired && value === ""; +}