fix(ui): preserve clear and default semantics in local forms

This commit is contained in:
Yuneng Jiang 2026-09-11 18:58:58 -07:00
parent f84f986b4e
commit 707b779c5d
No known key found for this signature in database
16 changed files with 312 additions and 119 deletions

View file

@ -74,7 +74,7 @@ const CacheFormField: React.FC<CacheFormFieldProps> = ({ field, embeddingModels,
name={name}
disabled={disabled}
value={typeof value === "string" && value !== "" ? value : null}
onValueChange={(selected: string | null) => onChange(selected ?? "")}
onValueChange={onChange}
>
<SelectTrigger
id={id}
@ -101,7 +101,7 @@ const CacheFormField: React.FC<CacheFormFieldProps> = ({ field, embeddingModels,
<Combobox
items={embeddingModels}
value={selected}
onValueChange={(model: EmbeddingModelOption | null) => onChange(model?.value ?? "")}
onValueChange={(model: EmbeddingModelOption | null) => onChange(model?.value ?? null)}
itemToStringLabel={(model: EmbeddingModelOption) => model.label}
isItemEqualToValue={(model: EmbeddingModelOption, other: EmbeddingModelOption) =>
model.value === other.value

View file

@ -1,6 +1,6 @@
import { CACHE_FIELDS, CacheField, CacheSection, REDACTED_VALUE, RedisType } from "./cacheSettingsFields";
export type CacheFormValue = string | number | boolean | undefined;
export type CacheFormValue = string | number | boolean | null | undefined;
export type CacheFormValues = Record<string, CacheFormValue>;
export type CacheSavePayloadValue = string | number | boolean | unknown[];
export type CacheSavePayload = Record<string, CacheSavePayloadValue>;
@ -38,6 +38,9 @@ const initialValueForField = (field: CacheField, raw: unknown): CacheFormValue =
return typeof source === "string" ? source : JSON.stringify(source, null, 2);
}
if ((field.type === "select" || field.type === "model-select") && !hasValue(source)) {
return null;
}
if (source === undefined || source === null) {
return "";
}
@ -77,7 +80,7 @@ const saveValueForField = (field: CacheField, raw: CacheFormValue): CacheSavePay
}
if (typeof raw !== "string") {
return raw === undefined ? undefined : String(raw);
return raw == null ? undefined : String(raw);
}
const trimmed = raw.trim();
return trimmed === "" ? undefined : trimmed;

View file

@ -1,7 +1,8 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { fireEvent, renderWithProviders, screen, waitFor } from "../../../../../../tests/test-utils";
import userEvent from "@testing-library/user-event";
import CacheSettings from "./index";
import { fetchAvailableModels } from "@/components/llm_calls/fetch_models";
const { getCacheSettingsCall, testCacheConnectionCall, updateCacheSettingsCall } = vi.hoisted(() => ({
getCacheSettingsCall: vi.fn(),
@ -29,7 +30,7 @@ const LOADED_WITH_ADVANCED = {
},
};
const renderSettings = () => render(<CacheSettings accessToken="sk-test" userRole="Admin" userID="u1" />);
const renderSettings = () => renderWithProviders(<CacheSettings accessToken="sk-test" userRole="Admin" userID="u1" />);
const save = async (user: ReturnType<typeof userEvent.setup>) =>
user.click(screen.getByRole("button", { name: /save changes/i }));
@ -174,4 +175,42 @@ describe("CacheSettings advanced settings round-trip", () => {
await waitFor(() => expect(updateCacheSettingsCall).toHaveBeenCalledTimes(1));
expect(updateCacheSettingsCall.mock.calls[0][1]).not.toHaveProperty("ttl");
});
it("should omit a cleared cache model from save and test while retaining other settings", async () => {
vi.mocked(fetchAvailableModels).mockResolvedValue([
{ model_group: "synthetic-embedding", mode: "embedding" },
] as Awaited<ReturnType<typeof fetchAvailableModels>>);
getCacheSettingsCall.mockResolvedValue({
current_values: {
redis_type: "semantic",
host: "localhost",
redis_semantic_cache_embedding_model: "synthetic-embedding",
password: "***REDACTED***",
ttl: 0,
ssl: false,
namespace: "synthetic-cache",
},
});
const user = userEvent.setup();
renderSettings();
await screen.findByRole("combobox", { name: "Embedding Model" });
await user.click(screen.getByRole("button", { name: "Clear" }));
const expected = {
type: "redis",
host: "localhost",
port: "6379",
similarity_threshold: 0.8,
semantic_cache_scope: "key",
ssl: false,
ssl_check_hostname: false,
ttl: 0,
namespace: "synthetic-cache",
};
await user.click(screen.getByRole("button", { name: "Test Connection" }));
await waitFor(() => expect(testCacheConnectionCall).toHaveBeenCalledWith("sk-test", expected));
await save(user);
await waitFor(() =>
expect(updateCacheSettingsCall).toHaveBeenCalledWith("sk-test", { ...expected, type: "redis-semantic" }),
);
});
});

View file

@ -79,13 +79,16 @@ const ToolArgumentControl: React.FC<{
return (
<select
{...control}
value={(control.value as string) ?? ""}
value={control.value == null ? -1 : prop.enum.indexOf(String(control.value))}
onChange={(event) => control.onChange(prop.enum?.[Number(event.target.value)] ?? null)}
className="w-full rounded-lg border border-input bg-transparent px-3 py-2 text-sm shadow-xs transition-colors focus:border-ring focus:ring-3 focus:ring-ring/50 focus:outline-hidden"
>
{!field.required && <option value="">Select {field.key}</option>}
{prop.enum.map((option) => (
<option key={option} value={option}>
{option}
<option value={-1} disabled={field.required}>
Select {field.key}
</option>
{prop.enum.map((option, index) => (
<option key={option} value={index}>
{option === "" ? "Empty string" : option}
</option>
))}
</select>
@ -108,8 +111,8 @@ const ToolArgumentControl: React.FC<{
if (prop.type === "boolean") {
return (
<Select
items={field.required ? BOOLEAN_ITEMS : [{ value: "", label: `Select ${field.key}` }, ...BOOLEAN_ITEMS]}
value={control.value ?? ""}
items={field.required ? BOOLEAN_ITEMS : [{ value: null, label: `Select ${field.key}` }, ...BOOLEAN_ITEMS]}
value={control.value ?? null}
onValueChange={control.onChange}
>
<SelectTrigger
@ -121,7 +124,7 @@ const ToolArgumentControl: React.FC<{
<SelectValue placeholder={`Select ${field.key}`} />
</SelectTrigger>
<SelectContent>
{!field.required && <SelectItem value="">Select {field.key}</SelectItem>}
{!field.required && <SelectItem value={null}>Select {field.key}</SelectItem>}
<SelectItem value={true}>True</SelectItem>
<SelectItem value={false}>False</SelectItem>
</SelectContent>

View file

@ -1,5 +1,5 @@
import React from "react";
import { fireEvent, render, screen } from "@testing-library/react";
import { fireEvent, renderWithProviders, screen } from "../../../../../tests/test-utils";
import userEvent from "@testing-library/user-event";
import type { UserEvent } from "@testing-library/user-event";
import { describe, expect, it, vi, type Mock } from "vitest";
@ -16,7 +16,7 @@ const buildTool = (schema: InputSchema | string): MCPTool => ({
});
const renderPanel = (schema: InputSchema | string) =>
render(
renderWithProviders(
<ToolTestPanel
tool={buildTool(schema)}
onSubmit={vi.fn()}
@ -84,10 +84,10 @@ describe("ToolTestPanel defaults", () => {
expect(screen.getByLabelText("ratio")).toHaveValue(0.4);
expect(screen.getByTitle("True")).toBeInTheDocument();
const keywordsTextarea = screen.getByTestId("textarea-keywords");
const keywordsTextarea = screen.getByTestId<HTMLTextAreaElement>("textarea-keywords");
expect(JSON.parse(keywordsTextarea.value)).toEqual([""]);
const payloadTextarea = screen.getByTestId("textarea-payload");
const payloadTextarea = screen.getByTestId<HTMLTextAreaElement>("textarea-payload");
expect(JSON.parse(payloadTextarea.value)).toEqual({
user: {
id: "",
@ -131,7 +131,7 @@ describe("ToolTestPanel defaults", () => {
renderPanel(schema);
expect(screen.getByLabelText("query")).toBeInTheDocument();
const filtersTextarea = screen.getByTestId("textarea-filters");
const filtersTextarea = screen.getByTestId<HTMLTextAreaElement>("textarea-filters");
expect(JSON.parse(filtersTextarea.value)).toEqual({
tag: "",
metadata: { source: "" },
@ -164,7 +164,7 @@ describe("ToolTestPanel defaults", () => {
describe("ToolTestPanel argument payload", () => {
const submitPanel = async (schema: InputSchema | string, drive?: (user: UserEvent) => Promise<void>) => {
const onSubmit = vi.fn();
render(
renderWithProviders(
<ToolTestPanel
tool={buildTool(schema)}
onSubmit={onSubmit}
@ -195,7 +195,7 @@ describe("ToolTestPanel argument payload", () => {
it("sends what the user typed into the fallback input when the tool has no real schema", async () => {
const onSubmit = vi.fn();
render(
renderWithProviders(
<ToolTestPanel
tool={buildTool("tool_input_schema")}
onSubmit={onSubmit}
@ -244,8 +244,8 @@ describe("ToolTestPanel argument payload", () => {
properties: { payload: { type: "object" }, tags: { type: "array" } },
},
async (user) => {
await user.clear(screen.getByTestId("textarea-payload"));
await user.type(screen.getByTestId("textarea-payload"), '{{"a":1}');
await user.clear(screen.getByTestId<HTMLTextAreaElement>("textarea-payload"));
await user.type(screen.getByTestId<HTMLTextAreaElement>("textarea-payload"), '{{"a":1}');
await user.clear(screen.getByTestId("textarea-tags"));
await user.type(screen.getByTestId("textarea-tags"), '[["x","y"]');
},
@ -279,7 +279,7 @@ describe("ToolTestPanel argument payload", () => {
const onSubmit = await submitPanel(
{ type: "object", properties: { mode: { type: "string", enum: ["fast", "thorough"] } } },
async (user) => {
await user.selectOptions(screen.getByLabelText("mode"), "thorough");
await user.selectOptions(screen.getByLabelText("mode"), screen.getByRole("option", { name: "thorough" }));
},
);
@ -317,8 +317,8 @@ describe("ToolTestPanel argument payload", () => {
const onSubmit = await submitPanel(
{ type: "object", properties: { payload: { type: "object" } } },
async (user) => {
await user.clear(screen.getByTestId("textarea-payload"));
await user.type(screen.getByTestId("textarea-payload"), "not json");
await user.clear(screen.getByTestId<HTMLTextAreaElement>("textarea-payload"));
await user.type(screen.getByTestId<HTMLTextAreaElement>("textarea-payload"), "not json");
},
);
@ -360,7 +360,7 @@ describe("ToolTestPanel schema changes under a stable tool name", () => {
properties: { query: { type: "string" }, limit: { type: "integer", default: 5 } },
};
const { rerender } = render(renderWith(before, onSubmit));
const { rerender } = renderWithProviders(renderWith(before, onSubmit));
const user = userEvent.setup();
await user.type(screen.getByLabelText("message"), "stale value");
@ -380,7 +380,7 @@ describe("ToolTestPanel schema changes under a stable tool name", () => {
const onSubmit = vi.fn();
const schema = (): InputSchema => ({ type: "object", properties: { message: { type: "string" } } });
const { rerender } = render(renderWith(schema(), onSubmit));
const { rerender } = renderWithProviders(renderWith(schema(), onSubmit));
const user = userEvent.setup();
await user.type(screen.getByLabelText("message"), "typed by hand");
@ -404,7 +404,7 @@ describe("ToolTestPanel optional union-typed parameters", () => {
const runPanel = async (drive: () => void) => {
const onSubmit = vi.fn();
render(
renderWithProviders(
<ToolTestPanel
tool={buildTool(qaEchoSchema)}
onSubmit={onSubmit}
@ -437,7 +437,7 @@ describe("ToolTestPanel optional union-typed parameters", () => {
},
});
const payload = screen.getByTestId("textarea-payload");
const payload = screen.getByTestId<HTMLTextAreaElement>("textarea-payload");
expect(payload).toHaveValue(JSON.stringify({ id: "" }, null, 2));
expect(screen.getByPlaceholderText("Enter JSON object for payload")).toBe(payload);
expect(screen.queryByPlaceholderText("Enter payload")).not.toBeInTheDocument();
@ -471,3 +471,37 @@ describe("ToolTestPanel optional union-typed parameters", () => {
expect(onSubmit).not.toHaveBeenCalled();
});
});
it("should submit an empty enum choice while omitting unset choices and retaining false", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
renderWithProviders(
<ToolTestPanel
tool={buildTool({
type: "object",
properties: {
mode: { type: "string", enum: ["", "fast"], default: "fast" },
active: { type: "boolean", default: true },
},
})}
onSubmit={onSubmit}
isLoading={false}
result={null}
error={null}
onClose={vi.fn()}
/>,
);
await user.selectOptions(
screen.getByRole("combobox", { name: "mode" }),
screen.getByRole("option", { name: "Select mode" }),
);
await chooseSelectOption(user, screen.getByRole("combobox", { name: "active" }), "False");
await user.click(screen.getByRole("button", { name: "Call Tool" }));
expect(onSubmit).toHaveBeenLastCalledWith({ active: false });
await user.selectOptions(
screen.getByRole("combobox", { name: "mode" }),
screen.getByRole("option", { name: "Empty string" }),
);
await user.click(screen.getByRole("button", { name: "Call Tool" }));
expect(onSubmit).toHaveBeenLastCalledWith({ mode: "", active: false });
});

View file

@ -48,12 +48,18 @@ const parseJson = (raw: unknown): ParsedJson => {
const isBlank = (value: unknown): boolean => value === undefined || value === null || value === "";
const isUnsetArgument = (prop: InputSchemaProperty, value: unknown): boolean =>
prop.type === "string" && prop.enum ? value == null : isBlank(typeof value === "string" ? value.trim() : value);
export const validateToolArgument = (field: ToolArgumentField, value: unknown): string | undefined => {
const prop = resolveSchemaProperty(field.prop);
const normalized = typeof value === "string" ? value.trim() : value;
if (field.required && isBlank(normalized)) {
if (field.required && isUnsetArgument(prop, value)) {
return `Please enter ${field.key}`;
}
if (prop.type === "string" && prop.enum) {
if (!isUnsetArgument(prop, value) && !prop.enum.includes(String(value)))
return `Please select a valid ${field.key}`;
}
if (!isJsonField(prop) || (isBlank(value) && !field.required)) {
return undefined;
}
@ -72,7 +78,7 @@ export const validateToolArgument = (field: ToolArgumentField, value: unknown):
const coerceArgument = (declared: InputSchemaProperty, value: unknown): unknown => {
const prop = resolveSchemaProperty(declared);
const normalized = typeof value === "string" ? value.trim() : value;
const normalized = typeof value === "string" && !prop.enum ? value.trim() : value;
switch (prop.type) {
case "boolean":
return normalized === "true" || normalized === true;
@ -104,7 +110,7 @@ export const buildToolCallArguments = (
Object.fromEntries(
fields
.map((field, index) => ({ field, value: values[index] }))
.filter(({ value }) => !isBlank(typeof value === "string" ? value.trim() : value))
.filter(({ field, value }) => !isUnsetArgument(resolveSchemaProperty(field.prop), value))
.map(({ field, value }) => [field.key, coerceArgument(field.prop, value)]),
);
@ -199,6 +205,7 @@ function buildDefaultValue(declared: InputSchemaProperty | undefined, overrideDe
export const initialArgumentValues = (fields: readonly ToolArgumentField[]): unknown[] =>
fields.map(({ prop }) => {
const resolved = resolveSchemaProperty(prop);
if (resolved.type === "string" && resolved.enum && resolved.default === undefined) return null;
const defaultValue = buildDefaultValue(resolved);
if (isJsonField(resolved)) {
return isBlank(defaultValue) ? "" : JSON.stringify(defaultValue, null, 2);

View file

@ -1,6 +1,6 @@
import { renderWithProviders, screen, within } from "../../../../../tests/test-utils";
import userEvent from "@testing-library/user-event";
import { vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import GeneralSettings from "./general_settings";
import { deleteConfigFieldSetting, getGeneralSettingsCall, updateConfigFieldSetting } from "@/components/networking";
@ -116,3 +116,48 @@ describe("GeneralSettings tabs", () => {
expect(screen.queryByRole("tab", { name: /auto.?router/i })).not.toBeInTheDocument();
});
});
it("should delete only the Default setting and retain explicit false and zero", async () => {
vi.mocked(getGeneralSettingsCall).mockResolvedValue([
{
field_name: "synthetic_choice",
field_type: "Select",
field_value: "enabled",
field_options: ["enabled"],
field_description: "choice",
stored_in_db: true,
},
{
field_name: "synthetic_flag",
field_type: "Boolean",
field_value: false,
field_description: "flag",
stored_in_db: true,
},
{
field_name: "synthetic_count",
field_type: "Integer",
field_value: 0,
field_description: "count",
stored_in_db: true,
},
]);
vi.mocked(updateConfigFieldSetting).mockClear();
vi.mocked(deleteConfigFieldSetting).mockClear();
const user = userEvent.setup();
renderWithProviders(<GeneralSettings accessToken="token" userRole="Admin" userID="user" />);
await user.click(screen.getByRole("tab", { name: "General" }));
const row = await screen.findByRole("row", { name: /synthetic_choice/ });
await user.click(within(row).getByRole("combobox"));
await user.click(await screen.findByRole("option", { name: "Default" }));
await user.click(within(row).getByRole("button", { name: "Update" }));
await user.click(within(screen.getByRole("row", { name: /synthetic_flag/ })).getByRole("button", { name: "Update" }));
await user.click(
within(screen.getByRole("row", { name: /synthetic_count/ })).getByRole("button", { name: "Update" }),
);
expect(vi.mocked(deleteConfigFieldSetting).mock.calls).toEqual([["token", "synthetic_choice"]]);
expect(vi.mocked(updateConfigFieldSetting).mock.calls).toEqual([
["token", "synthetic_flag", false],
["token", "synthetic_count", 0],
]);
});

View file

@ -92,10 +92,7 @@ const SettingValueEditor: React.FC<{
}
if (setting.field_type === "Select") {
return (
<Select
value={setting.field_value || null}
onValueChange={(newValue) => onChange(setting.field_name, newValue ?? "")}
>
<Select value={setting.field_value ?? null} onValueChange={(newValue) => onChange(setting.field_name, newValue)}>
<SelectTrigger className="min-w-32">
<SelectValue placeholder="Default" />
</SelectTrigger>
@ -161,8 +158,8 @@ export const PromptCachingPanel: React.FC<{
</div>
<Select
disabled={!enabled}
value={ttlSetting.field_value || null}
onValueChange={(newValue) => persist(ANTHROPIC_PROMPT_CACHING_TTL, newValue ?? "")}
value={ttlSetting.field_value ?? null}
onValueChange={(newValue) => persist(ANTHROPIC_PROMPT_CACHING_TTL, newValue)}
>
<SelectTrigger className="min-w-40">
<SelectValue placeholder="5m (default)" />
@ -209,9 +206,11 @@ const GeneralSettings: React.FC<GeneralSettingsPageProps> = ({ accessToken, user
return;
}
let fieldValue = generalSettings.find((setting) => setting.field_name === fieldName)?.field_value;
const setting = generalSettings.find((setting) => setting.field_name === fieldName);
const fieldValue = setting?.field_value;
if (fieldValue == null || fieldValue == undefined) {
if (fieldValue == null) {
if (setting?.field_type === "Select") handleResetField(fieldName);
return;
}
try {

View file

@ -1,5 +1,4 @@
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { fireEvent, renderWithProviders, screen, waitFor } from "../../../../../tests/test-utils";
import userEvent from "@testing-library/user-event";
import { beforeEach, describe, expect, it, vi } from "vitest";
import * as networking from "@/components/networking";
@ -23,20 +22,16 @@ const providers = [
{ provider_name: "tavily", ui_friendly_name: "Tavily Search" },
];
const renderModal = () => {
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: 0 } } });
return render(
<QueryClientProvider client={queryClient}>
<CreateSearchTool
userRole="Admin"
accessToken="test-token"
onCreateSuccess={vi.fn()}
isModalVisible
setModalVisible={vi.fn()}
/>
</QueryClientProvider>,
const renderModal = () =>
renderWithProviders(
<CreateSearchTool
userRole="Admin"
accessToken="test-token"
onCreateSuccess={vi.fn()}
isModalVisible
setModalVisible={vi.fn()}
/>,
);
};
const pickProvider = async (user: ReturnType<typeof userEvent.setup>, label: string) => {
await user.click(screen.getAllByRole("combobox")[0]);
@ -145,4 +140,23 @@ describe("CreateSearchTools submit payload", () => {
).toBeInTheDocument();
expect(networking.createSearchTool).not.toHaveBeenCalled();
});
it("should block creation after clearing the required provider and accept a restored choice", async () => {
const user = userEvent.setup();
renderModal();
fireEvent.change(await screen.findByLabelText(/Search Tool Name/), { target: { value: "synthetic-search" } });
await pickProvider(user, "Perplexity AI");
await user.click(screen.getByRole("button", { name: "Clear" }));
await user.click(screen.getByRole("button", { name: "Add Search Tool" }));
expect(await screen.findByText("Please select a search provider")).toBeInTheDocument();
expect(networking.createSearchTool).not.toHaveBeenCalled();
await pickProvider(user, "Tavily Search");
await user.click(screen.getByRole("button", { name: "Add Search Tool" }));
await waitFor(() =>
expect(networking.createSearchTool).toHaveBeenCalledWith("test-token", {
search_tool_name: "synthetic-search",
litellm_params: { search_provider: "tavily" },
}),
);
});
});

View file

@ -65,7 +65,10 @@ const createSearchToolShape = {
.string()
.min(1, "Please enter a search tool name")
.regex(/^[a-zA-Z0-9_-]+$/, "Name can only contain letters, numbers, hyphens, and underscores"),
search_provider: z.string().min(1, "Please select a search provider"),
search_provider: z
.string()
.nullable()
.pipe(z.string({ error: "Please select a search provider" }).min(1, "Please select a search provider")),
api_key: z.string().optional(),
description: z.string().optional(),
};
@ -74,7 +77,7 @@ const createSearchToolSchema = z.object(createSearchToolShape);
type CreateSearchToolFormValues = z.infer<typeof createSearchToolSchema>;
const EMPTY_VALUES: CreateSearchToolFormValues = { search_tool_name: "", search_provider: "" };
const EMPTY_VALUES: z.input<typeof createSearchToolSchema> = { search_tool_name: "", search_provider: null };
const labelWithHint = (label: string, hint: string): React.ReactNode => (
<>
@ -216,8 +219,8 @@ const CreateSearchTool: React.FC<CreateSearchToolProps> = ({
<Combobox
items={providerNames}
itemToStringLabel={providerLabel}
value={value === "" ? null : value}
onValueChange={(provider: string | null) => onChange(provider ?? "")}
value={value}
onValueChange={onChange}
>
<ComboboxInput
id={id}
@ -226,7 +229,7 @@ const CreateSearchTool: React.FC<CreateSearchToolProps> = ({
placeholder="Select a search provider"
className="h-10 w-full rounded-lg"
disabled={isLoadingProviders}
showClear={value !== ""}
showClear={value != null && value !== ""}
/>
<ComboboxContent>
<ComboboxEmpty>No matching search providers</ComboboxEmpty>
@ -326,7 +329,7 @@ const CreateSearchTool: React.FC<CreateSearchToolProps> = ({
<SearchConnectionTest
key={connectionTestId}
litellmParams={{
search_provider: watchedProvider,
search_provider: watchedProvider ?? undefined,
api_key: watchedApiKey,
api_base: undefined,
}}

View file

@ -55,7 +55,7 @@ const addPluginShape = {
domain: z.string(),
namespace: z.string(),
description: z.string(),
category: z.string(),
category: z.string().nullable(),
keywords: z.string(),
version: z.string(),
authorName: z.string(),
@ -76,7 +76,7 @@ const EMPTY_VALUES: AddPluginFormValues = {
domain: "",
namespace: "",
description: "",
category: "",
category: null,
keywords: "",
version: "",
authorName: "",
@ -346,18 +346,14 @@ const AddPluginForm: React.FC<AddPluginFormProps> = ({ visible, onClose, accessT
label={labelWithHint("Category (Optional)", "Select a category or enter a custom one")}
>
{({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => (
<Combobox
items={PREDEFINED_CATEGORIES}
value={value === "" ? null : value}
onValueChange={(category: string | null) => onChange(category ?? "")}
>
<Combobox items={PREDEFINED_CATEGORIES} value={value} onValueChange={onChange}>
<ComboboxInput
id={id}
aria-invalid={ariaInvalid}
aria-describedby={ariaDescribedBy}
placeholder="Select or type a category"
className="w-full rounded-lg"
showClear={value !== ""}
showClear={value != null && value !== ""}
/>
<ComboboxContent>
<ComboboxEmpty>No matching categories</ComboboxEmpty>

View file

@ -17,8 +17,8 @@ export interface ModelChoice {
interface ModelChoiceComboboxProps {
id: string;
value: string;
onChange: (value: string) => void;
value: string | null;
onChange: (value: string | null) => void;
choices: ModelChoice[];
placeholder: string;
ariaInvalid: true | undefined;
@ -40,7 +40,7 @@ const ModelChoiceCombobox: React.FC<ModelChoiceComboboxProps> = ({
<Combobox
items={choices}
value={selected}
onValueChange={(choice: ModelChoice | null) => onChange(choice?.value ?? "")}
onValueChange={(choice: ModelChoice | null) => onChange(choice?.value ?? null)}
itemToStringLabel={(choice: ModelChoice) => choice.label}
isItemEqualToValue={(choice: ModelChoice, current: ModelChoice) => choice.value === current.value}
>
@ -50,7 +50,7 @@ const ModelChoiceCombobox: React.FC<ModelChoiceComboboxProps> = ({
aria-describedby={ariaDescribedBy}
placeholder={placeholder}
className="w-full"
showClear={value !== ""}
showClear={value != null && value !== ""}
/>
<ComboboxContent>
<ComboboxEmpty>No models found</ComboboxEmpty>

View file

@ -0,0 +1,42 @@
import { z } from "zod/v4";
const sharedShape = {
auto_router_name: z.string().min(1, "Auto router name is required"),
model_access_group: z.array(z.string()),
};
const complexityRouterShape = {
...sharedShape,
auto_router_default_model: z
.string()
.nullable()
.transform((value) => value ?? ""),
auto_router_embedding_model: z
.string()
.nullable()
.transform((value) => value ?? ""),
};
const semanticRouterShape = {
...sharedShape,
auto_router_default_model: z
.string()
.nullable()
.pipe(z.string({ error: "Default model is required" }).min(1, "Default model is required")),
auto_router_embedding_model: z
.string()
.nullable()
.pipe(z.string({ error: "Embedding model is required" }).min(1, "Embedding model is required")),
};
export const complexityRouterSchema = z.object(complexityRouterShape);
export const semanticRouterSchema = z.object(semanticRouterShape);
export type EditAutoRouterFormValues = z.infer<typeof semanticRouterSchema>;
export const EMPTY_FORM_VALUES: z.input<typeof semanticRouterSchema> = {
auto_router_name: "",
auto_router_default_model: null,
auto_router_embedding_model: null,
model_access_group: [],
};

View file

@ -1,5 +1,10 @@
import React, { useEffect, useMemo, useState } from "react";
import { z } from "zod/v4";
import {
complexityRouterSchema,
semanticRouterSchema,
EMPTY_FORM_VALUES,
type EditAutoRouterFormValues,
} from "./editAutoRouterFormSchema";
import { toast } from "@/lib/toast";
import { CircleHelp } from "lucide-react";
import { FieldGroup } from "@/components/ui/field";
@ -400,35 +405,6 @@ export const buildUpdatedComplexityRouterConfig = (
};
};
const sharedShape = {
auto_router_name: z.string().min(1, "Auto router name is required"),
model_access_group: z.array(z.string()),
};
const complexityRouterShape = {
...sharedShape,
auto_router_default_model: z.string(),
auto_router_embedding_model: z.string(),
};
const semanticRouterShape = {
...sharedShape,
auto_router_default_model: z.string().min(1, "Default model is required"),
auto_router_embedding_model: z.string().min(1, "Embedding model is required"),
};
const complexityRouterSchema = z.object(complexityRouterShape);
const semanticRouterSchema = z.object(semanticRouterShape);
type EditAutoRouterFormValues = z.infer<typeof semanticRouterSchema>;
const EMPTY_FORM_VALUES: EditAutoRouterFormValues = {
auto_router_name: "",
auto_router_default_model: "",
auto_router_embedding_model: "",
model_access_group: [],
};
const labelWithHint = (label: string, hint: string): React.ReactNode => (
<>
{label}
@ -587,8 +563,8 @@ const EditAutoRouterModal: React.FC<EditAutoRouterModalProps> = ({
// Set form values
form.reset({
auto_router_name: modelData.model_name,
auto_router_default_model: modelData.litellm_params?.auto_router_default_model || "",
auto_router_embedding_model: modelData.litellm_params?.auto_router_embedding_model || "",
auto_router_default_model: modelData.litellm_params?.auto_router_default_model || null,
auto_router_embedding_model: modelData.litellm_params?.auto_router_embedding_model || null,
model_access_group: modelData.model_info?.access_groups || [],
});
} catch (error) {

View file

@ -1,5 +1,5 @@
import React from "react";
import { fireEvent, render, screen } from "@testing-library/react";
import { fireEvent, renderWithProviders, screen } from "../../../tests/test-utils";
import userEvent from "@testing-library/user-event";
import { describe, it, expect } from "vitest";
import MCPToolArgumentsForm, { MCPToolArgumentsFormRef } from "./MCPToolArgumentsForm";
@ -10,7 +10,7 @@ const toolWith = (schema: InputSchema | string): MCPTool =>
const renderForm = (schema: InputSchema | string) => {
const ref = React.createRef<MCPToolArgumentsFormRef>();
render(<MCPToolArgumentsForm ref={ref} tool={toolWith(schema)} />);
renderWithProviders(<MCPToolArgumentsForm ref={ref} tool={toolWith(schema)} />);
return ref;
};
@ -101,7 +101,7 @@ describe("MCPToolArgumentsForm", () => {
it("resets dotted defaults and positional values when the selected tool changes", async () => {
const ref = React.createRef<MCPToolArgumentsFormRef>();
const { rerender } = render(
const { rerender } = renderWithProviders(
<MCPToolArgumentsForm
ref={ref}
tool={toolWith({
@ -290,3 +290,22 @@ describe("MCPToolArgumentsForm", () => {
await expect(submit(ref)).resolves.toEqual({});
});
});
it("should distinguish an unset enum from empty string and retain explicit false", async () => {
const user = userEvent.setup();
const ref = renderForm({
type: "object",
properties: {
mode: { type: "string", enum: ["", "fast"], default: "fast" },
active: { type: "boolean", default: true },
},
});
await user.click(screen.getByRole("combobox", { name: "mode" }));
await user.click(await screen.findByRole("option", { name: "Select mode" }));
await user.click(screen.getByRole("combobox", { name: "active" }));
await user.click(await screen.findByRole("option", { name: "False" }));
await expect(submit(ref)).resolves.toEqual({ active: false });
await user.click(screen.getByRole("combobox", { name: "mode" }));
await user.click(await screen.findByRole("option", { name: "Empty string" }));
await expect(submit(ref)).resolves.toEqual({ mode: "", active: false });
});

View file

@ -23,6 +23,9 @@ const BOOLEAN_ITEMS = [
const isBlank = (value: unknown): boolean => value === undefined || value === null || value === "";
const isUnsetArgument = (prop: InputSchemaProperty | undefined, value: unknown): boolean =>
prop?.type === "string" && prop.enum ? value == null : isBlank(value);
const jsonErrorFor = (prop: InputSchemaProperty, value: unknown): string | null => {
try {
const parsed = typeof value === "string" ? JSON.parse(value) : value;
@ -45,10 +48,15 @@ const collectErrors = (
): Record<string, FieldError> => {
const entries = Object.entries(actualSchema.properties ?? {}).flatMap<[string, FieldError]>(([key, prop]) => {
const value = values[key];
const blank = isBlank(value);
const blank = isUnsetArgument(prop, value);
if (actualSchema.required?.includes(key) && blank) {
return [[key, { type: "required", message: requiredMessages[key] ?? `Please enter ${key}` }]];
}
if (prop.type === "string" && prop.enum) {
if (!blank && !prop.enum.includes(String(value))) {
return [[key, { type: "validate", message: `Please select a valid ${key}` }]];
}
}
if (prop.type !== "object" && prop.type !== "array") return [];
if (blank) return [];
const message = jsonErrorFor(prop, value);
@ -146,6 +154,7 @@ function buildDefaultValue(prop?: InputSchemaProperty, overrideDefault?: any): a
}
const getInitialValueForField = (prop: InputSchemaProperty): any => {
if (prop.type === "string" && prop.enum && prop.default === undefined) return null;
const defaultValue = buildDefaultValue(prop);
if (prop.type === "object" || prop.type === "array") {
const fallback = prop.type === "array" ? [] : {};
@ -164,7 +173,7 @@ function convertFormValues(
Object.entries(values).forEach(([key, value]) => {
const prop = schemaToUse.properties?.[key];
if (prop && value !== null && value !== undefined && value !== "") {
if (prop && !isUnsetArgument(prop, value)) {
switch (prop.type) {
case "boolean":
convertedValues[key] = value === "true" || value === true;
@ -202,7 +211,7 @@ function convertFormValues(
default:
convertedValues[key] = value;
}
} else if (value !== null && value !== undefined && value !== "") {
} else if (!isUnsetArgument(prop, value)) {
convertedValues[key] = value;
}
});
@ -342,7 +351,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
{(field) => {
if (prop.type === "string" && prop.enum) {
return (
<Select value={field.value ?? ""} onValueChange={field.onChange}>
<Select value={field.value ?? null} onValueChange={field.onChange}>
<SelectTrigger
id={field.id}
onBlur={field.onBlur}
@ -352,10 +361,10 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
<SelectValue placeholder={`Select ${key}`} />
</SelectTrigger>
<SelectContent>
{!required && <SelectItem value="">Select {key}</SelectItem>}
{!required && <SelectItem value={null}>Select {key}</SelectItem>}
{prop.enum.map((v) => (
<SelectItem key={v} value={v}>
{v}
{v === "" ? "Empty string" : v}
</SelectItem>
))}
</SelectContent>
@ -364,7 +373,11 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
}
if (prop.type === "boolean") {
return (
<Select items={BOOLEAN_ITEMS} value={field.value ?? ""} onValueChange={field.onChange}>
<Select
items={required ? BOOLEAN_ITEMS : [{ value: null, label: `Select ${key}` }, ...BOOLEAN_ITEMS]}
value={field.value ?? null}
onValueChange={field.onChange}
>
<SelectTrigger
id={field.id}
onBlur={field.onBlur}
@ -374,7 +387,7 @@ const MCPToolArgumentsForm = forwardRef<MCPToolArgumentsFormRef, MCPToolArgument
<SelectValue placeholder={`Select ${key}`} />
</SelectTrigger>
<SelectContent>
{!required && <SelectItem value="">Select {key}</SelectItem>}
{!required && <SelectItem value={null}>Select {key}</SelectItem>}
<SelectItem value={true}>True</SelectItem>
<SelectItem value={false}>False</SelectItem>
</SelectContent>