Skip to content

Commit 910f133

Browse files
fix(tables): preserve run validation and signals
1 parent 3c2538c commit 910f133

8 files changed

Lines changed: 151 additions & 40 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import {
8484
queryTableRows,
8585
replaceTableRows,
8686
TableRowsValidationError,
87+
tablePredicateNamesToFilter,
8788
upsertTableRow,
8889
} from '@/lib/table/application/rows'
8990

@@ -104,6 +105,19 @@ const TABLE: TableDefinition = {
104105

105106
const PRINCIPAL = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' }
106107

108+
describe('table predicate translation', () => {
109+
it('maps invalid run filters to the shared row validation error', () => {
110+
expect(() =>
111+
tablePredicateNamesToFilter({ all: [{ field: 'missing', op: 'eq', value: 'ready' }] }, TABLE)
112+
).toThrowError(
113+
expect.objectContaining({
114+
name: 'TableRowsValidationError',
115+
details: { code: 'INVALID_FILTER' },
116+
})
117+
)
118+
})
119+
})
120+
107121
describe('replaceTableRows application use case', () => {
108122
beforeEach(() => {
109123
vi.clearAllMocks()

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,14 @@ export function tablePredicateNamesToFilter(
9999
predicate: TablePredicate,
100100
table: TableDefinition
101101
): Filter {
102-
validatePredicateShape(predicate)
103-
const translated = predicateToStorage(predicate, table.schema)
104-
validateStoragePredicate(translated, table.schema.columns)
105-
return predicateToFilter(translated)
102+
try {
103+
validatePredicateShape(predicate)
104+
const translated = predicateToStorage(predicate, table.schema)
105+
validateStoragePredicate(translated, table.schema.columns)
106+
return predicateToFilter(translated)
107+
} catch (error) {
108+
rethrowQueryValidation(error)
109+
}
106110
}
107111

108112
async function throwValidationResponse(

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,10 @@ describe('table run application use cases', () => {
104104
billedAccountUserId: 'billing-owner-1',
105105
})
106106
mockGetRowById.mockResolvedValue({ id: 'row-1' })
107-
mockRunWorkflowColumn.mockResolvedValue({ dispatchId: 'dispatch-1' })
107+
mockRunWorkflowColumn.mockResolvedValue({
108+
dispatchId: 'dispatch-1',
109+
shouldSignalRowsChanged: true,
110+
})
108111
mockRequireTableRowIds.mockResolvedValue(undefined)
109112
mockCancelRuns.mockResolvedValue(1)
110113
mockTranslatePredicate.mockReturnValue({ all: [] })
@@ -200,7 +203,10 @@ describe('table run application use cases', () => {
200203
})
201204

202205
it('does not signal when the dispatcher reports a no-op', async () => {
203-
mockRunWorkflowColumn.mockResolvedValue({ dispatchId: null })
206+
mockRunWorkflowColumn.mockResolvedValue({
207+
dispatchId: null,
208+
shouldSignalRowsChanged: false,
209+
})
204210

205211
await startTableRun.execute({
206212
principal: PRINCIPAL,
@@ -215,6 +221,25 @@ describe('table run application use cases', () => {
215221
expect(mockSignalRowsChanged).not.toHaveBeenCalled()
216222
})
217223

224+
it('signals a cleared row state when cancellation wins before dispatch', async () => {
225+
mockRunWorkflowColumn.mockResolvedValue({
226+
dispatchId: null,
227+
shouldSignalRowsChanged: true,
228+
})
229+
230+
await startTableRun.execute({
231+
principal: PRINCIPAL,
232+
input: {
233+
kind: 'selection',
234+
tableId: TABLE.id,
235+
groupIds: ['group-1'],
236+
mode: 'all',
237+
},
238+
})
239+
240+
expect(mockSignalRowsChanged).toHaveBeenCalledWith(TABLE.id)
241+
})
242+
218243
it('requires a canonical row for row cancellation', async () => {
219244
mockGetRowById.mockResolvedValue(null)
220245

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export type StartTableRunInput = StartSelectionRunInput | StartRowEnrichmentInpu
6161

6262
export interface StartTableRunResult extends TableRunResult {
6363
dispatchId: string | null
64+
shouldSignalRowsChanged: boolean
6465
}
6566

6667
function requireCanonicalGroups(table: TableDefinition, groupIds: string[]): void {
@@ -96,7 +97,11 @@ export const startTableRun = defineAuthorizedTableUseCase({
9697
requestId: requestId(input),
9798
triggeredByUserId,
9899
})
99-
return { table: context.table, dispatchId: result.dispatchId }
100+
return {
101+
table: context.table,
102+
dispatchId: result.dispatchId,
103+
shouldSignalRowsChanged: result.shouldSignalRowsChanged,
104+
}
100105
}
101106

102107
if (input.rowIds && input.predicate) {
@@ -149,10 +154,14 @@ export const startTableRun = defineAuthorizedTableUseCase({
149154
requestId: requestId(input),
150155
triggeredByUserId,
151156
})
152-
return { table: context.table, dispatchId: result.dispatchId }
157+
return {
158+
table: context.table,
159+
dispatchId: result.dispatchId,
160+
shouldSignalRowsChanged: result.shouldSignalRowsChanged,
161+
}
153162
},
154163
afterSuccess: ({ context, result }) => {
155-
if (result.dispatchId !== null) signalTableRowsChanged(context.tableId)
164+
if (result.shouldSignalRowsChanged) signalTableRowsChanged(context.tableId)
156165
},
157166
})
158167

apps/sim/lib/table/dispatcher.ts

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { writeWorkflowGroupState } from '@/lib/table/cell-write'
2121
import { USER_TABLE_ROWS_SQL_NAME } from '@/lib/table/constants'
2222
import { isExecCancelledAfter } from '@/lib/table/deps'
2323
import { appendTableEvent } from '@/lib/table/events'
24-
import { type DbExecutor, withSeqscanOff } from '@/lib/table/planner'
24+
import { type DbExecutor, type DbTransaction, withSeqscanOff } from '@/lib/table/planner'
2525
import { updateTableRowsWithDerivedSecretProvenance } from '@/lib/table/rows/secret-provenance'
2626
import { buildFilterClause } from '@/lib/table/sql'
2727
import type {
@@ -87,6 +87,24 @@ export interface DispatchRow {
8787
requestedAt: Date
8888
}
8989

90+
async function deleteExecutionRows(trx: DbTransaction, filters: SQL[]): Promise<number> {
91+
const countRows = await trx.execute<{ count: number | string }>(sql`
92+
WITH deleted AS (
93+
DELETE FROM ${tableRowExecutions}
94+
WHERE ${and(...filters)}
95+
RETURNING 1
96+
)
97+
SELECT count(*)::integer AS count FROM deleted
98+
`)
99+
const [countRow] = Array.isArray(countRows) ? countRows : []
100+
if (!countRow) throw new Error('Workflow cell clearing did not return a deleted count')
101+
const count = Number(countRow.count)
102+
if (!Number.isSafeInteger(count) || count < 0) {
103+
throw new Error('Workflow cell clearing returned an invalid deleted count')
104+
}
105+
return count
106+
}
107+
90108
export type DispatcherStepResult = 'continue' | 'done'
91109

92110
/** Eager bulk clear at click time so the user sees every targeted cell go
@@ -96,17 +114,18 @@ export type DispatcherStepResult = 'continue' | 'done'
96114
* already filled, mirroring the eligibility predicate. */
97115
export async function bulkClearWorkflowGroupCells(input: {
98116
tableId: string
117+
workspaceId: string
99118
groups: Array<{ id: string; outputs: Array<{ columnName: string }> }>
100119
rowIds?: string[]
101120
/** Select-all scope: deselected rows whose outputs must NOT be wiped. */
102121
excludeRowIds?: string[]
103122
mode: DispatchMode
104-
}): Promise<void> {
105-
const { tableId, groups, rowIds, excludeRowIds, mode } = input
106-
if (groups.length === 0) return
123+
}): Promise<boolean> {
124+
const { tableId, workspaceId, groups, rowIds, excludeRowIds, mode } = input
125+
if (groups.length === 0) return false
107126
// `'new'` mode targets only rows with no prior attempt — nothing to clear.
108127
// Pre-existing outputs on any other row must not be wiped by an auto-fire.
109-
if (mode === 'new') return
128+
if (mode === 'new') return false
110129

111130
const groupIds = groups.map((g) => g.id)
112131
const rowScope = rowIds && rowIds.length > 0 ? rowIds : null
@@ -119,25 +138,34 @@ export async function bulkClearWorkflowGroupCells(input: {
119138
const outputCols = Array.from(
120139
new Set(groups.flatMap((g) => g.outputs.map((o) => o.columnName)))
121140
)
122-
const filters: SQL[] = [eq(userTableRows.tableId, tableId)]
141+
const filters: SQL[] = [
142+
eq(userTableRows.tableId, tableId),
143+
eq(userTableRows.workspaceId, workspaceId),
144+
]
123145
if (rowScope) filters.push(inArray(userTableRows.id, rowScope))
124146
if (excluded) filters.push(notInArray(userTableRows.id, excluded))
125147

126-
await db.transaction(async (trx) => {
148+
return db.transaction(async (trx) => {
127149
const rowWhere = and(...filters)!
128-
await updateTableRowsWithDerivedSecretProvenance(trx, {
150+
const clearedRows = await updateTableRowsWithDerivedSecretProvenance(trx, {
129151
rowWhere,
130152
transformation: { mode: 'remove-columns', columnIds: outputCols },
131153
})
132154
const execFilters: SQL[] = [
133155
eq(tableRowExecutions.tableId, tableId),
134156
inArray(tableRowExecutions.groupId, groupIds),
157+
sql`${tableRowExecutions.rowId} IN (
158+
SELECT ${userTableRows.id}
159+
FROM ${userTableRows}
160+
WHERE ${userTableRows.tableId} = ${tableId}
161+
AND ${userTableRows.workspaceId} = ${workspaceId}
162+
)`,
135163
]
136164
if (rowScope) execFilters.push(inArray(tableRowExecutions.rowId, rowScope))
137165
if (excluded) execFilters.push(notInArray(tableRowExecutions.rowId, excluded))
138-
await trx.delete(tableRowExecutions).where(and(...execFilters))
166+
const deletedExecutions = await deleteExecutionRows(trx, execFilters)
167+
return clearedRows > 0 || deletedExecutions > 0
139168
})
140-
return
141169
}
142170

143171
// `incomplete`: clear per-group, not per-row. Only groups that are
@@ -147,20 +175,25 @@ export async function bulkClearWorkflowGroupCells(input: {
147175
// because a *sibling* group on the same row is incomplete, re-running the
148176
// completed one. (`never-run` groups have no exec/output to clear — the
149177
// dispatcher runs them via eligibility.)
150-
await db.transaction(async (trx) => {
178+
return db.transaction(async (trx) => {
179+
let rowsChanged = false
151180
for (const group of groups) {
152181
const reRunnable = sql`EXISTS (
153182
SELECT 1 FROM ${tableRowExecutions} re
154183
WHERE re.row_id = ${userTableRows.id}
155184
AND re.group_id = ${group.id}
156185
AND re.status IN ('error', 'cancelled')
157186
)`
158-
const filters: SQL[] = [eq(userTableRows.tableId, tableId), reRunnable]
187+
const filters: SQL[] = [
188+
eq(userTableRows.tableId, tableId),
189+
eq(userTableRows.workspaceId, workspaceId),
190+
reRunnable,
191+
]
159192
if (rowScope) filters.push(inArray(userTableRows.id, rowScope))
160193
if (excluded) filters.push(notInArray(userTableRows.id, excluded))
161194

162195
const rowWhere = and(...filters)!
163-
await updateTableRowsWithDerivedSecretProvenance(trx, {
196+
const clearedRows = await updateTableRowsWithDerivedSecretProvenance(trx, {
164197
rowWhere,
165198
transformation: {
166199
mode: 'remove-columns',
@@ -172,11 +205,19 @@ export async function bulkClearWorkflowGroupCells(input: {
172205
eq(tableRowExecutions.tableId, tableId),
173206
eq(tableRowExecutions.groupId, group.id),
174207
sql`${tableRowExecutions.status} IN ('error', 'cancelled')`,
208+
sql`${tableRowExecutions.rowId} IN (
209+
SELECT ${userTableRows.id}
210+
FROM ${userTableRows}
211+
WHERE ${userTableRows.tableId} = ${tableId}
212+
AND ${userTableRows.workspaceId} = ${workspaceId}
213+
)`,
175214
]
176215
if (rowScope) execFilters.push(inArray(tableRowExecutions.rowId, rowScope))
177216
if (excluded) execFilters.push(notInArray(tableRowExecutions.rowId, excluded))
178-
await trx.delete(tableRowExecutions).where(and(...execFilters))
217+
const deletedExecutions = await deleteExecutionRows(trx, execFilters)
218+
rowsChanged ||= clearedRows > 0 || deletedExecutions > 0
179219
}
220+
return rowsChanged
180221
})
181222
}
182223

apps/sim/lib/table/rows/secret-provenance.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -464,14 +464,18 @@ describe('table row secret provenance', () => {
464464
it('binds derived rows only after their matching sidecars are written', async () => {
465465
queueTableRows(userTableRows, [{ id: 'legacy-row' }])
466466

467-
await updateTableRowsWithDerivedSecretProvenance(dbChainMock.db as unknown as DbTransaction, {
468-
rowWhere: eq(userTableRows.id, 'legacy-row'),
469-
transformation: {
470-
mode: 'remove-columns',
471-
columnIds: ['deleted-column', 'deleted-column'],
472-
},
473-
})
467+
const updatedCount = await updateTableRowsWithDerivedSecretProvenance(
468+
dbChainMock.db as unknown as DbTransaction,
469+
{
470+
rowWhere: eq(userTableRows.id, 'legacy-row'),
471+
transformation: {
472+
mode: 'remove-columns',
473+
columnIds: ['deleted-column', 'deleted-column'],
474+
},
475+
}
476+
)
474477

478+
expect(updatedCount).toBe(1)
475479
expect(dbChainMockFns.execute).toHaveBeenCalledTimes(2)
476480
expect(boundArrayValues(dbChainMockFns.execute.mock.calls[0][0])).toEqual([])
477481
expect(sqlText(dbChainMockFns.execute.mock.calls[0][0])).not.toMatch(

apps/sim/lib/table/rows/secret-provenance.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,13 +434,13 @@ export async function updateTableRowsWithDerivedSecretProvenance(
434434
rowWhere: SQL
435435
transformation: DerivedTableRowTransformation
436436
}
437-
): Promise<void> {
437+
): Promise<number> {
438438
const removedColumnIds =
439439
options.transformation.mode === 'remove-columns'
440440
? [...new Set(options.transformation.columnIds)]
441441
: []
442442
if (options.transformation.mode === 'remove-columns') {
443-
if (removedColumnIds.length === 0) return
443+
if (removedColumnIds.length === 0) return 0
444444
if (
445445
removedColumnIds.length > MAX_PROVENANCE_COLUMNS_PER_ROW ||
446446
removedColumnIds.some((columnId) => columnId.length === 0)
@@ -481,6 +481,7 @@ export async function updateTableRowsWithDerivedSecretProvenance(
481481
: sql`source.provenance_entries`
482482

483483
let afterId: string | undefined
484+
let updatedCount = 0
484485
for (;;) {
485486
const page = await trx
486487
.select({ id: userTableRows.id })
@@ -491,6 +492,7 @@ export async function updateTableRowsWithDerivedSecretProvenance(
491492
.for('update')
492493
if (page.length === 0) break
493494
const rowIds = page.map((row) => row.id)
495+
updatedCount += rowIds.length
494496

495497
await trx.execute(sql`
496498
WITH source AS MATERIALIZED (
@@ -628,6 +630,7 @@ export async function updateTableRowsWithDerivedSecretProvenance(
628630
afterId = rowIds[rowIds.length - 1]
629631
if (page.length < QUERY_CHUNK_SIZE) break
630632
}
633+
return updatedCount
631634
}
632635

633636
async function readTableRowsVersion(tableId: string, workspaceId: string): Promise<number | null> {

0 commit comments

Comments
 (0)