Skip to content

Commit b117758

Browse files
waleedlatif1waleedlatif1
authored
fix(agent): scope nested tool canonical-mode overrides by instance, not type (#5534)
Two tool entries of the same type inside an Agent block's tool-input array (e.g. two Table tools) shared a single canonical-mode override keyed by ${toolType}:${canonicalId}, so switching basic/advanced mode on one field silently switched it on every other instance of the same tool type - including at execution time, where the wrong basic/advanced value could be resolved for the second tool. Rescope the override key to the tool's position in its tool-input array (${toolIndex}:${canonicalId}) instead of its type, and thread that index through every consumer: the editor (read + write), execution (agent-handler/providers), search-index, and fork/promote remapping.
1 parent b09e0c0 commit b117758

24 files changed

Lines changed: 872 additions & 129 deletions

File tree

‎apps/realtime/src/database/operations.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,39 @@ async function handleBlockOperationTx(
576576
break
577577
}
578578

579+
case BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES: {
580+
if (!payload.id || !payload.data?.canonicalModes) {
581+
throw new Error('Missing required fields for replace canonical modes operation')
582+
}
583+
584+
const existingBlock = await tx
585+
.select({ data: workflowBlocks.data })
586+
.from(workflowBlocks)
587+
.where(and(eq(workflowBlocks.id, payload.id), eq(workflowBlocks.workflowId, workflowId)))
588+
.limit(1)
589+
590+
const currentData = (existingBlock?.[0]?.data as Record<string, unknown>) || {}
591+
592+
const updateResult = await tx
593+
.update(workflowBlocks)
594+
.set({
595+
data: {
596+
...currentData,
597+
canonicalModes: payload.data.canonicalModes,
598+
},
599+
updatedAt: new Date(),
600+
})
601+
.where(and(eq(workflowBlocks.id, payload.id), eq(workflowBlocks.workflowId, workflowId)))
602+
.returning({ id: workflowBlocks.id })
603+
604+
if (updateResult.length === 0) {
605+
throw new Error(`Block ${payload.id} not found in workflow ${workflowId}`)
606+
}
607+
608+
logger.debug(`Replaced block canonical modes: ${payload.id}`)
609+
break
610+
}
611+
579612
case BLOCK_OPERATIONS.TOGGLE_HANDLES: {
580613
if (!payload.id || payload.horizontalHandles === undefined) {
581614
throw new Error('Missing required fields for toggle handles operation')

‎apps/realtime/src/middleware/permissions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const WRITE_OPERATIONS: string[] = [
2828
BLOCK_OPERATIONS.UPDATE_PARENT,
2929
BLOCK_OPERATIONS.UPDATE_ADVANCED_MODE,
3030
BLOCK_OPERATIONS.UPDATE_CANONICAL_MODE,
31+
BLOCK_OPERATIONS.REPLACE_CANONICAL_MODES,
3132
BLOCK_OPERATIONS.TOGGLE_HANDLES,
3233
// Batch block operations
3334
BLOCKS_OPERATIONS.BATCH_UPDATE_POSITIONS,

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ import {
106106
type CanonicalModeOverrides,
107107
evaluateSubBlockCondition,
108108
isCanonicalPair,
109+
reindexToolCanonicalModes,
109110
resolveCanonicalMode,
110111
resolveDependencyValue,
111112
type SubBlockCondition,
@@ -488,7 +489,15 @@ export const ToolInput = memo(function ToolInput({
488489
[blockId]
489490
)
490491
)
491-
const { collaborativeSetBlockCanonicalMode } = useCollaborativeWorkflow()
492+
const { collaborativeSetBlockCanonicalMode, collaborativeSetBlockCanonicalModes } =
493+
useCollaborativeWorkflow()
494+
const reindexCanonicalModesOnMutate = useCallback(
495+
(oldTools: StoredTool[], newTools: StoredTool[]) => {
496+
const next = reindexToolCanonicalModes(oldTools, newTools, canonicalModeOverrides)
497+
if (next) collaborativeSetBlockCanonicalModes(blockId, next)
498+
},
499+
[canonicalModeOverrides, collaborativeSetBlockCanonicalModes, blockId]
500+
)
492501

493502
const value = isPreview ? previewValue : storeValue
494503

@@ -514,11 +523,15 @@ export const ToolInput = memo(function ToolInput({
514523
// Uses canonical resolution so the active field (basic vs advanced) is respected.
515524
const toolCredentialId = useMemo(() => {
516525
const allBlocks = getAllBlocks()
517-
for (const tool of selectedTools) {
526+
for (const [toolIndex, tool] of selectedTools.entries()) {
518527
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
519528
if (!blockConfig?.subBlocks) continue
520529
const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks)
521-
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
530+
const scopedOverrides = scopeCanonicalModesForTool(
531+
canonicalModeOverrides,
532+
toolIndex,
533+
tool.type
534+
)
522535
const reactiveSubBlock = blockConfig.subBlocks.find(
523536
(sb: { reactiveCondition?: unknown }) => sb.reactiveCondition
524537
)
@@ -909,19 +922,23 @@ export const ToolInput = memo(function ToolInput({
909922
const handleRemoveTool = useCallback(
910923
(toolIndex: number) => {
911924
if (isPreview || disabled) return
912-
setStoreValue(selectedTools.filter((_, index) => index !== toolIndex))
925+
const updatedTools = selectedTools.filter((_, index) => index !== toolIndex)
926+
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
927+
setStoreValue(updatedTools)
913928
},
914-
[isPreview, disabled, selectedTools, setStoreValue]
929+
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
915930
)
916931

917932
const handleRemoveAllFromServer = useCallback(
918933
(serverId: string | undefined) => {
919934
if (isPreview || disabled || !serverId) return
920-
setStoreValue(
921-
selectedTools.filter((t) => !(t.type === 'mcp' && t.params?.serverId === serverId))
935+
const updatedTools = selectedTools.filter(
936+
(t) => !(t.type === 'mcp' && t.params?.serverId === serverId)
922937
)
938+
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
939+
setStoreValue(updatedTools)
923940
},
924-
[isPreview, disabled, selectedTools, setStoreValue]
941+
[isPreview, disabled, selectedTools, reindexCanonicalModesOnMutate, setStoreValue]
925942
)
926943

927944
const handleDeleteTool = useCallback(
@@ -949,10 +966,11 @@ export const ToolInput = memo(function ToolInput({
949966
})
950967

951968
if (updatedTools.length !== selectedTools.length) {
969+
reindexCanonicalModesOnMutate(selectedTools, updatedTools)
952970
setStoreValue(updatedTools)
953971
}
954972
},
955-
[selectedTools, customTools, setStoreValue]
973+
[selectedTools, customTools, reindexCanonicalModesOnMutate, setStoreValue]
956974
)
957975

958976
const handleParamChange = useCallback(
@@ -1121,6 +1139,7 @@ export const ToolInput = memo(function ToolInput({
11211139
newTools.splice(adjustedDropIndex, 0, draggedTool)
11221140
}
11231141

1142+
reindexCanonicalModesOnMutate(selectedTools, newTools)
11241143
setStoreValue(newTools)
11251144
setDraggedIndex(null)
11261145
setDragOverIndex(null)
@@ -1420,6 +1439,10 @@ export const ToolInput = memo(function ToolInput({
14201439
description: mcpTool.description,
14211440
},
14221441
}))
1442+
// Diff against `filteredTools` (pre-spread, same refs as `selectedTools`) - the
1443+
// spread copy below preserves the same relative order, so this correctly reflects
1444+
// each surviving tool's new position.
1445+
reindexCanonicalModesOnMutate(selectedTools, filteredTools)
14231446
setStoreValue([...filteredTools.map((t) => ({ ...t, isExpanded: false })), ...newTools])
14241447
setMcpServerDrilldown(null)
14251448
setOpen(false)
@@ -1650,6 +1673,7 @@ export const ToolInput = memo(function ToolInput({
16501673
customUnsupported,
16511674
availableWorkflows,
16521675
isToolAlreadySelected,
1676+
reindexCanonicalModesOnMutate,
16531677
])
16541678

16551679
return (
@@ -1692,7 +1716,11 @@ export const ToolInput = memo(function ToolInput({
16921716
})
16931717
: null
16941718

1695-
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
1719+
const toolScopedOverrides = scopeCanonicalModesForTool(
1720+
canonicalModeOverrides,
1721+
toolIndex,
1722+
tool.type
1723+
)
16961724

16971725
const subBlocksResult: SubBlocksForToolInput | null =
16981726
!isCustomTool && !isMcpTool && currentToolId
@@ -2086,7 +2114,7 @@ export const ToolInput = memo(function ToolInput({
20862114
const nextMode = canonicalMode === 'advanced' ? 'basic' : 'advanced'
20872115
collaborativeSetBlockCanonicalMode(
20882116
blockId,
2089-
`${tool.type}:${canonicalId}`,
2117+
`${toolIndex}:${canonicalId}`,
20902118
nextMode
20912119
)
20922120
},

‎apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,64 @@ describe('copyWorkflowStateIntoTarget folder fallback', () => {
293293
expect(result.name).toBe('Orphaned placement')
294294
})
295295
})
296+
297+
describe('copyWorkflowStateIntoTarget canonicalModes reindex propagation', () => {
298+
it(
299+
"persists a transform's reindexed canonicalModes on the copied block, and uses that " +
300+
"SAME reindexed value (not the source's stale one) for every subsequent remap step",
301+
async () => {
302+
mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true })
303+
const seenCanonicalModes: Array<Record<string, 'basic' | 'advanced'> | undefined> = []
304+
const tx = {
305+
insert: () => ({ values: () => Promise.resolve() }),
306+
} as unknown as DbOrTx
307+
308+
await copyWorkflowStateIntoTarget({
309+
tx,
310+
targetWorkflowId: 'wf-child',
311+
targetWorkspaceId: 'ws-target',
312+
userId: 'target-user',
313+
mode: 'create',
314+
now: new Date('2026-07-01'),
315+
sourceState: {
316+
blocks: {
317+
block1: {
318+
id: 'block1',
319+
type: 'agent',
320+
name: 'Agent',
321+
subBlocks: {},
322+
// The source's ORIGINAL (pre-drop) canonicalModes - every step after the
323+
// transform must see the REINDEXED value below instead, not this one.
324+
data: { canonicalModes: { '1:credential': 'advanced' } },
325+
},
326+
},
327+
edges: [],
328+
loops: {},
329+
parallels: {},
330+
variables: {},
331+
},
332+
sourceMeta: { name: 'Reindex test', description: null, folderId: null, sortOrder: 0 },
333+
workflowIdMap: new Map(),
334+
folderIdMap: new Map(),
335+
nameRegistry: buildWorkflowNameRegistry([]),
336+
// Simulates a `tool-input` drop shifting tool 1 -> 0: returns subBlocks unchanged but
337+
// reports the reindexed canonicalModes via the callback, exactly like
338+
// `createForkBootstrapTransform`/`createForkSubBlockTransform` do.
339+
transformSubBlocks: (subBlocks, _blockType, canonicalModes, onCanonicalModesChanged) => {
340+
seenCanonicalModes.push(canonicalModes)
341+
onCanonicalModesChanged?.({ '0:credential': 'advanced' })
342+
return subBlocks
343+
},
344+
})
345+
346+
const [, remappedState] = mockSaveWorkflowToNormalizedTables.mock.calls.at(-1)!
347+
const persistedBlock = Object.values(remappedState.blocks)[0] as {
348+
data?: { canonicalModes?: Record<string, 'basic' | 'advanced'> }
349+
}
350+
// The transform received the source's original value...
351+
expect(seenCanonicalModes).toEqual([{ '1:credential': 'advanced' }])
352+
// ...and the PERSISTED block carries the reindexed one, not the stale source value.
353+
expect(persistedBlock.data?.canonicalModes).toEqual({ '0:credential': 'advanced' })
354+
}
355+
)
356+
})

‎apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
sanitizeSubBlocksForDuplicate,
1313
} from '@/lib/workflows/persistence/remap-internal-ids'
1414
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
15+
import type { CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility'
1516
import {
1617
deriveForkBlockId,
1718
type ForkBlockIdResolver,
@@ -20,6 +21,7 @@ import {
2021
applyDependentOverrides,
2122
collectClearedDependents,
2223
type NeedsConfigurationField,
24+
type SubBlockTransform,
2325
} from '@/ee/workspace-forking/lib/remap/remap-references'
2426
import type {
2527
BlockData,
@@ -33,12 +35,6 @@ import type {
3335

3436
const logger = createLogger('WorkspaceForkCopyWorkflows')
3537

36-
type SubBlockTransform = (
37-
subBlocks: SubBlockRecord,
38-
blockType: string,
39-
canonicalModes?: Record<string, 'basic' | 'advanced'>
40-
) => SubBlockRecord
41-
4238
interface ResolveForkFolderMappingParams {
4339
tx: DbOrTx
4440
sourceWorkspaceId: string
@@ -433,13 +429,18 @@ export async function copyWorkflowStateIntoTarget(
433429
const sourceSubBlocks = (block.subBlocks ?? {}) as unknown as SubBlockRecord
434430
const sanitizedSource = sanitizeSubBlocksForDuplicate(sourceSubBlocks)
435431
let subBlocks: SubBlockRecord = sanitizedSource
432+
// Tracks the block's live `canonicalModes` through this pass, so a `tool-input` reindex
433+
// (a dropped custom-tool/MCP entry shifts later tools' array positions) is visible to every
434+
// later step below that resolves a nested tool's basic/advanced mode - not just the final
435+
// persisted `updatedData`. Starts as the source value; `transformSubBlocks` may replace it.
436+
let activeCanonicalModes: CanonicalModeOverrides | undefined = (
437+
block.data as { canonicalModes?: Record<string, 'basic' | 'advanced'> } | undefined
438+
)?.canonicalModes
436439
if (transformSubBlocks) {
437-
subBlocks = transformSubBlocks(
438-
subBlocks,
439-
block.type,
440-
(block.data as { canonicalModes?: Record<string, 'basic' | 'advanced'> } | undefined)
441-
?.canonicalModes
442-
)
440+
subBlocks = transformSubBlocks(subBlocks, block.type, activeCanonicalModes, (next) => {
441+
activeCanonicalModes = next
442+
updatedData = { ...updatedData, canonicalModes: next } as BlockData
443+
})
443444
}
444445
if (varIdMapping.size > 0) {
445446
subBlocks = remapVariableIdsInSubBlocks(subBlocks, varIdMapping)
@@ -448,9 +449,7 @@ export async function copyWorkflowStateIntoTarget(
448449
// rather than leave them pointing at the source workspace.
449450
subBlocks = remapWorkflowReferencesInSubBlocks(subBlocks, workflowIdMap, {
450451
clearUnmapped: true,
451-
canonicalModes: (
452-
block.data as { canonicalModes?: Record<string, 'basic' | 'advanced'> } | undefined
453-
)?.canonicalModes,
452+
canonicalModes: activeCanonicalModes,
454453
})
455454
subBlocks = remapConditionIdsInSubBlocks(subBlocks, oldBlockId, newBlockId) as SubBlockRecord
456455

@@ -477,8 +476,7 @@ export async function copyWorkflowStateIntoTarget(
477476
block.name,
478477
targetCurrent,
479478
subBlocks,
480-
(block.data as { canonicalModes?: Record<string, 'basic' | 'advanced'> } | undefined)
481-
?.canonicalModes
479+
activeCanonicalModes
482480
)
483481
)
484482
}

0 commit comments

Comments
 (0)