LibreChat sends `scope` on the refresh_token grant by default (PR #7924) because some authorization servers expect it. Salesforce rejects any scope on refresh with HTTP 400 "scope parameter not supported" (confirmed in production logs for case 00046259), which broke token refresh and forced re-authentication — amplifying the multi-replica PKCE retry storm.
RFC 6749 §6 makes scope optional on refresh (the server reuses the original grant). postRefreshRequest now sends scope as before and retries once WITHOUT it only when the failure is specifically a scope-parameter rejection (isScopeParameterRejection), so servers that need scope are unaffected and Salesforce-like servers self-heal with no operator config.
* ♻️ fix: Reap Stale In-Memory Generation Jobs to Prevent Heap OOM
InMemoryJobStore only reaped terminal jobs, so a generation that hung
without reaching completeJob() stayed "running" forever, retaining its
full message context. Abandoned jobs accumulated until the V8 heap was
exhausted (#13391). RedisJobStore already guards this with a 20-minute
running-job TTL; the in-memory store had no equivalent failsafe.
- Add a configurable staleJobTimeout (default 20m) to InMemoryJobStore;
cleanup() now reaps running jobs older than the timeout.
- Abort a pending generation in GenerationJobManager.cleanup() when its
job has been reaped, releasing client/graph references for GC.
- Abort the previous generation in createJob() when a job is replaced
for the same stream, closing an untracked-orphan leak.
- Forward staleJobTimeout through createStreamServices.
* 🩹 fix: Remove same-stream replacement abort (codex P1)
The createJob replacement-abort could let a stale, replaced request take
the abort-during-initialization path and complete/error the replacement
job via the shared streamId, and was a no-op across Redis replicas.
Removed it; the reported OOM is handled by the running-job failsafe and
the orphan-loop abort, which only fires when no job holds the streamId.
* 🩹 fix: Reap stale jobs on inactivity rather than age (codex P2)
Age-based reaping would drop a legitimately long but actively-streaming
generation at the timeout. Track a last-activity timestamp (refreshed on
each emitted chunk via recordActivity) and reap on inactivity instead,
mirroring RedisJobStore refreshing the running TTL on each appendChunk.
* 🩹 fix: Notify reaped streams and reset activity on replacement (codex P2)
- Emit a terminal error to any client still attached when a stale job is
reaped, so the SSE connection closes instead of hanging open with no
final/error event.
- Clear lastActivity in createJob so a replacement reusing the same
streamId falls back to its fresh createdAt and isn't reaped immediately
on the previous generation's stale activity timestamp.
* 📤 feat: Model-Aware Max Output Tokens for Google/Gemini
Resolves#13384.
Current Gemini text models (2.5 and 3+, including Gemini 3.5 Flash)
support 64K output tokens, but LibreChat defaulted every Google model
to the legacy 8K value — most visibly in the Agents model-parameter
panel.
- Add model-aware `reset`/`set` to `googleSettings.maxOutputTokens`,
mirroring the Anthropic pattern: Gemini 2.5/3+ -> 65536, legacy
(2.0 and earlier) and Gemma -> 8192.
- Resolve the default server-side in `getGoogleConfig` and in the
Agents, preset, and standard Google settings panels via a shared
`applyModelAwareDefaults` helper.
- Make `compactGoogleSchema` and `generateGoogleSchema` model-aware so
explicit user values are preserved and not overwritten.
* 🛡️ fix: Cap Google max output at Vertex-safe limits
Addresses Codex review (P1) on #13390. Vertex AI caps current Gemini
text models at 65,535 output tokens (vs 65,536 on AI Studio) and image
models at 32,768, so an unconditional 65,536 default could make
otherwise-default Vertex requests fail validation.
- Lower the modern text default/ceiling to 65535 (valid on both Vertex
and AI Studio).
- Resolve Gemini image models (e.g. gemini-2.5-flash-image) to 32768.
- Add reset/set + getGoogleConfig tests for image models and the Vertex
default path.
* 🧮 fix: Respect configured Google defaults and legacy image caps
Addresses Codex review round 2 on #13390 (one P2, two P3).
- P2 (llm.ts): apply the model-aware maxOutputTokens default as the final
fallback instead of pre-filling it, so an explicit value, `defaultParams`,
and `addParams` all take precedence and `dropParams` is honored. Empty-string
values stay stripped (preserves prior Gemini empty-payload handling).
- P3 (panels): pass the resolved params endpoint (`overriddenEndpointKey`) to
`applyModelAwareDefaults`, so custom endpoints with
`defaultParamsEndpoint: 'google'` also surface the model-aware default.
- P3 (schemas): nest the image-model check inside the 2.5+/3+ version check, so
legacy image IDs (e.g. gemini-2.0-flash-preview-image-generation) keep the 8K
cap instead of being treated as 32K models.
- Add tests for defaultParams precedence, dropParams, legacy image models, and
the Vertex default path.
* 🧭 fix: Base Google defaults on final model and configured overrides
Addresses Codex review round 3 on #13390 (two P2).
- llm.ts: resolve the model-aware maxOutputTokens default from the final
`llmConfig.model` (after defaultParams/addParams) instead of the model
captured from modelOptions, so a model forced via addParams/paramDefinitions
on a Google-compatible custom endpoint gets its correct limit.
- Panels: apply model-aware defaults to the built-in settings first, then
overlay `customParams.paramDefinitions`, so an admin-configured
maxOutputTokens default wins in the UI (consistent with backend precedence).
- Add parameterSettings.spec for applyModelAwareDefaults (incl. override
precedence) and a getGoogleConfig final-model test.
* 🧠 fix: Replay DeepSeek `reasoning_content` via OpenRouter
DeepSeek's thinking-mode API rejects multi-turn tool-calling requests
unless `reasoning_content` from each tool-bearing assistant message is
replayed verbatim, returning HTTP 400 "The `reasoning_content` in the
thinking mode must be passed back to the API." The agents SDK already
handles this for direct `Providers.DEEPSEEK`, but DeepSeek models routed
via OpenRouter use `Providers.OPENROUTER` — `formatAgentMessages` skipped
the reasoning-preservation branch, and `ChatOpenRouter` left
`includeReasoningContent` unset, so the field silently dropped on every
subsequent turn.
Add `isDeepSeekReasoningProvider(provider, model)` and use it in two
places: (1) `getOpenAILLMConfig` flips `includeReasoningContent: true`
when OpenRouter is dispatching a `deepseek/*` model so the LangChain
client emits the field on assistant turns that have non-empty
`additional_kwargs.reasoning_content`, and (2) `AgentClient` spoofs the
provider hint to `Providers.DEEPSEEK` when calling
`formatAgentMessages`, triggering the SDK's existing
`preserveReasoningContent` path that re-attaches the field to
reconstructed tool-bearing AIMessages. The downstream
`_convertMessagesToOpenAIParams` is already gated on non-empty
`reasoning_content`, so the flag is a no-op outside thinking mode.
Resolves#13366.
* fix: Harden DeepSeek detection against OpenRouter routing edges
Address three Codex review findings on #13368:
1. Strip OpenRouter's `~` latest-routing prefix before applying the
DeepSeek model regex. `~deepseek-chat` and `~deepseek/r1` were
previously left unmatched because the regex's start/`/` boundary
only saw the `~`. Mirror the SDK's `normalizeOpenRouterModel()`
here and in `getOpenAILLMConfig`.
2. Add a custom-endpoint fallback: when the model id carries the
unambiguous `deepseek/...` OpenRouter namespace, accept it
regardless of the resolved provider. Covers the case where a user
configures OpenRouter under a non-standard endpoint name and
`initializeAgent` normalizes the unknown provider to `openai`,
stranding the spoof. Bare `deepseek-*` ids still require an
explicit DeepSeek/OpenRouter provider so unrelated endpoints
labelling a model `deepseek-r1` don't trigger.
3. Inspect every agent in `this.agentConfigs` when deciding whether
to spoof the format provider. Multi-agent handoff runs feed all
agents' messages through one `formatAgentMessages` call, so a
DeepSeek handoff under a non-DeepSeek primary previously lost its
persisted reasoning_content too.
Also addresses Copilot's review note: only pass the options object
to `formatAgentMessages` when the DeepSeek spoof is actually needed,
preserving the pre-fix behavior for everyone else.
* fix: Extend DeepSeek reasoning_content fix to OpenAI-compat agent paths
Address two more Codex P2 findings on #13368:
1. `getOpenAILLMConfig` no longer gates `includeReasoningContent` on
`useOpenRouter`. Any DeepSeek-style model id (with `~` latest-routing
prefix stripped) is sufficient. This re-aligns the LLM gate with
`AgentClient`'s formatter spoof, which already treats a `deepseek/*`
id as authoritative — so a custom-named OpenRouter endpoint or a
DeepSeek-compatible proxy gets the field both attached to history AND
serialized to the wire. Direct `ChatDeepSeek` ignores the flag (its
own conversion path hardcodes `includeReasoningContent: true`), so
this is a harmless no-op there.
2. Thread the same `Providers.DEEPSEEK` formatter hint through
`api/server/controllers/agents/openai.js` and `responses.js` (the
OpenAI-/Responses-compatible serving paths). Without it those paths
restored `additional_kwargs.reasoning_content` only in `AgentClient`
while the LLM config flipped `includeReasoningContent` on for them
too — so DeepSeek tool turns served from those endpoints would still
ship requests with the flag set but no field present, hitting the
same second-turn 400. The `needsDeepSeekFormatHint` helper in
`openai.js` mirrors `AgentClient`'s per-agent check.
* fix: Tighten DeepSeek detection and cover handoff sub-agents
Address four more Codex P2 findings on #13368:
- Tighten the DeepSeek model regex to `^deepseek(?:[-/]|$)/i` (anchored
to start). Rejects cloned/distilled slugs like
`mistral/deepseek-distilled-foo` and `community/deepseek-r1` that
previously matched via the `(?:^|/)` alternation, which could attach
the DeepSeek-only `reasoning_content` field on proxies that don't
accept it.
- Anchoring also collapses the namespace-only fallback into the same
pattern, so bare `deepseek-chat` / `deepseek-reasoner` on a
custom OpenAI-compatible DeepSeek proxy are now recognized — fixing
the asymmetry where `getOpenAILLMConfig` would flip
`includeReasoningContent` for those bare ids but `AgentClient`
wouldn't pass the formatter hint.
- Extend `needsDeepSeekFormatHint` in `openai.js` (and the inline
check in `responses.js`) to walk `handoffAgentConfigs` too. In
multi-agent runs where the primary isn't DeepSeek but a connected
handoff agent is, the SDK's `formatAgentMessages` previously dropped
the handoff's persisted reasoning_content before the next tool turn,
preserving the 400 the PR was meant to prevent.
- Mirror the regex change in `getOpenAILLMConfig`.
Out of scope: the OpenAI-compatible serving paths still don't
preserve incoming `reasoning_content`/`reasoning` fields in
`convertMessages`, nor does the Responses API persist reasoning in
`saveResponseOutput`. Those are deeper persistence/conversion fixes
worth a separate PR.
* test: Allow includeReasoningContent for Azure-serverless DeepSeek
CI surfaced a backward-compat expectation that snapshotted the
pre-fix behavior. Azure-serverless DeepSeek deployments (e.g.
`DeepSeek-R1`) forward to the same DeepSeek thinking-mode tool-call
contract, so the LLM gate now correctly flips
`includeReasoningContent: true` for them too. The downstream
gate on a non-empty `additional_kwargs.reasoning_content` keeps
this a no-op outside thinking mode.
* chore: Trim noisy comments
Per CLAUDE.md ("self-documenting code; no inline comments narrating
what code does"), strip the multi-paragraph rationale that crept into
the DeepSeek reasoning_content fix. The commit history and PR
description carry the why; the code says the what.
Keeps one single-line JSDoc on `isDeepSeekReasoningProvider` (linking
to the DeepSeek docs) and a `(#13366)` tag on each opt-in site so
future readers can find the context.
* revert: Drop non-functional DeepSeek hint from OpenAI-compat serving paths
Codex's later review passes correctly flagged that threading the
DeepSeek formatter hint through openai.js (`/v1/chat/completions`) and
responses.js (`/v1/responses`) doesn't actually fix the second-turn
400 in those paths. Empirical check against the real SDK confirmed the
gap is deeper and pre-existing:
formatAgentMessages(payload, ..., { provider: DEEPSEEK })
where payload is the `convertMessages`/`convertInputToMessages` output
shape (string content + TOP-LEVEL `tool_calls`) produces NO tool-bearing
AIMessage at all — `formatAssistantMessage` only reconstructs tool calls
from `tool_call`-typed *content parts*, never a top-level `tool_calls`
field. So those serving paths don't reconstruct tool-call history (let
alone reasoning) regardless of the hint. The Responses persistence layer
likewise stores only output text, not tool calls or reasoning.
Making those paths work requires reworking the wire->internal message
conversion (and Responses persistence) to emit content-part arrays — a
broad, pre-existing concern beyond this issue and risky to land here.
Rather than ship a hint that looks like a fix but is inert, revert the
serving-path changes and scope this PR to the validated AgentClient
chat path (the actual surface in #13366).
Reverts the openai.js/responses.js threading and their spec mocks to
main. Keeps the AgentClient fix, `isDeepSeekReasoningProvider`, the
`getOpenAILLMConfig` flag, and the type.
* 🛡️ fix: Cap Default Limit on Agent List Queries (#13363)
`GET /api/agents` accepted unbounded requests: when the client omitted
`limit`, the value flowed straight into `getListAgentsByAccess`, which
set `isPaginated = false` and issued an uncapped MongoDB query. Combined
with the unindexed `findPubliclyAccessibleResources` AclEntry scan run
on every request, this produced 10-19s response times and stalled the
connection pool on instances with 100+ agents.
- Default `limit` to 100 in the route handler so client requests without
`?limit=` paginate by default.
- Default `limit` to 100 in `getListAgentsByAccess` itself as
defense-in-depth. The function already caps numeric limits at 100, so
there is no client-facing change.
- Pass `limit: null` explicitly in the actions route, which legitimately
needs the full editable-agent set, to preserve its existing behavior.
- Add regression tests covering the default cap and the explicit
unbounded opt-out.
* 🛡️ fix: Avoid agent-list regression for users with 100+ agents
Codex review pointed out that capping `getListAgentsByAccess` at 100
silently truncated agents past the first page for the four consumers
(`useAgentsMap`, `AgentSelect`, `ModelSelectorContext`, `useMentions`)
that read `res.data` without following `has_more`/`after`.
- Raise the function's hard cap from 100 to 1000 to match
`MAX_AVATAR_REFRESH_AGENTS`, the realistic upper bound the
avatar-refresh path already assumes. (Side effect: the avatar refresh
call site was silently being capped at 100 by the old normalize step.)
- In `useListAgentsQuery`, merge `limit: 1000` into params so the four
consumers above get the user's full accessible set in a single
round-trip instead of needing cursor pagination.
- Route handler default stays at 100 as defense-in-depth for any other
caller that omits `limit`.
- Add a regression test asserting an explicit `limit` above 100 now
returns the full set instead of being clipped.
* 🪢 fix: Keep agent-list cache key stable for mutations
Codex P2 review noted that folding `limit: 1000` into the cache key
broke `allAgentViewAndEditQueryKeys` in `Agents/mutations.ts`, which
references `[QueryKeys.agents, { requiredPermission }]` directly across
eight mutation handlers. After my prior change the cached entry lived
under `[QueryKeys.agents, { limit: 1000, requiredPermission }]`, so
create/update/delete/avatar/action mutations stopped updating the list
the four consumer hooks render — and with `refetchOnMount` and focus/
reconnect refetches disabled, the UI would stay stale until something
else triggered a fetch.
Split the merged limit out of the cache key: the request to
`dataService.listAgents` still uses `requestParams` (with the default
limit applied), but the React Query cache key uses the caller's `params`
as-is. The mutation cache updates land again, and the request still
returns the user's full accessible set in one round-trip.
* 🛡️ fix: Index AclEntry and paginate agent list internally (#13363)
Completes the perf fix for #13363 properly — resolves both the
unbounded ACL scans Copilot flagged and Codex's tension between "show
all agents" and "don't bypass the server cap".
Backend:
- Add a compound index on `{ principalType, resourceType, permBits,
resourceId }` to the AclEntry schema. This is the index missing for
`findPublicResourceIds` and the public branch of the `$or` in
`findAccessibleResources`, both of which previously fell back to a
collection scan on every `GET /api/agents`. Adds an `explain`-based
regression test asserting the public query no longer COLLSCANs.
Client:
- Rewrite `useListAgentsQuery` to follow the server's cursor
pagination internally and concatenate every page into a single flat
`AgentListResponse`. Consumers (`useAgentsMap`, `AgentSelect`,
`ModelSelectorContext`, `useMentions`) get the user's complete
accessible-agent set without any of them needing to learn about
cursors, and each individual request uses the server's default
page size (so the route's 100-default defense-in-depth fires for
real). Cache key shape is unchanged, so the eight mutation handlers
in `Agents/mutations.ts` keep matching `allAgentViewAndEditQueryKeys`
and update the cached list as before.
- Drop the `FULL_AGENT_LIST_LIMIT = 1000` injection added in the
previous commit — no longer needed once pagination handles the full
set, and removing it stops bypassing the route default.
* 🧹 fix: CI fallout from C-done-properly refactor
- Collapse multi-line `fetchAllAgentPages` signature in queries.ts so
prettier stops complaining.
- In the new public-principal index test, grant one ACL entry before
calling `.explain()` so the collection exists (otherwise mongo returns
`nonExistentNamespace` and there is no winning plan to inspect).
- Cast the `.explain('queryPlanner')` result to a typed shape — the
mongoose return type doesn't expose `queryPlanner` directly and was
failing the TypeScript check.
* 🧪 fix: Test the AclEntry public-principal index via hint, not planner choice
The previous test asserted the query planner did not pick COLLSCAN for
the public-principal lookup. That assertion fails on small collections
(under the planner's collection-size heuristic) — the index exists and
is usable, but with a single document in the test the planner correctly
chooses COLLSCAN as the cheaper plan.
Reshape the assertion:
1. Confirm the new compound index is actually declared by inspecting
`collection.indexes()` after `syncIndexes()`.
2. Force the planner to that index via `.hint()` and assert the winning
plan is `IXSCAN` — proves the index is real and serves this query
shape, without depending on collection-size heuristics.
* 🧹 chore: Slim down verbose comments
The JSDoc and inline comments added across the perf fix had drifted
into multi-paragraph rationale better suited to the PR description than
the source. Collapse to single-line JSDoc that just describes what each
piece does; drop the inline comment in `actions.js` entirely — the call
is self-evident.
* feat: add Claude Opus 4.8 support
* fix: omit sampling params for Claude Opus 4.8
* fix: flatten Bedrock beta header merge
* fix: strip Bedrock sampling params for Opus 4.8
The `Tests: @librechat/api` job in `backend-review.yml` had
`timeout-minutes: 10`. Recent runs have started getting cancelled
right at the 10-minute mark with tests still actively passing — the
log streams `PASS …` lines through the final second before
`The operation was canceled.` fires.
Looking at recent runs in the wild:
- Warm `ubuntu-latest` runners: suite finishes in ~4–5 min.
- Cold/busy runners: 10+ min. No headroom = job killed mid-suite.
The suite has grown over the last few months (OpenTelemetry, MCP
OAuth, Bedrock, the Operational Prometheus Metrics work from #13265,
plus the agents/responses harness). 10-minute ceiling that worked
before is now a tail-latency cliff.
Bump to 20 minutes. Still well below the runner's hard 6-hour cap,
generous enough to absorb runner variance, and a stuck/genuine hang
will still get killed eventually rather than billing forever.
No code changes; one-line config bump.
In SidePanel/MCPBuilder/MCPServerCard the server title was rendered in
a span carrying the Tailwind truncate utility. overflow: hidden does
not apply to inline boxes, so long server names overflowed their slot
and ran underneath the Edit, Reconnect, and Revoke icons.
The title now renders in a div so truncate actually clips with an
ellipsis. No other styling changes.
* chore: Update @librechat/agents to version 3.1.93 and @langfuse packages to version 5.3.0 in package-lock.json and package.json files
* chore: Update browserify-sign to version 4.2.6 and qs to version 6.15.2 in package-lock.json
* fix(redis): add REDIS_CLUSTER_SAFE_DELETE for ElastiCache Serverless CROSSSLOT errors
ElastiCache Serverless and similar managed Redis services present a single-node
connection endpoint but shard keys internally. When USE_REDIS_CLUSTER=false (as
required for single-endpoint services), batchDeleteKeys() uses multi-key DEL
commands that fail with CROSSSLOT errors because the managed cluster rejects
cross-slot operations.
Adds REDIS_CLUSTER_SAFE_DELETE=true which forces per-key deletion (the same
cluster-safe path) without changing the connection mode. This makes the delete
strategy independent of the connection topology.
Closes#13261
* test(cache): add REDIS_CLUSTER_SAFE_DELETE config tests
* fix: Avoid nested Redis delete mode ternary
* docs: Add Redis cluster-safe delete env example
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
Adds a `Run Prettier --check on changed files` step to the existing
`eslint-ci.yml` workflow. Same path filter (api/**, client/**,
packages/**), same changed-files detection, runs after the ESLint step.
## Why
Today there is no `prettier --check` in CI — only the local
`lint-staged` pre-commit hook runs `prettier --write`. When a PR is
merged with the hook bypassed (e.g. GitHub UI edit-and-merge, or
`git commit --no-verify`), a file can land in a non-prettier-canonical
state and nobody notices. The next contributor who stages an unrelated
change in that file then ends up with a "drive-by" prettier diff in
their PR.
`packages/api/src` had 14 such files accumulated; #13281 fixes the
existing drift. This PR closes the gap so it doesn't regrow.
## What the step does
- Detects changed JS/TS files under `api/**`, `client/**`, or
`packages/**` against the PR base.
- Runs `npx prettier --check $CHANGED_FILES`.
- On failure, prints a `::error::` annotation telling the contributor
how to fix it locally (`npx prettier --write <files>`).
Same one-step shape as the existing ESLint check — no extra workflow
file, no extra `npm ci`, no extra checkout.
## Ordering note
#13281 (`chore: prettier --write packages/api/src`) should land first
so the existing drift is cleared. After both PRs merge, the
pre-commit hook + this CI check together prevent drift from
re-accumulating.
## Test plan
- [x] `npx js-yaml .github/workflows/eslint-ci.yml` validates.
- [ ] CI green on this PR itself (touches only `.github/workflows/`,
which the path filter includes, so the workflow runs on itself).
- [ ] After merge: a synthetic PR introducing prettier drift should
fail the new step with the diagnostic message.
Run `prettier --write` over the source trees of every workspace to align
with the repo's own `.prettierrc` (`printWidth: 100`, `singleQuote: true`,
`trailingComma: 'all'`, etc.). **19 files reformatted total** — purely
whitespace and line-wrap changes, no functional edits and no API changes.
Scope:
- `packages/api/src/**/*.{ts,tsx}` — 14 files
- `packages/client/src/**/*.{ts,tsx}` — 1 file
- `packages/data-schemas/src/**/*.{ts,tsx}` — 4 files
- `api/**`, `client/**`, `packages/data-provider/**` — already prettier-clean
Most of the drift is in argument-list / type-annotation wrapping where
the formatted form fits within `printWidth` but the current source keeps
a hand-wrapped multi-line shape. Example:
// before
function countWebSearchDefinitions(
toolDefinitions: Array<{ name: string }> | undefined,
): number { … }
// after (still well under 100 cols)
function countWebSearchDefinitions(toolDefinitions: Array<{ name: string }> | undefined): number { … }
`npx prettier --check` across all workspaces is now clean. The local
pre-commit hook (`lint-staged` → `prettier --write`) would have produced
the same result on any future edit to these files.
There are no prettier-checking workflows in CI today, so drift like this
can re-appear if PRs are merged with the hook bypassed. Companion PR
#13282 adds a `prettier --check` step to `eslint-ci.yml` so future
drift gets caught.
* 🪟 fix: Apply Admin-Panel Config Overrides To YAML-Defined MCP Servers
Admin-panel saves of MCP server fields for YAML-defined servers were
silently dropped by the registry. ensureConfigServers filtered out any
merged config entry whose name appeared in YAML, so overrides such as
iconPath, title, and description never reached getAllServerConfigs even
though the override row had been written to the configs collection and
the AppConfig merge layer had produced the correct merged result.
The filter is removed and replaced with a content-equivalence
short-circuit in ensureSingleConfigServer. YAML-defined servers whose
merged config matches the YAML cache entry skip lazy-init, so unmodified
YAML servers still avoid a redundant inspection round trip. The new
private helper matchesYamlConfig reuses the existing content-hash
function on configurable fields only.
getAllServerConfigs now overlays config-tier entries onto the YAML base
while preserving user-DB entries (source: 'user'), giving precedence of
YAML, then Config tier, then User DB. The docstring is updated to
describe the new order.
Multi-tenancy is already enforced upstream of the registry by the
AppConfig layer, so the registry stays tenant-agnostic and overrides
remain isolated per tenant.
Tests cover the new behavior: config-tier override on YAML-defined
server flows through to getAllServerConfigs, YAML servers without
effective overrides skip lazy-init, user-DB entries win over
config-tier overlays, pure config-tier servers still lazy-init, and the
merged config passed to lazy-init preserves all YAML fields when the
override only adds new ones.
* 🛠️ fix: Address Review Feedback For YAML Override Precedence
Both Copilot and Codex flagged matchesYamlConfig as broken in
production: the cached YAML config carries inspector-derived defaults
(requiresOAuth defaulted to false when YAML omits it, serverInstructions
rewritten from the YAML toggle to the fetched server-instructions
string, and so on) that are absent from appConfig.mcpConfig. The
content-hash comparison reports a mismatch for every YAML server with
no admin-panel override, so ensureSingleConfigServer re-inspects all of
them anyway. The optimization never fires in practice and reintroduces
the source tagging it was supposed to avoid.
Remove the short-circuit and the matchesYamlConfig helper. YAML-defined
servers that appear in the merged config go through lazy-init like any
other entry. A smarter optimization that overlays cosmetic-only
override fields onto the YAML cache without re-inspection belongs in a
follow-up once the boundary between configurable fields and
inspector-derived fields is well defined.
Update the existing ensureConfigServers tests that asserted the old
filter behavior (should exclude YAML servers from config-source
detection, should return empty when all servers are YAML) to assert
the new behavior: YAML servers pass through ensureConfigServers and
are lazy-initialized when they appear in the merged config. Make the
inspector mock more realistic by spreading the raw input first and
overlaying only runtime fields, so the test fixtures match production
where the inspector preserves configurable fields. Drop the companion
test in MCPServersRegistry.test.ts that asserted the short-circuit
fires for unchanged YAML servers; the hand-crafted fixture skipped
inspector defaults and was not representative.
Copilot also flagged that getServerConfig short-circuits to
configServers before checking the user DB, so the precedence enforced
in getAllServerConfigs (user-DB beats config-tier) was bypassed in the
single-server lookup. When configServers carries an entry and a userId
is available, check the user DB first and prefer a source: 'user'
entry so per-user servers are never shadowed by an admin-panel
override.
* 🛡️ fix: Harden Admin Override Overlay Against Failure Stubs
Hardens the admin-panel override path against transient inspect failures
and removes a defensive branch that guarded an impossible state.
The getAllServerConfigs overlay now skips failed-inspection stubs so a
healthy YAML or DB entry stays visible during the 5-minute retry window
instead of being clobbered by a stub. When the overlay does land, the
base entry's source tier is preserved, which keeps Tools/mcp.js routing
its failed-inspection recovery to the correct storage location.
The user-DB precedence block in getServerConfig is removed: configServers
is built from appConfig.mcpConfig which only ever carries admin-tier
entries, so the DB lookup defended a state that cannot occur via the
current call graph. The dead yamlServerNames memoization is also gone.
Adds two regression tests covering inspection-failure preservation and
source-tier preservation on successful overlay, and adds a debug log
when an admin override is suppressed by a user-tier entry. The
makeParsedConfig test factory now honors overrides correctly.
* 🧪 test: Strengthen Admin Override Coverage And Docs
Adds an end-to-end regression test that chains MCPServerInspector.inspect
failure through ensureConfigServers and getAllServerConfigs, asserting
the healthy YAML base entry survives a transient inspect failure
intact. The previous regression test hand-built a failure stub and
skipped ensureSingleConfigServer, leaving the production chain itself
untested.
The getAllServerConfigs docstring now spells out both overlay guards
(failed-stub skip and user-tier preservation) and the source-field
preservation contract that downstream recovery logic depends on.
The yamlLangfuseConfig test fixture is frozen so a future test cannot
mutate it and contaminate sibling tests in the describe block.
* 🔧 fix: Skip Lazy-Init For Unchanged YAML MCP Servers
Adds an admin-configurable-field equivalence check so YAML-defined MCP
servers that carry no admin override skip lazy-init in
ensureConfigServers. This avoids the per-request inspect storm and
keeps unmodified YAML servers out of the config-tier cache, so admin
saves that touch unrelated overrides no longer evict and tear down
those YAML connections.
A second guard in getServerConfig prevents failed-inspection stubs in
configServers from shadowing the healthy YAML base entry for the
duration of the retry window. The aggregate path already had this
guard via getAllServerConfigs; this brings the single-server path to
parity, so Tools/mcp.js recovery routes to YAML reinspection rather
than bailing on the config retry timer.
Adds three regression tests covering the unmodified-YAML skip, the
admin-override lazy-init trigger, and the failure-stub fallthrough.
Updates two existing ensureConfigServers tests that previously
documented the now-incorrect "always lazy-init YAML" behavior.
* 🔓 feat: Expose baseOnly Flag On Admin Config Base Endpoint
The admin getBaseConfig handler now reads req.query.baseOnly and
forwards it to getAppConfig so an admin panel client can request the
un-merged YAML and AppService base configuration without DB overrides
applied. The flag is opt-in; existing callers see no behaviour change
because the default remains the merged response.
The query value is coerced through String() so Express array forms
like baseOnly=true&baseOnly=true are treated as false rather than
truthy by accident. A handler test pins the forwarding behaviour and
the default-merged behaviour against future regressions.
* 🧹 fix: Address Codex Review Findings On MCP Registry Precedence Path
Four follow-ups from the Codex review of PR #13173:
P1. getServerConfig now preserves the configServers candidate as a
last-resort fallback when both YAML cache and user DB return nothing,
so admin-defined config-only servers carrying inspectionFailed=true
still surface the failure stub to callers in api/server/services/Tools/mcp.js
that rely on it to return the still-unreachable message. The
not-found memoization is preserved.
P2a. proxy is added to ADMIN_CONFIGURABLE_FIELDS so an admin override
on SSE/streamable-http proxy is no longer treated as an unchanged YAML
server and correctly triggers lazy-init.
P2b. isUnmodifiedYamlServer now treats absent-on-rawConfig fields as
equal, so inspector-derived values on the cached YAML entry
(notably requiresOAuth filled in by detectOAuth at startup) do not
force unmodified YAML servers to re-init on every request.
P3. getBaseConfig parses ?baseOnly strictly against the literal string
true instead of String-coercing, so array shapes like baseOnly[]=true
no longer pass through.
Regression tests cover all four paths.
* 🧹 fix: Drop Misleading Shadow Warning On Config Vs User-DB Collisions
The Config-tier branch of warnOnOperatorManagedNameCollisions logged
that Config MCP servers shadow DB-backed servers, but
getAllServerConfigs actually preserves the user-tier entry on a
Config-vs-user collision and skips the override. The warning was
describing the opposite of what the code does and would mislead
operational debugging.
The YAML-tier call is unchanged because YAML still legitimately
shadows DB-backed servers. The per-entry debug log inside the
collision branch already captures the actual outcome.
Test renamed and rewritten to assert the user-tier entry is
preserved and no shadow warning is emitted.
* 🔒 fix: Keep Tenant-Scoped configServers Candidate Out Of The Global Read-Through Cache
The prior fix for surfacing inspectionFailed stubs from admin-defined
config-only servers wrote the per-call configServers candidate into
readThroughCache when YAML and DB both missed. The cache key is keyed
by serverName plus userId, so a failed stub from one tenant could
satisfy a later no-userId lookup made by another tenant before any
configServers resolution ran.
getServerConfig now caches only the global YAML/DB resolution (still
caching undefined to memoize not-found lookups) and uses the candidate
strictly as an unmemoized function-level fallback that surfaces the
failure stub to the caller without leaking it across tenants.
Regression test exercises a no-userId call after a tenant-scoped
failure and asserts the cache returns undefined rather than the
stub, and that a second tenant sees their own healthy candidate.
* 🔄 fix: Mirror getAllServerConfigs Precedence Exactly In getServerConfig
getServerConfig was short-circuiting with the configServers candidate
on every healthy lookup, which made single-server callers diverge from
the aggregate path for name collisions between config-tier overrides
and user-DB entries. The aggregate path preserves the user-tier entry
on such collisions, so single-server callers saw the admin override
while list views saw the user server for the same name.
getServerConfig now resolves the YAML/DB base first and applies the
same four-step precedence used in getAllServerConfigs:
1. user-tier base wins absolutely over a config-tier candidate
2. healthy YAML/DB base wins over a failed (inspectionFailed)
candidate
3. healthy candidate overlays its fields onto the base, preserving
the base entry's source tag so downstream recovery routes to the
correct storage location
4. with no base, the candidate is returned as-is for config-only
servers
readThroughCache still memoizes only the global YAML/DB lookup, so
the per-call configServers candidate never enters the cache and the
tenant-isolation guarantee from the previous fix is preserved.
Regression tests cover the user-wins-over-config case and the
YAML-overlay-with-yaml-source-preserved case.
* ⚡ perf: Batch YAML Cache Read In ensureConfigServers
isUnmodifiedYamlServer was calling cacheConfigsRepo.get(serverName)
per entry. In the Redis aggregate-key backend, get() is implemented
as getAll() then map lookup, so N concurrent per-server lookups
inflate into N full-map reads and deserializations on every
ensureConfigServers pass.
The loop now takes a single getAll() snapshot at the top and hands
it into a synchronous isUnmodifiedYamlServer helper, turning O(n)
remote reads into O(1) regardless of how many MCP entries are
resolved. The snapshot also gives the unchanged-YAML comparison
one consistent view of YAML across all entries.
Regression test spies on cacheConfigsRepo.get and asserts it is
never called from ensureConfigServers, with getAll called exactly
once.
* feat: Add Bedrock API key support
* fix: Respect Bedrock credential mode
* fix: Support mixed Bedrock credential forms
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
* fix(agents): normalize empty MCP tool descriptions to undefined
MCP servers (e.g. Asana MCP) can return tools with an empty string
description. AWS Bedrock's converse API rejects toolSpec.description
with length < 1, so any empty-description MCP tool caused the entire
request to fail with a validation error.
Convert empty strings to undefined at the two sites in
loadToolDefinitions where MCP tool definitions are built. An undefined
description is omitted from JSON serialization, so Bedrock never sees
the empty value. OpenAI and Anthropic direct APIs are unaffected.
Fixes#13209
* test(agents): add coverage for empty MCP tool description normalization
Verify that loadToolDefinitions converts '' descriptions to undefined
for both the sys__all__sys pattern and directly named MCP tools.
