Skip to content

Conversation

@uc4w6c
Copy link
Collaborator

@uc4w6c uc4w6c commented Dec 10, 2025

Title

fix: MCP OAuth callback routing and URL handling

Relevant issues

#N/A

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • I have added a screenshot of my new test passing locally
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem

Type

🐛 Bug Fix

Changes

This PR addresses two issues with the MCP OAuth callback flow:

  1. The proxy’s UI restructuring logic only converted top-level *.html files into <route>/index.html.
    Nested routes like mcp/oauth/callback stayed as callback.html, so /ui/mcp/oauth/callback 404’d once the dashboard bundle was mounted behind the Python server. The fix walks the entire exported UI tree and moves every non-index HTML file into a matching folder, restoring nested routes automatically.

  2. Callback URLs generated in useMcpOAuthFlow (and the callback page itself) didn’t honor the /ui prefix or custom server_root_path, so they returned to / or the proxy root regardless of environment.
    The new logic inspects the current pathname to prepend /ui whenever the UI is being served from that path, ensuring both dev (npm run dev on port 3000) and proxy deployments redirect back to the correct dashboard entry point.

https://www.loom.com/share/84bfd182579e4cc69bd815f0755d4d20

@vercel
Copy link

vercel bot commented Dec 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
litellm Error Error Dec 11, 2025 0:21am

Copy link
Contributor

@ishaan-jaff ishaan-jaff left a comment

Choose a reason for hiding this comment

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

reviewed


app.mount("/ui", StaticFiles(directory=ui_path, html=True), name="ui")

def _restructure_ui_html_files(ui_root: str) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

can we get a unit test to ensure we don't loose this restructuring

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ishaan-jaff
I've added the test.

@krrishdholakia krrishdholakia merged commit e9571dd into main Dec 11, 2025
44 of 59 checks passed
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.

4 participants