Skip to content

Commit e1e66eb

Browse files
icecrasher321claude
andcommitted
fix(tool-input): address review round 2 — enum shape and legacy MCP arg values
- `getJsonSchemaValueShape` checked the enum branch before the declared type, so `{ type: 'integer', enum: [1,2,3] }` was shaped as text. The dropdown it renders as stores `String(option)`, so the server received '1' instead of 1. The declared type now wins, and an untyped enum is inferred from its members. - An MCP argument entered before its control was derived from the NORMALIZED schema type is stored as a string, so a union-typed boolean's 'false' would tick the switch it now renders as. `mcp-dynamic-args` decodes on read; already typed values pass through, so it is a no-op thereafter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c1b50e2 commit e1e66eb

3 files changed

Lines changed: 75 additions & 13 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflow
1212
import type { SubBlockConfig } from '@/blocks/types'
1313
import { useMcpTools } from '@/hooks/mcp/use-mcp-tools'
1414
import {
15+
decodeToolParamValue,
16+
getJsonSchemaValueShape,
1517
type JsonSchemaProperty,
1618
jsonSchemaType,
1719
subBlockTypeForJsonSchema,
@@ -235,7 +237,11 @@ export function McpDynamicArgs({
235237

236238
const renderParameterInput = (paramName: string, paramSchema: any) => {
237239
const current = currentArgs()
238-
const value = current[paramName]
240+
// An argument entered before this control was derived from the NORMALIZED schema type
241+
// was collected by a text field, so it is stored as a string — `'false'` would tick the
242+
// switch it now renders as. Decoding on read normalizes it to the shape the control
243+
// expects; already-typed values pass through untouched, so this is a no-op thereafter.
244+
const value = decodeToolParamValue(current[paramName], getJsonSchemaValueShape(paramSchema))
239245
const inputType = getInputType(paramSchema)
240246

241247
switch (inputType) {
@@ -262,6 +268,9 @@ export function McpDynamicArgs({
262268
label: String(option),
263269
value: String(option),
264270
}))
271+
// Options are stringified members, so a decoded numeric/boolean enum value has to
272+
// be stringified back to match one.
273+
const dropdownValue = value === undefined || value === null ? '' : String(value)
265274
const selectedLabel = value ? String(value) : ''
266275
const workflowSearchHighlight = getWorkflowSearchLabelHighlight({
267276
activeSearchTarget,
@@ -275,8 +284,8 @@ export function McpDynamicArgs({
275284
<div key={`${paramName}-dropdown`}>
276285
<Combobox
277286
options={dropdownOptions}
278-
value={value || ''}
279-
selectedValue={value || ''}
287+
value={dropdownValue}
288+
selectedValue={dropdownValue}
280289
onChange={(selectedValue) => {
281290
const matchedOption = dropdownOptions.find(
282291
(opt: { label: string; value: string }) => opt.value === selectedValue

apps/sim/tools/param-shape.test.ts

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
decodeToolParamValue,
1212
encodeToolParamValue,
1313
expandSubBlockValueToParams,
14+
getJsonSchemaValueShape,
1415
getSubBlockValueShape,
1516
getToolParamValueShape,
1617
subBlockTypeForValueType,
@@ -512,12 +513,55 @@ describe('buildJsonSchemaParamShapes', () => {
512513
expect(controls.get('nullableInt')).toBe('slider')
513514
})
514515

515-
it('treats a structured enum as JSON and a primitive one as text', () => {
516+
it('reads an enum from its declared type before its members', () => {
517+
// A dropdown stores `String(option)`, so a numeric enum read as text would send
518+
// '1' where the server expects 1.
516519
const shapes = buildJsonSchemaParamShapes({
517-
properties: { structured: { enum: [{ a: 1 }] }, primitive: { enum: ['x', 'y'] } },
520+
properties: {
521+
declaredInt: { type: 'integer', enum: [1, 2, 3] },
522+
declaredBool: { type: 'boolean', enum: [true, false] },
523+
declaredStr: { type: 'string', enum: ['a', 'b'] },
524+
},
518525
})
519526

527+
expect(shapes.get('declaredInt')).toBe('number')
528+
expect(shapes.get('declaredBool')).toBe('boolean')
529+
expect(shapes.get('declaredStr')).toBe('string')
530+
})
531+
532+
it('infers an untyped enum from its members', () => {
533+
const shapes = buildJsonSchemaParamShapes({
534+
properties: {
535+
nums: { enum: [1, 2] },
536+
bools: { enum: [true, false] },
537+
strs: { enum: ['x', 'y'] },
538+
structured: { enum: [{ a: 1 }] },
539+
mixed: { enum: [1, 'x'] },
540+
},
541+
})
542+
543+
expect(shapes.get('nums')).toBe('number')
544+
expect(shapes.get('bools')).toBe('boolean')
545+
expect(shapes.get('strs')).toBe('string')
520546
expect(shapes.get('structured')).toBe('json')
521-
expect(shapes.get('primitive')).toBe('string')
547+
expect(shapes.get('mixed')).toBe('string')
548+
})
549+
550+
it('round-trips a numeric enum through the dropdown it renders as', () => {
551+
const property = { type: 'integer', enum: [1, 2, 3] }
552+
const [subBlock] = buildSubBlocksFromJsonSchema({ properties: { n: property } }, (id) => id)
553+
554+
expect(subBlock.type).toBe('dropdown')
555+
// The dropdown stores the stringified member; the shape decodes it back.
556+
expect(decodeToolParamValue('2', getJsonSchemaValueShape(property))).toBe(2)
557+
})
558+
559+
it('normalizes a legacy string left by a control that has since changed type', () => {
560+
// A union-typed property used to render as a text field and now renders as a switch;
561+
// its stored 'false' must not tick the box.
562+
expect(
563+
decodeToolParamValue('false', getJsonSchemaValueShape({ type: ['boolean', 'null'] }))
564+
).toBe(false)
565+
expect(decodeToolParamValue(true, getJsonSchemaValueShape({ type: 'boolean' }))).toBe(true)
522566
})
523567
})

apps/sim/tools/param-shape.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -409,17 +409,26 @@ export function subBlockTypeForJsonSchema(property: JsonSchemaProperty): SubBloc
409409
* the control would answer `'string'` and the argument would reach the MCP server
410410
* undecoded. The same holds for a non-primitive enum, which renders as free text.
411411
*/
412-
function getJsonSchemaValueShape(property: JsonSchemaProperty): ToolParamValueShape {
413-
if (Array.isArray(property.enum)) {
414-
return property.enum.every((option) => option === null || typeof option !== 'object')
415-
? 'string'
416-
: 'json'
417-
}
418-
412+
export function getJsonSchemaValueShape(property: JsonSchemaProperty): ToolParamValueShape {
419413
const type = jsonSchemaType(property)
420414
if (type === 'boolean') return 'boolean'
421415
if (type === 'number' || type === 'integer') return 'number'
422416
if (type === 'object' || type === 'array') return 'json'
417+
if (type === 'string') return 'string'
418+
419+
// No declared type leaves the enum members as the only signal. This runs AFTER the
420+
// declared type, not before: a dropdown stores `String(option)`, so a numeric enum
421+
// read as text would send `'1'` where the server expects `1`.
422+
if (Array.isArray(property.enum)) return enumMemberShape(property.enum)
423+
424+
return 'string'
425+
}
426+
427+
/** The shape an enum's members share, for a property that declares no type. */
428+
function enumMemberShape(members: readonly unknown[]): ToolParamValueShape {
429+
if (members.some((member) => member !== null && typeof member === 'object')) return 'json'
430+
if (members.every((member) => typeof member === 'number')) return 'number'
431+
if (members.every((member) => typeof member === 'boolean')) return 'boolean'
423432
return 'string'
424433
}
425434

0 commit comments

Comments
 (0)