🥡 feat: Reload Custom Model Lists Across Replicas - #16471
lia-by-librechat[bot] wants to merge 3 commits into
Conversation
|
Please review exact head |
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
|
Review exact pushed head |
|
Please review exact pushed head |
|
@codex review the latest head |
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: ed83750f5d
ℹ️ 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 next = revision.data?.generation; | ||
| if (authenticated && next != null && next > 0) { | ||
| setTarget((current) => Math.max(current ?? 0, next)); |
There was a problem hiding this comment.
If the Redis generation key is lost or flushed, the Lua publisher restarts numbering at 1, and the backend tracker accepts that lower generation because it treats any unequal generation as a change. A browser that previously applied generation 5 will ignore the replacement generation 1 here due to Math.max, while applied.current also remains 5, so it never fetches the new catalog and stays stale until the counter exceeds 5 or the user session remounts. The revision protocol needs an epoch/opaque identifier or explicit reset handling rather than assuming the counter can never decrease.
Useful? React with 👍 / 👎.
| if (error instanceof ConfigReloadError) { | ||
| const validationErrors = error.validationErrors ?? []; | ||
| return res.status(400).json({ | ||
| error: | ||
| validationErrors.length > 0 | ||
| ? 'Custom config validation failed' | ||
| : 'Custom config source could not be loaded', | ||
| validationErrors, |
There was a problem hiding this comment.
When the reloaded YAML fails validation, this sends complete raw Zod issues to the client. Those issues can contain submitted source material beyond the path, such as an invalid enum's received value, unrecognized key names, and custom parameter error text; this can disclose deployment configuration or accidentally embedded credentials to a config manager even though other config reads redact secrets. Map these failures to approved stable codes and safe metadata before serializing them, and let the UI localize those codes.
AGENTS.md reference: AGENTS.md:L122-L130
Useful? React with 👍 / 👎.
| const modelConfig = await loadModels(req); | ||
| res.set(CONFIG_GENERATION_HEADER, getConfigGenerationForConfig(req.config)); | ||
| res.send(modelConfig); |
There was a problem hiding this comment.
Move the newly added generation lookup and response-header behavior into a TypeScript handler under packages/api, leaving this legacy CJS controller as wiring only. Calling the config-generation service and implementing the versioned response contract here extends backend behavior in /api, contrary to the repository's explicit module boundary and makes this reload protocol split across the legacy and typed layers.
AGENTS.md reference: AGENTS.md:L77-L81
Useful? React with 👍 / 👎.
| const { user } = useAuthContext(); | ||
| const { data: startupConfig } = useGetStartupConfig(); | ||
| const { data: configReloadAccess = false } = useConfigReloadAccessQuery(user?.id); |
There was a problem hiding this comment.
This query is enabled for every authenticated user who opens Settings, but its endpoint is mounted behind requireAdminAccess, whose normal denial path records a warning and performs a capability lookup. Consequently every non-admin settings visit generates an expected 403, misleading forbidden-access log traffic, and avoidable database work; because rejected query data is then treated as false, this happens again whenever the short-lived query is recreated or refetched. Use an authenticated, non-warning boolean capability probe or gate this request on an existing admin-access signal.
Useful? React with 👍 / 👎.
| async function getConfigRefreshStatus() { | ||
| const base = await ensureBaseConfig(); | ||
| const generation = getConfigGenerationForConfig(base); | ||
| return { | ||
| distributed: syncConfigGeneration != null, | ||
| generation: generation === '' ? null : Number(generation), | ||
| pollIntervalMs: | ||
| base.config?.configReload?.clientPollIntervalMs ?? DEFAULT_CONFIG_RELOAD_CLIENT_POLL_MS, |
There was a problem hiding this comment.
When Redis is configured but configReload.clusterReady is absent or false, the reload endpoint rejects every clustered reload, yet this status still reports distributed: true. The client therefore polls /api/config/revision every three seconds for every visible authenticated session even though the feature is disabled by its default, adding potentially substantial request and Redis-read load to existing deployments. Gate distributed or expose a separate polling-enabled flag based on clusterReady so the new toggle's disabled default preserves prior behavior.
AGENTS.md reference: AGENTS.md:L90-L92
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Changing the default model list for an existing custom endpoint in
librechat.yamlcurrently requires restarting every API replica. This PR validates a local or HTTP(S)CONFIG_PATHon an explicit admin reload, installs only existing custom endpoints'models.defaultchanges, and reports every other YAML edit as requiring a restart. A new endpoint, changed credentials, memory policy, MCP settings, tool filters, and startup configuration never take effect through this reload.The control appears in General settings only for a user with platform-scoped
MANAGE_CONFIGSandACCESS_ADMIN. It checks access when settings opens, not during every authenticated startup-config request. The POST independently enforces both grants. A model picker already open in another browser observes the applied replica generation and refreshes its models only after a serving replica proves it has that version.Supersedes the closed broad-scope PR #16385 and replaces the canary-targeted PR #16470 with a clean
dev-based change. Builds on last-good reload behavior merged in #16383. This PR targetsdevso it can be merged before validating the live deployment on canary.How it works
A slower admin reload cannot overwrite a newer Redis generation. A replica whose source has not caught up retains its last good model list and retries at a configurable rate; publication is not a claim that all replicas have applied it. Only the process-local
APP_CONFIGcache stores the accepted base. Redis is optional: without it, the operation and report are explicitly local-only. The initial Redis read is bounded; a server can start during a Redis outage and continue serving its last good config.Rollout:
configReload.clusterReadydefaults tofalse. After all replicas on canary run this version, enable it in the source configuration and restart them before invoking reload. The new Redis key is versioned and isolated from the previous broad reload protocol. Do not perform clustered reloads during a mixed-version rollout.configReload.clientPollIntervalMsandconfigReload.mismatchRetryMsdefault to 3000 ms and 5000 ms respectively; remote fetches retain the previous 10-second timeout unless configured.Type of change
Testing
tsc --noEmitinpackages/data-provider,packages/api, andclient; builds ofpackages/data-provider,packages/data-schemas,packages/api, andpackages/client; stagednpm run static-checks(all affected gates passed).@csstools/postcss-initialdependency in this worker; Chrome/Playwright is also not installed here. Full browser suites and canary deployment validation remain unrun. Lighthouse CI passed on this head; a full browser model-picker test remains unrun.Risk / compatibility
Only default model lists for existing, uniquely named custom endpoints can change live. Other edits remain at each replica's startup value, including values used by the expired-file sweep, GitHub skill synchronization, MCP recovery, and global static tool catalog. Source-validation errors leave the last good base intact. Redis publication uses a single-key Lua compare-and-publish, and a published generation never exposes YAML or configuration secrets. MongoDB config override priority remains unchanged. An unavailable Redis store permits a local-only reload and reports propagation failure without blocking normal cached requests.
Checklist