mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-02 22:30:18 +01:00
🎯 fix: Use Agents Endpoint Config for Agent Panel File Upload Validation (#11992)
* fix: Use correct endpoint for file validation in agent panel uploads Agent panel file uploads (FileSearch, FileContext, Code/Files) were validating against the active conversation's endpoint config instead of the agents endpoint config. This caused incorrect file size limits when the active chat used a different endpoint. Add endpointOverride option to useFileHandling so callers can specify the correct endpoint for validation independent of the active conversation. * fix: Use agents endpoint config for agent panel file upload validation Agent panel file uploads (FileSearch, FileContext, Code/Files) validated against the active conversation's endpoint config instead of the agents endpoint config. This caused wrong file size limits when the active chat used a different endpoint. Adds endpointOverride to useFileHandling so callers can specify the correct endpoint for both validation and upload routing, independent of the active conversation. * test: Add unit tests for useFileHandling hook to validate endpoint overrides Introduced comprehensive tests for the useFileHandling hook, ensuring correct behavior when using endpoint overrides for file validation and upload routing. The tests cover various scenarios, including fallback to conversation endpoints and proper handling of agent-specific configurations, enhancing the reliability of file handling in the application.
This commit is contained in:
parent
43ff3f8473
commit
cde5079886
6 changed files with 304 additions and 3 deletions
|
|
@ -35,6 +35,7 @@ export default function Files({
|
|||
const { abortUpload, handleFileChange } = useFileHandling({
|
||||
fileSetter: setFiles,
|
||||
additionalMetadata: { agent_id, tool_resource },
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
});
|
||||
|
||||
useLazyEffect(
|
||||
|
|
|
|||
|
|
@ -47,10 +47,12 @@ export default function FileContext({
|
|||
|
||||
const { handleFileChange } = useFileHandling({
|
||||
additionalMetadata: { agent_id, tool_resource: EToolResources.context },
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
fileSetter: setFiles,
|
||||
});
|
||||
const { handleSharePointFiles, isProcessing, downloadProgress } = useSharePointFileHandling({
|
||||
additionalMetadata: { agent_id, tool_resource: EToolResources.file_search },
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
fileSetter: setFiles,
|
||||
});
|
||||
useLazyEffect(
|
||||
|
|
|
|||
|
|
@ -44,11 +44,13 @@ export default function FileSearch({
|
|||
|
||||
const { handleFileChange } = useFileHandling({
|
||||
additionalMetadata: { agent_id, tool_resource: EToolResources.file_search },
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
fileSetter: setFiles,
|
||||
});
|
||||
|
||||
const { handleSharePointFiles, isProcessing, downloadProgress } = useSharePointFileHandling({
|
||||
additionalMetadata: { agent_id, tool_resource: EToolResources.file_search },
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
fileSetter: setFiles,
|
||||
});
|
||||
|
||||
|
|
|
|||
285
client/src/hooks/Files/__tests__/useFileHandling.test.ts
Normal file
285
client/src/hooks/Files/__tests__/useFileHandling.test.ts
Normal file
|
|
@ -0,0 +1,285 @@
|
|||
import { renderHook, act } from '@testing-library/react';
|
||||
import { Constants, EModelEndpoint, getEndpointFileConfig } from 'librechat-data-provider';
|
||||
|
||||
beforeAll(() => {
|
||||
global.URL.createObjectURL = jest.fn(() => 'blob:mock-url');
|
||||
global.URL.revokeObjectURL = jest.fn();
|
||||
});
|
||||
|
||||
const mockShowToast = jest.fn();
|
||||
const mockSetFilesLoading = jest.fn();
|
||||
const mockMutate = jest.fn();
|
||||
|
||||
let mockConversation: Record<string, string | null | undefined> = {};
|
||||
|
||||
jest.mock('~/Providers/ChatContext', () => ({
|
||||
useChatContext: jest.fn(() => ({
|
||||
files: new Map(),
|
||||
setFiles: jest.fn(),
|
||||
setFilesLoading: mockSetFilesLoading,
|
||||
conversation: mockConversation,
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/client', () => ({
|
||||
useToastContext: jest.fn(() => ({
|
||||
showToast: mockShowToast,
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('recoil', () => ({
|
||||
...jest.requireActual('recoil'),
|
||||
useSetRecoilState: jest.fn(() => jest.fn()),
|
||||
}));
|
||||
|
||||
jest.mock('~/store', () => ({
|
||||
ephemeralAgentByConvoId: jest.fn(() => ({ key: 'mock' })),
|
||||
}));
|
||||
|
||||
jest.mock('@tanstack/react-query', () => ({
|
||||
useQueryClient: jest.fn(() => ({
|
||||
getQueryData: jest.fn(),
|
||||
refetchQueries: jest.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useGetFileConfig: jest.fn(() => ({ data: null })),
|
||||
useUploadFileMutation: jest.fn((_opts: Record<string, unknown>) => ({
|
||||
mutate: mockMutate,
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/useLocalize', () => {
|
||||
const fn = jest.fn((key: string) => key);
|
||||
fn.TranslationKeys = {};
|
||||
return { __esModule: true, default: fn, TranslationKeys: {} };
|
||||
});
|
||||
|
||||
jest.mock('../useDelayedUploadToast', () => ({
|
||||
useDelayedUploadToast: jest.fn(() => ({
|
||||
startUploadTimer: jest.fn(),
|
||||
clearUploadTimer: jest.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/utils/heicConverter', () => ({
|
||||
processFileForUpload: jest.fn(async (file: File) => file),
|
||||
}));
|
||||
|
||||
jest.mock('../useClientResize', () => ({
|
||||
__esModule: true,
|
||||
default: jest.fn(() => ({
|
||||
resizeImageIfNeeded: jest.fn(async (file: File) => ({ file, resized: false })),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('../useUpdateFiles', () => ({
|
||||
__esModule: true,
|
||||
default: jest.fn(() => ({
|
||||
addFile: jest.fn(),
|
||||
replaceFile: jest.fn(),
|
||||
updateFileById: jest.fn(),
|
||||
deleteFileById: jest.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/utils', () => ({
|
||||
logger: { log: jest.fn() },
|
||||
validateFiles: jest.fn(() => true),
|
||||
}));
|
||||
|
||||
const mockValidateFiles = jest.requireMock('~/utils').validateFiles;
|
||||
|
||||
describe('useFileHandling', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockConversation = {};
|
||||
});
|
||||
|
||||
const loadHook = async () => (await import('../useFileHandling')).default;
|
||||
|
||||
describe('endpointOverride', () => {
|
||||
it('uses conversation endpoint when no override is provided', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'openAI',
|
||||
endpointType: 'custom',
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() => useFileHandling());
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockValidateFiles).toHaveBeenCalledTimes(1);
|
||||
const validateCall = mockValidateFiles.mock.calls[0][0];
|
||||
const configResult = getEndpointFileConfig({
|
||||
endpoint: 'openAI',
|
||||
endpointType: 'custom',
|
||||
fileConfig: null,
|
||||
});
|
||||
expect(validateCall.endpointFileConfig).toEqual(configResult);
|
||||
});
|
||||
|
||||
it('uses endpointOverride for validation instead of conversation endpoint', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'openAI',
|
||||
endpointType: 'custom',
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() =>
|
||||
useFileHandling({ endpointOverride: EModelEndpoint.agents }),
|
||||
);
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockValidateFiles).toHaveBeenCalledTimes(1);
|
||||
const validateCall = mockValidateFiles.mock.calls[0][0];
|
||||
const agentsConfig = getEndpointFileConfig({
|
||||
endpoint: EModelEndpoint.agents,
|
||||
endpointType: EModelEndpoint.agents,
|
||||
fileConfig: null,
|
||||
});
|
||||
expect(validateCall.endpointFileConfig).toEqual(agentsConfig);
|
||||
});
|
||||
|
||||
it('falls back to conversation endpoint when endpointOverride is undefined', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'anthropic',
|
||||
endpointType: undefined,
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() => useFileHandling({ endpointOverride: undefined }));
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockValidateFiles).toHaveBeenCalledTimes(1);
|
||||
const validateCall = mockValidateFiles.mock.calls[0][0];
|
||||
const anthropicConfig = getEndpointFileConfig({
|
||||
endpoint: 'anthropic',
|
||||
endpointType: undefined,
|
||||
fileConfig: null,
|
||||
});
|
||||
expect(validateCall.endpointFileConfig).toEqual(anthropicConfig);
|
||||
});
|
||||
|
||||
it('sends correct endpoint in upload form data when override is set', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'openAI',
|
||||
endpointType: 'custom',
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() =>
|
||||
useFileHandling({
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
additionalMetadata: { agent_id: 'agent-123' },
|
||||
}),
|
||||
);
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockMutate).toHaveBeenCalledTimes(1);
|
||||
const formData: FormData = mockMutate.mock.calls[0][0];
|
||||
expect(formData.get('endpoint')).toBe(EModelEndpoint.agents);
|
||||
expect(formData.get('endpointType')).toBe(EModelEndpoint.agents);
|
||||
});
|
||||
|
||||
it('does not enter assistants upload path when override is agents', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'assistants',
|
||||
endpointType: 'assistants',
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() =>
|
||||
useFileHandling({
|
||||
endpointOverride: EModelEndpoint.agents,
|
||||
additionalMetadata: { agent_id: 'agent-123' },
|
||||
}),
|
||||
);
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockMutate).toHaveBeenCalledTimes(1);
|
||||
const formData: FormData = mockMutate.mock.calls[0][0];
|
||||
expect(formData.get('endpoint')).toBe(EModelEndpoint.agents);
|
||||
expect(formData.get('message_file')).toBeNull();
|
||||
expect(formData.get('version')).toBeNull();
|
||||
expect(formData.get('model')).toBeNull();
|
||||
expect(formData.get('assistant_id')).toBeNull();
|
||||
});
|
||||
|
||||
it('enters assistants path without override when conversation is assistants', async () => {
|
||||
mockConversation = {
|
||||
conversationId: 'convo-1',
|
||||
endpoint: 'assistants',
|
||||
endpointType: 'assistants',
|
||||
assistant_id: 'asst-456',
|
||||
model: 'gpt-4',
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() => useFileHandling());
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockMutate).toHaveBeenCalledTimes(1);
|
||||
const formData: FormData = mockMutate.mock.calls[0][0];
|
||||
expect(formData.get('endpoint')).toBe('assistants');
|
||||
expect(formData.get('message_file')).toBe('true');
|
||||
});
|
||||
|
||||
it('falls back to "default" when no conversation endpoint and no override', async () => {
|
||||
mockConversation = {
|
||||
conversationId: Constants.NEW_CONVO,
|
||||
endpoint: null,
|
||||
endpointType: undefined,
|
||||
};
|
||||
|
||||
const useFileHandling = await loadHook();
|
||||
const { result } = renderHook(() => useFileHandling());
|
||||
|
||||
const textFile = new File(['hello'], 'test.txt', { type: 'text/plain' });
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleFiles([textFile]);
|
||||
});
|
||||
|
||||
expect(mockMutate).toHaveBeenCalledTimes(1);
|
||||
const formData: FormData = mockMutate.mock.calls[0][0];
|
||||
expect(formData.get('endpoint')).toBe('default');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -13,7 +13,7 @@ import {
|
|||
defaultAssistantsVersion,
|
||||
} from 'librechat-data-provider';
|
||||
import debounce from 'lodash/debounce';
|
||||
import type { TEndpointsConfig, TError } from 'librechat-data-provider';
|
||||
import type { EModelEndpoint, TEndpointsConfig, TError } from 'librechat-data-provider';
|
||||
import type { ExtendedFile, FileSetter } from '~/common';
|
||||
import { useGetFileConfig, useUploadFileMutation } from '~/data-provider';
|
||||
import useLocalize, { TranslationKeys } from '~/hooks/useLocalize';
|
||||
|
|
@ -29,6 +29,8 @@ type UseFileHandling = {
|
|||
fileSetter?: FileSetter;
|
||||
fileFilter?: (file: File) => boolean;
|
||||
additionalMetadata?: Record<string, string | undefined>;
|
||||
/** Overrides both `endpoint` and `endpointType` for validation and upload routing */
|
||||
endpointOverride?: EModelEndpoint;
|
||||
};
|
||||
|
||||
const useFileHandling = (params?: UseFileHandling) => {
|
||||
|
|
@ -50,8 +52,15 @@ const useFileHandling = (params?: UseFileHandling) => {
|
|||
|
||||
const agent_id = params?.additionalMetadata?.agent_id ?? '';
|
||||
const assistant_id = params?.additionalMetadata?.assistant_id ?? '';
|
||||
const endpointType = useMemo(() => conversation?.endpointType, [conversation?.endpointType]);
|
||||
const endpoint = useMemo(() => conversation?.endpoint ?? 'default', [conversation?.endpoint]);
|
||||
const endpointOverride = params?.endpointOverride;
|
||||
const endpointType = useMemo(
|
||||
() => endpointOverride ?? conversation?.endpointType,
|
||||
[endpointOverride, conversation?.endpointType],
|
||||
);
|
||||
const endpoint = useMemo(
|
||||
() => endpointOverride ?? conversation?.endpoint ?? 'default',
|
||||
[endpointOverride, conversation?.endpoint],
|
||||
);
|
||||
|
||||
const { data: fileConfig = null } = useGetFileConfig({
|
||||
select: (data) => mergeFileConfig(data),
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { useCallback } from 'react';
|
||||
import useFileHandling from './useFileHandling';
|
||||
import useSharePointDownload from './useSharePointDownload';
|
||||
import type { EModelEndpoint } from 'librechat-data-provider';
|
||||
import type { SharePointFile } from '~/data-provider/Files/sharepoint';
|
||||
|
||||
interface UseSharePointFileHandlingProps {
|
||||
|
|
@ -8,6 +9,7 @@ interface UseSharePointFileHandlingProps {
|
|||
toolResource?: string;
|
||||
fileFilter?: (file: File) => boolean;
|
||||
additionalMetadata?: Record<string, string | undefined>;
|
||||
endpointOverride?: EModelEndpoint;
|
||||
}
|
||||
|
||||
interface UseSharePointFileHandlingReturn {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue