Skip to content

Commit cd28370

Browse files
cliffhallcliffhall
authored
Merge pull request modelcontextprotocol#284 from max-stytch/max/default-tool-json
fix: When tool type cannot be determined, use DynamicJsonForm
2 parents 4adf4b1 + 33aad8a commit cd28370

File tree

3 files changed

+92
-125
lines changed

3 files changed

+92
-125
lines changed

‎client/src/components/DynamicJsonForm.tsx‎

Lines changed: 18 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { useState, useEffect, useCallback, useRef } from "react";
22
import { Button } from "@/components/ui/button";
33
import { Input } from "@/components/ui/input";
4-
import { Label } from "@/components/ui/label";
54
import JsonEditor from "./JsonEditor";
65
import { updateValueAtPath } from "@/utils/jsonUtils";
7-
import { generateDefaultValue, formatFieldLabel } from "@/utils/schemaUtils";
8-
import type { JsonValue, JsonSchemaType, JsonObject } from "@/utils/jsonUtils";
6+
import { generateDefaultValue } from "@/utils/schemaUtils";
7+
import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils";
98

109
interface DynamicJsonFormProps {
1110
schema: JsonSchemaType;
@@ -14,13 +13,23 @@ interface DynamicJsonFormProps {
1413
maxDepth?: number;
1514
}
1615

16+
const isSimpleObject = (schema: JsonSchemaType): boolean => {
17+
const supportedTypes = ["string", "number", "integer", "boolean", "null"];
18+
if (supportedTypes.includes(schema.type)) return true;
19+
if (schema.type !== "object") return false;
20+
return Object.values(schema.properties ?? {}).every((prop) =>
21+
supportedTypes.includes(prop.type),
22+
);
23+
};
24+
1725
const DynamicJsonForm = ({
1826
schema,
1927
value,
2028
onChange,
2129
maxDepth = 3,
2230
}: DynamicJsonFormProps) => {
23-
const [isJsonMode, setIsJsonMode] = useState(false);
31+
const isOnlyJSON = !isSimpleObject(schema);
32+
const [isJsonMode, setIsJsonMode] = useState(isOnlyJSON);
2433
const [jsonError, setJsonError] = useState<string>();
2534
// Store the raw JSON string to allow immediate feedback during typing
2635
// while deferring parsing until the user stops typing
@@ -207,111 +216,6 @@ const DynamicJsonForm = ({
207216
required={propSchema.required}
208217
/>
209218
);
210-
case "object": {
211-
// Handle case where we have a value but no schema properties
212-
const objectValue = (currentValue as JsonObject) || {};
213-
214-
// If we have schema properties, use them to render fields
215-
if (propSchema.properties) {
216-
return (
217-
<div className="space-y-4 border rounded-md p-4">
218-
{Object.entries(propSchema.properties).map(([key, prop]) => (
219-
<div key={key} className="space-y-2">
220-
<Label>{formatFieldLabel(key)}</Label>
221-
{renderFormFields(
222-
prop,
223-
objectValue[key],
224-
[...path, key],
225-
depth + 1,
226-
)}
227-
</div>
228-
))}
229-
</div>
230-
);
231-
}
232-
// If we have a value but no schema properties, render fields based on the value
233-
else if (Object.keys(objectValue).length > 0) {
234-
return (
235-
<div className="space-y-4 border rounded-md p-4">
236-
{Object.entries(objectValue).map(([key, value]) => (
237-
<div key={key} className="space-y-2">
238-
<Label>{formatFieldLabel(key)}</Label>
239-
<Input
240-
type="text"
241-
value={String(value)}
242-
onChange={(e) =>
243-
handleFieldChange([...path, key], e.target.value)
244-
}
245-
/>
246-
</div>
247-
))}
248-
</div>
249-
);
250-
}
251-
// If we have neither schema properties nor value, return null
252-
return null;
253-
}
254-
case "array": {
255-
const arrayValue = Array.isArray(currentValue) ? currentValue : [];
256-
if (!propSchema.items) return null;
257-
return (
258-
<div className="space-y-4">
259-
{propSchema.description && (
260-
<p className="text-sm text-gray-600">{propSchema.description}</p>
261-
)}
262-
263-
{propSchema.items?.description && (
264-
<p className="text-sm text-gray-500">
265-
Items: {propSchema.items.description}
266-
</p>
267-
)}
268-
269-
<div className="space-y-2">
270-
{arrayValue.map((item, index) => (
271-
<div key={index} className="flex items-center gap-2">
272-
{renderFormFields(
273-
propSchema.items as JsonSchemaType,
274-
item,
275-
[...path, index.toString()],
276-
depth + 1,
277-
)}
278-
<Button
279-
variant="outline"
280-
size="sm"
281-
onClick={() => {
282-
const newArray = [...arrayValue];
283-
newArray.splice(index, 1);
284-
handleFieldChange(path, newArray);
285-
}}
286-
>
287-
Remove
288-
</Button>
289-
</div>
290-
))}
291-
<Button
292-
variant="outline"
293-
size="sm"
294-
onClick={() => {
295-
const defaultValue = generateDefaultValue(
296-
propSchema.items as JsonSchemaType,
297-
);
298-
handleFieldChange(path, [
299-
...arrayValue,
300-
defaultValue ?? null,
301-
]);
302-
}}
303-
title={
304-
propSchema.items?.description
305-
? `Add new ${propSchema.items.description}`
306-
: "Add new item"
307-
}
308-
>
309-
Add Item
310-
</Button>
311-
</div>
312-
</div>
313-
);
314-
}
315219
default:
316220
return null;
317221
}
@@ -350,9 +254,11 @@ const DynamicJsonForm = ({
350254
Format JSON
351255
</Button>
352256
)}
353-
<Button variant="outline" size="sm" onClick={handleSwitchToFormMode}>
354-
{isJsonMode ? "Switch to Form" : "Switch to JSON"}
355-
</Button>
257+
{!isOnlyJSON && (
258+
<Button variant="outline" size="sm" onClick={handleSwitchToFormMode}>
259+
{isJsonMode ? "Switch to Form" : "Switch to JSON"}
260+
</Button>
261+
)}
356262
</div>
357263

358264
{isJsonMode ? (

‎client/src/components/ToolsTab.tsx‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@ const ToolsTab = ({
4343
const [isToolRunning, setIsToolRunning] = useState(false);
4444

4545
useEffect(() => {
46-
setParams({});
46+
const params = Object.entries(
47+
selectedTool?.inputSchema.properties ?? [],
48+
).map(([key, value]) => [
49+
key,
50+
generateDefaultValue(value as JsonSchemaType),
51+
]);
52+
setParams(Object.fromEntries(params));
4753
}, [selectedTool]);
4854

4955
const renderToolResult = () => {
@@ -217,29 +223,40 @@ const ToolsTab = ({
217223
}}
218224
/>
219225
</div>
220-
) : (
226+
) : prop.type === "number" ||
227+
prop.type === "integer" ? (
221228
<Input
222-
type={
223-
prop.type === "number" || prop.type === "integer"
224-
? "number"
225-
: "text"
226-
}
229+
type="number"
227230
id={key}
228231
name={key}
229232
placeholder={prop.description}
230233
value={(params[key] as string) ?? ""}
231234
onChange={(e) =>
232235
setParams({
233236
...params,
234-
[key]:
235-
prop.type === "number" ||
236-
prop.type === "integer"
237-
? Number(e.target.value)
238-
: e.target.value,
237+
[key]: Number(e.target.value),
239238
})
240239
}
241240
className="mt-1"
242241
/>
242+
) : (
243+
<div className="mt-1">
244+
<DynamicJsonForm
245+
schema={{
246+
type: prop.type,
247+
properties: prop.properties,
248+
description: prop.description,
249+
items: prop.items,
250+
}}
251+
value={params[key] as JsonValue}
252+
onChange={(newValue: JsonValue) => {
253+
setParams({
254+
...params,
255+
[key]: newValue,
256+
});
257+
}}
258+
/>
259+
</div>
243260
)}
244261
</div>
245262
);

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

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { render, screen, fireEvent } from "@testing-library/react";
1+
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
22
import { describe, it, expect, jest } from "@jest/globals";
33
import DynamicJsonForm from "../DynamicJsonForm";
44
import type { JsonSchemaType } from "@/utils/jsonUtils";
@@ -93,3 +93,47 @@ describe("DynamicJsonForm Integer Fields", () => {
9393
});
9494
});
9595
});
96+
97+
describe("DynamicJsonForm Complex Fields", () => {
98+
const renderForm = (props = {}) => {
99+
const defaultProps = {
100+
schema: {
101+
type: "object",
102+
properties: {
103+
// The simplified JsonSchemaType does not accept oneOf fields
104+
// But they exist in the more-complete JsonSchema7Type
105+
nested: { oneOf: [{ type: "string" }, { type: "integer" }] },
106+
},
107+
} as unknown as JsonSchemaType,
108+
value: undefined,
109+
onChange: jest.fn(),
110+
};
111+
return render(<DynamicJsonForm {...defaultProps} {...props} />);
112+
};
113+
114+
describe("Basic Operations", () => {
115+
it("should render textbox and autoformat button, but no switch-to-form button", () => {
116+
renderForm();
117+
const input = screen.getByRole("textbox");
118+
expect(input).toHaveProperty("type", "textarea");
119+
const buttons = screen.getAllByRole("button");
120+
expect(buttons).toHaveLength(1);
121+
expect(buttons[0]).toHaveProperty("textContent", "Format JSON");
122+
});
123+
124+
it("should pass changed values to onChange", () => {
125+
const onChange = jest.fn();
126+
renderForm({ onChange });
127+
128+
const input = screen.getByRole("textbox");
129+
fireEvent.change(input, {
130+
target: { value: `{ "nested": "i am string" }` },
131+
});
132+
133+
// The onChange handler is debounced when using the JSON view, so we need to wait a little bit
134+
waitFor(() => {
135+
expect(onChange).toHaveBeenCalledWith(`{ "nested": "i am string" }`);
136+
});
137+
});
138+
});
139+
});

0 commit comments

Comments
 (0)