Skip to content

Commit 32dc7a2

Browse files
fix(tables): restore scoped copilot imports
1 parent a5b72b2 commit 32dc7a2

6 files changed

Lines changed: 102 additions & 16 deletions

File tree

apps/sim/lib/copilot/application/execute-table-use-case.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,29 @@ describe('executeCopilotTableUseCase', () => {
4242
audience: 'sim:tables',
4343
issuedAt: new Date('2026-01-01T00:00:00Z'),
4444
expiresAt: new Date('2026-01-01T00:05:00Z'),
45-
resourceScope: { chatId: 'chat-1', executionId: 'execution-1' },
45+
resourceScope: {
46+
chatId: 'chat-1',
47+
executionId: 'execution-1',
48+
tableId: 'table-1',
49+
},
4650
},
4751
input: { tableId: 'table-1', workspaceId: 'workspace-1' },
4852
})
4953
})
5054

55+
it('fails fast when a table-scoped input has no valid table id', () => {
56+
const execute = vi.fn()
57+
58+
expect(() =>
59+
executeCopilotTableUseCase(
60+
trustedContext,
61+
{ operation: tableOperations.read, execute },
62+
{ tableId: '', workspaceId: 'workspace-1' }
63+
)
64+
).toThrow('invalid table ID')
65+
expect(execute).not.toHaveBeenCalled()
66+
})
67+
5168
it('rejects untrusted Copilot context before application execution', () => {
5269
const execute = vi.fn()
5370

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
834834
columns,
835835
headerToColumn,
836836
rows,
837+
assertNotAborted,
837838
})
838839
if (result.kind !== 'inline') {
839840
throw new Error('Inline table import returned a background result')
@@ -921,6 +922,7 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
921922
sourceFile: record,
922923
mode,
923924
mapping: rawMapping,
925+
assertNotAborted,
924926
loadRows: async () => {
925927
const { content } = await resolveWorkspaceFileRecordOrThrow(
926928
fileReference,

apps/sim/lib/table/application/authorization.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('table operation authorization', () => {
122122
)
123123
})
124124

125-
it('rejects wrong-audience, expired, cross-workspace, and wrong-table delegations before lookup', async () => {
125+
it('rejects wrong-audience, expired, cross-workspace, unscoped, and wrong-table delegations before lookup', async () => {
126126
const base = {
127127
kind: 'delegated' as const,
128128
serviceId: 'copilot' as const,
@@ -150,6 +150,10 @@ describe('table operation authorization', () => {
150150
expiresAt: new Date(Date.now() + 60_000),
151151
resourceScope: { tableId: 'table-1' },
152152
})
153+
await expectForbidden({
154+
...base,
155+
expiresAt: new Date(Date.now() + 60_000),
156+
})
153157
await expectForbidden({
154158
...base,
155159
expiresAt: new Date(Date.now() + 60_000),

apps/sim/lib/table/application/authorization.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@ export const tableDelegationPolicy: WorkspaceDelegationPolicy<TableAuthorization
2424
principal: Extract<Principal, { kind: 'delegated' }>,
2525
context: TableAuthorizationContext
2626
) {
27-
return (
28-
principal.resourceScope?.tableId === undefined ||
29-
principal.resourceScope.tableId === context.tableId
30-
)
27+
return context.tableId === undefined || principal.resourceScope?.tableId === context.tableId
3128
},
3229
}
3330

apps/sim/lib/table/application/workspace-file-imports.test.ts

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const mocks = vi.hoisted(() => ({
1616
resolvePermission: vi.fn(),
1717
resolveWorkspaceContext: vi.fn(),
1818
signal: vi.fn(),
19+
validateMapping: vi.fn(),
20+
CsvImportValidationError: class extends Error {},
1921
}))
2022

2123
vi.mock('@sim/audit', () => ({
@@ -39,14 +41,11 @@ vi.mock('@/lib/table', () => ({
3941
batchInsertRows: mocks.batchInsert,
4042
buildAutoMapping: vi.fn(() => ({ name: 'name' })),
4143
coerceRowsForTable: (rows: unknown[]) => rows,
44+
CsvImportValidationError: mocks.CsvImportValidationError,
4245
CSV_MAX_BATCH_SIZE: 1000,
4346
getWorkspaceTableLimits: vi.fn(() => ({ maxRowsPerTable: 100, maxTables: 5 })),
4447
replaceTableRows: vi.fn(),
45-
validateMapping: vi.fn(() => ({
46-
effectiveMap: new Map([['name', 'name']]),
47-
mappedHeaders: ['name'],
48-
skippedHeaders: [],
49-
})),
48+
validateMapping: mocks.validateMapping,
5049
}))
5150
vi.mock('@/lib/table/application/context', () => ({
5251
resolveActiveTableContext: mocks.resolveTableContext,
@@ -94,6 +93,7 @@ const principal = {
9493
audience: 'sim:tables',
9594
issuedAt: new Date('2026-08-01T00:00:00.000Z'),
9695
expiresAt: new Date('2099-08-01T00:00:00.000Z'),
96+
resourceScope: { tableId: 'table-1' },
9797
}
9898
const input = {
9999
kind: 'inline' as const,
@@ -136,6 +136,11 @@ describe('Copilot workspace-file table creation', () => {
136136
mocks.batchInsert.mockResolvedValue([{ id: 'row-1' }])
137137
mocks.markJob.mockResolvedValue(true)
138138
mocks.releaseJob.mockResolvedValue(true)
139+
mocks.validateMapping.mockReturnValue({
140+
effectiveMap: new Map([['name', 'name']]),
141+
mappedHeaders: ['name'],
142+
skippedHeaders: [],
143+
})
139144
})
140145

141146
it('owns table creation, row insertion, audit, and shared effects', async () => {
@@ -201,6 +206,53 @@ describe('Copilot workspace-file table creation', () => {
201206
expect(events).toEqual(['claim', 'load', 'mutate', 'release'])
202207
})
203208

209+
it('checks for a user stop after loading and before every inline insert batch', async () => {
210+
const assertNotAborted = vi.fn()
211+
212+
await importWorkspaceFileIntoTable.execute({
213+
principal,
214+
input: {
215+
kind: 'inline',
216+
tableId: 'table-1',
217+
assertedWorkspaceId: 'workspace-1',
218+
sourceFile: input.sourceFile,
219+
mode: 'append',
220+
assertNotAborted,
221+
loadRows: async () => ({
222+
headers: ['name'],
223+
rows: Array.from({ length: 1001 }, (_, index) => ({ name: `Person ${index}` })),
224+
}),
225+
},
226+
})
227+
228+
expect(assertNotAborted).toHaveBeenCalledTimes(3)
229+
expect(mocks.batchInsert).toHaveBeenCalledTimes(2)
230+
})
231+
232+
it('classifies mapping failures before mutation so Copilot can correct them', async () => {
233+
mocks.validateMapping.mockImplementationOnce(() => {
234+
throw new mocks.CsvImportValidationError('Mapping references an unknown column')
235+
})
236+
237+
await expect(
238+
importWorkspaceFileIntoTable.execute({
239+
principal,
240+
input: {
241+
kind: 'inline',
242+
tableId: 'table-1',
243+
assertedWorkspaceId: 'workspace-1',
244+
sourceFile: input.sourceFile,
245+
mode: 'append',
246+
loadRows: async () => ({ headers: ['name'], rows: [{ name: 'Ada' }] }),
247+
},
248+
})
249+
).rejects.toMatchObject({
250+
code: 'validation',
251+
message: 'Mapping references an unknown column',
252+
})
253+
expect(mocks.batchInsert).not.toHaveBeenCalled()
254+
})
255+
204256
it('rolls back and propagates unknown insertion failures without audit or effects', async () => {
205257
const failure = new Error('database unavailable')
206258
mocks.batchInsert.mockRejectedValueOnce(failure)

apps/sim/lib/table/application/workspace-file-imports.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
type ColumnDefinition,
1313
CSV_MAX_BATCH_SIZE,
1414
type CsvHeaderMapping,
15+
CsvImportValidationError,
1516
coerceRowsForTable,
1617
getWorkspaceTableLimits,
1718
type RowData,
@@ -60,6 +61,7 @@ export type CreateTableFromWorkspaceFileInput = CreateTableFromWorkspaceFileBase
6061
columns: ColumnDefinition[]
6162
headerToColumn: Map<string, string>
6263
rows: Record<string, unknown>[]
64+
assertNotAborted?: () => void
6365
}
6466
)
6567

@@ -94,6 +96,7 @@ export type ImportWorkspaceFileInput = ImportWorkspaceFileBaseInput &
9496
| {
9597
kind: 'inline'
9698
loadRows: () => Promise<{ headers: string[]; rows: Record<string, unknown>[] }>
99+
assertNotAborted?: () => void
97100
}
98101
)
99102

@@ -148,9 +151,11 @@ async function batchInsertAll(params: {
148151
rows: RowData[]
149152
workspaceId: string
150153
userId: string
154+
assertNotAborted?: () => void
151155
}): Promise<number> {
152156
let inserted = 0
153157
for (let index = 0; index < params.rows.length; index += CSV_MAX_BATCH_SIZE) {
158+
params.assertNotAborted?.()
154159
const batch = params.rows.slice(index, index + CSV_MAX_BATCH_SIZE)
155160
const result = await batchInsertRows(
156161
{
@@ -303,6 +308,7 @@ export const createTableFromWorkspaceFile = defineAuthorizedTableUseCase({
303308
rows: coerceRowsForTable(rows, table.schema, input.headerToColumn),
304309
workspaceId: context.workspaceId,
305310
userId,
311+
assertNotAborted: input.assertNotAborted,
306312
})
307313
return {
308314
kind: input.kind,
@@ -391,15 +397,22 @@ export const importWorkspaceFileIntoTable = defineAuthorizedTableUseCase({
391397
throw new OrchestrationError('conflict', 'A job is already in progress for this table')
392398
return withReleasedTableJobClaim(context.table.id, context.workspaceId, jobId, async () => {
393399
const { headers, rows: sourceRows } = await input.loadRows()
400+
input.assertNotAborted?.()
394401
if (sourceRows.length === 0) {
395402
return { kind: 'empty', table: context.table, mode: input.mode }
396403
}
397404
const mapping = input.mapping ?? buildAutoMapping(headers, context.table.schema)
398-
const validation = validateMapping({
399-
csvHeaders: headers,
400-
mapping,
401-
tableSchema: context.table.schema,
402-
})
405+
let validation: ReturnType<typeof validateMapping>
406+
try {
407+
validation = validateMapping({
408+
csvHeaders: headers,
409+
mapping,
410+
tableSchema: context.table.schema,
411+
})
412+
} catch (error) {
413+
if (!(error instanceof CsvImportValidationError)) throw error
414+
throw new OrchestrationError('validation', error.message)
415+
}
403416
if (validation.mappedHeaders.length === 0) {
404417
throw new OrchestrationError(
405418
'validation',
@@ -435,6 +448,7 @@ export const importWorkspaceFileIntoTable = defineAuthorizedTableUseCase({
435448
rows,
436449
workspaceId: context.workspaceId,
437450
userId,
451+
assertNotAborted: input.assertNotAborted,
438452
})
439453
return {
440454
kind: input.kind,

0 commit comments

Comments
 (0)