Skip to content

Commit c6b101a

Browse files
committed
fix(tables): write single-cell edits as an atomic JSONB patch (fix row-level LWW)
A table row stores every cell in one jsonb `data` column, and the row-edit paths called updateRow in the default replace mode — a full-object read-modify-write. Two users (or a user and an agent/copilot) editing DIFFERENT cells of the same row concurrently would clobber each other: whichever write committed last overwrote the whole row from its stale snapshot. Pass dataWriteMode: 'patch' (Postgres `data = data || patch::jsonb`) from the four partial-update callers — the UI row route, the v1 API row route, the copilot table tool, and the workflow-group cancel-write — so each cell edit merges atomically under the row lock. upsertRow (whole-row by design) and updateRowsByFilter (already uses ||) are unchanged. computedWrite stays unset on the user paths, preserving the update lock. Adds a route test asserting the UI route opts into patch mode; the updateRow patch mechanism itself is already covered.
1 parent 1a4bfe4 commit c6b101a

5 files changed

Lines changed: 106 additions & 4 deletions

File tree

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { NextRequest } from 'next/server'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { mockAuth, mockCheckAccess, mockUpdateRow } = vi.hoisted(() => ({
8+
mockAuth: vi.fn(),
9+
mockCheckAccess: vi.fn(),
10+
mockUpdateRow: vi.fn(),
11+
}))
12+
13+
vi.mock('@/lib/auth/hybrid', () => ({
14+
checkSessionOrInternalAuth: mockAuth,
15+
}))
16+
17+
vi.mock('@/lib/table', () => ({
18+
updateRow: mockUpdateRow,
19+
deleteRow: vi.fn(),
20+
}))
21+
22+
vi.mock('@/app/api/table/row-wire', () => ({
23+
rowWireTranslators: () => ({
24+
dataIn: (data: unknown) => data,
25+
dataOut: (data: unknown) => data,
26+
}),
27+
}))
28+
29+
vi.mock('@/app/api/table/utils', async () => {
30+
const { NextResponse } = await import('next/server')
31+
return {
32+
checkAccess: mockCheckAccess,
33+
accessError: (result: { status: number }) =>
34+
NextResponse.json({ error: 'denied' }, { status: result.status }),
35+
rowWriteErrorResponse: () => undefined,
36+
tableLockErrorResponse: () => undefined,
37+
rootErrorMessage: (error: unknown) => (error instanceof Error ? error.message : ''),
38+
}
39+
})
40+
41+
import { PATCH } from '@/app/api/table/[tableId]/rows/[rowId]/route'
42+
43+
const TABLE = {
44+
id: 'tbl_1',
45+
name: 'People',
46+
workspaceId: 'ws-1',
47+
schema: { columns: [{ id: 'col_1', name: 'name', type: 'text' }] },
48+
}
49+
50+
function patchRequest(body: unknown) {
51+
return new NextRequest('http://localhost:3000/api/table/tbl_1/rows/row_1', {
52+
method: 'PATCH',
53+
headers: { 'content-type': 'application/json' },
54+
body: JSON.stringify(body),
55+
})
56+
}
57+
58+
describe('PATCH /api/table/[tableId]/rows/[rowId]', () => {
59+
beforeEach(() => {
60+
vi.clearAllMocks()
61+
mockAuth.mockResolvedValue({ success: true, userId: 'user-1', authType: 'session' })
62+
mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE })
63+
mockUpdateRow.mockResolvedValue({
64+
id: 'row_1',
65+
data: { col_1: 'Bob' },
66+
position: 0,
67+
createdAt: new Date('2026-01-01T00:00:00.000Z'),
68+
updatedAt: new Date('2026-01-01T00:00:01.000Z'),
69+
})
70+
})
71+
72+
it("writes only the edited cells (dataWriteMode: 'patch') so a concurrent edit to another cell of the same row is not clobbered", async () => {
73+
const res = await PATCH(patchRequest({ workspaceId: 'ws-1', data: { col_1: 'Bob' } }), {
74+
params: Promise.resolve({ tableId: 'tbl_1', rowId: 'row_1' }),
75+
})
76+
77+
expect(res.status).toBe(200)
78+
expect(mockUpdateRow).toHaveBeenCalledTimes(1)
79+
// The 4th argument is the options object; it must opt into cell-atomic JSONB patch mode.
80+
expect(mockUpdateRow.mock.calls[0][3]).toEqual({ dataWriteMode: 'patch' })
81+
})
82+
})

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
133133
}
134134

135135
const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema)
136+
// Write only the edited cells via a Postgres JSONB concat (`data = data || patch`) instead of
137+
// replacing the whole `data` object. A table row stores every cell in one jsonb column, so a
138+
// full-object replace is last-write-wins across the ROW: two users editing different cells of
139+
// the same row concurrently would clobber each other. `patch` merges under the row lock, so
140+
// each cell edit is atomic. `computedWrite` stays unset — this is a user edit, still subject to
141+
// the normal update lock (unlike the workflow engine writing its own output cells).
136142
const updatedRow = await updateRow(
137143
{
138144
tableId,
@@ -142,7 +148,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
142148
actorUserId: authResult.userId,
143149
},
144150
table,
145-
requestId
151+
requestId,
152+
{ dataWriteMode: 'patch' }
146153
)
147154
// Only `null` when a `cancellationGuard` is supplied and the SQL guard
148155
// rejects the write — this route doesn't pass one, so reaching null is a bug.

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,10 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
139139

140140
const idByName = buildIdByName(table.schema as TableSchema)
141141
const toNamedRow = namedRowMapper((table.schema as TableSchema).columns)
142+
// Patch only the edited cells (`data = data || patch`) so concurrent edits to different cells
143+
// of the same row don't clobber each other — a row stores all its cells in one jsonb column,
144+
// so a full-object replace is last-write-wins across the whole row. See the app route for the
145+
// full rationale.
142146
const updatedRow = await updateRow(
143147
{
144148
tableId,
@@ -148,7 +152,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
148152
actorUserId,
149153
},
150154
table,
151-
requestId
155+
requestId,
156+
{ dataWriteMode: 'patch' }
152157
)
153158
// No `cancellationGuard` is passed here, so `updateRow` can't return null
154159
// from this caller. Defensive narrowing for TypeScript.

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,9 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
727727
assertNotAborted()
728728
const idByName = buildIdByName(table.schema)
729729
const toNamedRow = namedRowMapper(table.schema.columns)
730+
// Patch only the edited cells so a copilot edit doesn't clobber a concurrent user edit
731+
// to a different cell of the same row (all cells share one jsonb column — a full-object
732+
// replace is last-write-wins across the row). See the table row route for the rationale.
730733
const updatedRow = await updateRow(
731734
{
732735
tableId: args.tableId,
@@ -736,7 +739,8 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
736739
actorUserId: context.userId,
737740
},
738741
table,
739-
requestId
742+
requestId,
743+
{ dataWriteMode: 'patch' }
740744
)
741745
if (!updatedRow) {
742746
// Only the cell-task path passes a `cancellationGuard`; this caller

apps/sim/lib/table/workflow-columns.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,11 @@ export async function cancelWorkflowGroupRuns(
627627
executionsPatch: m.executionsPatch,
628628
},
629629
table,
630-
`wfgrp-cancel-${m.rowId}`
630+
`wfgrp-cancel-${m.rowId}`,
631+
// This write only touches execution state (data is `{}`). `patch` makes the data write a
632+
// true no-op (`data || '{}'`), whereas the default replace would rewrite the whole jsonb
633+
// column and could revert a user's concurrent edit to a cell of the same row.
634+
{ dataWriteMode: 'patch' }
631635
).catch((err) => {
632636
logger.error(`Failed to write cancelled state for row ${m.rowId}:`, err)
633637
})

0 commit comments

Comments
 (0)