Skip to content

Commit f21eaf1

Browse files
waleedlatif1waleedlatif1
authored
fix(vars): add socket persistence when variable names are changed, update variable name normalization to match block name normalization, added space constraint on envvar names (#2508)
* fix(vars): add socket persistence when variable names are changed, update variable name normalization to match block name normalization, added space constraint on envvar names * removed redundant queueing, removed unused immediate flag from sockets ops * ack PR comments
1 parent 942da88 commit f21eaf1

File tree

20 files changed

+460
-109
lines changed

20 files changed

+460
-109
lines changed

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/code/code.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { useWand } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-
3737
import type { GenerationType } from '@/blocks/types'
3838
import { createEnvVarPattern, createReferencePattern } from '@/executor/utils/reference-validation'
3939
import { useTagSelection } from '@/hooks/use-tag-selection'
40-
import { normalizeBlockName } from '@/stores/workflows/utils'
40+
import { normalizeName } from '@/stores/workflows/utils'
4141

4242
const logger = createLogger('Code')
4343

@@ -602,7 +602,7 @@ export function Code({
602602

603603
const inner = reference.slice(1, -1)
604604
const [prefix] = inner.split('.')
605-
const normalizedPrefix = normalizeBlockName(prefix)
605+
const normalizedPrefix = normalizeName(prefix)
606606

607607
if (SYSTEM_REFERENCE_PREFIXES.has(normalizedPrefix)) {
608608
return true

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/condition-input/condition-input.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c
3333
import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes'
3434
import { createEnvVarPattern, createReferencePattern } from '@/executor/utils/reference-validation'
3535
import { useTagSelection } from '@/hooks/use-tag-selection'
36-
import { normalizeBlockName } from '@/stores/workflows/utils'
36+
import { normalizeName } from '@/stores/workflows/utils'
3737
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
3838

3939
const logger = createLogger('ConditionInput')
@@ -139,7 +139,7 @@ export function ConditionInput({
139139

140140
const inner = reference.slice(1, -1)
141141
const [prefix] = inner.split('.')
142-
const normalizedPrefix = normalizeBlockName(prefix)
142+
const normalizedPrefix = normalizeName(prefix)
143143

144144
if (SYSTEM_REFERENCE_PREFIXES.has(normalizedPrefix)) {
145145
return true

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { ReactNode } from 'react'
44
import { splitReferenceSegment } from '@/lib/workflows/sanitization/references'
55
import { REFERENCE } from '@/executor/constants'
66
import { createCombinedPattern } from '@/executor/utils/reference-validation'
7-
import { normalizeBlockName } from '@/stores/workflows/utils'
7+
import { normalizeName } from '@/stores/workflows/utils'
88

99
export interface HighlightContext {
1010
accessiblePrefixes?: Set<string>
@@ -31,7 +31,7 @@ export function formatDisplayText(text: string, context?: HighlightContext): Rea
3131

3232
const inner = reference.slice(1, -1)
3333
const [prefix] = inner.split('.')
34-
const normalizedPrefix = normalizeBlockName(prefix)
34+
const normalizedPrefix = normalizeName(prefix)
3535

3636
if (SYSTEM_PREFIXES.has(normalizedPrefix)) {
3737
return true

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tag-dropdown/tag-dropdown.tsx‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { useVariablesStore } from '@/stores/panel/variables/store'
3434
import type { Variable } from '@/stores/panel/variables/types'
3535
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
3636
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
37+
import { normalizeName } from '@/stores/workflows/utils'
3738
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
3839
import type { BlockState } from '@/stores/workflows/workflow/types'
3940
import { getTool } from '@/tools/utils'
@@ -117,20 +118,6 @@ const TAG_PREFIXES = {
117118
VARIABLE: 'variable.',
118119
} as const
119120

120-
/**
121-
* Normalizes a block name by removing spaces and converting to lowercase
122-
*/
123-
const normalizeBlockName = (blockName: string): string => {
124-
return blockName.replace(/\s+/g, '').toLowerCase()
125-
}
126-
127-
/**
128-
* Normalizes a variable name by removing spaces
129-
*/
130-
const normalizeVariableName = (variableName: string): string => {
131-
return variableName.replace(/\s+/g, '')
132-
}
133-
134121
/**
135122
* Ensures the root tag is present in the tags array
136123
*/
@@ -521,7 +508,7 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
521508
if (sourceBlock.type === 'loop' || sourceBlock.type === 'parallel') {
522509
const mockConfig = { outputs: { results: 'array' } }
523510
const blockName = sourceBlock.name || sourceBlock.type
524-
const normalizedBlockName = normalizeBlockName(blockName)
511+
const normalizedBlockName = normalizeName(blockName)
525512

526513
const outputPaths = generateOutputPaths(mockConfig.outputs)
527514
const blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
@@ -542,7 +529,7 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
542529
}
543530

544531
const blockName = sourceBlock.name || sourceBlock.type
545-
const normalizedBlockName = normalizeBlockName(blockName)
532+
const normalizedBlockName = normalizeName(blockName)
546533

547534
const mergedSubBlocks = getMergedSubBlocks(activeSourceBlockId)
548535
const responseFormatValue = mergedSubBlocks?.responseFormat?.value
@@ -735,12 +722,12 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
735722
)
736723

737724
const variableTags = validVariables.map(
738-
(variable: Variable) => `${TAG_PREFIXES.VARIABLE}${normalizeVariableName(variable.name)}`
725+
(variable: Variable) => `${TAG_PREFIXES.VARIABLE}${normalizeName(variable.name)}`
739726
)
740727

741728
const variableInfoMap = validVariables.reduce(
742729
(acc, variable) => {
743-
const tagName = `${TAG_PREFIXES.VARIABLE}${normalizeVariableName(variable.name)}`
730+
const tagName = `${TAG_PREFIXES.VARIABLE}${normalizeName(variable.name)}`
744731
acc[tagName] = {
745732
type: variable.type,
746733
id: variable.id,
@@ -865,7 +852,7 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
865852

866853
const mockConfig = { outputs: { results: 'array' } }
867854
const blockName = accessibleBlock.name || accessibleBlock.type
868-
const normalizedBlockName = normalizeBlockName(blockName)
855+
const normalizedBlockName = normalizeName(blockName)
869856

870857
const outputPaths = generateOutputPaths(mockConfig.outputs)
871858
let blockTags = outputPaths.map((path) => `${normalizedBlockName}.${path}`)
@@ -885,7 +872,7 @@ export const TagDropdown: React.FC<TagDropdownProps> = ({
885872
}
886873

887874
const blockName = accessibleBlock.name || accessibleBlock.type
888-
const normalizedBlockName = normalizeBlockName(blockName)
875+
const normalizedBlockName = normalizeName(blockName)
889876

890877
const mergedSubBlocks = getMergedSubBlocks(accessibleBlockId)
891878
const responseFormatValue = mergedSubBlocks?.responseFormat?.value

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/hooks/use-subflow-editor.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { checkTagTrigger } from '@/app/workspace/[workspaceId]/w/[workflowId]/co
99
import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes'
1010
import { createEnvVarPattern, createReferencePattern } from '@/executor/utils/reference-validation'
1111
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
12-
import { normalizeBlockName } from '@/stores/workflows/utils'
12+
import { normalizeName } from '@/stores/workflows/utils'
1313
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
1414
import type { BlockState } from '@/stores/workflows/workflow/types'
1515

@@ -110,7 +110,7 @@ export function useSubflowEditor(currentBlock: BlockState | null, currentBlockId
110110

111111
const inner = reference.slice(1, -1)
112112
const [prefix] = inner.split('.')
113-
const normalizedPrefix = normalizeBlockName(prefix)
113+
const normalizedPrefix = normalizeName(prefix)
114114

115115
if (SYSTEM_REFERENCE_PREFIXES.has(normalizedPrefix)) {
116116
return true

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useMemo } from 'react'
22
import { useShallow } from 'zustand/react/shallow'
33
import { BlockPathCalculator } from '@/lib/workflows/blocks/block-path-calculator'
44
import { SYSTEM_REFERENCE_PREFIXES } from '@/lib/workflows/sanitization/references'
5-
import { normalizeBlockName } from '@/stores/workflows/utils'
5+
import { normalizeName } from '@/stores/workflows/utils'
66
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
77
import type { Loop, Parallel } from '@/stores/workflows/workflow/types'
88

@@ -53,10 +53,10 @@ export function useAccessibleReferencePrefixes(blockId?: string | null): Set<str
5353

5454
const prefixes = new Set<string>()
5555
accessibleIds.forEach((id) => {
56-
prefixes.add(normalizeBlockName(id))
56+
prefixes.add(normalizeName(id))
5757
const block = blocks[id]
5858
if (block?.name) {
59-
prefixes.add(normalizeBlockName(block.name))
59+
prefixes.add(normalizeName(block.name))
6060
}
6161
})
6262

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/environment/environment.tsx‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,17 @@ interface UIEnvironmentVariable {
5252
id?: number
5353
}
5454

55+
/**
56+
* Validates an environment variable key.
57+
* Returns an error message if invalid, undefined if valid.
58+
*/
59+
function validateEnvVarKey(key: string): string | undefined {
60+
if (!key) return undefined
61+
if (key.includes(' ')) return 'Spaces are not allowed'
62+
if (!ENV_VAR_PATTERN.test(key)) return 'Only letters, numbers, and underscores allowed'
63+
return undefined
64+
}
65+
5566
interface EnvironmentVariablesProps {
5667
registerBeforeLeaveHandler?: (handler: (onProceed: () => void) => void) => void
5768
}
@@ -222,6 +233,10 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
222233
return envVars.some((envVar) => !!envVar.key && Object.hasOwn(workspaceVars, envVar.key))
223234
}, [envVars, workspaceVars])
224235

236+
const hasInvalidKeys = useMemo(() => {
237+
return envVars.some((envVar) => !!envVar.key && validateEnvVarKey(envVar.key))
238+
}, [envVars])
239+
225240
useEffect(() => {
226241
hasChangesRef.current = hasChanges
227242
}, [hasChanges])
@@ -551,6 +566,7 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
551566
const renderEnvVarRow = useCallback(
552567
(envVar: UIEnvironmentVariable, originalIndex: number) => {
553568
const isConflict = !!envVar.key && Object.hasOwn(workspaceVars, envVar.key)
569+
const keyError = validateEnvVarKey(envVar.key)
554570
const maskedValueStyle =
555571
focusedValueIndex !== originalIndex && !isConflict
556572
? ({ WebkitTextSecurity: 'disc' } as React.CSSProperties)
@@ -571,7 +587,7 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
571587
spellCheck='false'
572588
readOnly
573589
onFocus={(e) => e.target.removeAttribute('readOnly')}
574-
className={`h-9 ${isConflict ? conflictClassName : ''}`}
590+
className={`h-9 ${isConflict ? conflictClassName : ''} ${keyError ? 'border-[var(--text-error)]' : ''}`}
575591
/>
576592
<div />
577593
<EmcnInput
@@ -627,7 +643,12 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
627643
</Tooltip.Root>
628644
</div>
629645
</div>
630-
{isConflict && (
646+
{keyError && (
647+
<div className='col-span-3 mt-[4px] text-[12px] text-[var(--text-error)] leading-tight'>
648+
{keyError}
649+
</div>
650+
)}
651+
{isConflict && !keyError && (
631652
<div className='col-span-3 mt-[4px] text-[12px] text-[var(--text-error)] leading-tight'>
632653
Workspace variable with the same name overrides this. Rename your personal key to use
633654
it.
@@ -707,14 +728,17 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
707728
<Tooltip.Trigger asChild>
708729
<Button
709730
onClick={handleSave}
710-
disabled={isLoading || !hasChanges || hasConflicts}
731+
disabled={isLoading || !hasChanges || hasConflicts || hasInvalidKeys}
711732
variant='primary'
712-
className={`${PRIMARY_BUTTON_STYLES} ${hasConflicts ? 'cursor-not-allowed opacity-50' : ''}`}
733+
className={`${PRIMARY_BUTTON_STYLES} ${hasConflicts || hasInvalidKeys ? 'cursor-not-allowed opacity-50' : ''}`}
713734
>
714735
Save
715736
</Button>
716737
</Tooltip.Trigger>
717738
{hasConflicts && <Tooltip.Content>Resolve all conflicts before saving</Tooltip.Content>}
739+
{hasInvalidKeys && !hasConflicts && (
740+
<Tooltip.Content>Fix invalid variable names before saving</Tooltip.Content>
741+
)}
718742
</Tooltip.Root>
719743
</div>
720744

@@ -808,27 +832,31 @@ export function EnvironmentVariables({ registerBeforeLeaveHandler }: Environment
808832
<ModalHeader>Unsaved Changes</ModalHeader>
809833
<ModalBody>
810834
<p className='text-[12px] text-[var(--text-tertiary)]'>
811-
{hasConflicts
812-
? 'You have unsaved changes, but conflicts must be resolved before saving. You can discard your changes to close the modal.'
835+
{hasConflicts || hasInvalidKeys
836+
? `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.`
813837
: 'You have unsaved changes. Do you want to save them before closing?'}
814838
</p>
815839
</ModalBody>
816840
<ModalFooter>
817841
<Button variant='default' onClick={handleCancel}>
818842
Discard Changes
819843
</Button>
820-
{hasConflicts ? (
844+
{hasConflicts || hasInvalidKeys ? (
821845
<Tooltip.Root>
822846
<Tooltip.Trigger asChild>
823847
<Button
824848
disabled={true}
825849
variant='primary'
826-
className='cursor-not-allowed opacity-50'
850+
className={`${PRIMARY_BUTTON_STYLES} cursor-not-allowed opacity-50`}
827851
>
828852
Save Changes
829853
</Button>
830854
</Tooltip.Trigger>
831-
<Tooltip.Content>Resolve all conflicts before saving</Tooltip.Content>
855+
<Tooltip.Content>
856+
{hasConflicts
857+
? 'Resolve all conflicts before saving'
858+
: 'Fix invalid variable names before saving'}
859+
</Tooltip.Content>
832860
</Tooltip.Root>
833861
) : (
834862
<Button onClick={handleSave} variant='primary' className={PRIMARY_BUTTON_STYLES}>

‎apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import type { BlockHandler, ExecutionContext, PauseMetadata } from '@/executor/types'
1717
import { collectBlockData } from '@/executor/utils/block-data'
1818
import type { SerializedBlock } from '@/serializer/types'
19-
import { normalizeBlockName } from '@/stores/workflows/utils'
19+
import { normalizeName } from '@/stores/workflows/utils'
2020
import { executeTool } from '@/tools'
2121

2222
const logger = createLogger('HumanInTheLoopBlockHandler')
@@ -591,7 +591,7 @@ export class HumanInTheLoopBlockHandler implements BlockHandler {
591591

592592
if (pauseBlockName) {
593593
blockNameMappingWithPause[pauseBlockName] = pauseBlockId
594-
blockNameMappingWithPause[normalizeBlockName(pauseBlockName)] = pauseBlockId
594+
blockNameMappingWithPause[normalizeName(pauseBlockName)] = pauseBlockId
595595
}
596596

597597
const notificationPromises = tools.map<Promise<NotificationToolResult>>(async (toolConfig) => {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
type Resolver,
66
} from '@/executor/variables/resolvers/reference'
77
import type { SerializedWorkflow } from '@/serializer/types'
8-
import { normalizeBlockName } from '@/stores/workflows/utils'
8+
import { normalizeName } from '@/stores/workflows/utils'
99

1010
export class BlockResolver implements Resolver {
1111
private blockByNormalizedName: Map<string, string>
@@ -15,7 +15,7 @@ export class BlockResolver implements Resolver {
1515
for (const block of workflow.blocks) {
1616
this.blockByNormalizedName.set(block.id, block.id)
1717
if (block.metadata?.name) {
18-
const normalized = normalizeBlockName(block.metadata.name)
18+
const normalized = normalizeName(block.metadata.name)
1919
this.blockByNormalizedName.set(normalized, block.id)
2020
}
2121
}
@@ -83,7 +83,7 @@ export class BlockResolver implements Resolver {
8383
if (this.blockByNormalizedName.has(name)) {
8484
return this.blockByNormalizedName.get(name)
8585
}
86-
const normalized = normalizeBlockName(name)
86+
const normalized = normalizeName(name)
8787
return this.blockByNormalizedName.get(normalized)
8888
}
8989

0 commit comments

Comments
 (0)