CERBERUS/LibreChat
1
0
Fork 0
mirror of https://github.com/danny-avila/LibreChat.git synced 2026-08-04 14:57:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

🔒 fix: Renew the /files/usage hold across queued runs, under a ceiling

Browse source 
Codex review on 2bd3c52.

The drain sends one queued item per run completion, and each item starts a
run that may itself pause for the full approval window. Since the hold was
taken once at enqueue and pinned to the upload time, an item several places
back could sit through multiple approval windows and lose its attachment
while its chip and the live approval were still there. Another regression
from this PR: the old `$unset` made retention permanent, so deep queues
happened to work.

The queue is unbounded, so no fixed lifetime covers it. Split the hold into
a renewable window and a ceiling:

  expiresAt = max(expiresAt, min(now + renewMs, createdAt + maxLifetimeMs))

`renewMs` covers one run's wait and is granted from now, so a queue that is
still draining re-asserts it at each transition; `useQueueDrain` now marks
the remaining items' files whenever it pops one. `maxLifetimeMs` is
measured from the immutable upload time and clamps every renewal, so
repeated touches converge on a ceiling instead of advancing per call, which
keeps the replay bound from the previous round intact.

This also tightens abandonment: a queue nobody drains now lapses one
`renewMs` after its last touch instead of surviving to the ceiling.

`useQueueDrain`'s spec gained a QueryClientProvider, since the renewal goes
through react-query.
This commit is contained in:
Danny Avila 2026-07-27 22:53:03 -04:00
parent 2bd3c5293a
commit f616bed333
7 changed files with 310 additions and 116 deletions
Download patch file Download diff file Expand all files Collapse all files

22
api/server/routes/files/files.test.js
View file

@ -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 () => {

79
client/src/hooks/Chat/__tests__/useQueueDrain.spec.tsx
View file

@ -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 }) => (
<RecoilRoot initializeState={initialize}>
<Harness />
{children}
</RecoilRoot>
<QueryClientProvider client={queryClient}>
<RecoilRoot initializeState={initialize}>
<Harness />
{children}
</RecoilRoot>
</QueryClientProvider>
);
renderHook(() => null, { wrapper });
return { ask, setters };
@ -57,6 +71,10 @@ const runEnd = (overrides: Partial<RunEnd> = {}): 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 }) => (
<RecoilRoot initializeState={initialize}>
<Harness />
{children}
</RecoilRoot>
<QueryClientProvider client={queryClient}>
<RecoilRoot initializeState={initialize}>
<Harness />
{children}
</RecoilRoot>
</QueryClientProvider>
);
renderHook(() => null, { wrapper });
return { ask, setters, state };

52
client/src/hooks/Chat/useQueueDrain.ts
View file

@ -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,
]);
}

58
packages/api/src/files/usage.spec.ts
View file

@ -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,
});
});
});

49
packages/api/src/files/usage.ts
View file

@ -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<number>;
/** 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,
});

129
packages/data-schemas/src/methods/file.spec.ts
View file

@ -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());
});
});

37
packages/data-schemas/src/methods/file.ts
View file

@ -84,7 +84,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
) => Promise<IMongoFile[]>;
extendFilesTTL: (
fileIds: string[],
holdMs: number,
hold: { renewMs: number; maxLifetimeMs: number },
owner: { user: string; tenantId?: string | null },
) => Promise<number>;
sweepOrphanedPreviews: (maxAgeMs?: number) => Promise<number>;
@ -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<number> {
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<IMongoFile>;
@ -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. */