mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
improvement(resolver): resovled empty sentinel to not pass through unexecuted valid refs to text inputs (#3266)
This commit is contained in:
@@ -7,7 +7,11 @@ import { BlockResolver } from '@/executor/variables/resolvers/block'
|
||||
import { EnvResolver } from '@/executor/variables/resolvers/env'
|
||||
import { LoopResolver } from '@/executor/variables/resolvers/loop'
|
||||
import { ParallelResolver } from '@/executor/variables/resolvers/parallel'
|
||||
import type { ResolutionContext, Resolver } from '@/executor/variables/resolvers/reference'
|
||||
import {
|
||||
RESOLVED_EMPTY,
|
||||
type ResolutionContext,
|
||||
type Resolver,
|
||||
} from '@/executor/variables/resolvers/reference'
|
||||
import { WorkflowResolver } from '@/executor/variables/resolvers/workflow'
|
||||
import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types'
|
||||
|
||||
@@ -104,7 +108,11 @@ export class VariableResolver {
|
||||
loopScope,
|
||||
}
|
||||
|
||||
return this.resolveReference(trimmed, resolutionContext)
|
||||
const result = this.resolveReference(trimmed, resolutionContext)
|
||||
if (result === RESOLVED_EMPTY) {
|
||||
return null
|
||||
}
|
||||
return result
|
||||
}
|
||||
}
|
||||
|
||||
@@ -174,6 +182,13 @@ export class VariableResolver {
|
||||
return match
|
||||
}
|
||||
|
||||
if (resolved === RESOLVED_EMPTY) {
|
||||
if (blockType === BlockType.FUNCTION) {
|
||||
return this.blockResolver.formatValueForBlock(null, blockType, language)
|
||||
}
|
||||
return ''
|
||||
}
|
||||
|
||||
return this.blockResolver.formatValueForBlock(resolved, blockType, language)
|
||||
} catch (error) {
|
||||
replacementError = error instanceof Error ? error : new Error(String(error))
|
||||
@@ -207,7 +222,6 @@ export class VariableResolver {
|
||||
|
||||
let replacementError: Error | null = null
|
||||
|
||||
// Use generic utility for smart variable reference replacement
|
||||
let result = replaceValidReferences(template, (match) => {
|
||||
if (replacementError) return match
|
||||
|
||||
@@ -217,6 +231,10 @@ export class VariableResolver {
|
||||
return match
|
||||
}
|
||||
|
||||
if (resolved === RESOLVED_EMPTY) {
|
||||
return 'null'
|
||||
}
|
||||
|
||||
if (typeof resolved === 'string') {
|
||||
const escaped = resolved.replace(/\\/g, '\\\\').replace(/'/g, "\\'")
|
||||
return `'${escaped}'`
|
||||
|
||||
@@ -2,7 +2,7 @@ import { loggerMock } from '@sim/testing'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { ExecutionState } from '@/executor/execution/state'
|
||||
import { BlockResolver } from './block'
|
||||
import type { ResolutionContext } from './reference'
|
||||
import { RESOLVED_EMPTY, type ResolutionContext } from './reference'
|
||||
|
||||
vi.mock('@sim/logger', () => loggerMock)
|
||||
vi.mock('@/blocks/registry', async () => {
|
||||
@@ -134,15 +134,18 @@ describe('BlockResolver', () => {
|
||||
expect(resolver.resolve('<source.items.1.id>', ctx)).toBe(2)
|
||||
})
|
||||
|
||||
it.concurrent('should return undefined for non-existent path when no schema defined', () => {
|
||||
const workflow = createTestWorkflow([{ id: 'source', type: 'unknown_block_type' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {
|
||||
source: { existing: 'value' },
|
||||
})
|
||||
it.concurrent(
|
||||
'should return RESOLVED_EMPTY for non-existent path when no schema defined',
|
||||
() => {
|
||||
const workflow = createTestWorkflow([{ id: 'source', type: 'unknown_block_type' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {
|
||||
source: { existing: 'value' },
|
||||
})
|
||||
|
||||
expect(resolver.resolve('<source.nonexistent>', ctx)).toBeUndefined()
|
||||
})
|
||||
expect(resolver.resolve('<source.nonexistent>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
}
|
||||
)
|
||||
|
||||
it.concurrent('should throw error for path not in output schema', () => {
|
||||
const workflow = createTestWorkflow([
|
||||
@@ -162,7 +165,7 @@ describe('BlockResolver', () => {
|
||||
expect(() => resolver.resolve('<source.invalidField>', ctx)).toThrow(/Available fields:/)
|
||||
})
|
||||
|
||||
it.concurrent('should return undefined for path in schema but missing in data', () => {
|
||||
it.concurrent('should return RESOLVED_EMPTY for path in schema but missing in data', () => {
|
||||
const workflow = createTestWorkflow([
|
||||
{
|
||||
id: 'source',
|
||||
@@ -175,7 +178,7 @@ describe('BlockResolver', () => {
|
||||
})
|
||||
|
||||
expect(resolver.resolve('<source.stdout>', ctx)).toBe('log output')
|
||||
expect(resolver.resolve('<source.result>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<source.result>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
})
|
||||
|
||||
it.concurrent(
|
||||
@@ -191,7 +194,7 @@ describe('BlockResolver', () => {
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {})
|
||||
|
||||
expect(resolver.resolve('<workflow.childTraceSpans>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<workflow.childTraceSpans>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
}
|
||||
)
|
||||
|
||||
@@ -208,7 +211,7 @@ describe('BlockResolver', () => {
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {})
|
||||
|
||||
expect(resolver.resolve('<workflowinput.childTraceSpans>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<workflowinput.childTraceSpans>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
}
|
||||
)
|
||||
|
||||
@@ -225,13 +228,13 @@ describe('BlockResolver', () => {
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {})
|
||||
|
||||
expect(resolver.resolve('<hitl.response>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<hitl.submission>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<hitl.resumeInput>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<hitl.response>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
expect(resolver.resolve('<hitl.submission>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
expect(resolver.resolve('<hitl.resumeInput>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
}
|
||||
)
|
||||
|
||||
it.concurrent('should return undefined for non-existent block', () => {
|
||||
it.concurrent('should return undefined for block not in workflow', () => {
|
||||
const workflow = createTestWorkflow([{ id: 'existing' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {})
|
||||
@@ -239,6 +242,21 @@ describe('BlockResolver', () => {
|
||||
expect(resolver.resolve('<nonexistent>', ctx)).toBeUndefined()
|
||||
})
|
||||
|
||||
it.concurrent('should return RESOLVED_EMPTY for block in workflow that did not execute', () => {
|
||||
const workflow = createTestWorkflow([
|
||||
{ id: 'start-block', name: 'Start', type: 'start_trigger' },
|
||||
{ id: 'slack-block', name: 'Slack', type: 'slack_trigger' },
|
||||
])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {
|
||||
'slack-block': { message: 'hello from slack' },
|
||||
})
|
||||
|
||||
expect(resolver.resolve('<slack.message>', ctx)).toBe('hello from slack')
|
||||
expect(resolver.resolve('<start>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
expect(resolver.resolve('<start.input>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
})
|
||||
|
||||
it.concurrent('should fall back to context blockStates', () => {
|
||||
const workflow = createTestWorkflow([{ id: 'source' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
@@ -1012,24 +1030,24 @@ describe('BlockResolver', () => {
|
||||
expect(resolver.resolve('<source.other>', ctx)).toBe('exists')
|
||||
})
|
||||
|
||||
it.concurrent('should handle output with undefined values', () => {
|
||||
it.concurrent('should return RESOLVED_EMPTY for output with undefined values', () => {
|
||||
const workflow = createTestWorkflow([{ id: 'source', type: 'unknown_block_type' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {
|
||||
source: { value: undefined, other: 'exists' },
|
||||
})
|
||||
|
||||
expect(resolver.resolve('<source.value>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<source.value>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
})
|
||||
|
||||
it.concurrent('should return undefined for deeply nested non-existent path', () => {
|
||||
it.concurrent('should return RESOLVED_EMPTY for deeply nested non-existent path', () => {
|
||||
const workflow = createTestWorkflow([{ id: 'source', type: 'unknown_block_type' }])
|
||||
const resolver = new BlockResolver(workflow)
|
||||
const ctx = createTestContext('current', {
|
||||
source: { level1: { level2: {} } },
|
||||
})
|
||||
|
||||
expect(resolver.resolve('<source.level1.level2.level3>', ctx)).toBeUndefined()
|
||||
expect(resolver.resolve('<source.level1.level2.level3>', ctx)).toBe(RESOLVED_EMPTY)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
import { formatLiteralForCode } from '@/executor/utils/code-formatting'
|
||||
import {
|
||||
navigatePath,
|
||||
RESOLVED_EMPTY,
|
||||
type ResolutionContext,
|
||||
type Resolver,
|
||||
} from '@/executor/variables/resolvers/reference'
|
||||
@@ -84,7 +85,12 @@ export class BlockResolver implements Resolver {
|
||||
return result.value
|
||||
}
|
||||
|
||||
return this.handleBackwardsCompat(block, output, pathParts)
|
||||
const backwardsCompat = this.handleBackwardsCompat(block, output, pathParts)
|
||||
if (backwardsCompat !== undefined) {
|
||||
return backwardsCompat
|
||||
}
|
||||
|
||||
return RESOLVED_EMPTY
|
||||
} catch (error) {
|
||||
if (error instanceof InvalidFieldError) {
|
||||
const fallback = this.handleBackwardsCompat(block, output, pathParts)
|
||||
|
||||
@@ -12,6 +12,14 @@ export interface Resolver {
|
||||
resolve(reference: string, context: ResolutionContext): any
|
||||
}
|
||||
|
||||
/**
|
||||
* Sentinel value indicating a reference was resolved to a known block
|
||||
* that produced no output (e.g., the block exists in the workflow but
|
||||
* didn't execute on this path). Distinct from `undefined`, which means
|
||||
* the reference couldn't be matched to any block at all.
|
||||
*/
|
||||
export const RESOLVED_EMPTY = Symbol('RESOLVED_EMPTY')
|
||||
|
||||
/**
|
||||
* Navigate through nested object properties using a path array.
|
||||
* Supports dot notation and array indices.
|
||||
|
||||
Reference in New Issue
Block a user