diff --git a/apps/sim/app/api/table/[tableId]/delete-async/route.test.ts b/apps/sim/app/api/table/[tableId]/delete-async/route.test.ts index cffb5dc8810..fb875ca66b6 100644 --- a/apps/sim/app/api/table/[tableId]/delete-async/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/delete-async/route.test.ts @@ -1,10 +1,15 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns, resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { + createTableDefinition, + hybridAuthMockFns, + resetEnvFlagsMock, + setEnvFlags, + type TableDefinitionFactoryOptions, +} from '@sim/testing' import { NextRequest, NextResponse } from 'next/server' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, @@ -58,22 +63,9 @@ import { POST } from '@/app/api/table/[tableId]/delete-async/route' afterAll(resetEnvFlagsMock) -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [{ name: 'status', type: 'string' }] }, - metadata: null, - rowCount: 1000, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } +const TABLE_FIXTURE: TableDefinitionFactoryOptions = { + columns: [{ name: 'status', type: 'string' }], + rowCount: 1000, } function makeRequest(body: unknown, tableId = 'tbl_1') { @@ -99,7 +91,7 @@ describe('POST /api/table/[tableId]/delete-async', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockMarkTableJobRunning.mockResolvedValue(true) mockRunTableDelete.mockResolvedValue(undefined) mockTableFilterError.mockReturnValue(null) @@ -168,7 +160,10 @@ describe('POST /api/table/[tableId]/delete-async', () => { }) it('returns 400 when the table is archived', async () => { - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable({ archivedAt: new Date() }) }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ ...TABLE_FIXTURE, archivedAt: new Date() }), + }) const response = await makeRequest(validBody) expect(response.status).toBe(400) expect(mockRunTableDelete).not.toHaveBeenCalled() diff --git a/apps/sim/app/api/table/[tableId]/dispatches/route.test.ts b/apps/sim/app/api/table/[tableId]/dispatches/route.test.ts index 3acddbbe325..b1c08d00328 100644 --- a/apps/sim/app/api/table/[tableId]/dispatches/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/dispatches/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockListActiveDispatches, mockCountRunningCells } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -27,24 +26,6 @@ vi.mock('@/app/api/table/utils', async () => { import { GET } from '@/app/api/table/[tableId]/dispatches/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [] }, - metadata: null, - rowCount: 0, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } -} - function makeRequest(tableId = 'tbl_1') { const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/dispatches`) return GET(req, { params: Promise.resolve({ tableId }) }) @@ -75,7 +56,7 @@ describe('GET /api/table/[tableId]/dispatches', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition() }) mockListActiveDispatches.mockResolvedValue([]) mockCountRunningCells.mockResolvedValue({ byRowId: {}, hasRunning: false }) }) diff --git a/apps/sim/app/api/table/[tableId]/export-async/route.test.ts b/apps/sim/app/api/table/[tableId]/export-async/route.test.ts index 5be95a85ee6..4922d49041c 100644 --- a/apps/sim/app/api/table/[tableId]/export-async/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/export-async/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockMarkTableJobRunning, mockRunTableExport } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -34,24 +33,6 @@ vi.mock('@/app/api/table/utils', async () => { import { POST } from '@/app/api/table/[tableId]/export-async/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [{ name: 'name', type: 'string' }] }, - metadata: null, - rowCount: 50000, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } -} - function makeRequest(body: unknown, tableId = 'tbl_1') { const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/export-async`, { method: 'POST', @@ -71,7 +52,13 @@ describe('POST /api/table/[tableId]/export-async', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ + columns: [{ name: 'name', type: 'string' }], + rowCount: 50000, + }), + }) mockMarkTableJobRunning.mockResolvedValue(true) mockRunTableExport.mockResolvedValue(undefined) }) diff --git a/apps/sim/app/api/table/[tableId]/export/download/route.test.ts b/apps/sim/app/api/table/[tableId]/export/download/route.test.ts index 53ed1a28cf3..b977f5ff9f8 100644 --- a/apps/sim/app/api/table/[tableId]/export/download/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/export/download/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockGetTableJob, mockGeneratePresignedDownloadUrl } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -27,24 +26,6 @@ vi.mock('@/app/api/table/utils', async () => { import { GET } from '@/app/api/table/[tableId]/export/download/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [] }, - metadata: null, - rowCount: 0, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } -} - function makeRequest(query: Record, tableId = 'tbl_1') { const qs = new URLSearchParams(query).toString() const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/export/download?${qs}`) @@ -61,7 +42,7 @@ describe('GET /api/table/[tableId]/export/download', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition() }) mockGetTableJob.mockResolvedValue({ id: 'job_1', type: 'export', diff --git a/apps/sim/app/api/table/[tableId]/export/route.test.ts b/apps/sim/app/api/table/[tableId]/export/route.test.ts index b420b5f97da..4e1dd314b3c 100644 --- a/apps/sim/app/api/table/[tableId]/export/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/export/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockQueryRows } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -27,27 +26,6 @@ vi.mock('@/lib/table/rows/service', () => ({ import { GET } from '@/app/api/table/[tableId]/export/route' /** Table with an id-native column whose stable id (`col_email`) differs from its display name. */ -function buildTable(): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { - columns: [ - { id: 'col_email', name: 'email', type: 'string' }, - { name: 'legacy', type: 'string' }, // legacy: id == name - ], - }, - metadata: null, - rowCount: 1, - maxRows: 100, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date('2024-01-01'), - updatedAt: new Date('2024-01-01'), - } -} function callGet(format: string) { const req = new NextRequest(`http://localhost:3000/api/table/tbl_1/export?format=${format}`, { @@ -64,7 +42,19 @@ describe('table export route — id→name translation', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ + columns: [ + { id: 'col_email', name: 'email', type: 'string' }, + { name: 'legacy', type: 'string' }, // legacy: id == name + ], + rowCount: 1, + maxRows: 100, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), + }), + }) // Row data is keyed by stable column id (`col_email`), not the display name. mockQueryRows.mockResolvedValue({ rows: [{ id: 'r1', data: { col_email: 'a@b.c', legacy: 'x' }, executions: {}, position: 0 }], diff --git a/apps/sim/app/api/table/[tableId]/import-async/route.test.ts b/apps/sim/app/api/table/[tableId]/import-async/route.test.ts index 271cf2b8ed6..a22bd6b5e7d 100644 --- a/apps/sim/app/api/table/[tableId]/import-async/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/import-async/route.test.ts @@ -1,10 +1,13 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { + createTableDefinition, + hybridAuthMockFns, + type TableDefinitionFactoryOptions, +} from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockMarkTableImporting, mockRunTableImport } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -34,22 +37,8 @@ vi.mock('@/app/api/table/utils', async () => { import { POST } from '@/app/api/table/[tableId]/import-async/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [{ name: 'name', type: 'string' }] }, - metadata: null, - rowCount: 0, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } +const TABLE_FIXTURE: TableDefinitionFactoryOptions = { + columns: [{ name: 'name', type: 'string' }], } function makeRequest(body: unknown, tableId = 'tbl_1') { @@ -76,7 +65,7 @@ describe('POST /api/table/[tableId]/import-async', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockMarkTableImporting.mockResolvedValue(true) mockRunTableImport.mockResolvedValue(undefined) }) @@ -126,7 +115,10 @@ describe('POST /api/table/[tableId]/import-async', () => { }) it('returns 400 when the target table is archived', async () => { - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable({ archivedAt: new Date() }) }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ ...TABLE_FIXTURE, archivedAt: new Date() }), + }) const response = await makeRequest(validBody) expect(response.status).toBe(400) expect(mockRunTableImport).not.toHaveBeenCalled() diff --git a/apps/sim/app/api/table/[tableId]/import/route.test.ts b/apps/sim/app/api/table/[tableId]/import/route.test.ts index a2689295725..45de3f210ee 100644 --- a/apps/sim/app/api/table/[tableId]/import/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/import/route.test.ts @@ -1,7 +1,11 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { + createTableDefinition, + hybridAuthMockFns, + type TableDefinitionFactoryOptions, +} from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { TableDefinition } from '@/lib/table' @@ -122,27 +126,14 @@ function createFormData( return form } -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { - columns: [ - { name: 'name', type: 'string', required: true }, - { name: 'age', type: 'number' }, - ], - }, - metadata: null, - rowCount: 0, - maxRows: 100, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date('2024-01-01'), - updatedAt: new Date('2024-01-01'), - ...overrides, - } +const TABLE_FIXTURE: TableDefinitionFactoryOptions = { + columns: [ + { name: 'name', type: 'string', required: true }, + { name: 'age', type: 'number' }, + ], + maxRows: 100, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), } /** Additions array the route passed to importAppendRows (2nd positional arg). */ @@ -173,7 +164,7 @@ describe('POST /api/table/[tableId]/import', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockImportAppendRows.mockImplementation( async (table: TableDefinition, _additions: unknown, rows: unknown[]) => ({ inserted: rows.map((_, i) => ({ id: `row_${i}` })), @@ -229,7 +220,7 @@ describe('POST /api/table/[tableId]/import', () => { it('returns 400 when the target table is archived', async () => { mockCheckAccess.mockResolvedValueOnce({ ok: true, - table: buildTable({ archivedAt: new Date('2024-01-02') }), + table: createTableDefinition({ ...TABLE_FIXTURE, archivedAt: new Date('2024-01-02') }), }) const response = await callPost(createFormData(createCsvFile('name,age\nAlice,30'))) expect(response.status).toBe(400) @@ -306,7 +297,10 @@ describe('POST /api/table/[tableId]/import', () => { }) it('rejects append when it would exceed the current plan row limit', async () => { - mockCheckAccess.mockResolvedValueOnce({ ok: true, table: buildTable({ rowCount: 99 }) }) + mockCheckAccess.mockResolvedValueOnce({ + ok: true, + table: createTableDefinition({ ...TABLE_FIXTURE, rowCount: 99 }), + }) mockGetMaxRowsPerTable.mockResolvedValueOnce(100) const response = await callPost( createFormData(createCsvFile('name,age\nAlice,30\nBob,40'), { mode: 'append' }) @@ -459,14 +453,13 @@ describe('POST /api/table/[tableId]/import', () => { it('dedupes when sanitized name collides with an existing column', async () => { mockCheckAccess.mockResolvedValueOnce({ ok: true, - table: buildTable({ - schema: { - columns: [ - { name: 'name', type: 'string', required: true }, - { name: 'age', type: 'number' }, - { name: 'email', type: 'string' }, - ], - }, + table: createTableDefinition({ + ...TABLE_FIXTURE, + columns: [ + { name: 'name', type: 'string', required: true }, + { name: 'age', type: 'number' }, + { name: 'email', type: 'string' }, + ], }), }) const response = await callPost( diff --git a/apps/sim/app/api/table/[tableId]/job/cancel/route.test.ts b/apps/sim/app/api/table/[tableId]/job/cancel/route.test.ts index dd0a16b2576..0bc244c2182 100644 --- a/apps/sim/app/api/table/[tableId]/job/cancel/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/job/cancel/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockMarkJobCanceled, mockGetTableJob, mockAppendTableEvent } = vi.hoisted( () => ({ @@ -31,24 +30,6 @@ vi.mock('@/app/api/table/utils', async () => { import { POST } from '@/app/api/table/[tableId]/job/cancel/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [] }, - metadata: null, - rowCount: 0, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), - ...overrides, - } -} - function makeRequest(body: unknown, tableId = 'tbl_1') { const req = new NextRequest(`http://localhost:3000/api/table/${tableId}/job/cancel`, { method: 'POST', @@ -68,7 +49,7 @@ describe('POST /api/table/[tableId]/job/cancel', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition() }) mockMarkJobCanceled.mockResolvedValue(true) mockGetTableJob.mockResolvedValue({ id: 'job_1', diff --git a/apps/sim/app/api/table/[tableId]/query/route.test.ts b/apps/sim/app/api/table/[tableId]/query/route.test.ts index e0857d6589c..9d58e383f7e 100644 --- a/apps/sim/app/api/table/[tableId]/query/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/query/route.test.ts @@ -5,10 +5,9 @@ * (session auth included — the string grammar is name-keyed for every caller), * cursor validation, and the response envelope. */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table/types' const { mockCheckAccess, mockQueryRows, mockGate } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -39,28 +38,6 @@ vi.mock('@/lib/table/rows/service', () => ({ import { encodeCursor } from '@/lib/table/rows/cursor' import { POST } from '@/app/api/table/[tableId]/query/route' -function buildTable(): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { - columns: [ - { id: 'col_aaa', name: 'name', type: 'string' }, - { id: 'col_bbb', name: 'wins', type: 'number' }, - ], - }, - metadata: null, - rowCount: 0, - maxRows: 100, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date('2024-01-01'), - updatedAt: new Date('2024-01-01'), - } -} - function authAs(authType: 'session' | 'internal_jwt') { hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ success: true, @@ -90,7 +67,18 @@ const EMPTY_RESULT = { describe('POST /api/table/[tableId]/query', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ + columns: [ + { id: 'col_aaa', name: 'name', type: 'string' }, + { id: 'col_bbb', name: 'wins', type: 'number' }, + ], + maxRows: 100, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), + }), + }) mockQueryRows.mockResolvedValue(EMPTY_RESULT) mockGate.mockResolvedValue(null) }) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts index 8ef809a71a1..1ae9263975f 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.test.ts @@ -1,118 +1,102 @@ /** * @vitest-environment node + * + * The enrichment-detail surface after moving onto the shared internal route + * builder. It previously queried the database from the adapter; the assertions + * below are the same wire outcomes, now with the use case as the seam. */ -import { hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { EnrichmentRunDetail, TableDefinition } from '@/lib/table' -const { mockCheckAccess, mockLoadEnrichmentDetail } = vi.hoisted(() => ({ - mockCheckAccess: vi.fn(), - mockLoadEnrichmentDetail: vi.fn(), +const { mocks } = vi.hoisted(() => ({ + mocks: { readDetail: vi.fn(), authenticate: vi.fn() }, })) -vi.mock('@/lib/table/rows/executions', () => ({ - loadEnrichmentDetail: mockLoadEnrichmentDetail, -})) -vi.mock('@/app/api/table/utils', async () => { - const { NextResponse } = await import('next/server') +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() return { - checkAccess: mockCheckAccess, - accessError: (result: { status: number }) => - NextResponse.json({ error: 'denied' }, { status: result.status }), + ...actual, + readTableRowEnrichmentDetail: { + operation: { id: 'tables.rows.read' }, + execute: mocks.readDetail, + }, } }) +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } +}) + +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { NoWorkspaceAccessError } from '@/lib/core/application' import { GET } from '@/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route' -function buildTable(): TableDefinition { +const TABLE = { id: 'tbl_1', workspaceId: 'workspace-1', schema: { columns: [] } } +const DETAIL = { providers: [{ id: 'clearbit', status: 'hit' }], costUsd: 0.01 } + +function routeContext() { return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [] }, - metadata: null, - rowCount: 1, - maxRows: 1_000_000, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date(), - updatedAt: new Date(), + params: Promise.resolve({ tableId: 'tbl_1', rowId: 'row_1', groupId: 'grp_1' }), } } -function makeRequest(tableId = 'tbl_1', rowId = 'row_1', groupId = 'grp_1') { - const req = new NextRequest( - `http://localhost:3000/api/table/${tableId}/rows/${rowId}/enrichment/${groupId}` - ) - return GET(req, { params: Promise.resolve({ tableId, rowId, groupId }) }) +function request() { + return new NextRequest('http://localhost/api/table/tbl_1/rows/row_1/enrichment/grp_1', { + method: 'GET', + }) } -const detail: EnrichmentRunDetail = { - startedAt: '2026-06-18T00:00:00.000Z', - completedAt: '2026-06-18T00:00:01.000Z', - durationMs: 1000, - totalCost: 0.05, - matchedProvider: 'hunter', - aborted: false, - providers: [ - { - id: 'hunter', - label: 'Hunter', - toolId: 'hunter_find_email', - status: 'matched', - cost: 0.05, - durationMs: 1000, - error: null, - }, - ], -} +beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) + mocks.readDetail.mockResolvedValue({ table: TABLE, detail: DETAIL }) +}) describe('GET /api/table/[tableId]/rows/[rowId]/enrichment/[groupId]', () => { - beforeEach(() => { - vi.clearAllMocks() - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ - success: true, - userId: 'user-1', - authType: 'session', - }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + it('returns 401 when the caller is not authenticated', async () => { + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) + + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.readDetail).not.toHaveBeenCalled() }) it('returns the enrichment detail', async () => { - mockLoadEnrichmentDetail.mockResolvedValue(detail) - const res = await makeRequest() - expect(res.status).toBe(200) - const json = await res.json() - expect(json).toEqual({ success: true, data: { detail } }) - expect(mockLoadEnrichmentDetail).toHaveBeenCalledWith( - expect.anything(), - 'tbl_1', - 'row_1', - 'grp_1' - ) + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ success: true, data: { detail: DETAIL } }) }) it('returns null when there is no recorded run', async () => { - mockLoadEnrichmentDetail.mockResolvedValue(null) - const res = await makeRequest() - expect(res.status).toBe(200) - const json = await res.json() - expect(json).toEqual({ success: true, data: { detail: null } }) + mocks.readDetail.mockResolvedValue({ table: TABLE, detail: null }) + + const body = await (await GET(request(), routeContext())).json() + + expect(body).toEqual({ success: true, data: { detail: null } }) }) - it('401s when unauthenticated', async () => { - hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ success: false }) - const res = await makeRequest() - expect(res.status).toBe(401) - expect(mockLoadEnrichmentDetail).not.toHaveBeenCalled() + it('passes the row and group through to the use case', async () => { + await GET(request(), routeContext()) + + expect(mocks.readDetail.mock.calls[0][0].input).toMatchObject({ + tableId: 'tbl_1', + rowId: 'row_1', + groupId: 'grp_1', + }) }) - it('denies when access check fails', async () => { - mockCheckAccess.mockResolvedValue({ ok: false, status: 403 }) - const res = await makeRequest() - expect(res.status).toBe(403) - expect(mockLoadEnrichmentDetail).not.toHaveBeenCalled() + it('conceals a cross-tenant table rather than confirming it exists', async () => { + mocks.readDetail.mockRejectedValue(new NoWorkspaceAccessError()) + + const response = await GET(request(), routeContext()) + + expect(response.status).toBe(404) }) }) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts index 34a045f7677..3caf42247f5 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/enrichment/[groupId]/route.ts @@ -1,51 +1,32 @@ -import { db } from '@sim/db' -import { createLogger } from '@sim/logger' -import { type NextRequest, NextResponse } from 'next/server' import { getEnrichmentDetailContract } from '@/lib/api/contracts/tables' -import { parseRequest } from '@/lib/api/server' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { loadEnrichmentDetail } from '@/lib/table/rows/executions' -import { accessError, checkAccess } from '@/app/api/table/utils' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' +import { tableOperations } from '@/lib/table/application/operations' +import { readTableRowEnrichmentDetail } from '@/lib/table/application/rows' -const logger = createLogger('EnrichmentDetailAPI') - -interface RouteParams { - params: Promise<{ tableId: string; rowId: string; groupId: string }> -} +export const dynamic = 'force-dynamic' /** * GET /api/table/[tableId]/rows/[rowId]/enrichment/[groupId] * - * Returns the enrichment cascade breakdown (provider outcomes, cost, timing) - * for one enrichment cell. Read on demand by the enrichment details panel — - * this data is deliberately kept off the hot grid read. Returns `null` for - * cells with no recorded run or runs that predate the feature. + * The enrichment cascade breakdown — provider outcomes, cost, timing — for one + * enrichment cell. Read on demand by the details panel; this data is + * deliberately kept off the hot grid read. */ -export const GET = withRouteHandler(async (request: NextRequest, { params }: RouteParams) => { - const requestId = generateRequestId() - - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const parsed = await parseRequest(getEnrichmentDetailContract, request, { params }) - if (!parsed.success) return parsed.response - const { tableId, rowId, groupId } = parsed.data.params - - const result = await checkAccess(tableId, authResult.userId, 'read') - if (!result.ok) return accessError(result, requestId, tableId) - - const detail = await loadEnrichmentDetail(db, tableId, rowId, groupId) - - logger.info(`[${requestId}] Loaded enrichment detail`, { - tableId, - rowId, - groupId, - hasDetail: detail !== null, - }) - - return NextResponse.json({ success: true, data: { detail } }) +export const GET = defineInternalJsonRoute({ + contract: getEnrichmentDetailContract, + operation: tableOperations.readRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit: internalRateLimits.none({ + reason: 'Preserve existing internal enrichment-detail behavior', + }), + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params }) => ({ + tableId: params.tableId, + rowId: params.rowId, + groupId: params.groupId, + }), + useCase: readTableRowEnrichmentDetail, + present: ({ detail }) => ({ success: true as const, data: { detail } }), }) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts new file mode 100644 index 00000000000..cf4935b3b1e --- /dev/null +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.test.ts @@ -0,0 +1,373 @@ +/** + * @vitest-environment node + * + * Characterization tests for the single-row surface, carried across its + * migration onto the shared internal route builder. + * + * The assertions are the ones written against the hand-rolled handler — status + * codes, body shapes, ISO-8601 timestamps, and the dual-caller wire keying, + * where a session speaks stable column ids and a workflow execution speaks + * column names. What moved is the seam they mock: the route no longer loads the + * table or calls the row primitives itself, so the use cases are stubbed and the + * real builder runs. + * + * Two wire changes are deliberate; see the `deliberate wire changes` block. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { + readRow: vi.fn(), + updateRow: vi.fn(), + deleteRow: vi.fn(), + authenticate: vi.fn(), + }, +})) + +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + readTableRow: { operation: { id: 'tables.rows.read' }, execute: mocks.readRow }, + updateTableRow: { operation: { id: 'tables.rows.update' }, execute: mocks.updateRow }, + deleteTableRow: { operation: { id: 'tables.rows.delete' }, execute: mocks.deleteRow }, + } +}) + +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } +}) + +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { NoWorkspaceAccessError } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { TableLockedError } from '@/lib/table/mutation-locks' +import { DELETE, GET, PATCH } from '@/app/api/table/[tableId]/rows/[rowId]/route' + +const TABLE_ID = 'tbl_1' +const ROW_ID = 'row_1' +const WORKSPACE_ID = 'workspace-1' +const CREATED_AT = new Date('2024-01-01T00:00:00.000Z') +const UPDATED_AT = new Date('2024-02-02T00:00:00.000Z') + +const TABLE = { + id: TABLE_ID, + workspaceId: WORKSPACE_ID, + schema: { + columns: [ + { id: 'col_aaa', name: 'Name', type: 'string' as const }, + { id: 'col_bbb', name: 'Age', type: 'number' as const }, + ], + }, +} + +const ROW = { + id: ROW_ID, + data: { col_aaa: 'Ada', col_bbb: 36 }, + executions: {}, + position: 0, + createdAt: CREATED_AT, + updatedAt: UPDATED_AT, +} + +function sessionPrincipal() { + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) +} + +function executorPrincipal() { + mocks.authenticate.mockResolvedValue({ + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'user-1', + workspaceId: WORKSPACE_ID, + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), + }) +} + +function unauthenticated() { + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) +} + +function routeContext() { + return { params: Promise.resolve({ tableId: TABLE_ID, rowId: ROW_ID }) } +} + +function getRequest(workspaceId: string | null = WORKSPACE_ID) { + const url = new URL(`http://localhost/api/table/${TABLE_ID}/rows/${ROW_ID}`) + if (workspaceId !== null) url.searchParams.set('workspaceId', workspaceId) + return new NextRequest(url, { method: 'GET' }) +} + +function bodyRequest(method: 'PATCH' | 'DELETE', body: unknown, headers: HeadersInit = {}) { + return new NextRequest(`http://localhost/api/table/${TABLE_ID}/rows/${ROW_ID}`, { + method, + headers: { 'content-type': 'application/json', ...headers }, + body: JSON.stringify(body), + }) +} + +beforeEach(() => { + vi.clearAllMocks() + sessionPrincipal() + mocks.readRow.mockResolvedValue({ table: TABLE, row: ROW }) + mocks.updateRow.mockResolvedValue({ table: TABLE, row: ROW, changed: true }) + mocks.deleteRow.mockResolvedValue({ table: TABLE, deletedRowId: ROW_ID }) +}) + +describe('GET /api/table/[tableId]/rows/[rowId]', () => { + it('returns 401 when the caller is not authenticated', async () => { + unauthenticated() + + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.readRow).not.toHaveBeenCalled() + }) + + it('returns 400 when workspaceId is absent from the query string', async () => { + const response = await GET(getRequest(null), routeContext()) + + expect(response.status).toBe(400) + expect(mocks.readRow).not.toHaveBeenCalled() + }) + + it('asserts the caller-supplied workspace on the use case rather than checking it here', async () => { + await GET(getRequest(), routeContext()) + + expect(mocks.readRow.mock.calls[0][0].input).toMatchObject({ + tableId: TABLE_ID, + rowId: ROW_ID, + assertedWorkspaceId: WORKSPACE_ID, + }) + }) + + it('returns 404 when the row does not exist', async () => { + mocks.readRow.mockRejectedValue(new OrchestrationError('not_found', 'Row not found')) + + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(404) + }) + + it('returns the row with ISO-8601 timestamps under data.row', async () => { + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ + success: true, + data: { + row: { + id: ROW_ID, + data: { col_aaa: 'Ada', col_bbb: 36 }, + position: 0, + createdAt: CREATED_AT.toISOString(), + updatedAt: UPDATED_AT.toISOString(), + }, + }, + }) + }) + + it('returns column names to a workflow execution', async () => { + executorPrincipal() + + const response = await GET(getRequest(), routeContext()) + + const body = await response.json() + expect(body.data.row.data).toEqual({ Name: 'Ada', Age: 36 }) + }) +}) + +describe('PATCH /api/table/[tableId]/rows/[rowId]', () => { + const patchBody = { workspaceId: WORKSPACE_ID, data: { col_aaa: 'Grace' } } + + it('returns 401 when the caller is not authenticated', async () => { + unauthenticated() + + const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.updateRow).not.toHaveBeenCalled() + }) + + it('returns 400 when the body fails contract validation', async () => { + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID }), + routeContext() + ) + + expect(response.status).toBe(400) + expect(mocks.updateRow).not.toHaveBeenCalled() + }) + + it('returns the updated row and the success message', async () => { + const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ + success: true, + data: { + row: { + id: ROW_ID, + data: { col_aaa: 'Ada', col_bbb: 36 }, + position: 0, + createdAt: CREATED_AT.toISOString(), + updatedAt: UPDATED_AT.toISOString(), + }, + message: 'Row updated successfully', + }, + }) + }) + + it('tells the use case a session speaks column ids', async () => { + await PATCH(bodyRequest('PATCH', patchBody), routeContext()) + + expect(mocks.updateRow.mock.calls[0][0].input).toMatchObject({ + data: { col_aaa: 'Grace' }, + dataKeying: 'ids', + strictWrite: false, + }) + }) + + it('tells the use case a workflow execution speaks column names', async () => { + executorPrincipal() + + await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { Name: 'Grace' } }), + routeContext() + ) + + expect(mocks.updateRow.mock.calls[0][0].input).toMatchObject({ + data: { Name: 'Grace' }, + dataKeying: 'names', + }) + }) + + it('hands the provenance envelope over unresolved rather than interpreting it', async () => { + await PATCH(bodyRequest('PATCH', patchBody), routeContext()) + + expect(mocks.updateRow.mock.calls[0][0].input.secretProvenanceEnvelope).toEqual({ + kind: 'none', + }) + }) + + it('forwards the originating tab so that tab can skip its own refetch', async () => { + await PATCH(bodyRequest('PATCH', patchBody, { 'x-sim-client-id': 'tab-42' }), routeContext()) + + expect(mocks.updateRow.mock.calls[0][0].input.actorClientId).toBe('tab-42') + }) + + it('projects a classified orchestration failure instead of a generic 500', async () => { + mocks.updateRow.mockRejectedValue(new OrchestrationError('conflict', 'Row changed')) + + const response = await PATCH(bodyRequest('PATCH', patchBody), routeContext()) + + expect(response.status).toBe(409) + }) +}) + +describe('DELETE /api/table/[tableId]/rows/[rowId]', () => { + const deleteBody = { workspaceId: WORKSPACE_ID } + + it('returns 401 when the caller is not authenticated', async () => { + unauthenticated() + + const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.deleteRow).not.toHaveBeenCalled() + }) + + it('reports a deleted count of one on success', async () => { + const response = await DELETE(bodyRequest('DELETE', deleteBody), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ + success: true, + data: { message: 'Row deleted successfully', deletedCount: 1 }, + }) + }) + + it('forwards the originating tab so that tab can skip its own refetch', async () => { + await DELETE(bodyRequest('DELETE', deleteBody, { 'x-sim-client-id': 'tab-42' }), routeContext()) + + expect(mocks.deleteRow.mock.calls[0][0].input.actorClientId).toBe('tab-42') + }) +}) + +/** + * The wire changes the migration makes on purpose. + * + * Both follow from adopting the shared concealment policy — what the v2 table + * surface already does, and what stops a caller learning whether a table it + * cannot reach exists. Nothing in `hooks/queries/tables.ts` branches on either + * status, which is why they are safe to change. + * + * Note the concealment is narrower than the handler it replaces: the old route + * answered a blanket 403 for every access failure, while this one conceals only + * *cross-tenant* denials and still answers 403 for an in-workspace role denial. + */ +describe('deliberate wire changes', () => { + it('conceals a cross-tenant table as 404, where it used to answer 403', async () => { + mocks.readRow.mockRejectedValue(new NoWorkspaceAccessError()) + + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toMatchObject({ error: 'Table not found' }) + }) + + it('still answers 403 for an in-workspace denial, which is not concealed', async () => { + mocks.readRow.mockRejectedValue(new OrchestrationError('forbidden', 'Insufficient role')) + + const response = await GET(getRequest(), routeContext()) + + expect(response.status).toBe(403) + }) + + it('keeps the lock on a 423 so the client knows which one to clear', async () => { + mocks.updateRow.mockRejectedValue(new TableLockedError('update')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(423) + await expect(response.json()).resolves.toMatchObject({ lock: 'update' }) + }) + + it('answers a generic 500 message where the handler named the operation', async () => { + // The builder has one internal error envelope, shared with ~80 other + // migrated routes. The old per-route text ("Failed to update row") was more + // specific; consistency won, and the client only ever toasts the message. + mocks.updateRow.mockRejectedValue(new Error('boom')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: WORKSPACE_ID, data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(500) + await expect(response.json()).resolves.toMatchObject({ error: 'Internal server error' }) + }) + + it('conceals a mismatched workspace assertion as 404, where it used to answer 400', async () => { + mocks.updateRow.mockRejectedValue(new OrchestrationError('not_found', 'Table not found')) + + const response = await PATCH( + bodyRequest('PATCH', { workspaceId: 'workspace-other', data: { col_aaa: 'x' } }), + routeContext() + ) + + expect(response.status).toBe(404) + }) +}) diff --git a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts index 66ec90870f8..02a5a45cfa4 100644 --- a/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/[rowId]/route.ts @@ -1,274 +1,101 @@ -import { db } from '@sim/db' -import { userTableRows } from '@sim/db/schema' -import { createLogger } from '@sim/logger' -import { and, eq } from 'drizzle-orm' -import { type NextRequest, NextResponse } from 'next/server' import { readClientId } from '@/lib/api/client-id' import { deleteTableRowContract, - getTableQuerySchema, + getTableRowContract, updateTableRowContract, } from '@/lib/api/contracts/tables' -import { isZodError, parseRequest, validationErrorResponse } from '@/lib/api/server/validation' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import type { RowData, TableSchema } from '@/lib/table' -import { updateRow } from '@/lib/table' -import { signalTableRowsChangedByActor } from '@/lib/table/events' -import { performDeleteTableRow } from '@/lib/table/orchestration' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' +import { tableOperations } from '@/lib/table/application/operations' +import { deleteTableRow, readTableRow, updateTableRow } from '@/lib/table/application/rows' +import type { RowData } from '@/lib/table/types' import { - createTableRowsResponse, - createTableWriteProvenanceTargets, - resolveTableWriteSecretProvenance, + finalizeTableRowsProvenance, + negotiateTableRowsProvenance, + readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { rowWireTranslators } from '@/app/api/table/row-wire' -import { - accessError, - checkAccess, - orchestrationErrorResponse, - orchestrationOutcomeErrorResponse, - tableLockErrorResponse, -} from '@/app/api/table/utils' - -const logger = createLogger('TableRowAPI') - -interface RowRouteParams { - params: Promise<{ tableId: string; rowId: string }> -} - -/** GET /api/table/[tableId]/rows/[rowId] - Retrieves a single row. */ -export const GET = withRouteHandler(async (request: NextRequest, { params }: RowRouteParams) => { - const requestId = generateRequestId() - const { tableId, rowId } = await params - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } +import { presentRowForPrincipal, rowKeyingForPrincipal } from '@/app/api/table/row-wire' - const { searchParams } = new URL(request.url) - const validated = getTableQuerySchema.parse({ - workspaceId: searchParams.get('workspaceId'), - }) +export const dynamic = 'force-dynamic' - const result = await checkAccess(tableId, authResult.userId, 'read') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const [row] = await db - .select({ - id: userTableRows.id, - data: userTableRows.data, - position: userTableRows.position, - createdAt: userTableRows.createdAt, - updatedAt: userTableRows.updatedAt, - }) - .from(userTableRows) - .where( - and( - eq(userTableRows.id, rowId), - eq(userTableRows.tableId, tableId), - eq(userTableRows.workspaceId, validated.workspaceId) - ) - ) - .limit(1) - - if (!row) { - return NextResponse.json({ error: 'Row not found' }, { status: 404 }) - } - - logger.info(`[${requestId}] Retrieved row ${rowId} from table ${tableId}`) - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - - const responseBody = { - success: true, - data: { - row: { - id: row.id, - data: wire.dataOut(row.data as RowData), - position: row.position, - createdAt: - row.createdAt instanceof Date ? row.createdAt.toISOString() : String(row.createdAt), - updatedAt: - row.updatedAt instanceof Date ? row.updatedAt.toISOString() : String(row.updatedAt), - }, - }, - } - return createTableRowsResponse({ - request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [{ ...row, data: row.data as RowData }], - }) - } catch (error) { - if (isZodError(error)) { - return validationErrorResponse(error) - } - - logger.error(`[${requestId}] Error getting row:`, error) - return NextResponse.json({ error: 'Failed to get row' }, { status: 500 }) - } +const rateLimit = internalRateLimits.none({ + reason: 'Preserve existing internal single-row table behavior', }) -/** PATCH /api/table/[tableId]/rows/[rowId] - Updates a single row (supports partial updates). */ -export const PATCH = withRouteHandler(async (request: NextRequest, context: RowRouteParams) => { - const requestId = generateRequestId() - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const parsed = await parseRequest(updateTableRowContract, request, context, { - validationErrorResponse: (error) => validationErrorResponse(error), - }) - if (!parsed.success) return parsed.response - - const { tableId, rowId } = parsed.data.params - const validated = parsed.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - const rowData = validated.data as RowData - const provenance = resolveTableWriteSecretProvenance({ - request, - payload: validated, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - targets: createTableWriteProvenanceTargets([rowData], wire.dataIn), - rowKeys: ['0'], - }) - if (!provenance.success) return provenance.response - const updatedRow = await updateRow( - { - tableId, - rowId, - data: wire.dataIn(rowData), - workspaceId: validated.workspaceId, - actorUserId: authResult.userId, - secretProvenance: provenance.provenanceByRowKey?.['0'], - }, - table, - requestId - ) - - // Live-collab: tell open viewers the change landed so they refetch. - signalTableRowsChangedByActor(tableId, readClientId(request)) - // 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. - if (!updatedRow) throw new Error('updateRow returned null without a cancellationGuard') - // Auto-dispatch for user edits is handled inside `updateRow` (mode: 'new'). - // Firing a second mode: 'incomplete' dispatch here would race with the - // `mode: 'new'` one AND bulk-clear sibling-group outputs (the incomplete - // bulk-clear wipes ALL targeted columns when any one column on the row - // is empty). - - const responseBody = { - success: true, - data: { - row: { - id: updatedRow.id, - data: wire.dataOut(updatedRow.data), - position: updatedRow.position, - createdAt: - updatedRow.createdAt instanceof Date - ? updatedRow.createdAt.toISOString() - : updatedRow.createdAt, - updatedAt: - updatedRow.updatedAt instanceof Date - ? updatedRow.updatedAt.toISOString() - : updatedRow.updatedAt, - }, - message: 'Row updated successfully', - }, - } - return createTableRowsResponse({ +export const GET = defineInternalJsonRoute({ + contract: getTableRowContract, + operation: tableOperations.readRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, query }, { principal, request }) => ({ + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: query.workspaceId, + includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [updatedRow], - }) - } catch (error) { - const response = orchestrationErrorResponse(error) - if (response) return response - - logger.error(`[${requestId}] Error updating row:`, error) - return NextResponse.json({ error: 'Failed to update row' }, { status: 500 }) - } + principal.kind !== 'session' + ), + }), + useCase: readTableRow, + present: ({ table, row }, { principal }) => ({ + success: true as const, + data: { row: presentRowForPrincipal(row, table.schema, principal) }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), }) -/** DELETE /api/table/[tableId]/rows/[rowId] - Deletes a single row. */ -export const DELETE = withRouteHandler(async (request: NextRequest, context: RowRouteParams) => { - const requestId = generateRequestId() - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) +export const PATCH = defineInternalJsonRoute({ + contract: updateTableRowContract, + operation: tableOperations.updateRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { principal, request }) => { + return { + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: body.workspaceId, + data: body.data as RowData, + dataKeying: rowKeyingForPrincipal(principal), + strictWrite: false, + // Handed over unresolved: interpreting the selections needs the canonical + // schema, which this adapter must not load. + secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), + includePersistedSecretProvenance: negotiateTableRowsProvenance( + request, + principal.kind !== 'session' + ), + actorClientId: readClientId(request), } + }, + useCase: updateTableRow, + present: ({ table, row }, { principal }) => ({ + success: true as const, + data: { + row: presentRowForPrincipal(row, table.schema, principal), + message: 'Row updated successfully', + }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), +}) - const parsed = await parseRequest(deleteTableRowContract, request, context, { - validationErrorResponse: (error) => validationErrorResponse(error), - }) - if (!parsed.success) return parsed.response - - const { tableId, rowId } = parsed.data.params - const validated = parsed.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const outcome = await performDeleteTableRow({ table, rowId, requestId }) - if (!outcome.success) { - return orchestrationOutcomeErrorResponse(outcome, 'Failed to delete row') - } - - // Live-collab: tell open viewers the change landed so they refetch. - signalTableRowsChangedByActor(tableId, readClientId(request)) - - return NextResponse.json({ - success: true, - data: { - message: 'Row deleted successfully', - deletedCount: 1, - }, - }) - } catch (error) { - const lockError = tableLockErrorResponse(error) - if (lockError) return lockError - - const classified = orchestrationErrorResponse(error) - if (classified) return classified - - logger.error(`[${requestId}] Error deleting row:`, error) - return NextResponse.json({ error: 'Failed to delete row' }, { status: 500 }) - } +export const DELETE = defineInternalJsonRoute({ + contract: deleteTableRowContract, + operation: tableOperations.deleteRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit, + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { request }) => ({ + tableId: params.tableId, + rowId: params.rowId, + assertedWorkspaceId: body.workspaceId, + actorClientId: readClientId(request), + }), + useCase: deleteTableRow, + present: () => ({ + success: true as const, + data: { message: 'Row deleted successfully', deletedCount: 1 }, + }), }) diff --git a/apps/sim/app/api/table/[tableId]/rows/find/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/find/route.test.ts index 4b892a39340..26ba08d9742 100644 --- a/apps/sim/app/api/table/[tableId]/rows/find/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/find/route.test.ts @@ -1,10 +1,9 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { createTableDefinition, hybridAuthMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, mockFindRowMatches, mockTableFilterError } = vi.hoisted(() => ({ mockCheckAccess: vi.fn(), @@ -31,24 +30,6 @@ vi.mock('@/lib/table/rows/service', () => ({ import { GET } from '@/app/api/table/[tableId]/rows/find/route' -function buildTable(overrides: Partial = {}): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { columns: [{ name: 'name', type: 'string' }] }, - metadata: null, - rowCount: 0, - maxRows: 100, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date('2024-01-01'), - updatedAt: new Date('2024-01-01'), - ...overrides, - } -} - function callGet( query: Record, { tableId }: { tableId: string } = { tableId: 'tbl_1' } @@ -68,7 +49,15 @@ describe('GET /api/table/[tableId]/rows/find', () => { userId: 'user-1', authType: 'session', }) - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ + ok: true, + table: createTableDefinition({ + columns: [{ name: 'name', type: 'string' }], + maxRows: 100, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), + }), + }) mockTableFilterError.mockReturnValue(null) mockFindRowMatches.mockResolvedValue({ matches: [{ ordinal: 4, rowId: 'row_4', column: 'name' }], diff --git a/apps/sim/app/api/table/[tableId]/rows/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/route.test.ts index 0711127e7a4..b277f2eb83c 100644 --- a/apps/sim/app/api/table/[tableId]/rows/route.test.ts +++ b/apps/sim/app/api/table/[tableId]/rows/route.test.ts @@ -1,10 +1,13 @@ /** * @vitest-environment node */ -import { hybridAuthMockFns } from '@sim/testing' +import { + createTableDefinition, + hybridAuthMockFns, + type TableDefinitionFactoryOptions, +} from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { TableDefinition } from '@/lib/table' const { mockCheckAccess, @@ -59,26 +62,14 @@ vi.mock('@/lib/table/sql', () => ({ import { DELETE, GET, POST, PUT } from '@/app/api/table/[tableId]/rows/route' -function buildTable(): TableDefinition { - return { - id: 'tbl_1', - name: 'People', - description: null, - schema: { - columns: [ - { id: 'col_aaa', name: 'Name', type: 'string' }, - { id: 'col_bbb', name: 'Age', type: 'number' }, - ], - }, - metadata: null, - rowCount: 0, - maxRows: 100, - workspaceId: 'workspace-1', - createdBy: 'user-1', - archivedAt: null, - createdAt: new Date('2024-01-01'), - updatedAt: new Date('2024-01-01'), - } +const TABLE_FIXTURE: TableDefinitionFactoryOptions = { + columns: [ + { id: 'col_aaa', name: 'Name', type: 'string' }, + { id: 'col_bbb', name: 'Age', type: 'number' }, + ], + maxRows: 100, + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), } function authAs(authType: 'session' | 'internal_jwt') { @@ -109,7 +100,7 @@ function callGet(query: Record) { describe('POST /api/table/[tableId]/rows', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockValidateRowData.mockResolvedValue({ valid: true }) mockInsertRow.mockResolvedValue({ id: 'row_1', @@ -166,7 +157,7 @@ describe('POST /api/table/[tableId]/rows', () => { describe('GET /api/table/[tableId]/rows', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockQueryRows.mockResolvedValue({ rows: [ { @@ -313,7 +304,7 @@ describe('GET /api/table/[tableId]/rows', () => { describe('PUT/DELETE /api/table/[tableId]/rows — predicate filters', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckAccess.mockResolvedValue({ ok: true, table: buildTable() }) + mockCheckAccess.mockResolvedValue({ ok: true, table: createTableDefinition(TABLE_FIXTURE) }) mockUpdateRowsByFilter.mockResolvedValue({ affectedCount: 1, affectedRowIds: ['row_1'] }) mockDeleteRowsByFilter.mockResolvedValue({ affectedCount: 1, affectedRowIds: ['row_1'] }) }) diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts new file mode 100644 index 00000000000..2d07bd5e0d0 --- /dev/null +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.test.ts @@ -0,0 +1,148 @@ +/** + * @vitest-environment node + * + * The upsert surface had no route-level tests. These pin what it emits now that + * it runs on the shared internal route builder, including the dual-caller wire + * keying and the provenance envelope being handed over unresolved. + */ +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { upsertRow: vi.fn(), authenticate: vi.fn() }, +})) + +vi.mock('@/lib/table/application/rows', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + upsertTableRow: { operation: { id: 'tables.rows.upsert' }, execute: mocks.upsertRow }, + } +}) + +vi.mock('@/lib/table/api', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, internalTableSessionOrExecutorAuth: { authenticate: mocks.authenticate } } +}) + +import { InternalUnauthenticatedError } from '@/lib/api/server/routes' +import { POST } from '@/app/api/table/[tableId]/rows/upsert/route' + +const TABLE_ID = 'tbl_1' +const WORKSPACE_ID = 'workspace-1' +const CREATED_AT = new Date('2024-01-01T00:00:00.000Z') +const UPDATED_AT = new Date('2024-02-02T00:00:00.000Z') + +const TABLE = { + id: TABLE_ID, + workspaceId: WORKSPACE_ID, + schema: { columns: [{ id: 'col_aaa', name: 'Name', type: 'string' as const }] }, +} +const ROW = { + id: 'row_1', + data: { col_aaa: 'Ada' }, + executions: {}, + position: 0, + createdAt: CREATED_AT, + updatedAt: UPDATED_AT, +} + +function routeContext() { + return { params: Promise.resolve({ tableId: TABLE_ID }) } +} + +function request(body: unknown) { + return new NextRequest(`http://localhost/api/table/${TABLE_ID}/rows/upsert`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }) +} + +const BODY = { workspaceId: WORKSPACE_ID, data: { col_aaa: 'Ada' }, conflictTarget: 'col_aaa' } + +beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue({ + kind: 'session', + userId: 'user-1', + sessionId: 'session-1', + }) + mocks.upsertRow.mockResolvedValue({ table: TABLE, row: ROW, operation: 'insert' }) +}) + +describe('POST /api/table/[tableId]/rows/upsert', () => { + it('returns 401 when the caller is not authenticated', async () => { + mocks.authenticate.mockRejectedValue(new InternalUnauthenticatedError()) + + const response = await POST(request(BODY), routeContext()) + + expect(response.status).toBe(401) + expect(mocks.upsertRow).not.toHaveBeenCalled() + }) + + it('names the operation it performed in the body and the message', async () => { + const response = await POST(request(BODY), routeContext()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ + success: true, + data: { + row: { + id: 'row_1', + data: { col_aaa: 'Ada' }, + position: 0, + createdAt: CREATED_AT.toISOString(), + updatedAt: UPDATED_AT.toISOString(), + }, + operation: 'insert', + message: 'Row inserted successfully', + }, + }) + }) + + it('says updated when the row already existed', async () => { + mocks.upsertRow.mockResolvedValue({ table: TABLE, row: ROW, operation: 'update' }) + + const body = await (await POST(request(BODY), routeContext())).json() + + expect(body.data.message).toBe('Row updated successfully') + }) + + it('tells the use case a session speaks column ids and forwards the conflict target', async () => { + await POST(request(BODY), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input).toMatchObject({ + tableId: TABLE_ID, + assertedWorkspaceId: WORKSPACE_ID, + dataKeying: 'ids', + strictWrite: false, + conflictTarget: 'col_aaa', + }) + }) + + it('tells the use case a workflow execution speaks column names', async () => { + mocks.authenticate.mockResolvedValue({ + kind: 'delegated', + serviceId: 'executor', + subjectUserId: 'user-1', + workspaceId: WORKSPACE_ID, + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), + }) + + await POST(request({ ...BODY, data: { Name: 'Ada' }, conflictTarget: 'Name' }), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input).toMatchObject({ dataKeying: 'names' }) + }) + + it('hands the provenance envelope over unresolved rather than interpreting it', async () => { + await POST(request(BODY), routeContext()) + + expect(mocks.upsertRow.mock.calls[0][0].input.secretProvenanceEnvelope).toEqual({ + kind: 'none', + }) + }) +}) diff --git a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts index abfbc7e0384..d34559cb66f 100644 --- a/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/table/[tableId]/rows/upsert/route.ts @@ -1,114 +1,53 @@ -import { createLogger } from '@sim/logger' -import { type NextRequest, NextResponse } from 'next/server' import { upsertTableRowContract } from '@/lib/api/contracts/tables' -import { parseRequest } from '@/lib/api/server' -import { isZodError, validationErrorResponse } from '@/lib/api/server/validation' -import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' -import { generateRequestId } from '@/lib/core/utils/request' -import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import type { RowData, TableSchema } from '@/lib/table' -import { upsertRow } from '@/lib/table' -import { signalTableRowsChanged } from '@/lib/table/events' +import { defineInternalJsonRoute, internalRateLimits } from '@/lib/api/server/routes' +import { internalTableSessionOrExecutorAuth } from '@/lib/table/api' +import { internalTableRowsErrorPolicy } from '@/lib/table/api/row-route-policies' +import { tableOperations } from '@/lib/table/application/operations' +import { upsertTableRow } from '@/lib/table/application/rows' +import type { RowData } from '@/lib/table/types' import { - createTableRowsResponse, - createTableWriteProvenanceTargets, - resolveTableWriteSecretProvenance, + finalizeTableRowsProvenance, + negotiateTableRowsProvenance, + readTableRowProvenanceEnvelope, } from '@/app/api/table/row-secret-provenance' -import { rowWireTranslators } from '@/app/api/table/row-wire' -import { accessError, checkAccess, orchestrationErrorResponse } from '@/app/api/table/utils' - -const logger = createLogger('TableUpsertAPI') - -interface UpsertRouteParams { - params: Promise<{ tableId: string }> -} - -/** POST /api/table/[tableId]/rows/upsert - Inserts or updates based on unique columns. */ -export const POST = withRouteHandler(async (request: NextRequest, context: UpsertRouteParams) => { - const requestId = generateRequestId() - const { tableId } = await context.params - - try { - const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false }) - if (!authResult.success || !authResult.userId) { - return NextResponse.json({ error: 'Authentication required' }, { status: 401 }) - } - - const validation = await parseRequest(upsertTableRowContract, request, context) - if (!validation.success) return validation.response - const validated = validation.data.body - - const result = await checkAccess(tableId, authResult.userId, 'write') - if (!result.ok) return accessError(result, requestId, tableId) - - const { table } = result - - if (table.workspaceId !== validated.workspaceId) { - return NextResponse.json({ error: 'Invalid workspace ID' }, { status: 400 }) - } - - const wire = rowWireTranslators(authResult.authType, table.schema as TableSchema) - const provenance = resolveTableWriteSecretProvenance({ - request, - payload: validated, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - targets: createTableWriteProvenanceTargets([validated.data as RowData], wire.dataIn), - rowKeys: ['0'], - }) - if (!provenance.success) return provenance.response - // conflictTarget passes through untranslated — upsertRow resolves it id-or-name. - const upsertResult = await upsertRow( - { - tableId, - workspaceId: validated.workspaceId, - data: wire.dataIn(validated.data as RowData), - userId: authResult.userId, - conflictTarget: validated.conflictTarget, - secretProvenance: provenance.provenanceByRowKey?.['0'], - }, - table, - requestId - ) - signalTableRowsChanged(tableId) - - const responseBody = { - success: true, - data: { - row: { - id: upsertResult.row.id, - data: wire.dataOut(upsertResult.row.data), - createdAt: - upsertResult.row.createdAt instanceof Date - ? upsertResult.row.createdAt.toISOString() - : upsertResult.row.createdAt, - updatedAt: - upsertResult.row.updatedAt instanceof Date - ? upsertResult.row.updatedAt.toISOString() - : upsertResult.row.updatedAt, - }, - operation: upsertResult.operation, - message: `Row ${upsertResult.operation === 'update' ? 'updated' : 'inserted'} successfully`, - }, - } - return createTableRowsResponse({ +import { presentRowForPrincipal, rowKeyingForPrincipal } from '@/app/api/table/row-wire' + +export const dynamic = 'force-dynamic' + +/** POST /api/table/[tableId]/rows/upsert — inserts or updates based on unique columns. */ +export const POST = defineInternalJsonRoute({ + contract: upsertTableRowContract, + operation: tableOperations.upsertRow, + auth: internalTableSessionOrExecutorAuth, + rateLimit: internalRateLimits.none({ + reason: 'Preserve existing internal table upsert behavior', + }), + errorPolicy: internalTableRowsErrorPolicy, + mapInput: ({ params, body }, { principal, request }) => ({ + tableId: params.tableId, + assertedWorkspaceId: body.workspaceId, + data: body.data as RowData, + dataKeying: rowKeyingForPrincipal(principal), + strictWrite: false, + // The conflict target follows the same keying as the data; the use case + // resolves it id-or-name against the canonical schema. + conflictTarget: body.conflictTarget, + // Handed over unresolved: interpreting the selections needs the canonical + // schema, which this adapter must not load. + secretProvenanceEnvelope: readTableRowProvenanceEnvelope(request, body), + includePersistedSecretProvenance: negotiateTableRowsProvenance( request, - authType: authResult.authType, - userId: authResult.userId, - workspaceId: table.workspaceId, - body: responseBody, - rows: [upsertResult.row], - }) - } catch (error) { - if (isZodError(error)) { - return validationErrorResponse(error) - } - - const response = orchestrationErrorResponse(error) - if (response) return response - - logger.error(`[${requestId}] Error upserting row:`, error) - return NextResponse.json({ error: 'Failed to upsert row' }, { status: 500 }) - } + principal.kind !== 'session' + ), + }), + useCase: upsertTableRow, + present: ({ table, row, operation }, { principal }) => ({ + success: true as const, + data: { + row: presentRowForPrincipal(row, table.schema, principal), + operation, + message: `Row ${operation === 'update' ? 'updated' : 'inserted'} successfully`, + }, + }), + finalizeResponse: ({ result }) => finalizeTableRowsProvenance(result.secretProvenance), }) diff --git a/apps/sim/app/api/table/row-secret-provenance.ts b/apps/sim/app/api/table/row-secret-provenance.ts index 9d72ce58990..d455a6617d8 100644 --- a/apps/sim/app/api/table/row-secret-provenance.ts +++ b/apps/sim/app/api/table/row-secret-provenance.ts @@ -10,6 +10,10 @@ import { RESOLVED_SECRET_PROVENANCE_METADATA_V1, serializePrivateToolMetadataResponseEnvelope, } from '@/lib/execution/private-tool-metadata' +import { + type TableRowProvenanceEnvelope, + TableRowProvenanceError, +} from '@/lib/table/application/row-secret-provenance' import { loadTableRowSecretProvenance } from '@/lib/table/rows/secret-provenance' import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' import type { RowData, TableRowSecretProvenanceWrite } from '@/lib/table/types' @@ -195,3 +199,57 @@ export async function createTableRowsResponse(options: { ) return NextResponse.json(envelope.body, { headers: envelope.headers }) } + +/** + * Reads the private provenance envelope off the request. Transport only — the + * selections are interpreted against the canonical schema inside the use case, + * by {@link resolveRowWriteProvenance}. + */ +export function readTableRowProvenanceEnvelope( + request: NextRequest, + payload: unknown +): TableRowProvenanceEnvelope { + const inspection = inspectPrivateSecretProvenanceRequest(request.headers, payload) + if (inspection.status === 'unsupported') return { kind: 'none' } + if (inspection.status !== 'verified') throw new TableRowProvenanceError() + return { kind: 'bundle', value: inspection.value } +} + +/** + * Whether this caller asked for persisted row provenance on the response. + * + * Only an authenticated internal caller that explicitly requested the capability + * gets it; every ordinary UI and API response keeps its existing wire shape. The + * answer feeds the use case's `includePersistedSecretProvenance`, so the load + * itself happens inside the authorized operation rather than in the adapter. + */ +export function negotiateTableRowsProvenance( + request: NextRequest, + isInternalCaller: boolean +): boolean { + const negotiation = negotiatePrivateToolMetadataResponse( + request.headers, + RESOLVED_SECRET_PROVENANCE_METADATA_V1, + isInternalCaller + ) + if (negotiation.status === 'rejected') throw new TableRowProvenanceError() + return negotiation.status !== 'not-requested' +} + +/** + * The response half of the envelope, shaped for a declarative route's + * `finalizeResponse`: one sibling body field and one capability header, added + * only when the use case actually loaded provenance. + */ +export function finalizeTableRowsProvenance(provenance: unknown): { + bodyFields?: Record + headers?: HeadersInit +} { + if (provenance === undefined) return {} + const envelope = serializePrivateToolMetadataResponseEnvelope( + {}, + RESOLVED_SECRET_PROVENANCE_METADATA_V1, + provenance + ) + return { bodyFields: envelope.body, headers: envelope.headers } +} diff --git a/apps/sim/app/api/table/row-wire.ts b/apps/sim/app/api/table/row-wire.ts index f4c0a85d988..b20121a621e 100644 --- a/apps/sim/app/api/table/row-wire.ts +++ b/apps/sim/app/api/table/row-wire.ts @@ -1,5 +1,15 @@ +import type { SessionPrincipal, WorkflowExecutionDelegatedPrincipal } from '@sim/auth/principal' import { AuthType, type AuthTypeValue } from '@/lib/auth/hybrid' -import type { Filter, RowData, Sort, SortSpec, TablePredicate, TableSchema } from '@/lib/table' +import type { + Filter, + RowData, + Sort, + SortSpec, + TablePredicate, + TableRow, + TableSchema, +} from '@/lib/table' +import type { TableRowDataKeying } from '@/lib/table/application/rows' import { namedRowMapper } from '@/lib/table/cell-format' import { buildIdByName, @@ -9,6 +19,7 @@ import { sortSpecNamesToIds, } from '@/lib/table/column-keys' import { predicateToStorage, resolveFilterSelectValues } from '@/lib/table/select-values' +import { toWireTimestamp } from '@/lib/table/wire' export interface RowWireTranslators { /** Inbound row data: wire keys → storage column ids. */ @@ -61,3 +72,55 @@ export function rowWireTranslators( sortSpecIn: (sort) => sortSpecNamesToIds(sort, idByName), } } + +/** + * The principal kinds the internal table row routes admit — the auth policy + * yields exactly these two. Typed as the union rather than `Principal` so a + * third kind becomes an exhaustiveness error here instead of silently taking + * the name-keyed branch, which would drop every id-keyed cell of a write and + * report success. + */ +type TableRowRoutePrincipal = SessionPrincipal | WorkflowExecutionDelegatedPrincipal + +/** + * The internal table routes serve two caller kinds on the same paths, and they + * speak different column keyings: the first-party grid holds the schema it + * rendered and addresses cells by stable id, while a workflow tool execution + * speaks column names, because names are what tool enrichment surfaces to the + * model. Keying is therefore a property of the caller, not of the endpoint. + */ +export function rowKeyingForPrincipal(principal: TableRowRoutePrincipal): TableRowDataKeying { + switch (principal.kind) { + case 'session': + return 'ids' + case 'delegated': + return 'names' + } +} + +/** + * One row in the narrower projection the single-row and upsert routes return: + * the stored cells in the caller's keying, plus position, with timestamps + * already serialized. See `tableRowWireSchema`, which is its contract. + */ +export function presentRowForPrincipal( + row: Pick, + schema: TableSchema, + principal: TableRowRoutePrincipal +) { + // Only the outbound mapper is needed here; building the full translator set + // would also index the schema name→id for inbound paths a presenter cannot reach. + const dataOut = + rowKeyingForPrincipal(principal) === 'names' ? namedRowMapper(schema.columns) : identity + return { + id: row.id, + data: dataOut(row.data), + position: row.position, + createdAt: toWireTimestamp(row.createdAt), + updatedAt: toWireTimestamp(row.updatedAt), + } +} + +function identity(value: T): T { + return value +} diff --git a/apps/sim/app/api/table/table-tool-auth.test.ts b/apps/sim/app/api/table/table-tool-auth.test.ts new file mode 100644 index 00000000000..9f2a6a3bede --- /dev/null +++ b/apps/sim/app/api/table/table-tool-auth.test.ts @@ -0,0 +1,45 @@ +/** + * @vitest-environment node + * + * The executor reaches the internal table row routes through the Table block's + * tools, and those routes now authenticate with the delegation policy rather + * than the legacy internal token. Two things have to line up for that to work, + * and neither is visible to a route test that mocks the auth policy: + * + * 1. the tool must ask the executor to mint a delegation token, and + * 2. the operation's policy must admit the `executor` delegated service. + * + * Both are pinned here because getting either wrong fails every workflow call + * to these endpoints — the first as a 401, the second as a 403 — while every + * route-level test keeps passing. + */ +import { describe, expect, it } from 'vitest' +import { tableOperations } from '@/lib/table/application/operations' +import { tableDeleteRowTool } from '@/tools/table/delete_row' +import { tableGetRowTool } from '@/tools/table/get_row' +import { tableUpdateRowTool } from '@/tools/table/update_row' +import { tableUpsertRowTool } from '@/tools/table/upsert_row' + +/** Tool → the operation its route runs under. */ +const EXECUTOR_ROW_TOOLS = [ + ['table_get_row', tableGetRowTool, tableOperations.readRow], + ['table_update_row', tableUpdateRowTool, tableOperations.updateRow], + ['table_delete_row', tableDeleteRowTool, tableOperations.deleteRow], + ['table_upsert_row', tableUpsertRowTool, tableOperations.upsertRow], +] as const + +describe('executor access to the migrated table row routes', () => { + it.each(EXECUTOR_ROW_TOOLS)('%s asks the executor for a delegation token', (_name, tool) => { + // Without this the executor mints a legacy internal token, which the + // delegation policy rejects outright. + expect(tool.request.internalAuth).toBe('executor_delegation') + }) + + it.each(EXECUTOR_ROW_TOOLS)( + '%s runs under an operation that admits the executor', + (_name, _tool, operation) => { + expect(operation.delegatedServices).toContain('executor') + expect(operation.principalKinds).toContain('delegated') + } + ) +}) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts index 387473a3935..08e8a510620 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.test.ts @@ -132,6 +132,7 @@ describe('/api/v2/tables/[tableId]/rows/[rowId]', () => { assertedWorkspaceId: WORKSPACE_ID, data: { name: 'Ada' }, strictWrite: true, + dataKeying: 'names', }, request: req, }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts index 465577e099a..fe3ce419375 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/[rowId]/route.ts @@ -42,6 +42,7 @@ export const PATCH = defineV2JsonRoute({ assertedWorkspaceId: body.workspaceId, data: body.data, strictWrite: true, + dataKeying: 'names' as const, }), useCase: updateTableRow, present: ({ table, row }) => ({ diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts index 1f0b8a3b5e7..bb5ccea0a76 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/route.test.ts @@ -190,6 +190,7 @@ describe('/api/v2/tables/[tableId]/rows', () => { // or a value the column cannot hold is a 400, not a dropped key or a // nulled cell. Every first-party surface leaves this unset. strictWrite: true, + dataKeying: 'names', }, request: single, }) @@ -207,6 +208,7 @@ describe('/api/v2/tables/[tableId]/rows', () => { assertedWorkspaceId: WORKSPACE_ID, rows: [{ name: 'Ada' }], strictWrite: true, + dataKeying: 'names', }, request: batch, }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts index 4727a7bba6a..1c5382064d8 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/route.ts @@ -71,6 +71,7 @@ export const POST = defineV2JsonRoute({ assertedWorkspaceId: body.workspaceId, rows: body.rows, strictWrite: true, + dataKeying: 'names' as const, } : { kind: 'single' as const, @@ -80,6 +81,7 @@ export const POST = defineV2JsonRoute({ afterRowId: body.afterRowId, beforeRowId: body.beforeRowId, strictWrite: true, + dataKeying: 'names' as const, }, useCase: createTableRows, present: (result) => { @@ -108,6 +110,7 @@ export const PATCH = defineV2JsonRoute({ data: body.data, limit: body.limit, strictWrite: true, + dataKeying: 'names' as const, }), useCase: updateTableRows, present: ({ affectedCount, affectedRowIds }) => ({ diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.test.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.test.ts index e17f2760632..29451b0db62 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.test.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.test.ts @@ -103,6 +103,7 @@ describe('POST /api/v2/tables/[tableId]/rows/upsert', () => { data: { email: 'ada@example.com' }, conflictTarget: 'email', strictWrite: true, + dataKeying: 'names', }, request, }) diff --git a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts index 26550374d43..ff66079c7d5 100644 --- a/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts +++ b/apps/sim/app/api/v2/tables/[tableId]/rows/upsert/route.ts @@ -21,6 +21,7 @@ export const POST = defineV2JsonRoute({ data: body.data, conflictTarget: body.conflictTarget, strictWrite: true, + dataKeying: 'names' as const, }), useCase: upsertTableRow, present: ({ table, row, operation }) => ({ diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts index 0ed44ed4543..e5eeefac94e 100644 --- a/apps/sim/lib/api/contracts/tables.ts +++ b/apps/sim/lib/api/contracts/tables.ts @@ -350,6 +350,25 @@ export const rowDataSchema = domainObjectSchema() export const tableDefinitionSchema = domainObjectSchema() export const tableRowSchema = domainObjectSchema() +/** + * One row as the single-row routes actually emit it: the stored cells plus + * position, with timestamps already serialized. + * + * Deliberately not {@link tableRowSchema}. That one describes a `TableRow`, + * which carries the per-cell `executions` sidecar and `Date` objects — accurate + * for the list and query routes, which return exactly that, and wrong for the + * single-row routes, which have always projected a narrower object with ISO + * strings. Two shapes on the wire need two schemas; collapsing them would make + * one of the two lie to its clients. + */ +export const tableRowWireSchema = z.object({ + id: z.string(), + data: rowDataSchema, + position: z.number(), + createdAt: z.string(), + updatedAt: z.string(), +}) + /** * Plain-object base for the single-row insert body. Kept un-refined so callers * (e.g. the v1 public contract) can `.omit()` fields before applying @@ -1425,7 +1444,7 @@ export const upsertTableRowContract = defineRouteContract({ mode: 'json', schema: successResponseSchema( z.object({ - row: tableRowSchema, + row: tableRowWireSchema, operation: z.enum(['insert', 'update']), message: z.string(), }) @@ -1433,6 +1452,22 @@ export const upsertTableRowContract = defineRouteContract({ }, }) +/** + * Reads one row. The sibling of {@link updateTableRowContract} and + * {@link deleteTableRowContract}, which take their workspace scope from a body; + * a GET has none, so it is asserted on the query string instead. + */ +export const getTableRowContract = defineRouteContract({ + method: 'GET', + path: '/api/table/[tableId]/rows/[rowId]', + params: tableRowParamsSchema, + query: getTableQuerySchema, + response: { + mode: 'json', + schema: successResponseSchema(z.object({ row: tableRowWireSchema })), + }, +}) + export const updateTableRowContract = defineRouteContract({ method: 'PATCH', path: '/api/table/[tableId]/rows/[rowId]', @@ -1442,7 +1477,7 @@ export const updateTableRowContract = defineRouteContract({ mode: 'json', schema: successResponseSchema( z.object({ - row: tableRowSchema, + row: tableRowWireSchema, message: z.string(), }) ), diff --git a/apps/sim/lib/api/list-convention.test.ts b/apps/sim/lib/api/list-convention.test.ts index 77453b972c1..255f6edd28b 100644 --- a/apps/sim/lib/api/list-convention.test.ts +++ b/apps/sim/lib/api/list-convention.test.ts @@ -37,7 +37,8 @@ vi.mock('@/lib/table/billing', () => ({ })) vi.mock('@/lib/table/jobs/service', () => ({ EMPTY_JOB_FIELDS: {}, - latestJobForTable: vi.fn(async () => null), + latestNonExportJobJson: vi.fn(() => null), + mapJobRow: vi.fn(() => ({})), latestJobsForTables: vi.fn(async () => new Map()), })) vi.mock('@/lib/table/events', () => ({ appendTableEvent: vi.fn() })) diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index 8e362c35f66..ae198a2684d 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -199,15 +199,33 @@ type InternalJsonParseOptions = Pick< 'maxBodyBytes' | 'validationErrorResponse' > -type InternalJsonPresenter = [R] extends [ - ContractJsonResponse, -] - ? { - present?(result: NoInfer): ContractJsonResponse | Promise> - } - : { - present(result: NoInfer): ContractJsonResponse | Promise> - } +/** + * What a presenter may render from, beyond the use case's result. + * + * A surface that serves more than one caller kind can owe them different wire + * shapes for the same domain result — the internal table row routes answer a + * session in stable column ids and a workflow execution in column names. That is + * presentation, not domain, so it belongs in the adapter rather than the use + * case. {@link InternalJsonRouteOptions.responseHeaders} and + * {@link InternalJsonRouteOptions.finalizeResponse} already receive this pair; + * this closes the same gap for `present`. + */ +export interface InternalJsonPresenterContext { + principal: P + input: I +} + +type InternalJsonPresentFn = ( + result: NoInfer, + context: InternalJsonPresenterContext, NoInfer

> +) => ContractJsonResponse | Promise> + +/** The presenter is optional exactly when the result already is the response body. */ +type InternalJsonPresenter = [ + R, +] extends [ContractJsonResponse] + ? { present?: InternalJsonPresentFn } + : { present: InternalJsonPresentFn } type InternalJsonRouteOptions< C extends JsonApiRouteContract, @@ -239,7 +257,7 @@ type InternalJsonRouteOptions< result: NoInfer body: ContractJsonResponse }): InternalJsonResponseFinalization | Promise -} & InternalJsonPresenter +} & InternalJsonPresenter function createJsonErrorResponse(descriptor: JsonErrorResponseDescriptor): NextResponse { return NextResponse.json(withRequestId(descriptor.body), { @@ -333,7 +351,7 @@ export function defineInternalJsonRoute< request, }) await options.onSuccess?.({ principal, input, result }) - const body = options.present ? await options.present(result) : result + const body = options.present ? await options.present(result, { principal, input }) : result const responseSchema = options.contract.response if (responseSchema.mode !== 'json') { throw new Error('Internal JSON route response mode changed after initialization') diff --git a/apps/sim/lib/copilot/tools/server/table/user-table.ts b/apps/sim/lib/copilot/tools/server/table/user-table.ts index da3fee1d8ce..c43fca7fafc 100644 --- a/apps/sim/lib/copilot/tools/server/table/user-table.ts +++ b/apps/sim/lib/copilot/tools/server/table/user-table.ts @@ -286,6 +286,7 @@ export const userTableServerTool: BaseServerTool tableId: args.tableId, assertedWorkspaceId: workspaceId, strictWrite: false, + dataKeying: 'names', data: args.data, position: args.position as number | undefined, secretProvenance: createExactEmptyTableRowSecretProvenance(args.data), @@ -329,6 +330,7 @@ export const userTableServerTool: BaseServerTool tableId: args.tableId, assertedWorkspaceId: workspaceId, strictWrite: false, + dataKeying: 'names', rows: sourceRows, secretProvenance: sourceRows.map(createExactEmptyTableRowSecretProvenance), }, @@ -476,6 +478,7 @@ export const userTableServerTool: BaseServerTool tableId: args.tableId, assertedWorkspaceId: workspaceId, strictWrite: false, + dataKeying: 'names', rowId: args.rowId, data: args.data, secretProvenance: createExactEmptyTableRowSecretProvenance(args.data), diff --git a/apps/sim/lib/table/__tests__/update-row.test.ts b/apps/sim/lib/table/__tests__/update-row.test.ts index cc2ed28df81..f2027de5445 100644 --- a/apps/sim/lib/table/__tests__/update-row.test.ts +++ b/apps/sim/lib/table/__tests__/update-row.test.ts @@ -8,13 +8,15 @@ import { deleteColumn, renameColumn } from '@/lib/table/columns/service' import { batchInsertRows, batchUpdateRows, + getRowById, + getRowSummaryById, insertRow, replaceTableRows, updateRow, upsertRow, } from '@/lib/table/rows/service' import type { TableDefinition } from '@/lib/table/types' -import { getUniqueColumns } from '@/lib/table/validation' +import { checkUniqueConstraintsDb, getUniqueColumns } from '@/lib/table/validation' // Capacity is exercised in billing.test.ts; here it's a no-op so the timeout-scaling // suites can use large synthetic row counts without tripping the plan limit. @@ -548,3 +550,147 @@ describe('batchUpdateRows — per-row partial merge', () => { expect(values).not.toContain(JSON.stringify({ name: 'Alice', age: 31 })) }) }) + +/** + * The uniqueness probe opens its own transaction and queries once per unique + * column, so on a table that has any unique column it used to cost several + * round trips on every edit — including edits nowhere near one. It is now + * scoped to the columns the patch actually writes. + * + * The safety argument is that a merge cannot newly violate uniqueness on a + * column it leaves alone: that value is the one already stored, and it + * satisfied the constraint when it was written. + * + * The one case that does not cover is a unique constraint added to a column + * that already held duplicates — such a row is no longer blocked from edits + * elsewhere in it, which is the intended outcome. + */ +describe('updateRow — uniqueness probe scoping', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + // The common case: one unique column. The two tests that need a different + // shape override this. + vi.mocked(getUniqueColumns).mockReturnValue([{ name: 'name', type: 'string', unique: true }]) + dbChainMockFns.limit.mockResolvedValue([EXISTING_ROW]) + dbChainMockFns.returning.mockResolvedValue([ + { id: EXISTING_ROW.id, updatedAt: PERSISTED_UPDATED_AT }, + ]) + }) + + it('does not probe when the patch touches no unique column', async () => { + await updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { age: 31 }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + + expect(checkUniqueConstraintsDb).not.toHaveBeenCalled() + }) + + it('still probes when the patch touches a unique column', async () => { + await updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { name: 'Grace' }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + + expect(checkUniqueConstraintsDb).toHaveBeenCalledTimes(1) + }) + + it('probes against the merged row, so the excluded row is still the one being edited', async () => { + await updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { name: 'Grace' }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + + // The merged row is what gets probed, the excluded row is the one being + // edited, and the schema is narrowed to the unique columns this patch + // touched — the probe issues one query per column it is handed. + expect(checkUniqueConstraintsDb).toHaveBeenCalledWith( + 'tbl-1', + { name: 'Grace', age: 30 }, + { ...TABLE.schema, columns: [{ name: 'name', type: 'string', unique: true }] }, + 'row-1' + ) + }) + + it('hands the probe only the unique columns the patch touched', async () => { + vi.mocked(getUniqueColumns).mockReturnValue([ + { name: 'name', type: 'string', unique: true }, + { name: 'email', type: 'string', unique: true }, + ]) + + await updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { name: 'Grace' }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + + const schemaArg = vi.mocked(checkUniqueConstraintsDb).mock.calls[0][2] + expect(schemaArg.columns).toEqual([{ name: 'name', type: 'string', unique: true }]) + }) + + it('surfaces a duplicate on a column the patch does write', async () => { + vi.mocked(getUniqueColumns).mockReturnValue([{ name: 'name', type: 'string', unique: true }]) + vi.mocked(checkUniqueConstraintsDb).mockResolvedValueOnce({ + valid: false, + errors: ['Duplicate value for name'], + }) + + await expect( + updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { name: 'Grace' }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + ).rejects.toThrow(/Duplicate value for name/) + }) + + it('does not probe on a table with no unique columns at all', async () => { + vi.mocked(getUniqueColumns).mockReturnValue([]) + + await updateRow( + { tableId: 'tbl-1', rowId: 'row-1', data: { name: 'Grace' }, workspaceId: 'ws-1' }, + TABLE, + 'req-1' + ) + + expect(checkUniqueConstraintsDb).not.toHaveBeenCalled() + }) +}) + +/** + * The read surfaces never put the executions sidecar on the wire, so loading it + * for them is a query whose result is discarded. Two readers rather than a flag: + * a caller that forgets a flag reads an empty sidecar and cannot tell that from + * a row that has none, whereas here the field is not on the type. + */ +describe('row readers', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + dbChainMockFns.limit.mockResolvedValue([EXISTING_ROW]) + }) + + it('getRowSummaryById issues one select and returns no sidecar', async () => { + const row = await getRowSummaryById('tbl-1', 'row-1', 'ws-1') + + expect(row).not.toHaveProperty('executions') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + + it('getRowById issues the extra select the sidecar needs', async () => { + const row = await getRowById('tbl-1', 'row-1', 'ws-1') + + expect(row).toHaveProperty('executions') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(2) + }) + + it('getRowSummaryById returns null for a missing row', async () => { + dbChainMockFns.limit.mockResolvedValue([]) + + await expect(getRowSummaryById('tbl-1', 'nope', 'ws-1')).resolves.toBeNull() + }) +}) diff --git a/apps/sim/lib/table/api/row-route-policies.ts b/apps/sim/lib/table/api/row-route-policies.ts index 08dba0ffdda..4a93c4e74c6 100644 --- a/apps/sim/lib/table/api/row-route-policies.ts +++ b/apps/sim/lib/table/api/row-route-policies.ts @@ -1,5 +1,10 @@ -import type { V2ErrorPolicy } from '@/lib/api/server/routes' -import { v2TableErrorPolicies } from '@/lib/table/api/route-policies' +import { + extendInternalErrorPolicy, + internalErrorResponse, + type V2ErrorPolicy, +} from '@/lib/api/server/routes' +import { internalTableErrorPolicies, v2TableErrorPolicies } from '@/lib/table/api/route-policies' +import { TableRowProvenanceError } from '@/lib/table/application/row-secret-provenance' import { TableRowsValidationError } from '@/lib/table/application/rows' import { v2Error } from '@/app/api/v2/lib/response' @@ -11,3 +16,22 @@ export const v2TableRowsErrorPolicy = { return v2TableErrorPolicies.concealTableAuthorization.render(error) }, } satisfies V2ErrorPolicy + +/** + * Row routes on the internal surface. The internal counterpart of + * {@link v2TableRowsErrorPolicy}: a row-shape complaint and a provenance + * envelope that does not authenticate are both the caller's to fix and answer + * 400; everything else conceals a cross-tenant table behind the same not-found + * wording the rest of the table surface uses. + * + * Built on the lock-aware base so a 423 keeps carrying `lock` — the only field + * that tells a client which lock to clear. A row write is exactly as lockable + * as a group mutation. + */ +export const internalTableRowsErrorPolicy = extendInternalErrorPolicy( + internalTableErrorPolicies.concealTableGroupAuthorization, + (error) => + error instanceof TableRowsValidationError || error instanceof TableRowProvenanceError + ? internalErrorResponse(400, { error: error.message }) + : null +) diff --git a/apps/sim/lib/table/application/context.test.ts b/apps/sim/lib/table/application/context.test.ts index d1b8ff08a66..5a41c4a48cc 100644 --- a/apps/sim/lib/table/application/context.test.ts +++ b/apps/sim/lib/table/application/context.test.ts @@ -16,6 +16,54 @@ vi.mock('@/lib/workspaces/application/workspace-context', () => ({ import { resolveActiveTableContext } from '@/lib/table/application/context' +const WORKSPACE_ONE = { + workspaceId: 'workspace-1', + workspaceOrganizationId: 'organization-1', + allowPersonalApiKeys: true, + billedAccountUserId: 'billing-user-1', +} + +const WORKSPACE_TWO = { + workspaceId: 'workspace-2', + workspaceOrganizationId: 'organization-2', + allowPersonalApiKeys: false, + billedAccountUserId: 'billing-user-2', +} + +/** Runs `body` while capturing any unhandled promise rejection it provokes. */ +async function withUnhandledRejectionWatch(body: () => Promise): Promise { + const seen: unknown[] = [] + const onUnhandled = (reason: unknown) => { + seen.push(reason) + } + process.on('unhandledRejection', onUnhandled) + try { + await body() + await new Promise((resolve) => setImmediate(resolve)) + await new Promise((resolve) => setImmediate(resolve)) + } finally { + process.off('unhandledRejection', onUnhandled) + } + return seen +} + +/** + * Holds the table load open so a test can observe what the resolver does before + * the table arrives. `release` resolves it with the canonical table. + */ +function deferTableLoad(): { release: () => void } { + let releaseTable: (table: unknown) => void = () => {} + getTableById.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseTable = resolve + }) + ) + return { + release: () => releaseTable({ id: 'table-1', workspaceId: 'workspace-1', name: 'Contacts' }), + } +} + describe('table application context', () => { beforeEach(() => { vi.clearAllMocks() @@ -24,12 +72,9 @@ describe('table application context', () => { workspaceId: 'workspace-1', name: 'Contacts', }) - loadWorkspace.mockResolvedValue({ - workspaceId: 'workspace-1', - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-user-1', - }) + loadWorkspace.mockImplementation(async (workspaceId: string) => + workspaceId === 'workspace-1' ? WORKSPACE_ONE : WORKSPACE_TWO + ) }) it('derives workspace scope from the canonical active table', async () => { @@ -44,13 +89,94 @@ describe('table application context', () => { expect(loadWorkspace).toHaveBeenCalledWith('workspace-1') }) - it('conceals an asserted cross-workspace table before workspace resolution', async () => { + it('starts the workspace load without waiting for the table when a workspace is asserted', async () => { + const { release } = deferTableLoad() + + const pending = resolveActiveTableContext({ + tableId: 'table-1', + assertedWorkspaceId: 'workspace-1', + }) + await Promise.resolve() + await Promise.resolve() + + expect(loadWorkspace).toHaveBeenCalledWith('workspace-1') + + release() + await expect(pending).resolves.toMatchObject({ tableId: 'table-1', workspaceId: 'workspace-1' }) + }) + + it('waits for the table before loading a workspace when none is asserted', async () => { + const { release } = deferTableLoad() + + const pending = resolveActiveTableContext({ tableId: 'table-1' }) + await Promise.resolve() + await Promise.resolve() + + expect(loadWorkspace).not.toHaveBeenCalled() + + release() + await expect(pending).resolves.toMatchObject({ tableId: 'table-1', workspaceId: 'workspace-1' }) + expect(loadWorkspace).toHaveBeenCalledWith('workspace-1') + }) + + it('conceals an asserted cross-workspace table as not found', async () => { await expect( resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-2' }) ).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' }) + }) + + it('conceals a table that does not exist at all', async () => { + getTableById.mockResolvedValueOnce(null) + + await expect( + resolveActiveTableContext({ tableId: 'missing', assertedWorkspaceId: 'workspace-1' }) + ).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' }) + }) + + it('conceals a missing table with no asserted workspace', async () => { + getTableById.mockResolvedValueOnce(null) + + await expect(resolveActiveTableContext({ tableId: 'missing' })).rejects.toMatchObject({ + code: 'not_found', + message: 'Table not found', + }) expect(loadWorkspace).not.toHaveBeenCalled() }) + it('surfaces not_found rather than a failing workspace load on a mismatched assertion', async () => { + const failure = new Error('workspace database unavailable') + loadWorkspace.mockRejectedValueOnce(failure) + + const unhandled = await withUnhandledRejectionWatch(async () => { + await expect( + resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-2' }) + ).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' }) + }) + + expect(unhandled).toEqual([]) + }) + + it('surfaces not_found rather than a failing table load on a matched assertion', async () => { + const failure = new Error('table database unavailable') + getTableById.mockRejectedValueOnce(failure) + + const unhandled = await withUnhandledRejectionWatch(async () => { + await expect( + resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' }) + ).rejects.toBe(failure) + }) + + expect(unhandled).toEqual([]) + }) + + it('refuses a workspace context that is not the canonical workspace of the table', async () => { + loadWorkspace.mockResolvedValueOnce(WORKSPACE_TWO) + + await expect( + resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' }) + ).rejects.toMatchObject({ code: 'not_found', message: 'Table not found' }) + }) + it('fails when the canonical workspace is unavailable', async () => { loadWorkspace.mockResolvedValueOnce(null) @@ -60,10 +186,31 @@ describe('table application context', () => { }) }) + it('fails when the canonical workspace is unavailable on the asserted path', async () => { + loadWorkspace.mockResolvedValueOnce(null) + + await expect( + resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' }) + ).rejects.toMatchObject({ code: 'not_found', message: 'Workspace not found' }) + }) + it('propagates canonical workspace database failures', async () => { const failure = new Error('workspace database unavailable') loadWorkspace.mockRejectedValueOnce(failure) await expect(resolveActiveTableContext({ tableId: 'table-1' })).rejects.toBe(failure) }) + + it('propagates canonical workspace database failures on the asserted path', async () => { + const failure = new Error('workspace database unavailable') + loadWorkspace.mockRejectedValueOnce(failure) + + const unhandled = await withUnhandledRejectionWatch(async () => { + await expect( + resolveActiveTableContext({ tableId: 'table-1', assertedWorkspaceId: 'workspace-1' }) + ).rejects.toBe(failure) + }) + + expect(unhandled).toEqual([]) + }) }) diff --git a/apps/sim/lib/table/application/context.ts b/apps/sim/lib/table/application/context.ts index 71e71de49b9..7bedaa7bfaa 100644 --- a/apps/sim/lib/table/application/context.ts +++ b/apps/sim/lib/table/application/context.ts @@ -32,12 +32,49 @@ async function requireTable(tableId: string, workspaceId: string | undefined) { return table } +/** + * Loads the canonical context a table use case authorizes against. + * + * When the caller asserts a workspace, the workspace load no longer has to wait for the table + * load: the asserted id is already in hand, so both round trips start together. What makes that + * safe is that {@link requireTable} still compares the table's canonical `workspaceId` against + * the assertion and reports any mismatch as `not_found`. The table outcome is inspected first and + * unconditionally, so on every path that returns, the assertion has been *proven* equal to the + * canonical workspace id — the context handed back is the table's own workspace, never a + * workspace the caller merely named. A mismatch throws before the workspace outcome is read, so a + * failing workspace load can never replace the concealing `not_found`. `Promise.allSettled` keeps + * the branch that is thrown away from surfacing as an unhandled rejection. A final identity check + * on the loaded context restates the invariant at the point of return, so the value handed back is + * only ever a context whose own `workspaceId` is the table's. + * + * Without an asserted id there is nothing to start early — the table load is what reveals which + * workspace to load — so that path stays sequential. + */ export async function resolveActiveTableContext(input: { tableId: string assertedWorkspaceId?: string }): Promise { - const table = await requireTable(input.tableId, input.assertedWorkspaceId) - const workspaceContext = await resolveTableWorkspaceContext(table.workspaceId) + const { tableId, assertedWorkspaceId } = input + + if (assertedWorkspaceId === undefined) { + const table = await requireTable(tableId, undefined) + const workspaceContext = await resolveTableWorkspaceContext(table.workspaceId) + return { ...workspaceContext, tableId: table.id, table } + } + + const [tableOutcome, workspaceOutcome] = await Promise.allSettled([ + requireTable(tableId, assertedWorkspaceId), + resolveTableWorkspaceContext(assertedWorkspaceId), + ]) + + if (tableOutcome.status === 'rejected') throw tableOutcome.reason + if (workspaceOutcome.status === 'rejected') throw workspaceOutcome.reason + + const table = tableOutcome.value + const workspaceContext = workspaceOutcome.value + if (workspaceContext.workspaceId !== table.workspaceId) { + throw new OrchestrationError('not_found', 'Table not found') + } return { ...workspaceContext, tableId: table.id, table } } diff --git a/apps/sim/lib/table/application/operations.test.ts b/apps/sim/lib/table/application/operations.test.ts index 3b9dc95cc27..269719d8655 100644 --- a/apps/sim/lib/table/application/operations.test.ts +++ b/apps/sim/lib/table/application/operations.test.ts @@ -65,17 +65,26 @@ describe('table operation registry', () => { tableOperations.cancelExport.id, tableOperations.downloadExport.id, ]) - const sharedGroupOperations = new Set([ + // Reachable from the executor's Table block as well as from Copilot. The + // single-row operations joined this set when their routes moved onto the + // delegation auth policy: the Table block's get/update/delete/upsert row + // tools run under them, and a policy without `executor` fails every one of + // those calls with a 403 while every route test still passes. + const sharedToolOperations = new Set([ tableOperations.createGroup.id, tableOperations.updateGroup.id, tableOperations.deleteGroup.id, + tableOperations.readRow.id, + tableOperations.updateRow.id, + tableOperations.deleteRow.id, + tableOperations.upsertRow.id, ]) for (const operation of Object.values(tableOperations)) { expect(operation.delegatedServices).toEqual( executorOnlyOperations.has(operation.id) ? ['executor'] - : sharedGroupOperations.has(operation.id) + : sharedToolOperations.has(operation.id) ? ['copilot', 'executor'] : ['copilot'] ) diff --git a/apps/sim/lib/table/application/operations.ts b/apps/sim/lib/table/application/operations.ts index 42476b3783a..dfc7fc8e698 100644 --- a/apps/sim/lib/table/application/operations.ts +++ b/apps/sim/lib/table/application/operations.ts @@ -46,6 +46,15 @@ function toolWriteOperation(id: Id) { }) } +function toolReadOperation(id: Id) { + return defineWorkspaceOperation({ + id, + minimumRole: 'read', + workspaceApiKey: 'allow', + ...ALL_TABLE_TOOL_PRINCIPAL_POLICY, + }) +} + function internalExecutorReadOperation(id: Id) { return defineWorkspaceOperation({ id, @@ -104,14 +113,14 @@ export const tableOperations = { listRows: readOperation('tables.rows.list'), queryRows: readOperation('tables.rows.query'), findRows: readOperation('tables.rows.find'), - readRow: readOperation('tables.rows.read'), + readRow: toolReadOperation('tables.rows.read'), createRows: writeOperation('tables.rows.create'), replaceRows: writeOperation('tables.rows.replace'), - updateRow: writeOperation('tables.rows.update'), + updateRow: toolWriteOperation('tables.rows.update'), updateRows: writeOperation('tables.rows.update_many'), - deleteRow: writeOperation('tables.rows.delete'), + deleteRow: toolWriteOperation('tables.rows.delete'), deleteRows: writeOperation('tables.rows.delete_many'), - upsertRow: writeOperation('tables.rows.upsert'), + upsertRow: toolWriteOperation('tables.rows.upsert'), listViews: readOperation('tables.views.list'), readView: readOperation('tables.views.read'), createView: writeOperation('tables.views.create'), diff --git a/apps/sim/lib/table/application/row-secret-provenance.test.ts b/apps/sim/lib/table/application/row-secret-provenance.test.ts new file mode 100644 index 00000000000..c43d7546dfe --- /dev/null +++ b/apps/sim/lib/table/application/row-secret-provenance.test.ts @@ -0,0 +1,216 @@ +/** + * @vitest-environment node + * + * The provenance envelope moved out of the route adapter and into the domain, + * because interpreting a caller's selections requires the canonical schema and + * the adapter must not load it. These pin the semantics that move with it. + */ +import { describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { scopeCompatible: vi.fn(() => true), isBundle: vi.fn(() => true) }, +})) + +vi.mock('@/lib/execution/durable-secret-provenance', () => ({ + isPrivateSecretProvenanceScopeCompatible: mocks.scopeCompatible, +})) + +vi.mock('@/lib/execution/model-input-provenance', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, isPrivateSecretProvenanceBundleV1: mocks.isBundle } +}) + +import { + resolveRowWriteProvenance, + TableRowProvenanceError, +} from '@/lib/table/application/row-secret-provenance' +import type { TableDefinition } from '@/lib/table/types' + +const TABLE = { + id: 'tbl_1', + workspaceId: 'workspace-1', + schema: { + columns: [ + { id: 'col_aaa', name: 'Name', type: 'string' }, + { id: 'col_bbb', name: 'Age', type: 'number' }, + ], + }, +} as unknown as TableDefinition + +const SESSION = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' } +const EXECUTOR = { + kind: 'delegated' as const, + serviceId: 'executor' as const, + subjectUserId: 'user-1', + workspaceId: 'workspace-1', + delegationId: 'delegation-1', + audience: 'table', + issuedAt: new Date('2026-01-01'), + expiresAt: new Date('2026-01-02'), +} + +function resolve(overrides: Partial[0]>) { + return resolveRowWriteProvenance({ + envelope: { kind: 'none' }, + principal: SESSION, + workspaceId: 'workspace-1', + table: TABLE, + keying: 'ids', + wireRows: [{ col_aaa: 'Ada' }], + storageRows: [{ col_aaa: 'Ada' }], + ...overrides, + }) +} + +describe('row write provenance', () => { + it('certifies an interactive write exact-empty over the columns it persists', () => { + const { stamps } = resolve({}) + + expect(stamps).toEqual([ + { complete: true, columns: { col_aaa: { version: 1, complete: true, entries: [] } } }, + ]) + }) + + it('leaves an internal caller that sent no envelope untracked', () => { + // Not exact-empty: stamping "this write introduced no secrets" on a runtime + // write that sent no envelope would be a false certification. + const { stamps } = resolve({ principal: EXECUTOR }) + + expect(stamps).toEqual([undefined]) + }) + + it('refuses a bundle from a session caller', () => { + expect(() => + resolve({ envelope: { kind: 'bundle', value: { complete: true, selections: [] } } }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a bundle that is not a recognised envelope', () => { + mocks.isBundle.mockReturnValueOnce(false) + + expect(() => + resolve({ principal: EXECUTOR, envelope: { kind: 'bundle', value: { nope: true } } }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a complete bundle that does not account for every written cell', () => { + expect(() => + resolve({ + principal: EXECUTOR, + envelope: { kind: 'bundle', value: { complete: true, selections: [] } }, + wireRows: [{ col_aaa: 'Ada', col_bbb: 36 }], + storageRows: [{ col_aaa: 'Ada', col_bbb: 36 }], + }) + ).toThrow(TableRowProvenanceError) + }) + + it('refuses a selection whose scope this principal may not read', () => { + mocks.scopeCompatible.mockReturnValueOnce(false) + + expect(() => + resolve({ + principal: EXECUTOR, + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'col_aaa']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + ).toThrow(TableRowProvenanceError) + }) + + it('marks an incomplete bundle unknown rather than certifying it', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + envelope: { kind: 'bundle', value: { complete: false, selections: [] } }, + }) + + expect(stamps).toEqual([{ complete: false, columns: {} }]) + }) + + it('keys a name-wire selection to the storage column it certifies', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'names', + wireRows: [{ Name: 'Ada' }], + storageRows: [{ col_aaa: 'Ada' }], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'Name']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + + expect(stamps[0]).toEqual({ + complete: true, + columns: { col_aaa: { scope: { kind: 'workspace' } } }, + }) + }) + + it('checks the scope against the acting principal, not a billing owner', () => { + resolve({ + principal: EXECUTOR, + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [{ key: JSON.stringify([0, 'col_aaa']), provenance: { scope: {} } }], + }, + }, + }) + + expect(mocks.scopeCompatible).toHaveBeenCalledWith( + {}, + { userId: 'user-1', workspaceId: 'workspace-1' } + ) + }) + + it('records provenance for an id-keyed key the write persists, recognised or not', () => { + // The id wire stores what it is given, so every key it sends is a storage + // key. Mapping an unrecognised one to null would leave a written cell + // uncertified under a complete stamp. + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'ids', + wireRows: [{ 'col-unknown': 'x' }], + storageRows: [{ 'col-unknown': 'x' }], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [{ key: JSON.stringify([0, 'col-unknown']), provenance: { scope: {} } }], + }, + }, + }) + + expect(stamps[0]).toEqual({ complete: true, columns: { 'col-unknown': { scope: {} } } }) + }) + + it('records nothing for a key that names no column, since it is never stored', () => { + const { stamps } = resolve({ + principal: EXECUTOR, + keying: 'names', + wireRows: [{ Nope: 'x' }], + storageRows: [{}], + envelope: { + kind: 'bundle', + value: { + complete: true, + selections: [ + { key: JSON.stringify([0, 'Nope']), provenance: { scope: { kind: 'workspace' } } }, + ], + }, + }, + }) + + expect(stamps[0]).toEqual({ complete: true, columns: {} }) + }) +}) diff --git a/apps/sim/lib/table/application/row-secret-provenance.ts b/apps/sim/lib/table/application/row-secret-provenance.ts new file mode 100644 index 00000000000..3911703ac42 --- /dev/null +++ b/apps/sim/lib/table/application/row-secret-provenance.ts @@ -0,0 +1,146 @@ +import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal' +import { isPrivateSecretProvenanceScopeCompatible } from '@/lib/execution/durable-secret-provenance' +import { isPrivateSecretProvenanceBundleV1 } from '@/lib/execution/model-input-provenance' +import { buildIdByName } from '@/lib/table/column-keys' +import { + createExactEmptyTableRowSecretProvenance, + createUnknownTableRowSecretProvenance, +} from '@/lib/table/rows/secret-provenance' +import { tableRowSecretProvenanceSelectionKey } from '@/lib/table/secret-provenance-selection' +import type { RowData, TableDefinition, TableRowSecretProvenanceWrite } from '@/lib/table/types' + +/** + * The private provenance envelope exactly as it arrived on the wire. + * + * A surface adapter can read the header and the payload field — that is + * transport — but it cannot decide what the selections mean, because mapping a + * caller's column key to the storage column id it certifies requires the + * canonical schema. That resolution lives here, behind authorization, which is + * what lets the row routes stop loading the table for themselves. + */ +export type TableRowProvenanceEnvelope = { kind: 'none' } | { kind: 'bundle'; value: unknown } + +/** Raised when an envelope does not authenticate against the canonical table. */ +export class TableRowProvenanceError extends Error { + constructor(message = 'Invalid table row secret provenance') { + super(message) + this.name = 'TableRowProvenanceError' + } +} + +/** + * Storage column id for each key the caller wrote, or `null` where the key names + * no column and is therefore never persisted. + * + * This must mirror {@link rowDataToStorage} exactly, or a cell could be written + * with no provenance recorded under a `complete` stamp. The two wires differ in + * what they do with an unrecognised key: the name path drops it, so it gets + * `null`; the id path stores what it is given, so every key it sends is a + * storage key and none of them is `null`. + */ +function storageKeyResolver( + table: TableDefinition, + keying: 'names' | 'ids' +): (wireKey: string) => string | null { + // Built once for the whole batch rather than once per row, matching how + // `rowsToStorage` hoists the same index. + if (keying === 'ids') return (wireKey) => wireKey + const idByName = buildIdByName(table.schema) + return (wireKey) => idByName.get(wireKey) ?? null +} + +/** + * What a write should stamp on its provenance sidecar. + * + * `undefined` is not "nothing to record" — it means *deliberately untracked*, the + * legacy protocol for an internal caller that sent no envelope. Defaulting it to + * an exact-empty stamp would certify "this write introduced no secrets" on a + * runtime write that may well have introduced some, so the two must stay + * distinguishable all the way to the sidecar. + */ +export interface ResolvedRowWriteProvenance { + stamps: Array +} + +/** + * Resolves a wire envelope into per-row provenance stamps against the canonical + * table. + * + * An interactive caller (session) certifies exact-empty over the storage columns + * its write actually persists. An internal execution may submit an encrypted + * bundle, which must name exactly the columns the write touched and must carry a + * scope this principal is allowed to read. Anything else fails closed. + */ +export function resolveRowWriteProvenance(options: { + envelope: TableRowProvenanceEnvelope + principal: Principal + workspaceId: string + table: TableDefinition + keying: 'names' | 'ids' + wireRows: readonly RowData[] + storageRows: readonly RowData[] +}): ResolvedRowWriteProvenance { + const { envelope, principal, table, keying, wireRows, storageRows } = options + const isDelegated = principal.kind !== 'session' + + if (envelope.kind === 'none') { + // An internal caller that sent nothing stays untracked, as it always has. + if (isDelegated) return { stamps: wireRows.map(() => undefined) } + return { stamps: storageRows.map((row) => createExactEmptyTableRowSecretProvenance(row)) } + } + + if (!isDelegated || !isPrivateSecretProvenanceBundleV1(envelope.value)) { + throw new TableRowProvenanceError() + } + const bundle = envelope.value + + const storageKeyFor = storageKeyResolver(table, keying) + /** Every cell this write touches, by the selection key a bundle must name. */ + const touchedBySelectionKey = new Map() + wireRows.forEach((row, rowIndex) => { + for (const wireKey of Object.keys(row)) { + touchedBySelectionKey.set(tableRowSecretProvenanceSelectionKey(rowIndex, wireKey), { + rowIndex, + columnId: storageKeyFor(wireKey), + }) + } + }) + + // A complete bundle must account for every cell the write touched, and only + // those — otherwise a caller could certify a column it never wrote. + if ( + bundle.complete && + (bundle.selections.length !== touchedBySelectionKey.size || + bundle.selections.some((selection) => !touchedBySelectionKey.has(selection.key))) + ) { + throw new TableRowProvenanceError() + } + + if (!bundle.complete) { + return { stamps: wireRows.map(() => createUnknownTableRowSecretProvenance()) } + } + + const stamps: TableRowSecretProvenanceWrite[] = wireRows.map(() => ({ + complete: true, + columns: {}, + })) + const subjectUserId = requirePrincipalSubjectUserId(principal) + for (const selection of bundle.selections) { + const touched = touchedBySelectionKey.get(selection.key) + if ( + !touched || + !isPrivateSecretProvenanceScopeCompatible(selection.provenance.scope, { + userId: subjectUserId, + workspaceId: options.workspaceId, + }) + ) { + throw new TableRowProvenanceError() + } + if (touched.columnId === null) continue + if (Object.hasOwn(stamps[touched.rowIndex].columns, touched.columnId)) { + throw new TableRowProvenanceError() + } + stamps[touched.rowIndex].columns[touched.columnId] = selection.provenance + } + return { stamps } +} diff --git a/apps/sim/lib/table/application/rows.test.ts b/apps/sim/lib/table/application/rows.test.ts index 2f8c9325562..2bfbeb337b9 100644 --- a/apps/sim/lib/table/application/rows.test.ts +++ b/apps/sim/lib/table/application/rows.test.ts @@ -19,6 +19,7 @@ const { mockResolveContext, mockResolvePermission, mockSignalRowsChanged, + mockSignalRowsChangedByActor, mockUpsertRow, mockWithLockedTable, mockInsertRow, @@ -41,6 +42,7 @@ const { mockResolveContext: vi.fn(), mockResolvePermission: vi.fn(), mockSignalRowsChanged: vi.fn(), + mockSignalRowsChangedByActor: vi.fn(), mockUpsertRow: vi.fn(), mockWithLockedTable: vi.fn(), mockInsertRow: vi.fn(), @@ -137,6 +139,7 @@ vi.mock('@/lib/table/application/context', () => ({ vi.mock('@/lib/table/events', () => ({ signalTableRowsChanged: mockSignalRowsChanged, + signalTableRowsChangedByActor: mockSignalRowsChangedByActor, })) import { @@ -172,6 +175,22 @@ const TABLE: TableDefinition = { const PRINCIPAL = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' } +/** + * The active-table context every row command resolves before it does any work. + * Pass a variant table when a test needs a different schema — the surrounding + * workspace scope is the same for every command under test. + */ +function contextFor(table: TableDefinition = TABLE) { + return { + tableId: table.id, + table, + workspaceId: table.workspaceId, + workspaceOrganizationId: 'organization-1', + allowPersonalApiKeys: true, + billedAccountUserId: 'billing-owner-1', + } +} + describe('table predicate translation', () => { it('maps invalid run filters to the shared row validation error', () => { expect(() => @@ -210,14 +229,7 @@ describe('replaceProjectedWireRows application command', () => { beforeEach(() => { vi.clearAllMocks() mockResolvePermission.mockResolvedValue('write') - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: TABLE, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor()) mockAssertRowCapacity.mockResolvedValue(10_000) mockWithLockedTable.mockImplementation( async (_tableId: string, run: (table: TableDefinition, trx: unknown) => unknown) => @@ -457,14 +469,7 @@ describe('replaceTableRows application use case', () => { beforeEach(() => { vi.clearAllMocks() mockResolvePermission.mockResolvedValue('write') - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: TABLE, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor()) mockReplaceRowsPrimitive.mockResolvedValue({ deletedCount: 2, insertedCount: 1 }) }) @@ -504,7 +509,12 @@ describe('replaceTableRows application use case', () => { await expect( replaceTableRows.execute({ principal: PRINCIPAL, - input: { tableId: TABLE.id, rows: [{ name: 'Ada', unknown: 'x' }], strictWrite: true }, + input: { + tableId: TABLE.id, + rows: [{ name: 'Ada', unknown: 'x' }], + strictWrite: true, + dataKeying: 'names', + }, }) ).rejects.toThrow(/Row 1: Unknown column: unknown/) expect(mockReplaceRowsPrimitive).not.toHaveBeenCalled() @@ -565,14 +575,7 @@ describe('row query and upsert application semantics', () => { beforeEach(() => { vi.clearAllMocks() mockResolvePermission.mockResolvedValue('write') - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: TABLE, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor()) }) it('rejects a malformed POST query cursor before querying storage', async () => { @@ -792,6 +795,7 @@ describe('row query and upsert application semantics', () => { requestId: 'request-1', data: { name: 'Ada' }, conflictTarget: 'name', + dataKeying: 'names', }, }) @@ -825,14 +829,7 @@ describe('table row write secret provenance defaulting', () => { beforeEach(() => { vi.clearAllMocks() mockResolvePermission.mockResolvedValue('write') - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: TABLE, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor()) mockValidateRowData.mockResolvedValue({ valid: true }) mockValidateBatchRows.mockResolvedValue({ valid: true }) mockInsertRow.mockResolvedValue(ROW) @@ -892,14 +889,7 @@ describe('table row write secret provenance defaulting', () => { ...TABLE, schema: { columns: [{ id: 'column_name', name: 'name', type: 'string' }] }, } - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: filterableTable, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor(filterableTable)) await updateTableRows.execute({ principal: PRINCIPAL, @@ -989,14 +979,7 @@ describe('unknown column names under strictWrite', () => { beforeEach(() => { vi.clearAllMocks() mockResolvePermission.mockResolvedValue('write') - mockResolveContext.mockResolvedValue({ - tableId: TABLE.id, - table: TABLE, - workspaceId: TABLE.workspaceId, - workspaceOrganizationId: 'organization-1', - allowPersonalApiKeys: true, - billedAccountUserId: 'billing-owner-1', - }) + mockResolveContext.mockResolvedValue(contextFor()) mockValidateRowData.mockResolvedValue({ valid: true }) mockValidateBatchRows.mockResolvedValue({ valid: true }) mockInsertRow.mockResolvedValue({ id: 'row-1', data: {} }) @@ -1010,7 +993,13 @@ describe('unknown column names under strictWrite', () => { await expect( createTableRows.execute({ principal: PRINCIPAL, - input: { kind: 'single', tableId: TABLE.id, data: { nosuchcol: 'x' }, strictWrite: true }, + input: { + kind: 'single', + tableId: TABLE.id, + data: { nosuchcol: 'x' }, + strictWrite: true, + dataKeying: 'names', + }, }) ).rejects.toThrow(/Unknown column: nosuchcol/) expect(mockInsertRow).not.toHaveBeenCalled() @@ -1040,6 +1029,7 @@ describe('unknown column names under strictWrite', () => { tableId: TABLE.id, data: { zzz: 'x', qqq: 'y' }, strictWrite: true, + dataKeying: 'names', }, }) ).rejects.toThrow(/Unknown columns: zzz, qqq/) @@ -1054,6 +1044,7 @@ describe('unknown column names under strictWrite', () => { tableId: TABLE.id, rows: [{ name: 'Ada' }, { zzz: 'x' }], strictWrite: true, + dataKeying: 'names', }, }) ).rejects.toThrow(/Row 2: Unknown column: zzz/) @@ -1069,6 +1060,7 @@ describe('unknown column names under strictWrite', () => { filter: { all: [{ field: 'name', op: 'eq', value: 'Ada' }] }, data: { zzz: 'x' }, strictWrite: true, + dataKeying: 'names', }, }) ).rejects.toThrow(/Unknown column: zzz/) @@ -1079,7 +1071,13 @@ describe('unknown column names under strictWrite', () => { await expect( updateTableRow.execute({ principal: PRINCIPAL, - input: { tableId: TABLE.id, rowId: 'row-1', data: { zzz: 'x' }, strictWrite: true }, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { zzz: 'x' }, + strictWrite: true, + dataKeying: 'names', + }, }) ).rejects.toThrow(/Unknown column: zzz/) expect(mockUpdateRow).not.toHaveBeenCalled() @@ -1107,7 +1105,13 @@ describe('unknown column names under strictWrite', () => { it('carries the strict value policy to the primitive, and nothing without it', async () => { await createTableRows.execute({ principal: PRINCIPAL, - input: { kind: 'single', tableId: TABLE.id, data: { name: 'Ada' }, strictWrite: true }, + input: { + kind: 'single', + tableId: TABLE.id, + data: { name: 'Ada' }, + strictWrite: true, + dataKeying: 'names', + }, }) expect(mockInsertRow).toHaveBeenLastCalledWith(expect.anything(), TABLE, expect.any(String), { uncoercibleValues: 'reject', @@ -1120,3 +1124,162 @@ describe('unknown column names under strictWrite', () => { expect(mockInsertRow).toHaveBeenLastCalledWith(expect.anything(), TABLE, expect.any(String), {}) }) }) + +/** + * The two wires a table write can arrive on. `/api/v2`, `/api/v1` and the + * Copilot tools publish column names; the first-party grid and the internal + * `/api/table` routes publish stable storage ids. + * + * The failure this guards is silent: the name remap drops what it does not + * recognise, and a storage id names no column *name*, so an id-keyed write sent + * down the name path stores nothing while reporting success. + */ +describe('row data keying', () => { + beforeEach(() => { + vi.clearAllMocks() + mockResolvePermission.mockResolvedValue('write') + mockResolveContext.mockResolvedValue(contextFor()) + mockAssertRowCapacity.mockResolvedValue(10_000) + mockCreateSecretProvenance.mockReturnValue({ complete: true, columns: {} }) + mockIsScopeCompatible.mockReturnValue(true) + }) + + it('stores an id-keyed write exactly as given', async () => { + await updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { 'column-name': 'Ada' }, + strictWrite: false, + dataKeying: 'ids', + }, + }) + + expect(mockUpdateRow).toHaveBeenCalledWith( + expect.objectContaining({ data: { 'column-name': 'Ada' } }), + TABLE, + expect.any(String), + expect.anything() + ) + }) + + it('does not silently drop an id-keyed write, which the name path would', async () => { + await updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { 'column-name': 'Ada' }, + strictWrite: false, + dataKeying: 'names', + }, + }) + + // Pins the hazard itself: the same payload on the name wire stores nothing. + expect(mockUpdateRow).toHaveBeenCalledWith( + expect.objectContaining({ data: {} }), + TABLE, + expect.any(String), + expect.anything() + ) + }) + + it('persists an unrecognised key on the lax id wire, unlike the name wire', async () => { + // The asymmetry a non-strict id-keyed caller sees, pinned deliberately: the + // name path drops what it cannot resolve, the id path stores what it is + // given. This is what the grid does today via the identity `dataIn` in + // `row-wire.ts`, so the discriminator preserved it rather than changing it. + // Closing it is a behaviour change and belongs with the route migration. + await updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { 'column-name': 'Ada', 'no-such-column': 'x' }, + strictWrite: false, + dataKeying: 'ids', + }, + }) + + expect(mockUpdateRow).toHaveBeenCalledWith( + expect.objectContaining({ data: { 'column-name': 'Ada', 'no-such-column': 'x' } }), + TABLE, + expect.any(String), + expect.anything() + ) + }) + + it('translates a name-keyed write to storage ids', async () => { + await updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { name: 'Ada' }, + strictWrite: false, + dataKeying: 'names', + }, + }) + + expect(mockUpdateRow).toHaveBeenCalledWith( + expect.objectContaining({ data: { 'column-name': 'Ada' } }), + TABLE, + expect.any(String), + expect.anything() + ) + }) + + it('refuses an unknown column id when the caller writes strictly', async () => { + await expect( + updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { 'column-nope': 'x' }, + strictWrite: true, + dataKeying: 'ids', + }, + }) + ).rejects.toThrow(/Unknown column: column-nope/) + expect(mockUpdateRow).not.toHaveBeenCalled() + }) + + it('accepts a legacy column that has no id and is stored under its name', async () => { + // Two production tables still carry pre-backfill columns with no `id`. + // Their storage key is the name, so a strict id-keyed write naming one must + // be accepted, not refused as unknown. + mockResolveContext.mockResolvedValue( + contextFor({ ...TABLE, schema: { columns: [{ name: 'legacy', type: 'string' }] } }) + ) + + await expect( + updateTableRow.execute({ + principal: PRINCIPAL, + input: { + tableId: TABLE.id, + rowId: 'row-1', + data: { legacy: 'x' }, + strictWrite: true, + dataKeying: 'ids', + }, + }) + ).resolves.toBeDefined() + }) + + it('names every unknown id at once, as the name wire does', async () => { + await expect( + createTableRows.execute({ + principal: PRINCIPAL, + input: { + kind: 'batch', + tableId: TABLE.id, + rows: [{ 'column-name': 'Ada' }, { zzz: 'x', qqq: 'y' }], + strictWrite: true, + dataKeying: 'ids', + }, + }) + ).rejects.toThrow(/Row 2: Unknown columns: zzz, qqq/) + }) +}) diff --git a/apps/sim/lib/table/application/rows.ts b/apps/sim/lib/table/application/rows.ts index 5185c9b0c52..3e0a06710fa 100644 --- a/apps/sim/lib/table/application/rows.ts +++ b/apps/sim/lib/table/application/rows.ts @@ -1,6 +1,11 @@ import { isDeepStrictEqual } from 'node:util' import { AuditAction, AuditResourceType } from '@sim/audit' -import { requirePrincipalSubjectUserId, resolvePrincipalAttribution } from '@sim/auth/principal' +import { + type Principal, + requirePrincipalSubjectUserId, + resolvePrincipalAttribution, +} from '@sim/auth/principal' +import { db } from '@sim/db' import { getRequestContext } from '@sim/logger' import { generateId } from '@sim/utils/id' import { isPlainRecord } from '@sim/utils/object' @@ -25,13 +30,14 @@ import { deleteRowsByFilter, deleteRowsByIds, findRowMatches, - getRowById, + getRowSummaryById, insertRow, queryRows, replaceTableRows as replaceTableRowsPrimitive, rowDataNameToId, sortSpecNamesToIds, TABLE_LIMITS, + type TableRowSummary, updateRow, updateRowsByFilter, upsertRow, @@ -42,11 +48,15 @@ import { import { defineAuthorizedTableUseCase } from '@/lib/table/application/authorized-table-use-case' import { resolveActiveTableContext } from '@/lib/table/application/context' import { tableOperations } from '@/lib/table/application/operations' +import { + resolveRowWriteProvenance, + type TableRowProvenanceEnvelope, +} from '@/lib/table/application/row-secret-provenance' import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' -import { buildIdByName, unknownColumnNames } from '@/lib/table/column-keys' +import { buildColumnNameById, buildIdByName, unknownColumnNames } from '@/lib/table/column-keys' import { columnTypeOf } from '@/lib/table/column-types' import { TableQueryValidationError } from '@/lib/table/errors' -import { signalTableRowsChanged } from '@/lib/table/events' +import { signalTableRowsChanged, signalTableRowsChangedByActor } from '@/lib/table/events' import { predicateToFilter } from '@/lib/table/query-builder/converters' import { validatePredicate, @@ -55,6 +65,7 @@ import { validateStoragePredicate, } from '@/lib/table/query-builder/validate' import { assertCursorQueryBinding, decodeCursor } from '@/lib/table/rows/cursor' +import { loadEnrichmentDetail } from '@/lib/table/rows/executions' import { createExactEmptyTableRowSecretProvenance, createTableRowSecretProvenanceFromRegistry, @@ -81,20 +92,21 @@ interface TableScopedInput { tableId: string assertedWorkspaceId?: string requestId?: string - /** - * Whether the calling surface publishes the stricter `/api/v2` write contract: - * a row naming a column the table does not have is refused rather than having - * that key dropped, and a value the column's type cannot coerce is answered - * with a 400 rather than stored as `null`. - * - * Absent — every first-party surface, and the only behavior any of them has - * ever had: the workspace grid, the internal `/api/table` routes, `/api/v1`, - * the Copilot table tools, and the executor's Table block all drop the - * unknown key and blank the uncoercible cell. Read-only use cases ignore it. - */ } -/** The write policy `strictWrite` selects, for the row-service primitives. */ +/** + * The write policy `strictWrite` selects, for the row-service primitives. + * + * `strictWrite` means the calling surface publishes the stricter `/api/v2` write + * contract: a row naming a column the table does not have is refused rather than + * having that key dropped, and a value the column's type cannot coerce is + * answered with a 400 rather than stored as `null`. + * + * Absent — every first-party surface, and the only behavior any of them has ever + * had: the workspace grid, the internal `/api/table` routes, `/api/v1`, the + * Copilot table tools, and the executor's Table block all drop the unknown key + * and blank the uncoercible cell. + */ function rowWriteOptions(input: { strictWrite: boolean }): RowWriteOptions { return input.strictWrite ? { uncoercibleValues: 'reject' } : {} } @@ -108,7 +120,9 @@ type TableRowsProvenance = Awaited[0], workspaceId: string, - rows: TableRow[], + // The loader reads only id, updatedAt and data, so a row without its + // executions sidecar is enough — see `TABLE_ROW_SIDECAR_SELECTION`. + rows: TableRowSummary[], include: boolean | undefined ): Promise { if (!include) return undefined @@ -133,7 +147,7 @@ function actorUserId( /** * Refuses a wire row naming a column the table does not have. Applied only to a - * `strictWrite` caller — see {@link TableScopedInput.strictWrite}. + * `strictWrite` caller — see {@link rowWriteOptions}. * * The name→id remap drops unrecognised keys, so without this an insert of * `{"nosuchcol":"x"}` created an empty row under a 201, and a patch of @@ -147,7 +161,14 @@ function assertKnownColumnNames( idByName: ReadonlyMap, rowLabel?: string ): void { - const unknown = unknownColumnNames(data, idByName) + assertNoUnknownColumns(unknownColumnNames(data, idByName), rowLabel) +} + +/** + * Refuses a wire row naming a column the table does not have, on either wire. + * Shared so the two keyings cannot drift in how they name the offending keys. + */ +function assertNoUnknownColumns(unknown: string[], rowLabel?: string): void { if (unknown.length === 0) return const where = rowLabel ? `${rowLabel}: ` : '' throw new TableRowsValidationError( @@ -155,22 +176,86 @@ function assertKnownColumnNames( ) } -function namedDataToStorage(data: RowData, table: TableDefinition, strict = false): RowData { +/** + * Which column keying a write's row data arrives in. + * + * `'names'` — the caller publishes column **names** on its wire: `/api/v2`, + * `/api/v1`, and the Copilot table tools, where a name is what the caller (or + * the model) can read off a row. Names are remapped to storage ids here. + * + * `'ids'` — the caller already speaks stable storage column **ids**: the + * first-party grid and the internal `/api/table` routes, which hold the schema + * they rendered and address cells by id. + * + * Required so a new write surface must state which contract it publishes. The + * name remap drops keys it does not recognise, and a storage id names no column + * *name*, so feeding id-keyed data through the name path silently drops every + * cell and reports the write as successful. + * + * Row data needs this and filters, sorts and predicates do not: their + * translators pass an unrecognised field through unchanged (`idByName.get(key) + * ?? key`), so they are already correct under either keying. Only the row-data + * remap is lossy, which is why only it carries a discriminator. + */ +export type TableRowDataKeying = 'names' | 'ids' + +/** + * The id-keyed counterpart of {@link assertKnownColumnNames}, applied on the same + * `strictWrite` condition so strictness means the same thing on either wire. + * + * `buildColumnNameById` keys by {@link getColumnId}, so a legacy pre-backfill + * column — which has no id and is stored under its name — is recognised rather + * than reported unknown. + */ +function assertKnownColumnIds(data: RowData, table: TableDefinition, rowLabel?: string): void { + assertNoUnknownColumns( + unknownColumnNames(data, buildColumnNameById(table.schema.columns)), + rowLabel + ) +} + +/** + * Normalizes one wire row to storage keying. See {@link TableRowDataKeying}. + * + * Note the asymmetry a lax (non-`strictWrite`) caller sees: the name path drops + * keys naming no column, while the id path stores what it is given. That + * matches what each wire did before this discriminator existed, and only + * `strictWrite` makes the two agree. + */ +function rowDataToStorage( + data: RowData, + table: TableDefinition, + keying: TableRowDataKeying, + strict = false +): RowData { + if (keying === 'ids') { + if (strict) assertKnownColumnIds(data, table) + return data + } const idByName = buildIdByName(table.schema) if (strict) assertKnownColumnNames(data, idByName) return rowDataNameToId(data, idByName) } /** - * {@link namedDataToStorage} over a batch. The name index is built once for the + * {@link rowDataToStorage} over a batch. Either index is built once for the * whole batch rather than per row — these paths run over up to * `MAX_BATCH_INSERT_SIZE` rows. */ -function namedRowsToStorage( +function rowsToStorage( rows: readonly RowData[], table: TableDefinition, + keying: TableRowDataKeying, strict = false ): RowData[] { + if (keying === 'ids') { + if (!strict) return [...rows] + const nameById = buildColumnNameById(table.schema.columns) + return rows.map((row, index) => { + assertNoUnknownColumns(unknownColumnNames(row, nameById), `Row ${index + 1}`) + return row + }) + } const idByName = buildIdByName(table.schema) return rows.map((row, index) => { if (strict) assertKnownColumnNames(row, idByName, `Row ${index + 1}`) @@ -192,6 +277,38 @@ function defaultedRowSecretProvenance( return provided ?? createExactEmptyTableRowSecretProvenance(storageData) } +/** + * The stamp a single-row write should carry: resolved from the caller's envelope + * when it handed one over, otherwise defaulted. Shared by the update and upsert + * use cases so the envelope contract has one implementation, not two. + */ +function singleRowWriteProvenance(options: { + principal: Principal + workspaceId: string + table: TableDefinition + input: { + dataKeying: TableRowDataKeying + data: RowData + secretProvenance?: TableRowSecretProvenanceWrite + secretProvenanceEnvelope?: TableRowProvenanceEnvelope + } + storageData: RowData +}): TableRowSecretProvenanceWrite | undefined { + const { principal, workspaceId, table, input, storageData } = options + if (!input.secretProvenanceEnvelope) { + return defaultedRowSecretProvenance(storageData, input.secretProvenance) + } + return resolveRowWriteProvenance({ + envelope: input.secretProvenanceEnvelope, + principal, + workspaceId, + table, + keying: input.dataKeying, + wireRows: [input.data], + storageRows: [storageData], + }).stamps[0] +} + function defaultedRowsSecretProvenance( storageRows: RowData[], provided: Array | undefined @@ -394,7 +511,8 @@ export interface ReadTableRowInput extends TableScopedInput { } export interface ReadTableRowResult extends TableResult { - row: TableRow + /** Without the executions sidecar — no read surface puts it on the wire. */ + row: TableRowSummary secretProvenance?: TableRowsProvenance } @@ -402,7 +520,7 @@ export const readTableRow = defineAuthorizedTableUseCase({ operation: tableOperations.readRow, resolveContext: ({ input }: { input: ReadTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { - const row = await getRowById(context.tableId, input.rowId, context.workspaceId) + const row = await getRowSummaryById(context.tableId, input.rowId, context.workspaceId) if (!row) throw new OrchestrationError('not_found', 'Row not found') return { table: context.table, @@ -417,10 +535,44 @@ export const readTableRow = defineAuthorizedTableUseCase({ }, }) +export interface ReadTableRowEnrichmentInput extends TableScopedInput { + rowId: string + groupId: string +} + +export interface ReadTableRowEnrichmentResult extends TableResult { + detail: Awaited> +} + +/** + * The enrichment cascade breakdown — provider outcomes, cost, timing — for one + * cell. Deliberately kept off the hot grid read and fetched on demand by the + * details panel; `null` for a cell with no recorded run, or a run predating the + * feature. + * + * Shares {@link tableOperations.readRow}: this is a projection of the same row, + * under the same role, so it is not a second semantic operation. + */ +export const readTableRowEnrichmentDetail = defineAuthorizedTableUseCase({ + operation: tableOperations.readRow, + resolveContext: ({ input }: { input: ReadTableRowEnrichmentInput }) => + resolveActiveTableContext(input), + async execute({ input, context }): Promise { + return { + table: context.table, + detail: await loadEnrichmentDetail(db, context.tableId, input.rowId, input.groupId), + } + }, +}) + interface CreateSingleTableRowInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying kind: 'single' + /** See {@link UpdateTableRowInput.actorClientId}. */ + actorClientId?: string data: RowData position?: number afterRowId?: string @@ -431,6 +583,8 @@ interface CreateSingleTableRowInput extends TableScopedInput { interface CreateBatchTableRowsInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying kind: 'batch' rows: RowData[] orderKeys?: string[] @@ -458,7 +612,7 @@ export const createTableRows = defineAuthorizedTableUseCase({ ) { throw new TableRowsValidationError('Position must be 0 or greater') } - const data = namedDataToStorage(input.data, context.table, input.strictWrite) + const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) const writeOptions = rowWriteOptions(input) await throwValidationResponse( await validateRowData({ @@ -496,7 +650,7 @@ export const createTableRows = defineAuthorizedTableUseCase({ if (input.orderKeys && input.orderKeys.length !== input.rows.length) { throw new TableRowsValidationError('orderKeys must align one-to-one with rows') } - const rows = namedRowsToStorage(input.rows, context.table, input.strictWrite) + const rows = rowsToStorage(input.rows, context.table, input.dataKeying, input.strictWrite) const batchWriteOptions = rowWriteOptions(input) await throwValidationResponse( await validateBatchRows({ @@ -521,9 +675,16 @@ export const createTableRows = defineAuthorizedTableUseCase({ ) return { kind: 'batch', table: context.table, rows: created } }, - afterSuccess: ({ context, result }) => { - const affected = result.kind === 'single' ? 1 : result.rows.length - if (affected > 0) signalTableRowsChanged(context.tableId) + afterSuccess: ({ context, input, result }) => { + // Narrowed on the input, not the result: only the single-row variant carries + // an actor, and the two discriminants always agree. + if (input.kind === 'single') { + signalTableRowsChangedByActor(context.tableId, input.actorClientId) + return + } + // A batch insert is not reconciled locally by the acting tab, so it must + // refetch like every other subscriber. + if (result.kind === 'batch' && result.rows.length > 0) signalTableRowsChanged(context.tableId) }, }) @@ -532,6 +693,8 @@ const MAX_REPLACE_TABLE_ROWS = 10_000 export interface ReplaceTableRowsInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying rows: RowData[] secretProvenance?: Array } @@ -551,7 +714,7 @@ export const replaceTableRows = defineAuthorizedTableUseCase({ throw new TableRowsValidationError('Secret provenance must align one-to-one with rows') } - const rows = namedRowsToStorage(input.rows, context.table, input.strictWrite) + const rows = rowsToStorage(input.rows, context.table, input.dataKeying, input.strictWrite) const result = await replaceTableRowsPrimitive( { tableId: context.tableId, @@ -663,7 +826,16 @@ function projectedRowsSecretProvenance( }) } -/** Atomically validates name-keyed projected rows against the locked schema and replaces the table. */ +/** + * Atomically validates name-keyed projected rows against the locked schema and + * replaces the table. + * + * Deliberately carries no {@link TableRowDataKeying}: unlike the six generic + * write use cases this one is not surface-agnostic. Its resolved-secret gate and + * its "row matches no column" check both compare by `column.name` (see + * {@link projectedRowsForTable}), and its only caller is Copilot's + * `Function.execute` output — keys a model can only have written as names. + */ export const replaceProjectedWireRows = defineAuthorizedTableUseCase({ operation: tableOperations.replaceRows, resolveContext: ({ input }: { input: ReplaceProjectedWireRowsInput }) => @@ -742,10 +914,28 @@ export const replaceProjectedWireRows = defineAuthorizedTableUseCase({ export interface UpdateTableRowInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying rowId: string data: RowData secretProvenance?: TableRowSecretProvenanceWrite + /** + * Private provenance envelope as it arrived on the wire, resolved here against + * the canonical schema. Mutually exclusive with {@link secretProvenance}: a + * surface either resolves its own stamp or hands over the envelope for this + * use case to resolve, never both. + */ + secretProvenanceEnvelope?: TableRowProvenanceEnvelope includePersistedSecretProvenance?: boolean + /** + * Tab that caused this write, when the calling surface knows it. Lets that tab + * skip refetching its own write — see {@link signalTableRowsChangedByActor}, + * whose soundness condition is that the caller's hook reconciles the write + * locally across every cached rows query. Only the single-row paths accept + * one: a batch or filter-scoped write genuinely needs the acting tab to + * refetch. Absent by default, which broadcasts to every subscriber as before. + */ + actorClientId?: string } export interface UpdateTableRowResult extends TableResult { @@ -758,7 +948,14 @@ export const updateTableRow = defineAuthorizedTableUseCase({ operation: tableOperations.updateRow, resolveContext: ({ input }: { input: UpdateTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { - const data = namedDataToStorage(input.data, context.table, input.strictWrite) + const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) + const secretProvenance = singleRowWriteProvenance({ + principal, + workspaceId: context.workspaceId, + table: context.table, + input, + storageData: data, + }) const row = await updateRow( { tableId: context.tableId, @@ -766,7 +963,7 @@ export const updateTableRow = defineAuthorizedTableUseCase({ rowId: input.rowId, data, actorUserId: actorUserId(principal, context.billedAccountUserId), - secretProvenance: defaultedRowSecretProvenance(data, input.secretProvenance), + secretProvenance, }, context.table, requestId(input), @@ -785,14 +982,16 @@ export const updateTableRow = defineAuthorizedTableUseCase({ ), } }, - afterSuccess: ({ context, result }) => { - if (result.changed) signalTableRowsChanged(context.tableId) + afterSuccess: ({ context, input, result }) => { + if (result.changed) signalTableRowsChangedByActor(context.tableId, input.actorClientId) }, }) export interface UpdateTableRowsInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying filter: TablePredicate data: RowData limit?: number @@ -809,7 +1008,7 @@ export const updateTableRows = defineAuthorizedTableUseCase({ if (input.limit !== undefined) { requireIntegerInRange(input.limit, 1, TABLE_LIMITS.MAX_BULK_OPERATION_SIZE, 'Limit') } - const data = namedDataToStorage(input.data, context.table, input.strictWrite) + const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) const result = await updateRowsByFilter( context.table, { @@ -834,6 +1033,8 @@ export const updateTableRows = defineAuthorizedTableUseCase({ export interface DeleteTableRowInput extends TableScopedInput { rowId: string + /** See {@link UpdateTableRowInput.actorClientId}. */ + actorClientId?: string } export interface DeleteTableRowResult extends TableResult { @@ -847,7 +1048,8 @@ export const deleteTableRow = defineAuthorizedTableUseCase({ await deleteRow(context.table, input.rowId, requestId(input)) return { table: context.table, deletedRowId: input.rowId } }, - afterSuccess: ({ context }) => signalTableRowsChanged(context.tableId), + afterSuccess: ({ context, input }) => + signalTableRowsChangedByActor(context.tableId, input.actorClientId), }) export type DeleteTableRowsInput = TableScopedInput & @@ -918,24 +1120,41 @@ export const deleteTableRows = defineAuthorizedTableUseCase({ export interface UpsertTableRowInput extends TableScopedInput { /** See {@link rowWriteOptions}. Required so a new write surface must choose. */ strictWrite: boolean + /** See {@link TableRowDataKeying}. Required so a new write surface must choose. */ + dataKeying: TableRowDataKeying data: RowData conflictTarget?: string secretProvenance?: TableRowSecretProvenanceWrite + /** See {@link UpdateTableRowInput.secretProvenanceEnvelope}. */ + secretProvenanceEnvelope?: TableRowProvenanceEnvelope + includePersistedSecretProvenance?: boolean } export interface UpsertTableRowResult extends TableResult { - row: TableRow + /** Without the executions sidecar — see {@link UpsertResult.row}. */ + row: TableRowSummary operation: 'insert' | 'update' + secretProvenance?: TableRowsProvenance } export const upsertTableRow = defineAuthorizedTableUseCase({ operation: tableOperations.upsertRow, resolveContext: ({ input }: { input: UpsertTableRowInput }) => resolveActiveTableContext(input), async execute({ principal, input, context }): Promise { - const conflictTarget = input.conflictTarget - ? (buildIdByName(context.table.schema).get(input.conflictTarget) ?? input.conflictTarget) - : undefined - const data = namedDataToStorage(input.data, context.table, input.strictWrite) + // An id-keyed caller already names the storage column; only a name-keyed + // one needs the lookup, and its miss falls through as before. + const conflictTarget = + input.conflictTarget && input.dataKeying !== 'ids' + ? (buildIdByName(context.table.schema).get(input.conflictTarget) ?? input.conflictTarget) + : input.conflictTarget + const data = rowDataToStorage(input.data, context.table, input.dataKeying, input.strictWrite) + const secretProvenance = singleRowWriteProvenance({ + principal, + workspaceId: context.workspaceId, + table: context.table, + input, + storageData: data, + }) const result = await upsertRow( { tableId: context.tableId, @@ -943,13 +1162,23 @@ export const upsertTableRow = defineAuthorizedTableUseCase({ data, conflictTarget, userId: actorUserId(principal, context.billedAccountUserId), - secretProvenance: defaultedRowSecretProvenance(data, input.secretProvenance), + secretProvenance, }, context.table, requestId(input), rowWriteOptions(input) ) - return { table: context.table, row: result.row, operation: result.operation } + return { + table: context.table, + row: result.row, + operation: result.operation, + secretProvenance: await loadAuthorizedRowsProvenance( + principal, + context.workspaceId, + [result.row], + input.includePersistedSecretProvenance + ), + } }, afterSuccess: ({ context }) => signalTableRowsChanged(context.tableId), }) diff --git a/apps/sim/lib/table/column-keys.ts b/apps/sim/lib/table/column-keys.ts index fc92ac7e9fb..5f8b0e7e19e 100644 --- a/apps/sim/lib/table/column-keys.ts +++ b/apps/sim/lib/table/column-keys.ts @@ -188,7 +188,7 @@ export function remapViewConfigColumnRefs( * drop — a key that survives to here unrecognised is a cell the caller asked to * write and the table never stored. Callers on a surface that can answer the * client check {@link unknownColumnNames} first; see - * `namedDataToStorage` in `application/rows.ts`. + * `rowDataToStorage` in `application/rows.ts`. */ export function rowDataNameToId(data: RowData, idByName: Map): RowData { const out: RowData = {} @@ -207,8 +207,11 @@ export function rowDataNameToId(data: RowData, idByName: Map): R * row, and letting it through would reinstate the silent drop for exactly the * callers most likely to believe they had written something. */ -export function unknownColumnNames(data: RowData, idByName: ReadonlyMap): string[] { - return Object.keys(data).filter((name) => !idByName.has(name)) +export function unknownColumnNames( + data: RowData, + knownKeys: ReadonlyMap +): string[] { + return Object.keys(data).filter((key) => !knownKeys.has(key)) } /** diff --git a/apps/sim/lib/table/events.attribution.test.ts b/apps/sim/lib/table/events.attribution.test.ts index e0f8c9ee448..351aa715f5e 100644 --- a/apps/sim/lib/table/events.attribution.test.ts +++ b/apps/sim/lib/table/events.attribution.test.ts @@ -9,16 +9,25 @@ import { describe, expect, it } from 'vitest' * `signalTableRowsChangedByActor` lets the acting tab skip its own refetch, which is only sound * where that tab's mutation hook already applies the server's answer to every cached rows query. * That invariant lives in `hooks/queries/tables.ts` — nothing in the type system ties it to the - * call site, so a well-meaning fourth call would silently strand that client on stale rows. + * call site, so a well-meaning extra call would silently strand that client on stale rows. * - * This pins the allowlist. If you are here because it failed: adding a call means proving the - * calling route's client hook reconciles locally, then adding it below. Removing one is always safe. + * Two lists are pinned, because a migrated single-row route now signals from inside its + * application use case rather than from the route. The call itself is no longer the decision: + * that use case is shared with `/api/v2` and Copilot, and it degrades to a broadcast whenever no + * actor is named. What actually selects the behavior is which surface supplies `actorClientId`, + * so that is pinned too and is the list to scrutinise. + * + * If you are here because it failed: adding a supplier means proving that surface's client hook + * reconciles the write locally across every cached rows query. Removing one is always safe. */ const ATTRIBUTED_CALL_SITES = [ 'app/api/table/[tableId]/rows/route.ts', - 'app/api/table/[tableId]/rows/[rowId]/route.ts', + 'lib/table/application/rows.ts', ] as const +/** Surfaces that name the acting tab. See the note above — this is the real allowlist. */ +const ACTOR_SUPPLYING_SURFACES = ['app/api/table/[tableId]/rows/[rowId]/route.ts'] as const + const APP_ROOT = join(import.meta.dirname, '../..') /** Declares the function; matching its own definition would say nothing about call sites. */ const DECLARING_MODULE = 'lib/table/events.ts' @@ -32,17 +41,37 @@ async function* walk(dir: string): AsyncGenerator { } } +async function filesContaining(needle: string, skip: (relative: string) => boolean = () => false) { + const found: string[] = [] + for await (const file of walk(APP_ROOT)) { + const source = await readFile(file, 'utf8') + if (!source.includes(needle)) continue + const relative = file.slice(APP_ROOT.length + 1) + if (skip(relative)) continue + found.push(relative) + } + return found.sort() +} + describe('signalTableRowsChangedByActor call sites', () => { it('is called only where the acting tab reconciles the write locally', async () => { - const callers: string[] = [] - for await (const file of walk(APP_ROOT)) { - const source = await readFile(file, 'utf8') - if (!source.includes('signalTableRowsChangedByActor(')) continue - const relative = file.slice(APP_ROOT.length + 1) - if (relative === DECLARING_MODULE) continue - callers.push(relative) - } - - expect(callers.sort()).toEqual([...ATTRIBUTED_CALL_SITES].sort()) + const callers = await filesContaining( + 'signalTableRowsChangedByActor(', + (relative) => relative === DECLARING_MODULE + ) + + expect(callers).toEqual([...ATTRIBUTED_CALL_SITES].sort()) + }) + + it('is given an actor only by surfaces whose client hook reconciles locally', async () => { + const suppliers = await filesContaining( + 'actorClientId:', + // These declare or forward the field rather than naming a tab. + (relative) => + relative === 'lib/table/application/rows.ts' || + relative === 'lib/table/application/row-secret-provenance.ts' + ) + + expect(suppliers).toEqual([...ACTOR_SUPPLYING_SURFACES].sort()) }) }) diff --git a/apps/sim/lib/table/jobs/service.test.ts b/apps/sim/lib/table/jobs/service.test.ts new file mode 100644 index 00000000000..b537f1f9cab --- /dev/null +++ b/apps/sim/lib/table/jobs/service.test.ts @@ -0,0 +1,176 @@ +/** + * @vitest-environment node + */ +import { dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + EMPTY_JOB_FIELDS, + type LatestJobRow, + latestJobsForTables, + latestNonExportJobJson, + mapJobRow, +} from '@/lib/table/jobs/service' + +function job(overrides: Partial): LatestJobRow { + return { + id: 'job-1', + type: 'delete', + status: 'running', + rowsProcessed: 0, + error: null, + doomedCount: null, + ...overrides, + } +} + +describe('mapJobRow', () => { + it('returns the empty fields when the table has no job row', () => { + expect(mapJobRow(null)).toEqual(EMPTY_JOB_FIELDS) + expect(mapJobRow(undefined)).toEqual(EMPTY_JOB_FIELDS) + }) + + it('projects a running delete job and its remaining doomed rows', () => { + expect(mapJobRow(job({ rowsProcessed: 4, doomedCount: 10 }))).toEqual({ + jobStatus: 'running', + jobId: 'job-1', + jobType: 'delete', + jobError: null, + jobRowsProcessed: 4, + pendingDeleteRemaining: 6, + }) + }) + + it('ignores doomedCount once the delete job is terminal', () => { + expect( + mapJobRow(job({ status: 'ready', rowsProcessed: 4, doomedCount: 10 })).pendingDeleteRemaining + ).toBe(0) + }) + + it('ignores doomedCount for a running job that is not a delete', () => { + expect( + mapJobRow(job({ type: 'import', rowsProcessed: 4, doomedCount: 10 })).pendingDeleteRemaining + ).toBe(0) + }) + + it('treats a missing doomedCount as zero and never goes negative', () => { + expect(mapJobRow(job({ rowsProcessed: 4 })).pendingDeleteRemaining).toBe(0) + expect(mapJobRow(job({ rowsProcessed: 25, doomedCount: 10 })).pendingDeleteRemaining).toBe(0) + }) + + it('carries a failed job error through', () => { + expect(mapJobRow(job({ status: 'failed', error: 'boom' }))).toMatchObject({ + jobStatus: 'failed', + jobError: 'boom', + }) + }) +}) + +/** + * The lateral is a raw `sql` fragment, so the mocked drizzle `sql` tag is the only + * place its text is observable — and the text IS the contract (`getTableById` would + * otherwise silently return a different job than `latestJobsForTables` does). + */ +function renderLateral(): { text: string; values: unknown[] } { + // double-cast-allowed: the mocked drizzle `sql` tag exposes the raw template parts + const fragment = latestNonExportJobJson(schemaMock.userTableDefinitions.id) as unknown as { + strings: string[] + values: unknown[] + } + return { text: fragment.strings.join(' ? ').replace(/\s+/g, ' '), values: fragment.values } +} + +describe('latestNonExportJobJson', () => { + it('excludes export jobs', () => { + expect(renderLateral().text).toContain("<> 'export'") + }) + + it('takes the single newest job by started_at', () => { + const { text, values } = renderLateral() + expect(text).toContain('order by ? desc') + expect(text).toContain('limit 1') + expect(values).toContain(schemaMock.tableJobs.startedAt) + }) + + it('correlates the subquery to the outer table id', () => { + const { text, values } = renderLateral() + expect(text).toContain('where ? = ?') + expect(values).toContain(schemaMock.tableJobs.tableId) + expect(values).toContain(schemaMock.userTableDefinitions.id) + }) + + // No drift test for the projected field list: the fragment derives its + // jsonb pairs from JOB_PROJECTION, which `satisfies Record`. A missing field is a compile error, which is + // stronger than anything asserted here could be. + + /** + * `table_jobs.payload` also holds a delete job's unbounded `excludeRowIds`, and + * this read runs on essentially every table request — so selecting the whole + * column is a payload leak the type system cannot see (`doomedCount` would still + * be present, just derived in JS). Only the rendered pair list shows it. + */ + it('projects doomedCount out of the payload instead of the payload column', () => { + const pairs = renderPairs() + expect(pairs.sql).toContain("'doomedCount'") + expect(pairs.sql).toContain("->'doomedCount'") + expect(pairs.sql).not.toContain("'payload'") + expect(pairs.sql).not.toContain(', payload') + }) +}) + +/** The `jsonb_build_object` key/value list the lateral builds from JOB_PROJECTION. */ +function renderPairs(): { sql: string; params: unknown[] } { + // double-cast-allowed: the mocked drizzle `sql` tag exposes the raw template parts + const fragment = latestNonExportJobJson(schemaMock.userTableDefinitions.id) as unknown as { + values: Array<{ fragments?: unknown[]; toSQL?: () => { sql: string; params: unknown[] } }> + } + const join = fragment.values.find((value) => Array.isArray(value?.fragments)) + if (!join?.toSQL) throw new Error('lateral no longer builds its pairs with sql.join') + return join.toSQL() +} + +/** + * The list endpoint runs this once per page, so the batch read must narrow too. It + * shares JOB_PROJECTION with the lateral, and this pins that sharing down. + */ +describe('latestJobsForTables', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('selects doomedCount rather than the whole payload column', async () => { + await latestJobsForTables(['table-1']) + + const projection = dbChainMockFns.selectDistinctOn.mock.calls[0][1] as Record + expect(Object.keys(projection)).toEqual([ + 'tableId', + 'id', + 'type', + 'status', + 'rowsProcessed', + 'error', + 'doomedCount', + ]) + expect(projection).not.toHaveProperty('payload') + expect(projection.doomedCount).not.toBe(schemaMock.tableJobs.payload) + }) + + it('still derives a running delete job from the narrowed row', async () => { + dbChainMockFns.orderBy.mockResolvedValueOnce([ + { + tableId: 'table-1', + id: 'job-1', + type: 'delete', + status: 'running', + rowsProcessed: 4, + error: null, + doomedCount: 10, + }, + ]) + + const jobs = await latestJobsForTables(['table-1']) + + expect(jobs.get('table-1')).toMatchObject({ jobId: 'job-1', pendingDeleteRemaining: 6 }) + }) +}) diff --git a/apps/sim/lib/table/jobs/service.ts b/apps/sim/lib/table/jobs/service.ts index c5f16114f8b..0f2f4ed27b0 100644 --- a/apps/sim/lib/table/jobs/service.ts +++ b/apps/sim/lib/table/jobs/service.ts @@ -13,8 +13,8 @@ import { db } from '@sim/db' import { tableJobs, userTableDefinitions, userTableRows } from '@sim/db/schema' +import type { Column, SQL } from 'drizzle-orm' import { and, asc, desc, eq, gt, inArray, ne, or, sql } from 'drizzle-orm' -import type { DbOrTx } from '@/lib/db/types' import { pendingDeleteMask } from '@/lib/table/rows/pending-delete-mask' import type { RowData, @@ -25,7 +25,7 @@ import type { } from '@/lib/table/types' /** Job fields projected onto a {@link TableDefinition}, derived from its latest `table_jobs` row. */ -interface DerivedJobFields { +export interface DerivedJobFields { jobStatus: TableDefinition['jobStatus'] jobId: string | null jobType: TableDefinition['jobType'] @@ -49,23 +49,29 @@ export const EMPTY_JOB_FIELDS: DerivedJobFields = { pendingDeleteRemaining: 0, } -function mapJobRow( - row: - | { - id: string - type: string - status: string - rowsProcessed: number - error: string | null - payload: unknown - } - | undefined -): DerivedJobFields { +/** + * The shape every latest-job read produces, whether it comes back as query columns + * (the batch `DISTINCT ON`) or as one jsonb object (the correlated lateral folded + * into the table SELECT). The single source of truth for the doomed-count rule is + * {@link mapJobRow} — never re-derive `pendingDeleteRemaining` at a call site. + */ +export interface LatestJobRow { + id: string + type: string + status: string + rowsProcessed: number + error: string | null + /** + * The one field {@link mapJobRow} needs out of {@link TableDeleteJobPayload}, + * projected on its own so the read never drags the rest of the payload back. + * `null` when the job has no payload, or a payload without the key. + */ + doomedCount: number | null +} + +export function mapJobRow(row: LatestJobRow | null | undefined): DerivedJobFields { if (!row) return EMPTY_JOB_FIELDS - const doomedCount = - row.type === 'delete' && row.status === 'running' - ? ((row.payload as TableDeleteJobPayload | null)?.doomedCount ?? 0) - : 0 + const doomedCount = row.type === 'delete' && row.status === 'running' ? (row.doomedCount ?? 0) : 0 return { jobStatus: row.status as TableDefinition['jobStatus'], jobId: row.id, @@ -76,32 +82,69 @@ function mapJobRow( } } +/** + * The one number {@link mapJobRow} wants out of `table_jobs.payload`, extracted in + * SQL so the payload itself never crosses the wire. `payload` also carries the + * delete job's `filter` and an unbounded `excludeRowIds` array, and the latest + * non-export job is read on essentially every table request — a table that once ran + * a large delete would otherwise ship that id list on every read, forever. + * + * `->` (not `->>`) keeps the value jsonb: postgres-js decodes jsonb through its + * built-in `JSON.parse` handler (OID 3802), so this arrives as a JS `number`, or + * `null` for a missing payload, a payload without the key, a payload that is not an + * object, or an explicit JSON `null`. `->>` would hand back text and force a parse. + * All four null cases collapse to the same `?? 0` {@link mapJobRow} already applied + * to `payload?.doomedCount`, so no boundary coercion is needed. + */ +const doomedCountExpr = sql`${tableJobs.payload}->'doomedCount'` + +/** + * What {@link mapJobRow} reads, as one source for both job reads: the batch + * `DISTINCT ON` selects it directly, and {@link latestNonExportJobJson} derives its + * `jsonb_build_object` pairs from it. Adding or renaming a field here reaches both — + * the two cannot drift into disagreeing about what a job row is. Entries may be a + * plain `Column` or a derived `SQL` expression; both render in either position. + */ const JOB_PROJECTION = { id: tableJobs.id, type: tableJobs.type, status: tableJobs.status, rowsProcessed: tableJobs.rowsProcessed, error: tableJobs.error, - payload: tableJobs.payload, -} as const + doomedCount: doomedCountExpr, +} as const satisfies Record /** - * The latest job for one table (the running one if present, else the most recent terminal). - * Exports are excluded: they're read-only, run concurrently with other jobs, and have their own - * client surface — surfacing one here would clobber the import/delete/backfill status the tray - * and SSE consumer derive from these fields. + * The latest non-export job for one table, as a single jsonb value correlated to + * `outerTableId` — i.e. a `LEFT JOIN LATERAL (... LIMIT 1) ON true` expressed in the + * select list, which is the form drizzle can type without `leftJoinLateral`. + * + * It exists so {@link getTableById} stays ONE database round trip. With prepared + * statements disabled (PgBouncer transaction mode) every extra `await` is a full + * round trip, and `getTableById` is on essentially every table request. The job row + * cannot simply be skipped: a table's reported `rowCount` is the stored count minus + * this job's `pendingDeleteRemaining`, so the count and the job row are one read. + * + * Semantics match the batch {@link latestJobsForTables} exactly — same + * {@link JOB_PROJECTION} fields (which is `doomedCount` extracted from the payload, + * never the payload itself), exports excluded (they run concurrently and have their + * own client surface), newest `started_at` first, one row. `NULL` when the table has + * no such job; feed the result straight to {@link mapJobRow}. */ -export async function latestJobForTable( - tableId: string, - executor: DbOrTx = db -): Promise { - const [row] = await executor - .select(JOB_PROJECTION) - .from(tableJobs) - .where(and(eq(tableJobs.tableId, tableId), ne(tableJobs.type, 'export'))) - .orderBy(desc(tableJobs.startedAt)) - .limit(1) - return mapJobRow(row) +export function latestNonExportJobJson(outerTableId: Column): SQL { + // Keys come from JOB_PROJECTION, never from input, so `sql.raw` here cannot + // carry anything a caller controls. + const pairs = Object.entries(JOB_PROJECTION).flatMap(([key, expression]) => [ + sql.raw(`'${key}'`), + expression, + ]) + return sql`( + select jsonb_build_object(${sql.join(pairs, sql`, `)}) + from ${tableJobs} + where ${tableJobs.tableId} = ${outerTableId} and ${tableJobs.type} <> 'export' + order by ${tableJobs.startedAt} desc + limit 1 + )` } /** Latest non-export job per table for a batch of ids, via `DISTINCT ON (table_id)`. */ diff --git a/apps/sim/lib/table/rows/service.ts b/apps/sim/lib/table/rows/service.ts index 4ed64ae8824..4857338e68a 100644 --- a/apps/sim/lib/table/rows/service.ts +++ b/apps/sim/lib/table/rows/service.ts @@ -791,12 +791,13 @@ export async function upsertRow( }) if (!updatedRow) throw new Error('Matched table row no longer exists') - const executions = await loadExecutionsForRow(trx, updatedRow.id) + // No executions sidecar: no upsert surface puts one on the wire, and + // loading it here would hold the write transaction open for a result that + // is discarded. See `getRowSummaryById` for the same reasoning on reads. return { row: { id: updatedRow.id, data: updatedRow.data as RowData, - executions, position: updatedRow.position, orderKey: updatedRow.orderKey ?? undefined, createdAt: updatedRow.createdAt, @@ -842,7 +843,6 @@ export async function upsertRow( row: { id: insertedRow.id, data: insertedRow.data as RowData, - executions: {}, position: insertedRow.position, orderKey: insertedRow.orderKey ?? undefined, createdAt: insertedRow.createdAt, @@ -1409,20 +1409,11 @@ async function fetchRowsBounded(params: BoundedFetchParams): Promise { - const results = await db +/** The stored row without its executions sidecar. */ +export type TableRowSummary = Omit + +function selectRowRecord(tableId: string, rowId: string, workspaceId: string) { + return db .select() .from(userTableRows) .where( @@ -1433,15 +1424,12 @@ export async function getRowById( ) ) .limit(1) +} - if (results.length === 0) return null - - const row = results[0] - const executions = await loadExecutionsForRow(db, row.id) +function toRowSummary(row: Awaited>[number]): TableRowSummary { return { id: row.id, data: row.data as RowData, - executions, position: row.position, orderKey: row.orderKey ?? undefined, createdAt: row.createdAt, @@ -1449,6 +1437,45 @@ export async function getRowById( } } +/** + * One row without its executions sidecar, for the surfaces that never put + * executions on the wire — the single-row read routes and the Copilot row tool. + * Loading the sidecar for them is a query whose result is discarded. + * + * Deliberately a separate function rather than a flag on {@link getRowById}: a + * caller that forgets to pass the flag reads an empty sidecar and cannot tell + * that from a row with no executions, whereas here the field simply is not on + * the type. + */ +export async function getRowSummaryById( + tableId: string, + rowId: string, + workspaceId: string +): Promise { + const [row] = await selectRowRecord(tableId, rowId, workspaceId) + return row ? toRowSummary(row) : null +} + +/** One row with its executions sidecar, for the write and background paths. */ +export async function getRowById( + tableId: string, + rowId: string, + workspaceId: string +): Promise { + // The executions sidecar is keyed on the row id the caller already gave us, so + // it does not depend on the row lookup — issuing both together makes this one + // round trip instead of two. A miss pays one redundant sidecar read, which is + // the rare path and costs no extra wall time. + const [results, executions] = await Promise.all([ + selectRowRecord(tableId, rowId, workspaceId), + loadExecutionsForRow(db, rowId), + ]) + + if (results.length === 0) return null + + return { ...toRowSummary(results[0]), executions } +} + /** * Verifies an explicit row selection against the canonical table/workspace in * bounded database chunks without materializing the complete row set. @@ -1604,13 +1631,33 @@ export async function updateRow( ) } - // Check unique constraints using optimized database query - const uniqueColumns = getUniqueColumns(table.schema) - if (uniqueColumns.length > 0) { + // Scoped to the columns this patch actually writes. A merge cannot newly + // violate uniqueness on a column it leaves alone: that value is the one + // already stored, and it satisfied the constraint when it was written. The + // probe opens its own transaction and queries once per unique column, so on a + // table that has any unique column this was several round trips on every + // edit, including edits nowhere near one. + // + // What this does not cover is a duplicate that already exists — either from a + // constraint added to a column that already held one, or from two concurrent + // inserts both passing this probe, since uniqueness here is advisory (a + // SELECT, not a DB constraint). Such a row is no longer blocked from edits + // elsewhere in it. That is the intended outcome: an unrelated cell edit + // should not fail on data it did not write, and blocking it was never a + // repair mechanism. + const patchedColumnIds = new Set(Object.keys(data.data)) + const patchedUniqueColumns = getUniqueColumns(table.schema).filter((column) => + patchedColumnIds.has(getColumnId(column)) + ) + if (patchedUniqueColumns.length > 0) { const uniqueValidation = await checkUniqueConstraintsDb( data.tableId, mergedData, - table.schema, + // Narrowed to the patched unique columns, not just used as a gate: the + // probe issues one SELECT per unique column it is given, so a table with + // several would otherwise re-check all of them to validate a patch that + // touched one. `schema` is read only for its unique columns here. + { ...table.schema, columns: patchedUniqueColumns }, data.rowId // Exclude current row ) if (!uniqueValidation.valid) { diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index 5774caea7ab..ea6ade3ea84 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -1,8 +1,15 @@ /** * @vitest-environment node */ -import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { + dbChainMockFns, + hasMockCondition, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' import type { TableSchema } from '@/lib/table/types' vi.mock('@/lib/realtime/notify', () => ({ @@ -14,7 +21,7 @@ vi.mock('@/lib/table/billing', () => ({ notifyTableRowUsage: vi.fn(), })) -import { createTable } from '@/lib/table/service' +import { createTable, getTableById } from '@/lib/table/service' const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8' @@ -96,3 +103,173 @@ describe('createTable schema invariants', () => { expect(dbChainMockFns.insert).toHaveBeenCalled() }) }) + +const TABLE_ID = '0f2b1a4a-1e0e-4b4a-9a0f-0a2b3c4d5e6f' + +/** A `user_table_definitions` row as the folded SELECT returns it. */ +function definitionRow(overrides: Record = {}) { + return { + id: TABLE_ID, + name: 'contacts', + description: null, + schema: { columns: [{ id: 'col_email', name: 'email', type: 'string' }] }, + metadata: null, + maxRows: 10000, + workspaceId: WORKSPACE_ID, + folderId: null, + createdBy: 'user-1', + archivedAt: null, + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), + rowCount: 100, + latestJob: null, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + ...overrides, + } +} + +/** + * The job row is folded into the table SELECT as a lateral, so these cover both that + * one query still carries every job field and that `rowCount` stays adjusted by a + * running delete — the reason the two reads cannot be split apart. + */ +describe('getTableById job derivation', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('reads the table and its latest job in a single query', async () => { + queueTableRows(schemaMock.userTableDefinitions, [definitionRow()]) + + const table = await getTableById(TABLE_ID) + + expect(table).toMatchObject({ id: TABLE_ID, rowCount: 100 }) + expect(table).toMatchObject({ + jobStatus: null, + jobId: null, + jobType: null, + jobError: null, + jobRowsProcessed: 0, + }) + expect(table).not.toHaveProperty('pendingDeleteRemaining') + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + // double-cast-allowed: the mocked drizzle `sql` tag exposes the raw template parts + const projected = dbChainMockFns.select.mock.calls[0][0] as unknown as { + latestJob?: { strings: string[]; values: unknown[] } + } + expect(projected.latestJob?.strings.join(' ? ')).toContain("<> 'export'") + expect(projected.latestJob?.values).toContain(schemaMock.userTableDefinitions.id) + }) + + it("reduces rowCount by a running delete job's remaining doomed rows", async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + definitionRow({ + latestJob: { + id: 'job-1', + type: 'delete', + status: 'running', + rowsProcessed: 4, + error: null, + doomedCount: 10, + }, + }), + ]) + + const table = await getTableById(TABLE_ID) + + expect(table).toMatchObject({ + rowCount: 94, + jobId: 'job-1', + jobType: 'delete', + jobStatus: 'running', + jobRowsProcessed: 4, + }) + }) + + it('leaves rowCount alone for a running job that is not a delete', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + definitionRow({ + latestJob: { + id: 'job-2', + type: 'import', + status: 'running', + rowsProcessed: 4, + error: null, + doomedCount: 10, + }, + }), + ]) + + expect(await getTableById(TABLE_ID)).toMatchObject({ rowCount: 100, jobType: 'import' }) + }) + + it('leaves rowCount alone once the delete job is terminal', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + definitionRow({ + latestJob: { + id: 'job-3', + type: 'delete', + status: 'ready', + rowsProcessed: 4, + error: null, + doomedCount: 10, + }, + }), + ]) + + expect(await getTableById(TABLE_ID)).toMatchObject({ rowCount: 100, jobStatus: 'ready' }) + }) + + it('filters out archived tables unless includeArchived is set', async () => { + queueTableRows(schemaMock.userTableDefinitions, [definitionRow()]) + await getTableById(TABLE_ID) + expect( + hasMockCondition( + dbChainMockFns.where.mock.calls[0][0], + (node) => + node.type === 'isNull' && node.column === schemaMock.userTableDefinitions.archivedAt + ) + ).toBe(true) + + const archivedAt = new Date('2026-01-03T00:00:00Z') + queueTableRows(schemaMock.userTableDefinitions, [definitionRow({ archivedAt })]) + const archived = await getTableById(TABLE_ID, { includeArchived: true }) + + expect(archived).toMatchObject({ archivedAt }) + expect( + hasMockCondition( + dbChainMockFns.where.mock.calls[1][0], + (node) => + node.type === 'isNull' && node.column === schemaMock.userTableDefinitions.archivedAt + ) + ).toBe(false) + }) + + it('runs the single query on a supplied transaction executor', async () => { + const limit = vi.fn().mockResolvedValue([ + definitionRow({ + latestJob: { + id: 'job-4', + type: 'delete', + status: 'running', + rowsProcessed: 1, + error: null, + doomedCount: 5, + }, + }), + ]) + const select = vi.fn(() => ({ from: () => ({ where: () => ({ limit }) }) })) + const tx = { select } as unknown as DbOrTx + + const table = await getTableById(TABLE_ID, { tx }) + + expect(table).toMatchObject({ rowCount: 96, jobId: 'job-4' }) + expect(select).toHaveBeenCalledTimes(1) + expect(limit).toHaveBeenCalledWith(1) + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 95d3ff94939..42aa16fc4a9 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -38,7 +38,12 @@ import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' import { generateColumnId, getColumnId, withGeneratedColumnIds } from '@/lib/table/column-keys' import { COLUMN_TYPES, NAME_PATTERN, TABLE_LIMITS } from '@/lib/table/constants' import { appendTableEvent } from '@/lib/table/events' -import { EMPTY_JOB_FIELDS, latestJobForTable, latestJobsForTables } from '@/lib/table/jobs/service' +import { + EMPTY_JOB_FIELDS, + latestJobsForTables, + latestNonExportJobJson, + mapJobRow, +} from '@/lib/table/jobs/service' import { assertSchemaMutable, TableLockedError } from '@/lib/table/mutation-locks' import { nKeysBetween } from '@/lib/table/order-key' import type { DbTransaction } from '@/lib/table/planner' @@ -168,6 +173,12 @@ function applyColumnOrderToSchema( /** * Gets a table by ID with full details. * + * One round trip: the table's latest non-export job comes back in the same SELECT as + * a correlated lateral ({@link latestNonExportJobJson}) rather than a second query. + * The two cannot be separated — the reported `rowCount` is the stored count minus the + * running delete job's `pendingDeleteRemaining`, so dropping the job read would + * overstate the count mid-delete and let over-capacity inserts through. + * * @param tableId - Table ID to fetch * @returns Table definition or null if not found */ @@ -192,6 +203,7 @@ export async function getTableById( createdAt: userTableDefinitions.createdAt, updatedAt: userTableDefinitions.updatedAt, rowCount: userTableDefinitions.rowCount, + latestJob: latestNonExportJobJson(userTableDefinitions.id), ...LOCK_SELECT, }) .from(userTableDefinitions) @@ -206,7 +218,7 @@ export async function getTableById( const table = results[0] const metadata = (table.metadata as TableMetadata) ?? null - const { pendingDeleteRemaining, ...jobFields } = await latestJobForTable(tableId, executor) + const { pendingDeleteRemaining, ...jobFields } = mapJobRow(table.latestJob) return { id: table.id, name: table.name, diff --git a/apps/sim/lib/table/trigger.ts b/apps/sim/lib/table/trigger.ts index cdb2b2a3169..a08ebc093a0 100644 --- a/apps/sim/lib/table/trigger.ts +++ b/apps/sim/lib/table/trigger.ts @@ -51,7 +51,9 @@ export async function fireTableTrigger( tableId: string, tableName: string, eventType: EventType, - rows: TableRow[], + // Accepts a row without its executions sidecar: the payload projects id and + // data only, and the upsert path deliberately does not load one. + rows: Array>, oldRows: Map | null, schema: TableSchema, requestId: string diff --git a/apps/sim/lib/table/types.ts b/apps/sim/lib/table/types.ts index 1e11fc1f857..b0950443756 100644 --- a/apps/sim/lib/table/types.ts +++ b/apps/sim/lib/table/types.ts @@ -718,7 +718,11 @@ export interface UpsertRowData { } export interface UpsertResult { - row: TableRow + /** + * Without the executions sidecar: no upsert surface puts one on the wire, and + * loading it would hold the write transaction open for a discarded result. + */ + row: Omit operation: 'insert' | 'update' previousData?: RowData } diff --git a/apps/sim/tools/table/delete_row.ts b/apps/sim/tools/table/delete_row.ts index 47d46d699aa..fa339c07c49 100644 --- a/apps/sim/tools/table/delete_row.ts +++ b/apps/sim/tools/table/delete_row.ts @@ -23,6 +23,7 @@ export const tableDeleteRowTool: ToolConfig `/api/table/${params.tableId}/rows/${params.rowId}`, method: 'DELETE', headers: () => ({ diff --git a/apps/sim/tools/table/get_row.ts b/apps/sim/tools/table/get_row.ts index 7b76e605fda..010dcc53af5 100644 --- a/apps/sim/tools/table/get_row.ts +++ b/apps/sim/tools/table/get_row.ts @@ -23,6 +23,7 @@ export const tableGetRowTool: ToolConfig = }, request: { + internalAuth: 'executor_delegation', secretProvenance: { response: { incomplete: 'propagate' } }, url: (params: TableRowGetParams) => { const workspaceId = params._context?.workspaceId diff --git a/apps/sim/tools/table/update_row.ts b/apps/sim/tools/table/update_row.ts index c9792f95680..24f781cf042 100644 --- a/apps/sim/tools/table/update_row.ts +++ b/apps/sim/tools/table/update_row.ts @@ -38,6 +38,7 @@ export const tableUpdateRowTool: ToolConfig selectTableRowSecretProvenance([params.data]), response: { incomplete: 'propagate' }, diff --git a/apps/sim/tools/table/upsert_row.ts b/apps/sim/tools/table/upsert_row.ts index 70afc179872..7a62a79ef82 100644 --- a/apps/sim/tools/table/upsert_row.ts +++ b/apps/sim/tools/table/upsert_row.ts @@ -39,6 +39,7 @@ export const tableUpsertRowTool: ToolConfig selectTableRowSecretProvenance([params.data]), response: { incomplete: 'propagate' }, diff --git a/packages/testing/src/factories/index.ts b/packages/testing/src/factories/index.ts index 586f7fea59b..a5f1e00ac24 100644 --- a/packages/testing/src/factories/index.ts +++ b/packages/testing/src/factories/index.ts @@ -118,6 +118,19 @@ export { type SerializedConnection, type SerializedWorkflow, } from './serialized-block.factory' +export { + createTableColumn, + createTableDefinition, + createTableRow, + type TableColumnFactoryOptions, + type TableColumnFixture, + type TableColumnType, + type TableDefinitionFactoryOptions, + type TableDefinitionFixture, + type TableLocksFixture, + type TableRowFactoryOptions, + type TableRowFixture, +} from './table.factory' // Tool mock responses export { mockDriveResponses, diff --git a/packages/testing/src/factories/table.factory.ts b/packages/testing/src/factories/table.factory.ts index 020787735ec..65ccf7dbfde 100644 --- a/packages/testing/src/factories/table.factory.ts +++ b/packages/testing/src/factories/table.factory.ts @@ -2,9 +2,18 @@ import { generateShortId } from '@sim/utils/id' const COLUMN_SUFFIX_ALPHABET = 'abcdefghijklmnopqrstuvwxyz0123456789_' -export type TableColumnType = 'string' | 'number' | 'boolean' | 'date' | 'json' +export type TableColumnType = + | 'string' + | 'number' + | 'currency' + | 'boolean' + | 'date' + | 'json' + | 'select' export interface TableColumnFixture { + /** Stable storage key. Absent on legacy columns, where the name is the key. */ + id?: string name: string type: TableColumnType required?: boolean @@ -20,6 +29,7 @@ export interface TableRowFixture { } export interface TableColumnFactoryOptions { + id?: string name?: string type?: TableColumnType required?: boolean @@ -39,6 +49,7 @@ export interface TableRowFactoryOptions { */ export function createTableColumn(options: TableColumnFactoryOptions = {}): TableColumnFixture { return { + id: options.id, name: options.name ?? `column_${generateShortId(6, COLUMN_SUFFIX_ALPHABET)}`, type: options.type ?? 'string', required: options.required, @@ -60,3 +71,88 @@ export function createTableRow(options: TableRowFactoryOptions = {}): TableRowFi updatedAt: options.updatedAt ?? timestamp, } } + +/** Per-table mutation locks. All false means fully unlocked. */ +export interface TableLocksFixture { + schemaLocked: boolean + insertLocked: boolean + updateLocked: boolean + deleteLocked: boolean +} + +/** + * Structural stand-in for `TableDefinition` in `apps/sim/lib/table/types.ts`. + * Declared here rather than imported because `@sim/testing` must not depend on + * `apps/*` (enforced by `scripts/check-monorepo-boundaries.ts`). + */ +export interface TableDefinitionFixture { + id: string + name: string + description: string | null + schema: { columns: TableColumnFixture[] } + metadata: Record | null + rowCount: number + maxRows: number + workspaceId: string + folderId?: string | null + createdBy: string + locks: TableLocksFixture + archivedAt: Date | string | null + createdAt: Date | string + updatedAt: Date | string +} + +export interface TableDefinitionFactoryOptions { + id?: string + name?: string + description?: string | null + /** Shorthand for `schema.columns` — the field call sites vary most. */ + columns?: TableColumnFixture[] + metadata?: Record | null + rowCount?: number + maxRows?: number + workspaceId?: string + folderId?: string | null + createdBy?: string + locks?: TableLocksFixture + archivedAt?: Date | string | null + createdAt?: Date | string + updatedAt?: Date | string +} + +const UNLOCKED_TABLE_LOCKS: TableLocksFixture = Object.freeze({ + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, +}) + +/** + * Creates a table definition fixture with sensible defaults — the shape route + * and service tests hand back from a table lookup. + * + * Callers most often override `columns` (the table's schema), `rowCount`, + * `maxRows`, and `archivedAt`. + */ +export function createTableDefinition( + options: TableDefinitionFactoryOptions = {} +): TableDefinitionFixture { + const timestamp = new Date() + + return { + id: options.id ?? 'tbl_1', + name: options.name ?? 'People', + description: options.description ?? null, + schema: { columns: options.columns ?? [] }, + metadata: options.metadata ?? null, + rowCount: options.rowCount ?? 0, + maxRows: options.maxRows ?? 1_000_000, + workspaceId: options.workspaceId ?? 'workspace-1', + folderId: options.folderId, + createdBy: options.createdBy ?? 'user-1', + locks: options.locks ?? UNLOCKED_TABLE_LOCKS, + archivedAt: options.archivedAt ?? null, + createdAt: options.createdAt ?? timestamp, + updatedAt: options.updatedAt ?? timestamp, + } +}