build(docker): rebuild dashboard UI from source on every image build - #31
Merged
Conversation
PROBLEM The upstream BerriAI/litellm flow commits the Next.js dashboard bundle to git at litellm/proxy/_experimental/out/ and the Dockerfile just COPYs it. docker/build_admin_ui.sh exists but is a no-op unless the enterprise_colors.json customization marker is present. For this fork that means: any UI source change in ui/litellm-dashboard/ src/ does NOT reach the runtime image — the image keeps serving whatever was last committed to _experimental/out/ (currently April 2026 upstream version). PR #25 (credential form reset), #27 (401 status-aware redirect v1), and #30 (auth error.type routing v2) all shipped UI source but their deployed effect on docker-built images is zero. The defect was only visible via e2e ("why doesn't the live bundle have my code?"). FIX Add an explicit UI build step in the Dockerfile builder stage that runs `npm ci && npm run build` from current source, then overwrites /app/litellm/proxy/_experimental/out/ with the fresh `out/` directory. node_modules and the build cache are pruned in the same RUN so the builder layer stays slim. The existing build_admin_ui.sh hook is kept as the enterprise customization path (no-op by default). Trade-offs vs the upstream commit-the-bundle approach: + UI source changes ALWAYS flow into the image (no "did I run npm run build?" foot-gun). + PR diffs stay clean — no more 100+ file _experimental/out/ churn per UI PR (the recent UI PRs would have been 685 fewer files). + Git history is honest about what's source vs build artifact. - docker build is 2-3 minutes slower (npm install + next build). - Diverges from upstream's pattern, so future upstream rebases that touch _experimental/out/ will need conflict resolution. Manageable because we're now ignoring that path and rebuilding ourselves. GITIGNORE litellm/proxy/_experimental/out/ is added to .gitignore along with the sibling next.js outputs (.next/, out/, node_modules under ui/litellm-dashboard/). 683 previously-tracked artifact files are removed via `git rm --cached` so the working tree stays usable for local non-docker dev (the files remain on disk) but they no longer add diff noise. LOCAL DEV NOTE Developers running the proxy outside docker need `cd ui/litellm-dashboard && npm run build` once to populate _experimental/out/. CLAUDE.md will be updated in a follow-up to reflect this; for now the comment in .gitignore documents the path. VERIFICATION - `git status` after this commit: clean except untracked _experimental/out/ on disk. - `e2e/tools/proxy build` (which invokes docker buildx against this Dockerfile) successfully produces an image. Tested in a follow-up step by stacking PR #30 onto this branch and verifying the served JS chunks contain the auth_session_expired / auth_permission_denied strings — proving the UI build step actually picks up source changes that were never committed to _experimental/out/.
3 tasks
songkuan-zheng
added a commit
that referenced
this pull request
May 29, 2026
…#32) PR #31 shipped the UI rebuild step but missed three refinements discovered during live e2e: (a) the builder image's older npm rejects the project's .npmrc `min-release-age=3d` (npm 11+ supply-chain hardening) as a fatal config error, so override it to 0 just for the docker RUN; (b) switch from `&& \` chain to `set -eux; ...; \` so a failed step actually aborts the build instead of being swallowed by the trailing `|| true`; (c) add `test -f out/index.html` validations after both the build and the copy so a silent-success build can't ship a broken image. Also adds `.gstack/` to .gitignore (gstack working dir — never useful to commit) and a trailing newline cleanup. Without (a), CI's release-docker workflow fails at `npm ci`. Without (b)+(c), failures are invisible and the image ships with a stale _experimental/out/.
1 task
songkuan-zheng
added a commit
that referenced
this pull request
Jun 4, 2026
…e (Wave 7 — final) (#61) * fix(ui): route 401/403 by backend error.type, heuristic only as fallback (#30) * fix(ui): route 401/403 by backend error.type, heuristic only as fallback Companion to fix/proxy-auth-error-type-taxonomy (PR D1) which gives the backend a structured `error.type` for every wrapped auth failure. This PR makes handleErrorResponse read that field FIRST and dispatch by table lookup, with the existing cookie + marker heuristic preserved as a fallback for: - older backend builds that don't emit the new types yet - `type=auth_error` (generic, backend couldn't classify) - unknown / typo'd / future type strings (graceful degradation) Three-tier decision: 1. errorData.error.type or errorData.type: auth_session_expired / auth_invalid_credentials → REDIRECT_LOGIN expired_key / token_not_found_in_db (legacy) → REDIRECT_LOGIN auth_permission_denied → TOAST key/team/user/org/project/vector_store *_access_denied → TOAST team_member_permission_error → TOAST budget_exceeded → TOAST auth_error or unknown → HEURISTIC (step 2) 2. status-based heuristic (legacy / unknown-type fallback): 401 + no cookie OR has marker → REDIRECT 401 + cookie + no marker → TOAST (safe default for 401-as-403) 403 → TOAST 3. non-auth errors → delegate to legacy handleError Why a static table instead of more code: the action is a pure function of the type. Adding a new auth_* type on the backend (e.g. for a new auth mode) requires one line here, no logic change. Test plan: - vitest run src/components/networking.test.ts → 35 passed - 14 new tests in "handleErrorResponse - type-based auth routing (D1 contract)" describe block: - REDIRECT_LOGIN: auth_session_expired (even with cookie present), auth_invalid_credentials, expired_key (legacy), token_not_found_in_db (legacy) - TOAST: auth_permission_denied (the original bug), key_model_access_denied, team_member_permission_error, budget_exceeded - HEURISTIC fallback: auth_error + cookie present (no redirect), auth_error + cookie absent (redirect), unknown type (heuristic) - Body-shape robustness: top-level type field, string body, no type field Graceful degradation: if D1 ships AFTER this PR, the legacy heuristic keeps the UI working. If D2 ships AFTER D1, the new types just route through the heuristic until D2 lands — no breakage either way. * fix(ui): make legacy handleError also read error.type Live e2e revealed handleErrorResponse was dead code — ~30 existing callers use handleError(errorData) directly, not handleErrorResponse. The new type-aware dispatch table only fired for tests, never for real fetch sites. Make the legacy handleError itself read errorData.error.type first and dispatch via AUTH_ERROR_TYPE_TO_ACTION: REDIRECT_LOGIN (auth_session_expired / auth_invalid_credentials / expired_key / token_not_found_in_db) -> triggerSessionExpiredRedirect() TOAST (auth_permission_denied / *_access_denied / *_permission_error / budget_exceeded) -> do nothing here (preserves legacy "silent for non-session" behavior — caller decides whether to toast) HEURISTIC / missing / unknown -> existing message-marker scan + redirect-on-marker This way every existing caller automatically benefits from D1's backend taxonomy with zero call-site migration. Live e2e (rebuilt docker bundle + actual served chunks): 9/9 cases pass — for each backend-emitted type, the live JS chunk maps it to the expected REDIRECT_LOGIN / TOAST action: C1 no-auth-header -> auth_invalid_credentials -> REDIRECT_LOGIN C2 invalid-bearer -> auth_invalid_credentials -> REDIRECT_LOGIN C3 admin-only -> auth_permission_denied -> TOAST (the bug) C5 empty-bearer -> auth_invalid_credentials -> REDIRECT_LOGIN C6 admin+nokey -> auth_invalid_credentials -> REDIRECT_LOGIN C8 unknown-sk-key -> token_not_found_in_db -> REDIRECT_LOGIN C10 unknown-sk-chat -> token_not_found_in_db -> REDIRECT_LOGIN C11 deleted-key -> token_not_found_in_db -> REDIRECT_LOGIN C12 lowpriv-userlist -> (no type, 403) -> TOAST Test plan: - vitest run src/components/networking.test.ts -> 43 passed (5 new "handleError (legacy) - now also reads error.type" cases covering session_expired / invalid_credentials / token_not_found_in_db -> REDIRECT, permission_denied / budget_exceeded -> no redirect, marker fallback when no type, type=auth_error fallback) - Live e2e: built UI -> copied to litellm/proxy/_experimental/out/ -> e2e/tools/proxy build -> 9/9 contract tests pass against live container * fix(ui): redirect to login on session-expired 401, not on all 401s The dashboard's handleError() only matched the exact string "Authentication Error - Expired Key" to detect session expiry. Three problems with that: 1. String-match is fragile — backend reworded the message (or returned an empty message, after the auth log cleanup) and the redirect stopped firing. Users saw 401 toasts and nothing happened. 2. The single marker missed every other session-loss variant: no auth header, key revoked, invalid token, master-key-required, ... 3. No HTTP status awareness — both "session is gone" (401 with no cookie) and "permission denied for this endpoint" (401 because LiteLLM uses 401 for role mismatch too) hit the same path. The "permission denied" case must NOT bounce the user to login because they're already logged in fine; only this specific call fails. Fix: - Add `handleErrorResponse(response, errorData)` — status-aware, preferred whenever the caller has the Response object: * 401 + no auth cookie OR session-expired marker -> redirect to login * 401 + cookie still present + no expiry marker -> toast, no redirect (this is the role-mismatch case) * 403 -> toast only, never redirect (permission denied) * other status -> delegate to handleError (rate-limited path) - Expand handleError's marker list (SESSION_EXPIRED_SIGNALS) to catch more wording variants. handleError keeps its existing signature for backward compat with the ~30 existing callers; new code should use handleErrorResponse instead. - Extract triggerSessionExpiredRedirect() so the "clear cookies + redirect" mechanics live in one place across both code paths. Test plan: - vitest run src/components/networking.test.ts -> 21 passed - New tests (5): handleErrorResponse with 401-no-cookie, 401-with- expiry-marker, 401-still-valid-cookie, 403, 500 — verifies redirect fires only when the session is actually gone. Companion to backend PR that downgrades these auth failures from ERROR to WARN — together they make 401 both quiet in logs and correctly handled in UI. * fix(ui): redirect to login on 401 token_not_found_in_db (#38) * fix(ui): redirect to login on 401 token_not_found_in_db (and siblings) Two-part fix for the bug where the dashboard never redirects to the login page on 401 errors with `error.type=token_not_found_in_db` (and other AUTH_ERROR_TYPE_TO_ACTION REDIRECT_LOGIN types). Patch 1 — root cause: fetch callsites were calling `handleError(deriveErrorMessage(errorData))`, pre-stringifying the error before `handleError` could read `error.type`. `extractErrorType` explicitly rejects strings, so the type-aware fast path never fired. Now callsites pass the raw `errorData` object; `handleError`'s already-implemented D1 taxonomy dispatch finally takes effect. Affects 178 callsites across `networking.tsx` (163) and `src/app/(dashboard)/hooks/` (15 files). Patch 2 — defense in depth: the string-based fallback `SESSION_EXPIRED_SIGNALS` listed `"Authentication Error - Invalid"` (dash) but the backend's token-not-found path emits `"Authentication Error, Invalid"` (comma). Added the comma variant so the heuristic path also catches it when the structured `type` field is absent (older backends). The existing test `handleError redirects when error.type= token_not_found_in_db` (networking.test.ts:394) already asserted the intended behavior; this PR makes production match the test. Updated `useAccessGroups.test.ts` to reflect the new contract (callers pass the object, not the derived message string). * fix(ui): handle MCP tool call errorData scope after Patch 1 sed sweep The previous commit (Patch 1) bulk-replaced `handleError(errorMessage)` with `handleError(errorData)` across 178 callsites. One callsite — the MCP tool call error path — declared `errorData` inside a `try` block, so the replacement referenced an out-of-scope variable and broke the Next.js TypeScript compile (verified via `next build` in the litellm-e2e Docker image rebuild). Fix: hoist `let errorData: any = null` outside the try; assign inside on successful JSON parse. Pass `errorData ?? responseText` to handleError so the structured-type path still fires when JSON is valid, falling back to the raw text otherwise (which `handleError` already handles via its string-marker heuristic). Verified end-to-end against the e2e Docker image: login → inject fake JWT (valid format, key field points to non-existent virtual key) → navigate to /ui/?page=teams → React fires /team/list and friends → backend returns 401 with `error.type=auth_invalid_credentials` → handleError reads the type → triggerSessionExpiredRedirect → window.location.href to /ui/ → server-side guard sends to /ui/login. Critically, the error message "Authentication Error, LiteLLM Virtual Key expected..." does NOT match any SESSION_EXPIRED_SIGNALS marker, so this redirect path is reachable ONLY via the structured-type channel (Patch 1), confirming the fix's contract. * build(docker): rebuild dashboard UI from source on every image build (#31) PROBLEM The upstream BerriAI/litellm flow commits the Next.js dashboard bundle to git at litellm/proxy/_experimental/out/ and the Dockerfile just COPYs it. docker/build_admin_ui.sh exists but is a no-op unless the enterprise_colors.json customization marker is present. For this fork that means: any UI source change in ui/litellm-dashboard/ src/ does NOT reach the runtime image — the image keeps serving whatever was last committed to _experimental/out/ (currently April 2026 upstream version). PR #25 (credential form reset), #27 (401 status-aware redirect v1), and #30 (auth error.type routing v2) all shipped UI source but their deployed effect on docker-built images is zero. The defect was only visible via e2e ("why doesn't the live bundle have my code?"). FIX Add an explicit UI build step in the Dockerfile builder stage that runs `npm ci && npm run build` from current source, then overwrites /app/litellm/proxy/_experimental/out/ with the fresh `out/` directory. node_modules and the build cache are pruned in the same RUN so the builder layer stays slim. The existing build_admin_ui.sh hook is kept as the enterprise customization path (no-op by default). Trade-offs vs the upstream commit-the-bundle approach: + UI source changes ALWAYS flow into the image (no "did I run npm run build?" foot-gun). + PR diffs stay clean — no more 100+ file _experimental/out/ churn per UI PR (the recent UI PRs would have been 685 fewer files). + Git history is honest about what's source vs build artifact. - docker build is 2-3 minutes slower (npm install + next build). - Diverges from upstream's pattern, so future upstream rebases that touch _experimental/out/ will need conflict resolution. Manageable because we're now ignoring that path and rebuilding ourselves. GITIGNORE litellm/proxy/_experimental/out/ is added to .gitignore along with the sibling next.js outputs (.next/, out/, node_modules under ui/litellm-dashboard/). 683 previously-tracked artifact files are removed via `git rm --cached` so the working tree stays usable for local non-docker dev (the files remain on disk) but they no longer add diff noise. LOCAL DEV NOTE Developers running the proxy outside docker need `cd ui/litellm-dashboard && npm run build` once to populate _experimental/out/. CLAUDE.md will be updated in a follow-up to reflect this; for now the comment in .gitignore documents the path. VERIFICATION - `git status` after this commit: clean except untracked _experimental/out/ on disk. - `e2e/tools/proxy build` (which invokes docker buildx against this Dockerfile) successfully produces an image. Tested in a follow-up step by stacking PR #30 onto this branch and verifying the served JS chunks contain the auth_session_expired / auth_permission_denied strings — proving the UI build step actually picks up source changes that were never committed to _experimental/out/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Upstream BerriAI/litellm commits the Next.js dashboard bundle to git at
litellm/proxy/_experimental/out/and the Dockerfile justCOPYs it.docker/build_admin_ui.shexists but is a no-op unless an enterprise customization marker is present. This means any UI source change inui/litellm-dashboard/src/does NOT reach the runtime image — the image keeps serving whatever was last committed to_experimental/out/(currently April 2026 upstream version).Concrete blast radius from this fork's recent UI PRs
error.typerouting (v2) + smart handleErrorAll three shipped TypeScript source but the runtime image keeps serving the upstream bundle from April. The defect was only visible via the e2e investigation in the prior session — "why doesn't the live bundle have my code?".
Fix
Add an explicit UI build step in the Dockerfile builder stage that runs
npm ci && npm run buildfrom current source, then overwrites/app/litellm/proxy/_experimental/out/with the freshout/directory.node_modulesand the build cache are pruned in the same RUN so the builder layer stays slim. The existingbuild_admin_ui.shhook is preserved as the enterprise customization path (no-op by default).Plus housekeeping:
.gitignoreaddslitellm/proxy/_experimental/out/(and sibling next.js outputs).git rm --cachedso the working tree stays usable for local non-docker dev (files remain on disk) but no longer add diff noise.Trade-offs vs the upstream commit-the-bundle approach
Wins
_experimental/out/churn per UI PR. The recent UI PRs would have been ~685 fewer files.Costs
_experimental/out/will need conflict resolution. Manageable now that we're ignoring that path and rebuilding ourselves.Local dev note
Developers running the proxy outside docker need:
once to populate
_experimental/out/for direct serving. The comment in.gitignoredocuments this.Test plan
git statusafter this commit: clean.e2e/tools/proxy buildsuccessfully produces an image with ourDockerfile.Stacked PR fix(ui): route 401/403 by backend error.type, heuristic only as fallback #30 onto this branch + rebuilt: served JS chunks in the running container contain the new dispatch map:
This proves PR fix(ui): route 401/403 by backend error.type, heuristic only as fallback #30's source changes (which never landed in the committed
_experimental/out/) DO reach the image now that the Dockerfile rebuilds the UI.Merge guidance
Merge this BEFORE cutting the next internal release tag — otherwise PR #25, #27, #30 ship to docker without effect (just like their previous merged versions).
After merging:
! scripts/release-tag.sh v1.83.10-internal.6The release-docker.yml workflow will then automatically produce an image whose UI bundle reflects current ship/v1.83.10 source.