Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions apps/sim/stores/workflows/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { BlockFactoryOptions } from '@sim/testing'
import {
createAgentBlock,
createBlock,
Expand Down Expand Up @@ -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<BlockFactoryOptions>, values?: Record<string, unknown>) {
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')
})
})
39 changes: 39 additions & 0 deletions apps/sim/stores/workflows/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,41 @@ function updateValueReferences(value: unknown, nameMap: Map<string, string>): 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<string, SubBlockState>,
subBlockValues: Record<string, unknown> | 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<string, BlockState>,
nameMap: Map<string, string>,
Expand Down Expand Up @@ -565,6 +600,10 @@ export function regenerateBlockIds(
})
})

Object.entries(newBlocks).forEach(([blockId, block]) => {
clearClonedWebhookPath(block.subBlocks, newSubBlockValues[blockId])
})

return {
blocks: newBlocks,
edges: newEdges,
Expand Down
69 changes: 69 additions & 0 deletions apps/sim/stores/workflows/workflow/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions apps/sim/stores/workflows/workflow/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { normalizeName } from '@/executor/constants'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import {
clearClonedWebhookPath,
filterNewEdges,
filterValidEdges,
getUniqueBlockName,
Expand Down Expand Up @@ -599,6 +600,7 @@ export const useWorkflowStore = create<WorkflowStore>()(
id,
newId
)
clearClonedWebhookPath(newSubBlocks as Record<string, SubBlockState>, clonedSubBlockValues)

const newState = {
blocks: {
Expand Down
Loading