Skip to content

Commit cd2b787

Browse files
cliffhallcliffhall
authored
Merge pull request #625 from olaservo/omit-optional-empty-strings
Update ToolsTab parameter handling to omit optional empty fields
2 parents 5e1aef2 + 5abf1d5 commit cd2b787

File tree

6 files changed

+327
-30
lines changed

6 files changed

+327
-30
lines changed

‎client/pr_demo.mp4‎

-709 KB
Binary file not shown.

‎client/src/App.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { SESSION_KEYS, getServerSpecificKey } from "./lib/constants";
2222
import { AuthDebuggerState, EMPTY_DEBUGGER_STATE } from "./lib/auth-types";
2323
import { OAuthStateMachine } from "./lib/oauth-state-machine";
2424
import { cacheToolOutputSchemas } from "./utils/schemaUtils";
25+
import { cleanParams } from "./utils/paramUtils";
26+
import type { JsonSchemaType } from "./utils/jsonUtils";
2527
import React, {
2628
Suspense,
2729
useCallback,
@@ -777,12 +779,18 @@ const App = () => {
777779
lastToolCallOriginTabRef.current = currentTabRef.current;
778780

779781
try {
782+
// Find the tool schema to clean parameters properly
783+
const tool = tools.find((t) => t.name === name);
784+
const cleanedParams = tool?.inputSchema
785+
? cleanParams(params, tool.inputSchema as JsonSchemaType)
786+
: params;
787+
780788
const response = await sendMCPRequest(
781789
{
782790
method: "tools/call" as const,
783791
params: {
784792
name,
785-
arguments: params,
793+
arguments: cleanedParams,
786794
_meta: {
787795
progressToken: progressTokenRef.current++,
788796
},
@@ -793,6 +801,8 @@ const App = () => {
793801
);
794802

795803
setToolResult(response);
804+
// Clear any validation errors since tool execution completed
805+
setErrors((prev) => ({ ...prev, tools: null }));
796806
} catch (e) {
797807
const toolResult: CompatibilityCallToolResult = {
798808
content: [
@@ -804,6 +814,8 @@ const App = () => {
804814
isError: true,
805815
};
806816
setToolResult(toolResult);
817+
// Clear validation errors - tool execution errors are shown in ToolResults
818+
setErrors((prev) => ({ ...prev, tools: null }));
807819
}
808820
};
809821

‎client/src/components/ToolsTab.tsx‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,15 @@ const ToolsTab = ({
131131
</h3>
132132
</div>
133133
<div className="p-4">
134-
{error ? (
135-
<Alert variant="destructive">
136-
<AlertCircle className="h-4 w-4" />
137-
<AlertTitle>Error</AlertTitle>
138-
<AlertDescription>{error}</AlertDescription>
139-
</Alert>
140-
) : selectedTool ? (
134+
{selectedTool ? (
141135
<div className="space-y-4">
136+
{error && (
137+
<Alert variant="destructive">
138+
<AlertCircle className="h-4 w-4" />
139+
<AlertTitle>Error</AlertTitle>
140+
<AlertDescription>{error}</AlertDescription>
141+
</Alert>
142+
)}
142143
<p className="text-sm text-gray-600 dark:text-gray-400">
143144
{selectedTool.description}
144145
</p>
@@ -184,16 +185,27 @@ const ToolsTab = ({
184185
id={key}
185186
name={key}
186187
placeholder={prop.description}
187-
value={(params[key] as string) ?? ""}
188-
onChange={(e) =>
189-
setParams({
190-
...params,
191-
[key]:
192-
e.target.value === ""
193-
? undefined
194-
: e.target.value,
195-
})
188+
value={
189+
params[key] === undefined
190+
? ""
191+
: String(params[key])
196192
}
193+
onChange={(e) => {
194+
const value = e.target.value;
195+
if (value === "") {
196+
// Field cleared - set to undefined
197+
setParams({
198+
...params,
199+
[key]: undefined,
200+
});
201+
} else {
202+
// Field has value - keep as string
203+
setParams({
204+
...params,
205+
[key]: value,
206+
});
207+
}
208+
}}
197209
className="mt-1"
198210
/>
199211
) : prop.type === "object" || prop.type === "array" ? (
@@ -227,13 +239,35 @@ const ToolsTab = ({
227239
id={key}
228240
name={key}
229241
placeholder={prop.description}
230-
value={(params[key] as string) ?? ""}
242+
value={
243+
params[key] === undefined
244+
? ""
245+
: String(params[key])
246+
}
231247
onChange={(e) => {
232248
const value = e.target.value;
233-
setParams({
234-
...params,
235-
[key]: value === "" ? "" : Number(value),
236-
});
249+
if (value === "") {
250+
// Field cleared - set to undefined
251+
setParams({
252+
...params,
253+
[key]: undefined,
254+
});
255+
} else {
256+
// Field has value - try to convert to number, but store input either way
257+
const num = Number(value);
258+
if (!isNaN(num)) {
259+
setParams({
260+
...params,
261+
[key]: num,
262+
});
263+
} else {
264+
// Store invalid input as string - let server validate
265+
setParams({
266+
...params,
267+
[key]: value,
268+
});
269+
}
270+
}
237271
}}
238272
className="mt-1"
239273
/>

‎client/src/components/__tests__/ToolsTab.test.tsx‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ describe("ToolsTab", () => {
644644
description: "Tool with JSON parameters",
645645
inputSchema: {
646646
type: "object" as const,
647+
required: ["config", "data"], // Make them required so they render as form fields
647648
properties: {
648649
config: {
649650
type: "object" as const,
@@ -693,15 +694,16 @@ describe("ToolsTab", () => {
693694
callTool: mockCallTool,
694695
});
695696

696-
// Find JSON editor textareas
697+
// Find JSON editor textareas (should have one for each required field: config and data)
697698
const textareas = screen.getAllByRole("textbox");
699+
expect(textareas.length).toBe(2);
698700

699-
// Enter valid JSON in the first textarea
701+
// Enter valid JSON in each textarea
700702
fireEvent.change(textareas[0], {
701-
target: {
702-
value:
703-
'{ "config": { "setting": "value" }, "data": ["item1", "item2"] }',
704-
},
703+
target: { value: '{ "setting": "value" }' },
704+
});
705+
fireEvent.change(textareas[1], {
706+
target: { value: '["item1", "item2"]' },
705707
});
706708

707709
// Wait for debounced updates
@@ -799,9 +801,16 @@ describe("ToolsTab", () => {
799801
});
800802

801803
const textareas = screen.getAllByRole("textbox");
804+
expect(textareas.length).toBe(2);
805+
806+
// Clear both textareas (empty JSON should be valid)
807+
fireEvent.change(textareas[0], { target: { value: "{}" } });
808+
fireEvent.change(textareas[1], { target: { value: "[]" } });
802809

803-
// Clear the textarea (empty JSON should be valid)
804-
fireEvent.change(textareas[0], { target: { value: "" } });
810+
// Wait for debounced updates
811+
await act(async () => {
812+
await new Promise((resolve) => setTimeout(resolve, 350));
813+
});
805814

806815
// Try to run the tool
807816
const runButton = screen.getByRole("button", { name: /run tool/i });

0 commit comments

Comments
 (0)