Skip to content

Commit 3d909d5

Browse files
fix(resolver): turn off resolver for opaque schema nodes, unrun paths (#4208)
* fix(resolver): turn off resolver for opaque schema nodes, unrun paths * fix subflows to make them consistent * fix tests
1 parent 3e2a7a2 commit 3d909d5

File tree

5 files changed

+232
-18
lines changed

5 files changed

+232
-18
lines changed

apps/sim/executor/utils/block-reference.test.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,15 +162,20 @@ describe('resolveBlockReference', () => {
162162
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
163163
})
164164

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

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

176181
it('should return undefined for valid field when block has no output', () => {
@@ -184,6 +189,57 @@ describe('resolveBlockReference', () => {
184189
const result = resolveBlockReference('start', ['input'], ctx)
185190
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
186191
})
192+
193+
it('should return undefined for nested path under json field when block has no output', () => {
194+
// Repro for the branched-path bug: a function block with a dynamic
195+
// `json` result that never ran should resolve to undefined regardless
196+
// of the nested path, not throw.
197+
const ctx = createContext({
198+
blockData: {},
199+
blockOutputSchemas: {
200+
'block-1': {
201+
result: { type: 'json' },
202+
stdout: { type: 'string' },
203+
},
204+
},
205+
})
206+
207+
const result = resolveBlockReference('start', ['result', 'summary'], ctx)
208+
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
209+
})
210+
211+
it('should not throw for nested path under json field on executed block', () => {
212+
// A `json` field declares dynamic shape, so drilling into it must be
213+
// permitted even when the runtime data doesn't happen to include that
214+
// key on this run.
215+
const ctx = createContext({
216+
blockData: { 'block-1': { result: { foo: 1 } } },
217+
blockOutputSchemas: {
218+
'block-1': {
219+
result: { type: 'json' },
220+
stdout: { type: 'string' },
221+
},
222+
},
223+
})
224+
225+
const result = resolveBlockReference('start', ['result', 'summary'], ctx)
226+
expect(result).toEqual({ value: undefined, blockId: 'block-1' })
227+
})
228+
229+
it('should resolve values nested under json field on executed block', () => {
230+
const ctx = createContext({
231+
blockData: { 'block-1': { result: { summary: 'hello' } } },
232+
blockOutputSchemas: {
233+
'block-1': {
234+
result: { type: 'json' },
235+
stdout: { type: 'string' },
236+
},
237+
},
238+
})
239+
240+
const result = resolveBlockReference('start', ['result', 'summary'], ctx)
241+
expect(result).toEqual({ value: 'hello', blockId: 'block-1' })
242+
})
187243
})
188244

189245
describe('without schema (pass-through mode)', () => {

apps/sim/executor/utils/block-reference.ts

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,18 @@ import { USER_FILE_ACCESSIBLE_PROPERTIES } from '@/lib/workflows/types'
22
import { normalizeName } from '@/executor/constants'
33
import { navigatePath } from '@/executor/variables/resolvers/reference'
44

5-
export type OutputSchema = Record<string, { type?: string; description?: string } | unknown>
5+
/**
6+
* A single schema node encountered while walking an `OutputSchema`. Captures
7+
* only the fields this module inspects — not a full schema type.
8+
*/
9+
interface SchemaNode {
10+
type?: string
11+
description?: string
12+
properties?: unknown
13+
items?: unknown
14+
}
15+
16+
export type OutputSchema = Record<string, SchemaNode | unknown>
617

718
export interface BlockReferenceContext {
819
blockNameMapping: Record<string, string>
@@ -29,25 +40,26 @@ export class InvalidFieldError extends Error {
2940
}
3041
}
3142

43+
function asSchemaNode(value: unknown): SchemaNode | undefined {
44+
if (typeof value !== 'object' || value === null) return undefined
45+
return value as SchemaNode
46+
}
47+
3248
function isFileType(value: unknown): boolean {
33-
if (typeof value !== 'object' || value === null) return false
34-
const typed = value as { type?: string }
35-
return typed.type === 'file' || typed.type === 'file[]'
49+
const node = asSchemaNode(value)
50+
return node?.type === 'file' || node?.type === 'file[]'
3651
}
3752

3853
function isArrayType(value: unknown): value is { type: 'array'; items?: unknown } {
39-
if (typeof value !== 'object' || value === null) return false
40-
return (value as { type?: string }).type === 'array'
54+
return asSchemaNode(value)?.type === 'array'
4155
}
4256

4357
function getArrayItems(schema: unknown): unknown {
44-
if (typeof schema !== 'object' || schema === null) return undefined
45-
return (schema as { items?: unknown }).items
58+
return asSchemaNode(schema)?.items
4659
}
4760

4861
function getProperties(schema: unknown): Record<string, unknown> | undefined {
49-
if (typeof schema !== 'object' || schema === null) return undefined
50-
const props = (schema as { properties?: unknown }).properties
62+
const props = asSchemaNode(schema)?.properties
5163
return typeof props === 'object' && props !== null
5264
? (props as Record<string, unknown>)
5365
: undefined
@@ -69,6 +81,19 @@ function lookupField(schema: unknown, fieldName: string): unknown | undefined {
6981
return undefined
7082
}
7183

84+
function isOpaqueSchemaNode(value: unknown): boolean {
85+
const node = asSchemaNode(value)
86+
if (!node) return false
87+
// A schema node whose nested shape isn't enumerated. Any path beneath it
88+
// is accepted because there's no declared structure to validate against.
89+
// `object` / `json` with declared `properties` are walked via lookupField.
90+
if (node.type === 'any') return true
91+
if ((node.type === 'json' || node.type === 'object') && node.properties === undefined) {
92+
return true
93+
}
94+
return false
95+
}
96+
7297
function isPathInSchema(schema: OutputSchema | undefined, pathParts: string[]): boolean {
7398
if (!schema || pathParts.length === 0) {
7499
return true
@@ -83,6 +108,10 @@ function isPathInSchema(schema: OutputSchema | undefined, pathParts: string[]):
83108
return false
84109
}
85110

111+
if (isOpaqueSchemaNode(current)) {
112+
return true
113+
}
114+
86115
if (/^\d+$/.test(part)) {
87116
if (isFileType(current)) {
88117
const nextPart = pathParts[i + 1]
@@ -183,14 +212,12 @@ export function resolveBlockReference(
183212
}
184213

185214
const blockOutput = context.blockData[blockId]
186-
const schema = context.blockOutputSchemas?.[blockId]
187215

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

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

201228
const value = navigatePath(blockOutput, pathParts)
202229

230+
const schema = context.blockOutputSchemas?.[blockId]
203231
if (value === undefined && schema) {
204232
if (!isPathInSchema(schema, pathParts)) {
205233
throw new InvalidFieldError(blockName, pathParts.join('.'), getSchemaFieldNames(schema))
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
import type { ExecutionContext } from '@/executor/types'
6+
import type { VariableResolver } from '@/executor/variables/resolver'
7+
import { resolveArrayInput } from './subflow-utils'
8+
9+
describe('resolveArrayInput', () => {
10+
const fakeCtx = {} as unknown as ExecutionContext
11+
12+
it('returns arrays as-is', () => {
13+
expect(resolveArrayInput(fakeCtx, [1, 2, 3], null)).toEqual([1, 2, 3])
14+
})
15+
16+
it('converts plain objects to entries', () => {
17+
expect(resolveArrayInput(fakeCtx, { a: 1, b: 2 }, null)).toEqual([
18+
['a', 1],
19+
['b', 2],
20+
])
21+
})
22+
23+
it('returns empty array when a pure reference resolves to null (skipped block)', () => {
24+
// `resolveSingleReference` returns `null` for a reference that points at a
25+
// block that exists in the workflow but did not execute on this path.
26+
// A loop/parallel over such a reference should run zero iterations rather
27+
// than fail the workflow.
28+
const resolver = {
29+
resolveSingleReference: vi.fn().mockReturnValue(null),
30+
} as unknown as VariableResolver
31+
32+
const result = resolveArrayInput(fakeCtx, '<SkippedBlock.result.items>', resolver)
33+
34+
expect(result).toEqual([])
35+
expect(resolver.resolveSingleReference).toHaveBeenCalled()
36+
})
37+
38+
it('returns the array from a pure reference that resolved to an array', () => {
39+
const resolver = {
40+
resolveSingleReference: vi.fn().mockReturnValue([1, 2, 3]),
41+
} as unknown as VariableResolver
42+
43+
expect(resolveArrayInput(fakeCtx, '<Block.items>', resolver)).toEqual([1, 2, 3])
44+
})
45+
46+
it('converts resolved objects to entries', () => {
47+
const resolver = {
48+
resolveSingleReference: vi.fn().mockReturnValue({ x: 1, y: 2 }),
49+
} as unknown as VariableResolver
50+
51+
expect(resolveArrayInput(fakeCtx, '<Block.obj>', resolver)).toEqual([
52+
['x', 1],
53+
['y', 2],
54+
])
55+
})
56+
57+
it('throws when a pure reference resolves to a non-array, non-object, non-null value', () => {
58+
const resolver = {
59+
resolveSingleReference: vi.fn().mockReturnValue(42),
60+
} as unknown as VariableResolver
61+
62+
expect(() => resolveArrayInput(fakeCtx, '<Block.count>', resolver)).toThrow(
63+
/did not resolve to an array or object/
64+
)
65+
})
66+
67+
it('throws when a pure reference resolves to undefined (unknown block)', () => {
68+
// `undefined` means the reference could not be matched to any block at
69+
// all (typo / deleted block). This must still fail loudly.
70+
const resolver = {
71+
resolveSingleReference: vi.fn().mockReturnValue(undefined),
72+
} as unknown as VariableResolver
73+
74+
expect(() => resolveArrayInput(fakeCtx, '<Missing.items>', resolver)).toThrow(
75+
/did not resolve to an array or object/
76+
)
77+
})
78+
79+
it('parses a JSON array string', () => {
80+
expect(resolveArrayInput(fakeCtx, '[1, 2, 3]', null)).toEqual([1, 2, 3])
81+
})
82+
83+
it('throws on a string that is neither a reference nor valid JSON array/object', () => {
84+
expect(() => resolveArrayInput(fakeCtx, 'not json', null)).toThrow()
85+
})
86+
})

apps/sim/executor/utils/subflow-utils.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,9 @@ export function resolveArrayInput(
216216
if (typeof resolved === 'object' && resolved !== null) {
217217
return Object.entries(resolved)
218218
}
219+
if (resolved === null) {
220+
return []
221+
}
219222
throw new Error(`Reference "${items}" did not resolve to an array or object`)
220223
} catch (error) {
221224
if (error instanceof Error && error.message.startsWith('Reference "')) {

apps/sim/executor/variables/resolvers/block.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,47 @@ describe('BlockResolver', () => {
383383
expect(resolver.resolve('<start.input>', ctx)).toBe(RESOLVED_EMPTY)
384384
})
385385

386+
it.concurrent(
387+
'should return RESOLVED_EMPTY for nested json path on function block that did not execute',
388+
() => {
389+
// Repro for the branched-trigger CRM workflow bug: a function block
390+
// on an untaken branch is referenced via a nested path under its
391+
// `result` (declared `type: 'json'`). The resolver must not validate
392+
// the path against the declared top-level schema keys in this case.
393+
const workflow = createTestWorkflow([
394+
{ id: 'normalize-email', name: 'NormalizeEmail', type: 'function' },
395+
{ id: 'normalize-calendar', name: 'NormalizeCalendar', type: 'function' },
396+
])
397+
const resolver = new BlockResolver(workflow)
398+
const ctx = createTestContext('current', {
399+
'normalize-email': { result: { summary: 'email summary' }, stdout: '' },
400+
})
401+
402+
expect(resolver.resolve('<NormalizeEmail.result.summary>', ctx)).toBe('email summary')
403+
expect(resolver.resolve('<NormalizeCalendar.result.summary>', ctx)).toBe(RESOLVED_EMPTY)
404+
expect(resolver.resolve('<NormalizeCalendar.result>', ctx)).toBe(RESOLVED_EMPTY)
405+
}
406+
)
407+
408+
it.concurrent(
409+
'should return RESOLVED_EMPTY for nested json path on executed block when data is missing',
410+
() => {
411+
// Even for a block that ran, drilling into a `json`-typed field that
412+
// the runtime output didn't include should resolve to empty rather
413+
// than throw — `json` explicitly means dynamic shape.
414+
const workflow = createTestWorkflow([
415+
{ id: 'normalize-email', name: 'NormalizeEmail', type: 'function' },
416+
])
417+
const resolver = new BlockResolver(workflow)
418+
const ctx = createTestContext('current', {
419+
'normalize-email': { result: { subject: 'hi' }, stdout: '' },
420+
})
421+
422+
expect(resolver.resolve('<NormalizeEmail.result.subject>', ctx)).toBe('hi')
423+
expect(resolver.resolve('<NormalizeEmail.result.summary>', ctx)).toBe(RESOLVED_EMPTY)
424+
}
425+
)
426+
386427
it.concurrent('should fall back to context blockStates', () => {
387428
const workflow = createTestWorkflow([{ id: 'source' }])
388429
const resolver = new BlockResolver(workflow)

0 commit comments

Comments
 (0)