437 commits

Author	SHA1	Message	Date
Danny Avila	21ff5e79cc	🧪 test: Live Host-Pipeline Usage Verification (Env-Gated)	2026-06-11 00:44:47 -04:00
Danny Avila	1843cff325	📊 feat: Granular Tool Token Breakdown with Deferred Splits	2026-06-11 00:06:55 -04:00
Danny Avila	e5ab7cf1b1	🩹 fix: Resolve Codex Findings for Context Usage Tracking	2026-06-10 23:43:24 -04:00
Danny Avila	2c7f5c38af	📊 feat: Real-Time Context Window & Token Usage Tracking	2026-06-10 23:26:23 -04:00
Danny Avila	c27d6b85a4	🤫 refactor: Silent MCP OAuth Refresh on Mid-Session 401 (#13369) ... * 🤫 fix: Silent MCP OAuth Refresh on Mid-Session 401 Avoids the hourly interactive re-auth prompt when an MCP server (e.g. Azure Entra ID) returns 401 mid-session by attempting a refresh token exchange first, and only falling back to the interactive OAuth flow when no refresh token is stored or the refresh server rejects it. Resolves #13364. * fix: Use distinct flow type for silent token refresh to avoid cache hit Addresses the Codex review on PR #13369: `attemptSilentTokenRefresh` was reusing the `'mcp_get_tokens'` flow type, so `FlowStateManager.createFlowWithHandler` would short-circuit and return the same tokens cached by an earlier `getOAuthTokens` call — the very tokens the server just rejected — without executing the forced-refresh handler. Switch silent refresh to the distinct `'mcp_force_refresh_tokens'` flow type so coalescing still works but stale `mcp_get_tokens` cache entries are not reused. After a successful refresh, invalidate the `mcp_get_tokens` flow cache so the next `getOAuthTokens` call reads the freshly persisted tokens from storage rather than the stale cached value. Add a regression test that simulates the real `FlowStateManager.createFlowWithHandler` cache-hit behavior for `mcp_get_tokens` and verifies the silent refresh handler still runs and returns the freshly refreshed tokens. * fix: Address Codex round-2 review on silent MCP OAuth refresh Three follow-up findings from Codex on PR #13369: 1. The new `mcp_force_refresh_tokens` flow type was itself cached by `FlowStateManager.createFlowWithHandler`, so a subsequent 401 within the refreshed token's `expires_at` could re-serve the just-rejected token without ever re-running the refresh handler. 2. The factory's `oauthRequired` listener was removed immediately after the initial `attemptToConnect` succeeded, so a real mid-session 401 emitted by `MCPConnection.connectClient` during transport recovery had no listener — the OAuth handled-promise would simply time out instead of triggering the silent refresh. 3. Routing the silent refresh through a distinct flow type broke coalescing with the `mcp_get_tokens` lock used by `getOAuthTokens`, letting two paths concurrently redeem the same stored refresh token. For providers that rotate refresh tokens (e.g. Azure Entra) the second redemption is rejected, kicking the user back into interactive OAuth despite a successful refresh elsewhere. Resolution: - Drop `FlowStateManager` from the silent-refresh path entirely. Replace with a process-local `inflightSilentRefreshes` Map keyed by `userId:serverName` that holds only the in-flight Promise (no cached result), so every fresh 401 after settlement triggers a fresh redemption while concurrent 401s for the same user/server still share one redemption. - Stop calling `cleanupOAuthHandlers()` on successful initial connect, keeping the OAuth handler attached for the connection's lifetime so mid-session 401s actually reach `attemptSilentTokenRefresh`. - Add a regression test reproducing the stale-cache scenario by faking the `mcp_get_tokens` cache hit and asserting silent refresh still runs against storage and returns the fresh tokens. - Add a coalescing test asserting two concurrent oauthRequired events for the same user/server result in a single `forceRefreshTokens` call. - Clear `inflightSilentRefreshes` in `beforeEach` to prevent cross-test leakage; switch the silent-refresh test mocks to `mockResolvedValueOnce` / `mockImplementationOnce` so leftover mock state cannot leak into later test cases. Acknowledged remaining gap: the silent refresh still races `getOAuthTokens`'s `mcp_get_tokens` flow when both run concurrently (narrow window when an existing connection's local `expires_at` is still valid but the server invalidated the token, and a new connection is being created in parallel). The race is self-healing on the next 401 and documented inline. * fix: Address Codex round-3 review on silent MCP OAuth refresh Three more findings from Codex on PR #13369: 1. The in-flight silent-refresh promise was unbounded. If `forceRefreshTokens()` ever hung (slow provider, dropped TCP), the `inflightSilentRefreshes` lock stayed occupied forever and every later 401 for the same user/server joined the stuck promise instead of starting a fresh attempt or falling back to interactive OAuth. 2. The interactive-OAuth fallback didn't invalidate the `mcp_get_tokens` flow cache after persisting fresh tokens. For providers that don't issue refresh tokens (so silent refresh returns null), the old cache could still feed stale access tokens to the next `getOAuthTokens` call until its TTL expired — causing an immediate reconnect with the same just-rejected token. 3. When silent refresh failed, the handler fell through to `handleOAuthRequired()` whose recent-completion fast path can reuse a COMPLETED `mcp_oauth` flow within `PENDING_STALE_MS`. Those cached tokens are exactly the ones the server just rejected, so the connection would keep adopting them and looping on 401s until the cache aged out. Resolution: - Wrap `runSilentRefresh()` with a 60-second `withTimeout` (well under `connectClient`'s 120s OAuth timeout). On timeout the `.catch` resolves to null and the `finally` clears the in-flight entry, so the next 401 starts fresh and falls through to interactive OAuth. - Extract two helpers — `invalidateGetTokensFlow` and `invalidateCompletedOAuthFlow` — and call them from the right branches: clear `mcp_get_tokens` after silent-refresh success AND after interactive-OAuth `storeTokens`; clear the COMPLETED `mcp_oauth` state (plus its CSRF mapping) before falling through to interactive OAuth so the fast-reuse path can't re-serve the rejected tokens. - Add three regression tests: hung refresh release-the-lock under fake timers, completed-OAuth cache invalidation pre-fallback, and `mcp_get_tokens` invalidation after interactive token store. * fix: Address Codex round-4 review on silent MCP OAuth refresh Three more findings from Codex on PR #13369: 1. (P1) The silent-refresh in-flight lock keyed only by `userId:serverName`. In multi-tenant setups where two tenants share a userId (e.g. username-based IDs) and the same MCP server name, a concurrent mid-session 401 from tenant B would join tenant A's in-flight refresh and adopt tenant A's freshly minted tokens onto a tenant-B connection — a cross-tenant credential leak. 2. (P2) `invalidateGetTokensFlow` deleted the `mcp_get_tokens` flow state regardless of its status. When another connection was currently in `getOAuthTokens()` (PENDING flow) and joiners were monitoring it, the unconditional delete made those waiters see "Flow state not found" and unnecessarily fall back to interactive OAuth — even though fresh tokens were already being written. 3. (P2) The 60s `withTimeout` wrapping `runSilentRefresh()` only races the promise; it does not cancel the underlying `forceRefreshTokens` / refresh-token HTTP request. If the request returned after a subsequent interactive OAuth had stored newer tokens, the late completion would `storeTokens` over the newer state. This requires a provider that doesn't rotate refresh tokens AND a refresh slower than 60s AND a successful interactive OAuth in that window — narrow but real. Resolution: - Capture `getTenantId()` into a new `factory.tenantId` field at factory construction time (before the OAuth handler closes over it outside the original request's async context) and include it in the silent-refresh lock key as `tenantId:userId:serverName`. - `invalidateGetTokensFlow` now calls `getFlowState` first and only deletes when `status === 'COMPLETED'`. PENDING lookups are left alone so concurrent `getOAuthTokens` waiters via `monitorFlow` can still settle. - For (3), document the race as a known limitation inline. Fully closing it requires threading an `AbortSignal` through `MCPTokenStorage.forceRefreshTokens` and the OAuth refresh handler to skip the late `storeTokens` after timeout — out of scope for this PR's surgical change. - Add `getTenantId` to the `MCPOAuthConnectionEvents` test's `@librechat/data-schemas` mock so the factory constructor doesn't blow up under that suite. - Add three regression tests: per-tenant lock isolation, PENDING-state preservation under `invalidateGetTokensFlow`, and (reused) the existing interactive-store invalidation test now driven through `getFlowState` returning the COMPLETED state. * fix: Address silent MCP OAuth refresh review Restore captured tenant context around token storage and OAuth fallback paths so mid-session callbacks do not lose tenant scope. Thread AbortSignal through forced refresh and OAuth token requests, cap silent refresh by the connection OAuth timeout, and prevent timed-out refreshes from writing stale credentials after fallback. Complete pending mcp_get_tokens flows with fresh tokens, add missing FlowState createdAt test fixtures, and cover the new tenant/abort/cache behaviors. * fix: Tighten tenant-scoped MCP token refresh Cap silent refresh by both the factory connect timeout and the connection OAuth wait timeout so fallback OAuth wins before the outer connect attempt expires. Tenant-scope mcp_get_tokens flow ids for both token lookup and refresh invalidation, preventing cross-tenant flow completion or cache deletion when tenants share user ids and server names. Add regression tests for the omitted initTimeout budget and tenant-prefixed token flow locks. * fix: Reserve MCP OAuth fallback budget * fix: Harden MCP OAuth refresh races * fix: Keep MCP OAuth fallback route-compatible * test: Add SDK MCP OAuth refresh repro * fix: Address MCP OAuth refresh review findings * fix: Address MCP OAuth tenant review findings * fix: Close MCP OAuth route tenant gaps * fix: Preserve MCP OAuth refresh flow guards * fix: Avoid reprocessing MCP OAuth reauth config * fix: Release timed-out MCP refresh locks * fix: Release MCP OAuth request callbacks * fix: Tenant-scope remaining MCP OAuth flow lookups * ci: Sort imports in MCP OAuth test suites	2026-06-10 13:12:42 -04:00
Dustin Healy	5867f1a065	🛡️ feat: Configurable Message PII Filter (#13602) ... * 🛡️ feat: Reject chat messages matching configured credential patterns Adds an opt-in `messagePiiFilter` middleware mounted on the agent chat route ahead of `moderateText`. When the configured patterns match the user's input the request is refused with 400, so the credential never reaches OpenAI moderation, the model, or MongoDB. Three starter patterns ship by default and operators can subset them or add their own regex via `customPatterns` in librechat.yaml. * 🧪 test: Memoize compiled patterns + add middleware spec Memoize the compiled pattern array via a WeakMap keyed by the messagePiiFilter config object so repeat requests against the same config skip the per-request RegExp construction. Cache entries are released automatically when the config object itself rotates. Adds packages/api/src/middleware/messagePiiFilter.spec.ts covering the default-starter rejections, the starterPatterns subset and empty-array semantics, customPatterns matching layered on top of and in place of the starters, the no-config and empty-text pass-through paths, and a memoization regression check. * 🛡️ fix: Skip invalid customPattern regexes instead of crashing the request Admin DB overrides for `messagePiiFilter.customPatterns` reach `req.config` via `mergeConfigOverrides`, which deep-merges raw override values without re-running `configSchema`. A typo'd regex like `(` would slip past the YAML-load validation and throw inside `new RegExp(...)` during `compile()`, returning 500 for every chat request until the operator rolled the override back. Wrapped the per-pattern compile in a try/catch that logs the invalid pattern id + reason and skips it, so other valid patterns (starters and other custom entries) keep filtering. Added a regression test alongside the existing spec. * 🛡️ feat: Extend PII filter to OpenAI-compatible and Responses agent APIs The chat-route middleware operates on `req.body.text`, but the remote agent API endpoints (`/api/agents/v1/chat/completions`, `/api/agents/v1/responses`) accept the same prompt content as a `messages` array or an `input` field. A caller using their API key could send a credential-shaped value through either route and bypass the configured PII filter even though they share the same agent and model backbone the middleware is meant to guard. Factored out `findPiiMatchInMessages`, a tolerant walker that handles both `content: string` and `content: ContentPart[]` user-message shapes against the same compiled, cached pattern list. Wired it into the OpenAI-compat controller after agent lookup and into the Responses controller right after `convertToInternalMessages`. Each returns the endpoint's native 400 error shape (`sendErrorResponse` / `sendResponsesErrorResponse`) with the `message_pii_filter_block` code when a user message matches. * 🩹 test: Add findPiiMatchInMessages to OpenAI + Responses controller mocks The OpenAI-compat and Responses controller specs mock `@librechat/api` with a hand-listed object. The new `findPiiMatchInMessages` export wired into both controllers in 3ea35af9a was missing from those mocks, so the production lookup returned undefined and the controllers threw at request time under jest. Added the missing entries (default mock: returns null so the handlers fall through to the existing happy paths). All 278 agents-controller tests pass locally. * 🧹 refactor: Namespace messagePiiFilter under messageFilter.pii + fix import order Renames the yaml field `messagePiiFilter` to `messageFilter.pii`, the module to `messageFilterPii`, the factory to `createMessageFilterPii`, the type to `MessageFilterPiiConfig`, and the error code to `message_filter_pii_block`. The wrapper `messageFilter` namespace gives future safety filters (e.g. `messageFilter.toxicity`) a place to plug in without restructuring the config later. The `findPiiMatchInMessages` helper kept its name because it already describes what it does at the value level. Also fixes import order Danny flagged on the OpenAI-compatible and Responses controllers: `findPiiMatchInMessages` was appended at the bottom of two `require('@librechat/api')` destructures rather than placed in the length-sorted slot the house style expects. * 🧹 chore: Length-sort the general require destructure in responses.js Reorders the general sub-group inside the `require('@librechat/api')` destructure shortest to longest so the whole block conforms to the length-sort rule the file's `// Responses API` sub-group already follows. Pure reorder, no other changes. * 🧹 chore: Length-sort the defaultConfig block in AppService Reorders the `defaultConfig` keys in `packages/data-schemas/src/app/service.ts` shortest-line to longest-line, with the explicit-value entries (`mcpConfig`, `fileStrategies`, `cloudfront`) trailing the shorthand ones. Pure reorder, no behavior change.	2026-06-10 09:03:05 -04:00
Danny Avila	793cbd49f0	✂️ fix: Deduplicate Skill Bodies Across Fresh Primes and History (#13610) ... When a skill is primed fresh this turn (manual $-popover or always-apply) AND also appears in history as a `skill` tool_call, its SKILL.md body was injected twice — once by injectSkillPrimes and once reconstructed by formatAgentMessages. - add `collectFreshSkillPrimeNames` helper (packages/api) — union of manual + always-apply prime names - client.js: pass the set as `skipSkillBodyNames` to formatAgentMessages for both the initialMessages and memoryMessages paths so the body reconstructs once. Names not primed this turn still reconstruct (sticky manual re-prime). Requires `@librechat/agents` with `skipSkillBodyNames` support; the published dist silently ignores the unknown option until upgraded.	2026-06-09 17:16:24 -04:00
Danny Avila	8fc2314208	🧠 fix: Bound Memory Agent Input (#13606)	2026-06-09 14:38:21 -04:00
Danny Avila	fd4728232c	🧵 fix: Reject Preliminary Parent Follow-Ups (#13619) ... * fix: Reject preliminary parent follow-ups * chore: Sort frontend imports * fix: Narrow preliminary parent detection * fix: Preserve refused submit state * fix: Propagate refused submit result	2026-06-09 12:06:51 -04:00
Danny Avila	2a956f143d	🪞 fix: Preserve Model Spec Icons Across Stream Resume and Abort (#13603)	2026-06-08 17:14:21 -04:00
Danny Avila	cb1d536874	📻 fix: Replay MCP OAuth Prompts for Coalesced Connections (#13565) ... * fix: Replay MCP OAuth URL for Joined Connections * chore: Sort MCP OAuth Imports * test: Restore MCP OAuth Registry Spies * fix: Replay pending MCP OAuth prompts * fix: Replay MCP OAuth on Stream Resume * fix: Preserve MCP OAuth Replay Context * chore: Format MCP OAuth Replay Context * test: Expect MCP OAuth Replay Expiry * fix: Render pending MCP OAuth prompts * chore: Clean MCP OAuth Replay Type Narrowing * fix: Stabilize new MCP OAuth chats * fix: Re-emit cached MCP OAuth prompts * fix: Replay pending OAuth for selected MCP tools * fix: Avoid stalling pending MCP OAuth replay * test: Clean MCP OAuth review findings * test: Restore MCP OAuth registry spy * fix: Resolve OAuth Typecheck Regressions * fix: Harden MCP OAuth replay edge cases * test: Cover MCP OAuth joined prompt expiry * test: Mark joined OAuth replay fixture * test: Use OAuth fixture for joined replay expiry * fix: Anchor resumed MCP OAuth prompts * fix: Seed resumable turn metadata before MCP init * test: Format resume metadata regression * fix: Prioritize resumable stream routes * fix: Preserve MCP OAuth resume message tree * test: Fix MCP OAuth Resume Test Types * fix: Replay MCP OAuth Regenerate Prompts * fix: Skip OAuth-only Abort Persistence * fix: Stabilize OAuth Resume Replay * fix: Target Non-Tail Regenerate Responses * fix: Scope Regenerate Step Updates * fix: Clean Up OAuth Abort State * fix: Preserve Regenerate Branch Siblings * fix: Preserve OAuth Resume Branch State * fix: Preserve OAuth Branch Resume State * chore: Sort OAuth Resume Imports * fix: Address OAuth Resume Review Findings * test: Fix Abort Fixture Typing	2026-06-07 10:45:54 -04:00
Danny Avila	1612dba353	🏷️ fix: Preserve Generated Conversation Title on Stop (#13568) ... Immediate title generation discarded an already-generated title when the user stopped the turn, both in the backend (skipped saveConvo) and the frontend (rolled back the streamed title), leaving the chat as "Untitled" in the interim and "New Chat" after refresh. Split the title abort into two signals: `signal` still cancels an in-flight title model call on Stop, while a new `discardSignal` discards an already-generated title only when the stream is superseded by a newer run or the turn fails. A plain user Stop now persists and keeps the title. The frontend no longer rolls back a real, already-applied title on an aborted final event.	2026-06-07 08:59:05 -04:00
Peter Boers	98822341ed	⏳ feat: Make OpenID Token Reuse Window Configurable (#13546) ... * feat: make OpenID token reuse window configurable via OPENID_REUSE_MAX_SESSION_AGE_MS The OpenID session-token reuse window in AuthController was a hardcoded 15-minute constant, forcing /api/auth/refresh to perform a real refreshTokenGrant against the IdP every 15 minutes even when the current access token is still valid. IdPs that rotate and revoke the previous access token on refresh then invalidate a token that is still in use by downstream consumers of the reused OpenID token (e.g. MCP servers that receive {{LIBRECHAT_OPENID_TOKEN}} and introspect the bearer), producing ~15-minute 401 cycles regardless of the access token's actual lifetime. Read the window from process.env.OPENID_REUSE_MAX_SESSION_AGE_MS via the existing math() helper, so it accepts an arithmetic expression like SESSION_EXPIRY (e.g. 60 * 60 * 24 * 1000), defaulting to the existing 15 minutes so behavior is unchanged unless explicitly configured. The existing 30s-before-expiry guard still forces a refresh before genuine expiry, so a larger window remains safe. * fix: extend OpenID reuse session lifetime --------- Co-authored-by: Danny Avila <danny@librechat.ai>	2026-06-06 15:15:58 -04:00
Danny Avila	8c71dbcb32	🛂 fix: Normalize Verification Flow Error Responses (#13558) ... * fix: normalize verification flow responses * fix: keep verification responses consistent	2026-06-06 15:08:43 -04:00
Danny Avila	21607ba3d7	📎 fix: Preserve Provider Document Uploads (#13550) ... * fix: Preserve provider document uploads * test: Add provider upload e2e coverage	2026-06-06 10:03:32 -04:00
Danny Avila	c374d08b64	🪪 fix: Filter ACL Principal Details (#13524) ... * fix: filter ACL principal details * test: type ACL permission pipeline assertions * test: add ACL permissions e2e coverage	2026-06-05 19:06:41 -04:00
Danny Avila	5118a566df	🧭 fix: Restore Empty Skill Allowlist Catalog (#13526)	2026-06-05 12:30:48 -04:00
Danny Avila	2c8d54e18c	🗂️ feat: Add Deployment Skill Directory (#13523) ... * feat: Add deployment skill directory * chore: Address deployment skill review feedback * fix: Include deployment skill file metadata * test: Add deployment skills e2e smoke test	2026-06-05 10:24:28 -04:00
Danny Avila	dc42748813	🧷 fix: Bind Agent File Context to Current Turn (#13506) ... * fix: Bind agent file context to current turn * fix: Avoid duplicating agent file context * fix: Export agent file context prepender * test: Use exported file context prepender * fix: Keep file context transient for memory and counts	2026-06-04 09:03:43 -04:00
Danny Avila	1da789bac0	🗂️ feat: Add Agent File Authoring Tools (#13435) ... * feat: add agent file authoring tools * style: format file authoring changes * style: satisfy file authoring prettier * test: fix file authoring initialization expectations * fix: complete skill file authoring flow * fix: pass skill authoring state on edit * test: mock missing bundled skill file * fix: harden agent file authoring gates * fix: preserve file authoring runtime context * test: fix authoring context mock typing * fix: preserve subagent skill primes * test: avoid array at in handler spec * refactor: deepen skill authoring runtime wiring * fix: address codex authoring review findings * test: fix authoring collision fixture type * test: add skill file authoring mock e2e * fix: Improve skill file authoring recovery * fix: Show file authoring args while running * fix: Clarify skill rename authoring errors * fix: Keep code-only file authoring schemas sandbox scoped * fix: Address skill authoring review findings * fix: Gate skill authoring on write access	2026-06-03 23:58:12 -04:00
Danny Avila	baa23a8e24	🗂️ feat: Add Private Chat Projects (#13467) ... * feat: Add private chat projects * fix: Format project files * fix: Address project review findings * fix: Resolve project review follow-ups * fix: Handle project stats and cache edge cases * style: align projects UI with sidebar patterns * fix: resolve projects UI lint issues * style: Align project menus and composer * fix: Avoid project placeholder shadowing * fix: Handle project search and stale ids * fix: Polish project sidebar behavior * fix: Preserve new chat stream after creation * fix: Stabilize project sidebar sections * fix: Smooth project sidebar organization * fix: stabilize project chat entry * fix: keep project workspace outside chat context * fix: show default model on project workspace * fix: fallback project workspace model label * fix: preserve project scope during draft hydration * fix: include route project in new chat submission * fix: persist project id in agent chat saves * fix: refine project sidebar and creation UX * fix: export chat project method types * fix: polish project landing context * fix: refine project navigation affordances * feat: rework projects UX — coexisting sidebar sections + URL-driven scope Sidebar - Replace the chronological/by-project mode toggle with coexisting Projects + Chats sections (both always visible) - Remove ProjectConversations (927 lines), the org-mode Header, and types - Add ProjectsSection: collapsible project rows that unfurl chats inline (full-size rows), with per-project new chat and an open/rename/delete menu - Lift the marketplace/favorites shortcuts above the Projects section Chat scope - Derive a new chat's project strictly from the URL ?projectId, so the global New Chat no longer stays stuck in a project after a project chat Surfaces - Chat landing: subtle, clickable project chip instead of the floating badge - Project workspace: modest header, composer-style entry, chats list - All-projects grid: Claude-style cards with pluralized chat counts * chore: prune unused i18n keys; fix project chat-count pluralization * fix: project new-chat keeps model spec; sidebar header + row polish - newConversation: ignore a chatProjectId-only template when deciding to apply the default model spec, so starting a chat in a project no longer strips the conversation `spec` - useSelectMention: the Model Selector and @ command now retain the active project across endpoint/spec/preset switches; other new-chat paths still clear it - Chats header now matches the Projects header (inline chevron + a new-chat icon button) and starts a non-project chat - Project rows: use the new-chat icon for the per-project add button, render at text-sm to match the chat list, and align the row actions + hover color with conversation rows * fix: read project scope from router params; align sidebar header icons - useSelectMention now reads the active project from React Router's search params instead of window.location, which can drift out of sync because new-chat params are written to the URL via raw history.pushState; the Model Selector and @ command now reliably keep the project on switch - Move the Chats section header out of the virtualized list so it renders in the same context as the Projects header and isn't shifted by the list scrollbar - Inset header action icons (pr-2) so Projects/Chats header icons line up with the project-row and conversation-row trailing actions - Extract getRouteChatProjectId into utils for the submit path * fix: preserve chatProjectId through the new-chat template reduction The param-endpoint guard in newConversation reduced a new chat's template to { endpoint } only, dropping the chatProjectId injected by the Model Selector / @ switch — so switching models cleared the project scope. Keep chatProjectId in the reduced template. * style: align chat-history panel top padding; improve projects page contrast - Add pt-2 to the chat-history panel so its top spacing matches the other side panels (agent builder, skills, files, etc.) - Projects grid + workspace now use the darkest surface for the page (surface-primary) with cards, inputs, and the composer one step lighter (surface-secondary) and tertiary on hover, so cards read as elevated rather than darker than the background * feat: interactive project landing chip + gallery icon for all-projects - All-projects sidebar button uses the gallery-vertical-end icon - The project landing chip is now interactive: click it to switch projects via a searchable combobox (ControlCombobox), or the trailing × to drop the project scope. Both update the draft conversation and the ?projectId search param in place, so the typed message and selected model are preserved * test: fix Conversations unit test for refactored sidebar; add projects e2e - Update Conversations.test.tsx mocks for the inline Chats header (useNewConvo, useQueryClient, conversation atom, NewChatIcon, TooltipAnchor), drop the removed chatsHeaderControls prop, and remove the mock for the deleted ../Header module — fixes the failing frontend Jest job - Add e2e/specs/mock/projects.spec.ts covering project creation, the project-scoped new-chat landing + interactive chip (switch/remove), and listing projects on /projects - Give the landing chip combobox a stable selectId for reliable targeting * fix: refresh project stats after project-chat activity; stabilize e2e - useEventHandlers: when a project chat is created/updated, invalidate the live [projects] query (gated on chatProjectId) instead of the now-unused projectConversations key, so the sidebar + all-projects stats refresh after a streamed reply (addresses a Codex finding) - projects e2e: assert the reliable project-landing behavior (chip, scoped composer, accepted send) rather than the /c/:id transition, which the mock LLM harness doesn't complete * test: verify a project chat saves and is filed under its project (e2e) - Switch to a mock endpoint before sending so the message streams without a real API key (the default model failed with "No key found", so no chat was saved and the page never left /c/new); this also asserts the project chip survives the model switch - Restore the reply + /c/:id transition assertions and add a check that the chat is listed under the expanded project in the sidebar - Add data-testid="project-chats-<id>" to the inline project chat list * fix: address Codex review findings (project scope edge cases) - useSelectMention: fall back to the conversation's chatProjectId when the URL has no projectId, so switching model/spec inside an existing project chat (/c/:id) keeps the project assignment - Conversations: include chatProjectId in the MemoizedConvo comparator so a sidebar row's project menu doesn't stay stale after a reassignment - useDeleteProjectMutation: clear the active conversation's chatProjectId when its project is deleted (mirrors the assignment mutation); drop the now-dead projectConversations invalidation - useQueryParams: carry the project into the new conversation when applying URL settings, so /c/new?projectId=...&<settings> stays scoped * fix: project stats pagination + archived-chat edge cases (data-schemas) - listChatProjects: include the null lastConversationAt bucket in the desc cursor so empty projects paginate (a $lt:<date> predicate excluded nulls, hiding chat-less projects from "Load more") - saveConvo: recompute project stats instead of the incremental fast path when the saved conversation is itself archived/temporary/expired, so a project's lastConversationAt/Id no longer points at a hidden chat * test: cover chat-less project pagination across the dated→null boundary * fix: validate project ownership in bulkSaveConvos Bulk paths (import/duplicate/fork) persisted whatever chatProjectId the payload carried; an id that does not belong to the user created an orphan assignment hidden from both the project and the unassigned sidebar. Validate ownership like saveConvo and strip un-owned project ids before persisting, refreshing stats only for owned projects. * fix(projects): preserve chatProjectId on continuation, basename-safe delete redirect, project-detail invalidation * fix(projects): navigate project workspace chats via useNavigateToConvo to avoid stale conversation state * fix(projects): include projectConversations cache when resolving deleted chat's project for detail invalidation * fix(projects): refresh both projects when a save or bulk write moves a chat between them * style(projects): use Folders icon for the sidebar Projects header * fix(projects): require id on ProjectUser so ProjectRequest extends Express Request cleanly * style(projects): taller project chip with hover-revealed remove button, upward combobox; sort en translations * style(projects): show endpoint/agent icon for project workspace chat rows	2026-06-03 15:29:18 -04:00
Atef Bellaaj	86fe79c37d	🔗 feat: Add Granular Access Control to Shared Links via ACL System (#13051) ... * feat: Add granular access control to shared links via ACL system * fix(shared-links): preserve isPublic on failed migration grants Transient ACL failures during auto-migration permanently stranded links — $unset ran unconditionally, removing the legacy flag that triggers retry. Now only $unset isPublic after all grants succeed. * fix(config): skip isPublic unset for failed ACL grants Bulk migration unconditionally removed isPublic from all links, even those whose ACL writes failed. Failed links then lost the legacy marker needed for auto-migration retry. Now tracks failed link IDs per-batch and excludes them from the $unset step. Also adds sharedLink to AccessRole resourceType schema enum — was missing, only worked because seedDefaultRoles uses findOneAndUpdate which bypasses validation. * ci(config): add jest config and PR workflow for migration tests config/__tests__/ specs depend on api/jest.config.js module mappings but had no dedicated runner. Adds config/jest.config.js extending api config with absolutized paths, npm test:config script, and a GitHub Actions workflow triggered by changes to config/, api/models/, api/db/, or packages/ ACL code. * fix(permissions): honor boolean sharedLinks config SHARED_LINKS has no USE permission, so boolean config produced an empty update payload — gate conditions only matched object form, making `sharedLinks: false` a no-op on existing perms. * fix(share): resolve role before creating shared link Role lookup between create and grant left an orphaned link without ACL entries if getRoleByName threw — retry then hit "Share already exists" with no recovery path. * fix: Restore Public ACL Access Checks * fix: Type Public ACL Lookup * fix: Preserve Private Legacy Shared Links * chore: Promote Shared Link Permission Migration * fix: Address Shared Link Review Findings * fix: Repair Shared Link CI Follow-Up * fix: Narrow Shared Link Mongoose Test Mock * fix: Address Shared Link Review Follow-Ups * fix: Close Shared Link Review Gaps * fix: Guard Missing Shared Link Permission Backfill * test: Add Shared Link Mock E2E * test: Stabilize Shared Link Mock E2E --------- Co-authored-by: Danny Avila <danny@librechat.ai>	2026-06-03 14:17:17 -04:00
Danny Avila	2ef7bdfbc2	⚡ feat: Immediate Conversation Title Generation (#13395) ... * ⚡ feat: Immediate Conversation Title Generation Generate conversation titles as soon as the request is made (in parallel with the response, from the user's first message) as the new default, fixing the #13318 race where a transient /gen_title 404 left new chats stuck on "New Chat". - Add per-endpoint `titleTiming` ('immediate' | 'final') to baseEndpointSchema; `endpoints.all` acts as the global default, unset = immediate. Resolve via a new `resolveTitleTiming` helper (`all` takes precedence). - Fire title generation in parallel with `sendMessage`; `titleConvo` waits (bounded, abortable) for the agent run and titles from the user input only. Persist after the conversation row exists; defer `disposeClient` until the title settles. - Expose `titleGenerationTiming` via startup config; `useTitleGeneration` fetches eagerly in immediate mode with a bounded 404 retry and never treats a transient 404 as final. Skip title queueing for temporary conversations. - Supersedes #13329 while incorporating its bounded 404-retry. * 🩹 fix: Address Copilot review findings on title timing - Guard against an undefined conversationId in addTitle (skip + warn) so the gen_title cache key can't collide as `userId-undefined` and saveConvo is never called without a conversationId. - Gate the title `useQueries` on `enabled` so no /gen_title request fires while unauthenticated (e.g. after logout) even if the module queue holds IDs. - Drop the stale `conversationId` param from the titleConvo JSDoc. - Add a regression test for the undefined-conversationId guard. * 🧵 fix: Harden immediate-title edge cases from codex review - Cancel in-flight immediate title generation when the request aborts: thread job.abortController.signal through addTitle so pressing Stop on a new chat neither consumes the title model nor surfaces a title for a cancelled turn. - Preserve a locally-applied title when the final SSE event's conversation carries no title yet (built before the title was saved), so long immediate-mode responses no longer revert the chat to "New Chat" until reload. - Guarantee one full post-completion gen_title fetch cycle before giving up, so a `final`-mode title (generated only after the stream ends) is still fetched under a global `immediate` default instead of being stranded. - Add regression tests for the abort propagation and the undefined-conversationId guard. * 🔁 fix: Correct title abort, post-completion refetch, and replacement ordering Follow-up to codex review of the immediate-title fixes: - Use a dedicated title AbortController instead of `job.abortController`. The latter is also aborted by `completeJob` on *successful* completion, which cancelled any title slower than a short response. The title is now cancelled only on a real user Stop or when the stream is replaced; a completed-then- aborted title is discarded (no save, cache cleared) rather than persisted. - Reset (not remove) the post-completion title query: `resetQueries` refetches the mounted observer with a fresh retry budget, whereas `removeQueries` left it stuck in its error state, so the promised post-completion cycle never ran. - Run the job-replacement check before resolving `convoReady`, and on a replaced stream cancel/discard the stale title so a discarded prompt can't persist a title. * 🧷 fix: Tighten title abort ordering and endpoint-level timing resolution Follow-up to codex review: - Abort the title controller before resolving `convoReady` on a stopped turn, so the title task can't resume and persist before the later abort. - Cancel the title and unblock its waits on ANY send failure (not just user aborts): a preflight/quota failure before the run exists otherwise hangs `_waitForRun`, deferring client disposal until the 45s title timeout. - Resolve `titleTiming` for custom endpoints via `getCustomEndpointConfig` (their config lives under `endpoints.custom[]`, not `endpoints[endpoint]`). - Derive the startup `titleGenerationTiming` via `resolveTitleTiming` for the agents endpoint so an endpoint-level `final` (without `endpoints.all`) is honored client-side instead of defaulting to immediate and burning eager gen_title polls. * 🪢 fix: Per-agent title timing and safer abort/replacement handling Follow-up to codex review: - Resolve `titleTiming` from the agent's actual endpoint after initialization, so a per-endpoint `final` override on a custom/provider endpoint backing an (ephemeral) agent is honored instead of always using the `agents` endpoint's value. - Don't preserve a locally-fetched title on a stopped (unfinished) turn: the server cancels and discards that title, so keeping it client-side would diverge from server state and leave the stopped chat titled until reload. - On abort/replacement, only delete the cached title if it still holds THIS task's value — a replacement stream shares the `userId-conversationId` key and may have already cached its own valid title that must not be removed. * 🪞 fix: Mirror AgentClient title-config resolution for titleTiming Per maintainer guidance, keep titleTiming resolution identical to how `AgentClient#titleConvo` already resolves the endpoint config — `endpoints.all` is the intended global override and the agent's actual provider endpoint is used: - Resolve via `endpoints.all ?? endpoints[endpoint] ?? getProviderConfig(endpoint) .customEndpointConfig` (was using `getCustomEndpointConfig` directly). Going through `getProviderConfig` picks up its case-insensitive fallback for normalized provider names (e.g. `openrouter` → `OpenRouter`), so a custom endpoint's `titleTiming` is honored like its other title settings. - Add `titleTiming` to the Azure endpoint schema `.pick()` so `endpoints.azureOpenAI.titleTiming` is no longer silently stripped by Zod. Note: per-endpoint title settings being skipped when `endpoints.all` is present is the existing, intended global-override behavior — not changed here. * 🧪 test: Cover useTitleGeneration effect logic (integration) Adds a deterministic white-box integration test that drives the real hook's React effects with a controllable react-query surface, locking down the stateful decisions that previously had no coverage: - immediate mode fetches a queued conversation while its stream is still active - final mode gates until the stream completes, then becomes eligible - success applies the fetched title to the conversation caches - a 404 while active defers (removeQueries) instead of giving up - a 404 after completion forces a fresh fetch via resetQueries (post-completion remount) * feat: Stream immediate title events * style: Format title SSE handler * test: Preserve data-provider exports in OAuth mock * test: Isolate OAuth route API mock * test: Keep OAuth callback factory capture * fix: Replay streamed title events on resume * fix: Honor agents title timing precedence * style: Format title timing fixes	2026-06-02 16:40:57 -04:00
jcbartle	268f095c1a	🔒 feat: Add On-Behalf-Of (OBO) token exchange support for MCP Servers (#13429) ... * Add OBO (On-Behalf-Of) token exchange support for MCP server connections Enables transparent authentication to Entra ID-backed MCP servers using the logged-in user's federated token via the OAuth 2.0 jwt-bearer grant. Configured via obo.scopes in librechat.yaml server config. - Extract generic OboTokenService from GraphTokenService (jwt-bearer grant + cache) - Refactor GraphTokenService to thin wrapper delegating to OboTokenService - Add obo schema field to BaseOptionsSchema in data-provider - Add resolveOboToken in packages/api/src/mcp/oauth/obo.ts (validates federated token, calls resolver, returns MCPOAuthTokens) - Wire oboTokenResolver through MCPConnectionFactory, MCPManager, UserConnectionManager - OBO tokens injected via request headers (not OAuth transport), refreshed on each tool call - Explicit error on OBO failure (no fallthrough to standard OAuth redirect) - Add unit tests for both resolveOboToken (9 tests) and exchangeOboToken (14 tests) * Add OBO authentication option to MCP server UI configuration Enable users to configure On-Behalf-Of (OBO) token exchange for MCP servers created via the UI (MongoDB-stored), in addition to the existing YAML-based configuration. - Add "On-Behalf-Of (OBO)" radio option to MCP server auth section with scopes input field - Remove obo from omitServerManagedFields so the field passes UI schema validation - Add OBO to AuthTypeEnum, obo_scopes to AuthConfig, and OBO handling in form defaults and submission - Add .min(1) validation on obo.scopes to reject empty strings - Add English localization keys: com_ui_obo, com_ui_obo_scopes, com_ui_obo_scopes_description - Add 5 schema validation tests for OBO field acceptance, transport compatibility, and edge cases * 🧊 fix: Add obo to safe properties in redactServerSecrets. Fixes the OBO configuration not showing up in the MCP UI after app restart * Address linter errors * 🧊 fix: fail closed on OBO refresh errors and retry transient token exchange failures - stop tool calls from falling back to stale Authorization headers when per-call OBO refresh fails - add one-time retry for transient Entra OBO exchange failures (network/429/5xx) - preserve structured OBO failure reasons and retryability in resolveOboToken - improve OBO auth error messaging for connection setup and tool execution - add tests for transient vs permanent OBO failure paths * Addressing linting errors / warnings * 🧊 fix: isolate OBO MCP auth to user-scoped connections - block OBO-enabled servers from app-level shared MCP connections - bypass shared connection lookup for OBO servers in MCPManager.getConnection - add regressions covering OBO connection scoping and preserve non-OBO app connection reuse * 🛠️ refactor: centralize MCP user-scoped connection policy - add shared requiresUserScopedConnection helper for OAuth, OBO, and customUserVars - use the shared predicate in MCPManager and ConnectionsRepository - add utils coverage for user-scoped connection policy * 🧊 fix: restrict MCP OBO config to header-capable transports - Move OBO configuration out of the shared MCP base options schema and allow it only on SSE and streamable-http transports, where request headers are applied. - Explicitly reject OBO on stdio and websocket configs to avoid accepted-but- nonfunctional server definitions. Add schema coverage for admin/config parsing and user-input websocket validation. * 🧊 fix: single-flight concurrent OBO token exchanges Concurrent tool calls that arrive on a cache miss were each issuing their own jwt-bearer request to the IdP. Under that fan-out, Entra intermittently returned errors that the retry classifier saw as non-retryable, surfacing as: "The identity provider rejected the OBO token exchange. Cannot execute tool <name>. Re-authenticate the user or verify the configured OBO scopes and retry." A user retry then hit the populated cache and succeeded, which matches the observed flakiness — the cache was empty at the moment of fan-out but populated by the time the user clicked retry. - Coalesce concurrent exchanges in `OboTokenService.exchangeOboToken` keyed by `${openidId}:${scopes}`. Callers that arrive while an exchange is in flight share the same upstream request and receive the same result. `fromCache=false` continues to force a fresh, independent exchange (and is not joined by `fromCache=true` callers). The IdP call, single-retry path, and cache write are unchanged — they were moved into a `performOboExchange` helper so the coalescing wrapper stays small. - Tests cover: coalescing on the same key, isolation between different keys, cleanup on success, cleanup on failure, and the `fromCache=false` bypass. * 🔒 feat: gate MCP OBO config behind MCP_SERVERS.CONFIGURE_OBO permission OBO silently mints per-user delegated tokens from the caller's federated access token and forwards them to whatever URL the server config points at. Previously, anyone with MCP_SERVERS.CREATE could configure obo.scopes — so if server creation is ever delegated beyond admins, a user could stand up an attacker-controlled server, attach it to a shared agent, and exfiltrate other users' downstream tokens on tool invocation. Add a dedicated MCP_SERVERS.CONFIGURE_OBO permission (ADMIN: true, USER: false by default) and enforce it at three layers so the safety property no longer depends on CREATE staying admin-only: - Create/update: POST/PATCH /api/mcp/servers returns 403 when the body carries `obo` and the caller's role lacks the permission. - Runtime fail-closed: for DB-sourced configs, MCPConnectionFactory and MCPManager.callTool re-check the original author's role before each OBO exchange. If the author has been downgraded, the exchange is skipped (factory) or refused (callTool) — retained configs lose their privileges automatically. - UI: the OBO option is hidden in the MCP server dialog for users without the permission; a CONFIGURE_OBO toggle is exposed in the MCP admin role editor. Existing role docs receive the new sub-key via the permission backfill in updateInterfacePermissions on next startup, preserving any operator-set values. YAML/Config-sourced server configs are unaffected since they're admin-controlled at the deployment level. * 🧊 fix: wire OBO machinery for servers with requiresOAuth: false The discovery and user-connection paths gated OAuth wiring (flow manager, token methods, oboTokenResolver, oboTrustChecker) behind isOAuthServer(), which only considers requiresOAuth/oauth fields. A DB-stored OBO server with requiresOAuth: false therefore landed in the non-OAuth branch, never received an oboTokenResolver, and the factory's usesObo getter evaluated to false — sending a bare request that the upstream rejected with invalid_token. Add requiresOAuthMachinery() (OAuth OR OBO) and use it at those two gates. isOAuthServer remains for the OAuth-handshake-only check (shouldInitiateOAuthBeforeConnect), where OBO must not initiate a handshake. Plumb the OBO resolver/trust-checker through ToolDiscoveryOptions so reinitMCPServer can pass them on the discovery path. * 🧊 fix: lock all OBO-target fields (URL, proxy, headers, auth) without CONFIGURE_OBO The CONFIGURE_OBO permission was meant to gate control of the endpoint that receives OBO-minted per-user delegated tokens and the scopes that are requested. The previous frontend lock + backend gate only covered obo.scopes and the auth section, leaving url/proxy/headers/etc. editable by anyone with UPDATE — meaning a non-permission user could still redirect an existing OBO server's token flow to an attacker endpoint. Switch to an allowlist policy: when editing an OBO server without CONFIGURE_OBO, only title/description/iconPath are mutable. Backend rejects any other field change with 403; frontend disables the non-allowlist sections (URL, transport, auth, trust) via fieldset. The comparison surface (MCP_USER_INPUT_FIELDS) is derived from MCPServerUserInputSchema's union members so it stays in sync with the schema. New schema fields land in the locked set by default — adding to the allowlist is the only way to unlock them, which preserves the security-review boundary. * 🧊 fix: skip unauthenticated MCP inspection for OBO-only servers MCPServerInspector.inspectServer() ran an unauthenticated temp connection unless the config had requiresOAuth or customUserVars set. For OBO-only servers without standard MCP OAuth advertisement, this caused MCPConnectionFactory.create to attempt the connection without a user or oboTokenResolver — failing on servers that reject the MCP initialize handshake without a valid bearer token, which surfaced as MCP_INSPECTION_FAILED on create/update. Add `obo` to the skip list alongside requiresOAuth and customUserVars, matching the existing pattern for user-scoped auth modes. * Addressed linting error: watchedTitle is declared but never referenced (the auto-fill logic at line 156 uses getValues('title') instead). Deleted constant.	2026-06-01 22:36:18 -04:00
Danny Avila	e0c346c0a4	🤫 chore: Quiet Repetitive Log Noise from Balance, CloudFront, and Capability Paths (#13461) ... * chore: reduce auth and balance operational noise * chore: tighten balance and capability noise handling * chore: avoid balance 404s when disabled * chore: use response locals for balance handoff	2026-06-01 20:40:16 -04:00
Marco Beretta	730878bc5a	🔐 feat: Use SecretInput for Sensitive Fields (#12955) ... * feat: use SecretInput for sensitive fields * fix: align auth SecretInput styles * chore: remove unused password i18n keys * fix: align SecretInput controls * fix: use SecretInput for dynamic credentials * fix: reveal SecretInput controls on hover * fix: align SecretInput eye icon and modernize controls The wrapper was a flex container, so passing 'mb-2' on the input made it contribute its margin to the wrapper's cross-axis size — the controls overlay spanned the inflated height and centered the toggle 4px below the input's true center. Switching the wrapper to a plain relative block collapses height back to the input. Also tightens the toggle/copy buttons (size-7 rounded-md with hover:bg-surface-hover) and adds a focus ring on the input. Auth pages still override className/buttonClassName so login/register styling is unchanged. * fix: remove focus ring from SecretInput * fix: keep green focus border on auth secret inputs SecretInput's modernized default uses focus-visible:border-border-heavy and hover:border-border-medium, which Tailwind emits after the auth pages' focus: rules and overrides them. Auth pages now also declare focus-visible:border-green-500 and hover:border-border-light so cn()/twMerge resolves them as the winners when classes are concatenated. * feat: add optional sensitive flag to MCP customUserVars Dynamic MCP credential fields all rendered as masked SecretInputs, which also hid non-secret setup values like usernames, project keys, and URLs. Add an optional `sensitive` flag to customUserVars and the plugin auth config. It defaults to masked when omitted, so existing configs keep the safe-by-default behavior; set `sensitive: false` to render a field as plain text. The flag is display-only — values remain encrypted at rest.	2026-06-01 18:14:12 -04:00
Danny Avila	75baa5b848	🧼 fix: Sanitize User Response Fields (#13421)	2026-05-30 19:35:52 -04:00
Danny Avila	479e9d59b7	🧠 refactor: Memoize MCP Permission Checks Per Request (#13419)	2026-05-30 18:32:06 -04:00
Danny Avila	100871c3ec	🛂 fix: Enforce MCP Permissions for Agent Tools (#13174) ... * fix: Enforce MCP Permissions for Agent Tools * fix: Measure MCP Image Limit by Decoded Size * fix: gate cached MCP tools and tighten remote image URL detection Addresses Codex review findings on the MCP permissions PR: - filterAuthorizedTools previously fast-accepted any tool present in the global tool cache before reaching the MCP-use permission gate. App-level MCP tools (keyed `name_mcp_server` by MCPServerInspector and merged into the cache via mergeAppTools) therefore bypassed the canUseMCP check, letting a user without MCP_SERVERS.USE persist/bind them. Route all MCP-delimited tools through the permission + server-access gate regardless of cache presence. - assertImageDataWithinLimit / image formatter used startsWith("http") to skip the size cap, which also matched base64 payloads that happen to begin with those chars. Require http:// or https:// via a shared isRemoteImageUrl helper so oversized inline base64 can no longer bypass MCP_IMAGE_DATA_MAX_BYTES. Adds regression tests for both paths. * fix: address Codex round-2 findings on MCP permissions PR - parsers.ts: parseAsString dropped the image payload for unrecognized providers, returning only `Image result: <mimeType>`. Pre-PR these items survived via JSON.stringify(item). Keep the size guard but fall through to JSON.stringify so the data/URL is preserved. - MCP.js: the runtime MCP-use check only read `configurable.user`, so paths that propagate `user_id` only (e.g. the OpenAI-compatible API in agents/openai/service.ts) rejected every MCP tool call for an authenticated user. Add resolveMCPPermissionUser: use the safe user directly when it already carries a role (no extra DB call), otherwise fall back to loading the role by user_id. Update fail-closed tests to the resolved behavior. - v1.js: the update path only re-filtered newly added MCP tools, so a user who lost MCP_SERVERS.USE kept existing MCP bindings on edit while create/duplicate/revert stripped them. Strip all MCP tools on update when the permission is revoked; keep the narrower new-tool gating (and disconnect/registry preservation) when it is intact. Updates and adds regression tests for all three paths. * fix: populate safe user at producer instead of resolving in runtime MCP check Corrects the Finding B approach from the previous commit. Rather than loading the user by id inside the runtime MCP permission check, populate `configurable.user` (and createRun's `user`) with the full safe user at the producer, matching the in-repo agent controllers (responses.js / openai.js) which already pass `createSafeUser(req.user)`. - service.ts: derive `safeUser` via createSafeUser(req.user) and pass it to both createRun and processStream's configurable, so the role-bearing identity reaches the runtime `userCanUseMCPServers(configurable.user)` check. Falls back to a bare id when the host app attached no user, which correctly leaves MCP gated (fail closed). - MCP.js: revert the resolveMCPPermissionUser DB-load fallback; the runtime check again reads configurable.user directly and fails closed when absent (defense in depth). - MCP.spec.js: revert to the matching runtime test expectations. * test: cover safe-user propagation in createAgentChatCompletion Adds a focused spec for the OpenAI-compatible chat completion service (the producer fixed for Codex Finding B). Injects mocked deps and asserts that createRun and processStream's configurable.user carry the role from req.user (with sensitive fields stripped by createSafeUser), and that an unauthenticated request falls back to a bare { id: 'api-user' } so the runtime MCP check fails closed. * fix: address Codex round-3 findings + TS6133 - MCP.js (P1): the assistants required-action path invokes tool._call( toolInput) with no LangChain config, so the runtime check saw no configurable.user and rejected authorized users. createToolInstance now captures the creation-time user (req.user via createMCPTool) and _call falls back to it for both the permission check and userId. Still fails closed when neither config nor captured user carries a role. - v1.js (P2): the update-path isMCPTool used a bare mcp_delimiter substring check, misclassifying action tools whose operationId contains "_mcp_" (e.g. sync_mcp_state_action_...) as MCP and dropping them on a permission-revoked edit. Delegate to the canonical isActionTool so only real MCP tools are gated. Regression test added. - service.ts: drop the now-unused IUser import (TS6133); derive reqUser's type from createSafeUser's own parameter instead. * fix: resolve TS7022 self-reference in service.spec mock res The mock response object referenced `res` inside its own `status`/`json` initializers without a type annotation, so tsc inferred `res` as `any` (TS7022). Annotate the object and assign the self-referencing chainable methods after declaration. * fix: correct round-4 findings (isActionTool import, captured user, partial-update) - v1.js: import isActionTool from librechat-data-provider (its real export; @librechat/api does not export it, so the prior import was undefined and threw TypeError). Exclude action tools from MCP classification in both the main filterAuthorizedTools loop and the update path, so action tools whose operationId contains _mcp_ (e.g. sync_mcp_state_action_...) are preserved regardless of MCP permission. - v1.js: evaluate the effective tool set (updateData.tools ?? existingAgent.tools) so a tools-less PATCH by a user who lost MCP_SERVERS.USE still strips stale MCP bindings, matching create/duplicate/revert. - MCP.js: createToolInstance now receives the construction-time user and _call falls back to it (permissionUser) when configurable.user is absent, fixing the assistants required-action path that invokes _call without a config and resolving the capturedUser no-undef/ReferenceError. - Tests: action-tool preservation (authorized + denied), tools-less revocation PATCH, updated revocation test to expect all MCP tools stripped. Affected specs pass locally: MCP 49/49, filterAuthorizedTools 49/49. * fix: guard isActionTool against non-string tools; correct actionDelimiter import Two test regressions from the prior commit: - The main filterAuthorizedTools loop called isActionTool(tool) directly, but isActionTool does toolName.indexOf(...) and throws on null/undefined. Compute isActionToolName = typeof tool === 'string' && isActionTool(tool) once and reuse it, restoring graceful null/undefined handling. - The action-tool test referenced Constants.actionDelimiter (undefined); actionDelimiter is a standalone librechat-data-provider export. Import and use it directly. filterAuthorizedTools 36/36 and MCP 40/40 pass locally. * fix: address MCP permission review follow-ups * fix: preserve shared agent MCP tools	2026-05-30 16:19:49 -04:00
Danny Avila	6d9c01927d	🧠 refactor: Replay DeepSeek reasoning_content via OpenRouter (#13368) ... * 🧠 fix: Replay DeepSeek `reasoning_content` via OpenRouter DeepSeek's thinking-mode API rejects multi-turn tool-calling requests unless `reasoning_content` from each tool-bearing assistant message is replayed verbatim, returning HTTP 400 "The `reasoning_content` in the thinking mode must be passed back to the API." The agents SDK already handles this for direct `Providers.DEEPSEEK`, but DeepSeek models routed via OpenRouter use `Providers.OPENROUTER` — `formatAgentMessages` skipped the reasoning-preservation branch, and `ChatOpenRouter` left `includeReasoningContent` unset, so the field silently dropped on every subsequent turn. Add `isDeepSeekReasoningProvider(provider, model)` and use it in two places: (1) `getOpenAILLMConfig` flips `includeReasoningContent: true` when OpenRouter is dispatching a `deepseek/*` model so the LangChain client emits the field on assistant turns that have non-empty `additional_kwargs.reasoning_content`, and (2) `AgentClient` spoofs the provider hint to `Providers.DEEPSEEK` when calling `formatAgentMessages`, triggering the SDK's existing `preserveReasoningContent` path that re-attaches the field to reconstructed tool-bearing AIMessages. The downstream `_convertMessagesToOpenAIParams` is already gated on non-empty `reasoning_content`, so the flag is a no-op outside thinking mode. Resolves #13366. * fix: Harden DeepSeek detection against OpenRouter routing edges Address three Codex review findings on #13368: 1. Strip OpenRouter's `~` latest-routing prefix before applying the DeepSeek model regex. `~deepseek-chat` and `~deepseek/r1` were previously left unmatched because the regex's start/`/` boundary only saw the `~`. Mirror the SDK's `normalizeOpenRouterModel()` here and in `getOpenAILLMConfig`. 2. Add a custom-endpoint fallback: when the model id carries the unambiguous `deepseek/...` OpenRouter namespace, accept it regardless of the resolved provider. Covers the case where a user configures OpenRouter under a non-standard endpoint name and `initializeAgent` normalizes the unknown provider to `openai`, stranding the spoof. Bare `deepseek-*` ids still require an explicit DeepSeek/OpenRouter provider so unrelated endpoints labelling a model `deepseek-r1` don't trigger. 3. Inspect every agent in `this.agentConfigs` when deciding whether to spoof the format provider. Multi-agent handoff runs feed all agents' messages through one `formatAgentMessages` call, so a DeepSeek handoff under a non-DeepSeek primary previously lost its persisted reasoning_content too. Also addresses Copilot's review note: only pass the options object to `formatAgentMessages` when the DeepSeek spoof is actually needed, preserving the pre-fix behavior for everyone else. * fix: Extend DeepSeek reasoning_content fix to OpenAI-compat agent paths Address two more Codex P2 findings on #13368: 1. `getOpenAILLMConfig` no longer gates `includeReasoningContent` on `useOpenRouter`. Any DeepSeek-style model id (with `~` latest-routing prefix stripped) is sufficient. This re-aligns the LLM gate with `AgentClient`'s formatter spoof, which already treats a `deepseek/*` id as authoritative — so a custom-named OpenRouter endpoint or a DeepSeek-compatible proxy gets the field both attached to history AND serialized to the wire. Direct `ChatDeepSeek` ignores the flag (its own conversion path hardcodes `includeReasoningContent: true`), so this is a harmless no-op there. 2. Thread the same `Providers.DEEPSEEK` formatter hint through `api/server/controllers/agents/openai.js` and `responses.js` (the OpenAI-/Responses-compatible serving paths). Without it those paths restored `additional_kwargs.reasoning_content` only in `AgentClient` while the LLM config flipped `includeReasoningContent` on for them too — so DeepSeek tool turns served from those endpoints would still ship requests with the flag set but no field present, hitting the same second-turn 400. The `needsDeepSeekFormatHint` helper in `openai.js` mirrors `AgentClient`'s per-agent check. * fix: Tighten DeepSeek detection and cover handoff sub-agents Address four more Codex P2 findings on #13368: - Tighten the DeepSeek model regex to `^deepseek(?:[-/]|$)/i` (anchored to start). Rejects cloned/distilled slugs like `mistral/deepseek-distilled-foo` and `community/deepseek-r1` that previously matched via the `(?:^|/)` alternation, which could attach the DeepSeek-only `reasoning_content` field on proxies that don't accept it. - Anchoring also collapses the namespace-only fallback into the same pattern, so bare `deepseek-chat` / `deepseek-reasoner` on a custom OpenAI-compatible DeepSeek proxy are now recognized — fixing the asymmetry where `getOpenAILLMConfig` would flip `includeReasoningContent` for those bare ids but `AgentClient` wouldn't pass the formatter hint. - Extend `needsDeepSeekFormatHint` in `openai.js` (and the inline check in `responses.js`) to walk `handoffAgentConfigs` too. In multi-agent runs where the primary isn't DeepSeek but a connected handoff agent is, the SDK's `formatAgentMessages` previously dropped the handoff's persisted reasoning_content before the next tool turn, preserving the 400 the PR was meant to prevent. - Mirror the regex change in `getOpenAILLMConfig`. Out of scope: the OpenAI-compatible serving paths still don't preserve incoming `reasoning_content`/`reasoning` fields in `convertMessages`, nor does the Responses API persist reasoning in `saveResponseOutput`. Those are deeper persistence/conversion fixes worth a separate PR. * test: Allow includeReasoningContent for Azure-serverless DeepSeek CI surfaced a backward-compat expectation that snapshotted the pre-fix behavior. Azure-serverless DeepSeek deployments (e.g. `DeepSeek-R1`) forward to the same DeepSeek thinking-mode tool-call contract, so the LLM gate now correctly flips `includeReasoningContent: true` for them too. The downstream gate on a non-empty `additional_kwargs.reasoning_content` keeps this a no-op outside thinking mode. * chore: Trim noisy comments Per CLAUDE.md ("self-documenting code; no inline comments narrating what code does"), strip the multi-paragraph rationale that crept into the DeepSeek reasoning_content fix. The commit history and PR description carry the why; the code says the what. Keeps one single-line JSDoc on `isDeepSeekReasoningProvider` (linking to the DeepSeek docs) and a `(#13366)` tag on each opt-in site so future readers can find the context. * revert: Drop non-functional DeepSeek hint from OpenAI-compat serving paths Codex's later review passes correctly flagged that threading the DeepSeek formatter hint through openai.js (`/v1/chat/completions`) and responses.js (`/v1/responses`) doesn't actually fix the second-turn 400 in those paths. Empirical check against the real SDK confirmed the gap is deeper and pre-existing: formatAgentMessages(payload, ..., { provider: DEEPSEEK }) where payload is the `convertMessages`/`convertInputToMessages` output shape (string content + TOP-LEVEL `tool_calls`) produces NO tool-bearing AIMessage at all — `formatAssistantMessage` only reconstructs tool calls from `tool_call`-typed *content parts*, never a top-level `tool_calls` field. So those serving paths don't reconstruct tool-call history (let alone reasoning) regardless of the hint. The Responses persistence layer likewise stores only output text, not tool calls or reasoning. Making those paths work requires reworking the wire->internal message conversion (and Responses persistence) to emit content-part arrays — a broad, pre-existing concern beyond this issue and risky to land here. Rather than ship a hint that looks like a fix but is inert, revert the serving-path changes and scope this PR to the validated AgentClient chat path (the actual surface in #13366). Reverts the openai.js/responses.js threading and their spec mocks to main. Keeps the AgentClient fix, `isDeepSeekReasoningProvider`, the `getOpenAILLMConfig` flag, and the type.	2026-05-28 22:10:49 -07:00
Danny Avila	94c73123ee	📋 fix: Cap Default Limit on Agent List Queries (#13382) ... * 🛡️ fix: Cap Default Limit on Agent List Queries (#13363) `GET /api/agents` accepted unbounded requests: when the client omitted `limit`, the value flowed straight into `getListAgentsByAccess`, which set `isPaginated = false` and issued an uncapped MongoDB query. Combined with the unindexed `findPubliclyAccessibleResources` AclEntry scan run on every request, this produced 10-19s response times and stalled the connection pool on instances with 100+ agents. - Default `limit` to 100 in the route handler so client requests without `?limit=` paginate by default. - Default `limit` to 100 in `getListAgentsByAccess` itself as defense-in-depth. The function already caps numeric limits at 100, so there is no client-facing change. - Pass `limit: null` explicitly in the actions route, which legitimately needs the full editable-agent set, to preserve its existing behavior. - Add regression tests covering the default cap and the explicit unbounded opt-out. * 🛡️ fix: Avoid agent-list regression for users with 100+ agents Codex review pointed out that capping `getListAgentsByAccess` at 100 silently truncated agents past the first page for the four consumers (`useAgentsMap`, `AgentSelect`, `ModelSelectorContext`, `useMentions`) that read `res.data` without following `has_more`/`after`. - Raise the function's hard cap from 100 to 1000 to match `MAX_AVATAR_REFRESH_AGENTS`, the realistic upper bound the avatar-refresh path already assumes. (Side effect: the avatar refresh call site was silently being capped at 100 by the old normalize step.) - In `useListAgentsQuery`, merge `limit: 1000` into params so the four consumers above get the user's full accessible set in a single round-trip instead of needing cursor pagination. - Route handler default stays at 100 as defense-in-depth for any other caller that omits `limit`. - Add a regression test asserting an explicit `limit` above 100 now returns the full set instead of being clipped. * 🪢 fix: Keep agent-list cache key stable for mutations Codex P2 review noted that folding `limit: 1000` into the cache key broke `allAgentViewAndEditQueryKeys` in `Agents/mutations.ts`, which references `[QueryKeys.agents, { requiredPermission }]` directly across eight mutation handlers. After my prior change the cached entry lived under `[QueryKeys.agents, { limit: 1000, requiredPermission }]`, so create/update/delete/avatar/action mutations stopped updating the list the four consumer hooks render — and with `refetchOnMount` and focus/ reconnect refetches disabled, the UI would stay stale until something else triggered a fetch. Split the merged limit out of the cache key: the request to `dataService.listAgents` still uses `requestParams` (with the default limit applied), but the React Query cache key uses the caller's `params` as-is. The mutation cache updates land again, and the request still returns the user's full accessible set in one round-trip. * 🛡️ fix: Index AclEntry and paginate agent list internally (#13363) Completes the perf fix for #13363 properly — resolves both the unbounded ACL scans Copilot flagged and Codex's tension between "show all agents" and "don't bypass the server cap". Backend: - Add a compound index on `{ principalType, resourceType, permBits, resourceId }` to the AclEntry schema. This is the index missing for `findPublicResourceIds` and the public branch of the `$or` in `findAccessibleResources`, both of which previously fell back to a collection scan on every `GET /api/agents`. Adds an `explain`-based regression test asserting the public query no longer COLLSCANs. Client: - Rewrite `useListAgentsQuery` to follow the server's cursor pagination internally and concatenate every page into a single flat `AgentListResponse`. Consumers (`useAgentsMap`, `AgentSelect`, `ModelSelectorContext`, `useMentions`) get the user's complete accessible-agent set without any of them needing to learn about cursors, and each individual request uses the server's default page size (so the route's 100-default defense-in-depth fires for real). Cache key shape is unchanged, so the eight mutation handlers in `Agents/mutations.ts` keep matching `allAgentViewAndEditQueryKeys` and update the cached list as before. - Drop the `FULL_AGENT_LIST_LIMIT = 1000` injection added in the previous commit — no longer needed once pagination handles the full set, and removing it stops bypassing the route default. * 🧹 fix: CI fallout from C-done-properly refactor - Collapse multi-line `fetchAllAgentPages` signature in queries.ts so prettier stops complaining. - In the new public-principal index test, grant one ACL entry before calling `.explain()` so the collection exists (otherwise mongo returns `nonExistentNamespace` and there is no winning plan to inspect). - Cast the `.explain('queryPlanner')` result to a typed shape — the mongoose return type doesn't expose `queryPlanner` directly and was failing the TypeScript check. * 🧪 fix: Test the AclEntry public-principal index via hint, not planner choice The previous test asserted the query planner did not pick COLLSCAN for the public-principal lookup. That assertion fails on small collections (under the planner's collection-size heuristic) — the index exists and is usable, but with a single document in the test the planner correctly chooses COLLSCAN as the cheaper plan. Reshape the assertion: 1. Confirm the new compound index is actually declared by inspecting `collection.indexes()` after `syncIndexes()`. 2. Force the planner to that index via `.hint()` and assert the winning plan is `IXSCAN` — proves the index is real and serves this query shape, without depending on collection-size heuristics. * 🧹 chore: Slim down verbose comments The JSDoc and inline comments added across the perf fix had drifted into multi-paragraph rationale better suited to the PR description than the source. Collapse to single-line JSDoc that just describes what each piece does; drop the inline comment in `actions.js` entirely — the call is self-evident.	2026-05-28 21:37:53 -07:00
Danny Avila	bd64251eb9	🪪 fix: Prevent MCP Server Name Collisions (#13256) ... * fix: prevent MCP server name collisions * chore: address MCP registry review nits * fix: reserve MCP config names from request context * chore: format MCP registry changes * chore: address MCP collision review findings	2026-05-22 20:46:14 -04:00
Danny Avila	9dd062e42e	🧯 fix: Harden Data Retention Semantics (#13049) ... * feat: support data retention for normal chats Add retentionMode config variable supporting "all" and "temporary" values. When "all" is set, data retention applies to all chats, not just temporary ones. Adds isTemporary field to conversations for proper filtering. Adapted to new TS method files in packages/data-schemas since upstream moved models out of api/models/. Based on danny-avila/LibreChat#10532 Co-Authored-By: WhammyLeaf <233105313+WhammyLeaf@users.noreply.github.com> (cherry picked from commit 30109e90b0) * feat: extend data retention to files, tool calls, and shared links Add expiredAt field and TTL indexes to file, toolCall, and share schemas. Set expiredAt on tool calls, shared links, and file uploads when retentionMode is "all" or chat is temporary. (cherry picked from commit 48973752d3) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: lint/test (cherry picked from commit 310c514e6a) * fix: address code review feedback for data retention PR Critical: - Fix BookmarkMenu crash: restore optional chaining on conversation - Fix migration hazard: backward-compatible sidebar filter that also checks expiredAt for documents without isTemporary field Major: - Add logging to getRetentionExpiry error path, align with tools.js - Add tests for retentionMode: ALL in saveConvo and saveMessage - Fix share route: apply expiredAt for temporary chats too by querying the conversation's isTemporary flag server-side - Add assertions for getRetentionExpiry mocks in process tests Minor: - Fix ChatRoute isTemporaryChat to be strictly boolean via Boolean() - Fix stale test description (expired -> temporary) - Comment out retentionMode default in example yaml - Simplify verbose if/else to isTemporary === true - Add compound index on { user: 1, isTemporary: 1 } - Remove narrating comment from process.spec.js Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit 6bad535f90) * chore: fix typescript (cherry picked from commit 826527a46b) * fix: lint (cherry picked from commit 77817e80ea) * fix: use mockSanitizeArtifactPath in retention test The 'getRetentionExpiry is called with the request object' test referenced an undefined `mockSanitizeFilename` identifier, breaking both lint (no-undef) and the test suite. Use the existing `mockSanitizeArtifactPath` mock that the surrounding tests already use, since `processCodeOutput` calls `sanitizeArtifactPath` (not `sanitizeFilename`) before invoking `getRetentionExpiry`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 52ea2da66d) * fix: forward isTemporary from client for retention on file uploads and tool calls Server-side `getRetentionExpiry` (file uploads) and the tool-call controller both read `req.body.isTemporary`, but the file upload multipart form and the tool-call payload did not include that field. In `retentionMode: temporary` (default), files uploaded and tool calls created from temporary chats were therefore retained indefinitely. Forward the Recoil `isTemporary` flag in both client paths so the existing server checks can fire correctly. `ToolParams` gains an optional `isTemporary` field. Addresses Codex P1 review feedback on PR #29. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 7e937df05a) * test: stub store.isTemporary in useFileHandling test mocks Previous commit added `useRecoilValue(store.isTemporary)` to the hook. The test file mocks `~/store` with only `ephemeralAgentByConvoId` and does not stub `useRecoilValue`, so all 7 cases threw "Invalid argument to useRecoilValue: expected an atom or selector but got undefined". Add a stub default export with `isTemporary` and a `useRecoilValue` mock returning `false`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit eb1609537d) * fix: harden data retention semantics * fix: provide sweep request context for expired files * fix: preserve temporary flags in all-retention updates * fix: honor assistant versions in retention sweeps * fix: retain non-temporary flags in all mode * fix: hide expired retained records * fix: propagate retained conversation expiry * fix: refresh meili retention cutoff * fix: prevent overlapping file sweeps * fix: show legacy retained conversations * fix: index legacy retained records * fix: harden retention cleanup edge cases * fix: count failed file storage sweeps * fix: preserve legacy temporary retention * fix: assign retention sweep worker deterministically * fix: hide expired shared links on reads * fix: prevent retention refresh after parent expiry * fix: break code output retention import cycle * fix: harden retention review findings * fix: ignore expired share duplicates * fix: reject expired retained share creation * fix: harden retention review edge cases * fix: address retention audit findings * fix: enforce expired conversation shares in all retention * fix: scope temporary upload flag to chat files * fix: address retention review findings * fix: address codex retention review findings * fix: tighten missing storage detection * test: remove unused file process spec bindings --------- Co-authored-by: WhammyLeaf <233105313+WhammyLeaf@users.noreply.github.com> Co-authored-by: Aron Gates <aron@muonspace.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-05-19 21:58:42 -04:00
Danny Avila	749eb06e67	🧭 fix: Reduce MCP Registry ACL Lookups (#13195)	2026-05-19 17:16:37 -04:00
Danny Avila	68eac104ad	🗂️ fix: Scope Handoff Agent Context Docs (#13167) ... * fix: Scope agent context docs to handoff agents * fix: Deduplicate scoped request context * refactor: Extract agent attachment helpers	2026-05-18 15:36:22 -04:00
Danny Avila	c342e2345b	🪪 fix: Resolve Group-Scoped Config Overrides (#13176) ... * fix: resolve group-scoped config overrides * test: fix endpoint config request mock typing * fix: keep remote agent preauth config tenant-scoped * test: align config scoping expectations * test: reproduce group endpoint override resolution	2026-05-18 10:16:20 -04:00
장병희	ac163c2212	🤝 fix: Honor OPENID_REUSE_TOKENS in Admin OAuth Exchange (#13154)	2026-05-18 09:34:58 -04:00
Danny Avila	89d10a0b83	🛡️ fix: Escape People Picker Search Regex (#13169)	2026-05-18 09:04:31 -04:00
Danny Avila	62da4c28ed	🛡️ fix: Sanitize Agent List Skill Scope (#13122)	2026-05-14 09:27:41 -04:00
Danny Avila	7b9a57a467	🛡️ fix: Harden OpenID Session Token Reuse (#13086) ... * fix: Harden OpenID Session Token Reuse * fix: Preserve OpenID Session Token On Forced Refresh * fix: Gate Preserved OpenID Id Token By Expiry * test: Cover OpenID Id Token Expiry Buffer	2026-05-11 23:29:01 -04:00
Danny Avila	17a08224e1	🍪 fix: Refresh CloudFront Cookies On Auth Refresh (#13083) ... * fix: Refresh CloudFront Cookies On Auth Refresh * fix: Exclude Federated Tokens From Refresh Lookup	2026-05-11 22:33:27 -04:00
Danny Avila	0a7255b234	🎭 feat: Support OpenID Audience On Refresh Grants (#13077)	2026-05-11 17:40:30 -04:00
Danny Avila	030dc98a1d	☁️ fix: Enable Azure Agent Provider Uploads (#13045)	2026-05-10 17:47:05 -04:00
Danny Avila	d90567204e	🛟 fix: persist Vertex Gemini 3 thoughtSignatures across DB round-trips (#13026) ... When a tool round-trip is interrupted between the tool result and the model's text reply (user aborted, network drop, pod restart, ...) and LibreChat persists the partial assistant message, the next conversation turn reconstructs an `AIMessage` from `formatAgentMessages` that has `tool_calls` populated but no `additional_kwargs.signatures`. Vertex Gemini 3 rejects the resumed request with 400 because the most recent historical functionCall has no `thought_signature`. ## Storage shape Capture as `Record<tool_call_id, signature>` rather than a flat array. This addresses the codex P1 review: > When an assistant turn contains multiple sequential tool-call batches, > this restoration path writes all persisted thoughtSignatures onto only > the last tool-bearing AIMessage. Vertex/Gemini validates signatures > for each step in the current tool-calling turn, so earlier > functionCall steps reconstructed without their signature can still > fail with 400. A single agent run can fire multiple `chat_model_end` events when the loop cycles the LLM with intervening tool results — each cycle owns a distinct `tool_call_id`. Per-id storage maps each signature back onto the right reconstructed `AIMessage`, not just the last one. ## Mapping `additional_kwargs.signatures` is a flat array indexed by *response part* (text + functionCall interleaved). `tool_calls` is just the function calls in their original order. Non-empty signatures correspond 1:1 with tool_calls in order — see `partsToSignatures` in `@langchain/google-common`. Single-pass walk maps `signatures[i]` (when non-empty) onto the i-th `tool_call.id`. ## Pipeline | Stage | File | Change | |---|---|---| | Capture | callbacks.js | `ModelEndHandler` accepts `Record<string,string>` map; walks signatures + tool_calls in tandem to record per-id. Gated on the map being provided — non-Vertex flows are no-op (and also no-op even when provided, since they don't emit signatures). | | Plumbing | initialize.js | Allocate `collectedThoughtSignatures = {}`, share with handler + client. Always allocated; the JSDoc explicitly documents that it stays empty for non-Vertex providers. | | Surface | client.js | `sendCompletion` returns `metadata.thoughtSignatures` when the map has entries; falls through unchanged when empty. | | Persist | (existing BaseClient.handleRespCompletion) | Writes `metadata` from `sendCompletion` onto `responseMessage.metadata`. Mongoose `Mixed` — no migration. | | Restore | formatMessages.js | Track every tool-bearing AIMessage produced from a TMessage. For each, build a position-aligned `additional_kwargs.signatures` array (empty placeholders for tool_calls without a stored sig). Agents' `fixThoughtSignatures` dispatches non-empty entries to functionCall parts in order. | ## Live verification - **Single-step:** real Vertex `gemini-3.1-flash-lite-preview` resume-after-tool case. With fix ✅ / without ❌ 400. - **Multi-step (codex case):** real two-step agent loop (list /tmp → echo done). Each step's signature attaches to its own reconstructed AIMessage. With fix ✅ / without ❌ 400. - **Cross-provider:** Anthropic Claude haiku-4.5 + OpenAI gpt-5-mini accept the persisted/restored shape unchanged. ## Tests `modelEndHandler.spec.js` (new) — 6 tests: - maps non-empty signatures onto tool_call_ids in order - accumulates per-id across multiple `model_end` events (multi-step) - no-op when `collectedThoughtSignatures` is null - no-op when `signatures` field missing (non-Vertex) - no-op when `tool_calls` missing - preserves existing `collectedUsage` array contract `formatAgentMessages.spec.js` — 6 new tests: - restores onto the AIMessage that owns the tool_call - per-step attachment for multi-step turns (codex review case) - preserves tool_call ordering when signatures are partial - no-op when metadata.thoughtSignatures absent - no-op when assistant has no tool_calls - no-op when stored ids don't match any current tool_call 37 passing across 3 suites; 15 existing formatAgentMessages tests unchanged. ## Compatibility - Backward-compatible — restore gated on `metadata.thoughtSignatures` being a populated object; capture gated on the map being provided. - No schema migration — uses `Message.metadata: Mixed` already in place. - Cross-provider safe — non-Vertex providers tolerate the field (verified live against Anthropic + OpenAI converters). - Pairs with [agents#159](https://github.com/danny-avila/agents/pull/159) for full coverage on histories that mix plain-text and toolcall AIMessages.	2026-05-08 18:51:34 -04:00
Dustin Healy	e262219c8f	🔄 feat: Cross-Origin Admin OAuth Refresh (#13007) ... * feat(admin-panel): add /api/admin/oauth/refresh endpoint for cross-origin BFF refresh The cookie-based /api/auth/refresh controller can't be reached cross-origin from a separately-hosted admin panel because the refresh-token cookie isn't sent on cross-origin fetches. Add a dedicated POST /api/admin/oauth/refresh endpoint that accepts the refresh token in the request body, exchanges it at the IdP via openid-client refreshTokenGrant, and returns the same response shape as /api/admin/oauth/exchange. Implementation lives in packages/api/src/auth/refresh.ts as the applyAdminRefresh helper. It validates the refreshed tokenset, looks up the admin user by openidId (with optional user_id disambiguation when multiple user docs share an openidId), mints the bearer via an injected mintToken hook, and runs an optional onRefreshSuccess hook for downstream forks that need to update server-side session state. The default mintToken passed by the OSS route signs an HS256 LibreChat JWT via generateToken so admin panel callers continue to use the existing local JWT strategy. Forks that prefer to hand back an IdP-signed token (e.g. for deployments where the JWT auth gate is JWKS-only) override mintToken without changing the helper or the route. Also threads expiresAt through AdminExchangeData and AdminExchangeResponse so admin panel clients can drive proactive refresh before the bearer expires. Defaults the OSS exchange flow to Date.now() + sessionExpiry. * fix(admin-panel): address review feedback on /api/admin/oauth/refresh mintToken now returns {token, expiresAt} so the minter is authoritative for the bearer's lifetime instead of deriving it from the IdP `exp` claim. The refresh response would otherwise lie to the admin panel and trigger premature or late refresh cycles. The helper now falls back to the inbound refresh_token when the IdP omits one on rotation (Auth0 with rotation off, Microsoft personal accounts). Without this the admin panel loses its refresh capability after one cycle. Other hardening: resolveAdminUser validates user_id with Types.ObjectId.isValid before hitting Mongoose, avoiding a CastError that would surface as a generic 500 with no useful information for the client. If user_id resolves to a user whose openidId does not match the refreshed sub, throw USER_ID_MISMATCH (401) instead of silently swapping in a different user matching the sub. Wrap tokenset.claims() in readClaims so an IdP that returns a tokenset without a usable id_token gets mapped to CLAIMS_INCOMPLETE (502) rather than bubbling a raw exception. findUsers now uses the same SAFE_USER_PROJECTION as getUserById so the fallback path no longer pulls password/totpSecret/backupCodes into memory. Removed dead fields (email on AdminRefreshClaims, id_token on RefreshTokenset) and fixed import ordering per AGENTS.md. Adds packages/api/src/auth/refresh.spec.ts: 18 tests covering the happy path, userId disambiguation (match, invalid ObjectId, null, mismatch), all error branches (IDP_INCOMPLETE, CLAIMS_INCOMPLETE for both throw and missing sub, USER_NOT_FOUND, mintToken/onRefreshSuccess propagation), and refresh-token preservation under rotation/no-rotation. * chore(admin-panel): polish per re-review on /api/admin/oauth/refresh readClaims now logs the original error name/message at warn before mapping to CLAIMS_INCOMPLETE so a programming bug doesn't get silently rebadged as an IdP problem in production logs. The route handler's JSDoc now enumerates every error response (status + error_code) so admin-panel implementors can plan for each branch without reading the source. Tightens the helper's surface: removed the now-dead `exp` field from `AdminRefreshClaims` (only `sub` is read since the v2 mintToken refactor), and tightened `AdminRefreshDeps.findUsers`'s projection parameter from `string | null` to `string` so the contract matches actual usage. Test polish: the userId-resolves-to-null fallthrough test now asserts the exact `findUsers` and `getUserById` call arguments so a regression in the fallthrough query shape is caught. The "skips onRefreshSuccess" test now asserts a populated response shape rather than just `toBeDefined`. Declined per prior triage and re-confirmed: a role guard inside `applyAdminRefresh` (downstream `/api/admin/*` already enforces ACCESS_ADMIN via requireCapability) and moving the IdP grant call out of the JS route into TypeScript (matches existing oauth.js / openidStrategy pattern; package-boundary refactor belongs in a separate PR). * fix(admin-panel): reject /api/admin/oauth/refresh tokensets from foreign issuers When the route handler can resolve the configured OpenID issuer, it now threads it into applyAdminRefresh as expectedIssuer. The helper compares that against the tokenset claims iss (after normalizeOpenIdIssuer on both sides to absorb trailing-slash differences) and throws ISSUER_MISMATCH (401) on mismatch. The check is skipped when either side is unset so behavior is unchanged for IdPs that don't return iss on a refresh-grant id_token, and for older deployments where the OpenID config doesn't expose serverMetadata. This is a defense-in-depth measure for the refresh path only. The deeper OIDC posture fix (binding IUser lookup to (sub, iss) as a pair) is pre-existing debt across openidStrategy.js and the regular exchange flow as well, and belongs in a separate PR with the schema change and backfill migration. * fix(admin-panel): bind refresh user lookup to (sub, iss) and handle getOpenIdConfig throw Two fixes raised on the PR thread that I previously misdescribed: The user lookup in resolveAdminUser was keyed on openidId alone, so a tokenset from a different issuer that happened to share the same sub could resolve to a local user from a different IdP. Now exports getIssuerBoundConditions and isUserIssuerAllowed from openid.ts (the helpers findOpenIDUser already uses) and reuses them. The findUsers filter becomes ($or of getIssuerBoundConditions for openidId) when an expectedIssuer is provided, with the same legacy backward-compat clause for users whose openidIssuer field was never populated. The direct user_id path now also checks isUserIssuerAllowed and throws USER_ID_MISMATCH if the stored openidIssuer disagrees with the configured issuer. The route's getOpenIdConfig() call was previously documented as returning null when uninitialized; the actual implementation throws. That made the if (!openIdConfig) guard unreachable, and an unconfigured server would surface as 500 INTERNAL_ERROR rather than 503 OPENID_NOT_CONFIGURED. Wraps the call in try/catch so the documented 503 response is what callers actually receive. Adds 4 tests covering the new lookup binding behavior. * fix(admin-panel): re-check ACCESS_ADMIN on /api/admin/oauth/refresh The IdP refresh token can outlive a capability/role change, so the initial requireAdminAccess on the OAuth callback isn't sufficient. Inject canAccessAdmin via the existing capability model (hasCapability with SystemCapabilities.ACCESS_ADMIN, matching requireAdminAccess so custom roles and user grants are honored) and gate token minting on it. Capability backend errors are warn-and-denied to keep the bearer-mint path fail-closed. * fix(admin-panel): scope /api/admin/oauth/refresh to the request tenant The same (openidId, openidIssuer) pair is allowed across tenants by the user schema's unique index. The refresh helper was wrapping both the direct getUserById and the fallback findUsers in runAsSystem, bypassing tenant isolation, so an IdP identity that exists in two tenants could resolve to the wrong tenant's user and mint a JWT bound to that tenant. Drop the runAsSystem wrappers, add a trusted tenantId option to applyAdminRefresh, AND it into the fallback findUsers filter, and assert it against the direct getUserById result. Mount preAuthTenantMiddleware on the refresh route so the deployment's X-Tenant-Id header drives the trusted tenant via ALS. Single-tenant deploys (no header) keep the existing openidId-only behaviour. Adds TENANT_MISMATCH (401) and a regression covering duplicate (sub, iss) across tenants plus the direct-userId tenant assertion. * fix(admin-panel): gate /api/admin/oauth/refresh on OPENID_REUSE_TOKENS The OSS refreshController only refreshes OpenID tokensets when OPENID_REUSE_TOKENS is enabled. The body-based admin variant was unconditionally calling refreshTokenGrant, which made the flag ineffective for the admin OAuth flow and let admin sessions keep renewing in deployments that explicitly turned token reuse off. Add the same isEnabled(process.env.OPENID_REUSE_TOKENS) check up front and return 403 TOKEN_REUSE_DISABLED so the admin panel BFF can surface the configuration mismatch instead of silently churning through retries.	2026-05-08 17:23:02 -04:00
Danny Avila	93c4ef4ba8	🧱 refactor: typed CodeEnvRef + kind discriminator + principal-aware sandbox cache (#12960) ... * 🧱 refactor: typed CodeEnvRef + kind discriminator + tenant-aware sandbox cache Final cutover for the LibreChat ↔ codeapi sandbox file identity. Replaces the magic string `${session_id}/${file_id}?entity_id=...` with a typed, discriminated `CodeEnvRef`. Pre-release lockstep deploy with codeapi #1455 and agents #148; no legacy aliases retained. ## Final shape ```ts type CodeEnvRef = | { kind: 'skill'; id: string; storage_session_id: string; file_id: string; version: number } | { kind: 'agent'; id: string; storage_session_id: string; file_id: string } | { kind: 'user'; id: string; storage_session_id: string; file_id: string }; ``` `kind` drives codeapi's sessionKey: `<tenant>:<kind>:<id>[✌️<version>]` for shared kinds, `<tenant>:user:<userId>` for user-private (auth context provides `userId`). `version` is statically required for `kind: 'skill'` and forbidden otherwise via discriminated union — constraint holds at compile time on every consumer, not just codeapi's runtime validator. `id` is sessionKey-meaningful for `'skill'` / `'agent'`; informational only for `'user'` (codeapi resolves user identity from auth context). ## What changed - `packages/data-provider/src/codeEnvRef.ts` — discriminated union + `CODE_ENV_KINDS` const-tuple keeps the runtime list and TS union locked together. - Schemas: `metadata.codeEnvRef` and `SkillFile.codeEnvRef` enums tightened to `['skill', 'agent', 'user']`. - `primeSkillFiles` writes `kind: 'skill'`, `id: skill._id`, `version: skill.version`. Cache-hit path reads `codeEnvRef` directly. Bumping `skill.version` on edit naturally invalidates the prior cache entry under the new sessionKey. - `processCodeOutput` writes `kind: 'user'`, `id: req.user.id`. Output bucket is always user-scoped, regardless of which skill the execution invoked. New regression test pins the asymmetry. - `primeFiles` reupload preserves `kind`/`id`/`version?` from the existing ref so a skill-cache-miss reupload doesn't silently demote to user bucket. - `crud.js` upload functions (`uploadCodeEnvFile` / `batchUploadCodeEnvFiles`) thread `kind`/`id`/`version?` to the multipart form (codeapi #1455 option α). Without these on the wire, codeapi falls back to user bucketing and skill-cache invalidation never fires. Client-side validation mirrors codeapi's validator. - `Files/process.js` — chat attachments use `kind: 'user'`; agent setup files use `kind: 'agent'`. - Drops `entity_id` everywhere (struct, schema sub-docs, write paths, upload form fields). Drops `'system'` from the kind enum (no emitter ever existed). ## Test plan - [x] `cd packages/data-provider && npx jest src/codeEnvRef.spec` — 4 / 4 - [x] `cd packages/data-schemas && npx jest` — 1447 / 1447 - [x] `cd packages/api && npx jest src/agents` — 81 / 81 in skillFiles + handlers + resources - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 436 / 436 - [x] `cd api && npx jest server/services/Files/Code` — 98 / 98 (incl. new "outputs are user-scoped regardless of which skill the execution invoked" regression and "reupload forwards kind/id/version from existing ref") - [x] `npx tsc --noEmit -p packages/data-{provider,schemas}/tsconfig.json && npx tsc --noEmit -p packages/api/tsconfig.json` — clean (only pre-existing unrelated dev errors in storage/balance, untouched here) ## Deploy notes - **24h cache-miss burst** on first deploy. Inputs (skill caches re-prime under new sessionKey shape) and outputs (any pre-Phase C skill-output cached files become unreadable). Bounded by codeapi's 24h TTL. - **Lockstep with codeapi #1455 and agents #148.** Either repo can land first since no aliases to drain, but the three deploys must overlap within the same maintenance window. - **`@librechat/agents` bump to `3.1.79-dev.0`** required after agents #148 lands and is published. ## What this enables Auth bridge work (JWT-based tenant/user identity between LC and codeapi) — codeapi now derives sessionKey purely from `req.codeApiAuthContext.{ tenantId, userId}`, so the next chapter is replacing the header-asserted user identity with a verified-claim path. * 🩹 fix: persist execute_code uploads under codeEnvRef metadata key Codex review P1 (chatgpt-codex-connector). `Files/process.js` was storing the upload result under `metadata.fileIdentifier` even though: - `uploadCodeEnvFile` now returns `{ storage_session_id, file_id }`, not the legacy magic string. - The post-cutover schema (`File.metadata.codeEnvRef`) only declares `codeEnvRef` — mongoose strict mode silently strips unknown keys. - All readers (`primeFiles`, `getCodeFilesByIds`, `categorizeFileForToolResources`, controller filtering) check `metadata.codeEnvRef`. Net effect of the bug: chat-attached and agent-setup execute_code files would lose their sandbox reference on save, and primeFiles would skip them on subsequent code-execution turns — the file blob would still be available locally but never re-mounted in the sandbox. Fix: construct the full `CodeEnvRef` (`{ kind, id, storage_session_id, file_id }`) at the write site and persist under `metadata.codeEnvRef`. `BaseClient`'s "is this a code-env file" presence check accepts the new shape alongside the legacy `fileIdentifier` for back-compat with any pre-cutover records still in the database. Mirrors the same change in `processAttachments.spec.ts` (which re-implements the BaseClient logic for testability). New regression tests in `process.spec.js` cover three cases: - chat attachments (`messageAttachment=true`) → `kind: 'user'` - agent setup (`messageAttachment=false`) → `kind: 'agent'` - legacy `fileIdentifier` key is NOT persisted (would be schema-stripped) * 🩹 fix: read storage_session_id on primed file refs (Codex P1) Codex review (chatgpt-codex-connector). After Phase B's per-file `session_id` → `storage_session_id` rename, `primeFiles` emits the new field — but `seedCodeFilesIntoSessions` was still reading `files[0].session_id` for the representative session and `f.session_id` for the dedupe key. In runs with only primed attachments (no skill seed), `representativeSessionId` was `undefined`, the function returned the unchanged map, and `seedCodeFilesIntoSessions` silently dropped the entire batch. The first `execute_code` call then started without `_injected_files` and the agent couldn't see prior-turn artifacts. Fix: - `codeFilesSession.ts`: read `f.storage_session_id` for both the dedupe key and the representative session id. JSDoc updated to match the new field name. - `callbacks.js`: the two output-file persistence paths read `file.session_id` to pass to `processCodeOutput` — switch to `file.storage_session_id`. The original comment explicitly says this should be the STORAGE session, which is exactly the field Phase B renamed. - `codeFilesSession.spec.ts`: fixture builder uses `storage_session_id` and `kind: 'user'` to match the post-cutover `CodeEnvFile` shape. Lockstep coordination: this matches the post-bump shape of `@librechat/agents` 3.1.79+. CI tsc errors against the currently-pinned 3.1.78 are expected and resolve when the dep bumps in this PR before merge. * 📦 chore: Bump `@librechat/agents` to version 3.1.80-dev.0 in package-lock and package.json files * 🪪 fix: thread kind/id/version through codeapi /download URLs (Phase C α) Symmetric fix for the upload-side wire change in 537725a. Codeapi's `sessionAuth` middleware now requires `kind`/`id`/`version?` on every download/freshness URL — without them it 400s with "kind must be one of: skill, agent, user" before serving the file. Three sites construct codeapi-side URLs that go through `sessionAuth`: - `processCodeOutput` (`Files/Code/process.js`): `/download/<sess>/<id>` for freshly-generated sandbox outputs. Always `kind: 'user'` + `id: req.user.id` — code-output files are always user-private, regardless of which skill the run invoked. - `getSessionInfo` (`Files/Code/process.js`): `/sessions/<sess>/objects/<id>` for the 23h freshness check. Pulls kind/id/version straight off the `codeEnvRef` already in scope — skill files stay skill-bucketed, user files stay user-bucketed. - `/code/download/:session_id/:fileId` LC route (`routes/files/files.js`): proxies to codeapi for manual downloads. Code-output files only on this route, so `kind: 'user'` + `id: req.user.id`. The `getCodeOutputDownloadStream` helper in `crud.js` now takes an `identity` param, validated by a `buildCodeEnvDownloadQuery` helper that mirrors `appendCodeEnvFileIdentity`'s shape rules: kind required from the closed `{skill, agent, user}` set, version required for 'skill' and forbidden otherwise. Bad callers fail fast on the client instead of round-tripping a 400. Also cleans up two log-noise sources reported alongside the 400: - `logAxiosError` in `packages/api/src/utils/axios.ts` was dumping `error.response.data` raw. With `responseType: 'arraybuffer'` that's a `Buffer` (~4 chars per byte after JSON-serialization); with `responseType: 'stream'` it's a `Readable` whose internal state serializes the entire ring buffer + socket. New `renderResponseData` decodes small buffers as UTF-8 (truncated past 2KB) and stubs streams as `'[stream]'`. Diagnostics stay useful, log lines stop being megabytes. - `/code/download` route's catch was bare `logger.error('...', error)`, bypassing the redactor. Switched to `logAxiosError` so it benefits from the same buffer/stream handling. Tests updated to match the new contract: - crud.spec: `getCodeOutputDownloadStream` fixtures pass `userIdentity`; new cases cover skill identity (with version), bad kind rejection, skill-without-version rejection. - process.spec: `getSessionInfo` test passes a full `codeEnvRef` object. * ♻️ refactor: extract codeEnv identity helpers into packages/api Per the project convention that new backend code lives in TypeScript under `packages/api`, moves `appendCodeEnvFileIdentity` and `buildCodeEnvDownloadQuery` from `api/server/services/Files/Code/crud.js` into a new `packages/api/src/files/code/identity.ts` module. Both helpers are pure validators that mirror codeapi's `parseUploadSessionKeyInput` server-side rules (closed kind set, `version` required for `'skill'` and forbidden otherwise) — they deserve TS support and a dedicated spec rather than living as JSDoc-typed helpers in the legacy `/api` workspace. The new module: - Exports a `CodeEnvIdentity` interface using the `librechat-data-provider` `CodeEnvKind` discriminated union. - Adds 13 unit tests in `identity.spec.ts` covering the validation matrix (skill+version, agent, user, and every rejection path) plus URL encoding for the download query. - Re-exported from `packages/api/src/files/code/index.ts` alongside `classify`, `extract`, and `form`. Consumer updates: - `api/server/services/Files/Code/crud.js`: drops the local helpers and imports them from `@librechat/api`. Net -64 lines. - `api/server/services/Files/Code/process.js`: same. - Test mocks for `@librechat/api` in three spec files now stub the helpers' validation behavior locally rather than pulling them through `requireActual` (which would drag in provider-config init-time side effects). The package's `exports` field only surfaces the root barrel, so leaf imports aren't reachable from legacy `/api` test setup. No runtime behavior change. Identity validation rules and emitted form/query shapes are byte-for-byte identical pre/post. * 🪪 fix: emit resource_id alongside id on _injected_files (skill 403 fix) Companion to codeapi #1455 fix and agents 3.1.80-dev.1 — the wire shape for shared-kind files now requires `resource_id` distinct from the storage `id`. Without this LC change, codeapi's sessionKey re-derivation on every shared-kind /exec rejects with 403 session_key_mismatch: cached: legacy:skill:69dcf561...✌️59 (signed at upload, skill _id) derived: legacy:skill:ysPwEURuPk-...✌️59 (storage nanoid) Emit sites updated: - `primeInvokedSkills` cache-hit path: `resource_id: ref.id` (the persisted skill `_id` from `codeEnvRef.id`); `id: ref.file_id` unchanged (storage uuid). - `primeInvokedSkills` fresh-upload path: `resource_id: skill._id.toString()` on every primed file (the `allPrimedFiles` builder type now carries the field). - `processCodeOutput`'s `pushFile` (Code/process.js): `resource_id: ref.id` — for `kind: 'user'` this is informational (codeapi derives sessionKey from auth context) but emitted for shape uniformity with shared kinds. Bumps `@librechat/agents` to `^3.1.80-dev.1` (the version that ships the matching `CodeEnvFile.resource_id` field). ## Test plan - [x] `cd packages/api && npx jest src/agents` — 67 / 67 pass (skillFiles fixtures updated to assert `resource_id` on the emitted CodeSessionContext.files). - [x] `cd api && npx jest server/services/Files server/controllers/agents` — 445 / 445 pass (process.spec fixtures updated for the reupload + cache-hit emission). - [x] `npx tsc --noEmit -p packages/api/tsconfig.json` — clean. * fix(skill-tool-call): carry resource_id through primeSkillFiles → artifact Codeapi was 400ing every /exec following a `handle_skill` tool call with `resource_id is invalid` (`type: 'undefined'`). Both code paths in `primeSkillFiles` (cache-hit + fresh-upload) returned files without `resource_id`/`kind`/`version`, and the artifact in `handlers.ts` forwarded the stripped shape into `tc.codeSessionContext.files` → `_injected_files`. `primeInvokedSkills` (the NL-detected loader) had already been fixed end-to-end; this commit aligns the tool-invoked path with the same contract: `resource_id` = `skill._id.toString()`, `kind: 'skill'`, `version` = the skill's monotonic counter. Tests added to `skillFiles.spec.ts` lock the contract on `primeSkillFiles` directly so future refactors can't silently drop the resource identity again. * fix(handlers.spec): align session_id → storage_session_id rename + kind discriminator Pre-existing TS errors against the post-rename `CodeEnvFile` shape: the test file still used `session_id` on per-file objects (renamed to `storage_session_id` in agents Phase B/C) and was missing the `kind` discriminator the discriminated union requires. Both inputs and the matching `expect.toEqual(...)` mirrors updated together so the runtime equality check still holds. Lines 723-732 stay as-is — they sit behind `as unknown as ToolCallRequest` and TS already skipped them. * chore: fix `@librechat/agents`, correct version to 3.1.80-dev.0 in package.json files * chore: bump `@librechat/agents` to version 3.1.80-dev.1 in package.json and package-lock.json * chore: bump `@librechat/agents` to version 3.1.80-dev.2 * feat(observability): trace file priming chain from primeCodeFiles to _injected_files Diagnosing the user-upload "files=[] on first /exec" bug requires seeing where in the LC chain a file ref disappears. Prior to this patch the chain (primeCodeFiles → primedCodeFiles → initialSessions → CodeSessionContext → _injected_files) was opaque end-to-end: - primeCodeFiles silently dropped files without `metadata.codeEnvRef` - reuploadFile catches all errors and continues with no signal - the handlers.ts handoff to codeapi never logged what it was sending After this patch, a single grep on `[primeCodeFiles]` plus `[code-env:inject]` shows the full per-file path: [primeCodeFiles] in: file_ids=N resourceFiles=M [primeCodeFiles] file=<id> path=skip reason=no-codeenvref filename=... [primeCodeFiles] file=<id> path=cache-hit-by-session storage_session_id=... [primeCodeFiles] file=<id> path=reupload reason=no-uploadtime ... [primeCodeFiles] file=<id> path=reupload reason=stale ... [primeCodeFiles] file=<id> path=reupload-success oldSession=... newSession=... newFileId=... [primeCodeFiles] file=<id> path=reupload-failed session=... [primeCodeFiles] file=<id> path=fresh-active storage_session_id=... [primeCodeFiles] out: returned=N skippedNoRef=M reuploadFailures=K [code-env:inject] tool=<name> files=N missingResourceId=K (debug) [code-env:inject] M/N files missing resource_id ... (warn) [code-env:inject] tool=<name> _injected_files=0 ... (warn) The boundary log warns when LC sends zero injected files on a code-execution tool call — that's the user's actual symptom showing up at the LC side instead of having to correlate against codeapi's `Request received { files: [] }`. Tag chosen as `[code-env:inject]` rather than `[handoff:exec]` to avoid collision with the app-level "handoff" semantic (subagent handoff workflow). Structural cleanup in primeFiles: replaced the `if (ref) { ... }` nesting with an early `if (!ref) continue` so the per-path instrumentation hooks land at top-level scope instead of indented inside a conditional. Behavior unchanged; pushFile / reuploadFile identical. Spec fixtures (handlers.spec.ts, codeFilesSession.spec.ts) updated to include `resource_id` on `CodeEnvFile` literals — required by the post-3.1.80-dev.2 type now installed. ## Test plan - [x] `cd packages/api && npx jest src/agents/handlers.spec.ts src/agents/codeFilesSession.spec.ts src/agents/skillFiles.spec.ts` — 69/69 pass - [x] `cd api && npx jest server/services/Files/Code/process.spec.js` — 84/84 pass - [x] `npx tsc --noEmit -p packages/api` — clean - [x] `npx eslint` on all four touched files — clean * chore: add CONSOLE_JSON_STRING_LENGTH to .env.example for JSON log string length configuration * fix(files): align codeapi upload filename with LC's sanitized DB filename User-attached files for code execution were uploading to codeapi under `file.originalname` (raw upload filename, may contain spaces / special chars) while LC's DB record stored the sanitized form (`sanitizeFilename(file.originalname)`, underscores). Codeapi preserves whatever filename the upload sent, so the sandbox saw `/mnt/data/<originalname>` while LC's `primeFiles` toolContext text + `_injected_files.name` referenced `file.filename` (sanitized). Visible failure: agent gets system prompt saying /mnt/data/librechat_code_api_-_active_customer_-_2025-11-05.xlsx …tries that path, hits `FileNotFoundError`, then notices the sandbox's actual `Available files` line says /mnt/data/librechat code api - active customer - 2025-11-05.xlsx …retries with spaces, succeeds. Wastes a tool call per upload and leaks raw filenames into model context. Fix: sanitize once and use the sanitized form in both the codeapi upload AND the LC DB record. Sandbox path = LC toolContext text = in-memory ref name. No drift. Reupload path (`Code/process.js` line 867 `filename: file.filename`) already uses the sanitized DB name, so it stays consistent with the fresh-upload path after this change. ## Test plan - [x] `cd api && npx jest server/services/Files/process` — 32/32 pass - [x] `npx eslint` on the touched file — clean * chore: bump `@librechat/agents` to version 3.1.80-dev.3 in package.json and package-lock.json	2026-05-08 12:29:43 -04:00
Danny Avila	5c338a4642	🛂 fix: Harden Agent File Preview Access (#12981) ... * fix: harden agent file access * style: format agent file query * fix: prune agent file refs on alternate writes * test: fix agent pruning specs	2026-05-06 19:56:04 -04:00
Danny Avila	9c81792d25	🔐 feat: Add Signed CloudFront File Downloads (#12970) ... * feat: add signed CloudFront downloads * fix: preserve local IdP avatar paths * fix: address signed download review findings * fix: harden CloudFront cookie scope validation * fix: preserve URL save API compatibility * fix: store CDN SSO avatars under shared prefix * fix: Harden CloudFront tenant file access * fix: Preserve CloudFront download compatibility * fix: Address CloudFront review follow-ups * fix: Preserve file URL fallback user paths * fix: Address download review hardening * fix: Use file owner for S3 RAG cleanup * fix: Address final download review nits * fix: Clear stale avatar CloudFront cookies * fix: Align download filename helpers with dev * fix: Address final CloudFront review follow-ups * fix: Stream S3 URL uploads * fix: Set S3 stream upload length * fix: Preserve download metadata filepath * fix: Avoid remote content length for stream uploads * fix: Use bounded multipart URL uploads * fix: Harden S3 filename boundaries	2026-05-06 19:48:30 -04:00
Danny Avila	6c6c72def7	🚀 feat: Decouple File Attachment Persistence from Preview Rendering (#12957) ... * 🗂️ feat: add `status` lifecycle to file records for two-phase previews Schema and model foundation for decoupling the agent's final response from CPU-heavy office-format HTML extraction. - `MongoFile.status: 'pending' | 'ready' | 'failed'` (indexed) and `previewError?: string` mirror the lifecycle: phase-1 emits the file record at `pending` so the response is unblocked; phase-2 transitions to `ready` (with text/textFormat) or `failed` (with previewError) in the background. Absent for legacy records — clients treat that as `ready` for back-compat. - Mirror types added to `TFile` in data-provider so frontend cache consumers see the new fields. - New `sweepOrphanedPreviews(maxAgeMs)` method on the file model recovers stale `pending` records left behind by a process restart mid-extraction; transitions them to `failed` with `previewError: 'orphaned'`. Cheap because `status` is indexed. * ⚡ feat: two-phase code-execution preview flow (unblocks final response) The agent's final response no longer waits on CPU-heavy office HTML extraction. Phase-1 (download + storage save + DB record at `status: 'pending'`) is awaited as before; phase-2 (extract + `updateFile`) runs in the background with a hard 60s ceiling. Three flows, all funneling through `processCodeOutput` and updated to the new `{ file, finalize? }` return shape: - `callbacks.js` (chat-completions + Open Responses streaming): emit the phase-1 attachment immediately (carries `status: 'pending'` for office buckets so the UI shows "preparing preview…"), then fire-and-forget `finalize()`. If the SSE stream is still open when phase-2 lands, push an `attachment` update event with the same `file_id` so the client merges over the placeholder in place. - `tools.js` direct endpoint: same split — return the phase-1 metadata immediately, run extraction in the background. Client polls for the resolved record. `finalize()` wraps the existing 12s per-render timeout in a 60s outer `withTimeout`. The HTML-or-null contract from #12934 is preserved: office types that fail extraction transition to `status: 'failed'` with `previewError: 'parser-error' | 'timeout'` rather than falling back to plain text (would be an XSS vector). Promises continue running after the HTTP response closes (Node doesn't kill them). The boot-time orphan sweep covers the only case that loses progress — actual process restart mid-extraction. `primeFiles` annotates the agent's `toolContext` line for prior-turn files: `(preview not yet generated)` for pending, `(preview unavailable: <reason>)` for failed. The model can volunteer "you can still download it" instead of pretending the preview is fine. `hasOfficeHtmlPath` exported from `@librechat/api` so `processCodeOutput` can decide whether a file expects a preview at all. * 🔍 feat: `GET /api/files/:file_id/preview` endpoint and boot orphan sweep - New `GET /api/files/:file_id/preview` route returns `{ status, text?, textFormat?, previewError? }`. The frontend's `useFilePreview` React Query hook polls this while phase-2 is in flight, then auto-stops on terminal status. ACL identical to the download route (reuses `fileAccess` middleware). Defaults `status` to `'ready'` for legacy records so back-compat is implicit. `text` only included when `status === 'ready'` and non-null — preserves the HTML-or-null security contract from #12934. - `sweepOrphanedPreviews()` invoked on boot in both `server/index.js` and `server/experimental.js`. Recovers any `pending` records left behind by a process restart mid-extraction (the only case the in-process two-phase flow can't handle on its own). Fire-and-forget so a transient sweep failure doesn't block startup. * 🖥️ feat: frontend two-phase preview consumer (polling + UI states) Wires the React side to the new lifecycle so the user sees what's happening with their file while phase-2 extraction runs in the background and after the response stream closes. - `useAttachmentHandler` upserts by `file_id` (was append-only) so the phase-2 SSE update event merges over the pending placeholder in place. Lightweight attachments without a `file_id` (web_search / file_search citations) keep the legacy append path. - `useFilePreview(file_id)` React Query hook with `refetchInterval: (data) => data?.status === 'pending' ? 2500 : false` so polling auto-stops on the first terminal response without the caller having to flip `enabled`. - `useAttachmentPreviewSync(attachment)` bridges polled data into `messageAttachmentsMap`. Polling enabled iff `status === 'pending' && isAnySubmitting` — per the design ask: active polling while the LLM is still generating, then quiet. Process-restart and post-stream cases are covered by polling on the next interaction. - `Attachment.tsx` renders a small `PreviewStatusIndicator` (spinner + "Preparing preview…" for pending, alert icon + "Preview unavailable" for failed) inside `FileAttachment`. Download button stays fully functional in both states. Two new English locale keys. - Data-provider scaffolding: `TFilePreview` type, `endpoints.filePreview`, `dataService.getFilePreview`, `QueryKeys.filePreview`. * 🧪 fix: stub `useAttachmentPreviewSync` in pre-existing Attachment test mocks The new `useAttachmentPreviewSync` hook is called unconditionally inside `FileAttachment` (added in the prior commit). Two pre-existing test files mock `~/hooks` to provide `useLocalize` only — the un-mocked preview hook reference resolved to undefined and crashed render with `(0 , _hooks.useAttachmentPreviewSync) is not a function` on the Ubuntu/Windows CI runners. Fix is local to the test mocks: add a no-op stub that returns `{ status: 'ready' }` so the component renders the legacy chip path. The two-phase preview behavior itself has its own dedicated suites (`useAttachmentHandler.spec.tsx`, `useAttachmentPreviewSync.spec.tsx`). * 🐛 fix: route phase-2 attachment update to current-run messageId Codex P1 review on PR #12957. `processCodeOutput` intentionally preserves the original DB `messageId` across cross-turn filename reuse so `getCodeGeneratedFiles` can still trace a file back to the assistant message that originally produced it. The phase-1 SSE emit already routes by the current run's messageId — `processCodeOutput` runtime-overlays it via `Object.assign(file, { messageId, toolCallId })` and the callback writes `result.file` directly. Phase-2 was passing the raw `updateFile` return through `attachmentFromFileMetadata`, which read `messageId` straight off the DB record. On a turn-N run that re-emitted a filename from turn-1 (e.g. agent writes `output.csv` again), the phase-2 SSE update routed to `turn-1-msg` instead of `turn-N-msg`. Frontend's `useAttachmentHandler` upserts under the wrong messageAttachmentsMap slot — turn-N's pending chip stays stuck at "preparing preview…" while turn-1's already-resolved attachment gets re-merged. Fix: thread `runtimeMessageId` through `attachmentFromFileMetadata` and pass `metadata.run_id` from the phase-2 emit site. Mirrors how phase-1 sources its messageId. Tests cover the cross-turn reuse case plus the writableEnded / null-finalize / no-finalize paths to lock in the broader phase-2 emit contract. * 🛠️ refactor: address codex audit findings (wire-shape parity, DRY, defensive catch) Comprehensive audit on PR #12957. Resolves all valid findings: - **MAJOR #1 — Wire-shape parity**: phase-1 ships the full `fileMetadata` record over SSE; phase-2 was using a tight `attachmentFromFileMetadata` projection. Drop the projection and have phase-2 spread `{...updated, messageId, toolCallId}` so both events match the long-standing legacy phase-1 shape clients depend on. - **MAJOR #2 — DRY**: extract `runPhase2Finalize({ finalize, fileId, onResolved })` into `process.js` (alongside `processCodeOutput` whose contract it pairs with). Both `callbacks.js` paths and `tools.js` now flow through it. Single catch path eliminates divergence surface — the fix landed in 01704d4f0 (cross-turn messageId routing) was a symptom of this duplication risk. - **MINOR #3 — JSDoc accuracy**: `finalizePreview`'s buffer is bounded by `fileSizeLimit`, not the 1MB extractor cap. Updated and added a note about peak heap from queued buffers. - **MINOR #4 — Defensive catch**: `runPhase2Finalize`'s catch attempts a best-effort `updateFile({ status: 'failed', previewError: 'unexpected' })` for the file_id, so a programming bug in `finalizePreview` doesn't leave the record stuck `'pending'` until the next boot-time orphan sweep. - **NIT #6 — Stale PR refs**: 12952 → 12957 in 3 places. - **NIT #7 — Schema bound**: `previewError` capped at `maxlength: 200` to prevent a future codepath from accidentally persisting a stack trace. Skipped per audit verdict (non-blocking): - #5 (memory pressure): documented in JSDoc; impl change was reviewer's "consider", not actionable. - #8 (double DB query per poll): low cost, indexed by_id, polling is gated narrow. - #9 (TAttachment cast): the union type is intentional; the casts are safe widening, refactoring TAttachment is invasive and out of scope. Tests: 11 new (7 `runPhase2Finalize` unit tests covering happy path, null-finalize, throws, double-fail, no-fileId, no-onResolved; +4 wire-shape parity assertions in the existing cross-turn test). 328 backend tests pass; 528 frontend tests pass; lint and typecheck clean. * 🛡️ refactor: address codex P1+P2 + rename to drop phase-1/2 jargon Codex round 2 review on PR #12957 caught two race conditions and one recovery gap, all triggered by cross-turn filename reuse (`claimCodeFile` intentionally returns the same `file_id` for the same `(filename, conversationId)` across turns). Plus naming cleanup the user requested — internal "phase 1 / phase 2" vocabulary leaks across sprints, replace it everywhere with terms describing what's actually happening. P1 — stale render overwrites newer revision (process.js) Two turns reusing `output.csv` share a `file_id`. If turn-1's background render resolves AFTER turn-2's persist step, the unconditional `updateFile` writes turn-1's stale text/status over turn-2's pending placeholder. Fix: stamp a fresh `previewRevision` UUID on every emit, thread it through `finalizePreview`, and make the commit conditional via a new optional `extraFilter` argument on `updateFile` (`{ previewRevision: <expected> }`). The defensive `updateFile` in `runPreviewFinalize`'s catch uses the same guard so a programming error from an older render also can't override a newer turn. P1 — stale React Query cache on pending remount (queries.ts) Same root cause from the frontend side. Cache key `[QueryKeys.filePreview, file_id]` may hold a prior turn's `'ready'` payload; with `refetchOnMount: false` and the polling gate on `pending`, polling never starts for the new placeholder. Fix: `useAttachmentHandler` invalidates that query whenever an attachment with a `file_id` arrives. Both initial-emit and update events trigger invalidation — uniform gate. P2 — quick-restart orphans skipped by boot sweep (files.js) Boot `sweepOrphanedPreviews` uses a 5-min cutoff for multi-instance safety. A crash + restart inside the cutoff leaves `pending` records that never get touched again. Fix: lazy sweep inside the preview endpoint — if a polled record is `pending` and `updatedAt` is older than 5 min, mark it `failed:orphaned` on the spot before responding. Conditional on the same `updatedAt` we observed so a concurrent legitimate update wins. Cheap, bounded by user activity. Naming cleanup - `runPhase2Finalize` → `runPreviewFinalize` - `PHASE_TWO_TIMEOUT_MS` → `PREVIEW_FINALIZE_TIMEOUT_MS` - All `phase-1` / `phase-2` / `two-phase` prose replaced with "the immediate emit", "the deferred render", "the persist step", "the deferred preview", etc. Skill-feature `phase 1/2` references (different feature) left alone. Tests: 10 new (4 lazy-sweep × preview endpoint, 3 cache-invalidation × useAttachmentHandler, 3 extraFilter × updateFile data-schemas). Backend 332/332, frontend 531/531, data-schemas 37/37, lint clean. * 🛠️ refactor: address comprehensive review (round 3) — stale-cache MAJOR + 3 minors Comprehensive review on PR #12957 caught a P1 follow-on bug from the prior `invalidateQueries` fix, plus 3 maintainability findings. MAJOR: stale React Query cache not actually fixed by `invalidateQueries` The previous fix called `invalidateQueries` to flush stale cached preview data on cross-turn filename reuse. But `useFilePreview` had `refetchOnMount: false`, which made the new observer read the stale-marked 'ready' data without refetching. The polling `refetchInterval` then evaluated against stale 'ready' → returned `false` → polling never started → user stuck on stale content. Fix (belt-and-suspenders): a) `useAttachmentHandler` switched to `removeQueries` — drops the cache entry entirely so the next mount has nothing to read and must fetch. b) `useFilePreview` no longer sets `refetchOnMount: false`, so the React Query default (`true`) kicks in — second line of defense if any future codepath observes stale data before the handler has a chance to evict. MINOR: `finalizePreview` JSDoc missing `previewRevision` param Added with explanation of the conditional update guard. MINOR: asymmetric stream-writable guard between SSE protocols Chat-completions delegated the gate to `writeAttachmentUpdate`; Open Responses inlined `!res.writableEnded && res.headersSent`. Extracted `isStreamWritable(res, streamId)` predicate; both paths + `writeAttachmentUpdate` now share the single source of truth. NIT: `(data as Partial<TFile>).file_id` cast repeated 4 times Extracted to a `fileId` local at the top of the handler. Tests: existing 9 invalidate-tests rewritten as remove-tests; +1 new lock-in test asserts removeQueries is called and invalidateQueries is NOT (regression guard against round-3 finding). 332 backend pass, 532 frontend pass, lint clean. Skipped findings (deferred / acceptable): - MINOR: post-submission pending state has no auto-recovery — the `isAnySubmitting` polling gate was the user's explicit design; LLM context surfaces failed/pending so the model can volunteer. Worth a follow-up if real users hit it. - NIT: double DB query per preview poll — reviewer marked acceptable; changing `fileAccess` middleware is out of scope. * 🛡️ test: address comprehensive review NITs (initial-emit guard + isStreamWritable coverage) NIT — chat-completions initial emit skips writableEnded check The Open Responses initial emit was switched to use the new `isStreamWritable` predicate in the round-3 commit, but the chat-completions initial emit kept the older narrower check (`streamId || res.headersSent`). On a client disconnect mid-stream (`writableEnded === true`) it would still hit `res.write` and raise `ERR_STREAM_WRITE_AFTER_END` — caught by the outer IIFE catch but logged as noise. Switch this site to `isStreamWritable` too so both initial-emit paths share the same gate as the deferred update emits. NIT — `isStreamWritable` not directly unit-tested The predicate was only covered indirectly via the deferred-preview SSE tests (writableEnded skip, headersSent check). Export from `callbacks.js` and add 5 parametric tests pinning down each branch (streamId truthy, res null, !headersSent, writableEnded, happy path) so a future condition addition can't silently regress. * 🐛 fix: stuck "Preparing preview…" + inline the chip subtitle Two related fixes for a stuck-spinner bug a user reported in manual testing of PR #12957. **Stuck spinner (the bug)** The deferred preview render can complete a few seconds AFTER the SSE stream closes (typical case: PPTX render finishes ~3s after the LLM emits FINAL). When that happens, the SSE update is silently dropped (`isStreamWritable` returns false on a closed stream) and polling is the only recovery path. The earlier polling gate was `status === 'pending' && isAnySubmitting`, which mirrored the original design intent ("only query while the LLM is still generating"). But `isAnySubmitting` flips false the moment the model emits FINAL — milliseconds before the deferred render commits. Polling never runs, the chip stays "Preparing preview…" forever even though the DB has `status: 'ready'` with valid HTML. Drop the `isAnySubmitting` part of the gate. `useFilePreview`'s `refetchInterval` is already a function-form that returns `false` on the first terminal response, so polling auto-stops within one tick of resolution. The server-side render ceiling (60s) plus the lazy sweep in the preview endpoint cap the worst case to ~24 polls per pending attachment. Polling itself never blocks UX — the gate's purpose was "don't waste cycles", and capping by terminal status is the correct expression of that. **Inline the chip subtitle (the visual)** The previous design rendered "Preparing preview…" as a loose-feeling spinner+text BELOW the file chip. The chip itself looked done while a floating annotation said it wasn't. `FileContainer` gains an optional `subtitle?: ReactNode` prop that overrides the default file-type label. `Attachment.tsx` passes a `PreviewStatusSubtitle` (spinner + "Preparing preview…" / alert + "Preview unavailable") into that slot when the file's preview is pending or failed. The chip footprint stays identical to its `'ready'` form — just the second row swaps from "PowerPoint Presentation" to the status indicator. No floating element, no layout shift. Tests: regression test pinning down "polling stays enabled after the LLM finishes" so a future revert can't reintroduce the stuck-spinner bug. Existing FileContainer tests pass unchanged (subtitle override is opt-in). 522 frontend tests pass; lint clean. * 🐛 fix: deferred-preview survives reload + matches artifact card chrome Fixes the remaining stuck-pending case after the polling gate fix: on a reloaded conversation, message.attachments come from the DB frozen at the immediate-persist `status: 'pending'`, but `messageAttachmentsMap` is empty because no SSE handler ever fired for that messageId. Polling now INSERTS a new live entry when no record matches the file_id, and `useAttachments` merges live entries onto DB entries by file_id so the resolved text/textFormat reach `artifactTypeForAttachment` and the chip routes through the proper PanelArtifact card. Also replaces the small file chip used during the pending state with a PreviewPlaceholderCard that mirrors ToolArtifactCard chrome, so the transition to the resolved PanelArtifact no longer reshapes the UI. * ✨ feat: auto-open panel when deferred preview resolves pending→ready The legacy auto-open path is gated only on `isSubmitting`, so an office-file preview that resolves *after* the SSE stream closes would render in place but never auto-open the panel — even though that's exactly the moment the result becomes meaningful to the user. Adds a per-file_id one-shot signal that `useAttachmentPreviewSync` flips on the pending→ready edge; `ToolArtifactCard` consumes it on mount and auto-opens regardless of submission state. The signal is *only* set on the actual transition (history loads of pre-resolved files don't trigger it) and is consumed once (panel close + reopen on the same card stays user-controlled). * 🐛 fix: drop placeholder Terminal overlay + scope auto-open to fresh resolutions Two fixes for issues spotted in manual testing of the deferred-preview auto-open feature: 1. PreviewPlaceholderCard was passing `file={attachment}` to FilePreview, which triggered SourceIcon's Terminal overlay (`metadata.fileIdentifier` is set on every code-execution file). The artifact card itself doesn't show that overlay; the placeholder shouldn't either, so the pending→resolved transition is visually seamless. 2. The `previewJustResolved` flag flipped on every pending→ready transition observed by the polling hook — including stale-pending DB records that resolve via the first poll on a *history load*. Conversations whose immediate-persist snapshot left attachments at `status: 'pending'` would yank the panel open every revisit. Adds `mountedDuringStreamRef` to the hook (mirroring ToolArtifactCard) so the flag fires only when the hook itself was mounted during an active turn — preserving the pre-PR contract that the panel only auto-opens for results the user is actively waiting on, never for history. * 🐛 fix: don't downgrade preview to failed when only the SSE emit throws Codex P2 finding on PR #12957: the original chain placed `.catch` after `.then(onResolved)`, so a throw inside `onResolved` (transport-side errors — SSE write race after stream close, an emitter listener throwing) would propagate into the finalize catch and persist `status: 'failed'` / `previewError: 'unexpected'`. That surfaced "preview unavailable" in the UI for a perfectly valid file, and degraded next-turn LLM context to reflect a non-existent failure. Wraps `onResolved` in its own try/catch so emit errors are logged but do not affect the file's persisted status. Extraction success and emit success are now independent: if extraction succeeds and `finalizePreview` writes the terminal status, the polling layer / next page load surfaces the resolved preview even if this turn's SSE emit didn't land. * 🛡️ fix: run boot-time orphan sweep under system tenant context Codex P2 finding on PR #12957: `File` is tenant-isolated, so under `TENANT_ISOLATION_STRICT=true` the boot-time `sweepOrphanedPreviews` threw `[TenantIsolation] Query attempted without tenant context in strict mode` and the recovery path silently failed every restart. Stale `status: 'pending'` records would be stuck until a user happened to poll the preview endpoint and trigger the lazy sweep — which only covers the file the user is currently looking at, not the bulk candidate set the boot sweep is designed to recover. Wraps the sweep in `runAsSystem(...)` in both boot paths (`api/server/index.js` and `api/server/experimental.js`) and pins the contract with regression tests in `file.spec.ts` — one test asserts the bare call throws under strict mode, the other asserts the `runAsSystem`-wrapped call succeeds. * 🧹 chore: trim verbose comments from previous commit * 🧹 chore: address review findings (dead branch, lazy-sweep cutoff, stale JSDoc) - finalizePreview: drop unreachable !isOfficeBucket branch (caller already gates on hasOfficeHtmlPath, so this path is always office) - preview endpoint: drop lazy-sweep cutoff from 5min to 2min — anything past the 60s render ceiling is definitively orphaned, and per-request sweep can be tighter than the per-instance boot sweep - strip stale `isSubmitting` references from JSDoc in 3 spots (the client-side gate was removed in 9a65840) Skipped: function-length (#3) and client-side polling cap (#4) — refactors without correctness/perf wins; remaining NITs. * 🧹 fix: trim 1 query off pending polls + clear stale lifecycle on cross-shape updates - Preview endpoint: reuse fileAccess middleware's record for the lifecycle check; only re-fetch with text on the terminal ready response. Cuts the typical poll lifecycle from 2(N+1) to N+1 queries, since the vast majority of polls hit while pending and don't need text at all. - processCodeOutput non-office branch: explicitly null out status, previewError, previewRevision (codex P2). Without this, an update at the same (filename, conversationId) where the prior emit was an office file leaves stale lifecycle fields and the client renders the wrong state for the now non-office artifact. - Tests: rewire preview.spec mocks for the new shape, add boundary test pinning the 2min cutoff, add regression test for the cross-shape update. * 🐛 fix: keep polling on transient errors but cap permanently-broken endpoint Codex P2: the previous `data?.status === 'pending' ? 2500 : false` gate killed polling on the first transient error. With `retry: false`, a 500 left `data` undefined, the callback returned false, and the chip was stuck "Preparing preview…" forever — exactly the bug the polling layer was supposed to recover from. Inverts the gate: stop on terminal success (`ready`/`failed`) or after 5 consecutive errors. Transient errors keep retrying; a permanently broken endpoint caps at ~12.5s instead of polling forever. Predicate extracted as `previewRefetchInterval` for direct unit testing without fighting React Query's timer machinery. * ✨ feat: render pending-preview files in their own row Pending deferred-preview chips now bucket into a separate row above the resolved attachments — reads as "this is still happening" rather than mixing with completed downloads. Once status flips to ready, the chip re-buckets into panelArtifacts; failed re-buckets into the file row alongside other downloads. * 🎨 fix: render pending-preview chips in the panel-artifact row, not the file row Previous bucketing put pending chips in the file row (since `artifactTypeForAttachment` returns null for empty-text records). The pending placeholder is a future panel artifact — sharing the row keeps the chip in place when it resolves instead of jumping rows. Plain files still get their own row. * 🐛 fix: phase-1 SSE replay must not regress a resolved attachment Codex P1: useEventHandlers.finalHandler iterates responseMessage.attachments at stream end and dispatches each through the attachment handler. Those records are the immediate-persist snapshot (status:pending, text:null) — if a deferred update has already moved the same file_id to ready/failed, the existing merge let the pending fields win and downgraded the resolved record. Result: chip flickers back to pending and polling restarts until the lazy sweep corrects. Pin the terminal lifecycle fields (status, text, textFormat, previewError) when existing is ready/failed and incoming is pending. Other field updates still go through. * 🐛 fix: track preview-poll error cap outside React Query state Codex P2: the previous cap relied on `query.state.fetchFailureCount`, but React Query v4's reducer resets that to 0 on every fetch dispatch (the `'fetch'` action). With `retry: false`, each failed poll left count at 1 and the next dispatch reset it back to 0, so the `>= 5` branch never fired and a permanently-broken endpoint polled forever. Track consecutive errors in a module-level Map keyed by file_id, incremented in a thin `fetchFilePreview` wrapper around the data service call. The Map is cleared on success and on cap-stop, so memory is bounded by in-flight pending file_ids per session.	2026-05-06 03:04:19 -04:00
Atef Bellaaj	187ab787da	🌩️ feat: CloudFront CDN File Strategy (#12193) ... * 🌩️ feat: CloudFront CDN File Strategy + signed cookies Squashed from PR #12193: - feat(storage): add CloudFront CDN file strategy - feat(auth): add CloudFront signed cookie support Note: package.json/package-lock.json dependency additions are intentionally omitted from this commit and will be re-added via `npm install` after rebase to avoid lock-file merge conflicts. The two new peer deps that need to be re-installed are: - @aws-sdk/client-cloudfront@^3.1032.0 - @aws-sdk/cloudfront-signer@^3.1012.0 Also fixes 4 missing destructured names in AuthService.spec.js (getUserById, generateToken, generateRefreshToken, createSession) that were referenced in tests but not imported from the mocked '~/models'. * 📦 chore: install CloudFront SDK deps for PR #12193 Adds the two AWS CloudFront packages required by the rebased CloudFront CDN strategy: - @aws-sdk/client-cloudfront - @aws-sdk/cloudfront-signer Following the @aws-sdk/client-s3 pattern: - api/package.json: regular dependency (runtime resolution) - packages/api/package.json: peerDependency Generated by `npm install` against the freshly rebased lock file to avoid the merge conflicts that came from the original PR's lock-file edits being made against an older base of dev. * 🐛 fix: CI failures + review findings on CloudFront PR #12193 CI fixes - Rename packages/data-provider/src/__tests__/cloudfront-config.test.ts → src/cloudfront-config.spec.ts. Jest's default testMatch picks up __tests__/ directories even inside dist/, so the compiled .d.ts shell was being executed as an empty test suite. Moving to .spec.ts (matching the rest of the package) avoids the dist/ pickup. - Add cookieExpiry: 1800 to CloudFront crud.test makeConfig: the schema applies a default so CloudFrontFullConfig requires it. Review findings addressed - #1 (Codex + comprehensive): Normalize CloudFront domain with /\/+$/ regex (and key with /^\/+/ regex) in buildCloudFrontUrl, matching the cookie code so resource policy and file URLs stay aligned even when the configured domain has multiple trailing slashes. Added tests. - #2: Move DEFAULT_BASE_PATH out of s3Config into shared packages/api/src/storage/constants.ts. ImageService no longer imports S3-specific config. - #3: getCloudFrontConfig() returns Readonly<CloudFrontFullConfig> | null to discourage mutation of the cached signing config. - #4: Add cross-field refinement tests for cloudfrontConfigSchema (invalidateOnDelete-without-distributionId, imageSigning="cookies"-without-cookieDomain). - #6: Revert unrelated MCP comment re-indentation in librechat.example.yaml. - #7: Add azure_blob to the strategy list comment. Skipped - #5 (extractKeyFromS3Url with CloudFront URLs): existing deleteFileFromCloudFront tests already cover the path-equivalence assumption; renaming the helper is real refactor work beyond this PR's scope. - #8, #9 (NIT, low confidence): leaving for author judgement. * 🧹 chore: drop dead DEFAULT_BASE_PATH from s3Config test mock After moving DEFAULT_BASE_PATH to ~/storage/constants, crud.ts no longer reads it from s3Config — so the entry in the s3Config jest mock was misleading dead config. The tests still pass because the unmocked real constants module provides the value. --------- Co-authored-by: Danny Avila <danny@librechat.ai>	2026-05-05 13:21:05 -04:00