diff --git a/.env.example b/.env.example index ecf08c9fdd..d6091a43e5 100644 --- a/.env.example +++ b/.env.example @@ -1096,8 +1096,10 @@ OPENWEATHER_API_KEY= # SCHEDULES_DISABLED=true # v1 runs the scheduler IN-PROCESS, and the standard server arms it in EVERY replica. -# That is only safe when replicas can see each other's generations. With USE_REDIS_STREAMS -# they share a job store and the scheduler arms on its own. Without it the job store is +# That is only safe when replicas can see each other's generations. With USE_REDIS=true AND +# USE_REDIS_STREAMS=true they share a job store and the scheduler arms on its own — the +# stream flag alone is NOT enough, because the Redis client it needs is only constructed +# when USE_REDIS is also on. Without a shared store the job store is # process-local, so a peer would reconcile runs whose jobs it cannot see and could mark a # still-running generation interrupted. A process cannot count its own replicas, so set # this to assert you run exactly ONE. If neither holds, the scheduler refuses to start and diff --git a/api/server/controllers/agents/resume.js b/api/server/controllers/agents/resume.js index abcc18370f..26fb78a07f 100644 --- a/api/server/controllers/agents/resume.js +++ b/api/server/controllers/agents/resume.js @@ -689,6 +689,17 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle) await GenerationJobManager.abortJob(streamId, { expectedCreatedAt: job.createdAt }).catch( (err) => logger.warn('[ResumeAgentController] Failed to abort a superseded resume', err), ); + // Settle the run too. The CAS already promoted it out of `requires_action`, and + // the abort deletes the job, so without this the row stays active with no + // generation and no evidence — holding a capacity slot and blocking deletion + // until the orphan sweep. + await recordScheduleOutcome({ + scheduleId: job.metadata.scheduleId, + scheduledFor: job.metadata.scheduledFor, + status: 'interrupted', + conversationId, + error: 'Schedule was no longer active when the approval was answered', + }); await decrementPendingRequest(userId); return res.status(409).json({ error: 'This scheduled run is no longer active' }); } diff --git a/api/server/routes/admin/users.js b/api/server/routes/admin/users.js index 411d3d4d22..2ef5dece72 100644 --- a/api/server/routes/admin/users.js +++ b/api/server/routes/admin/users.js @@ -19,6 +19,8 @@ const handlers = createAdminUsersHandlers({ deleteConfig: db.deleteConfig, deleteAclEntries: db.deleteAclEntries, quiesceUserSchedules, + markUserDeleting: db.markUserDeleting, + deleteSchedulesByUser: db.deleteSchedulesByUser, }); router.use(requireJwtAuth, requireAdminAccess); diff --git a/packages/api/src/admin/users.spec.ts b/packages/api/src/admin/users.spec.ts index 4b598bb34d..83ea1ca018 100644 --- a/packages/api/src/admin/users.spec.ts +++ b/packages/api/src/admin/users.spec.ts @@ -60,6 +60,8 @@ function createDeps(overrides: Partial = {}): AdminUsersDeps { deleteConfig: jest.fn().mockResolvedValue(null), deleteAclEntries: jest.fn().mockResolvedValue(undefined), quiesceUserSchedules: jest.fn().mockResolvedValue(true), + markUserDeleting: jest.fn().mockResolvedValue(new Date()), + deleteSchedulesByUser: jest.fn().mockResolvedValue(undefined), ...overrides, }; } @@ -496,6 +498,49 @@ describe('createAdminUsersHandlers', () => { * are not dormant data: an in-flight fire keeps persisting messages and billing * after the user document is gone. */ + it('raises the deletion barrier BEFORE quiescing', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + // The quiesce is a one-shot scan; only the durable barrier refuses admission to + // work created after it, so a create/Run Now racing the delete must hit the + // barrier rather than slip past the scan. + const barrier = (deps.markUserDeleting as jest.Mock).mock.invocationCallOrder[0]; + const quiesce = (deps.quiesceUserSchedules as jest.Mock).mock.invocationCallOrder[0]; + expect(barrier).toBeLessThan(quiesce); + }); + + it('refuses the delete when the barrier cannot be raised', async () => { + const deps = createDeps({ + markUserDeleting: jest.fn().mockRejectedValue(new Error('mongo down')), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(503); + expect(deps.quiesceUserSchedules).not.toHaveBeenCalled(); + expect(deps.deleteUserById).not.toHaveBeenCalled(); + }); + + it('hard-deletes the schedule rows rather than trusting the reconciler sweep', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + // The clustered entrypoint never arms the engine, so nothing would sweep them. + expect(deps.deleteSchedulesByUser).toHaveBeenCalledWith(validUserId); + const rows = (deps.deleteSchedulesByUser as jest.Mock).mock.invocationCallOrder[0]; + const userDel = (deps.deleteUserById as jest.Mock).mock.invocationCallOrder[0]; + expect(rows).toBeLessThan(userDel); + }); + it('quiesces the user schedules before removing the user', async () => { const deps = createDeps(); const handlers = createAdminUsersHandlers(deps); diff --git a/packages/api/src/admin/users.ts b/packages/api/src/admin/users.ts index 0dbd885447..9bdb1f770e 100644 --- a/packages/api/src/admin/users.ts +++ b/packages/api/src/admin/users.ts @@ -48,6 +48,13 @@ export interface AdminUsersDeps { * confirmed, in which case deletion must be refused rather than proceed. */ quiesceUserSchedules: (userId: string) => Promise; + /** Raises the durable, one-way account-deletion barrier. Must run BEFORE the quiesce: + * the quiesce is a one-shot scan, and only the barrier refuses admission to work + * created after it. */ + markUserDeleting: (userId: string) => Promise; + /** Hard-deletes the user's Schedule/ScheduleRun rows. Not left to the reconciler's + * `deleting` sweep, which the clustered entrypoint never runs. */ + deleteSchedulesByUser: (userId: string) => Promise; } export function createAdminUsersHandlers(deps: AdminUsersDeps): { @@ -62,6 +69,8 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): { deleteConfig, deleteAclEntries, quiesceUserSchedules, + markUserDeleting, + deleteSchedulesByUser, } = deps; async function listUsersHandler(req: ServerRequest, res: Response) { @@ -161,6 +170,23 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): { } } + // Raise the durable barrier FIRST, exactly as the self-service controller does. + // The quiesce below is a one-shot disable + active-run scan, so a schedule create + // or Run Now that overlaps it can pass its own admission check, land after the + // scan, and arm or dispatch while the user document is being removed. Only the + // barrier refuses that admission for the rest of the cascade. + const barrierRaised = await markUserDeleting(id).then( + () => true, + (error) => { + logger.error('[adminUsers] Failed to raise the deletion barrier', error); + return false; + }, + ); + if (!barrierRaised) { + res.set('Retry-After', '30'); + return res.status(503).json({ error: 'Could not start deletion. Please retry shortly.' }); + } + // Stop scheduled work BEFORE removing the user. The rest of this endpoint's // cascade is deliberately deferred (see deleteUserById), but scheduled runs are // not dormant data: an in-flight fire keeps persisting messages and billing after @@ -177,6 +203,14 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): { }); } + // Hard-delete the schedule rows rather than relying on the reconciler's + // `deleting` sweep: the clustered `experimental.js` entrypoint never arms the + // engine, so in that topology nothing would ever erase them and the deleted + // user's prompt text would persist indefinitely. + await deleteSchedulesByUser(id).catch((error) => { + logger.error('[adminUsers] Failed to delete schedules for the removed user', error); + }); + const result = await deleteUserById(id); if (result.deletedCount === 0) {