🔒 fix: Settle sweep retry state from the write, not the read

Two findings from the review of 0e52924.

- `recordFailure` derived the attempt number by adding one to the count the
  batch had queried. Two nodes sweeping the same file read the same value,
  so both believed themselves to be the same attempt: each `$inc` landed,
  the stored count crossed `FILE_RETENTION_SWEEP_MAX_ATTEMPTS`, and neither
  caller ever saw the threshold. The file drops out of the query — the
  starvation guard still holds — but the give-up is never reported, and
  that log line is the operator's only notice, and carries the instruction
  for resuming. Return the count from the increment itself so every caller
  gets a distinct attempt number and exactly one observes the cap.

  The same staleness shortened the backoff, so `deferExpiredFile` now
  writes with `$max`: a deferral can only move later, and a node that
  computed a shorter delay cannot pull the file forward past one another
  node already committed.

- The backoff doubled from a hard-coded hour while claiming to start from
  one sweep interval. At the default they coincide; away from it the
  schedule stops meaning anything — on a six-hour sweep the first three
  attempts all land on consecutive passes, and on a five-minute one the
  first retry skips twelve. Derive the base from
  `FILE_RETENTION_SWEEP_INTERVAL_MS`, floored at a minute so a
  pathologically short interval cannot spend the whole give-up budget on a
  transient outage.

api/server/services/Files/process.js

@@ -349,6 +349,7 @@ async function sweepExpiredFiles(options = {}) {
   return sweepExpiredFilesWithDeps(options, {
     getExpiredFiles: db.getExpiredFiles,
     processDeleteRequest,
+    incrementFileDeletionAttempts: db.incrementFileDeletionAttempts,
     deferExpiredFile: db.deferExpiredFile,
     logger,
   });

api/server/services/Files/process.spec.js

@@ -172,6 +172,7 @@ jest.mock('~/models', () => ({
   findFileById: jest.fn(),
   getConvo: jest.fn(),
   getExpiredFiles: jest.fn(),
+  incrementFileDeletionAttempts: jest.fn(),
   deferExpiredFile: jest.fn(),
   addAgentResourceFile: jest.fn().mockResolvedValue({}),
   removeAgentResourceFiles: jest.fn(),
@@ -1841,6 +1842,7 @@ describe('sweepExpiredFiles', () => {
       expect.objectContaining({
         getExpiredFiles: db.getExpiredFiles,
         processDeleteRequest: expect.any(Function),
+        incrementFileDeletionAttempts: db.incrementFileDeletionAttempts,
         deferExpiredFile: db.deferExpiredFile,
         logger: expect.objectContaining({
           error: expect.any(Function),

packages/api/src/files/sweep.spec.ts

@@ -16,9 +16,13 @@ describe('expired file sweep helpers', () => {
   };
 
   let deferExpiredFile: jest.Mock;
+  let incrementFileDeletionAttempts: jest.Mock;
+  let attemptCount: number;
 
   beforeEach(() => {
     jest.clearAllMocks();
+    attemptCount = 0;
+    incrementFileDeletionAttempts = jest.fn().mockImplementation(() => ++attemptCount);
     deferExpiredFile = jest.fn().mockResolvedValue(undefined);
     delete process.env.FILE_RETENTION_SWEEP_INTERVAL_MS;
     delete process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS;
@@ -51,7 +55,13 @@ describe('expired file sweep helpers', () => {
 
     const result = await sweepExpiredFiles(
       { appConfig: {} as AppConfig, loadAppConfig, limit: 1 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     expect(getExpiredFiles).toHaveBeenCalledWith(1, { maxAttempts: 10 });
@@ -75,7 +85,13 @@ describe('expired file sweep helpers', () => {
 
     const result = await sweepExpiredFiles(
       { appConfig: {} as AppConfig, limit: 1 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     expect(processDeleteRequest).not.toHaveBeenCalled();
@@ -92,18 +108,26 @@ describe('expired file sweep helpers', () => {
   ])('defers the file after %s', async (_case, outcome) => {
     const getExpiredFiles = jest
       .fn()
-      .mockResolvedValue([{ file_id: 'stuck-file', user: 'user-1', deletionAttempts: 2 }]);
+      .mockResolvedValue([{ file_id: 'stuck-file', user: 'user-1' }]);
     const processDeleteRequest = jest.fn().mockResolvedValue(outcome);
 
     const result = await sweepExpiredFiles(
       { appConfig: {} as AppConfig, limit: 1 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     const [fileId, retryAt] = deferExpiredFile.mock.calls[0];
     expect(fileId).toBe('stuck-file');
-    /* Third consecutive failure, so the backoff has doubled twice. */
-    expect(retryAt.getTime() - Date.now()).toBeCloseTo(4 * 60 * 60 * 1000, -3);
+    /* Backoff comes off the attempt the increment returned, not the count
+     * the queried snapshot carried. */
+    expect(incrementFileDeletionAttempts).toHaveBeenCalledWith('stuck-file');
+    expect(retryAt.getTime() - Date.now()).toBeCloseTo(60 * 60 * 1000, -3);
     expect(result).toEqual({ scanned: 1, deleted: 0, failed: 1 });
   });
 
@@ -115,7 +139,13 @@ describe('expired file sweep helpers', () => {
 
     const result = await sweepExpiredFiles(
       { appConfig: {} as AppConfig, limit: 1 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     expect(deferExpiredFile).toHaveBeenCalledWith('stuck-file', expect.any(Date));
@@ -124,16 +154,23 @@ describe('expired file sweep helpers', () => {
 
   it('reports giving up once the file reaches the attempt cap', async () => {
     process.env.FILE_RETENTION_SWEEP_MAX_ATTEMPTS = '3';
+    attemptCount = 2;
     const getExpiredFiles = jest
       .fn()
-      .mockResolvedValue([{ file_id: 'stranded-file', user: 'user-1', deletionAttempts: 2 }]);
+      .mockResolvedValue([{ file_id: 'stranded-file', user: 'user-1' }]);
     const processDeleteRequest = jest
       .fn()
       .mockResolvedValue({ deletedFileIds: [], failedFileIds: ['stranded-file'] });
 
     await sweepExpiredFiles(
       { appConfig: {} as AppConfig, limit: 1 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     expect(getExpiredFiles).toHaveBeenCalledWith(1, { maxAttempts: 3 });
@@ -151,11 +188,17 @@ describe('expired file sweep helpers', () => {
       .fn()
       .mockResolvedValueOnce({ deletedFileIds: [], failedFileIds: ['stuck-file'] })
       .mockResolvedValueOnce({ deletedFileIds: ['deletable-file'], failedFileIds: [] });
-    deferExpiredFile.mockRejectedValue(new Error('mongo unreachable'));
+    incrementFileDeletionAttempts.mockRejectedValue(new Error('mongo unreachable'));
 
     const result = await sweepExpiredFiles(
       { appConfig: {} as AppConfig, limit: 2 },
-      { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger },
+      {
+        getExpiredFiles,
+        processDeleteRequest,
+        incrementFileDeletionAttempts,
+        deferExpiredFile,
+        logger,
+      },
     );
 
     expect(logger.error).toHaveBeenCalledWith(
@@ -170,6 +213,16 @@ describe('expired file sweep helpers', () => {
     expect(getExpiredFileRetryDelay(4)).toBe(8 * 60 * 60 * 1000);
     expect(getExpiredFileRetryDelay(64)).toBe(24 * 60 * 60 * 1000);
 
+    /* The base tracks the configured cadence rather than a fixed hour. */
+    process.env.FILE_RETENTION_SWEEP_INTERVAL_MS = String(5 * 60 * 1000);
+    expect(getExpiredFileRetryDelay(1)).toBe(5 * 60 * 1000);
+    expect(getExpiredFileRetryDelay(3)).toBe(20 * 60 * 1000);
+
+    /* ...but never drops below the floor that protects the give-up budget. */
+    process.env.FILE_RETENTION_SWEEP_INTERVAL_MS = '5';
+    expect(getExpiredFileRetryDelay(1)).toBe(60 * 1000);
+    delete process.env.FILE_RETENTION_SWEEP_INTERVAL_MS;
+
     expect(getFileRetentionMaxAttempts('0')).toBe(10);
     expect(getFileRetentionMaxAttempts('2.5')).toBe(10);
     expect(getFileRetentionMaxAttempts('   ')).toBe(10);

packages/api/src/files/sweep.ts

@@ -8,7 +8,7 @@ import type { AppConfig } from '@librechat/data-schemas';
 
 const DEFAULT_FILE_RETENTION_SWEEP_INTERVAL_MS = 60 * 60 * 1000;
 const DEFAULT_FILE_RETENTION_MAX_ATTEMPTS = 10;
-const FILE_RETENTION_RETRY_BASE_MS = 60 * 60 * 1000;
+const MIN_FILE_RETENTION_RETRY_BASE_MS = 60 * 1000;
 const FILE_RETENTION_RETRY_MAX_MS = 24 * 60 * 60 * 1000;
 
 type ExpiredFile = {
@@ -16,7 +16,6 @@ type ExpiredFile = {
   source?: string;
   user?: string | { toString?: () => string };
   tenantId?: string;
-  deletionAttempts?: number;
 };
 
 type SweepRequest = {
@@ -58,6 +57,7 @@ type SweepDependencies = {
     req: SweepRequest;
     files: ExpiredFile[];
   }) => Promise<{ deletedFileIds: string[]; failedFileIds: string[] }>;
+  incrementFileDeletionAttempts: (file_id: string) => Promise<number>;
   deferExpiredFile: (file_id: string, deletionRetryAt: Date) => Promise<void>;
   logger: SweepLogger;
 };
@@ -119,10 +119,21 @@ export function getFileRetentionMaxAttempts(
  * Backoff before the next attempt, doubling from one sweep interval up to a
  * day. `attempts` is the file's failure count including the one just
  * recorded, so the first retry lands on the following sweep.
+ *
+ * The base tracks the configured interval so the schedule stays meaningful
+ * at any cadence — a fixed hour would be inert for the first three attempts
+ * of a six-hour sweep, and would skip twelve passes of a five-minute one.
+ * The floor keeps a pathologically short interval from spending the whole
+ * give-up budget on a transient outage.
  */
 export function getExpiredFileRetryDelay(attempts: number): number {
+  const interval = getFileRetentionSweepInterval();
+  const base = Math.max(
+    interval > 0 ? interval : DEFAULT_FILE_RETENTION_SWEEP_INTERVAL_MS,
+    MIN_FILE_RETENTION_RETRY_BASE_MS,
+  );
   const exponent = Math.max(0, attempts - 1);
-  return Math.min(FILE_RETENTION_RETRY_BASE_MS * 2 ** exponent, FILE_RETENTION_RETRY_MAX_MS);
+  return Math.min(base * 2 ** exponent, FILE_RETENTION_RETRY_MAX_MS);
 }
 
 export function getExpiredFileEndpoint(source?: string): string {
@@ -244,7 +255,13 @@ export async function resolveExpiredFileSweepConfig({
 
 export async function sweepExpiredFiles(
   { appConfig, limit = 100, loadAppConfig }: ExpiredFileSweepOptions | undefined = {},
-  { getExpiredFiles, processDeleteRequest, deferExpiredFile, logger }: SweepDependencies,
+  {
+    getExpiredFiles,
+    processDeleteRequest,
+    incrementFileDeletionAttempts,
+    deferExpiredFile,
+    logger,
+  }: SweepDependencies,
 ): Promise<ExpiredFileSweepResult> {
   const maxAttempts = getFileRetentionMaxAttempts();
   const files = (await getExpiredFiles(limit, { maxAttempts })) ?? [];
@@ -258,10 +275,17 @@ export async function sweepExpiredFiles(
    * expired after it for the life of the deployment. */
   const recordFailure = async (file: ExpiredFile): Promise<void> => {
     failed++;
-    const attempts = (file.deletionAttempts ?? 0) + 1;
-    const retryAt = new Date(Date.now() + getExpiredFileRetryDelay(attempts));
+    let attempts: number;
     try {
-      await deferExpiredFile(file.file_id, retryAt);
+      /* The attempt number comes from the increment rather than from the
+       * snapshot this batch queried, so two nodes sweeping the same file
+       * each get a distinct one and exactly one of them observes the cap
+       * being reached. */
+      attempts = await incrementFileDeletionAttempts(file.file_id);
+      await deferExpiredFile(
+        file.file_id,
+        new Date(Date.now() + getExpiredFileRetryDelay(attempts)),
+      );
     } catch (error) {
       logger.error(
         `[sweepExpiredFiles] Error recording failed deletion of expired file ${file.file_id}:`,

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

@@ -471,15 +471,11 @@ describe('File Methods', () => {
   });
 
   describe('deferExpiredFile', () => {
-    it('increments the attempt count and stamps the next retry', async () => {
-      const userId = new mongoose.Types.ObjectId();
-      const fileId = uuidv4();
-      const retryAt = new Date('2030-02-01T00:00:00.000Z');
-
+    const createDeferrableFile = async (file_id: string) => {
       await fileMethods.createFile(
         {
-          file_id: fileId,
-          user: userId,
+          file_id,
+          user: new mongoose.Types.ObjectId(),
           filename: 'deferred.txt',
           filepath: '/uploads/deferred.txt',
           type: 'text/plain',
@@ -488,14 +484,49 @@ describe('File Methods', () => {
         },
         true,
       );
+    };
 
-      await fileMethods.deferExpiredFile(fileId, retryAt);
+    it('stamps the next retry and counts the attempt', async () => {
+      const fileId = uuidv4();
+      const retryAt = new Date('2030-02-01T00:00:00.000Z');
+      await createDeferrableFile(fileId);
+
+      expect(await fileMethods.incrementFileDeletionAttempts(fileId)).toBe(1);
       await fileMethods.deferExpiredFile(fileId, retryAt);
 
       const [file] = (await fileMethods.getFiles({ file_id: fileId }))!;
-      expect(file.deletionAttempts).toBe(2);
+      expect(file.deletionAttempts).toBe(1);
       expect(file.deletionRetryAt).toEqual(retryAt);
     });
+
+    it('hands concurrent sweeps distinct attempt numbers', async () => {
+      const fileId = uuidv4();
+      await createDeferrableFile(fileId);
+
+      /* Two nodes recording a failure for the same file in the same pass.
+       * Deriving the attempt from the queried snapshot would give both the
+       * same number and neither would see the cap being reached. */
+      const attempts = await Promise.all([
+        fileMethods.incrementFileDeletionAttempts(fileId),
+        fileMethods.incrementFileDeletionAttempts(fileId),
+        fileMethods.incrementFileDeletionAttempts(fileId),
+      ]);
+
+      expect([...attempts].sort()).toEqual([1, 2, 3]);
+    });
+
+    it('never pulls a deferral earlier than one already committed', async () => {
+      const fileId = uuidv4();
+      const later = new Date('2030-02-01T00:00:00.000Z');
+      const earlier = new Date('2030-01-01T00:00:00.000Z');
+      await createDeferrableFile(fileId);
+
+      await fileMethods.deferExpiredFile(fileId, later);
+      await fileMethods.deferExpiredFile(fileId, earlier);
+
+      const [file] = (await fileMethods.getFiles({ file_id: fileId }))!;
+      expect(file.deletionRetryAt).toEqual(later);
+    });
   });
 
   describe('getToolFilesByIds', () => {

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

@@ -42,6 +42,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
     selectFields?: Record<string, 0 | 1> | string | null,
   ) => Promise<IMongoFile[] | null>;
   getExpiredFiles: (limit?: number, options?: ExpiredFileQueryOptions) => Promise<IMongoFile[]>;
+  incrementFileDeletionAttempts: (file_id: string) => Promise<number>;
   deferExpiredFile: (file_id: string, deletionRetryAt: Date) => Promise<void>;
   getToolFilesByIds: (
     fileIds: string[],
@@ -169,17 +170,37 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
   }
 
   /**
-   * Records a failed sweep deletion and holds the file back until
-   * `deletionRetryAt`. The counter is incremented server-side so
-   * concurrent sweeps on separate nodes cannot overwrite each other's
-   * progress toward the give-up cap.
+   * Records one failed sweep deletion and returns the file's resulting
+   * consecutive-failure count.
+   *
+   * The count comes back from the increment itself rather than being
+   * re-derived from the caller's snapshot. Two nodes sweeping the same file
+   * would otherwise each read the same value and each believe itself to be
+   * the same attempt, pushing the stored count past the give-up cap while
+   * both still think it is below — so the give-up would never be reported.
+   * Returning it here gives every caller a distinct attempt number.
+   */
+  async function incrementFileDeletionAttempts(file_id: string): Promise<number> {
+    const File = mongoose.models.File as Model<IMongoFile>;
+    const file = await File.findOneAndUpdate(
+      { file_id },
+      { $inc: { deletionAttempts: 1 } },
+      { new: true, projection: { deletionAttempts: 1 } },
+    ).lean<Pick<IMongoFile, 'deletionAttempts'> | null>();
+
+    return file?.deletionAttempts ?? 0;
+  }
+
+  /**
+   * Holds a file back from the sweep until `deletionRetryAt`.
+   *
+   * Written with `$max` so a deferral can only ever move later. A node that
+   * computed a shorter backoff from a lower attempt count cannot pull the
+   * file forward past a longer one another node already committed.
    */
   async function deferExpiredFile(file_id: string, deletionRetryAt: Date): Promise<void> {
     const File = mongoose.models.File as Model<IMongoFile>;
-    await File.updateOne(
-      { file_id },
-      { $inc: { deletionAttempts: 1 }, $set: { deletionRetryAt } },
-    ).exec();
+    await File.updateOne({ file_id }, { $max: { deletionRetryAt } }).exec();
   }
 
   /**
@@ -723,6 +744,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
     findFileById,
     getFiles,
     getExpiredFiles,
+    incrementFileDeletionAttempts,
     deferExpiredFile,
     getToolFilesByIds,
     getCodeGeneratedFiles,