🔒 fix: Track the configured approval window in the /files/usage hold

Codex review on 9277620.

`endpoints.agents.checkpointer.ttl` is a positive int with no upper bound,
and its docs invite raising it for longer review windows. It drives the
pending-action expiry, so a run can legitimately stay paused past 24h. The
fixed 24h lifetime would then let Mongo reap an attachment while its
approval was still live, and the later queue drain would send a file that
no longer exists.

Replace the fixed constant with `resolveFilesUsageHoldMs`, which adds the
configured approval window to a 24h baseline covering upload, enqueue, and
the run reaching its pause. The route reads the window from the same
`getApprovalTtlMs(checkpointerCfg)` the pending action uses, so the two
stay in lockstep.

The replay bound is unaffected: the window is a per-deployment constant and
the deadline is still `createdAt + holdMs`, so a replayed touch re-asserts
the same instant and `$max` skips the write. Only an operator config change
moves it, never a client.
This commit is contained in:
Danny Avila 2026-07-27 22:31:45 -04:00
parent 9277620282
commit 2bd3c5293a
4 changed files with 81 additions and 17 deletions

View file

@ -3,6 +3,7 @@ const express = require('express');
const { logger, SystemCapabilities } = require('@librechat/data-schemas');
const {
logAxiosError,
getApprovalTtlMs,
refreshS3FileUrls,
handleFilesUsageRequest,
shouldUseUploadSse,
@ -147,13 +148,16 @@ router.get('/config', async (req, res) => {
* Owner-scoped TTL hold for uploads sitting in a client-side queue (mid-run
* queued messages), so the upload-window TTL cannot reap them before drain.
* Extends the deadline rather than clearing it; the real release happens at
* send. Thin wrapper: validation, cap, hold window, and best-effort semantics
* live in `@librechat/api` (`handleFilesUsageRequest`).
* send. The approval window is passed through so a queue waiting on a paused
* run outlives that pause. Thin wrapper: validation, cap, hold window, and
* best-effort semantics live in `@librechat/api` (`handleFilesUsageRequest`).
*/
router.post('/usage', async (req, res) => {
try {
const checkpointerCfg = req.config?.endpoints?.[EModelEndpoint.agents]?.checkpointer;
const { status, body } = await handleFilesUsageRequest(req.user ?? {}, req.body ?? {}, {
extendFilesTTL: db.extendFilesTTL,
approvalTtlMs: getApprovalTtlMs(checkpointerCfg),
});
return res.status(status).json(body);
} catch (error) {

View file

@ -966,8 +966,11 @@ 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. */
expect(held.expiresAt.getTime()).toBe(held.createdAt.getTime() + 24 * 60 * 60 * 1000);
/* 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. */
const HOUR = 60 * 60 * 1000;
expect(held.expiresAt.getTime()).toBe(held.createdAt.getTime() + 24 * HOUR + 24 * HOUR);
/* A queue touch is not a send, so it must not inflate usage. */
expect(held.usage).toBe(0);
});

View file

@ -1,4 +1,9 @@
import { handleFilesUsageRequest, FILES_USAGE_MAX_IDS, FILES_USAGE_HOLD_MS } from './usage';
import {
handleFilesUsageRequest,
resolveFilesUsageHoldMs,
FILES_USAGE_BASE_HOLD_MS,
FILES_USAGE_MAX_IDS,
} from './usage';
describe('handleFilesUsageRequest', () => {
const user = { id: 'user-1', tenantId: 'tenant-1' };
@ -37,11 +42,11 @@ describe('handleFilesUsageRequest', () => {
expect(deps.extendFilesTTL).not.toHaveBeenCalled();
});
it('requests an owner-scoped hold of the fixed lifetime', async () => {
it('requests an owner-scoped hold of the resolved lifetime', 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_HOLD_MS, {
expect(deps.extendFilesTTL).toHaveBeenCalledWith(['f1', 'f2'], FILES_USAGE_BASE_HOLD_MS, {
user: 'user-1',
tenantId: 'tenant-1',
});
@ -58,11 +63,40 @@ describe('handleFilesUsageRequest', () => {
const [, firstHold] = deps.extendFilesTTL.mock.calls[0];
const [, secondHold] = deps.extendFilesTTL.mock.calls[1];
expect(firstHold).toBe(FILES_USAGE_HOLD_MS);
expect(firstHold).toBe(FILES_USAGE_BASE_HOLD_MS);
expect(secondHold).toBe(firstHold);
expect(typeof firstHold).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. */
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);
});
describe('resolveFilesUsageHoldMs', () => {
it('covers the approval window on top of the baseline', () => {
expect(resolveFilesUsageHoldMs(1000)).toBe(FILES_USAGE_BASE_HOLD_MS + 1000);
});
it.each([
['undefined', undefined],
['zero', 0],
['negative', -1],
['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);
});
});
it('returns 200 with zero held when no id resolves to an owned file', async () => {
const deps = createDeps(0);
const result = await handleFilesUsageRequest(user, { file_ids: ['not-owned'] }, deps);

View file

@ -2,15 +2,33 @@
export const FILES_USAGE_MAX_IDS: number = 10;
/**
* Total lifetime a held upload gets, measured from its upload time. Generous
* enough to outlast any realistic queue wait (long run, approval pause),
* short enough that a queue the user abandons still gets reaped.
*
* Measured from upload rather than from the request, so the deadline is 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.
* Baseline lifetime a held upload gets, measured from its upload time. Covers
* the span a queued attachment spends before a run can even pause: upload,
* enqueue, and the run itself reaching an approval point.
*/
export const FILES_USAGE_HOLD_MS: number = 24 * 60 * 60 * 1000;
export const FILES_USAGE_BASE_HOLD_MS: number = 24 * 60 * 60 * 1000;
/**
* Total lifetime granted to a held upload, measured from its upload time.
*
* 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.
*
* 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.
*
* @param approvalTtlMs - Configured approval window; falsy means none configured
*/
export function resolveFilesUsageHoldMs(approvalTtlMs?: number): number {
const approval = Number.isFinite(approvalTtlMs) && approvalTtlMs! > 0 ? approvalTtlMs! : 0;
return FILES_USAGE_BASE_HOLD_MS + approval;
}
export interface FilesUsageUser {
id?: string;
@ -34,6 +52,8 @@ export interface FilesUsageDeps {
holdMs: number,
owner: { user: string; tenantId?: string | null },
) => Promise<number>;
/** Configured approval window, so the hold outlasts a paused run. */
approvalTtlMs?: number;
}
/**
@ -49,6 +69,9 @@ export interface FilesUsageDeps {
* is idempotent under replay; 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.
*
* Best-effort 200: ids that do not resolve to a held file are not errors
* (they may be already-sent files, or not owned).
*/
@ -74,7 +97,7 @@ export async function handleFilesUsageRequest(
}
fileIds.push(value);
}
const held = await deps.extendFilesTTL(fileIds, FILES_USAGE_HOLD_MS, {
const held = await deps.extendFilesTTL(fileIds, resolveFilesUsageHoldMs(deps.approvalTtlMs), {
user: user.id,
tenantId: user.tenantId,
});