Skip to content

Commit 84f41ea

Browse files
authored
fix(execution): stop in-flight workflow runs when an account is deleted, removed or banned (#9145)
* fix(execution): stop in-flight workflow runs when an account is deleted, removed or banned No removal path stopped workflow executions. Account deletion cascades the doomed workspaces' log rows away, after which no cancel path can find the run, so it kept spending hosted provider keys for an account that no longer existed until it crashed on foreign keys at finalization. A ban archived the workspaces and stopped nothing; an admin removal cascaded them the same way. - Account deletion reads the running executions (and claimed resumes, which run under their own id) of the doomed workspaces in its transaction, while the workspace rows are locked, and records one outbox stop per workspace with the erasure. After commit the stops are delivered a few at a time; a stop that cannot be delivered stays in the outbox for its worker, and the committed deletion is never reported as failed. - A stop sets the cancellation flag each engine watches and sweeps the job queue for the workspace: a new JobQueueBackend.cancelByWorkspace cancels queued, delayed and running workflow jobs by the workspaceId tag on Trigger.dev (pages of 100, ten cancels at a time, 1,000 runs per attempt, continued without spending an attempt) and by metadata.workspaceId on the database queue, so queued runs no longer fail preprocessing later. - A missing Redis completes the stop: the flag is process-local by design and no retry could reach further. A refused Redis write or a failed sweep retries. - A ban stops the runs of the workspaces it archived; the admin remove-user hooks read the runs before the bare user delete and record them after it commits. Both sit in page module graphs that cannot load the delivery, so they signal in-flight runs at once and leave the queue sweep to the outbox worker. * fix(execution): keep removal stops off the deletion and ban critical paths - Account deletion no longer awaits the full stop delivery before purging storage. A slow Trigger.dev listing could hold the response for minutes and widen the window in which a restart orphans erased objects. It now signals in-flight runs fire-and-forget right after commit, ahead of every other post-commit step, under a 10-second deadline; the committed outbox stops repeat the signal and sweep the job queue in the outbox worker. - A failure to record a ban's stops is logged instead of thrown. The ban and archives have committed, so a 500 helped no one and a retry finds no unarchived workspace to stop. - Claimed resumes are found from the scope's running and paused log rows and joined through resume_queue's parent index, not by scanning resume_queue for claimed rows inside the deletion transaction. - Tests pin the database queue's job-type filter, so a workspace sweep never cancels a non-workflow job such as log cleanup. * test(execution): give each removal-stop fixture workflow a unique name * fix(execution): close the review's gaps in removal stops - A ban records and signals each workspace's stops as soon as its archive commits, through the archive's post-commit stop hook, instead of after every archive's socket and webhook cleanup. It also stops runs in the owner's workspaces archived before the ban, which an archive leaves running. - The outbox handler re-reads a workspace's running executions when it runs. An archived workspace keeps its rows, so a run admitted before the ban that logged after its stop was recorded is still signalled. - The admin remove-user hook handlers move to lib/auth/admin-removal-stops.ts, and signal the removed runs even when recording their stops fails. - The database queue's workspace sweep works in batches of 100 up to the caller's cap and checks its abort signal between them, like the Trigger.dev sweep, rather than one unbounded update reported as complete. * refactor(execution): tighten removal stops after final audit - disableUserResources reads owned workspaces once and stops earlier-archived ones up front, dropping the post-archive read that could fail the ban - Trigger.dev cancelByWorkspace reuses scanAndCancelTriggerRuns; workspace tag shares one builder with enqueue - database queue shares abortInlineJob with cancelJob and returns completeness from control flow - removal stop handler parses its payload with zod and signals while it sweeps - drop the unused event-id return and redundant bucketing state * fix(execution): finish the removal sweep's edge cases - The removal stop handler loads both job queues independently and sweeps whichever loaded, so a primary queue that fails to load no longer skips the database queue's sweep. The failure is still rethrown for the outbox to retry. - A claimed resume is found under a parent log row in the legacy `paused` status too, one of the statuses a resume claims from. - The database queue re-checks for a matching active job after a short batch, since a selected job that finished before the update reached it shrinks the batch while later jobs still match. - The ban's completion log no longer reports its runs stopped when recording their stops failed. * fix(execution): abort local runs before the removal signal deadline can skip them The signal deadline bounds only the Redis flag writes. Aborting a run this process holds needs no Redis, so every execution is aborted first, even after the deadline has passed. Corrects the ban's note on why a failed stop record is logged: banning again records the stops again. * fix(execution): guard ban stops against transfers and refusing runs - A ban records stops only for workspaces the user still owns, re-read in the recording transaction and held FOR SHARE until it commits, so a concurrent ownership transfer waits rather than having its new owner's runs stopped. Only workspace rows are locked, so no lock order is inverted. - Trigger.dev's workspace sweep counts only cancelled runs toward its cap, with a separate bound on failures, so runs that keep refusing cancellation no longer hold every retry at the front of the listing. cancelByExecution is unchanged. * fix(execution): sweep a removed workspace's Trigger.dev runs of any age The workspace sweep searched only runs created in the last 14 days, so an older run still queued or waiting survived the removal. It now passes only an upper time bound, which Trigger.dev treats as no lower bound, and falls back to the retention window only if Trigger.dev refuses that search as too expensive before returning a run. cancelByExecution is unchanged.
1 parent c3f2b30 commit 84f41ea

18 files changed

Lines changed: 1613 additions & 22 deletions
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* The `/admin/remove-user` hooks hand a plan from `before` to `after` across a non-transactional
3+
* delete. Failure modes:
4+
* - the outbox write fails after the delete committed → the in-flight runs must still be signalled
5+
* - `after` runs for a different delete than the one `before` read → nothing is stopped
6+
*/
7+
import { workflowExecutionLogs, workspace } from '@sim/db/schema'
8+
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
9+
import { redisConfigMockFns } from '@sim/testing/mocks/redis-config.mock'
10+
import { beforeEach, describe, expect, it, vi } from 'vitest'
11+
import { holdAdminRemovalStops, stopAdminRemovedRuns } from '@/lib/auth/admin-removal-stops'
12+
13+
const mockEval = vi.fn()
14+
15+
function queueRunningExecution(executionId: string) {
16+
queueTableRows(workspace, [])
17+
queueTableRows(workflowExecutionLogs, [
18+
{ executionId, executionDeadlineAt: null, workspaceId: 'workspace-elsewhere' },
19+
])
20+
queueTableRows(workflowExecutionLogs, [])
21+
}
22+
23+
describe('admin user removal stops', () => {
24+
beforeEach(() => {
25+
resetDbChainMock()
26+
mockEval.mockReset().mockResolvedValue(1)
27+
redisConfigMockFns.mockGetRedisClient.mockReturnValue({ eval: mockEval })
28+
})
29+
30+
it('signals the removed user’s runs even when their stops cannot be recorded', async () => {
31+
const user = { id: 'user-1' }
32+
queueRunningExecution('execution-1')
33+
await holdAdminRemovalStops(user)
34+
dbChainMockFns.insert.mockImplementationOnce(() => {
35+
throw new Error('outbox unavailable')
36+
})
37+
38+
await expect(stopAdminRemovedRuns(user)).resolves.toBeUndefined()
39+
40+
expect(mockEval).toHaveBeenCalledWith(
41+
expect.any(String),
42+
1,
43+
'execution:cancel:execution-1',
44+
expect.anything(),
45+
expect.anything(),
46+
expect.anything(),
47+
expect.anything()
48+
)
49+
})
50+
51+
it('stops nothing for a delete whose runs it did not read', async () => {
52+
queueRunningExecution('execution-2')
53+
await holdAdminRemovalStops({ id: 'user-2' })
54+
55+
await stopAdminRemovedRuns({ id: 'user-2' })
56+
57+
expect(mockEval).not.toHaveBeenCalled()
58+
})
59+
})
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { db } from '@sim/db'
2+
import { createLogger } from '@sim/logger'
3+
import { getErrorMessage } from '@sim/utils/errors'
4+
import {
5+
collectAccountRemovalStops,
6+
type ExecutionStopPlan,
7+
enqueueExecutionStops,
8+
signalExecutionStops,
9+
} from '@/lib/execution/removal-stop-records'
10+
11+
const logger = createLogger('AdminRemovalStops')
12+
13+
/**
14+
* Plans read by {@link holdAdminRemovalStops} and acted on by {@link stopAdminRemovedRuns},
15+
* keyed by the user entity Better Auth passes to both hooks of one delete. A delete that fails
16+
* never reaches `after`, so its plan is dropped with the entity.
17+
*/
18+
const pendingStops = new WeakMap<object, ExecutionStopPlan>()
19+
20+
/**
21+
* The user delete's `before` hook: `/admin/remove-user` deletes the user row outside any
22+
* transaction, and the cascade erases the rows naming its runs, so they are read now. A failed
23+
* read lets the removal proceed as before rather than blocking an admin.
24+
*/
25+
export async function holdAdminRemovalStops(user: { id: string }): Promise<void> {
26+
try {
27+
pendingStops.set(user, await collectAccountRemovalStops(db, user.id))
28+
} catch (error) {
29+
logger.error('Failed to read the runs a user removal must stop', {
30+
userId: user.id,
31+
error: getErrorMessage(error),
32+
})
33+
}
34+
}
35+
36+
/**
37+
* The user delete's `after` hook, once the delete has committed: signals the in-flight runs at
38+
* once, whether or not their stops could be recorded, and records the stops for the outbox
39+
* worker's queue sweep and retries. Never throws.
40+
*/
41+
export async function stopAdminRemovedRuns(user: { id: string }): Promise<void> {
42+
const plan = pendingStops.get(user)
43+
pendingStops.delete(user)
44+
if (!plan) return
45+
try {
46+
await enqueueExecutionStops(db, plan)
47+
} catch (error) {
48+
logger.error('Failed to record the stops of a removed user’s runs', {
49+
userId: user.id,
50+
error: getErrorMessage(error),
51+
})
52+
} finally {
53+
await signalExecutionStops(plan.executions)
54+
}
55+
}

‎apps/sim/lib/auth/auth.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { readAttributionProperties } from '@/lib/analytics/attribution'
3838
import { FREEBUFF_CLICK_ID_COOKIE } from '@/lib/analytics/freebuff'
3939
import { reportFreebuffConversion } from '@/lib/analytics/freebuff.server'
4040
import { getAccessControlConfig, isEmailBlockedByAccessControl } from '@/lib/auth/access-control'
41+
import { holdAdminRemovalStops, stopAdminRemovedRuns } from '@/lib/auth/admin-removal-stops'
4142
import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous'
4243
import { buildConnectorProviders } from '@/lib/auth/connectors/providers'
4344
import {
@@ -469,11 +470,15 @@ export const auth = betterAuth({
469470
},
470471
/**
471472
* The admin plugin's `/admin/remove-user` deletes the user row through Better Auth, so its
472-
* usage rows cascade away without `deleteUserAccount`. Better Auth runs this once the delete
473-
* commits, which is when the stored reporting prefixes that counted them must be discarded.
473+
* usage rows cascade away without `deleteUserAccount`. Better Auth runs `after` once the
474+
* delete commits, which is when the stored reporting prefixes that counted them must be
475+
* discarded, and when the runs the cascade orphaned are stopped: in-flight ones at once,
476+
* queued ones by the outbox worker, since this module cannot load that delivery.
474477
*/
475478
delete: {
476-
after: async () => {
479+
before: holdAdminRemovalStops,
480+
after: async (user) => {
481+
await stopAdminRemovedRuns(user)
477482
await invalidateReportingUsagePrefixes()
478483
},
479484
},

‎apps/sim/lib/core/async-jobs/backends/database.test.ts‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { dbChainMockFns, resetDbChainMock } from '@sim/testing/mocks/database.mock'
1+
import { asyncJobs } from '@sim/db'
2+
import {
3+
dbChainMockFns,
4+
flattenMockConditions,
5+
queueTableRows,
6+
resetDbChainMock,
7+
} from '@sim/testing/mocks/database.mock'
28
import { sleep } from '@sim/utils/helpers'
39
import { beforeEach, describe, expect, it, vi } from 'vitest'
410

@@ -503,3 +509,69 @@ describe('DatabaseJobQueue a workflow job that completed with a recorded failure
503509
})
504510
})
505511
})
512+
513+
describe('DatabaseJobQueue workspace cancellation', () => {
514+
beforeEach(() => {
515+
resetDbChainMock()
516+
})
517+
518+
it('cancels only workflow job types, so the workspace’s other jobs keep running', async () => {
519+
await new DatabaseJobQueue().cancelByWorkspace('workspace-1', { maxRuns: 1000 })
520+
521+
const typeFilter = flattenMockConditions(dbChainMockFns.where.mock.calls[0][0]).find(
522+
(node) => node.type === 'inArray' && node.column === asyncJobs.type
523+
)
524+
expect(typeFilter?.values).toEqual([
525+
'workflow-execution',
526+
'schedule-execution',
527+
'webhook-execution',
528+
'resume-execution',
529+
'workflow-group-cell',
530+
])
531+
})
532+
533+
const ids = (count: number, offset = 0) =>
534+
Array.from({ length: count }, (_, index) => ({ id: `job-${offset + index}` }))
535+
536+
it('keeps cancelling in batches until a short batch shows none are left', async () => {
537+
dbChainMockFns.returning
538+
.mockResolvedValueOnce(ids(100))
539+
.mockResolvedValueOnce(ids(100, 100))
540+
.mockResolvedValueOnce(ids(30, 200))
541+
542+
await expect(
543+
new DatabaseJobQueue().cancelByWorkspace('workspace-1', { maxRuns: 1000 })
544+
).resolves.toEqual({ cancelledJobs: 230, complete: true })
545+
})
546+
547+
it('reports a short batch incomplete while matching jobs remain', async () => {
548+
dbChainMockFns.returning.mockResolvedValueOnce(ids(99))
549+
queueTableRows(asyncJobs, [{ id: 'job-100' }])
550+
551+
await expect(
552+
new DatabaseJobQueue().cancelByWorkspace('workspace-1', { maxRuns: 1000 })
553+
).resolves.toEqual({ cancelledJobs: 99, complete: false })
554+
})
555+
556+
it('stops at its run cap and reports the sweep incomplete', async () => {
557+
dbChainMockFns.returning.mockResolvedValueOnce(ids(100)).mockResolvedValueOnce(ids(50, 100))
558+
559+
await expect(
560+
new DatabaseJobQueue().cancelByWorkspace('workspace-1', { maxRuns: 150 })
561+
).resolves.toEqual({ cancelledJobs: 150, complete: false })
562+
expect(dbChainMockFns.returning).toHaveBeenCalledTimes(2)
563+
})
564+
565+
it('cancels nothing once its caller’s time is up', async () => {
566+
const controller = new AbortController()
567+
controller.abort()
568+
569+
await expect(
570+
new DatabaseJobQueue().cancelByWorkspace('workspace-1', {
571+
maxRuns: 1000,
572+
signal: controller.signal,
573+
})
574+
).resolves.toEqual({ cancelledJobs: 0, complete: false })
575+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
576+
})
577+
})

‎apps/sim/lib/core/async-jobs/backends/database.ts‎

Lines changed: 73 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ import {
1818
type JobStatus,
1919
type JobType,
2020
validateMaxDurationSeconds,
21+
WORKSPACE_EXECUTION_JOB_TYPES,
22+
type WorkspaceJobCancellationOptions,
23+
type WorkspaceJobCancellationResult,
2124
} from '@/lib/core/async-jobs/types'
2225
import { recordExecutionCancellationBackendResult } from '@/lib/core/execution-limits/metrics'
2326

2427
const logger = createLogger('DatabaseJobQueue')
28+
const WORKSPACE_CANCELLATION_BATCH_SIZE = 100
2529

2630
type AsyncJobRow = typeof asyncJobs.$inferSelect
2731
type Runner = NonNullable<EnqueueOptions['runner']>
@@ -51,6 +55,15 @@ function rowToJob(row: AsyncJobRow): Job {
5155
const inlineAbortControllers = new Map<string, AbortController>()
5256
const inlineExecutionControllers = new Map<string, Map<AbortController, JobType>>()
5357

58+
/** Aborts the runner this process holds for the job, if any. */
59+
function abortInlineJob(jobId: string): boolean {
60+
const controller = inlineAbortControllers.get(jobId)
61+
if (!controller) return false
62+
controller.abort(new DOMException('user', 'AbortError'))
63+
inlineAbortControllers.delete(jobId)
64+
return true
65+
}
66+
5467
/**
5568
* Per-cancel-key abort controllers for the `batchEnqueueAndWait` direct-call
5669
* path. Distinct from `inlineAbortControllers` (which keys by jobId) — this
@@ -420,13 +433,7 @@ export class DatabaseJobQueue implements JobQueueBackend {
420433
// Abort any in-process inline execution first so the running workflow
421434
// observes the signal and stops mid-flight. Then mark the row cancelled so
422435
// any future poller skips it without treating a user action as a failure.
423-
const controller = inlineAbortControllers.get(jobId)
424-
let aborted = false
425-
if (controller) {
426-
controller.abort(new DOMException('user', 'AbortError'))
427-
inlineAbortControllers.delete(jobId)
428-
aborted = true
429-
}
436+
const aborted = abortInlineJob(jobId)
430437

431438
const now = new Date()
432439
await db
@@ -517,6 +524,65 @@ export class DatabaseJobQueue implements JobQueueBackend {
517524
}
518525
}
519526

527+
/**
528+
* Cancels the workspace's pending and processing rows, so no process can claim them, and
529+
* aborts the runners this process holds for them. A runner another process holds stops through
530+
* its execution's cancellation signal instead. Batches run through `batchEnqueueAndWait` keep
531+
* no row and are left to that signal and their dispatch's own cancellation. Works in batches
532+
* up to `maxRuns`, checking `signal` between them, like the Trigger.dev sweep.
533+
*/
534+
async cancelByWorkspace(
535+
workspaceId: string,
536+
options: WorkspaceJobCancellationOptions
537+
): Promise<WorkspaceJobCancellationResult> {
538+
const active = and(
539+
inArray(asyncJobs.status, [JOB_STATUS.PENDING, JOB_STATUS.PROCESSING]),
540+
inArray(asyncJobs.type, [...WORKSPACE_EXECUTION_JOB_TYPES]),
541+
sql`${asyncJobs.metadata}->>'workspaceId' = ${workspaceId}`
542+
)
543+
let cancelledJobs = 0
544+
while (cancelledJobs < options.maxRuns && !options.signal?.aborted) {
545+
const batchSize = Math.min(WORKSPACE_CANCELLATION_BATCH_SIZE, options.maxRuns - cancelledJobs)
546+
const now = new Date()
547+
const cancelled = await db
548+
.update(asyncJobs)
549+
.set({ status: JOB_STATUS.CANCELLED, completedAt: now, error: 'Cancelled', updatedAt: now })
550+
.where(
551+
and(
552+
active,
553+
inArray(
554+
asyncJobs.id,
555+
db.select({ id: asyncJobs.id }).from(asyncJobs).where(active).limit(batchSize)
556+
)
557+
)
558+
)
559+
.returning({ id: asyncJobs.id })
560+
561+
for (const { id } of cancelled) abortInlineJob(id)
562+
cancelledJobs += cancelled.length
563+
if (cancelled.length < batchSize) {
564+
/**
565+
* A short batch is not proof the sweep is done: a selected job that finished before the
566+
* update reached it shrinks the batch while later jobs may still match.
567+
*/
568+
const [remaining] = await db
569+
.select({ id: asyncJobs.id })
570+
.from(asyncJobs)
571+
.where(active)
572+
.limit(1)
573+
logger.info('Cancelled database jobs for workspace', {
574+
workspaceId,
575+
cancelledJobs,
576+
complete: !remaining,
577+
})
578+
return { cancelledJobs, complete: !remaining }
579+
}
580+
}
581+
582+
logger.info('Paused database job sweep for workspace', { workspaceId, cancelledJobs })
583+
return { cancelledJobs, complete: false }
584+
}
585+
520586
cancelByKey(cancelKey: string): boolean {
521587
const controller = inlineCancelKeyControllers.get(cancelKey)
522588
if (!controller) return false

0 commit comments

Comments
 (0)