Skip to content

Commit 5b25771

Browse files
committed
fix(tables): keep a cancelled dispatch cancelled when a step claims it
`dispatcherStep` reads the dispatch, then awaits the table load before writing `dispatching`. Keying that write on the id alone resurrected a dispatch cancelled inside that window — a Stop-all, or now the stale-dispatch sweep — and the fresh heartbeat it writes would then buy the resurrected row another full window before the sweep could reclaim it again. The race predates the sweep, but the sweep is a new writer of `cancelled` that no user action drives, so it is newly reachable without anyone touching Stop. Re-asserting the status the step already read is the whole fix.
1 parent 9bb9e72 commit 5b25771

2 files changed

Lines changed: 63 additions & 3 deletions

File tree

apps/sim/lib/table/dispatcher.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,19 @@ export async function dispatcherStep(
411411
// cleanup sweep ages it from that rather than from `requested_at`, which
412412
// was stamped when the run was merely requested.
413413
.set({ status: 'dispatching', heartbeatAt: new Date() })
414-
.where(eq(tableRunDispatches.id, dispatchId))
414+
/**
415+
* Re-asserts the status this step read two awaits ago. `dispatchId` alone
416+
* would resurrect a dispatch cancelled in that window — a Stop-all, or now
417+
* the stale-dispatch sweep — back to `dispatching`, and with a fresh
418+
* heartbeat the sweep would then wait out another full window before
419+
* reclaiming what it had already given up on.
420+
*/
421+
.where(
422+
and(
423+
eq(tableRunDispatches.id, dispatchId),
424+
inArray(tableRunDispatches.status, [...ACTIVE_DISPATCH_STATUSES])
425+
)
426+
)
415427
// Announce the dispatch the moment it starts — before the first window's
416428
// cells finish. Without this, auto-fired and capped dispatches (no client-
417429
// side optimistic seed) emit their first `dispatch` event only after window

apps/sim/lib/table/stale-dispatch-recovery.test.ts

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@
44
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockAppendTableEvent } = vi.hoisted(() => ({
7+
const { mockAppendTableEvent, mockGetTableById } = vi.hoisted(() => ({
88
mockAppendTableEvent: vi.fn(),
9+
mockGetTableById: vi.fn(),
910
}))
1011

1112
vi.mock('@/lib/table/events', () => ({
1213
appendTableEvent: mockAppendTableEvent,
1314
}))
15+
vi.mock('@/lib/table/service', () => ({
16+
getTableById: mockGetTableById,
17+
}))
1418

15-
import { cancelStaleDispatches } from '@/lib/table/dispatcher'
19+
import { cancelStaleDispatches, dispatcherStep } from '@/lib/table/dispatcher'
1620

1721
const STALE_BEFORE = new Date('2026-08-21T17:00:00.000Z')
1822

@@ -131,3 +135,47 @@ describe('cancelStaleDispatches', () => {
131135
expect(mockAppendTableEvent).not.toHaveBeenCalled()
132136
})
133137
})
138+
139+
describe('dispatcherStep pending transition', () => {
140+
beforeEach(() => {
141+
vi.clearAllMocks()
142+
resetDbChainMock()
143+
})
144+
145+
/**
146+
* The step reads the dispatch, then awaits the table load before writing
147+
* `dispatching`. A cancel landing in that window — a Stop-all, or the
148+
* stale-dispatch sweep — must win: keying the write on the id alone
149+
* resurrected the row, and with a fresh heartbeat the sweep would then wait
150+
* out another full window before reclaiming what it had already given up on.
151+
*/
152+
it('re-asserts the status it read before claiming the dispatch', async () => {
153+
mockGetTableById.mockResolvedValue({
154+
id: 'table-1',
155+
schema: { workflowGroups: [{ id: 'group-1' }] },
156+
})
157+
dbChainMockFns.limit.mockResolvedValue([{ ...ABANDONED_ROW, status: 'pending' }])
158+
159+
await dispatcherStep('tdsp_1').catch(() => {})
160+
161+
// The write must actually happen, or this asserts nothing.
162+
expect(
163+
dbChainMockFns.set.mock.calls.some(
164+
(call) => (call[0] as Record<string, unknown> | undefined)?.status === 'dispatching'
165+
)
166+
).toBe(true)
167+
168+
/**
169+
* `set` and `where` are separate spies, so their call indexes do not pair —
170+
* `readDispatch`'s own select filed a `where` first. Match on the predicate
171+
* that only the claim can produce: this row's id AND its status together.
172+
*/
173+
const claimPredicate = dbChainMockFns.where.mock.calls
174+
.map((call) => collectChunks(call[0]))
175+
.find(
176+
(chunks) =>
177+
chunks.includes('tableRunDispatches.id') && chunks.includes('tableRunDispatches.status')
178+
)
179+
expect(claimPredicate).toBeDefined()
180+
})
181+
})

0 commit comments

Comments
 (0)