Fix evented wflow nested loop#16312
Conversation
…vented-wflow-nested-loop
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
WalkthroughParentWorkflow gains execution metadata fields. processWorkflowEnd now routes nested loop completions into loop processing; start/resume payloads include richer parent metadata. Two nested-loop test workflows and a changeset were added. ChangesNested Loops Evented Workflow Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🦋 Changeset detectedLatest commit: d4b8e35 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/workflows/evented/workflow-event-processor/index.ts`:
- Around line 373-374: The code only looks at
parentWorkflow.stepGraph[parentWorkflow.executionPath[0]] which inspects the
root segment of executionPath and misses nested active parents; update the
loop-detection logic in the workflow-event-processor to use the current/deepest
active segment (e.g., the last element of parentWorkflow.executionPath or derive
the active index) when selecting the step from parentWorkflow.stepGraph, handle
the case where executionPath may be empty/undefined, and ensure the subsequent
branch that checks step?.type === 'loop' now correctly detects nested loops so
completion routing doesn't fall back to workflow.step.end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c90e9bbe-da32-4dc0-a1d6-8e43fcd94a12
📒 Files selected for processing (3)
.changeset/rare-bikes-wear.mdpackages/core/src/workflows/evented/workflow-event-processor/index.tsworkflows/_test-utils/src/domains/loops.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/workflows/evented/workflow-event-processor/index.ts`:
- Around line 62-70: The type for stepGraph is incorrect: change the property
declaration of stepGraph from Record<string, StepFlowEntry> to StepFlowEntry[]
so it matches how the code indexes it by numeric executionPath values (e.g.,
references like stepGraph[executionPath[0]!] and the StepFlowEntry[] usage at
other places); update the interface/type that contains stepGraph accordingly so
the compiler and callers treat it as an array of StepFlowEntry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8b89c87-fc7b-4cd8-9eb5-734d0d187f86
📒 Files selected for processing (1)
packages/core/src/workflows/evented/workflow-event-processor/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/workflows/evented/workflow-event-processor/index.ts (1)
373-375:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the full
executionPathwhen detecting loop parents.Line 374 still inspects only
executionPath[0], so a loop nested underparallel/conditionalis misclassified and falls back toworkflow.step.end. That leaves the nested-loop bug in place for non-root loops.Suggested fix
- const step = parentWorkflow.stepGraph[parentWorkflow.executionPath[0]!]; + const step = getStep(parentWorkflow as unknown as Workflow, parentWorkflow.executionPath);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/workflows/evented/workflow-event-processor/index.ts` around lines 373 - 375, The code checks only parentWorkflow.executionPath[0] to find the parent step so nested loops under parallel/conditional get misclassified; update the lookup to use the full executionPath (e.g., use the last index or traverse the path) when resolving the parent step in parentWorkflow.stepGraph instead of executionPath[0], then keep the existing check step?.type === 'loop' so nested loop parents are correctly detected and avoid falling back to workflow.step.end.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/workflows/evented/workflow-event-processor/index.ts`:
- Around line 407-411: The code is incorrectly forwarding the nested workflow's
resume metadata (resumeSteps, resumeData, activeSteps) into the parent
completion event; instead pass the parent's metadata so the parent's bookkeeping
isn't overwritten. Update the call/context that currently passes resumeSteps,
resumeData, and activeSteps to use the parent's values (e.g.,
parentWorkflow.resumeSteps, parentWorkflow.resumeData,
parentWorkflow.activeSteps) alongside stepResults: parentWorkflow.stepResults
and prevResult before calling processWorkflowStepEnd (or the function that
continues the parent step).
---
Duplicate comments:
In `@packages/core/src/workflows/evented/workflow-event-processor/index.ts`:
- Around line 373-375: The code checks only parentWorkflow.executionPath[0] to
find the parent step so nested loops under parallel/conditional get
misclassified; update the lookup to use the full executionPath (e.g., use the
last index or traverse the path) when resolving the parent step in
parentWorkflow.stepGraph instead of executionPath[0], then keep the existing
check step?.type === 'loop' so nested loop parents are correctly detected and
avoid falling back to workflow.step.end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47243b6d-073a-45d1-b292-7039c8197311
📒 Files selected for processing (1)
packages/core/src/workflows/evented/workflow-event-processor/index.ts
Description
Fix evented wflow nested loop
Related Issue(s)
Type of Change
Checklist
ELI5 (Explain Like I'm 5)
This PR fixes a bug where a workflow run started inside a loop (a "nested workflow") didn't let its parent loop resume or finish correctly. It detects when a finished child run belongs to a loop step in the parent and routes the completion into the loop handler so the parent can continue; tests ensure nested do-until and do-while loops work.
Changes Made
Bug Fix: Nested Loop Handling in Evented Workflows
Type/API Updates
Tests
Release/Packaging
Files Touched (high level)
Impact