Skip to content

Commit 2eb706a

Browse files
committed
fix(execution): address review regressions
1 parent 0acc905 commit 2eb706a

5 files changed

Lines changed: 133 additions & 2 deletions

File tree

apps/sim/app/api/mcp/tools/execute/route.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ describe('MCP tool execution private secret provenance', () => {
171171
const response = await POST(request, {})
172172
const body = (await response.json()) as Record<string, unknown>
173173

174+
expect(response.status).toBe(500)
175+
expect(response.ok).toBe(false)
174176
expect(body).toEqual({
175177
success: false,
176178
error: 'Internal MCP response could not be verified',

apps/sim/app/api/mcp/tools/execute/route.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ async function attachPrivateProvenance(
7272
provenance: ResolvedSecretTraceProvenanceAccumulator
7373
): Promise<NextResponse> {
7474
let payload: Record<string, unknown>
75+
let status = response.status
7576
try {
7677
const body = await readResponseToBufferWithLimit(response, {
7778
maxBytes: MAX_PRIVATE_MCP_RESPONSE_BYTES,
@@ -85,6 +86,7 @@ async function attachPrivateProvenance(
8586
payload = parsed as Record<string, unknown>
8687
} catch {
8788
payload = { success: false, error: 'Internal MCP response could not be verified' }
89+
status = 500
8890
provenance.markIncomplete({ discardEntries: true })
8991
}
9092

@@ -93,7 +95,7 @@ async function attachPrivateProvenance(
9395
headers.set(PRIVATE_TOOL_METADATA_RESPONSE_HEADER, RESOLVED_SECRET_PROVENANCE_METADATA_V1)
9496
return NextResponse.json(
9597
{ ...payload, [RESOLVED_SECRET_PROVENANCE_FIELD]: provenance.exportProvenance() },
96-
{ status: response.status, headers }
98+
{ status, headers }
9799
)
98100
}
99101

apps/sim/app/api/workflows/[id]/execute/route.async.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,49 @@ describe('workflow execute async route', () => {
374374
)
375375
})
376376

377+
it('recovers legacy starter input by execution ID without returning it to the client', async () => {
378+
const sourceInput = { token: 'legacy-retry-input', nested: { value: 42 } }
379+
queueTableRows(schemaMock.workflowExecutionLogs, [
380+
{
381+
executionId: 'source-execution',
382+
workflowId: 'workflow-1',
383+
workspaceId: 'workspace-1',
384+
executionData: {
385+
executionState: {
386+
blockStates: {
387+
start: {
388+
output: sourceInput,
389+
executed: false,
390+
executionTime: 0,
391+
},
392+
},
393+
},
394+
},
395+
},
396+
])
397+
const request = createMockRequest(
398+
'POST',
399+
{ inputFromExecutionId: 'source-execution' },
400+
{
401+
'Content-Type': 'application/json',
402+
'X-Execution-Mode': 'async',
403+
Cookie: 'session=value',
404+
}
405+
)
406+
407+
const response = await POST(request, { params: Promise.resolve({ id: 'workflow-1' }) })
408+
const responseBody = await response.json()
409+
410+
expect(response.status).toBe(202)
411+
expect(responseBody).not.toHaveProperty('input')
412+
expect(JSON.stringify(responseBody)).not.toContain('legacy-retry-input')
413+
expect(mockEnqueue).toHaveBeenCalledWith(
414+
'workflow-execution',
415+
expect.objectContaining({ input: sourceInput }),
416+
expect.any(Object)
417+
)
418+
})
419+
377420
it('rejects client input alongside a stored execution input reference', async () => {
378421
const response = await POST(
379422
createMockRequest(

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,66 @@ describe('execution state lookup', () => {
112112
})
113113
})
114114

115+
it('recovers legacy workflow input from the pre-populated starter block state', async () => {
116+
const legacyInput = { leadId: 'legacy-lead' }
117+
queueTableRows(schemaMock.workflowExecutionLogs, [
118+
{
119+
executionId: 'execution-1',
120+
workflowId: 'workflow-1',
121+
workspaceId: 'workspace-1',
122+
executionData: {},
123+
},
124+
])
125+
mockMaterializeExecutionData.mockResolvedValueOnce({
126+
executionState: {
127+
blockStates: {
128+
'executed-block': {
129+
output: { leadId: 'wrong-lead' },
130+
executed: true,
131+
executionTime: 10,
132+
},
133+
start: {
134+
output: legacyInput,
135+
executed: false,
136+
executionTime: 0,
137+
},
138+
},
139+
},
140+
})
141+
142+
const result = await getExecutionInputForWorkflow('execution-1', 'workflow-1')
143+
144+
expect(result).toEqual({ found: true, input: legacyInput })
145+
})
146+
147+
it('prefers persisted workflow input over the legacy starter block state', async () => {
148+
const workflowInput = { leadId: 'current-lead' }
149+
queueTableRows(schemaMock.workflowExecutionLogs, [
150+
{
151+
executionId: 'execution-1',
152+
workflowId: 'workflow-1',
153+
workspaceId: 'workspace-1',
154+
executionData: {},
155+
},
156+
])
157+
mockMaterializeExecutionData.mockResolvedValueOnce({
158+
workflowInput,
159+
executionState: {
160+
blockStates: {
161+
start: {
162+
output: { leadId: 'legacy-lead' },
163+
executed: false,
164+
executionTime: 0,
165+
},
166+
},
167+
},
168+
})
169+
170+
const result = await getExecutionInputForWorkflow('execution-1', 'workflow-1')
171+
172+
expect(result).toEqual({ found: true, input: workflowInput })
173+
})
174+
115175
it('checks older pointer-backed candidates when the latest has no execution state', async () => {
116176
queueTableRows(schemaMock.workflowExecutionLogs, [
117177
{

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { db } from '@sim/db'
22
import { workflowExecutionLogs } from '@sim/db/schema'
3+
import { isRecordLike } from '@sim/utils/object'
34
import { and, desc, eq, or, sql } from 'drizzle-orm'
45
import { materializeExecutionData, TRACE_STORE_REF_KEY } from '@/lib/logs/execution/trace-store'
56
import type { SerializableExecutionState } from '@/executor/execution/types'
@@ -30,6 +31,25 @@ function extractExecutionState(executionData: unknown): SerializableExecutionSta
3031
return isSerializableExecutionState(state) ? state : null
3132
}
3233

34+
function extractLegacyWorkflowInput(executionData: Record<string, unknown>): unknown | undefined {
35+
if (!isRecordLike(executionData.executionState)) return undefined
36+
const { blockStates } = executionData.executionState
37+
if (!isRecordLike(blockStates)) return undefined
38+
39+
for (const state of Object.values(blockStates)) {
40+
if (
41+
isRecordLike(state) &&
42+
state.executed === false &&
43+
state.executionTime === 0 &&
44+
state.output != null
45+
) {
46+
return state.output
47+
}
48+
}
49+
50+
return undefined
51+
}
52+
3353
interface ExecutionStateRow {
3454
executionId: string
3555
workflowId: string | null
@@ -109,7 +129,11 @@ export async function getExecutionInputForWorkflow(
109129
}
110130

111131
const data = await materializeExecutionDataFromRow(row)
112-
return { found: true, input: data?.workflowInput }
132+
if (!data) return { found: true }
133+
if (Object.hasOwn(data, 'workflowInput')) {
134+
return { found: true, input: data.workflowInput }
135+
}
136+
return { found: true, input: extractLegacyWorkflowInput(data) }
113137
}
114138

115139
export async function getLatestExecutionStateWithExecutionId(

0 commit comments

Comments
 (0)