mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: re-cap already temporary parents to a shortened ephemeral window
The forced-retention cascade only matched parents that were not yet temporary, so switching from a longer temporary TTL to a shorter ephemeral one skipped conversations already marked isTemporary: true. Their older messages kept the longer deadline and could outlive the forced ephemeral window. Match parents and messages whose expiration is missing or later than the forced deadline, even when already temporary, via a shared gap filter. The cascade re-caps those documents and stays a no-op once they already expire within the forced window.
This commit is contained in:
parent
b8babe5d5b
commit
32bf64b77c
5 changed files with 144 additions and 14 deletions
|
|
@ -927,6 +927,43 @@ describe('Conversation Operations', () => {
|
|||
expect(messages[0].expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
});
|
||||
|
||||
it('re-caps an already temporary conversation and its messages to a shorter window', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const longerExpiry = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000);
|
||||
await Conversation.create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
title: 'Already temporary chat',
|
||||
isTemporary: true,
|
||||
expiredAt: longerExpiry,
|
||||
});
|
||||
await Message().create({
|
||||
messageId: uuidv4(),
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
text: 'older temporary message',
|
||||
isTemporary: true,
|
||||
expiredAt: longerExpiry,
|
||||
});
|
||||
|
||||
await saveConvo(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
|
||||
},
|
||||
{ conversationId, isArchived: true },
|
||||
);
|
||||
|
||||
const convo = await Conversation.findOne<IConversation>({ conversationId }).lean();
|
||||
expect(convo?.expiredAt?.getTime()).toBeLessThan(longerExpiry.getTime());
|
||||
|
||||
const messages = await Message().find({ conversationId }).lean();
|
||||
expect(messages).toHaveLength(1);
|
||||
expect(messages[0].isTemporary).toBe(true);
|
||||
expect(messages[0].expiredAt?.getTime()).toBeLessThan(longerExpiry.getTime());
|
||||
});
|
||||
|
||||
it('leaves messages untouched when the conversation is already ephemeral', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const ctx = {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import type { AppConfig, IChatProjectDocument, IConversation, IMessage } from '~
|
|||
import type { MessageMethods } from './message';
|
||||
import {
|
||||
buildRetentionVisibilityFilter,
|
||||
conversationNeedsForcedRetention,
|
||||
createFallbackRetentionDate,
|
||||
forceConversationMessagesTemporary,
|
||||
} from '~/utils/retention';
|
||||
|
|
@ -245,14 +246,18 @@ export function createConversationMethods(
|
|||
Object.prototype.hasOwnProperty.call(update, 'chatProjectId') ||
|
||||
Object.prototype.hasOwnProperty.call(unsetFields, 'chatProjectId');
|
||||
let previousChatProjectId: string | null = null;
|
||||
let parentWasNotTemporary = false;
|
||||
let parentRetention: { isTemporary?: boolean | null; expiredAt?: Date | null } | null = null;
|
||||
if (mayChangeProjectMembership || isForcedRetention) {
|
||||
const existing = await Conversation.findOne(
|
||||
{ conversationId, user: userId },
|
||||
'chatProjectId isTemporary',
|
||||
).lean<{ chatProjectId?: string | null; isTemporary?: boolean | null } | null>();
|
||||
'chatProjectId isTemporary expiredAt',
|
||||
).lean<{
|
||||
chatProjectId?: string | null;
|
||||
isTemporary?: boolean | null;
|
||||
expiredAt?: Date | null;
|
||||
} | null>();
|
||||
previousChatProjectId = existing?.chatProjectId ?? null;
|
||||
parentWasNotTemporary = existing != null && existing.isTemporary !== true;
|
||||
parentRetention = existing;
|
||||
}
|
||||
|
||||
if (newConversationId) {
|
||||
|
|
@ -339,7 +344,11 @@ export function createConversationMethods(
|
|||
}
|
||||
|
||||
const forcedExpiredAt = update.expiredAt;
|
||||
if (isForcedRetention && parentWasNotTemporary && forcedExpiredAt instanceof Date) {
|
||||
if (
|
||||
isForcedRetention &&
|
||||
forcedExpiredAt instanceof Date &&
|
||||
conversationNeedsForcedRetention(parentRetention, forcedExpiredAt)
|
||||
) {
|
||||
const Message = mongoose.models.Message as Model<IMessage>;
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, forcedExpiredAt);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1088,6 +1088,45 @@ describe('Message Operations', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('re-caps an already temporary parent and its messages to a shorter ephemeral window', async () => {
|
||||
const conversationId = uuidv4();
|
||||
const longerExpiry = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000);
|
||||
await Conversation().create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
title: 'Already temporary chat',
|
||||
isTemporary: true,
|
||||
expiredAt: longerExpiry,
|
||||
});
|
||||
await Message.create({
|
||||
messageId: uuidv4(),
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
text: 'older temporary message',
|
||||
isTemporary: true,
|
||||
expiredAt: longerExpiry,
|
||||
});
|
||||
|
||||
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?.expiredAt?.getTime()).toBeLessThan(longerExpiry.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(longerExpiry.getTime());
|
||||
}
|
||||
});
|
||||
|
||||
it('does not touch the parent conversation outside forced retention', async () => {
|
||||
const conversationId = uuidv4();
|
||||
await Conversation().create({
|
||||
|
|
|
|||
|
|
@ -1,7 +1,11 @@
|
|||
import { RetentionMode, isForcedTemporaryRetention } from 'librechat-data-provider';
|
||||
import type { DeleteResult, FilterQuery, Model } from 'mongoose';
|
||||
import type { AppConfig, IConversation, IMessage } from '~/types';
|
||||
import { createFallbackRetentionDate, forceConversationMessagesTemporary } from '~/utils/retention';
|
||||
import {
|
||||
createFallbackRetentionDate,
|
||||
forceConversationMessagesTemporary,
|
||||
forcedRetentionGapFilter,
|
||||
} from '~/utils/retention';
|
||||
import { createTempChatExpirationDate } from '~/utils/tempChatRetention';
|
||||
import { tenantSafeBulkWrite } from '~/utils/tenantBulkWrite';
|
||||
import logger from '~/config/winston';
|
||||
|
|
@ -178,7 +182,11 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
|
|||
) {
|
||||
const Conversation = mongoose.models.Conversation as Model<IConversation>;
|
||||
const convoResult = await Conversation.updateOne(
|
||||
{ conversationId, user: userId, isTemporary: { $ne: true } },
|
||||
{
|
||||
conversationId,
|
||||
user: userId,
|
||||
...forcedRetentionGapFilter<IConversation>(forcedExpiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
if (convoResult.modifiedCount > 0) {
|
||||
|
|
|
|||
|
|
@ -33,14 +33,51 @@ 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-temporary messages.
|
||||
* Matches retention documents that do not yet conform to a forced (ephemeral) deadline:
|
||||
* not temporary, missing an expiration, or expiring later than the forced window. The
|
||||
* last clause re-caps documents carried over from a longer policy (`all`, or a longer
|
||||
* `temporary` TTL) while leaving already-conforming temporary documents untouched.
|
||||
*/
|
||||
export const forcedRetentionGapFilter = <
|
||||
T extends RetentionFilterDocument = RetentionFilterDocument,
|
||||
>(
|
||||
forcedExpiredAt: Date,
|
||||
): FilterQuery<T> =>
|
||||
({
|
||||
$or: [
|
||||
{ isTemporary: { $ne: true } },
|
||||
{ expiredAt: null },
|
||||
{ expiredAt: { $gt: forcedExpiredAt } },
|
||||
],
|
||||
}) as FilterQuery<T>;
|
||||
|
||||
/**
|
||||
* In-memory counterpart of {@link forcedRetentionGapFilter} for a conversation's prior
|
||||
* state: true when the parent must be re-capped to the forced deadline.
|
||||
*/
|
||||
export const conversationNeedsForcedRetention = (
|
||||
parent: RetentionFilterDocument | null | undefined,
|
||||
forcedExpiredAt: Date,
|
||||
): boolean => {
|
||||
if (parent == null) {
|
||||
return false;
|
||||
}
|
||||
if (parent.isTemporary !== true || parent.expiredAt == null) {
|
||||
return true;
|
||||
}
|
||||
return parent.expiredAt.getTime() > forcedExpiredAt.getTime();
|
||||
};
|
||||
|
||||
/**
|
||||
* Applies forced-retention deadlines to a conversation's messages that do not yet
|
||||
* conform to the forced window.
|
||||
*
|
||||
* 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.
|
||||
* predates the mode keeps non-conforming messages — `expiredAt: null` permanent messages,
|
||||
* `isTemporary: false` messages carried over from `all` retention, or temporary messages
|
||||
* whose `expiredAt` is later than a newly shortened window — that would otherwise outlive
|
||||
* the converted conversation. The gap filter pulls all of them onto the ephemeral schedule
|
||||
* and stays a no-op once a conversation already conforms.
|
||||
*/
|
||||
export const forceConversationMessagesTemporary = async (
|
||||
Message: Model<IMessage>,
|
||||
|
|
@ -49,7 +86,7 @@ export const forceConversationMessagesTemporary = async (
|
|||
expiredAt: Date,
|
||||
): Promise<void> => {
|
||||
await Message.updateMany(
|
||||
{ conversationId, user: userId, isTemporary: { $ne: true } },
|
||||
{ conversationId, user: userId, ...forcedRetentionGapFilter<IMessage>(expiredAt) },
|
||||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue