Fix canonical artifact ownership and frontend review regressions

This commit is contained in:
Marco Beretta 2026-09-06 04:06:56 +02:00
parent e3806c338d
commit 318da18d28
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
10 changed files with 34 additions and 108 deletions

View file

@ -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',
},
);

View file

@ -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 } : {}),

View file

@ -107,27 +107,29 @@ export default function AskUserQuestions({
{(onExpand != null || onDismiss != null) && (
<div className="flex shrink-0 items-center justify-end border-b border-border-light px-2 py-1">
{onExpand != null && (
<button
type="button"
<Button
variant="ghost"
size="icon-sm"
aria-label={localize('com_ui_expand')}
className="rounded p-1 text-text-secondary hover:bg-surface-hover"
className="text-text-secondary"
onClick={onExpand}
>
<ChevronUp className="h-4 w-4" aria-hidden="true" />
</button>
</Button>
)}
{/** 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 && (
<button
type="button"
<Button
variant="ghost"
size="icon-sm"
aria-label={localize('com_ui_close')}
className="rounded p-1 text-text-secondary hover:bg-surface-hover"
className="text-text-secondary"
onClick={onDismiss}
>
<X className="h-4 w-4" aria-hidden="true" />
</button>
</Button>
)}
</div>
)}

View file

@ -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();
});

View file

@ -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();
});
});

View file

@ -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(
<MessageContext.Provider
value={{ messageId: 'msg-1', isExpanded: false, isSubmitting, isLatestMessage }}
>
<Reasoning reasoning="<think>weighing the options</think>" isLast={isLast} />
</MessageContext.Provider>,
);
}
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');
});
});

View file

@ -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'>) => (
<button type="button" {...props}>
{children}
</button>
),
Clipboard: () => <span />,
CheckMark: () => <span />,
TooltipAnchor: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
}));
jest.mock('lucide-react', () => ({
ScrollText: () => <span />,
ChevronDown: () => <span />,
ChevronUp: () => <span />,
}));
const partialSummary = [{ type: 'text', text: 'Half a summary before the error' }] as never;
describe('Summary', () => {

View file

@ -200,11 +200,9 @@ function PlaceList({ places, label }: { places: PlaceResult[]; label: string })
>
{places.slice(0, MAX_PLACES).map((place, i) => (
<li
/** Several branches of one chain share a name and often carry no
* identifier, so name alone collides across rows whose address,
* rating and map target all differ. Verticals never stream, so the
* index is a stable tiebreaker for a snapshot-rendered list. */
key={place.identifier || `${place.name ?? ''}|${place.address ?? ''}|${i}`}
/** The same provider place can appear in several grouped searches;
* discriminate each occurrence even when its identifier is present. */
key={`${place.identifier || `${place.name ?? ''}|${place.address ?? ''}`}|${i}`}
className={cn(i > 0 && 'border-t border-border-light')}
>
<a

View file

@ -1,7 +1,7 @@
import React from 'react';
import { RecoilRoot } from 'recoil';
import { render, screen, fireEvent } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { act, cleanup, render, screen, fireEvent } from '@testing-library/react';
import type { NavLink } from '~/common';
jest.mock('~/hooks', () => ({
@ -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(
<QueryClientProvider client={new QueryClient()}>
<QueryClientProvider client={queryClient}>
<RecoilRoot>
<BottomBar links={links} onNewChat={onNewChat} />
</RecoilRoot>
@ -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(
<QueryClientProvider client={new QueryClient()}>
<QueryClientProvider client={queryClient}>
<RecoilRoot>
<BottomBar links={links} onNewChat={onNewChat} />
</RecoilRoot>

View file

@ -29,6 +29,7 @@ jest.mock('./useNewConvo', () => ({
const STORAGE_KEY = 'customKeyboardShortcuts';
const copyMock = copy as jest.MockedFunction<typeof copy>;
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(<Harness />, {
wrapper: ({ children }: { children: ReactNode }) => (
<QueryClientProvider client={new QueryClient()}>
<QueryClientProvider client={queryClient}>
<RecoilRoot initializeState={initializeState}>
<MemoryRouter initialEntries={[route]}>{children}</MemoryRouter>
</RecoilRoot>
@ -77,12 +78,14 @@ function renderHarness(
}
beforeEach(() => {
queryClient = new QueryClient();
window.localStorage.clear();
copyMock.mockClear();
});
afterEach(() => {
cleanup();
queryClient.clear();
document.body.replaceChildren();
});