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.
This commit is contained in:
Marco Beretta 2026-07-05 00:12:26 +02:00
parent b871ae4b5e
commit f415ca0e38
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
2 changed files with 70 additions and 7 deletions

View file

@ -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<IConversation>;
const SharedLink = () => mongoose.models.SharedLink as mongoose.Model<ISharedLink>;
const File = () => mongoose.models.File as mongoose.Model<IMongoFile>;
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<IMongoFile>;
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

View file

@ -237,15 +237,22 @@ export const cascadeForcedConversationRetention = async (
'isTemporary expiredAt',
).lean<RetentionFilterDocument | null>();
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<IConversation>(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);
}
};
/**