Skip to content

Commit ecae9b8

Browse files
TheodoreSpeaksTheodoreSpeaks
committed
fix(tables): complete fixed copilot composition
1 parent e52c047 commit ecae9b8

9 files changed

Lines changed: 388 additions & 32 deletions

File tree

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({ execute: vi.fn() }))
7+
8+
vi.mock('@/lib/workflows/application/resolve-workflow-outputs', () => ({
9+
resolveWorkflowOutputs: { execute: mocks.execute },
10+
}))
11+
12+
import { executeCopilotResolveWorkflowOutputs } from '@/lib/copilot/application/execute-workflow-use-case'
13+
14+
const trustedContext = {
15+
userId: 'user-1',
16+
workspaceId: 'workspace-1',
17+
chatId: 'chat-1',
18+
executionId: 'execution-1',
19+
toolCallId: 'tool-call-1',
20+
copilotToolExecution: true,
21+
} as const
22+
23+
describe('executeCopilotResolveWorkflowOutputs', () => {
24+
afterEach(() => {
25+
vi.clearAllMocks()
26+
vi.useRealTimers()
27+
})
28+
29+
it('enters the fixed Workflow resolver with trusted Copilot identity', async () => {
30+
vi.useFakeTimers()
31+
vi.setSystemTime(new Date('2026-01-01T00:00:00Z'))
32+
mocks.execute.mockResolvedValueOnce({
33+
workflowId: 'workflow-1',
34+
outputs: null,
35+
executionOrderByBlockId: {},
36+
})
37+
38+
await expect(
39+
executeCopilotResolveWorkflowOutputs(trustedContext, {
40+
workflowId: 'workflow-1',
41+
assertedWorkspaceId: 'workspace-1',
42+
})
43+
).resolves.toMatchObject({ workflowId: 'workflow-1' })
44+
45+
expect(mocks.execute).toHaveBeenCalledWith({
46+
principal: {
47+
kind: 'delegated',
48+
serviceId: 'copilot',
49+
subjectUserId: 'user-1',
50+
workspaceId: 'workspace-1',
51+
delegationId: 'copilot-tool:tool-call-1',
52+
audience: 'sim:workflows',
53+
issuedAt: new Date('2026-01-01T00:00:00Z'),
54+
expiresAt: new Date('2026-01-01T00:05:00Z'),
55+
resourceScope: { chatId: 'chat-1', executionId: 'execution-1' },
56+
},
57+
input: { workflowId: 'workflow-1', assertedWorkspaceId: 'workspace-1' },
58+
})
59+
})
60+
61+
it('rejects untrusted context before Workflow application execution', () => {
62+
expect(() =>
63+
executeCopilotResolveWorkflowOutputs(
64+
{ ...trustedContext, copilotToolExecution: false },
65+
{ workflowId: 'workflow-1', assertedWorkspaceId: 'workspace-1' }
66+
)
67+
).toThrow('trusted Copilot execution context')
68+
expect(mocks.execute).not.toHaveBeenCalled()
69+
})
70+
})

‎apps/sim/lib/copilot/application/execute-workflow-use-case.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,35 @@
1-
import { createCopilotWorkspaceUseCaseExecutor } from '@/lib/copilot/application/execute-workspace-use-case'
2-
import { WORKFLOW_DELEGATION_AUDIENCE } from '@/lib/workflows/application/authorization'
3-
import { workflowOperations } from '@/lib/workflows/application/operations'
1+
import {
2+
COPILOT_APPLICATION_DELEGATION_TTL_MS,
3+
type CopilotExecutionContext,
4+
createCopilotApplicationPrincipal,
5+
requireTrustedCopilotExecutionContext,
6+
} from '@/lib/copilot/auth/application-delegation'
7+
import { workflowDelegationPolicy } from '@/lib/workflows/application/authorization'
8+
import {
9+
type ResolveWorkflowOutputsInput,
10+
type ResolveWorkflowOutputsResult,
11+
resolveWorkflowOutputs,
12+
} from '@/lib/workflows/application/resolve-workflow-outputs'
413

5-
export const executeCopilotWorkflowUseCase = createCopilotWorkspaceUseCaseExecutor({
6-
audience: WORKFLOW_DELEGATION_AUDIENCE,
7-
operations: workflowOperations,
8-
})
14+
export type CopilotWorkflowDelegationContext = CopilotExecutionContext
15+
16+
const workflowDelegation = {
17+
audience: workflowDelegationPolicy.audience,
18+
ttlMs: COPILOT_APPLICATION_DELEGATION_TTL_MS,
19+
createDelegationId: (context: Parameters<typeof createCopilotApplicationPrincipal>[0]) =>
20+
`copilot-tool:${context.toolCallId}`,
21+
} as const
22+
23+
/** Resolves workflow output metadata through one fixed authorized Workflow command. */
24+
export function executeCopilotResolveWorkflowOutputs(
25+
context: CopilotWorkflowDelegationContext | undefined,
26+
input: ResolveWorkflowOutputsInput
27+
): Promise<ResolveWorkflowOutputsResult> {
28+
return resolveWorkflowOutputs.execute({
29+
principal: createCopilotApplicationPrincipal(
30+
requireTrustedCopilotExecutionContext(context),
31+
workflowDelegation
32+
),
33+
input,
34+
})
35+
}

‎apps/sim/lib/copilot/application/table-commands.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({
99
createEnrichment: vi.fn(),
1010
createFromFile: vi.fn(),
1111
createWorkflowGroup: vi.fn(),
12+
deleteTables: vi.fn(),
1213
importFile: vi.fn(),
1314
replaceProjectedRows: vi.fn(),
1415
resolvePrincipal: vi.fn(),
@@ -24,6 +25,9 @@ vi.mock('@/lib/table/application/groups', () => ({
2425
createWorkflowTableGroup: { execute: mocks.createWorkflowGroup },
2526
updateWorkflowTableGroup: { execute: mocks.updateWorkflowGroup },
2627
}))
28+
vi.mock('@/lib/table/application/copilot-table-lifecycle', () => ({
29+
deleteCopilotTables: { execute: mocks.deleteTables },
30+
}))
2731
vi.mock('@/lib/table/application/rows', () => ({
2832
replaceProjectedWireRows: { execute: mocks.replaceProjectedRows },
2933
}))
@@ -37,13 +41,15 @@ import {
3741
copilotCreateTableEnrichmentGroupPolicy,
3842
copilotCreateTableFromWorkspaceFilePolicy,
3943
copilotCreateWorkflowTableGroupPolicy,
44+
copilotDeleteTablesPolicy,
4045
copilotImportWorkspaceFileIntoTablePolicy,
4146
copilotReplaceProjectedWireRowsPolicy,
4247
copilotUpdateWorkflowTableGroupPolicy,
4348
executeCopilotAddWorkflowTableGroupOutput,
4449
executeCopilotCreateTableEnrichmentGroup,
4550
executeCopilotCreateTableFromWorkspaceFile,
4651
executeCopilotCreateWorkflowTableGroup,
52+
executeCopilotDeleteTables,
4753
executeCopilotImportWorkspaceFileIntoTable,
4854
executeCopilotReplaceProjectedWireRows,
4955
executeCopilotUpdateWorkflowTableGroup,
@@ -94,10 +100,25 @@ describe('fixed Copilot Table application commands', () => {
94100
expect(mocks.createFromFile).toHaveBeenCalledWith({ principal, input })
95101
})
96102

103+
it('uses one workspace-scoped Table command for best-effort multi-table deletion', async () => {
104+
mocks.deleteTables.mockResolvedValue({ deleted: ['table-1'], failed: ['table-2'] })
105+
const input = { workspaceId: 'workspace-1', tableIds: ['table-1', 'table-2'] }
106+
107+
await expect(executeCopilotDeleteTables(context, input)).resolves.toEqual({
108+
deleted: ['table-1'],
109+
failed: ['table-2'],
110+
})
111+
112+
expect(mocks.resolvePrincipal).toHaveBeenCalledWith(context)
113+
expect(mocks.deleteTables).toHaveBeenCalledWith({ principal, input })
114+
expect(mocks.deleteTables).toHaveBeenCalledTimes(1)
115+
})
116+
97117
it('declares inherited request-rate admission and no direct provider cost for every command', () => {
98118
const policies = [
99119
copilotReplaceProjectedWireRowsPolicy,
100120
copilotCreateWorkflowTableGroupPolicy,
121+
copilotDeleteTablesPolicy,
101122
copilotUpdateWorkflowTableGroupPolicy,
102123
copilotAddWorkflowTableGroupOutputPolicy,
103124
copilotCreateTableEnrichmentGroupPolicy,

‎apps/sim/lib/copilot/application/table-commands.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import type { CopilotTableDelegationContext } from '@/lib/copilot/auth/table-delegation'
22
import { resolveCopilotTablePrincipal } from '@/lib/copilot/auth/table-delegation'
3+
import {
4+
type DeleteCopilotTablesInput,
5+
deleteCopilotTables,
6+
} from '@/lib/table/application/copilot-table-lifecycle'
37
import {
48
type AddTableGroupOutputInput,
59
addWorkflowTableGroupOutput,
@@ -31,6 +35,21 @@ const NO_DIRECT_PROVIDER_COST_POLICY = {
3135
reason: 'This command does not invoke a paid provider; table quota and storage limits apply.',
3236
} as const
3337

38+
export const copilotDeleteTablesPolicy = {
39+
rate: INHERITED_COPILOT_RATE_POLICY,
40+
cost: NO_DIRECT_PROVIDER_COST_POLICY,
41+
} as const
42+
43+
export function executeCopilotDeleteTables(
44+
context: CopilotTableDelegationContext | undefined,
45+
input: DeleteCopilotTablesInput
46+
) {
47+
return deleteCopilotTables.execute({
48+
principal: resolveCopilotTablePrincipal(context),
49+
input,
50+
})
51+
}
52+
3453
export const copilotReplaceProjectedWireRowsPolicy = {
3554
rate: INHERITED_COPILOT_RATE_POLICY,
3655
cost: NO_DIRECT_PROVIDER_COST_POLICY,

‎apps/sim/lib/copilot/auth/table-delegation.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,9 @@ describe('Copilot table delegation', () => {
4141
resolveCopilotTablePrincipal({ ...context, toolCallId: undefined }, 'table-1')
4242
).toThrow('tool call ID')
4343
})
44+
45+
it('rejects an empty table scope before principal construction', () => {
46+
expect(() => resolveCopilotTablePrincipal(context, '')).toThrow('non-empty table ID')
47+
expect(() => resolveCopilotTablePrincipal(context, ' ')).toThrow('non-empty table ID')
48+
})
4449
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ vi.mock('@/lib/copilot/application/execute-file-use-case', () => ({
130130
}))
131131

132132
vi.mock('@/lib/copilot/application/execute-workflow-use-case', () => ({
133-
executeCopilotWorkflowUseCase: mockExecuteCopilotWorkflowUseCase,
133+
executeCopilotResolveWorkflowOutputs: mockExecuteCopilotWorkflowUseCase,
134134
}))
135135

136136
vi.mock('@sim/platform-authz/workspace', () => ({

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

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3-
import { executeCopilotWorkflowUseCase } from '@/lib/copilot/application/execute-workflow-use-case'
3+
import { executeCopilotResolveWorkflowOutputs } from '@/lib/copilot/application/execute-workflow-use-case'
44
import {
55
executeCopilotAddWorkflowTableGroupOutput,
66
executeCopilotCreateTableEnrichmentGroup,
77
executeCopilotCreateTableFromWorkspaceFile,
88
executeCopilotCreateWorkflowTableGroup,
9+
executeCopilotDeleteTables,
910
executeCopilotImportWorkspaceFileIntoTable,
1011
executeCopilotUpdateWorkflowTableGroup,
1112
} from '@/lib/copilot/application/table-commands'
@@ -46,7 +47,6 @@ import {
4647
import { cancelTableRuns, startTableRun } from '@/lib/table/application/runs'
4748
import {
4849
createTableUseCase,
49-
deleteTableUseCase,
5050
readTableUseCase,
5151
updateTableUseCase,
5252
} from '@/lib/table/application/tables'
@@ -66,7 +66,6 @@ import type {
6666
WorkflowGroupDependencies,
6767
WorkflowGroupDeploymentMode,
6868
} from '@/lib/table/types'
69-
import { resolveWorkflowOutputs } from '@/lib/workflows/application/resolve-workflow-outputs'
7069

7170
const logger = createLogger('UserTableServerTool')
7271

@@ -88,7 +87,7 @@ function resolveAuthorizedWorkflowOutputs(
8887
workspaceId: string,
8988
context: ServerToolContext
9089
) {
91-
return executeCopilotWorkflowUseCase(context, resolveWorkflowOutputs, {
90+
return executeCopilotResolveWorkflowOutputs(context, {
9291
workflowId,
9392
assertedWorkspaceId: workspaceId,
9493
})
@@ -267,26 +266,11 @@ export const userTableServerTool: BaseServerTool<UserTableArgs, UserTableResult>
267266
return { success: false, message: 'Workspace ID is required' }
268267
}
269268

270-
const deleted: string[] = []
271-
const failed: string[] = []
272-
273-
for (const tableId of tableIds) {
274-
try {
275-
assertNotAborted()
276-
await deleteTableUseCase.execute({
277-
principal: tablePrincipal(tableId),
278-
input: { tableId, workspaceId },
279-
})
280-
deleted.push(tableId)
281-
} catch (error) {
282-
const classified = messageForCopilotTableError(error, '')
283-
if (classified === 'Table not found') {
284-
failed.push(tableId)
285-
continue
286-
}
287-
throw error
288-
}
289-
}
269+
assertNotAborted()
270+
const { deleted, failed } = await executeCopilotDeleteTables(context, {
271+
tableIds,
272+
workspaceId,
273+
})
290274

291275
return {
292276
success: deleted.length > 0,

0 commit comments

Comments
 (0)