mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-20 17:13:44 +00:00
🛗 perf: Fetch Pinned Agents Directly Past the Global Agents Map (#13972)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* 🚀 perf: Decouple Pinned Agents from Global Agents Map in Sidebar Pinned/favorite agents in the sidebar waited for the full global agents map (useListAgentsQuery, which walks every pagination cursor) before rendering. In environments with many agents this left pinned items in a loading state even though their IDs were already known. FavoritesList now fetches pinned agent IDs directly via getAgentById when the global map is still loading, and falls back to filtering only missing IDs once the map is available. The loading state tracks just the small set of pinned-agent queries instead of the entire catalog, so pinned agents appear as soon as their own data resolves. Closes #13967 * 🩹 fix: Address Codex review on pinned-agent decoupling - Stop caching the {found, agent} wrapper under the shared [QueryKeys.agent, id] key; direct fetches now return a plain Agent like useGetAgentByIdQuery, so opening/selecting a pinned agent within the stale window can no longer read a wrapper as an agent. Missing (404/403) agents are detected via the query error state. - Gate the direct fetches on the agents endpoint being enabled, so pinned agents the endpoint list intentionally hides are not fetched, rendered, or cleaned up when the endpoint is disabled. - Keep the loading skeleton while a direct fetch fails with a transient (non-404/403) error and the global agents map is still loading, so a pinned agent no longer disappears on a momentary 500/network error during startup. - Remove the now-unused AgentQueryResult type. * 🩹 fix: Address Codex round 2 on pinned-agent decoupling - Keep the loading skeleton (not an empty/collapsed row) while the endpoints query is still loading. The endpoint gate previously treated the default empty config as disabled, so pinned-agent favorites rendered an empty row that could be measured and cached by the CellMeasurer before the config arrived. isAgentsLoading now stays true while isEndpointsLoading is true. - Replace the blanket retry:false on direct pinned-agent fetches with a predicate that skips missing-agent (404/403) errors but still retries transient 500/network failures, restoring the prior default-retry resilience on the fast path. - Add data-testid to the favorite skeleton and a regression test for the endpoints-loading window. * 🛡️ fix: Don't delete pinned favorites on a global agents 403 GET /api/agents/:id runs the role-level AGENTS.USE check (checkAgentAccess) before the per-agent VIEW ACL, so a temporarily revoked role returns 403 for every agent. Because direct fetches now run while the agents map is undefined, treating those 403s as missing agents made the cleanup effect persist reorderFavorites and wipe all pinned agent favorites. staleAgentIdsKey now returns early while agentsMap is undefined, restoring the original invariant that favorite cleanup only runs once the global map has loaded successfully (which also proves AGENTS.USE is granted). Rendering of pinned agents while the map loads is unaffected; only deletion is deferred.
This commit is contained in:
parent
4fa86be424
commit
c948606a8c
3 changed files with 165 additions and 47 deletions
|
|
@ -10,8 +10,6 @@ import type {
|
|||
} from 'librechat-data-provider';
|
||||
import type { OptionWithIcon, ExtendedFile } from './types';
|
||||
|
||||
export type AgentQueryResult = { found: true; agent: Agent } | { found: false };
|
||||
|
||||
export type TAgentOption = OptionWithIcon &
|
||||
Agent & {
|
||||
knowledge_files?: Array<[string, ExtendedFile]>;
|
||||
|
|
|
|||
|
|
@ -1,13 +1,12 @@
|
|||
import React, { useRef, useCallback, useMemo, useEffect } from 'react';
|
||||
import { LayoutGrid } from 'lucide-react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { LayoutGrid } from 'lucide-react';
|
||||
import { useDrag, useDrop } from 'react-dnd';
|
||||
import { Skeleton } from '@librechat/client';
|
||||
import { useNavigate } from 'react-router-dom';
|
||||
import { useQueries } from '@tanstack/react-query';
|
||||
import { QueryKeys, dataService } from 'librechat-data-provider';
|
||||
import { QueryKeys, EModelEndpoint, dataService } from 'librechat-data-provider';
|
||||
import type { Agent, TEndpointsConfig, TModelSpec } from 'librechat-data-provider';
|
||||
import type { AgentQueryResult } from '~/common';
|
||||
import {
|
||||
useGetConversation,
|
||||
useFavorites,
|
||||
|
|
@ -21,9 +20,21 @@ import useSelectMention from '~/hooks/Input/useSelectMention';
|
|||
import FavoriteItem from './FavoriteItem';
|
||||
import store from '~/store';
|
||||
|
||||
/** A 404/403 from getAgentById means the agent is gone or inaccessible; other errors are transient. */
|
||||
const isMissingAgentError = (error: unknown): boolean => {
|
||||
if (error && typeof error === 'object' && 'response' in error) {
|
||||
const status = (error as { response?: { status?: number } }).response?.status;
|
||||
return status === 404 || status === 403;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
/** Height intentionally matches FavoriteItem (px-3 py-2 + h-5 icon) to keep the CellMeasurerCache valid across the isAgentsLoading transition. */
|
||||
const FavoriteItemSkeleton = () => (
|
||||
<div className="flex w-full items-center rounded-lg px-3 py-2">
|
||||
<div
|
||||
className="flex w-full items-center rounded-lg px-3 py-2"
|
||||
data-testid="favorite-item-skeleton"
|
||||
>
|
||||
<Skeleton className="mr-2 h-5 w-5 rounded-full" />
|
||||
<Skeleton className="h-4 w-24" />
|
||||
</div>
|
||||
|
|
@ -133,7 +144,8 @@ export default function FavoritesList({
|
|||
const { newConversation } = useNewConvo();
|
||||
const assistantsMap = useAssistantsMapContext();
|
||||
const agentsMap = useAgentsMapContext();
|
||||
const { data: endpointsConfig = {} as TEndpointsConfig } = useGetEndpointsQuery();
|
||||
const { data: endpointsConfig = {} as TEndpointsConfig, isLoading: isEndpointsLoading } =
|
||||
useGetEndpointsQuery();
|
||||
const { data: startupConfig } = useGetStartupConfig();
|
||||
|
||||
const modelSpecs = useMemo(
|
||||
|
|
@ -213,45 +225,43 @@ export default function FavoritesList({
|
|||
[safeFavorites],
|
||||
);
|
||||
|
||||
const missingAgentIds = useMemo(() => {
|
||||
if (agentsMap === undefined) {
|
||||
const agentsEndpointEnabled = !!endpointsConfig?.[EModelEndpoint.agents];
|
||||
|
||||
const agentIdsToFetch = useMemo(() => {
|
||||
if (!agentsEndpointEnabled) {
|
||||
return [];
|
||||
}
|
||||
if (agentsMap === undefined) {
|
||||
return allAgentIds;
|
||||
}
|
||||
return allAgentIds.filter((id) => !agentsMap[id]);
|
||||
}, [allAgentIds, agentsMap]);
|
||||
}, [allAgentIds, agentsMap, agentsEndpointEnabled]);
|
||||
|
||||
const missingAgentQueries = useQueries({
|
||||
queries: missingAgentIds.map((agentId) => ({
|
||||
const agentQueries = useQueries({
|
||||
queries: agentIdsToFetch.map((agentId) => ({
|
||||
queryKey: [QueryKeys.agent, 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 } };
|
||||
const status = axiosError.response?.status;
|
||||
if (status === 404 || status === 403) {
|
||||
return { found: false };
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
queryFn: (): Promise<Agent> => dataService.getAgentById({ agent_id: agentId }),
|
||||
staleTime: 1000 * 60 * 5,
|
||||
retry: (failureCount: number, error: unknown) =>
|
||||
!isMissingAgentError(error) && failureCount < 3,
|
||||
})),
|
||||
});
|
||||
|
||||
const staleAgentIdsKey = useMemo(() => {
|
||||
// Only persist cleanup once the global map has loaded. A revoked AGENTS.USE role
|
||||
// makes every getAgentById return a global 403, which must not delete favorites.
|
||||
if (agentsMap === undefined) {
|
||||
return '';
|
||||
}
|
||||
const ids: string[] = [];
|
||||
for (let i = 0; i < missingAgentIds.length; i++) {
|
||||
const query = missingAgentQueries[i];
|
||||
if (query.data && !query.data.found) {
|
||||
ids.push(missingAgentIds[i]);
|
||||
for (let i = 0; i < agentIdsToFetch.length; i++) {
|
||||
const query = agentQueries[i];
|
||||
if (query.isError && isMissingAgentError(query.error)) {
|
||||
ids.push(agentIdsToFetch[i]);
|
||||
}
|
||||
}
|
||||
return ids.sort().join(',');
|
||||
}, [missingAgentIds, missingAgentQueries]);
|
||||
}, [agentIdsToFetch, agentQueries, agentsMap]);
|
||||
|
||||
const cleanupAttemptedRef = useRef('');
|
||||
|
||||
|
|
@ -293,26 +303,29 @@ export default function FavoritesList({
|
|||
}, [staleSpecNamesKey, safeFavorites, reorderFavorites]);
|
||||
|
||||
const combinedAgentsMap = useMemo(() => {
|
||||
if (agentsMap === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
const combined: Record<string, Agent> = {};
|
||||
for (const [key, value] of Object.entries(agentsMap)) {
|
||||
if (value) {
|
||||
combined[key] = value;
|
||||
if (agentsMap) {
|
||||
for (const [key, value] of Object.entries(agentsMap)) {
|
||||
if (value) {
|
||||
combined[key] = value;
|
||||
}
|
||||
}
|
||||
}
|
||||
missingAgentQueries.forEach((query) => {
|
||||
if (query.data?.found) {
|
||||
combined[query.data.agent.id] = query.data.agent;
|
||||
agentQueries.forEach((query) => {
|
||||
if (query.data) {
|
||||
combined[query.data.id] = query.data;
|
||||
}
|
||||
});
|
||||
return combined;
|
||||
}, [agentsMap, missingAgentQueries]);
|
||||
}, [agentsMap, agentQueries]);
|
||||
|
||||
const isAgentsLoading =
|
||||
(allAgentIds.length > 0 && agentsMap === undefined) ||
|
||||
(missingAgentIds.length > 0 && missingAgentQueries.some((q) => q.isLoading));
|
||||
allAgentIds.length > 0 &&
|
||||
(isEndpointsLoading ||
|
||||
agentQueries.some(
|
||||
(q) =>
|
||||
q.isLoading || (agentsMap === undefined && q.isError && !isMissingAgentError(q.error)),
|
||||
));
|
||||
|
||||
const draggedFavoritesRef = useRef(safeFavorites);
|
||||
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ import '@testing-library/jest-dom';
|
|||
import { RecoilRoot } from 'recoil';
|
||||
import { DndProvider } from 'react-dnd';
|
||||
import { BrowserRouter } from 'react-router-dom';
|
||||
import { dataService } from 'librechat-data-provider';
|
||||
import { HTML5Backend } from 'react-dnd-html5-backend';
|
||||
import { dataService, EModelEndpoint } from 'librechat-data-provider';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import type { Agent } from 'librechat-data-provider';
|
||||
|
||||
|
|
@ -60,9 +60,10 @@ jest.mock('~/hooks', () => ({
|
|||
useGetConversation: () => () => null,
|
||||
}));
|
||||
|
||||
const mockUseAgentsMapContext = jest.fn((): Record<string, Agent> | undefined => ({}));
|
||||
jest.mock('~/Providers', () => ({
|
||||
useAssistantsMapContext: () => ({}),
|
||||
useAgentsMapContext: () => ({}),
|
||||
useAgentsMapContext: () => mockUseAgentsMapContext(),
|
||||
}));
|
||||
|
||||
const mockOnSelectSpec = jest.fn();
|
||||
|
|
@ -75,8 +76,14 @@ const mockUseGetStartupConfig = jest.fn((): { data?: { modelSpecs: { list: unkno
|
|||
data: { modelSpecs: { list: [] as unknown[] } },
|
||||
}));
|
||||
|
||||
const mockUseGetEndpointsQuery = jest.fn(
|
||||
(): { data?: Record<string, unknown>; isLoading?: boolean } => ({
|
||||
data: { [EModelEndpoint.agents]: { order: 0 } },
|
||||
}),
|
||||
);
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useGetEndpointsQuery: () => ({ data: {} }),
|
||||
useGetEndpointsQuery: () => mockUseGetEndpointsQuery(),
|
||||
useGetStartupConfig: () => mockUseGetStartupConfig(),
|
||||
}));
|
||||
|
||||
|
|
@ -123,6 +130,10 @@ describe('FavoritesList', () => {
|
|||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockFavorites.length = 0;
|
||||
mockUseAgentsMapContext.mockImplementation(() => ({}));
|
||||
mockUseGetEndpointsQuery.mockImplementation(() => ({
|
||||
data: { [EModelEndpoint.agents]: { order: 0 } },
|
||||
}));
|
||||
mockUseFavorites.mockImplementation(() => ({
|
||||
favorites: mockFavorites,
|
||||
reorderFavorites: jest.fn(),
|
||||
|
|
@ -213,6 +224,102 @@ describe('FavoritesList', () => {
|
|||
expect(queryAllByTestId('favorite-item')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('renders pinned agents via direct fetch while the global agents map is still loading', async () => {
|
||||
const validAgent: Agent = {
|
||||
id: 'pinned-agent',
|
||||
name: 'Pinned Agent',
|
||||
author: 'test-author',
|
||||
} as Agent;
|
||||
|
||||
mockUseAgentsMapContext.mockImplementation(() => undefined);
|
||||
mockFavorites.push({ agentId: 'pinned-agent' });
|
||||
|
||||
(dataService.getAgentById as jest.Mock).mockResolvedValue(validAgent);
|
||||
|
||||
const { findAllByTestId } = renderWithProviders(<FavoritesList />);
|
||||
|
||||
const favoriteItems = await findAllByTestId('favorite-item');
|
||||
expect(favoriteItems).toHaveLength(1);
|
||||
expect(favoriteItems[0]).toHaveTextContent('Pinned Agent');
|
||||
expect(dataService.getAgentById as jest.Mock).toHaveBeenCalledWith({
|
||||
agent_id: 'pinned-agent',
|
||||
});
|
||||
});
|
||||
|
||||
it('does not fetch or render agents when the agents endpoint is disabled', async () => {
|
||||
mockUseGetEndpointsQuery.mockImplementation(() => ({ data: {} }));
|
||||
mockUseAgentsMapContext.mockImplementation(() => undefined);
|
||||
mockFavorites.push({ agentId: 'pinned-agent' });
|
||||
|
||||
const { queryAllByTestId } = renderWithProviders(<FavoritesList />);
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
expect(dataService.getAgentById as jest.Mock).not.toHaveBeenCalled();
|
||||
expect(queryAllByTestId('favorite-item')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('keeps skeletons (not an empty row) while the endpoints config is still loading', async () => {
|
||||
mockUseGetEndpointsQuery.mockImplementation(() => ({ data: undefined, isLoading: true }));
|
||||
mockUseAgentsMapContext.mockImplementation(() => undefined);
|
||||
mockFavorites.push({ agentId: 'pinned-agent' });
|
||||
|
||||
const { queryAllByTestId } = renderWithProviders(<FavoritesList />);
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
// Endpoint gate is undecided, so no direct fetch yet, but the row stays as a skeleton.
|
||||
expect(dataService.getAgentById as jest.Mock).not.toHaveBeenCalled();
|
||||
expect(queryAllByTestId('favorite-item-skeleton').length).toBeGreaterThan(0);
|
||||
expect(queryAllByTestId('favorite-item')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('does not delete favorites on a global 403 while the agents map is unavailable', async () => {
|
||||
const mockReorderFavorites = jest.fn();
|
||||
// A revoked AGENTS.USE role makes the list query (and thus the map) fail, so the
|
||||
// map stays undefined and every getAgentById returns a global 403.
|
||||
mockUseAgentsMapContext.mockImplementation(() => undefined);
|
||||
mockUseFavorites.mockImplementation(() => ({
|
||||
favorites: [{ agentId: 'agent-a' }, { agentId: 'agent-b' }],
|
||||
reorderFavorites: mockReorderFavorites,
|
||||
isLoading: false,
|
||||
}));
|
||||
|
||||
(dataService.getAgentById as jest.Mock).mockRejectedValue({ response: { status: 403 } });
|
||||
|
||||
const { queryAllByTestId } = renderWithProviders(<FavoritesList />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(dataService.getAgentById as jest.Mock).toHaveBeenCalledWith({ agent_id: 'agent-a' });
|
||||
});
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(mockReorderFavorites).not.toHaveBeenCalled();
|
||||
expect(queryAllByTestId('favorite-item')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('keeps a pinned agent as a skeleton on a transient error while the map is still loading', async () => {
|
||||
const mockReorderFavorites = jest.fn();
|
||||
mockUseAgentsMapContext.mockImplementation(() => undefined);
|
||||
mockUseFavorites.mockImplementation(() => ({
|
||||
favorites: [{ agentId: 'flaky-agent' }],
|
||||
reorderFavorites: mockReorderFavorites,
|
||||
isLoading: false,
|
||||
}));
|
||||
|
||||
(dataService.getAgentById as jest.Mock).mockRejectedValue({ response: { status: 500 } });
|
||||
|
||||
const { queryAllByTestId } = renderWithProviders(<FavoritesList />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(dataService.getAgentById as jest.Mock).toHaveBeenCalledWith({
|
||||
agent_id: 'flaky-agent',
|
||||
});
|
||||
});
|
||||
|
||||
// Item is neither rendered nor cleaned up — it stays in the loading skeleton state.
|
||||
expect(queryAllByTestId('favorite-item')).toHaveLength(0);
|
||||
expect(mockReorderFavorites).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should treat 403 the same as 404 — agent not rendered', async () => {
|
||||
const validAgent: Agent = {
|
||||
id: 'valid-agent',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue