Skip to content

Commit 8a3c289

Browse files
committed
fix(table): carry the acting person into the backfill's downstream cascade
An output backfill writes cells that satisfy downstream groups' deps, and `batchUpdateRows` starts those groups. The write passed an explicit null governed subject, so a cascade started by a person's schema change ran its tools ungated. The subject now rides the backfill payload beside `actorUserId` — a billing attribution that names the workspace billed account when the change carried no human.
1 parent ee40593 commit 8a3c289

5 files changed

Lines changed: 180 additions & 9 deletions

File tree

apps/sim/lib/table/application/groups.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,29 @@ describe('workflow and enrichment Table application commands', () => {
290290
expect(mocks.signal).toHaveBeenCalledWith(table.id)
291291
})
292292

293+
/**
294+
* Adding an output backfills it from saved runs, and a backfilled cell can
295+
* satisfy a downstream group's deps and start it. That cascade is gated on
296+
* the acting person, which is not the billing attribution beside it.
297+
*/
298+
it('names the acting person, not the billing actor, as the backfill cascade subject', async () => {
299+
await addWorkflowTableGroupOutput.execute({
300+
principal,
301+
input: {
302+
tableId: table.id,
303+
workspaceId: table.workspaceId,
304+
groupId: group.id,
305+
blockId: 'block-2',
306+
path: 'score',
307+
},
308+
})
309+
310+
expect(mocks.addOutput).toHaveBeenCalledWith(
311+
expect.objectContaining({ capabilityGovernedUserId: 'user-1' }),
312+
'request-1'
313+
)
314+
})
315+
293316
it('persists disabled auto-run on a newly created workflow group', async () => {
294317
const result = await createWorkflowTableGroup.execute({
295318
principal,

apps/sim/lib/table/application/groups.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,7 @@ export const addWorkflowTableGroupOutput = defineAuthorizedTableUseCase({
11491149
actorUserId: resolvePrincipalAttribution(principal, {
11501150
workspaceBillingOwnerUserId: context.billedAccountUserId,
11511151
}).attributedUserId,
1152+
capabilityGovernedUserId: capabilityGovernedPrincipalUserId(principal),
11521153
resolvedOutput: {
11531154
workflowId: resolvedWorkflow.workflowId,
11541155
columnType: columnTypeForLeaf(output.leafType),
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { tableRowExecutions, userTableRows, workflowExecutionLogs } from '@sim/db/schema'
6+
import { queueTableRows, resetDbChainMock } from '@sim/testing'
7+
import { beforeEach, describe, expect, it, vi } from 'vitest'
8+
import type { TableDefinition } from '@/lib/table/types'
9+
10+
const { mockBatchUpdateRows, mockMaterializeExecutionData, mockGetFunctionalBlockOutput } =
11+
vi.hoisted(() => ({
12+
mockBatchUpdateRows: vi.fn(),
13+
mockMaterializeExecutionData: vi.fn(),
14+
mockGetFunctionalBlockOutput: vi.fn(),
15+
}))
16+
17+
vi.mock('@/lib/table/rows/service', () => ({
18+
batchUpdateRows: mockBatchUpdateRows,
19+
}))
20+
vi.mock('@/lib/logs/execution/trace-store', () => ({
21+
materializeExecutionData: mockMaterializeExecutionData,
22+
}))
23+
vi.mock('@/lib/logs/execution/functional-outputs', () => ({
24+
getFunctionalBlockOutput: mockGetFunctionalBlockOutput,
25+
}))
26+
vi.mock('@/lib/table/rows/secret-provenance', () => ({
27+
createTableRowSecretProvenanceFromRegistry: () => ({ complete: true, columns: {} }),
28+
}))
29+
30+
import { maybeBackfillGroupOutputs } from '@/lib/table/backfill-runner'
31+
32+
const TABLE = {
33+
id: 'table-1',
34+
workspaceId: 'workspace-1',
35+
schema: { columns: [], workflowGroups: [] },
36+
} as unknown as TableDefinition
37+
38+
/** Queues the four reads one inline backfill page makes, in the order it makes them. */
39+
function queueOnePage(): void {
40+
queueTableRows(tableRowExecutions, [{ count: 1 }])
41+
queueTableRows(tableRowExecutions, [{ rowId: 'row-1', executionId: 'execution-1' }])
42+
queueTableRows(userTableRows, [{ id: 'row-1', data: {} }])
43+
queueTableRows(workflowExecutionLogs, [
44+
{
45+
executionId: 'execution-1',
46+
workflowId: 'workflow-1',
47+
workspaceId: 'workspace-1',
48+
executionData: {},
49+
},
50+
])
51+
queueTableRows(tableRowExecutions, [])
52+
}
53+
54+
describe('backfill cascade governance', () => {
55+
beforeEach(() => {
56+
vi.clearAllMocks()
57+
resetDbChainMock()
58+
mockMaterializeExecutionData.mockResolvedValue({})
59+
mockGetFunctionalBlockOutput.mockReturnValue({ value: 'filled' })
60+
mockBatchUpdateRows.mockResolvedValue({ affectedCount: 1, affectedRowIds: ['row-1'] })
61+
})
62+
63+
/**
64+
* A backfilled cell is a dependency: `batchUpdateRows` starts every downstream
65+
* group whose deps it just satisfied. Passing no subject there ran those
66+
* cells with no per-tool gate, which is what `null` means on this field.
67+
*/
68+
it('cascades under the person who made the schema change', async () => {
69+
queueOnePage()
70+
71+
await maybeBackfillGroupOutputs({
72+
table: TABLE,
73+
groupId: 'group-1',
74+
outputs: [{ blockId: 'block-1', path: 'value', columnName: 'value' }],
75+
overwrite: true,
76+
requestId: 'request-1',
77+
actorUserId: 'billed-owner',
78+
capabilityGovernedUserId: 'member-1',
79+
})
80+
81+
expect(mockBatchUpdateRows).toHaveBeenCalledWith(
82+
expect.objectContaining({
83+
actorUserId: 'billed-owner',
84+
capabilityGovernedUserId: 'member-1',
85+
}),
86+
expect.anything(),
87+
expect.anything(),
88+
expect.anything()
89+
)
90+
})
91+
92+
/** A change with no acting person still names one explicitly. */
93+
it('keeps an absent subject null rather than borrowing the billing actor', async () => {
94+
queueOnePage()
95+
96+
await maybeBackfillGroupOutputs({
97+
table: TABLE,
98+
groupId: 'group-1',
99+
outputs: [{ blockId: 'block-1', path: 'value', columnName: 'value' }],
100+
overwrite: true,
101+
requestId: 'request-1',
102+
actorUserId: 'billed-owner',
103+
})
104+
105+
expect(mockBatchUpdateRows).toHaveBeenCalledWith(
106+
expect.objectContaining({ capabilityGovernedUserId: null }),
107+
expect.anything(),
108+
expect.anything(),
109+
expect.anything()
110+
)
111+
})
112+
})

apps/sim/lib/table/backfill-runner.ts

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ export interface TableBackfillPayload {
5656
overwrite: boolean
5757
/** User who triggered the schema change, for usage attribution on the row writes. */
5858
actorUserId?: string | null
59+
/**
60+
* Person whose permission group gates any cell the backfill's writes cascade
61+
* into. Separate from `actorUserId`, which is a billing attribution and names
62+
* the workspace billed account when the schema change carried no human. Null
63+
* when the change had no acting person; absent on payloads enqueued before
64+
* this field existed, which read as null — the pre-existing behavior.
65+
*/
66+
capabilityGovernedUserId?: string | null
5967
}
6068

6169
/**
@@ -136,8 +144,11 @@ async function processBackfillPage(opts: {
136144
execs: Array<{ rowId: string; executionId: string | null }>
137145
requestId: string
138146
actorUserId?: string | null
147+
/** See {@link TableBackfillPayload.capabilityGovernedUserId}. */
148+
capabilityGovernedUserId?: string | null
139149
}): Promise<number> {
140-
const { table, outputs, overwrite, execs, requestId, actorUserId } = opts
150+
const { table, outputs, overwrite, execs, requestId, actorUserId, capabilityGovernedUserId } =
151+
opts
141152

142153
const executionIdsByRow = new Map<string, string>()
143154
for (const e of execs) {
@@ -225,11 +236,14 @@ async function processBackfillPage(opts: {
225236
workspaceId: table.workspaceId,
226237
actorUserId,
227238
/**
228-
* A backfill replays values already produced by earlier runs; it starts
229-
* no enrichment of its own and carries no acting person into this
230-
* background pass.
239+
* A backfill replays values already produced by earlier runs, but the
240+
* cells it fills are dependencies: `batchUpdateRows` starts every
241+
* downstream group whose deps just became satisfied. Those cells are
242+
* governed by whoever made the schema change, carried separately from
243+
* `actorUserId` — an attribution that names the workspace billed account
244+
* when the change carried no human, whose denylist is nobody's to run.
231245
*/
232-
capabilityGovernedUserId: null,
246+
capabilityGovernedUserId: capabilityGovernedUserId ?? null,
233247
secretProvenanceByRowId,
234248
},
235249
table,
@@ -248,7 +262,8 @@ async function processBackfillPage(opts: {
248262
* passes skip already-filled cells).
249263
*/
250264
export async function runTableBackfill(payload: TableBackfillPayload): Promise<void> {
251-
const { jobId, tableId, groupId, outputs, overwrite, actorUserId } = payload
265+
const { jobId, tableId, groupId, outputs, overwrite, actorUserId, capabilityGovernedUserId } =
266+
payload
252267
const requestId = generateId().slice(0, 8)
253268

254269
try {
@@ -274,6 +289,7 @@ export async function runTableBackfill(payload: TableBackfillPayload): Promise<v
274289
execs,
275290
requestId,
276291
actorUserId,
292+
capabilityGovernedUserId,
277293
})
278294
processed += execs.length
279295
}
@@ -328,8 +344,11 @@ export async function maybeBackfillGroupOutputs(opts: {
328344
overwrite: boolean
329345
requestId: string
330346
actorUserId?: string | null
347+
/** See {@link TableBackfillPayload.capabilityGovernedUserId}. */
348+
capabilityGovernedUserId?: string | null
331349
}): Promise<void> {
332-
const { table, groupId, outputs, overwrite, requestId, actorUserId } = opts
350+
const { table, groupId, outputs, overwrite, requestId, actorUserId, capabilityGovernedUserId } =
351+
opts
333352
if (outputs.length === 0) return
334353

335354
const [{ count: completedCount }] = await db
@@ -353,7 +372,15 @@ export async function maybeBackfillGroupOutputs(opts: {
353372
const execs = await selectCompletedExecPage(table.id, groupId, afterRowId, BACKFILL_PAGE_SIZE)
354373
if (execs.length === 0) break
355374
afterRowId = execs[execs.length - 1].rowId
356-
await processBackfillPage({ table, outputs, overwrite, execs, requestId, actorUserId })
375+
await processBackfillPage({
376+
table,
377+
outputs,
378+
overwrite,
379+
execs,
380+
requestId,
381+
actorUserId,
382+
capabilityGovernedUserId,
383+
})
357384
}
358385
return
359386
}
@@ -376,6 +403,7 @@ export async function maybeBackfillGroupOutputs(opts: {
376403
outputs,
377404
overwrite,
378405
actorUserId,
406+
capabilityGovernedUserId,
379407
}
380408
if (isTriggerDevEnabled) {
381409
try {

apps/sim/lib/table/workflow-groups/service.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ export async function updateWorkflowGroup(
571571
overwrite: false,
572572
requestId,
573573
actorUserId: data.actorUserId,
574+
capabilityGovernedUserId: data.capabilityGovernedUserId,
574575
})
575576
} catch (err) {
576577
logger.warn(
@@ -589,6 +590,7 @@ export async function updateWorkflowGroup(
589590
overwrite: true,
590591
requestId,
591592
actorUserId: data.actorUserId,
593+
capabilityGovernedUserId: data.capabilityGovernedUserId,
592594
})
593595
} catch (err) {
594596
logger.warn(
@@ -636,8 +638,12 @@ export async function addWorkflowGroupOutput(
636638
path: string
637639
/** Optional override; defaults to a slug derived from `path`. */
638640
columnName?: string
639-
/** The member adding the output — billed/gated for any backfill-triggered re-run. */
641+
/** The member adding the output — the billing attribution for the backfill's
642+
* row writes. Not the gate: see `capabilityGovernedUserId`. */
640643
actorUserId?: string | null
644+
/** Person whose permission group gates any cell the backfill's writes
645+
* cascade into; null when the change has no acting person. */
646+
capabilityGovernedUserId?: string | null
641647
resolvedOutput: {
642648
workflowId: string
643649
columnType: ColumnDefinition['type']
@@ -869,6 +875,7 @@ export async function addWorkflowGroupOutput(
869875
overwrite: false,
870876
requestId,
871877
actorUserId: data.actorUserId,
878+
capabilityGovernedUserId: data.capabilityGovernedUserId,
872879
})
873880
} catch (err) {
874881
logger.warn(

0 commit comments

Comments
 (0)