Skip to content

Commit 1d18386

Browse files
committed
improvement(tools): scope pinned params to the selected tool and trim the prose
Follow-ups from review of the pinned-param descriptions. A block's subblocks span every operation it supports, so a Gmail block switched from Send to Read still holds `to`, `subject` and `body` — and the tool stated them as constraints on a read tool, leaking stale draft content into the prompt. Literals are now filtered to the selected tool's declared params. Resources are exempt: an OAuth credential is a block input that never appears in a tool's param map. MCP tools carry configured params but have no subblocks, so they registered nothing and their pinned values went unstated. They now collect from their configured params directly. The duplicate hint claimed the copies differ whenever a tool had a sibling, even when both rendered identical text. It now compares the rendered statements, so the model is never told to pick between indistinguishable copies. Also: reuse `isPasswordParameter` instead of a second secret regex, applied only to literals since it matches `oauthCredential`; make the field type a real union so a field cannot be both a literal and a resource; and cut the stated-field cap from six to three, since every field costs tokens on every request in the loop.
1 parent ed03bbf commit 1d18386

7 files changed

Lines changed: 429 additions & 401 deletions

File tree

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ import {
8383
getInlineHydrationMaxBytes,
8484
} from '@/providers/file-attachments.server'
8585
import { isAutoModel, SIM_AUTO_MODEL_ID } from '@/providers/models'
86+
import { collectPinnedFieldsFromParams, registerToolPinnedFields } from '@/providers/tool-binding'
8687
import {
8788
type ProviderToolInputProvenance,
8889
registerProviderToolInputProvenance,
@@ -808,11 +809,9 @@ export class AgentBlockHandler implements BlockHandler {
808809
const tools = allTools.filter(
809810
(tool): tool is ProviderToolConfig => tool !== null && tool !== undefined
810811
)
811-
await annotateToolPinnedParams(ctx, tools, {
812-
// A tool whose params resolved an environment secret must not have its literal values
813-
// stated; the provenance map already identifies exactly those tools.
814-
hasResolvedSecretInputs: (tool) => inputProvenance.has(tool),
815-
})
812+
// A tool whose params resolved an environment secret must not have its literal values stated;
813+
// the provenance map already identifies exactly those tools.
814+
await annotateToolPinnedParams(ctx, tools, (tool) => inputProvenance.has(tool))
816815
return { tools, inputProvenance }
817816
}
818817

@@ -1378,13 +1377,26 @@ export class AgentBlockHandler implements BlockHandler {
13781377
const filteredSchema = filterSchemaForLLM(config.schema, config.userProvidedParams)
13791378
const toolId = createMcpToolId(config.serverId, config.toolName)
13801379

1381-
return {
1380+
const mcpTool = {
13821381
id: toolId,
13831382
description: config.description,
13841383
parameters: filteredSchema,
13851384
params: config.userProvidedParams,
13861385
usageControl: config.usageControl || 'auto',
13871386
}
1387+
1388+
// An MCP tool has no block subblocks to describe it, so its pinned params are read straight
1389+
// from the configured values, keyed by the remote schema's own names.
1390+
const { formatParameterLabel, isPasswordParameter } = await import('@/tools/params')
1391+
registerToolPinnedFields(
1392+
mcpTool,
1393+
collectPinnedFieldsFromParams(config.userProvidedParams, {
1394+
formatParamLabel: formatParameterLabel,
1395+
isPasswordParam: isPasswordParameter,
1396+
})
1397+
)
1398+
1399+
return mcpTool
13881400
}
13891401

13901402
private async transformBlockTool(

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,13 +233,16 @@ export async function buildSimToolSpecs(
233233
}
234234

235235
const providers = configuredTools.map(({ provider }) => provider)
236-
// Pi resolves secret provenance per tool CALL rather than per format, so it cannot say which
237-
// individual tool carries one. Withhold literal values for the whole run when any input
236+
// Pi resolves secret provenance per tool CALL rather than per format, so at this point it cannot
237+
// say which individual tool carries one. Withhold literal values for the whole run when any input
238238
// resolved a secret — coarse, but it errs toward stating less.
239-
const runResolvedSecrets = Boolean(ctx.resolvedSecretTraceRegistry?.hasResolvedInputProjections())
240-
await annotateToolPinnedParams(ctx, providers, {
241-
hasResolvedSecretInputs: () => runResolvedSecrets,
242-
})
239+
const withholdLiteralValues = Boolean(
240+
ctx.resolvedSecretTraceRegistry?.hasResolvedInputProjections()
241+
)
242+
if (withholdLiteralValues) {
243+
logger.debug('Withholding pinned literal values: an input in this run resolved a secret')
244+
}
245+
await annotateToolPinnedParams(ctx, providers, () => withholdLiteralValues)
243246
assignProviderToolIdentities(providers)
244247
return configuredTools.map(({ provider, toolIndex }) =>
245248
buildSimToolSpec(ctx, inputTools, provider, toolIndex)

apps/sim/executor/utils/tool-pinned-params.test.ts

Lines changed: 60 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type { ProviderToolConfig } from '@/providers/types'
2222

2323
const WORKSPACE_ID = 'workspace-1'
2424
const BASE = 'Read emails from Gmail'
25+
const NAMES: Record<string, string> = { 'cred-a': 'Support Inbox', 'cred-b': 'Billing Inbox' }
2526

2627
function providerTool(id: string, fields: ToolPinnedField[] = []): ProviderToolConfig {
2728
const tool: ProviderToolConfig = {
@@ -34,60 +35,60 @@ function providerTool(id: string, fields: ToolPinnedField[] = []): ProviderToolC
3435
return tool
3536
}
3637

37-
const credentialField = (id: string): ToolPinnedField => ({
38-
paramId: 'oauthCredential',
38+
const account = (id: string): ToolPinnedField => ({
3939
title: 'Gmail Account',
4040
resource: { kind: 'credential', id },
4141
})
4242

43-
const folderField = (value: string): ToolPinnedField => ({
44-
paramId: 'folder',
45-
title: 'Label',
46-
value,
47-
quoted: true,
43+
const label = (value: string): ToolPinnedField => ({ title: 'Label', value })
44+
45+
const ctx = (cache?: Map<string, string | null>) => ({
46+
workspaceId: WORKSPACE_ID,
47+
toolBindingLabelCache: cache,
4848
})
4949

50-
function ctx(cache?: Map<string, string | null>) {
51-
return { workspaceId: WORKSPACE_ID, toolBindingLabelCache: cache }
52-
}
53-
54-
function credentialsByName(names: Record<string, string>) {
55-
return async ({ credentialId }: { credentialId: string }) =>
56-
names[credentialId] ? { id: credentialId, displayName: names[credentialId] } : null
57-
}
50+
/** Text appended after the base description, or '' when nothing was appended. */
51+
const appended = (tool: ProviderToolConfig) => tool.description.slice(BASE.length).trim()
5852

5953
describe('annotateToolPinnedParams', () => {
6054
beforeEach(() => {
6155
vi.clearAllMocks()
6256
mockGetKnowledgeBaseNames.mockResolvedValue(new Map())
63-
mockFindWorkspaceCredentialLookup.mockImplementation(
64-
credentialsByName({ 'cred-a': 'Support Inbox', 'cred-b': 'Billing Inbox' })
57+
mockFindWorkspaceCredentialLookup.mockImplementation(async ({ credentialId }) =>
58+
NAMES[credentialId] ? { id: credentialId, displayName: NAMES[credentialId] } : null
6559
)
6660
})
6761

6862
it('distinguishes two copies that share a credential but differ by folder', async () => {
69-
const inbox = providerTool('gmail_read_email', [
70-
credentialField('cred-a'),
71-
folderField('INBOX'),
72-
])
73-
const sent = providerTool('gmail_read_email', [credentialField('cred-a'), folderField('SENT')])
63+
const inbox = providerTool('gmail_read_email', [account('cred-a'), label('INBOX')])
64+
const sent = providerTool('gmail_read_email', [account('cred-a'), label('SENT')])
7465

7566
await annotateToolPinnedParams(ctx(), [inbox, sent])
7667

77-
expect(inbox.description).toContain('Gmail Account "Support Inbox", Label "INBOX".')
78-
expect(sent.description).toContain('Gmail Account "Support Inbox", Label "SENT".')
79-
expect(inbox.description).toContain('This agent has 2 copies of this tool')
80-
expect(inbox.description).not.toBe(sent.description)
68+
expect(appended(inbox)).toContain('Gmail Account "Support Inbox", Label "INBOX".')
69+
expect(appended(sent)).toContain('Gmail Account "Support Inbox", Label "SENT".')
70+
expect(appended(inbox)).toContain('Other copies of this tool')
71+
})
72+
73+
it('does not claim copies differ when they render identically', async () => {
74+
const first = providerTool('gmail_read_email', [account('cred-a'), label('INBOX')])
75+
const second = providerTool('gmail_read_email', [account('cred-a'), label('INBOX')])
76+
77+
await annotateToolPinnedParams(ctx(), [first, second])
78+
79+
expect(appended(first)).toContain('Label "INBOX".')
80+
expect(appended(first)).not.toContain('Other copies')
81+
expect(appended(second)).not.toContain('Other copies')
8182
})
8283

8384
it('states pinned params on a single tool so the model knows what it cannot change', async () => {
84-
const only = providerTool('gmail_read_email', [folderField('INBOX')])
85+
const only = providerTool('gmail_read_email', [label('INBOX')])
8586

8687
await annotateToolPinnedParams(ctx(), [only])
8788

88-
expect(only.description).toContain('Pinned by the workflow and not changeable per call')
89-
expect(only.description).toContain('Label "INBOX".')
90-
expect(only.description).not.toContain('copies of this tool')
89+
expect(appended(only)).toBe(
90+
'Pinned by the workflow and not changeable per call: Label "INBOX".'
91+
)
9192
})
9293

9394
it('leaves a tool with no pinned fields untouched and issues no lookup', async () => {
@@ -99,69 +100,59 @@ describe('annotateToolPinnedParams', () => {
99100
expect(mockFindWorkspaceCredentialLookup).not.toHaveBeenCalled()
100101
})
101102

102-
it('resolves an opaque credential id to its display name without leaking the id', async () => {
103-
const first = providerTool('gmail_read_email', [credentialField('cred-a')])
104-
const second = providerTool('gmail_read_email', [credentialField('cred-b')])
103+
it('resolves an opaque credential id to its name without leaking the id', async () => {
104+
const first = providerTool('gmail_read_email', [account('cred-a')])
105+
const second = providerTool('gmail_read_email', [account('cred-b')])
105106

106107
await annotateToolPinnedParams(ctx(), [first, second])
107108

108-
expect(first.description).toContain('Gmail Account "Support Inbox"')
109-
expect(second.description).toContain('Gmail Account "Billing Inbox"')
109+
expect(appended(first)).toContain('Gmail Account "Support Inbox"')
110+
expect(appended(second)).toContain('Gmail Account "Billing Inbox"')
110111
expect(first.description).not.toContain('cred-a')
111112
expect(second.description).not.toContain('cred-b')
112113
})
113114

114115
it('omits an unresolvable resource but still states the other fields', async () => {
115-
const tool = providerTool('gmail_read_email', [
116-
credentialField('cred-deleted'),
117-
folderField('INBOX'),
118-
])
116+
const tool = providerTool('gmail_read_email', [account('cred-deleted'), label('INBOX')])
119117

120118
await annotateToolPinnedParams(ctx(), [tool])
121119

122-
expect(tool.description).toContain('Label "INBOX".')
123-
expect(tool.description).not.toContain('Gmail Account')
120+
expect(appended(tool)).toContain('Label "INBOX".')
121+
expect(appended(tool)).not.toContain('Gmail Account')
124122
expect(tool.description).not.toContain('cred-deleted')
125123
})
126124

127125
it('withholds literal values for a tool whose params resolved a secret', async () => {
128-
const tool = providerTool('gmail_read_email', [
129-
credentialField('cred-a'),
130-
folderField('SecretFolderName'),
131-
])
126+
const tool = providerTool('gmail_read_email', [account('cred-a'), label('SecretFolder')])
132127

133-
await annotateToolPinnedParams(ctx(), [tool], { hasResolvedSecretInputs: () => true })
128+
await annotateToolPinnedParams(ctx(), [tool], () => true)
134129

135-
expect(tool.description).toContain('Gmail Account "Support Inbox".')
136-
expect(tool.description).not.toContain('SecretFolderName')
130+
expect(appended(tool)).toContain('Gmail Account "Support Inbox".')
131+
expect(tool.description).not.toContain('SecretFolder')
137132
})
138133

139-
it('adds nothing at all when every field of a secret-bearing tool is a literal', async () => {
140-
const tool = providerTool('gmail_read_email', [folderField('SecretFolderName')])
134+
it('adds nothing when every field of a secret-bearing tool is a literal', async () => {
135+
const tool = providerTool('gmail_read_email', [label('SecretFolder')])
141136

142-
await annotateToolPinnedParams(ctx(), [tool], { hasResolvedSecretInputs: () => true })
137+
await annotateToolPinnedParams(ctx(), [tool], () => true)
143138

144139
expect(tool.description).toBe(BASE)
145140
})
146141

147142
it('degrades to no resource name when a resolver throws', async () => {
148143
mockFindWorkspaceCredentialLookup.mockRejectedValue(new Error('db down'))
149-
const tool = providerTool('gmail_read_email', [credentialField('cred-a'), folderField('INBOX')])
144+
const tool = providerTool('gmail_read_email', [account('cred-a'), label('INBOX')])
150145

151146
await expect(annotateToolPinnedParams(ctx(), [tool])).resolves.toBeUndefined()
152147

153-
expect(tool.description).toContain('Label "INBOX".')
154-
expect(tool.description).not.toContain('Gmail Account')
148+
expect(appended(tool)).toContain('Label "INBOX".')
149+
expect(appended(tool)).not.toContain('Gmail Account')
155150
})
156151

157152
it('omits a knowledge base belonging to another workspace', async () => {
158153
mockGetKnowledgeBaseNames.mockResolvedValue(new Map([['kb-a', 'Support Docs']]))
159154
const foreign = providerTool('knowledge_search', [
160-
{
161-
paramId: 'knowledgeBaseId',
162-
title: 'Knowledge Base',
163-
resource: { kind: 'knowledgeBase', id: 'kb-foreign' },
164-
},
155+
{ title: 'Knowledge Base', resource: { kind: 'knowledgeBase', id: 'kb-foreign' } },
165156
])
166157

167158
await annotateToolPinnedParams(ctx(), [foreign])
@@ -171,25 +162,22 @@ describe('annotateToolPinnedParams', () => {
171162
})
172163

173164
it('caps how many fields it states', async () => {
174-
const many = Array.from({ length: 10 }, (_, index) => ({
175-
paramId: `p${index}`,
176-
title: `Field ${index}`,
177-
value: String(index),
178-
quoted: false,
179-
}))
180-
const tool = providerTool('gmail_read_email', many)
165+
const tool = providerTool(
166+
'gmail_read_email',
167+
Array.from({ length: 10 }, (_, index) => ({ title: `F${index}`, value: index }))
168+
)
181169

182170
await annotateToolPinnedParams(ctx(), [tool])
183171

184-
expect(tool.description).toContain('Field 5 5.')
185-
expect(tool.description).not.toContain('Field 6')
172+
expect(appended(tool)).toContain('F0 0, F1 1, F2 2.')
173+
expect(appended(tool)).not.toContain('F3')
186174
})
187175

188176
it('resolves each distinct credential once and reuses the run cache', async () => {
189177
const cache = new Map<string, string | null>()
190178
const build = () => [
191-
providerTool('gmail_read_email', [credentialField('cred-a')]),
192-
providerTool('gmail_send', [credentialField('cred-a')]),
179+
providerTool('gmail_read_email', [account('cred-a')]),
180+
providerTool('gmail_send', [account('cred-a')]),
193181
]
194182

195183
await annotateToolPinnedParams(ctx(cache), build())
@@ -199,27 +187,14 @@ describe('annotateToolPinnedParams', () => {
199187
await annotateToolPinnedParams(ctx(cache), second)
200188

201189
expect(mockFindWorkspaceCredentialLookup).toHaveBeenCalledTimes(1)
202-
expect(second[0].description).toContain('Gmail Account "Support Inbox"')
190+
expect(appended(second[0])).toContain('Gmail Account "Support Inbox"')
203191
})
204192

205193
it('does nothing without a workspace', async () => {
206-
const tool = providerTool('gmail_read_email', [folderField('INBOX')])
194+
const tool = providerTool('gmail_read_email', [label('INBOX')])
207195

208196
await annotateToolPinnedParams({ workspaceId: undefined }, [tool])
209197

210198
expect(tool.description).toBe(BASE)
211199
})
212-
213-
it('annotates the exact objects it was given', async () => {
214-
const tools = [
215-
providerTool('gmail_read_email', [folderField('INBOX')]),
216-
providerTool('gmail_read_email', [folderField('SENT')]),
217-
]
218-
const [first, second] = tools
219-
220-
await annotateToolPinnedParams(ctx(), tools)
221-
222-
expect(tools[0]).toBe(first)
223-
expect(tools[1]).toBe(second)
224-
})
225200
})

0 commit comments

Comments
 (0)