From 0a2f59ab8654861c80ccc64fd523d310c96ddb2e Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 16 Aug 2026 02:36:58 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8B=20fix:=20Stop=20the=20Copy=20Butto?= =?UTF-8?q?n=20From=20Clobbering=20the=20Clipboard=20(#14876)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: keep the copy button from clobbering the clipboard Join only the non-empty TEXT parts instead of deciding the separator from the raw content index, so a tool call or error after the last visible text part can no longer append a stray trailing newline. Pasting that into a terminal submitted the command on its own. Skip the copy entirely when a message serializes to nothing, such as an error-only response, rather than overwriting the clipboard with an empty string and flipping the button to its copied state. * fix: stop copy controls from reporting a copy that never happened The copy hook now reports whether it wrote to the clipboard. The shared link announcement and the redirect URI and resource URL toasts fire on that result instead of on every click, so a click that copies nothing no longer tells the user, or a screen reader, that the value was copied. Give CopyButton a disabled prop and use it for the shared link and the MCP redirect URI, whose values are empty for a beat while the share id and the created server id resolve, so the control is not a dead target. * fix: honor clipboard write failures and disable copy with nothing to copy copy-to-clipboard reports whether the write landed. Return that instead of assuming success, so a browser that refuses the fallback no longer produces a copied checkmark, an announcement, or a toast. Export hasCopyableText and use it to disable the copy action in HoverButtons and MinimalHoverButtons. An error-only or tool-only response rendered an enabled control that could not do anything, which keyboard users had no way to tell apart from a working one. Reveal's copy test relied on the old always-succeed path, since jsdom has no execCommand for the real module to call. It now mocks the write and covers both outcomes. * fix: derive copy availability from the text the hook would copy hasCopyableText checked raw text parts while the hook copies the cleaned value, so a part holding only citation markup counted as copyable and then declined the copy, leaving the enabled-but-dead control this was meant to remove. Both now run buildClipboardText, so the predicate cannot drift from the guard. That needs search results to stay accurate, which HoverButtons does not have, so copyability is decided in useMessageActions and useMessageHelpers next to the hook call that owns those inputs, and passed down. * perf: stop scanning a response that is still streaming for copyability The copyability memo invalidated on every streamed chunk and rebuilt the clipboard text from the whole accumulated response, so a long answer paid quadratic cumulative work for a copy button that stays hidden until the stream ends. The hooks now hand down a getter and HoverButtons short-circuits it while isActiveStreamingMessage is true, so nothing is scanned before the control can be used, and the result is still memoized once the response settles. * style: drop the narrating comment on the copyability gate * chore: sort client imports --- .../components/Chat/Messages/HoverButtons.tsx | 11 ++ .../components/Chat/Messages/MessageParts.tsx | 2 + .../Chat/Messages/MinimalHoverButtons.tsx | 9 +- .../Messages/__tests__/HoverButtons.spec.tsx | 52 ++++++- .../Chat/Messages/ui/MessageRender.tsx | 2 + .../ConvoOptions/SharedLinkCopyButton.tsx | 5 +- .../__tests__/ShareButton.test.tsx | 13 +- .../src/components/Messages/ContentRender.tsx | 2 + .../ApiKeys/__tests__/Reveal.spec.tsx | 20 +++ .../Sharing/GenericGrantAccessDialog.tsx | 2 +- .../MCPBuilder/MCPServerDialog/index.tsx | 18 +-- .../MCPServerDialog/sections/AuthSection.tsx | 2 +- client/src/hooks/Messages/index.ts | 1 + .../hooks/Messages/useCopyToClipboard.spec.ts | 143 +++++++++++++++++- .../src/hooks/Messages/useCopyToClipboard.ts | 114 +++++++------- .../src/hooks/Messages/useMessageActions.tsx | 10 +- .../src/hooks/Messages/useMessageHelpers.tsx | 5 +- 17 files changed, 340 insertions(+), 71 deletions(-) diff --git a/client/src/components/Chat/Messages/HoverButtons.tsx b/client/src/components/Chat/Messages/HoverButtons.tsx index 98771fd2b6..ce3e629255 100644 --- a/client/src/components/Chat/Messages/HoverButtons.tsx +++ b/client/src/components/Chat/Messages/HoverButtons.tsx @@ -22,6 +22,7 @@ type THoverButtons = { isEditing: boolean; enterEdit: (cancel?: boolean) => void; copyToClipboard: (setIsCopied: React.Dispatch>) => void; + getCanCopy: () => boolean; conversation: TConversation | null; isSubmitting: boolean; message: TMessage; @@ -43,6 +44,7 @@ type HoverButtonProps = { className?: string; buttonStyle?: string; dataTestId?: string; + disabled?: boolean; }; const extractMessageContent = (message: TMessage): string => { @@ -87,6 +89,7 @@ const HoverButton = memo( isLast = false, className = '', dataTestId, + disabled = false, }: HoverButtonProps) => { const buttonStyle = hoverButtonClasses({ isActive, isLast, className }); @@ -102,6 +105,7 @@ const HoverButton = memo( aria-label={title} className={buttonStyle} onClick={onClick} + disabled={disabled} > {icon} @@ -118,6 +122,7 @@ const HoverButtons = ({ isEditing, enterEdit, copyToClipboard, + getCanCopy, conversation, isSubmitting, message, @@ -159,6 +164,11 @@ const HoverButtons = ({ isEditableEndpoint, } = generationCapabilities; + const canCopy = useMemo( + () => !isActiveStreamingMessage && getCanCopy(), + [isActiveStreamingMessage, getCanCopy], + ); + if (!conversation) { return null; } @@ -205,6 +215,7 @@ const HoverButtons = ({ } icon={isCopied ? : } isLast={isLast} + disabled={!canCopy} className={cn( 'ml-0 flex items-center gap-1.5 text-xs', isSubmitting && isCreatedByUser diff --git a/client/src/components/Chat/Messages/MessageParts.tsx b/client/src/components/Chat/Messages/MessageParts.tsx index 12fe53c0a8..620e642eba 100644 --- a/client/src/components/Chat/Messages/MessageParts.tsx +++ b/client/src/components/Chat/Messages/MessageParts.tsx @@ -40,6 +40,7 @@ function MessageParts(props: TMessageProps) { latestMessageId, handleContinue, copyToClipboard, + getCanCopy, regenerateMessage, } = useMessageHelpers(props); @@ -142,6 +143,7 @@ function MessageParts(props: TMessageProps) { conversation={conversation ?? null} regenerate={() => regenerateMessage()} copyToClipboard={copyToClipboard} + getCanCopy={getCanCopy} handleContinue={handleContinue} latestMessageId={latestMessageId} isLast={isLast} diff --git a/client/src/components/Chat/Messages/MinimalHoverButtons.tsx b/client/src/components/Chat/Messages/MinimalHoverButtons.tsx index 5c45919842..6cca34cbb4 100644 --- a/client/src/components/Chat/Messages/MinimalHoverButtons.tsx +++ b/client/src/components/Chat/Messages/MinimalHoverButtons.tsx @@ -1,7 +1,7 @@ -import { useState } from 'react'; +import { useState, useMemo } from 'react'; import { Button, Clipboard, CheckMark, TooltipAnchor } from '@librechat/client'; import type { TMessage, SearchResultData } from 'librechat-data-provider'; -import { useLocalize, useCopyToClipboard } from '~/hooks'; +import { useLocalize, useCopyToClipboard, hasCopyableText } from '~/hooks'; type THoverButtons = { message: TMessage; @@ -16,6 +16,10 @@ export default function MinimalHoverButtons({ message, searchResults }: THoverBu content: message.content, searchResults, }); + const canCopy = useMemo( + () => hasCopyableText({ text: message.text, content: message.content, searchResults }), + [message.text, message.content, searchResults], + ); return (
@@ -33,6 +37,7 @@ export default function MinimalHoverButtons({ message, searchResults }: THoverBu : localize('com_ui_copy_to_clipboard') } className="ml-0 flex size-auto items-center gap-1.5 rounded-lg p-1.5 text-xs text-text-secondary-alt transition-colors duration-200 hover:bg-surface-hover hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-text-primary group-focus-within:opacity-100 group-hover:opacity-100 [@media(hover:hover)]:opacity-0" + disabled={!canCopy} onClick={() => copyToClipboard(setIsCopied)} > {isCopied ? ( diff --git a/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx b/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx index 0e0a24792f..a11cfed65f 100644 --- a/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx +++ b/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx @@ -3,7 +3,13 @@ import { MemoryRouter } from 'react-router-dom'; import { render, screen } from '@testing-library/react'; import { RecoilRoot, type MutableSnapshot } from 'recoil'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; -import { EModelEndpoint, type TConversation, type TMessage } from 'librechat-data-provider'; +import { + ContentTypes, + EModelEndpoint, + type TConversation, + type TMessage, +} from 'librechat-data-provider'; +import { hasCopyableText } from '~/hooks/Messages/useCopyToClipboard'; import HoverButtons from '~/components/Chat/Messages/HoverButtons'; import store from '~/store'; @@ -26,11 +32,13 @@ function renderHoverButtons({ message = userMessage, isLast = false, latestMessageId = 'assistant-1', + getCanCopy = () => hasCopyableText({ text: message.text, content: message.content }), }: { isSubmitting: boolean; message?: TMessage; isLast?: boolean; latestMessageId?: string; + getCanCopy?: () => boolean; }) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, @@ -53,6 +61,7 @@ function renderHoverButtons({ regenerate={jest.fn()} handleContinue={jest.fn()} copyToClipboard={jest.fn()} + getCanCopy={getCanCopy} latestMessageId={latestMessageId} /> @@ -123,4 +132,45 @@ describe('HoverButtons edit affordance', () => { expect(screen.getByTestId('copy-response-button')).toBeEnabled(); }); + + it('disables copy when the response serializes to nothing', () => { + const errorPartMessage = { + ...userMessage, + messageId: 'assistant-error-part', + isCreatedByUser: false, + error: true, + text: '', + content: [{ type: ContentTypes.ERROR, error: 'Deployment lookup failed' }], + } as TMessage; + + renderHoverButtons({ + isSubmitting: false, + message: errorPartMessage, + isLast: true, + latestMessageId: errorPartMessage.messageId, + }); + + expect(screen.getByTestId('copy-response-button')).toBeDisabled(); + }); + + it('never inspects a response that is still streaming', () => { + const streamingMessage = { + ...userMessage, + messageId: 'assistant-1', + isCreatedByUser: false, + text: 'partial resp', + } as TMessage; + const getCanCopy = jest.fn(() => true); + + renderHoverButtons({ + isSubmitting: true, + message: streamingMessage, + isLast: true, + latestMessageId: streamingMessage.messageId, + getCanCopy, + }); + + expect(screen.queryByTestId('copy-response-button')).toBeNull(); + expect(getCanCopy).not.toHaveBeenCalled(); + }); }); diff --git a/client/src/components/Chat/Messages/ui/MessageRender.tsx b/client/src/components/Chat/Messages/ui/MessageRender.tsx index f496fa0440..110dea397b 100644 --- a/client/src/components/Chat/Messages/ui/MessageRender.tsx +++ b/client/src/components/Chat/Messages/ui/MessageRender.tsx @@ -90,6 +90,7 @@ const MessageRender = memo(function MessageRender({ handleContinue, latestMessageId, copyToClipboard, + getCanCopy, regenerateMessage, latestMessageDepth, } = useMessageActions({ @@ -188,6 +189,7 @@ const MessageRender = memo(function MessageRender({ conversation={conversation ?? null} regenerate={handleRegenerateMessage} copyToClipboard={copyToClipboard} + getCanCopy={getCanCopy} handleContinue={handleContinue} latestMessageId={latestMessageId} handleFeedback={handleFeedback} diff --git a/client/src/components/Conversations/ConvoOptions/SharedLinkCopyButton.tsx b/client/src/components/Conversations/ConvoOptions/SharedLinkCopyButton.tsx index 07463c6cf7..f8aaeab39e 100644 --- a/client/src/components/Conversations/ConvoOptions/SharedLinkCopyButton.tsx +++ b/client/src/components/Conversations/ConvoOptions/SharedLinkCopyButton.tsx @@ -15,7 +15,10 @@ export default function SharedLinkCopyButton({ sharedLink }: { sharedLink: strin return; } - copyLink(setIsCopied); + if (!copyLink(setIsCopied)) { + return; + } + announcePolite({ message: localize('com_ui_link_copied'), isStatus: true }); }; diff --git a/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx b/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx index 0f58e986d3..14541dcdd5 100644 --- a/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx +++ b/client/src/components/Conversations/ConvoOptions/__tests__/ShareButton.test.tsx @@ -15,7 +15,7 @@ let mockShare: { shareId: 'share-1', snapshotFiles: true, }; -const mockCopyLink = jest.fn(); +const mockCopyLink = jest.fn(() => true); const mockAnnouncePolite = jest.fn(); jest.mock('librechat-data-provider/react-query', () => ({ @@ -88,6 +88,7 @@ describe('ShareButton', () => { snapshotFiles: true, }; mockCopyLink.mockClear(); + mockCopyLink.mockReturnValue(true); mockAnnouncePolite.mockClear(); }); @@ -129,6 +130,16 @@ describe('ShareButton', () => { }); }); + it('does not announce a copy that never reached the clipboard', () => { + mockCopyLink.mockReturnValue(false); + renderShareButton(); + + fireEvent.click(screen.getByRole('button', { name: 'com_ui_copy_link' })); + + expect(mockCopyLink).toHaveBeenCalledTimes(1); + expect(mockAnnouncePolite).not.toHaveBeenCalled(); + }); + it('passes file-sharing changes to the link actions', () => { renderShareButton(); diff --git a/client/src/components/Messages/ContentRender.tsx b/client/src/components/Messages/ContentRender.tsx index 3d1e265eb3..0049a10d8c 100644 --- a/client/src/components/Messages/ContentRender.tsx +++ b/client/src/components/Messages/ContentRender.tsx @@ -92,6 +92,7 @@ const ContentRender = memo(function ContentRender({ handleFeedback, latestMessageId, copyToClipboard, + getCanCopy, regenerateMessage, latestMessageDepth, } = useMessageActions({ @@ -184,6 +185,7 @@ const ContentRender = memo(function ContentRender({ conversation={conversation ?? null} regenerate={handleRegenerateMessage} copyToClipboard={copyToClipboard} + getCanCopy={getCanCopy} handleContinue={handleContinue} latestMessageId={latestMessageId} handleFeedback={handleFeedback} diff --git a/client/src/components/Nav/SettingsTabs/ApiKeys/__tests__/Reveal.spec.tsx b/client/src/components/Nav/SettingsTabs/ApiKeys/__tests__/Reveal.spec.tsx index 5577280703..6f03753eb2 100644 --- a/client/src/components/Nav/SettingsTabs/ApiKeys/__tests__/Reveal.spec.tsx +++ b/client/src/components/Nav/SettingsTabs/ApiKeys/__tests__/Reveal.spec.tsx @@ -1,8 +1,13 @@ import React from 'react'; +import copy from 'copy-to-clipboard'; import '@testing-library/jest-dom/extend-expect'; import { render, fireEvent } from 'test/layout-test-utils'; import Reveal from '../Reveal'; +/** jsdom has no execCommand, so the real module always reports a failed write */ +jest.mock('copy-to-clipboard'); +const mockCopy = copy as jest.MockedFunction; + const createdKey = { id: 'new-1', name: 'CI key', @@ -12,6 +17,11 @@ const createdKey = { }; describe('Reveal', () => { + beforeEach(() => { + mockCopy.mockReset(); + mockCopy.mockReturnValue(true); + }); + it('shows the one-time warning and the key value', () => { const { getByText, getByDisplayValue } = render( , @@ -40,6 +50,16 @@ describe('Reveal', () => { it('shows copied feedback after clicking copy', async () => { const { getByRole, findByRole } = render(); fireEvent.click(getByRole('button', { name: 'Copy' })); + expect(mockCopy).toHaveBeenCalledWith('lc-secret-full-key', { format: 'text/plain' }); expect(await findByRole('button', { name: /Copied/ })).toBeInTheDocument(); }); + + it('keeps the copy control idle when the clipboard write fails', () => { + mockCopy.mockReturnValue(false); + const { getByRole, queryByRole } = render( + , + ); + fireEvent.click(getByRole('button', { name: 'Copy' })); + expect(queryByRole('button', { name: /Copied/ })).not.toBeInTheDocument(); + }); }); diff --git a/client/src/components/Sharing/GenericGrantAccessDialog.tsx b/client/src/components/Sharing/GenericGrantAccessDialog.tsx index 7e210ba1a0..5416d5d6df 100644 --- a/client/src/components/Sharing/GenericGrantAccessDialog.tsx +++ b/client/src/components/Sharing/GenericGrantAccessDialog.tsx @@ -421,7 +421,7 @@ export default function GenericGrantAccessDialog({ variant="outline" onClick={() => { if (isCopying) return; - copyResourceUrl(setIsCopying); + if (!copyResourceUrl(setIsCopying)) return; showToast({ message: localize('com_ui_agent_url_copied'), status: 'success', diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/index.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/index.tsx index c9d3473d60..c11c5d1f07 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/index.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/index.tsx @@ -1,5 +1,12 @@ import React, { useState, useEffect } from 'react'; import { Copy, CopyCheck } from 'lucide-react'; +import { + SystemRoles, + Permissions, + ResourceType, + PermissionBits, + PermissionTypes, +} from 'librechat-data-provider'; import { Label, Input, @@ -14,13 +21,6 @@ import { OGDialogContent, OGDialogTemplate, } from '@librechat/client'; -import { - SystemRoles, - Permissions, - ResourceType, - PermissionBits, - PermissionTypes, -} from 'librechat-data-provider'; import { useAuthContext, useHasAccess, useResourcePermissions, MCPServerDefinition } from '~/hooks'; import { GenericGrantAccessDialog } from '~/components/Sharing'; import { useMCPServerForm } from './hooks/useMCPServerForm'; @@ -164,10 +164,10 @@ export default function MCPServerDialog({ variant="outline" onClick={() => { if (isCopying) return; + if (!copyLink(setIsCopying)) return; showToast({ message: localize('com_ui_copied_to_clipboard') }); - copyLink(setIsCopying); }} - disabled={isCopying} + disabled={isCopying || !redirectUri} className="p-0" aria-label={localize('com_ui_copy_link')} > diff --git a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx index 12133fce5f..180ca31582 100644 --- a/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx +++ b/client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx @@ -323,8 +323,8 @@ export default function AuthSection({ isEditMode, serverName }: AuthSectionProps type="button" onClick={() => { if (isCopying) return; + if (!copyLink(setIsCopying)) return; showToast({ message: localize('com_ui_copied_to_clipboard') }); - copyLink(setIsCopying); }} className="flex size-10 shrink-0 items-center justify-center rounded-lg border border-border-light text-text-secondary transition-colors hover:bg-surface-hover hover:text-text-primary" aria-label={localize('com_ui_copy_link')} diff --git a/client/src/hooks/Messages/index.ts b/client/src/hooks/Messages/index.ts index 7cfbff3141..1818bbc065 100644 --- a/client/src/hooks/Messages/index.ts +++ b/client/src/hooks/Messages/index.ts @@ -17,6 +17,7 @@ export { default as useMemoizedChatContext } from './useMemoizedChatContext'; export { default as useMessageProcess } from './useMessageProcess'; export { default as useMessageHelpers } from './useMessageHelpers'; export { default as useCopyToClipboard } from './useCopyToClipboard'; +export { hasCopyableText } from './useCopyToClipboard'; export { default as useContentMetadata } from './useContentMetadata'; export { default as useMessageScrolling } from './useMessageScrolling'; export { default as useScrollbarGutter } from './useScrollbarGutter'; diff --git a/client/src/hooks/Messages/useCopyToClipboard.spec.ts b/client/src/hooks/Messages/useCopyToClipboard.spec.ts index 638e5bc9ed..7a1c43b9c4 100644 --- a/client/src/hooks/Messages/useCopyToClipboard.spec.ts +++ b/client/src/hooks/Messages/useCopyToClipboard.spec.ts @@ -6,7 +6,7 @@ import type { ProcessedOrganic, TMessageContentParts, } from 'librechat-data-provider'; -import useCopyToClipboard from '~/hooks/Messages/useCopyToClipboard'; +import useCopyToClipboard, { hasCopyableText } from '~/hooks/Messages/useCopyToClipboard'; // Mock the copy-to-clipboard module jest.mock('copy-to-clipboard'); @@ -17,6 +17,7 @@ describe('useCopyToClipboard', () => { beforeEach(() => { jest.clearAllMocks(); + mockCopy.mockReturnValue(true); jest.useFakeTimers(); }); @@ -92,6 +93,89 @@ describe('useCopyToClipboard', () => { }); }); + it('does not append a trailing newline when skipped parts follow the last text part', () => { + const content = [ + { type: ContentTypes.TEXT, text: 'sudo rm nothing' }, + { type: ContentTypes.ERROR, error: 'Deployment lookup failed' }, + ] as TMessageContentParts[]; + + const { result } = renderHook(() => useCopyToClipboard({ content })); + + act(() => { + result.current(mockSetIsCopied); + }); + + expect(mockCopy).toHaveBeenCalledWith('sudo rm nothing', { format: 'text/plain' }); + }); + + it('does not add blank lines for empty text parts', () => { + const content = [ + { type: ContentTypes.TEXT, text: '' }, + { type: ContentTypes.TEXT, text: 'Only line' }, + { type: ContentTypes.TEXT, text: { value: '' } }, + ] as TMessageContentParts[]; + + const { result } = renderHook(() => useCopyToClipboard({ content })); + + act(() => { + result.current(mockSetIsCopied); + }); + + expect(mockCopy).toHaveBeenCalledWith('Only line', { format: 'text/plain' }); + }); + + it('preserves the clipboard when the message has no response text', () => { + const content = [ + { type: ContentTypes.ERROR, error: 'Something went wrong' }, + ] as TMessageContentParts[]; + + const { result } = renderHook(() => useCopyToClipboard({ content })); + + let copied: boolean | undefined; + act(() => { + copied = result.current(mockSetIsCopied); + }); + + expect(copied).toBe(false); + expect(mockCopy).not.toHaveBeenCalled(); + expect(mockSetIsCopied).not.toHaveBeenCalled(); + }); + + it('reports whether the copy reached the clipboard', () => { + const { result } = renderHook(() => useCopyToClipboard({ text: 'Copy me' })); + + let copied: boolean | undefined; + act(() => { + copied = result.current(mockSetIsCopied); + }); + + expect(copied).toBe(true); + }); + + it('does not report success when the clipboard write fails', () => { + mockCopy.mockReturnValueOnce(false); + const { result } = renderHook(() => useCopyToClipboard({ text: 'Copy me' })); + + let copied: boolean | undefined; + act(() => { + copied = result.current(mockSetIsCopied); + }); + + expect(copied).toBe(false); + expect(mockSetIsCopied).not.toHaveBeenCalled(); + }); + + it('preserves the clipboard when the message text is empty', () => { + const { result } = renderHook(() => useCopyToClipboard({ text: ' ' })); + + act(() => { + result.current(mockSetIsCopied); + }); + + expect(mockCopy).not.toHaveBeenCalled(); + expect(mockSetIsCopied).not.toHaveBeenCalled(); + }); + it('should reset isCopied after timeout', () => { const { result } = renderHook(() => useCopyToClipboard({ @@ -113,6 +197,63 @@ describe('useCopyToClipboard', () => { }); }); + describe('hasCopyableText', () => { + it('is false for a response made only of skipped parts', () => { + const content = [ + { type: ContentTypes.TOOL_CALL, tool_call: { type: 'tool_call', name: 'search' } }, + { type: ContentTypes.ERROR, error: 'Something went wrong' }, + ] as TMessageContentParts[]; + + expect(hasCopyableText({ content })).toBe(false); + }); + + it('is true when any text part carries content', () => { + const content = [ + { type: ContentTypes.ERROR, error: 'Something went wrong' }, + { type: ContentTypes.TEXT, text: 'The service is down.' }, + ] as TMessageContentParts[]; + + expect(hasCopyableText({ content })).toBe(true); + }); + + it('is false for whitespace-only text parts', () => { + const content = [{ type: ContentTypes.TEXT, text: ' \n' }] as TMessageContentParts[]; + + expect(hasCopyableText({ content })).toBe(false); + }); + + it('falls back to the message text when there are no content parts', () => { + expect(hasCopyableText({ text: 'Plain response' })).toBe(true); + expect(hasCopyableText({ text: ' ' })).toBe(false); + expect(hasCopyableText({})).toBe(false); + }); + + it('is false for text that survives only as citation markup', () => { + expect(hasCopyableText({ text: '\\ue202turn0search0' })).toBe(false); + }); + + it('is true when citation markup resolves against search results', () => { + const searchResults = { + '0': { organic: [{ link: 'https://example.com/1', title: 'Source 1' }] }, + }; + + expect(hasCopyableText({ text: '\\ue202turn0search0', searchResults })).toBe(true); + }); + + it('agrees with the copy the hook would perform', () => { + const source = { text: '\\ue202turn0search0' }; + const { result } = renderHook(() => useCopyToClipboard(source)); + + let copied: boolean | undefined; + act(() => { + copied = result.current(mockSetIsCopied); + }); + + expect(copied).toBe(hasCopyableText(source)); + expect(copied).toBe(false); + }); + }); + describe('Citation formatting', () => { const mockSearchResults: { [key: string]: SearchResultData } = { '0': { diff --git a/client/src/hooks/Messages/useCopyToClipboard.ts b/client/src/hooks/Messages/useCopyToClipboard.ts index adc3d6fd54..f2d186c0f4 100644 --- a/client/src/hooks/Messages/useCopyToClipboard.ts +++ b/client/src/hooks/Messages/useCopyToClipboard.ts @@ -1,7 +1,7 @@ import { useCallback, useEffect, useRef } from 'react'; import copy from 'copy-to-clipboard'; import { ContentTypes, SearchResultData } from 'librechat-data-provider'; -import type { TMessage } from 'librechat-data-provider'; +import type { TMessage, TMessageContentParts } from 'librechat-data-provider'; import { SPAN_REGEX, CLEANUP_REGEX, @@ -27,6 +27,18 @@ const refTypeMap: Record = { video: 'videos', }; +type ClipboardSource = Partial> & { + searchResults?: { [key: string]: SearchResultData }; +}; + +function getPartText(part: TMessageContentParts): string { + if (part?.type !== ContentTypes.TEXT) { + return ''; + } + + return typeof part.text === 'string' ? part.text : (part.text?.value ?? ''); +} + export function serializeMessageForClipboard({ text, content, @@ -35,22 +47,45 @@ export function serializeMessageForClipboard({ return text ?? ''; } - return content.reduce((acc, curr, i) => { - if (curr?.type === ContentTypes.TEXT) { - const partText = typeof curr.text === 'string' ? curr.text : (curr.text?.value ?? ''); - return acc + partText + (i === content.length - 1 ? '' : '\n'); + const parts: string[] = []; + for (const part of content) { + const partText = getPartText(part); + if (partText.length > 0) { + parts.push(partText); } - return acc; - }, ''); + } + + return parts.join('\n'); } -export default function useCopyToClipboard({ - text, - content, - searchResults, -}: Partial> & { - searchResults?: { [key: string]: SearchResultData }; -}) { +function buildClipboardText({ text, content, searchResults }: ClipboardSource): string { + const messageText = serializeMessageForClipboard({ text, content }); + + if (!searchResults || Object.keys(searchResults).length === 0) { + return messageText.replace(INVALID_CITATION_REGEX, '').replace(CLEANUP_REGEX, ''); + } + + const citationManager = processCitations(messageText, searchResults); + + if (citationManager.citations.size === 0) { + return citationManager.formattedText; + } + + const sortedCitations = Array.from(citationManager.citations.entries()).sort( + (a, b) => a[1].referenceNumber - b[1].referenceNumber, + ); + const citationList = sortedCitations + .map(([, citation]) => `[${citation.referenceNumber}] ${citation.link}\n`) + .join(''); + + return `${citationManager.formattedText}\n\nCitations:\n${citationList}`; +} + +export function hasCopyableText(source: ClipboardSource): boolean { + return buildClipboardText(source).trim().length > 0; +} + +export default function useCopyToClipboard({ text, content, searchResults }: ClipboardSource) { const copyTimeoutRef = useRef(null); useEffect(() => { @@ -62,50 +97,27 @@ export default function useCopyToClipboard({ }, []); const copyToClipboard = useCallback( - (setIsCopied: React.Dispatch>) => { + (setIsCopied: React.Dispatch>): boolean => { + const clipboardText = buildClipboardText({ text, content, searchResults }); + + if (clipboardText.trim().length === 0) { + return false; + } + + if (!copy(clipboardText, { format: 'text/plain' })) { + return false; + } + if (copyTimeoutRef.current) { clearTimeout(copyTimeoutRef.current); } + setIsCopied(true); - - const messageText = serializeMessageForClipboard({ text, content }); - - // Early return if no search data - if (!searchResults || Object.keys(searchResults).length === 0) { - // Clean up any citation markers before returning - const cleanedText = messageText - .replace(INVALID_CITATION_REGEX, '') - .replace(CLEANUP_REGEX, ''); - - copy(cleanedText, { format: 'text/plain' }); - copyTimeoutRef.current = setTimeout(() => { - setIsCopied(false); - }, 3000); - return; - } - - // Process citations and build a citation manager - const citationManager = processCitations(messageText, searchResults); - let processedText = citationManager.formattedText; - - // Add citations list at the end if we have any - if (citationManager.citations.size > 0) { - processedText += '\n\nCitations:\n'; - // Sort citations by their reference number - const sortedCitations = Array.from(citationManager.citations.entries()).sort( - (a, b) => a[1].referenceNumber - b[1].referenceNumber, - ); - - // Add each citation to the text - for (const [_, citation] of sortedCitations) { - processedText += `[${citation.referenceNumber}] ${citation.link}\n`; - } - } - - copy(processedText, { format: 'text/plain' }); copyTimeoutRef.current = setTimeout(() => { setIsCopied(false); }, 3000); + + return true; }, [text, content, searchResults], ); diff --git a/client/src/hooks/Messages/useMessageActions.tsx b/client/src/hooks/Messages/useMessageActions.tsx index 590ba6a40e..f042441526 100644 --- a/client/src/hooks/Messages/useMessageActions.tsx +++ b/client/src/hooks/Messages/useMessageActions.tsx @@ -10,10 +10,10 @@ import { isAssistantsEndpoint, TUpdateFeedbackRequest, } from 'librechat-data-provider'; -import type { TMessageProps } from '~/common'; import type { TMessageChatContext } from '~/common/types'; +import type { TMessageProps } from '~/common'; import { useAssistantsMapContext, useAgentsMapContext } from '~/Providers'; -import useCopyToClipboard from './useCopyToClipboard'; +import useCopyToClipboard, { hasCopyableText } from './useCopyToClipboard'; import { useAuthContext } from '~/hooks/AuthContext'; import { useGetAddedConvo } from '~/hooks/Chat'; import { useLocalize } from '~/hooks'; @@ -123,6 +123,11 @@ export default function useMessageActions(props: TMessageActions) { const copyToClipboard = useCopyToClipboard({ text, content, searchResults }); + const getCanCopy = useCallback( + () => hasCopyableText({ text, content, searchResults }), + [text, content, searchResults], + ); + const messageLabel = useMemo(() => { if (message?.isCreatedByUser === true) { return UsernameDisplay ? (user?.name ?? '') || user?.username : localize('com_user_message'); @@ -173,6 +178,7 @@ export default function useMessageActions(props: TMessageActions) { index, agent, feedback, + getCanCopy, assistant, enterEdit, conversation, diff --git a/client/src/hooks/Messages/useMessageHelpers.tsx b/client/src/hooks/Messages/useMessageHelpers.tsx index fb92c17e8e..3d85cd9388 100644 --- a/client/src/hooks/Messages/useMessageHelpers.tsx +++ b/client/src/hooks/Messages/useMessageHelpers.tsx @@ -3,7 +3,7 @@ import throttle from 'lodash/throttle'; import { isAssistantsEndpoint, isAgentsEndpoint } from 'librechat-data-provider'; import type { TMessageProps } from '~/common'; import { useMessagesViewContext, useAssistantsMapContext, useAgentsMapContext } from '~/Providers'; -import useCopyToClipboard from './useCopyToClipboard'; +import useCopyToClipboard, { hasCopyableText } from './useCopyToClipboard'; import { useGetAddedConvo } from '~/hooks/Chat'; import { logger } from '~/utils'; @@ -83,6 +83,8 @@ export default function useMessageHelpers(props: TMessageProps) { const copyToClipboard = useCopyToClipboard({ text, content }); + const getCanCopy = useCallback(() => hasCopyableText({ text, content }), [text, content]); + return { ask, edit, @@ -90,6 +92,7 @@ export default function useMessageHelpers(props: TMessageProps) { index, isLast, assistant, + getCanCopy, enterEdit, conversation, isSubmitting,