🔄 fix: URL Param Race Condition and File Draft Persistence (#7257)

* chore(useAutoSave): linting

* fix: files attached during streaming disappear when stream finishes

* fix(useQueryParams): query parameter processing race condition with submission handling, add JSDocs to all functions/hooks

* test(useQueryParams): add comprehensive tests for query parameter handling and submission logic
This commit is contained in:
Danny Avila 2025-05-06 22:49:12 -04:00 committed by GitHub
parent 20c9f1a783
commit 7c4c3a8796
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 675 additions and 16 deletions

View file

@ -75,9 +75,9 @@ export const useAutoSave = ({
const { fileToRecover, fileIdToRecover } = fileData
? { fileToRecover: fileData, fileIdToRecover: fileId }
: {
fileToRecover: tempFileData,
fileIdToRecover: (tempFileData?.temp_file_id ?? '') || fileId,
};
fileToRecover: tempFileData,
fileIdToRecover: (tempFileData?.temp_file_id ?? '') || fileId,
};
if (fileToRecover) {
setFiles((currentFiles) => {
@ -188,7 +188,7 @@ export const useAutoSave = ({
`${LocalStorageKeys.TEXT_DRAFT}${Constants.PENDING_CONVO}`,
);
// Clear the pending draft, if it exists, and save the current draft to the new conversationId;
// Clear the pending text draft, if it exists, and save the current draft to the new conversationId;
// otherwise, save the current text area value to the new conversationId
localStorage.removeItem(`${LocalStorageKeys.TEXT_DRAFT}${Constants.PENDING_CONVO}`);
if (pendingDraft) {
@ -199,6 +199,21 @@ export const useAutoSave = ({
encodeBase64(textAreaRef.current.value),
);
}
const pendingFileDraft = localStorage.getItem(
`${LocalStorageKeys.FILES_DRAFT}${Constants.PENDING_CONVO}`,
);
if (pendingFileDraft) {
localStorage.setItem(
`${LocalStorageKeys.FILES_DRAFT}${conversationId}`,
pendingFileDraft,
);
localStorage.removeItem(`${LocalStorageKeys.FILES_DRAFT}${Constants.PENDING_CONVO}`);
const filesDraft = JSON.parse(pendingFileDraft || '[]') as string[];
if (filesDraft.length > 0) {
restoreFiles(conversationId);
}
}
} else if (currentConversationId != null && currentConversationId) {
saveText(currentConversationId);
}

View file

@ -0,0 +1,489 @@
// useQueryParams.spec.ts
jest.mock('recoil', () => {
const originalModule = jest.requireActual('recoil');
return {
...originalModule,
atom: jest.fn().mockImplementation((config) => ({
key: config.key,
default: config.default,
})),
useRecoilValue: jest.fn(),
};
});
// Move mock store definition after the mocks
jest.mock('~/store', () => ({
modularChat: { key: 'modularChat', default: false },
availableTools: { key: 'availableTools', default: [] },
}));
import { renderHook, act } from '@testing-library/react';
import { useSearchParams } from 'react-router-dom';
import { useQueryClient } from '@tanstack/react-query';
import { useRecoilValue } from 'recoil';
import useQueryParams from './useQueryParams';
import { useChatContext, useChatFormContext } from '~/Providers';
import useSubmitMessage from '~/hooks/Messages/useSubmitMessage';
import useDefaultConvo from '~/hooks/Conversations/useDefaultConvo';
import store from '~/store';
// Other mocks
jest.mock('react-router-dom', () => ({
useSearchParams: jest.fn(),
}));
jest.mock('@tanstack/react-query', () => ({
useQueryClient: jest.fn(),
}));
jest.mock('~/Providers', () => ({
useChatContext: jest.fn(),
useChatFormContext: jest.fn(),
}));
jest.mock('~/hooks/Messages/useSubmitMessage', () => ({
__esModule: true,
default: jest.fn(),
}));
jest.mock('~/hooks/Conversations/useDefaultConvo', () => ({
__esModule: true,
default: jest.fn(),
}));
jest.mock('~/utils', () => ({
getConvoSwitchLogic: jest.fn(() => ({
template: {},
shouldSwitch: false,
isNewModular: false,
newEndpointType: null,
isCurrentModular: false,
isExistingConversation: false,
})),
getModelSpecIconURL: jest.fn(() => 'icon-url'),
removeUnavailableTools: jest.fn((preset) => preset),
logger: { log: jest.fn() },
}));
// Mock the tQueryParamsSchema
jest.mock('librechat-data-provider', () => ({
...jest.requireActual('librechat-data-provider'),
tQueryParamsSchema: {
shape: {
model: { parse: jest.fn((value) => value) },
endpoint: { parse: jest.fn((value) => value) },
temperature: { parse: jest.fn((value) => value) },
// Add other schema shapes as needed
},
},
isAgentsEndpoint: jest.fn(() => false),
isAssistantsEndpoint: jest.fn(() => false),
QueryKeys: { startupConfig: 'startupConfig', endpoints: 'endpoints' },
EModelEndpoint: { custom: 'custom', assistants: 'assistants', agents: 'agents' },
}));
// Mock global window.history
global.window = Object.create(window);
global.window.history = {
replaceState: jest.fn(),
pushState: jest.fn(),
go: jest.fn(),
back: jest.fn(),
forward: jest.fn(),
length: 1,
scrollRestoration: 'auto',
state: null,
};
describe('useQueryParams', () => {
// Setup common mocks before each test
beforeEach(() => {
jest.useFakeTimers();
// Reset mock for window.history.replaceState
jest.spyOn(window.history, 'replaceState').mockClear();
// Create mocks for all dependencies
const mockSearchParams = new URLSearchParams();
(useSearchParams as jest.Mock).mockReturnValue([mockSearchParams, jest.fn()]);
const mockQueryClient = {
getQueryData: jest.fn().mockImplementation((key) => {
if (key === 'startupConfig') {
return { modelSpecs: { list: [] } };
}
if (key === 'endpoints') {
return {};
}
return null;
}),
};
(useQueryClient as jest.Mock).mockReturnValue(mockQueryClient);
(useRecoilValue as jest.Mock).mockImplementation((atom) => {
if (atom === store.modularChat) return false;
if (atom === store.availableTools) return [];
return null;
});
const mockConversation = { model: null, endpoint: null };
const mockNewConversation = jest.fn();
(useChatContext as jest.Mock).mockReturnValue({
conversation: mockConversation,
newConversation: mockNewConversation,
});
const mockMethods = {
setValue: jest.fn(),
getValues: jest.fn().mockReturnValue(''),
handleSubmit: jest.fn((callback) => () => callback({ text: 'test message' })),
};
(useChatFormContext as jest.Mock).mockReturnValue(mockMethods);
const mockSubmitMessage = jest.fn();
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
const mockGetDefaultConversation = jest.fn().mockReturnValue({});
(useDefaultConvo as jest.Mock).mockReturnValue(mockGetDefaultConversation);
});
afterEach(() => {
jest.clearAllMocks();
jest.useRealTimers();
});
// Helper function to set URL parameters for testing
const setUrlParams = (params: Record<string, string>) => {
const searchParams = new URLSearchParams();
Object.entries(params).forEach(([key, value]) => {
searchParams.set(key, value);
});
(useSearchParams as jest.Mock).mockReturnValue([searchParams, jest.fn()]);
};
// Test cases remain the same
it('should process query parameters on initial render', () => {
// Setup
const mockSetValue = jest.fn();
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: jest.fn((callback) => () => callback({ text: 'test message' })),
});
// Mock startup config to allow processing
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: jest.fn().mockReturnValue({ modelSpecs: { list: [] } }),
});
setUrlParams({ q: 'hello world' });
// Execute
renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
// Advance timer to trigger interval
act(() => {
jest.advanceTimersByTime(100);
});
// Assert
expect(mockSetValue).toHaveBeenCalledWith(
'text',
'hello world',
expect.objectContaining({ shouldValidate: true }),
);
expect(window.history.replaceState).toHaveBeenCalled();
});
it('should auto-submit message when submit=true and no settings to apply', () => {
// Setup
const mockSetValue = jest.fn();
const mockHandleSubmit = jest.fn((callback) => () => callback({ text: 'test message' }));
const mockSubmitMessage = jest.fn();
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: mockHandleSubmit,
});
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
// Mock startup config to allow processing
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: jest.fn().mockReturnValue({ modelSpecs: { list: [] } }),
});
setUrlParams({ q: 'hello world', submit: 'true' });
// Execute
renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
// Advance timer to trigger interval
act(() => {
jest.advanceTimersByTime(100);
});
// Assert
expect(mockSetValue).toHaveBeenCalledWith(
'text',
'hello world',
expect.objectContaining({ shouldValidate: true }),
);
expect(mockHandleSubmit).toHaveBeenCalled();
expect(mockSubmitMessage).toHaveBeenCalled();
});
it('should defer submission when settings need to be applied first', () => {
// Setup
const mockSetValue = jest.fn();
const mockHandleSubmit = jest.fn((callback) => () => callback({ text: 'test message' }));
const mockSubmitMessage = jest.fn();
const mockNewConversation = jest.fn();
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
// Mock getQueryData to return array format for startupConfig
const mockGetQueryData = jest.fn().mockImplementation((key) => {
if (Array.isArray(key) && key[0] === 'startupConfig') {
return { modelSpecs: { list: [] } };
}
if (key === 'startupConfig') {
return { modelSpecs: { list: [] } };
}
return null;
});
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: mockHandleSubmit,
});
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
(useChatContext as jest.Mock).mockReturnValue({
conversation: { model: null, endpoint: null },
newConversation: mockNewConversation,
});
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: mockGetQueryData,
});
setUrlParams({ q: 'hello world', submit: 'true', model: 'gpt-4' });
// Execute
const { rerender } = renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
// First interval tick should process params but not submit
act(() => {
jest.advanceTimersByTime(100);
});
// Assert initial state
expect(mockGetQueryData).toHaveBeenCalledWith(expect.anything());
expect(mockNewConversation).toHaveBeenCalled();
expect(mockSubmitMessage).not.toHaveBeenCalled(); // Not submitted yet
// Now mock conversation update to trigger settings application check
(useChatContext as jest.Mock).mockReturnValue({
conversation: { model: 'gpt-4', endpoint: null },
newConversation: mockNewConversation,
});
// Re-render to trigger the effect that watches for settings
rerender();
// Now the message should be submitted
expect(mockSetValue).toHaveBeenCalledWith(
'text',
'hello world',
expect.objectContaining({ shouldValidate: true }),
);
expect(mockHandleSubmit).toHaveBeenCalled();
expect(mockSubmitMessage).toHaveBeenCalled();
});
it('should submit after timeout if settings never get applied', () => {
// Setup
const mockSetValue = jest.fn();
const mockHandleSubmit = jest.fn((callback) => () => callback({ text: 'test message' }));
const mockSubmitMessage = jest.fn();
const mockNewConversation = jest.fn();
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: mockHandleSubmit,
});
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
(useChatContext as jest.Mock).mockReturnValue({
conversation: { model: null, endpoint: null },
newConversation: mockNewConversation,
});
// Mock startup config to allow processing
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: jest.fn().mockImplementation((key) => {
if (Array.isArray(key) && key[0] === 'startupConfig') {
return { modelSpecs: { list: [] } };
}
if (key === 'startupConfig') {
return { modelSpecs: { list: [] } };
}
return null;
}),
});
setUrlParams({ q: 'hello world', submit: 'true', model: 'non-existent-model' });
// Execute
renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
// First interval tick should process params but not submit
act(() => {
jest.advanceTimersByTime(100);
});
// Assert initial state
expect(mockSubmitMessage).not.toHaveBeenCalled(); // Not submitted yet
// Let the timeout happen naturally
act(() => {
// Advance timer to trigger the timeout in the hook
jest.advanceTimersByTime(3000); // MAX_SETTINGS_WAIT_MS
});
// Now the message should be submitted due to timeout
expect(mockSubmitMessage).toHaveBeenCalled();
});
it('should mark as submitted when no submit parameter is present', () => {
// Setup
const mockSetValue = jest.fn();
const mockHandleSubmit = jest.fn((callback) => () => callback({ text: 'test message' }));
const mockSubmitMessage = jest.fn();
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: mockHandleSubmit,
});
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
// Mock startup config to allow processing
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: jest.fn().mockReturnValue({ modelSpecs: { list: [] } }),
});
setUrlParams({ model: 'gpt-4' }); // No submit=true
// Execute
renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
// First interval tick should process params
act(() => {
jest.advanceTimersByTime(100);
});
// Assert initial state - submission should be marked as handled
expect(mockSubmitMessage).not.toHaveBeenCalled();
// Try to advance timer past the timeout
act(() => {
jest.advanceTimersByTime(4000);
});
// Submission still shouldn't happen
expect(mockSubmitMessage).not.toHaveBeenCalled();
});
it('should handle empty query parameters', () => {
// Setup
const mockSetValue = jest.fn();
const mockHandleSubmit = jest.fn();
const mockSubmitMessage = jest.fn();
// Force replaceState to be called
window.history.replaceState = jest.fn();
(useChatFormContext as jest.Mock).mockReturnValue({
setValue: mockSetValue,
getValues: jest.fn().mockReturnValue(''),
handleSubmit: mockHandleSubmit,
});
(useSubmitMessage as jest.Mock).mockReturnValue({
submitMessage: mockSubmitMessage,
});
// Mock startup config to allow processing
(useQueryClient as jest.Mock).mockReturnValue({
getQueryData: jest.fn().mockReturnValue({ modelSpecs: { list: [] } }),
});
setUrlParams({}); // Empty params
const mockTextAreaRef = {
current: {
focus: jest.fn(),
setSelectionRange: jest.fn(),
} as unknown as HTMLTextAreaElement,
};
// Execute
renderHook(() => useQueryParams({ textAreaRef: mockTextAreaRef }));
act(() => {
jest.advanceTimersByTime(100);
});
// Assert
expect(mockSetValue).not.toHaveBeenCalled();
expect(mockHandleSubmit).not.toHaveBeenCalled();
expect(mockSubmitMessage).not.toHaveBeenCalled();
expect(window.history.replaceState).toHaveBeenCalled();
});
});

View file

@ -17,6 +17,10 @@ import { useChatContext, useChatFormContext } from '~/Providers';
import useSubmitMessage from '~/hooks/Messages/useSubmitMessage';
import store from '~/store';
/**
* Parses query parameter values, converting strings to their appropriate types.
* Handles boolean strings, numbers, and preserves regular strings.
*/
const parseQueryValue = (value: string) => {
if (value === 'true') {
return true;
@ -30,6 +34,11 @@ const parseQueryValue = (value: string) => {
return value;
};
/**
* Processes and validates URL query parameters using schema definitions.
* Extracts valid settings based on tQueryParamsSchema and handles special endpoint cases
* for assistants and agents.
*/
const processValidSettings = (queryParams: Record<string, string>) => {
const validSettings = {} as TPreset;
@ -64,6 +73,11 @@ const processValidSettings = (queryParams: Record<string, string>) => {
return validSettings;
};
/**
* Hook that processes URL query parameters to initialize chat with specified settings and prompt.
* Handles model switching, prompt auto-filling, and optional auto-submission with race condition protection.
* Supports immediate or deferred submission based on whether settings need to be applied first.
*/
export default function useQueryParams({
textAreaRef,
}: {
@ -71,7 +85,15 @@ export default function useQueryParams({
}) {
const maxAttempts = 50;
const attemptsRef = useRef(0);
const MAX_SETTINGS_WAIT_MS = 3000;
const processedRef = useRef(false);
const pendingSubmitRef = useRef(false);
const settingsAppliedRef = useRef(false);
const submissionHandledRef = useRef(false);
const promptTextRef = useRef<string | null>(null);
const validSettingsRef = useRef<TPreset | null>(null);
const settingsTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const methods = useChatFormContext();
const [searchParams] = useSearchParams();
const getDefaultConversation = useDefaultConvo();
@ -82,6 +104,11 @@ export default function useQueryParams({
const queryClient = useQueryClient();
const { conversation, newConversation } = useChatContext();
/**
* Applies settings from URL query parameters to create a new conversation.
* Handles model spec lookup, endpoint normalization, and conversation switching logic.
* Ensures tools compatibility and preserves existing conversation when appropriate.
*/
const newQueryConvo = useCallback(
(_newPreset?: TPreset) => {
if (!_newPreset) {
@ -181,6 +208,85 @@ export default function useQueryParams({
],
);
/**
* Checks if all settings from URL parameters have been successfully applied to the conversation.
* Compares values from validSettings against the current conversation state, handling special properties.
* Returns true only when all relevant settings match the target values.
*/
const areSettingsApplied = useCallback(() => {
if (!validSettingsRef.current || !conversation) {
return false;
}
for (const [key, value] of Object.entries(validSettingsRef.current)) {
if (['presetOverride', 'iconURL', 'spec', 'modelLabel'].includes(key)) {
continue;
}
if (conversation[key] !== value) {
return false;
}
}
return true;
}, [conversation]);
/**
* Processes message submission exactly once, preventing duplicate submissions.
* Sets the prompt text, submits the message, and cleans up URL parameters afterward.
* Has internal guards to ensure it only executes once regardless of how many times it's called.
*/
const processSubmission = useCallback(() => {
if (submissionHandledRef.current || !pendingSubmitRef.current || !promptTextRef.current) {
return;
}
submissionHandledRef.current = true;
pendingSubmitRef.current = false;
methods.setValue('text', promptTextRef.current, { shouldValidate: true });
methods.handleSubmit((data) => {
if (data.text?.trim()) {
submitMessage(data);
const newUrl = window.location.pathname;
window.history.replaceState({}, '', newUrl);
console.log('Message submitted with conversation state:', conversation);
}
})();
}, [methods, submitMessage, conversation]);
useEffect(() => {
// Only proceed if we've already processed URL parameters but haven't yet handled submission
if (
!processedRef.current ||
submissionHandledRef.current ||
settingsAppliedRef.current ||
!validSettingsRef.current ||
!conversation
) {
return;
}
const allSettingsApplied = areSettingsApplied();
if (allSettingsApplied) {
settingsAppliedRef.current = true;
if (pendingSubmitRef.current) {
if (settingsTimeoutRef.current) {
clearTimeout(settingsTimeoutRef.current);
settingsTimeoutRef.current = null;
}
console.log('Settings fully applied, processing submission');
processSubmission();
}
}
}, [conversation, processSubmission, areSettingsApplied]);
useEffect(() => {
const processQueryParams = () => {
const queryParams: Record<string, string> = {};
@ -217,31 +323,68 @@ export default function useQueryParams({
if (!startupConfig) {
return;
}
const { decodedPrompt, validSettings, shouldAutoSubmit } = processQueryParams();
const currentText = methods.getValues('text');
/** Clean up URL parameters after successful processing */
const { decodedPrompt, validSettings, shouldAutoSubmit } = processQueryParams();
if (!shouldAutoSubmit) {
submissionHandledRef.current = true;
}
/** Mark processing as complete and clean up as needed */
const success = () => {
const newUrl = window.location.pathname;
window.history.replaceState({}, '', newUrl);
processedRef.current = true;
console.log('Parameters processed successfully');
clearInterval(intervalId);
// Only clean URL if there's no pending submission
if (!pendingSubmitRef.current) {
const newUrl = window.location.pathname;
window.history.replaceState({}, '', newUrl);
}
};
if (!currentText && decodedPrompt) {
methods.setValue('text', decodedPrompt, { shouldValidate: true });
textAreaRef.current.focus();
textAreaRef.current.setSelectionRange(decodedPrompt.length, decodedPrompt.length);
// Store settings for later comparison
if (Object.keys(validSettings).length > 0) {
validSettingsRef.current = validSettings;
}
// Save the prompt text for later use if needed
if (decodedPrompt) {
promptTextRef.current = decodedPrompt;
}
// Handle auto-submission
if (shouldAutoSubmit && decodedPrompt) {
if (Object.keys(validSettings).length > 0) {
// Settings are changing, defer submission
pendingSubmitRef.current = true;
// Set a timeout to handle the case where settings might never fully apply
settingsTimeoutRef.current = setTimeout(() => {
if (!submissionHandledRef.current && pendingSubmitRef.current) {
console.warn(
'Settings application timeout reached, proceeding with submission anyway',
);
processSubmission();
}
}, MAX_SETTINGS_WAIT_MS);
} else {
methods.setValue('text', decodedPrompt, { shouldValidate: true });
textAreaRef.current.focus();
textAreaRef.current.setSelectionRange(decodedPrompt.length, decodedPrompt.length);
// Auto-submit if the submit parameter is true
if (shouldAutoSubmit) {
methods.handleSubmit((data) => {
if (data.text?.trim()) {
submitMessage(data);
}
})();
}
} else if (decodedPrompt) {
methods.setValue('text', decodedPrompt, { shouldValidate: true });
textAreaRef.current.focus();
textAreaRef.current.setSelectionRange(decodedPrompt.length, decodedPrompt.length);
} else {
submissionHandledRef.current = true;
}
if (Object.keys(validSettings).length > 0) {
@ -253,6 +396,18 @@ export default function useQueryParams({
return () => {
clearInterval(intervalId);
if (settingsTimeoutRef.current) {
clearTimeout(settingsTimeoutRef.current);
}
};
}, [searchParams, methods, textAreaRef, newQueryConvo, newConversation, submitMessage]);
}, [
searchParams,
methods,
textAreaRef,
newQueryConvo,
newConversation,
submitMessage,
queryClient,
processSubmission,
]);
}