Skip to content

Conversation

@Sg312
Copy link
Contributor

@Sg312 Sg312 commented Jan 27, 2026

Summary

Fix convergent error edges

Type of Change

  • Bug fix

Testing

Manual

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 Jan 27, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 27, 2026 0:23am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

Fixed edge manager to correctly handle convergent error edges where multiple nodes have error edges pointing to the same error handler node.

Key changes:

  • Added nodesWithActivatedEdge Set to track nodes that received at least one activated edge during execution
  • When processing outgoing edges, nodes receiving activated edges are now tracked (lines 40-42)
  • Added logic to check deactivation targets and mark them ready if they were previously activated (lines 80-90)
  • Updated cascade prevention to skip nodes that received activated edges (line 239)
  • Cleanup methods now properly clear the activation tracking

Why this matters:
Previously, when multiple workflows had error edges to a shared error handler, the handler would not execute if one workflow succeeded (deactivating its error edge) after another errored (activating its error edge). The new tracking ensures the error handler executes once all source workflows complete, as long as at least one activated an edge to it.

Test coverage:
Added 4 comprehensive test cases covering all combinations: error then success, success then error, both succeed (handler shouldn't run), both error (handler should run).

Confidence Score: 4/5

  • Safe to merge with high confidence - addresses specific edge case with proper tracking
  • Implementation is sound with good test coverage. The tracking mechanism correctly handles convergent error edges. Minor uncertainty about edge cases in complex workflows with loops/parallel execution, but cleanup logic appears correct.
  • No files require special attention - both implementation and tests are well-structured

Important Files Changed

Filename Overview
apps/sim/executor/execution/edge-manager.ts Added nodesWithActivatedEdge tracking to handle convergent error edges correctly. The logic properly tracks nodes that receive activated edges and checks them when deactivating other edges.
apps/sim/executor/execution/edge-manager.test.ts Added comprehensive test coverage for multiple error edges converging to same target. Tests verify correct behavior when sources error/succeed in different orders.

Sequence Diagram

sequenceDiagram
    participant W1 as Workflow 1
    participant W7 as Workflow 7
    participant EM as EdgeManager
    participant EH as Error Handler

    Note over W1,EH: Initial state: EH has incoming edges from W1 and W7

    W1->>EM: processOutgoingEdges(error: "Failed")
    EM->>EM: shouldActivateEdge(error edge) → true
    EM->>EM: Add EH to activatedTargets
    EM->>EM: Add EH to nodesWithActivatedEdge
    EM->>EH: Remove W1 from incomingEdges
    EM->>EM: Check isTargetReady(EH) → false (W7 still pending)
    EM-->>W1: readyNodes: [] (EH not ready yet)

    W7->>EM: processOutgoingEdges(result: "success")
    EM->>EM: shouldActivateEdge(error edge) → false
    EM->>EM: Add W7→EH to edgesToDeactivate
    EM->>EM: deactivateEdgeAndDescendants(W7→EH)
    EM->>EM: Add edge to deactivatedEdges
    Note over EM: New logic (lines 80-90)
    EM->>EM: Check deactivation targets
    EM->>EM: Is EH in nodesWithActivatedEdge? → true
    EM->>EM: Check isTargetReady(EH) → true
    EM->>EM: Add EH to readyNodes
    EM-->>W7: readyNodes: [EH]
    
    Note over EH: Error Handler now ready to execute
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.

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@Sg312 Sg312 merged commit cb65013 into staging Jan 27, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/executor-error-edges branch January 27, 2026 02:37
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