Merge branch 'main' into auth-fix
This commit is contained in:
commit
de71fb9d1c
9 changed files with 1302 additions and 513 deletions
|
|
@ -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,
|
||||
|
|
@ -771,12 +773,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++,
|
||||
},
|
||||
|
|
@ -787,6 +795,8 @@ const App = () => {
|
|||
);
|
||||
|
||||
setToolResult(response);
|
||||
// Clear any validation errors since tool execution completed
|
||||
setErrors((prev) => ({ ...prev, tools: null }));
|
||||
} catch (e) {
|
||||
const toolResult: CompatibilityCallToolResult = {
|
||||
content: [
|
||||
|
|
@ -798,6 +808,8 @@ const App = () => {
|
|||
isError: true,
|
||||
};
|
||||
setToolResult(toolResult);
|
||||
// Clear validation errors - tool execution errors are shown in ToolResults
|
||||
setErrors((prev) => ({ ...prev, tools: null }));
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -1,11 +1,11 @@
|
|||
import { Alert, AlertDescription } from "@/components/ui/alert";
|
||||
import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Checkbox } from "@/components/ui/checkbox";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { TabsContent } from "@/components/ui/tabs";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import DynamicJsonForm from "./DynamicJsonForm";
|
||||
import DynamicJsonForm, { DynamicJsonFormRef } from "./DynamicJsonForm";
|
||||
import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils";
|
||||
import {
|
||||
generateDefaultValue,
|
||||
|
|
@ -22,10 +22,11 @@ import {
|
|||
Send,
|
||||
ChevronDown,
|
||||
ChevronUp,
|
||||
AlertCircle,
|
||||
Copy,
|
||||
CheckCheck,
|
||||
} from "lucide-react";
|
||||
import { useEffect, useState } from "react";
|
||||
import { useEffect, useState, useRef } from "react";
|
||||
import ListPane from "./ListPane";
|
||||
import JsonView from "./JsonView";
|
||||
import ToolResults from "./ToolResults";
|
||||
|
|
@ -45,6 +46,7 @@ const ToolsTab = ({
|
|||
setSelectedTool,
|
||||
toolResult,
|
||||
nextCursor,
|
||||
error,
|
||||
resourceContent,
|
||||
onReadResource,
|
||||
}: {
|
||||
|
|
@ -64,9 +66,20 @@ const ToolsTab = ({
|
|||
const [isToolRunning, setIsToolRunning] = useState(false);
|
||||
const [isOutputSchemaExpanded, setIsOutputSchemaExpanded] = useState(false);
|
||||
const [isMetaExpanded, setIsMetaExpanded] = useState(false);
|
||||
const [hasValidationErrors, setHasValidationErrors] = useState(false);
|
||||
const formRefs = useRef<Record<string, DynamicJsonFormRef | null>>({});
|
||||
const { toast } = useToast();
|
||||
const { copied, setCopied } = useCopy();
|
||||
|
||||
// Function to check if any form has validation errors
|
||||
const checkValidationErrors = () => {
|
||||
const errors = Object.values(formRefs.current).some(
|
||||
(ref) => ref && !ref.validateJson().isValid,
|
||||
);
|
||||
setHasValidationErrors(errors);
|
||||
return errors;
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
const params = Object.entries(
|
||||
selectedTool?.inputSchema.properties ?? [],
|
||||
|
|
@ -79,6 +92,12 @@ const ToolsTab = ({
|
|||
),
|
||||
]);
|
||||
setParams(Object.fromEntries(params));
|
||||
|
||||
// Reset validation errors when switching tools
|
||||
setHasValidationErrors(false);
|
||||
|
||||
// Clear form refs for the previous tool
|
||||
formRefs.current = {};
|
||||
}, [selectedTool]);
|
||||
|
||||
return (
|
||||
|
|
@ -114,6 +133,13 @@ const ToolsTab = ({
|
|||
<div className="p-4">
|
||||
{selectedTool ? (
|
||||
<div className="space-y-4">
|
||||
{error && (
|
||||
<Alert variant="destructive">
|
||||
<AlertCircle className="h-4 w-4" />
|
||||
<AlertTitle>Error</AlertTitle>
|
||||
<AlertDescription>{error}</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
<p className="text-sm text-gray-600 dark:text-gray-400">
|
||||
{selectedTool.description}
|
||||
</p>
|
||||
|
|
@ -159,21 +185,33 @@ 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" ? (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
|
|
@ -189,6 +227,8 @@ const ToolsTab = ({
|
|||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
|
|
@ -199,19 +239,42 @@ 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"
|
||||
/>
|
||||
) : (
|
||||
<div className="mt-1">
|
||||
<DynamicJsonForm
|
||||
ref={(ref) => (formRefs.current[key] = ref)}
|
||||
schema={{
|
||||
type: prop.type,
|
||||
properties: prop.properties,
|
||||
|
|
@ -224,6 +287,8 @@ const ToolsTab = ({
|
|||
...params,
|
||||
[key]: newValue,
|
||||
});
|
||||
// Check validation after a short delay to allow form to update
|
||||
setTimeout(checkValidationErrors, 100);
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
|
|
@ -305,6 +370,9 @@ const ToolsTab = ({
|
|||
<div className="flex gap-2">
|
||||
<Button
|
||||
onClick={async () => {
|
||||
// Validate JSON inputs before calling tool
|
||||
if (checkValidationErrors()) return;
|
||||
|
||||
try {
|
||||
setIsToolRunning(true);
|
||||
await callTool(selectedTool.name, params);
|
||||
|
|
@ -312,7 +380,7 @@ const ToolsTab = ({
|
|||
setIsToolRunning(false);
|
||||
}
|
||||
}}
|
||||
disabled={isToolRunning}
|
||||
disabled={isToolRunning || hasValidationErrors}
|
||||
>
|
||||
{isToolRunning ? (
|
||||
<>
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||
import "@testing-library/jest-dom";
|
||||
import { describe, it, expect, jest } from "@jest/globals";
|
||||
import DynamicJsonForm from "../DynamicJsonForm";
|
||||
import { useRef } from "react";
|
||||
import DynamicJsonForm, { DynamicJsonFormRef } from "../DynamicJsonForm";
|
||||
import type { JsonSchemaType } from "@/utils/jsonUtils";
|
||||
|
||||
describe("DynamicJsonForm String Fields", () => {
|
||||
|
|
@ -663,3 +664,221 @@ describe("DynamicJsonForm Copy JSON Functionality", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("DynamicJsonForm Validation Functionality", () => {
|
||||
const renderFormWithRef = (props = {}) => {
|
||||
const TestComponent = () => {
|
||||
const formRef = useRef<DynamicJsonFormRef>(null);
|
||||
const defaultProps = {
|
||||
schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
nested: { oneOf: [{ type: "string" }, { type: "integer" }] },
|
||||
},
|
||||
} as unknown as JsonSchemaType,
|
||||
value: { nested: "test value" },
|
||||
onChange: jest.fn(),
|
||||
ref: formRef,
|
||||
};
|
||||
|
||||
return (
|
||||
<div>
|
||||
<DynamicJsonForm {...defaultProps} {...props} />
|
||||
<button
|
||||
onClick={() => {
|
||||
const result = formRef.current?.validateJson();
|
||||
// Add data attributes to make validation result testable
|
||||
const button = document.querySelector(
|
||||
'[data-testid="validate-button"]',
|
||||
) as HTMLElement;
|
||||
if (button && result) {
|
||||
button.setAttribute(
|
||||
"data-validation-valid",
|
||||
result.isValid.toString(),
|
||||
);
|
||||
button.setAttribute(
|
||||
"data-validation-error",
|
||||
result.error || "",
|
||||
);
|
||||
}
|
||||
}}
|
||||
data-testid="validate-button"
|
||||
>
|
||||
Validate
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
return render(<TestComponent />);
|
||||
};
|
||||
|
||||
describe("validateJson method", () => {
|
||||
it("should return valid for form mode", () => {
|
||||
const simpleSchema = {
|
||||
type: "string" as const,
|
||||
description: "Test string field",
|
||||
};
|
||||
|
||||
const TestComponent = () => {
|
||||
const formRef = useRef<DynamicJsonFormRef>(null);
|
||||
|
||||
return (
|
||||
<div>
|
||||
<DynamicJsonForm
|
||||
ref={formRef}
|
||||
schema={simpleSchema}
|
||||
value="test"
|
||||
onChange={jest.fn()}
|
||||
/>
|
||||
<button
|
||||
onClick={() => {
|
||||
const result = formRef.current?.validateJson();
|
||||
const button = document.querySelector(
|
||||
'[data-testid="validate-button"]',
|
||||
) as HTMLElement;
|
||||
if (button && result) {
|
||||
button.setAttribute(
|
||||
"data-validation-valid",
|
||||
result.isValid.toString(),
|
||||
);
|
||||
button.setAttribute(
|
||||
"data-validation-error",
|
||||
result.error || "",
|
||||
);
|
||||
}
|
||||
}}
|
||||
data-testid="validate-button"
|
||||
>
|
||||
Validate
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
render(<TestComponent />);
|
||||
|
||||
const validateButton = screen.getByTestId("validate-button");
|
||||
fireEvent.click(validateButton);
|
||||
|
||||
expect(validateButton.getAttribute("data-validation-valid")).toBe("true");
|
||||
expect(validateButton.getAttribute("data-validation-error")).toBe("");
|
||||
});
|
||||
|
||||
it("should return valid for valid JSON in JSON mode", () => {
|
||||
renderFormWithRef();
|
||||
|
||||
const validateButton = screen.getByTestId("validate-button");
|
||||
fireEvent.click(validateButton);
|
||||
|
||||
expect(validateButton.getAttribute("data-validation-valid")).toBe("true");
|
||||
expect(validateButton.getAttribute("data-validation-error")).toBe("");
|
||||
});
|
||||
|
||||
it("should return invalid for malformed JSON in JSON mode", async () => {
|
||||
renderFormWithRef();
|
||||
|
||||
// Enter invalid JSON
|
||||
const textarea = screen.getByRole("textbox");
|
||||
fireEvent.change(textarea, { target: { value: '{ "invalid": json }' } });
|
||||
|
||||
// Wait a bit for any debounced updates
|
||||
await waitFor(() => {
|
||||
const validateButton = screen.getByTestId("validate-button");
|
||||
fireEvent.click(validateButton);
|
||||
|
||||
expect(validateButton.getAttribute("data-validation-valid")).toBe(
|
||||
"false",
|
||||
);
|
||||
expect(validateButton.getAttribute("data-validation-error")).toContain(
|
||||
"JSON",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("should return valid for empty JSON in JSON mode", () => {
|
||||
renderFormWithRef();
|
||||
|
||||
// Clear the textarea
|
||||
const textarea = screen.getByRole("textbox");
|
||||
fireEvent.change(textarea, { target: { value: "" } });
|
||||
|
||||
const validateButton = screen.getByTestId("validate-button");
|
||||
fireEvent.click(validateButton);
|
||||
|
||||
expect(validateButton.getAttribute("data-validation-valid")).toBe("true");
|
||||
expect(validateButton.getAttribute("data-validation-error")).toBe("");
|
||||
});
|
||||
|
||||
it("should set error state when validation fails", async () => {
|
||||
renderFormWithRef();
|
||||
|
||||
// Enter invalid JSON
|
||||
const textarea = screen.getByRole("textbox");
|
||||
fireEvent.change(textarea, {
|
||||
target: { value: '{ "trailing": "comma", }' },
|
||||
});
|
||||
|
||||
// Trigger validation
|
||||
const validateButton = screen.getByTestId("validate-button");
|
||||
fireEvent.click(validateButton);
|
||||
|
||||
// Check that validation result shows error
|
||||
expect(validateButton.getAttribute("data-validation-valid")).toBe(
|
||||
"false",
|
||||
);
|
||||
expect(validateButton.getAttribute("data-validation-error")).toContain(
|
||||
"JSON",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("forwardRef functionality", () => {
|
||||
it("should expose validateJson method through ref", () => {
|
||||
const TestComponent = () => {
|
||||
const formRef = useRef<DynamicJsonFormRef>(null);
|
||||
|
||||
return (
|
||||
<div>
|
||||
<DynamicJsonForm
|
||||
ref={formRef}
|
||||
schema={{
|
||||
type: "object",
|
||||
properties: {
|
||||
test: { type: "string" },
|
||||
},
|
||||
}}
|
||||
value={{ test: "value" }}
|
||||
onChange={jest.fn()}
|
||||
/>
|
||||
<button
|
||||
onClick={() => {
|
||||
const hasValidateMethod =
|
||||
typeof formRef.current?.validateJson === "function";
|
||||
const button = document.querySelector(
|
||||
'[data-testid="ref-test-button"]',
|
||||
) as HTMLElement;
|
||||
if (button) {
|
||||
button.setAttribute(
|
||||
"data-has-validate-method",
|
||||
hasValidateMethod.toString(),
|
||||
);
|
||||
}
|
||||
}}
|
||||
data-testid="ref-test-button"
|
||||
>
|
||||
Test Ref
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
render(<TestComponent />);
|
||||
|
||||
const testButton = screen.getByTestId("ref-test-button");
|
||||
fireEvent.click(testButton);
|
||||
|
||||
expect(testButton.getAttribute("data-has-validate-method")).toBe("true");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -637,4 +637,189 @@ describe("ToolsTab", () => {
|
|||
expect(screen.getByText(/version/i)).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe("JSON Validation Integration", () => {
|
||||
const toolWithJsonParams: Tool = {
|
||||
name: "jsonTool",
|
||||
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,
|
||||
// No properties defined - this will force JSON mode
|
||||
},
|
||||
data: {
|
||||
type: "array" as const,
|
||||
// No items defined - this will force JSON mode
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
it("should prevent tool execution when JSON validation fails", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
renderToolsTab({
|
||||
tools: [toolWithJsonParams],
|
||||
selectedTool: toolWithJsonParams,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
// Find JSON editor textareas (there should be at least 1 for JSON parameters)
|
||||
const textareas = screen.getAllByRole("textbox");
|
||||
expect(textareas.length).toBeGreaterThanOrEqual(1);
|
||||
|
||||
// Enter invalid JSON in the first textarea
|
||||
const configTextarea = textareas[0];
|
||||
fireEvent.change(configTextarea, {
|
||||
target: { value: '{ "invalid": json }' },
|
||||
});
|
||||
|
||||
// Try to run the tool
|
||||
const runButton = screen.getByRole("button", { name: /run tool/i });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should not have been called due to validation failure
|
||||
expect(mockCallTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should allow tool execution when JSON validation passes", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
renderToolsTab({
|
||||
tools: [toolWithJsonParams],
|
||||
selectedTool: toolWithJsonParams,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
// 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 each textarea
|
||||
fireEvent.change(textareas[0], {
|
||||
target: { value: '{ "setting": "value" }' },
|
||||
});
|
||||
fireEvent.change(textareas[1], {
|
||||
target: { value: '["item1", "item2"]' },
|
||||
});
|
||||
|
||||
// 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 });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should have been called successfully
|
||||
expect(mockCallTool).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should handle mixed valid and invalid JSON parameters", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
renderToolsTab({
|
||||
tools: [toolWithJsonParams],
|
||||
selectedTool: toolWithJsonParams,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
const textareas = screen.getAllByRole("textbox");
|
||||
|
||||
// Enter invalid JSON that contains both valid and invalid parts
|
||||
fireEvent.change(textareas[0], {
|
||||
target: {
|
||||
value:
|
||||
'{ "config": { "setting": "value" }, "data": ["unclosed array" }',
|
||||
},
|
||||
});
|
||||
|
||||
// Try to run the tool
|
||||
const runButton = screen.getByRole("button", { name: /run tool/i });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should not have been called due to validation failure
|
||||
expect(mockCallTool).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should work with tools that have no JSON parameters", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
const simpleToolWithStringParam: Tool = {
|
||||
name: "simpleTool",
|
||||
description: "Tool with simple parameters",
|
||||
inputSchema: {
|
||||
type: "object" as const,
|
||||
properties: {
|
||||
message: { type: "string" as const },
|
||||
count: { type: "number" as const },
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
renderToolsTab({
|
||||
tools: [simpleToolWithStringParam],
|
||||
selectedTool: simpleToolWithStringParam,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
// Fill in the simple parameters
|
||||
const messageInput = screen.getByRole("textbox");
|
||||
const countInput = screen.getByRole("spinbutton");
|
||||
|
||||
fireEvent.change(messageInput, { target: { value: "test message" } });
|
||||
fireEvent.change(countInput, { target: { value: "5" } });
|
||||
|
||||
// Run the tool
|
||||
const runButton = screen.getByRole("button", { name: /run tool/i });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should have been called successfully (no JSON validation needed)
|
||||
expect(mockCallTool).toHaveBeenCalledWith(
|
||||
simpleToolWithStringParam.name,
|
||||
{
|
||||
message: "test message",
|
||||
count: 5,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle empty JSON parameters correctly", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
renderToolsTab({
|
||||
tools: [toolWithJsonParams],
|
||||
selectedTool: toolWithJsonParams,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
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: "[]" } });
|
||||
|
||||
// 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 });
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
|
||||
// Tool should have been called (empty JSON is considered valid)
|
||||
expect(mockCallTool).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
208
client/src/utils/__tests__/paramUtils.test.ts
Normal file
208
client/src/utils/__tests__/paramUtils.test.ts
Normal file
|
|
@ -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
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -39,12 +39,12 @@ describe("generateDefaultValue", () => {
|
|||
).toBe(false);
|
||||
});
|
||||
|
||||
test("generates default array", () => {
|
||||
expect(generateDefaultValue({ type: "array" })).toEqual([]);
|
||||
test("generates undefined for optional array", () => {
|
||||
expect(generateDefaultValue({ type: "array" })).toBe(undefined);
|
||||
});
|
||||
|
||||
test("generates default empty object", () => {
|
||||
expect(generateDefaultValue({ type: "object" })).toEqual({});
|
||||
test("generates undefined for optional object", () => {
|
||||
expect(generateDefaultValue({ type: "object" })).toBe(undefined);
|
||||
});
|
||||
|
||||
test("generates default null for unknown types", () => {
|
||||
|
|
@ -52,12 +52,32 @@ describe("generateDefaultValue", () => {
|
|||
expect(generateDefaultValue({ type: "unknown" })).toBe(undefined);
|
||||
});
|
||||
|
||||
test("generates empty array for non-required array", () => {
|
||||
expect(generateDefaultValue({ type: "array" })).toEqual([]);
|
||||
test("generates empty array for required array", () => {
|
||||
const parentSchema = { required: ["testArray"] };
|
||||
expect(
|
||||
generateDefaultValue({ type: "array" }, "testArray", parentSchema),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
test("generates empty object for non-required object", () => {
|
||||
expect(generateDefaultValue({ type: "object" })).toEqual({});
|
||||
test("generates undefined for non-required array", () => {
|
||||
const parentSchema = { required: ["otherField"] };
|
||||
expect(
|
||||
generateDefaultValue({ type: "array" }, "testArray", parentSchema),
|
||||
).toBe(undefined);
|
||||
});
|
||||
|
||||
test("generates empty object for required object", () => {
|
||||
const parentSchema = { required: ["testObject"] };
|
||||
expect(
|
||||
generateDefaultValue({ type: "object" }, "testObject", parentSchema),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
test("generates undefined for non-required object", () => {
|
||||
const parentSchema = { required: ["otherField"] };
|
||||
expect(
|
||||
generateDefaultValue({ type: "object" }, "testObject", parentSchema),
|
||||
).toBe(undefined);
|
||||
});
|
||||
|
||||
test("generates undefined for non-required primitive types", () => {
|
||||
|
|
|
|||
34
client/src/utils/paramUtils.ts
Normal file
34
client/src/utils/paramUtils.ts
Normal file
|
|
@ -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<string, unknown>,
|
||||
schema: JsonSchemaType,
|
||||
): Record<string, unknown> {
|
||||
const cleaned: Record<string, unknown> = {};
|
||||
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;
|
||||
}
|
||||
|
|
@ -109,9 +109,9 @@ export function generateDefaultValue(
|
|||
case "boolean":
|
||||
return isRequired ? false : undefined;
|
||||
case "array":
|
||||
return [];
|
||||
return isRequired ? [] : undefined;
|
||||
case "object": {
|
||||
if (!schema.properties) return {};
|
||||
if (!schema.properties) return isRequired ? {} : undefined;
|
||||
|
||||
const obj: JsonObject = {};
|
||||
// Only include properties that are required according to the schema's required array
|
||||
|
|
@ -123,7 +123,7 @@ export function generateDefaultValue(
|
|||
}
|
||||
}
|
||||
});
|
||||
return obj;
|
||||
return isRequired ? obj : Object.keys(obj).length > 0 ? obj : undefined;
|
||||
}
|
||||
case "null":
|
||||
return null;
|
||||
|
|
|
|||
Loading…
Reference in a new issue