From 14afed6ec490306fc2c6f920157b29ee69dd23bf Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 29 Jul 2026 22:24:58 -0700 Subject: [PATCH] fix(triggers): stop cloned trigger blocks reusing the source's webhook URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/stores/workflows/utils.test.ts | 154 ++++++++++++++++++ apps/sim/stores/workflows/utils.ts | 39 +++++ .../stores/workflows/workflow/store.test.ts | 69 ++++++++ apps/sim/stores/workflows/workflow/store.ts | 2 + 4 files changed, 264 insertions(+) diff --git a/apps/sim/stores/workflows/utils.test.ts b/apps/sim/stores/workflows/utils.test.ts index 68966905752..78c602d2954 100644 --- a/apps/sim/stores/workflows/utils.test.ts +++ b/apps/sim/stores/workflows/utils.test.ts @@ -1,3 +1,4 @@ +import type { BlockFactoryOptions } from '@sim/testing' import { createAgentBlock, createBlock, @@ -857,3 +858,156 @@ describe('regenerateBlockIds', () => { expect(names).toContain('Agent 2') }) }) + +describe('regenerateBlockIds — cloned webhook path', () => { + const positionOffset = { x: 50, y: 50 } + const sourceId = 'webhook-source' + const deployedPath = 'webhook-source' + + function pasteOne(block: Partial, values?: Record) { + const blocks = { [sourceId]: createBlock({ id: sourceId, ...block }) } + const result = regenerateBlockIds( + blocks, + [], + {}, + {}, + values ? { [sourceId]: values } : {}, + positionOffset, + {}, + getUniqueBlockName + ) + return { newId: Object.keys(result.blocks)[0], result } + } + + /** + * The reported bug. A Webhook Trigger added from the toolbar carries `triggerMode: true` + * (confirmed against production rows), and after a deploy its `triggerPath` holds the registered + * path — its own block id. A clone that copies that value renders the SOURCE's URL. + */ + it('clears triggerPath on a pasted webhook trigger (triggerMode true)', () => { + const { newId, result } = pasteOne( + { + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + }, + }, + { triggerPath: deployedPath } + ) + + expect(newId).not.toBe(sourceId) + // Both sources must be cleared: the value map overrides the structure in mergeSubblockState. + expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull() + expect(result.subBlockValues[newId].triggerPath).toBeNull() + }) + + /** Rows written by the API/import path can carry `triggerMode: false`; same requirement. */ + it('clears triggerPath on a pasted webhook trigger (triggerMode false)', () => { + const { newId, result } = pasteOne( + { + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: false, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + }, + }, + { triggerPath: deployedPath } + ) + + expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull() + expect(result.subBlockValues[newId].triggerPath).toBeNull() + }) + + it('clears it when the path lives only in the value map', () => { + const { newId, result } = pasteOne( + { type: 'generic_webhook', name: 'Webhook 1', triggerMode: true, subBlocks: {} }, + { triggerPath: deployedPath } + ) + + expect(result.subBlockValues[newId].triggerPath).toBeNull() + }) + + it('clears it when the block has no value-map entry at all', () => { + const { newId, result } = pasteOne({ + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + }, + }) + + expect(result.blocks[newId].subBlocks.triggerPath?.value).toBeNull() + }) + + /** + * `webhookId` is a user-entered action field on Attio, Vercel, and Discord — and Attio/Vercel are + * trigger-capable, so any predicate keyed on trigger-ness would wipe it in trigger mode. It is + * deliberately NOT cleared: nothing reads it as trigger state. + */ + it('preserves a user-entered webhookId on a trigger-capable block in trigger mode', () => { + const { newId, result } = pasteOne( + { + type: 'attio', + name: 'Attio 1', + triggerMode: true, + subBlocks: { + webhookId: { id: 'webhookId', type: 'short-input', value: 'attio-wh-42' }, + }, + }, + { webhookId: 'attio-wh-42' } + ) + + expect(result.blocks[newId].subBlocks.webhookId?.value).toBe('attio-wh-42') + expect(result.subBlockValues[newId].webhookId).toBe('attio-wh-42') + }) + + it('preserves a user-entered webhookId on an action block', () => { + const { newId, result } = pasteOne( + { + type: 'discord', + name: 'Discord 1', + triggerMode: false, + subBlocks: { + webhookId: { id: 'webhookId', type: 'short-input', value: '1234567890' }, + webhookToken: { id: 'webhookToken', type: 'short-input', value: 'tok_abc' }, + }, + }, + { webhookId: '1234567890', webhookToken: 'tok_abc' } + ) + + expect(result.subBlockValues[newId].webhookId).toBe('1234567890') + expect(result.subBlockValues[newId].webhookToken).toBe('tok_abc') + }) + + /** Trigger configuration is user setup and must survive the copy. */ + it('preserves trigger configuration on a cloned trigger block', () => { + const { newId, result } = pasteOne( + { + type: 'generic_webhook', + name: 'Webhook 1', + triggerMode: true, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: deployedPath }, + triggerConfig: { id: 'triggerConfig', type: 'short-input', value: { labelIds: ['a'] } }, + triggerId: { id: 'triggerId', type: 'short-input', value: 'generic_webhook' }, + token: { id: 'token', type: 'short-input', value: 'user-secret' }, + }, + }, + { + triggerPath: deployedPath, + triggerConfig: { labelIds: ['a'] }, + triggerId: 'generic_webhook', + token: 'user-secret', + } + ) + + expect(result.subBlockValues[newId].triggerPath).toBeNull() + expect(result.subBlockValues[newId].triggerConfig).toEqual({ labelIds: ['a'] }) + expect(result.subBlockValues[newId].triggerId).toBe('generic_webhook') + expect(result.subBlockValues[newId].token).toBe('user-secret') + }) +}) diff --git a/apps/sim/stores/workflows/utils.ts b/apps/sim/stores/workflows/utils.ts index eaa60d714bd..3ab29e5d7b7 100644 --- a/apps/sim/stores/workflows/utils.ts +++ b/apps/sim/stores/workflows/utils.ts @@ -251,6 +251,41 @@ function updateValueReferences(value: unknown, nameMap: Map): un return value } +/** + * Clears a cloned block's `triggerPath` so it derives a fresh webhook URL from its own block id. + * + * Before a deploy this field is empty and the URL is DERIVED — `useWebhookManagement` and the canvas + * both fall back to the block id, which cloning already regenerates. Deploy then registers the + * webhook at `triggerPath || block.id` and writes that literal path back into the source block, so + * from then on the URL is STORED and a clone would copy it verbatim and render the source's URL. + * + * Clears BOTH the sub-block structure and the sub-block value map. Both are required: + * `mergeSubblockStateWithValues` treats the value map as authoritative — a `null` there overrides the + * structure — but only materializes an entry for a structure-less key when the value is non-null. So + * nulling the map covers the common shape (no trigger declares `triggerPath` as a subblock, so it + * normally lives only in the store) and clearing the structure covers blocks hydrated from a merge. + * + * Deliberately unconditional and limited to `triggerPath`. No block declares `triggerPath` as a + * subblock, so there is nothing to collide with and no need to classify the block first. The sibling + * `TRIGGER_RUNTIME_SUBBLOCK_IDS` entries are all left alone on purpose: `triggerConfig`/`triggerId` + * are user configuration a clone should keep, and `webhookId` is a user-entered action field on the + * Attio, Vercel, and Discord blocks while being unused as trigger state (deploy mints its own row id + * and matches existing rows by block id, and `useWebhookManagement` overwrites the field from the + * server), so clearing it would destroy real config for no benefit. + * + * Mutates both arguments in place; both must be clone-owned copies. `subBlockValues` is optional so + * a caller with no value-map entry passes `undefined` rather than a throwaway object literal whose + * writes would be silently discarded. + */ +export function clearClonedWebhookPath( + subBlocks: Record, + subBlockValues: Record | undefined +): void { + const subBlock = subBlocks.triggerPath + if (subBlock) subBlocks.triggerPath = { ...subBlock, value: null } + if (subBlockValues && 'triggerPath' in subBlockValues) subBlockValues.triggerPath = null +} + function updateBlockReferences( blocks: Record, nameMap: Map, @@ -565,6 +600,10 @@ export function regenerateBlockIds( }) }) + Object.entries(newBlocks).forEach(([blockId, block]) => { + clearClonedWebhookPath(block.subBlocks, newSubBlockValues[blockId]) + }) + return { blocks: newBlocks, edges: newEdges, diff --git a/apps/sim/stores/workflows/workflow/store.test.ts b/apps/sim/stores/workflows/workflow/store.test.ts index f5072971faf..43e79b4cac5 100644 --- a/apps/sim/stores/workflows/workflow/store.test.ts +++ b/apps/sim/stores/workflows/workflow/store.test.ts @@ -494,6 +494,75 @@ describe('workflow store', () => { }) }) + describe('duplicateBlock cloned webhook path', () => { + /** + * `duplicateBlock` is not currently reachable from the canvas (the context-menu Duplicate goes + * through `preparePasteData` → `regenerateBlockIds`), but it is part of the store's public API, + * so the clone it produces must not inherit the source's deployed webhook identity either. + */ + it('clears triggerPath when duplicating a webhook trigger block', () => { + const { duplicateBlock } = useWorkflowStore.getState() + useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' }) + useWorkflowStore.setState({ currentWorkflowId: 'wf-1' }) + + addBlock('original', 'generic_webhook', 'Webhook 1', { x: 0, y: 0 }) + useWorkflowStore.setState((state) => ({ + blocks: { + ...state.blocks, + original: { + ...state.blocks.original, + subBlocks: { + triggerPath: { id: 'triggerPath', type: 'short-input', value: 'original' }, + webhookId: { id: 'webhookId', type: 'short-input', value: 'wh_original' }, + }, + }, + }, + })) + useSubBlockStore.setState({ + workflowValues: { + 'wf-1': { original: { triggerPath: 'original', webhookId: 'wh_original' } }, + }, + }) + + duplicateBlock('original') + + const { blocks } = useWorkflowStore.getState() + const duplicatedId = Object.keys(blocks).find((id) => id !== 'original') + expect(duplicatedId).toBeDefined() + if (!duplicatedId) return + + // Both sources must be cleared: the value map overrides the structure in mergeSubblockState. + expect(blocks[duplicatedId].subBlocks.triggerPath?.value).toBeNull() + const values = useSubBlockStore.getState().workflowValues['wf-1'] + expect(values[duplicatedId].triggerPath).toBeNull() + // webhookId is user-entered action config on some blocks and is deliberately preserved. + expect(values[duplicatedId].webhookId).toBe('wh_original') + // The source keeps its own identity. + expect(values.original.triggerPath).toBe('original') + }) + + it('preserves a user-entered webhookId when duplicating an action block', () => { + const { duplicateBlock } = useWorkflowStore.getState() + useWorkflowRegistry.setState({ activeWorkflowId: 'wf-1' }) + useWorkflowStore.setState({ currentWorkflowId: 'wf-1' }) + + addBlock('original', 'discord', 'Discord 1', { x: 0, y: 0 }) + useSubBlockStore.setState({ + workflowValues: { 'wf-1': { original: { webhookId: '1234567890' } } }, + }) + + duplicateBlock('original') + + const { blocks } = useWorkflowStore.getState() + const duplicatedId = Object.keys(blocks).find((id) => id !== 'original') + expect(duplicatedId).toBeDefined() + if (!duplicatedId) return + + const values = useSubBlockStore.getState().workflowValues['wf-1'] + expect(values[duplicatedId].webhookId).toBe('1234567890') + }) + }) + describe('batchUpdatePositions', () => { it('should update block position', () => { const { batchUpdatePositions } = useWorkflowStore.getState() diff --git a/apps/sim/stores/workflows/workflow/store.ts b/apps/sim/stores/workflows/workflow/store.ts index 11008b1658f..740bf790eaf 100644 --- a/apps/sim/stores/workflows/workflow/store.ts +++ b/apps/sim/stores/workflows/workflow/store.ts @@ -12,6 +12,7 @@ import { import { normalizeName } from '@/executor/constants' import { useSubBlockStore } from '@/stores/workflows/subblock/store' import { + clearClonedWebhookPath, filterNewEdges, filterValidEdges, getUniqueBlockName, @@ -599,6 +600,7 @@ export const useWorkflowStore = create()( id, newId ) + clearClonedWebhookPath(newSubBlocks as Record, clonedSubBlockValues) const newState = { blocks: {