Skip to content

Commit 36ec68d

Browse files
authored
fix(serializer): validate required fields for blocks without tools (#3137)
1 parent fce566c commit 36ec68d

File tree

4 files changed

+232
-59
lines changed

4 files changed

+232
-59
lines changed

apps/sim/lib/core/utils/formatting.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ export function formatDuration(
185185
const precision = options?.precision ?? 0
186186

187187
if (ms < 1) {
188+
// Zero or near-zero: show "0ms" instead of "0.00ms"
189+
if (ms === 0 || ms < 0.005) {
190+
return '0ms'
191+
}
188192
// Sub-millisecond: show with 2 decimal places
189193
return `${ms.toFixed(2)}ms`
190194
}

apps/sim/serializer/index.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,108 @@ describe('Serializer', () => {
464464
}).not.toThrow()
465465
})
466466

467+
it.concurrent(
468+
'should validate required fields for blocks without tools (empty tools.access)',
469+
() => {
470+
const serializer = new Serializer()
471+
472+
const waitBlockMissingRequired: any = {
473+
id: 'wait-block',
474+
type: 'wait',
475+
name: 'Wait Block',
476+
position: { x: 0, y: 0 },
477+
subBlocks: {
478+
timeValue: { value: '' },
479+
timeUnit: { value: 'seconds' },
480+
},
481+
outputs: {},
482+
enabled: true,
483+
}
484+
485+
expect(() => {
486+
serializer.serializeWorkflow(
487+
{ 'wait-block': waitBlockMissingRequired },
488+
[],
489+
{},
490+
undefined,
491+
true
492+
)
493+
}).toThrow('Wait Block is missing required fields: Wait Amount')
494+
}
495+
)
496+
497+
it.concurrent(
498+
'should pass validation for blocks without tools when required fields are present',
499+
() => {
500+
const serializer = new Serializer()
501+
502+
const waitBlockWithFields: any = {
503+
id: 'wait-block',
504+
type: 'wait',
505+
name: 'Wait Block',
506+
position: { x: 0, y: 0 },
507+
subBlocks: {
508+
timeValue: { value: '10' },
509+
timeUnit: { value: 'seconds' },
510+
},
511+
outputs: {},
512+
enabled: true,
513+
}
514+
515+
expect(() => {
516+
serializer.serializeWorkflow(
517+
{ 'wait-block': waitBlockWithFields },
518+
[],
519+
{},
520+
undefined,
521+
true
522+
)
523+
}).not.toThrow()
524+
}
525+
)
526+
527+
it.concurrent('should report all missing required fields for blocks without tools', () => {
528+
const serializer = new Serializer()
529+
530+
const waitBlockAllMissing: any = {
531+
id: 'wait-block',
532+
type: 'wait',
533+
name: 'Wait Block',
534+
position: { x: 0, y: 0 },
535+
subBlocks: {
536+
timeValue: { value: null },
537+
timeUnit: { value: '' },
538+
},
539+
outputs: {},
540+
enabled: true,
541+
}
542+
543+
expect(() => {
544+
serializer.serializeWorkflow({ 'wait-block': waitBlockAllMissing }, [], {}, undefined, true)
545+
}).toThrow('Wait Block is missing required fields: Wait Amount, Unit')
546+
})
547+
548+
it.concurrent('should skip validation for disabled blocks without tools', () => {
549+
const serializer = new Serializer()
550+
551+
const disabledWaitBlock: any = {
552+
id: 'wait-block',
553+
type: 'wait',
554+
name: 'Wait Block',
555+
position: { x: 0, y: 0 },
556+
subBlocks: {
557+
timeValue: { value: null },
558+
timeUnit: { value: null },
559+
},
560+
outputs: {},
561+
enabled: false,
562+
}
563+
564+
expect(() => {
565+
serializer.serializeWorkflow({ 'wait-block': disabledWaitBlock }, [], {}, undefined, true)
566+
}).not.toThrow()
567+
})
568+
467569
it.concurrent('should handle empty string values as missing', () => {
468570
const serializer = new Serializer()
469571

apps/sim/serializer/index.ts

Lines changed: 94 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -416,21 +416,6 @@ export class Serializer {
416416
return
417417
}
418418

419-
// Get the tool configuration to check parameter visibility
420-
const toolAccess = blockConfig.tools?.access
421-
if (!toolAccess || toolAccess.length === 0) {
422-
return // No tools to validate against
423-
}
424-
425-
// Determine the current tool ID using the same logic as the serializer
426-
const currentToolId = this.selectToolId(blockConfig, params)
427-
428-
// Get the specific tool to validate against
429-
const currentTool = getTool(currentToolId)
430-
if (!currentTool) {
431-
return // Tool not found, skip validation
432-
}
433-
434419
const missingFields: string[] = []
435420
const displayAdvancedOptions = block.advancedMode ?? false
436421
const isTriggerContext = block.triggerMode ?? false
@@ -439,55 +424,105 @@ export class Serializer {
439424
const canonicalModeOverrides = block.data?.canonicalModes
440425
const allValues = buildSubBlockValues(block.subBlocks)
441426

442-
// Iterate through the tool's parameters, not the block's subBlocks
443-
Object.entries(currentTool.params || {}).forEach(([paramId, paramConfig]) => {
444-
if (paramConfig.required && paramConfig.visibility === 'user-only') {
445-
const matchingConfigs = blockConfig.subBlocks?.filter((sb: any) => sb.id === paramId) || []
446-
447-
let shouldValidateParam = true
448-
449-
if (matchingConfigs.length > 0) {
450-
shouldValidateParam = matchingConfigs.some((subBlockConfig: any) => {
451-
const includedByMode = shouldSerializeSubBlock(
452-
subBlockConfig,
453-
allValues,
454-
displayAdvancedOptions,
455-
isTriggerContext,
456-
isTriggerCategory,
457-
canonicalIndex,
458-
canonicalModeOverrides
427+
// Get the tool configuration to check parameter visibility
428+
const toolAccess = blockConfig.tools?.access
429+
const currentToolId = toolAccess?.length > 0 ? this.selectToolId(blockConfig, params) : null
430+
const currentTool = currentToolId ? getTool(currentToolId) : null
431+
432+
// Validate tool parameters (for blocks with tools)
433+
if (currentTool) {
434+
Object.entries(currentTool.params || {}).forEach(([paramId, paramConfig]) => {
435+
if (paramConfig.required && paramConfig.visibility === 'user-only') {
436+
const matchingConfigs =
437+
blockConfig.subBlocks?.filter((sb: any) => sb.id === paramId) || []
438+
439+
let shouldValidateParam = true
440+
441+
if (matchingConfigs.length > 0) {
442+
shouldValidateParam = matchingConfigs.some((subBlockConfig: any) => {
443+
const includedByMode = shouldSerializeSubBlock(
444+
subBlockConfig,
445+
allValues,
446+
displayAdvancedOptions,
447+
isTriggerContext,
448+
isTriggerCategory,
449+
canonicalIndex,
450+
canonicalModeOverrides
451+
)
452+
453+
const isRequired = (() => {
454+
if (!subBlockConfig.required) return false
455+
if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required
456+
return evaluateSubBlockCondition(subBlockConfig.required, params)
457+
})()
458+
459+
return includedByMode && isRequired
460+
})
461+
}
462+
463+
if (!shouldValidateParam) {
464+
return
465+
}
466+
467+
const fieldValue = params[paramId]
468+
if (fieldValue === undefined || fieldValue === null || fieldValue === '') {
469+
const activeConfig = matchingConfigs.find((config: any) =>
470+
shouldSerializeSubBlock(
471+
config,
472+
allValues,
473+
displayAdvancedOptions,
474+
isTriggerContext,
475+
isTriggerCategory,
476+
canonicalIndex,
477+
canonicalModeOverrides
478+
)
459479
)
480+
const displayName = activeConfig?.title || paramId
481+
missingFields.push(displayName)
482+
}
483+
}
484+
})
485+
}
460486

461-
const isRequired = (() => {
462-
if (!subBlockConfig.required) return false
463-
if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required
464-
return evaluateSubBlockCondition(subBlockConfig.required, params)
465-
})()
487+
// Validate required subBlocks not covered by tool params (e.g., blocks with empty tools.access)
488+
const validatedByTool = new Set(currentTool ? Object.keys(currentTool.params || {}) : [])
466489

467-
return includedByMode && isRequired
468-
})
469-
}
490+
blockConfig.subBlocks?.forEach((subBlockConfig: SubBlockConfig) => {
491+
// Skip if already validated via tool params
492+
if (validatedByTool.has(subBlockConfig.id)) {
493+
return
494+
}
470495

471-
if (!shouldValidateParam) {
472-
return
473-
}
496+
// Check if subBlock is visible
497+
const isVisible = shouldSerializeSubBlock(
498+
subBlockConfig,
499+
allValues,
500+
displayAdvancedOptions,
501+
isTriggerContext,
502+
isTriggerCategory,
503+
canonicalIndex,
504+
canonicalModeOverrides
505+
)
506+
507+
if (!isVisible) {
508+
return
509+
}
474510

475-
const fieldValue = params[paramId]
476-
if (fieldValue === undefined || fieldValue === null || fieldValue === '') {
477-
const activeConfig = matchingConfigs.find((config: any) =>
478-
shouldSerializeSubBlock(
479-
config,
480-
allValues,
481-
displayAdvancedOptions,
482-
isTriggerContext,
483-
isTriggerCategory,
484-
canonicalIndex,
485-
canonicalModeOverrides
486-
)
487-
)
488-
const displayName = activeConfig?.title || paramId
489-
missingFields.push(displayName)
490-
}
511+
// Check if subBlock is required
512+
const isRequired = (() => {
513+
if (!subBlockConfig.required) return false
514+
if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required
515+
return evaluateSubBlockCondition(subBlockConfig.required, params)
516+
})()
517+
518+
if (!isRequired) {
519+
return
520+
}
521+
522+
// Check if value is missing
523+
const fieldValue = params[subBlockConfig.id]
524+
if (fieldValue === undefined || fieldValue === null || fieldValue === '') {
525+
missingFields.push(subBlockConfig.title || subBlockConfig.id)
491526
}
492527
})
493528

packages/testing/src/mocks/blocks.mock.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,38 @@ export const mockBlockConfigs: Record<string, any> = {
259259
],
260260
inputs: {},
261261
},
262+
wait: {
263+
name: 'Wait',
264+
description: 'Pause workflow execution for a specified time delay',
265+
category: 'blocks',
266+
bgColor: '#F59E0B',
267+
tools: {
268+
access: [],
269+
},
270+
subBlocks: [
271+
{
272+
id: 'timeValue',
273+
title: 'Wait Amount',
274+
type: 'short-input',
275+
placeholder: '10',
276+
required: true,
277+
},
278+
{
279+
id: 'timeUnit',
280+
title: 'Unit',
281+
type: 'dropdown',
282+
required: true,
283+
},
284+
],
285+
inputs: {
286+
timeValue: { type: 'string' },
287+
timeUnit: { type: 'string' },
288+
},
289+
outputs: {
290+
waitDuration: { type: 'number' },
291+
status: { type: 'string' },
292+
},
293+
},
262294
}
263295

264296
/**

0 commit comments

Comments
 (0)