From 36ec68d93e629802bb6f01f4ee4e0f72a5e56cc0 Mon Sep 17 00:00:00 2001 From: Waleed Date: Wed, 4 Feb 2026 16:47:18 -0800 Subject: [PATCH] fix(serializer): validate required fields for blocks without tools (#3137) --- apps/sim/lib/core/utils/formatting.ts | 4 + apps/sim/serializer/index.test.ts | 102 +++++++++++++++ apps/sim/serializer/index.ts | 151 +++++++++++++--------- packages/testing/src/mocks/blocks.mock.ts | 32 +++++ 4 files changed, 231 insertions(+), 58 deletions(-) diff --git a/apps/sim/lib/core/utils/formatting.ts b/apps/sim/lib/core/utils/formatting.ts index af9bcb321..d6985147f 100644 --- a/apps/sim/lib/core/utils/formatting.ts +++ b/apps/sim/lib/core/utils/formatting.ts @@ -185,6 +185,10 @@ export function formatDuration( const precision = options?.precision ?? 0 if (ms < 1) { + // Zero or near-zero: show "0ms" instead of "0.00ms" + if (ms === 0 || ms < 0.005) { + return '0ms' + } // Sub-millisecond: show with 2 decimal places return `${ms.toFixed(2)}ms` } diff --git a/apps/sim/serializer/index.test.ts b/apps/sim/serializer/index.test.ts index d661a901d..8b0539144 100644 --- a/apps/sim/serializer/index.test.ts +++ b/apps/sim/serializer/index.test.ts @@ -464,6 +464,108 @@ describe('Serializer', () => { }).not.toThrow() }) + it.concurrent( + 'should validate required fields for blocks without tools (empty tools.access)', + () => { + const serializer = new Serializer() + + const waitBlockMissingRequired: any = { + id: 'wait-block', + type: 'wait', + name: 'Wait Block', + position: { x: 0, y: 0 }, + subBlocks: { + timeValue: { value: '' }, + timeUnit: { value: 'seconds' }, + }, + outputs: {}, + enabled: true, + } + + expect(() => { + serializer.serializeWorkflow( + { 'wait-block': waitBlockMissingRequired }, + [], + {}, + undefined, + true + ) + }).toThrow('Wait Block is missing required fields: Wait Amount') + } + ) + + it.concurrent( + 'should pass validation for blocks without tools when required fields are present', + () => { + const serializer = new Serializer() + + const waitBlockWithFields: any = { + id: 'wait-block', + type: 'wait', + name: 'Wait Block', + position: { x: 0, y: 0 }, + subBlocks: { + timeValue: { value: '10' }, + timeUnit: { value: 'seconds' }, + }, + outputs: {}, + enabled: true, + } + + expect(() => { + serializer.serializeWorkflow( + { 'wait-block': waitBlockWithFields }, + [], + {}, + undefined, + true + ) + }).not.toThrow() + } + ) + + it.concurrent('should report all missing required fields for blocks without tools', () => { + const serializer = new Serializer() + + const waitBlockAllMissing: any = { + id: 'wait-block', + type: 'wait', + name: 'Wait Block', + position: { x: 0, y: 0 }, + subBlocks: { + timeValue: { value: null }, + timeUnit: { value: '' }, + }, + outputs: {}, + enabled: true, + } + + expect(() => { + serializer.serializeWorkflow({ 'wait-block': waitBlockAllMissing }, [], {}, undefined, true) + }).toThrow('Wait Block is missing required fields: Wait Amount, Unit') + }) + + it.concurrent('should skip validation for disabled blocks without tools', () => { + const serializer = new Serializer() + + const disabledWaitBlock: any = { + id: 'wait-block', + type: 'wait', + name: 'Wait Block', + position: { x: 0, y: 0 }, + subBlocks: { + timeValue: { value: null }, + timeUnit: { value: null }, + }, + outputs: {}, + enabled: false, + } + + expect(() => { + serializer.serializeWorkflow({ 'wait-block': disabledWaitBlock }, [], {}, undefined, true) + }).not.toThrow() + }) + it.concurrent('should handle empty string values as missing', () => { const serializer = new Serializer() diff --git a/apps/sim/serializer/index.ts b/apps/sim/serializer/index.ts index 027c2dd4a..e22c654c4 100644 --- a/apps/sim/serializer/index.ts +++ b/apps/sim/serializer/index.ts @@ -416,21 +416,6 @@ export class Serializer { return } - // Get the tool configuration to check parameter visibility - const toolAccess = blockConfig.tools?.access - if (!toolAccess || toolAccess.length === 0) { - return // No tools to validate against - } - - // Determine the current tool ID using the same logic as the serializer - const currentToolId = this.selectToolId(blockConfig, params) - - // Get the specific tool to validate against - const currentTool = getTool(currentToolId) - if (!currentTool) { - return // Tool not found, skip validation - } - const missingFields: string[] = [] const displayAdvancedOptions = block.advancedMode ?? false const isTriggerContext = block.triggerMode ?? false @@ -439,55 +424,105 @@ export class Serializer { const canonicalModeOverrides = block.data?.canonicalModes const allValues = buildSubBlockValues(block.subBlocks) - // Iterate through the tool's parameters, not the block's subBlocks - Object.entries(currentTool.params || {}).forEach(([paramId, paramConfig]) => { - if (paramConfig.required && paramConfig.visibility === 'user-only') { - const matchingConfigs = blockConfig.subBlocks?.filter((sb: any) => sb.id === paramId) || [] + // Get the tool configuration to check parameter visibility + const toolAccess = blockConfig.tools?.access + const currentToolId = toolAccess?.length > 0 ? this.selectToolId(blockConfig, params) : null + const currentTool = currentToolId ? getTool(currentToolId) : null - let shouldValidateParam = true + // Validate tool parameters (for blocks with tools) + if (currentTool) { + Object.entries(currentTool.params || {}).forEach(([paramId, paramConfig]) => { + if (paramConfig.required && paramConfig.visibility === 'user-only') { + const matchingConfigs = + blockConfig.subBlocks?.filter((sb: any) => sb.id === paramId) || [] - if (matchingConfigs.length > 0) { - shouldValidateParam = matchingConfigs.some((subBlockConfig: any) => { - const includedByMode = shouldSerializeSubBlock( - subBlockConfig, - allValues, - displayAdvancedOptions, - isTriggerContext, - isTriggerCategory, - canonicalIndex, - canonicalModeOverrides + let shouldValidateParam = true + + if (matchingConfigs.length > 0) { + shouldValidateParam = matchingConfigs.some((subBlockConfig: any) => { + const includedByMode = shouldSerializeSubBlock( + subBlockConfig, + allValues, + displayAdvancedOptions, + isTriggerContext, + isTriggerCategory, + canonicalIndex, + canonicalModeOverrides + ) + + const isRequired = (() => { + if (!subBlockConfig.required) return false + if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required + return evaluateSubBlockCondition(subBlockConfig.required, params) + })() + + return includedByMode && isRequired + }) + } + + if (!shouldValidateParam) { + return + } + + const fieldValue = params[paramId] + if (fieldValue === undefined || fieldValue === null || fieldValue === '') { + const activeConfig = matchingConfigs.find((config: any) => + shouldSerializeSubBlock( + config, + allValues, + displayAdvancedOptions, + isTriggerContext, + isTriggerCategory, + canonicalIndex, + canonicalModeOverrides + ) ) - - const isRequired = (() => { - if (!subBlockConfig.required) return false - if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required - return evaluateSubBlockCondition(subBlockConfig.required, params) - })() - - return includedByMode && isRequired - }) + const displayName = activeConfig?.title || paramId + missingFields.push(displayName) + } } + }) + } - if (!shouldValidateParam) { - return - } + // Validate required subBlocks not covered by tool params (e.g., blocks with empty tools.access) + const validatedByTool = new Set(currentTool ? Object.keys(currentTool.params || {}) : []) - const fieldValue = params[paramId] - if (fieldValue === undefined || fieldValue === null || fieldValue === '') { - const activeConfig = matchingConfigs.find((config: any) => - shouldSerializeSubBlock( - config, - allValues, - displayAdvancedOptions, - isTriggerContext, - isTriggerCategory, - canonicalIndex, - canonicalModeOverrides - ) - ) - const displayName = activeConfig?.title || paramId - missingFields.push(displayName) - } + blockConfig.subBlocks?.forEach((subBlockConfig: SubBlockConfig) => { + // Skip if already validated via tool params + if (validatedByTool.has(subBlockConfig.id)) { + return + } + + // Check if subBlock is visible + const isVisible = shouldSerializeSubBlock( + subBlockConfig, + allValues, + displayAdvancedOptions, + isTriggerContext, + isTriggerCategory, + canonicalIndex, + canonicalModeOverrides + ) + + if (!isVisible) { + return + } + + // Check if subBlock is required + const isRequired = (() => { + if (!subBlockConfig.required) return false + if (typeof subBlockConfig.required === 'boolean') return subBlockConfig.required + return evaluateSubBlockCondition(subBlockConfig.required, params) + })() + + if (!isRequired) { + return + } + + // Check if value is missing + const fieldValue = params[subBlockConfig.id] + if (fieldValue === undefined || fieldValue === null || fieldValue === '') { + missingFields.push(subBlockConfig.title || subBlockConfig.id) } }) diff --git a/packages/testing/src/mocks/blocks.mock.ts b/packages/testing/src/mocks/blocks.mock.ts index b229404f7..1931b1cc7 100644 --- a/packages/testing/src/mocks/blocks.mock.ts +++ b/packages/testing/src/mocks/blocks.mock.ts @@ -259,6 +259,38 @@ export const mockBlockConfigs: Record = { ], inputs: {}, }, + wait: { + name: 'Wait', + description: 'Pause workflow execution for a specified time delay', + category: 'blocks', + bgColor: '#F59E0B', + tools: { + access: [], + }, + subBlocks: [ + { + id: 'timeValue', + title: 'Wait Amount', + type: 'short-input', + placeholder: '10', + required: true, + }, + { + id: 'timeUnit', + title: 'Unit', + type: 'dropdown', + required: true, + }, + ], + inputs: { + timeValue: { type: 'string' }, + timeUnit: { type: 'string' }, + }, + outputs: { + waitDuration: { type: 'number' }, + status: { type: 'string' }, + }, + }, } /**