Merge pull request #850 from gregsaab/greg/add_null_support_to_tools
Add support for specifying `null` values for tool fields
This commit is contained in:
Cliff Hall
GitHub
commit
ccb68b9253
5 changed files with 250 additions and 167 deletions
|
|
@ -160,184 +160,214 @@ const ToolsTab = ({
|
|||
const required = isPropertyRequired(key, inputSchema);
|
||||
return (
|
||||
<div key={key}>
|
||||
<Label
|
||||
htmlFor={key}
|
||||
className="block text-sm font-medium text-gray-700 dark:text-gray-300"
|
||||
>
|
||||
{key}
|
||||
{required && (
|
||||
<span className="text-red-500 ml-1">*</span>
|
||||
)}
|
||||
</Label>
|
||||
{prop.type === "boolean" ? (
|
||||
<div className="flex items-center space-x-2 mt-2">
|
||||
<Checkbox
|
||||
id={key}
|
||||
name={key}
|
||||
checked={!!params[key]}
|
||||
onCheckedChange={(checked: boolean) =>
|
||||
setParams({
|
||||
...params,
|
||||
[key]: checked,
|
||||
})
|
||||
}
|
||||
/>
|
||||
<label
|
||||
htmlFor={key}
|
||||
className="text-sm font-medium text-gray-700 dark:text-gray-300"
|
||||
>
|
||||
{prop.description || "Toggle this option"}
|
||||
</label>
|
||||
</div>
|
||||
) : prop.type === "string" && prop.enum ? (
|
||||
<Select
|
||||
value={
|
||||
params[key] === undefined
|
||||
? ""
|
||||
: String(params[key])
|
||||
}
|
||||
onValueChange={(value) => {
|
||||
if (value === "") {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: undefined,
|
||||
});
|
||||
} else {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: value,
|
||||
});
|
||||
}
|
||||
}}
|
||||
<div className="flex justify-between">
|
||||
<Label
|
||||
htmlFor={key}
|
||||
className="block text-sm font-medium text-gray-700 dark:text-gray-300"
|
||||
>
|
||||
<SelectTrigger id={key} className="mt-1">
|
||||
<SelectValue
|
||||
placeholder={
|
||||
prop.description || "Select an option"
|
||||
}
|
||||
/>
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{prop.enum.map((option) => (
|
||||
<SelectItem key={option} value={option}>
|
||||
{option}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
) : prop.type === "string" ? (
|
||||
<Textarea
|
||||
id={key}
|
||||
name={key}
|
||||
placeholder={prop.description}
|
||||
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" ? (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
description: prop.description,
|
||||
items: prop.items,
|
||||
}}
|
||||
value={
|
||||
(params[key] as JsonValue) ??
|
||||
generateDefaultValue(prop)
|
||||
}
|
||||
onChange={(newValue: JsonValue) => {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
) : prop.type === "number" ||
|
||||
prop.type === "integer" ? (
|
||||
<Input
|
||||
type="number"
|
||||
id={key}
|
||||
name={key}
|
||||
placeholder={prop.description}
|
||||
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 - try to convert to number, but store input either way
|
||||
const num = Number(value);
|
||||
if (!isNaN(num)) {
|
||||
{key}
|
||||
{required && (
|
||||
<span className="text-red-500 ml-1">*</span>
|
||||
)}
|
||||
</Label>
|
||||
{prop.nullable ? (
|
||||
<div className="flex items-center space-x-2">
|
||||
<Checkbox
|
||||
id={key}
|
||||
name={key}
|
||||
checked={params[key] === null}
|
||||
onCheckedChange={(checked: boolean) =>
|
||||
setParams({
|
||||
...params,
|
||||
[key]: num,
|
||||
[key]: checked ? null : prop.default,
|
||||
})
|
||||
}
|
||||
/>
|
||||
<label
|
||||
htmlFor={key}
|
||||
className="text-sm font-medium text-gray-700 dark:text-gray-300"
|
||||
>
|
||||
null
|
||||
</label>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
<div
|
||||
role="toolinputwrapper"
|
||||
className={`${prop.nullable && params[key] === null ? "pointer-events-none opacity-50" : ""}`}
|
||||
>
|
||||
{prop.type === "boolean" ? (
|
||||
<div className="flex items-center space-x-2 mt-2">
|
||||
<Checkbox
|
||||
id={key}
|
||||
name={key}
|
||||
checked={!!params[key]}
|
||||
onCheckedChange={(checked: boolean) =>
|
||||
setParams({
|
||||
...params,
|
||||
[key]: checked,
|
||||
})
|
||||
}
|
||||
/>
|
||||
<label
|
||||
htmlFor={key}
|
||||
className="text-sm font-medium text-gray-700 dark:text-gray-300"
|
||||
>
|
||||
{prop.description || "Toggle this option"}
|
||||
</label>
|
||||
</div>
|
||||
) : prop.type === "string" && prop.enum ? (
|
||||
<Select
|
||||
value={
|
||||
params[key] === undefined
|
||||
? ""
|
||||
: String(params[key])
|
||||
}
|
||||
onValueChange={(value) => {
|
||||
if (value === "") {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: undefined,
|
||||
});
|
||||
} else {
|
||||
// Store invalid input as string - let server validate
|
||||
setParams({
|
||||
...params,
|
||||
[key]: value,
|
||||
});
|
||||
}
|
||||
}}
|
||||
>
|
||||
<SelectTrigger id={key} className="mt-1">
|
||||
<SelectValue
|
||||
placeholder={
|
||||
prop.description || "Select an option"
|
||||
}
|
||||
/>
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{prop.enum.map((option) => (
|
||||
<SelectItem key={option} value={option}>
|
||||
{option}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
) : prop.type === "string" ? (
|
||||
<Textarea
|
||||
id={key}
|
||||
name={key}
|
||||
placeholder={prop.description}
|
||||
value={
|
||||
params[key] === undefined
|
||||
? ""
|
||||
: String(params[key])
|
||||
}
|
||||
}}
|
||||
className="mt-1"
|
||||
/>
|
||||
) : (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
description: prop.description,
|
||||
items: prop.items,
|
||||
}}
|
||||
value={params[key] as JsonValue}
|
||||
onChange={(newValue: JsonValue) => {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
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"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
) : prop.type === "object" ||
|
||||
prop.type === "array" ? (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
description: prop.description,
|
||||
items: prop.items,
|
||||
}}
|
||||
value={
|
||||
(params[key] as JsonValue) ??
|
||||
generateDefaultValue(prop)
|
||||
}
|
||||
onChange={(newValue: JsonValue) => {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
) : prop.type === "number" ||
|
||||
prop.type === "integer" ? (
|
||||
<Input
|
||||
type="number"
|
||||
id={key}
|
||||
name={key}
|
||||
placeholder={prop.description}
|
||||
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 - 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"
|
||||
/>
|
||||
) : (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
description: prop.description,
|
||||
items: prop.items,
|
||||
}}
|
||||
value={params[key] as JsonValue}
|
||||
onChange={(newValue: JsonValue) => {
|
||||
setParams({
|
||||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
},
|
||||
|
|
|
|||
|
|
@ -44,6 +44,16 @@ describe("ToolsTab", () => {
|
|||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "tool4",
|
||||
description: "Tool with nullable field",
|
||||
inputSchema: {
|
||||
type: "object" as const,
|
||||
properties: {
|
||||
num: { type: ["number", "null"] as const },
|
||||
},
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const defaultProps = {
|
||||
|
|
@ -135,6 +145,38 @@ describe("ToolsTab", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("should allow specifying null value", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
const toolWithNullableField = mockTools[3];
|
||||
|
||||
renderToolsTab({
|
||||
tools: [toolWithNullableField],
|
||||
selectedTool: toolWithNullableField,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
const nullToggleButton = screen.getByRole("checkbox", { name: /null/i });
|
||||
expect(nullToggleButton).toBeInTheDocument();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(nullToggleButton);
|
||||
});
|
||||
|
||||
expect(screen.getByRole("toolinputwrapper").classList).toContain(
|
||||
"pointer-events-none",
|
||||
);
|
||||
|
||||
const runButton = screen.getByRole("button", { name: /run tool/i });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should have been called with null value
|
||||
expect(mockCallTool).toHaveBeenCalledWith(toolWithNullableField.name, {
|
||||
num: 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;
|
||||
|
|
|
|||
|
|
@ -188,6 +188,7 @@ describe("normalizeUnionType", () => {
|
|||
expect(normalized.type).toBe("boolean");
|
||||
expect(normalized.anyOf).toBeUndefined();
|
||||
expect(normalized.description).toBe("Optional boolean parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes array type with string and null to string type", () => {
|
||||
|
|
@ -200,6 +201,7 @@ describe("normalizeUnionType", () => {
|
|||
|
||||
expect(normalized.type).toBe("string");
|
||||
expect(normalized.description).toBe("Optional string parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes array type with boolean and null to boolean type", () => {
|
||||
|
|
@ -212,6 +214,7 @@ describe("normalizeUnionType", () => {
|
|||
|
||||
expect(normalized.type).toBe("boolean");
|
||||
expect(normalized.description).toBe("Optional boolean parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes anyOf with number and null to number type", () => {
|
||||
|
|
@ -225,6 +228,7 @@ describe("normalizeUnionType", () => {
|
|||
expect(normalized.type).toBe("number");
|
||||
expect(normalized.anyOf).toBeUndefined();
|
||||
expect(normalized.description).toBe("Optional number parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes anyOf with integer and null to integer type", () => {
|
||||
|
|
@ -238,6 +242,7 @@ describe("normalizeUnionType", () => {
|
|||
expect(normalized.type).toBe("integer");
|
||||
expect(normalized.anyOf).toBeUndefined();
|
||||
expect(normalized.description).toBe("Optional integer parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes array type with number and null to number type", () => {
|
||||
|
|
@ -250,6 +255,7 @@ describe("normalizeUnionType", () => {
|
|||
|
||||
expect(normalized.type).toBe("number");
|
||||
expect(normalized.description).toBe("Optional number parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("normalizes array type with integer and null to integer type", () => {
|
||||
|
|
@ -262,6 +268,7 @@ describe("normalizeUnionType", () => {
|
|||
|
||||
expect(normalized.type).toBe("integer");
|
||||
expect(normalized.description).toBe("Optional integer parameter");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("handles anyOf with reversed order (null first)", () => {
|
||||
|
|
@ -273,6 +280,7 @@ describe("normalizeUnionType", () => {
|
|||
|
||||
expect(normalized.type).toBe("string");
|
||||
expect(normalized.anyOf).toBeUndefined();
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
|
||||
test("leaves non-union schemas unchanged", () => {
|
||||
|
|
@ -294,6 +302,7 @@ describe("normalizeUnionType", () => {
|
|||
const normalized = normalizeUnionType(schema);
|
||||
|
||||
expect(normalized).toEqual(schema);
|
||||
expect(normalized.nullable).toBeFalsy();
|
||||
});
|
||||
|
||||
test("leaves anyOf with more than two types unchanged", () => {
|
||||
|
|
@ -343,6 +352,7 @@ describe("normalizeUnionType", () => {
|
|||
expect(normalized.minLength).toBe(1);
|
||||
expect(normalized.maxLength).toBe(100);
|
||||
expect(normalized.pattern).toBe("^[a-z]+$");
|
||||
expect(normalized.nullable).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ export type JsonSchemaType = {
|
|||
maximum?: number;
|
||||
minLength?: number;
|
||||
maxLength?: number;
|
||||
nullable?: boolean;
|
||||
pattern?: string;
|
||||
format?: string;
|
||||
enum?: string[];
|
||||
|
|
|
|||
|
|
@ -158,7 +158,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "string") &&
|
||||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "null")
|
||||
) {
|
||||
return { ...schema, type: "string", anyOf: undefined };
|
||||
return { ...schema, type: "string", anyOf: undefined, nullable: true };
|
||||
}
|
||||
|
||||
// Handle anyOf with exactly boolean and null (FastMCP pattern)
|
||||
|
|
@ -168,7 +168,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "boolean") &&
|
||||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "null")
|
||||
) {
|
||||
return { ...schema, type: "boolean", anyOf: undefined };
|
||||
return { ...schema, type: "boolean", anyOf: undefined, nullable: true };
|
||||
}
|
||||
|
||||
// Handle anyOf with exactly number and null (FastMCP pattern)
|
||||
|
|
@ -178,7 +178,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "number") &&
|
||||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "null")
|
||||
) {
|
||||
return { ...schema, type: "number", anyOf: undefined };
|
||||
return { ...schema, type: "number", anyOf: undefined, nullable: true };
|
||||
}
|
||||
|
||||
// Handle anyOf with exactly integer and null (FastMCP pattern)
|
||||
|
|
@ -188,7 +188,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "integer") &&
|
||||
schema.anyOf.some((t) => (t as JsonSchemaType).type === "null")
|
||||
) {
|
||||
return { ...schema, type: "integer", anyOf: undefined };
|
||||
return { ...schema, type: "integer", anyOf: undefined, nullable: true };
|
||||
}
|
||||
|
||||
// Handle array type with exactly string and null
|
||||
|
|
@ -198,7 +198,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.type.includes("string") &&
|
||||
schema.type.includes("null")
|
||||
) {
|
||||
return { ...schema, type: "string" };
|
||||
return { ...schema, type: "string", nullable: true };
|
||||
}
|
||||
|
||||
// Handle array type with exactly boolean and null
|
||||
|
|
@ -208,7 +208,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.type.includes("boolean") &&
|
||||
schema.type.includes("null")
|
||||
) {
|
||||
return { ...schema, type: "boolean" };
|
||||
return { ...schema, type: "boolean", nullable: true };
|
||||
}
|
||||
|
||||
// Handle array type with exactly number and null
|
||||
|
|
@ -218,7 +218,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.type.includes("number") &&
|
||||
schema.type.includes("null")
|
||||
) {
|
||||
return { ...schema, type: "number" };
|
||||
return { ...schema, type: "number", nullable: true };
|
||||
}
|
||||
|
||||
// Handle array type with exactly integer and null
|
||||
|
|
@ -228,7 +228,7 @@ export function normalizeUnionType(schema: JsonSchemaType): JsonSchemaType {
|
|||
schema.type.includes("integer") &&
|
||||
schema.type.includes("null")
|
||||
) {
|
||||
return { ...schema, type: "integer" };
|
||||
return { ...schema, type: "integer", nullable: true };
|
||||
}
|
||||
|
||||
return schema;
|
||||
|
|
|
|||
Loading…
Reference in a new issue