🧩 feat: Ignore Unknown Theme Color Tokens Instead of Dropping the Theme - #16529
Conversation
A color token the reader does not know, with a plain token name and an RGB triplet value, now warns and is left out of the resolved theme, matching how unknown appearance tokens already behave. The yaml loader, the config schema, the client registry, stored definitions and the deployment theme all share the one rule, so a misspelled or newer role costs only itself. An invalid value, known token or not, and a malformed name still reject the theme.
berry-13
changed the title
A client older than the server tolerates only the unknown color names it was built to expect, so the loader now removes the tokens it warned about before the theme is served, instead of handing a cached client a definition it could reject whole.
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: 97b2ff1bf3
ℹ️ 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".
| const warningIssues = collectThemeWarningIssues(theme); | ||
| const warnings = warningIssues.map(formatIssue); | ||
| const served = isPlainThemeRecord(theme) ? withoutUnknownColors(theme, warningIssues) : theme; |
There was a problem hiding this comment.
When a principal-scoped DB override supplies an inline theme containing a newly accepted non-rgb- token such as surface-future, this sanitation never sees it: checkConfigTheme runs while loading the base YAML, then packages/api/src/app/service.ts merges overrides, and api/server/routes/config.js sends the resulting interfaceConfig directly. A cached pre-change client rejects that raw token and discards the entire theme, so the mixed-version protection described here only works for base YAML themes; apply equivalent validation/filtering to the final merged theme before it is served. CLAUDE.mdL145-L147
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6f3a5f9. getAppConfig now runs the loader's theme rules on the merged config whenever a DB override replaced the theme (checkOverrideTheme in packages/api/src/app/service.ts, via checkAppConfigTheme in theme.ts). Unknown colors are left out of what /api/config and shared links serve, and an override theme the client would reject falls back to the base theme. Covered by service.spec.ts and the Playwright scenarios override-theme-unknown-color-left-out and override-theme-invalid-keeps-base.
1 participant
Pull Request
Summary
Today a single color token that the reader does not know drops the whole theme. The yaml loader removes
interface.themeand the deployment falls back to the default look, and the client does the same for a deployment or stored theme. Unknown appearance tokens already only warn (#16373), so a typo in one color, or a theme written for a newer version that adds a color role, costs far more than an unknown appearance property does.Readers now ignore an unknown color token and keep the rest of the theme. The server warns with the token's path (
interface.theme.modes.light.colors.<token>: Unknown light color token ignored) and leaves the token out of the served theme, so a cached client older than the server never receives a name it could reject. The client applies the same rule to the deployment theme and to persisted themes. A value that is not an RGB triplet, or a key that is not a plain lowercase token, is still rejected with its path, and a theme naming only known tokens is served unchanged. This also makes future color roles forward compatible for older readers.Closes berry-13#198
Type of change
Testing
Tested environments/configuration:
interface.themeinlibrechat.yamlwith a misspelled color (surface-tertiary,rgb-surfce-secondary), plus a reload with a broken themergb-color, with a valid and an invalid valueAutomated tests:
packages/api/src/app/loader.spec.ts: an unknown color warns and is left out of the served theme instead of dropping itpackages/data-provider/src/config.spec.ts: the schema accepts an unknown token name and still rejects a non-token namepackages/client/src/theme/context/ThemeProvider.spec.tsxandregistry.spec.ts: stored themes with unknown colors apply; invalid values are rejectedyaml-theme-fallback.spec.tsandtheme-unknown-appearance.spec.tsScreenshots / recordings
No change to the default appearance. The only visible effect is that a theme with an unknown color now paints its known roles instead of falling back to the default, which the Playwright scenarios assert on computed styles.
Risk / compatibility
The color-name schema is looser: any plain lowercase token parses, and anything the server does not paint is dropped with a warning. A theme that previously failed to load because of one unknown color now loads without it. Nothing changes for valid themes or stored data.
Checklist