Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions apps/sim/executor/utils/block-reference.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,20 @@ describe('resolveBlockReference', () => {
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
})

it('should validate path when block has no output yet', () => {
it('should not validate path when block has no output yet', () => {
// Blocks with no output typically live on a branched path that wasn't
// taken this run. We resolve such references to undefined (which the
// caller maps to RESOLVED_EMPTY) rather than throwing on every nested
// path the schema doesn't pre-declare.
const ctx = createContext({
blockData: {},
blockOutputSchemas: {
'block-1': { input: { type: 'string' } },
},
})

expect(() => resolveBlockReference('start', ['invalid'], ctx)).toThrow(InvalidFieldError)
const result = resolveBlockReference('start', ['invalid'], ctx)
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
})

it('should return undefined for valid field when block has no output', () => {
Expand All @@ -184,6 +189,57 @@ describe('resolveBlockReference', () => {
const result = resolveBlockReference('start', ['input'], ctx)
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
})

it('should return undefined for nested path under json field when block has no output', () => {
// Repro for the branched-path bug: a function block with a dynamic
// `json` result that never ran should resolve to undefined regardless
// of the nested path, not throw.
const ctx = createContext({
blockData: {},
blockOutputSchemas: {
'block-1': {
result: { type: 'json' },
stdout: { type: 'string' },
},
},
})

const result = resolveBlockReference('start', ['result', 'summary'], ctx)
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
})

it('should not throw for nested path under json field on executed block', () => {
// A `json` field declares dynamic shape, so drilling into it must be
// permitted even when the runtime data doesn't happen to include that
// key on this run.
const ctx = createContext({
blockData: { 'block-1': { result: { foo: 1 } } },
blockOutputSchemas: {
'block-1': {
result: { type: 'json' },
stdout: { type: 'string' },
},
},
})

const result = resolveBlockReference('start', ['result', 'summary'], ctx)
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
})

it('should resolve values nested under json field on executed block', () => {
const ctx = createContext({
blockData: { 'block-1': { result: { summary: 'hello' } } },
blockOutputSchemas: {
'block-1': {
result: { type: 'json' },
stdout: { type: 'string' },
},
},
})

const result = resolveBlockReference('start', ['result', 'summary'], ctx)
expect(result).toEqual({ value: 'hello', blockId: 'block-1' })
})
})

describe('without schema (pass-through mode)', () => {
Expand Down
60 changes: 44 additions & 16 deletions apps/sim/executor/utils/block-reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,18 @@ 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<string, { type?: string; description?: string } | unknown>
/**
* A single schema node encountered while walking an `OutputSchema`. Captures
* only the fields this module inspects — not a full schema type.
*/
interface SchemaNode {
type?: string
description?: string
properties?: unknown
items?: unknown
}

export type OutputSchema = Record<string, SchemaNode | unknown>

export interface BlockReferenceContext {
blockNameMapping: Record<string, string>
Expand All @@ -29,25 +40,26 @@ export class InvalidFieldError extends Error {
}
}

function asSchemaNode(value: unknown): SchemaNode | undefined {
if (typeof value !== 'object' || value === null) return undefined
return value as SchemaNode
}

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 === 'file[]'
const node = asSchemaNode(value)
return node?.type === 'file' || node?.type === 'file[]'
}

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'
return asSchemaNode(value)?.type === 'array'
}

function getArrayItems(schema: unknown): unknown {
if (typeof schema !== 'object' || schema === null) return undefined
return (schema as { items?: unknown }).items
return asSchemaNode(schema)?.items
}

function getProperties(schema: unknown): Record<string, unknown> | undefined {
if (typeof schema !== 'object' || schema === null) return undefined
const props = (schema as { properties?: unknown }).properties
const props = asSchemaNode(schema)?.properties
return typeof props === 'object' && props !== null
? (props as Record<string, unknown>)
: undefined
Expand All @@ -69,6 +81,19 @@ function lookupField(schema: unknown, fieldName: string): unknown | undefined {
return undefined
}

function isOpaqueSchemaNode(value: unknown): boolean {
const node = asSchemaNode(value)
if (!node) return false
// A schema node whose nested shape isn't enumerated. Any path beneath it
// is accepted because there's no declared structure to validate against.
// `object` / `json` with declared `properties` are walked via lookupField.
if (node.type === 'any') return true
if ((node.type === 'json' || node.type === 'object') && node.properties === undefined) {
return true
}
return false
}

function isPathInSchema(schema: OutputSchema | undefined, pathParts: string[]): boolean {
if (!schema || pathParts.length === 0) {
return true
Expand All @@ -83,6 +108,10 @@ function isPathInSchema(schema: OutputSchema | undefined, pathParts: string[]):
return false
}

if (isOpaqueSchemaNode(current)) {
return true
}

if (/^\d+$/.test(part)) {
if (isFileType(current)) {
const nextPart = pathParts[i + 1]
Expand Down Expand Up @@ -183,14 +212,12 @@ export function resolveBlockReference(
}

const blockOutput = context.blockData[blockId]
const schema = context.blockOutputSchemas?.[blockId]

// When the block has not produced any output (e.g. it lives on a branched
// path that wasn't taken), resolve the reference to undefined without
// validating against the declared schema. Callers map this to an empty
// value so that references to skipped blocks don't fail the workflow.
if (blockOutput === undefined) {
if (schema && pathParts.length > 0) {
if (!isPathInSchema(schema, pathParts)) {
throw new InvalidFieldError(blockName, pathParts.join('.'), getSchemaFieldNames(schema))
}
}
return { value: undefined, blockId }
}

Expand All @@ -200,6 +227,7 @@ export function resolveBlockReference(

const value = navigatePath(blockOutput, pathParts)

const schema = context.blockOutputSchemas?.[blockId]
if (value === undefined && schema) {
if (!isPathInSchema(schema, pathParts)) {
throw new InvalidFieldError(blockName, pathParts.join('.'), getSchemaFieldNames(schema))
Expand Down
86 changes: 86 additions & 0 deletions apps/sim/executor/utils/subflow-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* @vitest-environment node
*/
import { describe, expect, it, vi } from 'vitest'
Comment thread
icecrasher321 marked this conversation as resolved.
import type { ExecutionContext } from '@/executor/types'
import type { VariableResolver } from '@/executor/variables/resolver'
import { resolveArrayInput } from './subflow-utils'
Comment thread
icecrasher321 marked this conversation as resolved.

describe('resolveArrayInput', () => {
const fakeCtx = {} as unknown as ExecutionContext

it('returns arrays as-is', () => {
expect(resolveArrayInput(fakeCtx, [1, 2, 3], null)).toEqual([1, 2, 3])
})

it('converts plain objects to entries', () => {
expect(resolveArrayInput(fakeCtx, { a: 1, b: 2 }, null)).toEqual([
['a', 1],
['b', 2],
])
})

it('returns empty array when a pure reference resolves to null (skipped block)', () => {
// `resolveSingleReference` returns `null` for a reference that points at a
// block that exists in the workflow but did not execute on this path.
// A loop/parallel over such a reference should run zero iterations rather
// than fail the workflow.
const resolver = {
resolveSingleReference: vi.fn().mockReturnValue(null),
} as unknown as VariableResolver

const result = resolveArrayInput(fakeCtx, '<SkippedBlock.result.items>', resolver)

expect(result).toEqual([])
expect(resolver.resolveSingleReference).toHaveBeenCalled()
})

it('returns the array from a pure reference that resolved to an array', () => {
const resolver = {
resolveSingleReference: vi.fn().mockReturnValue([1, 2, 3]),
} as unknown as VariableResolver

expect(resolveArrayInput(fakeCtx, '<Block.items>', resolver)).toEqual([1, 2, 3])
})

it('converts resolved objects to entries', () => {
const resolver = {
resolveSingleReference: vi.fn().mockReturnValue({ x: 1, y: 2 }),
} as unknown as VariableResolver

expect(resolveArrayInput(fakeCtx, '<Block.obj>', resolver)).toEqual([
['x', 1],
['y', 2],
])
})

it('throws when a pure reference resolves to a non-array, non-object, non-null value', () => {
const resolver = {
resolveSingleReference: vi.fn().mockReturnValue(42),
} as unknown as VariableResolver

expect(() => resolveArrayInput(fakeCtx, '<Block.count>', resolver)).toThrow(
/did not resolve to an array or object/
)
})

it('throws when a pure reference resolves to undefined (unknown block)', () => {
// `undefined` means the reference could not be matched to any block at
// all (typo / deleted block). This must still fail loudly.
const resolver = {
resolveSingleReference: vi.fn().mockReturnValue(undefined),
} as unknown as VariableResolver

expect(() => resolveArrayInput(fakeCtx, '<Missing.items>', resolver)).toThrow(
/did not resolve to an array or object/
)
})

it('parses a JSON array string', () => {
expect(resolveArrayInput(fakeCtx, '[1, 2, 3]', null)).toEqual([1, 2, 3])
})

it('throws on a string that is neither a reference nor valid JSON array/object', () => {
expect(() => resolveArrayInput(fakeCtx, 'not json', null)).toThrow()
})
})
3 changes: 3 additions & 0 deletions apps/sim/executor/utils/subflow-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ export function resolveArrayInput(
if (typeof resolved === 'object' && resolved !== null) {
return Object.entries(resolved)
}
if (resolved === null) {
return []
}
throw new Error(`Reference "${items}" did not resolve to an array or object`)
} catch (error) {
if (error instanceof Error && error.message.startsWith('Reference "')) {
Expand Down
41 changes: 41 additions & 0 deletions apps/sim/executor/variables/resolvers/block.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,47 @@ describe('BlockResolver', () => {
expect(resolver.resolve('<start.input>', ctx)).toBe(RESOLVED_EMPTY)
})

it.concurrent(
'should return RESOLVED_EMPTY for nested json path on function block that did not execute',
() => {
// Repro for the branched-trigger CRM workflow bug: a function block
// on an untaken branch is referenced via a nested path under its
// `result` (declared `type: 'json'`). The resolver must not validate
// the path against the declared top-level schema keys in this case.
const workflow = createTestWorkflow([
{ id: 'normalize-email', name: 'NormalizeEmail', type: 'function' },
{ id: 'normalize-calendar', name: 'NormalizeCalendar', type: 'function' },
])
const resolver = new BlockResolver(workflow)
const ctx = createTestContext('current', {
'normalize-email': { result: { summary: 'email summary' }, stdout: '' },
})

expect(resolver.resolve('<NormalizeEmail.result.summary>', ctx)).toBe('email summary')
expect(resolver.resolve('<NormalizeCalendar.result.summary>', ctx)).toBe(RESOLVED_EMPTY)
expect(resolver.resolve('<NormalizeCalendar.result>', ctx)).toBe(RESOLVED_EMPTY)
}
)

it.concurrent(
'should return RESOLVED_EMPTY for nested json path on executed block when data is missing',
() => {
// Even for a block that ran, drilling into a `json`-typed field that
// the runtime output didn't include should resolve to empty rather
// than throw — `json` explicitly means dynamic shape.
const workflow = createTestWorkflow([
{ id: 'normalize-email', name: 'NormalizeEmail', type: 'function' },
])
const resolver = new BlockResolver(workflow)
const ctx = createTestContext('current', {
'normalize-email': { result: { subject: 'hi' }, stdout: '' },
})

expect(resolver.resolve('<NormalizeEmail.result.subject>', ctx)).toBe('hi')
expect(resolver.resolve('<NormalizeEmail.result.summary>', ctx)).toBe(RESOLVED_EMPTY)
}
)

it.concurrent('should fall back to context blockStates', () => {
const workflow = createTestWorkflow([{ id: 'source' }])
const resolver = new BlockResolver(workflow)
Expand Down
Loading