Skip to content

fix(ui): use stored-credentials endpoint for tools fetch on MCP edit page - #26002

Merged
ryan-crabbe-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_fix-edit-page-tools-fetch-422
Apr 27, 2026
Merged

fix(ui): use stored-credentials endpoint for tools fetch on MCP edit page#26002
ryan-crabbe-berri merged 3 commits into
litellm_internal_stagingfrom
litellm_fix-edit-page-tools-fetch-422

Conversation

@ryan-crabbe-berri

@ryan-crabbe-berri ryan-crabbe-berri commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Summary

The MCP server edit page (mcp_server_edit.tsx) was firing a spurious POST /mcp-rest/test/tools/list on mount to populate the Tool Configuration panel. That endpoint is the temp-session preview path — it requires inline credentials in the request body — but the on-mount fetch deliberately omitted them. For any saved server with auth_type of api_key / bearer_token / basic / authorization, the call hit the upstream MCP without auth and the panel surfaced "Unable to load tools — Failed to connect to MCP server."

This PR redirects the edit page through the stored-credentials path:

  1. Edit page's local fetch → switched from testMCPToolsListRequest (POST /mcp-rest/test/tools/list) to listMCPTools (GET /mcp-rest/tools/list?server_id=...), which looks up stored credentials on the backend by server_id. No inline creds needed.

  2. Tool Configuration panel → now consumes the parent's GET-fetched tools via the existing externalTools / externalIsLoading / externalError / externalCanFetch props on MCPToolConfiguration, so its internal useTestMCPConnection hook (which still POSTs without creds) is bypassed on the edit flow. Without this second wiring the spurious POST and the user-visible "Unable to load tools" error would still happen — see commit ebffbd1a.

The POST /test/tools/list endpoint and useTestMCPConnection hook remain in use for the live "Test Connection" path (testing unsaved config in the create flow), which is unchanged.

screenshots

before
Screenshot 2026-04-25 at 4 49 36 PM

after
Screenshot 2026-04-25 at 5 07 32 PM

Test plan

  • All 94 MCP tools UI tests pass (vitest run src/components/mcp_tools/)
  • QA verified end-to-end against a local mock MCP server requiring X-API-Key auth:
    • demo_api_key_server registered with stored credentials.auth_value matching the mock's expected key
    • Opened Settings → Edit Settings for that server
    • Tool Configuration panel renders the ping tool with "1 of 1 tool enabled for user access" (previously: red "Unable to load tools")
    • Network tab shows only GET /mcp-rest/tools/list?server_id=... → 200 (previously: also fired POST /mcp-rest/test/tools/list that failed)
  • QA: Verify the "Test Connection" button in the create flow still works (unchanged path: useTestMCPConnectionPOST /test/tools/list with inline creds from the form)

…page

The edit page was calling POST /mcp-rest/test/tools/list (the temp-session
endpoint that requires inline credentials) on mount. Since fetchTools
deliberately omits credentials from the request body, any server with
auth_type api_key/bearer_token/basic/authorization would 422.

Switch to GET /mcp-rest/tools/list?server_id=... which looks up stored
credentials on the backend — no inline creds needed for saved servers.
@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a 422 error on the MCP server edit page by replacing the on-mount tool fetch (POST /mcp-rest/test/tools/list requiring inline credentials) with GET /mcp-rest/tools/list?server_id=... via the existing listMCPTools() function, which uses stored backend credentials. It also lifts tool state (tools, isLoadingTools, toolsError) into the parent and passes it to MCPToolConfiguration via new external* props, disabling the component's internal useTestMCPConnection auto-fetch.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped, replaces a broken endpoint call with a correct one, and all 94 tests pass.

No P0 or P1 issues found. The logic is correct: the GET endpoint uses stored credentials server-side so no inline auth is needed; guards for URL/OAuth token are correctly removed since the backend handles those; error state is properly surfaced through toolsError. The OAuth UX regression (tools not auto-refreshing after interactive token receipt) was noted in a prior review thread and is intentionally out of scope here.

No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx Switches on-mount tool fetch to GET /mcp-rest/tools/list via listMCPTools; removes unnecessary inline-credential guards; adds toolsError state and passes external tool state to MCPToolConfiguration
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx Updates mock from testMCPToolsListRequest to listMCPTools to match the production code change; existing test coverage and assertions are unaffected

Sequence Diagram

sequenceDiagram
    participant UI as MCPServerEdit (mount)
    participant FT as fetchTools()
    participant NET as listMCPTools()
    participant BE as GET /mcp-rest/tools/list

    UI->>FT: useEffect([mcpServer, accessToken])
    FT->>NET: listMCPTools(accessToken, server_id)
    NET->>BE: GET /mcp-rest/tools/list?server_id={id}
    BE-->>NET: {tools:[...]}
    NET-->>FT: {tools:[...], error:null}
    FT->>UI: setTools(tools) / setToolsError(msg)
    UI->>UI: MCPToolConfiguration externalTools={tools}

    note over UI: useTestMCPConnection inside MCPToolConfiguration disabled (enabled=false) when externalTools provided
Loading

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile

Comment on lines 275 to +280
useEffect(() => {
// Don't fetch if server hasn't been saved yet (no permanent server_id)
if (!mcpServer.server_id || mcpServer.server_id.trim() === "") {
return;
}
fetchTools();
}, [mcpServer, accessToken, oauthAccessToken]);
}, [mcpServer, accessToken]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 OAuth interactive flow: tools no longer refresh after token receive

Removing oauthAccessToken from the dep array is correct for the GET-based fetch, but as a side effect, interactive OAuth servers that complete the in-page OAuth flow (via "Authorize & Fetch Token") will no longer see an automatic tool refresh after the token arrives. Previously the effect re-ran with the new token and called the POST endpoint; now the user would need to save the server and re-open the edit page. This is a minor UX regression worth documenting, even if the "Test Connection" button covers the live-test use case.

@krrish-berri-2

Copy link
Copy Markdown
Contributor

@ryan-crabbe-berri — could you add a screenshot or short video showing that this change works as expected? It really helps reviewers verify the fix quickly. Thanks!

Pass externalTools/externalIsLoading/externalError/externalCanFetch from the
edit page so MCPToolConfiguration consumes the parent's GET fetch instead of
firing its own POST /test/tools/list via useTestMCPConnection. Eliminates
the spurious POST that caused the user-visible "Unable to load tools" error
for api_key/bearer_token/basic/authorization servers.
@veria-ai

veria-ai Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Low: No security issues

This PR refactors the MCP server edit page to fetch tools via the stored-credentials GET endpoint (listMCPTools) instead of the POST test endpoint that required sending inline credentials. This is a net positive for security — less credential data transits from the UI. No new security concerns.


Status: 0 open
Risk: 1/10

Posted by Veria AI · 2026-04-26T00:13:24.523Z

@codecov

codecov Bot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ryan-crabbe-berri
ryan-crabbe-berri merged commit d120ddf into litellm_internal_staging Apr 27, 2026
116 checks passed
@ryan-crabbe-berri
ryan-crabbe-berri deleted the litellm_fix-edit-page-tools-fetch-422 branch April 27, 2026 16:03
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
…ools-fetch-422

fix(ui): use stored-credentials endpoint for tools fetch on MCP edit page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants