diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 14d7bc5956..8337e0c330 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -21,6 +21,7 @@ import { useChatFormContext, useAddedChatContext, useAssistantsMapContext, + BadgeRowProvider, } from '~/Providers'; import useAskAnswerMode from '~/hooks/Input/useAskAnswerMode'; import AskUserQuestionPopover from './AskUserQuestionPopover'; @@ -32,7 +33,7 @@ import DuringRunSendButton from './DuringRunSendButton'; import useDictation from '~/hooks/Input/useDictation'; import { useGetStartupConfig } from '~/data-provider'; import useSteering from '~/hooks/Chat/useSteering'; -import { BadgeRowProvider } from '~/Providers'; + import TextareaHeader from './TextareaHeader'; import PromptsCommand from './PromptsCommand'; import SkillsCommand from './SkillsCommand'; diff --git a/client/src/components/Chat/Input/Composer/Effort.tsx b/client/src/components/Chat/Input/Composer/Effort.tsx index 885c7bcbcb..db1a45e198 100644 --- a/client/src/components/Chat/Input/Composer/Effort.tsx +++ b/client/src/components/Chat/Input/Composer/Effort.tsx @@ -3,8 +3,8 @@ import { CircleHelp } from 'lucide-react'; import { Constants } from 'librechat-data-provider'; import { HoverCard, HoverCardTrigger, HoverCardContent, HoverCardPortal } from '@librechat/client'; import type { SettingDefinition, TConversation } from 'librechat-data-provider'; +import type { TSetOption, LocalizeFunction } from '~/common'; import type { TranslationKeys } from '~/hooks'; -import type { TSetOption } from '~/common'; import useReducedMotion from '~/hooks/Generic/useReducedMotion'; import { useLocalize } from '~/hooks'; import { cn } from '~/utils'; @@ -33,8 +33,6 @@ const ARROW_STEP: Record = { }; const THUMB_FADE_MS = 140; -type Localize = (key: TranslationKeys) => string; - /** * `enumMappings` maps a raw value to a translation KEY, not to display text — * rendering it directly is what leaks `com_ui_medium` into the UI. Shared with @@ -43,7 +41,7 @@ type Localize = (key: TranslationKeys) => string; export function resolveEffortLabel( setting: SettingDefinition, value: string, - localize: Localize, + localize: LocalizeFunction, ): string { const mapped = setting.enumMappings?.[value]; if (mapped != null) { diff --git a/client/src/components/Chat/Input/Composer/Palette.tsx b/client/src/components/Chat/Input/Composer/Palette.tsx index 938c552ce9..418ea129de 100644 --- a/client/src/components/Chat/Input/Composer/Palette.tsx +++ b/client/src/components/Chat/Input/Composer/Palette.tsx @@ -29,9 +29,10 @@ import useToolFavorites from '~/hooks/Input/useToolFavorites'; import useElementSize from '~/hooks/Generic/useElementSize'; import useRecentFiles from '~/hooks/Input/useRecentFiles'; import useAttachItems from '~/hooks/Input/useAttachItems'; -import { getFileType } from '~/utils'; +import { isMacPlatform } from '~/utils/shortcuts'; +import { getFileType, cn } from '~/utils'; import { useLocalize } from '~/hooks'; -import { cn } from '~/utils'; + import store from '~/store'; const HEADER_HEIGHT = 26; @@ -62,6 +63,9 @@ const ROW_SHIFT_EASING = 'cubic-bezier(0.32, 0.72, 0, 1)'; const KEY_SEP = '\u0000'; const NO_ENTERING: ReadonlySet = new Set(); const NO_ROWS: PaletteRow[] = []; +/** Spelled out for the platform, since this is read aloud: "Mod" is a + * developer's shorthand and not a key on anybody's keyboard. */ +const FAVORITE_MODIFIER = isMacPlatform ? '\u2318' : 'Ctrl'; /** A row's element id, derived from its identity so the combobox keeps naming * the same row as the list rearranges under it. */ @@ -987,11 +991,17 @@ function Palette({ className="w-full border-0 bg-transparent text-sm text-text-primary shadow-none ring-0 placeholder:text-text-secondary focus:border-0 focus:outline-none focus:ring-0 focus-visible:outline-none focus-visible:ring-0" /> - {localize('com_ui_composer_palette_help')} + {localize('com_ui_composer_palette_help', { 0: FAVORITE_MODIFIER })} {rows.length === 0 ? ( -
+ /* A search that matches nothing is a state change with nothing + to focus, so it has to be said rather than only drawn. */ +
{localize('com_ui_composer_no_results')}
) : ( diff --git a/client/src/components/Chat/Input/Composer/Queue.tsx b/client/src/components/Chat/Input/Composer/Queue.tsx index 7879882d06..f7bda91862 100644 --- a/client/src/components/Chat/Input/Composer/Queue.tsx +++ b/client/src/components/Chat/Input/Composer/Queue.tsx @@ -1,4 +1,4 @@ -import { memo, useRef, useState, useCallback } from 'react'; +import { memo, useRef, useMemo, useState, useCallback } from 'react'; import { useRecoilValue } from 'recoil'; import { useDrag, useDrop } from 'react-dnd'; import { useMediaQuery } from '@librechat/client'; @@ -20,6 +20,8 @@ const REORDER_HINT_ID = 'composer-queue-reorder-hint'; interface DragItem { id: string; index: number; + /** The order the drag started from, so abandoning it puts things back. */ + order: string[]; } /** Restores a message's text into the composer, or refuses (false) when the @@ -47,6 +49,9 @@ interface QueueRowProps { message: QueuedMessage; index: number; total: number; + /** Every queued id in order, captured when a drag starts so abandoning it + * can put the queue back the way it was. */ + order: string[]; steering: SteeringControls; conversationId: string; onEditToComposer: QueueProps['onEditToComposer']; @@ -58,6 +63,7 @@ function QueueRow({ message, index, total, + order, steering, conversationId, onEditToComposer, @@ -67,7 +73,7 @@ function QueueRow({ const localize = useLocalize(); const rowRef = useRef(null); const gripRef = useRef(null); - const { reorderQueued } = steering; + const { reorderQueued, restoreQueuedOrder } = steering; /* The queue is sent in order, so one message cannot be ahead of or behind itself: the handle only means something once there is somewhere to go. */ const reorderable = total > 1; @@ -98,8 +104,16 @@ function QueueRow({ const [{ isDragging }, drag] = useDrag({ type: DRAG_TYPE, canDrag: reorderable && canDrag, - item: (): DragItem => ({ id: message.id, index }), + item: (): DragItem => ({ id: message.id, index, order }), collect: (monitor) => ({ isDragging: monitor.isDragging() }), + /* The rows are moved as the pointer crosses them, so a drag the user + abandons — Escape, or a release outside the rail — has already changed + the queue. Dropping nowhere puts the order back. */ + end: (item, monitor) => { + if (!monitor.didDrop()) { + restoreQueuedOrder(item.order); + } + }, }); const move = useCallback( @@ -216,13 +230,18 @@ function QueueRow({ type="button" aria-label={localize('com_ui_remove_queued')} onClick={() => { - onRestoreToComposer( + /* Only dropped once the words are somewhere else. The composer + refuses when it is occupied or the user has moved to another + chat, and removing the message anyway destroyed it. */ + const restored = onRestoreToComposer( message.text, message.files, { quotes: message.quotes, manualSkills: message.manualSkills }, conversationId, ); - steering.removeQueued(message.id); + if (restored) { + steering.removeQueued(message.id); + } }} className={ICON_BTN} > @@ -255,6 +274,7 @@ function Queue({ steering, conversationId, onEditToComposer, onRestoreToComposer /* Spoken only for the keys. A drag reorders on every crossing, and a reader narrating each one would be behind the pointer and in the way of it. */ const [announcement, setAnnouncement] = useState(''); + const order = useMemo(() => queued.map((message) => message.id), [queued]); if (queued.length === 0) { return null; @@ -279,6 +299,7 @@ function Queue({ steering, conversationId, onEditToComposer, onRestoreToComposer message={message} index={index} total={queued.length} + order={order} steering={steering} conversationId={conversationId} onEditToComposer={onEditToComposer} diff --git a/client/src/components/Chat/Input/Composer/__tests__/Palette.spec.tsx b/client/src/components/Chat/Input/Composer/__tests__/Palette.spec.tsx index ea840ba6ac..18c60780e3 100644 --- a/client/src/components/Chat/Input/Composer/__tests__/Palette.spec.tsx +++ b/client/src/components/Chat/Input/Composer/__tests__/Palette.spec.tsx @@ -251,10 +251,17 @@ describe('Palette', () => { expect(keys()).toEqual(['h:tool', 'b', 'a']); }); - it('says so when nothing matches', () => { + it('says so when nothing matches, out loud as well as on screen', () => { renderPalette(); search('nothing matches this'); - expect(screen.getByText('com_ui_composer_no_results')).toBeInTheDocument(); + const empty = screen.getByText('com_ui_composer_no_results'); + expect(empty).toBeInTheDocument(); + expect(empty).toHaveAttribute('role', 'status'); + /* And the field stops claiming to control a list that is not there. */ + expect(screen.getByTestId('composer-palette-search')).toHaveAttribute( + 'aria-expanded', + 'false', + ); }); }); diff --git a/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx b/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx index 3c17403c0b..791574282a 100644 --- a/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx +++ b/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx @@ -22,6 +22,7 @@ const CONVO_ID = 'convo-1'; const mockSendQueuedNow = jest.fn(); const mockRemoveQueued = jest.fn(); const mockReorderQueued = jest.fn(); +const mockRestoreQueuedOrder = jest.fn(); const steering = { queueKey: CONVO_ID, @@ -30,6 +31,7 @@ const steering = { sendQueuedNow: mockSendQueuedNow, removeQueued: mockRemoveQueued, reorderQueued: mockReorderQueued, + restoreQueuedOrder: mockRestoreQueuedOrder, } as unknown as SteeringControls; const pausedSteering = { ...steering, canSteer: false } as unknown as SteeringControls; @@ -169,11 +171,14 @@ describe('Queue', () => { expect(mockRemoveQueued).toHaveBeenCalledWith('q1'); }); - it('drops the message even when the composer refuses to take it back', () => { + /* The composer refuses when it is occupied or the user has moved on. Dropping + the message anyway is the only path here that can destroy text outright. */ + it('keeps the message queued when the composer refuses to take it back', () => { const onRestore = jest.fn().mockReturnValue(false); renderQueue([queued({ id: 'q1' })], steering, { onRestoreToComposer: onRestore }); fireEvent.click(screen.getByLabelText('com_ui_remove_queued')); - expect(mockRemoveQueued).toHaveBeenCalledWith('q1'); + expect(onRestore).toHaveBeenCalled(); + expect(mockRemoveQueued).not.toHaveBeenCalled(); }); it('hands the whole message to the composer to edit', () => { 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 63d29a47bb..6f0d5bc444 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.test.tsx @@ -74,6 +74,11 @@ jest.mock('../ParallelContent', () => ({ ), })); +jest.mock('../Parts/PendingSteers', () => ({ + __esModule: true, + default: () =>
, +})); + import ContentParts from '../ContentParts'; const baseProps = { @@ -259,3 +264,24 @@ describe('ContentParts — post-steer author re-attribution', () => { expect(screen.queryByTestId('post-steer-agent-update')).toBeNull(); }); }); + +/* The pending block belongs to the reply being written: shown under every + message, or after the run has ended, it reads as unsent words piling up. */ +describe('ContentParts — pending steers', () => { + const withGate = (over: Partial & { conversationId?: string }) => + render(); + + it('shows them on the last message while a run is live', () => { + withGate({ isLast: true, isSubmitting: true, conversationId: 'convo-1' }); + expect(screen.getByTestId('pending-steers')).toBeInTheDocument(); + }); + + it.each([ + ['an earlier message', { isLast: false, isSubmitting: true, conversationId: 'convo-1' }], + ['a finished run', { isLast: true, isSubmitting: false, conversationId: 'convo-1' }], + ['no conversation yet', { isLast: true, isSubmitting: true, conversationId: undefined }], + ])('shows nothing for %s', (_label, over) => { + withGate(over); + expect(screen.queryByTestId('pending-steers')).not.toBeInTheDocument(); + }); +}); diff --git a/client/src/data-provider/SSE/mutations.ts b/client/src/data-provider/SSE/mutations.ts index 0f6b3d0afd..4c24ba511d 100644 --- a/client/src/data-provider/SSE/mutations.ts +++ b/client/src/data-provider/SSE/mutations.ts @@ -211,9 +211,3 @@ export const cancelSteerMessage = async ( params, ) as Promise; }; - -export function useCancelSteerMutation() { - return useMutation({ - mutationFn: cancelSteerMessage, - }); -} diff --git a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx index 216c3fbc1f..d4cb0f95e4 100644 --- a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx @@ -177,6 +177,40 @@ describe('useSteerRecovery', () => { expect(queue).toEqual([expect.objectContaining({ id: 'srv-late', text: 'landed too late' })]); }); + /* The picks the message was written with have to survive the retry, or the + words are re-sent without the quotes and skills they referred to. */ + it('carries the quotes and skills the steer was written with', async () => { + mockMutateAsync.mockResolvedValue({ + steerId: 'srv-ctx', + status: 'queued', + position: 1, + conversationId: CONVO_ID, + }); + const { result } = setup(({ set }) => { + set(store.pendingSteersByConvoId(CONVO_ID), [ + { + steerId: 'local-ctx', + text: 'about that quote', + status: 'failed', + createdAt: 4, + quotes: ['carried quote'], + manualSkills: ['carried-skill'], + }, + ]); + }); + act(() => { + result.current.recovery.retry('local-ctx'); + }); + await flush(); + expect(result.current.chips).toEqual([ + expect.objectContaining({ + steerId: 'srv-ctx', + quotes: ['carried quote'], + manualSkills: ['carried-skill'], + }), + ]); + }); + it('no-ops when the steer id is no longer pending', () => { const { result } = setup(); act(() => { diff --git a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx index cbaf9291a6..5a89e6c003 100644 --- a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx @@ -124,326 +124,6 @@ describe('useSteering', () => { }); }); - describe('queueReclaimedSteer', () => { - const reclaimed = { - steerId: 's-reclaimed', - text: 'reclaimed words', - status: 'pending' as const, - createdAt: 1_000, - }; - - /** The run-end signal `useQueueDrain` consumes; seeded here to stand for a - * run that already finished by the time a reclaim resolved. */ - const runEnd = (outcome: 'completed' | 'aborted' | 'error', conversationId = CONVO_ID) => ({ - conversationId, - outcome, - endedAt: 2_000, - }); - - function setupWithQueue( - params: HookParams = {}, - initialize?: (snapshot: MutableSnapshot) => void, - ) { - const sendNow = jest.fn(); - const stopGenerating = jest.fn(); - const wrapper = ({ children }: { children: React.ReactNode }) => ( - {children} - ); - const rendered = renderHook( - () => ({ - steering: useSteering({ - index: 0, - conversationId: CONVO_ID, - conversation: agentsConversation, - isSubmitting: true, - answerModeActive: false, - sendNow, - stopGenerating, - ...params, - }), - queue: useQueue(CONVO_ID), - /** What `useQueueDrain` watches: re-posting it is how this hook asks - * the drain to reconsider a queue it already passed over. */ - parkedRunEnd: useRecoilValue(store.pendingRunEndByConvoId(CONVO_ID)), - /** Stands in for the drain CONSUMING a signal it has acted on. */ - consumeIndexSignal: useSetRecoilState(store.runEndByIndex(0)), - consumeParkedSignal: useSetRecoilState(store.pendingRunEndByConvoId(CONVO_ID)), - }), - { wrapper }, - ); - return { ...rendered, sendNow }; - } - - it('keeps the steer ahead of a follow-up queued after it', () => { - // The steer was accepted BEFORE the follow-up, so it must drain first. - // Minting a fresh id/createdAt here would sort it last. - const { result } = setupWithQueue(); - act(() => { - result.current.steering.enqueue('queued later', {}); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(result.current.queue.map((item) => item.text)).toEqual([ - 'reclaimed words', - 'queued later', - ]); - // The original identity survives, which is what the ordering rests on. - expect(result.current.queue[0].id).toBe('s-reclaimed'); - expect(result.current.queue[0].createdAt).toBe(1_000); - }); - - it('leaves the item to the drain while the run is still going', () => { - const { result, sendNow } = setupWithQueue({ isSubmitting: true }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(sendNow).not.toHaveBeenCalled(); - // The run's own end is still ahead of this item and will drain it, so - // nothing needs re-arming. - expect(result.current.parkedRunEnd).toBeNull(); - expect(result.current.queue).toHaveLength(1); - }); - - it('re-arms the drain when the run completed cleanly while the reclaim was in flight', () => { - // The drain already consumed its one-shot signal against an empty queue, - // so re-post it: the DRAIN sends (FIFO, via `ask`, which does not reset - // the composer), never this hook. - const { result, sendNow } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed')); - }); - act(() => { - // The drain ran against an empty queue and consumed the signal — the - // outcome was already captured during render. - result.current.consumeIndexSignal(null); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(result.current.parkedRunEnd).toMatchObject({ - conversationId: CONVO_ID, - outcome: 'completed', - }); - // The item stays queued for the drain to pick up in its turn. - expect(result.current.queue.map((item) => item.id)).toEqual(['s-reclaimed']); - expect(sendNow).not.toHaveBeenCalled(); - }); - - it('re-arms from a run-end parked while the user was in another chat', () => { - // The run finished with this conversation off-screen, so its signal was - // parked rather than delivered on the index. Without watching the parked - // carrier too, the outcome would never be seen and the item would strand. - const { result } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.pendingRunEndByConvoId(CONVO_ID), runEnd('completed')); - }); - act(() => { - result.current.consumeParkedSignal(null); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(result.current.parkedRunEnd).toMatchObject({ outcome: 'completed' }); - expect(result.current.queue.map((item) => item.id)).toEqual(['s-reclaimed']); - }); - - it.each(['aborted', 'error'] as const)( - 'leaves the item for manual send when the run %s', - (outcome) => { - // The drain auto-sends only on a clean completion: a Stop or an error - // means the user is taking over, so nothing may smuggle the text out. - const { result, sendNow } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.runEndByIndex(0), runEnd(outcome)); - }); - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(sendNow).not.toHaveBeenCalled(); - expect(result.current.parkedRunEnd).toBeNull(); - expect(result.current.queue.map((item) => item.id)).toEqual(['s-reclaimed']); - }, - ); - - it('leaves the item for manual send when the completed run was another chat', () => { - const { result, sendNow } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed', 'convo-elsewhere')); - }); - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(sendNow).not.toHaveBeenCalled(); - expect(result.current.parkedRunEnd).toBeNull(); - expect(result.current.queue).toHaveLength(1); - }); - - it('keeps older queued follow-ups ahead of the reclaimed steer', () => { - // The drain sends ONE item per run end, FIFO. Re-arming (rather than - // sending here) is what keeps an older follow-up from being skipped. - const { result } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed')); - set(store.queuedMessagesByConvoId(CONVO_ID), [ - { id: 'older', text: 'queued first', createdAt: 500 }, - ]); - }); - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(result.current.queue.map((item) => item.id)).toEqual(['older', 's-reclaimed']); - }); - - it('does not re-arm while this conversation’s run-end is still unconsumed', () => { - // The drain has not run yet, so it will see this item on its own. Arming - // a second carrier would drain twice and send two messages. - const { result } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.pendingRunEndByConvoId(CONVO_ID), runEnd('completed')); - set(store.runEndByIndex(0), runEnd('completed')); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - // Untouched: the already-armed signal drains it. - expect(result.current.parkedRunEnd).toMatchObject({ outcome: 'completed' }); - expect(result.current.queue.map((item) => item.id)).toEqual(['s-reclaimed']); - }); - - /** Renders the hook so the chat it points at can change under it, the way - * ChatForm reuses it when the user navigates. */ - function setupNavigable( - initialProps: { convoId: string; isSubmitting: boolean }, - initialize?: (snapshot: MutableSnapshot) => void, - ) { - const sendNow = jest.fn(); - const wrapper = ({ children }: { children: React.ReactNode }) => ( - {children} - ); - const rendered = renderHook( - ({ convoId, isSubmitting }: { convoId: string; isSubmitting: boolean }) => ({ - steering: useSteering({ - index: 0, - conversationId: convoId, - conversation: agentsConversation, - isSubmitting, - answerModeActive: false, - sendNow, - stopGenerating: jest.fn(), - }), - parkedHere: useRecoilValue(store.pendingRunEndByConvoId(CONVO_ID)), - queueHere: useQueue(CONVO_ID), - /** Stands in for the drain CONSUMING a signal it has acted on. */ - consumeIndexSignal: useSetRecoilState(store.runEndByIndex(0)), - }), - { wrapper, initialProps }, - ); - return { ...rendered, sendNow }; - } - - it('still re-arms the origin chat after the user navigates away', () => { - // Navigating away does not make the words any less owed a send: the run - // they belong to completed, so its queue must still drain on return. - const { result, rerender, sendNow } = setupNavigable( - { convoId: CONVO_ID, isSubmitting: false }, - ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed')); - }, - ); - // Captured while still on this chat, resolving after the user left. - const queueReclaimed = result.current.steering.queueReclaimedSteer; - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - rerender({ convoId: 'convo-elsewhere', isSubmitting: false }); - }); - act(() => { - queueReclaimed(reclaimed); - }); - - expect(result.current.parkedHere).toMatchObject({ - conversationId: CONVO_ID, - outcome: 'completed', - }); - expect(result.current.queueHere.map((item) => item.id)).toEqual(['s-reclaimed']); - expect(sendNow).not.toHaveBeenCalled(); - }); - - it('never parks another chat’s run-end under this conversation', () => { - // The run-end is keyed by conversation, so the new chat's end can never - // be mistaken for this one's. Parking it here would give `drainNext` a - // foreign `end.conversationId` and drain the wrong queue into this chat. - const { result, rerender } = setupNavigable({ convoId: CONVO_ID, isSubmitting: true }); - const queueReclaimed = result.current.steering.queueReclaimedSteer; - act(() => { - // The user leaves for a chat whose own run then completes. - rerender({ convoId: 'convo-elsewhere', isSubmitting: false }); - }); - act(() => { - result.current.consumeIndexSignal(runEnd('completed', 'convo-elsewhere')); - }); - act(() => { - queueReclaimed(reclaimed); - }); - - expect(result.current.parkedHere).toBeNull(); - expect(result.current.queueHere.map((item) => item.id)).toEqual(['s-reclaimed']); - }); - - it('does not re-arm from the end of an earlier run of the same chat', () => { - // A stale end must not authorize a drain: this chat's NEXT run is what - // owns the item, and its own end will drain it. - const { result, rerender } = setupNavigable( - { convoId: CONVO_ID, isSubmitting: false }, - ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed')); - }, - ); - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - // A new run starts on this same chat, superseding that end. - rerender({ convoId: CONVO_ID, isSubmitting: true }); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - - expect(result.current.parkedHere).toBeNull(); - expect(result.current.queueHere.map((item) => item.id)).toEqual(['s-reclaimed']); - }); - - it('re-arms even when another conversation’s run-end occupies the index slot', () => { - // The index slot is shared. The drain parks a foreign signal under ITS - // conversation and then only inspects the active one's queue, so treating - // it as proof of an upcoming drain would strand this item. - const { result } = setupWithQueue({ isSubmitting: false }, ({ set }) => { - set(store.runEndByIndex(0), runEnd('completed')); - }); - act(() => { - result.current.consumeIndexSignal(null); - }); - act(() => { - // A later run on the shared index slot, belonging to a different chat. - result.current.consumeIndexSignal(runEnd('completed', 'convo-elsewhere')); - }); - act(() => { - result.current.steering.queueReclaimedSteer(reclaimed); - }); - expect(result.current.parkedRunEnd).toMatchObject({ - conversationId: CONVO_ID, - outcome: 'completed', - }); - expect(result.current.queue.map((item) => item.id)).toEqual(['s-reclaimed']); - }); - }); - describe('submitDuringRun', () => { it('routes to the steer POST with an optimistic sending chip', () => { const { result } = setup({}, ({ set }) => { @@ -1297,27 +977,6 @@ describe('useSteering', () => { manualSkills: ['carried-skill'], }), ); - mockMutate.mockImplementationOnce((_params, { onSuccess }) => { - onSuccess({ - steerId: 'srv-retry', - status: 'queued', - position: 1, - conversationId: CONVO_ID, - }); - }); - act(() => { - result.current.steering.retrySteer(failed.steerId, failed.text, failed.files, { - quotes: failed.quotes, - manualSkills: failed.manualSkills, - }); - }); - expect(result.current.chips).toEqual([ - expect.objectContaining({ - steerId: 'srv-retry', - quotes: ['carried quote'], - manualSkills: ['carried-skill'], - }), - ]); }); it('leaves composer atoms staged when a composer-origin steer degrades', () => { diff --git a/client/src/hooks/Chat/index.ts b/client/src/hooks/Chat/index.ts index 3dc2a8fd65..cb10473ac5 100644 --- a/client/src/hooks/Chat/index.ts +++ b/client/src/hooks/Chat/index.ts @@ -8,5 +8,4 @@ export { default as useIdChangeEffect } from './useIdChangeEffect'; export { default as useFocusChatEffect } from './useFocusChatEffect'; export { default as useQueueDrain } from './useQueueDrain'; export { default as useSteering } from './useSteering'; -export { default as useSteerCancel, useSteerReclaim } from './useSteerCancel'; export { default as useSteerConvert } from './useSteerConvert'; diff --git a/client/src/hooks/Chat/useSteerCancel.ts b/client/src/hooks/Chat/useSteerCancel.ts deleted file mode 100644 index 85cb1f879b..0000000000 --- a/client/src/hooks/Chat/useSteerCancel.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { useCallback } from 'react'; -import { useRecoilCallback } from 'recoil'; -import type { PendingSteer } from '~/store/families'; -import { useCancelSteerMutation } from '~/data-provider'; -import store from '~/store'; - -/** - * `reclaimed` — the cancel beat the boundary; the words never entered the run. - * `applied` — the steer already injected (or the run ended): the events own it. - * `failed` — the POST failed, so the entry is restored and the server may still - * inject it. - */ -export type SteerCancelOutcome = 'reclaimed' | 'applied' | 'failed'; - -/** - * Asks the server to drop a steer before its injection boundary, touching no - * chip state — the caller owns what happens to the words. - * - * A steer leaves the server queue only by injecting, so only `reclaimed` proves - * the words never entered the run and are still the client's to re-home. Giving - * an `applied` steer a second life (queueing it, editing it back into the - * composer) would say the same thing twice; on `failed` the server may still - * inject it, so its fate is unknown and it must be left alone. - */ -export function useSteerReclaim(conversationId: string) { - const cancelMutation = useCancelSteerMutation(); - - return useCallback( - async (steer: PendingSteer): Promise => { - try { - const { removed } = await cancelMutation.mutateAsync({ - conversationId, - steerId: steer.steerId, - }); - return removed === true ? 'reclaimed' : 'applied'; - } catch { - return 'failed'; - } - }, - [conversationId, cancelMutation], - ); -} - -/** - * Cancels a steer still waiting on its injection boundary. Optimistic: the - * entry leaves the chip stack immediately; `removed: false` needs no handling - * (the steer already injected or the run ended — the events own the outcome). - * Only a failed POST restores the entry, since the server would still inject - * the supposedly-cancelled words. - */ -export default function useSteerCancel(conversationId: string) { - const reclaim = useSteerReclaim(conversationId); - - const removeEntry = useRecoilCallback( - ({ set }) => - (steerId: string) => { - set(store.pendingSteersByConvoId(conversationId), (prev) => - prev.filter((item) => item.steerId !== steerId), - ); - }, - [conversationId], - ); - const restoreEntry = useRecoilCallback( - ({ snapshot, set }) => - (entry: PendingSteer) => { - /* A steer that settled while the POST was in flight — applied on the - * server, or converted to a queued follow-up at run end — must NOT come - * back. The next run (a queue drain auto-sends one) would render this - * stale entry as an in-flight bubble beside its own queued copy. */ - const settled = snapshot - .getLoadable(store.appliedSteerIdsByConvoId(conversationId)) - .getValue(); - if (settled.includes(entry.steerId)) { - return; - } - set(store.pendingSteersByConvoId(conversationId), (prev) => - prev.some((item) => item.steerId === entry.steerId) ? prev : [...prev, entry], - ); - }, - [conversationId], - ); - - return useCallback( - async (steer: PendingSteer): Promise => { - removeEntry(steer.steerId); - const outcome = await reclaim(steer); - if (outcome === 'failed') { - restoreEntry(steer); - } - return outcome; - }, - [reclaim, removeEntry, restoreEntry], - ); -} diff --git a/client/src/hooks/Chat/useSteering.ts b/client/src/hooks/Chat/useSteering.ts index 34459a75c5..8957072416 100644 --- a/client/src/hooks/Chat/useSteering.ts +++ b/client/src/hooks/Chat/useSteering.ts @@ -435,6 +435,33 @@ export default function useSteering({ [queueKey], ); + /** + * Puts the queue back in a remembered order, for a drag the user abandoned. + * Ids that have since drained are skipped rather than resurrected, and + * anything queued mid-drag keeps its place at the back. + */ + const restoreQueuedOrder = useRecoilCallback( + ({ set }) => + (ids: readonly string[]) => { + set(store.queuedMessagesByConvoId(queueKey), (prev) => { + const byId = new Map(prev.map((item) => [item.id, item])); + const restored: QueuedMessage[] = []; + for (const id of ids) { + const item = byId.get(id); + if (item != null) { + restored.push(item); + byId.delete(id); + } + } + if (restored.length === 0) { + return prev; + } + return [...restored, ...byId.values()]; + }); + }, + [queueKey], + ); + /** Capture-then-remove, so a refused send can restore the ORIGINAL item. */ const takeQueued = useRecoilCallback( ({ snapshot, set }) => @@ -889,13 +916,10 @@ export default function useSteering({ steerFromComposer, queueFromComposer, submitSteer, - retrySteer, - removeSteer, - convertSteerToQueue, - queueReclaimedSteer, enqueue, removeQueued, reorderQueued, + restoreQueuedOrder, sendQueuedNow, interruptAndSend, interruptSteer, @@ -913,13 +937,10 @@ export default function useSteering({ steerFromComposer, queueFromComposer, submitSteer, - retrySteer, - removeSteer, - convertSteerToQueue, - queueReclaimedSteer, enqueue, removeQueued, reorderQueued, + restoreQueuedOrder, sendQueuedNow, interruptAndSend, interruptSteer, diff --git a/client/src/hooks/Files/__tests__/useAttachExisting.spec.tsx b/client/src/hooks/Files/__tests__/useAttachExisting.spec.tsx new file mode 100644 index 0000000000..3874286acf --- /dev/null +++ b/client/src/hooks/Files/__tests__/useAttachExisting.spec.tsx @@ -0,0 +1,196 @@ +import { renderHook } from '@testing-library/react'; +import { FileSources, EModelEndpoint, mergeFileConfig } from 'librechat-data-provider'; +import type { TFile } from 'librechat-data-provider'; +import type { ExtendedFile } from '~/common'; +import useAttachExisting from '../useAttachExisting'; + +/** + * Re-attaching a file already on the server. Every branch here is a refusal, + * and an inverted one either blocks every re-attachment or stages a file the + * endpoint will reject at send time, when the words are already gone. + */ + +const mockShowToast = jest.fn(); +const mockAddFile = jest.fn(); +let mockFileMap: Record; +let mockConversation: { endpoint?: string | null; endpointType?: string } | null; +let mockStaged: Map; +/** Raw, not merged: the hook's own `select` merges it, and merging twice + * scales the megabyte limits twice and puts them out of reach. */ +let mockFileConfig: Parameters[0]; + +jest.mock('@librechat/client', () => ({ + useToastContext: () => ({ showToast: mockShowToast }), +})); + +jest.mock('~/Providers', () => ({ + useFileMapContext: () => mockFileMap, + useChatContext: () => ({ + files: mockStaged, + setFiles: jest.fn(), + conversation: mockConversation, + }), +})); + +jest.mock('~/data-provider', () => ({ + useGetFileConfig: ({ select }: { select?: (data: unknown) => unknown }) => ({ + data: select != null ? select(mockFileConfig) : mockFileConfig, + }), +})); + +jest.mock('~/hooks/Files/useUpdateFiles', () => ({ + __esModule: true, + default: () => ({ addFile: mockAddFile }), +})); + +jest.mock('~/hooks/useLocalize', () => ({ + __esModule: true, + default: () => (key: string) => key, +})); + +const MB = 1024 * 1024; +/** The endpoint config is written in megabytes; it scales them itself. */ + +const file = (over: Partial = {}): TFile => + ({ + file_id: 'f1', + filename: 'notes.pdf', + filepath: '/files/notes.pdf', + type: 'application/pdf', + bytes: MB, + source: FileSources.local, + ...over, + }) as TFile; + +const staged = (over: Partial = {}): ExtendedFile => + ({ file_id: 'other', size: MB, progress: 1, ...over }) as ExtendedFile; + +const attach = (target: TFile = file()) => { + const { result } = renderHook(() => useAttachExisting()); + result.current(target); +}; + +describe('useAttachExisting', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockFileMap = { f1: file() }; + mockConversation = { endpoint: EModelEndpoint.openAI }; + mockStaged = new Map(); + mockFileConfig = { + endpoints: { + [EModelEndpoint.openAI]: { + fileLimit: 3, + fileSizeLimit: 5, + totalSizeLimit: 10, + supportedMimeTypes: ['application/pdf'], + }, + }, + }; + }); + + it('stages the file it was given, marked as already uploaded', () => { + attach(); + expect(mockAddFile).toHaveBeenCalledWith( + expect.objectContaining({ + file_id: 'f1', + filename: 'notes.pdf', + progress: 1, + attached: true, + size: MB, + }), + ); + expect(mockShowToast).not.toHaveBeenCalled(); + }); + + describe('refusals', () => { + const refused = (message: string) => { + expect(mockAddFile).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining(message), status: 'error' }), + ); + }; + + it('refuses a file it cannot find on the server', () => { + mockFileMap = {}; + attach(); + refused('com_ui_attach_error'); + }); + + it('refuses when there is no conversation to attach to', () => { + mockConversation = null; + attach(); + refused('com_ui_attach_error'); + }); + + it('refuses OpenAI-stored files outside the assistants endpoint', () => { + mockFileMap = { f1: file({ source: FileSources.openai }) }; + attach(); + refused('com_ui_attach_error_openai'); + }); + + it('refuses when the endpoint has uploads switched off', () => { + mockFileConfig = { endpoints: { [EModelEndpoint.openAI]: { disabled: true } } }; + attach(); + refused('com_ui_attach_error_disabled'); + }); + + it('refuses once the endpoint is already holding its limit', () => { + mockStaged = new Map([ + ['a', staged({ file_id: 'a' })], + ['b', staged({ file_id: 'b' })], + ['c', staged({ file_id: 'c' })], + ]); + attach(); + refused('com_ui_attach_error_limit'); + }); + + it('refuses a file bigger than the endpoint takes', () => { + mockFileMap = { f1: file({ bytes: 6 * MB }) }; + attach(); + refused('com_ui_attach_error_size'); + }); + + it('refuses a type the endpoint does not accept', () => { + const zip = file({ type: 'application/zip' }); + mockFileMap = { f1: zip }; + attach(zip); + refused('com_ui_attach_error_type'); + }); + + it('refuses when it would put the staged files over the total', () => { + mockStaged = new Map([['big', staged({ file_id: 'big', size: 9.5 * MB })]]); + attach(); + refused('com_ui_attach_error_total_size'); + }); + }); + + /* Re-attaching a file that is already staged replaces it, so its own size + must come out of the running total before the check. */ + it('counts a file it is replacing out of the total', () => { + mockFileMap = { f1: file({ bytes: 6 * MB }) }; + mockFileConfig = { + endpoints: { + [EModelEndpoint.openAI]: { + fileSizeLimit: 8, + totalSizeLimit: 10, + supportedMimeTypes: ['application/pdf'], + }, + }, + }; + mockStaged = new Map([['f1', staged({ file_id: 'f1', size: 6 * MB })]]); + attach(); + expect(mockAddFile).toHaveBeenCalled(); + }); + + it('warns but still attaches a non-OpenAI file on the assistants endpoint', () => { + mockConversation = { endpoint: EModelEndpoint.assistants }; + mockFileConfig = { + endpoints: { [EModelEndpoint.assistants]: { supportedMimeTypes: ['application/pdf'] } }, + }; + attach(); + expect(mockShowToast).toHaveBeenCalledWith( + expect.objectContaining({ message: 'com_ui_attach_warn_endpoint', status: 'warning' }), + ); + expect(mockAddFile).toHaveBeenCalled(); + }); +}); diff --git a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx index 2c2189cc14..4c38c3684f 100644 --- a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx +++ b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { RecoilRoot } from 'recoil'; -import { renderHook } from '@testing-library/react'; -import { Tools, EModelEndpoint } from 'librechat-data-provider'; +import { act, renderHook } from '@testing-library/react'; +import { Tools, EToolResources, EModelEndpoint } from 'librechat-data-provider'; import type { TConversation } from 'librechat-data-provider'; import useAttachItems from '../useAttachItems'; @@ -23,8 +23,9 @@ jest.mock('~/hooks/useLocalize', () => ({ default: () => (key: string) => key, })); +const mockHandleFileChange = jest.fn(); jest.mock('~/hooks/Files', () => ({ - useFileHandlingNoChatContext: () => ({ handleFileChange: jest.fn() }), + useFileHandlingNoChatContext: () => ({ handleFileChange: mockHandleFileChange }), })); jest.mock('~/hooks/Files/useSharePointFileHandling', () => ({ @@ -106,6 +107,42 @@ function renderEntries(options: Options = {}): string[] { const allCapabilities = { contextEnabled: true, fileSearchEnabled: true, codeEnabled: true }; +/** The hook, not just its entry labels, so a destination can be chosen. */ +function renderAttach(options: Options = {}) { + mockUseAgentToolPermissions.mockReturnValue({ tools: options.tools, provider: options.provider }); + mockUseAgentCapabilities.mockReturnValue({ + contextEnabled: options.contextEnabled ?? true, + fileSearchEnabled: options.fileSearchEnabled ?? true, + codeEnabled: options.codeEnabled ?? true, + }); + mockUseGetAgentsConfig.mockReturnValue({ agentsConfig: { capabilities: [] } }); + mockUseGetStartupConfig.mockReturnValue({ data: { sharePointFilePickerEnabled: false } }); + + return renderHook( + () => + useAttachItems({ + agentId: options.agentId ?? null, + endpoint: options.endpoint ?? null, + endpointType: options.endpointType, + useResponsesApi: options.useResponsesApi, + conversationId: 'convo-1', + conversation: { conversationId: 'convo-1' } as TConversation, + files: new Map(), + setFiles: jest.fn(), + setFilesLoading: jest.fn(), + }), + { + wrapper: ({ children }: { children: React.ReactNode }) => {children}, + }, + ); +} + +/** Stands in for the picker: a real one cannot open in jsdom, and what matters + * is that the change event arrives while the chosen destination is still set. */ +const pickFile = (input: HTMLInputElement, onFileChange: (e: never) => void) => { + onFileChange({ target: input } as never); +}; + describe('useAttachItems', () => { beforeEach(() => jest.clearAllMocks()); @@ -273,6 +310,53 @@ describe('useAttachItems', () => { }); }); + describe('where the picked file is routed', () => { + /* The destination is held in a ref precisely because the picker can fire + its change event before React has committed any state, so losing it here + lands a File Search upload as a plain provider attachment. */ + it('carries the chosen destination through the picker and then forgets it', () => { + const { result } = renderAttach(); + const input = document.createElement('input'); + Object.defineProperty(result.current.inputRef, 'current', { value: input, writable: true }); + + act(() => { + result.current.entries.find((entry) => entry.id === 'local:file_search')?.onSelect(); + }); + act(() => pickFile(input, result.current.onFileChange)); + expect(mockHandleFileChange).toHaveBeenLastCalledWith( + expect.anything(), + EToolResources.file_search, + ); + + /* And the next pick is a plain one unless a destination is chosen again. */ + act(() => { + result.current.entries.find((entry) => entry.id === 'local:provider')?.onSelect(); + }); + act(() => pickFile(input, result.current.onFileChange)); + expect(mockHandleFileChange).toHaveBeenLastCalledWith(expect.anything(), undefined); + }); + + it('scopes the picker to what the destination can send', () => { + const { result } = renderAttach({ endpointType: EModelEndpoint.bedrock }); + const input = document.createElement('input'); + const accepts: string[] = []; + input.click = () => accepts.push(input.accept); + Object.defineProperty(result.current.inputRef, 'current', { value: input, writable: true }); + + act(() => { + result.current.entries.find((entry) => entry.id === 'local:provider')?.onSelect(); + }); + expect(accepts[0]).toContain('image/'); + + act(() => { + result.current.entries.find((entry) => entry.id === 'local:context')?.onSelect(); + }); + /* Text extraction takes anything the server can read, so it does not + narrow the picker at all. */ + expect(accepts[1]).toBe(''); + }); + }); + describe('edge cases', () => { it.each([ ['undefined endpoint and provider', { endpoint: undefined, provider: undefined }], diff --git a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts index 7000f2d539..d723e7b2d1 100644 --- a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts +++ b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts @@ -2,8 +2,8 @@ import type { ComposerHintState } from '../useComposerHint'; import { composeHint } from '../useComposerHint'; /** Echoes the key so assertions read against the key, not English copy. */ -const localize = ((key: string, options?: Record) => - options ? `${key}:${options[0]}` : key) as Parameters[1]; +const localize = ((key: string, options?: Record) => + options ? `${key}:${options[0] ?? options.count}` : key) as Parameters[1]; const baseState: ComposerHintState = { hasText: false, @@ -99,6 +99,10 @@ describe('composeHint', () => { expect(hint({ uploadingCount: 2, duringRunActive: true, hasText: true })).toBe( 'com_ui_composer_hint_uploading:2', ); + /* One file is one file, not "1 file(s)". */ + expect(hint({ uploadingCount: 1, duringRunActive: true, hasText: true })).toBe( + 'com_ui_composer_hint_uploading_one:1', + ); }); it('stops reporting uploads once they settle', () => { diff --git a/client/src/hooks/Input/__tests__/useComposerItems.spec.tsx b/client/src/hooks/Input/__tests__/useComposerItems.spec.tsx new file mode 100644 index 0000000000..09db9a7ab1 --- /dev/null +++ b/client/src/hooks/Input/__tests__/useComposerItems.spec.tsx @@ -0,0 +1,72 @@ +import React from 'react'; +import { act, renderHook } from '@testing-library/react'; +import { RecoilRoot, useRecoilValue, type MutableSnapshot } from 'recoil'; +import useComposerItems from '../useComposerItems'; +import store from '~/store'; + +/** + * The tray's staged context. Removal is the whole surface here, and quotes are + * addressed by position, which is the way to dismiss the wrong one. + */ + +const CONVO_ID = 'convo-items'; + +function setup(initialize?: (snapshot: MutableSnapshot) => void) { + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + return renderHook( + () => ({ + items: useComposerItems(CONVO_ID), + quotes: useRecoilValue(store.pendingQuotesByConvoId(CONVO_ID)), + skills: useRecoilValue(store.pendingManualSkillsByConvoId(CONVO_ID)), + }), + { wrapper }, + ); +} + +const withStaged = (quotes: string[], skills: string[] = []) => + setup(({ set }) => { + set(store.pendingQuotesByConvoId(CONVO_ID), quotes); + set(store.pendingManualSkillsByConvoId(CONVO_ID), skills); + }); + +describe('useComposerItems', () => { + it('stages nothing for an untouched composer', () => { + expect(setup().result.current.items).toEqual([]); + }); + + it('lists quotes ahead of skills', () => { + const { result } = withStaged(['a quote'], ['writer']); + expect(result.current.items.map((item) => item.kind)).toEqual(['quote', 'skill']); + expect(result.current.items.map((item) => item.label)).toEqual(['a quote', 'writer']); + }); + + it('removes the quote that was dismissed, not the first one', () => { + const { result } = withStaged(['first', 'second', 'third']); + act(() => result.current.items[1].remove()); + expect(result.current.quotes).toEqual(['first', 'third']); + }); + + it('removes a skill by name', () => { + const { result } = withStaged([], ['writer', 'researcher']); + act(() => result.current.items[1].remove()); + expect(result.current.skills).toEqual(['writer']); + }); + + /* Two identical excerpts are two separate stagings, and dismissing one has to + leave the other alone. */ + it('keeps two identical quotes separately removable', () => { + const { result } = withStaged(['same words', 'same words']); + expect(new Set(result.current.items.map((item) => item.id)).size).toBe(2); + + act(() => result.current.items[0].remove()); + expect(result.current.quotes).toEqual(['same words']); + }); + + it('carries the full text for a chip that has to truncate it', () => { + const long = 'a quote long enough that the chip will have to cut it short somewhere'; + const { result } = withStaged([long]); + expect(result.current.items[0].title).toBe(long); + }); +}); diff --git a/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx b/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx index 78296d00e1..493365696b 100644 --- a/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx +++ b/client/src/hooks/Input/__tests__/usePaletteEntries.spec.tsx @@ -17,13 +17,16 @@ let mockCapabilities: Record; let mockContext: Record | null; let mockSkills: Array>; let mockAgentsMap: Record>; +let mockSkillsActive: boolean; jest.mock('~/hooks', () => ({ useHasAccess: ({ permissionType }: { permissionType: string }) => mockPermissions[permissionType] ?? false, useHasMemoryAccess: () => mockMemoryAccess, useAgentCapabilities: () => mockCapabilities, - useSkillActiveState: () => ({ isActive: () => false }), + /* The real popover filter is used here, and it drops anything the user + cannot invoke or that is not active for them. */ + useSkillActiveState: () => ({ isActive: () => mockSkillsActive }), })); jest.mock('~/hooks/useLocalize', () => ({ @@ -46,18 +49,25 @@ jest.mock('~/data-provider', () => ({ }), })); -jest.mock('~/components/Chat/Input/SkillsCommand', () => ({ - filterSkillsForPopover: (skills: Array>) => skills, -})); - -const toggle = (state: unknown) => ({ toggleState: state, debouncedChange: jest.fn() }); +const toggle = (state: unknown): ToggleFixture => ({ + toggleState: state, + debouncedChange: jest.fn(), +}); interface ServerFixture { serverName: string; config?: { title?: string; description?: string; iconPath?: string }; } +interface ToggleFixture { + toggleState: unknown; + debouncedChange: jest.Mock; +} + interface ContextFixture { + webSearch: ToggleFixture; + codeInterpreter: ToggleFixture; + artifacts: ToggleFixture; mcpServerManager: { selectableServers?: ServerFixture[]; mcpValues: string[]; @@ -119,6 +129,7 @@ describe('usePaletteEntries', () => { mockContext = fullContext(); mockSkills = []; mockAgentsMap = {}; + mockSkillsActive = true; }); it('offers nothing before the badge row has a context to read', () => { @@ -187,6 +198,37 @@ describe('usePaletteEntries', () => { }); }); + describe('choosing a row', () => { + it('flips each tool through its own toggle', () => { + const context = fullContext(); + mockContext = context; + const listed = entries().result.current; + act(() => listed.find((item) => item.key === 'builtin:web_search')?.onSelect()); + expect(context.webSearch.debouncedChange).toHaveBeenCalledWith({ value: true }); + expect(context.codeInterpreter.debouncedChange).not.toHaveBeenCalled(); + }); + + it('turns a tool back off from its on state', () => { + const context = { ...fullContext(), webSearch: toggle(true) }; + mockContext = context; + const listed = entries().result.current; + act(() => listed.find((item) => item.key === 'builtin:web_search')?.onSelect()); + expect(context.webSearch.debouncedChange).toHaveBeenCalledWith({ value: false }); + }); + + it('returns a mode pill to the default when it is already the stored mode', () => { + const context = { ...fullContext(), artifacts: toggle(ArtifactModes.SHADCNUI) }; + mockContext = context; + const modes = entries().result.current.find( + (item) => item.key === 'builtin:artifacts', + )?.modes; + act(() => modes?.find((mode) => mode.id === 'shadcn')?.onSelect()); + expect(context.artifacts.debouncedChange).toHaveBeenCalledWith({ + value: ArtifactModes.DEFAULT, + }); + }); + }); + describe('artifacts, which carries its modes on the row', () => { const artifactsRow = () => entries().result.current.find((item) => item.key === 'builtin:artifacts'); @@ -283,6 +325,39 @@ describe('usePaletteEntries', () => { }); }); + /* A third gate, and the one that fails closed: a persisted agent sees only + the skills it was built with, and none at all until the map has loaded. */ + describe('which skills an agent may see', () => { + beforeEach(() => { + mockSkills = [ + { _id: 's1', name: 'writer', displayTitle: 'Writing Helper' }, + { _id: 's2', name: 'researcher', displayTitle: 'Researcher' }, + ]; + }); + + const skillKeys = (agentId?: string | null) => + keysOf(entries(agentId).result).filter((key) => key.startsWith('skill:')); + + it('lists the whole catalog for an ephemeral agent', () => { + expect(skillKeys('openAI__gpt-5___GPT-5')).toEqual(['skill:s1', 'skill:s2']); + }); + + it('lists nothing for a persisted agent that has skills switched off', () => { + mockAgentsMap = { agent_1: { skills_enabled: false } }; + expect(skillKeys('agent_1')).toEqual([]); + }); + + it('lists nothing while the agents map is still loading', () => { + mockAgentsMap = {}; + expect(skillKeys('agent_1')).toEqual([]); + }); + + it('lists only the skills a persisted agent was built with', () => { + mockAgentsMap = { agent_1: { skills_enabled: true, skills: ['s2'] } }; + expect(skillKeys('agent_1')).toEqual(['skill:s2']); + }); + }); + describe('servers', () => { it('titles a server by its config, falling back to its name', () => { const listed = entries().result.current; @@ -290,6 +365,14 @@ describe('usePaletteEntries', () => { expect(listed.find((item) => item.key === 'mcp:spotify')?.label).toBe('spotify'); }); + it('toggles a server by its own name, not by its title', () => { + const context = fullContext(); + mockContext = context; + const row = entries().result.current.find((item) => item.key === 'mcp:github'); + act(() => row?.onSelect()); + expect(context.mcpServerManager.toggleServerSelection).toHaveBeenCalledWith('github'); + }); + it('lists none while the manager has no selectable servers', () => { const context = fullContext(); context.mcpServerManager = { ...context.mcpServerManager, selectableServers: undefined }; diff --git a/client/src/hooks/Input/useAttachItems.tsx b/client/src/hooks/Input/useAttachItems.tsx index dd8589d062..360c9db170 100644 --- a/client/src/hooks/Input/useAttachItems.tsx +++ b/client/src/hooks/Input/useAttachItems.tsx @@ -23,6 +23,7 @@ import type { EndpointFileConfig, MimeUploadCapability, } from 'librechat-data-provider'; +import type { SharePointFile } from '~/data-provider/Files/sharepoint'; import type { ExtendedFile, FileSetter } from '~/common'; import { useSharePointFileHandlingNoChatContext } from '~/hooks/Files/useSharePointFileHandling'; import { useAgentToolPermissions, useAgentCapabilities, useGetAgentsConfig } from '~/hooks'; @@ -65,6 +66,20 @@ export interface AttachEntry { onSelect: () => void; } +export interface UseAttachItems { + entries: AttachEntry[]; + /** The picker lives outside the popover, so selecting a file cannot race it + * unmounting; the caller mounts it. */ + inputRef: React.RefObject; + onFileChange: (event: React.ChangeEvent) => void; + isSharePointDialogOpen: boolean; + setIsSharePointDialogOpen: React.Dispatch>; + onSharePointFilesSelected: (files: SharePointFile[]) => Promise; + isProcessing: boolean; + downloadProgress: ReturnType['downloadProgress']; + maxSelectionCount: number | undefined; +} + interface UseAttachItemsParams { agentId?: string | null; endpoint?: string | null; @@ -98,7 +113,7 @@ export default function useAttachItems({ files, setFiles, setFilesLoading, -}: UseAttachItemsParams) { +}: UseAttachItemsParams): UseAttachItems { const localize = useLocalize(); const inputRef = useRef(null); const toolResourceRef = useRef(); diff --git a/client/src/hooks/Input/useComposerHint.ts b/client/src/hooks/Input/useComposerHint.ts index 92123a298d..b7376d4ed8 100644 --- a/client/src/hooks/Input/useComposerHint.ts +++ b/client/src/hooks/Input/useComposerHint.ts @@ -1,4 +1,5 @@ import type { TranslationKeys } from '~/hooks/useLocalize'; +import type { LocalizeFunction } from '~/common'; import { isMacPlatform } from '~/utils/shortcuts'; import useLocalize from '~/hooks/useLocalize'; @@ -14,8 +15,6 @@ export interface ComposerHintState { uploadingCount: number; } -type Localize = (key: TranslationKeys, options?: Record) => string; - /** * `tip` is ambient discovery copy, true of the composer at all times. `state` * reports something happening right now. Only the first is worth permanent @@ -41,7 +40,7 @@ const SEPARATOR = ' · '; */ export function composeHint( state: ComposerHintState, - localize: Localize, + localize: LocalizeFunction, isMac: boolean, ): ComposerHint { if (state.answerModeActive) { @@ -50,7 +49,12 @@ export function composeHint( if (state.uploadingCount > 0) { return { - text: localize('com_ui_composer_hint_uploading', { 0: String(state.uploadingCount) }), + text: localize( + state.uploadingCount === 1 + ? 'com_ui_composer_hint_uploading_one' + : 'com_ui_composer_hint_uploading', + { count: state.uploadingCount }, + ), kind: 'state', }; } diff --git a/client/src/hooks/Input/usePaletteEntries.tsx b/client/src/hooks/Input/usePaletteEntries.tsx index 63186e70e2..fcddbfa427 100644 --- a/client/src/hooks/Input/usePaletteEntries.tsx +++ b/client/src/hooks/Input/usePaletteEntries.tsx @@ -6,19 +6,21 @@ import { Permissions, ArtifactModes, PermissionTypes, + isEphemeralAgentId, defaultAgentCapabilities, } from 'librechat-data-provider'; import type { TSkillSummary, TToolFavoriteType } from 'librechat-data-provider'; -import { useHasAccess, useHasMemoryAccess, useAgentCapabilities } from '~/hooks'; +import { + useHasAccess, + useHasMemoryAccess, + useAgentCapabilities, + useSkillActiveState, +} from '~/hooks'; import { filterSkillsForPopover } from '~/components/Chat/Input/SkillsCommand'; +import { useAgentsMapContext, useBadgeRowContext } from '~/Providers'; import { useSkillsInfiniteQuery } from '~/data-provider'; -import { useAgentsMapContext } from '~/Providers'; -import { ephemeralAgentByConvoId } from '~/store'; -import { useBadgeRowContext } from '~/Providers'; -import { useSkillActiveState } from '~/hooks'; +import store, { ephemeralAgentByConvoId } from '~/store'; import useLocalize from '~/hooks/useLocalize'; -import { isEphemeralAgent } from '~/common'; -import store from '~/store'; export type PaletteSection = 'tool' | 'skill' | 'mcp'; @@ -167,7 +169,7 @@ export default function usePaletteEntries({ catalog; persisted agents gate on `skills_enabled` and fail closed while `agentsMap` is hydrating or when the agent is missing from it. */ const agentSkillIds = useMemo(() => { - if (!agentId || isEphemeralAgent(agentId)) { + if (!agentId || isEphemeralAgentId(agentId)) { return undefined; } if (!agentsMap) { diff --git a/client/src/hooks/Input/useToolFavorites.ts b/client/src/hooks/Input/useToolFavorites.ts index 61c81391f2..a1e16bc2f7 100644 --- a/client/src/hooks/Input/useToolFavorites.ts +++ b/client/src/hooks/Input/useToolFavorites.ts @@ -1,10 +1,6 @@ import { useMemo, useCallback } from 'react'; -import type { TToolFavorite, TToolFavoriteType } from 'librechat-data-provider'; -import { - useGetToolFavoritesQuery, - useAddToolFavoriteMutation, - useRemoveToolFavoriteMutation, -} from '~/data-provider'; +import type { TToolFavoriteType } from 'librechat-data-provider'; +import useMarketplaceFavorites from '~/hooks/useToolFavorites'; export interface ToolFavorites { /** `${itemType}:${itemId}` keys, for O(1) membership tests while building @@ -18,39 +14,32 @@ export const favoriteKey = (itemType: TToolFavoriteType, itemId: string) => `${i /** * Server-persisted favourites for anything the composer palette can list — - * built-in tools, plugin tools, MCP servers and skills. Backed by the same - * `{ user, itemType, itemId }` records the tools marketplace writes, so a - * favourite starred here is already starred there. + * built-in tools, plugin tools, MCP servers and skills. + * + * A shape adapter over the marketplace's own hook rather than a second copy of + * it: the palette addresses items by type and id where the marketplace uses + * `{ kind, id }`, and that was the whole difference. Going through it also + * carries its error handling, so starring past the server's cap says so here + * too instead of failing silently with the star left where it was. */ export default function useToolFavorites(): ToolFavorites { - const { data: favorites } = useGetToolFavoritesQuery(); - const addFavorite = useAddToolFavoriteMutation(); - const removeFavorite = useRemoveToolFavoriteMutation(); - - const keys = useMemo(() => { - const set = new Set(); - for (const favorite of favorites ?? []) { - set.add(favoriteKey(favorite.itemType, favorite.itemId)); - } - return set; - }, [favorites]); + const { favoriteKeys, toggle } = useMarketplaceFavorites(); const isFavorite = useCallback( - (itemType: TToolFavoriteType, itemId: string) => keys.has(favoriteKey(itemType, itemId)), - [keys], + (itemType: TToolFavoriteType, itemId: string) => + favoriteKeys.has(favoriteKey(itemType, itemId)), + [favoriteKeys], ); const toggleFavorite = useCallback( (itemType: TToolFavoriteType, itemId: string) => { - const favorite: TToolFavorite = { itemType, itemId }; - if (keys.has(favoriteKey(itemType, itemId))) { - removeFavorite.mutate(favorite); - return; - } - addFavorite.mutate(favorite); + void toggle({ kind: itemType, id: itemId }); }, - [keys, addFavorite, removeFavorite], + [toggle], ); - return { keys, isFavorite, toggleFavorite }; + return useMemo( + () => ({ keys: favoriteKeys, isFavorite, toggleFavorite }), + [favoriteKeys, isFavorite, toggleFavorite], + ); } diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 8f136ceb37..6c0d287656 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1007,11 +1007,12 @@ "com_ui_composer_hint_steer": "Enter steers", "com_ui_composer_hint_stop": "Esc to stop", "com_ui_composer_hint_typing": "Enter to send · Shift+Enter for newline", - "com_ui_composer_hint_uploading": "Uploading {{0}} file(s)…", + "com_ui_composer_hint_uploading": "Uploading {{count}} files…", + "com_ui_composer_hint_uploading_one": "Uploading {{count}} file…", "com_ui_composer_mcp": "MCP Servers", "com_ui_composer_no_results": "No matches", "com_ui_composer_palette": "Attach and tools", - "com_ui_composer_palette_help": "Use the arrow keys to browse, Enter to select, and Mod+D to favorite.", + "com_ui_composer_palette_help": "Use the arrow keys to browse, Enter to select, and {{0}}+D to favorite.", "com_ui_composer_palette_search": "Search tools, skills and servers", "com_ui_composer_staged_context": "Staged context", "com_ui_composer_thinking": "Thinking", @@ -1650,7 +1651,6 @@ "com_ui_remote_agents_allow_use": "Allow users to create API keys and query agents remotely", "com_ui_remove_agent_from_chain": "Remove {{0}} from chain", "com_ui_remove_all_quotes": "Remove all selections", - "com_ui_remove_file": "Remove file", "com_ui_remove_from_project": "Remove from project", "com_ui_remove_queued": "Remove message", "com_ui_remove_quote": "Remove quote", @@ -1843,7 +1843,6 @@ "com_ui_shop": "Shopping", "com_ui_show": "Show", "com_ui_show_all": "Show All", - "com_ui_show_all_count": "Show all {{0}}", "com_ui_show_code": "Show Code", "com_ui_show_image_details": "Show Image Details", "com_ui_show_less": "Show less", diff --git a/client/src/utils/files.ts b/client/src/utils/files.ts index fe887fa9a7..6833fae304 100644 --- a/client/src/utils/files.ts +++ b/client/src/utils/files.ts @@ -388,12 +388,6 @@ const isContextType = (type: string, fileConfig: FileConfig | null): boolean => ...(fileConfig?.stt?.supportedMimeTypes || []), ]); -/** - * Upload destinations a file set can be routed to, given the active endpoint and agent - * capabilities. `undefined` is direct provider attachment; the rest are tool resources. - * Each option requires every file to be valid for it, so the caller can decide between - * auto-routing (one option), prompting (multiple), or rejecting (none). - */ /** * Which tool destinations an upload may be routed to, before the files * themselves are considered. @@ -406,10 +400,15 @@ const isContextType = (type: string, fileConfig: FileConfig | null): boolean => * Shared by the `+` menu and the drag-and-drop router so a file has the same * destinations however it arrives. */ +export interface UploadToolAllowances { + fileSearchAllowedByAgent: boolean; + codeAllowedByAgent: boolean; +} + export const getUploadToolAllowances = ( agentId: string | null | undefined, tools: string[] | undefined, -): { fileSearchAllowedByAgent: boolean; codeAllowedByAgent: boolean } => { +): UploadToolAllowances => { const isSavedAgent = agentId != null && agentId !== '' && !isEphemeralAgentId(agentId); return { fileSearchAllowedByAgent: !isSavedAgent || (tools?.includes(Tools.file_search) ?? false), @@ -417,6 +416,12 @@ export const getUploadToolAllowances = ( }; }; +/** + * Upload destinations a file set can be routed to, given the active endpoint and agent + * capabilities. `undefined` is direct provider attachment; the rest are tool resources. + * Each option requires every file to be valid for it, so the caller can decide between + * auto-routing (one option), prompting (multiple), or rejecting (none). + */ export const getViableUploadOptions = ( fileList: File[], ctx: UploadOptionContext,