Skip to content

fix(serializer): validate required fields for blocks without tools - #3137

Merged
icecrasher321 merged 1 commit into
stagingfrom
fix/wait
Feb 5, 2026
Merged

fix(serializer): validate required fields for blocks without tools#3137
icecrasher321 merged 1 commit into
stagingfrom
fix/wait

Conversation

@waleedlatif1

@waleedlatif1waleedlatif1 commented Feb 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fixed serializer skipping required field validation for blocks with empty tools.access (e.g., wait block)
  • Added validation loop for subBlock required fields not covered by tool params
  • Minor fix for duration formatting to show "0ms" instead of "0.00ms"

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)
Cursor Bugbot found 1 potential issue for commit 60fcaf5

@vercel

vercelBot commented Feb 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedFeb 4, 2026 11:47pm

Request Review

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

Fixed serializer validation bug where required fields were not validated for blocks with empty tools.access arrays (like wait blocks).

The previous implementation early-returned when tools.access was empty, skipping validation entirely. The fix restructures the validation logic:

  • First validates tool parameters for blocks that have tools
  • Then validates required subBlock fields not already covered by tool params
  • Uses a Set to track which fields were already validated to avoid duplication

Also includes a minor formatting improvement to show "0ms" instead of "0.00ms" for zero/near-zero durations.

Key changes:

  • Moved tool existence check inside validation section rather than early return
  • Added secondary validation loop for subBlocks with required: true not covered by tool params
  • Added comprehensive test coverage for wait block validation scenarios
  • Tests cover: missing required fields, valid fields, multiple missing fields, and disabled block handling

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk
  • The changes fix a clear bug in validation logic with comprehensive test coverage. The refactoring is well-structured and maintains backward compatibility for blocks with tools. Minor deduction for lack of integration testing across different block types with varying tool configurations.
  • No files require special attention

Important Files Changed

FilenameOverview
apps/sim/serializer/index.tsRefactored validation logic to handle blocks without tools by adding secondary validation loop for required subBlock fields not covered by tool params
apps/sim/serializer/index.test.tsAdded comprehensive test coverage for wait block validation with empty tools.access array

Sequence Diagram

sequenceDiagram
participant Client
participant Serializer
participant ValidationLogic
participant BlockConfig
participant ToolRegistry
Client->>Serializer: serializeWorkflow(blocks, validateRequired=true)
Serializer->>Serializer: serializeBlock(block)
Serializer->>Serializer: extractParams(block)
alt validateRequired is true
Serializer->>ValidationLogic: validateRequiredFieldsBeforeExecution(block, blockConfig, params)
ValidationLogic->>BlockConfig: Check if block enabled
alt Block has tools.access
ValidationLogic->>ToolRegistry: getTool(currentToolId)
ToolRegistry-->>ValidationLogic: currentTool
loop For each tool param
ValidationLogic->>ValidationLogic: Check if required & user-only
ValidationLogic->>ValidationLogic: Check visibility via shouldSerializeSubBlock
ValidationLogic->>ValidationLogic: Validate field value
end
end
Note over ValidationLogic: NEW: Secondary validation for blocks without tools
ValidationLogic->>ValidationLogic: Build validatedByTool Set
loop For each subBlock in blockConfig
alt Not validated by tool params
ValidationLogic->>ValidationLogic: Check visibility via shouldSerializeSubBlock
ValidationLogic->>ValidationLogic: Check if required
ValidationLogic->>ValidationLogic: Validate field value
end
end
alt Missing fields found
ValidationLogic-->>Serializer: throw Error(missing fields)
Serializer-->>Client: Error
else All fields valid
ValidationLogic-->>Serializer: validation passed
end
end
Serializer->>Serializer: Continue serialization
Serializer-->>Client: SerializedBlock
Loading

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Comment threadapps/sim/lib/core/utils/formatting.ts
@icecrasher321
icecrasher321 merged commit 36ec68d into stagingFeb 5, 2026
12 checks passed
@icecrasher321
icecrasher321 deleted the fix/wait branch February 5, 2026 00:47
royceP2 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Mar 3, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@waleedlatif1@icecrasher321