mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(mcp): refresh tools when editing MCP server URL
When editing an MCP server's URL, the UI was caching stale tool lists from the previous server. The tool configuration component received static database values instead of live form values, so URL changes weren't detected. Changes: - Watch URL/spec_path form fields with Form.useWatch and pass live values to MCPToolConfiguration - Clear allowedTools when URL changes to trigger auto-select of new tools - Improve tool change detection by tracking tool names instead of count, to handle cases where two servers have the same number of tools - Add tests verifying URL changes clear allowedTools and pass live values Fixes LIT-1790 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
Krrish Dholakia
parent
121c633d6e
commit
f884d4cf6d
4 changed files with 128 additions and 13 deletions
|
|
@ -16289,7 +16289,7 @@
|
|||
"cache_read_input_token_cost": 3e-08,
|
||||
"input_cost_per_audio_token": 1e-06,
|
||||
"input_cost_per_token": 3e-07,
|
||||
"litellm_provider": "vertex_ai-language-models",
|
||||
"litellm_provider": "gemini",
|
||||
"max_audio_length_hours": 8.4,
|
||||
"max_audio_per_prompt": 1,
|
||||
"supports_reasoning": false,
|
||||
|
|
|
|||
|
|
@ -33,8 +33,9 @@ vi.mock("./MCPPermissionManagement", () => ({
|
|||
default: () => <div data-testid="mcp-permissions" />,
|
||||
}));
|
||||
|
||||
const MockMCPToolConfiguration = vi.fn(() => <div data-testid="mcp-tool-config" />);
|
||||
vi.mock("./mcp_tool_configuration", () => ({
|
||||
default: () => <div data-testid="mcp-tool-config" />,
|
||||
default: (props: any) => MockMCPToolConfiguration(props),
|
||||
}));
|
||||
|
||||
describe("MCPServerEdit (stdio)", () => {
|
||||
|
|
@ -152,3 +153,99 @@ describe("MCPServerEdit (stdio)", () => {
|
|||
expect(payload.env).toEqual({ CIRCLECI_TOKEN: "new-token", CIRCLECI_BASE_URL: "https://circleci.com" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServerEdit (URL change clears allowed tools)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("should pass live URL to MCPToolConfiguration when URL field changes", async () => {
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
server_id: "server-1",
|
||||
server_name: "exa",
|
||||
alias: "exa",
|
||||
description: "Exa MCP server",
|
||||
transport: "http",
|
||||
url: "https://exa.example.com/mcp",
|
||||
auth_type: "none",
|
||||
allowed_tools: ["web_search_exa", "get_code_context_exa"],
|
||||
created_at: "2024-01-01T00:00:00Z",
|
||||
created_by: "user-1",
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
updated_by: "user-1",
|
||||
mcp_access_groups: [],
|
||||
}}
|
||||
accessToken="access-token"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Initially, MCPToolConfiguration should receive the original URL
|
||||
const initialCall = MockMCPToolConfiguration.mock.calls.find(
|
||||
(call) => call[0]?.formValues?.url === "https://exa.example.com/mcp"
|
||||
);
|
||||
expect(initialCall).toBeDefined();
|
||||
|
||||
// Change the URL field
|
||||
const urlInput = screen.getByLabelText("MCP Server URL");
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, {
|
||||
target: { value: "https://seolinkmap.com/mcp" },
|
||||
});
|
||||
});
|
||||
|
||||
// After URL change, MCPToolConfiguration should receive the new URL
|
||||
await waitFor(() => {
|
||||
const latestCall = MockMCPToolConfiguration.mock.calls[MockMCPToolConfiguration.mock.calls.length - 1];
|
||||
expect(latestCall[0].formValues.url).toBe("https://seolinkmap.com/mcp");
|
||||
});
|
||||
});
|
||||
|
||||
it("should clear allowedTools when URL changes from original", async () => {
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
server_id: "server-1",
|
||||
server_name: "exa",
|
||||
alias: "exa",
|
||||
description: "Exa MCP server",
|
||||
transport: "http",
|
||||
url: "https://exa.example.com/mcp",
|
||||
auth_type: "none",
|
||||
allowed_tools: ["web_search_exa", "get_code_context_exa"],
|
||||
created_at: "2024-01-01T00:00:00Z",
|
||||
created_by: "user-1",
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
updated_by: "user-1",
|
||||
mcp_access_groups: [],
|
||||
}}
|
||||
accessToken="access-token"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Initially, allowedTools should be the existing tools
|
||||
const initialCall = MockMCPToolConfiguration.mock.calls[MockMCPToolConfiguration.mock.calls.length - 1];
|
||||
expect(initialCall[0].allowedTools).toEqual(["web_search_exa", "get_code_context_exa"]);
|
||||
|
||||
// Change the URL field to a different server
|
||||
const urlInput = screen.getByLabelText("MCP Server URL");
|
||||
await act(async () => {
|
||||
fireEvent.change(urlInput, {
|
||||
target: { value: "https://seolinkmap.com/mcp" },
|
||||
});
|
||||
});
|
||||
|
||||
// After URL change, allowedTools should be cleared
|
||||
await waitFor(() => {
|
||||
const latestCall = MockMCPToolConfiguration.mock.calls[MockMCPToolConfiguration.mock.calls.length - 1];
|
||||
expect(latestCall[0].allowedTools).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -41,6 +41,8 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
const [pendingRestoredValues, setPendingRestoredValues] = useState<Record<string, any> | null>(null);
|
||||
const authType = Form.useWatch("auth_type", form) as string | undefined;
|
||||
const transportType = Form.useWatch("transport", form) as string | undefined;
|
||||
const urlValue = Form.useWatch("url", form) as string | undefined;
|
||||
const specPathValue = Form.useWatch("spec_path", form) as string | undefined;
|
||||
const isStdioTransport = transportType === "stdio";
|
||||
const isOpenAPITransport = transportType === TRANSPORT.OPENAPI;
|
||||
const isMCPTransport = !isStdioTransport && !isOpenAPITransport;
|
||||
|
|
@ -191,6 +193,16 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
}
|
||||
}, [mcpServer]);
|
||||
|
||||
// Clear allowed tools when URL or spec_path changes to a different server
|
||||
// so that the auto-select logic in MCPToolConfiguration picks up the new tools
|
||||
useEffect(() => {
|
||||
const currentEndpoint = urlValue ?? specPathValue;
|
||||
const originalEndpoint = mcpServer.url ?? mcpServer.spec_path;
|
||||
if (currentEndpoint !== undefined && currentEndpoint !== originalEndpoint) {
|
||||
setAllowedTools([]);
|
||||
}
|
||||
}, [urlValue, specPathValue, mcpServer.url, mcpServer.spec_path]);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === "undefined") {
|
||||
return;
|
||||
|
|
@ -880,9 +892,10 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
|||
formValues={{
|
||||
server_id: mcpServer.server_id,
|
||||
server_name: mcpServer.server_name,
|
||||
url: mcpServer.url,
|
||||
transport: mcpServer.transport,
|
||||
auth_type: mcpServer.auth_type,
|
||||
url: urlValue ?? mcpServer.url,
|
||||
spec_path: specPathValue ?? mcpServer.spec_path,
|
||||
transport: transportType ?? mcpServer.transport,
|
||||
auth_type: authType ?? mcpServer.auth_type,
|
||||
mcp_info: mcpServer.mcp_info,
|
||||
oauth_flow_type: mcpServer.token_url ? OAUTH_FLOW.M2M : OAUTH_FLOW.INTERACTIVE,
|
||||
}}
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
existingAllowedTools,
|
||||
onAllowedToolsChange,
|
||||
}) => {
|
||||
const previousToolsLengthRef = useRef(0);
|
||||
const previousToolNamesRef = useRef<string>("");
|
||||
const [toolSearchTerm, setToolSearchTerm] = useState("");
|
||||
|
||||
const { tools, isLoadingTools, toolsError, canFetchTools } = useTestMCPConnection({
|
||||
|
|
@ -40,27 +40,32 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
|||
);
|
||||
});
|
||||
|
||||
// Auto-select tools when tools are first loaded
|
||||
// Auto-select tools when tools are first loaded or when tool list changes (e.g. URL changed)
|
||||
useEffect(() => {
|
||||
const currentToolNames = tools.map((t) => t.name).sort().join(",");
|
||||
// Only auto-select if:
|
||||
// 1. We have tools
|
||||
// 2. Tools length changed (new tools loaded)
|
||||
// 3. No tools are currently selected (initial state)
|
||||
if (tools.length > 0 && tools.length !== previousToolsLengthRef.current && allowedTools.length === 0) {
|
||||
// 2. Tool names changed from last fetch (detects both count and content changes)
|
||||
// 3. No tools are currently selected (initial state or cleared after URL change)
|
||||
if (tools.length > 0 && currentToolNames !== previousToolNamesRef.current && allowedTools.length === 0) {
|
||||
if (existingAllowedTools && existingAllowedTools.length > 0) {
|
||||
// If we have existing allowed tools, use those as the initial selection
|
||||
// Filter to only include tools that are actually available from the server
|
||||
const availableToolNames = tools.map((tool) => tool.name);
|
||||
const validExistingTools = existingAllowedTools.filter((toolName) => availableToolNames.includes(toolName));
|
||||
onAllowedToolsChange(validExistingTools);
|
||||
if (validExistingTools.length > 0) {
|
||||
onAllowedToolsChange(validExistingTools);
|
||||
} else {
|
||||
// No existing tools match the new server — select all new tools
|
||||
onAllowedToolsChange(availableToolNames);
|
||||
}
|
||||
} else {
|
||||
// If no existing allowed tools, auto-select all tools (create mode)
|
||||
const allToolNames = tools.map((tool) => tool.name);
|
||||
onAllowedToolsChange(allToolNames);
|
||||
}
|
||||
}
|
||||
// Update ref to track tools length (will be 0 when tools clear)
|
||||
previousToolsLengthRef.current = tools.length;
|
||||
previousToolNamesRef.current = currentToolNames;
|
||||
}, [tools, allowedTools.length, existingAllowedTools, onAllowedToolsChange]);
|
||||
|
||||
const handleToolToggle = (toolName: string) => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue