Skip to content

Commit fc7e35e

Browse files
fix(tables): enforce plan limits in mothership user_table tool (#4832)
* fix(tables): enforce plan limits in mothership user_table tool * improvement(tables): truncate over-limit CSV imports to the plan cap instead of rejecting * fix(tables): log rollback failures and surface a clear reason on failed CSV import
1 parent 8aa74a2 commit fc7e35e

2 files changed

Lines changed: 146 additions & 5 deletions

File tree

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

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ const {
1212
mockBatchInsertRows,
1313
mockReplaceTableRows,
1414
mockAddWorkflowGroup,
15+
mockCreateTable,
16+
mockDeleteTable,
17+
mockGetWorkspaceTableLimits,
1518
fakeEnrichment,
1619
} = vi.hoisted(() => ({
1720
mockResolveWorkspaceFileReference: vi.fn(),
@@ -20,6 +23,9 @@ const {
2023
mockBatchInsertRows: vi.fn(),
2124
mockReplaceTableRows: vi.fn(),
2225
mockAddWorkflowGroup: vi.fn(),
26+
mockCreateTable: vi.fn(),
27+
mockDeleteTable: vi.fn(),
28+
mockGetWorkspaceTableLimits: vi.fn(),
2329
fakeEnrichment: {
2430
id: 'work-email',
2531
name: 'Work Email',
@@ -54,13 +60,13 @@ vi.mock('@/lib/table/service', () => ({
5460
addWorkflowGroup: mockAddWorkflowGroup,
5561
batchInsertRows: mockBatchInsertRows,
5662
batchUpdateRows: vi.fn(),
57-
createTable: vi.fn(),
63+
createTable: mockCreateTable,
5864
deleteColumn: vi.fn(),
5965
deleteColumns: vi.fn(),
6066
deleteRow: vi.fn(),
6167
deleteRowsByFilter: vi.fn(),
6268
deleteRowsByIds: vi.fn(),
63-
deleteTable: vi.fn(),
69+
deleteTable: mockDeleteTable,
6470
getRowById: vi.fn(),
6571
getTableById: mockGetTableById,
6672
insertRow: vi.fn(),
@@ -74,6 +80,10 @@ vi.mock('@/lib/table/service', () => ({
7480
updateRowsByFilter: vi.fn(),
7581
}))
7682

83+
vi.mock('@/lib/table/billing', () => ({
84+
getWorkspaceTableLimits: mockGetWorkspaceTableLimits,
85+
}))
86+
7787
import { userTableServerTool } from '@/lib/copilot/tools/server/table/user-table'
7888

7989
function buildTable(overrides: Partial<TableDefinition> = {}): TableDefinition {
@@ -232,6 +242,93 @@ describe('userTableServerTool.import_file', () => {
232242
})
233243
})
234244

245+
describe('userTableServerTool.create_from_file', () => {
246+
beforeEach(() => {
247+
vi.clearAllMocks()
248+
mockResolveWorkspaceFileReference.mockResolvedValue({ name: 'people.csv', type: 'text/csv' })
249+
mockDownloadWorkspaceFile.mockResolvedValue(Buffer.from('name,age\nAlice,30\nBob,40'))
250+
mockGetWorkspaceTableLimits.mockResolvedValue({ maxRowsPerTable: 1000, maxTables: 3 })
251+
mockCreateTable.mockResolvedValue(buildTable({ id: 'tbl_new', name: 'people' }))
252+
mockDeleteTable.mockResolvedValue(undefined)
253+
mockBatchInsertRows.mockImplementation(async (data: { rows: unknown[] }) =>
254+
data.rows.map((_, i) => ({ id: `row_${i}` }))
255+
)
256+
})
257+
258+
it('stamps the workspace plan limits on the created table', async () => {
259+
const result = await userTableServerTool.execute(
260+
{ operation: 'create_from_file', args: { fileId: 'file-1' } },
261+
{ userId: 'user-1', workspaceId: 'workspace-1' }
262+
)
263+
264+
expect(result.success).toBe(true)
265+
expect(mockGetWorkspaceTableLimits).toHaveBeenCalledWith('workspace-1')
266+
expect(mockCreateTable).toHaveBeenCalledTimes(1)
267+
const createArgs = mockCreateTable.mock.calls[0][0] as { maxRows: number; maxTables: number }
268+
expect(createArgs.maxRows).toBe(1000)
269+
expect(createArgs.maxTables).toBe(3)
270+
})
271+
272+
it('truncates to the plan row limit and reports dropped rows', async () => {
273+
// File has 2 data rows (Alice, Bob); plan cap is 1.
274+
mockGetWorkspaceTableLimits.mockResolvedValueOnce({ maxRowsPerTable: 1, maxTables: 3 })
275+
276+
const result = await userTableServerTool.execute(
277+
{ operation: 'create_from_file', args: { fileId: 'file-1' } },
278+
{ userId: 'user-1', workspaceId: 'workspace-1' }
279+
)
280+
281+
expect(result.success).toBe(true)
282+
expect(mockCreateTable).toHaveBeenCalledTimes(1)
283+
const insertCall = mockBatchInsertRows.mock.calls[0][0] as { rows: unknown[] }
284+
expect(insertCall.rows).toHaveLength(1)
285+
expect(result.data?.rowCount).toBe(1)
286+
expect(result.message).toMatch(/dropped 1 row/i)
287+
expect(mockDeleteTable).not.toHaveBeenCalled()
288+
})
289+
290+
it('rolls back the created table and reports the reason when row insertion fails', async () => {
291+
mockBatchInsertRows.mockRejectedValueOnce(new Error('Row 2: Column "email" must be unique'))
292+
293+
const result = await userTableServerTool.execute(
294+
{ operation: 'create_from_file', args: { fileId: 'file-1' } },
295+
{ userId: 'user-1', workspaceId: 'workspace-1' }
296+
)
297+
298+
expect(result.success).toBe(false)
299+
expect(mockDeleteTable).toHaveBeenCalledWith('tbl_new', expect.any(String))
300+
expect(result.message).toMatch(/rolled back/i)
301+
expect(result.message).toMatch(/must be unique/i)
302+
})
303+
})
304+
305+
describe('userTableServerTool.create', () => {
306+
beforeEach(() => {
307+
vi.clearAllMocks()
308+
mockGetWorkspaceTableLimits.mockResolvedValue({ maxRowsPerTable: 1000, maxTables: 3 })
309+
mockCreateTable.mockResolvedValue(buildTable({ id: 'tbl_new', name: 'People' }))
310+
})
311+
312+
it('stamps the workspace plan limits on the created table', async () => {
313+
const result = await userTableServerTool.execute(
314+
{
315+
operation: 'create',
316+
args: {
317+
name: 'People',
318+
schema: { columns: [{ name: 'name', type: 'string', required: true }] },
319+
},
320+
},
321+
{ userId: 'user-1', workspaceId: 'workspace-1' }
322+
)
323+
324+
expect(result.success).toBe(true)
325+
expect(mockGetWorkspaceTableLimits).toHaveBeenCalledWith('workspace-1')
326+
const createArgs = mockCreateTable.mock.calls[0][0] as { maxRows: number; maxTables: number }
327+
expect(createArgs.maxRows).toBe(1000)
328+
expect(createArgs.maxTables).toBe(3)
329+
})
330+
})
331+
235332
describe('userTableServerTool.list_enrichments', () => {
236333
beforeEach(() => {
237334
vi.clearAllMocks()

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
type CsvHeaderMapping,
1515
CsvImportValidationError,
1616
coerceRowsForTable,
17+
getWorkspaceTableLimits,
1718
inferSchemaFromCsv,
1819
parseCsvBuffer,
1920
sanitizeName,
@@ -263,13 +264,16 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
263264

264265
const requestId = generateId().slice(0, 8)
265266
assertNotAborted()
267+
const planLimits = await getWorkspaceTableLimits(workspaceId)
266268
const table = await createTable(
267269
{
268270
name: args.name,
269271
description: args.description,
270272
schema: args.schema,
271273
workspaceId,
272274
userId: context.userId,
275+
maxRows: planLimits.maxRowsPerTable,
276+
maxTables: planLimits.maxTables,
273277
},
274278
requestId
275279
)
@@ -761,31 +765,71 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
761765
const tableName = args.name || file.name.replace(/\.[^.]+$/, '')
762766
const requestId = generateId().slice(0, 8)
763767
assertNotAborted()
768+
const planLimits = await getWorkspaceTableLimits(workspaceId)
769+
770+
const droppedRows = Math.max(0, rows.length - planLimits.maxRowsPerTable)
771+
const rowsToImport = droppedRows > 0 ? rows.slice(0, planLimits.maxRowsPerTable) : rows
772+
764773
const table = await createTable(
765774
{
766775
name: tableName,
767776
description: args.description || `Imported from ${file.name}`,
768777
schema: { columns },
769778
workspaceId,
770779
userId: context.userId,
780+
maxRows: planLimits.maxRowsPerTable,
781+
maxTables: planLimits.maxTables,
771782
},
772783
requestId
773784
)
774785

775-
const coerced = coerceRowsForTable(rows, { columns }, headerToColumn)
776-
const inserted = await batchInsertAll(table.id, coerced, table, workspaceId, context)
786+
const coerced = coerceRowsForTable(rowsToImport, { columns }, headerToColumn)
787+
let inserted: number
788+
try {
789+
inserted = await batchInsertAll(table.id, coerced, table, workspaceId, context)
790+
} catch (insertError) {
791+
const cleanupRequestId = generateId().slice(0, 8)
792+
await deleteTable(table.id, cleanupRequestId).catch((cleanupError) => {
793+
logger.error('Failed to roll back table after import failure', {
794+
tableId: table.id,
795+
error: toError(cleanupError).message,
796+
})
797+
})
798+
const reason = toError(insertError).message
799+
const cause =
800+
insertError instanceof Error && insertError.cause
801+
? toError(insertError.cause).message
802+
: undefined
803+
logger.error('Failed to import rows into new table', {
804+
tableId: table.id,
805+
fileName: file.name,
806+
error: reason,
807+
cause,
808+
})
809+
return {
810+
success: false,
811+
message: `Failed to import rows from "${file.name}" — the table was rolled back. ${cause ? `${reason} (${cause})` : reason}`,
812+
}
813+
}
777814

778815
logger.info('Table created from file', {
779816
tableId: table.id,
780817
fileName: file.name,
781818
columns: columns.length,
782819
rows: inserted,
820+
droppedRows,
783821
userId: context.userId,
784822
})
785823

824+
const createdMessage = `Created table "${table.name}" with ${columns.length} columns and ${inserted.toLocaleString()} rows from "${file.name}"`
825+
const message =
826+
droppedRows > 0
827+
? `${createdMessage}. Dropped ${droppedRows.toLocaleString()} row(s) that exceed this plan's limit of ${planLimits.maxRowsPerTable.toLocaleString()} rows per table.`
828+
: createdMessage
829+
786830
return {
787831
success: true,
788-
message: `Created table "${table.name}" with ${columns.length} columns and ${inserted} rows from "${file.name}"`,
832+
message,
789833
data: {
790834
tableId: table.id,
791835
tableName: table.name,

0 commit comments

Comments
 (0)