Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Async execution in condition blocks tries to execute directly in trigger dev context which fails in isolated vm worker run.

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)

@vercel
Copy link

vercel bot commented Dec 18, 2025

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 18, 2025 6:41pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 18, 2025

Greptile Summary

Fixed async execution failure in condition blocks when running in Trigger.dev's isolated VM worker environment by replacing executeInIsolatedVM with executeTool('function_execute').

Key Changes:

  • Replaced direct executeInIsolatedVM calls with executeTool('function_execute') API-based execution
  • Context variables are now injected directly into code via const context = ${JSON.stringify(evalContext)};
  • Updated error handling to match new tool response format (result.success instead of result.error)
  • Updated tests to mock executeTool instead of executeInIsolatedVM

Technical Context:
The previous implementation spawned Node.js worker processes using child_process.spawn(), which fails in Trigger.dev's isolated VM context. The new approach uses the existing function_execute tool that handles code execution via an API endpoint (/api/function/execute), avoiding the process spawning limitation.

Confidence Score: 4/5

  • This PR is safe to merge with low risk - it fixes a critical execution bug without introducing new functionality
  • The changes are well-tested with comprehensive test coverage. The context injection approach via JSON.stringify is sound and tests verify the behavior works correctly. Minor concern: the previous thread raised a valid point about context variable access that should be verified in production, but the implementation appears correct based on code analysis.
  • No files require special attention - both implementation and tests are straightforward

Important Files Changed

Filename Overview
apps/sim/executor/handlers/condition/condition-handler.ts Switched from executeInIsolatedVM to executeTool('function_execute') to fix async execution in Trigger.dev isolated VM context, manually injects context via JSON.stringify
apps/sim/executor/handlers/condition/condition-handler.test.ts Updated test mocks to use executeTool instead of executeInIsolatedVM, simulation function correctly handles the new code format with context setup

Sequence Diagram

sequenceDiagram
    participant CH as ConditionBlockHandler
    participant EV as evaluateConditionExpression
    participant ET as executeTool
    participant API as /api/function/execute
    participant VM as Execution Environment

    CH->>EV: evaluateConditionExpression(ctx, expression, block)
    EV->>EV: Resolve variable/block/env references
    EV->>EV: Build code with context injected via JSON.stringify
    EV->>ET: executeTool with function_execute toolId
    ET->>API: POST request with code and params
    API->>VM: Execute code in safe environment
    VM-->>API: Return execution result
    API-->>ET: Return success and output
    ET-->>EV: Return tool response
    EV->>EV: Extract Boolean from result.output.result
    EV-->>CH: Return Boolean evaluation result
    CH->>CH: Select execution path based on conditions
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. apps/sim/executor/handlers/condition/condition-handler.ts, line 23 (link)

    logic: evalContext is constructed but never used, conditions relying on context variables will fail

    The old executeInIsolatedVM implementation passed contextVariables: { context: evalContext } to make source block output available as context in evaluated expressions. The new implementation doesn't pass this anywhere, breaking conditions like context.value > 5.

1 file reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321 icecrasher321 merged commit 78b7643 into staging Dec 18, 2025
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/async-cond-exec branch December 18, 2025 19:19
Sign up for free to 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