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' }; }