Skip to content

Commit 5e84b2c

Browse files
waleedlatif1waleedlatif1
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 5e84b2c

10 files changed

Lines changed: 228 additions & 49 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -514,11 +514,11 @@ export const ToolInput = memo(function ToolInput({
514514
// Uses canonical resolution so the active field (basic vs advanced) is respected.
515515
const toolCredentialId = useMemo(() => {
516516
const allBlocks = getAllBlocks()
517-
for (const tool of selectedTools) {
517+
for (const [toolIndex, tool] of selectedTools.entries()) {
518518
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
519519
if (!blockConfig?.subBlocks) continue
520520
const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks)
521-
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
521+
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, toolIndex)
522522
const reactiveSubBlock = blockConfig.subBlocks.find(
523523
(sb: { reactiveCondition?: unknown }) => sb.reactiveCondition
524524
)
@@ -1692,7 +1692,7 @@ export const ToolInput = memo(function ToolInput({
16921692
})
16931693
: null
16941694

1695-
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
1695+
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, toolIndex)
16961696

16971697
const subBlocksResult: SubBlocksForToolInput | null =
16981698
!isCustomTool && !isMcpTool && currentToolId
@@ -2086,7 +2086,7 @@ export const ToolInput = memo(function ToolInput({
20862086
const nextMode = canonicalMode === 'advanced' ? 'basic' : 'advanced'
20872087
collaborativeSetBlockCanonicalMode(
20882088
blockId,
2089-
`${tool.type}:${canonicalId}`,
2089+
`${toolIndex}:${canonicalId}`,
20902090
nextMode
20912091
)
20922092
},

‎apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.test.ts‎

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ describe('collectForkDependentReconfigs', () => {
411411
return undefined as unknown as BlockConfig
412412
})
413413
// Agent block with a nested gmail tool; the dormant basic credential holds a stale id while the
414-
// tool-scoped `gmail:credential` override (when present) marks advanced as active.
414+
// tool-scoped `0:credential` override (when present) marks advanced as active.
415415
const agentState = (canonicalModes?: Record<string, 'basic' | 'advanced'>) =>
416416
({
417417
blocks: {
@@ -447,7 +447,7 @@ describe('collectForkDependentReconfigs', () => {
447447
// verbatim on sync, so its dependents are never cleared and no re-pick is offered.
448448
const withOverride = collectForkDependentReconfigs(
449449
[replaceItem],
450-
new Map([['wf-src', agentState({ 'gmail:credential': 'advanced' })]]),
450+
new Map([['wf-src', agentState({ '0:credential': 'advanced' })]]),
451451
resolve
452452
)
453453
expect(withOverride).toEqual([])
@@ -465,6 +465,95 @@ describe('collectForkDependentReconfigs', () => {
465465
})
466466
})
467467

468+
it('regression: two same-type nested tools resolve their canonical-mode override independently', () => {
469+
vi.mocked(getBlock).mockImplementation((type) => {
470+
if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }])
471+
if (type === 'gmail')
472+
return blockWith([
473+
{
474+
id: 'credential',
475+
title: 'Credential',
476+
type: 'oauth-input',
477+
canonicalParamId: 'credential',
478+
mode: 'basic',
479+
},
480+
{
481+
id: 'manualCredential',
482+
title: 'Credential ID',
483+
type: 'short-input',
484+
canonicalParamId: 'credential',
485+
mode: 'advanced',
486+
},
487+
{
488+
id: 'folder',
489+
title: 'Label',
490+
type: 'folder-selector',
491+
dependsOn: ['credential'],
492+
selectorKey: 'gmail.labels',
493+
required: true,
494+
},
495+
])
496+
return undefined as unknown as BlockConfig
497+
})
498+
const states = new Map<string, WorkflowState>([
499+
[
500+
'wf-src',
501+
{
502+
blocks: {
503+
'block-1': {
504+
id: 'block-1',
505+
type: 'agent',
506+
name: 'Block',
507+
// Tool #0 is scoped to advanced (manual mode passes through - no re-pick, per the
508+
// test above); tool #1 is scoped to basic (re-pick offered on its own value). Both
509+
// are the SAME tool type ("gmail") and the SAME canonicalId ("credential"), so only
510+
// the per-instance index can tell them apart - before the fix both shared the single
511+
// `gmail:credential` key, which would have made tool #1 advanced-active too (and
512+
// therefore ALSO skipped, dropping the result to zero entries instead of one).
513+
data: { canonicalModes: { '0:credential': 'advanced', '1:credential': 'basic' } },
514+
subBlocks: {
515+
tools: {
516+
value: [
517+
{
518+
type: 'gmail',
519+
title: 'Gmail 1',
520+
params: {
521+
credential: 'cred-0-stale',
522+
manualCredential: 'cred-0-active',
523+
folder: 'INBOX',
524+
},
525+
},
526+
{
527+
type: 'gmail',
528+
title: 'Gmail 2',
529+
params: {
530+
credential: 'cred-1-basic',
531+
manualCredential: 'cred-1-stale',
532+
folder: 'SENT',
533+
},
534+
},
535+
],
536+
},
537+
},
538+
},
539+
},
540+
edges: [],
541+
loops: {},
542+
parallels: {},
543+
variables: {},
544+
} as unknown as WorkflowState,
545+
],
546+
])
547+
548+
const result = collectForkDependentReconfigs([replaceItem], states, resolve)
549+
550+
expect(result).toHaveLength(1)
551+
expect(result[0]).toMatchObject({
552+
parentSourceId: 'cred-1-basic',
553+
subBlockKey: 'tools[1].folder',
554+
})
555+
})
556+
468557
it('offers a nested tool selector even when the source left it empty', () => {
469558
vi.mocked(getBlock).mockImplementation((type) => {
470559
if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }])

‎apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ export function collectForkDependentReconfigs(
319319
contextSubBlocks: toolContextSubBlocks,
320320
blockName: block.name,
321321
targetWorkflowId: item.targetWorkflowId,
322-
canonicalModes: scopeCanonicalModesForTool(block.data?.canonicalModes, tool.type),
322+
canonicalModes: scopeCanonicalModesForTool(block.data?.canonicalModes, toolIndex),
323323
resolveTargetBlockId: resolveBlockId,
324324
makeSubBlockKey: (id) => `${toolInputKey}[${toolIndex}].${id}`,
325325
makeTitle: (dependent) => dependent.title ?? dependent.id ?? '',

‎apps/sim/ee/workspace-forking/lib/remap/remap-references.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -349,10 +349,14 @@ interface ToolBlockRemapOptions {
349349
/** Copy provenance for a resolved target (see {@link RemapForkContext.isCopiedTarget}). */
350350
isCopiedTarget?: (kind: ForkRemapKind, sourceId: string) => boolean
351351
/**
352-
* The owning BLOCK's `data.canonicalModes` (keys scoped `${toolType}:${canonicalId}` for
353-
* nested tools), so the active canonical member per pair matches the tool-input UI.
352+
* The owning BLOCK's `data.canonicalModes` (keys scoped `${toolIndex}:${canonicalId}` for
353+
* nested tools - by the tool's position in its tool-input array, not its type, so two tools
354+
* of the same type don't share an override), so the active canonical member per pair matches
355+
* the tool-input UI.
354356
*/
355357
parentCanonicalModes?: CanonicalModeOverrides
358+
/** This tool's position in its parent's `tool-input` array - see {@link parentCanonicalModes}. */
359+
toolIndex?: number
356360
}
357361

358362
/**
@@ -387,7 +391,7 @@ export function remapToolBlockResources(
387391
// tool-input UI does: the block-level overrides scoped to this tool, then the value heuristic.
388392
const toolValues: Record<string, unknown> =
389393
typeof tool.operation === 'string' ? { operation: tool.operation, ...params } : { ...params }
390-
const scopedModes = scopeCanonicalModesForTool(opts.parentCanonicalModes, tool.type)
394+
const scopedModes = scopeCanonicalModesForTool(opts.parentCanonicalModes, opts.toolIndex)
391395
const toolBlockSubBlocks = (opts.blockConfigs?.[tool.type] ?? getBlock(tool.type))?.subBlocks
392396
const gates = createCanonicalModeGates(toolBlockSubBlocks, toolValues, scopedModes)
393397

@@ -434,6 +438,7 @@ export function remapToolBlockResources(
434438
try {
435439
configs = getToolInputParamConfigs({
436440
tool: toolView,
441+
toolIndex: opts.toolIndex,
437442
parentCanonicalModes: opts.parentCanonicalModes,
438443
blockConfigs: opts.blockConfigs,
439444
})
@@ -612,7 +617,7 @@ interface ForkToolInputOptions {
612617
resolveMcpServerMeta?: ForkMcpServerMetaResolver
613618
/** Copy provenance for a resolved target (see {@link RemapForkContext.isCopiedTarget}). */
614619
isCopiedTarget?: (kind: ForkRemapKind, sourceId: string) => boolean
615-
/** Block-level canonical-mode overrides (`${toolType}:`-scoped for nested tools). */
620+
/** Block-level canonical-mode overrides (`${toolIndex}:`-scoped for nested tools). */
616621
parentCanonicalModes?: CanonicalModeOverrides
617622
}
618623

@@ -632,7 +637,7 @@ function remapForkToolInputValue(
632637
const { array, wasString } = coerceObjectArray(value)
633638
if (!array) return value
634639
let changed = false
635-
const next = array.flatMap((tool) => {
640+
const next = array.flatMap((tool, toolIndex) => {
636641
if (!isRecord(tool) || typeof tool.type !== 'string') return [tool]
637642
if (tool.type === 'custom-tool' && typeof tool.customToolId === 'string') {
638643
const target = resolve('custom-tool', tool.customToolId)
@@ -694,6 +699,7 @@ function remapForkToolInputValue(
694699
clearUnresolved: opts.clearUnresolved,
695700
isCopiedTarget: opts.isCopiedTarget,
696701
parentCanonicalModes: opts.parentCanonicalModes,
702+
toolIndex,
697703
})
698704
if (remapped !== tool) changed = true
699705
return [remapped]
@@ -1156,7 +1162,7 @@ function collectClearedToolParamDependents(
11561162
const gates = createCanonicalModeGates(
11571163
toolConfig.subBlocks,
11581164
mergedValues,
1159-
scopeCanonicalModesForTool(parentCanonicalModes, tool.type)
1165+
scopeCanonicalModesForTool(parentCanonicalModes, index)
11601166
)
11611167
const toolLabel = typeof tool.title === 'string' && tool.title ? tool.title : toolConfig.name
11621168
for (const cfg of toolConfig.subBlocks) {

‎apps/sim/executor/handlers/agent/agent-handler.ts‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ export class AgentBlockHandler implements BlockHandler {
6666
block: SerializedBlock,
6767
inputs: AgentInputs
6868
): Promise<BlockOutput | StreamingExecution> {
69+
const toolIndexByRef = new Map<ToolInput, number>(
70+
(inputs.tools || []).map((tool, index) => [tool, index] as const)
71+
)
72+
6973
const filteredTools = await this.filterUnavailableMcpTools(ctx, inputs.tools || [])
7074
const filteredInputs = { ...inputs, tools: filteredTools }
7175

@@ -80,7 +84,8 @@ export class AgentBlockHandler implements BlockHandler {
8084
const formattedTools = await this.formatTools(
8185
ctx,
8286
filteredInputs.tools || [],
83-
block.canonicalModes
87+
block.canonicalModes,
88+
toolIndexByRef
8489
)
8590

8691
const skillInputs = filteredInputs.skills ?? []
@@ -205,39 +210,46 @@ export class AgentBlockHandler implements BlockHandler {
205210
})
206211
}
207212

213+
/**
214+
* `canonicalModes` overrides are keyed by each tool's position in the ORIGINAL, unfiltered
215+
* tools array (matching what the editor wrote), not by `tool.type` - so two tool entries of
216+
* the same type (e.g. two Table tools) resolve independently. `toolIndexByRef` preserves that
217+
* original position across the mcp-availability filter and the mcp/other split below, both of
218+
* which would otherwise renumber tools by their post-filter position.
219+
*/
208220
private async formatTools(
209221
ctx: ExecutionContext,
210222
inputTools: ToolInput[],
211-
canonicalModes?: Record<string, 'basic' | 'advanced'>
223+
canonicalModes?: Record<string, 'basic' | 'advanced'>,
224+
toolIndexByRef?: Map<ToolInput, number>
212225
): Promise<any[]> {
213226
if (!Array.isArray(inputTools)) return []
214227

215-
const filtered = inputTools.filter((tool) => {
216-
const usageControl = tool.usageControl || 'auto'
217-
return usageControl !== 'none'
218-
})
228+
const filtered = inputTools
229+
.map((tool, localIndex) => ({ tool, toolIndex: toolIndexByRef?.get(tool) ?? localIndex }))
230+
.filter(({ tool }) => (tool.usageControl || 'auto') !== 'none')
219231

220232
const mcpTools: ToolInput[] = []
221-
const otherTools: ToolInput[] = []
233+
const otherTools: Array<{ tool: ToolInput; toolIndex: number }> = []
222234

223-
for (const tool of filtered) {
224-
if (tool.type === 'mcp') {
225-
mcpTools.push(tool)
235+
for (const entry of filtered) {
236+
if (entry.tool.type === 'mcp') {
237+
mcpTools.push(entry.tool)
226238
} else {
227-
otherTools.push(tool)
239+
otherTools.push(entry)
228240
}
229241
}
230242

231243
const otherResults = await Promise.all(
232-
otherTools.map(async (tool) => {
244+
otherTools.map(async ({ tool, toolIndex }) => {
233245
try {
234246
if (tool.type && tool.type !== 'custom-tool') {
235247
await validateBlockType(ctx.userId, ctx.workspaceId, tool.type, ctx)
236248
}
237249
if (tool.type === 'custom-tool' && (tool.schema || tool.customToolId)) {
238250
return await this.createCustomTool(ctx, tool)
239251
}
240-
return this.transformBlockTool(ctx, tool, canonicalModes)
252+
return this.transformBlockTool(ctx, tool, canonicalModes, toolIndex)
241253
} catch (error) {
242254
logger.error(`[AgentHandler] Error creating tool:`, { tool, error })
243255
return null
@@ -554,7 +566,8 @@ export class AgentBlockHandler implements BlockHandler {
554566
private async transformBlockTool(
555567
ctx: ExecutionContext,
556568
tool: ToolInput,
557-
canonicalModes?: Record<string, 'basic' | 'advanced'>
569+
canonicalModes?: Record<string, 'basic' | 'advanced'>,
570+
toolIndex?: number
558571
) {
559572
const transformedTool = await transformBlockTool(tool, {
560573
selectedOperation: tool.operation,
@@ -567,6 +580,7 @@ export class AgentBlockHandler implements BlockHandler {
567580
}),
568581
getTool,
569582
canonicalModes,
583+
toolIndex,
570584
})
571585

572586
if (transformedTool) {

‎apps/sim/lib/workflows/search-replace/indexer.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,11 +676,14 @@ function isVisibleToolParameter(param: ToolParameterConfig, values: Record<strin
676676
*/
677677
export function getToolInputParamConfigs({
678678
tool,
679+
toolIndex,
679680
parentCanonicalModes,
680681
credentialTypeById,
681682
blockConfigs,
682683
}: {
683684
tool: ParsedStoredTool
685+
/** Position of `tool` within its parent's `tool-input` array - canonical-mode overrides are keyed by this, not `tool.type`, so same-type tools don't collide. Omit only when `parentCanonicalModes` is also absent (e.g. fork/promote remaps that don't resolve canonical modes). */
686+
toolIndex?: number
684687
parentCanonicalModes?: CanonicalModeOverrides
685688
credentialTypeById?: Record<string, string | undefined>
686689
blockConfigs?: WorkflowSearchIndexerOptions['blockConfigs']
@@ -724,7 +727,7 @@ export function getToolInputParamConfigs({
724727

725728
if (!toolId) return genericFallback()
726729

727-
const scopedCanonicalModes = scopeCanonicalModesForTool(parentCanonicalModes, tool.type)
730+
const scopedCanonicalModes = scopeCanonicalModesForTool(parentCanonicalModes, toolIndex)
728731
const blockConfig =
729732
tool.type !== 'custom-tool' && tool.type !== 'mcp'
730733
? (blockConfigs?.[tool.type] ?? getBlock(tool.type))
@@ -989,6 +992,7 @@ function addToolInputMatches({
989992

990993
const params = getToolInputParamConfigs({
991994
tool,
995+
toolIndex,
992996
parentCanonicalModes,
993997
credentialTypeById,
994998
blockConfigs,

0 commit comments

Comments
 (0)