feat(api): make v2 table PATCH state which operations landed on failure

Greptile held the PR at 4/5 on the residual non-atomicity and named two
acceptable resolutions: make PATCH atomic, or have the contract adopt and
expose partial-success explicitly. Atomicity would mean threading one
transaction through renameTable, moveTableToFolder and updateTableLocks —
three shared service functions with four non-test callers including the
first-party route and two copilot tools — and deferring their per-operation
audits to commit time. That is a refactor of shared write paths well
outside this PR.

So the contract states it instead. Every rejectable condition is already
pre-validated, so a failure here is a genuine fault; when one follows a
successful operation the error now carries `details.applied` listing what
is live. Absent when nothing applied, so its presence always means "these
changes took effect despite the error". Documented on the operation.

`v2ErrorForOrchestration` gained the optional `details` this needs.

Author: TheodoreSpeaks

apps/docs/openapi-v2-tables.json

@@ -373,7 +373,7 @@
       "patch": {
         "operationId": "updateTable",
         "summary": "Update Table",
-        "description": "Rename a table, move it between folders, and/or change its lock flags. Provide at least one of `name`, `folderId`, or `locks`. Each field is applied independently, so one request can rename and move at once, and the response reflects every applied change.\n\n`name` and `folderId` need workspace write. `locks` additionally needs workspace **admin** \u2014 a write-level caller gets 403. Clearing a lock always works; enabling one requires the table-locks feature to be on for the workspace, so an already-locked table can never be stranded.",
+        "description": "Rename a table, move it between folders, and/or change its lock flags. Provide at least one of `name`, `folderId`, or `locks`. Each field is applied independently, so one request can rename and move at once, and the response reflects every applied change.\n\n`name` and `folderId` need workspace write. `locks` additionally needs workspace **admin** \u2014 a write-level caller gets 403. Clearing a lock always works; enabling one requires the table-locks feature to be on for the workspace, so an already-locked table can never be stranded.\n\n**Partial-success semantics.** The three operations commit independently, so this endpoint is not atomic. Everything that can be *rejected* \u2014 the lock feature gate, the admin check, folder existence \u2014 is validated before the first write, so a rejected request changes nothing. If a genuine fault (a lost race, the table archived mid-request, a database error) fails a later operation after an earlier one has committed, the response is an error whose `error.details.applied` lists the operations that are nevertheless live (`\"locks\"`, `\"name\"`, `\"folderId\"`). The field is absent when nothing was applied, so its presence always means \"these changes took effect despite the error\" \u2014 re-read the table to confirm before retrying.",
         "tags": ["Tables"],
         "x-codeSamples": [
           {

apps/sim/app/api/v2/lib/response.ts

@@ -175,10 +175,14 @@ const V2_CODE_BY_ORCHESTRATION_ERROR: Record<OrchestrationErrorCode, V2ErrorCode
  */
 export function v2ErrorForOrchestration(
   code: OrchestrationErrorCode | undefined,
-  message: string
+  message: string,
+  /** Structured context for the failure — e.g. which lock rejected a write. */
+  details?: unknown
 ): NextResponse {
   const v2Code = code ? V2_CODE_BY_ORCHESTRATION_ERROR[code] : 'INTERNAL_ERROR'
-  return v2Error(v2Code, v2Code === 'INTERNAL_ERROR' ? 'Internal server error' : message)
+  return v2Error(v2Code, v2Code === 'INTERNAL_ERROR' ? 'Internal server error' : message, {
+    ...(details !== undefined ? { details } : {}),
+  })
 }
 
 /**

apps/sim/app/api/v2/tables/[tableId]/route.test.ts

@@ -282,6 +282,38 @@ describe('PATCH /api/v2/tables/[tableId]', () => {
     expect(mockPerformRenameTable).not.toHaveBeenCalled()
   })
 
+  it('reports which operations landed when a later one fails', async () => {
+    // The three writes commit independently, so rather than pretending
+    // atomicity the error states what is already live — a caller can reconcile
+    // instead of re-reading and diffing.
+    mockPerformRenameTable.mockResolvedValue({ success: true })
+    mockPerformMoveTableToFolder.mockResolvedValue({
+      success: false,
+      errorCode: 'not_found',
+      error: 'gone',
+    })
+
+    const res = await callPatch({ workspaceId: 'ws-1', name: 'Renamed', folderId: 'folder-1' })
+
+    expect(res.status).toBe(404)
+    expect((await res.json()).error.details).toEqual({ applied: ['name'] })
+  })
+
+  it('omits the applied list when the very first operation fails', async () => {
+    // `details.applied` present must always mean "these changes are live".
+    mockPerformRenameTable.mockResolvedValue({
+      success: false,
+      errorCode: 'conflict',
+      error: 'taken',
+    })
+
+    const res = await callPatch({ workspaceId: 'ws-1', name: 'Renamed', folderId: 'folder-1' })
+
+    expect(res.status).toBe(409)
+    expect((await res.json()).error.details).toBeUndefined()
+    expect(mockPerformMoveTableToFolder).not.toHaveBeenCalled()
+  })
+
   it('still signals collaborators when a later operation fails after an earlier one landed', async () => {
     // A mid-write fault can't be rolled back across three transactions, so the
     // clients must at least be told to refetch what did apply.

apps/sim/app/api/v2/tables/[tableId]/route.ts

@@ -1,6 +1,6 @@
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
-import type { NextRequest, NextResponse } from 'next/server'
+import type { NextRequest } from 'next/server'
 import {
   v2DeleteTableContract,
   v2GetTableContract,
@@ -32,6 +32,7 @@ import {
   v2ValidationError,
   v2WorkspaceAccessError,
 } from '@/app/api/v2/lib/response'
+import type { OrchestrationOutcome } from '@/app/api/v2/tables/utils'
 import {
   toApiTable,
   v2TableAccessError,
@@ -171,11 +172,16 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
     }
 
     // ── Apply ──
-    // `applied` tracks whether anything reached the database, so a failure
-    // partway through still signals open clients. Skipping the signal there
-    // would leave every viewer rendering state that has already changed.
-    let applied = false
-    let failure: NextResponse | null = null
+    // Every deterministic rejection is already behind us, so a failure here is
+    // a genuine fault (lost race, archived mid-request, database error) rather
+    // than a bad request. The three operations commit independently — a single
+    // transaction would have to span three shared service functions that also
+    // back the first-party route and two copilot tools, and would break their
+    // per-operation audits — so instead of pretending atomicity the response
+    // states exactly which operations landed. A caller that gets an error can
+    // then reconcile rather than having to re-read and diff.
+    const applied: ('locks' | 'name' | 'folderId')[] = []
+    let failure: { outcome: OrchestrationOutcome; fallback: string } | null = null
 
     if (validated.locks !== undefined) {
       const outcome = await performUpdateTableLocks({
@@ -185,11 +191,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
         requestId,
         request,
       })
-      if (outcome.success) {
-        applied = true
-      } else {
-        failure = v2TableOrchestrationError(outcome, 'Failed to update table locks')
-      }
+      if (outcome.success) applied.push('locks')
+      else failure = { outcome, fallback: 'Failed to update table locks' }
     }
 
     if (!failure && validated.name !== undefined) {
@@ -200,11 +203,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
         requestId,
         request,
       })
-      if (outcome.success) {
-        applied = true
-      } else {
-        failure = v2TableOrchestrationError(outcome, 'Failed to rename table')
-      }
+      if (outcome.success) applied.push('name')
+      else failure = { outcome, fallback: 'Failed to rename table' }
     }
 
     if (!failure && validated.folderId !== undefined) {
@@ -216,20 +216,29 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Tabl
         request,
       })
       if (outcome.success) {
-        applied = true
+        applied.push('folderId')
       } else {
         // The move re-asserts workspace and active state, so a miss means the
         // table was archived between `checkAccess` and the write.
-        failure = v2TableOrchestrationError(
-          outcome.errorCode === 'not_found' ? { ...outcome, error: 'Table not found' } : outcome,
-          'Failed to move table'
-        )
+        failure = {
+          outcome:
+            outcome.errorCode === 'not_found' ? { ...outcome, error: 'Table not found' } : outcome,
+          fallback: 'Failed to move table',
+        }
       }
     }
 
     // Live-collab: tell open viewers the definition changed so they refetch.
-    if (applied) signalTableSchemaChanged(tableId)
-    if (failure) return failure
+    if (applied.length > 0) signalTableSchemaChanged(tableId)
+    if (failure) {
+      return v2TableOrchestrationError(
+        failure.outcome,
+        failure.fallback,
+        // Omitted when nothing landed, so `details.applied` present always
+        // means "these changes are live despite the error".
+        applied.length > 0 ? { applied } : undefined
+      )
+    }
 
     // Re-read so the response reflects every applied change at once.
     const updated = await getTableById(tableId)

apps/sim/app/api/v2/tables/utils.ts

@@ -191,6 +191,13 @@ export function v2TableLockError(error: unknown): NextResponse | null {
   return null
 }
 
+/** The failure half of any `lib/table/orchestration` result. */
+export interface OrchestrationOutcome {
+  errorCode?: OrchestrationErrorCode
+  error?: string
+  lock?: TableLockKind
+}
+
 /**
  * Renders a `lib/table/orchestration` failure in the v2 envelope, naming the
  * lock when one caused it.
@@ -203,17 +210,22 @@ export function v2TableLockError(error: unknown): NextResponse | null {
  * orchestration result goes through this instead.
  */
 export function v2TableOrchestrationError(
-  outcome: { errorCode?: OrchestrationErrorCode; error?: string; lock?: TableLockKind },
-  fallback: string
+  outcome: OrchestrationOutcome,
+  fallback: string,
+  /** Merged into `details` — e.g. which operations of a composite write landed. */
+  extraDetails?: Record<string, unknown>
 ): NextResponse {
-  if (outcome.errorCode === 'locked') {
-    return v2Error('LOCKED', outcome.error ?? fallback, {
-      // Omitted rather than sent as null when the kind is unknown — a caller
-      // branching on `details.lock` should see absence, not a phantom value.
-      ...(outcome.lock ? { details: { lock: outcome.lock } } : {}),
-    })
+  // `lock` is omitted rather than sent as null when the kind is unknown — a
+  // caller branching on `details.lock` should see absence, not a phantom value.
+  const details = {
+    ...(outcome.errorCode === 'locked' && outcome.lock ? { lock: outcome.lock } : {}),
+    ...extraDetails,
   }
-  return v2ErrorForOrchestration(outcome.errorCode, outcome.error ?? fallback)
+  return v2ErrorForOrchestration(
+    outcome.errorCode,
+    outcome.error ?? fallback,
+    Object.keys(details).length > 0 ? details : undefined
+  )
 }
 
 /**