Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { useVariablesStore } from '@/stores/panel/variables/store'
import type { Variable } from '@/stores/panel/variables/types'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import { normalizeBlockName, normalizeVariableName } from '@/stores/workflows/utils'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
import type { BlockState } from '@/stores/workflows/workflow/types'
import { getTool } from '@/tools/utils'
Expand Down Expand Up @@ -117,20 +118,6 @@ const TAG_PREFIXES = {
VARIABLE: 'variable.',
} as const

/**
* Normalizes a block name by removing spaces and converting to lowercase
*/
const normalizeBlockName = (blockName: string): string => {
return blockName.replace(/\s+/g, '').toLowerCase()
}

/**
* Normalizes a variable name by removing spaces
*/
const normalizeVariableName = (variableName: string): string => {
return variableName.replace(/\s+/g, '')
}

/**
* Ensures the root tag is present in the tags array
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ interface UIEnvironmentVariable {
id?: number
}

/**
* Validates an environment variable key.
* Returns an error message if invalid, undefined if valid.
*/
function validateEnvVarKey(key: string): string | undefined {
if (!key) return undefined
if (key.includes(' ')) return 'Spaces are not allowed'
if (!ENV_VAR_PATTERN.test(key)) return 'Only letters, numbers, and underscores allowed'
return undefined
}

interface EnvironmentVariablesProps {
registerBeforeLeaveHandler?: (handler: (onProceed: () => void) => void) => void
}
Expand Down Expand Up @@ -222,6 +233,10 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
return envVars.some((envVar) => !!envVar.key && Object.hasOwn(workspaceVars, envVar.key))
}, [envVars, workspaceVars])

const hasInvalidKeys = useMemo(() => {
return envVars.some((envVar) => !!envVar.key && validateEnvVarKey(envVar.key))
}, [envVars])

useEffect(() => {
hasChangesRef.current = hasChanges
}, [hasChanges])
Expand Down Expand Up @@ -551,6 +566,7 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
const renderEnvVarRow = useCallback(
(envVar: UIEnvironmentVariable, originalIndex: number) => {
const isConflict = !!envVar.key && Object.hasOwn(workspaceVars, envVar.key)
const keyError = validateEnvVarKey(envVar.key)
const maskedValueStyle =
focusedValueIndex !== originalIndex && !isConflict
? ({ WebkitTextSecurity: 'disc' } as React.CSSProperties)
Expand All @@ -571,7 +587,7 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
spellCheck='false'
readOnly
onFocus={(e) => e.target.removeAttribute('readOnly')}
className={`h-9 ${isConflict ? conflictClassName : ''}`}
className={`h-9 ${isConflict ? conflictClassName : ''} ${keyError ? 'border-[var(--text-error)]' : ''}`}
/>
<div />
<EmcnInput
Expand Down Expand Up @@ -627,7 +643,12 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
</Tooltip.Root>
</div>
</div>
{isConflict && (
{keyError && (
<div className='col-span-3 mt-[4px] text-[12px] text-[var(--text-error)] leading-tight'>
{keyError}
</div>
)}
{isConflict && !keyError && (
<div className='col-span-3 mt-[4px] text-[12px] text-[var(--text-error)] leading-tight'>
Workspace variable with the same name overrides this. Rename your personal key to use
it.
Expand Down Expand Up @@ -707,14 +728,17 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
<Tooltip.Trigger asChild>
<Button
onClick={handleSave}
disabled={isLoading || !hasChanges || hasConflicts}
disabled={isLoading || !hasChanges || hasConflicts || hasInvalidKeys}
variant='primary'
className={`${PRIMARY_BUTTON_STYLES} ${hasConflicts ? 'cursor-not-allowed opacity-50' : ''}`}
className={`${PRIMARY_BUTTON_STYLES} ${hasConflicts || hasInvalidKeys ? 'cursor-not-allowed opacity-50' : ''}`}
>
Save
</Button>
</Tooltip.Trigger>
{hasConflicts && <Tooltip.Content>Resolve all conflicts before saving</Tooltip.Content>}
{hasInvalidKeys && !hasConflicts && (
<Tooltip.Content>Fix invalid variable names before saving</Tooltip.Content>
)}
</Tooltip.Root>
</div>

Expand Down Expand Up @@ -808,27 +832,31 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
<ModalHeader>Unsaved Changes</ModalHeader>
<ModalBody>
<p className='text-[12px] text-[var(--text-tertiary)]'>
{hasConflicts
? 'You have unsaved changes, but conflicts must be resolved before saving. You can discard your changes to close the modal.'
{hasConflicts || hasInvalidKeys
? `You have unsaved changes, but ${hasConflicts ? 'conflicts must be resolved' : 'invalid variable names must be fixed'} before saving. You can discard your changes to close the modal.`
: 'You have unsaved changes. Do you want to save them before closing?'}
</p>
</ModalBody>
<ModalFooter>
<Button variant='default' onClick={handleCancel}>
Discard Changes
</Button>
{hasConflicts ? (
{hasConflicts || hasInvalidKeys ? (
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Button
disabled={true}
variant='primary'
className='cursor-not-allowed opacity-50'
className={`${PRIMARY_BUTTON_STYLES} cursor-not-allowed opacity-50`}
>
Save Changes
</Button>
</Tooltip.Trigger>
<Tooltip.Content>Resolve all conflicts before saving</Tooltip.Content>
<Tooltip.Content>
{hasConflicts
? 'Resolve all conflicts before saving'
: 'Fix invalid variable names before saving'}
</Tooltip.Content>
</Tooltip.Root>
) : (
<Button onClick={handleSave} variant='primary' className={PRIMARY_BUTTON_STYLES}>
Expand Down
219 changes: 219 additions & 0 deletions apps/sim/executor/variables/resolvers/workflow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
import { describe, expect, it, vi } from 'vitest'
import type { ResolutionContext } from './reference'
import { WorkflowResolver } from './workflow'

vi.mock('@/lib/logs/console/logger', () => ({
createLogger: vi.fn().mockReturnValue({
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
}),
}))

vi.mock('@/lib/workflows/variables/variable-manager', () => ({
VariableManager: {
resolveForExecution: vi.fn((value) => value),
},
}))

/**
* Creates a minimal ResolutionContext for testing.
* The WorkflowResolver only uses context.executionContext.workflowVariables,
* so we only need to provide that field.
*/
function createTestContext(workflowVariables: Record<string, any>): ResolutionContext {
return {
executionContext: { workflowVariables },
executionState: {},
currentNodeId: 'test-node',
} as ResolutionContext
}

describe('WorkflowResolver', () => {
describe('canResolve', () => {
it.concurrent('should return true for variable references', () => {
const resolver = new WorkflowResolver({})
expect(resolver.canResolve('<variable.myvar>')).toBe(true)
expect(resolver.canResolve('<variable.test>')).toBe(true)
})

it.concurrent('should return false for non-variable references', () => {
const resolver = new WorkflowResolver({})
expect(resolver.canResolve('<block.output>')).toBe(false)
expect(resolver.canResolve('<loop.index>')).toBe(false)
expect(resolver.canResolve('plain text')).toBe(false)
})
})

describe('resolve with normalized matching', () => {
it.concurrent('should resolve variable with exact name match', () => {
const variables = {
'var-1': { id: 'var-1', name: 'myvar', type: 'plain', value: 'test-value' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.myvar>', createTestContext(variables))
expect(result).toBe('test-value')
})

it.concurrent('should resolve variable with normalized name (lowercase)', () => {
const variables = {
'var-1': { id: 'var-1', name: 'MyVar', type: 'plain', value: 'test-value' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.myvar>', createTestContext(variables))
expect(result).toBe('test-value')
})

it.concurrent('should resolve variable with normalized name (spaces removed)', () => {
const variables = {
'var-1': { id: 'var-1', name: 'My Variable', type: 'plain', value: 'test-value' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.myvariable>', createTestContext(variables))
expect(result).toBe('test-value')
})

it.concurrent(
'should resolve variable with fully normalized name (JIRA TEAM UUID case)',
() => {
const variables = {
'var-1': { id: 'var-1', name: 'JIRA TEAM UUID', type: 'plain', value: 'uuid-123' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.jirateamuuid>', createTestContext(variables))
expect(result).toBe('uuid-123')
}
)

it.concurrent('should resolve variable regardless of reference case', () => {
const variables = {
'var-1': { id: 'var-1', name: 'jirateamuuid', type: 'plain', value: 'uuid-123' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.JIRATEAMUUID>', createTestContext(variables))
expect(result).toBe('uuid-123')
})

it.concurrent('should resolve by variable ID (exact match)', () => {
const variables = {
'my-uuid-id': { id: 'my-uuid-id', name: 'Some Name', type: 'plain', value: 'id-value' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.my-uuid-id>', createTestContext(variables))
expect(result).toBe('id-value')
})

it.concurrent('should return undefined for non-existent variable', () => {
const variables = {
'var-1': { id: 'var-1', name: 'existing', type: 'plain', value: 'test' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.nonexistent>', createTestContext(variables))
expect(result).toBeUndefined()
})

it.concurrent('should handle nested path access', () => {
const variables = {
'var-1': {
id: 'var-1',
name: 'config',
type: 'object',
value: { nested: { value: 'deep' } },
},
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve(
'<variable.config.nested.value>',
createTestContext(variables)
)
expect(result).toBe('deep')
})

it.concurrent('should resolve with mixed case and spaces in reference', () => {
const variables = {
'var-1': { id: 'var-1', name: 'api key', type: 'plain', value: 'secret-key' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.APIKEY>', createTestContext(variables))
expect(result).toBe('secret-key')
})

it.concurrent('should handle real-world variable naming patterns', () => {
const testCases = [
{ varName: 'User ID', refName: 'userid', value: 'user-123' },
{ varName: 'API Key', refName: 'apikey', value: 'key-456' },
{ varName: 'STRIPE SECRET KEY', refName: 'stripesecretkey', value: 'sk_test' },
{ varName: 'Database URL', refName: 'databaseurl', value: 'postgres://...' },
]

for (const { varName, refName, value } of testCases) {
const variables = {
'var-1': { id: 'var-1', name: varName, type: 'plain', value },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve(`<variable.${refName}>`, createTestContext(variables))
expect(result).toBe(value)
}
})
})

describe('edge cases', () => {
it.concurrent('should handle empty workflow variables', () => {
const resolver = new WorkflowResolver({})

const result = resolver.resolve('<variable.anyvar>', createTestContext({}))
expect(result).toBeUndefined()
})

it.concurrent('should handle invalid reference format', () => {
const resolver = new WorkflowResolver({})

const result = resolver.resolve('<variable>', createTestContext({}))
expect(result).toBeUndefined()
})

it.concurrent('should handle null variable values in the map', () => {
const variables: Record<string, any> = {
'var-1': null,
'var-2': { id: 'var-2', name: 'valid', type: 'plain', value: 'exists' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.valid>', createTestContext(variables))
expect(result).toBe('exists')
})

it.concurrent('should handle variable with empty name', () => {
const variables = {
'var-1': { id: 'var-1', name: '', type: 'plain', value: 'empty-name' },
}
const resolver = new WorkflowResolver(variables)

// Empty name normalizes to empty string, which matches "<variable.>" reference
const result = resolver.resolve('<variable.>', createTestContext(variables))
expect(result).toBe('empty-name')
})

it.concurrent('should prefer name match over ID match when both could apply', () => {
const variables = {
apikey: { id: 'apikey', name: 'different', type: 'plain', value: 'by-id' },
'var-2': { id: 'var-2', name: 'apikey', type: 'plain', value: 'by-name' },
}
const resolver = new WorkflowResolver(variables)

const result = resolver.resolve('<variable.apikey>', createTestContext(variables))
expect(['by-id', 'by-name']).toContain(result)
})
})
})
8 changes: 7 additions & 1 deletion apps/sim/executor/variables/resolvers/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
type ResolutionContext,
type Resolver,
} from '@/executor/variables/resolvers/reference'
import { normalizeVariableName } from '@/stores/workflows/utils'

const logger = createLogger('WorkflowResolver')

Expand All @@ -32,12 +33,17 @@ export class WorkflowResolver implements Resolver {
}

const [_, variableName, ...pathParts] = parts
const normalizedRefName = normalizeVariableName(variableName)

const workflowVars = context.executionContext.workflowVariables || this.workflowVariables

for (const varObj of Object.values(workflowVars)) {
const v = varObj as any
if (v && (v.name === variableName || v.id === variableName)) {
if (!v) continue

// Match by normalized name or exact ID
const normalizedVarName = v.name ? normalizeVariableName(v.name) : ''
if (normalizedVarName === normalizedRefName || v.id === variableName) {
const normalizedType = (v.type === 'string' ? 'plain' : v.type) || 'plain'
let value: any
try {
Expand Down
Loading
Loading