🎨 fix: Fall Back From an Invalid Theme in librechat.yaml Instead of Exiting - #16462
Merged
Merged
Conversation
A mistake in interface.theme failed the whole librechat.yaml and stopped the server. The loader now checks the theme with the rules the client applies before painting it, drops a theme the client would reject with a warning naming each problem by path, and loads the rest of the config. Unknown appearance tokens only warn, matching the reader. Startup and reload behave the same. The token lists and the validator move to librechat-data-provider so the client registry and the server check against one list.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebdb7e6b78
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof theme !== 'string' && !isPlainThemeRecord(theme)) { | ||
| return [{ path: [], message: 'Expected a bundled theme name or an inline theme definition' }]; | ||
| } | ||
| const issues = typeof theme === 'string' ? [] : collectThemeIssues(theme); |
There was a problem hiding this comment.
When interface.theme is a string such as clickhous, this branch skips collectThemeIssues, and deploymentThemeSchema accepts every non-empty string. However, resolveDeploymentTheme in client/src/Providers/DeploymentTheme.tsx recognizes only librechat and clickhouse, so the client still rejects the value. The new loader therefore neither removes nor server-warns about this invalid theme, contrary to its intended server-side fallback behavior; validate string values against the bundled theme registry or otherwise surface them as unsupported.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Fixed in c7c4bb1. The bundled names now live in librechat-data-provider (bundledThemeNames); the client map in DeploymentTheme.tsx is typed Record<BundledThemeName, ThemeDefinition> so the two cannot drift, and the loader drops an unknown name with 'interface.theme: Unknown bundled theme "clickhous", expected one of: librechat, clickhouse'. Covered by loader.spec (unknown-name, bundled) and scenario yaml-theme-unknown-name-falls-back.
A misspelled bundled name such as clickhous passed the server and was rejected only in the browser. The bundled names now live in librechat-data-provider; the client's theme map is typed against that list and the loader drops a name outside it with a warning.
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
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.
Pull Request
Summary
A mistake in
interface.themein librechat.yaml failed validation of the whole config, and the loader exits the process at startup (packages/api/src/app/loader.ts). A color key without thergb-prefix, a mistyped field such ascolours:, a value spaced as'240 244 255', or a theme written as a list was enough to stop the server. Other mistakes, like anrgb-token that does not exist or an out-of-range value, passed the loose server schema and were rejected silently in the browser.The loader now checks
interface.themewith the same rules the client applies before painting a theme. A theme the client would reject is removed before the config is validated, with one warning that names each problem by its path (interface.theme.modes.light.colors.rgb-surfce-secondary: Unknown color token: rgb-surfce-secondary), so the deployment runs on the default theme and every other setting loads. Unknown appearance tokens only warn and the theme is kept, matching the reader semantics from #16373. Startup and reload behave the same, and an error anywhere else in the config still exits at startup or rejects the reload. A valid theme is returned as the same object.To keep one list, the color, appearance and brand token names and the validator move from the client registry to a new
librechat-data-providermodule (theme.ts). The client registry re-exports them with unchanged messages, and a compile-time check fails the build if the client theme types drift from the shared lists.config.tsis not touched.How it works
Type of change
Testing
Tested environments/configuration:
api/server/services/Config/loadCustomConfig.jswiring in a child process.Automated tests:
packages/api/src/app/loader.spec.tswith real yaml fixtures underpackages/api/src/app/__fixtures__/theme: valid theme (unchanged, no warning), unknown color token, bad values, a malformed theme, a value only the schema rejects, an unknown appearance token (kept), an unrelated config error (still exits), and the same cases in reload mode. 10 passed.e2e/specs/mock/scenarios/yaml-theme-fallback.spec.ts, 5 scenarios driving the/apiloader wiring. 5 passed.packages/clienttheme specs (registry, ThemeProvider, applyTheme, high contrast, ClickHouse, tailwind, tokens, SegmentedMeter): 314 passed with the moved validator.api/server/services/Config/loadCustomConfig.spec.js: 28 passed.clientDeploymentTheme spec: 16 passed.npm run static-checks:full -- --against origin/canary: passed (depcheck not installed locally).Screenshots / recordings
No user-facing change. An invalid theme already rendered the default theme in the browser; it now does so without stopping the server.
Risk / compatibility
A theme that used to load but was rejected by the client (for example an unknown
rgb-token) is now also removed from/api/config. The browser result is the same default theme, and the reason moves from the browser console to the server log. Principal-scoped admin overrides ofinterface.themedo not go through this loader and are unchanged. Two appearance checks (a named shadow color and the fullbox-shadowvalue) use the browser's CSS parser, which Node does not have, so the server can keep a shadow value the browser later rejects; the client then discards the theme and falls back as it does today. The server never drops a theme the browser would paint.Checklist