Skip to content

Commit 9f47896

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 9d44d42 commit 9f47896

9 files changed

Lines changed: 218 additions & 43 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
@@ -512,11 +512,11 @@ export const ToolInput = memo(function ToolInput({
512512
// Uses canonical resolution so the active field (basic vs advanced) is respected.
513513
const toolCredentialId = useMemo(() => {
514514
const allBlocks = getAllBlocks()
515-
for (const tool of selectedTools) {
515+
for (const [toolIndex, tool] of selectedTools.entries()) {
516516
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
517517
if (!blockConfig?.subBlocks) continue
518518
const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks)
519-
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
519+
const scopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, toolIndex)
520520
const reactiveSubBlock = blockConfig.subBlocks.find(
521521
(sb: { reactiveCondition?: unknown }) => sb.reactiveCondition
522522
)
@@ -1680,7 +1680,7 @@ export const ToolInput = memo(function ToolInput({
16801680
})
16811681
: null
16821682

1683-
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, tool.type)
1683+
const toolScopedOverrides = scopeCanonicalModesForTool(canonicalModeOverrides, toolIndex)
16841684

16851685
const subBlocksResult: SubBlocksForToolInput | null =
16861686
!isCustomTool && !isMcpTool && currentToolId
@@ -2072,7 +2072,7 @@ export const ToolInput = memo(function ToolInput({
20722072
const nextMode = canonicalMode === 'advanced' ? 'basic' : 'advanced'
20732073
collaborativeSetBlockCanonicalMode(
20742074
blockId,
2075-
`${tool.type}:${canonicalId}`,
2075+
`${toolIndex}:${canonicalId}`,
20762076
nextMode
20772077
)
20782078
},

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

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

@@ -79,7 +83,8 @@ export class AgentBlockHandler implements BlockHandler {
7983
const formattedTools = await this.formatTools(
8084
ctx,
8185
filteredInputs.tools || [],
82-
block.canonicalModes
86+
block.canonicalModes,
87+
toolIndexByRef
8388
)
8489

8590
const skillInputs = filteredInputs.skills ?? []
@@ -204,39 +209,46 @@ export class AgentBlockHandler implements BlockHandler {
204209
})
205210
}
206211

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

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

219231
const mcpTools: ToolInput[] = []
220-
const otherTools: ToolInput[] = []
232+
const otherTools: Array<{ tool: ToolInput; toolIndex: number }> = []
221233

222-
for (const tool of filtered) {
223-
if (tool.type === 'mcp') {
224-
mcpTools.push(tool)
234+
for (const entry of filtered) {
235+
if (entry.tool.type === 'mcp') {
236+
mcpTools.push(entry.tool)
225237
} else {
226-
otherTools.push(tool)
238+
otherTools.push(entry)
227239
}
228240
}
229241

230242
const otherResults = await Promise.all(
231-
otherTools.map(async (tool) => {
243+
otherTools.map(async ({ tool, toolIndex }) => {
232244
try {
233245
if (tool.type && tool.type !== 'custom-tool') {
234246
await validateBlockType(ctx.userId, ctx.workspaceId, tool.type, ctx)
235247
}
236248
if (tool.type === 'custom-tool' && (tool.schema || tool.customToolId)) {
237249
return await this.createCustomTool(ctx, tool)
238250
}
239-
return this.transformBlockTool(ctx, tool, canonicalModes)
251+
return this.transformBlockTool(ctx, tool, canonicalModes, toolIndex)
240252
} catch (error) {
241253
logger.error(`[AgentHandler] Error creating tool:`, { tool, error })
242254
return null
@@ -553,7 +565,8 @@ export class AgentBlockHandler implements BlockHandler {
553565
private async transformBlockTool(
554566
ctx: ExecutionContext,
555567
tool: ToolInput,
556-
canonicalModes?: Record<string, 'basic' | 'advanced'>
568+
canonicalModes?: Record<string, 'basic' | 'advanced'>,
569+
toolIndex?: number
557570
) {
558571
const transformedTool = await transformBlockTool(tool, {
559572
selectedOperation: tool.operation,
@@ -566,6 +579,7 @@ export class AgentBlockHandler implements BlockHandler {
566579
}),
567580
getTool,
568581
canonicalModes,
582+
toolIndex,
569583
})
570584

571585
if (transformedTool) {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,11 +675,14 @@ function isVisibleToolParameter(param: ToolParameterConfig, values: Record<strin
675675
*/
676676
export function getToolInputParamConfigs({
677677
tool,
678+
toolIndex,
678679
parentCanonicalModes,
679680
credentialTypeById,
680681
blockConfigs,
681682
}: {
682683
tool: ParsedStoredTool
684+
/** 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). */
685+
toolIndex?: number
683686
parentCanonicalModes?: CanonicalModeOverrides
684687
credentialTypeById?: Record<string, string | undefined>
685688
blockConfigs?: WorkflowSearchIndexerOptions['blockConfigs']
@@ -723,7 +726,7 @@ export function getToolInputParamConfigs({
723726

724727
if (!toolId) return genericFallback()
725728

726-
const scopedCanonicalModes = scopeCanonicalModesForTool(parentCanonicalModes, tool.type)
729+
const scopedCanonicalModes = scopeCanonicalModesForTool(parentCanonicalModes, toolIndex)
727730
const blockConfig =
728731
tool.type !== 'custom-tool' && tool.type !== 'mcp'
729732
? (blockConfigs?.[tool.type] ?? getBlock(tool.type))
@@ -977,6 +980,7 @@ function addToolInputMatches({
977980

978981
const params = getToolInputParamConfigs({
979982
tool,
983+
toolIndex,
980984
parentCanonicalModes,
981985
credentialTypeById,
982986
blockConfigs,

apps/sim/lib/workflows/subblocks/visibility.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { evaluateSubBlockCondition } from './visibility'
5+
import { evaluateSubBlockCondition, scopeCanonicalModesForTool } from './visibility'
66

77
describe('evaluateSubBlockCondition', () => {
88
describe('simple value matching', () => {
@@ -178,3 +178,39 @@ describe('evaluateSubBlockCondition', () => {
178178
})
179179
})
180180
})
181+
182+
describe('scopeCanonicalModesForTool', () => {
183+
it.concurrent('returns undefined when there are no overrides', () => {
184+
expect(scopeCanonicalModesForTool(undefined, 0)).toBeUndefined()
185+
})
186+
187+
it.concurrent('returns undefined when toolIndex is undefined', () => {
188+
expect(scopeCanonicalModesForTool({ '0:tableId': 'advanced' }, undefined)).toBeUndefined()
189+
})
190+
191+
it.concurrent('strips the toolIndex prefix for the matching tool instance', () => {
192+
const overrides = { '0:tableId': 'advanced', '1:tableId': 'basic' }
193+
expect(scopeCanonicalModesForTool(overrides, 0)).toEqual({ tableId: 'advanced' })
194+
expect(scopeCanonicalModesForTool(overrides, 1)).toEqual({ tableId: 'basic' })
195+
})
196+
197+
it.concurrent(
198+
'keeps two same-type tool instances independent (regression: two Table tools on one Agent block used to share a mode)',
199+
() => {
200+
const overrides = { '0:tableId': 'advanced', '1:tableId': 'basic' }
201+
// Both tools have type "table" and canonicalId "tableId" - only toolIndex disambiguates them.
202+
expect(scopeCanonicalModesForTool(overrides, 0)).toEqual({ tableId: 'advanced' })
203+
expect(scopeCanonicalModesForTool(overrides, 1)).toEqual({ tableId: 'basic' })
204+
}
205+
)
206+
207+
it.concurrent('returns undefined when no keys match the given toolIndex prefix', () => {
208+
expect(scopeCanonicalModesForTool({ '1:tableId': 'advanced' }, 0)).toBeUndefined()
209+
})
210+
211+
it.concurrent('ignores falsy override values', () => {
212+
expect(
213+
scopeCanonicalModesForTool({ '0:tableId': undefined as unknown as 'advanced' }, 0)
214+
).toBeUndefined()
215+
})
216+
})

apps/sim/lib/workflows/subblocks/visibility.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -260,18 +260,20 @@ export function resolveActiveCanonicalValue(
260260
}
261261

262262
/**
263-
* Strip the `${toolType}:` prefix from canonical-mode override keys, returning the overrides for a
263+
* Strip the `${toolIndex}:` prefix from canonical-mode override keys, returning the overrides for a
264264
* nested tool keyed by bare `canonicalId`. An agent block stores its nested tools' modes scoped as
265-
* `${toolType}:${canonicalId}` (to avoid cross-tool collisions when tools share a `canonicalParamId`),
266-
* so this is the canonical un-scoping primitive. Returns `undefined` when there are no overrides, no
267-
* `toolType`, or no matching keys.
265+
* `${toolIndex}:${canonicalId}` — keyed by the tool's position in the `tool-input` array, not its
266+
* `type` — so that two tool entries of the SAME type (e.g. two Table tools on one Agent block) get
267+
* independent canonical modes instead of colliding on a shared `${toolType}:${canonicalId}` key. This
268+
* is the canonical un-scoping primitive. Returns `undefined` when there are no overrides, no
269+
* `toolIndex`, or no matching keys.
268270
*/
269271
export function scopeCanonicalModesForTool(
270272
overrides: CanonicalModeOverrides | undefined,
271-
toolType: string | undefined
273+
toolIndex: number | undefined
272274
): CanonicalModeOverrides | undefined {
273-
if (!overrides || !toolType) return undefined
274-
const prefix = `${toolType}:`
275+
if (!overrides || toolIndex === undefined) return undefined
276+
const prefix = `${toolIndex}:`
275277
let scoped: CanonicalModeOverrides | undefined
276278
for (const [key, value] of Object.entries(overrides)) {
277279
if (key.startsWith(prefix) && value) {

apps/sim/lib/workspaces/fork/mapping/dependent-reconfigs.test.ts

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ describe('collectForkDependentReconfigs', () => {
414414
return undefined as unknown as BlockConfig
415415
})
416416
// Agent block with a nested gmail tool; the dormant basic credential holds a stale id while the
417-
// tool-scoped `gmail:credential` override (when present) marks advanced as active.
417+
// tool-scoped `0:credential` override (when present) marks advanced as active.
418418
const agentState = (canonicalModes?: Record<string, 'basic' | 'advanced'>) =>
419419
({
420420
blocks: {
@@ -449,7 +449,7 @@ describe('collectForkDependentReconfigs', () => {
449449
// Scoped override present -> anchors on the ACTIVE advanced member (today's heuristic missed it).
450450
const withOverride = collectForkDependentReconfigs(
451451
[replaceItem],
452-
new Map([['wf-src', agentState({ 'gmail:credential': 'advanced' })]]),
452+
new Map([['wf-src', agentState({ '0:credential': 'advanced' })]]),
453453
resolve
454454
)
455455
expect(withOverride).toHaveLength(1)
@@ -472,6 +472,97 @@ describe('collectForkDependentReconfigs', () => {
472472
})
473473
})
474474

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

apps/sim/lib/workspaces/fork/mapping/dependent-reconfigs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ export function collectForkDependentReconfigs(
289289
contextSubBlocks: toolContextSubBlocks,
290290
blockName: block.name,
291291
targetWorkflowId: item.targetWorkflowId,
292-
canonicalModes: scopeCanonicalModesForTool(block.data?.canonicalModes, tool.type),
292+
canonicalModes: scopeCanonicalModesForTool(block.data?.canonicalModes, toolIndex),
293293
resolveTargetBlockId: resolveBlockId,
294294
makeSubBlockKey: (id) => `${toolInputKey}[${toolIndex}].${id}`,
295295
makeTitle: (dependent) => `${toolLabel}: ${dependent.title ?? dependent.id ?? ''}`,

0 commit comments

Comments
 (0)