Skip to content

Commit 13c6741

Browse files
icecrasher321icecrasher321
committed
address comments
1 parent 76509e1 commit 13c6741

28 files changed

Lines changed: 1091 additions & 308 deletions

‎apps/sim/app/api/function/execute/route.test.ts‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import {
1212
import { NextRequest } from 'next/server'
1313
import { beforeEach, describe, expect, it, vi } from 'vitest'
1414

15-
const { mockExecuteInE2B, mockExecuteInIsolatedVM } = vi.hoisted(() => ({
15+
const { mockExecuteInE2B, mockExecuteInIsolatedVM, mockUploadFile } = vi.hoisted(() => ({
1616
mockExecuteInE2B: vi.fn(),
1717
mockExecuteInIsolatedVM: vi.fn(),
18+
mockUploadFile: vi.fn(),
1819
}))
1920

2021
vi.mock('@/lib/execution/isolated-vm', () => ({
@@ -42,11 +43,20 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
4243
uploadWorkspaceFile: vi.fn(),
4344
}))
4445

46+
vi.mock('@/lib/uploads', () => ({
47+
StorageService: {
48+
uploadFile: mockUploadFile,
49+
},
50+
}))
51+
4552
vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
4653

4754
vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
4855

4956
import { validateProxyUrl } from '@/lib/core/security/input-validation'
57+
import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
58+
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata'
59+
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
5060
import { POST } from '@/app/api/function/execute/route'
5161

5262
describe('Function Execute API Route', () => {
@@ -61,6 +71,8 @@ describe('Function Execute API Route', () => {
6171
})
6272

6373
mockExecuteInIsolatedVM.mockResolvedValue({ result: 'test', stdout: '' })
74+
mockUploadFile.mockImplementation(async ({ customKey }) => ({ key: customKey }))
75+
clearLargeValueCacheForTests()
6476

6577
mockExecuteInE2B.mockResolvedValue({
6678
result: 'e2b success',
@@ -202,6 +214,60 @@ describe('Function Execute API Route', () => {
202214
expect(data.output).toHaveProperty('executionTime')
203215
})
204216

217+
it('compacts large array result fields to manifests when execution context is durable', async () => {
218+
mockExecuteInIsolatedVM.mockResolvedValueOnce({
219+
result: {
220+
rows: Array.from({ length: 120_000 }, (_, index) => ({
221+
key: `SIM-${index}`,
222+
payload: 'x'.repeat(100),
223+
})),
224+
},
225+
stdout: '',
226+
})
227+
228+
const req = createMockRequest('POST', {
229+
code: 'return rows',
230+
workflowId: 'workflow-1',
231+
workspaceId: 'workspace-1',
232+
executionId: 'execution-1',
233+
})
234+
235+
const response = await POST(req)
236+
const data = await response.json()
237+
238+
expect(response.status).toBe(200)
239+
expect(data.success).toBe(true)
240+
expect(isLargeArrayManifest(data.output.result.rows)).toBe(true)
241+
expect(data.output.result.rows).toMatchObject({
242+
__simLargeArrayManifest: true,
243+
kind: 'array',
244+
totalCount: 120_000,
245+
})
246+
})
247+
248+
it('keeps large string result fields as generic large value refs', async () => {
249+
mockExecuteInIsolatedVM.mockResolvedValueOnce({
250+
result: {
251+
text: 'x'.repeat(9 * 1024 * 1024),
252+
},
253+
stdout: '',
254+
})
255+
256+
const req = createMockRequest('POST', {
257+
code: 'return text',
258+
workflowId: 'workflow-1',
259+
workspaceId: 'workspace-1',
260+
executionId: 'execution-1',
261+
})
262+
263+
const response = await POST(req)
264+
const data = await response.json()
265+
266+
expect(response.status).toBe(200)
267+
expect(data.success).toBe(true)
268+
expect(isLargeValueRef(data.output.result.text)).toBe(true)
269+
})
270+
205271
it('should return computed result for multi-line code', async () => {
206272
mockExecuteInIsolatedVM.mockResolvedValueOnce({ result: 10, stdout: '' })
207273

‎apps/sim/app/api/workflows/[id]/execute/response-block.test.ts‎

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,25 @@
55
* @vitest-environment node
66
*/
77

8-
import { beforeEach, describe, expect, it } from 'vitest'
8+
import { beforeEach, describe, expect, it, vi } from 'vitest'
99
import { AuthType } from '@/lib/auth/hybrid'
10+
import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
11+
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata'
12+
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
13+
import { compactExecutionPayload } from '@/lib/execution/payloads/serializer'
1014
import type { ExecutionResult } from '@/lib/workflows/types'
1115
import { createHttpResponseFromBlock, workflowHasResponseBlock } from '@/lib/workflows/utils'
1216

17+
const { mockUploadFile } = vi.hoisted(() => ({
18+
mockUploadFile: vi.fn(),
19+
}))
20+
21+
vi.mock('@/lib/uploads', () => ({
22+
StorageService: {
23+
uploadFile: mockUploadFile,
24+
},
25+
}))
26+
1327
function buildExecutionResult(overrides: Partial<ExecutionResult> = {}): ExecutionResult {
1428
return {
1529
success: true,
@@ -38,6 +52,9 @@ describe('Response block gating by auth type', () => {
3852
let resultWithResponseBlock: ExecutionResult
3953

4054
beforeEach(() => {
55+
vi.clearAllMocks()
56+
clearLargeValueCacheForTests()
57+
mockUploadFile.mockImplementation(async ({ customKey }) => ({ key: customKey }))
4158
resultWithResponseBlock = buildExecutionResult()
4259
})
4360

@@ -112,4 +129,58 @@ describe('Response block gating by auth type', () => {
112129
const response = createHttpResponseFromBlock(result)
113130
expect(response.status).toBe(404)
114131
})
132+
133+
it('should return manifest metadata directly for Response block data', async () => {
134+
const output = await compactExecutionPayload(
135+
{
136+
data: {
137+
rows: Array.from({ length: 120_000 }, (_, index) => ({
138+
key: `SIM-${index}`,
139+
payload: 'x'.repeat(100),
140+
})),
141+
},
142+
status: 200,
143+
headers: {},
144+
},
145+
{
146+
workspaceId: 'workspace-1',
147+
workflowId: 'workflow-1',
148+
executionId: 'execution-1',
149+
userId: 'user-1',
150+
requireDurable: true,
151+
preserveRoot: true,
152+
}
153+
)
154+
const response = createHttpResponseFromBlock(buildExecutionResult({ output }))
155+
const body = await response.json()
156+
157+
expect(response.status).toBe(200)
158+
expect(isLargeArrayManifest(body.rows)).toBe(true)
159+
expect(body.success).toBeUndefined()
160+
})
161+
162+
it('should keep large string Response block data bounded as a generic ref', async () => {
163+
const output = await compactExecutionPayload(
164+
{
165+
data: {
166+
text: 'x'.repeat(9 * 1024 * 1024),
167+
},
168+
status: 200,
169+
headers: {},
170+
},
171+
{
172+
workspaceId: 'workspace-1',
173+
workflowId: 'workflow-1',
174+
executionId: 'execution-1',
175+
userId: 'user-1',
176+
requireDurable: true,
177+
preserveRoot: true,
178+
}
179+
)
180+
const response = createHttpResponseFromBlock(buildExecutionResult({ output }))
181+
const body = await response.json()
182+
183+
expect(response.status).toBe(200)
184+
expect(isLargeValueRef(body.text)).toBe(true)
185+
})
115186
})

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,15 @@ async function handleExecutePost(
786786
: result.output
787787

788788
if (auth.authType !== AuthType.INTERNAL_JWT && workflowHasResponseBlock(result)) {
789-
return createHttpResponseFromBlock({ ...result, output: outputWithBase64 })
789+
const compactResponseBlockOutput = await compactRoutePayload(outputWithBase64, {
790+
workspaceId,
791+
workflowId,
792+
executionId,
793+
userId: actorUserId,
794+
preserveUserFileBase64: true,
795+
preserveRoot: true,
796+
})
797+
return createHttpResponseFromBlock({ ...result, output: compactResponseBlockOutput })
790798
}
791799

792800
const compactOutput = await compactRoutePayload(outputWithBase64, {

‎apps/sim/executor/execution/block-executor.test.ts‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
6+
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata'
7+
import { BlockType } from '@/executor/constants'
8+
import type { DAGNode } from '@/executor/dag/builder'
9+
import { BlockExecutor } from '@/executor/execution/block-executor'
10+
import { ExecutionState } from '@/executor/execution/state'
11+
import type { BlockHandler, ExecutionContext } from '@/executor/types'
12+
import { VariableResolver } from '@/executor/variables/resolver'
13+
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
14+
15+
const { mockUploadFile } = vi.hoisted(() => ({
16+
mockUploadFile: vi.fn(),
17+
}))
18+
19+
vi.mock('@/ee/access-control/utils/permission-check', () => ({
20+
validateBlockType: vi.fn(),
21+
}))
22+
23+
vi.mock('@/lib/uploads', () => ({
24+
StorageService: {
25+
uploadFile: mockUploadFile,
26+
},
27+
}))
28+
29+
function createBlock(): SerializedBlock {
30+
return {
31+
id: 'function-block-1',
32+
metadata: { id: BlockType.FUNCTION, name: 'Function' },
33+
position: { x: 0, y: 0 },
34+
config: { tool: BlockType.FUNCTION, params: {} },
35+
inputs: {},
36+
outputs: {},
37+
enabled: true,
38+
}
39+
}
40+
41+
function createContext(state: ExecutionState): ExecutionContext {
42+
return {
43+
workflowId: 'workflow-1',
44+
workspaceId: 'workspace-1',
45+
executionId: 'execution-1',
46+
userId: 'user-1',
47+
blockStates: state.getBlockStates(),
48+
blockLogs: [],
49+
metadata: { requestId: 'request-1', duration: 0 },
50+
environmentVariables: {},
51+
workflowVariables: {},
52+
decisions: { router: new Map(), condition: new Map() },
53+
loopExecutions: new Map(),
54+
executedBlocks: new Set(),
55+
activeExecutionPath: new Set(),
56+
completedLoops: new Set(),
57+
} as ExecutionContext
58+
}
59+
60+
function createNode(block: SerializedBlock): DAGNode {
61+
return {
62+
id: block.id,
63+
block,
64+
incomingEdges: new Set(),
65+
outgoingEdges: new Map(),
66+
metadata: {},
67+
}
68+
}
69+
70+
describe('BlockExecutor', () => {
71+
beforeEach(() => {
72+
vi.clearAllMocks()
73+
clearLargeValueCacheForTests()
74+
mockUploadFile.mockImplementation(async ({ customKey }) => ({ key: customKey }))
75+
})
76+
77+
it('persists function output arrays as manifests in execution state', async () => {
78+
const block = createBlock()
79+
const workflow: SerializedWorkflow = {
80+
version: '1',
81+
blocks: [block],
82+
connections: [],
83+
loops: {},
84+
parallels: {},
85+
}
86+
const state = new ExecutionState()
87+
const resolver = new VariableResolver(workflow, {}, state)
88+
const output = {
89+
result: Array.from({ length: 120_000 }, (_, index) => ({
90+
key: `SIM-${index}`,
91+
payload: 'x'.repeat(100),
92+
})),
93+
}
94+
const handler: BlockHandler = {
95+
canHandle: () => true,
96+
execute: async () => output,
97+
}
98+
const executor = new BlockExecutor(
99+
[handler],
100+
resolver,
101+
{
102+
workspaceId: 'workspace-1',
103+
executionId: 'execution-1',
104+
userId: 'user-1',
105+
metadata: {
106+
requestId: 'request-1',
107+
executionId: 'execution-1',
108+
workflowId: 'workflow-1',
109+
workspaceId: 'workspace-1',
110+
userId: 'user-1',
111+
triggerType: 'manual',
112+
useDraftState: false,
113+
startTime: new Date().toISOString(),
114+
},
115+
},
116+
state
117+
)
118+
119+
await executor.execute(createContext(state), createNode(block), block)
120+
121+
const storedOutput = state.getBlockOutput(block.id)
122+
expect(isLargeArrayManifest(storedOutput?.result)).toBe(true)
123+
expect(storedOutput?.result).toMatchObject({
124+
__simLargeArrayManifest: true,
125+
kind: 'array',
126+
totalCount: output.result.length,
127+
})
128+
})
129+
})

‎apps/sim/executor/handlers/variables/variables-handler.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55
import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache'
6-
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest'
6+
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest-metadata'
77
import { BlockType } from '@/executor/constants'
88
import { VariablesBlockHandler } from '@/executor/handlers/variables/variables-handler'
99
import type { ExecutionContext } from '@/executor/types'

‎apps/sim/executor/handlers/variables/variables-handler.ts‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3-
import { isLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest'
4-
import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref'
3+
import { parseLargeExecutionValue } from '@/lib/execution/payloads/large-execution-value'
54
import { compactWorkflowVariableValue } from '@/lib/execution/payloads/serializer'
65
import type { BlockOutput } from '@/blocks/types'
76
import { BlockType } from '@/executor/constants'
@@ -98,7 +97,7 @@ export class VariablesBlockHandler implements BlockHandler {
9897
}
9998

10099
private parseValueByType(value: any, type: string, variableName?: string): any {
101-
const refValue = this.parseLargeExecutionValue(value)
100+
const refValue = parseLargeExecutionValue(value)
102101
if (refValue !== undefined) {
103102
return refValue
104103
}
@@ -174,21 +173,4 @@ export class VariablesBlockHandler implements BlockHandler {
174173

175174
return value
176175
}
177-
178-
private parseLargeExecutionValue(value: any): any | undefined {
179-
if (isLargeValueRef(value) || isLargeArrayManifest(value)) {
180-
return value
181-
}
182-
183-
if (typeof value !== 'string' || !value.trim()) {
184-
return undefined
185-
}
186-
187-
try {
188-
const parsed = JSON.parse(value)
189-
return isLargeValueRef(parsed) || isLargeArrayManifest(parsed) ? parsed : undefined
190-
} catch {
191-
return undefined
192-
}
193-
}
194176
}

0 commit comments

Comments
 (0)