Skip to content

Commit 5fc1245

Browse files
committed
fix(agent): scope nested tool canonical-mode overrides by instance, not type
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 5db62b8 commit 5fc1245

12 files changed

Lines changed: 460 additions & 61 deletions

File tree

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

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { describe, expect, it } from 'vitest'
4+
import { describe, expect, it, vi } from 'vitest'
55
import type { StoredTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/types'
66
import {
77
isCustomToolAlreadySelected,
88
isMcpToolAlreadySelected,
99
isWorkflowAlreadySelected,
10+
reindexToolCanonicalModes,
1011
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/utils'
1112

1213
describe('isMcpToolAlreadySelected', () => {
@@ -416,3 +417,76 @@ describe('duplicate prevention integration scenarios', () => {
416417
})
417418
})
418419
})
420+
421+
describe('reindexToolCanonicalModes', () => {
422+
const table = (title: string): StoredTool => ({ type: 'table', title, params: {} })
423+
424+
it.concurrent('does nothing when there are no overrides', () => {
425+
const applyMode = vi.fn()
426+
reindexToolCanonicalModes([table('a')], [table('a')], undefined, applyMode)
427+
expect(applyMode).not.toHaveBeenCalled()
428+
})
429+
430+
it.concurrent('does nothing when a tool keeps its index', () => {
431+
const a = table('a')
432+
const b = table('b')
433+
const applyMode = vi.fn()
434+
reindexToolCanonicalModes([a, b], [a, b], { '0:tableId': 'advanced' }, applyMode)
435+
expect(applyMode).not.toHaveBeenCalled()
436+
})
437+
438+
it.concurrent('re-keys a surviving tool overrides to its new index after a removal', () => {
439+
const a = table('a')
440+
const b = table('b')
441+
const c = table('c')
442+
const applyMode = vi.fn()
443+
// Remove `a` (index 0): b shifts 1->0, c shifts 2->1.
444+
reindexToolCanonicalModes(
445+
[a, b, c],
446+
[b, c],
447+
{ '1:tableId': 'advanced', '2:tableId': 'basic' },
448+
applyMode
449+
)
450+
expect(applyMode).toHaveBeenCalledWith('0:tableId', 'advanced')
451+
expect(applyMode).toHaveBeenCalledWith('1:tableId', 'basic')
452+
expect(applyMode).toHaveBeenCalledTimes(2)
453+
})
454+
455+
it.concurrent('re-keys overrides after a drag reorder', () => {
456+
const a = table('a')
457+
const b = table('b')
458+
const applyMode = vi.fn()
459+
// Swap a and b: a moves 0->1, b moves 1->0.
460+
reindexToolCanonicalModes(
461+
[a, b],
462+
[b, a],
463+
{ '0:tableId': 'advanced', '1:tableId': 'basic' },
464+
applyMode
465+
)
466+
expect(applyMode).toHaveBeenCalledWith('1:tableId', 'advanced')
467+
expect(applyMode).toHaveBeenCalledWith('0:tableId', 'basic')
468+
expect(applyMode).toHaveBeenCalledTimes(2)
469+
})
470+
471+
it.concurrent('leaves a removed tool old key untouched (no applyMode call for it)', () => {
472+
const a = table('a')
473+
const b = table('b')
474+
const applyMode = vi.fn()
475+
// Remove `b` (index 1): nothing survives at index 1, so no call is made for its key.
476+
reindexToolCanonicalModes([a, b], [a], { '1:tableId': 'advanced' }, applyMode)
477+
expect(applyMode).not.toHaveBeenCalled()
478+
})
479+
480+
it.concurrent('ignores falsy override values', () => {
481+
const a = table('a')
482+
const b = table('b')
483+
const applyMode = vi.fn()
484+
reindexToolCanonicalModes(
485+
[a, b],
486+
[b, a],
487+
{ '0:tableId': undefined as unknown as 'advanced' },
488+
applyMode
489+
)
490+
expect(applyMode).not.toHaveBeenCalled()
491+
})
492+
})

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

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
isCustomToolAlreadySelected,
5050
isMcpToolAlreadySelected,
5151
isWorkflowAlreadySelected,
52+
reindexToolCanonicalModes,
5253
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/utils'
5354
import {
5455
getActiveWorkflowSearchHighlight,
@@ -489,6 +490,11 @@ export const ToolInput = memo(function ToolInput({
489490
)
490491
)
491492
const { collaborativeSetBlockCanonicalMode } = useCollaborativeWorkflow()
493+
const applyCanonicalModeReindex = useCallback(
494+
(key: string, mode: 'basic' | 'advanced') =>
495+
collaborativeSetBlockCanonicalMode(blockId, key, mode),
496+
[collaborativeSetBlockCanonicalMode, blockId]
497+
)
492498

493499
const value = isPreview ? previewValue : storeValue
494500

@@ -514,11 +520,15 @@ export const ToolInput = memo(function ToolInput({
514520
// Uses canonical resolution so the active field (basic vs advanced) is respected.
515521
const toolCredentialId = useMemo(() => {
516522
const allBlocks = getAllBlocks()
517-
for (const tool of selectedTools) {
523+
for (const [toolIndex, tool] of selectedTools.entries()) {
518524
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
519525
if (!blockConfig?.subBlocks) continue
520526
const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks)
521-
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
527+
const scopedOverrides = scopeCanonicalModesForTool(
528+
canonicalModeOverrides,
529+
toolIndex,
530+
tool.type
531+
)
522532
const reactiveSubBlock = blockConfig.subBlocks.find(
523533
(sb: { reactiveCondition?: unknown }) => sb.reactiveCondition
524534
)
@@ -909,19 +919,47 @@ export const ToolInput = memo(function ToolInput({
909919
const handleRemoveTool = useCallback(
910920
(toolIndex: number) => {
911921
if (isPreview || disabled) return
912-
setStoreValue(selectedTools.filter((_, index) => index !== toolIndex))
922+
const updatedTools = selectedTools.filter((_, index) => index !== toolIndex)
923+
reindexToolCanonicalModes(
924+
selectedTools,
925+
updatedTools,
926+
canonicalModeOverrides,
927+
applyCanonicalModeReindex
928+
)
929+
setStoreValue(updatedTools)
913930
},
914-
[isPreview, disabled, selectedTools, setStoreValue]
931+
[
932+
isPreview,
933+
disabled,
934+
selectedTools,
935+
canonicalModeOverrides,
936+
applyCanonicalModeReindex,
937+
setStoreValue,
938+
]
915939
)
916940

917941
const handleRemoveAllFromServer = useCallback(
918942
(serverId: string | undefined) => {
919943
if (isPreview || disabled || !serverId) return
920-
setStoreValue(
921-
selectedTools.filter((t) => !(t.type === 'mcp' && t.params?.serverId === serverId))
944+
const updatedTools = selectedTools.filter(
945+
(t) => !(t.type === 'mcp' && t.params?.serverId === serverId)
946+
)
947+
reindexToolCanonicalModes(
948+
selectedTools,
949+
updatedTools,
950+
canonicalModeOverrides,
951+
applyCanonicalModeReindex
922952
)
953+
setStoreValue(updatedTools)
923954
},
924-
[isPreview, disabled, selectedTools, setStoreValue]
955+
[
956+
isPreview,
957+
disabled,
958+
selectedTools,
959+
canonicalModeOverrides,
960+
applyCanonicalModeReindex,
961+
setStoreValue,
962+
]
925963
)
926964

927965
const handleDeleteTool = useCallback(
@@ -949,10 +987,16 @@ export const ToolInput = memo(function ToolInput({
949987
})
950988

951989
if (updatedTools.length !== selectedTools.length) {
990+
reindexToolCanonicalModes(
991+
selectedTools,
992+
updatedTools,
993+
canonicalModeOverrides,
994+
applyCanonicalModeReindex
995+
)
952996
setStoreValue(updatedTools)
953997
}
954998
},
955-
[selectedTools, customTools, setStoreValue]
999+
[selectedTools, customTools, canonicalModeOverrides, applyCanonicalModeReindex, setStoreValue]
9561000
)
9571001

9581002
const handleParamChange = useCallback(
@@ -1121,6 +1165,12 @@ export const ToolInput = memo(function ToolInput({
11211165
newTools.splice(adjustedDropIndex, 0, draggedTool)
11221166
}
11231167

1168+
reindexToolCanonicalModes(
1169+
selectedTools,
1170+
newTools,
1171+
canonicalModeOverrides,
1172+
applyCanonicalModeReindex
1173+
)
11241174
setStoreValue(newTools)
11251175
setDraggedIndex(null)
11261176
setDragOverIndex(null)
@@ -1420,6 +1470,15 @@ export const ToolInput = memo(function ToolInput({
14201470
description: mcpTool.description,
14211471
},
14221472
}))
1473+
// Diff against `filteredTools` (pre-spread, same refs as `selectedTools`) - the
1474+
// spread copy below preserves the same relative order, so this correctly reflects
1475+
// each surviving tool's new position.
1476+
reindexToolCanonicalModes(
1477+
selectedTools,
1478+
filteredTools,
1479+
canonicalModeOverrides,
1480+
applyCanonicalModeReindex
1481+
)
14231482
setStoreValue([...filteredTools.map((t) => ({ ...t, isExpanded: false })), ...newTools])
14241483
setMcpServerDrilldown(null)
14251484
setOpen(false)
@@ -1650,6 +1709,8 @@ export const ToolInput = memo(function ToolInput({
16501709
customUnsupported,
16511710
availableWorkflows,
16521711
isToolAlreadySelected,
1712+
canonicalModeOverrides,
1713+
applyCanonicalModeReindex,
16531714
])
16541715

16551716
return (
@@ -1692,7 +1753,11 @@ export const ToolInput = memo(function ToolInput({
16921753
})
16931754
: null
16941755

1695-
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
1756+
const toolScopedOverrides = scopeCanonicalModesForTool(
1757+
canonicalModeOverrides,
1758+
toolIndex,
1759+
tool.type
1760+
)
16961761

16971762
const subBlocksResult: SubBlocksForToolInput | null =
16981763
!isCustomTool && !isMcpTool && currentToolId
@@ -2086,7 +2151,7 @@ export const ToolInput = memo(function ToolInput({
20862151
const nextMode = canonicalMode === 'advanced' ? 'basic' : 'advanced'
20872152
collaborativeSetBlockCanonicalMode(
20882153
blockId,
2089-
`${tool.type}:${canonicalId}`,
2154+
`${toolIndex}:${canonicalId}`,
20902155
nextMode
20912156
)
20922157
},

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility'
12
import type { StoredTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/types'
23

34
/**
@@ -30,3 +31,32 @@ export function isWorkflowAlreadySelected(
3031
(tool) => tool.type === 'workflow_input' && tool.params?.workflowId === workflowId
3132
)
3233
}
34+
35+
/**
36+
* Canonical-mode overrides are keyed by a tool's position in the `tool-input` array
37+
* (`${toolIndex}:${canonicalId}`), so removing or reordering tools must carry each surviving
38+
* tool's overrides to its new position - otherwise a saved basic/advanced choice attaches to
39+
* whichever tool now sits at that old index. Diffs `oldTools` against `newTools` by object
40+
* identity (safe here: every mutation site filters/splices the existing array without cloning
41+
* the tool objects it keeps) and re-emits each surviving key under its new index via `applyMode`.
42+
* A removed tool's old key is left in place, unused - harmless, since nothing queries a
43+
* `toolIndex` no tool occupies anymore.
44+
*/
45+
export function reindexToolCanonicalModes(
46+
oldTools: StoredTool[],
47+
newTools: StoredTool[],
48+
overrides: CanonicalModeOverrides | undefined,
49+
applyMode: (key: string, mode: 'basic' | 'advanced') => void
50+
): void {
51+
if (!overrides) return
52+
const newIndexByRef = new Map(newTools.map((tool, index) => [tool, index]))
53+
oldTools.forEach((tool, oldIndex) => {
54+
const newIndex = newIndexByRef.get(tool)
55+
if (newIndex === undefined || newIndex === oldIndex) return
56+
const prefix = `${oldIndex}:`
57+
for (const [key, mode] of Object.entries(overrides)) {
58+
if (!key.startsWith(prefix) || !mode) continue
59+
applyMode(`${newIndex}:${key.slice(prefix.length)}`, mode)
60+
}
61+
})
62+
}

0 commit comments

Comments
 (0)