From b94c563527e627d661842fac03e1de64945368ea Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Wed, 24 Jun 2026 17:18:51 +0200 Subject: [PATCH] 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. --- api/server/services/Files/process.spec.js | 45 ++++++- packages/api/src/files/retention.spec.ts | 22 ++++ packages/api/src/files/retention.ts | 7 +- .../src/methods/conversation.spec.ts | 48 ++++++++ .../data-schemas/src/methods/conversation.ts | 11 +- .../data-schemas/src/methods/message.spec.ts | 90 ++++++++++++-- packages/data-schemas/src/utils/retention.ts | 111 ++++++++++++------ 7 files changed, 277 insertions(+), 57 deletions(-) diff --git a/api/server/services/Files/process.spec.js b/api/server/services/Files/process.spec.js index 1521f873bf..eb652ec7c6 100644 --- a/api/server/services/Files/process.spec.js +++ b/api/server/services/Files/process.spec.js @@ -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 }); diff --git a/packages/api/src/files/retention.spec.ts b/packages/api/src/files/retention.spec.ts index c2d49b8e66..b6b3ccc801 100644 --- a/packages/api/src/files/retention.spec.ts +++ b/packages/api/src/files/retention.spec.ts @@ -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( { diff --git a/packages/api/src/files/retention.ts b/packages/api/src/files/retention.ts index 96a510eadb..7487b2f5a0 100644 --- a/packages/api/src/files/retention.ts +++ b/packages/api/src/files/retention.ts @@ -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)) ); }; diff --git a/packages/data-schemas/src/methods/conversation.spec.ts b/packages/data-schemas/src/methods/conversation.spec.ts index e11b3db4ab..ac4af0b199 100644 --- a/packages/data-schemas/src/methods/conversation.spec.ts +++ b/packages/data-schemas/src/methods/conversation.spec.ts @@ -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; + 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({ 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); diff --git a/packages/data-schemas/src/methods/conversation.ts b/packages/data-schemas/src/methods/conversation.ts index eba2c7b72f..481097a289 100644 --- a/packages/data-schemas/src/methods/conversation.ts +++ b/packages/data-schemas/src/methods/conversation.ts @@ -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') { diff --git a/packages/data-schemas/src/methods/message.spec.ts b/packages/data-schemas/src/methods/message.spec.ts index 8fcc5f95b3..a4a047c9c1 100644 --- a/packages/data-schemas/src/methods/message.spec.ts +++ b/packages/data-schemas/src/methods/message.spec.ts @@ -1180,9 +1180,11 @@ describe('Message Operations', () => { describe('applyForcedRetentionToTag', () => { const Conversation = () => mongoose.models.Conversation as mongoose.Model; + const SharedLink = () => mongoose.models.SharedLink as mongoose.Model; 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 () => { diff --git a/packages/data-schemas/src/utils/retention.ts b/packages/data-schemas/src/utils/retention.ts index 216f60781d..71aca59324 100644 --- a/packages/data-schemas/src/utils/retention.ts +++ b/packages/data-schemas/src/utils/retention.ts @@ -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 => { + const parent = await Conversation.findOne( + { conversationId, user: userId }, + 'isTemporary expiredAt', + ).lean(); + const expiredAt = capForcedRetentionExpiry(parent?.expiredAt, forcedExpiredAt); const convoResult = await Conversation.updateOne( - { conversationId, user: userId, ...forcedRetentionGapFilter(forcedExpiredAt) }, - { $set: { isTemporary: true, expiredAt: forcedExpiredAt } }, + { conversationId, user: userId, ...forcedRetentionGapFilter(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 => { const taggedConversations = await Conversation.find( { user: userId, tags: tag }, - 'conversationId', - ).lean>(); + 'conversationId isTemporary expiredAt', + ).lean>(); if (taggedConversations.length === 0) { return; } - const conversationIds = taggedConversations.map((convo) => convo.conversationId); - await Conversation.updateMany( - { - user: userId, - conversationId: { $in: conversationIds }, - ...forcedRetentionGapFilter(forcedExpiredAt), - }, - { $set: { isTemporary: true, expiredAt: forcedExpiredAt } }, - ); - await Message.updateMany( - { - user: userId, - conversationId: { $in: conversationIds }, - ...forcedRetentionGapFilter(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(); + 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(expiredAt), + }, + { $set: { isTemporary: true, expiredAt } }, + ); + await Message.updateMany( + { + user: userId, + conversationId: { $in: conversationIds }, + ...forcedRetentionGapFilter(expiredAt), + }, + { $set: { isTemporary: true, expiredAt } }, + ); + await SharedLink.updateMany( + { + user: userId, + conversationId: { $in: conversationIds }, + $or: [{ expiredAt: null }, { expiredAt: { $gt: expiredAt } }], + }, + { $set: { expiredAt } }, + ); + } };