From 083042e56cdd4b99fbda254b2e253be559a109be Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 26 Mar 2026 16:40:37 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=9D=20fix:=20Safe=20Hook=20Fallbacks?= =?UTF-8?q?=20for=20Tool-Call=20Components=20in=20Search=20Route=20(#12423?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add useOptionalMessagesOperations hook for context-safe message operations Add a variant of useMessagesOperations that returns no-op functions when MessagesViewProvider is absent instead of throwing, enabling shared components to render safely outside the chat route. * fix: use optional message operations in ToolCallInfo and UIResourceCarousel Switch ToolCallInfo and UIResourceCarousel from useMessagesOperations to useOptionalMessagesOperations so they no longer crash when rendered in the /search route, which lacks MessagesViewProvider. * fix: update test mocks to use useOptionalMessagesOperations * fix: consolidate noops and narrow useMemo dependency in useOptionalMessagesOperations - Replace three noop variants (noopAsync, noopReturn, noop) with a single `const noop = () => undefined` that correctly returns void/undefined - Destructure individual fields from context before the useMemo so the dependency array tracks stable operation references, not the full context object (avoiding unnecessary re-renders on unrelated state changes) - Add useOptionalMessagesConversation for components that need conversation data outside MessagesViewProvider * fix: use optional hooks in MCPUIResource components to prevent search crash MCPUIResource and MCPUIResourceCarousel render inside Markdown prose and can appear in the /search route. Switch them from the strict useMessagesOperations/useMessagesConversation hooks to the optional variants that return safe defaults when MessagesViewProvider is absent. * test: update test mocks for optional hook renames * fix: update ToolCallInfo and UIResourceCarousel test mocks to useOptionalMessagesOperations * fix: use optional message operations in useConversationUIResources useConversationUIResources internally called the strict useMessagesOperations(), which still threw when MCPUIResource rendered outside MessagesViewProvider. Switch to useOptionalMessagesOperations so the entire MCPUIResource render chain is safe in the /search route. * style: fix import order per project conventions * fix: replace as-unknown-as casts with typed NOOP_OPS stubs - Define OptionalMessagesOps type and NOOP_OPS constant with properly typed no-op functions, eliminating all `as unknown as T` casts - Use conversationId directly from useOptionalMessagesConversation instead of re-deriving it from conversation object - Update JSDoc to reflect search route support * test: add no-provider regression tests for optional message hooks Verify useOptionalMessagesOperations and useOptionalMessagesConversation return safe defaults when rendered outside MessagesViewProvider, covering the core crash path this PR fixes. --- client/src/Providers/MessagesViewContext.tsx | 49 +++++++++++++++++ .../__tests__/MessagesViewContext.spec.tsx | 53 +++++++++++++++++++ .../Chat/Messages/Content/ToolCallInfo.tsx | 6 +-- .../Messages/Content/UIResourceCarousel.tsx | 4 +- .../Content/__tests__/Markdown.mcpui.test.tsx | 18 ++++--- .../Content/__tests__/ToolCallInfo.test.tsx | 2 +- .../__tests__/UIResourceCarousel.test.tsx | 4 +- .../MCPUIResource/MCPUIResource.tsx | 17 +++--- .../MCPUIResource/MCPUIResourceCarousel.tsx | 17 +++--- .../__tests__/MCPUIResource.test.tsx | 14 +++-- .../__tests__/MCPUIResourceCarousel.test.tsx | 14 +++-- .../Messages/useConversationUIResources.ts | 4 +- 12 files changed, 153 insertions(+), 49 deletions(-) create mode 100644 client/src/Providers/__tests__/MessagesViewContext.spec.tsx diff --git a/client/src/Providers/MessagesViewContext.tsx b/client/src/Providers/MessagesViewContext.tsx index f1cae204a4..c44972918c 100644 --- a/client/src/Providers/MessagesViewContext.tsx +++ b/client/src/Providers/MessagesViewContext.tsx @@ -140,6 +140,55 @@ export function useMessagesOperations() { ); } +type OptionalMessagesOps = Pick< + MessagesViewContextValue, + 'ask' | 'regenerate' | 'handleContinue' | 'getMessages' | 'setMessages' +>; + +const NOOP_OPS: OptionalMessagesOps = { + ask: () => {}, + regenerate: () => {}, + handleContinue: () => {}, + getMessages: () => undefined, + setMessages: () => {}, +}; + +/** + * Hook for components that need message operations but may render outside MessagesViewProvider + * (e.g. the /search route). Returns no-op stubs when the provider is absent — UI actions will + * be silently discarded rather than crashing. Callers must use optional chaining on + * `getMessages()` results, as it returns `undefined` outside the provider. + */ +export function useOptionalMessagesOperations(): OptionalMessagesOps { + const context = useContext(MessagesViewContext); + const ask = context?.ask; + const regenerate = context?.regenerate; + const handleContinue = context?.handleContinue; + const getMessages = context?.getMessages; + const setMessages = context?.setMessages; + return useMemo( + () => ({ + ask: ask ?? NOOP_OPS.ask, + regenerate: regenerate ?? NOOP_OPS.regenerate, + handleContinue: handleContinue ?? NOOP_OPS.handleContinue, + getMessages: getMessages ?? NOOP_OPS.getMessages, + setMessages: setMessages ?? NOOP_OPS.setMessages, + }), + [ask, regenerate, handleContinue, getMessages, setMessages], + ); +} + +/** + * Hook for components that need conversation data but may render outside MessagesViewProvider + * (e.g. the /search route). Returns `undefined` for both fields when the provider is absent. + */ +export function useOptionalMessagesConversation() { + const context = useContext(MessagesViewContext); + const conversation = context?.conversation; + const conversationId = context?.conversationId; + return useMemo(() => ({ conversation, conversationId }), [conversation, conversationId]); +} + /** Hook for components that only need message state */ export function useMessagesState() { const { index, latestMessageId, latestMessageDepth, setLatestMessage } = useMessagesViewContext(); diff --git a/client/src/Providers/__tests__/MessagesViewContext.spec.tsx b/client/src/Providers/__tests__/MessagesViewContext.spec.tsx new file mode 100644 index 0000000000..88cd6f702d --- /dev/null +++ b/client/src/Providers/__tests__/MessagesViewContext.spec.tsx @@ -0,0 +1,53 @@ +import { renderHook } from '@testing-library/react'; +import { + useOptionalMessagesOperations, + useOptionalMessagesConversation, +} from '../MessagesViewContext'; + +describe('useOptionalMessagesOperations', () => { + it('returns noop stubs when rendered outside MessagesViewProvider', () => { + const { result } = renderHook(() => useOptionalMessagesOperations()); + + expect(result.current.ask).toBeInstanceOf(Function); + expect(result.current.regenerate).toBeInstanceOf(Function); + expect(result.current.handleContinue).toBeInstanceOf(Function); + expect(result.current.getMessages).toBeInstanceOf(Function); + expect(result.current.setMessages).toBeInstanceOf(Function); + }); + + it('noop stubs do not throw when called', () => { + const { result } = renderHook(() => useOptionalMessagesOperations()); + + expect(() => result.current.ask({} as never)).not.toThrow(); + expect(() => result.current.regenerate({} as never)).not.toThrow(); + expect(() => result.current.handleContinue({} as never)).not.toThrow(); + expect(() => result.current.setMessages([])).not.toThrow(); + }); + + it('getMessages returns undefined outside the provider', () => { + const { result } = renderHook(() => useOptionalMessagesOperations()); + expect(result.current.getMessages()).toBeUndefined(); + }); + + it('returns stable references across re-renders', () => { + const { result, rerender } = renderHook(() => useOptionalMessagesOperations()); + const first = result.current; + rerender(); + expect(result.current).toBe(first); + }); +}); + +describe('useOptionalMessagesConversation', () => { + it('returns undefined fields when rendered outside MessagesViewProvider', () => { + const { result } = renderHook(() => useOptionalMessagesConversation()); + expect(result.current.conversation).toBeUndefined(); + expect(result.current.conversationId).toBeUndefined(); + }); + + it('returns stable references across re-renders', () => { + const { result, rerender } = renderHook(() => useOptionalMessagesConversation()); + const first = result.current; + rerender(); + expect(result.current).toBe(first); + }); +}); diff --git a/client/src/components/Chat/Messages/Content/ToolCallInfo.tsx b/client/src/components/Chat/Messages/Content/ToolCallInfo.tsx index 59a564be4d..79ac78dbb2 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallInfo.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallInfo.tsx @@ -3,11 +3,11 @@ import { ChevronDown } from 'lucide-react'; import { Tools } from 'librechat-data-provider'; import { UIResourceRenderer } from '@mcp-ui/client'; import type { TAttachment, UIResource } from 'librechat-data-provider'; +import { useOptionalMessagesOperations } from '~/Providers'; import { useLocalize, useExpandCollapse } from '~/hooks'; import UIResourceCarousel from './UIResourceCarousel'; -import { useMessagesOperations } from '~/Providers'; -import { OutputRenderer } from './ToolOutput'; import { handleUIAction, cn } from '~/utils'; +import { OutputRenderer } from './ToolOutput'; function isSimpleObject(obj: unknown): obj is Record { if (typeof obj !== 'object' || obj === null || Array.isArray(obj)) { @@ -102,7 +102,7 @@ export default function ToolCallInfo({ attachments?: TAttachment[]; }) { const localize = useLocalize(); - const { ask } = useMessagesOperations(); + const { ask } = useOptionalMessagesOperations(); const [showParams, setShowParams] = useState(false); const { style: paramsExpandStyle, ref: paramsExpandRef } = useExpandCollapse(showParams); diff --git a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx index c0829e5ad9..4cafa643c6 100644 --- a/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx +++ b/client/src/components/Chat/Messages/Content/UIResourceCarousel.tsx @@ -1,7 +1,7 @@ import React, { useState } from 'react'; import { UIResourceRenderer } from '@mcp-ui/client'; import type { UIResource } from 'librechat-data-provider'; -import { useMessagesOperations } from '~/Providers'; +import { useOptionalMessagesOperations } from '~/Providers'; import { handleUIAction } from '~/utils'; interface UIResourceCarouselProps { @@ -13,7 +13,7 @@ const UIResourceCarousel: React.FC = React.memo(({ uiRe const [showRightArrow, setShowRightArrow] = useState(true); const [isContainerHovered, setIsContainerHovered] = useState(false); const scrollContainerRef = React.useRef(null); - const { ask } = useMessagesOperations(); + const { ask } = useOptionalMessagesOperations(); const handleScroll = React.useCallback(() => { if (!scrollContainerRef.current) return; diff --git a/client/src/components/Chat/Messages/Content/__tests__/Markdown.mcpui.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/Markdown.mcpui.test.tsx index 6df66c9e15..6ca06056fa 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/Markdown.mcpui.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/Markdown.mcpui.test.tsx @@ -3,7 +3,11 @@ import { render, screen } from '@testing-library/react'; import Markdown from '../Markdown'; import { RecoilRoot } from 'recoil'; import { UI_RESOURCE_MARKER } from '~/components/MCPUIResource/plugin'; -import { useMessageContext, useMessagesConversation, useMessagesOperations } from '~/Providers'; +import { + useMessageContext, + useOptionalMessagesConversation, + useOptionalMessagesOperations, +} from '~/Providers'; import { useGetMessagesByConvoId } from '~/data-provider'; import { useLocalize } from '~/hooks'; @@ -12,8 +16,8 @@ import { useLocalize } from '~/hooks'; jest.mock('~/Providers', () => ({ ...jest.requireActual('~/Providers'), useMessageContext: jest.fn(), - useMessagesConversation: jest.fn(), - useMessagesOperations: jest.fn(), + useOptionalMessagesConversation: jest.fn(), + useOptionalMessagesOperations: jest.fn(), })); jest.mock('~/data-provider'); jest.mock('~/hooks'); @@ -26,11 +30,11 @@ jest.mock('@mcp-ui/client', () => ({ })); const mockUseMessageContext = useMessageContext as jest.MockedFunction; -const mockUseMessagesConversation = useMessagesConversation as jest.MockedFunction< - typeof useMessagesConversation +const mockUseMessagesConversation = useOptionalMessagesConversation as jest.MockedFunction< + typeof useOptionalMessagesConversation >; -const mockUseMessagesOperations = useMessagesOperations as jest.MockedFunction< - typeof useMessagesOperations +const mockUseMessagesOperations = useOptionalMessagesOperations as jest.MockedFunction< + typeof useOptionalMessagesOperations >; const mockUseGetMessagesByConvoId = useGetMessagesByConvoId as jest.MockedFunction< typeof useGetMessagesByConvoId diff --git a/client/src/components/Chat/Messages/Content/__tests__/ToolCallInfo.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ToolCallInfo.test.tsx index 4a4d80ae8d..38b792ccae 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ToolCallInfo.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ToolCallInfo.test.tsx @@ -25,7 +25,7 @@ jest.mock('~/hooks', () => ({ })); jest.mock('~/Providers', () => ({ - useMessagesOperations: () => ({ + useOptionalMessagesOperations: () => ({ ask: jest.fn(), }), })); diff --git a/client/src/components/Chat/Messages/Content/__tests__/UIResourceCarousel.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/UIResourceCarousel.test.tsx index 6d208c2cf2..6e472e3f49 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/UIResourceCarousel.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/UIResourceCarousel.test.tsx @@ -13,10 +13,10 @@ jest.mock('@mcp-ui/client', () => ({ ), })); -// Mock useMessagesOperations hook +// Mock useOptionalMessagesOperations hook const mockAsk = jest.fn(); jest.mock('~/Providers', () => ({ - useMessagesOperations: () => ({ + useOptionalMessagesOperations: () => ({ ask: mockAsk, }), })); diff --git a/client/src/components/MCPUIResource/MCPUIResource.tsx b/client/src/components/MCPUIResource/MCPUIResource.tsx index ddf65c4388..692db889c9 100644 --- a/client/src/components/MCPUIResource/MCPUIResource.tsx +++ b/client/src/components/MCPUIResource/MCPUIResource.tsx @@ -1,8 +1,8 @@ import React from 'react'; import { UIResourceRenderer } from '@mcp-ui/client'; -import { handleUIAction } from '~/utils'; +import { useOptionalMessagesConversation, useOptionalMessagesOperations } from '~/Providers'; import { useConversationUIResources } from '~/hooks/Messages/useConversationUIResources'; -import { useMessagesConversation, useMessagesOperations } from '~/Providers'; +import { handleUIAction } from '~/utils'; import { useLocalize } from '~/hooks'; interface MCPUIResourceProps { @@ -13,19 +13,14 @@ interface MCPUIResourceProps { }; } -/** - * Component that renders an MCP UI resource based on its resource ID. - * Works in both main app and share view. - */ +/** Renders an MCP UI resource based on its resource ID. Works in chat, share, and search views. */ export function MCPUIResource(props: MCPUIResourceProps) { const { resourceId } = props.node.properties; const localize = useLocalize(); - const { ask } = useMessagesOperations(); - const { conversation } = useMessagesConversation(); + const { ask } = useOptionalMessagesOperations(); + const { conversationId } = useOptionalMessagesConversation(); - const conversationResourceMap = useConversationUIResources( - conversation?.conversationId ?? undefined, - ); + const conversationResourceMap = useConversationUIResources(conversationId ?? undefined); const uiResource = conversationResourceMap.get(resourceId ?? ''); diff --git a/client/src/components/MCPUIResource/MCPUIResourceCarousel.tsx b/client/src/components/MCPUIResource/MCPUIResourceCarousel.tsx index cf32318491..ba81a2f153 100644 --- a/client/src/components/MCPUIResource/MCPUIResourceCarousel.tsx +++ b/client/src/components/MCPUIResource/MCPUIResourceCarousel.tsx @@ -1,8 +1,8 @@ import React, { useMemo } from 'react'; -import { useConversationUIResources } from '~/hooks/Messages/useConversationUIResources'; -import { useMessagesConversation } from '~/Providers'; -import UIResourceCarousel from '../Chat/Messages/Content/UIResourceCarousel'; import type { UIResource } from 'librechat-data-provider'; +import { useConversationUIResources } from '~/hooks/Messages/useConversationUIResources'; +import UIResourceCarousel from '../Chat/Messages/Content/UIResourceCarousel'; +import { useOptionalMessagesConversation } from '~/Providers'; interface MCPUIResourceCarouselProps { node: { @@ -12,16 +12,11 @@ interface MCPUIResourceCarouselProps { }; } -/** - * Component that renders multiple MCP UI resources in a carousel. - * Works in both main app and share view. - */ +/** Renders multiple MCP UI resources in a carousel. Works in chat, share, and search views. */ export function MCPUIResourceCarousel(props: MCPUIResourceCarouselProps) { - const { conversation } = useMessagesConversation(); + const { conversationId } = useOptionalMessagesConversation(); - const conversationResourceMap = useConversationUIResources( - conversation?.conversationId ?? undefined, - ); + const conversationResourceMap = useConversationUIResources(conversationId ?? undefined); const uiResources = useMemo(() => { const { resourceIds = [] } = props.node.properties; diff --git a/client/src/components/MCPUIResource/__tests__/MCPUIResource.test.tsx b/client/src/components/MCPUIResource/__tests__/MCPUIResource.test.tsx index 53896bb6fe..c37b6d5d51 100644 --- a/client/src/components/MCPUIResource/__tests__/MCPUIResource.test.tsx +++ b/client/src/components/MCPUIResource/__tests__/MCPUIResource.test.tsx @@ -2,7 +2,11 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { RecoilRoot } from 'recoil'; import { MCPUIResource } from '../MCPUIResource'; -import { useMessageContext, useMessagesConversation, useMessagesOperations } from '~/Providers'; +import { + useMessageContext, + useOptionalMessagesConversation, + useOptionalMessagesOperations, +} from '~/Providers'; import { useLocalize } from '~/hooks'; import { handleUIAction } from '~/utils'; @@ -22,11 +26,11 @@ jest.mock('@mcp-ui/client', () => ({ })); const mockUseMessageContext = useMessageContext as jest.MockedFunction; -const mockUseMessagesConversation = useMessagesConversation as jest.MockedFunction< - typeof useMessagesConversation +const mockUseMessagesConversation = useOptionalMessagesConversation as jest.MockedFunction< + typeof useOptionalMessagesConversation >; -const mockUseMessagesOperations = useMessagesOperations as jest.MockedFunction< - typeof useMessagesOperations +const mockUseMessagesOperations = useOptionalMessagesOperations as jest.MockedFunction< + typeof useOptionalMessagesOperations >; const mockUseLocalize = useLocalize as jest.MockedFunction; const mockHandleUIAction = handleUIAction as jest.MockedFunction; diff --git a/client/src/components/MCPUIResource/__tests__/MCPUIResourceCarousel.test.tsx b/client/src/components/MCPUIResource/__tests__/MCPUIResourceCarousel.test.tsx index a9f7962ab0..9a5ca934a0 100644 --- a/client/src/components/MCPUIResource/__tests__/MCPUIResourceCarousel.test.tsx +++ b/client/src/components/MCPUIResource/__tests__/MCPUIResourceCarousel.test.tsx @@ -2,7 +2,11 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; import { RecoilRoot } from 'recoil'; import { MCPUIResourceCarousel } from '../MCPUIResourceCarousel'; -import { useMessageContext, useMessagesConversation, useMessagesOperations } from '~/Providers'; +import { + useMessageContext, + useOptionalMessagesConversation, + useOptionalMessagesOperations, +} from '~/Providers'; // Mock dependencies jest.mock('~/Providers'); @@ -19,11 +23,11 @@ jest.mock('../../Chat/Messages/Content/UIResourceCarousel', () => ({ })); const mockUseMessageContext = useMessageContext as jest.MockedFunction; -const mockUseMessagesConversation = useMessagesConversation as jest.MockedFunction< - typeof useMessagesConversation +const mockUseMessagesConversation = useOptionalMessagesConversation as jest.MockedFunction< + typeof useOptionalMessagesConversation >; -const mockUseMessagesOperations = useMessagesOperations as jest.MockedFunction< - typeof useMessagesOperations +const mockUseMessagesOperations = useOptionalMessagesOperations as jest.MockedFunction< + typeof useOptionalMessagesOperations >; describe('MCPUIResourceCarousel', () => { diff --git a/client/src/hooks/Messages/useConversationUIResources.ts b/client/src/hooks/Messages/useConversationUIResources.ts index 2333f64e5f..28e9aa035a 100644 --- a/client/src/hooks/Messages/useConversationUIResources.ts +++ b/client/src/hooks/Messages/useConversationUIResources.ts @@ -2,7 +2,7 @@ import { useMemo } from 'react'; import { useRecoilValue } from 'recoil'; import { Tools } from 'librechat-data-provider'; import type { TAttachment, UIResource } from 'librechat-data-provider'; -import { useMessagesOperations } from '~/Providers'; +import { useOptionalMessagesOperations } from '~/Providers'; import store from '~/store'; /** @@ -16,7 +16,7 @@ import store from '~/store'; export function useConversationUIResources( conversationId: string | undefined, ): Map { - const { getMessages } = useMessagesOperations(); + const { getMessages } = useOptionalMessagesOperations(); const conversationAttachmentsMap = useRecoilValue( store.conversationAttachmentsSelector(conversationId),