From 151dc9e03e84d605eb50fa583ef9b9af0471586e Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:47:30 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8C=20fix:=20Keep=20Pinned=20Chats=20P?= =?UTF-8?q?inned=20Through=20a=20New=20Turn=20(#15230)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: keep pinned chats in the Pinned section after a new turn A chat's conversation state snapshots the sidebar-owned flags when the chat is opened, and the pin mutation never reaches it. Pinning a chat that is already open therefore leaves a stale `pinned: false` on that state, and the SSE handlers write it back over both list caches on the next turn. The chat drops out of Pinned and reappears under Today, since groupConversationsByDate only skips the rows the chats cache still marks pinned. Strip `pinned` and `isShared` from the conversation state before it reaches the caches, and carry the cached row's value forward whenever an updater omits one. The pin mutation still sends `pinned` explicitly, so unpinning a chat removes it from the section exactly as before. The server half of this bug was fixed in #14860, which put `pinned` in excludedKeys so saveMessageToDatabase's unset sweep stops clearing it, but it went in without a test. Cover it here: without that entry the sweep unpins the row on every message, which is what the released builds still do. * fix: preserve pinned flags during uncached upserts * fix: narrow nullable SAML profiles --------- Co-authored-by: Danny Avila --- api/app/clients/specs/BaseClient.test.js | 4 + .../__tests__/pinnedConversations.test.tsx | 123 +++++++++++++++++- client/src/hooks/SSE/useEventHandlers.ts | 11 +- client/src/utils/convos.ts | 83 ++++++++---- packages/api/src/auth/saml.ts | 4 +- 5 files changed, 189 insertions(+), 36 deletions(-) diff --git a/api/app/clients/specs/BaseClient.test.js b/api/app/clients/specs/BaseClient.test.js index 3bb7f57e63..3f4baa8528 100644 --- a/api/app/clients/specs/BaseClient.test.js +++ b/api/app/clients/specs/BaseClient.test.js @@ -1758,6 +1758,7 @@ describe('BaseClient', () => { anotherExistingField: 'anotherValue', temperature: 0.7, modelLabel: 'GPT-3.5', + pinned: true, subagentThread: { rootConversationId: 'root-conversation', parentConversationId: 'parent-conversation', @@ -1802,6 +1803,9 @@ describe('BaseClient', () => { // Only check that someExistingField is in unsetFields expect(saveOptions.unsetFields).toHaveProperty('someExistingField', 1); expect(saveOptions.unsetFields).not.toHaveProperty('subagentThread'); + // Sidebar metadata is never part of endpointOptions, so sweeping it would + // unpin a chat every time it received a message. + expect(saveOptions.unsetFields).not.toHaveProperty('pinned'); // Mock saveConvo to return the expected fields saveConvo.mockImplementation((req, fields) => { diff --git a/client/src/data-provider/__tests__/pinnedConversations.test.tsx b/client/src/data-provider/__tests__/pinnedConversations.test.tsx index a365c5780a..6805d4488d 100644 --- a/client/src/data-provider/__tests__/pinnedConversations.test.tsx +++ b/client/src/data-provider/__tests__/pinnedConversations.test.tsx @@ -8,18 +8,19 @@ import type { TConversation, } from 'librechat-data-provider'; import type { ReactNode } from 'react'; +import { + removeConvoFromAllQueries, + updateConvoInAllQueries, + upsertConvoInAllQueries, + collectPinnedConversations, + withoutListFlags, +} from '~/utils/convos'; import { useConversationTagMutation, useDeleteConversationMutation, useDeleteConversationTagMutation, usePinConversationMutation, } from '../mutations'; -import { - removeConvoFromAllQueries, - updateConvoInAllQueries, - upsertConvoInAllQueries, - collectPinnedConversations, -} from '~/utils/convos'; import { pinnedConversationsPageSize, usePinnedConversationsQuery } from '../queries'; jest.mock('librechat-data-provider', () => { @@ -315,6 +316,116 @@ describe('pinned list cache synchronization', () => { ).toEqual(['convo-pinned', 'convo-other']); }); + /** A chat pinned while it is open never hears about the pin in its own conversation + * state, so the state the SSE handlers write back still says `pinned: false`. Sending + * a message then dropped the chat out of Pinned and back into the date groups. */ + it('keeps a pin in the section when a new turn writes back stale chat state', () => { + const queryClient = createQueryClient(); + queryClient.setQueryData( + [QueryKeys.pinnedConversations, { tags: undefined }], + listResponse([pinnedConvo]), + ); + + const staleChatState = { ...pinnedConvo, title: 'Replied', pinned: false } as TConversation; + updateConvoInAllQueries( + queryClient, + pinnedConversationId, + () => withoutListFlags(staleChatState), + true, + ); + + const conversations = readPinnedCache(queryClient)?.conversations; + expect(conversations?.map((c) => c.conversationId)).toEqual([pinnedConversationId]); + expect(conversations?.[0].pinned).toBe(true); + expect(conversations?.[0].title).toBe('Replied'); + }); + + /** The date groups skip pinned chats, so losing the flag on the chats row put the same + * conversation back under Today beside the section it had just left. */ + it('keeps the flag on the chats row when a new turn writes back stale chat state', () => { + const queryClient = createQueryClient(); + queryClient.setQueryData([QueryKeys.allConversations, { tags: undefined }], { + pages: [{ conversations: [pinnedConvo], nextCursor: null }], + pageParams: [], + }); + + const staleChatState = { ...pinnedConvo, title: 'Replied', pinned: false } as TConversation; + updateConvoInAllQueries( + queryClient, + pinnedConversationId, + () => withoutListFlags(staleChatState), + true, + ); + + const data = queryClient.getQueryData<{ + pages: Array<{ conversations: TConversation[] }>; + }>([QueryKeys.allConversations, { tags: undefined }]); + expect(data?.pages[0].conversations[0].pinned).toBe(true); + }); + + /** Root-level SSE updates take the upsert path, and an older pin may be outside the + * loaded chats pages. The dedicated row has to supply the sidebar-owned flags when + * upsert inserts the conversation into that cache. */ + it('keeps list flags when an older pin upserts into the chats cache', () => { + const queryClient = createQueryClient(); + const cachedPin = { ...pinnedConvo, isShared: true } as TConversation; + queryClient.setQueryData( + [QueryKeys.pinnedConversations, { tags: undefined }], + listResponse([cachedPin]), + ); + queryClient.setQueryData([QueryKeys.allConversations, { tags: undefined }], { + pages: [ + { + conversations: [ + { + conversationId: 'other-recent', + title: 'Recent', + endpoint: 'openAI', + } as TConversation, + ], + nextCursor: 'cursor-2', + }, + ], + pageParams: [undefined], + }); + + const staleChatState = { + ...cachedPin, + title: 'Replied', + pinned: false, + isShared: false, + } as TConversation; + upsertConvoInAllQueries(queryClient, withoutListFlags(staleChatState)); + + expect(readPinnedCache(queryClient)?.conversations[0]).toEqual( + expect.objectContaining({ pinned: true, isShared: true, title: 'Replied' }), + ); + const chats = queryClient.getQueryData<{ + pages: Array<{ conversations: TConversation[] }>; + }>([QueryKeys.allConversations, { tags: undefined }]); + expect(chats?.pages[0].conversations[0]).toEqual( + expect.objectContaining({ + conversationId: pinnedConversationId, + pinned: true, + isShared: true, + title: 'Replied', + }), + ); + }); + + it('withoutListFlags drops only the sidebar-owned flags', () => { + const stripped = withoutListFlags({ + ...pinnedConvo, + pinned: false, + isShared: true, + } as TConversation); + + expect('pinned' in stripped).toBe(false); + expect('isShared' in stripped).toBe(false); + expect(stripped.conversationId).toBe(pinnedConversationId); + expect(stripped.title).toBe(pinnedConvo.title); + }); + it('leaves the pinned cache untouched for an unrelated conversation', () => { const queryClient = createQueryClient(); queryClient.setQueryData( diff --git a/client/src/hooks/SSE/useEventHandlers.ts b/client/src/hooks/SSE/useEventHandlers.ts index de4b0d1d13..239c6927dd 100644 --- a/client/src/hooks/SSE/useEventHandlers.ts +++ b/client/src/hooks/SSE/useEventHandlers.ts @@ -29,6 +29,7 @@ import { getConversationDraftId, scrollToEnd, hasRealTitle, + withoutListFlags, setDocumentTitle, requestChatFocus, getAllContentText, @@ -568,10 +569,11 @@ export default function useEventHandlers({ }); if (!isTemporary) { + const sidebarUpdate = withoutListFlags(update); if (requestMessage.parentMessageId === Constants.NO_PARENT) { - upsertConvoInAllQueries(queryClient, update); + upsertConvoInAllQueries(queryClient, sidebarUpdate); } else { - updateConvoInAllQueries(queryClient, update.conversationId!, (_c) => update, true); + updateConvoInAllQueries(queryClient, update.conversationId!, () => sidebarUpdate, true); } if (update.chatProjectId) { queryClient.invalidateQueries([QueryKeys.projects]); @@ -650,10 +652,11 @@ export default function useEventHandlers({ }); if (!isTemporary) { + const sidebarUpdate = withoutListFlags(update); if (parentMessageId === Constants.NO_PARENT) { - upsertConvoInAllQueries(queryClient, update); + upsertConvoInAllQueries(queryClient, sidebarUpdate); } else { - updateConvoInAllQueries(queryClient, update.conversationId!, (_c) => update, true); + updateConvoInAllQueries(queryClient, update.conversationId!, () => sidebarUpdate, true); } if (update.chatProjectId) { queryClient.invalidateQueries([QueryKeys.projects]); diff --git a/client/src/utils/convos.ts b/client/src/utils/convos.ts index 4708222ebf..7256c50c4e 100644 --- a/client/src/utils/convos.ts +++ b/client/src/utils/convos.ts @@ -474,6 +474,7 @@ export function upsertConvoInAllQueries( if (!nextConvo.conversationId) { return; } + const conversationId = nextConvo.conversationId; /* The history query excludes temporary conversations server-side, so seeding one into the list caches would surface it in the sidebar until the next @@ -483,17 +484,22 @@ export function upsertConvoInAllQueries( return; } + const cachedPin = findPinnedConversation(queryClient, conversationId); + const listConvo = cachedPin ? preserveListFlags(nextConvo, cachedPin) : nextConvo; + /* Root-level SSE updates and resumable settlement go through upsert, not update. Merge into any already-cached pin so that path cannot leave the - section at the old title or position. Do not insert: a new chat is not - pinned until the pin mutation refetches. */ + section at the old title or position. Carry its list flags into history + too when the conversation is older than the loaded pages. Do not insert + into the pinned cache: a new chat is not pinned until the pin mutation + refetches. */ updatePinnedConvosQuery( queryClient, - nextConvo.conversationId, + conversationId, (found) => ({ ...found, - ...nextConvo, - updatedAt: nextConvo.updatedAt ?? (moveToTop ? new Date().toISOString() : found.updatedAt), + ...listConvo, + updatedAt: listConvo.updatedAt ?? (moveToTop ? new Date().toISOString() : found.updatedAt), }), moveToTop, ); @@ -512,7 +518,7 @@ export function upsertConvoInAllQueries( let convoIdx = -1; for (let pi = 0; pi < oldData.pages.length; pi++) { const ci = oldData.pages[pi].conversations.findIndex( - (c) => c.conversationId === nextConvo.conversationId, + (c) => c.conversationId === conversationId, ); if (ci !== -1) { pageIdx = pi; @@ -523,7 +529,7 @@ export function upsertConvoInAllQueries( const now = new Date().toISOString(); if (pageIdx === -1) { - if (!conversationMatchesListQuery(query.queryKey, nextConvo)) { + if (!conversationMatchesListQuery(query.queryKey, listConvo)) { return oldData; } const firstPage = oldData.pages[0] ?? { conversations: [], nextCursor: null }; @@ -533,7 +539,7 @@ export function upsertConvoInAllQueries( { ...firstPage, conversations: [ - { ...nextConvo, updatedAt: nextConvo.updatedAt ?? now }, + { ...listConvo, updatedAt: listConvo.updatedAt ?? now }, ...firstPage.conversations, ], }, @@ -545,8 +551,8 @@ export function upsertConvoInAllQueries( const found = oldData.pages[pageIdx].conversations[convoIdx]; const updated = { ...found, - ...nextConvo, - updatedAt: nextConvo.updatedAt ?? (moveToTop ? now : found.updatedAt), + ...listConvo, + updatedAt: listConvo.updatedAt ?? (moveToTop ? now : found.updatedAt), }; if (!conversationMatchesProjectQuery(query.queryKey, updated)) { @@ -615,6 +621,43 @@ export function findPinnedConversation( return undefined; } +/** + * Flags the sidebar owns rather than the chat: `isShared` is derived per list request from + * the shared-links collection, and `pinned` is set by the pin mutation alone. Neither is + * carried by the single-conversation payloads callers swap in wholesale, so an omitted flag + * means "unchanged" rather than "cleared". + */ +const listFlags = ['isShared', 'pinned'] as const; + +function preserveListFlags(next: TConversation, found: TConversation): TConversation { + const carried = listFlags.filter((flag) => next[flag] === undefined && found[flag] !== undefined); + if (carried.length === 0) { + return next; + } + const merged = { ...next }; + for (const flag of carried) { + merged[flag] = found[flag]; + } + return merged; +} + +/** + * A chat's conversation state snapshots the sidebar flags when the chat is opened and never + * hears about a later change, so pinning an open chat leaves a stale `pinned: false` on it. + * Strip them before that state reaches the list caches, or the next message would write the + * stale value back over the sidebar and drop the chat out of Pinned. + */ +export function withoutListFlags(conversation: TConversation): TConversation { + if (listFlags.every((flag) => conversation[flag] === undefined)) { + return conversation; + } + const stripped = { ...conversation }; + for (const flag of listFlags) { + delete stripped[flag]; + } + return stripped; +} + /** * The pinned sidebar section is fed by its own request rather than by the paginated * chats list, so every edit that reaches the chats cache has to reach this one too or @@ -643,16 +686,13 @@ function updatePinnedConvosQuery( } const found = oldData.conversations[index]; const updated = updater(found); - if (!updated || updated.pinned !== true) { + const merged = updated && preserveListFlags(updated, found); + if (!merged || merged.pinned !== true) { return { ...oldData, conversations: oldData.conversations.filter((_, i) => i !== index), }; } - const merged = - updated.isShared === undefined && found.isShared !== undefined - ? { ...updated, isShared: found.isShared } - : updated; /* The server returns pins newest-first, so a pin that just received a message has to lead the section the same way it leads the chats list. The SSE payload can @@ -713,15 +753,10 @@ export function updateConvoInAllQueries( } const found = oldData.pages[pageIdx].conversations[convoIdx]; - /** `isShared` is derived per list request from the shared-links collection and is - * absent from single-conversation payloads, so callers that swap in a server - * response wholesale (rename, pin, SSE updates) would otherwise drop the sidebar - * badge until an unrelated list refetch. Carry it forward when the updater omits it. */ - const next = updater(found); - const merged = - next.isShared === undefined && found.isShared !== undefined - ? { ...next, isShared: found.isShared } - : next; + /** Callers that swap in a server response or the chat's own state wholesale (rename, + * pin, SSE updates) omit the sidebar-only flags, which would otherwise drop the + * shared badge and push a pinned chat back into the date groups. */ + const merged = preserveListFlags(updater(found), found); const updated = moveToTop ? { ...merged, updatedAt: new Date().toISOString() } : merged; if (!conversationMatchesProjectQuery(query.queryKey, updated)) { diff --git a/packages/api/src/auth/saml.ts b/packages/api/src/auth/saml.ts index f51a611ae8..7222783157 100644 --- a/packages/api/src/auth/saml.ts +++ b/packages/api/src/auth/saml.ts @@ -21,12 +21,12 @@ export function resolveSamlSubject( return { error: 'missing_name_id' }; } - if (profile.nameIDFormat === TRANSIENT_SAML_NAME_ID_FORMAT) { + if (profile?.nameIDFormat === TRANSIENT_SAML_NAME_ID_FORMAT) { return { error: 'transient_name_id' }; } const normalizedExpectedIssuer = expectedIssuer?.trim(); - const issuer = typeof profile.issuer === 'string' ? profile.issuer.trim() : ''; + const issuer = typeof profile?.issuer === 'string' ? profile.issuer.trim() : ''; if (normalizedExpectedIssuer && issuer !== normalizedExpectedIssuer) { return { error: 'issuer_mismatch' }; }