Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions client/src/common/agents-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type {
} from 'librechat-data-provider';
import type { OptionWithIcon, ExtendedFile } from './types';

export type AgentQueryResult = { found: boolean; agent?: Agent };
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AgentQueryResult type allows agent to be optional even when found is true, which isn't type-safe. Consider using a discriminated union type instead:

export type AgentQueryResult = 
  | { found: true; agent: Agent }
  | { found: false };

This would eliminate the need for non-null assertions (!) in FavoritesList.tsx line 219 and provide better type safety.

Suggested change
export type AgentQueryResult = { found: boolean; agent?: Agent };
export type AgentQueryResult =
| { found: true; agent: Agent }
| { found: false };

Copilot uses AI. Check for mistakes.

export type TAgentOption = OptionWithIcon &
Agent & {
knowledge_files?: Array<[string, ExtendedFile]>;
Expand Down
20 changes: 17 additions & 3 deletions client/src/components/Nav/Favorites/FavoritesList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { QueryKeys, dataService } from 'librechat-data-provider';
import type t from 'librechat-data-provider';
import { useFavorites, useLocalize, useShowMarketplace, useNewConvo } from '~/hooks';
import { useAssistantsMapContext, useAgentsMapContext } from '~/Providers';
import { AgentQueryResult } from '~/common';
import useSelectMention from '~/hooks/Input/useSelectMention';
import { useGetEndpointsQuery } from '~/data-provider';
import FavoriteItem from './FavoriteItem';
Expand Down Expand Up @@ -184,7 +185,20 @@ export default function FavoritesList({
const missingAgentQueries = useQueries({
queries: missingAgentIds.map((agentId) => ({
queryKey: [QueryKeys.agent, agentId],
queryFn: () => dataService.getAgentById({ agent_id: agentId }),
queryFn: async (): Promise<AgentQueryResult> => {
try {
const agent = await dataService.getAgentById({ agent_id: agentId });
return { found: true, agent };
} catch (error) {
if (error && typeof error === 'object' && 'response' in error) {
const axiosError = error as { response?: { status?: number } };
if (axiosError.response?.status === 404) {
return { found: false };
}
}
throw error;
}
},
staleTime: 1000 * 60 * 5,
enabled: missingAgentIds.length > 0,
})),
Expand All @@ -201,8 +215,8 @@ export default function FavoritesList({
}
}
missingAgentQueries.forEach((query) => {
if (query.data) {
combined[query.data.id] = query.data;
if (query.data?.found) {
combined[query.data.agent!.id] = query.data.agent!;
}
});
return combined;
Expand Down
243 changes: 243 additions & 0 deletions client/src/components/Nav/Favorites/tests/FavoritesList.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
import React from 'react';
import { render } from '@testing-library/react';
import '@testing-library/jest-dom';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { RecoilRoot } from 'recoil';
import { DndProvider } from 'react-dnd';
import { HTML5Backend } from 'react-dnd-html5-backend';
import { BrowserRouter } from 'react-router-dom';
import { dataService } from 'librechat-data-provider';
import type t from 'librechat-data-provider';
import type { AgentQueryResult } from '~/common';

// Mock store before importing FavoritesList
jest.mock('~/store', () => {
const { atom } = jest.requireActual('recoil');
return {
__esModule: true,
default: {
search: atom({
key: 'mock-search-atom',
default: { query: '' },
}),
conversationByIndex: () =>
atom({
key: 'mock-conversation-atom',
default: null,
}),
},
};
});
import FavoritesList from '../FavoritesList';

type FavoriteItem = {
agentId?: string;
model?: string;
endpoint?: string;
};

// Mock dataService
jest.mock('librechat-data-provider', () => ({
...jest.requireActual('librechat-data-provider'),
dataService: {
getAgentById: jest.fn(),
},
}));

// Mock hooks
const mockFavorites: FavoriteItem[] = [];
const mockUseFavorites = jest.fn(() => ({
favorites: mockFavorites,
reorderFavorites: jest.fn(),
isLoading: false,
}));

jest.mock('~/hooks', () => ({
useFavorites: () => mockUseFavorites(),
useLocalize: () => (key: string) => key,
useShowMarketplace: () => false,
useNewConvo: () => ({ newConversation: jest.fn() }),
}));

jest.mock('~/Providers', () => ({
useAssistantsMapContext: () => ({}),
useAgentsMapContext: () => ({}),
}));

jest.mock('~/hooks/Input/useSelectMention', () => () => ({
onSelectEndpoint: jest.fn(),
}));

jest.mock('~/data-provider', () => ({
useGetEndpointsQuery: () => ({ data: {} }),
}));

jest.mock('../FavoriteItem', () => ({
__esModule: true,
default: ({ item, type }: { item: any; type: string }) => (
<div data-testid="favorite-item" data-type={type}>
{type === 'agent' ? item.name : item.model}
</div>
),
}));

const createTestQueryClient = () =>
new QueryClient({
defaultOptions: {
queries: {
retry: false,
},
},
});

const renderWithProviders = (ui: React.ReactElement) => {
const queryClient = createTestQueryClient();
return render(
<QueryClientProvider client={queryClient}>
<RecoilRoot>
<BrowserRouter>
<DndProvider backend={HTML5Backend}>{ui}</DndProvider>
</BrowserRouter>
</RecoilRoot>
</QueryClientProvider>,
);
};

describe('FavoritesList', () => {
beforeEach(() => {
jest.clearAllMocks();
mockFavorites.length = 0;
});

describe('rendering', () => {
it('should render nothing when favorites is empty and marketplace is hidden', () => {
const { container } = renderWithProviders(<FavoritesList />);
expect(container.firstChild).toBeNull();
});

it('should render skeleton while loading', () => {
mockUseFavorites.mockReturnValueOnce({
favorites: [],
reorderFavorites: jest.fn(),
isLoading: true,
});

renderWithProviders(<FavoritesList />);
// Skeletons should be present during loading
expect(document.querySelector('.animate-pulse')).toBeInTheDocument();
});
});
});

describe('AgentQueryResult type behavior', () => {
it('should handle found agent result correctly', async () => {
const mockAgent: t.Agent = {
id: 'agent-123',
name: 'Test Agent',
author: 'test-author',
} as t.Agent;

(dataService.getAgentById as jest.Mock).mockResolvedValueOnce(mockAgent);

const result = await dataService.getAgentById({ agent_id: 'agent-123' });

// Simulate the query function logic
const queryResult: AgentQueryResult = { found: true, agent: result };

expect(queryResult.found).toBe(true);
expect(queryResult.agent).toEqual(mockAgent);
});

it('should handle not found agent result correctly', async () => {
const error = { response: { status: 404 } };
(dataService.getAgentById as jest.Mock).mockRejectedValueOnce(error);

// Simulate the query function logic from FavoritesList
let queryResult: AgentQueryResult | undefined;
try {
await dataService.getAgentById({ agent_id: 'deleted-agent' });
} catch (err) {
if (err && typeof err === 'object' && 'response' in err) {
const axiosError = err as { response?: { status?: number } };
if (axiosError.response?.status === 404) {
queryResult = { found: false };
}
}
}

expect(queryResult).toEqual({ found: false });
});

it('should rethrow non-404 errors', async () => {
const error = { response: { status: 500 }, message: 'Server error' };
(dataService.getAgentById as jest.Mock).mockRejectedValueOnce(error);

// Simulate the query function logic from FavoritesList
let caughtError;
try {
await dataService.getAgentById({ agent_id: 'agent-123' });
} catch (err) {
if (err && typeof err === 'object' && 'response' in err) {
const axiosError = err as { response?: { status?: number } };
if (axiosError.response?.status === 404) {
// Would return { found: false }, but this is a 500
} else {
caughtError = err;
}
}
}

expect(caughtError).toEqual(error);
});
});
Comment on lines +132 to +192
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests in the "AgentQueryResult type behavior" describe block (lines 132-192) test the logic in isolation but don't actually test the component's integration with the useQueries hook. Consider adding a test that renders the FavoritesList component with favorites containing missing agent IDs and verifies that:

  1. The component doesn't show infinite loading skeleton
  2. The missing agents are correctly excluded from the rendered favorites
  3. Valid agents are still rendered correctly

This would better validate the bug fix described in the PR.

Copilot uses AI. Check for mistakes.

describe('combinedAgentsMap logic', () => {
it('should only include agents where found is true', () => {
const mockAgentsMap: Record<string, t.Agent> = {
'existing-agent': { id: 'existing-agent', name: 'Existing' } as t.Agent,
};

const mockQueryResults: { data: AgentQueryResult | undefined }[] = [
{ data: { found: true, agent: { id: 'new-agent', name: 'New' } as t.Agent } },
{ data: { found: false } },
{ data: undefined }, // Still loading
];

// Simulate combinedAgentsMap logic
const combined: Record<string, t.Agent> = {};
for (const [key, value] of Object.entries(mockAgentsMap)) {
if (value) {
combined[key] = value;
}
}
mockQueryResults.forEach((query) => {
if (query.data?.found && query.data.agent) {
combined[query.data.agent.id] = query.data.agent;
}
});

expect(Object.keys(combined)).toHaveLength(2);
expect(combined['existing-agent']).toBeDefined();
expect(combined['new-agent']).toBeDefined();
expect(combined['not-found']).toBeUndefined();
});

it('should handle empty query results', () => {
const mockAgentsMap: Record<string, t.Agent> = {};
const mockQueryResults: { data: AgentQueryResult | undefined }[] = [];

const combined: Record<string, t.Agent> = {};
for (const [key, value] of Object.entries(mockAgentsMap)) {
if (value) {
combined[key] = value;
}
}
mockQueryResults.forEach((query) => {
if (query.data?.found && query.data.agent) {
combined[query.data.agent.id] = query.data.agent;
}
});

expect(Object.keys(combined)).toHaveLength(0);
});
});
Loading