Skip to content

Commit 926cffc

Browse files
committed
fix(tools): cover duplicate Pi tool instances
1 parent f7a9cbe commit 926cffc

4 files changed

Lines changed: 143 additions & 96 deletions

File tree

apps/sim/executor/handlers/pi/local/sim-tools.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,43 @@ describe('buildSimToolSpecs', () => {
7171
expect(specs[0].name).toMatch(/^[a-zA-Z0-9_-]{1,128}$/)
7272
})
7373

74+
it('aliases duplicate instances while executing each with its canonical id and bound params', async () => {
75+
mockTransformBlockTool
76+
.mockResolvedValueOnce({
77+
id: 'gmail_send',
78+
name: 'Gmail Send',
79+
description: 'Send an email',
80+
params: { oauthCredential: 'credential-a' },
81+
parameters: { type: 'object', properties: {} },
82+
})
83+
.mockResolvedValueOnce({
84+
id: 'gmail_send',
85+
name: 'Gmail Send',
86+
description: 'Send an email',
87+
params: { oauthCredential: 'credential-b' },
88+
parameters: { type: 'object', properties: {} },
89+
})
90+
mockExecuteTool.mockResolvedValue({ success: true, output: 'sent' })
91+
92+
const specs = await buildSimToolSpecs(executionContext(undefined), [
93+
{ type: 'gmail', operation: 'send', usageControl: 'auto' },
94+
{ type: 'gmail', operation: 'send', usageControl: 'auto' },
95+
])
96+
97+
expect(specs.map(({ name }) => name)).toEqual(['gmail_send', 'gmail_send__sim_2'])
98+
99+
await specs[1].execute({ subject: 'Hello' })
100+
101+
expect(mockExecuteTool).toHaveBeenCalledWith(
102+
'gmail_send',
103+
expect.objectContaining({
104+
oauthCredential: 'credential-b',
105+
subject: 'Hello',
106+
}),
107+
expect.any(Object)
108+
)
109+
})
110+
74111
it('skips mcp, custom, and usage-none tools without adapting them', async () => {
75112
const specs = await buildSimToolSpecs(completeExecutionContext(), [
76113
{ type: 'mcp', usageControl: 'auto' },

apps/sim/executor/handlers/pi/local/sim-tools.ts

Lines changed: 98 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import type { PiToolResult, PiToolSpec } from '@/executor/handlers/pi/core/backe
1616
import type { ExecutionContext } from '@/executor/types'
1717
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
1818
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
19+
import { assignProviderToolIdentities } from '@/providers/tool-identity'
20+
import type { ProviderToolConfig } from '@/providers/types'
1921
import { transformBlockTool } from '@/providers/utils'
2022
import { executeTool } from '@/tools'
2123
import { mergeToolParameters } from '@/tools/merge-params'
@@ -96,6 +98,96 @@ function projectToolResult(
9698
}
9799
}
98100

101+
function buildSimToolSpec(
102+
ctx: ExecutionContext,
103+
inputTools: ToolInput[],
104+
provider: ProviderToolConfig,
105+
toolIndex: number
106+
): PiToolSpec {
107+
const toolId = provider.canonicalId ?? provider.id
108+
const preseededParams = provider.params || {}
109+
110+
return {
111+
name: provider.id,
112+
description: provider.description || '',
113+
parameters: (provider.parameters as Record<string, unknown>) || {
114+
type: 'object',
115+
properties: {},
116+
},
117+
execute: async (args) => {
118+
const params = mergeToolParameters(preseededParams, args as Record<string, unknown>)
119+
const registry = ctx.resolvedSecretTraceRegistry
120+
const sourcePath = ['tools', String(toolIndex), 'params'] as const
121+
const toolCallRegistry = registry?.forkForInputPaths([sourcePath], {
122+
propagated: true,
123+
})
124+
if (toolCallRegistry && !toolCallRegistry.isComplete()) {
125+
return unavailableToolResult()
126+
}
127+
128+
if (toolCallRegistry) {
129+
const inputProjection = toolCallRegistry.projectResolvedInputSelection({
130+
tools: inputTools,
131+
})
132+
const projectedTool = inputProjection.complete
133+
? (inputProjection.value.tools as ToolInput[] | undefined)?.[toolIndex]
134+
: undefined
135+
if (!inputProjection.complete || !projectedTool) {
136+
return unavailableToolResult()
137+
}
138+
const projectedParams = mergeToolParameters(
139+
projectedTool.params || {},
140+
args as Record<string, unknown>
141+
)
142+
toolCallRegistry.recordTransformedInputProjection(params, projectedParams)
143+
if (!toolCallRegistry.isComplete()) return unavailableToolResult()
144+
}
145+
146+
try {
147+
const result = await executeTool(
148+
toolId,
149+
{
150+
// User-preseeded values win over model arguments, and inputMapping is deep-merged.
151+
...params,
152+
// Trusted execution context is written last so model arguments cannot override it.
153+
_context: {
154+
workflowId: ctx.workflowId,
155+
workspaceId: ctx.workspaceId,
156+
executionId: ctx.executionId,
157+
userId: ctx.userId,
158+
isDeployedContext: ctx.isDeployedContext,
159+
enforceCredentialAccess: ctx.enforceCredentialAccess,
160+
callChain: ctx.callChain,
161+
},
162+
},
163+
{
164+
executionContext: ctx,
165+
resolvedSecretTraceRegistry: toolCallRegistry,
166+
}
167+
)
168+
const projection = projectToolResult(result, toolCallRegistry?.forkForPropagatedEntries())
169+
if (projection.safe && registry && toolCallRegistry?.isComplete()) {
170+
registry.mergeToolCallRegistry(toolCallRegistry)
171+
}
172+
return projection.result
173+
} catch (error) {
174+
const projection = projectToolResult(
175+
{
176+
success: false,
177+
output: {},
178+
error: getErrorMessage(error, 'Tool execution failed'),
179+
},
180+
toolCallRegistry?.forkForPropagatedEntries()
181+
)
182+
if (projection.safe && registry && toolCallRegistry?.isComplete()) {
183+
registry.mergeToolCallRegistry(toolCallRegistry)
184+
}
185+
return projection.result
186+
}
187+
},
188+
}
189+
}
190+
99191
/**
100192
* Builds the Sim tool specs exposed to Pi for a local run. Only tools the user
101193
* added to the block are included, and `usageControl: 'none'` tools are dropped.
@@ -106,7 +198,7 @@ export async function buildSimToolSpecs(
106198
): Promise<PiToolSpec[]> {
107199
if (!Array.isArray(inputTools)) return []
108200

109-
const specs: PiToolSpec[] = []
201+
const configuredTools: Array<{ provider: ProviderToolConfig; toolIndex: number }> = []
110202

111203
for (const [toolIndex, tool] of (inputTools as ToolInput[]).entries()) {
112204
if ((tool.usageControl || 'auto') === 'none') continue
@@ -129,98 +221,7 @@ export async function buildSimToolSpecs(
129221
})
130222

131223
if (!provider?.id) continue
132-
133-
const toolId = provider.id
134-
const preseededParams = provider.params || {}
135-
136-
specs.push({
137-
name: toolId,
138-
description: provider.description || '',
139-
parameters: (provider.parameters as Record<string, unknown>) || {
140-
type: 'object',
141-
properties: {},
142-
},
143-
execute: async (args) => {
144-
const params = mergeToolParameters(preseededParams, args as Record<string, unknown>)
145-
const registry = ctx.resolvedSecretTraceRegistry
146-
const sourcePath = ['tools', String(toolIndex), 'params'] as const
147-
const toolCallRegistry = registry?.forkForInputPaths([sourcePath], {
148-
propagated: true,
149-
})
150-
if (toolCallRegistry && !toolCallRegistry.isComplete()) {
151-
return unavailableToolResult()
152-
}
153-
154-
if (toolCallRegistry) {
155-
const inputProjection = toolCallRegistry.projectResolvedInputSelection({
156-
tools: inputTools,
157-
})
158-
const projectedTool = inputProjection.complete
159-
? (inputProjection.value.tools as ToolInput[] | undefined)?.[toolIndex]
160-
: undefined
161-
if (!inputProjection.complete || !projectedTool) {
162-
return unavailableToolResult()
163-
}
164-
const projectedParams = mergeToolParameters(
165-
projectedTool.params || {},
166-
args as Record<string, unknown>
167-
)
168-
toolCallRegistry.recordTransformedInputProjection(params, projectedParams)
169-
if (!toolCallRegistry.isComplete()) return unavailableToolResult()
170-
}
171-
172-
try {
173-
const result = await executeTool(
174-
toolId,
175-
{
176-
// Same merge the Agent block's tool calls use: user-preseeded values
177-
// win over LLM args, and `inputMapping` is deep-merged rather than
178-
// replaced — a partial mapping from the model must not drop the
179-
// user-filled fields baked onto the block.
180-
...params,
181-
// Trusted execution context, spread last so an LLM-supplied
182-
// `_context` arg can't override it. executeTool reads this directly
183-
// for OAuth-credential resolution and internal-route identity, the
184-
// same way the Agent block's tool calls do.
185-
_context: {
186-
workflowId: ctx.workflowId,
187-
workspaceId: ctx.workspaceId,
188-
executionId: ctx.executionId,
189-
userId: ctx.userId,
190-
isDeployedContext: ctx.isDeployedContext,
191-
enforceCredentialAccess: ctx.enforceCredentialAccess,
192-
callChain: ctx.callChain,
193-
},
194-
},
195-
{
196-
executionContext: ctx,
197-
resolvedSecretTraceRegistry: toolCallRegistry,
198-
}
199-
)
200-
const projection = projectToolResult(
201-
result,
202-
toolCallRegistry?.forkForPropagatedEntries()
203-
)
204-
if (projection.safe && registry && toolCallRegistry?.isComplete()) {
205-
registry.mergeToolCallRegistry(toolCallRegistry)
206-
}
207-
return projection.result
208-
} catch (error) {
209-
const projection = projectToolResult(
210-
{
211-
success: false,
212-
output: {},
213-
error: getErrorMessage(error, 'Tool execution failed'),
214-
},
215-
toolCallRegistry?.forkForPropagatedEntries()
216-
)
217-
if (projection.safe && registry && toolCallRegistry?.isComplete()) {
218-
registry.mergeToolCallRegistry(toolCallRegistry)
219-
}
220-
return projection.result
221-
}
222-
},
223-
})
224+
configuredTools.push({ provider, toolIndex })
224225
} catch (error) {
225226
if (error instanceof ToolSchemaEnrichmentError) throw error
226227
logger.warn('Failed to adapt Sim tool for Pi', {
@@ -230,5 +231,8 @@ export async function buildSimToolSpecs(
230231
}
231232
}
232233

233-
return specs
234+
assignProviderToolIdentities(configuredTools.map(({ provider }) => provider))
235+
return configuredTools.map(({ provider, toolIndex }) =>
236+
buildSimToolSpec(ctx, inputTools, provider, toolIndex)
237+
)
234238
}

apps/sim/providers/tool-identity.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,15 @@ describe('provider tool identities', () => {
134134
timeSegments: [],
135135
},
136136
}
137+
let settleStream: (() => void) | undefined
138+
const canSettle = new Promise<void>((resolve) => {
139+
settleStream = resolve
140+
})
137141
const response: StreamingExecution = {
138142
streamFormat: 'agent-events-v1',
139143
stream: new ReadableStream<AgentStreamEvent>({
140-
start(controller) {
144+
async pull(controller) {
145+
await canSettle
141146
controller.enqueue({ type: 'tool_call_start', id: 'call-1', name: alias })
142147
output.toolCalls = { list: [{ name: alias }], count: 1 }
143148
output.providerTiming?.timeSegments?.push({
@@ -160,6 +165,7 @@ describe('provider tool identities', () => {
160165
}
161166

162167
projectStreamingExecutionToolIdentities(response, identities)
168+
settleStream?.()
163169
const events = await readEvents(response.stream)
164170

165171
expect(events).toEqual([

apps/sim/tools/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1732,7 +1732,7 @@ async function executeToolImplementation(
17321732
contextParams.credential = contextParams.oauthCredential
17331733
}
17341734
if (contextParams.credential) {
1735-
logger.info(`[${requestId}] Resolving tool access token`, { toolId })
1735+
logger.info(`[${requestId}] Resolving tool access token`, { toolId: normalizedToolId })
17361736
try {
17371737
const workflowId = scope.workflowId
17381738
const userId = scope.userId

0 commit comments

Comments
 (0)