improvement(executor): subflows, hitl handling cleanup #4604
improvement(executor): subflows, hitl handling cleanup #4604icecrasher321 wants to merge 10 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Makes parallel batching explicit in DAG control flow by wiring Hardens pause/resume by serializing/restoring Reviewed by Cursor Bugbot for commit 99634ae. Configure here. |
Greptile SummaryThis PR refactors the workflow execution engine by unifying subflow metadata (
Confidence Score: 3/5Not safe to merge without investigating the handleParentSubflowCompletion P1 regression for nested subflows inside parallels The P1 finding in handleParentSubflowCompletion affects result correctness for any workflow that nests a subflow (loop or parallel) inside a parallel branch with additional downstream blocks. The O(n) scan in getScopedBlockOutput is a P2 concern. The rest of the refactor is well-structured and the metadata unification and batch-on-demand expansion are sound. apps/sim/executor/orchestrators/node.ts (handleParentSubflowCompletion logic) and apps/sim/executor/execution/state.ts (getScopedBlockOutput scan) Important Files Changed
Sequence DiagramsequenceDiagram
participant E as Engine
participant SE as SentinelStart
participant PO as ParallelOrchestrator
participant B as BranchBlocks
participant EE as SentinelEnd
participant NO as NodeOrchestrator
E->>SE: processNode(sentinelStart)
SE->>PO: prepareCurrentBatch(ctx, scope)
PO->>PO: expandParallel() — clone DAG nodes
PO->>PO: remapClonedEdges() — wire edges
PO->>PO: markDirty(cloned nodes)
SE-->>E: sentinelStart complete
loop For each branch
E->>B: processNode(branchBlock)
B-->>E: output registered in blockStates
end
E->>EE: processNode(sentinelEnd)
EE->>NO: handleParentSubflowCompletion()
NO->>PO: handleParallelBranchCompletion(branchIndex)
PO->>PO: branchOutputs[i].push(results)
alt More batches remaining
EE->>SE: PARALLEL_CONTINUE back-edge
SE->>PO: prepareCurrentBatch() — next batch
else All batches done
EE-->>E: parallel.results assembled
end
|
|
bugbot run |
|
@greptile |
|
@greptile |
|
bugbot run |
|
@greptile |
|
@greptile |
|
bugbot run |
|
bugbot run |
|
@greptile |
|
bugbot run |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 99634ae. Configure here.
|
|
||
| if (output._pauseMetadata) { | ||
| await this.nodeOrchestrator.handleNodeCompletion(this.context, nodeId, output) | ||
|
|
There was a problem hiding this comment.
Paused HITL parallel branches record wrong output
High Severity
handleNodeCompletion is now called for paused (HITL) blocks before the pause is recorded. When a HITL block is a parallel branch node (isParallelBranch: true), this causes handleParallelBranchCompletion to push { _pauseMetadata: ... } into scope.branchOutputs for that branch. Because scope.branchOutputs is serialized into the pause snapshot, it persists across resume. When the workflow resumes and the parallel sentinel end eventually fires, aggregateParallelResults uses the stored pause metadata as the branch's actual output — not the human's response — producing incorrect aggregated results. The new test for this feature (records paused block completion before returning paused result) uses a node with no isParallelBranch metadata, so this path is not exercised.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 99634ae. Configure here.
Summary
Refactors parallel batching onto explicit DAG control flow while hardening nested subflow execution, pause/resume, run-from-block, and output resolution edge cases.
Type of Change
Testing
Tested manually
Checklist