Skip to content

Commit ba4beb6

Browse files
fix(tables): revalidate sync imports under the advisory lock, explain every blocked key
- The sync append/replace paths asserted only the request-start snapshot, so a lock committed while the CSV was parsed still let the write through. Both now re-read under the schema advisory lock at the top of their own transaction, taken before acquireRowOrderLock so the order stays advisory -> rows_pos -> definitions - Delete/Backspace, Cmd+D, typeahead and cut raised no notice on an update-locked table; they now explain the lock, and stay silent for users without write access - The multipart import fetch threw a plain Error, dropping the 423 status, so the lock self-heal never ran and the stale detail cache survived. It throws ApiClientError now and both import-into-table hooks call handleTableLockRejection - Force append at submit when the delete lock landed while the dialog was open with Replace already selected
1 parent 7ca070b commit ba4beb6

4 files changed

Lines changed: 76 additions & 16 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,7 +2209,12 @@ export function TableGrid({
22092209
!rowSelectionIsEmpty(rowSelectionRef.current)
22102210
) {
22112211
if (editingCellRef.current) return
2212-
if (!canEditCellRef.current) return
2212+
if (!canEditCellRef.current) {
2213+
// Explain the lock rather than no-op, but only for users who
2214+
// could otherwise edit — for the rest it is a permission limit.
2215+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2216+
return
2217+
}
22132218
e.preventDefault()
22142219
const rowSel = rowSelectionRef.current
22152220
void (async () => {
@@ -2419,7 +2424,12 @@ export function TableGrid({
24192424

24202425
if ((e.metaKey || e.ctrlKey) && e.key === 'd') {
24212426
e.preventDefault()
2422-
if (!canEditCellRef.current) return
2427+
if (!canEditCellRef.current) {
2428+
// Explain the lock rather than no-op, but only for users who
2429+
// could otherwise edit — for the rest it is a permission limit.
2430+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2431+
return
2432+
}
24232433
const sel = computeNormalizedSelection(anchor, selectionFocusRef.current)
24242434
if (!sel || sel.startRow === sel.endRow) return
24252435
const sourceRow = currentRows[sel.startRow]
@@ -2453,7 +2463,12 @@ export function TableGrid({
24532463
}
24542464

24552465
if (e.key === 'Delete' || e.key === 'Backspace') {
2456-
if (!canEditCellRef.current) return
2466+
if (!canEditCellRef.current) {
2467+
// Explain the lock rather than no-op, but only for users who
2468+
// could otherwise edit — for the rest it is a permission limit.
2469+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2470+
return
2471+
}
24572472
e.preventDefault()
24582473
const sel = computeNormalizedSelection(anchor, selectionFocusRef.current)
24592474
if (!sel) return
@@ -2514,7 +2529,12 @@ export function TableGrid({
25142529
}
25152530

25162531
if (e.key.length === 1 && !e.metaKey && !e.ctrlKey && !e.altKey) {
2517-
if (!canEditCellRef.current) return
2532+
if (!canEditCellRef.current) {
2533+
// Explain the lock rather than no-op, but only for users who
2534+
// could otherwise edit — for the rest it is a permission limit.
2535+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2536+
return
2537+
}
25182538
const col = cols[anchor.colIndex]
25192539
// Workflow-output cells are editable: the user can override the
25202540
// workflow's value if they want. Booleans toggle on space/click —
@@ -2714,7 +2734,12 @@ export function TableGrid({
27142734
const tag = (e.target as HTMLElement).tagName
27152735
if (tag === 'INPUT' || tag === 'TEXTAREA') return
27162736
if (editingCellRef.current) return
2717-
if (!canEditCellRef.current) return
2737+
if (!canEditCellRef.current) {
2738+
// Explain the lock rather than no-op, but only for users who
2739+
// could otherwise edit — for the rest it is a permission limit.
2740+
if (canEditRef.current) onBlockedActionRef.current('edit-cell')
2741+
return
2742+
}
27182743

27192744
const rowSel = rowSelectionRef.current
27202745
const cols = columnsRef.current

apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ export function ImportCsvDialog({
314314
async function handleSubmit() {
315315
if (!parsed || !canSubmit) return
316316
setSubmitError(null)
317+
// Hiding the Replace control doesn't clear an already-selected `mode`, so a
318+
// delete lock landing while the dialog is open would still submit replace.
319+
const effectiveMode: CsvImportMode = canReplace ? mode : 'append'
317320
const createColumns =
318321
canCreateColumns && createHeaders.size > 0 ? [...createHeaders] : undefined
319322

@@ -334,7 +337,7 @@ export function ImportCsvDialog({
334337
workspaceId,
335338
tableId: table.id,
336339
file: parsed.file,
337-
mode,
340+
mode: effectiveMode,
338341
mapping,
339342
createColumns,
340343
onProgress: (percent) => {
@@ -365,12 +368,12 @@ export function ImportCsvDialog({
365368
workspaceId,
366369
tableId: table.id,
367370
file: parsed.file,
368-
mode,
371+
mode: effectiveMode,
369372
mapping,
370373
createColumns,
371374
})
372375
const data = result.data
373-
if (mode === 'append') {
376+
if (effectiveMode === 'append') {
374377
toast.success(`Imported ${data?.insertedCount ?? 0} rows into "${table.name}"`)
375378
} else {
376379
toast.success(

apps/sim/hooks/queries/tables.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from '@tanstack/react-query'
1818
import { useRouter } from 'next/navigation'
1919
import {
20+
ApiClientError,
2021
extractValidationIssues,
2122
isApiClientError,
2223
isValidationError,
@@ -1532,7 +1533,13 @@ export function useUploadCsvToTable() {
15321533

15331534
if (!response.ok) {
15341535
const data = await response.json().catch(() => ({}))
1535-
throw new Error(data.error || 'CSV import failed')
1536+
// Carry the status: a plain Error drops it, and the 423 self-heal below
1537+
// keys off `error.status`.
1538+
throw new ApiClientError({
1539+
status: response.status,
1540+
body: data,
1541+
message: data.error || 'CSV import failed',
1542+
})
15361543
}
15371544

15381545
return response.json()
@@ -1668,7 +1675,8 @@ export function useImportCsvIntoTableAsync() {
16681675
})
16691676
return response.data
16701677
},
1671-
onError: (error) => {
1678+
onError: (error, variables) => {
1679+
if (handleTableLockRejection(error, queryClient, variables.tableId)) return
16721680
logger.error('Failed to start async CSV import:', error)
16731681
toast.error(error.message, { duration: 5000 })
16741682
},
@@ -1747,7 +1755,8 @@ export function useImportCsvIntoTable() {
17471755

17481756
return response.json()
17491757
},
1750-
onError: (error) => {
1758+
onError: (error, variables) => {
1759+
if (handleTableLockRejection(error, queryClient, variables.tableId)) return
17511760
logger.error('Failed to import CSV into table:', error)
17521761
toast.error(error.message, { duration: 5000 })
17531762
},

apps/sim/lib/table/import-data.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing'
1313
import { CSV_MAX_BATCH_SIZE } from '@/lib/table/import'
1414
import { assertRowDelete, assertRowInsert, assertSchemaMutable } from '@/lib/table/mutation-locks'
1515
import { nKeysBetween } from '@/lib/table/order-key'
16+
import type { DbTransaction } from '@/lib/table/planner'
1617
import {
1718
acquireRowOrderLock,
1819
guardBatch,
1920
type MutationRevalidator,
2021
} from '@/lib/table/rows/ordering'
2122
import { batchInsertRowsWithTx, replaceTableRowsWithTx } from '@/lib/table/rows/service'
22-
import { addTableColumnsWithTx, auditTableColumnsAdded } from '@/lib/table/service'
23+
import { addTableColumnsWithTx, auditTableColumnsAdded, getTableById } from '@/lib/table/service'
2324
import type {
2425
ReplaceRowsResult,
2526
RowData,
@@ -187,6 +188,28 @@ export async function setTableSchemaForImport(
187188
})
188189
}
189190

191+
/**
192+
* Re-reads the table under its schema advisory lock inside the caller's
193+
* transaction. The sync import paths own their transaction rather than taking a
194+
* revalidator, so they refresh here: a lock committed while the CSV was being
195+
* parsed must be visible to the asserts in `addTableColumnsWithTx` /
196+
* `batchInsertRowsWithTx` / `replaceTableRowsWithTx`, which all read the
197+
* definition they are handed.
198+
*
199+
* Taken before `acquireRowOrderLock` so the order stays advisory → rows_pos →
200+
* definitions, matching every other advisory-lock holder.
201+
*/
202+
async function refreshUnderLock(
203+
trx: DbTransaction,
204+
table: TableDefinition
205+
): Promise<TableDefinition> {
206+
const fresh = await guardBatch(trx, table.id, async (tx) => {
207+
const latest = await getTableById(table.id, { tx, includeArchived: true })
208+
return latest ?? undefined
209+
})
210+
return fresh ?? table
211+
}
212+
190213
/**
191214
* Owns the append-import transaction so the API route never holds a `trx`:
192215
* optionally creates the new columns, then inserts every row in CSV-sized
@@ -206,15 +229,15 @@ export async function importAppendRows(
206229
addedRows: rows.length,
207230
})
208231
const result = await db.transaction(async (trx) => {
209-
let working = table
232+
let working = await refreshUnderLock(trx, table)
210233
if (additions.length > 0) {
211234
// Take the row-order lock before creating columns so this path uses the
212235
// same rows_pos → user_table_definitions order as plain inserts. Creating
213236
// columns first would lock the definition row before rows_pos, inverting
214237
// the order and deadlocking concurrent inserts on this table. The lock is
215238
// re-entrant, so the per-batch acquire below is a no-op.
216239
await acquireRowOrderLock(trx, table.id)
217-
working = await addTableColumnsWithTx(trx, table, additions, ctx.requestId)
240+
working = await addTableColumnsWithTx(trx, working, additions, ctx.requestId)
218241
}
219242
const inserted: TableRow[] = []
220243
for (let i = 0; i < rows.length; i += CSV_MAX_BATCH_SIZE) {
@@ -264,10 +287,10 @@ export async function importReplaceRows(
264287
addedRows: data.rows.length,
265288
})
266289
const result = await db.transaction(async (trx) => {
267-
let working = table
290+
let working = await refreshUnderLock(trx, table)
268291
if (additions.length > 0) {
269292
await acquireRowOrderLock(trx, table.id)
270-
working = await addTableColumnsWithTx(trx, table, additions, requestId)
293+
working = await addTableColumnsWithTx(trx, working, additions, requestId)
271294
}
272295
return replaceTableRowsWithTx(
273296
trx,

0 commit comments

Comments
 (0)