fix(tables): validate a retype's unique against the values it writes

Validated the last partial-update seam with a focused investigation rather
than assuming. The answer was split.

`required` is already safe: `updateColumnType` runs the same `countEmptyCells`
against the constraint the request is about to set, which is why that check
exists.

`unique` was not, and the reachable case commits the unrecoverable half. A
text column holding "5" and "5.0", PATCHed with {type: number, unique: true}:
the conversion succeeds and coerces both to 5, then the separate constraint
write finds duplicates and 400s — with the column already numeric and "5.0"
irreversibly rewritten. A pre-scan of the raw text finds nothing; the
conversion is what manufactures the duplicate. The retype now carries `unique`
and checks it after the write-back, against the values it just wrote.

Constraint changes on a workflow-output column were the same shape — rejected
by the constraint write, after a type change had committed. Now rejected in the
route's pre-flight block, before any write.

The duplicate scan is extracted and shared between both paths for the same
reason `countEmptyCells` is: two copies of one rule is the drift that produced
the original required-check bug.

Deliberately NOT merging `updateColumnType` and `updateColumnConstraints`. They
assert different lock levels (destructive vs schema-only) and only the retype
needs the full row scan, so merging would either force a constraints-only
toggle to materialize every row or reintroduce the branching it was meant to
remove. With both reachable failures pre-validated, what remains at the seam is
concurrent races no in-process check can close.

Author: waleedlatif1

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

@@ -170,6 +170,50 @@ describe('PATCH /api/table/[tableId]/columns — pre-flight guards', () => {
     expect(mockRenameColumn).not.toHaveBeenCalled()
   })
 
+  it('forwards unique to the retype so it validates post-conversion values', async () => {
+    mockCheckAccess.mockResolvedValue({
+      ok: true,
+      table: {
+        workspaceId: WORKSPACE_ID,
+        schema: { columns: [{ id: 'col_a', name: 'amount', type: 'string' }] },
+      },
+    })
+    mockUpdateColumnType.mockResolvedValue({ schema: { columns: [] } })
+    mockUpdateColumnConstraints.mockResolvedValue({ schema: { columns: [] } })
+
+    const response = await patch({ type: 'number', unique: true })
+
+    expect(response.status).toBe(200)
+    // The conversion itself can manufacture duplicates ("5" and "5.0" both
+    // coerce to 5), so the retype has to see `unique` — discovering it in the
+    // separate constraint write would report an error with the conversion
+    // already committed and the original text irrecoverably rewritten.
+    expect(mockUpdateColumnType).toHaveBeenCalledWith(
+      expect.objectContaining({ newType: 'number', unique: true }),
+      expect.any(String)
+    )
+  })
+
+  it('rejects constraint changes on a workflow-output column before any write', async () => {
+    mockCheckAccess.mockResolvedValue({
+      ok: true,
+      table: {
+        workspaceId: WORKSPACE_ID,
+        schema: {
+          columns: [{ id: 'col_a', name: 'amount', type: 'number', workflowGroupId: 'g1' }],
+        },
+      },
+    })
+
+    const response = await patch({ type: 'string', required: true })
+
+    expect(response.status).toBe(400)
+    expect(await response.json()).toMatchObject({
+      error: expect.stringContaining('workflow-output column'),
+    })
+    expect(mockUpdateColumnType).not.toHaveBeenCalled()
+  })
+
   it('rejects unique on a type that cannot carry it without renaming first', async () => {
     mockCheckAccess.mockResolvedValue({
       ok: true,

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

@@ -182,6 +182,17 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
         { 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) {
       return NextResponse.json(
         { error: `Cannot set a ${resultingType} column as unique` },
@@ -201,6 +212,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
           // 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

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

@@ -216,6 +216,17 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
         { 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) {
       return NextResponse.json(
         { error: `Cannot set a ${resultingType} column as unique` },
@@ -235,6 +246,7 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: Colu
           // 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

apps/sim/lib/table/columns/service.ts

@@ -485,6 +485,26 @@ export async function deleteColumns(
   return def
 }
 
+/**
+ * Whether any two rows share a stored value in this column.
+ *
+ * Shared by the constraint write and the retype's pre-validation so the two
+ * cannot drift — the same reason {@link countEmptyCells} is shared. A retype
+ * that sets `unique` in the same request has to run this against the values the
+ * conversion is ABOUT to write, not the ones on disk: coercing `"5"` and `"5.0"`
+ * to a number manufactures a duplicate that no pre-scan of the raw text sees.
+ */
+async function hasDuplicateValues(
+  trx: DbTransaction,
+  tableId: string,
+  columnKey: string
+): Promise<boolean> {
+  const duplicates = (await trx.execute(
+    sql`SELECT ${userTableRows.data}->>${columnKey}::text AS val, count(*) AS cnt FROM ${userTableRows} WHERE table_id = ${tableId} AND ${userTableRows.data} ? ${columnKey} AND ${userTableRows.data}->>${columnKey}::text IS NOT NULL GROUP BY val HAVING count(*) > 1 LIMIT 1`
+  )) as { val: string; cnt: number }[]
+  return duplicates.length > 0
+}
+
 /**
  * Validates a pending rename against the schema it will land in, and returns
  * the renamed column.
@@ -762,6 +782,21 @@ export async function updateColumnType(
       await writeBackCoercedCells(trx, data.tableId, columnKey, coercedByRowId)
     }
 
+    // A `unique` arriving with this retype is validated HERE, against the values
+    // the conversion just wrote — not by the separate constraint write that
+    // follows. The conversion itself manufactures duplicates that no scan of the
+    // pre-conversion data can see (`"5"` and `"5.0"` both coerce to `5`), and
+    // that write runs in its own transaction, so discovering it there would
+    // report an error with the retype already committed and the original text
+    // irrecoverably rewritten.
+    if (data.unique === true && !column.unique) {
+      if (await hasDuplicateValues(trx, data.tableId, columnKey)) {
+        throw new Error(
+          `Cannot change column "${column.name}" to type "${data.newType}" and set it as unique: the converted values contain duplicates.`
+        )
+      }
+    }
+
     await trx
       .update(userTableDefinitions)
       .set({ schema: updatedSchema, updatedAt: now })
@@ -829,11 +864,7 @@ export async function updateColumnConstraints(
     }
 
     if (data.unique === true && !column.unique) {
-      const duplicates = (await trx.execute(
-        sql`SELECT ${userTableRows.data}->>${columnKey}::text AS val, count(*) AS cnt FROM ${userTableRows} WHERE table_id = ${data.tableId} AND ${userTableRows.data} ? ${columnKey} AND ${userTableRows.data}->>${columnKey}::text IS NOT NULL GROUP BY val HAVING count(*) > 1 LIMIT 1`
-      )) as { val: string; cnt: number }[]
-
-      if (duplicates.length > 0) {
+      if (await hasDuplicateValues(trx, data.tableId, columnKey)) {
         throw new Error(`Cannot set column "${column.name}" as unique: duplicate values exist`)
       }
     }

apps/sim/lib/table/types.ts

@@ -793,6 +793,12 @@ export interface UpdateColumnTypeData {
   multiple?: boolean
   /** Currency to set when changing to the `currency` type. */
   currencyCode?: string
+  /**
+   * The `unique` value the same request is about to set. Validated inside the
+   * retype against the post-conversion values, because the conversion is what
+   * can create the duplicates.
+   */
+  unique?: boolean
   /**
    * The `required` value the same request is about to set, when it changes type
    * and constraints together. Those are separate transactions, so the