Add proper support for progress flow during tool calling

This commit is contained in:
Pulkit Sharma
2025-04-05 01:46:57 +05:30
parent 06f237b1de
commit e35343537c
11 changed files with 204 additions and 61 deletions

View File

@@ -411,21 +411,34 @@ const App = () => {
}; };
const callTool = async (name: string, params: Record<string, unknown>) => { const callTool = async (name: string, params: Record<string, unknown>) => {
const response = await makeConnectionRequest( try {
{ const response = await makeConnectionRequest(
method: "tools/call" as const, {
params: { method: "tools/call" as const,
name, params: {
arguments: params, name,
_meta: { arguments: params,
progressToken: progressTokenRef.current++, _meta: {
progressToken: progressTokenRef.current++,
},
}, },
}, },
}, CompatibilityCallToolResultSchema,
CompatibilityCallToolResultSchema, "tools",
"tools", );
); setToolResult(response);
setToolResult(response); } catch (e) {
const toolResult: CompatibilityCallToolResult = {
content: [
{
type: "text",
text: (e as Error).message ?? String(e),
},
],
isError: true,
};
setToolResult(toolResult);
}
}; };
const handleRootsChange = async () => { const handleRootsChange = async () => {
@@ -633,9 +646,10 @@ const App = () => {
setTools([]); setTools([]);
setNextToolCursor(undefined); setNextToolCursor(undefined);
}} }}
callTool={(name, params) => { callTool={async (name, params) => {
clearError("tools"); clearError("tools");
callTool(name, params); setToolResult(null);
await callTool(name, params);
}} }}
selectedTool={selectedTool} selectedTool={selectedTool}
setSelectedTool={(tool) => { setSelectedTool={(tool) => {

View File

@@ -325,7 +325,7 @@ const Sidebar = ({
return ( return (
<div key={key} className="space-y-2"> <div key={key} className="space-y-2">
<div className="flex items-center gap-1"> <div className="flex items-center gap-1">
<label className="text-sm font-medium text-green-600"> <label className="text-sm font-medium text-green-600 break-all">
{configKey} {configKey}
</label> </label>
<Tooltip> <Tooltip>

View File

@@ -13,7 +13,7 @@ import {
ListToolsResult, ListToolsResult,
Tool, Tool,
} from "@modelcontextprotocol/sdk/types.js"; } from "@modelcontextprotocol/sdk/types.js";
import { Send } from "lucide-react"; import { Loader2, Send } from "lucide-react";
import { useEffect, useState } from "react"; import { useEffect, useState } from "react";
import ListPane from "./ListPane"; import ListPane from "./ListPane";
import JsonView from "./JsonView"; import JsonView from "./JsonView";
@@ -31,7 +31,7 @@ const ToolsTab = ({
tools: Tool[]; tools: Tool[];
listTools: () => void; listTools: () => void;
clearTools: () => void; clearTools: () => void;
callTool: (name: string, params: Record<string, unknown>) => void; callTool: (name: string, params: Record<string, unknown>) => Promise<void>;
selectedTool: Tool | null; selectedTool: Tool | null;
setSelectedTool: (tool: Tool | null) => void; setSelectedTool: (tool: Tool | null) => void;
toolResult: CompatibilityCallToolResult | null; toolResult: CompatibilityCallToolResult | null;
@@ -39,6 +39,8 @@ const ToolsTab = ({
error: string | null; error: string | null;
}) => { }) => {
const [params, setParams] = useState<Record<string, unknown>>({}); const [params, setParams] = useState<Record<string, unknown>>({});
const [isToolRunning, setIsToolRunning] = useState(false);
useEffect(() => { useEffect(() => {
setParams({}); setParams({});
}, [selectedTool]); }, [selectedTool]);
@@ -234,9 +236,28 @@ const ToolsTab = ({
); );
}, },
)} )}
<Button onClick={() => callTool(selectedTool.name, params)}> <Button
<Send className="w-4 h-4 mr-2" /> onClick={async () => {
Run Tool try {
setIsToolRunning(true);
await callTool(selectedTool.name, params);
} finally {
setIsToolRunning(false);
}
}}
disabled={isToolRunning}
>
{isToolRunning ? (
<>
<Loader2 className="w-4 h-4 mr-2 animate-spin" />
Running...
</>
) : (
<>
<Send className="w-4 h-4 mr-2" />
Run Tool
</>
)}
</Button> </Button>
{toolResult && renderToolResult()} {toolResult && renderToolResult()}
</div> </div>

View File

@@ -350,6 +350,54 @@ describe("Sidebar Environment Variables", () => {
); );
}); });
it("should update MCP server proxy address", () => {
const setConfig = jest.fn();
renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig });
openConfigSection();
const proxyAddressInput = screen.getByTestId(
"MCP_PROXY_FULL_ADDRESS-input",
);
fireEvent.change(proxyAddressInput, {
target: { value: "http://localhost:8080" },
});
expect(setConfig).toHaveBeenCalledWith(
expect.objectContaining({
MCP_PROXY_FULL_ADDRESS: {
description:
"Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577",
value: "http://localhost:8080",
},
}),
);
});
it("should update max total timeout", () => {
const setConfig = jest.fn();
renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig });
openConfigSection();
const maxTotalTimeoutInput = screen.getByTestId(
"MCP_REQUEST_MAX_TOTAL_TIMEOUT-input",
);
fireEvent.change(maxTotalTimeoutInput, {
target: { value: "10000" },
});
expect(setConfig).toHaveBeenCalledWith(
expect.objectContaining({
MCP_REQUEST_MAX_TOTAL_TIMEOUT: {
description:
"Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications)",
value: 10000,
},
}),
);
});
it("should handle invalid timeout values entered by user", () => { it("should handle invalid timeout values entered by user", () => {
const setConfig = jest.fn(); const setConfig = jest.fn();
renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig }); renderSidebar({ config: DEFAULT_INSPECTOR_CONFIG, setConfig });

View File

@@ -1,4 +1,4 @@
import { render, screen, fireEvent } from "@testing-library/react"; import { render, screen, fireEvent, act } from "@testing-library/react";
import { describe, it, expect, jest } from "@jest/globals"; import { describe, it, expect, jest } from "@jest/globals";
import "@testing-library/jest-dom"; import "@testing-library/jest-dom";
import ToolsTab from "../ToolsTab"; import ToolsTab from "../ToolsTab";
@@ -43,7 +43,7 @@ describe("ToolsTab", () => {
tools: mockTools, tools: mockTools,
listTools: jest.fn(), listTools: jest.fn(),
clearTools: jest.fn(), clearTools: jest.fn(),
callTool: jest.fn(), callTool: jest.fn(async () => {}),
selectedTool: null, selectedTool: null,
setSelectedTool: jest.fn(), setSelectedTool: jest.fn(),
toolResult: null, toolResult: null,
@@ -59,14 +59,16 @@ describe("ToolsTab", () => {
); );
}; };
it("should reset input values when switching tools", () => { it("should reset input values when switching tools", async () => {
const { rerender } = renderToolsTab({ const { rerender } = renderToolsTab({
selectedTool: mockTools[0], selectedTool: mockTools[0],
}); });
// Enter a value in the first tool's input // Enter a value in the first tool's input
const input = screen.getByRole("spinbutton") as HTMLInputElement; const input = screen.getByRole("spinbutton") as HTMLInputElement;
fireEvent.change(input, { target: { value: "42" } }); await act(async () => {
fireEvent.change(input, { target: { value: "42" } });
});
expect(input.value).toBe("42"); expect(input.value).toBe("42");
// Switch to second tool // Switch to second tool
@@ -80,7 +82,8 @@ describe("ToolsTab", () => {
const newInput = screen.getByRole("spinbutton") as HTMLInputElement; const newInput = screen.getByRole("spinbutton") as HTMLInputElement;
expect(newInput.value).toBe(""); expect(newInput.value).toBe("");
}); });
it("should handle integer type inputs", () => {
it("should handle integer type inputs", async () => {
renderToolsTab({ renderToolsTab({
selectedTool: mockTools[1], // Use the tool with integer type selectedTool: mockTools[1], // Use the tool with integer type
}); });
@@ -93,10 +96,49 @@ describe("ToolsTab", () => {
expect(input.value).toBe("42"); expect(input.value).toBe("42");
const submitButton = screen.getByRole("button", { name: /run tool/i }); const submitButton = screen.getByRole("button", { name: /run tool/i });
fireEvent.click(submitButton); await act(async () => {
fireEvent.click(submitButton);
});
expect(defaultProps.callTool).toHaveBeenCalledWith(mockTools[1].name, { expect(defaultProps.callTool).toHaveBeenCalledWith(mockTools[1].name, {
count: 42, count: 42,
}); });
}); });
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;
const mockPromise = new Promise((resolve) => {
resolvePromise = resolve;
});
// Mock callTool to return our promise
const mockCallTool = jest.fn().mockReturnValue(mockPromise);
renderToolsTab({
selectedTool: mockTools[0],
callTool: mockCallTool,
});
const submitButton = screen.getByRole("button", { name: /run tool/i });
expect(submitButton.getAttribute("disabled")).toBeNull();
// Click the button and verify immediate state changes
await act(async () => {
fireEvent.click(submitButton);
});
// Verify button is disabled and text changed
expect(submitButton.getAttribute("disabled")).not.toBeNull();
expect(submitButton.textContent).toBe("Running...");
// Resolve the promise to simulate tool completion
await act(async () => {
if (resolvePromise) {
await resolvePromise({});
}
});
expect(submitButton.getAttribute("disabled")).toBeNull();
});
}); });

View File

@@ -20,13 +20,13 @@ export type InspectorConfig = {
* Whether to reset the timeout on progress notifications. Useful for long-running operations that send periodic progress updates. * Whether to reset the timeout on progress notifications. Useful for long-running operations that send periodic progress updates.
* Refer: https://spec.modelcontextprotocol.io/specification/2025-03-26/basic/utilities/progress/#progress-flow * Refer: https://spec.modelcontextprotocol.io/specification/2025-03-26/basic/utilities/progress/#progress-flow
*/ */
MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS: ConfigItem; MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS: ConfigItem;
/** /**
* Maximum total time in milliseconds to wait for a response from the MCP server before timing out. Used in conjunction with MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS. * Maximum total time in milliseconds to wait for a response from the MCP server before timing out. Used in conjunction with MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS.
* Refer: https://spec.modelcontextprotocol.io/specification/2025-03-26/basic/utilities/progress/#progress-flow * Refer: https://spec.modelcontextprotocol.io/specification/2025-03-26/basic/utilities/progress/#progress-flow
*/ */
MCP_SERVER_REQUEST_TIMEOUT_MAX_TOTAL_TIMEOUT: ConfigItem; MCP_REQUEST_MAX_TOTAL_TIMEOUT: ConfigItem;
/** /**
* The full address of the MCP Proxy Server, in case it is running on a non-default address. Example: http://10.1.1.22:5577 * The full address of the MCP Proxy Server, in case it is running on a non-default address. Example: http://10.1.1.22:5577

View File

@@ -25,12 +25,13 @@ export const DEFAULT_INSPECTOR_CONFIG: InspectorConfig = {
description: "Timeout for requests to the MCP server (ms)", description: "Timeout for requests to the MCP server (ms)",
value: 10000, value: 10000,
}, },
MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS: { MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS: {
description: "Reset timeout on progress notifications", description: "Reset timeout on progress notifications",
value: true, value: true,
}, },
MCP_SERVER_REQUEST_TIMEOUT_MAX_TOTAL_TIMEOUT: { MCP_REQUEST_MAX_TOTAL_TIMEOUT: {
description: "Maximum total timeout for requests sent to the MCP server (ms)", description:
"Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications)",
value: 60000, value: 60000,
}, },
MCP_PROXY_FULL_ADDRESS: { MCP_PROXY_FULL_ADDRESS: {

View File

@@ -95,11 +95,10 @@ describe("useConnection", () => {
expect.objectContaining({ expect.objectContaining({
timeout: DEFAULT_INSPECTOR_CONFIG.MCP_SERVER_REQUEST_TIMEOUT.value, timeout: DEFAULT_INSPECTOR_CONFIG.MCP_SERVER_REQUEST_TIMEOUT.value,
maxTotalTimeout: maxTotalTimeout:
DEFAULT_INSPECTOR_CONFIG DEFAULT_INSPECTOR_CONFIG.MCP_REQUEST_MAX_TOTAL_TIMEOUT.value,
.MCP_SERVER_REQUEST_TIMEOUT_MAX_TOTAL_TIMEOUT.value,
resetTimeoutOnProgress: resetTimeoutOnProgress:
DEFAULT_INSPECTOR_CONFIG DEFAULT_INSPECTOR_CONFIG.MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS
.MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS.value, .value,
}), }),
); );
}); });

View File

@@ -8,7 +8,6 @@ import {
ClientRequest, ClientRequest,
CreateMessageRequestSchema, CreateMessageRequestSchema,
ListRootsRequestSchema, ListRootsRequestSchema,
ProgressNotificationSchema,
ResourceUpdatedNotificationSchema, ResourceUpdatedNotificationSchema,
LoggingMessageNotificationSchema, LoggingMessageNotificationSchema,
Request, Request,
@@ -23,6 +22,7 @@ import {
ResourceListChangedNotificationSchema, ResourceListChangedNotificationSchema,
ToolListChangedNotificationSchema, ToolListChangedNotificationSchema,
PromptListChangedNotificationSchema, PromptListChangedNotificationSchema,
Progress,
} from "@modelcontextprotocol/sdk/types.js"; } from "@modelcontextprotocol/sdk/types.js";
import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { RequestOptions } from "@modelcontextprotocol/sdk/shared/protocol.js";
import { useState } from "react"; import { useState } from "react";
@@ -99,21 +99,38 @@ export function useConnection({
if (!mcpClient) { if (!mcpClient) {
throw new Error("MCP client not connected"); throw new Error("MCP client not connected");
} }
try { try {
const abortController = new AbortController(); const abortController = new AbortController();
// prepare MCP Client request options
const mcpRequestOptions: RequestOptions = {
signal: options?.signal ?? abortController.signal,
resetTimeoutOnProgress:
options?.resetTimeoutOnProgress ??
resetRequestTimeoutOnProgress(config),
timeout: options?.timeout ?? getMCPServerRequestTimeout(config),
maxTotalTimeout:
options?.maxTotalTimeout ??
getMCPServerRequestMaxTotalTimeout(config),
};
// If progress notifications are enabled, add an onprogress hook to the MCP Client request options
// This is required by SDK to reset the timeout on progress notifications
if (mcpRequestOptions.resetTimeoutOnProgress) {
mcpRequestOptions.onprogress = (params: Progress) => {
// Add progress notification to `Server Notification` window in the UI
if (onNotification) {
onNotification({
method: "notification/progress",
params,
});
}
};
}
let response; let response;
try { try {
response = await mcpClient.request(request, schema, { response = await mcpClient.request(request, schema, mcpRequestOptions);
signal: options?.signal ?? abortController.signal,
resetTimeoutOnProgress:
options?.resetTimeoutOnProgress ??
resetRequestTimeoutOnProgress(config),
timeout: options?.timeout ?? getMCPServerRequestTimeout(config),
maxTotalTimeout:
options?.maxTotalTimeout ??
getMCPServerRequestMaxTotalTimeout(config),
});
pushHistory(request, response); pushHistory(request, response);
} catch (error) { } catch (error) {
@@ -291,7 +308,6 @@ export function useConnection({
if (onNotification) { if (onNotification) {
[ [
CancelledNotificationSchema, CancelledNotificationSchema,
ProgressNotificationSchema,
LoggingMessageNotificationSchema, LoggingMessageNotificationSchema,
ResourceUpdatedNotificationSchema, ResourceUpdatedNotificationSchema,
ResourceListChangedNotificationSchema, ResourceListChangedNotificationSchema,

View File

@@ -16,11 +16,11 @@ export const getMCPServerRequestTimeout = (config: InspectorConfig): number => {
export const resetRequestTimeoutOnProgress = ( export const resetRequestTimeoutOnProgress = (
config: InspectorConfig, config: InspectorConfig,
): boolean => { ): boolean => {
return config.MCP_SERVER_REQUEST_TIMEOUT_RESET_ON_PROGRESS.value as boolean; return config.MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS.value as boolean;
}; };
export const getMCPServerRequestMaxTotalTimeout = ( export const getMCPServerRequestMaxTotalTimeout = (
config: InspectorConfig, config: InspectorConfig,
): number => { ): number => {
return config.MCP_SERVER_REQUEST_TIMEOUT_MAX_TOTAL_TIMEOUT.value as number; return config.MCP_REQUEST_MAX_TOTAL_TIMEOUT.value as number;
}; };

22
package-lock.json generated
View File

@@ -1,20 +1,20 @@
{ {
"name": "@modelcontextprotocol/inspector", "name": "@modelcontextprotocol/inspector",
"version": "0.8.0", "version": "0.8.1",
"lockfileVersion": 3, "lockfileVersion": 3,
"requires": true, "requires": true,
"packages": { "packages": {
"": { "": {
"name": "@modelcontextprotocol/inspector", "name": "@modelcontextprotocol/inspector",
"version": "0.8.0", "version": "0.8.1",
"license": "MIT", "license": "MIT",
"workspaces": [ "workspaces": [
"client", "client",
"server" "server"
], ],
"dependencies": { "dependencies": {
"@modelcontextprotocol/inspector-client": "^0.8.0", "@modelcontextprotocol/inspector-client": "^0.8.1",
"@modelcontextprotocol/inspector-server": "^0.8.0", "@modelcontextprotocol/inspector-server": "^0.8.1",
"concurrently": "^9.0.1", "concurrently": "^9.0.1",
"shell-quote": "^1.8.2", "shell-quote": "^1.8.2",
"spawn-rx": "^5.1.2", "spawn-rx": "^5.1.2",
@@ -32,10 +32,10 @@
}, },
"client": { "client": {
"name": "@modelcontextprotocol/inspector-client", "name": "@modelcontextprotocol/inspector-client",
"version": "0.8.0", "version": "0.8.1",
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"@modelcontextprotocol/sdk": "^1.6.1", "@modelcontextprotocol/sdk": "^1.8.0",
"@radix-ui/react-checkbox": "^1.1.4", "@radix-ui/react-checkbox": "^1.1.4",
"@radix-ui/react-dialog": "^1.1.3", "@radix-ui/react-dialog": "^1.1.3",
"@radix-ui/react-icons": "^1.3.0", "@radix-ui/react-icons": "^1.3.0",
@@ -1329,11 +1329,13 @@
"link": true "link": true
}, },
"node_modules/@modelcontextprotocol/sdk": { "node_modules/@modelcontextprotocol/sdk": {
"version": "1.6.1", "version": "1.8.0",
"license": "MIT", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.8.0.tgz",
"integrity": "sha512-e06W7SwrontJDHwCawNO5SGxG+nU9AAx+jpHHZqGl/WrDBdWOpvirC+s58VpJTB5QemI4jTRcjWT4Pt3Q1NPQQ==",
"dependencies": { "dependencies": {
"content-type": "^1.0.5", "content-type": "^1.0.5",
"cors": "^2.8.5", "cors": "^2.8.5",
"cross-spawn": "^7.0.3",
"eventsource": "^3.0.2", "eventsource": "^3.0.2",
"express": "^5.0.1", "express": "^5.0.1",
"express-rate-limit": "^7.5.0", "express-rate-limit": "^7.5.0",
@@ -9483,10 +9485,10 @@
}, },
"server": { "server": {
"name": "@modelcontextprotocol/inspector-server", "name": "@modelcontextprotocol/inspector-server",
"version": "0.8.0", "version": "0.8.1",
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"@modelcontextprotocol/sdk": "^1.6.1", "@modelcontextprotocol/sdk": "^1.8.0",
"cors": "^2.8.5", "cors": "^2.8.5",
"express": "^4.21.0", "express": "^4.21.0",
"ws": "^8.18.0", "ws": "^8.18.0",