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
7 changes: 6 additions & 1 deletion apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,12 @@ export async function copyWorkflowStateIntoTarget(
clearUnmapped: true,
canonicalModes: activeCanonicalModes,
})
subBlocks = remapConditionIdsInSubBlocks(subBlocks, oldBlockId, newBlockId) as SubBlockRecord
subBlocks = remapConditionIdsInSubBlocks(
subBlocks,
block.type,
oldBlockId,
newBlockId
) as SubBlockRecord

// Apply the stored dependent values for this block (the modal's mapping). The reference
// transform already cleared the source's dependent values when their parent was remapped,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { describe, expect, it, vi } from 'vitest'
import {
applyTriggerConfigToBlockSubblocks,
createBlockFromParams,
normalizeSubblockValue,
} from '@/lib/copilot/tools/server/workflow/edit-workflow/builders'
Expand Down Expand Up @@ -33,14 +34,27 @@ const knowledgeBlockConfig = {
],
}

const slackBlockConfig = {
type: 'slack',
name: 'Slack',
outputs: {},
subBlocks: [{ id: 'channel', type: 'channel-selector' }],
}

const blocksByType: Record<string, unknown> = {
agent: agentBlockConfig,
condition: conditionBlockConfig,
knowledge: knowledgeBlockConfig,
slack: slackBlockConfig,
}

vi.mock('@/blocks/registry', () => ({
getAllBlocks: () => [agentBlockConfig, conditionBlockConfig, knowledgeBlockConfig],
getAllBlocks: () => [
agentBlockConfig,
conditionBlockConfig,
knowledgeBlockConfig,
slackBlockConfig,
],
getBlock: (type: string) => blocksByType[type],
}))

Expand Down Expand Up @@ -168,3 +182,40 @@ describe('normalizeSubblockValue', () => {
expect(JSON.parse(result as string)[0].id).not.toBe('filter-1')
})
})

describe('applyTriggerConfigToBlockSubblocks', () => {
it('uses the registry type for declared keys and short-input only for undeclared keys', () => {
const block = { id: 'b1', type: 'slack', subBlocks: {} as Record<string, unknown> }

applyTriggerConfigToBlockSubblocks(block, { channel: 'C123', customField: 'x' })

expect(block.subBlocks.channel).toEqual({
id: 'channel',
type: 'channel-selector',
value: 'C123',
})
expect(block.subBlocks.customField).toEqual({
id: 'customField',
type: 'short-input',
value: 'x',
})
})

it('keeps the existing entry metadata when the key already exists', () => {
const block = {
id: 'b1',
type: 'slack',
subBlocks: {
channel: { id: 'channel', type: 'channel-selector', value: 'C-old' },
} as Record<string, { id: string; type: string; value: unknown }>,
}

applyTriggerConfigToBlockSubblocks(block, { channel: 'C-new' })

expect(block.subBlocks.channel).toEqual({
id: 'channel',
type: 'channel-selector',
value: 'C-new',
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
isCanonicalPair,
} from '@/lib/workflows/subblocks/visibility'
import { hasTriggerCapability } from '@/lib/workflows/triggers/trigger-utils'
import { getAllBlocks } from '@/blocks/registry'
import { getAllBlocks, getBlock } from '@/blocks/registry'
import type { BlockConfig } from '@/blocks/types'
import { TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants'
import type { EditWorkflowOperation, SkippedItem, ValidationError } from './types'
Expand Down Expand Up @@ -624,9 +624,14 @@ export function applyTriggerConfigToBlockSubblocks(block: any, triggerConfig: Re
value: configValue,
}
} else {
// The registry type is authoritative for declared keys; `short-input` is
// only the keep-alive default for dynamic trigger-config keys the block
// config does not declare (an `unknown` type would be dropped on the next
// sanitize pass, losing the value).
const subBlockDef = getBlock(block.type)?.subBlocks.find((sb) => sb.id === configKey)
Comment thread
icecrasher321 marked this conversation as resolved.
block.subBlocks[configKey] = {
id: configKey,
type: 'short-input',
type: subBlockDef?.type || 'short-input',
value: configValue,
}
}
Expand Down
1 change: 1 addition & 0 deletions apps/sim/lib/workflows/persistence/duplicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ export async function duplicateWorkflow(
if (updatedSubBlocks && typeof updatedSubBlocks === 'object') {
updatedSubBlocks = remapConditionIdsInSubBlocks(
updatedSubBlocks as Record<string, any>,
block.type,
block.id,
newBlockId
)
Expand Down
73 changes: 73 additions & 0 deletions apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { remapConditionEdgeHandle } from '@/lib/workflows/condition-ids'
import {
coerceObjectArray,
remapConditionIdsInSubBlocks,
remapWorkflowReferencesInSubBlocks,
type SubBlockRecord,
} from '@/lib/workflows/persistence/remap-internal-ids'
Expand Down Expand Up @@ -378,6 +380,77 @@ describe('remapWorkflowReferencesInSubBlocks', () => {
})
})

describe('remapConditionIdsInSubBlocks', () => {
const OLD_ID = 'old-block'
const NEW_ID = 'new-block'
const conditionsValue = JSON.stringify([
{ id: `${OLD_ID}-if`, title: 'if', value: '<a.b> > 1' },
{ id: `${OLD_ID}-else`, title: 'else', value: '' },
])

it('remaps condition row ids on a condition block', () => {
const subBlocks: SubBlockRecord = {
conditions: { id: 'conditions', type: 'condition-input', value: conditionsValue },
}
const result = remapConditionIdsInSubBlocks(subBlocks, 'condition', OLD_ID, NEW_ID)
const rows = JSON.parse(result.conditions.value as string)
expect(rows.map((row: { id: string }) => row.id)).toEqual([`${NEW_ID}-if`, `${NEW_ID}-else`])
})

/**
* Regression: a fallback writer stamped the conditions subblock `short-input`.
* The remap must key on block type + subblock key, not the drifted stored type,
* so the row ids and the edge handle move together (previously the ids stayed
* stale while the handle remapped, orphaning every edge out of the block).
*/
it('remaps condition row ids even when the stored subblock type drifted', () => {
const subBlocks: SubBlockRecord = {
conditions: { id: 'conditions', type: 'short-input', value: conditionsValue },
}
const result = remapConditionIdsInSubBlocks(subBlocks, 'condition', OLD_ID, NEW_ID)
const rows = JSON.parse(result.conditions.value as string)
const handle = remapConditionEdgeHandle(`condition-${OLD_ID}-else`, OLD_ID, NEW_ID)
expect(handle).toBe(`condition-${NEW_ID}-else`)
expect(rows.map((row: { id: string }) => row.id)).toContain(`${NEW_ID}-else`)
})

it('remaps route ids on a router_v2 block', () => {
const subBlocks: SubBlockRecord = {
routes: {
id: 'routes',
type: 'router-input',
value: JSON.stringify([{ id: `${OLD_ID}-route1`, title: 'Route 1', value: 'desc' }]),
},
}
const result = remapConditionIdsInSubBlocks(subBlocks, 'router_v2', OLD_ID, NEW_ID)
const rows = JSON.parse(result.routes.value as string)
expect(rows[0].id).toBe(`${NEW_ID}-route1`)
})

it('leaves rows with a foreign block-id prefix untouched (matches the edge-handle remap)', () => {
const subBlocks: SubBlockRecord = {
conditions: {
id: 'conditions',
type: 'condition-input',
value: JSON.stringify([{ id: 'foreign-block-if', title: 'if', value: '' }]),
},
}
const result = remapConditionIdsInSubBlocks(subBlocks, 'condition', OLD_ID, NEW_ID)
expect(result.conditions).toBe(subBlocks.conditions)
expect(remapConditionEdgeHandle('condition-foreign-block-if', OLD_ID, NEW_ID)).toBe(
'condition-foreign-block-if'
)
})

it('does not touch subblocks on non-dynamic-handle block types', () => {
const subBlocks: SubBlockRecord = {
conditions: { id: 'conditions', type: 'condition-input', value: conditionsValue },
}
const result = remapConditionIdsInSubBlocks(subBlocks, 'function', OLD_ID, NEW_ID)
expect(result.conditions).toBe(subBlocks.conditions)
})
})

describe('coerceObjectArray', () => {
it('returns arrays directly', () => {
expect(coerceObjectArray([{ a: 1 }])).toEqual({ array: [{ a: 1 }], wasString: false })
Expand Down
10 changes: 9 additions & 1 deletion apps/sim/lib/workflows/persistence/remap-internal-ids.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { createLogger } from '@sim/logger'
import { remapConditionBlockIds } from '@/lib/workflows/condition-ids'
import { isDynamicHandleSubblock } from '@/lib/workflows/dynamic-handle-topology'
import {
type CanonicalModeOverrides,
resolveCanonicalMode,
Expand Down Expand Up @@ -321,9 +322,16 @@ function remapWorkflowInputTools(
/**
* Remap condition/router block IDs within subBlocks when a block is copied with
* a new ID. Returns a new object without mutating the input.
*
* Gated on the BLOCK type + canonical subblock key (`conditions`/`routes`), not
* the stored subblock `type`: edge handles are remapped by string prefix with no
* type gate, so keying this side on mutable stored metadata lets a drifted type
* (e.g. a `conditions` entry stamped `short-input` by a fallback writer) skip the
* id remap while the handles still move — orphaning every edge out of the block.
*/
export function remapConditionIdsInSubBlocks(
subBlocks: SubBlockRecord,
blockType: string | undefined,
oldBlockId: string,
newBlockId: string
): SubBlockRecord {
Expand All @@ -332,7 +340,7 @@ export function remapConditionIdsInSubBlocks(
if (
subBlock &&
typeof subBlock === 'object' &&
(subBlock.type === 'condition-input' || subBlock.type === 'router-input') &&
isDynamicHandleSubblock(blockType, key) &&
typeof subBlock.value === 'string'
) {
try {
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/lib/workflows/persistence/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { LRUCache } from 'lru-cache'
import type { Edge } from 'reactflow'
import { releaseWebhookPathClaims } from '@/lib/webhooks/path-claims'
import { remapConditionBlockIds, remapConditionEdgeHandle } from '@/lib/workflows/condition-ids'
import { isDynamicHandleSubblock } from '@/lib/workflows/dynamic-handle-topology'
import {
backfillCanonicalModes,
migrateSubblockIds,
Expand Down Expand Up @@ -717,7 +718,7 @@ export function regenerateWorkflowStateIds(state: RegenerateStateInput): Regener
}

if (
(updatedSubBlock.type === 'condition-input' || updatedSubBlock.type === 'router-input') &&
isDynamicHandleSubblock(block.type, subId) &&
typeof updatedSubBlock.value === 'string'
) {
try {
Expand Down
54 changes: 54 additions & 0 deletions apps/sim/lib/workflows/sanitization/subblocks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,5 +133,59 @@ describe('sanitizeMalformedSubBlocks', () => {
expect(changed).toBe(true)
expect(subBlocks.code).toEqual({ id: 'code', type: 'code', value: 'return 1' })
})

it('repairs a stored type that contradicts the configured type', () => {
const { subBlocks, changed } = sanitizeMalformedSubBlocks({
id: 'block-1',
type: 'function',
subBlocks: {
code: { id: 'code', type: 'short-input', value: 'return 1' },
},
})

expect(changed).toBe(true)
expect(subBlocks.code).toEqual({ id: 'code', type: 'code', value: 'return 1' })
})

/**
* Regression: a fallback writer stamped a condition block's `conditions`
* subblock `short-input`. Copy-time id remapping used to gate on that stored
* type, skipping the conditions array while edge handles still remapped —
* orphaning every edge out of the block on fork/duplicate/import.
*/
it('repairs a condition block conditions subblock stamped short-input by a fallback writer', () => {
const conditionsValue = JSON.stringify([
{ id: 'block-1-if', title: 'if', value: '<a.b>' },
{ id: 'block-1-else', title: 'else', value: '' },
])
const { subBlocks, changed } = sanitizeMalformedSubBlocks({
id: 'block-1',
type: 'condition',
subBlocks: {
conditions: { id: 'conditions', type: 'short-input', value: conditionsValue },
},
})

expect(changed).toBe(true)
expect(subBlocks.conditions).toEqual({
id: 'conditions',
type: 'condition-input',
value: conditionsValue,
})
})

it('preserves a valid stored type for keys the config does not declare (runtime values)', () => {
const input = {
webhookId: { id: 'webhookId', type: 'short-input', value: 'wh-1' },
}
const { subBlocks, changed } = sanitizeMalformedSubBlocks({
id: 'block-1',
type: 'function',
subBlocks: input,
})

expect(changed).toBe(false)
expect(subBlocks).toBe(input)
})
})
})
23 changes: 20 additions & 3 deletions apps/sim/lib/workflows/sanitization/subblocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ interface SanitizableBlock {
* Repairs legacy subBlock metadata when the map key identifies a real field,
* and drops entries that cannot be associated with a stable subBlock.
*
* For keys the block registry declares, the CONFIGURED type is authoritative
* and overwrites a contradicting stored type. Stored types drift in two ways:
* fallback writers stamp a plausible-but-wrong default (`short-input`) when
* they synthesize a missing structure entry, and block configs evolve their
* declared types over time while persisted rows keep the old one. A wrong
* stored type silently disables type-gated logic downstream — most damaging
* for `condition-input`/`router-input`, where copy-time id remapping skips
* the conditions array while edge handles still remap, orphaning the edges.
* Draft loads persist this repair via `persistMigratedBlocks`, so stored
* state converges back to the registry.
*
* Custom blocks are schema-agnostic here: their server-side config never
* declares the per-field input sub-blocks (the execution overlay passes bare
* wiring rows, and this may run with no overlay at all), so "not in config"
Expand Down Expand Up @@ -95,10 +106,11 @@ export function sanitizeMalformedSubBlocks(
continue
}

const type =
const storedType =
typeof subBlock.type === 'string' && subBlock.type.length > 0 && subBlock.type !== 'unknown'
? subBlock.type
: typeFromConfig || DEFAULT_SUBBLOCK_TYPE
: null
const type = typeFromConfig ?? storedType ?? DEFAULT_SUBBLOCK_TYPE
const hasValue = Object.hasOwn(subBlock, 'value')
const value =
options.convertEmptyStringToNull && subBlock.value === ''
Expand All @@ -111,7 +123,12 @@ export function sanitizeMalformedSubBlocks(
const normalizedValue = hasValue && value !== subBlock.value

if (repairedMetadata) {
logger.warn('Repairing malformed subBlock metadata', { blockId: block.id, subBlockId })
logger.warn('Repairing malformed subBlock metadata', {
blockId: block.id,
subBlockId,
storedType: subBlock.type,
repairedType: type,
})
changed = true
} else if (normalizedValue) {
logger.warn('Normalizing malformed subBlock value', { blockId: block.id, subBlockId })
Expand Down
Loading
Loading