Skip to content

Commit b028ce6

Browse files
committed
fix(table): run a drained pre-stamp under the subject that stamped it
A dispatcher pre-stamp outlives the worker that wrote it: a cell task that finds the row's cascade lock held bails, and the lock owner drains the marker under its own payload. With two dispatches overlapping on one row that ran another dispatch's request against the wrong person's tool denylist — or, when the owner was an actorless auto-fire, against none. The marker now carries its governed subject and the drain reads it back; 0316 also adds the partial index account deletion's cancel filter needs and repeats 0315's backfill to close its rolling-deploy window.
1 parent f4768b3 commit b028ce6

13 files changed

Lines changed: 21303 additions & 1 deletion
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { resetDbChainMock } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
getTableById: vi.fn(),
9+
getRowById: vi.fn(),
10+
pickNextEligibleGroupForRow: vi.fn(),
11+
writeWorkflowGroupState: vi.fn(),
12+
markWorkflowGroupPickedUp: vi.fn(),
13+
runEnrichment: vi.fn(),
14+
getEnrichment: vi.fn(),
15+
readStampedCapabilitySubject: vi.fn(),
16+
checkAttributedUsageLimits: vi.fn(),
17+
loadTableRowSecretProvenance: vi.fn(),
18+
}))
19+
20+
vi.mock('@/lib/table/service', () => ({ getTableById: mocks.getTableById }))
21+
vi.mock('@/lib/table/rows/service', () => ({ getRowById: mocks.getRowById, updateRow: vi.fn() }))
22+
vi.mock('@/lib/table/rows/executions', () => ({
23+
readStampedCapabilitySubject: mocks.readStampedCapabilitySubject,
24+
}))
25+
vi.mock('@/lib/table/workflow-columns', () => ({
26+
pickNextEligibleGroupForRow: mocks.pickNextEligibleGroupForRow,
27+
stashCellContextForResume: vi.fn(),
28+
buildWorkflowGroupExecutionCorrelation: vi.fn(),
29+
}))
30+
vi.mock('@/lib/table/cell-write', () => ({
31+
buildCancelledExecution: vi.fn(),
32+
createWorkflowCellProgressWriter: vi.fn(),
33+
writeWorkflowGroupState: mocks.writeWorkflowGroupState,
34+
markWorkflowGroupPickedUp: mocks.markWorkflowGroupPickedUp,
35+
}))
36+
vi.mock('@/lib/table/workflow-cell-result', () => ({
37+
classifyWorkflowCellTerminalResult: vi.fn(),
38+
}))
39+
vi.mock('@/enrichments/registry', () => ({ getEnrichment: mocks.getEnrichment }))
40+
vi.mock('@/enrichments/run', () => ({
41+
runEnrichment: mocks.runEnrichment,
42+
skippedEnrichmentDetail: () => ({}),
43+
}))
44+
vi.mock('@/lib/billing/core/billing-attribution', () => ({
45+
assertBillingAttributionSnapshot: (snapshot: unknown) => snapshot,
46+
checkAttributedUsageLimits: mocks.checkAttributedUsageLimits,
47+
toBillingContext: () => ({}),
48+
}))
49+
vi.mock('@/lib/table/rows/secret-provenance', () => ({
50+
createExactEmptyTableRowSecretProvenance: () => ({ complete: true, columns: {} }),
51+
createTableRowSecretProvenanceFromRegistry: () => ({ complete: true, columns: {} }),
52+
loadTableRowSecretProvenance: mocks.loadTableRowSecretProvenance,
53+
}))
54+
vi.mock('@/lib/table/events', () => ({ appendTableEvent: vi.fn() }))
55+
vi.mock('@/lib/table/dispatcher', () => ({
56+
readDispatch: vi.fn(async () => ({ id: 'tdsp_carrier', status: 'dispatching' })),
57+
completeDispatchIfActive: vi.fn(),
58+
}))
59+
60+
import { runRowCascadeLoop } from '@/background/workflow-column-execution'
61+
62+
function enrichmentGroup(id: string) {
63+
return {
64+
id,
65+
type: 'enrichment',
66+
enrichmentId: 'enrich-1',
67+
workflowId: '',
68+
outputs: [{ blockId: '', path: 'out', columnName: 'out' }],
69+
inputMappings: [{ inputName: 'domain', columnName: 'domain' }],
70+
}
71+
}
72+
73+
const TABLE = {
74+
id: 'table-1',
75+
name: 'Table',
76+
workspaceId: 'workspace-1',
77+
schema: {
78+
columns: [{ id: 'domain', name: 'domain', type: 'string' }],
79+
workflowGroups: [enrichmentGroup('group-1'), enrichmentGroup('group-2')],
80+
},
81+
}
82+
83+
/** The carrier belongs to an actorless auto-fire: no subject, so no tool gate. */
84+
const CARRIER = {
85+
tableId: 'table-1',
86+
tableName: 'Table',
87+
rowId: 'row-1',
88+
groupId: 'group-1',
89+
workflowId: '',
90+
enrichmentId: 'enrich-1',
91+
workspaceId: 'workspace-1',
92+
executionId: 'execution-1',
93+
dispatchId: 'tdsp_carrier',
94+
executionTimeoutMs: 10_000,
95+
capabilityGovernedUserId: null,
96+
billingAttribution: {
97+
actorUserId: 'user-1',
98+
workspaceId: 'workspace-1',
99+
organizationId: null,
100+
billedAccountUserId: 'user-1',
101+
billingEntity: { type: 'user' as const, id: 'user-1' },
102+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
103+
payerSubscription: null,
104+
},
105+
} as Parameters<typeof runRowCascadeLoop>[0]
106+
107+
describe('draining another dispatch’s pre-stamped marker', () => {
108+
beforeEach(() => {
109+
vi.clearAllMocks()
110+
resetDbChainMock()
111+
mocks.getTableById.mockResolvedValue(TABLE)
112+
mocks.getEnrichment.mockReturnValue({
113+
id: 'enrich-1',
114+
name: 'Enrich',
115+
inputs: [{ id: 'domain', required: true }],
116+
outputs: [{ id: 'out' }],
117+
})
118+
mocks.checkAttributedUsageLimits.mockResolvedValue({ isExceeded: false })
119+
mocks.markWorkflowGroupPickedUp.mockResolvedValue('picked-up')
120+
mocks.writeWorkflowGroupState.mockResolvedValue('wrote')
121+
mocks.loadTableRowSecretProvenance.mockResolvedValue({
122+
scope: { userId: 'user-1', workspaceId: 'workspace-1' },
123+
byRowId: {},
124+
})
125+
mocks.runEnrichment.mockResolvedValue({ result: { out: 'x' }, cost: 0, detail: {} })
126+
mocks.readStampedCapabilitySubject.mockResolvedValue('requesting-member')
127+
// group-1 completes, then group-2 is picked up carrying an unclaimed marker.
128+
mocks.getRowById.mockResolvedValue({
129+
id: 'row-1',
130+
data: { domain: 'example.com' },
131+
executions: { 'group-2': { status: 'pending', executionId: null, workflowId: '' } },
132+
})
133+
mocks.pickNextEligibleGroupForRow
134+
.mockReturnValueOnce(enrichmentGroup('group-2'))
135+
.mockReturnValue(null)
136+
})
137+
138+
/**
139+
* The lock owner drains markers it did not stamp. Running them under its own
140+
* subject applies the wrong person's tool denylist — and when the owner is an
141+
* actorless auto-fire, no denylist at all.
142+
*/
143+
it('runs the drained cell under the subject stamped with it', async () => {
144+
await runRowCascadeLoop(CARRIER)
145+
146+
expect(mocks.readStampedCapabilitySubject).toHaveBeenCalledWith('row-1', 'group-2')
147+
const subjects = mocks.runEnrichment.mock.calls.map(
148+
([, , ctx]) => (ctx as { userId: string | null }).userId
149+
)
150+
expect(subjects).toEqual([null, 'requesting-member'])
151+
})
152+
})

apps/sim/background/workflow-column-execution.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ export async function executeWorkflowGroupCellJob(
301301
const { getTableById } = await import('@/lib/table/service')
302302
const { getRowById } = await import('@/lib/table/rows/service')
303303
const { pickNextEligibleGroupForRow } = await import('@/lib/table/workflow-columns')
304+
const { readStampedCapabilitySubject } = await import('@/lib/table/rows/executions')
304305

305306
let currentPayload = payload
306307
while (true) {
@@ -344,6 +345,13 @@ export async function executeWorkflowGroupCellJob(
344345
// Re-derive so a workflow group after an enrichment group doesn't keep a stale enrichmentId.
345346
enrichmentId: next.enrichmentId,
346347
executionId: generateId(),
348+
/**
349+
* The marker was stamped by whichever dispatch requested THIS cell,
350+
* which is not necessarily the one that queued this carrier. Its gate
351+
* belongs to the person who asked for it, so take the subject off the
352+
* stamp rather than carrying our own into someone else's request.
353+
*/
354+
capabilityGovernedUserId: await readStampedCapabilitySubject(rowId, next.id),
347355
}
348356
}
349357
} finally {
@@ -362,12 +370,14 @@ export async function runRowCascadeLoop(
362370
const { getTableById } = await import('@/lib/table/service')
363371
const { getRowById } = await import('@/lib/table/rows/service')
364372
const { pickNextEligibleGroupForRow } = await import('@/lib/table/workflow-columns')
373+
const { readStampedCapabilitySubject } = await import('@/lib/table/rows/executions')
365374

366375
let currentGroupId = payload.groupId
367376
let currentWorkflowId = payload.workflowId
368377
// Fresh executionId per iteration: SQL guard rejects writes whose id ≠
369378
// row.executions[gid].executionId, so we need a new claim per group.
370379
let currentExecutionId = payload.executionId
380+
let currentCapabilityGovernedUserId = payload.capabilityGovernedUserId ?? null
371381

372382
while (true) {
373383
if (signal?.aborted) {
@@ -377,6 +387,7 @@ export async function runRowCascadeLoop(
377387
groupId: currentGroupId,
378388
workflowId: currentWorkflowId,
379389
executionId: currentExecutionId,
390+
capabilityGovernedUserId: currentCapabilityGovernedUserId,
380391
},
381392
signal
382393
)
@@ -400,6 +411,7 @@ export async function runRowCascadeLoop(
400411
groupId: currentGroupId,
401412
workflowId: currentWorkflowId,
402413
executionId: currentExecutionId,
414+
capabilityGovernedUserId: currentCapabilityGovernedUserId,
403415
},
404416
signal,
405417
freshTable,
@@ -416,6 +428,17 @@ export async function runRowCascadeLoop(
416428
if (!freshRow) break
417429
const next = pickNextEligibleGroupForRow(freshTable, freshRow, currentGroupId)
418430
if (!next) break
431+
const nextExec = freshRow.executions?.[next.id]
432+
/**
433+
* A dep-fill cascade stays under the subject that started it. A group
434+
* carrying an unclaimed pre-stamp is a different thing — an explicit
435+
* request from another dispatch that this cascade is draining — so it runs
436+
* under the subject stamped with that request.
437+
*/
438+
currentCapabilityGovernedUserId =
439+
nextExec?.status === 'pending' && nextExec.executionId == null
440+
? await readStampedCapabilitySubject(rowId, next.id)
441+
: currentCapabilityGovernedUserId
419442
currentGroupId = next.id
420443
currentWorkflowId = next.workflowId
421444
currentExecutionId = generateId()

apps/sim/lib/table/dispatcher.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,15 @@ async function stampQueuedForBatch(
815815
jobId: null,
816816
workflowId: runOpts.workflowId,
817817
error: null,
818+
/**
819+
* The marker outlives this dispatch's own worker: a cell task that
820+
* finds the row's cascade lock held bails, and whoever owns the lock
821+
* drains this marker instead. Persisting the subject is what makes
822+
* that drain run under the person who requested THIS cell rather
823+
* than under the owner's — a different dispatch, and often an
824+
* actorless auto-fire with no gate at all.
825+
*/
826+
capabilityGovernedUserId: runOpts.capabilityGovernedUserId ?? null,
818827
},
819828
}
820829
)
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
getTableById: vi.fn(),
9+
writeWorkflowGroupState: vi.fn(),
10+
batchEnqueueAndWait: vi.fn(),
11+
}))
12+
13+
vi.mock('@/lib/table/events', () => ({ appendTableEvent: vi.fn() }))
14+
vi.mock('@/lib/table/service', () => ({ getTableById: mocks.getTableById }))
15+
vi.mock('@/lib/table/cell-write', () => ({
16+
writeWorkflowGroupState: mocks.writeWorkflowGroupState,
17+
}))
18+
vi.mock('@/lib/billing/core/billing-attribution', () => ({
19+
assertBillingAttributionSnapshot: (snapshot: unknown) => snapshot,
20+
resolveBillingAttribution: async () => ({ actorUserId: 'billing-owner' }),
21+
resolveSystemBillingAttribution: async () => ({ actorUserId: null }),
22+
}))
23+
vi.mock('@/lib/core/async-jobs/config', () => ({
24+
getJobQueue: async () => ({ batchEnqueueAndWait: mocks.batchEnqueueAndWait }),
25+
}))
26+
27+
import { dispatcherStep } from '@/lib/table/dispatcher'
28+
29+
const GROUP = { id: 'group-1', workflowId: 'workflow-1', outputs: [] }
30+
31+
const DISPATCH = {
32+
id: 'tdsp_1',
33+
tableId: 'table-1',
34+
workspaceId: 'workspace-1',
35+
requestId: 'req-1',
36+
mode: 'incomplete',
37+
scope: { groupIds: ['group-1'] },
38+
status: 'dispatching',
39+
cursor: -1,
40+
limit: null,
41+
processedCount: 0,
42+
isManualRun: true,
43+
triggeredByUserId: 'billing-owner',
44+
capabilityGovernedUserId: 'requesting-member',
45+
requestedAt: new Date('2026-08-21T15:00:00.000Z'),
46+
completedAt: null,
47+
cancelledAt: null,
48+
}
49+
50+
describe('the dispatcher pre-stamp', () => {
51+
beforeEach(() => {
52+
vi.clearAllMocks()
53+
resetDbChainMock()
54+
mocks.getTableById.mockResolvedValue({
55+
id: 'table-1',
56+
workspaceId: 'workspace-1',
57+
schema: { columns: [], workflowGroups: [GROUP] },
58+
})
59+
mocks.writeWorkflowGroupState.mockResolvedValue('wrote')
60+
dbChainMockFns.limit
61+
.mockResolvedValueOnce([DISPATCH])
62+
.mockResolvedValueOnce([{ id: 'row-1', tableId: 'table-1', position: 0, data: {} }])
63+
.mockResolvedValueOnce([DISPATCH])
64+
})
65+
66+
/**
67+
* The marker outlives its own worker: a cell task that finds the row's
68+
* cascade lock held bails, and the lock owner drains the marker. Without the
69+
* subject on the stamp, that drain runs the request under the owner's
70+
* subject — a different dispatch, often an ungated auto-fire.
71+
*/
72+
it('stamps the dispatch’s governed subject onto every cell it queues', async () => {
73+
await dispatcherStep('tdsp_1')
74+
75+
expect(mocks.writeWorkflowGroupState).toHaveBeenCalledWith(
76+
expect.anything(),
77+
expect.objectContaining({
78+
executionState: expect.objectContaining({
79+
status: 'pending',
80+
capabilityGovernedUserId: 'requesting-member',
81+
}),
82+
})
83+
)
84+
})
85+
})

apps/sim/lib/table/rows/executions.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,48 @@ describe('writeExecutionsPatch guards', () => {
3333
resetDbChainMock()
3434
})
3535

36+
/**
37+
* The dispatcher's `pending` marker is drained by whichever worker owns the
38+
* row's cascade lock, which may belong to another dispatch entirely. Storing
39+
* the requesting subject with the marker is what lets that drain run under
40+
* the person who asked rather than under the owner's own subject.
41+
*/
42+
it('persists the pre-stamp’s governed subject on both the insert and the upsert', async () => {
43+
await writeExecutionsPatch(
44+
dbChainMock.db as unknown as Parameters<typeof writeExecutionsPatch>[0],
45+
'table-1',
46+
'row-1',
47+
{
48+
'group-1': {
49+
...EXECUTION_STATE,
50+
status: 'pending',
51+
executionId: null,
52+
capabilityGovernedUserId: 'requesting-member',
53+
},
54+
}
55+
)
56+
57+
const values = dbChainMockFns.values.mock.calls[0]?.[0] as Record<string, unknown>
58+
expect(values.capabilityGovernedUserId).toBe('requesting-member')
59+
const conflict = dbChainMockFns.onConflictDoUpdate.mock.calls[0]?.[0] as {
60+
set: Record<string, unknown>
61+
}
62+
expect(conflict.set.capabilityGovernedUserId).toBe('requesting-member')
63+
})
64+
65+
/** A write that names no subject clears it — only an unclaimed marker is read. */
66+
it('writes null for a state that carries no subject', async () => {
67+
await writeExecutionsPatch(
68+
dbChainMock.db as unknown as Parameters<typeof writeExecutionsPatch>[0],
69+
'table-1',
70+
'row-1',
71+
{ 'group-1': EXECUTION_STATE }
72+
)
73+
74+
const values = dbChainMockFns.values.mock.calls[0]?.[0] as Record<string, unknown>
75+
expect(values.capabilityGovernedUserId).toBeNull()
76+
})
77+
3678
it('rejects a worker write when the atomic stale-or-cancel predicate returns no row', async () => {
3779
dbChainMockFns.returning.mockResolvedValueOnce([])
3880

0 commit comments

Comments
 (0)