refactor(tables): move the audit log out of the table service

`lib/table/service.ts` wrote its own audit rows, so whether an operation was
audited depended on which function a caller reached for rather than on a user
having performed it. That is what let v1 and v2 audit a no-op delete, and what
made `deleteTable`'s optional `actingUserId` double as an audit opt-out flag.

Worse, most sites fell back to `actingUserId ?? createdBy`, so an unattributed
call was logged against the table's *creator*. The copilot `mv` path passed no
actor at all: renaming someone else's table recorded them as the renamer.

Audit now lives in the orchestration functions — performDeleteTable,
performRenameTable, performMoveTableToFolder, performUpdateTableLocks — and
the services just write. Internal callers (folder cascade, import rollback)
keep calling the service and are silent by construction rather than by
remembering to omit an argument.

Two services now return what the audit needs: `deleteTable` reports whether it
actually archived a row, so a repeat delete logs nothing; `updateTableLocks`
returns the before/after locks, since only the locked write can observe the
transition its description names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: TheodoreSpeaks

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)
     )
   })
 

apps/sim/app/api/table/[tableId]/route.ts

@@ -9,16 +9,14 @@ 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 {
-  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 } from '@/lib/table/orchestration'
+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,34 @@ 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,
+        })
+        if (!renameOutcome.success) {
+          return NextResponse.json(
+            { error: renameOutcome.error ?? 'Failed to rename table' },
+            { status: statusForOrchestrationError(renameOutcome.errorCode) }
+          )
+        }
       }
 
       if (validated.folderId !== undefined) {
@@ -196,21 +217,21 @@ 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,
+        })
+        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
         }
       }
 

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 }
 }

apps/sim/lib/copilot/tools/server/table/user-table.ts

@@ -40,7 +40,11 @@ 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 { performUpdateTableColumn } from '@/lib/table/orchestration'
+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'
@@ -59,7 +63,7 @@ import {
 } 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,
@@ -490,7 +494,14 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
 
             const requestId = generateId().slice(0, 8)
             assertNotAborted()
-            await deleteTable(tableId, requestId, context.userId)
+            const deleteOutcome = await performDeleteTable({
+              table,
+              userId: context.userId,
+              requestId,
+            })
+            if (!deleteOutcome.success) {
+              return { success: false, message: deleteOutcome.error ?? 'Failed to delete table' }
+            }
             captureServerEvent(
               context.userId,
               'table_deleted',
@@ -1703,12 +1714,20 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
 
           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 } },
           }
         }
 

apps/sim/lib/folders/config.ts

@@ -304,7 +304,7 @@ async function archiveTableChildren(context: CascadeChildrenContext): Promise<nu
   const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'active')
 
   for (const id of ids) {
-    await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, undefined, {
+    await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, {
       archivedAt: context.timestamp,
     })
   }

apps/sim/lib/table/orchestration/index.ts

@@ -1,3 +1,9 @@
 export { performUpdateTableColumn } from './columns'
 export { performRestoreTable } from './restore'
-export { performDeleteTable, performDeleteTableRow } from './tables'
+export {
+  performDeleteTable,
+  performDeleteTableRow,
+  performMoveTableToFolder,
+  performRenameTable,
+  performUpdateTableLocks,
+} from './tables'

apps/sim/lib/table/orchestration/tables.test.ts

@@ -4,13 +4,27 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { TableDefinition } from '@/lib/table/types'
 
-const { mockDeleteTable, mockDeleteRow, mockCaptureServerEvent } = vi.hoisted(() => ({
-  mockDeleteTable: vi.fn(),
-  mockDeleteRow: vi.fn(),
-  mockCaptureServerEvent: vi.fn(),
+const { mockDeleteTable, mockDeleteRow, mockCaptureServerEvent, mockRecordAudit } = vi.hoisted(
+  () => ({
+    mockDeleteTable: vi.fn(),
+    mockDeleteRow: 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 }))
+vi.mock('@/lib/table/service', () => ({
+  deleteTable: mockDeleteTable,
+  moveTableToFolder: vi.fn(),
+  renameTable: vi.fn(),
+  updateTableLocks: vi.fn(),
+}))
 vi.mock('@/lib/table/rows/service', () => ({ deleteRow: mockDeleteRow }))
 vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mockCaptureServerEvent }))
 
@@ -22,16 +36,18 @@ const TABLE = { id: 'table-1', name: 'Tasks', workspaceId: 'ws-1' } as unknown a
 describe('performDeleteTable', () => {
   beforeEach(() => vi.clearAllMocks())
 
-  it('hands the actor to the service so it owns the audit', async () => {
-    // deleteTable audits only when a row was actually archived AND an actor is
-    // given. Callers that omitted the actor and audited themselves emitted
-    // TABLE_DELETED for a no-op delete of an already-archived table.
-    mockDeleteTable.mockResolvedValue(undefined)
+  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)
-    expect(mockDeleteTable).toHaveBeenCalledWith('table-1', 'req-1', 'user-1')
+    // 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',
@@ -40,6 +56,15 @@ describe('performDeleteTable', () => {
     )
   })
 
+  it('does not audit 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()
+  })
+
   it('classifies a delete lock as locked and emits no telemetry', async () => {
     mockDeleteTable.mockRejectedValue(new TableLockedError('delete'))