diff --git a/apps/sim/app/api/table/[tableId]/columns/route.test.ts b/apps/sim/app/api/table/[tableId]/columns/route.test.ts index 4ac282861cd..e8497a0fa04 100644 --- a/apps/sim/app/api/table/[tableId]/columns/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/columns/route.test.ts @@ -42,6 +42,13 @@ vi.mock('@/lib/table', () => ({ updateColumnOptions: mockUpdateColumnOptions, updateColumnType: mockUpdateColumnType, })) +vi.mock('@/lib/table/columns/service', () => ({ + renameColumn: mockRenameColumn, + updateColumnConstraints: mockUpdateColumnConstraints, + updateColumnCurrency: mockUpdateColumnCurrency, + updateColumnOptions: mockUpdateColumnOptions, + updateColumnType: mockUpdateColumnType, +})) vi.mock('@/app/api/table/utils', () => ({ accessError: () => new Response('denied', { status: 403 }), checkAccess: mockCheckAccess, @@ -50,6 +57,7 @@ vi.mock('@/app/api/table/utils', () => ({ tableLockErrorResponse: () => null, })) +import { OrchestrationError } from '@/lib/core/orchestration/types' import { PATCH } from '@/app/api/table/[tableId]/columns/route' const WORKSPACE_ID = '11111111-1111-4111-8111-111111111111' @@ -159,7 +167,10 @@ describe('PATCH /api/table/[tableId]/columns — pre-flight guards', () => { // Stands in for the race the guards cannot close: the column stopped being // a currency between the snapshot the guards read and this write. mockUpdateColumnCurrency.mockRejectedValue( - new Error('Cannot set currency on column "amount" of type "string"') + new OrchestrationError( + 'validation', + 'Cannot set currency on column "amount" of type "string"' + ) ) const response = await patch({ name: 'renamed', currencyCode: 'USD' }) diff --git a/apps/sim/app/api/table/[tableId]/columns/route.ts b/apps/sim/app/api/table/[tableId]/columns/route.ts index 5cad8c5e610..f55c4a0bc52 100644 --- a/apps/sim/app/api/table/[tableId]/columns/route.ts +++ b/apps/sim/app/api/table/[tableId]/columns/route.ts @@ -8,20 +8,11 @@ import { import { parseRequest } from '@/lib/api/server' import { isZodError, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { - addTableColumn, - deleteColumn, - renameColumn, - updateColumnConstraints, - updateColumnCurrency, - updateColumnOptions, - updateColumnType, -} from '@/lib/table' -import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys' -import { columnTypeById } from '@/lib/table/column-types' -import { isSupportedCurrencyCode } from '@/lib/table/currency' +import { addTableColumn, deleteColumn } from '@/lib/table' +import { performUpdateTableColumn } from '@/lib/table/orchestration' import { accessError, checkAccess, @@ -120,215 +111,32 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - const { updates } = validated - let updatedTable = null - - // A payload that repeats the current type must not go through - // `updateColumnType` — it early-returns on an unchanged type and would drop - // any `options` alongside it. Only a real type change routes there; an - // unchanged type with options routes to the options-only update. - const currentColumn = table.schema.columns.find((c) => - columnMatchesRef(c, validated.columnName) - ) - // Address every write below by the stable id, not the name: a rename folded - // into one of them must not break the next one's lookup. - const columnRef = currentColumn ? getColumnId(currentColumn) : validated.columnName - // The constraints write below is a separate, unconditional step, so it is - // the last one whenever it runs — that is the write the rename rides on. - const typeChanging = updates.type !== undefined && updates.type !== currentColumn?.type - if (!currentColumn) { - return NextResponse.json( - { error: `Column "${validated.columnName}" not found` }, - { status: 404 } - ) - } - - // A retype applies and validates the constraints itself, so the separate - // constraint write only runs when the type is unchanged. The rename rides - // whichever write actually runs last. - const typedWriteRuns = - typeChanging || - updates.currencyCode !== undefined || - updates.options !== undefined || - updates.multiple !== undefined - const constraintsWriteRuns = - !typedWriteRuns && (updates.required !== undefined || updates.unique !== undefined) - const renameWithTypedWrite = - updates.name && !constraintsWriteRuns ? { newName: updates.name } : {} - - // Every write below is its own locked transaction, so one that is going to - // fail leaves the earlier ones committed. These guards reject the knowable - // cases up front, before any write at all. - // Gate on the type the column ENDS UP with, not on whether the type is - // changing: an options-only update on an existing select column carries the - // same hazard as a conversion does. - const resultingType = updates.type ?? currentColumn?.type - if (updates.currencyCode !== undefined) { - if (resultingType !== 'currency') { - return NextResponse.json( - { - error: `Cannot set currency on column "${validated.columnName}" of type "${resultingType}"`, - }, - { status: 400 } - ) - } - if (!isSupportedCurrencyCode(updates.currencyCode)) { - return NextResponse.json( - { - error: `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`, - }, - { status: 400 } - ) - } - } - // The rename runs last (see below), so a name already taken would fail after - // the typed write committed. This is the only rename failure a caller can - // cause; catching it here leaves just the concurrent-collision race, which - // no pre-flight check can close. - if ( - updates.name && - table.schema.columns.some( - (c) => - c.name.toLowerCase() === updates.name?.toLowerCase() && - !columnMatchesRef(c, validated.columnName) - ) - ) { - return NextResponse.json( - { error: `Column "${updates.name}" already exists` }, - { status: 400 } - ) - } - if ( - currentColumn?.workflowGroupId && - (updates.required !== undefined || updates.unique !== undefined) - ) { - return NextResponse.json( - { - error: `Cannot change constraints on workflow-output column "${currentColumn.name}". Constraints aren't applicable to columns whose values come from workflow execution.`, - }, - { status: 400 } - ) - } - if (updates.unique === true && !columnTypeById(resultingType).supportsUnique) { + const outcome = await performUpdateTableColumn({ + table, + columnName: validated.columnName, + userId: authResult.userId, + updates: validated.updates, + requestId, + request, + }) + if (!outcome.success || !outcome.table) { return NextResponse.json( - { error: `Cannot set a ${resultingType} column as unique` }, - { status: 400 } + { error: outcome.error ?? 'Failed to update column' }, + { status: statusForOrchestrationError(outcome.errorCode) } ) } - if (typeChanging) { - updatedTable = await updateColumnType( - { - tableId, - columnName: columnRef, - newType: updates.type as NonNullable, - ...(updates.options !== undefined ? { options: updates.options } : {}), - ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), - ...(updates.currencyCode !== undefined ? { currencyCode: updates.currencyCode } : {}), - // Forwarded so the conversion validates against the constraint this - // same request is about to set, not the column's current one. - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } else if (updates.currencyCode !== undefined) { - // Re-denominating an existing currency column: schema-only, no cell - // rewrite. Reached only when the type is unchanged — a conversion INTO - // currency carries the code through `updateColumnType` above. - updatedTable = await updateColumnCurrency( - { - tableId, - columnName: columnRef, - currencyCode: updates.currencyCode, - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } else if (updates.options !== undefined || updates.multiple !== undefined) { - updatedTable = await updateColumnOptions( - { - tableId, - columnName: columnRef, - options: updates.options ?? currentColumn?.options ?? [], - ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), - // Forwarded so the removal guard validates against the constraint this - // same request is about to set, not the column's current one. - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } - - // Skipped whenever a typed write ran: that write already applied and - // validated these, in one transaction with the change they accompany. - if (constraintsWriteRuns) { - updatedTable = await updateColumnConstraints( - { - tableId, - columnName: columnRef, - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...(updates.name ? { newName: updates.name } : {}), - }, - requestId - ) - } - - // A rename rides along with the LAST write above, inside that write's - // transaction — a rename is metadata-only (rows key on the stable column - // id), so nothing forces it to be its own write, and folding it in is what - // stops a combined request from committing one half and then failing. Only - // a rename with nothing to ride on runs standalone. - if (updates.name && !updatedTable) { - updatedTable = await renameColumn( - { tableId, oldName: columnRef, newName: updates.name }, - requestId - ) - } - - if (!updatedTable) { - return NextResponse.json({ error: 'No updates specified' }, { status: 400 }) - } - return NextResponse.json({ success: true, data: { - columns: updatedTable.schema.columns.map(normalizeColumn), + columns: outcome.table.schema.columns.map(normalizeColumn), }, }) } catch (error) { - const lockError = tableLockErrorResponse(error) - if (lockError) return lockError if (isZodError(error)) { return validationErrorResponse(error, 'Invalid request data') } - const msg = rootErrorMessage(error) - if (msg.includes('not found') || msg.includes('Table not found')) { - return NextResponse.json({ error: msg }, { status: 404 }) - } - if ( - msg.includes('already exists') || - msg.includes('Cannot delete the last column') || - msg.includes('Cannot set column') || - msg.includes('Cannot set unique column') || - msg.includes('Invalid column') || - msg.includes('exceeds maximum') || - msg.includes('incompatible') || - msg.includes('duplicate') || - msg.includes('option') || - msg.includes('currency') || - msg.includes('is already type') - ) { - return NextResponse.json({ error: msg }, { status: 400 }) - } - logger.error(`[${requestId}] Error updating column in table ${tableId}:`, error) return NextResponse.json({ error: 'Failed to update column' }, { status: 500 }) } diff --git a/apps/sim/app/api/table/[tableId]/columns/run/route.ts b/apps/sim/app/api/table/[tableId]/columns/run/route.ts index 824ce73ddb4..7a047120f75 100644 --- a/apps/sim/app/api/table/[tableId]/columns/run/route.ts +++ b/apps/sim/app/api/table/[tableId]/columns/run/route.ts @@ -8,7 +8,12 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { TableQueryValidationError } from '@/lib/table/errors' import { toLegacyFilter } from '@/lib/table/query-builder/converters' import { runWorkflowColumn } from '@/lib/table/workflow-columns' -import { accessError, checkAccess, tableFilterError } from '@/app/api/table/utils' +import { + accessError, + checkAccess, + orchestrationErrorResponse, + tableFilterError, +} from '@/app/api/table/utils' const logger = createLogger('TableRunColumnAPI') @@ -66,9 +71,8 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro if (error instanceof TableQueryValidationError) { return NextResponse.json({ error: error.message }, { status: 400 }) } - if (error instanceof Error && error.message === 'Invalid workspace ID') { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`run-column failed:`, error) return NextResponse.json({ error: 'Failed to run columns' }, { status: 500 }) } diff --git a/apps/sim/app/api/table/[tableId]/import/route.test.ts b/apps/sim/app/api/table/[tableId]/import/route.test.ts index baf8c313a4f..a2689295725 100644 --- a/apps/sim/app/api/table/[tableId]/import/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/import/route.test.ts @@ -79,6 +79,7 @@ vi.mock('@/lib/table/billing', () => ({ limit >= 0 && current + added > limit, })) +import { OrchestrationError } from '@/lib/core/orchestration/types' import { TableLockedError } from '@/lib/table/mutation-locks' import { POST } from '@/app/api/table/[tableId]/import/route' @@ -372,7 +373,10 @@ describe('POST /api/table/[tableId]/import', () => { it('surfaces unique violations from importAppendRows as 400', async () => { mockImportAppendRows.mockRejectedValueOnce( - new Error('Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx') + new OrchestrationError( + 'validation', + 'Row 1: Column "name" must be unique. Value "Alice" already exists in row row_xxx' + ) ) const response = await callPost( createFormData(createCsvFile('name,age\nAlice,30'), { mode: 'append' }) @@ -516,7 +520,9 @@ describe('POST /api/table/[tableId]/import', () => { }) it('surfaces column-creation failures from importAppendRows as 400', async () => { - mockImportAppendRows.mockRejectedValueOnce(new Error('Column "email" already exists')) + mockImportAppendRows.mockRejectedValueOnce( + new OrchestrationError('validation', 'Column "email" already exists') + ) const response = await callPost( createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), { mode: 'append', @@ -529,7 +535,9 @@ describe('POST /api/table/[tableId]/import', () => { }) it('surfaces row insert failures without success when schema was mutated', async () => { - mockImportAppendRows.mockRejectedValueOnce(new Error('must be unique')) + mockImportAppendRows.mockRejectedValueOnce( + new OrchestrationError('validation', 'must be unique') + ) const response = await callPost( createFormData(createCsvFile('name,age,email\nAlice,30,a@x.io'), { mode: 'append', diff --git a/apps/sim/app/api/table/[tableId]/import/route.ts b/apps/sim/app/api/table/[tableId]/import/route.ts index 7a9802442cc..0b6cf78a319 100644 --- a/apps/sim/app/api/table/[tableId]/import/route.ts +++ b/apps/sim/app/api/table/[tableId]/import/route.ts @@ -14,6 +14,7 @@ import { import { ianaTimezoneSchema } from '@/lib/api/contracts/user' import { getValidationErrorMessage } from '@/lib/api/server' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import { isMultipartError, readMultipart } from '@/lib/core/utils/multipart' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -349,21 +350,13 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro createdColumns: additions.length, error: message, }) - const isClientError = - message.includes('row limit') || - message.includes('Insufficient capacity') || - message.includes('Schema validation') || - message.includes('must be unique') || - message.includes('Row size exceeds') || - message.includes('already exists') || - message.includes('Invalid column name') || - /^Row \d+:/.test(message) + const classified = asOrchestrationError(err) return NextResponse.json( { - error: isClientError ? message : 'Failed to import CSV', + error: classified ? classified.message : 'Failed to import CSV', data: { insertedCount: 0 }, }, - { status: isClientError ? 400 : 500 } + { status: classified ? statusForOrchestrationError(classified.code) : 500 } ) } } @@ -400,17 +393,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro }, }) } catch (err) { - const message = toError(err).message - const isClientError = - message.includes('row limit') || - message.includes('Schema validation') || - message.includes('must be unique') || - message.includes('Row size exceeds') || - message.includes('already exists') || - message.includes('Invalid column name') || - /^Row \d+:/.test(message) - if (isClientError) { - return NextResponse.json({ error: message }, { status: 400 }) + const classified = asOrchestrationError(err) + if (classified) { + return NextResponse.json( + { error: classified.message }, + { status: statusForOrchestrationError(classified.code) } + ) } throw err } @@ -419,17 +407,12 @@ export const POST = withRouteHandler(async (request: NextRequest, { params }: Ro if (lockError) return lockError if (isMultipartError(error)) return multipartErrorResponse(error) - const message = toError(error).message logger.error(`[${requestId}] CSV import into existing table failed:`, error) - const isClientError = - message.includes('CSV file has no') || - message.includes('already exists') || - message.includes('Invalid column name') - + const classified = asOrchestrationError(error) return NextResponse.json( - { error: isClientError ? message : 'Failed to import CSV' }, - { status: isClientError ? 400 : 500 } + { error: classified ? classified.message : 'Failed to import CSV' }, + { status: classified ? statusForOrchestrationError(classified.code) : 500 } ) } finally { fileStream?.destroy() diff --git a/apps/sim/app/api/table/[tableId]/route.test.ts b/apps/sim/app/api/table/[tableId]/route.test.ts index 7f1f48243c7..2396ba13a21 100644 --- a/apps/sim/app/api/table/[tableId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/route.test.ts @@ -33,6 +33,13 @@ vi.mock('@/lib/table', () => ({ updateTableLocks: mockUpdateTableLocks, TableConflictError: class extends Error {}, })) +vi.mock('@/lib/table/service', () => ({ + deleteTable: mockDeleteTable, + getTableById: mockGetTableById, + moveTableToFolder: mockMoveTableToFolder, + renameTable: mockRenameTable, + updateTableLocks: mockUpdateTableLocks, +})) vi.mock('@/lib/table/billing', () => ({ getWorkspaceTableLimits: mockGetLimits })) vi.mock('@/lib/folders/queries', () => ({ findActiveFolder: mockFindActiveFolder })) vi.mock('@/lib/core/config/feature-flags', () => ({ isFeatureEnabled: vi.fn() })) @@ -77,6 +84,13 @@ const routeContext = { params: Promise.resolve({ tableId: 'tbl_1' }) } describe('PATCH /api/table/[tableId] folder moves', () => { beforeEach(() => { vi.clearAllMocks() + mockMoveTableToFolder.mockResolvedValue({ name: 'Table' }) + mockRenameTable.mockResolvedValue({ id: 'tbl_1', name: 'Table' }) + mockDeleteTable.mockResolvedValue({ archived: { name: 'Table', workspaceId: 'workspace-1' } }) + mockUpdateTableLocks.mockResolvedValue({ + table: { ...TABLE, locks: {} }, + previousLocks: {}, + }) hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, userId: 'user-1', @@ -99,8 +113,7 @@ describe('PATCH /api/table/[tableId] folder moves', () => { 'tbl_1', 'workspace-1', 'folder-1', - expect.any(String), - 'user-1' + expect.any(String) ) }) @@ -118,8 +131,7 @@ describe('PATCH /api/table/[tableId] folder moves', () => { 'tbl_1', 'workspace-1', null, - expect.any(String), - 'user-1' + expect.any(String) ) }) diff --git a/apps/sim/app/api/table/[tableId]/route.ts b/apps/sim/app/api/table/[tableId]/route.ts index 03eaa4c7243..7d75286cf83 100644 --- a/apps/sim/app/api/table/[tableId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/route.ts @@ -5,20 +5,18 @@ import { getTableQuerySchema, updateTableContract } from '@/lib/api/contracts/ta import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { findActiveFolder } from '@/lib/folders/queries' -import { captureServerEvent } from '@/lib/posthog/server' -import { - deleteTable, - getTableById, - moveTableToFolder, - renameTable, - TableConflictError, - type TableSchema, - updateTableLocks, -} from '@/lib/table' +import { getTableById, TableConflictError, type TableSchema } from '@/lib/table' import { getWorkspaceTableLimits } from '@/lib/table/billing' +import { + performDeleteTable, + performMoveTableToFolder, + performRenameTable, + performUpdateTableLocks, +} from '@/lib/table/orchestration' import { TABLE_LOCK_FLAGS, TABLE_LOCK_KINDS } from '@/lib/table/types' import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils' import { @@ -180,11 +178,35 @@ export const PATCH = withRouteHandler( { status: 403 } ) } - await updateTableLocks(tableId, validated.locks, authResult.userId, requestId, request) + const lockOutcome = await performUpdateTableLocks({ + tableId, + partial: validated.locks, + userId: authResult.userId, + requestId, + request, + }) + if (!lockOutcome.success) { + return NextResponse.json( + { error: lockOutcome.error ?? 'Failed to update table locks' }, + { status: statusForOrchestrationError(lockOutcome.errorCode) } + ) + } } if (validated.name !== undefined) { - await renameTable(tableId, validated.name, requestId, authResult.userId) + const renameOutcome = await performRenameTable({ + table, + newName: validated.name, + userId: authResult.userId, + requestId, + request, + }) + if (!renameOutcome.success) { + return NextResponse.json( + { error: renameOutcome.error ?? 'Failed to rename table' }, + { status: statusForOrchestrationError(renameOutcome.errorCode) } + ) + } } if (validated.folderId !== undefined) { @@ -196,21 +218,22 @@ export const PATCH = withRouteHandler( ) { return NextResponse.json({ error: 'Folder not found in this workspace' }, { status: 404 }) } - try { - await moveTableToFolder( - tableId, - table.workspaceId, - validated.folderId, - requestId, - authResult.userId + // The move re-asserts workspace and active state, so a miss means the table was + // archived between `checkAccess` and the write. That is a 404, not a server fault. + const moveOutcome = await performMoveTableToFolder({ + table, + folderId: validated.folderId, + userId: authResult.userId, + requestId, + request, + }) + if (!moveOutcome.success) { + return NextResponse.json( + { + error: moveOutcome.errorCode === 'not_found' ? 'Table not found' : moveOutcome.error, + }, + { status: statusForOrchestrationError(moveOutcome.errorCode) } ) - } catch (moveError) { - // The move re-asserts workspace and active state, so a miss means the table was - // archived between `checkAccess` and the write. That is a 404, not a server fault. - if (moveError instanceof Error && moveError.message.endsWith('not found')) { - return NextResponse.json({ error: 'Table not found' }, { status: 404 }) - } - throw moveError } } @@ -268,14 +291,18 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - await deleteTable(tableId, requestId, authResult.userId) - - captureServerEvent( - authResult.userId, - 'table_deleted', - { table_id: tableId, workspace_id: table.workspaceId }, - { groups: { workspace: table.workspaceId } } - ) + const outcome = await performDeleteTable({ + table, + userId: authResult.userId, + requestId, + request, + }) + if (!outcome.success) { + return NextResponse.json( + { error: outcome.error ?? 'Failed to delete table' }, + { status: statusForOrchestrationError(outcome.errorCode) } + ) + } return NextResponse.json({ success: true, diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 82ec048ca84..6f4636d9aa1 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -1,7 +1,6 @@ import { db } from '@sim/db' import { userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { @@ -11,15 +10,17 @@ import { } from '@/lib/api/contracts/tables' import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { RowData, TableSchema } from '@/lib/table' -import { deleteRow, updateRow } from '@/lib/table' +import { updateRow } from '@/lib/table' +import { performDeleteTableRow } from '@/lib/table/orchestration' import { rowWireTranslators } from '@/app/api/table/row-wire' import { accessError, checkAccess, - rootErrorMessage, + orchestrationErrorResponse, rowWriteErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' @@ -173,10 +174,6 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR }, }) } catch (error) { - if (rootErrorMessage(error) === 'Row not found') { - return NextResponse.json({ error: 'Row not found' }, { status: 404 }) - } - const response = rowWriteErrorResponse(error) if (response) return response @@ -212,7 +209,13 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - await deleteRow(table, rowId, requestId) + const outcome = await performDeleteTableRow({ table, rowId, requestId }) + if (!outcome.success) { + return NextResponse.json( + { error: outcome.error ?? 'Failed to delete row' }, + { status: statusForOrchestrationError(outcome.errorCode) } + ) + } return NextResponse.json({ success: true, @@ -225,11 +228,8 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row const lockError = tableLockErrorResponse(error) if (lockError) return lockError - const errorMessage = toError(error).message - - if (errorMessage === 'Row not found') { - return NextResponse.json({ error: errorMessage }, { status: 404 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error deleting row:`, error) return NextResponse.json({ error: 'Failed to delete row' }, { status: 500 }) diff --git a/apps/sim/app/api/table/import-async/route.ts b/apps/sim/app/api/table/import-async/route.ts index 57d879c1f32..04039178db7 100644 --- a/apps/sim/app/api/table/import-async/route.ts +++ b/apps/sim/app/api/table/import-async/route.ts @@ -18,11 +18,11 @@ import { releaseJobClaim, sanitizeName, TABLE_LIMITS, - TableConflictError, } from '@/lib/table' import { runTableImport, type TableImportPayload } from '@/lib/table/import-runner' import { getUserSettings } from '@/lib/users/queries' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' +import { orchestrationErrorResponse } from '@/app/api/table/utils' const logger = createLogger('TableImportAsync') @@ -101,12 +101,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { requestId ) } catch (error) { - if (error instanceof TableConflictError) { - return NextResponse.json({ error: error.message }, { status: 409 }) - } - if (error instanceof Error && error.message.includes('maximum table limit')) { - return NextResponse.json({ error: error.message }, { status: 400 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified throw error } diff --git a/apps/sim/app/api/table/import-csv/route.test.ts b/apps/sim/app/api/table/import-csv/route.test.ts index b85e1ccb01b..a9722924755 100644 --- a/apps/sim/app/api/table/import-csv/route.test.ts +++ b/apps/sim/app/api/table/import-csv/route.test.ts @@ -2,7 +2,6 @@ * @vitest-environment node */ import { hybridAuthMockFns, permissionsMock, permissionsMockFns } from '@sim/testing' -import { getErrorMessage } from '@sim/utils/errors' import type { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -32,6 +31,9 @@ vi.mock('@/lib/table/rows/service', () => ({ vi.mock('@/lib/table/billing', () => ({ getWorkspaceTableLimits: mockGetLimits })) vi.mock('@/app/api/table/utils', async () => { const { NextResponse } = await import('next/server') + const { asOrchestrationError, statusForOrchestrationError } = await import( + '@/lib/core/orchestration/types' + ) return { normalizeColumn: (column: unknown) => column, csvProxyBodyCapResponse: () => null, @@ -40,16 +42,20 @@ vi.mock('@/app/api/table/utils', async () => { { error: error.message }, { status: error.code === 'FILE_TOO_LARGE' ? 413 : 400 } ), - rowWriteErrorResponse: (error: unknown) => { - const message = getErrorMessage(error) - return message.includes('row limit') - ? NextResponse.json({ error: message }, { status: 400 }) + orchestrationErrorResponse: (error: unknown) => { + const classified = asOrchestrationError(error) + return classified + ? NextResponse.json( + { error: classified.message }, + { status: statusForOrchestrationError(classified.code) } + ) : null }, } }) vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock) +import { OrchestrationError } from '@/lib/core/orchestration/types' import { POST } from '@/app/api/table/import-csv/route' type Part = @@ -184,7 +190,10 @@ describe('POST /api/table/import-csv', () => { it('returns 400 with the reason when an insert exceeds the plan row limit', async () => { mockBatchInsertRows.mockRejectedValueOnce( - new Error('This table has reached its row limit (1,000 rows) on your current plan.') + new OrchestrationError( + 'validation', + 'This table has reached its row limit (1,000 rows) on your current plan.' + ) ) const response = await POST(makeRequest(uploadParts(csvWithRows(250)))) const data = await response.json() diff --git a/apps/sim/app/api/table/import-csv/route.ts b/apps/sim/app/api/table/import-csv/route.ts index 9ca0381fe90..f84f457e820 100644 --- a/apps/sim/app/api/table/import-csv/route.ts +++ b/apps/sim/app/api/table/import-csv/route.ts @@ -1,6 +1,5 @@ import type { Readable } from 'node:stream' import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { type NextRequest, NextResponse } from 'next/server' import { csvExtensionSchema, csvImportFormSchema } from '@/lib/api/contracts/tables' @@ -34,7 +33,7 @@ import { csvProxyBodyCapResponse, multipartErrorResponse, normalizeColumn, - rowWriteErrorResponse, + orchestrationErrorResponse, } from '@/app/api/table/utils' const logger = createLogger('TableImportCSV') @@ -250,22 +249,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { logger.error(`[${requestId}] CSV import failed:`, error) - // Row-write failures (e.g. the plan row-limit check) map to a 400 with the real reason. - const rowWriteError = rowWriteErrorResponse(error) - if (rowWriteError) return rowWriteError + // Every caller-fixable failure on this path — the plan row-limit check, the + // schema and CSV-shape validation, a name collision — arrives classified. + const classified = orchestrationErrorResponse(error) + if (classified) return classified - const message = toError(error).message - const isClientError = - message.includes('maximum table limit') || - message.includes('CSV file has no') || - message.includes('Invalid table name') || - message.includes('Invalid schema') || - message.includes('already exists') - - return NextResponse.json( - { error: isClientError ? message : 'Failed to import CSV' }, - { status: isClientError ? 400 : 500 } - ) + return NextResponse.json({ error: 'Failed to import CSV' }, { status: 500 }) } finally { fileStream?.destroy() } diff --git a/apps/sim/app/api/table/route.ts b/apps/sim/app/api/table/route.ts index 2522cddb7c6..28714885cb5 100644 --- a/apps/sim/app/api/table/route.ts +++ b/apps/sim/app/api/table/route.ts @@ -16,7 +16,7 @@ import { type TableScope, } from '@/lib/table' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' -import { normalizeColumn } from '@/app/api/table/utils' +import { normalizeColumn, orchestrationErrorResponse } from '@/app/api/table/utils' const logger = createLogger('TableAPI') @@ -153,18 +153,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { }, }) } catch (error) { - if (error instanceof Error) { - if (error.message.includes('maximum table limit')) { - return NextResponse.json({ error: error.message }, { status: 403 }) - } - if ( - error.message.includes('Invalid table name') || - error.message.includes('Invalid schema') || - error.message.includes('already exists') - ) { - return NextResponse.json({ error: error.message }, { status: 400 }) - } - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error creating table:`, error) return NextResponse.json({ error: 'Failed to create table' }, { status: 500 }) diff --git a/apps/sim/app/api/table/utils.test.ts b/apps/sim/app/api/table/utils.test.ts index 99d0ce0c5a5..fa7b57ac6dd 100644 --- a/apps/sim/app/api/table/utils.test.ts +++ b/apps/sim/app/api/table/utils.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { TableRowLimitError } from '@/lib/table/billing' import type { ColumnDefinition } from '@/lib/table/types' import { rootErrorMessage, rowWriteErrorResponse, tableFilterError } from '@/app/api/table/utils' @@ -38,15 +39,30 @@ describe('rowWriteErrorResponse', () => { ) }) - it('passes known validation messages through as 400', async () => { - const response = rowWriteErrorResponse(new Error('Value for column "email" must be unique')) + it('passes a classified validation failure through as 400', async () => { + const response = rowWriteErrorResponse( + new OrchestrationError('validation', 'Value for column "email" must be unique') + ) expect(response?.status).toBe(400) const body = await response?.json() expect(body.error).toBe('Value for column "email" must be unique') }) - it('matches per-row batch validation messages', () => { - expect(rowWriteErrorResponse(new Error('Row 3: name is required'))?.status).toBe(400) + it('answers the code the failure carries, not one derived from its wording', () => { + expect( + rowWriteErrorResponse(new OrchestrationError('not_found', 'Row not found'))?.status + ).toBe(404) + // The phrase that used to force a 400 no longer decides anything. + expect( + rowWriteErrorResponse(new OrchestrationError('conflict', 'Row 3: must be unique'))?.status + ).toBe(409) + }) + + it('unwraps a classified failure drizzle wrapped in a query error', () => { + expect( + rowWriteErrorResponse(wrapLikeDrizzle(new OrchestrationError('validation', 'Row 3: bad'))) + ?.status + ).toBe(400) }) it('returns null for unknown errors so callers keep their generic 500', () => { diff --git a/apps/sim/app/api/table/utils.ts b/apps/sim/app/api/table/utils.ts index ceb399556c4..805d2b16206 100644 --- a/apps/sim/app/api/table/utils.ts +++ b/apps/sim/app/api/table/utils.ts @@ -8,6 +8,7 @@ import { updateTableColumnBodySchema, } from '@/lib/api/contracts/tables' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' +import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types' import type { MultipartError } from '@/lib/core/utils/multipart' import type { ColumnDefinition, Filter, TableDefinition, TablePredicate } from '@/lib/table' import { buildFilterClause, getTableById, TableQueryValidationError } from '@/lib/table' @@ -42,8 +43,9 @@ export async function tablesV2GateError( * Maps a {@link TableLockedError} thrown by the service layer to a 423 response * carrying `{ error, lock }`; returns `null` for any other error so the caller * falls through to its existing handling. Call this as the FIRST statement of a - * table route's catch block — otherwise `rowWriteErrorResponse` (and the other - * substring funnels) turn the lock error into a generic 500. + * table route's catch block — `TableLockedError` is an `HttpError`, not an + * `OrchestrationError`, so nothing else classifies it and it would otherwise + * reach the route's generic 500. * * The body deliberately omits a `details` array: the client's `isValidationError` * treats any `ApiClientError` with array-valued `details` as a field-validation @@ -106,48 +108,36 @@ export function rootErrorMessage(error: unknown): string { } /** - * Known user-facing row-write failures (service validation + the best-effort - * plan row-limit check). Anything outside this list stays a generic 500 — - * unknown errors can carry SQL/internals that don't belong in a toast. - */ -const ROW_WRITE_ERROR_PATTERNS = [ - 'row limit', - 'Insufficient capacity', - 'Schema validation', - 'must be unique', - 'must be valid', - 'must be string', - 'must be number', - 'must be boolean', - 'unique column', - 'Unique constraint violation', - 'Row size exceeds', - 'conflictTarget', - 'Upsert requires', - 'Rows not found', - 'Filter is required', -] as const - -/** - * Maps a known user-facing row-write failure to a 400 carrying the real message - * (so client toasts can show the actual reason); `null` when the error is - * unrecognized and the caller should log it and return its generic 500. + * Maps a classified domain failure to its status, carrying the real message so + * client toasts can show the actual reason; `null` when the error carries no + * classification and the caller should log it and return its own generic 500 — + * an unrecognized error can hold SQL/internals that don't belong in a toast. + * + * This is the whole classification story for the UI and v1 table routes. It + * replaced per-route lists of message substrings, which decided a status by + * searching prose and so silently changed one whenever a message was reworded. */ -export function rowWriteErrorResponse(error: unknown): NextResponse | null { - // A lock violation is a 423, not a 400/500 — check before the pattern match, - // which would otherwise let it fall through to the caller's generic 500. +export function orchestrationErrorResponse(error: unknown): NextResponse | null { + // A lock violation is a 423, and `TableLockedError` is an `HttpError` rather + // than an `OrchestrationError`, so it needs its own check first. const lockResponse = tableLockErrorResponse(error) if (lockResponse) return lockResponse - const message = rootErrorMessage(error) - - if (ROW_WRITE_ERROR_PATTERNS.some((p) => message.includes(p)) || /^Row .+?:/.test(message)) { - return NextResponse.json({ error: message }, { status: 400 }) - } + const classified = asOrchestrationError(error) + if (!classified) return null - return null + return NextResponse.json( + { error: classified.message }, + { status: statusForOrchestrationError(classified.code) } + ) } +/** + * {@link orchestrationErrorResponse} under the name the row-write routes call + * it by. Row writes have no classification rules of their own any more. + */ +export const rowWriteErrorResponse = orchestrationErrorResponse + /** * Next.js buffers the request body for the proxy and silently truncates it past this * size (`experimental.proxyClientMaxBodySize`, default 10MB). The synchronous CSV diff --git a/apps/sim/app/api/tools/deployments/deploy/route.ts b/apps/sim/app/api/tools/deployments/deploy/route.ts index 5f35c91da86..0795475b549 100644 --- a/apps/sim/app/api/tools/deployments/deploy/route.ts +++ b/apps/sim/app/api/tools/deployments/deploy/route.ts @@ -3,10 +3,10 @@ import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/ import { type NextRequest, NextResponse } from 'next/server' import { deploymentsDeployContract } from '@/lib/api/contracts/tools/deployments' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { performFullDeploy } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { authenticateDeploymentToolRequest, authorizeDeploymentWorkflow, diff --git a/apps/sim/app/api/tools/deployments/promote/route.ts b/apps/sim/app/api/tools/deployments/promote/route.ts index a126c3dbd57..523a5630a32 100644 --- a/apps/sim/app/api/tools/deployments/promote/route.ts +++ b/apps/sim/app/api/tools/deployments/promote/route.ts @@ -3,10 +3,10 @@ import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/ import { type NextRequest, NextResponse } from 'next/server' import { deploymentsPromoteContract } from '@/lib/api/contracts/tools/deployments' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { performActivateVersion } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { authenticateDeploymentToolRequest, authorizeDeploymentWorkflow, diff --git a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts index d1a507fa195..78dca2d367f 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/columns/route.ts @@ -7,24 +7,16 @@ import { v1UpdateTableColumnContract, } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { - addTableColumn, - deleteColumn, - renameColumn, - updateColumnConstraints, - updateColumnCurrency, - updateColumnOptions, - updateColumnType, -} from '@/lib/table' -import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys' -import { columnTypeById } from '@/lib/table/column-types' -import { isSupportedCurrencyCode } from '@/lib/table/currency' +import { addTableColumn, deleteColumn } from '@/lib/table' +import { performUpdateTableColumn } from '@/lib/table/orchestration' import { accessError, checkAccess, normalizeColumn, + orchestrationErrorResponse, tableLockErrorResponse, } from '@/app/api/table/utils' import { @@ -101,22 +93,8 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Colum const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - if (error instanceof Error) { - // Same caller-error set the internal columns route maps — an invalid - // select option set is a bad request, not a server fault. - if ( - error.message.includes('already exists') || - error.message.includes('maximum column') || - error.message.includes('Invalid column') || - error.message.includes('exceeds maximum') || - error.message.includes('option') - ) { - return NextResponse.json({ error: error.message }, { status: 400 }) - } - if (error.message === 'Table not found') { - return NextResponse.json({ error: error.message }, { status: 404 }) - } - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error adding column to table:`, error) return NextResponse.json({ error: 'Failed to add column' }, { status: 500 }) @@ -154,227 +132,31 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - const { updates } = validated - let updatedTable = null - - // A payload that repeats the current type must not go through - // `updateColumnType` — it early-returns on an unchanged type and would drop - // any `options` alongside it. Only a real type change routes there; an - // unchanged type with options routes to the options-only update. - const currentColumn = table.schema.columns.find((c) => - columnMatchesRef(c, validated.columnName) - ) - // Address every write below by the stable id, not the name: a rename folded - // into one of them must not break the next one's lookup. - const columnRef = currentColumn ? getColumnId(currentColumn) : validated.columnName - // The constraints write below is a separate, unconditional step, so it is - // the last one whenever it runs — that is the write the rename rides on. - const typeChanging = updates.type !== undefined && updates.type !== currentColumn?.type - if (!currentColumn) { - return NextResponse.json( - { error: `Column "${validated.columnName}" not found` }, - { status: 404 } - ) - } - - // A retype applies and validates the constraints itself, so the separate - // constraint write only runs when the type is unchanged. The rename rides - // whichever write actually runs last. - const typedWriteRuns = - typeChanging || - updates.currencyCode !== undefined || - updates.options !== undefined || - updates.multiple !== undefined - const constraintsWriteRuns = - !typedWriteRuns && (updates.required !== undefined || updates.unique !== undefined) - const renameWithTypedWrite = - updates.name && !constraintsWriteRuns ? { newName: updates.name } : {} - - // Every write below is its own locked transaction, so one that is going to - // fail leaves the earlier ones committed. These guards reject the knowable - // cases up front, before any write at all. - // Gate on the type the column ENDS UP with, not on whether the type is - // changing: an options-only update on an existing select column carries the - // same hazard as a conversion does. - const resultingType = updates.type ?? currentColumn?.type - if (updates.currencyCode !== undefined) { - if (resultingType !== 'currency') { - return NextResponse.json( - { - error: `Cannot set currency on column "${validated.columnName}" of type "${resultingType}"`, - }, - { status: 400 } - ) - } - if (!isSupportedCurrencyCode(updates.currencyCode)) { - return NextResponse.json( - { - error: `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`, - }, - { status: 400 } - ) - } - } - // The rename runs last (see below), so a name already taken would fail after - // the typed write committed. This is the only rename failure a caller can - // cause; catching it here leaves just the concurrent-collision race, which - // no pre-flight check can close. - if ( - updates.name && - table.schema.columns.some( - (c) => - c.name.toLowerCase() === updates.name?.toLowerCase() && - !columnMatchesRef(c, validated.columnName) - ) - ) { - return NextResponse.json( - { error: `Column "${updates.name}" already exists` }, - { status: 400 } - ) - } - if ( - currentColumn?.workflowGroupId && - (updates.required !== undefined || updates.unique !== undefined) - ) { - return NextResponse.json( - { - error: `Cannot change constraints on workflow-output column "${currentColumn.name}". Constraints aren't applicable to columns whose values come from workflow execution.`, - }, - { status: 400 } - ) - } - if (updates.unique === true && !columnTypeById(resultingType).supportsUnique) { + const outcome = await performUpdateTableColumn({ + table, + columnName: validated.columnName, + userId, + updates: validated.updates, + requestId, + request, + }) + if (!outcome.success || !outcome.table) { return NextResponse.json( - { error: `Cannot set a ${resultingType} column as unique` }, - { status: 400 } + { error: outcome.error ?? 'Failed to update column' }, + { status: statusForOrchestrationError(outcome.errorCode) } ) } - if (typeChanging) { - updatedTable = await updateColumnType( - { - tableId, - columnName: columnRef, - newType: updates.type as NonNullable, - ...(updates.options !== undefined ? { options: updates.options } : {}), - ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), - ...(updates.currencyCode !== undefined ? { currencyCode: updates.currencyCode } : {}), - // Forwarded so the conversion validates against the constraint this - // same request is about to set, not the column's current one. - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } else if (updates.currencyCode !== undefined) { - // Re-denominating an existing currency column: schema-only, no cell - // rewrite. Reached only when the type is unchanged — a conversion INTO - // currency carries the code through `updateColumnType` above. - updatedTable = await updateColumnCurrency( - { - tableId, - columnName: columnRef, - currencyCode: updates.currencyCode, - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } else if (updates.options !== undefined || updates.multiple !== undefined) { - updatedTable = await updateColumnOptions( - { - tableId, - columnName: columnRef, - options: updates.options ?? currentColumn?.options ?? [], - ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), - // Forwarded so the removal guard validates against the constraint this - // same request is about to set, not the column's current one. - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...renameWithTypedWrite, - }, - requestId - ) - } - - // Skipped whenever a typed write ran: that write already applied and - // validated these, in one transaction with the change they accompany. - if (constraintsWriteRuns) { - updatedTable = await updateColumnConstraints( - { - tableId, - columnName: columnRef, - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - ...(updates.name ? { newName: updates.name } : {}), - }, - requestId - ) - } - - // A rename rides along with the LAST write above, inside that write's - // transaction — a rename is metadata-only (rows key on the stable column - // id), so nothing forces it to be its own write, and folding it in is what - // stops a combined request from committing one half and then failing. Only - // a rename with nothing to ride on runs standalone. - if (updates.name && !updatedTable) { - updatedTable = await renameColumn( - { tableId, oldName: columnRef, newName: updates.name }, - requestId - ) - } - - if (!updatedTable) { - return NextResponse.json({ error: 'No updates specified' }, { status: 400 }) - } - - recordAudit({ - workspaceId: validated.workspaceId, - actorId: userId, - action: AuditAction.TABLE_UPDATED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: table.name, - description: `Updated column "${validated.columnName}" in table "${table.name}"`, - metadata: { columnName: validated.columnName, updates }, - request, - }) - return NextResponse.json({ success: true, data: { - columns: updatedTable.schema.columns.map(normalizeColumn), + columns: outcome.table.schema.columns.map(normalizeColumn), }, }) } catch (error) { - const lockError = tableLockErrorResponse(error) - if (lockError) return lockError const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - if (error instanceof Error) { - const msg = error.message - if (msg.includes('not found') || msg.includes('Table not found')) { - return NextResponse.json({ error: msg }, { status: 404 }) - } - if ( - msg.includes('already exists') || - msg.includes('Cannot delete the last column') || - msg.includes('Cannot set column') || - msg.includes('Invalid column') || - msg.includes('exceeds maximum') || - msg.includes('incompatible') || - msg.includes('duplicate') || - msg.includes('option') || - msg.includes('currency') || - msg.includes('is already type') - ) { - return NextResponse.json({ error: msg }, { status: 400 }) - } - } - logger.error(`[${requestId}] Error updating column in table:`, error) return NextResponse.json({ error: 'Failed to update column' }, { status: 500 }) } @@ -441,14 +223,8 @@ export const DELETE = withRouteHandler( const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - if (error instanceof Error) { - if (error.message.includes('not found') || error.message === 'Table not found') { - return NextResponse.json({ error: error.message }, { status: 404 }) - } - if (error.message.includes('Cannot delete') || error.message.includes('last column')) { - return NextResponse.json({ error: error.message }, { status: 400 }) - } - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error deleting column from table:`, error) return NextResponse.json({ error: 'Failed to delete column' }, { status: 500 }) diff --git a/apps/sim/app/api/v1/tables/[tableId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/route.ts index c06492d02b7..149bc674651 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/route.ts @@ -1,11 +1,12 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' import { v1DeleteTableContract, v1GetTableContract } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { deleteTable, type TableSchema } from '@/lib/table' +import type { TableSchema } from '@/lib/table' +import { performDeleteTable } from '@/lib/table/orchestration' import { accessError, checkAccess, @@ -139,18 +140,13 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - await deleteTable(tableId, requestId) - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.TABLE_DELETED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: result.table.name, - description: `Archived table "${result.table.name}"`, - request, - }) + const outcome = await performDeleteTable({ table: result.table, userId, requestId, request }) + if (!outcome.success) { + return NextResponse.json( + { error: outcome.error ?? 'Failed to delete table' }, + { status: statusForOrchestrationError(outcome.errorCode) } + ) + } return NextResponse.json({ success: true, diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts index 5fee4f3d03b..dbe768ee830 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts @@ -1,7 +1,6 @@ import { db } from '@sim/db' import { userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { @@ -10,13 +9,20 @@ import { v1UpdateTableRowContract, } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { RowData, TableSchema } from '@/lib/table' -import { deleteRow, updateRow } from '@/lib/table' +import { updateRow } from '@/lib/table' import { namedRowMapper } from '@/lib/table/cell-format' import { buildIdByName, rowDataNameToId } from '@/lib/table/column-keys' -import { accessError, checkAccess, tableLockErrorResponse } from '@/app/api/table/utils' +import { performDeleteTableRow } from '@/lib/table/orchestration' +import { + accessError, + checkAccess, + orchestrationErrorResponse, + tableLockErrorResponse, +} from '@/app/api/table/utils' import { checkRateLimit, checkWorkspaceScope, @@ -188,21 +194,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - const errorMessage = toError(error).message - - if (errorMessage === 'Row not found') { - return NextResponse.json({ error: errorMessage }, { status: 404 }) - } - - if ( - errorMessage.includes('Row size exceeds') || - errorMessage.includes('Schema validation') || - errorMessage.includes('must be unique') || - errorMessage.includes('Unique constraint violation') || - errorMessage.includes('Cannot set unique column') - ) { - return NextResponse.json({ error: errorMessage }, { status: 400 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error updating row:`, error) return NextResponse.json({ error: 'Failed to update row' }, { status: 500 }) @@ -238,9 +231,13 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) } - // Route through the service (not a raw `db.delete`) so the delete lock is - // enforced — the raw path would return 200 on a locked table. - await deleteRow(result.table, rowId, requestId) + const outcome = await performDeleteTableRow({ table: result.table, rowId, requestId }) + if (!outcome.success) { + return NextResponse.json( + { error: outcome.error ?? 'Failed to delete row' }, + { status: statusForOrchestrationError(outcome.errorCode) } + ) + } return NextResponse.json({ success: true, @@ -252,9 +249,8 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row } catch (error) { const lockError = tableLockErrorResponse(error) if (lockError) return lockError - if (error instanceof Error && error.message === 'Row not found') { - return NextResponse.json({ error: 'Row not found' }, { status: 404 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error deleting row:`, error) return NextResponse.json({ error: 'Failed to delete row' }, { status: 500 }) } diff --git a/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts index bf4a00df91b..a32f17a9c8e 100644 --- a/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/v1/tables/[tableId]/rows/upsert/route.ts @@ -1,5 +1,4 @@ import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' import { type NextRequest, NextResponse } from 'next/server' import { v1UpsertTableRowContract } from '@/lib/api/contracts/v1/tables' import { parseRequest } from '@/lib/api/server' @@ -9,7 +8,12 @@ import type { RowData, TableSchema } from '@/lib/table' import { upsertRow } from '@/lib/table' import { namedRowMapper } from '@/lib/table/cell-format' import { buildIdByName, rowDataNameToId } from '@/lib/table/column-keys' -import { accessError, checkAccess, tableLockErrorResponse } from '@/app/api/table/utils' +import { + accessError, + checkAccess, + orchestrationErrorResponse, + tableLockErrorResponse, +} from '@/app/api/table/utils' import { checkRateLimit, checkWorkspaceScope, @@ -101,19 +105,8 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Upser const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - const errorMessage = toError(error).message - - if ( - errorMessage.includes('unique column') || - errorMessage.includes('Unique constraint violation') || - errorMessage.includes('conflictTarget') || - errorMessage.includes('row limit') || - errorMessage.includes('Schema validation') || - errorMessage.includes('Upsert requires') || - errorMessage.includes('Row size exceeds') - ) { - return NextResponse.json({ error: errorMessage }, { status: 400 }) - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error upserting row:`, error) return NextResponse.json({ error: 'Failed to upsert row' }, { status: 500 }) diff --git a/apps/sim/app/api/v1/tables/route.ts b/apps/sim/app/api/v1/tables/route.ts index 82bc6618247..6213fd59053 100644 --- a/apps/sim/app/api/v1/tables/route.ts +++ b/apps/sim/app/api/v1/tables/route.ts @@ -6,7 +6,7 @@ import { parseRequest } from '@/lib/api/server' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { createTable, getWorkspaceTableLimits, listTables, type TableSchema } from '@/lib/table' -import { normalizeColumn } from '@/app/api/table/utils' +import { normalizeColumn, orchestrationErrorResponse } from '@/app/api/table/utils' import { checkRateLimit, createRateLimitResponse, @@ -171,18 +171,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const validationResponse = v1ValidationErrorResponseFromError(error) if (validationResponse) return validationResponse - if (error instanceof Error) { - if (error.message.includes('maximum table limit')) { - return NextResponse.json({ error: error.message }, { status: 403 }) - } - if ( - error.message.includes('Invalid table name') || - error.message.includes('Invalid schema') || - error.message.includes('already exists') - ) { - return NextResponse.json({ error: error.message }, { status: 400 }) - } - } + const classified = orchestrationErrorResponse(error) + if (classified) return classified logger.error(`[${requestId}] Error creating table:`, error) return NextResponse.json({ error: 'Failed to create table' }, { status: 500 }) diff --git a/apps/sim/app/api/v1/workflows/[id]/deploy/route.ts b/apps/sim/app/api/v1/workflows/[id]/deploy/route.ts index 7068239e134..304d69f63d8 100644 --- a/apps/sim/app/api/v1/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/v1/workflows/[id]/deploy/route.ts @@ -8,11 +8,11 @@ import { v1UndeployWorkflowContract, } from '@/lib/api/contracts/v1/workflows' import { parseOptionalJsonBody, parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { captureServerEvent } from '@/lib/posthog/server' import { performFullDeploy, performFullUndeploy } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta' import { checkRateLimit, diff --git a/apps/sim/app/api/v1/workflows/[id]/rollback/route.ts b/apps/sim/app/api/v1/workflows/[id]/rollback/route.ts index a0779babf51..d015ba4b024 100644 --- a/apps/sim/app/api/v1/workflows/[id]/rollback/route.ts +++ b/apps/sim/app/api/v1/workflows/[id]/rollback/route.ts @@ -7,10 +7,10 @@ import { v1RollbackWorkflowContract, } from '@/lib/api/contracts/v1/workflows' import { parseOptionalJsonBody, parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { performActivateVersion } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { findPreviousDeploymentVersion } from '@/lib/workflows/persistence/utils' import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta' import { diff --git a/apps/sim/app/api/v2/lib/response.ts b/apps/sim/app/api/v2/lib/response.ts index bd326d1218d..45ed1e6fcb3 100644 --- a/apps/sim/app/api/v2/lib/response.ts +++ b/apps/sim/app/api/v2/lib/response.ts @@ -1,6 +1,7 @@ import { NextResponse } from 'next/server' import type { ZodError } from 'zod' import { getValidationErrorMessage, serializeZodIssues } from '@/lib/api/server' +import { asOrchestrationError, type OrchestrationErrorCode } from '@/lib/core/orchestration/types' import type { RateLimitResult, WorkspaceAccessError } from '@/app/api/v1/middleware' /** @@ -155,3 +156,36 @@ export function decodeCursor>(cursor: string): T | n return null } } + +const V2_CODE_BY_ORCHESTRATION_ERROR: Record = { + validation: 'BAD_REQUEST', + forbidden: 'FORBIDDEN', + not_found: 'NOT_FOUND', + conflict: 'CONFLICT', + locked: 'LOCKED', + internal: 'INTERNAL_ERROR', +} + +/** + * Renders a `lib/[resource]/orchestration` failure in the v2 envelope, so every + * v2 route maps a given failure class to the same status without restating the + * mapping. Mirrors `statusForOrchestrationError` for the v1/UI surfaces. + */ +export function v2ErrorForOrchestration( + code: OrchestrationErrorCode | undefined, + message: string +): NextResponse { + const v2Code = code ? V2_CODE_BY_ORCHESTRATION_ERROR[code] : 'INTERNAL_ERROR' + return v2Error(v2Code, v2Code === 'INTERNAL_ERROR' ? 'Internal server error' : message) +} + +/** + * Renders a thrown domain failure in the v2 envelope, or `null` when the error + * carries no classification and the caller should log it and return its own + * generic 500. The v2 counterpart of `orchestrationErrorResponse`. + */ +export function v2CaughtOrchestrationError(error: unknown): NextResponse | null { + const classified = asOrchestrationError(error) + if (!classified) return null + return v2ErrorForOrchestration(classified.code, classified.message) +} diff --git a/apps/sim/app/api/v2/tables/[tableId]/columns/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/columns/route.test.ts new file mode 100644 index 00000000000..a7d6235dca0 --- /dev/null +++ b/apps/sim/app/api/v2/tables/[tableId]/columns/route.test.ts @@ -0,0 +1,105 @@ +/** + * @vitest-environment node + * + * v2 column update wiring: the route authenticates, scopes, delegates to the + * orchestration function, and maps its failure classes onto the v2 envelope. + * The guards themselves are covered in lib/table/orchestration/columns.test.ts. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckRateLimit, mockResolveWorkspaceScope, mockCheckAccess, mockPerformUpdate } = + vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockResolveWorkspaceScope: vi.fn(), + mockCheckAccess: vi.fn(), + mockPerformUpdate: vi.fn(), + })) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + resolveWorkspaceScope: mockResolveWorkspaceScope, +})) + +vi.mock('@/app/api/table/utils', () => ({ + checkAccess: mockCheckAccess, + normalizeColumn: (col: Record) => col, +})) + +vi.mock('@/lib/table', () => ({ addTableColumn: vi.fn(), deleteColumn: vi.fn() })) + +vi.mock('@/lib/table/orchestration', () => ({ + performUpdateTableColumn: mockPerformUpdate, +})) + +vi.mock('@/app/api/v2/lib/gate', () => ({ + v2ApiGateError: vi.fn().mockResolvedValue(null), +})) + +import { PATCH } from '@/app/api/v2/tables/[tableId]/columns/route' + +const COLUMN = { id: 'col-1', name: 'Status', type: 'text' } +const TABLE = { id: 'table-1', name: 'Tasks', workspaceId: 'ws-1', schema: { columns: [COLUMN] } } + +function patch(updates: Record = { name: 'State' }) { + const req = new NextRequest('http://localhost:3000/api/v2/tables/table-1/columns', { + method: 'PATCH', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ workspaceId: 'ws-1', columnName: 'Status', updates }), + }) + return PATCH(req, { params: Promise.resolve({ tableId: 'table-1' }) }) +} + +describe('PATCH /api/v2/tables/[tableId]/columns', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ + allowed: true, + userId: 'user-1', + keyType: 'workspace', + workspaceId: 'ws-1', + limit: 100, + remaining: 99, + resetAt: new Date('2026-01-01T01:00:00Z'), + }) + mockResolveWorkspaceScope.mockResolvedValue(null) + mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE }) + mockPerformUpdate.mockResolvedValue({ success: true, table: TABLE }) + }) + + it('delegates to the orchestration function with the resolved table and actor', async () => { + const res = await patch() + + expect(res.status).toBe(200) + expect((await res.json()).data).toEqual({ columns: [COLUMN] }) + expect(mockPerformUpdate).toHaveBeenCalledWith( + expect.objectContaining({ table: TABLE, columnName: 'Status', userId: 'user-1' }) + ) + }) + + it.each([ + ['validation', 400, 'BAD_REQUEST'], + ['not_found', 404, 'NOT_FOUND'], + ['locked', 423, 'LOCKED'], + ])('maps a %s failure to %i', async (errorCode, status, code) => { + mockPerformUpdate.mockResolvedValue({ success: false, errorCode, error: 'nope' }) + + const res = await patch() + + expect(res.status).toBe(status) + expect((await res.json()).error.code).toBe(code) + }) + + it('does not leak an internal failure message', async () => { + mockPerformUpdate.mockResolvedValue({ + success: false, + errorCode: 'internal', + error: 'connection string leaked', + }) + + const res = await patch() + + expect(res.status).toBe(500) + expect(await res.text()).not.toContain('connection string') + }) +}) diff --git a/apps/sim/app/api/v2/tables/[tableId]/columns/route.ts b/apps/sim/app/api/v2/tables/[tableId]/columns/route.ts index c7c1e538600..ce480142cab 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/columns/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/columns/route.ts @@ -10,19 +10,16 @@ import { import { isZodError, parseRequest } from '@/lib/api/server' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { - addTableColumn, - deleteColumn, - renameColumn, - updateColumnConstraints, - updateColumnType, -} from '@/lib/table' +import { addTableColumn, deleteColumn } from '@/lib/table' +import { performUpdateTableColumn } from '@/lib/table/orchestration' import { checkAccess, normalizeColumn } from '@/app/api/table/utils' import { checkRateLimit, resolveWorkspaceScope } from '@/app/api/v1/middleware' import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { + v2CaughtOrchestrationError, v2Data, v2Error, + v2ErrorForOrchestration, v2RateLimitError, v2ValidationError, v2WorkspaceAccessError, @@ -88,14 +85,8 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Colum } catch (error) { if (isZodError(error)) return v2ValidationError(error) - if (error instanceof Error) { - if (error.message.includes('already exists') || error.message.includes('maximum column')) { - return v2Error('BAD_REQUEST', error.message) - } - if (error.message === 'Table not found') { - return v2Error('NOT_FOUND', error.message) - } - } + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error adding column to table`, { error: getErrorMessage(error, 'Unknown error'), @@ -136,73 +127,21 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu return v2Error('NOT_FOUND', 'Table not found') } - const { updates } = validated - let updatedTable = null - - if (updates.name) { - updatedTable = await renameColumn( - { tableId, oldName: validated.columnName, newName: updates.name }, - requestId - ) - } - - if (updates.type) { - updatedTable = await updateColumnType( - { tableId, columnName: updates.name ?? validated.columnName, newType: updates.type }, - requestId - ) - } - - if (updates.required !== undefined || updates.unique !== undefined) { - updatedTable = await updateColumnConstraints( - { - tableId, - columnName: updates.name ?? validated.columnName, - ...(updates.required !== undefined ? { required: updates.required } : {}), - ...(updates.unique !== undefined ? { unique: updates.unique } : {}), - }, - requestId - ) - } - - if (!updatedTable) { - return v2Error('BAD_REQUEST', 'No updates specified') - } - - recordAudit({ - workspaceId: validated.workspaceId, - actorId: userId, - action: AuditAction.TABLE_UPDATED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: table.name, - description: `Updated column "${validated.columnName}" in table "${table.name}"`, - metadata: { columnName: validated.columnName, updates }, + const outcome = await performUpdateTableColumn({ + table, + columnName: validated.columnName, + userId, + updates: validated.updates, + requestId, request, }) + if (!outcome.success || !outcome.table) { + return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to update column') + } - return v2Data({ columns: updatedTable.schema.columns.map(normalizeColumn) }, { rateLimit }) + return v2Data({ columns: outcome.table.schema.columns.map(normalizeColumn) }, { rateLimit }) } catch (error) { if (isZodError(error)) return v2ValidationError(error) - - if (error instanceof Error) { - const msg = error.message - if (msg.includes('not found') || msg.includes('Table not found')) { - return v2Error('NOT_FOUND', msg) - } - if ( - msg.includes('already exists') || - msg.includes('Cannot delete the last column') || - msg.includes('Cannot set column') || - msg.includes('Invalid column') || - msg.includes('exceeds maximum') || - msg.includes('incompatible') || - msg.includes('duplicate') - ) { - return v2Error('BAD_REQUEST', msg) - } - } - logger.error(`[${requestId}] Error updating column in table`, { error: getErrorMessage(error, 'Unknown error'), }) @@ -264,14 +203,8 @@ export const DELETE = withRouteHandler( } catch (error) { if (isZodError(error)) return v2ValidationError(error) - if (error instanceof Error) { - if (error.message.includes('not found') || error.message === 'Table not found') { - return v2Error('NOT_FOUND', error.message) - } - if (error.message.includes('Cannot delete') || error.message.includes('last column')) { - return v2Error('BAD_REQUEST', error.message) - } - } + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error deleting column from table`, { error: getErrorMessage(error, 'Unknown error'), diff --git a/apps/sim/app/api/v2/tables/[tableId]/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/route.test.ts new file mode 100644 index 00000000000..43210d8a8e8 --- /dev/null +++ b/apps/sim/app/api/v2/tables/[tableId]/route.test.ts @@ -0,0 +1,109 @@ +/** + * @vitest-environment node + * + * Public v2 table delete: the actor is handed to the service so the audit is + * emitted there — and only for a delete that actually archived a row. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockCheckRateLimit, + mockResolveWorkspaceScope, + mockCheckAccess, + mockPerformDeleteTable, + mockRecordAudit, +} = vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockResolveWorkspaceScope: vi.fn(), + mockCheckAccess: vi.fn(), + mockPerformDeleteTable: vi.fn(), + mockRecordAudit: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { TABLE_DELETED: 'table.deleted', TABLE_UPDATED: 'table.updated' }, + AuditResourceType: { TABLE: 'table' }, + recordAudit: mockRecordAudit, +})) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + resolveWorkspaceScope: mockResolveWorkspaceScope, +})) + +vi.mock('@/app/api/table/utils', () => ({ + checkAccess: mockCheckAccess, + normalizeColumn: (col: Record) => col, + rootErrorMessage: (error: unknown) => String(error), + rowWriteErrorResponse: () => null, +})) + +vi.mock('@/lib/table', () => ({ + updateTable: vi.fn(), + getTableById: vi.fn(), + updateRow: vi.fn(), + rowDataNameToId: vi.fn(), + buildIdByName: vi.fn(), +})) + +vi.mock('@/lib/table/orchestration', () => ({ performDeleteTable: mockPerformDeleteTable })) + +vi.mock('@/app/api/v2/lib/gate', () => ({ + v2ApiGateError: vi.fn().mockResolvedValue(null), +})) + +import { DELETE } from '@/app/api/v2/tables/[tableId]/route' + +const TABLE = { id: 'table-1', name: 'Tasks', workspaceId: 'ws-1', schema: { columns: [] } } + +function callDelete() { + const req = new NextRequest('http://localhost:3000/api/v2/tables/table-1?workspaceId=ws-1', { + method: 'DELETE', + }) + return DELETE(req, { params: Promise.resolve({ tableId: 'table-1' }) }) +} + +describe('DELETE /api/v2/tables/[tableId]', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ + allowed: true, + userId: 'user-1', + keyType: 'workspace', + workspaceId: 'ws-1', + limit: 100, + remaining: 99, + resetAt: new Date('2026-01-01T01:00:00Z'), + }) + mockResolveWorkspaceScope.mockResolvedValue(null) + mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE }) + }) + + it('delegates to the orchestration function with the resolved table and actor', async () => { + mockPerformDeleteTable.mockResolvedValue({ success: true }) + + const res = await callDelete() + + expect(res.status).toBe(200) + expect(mockPerformDeleteTable).toHaveBeenCalledWith( + expect.objectContaining({ table: TABLE, userId: 'user-1' }) + ) + // The route no longer audits: doing so out here fired TABLE_DELETED even + // when the delete was a no-op on an already-archived table. + expect(mockRecordAudit).not.toHaveBeenCalled() + }) + + it('returns 423 LOCKED for a delete-locked table instead of a 500', async () => { + mockPerformDeleteTable.mockResolvedValue({ + success: false, + errorCode: 'locked', + error: 'Table is locked', + }) + + const res = await callDelete() + + expect(res.status).toBe(423) + expect((await res.json()).error.code).toBe('LOCKED') + }) +}) diff --git a/apps/sim/app/api/v2/tables/[tableId]/route.ts b/apps/sim/app/api/v2/tables/[tableId]/route.ts index 03e97d590fa..55e2d792f8f 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/route.ts @@ -1,4 +1,3 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import type { NextRequest } from 'next/server' @@ -6,18 +5,19 @@ import { v2DeleteTableContract, v2GetTableContract } from '@/lib/api/contracts/v import { parseRequest } from '@/lib/api/server' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { deleteTable } from '@/lib/table' +import { performDeleteTable } from '@/lib/table/orchestration' import { checkAccess } from '@/app/api/table/utils' import { checkRateLimit, resolveWorkspaceScope } from '@/app/api/v1/middleware' import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { v2Data, v2Error, + v2ErrorForOrchestration, v2RateLimitError, v2ValidationError, v2WorkspaceAccessError, } from '@/app/api/v2/lib/response' -import { toApiTable, v2TableAccessError } from '@/app/api/v2/tables/utils' +import { toApiTable, v2TableAccessError, v2TableLockError } from '@/app/api/v2/tables/utils' const logger = createLogger('V2TableDetailAPI') @@ -100,21 +100,15 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Tab return v2Error('NOT_FOUND', 'Table not found') } - await deleteTable(tableId, requestId) - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.TABLE_DELETED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: result.table.name, - description: `Archived table "${result.table.name}"`, - request, - }) + const outcome = await performDeleteTable({ table: result.table, userId, requestId, request }) + if (!outcome.success) { + return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to delete table') + } return v2Data({ id: tableId }, { rateLimit }) } catch (error) { + const lockError = v2TableLockError(error) + if (lockError) return lockError logger.error(`[${requestId}] Error deleting table`, { error: getErrorMessage(error, 'Unknown error'), }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts new file mode 100644 index 00000000000..2139216449a --- /dev/null +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts @@ -0,0 +1,99 @@ +/** + * @vitest-environment node + * + * Public v2 single-row delete: goes through the row service so the delete lock + * and row-count bookkeeping are enforced, and renders lock/not-found in the v2 + * error envelope. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckRateLimit, mockResolveWorkspaceScope, mockCheckAccess, mockPerformDeleteRow } = + vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockResolveWorkspaceScope: vi.fn(), + mockCheckAccess: vi.fn(), + mockPerformDeleteRow: vi.fn(), + })) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + resolveWorkspaceScope: mockResolveWorkspaceScope, +})) + +vi.mock('@/app/api/table/utils', () => ({ + checkAccess: mockCheckAccess, + normalizeColumn: (col: Record) => col, + rootErrorMessage: (error: unknown) => String(error), + rowWriteErrorResponse: () => null, +})) + +vi.mock('@/lib/table', () => ({ + updateTable: vi.fn(), + getTableById: vi.fn(), + updateRow: vi.fn(), + rowDataNameToId: vi.fn(), + buildIdByName: vi.fn(), +})) + +vi.mock('@/lib/table/orchestration', () => ({ performDeleteTableRow: mockPerformDeleteRow })) + +vi.mock('@/app/api/v2/lib/gate', () => ({ + v2ApiGateError: vi.fn().mockResolvedValue(null), +})) + +import { DELETE } from '@/app/api/v2/tables/[tableId]/rows/[rowId]/route' + +const TABLE = { id: 'table-1', workspaceId: 'ws-1', schema: { columns: [] } } + +function callDelete() { + const req = new NextRequest( + 'http://localhost:3000/api/v2/tables/table-1/rows/row-1?workspaceId=ws-1', + { method: 'DELETE' } + ) + return DELETE(req, { params: Promise.resolve({ tableId: 'table-1', rowId: 'row-1' }) }) +} + +describe('DELETE /api/v2/tables/[tableId]/rows/[rowId]', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ + allowed: true, + userId: 'user-1', + keyType: 'workspace', + workspaceId: 'ws-1', + limit: 100, + remaining: 99, + resetAt: new Date('2026-01-01T01:00:00Z'), + }) + mockResolveWorkspaceScope.mockResolvedValue(null) + mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE }) + }) + + it('delegates to the orchestration function rather than deleting inline', async () => { + mockPerformDeleteRow.mockResolvedValue({ success: true }) + + const res = await callDelete() + + expect(res.status).toBe(200) + expect((await res.json()).data).toEqual({ deletedCount: 1, deletedRowIds: ['row-1'] }) + // The orchestration function routes through the row service, which applies + // the delete lock and the row-count decrement; the raw delete this replaced + // skipped both. + expect(mockPerformDeleteRow).toHaveBeenCalledWith( + expect.objectContaining({ table: TABLE, rowId: 'row-1' }) + ) + }) + + it.each([ + ['locked', 423, 'LOCKED'], + ['not_found', 404, 'NOT_FOUND'], + ])('maps a %s failure to %i', async (errorCode, status, code) => { + mockPerformDeleteRow.mockResolvedValue({ success: false, errorCode, error: 'nope' }) + + const res = await callDelete() + + expect(res.status).toBe(status) + expect((await res.json()).error.code).toBe(code) + }) +}) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts index f11cf9b2c74..b0bb10b78d3 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts @@ -1,7 +1,7 @@ import { db } from '@sim/db' import { userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' +import { getErrorMessage } from '@sim/utils/errors' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { @@ -15,17 +15,20 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import type { RowData, TableSchema } from '@/lib/table' import { buildIdByName, rowDataNameToId, updateRow } from '@/lib/table' import { namedRowMapper } from '@/lib/table/cell-format' +import { performDeleteTableRow } from '@/lib/table/orchestration' import { checkAccess } from '@/app/api/table/utils' import { checkRateLimit, resolveWorkspaceScope } from '@/app/api/v1/middleware' import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { + v2CaughtOrchestrationError, v2Data, v2Error, + v2ErrorForOrchestration, v2RateLimitError, v2ValidationError, v2WorkspaceAccessError, } from '@/app/api/v2/lib/response' -import { toApiRow, v2TableAccessError } from '@/app/api/v2/tables/utils' +import { toApiRow, v2TableAccessError, v2TableLockError } from '@/app/api/v2/tables/utils' const logger = createLogger('V2TableRowAPI') @@ -163,18 +166,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR } catch (error) { if (isZodError(error)) return v2ValidationError(error) - const errorMessage = toError(error).message - if (errorMessage === 'Row not found') return v2Error('NOT_FOUND', errorMessage) - - if ( - errorMessage.includes('Row size exceeds') || - errorMessage.includes('Schema validation') || - errorMessage.includes('must be unique') || - errorMessage.includes('Unique constraint violation') || - errorMessage.includes('Cannot set unique column') - ) { - return v2Error('BAD_REQUEST', errorMessage) - } + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error updating row`, { error: getErrorMessage(error, 'Unknown error'), @@ -214,22 +207,18 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Row return v2Error('NOT_FOUND', 'Table not found') } - const [deletedRow] = await db - .delete(userTableRows) - .where( - and( - eq(userTableRows.id, rowId), - eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, workspaceId) - ) - ) - .returning({ id: userTableRows.id }) - - if (!deletedRow) return v2Error('NOT_FOUND', 'Row not found') + const outcome = await performDeleteTableRow({ table: result.table, rowId, requestId }) + if (!outcome.success) { + return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? 'Failed to delete row') + } // v2 mirrors the bulk delete shape: always returns `deletedRowIds`. - return v2Data({ deletedCount: 1, deletedRowIds: [deletedRow.id] }, { rateLimit }) + return v2Data({ deletedCount: 1, deletedRowIds: [rowId] }, { rateLimit }) } catch (error) { + const lockError = v2TableLockError(error) + if (lockError) return lockError + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error deleting row`, { error: getErrorMessage(error, 'Unknown error'), }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts index 08f4b0873af..a8b4c21593b 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts @@ -1,5 +1,5 @@ import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' +import { getErrorMessage } from '@sim/utils/errors' import type { NextRequest } from 'next/server' import { v2UpsertTableRowContract } from '@/lib/api/contracts/v2/tables' import { isZodError, parseRequest } from '@/lib/api/server' @@ -12,6 +12,7 @@ import { checkAccess } from '@/app/api/table/utils' import { checkRateLimit, resolveWorkspaceScope } from '@/app/api/v1/middleware' import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { + v2CaughtOrchestrationError, v2Data, v2Error, v2RateLimitError, @@ -82,18 +83,8 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Upser } catch (error) { if (isZodError(error)) return v2ValidationError(error) - const errorMessage = toError(error).message - if ( - errorMessage.includes('unique column') || - errorMessage.includes('Unique constraint violation') || - errorMessage.includes('conflictTarget') || - errorMessage.includes('row limit') || - errorMessage.includes('Schema validation') || - errorMessage.includes('Upsert requires') || - errorMessage.includes('Row size exceeds') - ) { - return v2Error('BAD_REQUEST', errorMessage) - } + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error upserting row`, { error: getErrorMessage(error, 'Unknown error'), diff --git a/apps/sim/app/api/v2/tables/route.ts b/apps/sim/app/api/v2/tables/route.ts index 9082e9280b9..85df923214c 100644 --- a/apps/sim/app/api/v2/tables/route.ts +++ b/apps/sim/app/api/v2/tables/route.ts @@ -11,6 +11,7 @@ import { normalizeColumn } from '@/app/api/table/utils' import { checkRateLimit, resolveWorkspaceAccess } from '@/app/api/v1/middleware' import { v2ApiGateError } from '@/app/api/v2/lib/gate' import { + v2CaughtOrchestrationError, v2CursorList, v2Data, v2Error, @@ -128,18 +129,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } catch (error) { if (isZodError(error)) return v2ValidationError(error) - if (error instanceof Error) { - if (error.message.includes('maximum table limit')) { - return v2Error('FORBIDDEN', error.message) - } - if ( - error.message.includes('Invalid table name') || - error.message.includes('Invalid schema') || - error.message.includes('already exists') - ) { - return v2Error('BAD_REQUEST', error.message) - } - } + const classified = v2CaughtOrchestrationError(error) + if (classified) return classified logger.error(`[${requestId}] Error creating table`, { error: getErrorMessage(error, 'Unknown error'), diff --git a/apps/sim/app/api/v2/tables/utils.ts b/apps/sim/app/api/v2/tables/utils.ts index 00a9510feb8..8d662be3d4a 100644 --- a/apps/sim/app/api/v2/tables/utils.ts +++ b/apps/sim/app/api/v2/tables/utils.ts @@ -1,5 +1,6 @@ import type { NextResponse } from 'next/server' import type { RowData, TableDefinition, TablePredicate, TableSchema } from '@/lib/table' +import { TableLockedError } from '@/lib/table/mutation-locks' import { predicateToFilter } from '@/lib/table/query-builder/converters' import { validatePredicateShape, @@ -95,6 +96,16 @@ export function v2TableAccessError(result: { ok: false; status: 404 | 403 }): Ne : v2Error('FORBIDDEN', 'Access denied') } +/** + * Maps a delete/write rejected by a table lock to the v2 `LOCKED` envelope, + * mirroring v1's {@link tableLockErrorResponse}. Returns `null` for anything + * else so the caller falls through to its own classification. + */ +export function v2TableLockError(error: unknown): NextResponse | null { + if (error instanceof TableLockedError) return v2Error('LOCKED', error.message) + return null +} + /** * Maps a known user-facing row-write failure (schema/size/unique/limit) to a v2 * `BAD_REQUEST`, reusing v1's {@link rowWriteErrorResponse} classifier as the diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 4c7e1027161..4fb34919b76 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -7,6 +7,7 @@ import { eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { updatePublicApiContract } from '@/lib/api/contracts/deployments' import { parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { captureServerEvent } from '@/lib/posthog/server' @@ -15,7 +16,6 @@ import { performFullDeploy, performFullUndeploy, } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { validateWorkflowPermissions } from '@/lib/workflows/utils' import { checkNeedsRedeployment, diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index d3c3337e62f..5d5300ec13d 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -4,10 +4,10 @@ import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { updateDeploymentVersionMetadataContract } from '@/lib/api/contracts/deployments' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { performActivateVersion } from '@/lib/workflows/orchestration' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { getWorkflowDeploymentVersion, updateDeploymentVersionMetadata, diff --git a/apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts b/apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts index e5b7ffefda4..602d71664cb 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts @@ -22,7 +22,8 @@ import { getKnowledgeBases, updateKnowledgeBase, } from '@/lib/knowledge/service' -import { deleteTable, listTables, renameTable } from '@/lib/table/service' +import { performDeleteTable, performRenameTable } from '@/lib/table/orchestration' +import { listTables } from '@/lib/table/service' import { ensureWorkspaceFileFolderPath, findWorkspaceFileFolderIdByPath, @@ -759,11 +760,19 @@ async function renameFlatResource( return { success: false, error: `Table not found at ${sources[0]}` } } assertMutationNotAborted(context) - const renamed = await renameTable(match.id, newName, generateRequestId()) + const renameOutcome = await performRenameTable({ + table: match, + newName, + userId: context.userId, + requestId: generateRequestId(), + }) + if (!renameOutcome.success) { + return { success: false, error: renameOutcome.error ?? 'Failed to rename table' } + } return buildResult(verb, [ { from: sources[0], - to: `tables/${normalizeVfsSegment(renamed.name)}`, + to: `tables/${normalizeVfsSegment(newName)}`, kind, id: match.id, }, @@ -1018,7 +1027,14 @@ async function removeTablePath( ) if (!match) return { from: path, kind: 'table', error: `Table not found at ${path}` } - await deleteTable(match.id, generateRequestId(), context.userId) + const outcome = await performDeleteTable({ + table: match, + userId: context.userId, + requestId: generateRequestId(), + }) + if (!outcome.success) { + return { from: path, kind: 'table', error: outcome.error ?? 'Failed to archive table' } + } logger.info('Archived table via rm', { tableId: match.id, workspaceId }) return { from: path, kind: 'table', id: match.id } } diff --git a/apps/sim/lib/copilot/tools/server/table/user-table.test.ts b/apps/sim/lib/copilot/tools/server/table/user-table.test.ts index adebd05bd7a..512401f65ea 100644 --- a/apps/sim/lib/copilot/tools/server/table/user-table.test.ts +++ b/apps/sim/lib/copilot/tools/server/table/user-table.test.ts @@ -137,10 +137,7 @@ vi.mock('@/lib/table/billing', () => ({ getWorkspaceTableLimits: mockGetWorkspaceTableLimits, })) -import { - normalizeSelectOptionsInput, - userTableServerTool, -} from '@/lib/copilot/tools/server/table/user-table' +import { userTableServerTool } from '@/lib/copilot/tools/server/table/user-table' import { encodeCursor } from '@/lib/table/rows/cursor' function buildTable(overrides: Partial = {}): TableDefinition { @@ -172,60 +169,6 @@ async function flushDetached(): Promise { await Promise.resolve() } -describe('normalizeSelectOptionsInput', () => { - it('generates a stable id for a bare-name string option', () => { - const [opt] = normalizeSelectOptionsInput(['Open']) ?? [] - expect(opt.name).toBe('Open') - expect(typeof opt.id).toBe('string') - expect(opt.id.length).toBeGreaterThan(0) - }) - - it('generates an id for an object option without one', () => { - const [opt] = normalizeSelectOptionsInput([{ name: 'Closed' }]) ?? [] - expect(opt.name).toBe('Closed') - expect(opt.id.length).toBeGreaterThan(0) - }) - - it('preserves an explicitly supplied id', () => { - const result = normalizeSelectOptionsInput([{ id: 'opt_keep', name: 'Open' }]) - expect(result).toEqual([{ id: 'opt_keep', name: 'Open' }]) - }) - - it('reuses the id of an existing option with the same name', () => { - // The agent re-sends options as bare names on every edit. Minting fresh ids - // would orphan every cell holding them — silently clearing the column. - const existing = [ - { id: 'opt_low', name: 'Low' }, - { id: 'opt_high', name: 'High' }, - ] - const result = normalizeSelectOptionsInput(['Low', 'Medium', 'High'], existing) ?? [] - - expect(result[0]).toEqual({ id: 'opt_low', name: 'Low' }) - expect(result[2]).toEqual({ id: 'opt_high', name: 'High' }) - // Only the genuinely new option gets a fresh id. - expect(result[1].name).toBe('Medium') - expect(result[1].id).not.toBe('opt_low') - expect(result[1].id).not.toBe('opt_high') - }) - - it('matches an existing option name case-insensitively', () => { - const result = normalizeSelectOptionsInput(['open'], [{ id: 'opt_open', name: 'Open' }]) ?? [] - expect(result[0].id).toBe('opt_open') - expect(result[0].name).toBe('open') - }) - - it('mints a fresh id when there is no existing column to match against', () => { - const result = normalizeSelectOptionsInput(['Open']) ?? [] - expect(result[0].id.length).toBeGreaterThan(0) - expect(result[0].name).toBe('Open') - }) - - it('returns undefined for a non-array (validation rejects it downstream)', () => { - expect(normalizeSelectOptionsInput(undefined)).toBeUndefined() - expect(normalizeSelectOptionsInput('Open')).toBeUndefined() - }) -}) - describe('userTableServerTool.import_file', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/copilot/tools/server/table/user-table.ts b/apps/sim/lib/copilot/tools/server/table/user-table.ts index 036ceb2cd40..93b0062c7b6 100644 --- a/apps/sim/lib/copilot/tools/server/table/user-table.ts +++ b/apps/sim/lib/copilot/tools/server/table/user-table.ts @@ -1,7 +1,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' -import { generateId, generateShortId } from '@sim/utils/id' +import { generateId } from '@sim/utils/id' import { UserTable } from '@/lib/copilot/generated/tool-catalog-v1' import { assertServerToolNotAborted, @@ -10,7 +10,6 @@ import { } from '@/lib/copilot/tools/server/base-tool' import { isTriggerDevEnabled } from '@/lib/core/config/env-flags' import { runDetached } from '@/lib/core/utils/background' -import { captureServerEvent } from '@/lib/posthog/server' import { buildAutoMapping, COLUMN_TYPES, @@ -27,29 +26,24 @@ import { validateMapping, } from '@/lib/table' import { namedRowMapper } from '@/lib/table/cell-format' -import { - buildIdByName, - columnMatchesRef, - rowDataNameToId, - sortSpecNamesToIds, -} from '@/lib/table/column-keys' +import { buildIdByName, rowDataNameToId, sortSpecNamesToIds } from '@/lib/table/column-keys' import { columnTypeForLeaf, deriveOutputColumnName } from '@/lib/table/column-naming' -import { columnTypeById } from '@/lib/table/column-types' import { addTableColumn, deleteColumn, deleteColumns, renameColumn, - updateColumnConstraints, - updateColumnCurrency, - updateColumnOptions, - updateColumnType, } from '@/lib/table/columns/service' import { isSupportedCurrencyCode } from '@/lib/table/currency' import { markTableDeleteFailed, runTableDelete } from '@/lib/table/delete-runner' import { runTableImport, type TableImportPayload } from '@/lib/table/import-runner' import { markTableJobRunning, releaseJobClaim } from '@/lib/table/jobs/service' import { assertRowDelete, assertRowUpdate, patchColumnIds } from '@/lib/table/mutation-locks' +import { + performDeleteTable, + performRenameTable, + performUpdateTableColumn, +} from '@/lib/table/orchestration' import { predicateToFilter } from '@/lib/table/query-builder/converters' import { validatePredicate, validateSortSpec } from '@/lib/table/query-builder/validate' import { assertCursorSortBinding, decodeCursor } from '@/lib/table/rows/cursor' @@ -66,13 +60,13 @@ import { updateRow, updateRowsByFilter, } from '@/lib/table/rows/service' +import { normalizeSelectOptionsInput } from '@/lib/table/select-options' import { predicateToStorage } from '@/lib/table/select-values' -import { createTable, deleteTable, getTableById, renameTable } from '@/lib/table/service' +import { createTable, deleteTable, getTableById } from '@/lib/table/service' import type { ColumnDefinition, Filter, RowData, - SelectOption, SortSpec, TableDefinition, TableDeleteJobPayload, @@ -334,30 +328,6 @@ function limitError(limit: unknown): string | null { * cell data survives the update. Non-array input returns `undefined`, letting * downstream validation reject a malformed / missing option set. */ -export function normalizeSelectOptionsInput( - raw: unknown, - existing: SelectOption[] = [] -): SelectOption[] | undefined { - if (!Array.isArray(raw)) return undefined - // Cells reference the option id, so an edit that re-sends the same option by - // name must reuse its id — minting a fresh one would orphan every cell - // holding it, silently clearing the column. - const idByName = new Map() - for (const option of existing) { - const key = option.name.toLowerCase() - if (!idByName.has(key)) idByName.set(key, option.id) - } - const resolveId = (name: string): string => idByName.get(name.toLowerCase()) ?? generateShortId() - - return raw.map((entry) => { - if (typeof entry === 'string') return { id: resolveId(entry), name: entry } - const e = (entry ?? {}) as { id?: unknown; name?: unknown } - const name = typeof e.name === 'string' ? e.name : String(e.name ?? '') - const id = typeof e.id === 'string' && e.id.length > 0 ? e.id : resolveId(name) - return { id, name } - }) -} - /** Rewrites every `select` column's options in an agent-authored create schema. */ function normalizeSchemaSelectColumns(schema: TableSchema): TableSchema { if (!schema || !Array.isArray(schema.columns)) return schema @@ -523,13 +493,14 @@ export const userTableServerTool: BaseServerTool const requestId = generateId().slice(0, 8) assertNotAborted() - await deleteTable(tableId, requestId, context.userId) - captureServerEvent( - context.userId, - 'table_deleted', - { table_id: tableId, workspace_id: workspaceId }, - { groups: { workspace: workspaceId } } - ) + const deleteOutcome = await performDeleteTable({ + table, + userId: context.userId, + requestId, + }) + if (!deleteOutcome.success) { + return { success: false, message: deleteOutcome.error ?? 'Failed to delete table' } + } deleted.push(tableId) } @@ -1683,117 +1654,38 @@ export const userTableServerTool: BaseServerTool message: `Invalid currency code "${currencyCode}". Use an ISO 4217 code, e.g. USD`, } } - const tableForUpdate = await getTableById(args.tableId) - if (!tableForUpdate || tableForUpdate.workspaceId !== workspaceId) { - return { success: false, message: `Table not found: ${args.tableId}` } - } - const requestId = generateId().slice(0, 8) - // The agent authors options by name; mint ids here, reusing the id of - // any option whose name already exists so its cells survive the edit. - const currentColumn = tableForUpdate.schema.columns.find((c) => - columnMatchesRef(c, colName) - ) - const existingOptions = currentColumn?.options ?? [] - const options = normalizeSelectOptionsInput(rawOptions, existingOptions) - // An agent restating the current type alongside new options must not - // go through `updateColumnType` — it early-returns on an unchanged - // type and would drop them. Mirrors the HTTP columns route. - const typeChanging = newType !== undefined && newType !== currentColumn?.type - let result: TableDefinition | undefined if (newType !== undefined && !(COLUMN_TYPES as readonly string[]).includes(newType)) { return { success: false, message: `Invalid column type "${newType}". Must be one of: ${COLUMN_TYPES.join(', ')}`, } } - // Each write below is its own locked transaction, so pairing any of - // them with a constraint write that is going to fail commits and then - // errors. Gate on the type the column ENDS UP with — an options-only - // update on an existing select column carries the same hazard as a - // conversion. Same guard the HTTP column routes apply. - const resultingType = newType ?? currentColumn?.type - if (uniqFlag === true && !columnTypeById(resultingType).supportsUnique) { - return { - success: false, - message: `Cannot set column "${colName}" as unique: ${resultingType} columns cannot be unique.`, - } - } - if (typeChanging) { - assertNotAborted() - result = await updateColumnType( - { - tableId: args.tableId, - columnName: colName, - newType: newType as (typeof COLUMN_TYPES)[number], - options, - multiple, - ...(currencyCode !== undefined ? { currencyCode } : {}), - ...(uniqFlag !== undefined ? { unique: uniqFlag } : {}), - }, - requestId - ) - } else if (currencyCode !== undefined) { - // Re-denominating an existing currency column: schema-only, no cell - // rewrite. Mirrors the HTTP columns routes. - if (currentColumn?.type !== 'currency') { - return { - success: false, - message: `Column "${colName}" is not a currency column. Pass newType: "currency" with currencyCode to convert it.`, - } - } - assertNotAborted() - result = await updateColumnCurrency( - { - tableId: args.tableId, - columnName: colName, - currencyCode, - ...(uniqFlag !== undefined ? { unique: uniqFlag } : {}), - }, - requestId - ) - } else if (options !== undefined || multiple !== undefined) { - // Editing an existing select column's option set / mode without a - // type change. `multiple` alone is a valid update — the catalog - // documents it as independent — so fall back to the column's current - // options rather than demanding the caller resend the whole list. - const nextOptions = options ?? existingOptions - if (nextOptions.length === 0) { - return { - success: false, - message: `Column "${colName}" is not a select column. Pass newType: "select" with options to convert it.`, - } - } - assertNotAborted() - result = await updateColumnOptions( - { - tableId: args.tableId, - columnName: colName, - options: nextOptions, - multiple, - ...(uniqFlag !== undefined ? { unique: uniqFlag } : {}), - }, - requestId - ) + const tableForUpdate = await getTableById(args.tableId) + if (!tableForUpdate || tableForUpdate.workspaceId !== workspaceId) { + return { success: false, message: `Table not found: ${args.tableId}` } } - // Skipped when a typed write ran: that write already applied and - // validated the constraint, in one transaction with the change it - // accompanies. Mirrors the HTTP columns routes. - if (uniqFlag !== undefined && result === undefined) { - assertNotAborted() - result = await updateColumnConstraints( - { tableId: args.tableId, columnName: colName, unique: uniqFlag }, - requestId - ) + assertNotAborted() + const outcome = await performUpdateTableColumn({ + table: tableForUpdate, + columnName: colName, + userId: context.userId, + updates: { + ...(newType !== undefined ? { type: newType as (typeof COLUMN_TYPES)[number] } : {}), + ...(uniqFlag !== undefined ? { unique: uniqFlag } : {}), + ...(rawOptions !== undefined ? { options: rawOptions } : {}), + ...(multiple !== undefined ? { multiple } : {}), + ...(currencyCode !== undefined ? { currencyCode } : {}), + }, + }) + if (!outcome.success || !outcome.table) { + return { success: false, message: outcome.error ?? 'Failed to update column' } } return { success: true, message: `Updated column "${colName}"`, - // A payload that only restates the current type is a no-op; still - // report the live schema rather than an undefined one. - data: { schema: (result ?? tableForUpdate).schema }, + data: { schema: outcome.table.schema }, } } - case 'rename': { if (!args.tableId) { return { success: false, message: 'Table ID is required' } @@ -1813,12 +1705,20 @@ export const userTableServerTool: BaseServerTool const requestId = generateId().slice(0, 8) assertNotAborted() - const renamed = await renameTable(args.tableId, newName, requestId, context.userId) + const renameOutcome = await performRenameTable({ + table, + newName, + userId: context.userId, + requestId, + }) + if (!renameOutcome.success) { + return { success: false, message: renameOutcome.error ?? 'Failed to rename table' } + } return { success: true, - message: `Renamed table to "${renamed.name}"`, - data: { table: { id: renamed.id, name: renamed.name } }, + message: `Renamed table to "${newName}"`, + data: { table: { id: args.tableId, name: newName } }, } } diff --git a/apps/sim/lib/workflows/orchestration/types.test.ts b/apps/sim/lib/core/orchestration/types.test.ts similarity index 82% rename from apps/sim/lib/workflows/orchestration/types.test.ts rename to apps/sim/lib/core/orchestration/types.test.ts index 26be2502be4..af8104841a1 100644 --- a/apps/sim/lib/workflows/orchestration/types.test.ts +++ b/apps/sim/lib/core/orchestration/types.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' +import { statusForOrchestrationError } from '@/lib/core/orchestration/types' describe('statusForOrchestrationError', () => { it.each([ diff --git a/apps/sim/lib/core/orchestration/types.ts b/apps/sim/lib/core/orchestration/types.ts new file mode 100644 index 00000000000..59ccf54aad6 --- /dev/null +++ b/apps/sim/lib/core/orchestration/types.ts @@ -0,0 +1,74 @@ +export type OrchestrationErrorCode = + | 'validation' + | 'not_found' + | 'forbidden' + | 'conflict' + | 'locked' + | 'internal' + +/** + * Transport-neutral failure classes returned by every `lib/[resource]/orchestration` + * module, so the UI routes, the public API, and the copilot tools map the same + * failure to the same status. + */ +export function statusForOrchestrationError(code: OrchestrationErrorCode | undefined): number { + if (code === 'validation') return 400 + if (code === 'forbidden') return 403 + if (code === 'not_found') return 404 + if (code === 'conflict') return 409 + if (code === 'locked') return 423 + return 500 +} + +/** + * A domain failure that already knows its own class. + * + * Services throw this instead of a bare `Error` whenever the failure is + * caller-fixable, so the layers above classify by `instanceof` and read `code` + * rather than searching the message for a phrase. Message text is then free to + * be reworded, translated, or made more specific without silently changing the + * status every caller returns — the failure mode this replaced, where adding + * "already exists" to a message demoted a 409 to a 400. + * + * The code is transport-neutral on purpose: `statusForOrchestrationError` maps + * it for the UI and v1 routes, `v2ErrorForOrchestration` maps it to the v2 + * error vocabulary, and the copilot tools surface `message` with no status at + * all. An anything-else error stays unclassified and becomes a generic 500, + * which is what an unexpected fault should be. + */ +export class OrchestrationError extends Error { + constructor( + readonly code: OrchestrationErrorCode, + message: string + ) { + super(message) + this.name = 'OrchestrationError' + } +} + +/** + * The {@link OrchestrationError} in `error`'s cause chain, or `null` when the + * failure is not a classified one. + * + * Walks `cause` rather than testing `error` alone because drizzle wraps a throw + * raised inside a transaction callback in a `DrizzleQueryError` whose own + * message is the failed SQL — the same reason the message-matching this + * replaced had to dig for a root cause before it could classify anything. + */ +export function asOrchestrationError(error: unknown): OrchestrationError | null { + let current: unknown = error + while (current instanceof Error) { + if (current instanceof OrchestrationError) return current + current = current.cause + } + return null +} + +/** + * The slice of an HTTP request the audit log reads for client IP and user-agent + * capture. Optional on every orchestration function so the non-HTTP callers — + * copilot tools, background jobs — can omit what they do not have. + */ +export interface OrchestrationRequestContext { + headers: { get(name: string): string | null } +} diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 4c6b3d6333e..5c69755b75e 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -304,7 +304,7 @@ async function archiveTableChildren(context: CascadeChildrenContext): Promise { assertSchemaMutable(table) if (!NAME_PATTERN.test(column.name)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column name "${column.name}". Must start with a letter or underscore and contain only alphanumeric characters and underscores.` ) } if (column.name.length > TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column name exceeds maximum length (${TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH} characters)` ) } if (!COLUMN_TYPES.includes(column.type as (typeof COLUMN_TYPES)[number])) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column type "${column.type}". Must be one of: ${COLUMN_TYPES.join(', ')}` ) } const schema = table.schema if (schema.columns.some((c) => c.name.toLowerCase() === column.name.toLowerCase())) { - throw new Error(`Column "${column.name}" already exists`) + throw new OrchestrationError('validation', `Column "${column.name}" already exists`) } if (schema.columns.length >= TABLE_LIMITS.MAX_COLUMNS_PER_TABLE) { - throw new Error( + throw new OrchestrationError( + 'validation', `Table has reached maximum column limit (${TABLE_LIMITS.MAX_COLUMNS_PER_TABLE})` ) } @@ -124,7 +135,10 @@ export async function addTableColumn( const columnValidation = validateColumnDefinition(newColumn) if (!columnValidation.valid) { - throw new Error(`Invalid column: ${columnValidation.errors.join('; ')}`) + throw new OrchestrationError( + 'validation', + `Invalid column: ${columnValidation.errors.join('; ')}` + ) } const newColumnId = getColumnId(newColumn) @@ -194,13 +208,15 @@ export async function renameColumn( return withLockedTable(data.tableId, async (table, trx) => { assertSchemaMutable(table) if (!NAME_PATTERN.test(data.newName)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column name "${data.newName}". Column names must start with a letter or underscore, followed by alphanumeric characters or underscores.` ) } if (data.newName.length > TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column name exceeds maximum length (${TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH} characters)` ) } @@ -208,7 +224,7 @@ export async function renameColumn( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.oldName)) if (columnIndex === -1) { - throw new Error(`Column "${data.oldName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.oldName}" not found`) } if ( @@ -216,7 +232,7 @@ export async function renameColumn( (c, i) => i !== columnIndex && c.name.toLowerCase() === data.newName.toLowerCase() ) ) { - throw new Error(`Column "${data.newName}" already exists`) + throw new OrchestrationError('validation', `Column "${data.newName}" already exists`) } const targetColumn = schema.columns[columnIndex] @@ -331,11 +347,11 @@ export async function deleteColumn( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.columnName)) if (columnIndex === -1) { - throw new Error(`Column "${data.columnName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.columnName}" not found`) } if (schema.columns.length <= 1) { - throw new Error('Cannot delete the last column in a table') + throw new OrchestrationError('validation', 'Cannot delete the last column in a table') } const targetColumn = schema.columns[columnIndex] @@ -423,12 +439,12 @@ export async function deleteColumns( } if (notFound.length > 0) { - throw new Error(`Columns not found: ${notFound.join(', ')}`) + throw new OrchestrationError('not_found', `Columns not found: ${notFound.join(', ')}`) } const remaining = schema.columns.filter((c) => !namesToDelete.has(c.name)) if (remaining.length === 0) { - throw new Error('Cannot delete all columns from a table') + throw new OrchestrationError('validation', 'Cannot delete all columns from a table') } // For each group, drop outputs whose column (by id) is being deleted. Groups @@ -506,26 +522,32 @@ async function applyConstraints( if (data.required === undefined && data.unique === undefined) return column if (column.workflowGroupId) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot change constraints on workflow-output column "${column.name}". Constraints aren't applicable to columns whose values come from workflow execution.` ) } if (data.required === true && !column.required) { const emptyCount = await countEmptyCells(trx, tableId, columnKey) if (emptyCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot set column "${column.name}" as required: ${emptyCount} row(s) have null, missing, or empty values` ) } } if (data.unique === true && !column.unique) { if (!columnTypeOf(column).supportsUnique) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot set column "${column.name}" as unique: ${column.type} columns compare stored values that would allow only one row per value.` ) } if (await hasDuplicateValues(trx, tableId, columnKey)) { - throw new Error(`Cannot set column "${column.name}" as unique: duplicate values exist`) + throw new OrchestrationError( + 'validation', + `Cannot set column "${column.name}" as unique: duplicate values exist` + ) } } return { @@ -592,17 +614,19 @@ export function applyPendingRename( if (newName === undefined || newName === column.name) return column if (!NAME_PATTERN.test(newName)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column name "${newName}". Column names must start with a letter or underscore, followed by alphanumeric characters or underscores.` ) } if (newName.length > TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column name exceeds maximum length (${TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH} characters)` ) } if (columns.some((c, i) => i !== columnIndex && c.name.toLowerCase() === newName.toLowerCase())) { - throw new Error(`Column "${newName}" already exists`) + throw new OrchestrationError('validation', `Column "${newName}" already exists`) } return { ...column, name: newName } } @@ -688,7 +712,8 @@ export async function updateColumnType( await setTableTxTimeouts(trx, { statementMs: timeoutMs, idleMs: timeoutMs }) if (!(COLUMN_TYPES as readonly string[]).includes(data.newType)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column type "${data.newType}". Valid types: ${COLUMN_TYPES.join(', ')}` ) } @@ -696,7 +721,7 @@ export async function updateColumnType( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.columnName)) if (columnIndex === -1) { - throw new Error(`Column "${data.columnName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.columnName}" not found`) } const column = schema.columns[columnIndex] @@ -714,7 +739,8 @@ export async function updateColumnType( data.multiple !== undefined || data.currencyCode !== undefined if (carriesOtherWork) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column "${column.name}" is already type "${data.newType}"; re-issue the request without a type change.` ) } @@ -760,7 +786,8 @@ export async function updateColumnType( if (targetRequired) { const emptyCount = await countEmptyCells(trx, data.tableId, columnKey) if (emptyCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot change column "${column.name}" to a required "${data.newType}": ${emptyCount} row(s) have null, missing, or empty values. Fill them first, or apply the type change without making the column required.` ) } @@ -828,13 +855,15 @@ export async function updateColumnType( } if (blankCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot change column "${column.name}" to a required "${data.newType}": ${blankCount} row(s) are empty. Fill them first, or apply the type change without making the column required.` ) } if (incompatibleCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot change column "${column.name}" to type "${data.newType}": ${incompatibleCount} row(s) have incompatible values. Fix or remove the incompatible values first.` ) } @@ -846,7 +875,10 @@ export async function updateColumnType( const columnValidation = validateColumnDefinition(updatedColumns[columnIndex]) if (!columnValidation.valid) { - throw new Error(`Invalid column: ${columnValidation.errors.join('; ')}`) + throw new OrchestrationError( + 'validation', + `Invalid column: ${columnValidation.errors.join('; ')}` + ) } const updatedSchema: TableSchema = { ...schema, columns: updatedColumns } @@ -879,7 +911,8 @@ export async function updateColumnType( // irrecoverably rewritten. if (data.unique === true && !column.unique) { if (await hasDuplicateValues(trx, data.tableId, columnKey)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot change column "${column.name}" to type "${data.newType}" and set it as unique: the converted values contain duplicates.` ) } @@ -926,7 +959,7 @@ export async function updateColumnConstraints( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.columnName)) if (columnIndex === -1) { - throw new Error(`Column "${data.columnName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.columnName}" not found`) } const column = schema.columns[columnIndex] @@ -966,12 +999,15 @@ export async function updateColumnOptions( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.columnName)) if (columnIndex === -1) { - throw new Error(`Column "${data.columnName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.columnName}" not found`) } const column = schema.columns[columnIndex] if (column.type !== 'select') { - throw new Error(`Cannot set options on column "${column.name}" of type "${column.type}"`) + throw new OrchestrationError( + 'validation', + `Cannot set options on column "${column.name}" of type "${column.type}"` + ) } const columnKey = getColumnId(column) @@ -984,7 +1020,10 @@ export async function updateColumnOptions( } const columnValidation = validateColumnDefinition(updatedColumn) if (!columnValidation.valid) { - throw new Error(`Invalid column: ${columnValidation.errors.join('; ')}`) + throw new OrchestrationError( + 'validation', + `Invalid column: ${columnValidation.errors.join('; ')}` + ) } const nextMultiple = !!(data.multiple ?? column.multiple) @@ -1033,7 +1072,8 @@ export async function updateColumnOptions( wasMultiple ) if (strandedCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot remove options from required column "${column.name}": ${strandedCount} row(s) would be left empty. Reassign those rows to a remaining option first.` ) } @@ -1060,7 +1100,8 @@ export async function updateColumnOptions( } if (multiValuedCount > 0) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot switch column "${column.name}" to single-select: ${multiValuedCount} row(s) have multiple options selected. Reduce them to one option first.` ) } @@ -1132,12 +1173,15 @@ export async function updateColumnCurrency( const schema = table.schema const columnIndex = schema.columns.findIndex((c) => columnMatchesRef(c, data.columnName)) if (columnIndex === -1) { - throw new Error(`Column "${data.columnName}" not found`) + throw new OrchestrationError('not_found', `Column "${data.columnName}" not found`) } const column = schema.columns[columnIndex] if (column.type !== 'currency') { - throw new Error(`Cannot set currency on column "${column.name}" of type "${column.type}"`) + throw new OrchestrationError( + 'validation', + `Cannot set currency on column "${column.name}" of type "${column.type}"` + ) } const updatedColumn: ColumnDefinition = { @@ -1146,7 +1190,10 @@ export async function updateColumnCurrency( } const columnValidation = validateColumnDefinition(updatedColumn) if (!columnValidation.valid) { - throw new Error(`Invalid column: ${columnValidation.errors.join('; ')}`) + throw new OrchestrationError( + 'validation', + `Invalid column: ${columnValidation.errors.join('; ')}` + ) } const constrained = await applyConstraints( diff --git a/apps/sim/lib/table/import-data.ts b/apps/sim/lib/table/import-data.ts index 84664e077b2..f8e5fd8d0c6 100644 --- a/apps/sim/lib/table/import-data.ts +++ b/apps/sim/lib/table/import-data.ts @@ -9,6 +9,7 @@ import { userTableDefinitions, userTableRows } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { eq } from 'drizzle-orm' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' import { CSV_MAX_BATCH_SIZE } from '@/lib/table/import' import { assertRowDelete, assertRowInsert, assertSchemaMutable } from '@/lib/table/mutation-locks' @@ -77,11 +78,17 @@ export async function bulkInsertImportBatch( for (let i = 0; i < data.rows.length; i++) { const sizeValidation = validateRowSize(data.rows[i]) if (!sizeValidation.valid) { - throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${sizeValidation.errors.join(', ')}` + ) } const schemaValidation = coerceRowToSchema(data.rows[i], table.schema) if (!schemaValidation.valid) { - throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${schemaValidation.errors.join(', ')}` + ) } } @@ -94,7 +101,8 @@ export async function bulkInsertImportBatch( db ) if (!uniqueResult.valid) { - throw new Error( + throw new OrchestrationError( + 'validation', uniqueResult.errors.map((e) => `Row ${e.row + 1}: ${e.errors.join(', ')}`).join('; ') ) } diff --git a/apps/sim/lib/table/import.ts b/apps/sim/lib/table/import.ts index 02c8a6e431a..3759e4eefe0 100644 --- a/apps/sim/lib/table/import.ts +++ b/apps/sim/lib/table/import.ts @@ -12,6 +12,7 @@ */ import { type Options as CsvParseOptions, type Parser, parse as parseCsvStream } from 'csv-parse' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { getColumnId } from '@/lib/table/column-keys' import type { ColumnType } from '@/lib/table/column-types' import { parseCurrencyInput } from '@/lib/table/currency' @@ -274,11 +275,11 @@ export async function parseCsvBuffer( const parsed = parse(text, options) as unknown as Record[] if (parsed.length === 0) { - throw new Error('CSV file has no data rows') + throw new OrchestrationError('validation', 'CSV file has no data rows') } if (headers.length === 0) { - throw new Error('CSV file has no headers') + throw new OrchestrationError('validation', 'CSV file has no headers') } return { headers, rows: parsed } @@ -648,15 +649,18 @@ export function parseJsonRows(buffer: Buffer | string): { const text = typeof buffer === 'string' ? buffer : buffer.toString('utf-8') const parsed = JSON.parse(text) if (!Array.isArray(parsed)) { - throw new Error('JSON file must contain an array of objects') + throw new OrchestrationError('validation', 'JSON file must contain an array of objects') } if (parsed.length === 0) { - throw new Error('JSON file contains an empty array') + throw new OrchestrationError('validation', 'JSON file contains an empty array') } const headerSet = new Set() for (const row of parsed) { if (typeof row !== 'object' || row === null || Array.isArray(row)) { - throw new Error('Each element in the JSON array must be a plain object') + throw new OrchestrationError( + 'validation', + 'Each element in the JSON array must be a plain object' + ) } for (const key of Object.keys(row)) headerSet.add(key) } @@ -685,5 +689,8 @@ export async function parseFileRows( ) return parseCsvBuffer(buffer, delimiter) } - throw new Error(`Unsupported file format: "${ext ?? fileName}". Supported: csv, tsv, json`) + throw new OrchestrationError( + 'validation', + `Unsupported file format: "${ext ?? fileName}". Supported: csv, tsv, json` + ) } diff --git a/apps/sim/lib/table/orchestration/columns.test.ts b/apps/sim/lib/table/orchestration/columns.test.ts new file mode 100644 index 00000000000..d99eff54342 --- /dev/null +++ b/apps/sim/lib/table/orchestration/columns.test.ts @@ -0,0 +1,224 @@ +/** + * @vitest-environment node + * + * The column-update guards. These used to live in four callers (UI route, v1, + * v2, copilot tool) and had drifted apart; they are asserted here once. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TableDefinition } from '@/lib/table/types' + +const { + mockRenameColumn, + mockUpdateColumnType, + mockUpdateColumnOptions, + mockUpdateColumnConstraints, + mockUpdateColumnCurrency, + mockRecordAudit, +} = vi.hoisted(() => ({ + mockRenameColumn: vi.fn(), + mockUpdateColumnType: vi.fn(), + mockUpdateColumnOptions: vi.fn(), + mockUpdateColumnConstraints: vi.fn(), + mockUpdateColumnCurrency: vi.fn(), + mockRecordAudit: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { TABLE_UPDATED: 'table.updated' }, + AuditResourceType: { TABLE: 'table' }, + recordAudit: mockRecordAudit, +})) + +vi.mock('@/lib/table/columns/service', () => ({ + renameColumn: mockRenameColumn, + updateColumnConstraints: mockUpdateColumnConstraints, + updateColumnCurrency: mockUpdateColumnCurrency, + updateColumnOptions: mockUpdateColumnOptions, + updateColumnType: mockUpdateColumnType, +})) + +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { TableLockedError } from '@/lib/table/mutation-locks' +import { performUpdateTableColumn } from '@/lib/table/orchestration/columns' + +const SELECT_COLUMN = { + id: 'col-1', + name: 'Status', + type: 'select' as const, + options: [{ id: 'opt_open', name: 'Open' }], +} +const TEXT_COLUMN = { id: 'col-2', name: 'Priority', type: 'text' as const } + +const TABLE = { + id: 'table-1', + name: 'Tasks', + workspaceId: 'ws-1', + schema: { columns: [SELECT_COLUMN, TEXT_COLUMN] }, +} as unknown as TableDefinition + +const UPDATED = { schema: { columns: [SELECT_COLUMN] } } as unknown as TableDefinition + +function run(updates: Record, columnName = 'Status') { + return performUpdateTableColumn({ + table: TABLE, + columnName, + userId: 'user-1', + updates, + requestId: 'req-1', + }) +} + +describe('performUpdateTableColumn', () => { + beforeEach(() => { + vi.clearAllMocks() + mockRenameColumn.mockResolvedValue(UPDATED) + mockUpdateColumnType.mockResolvedValue(UPDATED) + mockUpdateColumnOptions.mockResolvedValue(UPDATED) + mockUpdateColumnConstraints.mockResolvedValue(UPDATED) + mockUpdateColumnCurrency.mockResolvedValue(UPDATED) + }) + + it('refuses to make a select column unique before writing anything', async () => { + // Each write is its own locked transaction, so an un-gated constraint write + // commits the earlier writes and then throws, half-applying the change. + const result = await run({ unique: true }) + + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(mockUpdateColumnConstraints).not.toHaveBeenCalled() + }) + + it('refuses a conversion to select that is also made unique', async () => { + const result = await run({ type: 'select', options: ['Done'], unique: true }, 'Priority') + + expect(result.errorCode).toBe('validation') + expect(mockUpdateColumnType).not.toHaveBeenCalled() + }) + + it('routes an unchanged type with options to the options update', async () => { + // updateColumnType early-returns on an unchanged type and would drop them. + await run({ type: 'select', options: ['Open', 'Closed'] }) + + expect(mockUpdateColumnType).not.toHaveBeenCalled() + // Addressed by stable id so a rename folded into the write can't break it. + expect(mockUpdateColumnOptions).toHaveBeenCalledWith( + expect.objectContaining({ columnName: 'col-1' }), + 'req-1' + ) + }) + + it('reuses the id of an option resent by name so its cells survive', async () => { + await run({ options: ['Open', 'Blocked'] }) + + const [{ options }] = mockUpdateColumnOptions.mock.calls[0] + expect(options[0]).toEqual({ id: 'opt_open', name: 'Open' }) + expect(options[1].id).not.toBe('opt_open') + }) + + it('carries options and required through a real type change', async () => { + await run({ type: 'select', options: ['Done'], required: true }, 'Priority') + + expect(mockUpdateColumnOptions).not.toHaveBeenCalled() + expect(mockUpdateColumnType).toHaveBeenCalledWith( + expect.objectContaining({ newType: 'select', required: true }), + 'req-1' + ) + }) + + it('folds a rename into the write it rides on rather than running it separately', async () => { + // A rename is metadata-only, so folding it into the last write's transaction + // is what stops a combined request committing one half and failing the other. + await run({ name: 'State', required: true }) + + expect(mockRenameColumn).not.toHaveBeenCalled() + expect(mockUpdateColumnConstraints).toHaveBeenCalledWith( + expect.objectContaining({ columnName: 'col-1', newName: 'State' }), + 'req-1' + ) + }) + + it('runs a rename standalone when there is no write to ride on', async () => { + mockRenameColumn.mockResolvedValue(UPDATED) + + await run({ name: 'State' }) + + expect(mockRenameColumn).toHaveBeenCalledWith( + { tableId: 'table-1', oldName: 'col-1', newName: 'State' }, + 'req-1' + ) + }) + + it('rejects setting a currency code on a non-currency column', async () => { + const result = await run({ currencyCode: 'USD' }) + + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(mockUpdateColumnCurrency).not.toHaveBeenCalled() + }) + + it('rejects an unsupported currency code before any write', async () => { + const result = await run({ type: 'currency', currencyCode: 'XX' }, 'Priority') + + expect(result.errorCode).toBe('validation') + expect(mockUpdateColumnType).not.toHaveBeenCalled() + }) + + it('reports an empty payload as a validation failure', async () => { + const result = await run({}) + + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(result.error).toBe('No updates specified') + expect(mockRecordAudit).not.toHaveBeenCalled() + }) + + it("names the type when a payload only restates the column's current type", async () => { + const result = await run({ type: 'select' }) + + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(result.error).toContain('is already type "select"') + expect(mockUpdateColumnType).not.toHaveBeenCalled() + }) + + it('classifies a table lock as locked and does not audit', async () => { + mockUpdateColumnConstraints.mockRejectedValue(new TableLockedError('update')) + + const result = await run({ required: true }) + + expect(result.errorCode).toBe('locked') + expect(mockRecordAudit).not.toHaveBeenCalled() + }) + + it('reports the code the service failure carries', async () => { + mockUpdateColumnConstraints.mockRejectedValue( + new OrchestrationError('validation', 'Column "State" already exists') + ) + + expect((await run({ required: true })).errorCode).toBe('validation') + }) + + it('classifies a missing column as not_found', async () => { + mockUpdateColumnConstraints.mockRejectedValue( + new OrchestrationError('not_found', 'Column "Nope" not found') + ) + + expect((await run({ required: true })).errorCode).toBe('not_found') + }) + + it('keeps an unclassified fault internal and hides its message', async () => { + // Wording alone must never buy a status: this reads exactly like the + // caller-fixable failure above but carries no classification. + mockUpdateColumnConstraints.mockRejectedValue(new Error('Column "State" already exists')) + + const result = await run({ required: true }) + + expect(result.errorCode).toBe('internal') + expect(result.error).toBe('Failed to update column') + }) + + it('audits a successful update on every caller', async () => { + // The UI route and the copilot tool previously emitted no audit at all. + await run({ required: true }) + + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: 'ws-1', actorId: 'user-1', resourceId: 'table-1' }) + ) + }) +}) diff --git a/apps/sim/lib/table/orchestration/columns.ts b/apps/sim/lib/table/orchestration/columns.ts new file mode 100644 index 00000000000..18d321d3e3b --- /dev/null +++ b/apps/sim/lib/table/orchestration/columns.ts @@ -0,0 +1,266 @@ +import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { createLogger } from '@sim/logger' +import { + OrchestrationError, + type OrchestrationErrorCode, + type OrchestrationRequestContext, +} from '@/lib/core/orchestration/types' +import { generateRequestId } from '@/lib/core/utils/request' +import { columnMatchesRef, getColumnId } from '@/lib/table/column-keys' +import { columnTypeById } from '@/lib/table/column-types' +import { + renameColumn, + updateColumnConstraints, + updateColumnCurrency, + updateColumnOptions, + updateColumnType, +} from '@/lib/table/columns/service' +import { isSupportedCurrencyCode } from '@/lib/table/currency' +import { TableLockedError } from '@/lib/table/mutation-locks' +import { normalizeSelectOptionsInput } from '@/lib/table/select-options' +import type { ColumnType, SelectOption, TableDefinition } from '@/lib/table/types' + +const logger = createLogger('TableColumnOrchestration') + +export interface PerformUpdateTableColumnParams { + table: TableDefinition + columnName: string + userId: string + updates: { + name?: string + type?: ColumnType + required?: boolean + unique?: boolean + /** Accepts `{id,name}` pairs or bare names; ids are minted/reused as needed. */ + options?: unknown + multiple?: boolean + currencyCode?: string + } + requestId?: string + /** Forwarded to the audit record for IP / user-agent capture. */ + request?: OrchestrationRequestContext +} + +export interface PerformUpdateTableColumnResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + table?: TableDefinition +} + +function classify(error: unknown): PerformUpdateTableColumnResult { + if (error instanceof TableLockedError) { + return { success: false, error: error.message, errorCode: 'locked' } + } + if (error instanceof OrchestrationError) { + return { success: false, error: error.message, errorCode: error.code } + } + return { success: false, error: 'Failed to update column', errorCode: 'internal' } +} + +function fail(error: string, errorCode: OrchestrationErrorCode): PerformUpdateTableColumnResult { + return { success: false, error, errorCode } +} + +/** + * Applies a column update — rename, type conversion, currency re-denomination, + * option-set edit, and constraint change — as the single implementation behind + * the UI route, the v1 and v2 public APIs, and the copilot table tool. + * + * Each underlying write is its own locked transaction, so a request that spans + * several of them could commit one and then fail. Two things prevent that and + * both are load-bearing: the guards below reject every knowable failure before + * any write runs, and the rename and constraint changes ride *inside* the typed + * write's transaction rather than following it. The caller owns authentication + * and workspace scoping; by the time this runs, `table` is one the actor may write. + */ +export async function performUpdateTableColumn( + params: PerformUpdateTableColumnParams +): Promise { + const { table, columnName, userId, updates, request } = params + const requestId = params.requestId ?? generateRequestId() + const tableId = table.id + + const currentColumn = table.schema.columns.find((c) => columnMatchesRef(c, columnName)) + if (!currentColumn) { + return fail(`Column "${columnName}" not found`, 'not_found') + } + + // Address every write by the stable id, not the name: a rename folded into + // one of them must not break the next one's lookup. + const columnRef = getColumnId(currentColumn) + const existingOptions: SelectOption[] = currentColumn.options ?? [] + const options = normalizeSelectOptionsInput(updates.options, existingOptions) + + // A payload that repeats the current type must not go through + // `updateColumnType` — it early-returns on an unchanged type and would drop + // any options alongside it. Only a real type change routes there. + const typeChanging = updates.type !== undefined && updates.type !== currentColumn.type + + // A retype applies and validates the constraints itself, so the separate + // constraint write only runs when no typed write does. The rename rides + // whichever write actually runs last. + const typedWriteRuns = + typeChanging || + updates.currencyCode !== undefined || + options !== undefined || + updates.multiple !== undefined + const constraintsWriteRuns = + !typedWriteRuns && (updates.required !== undefined || updates.unique !== undefined) + const renameWithTypedWrite = + updates.name && !constraintsWriteRuns ? { newName: updates.name } : {} + + // Gate on the type the column ENDS UP with, not on whether the type is + // changing: an options-only update on an existing select column carries the + // same hazard as a conversion does. + const resultingType = updates.type ?? currentColumn.type + + if (updates.currencyCode !== undefined) { + if (resultingType !== 'currency') { + return fail( + `Cannot set currency on column "${columnName}" of type "${resultingType}"`, + 'validation' + ) + } + if (!isSupportedCurrencyCode(updates.currencyCode)) { + return fail( + `Invalid currency code "${updates.currencyCode}". Use an ISO 4217 code, e.g. USD`, + 'validation' + ) + } + } + // The rename runs last, so a name already taken would fail after the typed + // write committed. This is the only rename failure a caller can cause; + // catching it here leaves just the concurrent-collision race. + if ( + updates.name && + table.schema.columns.some( + (c) => + c.name.toLowerCase() === updates.name?.toLowerCase() && !columnMatchesRef(c, columnName) + ) + ) { + return fail(`Column "${updates.name}" already exists`, 'validation') + } + if ( + currentColumn.workflowGroupId && + (updates.required !== undefined || updates.unique !== undefined) + ) { + return fail( + `Cannot change constraints on workflow-output column "${currentColumn.name}". Constraints aren't applicable to columns whose values come from workflow execution.`, + 'validation' + ) + } + if (updates.unique === true && !columnTypeById(resultingType).supportsUnique) { + return fail(`Cannot set a ${resultingType} column as unique`, 'validation') + } + + let updated: TableDefinition | undefined + + try { + if (typeChanging) { + updated = await updateColumnType( + { + tableId, + columnName: columnRef, + newType: updates.type as ColumnType, + ...(options !== undefined ? { options } : {}), + ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), + ...(updates.currencyCode !== undefined ? { currencyCode: updates.currencyCode } : {}), + // Forwarded so the conversion validates against the constraints this + // same request is about to set, not the column's current ones. + ...(updates.required !== undefined ? { required: updates.required } : {}), + ...(updates.unique !== undefined ? { unique: updates.unique } : {}), + ...renameWithTypedWrite, + }, + requestId + ) + } else if (updates.currencyCode !== undefined) { + // Re-denominating an existing currency column: schema-only, no cell + // rewrite. Reached only when the type is unchanged — a conversion INTO + // currency carries the code through `updateColumnType` above. + updated = await updateColumnCurrency( + { + tableId, + columnName: columnRef, + currencyCode: updates.currencyCode, + ...(updates.required !== undefined ? { required: updates.required } : {}), + ...(updates.unique !== undefined ? { unique: updates.unique } : {}), + ...renameWithTypedWrite, + }, + requestId + ) + } else if (options !== undefined || updates.multiple !== undefined) { + updated = await updateColumnOptions( + { + tableId, + columnName: columnRef, + options: options ?? existingOptions, + ...(updates.multiple !== undefined ? { multiple: updates.multiple } : {}), + ...(updates.required !== undefined ? { required: updates.required } : {}), + ...(updates.unique !== undefined ? { unique: updates.unique } : {}), + ...renameWithTypedWrite, + }, + requestId + ) + } + + // Skipped whenever a typed write ran: that write already applied and + // validated these, in one transaction with the change they accompany. + if (constraintsWriteRuns) { + updated = await updateColumnConstraints( + { + tableId, + columnName: columnRef, + ...(updates.required !== undefined ? { required: updates.required } : {}), + ...(updates.unique !== undefined ? { unique: updates.unique } : {}), + ...(updates.name ? { newName: updates.name } : {}), + }, + requestId + ) + } + + // A rename rides along with the LAST write above, inside that write's + // transaction — a rename is metadata-only (rows key on the stable column + // id), so nothing forces it to be its own write, and folding it in is what + // stops a combined request from committing one half and then failing. Only + // a rename with nothing to ride on runs standalone. + if (updates.name && !updated) { + updated = await renameColumn( + { tableId, oldName: columnRef, newName: updates.name }, + requestId + ) + } + } catch (error) { + logger.error(`[${requestId}] Failed to update column "${columnName}" on table ${tableId}`, { + error, + }) + return classify(error) + } + + if (!updated) { + // A payload whose only content is the type the column already has names a + // change and asks for nothing. Say which, the way `updateColumnType` does + // when it loses the same race, rather than claiming the request was empty. + if (updates.type !== undefined) { + return fail( + `Column "${currentColumn.name}" is already type "${currentColumn.type}"; re-issue the request without a type change.`, + 'validation' + ) + } + return fail('No updates specified', 'validation') + } + + recordAudit({ + workspaceId: table.workspaceId, + actorId: userId, + action: AuditAction.TABLE_UPDATED, + resourceType: AuditResourceType.TABLE, + resourceId: tableId, + resourceName: table.name, + description: `Updated column "${columnName}" in table "${table.name}"`, + metadata: { columnName, updates }, + ...(request ? { request } : {}), + }) + + return { success: true, table: updated } +} diff --git a/apps/sim/lib/table/orchestration/index.ts b/apps/sim/lib/table/orchestration/index.ts index af29da6aced..b1ea82abddf 100644 --- a/apps/sim/lib/table/orchestration/index.ts +++ b/apps/sim/lib/table/orchestration/index.ts @@ -1,64 +1,9 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' -import { createLogger } from '@sim/logger' -import { toError } from '@sim/utils/errors' -import { generateRequestId } from '@/lib/core/utils/request' -import { getTableById, restoreTable, TableConflictError } from '@/lib/table/service' -import type { TableDefinition } from '@/lib/table/types' - -const logger = createLogger('TableOrchestration') - -export type TableOrchestrationErrorCode = 'not_found' | 'validation' | 'conflict' | 'internal' - -export interface PerformRestoreTableParams { - tableId: string - userId: string - requestId?: string -} - -export interface PerformRestoreTableResult { - success: boolean - error?: string - errorCode?: TableOrchestrationErrorCode - table?: TableDefinition -} - -export async function performRestoreTable( - params: PerformRestoreTableParams -): Promise { - const { tableId, userId } = params - const requestId = params.requestId ?? generateRequestId() - - const archivedTable = await getTableById(tableId, { includeArchived: true }) - if (!archivedTable) { - return { success: false, error: 'Table not found', errorCode: 'not_found' } - } - - try { - await restoreTable(tableId, requestId) - const table = (await getTableById(tableId)) ?? archivedTable - - logger.info(`[${requestId}] Restored table ${tableId}`) - - recordAudit({ - workspaceId: archivedTable.workspaceId, - actorId: userId, - action: AuditAction.TABLE_RESTORED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: table.name, - description: `Restored table "${table.name}"`, - metadata: { - tableName: table.name, - workspaceId: table.workspaceId, - }, - }) - - return { success: true, table } - } catch (error) { - logger.error(`[${requestId}] Failed to restore table ${tableId}`, { error }) - if (error instanceof TableConflictError) { - return { success: false, error: error.message, errorCode: 'conflict' } - } - return { success: false, error: toError(error).message, errorCode: 'internal' } - } -} +export { performUpdateTableColumn } from './columns' +export { performRestoreTable } from './restore' +export { + performDeleteTable, + performDeleteTableRow, + performMoveTableToFolder, + performRenameTable, + performUpdateTableLocks, +} from './tables' diff --git a/apps/sim/lib/table/orchestration/restore.ts b/apps/sim/lib/table/orchestration/restore.ts new file mode 100644 index 00000000000..1aff4de5da2 --- /dev/null +++ b/apps/sim/lib/table/orchestration/restore.ts @@ -0,0 +1,63 @@ +import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' +import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' +import { generateRequestId } from '@/lib/core/utils/request' +import { getTableById, restoreTable, TableConflictError } from '@/lib/table/service' +import type { TableDefinition } from '@/lib/table/types' + +const logger = createLogger('TableOrchestration') + +export interface PerformRestoreTableParams { + tableId: string + userId: string + requestId?: string +} + +export interface PerformRestoreTableResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + table?: TableDefinition +} + +export async function performRestoreTable( + params: PerformRestoreTableParams +): Promise { + const { tableId, userId } = params + const requestId = params.requestId ?? generateRequestId() + + const archivedTable = await getTableById(tableId, { includeArchived: true }) + if (!archivedTable) { + return { success: false, error: 'Table not found', errorCode: 'not_found' } + } + + try { + await restoreTable(tableId, requestId) + const table = (await getTableById(tableId)) ?? archivedTable + + logger.info(`[${requestId}] Restored table ${tableId}`) + + recordAudit({ + workspaceId: archivedTable.workspaceId, + actorId: userId, + action: AuditAction.TABLE_RESTORED, + resourceType: AuditResourceType.TABLE, + resourceId: tableId, + resourceName: table.name, + description: `Restored table "${table.name}"`, + metadata: { + tableName: table.name, + workspaceId: table.workspaceId, + }, + }) + + return { success: true, table } + } catch (error) { + logger.error(`[${requestId}] Failed to restore table ${tableId}`, { error }) + if (error instanceof TableConflictError) { + return { success: false, error: error.message, errorCode: 'conflict' } + } + return { success: false, error: toError(error).message, errorCode: 'internal' } + } +} diff --git a/apps/sim/lib/table/orchestration/tables.test.ts b/apps/sim/lib/table/orchestration/tables.test.ts new file mode 100644 index 00000000000..09127e8e40b --- /dev/null +++ b/apps/sim/lib/table/orchestration/tables.test.ts @@ -0,0 +1,142 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { TableDefinition } from '@/lib/table/types' + +const { mockDeleteTable, mockDeleteRow, mockRenameTable, mockCaptureServerEvent, mockRecordAudit } = + vi.hoisted(() => ({ + mockDeleteTable: vi.fn(), + mockDeleteRow: vi.fn(), + mockRenameTable: vi.fn(), + mockCaptureServerEvent: vi.fn(), + mockRecordAudit: vi.fn(), + })) + +vi.mock('@sim/audit', () => ({ + AuditAction: { TABLE_DELETED: 'table.deleted', TABLE_UPDATED: 'table.updated' }, + AuditResourceType: { TABLE: 'table' }, + recordAudit: mockRecordAudit, +})) + +vi.mock('@/lib/table/service', () => ({ + deleteTable: mockDeleteTable, + moveTableToFolder: vi.fn(), + renameTable: mockRenameTable, + updateTableLocks: vi.fn(), +})) +vi.mock('@/lib/table/rows/service', () => ({ deleteRow: mockDeleteRow })) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent })) + +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { TableLockedError } from '@/lib/table/mutation-locks' +import { + performDeleteTable, + performDeleteTableRow, + performRenameTable, +} from '@/lib/table/orchestration/tables' + +const TABLE = { id: 'table-1', name: 'Tasks', workspaceId: 'ws-1' } as unknown as TableDefinition + +describe('performDeleteTable', () => { + beforeEach(() => vi.clearAllMocks()) + + it('audits a genuine archive against the acting user', async () => { + mockDeleteTable.mockResolvedValue({ archived: { name: 'Tasks', workspaceId: 'ws-1' } }) + + const result = await performDeleteTable({ table: TABLE, userId: 'user-1', requestId: 'req-1' }) + + expect(result.success).toBe(true) + // The service no longer takes an actor — auditing follows from a user + // performing the operation, not from which function the caller reached for. + expect(mockDeleteTable).toHaveBeenCalledWith('table-1', 'req-1') + expect(mockRecordAudit).toHaveBeenCalledWith( + expect.objectContaining({ actorId: 'user-1', resourceId: 'table-1' }) + ) + expect(mockCaptureServerEvent).toHaveBeenCalledWith( + 'user-1', + 'table_deleted', + expect.objectContaining({ table_id: 'table-1' }), + expect.anything() + ) + }) + + it('carries request provenance into the audit row', async () => { + mockDeleteTable.mockResolvedValue({ archived: { name: 'Tasks', workspaceId: 'ws-1' } }) + const request = new Request('https://sim.ai', { headers: { 'user-agent': 'curl/8' } }) + + await performDeleteTable({ table: TABLE, userId: 'user-1', request }) + + expect(mockRecordAudit).toHaveBeenCalledWith(expect.objectContaining({ request })) + }) + + it('neither audits nor reports a repeat delete of an already-archived table', async () => { + mockDeleteTable.mockResolvedValue({ archived: null }) + + const result = await performDeleteTable({ table: TABLE, userId: 'user-1' }) + + expect(result.success).toBe(true) + expect(mockRecordAudit).not.toHaveBeenCalled() + expect(mockCaptureServerEvent).not.toHaveBeenCalled() + }) + + it('classifies a delete lock as locked and emits no telemetry', async () => { + mockDeleteTable.mockRejectedValue(new TableLockedError('delete')) + + const result = await performDeleteTable({ table: TABLE, userId: 'user-1' }) + + expect(result).toMatchObject({ success: false, errorCode: 'locked' }) + expect(mockCaptureServerEvent).not.toHaveBeenCalled() + }) +}) + +describe('performRenameTable', () => { + beforeEach(() => vi.clearAllMocks()) + + it('classifies a name collision as a conflict, not bad input', async () => { + // `TableConflictError` is an `OrchestrationError('conflict')` — the class + // decides the status, so the 409 no longer rides on the message wording. + mockRenameTable.mockRejectedValue( + new OrchestrationError('conflict', 'A table named "Tasks" already exists in this workspace') + ) + + const result = await performRenameTable({ table: TABLE, newName: 'Tasks', userId: 'user-1' }) + + expect(result).toMatchObject({ success: false, errorCode: 'conflict' }) + }) + + it('keeps an unclassified rename failure internal', async () => { + mockRenameTable.mockRejectedValue(new Error('A table named "Tasks" already exists')) + + expect( + (await performRenameTable({ table: TABLE, newName: 'Tasks', userId: 'user-1' })).errorCode + ).toBe('internal') + }) +}) + +describe('performDeleteTableRow', () => { + beforeEach(() => vi.clearAllMocks()) + + it('deletes through the row service so the lock and bookkeeping apply', async () => { + mockDeleteRow.mockResolvedValue(undefined) + + const result = await performDeleteTableRow({ table: TABLE, rowId: 'row-1', requestId: 'req-1' }) + + expect(result.success).toBe(true) + expect(mockDeleteRow).toHaveBeenCalledWith(TABLE, 'row-1', 'req-1') + }) + + it('classifies a delete lock as locked', async () => { + mockDeleteRow.mockRejectedValue(new TableLockedError('delete')) + + expect((await performDeleteTableRow({ table: TABLE, rowId: 'row-1' })).errorCode).toBe('locked') + }) + + it('classifies a missing row as not_found', async () => { + mockDeleteRow.mockRejectedValue(new OrchestrationError('not_found', 'Row not found')) + + expect((await performDeleteTableRow({ table: TABLE, rowId: 'row-1' })).errorCode).toBe( + 'not_found' + ) + }) +}) diff --git a/apps/sim/lib/table/orchestration/tables.ts b/apps/sim/lib/table/orchestration/tables.ts new file mode 100644 index 00000000000..dcec50a25cc --- /dev/null +++ b/apps/sim/lib/table/orchestration/tables.ts @@ -0,0 +1,274 @@ +import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' +import { + OrchestrationError, + type OrchestrationErrorCode, + type OrchestrationRequestContext, +} from '@/lib/core/orchestration/types' +import { generateRequestId } from '@/lib/core/utils/request' +import { captureServerEvent } from '@/lib/posthog/server' +import { TableLockedError } from '@/lib/table/mutation-locks' +import { deleteRow } from '@/lib/table/rows/service' +import { deleteTable, moveTableToFolder, renameTable, updateTableLocks } from '@/lib/table/service' +import { + TABLE_LOCK_FLAGS, + TABLE_LOCK_KINDS, + type TableDefinition, + type TableLocks, +} from '@/lib/table/types' + +const logger = createLogger('TableOrchestration') + +export interface PerformDeleteTableParams { + table: TableDefinition + userId: string + requestId?: string + /** Forwarded to the audit record for IP / user-agent capture. */ + request?: OrchestrationRequestContext +} + +export interface PerformDeleteTableResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode +} + +/** + * Archives a table on behalf of `userId`. + * + * The audit lives here rather than in `deleteTable` so that auditing follows + * from "a user performed this operation", not from which function a caller + * reached for. The rollback and cleanup paths call the service directly and + * are silent by construction, and a repeat delete of an already-archived table + * logs nothing because the service reports that it archived no row. + */ +export async function performDeleteTable( + params: PerformDeleteTableParams +): Promise { + const { table, userId, request } = params + const requestId = params.requestId ?? generateRequestId() + + let archived: { name: string; workspaceId: string | null } | null + try { + ;({ archived } = await deleteTable(table.id, requestId)) + } catch (error) { + if (error instanceof TableLockedError) { + return { success: false, error: error.message, errorCode: 'locked' } + } + if (error instanceof OrchestrationError) { + return { success: false, error: error.message, errorCode: error.code } + } + logger.error(`[${requestId}] Failed to delete table ${table.id}`, { error }) + return { success: false, error: toError(error).message, errorCode: 'internal' } + } + + // Both the audit and the analytics event describe an archive that happened, so + // both hang off the same evidence that one did. A repeat delete of an + // already-archived table succeeds and records nothing. + if (archived) { + recordAudit({ + workspaceId: archived.workspaceId, + actorId: userId, + action: AuditAction.TABLE_DELETED, + resourceType: AuditResourceType.TABLE, + resourceId: table.id, + resourceName: archived.name, + description: `Archived table "${archived.name}"`, + ...(request ? { request } : {}), + }) + captureServerEvent( + userId, + 'table_deleted', + { table_id: table.id, workspace_id: table.workspaceId }, + { groups: { workspace: table.workspaceId } } + ) + } + + return { success: true } +} + +export interface PerformDeleteTableRowParams { + table: TableDefinition + rowId: string + requestId?: string +} + +export interface PerformDeleteTableRowResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode +} + +/** + * Deletes a single row through the row service, so the delete lock is enforced + * and the row-count and ordering bookkeeping runs. A raw `db.delete` skips both + * and returns success on a locked table. + */ +export async function performDeleteTableRow( + params: PerformDeleteTableRowParams +): Promise { + const { table, rowId } = params + const requestId = params.requestId ?? generateRequestId() + + try { + await deleteRow(table, rowId, requestId) + return { success: true } + } catch (error) { + if (error instanceof TableLockedError) { + return { success: false, error: error.message, errorCode: 'locked' } + } + if (error instanceof OrchestrationError) { + return { success: false, error: error.message, errorCode: error.code } + } + logger.error(`[${requestId}] Failed to delete row ${rowId} from table ${table.id}`, { error }) + return { success: false, error: toError(error).message, errorCode: 'internal' } + } +} + +export interface PerformRenameTableParams { + table: TableDefinition + newName: string + userId: string + requestId?: string + /** Forwarded to the audit record for IP / user-agent capture. */ + request?: OrchestrationRequestContext +} + +export interface PerformTableMutationResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + table?: TableDefinition +} + +function classifyTableMutation(error: unknown, requestId: string, tableId: string) { + if (error instanceof TableLockedError) { + return { success: false as const, error: error.message, errorCode: 'locked' as const } + } + // `TableConflictError` is an `OrchestrationError('conflict')`, so a duplicate + // rename reaches 409 through this branch — by class, not by the message + // happening to contain "already exists". + if (error instanceof OrchestrationError) { + return { success: false as const, error: error.message, errorCode: error.code } + } + logger.error(`[${requestId}] Table mutation failed for ${tableId}`, { error }) + return { + success: false as const, + error: toError(error).message, + errorCode: 'internal' as const, + } +} + +/** Renames a table and records the rename against `userId`. */ +export async function performRenameTable( + params: PerformRenameTableParams +): Promise { + const { table, newName, userId, request } = params + const requestId = params.requestId ?? generateRequestId() + + try { + const renamed = await renameTable(table.id, newName, requestId) + recordAudit({ + workspaceId: table.workspaceId, + actorId: userId, + action: AuditAction.TABLE_UPDATED, + resourceType: AuditResourceType.TABLE, + resourceId: table.id, + resourceName: renamed.name, + description: `Renamed table to "${renamed.name}"`, + metadata: { op: 'rename', previousName: table.name }, + ...(request ? { request } : {}), + }) + return { success: true } + } catch (error) { + return classifyTableMutation(error, requestId, table.id) + } +} + +export interface PerformMoveTableParams { + table: TableDefinition + folderId: string | null + userId: string + requestId?: string + /** Forwarded to the audit record for IP / user-agent capture. */ + request?: OrchestrationRequestContext +} + +/** Moves a table between folders (or to the workspace root). */ +export async function performMoveTableToFolder( + params: PerformMoveTableParams +): Promise { + const { table, folderId, userId, request } = params + const requestId = params.requestId ?? generateRequestId() + if (!table.workspaceId) { + return { success: false, error: 'Table is not in a workspace', errorCode: 'validation' } + } + + try { + const { name } = await moveTableToFolder(table.id, table.workspaceId, folderId, requestId) + recordAudit({ + workspaceId: table.workspaceId, + actorId: userId, + action: AuditAction.TABLE_UPDATED, + resourceType: AuditResourceType.TABLE, + resourceId: table.id, + resourceName: name, + description: folderId + ? `Moved table "${name}" into a folder` + : `Moved table "${name}" to the workspace root`, + metadata: { op: 'move', folderId }, + ...(request ? { request } : {}), + }) + return { success: true } + } catch (error) { + return classifyTableMutation(error, requestId, table.id) + } +} + +export interface PerformUpdateTableLocksParams { + tableId: string + partial: Partial + userId: string + requestId?: string + /** Forwarded to the audit record for IP / user-agent capture. */ + request?: OrchestrationRequestContext +} + +/** + * Applies a lock change and names the transitions in the audit description, so + * the audit list answers "who locked my production table" without expanding + * metadata. The before/after state comes back from the service because only the + * locked write can observe it. + */ +export async function performUpdateTableLocks( + params: PerformUpdateTableLocksParams +): Promise { + const { tableId, partial, userId, request } = params + const requestId = params.requestId ?? generateRequestId() + + try { + const { table, previousLocks } = await updateTableLocks(tableId, partial, requestId) + const flipped = TABLE_LOCK_KINDS.filter( + (kind) => previousLocks[TABLE_LOCK_FLAGS[kind]] !== table.locks[TABLE_LOCK_FLAGS[kind]] + ) + recordAudit({ + workspaceId: table.workspaceId, + actorId: userId, + action: AuditAction.TABLE_UPDATED, + resourceType: AuditResourceType.TABLE, + resourceId: tableId, + resourceName: table.name, + description: flipped.length + ? `Table locks changed: ${flipped + .map((kind) => `${kind} ${table.locks[TABLE_LOCK_FLAGS[kind]] ? 'locked' : 'unlocked'}`) + .join(', ')}` + : 'Updated table locks (no change)', + metadata: { op: 'update_locks', before: previousLocks, after: table.locks }, + ...(request ? { request } : {}), + }) + return { success: true, table } + } catch (error) { + return classifyTableMutation(error, requestId, tableId) + } +} diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 2cf9e357b63..5637daad5b5 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -16,6 +16,7 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, count, eq, inArray, lte, notInArray, type SQL, sql } from 'drizzle-orm' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { assertRowCapacity, getMaxRowsPerTable, @@ -124,13 +125,16 @@ export async function insertRow( // Validate row size const sizeValidation = validateRowSize(data.data) if (!sizeValidation.valid) { - throw new Error(sizeValidation.errors.join(', ')) + throw new OrchestrationError('validation', sizeValidation.errors.join(', ')) } // Validate against schema const schemaValidation = coerceRowToSchema(data.data, table.schema) if (!schemaValidation.valid) { - throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Schema validation failed: ${schemaValidation.errors.join(', ')}` + ) } // Check unique constraints using optimized database query @@ -138,7 +142,7 @@ export async function insertRow( if (uniqueColumns.length > 0) { const uniqueValidation = await checkUniqueConstraintsDb(data.tableId, data.data, table.schema) if (!uniqueValidation.valid) { - throw new Error(uniqueValidation.errors.join(', ')) + throw new OrchestrationError('validation', uniqueValidation.errors.join(', ')) } } @@ -261,12 +265,18 @@ export async function batchInsertRowsWithTx( const sizeValidation = validateRowSize(row) if (!sizeValidation.valid) { - throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${sizeValidation.errors.join(', ')}` + ) } const schemaValidation = coerceRowToSchema(row, table.schema) if (!schemaValidation.valid) { - throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${schemaValidation.errors.join(', ')}` + ) } } @@ -282,7 +292,7 @@ export async function batchInsertRowsWithTx( const errorMessages = uniqueResult.errors .map((e) => `Row ${e.row + 1}: ${e.errors.join(', ')}`) .join('; ') - throw new Error(errorMessages) + throw new OrchestrationError('validation', errorMessages) } } @@ -422,12 +432,18 @@ export async function replaceTableRowsWithTx( const sizeValidation = validateRowSize(row) if (!sizeValidation.valid) { - throw new Error(`Row ${i + 1}: ${sizeValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${sizeValidation.errors.join(', ')}` + ) } const schemaValidation = coerceRowToSchema(row, table.schema) if (!schemaValidation.valid) { - throw new Error(`Row ${i + 1}: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${i + 1}: ${schemaValidation.errors.join(', ')}` + ) } } @@ -449,7 +465,8 @@ export async function replaceTableRowsWithTx( const normalized = typeof value === 'string' ? value : JSON.stringify(value) const map = seen.get(colId)! if (map.has(normalized)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Row ${i + 1}: Column "${col.name}" must be unique. Value "${String(value)}" duplicates row ${map.get(normalized)! + 1} in batch` ) } @@ -538,7 +555,8 @@ export async function upsertRow( const uniqueColumns = getUniqueColumns(schema) if (uniqueColumns.length === 0) { - throw new Error( + throw new OrchestrationError( + 'validation', 'Upsert requires at least one unique column in the schema. Please add a unique constraint to a column or use insert instead.' ) } @@ -552,7 +570,8 @@ export async function upsertRow( (c) => getColumnId(c) === data.conflictTarget || c.name === data.conflictTarget ) if (!col) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column "${data.conflictTarget}" is not a unique column. Available unique columns: ${uniqueColumns.map((c) => c.name).join(', ')}` ) } @@ -560,7 +579,8 @@ export async function upsertRow( } else if (uniqueColumns.length === 1) { targetColumnKey = getColumnId(uniqueColumns[0]) } else { - throw new Error( + throw new OrchestrationError( + 'validation', `Table has multiple unique columns (${uniqueColumns.map((c) => c.name).join(', ')}). Specify a conflict column to indicate which one to match on.` ) } @@ -568,12 +588,15 @@ export async function upsertRow( // Validate row data const sizeValidation = validateRowSize(data.data) if (!sizeValidation.valid) { - throw new Error(sizeValidation.errors.join(', ')) + throw new OrchestrationError('validation', sizeValidation.errors.join(', ')) } const schemaValidation = coerceRowToSchema(data.data, schema) if (!schemaValidation.valid) { - throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Schema validation failed: ${schemaValidation.errors.join(', ')}` + ) } // Read the conflict-target value *after* coercion so `matchFilter` branches on @@ -583,7 +606,10 @@ export async function upsertRow( // Surface the display name, not the internal id — v1 callers pass a name. const targetColumnName = uniqueColumns.find((c) => getColumnId(c) === targetColumnKey)?.name ?? targetColumnKey - throw new Error(`Upsert requires a value for the conflict target column "${targetColumnName}"`) + throw new OrchestrationError( + 'validation', + `Upsert requires a value for the conflict target column "${targetColumnName}"` + ) } // Build the conflict probe through the SAME leaf as the unique-constraint check @@ -636,7 +662,10 @@ export async function upsertRow( trx ) if (!uniqueValidation.valid) { - throw new Error(`Unique constraint violation: ${uniqueValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Unique constraint violation: ${uniqueValidation.errors.join(', ')}` + ) } const now = new Date() @@ -1429,7 +1458,7 @@ export async function updateRow( // Get existing row const existingRow = await getRowById(data.tableId, data.rowId, data.workspaceId) if (!existingRow) { - throw new Error('Row not found') + throw new OrchestrationError('not_found', 'Row not found') } // Merge partial update with existing row data so callers can pass only changed fields @@ -1453,13 +1482,16 @@ export async function updateRow( // Validate size const sizeValidation = validateRowSize(mergedData) if (!sizeValidation.valid) { - throw new Error(sizeValidation.errors.join(', ')) + throw new OrchestrationError('validation', sizeValidation.errors.join(', ')) } // Validate against schema const schemaValidation = coerceRowToSchema(mergedData, table.schema) if (!schemaValidation.valid) { - throw new Error(`Schema validation failed: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Schema validation failed: ${schemaValidation.errors.join(', ')}` + ) } // Check unique constraints using optimized database query @@ -1472,7 +1504,7 @@ export async function updateRow( data.rowId // Exclude current row ) if (!uniqueValidation.valid) { - throw new Error(uniqueValidation.errors.join(', ')) + throw new OrchestrationError('validation', uniqueValidation.errors.join(', ')) } } @@ -1612,7 +1644,7 @@ export async function deleteRow( workspaceId: table.workspaceId, proof, }) - if (!deleted) throw new Error('Row not found') + if (!deleted) throw new OrchestrationError('not_found', 'Row not found') logger.info(`[${requestId}] Deleted row ${rowId} from table ${table.id}`) } @@ -1636,7 +1668,7 @@ export async function updateRowsByFilter( const filterClause = buildFilterClause(data.filter, tableName, table.schema.columns) if (!filterClause) { - throw new Error('Filter is required for bulk update') + throw new OrchestrationError('validation', 'Filter is required for bulk update') } const baseConditions = and( @@ -1678,12 +1710,18 @@ export async function updateRowsByFilter( const sizeValidation = validateRowSize(mergedData) if (!sizeValidation.valid) { - throw new Error(`Row ${row.id}: ${sizeValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${row.id}: ${sizeValidation.errors.join(', ')}` + ) } const schemaValidation = coerceRowToSchema(mergedData, table.schema) if (!schemaValidation.valid) { - throw new Error(`Row ${row.id}: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${row.id}: ${schemaValidation.errors.join(', ')}` + ) } } @@ -1691,7 +1729,8 @@ export async function updateRowsByFilter( const uniqueColumnsInUpdate = uniqueColumns.filter((col) => col.name in data.data) if (uniqueColumnsInUpdate.length > 0) { if (matchingRows.length > 1) { - throw new Error( + throw new OrchestrationError( + 'validation', `Cannot set unique column values when updating multiple rows. ` + `Columns with unique constraint: ${uniqueColumnsInUpdate.map((c) => c.name).join(', ')}. ` + `Updating ${matchingRows.length} rows with the same value would violate uniqueness.` @@ -1708,7 +1747,10 @@ export async function updateRowsByFilter( row.id ) if (!uniqueValidation.valid) { - throw new Error(`Unique constraint violation: ${uniqueValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Unique constraint violation: ${uniqueValidation.errors.join(', ')}` + ) } } @@ -1826,7 +1868,7 @@ export async function batchUpdateRows( const missing = rowIds.filter((id) => !existingMap.has(id)) if (missing.length > 0) { - throw new Error(`Rows not found: ${missing.join(', ')}`) + throw new OrchestrationError('validation', `Rows not found: ${missing.join(', ')}`) } const mergedUpdates: Array<{ @@ -1856,12 +1898,18 @@ export async function batchUpdateRows( const sizeValidation = validateRowSize(merged) if (!sizeValidation.valid) { - throw new Error(`Row ${update.rowId}: ${sizeValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${update.rowId}: ${sizeValidation.errors.join(', ')}` + ) } const schemaValidation = coerceRowToSchema(merged, table.schema) if (!schemaValidation.valid) { - throw new Error(`Row ${update.rowId}: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${update.rowId}: ${schemaValidation.errors.join(', ')}` + ) } mergedUpdates.push({ @@ -1884,7 +1932,10 @@ export async function batchUpdateRows( rowId ) if (!uniqueValidation.valid) { - throw new Error(`Row ${rowId}: ${uniqueValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Row ${rowId}: ${uniqueValidation.errors.join(', ')}` + ) } } } @@ -2006,7 +2057,7 @@ export async function deleteRowsByFilter( // Build filter clause const filterClause = buildFilterClause(data.filter, tableName, table.schema.columns) if (!filterClause) { - throw new Error('Filter is required for bulk delete') + throw new OrchestrationError('validation', 'Filter is required for bulk delete') } // Find matching rows diff --git a/apps/sim/lib/table/select-options.test.ts b/apps/sim/lib/table/select-options.test.ts new file mode 100644 index 00000000000..36841dc9926 --- /dev/null +++ b/apps/sim/lib/table/select-options.test.ts @@ -0,0 +1,59 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { normalizeSelectOptionsInput } from '@/lib/table/select-options' + +describe('normalizeSelectOptionsInput', () => { + it('generates a stable id for a bare-name string option', () => { + const [opt] = normalizeSelectOptionsInput(['Open']) ?? [] + expect(opt.name).toBe('Open') + expect(typeof opt.id).toBe('string') + expect(opt.id.length).toBeGreaterThan(0) + }) + + it('generates an id for an object option without one', () => { + const [opt] = normalizeSelectOptionsInput([{ name: 'Closed' }]) ?? [] + expect(opt.name).toBe('Closed') + expect(opt.id.length).toBeGreaterThan(0) + }) + + it('preserves an explicitly supplied id', () => { + const result = normalizeSelectOptionsInput([{ id: 'opt_keep', name: 'Open' }]) + expect(result).toEqual([{ id: 'opt_keep', name: 'Open' }]) + }) + + it('reuses the id of an existing option with the same name', () => { + // The agent re-sends options as bare names on every edit. Minting fresh ids + // would orphan every cell holding them — silently clearing the column. + const existing = [ + { id: 'opt_low', name: 'Low' }, + { id: 'opt_high', name: 'High' }, + ] + const result = normalizeSelectOptionsInput(['Low', 'Medium', 'High'], existing) ?? [] + + expect(result[0]).toEqual({ id: 'opt_low', name: 'Low' }) + expect(result[2]).toEqual({ id: 'opt_high', name: 'High' }) + // Only the genuinely new option gets a fresh id. + expect(result[1].name).toBe('Medium') + expect(result[1].id).not.toBe('opt_low') + expect(result[1].id).not.toBe('opt_high') + }) + + it('matches an existing option name case-insensitively', () => { + const result = normalizeSelectOptionsInput(['open'], [{ id: 'opt_open', name: 'Open' }]) ?? [] + expect(result[0].id).toBe('opt_open') + expect(result[0].name).toBe('open') + }) + + it('mints a fresh id when there is no existing column to match against', () => { + const result = normalizeSelectOptionsInput(['Open']) ?? [] + expect(result[0].id.length).toBeGreaterThan(0) + expect(result[0].name).toBe('Open') + }) + + it('returns undefined for a non-array (validation rejects it downstream)', () => { + expect(normalizeSelectOptionsInput(undefined)).toBeUndefined() + expect(normalizeSelectOptionsInput('Open')).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/table/select-options.ts b/apps/sim/lib/table/select-options.ts index aa62cfa0bfd..332b465cb7d 100644 --- a/apps/sim/lib/table/select-options.ts +++ b/apps/sim/lib/table/select-options.ts @@ -12,6 +12,7 @@ * Keeping the option primitives here lets both sides share one implementation. */ +import { generateShortId } from '@sim/utils/id' import type { ColumnDefinition, JsonValue, SelectOption } from '@/lib/table/types' /** Set of valid option ids for a `select`/`multiselect` column. */ @@ -85,3 +86,34 @@ export function resolveSelectCellValue(value: JsonValue, column: ColumnDefinitio const single = Array.isArray(value) ? value[0] : value return single === undefined ? null : resolveSelectOptionId(single, options) } + +/** + * Normalizes caller-supplied select options to `{ id, name }` pairs. + * + * Cells reference the option id, so an edit that re-sends an option by name + * must reuse the id it already has — minting a fresh one would orphan every + * cell holding it, silently clearing the column. A caller that already supplies + * an id keeps it, which makes this a no-op for the fully-formed options the + * HTTP contracts accept and a repair for the name-only options agents author. + */ +export function normalizeSelectOptionsInput( + raw: unknown, + existing: SelectOption[] = [] +): SelectOption[] | undefined { + if (!Array.isArray(raw)) return undefined + + const idByName = new Map() + for (const option of existing) { + const key = option.name.toLowerCase() + if (!idByName.has(key)) idByName.set(key, option.id) + } + const resolveId = (name: string): string => idByName.get(name.toLowerCase()) ?? generateShortId() + + return raw.map((entry) => { + if (typeof entry === 'string') return { id: resolveId(entry), name: entry } + const e = (entry ?? {}) as { id?: unknown; name?: unknown } + const name = typeof e.name === 'string' ? e.name : String(e.name ?? '') + const id = typeof e.id === 'string' && e.id.length > 0 ? e.id : resolveId(name) + return { id, name } + }) +} diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 5d23ec56931..5a6f78dd84a 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -14,6 +14,7 @@ import { createLogger } from '@sim/logger' import { getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, count, eq, isNull, sql } from 'drizzle-orm' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { generateRestoreName } from '@/lib/core/utils/restore-name' import type { DbOrTx } from '@/lib/db/types' import { resolveRestoredFolderId } from '@/lib/folders/queries' @@ -28,8 +29,6 @@ import type { DbTransaction } from '@/lib/table/planner' import { setTableTxTimeouts } from '@/lib/table/tx' import { type CreateTableData, - TABLE_LOCK_FLAGS, - TABLE_LOCK_KINDS, type TableDefinition, type TableLocks, type TableMetadata, @@ -41,10 +40,15 @@ import { stripGroupDeps } from '@/lib/table/workflow-columns' const logger = createLogger('TableService') -export class TableConflictError extends Error { - readonly code = 'TABLE_EXISTS' as const +/** + * A table name already taken in the workspace. Kept as its own class because + * several routes branch on it specifically, and typed as a `conflict` so the + * generic classifiers reach the same 409 without reading the message. + */ +export class TableConflictError extends OrchestrationError { constructor(name: string) { - super(`A table named "${name}" already exists in this workspace`) + super('conflict', `A table named "${name}" already exists in this workspace`) + this.name = 'TableConflictError' } } @@ -103,7 +107,7 @@ export async function withLockedTable( ) const table = await getTableById(tableId, { tx: trx, includeArchived: opts?.includeArchived }) if (!table) { - throw new Error('Table not found') + throw new OrchestrationError('not_found', 'Table not found') } return mutate(table, trx) }) @@ -285,13 +289,19 @@ export async function createTable( // Validate table name const nameValidation = validateTableName(data.name) if (!nameValidation.valid) { - throw new Error(`Invalid table name: ${nameValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Invalid table name: ${nameValidation.errors.join(', ')}` + ) } // Validate schema const schemaValidation = validateTableSchema(data.schema) if (!schemaValidation.valid) { - throw new Error(`Invalid schema: ${schemaValidation.errors.join(', ')}`) + throw new OrchestrationError( + 'validation', + `Invalid schema: ${schemaValidation.errors.join(', ')}` + ) } const tableId = `tbl_${generateId().replace(/-/g, '')}` @@ -355,7 +365,12 @@ export async function createTable( ) if (Number(existingCount) >= maxTables) { - throw new Error(`Workspace has reached maximum table limit (${maxTables})`) + // A quota ceiling, not bad input — both create routes have always + // answered 403 for it. + throw new OrchestrationError( + 'forbidden', + `Workspace has reached maximum table limit (${maxTables})` + ) } const duplicateName = await trx @@ -474,23 +489,26 @@ export async function addTableColumnsWithTx( for (const column of columns) { if (!NAME_PATTERN.test(column.name)) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column name "${column.name}". Must start with a letter or underscore and contain only alphanumeric characters and underscores.` ) } if (column.name.length > TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH) { - throw new Error( + throw new OrchestrationError( + 'validation', `Column name exceeds maximum length (${TABLE_LIMITS.MAX_COLUMN_NAME_LENGTH} characters)` ) } if (!COLUMN_TYPES.includes(column.type as (typeof COLUMN_TYPES)[number])) { - throw new Error( + throw new OrchestrationError( + 'validation', `Invalid column type "${column.type}". Must be one of: ${COLUMN_TYPES.join(', ')}` ) } const lower = column.name.toLowerCase() if (usedNames.has(lower)) { - throw new Error(`Column "${column.name}" already exists`) + throw new OrchestrationError('validation', `Column "${column.name}" already exists`) } usedNames.add(lower) // Honor a caller-assigned id (the CSV append path pre-assigns so coercion @@ -506,7 +524,8 @@ export async function addTableColumnsWithTx( } if (table.schema.columns.length + additions.length > TABLE_LIMITS.MAX_COLUMNS_PER_TABLE) { - throw new Error( + throw new OrchestrationError( + 'validation', `Adding ${additions.length} column(s) would exceed maximum column limit (${TABLE_LIMITS.MAX_COLUMNS_PER_TABLE})` ) } @@ -572,12 +591,11 @@ export function auditTableColumnsAdded( export async function renameTable( tableId: string, newName: string, - requestId: string, - actingUserId?: string + requestId: string ): Promise<{ id: string; name: string }> { const nameValidation = validateTableName(newName) if (!nameValidation.valid) { - throw new Error(nameValidation.errors.join(', ')) + throw new OrchestrationError('validation', nameValidation.errors.join(', ')) } const now = new Date() @@ -586,29 +604,10 @@ export async function renameTable( .update(userTableDefinitions) .set({ name: newName, updatedAt: now }) .where(eq(userTableDefinitions.id, tableId)) - .returning({ - id: userTableDefinitions.id, - createdBy: userTableDefinitions.createdBy, - workspaceId: userTableDefinitions.workspaceId, - }) + .returning({ id: userTableDefinitions.id }) if (result.length === 0) { - throw new Error(`Table ${tableId} not found`) - } - - const { createdBy, workspaceId } = result[0] - const renameActorId = actingUserId ?? createdBy - if (renameActorId) { - recordAudit({ - workspaceId: workspaceId ?? null, - actorId: renameActorId, - action: AuditAction.TABLE_UPDATED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: newName, - description: `Renamed table to "${newName}"`, - metadata: { op: 'rename' }, - }) + throw new OrchestrationError('not_found', `Table ${tableId} not found`) } logger.info(`[${requestId}] Renamed table ${tableId} to "${newName}"`) @@ -636,9 +635,8 @@ export async function moveTableToFolder( tableId: string, workspaceId: string, folderId: string | null, - requestId: string, - actingUserId?: string -): Promise { + requestId: string +): Promise<{ name: string }> { const updates: Partial = { folderId, updatedAt: new Date(), @@ -666,27 +664,13 @@ export async function moveTableToFolder( }) if (result.length === 0) { - throw new Error(`Table ${tableId} not found`) + throw new OrchestrationError('not_found', `Table ${tableId} not found`) } - const { name, createdBy } = result[0] - const actorId = actingUserId ?? createdBy - if (actorId) { - recordAudit({ - workspaceId, - actorId, - action: AuditAction.TABLE_UPDATED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: name, - description: folderId - ? `Moved table "${name}" into a folder` - : `Moved table "${name}" to the workspace root`, - metadata: { op: 'move', folderId }, - }) - } + const { name } = result[0] logger.info(`[${requestId}] Moved table ${tableId} to folder ${folderId ?? 'root'}`) + return { name } } /** @@ -703,11 +687,8 @@ export async function moveTableToFolder( export async function updateTableLocks( tableId: string, partial: Partial, - actingUserId: string, - requestId: string, - /** Forwarded to the audit record for IP / user-agent capture. */ - request?: { headers: { get(name: string): string | null } } -): Promise { + requestId: string +): Promise<{ table: TableDefinition; previousLocks: TableLocks }> { let previousLocks: TableLocks = UNLOCKED_TABLE_LOCKS const updated = await withLockedTable(tableId, async (table, trx) => { previousLocks = table.locks @@ -720,36 +701,12 @@ export async function updateTableLocks( return { ...table, locks: nextLocks, updatedAt: now } }) - // Name the transitions in the description so the audit list is readable - // without expanding metadata — "who locked my production table" is the - // question this feature exists to answer. - const flipped = TABLE_LOCK_KINDS.filter( - (kind) => previousLocks[TABLE_LOCK_FLAGS[kind]] !== updated.locks[TABLE_LOCK_FLAGS[kind]] - ) - const description = flipped.length - ? `Table locks changed: ${flipped - .map((kind) => `${kind} ${updated.locks[TABLE_LOCK_FLAGS[kind]] ? 'locked' : 'unlocked'}`) - .join(', ')}` - : 'Updated table locks (no change)' - - recordAudit({ - workspaceId: updated.workspaceId, - actorId: actingUserId, - action: AuditAction.TABLE_UPDATED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: updated.name, - description, - metadata: { op: 'update_locks', before: previousLocks, after: updated.locks }, - ...(request ? { request } : {}), - }) - await appendTableEvent({ kind: 'definition', tableId, reason: 'locks' }).catch((error) => { logger.warn(`[${requestId}] Failed to emit lock-change event for table ${tableId}`, { error }) }) logger.info(`[${requestId}] Updated locks for table ${tableId}`) - return updated + return { table: updated, previousLocks } } /** @@ -830,9 +787,8 @@ export async function updateTableMetadata( export async function deleteTable( tableId: string, requestId: string, - actingUserId?: string, options?: { archivedAt?: Date } -): Promise { +): Promise<{ archived: { name: string; workspaceId: string | null } | null }> { const now = options?.archivedAt ?? new Date() // Archiving destroys access to every row, so it is gated on the delete lock. // The guard is inline in the WHERE (atomic — no separate read, no TOCTOU); @@ -874,21 +830,10 @@ export async function deleteTable( } // Otherwise the table is missing or already archived — a silent no-op, as before. } - // Audit only genuine user deletes — rollback callers omit `actingUserId`. The - // caller emits the `table_deleted` PostHog event, so it is not duplicated here. - if (deleted && actingUserId) { - recordAudit({ - workspaceId: deleted.workspaceId ?? null, - actorId: actingUserId, - action: AuditAction.TABLE_DELETED, - resourceType: AuditResourceType.TABLE, - resourceId: tableId, - resourceName: deleted.name, - description: `Archived table "${deleted.name}"`, - }) - } - logger.info(`[${requestId}] Archived table ${tableId}`) + // Null when the table was missing or already archived — a silent no-op. The + // caller audits only a genuine archive, so a repeat delete logs nothing. + return { archived: deleted ? { name: deleted.name, workspaceId: deleted.workspaceId } : null } } /** @@ -907,18 +852,18 @@ export async function restoreTable( ): Promise { const table = await getTableById(tableId, { includeArchived: true }) if (!table) { - throw new Error('Table not found') + throw new OrchestrationError('not_found', 'Table not found') } if (!table.archivedAt) { - throw new Error('Table is not archived') + throw new OrchestrationError('validation', 'Table is not archived') } if (table.workspaceId) { const { getWorkspaceWithOwner } = await import('@/lib/workspaces/permissions/utils') const ws = await getWorkspaceWithOwner(table.workspaceId) if (!ws || ws.archivedAt) { - throw new Error('Cannot restore table into an archived workspace') + throw new OrchestrationError('validation', 'Cannot restore table into an archived workspace') } } diff --git a/apps/sim/lib/table/workflow-columns.ts b/apps/sim/lib/table/workflow-columns.ts index 4054113d4e2..a2ccc24d4d6 100644 --- a/apps/sim/lib/table/workflow-columns.ts +++ b/apps/sim/lib/table/workflow-columns.ts @@ -18,6 +18,7 @@ import { and, eq, inArray, notInArray, sql } from 'drizzle-orm' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' import type { EnqueueOptions } from '@/lib/core/async-jobs/types' import { isTriggerDevEnabled } from '@/lib/core/config/env-flags' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { buildCancelledExecution } from '@/lib/table/cell-write' import type { Filter, @@ -731,8 +732,9 @@ export async function runWorkflowColumn(opts: { // this module; `@trigger.dev/sdk` is heavy and only needed on this op. const { getTableById } = await import('@/lib/table/service') const table = await getTableById(tableId) - if (!table) throw new Error('Table not found') - if (table.workspaceId !== workspaceId) throw new Error('Invalid workspace ID') + if (!table) throw new OrchestrationError('not_found', 'Table not found') + if (table.workspaceId !== workspaceId) + throw new OrchestrationError('validation', 'Invalid workspace ID') const allGroups = table.schema.workflowGroups ?? [] const targetGroups = groupIds ? allGroups.filter((g) => groupIds.includes(g.id)) : allGroups diff --git a/apps/sim/lib/workflows/orchestration/deploy.ts b/apps/sim/lib/workflows/orchestration/deploy.ts index 8a9f49f4a14..69ce7adac44 100644 --- a/apps/sim/lib/workflows/orchestration/deploy.ts +++ b/apps/sim/lib/workflows/orchestration/deploy.ts @@ -7,6 +7,7 @@ import { getErrorMessage } from '@sim/utils/errors' import { and, eq } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { env } from '@/lib/core/config/env' +import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { getSocketServerUrl } from '@/lib/core/utils/urls' import { captureServerEvent } from '@/lib/posthog/server' @@ -26,7 +27,6 @@ import { notifySocketDeploymentChanged, processWorkflowDeploymentOutboxEvent, } from '@/lib/workflows/deployment-outbox' -import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' import { getWorkflowDeploymentStatus, prepareWorkflowDeployment, diff --git a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts index e4903974aa4..bc1c12e8c73 100644 --- a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts @@ -1,8 +1,8 @@ import type { folder as folderTable } from '@sim/db/schema' import type { FolderResourceType } from '@/lib/api/contracts/folders' +import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { createFolder, deleteFolder, restoreFolder, updateFolder } from '@/lib/folders/lifecycle' import type { FolderMutationErrorCode } from '@/lib/folders/status' -import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' /** * Workflow-bound entry points into the generic folder engine in `lib/folders/lifecycle.ts`. diff --git a/apps/sim/lib/workflows/orchestration/types.ts b/apps/sim/lib/workflows/orchestration/types.ts deleted file mode 100644 index 70c715ffb2c..00000000000 --- a/apps/sim/lib/workflows/orchestration/types.ts +++ /dev/null @@ -1,12 +0,0 @@ -export type OrchestrationErrorCode = 'validation' | 'not_found' | 'conflict' | 'internal' - -/** - * Maps an orchestration error code to its HTTP status. Shared by every route - * surface (UI, v1, tool routes) so deployment errors map identically. - */ -export function statusForOrchestrationError(code: OrchestrationErrorCode | undefined): number { - if (code === 'validation') return 400 - if (code === 'not_found') return 404 - if (code === 'conflict') return 409 - return 500 -} diff --git a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts index 9ccee7d5171..07588af4460 100644 --- a/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts @@ -6,11 +6,11 @@ import { isFolderInWorkspace } from '@sim/platform-authz/workflow' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, eq, isNull, min, ne } from 'drizzle-orm' +import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { captureServerEvent } from '@/lib/posthog/server' import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults' import { archiveWorkflow, restoreWorkflow } from '@/lib/workflows/lifecycle' -import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' import { deduplicateWorkflowName } from '@/lib/workflows/utils'