-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
fix(ui): fix /ui/chat 404 — Dockerfile pre-restructure + heuristic false-positive #24247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1204,7 +1204,27 @@ def _is_ui_pre_restructured(ui_dir: str) -> bool: | |
| if entry.is_dir() and not entry.name.startswith("_"): | ||
| index_path = os.path.join(entry.path, "index.html") | ||
| if os.path.exists(index_path): | ||
| # Found at least one restructured route - this proves the pattern | ||
| # Found at least one restructured route. | ||
| # Also verify no root-level .html files still need restructuring. | ||
| # Next.js static export may generate both pre-restructured pages | ||
| # (e.g. login/index.html) and new pages as root-level .html files | ||
| # (e.g. chat.html), causing the heuristic to fire prematurely. | ||
| try: | ||
| orphaned = [ | ||
| e.name | ||
| for e in os.scandir(ui_dir) | ||
| if e.is_file() | ||
| and e.name.endswith(".html") | ||
| and e.name not in ("index.html", "404.html") | ||
| ] | ||
| except (PermissionError, OSError): | ||
| orphaned = [] | ||
| if orphaned: | ||
| verbose_proxy_logger.debug( | ||
| f"Found un-restructured HTML files at root: {orphaned}. " | ||
| f"Restructuring needed despite existing {entry.name}/index.html." | ||
| ) | ||
| return False | ||
|
Comment on lines
+1212
to
+1227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The inner Consider hoisting the orphan check outside the loop so it is only performed once after confirming at least one restructured directory exists: restructured_found = None
for entry in os.scandir(ui_dir):
if entry.is_dir() and not entry.name.startswith("_"):
index_path = os.path.join(entry.path, "index.html")
if os.path.exists(index_path):
restructured_found = entry.name
break
if restructured_found:
try:
orphaned = [
e.name
for e in os.scandir(ui_dir)
if e.is_file()
and e.name.endswith(".html")
and e.name not in ("index.html", "404.html")
]
except (PermissionError, OSError):
orphaned = []
if orphaned:
verbose_proxy_logger.debug(...)
return False
verbose_proxy_logger.debug(...)
return True |
||
| verbose_proxy_logger.debug( | ||
| f"Detected restructured UI via pattern: found {entry.name}/index.html" | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any individual
mvoperation fails (e.g. a pre-existing{page}/index.htmlconflicts, or a filesystem error occurs), theforloop continues because shellforloops do not propagate inner command failures to the outer flow. The finaltouch .litellm_ui_readythen still executes, marking the UI as fully restructured even if some HTML files were not moved.At runtime
_is_ui_pre_restructured()sees the marker file and returnsTrueearly, skipping the Python fallback, so the unprocessed route will still 404.Consider adding
set -eat the top of theRUNor using|| exit 1after the move to ensure the build fails loudly rather than silently producing a partial result: