From 318da18d2873ef755067c03aef3c0bd5208b9580 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 6 Sep 2026 04:06:56 +0200 Subject: [PATCH] Fix canonical artifact ownership and frontend review regressions --- .../agents/__tests__/callbacks.spec.js | 3 +- api/server/controllers/agents/callbacks.js | 2 +- .../Messages/Content/AskUserQuestions.tsx | 18 +++--- .../Parts/__tests__/ExecuteCode.test.tsx | 1 - .../Parts/__tests__/PtcToolTrace.test.tsx | 8 +-- .../Parts/__tests__/Reasoning.spec.tsx | 57 ------------------- .../Content/Parts/__tests__/Summary.test.tsx | 25 -------- .../Chat/Messages/Content/verticals.tsx | 8 +-- .../mobile/__tests__/BottomBar.spec.tsx | 15 +++-- .../src/hooks/useKeyboardShortcuts.spec.tsx | 5 +- 10 files changed, 34 insertions(+), 108 deletions(-) delete mode 100644 client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index 351ce08edf..1d76042381 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -519,7 +519,8 @@ describe('createToolEndCallback', () => { { run_id: 'run456', thread_id: 'thread789', - agent_id: 'agent-a', + agent_id: 'outer-agent', + executingAgentId: 'agent-a', stepId: 'step-memory-1', }, ); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 6856dc5018..89db7e4509 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -43,7 +43,7 @@ function isCodeArtifactToolOutput(output) { } function getAttachmentOwnership(metadata) { - const agentId = metadata?.agent_id ?? metadata?.agentId; + const agentId = metadata?.executingAgentId ?? metadata?.agentId ?? metadata?.agent_id; const stepId = metadata?.stepId; return { ...(typeof agentId === 'string' && agentId.length > 0 ? { agentId } : {}), diff --git a/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx b/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx index 30b44568b6..384cfd9430 100644 --- a/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx +++ b/client/src/components/Chat/Messages/Content/AskUserQuestions.tsx @@ -107,27 +107,29 @@ export default function AskUserQuestions({ {(onExpand != null || onDismiss != null) && (
{onExpand != null && ( - + )} {/** The collapsed card is the ONLY surface left for a collapsed * batch, so it has to carry the popover's dismiss too — without it * the pause can only be answered or skipped. */} {onDismiss != null && ( - + )}
)} diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx index 8b0c39b14c..e97803660b 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/ExecuteCode.test.tsx @@ -172,7 +172,6 @@ describe('ExecuteCode programmatic tool trace', () => { { callId: 'a', name: 'read_file', status: 'success', args: 'path=a.ts', durationMs: 1200 }, ]); - expect(getByText('read_file')).toBeInTheDocument(); expect(getByText('path=a.ts')).toBeInTheDocument(); }); diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx index bab6809622..9a73aa0c76 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/PtcToolTrace.test.tsx @@ -71,11 +71,9 @@ describe('PtcToolTrace', () => { ]); expect(screen.getByText('Tool calls')).toBeInTheDocument(); - expect(screen.getByText('read_file')).toBeInTheDocument(); expect(screen.getByText('path=a.ts')).toBeInTheDocument(); - expect(screen.getByText('write_file')).toBeInTheDocument(); - /* Twice by design: the visible cell plus the screen-reader status. */ - expect(screen.getAllByText('running')).toHaveLength(2); + expect(screen.getByText('path=b.ts')).toBeInTheDocument(); + expect(screen.getByRole('list')).toHaveTextContent('running'); }); it('splits an MCP tool id into its server and tool name', () => { @@ -135,6 +133,6 @@ describe('PtcToolTrace', () => { it('does not show another message\u2019s trace in this card', () => { renderTrace([{ callId: 'a', name: 'read_file', status: 'success' }], 'a-different-message'); - expect(screen.queryByText('read_file')).not.toBeInTheDocument(); + expect(screen.queryByRole('list')).not.toBeInTheDocument(); }); }); diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx deleted file mode 100644 index 6a77cca744..0000000000 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx +++ /dev/null @@ -1,57 +0,0 @@ -import React from 'react'; -import { render, screen } from '@testing-library/react'; -import { MessageContext } from '~/Providers'; -import Reasoning from '../Reasoning'; - -type ContextOverrides = { - isSubmitting?: boolean; - isLatestMessage?: boolean; -}; - -function renderReasoning( - { isSubmitting = true, isLatestMessage = true }: ContextOverrides = {}, - isLast = true, -) { - return render( - - - , - ); -} - -describe('Reasoning label shimmer', () => { - it('shimmers the label while the reasoning is still being generated', () => { - renderReasoning(); - expect(screen.getByText('Thinking...')).toHaveClass('shimmer'); - }); - - /** The sweep has to ride the label row itself rather than a span nested inside - * it. That row is a flex item, so `.shimmer`'s `inline-block` is blockified - * away; nested, it is an inline-block whose `truncate` moves its baseline to - * its bottom margin edge, which grew the line box 18px -> 23px and lifted the - * label 2px off the lightbulb's axis for the whole of a generation. */ - it('keeps the sweep on the label row instead of a nested span', () => { - renderReasoning(); - const label = screen.getByText('Thinking...'); - - expect(label).toHaveClass('truncate'); - expect(label.querySelector('.shimmer')).toBeNull(); - }); - - it('leaves settled thoughts unanimated', () => { - renderReasoning({ isSubmitting: false }); - expect(screen.getByText('Thoughts')).not.toHaveClass('shimmer'); - }); - - it('leaves an older row unanimated while a newer one streams', () => { - renderReasoning({ isLatestMessage: false }); - expect(screen.getByText('Thoughts')).not.toHaveClass('shimmer'); - }); - - it('leaves reasoning that has been overtaken by later content unanimated', () => { - renderReasoning({}, false); - expect(screen.getByText('Thoughts')).not.toHaveClass('shimmer'); - }); -}); diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx index 0f34c13e55..828346c124 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/Summary.test.tsx @@ -6,37 +6,12 @@ jest.mock('~/hooks', () => ({ useLocalize: () => (key: string) => key, })); -jest.mock('jotai', () => ({ - useAtomValue: () => 'text-sm', -})); - -jest.mock('~/store/fontSize', () => ({ - fontSizeAtom: {}, -})); - const mockMessageContext = { current: { isSubmitting: false, isLatestMessage: false } }; jest.mock('~/Providers', () => ({ useMessageContext: () => mockMessageContext.current, })); -jest.mock('@librechat/client', () => ({ - Button: ({ children, ...props }: React.ComponentProps<'button'>) => ( - - ), - Clipboard: () => , - CheckMark: () => , - TooltipAnchor: ({ children }: { children?: React.ReactNode }) => <>{children}, -})); - -jest.mock('lucide-react', () => ({ - ScrollText: () => , - ChevronDown: () => , - ChevronUp: () => , -})); - const partialSummary = [{ type: 'text', text: 'Half a summary before the error' }] as never; describe('Summary', () => { diff --git a/client/src/components/Chat/Messages/Content/verticals.tsx b/client/src/components/Chat/Messages/Content/verticals.tsx index 6b3523147d..3887dc7391 100644 --- a/client/src/components/Chat/Messages/Content/verticals.tsx +++ b/client/src/components/Chat/Messages/Content/verticals.tsx @@ -200,11 +200,9 @@ function PlaceList({ places, label }: { places: PlaceResult[]; label: string }) > {places.slice(0, MAX_PLACES).map((place, i) => (
  • 0 && 'border-t border-border-light')} > ({ @@ -47,12 +47,19 @@ jest.mock('~/hooks/useNewConvo', () => ({ import BottomBar from '../BottomBar'; const links = [] as NavLink[]; +let queryClient: QueryClient; describe('mobile bottom bar — new chat ordering', () => { beforeEach(() => { + queryClient = new QueryClient(); jest.clearAllMocks(); }); + afterEach(() => { + cleanup(); + queryClient.clear(); + }); + /** * The drawer close must start BEFORE the conversation reset: run * synchronously, the reset's cache clearing and navigation flush in the @@ -71,7 +78,7 @@ describe('mobile bottom bar — new chat ordering', () => { }); render( - + @@ -82,14 +89,14 @@ describe('mobile bottom bar — new chat ordering', () => { expect(order).toEqual(['close']); expect(afterSlide).toBeDefined(); - afterSlide?.(); + act(() => afterSlide?.()); expect(order).toEqual(['close', 'reset']); }); it('leaves modified clicks to the browser (new tab)', () => { const onNewChat = jest.fn(); render( - + diff --git a/client/src/hooks/useKeyboardShortcuts.spec.tsx b/client/src/hooks/useKeyboardShortcuts.spec.tsx index ae9c70f0c8..4155e92dd1 100644 --- a/client/src/hooks/useKeyboardShortcuts.spec.tsx +++ b/client/src/hooks/useKeyboardShortcuts.spec.tsx @@ -29,6 +29,7 @@ jest.mock('./useNewConvo', () => ({ const STORAGE_KEY = 'customKeyboardShortcuts'; const copyMock = copy as jest.MockedFunction; +let queryClient: QueryClient; function buildConversation(conversationId: string, title: string): TConversation { return { conversationId, title, endpoint: 'agents' } as TConversation; @@ -67,7 +68,7 @@ function renderHarness( }; return render(, { wrapper: ({ children }: { children: ReactNode }) => ( - + {children} @@ -77,12 +78,14 @@ function renderHarness( } beforeEach(() => { + queryClient = new QueryClient(); window.localStorage.clear(); copyMock.mockClear(); }); afterEach(() => { cleanup(); + queryClient.clear(); document.body.replaceChildren(); });