From d2306ad9cb0f673e847f630226e34c2908fb8ec2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:27:50 -0700 Subject: [PATCH 1/4] fix(agent): keep nested tool basic/advanced modes attached to their tools Tool canonical-mode overrides are keyed by array position, so a reorder or removal must move them with the tools. - Workflow edit engine (v2 operations API, Chat) reindexes modes when a batch rewrites a block's tool list, matching tools by content, then by type - Editor persists the tool list and reindexed modes in one realtime operation instead of two independent writes that could partially persist --- apps/realtime/src/database/operations.test.ts | 48 ++++++++++ apps/realtime/src/database/operations.ts | 74 ++++++++++++--- .../src/middleware/permissions.test.ts | 10 ++ apps/realtime/src/middleware/permissions.ts | 1 + .../components/tool-input/tool-input.tsx | 65 ++++++++----- apps/sim/hooks/use-collaborative-workflow.ts | 89 ++++++++++------- apps/sim/lib/workflows/editing/engine.ts | 38 ++++++++ .../lib/workflows/editing/operations.test.ts | 82 ++++++++++++++++ .../workflows/subblocks/visibility.test.ts | 95 +++++++++++++++++++ .../sim/lib/workflows/subblocks/visibility.ts | 68 ++++++++++++- apps/sim/stores/operation-queue/store.test.ts | 45 +++++++++ apps/sim/stores/operation-queue/store.ts | 20 +++- packages/realtime-protocol/src/constants.ts | 6 ++ packages/realtime-protocol/src/schemas.ts | 23 ++++- 14 files changed, 583 insertions(+), 81 deletions(-) diff --git a/apps/realtime/src/database/operations.test.ts b/apps/realtime/src/database/operations.test.ts index 061a37af21d..1f1c5585842 100644 --- a/apps/realtime/src/database/operations.test.ts +++ b/apps/realtime/src/database/operations.test.ts @@ -121,3 +121,51 @@ describe('search replacement persistence', () => { expect(mockSet).toHaveBeenCalledTimes(1) }) }) + +describe('subblock update with canonical modes persistence', () => { + const tools = [{ type: 'jira', params: { manualProjectId: '{{PROJECT}}' } }] + const canonicalModes = { '0:projectId': 'advanced' as const, model: 'basic' as const } + + beforeEach(() => { + vi.clearAllMocks() + mockTransaction.mockImplementation( + async (callback: (tx: typeof transaction) => Promise) => callback(transaction) + ) + mockSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) + }) + + function updateTools(block: Record) { + mockSelectWhere.mockResolvedValue([ + { + id: 'agent-1', + locked: false, + data: { width: 350, canonicalModes: { '1:projectId': 'advanced' } }, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: [] } }, + ...block, + }, + ]) + return persistWorkflowOperation('workflow-1', { + operation: SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES, + target: OPERATION_TARGETS.SUBBLOCK, + timestamp: Date.now(), + payload: { blockId: 'agent-1', subblockId: 'tools', value: tools, canonicalModes }, + }) + } + + it('writes the subblock value and replaces canonical modes in one block update', async () => { + await expect(updateTools({})).resolves.toBeUndefined() + + expect(mockSet).toHaveBeenCalledTimes(2) + expect(mockSet).toHaveBeenLastCalledWith( + expect.objectContaining({ + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: tools } }, + data: { width: 350, canonicalModes }, + }) + ) + }) + + it('skips a locked block without writing either field', async () => { + await expect(updateTools({ locked: true })).resolves.toBeUndefined() + expect(mockSet).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/realtime/src/database/operations.ts b/apps/realtime/src/database/operations.ts index 777c6078316..84b8722177b 100644 --- a/apps/realtime/src/database/operations.ts +++ b/apps/realtime/src/database/operations.ts @@ -1989,6 +1989,30 @@ async function handleSubflowOperationTx( } } +interface SubblockUpdateBlockRecord { + id: string + subBlocks: unknown + locked: boolean + data: unknown +} + +/** Every block in the workflow by id, for the locked-container check subblock writes need. */ +async function loadSubblockUpdateBlocks( + tx: any, + workflowId: string +): Promise> { + const allBlocks: SubblockUpdateBlockRecord[] = await tx + .select({ + id: workflowBlocks.id, + subBlocks: workflowBlocks.subBlocks, + locked: workflowBlocks.locked, + data: workflowBlocks.data, + }) + .from(workflowBlocks) + .where(eq(workflowBlocks.workflowId, workflowId)) + return Object.fromEntries(allBlocks.map((block) => [block.id, block])) +} + // Subblock operations - targeted value updates without replacing workflow state async function handleSubblockOperationTx( tx: any, @@ -2003,20 +2027,7 @@ async function handleSubblockOperationTx( return } - const allBlocks = await tx - .select({ - id: workflowBlocks.id, - subBlocks: workflowBlocks.subBlocks, - locked: workflowBlocks.locked, - data: workflowBlocks.data, - }) - .from(workflowBlocks) - .where(eq(workflowBlocks.workflowId, workflowId)) - - type SubblockUpdateBlockRecord = (typeof allBlocks)[number] - const blocksById: Record = Object.fromEntries( - allBlocks.map((block: SubblockUpdateBlockRecord) => [block.id, block]) - ) + const blocksById = await loadSubblockUpdateBlocks(tx, workflowId) for (const update of updates) { const { blockId, subblockId, value, expectedValue } = update @@ -2060,6 +2071,41 @@ async function handleSubblockOperationTx( break } + case SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES: { + const { blockId, subblockId, value, canonicalModes } = payload + if (!blockId || !subblockId || !canonicalModes) { + throw new Error('Missing required fields for subblock update with canonical modes') + } + + const blocksById = await loadSubblockUpdateBlocks(tx, workflowId) + const block = blocksById[blockId] + if (!block) { + throw new Error(`Block ${blockId} not found`) + } + if (isWorkflowBlockProtected(blockId, blocksById)) { + logger.info(`Skipping subblock update of locked block ${blockId}`) + break + } + + const subBlocks = { ...((block.subBlocks as Record) || {}) } + const currentSubBlock = subBlocks[subblockId] + subBlocks[subblockId] = currentSubBlock + ? { ...currentSubBlock, value } + : { id: subblockId, type: 'unknown', value } + + await tx + .update(workflowBlocks) + .set({ + subBlocks, + data: { ...((block.data as Record) || {}), canonicalModes }, + updatedAt: new Date(), + }) + .where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId))) + + logger.debug(`Updated subblock ${blockId}.${subblockId} with canonical modes`) + break + } + default: logger.warn(`Unknown subblock operation: ${operation}`) throw new Error(`Unsupported subblock operation: ${operation}`) diff --git a/apps/realtime/src/middleware/permissions.test.ts b/apps/realtime/src/middleware/permissions.test.ts index 4edc05d795a..97f32b1d2d2 100644 --- a/apps/realtime/src/middleware/permissions.test.ts +++ b/apps/realtime/src/middleware/permissions.test.ts @@ -114,6 +114,11 @@ describe('checkRolePermission', () => { const result = checkRolePermission('write', 'subblock-batch-update') expectPermissionAllowed(result) }) + + it('should allow subblock-update-with-canonical-modes operation', () => { + const result = checkRolePermission('write', 'subblock-update-with-canonical-modes') + expectPermissionAllowed(result) + }) }) describe('read role', () => { @@ -155,6 +160,11 @@ describe('checkRolePermission', () => { expectPermissionDenied(result, 'read') }) + it('should deny subblock-update-with-canonical-modes operation for read role', () => { + const result = checkRolePermission('read', 'subblock-update-with-canonical-modes') + expectPermissionDenied(result, 'read') + }) + it('should deny toggle-enabled operation for read role', () => { const result = checkRolePermission('read', 'toggle-enabled') expectPermissionDenied(result, 'read') diff --git a/apps/realtime/src/middleware/permissions.ts b/apps/realtime/src/middleware/permissions.ts index e17678461a6..a007de6c8b2 100644 --- a/apps/realtime/src/middleware/permissions.ts +++ b/apps/realtime/src/middleware/permissions.ts @@ -52,6 +52,7 @@ const WRITE_OPERATIONS: string[] = [ // Subblock operations SUBBLOCK_OPERATIONS.UPDATE, SUBBLOCK_OPERATIONS.BATCH_UPDATE, + SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES, // Variable operations VARIABLE_OPERATIONS.UPDATE, // Workflow operations diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index abd43aa7c1c..40f7c25a165 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -392,15 +392,8 @@ export const ToolInput = memo(function ToolInput({ [blockId] ) ) - const { collaborativeSetBlockCanonicalMode, collaborativeSetBlockCanonicalModes } = + const { collaborativeSetBlockCanonicalMode, collaborativeSetSubblockValueWithCanonicalModes } = useCollaborativeWorkflow() - const reindexCanonicalModesOnMutate = useCallback( - (oldTools: StoredTool[], newTools: StoredTool[]) => { - const next = reindexToolCanonicalModes(oldTools, newTools, canonicalModeOverrides) - if (next) collaborativeSetBlockCanonicalModes(blockId, next) - }, - [canonicalModeOverrides, collaborativeSetBlockCanonicalModes, blockId] - ) const value = isPreview ? previewValue : storeValue @@ -412,6 +405,39 @@ export const ToolInput = memo(function ToolInput({ ? (value as StoredTool[]) : [] + /** + * Commits a tool list that moves or drops selected tools. Their canonical-mode overrides are + * keyed by position, so when any must move they persist in the same operation as the list. + * `positionedTools` is the list holding the kept tool references when `nextTools` clones them. + */ + const setToolsWithReindexedModes = useCallback( + (nextTools: StoredTool[], positionedTools: StoredTool[] = nextTools) => { + const canonicalModes = reindexToolCanonicalModes( + selectedTools, + positionedTools, + canonicalModeOverrides + ) + if (!canonicalModes) { + setStoreValue(nextTools) + return + } + collaborativeSetSubblockValueWithCanonicalModes( + blockId, + subBlockId, + structuredClone(nextTools), + canonicalModes + ) + }, + [ + selectedTools, + canonicalModeOverrides, + setStoreValue, + collaborativeSetSubblockValueWithCanonicalModes, + blockId, + subBlockId, + ] + ) + // Tool categories the consuming block can't run (declared on its tool-input // subBlock): shown in the picker but greyed out with a tooltip instead of added. const blockType = useWorkflowStore(useCallback((state) => state.blocks[blockId]?.type, [blockId])) @@ -857,10 +883,9 @@ export const ToolInput = memo(function ToolInput({ (toolIndex: number) => { if (isPreview || disabled) return const updatedTools = selectedTools.filter((_, index) => index !== toolIndex) - reindexCanonicalModesOnMutate(selectedTools, updatedTools) - setStoreValue(updatedTools) + setToolsWithReindexedModes(updatedTools) }, - [isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue] + [isPreview, disabled, selectedTools, setToolsWithReindexedModes] ) const handleRemoveAllFromServer = useCallback( @@ -869,10 +894,9 @@ export const ToolInput = memo(function ToolInput({ const updatedTools = selectedTools.filter( (t) => !(t.type === 'mcp' && t.params?.serverId === serverId) ) - reindexCanonicalModesOnMutate(selectedTools, updatedTools) - setStoreValue(updatedTools) + setToolsWithReindexedModes(updatedTools) }, - [isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue] + [isPreview, disabled, selectedTools, setToolsWithReindexedModes] ) const handleDeleteTool = useCallback( @@ -900,11 +924,10 @@ export const ToolInput = memo(function ToolInput({ }) if (updatedTools.length !== selectedTools.length) { - reindexCanonicalModesOnMutate(selectedTools, updatedTools) - setStoreValue(updatedTools) + setToolsWithReindexedModes(updatedTools) } }, - [selectedTools, customTools, reindexCanonicalModesOnMutate, setStoreValue] + [selectedTools, customTools, setToolsWithReindexedModes] ) const handleParamChange = useCallback( @@ -1077,8 +1100,7 @@ export const ToolInput = memo(function ToolInput({ newTools.splice(adjustedDropIndex, 0, draggedTool) } - reindexCanonicalModesOnMutate(selectedTools, newTools) - setStoreValue(newTools) + setToolsWithReindexedModes(newTools) setDraggedIndex(null) setDragOverIndex(null) } @@ -1177,8 +1199,7 @@ export const ToolInput = memo(function ToolInput({ ...filteredTools.map((tool) => ({ ...tool, isExpanded: false })), serverBinding, ] - reindexCanonicalModesOnMutate(selectedTools, filteredTools) - setStoreValue(nextTools) + setToolsWithReindexedModes(nextTools, filteredTools) setMcpServerDrilldown(null) setOpen(false) }, @@ -1445,7 +1466,7 @@ export const ToolInput = memo(function ToolInput({ supportsAdvancedMcpServer, availableWorkflows, isToolAlreadySelected, - reindexCanonicalModesOnMutate, + setToolsWithReindexedModes, ]) return ( diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index eb70e2f9089..85a63969fe1 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -59,6 +59,18 @@ import { findAllDescendantNodes, isBlockProtected } from '@/stores/workflows/wor const logger = createLogger('CollaborativeWorkflow') +/** Applies a subblock value and its block's replacement `canonicalModes` to the local stores. */ +function applySubblockValueWithCanonicalModes( + blockId: string, + subblockId: string, + value: unknown, + canonicalModes: Record +) { + useSubBlockStore.getState().setValue(blockId, subblockId, value) + useWorkflowStore.getState().syncDynamicHandleSubblockValue(blockId, subblockId, value) + useWorkflowStore.getState().setBlockCanonicalModes(blockId, canonicalModes) +} + export function useCollaborativeWorkflow() { const queryClient = useQueryClient() const undoRedo = useUndoRedo() @@ -259,6 +271,11 @@ export function useCollaborativeWorkflow() { } } else if (target === OPERATION_TARGETS.SUBBLOCK) { switch (operation) { + case SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES: { + const { blockId, subblockId, value, canonicalModes } = payload + applySubblockValueWithCanonicalModes(blockId, subblockId, value, canonicalModes) + break + } case SUBBLOCK_OPERATIONS.BATCH_UPDATE: { const { updates } = payload if (Array.isArray(updates)) { @@ -1362,39 +1379,6 @@ export function useCollaborativeWorkflow() { [isBaselineDiffView, activeWorkflowId, addToQueue, session?.user?.id] ) - /** - * Wholesale-replaces `block.data.canonicalModes`, rather than merging one key like - * {@link collaborativeSetBlockCanonicalMode}. Needed to reindex nested tool-input overrides on - * reorder/removal: a merge can't atomically drop a now-stale index key, and sequential - * per-key sets can clobber each other when two tools swap positions. - */ - const collaborativeSetBlockCanonicalModes = useCallback( - (id: string, canonicalModes: Record) => { - if (isBaselineDiffView) { - return - } - - useWorkflowStore.getState().setBlockCanonicalModes(id, canonicalModes) - - if (!activeWorkflowId) { - return - } - - const operationId = generateId() - addToQueue({ - id: operationId, - operation: { - operation: BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES, - target: OPERATION_TARGETS.BLOCK, - payload: { id, data: { canonicalModes } }, - }, - workflowId: activeWorkflowId, - userId: session?.user?.id || 'unknown', - }) - }, - [isBaselineDiffView, activeWorkflowId, addToQueue, session?.user?.id] - ) - const collaborativeBatchToggleBlockHandles = useCallback( (ids: string[]) => { if (isBaselineDiffView) { @@ -1649,6 +1633,43 @@ export function useCollaborativeWorkflow() { [activeWorkflowId, addToQueue, session?.user?.id, isBaselineDiffView] ) + /** + * Sets a subblock value and wholesale-replaces its block's `canonicalModes` as ONE persisted + * operation, so a `tool-input` reorder or removal can never save its list without the modes + * keyed to its positions. + */ + const collaborativeSetSubblockValueWithCanonicalModes = useCallback( + ( + blockId: string, + subblockId: string, + value: unknown, + canonicalModes: Record + ) => { + if (isApplyingRemoteChange.current) return + + if (isBaselineDiffView) { + logger.debug('Skipping collaborative subblock update while viewing baseline diff') + return + } + + applySubblockValueWithCanonicalModes(blockId, subblockId, value, canonicalModes) + + if (!activeWorkflowId) return + + addToQueue({ + id: generateId(), + operation: { + operation: SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES, + target: OPERATION_TARGETS.SUBBLOCK, + payload: { blockId, subblockId, value, canonicalModes }, + }, + workflowId: activeWorkflowId, + userId: session?.user?.id || 'unknown', + }) + }, + [activeWorkflowId, addToQueue, session?.user?.id, isBaselineDiffView] + ) + const collaborativeBatchSetSubblockValues = useCallback( ( updates: Array<{ @@ -2292,7 +2313,6 @@ export function useCollaborativeWorkflow() { collaborativeSetBlockErrorEnabled, collaborativeSetBlockRetry, collaborativeSetBlockCanonicalMode, - collaborativeSetBlockCanonicalModes, collaborativeBatchToggleBlockHandles, collaborativeBatchToggleLocked, collaborativeBatchAddBlocks, @@ -2300,6 +2320,7 @@ export function useCollaborativeWorkflow() { collaborativeBatchAddEdges, collaborativeBatchRemoveEdges, collaborativeSetSubblockValue, + collaborativeSetSubblockValueWithCanonicalModes, collaborativeBatchSetSubblockValues, collaborativeSetTagSelection, diff --git a/apps/sim/lib/workflows/editing/engine.ts b/apps/sim/lib/workflows/editing/engine.ts index ba98f968ac4..05769ff95f0 100644 --- a/apps/sim/lib/workflows/editing/engine.ts +++ b/apps/sim/lib/workflows/editing/engine.ts @@ -1,6 +1,11 @@ import { createLogger } from '@sim/logger' +import type { BlockState } from '@sim/workflow-types/workflow' +import { isEqual } from 'es-toolkit' import type { PermissionGroupConfig } from '@/lib/permission-groups/fields' +import { coerceObjectArray } from '@/lib/workflows/persistence/remap-internal-ids' import { isValidKey } from '@/lib/workflows/sanitization/key-validation' +import { reindexRewrittenToolCanonicalModes } from '@/lib/workflows/subblocks/visibility' +import { getBlock } from '@/blocks/registry' import { validateEdges } from '@/stores/workflows/workflow/edge-validation' import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' import { @@ -256,6 +261,8 @@ export function applyOperationsToWorkflowState( // blocks that are both being moved into the same subflow in one batch. removeInvalidScopeEdges(modifiedState, skippedItems) + reindexToolCanonicalModesAfterEdits((workflowState as any).blocks, (modifiedState as any).blocks) + // Regenerate loops and parallels after modifications ;(modifiedState as any).loops = generateLoopBlocks((modifiedState as any).blocks) ;(modifiedState as any).parallels = generateParallelBlocks((modifiedState as any).blocks) @@ -295,6 +302,37 @@ export function applyOperationsToWorkflowState( } } +/** + * Nested tool canonical-mode overrides are keyed by the tool's position in its `tool-input` + * array, so when this batch rewrote a surviving block's tool list, carry each tool's overrides to + * wherever that tool now sits and drop those of removed tools, as the editor does on reorder and + * removal. Compares the original and final lists so any number of edits in the batch compose. + */ +function reindexToolCanonicalModesAfterEdits( + originalBlocks: Record | undefined, + blocks: Record | undefined +): void { + for (const [blockId, block] of Object.entries(blocks ?? {})) { + const originalBlock = originalBlocks?.[blockId] + if (!originalBlock || originalBlock.type !== block.type || !block.data?.canonicalModes) continue + + for (const subBlock of getBlock(block.type)?.subBlocks ?? []) { + if (subBlock.type !== 'tool-input') continue + const originalTools = coerceObjectArray(originalBlock.subBlocks?.[subBlock.id]?.value).array + if (!originalTools) continue + const tools = coerceObjectArray(block.subBlocks?.[subBlock.id]?.value).array ?? [] + if (isEqual(originalTools, tools)) continue + + const canonicalModes = reindexRewrittenToolCanonicalModes( + originalTools, + tools, + block.data.canonicalModes + ) + if (canonicalModes) block.data = { ...block.data, canonicalModes } + } + } +} + /** * Resolves pending forward-reference connections recorded on block.data. * diff --git a/apps/sim/lib/workflows/editing/operations.test.ts b/apps/sim/lib/workflows/editing/operations.test.ts index c9f8511bc60..bf8d90869a0 100644 --- a/apps/sim/lib/workflows/editing/operations.test.ts +++ b/apps/sim/lib/workflows/editing/operations.test.ts @@ -1169,3 +1169,85 @@ describe('permission-group tool access', () => { ) }) }) + +describe('tool canonical-mode reindexing', () => { + const selectorTool = { + type: 'jira', + operation: 'jira_get_issue', + title: 'Selector', + params: { projectId: 'PROJ' }, + usageControl: 'auto', + isExpanded: false, + } + const variableTool = { + type: 'jira', + operation: 'jira_get_issue', + title: 'Variable', + params: { manualProjectId: '{{PROJECT}}' }, + usageControl: 'auto', + isExpanded: false, + } + + function agentWithTools(tools: unknown[], canonicalModes: Record) { + return { + blocks: { + agent: { + id: 'agent', + type: 'agent', + name: 'Agent', + position: { x: 0, y: 0 }, + enabled: true, + outputs: {}, + subBlocks: { tools: { id: 'tools', type: 'tool-input', value: tools } }, + data: { canonicalModes }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } + } + + function editTools(workflow: Record, tools: unknown[]) { + const { state } = applyOperationsToWorkflowState(workflow, [ + { operation_type: 'edit', block_id: 'agent', params: { inputs: { tools } } }, + ]) + return (state as any).blocks.agent.data.canonicalModes + } + + it('moves each tool mode with it when the edit reorders the tools', () => { + const workflow = agentWithTools([selectorTool, variableTool], { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }) + + expect(editTools(workflow, [variableTool, selectorTool])).toEqual({ + '0:projectId': 'advanced', + '1:projectId': 'basic', + }) + }) + + it('keeps modes in place when an edit changes tool params without moving them', () => { + const workflow = agentWithTools([selectorTool, variableTool], { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }) + + expect( + editTools(workflow, [ + { ...selectorTool, params: { projectId: 'OTHER' } }, + { ...variableTool, params: { manualProjectId: '{{OTHER}}' } }, + ]) + ).toEqual({ '0:projectId': 'basic', '1:projectId': 'advanced' }) + }) + + it('drops a removed tool mode so a later tool cannot inherit its position', () => { + const workflow = agentWithTools([selectorTool, variableTool], { + '0:projectId': 'basic', + '1:projectId': 'advanced', + }) + + expect(editTools(workflow, [selectorTool])).toEqual({ '0:projectId': 'basic' }) + expect(editTools(workflow, [])).toEqual({}) + }) +}) diff --git a/apps/sim/lib/workflows/subblocks/visibility.test.ts b/apps/sim/lib/workflows/subblocks/visibility.test.ts index fa9d2d7eaba..daa2860f3cb 100644 --- a/apps/sim/lib/workflows/subblocks/visibility.test.ts +++ b/apps/sim/lib/workflows/subblocks/visibility.test.ts @@ -7,6 +7,7 @@ import { buildCanonicalIndexForSurface, evaluateSubBlockCondition, getCanonicalSubBlocksForSurface, + reindexRewrittenToolCanonicalModes, reindexToolCanonicalModes, resolveActiveDependencyValue, resolveDependencyValue, @@ -347,6 +348,100 @@ describe('reindexToolCanonicalModes', () => { }) }) +describe('reindexRewrittenToolCanonicalModes', () => { + /** Serialized tools: each call builds fresh objects, so object identity never matches. */ + const jira = (projectId: string, extra: Record = {}) => ({ + type: 'jira', + operation: 'jira_get_issue', + params: { projectId }, + ...extra, + }) + const gmail = () => ({ type: 'gmail', operation: 'gmail_send', params: {} }) + + it.concurrent('returns undefined when there are no overrides', () => { + expect(reindexRewrittenToolCanonicalModes([jira('A')], [jira('B')], undefined)).toBeUndefined() + }) + + it.concurrent('moves each tool overrides with it when serialized tools are reordered', () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A'), jira('B')], + [jira('B'), jira('A')], + { '0:projectId': 'basic', '1:projectId': 'advanced' } + ) + expect(result).toEqual({ '0:projectId': 'advanced', '1:projectId': 'basic' }) + }) + + it.concurrent('ignores isExpanded, which serialized input may omit or default', () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A', { isExpanded: false }), jira('B', { isExpanded: true })], + [jira('B'), jira('A', { isExpanded: true })], + { '1:projectId': 'advanced' } + ) + expect(result).toEqual({ '0:projectId': 'advanced' }) + }) + + it.concurrent('keeps overrides in place when a tool params are edited without moving', () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A'), jira('B')], + [jira('A-edited'), jira('B-edited')], + { '0:projectId': 'basic', '1:projectId': 'advanced' } + ) + expect(result).toBeUndefined() + }) + + it.concurrent( + 'lets an edited tool claim its old slot after an unchanged tool moves past it', + () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A'), gmail(), jira('B')], + [gmail(), jira('B'), jira('A-edited')], + { '0:projectId': 'advanced', '2:projectId': 'basic' } + ) + expect(result).toEqual({ '1:projectId': 'basic', '2:projectId': 'advanced' }) + } + ) + + it.concurrent('drops a removed tool overrides and shifts the survivors', () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A'), jira('B'), jira('C')], + [jira('B'), jira('C')], + { '0:projectId': 'advanced', '1:projectId': 'basic', '2:projectId': 'advanced' } + ) + expect(result).toEqual({ '0:projectId': 'basic', '1:projectId': 'advanced' }) + }) + + it.concurrent('gives a new tool no overrides even when it lands on a used position', () => { + const result = reindexRewrittenToolCanonicalModes([jira('A')], [gmail(), jira('A')], { + '0:projectId': 'advanced', + }) + expect(result).toEqual({ '1:projectId': 'advanced' }) + }) + + it.concurrent('does not hand a replaced tool overrides to a tool of another type', () => { + const result = reindexRewrittenToolCanonicalModes([jira('A')], [gmail()], { + '0:projectId': 'advanced', + }) + expect(result).toEqual({}) + }) + + it.concurrent('keeps identical duplicates on their own positions', () => { + const result = reindexRewrittenToolCanonicalModes( + [jira('A'), jira('A')], + [jira('A'), jira('A'), gmail()], + { '0:projectId': 'basic', '1:projectId': 'advanced' } + ) + expect(result).toBeUndefined() + }) + + it.concurrent('carries a legacy (non-index-scoped) key through unchanged', () => { + const result = reindexRewrittenToolCanonicalModes([jira('A'), jira('B')], [jira('B')], { + '0:projectId': 'advanced', + 'jira:projectId': 'basic', + }) + expect(result).toEqual({ 'jira:projectId': 'basic' }) + }) +}) + describe('canonical index scoping by surface', () => { /** Webflow's shape: an action pair and a trigger alias sharing one `canonicalParamId`. */ const MIXED: SubBlockConfig[] = [ diff --git a/apps/sim/lib/workflows/subblocks/visibility.ts b/apps/sim/lib/workflows/subblocks/visibility.ts index 2e9ed82e8ff..15b797fe72f 100644 --- a/apps/sim/lib/workflows/subblocks/visibility.ts +++ b/apps/sim/lib/workflows/subblocks/visibility.ts @@ -1,3 +1,5 @@ +import { isRecordLike, omit } from '@sim/utils/object' +import { isEqual } from 'es-toolkit' import { getDeploymentShape } from '@/lib/core/config/deployment-shape' import { getEnv, isTruthy } from '@/lib/core/config/env' import type { SubBlockConfig } from '@/blocks/types' @@ -373,11 +375,11 @@ const INDEX_SCOPED_KEY = /^(\d+):(.+)$/ /** * Canonical-mode overrides are keyed by a tool's position in its `tool-input` array * (`${toolIndex}:${canonicalId}`), so anything that reorders or removes tools - the editor - * (drag-reorder, remove, delete), fork/promote copy (dropping an unresolved custom-tool/MCP - * entry) - must carry each surviving tool's overrides to its new position and DROP the - * vacated index. Otherwise a saved basic/advanced choice can attach to whichever DIFFERENT - * tool later lands on that old index (e.g. a newly-added tool, always appended at the end, - * can refill a slot a removal just freed). + * (drag-reorder, remove, delete), the workflow edit engine (a rewritten tool list), fork/promote + * copy (dropping an unresolved custom-tool/MCP entry) - must carry each surviving tool's + * overrides to its new position and DROP the vacated index. Otherwise a saved basic/advanced + * choice can attach to whichever DIFFERENT tool later lands on that old index (e.g. a newly-added + * tool, always appended at the end, can refill a slot a removal just freed). * * Returns the full replacement `canonicalModes` object (for an atomic whole-map write - a * per-key merge can't drop a key, and sequential per-key writes can clobber each other when @@ -435,6 +437,62 @@ export function reindexToolCanonicalModes( return reindexCanonicalModesByPosition(newIndexByOldIndex, overrides) } +type ToolMatcher = (oldTool: unknown, newTool: unknown) => boolean + +const isSameToolType: ToolMatcher = (oldTool, newTool) => + isRecordLike(oldTool) && + isRecordLike(newTool) && + typeof oldTool.type === 'string' && + oldTool.type === newTool.type + +/** Drops `isExpanded`, editor UI state that serialized input may omit or default. */ +function withoutExpandedState(tool: unknown): unknown { + return isRecordLike(tool) ? omit(tool, ['isExpanded']) : tool +} + +/** + * {@link reindexCanonicalModesByPosition} for a tool array rewritten from serialized input (the + * workflow edit API and Chat), where object identity is gone. Each rewritten tool claims an + * unclaimed old tool with the same content, then any tool still unmatched claims an unclaimed + * old tool of the same `type`, so a tool whose params were edited in place keeps its modes. Each + * pass tries the same position before any other, so unmoved tools and identical duplicates keep + * their own overrides. A tool left unmatched is new, and an old tool left unmatched was removed. + */ +export function reindexRewrittenToolCanonicalModes( + oldTools: readonly unknown[], + newTools: readonly unknown[], + overrides: CanonicalModeOverrides | undefined +): Record | undefined { + if (!overrides) return undefined + + const oldContents = oldTools.map(withoutExpandedState) + const newContents = newTools.map(withoutExpandedState) + const newIndexByOldIndex = new Map() + const matchedNewIndices = new Set() + + for (const matches of [isEqual, isSameToolType] as ToolMatcher[]) { + const isUnclaimedMatch = (oldIndex: number, newIndex: number) => + oldIndex < oldContents.length && + !newIndexByOldIndex.has(oldIndex) && + matches(oldContents[oldIndex], newContents[newIndex]) + const findSamePosition = (newIndex: number) => + isUnclaimedMatch(newIndex, newIndex) ? newIndex : -1 + const findAnyPosition = (newIndex: number) => + oldContents.findIndex((_, oldIndex) => isUnclaimedMatch(oldIndex, newIndex)) + + for (const findOldIndex of [findSamePosition, findAnyPosition]) { + newContents.forEach((_, newIndex) => { + if (matchedNewIndices.has(newIndex)) return + const oldIndex = findOldIndex(newIndex) + if (oldIndex === -1) return + newIndexByOldIndex.set(oldIndex, newIndex) + matchedNewIndices.add(newIndex) + }) + } + } + return reindexCanonicalModesByPosition(newIndexByOldIndex, overrides) +} + /** * True for the modes that make a field advanced-only when it is not part of a * canonical basic/advanced pair: a standalone `advanced` field, or a standalone diff --git a/apps/sim/stores/operation-queue/store.test.ts b/apps/sim/stores/operation-queue/store.test.ts index 33cb8e0eb11..5c12081780e 100644 --- a/apps/sim/stores/operation-queue/store.test.ts +++ b/apps/sim/stores/operation-queue/store.test.ts @@ -266,6 +266,51 @@ describe('operation queue room gating', () => { ]) }) + it('supersedes pending tool updates with the latest value and canonical modes together', () => { + const queue = useOperationQueueStore.getState() + const toolsUpdate = (id: string, operation: string, value: string[]) => + queue.addToQueue({ + id, + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation, + target: 'subblock', + payload: { + blockId: 'agent-1', + subblockId: 'tools', + value, + ...(operation === 'subblock-update-with-canonical-modes' && { + canonicalModes: { [`${value.indexOf('a')}:projectId`]: 'advanced' }, + }), + }, + }, + }) + + toolsUpdate('op-1', 'subblock-update', ['a', 'b']) + toolsUpdate('op-2', 'subblock-update-with-canonical-modes', ['b', 'a']) + toolsUpdate('op-3', 'subblock-update-with-canonical-modes', ['a', 'b']) + + expect(useOperationQueueStore.getState().operations).toEqual([ + expect.objectContaining({ + id: 'op-3', + operation: expect.objectContaining({ + payload: expect.objectContaining({ + value: ['a', 'b'], + canonicalModes: { '0:projectId': 'advanced' }, + }), + }), + }), + ]) + + toolsUpdate('op-4', 'subblock-update', ['a', 'b', 'c']) + + expect(useOperationQueueStore.getState().operations.map((op) => op.id)).toEqual([ + 'op-3', + 'op-4', + ]) + }) + it('does not coalesce matching subblock updates across workflows', () => { useOperationQueueStore.getState().addToQueue({ id: 'op-1', diff --git a/apps/sim/stores/operation-queue/store.ts b/apps/sim/stores/operation-queue/store.ts index e1655007aba..502435e1581 100644 --- a/apps/sim/stores/operation-queue/store.ts +++ b/apps/sim/stores/operation-queue/store.ts @@ -77,6 +77,18 @@ export function registerEmitFunctions( let currentRegisteredWorkflowId: string | null = null +/** + * Pending subblock operations a newer one for the same field makes redundant. A value-only update + * never supersedes one that also carries canonical modes, or those modes would be lost. + */ +const SUPERSEDED_SUBBLOCK_OPERATIONS: Partial> = { + 'subblock-update': ['subblock-update'], + 'subblock-update-with-canonical-modes': [ + 'subblock-update', + 'subblock-update-with-canonical-modes', + ], +} + /** Targets whose payload id refers to a canvas block (subflow ids are loop/parallel blocks). */ const BLOCK_SCOPED_TARGETS = ['block', 'subblock', 'subflow'] @@ -150,15 +162,13 @@ export const useOperationQueueStore = create((set, get) => let shouldDropPendingOperation = (_op: QueuedOperation) => false - if ( - operation.operation.operation === 'subblock-update' && - operation.operation.target === 'subblock' - ) { + const supersededOperations = SUPERSEDED_SUBBLOCK_OPERATIONS[operation.operation.operation] + if (supersededOperations && operation.operation.target === 'subblock') { const { blockId, subblockId } = operation.operation.payload shouldDropPendingOperation = (op) => op.status === 'pending' && op.workflowId === operation.workflowId && - op.operation.operation === 'subblock-update' && + supersededOperations.includes(op.operation.operation) && op.operation.target === 'subblock' && op.operation.payload?.blockId === blockId && op.operation.payload?.subblockId === subblockId diff --git a/packages/realtime-protocol/src/constants.ts b/packages/realtime-protocol/src/constants.ts index fe6c2c87bf6..1bae03b498c 100644 --- a/packages/realtime-protocol/src/constants.ts +++ b/packages/realtime-protocol/src/constants.ts @@ -7,6 +7,11 @@ export const BLOCK_OPERATIONS = { UPDATE_ERROR_ENABLED: 'update-error-enabled', UPDATE_RETRY: 'update-retry', UPDATE_CANONICAL_MODE: 'update-canonical-mode', + /** + * No longer sent by the editor, which persists tool reindexing with + * `SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES`. Still accepted and applied so tabs loaded + * before that change keep saving and syncing across a rolling deploy. + */ REPLACE_CANONICAL_MODES: 'replace-canonical-modes', TOGGLE_HANDLES: 'toggle-handles', } as const @@ -62,6 +67,7 @@ export type WorkflowOperation = (typeof WORKFLOW_OPERATIONS)[keyof typeof WORKFL export const SUBBLOCK_OPERATIONS = { UPDATE: 'subblock-update', BATCH_UPDATE: 'subblock-batch-update', + UPDATE_WITH_CANONICAL_MODES: 'subblock-update-with-canonical-modes', } as const export type SubblockOperation = (typeof SUBBLOCK_OPERATIONS)[keyof typeof SUBBLOCK_OPERATIONS] diff --git a/packages/realtime-protocol/src/schemas.ts b/packages/realtime-protocol/src/schemas.ts index dd15b86af62..5de6d60397a 100644 --- a/packages/realtime-protocol/src/schemas.ts +++ b/packages/realtime-protocol/src/schemas.ts @@ -36,6 +36,8 @@ const AutoConnectEdgeSchema = z.object({ type: z.string().optional(), }) +const CanonicalModeSchema = z.enum(['basic', 'advanced']) + export const BlockOperationSchema = z.object({ operation: z.enum([ BLOCK_OPERATIONS.UPDATE_POSITION, @@ -67,7 +69,7 @@ export const BlockOperationSchema = z.object({ retry: BlockRetrySchema.optional(), horizontalHandles: z.boolean().optional(), canonicalId: z.string().optional(), - canonicalMode: z.enum(['basic', 'advanced']).optional(), + canonicalMode: CanonicalModeSchema.optional(), triggerMode: z.boolean().optional(), height: z.number().optional(), }), @@ -168,6 +170,24 @@ export const SubblockOperationSchema = z.object({ operationId: z.string().optional(), }) +/** + * Writes one subblock value and replaces its block's `canonicalModes` in the same transaction. + * A `tool-input` keys its tools' modes by array position, so a reorder or removal must persist + * both together or a failure between two separate writes leaves modes on the wrong tools. + */ +export const SubblockCanonicalModesUpdateSchema = z.object({ + operation: z.literal(SUBBLOCK_OPERATIONS.UPDATE_WITH_CANONICAL_MODES), + target: z.literal(OPERATION_TARGETS.SUBBLOCK), + payload: z.object({ + blockId: z.string(), + subblockId: z.string(), + value: z.any(), + canonicalModes: z.record(z.string(), CanonicalModeSchema), + }), + timestamp: z.number(), + operationId: z.string().optional(), +}) + export const BatchAddBlocksSchema = z.object({ operation: z.literal(BLOCKS_OPERATIONS.BATCH_ADD_BLOCKS), target: z.literal(OPERATION_TARGETS.BLOCKS), @@ -285,4 +305,5 @@ export const WorkflowOperationSchema = z.union([ VariableOperationSchema, WorkflowStateOperationSchema, SubblockOperationSchema, + SubblockCanonicalModesUpdateSchema, ]) From 112fa04e7371431fc59579e3112cf7471109a120 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:33:43 -0700 Subject: [PATCH 2/4] fix(realtime): reject locked-block tool updates and tighten new types --- apps/realtime/src/database/operations.test.ts | 4 ++-- apps/realtime/src/database/operations.ts | 11 ++++++----- apps/sim/lib/workflows/editing/engine.ts | 5 ++++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/realtime/src/database/operations.test.ts b/apps/realtime/src/database/operations.test.ts index 1f1c5585842..fb81bbd304d 100644 --- a/apps/realtime/src/database/operations.test.ts +++ b/apps/realtime/src/database/operations.test.ts @@ -164,8 +164,8 @@ describe('subblock update with canonical modes persistence', () => { ) }) - it('skips a locked block without writing either field', async () => { - await expect(updateTools({ locked: true })).resolves.toBeUndefined() + it('rejects a locked block without writing either field', async () => { + await expect(updateTools({ locked: true })).rejects.toThrow('is locked') expect(mockSet).toHaveBeenCalledTimes(1) }) }) diff --git a/apps/realtime/src/database/operations.ts b/apps/realtime/src/database/operations.ts index 84b8722177b..0d0097a50bb 100644 --- a/apps/realtime/src/database/operations.ts +++ b/apps/realtime/src/database/operations.ts @@ -1998,10 +1998,10 @@ interface SubblockUpdateBlockRecord { /** Every block in the workflow by id, for the locked-container check subblock writes need. */ async function loadSubblockUpdateBlocks( - tx: any, + tx: Pick, workflowId: string ): Promise> { - const allBlocks: SubblockUpdateBlockRecord[] = await tx + const allBlocks = await tx .select({ id: workflowBlocks.id, subBlocks: workflowBlocks.subBlocks, @@ -2083,11 +2083,12 @@ async function handleSubblockOperationTx( throw new Error(`Block ${blockId} not found`) } if (isWorkflowBlockProtected(blockId, blocksById)) { - logger.info(`Skipping subblock update of locked block ${blockId}`) - break + throw new Error(`Block ${blockId} is locked or inside a locked container`) } - const subBlocks = { ...((block.subBlocks as Record) || {}) } + const subBlocks = { + ...((block.subBlocks as Record> | null) || {}), + } const currentSubBlock = subBlocks[subblockId] subBlocks[subblockId] = currentSubBlock ? { ...currentSubBlock, value } diff --git a/apps/sim/lib/workflows/editing/engine.ts b/apps/sim/lib/workflows/editing/engine.ts index 05769ff95f0..5624d684883 100644 --- a/apps/sim/lib/workflows/editing/engine.ts +++ b/apps/sim/lib/workflows/editing/engine.ts @@ -261,7 +261,10 @@ export function applyOperationsToWorkflowState( // blocks that are both being moved into the same subflow in one batch. removeInvalidScopeEdges(modifiedState, skippedItems) - reindexToolCanonicalModesAfterEdits((workflowState as any).blocks, (modifiedState as any).blocks) + reindexToolCanonicalModesAfterEdits( + workflowState.blocks as Record | undefined, + modifiedState.blocks as Record | undefined + ) // Regenerate loops and parallels after modifications ;(modifiedState as any).loops = generateLoopBlocks((modifiedState as any).blocks) From 34fdb94aaeb6ef266c199cd25fca988ec9cd0c95 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 12 Sep 2026 20:38:31 -0700 Subject: [PATCH 3/4] test(agent): drop redundant cast in tool mode reindex tests --- apps/sim/lib/workflows/editing/operations.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/lib/workflows/editing/operations.test.ts b/apps/sim/lib/workflows/editing/operations.test.ts index bf8d90869a0..8e6286cc79e 100644 --- a/apps/sim/lib/workflows/editing/operations.test.ts +++ b/apps/sim/lib/workflows/editing/operations.test.ts @@ -1212,7 +1212,7 @@ describe('tool canonical-mode reindexing', () => { const { state } = applyOperationsToWorkflowState(workflow, [ { operation_type: 'edit', block_id: 'agent', params: { inputs: { tools } } }, ]) - return (state as any).blocks.agent.data.canonicalModes + return state.blocks.agent.data.canonicalModes } it('moves each tool mode with it when the edit reorders the tools', () => { From b46daaef67238e7898f9b13035148db9ba412a48 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 13 Sep 2026 00:27:00 -0700 Subject: [PATCH 4/4] improvement(realtime): share the writable-block check across subblock writes --- apps/realtime/src/database/operations.ts | 48 +++++++++++------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/apps/realtime/src/database/operations.ts b/apps/realtime/src/database/operations.ts index 0d0097a50bb..cd4316448bf 100644 --- a/apps/realtime/src/database/operations.ts +++ b/apps/realtime/src/database/operations.ts @@ -26,6 +26,7 @@ import { import { randomFloat } from '@sim/utils/random' import { loadWorkflowFromNormalizedTablesRaw } from '@sim/workflow-persistence/load' import { mergeSubBlockValues } from '@sim/workflow-persistence/subblocks' +import type { DbOrTx } from '@sim/workflow-persistence/types' import { filterAcyclicEdges, filterUniqueWorkflowEdges, @@ -1989,18 +1990,8 @@ async function handleSubflowOperationTx( } } -interface SubblockUpdateBlockRecord { - id: string - subBlocks: unknown - locked: boolean - data: unknown -} - /** Every block in the workflow by id, for the locked-container check subblock writes need. */ -async function loadSubblockUpdateBlocks( - tx: Pick, - workflowId: string -): Promise> { +async function loadSubblockUpdateBlocks(tx: DbOrTx, workflowId: string) { const allBlocks = await tx .select({ id: workflowBlocks.id, @@ -2013,6 +2004,24 @@ async function loadSubblockUpdateBlocks( return Object.fromEntries(allBlocks.map((block) => [block.id, block])) } +/** + * The block a subblock write targets, rejecting one that is missing, locked, or in a locked + * container. + */ +function getWritableSubblockUpdateBlock( + blocksById: Awaited>, + blockId: string +) { + const block = blocksById[blockId] + if (!block) { + throw new Error(`Block ${blockId} not found`) + } + if (isWorkflowBlockProtected(blockId, blocksById)) { + throw new Error(`Block ${blockId} is locked or inside a locked container`) + } + return block +} + // Subblock operations - targeted value updates without replacing workflow state async function handleSubblockOperationTx( tx: any, @@ -2035,14 +2044,7 @@ async function handleSubblockOperationTx( throw new Error('Missing required fields for subblock batch update') } - const block = blocksById[blockId] - if (!block) { - throw new Error(`Block ${blockId} not found`) - } - - if (isWorkflowBlockProtected(blockId, blocksById)) { - throw new Error(`Block ${blockId} is locked or inside a locked container`) - } + const block = getWritableSubblockUpdateBlock(blocksById, blockId) const subBlocks = { ...((block.subBlocks as Record) || {}) } const currentSubBlock = subBlocks[subblockId] @@ -2078,13 +2080,7 @@ async function handleSubblockOperationTx( } const blocksById = await loadSubblockUpdateBlocks(tx, workflowId) - const block = blocksById[blockId] - if (!block) { - throw new Error(`Block ${blockId} not found`) - } - if (isWorkflowBlockProtected(blockId, blocksById)) { - throw new Error(`Block ${blockId} is locked or inside a locked container`) - } + const block = getWritableSubblockUpdateBlock(blocksById, blockId) const subBlocks = { ...((block.subBlocks as Record> | null) || {}),