fix(admin-ui): emit nested routes as <dir>/index.html so /ui/mcp/oauth/callback works - #28106
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Linear and other OAuth providers redirect the user back to /ui/mcp/oauth/callback?code=...&state=... after the consent step. The packaged Next.js static export only produced /ui/mcp/oauth/callback.html, so FastAPI's StaticFiles served a 404 on the extensionless URL and the OAuth handshake never completed. The Dockerfile.non_root build step tried to paper over this at image-build time with `for html_file in *.html; do ...`, but that shell glob does not recurse, so nested routes like mcp/oauth/callback.html were left stranded next to an empty mcp/oauth/callback/ directory containing only Next.js metadata. The runtime restructure step in proxy_server.py was then skipped because the .litellm_ui_ready marker had already been dropped. Set trailingSlash: true in the dashboard's Next.js config so the export emits every nested route as <dir>/index.html natively. The Dockerfile loop is now a no-op for the bundled UI and has been removed; the .litellm_ui_ready marker is still written so the proxy keeps skipping the redundant Python restructure step at startup. Stacks on top of the static export regeneration in the parent branch.
fa2a492 to
3cc3557
Compare
Greptile SummaryThis PR fixes the
Confidence Score: 5/5This PR is safe to merge — it removes dead workaround code, fixes the export layout at the source, and adds a test that will catch the same regression in the future. All three changes are tightly scoped: a one-line Next.js config change that produces a correct export layout, removal of a shell loop that was demonstrably broken for nested paths, and a new test that directly validates the fix. No logic changes touch the request path or auth layer. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/next.config.mjs | Adds trailingSlash: true so Next.js static export emits every route as <dir>/index.html, fixing the root cause of the 404 on nested routes like /ui/mcp/oauth/callback. |
| docker/Dockerfile.non_root | Removes the broken shell-glob restructure loop (which only iterated top-level *.html files and missed nested routes); .litellm_ui_ready marker is still written so the proxy startup skips its Python-side restructure step. |
| tests/test_litellm/proxy/test_proxy_server.py | Adds test_admin_ui_export_serves_nested_extensionless_routes that validates the actual bundled export has no stray <name>.html nested files, confirms callback/index.html exists, and verifies the 307→200 redirect chain via TestClient (in-process, no real network calls). |
Reviews (2): Last reviewed commit: "fix(admin-ui): emit nested routes as <di..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def test_admin_ui_export_serves_nested_extensionless_routes(): | ||
| out_dir = ( | ||
| Path(litellm.__file__).parent / "proxy" / "_experimental" / "out" | ||
| ) | ||
| assert out_dir.is_dir(), f"missing UI export at {out_dir}" | ||
|
|
||
| nested_html_offenders = [ | ||
| path.relative_to(out_dir).as_posix() | ||
| for path in out_dir.rglob("*.html") | ||
| if path.parent != out_dir | ||
| and path.name != "index.html" | ||
| and "_next" not in path.parts | ||
| and "litellm-asset-prefix" not in path.parts | ||
| ] | ||
| assert not nested_html_offenders, ( | ||
| "Nested routes must be named index.html. Offenders: " | ||
| f"{nested_html_offenders}" | ||
| ) | ||
|
|
||
| callback_index = out_dir / "mcp" / "oauth" / "callback" / "index.html" | ||
| assert callback_index.is_file(), ( | ||
| f"MCP OAuth callback page must exist at {callback_index}; " | ||
| "without it /ui/mcp/oauth/callback 404s after Linear redirects back." | ||
| ) | ||
|
|
||
| fastapi_app = FastAPI() | ||
| fastapi_app.mount( | ||
| "/ui", StaticFiles(directory=str(out_dir), html=True), name="ui" | ||
| ) | ||
| client = TestClient(fastapi_app) | ||
|
|
||
| redirect = client.get( | ||
| "/ui/mcp/oauth/callback?code=abc&state=xyz", | ||
| follow_redirects=False, | ||
| ) | ||
| assert redirect.status_code == 307 | ||
| assert redirect.headers["location"].endswith("/ui/mcp/oauth/callback/?code=abc&state=xyz") | ||
|
|
||
| landed = client.get("/ui/mcp/oauth/callback?code=abc&state=xyz") | ||
| assert landed.status_code == 200 | ||
| assert "<html" in landed.text.lower() |
There was a problem hiding this comment.
test_admin_ui_export_serves_nested_extensionless_routes reads directly from the real litellm/proxy/_experimental/out/ directory rather than a tmp_path fixture. If this test is run before PR #28112 (the regenerated export artifact) is merged, it will fail at the assert out_dir.is_dir() or assert callback_index.is_file() checks with a descriptive error — but it will also fail in any CI run for this PR alone. This is intentional per the PR description, but it is worth confirming that the CI pipeline for this branch gates on #28112 being present before running the test suite.
There was a problem hiding this comment.
Yeah, it's fine. We will merge the other one first
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high mode and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3cc3557. Configure here.
| if path.parent != out_dir | ||
| and path.name != "index.html" | ||
| and "_next" not in path.parts | ||
| and "litellm-asset-prefix" not in path.parts |
There was a problem hiding this comment.
Test filters on absolute path parts, not relative
Low Severity
The offender-detection filter checks "_next" not in path.parts and "litellm-asset-prefix" not in path.parts, but path is an absolute Path from rglob, so path.parts includes every component of the full filesystem path — not just the portion under out_dir. If the repository happens to be checked out inside a directory named _next or litellm-asset-prefix, the filter would silently exclude all candidate files, causing the assertion to pass even when genuine offenders exist. Using path.relative_to(out_dir).parts instead of path.parts would scope the check to only the export-internal directory structure.
Reviewed by Cursor Bugbot for commit 3cc3557. Configure here.
| fi; \ | ||
| done && \ | ||
| touch .litellm_ui_ready ) | ||
| touch /var/lib/litellm/ui/.litellm_ui_ready |
There was a problem hiding this comment.
I believe all the docker files re-architect the files, not just non_root. Have you tested this with the other files?
There was a problem hiding this comment.
Dockerfile.non_root was the only one with a build-time restructure loop. Dockerfile and docker/Dockerfile.database never restructured at build time. They ship the raw _experimental/out/ and rely on the runtime restructure in proxy_server.py::_restructure_ui_html_files
After this PR, with trailingSlash: true, the Next.js export already emits /index.html natively. So for non-root -> no build-time restructure needed; we still touch .litellm_ui_ready to short-circuit the Python step on the read-only fs. ForsStandard / database -> nothing to change
7e0dced
into
litellm_rebuild_admin_ui_static_export
…28112) * chore(admin-ui): regenerate static export with trailingSlash: true Rebuilds litellm/proxy/_experimental/out/ from ui/litellm-dashboard with `trailingSlash: true` enabled in next.config.mjs. Next.js now emits every route as <dir>/index.html (e.g. mcp/oauth/callback/index.html) instead of <dir>.html with a sibling metadata-only directory, which fixes the 404 on extensionless URLs served through FastAPI's StaticFiles(html=True) mount. This is the build artifact half of the fix; the config change, Dockerfile cleanup, and regression test live in the follow-up source PR that stacks on top of this branch. * fix(admin-ui): emit nested routes as <dir>/index.html (#28106) Linear and other OAuth providers redirect the user back to /ui/mcp/oauth/callback?code=...&state=... after the consent step. The packaged Next.js static export only produced /ui/mcp/oauth/callback.html, so FastAPI's StaticFiles served a 404 on the extensionless URL and the OAuth handshake never completed. The Dockerfile.non_root build step tried to paper over this at image-build time with `for html_file in *.html; do ...`, but that shell glob does not recurse, so nested routes like mcp/oauth/callback.html were left stranded next to an empty mcp/oauth/callback/ directory containing only Next.js metadata. The runtime restructure step in proxy_server.py was then skipped because the .litellm_ui_ready marker had already been dropped. Set trailingSlash: true in the dashboard's Next.js config so the export emits every nested route as <dir>/index.html natively. The Dockerfile loop is now a no-op for the bundled UI and has been removed; the .litellm_ui_ready marker is still written so the proxy keeps skipping the redundant Python restructure step at startup. Stacks on top of the static export regeneration in the parent branch. * chore: restore origin/litellm_internal_staging out files
…erriAI#28112) * chore(admin-ui): regenerate static export with trailingSlash: true Rebuilds litellm/proxy/_experimental/out/ from ui/litellm-dashboard with `trailingSlash: true` enabled in next.config.mjs. Next.js now emits every route as <dir>/index.html (e.g. mcp/oauth/callback/index.html) instead of <dir>.html with a sibling metadata-only directory, which fixes the 404 on extensionless URLs served through FastAPI's StaticFiles(html=True) mount. This is the build artifact half of the fix; the config change, Dockerfile cleanup, and regression test live in the follow-up source PR that stacks on top of this branch. * fix(admin-ui): emit nested routes as <dir>/index.html (BerriAI#28106) Linear and other OAuth providers redirect the user back to /ui/mcp/oauth/callback?code=...&state=... after the consent step. The packaged Next.js static export only produced /ui/mcp/oauth/callback.html, so FastAPI's StaticFiles served a 404 on the extensionless URL and the OAuth handshake never completed. The Dockerfile.non_root build step tried to paper over this at image-build time with `for html_file in *.html; do ...`, but that shell glob does not recurse, so nested routes like mcp/oauth/callback.html were left stranded next to an empty mcp/oauth/callback/ directory containing only Next.js metadata. The runtime restructure step in proxy_server.py was then skipped because the .litellm_ui_ready marker had already been dropped. Set trailingSlash: true in the dashboard's Next.js config so the export emits every nested route as <dir>/index.html natively. The Dockerfile loop is now a no-op for the bundled UI and has been removed; the .litellm_ui_ready marker is still written so the proxy keeps skipping the redundant Python restructure step at startup. Stacks on top of the static export regeneration in the parent branch. * chore: restore origin/litellm_internal_staging out files
2 participants
Relevant issues
Stacked on top of #28112 (the regenerated static export artifact). Merge that one first so this PR collapses cleanly into
litellm_internal_staging.After clicking Approve on the Linear MCP OAuth consent screen, the browser is redirected back to
http://<proxy>/ui/mcp/oauth/callback?code=...&state=...and the proxy returns{"detail":"Not Found"}, so the handshake never completes.Root cause is the packaged Next.js static export. With the default
next.config.mjs, every route is emitted as<name>.htmlat the parent level, with a sibling directory (e.g.mcp/oauth/callback/) containing only Next.js metadata.txtfiles and noindex.html. FastAPI'sStaticFiles(html=True)mount at/uifollows Starlette's rule of serving<dir>/index.htmlfor directory paths; it does not fall back to<path>.htmlfor extensionless requests, so a request for/ui/mcp/oauth/callbacklands on the emptycallback/directory and 404s.docker/Dockerfile.non_roottried to compensate at image-build time by walking the export and moving*.htmlinto<name>/index.html, but the loop uses a shell glob (for html_file in *.html) which does not recurse. The fix only touched top-level files; nested routes likemcp/oauth/callback.htmlwere left untouched. The marker.litellm_ui_readywas still dropped, so the Python-side runtime restructure step inproxy_server.pywas skipped at startup.Linear ticket
N/A
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Screenshots / Proof of Fix
Pulled the latest rc image, then ran the proxy with the rebuilt
_experimental/out/mounted into the runtime UI path that the non-root image expects:Static routing for the URL Linear actually redirects to, plus a handful of other UI routes for regression coverage:
Live LLM round-trip against a real provider through the upgraded image:
End-to-end browser flow to repro the original bug and confirm the fix:
http://localhost:4000/ui/mcp/oauth/callback?code=...&state=.... Before this PR that URL returned{"detail":"Not Found"}; after the PR it 307s to/ui/mcp/oauth/callback/and the dashboard finishes the token exchange.Type
Bug Fix
Changes
ui/litellm-dashboard/next.config.mjsnow setstrailingSlash: true, so the static export emits every nested route as<dir>/index.htmlnatively instead of<dir>.html. The matching regeneration oflitellm/proxy/_experimental/out/lives in the stacked artifact PR #28112.docker/Dockerfile.non_rootdrops the broken shell-glob restructure loop. It only saw top-level*.htmlfiles and never reachedmcp/oauth/callback.html, so it added no value once the export is already in the desired layout. The.litellm_ui_readyreadiness marker is still written so the proxy startup path keeps skipping the redundant Python restructure step.A regression test in
tests/test_litellm/proxy/test_proxy_server.pymounts the actual bundled export throughStaticFilesand asserts that/ui/mcp/oauth/callback?code=...&state=...307s to/ui/mcp/oauth/callback/?code=...&state=...and the followed redirect returns HTML. It also asserts that no nested route in the export ships as a stray<name>.html, which is what catches future regressions of eithertrailingSlashbeing removed or someone moving back to a recursion-broken restructure step.Note
Medium Risk
Changes the Admin UI static export layout and Docker image build steps, which could break existing UI routing if the exported artifact or proxy assumptions diverge.
Overview
Fixes Admin UI routing for extensionless nested paths by enabling
trailingSlashin the Next.js static export so routes are emitted as<dir>/index.html(e.g.,mcp/oauth/callback/index.html).Simplifies the non-root Docker build by removing the non-recursive HTML “restructure” loop and only writing the
.litellm_ui_readymarker after copying the prebuilt UI.Adds a regression test that validates the bundled export contains no nested non-
index.htmlpages and that/ui/mcp/oauth/callback?…redirects and serves HTML viaStaticFiles(html=True).Reviewed by Cursor Bugbot for commit 3cc3557. Bugbot is set up for automated code reviews on this repo. Configure here.