Skip to content

Commit 791cede

Browse files
committed
fix(triggers): stop workflow copy paths wiping a user-entered webhookId
The trigger-runtime strip on the workflow-level copy paths was keyed on subblock id alone. Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId` field, so duplicating, forking, or importing a workflow blanked it while leaving the `webhookToken` it pairs with — a silently half-configured copy that fails at runtime with an error pointing nowhere near the copy that caused it. - Gate the TRIGGER_RUNTIME_SUBBLOCK_IDS strip on isBlockActingAsTrigger in sanitizeSubBlocksForDuplicate (server duplicate + workspace forking) and in updateBlockReferences (export/import). - Make sanitizeSubBlocksForDuplicate's flag a required parameter so every call site must decide rather than inheriting a silent default. - Widen isBlockActingAsTrigger to accept a DB block row as well as BlockState. Trigger blocks are unaffected — they still lose their webhook identity on a copy. Display-only system subblocks are still stripped unconditionally; those ids exist only on trigger definitions. Verified `scheduleInfo` is the only other collision and it belongs to the schedule block, which is category: 'triggers'.
1 parent 8cd0127 commit 791cede

7 files changed

Lines changed: 184 additions & 8 deletions

File tree

apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from '@/lib/workflows/persistence/remap-internal-ids'
1414
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
1515
import type { CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility'
16+
import { isBlockActingAsTrigger } from '@/lib/workflows/triggers/trigger-utils'
1617
import {
1718
deriveForkBlockId,
1819
type ForkBlockIdResolver,
@@ -436,7 +437,10 @@ export async function copyWorkflowStateIntoTarget(
436437

437438
// double-cast-allowed: SubBlockState is structurally a SubBlockRecord entry but lacks the open index signature SubBlockRecord declares
438439
const sourceSubBlocks = (block.subBlocks ?? {}) as unknown as SubBlockRecord
439-
const sanitizedSource = sanitizeSubBlocksForDuplicate(sourceSubBlocks)
440+
const sanitizedSource = sanitizeSubBlocksForDuplicate(
441+
sourceSubBlocks,
442+
isBlockActingAsTrigger(block)
443+
)
440444
let subBlocks: SubBlockRecord = sanitizedSource
441445
// Tracks the block's live `canonicalModes` through this pass, so a `tool-input` reindex
442446
// (a dropped custom-tool/MCP entry shifts later tools' array positions) is visible to every

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
type SubBlockRecord,
2323
sanitizeSubBlocksForDuplicate,
2424
} from '@/lib/workflows/persistence/remap-internal-ids'
25+
import { isBlockActingAsTrigger } from '@/lib/workflows/triggers/trigger-utils'
2526
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
2627
import type { Variable } from '@/stores/variables/types'
2728
import type { LoopConfig, ParallelConfig } from '@/stores/workflows/workflow/types'
@@ -304,7 +305,10 @@ export async function duplicateWorkflow(
304305
typeof updatedSubBlocks === 'object' &&
305306
!Array.isArray(updatedSubBlocks)
306307
) {
307-
updatedSubBlocks = sanitizeSubBlocksForDuplicate(updatedSubBlocks as SubBlockRecord)
308+
updatedSubBlocks = sanitizeSubBlocksForDuplicate(
309+
updatedSubBlocks as SubBlockRecord,
310+
isBlockActingAsTrigger(block)
311+
)
308312
}
309313
if (
310314
varIdMapping.size > 0 &&

apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
remapConditionIdsInSubBlocks,
99
remapWorkflowReferencesInSubBlocks,
1010
type SubBlockRecord,
11+
sanitizeSubBlocksForDuplicate,
1112
} from '@/lib/workflows/persistence/remap-internal-ids'
1213

1314
describe('remapWorkflowReferencesInSubBlocks', () => {
@@ -463,3 +464,78 @@ describe('coerceObjectArray', () => {
463464
expect(coerceObjectArray(42)).toEqual({ array: null, wasString: false })
464465
})
465466
})
467+
468+
describe('sanitizeSubBlocksForDuplicate', () => {
469+
/**
470+
* Regression: the strip was keyed on subblock id alone, so duplicating or forking a workflow
471+
* blanked Discord/Attio/Vercel's REQUIRED user-entered `webhookId` while leaving the
472+
* `webhookToken` it pairs with — a silently half-configured copy.
473+
*/
474+
it('keeps a user-entered webhookId on a block that is not acting as a trigger', () => {
475+
const discord: SubBlockRecord = {
476+
operation: { type: 'dropdown', value: 'discord_execute_webhook' },
477+
webhookId: { type: 'short-input', value: '1234567890' },
478+
webhookToken: { type: 'short-input', value: 'tok_abc' },
479+
content: { type: 'long-input', value: 'hello' },
480+
}
481+
482+
const out = sanitizeSubBlocksForDuplicate(discord, false)
483+
484+
expect(out.webhookId).toEqual({ type: 'short-input', value: '1234567890' })
485+
expect(out.webhookToken).toEqual({ type: 'short-input', value: 'tok_abc' })
486+
expect(out.content).toBeDefined()
487+
expect(out.operation).toBeDefined()
488+
})
489+
490+
it('keeps prefixed keys on a block that is not acting as a trigger', () => {
491+
const out = sanitizeSubBlocksForDuplicate(
492+
{
493+
webhookId_custom: { type: 'short-input', value: 'x' },
494+
triggerConfig: { type: 'short-input', value: { labelIds: ['a'] } },
495+
},
496+
false
497+
)
498+
499+
expect(out.webhookId_custom).toBeDefined()
500+
expect(out.triggerConfig).toBeDefined()
501+
})
502+
503+
it('still strips webhook runtime identity on a real trigger block', () => {
504+
const out = sanitizeSubBlocksForDuplicate(
505+
{
506+
webhookId: { type: 'short-input', value: 'wh_original' },
507+
triggerPath: { type: 'short-input', value: 'deployed-path' },
508+
triggerConfig: { type: 'short-input', value: { labelIds: ['a'] } },
509+
triggerId: { type: 'short-input', value: 'generic_webhook' },
510+
requireAuth: { type: 'switch', value: true },
511+
},
512+
true
513+
)
514+
515+
expect(out.webhookId).toBeUndefined()
516+
expect(out.triggerPath).toBeUndefined()
517+
expect(out.triggerConfig).toBeUndefined()
518+
expect(out.triggerId).toBeUndefined()
519+
expect(out.requireAuth).toBeDefined()
520+
})
521+
522+
it('strips display-only system subblocks regardless of trigger status', () => {
523+
for (const isTrigger of [true, false]) {
524+
const out = sanitizeSubBlocksForDuplicate(
525+
{
526+
webhookUrlDisplay: { type: 'short-input', value: 'https://x' },
527+
samplePayload: { type: 'code', value: '{}' },
528+
scheduleInfo: { type: 'schedule-info', value: null },
529+
triggerCredentials: { type: 'oauth-input', value: 'cred-1' },
530+
},
531+
isTrigger
532+
)
533+
534+
expect(out.webhookUrlDisplay).toBeUndefined()
535+
expect(out.samplePayload).toBeUndefined()
536+
expect(out.scheduleInfo).toBeUndefined()
537+
// triggerCredentials is deliberately preserved so a copy keeps its OAuth connection.
538+
expect(out.triggerCredentials).toBeDefined()
539+
}
540+
})
541+
})

apps/sim/lib/workflows/persistence/remap-internal-ids.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,25 @@ export function isSystemSubBlockKey(key: string, ids: Set<string> | string[]): b
4747
return idList.some((id) => key === id || key.startsWith(`${id}_`))
4848
}
4949

50-
/** Strip trigger-runtime and non-credential system subblocks for a fresh copy. */
51-
export function sanitizeSubBlocksForDuplicate(subBlocks: SubBlockRecord): SubBlockRecord {
50+
/**
51+
* Strip trigger-runtime and non-credential system subblocks for a fresh copy.
52+
*
53+
* `isActingAsTrigger` is required, not optional, so every call site must decide: the
54+
* {@link TRIGGER_RUNTIME_SUBBLOCK_IDS} strip is only correct for a block that actually owns a
55+
* webhook. Discord, Attio, and Vercel action blocks expose a REQUIRED user-entered `webhookId`
56+
* field, and stripping it left the copy half-configured (`webhookToken` survives while the id it
57+
* pairs with vanishes). Resolve the flag with `isBlockActingAsTrigger(block)`.
58+
*
59+
* The display-only system subblocks are stripped unconditionally: those ids exist only on trigger
60+
* definitions, so the strip is a no-op for anything else.
61+
*/
62+
export function sanitizeSubBlocksForDuplicate(
63+
subBlocks: SubBlockRecord,
64+
isActingAsTrigger: boolean
65+
): SubBlockRecord {
5266
const sanitized: SubBlockRecord = {}
5367
for (const [key, subBlock] of Object.entries(subBlocks)) {
54-
if (isSystemSubBlockKey(key, TRIGGER_RUNTIME_SUBBLOCK_IDS)) continue
68+
if (isActingAsTrigger && isSystemSubBlockKey(key, TRIGGER_RUNTIME_SUBBLOCK_IDS)) continue
5569
if (isSystemSubBlockKey(key, DUPLICATE_STRIPPED_SYSTEM_SUBBLOCK_IDS)) continue
5670
sanitized[key] = subBlock
5771
}

apps/sim/lib/workflows/triggers/trigger-utils.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,10 @@ export function getAllTriggerBlocks(): TriggerInfo[] {
210210
* silently clear it. Mirrors the deploy-side `resolveTriggerId` gate so both sides agree on which
211211
* blocks own a webhook.
212212
*/
213-
export function isBlockActingAsTrigger(block: BlockState): boolean {
213+
export function isBlockActingAsTrigger(block: {
214+
type: string
215+
triggerMode?: boolean | null
216+
}): boolean {
214217
if (block.triggerMode === true) return true
215218
return getBlock(block.type)?.category === 'triggers'
216219
}

apps/sim/stores/workflows/utils.test.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
import type { Edge } from 'reactflow'
99
import { describe, expect, it } from 'vitest'
1010
import { normalizeName } from '@/executor/constants'
11-
import { getUniqueBlockName, regenerateBlockIds } from './utils'
11+
import { getUniqueBlockName, regenerateBlockIds, regenerateWorkflowIds } from './utils'
1212

1313
describe('normalizeName', () => {
1414
it.concurrent('should convert to lowercase', () => {
@@ -989,3 +989,75 @@ describe('regenerateBlockIds — trigger webhook identity', () => {
989989
expect(result.blocks[newId].subBlocks.token?.value).toBe('user-secret')
990990
})
991991
})
992+
993+
describe('regenerateWorkflowIds — trigger webhook identity', () => {
994+
function stateWith(block: Record<string, unknown>) {
995+
return {
996+
blocks: { 'block-1': { id: 'block-1', position: { x: 0, y: 0 }, outputs: {}, ...block } },
997+
edges: [],
998+
loops: {},
999+
parallels: {},
1000+
} as never
1001+
}
1002+
1003+
/**
1004+
* Regression: import/export cleared runtime ids by subblock id alone, so an imported Discord
1005+
* block lost its REQUIRED user-entered `webhookId` while keeping the `webhookToken` it pairs
1006+
* with. Trigger blocks (below) must still be cleared.
1007+
*/
1008+
it('keeps a user-entered webhookId on a non-trigger action block', () => {
1009+
const out = regenerateWorkflowIds(
1010+
stateWith({
1011+
type: 'discord',
1012+
name: 'Discord 1',
1013+
triggerMode: false,
1014+
subBlocks: {
1015+
operation: { id: 'operation', type: 'dropdown', value: 'discord_execute_webhook' },
1016+
webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' },
1017+
webhookToken: { id: 'webhookToken', type: 'short-input', value: 'tok_abc' },
1018+
},
1019+
})
1020+
)
1021+
1022+
const block = Object.values(out.blocks)[0]
1023+
expect(block.subBlocks.webhookId?.value).toBe('1234567890')
1024+
expect(block.subBlocks.webhookToken?.value).toBe('tok_abc')
1025+
})
1026+
1027+
it('still clears runtime identity on a block in trigger mode', () => {
1028+
const out = regenerateWorkflowIds(
1029+
stateWith({
1030+
type: 'generic_webhook',
1031+
name: 'Webhook 1',
1032+
triggerMode: true,
1033+
subBlocks: {
1034+
webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' },
1035+
triggerPath: { id: 'triggerPath', type: 'short-input', value: 'deployed-path' },
1036+
requireAuth: { id: 'requireAuth', type: 'switch', value: true },
1037+
},
1038+
})
1039+
)
1040+
1041+
const block = Object.values(out.blocks)[0]
1042+
expect(block.subBlocks.webhookId?.value).toBeNull()
1043+
expect(block.subBlocks.triggerPath?.value).toBeNull()
1044+
expect(block.subBlocks.requireAuth?.value).toBe(true)
1045+
})
1046+
1047+
it('leaves runtime identity alone when clearTriggerRuntimeValues is opted out', () => {
1048+
const out = regenerateWorkflowIds(
1049+
stateWith({
1050+
type: 'generic_webhook',
1051+
name: 'Webhook 1',
1052+
triggerMode: true,
1053+
subBlocks: {
1054+
triggerPath: { id: 'triggerPath', type: 'short-input', value: 'deployed-path' },
1055+
},
1056+
}),
1057+
{ clearTriggerRuntimeValues: false }
1058+
)
1059+
1060+
const block = Object.values(out.blocks)[0]
1061+
expect(block.subBlocks.triggerPath?.value).toBe('deployed-path')
1062+
})
1063+
})

apps/sim/stores/workflows/utils.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,12 @@ function updateBlockReferences(
290290
clearTriggerRuntimeValues = false
291291
): void {
292292
Object.entries(blocks).forEach(([_, block]) => {
293+
// The clear is only correct for a block that actually owns a webhook — Discord, Attio, and
294+
// Vercel action blocks expose a REQUIRED user-entered `webhookId` field that must survive.
295+
const clearRuntimeValues = clearTriggerRuntimeValues && isBlockActingAsTrigger(block)
293296
if (block.subBlocks) {
294297
Object.entries(block.subBlocks).forEach(([subBlockId, subBlock]) => {
295-
if (clearTriggerRuntimeValues && TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(subBlockId)) {
298+
if (clearRuntimeValues && TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(subBlockId)) {
296299
block.subBlocks[subBlockId] = { ...subBlock, value: null }
297300
return
298301
}

0 commit comments

Comments
 (0)