From b2a8afdbd552a917f6e34f28c2ae6eb4f4f7d093 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:48:06 -0700 Subject: [PATCH] fix(workflows): remap all tool canonical modes across API edits --- apps/sim/lib/workflows/editing/builders.ts | 48 +++++- .../lib/workflows/editing/operations.test.ts | 86 ++++++++++ apps/sim/lib/workflows/editing/operations.ts | 11 +- .../editing/tool-canonical-modes.test.ts | 158 ++++++++++++++++++ .../workflows/editing/tool-canonical-modes.ts | 91 ++++++++++ 5 files changed, 389 insertions(+), 5 deletions(-) create mode 100644 apps/sim/lib/workflows/editing/tool-canonical-modes.test.ts create mode 100644 apps/sim/lib/workflows/editing/tool-canonical-modes.ts diff --git a/apps/sim/lib/workflows/editing/builders.ts b/apps/sim/lib/workflows/editing/builders.ts index 3b45b30325c..340b050f2a2 100644 --- a/apps/sim/lib/workflows/editing/builders.ts +++ b/apps/sim/lib/workflows/editing/builders.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' import { generateId, isValidUuid } from '@sim/utils/id' -import { sortObjectKeysDeep } from '@sim/utils/object' +import { isRecordLike, sortObjectKeysDeep } from '@sim/utils/object' import { type BlockRetryConfig, normalizeBlockRetryTries, @@ -19,8 +19,10 @@ import { } from '@/lib/permission-groups/operation-access' import { getEffectiveBlockOutputs } from '@/lib/workflows/blocks/block-outputs' import { isRetryEligibleBlock } from '@/lib/workflows/blocks/retry-eligibility' +import { remapToolCanonicalModes } from '@/lib/workflows/editing/tool-canonical-modes' import { buildCanonicalIndex, + buildCanonicalIndexForSurface, buildDefaultCanonicalModes, isCanonicalPair, } from '@/lib/workflows/subblocks/visibility' @@ -277,9 +279,13 @@ export function createBlockFromParams( } export function updateCanonicalModesForInputs( - block: { data?: { canonicalModes?: Record } }, + block: { + data?: { canonicalModes?: Record } + subBlocks?: Record + }, inputKeys: string[], - blockConfig: BlockConfig + blockConfig: BlockConfig, + previousTools?: unknown ): void { if (!blockConfig.subBlocks?.length) return @@ -308,6 +314,42 @@ export function updateCanonicalModesForInputs( if (!block.data.canonicalModes) block.data.canonicalModes = {} Object.assign(block.data.canonicalModes, canonicalModeUpdates) } + + if (blockConfig.type === 'agent' && inputKeys.includes('tools')) { + const tools = block.subBlocks?.tools?.value + if (Array.isArray(tools)) { + const canonicalModes = remapToolCanonicalModes( + Array.isArray(previousTools) ? normalizeTools(previousTools) : [], + tools, + block.data?.canonicalModes ?? {}, + collectExplicitToolCanonicalModes(tools) + ) + block.data = { ...block.data, canonicalModes } + } + } +} + +function collectExplicitToolCanonicalModes(tools: unknown[]) { + const modes = new Map>() + tools.forEach((tool, index) => { + if (!isRecordLike(tool)) return + const choices: Record = {} + const config = typeof tool.type === 'string' ? getBlock(tool.type) : undefined + if (config && isRecordLike(tool.params)) { + const params = tool.params + const canonicalIndex = buildCanonicalIndexForSurface(config.subBlocks, false) + for (const group of Object.values(canonicalIndex.groupsById)) { + if (!isCanonicalPair(group) || !group.basicId) continue + const hasBasic = params[group.basicId] !== undefined + const hasAdvanced = group.advancedIds.some((id) => params[id] !== undefined) + if (hasBasic !== hasAdvanced) { + choices[group.canonicalId] = hasAdvanced ? 'advanced' : 'basic' + } + } + } + if (Object.keys(choices).length) modes.set(index, choices) + }) + return modes } /** diff --git a/apps/sim/lib/workflows/editing/operations.test.ts b/apps/sim/lib/workflows/editing/operations.test.ts index c9f8511bc60..43f5956e769 100644 --- a/apps/sim/lib/workflows/editing/operations.test.ts +++ b/apps/sim/lib/workflows/editing/operations.test.ts @@ -1169,3 +1169,89 @@ describe('permission-group tool access', () => { ) }) }) + +describe('API tool canonical mode remapping', () => { + const first = { + type: 'jira', + operation: 'get_issue', + params: { projectId: 'project-a', manualProjectId: '' }, + } + const second = { + type: 'jira', + operation: 'get_issue', + params: { projectId: 'project-b', manualProjectId: '' }, + } + + it.each([ + { operation_type: 'edit', explicit: false }, + { operation_type: 'insert_into_subflow', explicit: false }, + { operation_type: 'edit', explicit: true }, + { operation_type: 'insert_into_subflow', explicit: true }, + ] as const)( + 'moves selector modes during $operation_type (explicit choice: $explicit)', + ({ operation_type, explicit }) => { + const blockId = 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' + const workflow = { + blocks: { + [blockId]: { + id: blockId, + type: 'agent', + name: 'Agent', + position: { x: 0, y: 0 }, + enabled: true, + outputs: {}, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: [first, second] } }, + data: { + canonicalModes: { + '0:projectId': 'advanced' as const, + '1:projectId': 'basic' as const, + '0:issueKey': 'basic' as const, + model: 'advanced' as const, + }, + }, + }, + loop: { + id: 'loop', + type: 'loop', + name: 'Loop', + position: { x: 0, y: 0 }, + enabled: true, + outputs: {}, + subBlocks: {}, + data: { loopType: 'for', count: 2 }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } + const result = applyOperationsToWorkflowState(workflow, [ + { + operation_type, + block_id: blockId, + params: { + ...(operation_type === 'insert_into_subflow' + ? { subflowId: 'loop', type: 'agent', name: 'Agent' } + : {}), + inputs: { + tools: structuredClone([ + second, + explicit ? { ...first, params: { projectId: 'edited-project' } } : first, + ]), + }, + }, + }, + ]) + expect(result.validationErrors).toEqual([]) + expect(result.state.blocks[blockId].subBlocks.tools.value[0].params.projectId).toBe( + 'project-b' + ) + expect(result.state.blocks[blockId].data.canonicalModes).toEqual({ + '0:projectId': 'basic', + '1:projectId': explicit ? 'basic' : 'advanced', + '1:issueKey': 'basic', + model: 'advanced', + }) + } + ) +}) diff --git a/apps/sim/lib/workflows/editing/operations.ts b/apps/sim/lib/workflows/editing/operations.ts index eb677e3683d..b3a19be77b2 100644 --- a/apps/sim/lib/workflows/editing/operations.ts +++ b/apps/sim/lib/workflows/editing/operations.ts @@ -572,7 +572,12 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon const editBlockConfig = getBlock(block.type) if (editBlockConfig) { - updateCanonicalModesForInputs(block, [...explicitInputKeys], editBlockConfig) + updateCanonicalModesForInputs( + block, + [...explicitInputKeys], + editBlockConfig, + previousSubBlockValues.get('tools') + ) const changedInputKeys = editBlockConfig.subBlocks .filter((subBlock) => { @@ -957,6 +962,7 @@ export function handleInsertIntoSubflowOperation( // Update inputs if provided (with validation) if (params.inputs) { + const previousTools = existingBlock.subBlocks?.tools?.value // Validate inputs against block configuration const validationResult = validateInputsForBlock(existingBlock.type, params.inputs, block_id) validationErrors.push(...validationResult.errors) @@ -1014,7 +1020,8 @@ export function handleInsertIntoSubflowOperation( updateCanonicalModesForInputs( existingBlock, Object.keys(validationResult.validInputs), - existingBlockConfig + existingBlockConfig, + previousTools ) } } diff --git a/apps/sim/lib/workflows/editing/tool-canonical-modes.test.ts b/apps/sim/lib/workflows/editing/tool-canonical-modes.test.ts new file mode 100644 index 00000000000..a1ba89d9eb8 --- /dev/null +++ b/apps/sim/lib/workflows/editing/tool-canonical-modes.test.ts @@ -0,0 +1,158 @@ +/** @vitest-environment node */ +import { describe, expect, it } from 'vitest' +import { remapToolCanonicalModes } from '@/lib/workflows/editing/tool-canonical-modes' + +const first = { type: 'jira', operation: 'get_issue', params: { projectId: 'first' } } +const second = { type: 'jira', operation: 'get_issue', params: { projectId: 'second' } } + +describe('remapToolCanonicalModes', () => { + it('moves every indexed field and preserves block-level and legacy keys', () => { + expect( + remapToolCanonicalModes([first, second], structuredClone([second, first]), { + '0:projectId': 'advanced', + '0:issueKey': 'basic', + '1:projectId': 'basic', + model: 'advanced', + 'jira:issueKey': 'advanced', + }) + ).toEqual({ + '1:projectId': 'advanced', + '1:issueKey': 'basic', + '0:projectId': 'basic', + model: 'advanced', + 'jira:issueKey': 'advanced', + }) + }) + + it('reserves exact matches before matching an edited duplicate by callable identity', () => { + const edited = { ...first, params: { projectId: 'edited' } } + expect( + remapToolCanonicalModes([first, second], [edited, first], { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }) + ).toEqual({ '1:projectId': 'basic', '0:projectId': 'advanced' }) + }) + + it('drops removed and stale indexes so replacements do not inherit settings', () => { + expect( + remapToolCanonicalModes([first], [{ type: 'slack' }], { + '0:projectId': 'advanced', + '9:channel': 'basic', + model: 'basic', + }) + ).toEqual({ model: 'basic' }) + }) + + it('clears indexed modes for an empty list', () => { + expect( + remapToolCanonicalModes([first], [], { + '0:projectId': 'advanced', + model: 'basic', + }) + ).toEqual({ model: 'basic' }) + }) + + it('preserves separate modes for identical tools when the array is unchanged', () => { + const modes = { '0:projectId': 'basic', '1:projectId': 'advanced' } as const + expect(remapToolCanonicalModes([first, first], structuredClone([first, first]), modes)).toEqual( + modes + ) + }) + + it('ignores visual-only changes on an unchanged duplicate list', () => { + expect( + remapToolCanonicalModes( + [first, first], + [{ ...first, isExpanded: true, title: 'Renamed' }, first], + { '1:projectId': 'advanced' } + ) + ).toEqual({ '1:projectId': 'advanced' }) + }) + + it('rejects indistinguishable duplicates with different saved modes', () => { + expect(() => + remapToolCanonicalModes([first, first], [first], { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }) + ).toThrow('ambiguous canonical modes') + }) + + it('permits ambiguous duplicates when their saved modes agree', () => { + expect( + remapToolCanonicalModes([first, first], [first], { + '0:projectId': 'advanced', + '1:projectId': 'advanced', + }) + ).toEqual({ '0:projectId': 'advanced' }) + }) + + it('uses an explicit field selection to resolve ambiguity', () => { + expect( + remapToolCanonicalModes( + [first, first], + [first], + { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }, + new Map([[0, { projectId: 'basic' }]]) + ) + ).toEqual({ '0:projectId': 'basic' }) + }) + + it('does not let one explicit selection hide a conflict in another field', () => { + expect(() => + remapToolCanonicalModes( + [first, first], + [first], + { + '0:projectId': 'basic', + '1:projectId': 'advanced', + '1:issueKey': 'advanced', + }, + new Map([[0, { projectId: 'basic' }]]) + ) + ).toThrow('ambiguous canonical modes') + }) + + it.each([ + [ + { type: 'custom-tool', customToolId: 'one' }, + { type: 'custom-tool', customToolId: 'two' }, + ], + [ + { type: 'mcp', params: { serverId: 'one', toolName: 'search' } }, + { type: 'mcp', params: { serverId: 'two', toolName: 'search' } }, + ], + [ + { type: 'mcp-server-advanced', params: { serverId: 'one' } }, + { type: 'mcp-server-advanced', params: { serverId: 'two' } }, + ], + [ + { type: 'workflow', params: { workflowId: 'one' } }, + { type: 'workflow', params: { workflowId: 'two' } }, + ], + ])('keeps callable and target identities distinct: %j', (a, b) => { + expect(remapToolCanonicalModes([a, b], [b, a], { '0:field': 'advanced' })).toEqual({ + '1:field': 'advanced', + }) + }) + + it('preserves all modes when reversing the maximum API tool count', () => { + const tools = Array.from({ length: 100 }, (_, index) => ({ + ...first, + params: { projectId: `${index}` }, + })) + const modes = Object.fromEntries( + tools.map((_, index) => [`${index}:projectId`, index % 2 ? 'basic' : 'advanced'] as const) + ) + const expected = Object.fromEntries( + tools.map( + (_, index) => [`${99 - index}:projectId`, index % 2 ? 'basic' : 'advanced'] as const + ) + ) + expect(remapToolCanonicalModes(tools, [...tools].reverse(), modes)).toEqual(expected) + }) +}) diff --git a/apps/sim/lib/workflows/editing/tool-canonical-modes.ts b/apps/sim/lib/workflows/editing/tool-canonical-modes.ts new file mode 100644 index 00000000000..a1d67519cb8 --- /dev/null +++ b/apps/sim/lib/workflows/editing/tool-canonical-modes.ts @@ -0,0 +1,91 @@ +import { isRecordLike, omit } from '@sim/utils/object' +import { isEqual } from 'es-toolkit' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { MCP_SERVER_ADVANCED_TOOL_TYPE } from '@/lib/mcp/shared' +import { reindexCanonicalModesByPosition } from '@/lib/workflows/subblocks/visibility' + +type CanonicalModes = Record + +function describeTool(tool: unknown) { + if (!isRecordLike(tool)) return undefined + const params = isRecordLike(tool.params) ? tool.params : {} + const schema = isRecordLike(tool.schema) ? tool.schema : {} + const fn = isRecordLike(schema.function) ? schema.function : {} + const identity = + tool.type === 'custom-tool' + ? [tool.type, tool.customToolId ?? fn.name] + : tool.type === 'mcp' || tool.type === MCP_SERVER_ADVANCED_TOOL_TYPE + ? [tool.type, params.serverId, params.toolName] + : [tool.type, tool.operation ?? tool.toolId] + return { identity, configuration: omit(tool, ['isExpanded', 'title']) } +} + +/** + * API arrays lose object identity. Match unchanged configurations before edited callable + * identities, then move every indexed mode together. Explicit choices disambiguate only + * the fields they override; other conflicting choices still require an unambiguous edit. + */ +export function remapToolCanonicalModes( + previousTools: readonly unknown[], + tools: readonly unknown[], + modes: CanonicalModes, + explicitModes: ReadonlyMap = new Map() +): CanonicalModes { + const previous = previousTools.map(describeTool) + const current = tools.map(describeTool) + const newIndexByOldIndex = new Map() + const modesByOldIndex = new Map() + for (const [key, mode] of Object.entries(modes)) { + const match = /^(\d+):(.+)$/.exec(key) + if (!match) continue + const index = Number(match[1]) + const scoped = modesByOldIndex.get(index) ?? {} + scoped[match[2]] = mode + modesByOldIndex.set(index, scoped) + } + + if (isEqual(previous, current)) { + current.forEach((_, index) => newIndexByOldIndex.set(index, index)) + } else { + const available = new Set(previous.map((_, index) => index)) + const matched = new Set() + for (const exact of [true, false]) { + current.forEach((tool, index) => { + if (!tool || matched.has(index)) return + const candidates = [...available].filter((oldIndex) => { + const oldTool = previous[oldIndex] + return ( + oldTool && isEqual(exact ? oldTool : oldTool.identity, exact ? tool : tool.identity) + ) + }) + if (!candidates.length) return + const effectiveModes = (oldIndex: number) => ({ + ...modesByOldIndex.get(oldIndex), + ...explicitModes.get(index), + }) + if ( + candidates.some( + (oldIndex) => !isEqual(effectiveModes(oldIndex), effectiveModes(candidates[0])) + ) + ) { + throw new OrchestrationError( + 'validation', + `Tool ${index + 1} has ambiguous canonical modes after editing repeated tools. Keep its configuration distinct or explicitly choose each conflicting mode.` + ) + } + const oldIndex = candidates[0] + newIndexByOldIndex.set(oldIndex, index) + available.delete(oldIndex) + matched.add(index) + }) + } + } + + const result = { ...(reindexCanonicalModesByPosition(newIndexByOldIndex, modes) ?? modes) } + for (const [index, choices] of explicitModes) { + for (const [canonicalId, mode] of Object.entries(choices)) { + result[`${index}:${canonicalId}`] = mode + } + } + return result +}