mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-10 08:13:46 +00:00
fix: never extend a sooner-expiring conversation under forced retention
Centralize the earlier-deadline rule in a capForcedRetentionExpiry helper and apply it everywhere a forced-retention deadline is written: capForcedRetentionToParent, the conversation cascade (which now re-reads the parent and caps to it), and saveConvo's ephemeral branch (which previously always refreshed to a fresh window, extending a conversation that already expired sooner). No forced-retention path lengthens an existing earlier expiry now. Also scope the retainAgentFiles exception to RetentionMode.ALL, since isAllDataRetention matches ephemeral too and would otherwise let agent files persist past an ephemeral conversation's deadline.
This commit is contained in:
parent
ff2842d0dd
commit
b94c563527
7 changed files with 277 additions and 57 deletions
|
|
@ -21,7 +21,11 @@ jest.mock('librechat-data-provider', () => {
|
|||
return {
|
||||
...actual,
|
||||
Providers: actual.Providers,
|
||||
RetentionMode: actual.RetentionMode ?? { ALL: 'all', TEMPORARY: 'temporary' },
|
||||
RetentionMode: actual.RetentionMode ?? {
|
||||
ALL: 'all',
|
||||
TEMPORARY: 'temporary',
|
||||
EPHEMERAL: 'ephemeral',
|
||||
},
|
||||
documentParserMimeTypes: actual.documentParserMimeTypes ?? [
|
||||
/^application\/pdf$/,
|
||||
/^application\/vnd\.openxmlformats-officedocument\./,
|
||||
|
|
@ -35,7 +39,14 @@ jest.mock('librechat-data-provider', () => {
|
|||
|
||||
jest.mock('@librechat/api', () => {
|
||||
const actualDataProvider = jest.requireActual('librechat-data-provider');
|
||||
const RetentionMode = actualDataProvider.RetentionMode ?? { ALL: 'all', TEMPORARY: 'temporary' };
|
||||
const RetentionMode = actualDataProvider.RetentionMode ?? {
|
||||
ALL: 'all',
|
||||
TEMPORARY: 'temporary',
|
||||
EPHEMERAL: 'ephemeral',
|
||||
};
|
||||
const isAllDataRetention =
|
||||
actualDataProvider.isAllDataRetention ??
|
||||
((mode) => mode === RetentionMode.ALL || mode === RetentionMode.EPHEMERAL);
|
||||
const getRetentionExpiry = jest.fn(() => ({}));
|
||||
return {
|
||||
sanitizeFilename: jest.fn((n) => n),
|
||||
|
|
@ -45,11 +56,12 @@ jest.mock('@librechat/api', () => {
|
|||
getRetentionExpiry,
|
||||
getAgentFileRetentionExpiry: jest.fn(({ req, messageAttachment, toolResource }) => {
|
||||
const interfaceConfig = req?.config?.interfaceConfig;
|
||||
const retentionMode = interfaceConfig?.retentionMode;
|
||||
if (
|
||||
!messageAttachment &&
|
||||
!!toolResource &&
|
||||
(interfaceConfig?.retentionMode !== RetentionMode.ALL ||
|
||||
interfaceConfig?.retainAgentFiles === true)
|
||||
(!isAllDataRetention(retentionMode) ||
|
||||
(retentionMode === RetentionMode.ALL && interfaceConfig?.retainAgentFiles === true))
|
||||
) {
|
||||
return {};
|
||||
}
|
||||
|
|
@ -541,6 +553,31 @@ describe('processAgentFileUpload', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('applies ephemeral retention metadata to persistent agent context files when retainAgentFiles is enabled', async () => {
|
||||
const expiredAt = new Date('2030-01-01T00:00:00.000Z');
|
||||
getRetentionExpiry.mockResolvedValueOnce({ expiredAt });
|
||||
const req = makeReq({
|
||||
mimetype: PDF_MIME,
|
||||
ocrConfig: null,
|
||||
interfaceConfig: {
|
||||
retentionMode: RetentionMode.EPHEMERAL,
|
||||
retainAgentFiles: true,
|
||||
},
|
||||
});
|
||||
|
||||
await processAgentFileUpload({ req, res: mockRes, metadata: makeMetadata() });
|
||||
|
||||
expect(getRetentionExpiry).toHaveBeenCalledTimes(1);
|
||||
expect(getRetentionExpiry.mock.calls[0][0]).toBe(req);
|
||||
expect(db.createFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
expiredAt,
|
||||
context: FileContext.agents,
|
||||
}),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test('applies retention metadata to context files uploaded as message attachments', async () => {
|
||||
const expiredAt = new Date('2030-01-01T00:00:00.000Z');
|
||||
getRetentionExpiry.mockResolvedValueOnce({ expiredAt });
|
||||
|
|
|
|||
|
|
@ -304,6 +304,28 @@ describe('retention helpers', () => {
|
|||
expect(dependencies.createExpirationDate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('applies ephemeral retention to persistent agent files even when retainAgentFiles is enabled', async () => {
|
||||
const result = await getAgentFileRetentionExpiry(
|
||||
{
|
||||
req: request({
|
||||
config: {
|
||||
interfaceConfig: {
|
||||
retentionMode: RetentionMode.EPHEMERAL,
|
||||
retainAgentFiles: true,
|
||||
},
|
||||
},
|
||||
}),
|
||||
messageAttachment: false,
|
||||
toolResource: 'context',
|
||||
},
|
||||
dependencies,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ expiredAt: expirationDate });
|
||||
expect(dependencies.getConvo).not.toHaveBeenCalled();
|
||||
expect(dependencies.createExpirationDate).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('still applies all-data retention to agent message attachments when retainAgentFiles is enabled', async () => {
|
||||
const result = await getAgentFileRetentionExpiry(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { isAllDataRetention } from 'librechat-data-provider';
|
||||
import { createFallbackRetentionDate } from '@librechat/data-schemas';
|
||||
import { RetentionMode, isAllDataRetention } from 'librechat-data-provider';
|
||||
import type { AppConfig } from '@librechat/data-schemas';
|
||||
|
||||
type InterfaceConfig = AppConfig['interfaceConfig'];
|
||||
|
|
@ -177,10 +177,11 @@ const shouldRetainPersistentAgentFile = ({
|
|||
toolResource,
|
||||
}: AgentFileRetentionRequest): boolean => {
|
||||
const interfaceConfig = req?.config?.interfaceConfig;
|
||||
const retentionMode = interfaceConfig?.retentionMode;
|
||||
return (
|
||||
isPersistentAgentResourceUpload({ messageAttachment, toolResource }) &&
|
||||
(!isAllDataRetention(interfaceConfig?.retentionMode) ||
|
||||
interfaceConfig?.retainAgentFiles === true)
|
||||
(!isAllDataRetention(retentionMode) ||
|
||||
(retentionMode === RetentionMode.ALL && interfaceConfig?.retainAgentFiles === true))
|
||||
);
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -927,6 +927,54 @@ describe('Conversation Operations', () => {
|
|||
expect(messages[0].expiredAt?.getTime()).toBeLessThan(retainedUntil.getTime());
|
||||
});
|
||||
|
||||
it('preserves an earlier retained expiration when switching to ephemeral', async () => {
|
||||
const SharedLink = mongoose.models.SharedLink as mongoose.Model<ISharedLink>;
|
||||
await SharedLink.deleteMany({});
|
||||
const conversationId = uuidv4();
|
||||
const soonerExpiry = new Date(Date.now() + 60 * 60 * 1000);
|
||||
await Conversation.create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
title: 'Sooner retained all-mode chat',
|
||||
isTemporary: false,
|
||||
expiredAt: soonerExpiry,
|
||||
});
|
||||
await Message().create({
|
||||
messageId: uuidv4(),
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
text: 'retained',
|
||||
isTemporary: false,
|
||||
expiredAt: soonerExpiry,
|
||||
});
|
||||
await SharedLink.create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
shareId: uuidv4(),
|
||||
expiredAt: soonerExpiry,
|
||||
});
|
||||
|
||||
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()).toBe(soonerExpiry.getTime());
|
||||
|
||||
const message = await Message().findOne({ conversationId }).lean();
|
||||
expect(message?.isTemporary).toBe(true);
|
||||
expect(message?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
|
||||
|
||||
const share = await SharedLink.findOne({ conversationId }).lean();
|
||||
expect(share?.expiredAt?.getTime()).toBe(soonerExpiry.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);
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import type { MessageMethods } from './message';
|
|||
import {
|
||||
buildRetentionVisibilityFilter,
|
||||
capConversationSharedLinks,
|
||||
capForcedRetentionExpiry,
|
||||
conversationNeedsForcedRetention,
|
||||
createFallbackRetentionDate,
|
||||
forceConversationMessagesTemporary,
|
||||
|
|
@ -274,11 +275,17 @@ export function createConversationMethods(
|
|||
if (interfaceConfig?.retentionMode === RetentionMode.EPHEMERAL) {
|
||||
update.isTemporary = true;
|
||||
try {
|
||||
update.expiredAt = createTempChatExpirationDate(interfaceConfig);
|
||||
update.expiredAt = capForcedRetentionExpiry(
|
||||
parentRetention?.expiredAt,
|
||||
createTempChatExpirationDate(interfaceConfig),
|
||||
);
|
||||
} catch (err) {
|
||||
logger.error('Error creating temporary chat expiration date:', err);
|
||||
logger.info(`---\`saveConvo\` context: ${metadata?.context}`);
|
||||
update.expiredAt = createFallbackRetentionDate();
|
||||
update.expiredAt = capForcedRetentionExpiry(
|
||||
parentRetention?.expiredAt,
|
||||
createFallbackRetentionDate(),
|
||||
);
|
||||
}
|
||||
} else if (interfaceConfig?.retentionMode === RetentionMode.ALL) {
|
||||
if (typeof isTemporary === 'boolean') {
|
||||
|
|
|
|||
|
|
@ -1180,9 +1180,11 @@ describe('Message Operations', () => {
|
|||
|
||||
describe('applyForcedRetentionToTag', () => {
|
||||
const Conversation = () => mongoose.models.Conversation as mongoose.Model<IConversation>;
|
||||
const SharedLink = () => mongoose.models.SharedLink as mongoose.Model<ISharedLink>;
|
||||
|
||||
beforeEach(async () => {
|
||||
await Conversation().deleteMany({});
|
||||
await SharedLink().deleteMany({});
|
||||
});
|
||||
|
||||
it('converts every permanent conversation carrying the tag under ephemeral mode', async () => {
|
||||
|
|
@ -1225,17 +1227,55 @@ describe('Message Operations', () => {
|
|||
expect(untouched?.expiredAt ?? null).toBeNull();
|
||||
});
|
||||
|
||||
it('does not extend a tagged conversation that already expires sooner', async () => {
|
||||
const conversationId = uuidv4();
|
||||
it('preserves earlier expirations per tagged conversation when cascading forced retention', async () => {
|
||||
const soonerConversationId = uuidv4();
|
||||
const permanentConversationId = uuidv4();
|
||||
const soonerExpiry = new Date(Date.now() + 60 * 60 * 1000);
|
||||
await Conversation().create({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
tags: ['work'],
|
||||
isTemporary: true,
|
||||
expiredAt: soonerExpiry,
|
||||
});
|
||||
await Conversation().create([
|
||||
{
|
||||
conversationId: soonerConversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
tags: ['work'],
|
||||
isTemporary: false,
|
||||
expiredAt: soonerExpiry,
|
||||
},
|
||||
{
|
||||
conversationId: permanentConversationId,
|
||||
user: 'user123',
|
||||
endpoint: 'openAI',
|
||||
tags: ['work'],
|
||||
},
|
||||
]);
|
||||
await Message.create([
|
||||
{
|
||||
messageId: uuidv4(),
|
||||
conversationId: soonerConversationId,
|
||||
user: 'user123',
|
||||
text: 'sooner',
|
||||
isTemporary: false,
|
||||
expiredAt: soonerExpiry,
|
||||
},
|
||||
{
|
||||
messageId: uuidv4(),
|
||||
conversationId: permanentConversationId,
|
||||
user: 'user123',
|
||||
text: 'permanent',
|
||||
},
|
||||
]);
|
||||
await SharedLink().create([
|
||||
{
|
||||
conversationId: soonerConversationId,
|
||||
user: 'user123',
|
||||
shareId: uuidv4(),
|
||||
expiredAt: soonerExpiry,
|
||||
},
|
||||
{
|
||||
conversationId: permanentConversationId,
|
||||
user: 'user123',
|
||||
shareId: uuidv4(),
|
||||
},
|
||||
]);
|
||||
|
||||
await applyForcedRetentionToTag(
|
||||
{
|
||||
|
|
@ -1246,8 +1286,34 @@ describe('Message Operations', () => {
|
|||
{ context: 'PUT /api/tags/:tag' },
|
||||
);
|
||||
|
||||
const convo = await Conversation().findOne({ conversationId }).lean();
|
||||
expect(convo?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
|
||||
const soonerConvo = await Conversation()
|
||||
.findOne({ conversationId: soonerConversationId })
|
||||
.lean();
|
||||
expect(soonerConvo?.isTemporary).toBe(true);
|
||||
expect(soonerConvo?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
|
||||
const soonerMessage = await Message.findOne({ conversationId: soonerConversationId }).lean();
|
||||
expect(soonerMessage?.isTemporary).toBe(true);
|
||||
expect(soonerMessage?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
|
||||
const soonerShare = await SharedLink()
|
||||
.findOne({ conversationId: soonerConversationId })
|
||||
.lean();
|
||||
expect(soonerShare?.expiredAt?.getTime()).toBe(soonerExpiry.getTime());
|
||||
|
||||
const permanentConvo = await Conversation()
|
||||
.findOne({ conversationId: permanentConversationId })
|
||||
.lean();
|
||||
expect(permanentConvo?.isTemporary).toBe(true);
|
||||
expect(permanentConvo?.expiredAt).toBeInstanceOf(Date);
|
||||
expect(permanentConvo?.expiredAt?.getTime()).toBeGreaterThan(soonerExpiry.getTime());
|
||||
const permanentMessage = await Message.findOne({
|
||||
conversationId: permanentConversationId,
|
||||
}).lean();
|
||||
expect(permanentMessage?.isTemporary).toBe(true);
|
||||
expect(permanentMessage?.expiredAt?.getTime()).toBeGreaterThan(soonerExpiry.getTime());
|
||||
const permanentShare = await SharedLink()
|
||||
.findOne({ conversationId: permanentConversationId })
|
||||
.lean();
|
||||
expect(permanentShare?.expiredAt?.getTime()).toBeGreaterThan(soonerExpiry.getTime());
|
||||
});
|
||||
|
||||
it('is a no-op outside forced retention', async () => {
|
||||
|
|
|
|||
|
|
@ -68,6 +68,22 @@ export const conversationNeedsForcedRetention = (
|
|||
return parent.expiredAt.getTime() > forcedExpiredAt.getTime();
|
||||
};
|
||||
|
||||
export const capForcedRetentionExpiry = (
|
||||
expiredAt: Date | null | undefined,
|
||||
forcedExpiredAt: Date,
|
||||
): Date => {
|
||||
if (!(expiredAt instanceof Date)) {
|
||||
return forcedExpiredAt;
|
||||
}
|
||||
|
||||
const existingTime = expiredAt.getTime();
|
||||
if (!Number.isNaN(existingTime) && existingTime < forcedExpiredAt.getTime()) {
|
||||
return expiredAt;
|
||||
}
|
||||
|
||||
return forcedExpiredAt;
|
||||
};
|
||||
|
||||
/**
|
||||
* Applies forced-retention deadlines to a conversation's messages that do not yet
|
||||
* conform to the forced window.
|
||||
|
|
@ -137,12 +153,12 @@ export const capForcedRetentionToParent = async (
|
|||
const parent = await Conversation.findOne({ conversationId, user: userId }, 'expiredAt').lean<{
|
||||
expiredAt?: Date | null;
|
||||
} | null>();
|
||||
if (parent?.expiredAt instanceof Date && parent.expiredAt.getTime() < forcedExpiredAt.getTime()) {
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, parent.expiredAt);
|
||||
await capConversationSharedLinks(SharedLink, userId, conversationId, parent.expiredAt);
|
||||
return parent.expiredAt;
|
||||
const expiredAt = capForcedRetentionExpiry(parent?.expiredAt, forcedExpiredAt);
|
||||
if (expiredAt !== forcedExpiredAt) {
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, expiredAt);
|
||||
await capConversationSharedLinks(SharedLink, userId, conversationId, expiredAt);
|
||||
}
|
||||
return forcedExpiredAt;
|
||||
return expiredAt;
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -159,13 +175,18 @@ export const cascadeForcedConversationRetention = async (
|
|||
conversationId: string,
|
||||
forcedExpiredAt: Date,
|
||||
): Promise<void> => {
|
||||
const parent = await Conversation.findOne(
|
||||
{ conversationId, user: userId },
|
||||
'isTemporary expiredAt',
|
||||
).lean<RetentionFilterDocument | null>();
|
||||
const expiredAt = capForcedRetentionExpiry(parent?.expiredAt, forcedExpiredAt);
|
||||
const convoResult = await Conversation.updateOne(
|
||||
{ conversationId, user: userId, ...forcedRetentionGapFilter<IConversation>(forcedExpiredAt) },
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
{ conversationId, user: userId, ...forcedRetentionGapFilter<IConversation>(expiredAt) },
|
||||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
if (convoResult.modifiedCount > 0) {
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, forcedExpiredAt);
|
||||
await capConversationSharedLinks(SharedLink, userId, conversationId, forcedExpiredAt);
|
||||
await forceConversationMessagesTemporary(Message, userId, conversationId, expiredAt);
|
||||
await capConversationSharedLinks(SharedLink, userId, conversationId, expiredAt);
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -187,34 +208,52 @@ export const cascadeForcedRetentionByTag = async (
|
|||
): Promise<void> => {
|
||||
const taggedConversations = await Conversation.find(
|
||||
{ user: userId, tags: tag },
|
||||
'conversationId',
|
||||
).lean<Array<{ conversationId: string }>>();
|
||||
'conversationId isTemporary expiredAt',
|
||||
).lean<Array<RetentionFilterDocument & { conversationId?: string }>>();
|
||||
if (taggedConversations.length === 0) {
|
||||
return;
|
||||
}
|
||||
const conversationIds = taggedConversations.map((convo) => convo.conversationId);
|
||||
await Conversation.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
...forcedRetentionGapFilter<IConversation>(forcedExpiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
await Message.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
...forcedRetentionGapFilter<IMessage>(forcedExpiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
await SharedLink.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
$or: [{ expiredAt: null }, { expiredAt: { $gt: forcedExpiredAt } }],
|
||||
},
|
||||
{ $set: { expiredAt: forcedExpiredAt } },
|
||||
);
|
||||
|
||||
const retentionBuckets = new Map<number, { expiredAt: Date; conversationIds: string[] }>();
|
||||
for (const convo of taggedConversations) {
|
||||
if (typeof convo.conversationId !== 'string' || convo.conversationId.length === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const expiredAt = capForcedRetentionExpiry(convo.expiredAt, forcedExpiredAt);
|
||||
const key = expiredAt.getTime();
|
||||
const bucket = retentionBuckets.get(key);
|
||||
if (bucket) {
|
||||
bucket.conversationIds.push(convo.conversationId);
|
||||
continue;
|
||||
}
|
||||
retentionBuckets.set(key, { expiredAt, conversationIds: [convo.conversationId] });
|
||||
}
|
||||
|
||||
for (const { expiredAt, conversationIds } of retentionBuckets.values()) {
|
||||
await Conversation.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
...forcedRetentionGapFilter<IConversation>(expiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
await Message.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
...forcedRetentionGapFilter<IMessage>(expiredAt),
|
||||
},
|
||||
{ $set: { isTemporary: true, expiredAt } },
|
||||
);
|
||||
await SharedLink.updateMany(
|
||||
{
|
||||
user: userId,
|
||||
conversationId: { $in: conversationIds },
|
||||
$or: [{ expiredAt: null }, { expiredAt: { $gt: expiredAt } }],
|
||||
},
|
||||
{ $set: { expiredAt } },
|
||||
);
|
||||
}
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue