From 66dfe2c6b2621c735bd96c3c2c67132812d978a2 Mon Sep 17 00:00:00 2001 From: Waleed Date: Thu, 22 Jan 2026 11:26:47 -0800 Subject: [PATCH 1/8] improvement(workflow-item): stabilize avatar layout and fix name truncation (#2939) * improvement(workflow-item): stabilize avatar layout and fix name truncation * fix(avatars): revert overflow bg to hardcoded color for contrast --- .../components/workflow-item/avatars/avatars.tsx | 12 +++++++----- .../components/workflow-item/workflow-item.tsx | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/avatars/avatars.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/avatars/avatars.tsx index fce2782406..24433f83f2 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/avatars/avatars.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/avatars/avatars.tsx @@ -11,7 +11,7 @@ import { useSidebarStore } from '@/stores/sidebar/store' * Avatar display configuration for responsive layout. */ const AVATAR_CONFIG = { - MIN_COUNT: 3, + MIN_COUNT: 4, MAX_COUNT: 12, WIDTH_PER_AVATAR: 20, } as const @@ -106,7 +106,9 @@ export function Avatars({ workflowId }: AvatarsProps) { }, [presenceUsers, currentWorkflowId, workflowId, currentSocketId]) /** - * Calculate visible users and overflow count + * Calculate visible users and overflow count. + * Shows up to maxVisible avatars, with overflow indicator for any remaining. + * Users are reversed so new avatars appear on the left (keeping right side stable). */ const { visibleUsers, overflowCount } = useMemo(() => { if (workflowUsers.length === 0) { @@ -116,7 +118,8 @@ export function Avatars({ workflowId }: AvatarsProps) { const visible = workflowUsers.slice(0, maxVisible) const overflow = Math.max(0, workflowUsers.length - maxVisible) - return { visibleUsers: visible, overflowCount: overflow } + // Reverse so rightmost avatars stay stable as new ones are revealed on the left + return { visibleUsers: [...visible].reverse(), overflowCount: overflow } }, [workflowUsers, maxVisible]) if (visibleUsers.length === 0) { @@ -139,9 +142,8 @@ export function Avatars({ workflowId }: AvatarsProps) { )} - {visibleUsers.map((user, index) => ( - 0 ? index + 1 : index} /> + ))} ) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx index 8050f7b0e9..072d0ee123 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx @@ -347,7 +347,7 @@ export function WorkflowItem({ ) : (
Date: Thu, 22 Jan 2026 11:34:40 -0800 Subject: [PATCH 2/8] fix(executor): stop parallel execution when block errors (#2940) --- apps/sim/executor/dag/builder.test.ts | 65 +++ apps/sim/executor/dag/builder.ts | 11 +- apps/sim/executor/execution/engine.test.ts | 407 ++++++++++++++++++ apps/sim/executor/execution/engine.ts | 29 +- .../workflow/workflow-handler.test.ts | 23 +- .../handlers/workflow/workflow-handler.ts | 43 +- 6 files changed, 522 insertions(+), 56 deletions(-) diff --git a/apps/sim/executor/dag/builder.test.ts b/apps/sim/executor/dag/builder.test.ts index 32bf14d0f1..39df0681a0 100644 --- a/apps/sim/executor/dag/builder.test.ts +++ b/apps/sim/executor/dag/builder.test.ts @@ -24,6 +24,71 @@ function createBlock(id: string, metadataId: string): SerializedBlock { } } +describe('DAGBuilder disabled subflow validation', () => { + it('skips validation for disabled loops with no blocks inside', () => { + const workflow: SerializedWorkflow = { + version: '1', + blocks: [ + createBlock('start', BlockType.STARTER), + { ...createBlock('loop-block', BlockType.FUNCTION), enabled: false }, + ], + connections: [], + loops: { + 'loop-1': { + id: 'loop-1', + nodes: [], // Empty loop - would normally throw + iterations: 3, + }, + }, + } + + const builder = new DAGBuilder() + // Should not throw even though loop has no blocks inside + expect(() => builder.build(workflow)).not.toThrow() + }) + + it('skips validation for disabled parallels with no blocks inside', () => { + const workflow: SerializedWorkflow = { + version: '1', + blocks: [createBlock('start', BlockType.STARTER)], + connections: [], + loops: {}, + parallels: { + 'parallel-1': { + id: 'parallel-1', + nodes: [], // Empty parallel - would normally throw + }, + }, + } + + const builder = new DAGBuilder() + // Should not throw even though parallel has no blocks inside + expect(() => builder.build(workflow)).not.toThrow() + }) + + it('skips validation for loops where all inner blocks are disabled', () => { + const workflow: SerializedWorkflow = { + version: '1', + blocks: [ + createBlock('start', BlockType.STARTER), + { ...createBlock('inner-block', BlockType.FUNCTION), enabled: false }, + ], + connections: [], + loops: { + 'loop-1': { + id: 'loop-1', + nodes: ['inner-block'], // Has node but it's disabled + iterations: 3, + }, + }, + } + + const builder = new DAGBuilder() + // Should not throw - loop is effectively disabled since all inner blocks are disabled + expect(() => builder.build(workflow)).not.toThrow() + }) +}) + describe('DAGBuilder human-in-the-loop transformation', () => { it('creates trigger nodes and rewires edges for pause blocks', () => { const workflow: SerializedWorkflow = { diff --git a/apps/sim/executor/dag/builder.ts b/apps/sim/executor/dag/builder.ts index b072faaede..5465df6342 100644 --- a/apps/sim/executor/dag/builder.ts +++ b/apps/sim/executor/dag/builder.ts @@ -136,17 +136,18 @@ export class DAGBuilder { nodes: string[] | undefined, type: 'Loop' | 'Parallel' ): void { + const sentinelStartId = + type === 'Loop' ? buildSentinelStartId(id) : buildParallelSentinelStartId(id) + const sentinelStartNode = dag.nodes.get(sentinelStartId) + + if (!sentinelStartNode) return + if (!nodes || nodes.length === 0) { throw new Error( `${type} has no blocks inside. Add at least one block to the ${type.toLowerCase()}.` ) } - const sentinelStartId = - type === 'Loop' ? buildSentinelStartId(id) : buildParallelSentinelStartId(id) - const sentinelStartNode = dag.nodes.get(sentinelStartId) - if (!sentinelStartNode) return - const hasConnections = Array.from(sentinelStartNode.outgoingEdges.values()).some((edge) => nodes.includes(extractBaseBlockId(edge.target)) ) diff --git a/apps/sim/executor/execution/engine.test.ts b/apps/sim/executor/execution/engine.test.ts index f93ebc2068..880dc77c86 100644 --- a/apps/sim/executor/execution/engine.test.ts +++ b/apps/sim/executor/execution/engine.test.ts @@ -554,6 +554,413 @@ describe('ExecutionEngine', () => { }) }) + describe('Error handling in execution', () => { + it('should fail execution when a single node throws an error', async () => { + const startNode = createMockNode('start', 'starter') + const errorNode = createMockNode('error-node', 'function') + startNode.outgoingEdges.set('edge1', { target: 'error-node' }) + + const dag = createMockDAG([startNode, errorNode]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['error-node'] + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'error-node') { + throw new Error('Block execution failed') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('Block execution failed') + }) + + it('should stop parallel branches when one branch throws an error', async () => { + const startNode = createMockNode('start', 'starter') + const parallelNodes = Array.from({ length: 5 }, (_, i) => + createMockNode(`parallel${i}`, 'function') + ) + + parallelNodes.forEach((_, i) => { + startNode.outgoingEdges.set(`edge${i}`, { target: `parallel${i}` }) + }) + + const dag = createMockDAG([startNode, ...parallelNodes]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return parallelNodes.map((_, i) => `parallel${i}`) + return [] + }) + + const executedNodes: string[] = [] + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + executedNodes.push(nodeId) + if (nodeId === 'parallel0') { + await new Promise((resolve) => setTimeout(resolve, 10)) + throw new Error('Parallel branch failed') + } + await new Promise((resolve) => setTimeout(resolve, 100)) + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('Parallel branch failed') + }) + + it('should capture only the first error when multiple parallel branches fail', async () => { + const startNode = createMockNode('start', 'starter') + const parallelNodes = Array.from({ length: 3 }, (_, i) => + createMockNode(`parallel${i}`, 'function') + ) + + parallelNodes.forEach((_, i) => { + startNode.outgoingEdges.set(`edge${i}`, { target: `parallel${i}` }) + }) + + const dag = createMockDAG([startNode, ...parallelNodes]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return parallelNodes.map((_, i) => `parallel${i}`) + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'parallel0') { + await new Promise((resolve) => setTimeout(resolve, 10)) + throw new Error('First error') + } + if (nodeId === 'parallel1') { + await new Promise((resolve) => setTimeout(resolve, 20)) + throw new Error('Second error') + } + if (nodeId === 'parallel2') { + await new Promise((resolve) => setTimeout(resolve, 30)) + throw new Error('Third error') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('First error') + }) + + it('should wait for ongoing executions to complete before throwing error', async () => { + const startNode = createMockNode('start', 'starter') + const fastErrorNode = createMockNode('fast-error', 'function') + const slowNode = createMockNode('slow', 'function') + + startNode.outgoingEdges.set('edge1', { target: 'fast-error' }) + startNode.outgoingEdges.set('edge2', { target: 'slow' }) + + const dag = createMockDAG([startNode, fastErrorNode, slowNode]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['fast-error', 'slow'] + return [] + }) + + let slowNodeCompleted = false + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'fast-error') { + await new Promise((resolve) => setTimeout(resolve, 10)) + throw new Error('Fast error') + } + if (nodeId === 'slow') { + await new Promise((resolve) => setTimeout(resolve, 50)) + slowNodeCompleted = true + return { nodeId, output: {}, isFinalOutput: false } + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('Fast error') + + expect(slowNodeCompleted).toBe(true) + }) + + it('should not queue new nodes after an error occurs', async () => { + const startNode = createMockNode('start', 'starter') + const errorNode = createMockNode('error-node', 'function') + const afterErrorNode = createMockNode('after-error', 'function') + + startNode.outgoingEdges.set('edge1', { target: 'error-node' }) + errorNode.outgoingEdges.set('edge2', { target: 'after-error' }) + + const dag = createMockDAG([startNode, errorNode, afterErrorNode]) + const context = createMockContext() + + const queuedNodes: string[] = [] + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') { + queuedNodes.push('error-node') + return ['error-node'] + } + if (node.id === 'error-node') { + queuedNodes.push('after-error') + return ['after-error'] + } + return [] + }) + + const executedNodes: string[] = [] + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + executedNodes.push(nodeId) + if (nodeId === 'error-node') { + throw new Error('Node error') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('Node error') + + expect(executedNodes).not.toContain('after-error') + }) + + it('should populate error result with metadata when execution fails', async () => { + const startNode = createMockNode('start', 'starter') + const errorNode = createMockNode('error-node', 'function') + startNode.outgoingEdges.set('edge1', { target: 'error-node' }) + + const dag = createMockDAG([startNode, errorNode]) + const context = createMockContext() + context.blockLogs.push({ + blockId: 'start', + blockName: 'Start', + blockType: 'starter', + startedAt: new Date().toISOString(), + endedAt: new Date().toISOString(), + durationMs: 10, + success: true, + }) + + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['error-node'] + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'error-node') { + const error = new Error('Execution failed') as any + error.executionResult = { + success: false, + output: { partial: 'data' }, + logs: context.blockLogs, + metadata: context.metadata, + } + throw error + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + try { + await engine.run('start') + expect.fail('Should have thrown') + } catch (error: any) { + expect(error.executionResult).toBeDefined() + expect(error.executionResult.metadata.endTime).toBeDefined() + expect(error.executionResult.metadata.duration).toBeDefined() + } + }) + + it('should prefer cancellation status over error when both occur', async () => { + const abortController = new AbortController() + + const startNode = createMockNode('start', 'starter') + const errorNode = createMockNode('error-node', 'function') + startNode.outgoingEdges.set('edge1', { target: 'error-node' }) + + const dag = createMockDAG([startNode, errorNode]) + const context = createMockContext({ abortSignal: abortController.signal }) + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['error-node'] + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'error-node') { + abortController.abort() + throw new Error('Node error') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + const result = await engine.run('start') + + expect(result.status).toBe('cancelled') + expect(result.success).toBe(false) + }) + + it('should stop loop iteration when error occurs in loop body', async () => { + const loopStartNode = createMockNode('loop-start', 'loop_sentinel') + loopStartNode.metadata = { isSentinel: true, sentinelType: 'start', loopId: 'loop1' } + + const loopBodyNode = createMockNode('loop-body', 'function') + loopBodyNode.metadata = { isLoopNode: true, loopId: 'loop1' } + + const loopEndNode = createMockNode('loop-end', 'loop_sentinel') + loopEndNode.metadata = { isSentinel: true, sentinelType: 'end', loopId: 'loop1' } + + const afterLoopNode = createMockNode('after-loop', 'function') + + loopStartNode.outgoingEdges.set('edge1', { target: 'loop-body' }) + loopBodyNode.outgoingEdges.set('edge2', { target: 'loop-end' }) + loopEndNode.outgoingEdges.set('loop_continue', { + target: 'loop-start', + sourceHandle: 'loop_continue', + }) + loopEndNode.outgoingEdges.set('loop_complete', { + target: 'after-loop', + sourceHandle: 'loop_complete', + }) + + const dag = createMockDAG([loopStartNode, loopBodyNode, loopEndNode, afterLoopNode]) + const context = createMockContext() + + let iterationCount = 0 + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'loop-start') return ['loop-body'] + if (node.id === 'loop-body') return ['loop-end'] + if (node.id === 'loop-end') { + iterationCount++ + if (iterationCount < 5) return ['loop-start'] + return ['after-loop'] + } + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'loop-body' && iterationCount >= 2) { + throw new Error('Loop body error on iteration 3') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('loop-start')).rejects.toThrow('Loop body error on iteration 3') + + expect(iterationCount).toBeLessThanOrEqual(3) + }) + + it('should handle error that is not an Error instance', async () => { + const startNode = createMockNode('start', 'starter') + const errorNode = createMockNode('error-node', 'function') + startNode.outgoingEdges.set('edge1', { target: 'error-node' }) + + const dag = createMockDAG([startNode, errorNode]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['error-node'] + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'error-node') { + throw 'String error message' + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + await expect(engine.run('start')).rejects.toThrow('String error message') + }) + + it('should preserve partial output when error occurs after some blocks complete', async () => { + const startNode = createMockNode('start', 'starter') + const successNode = createMockNode('success', 'function') + const errorNode = createMockNode('error-node', 'function') + + startNode.outgoingEdges.set('edge1', { target: 'success' }) + successNode.outgoingEdges.set('edge2', { target: 'error-node' }) + + const dag = createMockDAG([startNode, successNode, errorNode]) + const context = createMockContext() + const edgeManager = createMockEdgeManager((node) => { + if (node.id === 'start') return ['success'] + if (node.id === 'success') return ['error-node'] + return [] + }) + + const nodeOrchestrator = { + executionCount: 0, + executeNode: vi.fn().mockImplementation(async (_ctx: ExecutionContext, nodeId: string) => { + if (nodeId === 'success') { + return { nodeId, output: { successData: 'preserved' }, isFinalOutput: false } + } + if (nodeId === 'error-node') { + throw new Error('Late error') + } + return { nodeId, output: {}, isFinalOutput: false } + }), + handleNodeCompletion: vi.fn(), + } as unknown as MockNodeOrchestrator + + const engine = new ExecutionEngine(context, dag, edgeManager, nodeOrchestrator) + + try { + await engine.run('start') + expect.fail('Should have thrown') + } catch (error: any) { + // Verify the error was thrown + expect(error.message).toBe('Late error') + // The partial output should be available in executionResult if attached + if (error.executionResult) { + expect(error.executionResult.output).toBeDefined() + } + } + }) + }) + describe('Cancellation flag behavior', () => { it('should set cancelledFlag when abort signal fires', async () => { const abortController = new AbortController() diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index 7c2317b047..58792ef2b8 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -25,6 +25,8 @@ export class ExecutionEngine { private pausedBlocks: Map = new Map() private allowResumeTriggers: boolean private cancelledFlag = false + private errorFlag = false + private executionError: Error | null = null private lastCancellationCheck = 0 private readonly useRedisCancellation: boolean private readonly CANCELLATION_CHECK_INTERVAL_MS = 500 @@ -103,7 +105,7 @@ export class ExecutionEngine { this.initializeQueue(triggerBlockId) while (this.hasWork()) { - if (await this.checkCancellation()) { + if ((await this.checkCancellation()) || this.errorFlag) { break } await this.processQueue() @@ -113,6 +115,11 @@ export class ExecutionEngine { await this.waitForAllExecutions() } + // Rethrow the captured error so it's handled by the catch block + if (this.errorFlag && this.executionError) { + throw this.executionError + } + if (this.pausedBlocks.size > 0) { return this.buildPausedResult(startTime) } @@ -196,11 +203,17 @@ export class ExecutionEngine { } private trackExecution(promise: Promise): void { - this.executing.add(promise) - promise.catch(() => {}) - promise.finally(() => { - this.executing.delete(promise) - }) + const trackedPromise = promise + .catch((error) => { + if (!this.errorFlag) { + this.errorFlag = true + this.executionError = error instanceof Error ? error : new Error(String(error)) + } + }) + .finally(() => { + this.executing.delete(trackedPromise) + }) + this.executing.add(trackedPromise) } private async waitForAnyExecution(): Promise { @@ -315,7 +328,7 @@ export class ExecutionEngine { private async processQueue(): Promise { while (this.readyQueue.length > 0) { - if (await this.checkCancellation()) { + if ((await this.checkCancellation()) || this.errorFlag) { break } const nodeId = this.dequeue() @@ -324,7 +337,7 @@ export class ExecutionEngine { this.trackExecution(promise) } - if (this.executing.size > 0 && !this.cancelledFlag) { + if (this.executing.size > 0 && !this.cancelledFlag && !this.errorFlag) { await this.waitForAnyExecution() } } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index bf304c786d..21d9cd3fdb 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -204,26 +204,21 @@ describe('WorkflowBlockHandler', () => { }) }) - it('should map failed child output correctly', () => { + it('should throw error for failed child output so BlockExecutor can check error port', () => { const childResult = { success: false, error: 'Child workflow failed', } - const result = (handler as any).mapChildOutputToParent( - childResult, - 'child-id', - 'Child Workflow', - 100 - ) + expect(() => + (handler as any).mapChildOutputToParent(childResult, 'child-id', 'Child Workflow', 100) + ).toThrow('Error in child workflow "Child Workflow": Child workflow failed') - expect(result).toEqual({ - success: false, - childWorkflowName: 'Child Workflow', - result: {}, - error: 'Child workflow failed', - childTraceSpans: [], - }) + try { + ;(handler as any).mapChildOutputToParent(childResult, 'child-id', 'Child Workflow', 100) + } catch (error: any) { + expect(error.childTraceSpans).toEqual([]) + } }) it('should handle nested response structures', () => { diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 94f0908897..d8f7ced358 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -144,6 +144,11 @@ export class WorkflowBlockHandler implements BlockHandler { const workflowMetadata = workflows[workflowId] const childWorkflowName = workflowMetadata?.name || workflowId + const originalError = error.message || 'Unknown error' + const wrappedError = new Error( + `Error in child workflow "${childWorkflowName}": ${originalError}` + ) + if (error.executionResult?.logs) { const executionResult = error.executionResult as ExecutionResult @@ -159,28 +164,12 @@ export class WorkflowBlockHandler implements BlockHandler { ) logger.info(`Captured ${childTraceSpans.length} child trace spans from failed execution`) - - return { - success: false, - childWorkflowName, - result: {}, - error: error.message || 'Child workflow execution failed', - childTraceSpans: childTraceSpans, - } as Record + ;(wrappedError as any).childTraceSpans = childTraceSpans + } else if (error.childTraceSpans && Array.isArray(error.childTraceSpans)) { + ;(wrappedError as any).childTraceSpans = error.childTraceSpans } - if (error.childTraceSpans && Array.isArray(error.childTraceSpans)) { - return { - success: false, - childWorkflowName, - result: {}, - error: error.message || 'Child workflow execution failed', - childTraceSpans: error.childTraceSpans, - } as Record - } - - const originalError = error.message || 'Unknown error' - throw new Error(`Error in child workflow "${childWorkflowName}": ${originalError}`) + throw wrappedError } } @@ -452,17 +441,13 @@ export class WorkflowBlockHandler implements BlockHandler { if (!success) { logger.warn(`Child workflow ${childWorkflowName} failed`) - // Return failure with child trace spans so they can be displayed - return { - success: false, - childWorkflowName, - result, - error: childResult.error || 'Child workflow execution failed', - childTraceSpans: childTraceSpans || [], - } as Record + const error = new Error( + `Error in child workflow "${childWorkflowName}": ${childResult.error || 'Child workflow execution failed'}` + ) + ;(error as any).childTraceSpans = childTraceSpans || [] + throw error } - // Success case return { success: true, childWorkflowName, From 9a8b59125782c0a9be9c8d4f0bdeb72712bae176 Mon Sep 17 00:00:00 2001 From: Waleed Date: Thu, 22 Jan 2026 11:35:23 -0800 Subject: [PATCH 3/8] improvement(helm): add per-deployment extraVolumes support (#2942) --- helm/sim/templates/deployment-app.yaml | 14 ++++++++++++-- helm/sim/templates/deployment-ollama.yaml | 12 +++++++++++- helm/sim/templates/deployment-realtime.yaml | 14 ++++++++++++-- helm/sim/values.yaml | 12 ++++++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/helm/sim/templates/deployment-app.yaml b/helm/sim/templates/deployment-app.yaml index 04eedd755d..6b1d632899 100644 --- a/helm/sim/templates/deployment-app.yaml +++ b/helm/sim/templates/deployment-app.yaml @@ -110,12 +110,22 @@ spec: {{- end }} {{- include "sim.resources" .Values.app | nindent 10 }} {{- include "sim.securityContext" .Values.app | nindent 10 }} - {{- with .Values.extraVolumeMounts }} + {{- if or .Values.extraVolumeMounts .Values.app.extraVolumeMounts }} volumeMounts: + {{- with .Values.extraVolumeMounts }} {{- toYaml . | nindent 12 }} + {{- end }} + {{- with .Values.app.extraVolumeMounts }} + {{- toYaml . | nindent 12 }} + {{- end }} {{- end }} - {{- with .Values.extraVolumes }} + {{- if or .Values.extraVolumes .Values.app.extraVolumes }} volumes: + {{- with .Values.extraVolumes }} {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.app.extraVolumes }} + {{- toYaml . | nindent 8 }} + {{- end }} {{- end }} {{- end }} \ No newline at end of file diff --git a/helm/sim/templates/deployment-ollama.yaml b/helm/sim/templates/deployment-ollama.yaml index a46f2b6611..620b84f482 100644 --- a/helm/sim/templates/deployment-ollama.yaml +++ b/helm/sim/templates/deployment-ollama.yaml @@ -92,6 +92,7 @@ spec: {{- toYaml .Values.ollama.readinessProbe | nindent 12 }} {{- end }} {{- include "sim.resources" .Values.ollama | nindent 10 }} + {{- if or .Values.ollama.persistence.enabled .Values.extraVolumeMounts .Values.ollama.extraVolumeMounts }} volumeMounts: {{- if .Values.ollama.persistence.enabled }} - name: ollama-data @@ -100,13 +101,22 @@ spec: {{- with .Values.extraVolumeMounts }} {{- toYaml . | nindent 12 }} {{- end }} - {{- if .Values.ollama.persistence.enabled }} + {{- with .Values.ollama.extraVolumeMounts }} + {{- toYaml . | nindent 12 }} + {{- end }} + {{- end }} + {{- if or .Values.ollama.persistence.enabled .Values.extraVolumes .Values.ollama.extraVolumes }} volumes: + {{- if .Values.ollama.persistence.enabled }} - name: ollama-data persistentVolumeClaim: claimName: {{ include "sim.fullname" . }}-ollama-data + {{- end }} {{- with .Values.extraVolumes }} {{- toYaml . | nindent 8 }} {{- end }} + {{- with .Values.ollama.extraVolumes }} + {{- toYaml . | nindent 8 }} + {{- end }} {{- end }} {{- end }} \ No newline at end of file diff --git a/helm/sim/templates/deployment-realtime.yaml b/helm/sim/templates/deployment-realtime.yaml index 746516594e..00054ecf38 100644 --- a/helm/sim/templates/deployment-realtime.yaml +++ b/helm/sim/templates/deployment-realtime.yaml @@ -84,12 +84,22 @@ spec: {{- end }} {{- include "sim.resources" .Values.realtime | nindent 10 }} {{- include "sim.securityContext" .Values.realtime | nindent 10 }} - {{- with .Values.extraVolumeMounts }} + {{- if or .Values.extraVolumeMounts .Values.realtime.extraVolumeMounts }} volumeMounts: + {{- with .Values.extraVolumeMounts }} {{- toYaml . | nindent 12 }} + {{- end }} + {{- with .Values.realtime.extraVolumeMounts }} + {{- toYaml . | nindent 12 }} + {{- end }} {{- end }} - {{- with .Values.extraVolumes }} + {{- if or .Values.extraVolumes .Values.realtime.extraVolumes }} volumes: + {{- with .Values.extraVolumes }} {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.realtime.extraVolumes }} + {{- toYaml . | nindent 8 }} + {{- end }} {{- end }} {{- end }} \ No newline at end of file diff --git a/helm/sim/values.yaml b/helm/sim/values.yaml index 2ee7cbfadf..92db160dfa 100644 --- a/helm/sim/values.yaml +++ b/helm/sim/values.yaml @@ -224,6 +224,10 @@ app: timeoutSeconds: 5 failureThreshold: 3 + # Additional volumes for app deployment (e.g., branding assets, custom configs) + extraVolumes: [] + extraVolumeMounts: [] + # Realtime socket server configuration realtime: # Enable/disable the realtime service @@ -301,6 +305,10 @@ realtime: timeoutSeconds: 5 failureThreshold: 3 + # Additional volumes for realtime deployment + extraVolumes: [] + extraVolumeMounts: [] + # Database migrations job configuration migrations: # Enable/disable migrations job @@ -539,6 +547,10 @@ ollama: timeoutSeconds: 5 failureThreshold: 3 + # Additional volumes for ollama deployment + extraVolumes: [] + extraVolumeMounts: [] + # Ingress configuration ingress: # Enable/disable ingress From 4e4149792a1739bc0252539fb41273253f639c4d Mon Sep 17 00:00:00 2001 From: Waleed Date: Thu, 22 Jan 2026 11:46:34 -0800 Subject: [PATCH 4/8] fix(gmail): expose messageId field in read email block (#2943) --- apps/sim/blocks/blocks/gmail.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/apps/sim/blocks/blocks/gmail.ts b/apps/sim/blocks/blocks/gmail.ts index f5543b8351..e389d3b3d9 100644 --- a/apps/sim/blocks/blocks/gmail.ts +++ b/apps/sim/blocks/blocks/gmail.ts @@ -242,15 +242,9 @@ Return ONLY the email body - no explanations, no extra text.`, id: 'messageId', title: 'Message ID', type: 'short-input', - placeholder: 'Enter message ID to read (optional)', - condition: { - field: 'operation', - value: 'read_gmail', - and: { - field: 'folder', - value: '', - }, - }, + placeholder: 'Read specific email by ID (overrides label/folder)', + condition: { field: 'operation', value: 'read_gmail' }, + mode: 'advanced', }, // Search Fields { From fcd0240db60a7dbec30ab2a2382d270da41b229f Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 22 Jan 2026 12:38:50 -0800 Subject: [PATCH 5/8] fix(resolver): consolidate reference resolution (#2941) * fix(resolver): consolidate code to resolve references * fix edge cases * use already formatted error * fix multi index * fix backwards compat reachability * handle backwards compatibility accurately * use shared constant correctly --- .../app/api/function/execute/route.test.ts | 4 +- apps/sim/app/api/function/execute/route.ts | 94 ++++--- .../executor/handlers/agent/agent-handler.ts | 3 +- .../handlers/condition/condition-handler.ts | 3 +- .../function/function-handler.test.ts | 29 +- .../handlers/function/function-handler.ts | 3 +- apps/sim/executor/utils/block-data.ts | 31 ++- .../executor/utils/block-reference.test.ts | 255 ++++++++++++++++++ apps/sim/executor/utils/block-reference.ts | 210 +++++++++++++++ .../sim/executor/variables/resolvers/block.ts | 232 +++++----------- .../executor/variables/resolvers/reference.ts | 21 +- apps/sim/lib/execution/isolated-vm-worker.cjs | 6 +- apps/sim/tools/function/execute.test.ts | 3 + apps/sim/tools/function/execute.ts | 8 + apps/sim/tools/function/types.ts | 1 + 15 files changed, 671 insertions(+), 232 deletions(-) create mode 100644 apps/sim/executor/utils/block-reference.test.ts create mode 100644 apps/sim/executor/utils/block-reference.ts diff --git a/apps/sim/app/api/function/execute/route.test.ts b/apps/sim/app/api/function/execute/route.test.ts index ea020abaf5..e0e7723bd4 100644 --- a/apps/sim/app/api/function/execute/route.test.ts +++ b/apps/sim/app/api/function/execute/route.test.ts @@ -313,7 +313,7 @@ describe('Function Execute API Route', () => { 'block-2': 'world', }, blockNameMapping: { - validVar: 'block-1', + validvar: 'block-1', another_valid: 'block-2', }, }) @@ -539,7 +539,7 @@ describe('Function Execute API Route', () => { 'block-complex': complexData, }, blockNameMapping: { - complexData: 'block-complex', + complexdata: 'block-complex', }, }) diff --git a/apps/sim/app/api/function/execute/route.ts b/apps/sim/app/api/function/execute/route.ts index 8868c2d404..0940054b2e 100644 --- a/apps/sim/app/api/function/execute/route.ts +++ b/apps/sim/app/api/function/execute/route.ts @@ -6,11 +6,11 @@ import { executeInE2B } from '@/lib/execution/e2b' import { executeInIsolatedVM } from '@/lib/execution/isolated-vm' import { CodeLanguage, DEFAULT_CODE_LANGUAGE, isValidCodeLanguage } from '@/lib/execution/languages' import { escapeRegExp, normalizeName, REFERENCE } from '@/executor/constants' +import { type OutputSchema, resolveBlockReference } from '@/executor/utils/block-reference' import { createEnvVarPattern, createWorkflowVariablePattern, } from '@/executor/utils/reference-validation' -import { navigatePath } from '@/executor/variables/resolvers/reference' export const dynamic = 'force-dynamic' export const runtime = 'nodejs' @@ -470,14 +470,17 @@ function resolveEnvironmentVariables( function resolveTagVariables( code: string, - blockData: Record, + blockData: Record, blockNameMapping: Record, - contextVariables: Record + blockOutputSchemas: Record, + contextVariables: Record, + language = 'javascript' ): string { let resolvedCode = code + const undefinedLiteral = language === 'python' ? 'None' : 'undefined' const tagPattern = new RegExp( - `${REFERENCE.START}([a-zA-Z_][a-zA-Z0-9_${REFERENCE.PATH_DELIMITER}]*[a-zA-Z0-9_])${REFERENCE.END}`, + `${REFERENCE.START}([a-zA-Z_](?:[a-zA-Z0-9_${REFERENCE.PATH_DELIMITER}]*[a-zA-Z0-9_])?)${REFERENCE.END}`, 'g' ) const tagMatches = resolvedCode.match(tagPattern) || [] @@ -486,41 +489,37 @@ function resolveTagVariables( const tagName = match.slice(REFERENCE.START.length, -REFERENCE.END.length).trim() const pathParts = tagName.split(REFERENCE.PATH_DELIMITER) const blockName = pathParts[0] + const fieldPath = pathParts.slice(1) - const blockId = blockNameMapping[blockName] - if (!blockId) { - continue - } + const result = resolveBlockReference(blockName, fieldPath, { + blockNameMapping, + blockData, + blockOutputSchemas, + }) - const blockOutput = blockData[blockId] - if (blockOutput === undefined) { + if (!result) { continue } - let tagValue: any - if (pathParts.length === 1) { - tagValue = blockOutput - } else { - tagValue = navigatePath(blockOutput, pathParts.slice(1)) - } + let tagValue = result.value if (tagValue === undefined) { + resolvedCode = resolvedCode.replace(new RegExp(escapeRegExp(match), 'g'), undefinedLiteral) continue } - if ( - typeof tagValue === 'string' && - tagValue.length > 100 && - (tagValue.startsWith('{') || tagValue.startsWith('[')) - ) { - try { - tagValue = JSON.parse(tagValue) - } catch { - // Keep as-is + if (typeof tagValue === 'string') { + const trimmed = tagValue.trimStart() + if (trimmed.startsWith('{') || trimmed.startsWith('[')) { + try { + tagValue = JSON.parse(tagValue) + } catch { + // Keep as string if not valid JSON + } } } - const safeVarName = `__tag_${tagName.replace(/[^a-zA-Z0-9_]/g, '_')}` + const safeVarName = `__tag_${tagName.replace(/_/g, '_1').replace(/\./g, '_0')}` contextVariables[safeVarName] = tagValue resolvedCode = resolvedCode.replace(new RegExp(escapeRegExp(match), 'g'), safeVarName) } @@ -537,18 +536,27 @@ function resolveTagVariables( */ function resolveCodeVariables( code: string, - params: Record, + params: Record, envVars: Record = {}, - blockData: Record = {}, + blockData: Record = {}, blockNameMapping: Record = {}, - workflowVariables: Record = {} -): { resolvedCode: string; contextVariables: Record } { + blockOutputSchemas: Record = {}, + workflowVariables: Record = {}, + language = 'javascript' +): { resolvedCode: string; contextVariables: Record } { let resolvedCode = code - const contextVariables: Record = {} + const contextVariables: Record = {} resolvedCode = resolveWorkflowVariables(resolvedCode, workflowVariables, contextVariables) resolvedCode = resolveEnvironmentVariables(resolvedCode, params, envVars, contextVariables) - resolvedCode = resolveTagVariables(resolvedCode, blockData, blockNameMapping, contextVariables) + resolvedCode = resolveTagVariables( + resolvedCode, + blockData, + blockNameMapping, + blockOutputSchemas, + contextVariables, + language + ) return { resolvedCode, contextVariables } } @@ -585,6 +593,7 @@ export async function POST(req: NextRequest) { envVars = {}, blockData = {}, blockNameMapping = {}, + blockOutputSchemas = {}, workflowVariables = {}, workflowId, isCustomTool = false, @@ -601,20 +610,21 @@ export async function POST(req: NextRequest) { isCustomTool, }) - // Resolve variables in the code with workflow environment variables + const lang = isValidCodeLanguage(language) ? language : DEFAULT_CODE_LANGUAGE + const codeResolution = resolveCodeVariables( code, executionParams, envVars, blockData, blockNameMapping, - workflowVariables + blockOutputSchemas, + workflowVariables, + lang ) resolvedCode = codeResolution.resolvedCode const contextVariables = codeResolution.contextVariables - const lang = isValidCodeLanguage(language) ? language : DEFAULT_CODE_LANGUAGE - let jsImports = '' let jsRemainingCode = resolvedCode let hasImports = false @@ -670,7 +680,11 @@ export async function POST(req: NextRequest) { prologue += `const environmentVariables = JSON.parse(${JSON.stringify(JSON.stringify(envVars))});\n` prologueLineCount++ for (const [k, v] of Object.entries(contextVariables)) { - prologue += `const ${k} = JSON.parse(${JSON.stringify(JSON.stringify(v))});\n` + if (v === undefined) { + prologue += `const ${k} = undefined;\n` + } else { + prologue += `const ${k} = JSON.parse(${JSON.stringify(JSON.stringify(v))});\n` + } prologueLineCount++ } @@ -741,7 +755,11 @@ export async function POST(req: NextRequest) { prologue += `environmentVariables = json.loads(${JSON.stringify(JSON.stringify(envVars))})\n` prologueLineCount++ for (const [k, v] of Object.entries(contextVariables)) { - prologue += `${k} = json.loads(${JSON.stringify(JSON.stringify(v))})\n` + if (v === undefined) { + prologue += `${k} = None\n` + } else { + prologue += `${k} = json.loads(${JSON.stringify(JSON.stringify(v))})\n` + } prologueLineCount++ } const wrapped = [ diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 3240da8974..9cbd6692ac 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -305,7 +305,7 @@ export class AgentBlockHandler implements BlockHandler { base.executeFunction = async (callParams: Record) => { const mergedParams = mergeToolParameters(userProvidedParams, callParams) - const { blockData, blockNameMapping } = collectBlockData(ctx) + const { blockData, blockNameMapping, blockOutputSchemas } = collectBlockData(ctx) const result = await executeTool( 'function_execute', @@ -317,6 +317,7 @@ export class AgentBlockHandler implements BlockHandler { workflowVariables: ctx.workflowVariables || {}, blockData, blockNameMapping, + blockOutputSchemas, isCustomTool: true, _context: { workflowId: ctx.workflowId, diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index f450460589..2954b06d89 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -26,7 +26,7 @@ export async function evaluateConditionExpression( const contextSetup = `const context = ${JSON.stringify(evalContext)};` const code = `${contextSetup}\nreturn Boolean(${conditionExpression})` - const { blockData, blockNameMapping } = collectBlockData(ctx) + const { blockData, blockNameMapping, blockOutputSchemas } = collectBlockData(ctx) const result = await executeTool( 'function_execute', @@ -37,6 +37,7 @@ export async function evaluateConditionExpression( workflowVariables: ctx.workflowVariables || {}, blockData, blockNameMapping, + blockOutputSchemas, _context: { workflowId: ctx.workflowId, workspaceId: ctx.workspaceId, diff --git a/apps/sim/executor/handlers/function/function-handler.test.ts b/apps/sim/executor/handlers/function/function-handler.test.ts index f04de4662b..5426610c70 100644 --- a/apps/sim/executor/handlers/function/function-handler.test.ts +++ b/apps/sim/executor/handlers/function/function-handler.test.ts @@ -75,7 +75,12 @@ describe('FunctionBlockHandler', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, - _context: { workflowId: mockContext.workflowId, workspaceId: mockContext.workspaceId }, + blockOutputSchemas: {}, + _context: { + workflowId: mockContext.workflowId, + workspaceId: mockContext.workspaceId, + isDeployedContext: mockContext.isDeployedContext, + }, } const expectedOutput: any = { result: 'Success' } @@ -84,8 +89,8 @@ describe('FunctionBlockHandler', () => { expect(mockExecuteTool).toHaveBeenCalledWith( 'function_execute', expectedToolParams, - false, // skipPostProcess - mockContext // execution context + false, + mockContext ) expect(result).toEqual(expectedOutput) }) @@ -107,7 +112,12 @@ describe('FunctionBlockHandler', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, - _context: { workflowId: mockContext.workflowId, workspaceId: mockContext.workspaceId }, + blockOutputSchemas: {}, + _context: { + workflowId: mockContext.workflowId, + workspaceId: mockContext.workspaceId, + isDeployedContext: mockContext.isDeployedContext, + }, } const expectedOutput: any = { result: 'Success' } @@ -116,8 +126,8 @@ describe('FunctionBlockHandler', () => { expect(mockExecuteTool).toHaveBeenCalledWith( 'function_execute', expectedToolParams, - false, // skipPostProcess - mockContext // execution context + false, + mockContext ) expect(result).toEqual(expectedOutput) }) @@ -132,7 +142,12 @@ describe('FunctionBlockHandler', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, - _context: { workflowId: mockContext.workflowId, workspaceId: mockContext.workspaceId }, + blockOutputSchemas: {}, + _context: { + workflowId: mockContext.workflowId, + workspaceId: mockContext.workspaceId, + isDeployedContext: mockContext.isDeployedContext, + }, } await handler.execute(mockContext, mockBlock, inputs) diff --git a/apps/sim/executor/handlers/function/function-handler.ts b/apps/sim/executor/handlers/function/function-handler.ts index c7b9b00978..624a262d3a 100644 --- a/apps/sim/executor/handlers/function/function-handler.ts +++ b/apps/sim/executor/handlers/function/function-handler.ts @@ -23,7 +23,7 @@ export class FunctionBlockHandler implements BlockHandler { ? inputs.code.map((c: { content: string }) => c.content).join('\n') : inputs.code - const { blockData, blockNameMapping } = collectBlockData(ctx) + const { blockData, blockNameMapping, blockOutputSchemas } = collectBlockData(ctx) const result = await executeTool( 'function_execute', @@ -35,6 +35,7 @@ export class FunctionBlockHandler implements BlockHandler { workflowVariables: ctx.workflowVariables || {}, blockData, blockNameMapping, + blockOutputSchemas, _context: { workflowId: ctx.workflowId, workspaceId: ctx.workspaceId, diff --git a/apps/sim/executor/utils/block-data.ts b/apps/sim/executor/utils/block-data.ts index fc7b26ae32..6941803741 100644 --- a/apps/sim/executor/utils/block-data.ts +++ b/apps/sim/executor/utils/block-data.ts @@ -1,24 +1,43 @@ +import { getBlockOutputs } from '@/lib/workflows/blocks/block-outputs' import { normalizeName } from '@/executor/constants' import type { ExecutionContext } from '@/executor/types' +import type { OutputSchema } from '@/executor/utils/block-reference' export interface BlockDataCollection { - blockData: Record + blockData: Record blockNameMapping: Record + blockOutputSchemas: Record } export function collectBlockData(ctx: ExecutionContext): BlockDataCollection { - const blockData: Record = {} + const blockData: Record = {} const blockNameMapping: Record = {} + const blockOutputSchemas: Record = {} for (const [id, state] of ctx.blockStates.entries()) { if (state.output !== undefined) { blockData[id] = state.output - const workflowBlock = ctx.workflow?.blocks?.find((b) => b.id === id) - if (workflowBlock?.metadata?.name) { - blockNameMapping[normalizeName(workflowBlock.metadata.name)] = id + } + + const workflowBlock = ctx.workflow?.blocks?.find((b) => b.id === id) + if (!workflowBlock) continue + + if (workflowBlock.metadata?.name) { + blockNameMapping[normalizeName(workflowBlock.metadata.name)] = id + } + + const blockType = workflowBlock.metadata?.id + if (blockType) { + const params = workflowBlock.config?.params as Record | undefined + const subBlocks = params + ? Object.fromEntries(Object.entries(params).map(([k, v]) => [k, { value: v }])) + : undefined + const schema = getBlockOutputs(blockType, subBlocks) + if (schema && Object.keys(schema).length > 0) { + blockOutputSchemas[id] = schema } } } - return { blockData, blockNameMapping } + return { blockData, blockNameMapping, blockOutputSchemas } } diff --git a/apps/sim/executor/utils/block-reference.test.ts b/apps/sim/executor/utils/block-reference.test.ts new file mode 100644 index 0000000000..6f110c2bc6 --- /dev/null +++ b/apps/sim/executor/utils/block-reference.test.ts @@ -0,0 +1,255 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + type BlockReferenceContext, + InvalidFieldError, + resolveBlockReference, +} from './block-reference' + +describe('resolveBlockReference', () => { + const createContext = ( + overrides: Partial = {} + ): BlockReferenceContext => ({ + blockNameMapping: { start: 'block-1', agent: 'block-2' }, + blockData: {}, + blockOutputSchemas: {}, + ...overrides, + }) + + describe('block name resolution', () => { + it('should return undefined when block name does not exist', () => { + const ctx = createContext() + const result = resolveBlockReference('unknown', ['field'], ctx) + expect(result).toBeUndefined() + }) + + it('should normalize block name before lookup', () => { + const ctx = createContext({ + blockNameMapping: { myblock: 'block-1' }, + blockData: { 'block-1': { value: 'test' } }, + }) + + const result = resolveBlockReference('MyBlock', ['value'], ctx) + expect(result).toEqual({ value: 'test', blockId: 'block-1' }) + }) + + it('should handle block names with spaces', () => { + const ctx = createContext({ + blockNameMapping: { myblock: 'block-1' }, + blockData: { 'block-1': { value: 'test' } }, + }) + + const result = resolveBlockReference('My Block', ['value'], ctx) + expect(result).toEqual({ value: 'test', blockId: 'block-1' }) + }) + }) + + describe('field resolution', () => { + it('should return entire block output when no path specified', () => { + const ctx = createContext({ + blockData: { 'block-1': { input: 'hello', other: 'data' } }, + }) + + const result = resolveBlockReference('start', [], ctx) + expect(result).toEqual({ + value: { input: 'hello', other: 'data' }, + blockId: 'block-1', + }) + }) + + it('should resolve simple field path', () => { + const ctx = createContext({ + blockData: { 'block-1': { input: 'hello' } }, + }) + + const result = resolveBlockReference('start', ['input'], ctx) + expect(result).toEqual({ value: 'hello', blockId: 'block-1' }) + }) + + it('should resolve nested field path', () => { + const ctx = createContext({ + blockData: { 'block-1': { response: { data: { name: 'test' } } } }, + }) + + const result = resolveBlockReference('start', ['response', 'data', 'name'], ctx) + expect(result).toEqual({ value: 'test', blockId: 'block-1' }) + }) + + it('should resolve array index path', () => { + const ctx = createContext({ + blockData: { 'block-1': { items: ['a', 'b', 'c'] } }, + }) + + const result = resolveBlockReference('start', ['items', '1'], ctx) + expect(result).toEqual({ value: 'b', blockId: 'block-1' }) + }) + + it('should return undefined value when field exists but has no value', () => { + const ctx = createContext({ + blockData: { 'block-1': { input: undefined } }, + blockOutputSchemas: { + 'block-1': { input: { type: 'string' } }, + }, + }) + + const result = resolveBlockReference('start', ['input'], ctx) + expect(result).toEqual({ value: undefined, blockId: 'block-1' }) + }) + + it('should return null value when field has null', () => { + const ctx = createContext({ + blockData: { 'block-1': { input: null } }, + }) + + const result = resolveBlockReference('start', ['input'], ctx) + expect(result).toEqual({ value: null, blockId: 'block-1' }) + }) + }) + + describe('schema validation', () => { + it('should throw InvalidFieldError when field not in schema', () => { + const ctx = createContext({ + blockData: { 'block-1': { existing: 'value' } }, + blockOutputSchemas: { + 'block-1': { + input: { type: 'string' }, + conversationId: { type: 'string' }, + }, + }, + }) + + expect(() => resolveBlockReference('start', ['invalid'], ctx)).toThrow(InvalidFieldError) + expect(() => resolveBlockReference('start', ['invalid'], ctx)).toThrow( + /"invalid" doesn't exist on block "start"/ + ) + }) + + it('should include available fields in error message', () => { + const ctx = createContext({ + blockData: { 'block-1': {} }, + blockOutputSchemas: { + 'block-1': { + input: { type: 'string' }, + conversationId: { type: 'string' }, + files: { type: 'files' }, + }, + }, + }) + + try { + resolveBlockReference('start', ['typo'], ctx) + expect.fail('Should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(InvalidFieldError) + const fieldError = error as InvalidFieldError + expect(fieldError.availableFields).toContain('input') + expect(fieldError.availableFields).toContain('conversationId') + expect(fieldError.availableFields).toContain('files') + } + }) + + it('should allow valid field even when value is undefined', () => { + const ctx = createContext({ + blockData: { 'block-1': {} }, + blockOutputSchemas: { + 'block-1': { input: { type: 'string' } }, + }, + }) + + const result = resolveBlockReference('start', ['input'], ctx) + expect(result).toEqual({ value: undefined, blockId: 'block-1' }) + }) + + it('should validate path when block has no output yet', () => { + const ctx = createContext({ + blockData: {}, + blockOutputSchemas: { + 'block-1': { input: { type: 'string' } }, + }, + }) + + expect(() => resolveBlockReference('start', ['invalid'], ctx)).toThrow(InvalidFieldError) + }) + + it('should return undefined for valid field when block has no output', () => { + const ctx = createContext({ + blockData: {}, + blockOutputSchemas: { + 'block-1': { input: { type: 'string' } }, + }, + }) + + const result = resolveBlockReference('start', ['input'], ctx) + expect(result).toEqual({ value: undefined, blockId: 'block-1' }) + }) + }) + + describe('without schema (pass-through mode)', () => { + it('should return undefined value without throwing when no schema', () => { + const ctx = createContext({ + blockData: { 'block-1': { existing: 'value' } }, + }) + + const result = resolveBlockReference('start', ['missing'], ctx) + expect(result).toEqual({ value: undefined, blockId: 'block-1' }) + }) + }) + + describe('file type handling', () => { + it('should allow file property access', () => { + const ctx = createContext({ + blockData: { + 'block-1': { + files: [{ name: 'test.txt', url: 'http://example.com/file' }], + }, + }, + blockOutputSchemas: { + 'block-1': { files: { type: 'files' } }, + }, + }) + + const result = resolveBlockReference('start', ['files', '0', 'name'], ctx) + expect(result).toEqual({ value: 'test.txt', blockId: 'block-1' }) + }) + + it('should validate file property names', () => { + const ctx = createContext({ + blockData: { 'block-1': { files: [] } }, + blockOutputSchemas: { + 'block-1': { files: { type: 'files' } }, + }, + }) + + expect(() => resolveBlockReference('start', ['files', '0', 'invalid'], ctx)).toThrow( + InvalidFieldError + ) + }) + }) +}) + +describe('InvalidFieldError', () => { + it('should have correct properties', () => { + const error = new InvalidFieldError('myBlock', 'invalid.path', ['field1', 'field2']) + + expect(error.blockName).toBe('myBlock') + expect(error.fieldPath).toBe('invalid.path') + expect(error.availableFields).toEqual(['field1', 'field2']) + expect(error.name).toBe('InvalidFieldError') + }) + + it('should format message correctly', () => { + const error = new InvalidFieldError('start', 'typo', ['input', 'files']) + + expect(error.message).toBe( + '"typo" doesn\'t exist on block "start". Available fields: input, files' + ) + }) + + it('should handle empty available fields', () => { + const error = new InvalidFieldError('start', 'field', []) + + expect(error.message).toBe('"field" doesn\'t exist on block "start". Available fields: none') + }) +}) diff --git a/apps/sim/executor/utils/block-reference.ts b/apps/sim/executor/utils/block-reference.ts new file mode 100644 index 0000000000..590e9d869d --- /dev/null +++ b/apps/sim/executor/utils/block-reference.ts @@ -0,0 +1,210 @@ +import { USER_FILE_ACCESSIBLE_PROPERTIES } from '@/lib/workflows/types' +import { normalizeName } from '@/executor/constants' +import { navigatePath } from '@/executor/variables/resolvers/reference' + +export type OutputSchema = Record + +export interface BlockReferenceContext { + blockNameMapping: Record + blockData: Record + blockOutputSchemas?: Record +} + +export interface BlockReferenceResult { + value: unknown + blockId: string +} + +export class InvalidFieldError extends Error { + constructor( + public readonly blockName: string, + public readonly fieldPath: string, + public readonly availableFields: string[] + ) { + super( + `"${fieldPath}" doesn't exist on block "${blockName}". ` + + `Available fields: ${availableFields.length > 0 ? availableFields.join(', ') : 'none'}` + ) + this.name = 'InvalidFieldError' + } +} + +function isFileType(value: unknown): boolean { + if (typeof value !== 'object' || value === null) return false + const typed = value as { type?: string } + return typed.type === 'file[]' || typed.type === 'files' +} + +function isArrayType(value: unknown): value is { type: 'array'; items?: unknown } { + if (typeof value !== 'object' || value === null) return false + return (value as { type?: string }).type === 'array' +} + +function getArrayItems(schema: unknown): unknown { + if (typeof schema !== 'object' || schema === null) return undefined + return (schema as { items?: unknown }).items +} + +function getProperties(schema: unknown): Record | undefined { + if (typeof schema !== 'object' || schema === null) return undefined + const props = (schema as { properties?: unknown }).properties + return typeof props === 'object' && props !== null + ? (props as Record) + : undefined +} + +function lookupField(schema: unknown, fieldName: string): unknown | undefined { + if (typeof schema !== 'object' || schema === null) return undefined + const typed = schema as Record + + if (fieldName in typed) { + return typed[fieldName] + } + + const props = getProperties(schema) + if (props && fieldName in props) { + return props[fieldName] + } + + return undefined +} + +function isPathInSchema(schema: OutputSchema | undefined, pathParts: string[]): boolean { + if (!schema || pathParts.length === 0) { + return true + } + + let current: unknown = schema + + for (let i = 0; i < pathParts.length; i++) { + const part = pathParts[i] + + if (current === null || current === undefined) { + return false + } + + if (/^\d+$/.test(part)) { + if (isFileType(current)) { + const nextPart = pathParts[i + 1] + return ( + !nextPart || + USER_FILE_ACCESSIBLE_PROPERTIES.includes( + nextPart as (typeof USER_FILE_ACCESSIBLE_PROPERTIES)[number] + ) + ) + } + if (isArrayType(current)) { + current = getArrayItems(current) + } + continue + } + + const arrayMatch = part.match(/^([^[]+)\[(\d+)\]$/) + if (arrayMatch) { + const [, prop] = arrayMatch + const fieldDef = lookupField(current, prop) + if (!fieldDef) return false + + if (isFileType(fieldDef)) { + const nextPart = pathParts[i + 1] + return ( + !nextPart || + USER_FILE_ACCESSIBLE_PROPERTIES.includes( + nextPart as (typeof USER_FILE_ACCESSIBLE_PROPERTIES)[number] + ) + ) + } + + current = isArrayType(fieldDef) ? getArrayItems(fieldDef) : fieldDef + continue + } + + if ( + isFileType(current) && + USER_FILE_ACCESSIBLE_PROPERTIES.includes( + part as (typeof USER_FILE_ACCESSIBLE_PROPERTIES)[number] + ) + ) { + return true + } + + const fieldDef = lookupField(current, part) + if (fieldDef !== undefined) { + if (isFileType(fieldDef)) { + const nextPart = pathParts[i + 1] + if (!nextPart) return true + if (/^\d+$/.test(nextPart)) { + const afterIndex = pathParts[i + 2] + return ( + !afterIndex || + USER_FILE_ACCESSIBLE_PROPERTIES.includes( + afterIndex as (typeof USER_FILE_ACCESSIBLE_PROPERTIES)[number] + ) + ) + } + return USER_FILE_ACCESSIBLE_PROPERTIES.includes( + nextPart as (typeof USER_FILE_ACCESSIBLE_PROPERTIES)[number] + ) + } + current = fieldDef + continue + } + + if (isArrayType(current)) { + const items = getArrayItems(current) + const itemField = lookupField(items, part) + if (itemField !== undefined) { + current = itemField + continue + } + } + + return false + } + + return true +} + +function getSchemaFieldNames(schema: OutputSchema | undefined): string[] { + if (!schema) return [] + return Object.keys(schema) +} + +export function resolveBlockReference( + blockName: string, + pathParts: string[], + context: BlockReferenceContext +): BlockReferenceResult | undefined { + const normalizedName = normalizeName(blockName) + const blockId = context.blockNameMapping[normalizedName] + + if (!blockId) { + return undefined + } + + const blockOutput = context.blockData[blockId] + const schema = context.blockOutputSchemas?.[blockId] + + if (blockOutput === undefined) { + if (schema && pathParts.length > 0) { + if (!isPathInSchema(schema, pathParts)) { + throw new InvalidFieldError(blockName, pathParts.join('.'), getSchemaFieldNames(schema)) + } + } + return { value: undefined, blockId } + } + + if (pathParts.length === 0) { + return { value: blockOutput, blockId } + } + + const value = navigatePath(blockOutput, pathParts) + + if (value === undefined && schema) { + if (!isPathInSchema(schema, pathParts)) { + throw new InvalidFieldError(blockName, pathParts.join('.'), getSchemaFieldNames(schema)) + } + } + + return { value, blockId } +} diff --git a/apps/sim/executor/variables/resolvers/block.ts b/apps/sim/executor/variables/resolvers/block.ts index 2bdee595b1..a29339b2bf 100644 --- a/apps/sim/executor/variables/resolvers/block.ts +++ b/apps/sim/executor/variables/resolvers/block.ts @@ -1,11 +1,15 @@ import { getBlockOutputs } from '@/lib/workflows/blocks/block-outputs' -import { USER_FILE_ACCESSIBLE_PROPERTIES } from '@/lib/workflows/types' import { isReference, normalizeName, parseReferencePath, SPECIAL_REFERENCE_PREFIXES, } from '@/executor/constants' +import { + InvalidFieldError, + type OutputSchema, + resolveBlockReference, +} from '@/executor/utils/block-reference' import { navigatePath, type ResolutionContext, @@ -14,123 +18,6 @@ import { import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types' import { getTool } from '@/tools/utils' -function isPathInOutputSchema( - outputs: Record | undefined, - pathParts: string[] -): boolean { - if (!outputs || pathParts.length === 0) { - return true - } - - const isFileArrayType = (value: any): boolean => - value?.type === 'file[]' || value?.type === 'files' - - let current: any = outputs - for (let i = 0; i < pathParts.length; i++) { - const part = pathParts[i] - - const arrayMatch = part.match(/^([^[]+)\[(\d+)\]$/) - if (arrayMatch) { - const [, prop] = arrayMatch - let fieldDef: any - - if (prop in current) { - fieldDef = current[prop] - } else if (current.properties && prop in current.properties) { - fieldDef = current.properties[prop] - } else if (current.type === 'array' && current.items) { - if (current.items.properties && prop in current.items.properties) { - fieldDef = current.items.properties[prop] - } else if (prop in current.items) { - fieldDef = current.items[prop] - } - } - - if (!fieldDef) { - return false - } - - if (isFileArrayType(fieldDef)) { - if (i + 1 < pathParts.length) { - return USER_FILE_ACCESSIBLE_PROPERTIES.includes(pathParts[i + 1] as any) - } - return true - } - - if (fieldDef.type === 'array' && fieldDef.items) { - current = fieldDef.items - continue - } - - current = fieldDef - continue - } - - if (/^\d+$/.test(part)) { - if (isFileArrayType(current)) { - if (i + 1 < pathParts.length) { - const nextPart = pathParts[i + 1] - return USER_FILE_ACCESSIBLE_PROPERTIES.includes(nextPart as any) - } - return true - } - continue - } - - if (current === null || current === undefined) { - return false - } - - if (part in current) { - const nextCurrent = current[part] - if (nextCurrent?.type === 'file[]' && i + 1 < pathParts.length) { - const nextPart = pathParts[i + 1] - if (/^\d+$/.test(nextPart) && i + 2 < pathParts.length) { - const propertyPart = pathParts[i + 2] - return USER_FILE_ACCESSIBLE_PROPERTIES.includes(propertyPart as any) - } - } - current = nextCurrent - continue - } - - if (current.properties && part in current.properties) { - current = current.properties[part] - continue - } - - if (current.type === 'array' && current.items) { - if (current.items.properties && part in current.items.properties) { - current = current.items.properties[part] - continue - } - if (part in current.items) { - current = current.items[part] - continue - } - } - - if (isFileArrayType(current) && USER_FILE_ACCESSIBLE_PROPERTIES.includes(part as any)) { - return true - } - - if ('type' in current && typeof current.type === 'string') { - if (!current.properties && !current.items) { - return false - } - } - - return false - } - - return true -} - -function getSchemaFieldNames(outputs: Record | undefined): string[] { - if (!outputs) return [] - return Object.keys(outputs) -} - export class BlockResolver implements Resolver { private nameToBlockId: Map private blockById: Map @@ -170,83 +57,94 @@ export class BlockResolver implements Resolver { return undefined } - const block = this.blockById.get(blockId) + const block = this.blockById.get(blockId)! const output = this.getBlockOutput(blockId, context) - if (output === undefined) { - return undefined + const blockData: Record = {} + const blockOutputSchemas: Record = {} + + if (output !== undefined) { + blockData[blockId] = output } - if (pathParts.length === 0) { - return output + + const blockType = block.metadata?.id + const params = block.config?.params as Record | undefined + const subBlocks = params + ? Object.fromEntries(Object.entries(params).map(([k, v]) => [k, { value: v }])) + : undefined + const toolId = block.config?.tool + const toolConfig = toolId ? getTool(toolId) : undefined + const outputSchema = + toolConfig?.outputs ?? (blockType ? getBlockOutputs(blockType, subBlocks) : block.outputs) + + if (outputSchema && Object.keys(outputSchema).length > 0) { + blockOutputSchemas[blockId] = outputSchema } - // Try the original path first - let result = navigatePath(output, pathParts) + try { + const result = resolveBlockReference(blockName, pathParts, { + blockNameMapping: Object.fromEntries(this.nameToBlockId), + blockData, + blockOutputSchemas, + })! - // If successful, return it immediately - if (result !== undefined) { - return result + if (result.value !== undefined) { + return result.value + } + + return this.handleBackwardsCompat(block, output, pathParts) + } catch (error) { + if (error instanceof InvalidFieldError) { + const fallback = this.handleBackwardsCompat(block, output, pathParts) + if (fallback !== undefined) { + return fallback + } + throw new Error(error.message) + } + throw error + } + } + + private handleBackwardsCompat( + block: SerializedBlock, + output: unknown, + pathParts: string[] + ): unknown { + if (output === undefined || pathParts.length === 0) { + return undefined } - // Response block backwards compatibility: - // Old: -> New: - // Only apply fallback if: - // 1. Block type is 'response' - // 2. Path starts with 'response.' - // 3. Output doesn't have a 'response' key (confirming it's the new format) if ( - block?.metadata?.id === 'response' && + block.metadata?.id === 'response' && pathParts[0] === 'response' && - output?.response === undefined + (output as Record)?.response === undefined ) { const adjustedPathParts = pathParts.slice(1) if (adjustedPathParts.length === 0) { return output } - result = navigatePath(output, adjustedPathParts) - if (result !== undefined) { - return result + const fallbackResult = navigatePath(output, adjustedPathParts) + if (fallbackResult !== undefined) { + return fallbackResult } } - // Workflow block backwards compatibility: - // Old: -> New: - // Only apply fallback if: - // 1. Block type is 'workflow' or 'workflow_input' - // 2. Path starts with 'result.response.' - // 3. output.result.response doesn't exist (confirming child used new format) const isWorkflowBlock = - block?.metadata?.id === 'workflow' || block?.metadata?.id === 'workflow_input' + block.metadata?.id === 'workflow' || block.metadata?.id === 'workflow_input' + const outputRecord = output as Record | undefined> if ( isWorkflowBlock && pathParts[0] === 'result' && pathParts[1] === 'response' && - output?.result?.response === undefined + outputRecord?.result?.response === undefined ) { const adjustedPathParts = ['result', ...pathParts.slice(2)] - result = navigatePath(output, adjustedPathParts) - if (result !== undefined) { - return result + const fallbackResult = navigatePath(output, adjustedPathParts) + if (fallbackResult !== undefined) { + return fallbackResult } } - const blockType = block?.metadata?.id - const params = block?.config?.params as Record | undefined - const subBlocks = params - ? Object.fromEntries(Object.entries(params).map(([k, v]) => [k, { value: v }])) - : undefined - const toolId = block?.config?.tool - const toolConfig = toolId ? getTool(toolId) : undefined - const outputSchema = - toolConfig?.outputs ?? (blockType ? getBlockOutputs(blockType, subBlocks) : block?.outputs) - const schemaFields = getSchemaFieldNames(outputSchema) - if (schemaFields.length > 0 && !isPathInOutputSchema(outputSchema, pathParts)) { - throw new Error( - `"${pathParts.join('.')}" doesn't exist on block "${blockName}". ` + - `Available fields: ${schemaFields.join(', ')}` - ) - } - return undefined } diff --git a/apps/sim/executor/variables/resolvers/reference.ts b/apps/sim/executor/variables/resolvers/reference.ts index 986ee2ab64..9f4b69eec5 100644 --- a/apps/sim/executor/variables/resolvers/reference.ts +++ b/apps/sim/executor/variables/resolvers/reference.ts @@ -27,23 +27,28 @@ export function navigatePath(obj: any, path: string[]): any { return undefined } - // Handle array indexing like "items[0]" or just numeric indices - const arrayMatch = part.match(/^([^[]+)\[(\d+)\](.*)$/) + const arrayMatch = part.match(/^([^[]+)(\[.+)$/) if (arrayMatch) { - // Handle complex array access like "items[0]" - const [, prop, index] = arrayMatch + const [, prop, bracketsPart] = arrayMatch current = current[prop] if (current === undefined || current === null) { return undefined } - const idx = Number.parseInt(index, 10) - current = Array.isArray(current) ? current[idx] : undefined + + const indices = bracketsPart.match(/\[(\d+)\]/g) + if (indices) { + for (const indexMatch of indices) { + if (current === null || current === undefined) { + return undefined + } + const idx = Number.parseInt(indexMatch.slice(1, -1), 10) + current = Array.isArray(current) ? current[idx] : undefined + } + } } else if (/^\d+$/.test(part)) { - // Handle plain numeric index const index = Number.parseInt(part, 10) current = Array.isArray(current) ? current[index] : undefined } else { - // Handle regular property access current = current[part] } } diff --git a/apps/sim/lib/execution/isolated-vm-worker.cjs b/apps/sim/lib/execution/isolated-vm-worker.cjs index 53aa5b6fc5..f6c587a15f 100644 --- a/apps/sim/lib/execution/isolated-vm-worker.cjs +++ b/apps/sim/lib/execution/isolated-vm-worker.cjs @@ -130,7 +130,11 @@ async function executeCode(request) { await jail.set('environmentVariables', new ivm.ExternalCopy(envVars).copyInto()) for (const [key, value] of Object.entries(contextVariables)) { - await jail.set(key, new ivm.ExternalCopy(value).copyInto()) + if (value === undefined) { + await jail.set(key, undefined) + } else { + await jail.set(key, new ivm.ExternalCopy(value).copyInto()) + } } const fetchCallback = new ivm.Reference(async (url, optionsJson) => { diff --git a/apps/sim/tools/function/execute.test.ts b/apps/sim/tools/function/execute.test.ts index c5ab2147c3..dc1b6eb20e 100644 --- a/apps/sim/tools/function/execute.test.ts +++ b/apps/sim/tools/function/execute.test.ts @@ -56,6 +56,7 @@ describe('Function Execute Tool', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, + blockOutputSchemas: {}, isCustomTool: false, language: 'javascript', timeout: 5000, @@ -83,6 +84,7 @@ describe('Function Execute Tool', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, + blockOutputSchemas: {}, isCustomTool: false, language: 'javascript', workflowId: undefined, @@ -101,6 +103,7 @@ describe('Function Execute Tool', () => { workflowVariables: {}, blockData: {}, blockNameMapping: {}, + blockOutputSchemas: {}, isCustomTool: false, language: 'javascript', workflowId: undefined, diff --git a/apps/sim/tools/function/execute.ts b/apps/sim/tools/function/execute.ts index 516c701270..d7f59daa89 100644 --- a/apps/sim/tools/function/execute.ts +++ b/apps/sim/tools/function/execute.ts @@ -53,6 +53,13 @@ export const functionExecuteTool: ToolConfig blockData?: Record blockNameMapping?: Record + blockOutputSchemas?: Record> _context?: { workflowId?: string } From ab09a5ad23bb68038f373794d88c3caa359ac9d3 Mon Sep 17 00:00:00 2001 From: Waleed Date: Thu, 22 Jan 2026 12:43:57 -0800 Subject: [PATCH 6/8] feat(router): expose reasoning output in router v2 block (#2945) --- apps/sim/blocks/blocks/router.ts | 11 +- .../handlers/router/router-handler.test.ts | 299 +++++++++++++++++- .../handlers/router/router-handler.ts | 44 ++- 3 files changed, 332 insertions(+), 22 deletions(-) diff --git a/apps/sim/blocks/blocks/router.ts b/apps/sim/blocks/blocks/router.ts index ecc48f5b70..fa086becb4 100644 --- a/apps/sim/blocks/blocks/router.ts +++ b/apps/sim/blocks/blocks/router.ts @@ -129,12 +129,9 @@ ROUTING RULES: 3. If the context is even partially related to a route's description, select that route 4. ONLY output NO_MATCH if the context is completely unrelated to ALL route descriptions -OUTPUT FORMAT: -- Output EXACTLY one route ID (copied exactly as shown above) OR "NO_MATCH" -- No explanation, no punctuation, no additional text -- Just the route ID or NO_MATCH - -Your response:` +Respond with a JSON object containing: +- route: EXACTLY one route ID (copied exactly as shown above) OR "NO_MATCH" +- reasoning: A brief explanation (1-2 sentences) of why you chose this route` } /** @@ -272,6 +269,7 @@ interface RouterV2Response extends ToolResponse { total: number } selectedRoute: string + reasoning: string selectedPath: { blockId: string blockType: string @@ -355,6 +353,7 @@ export const RouterV2Block: BlockConfig = { tokens: { type: 'json', description: 'Token usage' }, cost: { type: 'json', description: 'Cost information' }, selectedRoute: { type: 'string', description: 'Selected route ID' }, + reasoning: { type: 'string', description: 'Explanation of why this route was chosen' }, selectedPath: { type: 'json', description: 'Selected routing path' }, }, } diff --git a/apps/sim/executor/handlers/router/router-handler.test.ts b/apps/sim/executor/handlers/router/router-handler.test.ts index d0e28f97eb..cde4323740 100644 --- a/apps/sim/executor/handlers/router/router-handler.test.ts +++ b/apps/sim/executor/handlers/router/router-handler.test.ts @@ -1,7 +1,7 @@ import '@sim/testing/mocks/executor' import { beforeEach, describe, expect, it, type Mock, vi } from 'vitest' -import { generateRouterPrompt } from '@/blocks/blocks/router' +import { generateRouterPrompt, generateRouterV2Prompt } from '@/blocks/blocks/router' import { BlockType } from '@/executor/constants' import { RouterBlockHandler } from '@/executor/handlers/router/router-handler' import type { ExecutionContext } from '@/executor/types' @@ -9,6 +9,7 @@ import { getProviderFromModel } from '@/providers/utils' import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types' const mockGenerateRouterPrompt = generateRouterPrompt as Mock +const mockGenerateRouterV2Prompt = generateRouterV2Prompt as Mock const mockGetProviderFromModel = getProviderFromModel as Mock const mockFetch = global.fetch as unknown as Mock @@ -44,7 +45,7 @@ describe('RouterBlockHandler', () => { metadata: { id: BlockType.ROUTER, name: 'Test Router' }, position: { x: 50, y: 50 }, config: { tool: BlockType.ROUTER, params: {} }, - inputs: { prompt: 'string', model: 'string' }, // Using ParamType strings + inputs: { prompt: 'string', model: 'string' }, outputs: {}, enabled: true, } @@ -72,14 +73,11 @@ describe('RouterBlockHandler', () => { workflow: mockWorkflow as SerializedWorkflow, } - // Reset mocks using vi vi.clearAllMocks() - // Default mock implementations mockGetProviderFromModel.mockReturnValue('openai') mockGenerateRouterPrompt.mockReturnValue('Generated System Prompt') - // Set up fetch mock to return a successful response mockFetch.mockImplementation(() => { return Promise.resolve({ ok: true, @@ -147,7 +145,6 @@ describe('RouterBlockHandler', () => { }) ) - // Verify the request body contains the expected data const fetchCallArgs = mockFetch.mock.calls[0] const requestBody = JSON.parse(fetchCallArgs[1].body) expect(requestBody).toMatchObject({ @@ -180,7 +177,6 @@ describe('RouterBlockHandler', () => { const inputs = { prompt: 'Test' } mockContext.workflow!.blocks = [mockBlock, mockTargetBlock2] - // Expect execute to throw because getTargetBlocks (called internally) will throw await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( 'Target block target-block-1 not found' ) @@ -190,7 +186,6 @@ describe('RouterBlockHandler', () => { it('should throw error if LLM response is not a valid target block ID', async () => { const inputs = { prompt: 'Test', apiKey: 'test-api-key' } - // Override fetch mock to return an invalid block ID mockFetch.mockImplementationOnce(() => { return Promise.resolve({ ok: true, @@ -228,7 +223,6 @@ describe('RouterBlockHandler', () => { it('should handle server error responses', async () => { const inputs = { prompt: 'Test error handling.', apiKey: 'test-api-key' } - // Override fetch mock to return an error mockFetch.mockImplementationOnce(() => { return Promise.resolve({ ok: false, @@ -276,13 +270,12 @@ describe('RouterBlockHandler', () => { mockGetProviderFromModel.mockReturnValue('vertex') - // Mock the database query for Vertex credential const mockDb = await import('@sim/db') const mockAccount = { id: 'test-vertex-credential-id', accessToken: 'mock-access-token', refreshToken: 'mock-refresh-token', - expiresAt: new Date(Date.now() + 3600000), // 1 hour from now + expiresAt: new Date(Date.now() + 3600000), } vi.spyOn(mockDb.db.query.account, 'findFirst').mockResolvedValue(mockAccount as any) @@ -300,3 +293,287 @@ describe('RouterBlockHandler', () => { expect(requestBody.apiKey).toBe('mock-access-token') }) }) + +describe('RouterBlockHandler V2', () => { + let handler: RouterBlockHandler + let mockRouterV2Block: SerializedBlock + let mockContext: ExecutionContext + let mockWorkflow: Partial + let mockTargetBlock1: SerializedBlock + let mockTargetBlock2: SerializedBlock + + beforeEach(() => { + mockTargetBlock1 = { + id: 'target-block-1', + metadata: { id: 'agent', name: 'Support Agent' }, + position: { x: 100, y: 100 }, + config: { tool: 'agent', params: {} }, + inputs: {}, + outputs: {}, + enabled: true, + } + mockTargetBlock2 = { + id: 'target-block-2', + metadata: { id: 'agent', name: 'Sales Agent' }, + position: { x: 100, y: 150 }, + config: { tool: 'agent', params: {} }, + inputs: {}, + outputs: {}, + enabled: true, + } + mockRouterV2Block = { + id: 'router-v2-block-1', + metadata: { id: BlockType.ROUTER_V2, name: 'Test Router V2' }, + position: { x: 50, y: 50 }, + config: { tool: BlockType.ROUTER_V2, params: {} }, + inputs: {}, + outputs: {}, + enabled: true, + } + mockWorkflow = { + blocks: [mockRouterV2Block, mockTargetBlock1, mockTargetBlock2], + connections: [ + { + source: mockRouterV2Block.id, + target: mockTargetBlock1.id, + sourceHandle: 'router-route-support', + }, + { + source: mockRouterV2Block.id, + target: mockTargetBlock2.id, + sourceHandle: 'router-route-sales', + }, + ], + } + + handler = new RouterBlockHandler({}) + + mockContext = { + workflowId: 'test-workflow-id', + blockStates: new Map(), + blockLogs: [], + metadata: { duration: 0 }, + environmentVariables: {}, + decisions: { router: new Map(), condition: new Map() }, + loopExecutions: new Map(), + completedLoops: new Set(), + executedBlocks: new Set(), + activeExecutionPath: new Set(), + workflow: mockWorkflow as SerializedWorkflow, + } + + vi.clearAllMocks() + + mockGetProviderFromModel.mockReturnValue('openai') + mockGenerateRouterV2Prompt.mockReturnValue('Generated V2 System Prompt') + }) + + it('should handle router_v2 blocks', () => { + expect(handler.canHandle(mockRouterV2Block)).toBe(true) + }) + + it('should execute router V2 and return reasoning', async () => { + const inputs = { + context: 'I need help with a billing issue', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: JSON.stringify([ + { id: 'route-support', title: 'Support', value: 'Customer support inquiries' }, + { id: 'route-sales', title: 'Sales', value: 'Sales and pricing questions' }, + ]), + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: JSON.stringify({ + route: 'route-support', + reasoning: 'The user mentioned a billing issue which is a customer support matter.', + }), + model: 'gpt-4o', + tokens: { input: 150, output: 25, total: 175 }, + }), + }) + }) + + const result = await handler.execute(mockContext, mockRouterV2Block, inputs) + + expect(result).toMatchObject({ + context: 'I need help with a billing issue', + model: 'gpt-4o', + selectedRoute: 'route-support', + reasoning: 'The user mentioned a billing issue which is a customer support matter.', + selectedPath: { + blockId: 'target-block-1', + blockType: 'agent', + blockTitle: 'Support Agent', + }, + }) + }) + + it('should include responseFormat in provider request', async () => { + const inputs = { + context: 'Test context', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: JSON.stringify([{ id: 'route-1', title: 'Route 1', value: 'Description 1' }]), + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: JSON.stringify({ route: 'route-1', reasoning: 'Test reasoning' }), + model: 'gpt-4o', + tokens: { input: 100, output: 20, total: 120 }, + }), + }) + }) + + await handler.execute(mockContext, mockRouterV2Block, inputs) + + const fetchCallArgs = mockFetch.mock.calls[0] + const requestBody = JSON.parse(fetchCallArgs[1].body) + + expect(requestBody.responseFormat).toEqual({ + name: 'router_response', + schema: { + type: 'object', + properties: { + route: { + type: 'string', + description: 'The selected route ID or NO_MATCH', + }, + reasoning: { + type: 'string', + description: 'Brief explanation of why this route was chosen', + }, + }, + required: ['route', 'reasoning'], + additionalProperties: false, + }, + strict: true, + }) + }) + + it('should handle NO_MATCH response with reasoning', async () => { + const inputs = { + context: 'Random unrelated query', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: JSON.stringify([{ id: 'route-1', title: 'Route 1', value: 'Specific topic' }]), + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: JSON.stringify({ + route: 'NO_MATCH', + reasoning: 'The query does not relate to any available route.', + }), + model: 'gpt-4o', + tokens: { input: 100, output: 20, total: 120 }, + }), + }) + }) + + await expect(handler.execute(mockContext, mockRouterV2Block, inputs)).rejects.toThrow( + 'Router could not determine a matching route: The query does not relate to any available route.' + ) + }) + + it('should throw error for invalid route ID in response', async () => { + const inputs = { + context: 'Test context', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: JSON.stringify([{ id: 'route-1', title: 'Route 1', value: 'Description' }]), + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: JSON.stringify({ route: 'invalid-route', reasoning: 'Some reasoning' }), + model: 'gpt-4o', + tokens: { input: 100, output: 20, total: 120 }, + }), + }) + }) + + await expect(handler.execute(mockContext, mockRouterV2Block, inputs)).rejects.toThrow( + /Router could not determine a valid route/ + ) + }) + + it('should handle routes passed as array instead of JSON string', async () => { + const inputs = { + context: 'Test context', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: [{ id: 'route-1', title: 'Route 1', value: 'Description' }], + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: JSON.stringify({ route: 'route-1', reasoning: 'Matched route 1' }), + model: 'gpt-4o', + tokens: { input: 100, output: 20, total: 120 }, + }), + }) + }) + + const result = await handler.execute(mockContext, mockRouterV2Block, inputs) + + expect(result.selectedRoute).toBe('route-1') + expect(result.reasoning).toBe('Matched route 1') + }) + + it('should throw error when no routes are defined', async () => { + const inputs = { + context: 'Test context', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: '[]', + } + + await expect(handler.execute(mockContext, mockRouterV2Block, inputs)).rejects.toThrow( + 'No routes defined for router' + ) + }) + + it('should handle fallback when JSON parsing fails', async () => { + const inputs = { + context: 'Test context', + model: 'gpt-4o', + apiKey: 'test-api-key', + routes: JSON.stringify([{ id: 'route-1', title: 'Route 1', value: 'Description' }]), + } + + mockFetch.mockImplementationOnce(() => { + return Promise.resolve({ + ok: true, + json: () => + Promise.resolve({ + content: 'route-1', + model: 'gpt-4o', + tokens: { input: 100, output: 5, total: 105 }, + }), + }) + }) + + const result = await handler.execute(mockContext, mockRouterV2Block, inputs) + + expect(result.selectedRoute).toBe('route-1') + expect(result.reasoning).toBe('') + }) +}) diff --git a/apps/sim/executor/handlers/router/router-handler.ts b/apps/sim/executor/handlers/router/router-handler.ts index c6290fd173..eb5cf85aef 100644 --- a/apps/sim/executor/handlers/router/router-handler.ts +++ b/apps/sim/executor/handlers/router/router-handler.ts @@ -238,6 +238,25 @@ export class RouterBlockHandler implements BlockHandler { apiKey: finalApiKey, workflowId: ctx.workflowId, workspaceId: ctx.workspaceId, + responseFormat: { + name: 'router_response', + schema: { + type: 'object', + properties: { + route: { + type: 'string', + description: 'The selected route ID or NO_MATCH', + }, + reasoning: { + type: 'string', + description: 'Brief explanation of why this route was chosen', + }, + }, + required: ['route', 'reasoning'], + additionalProperties: false, + }, + strict: true, + }, } if (providerId === 'vertex') { @@ -277,16 +296,31 @@ export class RouterBlockHandler implements BlockHandler { const result = await response.json() - const chosenRouteId = result.content.trim() + let chosenRouteId: string + let reasoning = '' + + try { + const parsedResponse = JSON.parse(result.content) + chosenRouteId = parsedResponse.route?.trim() || '' + reasoning = parsedResponse.reasoning || '' + } catch (_parseError) { + logger.error('Router response was not valid JSON despite responseFormat', { + content: result.content, + }) + chosenRouteId = result.content.trim() + } if (chosenRouteId === 'NO_MATCH' || chosenRouteId.toUpperCase() === 'NO_MATCH') { logger.info('Router determined no route matches the context, routing to error path') - throw new Error('Router could not determine a matching route for the given context') + throw new Error( + reasoning + ? `Router could not determine a matching route: ${reasoning}` + : 'Router could not determine a matching route for the given context' + ) } const chosenRoute = routes.find((r) => r.id === chosenRouteId) - // Throw error if LLM returns invalid route ID - this routes through error path if (!chosenRoute) { const availableRoutes = routes.map((r) => ({ id: r.id, title: r.title })) logger.error( @@ -298,7 +332,6 @@ export class RouterBlockHandler implements BlockHandler { ) } - // Find the target block connected to this route's handle const connection = ctx.workflow?.connections.find( (conn) => conn.source === block.id && conn.sourceHandle === `router-${chosenRoute.id}` ) @@ -334,6 +367,7 @@ export class RouterBlockHandler implements BlockHandler { total: cost.total, }, selectedRoute: chosenRoute.id, + reasoning, selectedPath: targetBlock ? { blockId: targetBlock.id, @@ -353,7 +387,7 @@ export class RouterBlockHandler implements BlockHandler { } /** - * Parse routes from input (can be JSON string or array). + * Parse routes from input (can be JSON string or array) */ private parseRoutes(input: any): RouteDefinition[] { try { From 91da7e183a800a329dcd62d384be01c00dd79e8c Mon Sep 17 00:00:00 2001 From: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com> Date: Thu, 22 Jan 2026 13:07:16 -0800 Subject: [PATCH 7/8] fix(copilot): always allow, credential masking (#2947) * Fix always allow, credential validation * Credential masking * Autoload --- .../copilot-message/copilot-message.tsx | 10 +- .../components/tool-call/tool-call.tsx | 36 ++- .../hooks/use-copilot-initialization.ts | 4 +- .../tools/server/workflow/edit-workflow.ts | 9 +- .../copilot/validation/selector-validator.ts | 25 ++ apps/sim/stores/panel/copilot/store.ts | 244 ++++++++++++++++-- apps/sim/stores/panel/copilot/types.ts | 7 + 7 files changed, 300 insertions(+), 35 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/copilot-message/copilot-message.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/copilot-message/copilot-message.tsx index ea780add22..acbb30ff20 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/copilot-message/copilot-message.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/copilot-message/copilot-message.tsx @@ -78,6 +78,7 @@ const CopilotMessage: FC = memo( mode, setMode, isAborting, + maskCredentialValue, } = useCopilotStore() const messageCheckpoints = isUser ? allMessageCheckpoints[message.id] || [] : [] @@ -210,7 +211,10 @@ const CopilotMessage: FC = memo( const isLastTextBlock = index === message.contentBlocks!.length - 1 && block.type === 'text' const parsed = parseSpecialTags(block.content) - const cleanBlockContent = parsed.cleanContent.replace(/\n{3,}/g, '\n\n') + // Mask credential IDs in the displayed content + const cleanBlockContent = maskCredentialValue( + parsed.cleanContent.replace(/\n{3,}/g, '\n\n') + ) if (!cleanBlockContent.trim()) return null @@ -238,7 +242,7 @@ const CopilotMessage: FC = memo( return (
= memo( } return null }) - }, [message.contentBlocks, isActivelyStreaming, parsedTags, isLastMessage]) + }, [message.contentBlocks, isActivelyStreaming, parsedTags, isLastMessage, maskCredentialValue]) if (isUser) { return ( diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx index fd601b7ea7..c97025aeeb 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/tool-call/tool-call.tsx @@ -782,6 +782,7 @@ const SubagentContentRenderer = memo(function SubagentContentRenderer({ const [isExpanded, setIsExpanded] = useState(true) const [duration, setDuration] = useState(0) const startTimeRef = useRef(Date.now()) + const maskCredentialValue = useCopilotStore((s) => s.maskCredentialValue) const wasStreamingRef = useRef(false) // Only show streaming animations for current message @@ -816,14 +817,16 @@ const SubagentContentRenderer = memo(function SubagentContentRenderer({ currentText += parsed.cleanContent } else if (block.type === 'subagent_tool_call' && block.toolCall) { if (currentText.trim()) { - segments.push({ type: 'text', content: currentText }) + // Mask any credential IDs in the accumulated text before displaying + segments.push({ type: 'text', content: maskCredentialValue(currentText) }) currentText = '' } segments.push({ type: 'tool', block }) } } if (currentText.trim()) { - segments.push({ type: 'text', content: currentText }) + // Mask any credential IDs in the accumulated text before displaying + segments.push({ type: 'text', content: maskCredentialValue(currentText) }) } const allParsed = parseSpecialTags(allRawText) @@ -952,6 +955,7 @@ const WorkflowEditSummary = memo(function WorkflowEditSummary({ toolCall: CopilotToolCall }) { const blocks = useWorkflowStore((s) => s.blocks) + const maskCredentialValue = useCopilotStore((s) => s.maskCredentialValue) const cachedBlockInfoRef = useRef>({}) @@ -983,6 +987,7 @@ const WorkflowEditSummary = memo(function WorkflowEditSummary({ title: string value: any isPassword?: boolean + isCredential?: boolean } interface BlockChange { @@ -1091,6 +1096,7 @@ const WorkflowEditSummary = memo(function WorkflowEditSummary({ title: subBlockConfig.title ?? subBlockConfig.id, value, isPassword: subBlockConfig.password === true, + isCredential: subBlockConfig.type === 'oauth-input', }) } } @@ -1172,8 +1178,15 @@ const WorkflowEditSummary = memo(function WorkflowEditSummary({ {subBlocksToShow && subBlocksToShow.length > 0 && (
{subBlocksToShow.map((sb) => { - // Mask password fields like the canvas does - const displayValue = sb.isPassword ? '•••' : getDisplayValue(sb.value) + // Mask password fields and credential IDs + let displayValue: string + if (sb.isPassword) { + displayValue = '•••' + } else { + // Get display value first, then mask any credential IDs that might be in it + const rawValue = getDisplayValue(sb.value) + displayValue = maskCredentialValue(rawValue) + } return (
+ + + {getTooltipMessage(isEnabled ? 'Disable Block' : 'Enable Block')} + + + )} + {!isStartBlock && !isResponseBlock && ( @@ -222,29 +245,6 @@ export const ActionBar = memo( )} - {isSubflowBlock && ( - - - - - - {getTooltipMessage(isEnabled ? 'Disable Block' : 'Enable Block')} - - - )} -