From a9d99b3771ec26c43915be0644cd08ed2a9993c3 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 06:26:17 -0400 Subject: [PATCH 01/14] =?UTF-8?q?=F0=9F=93=8C=20fix:=20Keep=20the=20Settle?= =?UTF-8?q?d=20Turn=20Mounted=20Through=20Final=20Content=20Compaction=20(?= =?UTF-8?q?#15186)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * πŸ“Œ fix: Keep the Settled Turn Mounted Through Final Content Compaction The agent aggregator writes content parts at provider-source indexes, so the streamed array is sparse wherever a step produced nothing; the final SSE event carries the persisted, compacted array. Adopting it verbatim shifted every part after a hole, re-keying every index-derived React identity: the settled message remounted wholesale, activity-phase groups replayed their fold-in entrance, code panes re-highlighted, and the thread visibly snapped up and down at the end of every tool-calling run. finalHandler now pairs the compacted parts with their streamed counterparts in order and stamps each with the index it streamed at (`streamedIndex`, client-only); render keys read the stamp while all coordinate logic (edit indexes, phase bounds, cursor) stays on the live compacted positions the server persisted. Phase-segment keys also anchor to their first defined part instead of the segment ordinal, since phantom hole-only segments vanish at compaction and shifted every segment after them. * πŸ” fix: Carry Identity Stamps Through Re-Delivered Finals and Parallel Attribution Codex round 1, both real: - P1: a later final event can re-deliver an already-settled message as a fresh compact array (Assistants runMessages resync); index-aligned pairing returned it unstamped, wiping the previous settle's stamps and re-keying the older turn all over again. The pairing now carries the matched current part's stamp forward, so a settled turn keeps its keys through every subsequent final. - P2: ParallelContentRenderer's sequential stretches invoked renderResumeAttribution with only the live index, so steer attribution nodes in parallel content still re-keyed at the swap. The stable key index now threads through both call sites; getPartKeyIndex moves to utils/messages beside the stamp writer it reads. * 🧿 fix: Require Content Agreement Before Pairing Streamed Identity Codex round 2 (P2, real): hide_sequential_outputs runs omit intermediate parts from the final array, so a type-only match could hand the retained output an omitted intermediate's identity β€” transferring its key and any UI state. Non-tool pairing now requires content agreement: mutual-prefix text for TEXT/THINK/ACTIVITY_LABEL (one side extending the other is the same part observed at two moments), the Open Responses phase for TEXT, and the label kind for activity labels β€” a blank reservation still pairs with its filled label. Ambiguous shapes fall back to the pre-stamp full re-key, which is honest for a final that visibly removes parts. * πŸͺ’ fix: Refuse Stamping When the Server Removed Content; Strip Stamps on Edited Reruns Codex round 3, two of three real: - Prefix agreement alone still mis-paired when an omitted intermediate happened to prefix the retained output. Pairing now also requires that no substantial streamed part is left over: leftovers mean the server removed content (hide_sequential_outputs), so every in-order pairing is suspect and the message re-keys plainly instead. - An edited resubmission clones the settled (stamped) prefix and appends the rerun's parts at the prefix length; a retained stamp at or above that length collides with an appended part's key. The clone now strips the client-only stamps, reverting the retained prefix to physical identity for the rerun. The third finding (content-segment keys under late-phase recovery) is declined with rationale on the PR: user expansion overrides survive via the message-wide expansion map with stable group ids, recovery is a genuine restructure at the moment a phase materializes, and first-child anchoring is the only choice stable under the two high-frequency events (streaming appends and final compaction). --- .../Chat/Messages/Content/ContentParts.tsx | 84 ++++-- .../Chat/Messages/Content/ParallelContent.tsx | 8 +- .../Content/__tests__/ContentParts.test.tsx | 68 +++++ client/src/hooks/Chat/useChatFunctions.ts | 6 +- client/src/hooks/SSE/useEventHandlers.ts | 29 ++- client/src/utils/messages.spec.ts | 242 ++++++++++++++++++ client/src/utils/messages.ts | 199 ++++++++++++++ .../data-provider/src/types/assistants.ts | 12 +- 8 files changed, 605 insertions(+), 43 deletions(-) create mode 100644 client/src/utils/messages.spec.ts diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 2324486d00e..2133d60a7ce 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -8,7 +8,12 @@ import type { } from 'librechat-data-provider'; import type { ReactNode, ReactElement } from 'react'; import type { ToolCallGroupExpansionState } from './ToolCallGroup'; -import { mapAttachments, filterAttachmentsForPart, groupSequentialToolCalls } from '~/utils'; +import { + mapAttachments, + getPartKeyIndex, + filterAttachmentsForPart, + groupSequentialToolCalls, +} from '~/utils'; import WorkspaceChanges, { partitionWorkspaceChanges } from './Parts/WorkspaceChanges'; import { groupActivityPhases, lastCursorContentIdx } from '~/utils/activityLabels'; import { ParallelContentRenderer, type PartWithIndex } from './ParallelContent'; @@ -51,20 +56,20 @@ const getToolGroupId = (parts: PartWithIndex[], fallbackScope: number): string = * absorbs the block's leading THINK part when its text lands, so keying on * `parts[0]` would flip the key mid-run β€” remounting the group and losing * whatever the user had expanded. The tool calls themselves do not move. */ - let firstToolIdx: number | undefined; + let firstToolKeyIdx: number | undefined; for (const { part, idx } of parts) { const toolCallId = getToolCallId(part); if (toolCallId) { return `tool:${toolCallId}`; } - if (firstToolIdx === undefined && part?.type === ContentTypes.TOOL_CALL) { - firstToolIdx = idx; + if (firstToolKeyIdx === undefined && part?.type === ContentTypes.TOOL_CALL) { + firstToolKeyIdx = getPartKeyIndex(part, idx); } } /** Same reasoning for id-less tool calls: anchor to the first TOOL entry's * index rather than the block's first part, which shifts when reasoning is * absorbed. Only a block with no tool call at all falls back to `parts[0]`. */ - return `fallback:${fallbackScope}:${firstToolIdx ?? firstPart.idx}`; + return `fallback:${fallbackScope}:${firstToolKeyIdx ?? getPartKeyIndex(firstPart.part, firstPart.idx)}`; }; type PartWithContextProps = { @@ -117,7 +122,7 @@ const PartWithContext = memo(function PartWithContext({ part={part} attachments={partAttachments} isSubmitting={isSubmitting} - key={`part-${messageId}-${idx}`} + key={`part-${messageId}-${getPartKeyIndex(part, idx)}`} isCreatedByUser={isCreatedByUser} isLast={isLastPart} showCursor={isLastPart && isLast} @@ -248,7 +253,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ const indices = new Set(); for (const segment of phaseSegments ?? []) { if (segment.type === 'phase') { - indices.add(segment.labelIndex); + indices.add(getPartKeyIndex(segment.labelPart, segment.labelIndex)); } } return indices; @@ -332,7 +337,7 @@ const ContentPartsBody = memo(function ContentPartsBody({ const localIdx = localIndexByAbsolute?.get(idx) ?? idx - contentIndexOffset; return ( { + (idx: number, keyIdx: number = idx): ReactElement | null => { if (authorHeader == null || !postSteerAuthors.has(idx)) { return null; } const activeAgentId = postSteerAuthors.get(idx); if (activeAgentId != null) { - return ; + return ; } - return {authorHeader}; + return {authorHeader}; }, [authorHeader, postSteerAuthors, messageId], ); @@ -500,6 +505,23 @@ const ContentPartsBody = memo(function ContentPartsBody({ const relativeGlobalLastContentIdx = lastCursorContentIdx(content ?? []); const globalLastContentIdx = relativeGlobalLastContentIdx < 0 ? -1 : absoluteIndexAt(relativeGlobalLastContentIdx); + /** Segment keys anchor to their first defined part's stable index, never + * to the segment's ordinal: hole-only slots form phantom segments while + * a run streams and vanish from the compacted final content, so ordinal + * keys shift at settle and remount every segment body after them. */ + const segmentKeyIndex = (segment: { + content: Array; + contentIndices: number[]; + startIndex: number; + }): number => { + for (let i = 0; i < segment.content.length; i++) { + const part = segment.content[i]; + if (part != null) { + return getPartKeyIndex(part, absoluteIndexAt(segment.contentIndices[i])); + } + } + return absoluteIndexAt(segment.startIndex); + }; const renderSegment = ( segmentContent: Array, segmentStartIndex: number, @@ -538,17 +560,26 @@ const ContentPartsBody = memo(function ContentPartsBody({ )} {renderPendingSkills()} - {phaseSegments.map((segment, index) => - segment.type === 'phase' ? ( + {phaseSegments.map((segment) => { + if (segment.type !== 'phase') { + return renderSegment( + segment.content, + absoluteIndexAt(segment.startIndex), + segment.contentIndices.map(absoluteIndexAt), + `phase-adjacent-${segmentKeyIndex(segment)}`, + ); + } + const phaseKeyIndex = getPartKeyIndex(segment.labelPart, segment.labelIndex); + return ( part != null && hasPendingApprovalInPart(part), )} animateEntrance={ - previousPhaseIndices != null && !previousPhaseIndices.has(segment.labelIndex) + previousPhaseIndices != null && !previousPhaseIndices.has(phaseKeyIndex) } showCursor={ isLast && @@ -560,18 +591,11 @@ const ContentPartsBody = memo(function ContentPartsBody({ segment.content, absoluteIndexAt(segment.startIndex), segment.contentIndices.map(absoluteIndexAt), - `phase-content-${index}`, + `phase-content-${phaseKeyIndex}`, )} - ) : ( - renderSegment( - segment.content, - absoluteIndexAt(segment.startIndex), - segment.contentIndices.map(absoluteIndexAt), - `phase-adjacent-${index}`, - ) - ), - )} + ); + })} @@ -639,9 +663,13 @@ const ContentPartsBody = memo(function ContentPartsBody({ )} {!showEmptyCursor && groupedParts.flatMap((group) => { - const firstIdx = group.type === 'single' ? group.part.idx : (group.parts[0]?.idx ?? -1); + const first = group.type === 'single' ? group.part : group.parts[0]; + const firstIdx = first?.idx ?? -1; const nodes: ReactElement[] = []; - const attribution = renderResumeAttribution(firstIdx); + const attribution = renderResumeAttribution( + firstIdx, + first ? getPartKeyIndex(first.part, first.idx) : firstIdx, + ); if (attribution != null) { nodes.push(attribution); } diff --git a/client/src/components/Chat/Messages/Content/ParallelContent.tsx b/client/src/components/Chat/Messages/Content/ParallelContent.tsx index 6348e258704..27b961a3dfd 100644 --- a/client/src/components/Chat/Messages/Content/ParallelContent.tsx +++ b/client/src/components/Chat/Messages/Content/ParallelContent.tsx @@ -8,11 +8,11 @@ import { } from '~/utils/activityLabels'; import MemoryArtifacts from './MemoryArtifacts'; import Sources from '~/components/Web/Sources'; +import { cn, getPartKeyIndex } from '~/utils'; import { SearchContext } from '~/Providers'; import SiblingHeader from './SiblingHeader'; import { EmptyText } from './Parts'; import Container from './Container'; -import { cn } from '~/utils'; export type PartWithIndex = { part: TMessageContentParts; idx: number }; @@ -235,7 +235,7 @@ type ParallelContentRendererProps = { * sequential before/after stretches consult it: column content already * carries per-agent identity. */ - renderResumeAttribution?: (idx: number) => React.ReactNode; + renderResumeAttribution?: (idx: number, keyIdx?: number) => React.ReactNode; showDecorations?: boolean; /** Absolute transcript index represented by `content[0]` in a phase slice. */ contentIndexOffset?: number; @@ -302,7 +302,7 @@ export const ParallelContentRenderer = memo(function ParallelContentRenderer({ {/* Sequential content BEFORE parallel sections */} {before.flatMap(({ part, idx }) => { - const attribution = renderResumeAttribution?.(idx); + const attribution = renderResumeAttribution?.(idx, getPartKeyIndex(part, idx)); const rendered = renderPart(part, idx, false); return attribution != null ? [attribution, rendered] : [rendered]; })} @@ -324,7 +324,7 @@ export const ParallelContentRenderer = memo(function ParallelContentRenderer({ {/* Sequential content AFTER parallel sections */} {after.flatMap(({ part, idx }) => { - const attribution = renderResumeAttribution?.(idx); + const attribution = renderResumeAttribution?.(idx, getPartKeyIndex(part, idx)); const rendered = renderPart(part, idx, idx === lastContentIdx); return attribution != null ? [attribution, rendered] : [rendered]; })} diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx index 33191b8ff99..0958825d8e0 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { ContentTypes, Tools } from 'librechat-data-provider'; import { fireEvent, render, screen } from '@testing-library/react'; import type { TMessageContentParts, TAttachment } from 'librechat-data-provider'; +import { preserveStreamedContentIdentity } from '~/utils/messages'; import { groupSequentialToolCalls } from '~/utils'; jest.mock('~/utils', () => ({ @@ -10,6 +11,7 @@ jest.mock('~/utils', () => ({ filterAttachmentsForPart: (attachments: unknown) => attachments, groupSequentialToolCalls: jest.fn(), hasPendingApprovalInPart: jest.requireActual('~/utils/groupToolCalls').hasPendingApprovalInPart, + getPartKeyIndex: jest.requireActual('~/utils/messages').getPartKeyIndex, })); jest.mock('~/Providers', () => { @@ -651,3 +653,69 @@ describe('ContentParts β€” activity phase state', () => { ); }); }); + +describe('ContentParts β€” settled content identity across compaction', () => { + /** Mirrors a captured run: the aggregator leaves holes at the source indexes + * of steps that produced nothing, and `finalHandler` swaps in the server's + * compacted array. Without the streamed-index stamp every index-derived key + * shifts and the settled message remounts wholesale. */ + const toolPart = { + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { id: 'call_a', name: 'search', args: {}, output: 'one' }, + } as unknown as TMessageContentParts; + const batchLabel = { + type: ContentTypes.ACTIVITY_LABEL, + [ContentTypes.ACTIVITY_LABEL]: 'Recorded the fact', + tool_call_ids: ['call_a'], + } as unknown as TMessageContentParts; + const answer = { type: ContentTypes.TEXT, text: 'done' } as unknown as TMessageContentParts; + const phaseLabel = (bounds: { start: number; end: number }) => + ({ + type: ContentTypes.ACTIVITY_LABEL, + [ContentTypes.ACTIVITY_LABEL]: 'Researched the question', + activity_label_type: 'phase', + activity_start_index: bounds.start, + activity_end_index: bounds.end, + activity_count: 1, + pending: false, + }) as unknown as TMessageContentParts; + + const streamed: Array = [ + undefined, + toolPart, + batchLabel, + undefined, + answer, + phaseLabel({ start: 1, end: 4 }), + ]; + const compacted = [toolPart, batchLabel, answer, phaseLabel({ start: 0, end: 2 })]; + + const renderStreaming = () => + render(); + + it('keeps every part and the phase group mounted when the final content is stamped', () => { + const { rerender } = renderStreaming(); + const phaseNode = screen.getByTestId('activity-phase-group'); + const toolNode = screen.getByTestId('real-part-tool_call'); + const textNode = screen.getByTestId('real-part-text'); + + const finalContent = preserveStreamedContentIdentity(streamed, compacted); + rerender(); + + expect(screen.getByTestId('activity-phase-group')).toBe(phaseNode); + expect(screen.getByTestId('real-part-tool_call')).toBe(toolNode); + expect(screen.getByTestId('real-part-text')).toBe(textNode); + expect(phaseNode).toHaveAttribute('data-animate-entrance', 'false'); + }); + + it('remounts and replays the phase entrance without the stamp (regression control)', () => { + const { rerender } = renderStreaming(); + const phaseNode = screen.getByTestId('activity-phase-group'); + + rerender(); + + const settledPhase = screen.getByTestId('activity-phase-group'); + expect(settledPhase).not.toBe(phaseNode); + expect(settledPhase).toHaveAttribute('data-animate-entrance', 'true'); + }); +}); diff --git a/client/src/hooks/Chat/useChatFunctions.ts b/client/src/hooks/Chat/useChatFunctions.ts index 195d23a5576..cb2bc929ad8 100644 --- a/client/src/hooks/Chat/useChatFunctions.ts +++ b/client/src/hooks/Chat/useChatFunctions.ts @@ -33,6 +33,7 @@ import { isSubmittableMessage, createDualMessageContent, getRouteChatProjectId, + stripStreamedIndexStamps, } from '~/utils'; import useFocusRegeneratedResponse from '~/hooks/Chat/useFocusRegeneratedResponse'; import useSetFilesToDelete from '~/hooks/Files/useSetFilesToDelete'; @@ -626,7 +627,10 @@ export default function useChatFunctions({ initialResponse.text = ''; if (editedContent && latestMessage?.content) { - initialResponse.content = cloneDeep(latestMessage.content); + /** Stamps off: the rerun appends provider parts at the prefix LENGTH, + * and a retained `streamedIndex` at or above it would collide with an + * appended part's render key (see `stripStreamedIndexStamps`). */ + initialResponse.content = stripStreamedIndexStamps(cloneDeep(latestMessage.content)); /** Captured now, while it is still the retained prefix: a later resume * sync replaces this array with the server's completion-local * snapshot, after which its length no longer describes the offset. */ diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index 1550c24e06f..de4b0d1d13d 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -36,6 +36,7 @@ import { updateConvoInAllQueries, removeConvoFromAllQueries, findConversationInInfinite, + preserveStreamedContentIdentity, } from '~/utils'; import { startupConfigKey, @@ -870,19 +871,29 @@ export default function useEventHandlers({ finalMessages = [...messages, requestMessage, responseMessage]; } - /* Preserve files from current messages when server response lacks them */ + /* Preserve files and streamed content identity from current messages: + * files fill in when the server response lacks them, and the persisted + * (compacted) content is stamped with the indexes it streamed at so + * index-keyed renders don't remount the settled message. */ if (finalMessages.length > 0) { - const currentMsgMap = new Map( - currentMessages - .filter((m) => m.files && m.files.length > 0) - .map((m) => [m.messageId, m.files]), - ); + const currentMsgMap = new Map(currentMessages.map((m) => [m.messageId, m])); for (let i = 0; i < finalMessages.length; i++) { const msg = finalMessages[i]; - const preservedFiles = currentMsgMap.get(msg.messageId); - if (msg.files == null && preservedFiles) { - finalMessages[i] = { ...msg, files: preservedFiles }; + const currentMsg = currentMsgMap.get(msg.messageId); + if (!currentMsg) { + continue; + } + const preservedFiles = + msg.files == null && currentMsg.files?.length ? currentMsg.files : undefined; + const content = preserveStreamedContentIdentity(currentMsg.content, msg.content); + if (preservedFiles == null && content === msg.content) { + continue; } + finalMessages[i] = { + ...msg, + ...(preservedFiles != null ? { files: preservedFiles } : {}), + ...(content !== msg.content ? { content } : {}), + }; } } diff --git a/client/src/utils/messages.spec.ts b/client/src/utils/messages.spec.ts new file mode 100644 index 00000000000..b06f3e5a485 --- /dev/null +++ b/client/src/utils/messages.spec.ts @@ -0,0 +1,242 @@ +import { ContentTypes } from 'librechat-data-provider'; +import type { TMessage, TMessageContentParts } from 'librechat-data-provider'; +import { preserveStreamedContentIdentity, stripStreamedIndexStamps } from './messages'; + +const text = (value: string, extra: Record = {}): TMessageContentParts => + ({ type: ContentTypes.TEXT, text: value, ...extra }) as TMessageContentParts; + +const think = (value: string): TMessageContentParts => + ({ type: ContentTypes.THINK, think: value }) as TMessageContentParts; + +const tool = (id: string | undefined, name = 'search'): TMessageContentParts => + ({ + type: ContentTypes.TOOL_CALL, + tool_call: { id, name, args: '' }, + }) as TMessageContentParts; + +const label = (value: string, extra: Record = {}): TMessageContentParts => + ({ type: ContentTypes.ACTIVITY_LABEL, activity_label: value, ...extra }) as TMessageContentParts; + +const streamedIndexes = (content: TMessage['content']): Array => + (content ?? []).map((part) => part?.streamedIndex); + +describe('preserveStreamedContentIdentity', () => { + it('stamps every part shifted by compacted holes with its streamed index', () => { + const streamed = [ + undefined, + tool('call_a'), + label('first'), + undefined, + tool('call_b'), + label('second'), + text('answer'), + label('phase', { activity_label_type: 'phase' }), + ]; + const final = [ + tool('call_a'), + label('first'), + tool('call_b'), + label('second'), + text('answer'), + label('phase', { activity_label_type: 'phase' }), + ]; + + const result = preserveStreamedContentIdentity(streamed, final); + + expect(streamedIndexes(result)).toEqual([1, 2, 4, 5, 6, 7]); + expect(final.every((part) => part.streamedIndex === undefined)).toBe(true); + }); + + it('returns the final array untouched when no hole shifted anything', () => { + const streamed = [tool('call_a'), text('answer')]; + const final = [tool('call_a'), text('answer')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('leaves aligned prefix parts unstamped while stamping the shifted tail', () => { + const streamed = [text('intro'), undefined, tool('call_a')]; + const final = [text('intro'), tool('call_a')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([ + undefined, + 2, + ]); + }); + + it('skips streamed empty-text placeholders the compaction dropped', () => { + const streamed = [text(''), tool('call_a'), text('answer')]; + const final = [tool('call_a'), text('answer')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([1, 2]); + }); + + it('skips streamed empty think parts and typeless placeholders', () => { + const streamed = [ + { type: '' } as unknown as TMessageContentParts, + think(''), + think('reasoned'), + text('answer'), + ]; + const final = [think('reasoned'), text('answer')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([2, 3]); + }); + + it('matches by identity, not equality: richer final text keeps its streamed slot', () => { + const streamed = [undefined, text('partial ans')]; + const final = [text('partial answer, completed.')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([1]); + }); + + it('pairs tool calls by id and abandons stamping on an id mismatch', () => { + const streamed = [undefined, tool('call_a')]; + const final = [tool('call_other')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('abandons stamping when the server appended a part that never streamed', () => { + const streamed = [undefined, tool('call_a')]; + const final = [tool('call_a'), text('server-added')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('abandons stamping on a type mismatch instead of mispairing', () => { + const streamed = [think('reasoned'), text('answer')]; + const final = [text('answer')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('returns final content untouched when nothing streamed', () => { + const final = [text('answer')]; + + expect(preserveStreamedContentIdentity(undefined, final)).toBe(final); + expect(preserveStreamedContentIdentity([], final)).toBe(final); + }); + + it('abandons stamping when a filtered run retains only a same-type later part', () => { + const streamed = [text('intermediate agent output'), text('final agent answer')]; + const final = [text('final agent answer')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('abandons stamping when an omitted intermediate is a prefix of the retained output', () => { + const streamed = [text('Answer:'), text('Answer: final details')]; + const final = [text('Answer: final details')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('ignores trailing holes and empty slots when checking for removed content', () => { + const streamed = [undefined, tool('call_a'), text('answer'), text(''), undefined]; + const final = [tool('call_a'), text('answer')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([1, 2]); + }); + + it('abandons stamping when streamed and final text diverge', () => { + const streamed = [undefined, text('answer A')]; + const final = [text('answer B')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('never pairs a batch label with a phase label of the same text', () => { + const streamed = [undefined, label('Ran the tools')]; + const final = [label('Ran the tools', { activity_label_type: 'phase' })]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('pairs a blank label reservation with its filled final label', () => { + const streamed = [undefined, label('')]; + const final = [label('Recorded the fact')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([1]); + }); + + it('never pairs text parts across different phases', () => { + const streamed = [undefined, text('note', { phase: 'commentary' })]; + const final = [text('note')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); + + it('carries existing stamps forward when a settled message is re-delivered compact', () => { + const streamedSparse = [undefined, tool('call_a'), label('first'), undefined, text('answer')]; + const settled = preserveStreamedContentIdentity(streamedSparse, [ + tool('call_a'), + label('first'), + text('answer'), + ]); + expect(streamedIndexes(settled)).toEqual([1, 2, 4]); + + const redelivered = [tool('call_a'), label('first'), text('answer')]; + const result = preserveStreamedContentIdentity(settled, redelivered); + + expect(streamedIndexes(result)).toEqual([1, 2, 4]); + }); + + it('carries a partially stamped message forward without stamping its aligned prefix', () => { + const streamedSparse = [text('intro'), undefined, tool('call_a')]; + const settled = preserveStreamedContentIdentity(streamedSparse, [ + text('intro'), + tool('call_a'), + ]); + expect(streamedIndexes(settled)).toEqual([undefined, 2]); + + const result = preserveStreamedContentIdentity(settled, [text('intro'), tool('call_a')]); + + expect(streamedIndexes(result)).toEqual([undefined, 2]); + }); + + it('pairs id-less tool calls by name', () => { + const streamed = [undefined, tool(undefined, 'execute_code')]; + const final = [tool(undefined, 'execute_code')]; + + expect(streamedIndexes(preserveStreamedContentIdentity(streamed, final))).toEqual([1]); + }); + + it('abandons stamping when id-less tool call names differ', () => { + const streamed = [undefined, tool(undefined, 'execute_code')]; + const final = [tool(undefined, 'web_search')]; + + expect(preserveStreamedContentIdentity(streamed, final)).toBe(final); + }); +}); + +describe('stripStreamedIndexStamps', () => { + const tool = (id: string): TMessageContentParts => + ({ + type: ContentTypes.TOOL_CALL, + tool_call: { id, name: 'search', args: '' }, + }) as TMessageContentParts; + + it('drops every stamp from a settled content array', () => { + const settled = preserveStreamedContentIdentity( + [ + undefined, + tool('call_a'), + { type: ContentTypes.TEXT, text: 'answer' } as TMessageContentParts, + ], + [tool('call_a'), { type: ContentTypes.TEXT, text: 'answer' } as TMessageContentParts], + ); + expect((settled ?? []).some((part) => part?.streamedIndex !== undefined)).toBe(true); + + const stripped = stripStreamedIndexStamps(settled); + + expect((stripped ?? []).every((part) => part?.streamedIndex === undefined)).toBe(true); + }); + + it('returns the same reference when nothing is stamped', () => { + const plain = [tool('call_a')]; + + expect(stripStreamedIndexStamps(plain)).toBe(plain); + expect(stripStreamedIndexStamps(undefined)).toBeUndefined(); + }); +}); diff --git a/client/src/utils/messages.ts b/client/src/utils/messages.ts index aaa4444b185..e8afaac986e 100644 --- a/client/src/utils/messages.ts +++ b/client/src/utils/messages.ts @@ -9,6 +9,7 @@ import { encodeEphemeralAgentId, } from 'librechat-data-provider'; import type { + Agents, TMessage, TConversation, TEndpointsConfig, @@ -192,6 +193,204 @@ export const getAllContentText = (message?: TMessage | null): string => { return ''; }; +const getPartTextValue = (value?: string | { value?: string }): string => + (typeof value === 'string' ? value : value?.value) ?? ''; + +const getPartToolCall = (part: TMessageContentParts): Agents.ToolCall | undefined => + part.type === ContentTypes.TOOL_CALL + ? (part[ContentTypes.TOOL_CALL] as Agents.ToolCall | undefined) + : undefined; + +/** Slots the persistence compaction leaves nothing behind for: the + * dual-message `type: ''` placeholders, text/think parts that never received a + * delta, and tool calls missing their `tool_call` payload. */ +const isEmptyContentPart = (part: TMessageContentParts): boolean => { + if (!part.type) { + return true; + } + if (part.type === ContentTypes.TEXT) { + return getPartTextValue(part.text).length === 0; + } + if (part.type === ContentTypes.THINK) { + return getPartTextValue(part.think).length === 0; + } + if (part.type === ContentTypes.TOOL_CALL) { + return getPartToolCall(part) == null; + } + return false; +}; + +/** One side extending the other is the same part observed at two moments β€” + * a flushed tail or a server-side trim β€” while divergent content is a + * different part that merely shares the type. */ +const isMutualPrefix = (streamed: string, final: string): boolean => + final.startsWith(streamed) || streamed.startsWith(final); + +/** Identity match, not equality: the persisted part may carry richer content + * (flushed text, tool output) than its streamed counterpart, and updating a + * kept identity in place is exactly the point. Content still has to agree as + * an extension of what streamed: a filtered run (`hide_sequential_outputs`) + * omits intermediate parts from the final array, and a type-only match would + * hand the retained output an omitted intermediate's identity. */ +const isSameStreamedPart = ( + streamed: TMessageContentParts, + final: TMessageContentParts, +): boolean => { + if (streamed.type !== final.type) { + return false; + } + if (streamed.type === ContentTypes.TOOL_CALL) { + const streamedCall = getPartToolCall(streamed); + const finalCall = getPartToolCall(final); + if (streamedCall?.id != null && finalCall?.id != null) { + return streamedCall.id === finalCall.id; + } + if (streamedCall?.name != null && finalCall?.name != null) { + return streamedCall.name === finalCall.name; + } + return true; + } + if (streamed.type === ContentTypes.TEXT && final.type === ContentTypes.TEXT) { + if ((streamed.phase ?? null) !== (final.phase ?? null)) { + return false; + } + return isMutualPrefix(getPartTextValue(streamed.text), getPartTextValue(final.text)); + } + if (streamed.type === ContentTypes.THINK && final.type === ContentTypes.THINK) { + return isMutualPrefix(getPartTextValue(streamed.think), getPartTextValue(final.think)); + } + if (streamed.type === ContentTypes.ACTIVITY_LABEL && final.type === ContentTypes.ACTIVITY_LABEL) { + if ((streamed.activity_label_type ?? null) !== (final.activity_label_type ?? null)) { + return false; + } + return isMutualPrefix( + getPartTextValue(streamed.activity_label), + getPartTextValue(final.activity_label), + ); + } + return true; +}; + +/** + * Stamps each part of a final (persisted, compacted) content array with the + * index it occupied while it streamed, pairing the two arrays in order. + * + * The aggregator writes parts at provider-source indexes, so the streamed + * array is sparse wherever a step produced nothing; persistence compacts the + * holes away and every later part shifts down. Adopting the compacted array + * verbatim re-keys every index-derived React identity at the final event β€” + * the settled message remounts wholesale, entrance animations replay, and the + * thread visibly jumps. The stamp (`streamedIndex`) lets renderers keep the + * streamed key while all coordinate logic uses the compacted positions the + * server persisted. + * + * Pairing is all-or-nothing: a partially stamped array could collide a + * streamed key with a compacted fallback key. When any final part has no + * streamed counterpart (server-enriched content), or any substantial streamed + * part has no final counterpart (a filtered run that dropped intermediate + * outputs β€” where in-order pairing could hand a retained part an omitted + * part's identity), the final array is returned untouched and the message + * re-keys as before. + */ +export const preserveStreamedContentIdentity = ( + streamedContent: Array | undefined, + finalContent: TMessage['content'], +): TMessage['content'] => { + if (!streamedContent?.length || !finalContent?.length) { + return finalContent; + } + + let cursor = 0; + let stamped: TMessageContentParts[] | null = null; + for (let index = 0; index < finalContent.length; index++) { + const finalPart = finalContent[index] as TMessageContentParts | undefined; + if (finalPart == null) { + return finalContent; + } + let matchedIndex = -1; + let matchedPart: TMessageContentParts | null = null; + while (cursor < streamedContent.length) { + const streamedPart = streamedContent[cursor]; + if (streamedPart == null) { + cursor += 1; + continue; + } + /** An empty streamed slot facing a filled final part was dropped by the + * compaction β€” never let it steal the match from the filled streamed + * part behind it (an empty THINK ahead of the real one, say). */ + if (isEmptyContentPart(streamedPart) && !isEmptyContentPart(finalPart)) { + cursor += 1; + continue; + } + if (isSameStreamedPart(streamedPart, finalPart)) { + matchedIndex = cursor; + matchedPart = streamedPart; + cursor += 1; + } + break; + } + if (matchedIndex === -1 || matchedPart == null) { + return finalContent; + } + /** A settled message can be re-delivered by a LATER final event (e.g. an + * Assistants run resyncing prior turns): both sides arrive compact, but + * the current parts already carry stamps from their own settle. Carrying + * them forward keeps their keys stable forever, instead of silently + * reverting the identity this stamp exists to preserve. */ + const stampIndex = matchedPart.streamedIndex ?? matchedIndex; + if (stampIndex !== index && stamped == null) { + stamped = [...finalContent]; + } + if (stamped != null && stampIndex !== index) { + stamped[index] = { ...finalPart, streamedIndex: stampIndex }; + } + } + /** Leftover substantial streamed parts mean the server REMOVED content + * (`hide_sequential_outputs`), so every pairing above is suspect β€” an + * omitted intermediate that happens to prefix the retained output would + * have claimed its identity. Only holes and empty slots may remain. */ + for (let rest = cursor; rest < streamedContent.length; rest++) { + const leftover = streamedContent[rest]; + if (leftover != null && !isEmptyContentPart(leftover)) { + return finalContent; + } + } + return stamped ?? finalContent; +}; + +/** + * Drops the client-only `streamedIndex` stamps from a content array. An + * edited resubmission retains the settled prefix and appends the rerun's + * parts at the prefix LENGTH β€” a stamp at or above that length would collide + * with an appended part's key β€” so the retained prefix reverts to physical + * identity for the rerun. Returns the input untouched when nothing is + * stamped. + */ +export function stripStreamedIndexStamps(content: TMessageContentParts[]): TMessageContentParts[]; +export function stripStreamedIndexStamps(content: TMessage['content']): TMessage['content']; +export function stripStreamedIndexStamps(content: TMessage['content']): TMessage['content'] { + if (!content?.length) { + return content; + } + let changed = false; + const next = content.map((part) => { + if (part == null || part.streamedIndex === undefined) { + return part; + } + changed = true; + const { streamedIndex: _streamedIndex, ...rest } = part; + return rest as TMessageContentParts; + }); + return changed ? next : content; +} + +/** Render-identity index for content-part keys: the streamed position stamped + * by the final handler survives the sparseβ†’compact swap; everything else keys + * by the live index. Coordinate logic (edit indexes, phase bounds, cursor) + * must keep using the live index. */ +export const getPartKeyIndex = (part: TMessageContentParts | undefined, idx: number): number => + part?.streamedIndex ?? idx; + /** * Whether a draft message has enough content to submit: non-whitespace * text, or at least one attached file. Lets users send a file without diff --git a/packages/data-provider/src/types/assistants.ts b/packages/data-provider/src/types/assistants.ts index 97752e545bd..9d309086f08 100644 --- a/packages/data-provider/src/types/assistants.ts +++ b/packages/data-provider/src/types/assistants.ts @@ -638,10 +638,20 @@ export type PartMetadata = { * as dispatch time rather than the task's runtime. */ backgrounded?: boolean; + /** + * Content index this part occupied while its run streamed. The aggregator + * writes parts at provider-source indexes, so the streamed array is sparse; + * persistence compacts it and every part after a hole shifts down. The + * client's final handler stamps the streamed position onto the compacted + * parts it adopts, so index-derived render identity survives the swap + * instead of remounting the settled message. Client-only and absent + * everywhere else β€” persisted content never carries it. + */ + streamedIndex?: number; }; /** Metadata for parallel content rendering - subset of PartMetadata */ -export type ContentMetadata = Pick; +export type ContentMetadata = Pick; export type ContentPart = ( | CodeToolCall From 862ebf3235d482eee8a8c247f3e4d2e51c0d8f26 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 06:50:46 -0400 Subject: [PATCH 02/14] =?UTF-8?q?=F0=9F=AA=A2=20fix:=20Persist=20Failed=20?= =?UTF-8?q?Agent=20Turns=20Before=20Error=20Publication=20(#14118)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../__tests__/request.resumeMetadata.spec.js | 260 +++++++++++++++++- api/server/controllers/agents/request.js | 219 ++++++++++++++- .../api/src/stream/GenerationJobManager.ts | 46 +++- .../api/src/stream/__tests__/startup.spec.ts | 80 ++++++ 4 files changed, 592 insertions(+), 13 deletions(-) diff --git a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js index 59e24d3c8df..02d35d6643c 100644 --- a/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js +++ b/api/server/controllers/agents/__tests__/request.resumeMetadata.spec.js @@ -63,6 +63,7 @@ const mockFilterPersistableAbortContent = jest.fn((content) => const mockGetConvo = jest.fn(); const mockGetMessages = jest.fn(); const mockSaveMessage = jest.fn(); +const mockSaveConvo = jest.fn(); const mockIsAgentTriggerPrincipalActive = jest.fn(); const mockIsSubagentOwnerAdmissible = jest.fn(); const mockAcquireEventChildGenerationLease = jest.fn(); @@ -224,6 +225,7 @@ jest.mock('~/cache', () => ({ jest.mock('~/models', () => ({ saveMessage: (...args) => mockSaveMessage(...args), + saveConvo: (...args) => mockSaveConvo(...args), getMessages: (...args) => mockGetMessages(...args), getConvo: (...args) => mockGetConvo(...args), isAgentTriggerPrincipalActive: (...args) => mockIsAgentTriggerPrincipalActive(...args), @@ -319,7 +321,12 @@ describe('ResumableAgentController resume metadata', () => { }), ); mockGenerationJobManager.finishTerminalJob.mockResolvedValue(undefined); - mockGenerationJobManager.completeJob.mockResolvedValue(true); + mockGenerationJobManager.completeJob.mockImplementation( + async (_streamId, _error, _createdAt, options) => { + await options?.beforeErrorPublication?.(); + return true; + }, + ); mockGenerationJobManager.beginProviderExecution.mockResolvedValue(true); mockGenerationJobManager.markProviderExecutionDrained.mockResolvedValue(true); mockGenerationJobManager.failPausePersistence.mockResolvedValue(true); @@ -334,6 +341,7 @@ describe('ResumableAgentController resume metadata', () => { mockGenerationJobManager.steering.park.mockResolvedValue(undefined); mockGenerationJobManager.steering.consumeRecovered.mockResolvedValue(true); mockSaveMessage.mockResolvedValue({}); + mockSaveConvo.mockResolvedValue({}); mockDeleteAgentCheckpoint.mockResolvedValue(undefined); }); @@ -1362,6 +1370,8 @@ describe('ResumableAgentController resume metadata', () => { await AgentController(req, res, jest.fn(), initializeClient, null); expect(allSubscribersLeftHandler).toEqual(expect.any(Function)); + mockSaveMessage.mockClear(); + mockSaveConvo.mockClear(); const oauthPart = { type: 'tool_call', @@ -2289,6 +2299,7 @@ describe('ResumableAgentController resume metadata', () => { error: 'Attached resources could not be restored', }), 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); }); @@ -2326,6 +2337,7 @@ describe('ResumableAgentController resume metadata', () => { error: 'Stateful code environment is not allowed by this deployment: conversation', }), 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); }); @@ -2516,6 +2528,7 @@ describe('ResumableAgentController resume metadata', () => { 'conversation-123', 'provider init failed', 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); }); @@ -2552,6 +2565,7 @@ describe('ResumableAgentController resume metadata', () => { 'conversation-123', 'Recovered steer cannot skip user message persistence', 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); }); @@ -3024,6 +3038,247 @@ describe('ResumableAgentController resume metadata', () => { expect(mockGenerationJobManager.claimTerminalJob).not.toHaveBeenCalled(); }); + describe('failed-turn persistence', () => { + const conversationId = 'conversation-123'; + + const createFailedRequest = (bodyOverrides = {}) => ({ + user: { id: 'user-123' }, + body: { + text: 'Hello with a removed model.', + messageId: 'user-message', + parentMessageId: 'prior-response', + conversationId, + endpointOption: { + endpoint: 'azureOpenAI', + modelOptions: { model: 'gpt-4o' }, + }, + ...bodyOverrides, + }, + config: {}, + }); + + async function flushBackgroundGeneration() { + for (let i = 0; i < 10; i++) { + await nextTick(); + } + } + + it('persists an initialization failure before terminal error publication', async () => { + const events = []; + mockSaveConvo.mockImplementation(async () => { + events.push('turn-persisted'); + return {}; + }); + mockGenerationJobManager.completeJob.mockImplementation( + async (_streamId, _error, _createdAt, options) => { + await options.beforeErrorPublication(); + events.push('error-published'); + return true; + }, + ); + const initializeClient = jest + .fn() + .mockRejectedValue(new Error('The model "gpt-4o" is not available.')); + + await AgentController( + createFailedRequest(), + createResumableResponse(), + jest.fn(), + initializeClient, + null, + ); + + expect(mockSaveMessage).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-123' }), + expect.objectContaining({ + messageId: 'user-message', + parentMessageId: 'prior-response', + conversationId, + text: 'Hello with a removed model.', + isCreatedByUser: true, + error: false, + }), + expect.any(Object), + ); + expect(mockSaveMessage).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-123' }), + expect.objectContaining({ + messageId: 'user-message_', + parentMessageId: 'user-message', + conversationId, + endpoint: 'azureOpenAI', + model: 'gpt-4o', + text: 'The model "gpt-4o" is not available.', + error: true, + isCreatedByUser: false, + }), + expect.any(Object), + ); + expect(events).toEqual(['turn-persisted', 'error-published']); + expect(mockSaveConvo).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-123' }), + { conversationId }, + expect.objectContaining({ noUpsert: true }), + ); + }); + + it('allows a follow-up to chain from the persisted failed response', async () => { + const initializeClient = jest.fn().mockRejectedValue(new Error('model unavailable')); + await AgentController( + createFailedRequest(), + createResumableResponse(), + jest.fn(), + initializeClient, + null, + ); + + expect(mockSaveMessage.mock.calls.map(([, message]) => message.messageId)).toContain( + 'user-message_', + ); + mockGetMessages.mockResolvedValue([{ _id: 'persisted-error-turn' }]); + const followUpRes = createResumableResponse(); + + await AgentController( + createFailedRequest({ + text: 'Retry with a valid model.', + messageId: 'follow-up-user', + parentMessageId: 'user-message_', + }), + followUpRes, + jest.fn(), + initializeClient, + null, + ); + + expect(followUpRes.status).not.toHaveBeenCalledWith(409); + expect(mockCheckAndIncrementPendingRequest).toHaveBeenCalledTimes(2); + }); + + it('persists failures raised before generation saves any message', async () => { + const client = { + options: {}, + sendMessage: jest.fn().mockRejectedValue(new Error('provider exploded')), + }; + + await AgentController( + createFailedRequest(), + createResumableResponse(), + jest.fn(), + jest.fn().mockResolvedValue({ client }), + null, + ); + await flushBackgroundGeneration(); + + expect(mockSaveMessage).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-123' }), + expect.objectContaining({ + messageId: 'user-message_', + text: 'provider exploded', + error: true, + }), + expect.any(Object), + ); + expect(mockGenerationJobManager.completeJob).toHaveBeenCalledWith( + conversationId, + 'provider exploded', + 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), + ); + }); + + it('uses the live user identity after generation starts', async () => { + const serverUserMessage = { + messageId: 'server-user', + parentMessageId: 'prior-response', + conversationId, + sender: 'User', + text: 'Hello with a removed model.', + isCreatedByUser: true, + }; + const client = { + options: {}, + sendMessage: jest.fn(async (_text, options) => { + options.onStart(serverUserMessage, 'server-response-uuid'); + throw new Error('failed after onStart'); + }), + }; + + await AgentController( + createFailedRequest(), + createResumableResponse(), + jest.fn(), + jest.fn().mockResolvedValue({ client }), + null, + ); + await flushBackgroundGeneration(); + + const savedIds = mockSaveMessage.mock.calls.map(([, message]) => message.messageId); + expect(savedIds).toEqual(expect.arrayContaining(['server-user', 'server-user_'])); + expect(savedIds).not.toContain('user-message_'); + }); + + it('does not overwrite an existing response row', async () => { + mockGetMessages.mockResolvedValue([{ _id: 'already-saved' }]); + + await AgentController( + createFailedRequest(), + createResumableResponse(), + jest.fn(), + jest.fn().mockRejectedValue(new Error('late failure')), + null, + ); + + expect(mockSaveMessage).not.toHaveBeenCalled(); + expect(mockSaveConvo).not.toHaveBeenCalled(); + }); + + it('creates the conversation row for a failed first turn', async () => { + const res = createResumableResponse(); + mockGenerationJobManager.claimGeneration.mockImplementation( + async (_userId, _clientRequestId, streamId, claimedConversationId) => + wonGenerationClaim({ streamId, conversationId: claimedConversationId }), + ); + const req = createFailedRequest({ + conversationId: undefined, + clientRequestId: 'failed-new-conversation', + parentMessageId: '00000000-0000-0000-0000-000000000000', + endpointOption: { + endpoint: 'azureOpenAI', + modelOptions: { model: 'gpt-4o' }, + chatProjectId: '507f1f77bcf86cd799439011', + }, + }); + + await AgentController( + req, + res, + jest.fn(), + jest.fn().mockRejectedValue(new Error('model unavailable')), + null, + ); + + const mintedConversationId = res.json.mock.calls[0][0].conversationId; + expect(mockSaveMessage).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ + messageId: 'user-message_', + conversationId: mintedConversationId, + }), + expect.any(Object), + ); + expect(mockSaveConvo).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'user-123' }), + expect.objectContaining({ + conversationId: mintedConversationId, + endpoint: 'azureOpenAI', + model: 'gpt-4o', + chatProjectId: '507f1f77bcf86cd799439011', + }), + expect.any(Object), + ); + }); + }); + it('finalizes the failed job before releasing the idempotency claim', async () => { mockGenerationJobManager.claimGeneration.mockResolvedValue(wonGenerationClaim()); const initializeClient = jest.fn().mockRejectedValue(new Error('init boom after res.json')); @@ -3046,6 +3301,7 @@ describe('ResumableAgentController resume metadata', () => { 'conversation-123', expect.any(String), 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); expect(mockGenerationJobManager.releaseGeneration).toHaveBeenCalledWith( 'user-123', @@ -3112,6 +3368,7 @@ describe('ResumableAgentController resume metadata', () => { 'conversation-123', 'init boom after res.json', 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); expect(mockGenerationJobManager.releaseGeneration).toHaveBeenCalledWith( 'user-123', @@ -3227,6 +3484,7 @@ describe('ResumableAgentController resume metadata', () => { 'conversation-123', generationError.message, 1000, + expect.objectContaining({ beforeErrorPublication: expect.any(Function) }), ); expect(mockGenerationJobManager.completeJob.mock.invocationCallOrder[0]).toBeLessThan( mockDecrementPendingRequest.mock.invocationCallOrder[0], diff --git a/api/server/controllers/agents/request.js b/api/server/controllers/agents/request.js index 75a1ee744c6..55ac564bb4e 100644 --- a/api/server/controllers/agents/request.js +++ b/api/server/controllers/agents/request.js @@ -40,6 +40,7 @@ const { logViolation } = require('~/cache'); const { recordScheduleOutcome, isScheduleLive } = require('~/server/services/Schedules'); const { saveMessage, + saveConvo, getMessages, getConvo, isAgentTriggerPrincipalActive, @@ -107,6 +108,18 @@ async function attachConversationCreatedAt(req, conversationId, conversationAnch } } +function getPreliminaryResponseMessageId({ messageId, responseMessageId }) { + if (typeof responseMessageId === 'string' && responseMessageId.length > 0) { + return responseMessageId; + } + + if (typeof messageId !== 'string' || messageId.length === 0) { + return null; + } + + return `${messageId.replace(/_+$/, '')}_`; +} + function getPreliminaryUserMessage( { messageId, parentMessageId, text, quotes, files, manualSkills, alwaysAppliedSkills }, conversationId, @@ -190,6 +203,165 @@ async function finishResumableRequest(req, userId) { } } +async function saveErrorTurn( + req, + { + conversationId, + endpointOption, + isNewConvo, + errorText, + liveUserMessage, + liveResponseMessageId, + sender, + }, +) { + try { + const { isContinued, isRegenerate, editedContent, responseMessageId, overrideParentMessageId } = + req.body ?? {}; + if ( + isContinued || + editedContent != null || + (responseMessageId && !isRegenerate) || + req.body?.recoverySteerId != null || + req.body?.clientRequestId?.startsWith?.('steer-recovery:') === true + ) { + return; + } + + let userMessage = null; + let errorMessageId = null; + let errorParentMessageId = null; + if (isRegenerate) { + errorMessageId = + typeof responseMessageId === 'string' && responseMessageId.length > 0 + ? responseMessageId + : null; + errorParentMessageId = liveUserMessage?.messageId ?? overrideParentMessageId ?? null; + } else { + userMessage = + liveUserMessage != null + ? { + ...liveUserMessage, + ...(liveUserMessage.files == null && + Array.isArray(req.body?.files) && + req.body.files.length > 0 && { files: req.body.files }), + ...(liveUserMessage.manualSkills == null && + Array.isArray(req.body?.manualSkills) && + req.body.manualSkills.length > 0 && { manualSkills: req.body.manualSkills }), + ...(liveUserMessage.alwaysAppliedSkills == null && + Array.isArray(req.body?.alwaysAppliedSkills) && + req.body.alwaysAppliedSkills.length > 0 && { + alwaysAppliedSkills: req.body.alwaysAppliedSkills, + }), + } + : getPreliminaryUserMessage(req.body, conversationId); + if (!userMessage) { + return; + } + errorMessageId = getPreliminaryResponseMessageId( + liveUserMessage != null ? { messageId: liveUserMessage.messageId } : req.body, + ); + errorParentMessageId = userMessage.messageId; + } + if (!errorMessageId || !errorParentMessageId) { + return; + } + + const userId = req.user.id; + const existing = await getMessages( + { user: userId, messageId: errorMessageId, conversationId }, + '_id', + ); + if (existing.length > 0) { + return; + } + if (liveResponseMessageId != null && liveResponseMessageId !== errorMessageId) { + const partial = await getMessages( + { user: userId, messageId: liveResponseMessageId, conversationId }, + '_id', + ); + if (partial.length > 0) { + return; + } + } + + const reqCtx = { + userId, + isTemporary: req?._agentEventBindingRetention?.isTemporary ?? req?.body?.isTemporary, + expiredAt: req?._agentEventBindingRetention?.expiredAt, + interfaceConfig: req?.config?.interfaceConfig, + }; + const context = 'api/server/controllers/agents/request.js - failed turn'; + const endpoint = endpointOption?.endpoint; + const model = getAgentResponseModel(req, endpointOption); + const iconURL = getEndpointIconURL(req, endpointOption); + + if (userMessage) { + const savedUserMessage = await saveMessage( + reqCtx, + { + ...userMessage, + user: userId, + sender: 'User', + isCreatedByUser: true, + error: false, + unfinished: false, + }, + { context }, + ); + if (!savedUserMessage) { + throw new Error('Failed user message could not be persisted'); + } + } + const savedErrorMessage = await saveMessage( + reqCtx, + { + messageId: errorMessageId, + conversationId, + parentMessageId: errorParentMessageId, + sender: sender ?? 'AI', + ...(endpoint != null && { endpoint }), + ...(model != null && { model }), + ...(iconURL != null && { iconURL }), + user: userId, + text: errorText, + error: true, + unfinished: false, + isCreatedByUser: false, + }, + { context }, + ); + if (!savedErrorMessage) { + throw new Error('Failed response message could not be persisted'); + } + + const agentId = endpointOption?.agent_id ?? req.body?.agent_id; + const chatProjectId = endpointOption?.chatProjectId ?? req.body?.chatProjectId; + const seedConvo = isNewConvo || req.resolvedConversation === null; + const convoFields = seedConvo + ? { + ...(endpoint != null && { endpoint }), + ...(endpointOption?.endpointType != null && { + endpointType: endpointOption.endpointType, + }), + ...(model != null && { model }), + ...(iconURL != null && { iconURL }), + ...(endpointOption?.spec != null && { spec: endpointOption.spec }), + ...(agentId != null && { agent_id: agentId }), + ...(typeof chatProjectId === 'string' && chatProjectId.length > 0 && { chatProjectId }), + } + : {}; + await saveConvo( + reqCtx, + { conversationId, ...convoFields }, + seedConvo ? { context } : { context, noUpsert: true }, + ); + } catch (err) { + logger.error('[AgentController] Failed to persist error turn', err); + throw err; + } +} + function classifyScheduledFailure(error, aborted = false) { if (aborted || error?.code === 'SCHEDULE_NO_LONGER_ACTIVE') { return { status: 'interrupted', error: error?.message }; @@ -1450,11 +1622,15 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit } let userMessage; + let liveResponseMessageId = preallocatedResponseMessageId; const getReqData = (data = {}) => { if (data.userMessage) { userMessage = data.userMessage; } + if (data.responseMessageId) { + liveResponseMessageId = data.responseMessageId; + } // conversationId is pre-generated, no need to update from callback }; @@ -1593,6 +1769,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit try { const onStart = (userMsg, respMsgId, _isNewConvo) => { userMessage = userMsg; + liveResponseMessageId = respMsgId; // Store userMessage and responseMessageId upfront for resume capability GenerationJobManager.updateMetadata( @@ -2179,8 +2356,18 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit // completeJob first wins running -> error and atomically parks // steers, then publishes. A competing abort/pause emits nothing. ownsScheduledFailure = - (await GenerationJobManager.completeJob(streamId, generationError, jobCreatedAt)) === - true; + (await GenerationJobManager.completeJob(streamId, generationError, jobCreatedAt, { + beforeErrorPublication: () => + saveErrorTurn(req, { + conversationId, + endpointOption, + isNewConvo, + errorText: generationError, + liveUserMessage: userMessage, + liveResponseMessageId, + sender: client?.sender, + }), + })) === true; } catch (completeErr) { logger.warn( '[ResumableAgentController] completeJob failed during generation-error cleanup', @@ -2262,6 +2449,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit } catch (error) { logger.error('[ResumableAgentController] Initialization error:', error); const initializationFailure = getInitializationFailure(error); + const streamStarted = res.headersSent; try { if (!res.headersSent) { if (error?.code === 'GENERATION_PREDECESSOR_MISMATCH') { @@ -2354,16 +2542,25 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit const initializationError = initializationFailure ? JSON.stringify(initializationFailure) : error.message || 'Failed to start generation'; + const completionPromise = streamStarted + ? GenerationJobManager.completeJob(streamId, initializationError, jobCreatedAt, { + beforeErrorPublication: () => + saveErrorTurn(req, { + conversationId, + endpointOption, + isNewConvo, + errorText: initializationError, + }), + }) + : GenerationJobManager.completeJob(streamId, initializationError, jobCreatedAt); initializationFinalized = - (await GenerationJobManager.completeJob(streamId, initializationError, jobCreatedAt).catch( - (completeErr) => { - logger.warn( - '[ResumableAgentController] completeJob failed during init-error cleanup', - completeErr, - ); - return false; - }, - )) === true; + (await completionPromise.catch((completeErr) => { + logger.warn( + '[ResumableAgentController] completeJob failed during init-error cleanup', + completeErr, + ); + return false; + })) === true; } if (initializationFinalized && !scheduleTerminalOutcomeRecorded) { await settleScheduledRun(classifyScheduledFailure(error)); diff --git a/packages/api/src/stream/GenerationJobManager.ts b/packages/api/src/stream/GenerationJobManager.ts index 632c1ce1aaa..24e9b210067 100644 --- a/packages/api/src/stream/GenerationJobManager.ts +++ b/packages/api/src/stream/GenerationJobManager.ts @@ -710,6 +710,10 @@ class GenerationJobManagerClass { * reconnect can replay that authoritative final payload. */ private terminalPublicationFailures = new WeakSet(); + /** Persistence-pending error claims whose terminal output was already + * reconciled by a competing owner or stale-owner recovery. */ + private terminalErrorPublicationSuppressions = new WeakSet(); + private cleanupInterval: NodeJS.Timeout | null = null; /** Generation-scoped retirement callbacks must not outlive the configured @@ -3535,7 +3539,7 @@ class GenerationJobManagerClass { // Error jobs stay durable long enough for late subscribers to receive the // stored error. A publication failure must never bypass the finally cleanup. try { - if (status === 'error') { + if (status === 'error' && !this.terminalErrorPublicationSuppressions.has(claim)) { const terminalError = error ?? 'Generation failed'; if (runtime) { runtime.errorEvent = terminalError; @@ -3606,6 +3610,7 @@ class GenerationJobManagerClass { this.releaseJobOwnership(streamId, createdAt); this.terminalPublicationFailures.delete(claim); + this.terminalErrorPublicationSuppressions.delete(claim); let metricStatus: 'completed' | 'error' | 'aborted' = 'aborted'; if (status === 'complete') { metricStatus = 'completed'; @@ -3640,16 +3645,55 @@ class GenerationJobManagerClass { streamId: string, error?: string, expectedCreatedAt?: number, + options: { beforeErrorPublication?: () => Promise } = {}, ): Promise { + const beforeErrorPublication = error ? options.beforeErrorPublication : undefined; const claim = await this.claimTerminalJob( streamId, error ? 'error' : 'complete', error, expectedCreatedAt, + beforeErrorPublication ? { persistencePending: true } : undefined, ); if (!claim) { return false; } + + if (beforeErrorPublication) { + let persistenceFinalized = false; + try { + await beforeErrorPublication(); + persistenceFinalized = await this.jobStore.finalizeTerminalPersistence( + streamId, + claim.createdAt, + JSON.stringify( + buildTerminalPersistenceReconcile({ + createdAt: claim.createdAt, + conversationId: claim.conversationId, + status: claim.status, + }), + ), + ); + } catch (persistenceError) { + logger.error( + `[GenerationJobManager] Failed required error persistence for ${streamId}:`, + persistenceError, + ); + try { + await this.publishTerminalClaim(claim, null); + } catch (publishError) { + logger.error( + `[GenerationJobManager] Failed to publish error persistence reconciliation for ${streamId}:`, + publishError, + ); + } + } + + if (!persistenceFinalized) { + this.terminalErrorPublicationSuppressions.add(claim); + } + } + await this.finishTerminalJob(claim); return true; } diff --git a/packages/api/src/stream/__tests__/startup.spec.ts b/packages/api/src/stream/__tests__/startup.spec.ts index 227635cd496..6709419b49f 100644 --- a/packages/api/src/stream/__tests__/startup.spec.ts +++ b/packages/api/src/stream/__tests__/startup.spec.ts @@ -2783,6 +2783,86 @@ describe('GenerationJobManager startup telemetry', () => { await manager.destroy(); }); + it('holds terminal error publication until required persistence finishes', async () => { + const jobStore = new InMemoryJobStore({ ttlAfterComplete: 60_000 }); + const manager = new GenerationJobManagerClass(); + manager.configure({ + jobStore, + eventTransport: new InMemoryEventTransport(), + isRedis: false, + cleanupOnComplete: false, + }); + manager.initialize(); + const streamId = 'stream-error-persistence-barrier'; + const job = await manager.createJob(streamId, 'user-1'); + const onError = jest.fn(); + const subscription = await manager.subscribe(streamId, () => undefined, undefined, onError); + let releasePersistence!: () => void; + const persistence = new Promise((resolve) => { + releasePersistence = resolve; + }); + + const completing = manager.completeJob(streamId, 'initialization failed', job.createdAt, { + beforeErrorPublication: () => persistence, + }); + await new Promise((resolve) => setImmediate(resolve)); + + expect(onError).not.toHaveBeenCalled(); + await expect(jobStore.getJob(streamId)).resolves.toMatchObject({ + status: 'error', + error: 'initialization failed', + terminalPersistencePending: true, + }); + + releasePersistence(); + await expect(completing).resolves.toBe(true); + + expect(onError).toHaveBeenCalledWith('initialization failed'); + await expect(jobStore.getJob(streamId)).resolves.toMatchObject({ + status: 'error', + error: 'initialization failed', + terminalPersistencePending: false, + }); + subscription?.unsubscribe(); + await manager.destroy(); + }); + + it('publishes reconciliation when required error persistence fails', async () => { + const jobStore = new InMemoryJobStore({ ttlAfterComplete: 60_000 }); + const manager = new GenerationJobManagerClass(); + manager.configure({ + jobStore, + eventTransport: new InMemoryEventTransport(), + isRedis: false, + cleanupOnComplete: false, + }); + manager.initialize(); + const streamId = 'stream-error-persistence-fails'; + const job = await manager.createJob(streamId, 'user-1'); + const onDone = jest.fn(); + const onError = jest.fn(); + const subscription = await manager.subscribe(streamId, () => undefined, onDone, onError); + + await expect( + manager.completeJob(streamId, 'initialization failed', job.createdAt, { + beforeErrorPublication: async () => { + throw new Error('message store unavailable'); + }, + }), + ).resolves.toBe(true); + + expect(onError).not.toHaveBeenCalled(); + expect(onDone).toHaveBeenCalledWith( + expect.objectContaining({ + final: true, + reconcile: true, + terminalStatus: 'error', + }), + ); + subscription?.unsubscribe(); + await manager.destroy(); + }); + it('atomically terminalizes a paused job when post-HITL persistence fails', async () => { const jobStore = new InMemoryJobStore({ ttlAfterComplete: 60_000 }); const manager = new GenerationJobManagerClass(); From cc0111b3cf90162799e278f907247ddbe24089bb Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 07:52:37 -0400 Subject: [PATCH 03/14] =?UTF-8?q?=F0=9F=93=90=20fix:=20Set=20the=20Elapsed?= =?UTF-8?q?=20Reading=20on=20the=20Column=20Its=20Neighbors=20Share=20(#15?= =?UTF-8?q?195)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The timer sat at the footer's flush left while everything around it is inset 6px: the streaming dot pads (24 βˆ’ 12) / 2 to center on the size-6 header icon's axis, and the hover-button glyphs that replace the timer sit behind their own p-1.5. The same ps-1.5 inline-start inset lines the reading up with the dot above it and the glyphs that follow it β€” measured in the live app: timer x 382, dot x 382, first settled glyph x 382. --- client/src/components/Chat/Messages/Elapsed.tsx | 7 ++++++- .../components/Chat/Messages/__tests__/Elapsed.spec.tsx | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/client/src/components/Chat/Messages/Elapsed.tsx b/client/src/components/Chat/Messages/Elapsed.tsx index 913f75b81b0..62c330ce2b9 100644 --- a/client/src/components/Chat/Messages/Elapsed.tsx +++ b/client/src/components/Chat/Messages/Elapsed.tsx @@ -59,8 +59,13 @@ const Elapsed = memo(function Elapsed({ index }: { index: number }) { }, [start]); const labels = getElapsedDurationLabels(seconds * 1000, i18n.language); + /** `ps-1.5` puts the reading on the column everything else in this slot + * shares: the streaming dot pads the same 6px to center on the size-6 + * header icon's axis (see `EmptyTextPart`), and the hover-button glyphs + * that replace the timer sit behind the same `p-1.5`. Inline-start, so + * the alignment holds in RTL. */ return ( - + diff --git a/client/src/components/Chat/Messages/__tests__/Elapsed.spec.tsx b/client/src/components/Chat/Messages/__tests__/Elapsed.spec.tsx index dbc86e732eb..15da4543160 100644 --- a/client/src/components/Chat/Messages/__tests__/Elapsed.spec.tsx +++ b/client/src/components/Chat/Messages/__tests__/Elapsed.spec.tsx @@ -34,6 +34,9 @@ describe('Elapsed', () => { expect(screen.getByTestId('stream-elapsed')).toHaveTextContent(/^5s$/); expect(screen.getByTestId('stream-elapsed')).toHaveAttribute('aria-hidden', 'true'); expect(screen.getByText('5 seconds elapsed')).toHaveClass('sr-only'); + /** The 6px inline-start inset that lines the reading up with the + * streaming dot and the hover-button glyphs that replace it. */ + expect(screen.getByTestId('stream-elapsed').parentElement).toHaveClass('ps-1.5'); advance(54_000); expect(screen.getByTestId('stream-elapsed')).toHaveTextContent(/^59s$/); From 4d246469dd3987ab05f22ff09b68e8f99932c907 Mon Sep 17 00:00:00 2001 From: James Todaro <30529065+jtodaroii@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:10:26 -0400 Subject: [PATCH 04/14] =?UTF-8?q?=F0=9F=A7=BE=20fix:=20Honor=20Disabled=20?= =?UTF-8?q?Transactions=20on=20the=20Abort=20Paths=20(#15099)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the transactions config from the request and forward it to both abort write paths, so `transactions.enabled: false` is honored when a generation is stopped. --- api/server/middleware/abortMiddleware.js | 9 +- api/server/middleware/abortMiddleware.spec.js | 111 +++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/api/server/middleware/abortMiddleware.js b/api/server/middleware/abortMiddleware.js index feed002b0e3..96db0b60d43 100644 --- a/api/server/middleware/abortMiddleware.js +++ b/api/server/middleware/abortMiddleware.js @@ -6,6 +6,7 @@ const { countTokens, GenerationJobManager, recordCollectedUsage, + getTransactionsConfig, sanitizeMessageForTransmit, buildAbortedResponseMetadata, } = require('@librechat/api'); @@ -59,6 +60,7 @@ const isAbortError = (error) => { * @param {Array} params.collectedUsage - Usage metadata from all models * @param {string} [params.fallbackModel] - Fallback model name if not in usage * @param {string} [params.messageId] - The response message ID for transaction correlation + * @param {AppConfig['transactions']} [params.transactions] - Resolved transactions config */ async function spendCollectedUsage({ userId, @@ -66,6 +68,7 @@ async function spendCollectedUsage({ collectedUsage, fallbackModel, messageId, + transactions, }) { if (!collectedUsage || collectedUsage.length === 0) { return; @@ -85,6 +88,7 @@ async function spendCollectedUsage({ context: 'abort', messageId, model: fallbackModel, + transactions, }, ); @@ -149,6 +153,8 @@ async function abortMessage(req, res) { responseMessage.metadata = abortMetadata; } + const transactions = getTransactionsConfig(req.config); + // Spend tokens for ALL models from collectedUsage (handles parallel agents/addedConvo) if (collectedUsage && collectedUsage.length > 0) { await spendCollectedUsage({ @@ -157,11 +163,12 @@ async function abortMessage(req, res) { collectedUsage, fallbackModel: jobData?.model, messageId: jobData?.responseMessageId, + transactions, }); } else { // Fallback: no collected usage, use text-based token counting for primary model only await db.spendTokens( - { ...responseMessage, context: 'incomplete', user: userId }, + { ...responseMessage, context: 'incomplete', user: userId, transactions }, { promptTokens, completionTokens }, ); } diff --git a/api/server/middleware/abortMiddleware.spec.js b/api/server/middleware/abortMiddleware.spec.js index 06e434065ab..20c47004213 100644 --- a/api/server/middleware/abortMiddleware.spec.js +++ b/api/server/middleware/abortMiddleware.spec.js @@ -19,6 +19,7 @@ const mockRecordCollectedUsage = jest const mockGetMultiplier = jest.fn().mockReturnValue(1); const mockGetCacheMultiplier = jest.fn().mockReturnValue(null); +const mockGetTransactionsConfig = jest.fn().mockReturnValue({ enabled: false }); jest.mock('@librechat/data-schemas', () => ({ logger: { @@ -37,7 +38,9 @@ jest.mock('@librechat/api', () => ({ abortJob: jest.fn(), }, recordCollectedUsage: mockRecordCollectedUsage, + getTransactionsConfig: (...args) => mockGetTransactionsConfig(...args), sanitizeMessageForTransmit: jest.fn((msg) => msg), + buildAbortedResponseMetadata: jest.fn().mockReturnValue(null), })); jest.mock('librechat-data-provider', () => ({ @@ -75,7 +78,9 @@ jest.mock('./abortRun', () => ({ const { logger } = require('@librechat/data-schemas'); const { sendError } = require('~/server/middleware/error'); -const { handleAbortError, spendCollectedUsage } = require('./abortMiddleware'); +const { GenerationJobManager } = require('@librechat/api'); +const db = require('~/models'); +const { handleAbort, handleAbortError, spendCollectedUsage } = require('./abortMiddleware'); const buildAbortRequest = () => ({ body: { @@ -310,3 +315,107 @@ describe('abortMiddleware - handleAbortError', () => { expect(sendError).toHaveBeenCalledTimes(1); }); }); + +/** + * The transactions config is resolved from the request's app config and must reach + * every write path in this file. `createTransaction` reads `transactions` from the + * caller-supplied data, so an omitted value is indistinguishable from enabled and + * the write proceeds even when `transactions.enabled` is false. + */ +describe('abortMiddleware - transactions config', () => { + const buildJobData = () => ({ + model: 'gpt-4', + responseMessageId: 'msg-123', + conversationId: 'convo-123', + endpoint: 'agents', + sender: 'AI', + promptTokens: 25, + userMessage: { + messageId: 'user-msg-123', + parentMessageId: 'parent-123', + conversationId: 'convo-123', + text: 'hello', + }, + }); + + const buildReq = () => ({ + body: { abortKey: 'convo-123:1', endpoint: 'agents' }, + user: { id: 'user-123', email: 'user@example.com' }, + config: { transactions: { enabled: false } }, + }); + + const buildRes = () => ({ + headersSent: false, + setHeader: jest.fn(), + send: jest.fn(), + }); + + beforeEach(() => { + jest.clearAllMocks(); + mockGetTransactionsConfig.mockReturnValue({ enabled: false }); + mockRecordCollectedUsage.mockResolvedValue({ input_tokens: 100, output_tokens: 50 }); + db.getConvo.mockResolvedValue({ title: 'Test Chat' }); + }); + + it('forwards transactions through spendCollectedUsage to recordCollectedUsage', async () => { + const collectedUsage = [{ input_tokens: 100, output_tokens: 50, model: 'gpt-4' }]; + + await spendCollectedUsage({ + userId: 'user-123', + conversationId: 'convo-123', + collectedUsage, + fallbackModel: 'gpt-4', + transactions: { enabled: false }, + }); + + expect(mockRecordCollectedUsage).toHaveBeenCalledTimes(1); + expect(mockRecordCollectedUsage).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ context: 'abort', transactions: { enabled: false } }), + ); + }); + + it('resolves the config from req and forwards it on the collected-usage path', async () => { + const collectedUsage = [{ input_tokens: 100, output_tokens: 50, model: 'gpt-4' }]; + GenerationJobManager.abortJob.mockResolvedValue({ + success: true, + jobData: buildJobData(), + content: [], + text: 'partial', + collectedUsage, + }); + + const req = buildReq(); + await handleAbort()(req, buildRes()); + + expect(logger.error).not.toHaveBeenCalled(); + expect(mockGetTransactionsConfig).toHaveBeenCalledWith(req.config); + expect(mockRecordCollectedUsage).toHaveBeenCalledTimes(1); + expect(mockRecordCollectedUsage).toHaveBeenCalledWith( + expect.any(Object), + expect.objectContaining({ context: 'abort', transactions: { enabled: false } }), + ); + }); + + it('resolves the config from req and forwards it on the token-count fallback path', async () => { + GenerationJobManager.abortJob.mockResolvedValue({ + success: true, + jobData: buildJobData(), + content: [], + text: 'partial', + collectedUsage: [], + }); + + const req = buildReq(); + await handleAbort()(req, buildRes()); + + expect(logger.error).not.toHaveBeenCalled(); + expect(mockGetTransactionsConfig).toHaveBeenCalledWith(req.config); + expect(mockRecordCollectedUsage).not.toHaveBeenCalled(); + expect(mockSpendTokens).toHaveBeenCalledTimes(1); + expect(mockSpendTokens).toHaveBeenCalledWith( + expect.objectContaining({ context: 'incomplete', transactions: { enabled: false } }), + expect.any(Object), + ); + }); +}); From 4b113697b5ea6a637ec04467fe218e1d573b6658 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 08:13:13 -0400 Subject: [PATCH 05/14] =?UTF-8?q?=F0=9F=94=8C=20feat:=20Background=20Execu?= =?UTF-8?q?tion=20Toggles=20for=20Actions=20&=20Plugin=20Tools=20(#14407)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🧡 feat: Background Execution Toggles for Actions & Plugin Tools * 🩹 fix: Resolve action background opt-in across encoded-domain forms and scope it per action * 🧹 refactor: Resolve action domain in a single pass * 🧩 fix: Merge Normalized Action Background Options * πŸͺ’ fix: Reconcile Action Background Aliases * 🧭 fix: Harden Action Background Compatibility * πŸ•°οΈ test: Allow Settled Task TTL Expiry * 🧬 fix: Merge Refreshed Action Tool Registrations --- api/server/services/ToolService.js | 89 ++++----- .../services/__tests__/ToolService.spec.js | 35 ++++ .../SidePanel/Agents/Actions/Background.tsx | 92 +++++++++ .../Actions/__tests__/Background.spec.tsx | 183 ++++++++++++++++++ .../SidePanel/Agents/AgentSelect.tsx | 28 ++- .../SidePanel/Agents/Background.tsx | 90 +++++++++ .../ItemDialog/__tests__/ToolSection.spec.tsx | 60 ++++++ .../ItemDialog/sections/ActionSection.tsx | 4 +- .../Tools/ItemDialog/sections/ToolSection.tsx | 16 ++ .../Agents/__tests__/Background.spec.tsx | 133 +++++++++++++ .../Agents/__tests__/agentTools.spec.ts | 21 ++ .../components/SidePanel/Agents/agentTools.ts | 20 ++ client/src/locales/en/translation.json | 2 + packages/api/src/actions/tools.spec.ts | 1 + packages/api/src/actions/tools.ts | 2 + packages/api/src/agents/background.spec.ts | 82 ++++++++ packages/api/src/agents/background.ts | 34 +++- packages/api/src/agents/initialize.ts | 12 +- .../api/src/agents/subagentThreads.spec.ts | 8 +- packages/api/src/tools/definitions.spec.ts | 48 +++++ packages/api/src/tools/definitions.ts | 23 ++- .../src/agentToolOptions.spec.ts | 17 +- .../data-provider/src/agentToolOptions.ts | 24 ++- .../data-provider/src/types/assistants.ts | 2 + 24 files changed, 960 insertions(+), 66 deletions(-) create mode 100644 client/src/components/SidePanel/Agents/Actions/Background.tsx create mode 100644 client/src/components/SidePanel/Agents/Actions/__tests__/Background.spec.tsx create mode 100644 client/src/components/SidePanel/Agents/Background.tsx create mode 100644 client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/ToolSection.spec.tsx create mode 100644 client/src/components/SidePanel/Agents/__tests__/Background.spec.tsx create mode 100644 client/src/components/SidePanel/Agents/__tests__/agentTools.spec.ts create mode 100644 client/src/components/SidePanel/Agents/agentTools.ts diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 51fed9159b4..485694dc528 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -35,6 +35,7 @@ const { getSafeErrorMetadata, isContentFilterError, isFileAuthoringToolDefinition, + normalizeActionToolName, ASK_USER_QUESTION_TOOL_NAME, splitMCPToolKey, buildServerNameAliases, @@ -53,6 +54,7 @@ const { ErrorTypes, ContentTypes, imageGenTools, + AuthTypeEnum, EModelEndpoint, EToolResources, isActionTool, @@ -202,32 +204,6 @@ const prepareActionSnapshotForTools = async ({ agentId, toolNames, filters, decr return { storedActions, actionSets }; }; -/** - * Collapse every `actionDomainSeparator` sequence in the encoded-domain - * suffix of a fully-qualified action tool name to an underscore. Agents - * can store tool names in the raw `domainParser(..., true)` output, - * which for short hostnames is a `---`-separated string (e.g. - * `medium---com`). The lookup maps below are always keyed with the - * `_`-collapsed domain, so every read must normalize that suffix or - * short-hostname tools silently fail to resolve. - * - * The operationId portion (everything before the last `actionDelimiter`) - * is deliberately left untouched: `openapiToFunction` preserves hyphens - * in generated operationIds, so two specs can legitimately produce - * operationIds that differ only in hyphens-vs-underscores (e.g. - * `get_foo---bar` vs `get_foo_bar`). Collapsing the operationId would - * merge those into a single map slot and silently drop one tool. - */ -const normalizeActionToolName = (toolName) => { - const delimiterIndex = toolName.lastIndexOf(actionDelimiter); - if (delimiterIndex === -1) { - return toolName; - } - const prefixEnd = delimiterIndex + actionDelimiter.length; - const encodedDomain = toolName.slice(prefixEnd); - return toolName.slice(0, prefixEnd) + encodedDomain.replace(domainSeparatorRegex, '_'); -}; - /** * Populate a `toolToAction` map with one slot per fully-qualified tool * name (``). Both the new @@ -1208,14 +1184,19 @@ async function loadToolDefinitionsWrapper({ for (const sig of functionSignatures) { const toolName = `${sig.name}${actionDelimiter}${normalizedDomain}`; const legacyToolName = `${sig.name}${actionDelimiter}${legacyNormalized}`; - if (!normalizedToolNames.has(toolName) && !normalizedToolNames.has(legacyToolName)) { + const matchesCurrentName = normalizedToolNames.has(toolName); + const matchesLegacyName = normalizedToolNames.has(legacyToolName); + if (!matchesCurrentName && !matchesLegacyName) { continue; } definitions.push({ - name: toolName, + /** Keep the selected legacy spelling when that is the only match so + * persisted tool_options resolve against the emitted definition. */ + name: matchesCurrentName ? toolName : legacyToolName, description: sig.description, parameters: sig.parameters, + oauth: action.metadata.auth?.type === AuthTypeEnum.OAuth, }); } } @@ -1223,28 +1204,34 @@ async function loadToolDefinitionsWrapper({ return definitions; }; - let { toolDefinitions, toolRegistry, hasDeferredTools, mcpToolAliases, mcpResolution } = - await loadToolDefinitions( - { - userId: req.user.id, - agentId: agent.id, - tools: defsFilteredTools, - toolOptions: agent.tool_options, - deferredToolsEnabled, - programmaticToolsEnabled, - codeExecutionEnabled, - provider: agent.provider, - mcpServerNames, - rawServerNames: mcpRawServerNames, - accessibleServerNames: defsAccessibleServerNames, - }, - { - isBuiltInTool, - getOrFetchMCPServerTools, - refreshMCPServerTools, - getActionToolDefinitions, - }, - ); + let { + toolDefinitions, + toolRegistry, + hasDeferredTools, + mcpToolAliases, + mcpResolution, + oauthActionToolNames, + } = await loadToolDefinitions( + { + userId: req.user.id, + agentId: agent.id, + tools: defsFilteredTools, + toolOptions: agent.tool_options, + deferredToolsEnabled, + programmaticToolsEnabled, + codeExecutionEnabled, + provider: agent.provider, + mcpServerNames, + rawServerNames: mcpRawServerNames, + accessibleServerNames: defsAccessibleServerNames, + }, + { + isBuiltInTool, + getOrFetchMCPServerTools, + refreshMCPServerTools, + getActionToolDefinitions, + }, + ); /** OAuth discovery must not reconnect (or prompt for) a server whose * definitions the collision filter deliberately rejected. */ @@ -1338,6 +1325,7 @@ async function loadToolDefinitionsWrapper({ hasDeferredTools = reloadResult.hasDeferredTools; mcpToolAliases = reloadResult.mcpToolAliases; mcpResolution = reloadResult.mcpResolution; + oauthActionToolNames = reloadResult.oauthActionToolNames; } } @@ -1454,6 +1442,7 @@ async function loadToolDefinitionsWrapper({ mcpToolAliases, actionsEnabled, primedCodeFiles, + oauthActionToolNames, }; } diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 8bf3126d727..4281e485ea1 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -3348,6 +3348,41 @@ describe('ToolService - Action Capability Gating', () => { expect(callArgs.requestBuilder.path).toBe('/echo'); }); + it('definitions-only loading emits the selected legacy action name', async () => { + mockLoadActionSets.mockResolvedValue([actionA]); + const legacyToolName = `echoMessage${actionDelimiter}${LEGACY_ENCODED_DOMAIN}`; + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + mockLoadToolDefinitions.mockImplementationOnce(async (_options, dependencies) => { + const definitions = await dependencies.getActionToolDefinitions('agent_legacy', [ + legacyToolName, + ]); + expect(definitions).toEqual([ + expect.objectContaining({ name: legacyToolName, description: 'Mock echoMessage' }), + ]); + return { + toolDefinitions: definitions, + toolRegistry: new Map(), + hasDeferredTools: false, + }; + }); + + await loadAgentTools({ + req, + res: {}, + agent: { + id: 'agent_legacy', + tools: [legacyToolName], + tool_options: { [legacyToolName]: { run_in_background: true } }, + }, + definitionsOnly: true, + }); + + expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1); + }); + it('loadAgentTools distinguishes operationIds that differ only by `---` vs `_`', async () => { // `openapiToFunction` uses the user-supplied operationId verbatim // and only sanitizes the synthetic `_` fallback, and diff --git a/client/src/components/SidePanel/Agents/Actions/Background.tsx b/client/src/components/SidePanel/Agents/Actions/Background.tsx new file mode 100644 index 00000000000..f8a405535cb --- /dev/null +++ b/client/src/components/SidePanel/Agents/Actions/Background.tsx @@ -0,0 +1,92 @@ +import { useMemo } from 'react'; +import { + AuthTypeEnum, + actionDelimiter, + openapiToFunction, + validateAndParseOpenAPISpec, +} from 'librechat-data-provider'; +import { useAgentCapabilities, useGetAgentsConfig } from '~/hooks'; +import { useGetExpandedAgentByIdQuery } from '~/data-provider'; +import { useAgentPanelContext } from '~/Providers'; +import { isEphemeralAgent } from '~/common'; +import Background from '../Background'; + +/** "Background execution" switch for a saved action β€” opts every operation of + * the action into background dispatch via `tool_options`. Hidden for OAuth + * actions: their calls can block on an interactive login prompt that a + * detached run could never surface (the server excludes them regardless). */ +export default function ActionBackground({ agentId }: { agentId: string }) { + const { action } = useAgentPanelContext(); + const { agentsConfig } = useGetAgentsConfig(); + const { backgroundToolsEnabled } = useAgentCapabilities(agentsConfig?.capabilities); + const { data: agent } = useGetExpandedAgentByIdQuery(agentId, { + enabled: backgroundToolsEnabled && action != null && !isEphemeralAgent(agentId), + }); + + /** The agent's `actions` entries are `${encodedDomain}_action_${action_id}`, + * so the saved encoded domain is recoverable without re-implementing the + * server's domain encoding. The domain alone is NOT enough to identify this + * action's tools: two actions may share a hostname, and their operations + * then share the suffix. Narrow by this spec's own operation ids, falling + * back to the suffix only when no other action shares the domain. */ + const actionToolIds = useMemo(() => { + const actionId = action?.action_id; + if (!actionId || !agent) { + return []; + } + let domain = ''; + const domainCounts = new Map(); + for (const entry of agent.actions ?? []) { + const idx = entry.indexOf(actionDelimiter); + if (idx < 1) { + continue; + } + const entryDomain = entry.slice(0, idx); + domainCounts.set(entryDomain, (domainCounts.get(entryDomain) ?? 0) + 1); + if (entry.slice(idx + actionDelimiter.length) === actionId) { + domain = entryDomain; + } + } + if (!domain) { + return []; + } + + const sharesDomain = (domainCounts.get(domain) ?? 0) > 1; + const suffix = `${actionDelimiter}${domain}`; + const domainTools = (agent.tools ?? []).filter((tool) => tool.endsWith(suffix)); + const spec = action?.metadata.raw_spec; + const parsed = spec ? validateAndParseOpenAPISpec(spec) : undefined; + if (!parsed?.spec) { + return sharesDomain ? [] : domainTools; + } + const functionSignatures = openapiToFunction(parsed.spec).functionSignatures; + const operationIds = new Set(functionSignatures.map((sig) => sig.name)); + const backgroundOperationIds = new Set( + functionSignatures + .filter((sig) => sig.parameters.properties.run_in_background == null) + .map((sig) => sig.name), + ); + const ownTools = domainTools.filter((tool) => + operationIds.has(tool.slice(0, tool.length - suffix.length)), + ); + if (ownTools.length === 0) { + return sharesDomain ? [] : domainTools; + } + return ownTools.filter((tool) => + backgroundOperationIds.has(tool.slice(0, tool.length - suffix.length)), + ); + }, [action?.action_id, action?.metadata.raw_spec, agent]); + + if (action?.metadata.auth?.type === AuthTypeEnum.OAuth) { + return null; + } + + return ( + + ); +} diff --git a/client/src/components/SidePanel/Agents/Actions/__tests__/Background.spec.tsx b/client/src/components/SidePanel/Agents/Actions/__tests__/Background.spec.tsx new file mode 100644 index 00000000000..a97f6d0d7d6 --- /dev/null +++ b/client/src/components/SidePanel/Agents/Actions/__tests__/Background.spec.tsx @@ -0,0 +1,183 @@ +import '@testing-library/jest-dom/extend-expect'; +import { AuthTypeEnum } from 'librechat-data-provider'; +import { useForm, FormProvider, useWatch } from 'react-hook-form'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type { Action, Agent } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; +import type { AgentForm } from '~/common'; +import ActionBackground from '../Background'; + +let mockBackgroundEnabled = true; +let mockAction: Partial | undefined; +let mockAgent: Partial | undefined; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, + useGetAgentsConfig: () => ({ agentsConfig: undefined }), + useAgentCapabilities: () => ({ backgroundToolsEnabled: mockBackgroundEnabled }), +})); +jest.mock('~/Providers', () => ({ + useAgentPanelContext: () => ({ action: mockAction }), +})); +jest.mock('~/data-provider', () => ({ + useGetExpandedAgentByIdQuery: () => ({ data: mockAgent }), +})); + +function OptionsProbe() { + const value = useWatch({ name: 'tool_options' }); + return {JSON.stringify(value ?? null)}; +} + +function renderActionBackground(defaultValues: Partial = {}) { + function Wrapper({ children }: { children: ReactNode }) { + const methods = useForm({ defaultValues: defaultValues as AgentForm }); + return ( + + {children} + + + ); + } + + return render(, { wrapper: Wrapper }); +} + +describe('ActionBackground', () => { + beforeEach(() => { + mockBackgroundEnabled = true; + mockAction = { action_id: 'act123', metadata: {} }; + mockAgent = { + id: 'agent_abc', + tools: [ + 'getWeather_action_weather---com', + 'getForecast_action_weather---com', + 'sendMail_action_mail---com', + 'web_search', + ], + actions: ['weather---com_action_act123', 'mail---com_action_act456'], + }; + }); + + test('toggling opts in every operation of this action and no other tools', () => { + renderActionBackground(); + const switchEl = screen.getByTestId('action-background-tools'); + expect(switchEl).not.toBeChecked(); + + fireEvent.click(switchEl); + const options = JSON.parse(screen.getByTestId('options').textContent ?? 'null'); + expect(options).toEqual({ + 'getWeather_action_weather---com': { run_in_background: true }, + getWeather_action_weather_com: { run_in_background: true }, + 'getForecast_action_weather---com': { run_in_background: true }, + getForecast_action_weather_com: { run_in_background: true }, + }); + }); + + test('reflects enabled when one operation is already opted in', () => { + renderActionBackground({ + tool_options: { 'getForecast_action_weather---com': { run_in_background: true } }, + }); + expect(screen.getByTestId('action-background-tools')).toBeChecked(); + }); + + test('opts in only the selected action when two actions share a hostname', () => { + mockAgent = { + id: 'agent_abc', + tools: [ + 'getWeather_action_api---example---com', + 'sendMail_action_api---example---com', + 'web_search', + ], + actions: ['api---example---com_action_act123', 'api---example---com_action_act456'], + }; + mockAction = { + action_id: 'act123', + metadata: { + raw_spec: JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Weather', version: '1.0.0' }, + servers: [{ url: 'https://api.example.com' }], + paths: { + '/weather': { + get: { operationId: 'getWeather', responses: { '200': { description: 'ok' } } }, + }, + }, + }), + }, + }; + + renderActionBackground(); + fireEvent.click(screen.getByTestId('action-background-tools')); + const options = JSON.parse(screen.getByTestId('options').textContent ?? 'null'); + expect(options).toEqual({ + 'getWeather_action_api---example---com': { run_in_background: true }, + getWeather_action_api_example_com: { run_in_background: true }, + }); + }); + + test('hides rather than guesses when a shared-hostname spec cannot be parsed', () => { + mockAgent = { + id: 'agent_abc', + tools: ['getWeather_action_api---example---com', 'sendMail_action_api---example---com'], + actions: ['api---example---com_action_act123', 'api---example---com_action_act456'], + }; + mockAction = { action_id: 'act123', metadata: {} }; + + renderActionBackground(); + expect(screen.queryByTestId('action-background-tools')).toBeNull(); + }); + + test('hidden for OAuth actions', () => { + mockAction = { action_id: 'act123', metadata: { auth: { type: AuthTypeEnum.OAuth } } }; + renderActionBackground(); + expect(screen.queryByTestId('action-background-tools')).toBeNull(); + }); + + test('hidden when every operation owns the run_in_background parameter', () => { + mockAgent = { + id: 'agent_abc', + tools: ['getWeather_action_weather---com'], + actions: ['weather---com_action_act123'], + }; + mockAction = { + action_id: 'act123', + metadata: { + raw_spec: JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Weather', version: '1.0.0' }, + servers: [{ url: 'https://weather.com' }], + paths: { + '/weather': { + get: { + operationId: 'getWeather', + parameters: [ + { + name: 'run_in_background', + in: 'query', + schema: { type: 'boolean' }, + }, + ], + responses: { '200': { description: 'ok' } }, + }, + }, + }, + }), + }, + }; + + renderActionBackground(); + expect(screen.queryByTestId('action-background-tools')).toBeNull(); + }); + + test('hidden when the action is not registered on the agent', () => { + mockAction = { action_id: 'act999', metadata: {} }; + renderActionBackground(); + expect(screen.queryByTestId('action-background-tools')).toBeNull(); + }); + + test('hidden when the background capability is off', () => { + mockBackgroundEnabled = false; + renderActionBackground(); + expect(screen.queryByTestId('action-background-tools')).toBeNull(); + }); +}); diff --git a/client/src/components/SidePanel/Agents/AgentSelect.tsx b/client/src/components/SidePanel/Agents/AgentSelect.tsx index 4bea8c1416e..08449d6fa8a 100644 --- a/client/src/components/SidePanel/Agents/AgentSelect.tsx +++ b/client/src/components/SidePanel/Agents/AgentSelect.tsx @@ -8,6 +8,7 @@ import type { UseMutationResult, QueryObserverResult } from '@tanstack/react-que import type { TAgentCapabilities, AgentForm } from '~/common'; import { cn, createProviderOption, processAgentOption, getDefaultAgentFormValues } from '~/utils'; import { useLocalize, useAgentDefaultPermissionLevel } from '~/hooks'; +import { mergeDirtyToolsWithServerActions } from './agentTools'; import { useListAgentsQuery } from '~/data-provider'; const keys = new Set(Object.keys(defaultAgentFormValues)); @@ -27,7 +28,17 @@ function AgentSelect({ }) { const localize = useLocalize(); const lastSelectedAgent = useRef(null); - const { control, reset } = useFormContext(); + const { + control, + getValues, + reset, + setValue, + /** Subscribing dirtyFields is required for reset({ keepDirtyValues: true }) + * to preserve edits when an action mutation refreshes the agent query. */ + formState: { dirtyFields }, + } = useFormContext(); + const dirtyFieldsRef = useRef(dirtyFields); + dirtyFieldsRef.current = dirtyFields; const permissionLevel = useAgentDefaultPermissionLevel(); const { data: agents = null } = useListAgentsQuery( @@ -46,7 +57,7 @@ function AgentSelect({ ); const resetAgentForm = useCallback( - (fullAgent: Agent) => { + (fullAgent: Agent, preserveDirtyValues = false) => { const isGlobal = fullAgent.isPublic ?? false; const update = { ...fullAgent, @@ -168,9 +179,16 @@ function AgentSelect({ formValues.skills_enabled = true; } - reset(formValues); + const mergedDirtyTools = + preserveDirtyValues && dirtyFieldsRef.current.tools != null + ? mergeDirtyToolsWithServerActions(getValues('tools') ?? [], agentTools) + : undefined; + reset(formValues, { keepDirtyValues: preserveDirtyValues }); + if (mergedDirtyTools != null) { + setValue('tools', mergedDirtyTools, { shouldDirty: true }); + } }, - [reset], + [getValues, reset, setValue], ); const onSelect = useCallback( @@ -207,7 +225,7 @@ function AgentSelect({ useEffect(() => { if (agentQuery.data && agentQuery.isSuccess) { - resetAgentForm(agentQuery.data); + resetAgentForm(agentQuery.data, true); } }, [agentQuery.data, agentQuery.isSuccess, resetAgentForm]); diff --git a/client/src/components/SidePanel/Agents/Background.tsx b/client/src/components/SidePanel/Agents/Background.tsx new file mode 100644 index 00000000000..054e9318e2f --- /dev/null +++ b/client/src/components/SidePanel/Agents/Background.tsx @@ -0,0 +1,90 @@ +import { useCallback } from 'react'; +import { useFormContext, useWatch } from 'react-hook-form'; +import { normalizeActionToolName } from 'librechat-data-provider'; +import { + Switch, + HoverCard, + HoverCardPortal, + HoverCardContent, + HoverCardTrigger, + CircleHelpIcon, +} from '@librechat/client'; +import type { TranslationKeys } from '~/hooks/useLocalize'; +import type { AgentForm } from '~/common'; +import { useAgentCapabilities, useGetAgentsConfig, useLocalize } from '~/hooks'; +import { withBooleanOption } from '~/hooks/Agents/useMCPToolOptions'; +import { ESide } from '~/common'; + +interface Props { + toolIds: string[]; + switchId: string; + labelKey: TranslationKeys; + infoKey: TranslationKeys; +} + +/** Shared "Background execution" switch β€” opts the given tool ids into + * background dispatch via `tool_options`. Reflects enabled when ANY id is + * opted in, mirroring the server, which honors each id independently and + * expands grouped ids (e.g. the code pair) across the group. */ +export default function Background({ toolIds, switchId, labelKey, infoKey }: Props) { + const localize = useLocalize(); + const { agentsConfig } = useGetAgentsConfig(); + const { backgroundToolsEnabled } = useAgentCapabilities(agentsConfig?.capabilities); + const { control, getValues, setValue } = useFormContext(); + const toolOptions = useWatch({ control, name: 'tool_options' }); + const enabled = toolIds.some((toolId) => { + const normalized = normalizeActionToolName(toolId); + const normalizedValue = toolOptions?.[normalized]?.run_in_background; + if (normalized !== toolId && normalizedValue != null) { + return normalizedValue; + } + return toolOptions?.[toolId]?.run_in_background === true; + }); + + const handleChange = useCallback( + (value: boolean) => { + let updated = getValues('tool_options') || {}; + for (const toolId of toolIds) { + const normalized = normalizeActionToolName(toolId); + const aliases = normalized === toolId ? [toolId] : [toolId, normalized]; + for (const alias of aliases) { + updated = withBooleanOption(updated, alias, 'run_in_background', value); + } + } + setValue('tool_options', updated, { shouldDirty: true }); + }, + [toolIds, getValues, setValue], + ); + + if (!backgroundToolsEnabled || toolIds.length === 0) { + return null; + } + + return ( + +
+
+
{localize(labelKey)}
+ + + +
+ + +
+

{localize(infoKey)}

+
+ + + +
+ + ); +} diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/ToolSection.spec.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/ToolSection.spec.tsx new file mode 100644 index 00000000000..a599afc81b0 --- /dev/null +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/ToolSection.spec.tsx @@ -0,0 +1,60 @@ +import '@testing-library/jest-dom/extend-expect'; +import { useForm, FormProvider } from 'react-hook-form'; +import { render, screen } from '@testing-library/react'; +import type { TPlugin } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; +import type { ToolItem } from '../../items/types'; +import type { AgentForm } from '~/common'; +import ToolSection from '../sections/ToolSection'; + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, + useGetAgentsConfig: () => ({ agentsConfig: undefined }), + useAgentCapabilities: () => ({ backgroundToolsEnabled: true }), +})); +jest.mock('librechat-data-provider/react-query', () => ({ + useUpdateUserPluginsMutation: () => ({ mutate: jest.fn(), isLoading: false }), +})); +jest.mock('@librechat/client', () => ({ + ...jest.requireActual('@librechat/client'), + useToastContext: () => ({ showToast: jest.fn() }), +})); +jest.mock('~/components/Plugins/Store/PluginAuthForm', () => ({ + __esModule: true, + default: () =>
, +})); + +function toolItem(id: string): ToolItem { + return { + kind: 'tool', + id, + name: id, + description: 'A tool', + iconKey: 'tool', + plugin: { pluginKey: id, name: id, authConfig: [] } as unknown as TPlugin, + }; +} + +function renderSection(item: ToolItem) { + function Wrapper({ children }: { children: ReactNode }) { + const methods = useForm({ defaultValues: {} as AgentForm }); + return {children}; + } + + return render(, { wrapper: Wrapper }); +} + +describe('ToolSection background switch', () => { + test('renders the switch for a background-eligible plugin tool', () => { + renderSection(toolItem('wolfram')); + expect(screen.getByTestId('tool-background')).toBeInTheDocument(); + }); + + test('does not render the switch for image generation tools', () => { + renderSection(toolItem('dalle')); + expect(screen.queryByTestId('tool-background')).toBeNull(); + + renderSection(toolItem('image_gen_oai')); + expect(screen.queryByTestId('tool-background')).toBeNull(); + }); +}); diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/ActionSection.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/ActionSection.tsx index 5d34c3ad4e5..f62a70fa9d3 100644 --- a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/ActionSection.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/ActionSection.tsx @@ -1,5 +1,6 @@ import { useEffect } from 'react'; import type { ActionItem } from '../../items/types'; +import ActionBackground from '../../../Actions/Background'; import { useAgentPanelContext } from '~/Providers'; import { NEW_ACTION_ID } from '../../items/types'; import ActionEditor from '../../ActionEditor'; @@ -20,7 +21,8 @@ export default function ActionSection({ item, agentId, onClose }: Props) { }, [isCreate, item.action, setAction]); return ( -
+
+ {!isCreate && } + !imageGenTools.has(toolId) && toolId !== 'image_gen_oai' && toolId !== 'image_edit_oai'; + export default function ToolSection({ item }: Props) { const localize = useLocalize(); const { showToast } = useToastContext(); @@ -94,6 +102,14 @@ export default function ToolSection({ item }: Props) { onSubmit={handleSubmit} /> )} + {isBackgroundEligibleTool(item.id) && ( + + )}
); } diff --git a/client/src/components/SidePanel/Agents/__tests__/Background.spec.tsx b/client/src/components/SidePanel/Agents/__tests__/Background.spec.tsx new file mode 100644 index 00000000000..ad3a0da0eaf --- /dev/null +++ b/client/src/components/SidePanel/Agents/__tests__/Background.spec.tsx @@ -0,0 +1,133 @@ +import '@testing-library/jest-dom/extend-expect'; +import { useForm, FormProvider, useWatch } from 'react-hook-form'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import type { AgentForm } from '~/common'; +import Background from '../Background'; + +let mockBackgroundEnabled = true; +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, + useGetAgentsConfig: () => ({ agentsConfig: undefined }), + useAgentCapabilities: () => ({ backgroundToolsEnabled: mockBackgroundEnabled }), +})); + +function OptionsProbe() { + const value = useWatch({ name: 'tool_options' }); + return {JSON.stringify(value ?? null)}; +} + +function renderBackground(toolIds: string[], defaultValues: Partial = {}) { + function Wrapper({ children }: { children: ReactNode }) { + const methods = useForm({ defaultValues: defaultValues as AgentForm }); + return ( + + {children} + + + ); + } + + return render( + , + { wrapper: Wrapper }, + ); +} + +describe('Background switch', () => { + beforeEach(() => { + mockBackgroundEnabled = true; + }); + + test('hidden when the background capability is off', () => { + mockBackgroundEnabled = false; + renderBackground(['wolfram']); + expect(screen.queryByTestId('bg-switch')).toBeNull(); + }); + + test('hidden when there are no tool ids to opt in', () => { + renderBackground([]); + expect(screen.queryByTestId('bg-switch')).toBeNull(); + }); + + test('reflects enabled when ANY grouped id is opted in', () => { + renderBackground(['execute_code', 'bash_tool'], { + tool_options: { bash_tool: { run_in_background: true } }, + }); + expect(screen.getByTestId('bg-switch')).toBeChecked(); + }); + + test('toggling writes every grouped id and clears entries on disable', () => { + renderBackground(['execute_code', 'bash_tool']); + const switchEl = screen.getByTestId('bg-switch'); + expect(switchEl).not.toBeChecked(); + + fireEvent.click(switchEl); + expect(switchEl).toBeChecked(); + const enabled = JSON.parse(screen.getByTestId('options').textContent ?? 'null'); + expect(enabled).toEqual({ + execute_code: { run_in_background: true }, + bash_tool: { run_in_background: true }, + }); + + fireEvent.click(switchEl); + expect(switchEl).not.toBeChecked(); + const disabled = JSON.parse(screen.getByTestId('options').textContent ?? 'null'); + expect(disabled).toEqual({}); + }); + + test('preserves unrelated per-tool options when toggling', () => { + renderBackground(['wolfram'], { + tool_options: { search_mcp_docs: { defer_loading: true } }, + }); + fireEvent.click(screen.getByTestId('bg-switch')); + const options = JSON.parse(screen.getByTestId('options').textContent ?? 'null'); + expect(options).toEqual({ + search_mcp_docs: { defer_loading: true }, + wolfram: { run_in_background: true }, + }); + }); + + test('reconciles raw and normalized action aliases when enabling', () => { + const rawToolId = 'getPerson_action_swapi---tech'; + const normalizedToolId = 'getPerson_action_swapi_tech'; + renderBackground([rawToolId], { + tool_options: { + [rawToolId]: { run_in_background: true }, + [normalizedToolId]: { defer_loading: true, run_in_background: false }, + }, + }); + + const switchEl = screen.getByTestId('bg-switch'); + expect(switchEl).not.toBeChecked(); + fireEvent.click(switchEl); + + expect(JSON.parse(screen.getByTestId('options').textContent ?? 'null')).toEqual({ + [rawToolId]: { run_in_background: true }, + [normalizedToolId]: { defer_loading: true, run_in_background: true }, + }); + }); + + test('clears both action aliases when disabling', () => { + const rawToolId = 'getPerson_action_swapi---tech'; + const normalizedToolId = 'getPerson_action_swapi_tech'; + renderBackground([rawToolId], { + tool_options: { + [normalizedToolId]: { defer_loading: true, run_in_background: true }, + }, + }); + + const switchEl = screen.getByTestId('bg-switch'); + expect(switchEl).toBeChecked(); + fireEvent.click(switchEl); + + expect(JSON.parse(screen.getByTestId('options').textContent ?? 'null')).toEqual({ + [normalizedToolId]: { defer_loading: true }, + }); + }); +}); diff --git a/client/src/components/SidePanel/Agents/__tests__/agentTools.spec.ts b/client/src/components/SidePanel/Agents/__tests__/agentTools.spec.ts new file mode 100644 index 00000000000..631b48816a4 --- /dev/null +++ b/client/src/components/SidePanel/Agents/__tests__/agentTools.spec.ts @@ -0,0 +1,21 @@ +import { mergeDirtyToolsWithServerActions } from '../agentTools'; + +describe('mergeDirtyToolsWithServerActions', () => { + it('preserves dirty non-action choices and replaces action registrations', () => { + expect( + mergeDirtyToolsWithServerActions( + ['local_plugin', 'removed_action_old---example---com'], + ['server_plugin', 'added_action_new---example---com'], + ), + ).toEqual(['local_plugin', 'added_action_new---example---com']); + }); + + it('does not duplicate a server action registration', () => { + expect( + mergeDirtyToolsWithServerActions( + [], + ['get_action_api---example---com', 'get_action_api---example---com'], + ), + ).toEqual(['get_action_api---example---com']); + }); +}); diff --git a/client/src/components/SidePanel/Agents/agentTools.ts b/client/src/components/SidePanel/Agents/agentTools.ts new file mode 100644 index 00000000000..9d3806d4c5f --- /dev/null +++ b/client/src/components/SidePanel/Agents/agentTools.ts @@ -0,0 +1,20 @@ +import { isActionTool } from 'librechat-data-provider'; + +/** + * Keeps unsaved non-action tool choices while accepting the server's canonical + * action registrations after an adjacent action create/update mutation. + */ +export function mergeDirtyToolsWithServerActions( + dirtyTools: readonly string[], + serverTools: readonly string[], +): string[] { + const merged = dirtyTools.filter((tool) => !isActionTool(tool)); + const seen = new Set(merged); + for (const tool of serverTools) { + if (isActionTool(tool) && !seen.has(tool)) { + merged.push(tool); + seen.add(tool); + } + } + return merged; +} diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 1b16c3ed040..1843c97b129 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -554,6 +554,7 @@ "com_nav_info_smooth_streaming": "When enabled, newly streamed words fade in smoothly for the latest response. This is purely visual β€” it does not delay token delivery β€” and is disabled automatically when your device prefers reduced motion.", "com_nav_info_stateful_sessions": "When enabled, this agent uses the dedicated stateful Code API instead of the default stateless service. Files, installed packages, and working state usually carry over between runs. The workspace may occasionally reset, so save anything important under /mnt/data. Requires Code Interpreter and the app-level stateful sessions capability.", "com_nav_info_stateful_code_environment": "Choose who shares this agent's stateful workspace. This does not share live files with stateless code sessions.", + "com_nav_info_tool_background": "When enabled, the model can run this tool in the background: the conversation continues immediately while the tool runs, and the model retrieves the result later with the background task tool. Requires the app-level background tools capability.", "com_nav_info_user_name_display": "When enabled, the username of the sender will be shown above each message you send. When disabled, you will only see \"You\" above your messages.", "com_nav_keep_screen_awake": "Keep screen awake during response generation", "com_nav_lang_arabic": "Ψ§Ω„ΨΉΨ±Ψ¨ΩŠΨ©", @@ -2279,6 +2280,7 @@ "com_ui_token_exchange_method": "Token Exchange Method", "com_ui_token_url": "Token URL", "com_ui_tokens": "tokens", + "com_ui_tool_background": "Background execution", "com_ui_tool_collection_prefix": "A collection of tools from", "com_ui_tool_credentials_saved": "Credentials saved", "com_ui_tool_failed": "failed", diff --git a/packages/api/src/actions/tools.spec.ts b/packages/api/src/actions/tools.spec.ts index 2b4747de646..e0c18f611e0 100644 --- a/packages/api/src/actions/tools.spec.ts +++ b/packages/api/src/actions/tools.spec.ts @@ -2,6 +2,7 @@ jest.mock( 'librechat-data-provider', () => ({ actionDelimiter: '_action_', + normalizeActionToolName: (toolName: string) => toolName, validateAndParseOpenAPISpec: (specString: string) => { const spec = JSON.parse(specString) as { paths?: Record }; return { diff --git a/packages/api/src/actions/tools.ts b/packages/api/src/actions/tools.ts index 2647d5cde50..89c96c019a9 100644 --- a/packages/api/src/actions/tools.ts +++ b/packages/api/src/actions/tools.ts @@ -1,5 +1,7 @@ import { actionDelimiter, validateAndParseOpenAPISpec } from 'librechat-data-provider'; +export { normalizeActionToolName } from 'librechat-data-provider'; + export type ActionToolLike = { function?: { name?: string; diff --git a/packages/api/src/agents/background.spec.ts b/packages/api/src/agents/background.spec.ts index 810b2a27502..cb73053bcf6 100644 --- a/packages/api/src/agents/background.spec.ts +++ b/packages/api/src/agents/background.spec.ts @@ -237,6 +237,88 @@ describe('applyBackgroundToolCalls', () => { ).toBeUndefined(); }); + it('resolves an action opt-in stored with the raw `---` domain against the collapsed def name', () => { + /** Agents persist `swapi---tech`; the runtime def is named `swapi_tech`. */ + const defs = [mcpDef('getPerson_action_swapi_tech')]; + const registry: LCToolRegistry = new Map(defs.map((d) => [d.name, { ...d }])); + const result = applyBackgroundToolCalls({ + toolDefinitions: defs, + toolRegistry: registry, + toolOptions: { 'getPerson_action_swapi---tech': { run_in_background: true } }, + }); + expect(result.backgroundToolNames).toEqual(['getPerson_action_swapi_tech']); + expect(registry.has(CHECK_BACKGROUND_TASK_NAME)).toBe(true); + }); + + it('merges a raw action opt-in into an existing normalized option entry', () => { + const defs = [mcpDef('getPerson_action_swapi_tech')]; + const registry: LCToolRegistry = new Map(defs.map((d) => [d.name, { ...d }])); + const result = applyBackgroundToolCalls({ + toolDefinitions: defs, + toolRegistry: registry, + toolOptions: { + 'getPerson_action_swapi---tech': { run_in_background: true }, + getPerson_action_swapi_tech: { defer_loading: true }, + }, + }); + expect(result.backgroundToolNames).toEqual(['getPerson_action_swapi_tech']); + }); + + it('keeps an explicit normalized action background option authoritative', () => { + const defs = [mcpDef('getPerson_action_swapi_tech')]; + const registry: LCToolRegistry = new Map(defs.map((d) => [d.name, { ...d }])); + const result = applyBackgroundToolCalls({ + toolDefinitions: defs, + toolRegistry: registry, + toolOptions: { + 'getPerson_action_swapi---tech': { run_in_background: true }, + getPerson_action_swapi_tech: { run_in_background: false }, + }, + }); + expect(result.backgroundToolNames).toEqual([]); + }); + + it('does not collapse hyphens in the operationId when normalizing an action key', () => { + const defs = [ + mcpDef('get_foo---bar_action_swapi_tech'), + mcpDef('get_foo_bar_action_swapi_tech'), + ]; + const registry: LCToolRegistry = new Map(defs.map((d) => [d.name, { ...d }])); + const result = applyBackgroundToolCalls({ + toolDefinitions: defs, + toolRegistry: registry, + toolOptions: { 'get_foo---bar_action_swapi---tech': { run_in_background: true } }, + }); + expect(result.backgroundToolNames).toEqual(['get_foo---bar_action_swapi_tech']); + }); + + it('injects an opted-in action tool but not one the OAuth excludeTool rejects', () => { + const oauthActionNames = new Set(['sendMail_action_mail---example---com']); + const defs = [ + mcpDef('getWeather_action_weather---com'), + mcpDef('sendMail_action_mail---example---com'), + ]; + const registry: LCToolRegistry = new Map(defs.map((d) => [d.name, { ...d }])); + const result = applyBackgroundToolCalls({ + toolDefinitions: defs, + toolRegistry: registry, + toolOptions: { + 'getWeather_action_weather---com': { run_in_background: true }, + 'sendMail_action_mail---example---com': { run_in_background: true }, + }, + excludeTool: (name) => oauthActionNames.has(name), + }); + expect(result.backgroundToolNames).toEqual(['getWeather_action_weather---com']); + const oauthDef = result.toolDefinitions.find( + (d) => d.name === 'sendMail_action_mail---example---com', + ); + expect( + (oauthDef?.parameters as { properties?: Record }).properties?.[ + RUN_IN_BACKGROUND_ARG + ], + ).toBeUndefined(); + }); + it('skips a non-object (string-input) schema without rewriting it', () => { const defs = [{ name: 'legacy_tool', parameters: { type: 'string' } } as unknown as LCTool]; const result = applyBackgroundToolCalls({ diff --git a/packages/api/src/agents/background.ts b/packages/api/src/agents/background.ts index 1dde2b56548..079ae3ff627 100644 --- a/packages/api/src/agents/background.ts +++ b/packages/api/src/agents/background.ts @@ -61,6 +61,7 @@ import { SubagentTaskOwnerUnavailableError } from './subagentTaskRouting'; import { SET_MEMORY_TOOL_NAME, DELETE_MEMORY_TOOL_NAME } from './memory'; import { ASK_USER_QUESTION_TOOL_NAME } from './hitl/askUserQuestionTool'; import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME } from './tools'; +import { normalizeActionToolName } from '~/actions/tools'; import { truncateMiddle } from '~/utils'; /** Argument the model sets on a tool call to dispatch it in the background. */ @@ -125,6 +126,36 @@ const EXCLUDED_BACKGROUND_TOOL_NAMES: ReadonlySet = new Set([ 'image_edit_oai', ]); +/** + * Agents persist action tool names with the raw encoded domain (`---` for short + * hostnames), while the runtime definitions those names must match against are + * always `_`-collapsed. The builder writes `tool_options` keyed by the persisted + * name, so alias every action-shaped key to its normalized form; without this + * the opt-in silently never resolves for short-hostname actions. Merge the raw + * background option into any normalized entry while keeping an explicit + * normalized background value authoritative. + */ +function expandActionToolOptions(toolOptions: AgentToolOptions): AgentToolOptions { + let expanded: AgentToolOptions | undefined; + for (const [name, options] of Object.entries(toolOptions)) { + const normalized = normalizeActionToolName(name); + const runInBackground = options?.run_in_background; + if ( + normalized === name || + runInBackground == null || + toolOptions[normalized]?.run_in_background != null + ) { + continue; + } + expanded = expanded ?? { ...toolOptions }; + expanded[normalized] = { + ...toolOptions[normalized], + run_in_background: runInBackground, + }; + } + return expanded ?? toolOptions; +} + /** * Whether a tool may be dispatched in the background. Handoff tools * (`lc_transfer_to_*`) run through the direct path and are excluded by prefix. @@ -466,7 +497,8 @@ export function applyBackgroundToolCalls(params: { */ excludeTool?: (toolName: string) => boolean; }): { toolDefinitions: LCTool[]; backgroundToolNames: string[] } { - const { toolRegistry, toolOptions, capabilityToolNames, excludeTool } = params; + const { toolRegistry, capabilityToolNames, excludeTool } = params; + const toolOptions = params.toolOptions && expandActionToolOptions(params.toolOptions); const defs = params.toolDefinitions ?? []; const selectionNames = getSelectionNames(toolOptions, 'run_in_background'); const effectiveSources = new Set(); diff --git a/packages/api/src/agents/initialize.ts b/packages/api/src/agents/initialize.ts index 2a1fcdbdc95..5831c95b954 100644 --- a/packages/api/src/agents/initialize.ts +++ b/packages/api/src/agents/initialize.ts @@ -555,6 +555,8 @@ export interface InitializeAgentParams { hasDeferredTools?: boolean; mcpToolAliases?: MCPToolAlias[]; actionsEnabled?: boolean; + /** Action tool names backed by OAuth β€” excluded from background dispatch. */ + oauthActionToolNames?: string[]; /** * Pre-uploaded code-env file refs for the agent's * `tool_resources.execute_code`. Bubbled up so the run host can seed @@ -1308,6 +1310,7 @@ export async function initializeAgent( hasDeferredTools, mcpToolAliases, actionsEnabled, + oauthActionToolNames, tools: structuredTools, primedCodeFiles, } = loadToolsResult ?? { @@ -1322,6 +1325,7 @@ export async function initializeAgent( hasDeferredTools: false, mcpToolAliases: [], actionsEnabled: undefined, + oauthActionToolNames: undefined, primedCodeFiles: undefined, }; @@ -1540,6 +1544,7 @@ export async function initializeAgent( * ephemeral subset: a non-ephemeral name ending in an ephemeral one would * otherwise be misread as ephemeral. */ const allServerNames = Object.keys(req.config?.mcpConfig ?? {}).map(normalizeServerName); + const oauthActionNames = new Set(oauthActionToolNames ?? []); const backgroundResult = applyBackgroundToolCalls({ toolDefinitions, toolRegistry, @@ -1549,8 +1554,13 @@ export async function initializeAgent( * placeholders) never get the param: their connection dies at request * end, so the executor would only downgrade the call to foreground. * Unknown servers stay eligible β€” the executor's per-instance tag is - * the fail-safe for those. */ + * the fail-safe for those. OAuth-backed action tools are excluded too: + * a detached call can block on an interactive login prompt the user + * never sees. */ excludeTool: (toolName) => { + if (oauthActionNames.has(toolName)) { + return true; + } const [, serverName] = splitMCPToolKey(toolName, allServerNames); return serverName != null && ephemeralServerNames.has(serverName); }, diff --git a/packages/api/src/agents/subagentThreads.spec.ts b/packages/api/src/agents/subagentThreads.spec.ts index f1f8c383e06..4861022d7b5 100644 --- a/packages/api/src/agents/subagentThreads.spec.ts +++ b/packages/api/src/agents/subagentThreads.spec.ts @@ -3778,7 +3778,13 @@ describe('SubagentThreadTaskStore', () => { expect(store.get(config.scopeId, liveTaskId)?.pendingControls).toBe(2); finish({ content: 'done' }); - await waitForSettled(store, config.scopeId, live); + /** This store deliberately uses a 20 ms completed TTL. Under coverage the + * task can settle and expire between polling ticks, which is also a valid + * terminal outcome for the cleanup asserted by this test. */ + await waitUntil( + () => store.get(config.scopeId, liveTaskId)?.status !== 'running', + 'the live replay-window task to settle or expire', + ); await store.destroyTaskControlTransport(); }); diff --git a/packages/api/src/tools/definitions.spec.ts b/packages/api/src/tools/definitions.spec.ts index 9e7fc239e19..809e1ba9b55 100644 --- a/packages/api/src/tools/definitions.spec.ts +++ b/packages/api/src/tools/definitions.spec.ts @@ -48,6 +48,7 @@ describe('definitions.ts', () => { expect(result.toolDefinitions).toHaveLength(0); expect(result.toolRegistry.size).toBe(0); expect(result.hasDeferredTools).toBe(false); + expect(result.oauthActionToolNames).toEqual([]); }); describe('action tool definitions', () => { @@ -130,6 +131,53 @@ describe('definitions.ts', () => { expect(actionDef?.parameters).toBeUndefined(); }); + it('collects OAuth action tool names and strips the marker from emitted defs', async () => { + const mockActionDefs: ActionToolDefinition[] = [ + { + name: 'getWeather_action_weather_com', + description: 'Get weather for a location', + oauth: false, + }, + { + name: 'sendMail_action_mail_example_com', + description: 'Send an email', + oauth: true, + }, + { + name: 'listItems_action_api_example_com', + description: 'List all items', + }, + ]; + + const mockGetActionToolDefinitions = jest.fn().mockResolvedValue(mockActionDefs); + + const params: LoadToolDefinitionsParams = { + userId: 'user-123', + agentId: 'agent-123', + tools: [ + 'getWeather_action_weather---com', + 'sendMail_action_mail---example---com', + 'listItems_action_api---example---com', + ], + }; + + const deps: LoadToolDefinitionsDeps = { + getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, + isBuiltInTool: mockIsBuiltInTool, + getActionToolDefinitions: mockGetActionToolDefinitions, + }; + + const result = await loadToolDefinitions(params, deps); + + expect(result.oauthActionToolNames).toEqual(['sendMail_action_mail_example_com']); + for (const def of result.toolDefinitions) { + expect(def).not.toHaveProperty('oauth'); + } + const registryEntry = result.toolRegistry.get('sendMail_action_mail_example_com'); + expect(registryEntry).toBeDefined(); + expect(registryEntry).not.toHaveProperty('oauth'); + }); + it('should not classify MCP tools with _action in name as action tools', async () => { const mockGetActionToolDefinitions = jest.fn(); const mcpTool = 'get_action_mcp_myserver'; diff --git a/packages/api/src/tools/definitions.ts b/packages/api/src/tools/definitions.ts index d4b5967b1d6..ea0cf9844d6 100644 --- a/packages/api/src/tools/definitions.ts +++ b/packages/api/src/tools/definitions.ts @@ -75,6 +75,9 @@ export interface ActionToolDefinition { name: string; description?: string; parameters?: JsonSchemaType; + /** True when the action authenticates via OAuth β€” its calls may block on an + * interactive login prompt, so it must never be dispatched in the background. */ + oauth?: boolean; } export interface LoadToolDefinitionsDeps { @@ -101,6 +104,8 @@ export interface LoadToolDefinitionsResult { expectedToolCount: number; resolvedToolCount: number; }; + /** Action tool names backed by OAuth β€” excluded from background dispatch. */ + oauthActionToolNames: string[]; } const mcpToolPattern = /_mcp_/; @@ -152,6 +157,7 @@ export async function loadToolDefinitions( hasDeferredTools: false, mcpToolAliases: [], mcpResolution: { expectedToolCount: 0, resolvedToolCount: 0 }, + oauthActionToolNames: [], }; if (!tools || tools.length === 0) { @@ -324,13 +330,19 @@ export async function loadToolDefinitions( } } + const oauthActionToolNames: string[] = []; if (actionToolNames.length > 0 && getActionToolDefinitions) { const fetchedActionDefs = await getActionToolDefinitions(agentId, actionToolNames); - actionToolDefs = fetchedActionDefs.map((def) => ({ - name: def.name, - description: def.description, - parameters: def.parameters, - })); + actionToolDefs = fetchedActionDefs.map((def) => { + if (def.oauth === true) { + oauthActionToolNames.push(def.name); + } + return { + name: def.name, + description: def.description, + parameters: def.parameters, + }; + }); } const loadedTools = mcpToolDefs.map((def) => ({ @@ -395,5 +407,6 @@ export async function loadToolDefinitions( expectedToolCount: expectedMCPToolCount, resolvedToolCount: resolvedMCPToolCount, }, + oauthActionToolNames, }; } diff --git a/packages/data-provider/src/agentToolOptions.spec.ts b/packages/data-provider/src/agentToolOptions.spec.ts index f0ce65814dd..be52b21b98b 100644 --- a/packages/data-provider/src/agentToolOptions.spec.ts +++ b/packages/data-provider/src/agentToolOptions.spec.ts @@ -1,5 +1,20 @@ import type { AgentToolOptions } from './types/assistants'; -import { removeCodeExecutionCaller } from './agentToolOptions'; +import { normalizeActionToolName, removeCodeExecutionCaller } from './agentToolOptions'; + +describe('normalizeActionToolName', () => { + it('normalizes only the encoded action domain', () => { + expect(normalizeActionToolName('get_foo---bar_action_swapi---tech')).toBe( + 'get_foo---bar_action_swapi_tech', + ); + }); + + it('leaves non-action tool names unchanged', () => { + expect(normalizeActionToolName('search_mcp_docs---server')).toBe('search_mcp_docs---server'); + expect(normalizeActionToolName('get_action_data---x_mcp_srv')).toBe( + 'get_action_data---x_mcp_srv', + ); + }); +}); describe('removeCodeExecutionCaller', () => { it('removes a programmatic-only entry that has no other options', () => { diff --git a/packages/data-provider/src/agentToolOptions.ts b/packages/data-provider/src/agentToolOptions.ts index a611b497046..269e27a7b7b 100644 --- a/packages/data-provider/src/agentToolOptions.ts +++ b/packages/data-provider/src/agentToolOptions.ts @@ -1,4 +1,26 @@ -import type { AgentToolOptions, AllowedCaller } from './types/assistants'; +import { + actionDelimiter, + actionDomainSeparator, + isActionTool, + type AgentToolOptions, + type AllowedCaller, +} from './types/assistants'; + +const actionDomainSeparatorRegex = new RegExp(actionDomainSeparator, 'g'); + +/** + * Collapses the encoded-domain suffix of an action tool name to the shape used + * by runtime tool definitions. The operation id is deliberately preserved. + */ +export function normalizeActionToolName(toolName: string): string { + if (!isActionTool(toolName)) { + return toolName; + } + const delimiterIndex = toolName.lastIndexOf(actionDelimiter); + const prefixEnd = delimiterIndex + actionDelimiter.length; + const encodedDomain = toolName.slice(prefixEnd); + return toolName.slice(0, prefixEnd) + encodedDomain.replace(actionDomainSeparatorRegex, '_'); +} /** * Removes Code Interpreter as an allowed caller without mutating the input. diff --git a/packages/data-provider/src/types/assistants.ts b/packages/data-provider/src/types/assistants.ts index 9d309086f08..764d8a0bf3a 100644 --- a/packages/data-provider/src/types/assistants.ts +++ b/packages/data-provider/src/types/assistants.ts @@ -359,6 +359,8 @@ export type Agent = { owner_contact?: AgentOwnerContact; /** Per-tool configuration options (deferred loading, allowed callers, etc.) */ tool_options?: AgentToolOptions; + /** Attached action registrations, each `${encodedDomain}${actionDelimiter}${action_id}` */ + actions?: string[]; /** Optional allowlist of skill ObjectIds. Only applies when `skills_enabled`. */ skills?: string[]; /** Master toggle for skill use on this agent. `true` = active (full catalog unless From 3d2da403cebf2e92da4b4617ae8bfdb629c3fcb9 Mon Sep 17 00:00:00 2001 From: Ravi Kumar L Date: Tue, 25 Aug 2026 14:18:17 +0200 Subject: [PATCH 06/14] =?UTF-8?q?=F0=9F=A5=9B=20fix:=20Drop=20Stale=20Save?= =?UTF-8?q?d=20Model=20Defaults=20in=20Builder=20Forms=20(#15179)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/common/types.ts | 1 + .../SidePanel/Agents/AgentPanel.test.tsx | 130 ++++++++++++-- .../SidePanel/Agents/AgentPanel.tsx | 68 ++++++- .../SidePanel/Agents/ModelPanel.test.tsx | 167 ++++++++++++++++++ .../SidePanel/Agents/ModelPanel.tsx | 50 +++--- .../SidePanel/Builder/AssistantPanel.tsx | 59 ++++++- .../SidePanel/Builder/AssistantSelect.tsx | 30 +--- client/src/utils/agentModelSelection.spec.ts | 62 +++++++ client/src/utils/agentModelSelection.ts | 32 ++++ client/src/utils/forms.spec.tsx | 4 - client/src/utils/forms.tsx | 8 +- client/src/utils/index.ts | 1 + 12 files changed, 533 insertions(+), 79 deletions(-) create mode 100644 client/src/components/SidePanel/Agents/ModelPanel.test.tsx create mode 100644 client/src/utils/agentModelSelection.spec.ts create mode 100644 client/src/utils/agentModelSelection.ts diff --git a/client/src/common/types.ts b/client/src/common/types.ts index 768386c50be..cd9c5a79626 100644 --- a/client/src/common/types.ts +++ b/client/src/common/types.ts @@ -227,6 +227,7 @@ export type AgentModelPanelProps = { agent_id?: string; providers: Option[]; models: Record; + modelsReady: boolean; setActivePanel: React.Dispatch>; }; diff --git a/client/src/components/SidePanel/Agents/AgentPanel.test.tsx b/client/src/components/SidePanel/Agents/AgentPanel.test.tsx index 213d94a10f0..74cbb14f64b 100644 --- a/client/src/components/SidePanel/Agents/AgentPanel.test.tsx +++ b/client/src/components/SidePanel/Agents/AgentPanel.test.tsx @@ -10,6 +10,20 @@ import type { AgentForm } from '~/common'; // Mock toast context - define this after all mocks let mockShowToast: jest.Mock; +let mockModelsQuery: { + data: Record; + isFetchedAfterMount: boolean; + isSuccess: boolean; + isFetching?: boolean; +} = { data: {}, isFetchedAfterMount: true, isSuccess: true }; +let mockAgentPanelContext = { + activePanel: 'builder', + agentsConfig: { allowedProviders: [] as string[] }, + setActivePanel: jest.fn(), + endpointsConfig: {}, + setCurrentAgentId: jest.fn(), + agent_id: 'agent-123' as string | undefined, +}; // Mock notification severity enum before other imports jest.mock('~/common/types', () => ({ @@ -77,7 +91,7 @@ jest.mock('@librechat/client', () => ({ // Mock other dependencies jest.mock('librechat-data-provider/react-query', () => ({ - useGetModelsQuery: () => ({ data: {} }), + useGetModelsQuery: () => mockModelsQuery, useGetEffectivePermissionsQuery: () => ({ data: { permissionBits: 0xffffffff }, // All permissions isLoading: false, @@ -87,6 +101,8 @@ jest.mock('librechat-data-provider/react-query', () => ({ jest.mock('~/utils', () => ({ createProviderOption: jest.fn((provider: string) => ({ value: provider, label: provider })), + getAvailableAgentSelection: jest.requireActual('~/utils/agentModelSelection') + .getAvailableAgentSelection, getDefaultAgentFormValues: jest.fn(() => ({ id: '', name: '', @@ -110,14 +126,7 @@ jest.mock('~/hooks/useResourcePermissions', () => ({ })); jest.mock('~/Providers/AgentPanelContext', () => ({ - useAgentPanelContext: () => ({ - activePanel: 'builder', - agentsConfig: { allowedProviders: [] }, - setActivePanel: jest.fn(), - endpointsConfig: {}, - setCurrentAgentId: jest.fn(), - agent_id: 'agent-123', - }), + useAgentPanelContext: () => mockAgentPanelContext, })); jest.mock('~/common', () => ({ @@ -202,7 +211,6 @@ jest.mock('react-hook-form', () => { }; }, FormProvider: ({ children }: any) => children, - useWatch: () => 'agent-123', }; }); @@ -300,9 +308,111 @@ describe('AgentPanel - Update Agent Toast Messages', () => { mockShowToast = jest.fn(); mockFormSubmitHandler = null; capturedFormMethods = null; + mockModelsQuery = { + data: { openai: ['gpt-4'] }, + isFetchedAfterMount: true, + isSuccess: true, + }; + mockAgentPanelContext = { + activePanel: 'builder', + agentsConfig: { allowedProviders: [] }, + setActivePanel: jest.fn(), + endpointsConfig: { openai: {} }, + setCurrentAgentId: jest.fn(), + agent_id: 'agent-123', + }; + localStorage.clear(); }); describe('AgentPanel', () => { + it('restores saved defaults from the current model catalogue', async () => { + const { mockUseGetAgentByIdQuery } = setupMocks(); + mockAgentQuery(mockUseGetAgentByIdQuery, {}); + mockAgentPanelContext = { + ...mockAgentPanelContext, + endpointsConfig: { custom: {} }, + agent_id: undefined, + }; + mockModelsQuery = { + data: { custom: ['cached-model'] }, + isFetchedAfterMount: true, + isSuccess: true, + isFetching: true, + }; + localStorage.setItem('lastAgentProvider', 'custom'); + localStorage.setItem('lastAgentModel', 'current-model'); + + const Wrapper = createWrapper(); + const { rerender } = render(, { wrapper: Wrapper }); + + mockModelsQuery = { + data: { custom: ['current-model'] }, + isFetchedAfterMount: true, + isSuccess: true, + isFetching: false, + }; + rerender(); + + await waitFor(() => { + expect(capturedFormMethods?.getValues('provider')).toEqual({ + value: 'custom', + label: 'custom', + }); + expect(capturedFormMethods?.getValues('model')).toBe('current-model'); + }); + }); + + it('clears unavailable saved defaults', async () => { + const { mockUseGetAgentByIdQuery } = setupMocks(); + mockAgentQuery(mockUseGetAgentByIdQuery, {}); + mockAgentPanelContext = { + ...mockAgentPanelContext, + endpointsConfig: { custom: {} }, + agent_id: undefined, + }; + mockModelsQuery = { + data: { custom: ['current-model'] }, + isFetchedAfterMount: true, + isSuccess: true, + }; + localStorage.setItem('lastAgentProvider', 'custom'); + localStorage.setItem('lastAgentModel', 'removed-model'); + + const Wrapper = createWrapper(); + render(, { wrapper: Wrapper }); + + await waitFor(() => { + expect(capturedFormMethods?.getValues('provider')).toEqual({ + value: 'custom', + label: 'custom', + }); + expect(capturedFormMethods?.getValues('model')).toBe(''); + }); + expect(localStorage.getItem('lastAgentModel')).toBeNull(); + }); + + it("preserves an existing agent's configured model", async () => { + const { mockUseGetAgentByIdQuery } = setupMocks(); + mockAgentQuery(mockUseGetAgentByIdQuery, {}); + mockAgentPanelContext = { + ...mockAgentPanelContext, + endpointsConfig: { bedrock: {} }, + }; + mockModelsQuery = { + data: { bedrock: ['current-model'] }, + isFetchedAfterMount: true, + isSuccess: true, + }; + + const Wrapper = createWrapper(); + render(, { wrapper: Wrapper }); + + await waitFor(() => { + expect(capturedFormMethods?.getValues('provider')).toBe('openai'); + expect(capturedFormMethods?.getValues('model')).toBe('gpt-4'); + }); + }); + it('should show "no changes" toast when version does not change', async () => { const { mockUseGetAgentByIdQuery, mockUpdateAgent } = setupMocks(); diff --git a/client/src/components/SidePanel/Agents/AgentPanel.tsx b/client/src/components/SidePanel/Agents/AgentPanel.tsx index 7f2517c278e..c6b501116f8 100644 --- a/client/src/components/SidePanel/Agents/AgentPanel.tsx +++ b/client/src/components/SidePanel/Agents/AgentPanel.tsx @@ -1,4 +1,4 @@ -import React, { useMemo, useCallback, useRef, useState } from 'react'; +import React, { useMemo, useCallback, useEffect, useRef, useState } from 'react'; import { Plus } from 'lucide-react'; import isEqual from 'lodash/isEqual'; import { Button, useToastContext } from '@librechat/client'; @@ -10,6 +10,7 @@ import { SystemRoles, ResourceType, EModelEndpoint, + LocalStorageKeys, PermissionBits, removeCodeExecutionCaller, resolveStatefulCodeEnvironment, @@ -26,7 +27,11 @@ import { useGetExpandedAgentByIdQuery, useUploadAgentAvatarMutation, } from '~/data-provider'; -import { createProviderOption, getDefaultAgentFormValues } from '~/utils'; +import { + createProviderOption, + getAvailableAgentSelection, + getDefaultAgentFormValues, +} from '~/utils'; import { useResourcePermissions } from '~/hooks/useResourcePermissions'; import { useSelectAgent, useLocalize, useAuthContext } from '~/hooks'; import { useAgentPanelContext } from '~/Providers/AgentPanelContext'; @@ -315,6 +320,7 @@ export default function AgentPanel() { const agentQuery = canEdit && expandedAgentQuery.data ? expandedAgentQuery : basicAgentQuery; const models = useMemo(() => modelsQuery.data ?? {}, [modelsQuery.data]); + const modelsReady = modelsQuery.isFetchedAfterMount && !modelsQuery.isFetching; const methods = useForm({ defaultValues: getDefaultAgentFormValues(defaultStatefulCodeEnvironment), mode: 'onChange', @@ -329,6 +335,7 @@ export default function AgentPanel() { formState: { dirtyFields }, } = methods; const [isAvatarUploadInFlight, setIsAvatarUploadInFlight] = useState(false); + const uploadAvatarMutation = useUploadAgentAvatarMutation({ onSuccess: (updatedAgent) => { showToast({ message: localize('com_ui_upload_agent_avatar') }); @@ -394,6 +401,56 @@ export default function AgentPanel() { .map((provider) => createProviderOption(provider)), [endpointsConfig, allowedProviders], ); + useEffect(() => { + if (endpointsConfig == null || !modelsReady || !modelsQuery.isSuccess) { + return; + } + + const storedProvider = localStorage.getItem(LocalStorageKeys.LAST_AGENT_PROVIDER) ?? ''; + const storedModel = localStorage.getItem(LocalStorageKeys.LAST_AGENT_MODEL) ?? ''; + const storedSelection = getAvailableAgentSelection({ + provider: storedProvider, + model: storedModel, + providers, + models, + }); + + if (storedSelection.provider !== storedProvider) { + localStorage.removeItem(LocalStorageKeys.LAST_AGENT_PROVIDER); + localStorage.removeItem(LocalStorageKeys.LAST_AGENT_MODEL); + } else if (storedSelection.model !== storedModel) { + localStorage.removeItem(LocalStorageKeys.LAST_AGENT_MODEL); + } + + if (current_agent_id || dirtyFields.provider === true || dirtyFields.model === true) { + return; + } + + const selectedProviderOption = getValues('provider'); + const selectedProvider = + (typeof selectedProviderOption === 'string' + ? selectedProviderOption + : (selectedProviderOption as StringOption | undefined)?.value) ?? ''; + const selectedModel = getValues('model') ?? ''; + + if (storedSelection.provider !== selectedProvider) { + setValue('provider', createProviderOption(storedSelection.provider)); + } + if (storedSelection.model !== selectedModel) { + setValue('model', storedSelection.model); + } + }, [ + current_agent_id, + dirtyFields.model, + dirtyFields.provider, + endpointsConfig, + getValues, + models, + modelsQuery.isSuccess, + modelsReady, + providers, + setValue, + ]); /* Mutations */ const update = useUpdateAgentMutation({ @@ -633,7 +690,12 @@ export default function AgentPanel() {
)} {canEditAgent && !agentQuery.isInitialLoading && activePanel === Panel.model && ( - + )} {canEditAgent && !agentQuery.isInitialLoading && activePanel === Panel.builder && ( diff --git a/client/src/components/SidePanel/Agents/ModelPanel.test.tsx b/client/src/components/SidePanel/Agents/ModelPanel.test.tsx new file mode 100644 index 00000000000..caf808b8446 --- /dev/null +++ b/client/src/components/SidePanel/Agents/ModelPanel.test.tsx @@ -0,0 +1,167 @@ +/** + * @jest-environment jsdom + */ +import React from 'react'; +import { FormProvider, useForm } from 'react-hook-form'; +import { fireEvent, render } from '@testing-library/react'; +import type { AgentForm } from '~/common'; +import ModelPanel from './ModelPanel'; + +jest.mock('@librechat/client', () => ({ + Button: ({ children, onClick, type }: React.ButtonHTMLAttributes) => ( + + ), + ControlCombobox: ({ + ariaLabel, + disabled, + items, + selectedValue, + setValue, + }: { + ariaLabel: string; + disabled?: boolean; + items: Array<{ label: string; value: string }>; + selectedValue: string; + setValue: (value: string) => void; + }) => ( +
+ {selectedValue} + {items.map((item) => ( + + ))} +
+ ), +})); + +jest.mock('~/components/SidePanel/Parameters/components', () => ({ + componentMapping: {}, +})); + +jest.mock('~/data-provider', () => ({ + useGetEndpointsQuery: () => ({ data: {} }), +})); + +jest.mock('~/Providers', () => ({ + useLiveAnnouncer: () => ({ announcePolite: jest.fn() }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('~/utils', () => ({ + cn: (...classes: Array) => classes.filter(Boolean).join(' '), +})); + +function TestForm({ + defaultModel = '', + defaultProvider = '', + models, + modelsReady, + providers = [{ label: 'Custom', value: 'custom' }], +}: { + defaultModel?: string; + defaultProvider?: string; + models: Record; + modelsReady: boolean; + providers?: Array<{ label: string; value: string }>; +}) { + const methods = useForm({ + defaultValues: { + provider: defaultProvider, + model: defaultModel, + model_parameters: {}, + }, + }); + + return ( + + + + ); +} + +describe('ModelPanel', () => { + beforeEach(() => localStorage.clear()); + + it('disables model selection until the model catalogue is ready', () => { + const { getByTestId } = render( + , + ); + + expect(getByTestId('com_ui_provider-custom')).toBeDisabled(); + expect(getByTestId('com_ui_model-custom-model')).toBeDisabled(); + }); + + it('selects and saves the first model when the provider changes', () => { + const providers = [ + { label: 'Original', value: 'original' }, + { label: 'Alternate', value: 'alternate' }, + ]; + const { getByTestId } = render( + , + ); + + fireEvent.click(getByTestId('com_ui_provider-alternate')); + + expect(getByTestId('com_ui_model-selected')).toHaveTextContent('alternate-model'); + expect(localStorage.getItem('lastAgentProvider')).toBe('alternate'); + expect(localStorage.getItem('lastAgentModel')).toBe('alternate-model'); + }); + + it('preserves the model when the current provider is selected again', () => { + const { getByTestId } = render( + , + ); + + fireEvent.click(getByTestId('com_ui_provider-custom')); + + expect(getByTestId('com_ui_model-selected')).toHaveTextContent('second-model'); + }); + + it('saves an explicitly selected model', () => { + const { getByTestId } = render( + , + ); + + fireEvent.click(getByTestId('com_ui_model-second-model')); + + expect(localStorage.getItem('lastAgentProvider')).toBe('custom'); + expect(localStorage.getItem('lastAgentModel')).toBe('second-model'); + }); +}); diff --git a/client/src/components/SidePanel/Agents/ModelPanel.tsx b/client/src/components/SidePanel/Agents/ModelPanel.tsx index a06b56f3f05..25aef6755cf 100644 --- a/client/src/components/SidePanel/Agents/ModelPanel.tsx +++ b/client/src/components/SidePanel/Agents/ModelPanel.tsx @@ -1,4 +1,4 @@ -import React, { useMemo, useEffect } from 'react'; +import React, { useMemo } from 'react'; import keyBy from 'lodash/keyBy'; import { ChevronLeft, RotateCcw } from 'lucide-react'; import { Button, ControlCombobox } from '@librechat/client'; @@ -25,7 +25,8 @@ export default function ModelPanel({ providers, setActivePanel, models: modelsData, -}: Pick) { + modelsReady, +}: Pick) { const localize = useLocalize(); const { announcePolite } = useLiveAnnouncer(); @@ -47,23 +48,6 @@ export default function ModelPanel({ [modelsData, provider], ); - useEffect(() => { - const _model = model ?? ''; - if (provider && _model) { - const modelExists = models.includes(_model); - if (!modelExists) { - const newModels = modelsData[provider] ?? []; - setValue('model', newModels[0] ?? ''); - } - localStorage.setItem(LocalStorageKeys.LAST_AGENT_MODEL, _model); - localStorage.setItem(LocalStorageKeys.LAST_AGENT_PROVIDER, provider); - } - - if (provider && !_model) { - setValue('model', models[0] ?? ''); - } - }, [provider, models, modelsData, setValue, model]); - const { data: endpointsConfig = {} } = useGetEndpointsQuery(); const bedrockRegions = useMemo(() => { @@ -150,13 +134,27 @@ export default function ModelPanel({ displayValue={alternateName[display] ?? display} selectPlaceholder={localize('com_ui_select_provider')} searchPlaceholder={localize('com_ui_select_search_provider')} - setValue={field.onChange} + setValue={(value) => { + if (value === provider) { + return; + } + const nextModel = modelsData[value]?.[0] ?? ''; + field.onChange(value); + setValue('model', nextModel); + localStorage.setItem(LocalStorageKeys.LAST_AGENT_PROVIDER, value); + if (nextModel) { + localStorage.setItem(LocalStorageKeys.LAST_AGENT_MODEL, nextModel); + } else { + localStorage.removeItem(LocalStorageKeys.LAST_AGENT_MODEL); + } + }} items={providers.map((provider) => ({ label: typeof provider === 'string' ? provider : provider.label, value: typeof provider === 'string' ? provider : provider.value, }))} className={cn(error ? 'border-2 border-red-500' : '')} ariaLabel={localize('com_ui_provider')} + disabled={!modelsReady} isCollapsed={false} showCarat={true} /> @@ -197,12 +195,20 @@ export default function ModelPanel({ : localize('com_ui_select_provider_first') } searchPlaceholder={localize('com_ui_select_model')} - setValue={field.onChange} + setValue={(value) => { + field.onChange(value); + localStorage.setItem(LocalStorageKeys.LAST_AGENT_PROVIDER, provider); + if (value) { + localStorage.setItem(LocalStorageKeys.LAST_AGENT_MODEL, value); + } else { + localStorage.removeItem(LocalStorageKeys.LAST_AGENT_MODEL); + } + }} items={models.map((model) => ({ label: model, value: model, }))} - disabled={!provider} + disabled={!provider || !modelsReady} className={cn('disabled:opacity-50', error ? 'border-2 border-red-500' : '')} ariaLabel={localize('com_ui_model')} isCollapsed={false} diff --git a/client/src/components/SidePanel/Builder/AssistantPanel.tsx b/client/src/components/SidePanel/Builder/AssistantPanel.tsx index d8d9e779021..67e47350531 100644 --- a/client/src/components/SidePanel/Builder/AssistantPanel.tsx +++ b/client/src/components/SidePanel/Builder/AssistantPanel.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from 'react'; +import { useState, useMemo, useEffect } from 'react'; import { useGetModelsQuery } from 'librechat-data-provider/react-query'; import { Spinner, useToastContext, SelectDropDown } from '@librechat/client'; import { useForm, FormProvider, Controller, useWatch } from 'react-hook-form'; @@ -7,19 +7,26 @@ import { Capabilities, isActionTool, ImageVisionTool, + LocalStorageKeys, defaultAssistantFormValues, } from 'librechat-data-provider'; import type { FunctionTool, TConfig } from 'librechat-data-provider'; -import type { AssistantForm, AssistantPanelProps } from '~/common'; +import type { AssistantForm, AssistantPanelProps, LastSelectedModels } from '~/common'; import { useCreateAssistantMutation, useUpdateAssistantMutation, useAvailableAgentToolsQuery, } from '~/data-provider'; -import { cn, cardStyle, defaultTextProps, removeFocusOutlines } from '~/utils'; +import { + cn, + cardStyle, + defaultTextProps, + getAvailableModelSelection, + removeFocusOutlines, +} from '~/utils'; import AssistantConversationStarters from './AssistantConversationStarters'; import AssistantToolsDialog from '~/components/Tools/AssistantToolsDialog'; -import { useSelectAssistant, useLocalize } from '~/hooks'; +import { useSelectAssistant, useLocalize, useLocalStorage } from '~/hooks'; import { useAssistantsMapContext } from '~/Providers'; import AppendDateCheckbox from './AppendDateCheckbox'; import CapabilitiesForm from './CapabilitiesForm'; @@ -50,7 +57,13 @@ export default function AssistantPanel({ assistantsConfig, version, }: AssistantPanelProps & { assistantsConfig?: TConfig | null }) { - const modelsQuery = useGetModelsQuery(); + const modelsQuery = useGetModelsQuery({ refetchOnMount: 'always' }); + const models = useMemo(() => modelsQuery.data?.[endpoint] ?? [], [endpoint, modelsQuery.data]); + const modelsReady = modelsQuery.isFetchedAfterMount && !modelsQuery.isFetching; + const [lastSelectedModels] = useLocalStorage( + LocalStorageKeys.LAST_MODEL, + {} as LastSelectedModels, + ); const assistantMap = useAssistantsMapContext(); const { data: allTools = [] } = useAvailableAgentToolsQuery(); @@ -64,10 +77,41 @@ export default function AssistantPanel({ const [showToolDialog, setShowToolDialog] = useState(false); - const { control, handleSubmit, reset, setValue, getValues } = methods; + const { + control, + handleSubmit, + reset, + setValue, + getValues, + formState: { dirtyFields }, + } = methods; const assistant = useWatch({ control, name: 'assistant' }); const functions = useWatch({ control, name: 'functions' }); const assistant_id = useWatch({ control, name: 'id' }); + const model = useWatch({ control, name: 'model' }); + + useEffect(() => { + if (!modelsReady || !modelsQuery.isSuccess || current_assistant_id || assistant_id) { + return; + } + + const candidate = dirtyFields.model === true ? model : (lastSelectedModels?.[endpoint] ?? ''); + const nextModel = getAvailableModelSelection(candidate, models); + if (nextModel !== model) { + setValue('model', nextModel, { shouldDirty: false }); + } + }, [ + assistant_id, + current_assistant_id, + dirtyFields.model, + endpoint, + lastSelectedModels, + model, + models, + modelsQuery.isSuccess, + modelsReady, + setValue, + ]); const activeModel = useMemo(() => { return assistantMap?.[endpoint]?.[assistant_id]?.model; @@ -363,7 +407,8 @@ export default function AssistantPanel({ emptyTitle={true} value={field.value} setValue={field.onChange} - availableValues={modelsQuery.data?.[endpoint] ?? []} + availableValues={models} + disabled={!modelsReady} showAbove={false} showLabel={false} className={cn( diff --git a/client/src/components/SidePanel/Builder/AssistantSelect.tsx b/client/src/components/SidePanel/Builder/AssistantSelect.tsx index 54310ed6e7c..44a39083353 100644 --- a/client/src/components/SidePanel/Builder/AssistantSelect.tsx +++ b/client/src/components/SidePanel/Builder/AssistantSelect.tsx @@ -6,7 +6,6 @@ import { FileSources, Capabilities, EModelEndpoint, - LocalStorageKeys, isImageVisionTool, defaultAssistantFormValues, } from 'librechat-data-provider'; @@ -19,17 +18,11 @@ import type { } from 'librechat-data-provider'; import type { UseMutationResult } from '@tanstack/react-query'; import type { UseFormReset } from 'react-hook-form'; -import type { - Actions, - ExtendedFile, - AssistantForm, - TAssistantOption, - LastSelectedModels, -} from '~/common'; +import type { Actions, ExtendedFile, AssistantForm, TAssistantOption } from '~/common'; import { useListAssistantsQuery } from '~/data-provider'; -import { useLocalize, useLocalStorage } from '~/hooks'; import { cn, createDropdownSetter } from '~/utils'; import { useFileMapContext } from '~/Providers'; +import { useLocalize } from '~/hooks'; const keys = new Set([ 'name', @@ -63,10 +56,6 @@ export default function AssistantSelect({ const localize = useLocalize(); const fileMap = useFileMapContext(); const lastSelectedAssistant = useRef(null); - const [lastSelectedModels] = useLocalStorage( - LocalStorageKeys.LAST_MODEL, - {} as LastSelectedModels, - ); const toolkits = useMemo( () => new Set(allTools?.filter((tool) => tool.toolkit === true).map((tool) => tool.pluginKey)), @@ -152,10 +141,7 @@ export default function AssistantSelect({ createMutation.reset(); if (!assistant) { setCurrentAssistantId(undefined); - return reset({ - ...defaultAssistantFormValues, - model: lastSelectedModels?.[endpoint] ?? '', - }); + return reset(defaultAssistantFormValues); } const update = { @@ -231,15 +217,7 @@ export default function AssistantSelect({ reset(formValues); setCurrentAssistantId(assistant.id); }, - [ - query.data, - reset, - setCurrentAssistantId, - createMutation, - endpoint, - lastSelectedModels, - toolkits, - ], + [query.data, reset, setCurrentAssistantId, createMutation, toolkits], ); useEffect(() => { diff --git a/client/src/utils/agentModelSelection.spec.ts b/client/src/utils/agentModelSelection.spec.ts new file mode 100644 index 00000000000..0340152aaeb --- /dev/null +++ b/client/src/utils/agentModelSelection.spec.ts @@ -0,0 +1,62 @@ +import { getAvailableAgentSelection, getAvailableModelSelection } from './agentModelSelection'; + +describe('getAvailableModelSelection', () => { + it('returns an empty value when a saved model is unavailable', () => { + expect(getAvailableModelSelection('gpt-removed', ['gpt-4.1'])).toBe(''); + }); +}); + +describe('getAvailableAgentSelection', () => { + const providers = [ + { label: 'Anthropic', value: 'anthropic' }, + { label: 'Bedrock', value: 'bedrock' }, + ]; + const models = { + anthropic: ['claude-sonnet-4'], + bedrock: ['claude-sonnet-4', 'claude-haiku-3'], + }; + + it('keeps an available provider and model', () => { + expect( + getAvailableAgentSelection({ + provider: 'bedrock', + model: 'claude-sonnet-4', + providers, + models, + }), + ).toEqual({ provider: 'bedrock', model: 'claude-sonnet-4' }); + }); + + it('returns an empty selection when the provider is unavailable', () => { + expect( + getAvailableAgentSelection({ + provider: 'openAI', + model: 'gpt-5', + providers, + models, + }), + ).toEqual({ provider: '', model: '' }); + }); + + it('returns an empty selection when the provider has no model catalogue', () => { + expect( + getAvailableAgentSelection({ + provider: 'anthropic', + model: 'claude-sonnet-4', + providers, + models: { bedrock: models.bedrock }, + }), + ).toEqual({ provider: '', model: '' }); + }); + + it('keeps the provider but clears an unavailable model', () => { + expect( + getAvailableAgentSelection({ + provider: 'bedrock', + model: 'claude-opus-3', + providers, + models, + }), + ).toEqual({ provider: 'bedrock', model: '' }); + }); +}); diff --git a/client/src/utils/agentModelSelection.ts b/client/src/utils/agentModelSelection.ts new file mode 100644 index 00000000000..067d2196802 --- /dev/null +++ b/client/src/utils/agentModelSelection.ts @@ -0,0 +1,32 @@ +type ProviderOption = string | { value?: string | number | null }; + +export function getAvailableModelSelection(model: string, models: readonly string[]): string { + return models.includes(model) ? model : ''; +} + +export function getAvailableAgentSelection({ + provider, + model, + providers, + models, +}: { + provider: string; + model: string; + providers: readonly ProviderOption[]; + models: Record; +}): { provider: string; model: string } { + const providerExists = + models[provider] != null && + providers.some((option) => + typeof option === 'string' ? option === provider : option.value === provider, + ); + + if (!providerExists) { + return { provider: '', model: '' }; + } + + return { + provider, + model: getAvailableModelSelection(model, models[provider] ?? []), + }; +} diff --git a/client/src/utils/forms.spec.tsx b/client/src/utils/forms.spec.tsx index ff70e1a2928..477c3c5f2e0 100644 --- a/client/src/utils/forms.spec.tsx +++ b/client/src/utils/forms.spec.tsx @@ -1,10 +1,6 @@ import { getDefaultAgentFormValues } from './forms'; describe('getDefaultAgentFormValues', () => { - beforeEach(() => { - localStorage.clear(); - }); - it('uses the scalable user workspace by default', () => { expect(getDefaultAgentFormValues().stateful_code_environment).toBe('user'); }); diff --git a/client/src/utils/forms.tsx b/client/src/utils/forms.tsx index 746d4659f7e..8d019cea32f 100644 --- a/client/src/utils/forms.tsx +++ b/client/src/utils/forms.tsx @@ -4,7 +4,6 @@ import { alternateName, EModelEndpoint, EToolResources, - LocalStorageKeys, defaultAgentFormValues, } from 'librechat-data-provider'; import type { Agent, TFile, StatefulCodeEnvironment } from 'librechat-data-provider'; @@ -44,17 +43,12 @@ export const createProviderOption = (provider: string) => ({ value: provider, }); -/** - * Gets default agent form values with localStorage values for model and provider. - * This is used to initialize agent forms with the last used model and provider. - **/ +/** Gets default agent form values. */ export const getDefaultAgentFormValues = ( statefulCodeEnvironment: StatefulCodeEnvironment = 'user', ) => ({ ...defaultAgentFormValues, stateful_code_environment: statefulCodeEnvironment, - model: localStorage.getItem(LocalStorageKeys.LAST_AGENT_MODEL) ?? '', - provider: createProviderOption(localStorage.getItem(LocalStorageKeys.LAST_AGENT_PROVIDER) ?? ''), avatar_file: null, avatar_preview: '', avatar_action: null, diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index 77d93e4fec8..5a42efa8048 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -44,6 +44,7 @@ export * from './favoritesError'; export * from './approval'; export * from './steer'; export * from './activityLabels'; +export * from './agentModelSelection'; export * from './runStepDuration'; export * from './toolCallPhase'; export * from './documentTitle'; From 2ef12b1e1d7d7674d12a7ba1776e32ecedc460e5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 08:21:39 -0400 Subject: [PATCH 07/14] =?UTF-8?q?=F0=9F=A6=BA=20feat:=20Configurable=20Bas?= =?UTF-8?q?eline=20HTTP=20Security=20Headers=20(#14445)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds helmet's CSP-independent headers (HSTS, X-Frame-Options, X-Content-Type-Options, COOP, CORP, Referrer-Policy) on every response, with contentSecurityPolicy explicitly disabled. Every header that can break a deployment is configurable, so there is no allow-list to go stale the way #7377's hardcoded CSP directives did. HSTS includeSubDomains defaults off rather than matching helmet's on-by-default: it would otherwise pin every sibling subdomain to HTTPS for a year in every visitor's browser, and undoing that requires serving max-age=0 from each affected host. --- .env.example | 32 ++++ api/server/experimental.js | 7 + api/server/index.js | 7 + api/server/index.spec.js | 37 ++++ package-lock.json | 13 ++ packages/api/package.json | 1 + packages/api/src/index.ts | 2 + packages/api/src/security/headers.spec.ts | 134 +++++++++++++++ packages/api/src/security/headers.ts | 197 ++++++++++++++++++++++ packages/api/src/security/index.ts | 1 + 10 files changed, 431 insertions(+) create mode 100644 packages/api/src/security/headers.spec.ts create mode 100644 packages/api/src/security/headers.ts create mode 100644 packages/api/src/security/index.ts diff --git a/.env.example b/.env.example index eb7350a93e2..8fdf932ddcf 100644 --- a/.env.example +++ b/.env.example @@ -73,6 +73,38 @@ NO_INDEX=true # Defaulted to 1. TRUST_PROXY=1 +#===============================# +# Security Headers # +#===============================# + +# Baseline HTTP security headers (HSTS, X-Frame-Options, X-Content-Type-Options, +# COOP, CORP, Referrer-Policy) are sent on every response. Content-Security-Policy +# is never set here. Set to false to send no security headers at all. +# SECURITY_HEADERS=true + +# Strict-Transport-Security. Only meaningful over HTTPS; browsers ignore it on +# plain HTTP. HSTS_INCLUDE_SUBDOMAINS applies the policy to every subdomain of +# this host for the full max-age, so enable it only if all of them serve HTTPS. +# HSTS_ENABLED=true +# HSTS_MAX_AGE=31536000 +# HSTS_INCLUDE_SUBDOMAINS=false +# HSTS_PRELOAD=false + +# X-Frame-Options. Set to DENY to block all framing, or to `off` if you embed +# LibreChat in an iframe on another origin. +# X_FRAME_OPTIONS=SAMEORIGIN + +# Referrer-Policy. Any standard token, or `off` to omit the header. +# REFERRER_POLICY=no-referrer + +# Cross-Origin-Opener-Policy. Use same-origin-allow-popups if a popup-based +# sign-in flow needs to reach back to the window that opened it. +# CROSS_ORIGIN_OPENER_POLICY=same-origin + +# Cross-Origin-Resource-Policy. Use cross-origin if other sites need to load +# resources served by LibreChat, such as uploaded images. +# CROSS_ORIGIN_RESOURCE_POLICY=same-origin + # Trust X-Tenant-Id on unauthenticated routes. Disabled by default. # Enable only when a trusted reverse proxy strips any client-supplied value and sets its own. # TRUST_TENANT_HEADER=false diff --git a/api/server/experimental.js b/api/server/experimental.js index 8d03f8ae59e..1885235ca7d 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -17,6 +17,7 @@ const { apiNotFound, ErrorController, QUERY_DEVTOOLS_HEADER, + createSecurityHeaders, performStartupChecks, handleJsonParseError, initializeFileStorage, @@ -353,6 +354,12 @@ if (cluster.isMaster) { app.disable('x-powered-by'); app.set('trust proxy', trusted_proxy); + /* Registered ahead of every route so health checks carry the headers too. */ + const securityHeaders = createSecurityHeaders(); + if (securityHeaders) { + app.use(securityHeaders); + } + if (isEnabled(process.env.TRUST_TENANT_HEADER)) { logger.warn( '[Security] TRUST_TENANT_HEADER is active. Ensure your reverse proxy strips and sets ' + diff --git a/api/server/index.js b/api/server/index.js index 1c4a91244f5..68d37111e79 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -18,6 +18,7 @@ const { createMetrics, ErrorController, memoryDiagnostics, + createSecurityHeaders, performStartupChecks, handleJsonParseError, GenerationJobManager, @@ -155,6 +156,12 @@ const startServer = async () => { app.disable('x-powered-by'); app.set('trust proxy', trusted_proxy); + /* Registered ahead of every route so health checks carry the headers too. */ + const securityHeaders = createSecurityHeaders(); + if (securityHeaders) { + app.use(securityHeaders); + } + if (isEnabled(process.env.TRUST_TENANT_HEADER)) { logger.warn( '[Security] TRUST_TENANT_HEADER is active. Ensure your reverse proxy strips and sets ' + diff --git a/api/server/index.spec.js b/api/server/index.spec.js index 73ad042865e..adb8a0359a5 100644 --- a/api/server/index.spec.js +++ b/api/server/index.spec.js @@ -170,6 +170,22 @@ describe('Startup readiness wiring', () => { expect(timeoutConfigIndex).toBeLessThan(shutdownIndex); }); + it('registers security headers ahead of the health endpoints in both server entries', () => { + const experimental = fs.readFileSync(path.join(__dirname, 'experimental.js'), 'utf8'); + + for (const [name, contents] of [ + ['index.js', source], + ['experimental.js', experimental], + ]) { + const headersIndex = contents.indexOf('const securityHeaders = createSecurityHeaders();'); + const healthIndex = contents.indexOf("app.get('/health'"); + + expect([name, headersIndex > -1]).toEqual([name, true]); + expect([name, healthIndex > -1]).toEqual([name, true]); + expect([name, headersIndex < healthIndex]).toEqual([name, true]); + } + }); + it('mounts the chat-start readiness gate before agent routes', () => { const readinessGateIndex = source.indexOf( "app.use('/api/agents/chat', rejectChatStartsUntilReady);", @@ -250,6 +266,27 @@ describe('Server Configuration', () => { expect(response.text).toBe('OK'); }); + it('should set baseline security headers on health checks', async () => { + const response = await request(app).get('/health'); + + expect(response.headers['strict-transport-security']).toBe('max-age=31536000'); + expect(response.headers['x-frame-options']).toBe('SAMEORIGIN'); + expect(response.headers['x-content-type-options']).toBe('nosniff'); + expect(response.headers['cross-origin-opener-policy']).toBe('same-origin'); + expect(response.headers['cross-origin-resource-policy']).toBe('same-origin'); + expect(response.headers['referrer-policy']).toBe('no-referrer'); + }); + + it('should set baseline security headers on the index page without a CSP', async () => { + const response = await request(app).get('/'); + + expect(response.status).toBe(200); + expect(response.headers['x-frame-options']).toBe('SAMEORIGIN'); + expect(response.headers['x-content-type-options']).toBe('nosniff'); + expect(response.headers['content-security-policy']).toBeUndefined(); + expect(response.headers['content-security-policy-report-only']).toBeUndefined(); + }); + it('should not cache index page', async () => { const response = await request(app).get('/'); expect(response.status).toBe(200); diff --git a/package-lock.json b/package-lock.json index 0b0a01e35dc..5574f1fdf6f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27506,6 +27506,18 @@ "integrity": "sha512-CxJE27BF6JcQvrL1giK478iSZr7EJNTnAN2Th1rAJiN1BSMYZxDLm4PL/p/ha3aSqVHvCo+YNk++5tIj0JVxLQ==", "license": "LGPL-3.0" }, + "node_modules/helmet": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/helmet/-/helmet-8.3.0.tgz", + "integrity": "sha512-Qgpiaws3Sm30Av8Eah6sjMCZZwjlBu+E68rhpCWBshY1lb09HtLwj5GviX0OyQIn+ulUS0iX0AxN5n3tLZzz1w==", + "license": "MIT", + "engines": { + "node": ">=18.0.0" + }, + "funding": { + "url": "https://github.com/sponsors/EvanHahn" + } + }, "node_modules/highlight.js": { "version": "11.8.0", "resolved": "https://registry.npmjs.org/highlight.js/-/highlight.js-11.8.0.tgz", @@ -42771,6 +42783,7 @@ "@langchain/langgraph-checkpoint-mongodb": "^1.4.0", "cluster-key-slot": "^1.1.2", "croner": "^10.0.1", + "helmet": "^8.3.0", "proxy-from-env": "^2.1.0", "re2js": "^2.8.6" }, diff --git a/packages/api/package.json b/packages/api/package.json index 990268d677a..0cb25628af4 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -171,6 +171,7 @@ "@langchain/langgraph-checkpoint-mongodb": "^1.4.0", "cluster-key-slot": "^1.1.2", "croner": "^10.0.1", + "helmet": "^8.3.0", "proxy-from-env": "^2.1.0", "re2js": "^2.8.6" } diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index dcac12cda02..febecf54052 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -43,6 +43,8 @@ export * from './crypto'; export * from './flow/manager'; /* Middleware */ export * from './middleware'; +/* Security */ +export * from './security'; /* Content protection */ export * from './protection'; /* Imports */ diff --git a/packages/api/src/security/headers.spec.ts b/packages/api/src/security/headers.spec.ts new file mode 100644 index 00000000000..51ad3b1b692 --- /dev/null +++ b/packages/api/src/security/headers.spec.ts @@ -0,0 +1,134 @@ +import express from 'express'; +import request from 'supertest'; + +import type { Express } from 'express'; + +import { buildSecurityHeaderOptions, createSecurityHeaders } from './headers'; + +function appWith(env: NodeJS.ProcessEnv): Express { + const app = express(); + const securityHeaders = createSecurityHeaders(env); + if (securityHeaders) { + app.use(securityHeaders); + } + app.get('/health', (_req, res) => { + res.status(200).send('OK'); + }); + return app; +} + +describe('buildSecurityHeaderOptions', () => { + it('always disables CSP so no directive allow-list can go stale', () => { + expect(buildSecurityHeaderOptions({})?.contentSecurityPolicy).toBe(false); + expect( + buildSecurityHeaderOptions({ CONTENT_SECURITY_POLICY: 'true' })?.contentSecurityPolicy, + ).toBe(false); + }); + + it('returns null when disabled outright', () => { + expect(buildSecurityHeaderOptions({ SECURITY_HEADERS: 'false' })).toBeNull(); + expect(buildSecurityHeaderOptions({ SECURITY_HEADERS: 'off' })).toBeNull(); + expect(createSecurityHeaders({ SECURITY_HEADERS: 'false' })).toBeNull(); + }); + + it('leaves HSTS includeSubDomains off unless opted in', () => { + expect(buildSecurityHeaderOptions({})?.hsts).toEqual({ + maxAge: 31536000, + includeSubDomains: false, + preload: false, + }); + expect(buildSecurityHeaderOptions({ HSTS_INCLUDE_SUBDOMAINS: 'true' })?.hsts).toMatchObject({ + includeSubDomains: true, + }); + }); + + it('falls back to defaults for unparseable values', () => { + const options = buildSecurityHeaderOptions({ + HSTS_MAX_AGE: 'forever', + X_FRAME_OPTIONS: 'ALLOW-FROM https://portal.example.com', + REFERRER_POLICY: 'whatever', + SECURITY_HEADERS: 'maybe', + }); + + expect(options?.hsts).toMatchObject({ maxAge: 31536000 }); + expect(options?.frameguard).toEqual({ action: 'sameorigin' }); + expect(options?.referrerPolicy).toEqual({ policy: 'no-referrer' }); + }); + + it('disables individual headers without disabling the rest', () => { + const options = buildSecurityHeaderOptions({ + HSTS_ENABLED: 'false', + X_FRAME_OPTIONS: 'off', + CROSS_ORIGIN_RESOURCE_POLICY: 'false', + }); + + expect(options?.hsts).toBe(false); + expect(options?.frameguard).toBe(false); + expect(options?.crossOriginResourcePolicy).toBe(false); + expect(options?.crossOriginOpenerPolicy).toEqual({ policy: 'same-origin' }); + expect(options?.referrerPolicy).toEqual({ policy: 'no-referrer' }); + }); +}); + +describe('createSecurityHeaders', () => { + it('sets the baseline headers and never sets CSP', async () => { + const response = await request(appWith({})).get('/health'); + + expect(response.status).toBe(200); + expect(response.headers['strict-transport-security']).toBe('max-age=31536000'); + expect(response.headers['x-frame-options']).toBe('SAMEORIGIN'); + expect(response.headers['x-content-type-options']).toBe('nosniff'); + expect(response.headers['cross-origin-opener-policy']).toBe('same-origin'); + expect(response.headers['cross-origin-resource-policy']).toBe('same-origin'); + expect(response.headers['referrer-policy']).toBe('no-referrer'); + expect(response.headers['origin-agent-cluster']).toBe('?1'); + expect(response.headers['content-security-policy']).toBeUndefined(); + expect(response.headers['content-security-policy-report-only']).toBeUndefined(); + }); + + it('honors per-header overrides', async () => { + const response = await request( + appWith({ + HSTS_MAX_AGE: '600', + HSTS_INCLUDE_SUBDOMAINS: 'true', + HSTS_PRELOAD: 'true', + X_FRAME_OPTIONS: 'DENY', + CROSS_ORIGIN_RESOURCE_POLICY: 'cross-origin', + CROSS_ORIGIN_OPENER_POLICY: 'same-origin-allow-popups', + REFERRER_POLICY: 'strict-origin-when-cross-origin', + }), + ).get('/health'); + + expect(response.headers['strict-transport-security']).toBe( + 'max-age=600; includeSubDomains; preload', + ); + expect(response.headers['x-frame-options']).toBe('DENY'); + expect(response.headers['cross-origin-resource-policy']).toBe('cross-origin'); + expect(response.headers['cross-origin-opener-policy']).toBe('same-origin-allow-popups'); + expect(response.headers['referrer-policy']).toBe('strict-origin-when-cross-origin'); + }); + + it('omits headers the operator turned off', async () => { + const response = await request( + appWith({ + HSTS_ENABLED: 'false', + X_FRAME_OPTIONS: 'off', + CROSS_ORIGIN_RESOURCE_POLICY: 'off', + }), + ).get('/health'); + + expect(response.headers['strict-transport-security']).toBeUndefined(); + expect(response.headers['x-frame-options']).toBeUndefined(); + expect(response.headers['cross-origin-resource-policy']).toBeUndefined(); + expect(response.headers['x-content-type-options']).toBe('nosniff'); + }); + + it('sets no headers at all when SECURITY_HEADERS is false', async () => { + const response = await request(appWith({ SECURITY_HEADERS: 'false' })).get('/health'); + + expect(response.status).toBe(200); + expect(response.headers['x-content-type-options']).toBeUndefined(); + expect(response.headers['x-frame-options']).toBeUndefined(); + expect(response.headers['strict-transport-security']).toBeUndefined(); + }); +}); diff --git a/packages/api/src/security/headers.ts b/packages/api/src/security/headers.ts new file mode 100644 index 00000000000..4453e1e28c7 --- /dev/null +++ b/packages/api/src/security/headers.ts @@ -0,0 +1,197 @@ +import helmet from 'helmet'; +import { logger } from '@librechat/data-schemas'; + +import type { RequestHandler } from 'express'; + +const DEFAULT_HSTS_MAX_AGE = 31536000; + +export type FrameOptionsAction = 'deny' | 'sameorigin'; +export type OpenerPolicy = + | 'same-origin' + | 'same-origin-allow-popups' + | 'noopener-allow-popups' + | 'unsafe-none'; +export type ResourcePolicy = 'same-origin' | 'same-site' | 'cross-origin'; +export type ReferrerPolicyToken = + | 'no-referrer' + | 'no-referrer-when-downgrade' + | 'same-origin' + | 'origin' + | 'strict-origin' + | 'origin-when-cross-origin' + | 'strict-origin-when-cross-origin' + | 'unsafe-url'; + +export interface HstsOptions { + maxAge: number; + includeSubDomains: boolean; + preload: boolean; +} + +export interface SecurityHeaderOptions { + contentSecurityPolicy: false; + hsts: HstsOptions | false; + frameguard: { action: FrameOptionsAction } | false; + crossOriginOpenerPolicy: { policy: OpenerPolicy } | false; + crossOriginResourcePolicy: { policy: ResourcePolicy } | false; + referrerPolicy: { policy: ReferrerPolicyToken } | false; +} + +const TRUTHY = new Set(['true', '1', 'yes', 'on', 'enabled']); +const FALSY = new Set(['false', '0', 'no', 'off', 'disabled', 'none']); + +const FRAME_ACTIONS = new Set(['deny', 'sameorigin']); +const OPENER_POLICIES = new Set([ + 'same-origin', + 'same-origin-allow-popups', + 'noopener-allow-popups', + 'unsafe-none', +]); +const RESOURCE_POLICIES = new Set(['same-origin', 'same-site', 'cross-origin']); +const REFERRER_TOKENS = new Set([ + 'no-referrer', + 'no-referrer-when-downgrade', + 'same-origin', + 'origin', + 'strict-origin', + 'origin-when-cross-origin', + 'strict-origin-when-cross-origin', + 'unsafe-url', +]); + +function normalize(value: string | undefined): string { + return value == null ? '' : value.trim().toLowerCase(); +} + +function parseSwitch(name: string, value: string | undefined, fallback: boolean): boolean { + const normalized = normalize(value); + if (normalized === '') { + return fallback; + } + if (TRUTHY.has(normalized)) { + return true; + } + if (FALSY.has(normalized)) { + return false; + } + logger.warn(`[SecurityHeaders] Ignoring invalid ${name}="${value}"; using ${fallback}.`); + return fallback; +} + +function parseMaxAge(value: string | undefined, fallback: number): number { + const normalized = normalize(value); + if (normalized === '') { + return fallback; + } + const parsed = Number(normalized); + if (!Number.isInteger(parsed) || parsed < 0) { + logger.warn(`[SecurityHeaders] Ignoring invalid HSTS_MAX_AGE="${value}"; using ${fallback}.`); + return fallback; + } + return parsed; +} + +/** + * Resolves a header that is either disabled outright or set to one of a fixed + * set of policy tokens. Returns `false` when the operator disabled it. + */ +function parsePolicy( + name: string, + value: string | undefined, + allowed: ReadonlySet, + fallback: T, +): T | false { + const normalized = normalize(value); + if (normalized === '') { + return fallback; + } + if (FALSY.has(normalized)) { + return false; + } + if (allowed.has(normalized as T)) { + return normalized as T; + } + logger.warn(`[SecurityHeaders] Ignoring invalid ${name}="${value}"; using "${fallback}".`); + return fallback; +} + +function buildHsts(env: NodeJS.ProcessEnv): HstsOptions | false { + if (!parseSwitch('HSTS_ENABLED', env.HSTS_ENABLED, true)) { + return false; + } + return { + maxAge: parseMaxAge(env.HSTS_MAX_AGE, DEFAULT_HSTS_MAX_AGE), + /* Opt-in rather than helmet's on-by-default: a bare-domain or `chat.example.com` + * deployment would otherwise pin every sibling subdomain to HTTPS for a year in + * every visitor's browser, and reversing that means serving `max-age=0` from each + * affected host. */ + includeSubDomains: parseSwitch('HSTS_INCLUDE_SUBDOMAINS', env.HSTS_INCLUDE_SUBDOMAINS, false), + preload: parseSwitch('HSTS_PRELOAD', env.HSTS_PRELOAD, false), + }; +} + +/** + * Builds helmet options from the environment. CSP is always disabled here so the + * CSP-independent headers never depend on a directive allow-list staying current + * with whichever optional features a deployment has enabled. + */ +export function buildSecurityHeaderOptions( + env: NodeJS.ProcessEnv = process.env, +): SecurityHeaderOptions | null { + if (!parseSwitch('SECURITY_HEADERS', env.SECURITY_HEADERS, true)) { + return null; + } + + const frameAction = parsePolicy( + 'X_FRAME_OPTIONS', + env.X_FRAME_OPTIONS, + FRAME_ACTIONS, + 'sameorigin', + ); + const openerPolicy = parsePolicy( + 'CROSS_ORIGIN_OPENER_POLICY', + env.CROSS_ORIGIN_OPENER_POLICY, + OPENER_POLICIES, + 'same-origin', + ); + const resourcePolicy = parsePolicy( + 'CROSS_ORIGIN_RESOURCE_POLICY', + env.CROSS_ORIGIN_RESOURCE_POLICY, + RESOURCE_POLICIES, + 'same-origin', + ); + const referrerToken = parsePolicy( + 'REFERRER_POLICY', + env.REFERRER_POLICY, + REFERRER_TOKENS, + 'no-referrer', + ); + + return { + contentSecurityPolicy: false, + hsts: buildHsts(env), + frameguard: frameAction === false ? false : { action: frameAction }, + crossOriginOpenerPolicy: openerPolicy === false ? false : { policy: openerPolicy }, + crossOriginResourcePolicy: resourcePolicy === false ? false : { policy: resourcePolicy }, + referrerPolicy: referrerToken === false ? false : { policy: referrerToken }, + }; +} + +/** + * Creates the baseline security-header middleware, or `null` when the operator + * disabled it via `SECURITY_HEADERS=false`. + * + * @example + * const securityHeaders = createSecurityHeaders(); + * if (securityHeaders) { + * app.use(securityHeaders); + * } + */ +export function createSecurityHeaders(env: NodeJS.ProcessEnv = process.env): RequestHandler | null { + const options = buildSecurityHeaderOptions(env); + if (!options) { + logger.warn('[SecurityHeaders] Disabled via SECURITY_HEADERS; no baseline headers are set.'); + return null; + } + return helmet(options) as RequestHandler; +} diff --git a/packages/api/src/security/index.ts b/packages/api/src/security/index.ts new file mode 100644 index 00000000000..356854ae0c9 --- /dev/null +++ b/packages/api/src/security/index.ts @@ -0,0 +1 @@ +export * from './headers'; From 16dd677be42b8b29c2f6a19f6b434224a8c774aa Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 08:29:42 -0400 Subject: [PATCH 08/14] =?UTF-8?q?=F0=9F=8E=A8=20style:=20Set=20Question=20?= =?UTF-8?q?Popover=20and=20Subagent=20Panel=20on=20the=20Sidebar=20Surface?= =?UTF-8?q?=20(#15201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both floated over the chat on surfaces one step too close to it β€” the question popover on surface-secondary, the subagent thread panel on the chat's own surface-primary. Both now use surface-primary-alt, the conversation-list sidebar's role, verified in the running app: popover, panel, and sidebar all resolve to the same computed background in dark (rgb 23,23,23) and light (rgb 247,247,248). Inline question cards keep surface-secondary deliberately β€” that is the tool-record family's surface, and settled questions collapse into that family. --- client/src/components/Chat/Input/AskUserQuestionPopover.tsx | 4 ++-- client/src/components/Chat/Subagents/SubagentThreadPanel.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx index d6568da1af3..66c73e26543 100644 --- a/client/src/components/Chat/Input/AskUserQuestionPopover.tsx +++ b/client/src/components/Chat/Input/AskUserQuestionPopover.tsx @@ -51,7 +51,7 @@ function AskUserQuestionsPopoverPanel({ ask }: { ask: ReturnType -
+

{localize( @@ -156,7 +156,7 @@ function AskUserQuestionPopoverPanel({ scroll region: the panel is absolutely positioned, so anything that overflows it is unreachable by page scroll. */}

diff --git a/client/src/components/Chat/Subagents/SubagentThreadPanel.tsx b/client/src/components/Chat/Subagents/SubagentThreadPanel.tsx index 59b1ec8755b..599a7c2012f 100644 --- a/client/src/components/Chat/Subagents/SubagentThreadPanel.tsx +++ b/client/src/components/Chat/Subagents/SubagentThreadPanel.tsx @@ -585,7 +585,7 @@ export default function SubagentThreadPanel({ selection }: { selection: ActiveSu role={isMobile ? 'dialog' : 'region'} aria-modal={isMobile || undefined} aria-label={localize('com_ui_subagent_thread_panel')} - className="flex h-full w-full flex-col overflow-hidden bg-surface-primary text-text-primary" + className="flex h-full w-full flex-col overflow-hidden bg-surface-primary-alt text-text-primary" >
From 018775de07564467822c5732ee2615930c527094 Mon Sep 17 00:00:00 2001 From: James Todaro <30529065+jtodaroii@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:30:12 -0400 Subject: [PATCH 09/14] =?UTF-8?q?=F0=9F=A7=BE=20fix:=20Honor=20Disabled=20?= =?UTF-8?q?Transactions=20on=20the=20Assistants=20Usage=20Path=20(#15100)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🧾 fix: Honor Disabled Transactions on the Assistants Usage Path Thread the resolved transactions config through `recordUsage` from each of its callers, so `transactions.enabled: false` is honored on the assistants token spend path. * 🧾 fix: Thread the transactions config through the vision-request caller Address review: `ToolService.processVisionRequest` also records usage without the resolved config, and `recordUsage`'s documented return type did not match the function. * 🧾 fix: Set the resolved transactions config after the usage spread - provider usage could carry a `transactions` key that overwrote the trusted value - matches the ordering the other `recordUsage` callers already use --- api/server/controllers/agents/errors.js | 2 + api/server/controllers/assistants/chatV1.js | 4 + api/server/controllers/assistants/chatV2.js | 3 + api/server/controllers/assistants/errors.js | 2 + api/server/middleware/abortRun.js | 3 +- api/server/services/Threads/manage.js | 5 +- api/server/services/Threads/manage.spec.js | 91 +++++++++++++++++++++ api/server/services/ToolService.js | 2 + 8 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 api/server/services/Threads/manage.spec.js diff --git a/api/server/controllers/agents/errors.js b/api/server/controllers/agents/errors.js index b16ce75591c..308e10cfc41 100644 --- a/api/server/controllers/agents/errors.js +++ b/api/server/controllers/agents/errors.js @@ -1,5 +1,6 @@ // errorHandler.js const { logger } = require('@librechat/data-schemas'); +const { getTransactionsConfig } = require('@librechat/api'); const { CacheKeys, ViolationTypes } = require('librechat-data-provider'); const { sendResponse } = require('~/server/middleware/error'); const { recordUsage } = require('~/server/services/Threads'); @@ -118,6 +119,7 @@ const createErrorHandler = ({ req, res, getContext, originPath = '/assistants/ch model: run.model, user: req.user.id, conversationId, + transactions: getTransactionsConfig(req.config), }); } catch (error) { logger.error(`[${originPath}] Error fetching or processing run`, error); diff --git a/api/server/controllers/assistants/chatV1.js b/api/server/controllers/assistants/chatV1.js index 2e6e6278756..cac475dee6b 100644 --- a/api/server/controllers/assistants/chatV1.js +++ b/api/server/controllers/assistants/chatV1.js @@ -7,6 +7,7 @@ const { checkBalance, getBalanceConfig, getModelMaxTokens, + getTransactionsConfig, ATTACHMENT_ONLY_TEXT, isContentFilterError, hasActiveFilePolicy, @@ -194,6 +195,7 @@ const chatV1 = async (req, res) => { model: run.model, user: req.user.id, conversationId, + transactions: getTransactionsConfig(req.config), }); } catch (error) { logger.error('[/assistants/chat/] Error fetching or processing run', error); @@ -734,6 +736,7 @@ const chatV1 = async (req, res) => { user: req.user.id, model: completedRun.model ?? model, conversationId, + transactions: getTransactionsConfig(req.config), }); } } else { @@ -742,6 +745,7 @@ const chatV1 = async (req, res) => { user: req.user.id, model: response.run.model ?? model, conversationId, + transactions: getTransactionsConfig(req.config), }); } } catch (error) { diff --git a/api/server/controllers/assistants/chatV2.js b/api/server/controllers/assistants/chatV2.js index cae3da468d3..ac451fa1732 100644 --- a/api/server/controllers/assistants/chatV2.js +++ b/api/server/controllers/assistants/chatV2.js @@ -6,6 +6,7 @@ const { countTokens, checkBalance, getBalanceConfig, + getTransactionsConfig, getModelMaxTokens, ATTACHMENT_ONLY_TEXT, isContentFilterError, @@ -574,6 +575,7 @@ const chatV2 = async (req, res) => { user: req.user.id, model: completedRun.model ?? model, conversationId, + transactions: getTransactionsConfig(req.config), }); } } else { @@ -582,6 +584,7 @@ const chatV2 = async (req, res) => { user: req.user.id, model: response.run.model ?? model, conversationId, + transactions: getTransactionsConfig(req.config), }); } } catch (error) { diff --git a/api/server/controllers/assistants/errors.js b/api/server/controllers/assistants/errors.js index f8dcf39f2bc..4aaa4c68d07 100644 --- a/api/server/controllers/assistants/errors.js +++ b/api/server/controllers/assistants/errors.js @@ -1,5 +1,6 @@ // errorHandler.js const { logger } = require('@librechat/data-schemas'); +const { getTransactionsConfig } = require('@librechat/api'); const { CacheKeys, ViolationTypes, ContentTypes } = require('librechat-data-provider'); const { recordUsage, checkMessageGaps } = require('~/server/services/Threads'); const { sendResponse } = require('~/server/middleware/error'); @@ -124,6 +125,7 @@ const createErrorHandler = ({ req, res, getContext, originPath = '/assistants/ch model: run.model, user: req.user.id, conversationId, + transactions: getTransactionsConfig(req.config), }); } catch (error) { logger.error(`[${originPath}] Error fetching or processing run`, error); diff --git a/api/server/middleware/abortRun.js b/api/server/middleware/abortRun.js index 318693fe15c..04623597ad7 100644 --- a/api/server/middleware/abortRun.js +++ b/api/server/middleware/abortRun.js @@ -1,4 +1,4 @@ -const { sendEvent } = require('@librechat/api'); +const { sendEvent, getTransactionsConfig } = require('@librechat/api'); const { logger } = require('@librechat/data-schemas'); const { CacheKeys, RunStatus, isUUID } = require('librechat-data-provider'); const { initializeClient } = require('~/server/services/Endpoints/assistants'); @@ -66,6 +66,7 @@ async function abortRun(req, res) { model: run.model, user: req.user.id, conversationId, + transactions: getTransactionsConfig(req.config), }); } catch (error) { logger.error('[abortRun] Error fetching or processing run', error); diff --git a/api/server/services/Threads/manage.js b/api/server/services/Threads/manage.js index 772cbd977bf..4e390c39863 100644 --- a/api/server/services/Threads/manage.js +++ b/api/server/services/Threads/manage.js @@ -497,7 +497,8 @@ async function checkMessageGaps({ * @param {string} params.user - The user's ID. * @param {string} params.conversationId - LibreChat conversation ID. * @param {string} [params.context='message'] - The context of the usage. Defaults to 'message'. - * @return {Promise} A promise that resolves to the updated messages + * @param {AppConfig['transactions']} [params.transactions] - Resolved transactions config. + * @return {Promise} */ const recordUsage = async ({ prompt_tokens, @@ -506,6 +507,7 @@ const recordUsage = async ({ user, conversationId, context = 'message', + transactions, }) => { await spendTokens( { @@ -513,6 +515,7 @@ const recordUsage = async ({ model, context, conversationId, + transactions, }, { promptTokens: prompt_tokens, completionTokens: completion_tokens }, ); diff --git a/api/server/services/Threads/manage.spec.js b/api/server/services/Threads/manage.spec.js new file mode 100644 index 00000000000..305a78ee348 --- /dev/null +++ b/api/server/services/Threads/manage.spec.js @@ -0,0 +1,91 @@ +/** + * Tests for recordUsage - the assistants-side token spend path. + * + * `createTransaction` reads the guard out of caller-supplied data, so an + * omitted `transactions` is indistinguishable from an enabled one and the + * write proceeds even when the resolved config says `enabled: false`. + */ + +const mockSpendTokens = jest.fn().mockResolvedValue(); + +jest.mock('@librechat/api', () => ({ + countTokens: jest.fn().mockResolvedValue(0), +})); + +jest.mock('@librechat/data-schemas', () => ({ + escapeRegExp: jest.fn((str) => str), +})); + +jest.mock('~/models', () => ({ + recordMessage: jest.fn(), + getMessages: jest.fn(), + saveConvo: jest.fn(), + spendTokens: (...args) => mockSpendTokens(...args), +})); + +jest.mock('~/server/services/Files/process', () => ({ + retrieveAndProcessFile: jest.fn(), +})); + +const { recordUsage } = require('./manage'); + +describe('recordUsage', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('forwards the resolved transactions config to spendTokens', async () => { + await recordUsage({ + prompt_tokens: 100, + completion_tokens: 50, + model: 'gpt-4', + user: 'user-123', + conversationId: 'convo-123', + transactions: { enabled: false }, + }); + + expect(mockSpendTokens).toHaveBeenCalledTimes(1); + expect(mockSpendTokens).toHaveBeenCalledWith( + { + user: 'user-123', + model: 'gpt-4', + context: 'message', + conversationId: 'convo-123', + transactions: { enabled: false }, + }, + { promptTokens: 100, completionTokens: 50 }, + ); + }); + + it('forwards the config alongside an explicit context', async () => { + await recordUsage({ + prompt_tokens: 10, + completion_tokens: 5, + model: 'gpt-4', + user: 'user-123', + conversationId: 'convo-123', + context: 'incomplete', + transactions: { enabled: true }, + }); + + expect(mockSpendTokens).toHaveBeenCalledWith( + expect.objectContaining({ context: 'incomplete', transactions: { enabled: true } }), + expect.any(Object), + ); + }); + + it('leaves the call unchanged when no config is supplied', async () => { + await recordUsage({ + prompt_tokens: 10, + completion_tokens: 5, + model: 'gpt-4', + user: 'user-123', + conversationId: 'convo-123', + }); + + expect(mockSpendTokens).toHaveBeenCalledWith( + expect.objectContaining({ transactions: undefined }), + expect.any(Object), + ); + }); +}); diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 485694dc528..7d5a161896f 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -45,6 +45,7 @@ const { isFatalAgentInitializationError, resolveCodeExecutionContext, resolveCallerCapabilityProjectionSnapshot, + getTransactionsConfig, } = require('@librechat/api'); const { Time, @@ -296,6 +297,7 @@ const processVisionRequest = async (client, currentAction) => { model: client.req.body.model, conversationId: (client.responseMessage ?? client.finalMessage).conversationId, ...completion.usage, + transactions: getTransactionsConfig(client.req.config), }); } const output = completion?.choices?.[0]?.message?.content ?? 'No image details found.'; From bf6144c9e1f57cb0348c0fe2ddfa0ab781a9e966 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:58:02 +0200 Subject: [PATCH 10/14] =?UTF-8?q?=F0=9F=8E=9B=EF=B8=8F=20fix:=20Withhold?= =?UTF-8?q?=20the=20Seeded=20Model=20Catalogue=20Until=20Models=20Resolve?= =?UTF-8?q?=20(#15035)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `useGetModelsQuery` seeds from a static fallback config, so `modelsQuery.data` describes a hardcoded model list both before the mounted fetch resolves and after it fails outright. The agent builder read that seed as authoritative and offered models the active server configuration never exposed. Blank the catalogue until the mounted fetch actually succeeds, surface the failure in the model panel instead of silently falling back to the seed, and refuse to create an agent against a provider/model pair the resolved catalogue does not offer. Also wires the builder's orphaned `htmlFor` labels to the controls they name. --- client/src/common/types.ts | 1 + .../Agents/AgentCategorySelector.tsx | 1 + .../SidePanel/Agents/AgentConfig.tsx | 1 + .../SidePanel/Agents/AgentPanel.test.tsx | 137 +++++++++++++++++- .../SidePanel/Agents/AgentPanel.tsx | 35 ++++- .../SidePanel/Agents/ModelPanel.test.tsx | 45 ++++++ .../SidePanel/Agents/ModelPanel.tsx | 48 ++++-- 7 files changed, 251 insertions(+), 17 deletions(-) diff --git a/client/src/common/types.ts b/client/src/common/types.ts index cd9c5a79626..d1c53bbfa56 100644 --- a/client/src/common/types.ts +++ b/client/src/common/types.ts @@ -227,6 +227,7 @@ export type AgentModelPanelProps = { agent_id?: string; providers: Option[]; models: Record; + modelsError: boolean; modelsReady: boolean; setActivePanel: React.Dispatch>; }; diff --git a/client/src/components/SidePanel/Agents/AgentCategorySelector.tsx b/client/src/components/SidePanel/Agents/AgentCategorySelector.tsx index 4485c0b08de..7f47061b2eb 100644 --- a/client/src/components/SidePanel/Agents/AgentCategorySelector.tsx +++ b/client/src/components/SidePanel/Agents/AgentCategorySelector.tsx @@ -77,6 +77,7 @@ const AgentCategorySelector: React.FC<{ className?: string }> = ({ className }) return ( * {selectedValue} + {selectPlaceholder} {items.map((item) => (
{/* Endpoint aka Provider for Agents */} -
+
{/* Model */} -
+
{/* Model Parameters */} From 877b9b2f1a356393c4713ca54e44c817b61542ed Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 25 Aug 2026 09:18:52 -0400 Subject: [PATCH 11/14] =?UTF-8?q?=F0=9F=90=9A=20feat:=20Nonce-Based=20Cont?= =?UTF-8?q?ent=20Security=20Policy=20for=20the=20SPA=20Shell=20(#14446)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * πŸ›‘οΈ feat: Configurable Baseline HTTP Security Headers Adds helmet's CSP-independent headers (HSTS, X-Frame-Options, X-Content-Type-Options, COOP, CORP, Referrer-Policy) on every response, with contentSecurityPolicy explicitly disabled. Every header that can break a deployment is configurable, so there is no allow-list to go stale the way #7377's hardcoded CSP directives did. HSTS includeSubDomains defaults off rather than matching helmet's on-by-default: it would otherwise pin every sibling subdomain to HTTPS for a year in every visitor's browser, and undoing that requires serving max-age=0 from each affected host. * πŸ›‘οΈ feat: Nonce-Based Content Security Policy for the SPA Shell Adds an opt-in, per-response nonce CSP on the HTML response, resolved once at startup so each request only mints a nonce and concatenates the header. Report-only by default, since that is the rollout step #7377 skipped. Rebase and correctness pass over #13226: - Styles carry no nonce. A nonce in style-src makes browsers ignore 'unsafe-inline', which would have blocked the ' + + '' + + '' + + '' + + '' + + '' + + '
'; + +jest.mock('~/server/services/Config', () => ({ + syncStaticTools: jest.fn().mockResolvedValue(undefined), + mergeAppTools: jest.fn().mockResolvedValue(undefined), + loadCustomConfig: jest.fn(() => Promise.resolve({})), + getAppConfig: jest.fn().mockResolvedValue({ + paths: { + uploads: '/tmp', + dist: '/tmp/dist-csp', + fonts: '/tmp/fonts-csp', + assets: '/tmp/assets-csp', + }, + fileStrategy: 'local', + imageOutputType: 'PNG', + }), + setCachedTools: jest.fn(), +})); + +jest.mock('~/server/services/Agents/triggers', () => ({ + initializeAgentTriggerService: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock('~/server/services/Schedules', () => ({ + initializeScheduleEngine: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock('~/app/clients/tools', () => ({ + createOpenAIImageTools: jest.fn(() => []), + createYouTubeTools: jest.fn(() => []), + manifestToolMap: {}, + toolkits: [], +})); + +jest.mock('~/config', () => ({ + createMCPServersRegistry: jest.fn(), + createMCPManager: jest.fn().mockResolvedValue({ + getAppToolFunctions: jest.fn().mockResolvedValue({}), + }), +})); + +jest.mock( + '@librechat/api/telemetry', + () => ({ + initializeTelemetry: jest.fn(() => ({ + enabled: false, + status: 'disabled', + shutdown: jest.fn(), + })), + telemetryMiddleware: jest.fn((_req, _res, next) => next()), + telemetryErrorMiddleware: jest.fn((err, _req, _res, next) => next(err)), + }), + { virtual: true }, +); + +describe('Content Security Policy', () => { + jest.setTimeout(30_000); + + let mongoServer; + let app; + + const originalReadFileSync = fs.readFileSync; + + beforeAll(async () => { + fs.readFileSync = function (filepath, options) { + if (filepath.includes('index.html')) { + return INDEX_HTML; + } + return originalReadFileSync(filepath, options); + }; + + for (const dir of ['/tmp/dist-csp', '/tmp/fonts-csp', '/tmp/assets-csp']) { + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + } + fs.writeFileSync(path.join('/tmp/dist-csp', 'index.html'), INDEX_HTML); + + mongoServer = await MongoMemoryServer.create(); + process.env.MONGO_URI = mongoServer.getUri(); + process.env.PORT = '0'; + + /* Read once at startup, so they must be set before the server module loads. */ + process.env.CSP_ENABLED = 'true'; + process.env.CSP_REPORT_ONLY = 'false'; + process.env.CSP_CONNECT_SRC_EXTRA = 'https://telemetry.example.com'; + /* A cacheable override that CSP must refuse for the shell. */ + process.env.INDEX_CACHE_CONTROL = 'public, max-age=3600'; + + app = require('~/server'); + await healthCheckPoll(app); + }); + + afterAll(async () => { + fs.readFileSync = originalReadFileSync; + delete process.env.CSP_ENABLED; + delete process.env.CSP_REPORT_ONLY; + delete process.env.CSP_CONNECT_SRC_EXTRA; + delete process.env.INDEX_CACHE_CONTROL; + await mongoServer.stop(); + await mongoose.disconnect(); + }); + + it('sends an enforcing policy whose nonce matches the served scripts', async () => { + const response = await request(app).get('/'); + const csp = response.headers['content-security-policy']; + const nonce = csp?.match(/script-src 'nonce-([^']+)'/)?.[1]; + + expect(response.status).toBe(200); + expect(response.headers['content-security-policy-report-only']).toBeUndefined(); + expect(nonce).toBeTruthy(); + expect(response.text).toContain(``); + expect(response.text).toContain(`', + '', + ].join(''); + + expect(applyCspNonce(html, 'abc123')).toBe( + [ + '', + '', + '', + ].join(''), + ); + }); + + it('replaces a stale nonce rather than preserving it', () => { + const html = ''; + + expect(applyCspNonce(html, 'abc123')).toBe(''); + expect(applyCspNonce(html, 'abc123')).not.toContain('from-the-build'); + }); + + it('stamps module preloads, which strict-dynamic does not cover', () => { + const html = [ + '', + '', + '', + '', + ].join(''); + + expect(applyCspNonce(html, 'abc123')).toBe( + [ + '', + '', + '', + '', + ].join(''), + ); + }); + + it('returns the html untouched without a nonce', () => { + const html = ''; + expect(applyCspNonce(html, '')).toBe(html); + }); +}); diff --git a/packages/api/src/security/csp.ts b/packages/api/src/security/csp.ts new file mode 100644 index 00000000000..11c5a1dbee6 --- /dev/null +++ b/packages/api/src/security/csp.ts @@ -0,0 +1,308 @@ +import { randomBytes } from 'crypto'; +import { logger } from '@librechat/data-schemas'; +import { parseEnvSwitch } from './env'; +import { isEnabled } from '../utils'; + +/** Split point for the per-request nonce. Randomized so no env value can collide. */ +const NONCE_SLOT = `__csp_nonce_${randomBytes(8).toString('hex')}__`; + +const DIRECTIVE_NAME_PATTERN = /^[a-z][a-z0-9-]*$/; +/** `` is in here because module preloads are fetched under `script-src`. */ +const NONCEABLE_TAG_PATTERN = /<(script|link)\b([^>]*)>/gi; +const NONCE_ATTRIBUTE_PATTERN = /\snonce\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi; +const REL_PATTERN = /\srel\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i; +const AS_SCRIPT_PATTERN = /\sas\s*=\s*(?:"script"|'script'|script\b)/i; + +type CspDirective = [string, string[]]; + +/** Precomputed once at startup; only the nonce varies per response. */ +export interface CspPolicy { + headerName: 'Content-Security-Policy' | 'Content-Security-Policy-Report-Only'; + prefix: string; + suffix: string; +} + +export interface CspResponse { + headerName: CspPolicy['headerName']; + headerValue: string; + nonce: string; +} + +const SOURCE_EXTRA_ENV: Record = { + 'default-src': 'CSP_DEFAULT_SRC_EXTRA', + 'script-src': 'CSP_SCRIPT_SRC_EXTRA', + 'style-src': 'CSP_STYLE_SRC_EXTRA', + 'img-src': 'CSP_IMG_SRC_EXTRA', + 'font-src': 'CSP_FONT_SRC_EXTRA', + 'connect-src': 'CSP_CONNECT_SRC_EXTRA', + 'media-src': 'CSP_MEDIA_SRC_EXTRA', + 'frame-src': 'CSP_FRAME_SRC_EXTRA', + 'worker-src': 'CSP_WORKER_SRC_EXTRA', + 'form-action': 'CSP_FORM_ACTION_EXTRA', +}; + +function splitSourceList(value: string | undefined): string[] { + if (!value) { + return []; + } + return value + .split(/[,\s]+/) + .map((source) => source.trim()) + .filter(Boolean); +} + +/** + * Only an explicitly recognized false value enforces. A typo or an unrecognized + * truthy spelling stays report-only, so a config slip cannot turn a rollout into + * a blocked SPA. + */ +function isReportOnly(env: NodeJS.ProcessEnv): boolean { + return parseEnvSwitch('CSP_REPORT_ONLY', env.CSP_REPORT_ONLY, true); +} + +/** + * `'strict-dynamic'` makes browsers ignore every host source in `script-src`, so it + * cannot coexist with operator-supplied script hosts. When extras are configured we + * drop it and let the (now honored) `'self'` plus those hosts govern script loading. + */ +function scriptSources(scriptExtras: string[], allowWasm: boolean): string[] { + /* 'wasm-unsafe-eval' permits WebAssembly compilation without permitting eval(); + * the HEIC upload path (client/src/utils/heicConverter.ts -> heic-to) needs it. */ + const wasm = allowWasm ? ["'wasm-unsafe-eval'"] : []; + if (scriptExtras.length === 0) { + return [`'nonce-${NONCE_SLOT}'`, "'strict-dynamic'", ...wasm, "'self'"]; + } + logger.info( + "[CSP] CSP_SCRIPT_SRC_EXTRA is set; omitting 'strict-dynamic' so the configured script hosts take effect.", + ); + return [`'nonce-${NONCE_SLOT}'`, ...wasm, "'self'"]; +} + +/** + * Styles intentionally carry no nonce. A nonce in `style-src` makes browsers ignore + * `'unsafe-inline'`, which would block every `