diff --git a/api/server/routes/files/files.test.js b/api/server/routes/files/files.test.js index d9c81efa09..d087fc508d 100644 --- a/api/server/routes/files/files.test.js +++ b/api/server/routes/files/files.test.js @@ -966,11 +966,14 @@ describe('File Routes - Delete with Agent Access', () => { /* The hold must remain a hold: still reapable, just later. */ expect(held.expiresAt).toBeDefined(); expect(held.expiresAt.getTime()).toBeGreaterThan(soon.getTime()); - /* Anchored to upload time, so the deadline is a fixed point per file: - * the 24h baseline plus the default 24h approval window, so a queue - * waiting on a paused run outlives that pause. */ + /* The 24h baseline plus the default 24h approval window, so a queue + * waiting on a paused run outlives that pause. Renewed from now, but + * never past the ceiling measured from upload time. */ const HOUR = 60 * 60 * 1000; - expect(held.expiresAt.getTime()).toBe(held.createdAt.getTime() + 24 * HOUR + 24 * HOUR); + expect(held.expiresAt.getTime()).toBeGreaterThan(Date.now() + 47 * HOUR); + expect(held.expiresAt.getTime()).toBeLessThanOrEqual( + held.createdAt.getTime() + 24 * HOUR + 8 * 24 * HOUR, + ); /* A queue touch is not a send, so it must not inflate usage. */ expect(held.usage).toBe(0); }); @@ -982,21 +985,22 @@ describe('File Routes - Delete with Agent Access', () => { .post('/files/usage') .send({ file_ids: [ownFileId] }); expect(first.body).toEqual({ held: 1 }); - const afterFirst = (await File.findOne({ file_id: ownFileId }).lean()).expiresAt; - /* Replay must be inert, not merely bounded: a deadline derived from the - * request clock would advance a window per call and never converge. */ for (let i = 0; i < 5; i++) { const repeat = await request(app) .post('/files/usage') .send({ file_ids: [ownFileId] }); expect(repeat.status).toBe(200); - expect(repeat.body).toEqual({ held: 0 }); } + /* Every renewal is clamped to the ceiling measured from upload time, so + * replay converges there instead of advancing a window per call. */ + const HOUR = 60 * 60 * 1000; const held = await File.findOne({ file_id: ownFileId }).lean(); expect(held.expiresAt).toBeDefined(); - expect(held.expiresAt.getTime()).toBe(afterFirst.getTime()); + expect(held.expiresAt.getTime()).toBeLessThanOrEqual( + held.createdAt.getTime() + 24 * HOUR + 8 * 24 * HOUR, + ); }); it('never re-adds a TTL to a file that was already sent', async () => { diff --git a/client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx b/client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx index fce29c79a2..d638f89f99 100644 --- a/client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx +++ b/client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx @@ -1,11 +1,18 @@ import React from 'react'; import { Constants } from 'librechat-data-provider'; import { act, renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { RecoilRoot, useRecoilValue, useSetRecoilState, type MutableSnapshot } from 'recoil'; import type { RunEnd, QueuedMessage } from '~/store/families'; import useQueueDrain from '../useQueueDrain'; import store from '~/store'; +const mockMarkFilesUsage = jest.fn(); +jest.mock('~/data-provider', () => ({ + ...jest.requireActual('~/data-provider'), + useMarkFilesUsageMutation: () => ({ mutate: mockMarkFilesUsage }), +})); + const INDEX = 0; const CONVO_ID = 'convo-drain'; @@ -35,11 +42,18 @@ function setup( return null; } + /* The drain renews the TTL hold on what stays queued, which goes through + * react-query, so the harness needs a client. */ + const queryClient = new QueryClient({ + defaultOptions: { mutations: { retry: false } }, + }); const wrapper = ({ children }: { children: React.ReactNode }) => ( - - - {children} - + + + + {children} + + ); renderHook(() => null, { wrapper }); return { ask, setters }; @@ -57,6 +71,10 @@ const runEnd = (overrides: Partial = {}): RunEnd => ({ }); describe('useQueueDrain', () => { + beforeEach(() => { + mockMarkFilesUsage.mockClear(); + }); + it('drains exactly one queued message on clean completion', async () => { const { ask, setters } = setup(({ set }) => { set(store.queuedMessagesByConvoId(CONVO_ID), [ @@ -117,6 +135,46 @@ describe('useQueueDrain', () => { ); }); + /** Items behind the drained one wait another full run, which may itself + * pause for approval, so a hold taken once at enqueue would lapse before a + * deep queue finishes draining. */ + it('renews the TTL hold on attachments that stay queued', async () => { + const { setters } = setup(({ set }) => { + set(store.queuedMessagesByConvoId(CONVO_ID), [ + { ...queuedMessage('q1', 'first'), files: [{ file_id: 'sent', type: 'image/png' }] }, + { + ...queuedMessage('q2', 'second'), + files: [{ file_id: 'still-queued', type: 'image/png' }], + }, + { ...queuedMessage('q3', 'third'), files: [{ file_id: 'also-queued', type: 'image/png' }] }, + ]); + }); + + act(() => { + setters.setRunEnd!(runEnd()); + }); + + await waitFor(() => expect(mockMarkFilesUsage).toHaveBeenCalledTimes(1)); + /* Only what remains: the drained item's file is released at send. */ + expect(mockMarkFilesUsage).toHaveBeenCalledWith({ + file_ids: ['still-queued', 'also-queued'], + }); + }); + + it('does not renew when nothing with attachments stays queued', async () => { + const { setters } = setup(({ set }) => { + set(store.queuedMessagesByConvoId(CONVO_ID), [ + { ...queuedMessage('q1', 'only one'), files: [{ file_id: 'sent', type: 'image/png' }] }, + ]); + }); + + act(() => { + setters.setRunEnd!(runEnd()); + }); + + await waitFor(() => expect(mockMarkFilesUsage).not.toHaveBeenCalled()); + }); + it('passes carried quotes + manual skills through as overrides', async () => { const { ask, setters } = setup(({ set }) => { set(store.queuedMessagesByConvoId(CONVO_ID), [ @@ -235,11 +293,16 @@ describe('useQueueDrain', () => { return null; } + const queryClient = new QueryClient({ + defaultOptions: { mutations: { retry: false } }, + }); const wrapper = ({ children }: { children: React.ReactNode }) => ( - - - {children} - + + + + {children} + + ); renderHook(() => null, { wrapper }); return { ask, setters, state }; diff --git a/client/src/hooks/Chat/useQueueDrain.ts b/client/src/hooks/Chat/useQueueDrain.ts index 588038012e..8498d5e7da 100644 --- a/client/src/hooks/Chat/useQueueDrain.ts +++ b/client/src/hooks/Chat/useQueueDrain.ts @@ -3,8 +3,27 @@ import { Constants } from 'librechat-data-provider'; import { useRecoilValue, useRecoilCallback } from 'recoil'; import type { QueuedMessage } from '~/store/families'; import type { TAskFunction } from '~/common'; +import { useMarkFilesUsageMutation } from '~/data-provider'; import store from '~/store'; +/** Mirrors the server's per-request cap on a usage touch. */ +const QUEUE_USAGE_MAX_FILES = 10; + +const collectQueuedFileIds = (items: QueuedMessage[]): string[] => { + const fileIds: string[] = []; + for (const item of items) { + for (const file of item.files ?? []) { + if (typeof file.file_id === 'string' && file.file_id.length > 0) { + fileIds.push(file.file_id); + } + if (fileIds.length === QUEUE_USAGE_MAX_FILES) { + return fileIds; + } + } + } + return fileIds; +}; + /** * Auto-sends queued follow-up messages when a run finishes. * @@ -30,13 +49,18 @@ export default function useQueueDrain( store.pendingRunEndByConvoId(activeConversationId ?? Constants.NEW_CONVO), ); const isSubmitting = useRecoilValue(store.isSubmittingFamily(index)); + const { mutate: markFilesUsage } = useMarkFilesUsageMutation(); // Fully synchronous reads (getLoadable): a useRecoilCallback snapshot is // only guaranteed valid for the callback's synchronous execution, so no // awaits may interleave with the reads. const drainNext = useRecoilCallback( ({ snapshot, set }) => - (): { next: QueuedMessage; conversationId: string } | null => { + (): { + next: QueuedMessage; + conversationId: string; + remainderFileIds: string[]; + } | null => { let end = snapshot.getLoadable(store.runEndByIndex(index)).getValue(); let fromParked = false; if ( @@ -115,7 +139,9 @@ export default function useQueueDrain( if (remainder.length !== ownQueue.length || shouldMigrate || next != null) { set(store.queuedMessagesByConvoId(conversationId), remainder); } - return next ? { next, conversationId } : null; + return next + ? { next, conversationId, remainderFileIds: collectQueuedFileIds(remainder) } + : null; }, [index, activeConversationId], ); @@ -138,7 +164,16 @@ export default function useQueueDrain( if (drained == null) { return; } - const { next, conversationId } = drained; + const { next, conversationId, remainderFileIds } = drained; + /** Renew the TTL hold on what stays queued. Items behind this one wait + * another full run (which may itself pause for approval), so a hold + * taken once at enqueue would lapse before a deep queue drains. The + * server caps each renewal against the upload time, so this cannot + * extend a file indefinitely. Fire-and-forget: send-time marking is + * the backstop. */ + if (remainderFileIds.length > 0) { + markFilesUsage({ file_ids: remainderFileIds }); + } // The queued item is the FULL submission context: explicit (possibly // empty) overrides stop `ask` from vacuuming up files, quotes, or skill // picks the user has staged in the composer for their NEXT message. @@ -157,5 +192,14 @@ export default function useQueueDrain( // available for manual send. restoreQueued(conversationId, next); } - }, [runEnd, parkedRunEnd, isSubmitting, activeConversationId, drainNext, restoreQueued, ask]); + }, [ + runEnd, + parkedRunEnd, + isSubmitting, + activeConversationId, + drainNext, + restoreQueued, + markFilesUsage, + ask, + ]); } diff --git a/packages/api/src/files/usage.spec.ts b/packages/api/src/files/usage.spec.ts index 133d775488..102f235cfb 100644 --- a/packages/api/src/files/usage.spec.ts +++ b/packages/api/src/files/usage.spec.ts @@ -1,6 +1,7 @@ import { handleFilesUsageRequest, - resolveFilesUsageHoldMs, + resolveFilesUsageHold, + FILES_USAGE_QUEUED_RUN_ALLOWANCE, FILES_USAGE_BASE_HOLD_MS, FILES_USAGE_MAX_IDS, } from './usage'; @@ -42,48 +43,58 @@ describe('handleFilesUsageRequest', () => { expect(deps.extendFilesTTL).not.toHaveBeenCalled(); }); - it('requests an owner-scoped hold of the resolved lifetime', async () => { + it('requests an owner-scoped hold of the resolved window', async () => { const deps = createDeps(1); const result = await handleFilesUsageRequest(user, { file_ids: ['f1', 'f2'] }, deps); expect(deps.extendFilesTTL).toHaveBeenCalledTimes(1); - expect(deps.extendFilesTTL).toHaveBeenCalledWith(['f1', 'f2'], FILES_USAGE_BASE_HOLD_MS, { - user: 'user-1', - tenantId: 'tenant-1', - }); + expect(deps.extendFilesTTL).toHaveBeenCalledWith( + ['f1', 'f2'], + { renewMs: FILES_USAGE_BASE_HOLD_MS, maxLifetimeMs: FILES_USAGE_BASE_HOLD_MS }, + { user: 'user-1', tenantId: 'tenant-1' }, + ); expect(result).toEqual({ status: 200, body: { held: 1 } }); }); - /** The hold must be a lifetime the data layer anchors to the upload, not a - * deadline derived here: a request-clock deadline would let a caller walk - * the file's lifetime forward one window per call. */ - it('passes a constant lifetime, never a request-derived deadline', async () => { + /** The ceiling must be a constant the data layer clamps against the upload + * time, not a deadline derived here: a request-clock deadline with no + * ceiling would let a caller walk the file's lifetime forward per call. */ + it('passes a constant ceiling, never a request-derived deadline', async () => { const deps = createDeps(1); await handleFilesUsageRequest(user, { file_ids: ['f1'] }, deps); await handleFilesUsageRequest(user, { file_ids: ['f1'] }, deps); const [, firstHold] = deps.extendFilesTTL.mock.calls[0]; const [, secondHold] = deps.extendFilesTTL.mock.calls[1]; - expect(firstHold).toBe(FILES_USAGE_BASE_HOLD_MS); - expect(secondHold).toBe(firstHold); - expect(typeof firstHold).toBe('number'); + expect(secondHold).toEqual(firstHold); + expect(typeof firstHold.maxLifetimeMs).toBe('number'); }); /** A paused run's approval window has no configured upper bound, so a fixed - * lifetime shorter than it would reap an attachment whose approval is - * still live and leave the drain sending a missing file. */ + * window shorter than it would reap an attachment whose approval is still + * live and leave the drain sending a missing file. */ it('stretches the hold to outlast a long configured approval window', async () => { const sevenDaysMs = 7 * 24 * 60 * 60 * 1000; const deps = { ...createDeps(1), approvalTtlMs: sevenDaysMs }; await handleFilesUsageRequest(user, { file_ids: ['f1'] }, deps); - const [, holdMs] = deps.extendFilesTTL.mock.calls[0]; - expect(holdMs).toBe(FILES_USAGE_BASE_HOLD_MS + sevenDaysMs); - expect(holdMs).toBeGreaterThan(sevenDaysMs); + const [, hold] = deps.extendFilesTTL.mock.calls[0]; + expect(hold.renewMs).toBe(FILES_USAGE_BASE_HOLD_MS + sevenDaysMs); + expect(hold.renewMs).toBeGreaterThan(sevenDaysMs); }); - describe('resolveFilesUsageHoldMs', () => { - it('covers the approval window on top of the baseline', () => { - expect(resolveFilesUsageHoldMs(1000)).toBe(FILES_USAGE_BASE_HOLD_MS + 1000); + describe('resolveFilesUsageHold', () => { + it('covers one approval window per renewal', () => { + expect(resolveFilesUsageHold(1000).renewMs).toBe(FILES_USAGE_BASE_HOLD_MS + 1000); + }); + + /** The drain sends one queued item per run completion, so a deep queue + * waits behind several approval windows; the ceiling has to span them. */ + it('scales the ceiling to the queued run chain', () => { + const hold = resolveFilesUsageHold(1000); + expect(hold.maxLifetimeMs).toBe( + FILES_USAGE_BASE_HOLD_MS + 1000 * FILES_USAGE_QUEUED_RUN_ALLOWANCE, + ); + expect(hold.maxLifetimeMs).toBeGreaterThan(hold.renewMs); }); it.each([ @@ -93,7 +104,10 @@ describe('handleFilesUsageRequest', () => { ['NaN', Number.NaN], ['Infinity', Number.POSITIVE_INFINITY], ])('falls back to the baseline for %s', (_label, value) => { - expect(resolveFilesUsageHoldMs(value)).toBe(FILES_USAGE_BASE_HOLD_MS); + expect(resolveFilesUsageHold(value)).toEqual({ + renewMs: FILES_USAGE_BASE_HOLD_MS, + maxLifetimeMs: FILES_USAGE_BASE_HOLD_MS, + }); }); }); diff --git a/packages/api/src/files/usage.ts b/packages/api/src/files/usage.ts index dc7a3e024a..149e6e1ddb 100644 --- a/packages/api/src/files/usage.ts +++ b/packages/api/src/files/usage.ts @@ -9,25 +9,43 @@ export const FILES_USAGE_MAX_IDS: number = 10; export const FILES_USAGE_BASE_HOLD_MS: number = 24 * 60 * 60 * 1000; /** - * Total lifetime granted to a held upload, measured from its upload time. + * Queued runs a held attachment is assumed to wait behind. The queue is + * unbounded, so no multiple is provably sufficient; this is the depth the + * ceiling covers before a still-queued attachment can lapse. + */ +export const FILES_USAGE_QUEUED_RUN_ALLOWANCE: number = 8; + +export interface FilesUsageHold { + /** Granted from now, so an actively draining queue keeps renewing. */ + renewMs: number; + /** Hard ceiling from upload time, so renewals converge instead of walking. */ + maxLifetimeMs: number; +} + +/** + * Resolves the hold window for one touch. * * A queued message legitimately outlives the upload window when a run pauses * for approval, and that pause is bounded by the deployment's configured * approval window (`endpoints.agents.checkpointer.ttl`), which has no upper - * limit. A fixed lifetime shorter than that window would let Mongo reap an - * attachment while its approval is still live, so the hold tracks the - * configured window instead of assuming one. + * limit. Deeper queues wait behind several such runs, since the drain sends + * one item per run completion. * - * Still a constant per deployment, and still measured from upload rather than - * from the request, so the deadline stays a fixed point per file: replaying - * the touch re-asserts the same instant instead of walking the file's - * lifetime forward a window at a time. + * Hence two numbers rather than one. `renewMs` covers a single run's wait and + * is granted from now, so a queue that is still draining re-asserts it at each + * transition. `maxLifetimeMs` is measured from the immutable upload time and + * caps every renewal, so repeated touches converge on a fixed ceiling instead + * of advancing a window per call. An abandoned queue therefore lapses one + * `renewMs` after its last touch rather than surviving to the ceiling. * * @param approvalTtlMs - Configured approval window; falsy means none configured */ -export function resolveFilesUsageHoldMs(approvalTtlMs?: number): number { +export function resolveFilesUsageHold(approvalTtlMs?: number): FilesUsageHold { const approval = Number.isFinite(approvalTtlMs) && approvalTtlMs! > 0 ? approvalTtlMs! : 0; - return FILES_USAGE_BASE_HOLD_MS + approval; + return { + renewMs: FILES_USAGE_BASE_HOLD_MS + approval, + maxLifetimeMs: FILES_USAGE_BASE_HOLD_MS + approval * FILES_USAGE_QUEUED_RUN_ALLOWANCE, + }; } export interface FilesUsageUser { @@ -49,7 +67,7 @@ export interface FilesUsageDeps { /** Owner-scoped TTL hold (`db.extendFilesTTL`-shaped). */ extendFilesTTL: ( fileIds: string[], - holdMs: number, + hold: FilesUsageHold, owner: { user: string; tenantId?: string | null }, ) => Promise; /** Configured approval window, so the hold outlasts a paused run. */ @@ -65,9 +83,10 @@ export interface FilesUsageDeps { * A hold, not a release. The client queue is ephemeral browser state, so a * closed tab or a cleared queue leaves nothing referencing these files, and * clearing the TTL outright would strand them in storage permanently. The - * hold only widens the window to a fixed point measured from upload, so it - * is idempotent under replay; the real release happens at send, where - * `updateFilesUsage` marks the files used against an actual message. + * hold only ever widens, and every renewal is capped against the file's + * upload time, so replaying it converges on a ceiling instead of advancing + * indefinitely; the real release happens at send, where `updateFilesUsage` + * marks the files used against an actual message. * * The window tracks the deployment's configured approval TTL so a queue * waiting on a paused run cannot be reaped while that approval is live. @@ -97,7 +116,7 @@ export async function handleFilesUsageRequest( } fileIds.push(value); } - const held = await deps.extendFilesTTL(fileIds, resolveFilesUsageHoldMs(deps.approvalTtlMs), { + const held = await deps.extendFilesTTL(fileIds, resolveFilesUsageHold(deps.approvalTtlMs), { user: user.id, tenantId: user.tenantId, }); diff --git a/packages/data-schemas/src/methods/file.spec.ts b/packages/data-schemas/src/methods/file.spec.ts index 2fec751042..4aa424301b 100644 --- a/packages/data-schemas/src/methods/file.spec.ts +++ b/packages/data-schemas/src/methods/file.spec.ts @@ -1124,9 +1124,14 @@ describe('File Methods', () => { }); describe('extendFilesTTL', () => { - const HOLD_MS = 24 * 3_600_000; + const HOUR = 3_600_000; + const HOLD = { renewMs: 24 * HOUR, maxLifetimeMs: 48 * HOUR }; - const seedTempFile = async (userId: mongoose.Types.ObjectId, expiresAt: Date) => { + const seedTempFile = async ( + userId: mongoose.Types.ObjectId, + expiresAt: Date, + createdAt?: Date, + ) => { const fileId = uuidv4(); await fileMethods.createFile({ file_id: fileId, @@ -1136,52 +1141,77 @@ describe('File Methods', () => { type: 'text/plain', bytes: 100, }); - await mongoose.models.File.updateOne({ file_id: fileId }, { $set: { expiresAt } }); + await mongoose.models.File.updateOne( + { file_id: fileId }, + { $set: { expiresAt, ...(createdAt ? { createdAt } : {}) } }, + { timestamps: false }, + ); return fileId; }; - const readCreatedAt = async (fileId: string) => { - const doc = await mongoose.models.File.findOne({ file_id: fileId }) - .lean<{ createdAt: Date }>() - .exec(); - return doc!.createdAt; - }; + const readFile = async (fileId: string) => + (await mongoose.models.File.findOne({ file_id: fileId }) + .lean<{ createdAt: Date; expiresAt?: Date }>() + .exec())!; - it('widens the TTL to createdAt + holdMs without unsetting it', async () => { + it('widens the TTL toward the renewal window without unsetting it', async () => { const userId = new mongoose.Types.ObjectId(); const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000)); - const createdAt = await readCreatedAt(fileId); - const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) }); + const count = await fileMethods.extendFilesTTL([fileId], HOLD, { user: String(userId) }); expect(count).toBe(1); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt).toBeDefined(); - expect(file?.expiresAt?.getTime()).toBe(createdAt.getTime() + HOLD_MS); + const file = await readFile(fileId); + expect(file.expiresAt).toBeDefined(); + expect(file.expiresAt!.getTime()).toBeGreaterThan(Date.now() + 23 * HOUR); + expect(file.expiresAt!.getTime()).toBeLessThanOrEqual( + file.createdAt.getTime() + HOLD.maxLifetimeMs, + ); }); - /** The bound that makes the endpoint safe to expose: the deadline is a - * function of the immutable createdAt, so replaying the call can never - * walk a file's lifetime forward one window at a time. */ - it('is idempotent under replay, never advancing the deadline', async () => { + /** The bound that makes the endpoint safe to expose: every renewal is + * clamped to createdAt + maxLifetimeMs, so replaying the call converges + * on a ceiling instead of advancing a window at a time. */ + it('clamps every renewal to the ceiling, however often it is replayed', async () => { const userId = new mongoose.Types.ObjectId(); const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000)); - const createdAt = await readCreatedAt(fileId); + const ceilingHold = { renewMs: 24 * HOUR, maxLifetimeMs: 10 * 60_000 }; + const { createdAt } = await readFile(fileId); - const first = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) }); - const afterFirst = (await fileMethods.findFileById(fileId))?.expiresAt; + const first = await fileMethods.extendFilesTTL([fileId], ceilingHold, { + user: String(userId), + }); + expect(first).toBe(1); for (let i = 0; i < 5; i++) { - const repeat = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { - user: String(userId), - }); - expect(repeat).toBe(0); + expect( + await fileMethods.extendFilesTTL([fileId], ceilingHold, { user: String(userId) }), + ).toBe(0); } - expect(first).toBe(1); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt?.getTime()).toBe(afterFirst?.getTime()); - expect(file?.expiresAt?.getTime()).toBe(createdAt.getTime() + HOLD_MS); + const file = await readFile(fileId); + expect(file.expiresAt!.getTime()).toBe(createdAt.getTime() + ceilingHold.maxLifetimeMs); + /* The renewal window alone would have granted a full day. */ + expect(file.expiresAt!.getTime()).toBeLessThan(Date.now() + HOUR); + }); + + /** Renewal is measured from now, so a queue still draining across + * successive runs keeps its attachments past the first hold. */ + it('renews from now while the ceiling still allows it', async () => { + const userId = new mongoose.Types.ObjectId(); + const twoHoursAgo = new Date(Date.now() - 2 * HOUR); + const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000), twoHoursAgo); + + const count = await fileMethods.extendFilesTTL( + [fileId], + { renewMs: HOUR, maxLifetimeMs: 24 * HOUR }, + { user: String(userId) }, + ); + + expect(count).toBe(1); + const file = await readFile(fileId); + /* Anchoring to createdAt alone would have expired this an hour ago. */ + expect(file.expiresAt!.getTime()).toBeGreaterThan(Date.now() + 50 * 60_000); }); it('does not resurrect a TTL on an already-released file', async () => { @@ -1189,23 +1219,23 @@ describe('File Methods', () => { const fileId = await seedTempFile(userId, new Date(Date.now() + 60_000)); await fileMethods.updateFileUsage({ file_id: fileId, user: String(userId) }); - const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) }); + const count = await fileMethods.extendFilesTTL([fileId], HOLD, { user: String(userId) }); expect(count).toBe(0); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt).toBeUndefined(); + const file = await readFile(fileId); + expect(file.expiresAt).toBeUndefined(); }); it('never moves an expiry earlier', async () => { const userId = new mongoose.Types.ObjectId(); - const farOut = new Date(Date.now() + 7 * 24 * 3_600_000); + const farOut = new Date(Date.now() + 7 * 24 * HOUR); const fileId = await seedTempFile(userId, farOut); - const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: String(userId) }); + const count = await fileMethods.extendFilesTTL([fileId], HOLD, { user: String(userId) }); expect(count).toBe(0); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt?.getTime()).toBe(farOut.getTime()); + const file = await readFile(fileId); + expect(file.expiresAt!.getTime()).toBe(farOut.getTime()); }); it("leaves another user's file untouched", async () => { @@ -1214,13 +1244,13 @@ describe('File Methods', () => { const soon = new Date(Date.now() + 60_000); const fileId = await seedTempFile(ownerId, soon); - const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { + const count = await fileMethods.extendFilesTTL([fileId], HOLD, { user: String(attackerId), }); expect(count).toBe(0); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt?.getTime()).toBe(soon.getTime()); + const file = await readFile(fileId); + expect(file.expiresAt!.getTime()).toBe(soon.getTime()); }); it('is a no-op without an owner scope rather than a cross-user update', async () => { @@ -1228,24 +1258,29 @@ describe('File Methods', () => { const soon = new Date(Date.now() + 60_000); const fileId = await seedTempFile(userId, soon); - const count = await fileMethods.extendFilesTTL([fileId], HOLD_MS, { user: '' } as { + const count = await fileMethods.extendFilesTTL([fileId], HOLD, { user: '' } as { user: string; }); expect(count).toBe(0); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt?.getTime()).toBe(soon.getTime()); + const file = await readFile(fileId); + expect(file.expiresAt!.getTime()).toBe(soon.getTime()); }); it('is a no-op for a non-positive hold', async () => { const userId = new mongoose.Types.ObjectId(); const soon = new Date(Date.now() + 60_000); const fileId = await seedTempFile(userId, soon); + const owner = { user: String(userId) }; - expect(await fileMethods.extendFilesTTL([fileId], 0, { user: String(userId) })).toBe(0); - expect(await fileMethods.extendFilesTTL([fileId], -1, { user: String(userId) })).toBe(0); - const file = await fileMethods.findFileById(fileId); - expect(file?.expiresAt?.getTime()).toBe(soon.getTime()); + expect( + await fileMethods.extendFilesTTL([fileId], { renewMs: 0, maxLifetimeMs: 0 }, owner), + ).toBe(0); + expect( + await fileMethods.extendFilesTTL([fileId], { renewMs: -1, maxLifetimeMs: -1 }, owner), + ).toBe(0); + const file = await readFile(fileId); + expect(file.expiresAt!.getTime()).toBe(soon.getTime()); }); }); diff --git a/packages/data-schemas/src/methods/file.ts b/packages/data-schemas/src/methods/file.ts index 3d12454dc5..ac428bdce0 100644 --- a/packages/data-schemas/src/methods/file.ts +++ b/packages/data-schemas/src/methods/file.ts @@ -84,7 +84,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { ) => Promise; extendFilesTTL: ( fileIds: string[], - holdMs: number, + hold: { renewMs: number; maxLifetimeMs: number }, owner: { user: string; tenantId?: string | null }, ) => Promise; sweepOrphanedPreviews: (maxAgeMs?: number) => Promise; @@ -553,39 +553,45 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { /** * Widens the upload-window TTL of owned, still-temporary files to - * `createdAt + holdMs`. + * `min(now + renewMs, createdAt + maxLifetimeMs)`. * * A renewable hold, not a release: unlike `updateFileUsage` this never * unsets `expiresAt`, so a file that is held but never actually sent is - * still reaped once the hold lapses. Three properties hold by + * still reaped once the hold lapses. Four properties hold by * construction, which is what makes the write safe to drive from a * client-supplied id list: - * - the new deadline is anchored to the immutable `createdAt`, never to - * the request clock, so replaying the call is idempotent and cannot - * walk a file's lifetime forward indefinitely; + * - `$min` against `createdAt + maxLifetimeMs` caps every renewal against + * an immutable anchor, so repeated calls converge on a fixed ceiling + * instead of walking a file's lifetime forward a window at a time; + * - renewing from `now` up to that ceiling lets a queue that is still + * draining keep its attachments alive across successive runs, while an + * abandoned queue lapses a single `renewMs` after its last touch rather + * than surviving to the ceiling; * - `$max` against the current value means a hold only ever widens; * - `expiresAt: { $exists: true }` means a file whose TTL was already * cleared by a real send stays permanent. Re-adding `expiresAt` there * would schedule a live file for deletion. * * `createdAt` is required rather than defaulted: without the anchor there - * is no bound to enforce, so such a file is skipped instead of held. + * is no ceiling to enforce, so such a file is skipped instead of held. * * The owner scope is required, not optional: an unscoped call would hold * every user's matching file. A missing owner is a no-op, not a wide * update. * * @param fileIds - File IDs to hold - * @param holdMs - Lifetime granted from upload time + * @param hold - `renewMs` granted from now, capped at `maxLifetimeMs` from upload * @param owner - Owner scope; mismatches leave the TTL unchanged * @returns Number of files whose hold was widened */ async function extendFilesTTL( fileIds: string[], - holdMs: number, + hold: { renewMs: number; maxLifetimeMs: number }, owner: { user: string; tenantId?: string | null }, ): Promise { - if (fileIds.length === 0 || !owner?.user || !(holdMs > 0)) { + const renewMs = hold?.renewMs; + const maxLifetimeMs = hold?.maxLifetimeMs; + if (fileIds.length === 0 || !owner?.user || !(renewMs > 0) || !(maxLifetimeMs > 0)) { return 0; } const File = mongoose.models.File as Model; @@ -597,9 +603,18 @@ export function createFileMethods(mongoose: typeof import('mongoose')): { }, { userId: owner.user, tenantId: owner.tenantId }, ); + const renewUntil = new Date(Date.now() + renewMs); const result = await File.updateMany( filter, - [{ $set: { expiresAt: { $max: ['$expiresAt', { $add: ['$createdAt', holdMs] }] } } }], + [ + { + $set: { + expiresAt: { + $max: ['$expiresAt', { $min: [renewUntil, { $add: ['$createdAt', maxLifetimeMs] }] }], + }, + }, + }, + ], /** `timestamps: false`: a hold is TTL bookkeeping, not a content write. * Bumping `updatedAt` would also make every re-touch count as a * modification, hiding whether the deadline actually moved. */