Skip to content

Commit 90dc147

Browse files
committed
fix duplicate var remap bug
1 parent 7e40a59 commit 90dc147

2 files changed

Lines changed: 123 additions & 1 deletion

File tree

  • apps/sim
    • app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/variables-input
    • lib/workflows/persistence

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c
2222
import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes'
2323
import { useVariablesStore } from '@/stores/variables/store'
2424
import type { Variable } from '@/stores/variables/types'
25+
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
2526

2627
interface VariableAssignment {
2728
id: string
@@ -89,6 +90,17 @@ export function VariablesInput({
8990
const workflowVariables = useVariablesStore((s) => s.variables)
9091
const accessiblePrefixes = useAccessibleReferencePrefixes(blockId)
9192

93+
/**
94+
* Gate destructive cleanup on the active workflow being fully hydrated. Without this gate,
95+
* a brief render frame where the variables store has not yet hydrated for the current
96+
* workflow would treat all assignments as orphaned and persist an empty array — destroying
97+
* legitimate references. Only trust `currentWorkflowVariables` once hydration says it owns
98+
* this workflow's data.
99+
*/
100+
const isHydratedForWorkflow = useWorkflowRegistry(
101+
(s) => s.hydration.phase === 'ready' && s.hydration.workflowId === workflowId
102+
)
103+
92104
const [showTags, setShowTags] = useState(false)
93105
const [cursorPosition, setCursorPosition] = useState(0)
94106
const [activeFieldId, setActiveFieldId] = useState<string | null>(null)
@@ -133,6 +145,11 @@ export function VariablesInput({
133145

134146
useEffect(() => {
135147
if (isReadOnly || assignments.length === 0) return
148+
// Only purge stale `variableId` references after the variables store is authoritative
149+
// for this workflow. Otherwise a transient empty store on mount would wipe valid
150+
// assignments — see the duplicate-of-duplicate regression where this destroyed
151+
// remapped variable references on the second copy.
152+
if (!isHydratedForWorkflow) return
136153

137154
const currentVariableIds = new Set(currentWorkflowVariables.map((v) => v.id))
138155
const validAssignments = assignments.filter((assignment) => {
@@ -145,7 +162,7 @@ export function VariablesInput({
145162
} else if (validAssignments.length !== assignments.length) {
146163
setStoreValue(validAssignments.length > 0 ? validAssignments : [])
147164
}
148-
}, [currentWorkflowVariables, assignments, isReadOnly, setStoreValue])
165+
}, [currentWorkflowVariables, assignments, isReadOnly, isHydratedForWorkflow, setStoreValue])
149166

150167
const addAssignment = () => {
151168
if (isReadOnly || allVariablesAssigned) return

apps/sim/lib/workflows/persistence/duplicate.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,109 @@ describe('duplicateWorkflow ordering', () => {
277277
expect(copiedSubBlocks.variables.value[0].variableName).toBe('customerName')
278278
expect(insertedBlocks?.[0].locked).toBe(false)
279279
})
280+
281+
it('remaps variable assignments when duplicating an already-duplicated source (array value)', async () => {
282+
let insertedBlocks: Array<Record<string, unknown>> | null = null
283+
let insertedWorkflowValues: Record<string, unknown> | null = null
284+
const tx = createMockTx(
285+
[
286+
[
287+
{
288+
id: 'first-copy-workflow-id',
289+
workspaceId: 'workspace-123',
290+
folderId: null,
291+
description: 'first copy',
292+
color: '#000000',
293+
variables: {
294+
'first-copy-var-id': {
295+
id: 'first-copy-var-id',
296+
workflowId: 'first-copy-workflow-id',
297+
name: 'customerName',
298+
type: 'string',
299+
value: 'Ada',
300+
},
301+
},
302+
},
303+
],
304+
[],
305+
[],
306+
[
307+
{
308+
id: 'first-copy-block-id',
309+
workflowId: 'first-copy-workflow-id',
310+
type: 'agent',
311+
name: 'Agent',
312+
parentId: null,
313+
extent: null,
314+
data: {},
315+
subBlocks: {
316+
variables: {
317+
id: 'variables',
318+
type: 'variables-input',
319+
value: [
320+
{
321+
id: 'assignment-1',
322+
variableId: 'first-copy-var-id',
323+
variableName: 'customerName',
324+
type: 'string',
325+
value: 'Grace',
326+
isExisting: true,
327+
},
328+
],
329+
},
330+
},
331+
position: { x: 0, y: 0 },
332+
enabled: true,
333+
horizontalHandles: true,
334+
isWide: false,
335+
height: 0,
336+
advancedMode: false,
337+
triggerMode: false,
338+
locked: false,
339+
createdAt: new Date(),
340+
updatedAt: new Date(),
341+
},
342+
],
343+
[],
344+
[],
345+
],
346+
(values) => {
347+
if (!insertedWorkflowValues && !Array.isArray(values)) {
348+
insertedWorkflowValues = values
349+
}
350+
},
351+
(values) => {
352+
if (Array.isArray(values)) {
353+
insertedBlocks = values as Array<Record<string, unknown>>
354+
}
355+
}
356+
)
357+
358+
mockDb.transaction.mockImplementation(async (callback: (txArg: unknown) => Promise<unknown>) =>
359+
callback(tx)
360+
)
361+
362+
await duplicateWorkflow({
363+
sourceWorkflowId: 'first-copy-workflow-id',
364+
userId: 'user-123',
365+
name: 'Duplicated Again',
366+
workspaceId: 'workspace-123',
367+
folderId: null,
368+
requestId: 'req-second-copy',
369+
})
370+
371+
expect(insertedBlocks).toHaveLength(1)
372+
const copiedSubBlocks = insertedBlocks?.[0].subBlocks as Record<string, any>
373+
expect(Array.isArray(copiedSubBlocks.variables.value)).toBe(true)
374+
expect(copiedSubBlocks.variables.value).toHaveLength(1)
375+
376+
const newVarIds = Object.keys(
377+
(insertedWorkflowValues?.variables as Record<string, unknown>) || {}
378+
)
379+
expect(newVarIds).toHaveLength(1)
380+
const remappedVarId = copiedSubBlocks.variables.value[0].variableId
381+
expect(remappedVarId).not.toBe('first-copy-var-id')
382+
expect(remappedVarId).toBe(newVarIds[0])
383+
expect(copiedSubBlocks.variables.value[0].variableName).toBe('customerName')
384+
})
280385
})

0 commit comments

Comments
 (0)