diff --git a/api/server/controllers/agents/request.js b/api/server/controllers/agents/request.js index 39fc215b89..81aaa46253 100644 --- a/api/server/controllers/agents/request.js +++ b/api/server/controllers/agents/request.js @@ -274,6 +274,11 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit * `completeJob` also aborts on *successful* completion and would otherwise * cancel a title that is merely slower than a short response. */ const titleAbortController = new AbortController(); + /** Separate from `titleAbortController`: a user Stop cancels the in-flight + * title model call but keeps a title that already finished generating. + * Only a superseded/failed stream aborts this to discard such a title so it + * cannot clobber the conversation now owned by the newer run. */ + const titleDiscardController = new AbortController(); const abortTitleOnJobAbort = () => titleAbortController.abort(); if (job.abortController.signal.aborted) { titleAbortController.abort(); @@ -363,6 +368,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit immediate: true, convoReady, signal: titleAbortController.signal, + discardSignal: titleDiscardController.signal, onTitleGenerated: emitTitleEvent, }).catch((err) => { logger.error('[ResumableAgentController] Error in immediate title generation', err); @@ -439,6 +445,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit // unblock its persistence wait without letting it save (the newer job // owns the conversation now). titleAbortController.abort(); + titleDiscardController.abort(); job.abortController.signal.removeEventListener('abort', abortTitleOnJobAbort); acceptsTitleEvents = false; resolveConvoReady(); @@ -554,6 +561,7 @@ const ResumableAgentController = async (req, res, next, initializeClient, addTit // `_waitForRun` would otherwise never resolve, deferring client disposal // until the 45s title timeout, and no title should persist for a failed turn. titleAbortController.abort(); + titleDiscardController.abort(); job.abortController.signal.removeEventListener('abort', abortTitleOnJobAbort); acceptsTitleEvents = false; resolveConvoReady(); diff --git a/api/server/services/Endpoints/agents/title.js b/api/server/services/Endpoints/agents/title.js index 350b00142e..0aa955055e 100644 --- a/api/server/services/Endpoints/agents/title.js +++ b/api/server/services/Endpoints/agents/title.js @@ -21,7 +21,13 @@ const { saveConvo } = require('~/models'); * persisted; awaited before saving the title in `immediate` mode. * @param {AbortSignal} [params.signal] - When aborted (e.g. the user stops an * immediate-mode generation), cancels the in-flight title model call so a - * cancelled turn neither consumes the title model nor surfaces a title. + * turn stopped before the title finished does not consume the title model. A + * title that already finished generating is still persisted and surfaced. + * @param {AbortSignal} [params.discardSignal] - When aborted, discards an + * already-generated title instead of persisting it. Used only when this stream + * is superseded by a newer run (or the turn failed), so a stale title does not + * clobber the conversation now owned by the newer run. A plain user Stop does + * NOT abort this — its generated title is kept. * @param {(params: { conversationId: string, title: string }) => Promise|void} [params.onTitleGenerated] * Called after the title is cached and before persistence waits for the * conversation row. Used by live streams to push the title immediately. @@ -36,6 +42,7 @@ const addTitle = async ( immediate = false, convoReady, signal, + discardSignal, onTitleGenerated, }, ) => { @@ -130,12 +137,14 @@ const addTitle = async ( await convoReady; } - if (signal?.aborted) { - // The turn was stopped, or this stream was replaced, after the title had - // already been generated — discard it instead of persisting a title for a - // cancelled/discarded response. Only clear the cache if it still holds THIS - // task's title: a replacement stream shares the `userId-conversationId` key - // and may have already cached its own (valid) title that we must not remove. + if (discardSignal?.aborted) { + // This stream was superseded by a newer run (or the turn failed) after the + // title had already been generated — discard it so a stale title does not + // clobber the conversation now owned by the newer run. A plain user Stop is + // not a discard: its generated title falls through and is persisted below. + // Only clear the cache if it still holds THIS task's title: a replacement + // stream shares the `userId-conversationId` key and may have already cached + // its own (valid) title that we must not remove. const cached = await titleCache.get(key); if (cached === title) { await titleCache.delete(key); diff --git a/api/server/services/Endpoints/agents/title.test.js b/api/server/services/Endpoints/agents/title.test.js index cf6fe250d8..4153761990 100644 --- a/api/server/services/Endpoints/agents/title.test.js +++ b/api/server/services/Endpoints/agents/title.test.js @@ -212,7 +212,25 @@ describe('agents addTitle', () => { expect(mockSaveConvo).not.toHaveBeenCalled(); }); - it('propagates an aborted request signal and discards the title without persisting', async () => { + it('propagates the abort signal to the title model call', async () => { + const client = makeClient(); + const ac = new AbortController(); + ac.abort(); + + await addTitle(makeReq(), { + text: 'hi', + client, + conversationId: 'cid', + immediate: true, + convoReady: Promise.resolve(), + signal: ac.signal, + }); + + const { abortController } = client.titleConvo.mock.calls[0][0]; + expect(abortController.signal.aborted).toBe(true); + }); + + it('discards the title without persisting when the stream is superseded', async () => { const client = makeClient(); const ac = new AbortController(); const onTitleGenerated = jest.fn(); @@ -225,17 +243,16 @@ describe('agents addTitle', () => { immediate: true, convoReady: Promise.resolve(), signal: ac.signal, + discardSignal: ac.signal, onTitleGenerated, }); - const { abortController } = client.titleConvo.mock.calls[0][0]; - expect(abortController.signal.aborted).toBe(true); expect(onTitleGenerated).not.toHaveBeenCalled(); expect(mockSaveConvo).not.toHaveBeenCalled(); expect(mockCache.delete).toHaveBeenCalledWith('user-1-cid'); }); - it("does not delete a replacement stream's cached title when aborted", async () => { + it("does not delete a replacement stream's cached title when superseded", async () => { const client = makeClient('Stale Title'); const ac = new AbortController(); ac.abort(); @@ -250,9 +267,47 @@ describe('agents addTitle', () => { immediate: true, convoReady: Promise.resolve(), signal: ac.signal, + discardSignal: ac.signal, }); expect(mockCache.delete).not.toHaveBeenCalled(); expect(mockSaveConvo).not.toHaveBeenCalled(); }); + + it('persists a title generated before a user Stop (signal aborted, not superseded)', async () => { + const client = makeClient('Kept Title'); + // `signal` represents a user Stop; no `discardSignal` since the stream is not + // superseded. The title finishes generating and is emitted before the Stop. + const ac = new AbortController(); + const onTitleGenerated = jest.fn(); + let resolveConvo; + const convoReady = new Promise((resolve) => { + resolveConvo = resolve; + }); + + const pending = addTitle(makeReq(), { + text: 'hi', + client, + conversationId: 'cid', + immediate: true, + convoReady, + signal: ac.signal, + onTitleGenerated, + }); + + await flush(); + expect(onTitleGenerated).toHaveBeenCalledWith({ conversationId: 'cid', title: 'Kept Title' }); + + // User stops mid-response, then the conversation row is persisted. + ac.abort(); + resolveConvo(); + await pending; + + expect(mockSaveConvo).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ conversationId: 'cid', title: 'Kept Title' }), + expect.objectContaining({ noUpsert: true }), + ); + expect(mockCache.delete).not.toHaveBeenCalled(); + }); }); diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index ef69c4a397..93c6ae1294 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -19,9 +19,9 @@ import type { EventSubmission, TStartupConfig, } from 'librechat-data-provider'; -import type { TResData, TFinalResData, ConvoGenerator } from '~/common'; import type { InfiniteData } from '@tanstack/react-query'; import type { SetterOrUpdater } from 'recoil'; +import type { TResData, TFinalResData, ConvoGenerator } from '~/common'; import type { ConversationCursorData } from '~/utils'; import { logger, @@ -38,6 +38,7 @@ import { queueTitleGeneration, markTitleGenerationProcessed, } from '~/data-provider'; +import { shouldResetSubagentAtomsOnConversationChange } from './cleanup'; import useAttachmentHandler from '~/hooks/SSE/useAttachmentHandler'; import useContentHandler from '~/hooks/SSE/useContentHandler'; import useStepHandler from '~/hooks/SSE/useStepHandler'; @@ -45,7 +46,6 @@ import { useApplyAgentTemplate } from '~/hooks/Agents'; import { useAuthContext } from '~/hooks/AuthContext'; import { MESSAGE_UPDATE_INTERVAL } from '~/common'; import { useLiveAnnouncer } from '~/Providers'; -import { shouldResetSubagentAtomsOnConversationChange } from './cleanup'; import store from '~/store'; type TSyncData = { @@ -663,25 +663,9 @@ export default function useEventHandlers({ /** A title applied locally (e.g. an immediate-mode title fetched while the * response was still streaming) must survive the final event, whose * `conversation` was built before the title was saved and so carries no - * title yet — otherwise the chat reverts to "New Chat" until reload. - * Skip preservation for a stopped (unfinished) turn: the server cancels - * and discards that title, so the local one would diverge from server state. */ - const titlePreservable = responseMessage?.unfinished !== true; - const finalConversationId = conversation.conversationId; - const shouldRollbackStreamedTitle = - !titlePreservable && finalConversationId && !hasRealTitle(serverConversation.title); - - if (shouldRollbackStreamedTitle && finalConversationId) { - updateConvoInAllQueries(queryClient, finalConversationId, (convo) => ({ - ...convo, - title: null, - })); - if (location.pathname.includes(finalConversationId)) { - const startupConfig = queryClient.getQueryData(startupConfigKey(true)); - document.title = startupConfig?.appTitle ?? 'LibreChat'; - } - } - + * title yet — otherwise the chat reverts to "New Chat" until reload. This + * holds for a stopped turn too: the server persists a title that finished + * generating before the Stop, so the local one stays in sync. */ if (setConversation && isAddedRequest !== true) { setConversation((prevState) => { const update = { @@ -692,7 +676,7 @@ export default function useEventHandlers({ update.model = prevState.model; } const prevTitle = prevState?.title; - if (titlePreservable && !hasRealTitle(conversation.title) && hasRealTitle(prevTitle)) { + if (!hasRealTitle(conversation.title) && hasRealTitle(prevTitle)) { update.title = prevTitle; } if (conversation.conversationId) { @@ -704,11 +688,7 @@ export default function useEventHandlers({ ...serverConversation, } as TConversation; const cachedTitle = cachedConvo?.title; - if ( - titlePreservable && - !hasRealTitle(serverConversation.title) && - hasRealTitle(cachedTitle) - ) { + if (!hasRealTitle(serverConversation.title) && hasRealTitle(cachedTitle)) { merged.title = cachedTitle; } return merged;