mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-06 14:39:10 +00:00
4006 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Danny Avila
|
452af50eff
|
🧮 fix: Atomize Redis Event Sequence Counters for Multi-Replica Deployments (#12578)
* fix: atomize Redis event sequence counters for multi-replica deployments Replace in-memory sequenceCounters Map with shared atomic Redis counter (INCR via Lua script) so all replicas share an authoritative sequence source. Make syncReorderBuffer async to read current sequence from Redis instead of defaulting to 0 on non-publishing replicas. Closes #12575 * fix: harden emitChunk error handling and syncReorderBuffer race safety Wrap getNextSequence inside emitChunk's try/catch so a transient Redis failure is logged-and-swallowed, preserving the non-fatal chunk emission contract. In syncReorderBuffer, replace pending.clear() with selective discard of stale entries (seq < currentSeq) followed by flushPendingMessages, preventing loss of chunks that arrived during the async GET window. * fix: address review findings — harden error paths, validate types, DRY tests - Wrap syncReorderBuffer in try/catch in GenerationJobManager.subscribe() so a Redis GET failure degrades gracefully instead of crashing SSE reconnect. - Validate eval return type in getNextSequence; throw on unexpected type instead of silently computing NaN/-1 and dropping all messages. - Add NaN guard to parseInt in syncReorderBuffer for corrupted Redis values. - Consolidate two streams loops in destroy() into a single pass. - Extract createMockPublisher to shared helpers/publisher.ts (DRY). - Add tests for eval failure and unexpected eval return type in emitChunk. - Document performance tradeoff (2x RTT per emit) and TTL rationale. * test: add cross-replica integration tests for sequence desync fix (#12575) Three real-Redis tests that reproduce the exact multi-replica failure: - Late subscriber on a different replica syncs to the shared counter and receives chunks immediately (no 500ms force-flush). - Multiple subscribe/unsubscribe cycles across replicas maintain correct sequence alignment on every reconnect. - Shared counter key is cleaned up when the stream is destroyed. * fix: close syncReorderBuffer race, stop destroy() from nuking shared keys - Replace selective prune with Math.min(currentSeq, minPendingSeq) so chunks that arrive in pending during the async GET window are preserved instead of incorrectly pruned. Since unsubscribe() already clears pending, any entries at sync time are live messages from the current subscription and must not be discarded. - Remove DEL from destroy() — the sequence key is shared across replicas and a shutting-down instance must not nuke it for active publishers. Keys expire naturally via their 1-hour TTL; cleanup() handles single-stream lifecycle teardown. - Add race-condition test: pauses GET, injects a message into pending during the window, resolves GET, asserts the chunk is delivered. - Add emitDone/emitError eval failure tests to cover the asymmetric error contract (chunk swallows, done/error propagates). - Add explicit MockPublisher return type to helpers/publisher.ts. * fix: context-aware syncReorderBuffer to prevent duplicate delivery, silence test logs The Math.min(currentSeq, minPending) fix from the prior commit correctly handles the cross-replica race (chunk arriving during async GET), but causes duplicate delivery in same-replica mode: onAbort subscribes to the Redis channel during createJob, so chunks published before subscribe() may arrive via pub/sub AND earlyEventBuffer. The Math.min logic then flushes the pub/sub copy as a "live" message. Fix: add a clearPending parameter to syncReorderBuffer. The manager passes true when earlyEventBuffer was replayed (same-replica: pending entries are duplicates → clear them) and false/undefined when it was not (cross-replica: pending entries are live → preserve via Math.min). - Silence winston logger in stream test files via logger.silent = true, following the existing pattern in data-schemas/prompt.spec.ts. - Remove sequence key DEL from destroy() — shared across replicas, a shutting-down instance must not nuke the counter for active publishers. Keys expire via TTL; cleanup() handles stream teardown. * fix: share publisher client in cross-replica tests for Redis Cluster compat The cross-replica tests created publisherB via (ioredisClient as Redis) .duplicate(), which produces a plain Redis connection to a single node. In Redis Cluster, GET/EVAL on this client can't follow MOVED redirects, so syncReorderBuffer reads null → nextSeq=0 → chunks are buffered. Fix: share ioredisClient as the publisher for both replicas. It's the correct Cluster-aware client and handles slot routing automatically. Only subscriber connections need to be separate (pub/sub requirement). * fix: increase cross-replica test timeouts and use polling for CI cluster Replace fixed 300ms delivery waits with polling (50ms intervals, 2s max) to handle Redis Cluster's cross-node pub/sub broadcast latency in CI. Increase subscription activation wait from 100ms to 500ms to match the pattern used by existing same-instance tests. * chore: silence winston logs in remaining stream test files Add logger.silent = true to GenerationJobManager, RedisJobStore, and collectedUsage test files, matching the pattern already applied to RedisEventTransport and reconnect-reorder-desync tests. * fix: remove flaky cluster pub/sub tests, fix log suppression for resetModules Remove two cross-replica transport-level integration tests that are inherently non-deterministic in Redis Cluster: cluster pub/sub fan-out is async across nodes, so pre-subscribe PUBLISHes can arrive at the subscriber's node after SUBSCRIBE takes effect, causing random +/- 1 message counts. The core logic is already covered by deterministic unit tests (mock publisher) and GenerationJobManager integration tests (end-to-end with earlyEventBuffer). The cleanup test is retained. Switch log suppression from logger.silent (which doesn't survive jest.resetModules) to jest.spyOn(console, 'log').mockImplementation() for files that use resetModules (GenerationJobManager, RedisJobStore, collectedUsage). The logger.silent approach remains for files that don't use resetModules (RedisEventTransport, reconnect-reorder-desync). * fix: replace Lua EVAL+TTL with plain INCR, preserve live chunks during same-replica sync P1: syncReorderBuffer with clearPending=true was unconditionally clearing pending, dropping NEW chunks (seq >= currentSeq) from ongoing generation that arrived via pub/sub during the async GET. Fix: selectively prune only entries with seq < currentSeq (duplicates of earlyEventBuffer) and flush remaining live entries. P2: The 1-hour TTL on sequence keys could expire mid-stream during long quiet periods (e.g., slow tool calls), restarting the counter from zero and causing subscribers to silently drop all subsequent messages. Fix: remove the Lua EVAL+EXPIRE script entirely and use plain Redis INCR — no TTL. Keys are cleaned up explicitly by cleanup()/resetSequence() when streams end. Orphaned keys from crashed processes are a few bytes each, negligible compared to the production risk of TTL expiry. - Update mock publisher helper: eval → incr - Remove "unexpected eval type" tests (not applicable to incr) - Update remaining eval error tests to reference incr * fix: re-arm flush timeout after syncReorderBuffer when gaps remain syncReorderBuffer clears flushTimeout before processing, but if pending still has gaps after flushPendingMessages, no timeout is re-armed. Without new messages arriving to trigger scheduleFlushTimeout, those buffered entries sit indefinitely — stalling the stream after reconnect. * chore: address final review — fix stale docs, rename clearPending, tighten tests Fix all 10 findings from final review pass: F1: Fix stale TTL comment in destroy() — no TTL exists after EVAL removal F2: Fix stale EVAL reference in emitChunk JSDoc → INCR + PUBLISH F3: Fix syncReorderBuffer JSDoc — describes old broken behavior, not the current selective prune F4: Rename clearPending → pruneStaleEntries for clarity at call sites F5: Add publish-not-called assertion to INCR failure test F6: Replace Math.min(...spread) with explicit loop to avoid heap alloc F7: Remove resetSequence from IEventTransport interface (no external callers; implementation detail only) F8: Consolidate cleanup/resetSequence overlap — cleanup() owns the DEL, resetReorderBuffer() (now private) handles buffer state only F9: Fix import order in reconnect test (value before type imports) F10: Export MockPublisher interface from helpers/publisher.ts * chore: fix stale resetSequence references and hadBufferReplay JSDoc Two comments referenced resetSequence() which was removed in the F7/F8 refactor (now private resetReorderBuffer(), which doesn't DEL the key). Only cleanup() deletes the Redis key. Also update hadBufferReplay JSDoc to say "prune stale entries" instead of the pre-refactor "clear pending". * chore: grammar * fix: use earlyReplayCount as prune cutoff instead of Redis counter The boolean pruneStaleEntries flag used currentSeq (from Redis GET) as the prune threshold, but INCR can advance the counter past a live chunk's seq during the GET window. Example: earlyEventBuffer held seqs 0-4, generation emits seq 5 during GET, INCR advances counter to 6, GET returns 6, prune condition 5 < 6 deletes the live chunk. Fix: pass the earlyEventBuffer replay count (5) as the prune cutoff. Entries with seq < earlyReplayCount are true duplicates; entries at or above are live regardless of what the Redis counter reads. After pruning, the unified Math.min(currentSeq, minPending) logic handles both same-replica and cross-replica paths correctly. Add a targeted test that exercises this exact race: pauses GET, emits seq 5 during the window, resolves GET with counter=6, asserts seq 5 is preserved (would have been dropped with the old boolean approach). * fix: pass earlyReplayCount for skipBufferReplay path to prune pub/sub duplicates When skipBufferReplay is true (resume scenario), earlyEventBuffer events are delivered via the resume sync payload, not replayed directly. But earlyReplayCount was left at 0, so syncReorderBuffer treated all pending entries as live — meaning delayed pub/sub copies of those buffered events could be delivered again, duplicating content. Fix: capture earlyEventBuffer.length before the skip/replay branch so the count is always passed to syncReorderBuffer regardless of delivery method. Seqs 0..earlyReplayCount-1 are pruned as duplicates whether they were replayed or delivered via sync payload. * fix: keep nextSeq monotonic in syncReorderBuffer after async GET handleOrderedChunk can deliver in-order messages and advance nextSeq during the async GET window. If those messages leave pending empty, the unconditional nextSeq = currentSeq could regress nextSeq below its already-advanced value, reopening a delivered gap and causing subsequent messages to be buffered until force-flush. Fix: wrap both nextSeq assignments with Math.max(nextSeq, ...) so syncReorderBuffer never moves the delivery frontier backward. * fix: cap nextSeq at earlyReplayCount, add 24h safety TTL, add post-GET race test Finding 1 (CRITICAL): When earlyReplayCount > 0 and pending is empty, nextSeq was set to currentSeq — but INCR can advance the counter past a live chunk whose pub/sub hasn't arrived yet. Cap at earlyReplayCount instead (what was actually delivered), so in-flight chunks are not skipped. Adds test for the message-arrives-AFTER-GET-resolves scenario. Finding 3: Add a 24-hour safety-net TTL set once on first INCR only (val === 1), never refreshed. This caps orphan lifetime from crashed processes without risking mid-stream counter resets. Finding 6: Replace setTimeout(100) in cleanup test with polling loop. Finding 9: Fix variadic DEL mock + add expire mock for the new TTL. Revert P2 fix (earlyReplayCount for skipBufferReplay path) — when skipBufferReplay is true, the resume sync payload delivers everything up to currentSeq, so syncReorderBuffer should trust the Redis counter as the frontier, not the buffer length. * chore: log expire failures consistently with other fire-and-forget errors |
||
|
Danny Avila
|
632ffbcb87
|
🧬 fix: Merge Custom Endpoints by Name Instead of Replacing Entire Array (#12586)
* fix: Merge Custom Endpoints by Name Instead of Replacing Entire Array The DB base config's `endpoints.custom` array was wholesale-replacing the YAML-derived array, causing YAML endpoint additions to be silently lost after the first admin panel save. Add path-aware array merging to `deepMerge` so keyed arrays (matched by `name`) are merged item-by-item instead of replaced. * fix: Harden mergeArrayByKey — deduplicate, sanitize, and prevent mutation - Remove redundant sourceOrder array; iterate Map.keys() instead to prevent duplicate entries when source contains repeated names. - Sanitize override-only appended items through deepMerge to enforce UNSAFE_KEYS prototype-pollution protection on all code paths. - Shallow-copy unmatched base items to prevent mutation leak-back. - Add post-OVERRIDE_KEY_MAP remapping note to ARRAY_MERGE_KEYS JSDoc. - Add tests: duplicate source names, base mutation safety, multi-priority sequential merging of the same custom endpoint. * fix: Add inline comments and test for keyless source items - Document keyless item drop behavior in mergeArrayByKey with inline comment and matching test case. - Add last-write-wins comment to deduplication test assertion. - Clarify path semantics in mergeArrayByKey target-iteration comment. * fix: Relocate keyless-item comment and test target-side preserve - Move keyless-item comment to the if-guard where the skip happens and clarify that target-side keyless items are preserved, not dropped. - Add test verifying base items without a name field are kept in output. |
||
github-actions[bot]
|
72cdeb0437
|
🌍 i18n: Update translation.json with latest translations (#12583)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
Marco Beretta
|
b8158a613e
|
🧑🎨 refactor: Prompts/Sidebar styles for improved UI Consistency (#12426)
* style(sidebar): polish button styles, icon sizes, and nav separator
- Match new chat button style to sidebar toggle
- Unify icon sizes and stroke weights across sidebar icons
- Add separator between new chat and nav links
feat(prompts): inline prompt editing with route-based navigation
- Show prompts dashboard inline when editing/creating from chat
- Use route-based navigation instead of state-driven view switching
- Strip inline view to form-only without duplicate sidebar
style(prompts): unify borders, backgrounds, and admin controls
- Unify borders to border-medium and remove opaque backgrounds
- Add admin/advanced controls to sidebar
- Match chat background, move advanced toggle to versions panel
refactor(prompts): consolidate list actions into dropdown with compact layout
- Unify prompt list to single ChatGroupItem with all actions
- Consolidate actions into dropdown menu
- Distinct dropdown icons with status icon tooltips
- Remove rename action, use Trash icon matching convo items
refactor(prompts): inline-edit title with click-to-edit and save indicator
- Click title text to enter edit mode with pencil icon on hover
- Clean title input with inline save status indicator
- Uniform h-9 header height, smaller title, icon-only save status
- Remove separate edit icon button and confirm/cancel controls
style(prompts): sticky click-to-edit pill with surface-primary background
fix: preserve AuthContext identity across Vite HMR updates
createContext() was re-executed on every HMR module replacement, creating
a new context object that disconnected the existing provider from its
consumers. useAuthContext() would throw because useContext(newContext)
returned undefined while the provider was still on the old context.
Stash the context object in import.meta.hot.data so it survives HMR
re-execution. Dead-code-eliminated in production builds.
feat: add advanced prompts editor toggle to Chat settings
- Add AdvancedPrompts switch in Settings > Chat that maps the
promptsEditorMode enum (simple/advanced) to a boolean toggle
- Preserve existing behavior: switching to simple forces alwaysMakeProd
- Add focus styling to PromptName input (border-medium on focus,
outline on focus-visible, no ring)
- Add localization keys: com_nav_advanced_prompts,
com_nav_advanced_prompts_desc
- Various Prompts UI refinements
style(sidebar): update nav icons and reorder links for clarity
- Prompts: MessageSquareQuote → NotebookPen (distinct from chat bubble)
- Agent Builder: Blocks → Bot (no longer identical to Assistants)
- Memories: Database → BrainCircuit (brain + AI circuit)
- Parameters: Settings2 → SlidersHorizontal (tuning-specific)
- Reorder: Chat → Prompts → Agents → Assistants → Memories →
Bookmarks → Files → Parameters → MCP (frequency and grouping)
refactor(prompts): remove AdvancedSwitch component
The simple/advanced toggle is now in Settings > Chat, making
the standalone AdvancedSwitch redundant. Remove the component,
its barrel exports, and all render sites (PromptForm,
DashBreadcrumb, PromptsView).
style(prompts): tighten panel layout and spacing to match sidebar conventions
- Remove top margin from form wrapper, add mt-2 to header row
- Reduce header gaps and remove flex-wrap for tighter alignment
- Shrink prompt title from text-lg/h-9 to text-base/h-8
- Add pl-2 inner padding to PromptName input and static button
- Compact versions panel and deploy button container padding
- Reduce editor header and content spacing
- Compact mobile versions panel header
fix(prompts): rebuild mobile versions panel with proper transitions
- Add background (bg-surface-primary-alt) and border-l so content
doesn't bleed through
- Replace conditional render with CSS opacity/translate so both
overlay fade-out and panel slide-out animate on close
- Use inert attribute to disable focus when panel is hidden
- Replace raw × character with X icon from lucide-react
- Use size-icon button variant for close instead of text sm
- Use Tailwind classes (w-80, translate-x-full) instead of inline
style object for width and transform
- Remove unused sidePanelWidth constant
- Overlay stays in DOM with pointer-events-none when hidden
chore: remove stale tooling artifacts
Remove .bg-shell/manifest.json and .gsd symlink that were
accidentally committed — these are local dev tooling files.
fix: resolve review issues in PromptName and ChatGroupItem
- Fix Escape/onBlur race in PromptName via cancelledRef guard
- Clear existing timer before creating new one to prevent status flicker
- Guard delete mutation against empty group id
- Attach menuButtonRef to MenuButton for proper focus restoration after preview dialog closes
fix: address review findings across prompts UI and sidebar
PromptName:
- Fix Enter key double-save: set skipBlurRef before inline save to prevent
onBlur from re-firing saveName on input unmount
- Add isError prop to distinguish mutation failure from success, preventing
false 'saved' checkmark on error
- Remove dead cn ternary that always evaluated to opacity-100
- Remove unused cn import
PromptForm:
- Pass isError from updateGroupMutation to PromptName
- Remove dead default arg (= {}) on function component
ChatGroupItem:
- Stabilize handleDelete via ref pattern to prevent memo-defeating
recreation on every render (deleteGroup is a new ref each render)
- Disable delete button during mutation to prevent double-fire
ExpandedPanel:
- Restore <a> element for NewChatButton to fix middle-click (open in
new tab) regression caused by anchor-to-button migration
DashGroupItem:
- Delete dead file (no longer exported or imported anywhere)
fix: address review findings across prompts UI and sidebar
- PromptName: render error state (red X) when save fails, extract
shared commitName helper to deduplicate blur/Enter save paths
- ChatGroupItem: navigate away after deleting the active prompt in
sidebar view; use context-aware route prefix (/prompts vs /d/prompts)
for edit and card-click navigation
- InlinePromptsView: redirect to /c/new when user lacks prompts access
instead of rendering a blank screen
- Remove dead ManagePrompts component and its barrel exports (no
remaining consumers after GroupSidePanel cleanup)
fix: remove duplicate showThinking Recoil atom key
The 'showThinking' key was defined in both store/settings.ts (Recoil)
and store/showThinking.ts (Jotai). Only the Jotai atom is consumed;
the stale Recoil duplicate causes 'A key option with a unique string
value must be provided' at startup.
refactor: remove /d/prompts dashboard route and dead code
The prompts UI now lives at /prompts/* inline under the chat layout.
The old /d/prompts/* dashboard route, its layout (PromptsView), and
its breadcrumb (DashBreadcrumb) are no longer used.
- Delete PromptsView and DashBreadcrumb (zero consumers)
- Delete BackToChat button (zero consumers)
- Replace /d/prompts/* child routes with a redirect to /prompts/new
- Add /prompts index route that redirects to /prompts/new
- Update all /d/prompts navigation to /prompts:
- ChatGroupItem: always use /prompts prefix
- NoPromptGroup: navigate to /prompts
- CreatePromptForm: fallback navigate to /prompts/:id
- CreatePromptButton: simplified to /prompts/new (no dual-path)
- Strip GroupSidePanel of dashboard-only breadcrumb nav, recoil
state clearing, and useDashboardContext dependency
refactor: remove dead Dashboard code and unused translation keys
- Delete DashboardContext provider (zero remaining consumers)
- Simplify DashboardRoute layout to a plain Outlet
- Remove commented-out file/vector-store route blocks
- Change catch-all redirect from /d/files to /c/new
- Remove 7 unused translation keys (com_nav_toggle_sidebar,
com_ui_back_to_chat, com_ui_dashboard, com_ui_delete_prompt_name,
com_ui_global_group, com_ui_prompt_renamed, com_ui_rename_prompt,
com_ui_rename_prompt_name)
fix: add Babel plugin to transform import.meta.hot for Jest
`babel-plugin-transform-import-meta` handles standard properties (url,
filename, dirname, resolve) but not Vite's `hot` property. Jest runs in
CommonJS where `import.meta` is unavailable, so `import.meta.hot` in
AuthContext.tsx (added for HMR preservation) causes a SyntaxError that
breaks 27 test suites.
Add a small Babel plugin that replaces `import.meta.hot` with `undefined`
during Jest transforms, making the HMR guard blocks dead-code in tests.
fix: address review findings — inert typing, accessibility, and cleanup
- Add React type augmentation for `inert` attribute (React 18 compat)
- Replace spread hack `{...{ inert: }}` with direct prop in all 3 files
- Add `aria-hidden` to mobile overlay in PromptForm for screen readers
- Simplify deleteGroupRef pattern to direct mutation call
- Remove unused `useEffect` import and stale useMemo dependency
- Add clarifying comment for skipBlurRef mechanism in PromptName
fix: mobile UX for marketplace and prompts views
- Remove mobile new chat button from chat history section
- Add OpenSidebar entry points to marketplace and prompts views on mobile
- Move marketplace admin settings to a compact mobile top row
- Restructure prompt forms to surface category selector beside sidebar toggle
- Make versions panel slide content like the main sidebar and drop redundant borders
- Collapse versions button to icon-only on mobile
- Remove theme selector from prompt panel navigation
* fix: address PR review findings for prompts refactor
- Preserve prompt ID in /d/prompts/:id → /prompts/:id redirect
- Gate PreviewPrompt and VariableDialog behind isChatRoute to avoid
mounting dead dialogs in dashboard mode
- Add onError handler to useDeletePromptGroup and close dialog on
success
- Use useId() instead of hardcoded labelId in AdvancedPrompts
- Extract shared lazy loader for InlinePromptsView routes
* fix: complete review fixes for prompts refactor
- Move OGDialog (delete) inside isChatRoute gate with other dialogs
- Use useId() for both Switch id and label id in AdvancedPrompts
- Add com_ui_prompt_delete_error i18n key for actionable error context
- Drop no-op useCallback on handleDelete (unstable deleteGroup dep)
* refactor: hoist promptPath to module-scope constant
Eliminates stale-closure lint concern in dropdownItems useMemo and
removes the unnecessary dep array entry from onCardClick.
* refactor(sidebar): update icons and reorder links for clarity
- Replace Blocks icon with OpenAIMinimalIcon for the Assistant Builder link.
- Update Memories icon from BrainCircuit to Brain.
- Reintroduce Prompts link conditionally based on access permissions.
- Change Conversations icon from MessageSquare to MessagesSquare for consistency.
* refactor(sidebar): update icons and improve file attachment link
- Replace NewChatIcon with SquarePen in the NewChatButton for better visual consistency.
- Change AttachmentIcon to Paperclip in the file attachment link for clarity.
* refactor(sidebar): update file attachment icon for consistency
- Replace Paperclip icon with AttachmentIcon in the file attachment link for improved clarity and visual consistency.
* refactor(admin-settings): remove unused button and streamline dialog integration
- Eliminate the Admin button and its associated icon from the AdminSettings component for a cleaner interface.
- Simplify the confirm dialog integration by directly using the OGDialog without the button trigger.
* fix: context HMR issue
* style(prompts): enhance component structure and accessibility
- Update AutoSendPrompt button class for improved styling.
- Refactor List component to streamline loading and empty states.
- Ensure FilterPrompts handles context gracefully with null checks.
- Modify GroupSidePanel to prevent rendering without context.
- Simplify PromptsAccordion layout for better readability.
- Adjust CategoryIcon fallback behavior for undefined categories.
* refactor(useMCPServerManager): clean up import statements
- Remove duplicate import of MCPServerInitState for better clarity and organization.
- Adjust import order to maintain consistency with project structure.
* refactor(GroupSidePanel): restructure layout for improved readability and accessibility
- Adjust the structure of the GroupSidePanel component to enhance layout clarity.
- Move the PanelNavigation component into a more appropriate position within the hierarchy.
- Ensure consistent styling and behavior based on the isChatRoute condition.
* style(GroupSidePanel): adjust padding for improved layout consistency
- Update padding in the GroupSidePanel component to enhance visual alignment and readability.
- Ensure consistent styling across the component for a better user experience.
* fix(Conversations): add cache clearing and row height recomputation on search query change
- Implement useEffect to clear cache and recompute row heights when the search query changes.
- Enhance performance and responsiveness of the Conversations component during search operations.
* refactor(GroupSidePanel, PromptsAccordion): simplify layout and improve styling
* chore: import order
* fix: redirect users without CREATE permission from /prompts/new
Users with USE but not CREATE permission were seeing a blank page at
/prompts/new because InlinePromptsView only checked USE access.
CreatePromptForm's internal redirect was bypassed by the onSuccess
prop always being passed. Add CREATE check in InlinePromptsView so
the redirect happens before CreatePromptForm mounts.
* fix: restore dropdown actions for all routes and handle non-creator landing
- Remove isChatRoute gate on dropdown menu so preview, edit, and
delete actions are available on the prompts management route
- Un-gate PreviewPrompt and OGDialog (delete) since both are
triggered from the now-always-visible dropdown
- Keep VariableDialog gated behind isChatRoute (chat submission only)
- Show EmptyPromptPreview for non-creators at /prompts/new instead
of redirecting to /c/new, so they stay in the prompts section
with sidebar access to browse existing prompts
* fix: add isPublic to TPromptGroup type
The database schema (IPromptGroup in data-schemas) has isPublic but
the shared TPromptGroup type in data-provider was missing it,
causing a TS2339 error in ChatGroupItem.
* fix: prevent duplicate rename and restore name on error in PromptName
- Block re-entry to edit mode while a save is in flight by guarding
the click handler with isLoading/saveStatus checks
- Reset newName to the prop value when mutation fails so the UI
doesn't display the unsaved name after the error icon clears
* fix: address review findings across prompts refactor
- Consolidate duplicate usePromptGroupsContext() calls in PromptForm
- Remove invalid aria-labelledby (text string, not ID) from
AutoSendPrompt checkbox that is already aria-hidden
- Remove useMemo wrapping trivial `disabled ?? false` in ToolsDropdown
- Remove dead context spread in PromptsAccordion (GroupSidePanel
reads context internally)
- Wrap search cache-clear effect in requestAnimationFrame to match
favorites effect pattern in Conversations
- Use Set for O(1) lookups in MCPSelect server filtering
- Fix unnecessary JSX expression wrapper on string literal in
CreatePromptButton Link
* style(PromptTextCard): update icon classes for improved accessibility
- Add 'text-text-secondary' class to Check and Copy icons for better visibility and consistency in the PromptTextCard component.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
Dustin Healy
|
55fc37ff49
|
🔐 fix: Add Tenant Context to Admin OAuth Callback Routes (#12579)
* 🔐 fix: Add Tenant Context to Admin OAuth Callback Routes
* fix: add tenant context to admin local login route
|
||
|
Danny Avila
|
01a1bc1689
|
📊 experimental: Add GitNexus CI/CD and deployment configuration (#12577)
* feat: Add GitNexus CI/CD and deployment configuration - Introduced a Dockerfile for building the GitNexus application with necessary dependencies and configurations. - Added a Caddyfile to set up a reverse proxy with bearer token authentication for secure access to GitNexus. - Created an entrypoint script to validate the API token and start both GitNexus and Caddy services. - Configured Fly.io deployment settings in fly.toml, including health checks and service parameters. - Established GitHub Actions workflows for deploying the GitNexus index and managing deployments to Fly.io. * fix: use npx instead of bunx for native addon compatibility bunx skips node-gyp lifecycle scripts, so @ladybugdb/core's native .node binary never gets compiled/downloaded. npx handles this correctly. |
||
|
Danny Avila
|
96312aa4fd
|
🎯 fix: Use Resolved Provider for Agent Token Lookup on Custom Endpoints (#12574)
* fix: Use resolved provider for agent token lookup on custom endpoints
The providerEndpointMap lookup in initializeAgent used the original
provider name (e.g. "EduGPT") instead of the resolved overrideProvider
("openai"). Since providerEndpointMap only contains 4 built-in
providers, custom providers resolved to undefined, causing
getModelMaxTokens to miss the token map and fall back to 18000 tokens.
With agent instructions + tool schemas consuming most of that budget,
createPruneMessages would strip all messages on the first turn.
* fix: Use correct EndpointTokenConfig type in test
* refactor: Unify test factory, remove non-discriminating test
Address review findings:
- Remove Test 2 ("uses the model real context window") which passed
with and without the fix due to getModelMaxTokens defaulting to
openAI when endpoint is undefined (JS default parameter semantics)
- Merge createCustomProviderMocks into createMocks via provider,
overrideProvider, and useRealTokenLookup parameters
- Hoist jest.requireActual to file scope for shared access
* refactor: Address followup review findings
- Replace loose `maxContextTokens > 18000` assertion with precise
computed value `Math.round((65536 - 4096) * 0.95)` so the outcome
assertion is meaningful and self-documenting
- Hoist `customProvider` to describe-level constant `CUSTOM_PROVIDER`
- Document `overrideProvider` semantics and `useRealTokenLookup` in
factory JSDoc
- Add comment on real `optionalChainWithEmptyCheck` noting its
zero-handling semantics are load-bearing for the maxContextTokens=0
test
* style: Use // for inline comment, clarify pipeline assertion role
|
||
neuralnotesyt-netizen
|
622a934a82
|
🗒️ docs: Update LICENSE.md Year: 2025 -> 2026 (#12554) | ||
|
github-actions[bot]
|
c940486a06
|
🌍 i18n: Update translation.json with latest translations (#12571)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Danny Avila
|
d350c58633
|
🚫 fix: Hide Delete Account Button When ALLOW_ACCOUNT_DELETION Is Disabled (#12568)
* fix: hide Delete Account button when ALLOW_ACCOUNT_DELETION is false * fix: add admin bypass, inline env read, and tests for allowAccountDeletion - Show delete button for admin users even when ALLOW_ACCOUNT_DELETION=false, matching the canDeleteAccount middleware's ACCESS_ADMIN bypass - Move env var read inline in buildSharedPayload() for per-request evaluation - Add 4 frontend tests for Account conditional rendering - Add 3 backend tests for allowAccountDeletion config field * fix: use server-side ACCESS_ADMIN capability check instead of frontend role check - Replace frontend SystemRoles.ADMIN check with server-side hasCapability() in the authenticated config route, matching canDeleteAccount middleware exactly - Admin bypass now evaluates ACCESS_ADMIN capability per-user in GET /api/config, so users with the grant (regardless of role) see the button, and admins without the grant do not - Add 3 authenticated backend tests: without capability, with capability, and skip-when-already-enabled - Simplify frontend to pure config check (no role logic) - Remove redundant jest-dom import; add inline env var comment * test: add missing toHaveBeenCalled assertion in ACCESS_ADMIN test |
||
|
Danny Avila
|
223065c411
|
📦 chore: npm audit (#12570)
* 📦 chore: npm audit fix - Bump `vite` from 7.3.1 to 7.3.2. - Upgrade `@chevrotain/cst-dts-gen`, `@chevrotain/gast`, `@chevrotain/regexp-to-ast`, `@chevrotain/types`, and `@chevrotain/utils` from 11.1.2 to 12.0.0. - Update `@hono/node-server` from 1.19.10 to 1.19.13. - Upgrade `chevrotain` from 11.1.2 to 12.0.0. - Bump `chevrotain-allstar` from 0.3.1 to 0.4.1. * 🔧 chore: Remove `serialize-javascript` dependency from `package.json` |
||
|
Danny Avila
|
15fc27950d
|
⚡ refactor: Short-Circuit Config Override Resolution (#12553) | ||
Denis Palnitsky
|
8ed0bcf5ca
|
♻️ fix: Reuse Existing MCP OAuth Client Registrations to Prevent client_id Mismatch (#11925)
* fix: reuse existing OAuth client registrations to prevent client_id mismatch
When using auto-discovered OAuth (DCR), LibreChat calls /register on every
flow initiation, getting a new client_id each time. When concurrent
connections or reconnections happen, the client_id used during /authorize
differs from the one used during /token, causing the server to reject the
exchange.
Before registering a new client, check if a valid client registration
already exists in the database and reuse it.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Handle re-registration of OAuth clients when redirect_uri changes
* Add undefined fields for logo_uri and tos_uri in OAuth metadata tests
* test: add client registration reuse tests for horizontal scaling race condition
Reproduces the client_id mismatch bug that occurs in multi-replica deployments
where concurrent initiateOAuthFlow calls each register a new OAuth client.
Tests verify that the findToken-based client reuse prevents re-registration.
* fix: address review findings for client registration reuse
- Fix empty redirect_uris bug: invert condition so missing/empty
redirect_uris triggers re-registration instead of silent reuse
- Revert undocumented config?.redirect_uri in auto-discovery path
- Change DB error logging from debug to warn for operator visibility
- Fix import order: move package type import to correct section
- Remove redundant type cast and misleading JSDoc comment
- Test file: remove dead imports, restore process.env.DOMAIN_SERVER,
rename describe blocks, add empty redirect_uris edge case test,
add concurrent reconnection test with pre-seeded token,
scope documentation to reconnection stabilization
* fix: resolve type check errors for OAuthClientInformation redirect_uris
The SDK's OAuthClientInformation type lacks redirect_uris (only on
OAuthClientInformationFull). Cast to the local OAuthClientInformation
type in handler.ts when accessing deserialized client info from DB,
and use intersection types in tests for clientInfo with redirect_uris.
* fix: address follow-up review findings R1, R2, R3
- R1: Move `import type { TokenMethods }` to the type-imports section,
before local types, per CLAUDE.md import order rules
- R2: Add unit test for empty redirect_uris in handler.test.ts to
verify the inverted condition triggers re-registration
- R3: Use delete for process.env.DOMAIN_SERVER restoration when the
original value was undefined to avoid coercion to string "undefined"
* fix: clear stale client registration on OAuth flow failure
When a stored client_id is no longer recognized by the OAuth server,
the flow fails but the stale client stays in MongoDB, causing every
retry to reuse the same invalid registration in an infinite loop.
On OAuth failure, clear the stored client registration so the next
attempt falls through to fresh Dynamic Client Registration.
- Add MCPTokenStorage.deleteClientRegistration() for targeted cleanup
- Call it from MCPConnectionFactory's OAuth failure path
- Add integration test proving recovery from stale client reuse
* fix: validate auth server identity and target cleanup to reused clients
- Gate client reuse on authorization server identity: compare stored
issuer against freshly discovered metadata before reusing, preventing
wrong-client reuse when the MCP server switches auth providers
- Add reusedStoredClient flag to MCPOAuthFlowMetadata so cleanup only
runs when the failed flow actually reused a stored registration,
not on unrelated failures (timeouts, user-denied consent, etc.)
- Add cleanup in returnOnOAuth path: when a prior flow that reused a
stored client is detected as failed, clear the stale registration
before re-initiating
- Add tests for issuer mismatch and reusedStoredClient flag assertions
* fix: address minor review findings N3, N5, N6
- N3: Type deleteClientRegistration param as TokenMethods['deleteTokens']
instead of Promise<unknown>
- N5: Elevate deletion failure logging from debug to warn for operator
visibility when stale client cleanup fails
- N6: Use getLogPrefix() instead of hardcoded log prefix to respect
system-user privacy convention
* fix: correct stale-client cleanup in both OAuth paths
- Blocking path: remove result?.clientInfo guard that made cleanup
unreachable (handleOAuthRequired returns null on failure, so
result?.clientInfo was always false in the failure branch)
- returnOnOAuth path: only clear stored client when the prior flow
status is FAILED, not on COMPLETED or PENDING flows, to avoid
deleting valid registrations during normal flow replacement
* fix: remove redundant cast on clientMetadata
clientMetadata is already typed as Record<string, unknown>; the
as Record<string, unknown> cast was a no-op.
* fix: thread reusedStoredClient through return type instead of re-reading flow state
FlowStateManager.createFlow() deletes FAILED flow state before
rejecting, so getFlowState() after handleOAuthRequired() returns null
would find nothing — making the stale-client cleanup dead code.
Fix: hoist reusedStoredClient flag from flowMetadata into a local
variable, include it in handleOAuthRequired()'s return type (both
success and catch paths), and use result.reusedStoredClient directly
in the caller instead of a second getFlowState() round-trip.
* fix: selective stale-client cleanup in returnOnOAuth path
The returnOnOAuth cleanup was unreliable: it depended on reading
FAILED flow state, but FlowStateManager.monitorFlow() deletes FAILED
state before rejecting. Move cleanup into createFlow's catch handler
where flowMetadata.reusedStoredClient is still in scope.
Make cleanup selective in both paths: add isClientRejection() helper
that only matches errors indicating the OAuth server rejected the
client_id (invalid_client, unauthorized_client, client not found).
Timeouts, user-cancelled flows, and other transient failures no
longer wipe valid stored registrations.
Thread the error from handleOAuthRequired() through the return type
so the blocking path can also check isClientRejection().
* fix: tighten isClientRejection heuristic
Narrow 'client_id' match to 'client_id mismatch' to avoid
false-positive cleanup on unrelated errors that happen to
mention client_id.
* test: add isClientRejection tests and enforced client_id on test server
- Add isClientRejection unit tests: invalid_client, unauthorized_client,
client_id mismatch, client not found, unknown client, and negative
cases (timeout, flow state not found, user denied, null, undefined)
- Enhance OAuth test server with enforceClientId option: binds auth
codes to the client_id that initiated /authorize, rejects token
exchange with mismatched or unregistered client_id (401 invalid_client)
- Add integration tests proving the test server correctly rejects
stale client_ids and accepts matching ones at /token
* fix: issuer validation, callback error propagation, and cleanup DRY
- Issuer check: re-register when storedIssuer is absent or non-string
instead of silently reusing. Narrows unknown type with typeof guard
and inverts condition so missing issuer → fresh DCR (safer default).
- OAuth callback route: call failFlow with the OAuth error when the
authorization server redirects back with error= parameter, so the
waiting flow receives the actual rejection instead of timing out.
This lets isClientRejection match stale-client errors correctly.
- Extract duplicated cleanup block to clearStaleClientIfRejected()
private method, called from both returnOnOAuth and blocking paths.
- Test fixes: add issuer to stored metadata in reuse tests, reset
server to undefined in afterEach to prevent double-close.
* fix: gate failFlow behind callback validation, propagate reusedStoredClient on join
- OAuth callback: move failFlow call to after CSRF/session/active-flow
validation so an attacker with only a leaked state parameter cannot
force-fail a flow without passing the same integrity checks required
for legitimate callbacks
- PENDING join path: propagate reusedStoredClient from flow metadata
into the return object so joiners can trigger stale-client cleanup
if the joined flow later fails with a client rejection
* fix: restore early oauthError/code redirects, gate only failFlow behind CSRF
The previous restructuring moved oauthError and missing-code checks
behind CSRF validation, breaking tests that expect those redirects
without cookies. The redirect itself is harmless (just shows an error
page). Only the failFlow call needs CSRF gating to prevent DoS.
Restructure: oauthError check stays early (redirects immediately),
but failFlow inside it runs the full CSRF/session/active-flow
validation before marking the flow as FAILED.
* fix: require deleteTokens for client reuse, add missing import in MCP.js
Client registration reuse without cleanup capability creates a
permanent failure loop: if the reused client is stale, the code
detects the rejection but cannot clear the stored registration
because deleteTokens is missing, so every retry reuses the same
broken client_id.
- MCPConnectionFactory: only pass findToken to initiateOAuthFlow
when deleteTokens is also available, ensuring reuse is only
enabled when recovery is possible
- api/server/services/MCP.js: add deleteTokens to the tokenMethods
object (was the only MCP call site missing it)
* fix: set reusedStoredClient before createFlow in joined-flow path
When joining a PENDING flow, reusedStoredClient was only set on the
success return but not before the await. If createFlow throws (e.g.
invalid_client during token exchange), the outer catch returns the
local variable which was still false, skipping stale-client cleanup.
* fix: require browser binding (CSRF/session) for failFlow on OAuth error
hasActiveFlow only proves a PENDING flow exists, not that the caller
is the same browser that initiated it. An attacker with a leaked state
could force-fail the flow without any user binding. Require hasCsrf or
hasSession before calling failFlow on the oauthError path.
* fix: guard findToken with deleteTokens check in blocking OAuth path
Match the returnOnOAuth path's defense-in-depth: only enable client
registration reuse when deleteTokens is also available, ensuring
cleanup is possible if the reused client turns out to be stale.
* fix: address review findings — tests, types, normalization, docs
- Add deleteTokens method to InMemoryTokenStore matching TokenMethods
contract; update test call site from deleteToken to deleteTokens
- Add MCPConnectionFactory test: returnOnOAuth flow fails with
invalid_client → clearStaleClientIfRejected invoked automatically
- Add mcp.spec.js tests: OAuth error with CSRF → failFlow called;
OAuth error without cookies → failFlow NOT called (DoS prevention)
- Add JSDoc to isClientRejection with RFC 6749 and vendor attribution
- Add inline comment explaining findToken/deleteTokens coupling guard
- Normalize issuer comparison: strip trailing slashes to prevent
spurious re-registrations from URL formatting differences
- Fix dead-code: use local reusedStoredClient variable in PENDING
join return instead of re-reading flowMeta
* fix: address final review nits N1-N4
- N1: Add session cookie failFlow test — validates the hasSession
branch triggers failFlow on OAuth error callback
- N2: Replace setTimeout(50) with setImmediate for microtask drain
- N3: Add 'unknown client' attribution to isClientRejection JSDoc
- N4: Remove dead getFlowState mock from failFlow tests
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
Danny Avila
|
33ee7dea1e
|
🔎 fix: Specify Explicit Primary Key for Meilisearch Document Operations (#12542)
* fix: pass explicit primaryKey to Meilisearch addDocuments/updateDocuments calls
Meilisearch v1.0+ refuses to auto-infer the primary key when a document
contains multiple fields ending with 'id'. The messages index has both
conversationId and messageId, causing addDocuments to silently fail with
index_primary_key_multiple_candidates_found, leaving message search empty.
Pass { primaryKey } to addDocumentsInBatches, addDocuments, and
updateDocuments — the variable was already in scope.
Also replace raw this.collection.updateMany with Mongoose Model.updateMany
to satisfy the no-restricted-syntax ESLint rule (tenant isolation guard).
Closes #12538
* fix: resolve additional Meilisearch plugin bugs found in review
Address review findings from PR #12542:
- Fix deleteObjectFromMeili using MongoDB _id instead of the Meilisearch
primary key (conversationId/messageId), causing post-remove cleanup to
silently no-op and leave orphaned documents in the index.
- Pass options.primaryKey explicitly to createMeiliMongooseModel factory
instead of deriving it from attributesToIndex[0] (schema field order),
eliminating a fragile implicit contract.
- Fix updateObjectToMeili skipping preprocessObjectForIndex, which meant
updates bypassed content array-to-text conversion and conversationId
pipe character escaping.
- Change collection.updateMany to collection.updateOne in addObjectToMeili
since _id is unique (semantic correctness).
- Add primaryKey to validateOptions required keys.
- Strengthen test assertions to verify { primaryKey } argument is passed
to addDocuments, addDocumentsInBatches, and updateDocuments. Add tests
for the update path including preprocessObjectForIndex pipe escaping.
* fix: add regression tests for delete and message update paths
Address follow-up review findings:
- Add test for deleteObjectFromMeili verifying it uses messageId (not
MongoDB _id) when calling index.deleteDocument, guarding against
regression of the silent orphaned-document bug.
- Add test for message model update path asserting { primaryKey:
'messageId' } is passed to updateDocuments (previously only the
conversation model update path was tested).
- Add @param config.primaryKey to createMeiliMongooseModel JSDoc.
|
||
|
Danny Avila
|
b44ce264a4
|
📦 chore: Bump mongodb-memory-server to v11.0.1, mermaid to v11.14.0, npm audit (#12543)
* 🔧 chore: Update `mongodb-memory-server` to v11.0.1
- Bump `mongodb-memory-server` version in `package-lock.json`, `api/package.json`, and `packages/data-schemas/package.json` from 10.1.4 to 11.0.1.
- Update related dependencies in `mongodb-memory-server` and `mongodb-memory-server-core` to ensure compatibility with the new version.
- Adjust `tslib` version in `mongodb-memory-server` to 2.8.1 and `debug` to 4.4.3 for consistency.
* chore: npm audit fix
* chore: Update `mermaid` dependency to version 11.14.0 in `package-lock.json` and `client/package.json`
* fix: use deterministic timestamps in convoStructure test
MongoDB 8.x (from mongodb-memory-server v11) no longer guarantees
insertion-order return for documents with identical timestamps.
Use sequential timestamps with overrideTimestamp to ensure buildTree
processes parents before children.
|
||
Shahryar Tayeb
|
2140729a54
|
🗣️ fix: Prevent @librechat/client useLocalize from Overwriting Host App Language State (#12515)
* directly returns the translation function without managing language state in client package * chore: remove unused langAtom from packages/client store * fix: add useCallback to match canonical useLocalize, add guard comment --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
Daniel Lew
|
162ac9c253
|
📝 fix: Properly Restore Draft Text When Switching Conversations (#12384)
Right now, if you have draft text in conversation A, but no draft text in conversation B, then switching from A -> B inserts the draft from A into B (oops). This was caused by a bug in the `restoreText()` logic which did not restore *blank* text as the saved draft. Now, it'll always restore whatever is found as a draft (or set to blank if there is no draft). |
||
|
Dustin Healy
|
261941c05f
|
🔨 fix: Custom Role Permissions (#12528)
* fix: Resolve custom role permissions not loading in frontend Users assigned to custom roles (non-USER/ADMIN) had all permission checks fail because AuthContext only fetched system role permissions. The roles map keyed by USER/ADMIN never contained the custom role name, so useHasAccess returned false for every feature gate. - Fetch the user's custom role in AuthContext and include it in the roles map so useHasAccess can resolve permissions correctly - Use encodeURIComponent instead of toLowerCase for role name URLs to preserve custom role casing through the API roundtrip - Only uppercase system role names on the backend GET route; pass custom role names through as-is for exact DB lookup - Allow users to fetch their own assigned role without READ_ROLES capability * refactor: Normalize all role names to uppercase Custom role names were stored in original casing, causing case-sensitivity bugs across the stack — URL lowercasing, route uppercasing, and case-sensitive DB lookups all conflicted for mixed-case custom roles. Enforce uppercase normalization at every boundary: - createRoleByName trims and uppercases the name before storage - createRoleHandler uppercases before passing to createRoleByName - All admin route handlers (get, update, delete, members, permissions) uppercase the :name URL param before DB lookups - addRoleMemberHandler uppercases before setting user.role - Startup migration (normalizeRoleNames) finds non-uppercase custom roles, renames them, and updates affected user.role values with collision detection Legacy GET /api/roles/:roleName retains always-uppercase behavior. Tests updated to expect uppercase role names throughout. * fix: Use case-preserved role names with strict equality Remove uppercase normalization — custom role names are stored and compared exactly as the user sets them, with only trimming applied. USER and ADMIN remain reserved case-insensitively via isSystemRoleName. - Remove toUpperCase from createRoleByName, createRoleHandler, and all admin route handlers (get, update, delete, members, permissions) - Remove toUpperCase from legacy GET and PUT routes in roles.js; the frontend now sends exact casing via encodeURIComponent - Remove normalizeRoleNames startup migration - Revert test expectations to original casing * fix: Format useMemo dependency array for Prettier * feat: Add custom role support to admin settings + review fixes - Add backend tests for isOwnRole authorization gate on GET /api/roles/:roleName - Add frontend tests for custom role detection and fetching in AuthContext - Fix transient null permission flash by only spreading custom role once loaded - Add isSystemRoleName helper to data-provider for case-insensitive system role detection - Use sentinel value in useGetRole to avoid ghost cache entry from empty string - Add useListRoles hook and listRoles data service for fetching all roles - Update AdminSettingsDialog and PeoplePickerAdminSettings to dynamically list custom roles in the role dropdown, with proper fallback defaults * fix: Address review findings for custom role permissions - Add assertions to AuthContext test verifying custom role in roles map - Fix empty array bypassing nullish coalescing fallback in role dropdowns - Add null/undefined guard to isSystemRoleName helper - Memoize role dropdown items to avoid unnecessary re-renders - Apply sentinel pattern to useGetRole in admin settings for consistency - Mark ListRolesResponse description as required to match schema * fix: Prevent prototype pollution in role authorization gate - Replace roleDefaults[roleName] with Object.hasOwn to prevent prototype chain bypass for names like constructor or __proto__ - Add dedicated rolesList query key to avoid cache collision when a custom role is named 'list' - Add regression test for prototype property name authorization * fix: Resolve Prettier formatting and unused variable lint errors * fix: Address review findings for custom role permissions - Add ADMIN self-read test documenting isOwnRole bypass behavior - Guard save button while custom role data loads to prevent data loss - Extract useRoleSelector hook eliminating ~55 lines of duplication - Unify defaultValues/useEffect permission resolution (fixes inconsistency) - Make ListRolesResponse.description and _id optional to match schema - Fix vacuous test assertions to verify sentinel calls exist - Only fetch userRole when user.role === USER (avoid unnecessary requests) - Remove redundant empty string guard in custom role detection * fix: Revert USER role fetch restriction to preserve admin settings Admins need the USER role loaded in AuthContext.roles so the admin settings dialog shows persisted USER permissions instead of defaults. * fix: Remove unused useEffect import from useRoleSelector * fix: Clean up useRoleSelector hook - Use existing isCustom variable instead of re-calling isSystemRoleName - Remove unused roles and availableRoleNames from return object * fix: Address review findings for custom role permissions - Use Set-based isSystemRoleName to auto-expand with future SystemRoles - Add isCustomRoleError handling: guard useEffect reset and disable Save - Remove resolvePermissions from hook return; use defaultValues in useEffect to eliminate redundant computation and stale-closure reset race - Rename customRoleName to userRoleName in AuthContext for clarity * fix: Request server-max roles for admin dropdown listRoles now passes limit=200 (the server's MAX_PAGE_LIMIT) so the admin role selector shows all roles instead of silently truncating at the default page size of 50. --------- Co-authored-by: Danny Avila <danny@librechat.ai> |
||
|
Daniel Lew
|
936936596b
|
🔍 fix: only show Searchbar if enabled (#12424)
The search bar was showing even if you have no search capability; now it respects the `enabled` field. |
||
|
Danny Avila
|
ea28dbfa89
|
🧹 chore: Clean Up Config Fields (#12537)
* chore: remove unused `interface.endpointsMenu` config field * chore: address review — restore JSDoc UI-only example, add Zod strip test * chore: remove unused `interface.sidePanel` config field * chore: restrict fileStrategy/fileStrategies schema to valid storage backends * fix: use valid FileStorage value in AppService test * chore: address review — version bump, exhaustiveness guard, JSDoc, configSchema test * chore: remove debug logger.log from MessageIcon render path * fix: rewrite MessageIcon render tests to use render counting instead of logger spying * chore: bump librechat-data-provider to 0.8.407 * chore: sync example YAML version to 1.3.7 |
||
|
Danny Avila
|
b4d97bd888
|
🗜️ refactor: Eliminate Unstable React Keys During SSE Lifecycle (#12536)
* debug: add instrumentation to MessageIcon arePropsEqual + render cycle tests
Temporary debug commit to identify which field triggers MessageIcon
re-renders during message creation and streaming.
arePropsEqual now logs 'icon_memo_diff' with the exact field name and
prev/next values whenever it returns false. Filter browser console for
'icon_memo_diff' to see the trigger.
Also adds render-level integration tests that simulate the message
lifecycle (initial mount, streaming chunks, context updates) and
assert render counts via logger spy.
* perf: stabilize MultiMessage key to prevent unmount/remount during SSE lifecycle
messageId changes 3 times during the SSE message lifecycle:
1. useChatFunctions creates initialResponse with client-generated UUID
2. createdHandler replaces it with userMessageId + '_'
3. finalHandler replaces it with server-assigned messageId
Since MultiMessage used key={message.messageId}, each change caused
React to destroy and recreate the entire message component subtree,
unmounting MessageIcon and all memoized children. This produced visible
icon/image flickering that no memo comparator could prevent.
Switch to key={parentMessageId + '_' + siblingIdx}:
- parentMessageId is stable from creation through final response
- siblingIdx ensures sibling switches still get clean remounts
- Eliminates 2 unnecessary unmount/remount cycles per message
Add key stability tests verifying:
- Current key={messageId} causes 3 mounts / 2 unmounts per lifecycle
- Stable key causes 1 mount / 0 unmounts per lifecycle
- Sibling switches still trigger clean remounts with stable key
* perf: stabilize root MultiMessage key across new conversation lifecycle
When a user sends their first message in a new conversation,
conversationId transitions from null/'new' to the server-assigned
UUID. MessagesView used key={conversationId} on the root MultiMessage,
so this transition destroyed the entire message tree and rebuilt it
from scratch — causing all MessageIcons to unmount/remount (visible
as image flickering).
Use a ref-based stable key that captures the first real conversationId
and only changes on genuine conversation switches (navigating to a
different conversation), not on the null→UUID transition within the
same conversation.
* debug: add mount/unmount lifecycle tracking to MessageIcon
Adds icon_lifecycle logs (MOUNT/UNMOUNT) and render count to
distinguish between fresh mounts (memo comparator not called)
and internal re-renders (hook bypassing memo).
Enable: localStorage.setItem('DEBUG_LOGGING', 'icon_lifecycle,icon_data,icon_memo_diff')
* debug: add key and root tracking to MultiMessage and MessagesView
Logs multi_message_key (stableKey, messageId, parentMessageId, route)
and messages_view_key (rootKey, conversationId) to trace which key
changes trigger unmount/remount cycles.
Enable: localStorage.setItem('DEBUG_LOGGING', 'icon_lifecycle,icon_data,icon_memo_diff,multi_message_key,messages_view_key')
* perf: remove key from root MultiMessage to prevent tree destruction
The ref-based stable key still changed during 'new' → real UUID
transition, destroying the entire tree. The root MultiMessage is the
sole child at its position, so React reuses the instance via
positional reconciliation without any key. The messageId prop
(conversationId) naturally resets Recoil siblingIdxFamily state on
conversation switches.
* perf: remove unstable keys from MultiMessage to prevent SSE lifecycle remounts
Both messageId and parentMessageId change during the SSE lifecycle
(client UUID → CREATED server ID → FINAL server ID), making neither
viable as a stable React key. Each key change caused React to destroy
and recreate the entire message component subtree, including all
memoized children — visible as icon/image flickering.
Remove explicit keys entirely and rely on React's positional
reconciliation. MultiMessage always renders exactly one child at
the same position, so React reuses the component instance and
updates props in place. The existing memo comparators on
ContentRender/MessageRender handle field-level diffing correctly.
Update tests to verify: key={messageId} causes 3 mounts/2 unmounts
per lifecycle, while no key causes 1 mount/0 unmounts.
* perf: remove unstable keys from child MultiMessage in message wrappers
Message.tsx, MessageContent.tsx, and MessageParts.tsx each render a
child MultiMessage with key={messageId} for the current message's
children. Since messageId changes during the SSE lifecycle (CREATED
event replaces the user message ID), the child MultiMessage gets
destroyed and recreated, unmounting the entire agent response subtree
including its MessageIcon.
Remove these keys for the same reason as the parent MultiMessage:
each child MultiMessage renders exactly one child at a fixed position,
so positional reconciliation correctly reuses the instance.
* chore: remove MultiMessage key tests — they test React behavior, not our code
The tests verified that key={messageId} causes remounts while no key
doesn't, but this is React's own reconciliation behavior. No unit test
can prevent someone from re-adding a key prop to JSX. The JSDoc comments
on MultiMessage document the decision and rationale.
|
||
|
Danny Avila
|
fa4a43da21
|
🔐 fix: Strip code_challenge from Admin OAuth requests before Passport (#12534)
* 🔐 fix: Strip code_challenge from admin OAuth requests before Passport openid-client v6's Passport Strategy uses `currentUrl.searchParams.size === 0` to distinguish initial authorization requests from OAuth callbacks. The admin-panel-specific `code_challenge` query parameter caused the strategy to misclassify the request as a callback and return 401 Unauthorized. * 🔐 fix: Strip code_challenge from admin OAuth requests before Passport openid-client v6's Passport Strategy uses `currentUrl.searchParams.size === 0` to distinguish initial authorization requests from OAuth callbacks. The admin-panel-specific `code_challenge` query parameter caused the strategy to misclassify the request as a callback and return 401 Unauthorized. - Fix regex to handle `code_challenge` in any query position without producing malformed URLs, and handle empty `code_challenge=` values (`[^&]*` vs `[^&]+`) - Combine `storePkceChallenge` + `stripCodeChallenge` into a single `storeAndStripChallenge` helper to enforce read-store-strip ordering - Apply defensively to all 7 admin OAuth providers - Add 12 unit tests covering stripCodeChallenge and storeAndStripChallenge * refactor: Extract PKCE helpers to utility file, harden tests - Move stripCodeChallenge and storeAndStripChallenge to api/server/utils/adminPkce.js — eliminates _test production export and avoids loading the full auth.js module tree in tests - Add missing req.originalUrl/req.url assertions to invalid-challenge and no-challenge test branches (regression blind spots) - Hoist cache reference to module scope in tests (was redundantly re-acquired from mock factory on every beforeEach) * chore: Address review NITs — imports, exports, naming, assertions - Fix import order in auth.js (longest-to-shortest per CLAUDE.md) - Remove unused PKCE_CHALLENGE_TTL/PKCE_CHALLENGE_PATTERN exports - Hoist strip arrow to module-scope stripChallengeFromUrl - Rename auth.test.js → auth.spec.js (project convention) - Tighten cache-failure test: toBe instead of toContain, add req.url * refactor: Move PKCE helpers to packages/api with dependency injection Move stripCodeChallenge and storeAndStripChallenge from api/server/utils into packages/api/src/auth/exchange.ts alongside the existing PKCE verification logic. Cache is now injected as a Keyv parameter, matching the dependency-injection pattern used throughout packages/api/. - Add PkceStrippableRequest interface for minimal req typing - auth.js imports storeAndStripChallenge from @librechat/api - Delete api/server/utils/adminPkce.js - Move tests to packages/api/src/auth/adminPkce.spec.ts (TypeScript, real Keyv instances, no getLogStores mock needed) |
||
|
Danny Avila
|
ed02fe40e0
|
🪆 fix: Allow Nested addParams in Config Schema (#12526)
* fix: allow nested addParams in config schema * Respect no-op task constraint Constraint: Task 2 explicitly forbids code changes Directive: Keep this worker branch code-identical to the assigned base for this task Confidence: high Scope-risk: narrow Tested: git status --short (clean) * fix: align addParams web_search validation with runtime * test: cover addParams edge cases * chore: ignore .codex directory |
||
|
Danny Avila
|
6ecd1b510f
|
📎 fix: Route Unrecognized File Types via supportedMimeTypes Config (#12508)
* fix: check supportedMimeTypes before routing unrecognized file types In processAttachments, files not matching the hardcoded mime type categories (image, PDF, video, audio) were silently dropped. Now resolves the endpoint's file config and checks the file type against supportedMimeTypes before routing to the documents pipeline. Files not matching any config are still skipped (original behavior). Closes #12482 * feat: encode generic document types for supported providers Remove restrictive mime type filter in encodeAndFormatDocuments that only allowed PDFs and application/* types. Add a generic encoding path for non-PDF, non-Bedrock files using the provider's native format (Anthropic base64 document, OpenAI file block, Google media block). Files are already validated upstream by supportedMimeTypes. * fix: guard file.type and cache file config in processAttachments - Add file.type truthiness check before checkType to prevent coercion of null/undefined to string 'null'/'undefined' - Cache mergedFileConfig and endpointFileConfig on the instance so addPreviousAttachments doesn't recompute per message * refactor: harden generic document encoding with validation and tests - Extract formatDocumentBlock helper to eliminate ~30 lines of duplicate provider-dispatch code between PDF and generic paths - Add size validation in generic encoding path using configuredFileSizeLimit (was fetched but unused) - Guard Bedrock from generic path — non-bedrockDocumentFormats types are now skipped instead of silently tracking metadata - Only push metadata to result.files when a document block was actually created, preventing silent inconsistent state - Enable Anthropic citations for text/plain, text/html, text/markdown (supported by Anthropic's document API) - Fix != to !== for Providers.AZURE comparison - Add 9 tests covering all four provider branches, Bedrock exclusion, size limit enforcement, and unhandled provider * fix: resolve filename type mismatch in formatDocumentBlock filename parameter is string | undefined but OpenAIFileBlock and OpenAIInputFileBlock require string. Default to 'document' when filename is undefined. * fix: use endpoint name for file config lookup in processAttachments Agent runs can have agent.provider set to a base provider (e.g., openAI) while agent.endpoint is a custom endpoint name. Using provider for the getEndpointFileConfig lookup bypassed custom endpoint supportedMimeTypes config. Now uses agent.endpoint, matching the pattern in addDocuments. * perf: filter non-Bedrock files before fetching streams Bedrock only supports types in bedrockDocumentFormats. Previously, getFileStream was called for all files and unsupported types were discarded after download. Now pre-filters the file list for Bedrock to avoid unnecessary network and memory overhead for large unsupported attachments. * refactor: clean up processAttachments file config handling - Remove redundant ?? null intermediaries; use instance properties directly in the else-if condition - Add JSDoc @type annotations for _mergedFileConfig and _endpointFileConfig in the constructor * refactor: harden document encoding and add routing tests - Hoist configuredFileSizeLimit above the loop to avoid recomputing mergeFileConfig per file - Replace Buffer.from decode with base64 length formula in the generic size check to avoid unnecessary heap allocation - Use nullish coalescing (??) for filename fallback - Clean up test: remove unnecessary type cast, use createMockRequest helper for size-limit test - Add 14 tests for processAttachments categorization logic covering supportedMimeTypes routing, null/undefined guards, standard type passthrough, and edge cases * fix: use optional chaining for checkType in routing tests FileConfig.checkType is typed as optional. Use optional chaining to satisfy strict type checking. * fix: skip stream fetches for unsupported providers, block Bedrock generic routing - Return early from encodeAndFormatDocuments when the provider is neither document-supported nor Bedrock, avoiding unnecessary getFileStream calls for providers that would discard all results - Add !isBedrock guard to the supportedMimeTypes fallback branch in processAttachments so permissive patterns like '.*' don't route non-Bedrock types into documents that would be silently dropped - Add test for Bedrock + non-Bedrock-document-type skipping * fix: respect supportedMimeTypes config for Bedrock endpoints Remove !isBedrock guard from the generic supportedMimeTypes routing branch. If a user configures permissive supportedMimeTypes for a Bedrock endpoint, the upload validation already accepted the file. The encoding layer pre-filters to Bedrock-supported types before fetching streams, so unsupported types are handled there without silently dropping files the user explicitly allowed. |
||
|
Danny Avila
|
275af48592
|
🎯 fix: MCP Tool Misclassification from Action Delimiter Collision (#12512)
* fix: prevent MCP tools with `_action` in name from being misclassified as OpenAPI action tools
Add `isActionTool()` helper that checks for the `_action_` delimiter
while guarding against cross-delimiter collision with `_mcp_`. Replace
all `includes(actionDelimiter)` classification checks with the new
helper across backend and frontend.
* test: add coverage for MCP/action cross-delimiter collision
Verify that `isActionTool` correctly rejects MCP tool names containing
`_action` and that `loadAgentTools` does not filter them based on
`actionsEnabled`. Add ToolIcon and definitions test cases.
* fix: simplify isActionTool to handle all MCP name patterns
- Use `!toolName.includes('_mcp_')` instead of checking only after the
first `_action_` occurrence, which missed MCP tools with `_action_` in
the middle of their name (e.g. `get_action_data_mcp_myserver`).
- Reference `Constants.mcp_delimiter` value via a local const to avoid
circular import from config.ts, with a comment explaining why.
- Remove dead `actionDelimiter` import from definitions.ts.
- Replace double-filter with single-pass partition in loadToolsForExecution.
- Add test for mid-name `_action_` collision case.
* fix: narrow MCP exclusion to delimiter position in isActionTool
Only reject when `_mcp_` appears after `_action_` (the MCP suffix
position). `_mcp_` before `_action_` is part of the operationId and
is valid — e.g. `sync_mcp_state_action_api---example---com` is a
legitimate action tool whose operationId happens to contain `_mcp_`.
* fix: document positional _mcp_ guard and known RFC-invalid domain limitation
Expand JSDoc on isActionTool to explain the action/MCP format
disambiguation and the theoretical false negative for non-RFC-compliant
domains containing `_mcp_`. Add test documenting this known edge case.
|
||
|
Danny Avila
|
611a1ef5dc
|
🏖️ fix: Sandpack ExternalResources for Static HTML Artifact Previews (#12509)
* fix: omit externalResources for static Sandpack previews The Tailwind CDN URL lacks a file extension, causing Sandpack's static template to throw a runtime injection error. Static previews already load Tailwind via a script tag in the shared index.html, so externalResources is unnecessary for them. Closes #12507 * refactor: extract buildSandpackOptions and add tests - Surgically omit only externalResources for static templates instead of discarding all sharedOptions, preventing future regression if new template-agnostic options are added. - Extract options logic into a pure, testable helper function. - Add unit tests covering all template/config combinations. * chore: fix import order and pin test assertions * fix: use URL fragment hint instead of omitting externalResources Sandpack's static template regex detects resource type from the URL's last file extension. The versioned CDN path (/3.4.17) matched ".17" instead of ".js", throwing "Unable to determine file type". Rather than omitting externalResources for static templates (which would remove the only Tailwind injection path for HTML artifacts that don't embed their own script tag), append a #tailwind.js fragment hint so the regex matches ".js". Fragments are not sent to the server, so the CDN response is unchanged. |
||
|
Danny Avila
|
cb41ba14b2
|
🔁 fix: Pass recursionLimit to OpenAI-Compatible Agents API Endpoint (#12510)
* fix: pass recursionLimit to processStream in OpenAI-compatible agents API The OpenAI-compatible endpoint never passed recursionLimit to LangGraph's processStream(), silently capping all API-based agent calls at the default 25 steps. Mirror the 3-step cascade already used by the UI path (client.js): yaml config default → per-agent DB override → max cap. * refactor: extract resolveRecursionLimit into shared utility Extract the 3-step recursion limit cascade into a shared resolveRecursionLimit() function in @librechat/api. Both openai.js and client.js now call this single source of truth. Also fixes falsy-guard edge cases where recursion_limit=0 or maxRecursionLimit=0 would silently misbehave, by using explicit typeof + positive checks. Includes unit tests covering all cascade branches and edge cases. * refactor: use resolveRecursionLimit in openai.js and client.js Replace duplicated cascade logic in both controllers with the shared resolveRecursionLimit() utility from @librechat/api. In openai.js: hoist agentsEConfig to avoid double property walk, remove displaced comment, add integration test assertions. In client.js: remove inline cascade that was overriding config after initial assignment. * fix: hoist processStream mock for test accessibility The processStream mock was created inline inside mockResolvedValue, making it inaccessible via createRun.mock.results (which returns the Promise, not the resolved value). Hoist it to a module-level variable so tests can assert on it directly. * test: improve test isolation and boundary coverage Use mockReturnValueOnce instead of mockReturnValue to prevent mock leaking across test boundaries. Add boundary tests for downward agent override and exact-match maxRecursionLimit. |
||
|
Danny Avila
|
aa575b274b
|
🛡️ refactor: Self-Healing Tenant Isolation Update Guard (#12506)
* refactor: self-healing tenant isolation update guard Replace the strict throw-on-any-tenantId guard with a strip-or-throw approach: - $set/$setOnInsert: strip when value matches current tenant or no context is active; throw only on cross-tenant mutations - $unset/$rename: always strip (unsetting/renaming tenantId is never valid) - Top-level tenantId: same logic as $set This eliminates the entire class of "tenantId in update payload" bugs at the plugin level while preserving the cross-tenant security invariant. * test: update mutation guard tests for self-healing behavior - Convert same-tenant $set/$setOnInsert tests to expect silent stripping instead of throws - Convert $unset test to expect silent stripping - Add cross-tenant throw tests for $set, $setOnInsert, top-level - Add same-tenant stripping tests for $set, $setOnInsert, top-level - Add $rename stripping test - Add no-context stripping test - Update error message assertions to match new cross-tenant message * revert: remove call-site tenantId stripping patches Revert the per-call-site tenantId stripping from #12498 and the excludedKeys patch from #12501. These are no longer needed since the self-healing guard handles tenantId in update payloads at the plugin level. Reverted patches: - conversation.ts: delete update.tenantId in saveConvo(), tenantId destructuring in bulkSaveConvos() - message.ts: delete update.tenantId in saveMessage() and recordMessage(), tenantId destructuring in bulkSaveMessages() and updateMessage() - config.ts: tenantId in excludedKeys Set - config.spec.ts: tenantId in excludedKeys test assertion * fix: strip tenantId from update documents in tenantSafeBulkWrite Mongoose middleware does not fire for bulkWrite, so the plugin-level guard never sees update payloads in bulk operations. Extend injectTenantId() to strip tenantId from update documents for updateOne/updateMany operations, preventing cross-tenant overwrites. * refactor: rename guard, add empty-op cleanup and strict-mode warning - Rename assertNoTenantIdMutation to sanitizeTenantIdMutation - Remove empty operator objects after stripping to avoid MongoDB errors - Log warning in strict mode when stripping tenantId without context - Fix $setOnInsert test to use upsert:true with non-matching filter * test: fix bulk-save tests and add negative excludedKeys assertion - Wrap bulkSaveConvos/bulkSaveMessages tests in tenantStorage.run() to exercise the actual multi-tenant stripping path - Assert tenantId equals the real tenant, not undefined - Add negative assertion: excludedKeys must NOT contain tenantId * fix: type-safe tenantId stripping in tenantSafeBulkWrite - Fix TS2345 error: replace conditional type inference with UpdateQuery<Record<string, unknown>> for stripTenantIdFromUpdate - Handle empty updates after stripping (e.g., $set: { tenantId } as sole field) by filtering null ops from the bulk array - Add 4 tests for bulk update tenantId stripping: plain-object update, $set stripping, $unset stripping, and sole-field-in-$set edge case * fix: resolve TS2345 in stripTenantIdFromUpdate parameter type Use Record<string, unknown> instead of UpdateQuery<> to avoid type incompatibility with Mongoose's AnyObject-based UpdateQuery resolution in CI. * fix: strip tenantId from bulk updates unconditionally Separate sanitization from injection in tenantSafeBulkWrite: tenantId is now stripped from all update documents before any tenant-context checks, closing the gap where no-context and system-context paths passed caller-supplied tenantId through to MongoDB unmodified. * refactor: address review findings in tenant isolation - Fix early-return gap in stripTenantIdFromUpdate that skipped operator-level tenantId when top-level was also present - Lazy-allocate copy in stripTenantIdFromUpdate (no allocation when no tenantId is present) - Document behavioral asymmetry: plugin throws on cross-tenant, bulkWrite strips silently (intentional, documented in JSDoc) - Remove double JSDoc on injectTenantId - Remove redundant cast in stripTenantIdFromUpdate - Use shared frozen EMPTY_BULK_RESULT constant - Remove Record<string, unknown> annotation in recordMessage - Isolate bulkSave* tests: pre-create docs then update with cross-tenant payload, read via runAsSystem to prove stripping is independent of filter injection * fix: no-op empty updates after tenantId sanitization When tenantId is the sole field in an update (e.g., { $set: { tenantId } }), sanitization leaves an empty update object that would fail with "Update document requires atomic operators." The updateGuard now detects this and short-circuits the query by adding an unmatchable filter condition and disabling upsert, matching the bulk-write handling that filters out null ops. * refactor: remove dead logger.warn branches, add mixed-case test - Remove unreachable logger.warn calls in sanitizeTenantIdMutation: queryMiddleware throws before updateGuard in strict+no-context, and isStrict() is false in non-strict+no-context - Add test for combined top-level + operator-level tenantId stripping to lock in the early-return fix * feat: ESLint rule to ban raw bulkWrite and collection.* in data-schemas Add no-restricted-syntax rules to the data-schemas ESLint config that flag direct Model.bulkWrite() and Model.collection.* calls. These bypass Mongoose middleware and the tenant isolation plugin — all bulk writes must use tenantSafeBulkWrite() instead. Test files are excluded since they intentionally use raw driver calls for fixture setup. Also migrate the one remaining raw bulkWrite in seedSystemGrants() to use tenantSafeBulkWrite() for consistency. * test: add findByIdAndUpdate coverage to mutation guard tests * fix: keep tenantSafeBulkWrite in seedSystemGrants, fix ESLint config - Revert to tenantSafeBulkWrite in seedSystemGrants (always runs under runAsSystem, so the wrapper passes through correctly) - Split data-schemas ESLint config: shared TS rules for all files, no-restricted-syntax only for production non-wrapper files - Fix unused destructure vars to use _tenantId pattern |
||
|
Danny Avila
|
7b368916d5
|
🔑 fix: Auth-Aware Startup Config Caching for Fresh Sessions (#12505)
* fix: auth-aware config caching for fresh sessions - Add auth state to startup config query key via shared `startupConfigKey` builder so login (unauthenticated) and chat (authenticated) configs are cached independently - Disable queries during login onMutate to prevent premature unauthenticated refetches after cache clear - Re-enable queries in setUserContext only after setTokenHeader runs, with positive-only guard to avoid redundant disable on logout - Update all getQueryData call sites to use the shared key builder - Fall back to getConfigDefaults().interface in useEndpoints, hoisted to module-level constant to avoid per-render recomputation * fix: address review findings for auth-aware config caching - Move defaultInterface const after all imports in ModelSelector.tsx - Remove dead QueryKeys import, use import type for TStartupConfig in ImportConversations.tsx - Spread real exports in useQueryParams.spec.ts mock to preserve startupConfigKey, fixing TypeError in all 6 tests * chore: import order * fix: re-enable queries on login failure When login fails, onSuccess never fires so queriesEnabled stays false. Re-enable in onError so the login page can re-fetch config (needed for LDAP username validation and social login options). |
||
|
Danny Avila
|
c4b5dedb77
|
🔒 fix: Exclude Unnecessary fields from Conversation $unset (#12501)
`BaseClient.js` iterates existing conversation keys to build `unsetFields` for removal when `endpointOptions` doesn't include them. When tenant isolation stamps `tenantId` on the document, it gets swept into `$unset`, triggering `assertNoTenantIdMutation`. Adding `tenantId` to `excludedKeys` prevents this — it's a system field, not an endpoint option. |
||
|
Danny Avila
|
5e789f589f
|
🔏 fix: Strip Unnecessary Fields Across Write Paths in Conversation & Message Methods (#12498)
* fix: Exclude field from conversation and message updates * fix: Remove tenantId from conversation and message update objects to prevent unintended data exposure. * refactor: Adjust update logic in createConversationMethods and createMessageMethods to ensure tenantId is not included in the updates, maintaining data integrity. * fix: Strip tenantId from all write paths in conversation and message methods Extends the existing tenantId stripping to bulkSaveConvos, bulkSaveMessages, recordMessage, and updateMessage — all of which previously passed caller-supplied tenantId straight through to the update document. Renames discard alias from _t to _tenantId for clarity. Adds regression tests for all six write paths. * fix: Eliminate double-copy overhead and strengthen test assertions Replace destructure-then-spread with spread-once-then-delete for saveConvo, saveMessage, and recordMessage — removes one O(n) copy per call on hot paths. Add missing not-null and positive data assertions to tenantId stripping tests. * test: Add positive data assertions to bulkSaveMessages and recordMessage tests |
||
|
Danny Avila
|
419613fdaf | 📋 chore: Move project instructions from AGENTS.md to CLAUDE.md | ||
|
Danny Avila
|
f8405e731b
|
🗂️ fix: Allow Empty-Overrides Scope Creation in Admin Config (#12492)
* fix: Allow empty-overrides scope creation when priority is provided
The upsertConfigOverrides handler short-circuited when overrides was
empty, returning a plain message instead of creating the config document.
This broke the admin panel's "create blank scope" flow which sends
`{ overrides: {}, priority: N }` — the missing `config` property in the
response caused an `_id` error on the client.
The early return now only triggers when both overrides are empty and no
priority is provided. Per-section permission checks are scoped to cases
where override sections are actually present.
* test: Add tests for empty-overrides scope creation with priority
* test: Address review nits for empty-overrides scope tests
- Add res.statusCode/res.body assertions to capability-check test
- Add 403/401 tests for empty overrides + priority path
- Use mockResolvedValue(null) for consistency on bare jest.fn()
- Remove narrating comment; fold intent into test name
|
||
|
Dustin Healy
|
2451bf54cf
|
🛡️ fix: Restrict System Grants to Role Principals (#12491)
* 🛡️ fix: restrict system grants to role principals only Narrows GrantPrincipalType to PrincipalType.ROLE, rejecting GROUP and USER with 400. Removes grant cascade cleanup from group/user deletion handlers and their route wiring since only roles can hold grants. * 🛡️ fix: address review findings for grants roles-only restriction Add missing GROUP rejection test for revokeGrant (symmetric with getPrincipalGrants and assignGrant coverage), add extensibility comment to GrantPrincipalType, and document the checkRoleExists guard. |
||
|
Danny Avila
|
2e706ebcb3
|
⚖️ refactor: Split Config Route into Unauthenticated and Authenticated Paths (#12490)
* refactor: split /api/config into unauthenticated and authenticated response paths
- Replace preAuthTenantMiddleware with optionalJwtAuth on the /api/config
route so the handler can detect whether the request is authenticated
- When unauthenticated: call getAppConfig({ baseOnly: true }) for zero DB
queries, return only login-relevant fields (social logins, turnstile,
privacy policy / terms of service from interface config)
- When authenticated: call getAppConfig({ role, userId, tenantId }) to
resolve per-user DB overrides (USER + ROLE + GROUP + PUBLIC principals),
return full payload including modelSpecs, balance, webSearch, etc.
- Extract buildSharedPayload() and addWebSearchConfig() helpers to avoid
duplication between the two code paths
- Fixes per-user balance overrides not appearing in the frontend because
userId was never passed to getAppConfig (follow-up to #12474)
* test: rewrite config route tests for unauthenticated vs authenticated paths
- Replace the previously-skipped supertest tests with proper mocked tests
- Cover unauthenticated path: baseOnly config call, minimal payload,
interface subset (privacyPolicy/termsOfService only), exclusion of
authenticated-only fields
- Cover authenticated path: getAppConfig called with userId, full payload
including modelSpecs/balance/webSearch, per-user balance override merging
* fix: address review findings — restore multi-tenant support, improve tests
- Chain preAuthTenantMiddleware back before optionalJwtAuth on /api/config
so unauthenticated requests in multi-tenant deployments still get
tenant-scoped config via X-Tenant-Id header (Finding #1)
- Use getAppConfig({ tenantId }) instead of getAppConfig({ baseOnly: true })
when a tenant context is present; fall back to baseOnly for single-tenant
- Fix @type annotation: unauthenticated payload is Partial<TStartupConfig>
- Refactor addWebSearchConfig into pure buildWebSearchConfig that returns a
value instead of mutating the payload argument
- Hoist isBirthday() to module level
- Remove inline narration comments
- Assert tenantId propagation in tests, including getTenantId fallback and
user.tenantId preference
- Add error-path tests for both unauthenticated and authenticated branches
- Expand afterEach env var cleanup for proper test isolation
* test: fix mock isolation and add tenant-scoped response test
- Replace jest.clearAllMocks() with jest.resetAllMocks() so
mockReturnValue implementations don't leak between tests
- Add test verifying tenant-scoped socialLogins and turnstile are
correctly mapped in the unauthenticated response
* fix: add optionalJwtAuth to /api/config in experimental.js
Without this middleware, req.user is never populated in the experimental
cluster entrypoint, so authenticated users always receive the minimal
unauthenticated config payload.
|
||
|
Danny Avila
|
7181174c3b
|
🖼️ fix: Message Icon Flickering from Context-Triggered Re-renders (#12489)
* perf: add custom memo comparator to MessageIcon for stable re-render gating MessageIcon receives full `agent` and `assistant` objects as props from useMessageActions, which recomputes them when AgentsMapContext or AssistantsMapContext update (e.g., react-query refetch on window focus). These context-triggered re-renders bypass MessageRender's React.memo, producing new object references for agent/assistant even when the underlying data is unchanged. The default shallow comparison in MessageIcon's memo then fails, causing unnecessary re-renders that manifest as visible icon flickering. Add arePropsEqual comparator that checks only the fields MessageIcon actually uses (name, avatar filepath, metadata avatar) instead of object identity, so the component correctly bails out when icon-relevant data hasn't changed. * refactor: export arePropsEqual, drop redundant useMemos, add JSDoc - Export arePropsEqual so it can be tested in isolation - Add JSDoc documenting which fields are intentionally omitted (id) and why iconData uses reference equality - Replace five trivial useMemo calls (agent?.name ?? '', etc.) with direct computed values — the custom comparator already gates re-renders, so these memos only add closure/dep-array overhead without ever providing cache hits - Remove unused React import * test: add unit tests for MessageIcon arePropsEqual comparator Exercise the custom memo comparator to ensure: - New object references with same display fields are treated as equal - Changed name or avatar filepath triggers re-render - iconData reference change triggers re-render - undefined→defined agent with undefined fields is treated as equal * fix: replace nested ternary with if-else for eslint compliance * test: add comment on subtle equality invariant and defined→undefined case * perf: compare iconData by field values instead of reference iconData is a useMemo'd object from the parent, but comparing by reference still causes unnecessary re-renders when the parent recomputes the memo with identical primitive values. Compare all five fields individually (endpoint, model, iconURL, modelLabel, isCreatedByUser) for consistency with how agent/assistant are handled. |
||
|
Danny Avila
|
aa7e5ba051
|
📦 chore: bump axios to exact v1.13.6, @librechat/agents to v3.1.63, @aws-sdk/client-bedrock-runtime to v3.1013.0 (#12488)
* 🔧 chore: bump @librechat/agents to v3.1.63 * 🔧 chore: update axios dependency to exact version 1.13.6 * 🔧 chore: update @aws-sdk/client-bedrock-runtime to version 3.1013.0 in package.json and package-lock.json - Bump the version of @aws-sdk/client-bedrock-runtime across package.json files in api and packages/api to ensure compatibility with the latest features and fixes. - Reflect the updated version in package-lock.json to maintain consistency in dependency resolution. * 🔧 chore: update axios dependency to version 1.13.6 across multiple package.json and package-lock.json files - Bump axios version from ^1.13.5 to 1.13.6 in package.json and package-lock.json for improved performance and security. - Ensure consistency in dependency resolution across the project by updating all relevant files. |
||
|
Danny Avila
|
d9f216c11a
|
📦 chore: bump dependabot packages (#12487)
* chore: Update Handlebars and package versions in package-lock.json and package.json - Upgrade Handlebars from version 4.7.7 to 4.7.9 in both package-lock.json and package.json for improved performance and security. - Update librechat-data-provider version from 0.8.401 to 0.8.406 in package-lock.json. - Update @librechat/data-schemas version from 0.0.40 to 0.0.48 in package-lock.json. * chore: Upgrade @happy-dom/jest-environment and happy-dom versions in package-lock.json and package.json - Update @happy-dom/jest-environment from version 20.8.3 to 20.8.9 for improved compatibility. - Upgrade happy-dom from version 20.8.3 to 20.8.9 to ensure consistency across dependencies. * chore: Upgrade @rollup/plugin-terser to version 1.0.0 in package-lock.json and package.json - Update @rollup/plugin-terser from version 0.4.4 to 1.0.0 in both package-lock.json and package.json for improved performance and compatibility. - Reflect the new version in the dependencies of data-provider and data-schemas packages. * chore: Upgrade rollup-plugin-typescript2 to version 0.37.0 in package-lock.json and package.json - Update rollup-plugin-typescript2 from version 0.35.0 to 0.37.0 in package-lock.json and all relevant package.json files for improved compatibility and performance. - Adjust dependencies for semver and tslib to their latest versions in line with the rollup-plugin-typescript2 upgrade. * chore: Upgrade nodemailer to version 8.0.4 in package-lock.json and package.json - Update nodemailer from version 7.0.11 to 8.0.4 in both package-lock.json and package.json to enhance functionality and security. * chore: Upgrade picomatch, yaml, brace-expansion versions in package-lock.json - Update picomatch from version 4.0.3 to 4.0.4 across multiple dependencies for improved functionality. - Upgrade brace-expansion from version 2.0.2 to 2.0.3 and from 5.0.3 to 5.0.5 to enhance compatibility and performance. - Update yaml from version 1.10.2 to 1.10.3 for better stability. |
||
|
Danny Avila
|
c0ce7fee91
|
🚫 refactor: Remove Interface Config from Override Processing (#12473)
Add INTERFACE_PERMISSION_FIELDS set defining the interface fields that seed role permissions at startup (prompts, agents, marketplace, etc.). These fields are now stripped from DB config overrides in the merge layer because updateInterfacePermissions() only runs at boot — DB overrides for these fields create a client/server permission mismatch. Pure UI fields (endpointsMenu, modelSelect, parameters, presets, sidePanel, customWelcome, etc.) continue to work in overrides as before. YAML startup path is completely unaffected. |
||
|
Dustin Healy
|
3d1b883e9d
|
👨👨👦👦 feat: Admin Users API Endpoints (#12446)
* feat: add admin user management endpoints
Add /api/admin/users with list, search, and delete handlers gated by
ACCESS_ADMIN + READ_USERS/MANAGE_USERS system grants. Handler factory
in packages/api uses findUsers, countUsers, and deleteUserById from
data-schemas.
* fix: address convention violations in admin users handlers
* fix: add pagination, self-deletion guard, and DB-level search limit
- listUsers now uses parsePagination + countUsers for proper pagination
matching the roles/groups pattern
- findUsers extended with optional limit/offset options
- deleteUser returns 403 when caller tries to delete own account
- searchUsers passes limit to DB query instead of fetching all and
slicing in JS
- Fix import ordering per CLAUDE.md, complete logger mock
- Replace fabricated date fallback with undefined
* fix: deterministic sort, null-safe pagination, consistent search filter
- Add sort option to findUsers; listUsers sorts by createdAt desc for
deterministic pagination
- Use != null guards for offset/limit to handle zero values correctly
- Remove username from search filter since it is not in the projection
or AdminUserSearchResult response type
* fix: last-admin deletion guard and search query max-length
- Prevent deleting the last admin user (look up target role, count
admins, reject with 400 if count <= 1)
- Cap search query at 200 characters to prevent regex DoS
- Add tests for both guards
* fix: include missing capability name in 403 Forbidden response
* fix: cascade user deletion cleanup, search username, parallel capability checks
- Cascade Config, AclEntry, and SystemGrant cleanup on user deletion
(matching the pattern in roles/groups handlers)
- Add username to admin search $or filter for parity with searchUsers
- Parallelize READ_* capability checks in listAllGrants with Promise.all
* fix: TOCTOU safety net, capability info leak, DRY/style cleanup, data-layer tests
- Add post-delete admin recount with CRITICAL log if race leaves 0 admins
- Revert capability name from 403 response to server-side log only
- Document thin deleteUserById limitation (full cascade is a future task)
- DRY: extract query.trim() to local variable in searchUsersHandler
- Add username to search projection, response type, and AdminUserSearchResult
- Functional filter/map in grants.ts parallel capability check
- Consistent null guards and limit>0 guard in findUsers options
- Fallback for empty result.message on delete response
- Fix mockUser() to generate unique _id per call
- Break long destructuring across multiple lines
- Assert countUsers filter and non-admin skip in delete tests
- Add data-layer tests for findUsers limit, offset, sort, and pagination
* chore: comment out admin delete user endpoint (out of scope)
* fix: cast USER principalId to ObjectId for ACL entry cleanup
ACL entries store USER principalId as ObjectId (via grantPermission casting),
but deleteAclEntries is a raw deleteMany that passes the filter through.
Passing a string won't match stored ObjectIds, leaving orphaned entries.
* chore: comment out unused requireManageUsers alongside disabled delete route
* fix: add missing logger.warn mock in capabilities test
* fix: harden admin users handlers — type safety, response consistency, test coverage
- Unify response shape: AdminUserSearchResult.userId → id, add AdminUserListItem type
- Fix unsafe req.query type assertion in searchUsersHandler (typeof guards)
- Anchor search regex with ^ for prefix matching (enables index usage)
- Add total/capped to search response for truncation signaling
- Add parseInt radix, remove redundant new Date() wrap
- Add tests: countUsers throw, countUsers call args, array query param, capped flag
* fix: scope deleteGrantsForPrincipal to tenant, deterministic search sort, align test mocks
- Add tenantId option to AdminUsersDeps.deleteGrantsForPrincipal and
pass req.user.tenantId at the call site, matching the pattern already
used by the roles and groups handlers
- Add sort: { name: 1 } to searchUsersHandler for deterministic results
- Align test mock deleteUserById messages with production output
('User was deleted successfully.')
- Make capped-results test explicitly set limit: '20' instead of
relying on the implicit default
* test: add tenantId propagation test for deleteGrantsForPrincipal
Add tenantId to createReqRes user type and test that a non-undefined
tenantId is threaded through to deleteGrantsForPrincipal.
* test: remove redundant deleteUserById override in tenantId test
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
Danny Avila
|
fd01dfc083
|
💰 fix: Lazy-Initialize Balance Record at Check Time for Overrides (#12474)
* fix: Lazy-initialize balance record when missing at check time When balance is configured via admin panel DB overrides, users with existing sessions never pass through the login middleware that creates their balance record. This causes checkBalanceRecord to find no record and return balance: 0, blocking the user. Add optional balanceConfig and upsertBalanceFields deps to CheckBalanceDeps. When no balance record exists but startBalance is configured, lazily create the record instead of returning canSpend: false. Pass the new deps from BaseClient, chatV1, and chatV2 callers. * test: Add checkBalance lazy initialization tests Cover lazy balance init scenarios: successful init with startBalance, insufficient startBalance, missing config fallback, undefined startBalance, missing upsertBalanceFields dep, and startBalance of 0. * fix: Address review findings for lazy balance initialization - Use canonical BalanceConfig and IBalanceUpdate types from @librechat/data-schemas instead of inline type definitions - Include auto-refill fields (autoRefillEnabled, refillIntervalValue, refillIntervalUnit, refillAmount, lastRefill) during lazy init, mirroring the login middleware's buildUpdateFields logic - Add try/catch around upsertBalanceFields with graceful fallback to canSpend: false on DB errors - Read balance from DB return value instead of raw startBalance constant - Fix misleading test names to describe observable throw behavior - Add tests: upsertBalanceFields rejection, auto-refill field inclusion, DB-returned balance value, and logViolation assertions * fix: Address second review pass findings - Fix import ordering: package type imports before local type imports - Remove misleading comment on DB-fallback test, rename for clarity - Add logViolation assertion to insufficient-balance lazy-init test - Add test for partial auto-refill config (autoRefillEnabled without required dependent fields) * refactor: Replace createMockReqRes factory with describe-scoped consts Replace zero-argument factory with plain const declarations using direct type casts instead of double-cast through unknown. * fix: Sort local type imports longest-first, add missing logViolation assertion - Reorder local type imports in spec file per AGENTS.md (longest to shortest within sub-group) - Add logViolation assertion to startBalance: 0 test for consistent violation payload coverage across all throw paths |
||
|
Danny Avila
|
4f37e8adb9
|
🔐 feat: Admin Auth Support for SAML and Social OAuth Providers (#12472)
* refactor: Add existingUsersOnly support to social and SAML auth callbacks
- Add `existingUsersOnly` option to the `socialLogin` handler factory to
reject unknown users instead of creating new accounts
- Refactor SAML strategy callback into `createSamlCallback(existingUsersOnly)`
factory function, mirroring the OpenID `createOpenIDCallback` pattern
- Extract shared SAML config into `getBaseSamlConfig()` helper
- Register `samlAdmin` passport strategy with `existingUsersOnly: true` and
admin-specific callback URL, called automatically from `setupSaml()`
* feat: Register admin OAuth strategy variants for all social providers
- Add admin strategy exports to Google, GitHub, Discord, Facebook, and
Apple strategy files with admin callback URLs and existingUsersOnly
- Extract provider configs into reusable helpers to avoid duplication
between regular and admin strategy constructors
- Re-export all admin strategy factories from strategies/index.js
- Register admin passport strategies (googleAdmin, githubAdmin, etc.)
alongside regular ones in socialLogins.js when env vars are present
* feat: Add admin auth routes for SAML and social OAuth providers
- Add initiation and callback routes for SAML, Google, GitHub, Discord,
Facebook, and Apple to the admin auth router
- Each provider follows the exchange code + PKCE pattern established by
OpenID admin auth: store PKCE challenge on initiation, retrieve on
callback, generate exchange code for the admin panel
- SAML and Apple use POST callbacks with state extracted from
req.body.RelayState and req.body.state respectively
- Extract storePkceChallenge(), retrievePkceChallenge(), and
generateState() helpers; refactor existing OpenID routes to use them
- All callback chains enforce requireAdminAccess, setBalanceConfig,
checkDomainAllowed, and the shared createOAuthHandler
- No changes needed to the generic POST /oauth/exchange endpoint
* fix: Update SAML strategy test to handle dual strategy registration
setupSaml() now registers both 'saml' and 'samlAdmin' strategies,
causing the SamlStrategy mock to be called twice. The verifyCallback
variable was getting overwritten with the admin callback (which has
existingUsersOnly: true), making all new-user tests fail.
Fix: capture only the first callback per setupSaml() call and reset
between tests.
* fix: Address review findings for admin OAuth strategy changes
- Fix existingUsersOnly rejection in socialLogin.js to use
cb(null, false, { message }) instead of cb(error), ensuring
passport's failureRedirect fires correctly for admin flows
- Consolidate duplicate require() calls in strategies/index.js by
destructuring admin exports from the already-imported default export
- Pass pre-parsed baseConfig to setupSamlAdmin() to avoid redundant
certificate file I/O at startup
- Extract getGoogleConfig() helper in googleStrategy.js for consistency
with all other provider strategy files
- Replace randomState() (openid-client) with generateState() (crypto)
in the OpenID admin route for consistency with all other providers,
and remove the now-unused openid-client import
* Reorder import statements in auth.js
|
||
|
Danny Avila
|
2bf0f892d6
|
🛡️ fix: Add Origin Binding to Admin OAuth Exchange Codes (#12469)
* fix(auth): add origin binding to admin OAuth exchange codes Bind admin OAuth exchange codes to the admin panel's origin at generation time and validate the origin on redemption. This prevents an intercepted code (via referrer leakage, logs, or network capture) from being redeemed by a different origin within the 30-second TTL. - Store the admin panel origin alongside the exchange code in cache - Extract the request origin (from Origin/Referer headers) on the exchange endpoint and pass it for validation - Reject code redemption when the request origin does not match the stored origin (code is still consumed to prevent replay) - Backward compatible: codes without a stored origin are accepted * fix(auth): add PKCE proof-of-possession to admin OAuth exchange codes Add a PKCE-like code_challenge/code_verifier flow to the admin OAuth exchange so that intercepting the exchange code alone is insufficient to redeem it. The admin panel generates a code_verifier (stored in its HttpOnly session cookie) and sends sha256(verifier) as code_challenge through the OAuth initiation URL. LibreChat stores the challenge keyed by OAuth state and attaches it to the exchange code. On redemption, the admin panel sends the verifier and LibreChat verifies the hash match. - Add verifyCodeChallenge() helper using SHA-256 - Store code_challenge in ADMIN_OAUTH_EXCHANGE cache (pkce: prefix, 5min TTL) - Capture OAuth state in callback middleware before passport processes it - Accept code_verifier in exchange endpoint body - Backward compatible: no challenge stored → PKCE check skipped * fix(auth): harden PKCE and origin binding in admin OAuth exchange - Await cache.set for PKCE challenge storage with error handling - Use crypto.timingSafeEqual for PKCE hash comparison - Drop case-insensitive flag from hex validation regexes - Add code_verifier length validation (max 512 chars) - Normalize Origin header via URL parsing in resolveRequestOrigin - Add test for undefined requestOrigin rejection - Clarify JSDoc: hex-encoded SHA-256, not RFC 7636 S256 * fix(auth): fail closed on PKCE callback cache errors, clean up origin/buffer handling - Callback middleware now redirects to error URL on cache.get failure instead of silently continuing without PKCE challenge - resolveRequestOrigin returns undefined (not raw header) on parse failure - Remove dead try/catch around Buffer.from which never throws for string input * chore(auth): remove narration comments, scope eslint-disable to lines * chore(auth): narrow query.state to string, remove narration comments in exchange.ts * fix(auth): address review findings — warn on missing PKCE challenge, validate verifier length, deduplicate URL parse - Log warning when OAuth state is present but no PKCE challenge found - Add minimum length check (>= 1) on code_verifier input validation - Update POST /oauth/exchange JSDoc to document code_verifier param - Deduplicate new URL(redirectUri) parse in createOAuthHandler - Restore intent comment on pre-delete pattern in exchangeAdminCode * test(auth): replace mock cache with real Keyv, remove all as-any casts - Use real Keyv in-memory store instead of hand-rolled Map mock - Replace jest.fn mocks with jest.spyOn on real Keyv instance - Remove redundant store.has() assertion, use cache.get() instead - Eliminate all eslint-disable and as-any suppressions - User fixture no longer needs any cast (Keyv accepts plain objects) * fix(auth): add IUser type cast for test fixture to satisfy tsc |
||
|
Danny Avila
|
1455f15b7b
|
📄 feat: Model-Aware Bedrock Document Size Validation (#12467)
* 📄 fix: Model-Aware Bedrock Document Size Validation
Remove the hard 4.5MB clamp on Bedrock document uploads so that
Claude 4+ (PDF) and Nova (PDF/DOCX) models can accept larger files
per AWS documentation. The default 4.5MB limit is preserved for
other models/formats, and fileConfig can now override it in either
direction—consistent with every other provider.
* address review: restore Math.min for non-exempt docs, tighten regexes, add tests
- Restore Math.min clamp for non-exempt Bedrock documents (fileConfig can
only lower the hard 4.5 MB API limit, not raise it); only exempt models
(Claude 4+ PDF, Nova PDF/DOCX) use ?? to allow fileConfig override
- Replace copied isBedrockClaude4Plus regex with cleaner anchored pattern
that correctly handles multi-digit version numbers (e.g. sonnet-40)
and removes dead Alt 1 branch matching no real Bedrock model IDs
- Tighten isBedrockNova from includes() to startsWith() to prevent
substring matching in unexpected positions
- Add integration test verifying model is threaded to validateBedrockDocument
- Add boundary tests for exempt + low configuredFileSizeLimit, non-exempt
+ high configuredFileSizeLimit, and exempt model accepting files up to 32 MB
- Revert two tests that were incorrectly inverted to prove wrong behavior
- Fix inaccurate JSDoc and misleading test name
* simplify: allow fileConfig to override Bedrock limit in either direction
Make Bedrock consistent with all other providers — fileConfig sets the
effective limit unconditionally via ?? rather than clamping with Math.min.
The model-aware defaults (4.5 MB for non-exempt, 32 MB for exempt) remain
as sensible fallbacks when no fileConfig is set.
* fix: handle cross-region inference profile IDs in Bedrock model matchers
Bedrock cross-region inference profiles prepend a region code to the
model ID (e.g. "us.amazon.nova-pro-v1:0", "eu.anthropic.claude-sonnet-4-...").
Both isBedrockNova and isBedrockClaude4Plus would miss these prefixed IDs,
silently falling back to the 4.5 MB default for eligible models.
Switch both matchers to use (?:^|\.) to anchor the vendor segment so the
pattern matches with or without a leading region prefix.
|
||
|
Danny Avila
|
fda72ac621
|
🏗️ refactor: Remove Redundant Caching, Migrate Config Services to TypeScript (#12466)
* ♻️ refactor: Remove redundant scopedCacheKey caching, support user-provided key model fetching Remove redundant cache layers that used `scopedCacheKey()` (tenant-only scoping) on top of `getAppConfig()` which already caches per-principal (role+user+tenant). This caused config overrides for different principals within the same tenant to be invisible due to stale cached data. Changes: - Add `requireJwtAuth` to `/api/endpoints` route for proper user context - Remove ENDPOINT_CONFIG, STARTUP_CONFIG, PLUGINS, TOOLS, and MODELS_CONFIG cache layers — all derive from `getAppConfig()` with cheap computation - Enhance MODEL_QUERIES cache: hash(baseURL+apiKey) keys, 2-minute TTL, caching centralized in `fetchModels()` base function - Support fetching models with user-provided API keys in `loadConfigModels` via `getUserKeyValues` lookup (no caching for user keys) - Update all affected tests Closes #1028 * ♻️ refactor: Migrate config services to TypeScript in packages/api Move core config logic from CJS /api wrappers to typed TypeScript in packages/api using dependency injection factories: - `createEndpointsConfigService` — endpoint config merging + checkCapability - `createLoadConfigModels` — custom endpoint model loading with user key support - `createMCPToolCacheService` — MCP tool cache operations (update, merge, cache) /api files become thin wrappers that wire dependencies (getAppConfig, loadDefaultEndpointsConfig, getUserKeyValues, getCachedTools, etc.) into the typed factories. Also moves existing `endpoints/config.ts` → `endpoints/config/providers.ts` to accommodate the new `config/` directory structure. * 🔄 fix: Invalidate models query when user API key is set or revoked Without this, users had to refresh the page after entering their API key to see the updated model list fetched with their credentials. - Invalidate QueryKeys.models in useUpdateUserKeysMutation onSuccess - Invalidate QueryKeys.models in useRevokeUserKeyMutation onSuccess - Invalidate QueryKeys.models in useRevokeAllUserKeysMutation onSuccess * 🗺️ fix: Remap YAML-level override keys to AppConfig equivalents in mergeConfigOverrides Config overrides stored in the DB use YAML-level keys (TCustomConfig), but they're merged into the already-processed AppConfig where some fields have been renamed by AppService. This caused mcpServers overrides to land on a nonexistent key instead of mcpConfig, so config-override MCP servers never appeared in the UI. - Add OVERRIDE_KEY_MAP to remap mcpServers→mcpConfig, interface→interfaceConfig - Apply remapping before deep merge in mergeConfigOverrides - Add test for YAML-level key remapping behavior - Update existing tests to use AppConfig field names in assertions * 🧪 test: Update service.spec to use AppConfig field names after override key remapping * 🛡️ fix: Address code review findings — reliability, types, tests, and performance - Pass tenant context (getTenantId) in importers.js getEndpointsConfig call - Add 5 tests for user-provided API key model fetching (key found, no key, DB error, missing userId, apiKey-only with fixed baseURL) - Distinguish NO_USER_KEY (debug) from infrastructure errors (warn) in catch - Switch fetchPromisesMap from Promise.all to Promise.allSettled so one failing provider doesn't kill the entire model config - Parallelize getUserKeyValues DB lookups via batched Promise.allSettled instead of sequential awaits in the loop - Hoist standardCache instance in fetchModels to avoid double instantiation - Replace Record<string, unknown> types with Partial<TConfig>-based types; remove as unknown as T double-cast in endpoints config - Narrow Bedrock availableRegions to typed destructure - Narrow version field from string|number|undefined to string|undefined - Fix import ordering in mcp/tools.ts and config/models.ts per AGENTS.md - Add JSDoc to getModelsConfig alias clarifying caching semantics * fix: Guard against null getCachedTools in mergeAppTools * 🔍 fix: Address follow-up review — deduplicate extractEnvVariable, fix error discrimination, add log-level tests - Deduplicate extractEnvVariable calls: resolve apiKey/baseURL once, reuse for both the entry and isUserProvided checks (Finding A) - Move ResolvedEndpoint interface from function closure to module scope (Finding B) - Replace fragile msg.includes('NO_USER_KEY') with ErrorTypes.NO_USER_KEY enum check against actual error message format (Finding C). Also handle ErrorTypes.INVALID_USER_KEY as an expected "no key" case. - Add test asserting logger.warn is called for infra errors (not debug) - Add test asserting logger.debug is called for NO_USER_KEY errors (not warn) * fix: Preserve numeric assistants version via String() coercion * 🐛 fix: Address secondary review — Ollama cache bypass, cache tests, type safety - Fix Ollama success path bypassing cache write in fetchModels (CRITICAL): store result before returning so Ollama models benefit from 2-minute TTL - Add 4 fetchModels cache behavior tests: cache write with TTL, cache hit short-circuits HTTP, skipCache bypasses read+write, empty results not cached - Type-safe OVERRIDE_KEY_MAP: Partial<Record<keyof TCustomConfig, keyof AppConfig>> so compiler catches future field rename mismatches - Fix import ordering in config/models.ts (package types longest→shortest) - Rename ToolCacheDeps → MCPToolCacheDeps for naming consistency - Expand getModelsConfig JSDoc to explain caching granularity * fix: Narrow OVERRIDE_KEY_MAP index to satisfy strict tsconfig * 🧩 fix: Add allowedProviders to TConfig, remove Record<string, unknown> from PartialEndpointEntry The agents endpoint config includes allowedProviders (used by the frontend AgentPanel to filter available providers), but it was missing from TConfig. This forced PartialEndpointEntry to use & Record<string, unknown> as an escape hatch, violating AGENTS.md type policy. - Add allowedProviders?: (string | EModelEndpoint)[] to TConfig - Remove Record<string, unknown> from PartialEndpointEntry — now just Partial<TConfig> * 🛡️ fix: Isolate Ollama cache write from fetch try-catch, add Ollama cache tests - Separate Ollama fetch and cache write into distinct scopes so a cache failure (e.g., Redis down) doesn't misattribute the error as an Ollama API failure and fall through to the OpenAI-compatible path (Issue A) - Add 2 Ollama-specific cache tests: models written with TTL on fetch, cached models returned without hitting server (Issue B) - Replace hardcoded 120000 with Time.TWO_MINUTES constant in cache TTL test assertion (Issue C) - Fix OVERRIDE_KEY_MAP JSDoc to accurately describe runtime vs compile-time type enforcement (Issue D) - Add global beforeEach for cache mock reset to prevent cross-test leakage * 🧪 fix: Address third review — DI consistency, cache key width, MCP tests - Inject loadCustomEndpointsConfig via EndpointsConfigDeps with default fallback, matching loadDefaultEndpointsConfig DI pattern (Finding 3) - Widen modelsCacheKey from 64-bit (.slice(0,16)) to 128-bit (.slice(0,32)) for collision-sensitive cross-credential cache key (Finding 4) - Add fetchModels.mockReset() in loadConfigModels.spec beforeEach to prevent mock implementation leaks across tests (Finding 5) - Add 11 unit tests for createMCPToolCacheService covering all three functions: null/empty input, successful ops, error propagation, cold-cache merge (Finding 2) - Simplify getModelsConfig JSDoc to @see reference (Finding 10) * ♻️ refactor: Address remaining follow-ups from reviews OVERRIDE_KEY_MAP completeness: - Add missing turnstile→turnstileConfig mapping - Add exhaustiveness test verifying all three renamed keys are remapped and original YAML keys don't leak through Import role context: - Pass userRole through importConversations job → importLibreChatConvo so role-based endpoint overrides are honored during conversation import - Update convos.js route to include req.user.role in the job payload createEndpointsConfigService unit tests: - Add 8 tests covering: default+custom merge, Azure/AzureAssistants/ Anthropic Vertex/Bedrock config enrichment, assistants version coercion, agents allowedProviders, req.config bypass Plugins/tools efficiency: - Use Set for includedTools/filteredTools lookups (O(1) vs O(n) per plugin) - Combine auth check + filter into single pass (eliminates intermediate array) - Pre-compute toolDefKeys Set for O(1) tool definition lookups * fix: Scope model query cache by user when userIdQuery is enabled * fix: Skip model cache for userIdQuery endpoints, fix endpoints test types - When userIdQuery is true, skip caching entirely (like user_provided keys) to avoid cross-user model list leakage without duplicating cache data - Fix AgentCapabilities type error in endpoints.spec.ts — use enum values and appConfig() helper for partial mock typing * 🐛 fix: Restore filteredTools+includedTools composition, add checkCapability tests - Fix filteredTools regression: whitelist and blacklist are now applied independently (two flat guards), matching original behavior where includedTools=['a','b'] + filteredTools=['b'] produces ['a'] (Finding A) - Fix Set spread in toolkit loop: pre-compute toolDefKeysList array once alongside the Set, reuse for .some() without per-plugin allocation (Finding B) - Add 2 filteredTools tests: blacklist-only path and combined whitelist+blacklist composition (Finding C) - Add 3 checkCapability tests: capability present, capability absent, fallback to defaultAgentCapabilities for non-agents endpoints (Finding D) * 🔑 fix: Include config-override MCP servers in filterAuthorizedTools Config-override MCP servers (defined via admin config overrides for roles/groups) were rejected by filterAuthorizedTools because it called getAllServerConfigs(userId) without the configServers parameter. Only YAML and DB-backed user servers were included in the access check. - Add configServers parameter to filterAuthorizedTools - Resolve config servers via resolveConfigServers(req) at all 4 callsites (create, update, duplicate, revert) using parallel Promise.all - Pass configServers through to getAllServerConfigs(userId, configServers) so the registry merges config-source servers into the access check - Update filterAuthorizedTools.spec.js mock for resolveConfigServers * fix: Skip model cache for userIdQuery endpoints, fix endpoints test types For user-provided key endpoints (userProvide: true), skip the full model list re-fetch during message validation — the user already selected from a list we served them, and re-fetching with skipCache:true on every message send is both slow and fragile (5s provider timeout = rejected model). Instead, validate the model string format only: - Must be a string, max 256 chars - Must match [a-zA-Z0-9][a-zA-Z0-9_.:\-/@+ ]* (covers all known provider model ID formats while rejecting injection attempts) System-configured endpoints still get full model list validation as before. * 🧪 test: Add regression tests for filterAuthorizedTools configServers and validateModel filterAuthorizedTools: - Add test verifying configServers is passed to getAllServerConfigs and config-override server tools are allowed through - Guard resolveConfigServers in createAgentHandler to only run when MCP tools are present (skip for tool-free agent creates) validateModel (12 new tests): - Format validation: missing model, non-string, length overflow, leading special char, script injection, standard model ID acceptance - userProvide early-return: next() called immediately, getModelsConfig not invoked (regression guard for the exact bug this fixes) - System endpoint list validation: reject unknown model, accept known model, handle null/missing models config Also fix unnecessary backslash escape in MODEL_PATTERN regex. * 🧹 fix: Remove space from MODEL_PATTERN, trim input, clean up nits - Remove space character from MODEL_PATTERN regex — no real model ID uses spaces; prevents spurious violation logs from whitespace artifacts - Add model.trim() before validation to handle accidental whitespace - Remove redundant filterUniquePlugins call on already-deduplicated output - Add comment documenting intentional whitelist+blacklist composition - Add getUserKeyValues.mockReset() in loadConfigModels.spec beforeEach - Remove narrating JSDoc from getModelsConfig one-liner - Add 2 tests: trim whitespace handling, reject spaces in model ID * fix: Match startup tool loader semantics — includedTools takes precedence over filteredTools The startup tool loader (loadAndFormatTools) explicitly ignores filteredTools when includedTools is set, with a warning log. The PluginController was applying both independently, creating inconsistent behavior where the same config produced different results at startup vs plugin listing time. Restored mutually exclusive semantics: when includedTools is non-empty, filteredTools is not evaluated. * 🧹 chore: Simplify validateModel flow, note auth requirement on endpoints route - Separate missing-model from invalid-model checks cleanly: type+presence guard first, then trim+format guard (reviewer NIT) - Add route comment noting auth is required for role/tenant scoping * fix: Write trimmed model back to req.body.model for downstream consumers |
||
|
Dustin Healy
|
a4a17ac771
|
⛩️ feat: Admin Grants API Endpoints (#12438)
* feat: add System Grants handler factory with tests
Handler factory with 4 endpoints: getEffectiveCapabilities (expanded
capability set for authenticated user), getPrincipalGrants (list grants
for a specific principal), assignGrant, and revokeGrant. Write ops
dynamically check MANAGE_ROLES/GROUPS/USERS based on target principal
type. 31 unit tests covering happy paths, validation, 403, and errors.
* feat: wire System Grants REST routes
Mount /api/admin/grants with requireJwtAuth + ACCESS_ADMIN gate.
Add barrel export for createAdminGrantsHandlers and AdminGrantsDeps.
* fix: cascade grant cleanup on role deletion
Add deleteGrantsForPrincipal to AdminRolesDeps and call it in
deleteRoleHandler via Promise.allSettled after successful deletion,
matching the groups cleanup pattern. 3 tests added for cleanup call,
skip on 404, and resilience to cleanup failure.
* fix: simplify cascade grant cleanup on role deletion
Replace Promise.allSettled wrapper with a direct try/catch for the
single deleteGrantsForPrincipal call.
* fix: harden grant handlers with auth, validation, types, and RESTful revoke
- Add per-handler auth checks (401) and granular capability gates
(READ_* for getPrincipalGrants, possession check for assignGrant)
- Extract validatePrincipal helper; rewrite validateGrantBody to use
direct type checks instead of unsafe `as string` casts
- Align DI types with data layer (ResolvedPrincipal.principalType
widened to string, getUserPrincipals role made optional)
- Switch revoke route from DELETE body to RESTful URL params
- Return 201 for assignGrant to match roles/groups create convention
- Handle null grantCapability return with 500
- Add comprehensive test coverage for new auth/validation paths
* fix: deduplicate ResolvedPrincipal, typed body, defensive auth checks
- Remove duplicate ResolvedPrincipal from capabilities.ts; import the
canonical export from grants.ts
- Replace Record<string, unknown> with explicit GrantRequestBody interface
- Add defensive 403 when READ_CAPABILITY_BY_TYPE lookup misses
- Document revoke asymmetry (no possession check) with JSDoc
- Use _id only in resolveUser (avoid Mongoose virtual reliance)
- Improve null-grant error message
- Complete logger mock in tests
* refactor: move ResolvedPrincipal to shared types to fix circular dep
Extract ResolvedPrincipal from admin/grants.ts to types/principal.ts
so middleware/capabilities.ts imports from shared types rather than
depending upward on the admin handler layer.
* chore: remove dead re-export, align logger mocks across admin tests
- Remove unused ResolvedPrincipal re-export from grants.ts (canonical
source is types/principal.ts)
- Align logger mocks in roles.spec.ts and groups.spec.ts to include
all log levels (error, warn, info, debug) matching grants.spec.ts
* fix: cascade Config and AclEntry cleanup on role deletion
Add deleteConfig and deleteAclEntries to role deletion cascade,
matching the group deletion pattern. Previously only grants were
cleaned up, leaving orphaned config overrides and ACL entries.
* perf: single-query batch for getEffectiveCapabilities
Add getCapabilitiesForPrincipals (plural) to the data layer — a single
$or query across all principals instead of N+1 parallel queries. Wire
it into the grants handler so getEffectiveCapabilities hits the DB once
regardless of how many principals the user has.
* fix: defer SystemCapabilities access to factory call time
Move all SystemCapabilities usage (VALID_CAPABILITIES,
MANAGE_CAPABILITY_BY_TYPE, READ_CAPABILITY_BY_TYPE) inside the
createAdminGrantsHandlers factory. External test suites that mock
@librechat/data-schemas without providing SystemCapabilities crashed
at import time when grants.ts was loaded transitively.
* test: add data-layer and handler test coverage for review findings
- Add 6 mongodb-memory-server tests for getCapabilitiesForPrincipals:
multi-principal batch, empty array, filtering, tenant scoping
- Add handler test: all principals filtered (only PUBLIC)
- Add handler test: granting an implied capability succeeds
- Add handler test: all cascade cleanup operations fail simultaneously
- Document platform-scope-only tenantId behavior in JSDoc
* fix: resolveUser fallback to user.id, early-return empty principals
- Match capabilities middleware pattern: _id?.toString() ?? user.id
to handle JWT-deserialized users without Mongoose _id
- Move empty-array guard before principals.map() to skip unnecessary
normalizePrincipalId calls
- Add comment explaining VALID_PRINCIPAL_TYPES module-scope asymmetry
* refactor: derive VALID_PRINCIPAL_TYPES from capability maps
Make MANAGE_CAPABILITY_BY_TYPE and READ_CAPABILITY_BY_TYPE
non-Partial Records over a shared GrantPrincipalType union, then
derive VALID_PRINCIPAL_TYPES from the map keys. This makes divergence
between the three data structures structurally impossible.
* feat: add GET /api/admin/grants list-all-grants endpoint
Add listAllGrants data-layer method and handler so the admin panel
can fetch all grants in a single request instead of fanning out
N+M calls per role and group. Response is filtered to only include
grants for principal types the caller has read access to.
* fix: update principalType to use GrantPrincipalType for consistency in grants handling
- Refactor principalType in createAdminGrantsHandlers to use GrantPrincipalType instead of PrincipalType for better type accuracy.
- Ensure type consistency across the grants handling logic in the API.
* fix: address admin grants review findings — tenantId propagation, capability validation, pagination, and test coverage
Propagate tenantId through all grant operations for multi-tenancy support.
Extract isValidCapability to accept full SystemCapability union (base, section,
assign) and reuse it in both Mongoose schema validation and handler input checks.
Replace listAllGrants with paginated listGrants + countGrants. Filter PUBLIC
principals from getCapabilitiesForPrincipals queries. Export getCachedPrincipals
from ALS store for fast-path principal resolution. Move DELETE capability param
to query string to avoid colon-in-URL issues. Remove dead code and add
comprehensive handler and data-layer test coverage.
* refactor: harden admin grants — FilterQuery types, auth-first ordering, DELETE path param, isValidCapability tests
Replace Record<string, unknown> with FilterQuery<ISystemGrant> across all
data-layer query filters. Refactor buildTenantFilter to a pure tenantCondition
function that returns a composable FilterQuery fragment, eliminating the $or
collision between tenant and principal queries. Move auth check before input
validation in getPrincipalGrantsHandler, assignGrantHandler, and
revokeGrantHandler to avoid leaking valid type names to unauthenticated callers.
Switch DELETE route from query param back to path param (/:capability) with
encodeURIComponent per project conventions. Add compound index for listGrants
sort. Type VALID_PRINCIPAL_TYPES as Set<GrantPrincipalType>. Remove unused
GetCachedPrincipalsFn type export. Add dedicated isValidCapability unit tests
and revokeGrant idempotency test.
* refactor: batch capability checks in listGrantsHandler via getHeldCapabilities
Replace 3 parallel hasCapabilityForPrincipals DB calls with a single
getHeldCapabilities query that returns the subset of capabilities any
principal holds. Also: defensive limit(0) clamp, parallelized assignGrant
auth checks, principalId type-vs-required error split, tenantCondition
hoisted to factory top, JSDoc on cascade deps, DELETE route encoding note.
* fix: normalize principalId and filter undefined in getHeldCapabilities
Add normalizePrincipalId + null guard to getHeldCapabilities, matching
the contract of getCapabilitiesForPrincipals. Simplify allCaps build
with flatMap, add no-tenantId cross-check and undefined-principalId
test cases.
* refactor: use concrete types in GrantRequestBody, rename encoding test
Replace unknown fields with explicit string types in GrantRequestBody,
matching the established pattern in roles/groups/config handlers. Rename
misleading 'encoded' test to 'with colons' since Express auto-decodes
req.params.
* fix: support hierarchical parent capabilities in possession checks
hasCapabilityForPrincipals and getHeldCapabilities now resolve parent
base capabilities for section/assignment grants. An admin holding
manage:configs can now grant manage:configs:<section> and transitively
read:configs:<section>. Fixes anti-escalation 403 blocking config
capability delegation.
* perf: use getHeldCapabilities in assignGrant to halve DB round-trips
assignGrantHandler was making two parallel hasCapabilityForPrincipals
calls to check manage + capability possession. getHeldCapabilities was
introduced in this PR specifically for this pattern. Replace with a
single batched call. Update corresponding spec assertions.
* fix: validate role existence before granting capabilities
Grants for non-existent role names were silently persisted, creating
orphaned grants that could surprise-activate if a role with that name
was later created. Add optional checkRoleExists dep to assignGrant and
wire it to getRoleByName in the route file.
* refactor: tighten principalType typing and use grantCapability in tests
Narrow getCapabilitiesForPrincipals parameter from string to
PrincipalType, removing the redundant cast. Replace direct
SystemGrant.create() calls in getCapabilitiesForPrincipals tests with
methods.grantCapability() to honor the schema's normalization invariant.
Add getHeldCapabilities extended capability tests.
* test: rename misleading cascade cleanup test name
The test only injects failure into deleteGrantsForPrincipal, not all
cascade operations. Rename from 'cascade cleanup fails' to 'grant
cleanup fails' to match the actual scope.
* fix: reorder role check after permission guard, add tenantId to index
Move checkRoleExists after the getHeldCapabilities permission check so
that a sub-MANAGE_ROLES admin cannot probe role name existence via
400 vs 403 response codes.
Add tenantId to the { principalType, capability } index so listGrants
queries in multi-tenant deployments can use a covering index instead
of post-scanning for tenant condition.
Add missing test for checkRoleExists throwing.
* fix: scope deleteGrantsForPrincipal to tenant on role deletion
deleteGrantsForPrincipal previously filtered only on principalType +
principalId, deleting grants across all tenants. Since the role schema
supports multi-tenancy (compound unique index on name + tenantId), two
tenants can share a role name like 'editor'. Deleting that role in one
tenant would wipe grants for identically-named roles in other tenants.
Add optional tenantId parameter to deleteGrantsForPrincipal. When
provided, scopes the delete to that tenant plus platform-level grants.
Propagate req.user.tenantId through the role deletion cascade.
* fix: scope grant cleanup to tenant on group deletion
Same cross-tenant gap as the role deletion path: deleteGroupHandler
called deleteGrantsForPrincipal without tenantId, so deleting a group
would wipe its grants across all tenants. Extract req.user.tenantId
and pass it through.
* test: add HTTP integration test for admin grants routes
Supertest-based test with real MongoMemoryServer exercising the full
Express wiring: route registration, injected auth middleware, handler
DI deps, and real DB round-trips.
Covers GET /, GET /effective, POST / + DELETE / lifecycle, role
existence validation, and 401 for unauthenticated callers.
Also documents the expandImplications scope: the /effective endpoint
returns base-level capabilities only; section-level resolution is
handled at authorization check time by getParentCapabilities.
* fix: use exact tenant match in deleteGrantsForPrincipal, normalize principalId, harden API
CRITICAL: deleteGrantsForPrincipal was using tenantCondition (a
read-query helper) for deleteMany, which includes the
{ tenantId: { $exists: false } } arm. This silently destroyed
platform-level grants when a tenant-scoped role/group deletion
occurred. Replace with exact { tenantId } match for deletes so
platform-level grants survive tenant-scoped cascade cleanup.
Refactor deleteGrantsForPrincipal signature from fragile positional
overload (sessionOrTenantId union + maybeSession) to a clean options
object: { tenantId?, session? }. Update all callers and test assertions.
Add normalizePrincipalId to hasCapabilityForPrincipals to match the
pattern already used by getHeldCapabilities — prevents string/ObjectId
type mismatch on USER/GROUP principal queries.
Also: export GrantPrincipalType from barrel, add upper-bound cap to
listGrants, document GROUP/USER existence check trade-off, add
integration tests for tenant-isolation property of deleteGrantsForPrincipal.
* fix: forward tenantId to getUserPrincipals in resolvePrincipals
resolvePrincipals had tenantId available from the caller but only
forwarded it to getCachedPrincipals (cache lookup). The DB fallback
via getUserPrincipals omitted it. While the Group schema's
applyTenantIsolation Mongoose plugin handles scoping via
AsyncLocalStorage in HTTP request context, explicitly passing tenantId
makes the contract visible and prevents silent cross-tenant group
resolution if called outside request context.
* fix: remove unused import and add assertion to 401 integration test
Remove unused SystemCapabilities import flagged by ESLint. Add explicit
body assertion to the 401 test so it has a jest expect() call.
* chore: hoist grant limit constants to scope, remove dead isolateModules
Move GRANTS_DEFAULT_LIMIT / GRANTS_MAX_LIMIT from inside listGrants
function body to createSystemGrantMethods scope so they are evaluated
once at module load. Remove dead jest.isolateModules + jest.doMock
block in integration test — the ~/models mock was never exercised
since handlers are built with explicit DI deps.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
|
||
|
github-actions[bot]
|
56d994e9ec
|
🌍 i18n: Update translation.json with latest translations (#12458)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Danny Avila
|
7e2b51697e
|
🪢 refactor: Eliminate Unnecessary Re-renders During Message Streaming (#12454)
* refactor: add TMessageChatContext type for stable context passing Defines a type for a stable context object that wrapper components pass to memo'd message components, avoiding direct ChatContext subscriptions that bypass React.memo during streaming. * perf: remove ChatContext subscription from useMessageActions useMessageActions previously called useChatContext() inside memo'd components (MessageRender, ContentRender), bypassing React.memo when isSubmitting changed during streaming. Now accepts a stable chatContext param instead, using a ref for the isSubmitting guard in regenerateMessage. Also stabilizes handleScroll in useMessageProcess by using a ref for isSubmitting instead of including it in useCallback deps. * perf: pass stable chatContext to memo'd message components Wrapper components (Message, MessageContent) now create a stable chatContext object via useMemo with a getter-backed isSubmitting, and compute effectiveIsSubmitting (false for non-latest messages). This ensures MessageRender and ContentRender (both React.memo'd) only re-render for the latest message during streaming, preventing unnecessary re-renders of all prior messages and their SubRow, HoverButtons, and SiblingSwitch children. * perf: add custom memo comparators to prevent message reference re-renders buildTree creates new message objects on every streaming update for ALL messages, not just the changed one. This defeats React.memo's default shallow comparison since the message prop has a new reference even when the content hasn't changed. Custom areEqual comparators now compare message by key fields (messageId, text, error, depth, children length, etc.) instead of reference equality, preventing unnecessary re-renders of SubRow, Files, HoverButtons and other children for non-latest messages. * perf: memoize ChatForm children to prevent streaming re-renders - Wrap StopButton in React.memo - Wrap AudioRecorder in React.memo, use ref for isSubmitting in onTranscriptionComplete callback to stabilize it - Remove useChatContext() from FileFormChat (bypassed its memo during streaming), accept files/setFiles/setFilesLoading as props from ChatForm instead * perf: stabilize ChatForm child props to prevent cascading re-renders ChatForm re-renders frequently during streaming (ChatContext changes). This caused StopButton and AttachFileChat/AttachFileMenu to re-render despite being memo'd, because their props were new references each time. - Wrap handleStopGenerating in a ref-based stable callback so StopButton always receives the same function reference - Create stableConversation via useMemo keyed on rendering-relevant fields only (conversationId, endpoint, agent_id, etc.), so AttachFileChat and FileFormChat don't re-render from unrelated conversation metadata updates (e.g., title generation) * perf: remove ChatContext subscription from AttachFileMenu and FileFormChat Both components used useFileHandling() which internally calls useChatContext(), bypassing their React.memo wrappers and causing re-renders on every streaming chunk. Switch to useFileHandlingNoChatContext() which accepts file state as parameters. The state (files, setFiles, setFilesLoading, conversation) is passed down from ChatForm → AttachFileChat → AttachFileMenu as props, keeping the memo chain intact. * fix: update imports and test mocks for useFileHandlingNoChatContext - Re-export useFileHandlingNoChatContext from hooks barrel - Import from ~/hooks instead of direct path for test compatibility - Add useToastContext mock to @librechat/client in AttachFileMenu tests since useFileHandlingNoChatContext runs the core hook which needs it - Add useFileHandlingNoChatContext to ~/hooks test mock * perf: fix remaining ChatForm streaming re-renders - Switch AttachFileMenu from useSharePointFileHandling (subscribes to ChatContext) to useSharePointFileHandlingNoChatContext with explicit file state props - Memoize ChatForm textarea onFocus/onBlur handlers with useCallback to prevent TextareaAutosize re-renders (inline arrow functions and .bind() created new references on every ChatForm render) - Update AttachFileMenu test mocks for new hook variants * refactor: add displayName to ChatForm for React DevTools * perf: prevent ChatForm re-renders during streaming via wrapper pattern ChatForm was re-rendering on every streaming chunk because it subscribed to useChatContext() internally, and the ChatContext value changed frequently during streaming. Extract context subscription into a ChatFormWrapper that: - Subscribes to useChatContext() (re-renders on every chunk, cheap) - Stabilizes conversation via selective useMemo - Stabilizes handleStopGenerating via ref-based callback - Passes individual stable values as props to ChatForm ChatForm (memo'd) now receives context values as props instead of subscribing directly. Since individual values (files, setFiles, isSubmitting, etc.) are stable references during streaming, ChatForm's memo prevents re-renders entirely — it only re-renders when isSubmitting actually toggles (2x per stream: start/end). * perf: stabilize newConversation prop and memoize CollapseChat - Wrap newConversation in ref-based stable callback in ChatFormWrapper (was the remaining unstable prop causing ChatForm to re-render) - Wrap CollapseChat in React.memo to prevent re-renders from parent * perf: memoize useAddedResponse return value useAddedResponse returned a new object literal on every render, causing AddedChatContext.Provider to trigger re-renders of all consumers (including ChatForm) on every streaming chunk. Wrap in useMemo so the context value stays referentially stable. * perf: memoize TextareaHeader to prevent re-renders from ChatForm * perf: address review findings for streaming render optimization Finding 1: Switch AttachFile.tsx from useFileHandling to useFileHandlingNoChatContext, closing the optimization hole for standard (non-agent) chat endpoints. Finding 2: Replace content reference equality with length comparison in both memo comparators — safer against buildTree array reconstruction. Finding 3: Add conversation?.model to stableConversation deps in ChatFormWrapper so file uploads use the correct model after switches. Finding 4/14: Fix stableNewConversation to explicitly return the underlying call's result instead of discarding it via `as` cast. Finding 5/6: Extract useMemoizedChatContext hook shared by Message.tsx and MessageContent.tsx — eliminates ~70 lines of duplication and stabilizes chatContext.conversation via selective useMemo to prevent post-stream metadata updates from re-rendering all messages. Finding 8: Use TMessage type for regenerate param instead of Record<string, unknown>. Finding 9: Use FileSetter alias in FileFormChat instead of inline type. Finding 11: Fix pre-existing broken throttle in useMessageProcess — was creating a new throttle instance per call, providing zero deduplication. Now retains the instance via useMemo. Finding 12: Initialize isSubmittingRef with chatContext.isSubmitting instead of false for consistency. Finding 13: Add ChatFormWrapper displayName. * fix: revert content comparison to reference equality in memo comparators The length-based comparison (content?.length) missed updates within existing content parts during streaming — text chunks update a part's content without changing the array length, so the comparator returned true and skipped re-renders for the latest message. Reference equality (===) is correct here: buildTree preserves content array references for unchanged messages via shallow spread, while React Query gives the latest message a new reference when its content updates during streaming. * fix: cancel throttled handleScroll on unmount and remove unused import * fix: use chatContext getter directly in regenerateMessage callback The local isSubmittingRef was stale for non-latest messages (which don't re-render during streaming by design). chatContext.isSubmitting is a getter backed by the wrapper's ref, so reading it at call-time always returns the current value regardless of whether the component has re-rendered. * fix: remove unused useCallback import from useMemoizedChatContext * fix: pass global isSubmitting to HoverButtons for action gating HoverButtons uses isSubmitting via useGenerationsByLatest to disable regenerate and hide edit buttons during streaming. Passing the effective value (false for non-latest messages) re-enabled those actions mid-stream, risking overlapping edits/regenerations. Use chatContext.isSubmitting (getter, always returns current value) for HoverButtons while keeping the effective value for rendering-only UI (cursor, placeholder, streaming indicator). * fix: address second review — stale HoverButtons, messages dep, cleanup - Add isSubmitting to chatContext useMemo deps in useMemoizedChatContext so HoverButtons correctly updates when streaming starts/ends (2 extra re-renders per session, belt-and-suspenders for post-stream state) - Change conversation?.messages?.length dep to boolean in ChatFormWrapper stableConversation — only need 0↔1+ transition for landing page check, not exact count on every message addition - Add defensive comment at chatContext destructuring point in useMessageActions explaining why isSubmitting must not be destructured - Remove dead mockUseFileHandling.mockReturnValue from AttachFileMenu tests * chore: remove dead useFileHandling mock artifacts from AttachFileMenu tests * fix: resolve eslint warnings for useMemo dependencies - Extract complex expression (conversation?.messages?.length ?? 0) > 0 to hasMessages variable for static analysis in ChatFormWrapper - Add eslint-disable for intentional isSubmitting dep in useMemoizedChatContext (forces new chatContext reference on streaming start/end so HoverButtons re-renders) |
||
|
Danny Avila
|
0d94881c2d
|
🧹 refactor: Tighten Config Schema Typing and Remove Deprecated Fields (#12452)
* refactor: Remove deprecated and unused fields from endpoint schemas - Remove summarize, summaryModel from endpointSchema and azureEndpointSchema - Remove plugins from azureEndpointSchema - Remove customOrder from endpointSchema and azureEndpointSchema - Remove baseURL from all and agents endpoint schemas - Type paramDefinitions with full SettingDefinition-based schema - Clean up summarize/summaryModel references in initialize.ts and config.spec.ts * refactor: Improve MCP transport schema typing - Add defaults to transport type discriminators (stdio, websocket, sse) - Type stderr field as IOType union instead of z.any() * refactor: Add narrowed preset schema for model specs - Create tModelSpecPresetSchema omitting system/DB/deprecated fields - Update tModelSpecSchema to use the narrowed preset schema * test: Add explicit type field to MCP test fixtures Add transport type discriminator to test objects that construct MCPOptions/ParsedServerConfig directly, required after type field changed from optional to default in schema definitions. * chore: Bump librechat-data-provider to 0.8.404 * refactor: Tighten z.record(z.any()) fields to precise value types - Type headers fields as z.record(z.string()) in endpoint, assistant, and azure schemas - Type addParams as z.record(z.union([z.string(), z.number(), z.boolean(), z.null()])) - Type azure additionalHeaders as z.record(z.string()) - Type memory model_parameters as z.record(z.union([z.string(), z.number(), z.boolean()])) - Type firecrawl changeTrackingOptions.schema as z.record(z.string()) * refactor: Type supportedMimeTypes schema as z.array(z.string()) Replace z.array(z.any()).refine() with z.array(z.string()) since config input is always strings that get converted to RegExp via convertStringsToRegex() after parsing. Destructure supportedMimeTypes from spreads to avoid string[]/RegExp[] type mismatch. * refactor: Tighten enum, role, and numeric constraint schemas - Type engineSTT as enum ['openai', 'azureOpenAI'] - Type engineTTS as enum ['openai', 'azureOpenAI', 'elevenlabs', 'localai'] - Constrain playbackRate to 0.25–4 range - Type titleMessageRole as enum ['system', 'user', 'assistant'] - Add int().nonnegative() to MCP timeout and firecrawl timeout * chore: Bump librechat-data-provider to 0.8.405 * fix: Accept both string and RegExp in supportedMimeTypes schema The schema must accept both string[] (config input) and RegExp[] (post-merge runtime) since tests validate merged output against the schema. Use z.union([z.string(), z.instanceof(RegExp)]) to handle both. * refactor: Address review findings for schema tightening PR - Revert changeTrackingOptions.schema to z.record(z.unknown()) (JSON Schema is nested, not flat strings) - Remove dead contextStrategy code from BaseClient.js and cleanup.js - Extract paramDefinitionSchema to named exported constant - Add .int() constraint to columnSpan and columns - Apply consistent .int().nonnegative() to initTimeout, sseReadTimeout, scraperTimeout - Update stale stderr JSDoc to match actual accepted types - Add comprehensive tests for paramDefinitionSchema, tModelSpecPresetSchema, endpointSchema deprecated field stripping, and azureEndpointSchema * fix: Address second review pass findings - Revert supportedMimeTypesSchema to z.array(z.string()) and remove as string[] casts — fix tests to not validate merged RegExp[] output against the config input schema - Remove unused tModelSpecSchema import from test file - Consolidate duplicate '../src/schemas' imports - Add expiredAt coverage to tModelSpecPresetSchema test - Assert plugins is absent in azureEndpointSchema test - Add sync comments for engineSTT/engineTTS enum literals * refactor: Omit preset-management fields from tModelSpecPresetSchema Omit conversationId, presetId, title, defaultPreset, and order from the model spec preset schema — these are preset-management fields that don't belong in model spec configuration. |
||
|
Danny Avila
|
f82d4300a4
|
🧹 chore: Remove Deprecated Gemini 2.0 Models & Fix Mistral-Large-3 Context Window (#12453)
* chore: remove deprecated Gemini 2.0 models from default models list Remove gemini-2.0-flash-001 and gemini-2.0-flash-lite from the Google default models array, as they have been deprecated by Google. Closes #12444 * fix: add mistral-large-3 max context tokens (256k) Add mistral-large-3 with 255000 max context tokens to the mistralModels map. Without this entry, the model falls back to the generic mistral-large key (131k), causing context window errors when using tools with Azure AI Foundry deployments. Closes #12429 * test: add mistral-large-3 token resolution tests and fix key ordering Add test coverage for mistral-large-3 context token resolution, verifying exact match, suffixed variants, and longest-match precedence over the generic mistral-large key. Reorder the mistral-large-3 entry after mistral-large to follow the file's documented convention of listing newer models last for reverse-scan performance. |