diff --git a/client/src/Providers/__tests__/BadgeRowContext.spec.tsx b/client/src/Providers/__tests__/BadgeRowContext.spec.tsx new file mode 100644 index 0000000000..54aaf23028 --- /dev/null +++ b/client/src/Providers/__tests__/BadgeRowContext.spec.tsx @@ -0,0 +1,110 @@ +import React from 'react'; +import { RecoilRoot } from 'recoil'; +import { render } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import BadgeRowProvider, { useBadgeRowContext } from '../BadgeRowContext'; + +/** + * `BadgeRowProvider` is an inline child of `ChatForm`, which re-renders on every + * keystroke in the composer. Its context value is memoized for exactly that + * reason — but a memo is only worth as much as its dependencies, and two of them + * were hooks handing back a fresh object literal every render. The value changed + * on each character typed, and every consumer of it rebuilt: the palette's whole + * tool, skill and MCP server catalog, per keystroke. + */ + +/* Held outside the factory so it is one function for the whole test, the way + the real `ToastProvider` sits above the composer and does not re-render with + it. A fresh `showToast` per render would be the mock's instability, not the + provider's. */ +const mockShowToast = jest.fn(); +jest.mock('@librechat/client', () => ({ + useToastContext: () => ({ showToast: mockShowToast }), +})); + +/* The suite-wide `react-i18next` stub builds a new `t` on every render, which + the real one does not: it holds `t` in state and replaces it only when the + language or namespace changes. Left as-is, every `useLocalize` consumer here + would look unstable for a reason that does not exist outside the tests. */ +const mockTranslate = (key: string) => key; +const mockToolAuth = { authenticated: false }; +jest.mock('react-i18next', () => ({ + ...jest.requireActual('react-i18next'), + useTranslation: () => ({ t: mockTranslate, i18n: { changeLanguage: jest.fn() } }), +})); + +/* Startup config and the server catalog are external HTTP queries. Their + contents do not matter to this identity test, and leaving them live lets a + rejected `/api/config` request report after Jest has torn the test down. */ +jest.mock('~/data-provider', () => ({ + ...jest.requireActual('~/data-provider'), + useGetEndpointsQuery: () => ({ data: undefined }), + useGetStartupConfig: () => ({ data: undefined }), + useMCPServersQuery: () => ({ data: undefined, isLoading: false }), + useVerifyAgentToolAuth: () => ({ data: mockToolAuth }), +})); + +function renderProvider() { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false, cacheTime: 0 } }, + }); + let value: ReturnType; + const Consumer = () => { + value = useBadgeRowContext(); + return null; + }; + const Tree = ({ marker }: { marker: number }) => ( + + + + {/* Re-rendered along with the provider, the way `Bar` is. */} + {marker} + + + + + ); + const { rerender } = render(); + return { + getValue: () => value, + retype: (marker: number) => rerender(), + }; +} + +describe('BadgeRowProvider', () => { + it('hands consumers the same value across a parent re-render', () => { + const { getValue, retype } = renderProvider(); + const first = getValue(); + retype(1); + expect(getValue()).toBe(first); + retype(2); + expect(getValue()).toBe(first); + }); + + /* The two that were rebuilding: both are carried into the context value + whole, so an unstable reference from either defeats the memo by itself. */ + it('keeps the search form and MCP manager referentially stable', () => { + const { getValue, retype } = renderProvider(); + const first = getValue(); + retype(1); + + const last = getValue(); + expect(last?.searchApiKeyForm).toBe(first?.searchApiKeyForm); + expect(last?.mcpServerManager).toBe(first?.mcpServerManager); + }); + + /* Without this the cases above would still pass against a provider that had + stopped providing: every value would be `undefined`, and identical. */ + it('is undefined outside the provider, and an object inside it', () => { + let outside: unknown = 'unset'; + const Consumer = () => { + outside = useBadgeRowContext(); + return null; + }; + render(); + expect(outside).toBeUndefined(); + + const { getValue } = renderProvider(); + expect(getValue()).toEqual(expect.objectContaining({ conversationId: 'convo-badge-row' })); + }); +}); diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 28ce15d9b4..3b072770b2 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -4,7 +4,7 @@ import { TextareaAutosize } from '@librechat/client'; import { useRecoilState, useRecoilValue } from 'recoil'; import { Constants, isAssistantsEndpoint, isAgentsEndpoint } from 'librechat-data-provider'; import type { TMessage, TConversation } from 'librechat-data-provider'; -import type { ExtendedFile, FileSetter, ConvoGenerator } from '~/common'; +import type { ExtendedFile, FileSetter, ConvoGenerator, TAskFunction } from '~/common'; import type { QueuedMessageContext } from '~/hooks/Chat/useSteering'; import { useTextarea, @@ -295,7 +295,27 @@ const ChatForm = memo(function ChatForm({ const composerItems = useComposerItems(conversationId, quotesEnabled); const attachTarget = useAttachTarget(conversation, disableInputs); - const dictation = useDictation({ ask: submitMessage, methods, isSubmitting }); + const { active: answerModeActive, submitText: submitAnswerText } = answerMode; + /** The same gate `onSubmit` applies: while a question pause is live the + * composer IS the answer box, so a dictated turn has to answer it rather + * than start a turn the paused run would drop. */ + const dictationAsk = useCallback( + (props) => { + if (answerModeActive && submitAnswerText(props.text)) { + return; + } + return submitMessage({ text: props.text }); + }, + [answerModeActive, submitAnswerText, submitMessage], + ); + const dictation = useDictation({ + ask: dictationAsk, + methods, + /* Answer mode leaves the run submitting while handing the composer over, + which is exactly when speech must still reach it — the send button is + enabled on the same terms. */ + isSubmitting: isSubmitting && !answerModeActive, + }); const uploadingCount = useMemo(() => { let count = 0; for (const file of files.values()) { @@ -561,7 +581,12 @@ const ChatForm = memo(function ChatForm({ isRTL={isRTL} disabled={disableInputs} agentId={conversation?.agent_id} - endpoint={endpoint} + /* The RAW endpoint, not the effective type above: the attach + destinations resolve the provider from its name, so a + custom endpoint reduced to `custom` loses the uploads its + provider actually takes (OpenRouter's video and audio). + `endpointType` beside it carries the resolved type. */ + endpoint={conversation?.endpoint} endpointType={attachTarget.endpointType} endpointFileConfig={attachTarget.endpointFileConfig} useResponsesApi={attachTarget.useResponsesApi} diff --git a/client/src/components/Chat/Input/Composer/Queue.tsx b/client/src/components/Chat/Input/Composer/Queue.tsx index 9e84e99115..bf11c313d6 100644 --- a/client/src/components/Chat/Input/Composer/Queue.tsx +++ b/client/src/components/Chat/Input/Composer/Queue.tsx @@ -37,11 +37,12 @@ export type RestoreToComposer = ( interface QueueProps { steering: SteeringControls; conversationId: string; + /** Returns whether the composer took the words; see `editToComposer`. */ onEditToComposer: ( text: string, files?: TMessage['files'], context?: QueuedMessageContext, - ) => void; + ) => boolean; onRestoreToComposer: RestoreToComposer; } @@ -217,11 +218,21 @@ function QueueRow({ type="button" aria-label={localize('com_ui_edit_message')} onClick={() => { - steering.removeQueued(message.id); - onEditToComposer(message.text, message.files, { + /* Same order as the trash below: dropped only once the words are + somewhere else. A paused question owns the composer, and removing + the row anyway would leave the message nowhere at all. */ + const taken = onEditToComposer(message.text, message.files, { quotes: message.quotes, manualSkills: message.manualSkills, }); + if (taken) { + steering.removeQueued(message.id); + return; + } + showToast({ + message: localize('com_ui_queue_edit_blocked'), + status: 'warning', + }); }} className={ICON_BTN} > 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 fb49d41014..71b379d833 100644 --- a/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx +++ b/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx @@ -198,7 +198,7 @@ describe('Queue', () => { }); it('hands the whole message to the composer to edit', () => { - const onEdit = jest.fn(); + const onEdit = jest.fn().mockReturnValue(true); renderQueue([queued({ id: 'q1', quotes: ['a quote'], manualSkills: ['writer'] })], steering, { onEditToComposer: onEdit, }); @@ -210,6 +210,20 @@ describe('Queue', () => { expect(mockRemoveQueued).toHaveBeenCalledWith('q1'); }); + /* Edit used to drop the row first and hand the words over second, so a + composer that refuses — a paused question owns it — destroyed the message + outright. Same restore-then-remove order as the trash. */ + it('keeps the message queued when the composer refuses to take it for editing', () => { + const onEdit = jest.fn().mockReturnValue(false); + renderQueue([queued({ id: 'q1' })], steering, { onEditToComposer: onEdit }); + fireEvent.click(screen.getByLabelText('com_ui_edit_message')); + expect(onEdit).toHaveBeenCalled(); + expect(mockRemoveQueued).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith( + expect.objectContaining({ message: 'com_ui_queue_edit_blocked' }), + ); + }); + /* The region is removed with the rail and re-inserted with its old text still in it, which readers announce on insertion. */ it('forgets its last announcement once the queue empties', () => { @@ -244,6 +258,70 @@ describe('Queue', () => { expect(screen.getByRole('status')).toHaveTextContent(''); }); + /* The rows move as the pointer crosses them, so the queue has already changed + by the time a drag ends. Only a drag that never landed anywhere puts it + back — and `didDrop` reports a landing even though the rows declare no + `drop` handler, which is what makes the plain `hover` sortable work. */ + describe('drag reordering', () => { + /* The handle only drags on a hover-capable pointer, and the suite's + `matchMedia` answers `false` to everything — which is the touch device + the rail deliberately refuses to drag on. */ + const realMatchMedia = window.matchMedia; + beforeEach(() => { + window.matchMedia = ((query: string) => + ({ + matches: query === '(hover: hover)', + media: query, + onchange: null, + addListener: jest.fn(), + removeListener: jest.fn(), + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + }) as unknown as MediaQueryList) as typeof window.matchMedia; + }); + afterEach(() => { + window.matchMedia = realMatchMedia; + }); + + /* jsdom has no DataTransfer, and the HTML5 backend reads one off every + event it handles. */ + const dataTransfer = () => ({ + dropEffect: 'move', + effectAllowed: 'move', + files: [], + items: [], + types: [], + setData: () => undefined, + getData: () => '', + setDragImage: () => undefined, + }); + + const dragFirstRowOntoSecond = (drop: boolean) => { + renderQueue([queued({ id: 'q1' }), queued({ id: 'q2' })]); + const grip = screen.getAllByTestId('queued-message-grip')[0]; + const secondRow = screen.getAllByTestId('queued-message-row')[1]; + const dt = dataTransfer(); + + fireEvent.dragStart(grip, { dataTransfer: dt }); + fireEvent.dragOver(secondRow, { dataTransfer: dt }); + if (drop) { + fireEvent.drop(secondRow, { dataTransfer: dt }); + } + fireEvent.dragEnd(grip, { dataTransfer: dt }); + }; + + it('keeps the new order when the drag is dropped on the rail', () => { + dragFirstRowOntoSecond(true); + expect(mockRestoreQueuedOrder).not.toHaveBeenCalled(); + }); + + it('puts the order back when the drag is abandoned', () => { + dragFirstRowOntoSecond(false); + expect(mockRestoreQueuedOrder).toHaveBeenCalledWith(['q1', 'q2']); + }); + }); + it('shows an attachment count when files ride along', () => { renderQueue([queued({ files: [{ file_id: 'f1' }, { file_id: 'f2' }] as never })]); const attachmentLabel = screen.getByText('com_ui_attachment_count:2'); diff --git a/client/src/components/Skills/dialogs/__tests__/UploadSkillDialog.spec.tsx b/client/src/components/Skills/dialogs/__tests__/UploadSkillDialog.spec.tsx index 803124adbc..7a0a91c126 100644 --- a/client/src/components/Skills/dialogs/__tests__/UploadSkillDialog.spec.tsx +++ b/client/src/components/Skills/dialogs/__tests__/UploadSkillDialog.spec.tsx @@ -1,6 +1,9 @@ +/** + * @jest-environment jsdom + */ import { fireEvent, render, screen } from '@testing-library/react'; -import type { ReactNode } from 'react'; import type { FileConfigInput } from 'librechat-data-provider'; +import type { ReactNode } from 'react'; import UploadSkillDialog from '../UploadSkillDialog'; const mockMutate = jest.fn(); @@ -71,8 +74,8 @@ jest.mock('~/utils', () => ({ })); function getFileInput(container: HTMLElement): HTMLInputElement { - const input = container.querySelector('input[type="file"]'); - if (!(input instanceof HTMLInputElement)) { + const input = container.querySelector('input[type="file"]'); + if (input == null) { throw new Error('Upload input was not rendered'); } return input; @@ -113,9 +116,7 @@ describe('UploadSkillDialog', () => { }); fireEvent.change(getFileInput(container), { - target: { - files: [file], - }, + target: { files: [file] }, }); expect(mockMutate).not.toHaveBeenCalled(); @@ -133,9 +134,7 @@ describe('UploadSkillDialog', () => { }); fireEvent.change(getFileInput(container), { - target: { - files: [file], - }, + target: { files: [file] }, }); expect(mockShowToast).not.toHaveBeenCalled(); @@ -152,9 +151,7 @@ describe('UploadSkillDialog', () => { }); fireEvent.change(getFileInput(container), { - target: { - files: [file], - }, + target: { files: [file] }, }); expect(mockShowToast).not.toHaveBeenCalled(); diff --git a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx index 0a46cacdc9..11ed2e8f0f 100644 --- a/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx @@ -210,6 +210,135 @@ describe('useSteerRecovery', () => { expect(queue).toEqual([]); }); + /* Navigating to another chat swaps the conversation held in the index slot, + so the run this steer belongs to is in no slot at all. Absence is not an + ending: the server is still injecting the accepted steer, and reading the + empty scan as "run over" queued the same words a second time. */ + it('leaves the ack pending when the user has navigated to another chat', async () => { + let settle: (value: unknown) => void = () => undefined; + mockMutateAsync.mockReturnValue(new Promise((resolve) => (settle = resolve))); + + let recovery: ReturnType | undefined; + let navigateAway: (() => void) | undefined; + let chips: unknown[] = []; + let queue: unknown[] = []; + const Tree = () => { + recovery = useSteerRecovery(CONVO_ID); + chips = useRecoilValue(store.pendingSteersByConvoId(CONVO_ID)); + queue = useRecoilValue(store.queuedMessagesByConvoId(CONVO_ID)); + const setConvo = useSetRecoilState(store.conversationByIndex(0)); + navigateAway = () => setConvo({ conversationId: 'a-different-chat' } as never); + return null; + }; + + render( + { + seedRun(snapshot, true); + snapshot.set(store.pendingSteersByConvoId(CONVO_ID), [ + { steerId: 'local-nav', text: 'still running', status: 'failed', createdAt: 2 }, + ]); + }} + > + + , + ); + + act(() => { + recovery?.retry('local-nav'); + }); + act(() => navigateAway?.()); + await act(async () => { + settle({ steerId: 'srv-nav', status: 'queued', position: 1, conversationId: CONVO_ID }); + }); + + expect(chips).toEqual([expect.objectContaining({ steerId: 'srv-nav', status: 'pending' })]); + expect(queue).toEqual([]); + }); + + /* An interrupt-steer that failed has to retry AS an interrupt: resent as an + ordinary steer it lands at the run's next tool step instead of sealing the + stream, which is not the action the chip says it is. */ + it('resubmits a failed interrupt-steer as an interrupt', async () => { + mockMutateAsync.mockResolvedValue({ + steerId: 'srv-preempt', + status: 'queued', + position: 1, + conversationId: CONVO_ID, + preempt: true, + }); + const { result } = setup(({ set }) => { + set(store.pendingSteersByConvoId(CONVO_ID), [ + { + steerId: 'local-preempt', + text: 'stop and read this', + status: 'failed', + createdAt: 6, + preempt: true, + }, + ]); + }); + act(() => { + result.current.recovery.retry('local-preempt'); + }); + await flush(); + expect(mockMutateAsync).toHaveBeenCalledWith( + expect.objectContaining({ text: 'stop and read this', preempt: true }), + ); + expect(result.current.chips).toEqual([ + expect.objectContaining({ steerId: 'srv-preempt', preempt: true }), + ]); + }); + + it('leaves an ordinary steer non-preempting on retry', async () => { + mockMutateAsync.mockResolvedValue({ + steerId: 'srv-plain', + status: 'queued', + position: 1, + conversationId: CONVO_ID, + }); + const { result } = setup(({ set }) => { + set(store.pendingSteersByConvoId(CONVO_ID), [ + { steerId: 'local-plain', text: 'just a steer', status: 'failed', createdAt: 6 }, + ]); + }); + act(() => { + result.current.recovery.retry('local-plain'); + }); + await flush(); + expect(mockMutateAsync).toHaveBeenCalledWith(expect.not.objectContaining({ preempt: true })); + }); + + /* Capability degradation is a relabel, never an error: a server without the + seal still queues the steer and echoes `preempt: false`. */ + it('follows the server down to an ordinary steer when the seal was not armed', async () => { + mockMutateAsync.mockResolvedValue({ + steerId: 'srv-degraded', + status: 'queued', + position: 1, + conversationId: CONVO_ID, + preempt: false, + }); + const { result } = setup(({ set }) => { + set(store.pendingSteersByConvoId(CONVO_ID), [ + { + steerId: 'local-degraded', + text: 'seal it', + status: 'failed', + createdAt: 6, + preempt: true, + }, + ]); + }); + act(() => { + result.current.recovery.retry('local-degraded'); + }); + await flush(); + expect(result.current.chips).toEqual([ + expect.objectContaining({ steerId: 'srv-degraded', status: 'pending', preempt: false }), + ]); + }); + /* 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 () => { diff --git a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx index 0ebaf2f219..dc03e5dbe7 100644 --- a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx @@ -305,19 +305,13 @@ describe('useSteering', () => { ); }); - it('retry resubmits a failed interrupt-steer AS an interrupt', () => { - const { result } = setup(); - act(() => { - result.current.retrySteer('chip-1', 'retry me', undefined, undefined, { preempt: true }); - }); - expect(mockMutate).toHaveBeenCalledWith( - expect.objectContaining({ text: 'retry me', preempt: true }), - expect.anything(), - ); - }); + /* Retry lives in `useSteerRecovery` now, which owns the chip actions the + thread's pending block renders; its own spec covers the preempt carry. */ it('an ordinary steer does not preempt by default', () => { - const { result } = setup(); + const { result } = setup({}, ({ set }) => { + set(store.duringRunDefaultAction, 'steer'); + }); act(() => { result.current.submitDuringRun('just steer'); }); @@ -329,6 +323,7 @@ describe('useSteering', () => { it('steerInterruptsByDefault makes the default Enter route preempt', () => { const { result } = setup({}, ({ set }) => { + set(store.duringRunDefaultAction, 'steer'); set(store.steerInterruptsByDefault, true); }); act(() => { diff --git a/client/src/hooks/Chat/useSteerRecovery.ts b/client/src/hooks/Chat/useSteerRecovery.ts index 14b37b8a0d..9b2b51df05 100644 --- a/client/src/hooks/Chat/useSteerRecovery.ts +++ b/client/src/hooks/Chat/useSteerRecovery.ts @@ -32,7 +32,10 @@ export default function useSteerRecovery(conversationId: string) { } return snapshot.getLoadable(store.isSubmittingFamily(key)).getValue() !== true; } - return true; + /* Held by no slot at all: the user navigated to another chat, which says + nothing about the run they left behind. Reading that as the end is the + same mistake as reading the unmount as the end, one step further out. */ + return false; }, [conversationId], ); @@ -97,9 +100,26 @@ export default function useSteerRecovery(conversationId: string) { run ends, which is exactly when a retry's ack tends to land: those callbacks never ran, and the chip was left saying `sending` for the rest of the conversation with the words neither sent nor queued. */ - steerMessage({ conversationId, text: steer.text, files: steer.files }) + /* `preempt` rides along: a chip that failed as an interrupt-steer has to + retry AS one. Resent without it the words land at the run's next tool + step instead of sealing the stream, which is a different action than + the one the user asked for and the chip still claims to be. */ + steerMessage({ + conversationId, + text: steer.text, + files: steer.files, + ...(steer.preempt === true && { preempt: true }), + }) .then((response) => { - acknowledgeRetry(steerId, { ...steer, steerId: response.steerId, status: 'pending' }); + acknowledgeRetry(steerId, { + ...steer, + steerId: response.steerId, + status: 'pending', + /* The server echoes what it actually armed: without the + capability it queues the steer and reports `preempt: false`, + which relabels the chip rather than failing it. */ + preempt: response.preempt === true, + }); }) .catch((error: unknown) => { const code = getSteerErrorCode(error); diff --git a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx index 16716abf34..c8ed59369e 100644 --- a/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx +++ b/client/src/hooks/Input/__tests__/useAttachItems.spec.tsx @@ -352,6 +352,29 @@ describe('useAttachItems', () => { expect(mockHandleFileChange).toHaveBeenLastCalledWith(expect.anything(), undefined); }); + /* A custom endpoint's own NAME is what identifies its provider; the + resolved `endpointType` flattens every one of them to `custom`. Handing + the palette the type in place of the name cost an OpenRouter endpoint the + video and audio it takes. */ + it('reads the provider from a custom endpoint name, not its resolved type', () => { + const acceptFor = (endpoint: string) => { + const { result } = renderAttach({ endpoint, endpointType: EModelEndpoint.custom }); + 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(); + }); + return accepts[0]; + }; + + expect(acceptFor('OpenRouter')).toContain('video/'); + expect(acceptFor('OpenRouter')).toContain('audio/'); + /* The type on its own is a document-and-image provider and nothing more. */ + expect(acceptFor(EModelEndpoint.custom)).not.toContain('video/'); + }); + it('scopes the picker to what the destination can send', () => { const { result } = renderAttach({ endpointType: EModelEndpoint.bedrock }); const input = document.createElement('input'); diff --git a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts index 4cc7f028ac..dd4a463f1a 100644 --- a/client/src/hooks/Input/__tests__/useComposerHint.spec.ts +++ b/client/src/hooks/Input/__tests__/useComposerHint.spec.ts @@ -15,11 +15,14 @@ const baseState: ComposerHintState = { enterToSend: true, }; -const hint = (overrides: Partial, isMac = true) => - composeHint({ ...baseState, ...overrides }, localize, isMac).text; +/** What `useShortcutDisplay('stopGenerating')` resolves to by default on a Mac. */ +const STOP = '⌘ ⇧ X'; + +const hint = (overrides: Partial, isMac = true, stop = STOP) => + composeHint({ ...baseState, ...overrides }, localize, isMac, stop).text; const kindOf = (overrides: Partial) => - composeHint({ ...baseState, ...overrides }, localize, true).kind; + composeHint({ ...baseState, ...overrides }, localize, true, STOP).kind; describe('composeHint', () => { it('shows discovery affordances on an untouched composer', () => { @@ -30,8 +33,18 @@ describe('composeHint', () => { expect(hint({ hasText: true })).toBe('com_ui_composer_hint_typing'); }); - it('offers stop while generating with an empty composer', () => { - expect(hint({ isSubmitting: true })).toBe('com_ui_composer_hint_stop'); + /* The line used to read "Esc to stop", which no handler anywhere implements: + stopping is bound to the `stopGenerating` shortcut, and the user can rebind + it. Naming a key the composer does not answer to is worse than naming none. */ + it('names the live stop binding while generating with an empty composer', () => { + expect(hint({ isSubmitting: true })).toBe('⌘ ⇧ X com_ui_composer_hint_stop'); + expect(hint({ isSubmitting: true }, false, 'Ctrl+Shift+X')).toBe( + 'Ctrl+Shift+X com_ui_composer_hint_stop', + ); + }); + + it('names no key at all once the binding is cleared', () => { + expect(hint({ isSubmitting: true }, true, '')).toBe('com_ui_composer_hint_running'); }); describe('during a run with text', () => { @@ -64,7 +77,7 @@ describe('composeHint', () => { it('falls back to stop when the modifiers have no text to act on', () => { expect(hint({ duringRunActive: true, hasText: false, isSubmitting: true })).toBe( - 'com_ui_composer_hint_stop', + '⌘ ⇧ X com_ui_composer_hint_stop', ); }); }); diff --git a/client/src/hooks/Input/__tests__/useComposerRestore.spec.tsx b/client/src/hooks/Input/__tests__/useComposerRestore.spec.tsx index 5741f048a2..9932880052 100644 --- a/client/src/hooks/Input/__tests__/useComposerRestore.spec.tsx +++ b/client/src/hooks/Input/__tests__/useComposerRestore.spec.tsx @@ -184,12 +184,42 @@ describe('useComposerRestore', () => { describe('editToComposer', () => { it('replaces the draft outright, unlike the guarded restore', () => { const { result, currentText } = setup({ draft: 'half a thought' }); + let taken = false; act(() => { - result.current.editToComposer('the queued message'); + taken = result.current.editToComposer('the queued message'); }); + expect(taken).toBe(true); expect(currentText()).toBe('the queued message'); }); + /* The one thing it refuses: while a question pause is live the composer is + the answer box, so a queued message dropped in here reads as a draft and + answers the tool on the next Enter instead of being edited. */ + it('refuses while a paused question owns the composer', () => { + const { result, currentText, setFiles } = setup({ answerModeActive: true }); + let taken = true; + act(() => { + taken = result.current.editToComposer('the queued message', [ + { file_id: 'f1', filename: 'notes.pdf', filepath: '/f1', type: 'application/pdf' }, + ]); + }); + expect(taken).toBe(false); + expect(currentText()).toBe(''); + expect(setFiles).not.toHaveBeenCalled(); + }); + + /* A question can start pausing the run between render and click. */ + it('refuses on a pause that started after the last render it was read in', () => { + const { result, rerender, currentText } = setup(); + rerender({ conversationId: CONVO_ID, answerModeActive: true }); + let taken = true; + act(() => { + taken = result.current.editToComposer('too late'); + }); + expect(taken).toBe(false); + expect(currentText()).toBe(''); + }); + it('restores attachments as already-uploaded entries', () => { const { result, setFiles } = setup(); act(() => { diff --git a/client/src/hooks/Input/__tests__/useDictation.spec.tsx b/client/src/hooks/Input/__tests__/useDictation.spec.tsx index c611e7ce27..70e24685c3 100644 --- a/client/src/hooks/Input/__tests__/useDictation.spec.tsx +++ b/client/src/hooks/Input/__tests__/useDictation.spec.tsx @@ -52,7 +52,11 @@ jest.mock('../../useLocalize', () => ({ const ask = jest.fn(() => true) as unknown as jest.Mock & TAskFunction; -function setup({ autoSendText = -1, draft = '' }: { autoSendText?: number; draft?: string } = {}) { +function setup({ + autoSendText = -1, + draft = '', + isSubmitting = false, +}: { autoSendText?: number; draft?: string; isSubmitting?: boolean } = {}) { let text = draft; const methods = { setValue: jest.fn((_name: string, value: string) => { @@ -73,7 +77,7 @@ function setup({ autoSendText = -1, draft = '' }: { autoSendText?: number; draft useDictation({ ask: ask as unknown as TAskFunction, methods: methods as never, - isSubmitting: false, + isSubmitting, }), { wrapper }, ); @@ -200,4 +204,91 @@ describe('useDictation', () => { expect(ask).not.toHaveBeenCalled(); expect(currentText()).toBe('my unsent draft'); }); + + /* A run in flight has nothing to send into, so the take is refused outright. + `ChatForm` reports a paused `ask_user_question` as NOT submitting for + exactly this reason: the run is still open, but the composer has become the + answer box and speech has to reach it the way typing does. */ + it('refuses a take while a turn is in flight', async () => { + const { result, rerender, currentText } = setup({ draft: '', isSubmitting: true }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => mockSetTextCallback('answer the question')); + act(() => result.current.stopAndSend()); + await settle(rerender); + + expect(ask).not.toHaveBeenCalled(); + expect(currentText()).toBe('answer the question'); + }); + + it('sends the take when the composer is free to submit', async () => { + const { result, rerender } = setup({ draft: '', isSubmitting: false }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => mockSetTextCallback('answer the question')); + act(() => result.current.stopAndSend()); + await settle(rerender); + + expect(ask).toHaveBeenCalledWith({ text: 'answer the question' }); + }); + + /* The armed send reads whatever the transcript left in the composer. A take + that produced nothing leaves the draft that was there BEFORE recording, and + sending that is the user's own unfinished words going out as if dictated. */ + describe('a take that produced nothing', () => { + it('stands the armed send down when the transcription fails', async () => { + mockSpeechEndpoint = 'external'; + const { result, rerender, currentText } = setup({ draft: 'half a thought' }); + act(() => result.current.start()); + mockIsListening = true; + mockIsLoading = true; + act(() => rerender()); + + act(() => result.current.stopAndSend()); + /* The request fails: the engine toasts, clears its loading flag, and + never reports a transcript. */ + mockIsLoading = false; + await settle(rerender); + + expect(ask).not.toHaveBeenCalled(); + expect(currentText()).toBe('half a thought'); + }); + + it('stands it down for a take too short to reach the engine', async () => { + const { result, rerender, currentText } = setup({ draft: 'half a thought' }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + + act(() => result.current.stopAndSend()); + await settle(rerender); + + expect(ask).not.toHaveBeenCalled(); + expect(currentText()).toBe('half a thought'); + }); + + /* The gate is per take, not for good. */ + it('sends the next take that does produce words', async () => { + const { result, rerender } = setup({ draft: 'half a thought' }); + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + act(() => result.current.stopAndSend()); + await settle(rerender); + expect(ask).not.toHaveBeenCalled(); + + act(() => result.current.start()); + mockIsListening = true; + act(() => rerender()); + act(() => mockSetTextCallback('this time it heard me')); + act(() => result.current.stopAndSend()); + await settle(rerender); + + expect(ask).toHaveBeenCalledWith({ text: 'half a thought this time it heard me' }); + }); + }); }); diff --git a/client/src/hooks/Input/useComposerHint.ts b/client/src/hooks/Input/useComposerHint.ts index 37a7033741..3071bff1b0 100644 --- a/client/src/hooks/Input/useComposerHint.ts +++ b/client/src/hooks/Input/useComposerHint.ts @@ -1,4 +1,5 @@ import type { LocalizeFunction } from '~/common'; +import { useShortcutDisplay } from '~/hooks/useKeyboardShortcuts'; import { isMacPlatform } from '~/utils/shortcuts'; import useLocalize from '~/hooks/useLocalize'; @@ -44,6 +45,9 @@ export function composeHint( state: ComposerHintState, localize: LocalizeFunction, isMac: boolean, + /** The live binding for `stopGenerating`, which the user can rebind or clear + * outright — so the stop line is built from it rather than naming a key. */ + stopShortcut: string, ): ComposerHint { if (state.answerModeActive) { return { text: localize('com_ui_composer_hint_answer'), kind: 'state' }; @@ -85,7 +89,15 @@ export function composeHint( } if (state.isSubmitting) { - return { text: localize('com_ui_composer_hint_stop'), kind: 'state' }; + /* Nothing to advertise when the binding has been cleared: the stop button + is right there, and naming a key that does nothing is worse than saying + only that a reply is running. */ + return { + text: stopShortcut + ? `${stopShortcut} ${localize('com_ui_composer_hint_stop')}` + : localize('com_ui_composer_hint_running'), + kind: 'state', + }; } if (state.hasText) { @@ -107,5 +119,6 @@ export function composeHint( export default function useComposerHint(state: ComposerHintState): ComposerHint { const localize = useLocalize(); - return composeHint(state, localize, isMacPlatform); + const stopShortcut = useShortcutDisplay('stopGenerating'); + return composeHint(state, localize, isMacPlatform, stopShortcut); } diff --git a/client/src/hooks/Input/useComposerRestore.ts b/client/src/hooks/Input/useComposerRestore.ts index a635b4a64f..f6650846c1 100644 --- a/client/src/hooks/Input/useComposerRestore.ts +++ b/client/src/hooks/Input/useComposerRestore.ts @@ -7,9 +7,14 @@ import type { useChatFormContext } from '~/Providers'; import store from '~/store'; export interface ComposerRestore { - /** Chip "Edit message": replaces the draft outright. The caller has already - * decided this is wanted, so nothing is guarded. */ - editToComposer: (text: string, files?: TMessage['files'], context?: QueuedMessageContext) => void; + /** Chip "Edit message": replaces the draft outright, since the caller has + * already decided that is wanted. Returns whether the composer took the + * words — only a paused question, which owns the box, refuses. */ + editToComposer: ( + text: string, + files?: TMessage['files'], + context?: QueuedMessageContext, + ) => boolean; /** The same restore for a steer whose reclaim was a round-trip, guarded * against everything that can change while it is in flight. Returns whether * the words were taken, so a refusal can be re-homed rather than dropped. */ @@ -68,11 +73,25 @@ export default function useComposerRestore({ [conversationId], ); + /** Read at call time rather than captured, so both the synchronous edit below + * and the round-trip restore further down see the pause that started while + * they were in flight. */ + const liveAnswerModeRef = useRef(answerModeActive); + liveAnswerModeRef.current = answerModeActive; + /** The text replaces the composer draft and the chip's attachments merge back * into the composer file map (already uploaded, so they restore as completed - * entries — same shape as draft recovery). */ + * entries — same shape as draft recovery). + * + * A paused `ask_user_question` is the one thing that refuses: `onSubmit` + * hands the composer's text to `answerMode.submitText` before any send + * routing, so a queued message dropped in here would leave the box on + * screen looking like a draft and answer the tool on the next Enter. */ const editToComposer = useCallback( - (text: string, chipFiles?: TMessage['files'], context?: QueuedMessageContext) => { + (text: string, chipFiles?: TMessage['files'], context?: QueuedMessageContext): boolean => { + if (liveAnswerModeRef.current) { + return false; + } methods.setValue('text', text, { shouldDirty: true }); if (chipFiles != null && chipFiles.length > 0) { setFiles((prev) => { @@ -98,6 +117,7 @@ export default function useComposerRestore({ } restoreComposerContext(context); textAreaRef.current?.focus(); + return true; }, [methods, setFiles, restoreComposerContext, textAreaRef], ); @@ -111,9 +131,6 @@ export default function useComposerRestore({ /** Same reason: attachments staged after the click must be seen. */ const liveFilesRef = useRef(files); liveFilesRef.current = files; - /** Same reason: the run can pause on `ask_user_question` mid-reclaim. */ - const liveAnswerModeRef = useRef(answerModeActive); - liveAnswerModeRef.current = answerModeActive; /** A reclaim can resolve after the composer unmounts (left the route, closed * the pane). Its refs still hold the origin chat, so the restore would pass * its checks and write into a dead form — reporting success and making the @@ -160,12 +177,6 @@ export default function useComposerRestore({ if (originConversationId !== liveConversationId) { return false; } - /** Answer mode owns the composer: `onSubmit` hands its text to - * `answerMode.submitText` before any send/steer routing, so restoring - * here would turn the steer into the tool's answer on the next Enter. */ - if (liveAnswerModeRef.current) { - return false; - } if ( (methods.getValues('text') ?? '').trim().length > 0 || (liveFilesRef.current?.size ?? 0) > 0 || @@ -173,8 +184,10 @@ export default function useComposerRestore({ ) { return false; } - editToComposer(text, steerFiles, context); - return true; + /** The answer-mode refusal lives in `editToComposer`: a question can pause + * the run mid-reclaim, and restoring into the answer box would turn the + * steer into the tool's answer on the next Enter. */ + return editToComposer(text, steerFiles, context); }, [methods, editToComposer, hasStagedContext], ); diff --git a/client/src/hooks/Input/useDictation.ts b/client/src/hooks/Input/useDictation.ts index 5f39dc65af..5b35c969ed 100644 --- a/client/src/hooks/Input/useDictation.ts +++ b/client/src/hooks/Input/useDictation.ts @@ -68,6 +68,11 @@ export default function useDictation({ * auto-send callback can reach the same transcript, and whichever gets * there first is the one that spends it. */ const spentRef = useRef(false); + /** Whether this take produced any words at all. A transcription that fails, + * or a take too short to reach the engine, reports nothing back and leaves + * the composer holding the draft that was there before recording — which an + * armed stop-and-send would then send as if it had been dictated. */ + const heardRef = useRef(false); const submit = useCallback( (text: string) => { @@ -101,6 +106,10 @@ export default function useDictation({ (text: string) => { const mode = modeRef.current; + if (text) { + heardRef.current = true; + } + if (mode === 'cancel') { /* The draft stays on the ref until a take is actually spent: an external transcription already in flight cannot be recalled, and @@ -134,6 +143,9 @@ export default function useDictation({ if (modeRef.current === 'cancel') { return; } + if (text) { + heardRef.current = true; + } setValue('text', existingTextRef.current ? `${existingTextRef.current} ${text}` : text, { shouldValidate: true, }); @@ -190,12 +202,19 @@ export default function useDictation({ return; } setPendingSend(false); + /* Nothing was heard: the request failed, or the take never reached the + engine. The engines report that themselves, so the send simply stands + down rather than sending the pre-recording draft in its place. */ + if (!heardRef.current) { + return; + } submit(getValues('text') || ''); }, [pendingSend, active, isLoading, settling, submit, getValues]); const start = useCallback(() => { modeRef.current = 'compose'; spentRef.current = false; + heardRef.current = false; setPendingSend(false); existingTextRef.current = getValues('text') || ''; startRecording(); diff --git a/client/src/hooks/MCP/useMCPServerManager.ts b/client/src/hooks/MCP/useMCPServerManager.ts index ead897c439..98bb87b972 100644 --- a/client/src/hooks/MCP/useMCPServerManager.ts +++ b/client/src/hooks/MCP/useMCPServerManager.ts @@ -114,10 +114,15 @@ export function useMCPServerManager({ [], ); - const reinitializeMutation = useReinitializeMCPServerMutation(); - const cancelOAuthMutation = useCancelMCPOAuthMutation(); + /* Destructured to the callables: react-query hands back a fresh result + object every render, and the callbacks below that depended on the whole + object were new identities each time — which is what kept this hook's + return, and `BadgeRowProvider`'s context value with it, changing on every + keystroke in the composer. */ + const { mutateAsync: reinitializeServer } = useReinitializeMCPServerMutation(); + const { mutate: cancelMCPOAuth } = useCancelMCPOAuthMutation(); - const updateUserPluginsMutation = useUpdateUserPluginsMutation({ + const { mutate: updateUserPlugins, isLoading: isUpdatingPlugins } = useUpdateUserPluginsMutation({ onSuccess: async (_data, variables) => { const isRevoke = variables.action === 'uninstall'; const message = isRevoke @@ -350,7 +355,7 @@ export function useMCPServerManager({ * attempt can never be mistaken for this attempt's outcome. */ updateServerInitState(serverName, { isInitializing: true, connectionDeferred: false }); try { - const response = await reinitializeMutation.mutateAsync(serverName); + const response = await reinitializeServer(serverName); /** Record whether this attempt deferred to a chat turn (request-scoped * server) so consumers that didn't await this call — e.g. the agent * builder behind the customUserVars config dialog — can react to it. */ @@ -411,7 +416,7 @@ export function useMCPServerManager({ }, [ updateServerInitState, - reinitializeMutation, + reinitializeServer, startServerPolling, queryClient, showToast, @@ -424,7 +429,7 @@ export function useMCPServerManager({ const cancelOAuthFlow = useCallback( (serverName: string) => { - cancelOAuthMutation.mutate(serverName, { + cancelMCPOAuth(serverName, { onSuccess: () => { cleanupServerState(serverName); Promise.all([ @@ -448,7 +453,7 @@ export function useMCPServerManager({ }, }); }, - [queryClient, cleanupServerState, showToast, localize, cancelOAuthMutation], + [queryClient, cleanupServerState, showToast, localize, cancelMCPOAuth], ); const isInitializing = useCallback( @@ -521,10 +526,10 @@ export function useMCPServerManager({ action: 'install', auth: authData, }; - updateUserPluginsMutation.mutate(payload); + updateUserPlugins(payload); } }, - [selectedToolForConfig, updateUserPluginsMutation], + [selectedToolForConfig, updateUserPlugins], ); const handleConfigRevoke = useCallback( @@ -535,11 +540,11 @@ export function useMCPServerManager({ action: 'uninstall', auth: {}, }; - updateUserPluginsMutation.mutate(payload); - /** Deselection is now handled centrally in updateUserPluginsMutation.onSuccess */ + updateUserPlugins(payload); + /** Deselection is now handled centrally in the mutation's onSuccess */ } }, - [selectedToolForConfig, updateUserPluginsMutation], + [selectedToolForConfig, updateUserPlugins], ); /** Standalone revoke function for OAuth servers - doesn't require selectedToolForConfig */ @@ -550,9 +555,9 @@ export function useMCPServerManager({ action: 'uninstall', auth: {}, }; - updateUserPluginsMutation.mutate(payload); + updateUserPlugins(payload); }, - [updateUserPluginsMutation], + [updateUserPlugins], ); const handleSave = useCallback( @@ -681,7 +686,7 @@ export function useMCPServerManager({ initialValues, onSave: handleSave, onRevoke: handleRevoke, - isSubmitting: updateUserPluginsMutation.isLoading, + isSubmitting: isUpdatingPlugins, }; }, [ selectedToolForConfig, @@ -690,41 +695,77 @@ export function useMCPServerManager({ handleDialogOpenChange, handleSave, handleRevoke, - updateUserPluginsMutation.isLoading, + isUpdatingPlugins, ]); - return { - availableMCPServers, - /** MCP servers filtered for chat menu selection (chatMenu !== false && !consumeOnly) */ - selectableServers, - availableMCPServersMap: loadedServers, - isLoading, - connectionStatus, - initializeServer, - cancelOAuthFlow, - isInitializing, - isCancellable, - isConnectionDeferred, - resetConnectionDeferred, - getOAuthUrl, - mcpValues, - setMCPValues, + /* Memoized because `BadgeRowProvider` carries this straight into its context + value: a fresh object here changed that value on every keystroke in the + composer, rebuilding the palette's whole server catalog per character. */ + return useMemo( + () => ({ + availableMCPServers, + /** MCP servers filtered for chat menu selection (chatMenu !== false && !consumeOnly) */ + selectableServers, + availableMCPServersMap: loadedServers, + isLoading, + connectionStatus, + initializeServer, + cancelOAuthFlow, + isInitializing, + isCancellable, + isConnectionDeferred, + resetConnectionDeferred, + getOAuthUrl, + mcpValues, + setMCPValues, - isPinned, - setIsPinned, - placeholderText, - toggleServerSelection, - localize, + isPinned, + setIsPinned, + placeholderText, + toggleServerSelection, + localize, - isConfigModalOpen, - handleDialogOpenChange, - selectedToolForConfig, - setSelectedToolForConfig, - handleSave, - handleRevoke, - revokeOAuthForServer, - getServerStatusIconProps, - getConfigDialogProps, - checkEffectivePermission, - }; + isConfigModalOpen, + handleDialogOpenChange, + selectedToolForConfig, + setSelectedToolForConfig, + handleSave, + handleRevoke, + revokeOAuthForServer, + getServerStatusIconProps, + getConfigDialogProps, + checkEffectivePermission, + }), + [ + availableMCPServers, + selectableServers, + loadedServers, + isLoading, + connectionStatus, + initializeServer, + cancelOAuthFlow, + isInitializing, + isCancellable, + isConnectionDeferred, + resetConnectionDeferred, + getOAuthUrl, + mcpValues, + setMCPValues, + isPinned, + setIsPinned, + placeholderText, + toggleServerSelection, + localize, + isConfigModalOpen, + handleDialogOpenChange, + selectedToolForConfig, + setSelectedToolForConfig, + handleSave, + handleRevoke, + revokeOAuthForServer, + getServerStatusIconProps, + getConfigDialogProps, + checkEffectivePermission, + ], + ); } diff --git a/client/src/hooks/Plugins/useAuthSearchTool.ts b/client/src/hooks/Plugins/useAuthSearchTool.ts index ffb156dba1..ca26f12848 100644 --- a/client/src/hooks/Plugins/useAuthSearchTool.ts +++ b/client/src/hooks/Plugins/useAuthSearchTool.ts @@ -23,7 +23,11 @@ export type SearchApiKeyFormData = { const useAuthSearchTool = (options?: { isEntityTool: boolean }) => { const queryClient = useQueryClient(); const isEntityTool = options?.isEntityTool ?? true; - const updateUserPlugins = useUpdateUserPluginsMutation({ + /* `mutate` rather than the mutation object: react-query hands back a fresh + result object on every render, and depending on it made both callbacks + below new identities each time — which travelled up through + `useSearchApiKeyForm` into `BadgeRowProvider`'s context value. */ + const { mutate: updateUserPlugins } = useUpdateUserPluginsMutation({ onMutate: (vars) => { queryClient.setQueryData([QueryKeys.toolAuth, Tools.web_search], () => { return { @@ -69,7 +73,7 @@ const useAuthSearchTool = (options?: { isEntityTool: boolean }) => { {} as Record, ); - updateUserPlugins.mutate({ + updateUserPlugins({ pluginKey: Tools.web_search, action: 'install', auth, @@ -80,7 +84,7 @@ const useAuthSearchTool = (options?: { isEntityTool: boolean }) => { ); const removeTool = useCallback(() => { - updateUserPlugins.mutate({ + updateUserPlugins({ pluginKey: Tools.web_search, action: 'uninstall', auth: {}, diff --git a/client/src/hooks/Plugins/useSearchApiKeyForm.ts b/client/src/hooks/Plugins/useSearchApiKeyForm.ts index 9133abf3af..909064c2a4 100644 --- a/client/src/hooks/Plugins/useSearchApiKeyForm.ts +++ b/client/src/hooks/Plugins/useSearchApiKeyForm.ts @@ -1,7 +1,7 @@ -import { useRef, useState, useCallback } from 'react'; +import { useRef, useMemo, useState, useCallback } from 'react'; import { useForm } from 'react-hook-form'; -import useAuthSearchTool from '~/hooks/Plugins/useAuthSearchTool'; import type { SearchApiKeyFormData } from '~/hooks/Plugins/useAuthSearchTool'; +import useAuthSearchTool from '~/hooks/Plugins/useAuthSearchTool'; export default function useSearchApiKeyForm({ onSubmit, @@ -33,13 +33,19 @@ export default function useSearchApiKeyForm({ onRevoke?.(); }, [reset, onRevoke, removeTool]); - return { - methods, - isDialogOpen, - setIsDialogOpen, - handleRevokeApiKey, - onSubmit: onSubmitHandler, - badgeTriggerRef, - menuTriggerRef, - }; + /* Memoized because `BadgeRowProvider` carries this straight into its context + value: a fresh object here changed that value on every keystroke in the + composer, which is the one thing the provider's own memo exists to stop. */ + return useMemo( + () => ({ + methods, + isDialogOpen, + setIsDialogOpen, + handleRevokeApiKey, + onSubmit: onSubmitHandler, + badgeTriggerRef, + menuTriggerRef, + }), + [methods, isDialogOpen, handleRevokeApiKey, onSubmitHandler], + ); } diff --git a/client/src/hooks/__tests__/useIsActiveItem.spec.tsx b/client/src/hooks/__tests__/useIsActiveItem.spec.tsx index c77252ce40..094fd8d5e7 100644 --- a/client/src/hooks/__tests__/useIsActiveItem.spec.tsx +++ b/client/src/hooks/__tests__/useIsActiveItem.spec.tsx @@ -1,5 +1,5 @@ /** - * @jest-environment @happy-dom/jest-environment + * @jest-environment jsdom */ import React from 'react'; import { act, render } from '@testing-library/react'; @@ -26,8 +26,7 @@ describe('useIsActiveItem', () => { await act(async () => { probe.setAttribute('data-active-item', ''); - // Allow the MutationObserver microtask to run - await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(probe.getAttribute('data-active')).toBe('true'); @@ -39,13 +38,13 @@ describe('useIsActiveItem', () => { await act(async () => { probe.setAttribute('data-active-item', ''); - await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(probe.getAttribute('data-active')).toBe('true'); await act(async () => { probe.removeAttribute('data-active-item'); - await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(probe.getAttribute('data-active')).toBe('false'); }); @@ -56,7 +55,7 @@ describe('useIsActiveItem', () => { await act(async () => { probe.setAttribute('data-something-else', 'x'); - await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); }); expect(probe.getAttribute('data-active')).toBe('false'); diff --git a/client/src/hooks/useKeyboardShortcuts.ts b/client/src/hooks/useKeyboardShortcuts.ts index 4f3c5becd3..f71b8fde44 100644 --- a/client/src/hooks/useKeyboardShortcuts.ts +++ b/client/src/hooks/useKeyboardShortcuts.ts @@ -289,6 +289,11 @@ export const EDITING_ALLOWED_SHORTCUTS: ReadonlySet = new Set( 'focusSearch', 'showShortcuts', 'submitMessage', + /* The composer keeps focus across a send, so this is where a user reads the + hint naming it and where they press it. Filtering it out as an editing + chord made the one shortcut the composer advertises the one that did + nothing; it is a no-op whenever no reply is running. */ + 'stopGenerating', ]); export type ShortcutAction = ShortcutDefinition & { diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 4c6cc1fd02..4a14f41401 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1004,11 +1004,12 @@ "com_ui_composer_hint_queue": "queue", "com_ui_composer_hint_queue_default": "Enter queues", "com_ui_composer_hint_queue_verb": "queues", + "com_ui_composer_hint_running": "Generating a reply…", "com_ui_composer_hint_send": "to send", "com_ui_composer_hint_send_now": "send now", "com_ui_composer_hint_steer": "Enter steers", "com_ui_composer_hint_steer_verb": "steers", - "com_ui_composer_hint_stop": "Esc to stop", + "com_ui_composer_hint_stop": "to stop", "com_ui_composer_hint_typing": "Enter to send · Shift+Enter for newline", "com_ui_composer_hint_uploading": "Uploading {{count}} files…", "com_ui_composer_hint_uploading_one": "Uploading {{count}} file…", @@ -1609,6 +1610,7 @@ "com_ui_question_failed_description": "The agent couldn't show this question and may retry automatically.", "com_ui_question_unanswered": "No answer was given", "com_ui_queue": "Queue", + "com_ui_queue_edit_blocked": "Finish answering the question before editing a queued message.", "com_ui_queue_moved": "Moved to {{0}} of {{1}}", "com_ui_queue_remove_blocked": "Clear the message box in this chat to take this message back.", "com_ui_queue_reorder": "Reorder message, {{0}} of {{1}}",