Skip to content

Commit 1ce2aef

Browse files
authored
fix(triggers): stop cloned trigger blocks reusing the source's webhook URL (#6091)
Before a deploy, a trigger block's webhook URL is DERIVED — useWebhookManagement and the canvas both fall back to the block's own id, which cloning already regenerates, so copying an undeployed trigger yields a fresh URL. Deploy then registers the webhook at `path = triggerPath || block.id` and writes that literal path back into the source block's `triggerPath`. From then on the URL is STORED, and copy/paste copies it verbatim, so the clone renders the source's URL. Clear `triggerPath` on in-canvas clones so the clone derives a path from its new block id again. - Clears both the subBlocks structure and the sub-block value map: the value map is authoritative in mergeSubblockStateWithValues, and since no trigger declares `triggerPath` as a subblock it normally lives only in the store. - Deliberately unconditional and limited to `triggerPath`. No block declares that id, so there is nothing to collide with and no need to classify the block. - The sibling TRIGGER_RUNTIME_SUBBLOCK_IDS entries are left alone on purpose. `triggerConfig`/`triggerId` are user configuration. `webhookId` is a user-entered action field on Attio, Vercel, and Discord — and Attio/Vercel are trigger-capable, so clearing it by trigger-ness would destroy real config — while being unused as trigger state, since deploy mints its own row id and matches existing rows by block id. Scoped to the clone paths. No deploy-side, server-side, or import/export change.
1 parent edef789 commit 1ce2aef

4 files changed

Lines changed: 264 additions & 0 deletions

File tree

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

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { BlockFactoryOptions } from '@sim/testing'
12
import {
23
createAgentBlock,
34
createBlock,
@@ -857,3 +858,156 @@ describe('regenerateBlockIds', () => {
857858
expect(names).toContain('Agent 2')
858859
})
859860
})
861+
862+
describe('regenerateBlockIds — cloned webhook path', () => {
863+
const positionOffset = { x: 50, y: 50 }
864+
const sourceId = 'webhook-source'
865+
const deployedPath = 'webhook-source'
866+
867+
function pasteOne(block: Partial<BlockFactoryOptions>, values?: Record<string, unknown>) {
868+
const blocks = { [sourceId]: createBlock({ id: sourceId, ...block }) }
869+
const result = regenerateBlockIds(
870+
blocks,
871+
[],
872+
{},
873+
{},
874+
values ? { [sourceId]: values } : {},
875+
positionOffset,
876+
{},
877+
getUniqueBlockName
878+
)
879+
return { newId: Object.keys(result.blocks)[0], result }
880+
}
881+
882+
/**
883+
* The reported bug. A Webhook Trigger added from the toolbar carries `triggerMode: true`
884+
* (confirmed against production rows), and after a deploy its `triggerPath` holds the registered
885+
* path — its own block id. A clone that copies that value renders the SOURCE's URL.
886+
*/
887+
it('clears triggerPath on a pasted webhook trigger (triggerMode true)', () => {
888+
const { newId, result } = pasteOne(
889+
{
890+
type: 'generic_webhook',
891+
name: 'Webhook 1',
892+
triggerMode: true,
893+
subBlocks: {
894+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
895+
},
896+
},
897+
{ triggerPath: deployedPath }
898+
)
899+
900+
expect(newId).not.toBe(sourceId)
901+
// Both sources must be cleared: the value map overrides the structure in mergeSubblockState.
902+
expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull()
903+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
904+
})
905+
906+
/** Rows written by the API/import path can carry `triggerMode: false`; same requirement. */
907+
it('clears triggerPath on a pasted webhook trigger (triggerMode false)', () => {
908+
const { newId, result } = pasteOne(
909+
{
910+
type: 'generic_webhook',
911+
name: 'Webhook 1',
912+
triggerMode: false,
913+
subBlocks: {
914+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
915+
},
916+
},
917+
{ triggerPath: deployedPath }
918+
)
919+
920+
expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull()
921+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
922+
})
923+
924+
it('clears it when the path lives only in the value map', () => {
925+
const { newId, result } = pasteOne(
926+
{ type: 'generic_webhook', name: 'Webhook 1', triggerMode: true, subBlocks: {} },
927+
{ triggerPath: deployedPath }
928+
)
929+
930+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
931+
})
932+
933+
it('clears it when the block has no value-map entry at all', () => {
934+
const { newId, result } = pasteOne({
935+
type: 'generic_webhook',
936+
name: 'Webhook 1',
937+
triggerMode: true,
938+
subBlocks: {
939+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
940+
},
941+
})
942+
943+
expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull()
944+
})
945+
946+
/**
947+
* `webhookId` is a user-entered action field on Attio, Vercel, and Discord — and Attio/Vercel are
948+
* trigger-capable, so any predicate keyed on trigger-ness would wipe it in trigger mode. It is
949+
* deliberately NOT cleared: nothing reads it as trigger state.
950+
*/
951+
it('preserves a user-entered webhookId on a trigger-capable block in trigger mode', () => {
952+
const { newId, result } = pasteOne(
953+
{
954+
type: 'attio',
955+
name: 'Attio 1',
956+
triggerMode: true,
957+
subBlocks: {
958+
webhookId: { id: 'webhookId', type: 'short-input', value: 'attio-wh-42' },
959+
},
960+
},
961+
{ webhookId: 'attio-wh-42' }
962+
)
963+
964+
expect(result.blocks[newId].subBlocks.webhookId?.value).toBe('attio-wh-42')
965+
expect(result.subBlockValues[newId].webhookId).toBe('attio-wh-42')
966+
})
967+
968+
it('preserves a user-entered webhookId on an action block', () => {
969+
const { newId, result } = pasteOne(
970+
{
971+
type: 'discord',
972+
name: 'Discord 1',
973+
triggerMode: false,
974+
subBlocks: {
975+
webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' },
976+
webhookToken: { id: 'webhookToken', type: 'short-input', value: 'tok_abc' },
977+
},
978+
},
979+
{ webhookId: '1234567890', webhookToken: 'tok_abc' }
980+
)
981+
982+
expect(result.subBlockValues[newId].webhookId).toBe('1234567890')
983+
expect(result.subBlockValues[newId].webhookToken).toBe('tok_abc')
984+
})
985+
986+
/** Trigger configuration is user setup and must survive the copy. */
987+
it('preserves trigger configuration on a cloned trigger block', () => {
988+
const { newId, result } = pasteOne(
989+
{
990+
type: 'generic_webhook',
991+
name: 'Webhook 1',
992+
triggerMode: true,
993+
subBlocks: {
994+
triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath },
995+
triggerConfig: { id: 'triggerConfig', type: 'short-input', value: { labelIds: ['a'] } },
996+
triggerId: { id: 'triggerId', type: 'short-input', value: 'generic_webhook' },
997+
token: { id: 'token', type: 'short-input', value: 'user-secret' },
998+
},
999+
},
1000+
{
1001+
triggerPath: deployedPath,
1002+
triggerConfig: { labelIds: ['a'] },
1003+
triggerId: 'generic_webhook',
1004+
token: 'user-secret',
1005+
}
1006+
)
1007+
1008+
expect(result.subBlockValues[newId].triggerPath).toBeNull()
1009+
expect(result.subBlockValues[newId].triggerConfig).toEqual({ labelIds: ['a'] })
1010+
expect(result.subBlockValues[newId].triggerId).toBe('generic_webhook')
1011+
expect(result.subBlockValues[newId].token).toBe('user-secret')
1012+
})
1013+
})

apps/sim/stores/workflows/utils.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,41 @@ function updateValueReferences(value: unknown, nameMap: Map<string, string>): un
251251
return value
252252
}
253253

254+
/**
255+
* Clears a cloned block's `triggerPath` so it derives a fresh webhook URL from its own block id.
256+
*
257+
* Before a deploy this field is empty and the URL is DERIVED — `useWebhookManagement` and the canvas
258+
* both fall back to the block id, which cloning already regenerates. Deploy then registers the
259+
* webhook at `triggerPath || block.id` and writes that literal path back into the source block, so
260+
* from then on the URL is STORED and a clone would copy it verbatim and render the source's URL.
261+
*
262+
* Clears BOTH the sub-block structure and the sub-block value map. Both are required:
263+
* `mergeSubblockStateWithValues` treats the value map as authoritative — a `null` there overrides the
264+
* structure — but only materializes an entry for a structure-less key when the value is non-null. So
265+
* nulling the map covers the common shape (no trigger declares `triggerPath` as a subblock, so it
266+
* normally lives only in the store) and clearing the structure covers blocks hydrated from a merge.
267+
*
268+
* Deliberately unconditional and limited to `triggerPath`. No block declares `triggerPath` as a
269+
* subblock, so there is nothing to collide with and no need to classify the block first. The sibling
270+
* `TRIGGER_RUNTIME_SUBBLOCK_IDS` entries are all left alone on purpose: `triggerConfig`/`triggerId`
271+
* are user configuration a clone should keep, and `webhookId` is a user-entered action field on the
272+
* Attio, Vercel, and Discord blocks while being unused as trigger state (deploy mints its own row id
273+
* and matches existing rows by block id, and `useWebhookManagement` overwrites the field from the
274+
* server), so clearing it would destroy real config for no benefit.
275+
*
276+
* Mutates both arguments in place; both must be clone-owned copies. `subBlockValues` is optional so
277+
* a caller with no value-map entry passes `undefined` rather than a throwaway object literal whose
278+
* writes would be silently discarded.
279+
*/
280+
export function clearClonedWebhookPath(
281+
subBlocks: Record<string, SubBlockState>,
282+
subBlockValues: Record<string, unknown> | undefined
283+
): void {
284+
const subBlock = subBlocks.triggerPath
285+
if (subBlock) subBlocks.triggerPath = { ...subBlock, value: null }
286+
if (subBlockValues && 'triggerPath' in subBlockValues) subBlockValues.triggerPath = null
287+
}
288+
254289
function updateBlockReferences(
255290
blocks: Record<string, BlockState>,
256291
nameMap: Map<string, string>,
@@ -565,6 +600,10 @@ export function regenerateBlockIds(
565600
})
566601
})
567602

603+
Object.entries(newBlocks).forEach(([blockId, block]) => {
604+
clearClonedWebhookPath(block.subBlocks, newSubBlockValues[blockId])
605+
})
606+
568607
return {
569608
blocks: newBlocks,
570609
edges: newEdges,

apps/sim/stores/workflows/workflow/store.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,75 @@ describe('workflow store', () => {
494494
})
495495
})
496496

497+
describe('duplicateBlock cloned webhook path', () => {
498+
/**
499+
* `duplicateBlock` is not currently reachable from the canvas (the context-menu Duplicate goes
500+
* through `preparePasteData` → `regenerateBlockIds`), but it is part of the store's public API,
501+
* so the clone it produces must not inherit the source's deployed webhook identity either.
502+
*/
503+
it('clears triggerPath when duplicating a webhook trigger block', () => {
504+
const { duplicateBlock } = useWorkflowStore.getState()
505+
useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' })
506+
useWorkflowStore.setState({ currentWorkflowId: 'wf-1' })
507+
508+
addBlock('original', 'generic_webhook', 'Webhook 1', { x: 0, y: 0 })
509+
useWorkflowStore.setState((state) => ({
510+
blocks: {
511+
...state.blocks,
512+
original: {
513+
...state.blocks.original,
514+
subBlocks: {
515+
triggerPath: { id: 'triggerPath', type: 'short-input', value: 'original' },
516+
webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' },
517+
},
518+
},
519+
},
520+
}))
521+
useSubBlockStore.setState({
522+
workflowValues: {
523+
'wf-1': { original: { triggerPath: 'original', webhookId: 'wh_original' } },
524+
},
525+
})
526+
527+
duplicateBlock('original')
528+
529+
const { blocks } = useWorkflowStore.getState()
530+
const duplicatedId = Object.keys(blocks).find((id) => id !== 'original')
531+
expect(duplicatedId).toBeDefined()
532+
if (!duplicatedId) return
533+
534+
// Both sources must be cleared: the value map overrides the structure in mergeSubblockState.
535+
expect(blocks[duplicatedId].subBlocks.triggerPath?.value).toBeNull()
536+
const values = useSubBlockStore.getState().workflowValues['wf-1']
537+
expect(values[duplicatedId].triggerPath).toBeNull()
538+
// webhookId is user-entered action config on some blocks and is deliberately preserved.
539+
expect(values[duplicatedId].webhookId).toBe('wh_original')
540+
// The source keeps its own identity.
541+
expect(values.original.triggerPath).toBe('original')
542+
})
543+
544+
it('preserves a user-entered webhookId when duplicating an action block', () => {
545+
const { duplicateBlock } = useWorkflowStore.getState()
546+
useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' })
547+
useWorkflowStore.setState({ currentWorkflowId: 'wf-1' })
548+
549+
addBlock('original', 'discord', 'Discord 1', { x: 0, y: 0 })
550+
useSubBlockStore.setState({
551+
workflowValues: { 'wf-1': { original: { webhookId: '1234567890' } } },
552+
})
553+
554+
duplicateBlock('original')
555+
556+
const { blocks } = useWorkflowStore.getState()
557+
const duplicatedId = Object.keys(blocks).find((id) => id !== 'original')
558+
expect(duplicatedId).toBeDefined()
559+
if (!duplicatedId) return
560+
561+
const values = useSubBlockStore.getState().workflowValues['wf-1']
562+
expect(values[duplicatedId].webhookId).toBe('1234567890')
563+
})
564+
})
565+
497566
describe('batchUpdatePositions', () => {
498567
it('should update block position', () => {
499568
const { batchUpdatePositions } = useWorkflowStore.getState()

apps/sim/stores/workflows/workflow/store.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import { normalizeName } from '@/executor/constants'
1313
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
1414
import {
15+
clearClonedWebhookPath,
1516
filterNewEdges,
1617
filterValidEdges,
1718
getUniqueBlockName,
@@ -599,6 +600,7 @@ export const useWorkflowStore = create<WorkflowStore>()(
599600
id,
600601
newId
601602
)
603+
clearClonedWebhookPath(newSubBlocks as Record<string, SubBlockState>, clonedSubBlockValues)
602604

603605
const newState = {
604606
blocks: {

0 commit comments

Comments
 (0)