Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/**
* @vitest-environment node
*/
import { NextRequest } from 'next/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const { mockAuth, mockCheckAccess, mockUpdateRow } = vi.hoisted(() => ({
mockAuth: vi.fn(),
mockCheckAccess: vi.fn(),
mockUpdateRow: vi.fn(),
}))

vi.mock('@/lib/auth/hybrid', () => ({
checkSessionOrInternalAuth: mockAuth,
}))

vi.mock('@/lib/table', () => ({
updateRow: mockUpdateRow,
deleteRow: vi.fn(),
}))

vi.mock('@/app/api/table/row-wire', () => ({
rowWireTranslators: () => ({
dataIn: (data: unknown) => data,
dataOut: (data: unknown) => data,
}),
}))

vi.mock('@/app/api/table/utils', async () => {
const { NextResponse } = await import('next/server')
return {
checkAccess: mockCheckAccess,
accessError: (result: { status: number }) =>
NextResponse.json({ error: 'denied' }, { status: result.status }),
rowWriteErrorResponse: () => undefined,
tableLockErrorResponse: () => undefined,
rootErrorMessage: () => '',
}
})

import { PATCH } from '@/app/api/table/[tableId]/rows/[rowId]/route'

const TABLE = {
id: 'tbl_1',
name: 'People',
workspaceId: 'ws-1',
schema: { columns: [{ id: 'col_1', name: 'name', type: 'text' }] },
}

function patchRequest(body: unknown) {
return new NextRequest('http://localhost:3000/api/table/tbl_1/rows/row_1', {
method: 'PATCH',
headers: { 'content-type': 'application/json' },
body: JSON.stringify(body),
})
}

describe('PATCH /api/table/[tableId]/rows/[rowId]', () => {
beforeEach(() => {
vi.clearAllMocks()
mockAuth.mockResolvedValue({ success: true, userId: 'user-1', authType: 'session' })
mockCheckAccess.mockResolvedValue({ ok: true, table: TABLE })
mockUpdateRow.mockResolvedValue({
id: 'row_1',
data: { col_1: 'Bob' },
position: 0,
createdAt: new Date('2026-01-01T00:00:00.000Z'),
updatedAt: new Date('2026-01-01T00:00:01.000Z'),
})
})

it("writes only the edited cells (dataWriteMode: 'patch') so a concurrent edit to another cell of the same row is not clobbered", async () => {
const res = await PATCH(patchRequest({ workspaceId: 'ws-1', data: { col_1: 'Bob' } }), {
params: Promise.resolve({ tableId: 'tbl_1', rowId: 'row_1' }),
})

expect(res.status).toBe(200)
expect(mockUpdateRow).toHaveBeenCalledTimes(1)
// The 4th argument is the options object; it must opt into cell-atomic JSONB patch mode.
expect(mockUpdateRow.mock.calls[0][3]).toEqual({ dataWriteMode: 'patch' })
})
})
9 changes: 8 additions & 1 deletion apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
}

const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema)
// Write only the edited cells via a Postgres JSONB concat (`data = data || patch`) instead of
// replacing the whole `data` object. A table row stores every cell in one jsonb column, so a
// full-object replace is last-write-wins across the ROW: two users editing different cells of
// the same row concurrently would clobber each other. `patch` merges under the row lock, so
// each cell edit is atomic. `computedWrite` stays unset — this is a user edit, still subject to
// the normal update lock (unlike the workflow engine writing its own output cells).
const updatedRow = await updateRow(
{
tableId,
Expand All @@ -142,7 +148,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
actorUserId: authResult.userId,
},
table,
requestId
requestId,
{ dataWriteMode: 'patch' }
)
// Only `null` when a `cancellationGuard` is supplied and the SQL guard
// rejects the write — this route doesn't pass one, so reaching null is a bug.
Expand Down
7 changes: 6 additions & 1 deletion apps/sim/app/api/v1/tables/[tableId]/rows/[rowId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR

const idByName = buildIdByName(table.schema as TableSchema)
const toNamedRow = namedRowMapper((table.schema as TableSchema).columns)
// Patch only the edited cells (`data = data || patch`) so concurrent edits to different cells
// of the same row don't clobber each other — a row stores all its cells in one jsonb column,
// so a full-object replace is last-write-wins across the whole row. See the app route for the
// full rationale.
const updatedRow = await updateRow(
{
tableId,
Expand All @@ -148,7 +152,8 @@ export const PATCH = withRouteHandler(async (request: NextRequest, context: RowR
actorUserId,
},
table,
requestId
requestId,
{ dataWriteMode: 'patch' }
)
// No `cancellationGuard` is passed here, so `updateRow` can't return null
// from this caller. Defensive narrowing for TypeScript.
Expand Down
6 changes: 5 additions & 1 deletion apps/sim/lib/copilot/tools/server/table/user-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,9 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
assertNotAborted()
const idByName = buildIdByName(table.schema)
const toNamedRow = namedRowMapper(table.schema.columns)
// Patch only the edited cells so a copilot edit doesn't clobber a concurrent user edit
// to a different cell of the same row (all cells share one jsonb column — a full-object
// replace is last-write-wins across the row). See the table row route for the rationale.
const updatedRow = await updateRow(
{
tableId: args.tableId,
Expand All @@ -736,7 +739,8 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
actorUserId: context.userId,
},
table,
requestId
requestId,
{ dataWriteMode: 'patch' }
)
if (!updatedRow) {
// Only the cell-task path passes a `cancellationGuard`; this caller
Expand Down
6 changes: 5 additions & 1 deletion apps/sim/lib/table/workflow-columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,11 @@ export async function cancelWorkflowGroupRuns(
executionsPatch: m.executionsPatch,
},
table,
`wfgrp-cancel-${m.rowId}`
`wfgrp-cancel-${m.rowId}`,
// This write only touches execution state (data is `{}`). `patch` makes the data write a
// true no-op (`data || '{}'`), whereas the default replace would rewrite the whole jsonb
// column and could revert a user's concurrent edit to a cell of the same row.
{ dataWriteMode: 'patch' }
).catch((err) => {
logger.error(`Failed to write cancelled state for row ${m.rowId}:`, err)
})
Expand Down
Loading