🧵 fix: Reject Preliminary Parent Follow-Ups (#13619)

* fix: Reject preliminary parent follow-ups

* chore: Sort frontend imports

* fix: Narrow preliminary parent detection

* fix: Preserve refused submit state

* fix: Propagate refused submit result
This commit is contained in:
Danny Avila 2026-06-09 12:06:51 -04:00 committed by GitHub
parent 753e53eddd
commit fd4728232c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 426 additions and 16 deletions

View file

@ -19,6 +19,7 @@ const mockFilterPersistableAbortContent = jest.fn((content) =>
content.filter((part) => part?.type !== 'tool_call'),
);
const mockGetConvo = jest.fn();
const mockGetMessages = jest.fn();
const mockSaveMessage = jest.fn();
jest.mock('@librechat/data-schemas', () => ({
@ -35,6 +36,24 @@ jest.mock('@librechat/api', () => ({
decrementPendingRequest: (...args) => mockDecrementPendingRequest(...args),
sanitizeMessageForTransmit: jest.fn((message) => message),
checkAndIncrementPendingRequest: (...args) => mockCheckAndIncrementPendingRequest(...args),
isUnpersistedPreliminaryParent: async ({
userId,
conversationId,
parentMessageId,
getMessages,
}) => {
if (typeof parentMessageId !== 'string' || !parentMessageId.endsWith('_')) {
return false;
}
const filter = { user: userId, messageId: parentMessageId };
if (conversationId && conversationId !== 'new') {
filter.conversationId = conversationId;
}
const messages = await getMessages(filter, '_id');
return messages.length === 0;
},
}));
jest.mock('~/server/cleanup', () => ({
@ -55,6 +74,7 @@ jest.mock('~/cache', () => ({
jest.mock('~/models', () => ({
saveMessage: (...args) => mockSaveMessage(...args),
getMessages: (...args) => mockGetMessages(...args),
getConvo: (...args) => mockGetConvo(...args),
}));
@ -66,6 +86,7 @@ describe('ResumableAgentController resume metadata', () => {
mockCheckAndIncrementPendingRequest.mockResolvedValue({ allowed: true });
mockDecrementPendingRequest.mockResolvedValue(undefined);
mockGetConvo.mockResolvedValue({ createdAt: '2026-06-07T00:00:00.000Z' });
mockGetMessages.mockResolvedValue([]);
mockGenerationJobManager.createJob.mockResolvedValue({
createdAt: 1000,
readyPromise: Promise.resolve(),
@ -78,6 +99,86 @@ describe('ResumableAgentController resume metadata', () => {
mockSaveMessage.mockResolvedValue({});
});
it('rejects an underscore-suffixed parent that is not persisted', async () => {
const conversationId = 'conversation-123';
const initializeClient = jest.fn();
const req = {
user: { id: 'user-123' },
body: {
text: 'Follow up too early.',
messageId: 'follow-up-user',
parentMessageId: 'pending-response_',
conversationId,
endpointOption: {
endpoint: 'agents',
modelOptions: { model: 'gpt-3.5-turbo' },
},
},
config: {},
};
const res = {
json: jest.fn(),
status: jest.fn(() => res),
};
await AgentController(req, res, jest.fn(), initializeClient, null);
expect(mockGetMessages).toHaveBeenCalledWith(
{ user: 'user-123', messageId: 'pending-response_', conversationId },
'_id',
);
expect(res.status).toHaveBeenCalledWith(409);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({
error: expect.stringContaining('selected parent response is still being saved'),
}),
);
expect(mockCheckAndIncrementPendingRequest).not.toHaveBeenCalled();
expect(mockGenerationJobManager.createJob).not.toHaveBeenCalled();
expect(initializeClient).not.toHaveBeenCalled();
});
it('allows an underscore-suffixed parent when it is already persisted', async () => {
const conversationId = 'conversation-123';
mockGetMessages.mockResolvedValue([{ _id: 'persisted-parent' }]);
const initializeClient = jest.fn().mockRejectedValue(new Error('stop before tool loading'));
const req = {
user: { id: 'user-123' },
body: {
text: 'Follow up to persisted underscore id.',
messageId: 'follow-up-user',
parentMessageId: 'persisted-response_',
conversationId,
endpointOption: {
endpoint: 'agents',
modelOptions: { model: 'gpt-3.5-turbo' },
},
},
config: {},
};
const res = {
headersSent: true,
json: jest.fn(() => {
res.headersSent = true;
}),
status: jest.fn(() => res),
};
await AgentController(req, res, jest.fn(), initializeClient, null);
expect(mockGetMessages).toHaveBeenCalledWith(
{ user: 'user-123', messageId: 'persisted-response_', conversationId },
'_id',
);
expect(res.status).not.toHaveBeenCalledWith(409);
expect(mockCheckAndIncrementPendingRequest).toHaveBeenCalledWith('user-123');
expect(mockGenerationJobManager.createJob).toHaveBeenCalledWith(
conversationId,
'user-123',
conversationId,
);
});
it('stores the in-flight turn before MCP initialization can emit OAuth', async () => {
const conversationId = 'conversation-123';
const initializeClient = jest.fn().mockRejectedValue(new Error('stop before tool loading'));

View file

@ -10,11 +10,12 @@ const {
decrementPendingRequest,
sanitizeMessageForTransmit,
checkAndIncrementPendingRequest,
isUnpersistedPreliminaryParent,
} = require('@librechat/api');
const { disposeClient, clientRegistry, requestDataMap } = require('~/server/cleanup');
const { handleAbortError } = require('~/server/middleware');
const { logViolation } = require('~/cache');
const { saveMessage, getConvo } = require('~/models');
const { saveMessage, getMessages, getConvo } = require('~/models');
function createCloseHandler(abortController) {
return function (manual) {
@ -138,6 +139,13 @@ function getAgentResponseModel(req, endpointOption) {
return getEndpointResponseModel(endpointOption);
}
function rejectPreliminaryParentMessageId(res) {
return res.status(409).json({
error:
'Cannot submit a follow-up while the selected parent response is still being saved. Please wait and try again.',
});
}
/**
* Resumable Agent Controller - Generation runs independently of HTTP connection.
* Returns streamId immediately, client subscribes separately via SSE.
@ -157,6 +165,17 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit
const userId = req.user.id;
if (
await isUnpersistedPreliminaryParent({
userId,
conversationId: reqConversationId,
parentMessageId,
getMessages,
})
) {
return rejectPreliminaryParentMessageId(res);
}
/** When to generate the conversation title. `immediate` (default) fires title
* generation in parallel with the response, from the user's first message;
* `final` defers it until the full response completes (legacy behavior).
@ -744,6 +763,17 @@ const _LegacyAgentController = async (req, res, next, initializeClient, addTitle
// Match the same logic used for conversationId generation above
const userId = req.user.id;
if (
await isUnpersistedPreliminaryParent({
userId,
conversationId: reqConversationId,
parentMessageId,
getMessages,
})
) {
return rejectPreliminaryParentMessageId(res);
}
await attachConversationCreatedAt(req, { userId, conversationId, isNewConvo });
// Create handler to avoid capturing the entire parent scope

View file

@ -369,7 +369,7 @@ export type TOptions = {
addedConvo?: t.TConversation;
};
export type TAskFunction = (props: TAskProps, options?: TOptions) => void;
export type TAskFunction = (props: TAskProps, options?: TOptions) => false | void;
/**
* Stable context object passed from non-memo'd wrapper components (Message, MessageContent)

View file

@ -2,8 +2,8 @@ import { memo, useCallback, useRef } from 'react';
import { MicOff } from 'lucide-react';
import { useToastContext, TooltipAnchor, ListeningIcon, Spinner } from '@librechat/client';
import { useLocalize, useSpeechToText, useGetAudioSettings } from '~/hooks';
import { globalAudioId, type TAskFunction } from '~/common';
import { useChatFormContext } from '~/Providers';
import { globalAudioId } from '~/common';
import { cn } from '~/utils';
const isExternalSTT = (speechToTextEndpoint: string) => speechToTextEndpoint === 'external';
@ -15,7 +15,7 @@ export default memo(function AudioRecorder({
isSubmitting,
}: {
disabled: boolean;
ask: (data: { text: string }) => void;
ask: TAskFunction;
methods: ReturnType<typeof useChatFormContext>;
textAreaRef: React.RefObject<HTMLTextAreaElement>;
isSubmitting: boolean;
@ -49,7 +49,10 @@ export default memo(function AudioRecorder({
isExternalSTT(speechToTextEndpoint) && existingTextRef.current
? `${existingTextRef.current} ${text}`
: text;
ask({ text: finalText });
const submitted = ask({ text: finalText });
if (submitted === false) {
return;
}
reset({ text: '' });
existingTextRef.current = '';
}

View file

@ -7,12 +7,10 @@ import SiblingSwitch from '~/components/Chat/Messages/SiblingSwitch';
import SubRow from '~/components/Chat/Messages/SubRow';
import { fontSizeAtom } from '~/store/fontSize';
import { MessageContext } from '~/Providers';
import { useAttachments } from '~/hooks';
import MultiMessage from './MultiMessage';
import { cn } from '~/utils';
import { useAttachments } from '~/hooks';
import Icon from './MessageIcon';
import { cn } from '~/utils';
export default function Message(props: TMessageProps) {
const fontSize = useAtomValue(fontSizeAtom);
const {
@ -90,7 +88,7 @@ export default function Message(props: TMessageProps) {
edit={false}
error={error}
isLast={false}
ask={() => ({})}
ask={() => {}}
text={text || ''}
message={message}
isSubmitting={false}

View file

@ -1,7 +1,7 @@
import React, { useMemo } from 'react';
import type { TMessage } from 'librechat-data-provider';
import { MessagesViewContext } from '~/Providers/MessagesViewContext';
import type { MessagesViewContextValue } from '~/Providers/MessagesViewContext';
import { MessagesViewContext } from '~/Providers/MessagesViewContext';
interface ShareMessagesProviderProps {
messages: TMessage[];
@ -22,7 +22,7 @@ export function ShareMessagesProvider({ messages, children }: ShareMessagesProvi
conversation: null,
conversationId: undefined,
// These are required by the context but not used in share view
ask: () => Promise.resolve(),
ask: () => {},
regenerate: () => {},
handleContinue: () => {},
latestMessageId: messages[messages.length - 1]?.messageId,

View file

@ -70,6 +70,14 @@ const getAppendParentMessageId = ({
return failedUserMessage.parentMessageId ?? Constants.NO_PARENT;
};
const hasPendingAssistantParent = (message: TMessage | null) =>
!!message?.messageId &&
message.isCreatedByUser !== true &&
message.messageId.endsWith('_') &&
message.createdAt == null &&
message.updatedAt == null &&
!hasStreamStartFailed(message);
type RegenerateTargetResponseArgs = {
messages: TMessage[];
parentMessageId?: string | null;
@ -257,6 +265,14 @@ export default function useChatFunctions({
return;
}
if (parentMessageId == null && hasPendingAssistantParent(latestMessage)) {
logger.warn(
'[useChatFunctions] Refusing to append to a preliminary assistant message',
latestMessage,
);
return false;
}
const ephemeralAgent = getEphemeralAgent(conversationId ?? Constants.NEW_CONVO);
/**
* Manual skill selection resolution:
@ -297,8 +313,9 @@ export default function useChatFunctions({
// construct the query message
// this is not a real messageId, it is used as placeholder before real messageId returned
const intermediateId = overrideUserMessageId ?? v4();
parentMessageId =
parentMessageId ?? getAppendParentMessageId({ latestMessage, currentMessages });
if (parentMessageId == null) {
parentMessageId = getAppendParentMessageId({ latestMessage, currentMessages });
}
logChatRequest({
index,

View file

@ -0,0 +1,85 @@
import { act, renderHook } from '@testing-library/react';
import { useRecoilValue, useSetRecoilState } from 'recoil';
import { useChatContext, useChatFormContext, useAddedChatContext } from '~/Providers';
import { useLatestMessage } from '~/hooks/Messages/useLatestMessage';
import { useAuthContext } from '~/hooks/AuthContext';
import useSubmitMessage from '../useSubmitMessage';
const mockSetActivePrompt = jest.fn();
jest.mock('recoil', () => ({
useRecoilValue: jest.fn(),
useSetRecoilState: jest.fn(),
}));
jest.mock('librechat-data-provider', () => ({
replaceSpecialVars: jest.fn(({ text }) => text),
}));
jest.mock('~/Providers', () => ({
useChatContext: jest.fn(),
useChatFormContext: jest.fn(),
useAddedChatContext: jest.fn(),
}));
jest.mock('~/hooks/AuthContext', () => ({
useAuthContext: jest.fn(),
}));
jest.mock('~/hooks/Messages/useLatestMessage', () => ({
useLatestMessage: jest.fn(),
}));
jest.mock('~/store', () => ({
__esModule: true,
default: {
autoSendPrompts: 'autoSendPrompts',
activePromptByIndex: jest.fn(() => 'activePromptByIndex'),
},
}));
const mockUseRecoilValue = useRecoilValue as jest.Mock;
const mockUseSetRecoilState = useSetRecoilState as jest.Mock;
const mockUseChatContext = useChatContext as jest.Mock;
const mockUseChatFormContext = useChatFormContext as jest.Mock;
const mockUseAddedChatContext = useAddedChatContext as jest.Mock;
const mockUseAuthContext = useAuthContext as jest.Mock;
const mockUseLatestMessage = useLatestMessage as jest.Mock;
describe('useSubmitMessage', () => {
const ask = jest.fn();
const reset = jest.fn();
const setMessages = jest.fn();
const getMessages = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
mockUseRecoilValue.mockReturnValue(false);
mockUseSetRecoilState.mockReturnValue(mockSetActivePrompt);
mockUseAuthContext.mockReturnValue({ user: { id: 'user-1' } });
mockUseAddedChatContext.mockReturnValue({ conversation: null });
mockUseChatFormContext.mockReturnValue({ reset, getValues: jest.fn(() => '') });
mockUseLatestMessage.mockReturnValue({ messageId: 'assistant-message' });
getMessages.mockReturnValue([{ messageId: 'assistant-message' }]);
mockUseChatContext.mockReturnValue({
ask,
index: 0,
getMessages,
setMessages,
});
});
it('propagates blocked submits so direct callers can preserve their text', () => {
ask.mockReturnValue(false);
const { result } = renderHook(() => useSubmitMessage());
let submitted: false | void = undefined;
act(() => {
submitted = result.current.submitMessage({ text: 'dictated follow-up' });
});
expect(submitted).toBe(false);
expect(reset).not.toHaveBeenCalled();
});
});

View file

@ -2,8 +2,8 @@ import { useCallback } from 'react';
import { useRecoilValue, useSetRecoilState } from 'recoil';
import { replaceSpecialVars } from 'librechat-data-provider';
import { useChatContext, useChatFormContext, useAddedChatContext } from '~/Providers';
import { useAuthContext } from '~/hooks/AuthContext';
import { useLatestMessage } from '~/hooks/Messages/useLatestMessage';
import { useAuthContext } from '~/hooks/AuthContext';
import { mainTextareaId } from '~/common';
import store from '~/store';
@ -30,7 +30,7 @@ export default function useSubmitMessage() {
setMessages([...(rootMessages || []), latestMessage]);
}
ask(
const submitted = ask(
{
text: data.text,
},
@ -38,6 +38,9 @@ export default function useSubmitMessage() {
addedConvo: addedConvo ?? undefined,
},
);
if (submitted === false) {
return false;
}
methods.reset();
},
[ask, methods, addedConvo, setMessages, getMessages, latestMessage],

View file

@ -36,6 +36,13 @@ type ForkResponse = {
messages: E2EMessage[];
};
type JsonResponse = {
ok: boolean;
status: number;
text: string;
json: unknown;
};
const uniqueLabel = (name: string) => `${name}-${Date.now()}-${Math.floor(Math.random() * 1e6)}`;
const replyPrompt = (label: string) => `E2E_REPLY:${label}`;
@ -522,6 +529,31 @@ async function fetchMessages(
);
}
async function postJsonWithStatus(page: Page, path: string, token: string, body: unknown) {
return page.evaluate(
async ({ accessToken, requestBody, urlPath }): Promise<JsonResponse> => {
const response = await fetch(urlPath, {
method: 'POST',
credentials: 'include',
headers: {
Authorization: `Bearer ${accessToken}`,
'Content-Type': 'application/json',
},
body: JSON.stringify(requestBody),
});
const text = await response.text();
let json: unknown = null;
try {
json = text ? JSON.parse(text) : null;
} catch {
json = null;
}
return { ok: response.ok, status: response.status, text, json };
},
{ accessToken: token, requestBody: body, urlPath: path },
);
}
async function waitForMessages(
page: Page,
conversationId: string,
@ -993,6 +1025,60 @@ test.describe('message tree stream operations', () => {
]);
});
test('rejects normal follow-ups whose parent is a preliminary assistant placeholder', async ({
page,
}) => {
const label = uniqueLabel('placeholder-parent');
const basePrompt = replyPrompt(`${label}-base`);
const baseReply = replyText(`${label}-base`);
const followPrompt = replyPrompt(`${label}-follow`);
await openMockChat(page);
await sendAndExpectReply(page, basePrompt, baseReply);
const conversationId = await conversationIdFromPage(page);
const token = await getAccessToken(page);
const beforeMessages = await fetchMessages(page, conversationId, token);
const stableParent = findMessage(beforeMessages, baseReply, false);
const response = await postJsonWithStatus(
page,
`/api/agents/chat/${encodeURIComponent(MOCK_ENDPOINTS[0].label)}`,
token,
{
text: followPrompt,
sender: 'User',
clientTimestamp: new Date().toLocaleString('sv').replace(' ', 'T'),
isCreatedByUser: true,
parentMessageId: `${stableParent.messageId}_`,
conversationId,
messageId: `${label}-user-message`,
endpoint: MOCK_ENDPOINTS[0].label,
endpointType: 'custom',
model: MOCK_ENDPOINTS[0].model,
spec: 'e2e-mock-provider-a',
isTemporary: false,
isRegenerate: false,
error: false,
},
);
expect(response.status).toBe(409);
expect(response.json).toEqual(
expect.objectContaining({
error: expect.stringContaining('selected parent response is still being saved'),
}),
);
const afterMessages = await fetchMessages(page, conversationId, token);
expect(afterMessages.map((message) => message.messageId).sort()).toEqual(
beforeMessages.map((message) => message.messageId).sort(),
);
expect(afterMessages.some((message) => messageText(message).includes(followPrompt))).toBe(
false,
);
expectNoFoldedMessages(afterMessages);
});
test('generation-start failures recover without folding the next follow-up', async ({ page }) => {
const label = uniqueLabel('start-error');
const basePrompt = replyPrompt(`${label}-base`);

View file

@ -4,6 +4,8 @@ import {
sanitizeFileForTransmit,
buildMessageFiles,
getThreadData,
isPreliminaryMessageId,
isUnpersistedPreliminaryParent,
} from './message';
/** Cast to string for type compatibility with ThreadMessage */
@ -130,6 +132,55 @@ describe('sanitizeMessageForTransmit', () => {
});
});
describe('isUnpersistedPreliminaryParent', () => {
it('returns false without querying for non-preliminary parent ids', async () => {
const getMessages = jest.fn();
await expect(
isUnpersistedPreliminaryParent({
userId: 'user-123',
conversationId: 'conversation-123',
parentMessageId: 'persisted-response',
getMessages,
}),
).resolves.toBe(false);
expect(isPreliminaryMessageId('persisted-response')).toBe(false);
expect(getMessages).not.toHaveBeenCalled();
});
it('returns true when the underscore-suffixed parent is not persisted', async () => {
const getMessages = jest.fn().mockResolvedValue([]);
await expect(
isUnpersistedPreliminaryParent({
userId: 'user-123',
conversationId: 'conversation-123',
parentMessageId: 'pending-response_',
getMessages,
}),
).resolves.toBe(true);
expect(getMessages).toHaveBeenCalledWith(
{ user: 'user-123', messageId: 'pending-response_', conversationId: 'conversation-123' },
'_id',
);
});
it('returns false when the underscore-suffixed parent is already persisted', async () => {
const getMessages = jest.fn().mockResolvedValue([{ _id: 'persisted-parent' }]);
await expect(
isUnpersistedPreliminaryParent({
userId: 'user-123',
conversationId: 'conversation-123',
parentMessageId: 'persisted-response_',
getMessages,
}),
).resolves.toBe(false);
});
});
describe('buildMessageFiles', () => {
const baseAttachment = {
file_id: 'file-1',

View file

@ -4,6 +4,11 @@ import type { TFile, TMessage } from 'librechat-data-provider';
/** Minimal shape for request file entries (from `req.body.files`) */
type RequestFile = { file_id?: string };
type GetMessagesByParentId = (
filter: { user: string; messageId: string; conversationId?: string },
select: '_id',
) => Promise<unknown[]>;
/** Fields to strip from files before client transmission */
const FILE_STRIP_FIELDS = ['text', '_id', '__v'] as const;
@ -92,6 +97,37 @@ export function sanitizeMessageForTransmit<T extends Partial<TMessage>>(
return sanitized;
}
export function isPreliminaryMessageId(messageId: unknown): messageId is string {
return typeof messageId === 'string' && messageId.endsWith('_');
}
export async function isUnpersistedPreliminaryParent({
userId,
conversationId,
parentMessageId,
getMessages,
}: {
userId: string;
conversationId?: string | null;
parentMessageId?: string | null;
getMessages: GetMessagesByParentId;
}): Promise<boolean> {
if (!isPreliminaryMessageId(parentMessageId)) {
return false;
}
const filter: { user: string; messageId: string; conversationId?: string } = {
user: userId,
messageId: parentMessageId,
};
if (conversationId && conversationId !== Constants.NEW_CONVO) {
filter.conversationId = conversationId;
}
const messages = await getMessages(filter, '_id');
return messages.length === 0;
}
/** Minimal message shape for thread traversal.
*
* Both `files` and `attachments` carry `file_id` references, but they