fix: raise the CLI barrier through markUserDeleting, gate its deletes, reject a lost arming race

- delete-user.js raised the barrier with a raw update, which stamps
  deletionRequestedAt without dropping the cached auth user document, so a live
  server kept admitting requests with a pre-barrier req.user while the cascade ran.
- The deletion task list dispatched every deleteMany the moment it was built,
  above the barrier and the active-run refusal, so both guards were decorative.
- A null arming result means the deletion cascade claimed the row; falling back
  to the pre-delete snapshot answered 201 for a schedule pending erasure.
- Retry guidance no longer says to stop the server, which cannot settle a
  persisted run row and would deadlock every offline retry.

config/__tests__/delete-user.spec.js

@@ -0,0 +1,124 @@
+const mockModelRegistry = {};
+const mockModelFor = (name) => {
+  if (!mockModelRegistry[name]) {
+    mockModelRegistry[name] = {
+      findOne: jest.fn(async () => null),
+      deleteOne: jest.fn(async () => ({ deletedCount: 1 })),
+      deleteMany: jest.fn(async () => ({ deletedCount: 0 })),
+      updateOne: jest.fn(async () => ({ modifiedCount: 0 })),
+      updateMany: jest.fn(async () => ({ modifiedCount: 0 })),
+      countDocuments: jest.fn(async () => 0),
+    };
+  }
+  return mockModelRegistry[name];
+};
+
+const mockMarkUserDeleting = jest.fn(async () => new Date());
+const mockSilentExit = jest.fn();
+const mockAskQuestion = jest.fn();
+
+jest.mock('../connect', () => jest.fn(async () => {}));
+jest.mock('mongoose', () => ({ disconnect: jest.fn(async () => {}) }));
+jest.mock('@librechat/data-schemas', () => ({
+  createModels: () => new Proxy({}, { get: (_target, prop) => mockModelFor(prop) }),
+}));
+jest.mock('~/models', () => ({ markUserDeleting: mockMarkUserDeleting }));
+jest.mock('../helpers', () => ({
+  ...jest.requireActual('../helpers'),
+  askQuestion: mockAskQuestion,
+  silentExit: mockSilentExit,
+}));
+
+const USER_ID = 'user-being-deleted';
+
+/** Runs the CLI to completion, resolving with the exit code it passed to silentExit. */
+const runCli = () =>
+  new Promise((resolve, reject) => {
+    mockSilentExit.mockImplementation((code = 0) => resolve(code));
+    jest.isolateModules(() => {
+      try {
+        require('../delete-user');
+      } catch (err) {
+        reject(err);
+      }
+    });
+  });
+
+describe('Delete user CLI', () => {
+  const originalArgv = process.argv;
+  let logSpy;
+
+  beforeEach(() => {
+    process.argv = ['node', 'delete-user.js', 'deleted@example.com'];
+    logSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
+    jest.spyOn(console, 'error').mockImplementation(() => {});
+    for (const model of Object.values(mockModelRegistry)) {
+      for (const fn of Object.values(model)) {
+        fn.mockClear();
+      }
+    }
+    mockModelFor('User').findOne.mockResolvedValue({
+      _id: { toString: () => USER_ID },
+      email: 'deleted@example.com',
+    });
+    mockModelFor('ScheduleRun').countDocuments.mockResolvedValue(0);
+    mockMarkUserDeleting.mockReset().mockResolvedValue(new Date());
+    // Confirm deletion, decline transaction history.
+    mockAskQuestion.mockReset().mockResolvedValueOnce('y').mockResolvedValueOnce('n');
+  });
+
+  afterEach(() => {
+    process.argv = originalArgv;
+    logSpy.mockRestore();
+    jest.restoreAllMocks();
+  });
+
+  it('raises the barrier through markUserDeleting, never a raw update', async () => {
+    const code = await runCli();
+
+    expect(code).toBe(0);
+    expect(mockMarkUserDeleting).toHaveBeenCalledWith(USER_ID);
+    // A raw User.updateOne would stamp deletionRequestedAt without dropping the cached
+    // auth user document, so a live server keeps admitting requests with a pre-barrier
+    // req.user while the cascade below runs.
+    expect(mockModelFor('User').updateOne).not.toHaveBeenCalled();
+    expect(mockModelFor('User').deleteOne).toHaveBeenCalled();
+  });
+
+  it('raises the barrier before counting active scheduled runs', async () => {
+    await runCli();
+
+    const barrierAt = mockMarkUserDeleting.mock.invocationCallOrder[0];
+    const countAt = mockModelFor('ScheduleRun').countDocuments.mock.invocationCallOrder[0];
+    // The count only means anything once new fires are refused; otherwise a fire can be
+    // claimed and accepted between a zero result and the deletes.
+    expect(barrierAt).toBeLessThan(countAt);
+  });
+
+  it('deletes nothing when the barrier cannot be raised', async () => {
+    mockMarkUserDeleting.mockRejectedValue(
+      new Error('Auth user-doc cache is enabled but unavailable'),
+    );
+
+    const code = await runCli();
+
+    expect(code).toBe(1);
+    expect(mockModelFor('User').deleteOne).not.toHaveBeenCalled();
+    expect(mockModelFor('Schedule').deleteMany).not.toHaveBeenCalled();
+    expect(mockModelFor('ScheduleRun').deleteMany).not.toHaveBeenCalled();
+    expect(mockModelFor('Message').deleteMany).not.toHaveBeenCalled();
+  });
+
+  it('refuses while a scheduled run is still active', async () => {
+    mockModelFor('ScheduleRun').countDocuments.mockResolvedValue(2);
+
+    const code = await runCli();
+
+    expect(code).toBe(1);
+    expect(mockModelFor('User').deleteOne).not.toHaveBeenCalled();
+    expect(mockModelFor('Schedule').deleteMany).not.toHaveBeenCalled();
+    // The barrier stays up: it is one-way, and a live server must keep refusing fires
+    // for an account the operator has already committed to erasing.
+    expect(mockMarkUserDeleting).toHaveBeenCalledWith(USER_ID);
+  });
+});

config/delete-user.js

@@ -31,6 +31,7 @@ const {
   ScheduleRun,
 } = require('@librechat/data-schemas').createModels(mongoose);
 require('module-alias')({ base: path.resolve(__dirname, '..', 'api') });
+const { markUserDeleting } = require('~/models');
 const { askQuestion, silentExit } = require('./helpers');
 const connect = require('./connect');
 
@@ -78,7 +79,62 @@ async function gracefulExit(code = 0) {
 
   const uid = user._id.toString();
 
-  // 5) Build and run deletion tasks
+  // Raise the durable deletion barrier BEFORE counting. A bare count is a
+  // time-of-check/time-of-use read: a fire can be claimed and accepted between the zero
+  // result and the deletes below, and this script cannot abort or drain it. The barrier
+  // is what makes the count meaningful — a live server refuses new fires at the dispatch
+  // boundary (fireSchedule's isOwnerDeleting probe) from this point on, so anything the
+  // count then misses cannot have started after it.
+  //
+  // Through markUserDeleting, never a raw update: the barrier is only in force once no
+  // CACHED pre-barrier user document can still populate req.user on a live server, and
+  // that method is what drops the auth cache entry. It fails closed, so a cache it
+  // cannot reach aborts the deletion rather than proceeding behind a barrier that was
+  // never actually raised.
+  try {
+    await markUserDeleting(uid);
+  } catch (err) {
+    console.red('✖ Could not raise the deletion barrier. Nothing was deleted.');
+    console.error(err);
+    return gracefulExit(1);
+  }
+
+  // REFUSE rather than warn when a scheduled run is in flight. This script talks to the
+  // database directly, so unlike the HTTP deletion paths it cannot abort a live loopback
+  // generation or wait for it to drain. That generation can already have passed its
+  // owner lookup, and it will persist its messages after the rows deleted here are gone
+  // — resurrecting data for an account the operator believes is erased.
+  const activeRuns = await ScheduleRun.countDocuments({
+    user: uid,
+    status: { $in: ['started', 'requires_action'] },
+  });
+  if (activeRuns > 0) {
+    console.red(
+      `✖ ${activeRuns} scheduled run(s) are still active for this user, and this script cannot abort them.`,
+    );
+    // Deliberately NOT "stop the server and retry": stopping it cannot transition a
+    // persisted row, so every offline retry would see the same active status forever.
+    // Only a running server settles these — by draining them (app deletion) or by
+    // reconciling an abandoned run whose lease expired.
+    console.yellow(
+      'Delete the account through the app instead, which drains active runs. If the server is',
+    );
+    console.yellow(
+      'already stopped, start it and let reconciliation settle the abandoned run, then retry.',
+    );
+    return gracefulExit(1);
+  }
+
+  // Runs BEFORE schedules so a partial failure stays retryable, mirroring
+  // deleteSchedulesByUser. A schedule carries the user's prompt text and has no TTL,
+  // so leaving it behind retains that content indefinitely.
+  await ScheduleRun.deleteMany({ user: uid });
+  await Schedule.deleteMany({ user: uid });
+
+  // 5) Run the deletion tasks. Constructed HERE rather than earlier: a Model.deleteMany()
+  // call dispatches the moment it is written, so building this list above the guards
+  // would start erasing the account before the barrier could be raised or an in-flight
+  // scheduled run could refuse the whole operation.
   const tasks = [
     Action.deleteMany({ user: uid }),
     Agent.deleteMany({ author: uid }),
@@ -102,42 +158,6 @@ async function gracefulExit(code = 0) {
     AclEntry.deleteMany({ principalId: user._id }),
   ];
 
-  // Raise the durable deletion barrier BEFORE counting. A bare count is a
-  // time-of-check/time-of-use read: a fire can be claimed and accepted between the zero
-  // result and the deletes below, and this script cannot abort or drain it. The barrier
-  // is what makes the count meaningful — a live server refuses new fires at the dispatch
-  // boundary (fireSchedule's isOwnerDeleting probe) from this point on, so anything the
-  // count then misses cannot have started after it.
-  await User.updateOne(
-    { _id: uid, deletionRequestedAt: { $exists: false } },
-    { $set: { deletionRequestedAt: new Date() } },
-  );
-
-  // REFUSE rather than warn when a scheduled run is in flight. This script talks to the
-  // database directly, so unlike the HTTP deletion paths it cannot abort a live loopback
-  // generation or wait for it to drain. That generation can already have passed its
-  // owner lookup, and it will persist its messages after the rows deleted here are gone
-  // — resurrecting data for an account the operator believes is erased.
-  const activeRuns = await ScheduleRun.countDocuments({
-    user: uid,
-    status: { $in: ['started', 'requires_action'] },
-  });
-  if (activeRuns > 0) {
-    console.red(
-      `✖ ${activeRuns} scheduled run(s) are still active for this user, and this script cannot abort them.`,
-    );
-    console.yellow(
-      'Stop the server (or delete the account through the app, which drains them) and retry.',
-    );
-    return gracefulExit(1);
-  }
-
-  // Runs BEFORE schedules so a partial failure stays retryable, mirroring
-  // deleteSchedulesByUser. A schedule carries the user's prompt text and has no TTL,
-  // so leaving it behind retains that content indefinitely.
-  await ScheduleRun.deleteMany({ user: uid });
-  await Schedule.deleteMany({ user: uid });
-
   if (deleteTx) {
     tasks.push(Transaction.deleteMany({ user: uid }));
   }

packages/api/src/schedules/handlers.spec.ts

@@ -219,4 +219,20 @@ describe('createSchedule late-create compensation', () => {
       expect.objectContaining({ nextRunAt: expect.any(Date) }),
     );
   });
+
+  /**
+   * updateScheduleById filters out rows marked `deleting`, so a null arming result means
+   * the deletion cascade claimed this row between the barrier re-check and the arming
+   * write. Reporting the pre-delete snapshot as a 201 tells the client a schedule exists
+   * that is already hidden and pending erasure.
+   */
+  it('does not report success when the arming write loses a delete race', async () => {
+    const deps = makeCreateDeps({ isUserDeleting: jest.fn(async () => false) });
+    (deps.methods.updateScheduleById as jest.Mock).mockResolvedValue(null);
+    const { res, captured } = makeRes();
+    await createSchedulesHandlers(deps).createSchedule(makeCreateReq(), res);
+
+    expect(captured.status).toBe(410);
+    expect(captured.status).not.toBe(201);
+  });
 });

packages/api/src/schedules/handlers.ts

@@ -297,11 +297,22 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH
       res.status(410).json({ error: 'This account is being deleted' });
       return;
     }
-    // ARM last. A failure here leaves the schedule visible but unclaimed; the owner's
+    // ARM last. A transient write fault THROWS, leaving the schedule visible but
-    // next edit recomputes nextRunAt.
+    // unclaimed; the owner's next edit (or the reconciler's unarmed sweep) recomputes
-    const armed = nextRunAt
+    // nextRunAt. A null result is different in kind: updateScheduleById filters out
-      ? ((await deps.methods.updateScheduleById(id, user.id, { nextRunAt })) ?? created)
+    // deleting rows, so null means the row stopped being ours between the barrier
-      : created;
+    // re-check and here — the deletion cascade marked or erased it. Falling back to the
+    // pre-delete snapshot would answer 201 for a schedule that is already hidden and
+    // pending erasure.
+    let armed = created;
+    if (nextRunAt) {
+      const updated = await deps.methods.updateScheduleById(id, user.id, { nextRunAt });
+      if (updated == null) {
+        res.status(410).json({ error: 'Schedule no longer exists' });
+        return;
+      }
+      armed = updated;
+    }
     logger.info(`[schedules] created ${id} for user ${user.id}`);
     res.status(201).json(toWireSchedule(armed));
   }