From 1c406eafbd0a377c0094257810829bff1a343429 Mon Sep 17 00:00:00 2001 From: "lorenzo.neumann" <36760115+ln-12@users.noreply.github.com> Date: Fri, 29 Aug 2025 15:52:31 +0200 Subject: [PATCH 01/83] Added UI to provide additional _meta values --- client/src/App.tsx | 15 ++- client/src/components/ToolResults.tsx | 2 +- client/src/components/ToolsTab.tsx | 120 +++++++++++++++++- .../components/__tests__/ToolsTab.test.tsx | 70 ++++++++-- 4 files changed, 188 insertions(+), 19 deletions(-) diff --git a/client/src/App.tsx b/client/src/App.tsx index fecd9839..3bf3b887 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -698,7 +698,11 @@ const App = () => { cacheToolOutputSchemas(response.tools); }; - const callTool = async (name: string, params: Record) => { + const callTool = async ( + name: string, + params: Record, + meta?: Record, + ) => { lastToolCallOriginTabRef.current = currentTabRef.current; try { @@ -710,6 +714,7 @@ const App = () => { arguments: params, _meta: { progressToken: progressTokenRef.current++, + ...(meta ?? {}), }, }, }, @@ -1008,10 +1013,14 @@ const App = () => { setNextToolCursor(undefined); cacheToolOutputSchemas([]); }} - callTool={async (name, params) => { + callTool={async ( + name: string, + params: Record, + meta?: Record, + ) => { clearError("tools"); setToolResult(null); - await callTool(name, params); + await callTool(name, params, meta); }} selectedTool={selectedTool} setSelectedTool={(tool) => { diff --git a/client/src/components/ToolResults.tsx b/client/src/components/ToolResults.tsx index 6479b5fb..64798cd9 100644 --- a/client/src/components/ToolResults.tsx +++ b/client/src/components/ToolResults.tsx @@ -156,7 +156,7 @@ const ToolResults = ({ )} {structuredResult._meta && (
-
Meta:
+
Meta Schema:
diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx index 2654feed..798467c1 100644 --- a/client/src/components/ToolsTab.tsx +++ b/client/src/components/ToolsTab.tsx @@ -42,7 +42,11 @@ const ToolsTab = ({ tools: Tool[]; listTools: () => void; clearTools: () => void; - callTool: (name: string, params: Record) => Promise; + callTool: ( + name: string, + params: Record, + meta?: Record, + ) => Promise; selectedTool: Tool | null; setSelectedTool: (tool: Tool | null) => void; toolResult: CompatibilityCallToolResult | null; @@ -55,6 +59,9 @@ const ToolsTab = ({ const [isToolRunning, setIsToolRunning] = useState(false); const [isOutputSchemaExpanded, setIsOutputSchemaExpanded] = useState(false); const [isMetaExpanded, setIsMetaExpanded] = useState(false); + const [metaEntries, setMetaEntries] = useState< + { id: string; key: string; value: string }[] + >([]); useEffect(() => { const params = Object.entries( @@ -221,6 +228,102 @@ const ToolsTab = ({ ); }, )} +
+
+

Meta:

+ +
+ {metaEntries.length === 0 ? ( +

+ No meta pairs. +

+ ) : ( +
+ {metaEntries.map((entry, index) => ( +
+ + { + const value = e.target.value; + setMetaEntries((prev) => + prev.map((m, i) => + i === index ? { ...m, key: value } : m, + ), + ); + }} + className="h-8 flex-1" + /> + + { + const value = e.target.value; + setMetaEntries((prev) => + prev.map((m, i) => + i === index ? { ...m, value } : m, + ), + ); + }} + className="h-8 flex-1" + /> + +
+ ))} +
+ )} +
{selectedTool.outputSchema && (
@@ -262,7 +365,7 @@ const ToolsTab = ({ selectedTool._meta && (
-

Meta:

+

Meta Schema:

- +
+ +
+ {entries.map((entry, index) => ( +
+
+ + updateEntry(index, "key", e.target.value)} + /> +
+
+ + updateEntry(index, "value", e.target.value)} + /> +
+ +
+ ))} +
+ + {entries.length === 0 && ( +
+

+ No meta data entries. Click "Add Entry" to add key-value pairs. +

+
+ )} +
+ + ); +}; + +export default MetaDataTab; diff --git a/client/src/components/__tests__/MetaDataTab.test.tsx b/client/src/components/__tests__/MetaDataTab.test.tsx new file mode 100644 index 00000000..e26d0b65 --- /dev/null +++ b/client/src/components/__tests__/MetaDataTab.test.tsx @@ -0,0 +1,558 @@ +import { render, screen, fireEvent } from "@testing-library/react"; +import "@testing-library/jest-dom"; +import MetaDataTab from "../MetaDataTab"; +import { Tabs } from "@/components/ui/tabs"; + +describe("MetaDataTab", () => { + const defaultProps = { + metaData: {}, + onMetaDataChange: jest.fn(), + }; + + const renderMetaDataTab = (props = {}) => { + return render( + + + , + ); + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe("Initial Rendering", () => { + it("should render the metadata tab with title and description", () => { + renderMetaDataTab(); + + expect(screen.getByText("Meta Data")).toBeInTheDocument(); + expect( + screen.getByText( + "Key-value pairs that will be included in all MCP requests", + ), + ).toBeInTheDocument(); + }); + + it("should render Add Entry button", () => { + renderMetaDataTab(); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + expect(addButton).toBeInTheDocument(); + }); + + it("should show empty state message when no entries exist", () => { + renderMetaDataTab(); + + expect( + screen.getByText( + 'No meta data entries. Click "Add Entry" to add key-value pairs.', + ), + ).toBeInTheDocument(); + }); + + it("should not show empty state message when entries exist", () => { + renderMetaDataTab({ + metaData: { key1: "value1" }, + }); + + expect( + screen.queryByText( + 'No meta data entries. Click "Add Entry" to add key-value pairs.', + ), + ).not.toBeInTheDocument(); + }); + }); + + describe("Initial Data Handling", () => { + it("should initialize with existing metadata", () => { + const initialMetaData = { + API_KEY: "test-key", + VERSION: "1.0.0", + }; + + renderMetaDataTab({ metaData: initialMetaData }); + + expect(screen.getByDisplayValue("API_KEY")).toBeInTheDocument(); + expect(screen.getByDisplayValue("test-key")).toBeInTheDocument(); + expect(screen.getByDisplayValue("VERSION")).toBeInTheDocument(); + expect(screen.getByDisplayValue("1.0.0")).toBeInTheDocument(); + }); + + it("should render multiple entries in correct order", () => { + const initialMetaData = { + FIRST: "first-value", + SECOND: "second-value", + THIRD: "third-value", + }; + + renderMetaDataTab({ metaData: initialMetaData }); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + expect(keyInputs).toHaveLength(3); + expect(valueInputs).toHaveLength(3); + + // Check that entries are rendered in the order they appear in the object + const entries = Object.entries(initialMetaData); + entries.forEach(([key, value], index) => { + expect(keyInputs[index]).toHaveValue(key); + expect(valueInputs[index]).toHaveValue(value); + }); + }); + }); + + describe("Adding Entries", () => { + it("should add a new empty entry when Add Entry button is clicked", () => { + renderMetaDataTab(); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + expect(keyInputs).toHaveLength(1); + expect(valueInputs).toHaveLength(1); + expect(keyInputs[0]).toHaveValue(""); + expect(valueInputs[0]).toHaveValue(""); + }); + + it("should add multiple entries when Add Entry button is clicked multiple times", () => { + renderMetaDataTab(); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + + fireEvent.click(addButton); + fireEvent.click(addButton); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + expect(keyInputs).toHaveLength(3); + expect(valueInputs).toHaveLength(3); + }); + + it("should hide empty state message after adding first entry", () => { + renderMetaDataTab(); + + expect( + screen.getByText( + 'No meta data entries. Click "Add Entry" to add key-value pairs.', + ), + ).toBeInTheDocument(); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + expect( + screen.queryByText( + 'No meta data entries. Click "Add Entry" to add key-value pairs.', + ), + ).not.toBeInTheDocument(); + }); + }); + + describe("Removing Entries", () => { + it("should render remove button for each entry", () => { + renderMetaDataTab({ + metaData: { key1: "value1", key2: "value2" }, + }); + + const removeButtons = screen.getAllByRole("button", { name: "" }); // Trash icon buttons have no text + expect(removeButtons).toHaveLength(2); + }); + + it("should remove entry when remove button is clicked", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { key1: "value1", key2: "value2" }, + onMetaDataChange, + }); + + const removeButtons = screen.getAllByRole("button", { name: "" }); + fireEvent.click(removeButtons[0]); + + expect(onMetaDataChange).toHaveBeenCalledWith({ key2: "value2" }); + }); + + it("should remove correct entry when multiple entries exist", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { + FIRST: "first-value", + SECOND: "second-value", + THIRD: "third-value", + }, + onMetaDataChange, + }); + + const removeButtons = screen.getAllByRole("button", { name: "" }); + fireEvent.click(removeButtons[1]); // Remove second entry + + expect(onMetaDataChange).toHaveBeenCalledWith({ + FIRST: "first-value", + THIRD: "third-value", + }); + }); + + it("should show empty state message after removing all entries", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { key1: "value1" }, + onMetaDataChange, + }); + + const removeButton = screen.getByRole("button", { name: "" }); + fireEvent.click(removeButton); + + expect(onMetaDataChange).toHaveBeenCalledWith({}); + }); + }); + + describe("Editing Entries", () => { + it("should update key when key input is changed", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { oldKey: "value1" }, + onMetaDataChange, + }); + + const keyInput = screen.getByDisplayValue("oldKey"); + fireEvent.change(keyInput, { target: { value: "newKey" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ newKey: "value1" }); + }); + + it("should update value when value input is changed", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { key1: "oldValue" }, + onMetaDataChange, + }); + + const valueInput = screen.getByDisplayValue("oldValue"); + fireEvent.change(valueInput, { target: { value: "newValue" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ key1: "newValue" }); + }); + + it("should handle editing multiple entries independently", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { + key1: "value1", + key2: "value2", + }, + onMetaDataChange, + }); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + // Edit first entry key + fireEvent.change(keyInputs[0], { target: { value: "newKey1" } }); + expect(onMetaDataChange).toHaveBeenCalledWith({ + newKey1: "value1", + key2: "value2", + }); + + // Clear mock to test second edit independently + onMetaDataChange.mockClear(); + + // Edit second entry value + fireEvent.change(valueInputs[1], { target: { value: "newValue2" } }); + expect(onMetaDataChange).toHaveBeenCalledWith({ + newKey1: "value1", + key2: "newValue2", + }); + }); + }); + + describe("Data Validation and Trimming", () => { + it("should trim whitespace from keys and values", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInput = screen.getByPlaceholderText("Key"); + const valueInput = screen.getByPlaceholderText("Value"); + + fireEvent.change(keyInput, { target: { value: " trimmedKey " } }); + fireEvent.change(valueInput, { target: { value: " trimmedValue " } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + trimmedKey: "trimmedValue", + }); + }); + + it("should exclude entries with empty keys or values after trimming", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + // First entry: valid key and value + fireEvent.change(keyInputs[0], { target: { value: "validKey" } }); + fireEvent.change(valueInputs[0], { target: { value: "validValue" } }); + + // Second entry: empty key (should be excluded) + fireEvent.change(keyInputs[1], { target: { value: "" } }); + fireEvent.change(valueInputs[1], { target: { value: "someValue" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + validKey: "validValue", + }); + }); + + it("should exclude entries with whitespace-only keys or values", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + // First entry: valid key and value + fireEvent.change(keyInputs[0], { target: { value: "validKey" } }); + fireEvent.change(valueInputs[0], { target: { value: "validValue" } }); + + // Second entry: whitespace-only key (should be excluded) + fireEvent.change(keyInputs[1], { target: { value: " " } }); + fireEvent.change(valueInputs[1], { target: { value: "someValue" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + validKey: "validValue", + }); + }); + + it("should handle mixed valid and invalid entries", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + fireEvent.click(addButton); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + // First entry: valid + fireEvent.change(keyInputs[0], { target: { value: "key1" } }); + fireEvent.change(valueInputs[0], { target: { value: "value1" } }); + + // Second entry: empty key (invalid) + fireEvent.change(keyInputs[1], { target: { value: "" } }); + fireEvent.change(valueInputs[1], { target: { value: "value2" } }); + + // Third entry: valid + fireEvent.change(keyInputs[2], { target: { value: "key3" } }); + fireEvent.change(valueInputs[2], { target: { value: "value3" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + key1: "value1", + key3: "value3", + }); + }); + }); + + describe("Input Accessibility", () => { + it("should have proper labels for screen readers", () => { + renderMetaDataTab({ + metaData: { key1: "value1" }, + }); + + const keyLabel = screen.getByLabelText("Key", { selector: "input" }); + const valueLabel = screen.getByLabelText("Value", { selector: "input" }); + + expect(keyLabel).toBeInTheDocument(); + expect(valueLabel).toBeInTheDocument(); + }); + + it("should have unique IDs for each input pair", () => { + renderMetaDataTab({ + metaData: { + key1: "value1", + key2: "value2", + }, + }); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + expect(keyInputs[0]).toHaveAttribute("id", "key-0"); + expect(keyInputs[1]).toHaveAttribute("id", "key-1"); + expect(valueInputs[0]).toHaveAttribute("id", "value-0"); + expect(valueInputs[1]).toHaveAttribute("id", "value-1"); + }); + + it("should have proper placeholder text", () => { + renderMetaDataTab(); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInput = screen.getByPlaceholderText("Key"); + const valueInput = screen.getByPlaceholderText("Value"); + + expect(keyInput).toHaveAttribute("placeholder", "Key"); + expect(valueInput).toHaveAttribute("placeholder", "Value"); + }); + }); + + describe("Edge Cases", () => { + it("should handle special characters in keys and values", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInput = screen.getByPlaceholderText("Key"); + const valueInput = screen.getByPlaceholderText("Value"); + + fireEvent.change(keyInput, { + target: { value: "key-with-special@chars!" }, + }); + fireEvent.change(valueInput, { + target: { value: "value with spaces & symbols $%^" }, + }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + "key-with-special@chars!": "value with spaces & symbols $%^", + }); + }); + + it("should handle unicode characters", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInput = screen.getByPlaceholderText("Key"); + const valueInput = screen.getByPlaceholderText("Value"); + + fireEvent.change(keyInput, { target: { value: "🔑_key" } }); + fireEvent.change(valueInput, { target: { value: "值_value_🎯" } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + "🔑_key": "值_value_🎯", + }); + }); + + it("should handle very long keys and values", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + + const keyInput = screen.getByPlaceholderText("Key"); + const valueInput = screen.getByPlaceholderText("Value"); + + const longKey = "A".repeat(100); + const longValue = "B".repeat(500); + + fireEvent.change(keyInput, { target: { value: longKey } }); + fireEvent.change(valueInput, { target: { value: longValue } }); + + expect(onMetaDataChange).toHaveBeenCalledWith({ + [longKey]: longValue, + }); + }); + + it("should handle duplicate keys by keeping the last one", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ onMetaDataChange }); + + const addButton = screen.getByRole("button", { name: /add entry/i }); + fireEvent.click(addButton); + fireEvent.click(addButton); + + const keyInputs = screen.getAllByPlaceholderText("Key"); + const valueInputs = screen.getAllByPlaceholderText("Value"); + + // Set same key for both entries + fireEvent.change(keyInputs[0], { target: { value: "duplicateKey" } }); + fireEvent.change(valueInputs[0], { target: { value: "firstValue" } }); + + fireEvent.change(keyInputs[1], { target: { value: "duplicateKey" } }); + fireEvent.change(valueInputs[1], { target: { value: "secondValue" } }); + + // The second value should overwrite the first + expect(onMetaDataChange).toHaveBeenCalledWith({ + duplicateKey: "secondValue", + }); + }); + }); + + describe("Integration with Parent Component", () => { + it("should not call onMetaDataChange when component mounts with existing data", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { key1: "value1" }, + onMetaDataChange, + }); + + expect(onMetaDataChange).not.toHaveBeenCalled(); + }); + + it("should call onMetaDataChange only when user makes changes", () => { + const onMetaDataChange = jest.fn(); + renderMetaDataTab({ + metaData: { key1: "value1" }, + onMetaDataChange, + }); + + // Should not be called on mount + expect(onMetaDataChange).not.toHaveBeenCalled(); + + // Should be called when user changes value + const valueInput = screen.getByDisplayValue("value1"); + fireEvent.change(valueInput, { target: { value: "newValue" } }); + + expect(onMetaDataChange).toHaveBeenCalledTimes(1); + expect(onMetaDataChange).toHaveBeenCalledWith({ key1: "newValue" }); + }); + + it("should maintain internal state when props change (component doesn't sync with prop changes)", () => { + const { rerender } = renderMetaDataTab({ + metaData: { key1: "value1" }, + }); + + expect(screen.getByDisplayValue("key1")).toBeInTheDocument(); + expect(screen.getByDisplayValue("value1")).toBeInTheDocument(); + + // Rerender with different props - component should maintain its internal state + // This is the intended behavior since useState initializer only runs once + rerender( + + + , + ); + + // The component should still show the original values since it maintains internal state + expect(screen.getByDisplayValue("key1")).toBeInTheDocument(); + expect(screen.getByDisplayValue("value1")).toBeInTheDocument(); + // The new prop values should not be displayed + expect(screen.queryByDisplayValue("key2")).not.toBeInTheDocument(); + expect(screen.queryByDisplayValue("value2")).not.toBeInTheDocument(); + }); + }); +}); diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 37073e9b..7bb3d01c 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -79,6 +79,7 @@ interface UseConnectionOptions { // eslint-disable-next-line @typescript-eslint/no-explicit-any getRoots?: () => any[]; defaultLoggingLevel?: LoggingLevel; + metaData?: Record; } export function useConnection({ @@ -97,6 +98,7 @@ export function useConnection({ onElicitationRequest, getRoots, defaultLoggingLevel, + metaData = {}, }: UseConnectionOptions) { const [connectionStatus, setConnectionStatus] = useState("disconnected"); @@ -153,6 +155,20 @@ export function useConnection({ try { const abortController = new AbortController(); + // Add metadata to the request if available, but skip for tool calls + // as they handle metadata merging separately + const shouldAddGeneralMeta = + request.method !== "tools/call" && Object.keys(metaData).length > 0; + const requestWithMeta = shouldAddGeneralMeta + ? { + ...request, + params: { + ...request.params, + _meta: metaData, + }, + } + : request; + // prepare MCP Client request options const mcpRequestOptions: RequestOptions = { signal: options?.signal ?? abortController.signal, @@ -181,13 +197,17 @@ export function useConnection({ let response; try { - response = await mcpClient.request(request, schema, mcpRequestOptions); + response = await mcpClient.request( + requestWithMeta, + schema, + mcpRequestOptions, + ); - pushHistory(request, response); + pushHistory(requestWithMeta, response); } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); - pushHistory(request, { error: errorMessage }); + pushHistory(requestWithMeta, { error: errorMessage }); throw error; } From 0123a71da49f9fd6b2066b644e35042c3099b482 Mon Sep 17 00:00:00 2001 From: olaservo Date: Wed, 8 Oct 2025 08:43:11 -0700 Subject: [PATCH 05/83] Add input validation guidelines and enhance cleanParams function to preserve default values --- CLAUDE.md | 11 +++ client/src/utils/__tests__/paramUtils.test.ts | 72 +++++++++++++++++++ client/src/utils/paramUtils.ts | 12 +++- 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index ec826b1f..1e6a37c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,3 +31,14 @@ The project is organized as a monorepo with workspaces: - `client/`: React frontend with Vite, TypeScript and Tailwind - `server/`: Express backend with TypeScript - `cli/`: Command-line interface for testing and invoking MCP server methods directly + +## Tool Input Validation Guidelines + +When handling tool input parameters and form fields: + +- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages +- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema + +These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. diff --git a/client/src/utils/__tests__/paramUtils.test.ts b/client/src/utils/__tests__/paramUtils.test.ts index 3dfd5382..6c853490 100644 --- a/client/src/utils/__tests__/paramUtils.test.ts +++ b/client/src/utils/__tests__/paramUtils.test.ts @@ -205,4 +205,76 @@ 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") + }); + }); }); diff --git a/client/src/utils/paramUtils.ts b/client/src/utils/paramUtils.ts index dfc41802..aa538f40 100644 --- a/client/src/utils/paramUtils.ts +++ b/client/src/utils/paramUtils.ts @@ -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 { const cleaned: Record = {}; 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) { From 89dc57c4883372c51acb4600b54404d6b0d1c562 Mon Sep 17 00:00:00 2001 From: olaservo Date: Sun, 12 Oct 2025 21:13:56 -0700 Subject: [PATCH 06/83] Add tool input validation guidelines to AGENTS.md --- AGENTS.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index ec826b1f..1e6a37c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,3 +31,14 @@ The project is organized as a monorepo with workspaces: - `client/`: React frontend with Vite, TypeScript and Tailwind - `server/`: Express backend with TypeScript - `cli/`: Command-line interface for testing and invoking MCP server methods directly + +## Tool Input Validation Guidelines + +When handling tool input parameters and form fields: + +- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value +- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects +- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages +- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema + +These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers. From c2dbff738e582941d6b1af04c4b9f41c28305487 Mon Sep 17 00:00:00 2001 From: Stanislav Deviatykh Date: Thu, 16 Oct 2025 00:50:42 +0300 Subject: [PATCH 07/83] fix: set user specified scope in DCR --- client/src/lib/auth.ts | 34 ++++++++++++++++++++++----- client/src/lib/constants.ts | 1 + client/src/lib/hooks/useConnection.ts | 12 ++++++---- client/src/lib/oauth-state-machine.ts | 29 ++++++++++++++--------- 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 23e21b6a..edd3ef58 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -102,16 +102,35 @@ export const clearClientInformationFromSessionStorage = ({ sessionStorage.removeItem(key); }; +export const getScopeFromSessionStorage = ( + serverUrl: string, +): string | undefined => { + const key = getServerSpecificKey(SESSION_KEYS.SCOPE, serverUrl); + const value = sessionStorage.getItem(key); + return value || undefined; +}; + +export const saveScopeToSessionStorage = ( + serverUrl: string, + scope: string | undefined, +) => { + const key = getServerSpecificKey(SESSION_KEYS.SCOPE, serverUrl); + if (scope) { + sessionStorage.setItem(key, scope); + } else { + sessionStorage.removeItem(key); + } +}; + export class InspectorOAuthClientProvider implements OAuthClientProvider { - constructor( - protected serverUrl: string, - scope?: string, - ) { - this.scope = scope; + constructor(protected serverUrl: string) { // Save the server URL to session storage sessionStorage.setItem(SESSION_KEYS.SERVER_URL, serverUrl); } - scope: string | undefined; + + get scope(): string | undefined { + return getScopeFromSessionStorage(this.serverUrl); + } get redirectUrl() { return window.location.origin + "/oauth/callback"; @@ -224,6 +243,9 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { sessionStorage.removeItem( getServerSpecificKey(SESSION_KEYS.CODE_VERIFIER, this.serverUrl), ); + sessionStorage.removeItem( + getServerSpecificKey(SESSION_KEYS.SCOPE, this.serverUrl), + ); } } diff --git a/client/src/lib/constants.ts b/client/src/lib/constants.ts index 8e4ff2b5..7e4e0697 100644 --- a/client/src/lib/constants.ts +++ b/client/src/lib/constants.ts @@ -17,6 +17,7 @@ export const SESSION_KEYS = { PREREGISTERED_CLIENT_INFORMATION: "mcp_preregistered_client_information", SERVER_METADATA: "mcp_server_metadata", AUTH_DEBUGGER_STATE: "mcp_auth_debugger_state", + SCOPE: "mcp_scope", } as const; // Generate server-specific session storage keys diff --git a/client/src/lib/hooks/useConnection.ts b/client/src/lib/hooks/useConnection.ts index 8639feeb..f6d42380 100644 --- a/client/src/lib/hooks/useConnection.ts +++ b/client/src/lib/hooks/useConnection.ts @@ -45,6 +45,7 @@ import { clearClientInformationFromSessionStorage, InspectorOAuthClientProvider, saveClientInformationToSessionStorage, + saveScopeToSessionStorage, discoverScopes, } from "../auth"; import { @@ -142,6 +143,10 @@ export function useConnection({ }); }, [oauthClientId, oauthClientSecret, sseUrl]); + useEffect(() => { + saveScopeToSessionStorage(sseUrl, oauthScope); + }, [oauthScope, sseUrl]); + const pushHistory = (request: object, response?: object) => { setRequestHistory((prev) => [ ...prev, @@ -346,10 +351,9 @@ export function useConnection({ } scope = await discoverScopes(sseUrl, resourceMetadata); } - const serverAuthProvider = new InspectorOAuthClientProvider( - sseUrl, - scope, - ); + + saveScopeToSessionStorage(sseUrl, scope); + const serverAuthProvider = new InspectorOAuthClientProvider(sseUrl); const result = await auth(serverAuthProvider, { serverUrl: sseUrl, diff --git a/client/src/lib/oauth-state-machine.ts b/client/src/lib/oauth-state-machine.ts index c505f698..8dc9da8f 100644 --- a/client/src/lib/oauth-state-machine.ts +++ b/client/src/lib/oauth-state-machine.ts @@ -80,13 +80,16 @@ export const oauthTransitions: Record = { const metadata = context.state.oauthMetadata!; const clientMetadata = context.provider.clientMetadata; - // Prefer scopes from resource metadata if available - const scopesSupported = - context.state.resourceMetadata?.scopes_supported || - metadata.scopes_supported; - // Add all supported scopes to client registration - if (scopesSupported) { - clientMetadata.scope = scopesSupported.join(" "); + // Priority: user-provided scope > discovered scopes + if (!context.provider.scope || context.provider.scope.trim() === "") { + // Prefer scopes from resource metadata if available + const scopesSupported = + context.state.resourceMetadata?.scopes_supported || + metadata.scopes_supported; + // Add all supported scopes to client registration + if (scopesSupported) { + clientMetadata.scope = scopesSupported.join(" "); + } } // Try Static client first, with DCR as fallback @@ -113,10 +116,14 @@ export const oauthTransitions: Record = { const metadata = context.state.oauthMetadata!; const clientInformation = context.state.oauthClientInfo!; - const scope = await discoverScopes( - context.serverUrl, - context.state.resourceMetadata ?? undefined, - ); + // Priority: user-provided scope > discovered scopes + let scope = context.provider.scope; + if (!scope || scope.trim() === "") { + scope = await discoverScopes( + context.serverUrl, + context.state.resourceMetadata ?? undefined, + ); + } const { authorizationUrl, codeVerifier } = await startAuthorization( context.serverUrl, From 40fa6cb27d65fec026bec36f54e7f8d059817e2f Mon Sep 17 00:00:00 2001 From: Stanislav Deviatykh Date: Thu, 16 Oct 2025 00:57:06 +0300 Subject: [PATCH 08/83] refactor test --- client/src/lib/hooks/__tests__/useConnection.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/client/src/lib/hooks/__tests__/useConnection.test.tsx b/client/src/lib/hooks/__tests__/useConnection.test.tsx index a9ba6825..2d09b8b2 100644 --- a/client/src/lib/hooks/__tests__/useConnection.test.tsx +++ b/client/src/lib/hooks/__tests__/useConnection.test.tsx @@ -112,6 +112,7 @@ jest.mock("../../auth", () => ({ })), clearClientInformationFromSessionStorage: jest.fn(), saveClientInformationToSessionStorage: jest.fn(), + saveScopeToSessionStorage: jest.fn(), discoverScopes: jest.fn(), })); From d6b179e6a8d5a2a84f6800aeed26c41609688499 Mon Sep 17 00:00:00 2001 From: Basil Hosmer Date: Thu, 16 Oct 2025 17:13:14 -0400 Subject: [PATCH 09/83] Increase default request timeout to 5 minutes - Changed MCP_SERVER_REQUEST_TIMEOUT from 10s to 5m (300000ms) - Supports elicitation tools and other long-running operations - Updated documentation to clarify client-side vs server-side timeouts - Users can still customize via UI config or URL parameters --- README.md | 18 ++++++++++-------- client/src/lib/configurationTypes.ts | 3 ++- client/src/lib/constants.ts | 5 +++-- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 29037550..040edb94 100644 --- a/README.md +++ b/README.md @@ -236,13 +236,15 @@ ALLOWED_ORIGINS=http://localhost:6274,http://localhost:8000 npm start The MCP Inspector supports the following configuration settings. To change them, click on the `Configuration` button in the MCP Inspector UI: -| Setting | Description | Default | -| --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | -| `MCP_SERVER_REQUEST_TIMEOUT` | Timeout for requests to the MCP server (ms) | 10000 | -| `MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS` | Reset timeout on progress notifications | true | -| `MCP_REQUEST_MAX_TOTAL_TIMEOUT` | Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications) | 60000 | -| `MCP_PROXY_FULL_ADDRESS` | Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577 | "" | -| `MCP_AUTO_OPEN_ENABLED` | Enable automatic browser opening when inspector starts (works with authentication enabled). Only as environment var, not configurable in browser. | true | +| Setting | Description | Default | +| --------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | +| `MCP_SERVER_REQUEST_TIMEOUT` | Client-side timeout (ms) - Inspector will cancel the request if no response is received within this time. Note: servers may have their own timeouts | 300000 | +| `MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS` | Reset timeout on progress notifications | true | +| `MCP_REQUEST_MAX_TOTAL_TIMEOUT` | Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications) | 60000 | +| `MCP_PROXY_FULL_ADDRESS` | Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577 | "" | +| `MCP_AUTO_OPEN_ENABLED` | Enable automatic browser opening when inspector starts (works with authentication enabled). Only as environment var, not configurable in browser. | true | + +**Note on Timeouts:** The timeout settings above control when the Inspector (as an MCP client) will cancel requests. These are independent of any server-side timeouts. For example, if a server tool has a 10-minute timeout but the Inspector's timeout is set to 30 seconds, the Inspector will cancel the request after 30 seconds. Conversely, if the Inspector's timeout is 10 minutes but the server times out after 30 seconds, you'll receive the server's timeout error. For tools that require user interaction (like elicitation) or long-running operations, ensure the Inspector's timeout is set appropriately. These settings can be adjusted in real-time through the UI and will persist across sessions. @@ -361,7 +363,7 @@ http://localhost:6274/?transport=stdio&serverCommand=npx&serverArgs=arg1%20arg2 You can also set initial config settings via query params, for example: ``` -http://localhost:6274/?MCP_SERVER_REQUEST_TIMEOUT=10000&MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS=false&MCP_PROXY_FULL_ADDRESS=http://10.1.1.22:5577 +http://localhost:6274/?MCP_SERVER_REQUEST_TIMEOUT=60000&MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS=false&MCP_PROXY_FULL_ADDRESS=http://10.1.1.22:5577 ``` Note that if both the query param and the corresponding localStorage item are set, the query param will take precedence. diff --git a/client/src/lib/configurationTypes.ts b/client/src/lib/configurationTypes.ts index 263b2cf2..2a849299 100644 --- a/client/src/lib/configurationTypes.ts +++ b/client/src/lib/configurationTypes.ts @@ -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; diff --git a/client/src/lib/constants.ts b/client/src/lib/constants.ts index 8e4ff2b5..85cc0b36 100644 --- a/client/src/lib/constants.ts +++ b/client/src/lib/constants.ts @@ -43,8 +43,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: { From e74cbf0d408c8e21936e13ac2ca2b7f0f37d87d2 Mon Sep 17 00:00:00 2001 From: Basil Hosmer Date: Thu, 16 Oct 2025 17:25:51 -0400 Subject: [PATCH 10/83] Fix tests to match updated timeout description --- client/src/components/__tests__/Sidebar.test.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/client/src/components/__tests__/Sidebar.test.tsx b/client/src/components/__tests__/Sidebar.test.tsx index c1f39592..03e898ca 100644 --- a/client/src/components/__tests__/Sidebar.test.tsx +++ b/client/src/components/__tests__/Sidebar.test.tsx @@ -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, }, From 9dc3b748a86d1c0b31de4d38679d16c37be04b01 Mon Sep 17 00:00:00 2001 From: Jesse Lumarie Date: Sat, 18 Oct 2025 20:42:46 -0600 Subject: [PATCH 11/83] mcp: add icons --- client/src/App.tsx | 2 + client/src/__tests__/App.routing.test.tsx | 1 + client/src/components/IconDisplay.tsx | 57 +++++++++++++++++++++++ client/src/components/PromptsTab.tsx | 3 ++ client/src/components/Sidebar.tsx | 45 ++++++++++++++++++ client/src/components/ToolsTab.tsx | 13 ++++++ client/src/lib/hooks/useConnection.ts | 8 ++++ 7 files changed, 129 insertions(+) create mode 100644 client/src/components/IconDisplay.tsx diff --git a/client/src/App.tsx b/client/src/App.tsx index 5937d738..1427a4f6 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -249,6 +249,7 @@ const App = () => { const { connectionStatus, serverCapabilities, + serverImplementation, mcpClient, requestHistory, clearRequestHistory, @@ -920,6 +921,7 @@ const App = () => { loggingSupported={!!serverCapabilities?.logging || false} connectionType={connectionType} setConnectionType={setConnectionType} + serverImplementation={serverImplementation} />
{ + if (!icons || icons.length === 0) { + return null; + } + + const sizeClasses = { + sm: "w-4 h-4", + md: "w-6 h-6", + lg: "w-8 h-8", + }; + + const sizeClass = sizeClasses[size]; + + return ( +
+ {icons.map((icon, index) => ( + { + // Hide broken images + e.currentTarget.style.display = "none"; + }} + /> + ))} +
+ ); +}; + +export default IconDisplay; diff --git a/client/src/components/PromptsTab.tsx b/client/src/components/PromptsTab.tsx index 424845d0..3e53df11 100644 --- a/client/src/components/PromptsTab.tsx +++ b/client/src/components/PromptsTab.tsx @@ -14,6 +14,7 @@ import { useEffect, useState } from "react"; import ListPane from "./ListPane"; import { useCompletionState } from "@/lib/hooks/useCompletionState"; import JsonView from "./JsonView"; +import IconDisplay from "./IconDisplay"; export type Prompt = { name: string; @@ -23,6 +24,7 @@ export type Prompt = { description?: string; required?: boolean; }[]; + icons?: { src: string; mimeType?: string; sizes?: string[] }[]; }; const PromptsTab = ({ @@ -109,6 +111,7 @@ const PromptsTab = ({ }} renderItem={(prompt) => (
+ {prompt.name} {prompt.description} diff --git a/client/src/components/Sidebar.tsx b/client/src/components/Sidebar.tsx index b369a967..13a4f24f 100644 --- a/client/src/components/Sidebar.tsx +++ b/client/src/components/Sidebar.tsx @@ -14,6 +14,7 @@ import { RefreshCwOff, Copy, CheckCheck, + Server, } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; @@ -40,6 +41,7 @@ import { import CustomHeaders from "./CustomHeaders"; import { CustomHeaders as CustomHeadersType } from "@/lib/types/customHeaders"; import { useToast } from "../lib/hooks/useToast"; +import IconDisplay, { WithIcons } from "./IconDisplay"; interface SidebarProps { connectionStatus: ConnectionStatus; @@ -71,6 +73,9 @@ interface SidebarProps { setConfig: (config: InspectorConfig) => void; connectionType: "direct" | "proxy"; setConnectionType: (type: "direct" | "proxy") => void; + serverImplementation?: + | (WithIcons & { name?: string; version?: string; websiteUrl?: string }) + | null; } const Sidebar = ({ @@ -102,6 +107,7 @@ const Sidebar = ({ setConfig, connectionType, setConnectionType, + serverImplementation, }: SidebarProps) => { const [theme, setTheme] = useTheme(); const [showEnvVars, setShowEnvVars] = useState(false); @@ -776,6 +782,45 @@ const Sidebar = ({
+ {connectionStatus === "connected" && serverImplementation && ( +
+
+ {(serverImplementation as WithIcons).icons && + (serverImplementation as WithIcons).icons!.length > 0 ? ( + + ) : ( + + )} + {(serverImplementation as { websiteUrl?: string }) + .websiteUrl ? ( + + {serverImplementation.name || "MCP Server"} + + ) : ( + + {serverImplementation.name || "MCP Server"} + + )} +
+ {serverImplementation.version && ( +
+ Version: {serverImplementation.version} +
+ )} +
+ )} + {loggingSupported && connectionStatus === "connected" && (