mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
fix: convert active retained parents when forcing ephemeral retention
The forced-retention cascade keyed conversion off expiredAt: null, so a switch from all to ephemeral retention skipped conversations that were already isTemporary: false with a future expiredAt. Message-only paths (branch, artifact, abort) then produced a temporary message under a parent that stayed visible in history via the active non-temporary branch of the visibility filter. Gate the cascade on isTemporary !== true instead so saveMessage, saveConvo, and the message backfill all convert non-temporary parents and their messages regardless of an existing active expiration, while remaining a no-op once a conversation is already temporary.
This commit is contained in:
parent
b00676872b
commit
df36cd8d44
5 changed files with 92 additions and 12 deletions
|
|
@ -889,6 +889,44 @@ describe('Conversation Operations', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('converts an active retained (all-mode) conversation when switching to ephemeral', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const retainedUntil = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000);
|
||||
await Conversation.create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
title: 'Retained all-mode chat',
|
||||
isTemporary: false,
|
||||
expiredAt: retainedUntil,
|
||||
});
|
||||
await Message().create({
|
||||
messageId: uuidv4(),
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
text: 'retained',
|
||||
isTemporary: false,
|
||||
expiredAt: retainedUntil,
|
||||
});
|
||||
|
||||
await saveConvo(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
|
||||
},
|
||||
{ conversationId, isArchived: true },
|
||||
);
|
||||
|
||||
const convo = await Conversation.findOne<IConversation>({ conversationId }).lean();
|
||||
expect(convo?.isTemporary).toBe(true);
|
||||
expect(convo?.expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
|
||||
const messages = await Message().find({ conversationId }).lean();
|
||||
expect(messages).toHaveLength(1);
|
||||
expect(messages[0].isTemporary).toBe(true);
|
||||
expect(messages[0].expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
});
|
||||
|
||||
it('leaves messages untouched when the conversation is already ephemeral', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const ctx = {
|
||||
|
|
|
|||
|
|
@ -245,14 +245,14 @@ export function createConversationMethods(
|
|||
Object.prototype.hasOwnProperty.call(update, 'chatProjectId') ||
|
||||
Object.prototype.hasOwnProperty.call(unsetFields, 'chatProjectId');
|
||||
let previousChatProjectId: string | null = null;
|
||||
let wasConversationPermanent = false;
|
||||
let parentWasNotTemporary = false;
|
||||
if (mayChangeProjectMembership || isForcedRetention) {
|
||||
const existing = await Conversation.findOne(
|
||||
{ conversationId, user: userId },
|
||||
'chatProjectId expiredAt',
|
||||
).lean<{ chatProjectId?: string | null; expiredAt?: Date | null } | null>();
|
||||
'chatProjectId isTemporary',
|
||||
).lean<{ chatProjectId?: string | null; isTemporary?: boolean | null } | null>();
|
||||
previousChatProjectId = existing?.chatProjectId ?? null;
|
||||
wasConversationPermanent = existing != null && existing.expiredAt == null;
|
||||
parentWasNotTemporary = existing != null && existing.isTemporary !== true;
|
||||
}
|
||||
|
||||
if (newConversationId) {
|
||||
|
|
@ -339,7 +339,7 @@ export function createConversationMethods(
|
|||
}
|
||||
|
||||
const forcedExpiredAt = update.expiredAt;
|
||||
if (isForcedRetention && wasConversationPermanent && forcedExpiredAt instanceof Date) {
|
||||
if (isForcedRetention && parentWasNotTemporary && forcedExpiredAt instanceof Date) {
|
||||
const Message = mongoose.models.Message as Model<IMessage>;
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, forcedExpiredAt);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -793,6 +793,46 @@ describe('Message Operations', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('converts an active retained (all-mode) parent when switching to ephemeral', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const retainedUntil = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000);
|
||||
await Conversation().create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
title: 'Retained all-mode chat',
|
||||
isTemporary: false,
|
||||
expiredAt: retainedUntil,
|
||||
});
|
||||
await Message.create({
|
||||
messageId: uuidv4(),
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
text: 'retained',
|
||||
isTemporary: false,
|
||||
expiredAt: retainedUntil,
|
||||
});
|
||||
|
||||
await saveMessage(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
|
||||
},
|
||||
{ messageId: uuidv4(), conversationId, text: 'branch', user: 'user123' },
|
||||
);
|
||||
|
||||
const convo = await Conversation().findOne({ conversationId }).lean();
|
||||
expect(convo?.isTemporary).toBe(true);
|
||||
expect(convo?.expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
|
||||
const messages = await getMessages({ conversationId, user: 'user123' });
|
||||
expect(messages).toHaveLength(2);
|
||||
for (const message of messages) {
|
||||
expect(message.isTemporary).toBe(true);
|
||||
expect(message.expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
}
|
||||
});
|
||||
|
||||
it('does not touch the parent conversation outside forced retention', async () => {
|
||||
const conversationId = uuidv4();
|
||||
await Conversation().create({
|
||||
|
|
|
|||
|
|
@ -169,7 +169,7 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
|
|||
) {
|
||||
const Conversation = mongoose.models.Conversation as Model<IConversation>;
|
||||
const convoResult = await Conversation.updateOne(
|
||||
{ conversationId, user: userId, expiredAt: null },
|
||||
{ conversationId, user: userId, isTemporary: { $ne: true } },
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
if (convoResult.modifiedCount > 0) {
|
||||
|
|
|
|||
|
|
@ -33,12 +33,14 @@ export const createFallbackRetentionDate = (now: number = Date.now()): Date =>
|
|||
new Date(now + DEFAULT_RETENTION_HOURS * 60 * 60 * 1000);
|
||||
|
||||
/**
|
||||
* Applies forced-retention deadlines to a conversation's not-yet-expiring messages.
|
||||
* Applies forced-retention deadlines to a conversation's not-yet-temporary messages.
|
||||
*
|
||||
* Forced (ephemeral) retention must cover existing messages too: a conversation that
|
||||
* predates the mode keeps `expiredAt: null` messages that the TTL index never removes,
|
||||
* so they would outlive the conversation. Only messages still lacking an expiration are
|
||||
* touched, making this a no-op once a conversation has been converted.
|
||||
* Forced (ephemeral) retention must cover existing messages too. A conversation that
|
||||
* predates the mode keeps non-temporary messages — `expiredAt: null` permanent messages,
|
||||
* or `isTemporary: false` messages with a future `expiredAt` carried over from `all`
|
||||
* retention — that outlive the converted conversation. Targeting `isTemporary !== true`
|
||||
* pulls both onto the ephemeral schedule and stays a no-op once a conversation has been
|
||||
* converted.
|
||||
*/
|
||||
export const forceConversationMessagesTemporary = async (
|
||||
Message: Model<IMessage>,
|
||||
|
|
@ -47,7 +49,7 @@ export const forceConversationMessagesTemporary = async (
|
|||
expiredAt: Date,
|
||||
): Promise<void> => {
|
||||
await Message.updateMany(
|
||||
{ conversationId, user: userId, expiredAt: null },
|
||||
{ conversationId, user: userId, isTemporary: { $ne: true } },
|
||||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue