From f415ca0e38f04003d489b1c5eebb2368196f28a1 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 5 Jul 2026 00:12:26 +0200 Subject: [PATCH] fix: keep the forced-retention cascade re-runnable after a child failure cascadeForcedConversationRetention converted the parent conversation first and only backfilled its messages, shares, and files when that update modified a row. If a child backfill threw after the parent was converted, a later forced-retention write saw the parent already conforming (modifiedCount 0) and skipped the children, leaving not-yet-updated messages, shares, or files permanent. Gate on an in-memory conversationNeedsForcedRetention check and backfill the children before marking the parent conforming, so a failed child leaves the conversation non-conforming and the next write re-runs the whole cascade. The early return preserves the no-op-when-already-conforming behavior. --- .../data-schemas/src/methods/message.spec.ts | 58 ++++++++++++++++++- packages/data-schemas/src/utils/retention.ts | 19 ++++-- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/packages/data-schemas/src/methods/message.spec.ts b/packages/data-schemas/src/methods/message.spec.ts index 79e368658a..e50631fa68 100644 --- a/packages/data-schemas/src/methods/message.spec.ts +++ b/packages/data-schemas/src/methods/message.spec.ts @@ -3,8 +3,8 @@ import { v4 as uuidv4 } from 'uuid'; import { RetentionMode } from 'librechat-data-provider'; import { MongoMemoryServer } from 'mongodb-memory-server'; import type { IChatProject, IConversation, IMessage, IMongoFile, ISharedLink } from '..'; +import { cascadeForcedConversationRetention, sweepForcedRetention } from '../utils/retention'; import { tenantStorage, runAsSystem } from '~/config/tenantContext'; -import { sweepForcedRetention } from '../utils/retention'; import { createMessageMethods } from './message'; import { createModels } from '../models'; import logger from '~/config/winston'; @@ -1661,6 +1661,62 @@ describe('Message Operations', () => { }); }); + describe('cascadeForcedConversationRetention', () => { + const Conversation = () => mongoose.models.Conversation as mongoose.Model; + const SharedLink = () => mongoose.models.SharedLink as mongoose.Model; + const File = () => mongoose.models.File as mongoose.Model; + + beforeEach(async () => { + await Conversation().deleteMany({}); + await SharedLink().deleteMany({}); + }); + + it('leaves the conversation non-conforming when a child backfill fails so a re-run retries it', async () => { + const forcedExpiredAt = new Date(Date.now() + 24 * 60 * 60 * 1000); + const conversationId = uuidv4(); + await Conversation().create({ + conversationId, + user: 'user123', + endpoint: 'openAI', + isTemporary: false, + }); + + const throwingFile = { + updateMany: jest.fn().mockRejectedValue(new Error('file backfill failed')), + } as unknown as mongoose.Model; + + await expect( + cascadeForcedConversationRetention( + Conversation(), + Message, + SharedLink(), + throwingFile, + 'user123', + conversationId, + forcedExpiredAt, + ), + ).rejects.toThrow('file backfill failed'); + + const stillPermanent = await Conversation().findOne({ conversationId }).lean(); + expect(stillPermanent?.isTemporary ?? null).not.toBe(true); + expect(stillPermanent?.expiredAt ?? null).toBeNull(); + + await cascadeForcedConversationRetention( + Conversation(), + Message, + SharedLink(), + File(), + 'user123', + conversationId, + forcedExpiredAt, + ); + + const converted = await Conversation().findOne({ conversationId }).lean(); + expect(converted?.isTemporary).toBe(true); + expect(converted?.expiredAt?.getTime()).toBe(forcedExpiredAt.getTime()); + }); + }); + describe('Message cursor pagination', () => { /** * Helper to create messages with specific timestamps diff --git a/packages/data-schemas/src/utils/retention.ts b/packages/data-schemas/src/utils/retention.ts index 76d2562a5c..1c3b90b539 100644 --- a/packages/data-schemas/src/utils/retention.ts +++ b/packages/data-schemas/src/utils/retention.ts @@ -237,15 +237,22 @@ export const cascadeForcedConversationRetention = async ( 'isTemporary expiredAt', ).lean(); const expiredAt = capForcedRetentionExpiry(parent?.expiredAt, forcedExpiredAt); - const convoResult = await Conversation.updateOne( + if (!conversationNeedsForcedRetention(parent, expiredAt)) { + return; + } + /** + * Backfill the dependent messages, shares, and files before marking the conversation + * conforming. If a child update throws, the conversation stays non-conforming (the in-memory + * gap check above still matches it), so a later forced-retention write re-runs the whole + * cascade instead of skipping it because the parent already satisfies the gap filter. + */ + await forceConversationMessagesTemporary(Message, userId, conversationId, expiredAt); + await capConversationSharedLinks(SharedLink, userId, conversationId, expiredAt); + await capConversationFiles(File, conversationId, expiredAt); + await Conversation.updateOne( { conversationId, user: userId, ...forcedRetentionGapFilter(expiredAt) }, { $set: { isTemporary: true, expiredAt } }, ); - if (convoResult.modifiedCount > 0) { - await forceConversationMessagesTemporary(Message, userId, conversationId, expiredAt); - await capConversationSharedLinks(SharedLink, userId, conversationId, expiredAt); - await capConversationFiles(File, conversationId, expiredAt); - } }; /**