Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apps/sim/app/api/table/[tableId]/rows/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ async function handleBatchInsert(
rows,
workspaceId: validated.workspaceId,
userId,
positions: validated.positions,
orderKeys: validated.orderKeys,
},
table,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1017,14 +1017,13 @@ export function TableGrid({
const anchorId = contextMenu.row.id
// Fractional ordering: express intent by neighbor id, not integer position.
const intent = offset === 0 ? { beforeRowId: anchorId } : { afterRowId: anchorId }
const position = contextMenu.row.position + offset
createRef.current(
{ data: {}, ...intent },
{
onSuccess: (response: Record<string, unknown>) => {
const newRowId = extractCreatedRowId(response)
if (newRowId) {
pushUndoRef.current({ type: 'create-row', rowId: newRowId, position })
pushUndoRef.current({ type: 'create-row', rowId: newRowId })
}
},
}
Expand Down Expand Up @@ -1096,7 +1095,6 @@ export function TableGrid({
const contextRow = contextMenu.row
if (!contextRow) return
const rowData = { ...contextRow.data }
const position = contextRow.position + 1
const sourceArrayIndex = rowsRef.current.findIndex((r) => r.id === contextRow.id)
closeContextMenu()
createRef.current(
Expand All @@ -1108,7 +1106,6 @@ export function TableGrid({
pushUndoRef.current({
type: 'create-row',
rowId: newRowId,
position,
data: rowData,
})
}
Expand Down Expand Up @@ -1143,11 +1140,9 @@ export function TableGrid({
onSuccess: (response: Record<string, unknown>) => {
const newRowId = extractCreatedRowId(response)
if (newRowId) {
const maxPosition = rowsRef.current.reduce((max, r) => Math.max(max, r.position), -1)
pushUndoRef.current({
type: 'create-row',
rowId: newRowId,
position: maxPosition + 1,
})
}
},
Expand Down Expand Up @@ -2187,7 +2182,7 @@ export function TableGrid({
onSuccess: (response: Record<string, unknown>) => {
const newRowId = extractCreatedRowId(response)
if (newRowId) {
pushUndoRef.current({ type: 'create-row', rowId: newRowId, position })
pushUndoRef.current({ type: 'create-row', rowId: newRowId })
}
setSelectionAnchor({ rowIndex: anchor.rowIndex + 1, colIndex })
setSelectionFocus(null)
Expand Down Expand Up @@ -2725,14 +2720,10 @@ export function TableGrid({

const currentCols = columnsRef.current
const currentRows = rowsRef.current
// Captured once before the loop so each new row in the batch gets a unique,
// sequential position via `+ (newRowIndex - currentRows.length)` below.
const lastRowPosition = currentRows.reduce((max, r) => Math.max(max, r.position), -1)

const undoCells: Array<{ rowId: string; data: Record<string, unknown> }> = []
const updateBatch: Array<{ rowId: string; data: Record<string, unknown> }> = []
const createBatchRows: Array<Record<string, unknown>> = []
const createBatchPositions: number[] = []

for (let r = 0; r < pasteRows.length; r++) {
const targetArrayIndex = currentAnchor.rowIndex + r
Expand Down Expand Up @@ -2764,7 +2755,6 @@ export function TableGrid({
updateBatch.push({ rowId: existingRow.id, data: rowData })
} else {
createBatchRows.push(rowData)
createBatchPositions.push(lastRowPosition + 1 + (targetArrayIndex - currentRows.length))
}
}

Expand All @@ -2782,20 +2772,18 @@ export function TableGrid({

if (createBatchRows.length > 0) {
batchCreateRef.current(
{ rows: createBatchRows, positions: createBatchPositions },
{ rows: createBatchRows },
{
onSuccess: (response) => {
const createdRows = response?.data?.rows ?? []
const undoRows: Array<{
rowId: string
position: number
data: Record<string, unknown>
}> = []
for (let i = 0; i < createdRows.length; i++) {
if (createdRows[i]?.id) {
undoRows.push({
rowId: createdRows[i].id,
position: createBatchPositions[i],
data: createBatchRows[i],
})
}
Expand Down
3 changes: 1 addition & 2 deletions apps/sim/hooks/queries/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ type BatchCreateTableRowsParams = Omit<BatchInsertTableRowsBodyInput, 'workspace
type BatchCreateTableRowsResponse = ContractJsonResponse<typeof batchCreateTableRowsContract>

/**
* Batch create rows in a table. Supports optional per-row positions for undo restore.
* Batch create rows in a table. Supports optional per-row order keys for undo restore.
*/
export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationContext) {
const queryClient = useQueryClient()
Expand All @@ -832,7 +832,6 @@ export function useBatchCreateTableRows({ workspaceId, tableId }: RowMutationCon
body: {
workspaceId,
rows: variables.rows as RowData[],
positions: variables.positions,
orderKeys: variables.orderKeys,
},
})
Expand Down
7 changes: 2 additions & 5 deletions apps/sim/hooks/use-table-undo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ export function useTableUndo({
deleteRowMutation.mutate(action.rowId)
} else {
// Redo via the batch path so the saved orderKey restores exact placement.
// The single-insert API has no orderKey field, and under the fractional-ordering
// flag its `position` is read as a rank — a gappy saved position misplaces.
// The single-insert API has no orderKey field, and order_key is authoritative —
// a gappy saved position would misplace the row.
batchCreateRowsMutation.mutate(
{
rows: [action.data ?? {}],
positions: [action.position],
orderKeys: action.orderKey ? [action.orderKey] : undefined,
},
{
Expand Down Expand Up @@ -169,7 +168,6 @@ export function useTableUndo({
batchCreateRowsMutation.mutate(
{
rows: action.rows.map((r) => r.data),
positions: action.rows.map((r) => r.position),
orderKeys: action.rows.every((r) => r.orderKey)
? action.rows.map((r) => r.orderKey as string)
: undefined,
Expand All @@ -194,7 +192,6 @@ export function useTableUndo({
batchCreateRowsMutation.mutate(
{
rows: action.rows.map((row) => row.data),
positions: action.rows.map((row) => row.position),
orderKeys: action.rows.every((row) => row.orderKey)
? action.rows.map((row) => row.orderKey as string)
: undefined,
Expand Down
9 changes: 1 addition & 8 deletions apps/sim/lib/api/contracts/tables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,9 @@ export const batchInsertTableRowsBodySchema = z
TABLE_LIMITS.MAX_BATCH_INSERT_SIZE,
`Cannot insert more than ${TABLE_LIMITS.MAX_BATCH_INSERT_SIZE} rows per batch`
),
positions: z.array(z.number().int().min(0)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(),
/** Fractional ordering: exact per-row order keys (undo restore). Takes precedence over `positions`. */
/** Fractional ordering: exact per-row order keys (undo restore). */
orderKeys: z.array(z.string().min(1)).max(TABLE_LIMITS.MAX_BATCH_INSERT_SIZE).optional(),
})
.refine((data) => !data.positions || data.positions.length === data.rows.length, {
message: 'positions array length must match rows array length',
})
.refine((data) => !data.positions || new Set(data.positions).size === data.positions.length, {
message: 'positions must not contain duplicates',
})
.refine((data) => !data.orderKeys || data.orderKeys.length === data.rows.length, {
message: 'orderKeys array length must match rows array length',
})
Expand Down
15 changes: 0 additions & 15 deletions apps/sim/lib/copilot/tools/server/table/user-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,20 +506,6 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
return { success: false, message: 'Workspace ID is required' }
}

const positions = args.positions as number[] | undefined
if (positions !== undefined && positions.length !== args.rows.length) {
return {
success: false,
message: `positions length (${positions.length}) must match rows length (${args.rows.length})`,
}
}
if (positions !== undefined && new Set(positions).size !== positions.length) {
return {
success: false,
message: 'positions must not contain duplicate values',
}
}

const table = await getTableById(args.tableId)
if (!table || table.workspaceId !== workspaceId) {
return { success: false, message: `Table not found: ${args.tableId}` }
Expand All @@ -535,7 +521,6 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
rows: args.rows.map((r: RowData) => rowDataNameToId(r, idByName)),
workspaceId,
userId: context.userId,
positions,
},
table,
requestId
Expand Down
3 changes: 1 addition & 2 deletions apps/sim/lib/core/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ export const env = createEnv({
ENTERPRISE_TIER_COST_LIMIT: z.number().optional(), // Cost limit for enterprise tier users
ENTERPRISE_STORAGE_LIMIT_GB: z.number().optional().default(500), // Default storage limit in GB for enterprise tier (can be overridden per org)
BILLING_ENABLED: z.boolean().optional(), // Enable billing enforcement and usage tracking
FREE_API_DEPLOYMENT_GATE_ENABLED: z.boolean().optional(), // Block free-plan accounts from programmatic execution (API/MCP/generic webhooks/chat embeds). Requires BILLING_ENABLED. Off by default for dark rollout
TABLES_FRACTIONAL_ORDERING: z.boolean().optional(), // Order table rows by fractional order_key (O(1) insert/delete) instead of integer position
FREE_API_DEPLOYMENT_GATE_ENABLED: z.boolean().optional(), // Block free-plan accounts from programmatic execution (API/MCP/A2A/generic webhooks/chat embeds). Requires BILLING_ENABLED. Off by default for dark rollout
TABLE_SNAPSHOT_CACHE: z.boolean().optional(), // Mount tables into sandboxes by reference via a version-keyed CSV snapshot in object storage instead of draining the whole table into web-process heap
PII_REDACTION: z.boolean().optional(), // Redact PII from workflow logs via configurable Data Retention rules (Presidio at the logger persist choke point) and expose the Data Retention config UI
PII_GRANULAR_REDACTION: z.boolean().optional(), // Expose the execution-altering PII redaction stages (redact workflow input + block outputs in-flight) in the Data Retention config; layered on top of PII_REDACTION
Expand Down
8 changes: 3 additions & 5 deletions apps/sim/lib/core/config/feature-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ function withAppConfig(doc: unknown) {
}

/**
* `isFeatureEnabled` only accepts registered `FeatureFlagName`s. The registry is
* empty in this PR, so tests reference flags through the AppConfig document and
* cast their throwaway names through this helper.
* `isFeatureEnabled` only accepts registered `FeatureFlagName`s. These tests
* exercise the evaluation logic with throwaway flag names supplied through the
* AppConfig document, cast to `FeatureFlagName` through this helper.
*/
const enabled = (flag: string, ctx?: FeatureFlagContext) =>
isFeatureEnabled(flag as FeatureFlagName, ctx)
Expand All @@ -62,7 +62,6 @@ describe('getFeatureFlags', () => {
it('derives flags from fallback secrets when AppConfig is disabled, without fetching', async () => {
const flags = await getFeatureFlags()
// All registered flags should be present, disabled (env vars unset in test env)
expect(flags['tables-fractional-ordering']).toEqual({ enabled: false })
expect(flags['mothership-beta']).toEqual({ enabled: false })
expect(flags['pii-redaction']).toEqual({ enabled: false })
expect(flags['pii-granular-redaction']).toEqual({ enabled: false })
Expand Down Expand Up @@ -91,7 +90,6 @@ describe('getFeatureFlags', () => {
flagRef.isAppConfigEnabled = true
mockFetch.mockResolvedValue(null)
const flags = await getFeatureFlags()
expect(flags['tables-fractional-ordering']).toEqual({ enabled: false })
expect(flags['mothership-beta']).toEqual({ enabled: false })
expect(flags['pii-redaction']).toEqual({ enabled: false })
expect(flags['pii-granular-redaction']).toEqual({ enabled: false })
Expand Down
4 changes: 0 additions & 4 deletions apps/sim/lib/core/config/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ interface FeatureFlagDefinition {

/** The single registry of known flags. To add a flag, add one entry here. */
const FEATURE_FLAGS = {
'tables-fractional-ordering': {
description: 'Order table rows by fractional order_key instead of legacy integer position',
fallback: 'TABLES_FRACTIONAL_ORDERING',
},
'mothership-beta': {
description:
'Mothership beta plan/changelog artifact surfaces in the copilot VFS and doc compiler. ' +
Expand Down
74 changes: 10 additions & 64 deletions apps/sim/lib/table/__tests__/update-row.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ vi.mock('@/lib/table/billing', () => ({
TableRowLimitError: class TableRowLimitError extends Error {},
}))

// These suites assert flag-off position-shift semantics; pin the flag so they're
// deterministic regardless of a local TABLES_FRACTIONAL_ORDERING env value.
vi.mock('@/lib/core/config/feature-flags', () => ({
isFeatureEnabled: vi.fn().mockResolvedValue(false),
}))

vi.mock('@/lib/table/validation', () => ({
validateRowSize: vi.fn(() => ({ valid: true, errors: [] })),
validateRowAgainstSchema: vi.fn(() => ({ valid: true, errors: [] })),
Expand Down Expand Up @@ -187,29 +181,17 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)',
expect(findExecutedSqlContaining('hashtextextended')).toBe(true)
})

it('explicit-position inserts also acquire the advisory lock to serialize position shifts', async () => {
dbChainMockFns.limit.mockResolvedValueOnce([])
dbChainMockFns.returning.mockResolvedValueOnce([
{
id: 'row-1',
tableId: 'tbl-1',
workspaceId: 'ws-1',
data: { name: 'a' },
position: 5,
createdAt: new Date(),
updatedAt: new Date(),
},
])

await insertRow(
{ tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 },
TABLE,
'req-1'
)
it('explicit-position inserts also acquire the advisory lock to serialize order-key minting', async () => {
await expect(
insertRow(
{ tableId: 'tbl-1', data: { name: 'a' }, workspaceId: 'ws-1', position: 5 },
TABLE,
'req-1'
)
).rejects.toBeDefined()

// `(table_id, position)` index is non-unique, so concurrent explicit-position
// inserts at the same slot could both skip the shift and duplicate — lock
// serializes them.
// A position-based insert resolves its order_key from the neighbor at that
// rank; the lock serializes concurrent minting at the same slot.
expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
})

Expand All @@ -225,42 +207,6 @@ describe('insertRow — position race safety (migration 0198 + advisory lock)',
expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
})

it('batchInsertRows with explicit positions acquires the advisory lock', async () => {
dbChainMockFns.returning.mockResolvedValueOnce([
{
id: 'row-1',
tableId: 'tbl-1',
workspaceId: 'ws-1',
data: { name: 'a' },
position: 3,
createdAt: new Date(),
updatedAt: new Date(),
},
{
id: 'row-2',
tableId: 'tbl-1',
workspaceId: 'ws-1',
data: { name: 'b' },
position: 4,
createdAt: new Date(),
updatedAt: new Date(),
},
])

await batchInsertRows(
{
tableId: 'tbl-1',
rows: [{ name: 'a' }, { name: 'b' }],
workspaceId: 'ws-1',
positions: [3, 4],
},
TABLE,
'req-1'
)

expect(findExecutedSqlContaining('pg_advisory_xact_lock')).toBe(true)
})

it('upsertRow skips the advisory lock on the update path (match found)', async () => {
vi.mocked(getUniqueColumns).mockReturnValue([{ name: 'name', type: 'string', unique: true }])
dbChainMockFns.limit.mockResolvedValueOnce([
Expand Down
Loading
Loading