Skip to content

Commit ea06a77

Browse files
committed
fix(copilot): trust compacted workflow completion
1 parent f9dddb4 commit ea06a77

9 files changed

Lines changed: 232 additions & 23 deletions

File tree

apps/sim/app/api/copilot/confirm/route.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -910,13 +910,18 @@ describe('Copilot Confirm API Route', () => {
910910
})
911911
})
912912

913-
it('derives workflow outcome from the terminal server execution', async () => {
913+
it('derives workflow outcome from a content-unavailable trusted terminal execution', async () => {
914914
getAsyncToolCall.mockResolvedValue({
915915
...existingRow,
916916
toolName: 'run_workflow',
917917
args: { workflowId: 'workflow-1' },
918918
})
919-
getTrustedWorkflowToolExecution.mockResolvedValueOnce({ status: 'failed' })
919+
getTrustedWorkflowToolExecution.mockResolvedValueOnce({
920+
executionId: 'execution-1',
921+
workflowId: 'workflow-1',
922+
status: 'failed',
923+
contentAvailable: false,
924+
})
920925

921926
const response = await POST(
922927
createMockPostRequest({

apps/sim/lib/copilot/request/tools/client.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ function trustedExecution(executionId: string) {
7979
executionId,
8080
workflowId: 'workflow-1',
8181
status: 'completed' as const,
82+
contentAvailable: true as const,
8283
finalOutput: { value: `child read parent-secret-value from ${executionId}` },
8384
blockLogs: [],
8485
provenance: {
@@ -169,6 +170,39 @@ describe('workflow client tool completion', () => {
169170
expect(JSON.stringify(completion)).not.toContain('untrusted')
170171
})
171172

173+
it('uses compacted terminal status without exposing unavailable execution content', async () => {
174+
const registry = createParentRegistry()
175+
waitForToolConfirmation.mockResolvedValue({
176+
status: 'success',
177+
data: { workflowId: 'workflow-1', executionId: 'execution-1' },
178+
})
179+
getTrustedWorkflowToolExecution.mockResolvedValue({
180+
executionId: 'execution-1',
181+
workflowId: 'workflow-1',
182+
status: 'failed',
183+
contentAvailable: false,
184+
})
185+
186+
const completion = await waitForWorkflowToolCompletion({
187+
toolCallId: 'tool-1',
188+
workflowId: 'workflow-1',
189+
timeoutMs: 1_000,
190+
registry,
191+
})
192+
193+
expect(completion).toEqual({
194+
status: 'error',
195+
message: 'Workflow execution failed.',
196+
data: {
197+
success: false,
198+
workflowId: 'workflow-1',
199+
executionId: 'execution-1',
200+
},
201+
})
202+
expect(registry.isComplete()).toBe(false)
203+
expect(replaceTerminalAsyncToolCallResult).not.toHaveBeenCalled()
204+
})
205+
172206
it('preserves cancellation when the bound terminal execution is not yet readable', async () => {
173207
const registry = createParentRegistry()
174208
waitForToolConfirmation.mockResolvedValue({
@@ -271,6 +305,7 @@ describe('workflow client tool completion', () => {
271305
executionId: 'execution-1',
272306
workflowId: 'workflow-1',
273307
status: 'completed',
308+
contentAvailable: true,
274309
finalOutput: { value: 'child-secret-value' },
275310
blockLogs: [],
276311
provenance: {
@@ -309,6 +344,7 @@ describe('workflow client tool completion', () => {
309344
executionId: 'execution-1',
310345
workflowId: 'workflow-1',
311346
status: 'failed',
347+
contentAvailable: true,
312348
error: 'trusted failure',
313349
blockLogs: [],
314350
provenance: { version: 1, complete: true, entries: [], scope: TRACE_SCOPE },

apps/sim/lib/copilot/request/tools/client.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,15 @@ export async function waitForWorkflowToolCompletion({
245245
return structuralWorkflowCompletion(completion.status, workflowId, executionId)
246246
}
247247

248+
if (!trustedExecution.contentAvailable) {
249+
registry?.markIncomplete()
250+
return structuralWorkflowCompletion(
251+
getWorkflowToolConfirmationStatus(trustedExecution.status),
252+
workflowId,
253+
executionId
254+
)
255+
}
256+
248257
if (!registry || registry.isPermanentlyIncomplete() || !trustedExecution.provenance.complete) {
249258
if (!trustedExecution.provenance.complete) registry?.markIncomplete()
250259
return structuralWorkflowCompletion(

apps/sim/lib/logs/execution/logger.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,42 @@ describe('ExecutionLogger', () => {
380380
expect(compacted.traceSpans?.[0]?.children?.[0]).not.toHaveProperty('input')
381381
})
382382

383+
test('retains the trusted Copilot binding in metadata-only compaction', () => {
384+
const loggerInstance = new ExecutionLogger() as unknown as {
385+
compactExecutionDataForStorage(
386+
executionData: WorkflowExecutionLog['executionData'],
387+
executionId: string
388+
): WorkflowExecutionLog['executionData']
389+
}
390+
const correlation = {
391+
executionId: 'execution-metadata-only',
392+
requestId: 'request-1',
393+
source: 'workflow' as const,
394+
workflowId: 'workflow-1',
395+
copilotToolCallId: 'tool-call-1',
396+
}
397+
398+
const compacted = loggerInstance.compactExecutionDataForStorage(
399+
{
400+
environment: {
401+
variables: { OVERSIZED: 'x'.repeat(3.5 * 1024 * 1024) },
402+
workflowId: 'workflow-1',
403+
executionId: 'execution-metadata-only',
404+
userId: 'user-1',
405+
workspaceId: 'workspace-1',
406+
},
407+
correlation,
408+
hasTraceSpans: false,
409+
traceSpanCount: 0,
410+
},
411+
'execution-metadata-only'
412+
)
413+
414+
expect(compacted.executionDataTruncated).toBe(true)
415+
expect(compacted.correlation).toEqual(correlation)
416+
expect(compacted).not.toHaveProperty('environment')
417+
})
418+
383419
test('retains tool-call structure when aggregate trace content exceeds the compaction cap', () => {
384420
const loggerInstance = new ExecutionLogger() as unknown as {
385421
compactExecutionDataForStorage(

apps/sim/lib/logs/execution/logger.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
491491
...(executionData.billingAttribution
492492
? { billingAttribution: executionData.billingAttribution }
493493
: {}),
494+
...(executionData.correlation ? { correlation: executionData.correlation } : {}),
494495
hasTraceSpans: executionData.hasTraceSpans,
495496
traceSpanCount: executionData.traceSpanCount,
496497
tokens: executionData.tokens,

apps/sim/lib/logs/execution/trace-store.test.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,27 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { decryptSecretMock } = vi.hoisted(() => ({
6+
const { decryptSecretMock, materializeLargeValueRefMock, storeLargeValueMock } = vi.hoisted(() => ({
77
decryptSecretMock: vi.fn(),
8+
materializeLargeValueRefMock: vi.fn(),
9+
storeLargeValueMock: vi.fn(),
810
}))
911

1012
vi.mock('@/lib/core/security/encryption', () => ({
1113
decryptSecret: decryptSecretMock,
1214
}))
1315

14-
import { projectExecutionDataForDisplay } from '@/lib/logs/execution/trace-store'
16+
vi.mock('@/lib/execution/payloads/store', () => ({
17+
materializeLargeValueRef: materializeLargeValueRefMock,
18+
storeLargeValue: storeLargeValueMock,
19+
}))
20+
21+
import {
22+
externalizeExecutionData,
23+
materializeExecutionData,
24+
projectExecutionDataForDisplay,
25+
TRACE_STORE_REF_KEY,
26+
} from '@/lib/logs/execution/trace-store'
1527

1628
const CONTEXT = {
1729
workspaceId: 'workspace-1',
@@ -25,6 +37,55 @@ beforeEach(() => {
2537
decryptSecretMock.mockResolvedValue({ decrypted: '1234' })
2638
})
2739

40+
describe('execution data storage', () => {
41+
it('keeps the trusted Copilot binding when an externalized payload is unavailable', async () => {
42+
const correlation = { copilotToolCallId: 'tool-call-1' }
43+
const ref = {
44+
__simLargeValueRef: true,
45+
version: 1,
46+
id: 'lv_bbbbbbbbbbbb',
47+
kind: 'object',
48+
size: 128,
49+
key: 'execution/workspace-1/workflow-1/execution-1/large-value-lv_bbbbbbbbbbbb.json',
50+
executionId: 'execution-1',
51+
preview: { unsafe: 'must-not-remain-inline' },
52+
} as const
53+
storeLargeValueMock.mockResolvedValue(ref)
54+
materializeLargeValueRefMock.mockRejectedValue(new Error('object unavailable'))
55+
56+
const slim = await externalizeExecutionData(
57+
{
58+
correlation,
59+
hasTraceSpans: true,
60+
traceSpanCount: 2,
61+
finalOutput: { unsafe: 'must-not-remain-inline' },
62+
},
63+
CONTEXT
64+
)
65+
66+
expect(slim).toEqual({
67+
[TRACE_STORE_REF_KEY]: {
68+
__simLargeValueRef: true,
69+
version: 1,
70+
id: 'lv_bbbbbbbbbbbb',
71+
kind: 'object',
72+
size: 128,
73+
key: 'execution/workspace-1/workflow-1/execution-1/large-value-lv_bbbbbbbbbbbb.json',
74+
executionId: 'execution-1',
75+
},
76+
correlation,
77+
hasTraceSpans: true,
78+
traceSpanCount: 2,
79+
})
80+
81+
await expect(materializeExecutionData(slim, CONTEXT)).resolves.toEqual({
82+
correlation,
83+
hasTraceSpans: true,
84+
traceSpanCount: 2,
85+
})
86+
})
87+
})
88+
2889
describe('projectExecutionDataForDisplay', () => {
2990
it('projects persisted output, input, errors, and spans from trusted provenance', async () => {
3091
const executionData = {

apps/sim/lib/logs/execution/trace-store.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@ export const TRACE_STORE_REF_KEY = 'traceStoreRef'
2020

2121
/**
2222
* The only metadata kept inline on the slim row (everything else lives in the
23-
* externalized object). These two describe trace presence/count and uniquely
24-
* survive object expiry — so a reader can still report "trace data expired (N
25-
* spans)" after retention without an object fetch. All other fields
23+
* externalized object). Trace presence/count survives object expiry for log
24+
* diagnostics, while correlation preserves the server-issued binding used to
25+
* authenticate terminal Copilot workflow-tool executions. All other fields
2626
* (environment, trigger, tokens, models, truncation flags, and of course the
27-
* heavy payloads) are in the stored object and recovered on materialize, so
28-
* keeping them inline too would just be duplication.
27+
* heavy payloads) are recovered from the stored object.
2928
*/
30-
const INLINE_MARKER_KEYS = ['hasTraceSpans', 'traceSpanCount'] as const
29+
const INLINE_MARKER_KEYS = ['hasTraceSpans', 'traceSpanCount', 'correlation'] as const
3130

3231
/**
3332
* Read-path context. Resolves an externalized payload by storage key, authorized

apps/sim/lib/workflows/executor/execution-state.test.ts

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ describe('execution state lookup', () => {
104104
executionId: 'execution-1',
105105
workflowId: 'workflow-1',
106106
status: 'completed',
107+
contentAvailable: true,
107108
finalOutput: { token: 'raw-secret' },
108109
blockLogs: [],
109110
provenance,
@@ -157,7 +158,7 @@ describe('execution state lookup', () => {
157158
})
158159
})
159160

160-
it('rejects mismatched bindings, malformed provenance, and nonterminal rows', async () => {
161+
it('trusts compacted terminal status while withholding unavailable execution content', async () => {
161162
queueTableRows(schemaMock.workflowExecutionLogs, [
162163
{
163164
executionId: 'execution-1',
@@ -168,20 +169,31 @@ describe('execution state lookup', () => {
168169
},
169170
])
170171
mockMaterializeExecutionData.mockResolvedValueOnce({
171-
correlation: { copilotToolCallId: 'another-tool-call' },
172-
executionState: {
173-
...EXECUTION_STATE,
174-
resolvedSecretTraceProvenance: { version: 1, complete: true, entries: [] },
172+
correlation: { copilotToolCallId: 'tool-call-1' },
173+
executionStateSummary: {
174+
executedBlockCount: 1,
175+
blockLogCount: 1,
176+
completedLoopCount: 0,
177+
activeExecutionPathLength: 0,
178+
pendingQueueLength: 0,
175179
},
180+
finalOutput: { token: 'must-not-cross' },
176181
})
177182

178183
await expect(
179184
getTrustedWorkflowToolExecution('execution-1', 'workflow-1', 'tool-call-1')
180-
).resolves.toBeNull()
185+
).resolves.toEqual({
186+
executionId: 'execution-1',
187+
workflowId: 'workflow-1',
188+
status: 'completed',
189+
contentAvailable: false,
190+
})
191+
})
181192

193+
it('withholds execution content when persisted provenance is malformed', async () => {
182194
queueTableRows(schemaMock.workflowExecutionLogs, [
183195
{
184-
executionId: 'execution-2',
196+
executionId: 'execution-1',
185197
workflowId: 'workflow-1',
186198
workspaceId: 'workspace-1',
187199
status: 'completed',
@@ -190,19 +202,48 @@ describe('execution state lookup', () => {
190202
])
191203
mockMaterializeExecutionData.mockResolvedValueOnce({
192204
correlation: { copilotToolCallId: 'tool-call-1' },
205+
finalOutput: { token: 'must-not-cross' },
193206
executionState: {
194207
...EXECUTION_STATE,
195208
resolvedSecretTraceProvenance: { version: 2, complete: true, entries: [] },
196209
},
197210
})
198211

199212
await expect(
200-
getTrustedWorkflowToolExecution('execution-2', 'workflow-1', 'tool-call-1')
213+
getTrustedWorkflowToolExecution('execution-1', 'workflow-1', 'tool-call-1')
214+
).resolves.toEqual({
215+
executionId: 'execution-1',
216+
workflowId: 'workflow-1',
217+
status: 'completed',
218+
contentAvailable: false,
219+
})
220+
})
221+
222+
it('rejects mismatched bindings and nonterminal rows', async () => {
223+
queueTableRows(schemaMock.workflowExecutionLogs, [
224+
{
225+
executionId: 'execution-1',
226+
workflowId: 'workflow-1',
227+
workspaceId: 'workspace-1',
228+
status: 'completed',
229+
executionData: {},
230+
},
231+
])
232+
mockMaterializeExecutionData.mockResolvedValueOnce({
233+
correlation: { copilotToolCallId: 'another-tool-call' },
234+
executionState: {
235+
...EXECUTION_STATE,
236+
resolvedSecretTraceProvenance: { version: 1, complete: true, entries: [] },
237+
},
238+
})
239+
240+
await expect(
241+
getTrustedWorkflowToolExecution('execution-1', 'workflow-1', 'tool-call-1')
201242
).resolves.toBeNull()
202243

203244
queueTableRows(schemaMock.workflowExecutionLogs, [
204245
{
205-
executionId: 'execution-3',
246+
executionId: 'execution-2',
206247
workflowId: 'workflow-1',
207248
workspaceId: 'workspace-1',
208249
status: 'running',
@@ -211,7 +252,7 @@ describe('execution state lookup', () => {
211252
])
212253

213254
await expect(
214-
getTrustedWorkflowToolExecution('execution-3', 'workflow-1', 'tool-call-1')
255+
getTrustedWorkflowToolExecution('execution-2', 'workflow-1', 'tool-call-1')
215256
).resolves.toBeNull()
216257
})
217258

0 commit comments

Comments
 (0)