Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

To work with app running on multiple tasks + executor implementation the abort signal to execute route is the correct pattern to properly pause execution after current block finishes running.

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 24, 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 24, 2025 10:47am

@icecrasher321 icecrasher321 requested a review from Sg312 December 24, 2025 10:48
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 24, 2025

Greptile Summary

This PR refactors the workflow cancellation mechanism from a callback-based pattern (onExecutorCreated + executor.cancel()) to a standard web API pattern using AbortController and AbortSignal.

Key changes:

  • The SSE route now creates an AbortController and passes its signal to the execution engine
  • When the client disconnects (SSE stream cancel() is called), abortController.abort() is invoked
  • The executor and engine check ctx.abortSignal?.aborted instead of maintaining an internal isCancelled flag
  • The wait handler's sleep() function now properly responds to abort signals using event listeners
  • Removed the onExecutorCreated callback which was only used to capture the executor instance for cancellation

Benefits:

  • Aligns with standard JavaScript cancellation patterns (same as fetch API)
  • Works correctly with multiple concurrent task execution
  • Signal propagates naturally through context without needing property getters or instance references
  • More maintainable and testable code

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The refactoring follows standard web API patterns, maintains backward compatibility, includes comprehensive checks across all execution points, and improves the architecture. All changes are well-tested manual abort scenarios. The implementation correctly handles the abort signal with proper cleanup and event listener management.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/[id]/execute/route.ts Replaced executor instance cancellation with AbortController pattern for proper signal-based cancellation
apps/sim/executor/execution/executor.ts Removed cancel() method and isCancelled flag, now passes abortSignal through context extensions
apps/sim/executor/execution/engine.ts Replaced all isCancelled checks with abortSignal?.aborted checks throughout execution loop
apps/sim/executor/handlers/wait/wait-handler.ts Refactored sleep() function to use AbortSignal with event listeners for immediate cancellation
apps/sim/lib/workflows/executor/execution-core.ts Added abortSignal parameter, passes it to executor, removed onExecutorCreated callback

Sequence Diagram

sequenceDiagram
    participant Client as SSE Client
    participant Route as Execute Route
    participant AC as AbortController
    participant Core as ExecutionCore
    participant Executor as DAGExecutor
    participant Engine as ExecutionEngine
    participant Handler as BlockHandler

    Client->>Route: POST /api/workflows/[id]/execute
    Route->>AC: new AbortController()
    Route->>Core: executeWorkflowCore({..., abortSignal})
    Core->>Executor: new DAGExecutor({contextExtensions: {abortSignal}})
    Executor->>Engine: new ExecutionEngine(context)
    Note over Engine: context.abortSignal = signal
    
    Engine->>Engine: while (hasWork())
    Engine->>Engine: check abortSignal?.aborted
    Engine->>Handler: execute block
    Handler->>Handler: check ctx.abortSignal?.aborted
    Handler-->>Engine: block result
    
    Client->>Route: disconnect (SSE cancel)
    Route->>AC: abortController.abort()
    AC->>Engine: signal.aborted = true
    
    Engine->>Engine: check abortSignal?.aborted
    Engine->>Engine: break execution loop
    Engine-->>Core: {status: 'cancelled'}
    Core-->>Route: ExecutionResult
    Route-->>Client: execution:cancelled event
Loading

@icecrasher321 icecrasher321 merged commit b1cd8d1 into staging Dec 24, 2025
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/workflow-abort branch December 24, 2025 18:23
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