fix(schedules): repair the deletion CLI and order restore before the fence release

Addresses three findings from the fresh Codex review.

P1 — config/delete-user.js called methods.disableUserSchedulesForDeletion, which
Wave B removed in favor of suspendUserSchedulesForDeletion. The file is
@ts-nocheck and its spec mocked the removed name, so neither typecheck nor tests
caught it; the real CLI would throw a TypeError before deleting anything and then
only unwind the fence. The CLI now uses the tokenized protocol: it mints a
suspension token, suspends with it, and restores that exact attempt's rows in its
finally block when the deletion does not commit. Its spec mocks the real methods,
so the breakage can no longer hide.

P2 — both the HTTP controller and the CLI released the user-deletion fence BEFORE
restoring schedules. That fence is what refuses new schedule writes/claims, so the
gap let an owner PATCH edit a still-suspended row and have its enabled/next-run
state overwritten by the older snapshot, and let a second deletion attempt
re-suspend under a new token — making the first restore a no-op and stranding the
disabled snapshot permanently. Restore now runs first, while writes are still
fenced.

Hardening for the same defect class across a crash: suspendUserSchedulesForDeletion
now ADOPTS an existing suspension's snapshot when re-suspending a row abandoned by
an earlier attempt, instead of re-capturing the row's current (already-suspended)
state. Without this, an attempt that died before restoring would have its
successor snapshot "disabled, no next run" and permanently strand the schedule.

Tests: CLI restore-before-fence ordering and no-restore-on-success; the same
ordering assertion on both controller post-quiesce failure paths; a data-method
regression that a second attempt adopts the abandoned snapshot and restores the
original enabled/next-run state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq

api/server/controllers/UserController.js

@@ -470,6 +470,27 @@ const deleteUserController = async (req, res) => {
     logger.info(`User deleted account. Email: ${user.email} ID: ${user.id}`);
     res.status(200).send({ message: 'User deleted' });
   } catch (err) {
+    // The account survives this failed attempt, so its schedules must too: restore the
+    // exact rows this attempt suspended (re-enabled/re-armed from their snapshot). Fenced
+    // to the token, so a schedule the owner deleted meanwhile is not resurrected. A
+    // successful deletion never reaches here (userDeleted short-circuits it).
+    //
+    // RESTORE BEFORE RELEASING THE DELETION FENCE. That fence is what refuses new schedule
+    // writes/claims for this user; releasing it first opens a window where an owner PATCH
+    // could edit a still-suspended row and then have its enabled/next-run state overwritten
+    // by this older snapshot, and where a second deletion attempt could re-suspend these
+    // rows under a new token — making this restore a no-op and stranding the disabled
+    // snapshot permanently.
+    if (scheduleSuspensionToken != null && !userDeleted) {
+      try {
+        await restoreUserSchedulesFromDeletion(user.id, scheduleSuspensionToken);
+      } catch (restoreError) {
+        logger.error(
+          '[deleteUserController] Failed to restore suspended schedules after a cancelled deletion',
+          restoreError,
+        );
+      }
+    }
     if (triggerDeletionFence != null && !userDeleted) {
       try {
         await cancelAgentTriggerUserPurge(user.id, triggerDeletionFence);
@@ -485,20 +506,6 @@ const deleteUserController = async (req, res) => {
         logger.error('[deleteUserController] Failed to release trigger deletion fence', fenceError);
       }
     }
-    // The account survives this failed attempt, so its schedules must too: restore the
-    // exact rows this attempt suspended (re-enabled/re-armed from their snapshot). Fenced
-    // to the token, so a schedule the owner deleted meanwhile is not resurrected. A
-    // successful deletion never reaches here (userDeleted short-circuits it).
-    if (scheduleSuspensionToken != null && !userDeleted) {
-      try {
-        await restoreUserSchedulesFromDeletion(user.id, scheduleSuspensionToken);
-      } catch (restoreError) {
-        logger.error(
-          '[deleteUserController] Failed to restore suspended schedules after a cancelled deletion',
-          restoreError,
-        );
-      }
-    }
     logger.error('[deleteUserController]', err);
     return res.status(500).json({ message: 'Something went wrong.' });
   }

api/server/controllers/UserController.spec.js

@@ -449,6 +449,12 @@ describe('deleteUserController', () => {
       userIdString,
       mockQuiesceUserSchedules.mock.calls[0][1],
     );
+    // BEFORE the deletion fence is released: that fence is what refuses new schedule writes,
+    // so restoring after it would let an owner PATCH — or a second deletion attempt
+    // re-suspending under a new token — race the restore and strand the disabled snapshot.
+    expect(mockRestoreUserSchedules.mock.invocationCallOrder[0]).toBeLessThan(
+      cancelAgentTriggerUserDeletion.mock.invocationCallOrder[0],
+    );
   });
 
   it('fails closed before data cleanup when detached subagents do not drain', async () => {
@@ -501,6 +507,12 @@ describe('deleteUserController', () => {
       userIdString,
       mockQuiesceUserSchedules.mock.calls[0][1],
     );
+    // BEFORE the deletion fence is released: that fence is what refuses new schedule writes,
+    // so restoring after it would let an owner PATCH — or a second deletion attempt
+    // re-suspending under a new token — race the restore and strand the disabled snapshot.
+    expect(mockRestoreUserSchedules.mock.invocationCallOrder[0]).toBeLessThan(
+      cancelAgentTriggerUserDeletion.mock.invocationCallOrder[0],
+    );
   });
 
   it('should remove the user from all groups via $pullAll', async () => {

config/__tests__/delete-user.spec.js

@@ -14,7 +14,8 @@ const mockMethods = {
   beginAgentTriggerUserDeletion: jest.fn(),
   recoverStaleAgentTriggerUserDeletion: jest.fn(),
   prepareAgentTriggerUserPurge: jest.fn(),
-  disableUserSchedulesForDeletion: jest.fn(),
+  suspendUserSchedulesForDeletion: jest.fn(),
+  restoreUserSchedulesFromDeletion: jest.fn(),
   countActiveAgentTriggerDeliveriesByUser: jest.fn(),
   deleteSchedulesByUser: jest.fn(),
   deleteUserById: jest.fn(),
@@ -91,7 +92,8 @@ describe('Delete user CLI', () => {
     mockMethods.beginAgentTriggerUserDeletion.mockReset().mockResolvedValue('acquired');
     mockMethods.recoverStaleAgentTriggerUserDeletion.mockReset().mockResolvedValue('acquired');
     mockMethods.prepareAgentTriggerUserPurge.mockReset().mockResolvedValue(undefined);
-    mockMethods.disableUserSchedulesForDeletion.mockReset().mockResolvedValue(undefined);
+    mockMethods.suspendUserSchedulesForDeletion.mockReset().mockResolvedValue(undefined);
+    mockMethods.restoreUserSchedulesFromDeletion.mockReset().mockResolvedValue(undefined);
     mockMethods.countActiveAgentTriggerDeliveriesByUser.mockReset().mockResolvedValue(0);
     mockMethods.deleteSchedulesByUser.mockReset().mockResolvedValue(undefined);
     mockMethods.deleteUserById.mockReset().mockResolvedValue({ deletedCount: 1 });
@@ -122,14 +124,17 @@ describe('Delete user CLI', () => {
       expect.any(Date),
       'tenant-1',
     );
-    expect(mockMethods.disableUserSchedulesForDeletion).toHaveBeenCalledWith(USER_ID);
+    expect(mockMethods.suspendUserSchedulesForDeletion).toHaveBeenCalledWith(
+      USER_ID,
+      expect.any(String),
+    );
     expect(mockMethods.beginAgentTriggerUserDeletion.mock.invocationCallOrder[0]).toBeLessThan(
       mockMethods.prepareAgentTriggerUserPurge.mock.invocationCallOrder[0],
     );
     expect(mockMethods.prepareAgentTriggerUserPurge.mock.invocationCallOrder[0]).toBeLessThan(
-      mockMethods.disableUserSchedulesForDeletion.mock.invocationCallOrder[0],
+      mockMethods.suspendUserSchedulesForDeletion.mock.invocationCallOrder[0],
     );
-    expect(mockMethods.disableUserSchedulesForDeletion.mock.invocationCallOrder[0]).toBeLessThan(
+    expect(mockMethods.suspendUserSchedulesForDeletion.mock.invocationCallOrder[0]).toBeLessThan(
       mockMethods.countActiveAgentTriggerDeliveriesByUser.mock.invocationCallOrder[0],
     );
     expect(
@@ -154,7 +159,7 @@ describe('Delete user CLI', () => {
   });
 
   it('deletes nothing and releases both fences when schedule quiescing fails', async () => {
-    mockMethods.disableUserSchedulesForDeletion.mockRejectedValueOnce(
+    mockMethods.suspendUserSchedulesForDeletion.mockRejectedValueOnce(
       new Error('schedule write failed'),
     );
 
@@ -168,6 +173,33 @@ describe('Delete user CLI', () => {
     expect(mockMethods.cancelAgentTriggerUserDeletion).toHaveBeenCalledWith(USER_ID, deletionFence);
   });
 
+  /**
+   * The suspension is reversible, so a CLI attempt that does not commit must restore the
+   * rows it suspended — and must do so while the user-deletion fence still refuses new
+   * schedule writes, or an owner edit (or a second attempt re-suspending under a new token)
+   * can race the restore and strand the disabled snapshot.
+   */
+  it('restores suspended schedules BEFORE releasing the deletion fence on failure', async () => {
+    mockGetCleanupBlockingJobIdsForUser.mockResolvedValueOnce(['stream-1']);
+    mockAbortJob.mockRejectedValueOnce(new Error('provider drain failed'));
+
+    expect(await runCli()).toBe(1);
+
+    const token = mockMethods.suspendUserSchedulesForDeletion.mock.calls[0][1];
+    expect(token).toEqual(expect.any(String));
+    expect(mockMethods.restoreUserSchedulesFromDeletion).toHaveBeenCalledWith(USER_ID, token);
+    expect(mockMethods.restoreUserSchedulesFromDeletion.mock.invocationCallOrder[0]).toBeLessThan(
+      mockMethods.cancelAgentTriggerUserDeletion.mock.invocationCallOrder[0],
+    );
+  });
+
+  it('never restores schedules after a committed deletion', async () => {
+    expect(await runCli()).toBe(0);
+
+    expect(mockMethods.deleteSchedulesByUser).toHaveBeenCalledWith(USER_ID);
+    expect(mockMethods.restoreUserSchedulesFromDeletion).not.toHaveBeenCalled();
+  });
+
   it('deletes nothing and releases both fences when provider drain fails', async () => {
     mockGetCleanupBlockingJobIdsForUser.mockResolvedValueOnce(['stream-1']);
     mockAbortJob.mockRejectedValueOnce(new Error('provider drain failed'));

config/delete-user.js

@@ -3,6 +3,7 @@
 // @ts-nocheck
 const path = require('path');
 const mongoose = require('mongoose');
+const { randomUUID } = require('node:crypto');
 const { createModels, createMethods, runAsSystem } = require('@librechat/data-schemas');
 const {
   Key,
@@ -118,6 +119,7 @@ async function gracefulExit(code = 0) {
   }
 
   let deletionFence;
+  let scheduleSuspensionToken;
   let userDeleted = false;
 
   try {
@@ -158,7 +160,13 @@ async function gracefulExit(code = 0) {
       await runAsSystem(() =>
         methods.prepareAgentTriggerUserPurge(uid, deletionFence, user.tenantId),
       );
-      await runAsSystem(() => methods.disableUserSchedulesForDeletion(uid));
+      // Reversible, token-fenced suspension (the same protocol the HTTP controller uses):
+      // an attempt that does not commit restores exactly these rows in the finally block,
+      // rather than leaving a surviving account with disabled, erasure-eligible schedules.
+      scheduleSuspensionToken = randomUUID();
+      await runAsSystem(() =>
+        methods.suspendUserSchedulesForDeletion(uid, scheduleSuspensionToken),
+      );
       if (hasSharedGenerationStore) {
         const deadline = Date.now() + TRIGGER_DRAIN_TIMEOUT_MS;
         while (
@@ -228,6 +236,14 @@ async function gracefulExit(code = 0) {
     userDeleted = true;
     await runAsSystem(() => methods.deleteAgentTriggerDeliveriesByUser(uid));
   } finally {
+    // RESTORE BEFORE RELEASING THE FENCE. While the user-deletion fence is still armed, new
+    // schedule writes/claims are refused, so this restore cannot race an owner PATCH nor be
+    // superseded by a second deletion attempt re-suspending these rows under a new token.
+    if (scheduleSuspensionToken != null && !userDeleted) {
+      await runAsSystem(() =>
+        methods.restoreUserSchedulesFromDeletion(uid, scheduleSuspensionToken),
+      ).catch((error) => console.error('Failed to restore suspended schedules:', error));
+    }
     if (deletionFence != null && !userDeleted) {
       await runAsSystem(() => methods.cancelAgentTriggerUserPurge(uid, deletionFence)).catch(
         (error) => console.error('Failed to disarm trigger purge recovery:', error),

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

@@ -1143,6 +1143,33 @@ describe('reversible account-deletion suspension', () => {
     expect(after.deletionSuspension?.nextRunAt?.getTime()).toBe(armedAt.getTime());
   });
 
+  /**
+   * A crash between suspend and restore leaves an ABANDONED suspension. A later attempt
+   * must adopt that attempt's snapshot rather than re-snapshotting the row's current
+   * (already-suspended) state — otherwise the true pre-suspension state is lost and the
+   * schedule stays disabled forever once the second attempt restores.
+   */
+  it('a second attempt adopts the abandoned snapshot instead of capturing suspended state', async () => {
+    const armedAt = new Date('2030-01-01T00:00:00Z');
+    const schedule = await methods.createSchedule(scheduleData({ nextRunAt: armedAt }));
+    await methods.suspendUserSchedulesForDeletion(schedule.user, 'attempt-1');
+
+    // attempt-1's process died without restoring; attempt-2 now suspends the same rows.
+    await methods.suspendUserSchedulesForDeletion(schedule.user, 'attempt-2');
+
+    const suspended = await getSchedule(schedule.id);
+    expect(suspended.deletionSuspension?.token).toBe('attempt-2');
+    // The ORIGINAL pre-suspension state, not the suspended state attempt-2 observed.
+    expect(suspended.deletionSuspension?.enabled).toBe(true);
+    expect(suspended.deletionSuspension?.nextRunAt?.getTime()).toBe(armedAt.getTime());
+
+    await methods.restoreUserSchedulesFromDeletion(schedule.user, 'attempt-2');
+    const restored = await getSchedule(schedule.id);
+    expect(restored.enabled).toBe(true);
+    expect(restored.nextRunAt?.getTime()).toBe(armedAt.getTime());
+    expect(restored.deletionSuspension).toBeUndefined();
+  });
+
   it('a successful deletion removes suspended rows and their snapshots', async () => {
     const userId = new mongoose.Types.ObjectId();
     const schedule = await methods.createSchedule(scheduleData({ user: userId }));

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

@@ -1672,33 +1672,44 @@ export function createScheduleMethods(mongoose: typeof import('mongoose')): Sche
         deleting: { $ne: true },
         'deletionSuspension.token': { $ne: token },
       })
-      .select('id enabled nextRunAt')
-      .lean<Array<Pick<ISchedule, 'id' | 'enabled' | 'nextRunAt'>>>();
+      .select('id enabled nextRunAt deletionSuspension')
+      .lean<Array<Pick<ISchedule, 'id' | 'enabled' | 'nextRunAt' | 'deletionSuspension'>>>();
     if (schedules.length === 0) {
       return;
     }
-    const ops = schedules.map((schedule) => ({
-      updateOne: {
-        filter: {
-          id: schedule.id,
-          user: userId,
-          deleting: { $ne: true },
-          'deletionSuspension.token': { $ne: token },
-        },
-        update: {
-          $set: {
-            enabled: false,
-            claimToken: randomUUID(),
-            deletionSuspension: {
-              token,
-              enabled: schedule.enabled === true,
-              ...(schedule.nextRunAt != null ? { nextRunAt: schedule.nextRunAt } : {}),
-            },
+    const ops = schedules.map((schedule) => {
+      // A row already suspended by an ABANDONED earlier attempt (its process died before
+      // restoring) still holds the only record of the true pre-suspension state. Carry that
+      // snapshot forward and merely take ownership with this attempt's token — re-reading
+      // the row's CURRENT values would snapshot the suspended state itself (disabled, no
+      // next run) and permanently strand the schedule when this attempt later restores.
+      const existing = schedule.deletionSuspension;
+      const snapshotEnabled =
+        existing != null ? existing.enabled === true : schedule.enabled === true;
+      const snapshotNextRunAt = existing != null ? existing.nextRunAt : schedule.nextRunAt;
+      return {
+        updateOne: {
+          filter: {
+            id: schedule.id,
+            user: userId,
+            deleting: { $ne: true },
+            'deletionSuspension.token': { $ne: token },
+          },
+          update: {
+            $set: {
+              enabled: false,
+              claimToken: randomUUID(),
+              deletionSuspension: {
+                token,
+                enabled: snapshotEnabled,
+                ...(snapshotNextRunAt != null ? { nextRunAt: snapshotNextRunAt } : {}),
+              },
+            },
+            $unset: { nextRunAt: 1 as const },
           },
-          $unset: { nextRunAt: 1 as const },
         },
-      },
-    }));
+      };
+    });
     await tenantSafeBulkWrite(Schedule(), ops as AnyBulkWriteOperation[], { ordered: false });
   }