Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,11 @@ HELP_AND_FAQ_URL=https://librechat.ai
# Enable Redis for resumable LLM streams (defaults to USE_REDIS value if not set)
# Set to false to use in-memory storage for streams while keeping Redis for other caches
# USE_REDIS_STREAMS=true
# Scheduled chats require shared Redis streams in multi-replica deployments.
# Set this only when the deployment truly runs one LibreChat process without Redis.
# SCHEDULES_SINGLE_PROCESS=true
# Emergency global stop for both automatic and manual scheduled runs.
# SCHEDULES_DISABLED=true

# Generation stream wire/state protocol. Redis-backed deployments default to the
# rolling-upgrade-safe v1 protocol when this is unset; in-memory deployments use v2.
Expand Down
2 changes: 1 addition & 1 deletion api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"@azure/storage-blob": "^12.30.0",
"@google/genai": "^2.8.0",
"@keyv/redis": "^4.3.3",
"@librechat/agents": "^3.6.8",
"@librechat/agents": "^3.6.9",
"@librechat/api": "*",
"@librechat/data-schemas": "*",
"@microsoft/microsoft-graph-client": "^3.0.7",
Expand Down
42 changes: 42 additions & 0 deletions api/server/controllers/UserController.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const {
purgeAgentTriggerDeliveriesForUser,
} = require('~/server/services/Agents/triggers');
const { getAppConfig } = require('~/server/services/Config');
const { randomUUID } = require('node:crypto');
const {
quiesceUserSchedules,
restoreUserSchedulesFromDeletion,
} = require('~/server/services/Schedules');
const { getLogStores } = require('~/cache');
const db = require('~/models');

Expand Down Expand Up @@ -360,6 +365,7 @@ const updateUserPluginsController = async (req, res) => {
const deleteUserController = async (req, res) => {
const { user } = req;
let triggerDeletionFence;
let scheduleSuspensionToken;
let userDeleted = false;

try {
Expand Down Expand Up @@ -395,6 +401,14 @@ const deleteUserController = async (req, res) => {
}
await drainAgentTriggerDeliveriesForUser(user.id);
await subagentThreadTaskStore.cancelAndDrainForOwner(user.id, user.tenantId);
// Reversibly suspend the user's schedules under a per-attempt token BEFORE draining.
// A later cascade step (or this drain) can still fail and cancel the deletion, and the
// catch below restores exactly this attempt's rows — so a failed deletion never leaves
// a live user with silently disabled, erasure-eligible schedules.
scheduleSuspensionToken = randomUUID();
if (!(await quiesceUserSchedules(user.id, scheduleSuspensionToken))) {
throw new Error('Scheduled executions could not be confirmed stopped');
}
const activeAgentRuns = await GenerationJobManager.getCleanupBlockingJobIdsForUser(
user.id,
user.tenantId,
Expand Down Expand Up @@ -446,6 +460,7 @@ const deleteUserController = async (req, res) => {
await db.deleteTokens({ userId: user.id });
await db.removeUserFromAllGroups(user.id);
await db.deleteAclEntries({ principalId: user._id });
await db.deleteSchedulesByUser(user.id);
const deleteResult = await db.deleteUserById(user.id);
if (deleteResult.deletedCount !== 1) {
throw new Error('User disappeared before account deletion could commit');
Expand All @@ -455,6 +470,33 @@ 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) {
// Every retry is exhausted at this point. The fence is still released below on
// purpose: retaining it would refuse this live account's schedule writes AND make
// `beginAgentTriggerUserDeletion` report `in_progress` forever, blocking the retry
// that is the convergence path — a later attempt re-suspends by ADOPTING this
// snapshot, so its cancel restores these exact rows. Log the token so the state is
// recoverable directly if that never happens.
logger.error(
`[deleteUserController] Failed to restore suspended schedules after a cancelled deletion; they remain disabled for user ${user.id} under suspension token ${scheduleSuspensionToken}`,
restoreError,
);
}
}
if (triggerDeletionFence != null && !userDeleted) {
try {
await cancelAgentTriggerUserPurge(user.id, triggerDeletionFence);
Expand Down
62 changes: 62 additions & 0 deletions api/server/controllers/UserController.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const mockPrepareAgentTriggerUserPurge = jest.fn().mockResolvedValue(undefined);
const mockCancelAgentTriggerUserPurge = jest.fn().mockResolvedValue(true);
const mockPurgeAgentTriggerDeliveriesForUser = jest.fn().mockResolvedValue(undefined);
const mockCancelAndDrainSubagentThreads = jest.fn().mockResolvedValue(undefined);
const mockQuiesceUserSchedules = jest.fn().mockResolvedValue(true);
const mockRestoreUserSchedules = jest.fn().mockResolvedValue(undefined);

jest.mock('@librechat/data-schemas', () => {
const actual = jest.requireActual('@librechat/data-schemas');
Expand All @@ -30,6 +32,7 @@ jest.mock('~/models', () => {
deleteAllAgentApiKeys: jest.fn().mockResolvedValue(undefined),
deleteConversationTags: jest.fn().mockResolvedValue(undefined),
deleteAllUserMemories: jest.fn().mockResolvedValue(undefined),
deleteSchedulesByUser: jest.fn().mockResolvedValue(undefined),
deleteTransactions: jest.fn().mockResolvedValue(undefined),
deleteAclEntries: jest.fn().mockResolvedValue(undefined),
updateUserPlugins: jest.fn(),
Expand Down Expand Up @@ -100,6 +103,11 @@ jest.mock('~/server/services/Endpoints/agents/subagentThreadStore', () => ({
cancelAndDrainForOwner: (...args) => mockCancelAndDrainSubagentThreads(...args),
}));

jest.mock('~/server/services/Schedules', () => ({
quiesceUserSchedules: (...args) => mockQuiesceUserSchedules(...args),
restoreUserSchedulesFromDeletion: (...args) => mockRestoreUserSchedules(...args),
}));

jest.mock('~/server/services/Files/process', () => ({
processDeleteRequest: jest.fn().mockResolvedValue({ deletedFileIds: [], failedFileIds: [] }),
}));
Expand Down Expand Up @@ -160,6 +168,7 @@ describe('verifyEmailController', () => {

beforeEach(() => {
jest.clearAllMocks();
mockQuiesceUserSchedules.mockResolvedValue(true);
});

it('returns the generic verification error message from service failures', async () => {
Expand Down Expand Up @@ -343,6 +352,7 @@ describe('deleteUserController', () => {

beforeEach(() => {
jest.clearAllMocks();
mockQuiesceUserSchedules.mockResolvedValue(true);
});

it('should return 200 on successful deletion', async () => {
Expand All @@ -361,6 +371,7 @@ describe('deleteUserController', () => {
);
expect(mockDrainAgentTriggerDeliveriesForUser).toHaveBeenCalledWith(userId.toString());
expect(mockCancelAndDrainSubagentThreads).toHaveBeenCalledWith(userId.toString(), undefined);
expect(mockQuiesceUserSchedules).toHaveBeenCalledWith(userId.toString(), expect.any(String));
expect(beginAgentTriggerUserDeletion.mock.invocationCallOrder[0]).toBeLessThan(
mockPrepareAgentTriggerUserPurge.mock.invocationCallOrder[0],
);
Expand All @@ -371,6 +382,9 @@ describe('deleteUserController', () => {
mockCancelAndDrainSubagentThreads.mock.invocationCallOrder[0],
);
expect(mockCancelAndDrainSubagentThreads.mock.invocationCallOrder[0]).toBeLessThan(
mockQuiesceUserSchedules.mock.invocationCallOrder[0],
);
expect(mockQuiesceUserSchedules.mock.invocationCallOrder[0]).toBeLessThan(
deleteMessages.mock.invocationCallOrder[0],
);
expect(deleteMessages.mock.invocationCallOrder[0]).toBeLessThan(
Expand All @@ -382,6 +396,8 @@ describe('deleteUserController', () => {
expect(mockPurgeAgentTriggerDeliveriesForUser).toHaveBeenCalledWith(userId.toString());
expect(cancelAgentTriggerUserDeletion).not.toHaveBeenCalled();
expect(mockCancelAgentTriggerUserPurge).not.toHaveBeenCalled();
// A successful deletion hard-deletes the schedules; it must never restore them.
expect(mockRestoreUserSchedules).not.toHaveBeenCalled();
});

it('aborts generations admitted before the deletion fence before erasing messages', async () => {
Expand Down Expand Up @@ -428,6 +444,17 @@ describe('deleteUserController', () => {
expect(mockCancelAgentTriggerUserPurge).toHaveBeenCalledWith(userIdString, deletionFence);
expect(cancelAgentTriggerUserDeletion).toHaveBeenCalledWith(userIdString, deletionFence);
expect(deleteUserById).not.toHaveBeenCalled();
// Account survives -> its suspended schedules are restored under the quiesce token.
expect(mockRestoreUserSchedules).toHaveBeenCalledWith(
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 () => {
Expand All @@ -453,6 +480,41 @@ describe('deleteUserController', () => {
expect(deleteUserById).not.toHaveBeenCalled();
});

it('fails closed and releases deletion fences when schedules cannot be quiesced', async () => {
const userId = new mongoose.Types.ObjectId();
const userIdString = userId.toString();
mockQuiesceUserSchedules.mockResolvedValueOnce(false);
const req = {
user: {
id: userIdString,
_id: userId,
email: 'scheduled@test.com',
tenantId: 'tenant-1',
},
};

await deleteUserController(req, mockRes);

expect(mockRes.status).toHaveBeenCalledWith(500);
expect(deleteMessages).not.toHaveBeenCalled();
expect(mockGetActiveJobIdsForUser).not.toHaveBeenCalled();
const deletionFence = beginAgentTriggerUserDeletion.mock.calls[0][1];
expect(mockCancelAgentTriggerUserPurge).toHaveBeenCalledWith(userIdString, deletionFence);
expect(cancelAgentTriggerUserDeletion).toHaveBeenCalledWith(userIdString, deletionFence);
expect(deleteUserById).not.toHaveBeenCalled();
// Account survives -> its suspended schedules are restored under the quiesce token.
expect(mockRestoreUserSchedules).toHaveBeenCalledWith(
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 () => {
const userId = new mongoose.Types.ObjectId();
const userIdStr = userId.toString();
Expand Down
9 changes: 9 additions & 0 deletions api/server/controllers/__tests__/deleteUser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const mockPurgeAgentTriggerDeliveriesForUser = jest.fn();
const mockBeginAgentTriggerUserDeletion = jest.fn();
const mockCancelAgentTriggerUserDeletion = jest.fn();
const mockCancelAndDrainSubagentThreads = jest.fn();
const mockQuiesceUserSchedules = jest.fn();
const mockDeleteSchedulesByUser = jest.fn();

jest.mock('@librechat/data-schemas', () => ({
logger: { error: jest.fn(), info: jest.fn() },
Expand Down Expand Up @@ -85,6 +87,7 @@ jest.mock('~/models', () => ({
deleteTokens: jest.fn(),
removeUserFromAllGroups: jest.fn(),
deleteAclEntries: jest.fn(),
deleteSchedulesByUser: (...args) => mockDeleteSchedulesByUser(...args),
getSoleOwnedResourceIds: jest.fn().mockResolvedValue([]),
}));

Expand Down Expand Up @@ -127,6 +130,10 @@ jest.mock('~/server/services/Endpoints/agents/subagentThreadStore', () => ({
cancelAndDrainForOwner: (...args) => mockCancelAndDrainSubagentThreads(...args),
}));

jest.mock('~/server/services/Schedules', () => ({
quiesceUserSchedules: (...args) => mockQuiesceUserSchedules(...args),
}));

jest.mock('~/server/services/Config', () => ({
getAppConfig: jest.fn(),
}));
Expand Down Expand Up @@ -171,6 +178,8 @@ function stubDeletionMocks() {
mockBeginAgentTriggerUserDeletion.mockResolvedValue('acquired');
mockCancelAgentTriggerUserDeletion.mockResolvedValue(true);
mockCancelAndDrainSubagentThreads.mockResolvedValue();
mockQuiesceUserSchedules.mockResolvedValue(true);
mockDeleteSchedulesByUser.mockResolvedValue();
}

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ jest.mock('@librechat/data-schemas', () => ({

jest.mock('@librechat/api', () => ({
sendEvent: jest.fn(),
isScheduleFireRequest: jest.fn(() => false),
exemptFromConcurrencyLimiter: jest.fn(() => false),
toPendingSteer: jest.fn((item) => item),
isSteerPreemptSupported: jest.fn(() => true),
buildRecoveredSteerPayload: jest.fn(() => null),
Expand Down
Loading
Loading