Skip to content

Commit f00a99a

Browse files
committed
improvement(tools): decide pinned fields by subblock condition, not param name
The membership check asked a proxy question — "does this subblock's id match a declared tool param?" — when the real one is "is this field part of the operation the tool was selected for". 41 blocks rename a field on its way to the tool inside `tools.config.params`, so their pinned values failed the name match and were never stated. Datadog's `listMonitorName` feeds the tool param `name`; it is now stated as Filter by Name "CPU" instead of dropped. `evaluateSubBlockCondition` answers the real question directly and survives a rename, because it never looks at tool param names. It also still excludes the stale-field case the name match was introduced for: Gmail's to/subject/body are gated to the send operations, so a block switched to Read drops them. The operation selector and trigger-mode subblocks are excluded explicitly — they carry values but do not constrain the call. Trigger mode is skipped per subblock rather than blocking its canonical group. Gmail puts `triggerCredentials` in the same group as `credential`, and blocking the group dropped the account from every Gmail tool. Only value-level disqualifiers — password, hidden, unstateable type — block a whole group, since a canonical group shares one value.
1 parent 31a8f8e commit f00a99a

3 files changed

Lines changed: 131 additions & 15 deletions

File tree

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

Lines changed: 91 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const collect = (over: Partial<CollectInput>) =>
2929
subBlocks: [],
3030
userProvidedParams: {},
3131
resolvedResourceParams: {},
32+
conditionValues: {},
3233
...sourceOptions,
3334
...over,
3435
} as CollectInput)
@@ -127,18 +128,101 @@ describe('collectToolPinnedFields', () => {
127128
it('omits a field left over from a different operation on the same block', () => {
128129
const fields = collect({
129130
subBlocks: [
130-
sub({ id: 'folder', title: 'Label', type: 'folder-selector' }),
131-
sub({ id: 'to', title: 'To', type: 'short-input' }),
132-
sub({ id: 'body', title: 'Body', type: 'long-input' }),
131+
sub({
132+
id: 'folder',
133+
title: 'Label',
134+
type: 'folder-selector',
135+
condition: { field: 'operation', value: 'read_gmail' },
136+
}),
137+
sub({
138+
id: 'to',
139+
title: 'To',
140+
type: 'short-input',
141+
condition: { field: 'operation', value: ['send_gmail', 'draft_gmail'] },
142+
}),
143+
sub({
144+
id: 'body',
145+
title: 'Body',
146+
type: 'long-input',
147+
condition: { field: 'operation', value: ['send_gmail', 'draft_gmail'] },
148+
}),
133149
],
134150
// A block switched from Send to Read keeps the send fields in its params.
135151
userProvidedParams: { folder: 'INBOX', to: 'someone@example.com', body: 'stale draft' },
136-
toolParams: toolParams('folder', 'unreadOnly', 'maxResults'),
152+
toolParams: toolParams('folder'),
153+
conditionValues: { operation: 'read_gmail', folder: 'INBOX' },
137154
})
138155

139156
expect(fields).toEqual([{ title: 'Label', value: 'INBOX' }])
140157
})
141158

159+
it('states a field the block renames on its way to the tool', () => {
160+
// Datadog's `listMonitorName` subblock feeds the tool param `name`. Matching against the
161+
// tool's declared params would drop it; the subblock's own condition does not.
162+
const fields = collect({
163+
subBlocks: [
164+
sub({
165+
id: 'listMonitorName',
166+
title: 'Filter by Name',
167+
type: 'short-input',
168+
condition: { field: 'operation', value: 'datadog_list_monitors' },
169+
}),
170+
],
171+
userProvidedParams: { listMonitorName: 'CPU' },
172+
toolParams: toolParams('name', 'tags', 'page'),
173+
conditionValues: { operation: 'datadog_list_monitors', listMonitorName: 'CPU' },
174+
})
175+
176+
expect(fields).toEqual([{ title: 'Filter by Name', value: 'CPU' }])
177+
})
178+
179+
it('never states the operation selector itself', () => {
180+
expect(
181+
collect({
182+
subBlocks: [sub({ id: 'operation', title: 'Operation', type: 'dropdown' })],
183+
userProvidedParams: { operation: 'read_gmail' },
184+
conditionValues: { operation: 'read_gmail' },
185+
})
186+
).toEqual([])
187+
})
188+
189+
it('still states the action field when a trigger sibling shares its canonical group', () => {
190+
// Gmail puts `triggerCredentials` in the same canonical group as `credential`. A trigger
191+
// sibling is a different surface, not a statement about the value, so it must not block it.
192+
expect(
193+
collect({
194+
subBlocks: [
195+
sub({
196+
id: 'credential',
197+
title: 'Gmail Account',
198+
type: 'oauth-input',
199+
canonicalParamId: 'oauthCredential',
200+
}),
201+
sub({
202+
id: 'triggerCredentials',
203+
title: 'Gmail Account',
204+
type: 'oauth-input',
205+
mode: 'trigger',
206+
canonicalParamId: 'oauthCredential',
207+
}),
208+
],
209+
resolvedResourceParams: { oauthCredential: 'cred-a' },
210+
})
211+
).toEqual([{ title: 'Gmail Account', resource: { kind: 'credential', id: 'cred-a' } }])
212+
})
213+
214+
it('never states a trigger-mode field', () => {
215+
expect(
216+
collect({
217+
subBlocks: [
218+
sub({ id: 'selectedTriggerId', title: 'Trigger', type: 'short-input', mode: 'trigger' }),
219+
],
220+
userProvidedParams: { selectedTriggerId: 'gmail_new_email' },
221+
conditionValues: {},
222+
})
223+
).toEqual([])
224+
})
225+
142226
it('respects a hidden tool-param declaration', () => {
143227
expect(
144228
collect({
@@ -226,13 +310,14 @@ describe('collectToolPinnedFields', () => {
226310
}
227311
})
228312

229-
it('states nothing when the caller omits the tool param map', () => {
313+
it('states an unconditional field even when the tool does not declare it', () => {
314+
// No condition means the field applies to every operation the block supports.
230315
expect(
231316
collect({
232317
subBlocks: [sub({ id: 'folder', title: 'Label', type: 'folder-selector' })],
233318
userProvidedParams: { folder: 'INBOX' },
234319
})
235-
).toEqual([])
320+
).toEqual([{ title: 'Label', value: 'INBOX' }])
236321
})
237322

238323
it('drops a field whose title sanitizes to nothing', () => {

apps/sim/providers/tool-binding.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { truncate } from '@sim/utils/string'
22
import type { SubBlockType } from '@sim/workflow-types/blocks'
3+
import {
4+
evaluateSubBlockCondition,
5+
isTriggerModeSubBlock,
6+
} from '@/lib/workflows/subblocks/visibility'
37
import type { SubBlockConfig } from '@/blocks/types'
48
import { isNonEmpty } from '@/tools/merge-params'
59

@@ -51,6 +55,12 @@ const UNSTATEABLE_SUBBLOCK_TYPES: ReadonlySet<string> = new Set([
5155
'secrets-management',
5256
])
5357

58+
/**
59+
* The operation selector itself. It is a real subblock with a value, but it names the tool rather
60+
* than constraining it, so stating it would only repeat what the tool id already says.
61+
*/
62+
const OPERATION_PARAM_ID = 'operation'
63+
5464
/**
5565
* Secret-ish names `isPasswordParameter` does not cover. It is tuned to Sim-authored param ids
5666
* (`password`, `apiKey`, `token`, `secret`, `key`, `credential`, …), but this module also states
@@ -154,11 +164,13 @@ interface CollectToolPinnedFieldsInput extends PinnedFieldSourceOptions {
154164
userProvidedParams: Record<string, unknown>
155165
/** Params after canonical basic/advanced pairs have collapsed onto their canonical id. */
156166
resolvedResourceParams: Record<string, unknown>
167+
/** The selected tool's declared params, consulted only for `hidden` visibility. */
168+
toolParams?: Record<string, { visibility?: string } | undefined>
157169
/**
158-
* The selected tool's declared params. A block's subblocks span every operation it supports, so
159-
* this is what keeps a field left over from another operation out of this tool's description.
170+
* Values the subblock conditions are evaluated against — the configured params plus the selected
171+
* operation, matching how the block's own tool selector resolves them.
160172
*/
161-
toolParams?: Record<string, { visibility?: string } | undefined>
173+
conditionValues: Record<string, unknown>
162174
/** `toolEnrichment.dependsOn`, when the tool rewrote its own description from that param. */
163175
selfDescribedParamId?: string
164176
/** Name for a `workflow` field the caller already fetched. */
@@ -182,6 +194,7 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
182194
selfDescribedParamId,
183195
workflowLabel,
184196
formatParamLabel,
197+
conditionValues,
185198
} = input
186199
if (!subBlocks?.length) return []
187200

@@ -199,6 +212,10 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
199212
const paramId = subBlock.canonicalParamId ?? subBlock.id
200213
const kind = RESOURCE_SUBBLOCK_KINDS[subBlock.type]
201214
if (kind) kindByParamId.set(paramId, kind)
215+
// Only value-level disqualifiers block the whole group: a canonical group shares one value, so
216+
// if any half calls it secret or unstateable, the value is. Trigger mode is a property of the
217+
// SURFACE, not the value — several blocks put a trigger-mode credential in the same canonical
218+
// group as the action one — so it is skipped per subblock below instead.
202219
if (subBlock.password || subBlock.hidden || UNSTATEABLE_SUBBLOCK_TYPES.has(subBlock.type)) {
203220
blockedParamIds.add(paramId)
204221
}
@@ -208,6 +225,10 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
208225
const seenParamIds = new Set<string>()
209226

210227
for (const subBlock of subBlocks) {
228+
// Not a candidate, and deliberately without claiming the param: an action-mode sibling in the
229+
// same canonical group still has to be considered.
230+
if (isTriggerModeSubBlock(subBlock)) continue
231+
211232
const paramId = subBlock.canonicalParamId ?? subBlock.id
212233
if (seenParamIds.has(paramId)) continue
213234
if (selfDescribedParamId && paramId === selfDescribedParamId) continue
@@ -216,15 +237,19 @@ export function collectToolPinnedFields(input: CollectToolPinnedFieldsInput): To
216237

217238
const kind = kindByParamId.get(paramId)
218239

219-
// Both checks apply only to a literal. A resource never reaches the model as its configured
220-
// value — only as a name looked up from it — so the secret-name heuristic would misfire
240+
// These apply only to a literal. A resource never reaches the model as its configured value —
241+
// only as a name looked up from it — so the secret-name heuristic would misfire here
221242
// (`isPasswordParameter` matches `oauthCredential`), and a resource is a block-level input
222-
// that is legitimately absent from the tool's own params. Anything else must belong to the
223-
// selected tool, or it is left over from a different operation on the same block.
243+
// legitimately absent from the tool's own params.
224244
if (!kind) {
245+
if (paramId === OPERATION_PARAM_ID) continue
225246
if (isSecretParamId(paramId, input)) continue
226-
const declared = toolParams?.[paramId]
227-
if (!declared || declared.visibility === 'hidden') continue
247+
if (toolParams?.[paramId]?.visibility === 'hidden') continue
248+
// A block's subblocks span every operation it supports, so a field belonging to a different
249+
// operation must not be advertised as this tool's constraint. The subblock's own condition is
250+
// exactly that statement, and unlike matching against the tool's param names it survives a
251+
// block that renames a field on its way to the tool.
252+
if (!evaluateSubBlockCondition(subBlock.condition, conditionValues)) continue
228253
}
229254

230255
const raw = subBlock.canonicalParamId

apps/sim/providers/utils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,12 @@ export async function transformBlockTool(
910910
userProvidedParams,
911911
resolvedResourceParams,
912912
toolParams: toolConfig.params,
913+
// Matches how the block's own tool selector resolves the operation (see `tools.config.tool`
914+
// above): stored params, with the agent's selected operation taking precedence.
915+
conditionValues: {
916+
...userProvidedParams,
917+
...(selectedOperation ? { operation: selectedOperation } : {}),
918+
},
913919
selfDescribedParamId,
914920
workflowLabel,
915921
formatParamLabel: formatParameterLabel,

0 commit comments

Comments
 (0)