Merge branch 'main' into fix/set-scope-dcr
This commit is contained in:
commit
99120937e2
7 changed files with 986 additions and 588 deletions
|
|
@ -179,7 +179,18 @@ const ToolsTab = ({
|
|||
onCheckedChange={(checked: boolean) =>
|
||||
setParams({
|
||||
...params,
|
||||
[key]: checked ? null : prop.default,
|
||||
[key]: checked
|
||||
? null
|
||||
: prop.default !== null
|
||||
? prop.default
|
||||
: prop.type === "boolean"
|
||||
? false
|
||||
: prop.type === "string"
|
||||
? ""
|
||||
: prop.type === "number" ||
|
||||
prop.type === "integer"
|
||||
? undefined
|
||||
: undefined,
|
||||
})
|
||||
}
|
||||
/>
|
||||
|
|
|
|||
|
|
@ -913,7 +913,8 @@ describe("Sidebar", () => {
|
|||
expect.objectContaining({
|
||||
MCP_SERVER_REQUEST_TIMEOUT: {
|
||||
label: "Request Timeout",
|
||||
description: "Timeout for requests to the MCP server (ms)",
|
||||
description:
|
||||
"Client-side timeout (ms) - Inspector will cancel requests after this time",
|
||||
value: 5000,
|
||||
is_session_item: false,
|
||||
},
|
||||
|
|
@ -988,7 +989,8 @@ describe("Sidebar", () => {
|
|||
expect.objectContaining({
|
||||
MCP_SERVER_REQUEST_TIMEOUT: {
|
||||
label: "Request Timeout",
|
||||
description: "Timeout for requests to the MCP server (ms)",
|
||||
description:
|
||||
"Client-side timeout (ms) - Inspector will cancel requests after this time",
|
||||
value: 0,
|
||||
is_session_item: false,
|
||||
},
|
||||
|
|
@ -1035,7 +1037,8 @@ describe("Sidebar", () => {
|
|||
expect.objectContaining({
|
||||
MCP_SERVER_REQUEST_TIMEOUT: {
|
||||
label: "Request Timeout",
|
||||
description: "Timeout for requests to the MCP server (ms)",
|
||||
description:
|
||||
"Client-side timeout (ms) - Inspector will cancel requests after this time",
|
||||
value: 3000,
|
||||
is_session_item: false,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -177,6 +177,103 @@ describe("ToolsTab", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("should support tri-state nullable boolean (null -> false -> true -> null)", async () => {
|
||||
const mockCallTool = jest.fn();
|
||||
const toolWithNullableBoolean: Tool = {
|
||||
name: "testTool",
|
||||
description: "Tool with nullable boolean",
|
||||
inputSchema: {
|
||||
type: "object" as const,
|
||||
properties: {
|
||||
optionalBoolean: {
|
||||
type: ["boolean", "null"] as const,
|
||||
default: null,
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
renderToolsTab({
|
||||
tools: [toolWithNullableBoolean],
|
||||
selectedTool: toolWithNullableBoolean,
|
||||
callTool: mockCallTool,
|
||||
});
|
||||
|
||||
const nullCheckbox = screen.getByRole("checkbox", { name: /null/i });
|
||||
const runButton = screen.getByRole("button", { name: /run tool/i });
|
||||
|
||||
// State 1: Initial state should be null (input disabled)
|
||||
const wrapper = screen.getByRole("toolinputwrapper");
|
||||
expect(wrapper.classList).toContain("pointer-events-none");
|
||||
expect(wrapper.classList).toContain("opacity-50");
|
||||
|
||||
// Verify tool is called with null initially
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
expect(mockCallTool).toHaveBeenCalledWith(toolWithNullableBoolean.name, {
|
||||
optionalBoolean: null,
|
||||
});
|
||||
|
||||
// State 2: Uncheck null checkbox -> should set value to false and enable input
|
||||
await act(async () => {
|
||||
fireEvent.click(nullCheckbox);
|
||||
});
|
||||
expect(wrapper.classList).not.toContain("pointer-events-none");
|
||||
|
||||
// Clear previous calls to make assertions clearer
|
||||
mockCallTool.mockClear();
|
||||
|
||||
// Verify tool can be called with false
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
expect(mockCallTool).toHaveBeenLastCalledWith(
|
||||
toolWithNullableBoolean.name,
|
||||
{
|
||||
optionalBoolean: false,
|
||||
},
|
||||
);
|
||||
|
||||
// State 3: Check boolean checkbox -> should set value to true
|
||||
// Find the boolean checkbox within the input wrapper (to avoid ID conflict with null checkbox)
|
||||
const booleanCheckbox = within(wrapper).getByRole("checkbox");
|
||||
|
||||
mockCallTool.mockClear();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(booleanCheckbox);
|
||||
});
|
||||
|
||||
// Verify tool can be called with true
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
expect(mockCallTool).toHaveBeenLastCalledWith(
|
||||
toolWithNullableBoolean.name,
|
||||
{
|
||||
optionalBoolean: true,
|
||||
},
|
||||
);
|
||||
|
||||
// State 4: Check null checkbox again -> should set value back to null and disable input
|
||||
await act(async () => {
|
||||
fireEvent.click(nullCheckbox);
|
||||
});
|
||||
expect(wrapper.classList).toContain("pointer-events-none");
|
||||
|
||||
// Verify tool can be called with null again
|
||||
await act(async () => {
|
||||
fireEvent.click(runButton);
|
||||
});
|
||||
expect(mockCallTool).toHaveBeenLastCalledWith(
|
||||
toolWithNullableBoolean.name,
|
||||
{
|
||||
optionalBoolean: 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;
|
||||
|
|
|
|||
|
|
@ -14,7 +14,8 @@ export type ConfigItem = {
|
|||
*/
|
||||
export type InspectorConfig = {
|
||||
/**
|
||||
* Maximum time in milliseconds to wait for a response from the MCP server before timing out.
|
||||
* Client-side timeout in milliseconds. The Inspector will cancel the request if no response
|
||||
* is received within this time. Note: This is independent of any server-side timeouts.
|
||||
*/
|
||||
MCP_SERVER_REQUEST_TIMEOUT: ConfigItem;
|
||||
|
||||
|
|
|
|||
|
|
@ -44,8 +44,9 @@ export const DEFAULT_MCP_PROXY_LISTEN_PORT = "6277";
|
|||
export const DEFAULT_INSPECTOR_CONFIG: InspectorConfig = {
|
||||
MCP_SERVER_REQUEST_TIMEOUT: {
|
||||
label: "Request Timeout",
|
||||
description: "Timeout for requests to the MCP server (ms)",
|
||||
value: 10000,
|
||||
description:
|
||||
"Client-side timeout (ms) - Inspector will cancel requests after this time",
|
||||
value: 300000, // 5 minutes - increased to support elicitation and other long-running tools
|
||||
is_session_item: false,
|
||||
},
|
||||
MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS: {
|
||||
|
|
|
|||
|
|
@ -205,4 +205,112 @@ describe("cleanParams", () => {
|
|||
// optionalField omitted entirely
|
||||
});
|
||||
});
|
||||
|
||||
it("should preserve null values when field has default: null", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
optionalFieldWithNullDefault: { type: "string", default: null },
|
||||
optionalFieldWithoutDefault: { type: "string" },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
optionalFieldWithNullDefault: null,
|
||||
optionalFieldWithoutDefault: null,
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
optionalFieldWithNullDefault: null, // preserved because default: null
|
||||
// optionalFieldWithoutDefault omitted
|
||||
});
|
||||
});
|
||||
|
||||
it("should preserve default values that match current value", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
fieldWithDefaultString: { type: "string", default: "defaultValue" },
|
||||
fieldWithDefaultNumber: { type: "number", default: 42 },
|
||||
fieldWithDefaultNull: { type: "string", default: null },
|
||||
fieldWithDefaultBoolean: { type: "boolean", default: false },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
fieldWithDefaultString: "defaultValue",
|
||||
fieldWithDefaultNumber: 42,
|
||||
fieldWithDefaultNull: null,
|
||||
fieldWithDefaultBoolean: false,
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
fieldWithDefaultString: "defaultValue",
|
||||
fieldWithDefaultNumber: 42,
|
||||
fieldWithDefaultNull: null,
|
||||
fieldWithDefaultBoolean: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("should omit values that do not match their default", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
fieldWithDefault: { type: "string", default: "defaultValue" },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
fieldWithDefault: null, // doesn't match default
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
// fieldWithDefault omitted because value (null) doesn't match default ("defaultValue")
|
||||
});
|
||||
});
|
||||
|
||||
it("should fix regression from issue #846 - tools with multiple null defaults", () => {
|
||||
// Reproduces the exact scenario from https://github.com/modelcontextprotocol/inspector/issues/846
|
||||
// In v0.17.0, the cleanParams function would remove all null values,
|
||||
// breaking tools that have parameters with explicit default: null
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: ["requiredString"],
|
||||
properties: {
|
||||
optionalString: { type: ["string", "null"], default: null },
|
||||
optionalNumber: { type: ["number", "null"], default: null },
|
||||
optionalBoolean: { type: ["boolean", "null"], default: null },
|
||||
requiredString: { type: "string" },
|
||||
},
|
||||
};
|
||||
|
||||
// When a user opens the tool in Inspector, fields initialize with their defaults
|
||||
const params = {
|
||||
optionalString: null, // initialized to default
|
||||
optionalNumber: null, // initialized to default
|
||||
optionalBoolean: null, // initialized to default
|
||||
requiredString: "test",
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
// In v0.16, null defaults were preserved (working behavior)
|
||||
// In v0.17.0, they were removed (regression)
|
||||
// This fix restores the v0.16 behavior
|
||||
expect(cleaned).toEqual({
|
||||
optionalString: null,
|
||||
optionalNumber: null,
|
||||
optionalBoolean: null,
|
||||
requiredString: "test",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ 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.
|
||||
* while preserving all values for required fields and fields with explicit default values.
|
||||
*
|
||||
* @param params - The parameters object to clean
|
||||
* @param schema - The JSON schema defining which fields are required
|
||||
|
|
@ -14,13 +14,23 @@ export function cleanParams(
|
|||
): Record<string, unknown> {
|
||||
const cleaned: Record<string, unknown> = {};
|
||||
const required = schema.required || [];
|
||||
const properties = schema.properties || {};
|
||||
|
||||
for (const [key, value] of Object.entries(params)) {
|
||||
const isFieldRequired = required.includes(key);
|
||||
const fieldSchema = properties[key] as JsonSchemaType | undefined;
|
||||
|
||||
// Check if the field has an explicit default value
|
||||
const hasDefault = fieldSchema && "default" in fieldSchema;
|
||||
const defaultValue = hasDefault ? fieldSchema.default : undefined;
|
||||
|
||||
if (isFieldRequired) {
|
||||
// Required fields: always include, even if empty string or falsy
|
||||
cleaned[key] = value;
|
||||
} else if (hasDefault && value === defaultValue) {
|
||||
// Field has a default value and current value matches it - preserve it
|
||||
// This is important for cases like default: null
|
||||
cleaned[key] = value;
|
||||
} else {
|
||||
// Optional fields: only include if they have meaningful values
|
||||
if (value !== undefined && value !== "" && value !== null) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue