Skip to content

Conversation

@Sg312
Copy link
Contributor

@Sg312 Sg312 commented Dec 4, 2025

Summary

Fix nested error activation

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 Dec 4, 2025

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

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 4, 2025 6:43pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

Fixed a race condition in edge processing where nodes could be marked as ready before nested error path deactivations completed. The change separates edge activation from readiness checking by:

  • Collecting activated target nodes during edge processing
  • Deferring readiness checks until after all edges are processed and cascade deactivations complete

This ensures that when an error edge is not activated and triggers deactivateEdgeAndDescendants, all nested deactivations propagate fully before any nodes are added to the execution queue. Without this fix, nodes in error paths could execute prematurely before their incoming edges were properly deactivated.

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - it's a focused bug fix with clear logic
  • The change is well-scoped and addresses a specific race condition. The refactoring is minimal and preserves all existing logic while fixing the timing issue. However, given the manual testing only and the critical nature of execution flow, thorough testing in staging is recommended before production deployment
  • No files require special attention - the single file change is straightforward

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/executor/execution/edge-manager.ts 4/5 Refactored edge processing to defer readiness checks until after cascade deactivations complete, fixing premature node activation in nested error paths

Sequence Diagram

sequenceDiagram
    participant Engine as ExecutionEngine
    participant EM as EdgeManager
    participant Node as DAGNode
    participant Queue as ReadyQueue

    Engine->>EM: processOutgoingEdges(node, output)
    
    Note over EM: Phase 1: Process all edges
    loop For each outgoing edge
        EM->>EM: shouldActivateEdge(edge, output)?
        alt Edge should NOT activate
            EM->>EM: deactivateEdgeAndDescendants(target)
            Note over EM: Cascade deactivation<br/>to descendants
        else Edge should activate
            EM->>Node: incomingEdges.delete(nodeId)
            EM->>EM: Add to activatedTargets[]
        end
    end
    
    Note over EM: Phase 2: Check readiness<br/>after cascade complete
    loop For each activated target
        EM->>EM: isNodeReady(targetNode)?
        alt Node is ready
            EM->>Queue: Add to readyNodes[]
        end
    end
    
    EM-->>Engine: Return readyNodes[]
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.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@Sg312 Sg312 merged commit cd5d1c0 into staging Dec 4, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/executor-nested-errors branch December 4, 2025 19:32
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