feat(workflow): add execution primitives for step context#1102
Conversation
🦋 Changeset detectedLatest commit: f5d1d02 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughAdds four step-context primitives — Changes
Sequence DiagramsequenceDiagram
participant Step as Step Execution
participant Ctx as StepExecuteContext
participant Core as Core Orchestrator
participant Store as Memory/Store
Step->>Ctx: call bail(result?)
Ctx->>Core: throw WorkflowBailSignal
Core->>Core: completeBail() / finalize workflow
Core->>Store: persist final output & metadata
Core->>Store: emit step-complete, workflow-complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/workflow/core.ts">
<violation number="1" location="packages/core/src/workflow/core.ts:1851">
P2: Bug: `onStepEnd` hook receives stale state during bail. The state manager is not updated with `finalResult` before the hook call — `completeSuccessfulExecution` updates it afterwards. In the normal execution path, `stateManager.update({ data: result, result })` happens *before* `onStepEnd`, so this is an inconsistency that will surface incorrect data to hook consumers.</violation>
<violation number="2" location="packages/core/src/workflow/core.ts:1859">
P2: Design concern: `bail()` bypasses output guardrails. When a step bails early, `completeSuccessfulExecution` is called directly from `completeBail` without running through the `applyWorkflowOutputGuardrails` check that guards the normal completion path. If this is intentional, consider adding a code comment; otherwise, the bail result should also be validated by output guardrails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| output: finalResult, | ||
| }); | ||
|
|
||
| return completeSuccessfulExecution(finalResult, { |
There was a problem hiding this comment.
P2: Design concern: bail() bypasses output guardrails. When a step bails early, completeSuccessfulExecution is called directly from completeBail without running through the applyWorkflowOutputGuardrails check that guards the normal completion path. If this is intentional, consider adding a code comment; otherwise, the bail result should also be validated by output guardrails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/workflow/core.ts, line 1859:
<comment>Design concern: `bail()` bypasses output guardrails. When a step bails early, `completeSuccessfulExecution` is called directly from `completeBail` without running through the `applyWorkflowOutputGuardrails` check that guards the normal completion path. If this is intentional, consider adding a code comment; otherwise, the bail result should also be validated by output guardrails.</comment>
<file context>
@@ -1688,11 +1809,70 @@ export function createWorkflow<
+ output: finalResult,
+ });
+
+ return completeSuccessfulExecution(finalResult, {
+ stepId: step.id,
+ stepName,
</file context>
Deploying voltagent with
|
| Latest commit: |
f5d1d02
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2cb58fad.voltagent.pages.dev |
| Branch Preview URL: | https://feat-workflow-execution-prim.voltagent.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/workflow/core.ts (1)
2488-2507: Outer catch bail handling is a reasonable safety net, but step span may be orphaned.If a
WorkflowBailSignalsomehow escapes the step-level catch and reaches this outer catch,completeSuccessfulExecutionis called directly without ending the current step span (unlikecompleteBailwhich callstraceContext.endStepSpan). This could leave an orphaned trace span. In practice this path should be rare, but it's worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.ts` around lines 2488 - 2507, The outer catch handling for isWorkflowBailSignal returns via completeSuccessfulExecution but does not end the current step trace span, risking an orphaned span; update this branch (inside the isWorkflowBailSignal block) to mirror completeBail by calling traceContext.endStepSpan(...) for the current step (use executionContext.currentStepIndex and the bailStep info/ids) before returning completeSuccessfulExecution so the step span is properly closed; ensure you use the same traceContext methods and arguments pattern as completeBail and still pass the finalResult and bail step metadata into completeSuccessfulExecution.packages/core/src/workflow/core.spec.ts (1)
337-381: Add a test forbail()with no argumentsThe PR description documents
bail(result?)—resultis optional — but all bail tests invoke it with an argument. An uncoveredbail()call could hide a bug in the no-result finalisation path (e.g.,result.resultshould benull/undefinedandstatusshould still be"completed").💡 Suggested additional test case
+ it("should support bail() with no result to complete early with null output", async () => { + const memory = new Memory({ storage: new InMemoryStorageAdapter() }); + + const workflow = createWorkflow( + { + id: "execution-primitives-bail-no-result", + name: "Execution Primitives Bail No Result", + input: z.object({ value: z.number() }), + result: z.object({ final: z.number() }), + memory, + }, + andThen({ + id: "bail-no-result", + execute: async ({ bail }) => { + bail(); + }, + }), + ); + + const registry = WorkflowRegistry.getInstance(); + registry.registerWorkflow(workflow); + + const result = await workflow.run({ value: 1 }); + + expect(result.status).toBe("completed"); + expect(result.result).toBeNull(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.spec.ts` around lines 337 - 381, Add a new spec that mirrors the existing "should support bail(result)..." test but calls bail() with no argument to verify the workflow completes with no result value; locate the test setup using createWorkflow, andThen blocks (use a prepare step, a "bail-step" whose execute calls bail() with no args, and a subsequent step that must not run), run the workflow and assert result.status === "completed", result.result is null or undefined (adjust expectation to the project's convention), final step flag remains false, and the persisted memory (Memory.getWorkflowState) shows status "completed" and no output value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/workflow/core.spec.ts`:
- Around line 337-381: Add a new spec that mirrors the existing "should support
bail(result)..." test but calls bail() with no argument to verify the workflow
completes with no result value; locate the test setup using createWorkflow,
andThen blocks (use a prepare step, a "bail-step" whose execute calls bail()
with no args, and a subsequent step that must not run), run the workflow and
assert result.status === "completed", result.result is null or undefined (adjust
expectation to the project's convention), final step flag remains false, and the
persisted memory (Memory.getWorkflowState) shows status "completed" and no
output value.
In `@packages/core/src/workflow/core.ts`:
- Around line 2488-2507: The outer catch handling for isWorkflowBailSignal
returns via completeSuccessfulExecution but does not end the current step trace
span, risking an orphaned span; update this branch (inside the
isWorkflowBailSignal block) to mirror completeBail by calling
traceContext.endStepSpan(...) for the current step (use
executionContext.currentStepIndex and the bailStep info/ids) before returning
completeSuccessfulExecution so the step span is properly closed; ensure you use
the same traceContext methods and arguments pattern as completeBail and still
pass the finalResult and bail step metadata into completeSuccessfulExecution.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/workflow/core.ts">
<violation number="1" location="packages/core/src/workflow/core.ts:2499">
P2: This bail handling logic is unreachable dead code. Bail signals are fully caught and handled by the inner step execution loop.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/workflow/core.ts (1)
2261-2279:⚠️ Potential issue | 🟠 MajorAdd
RESULT_SCHEMAgeneric parameter tocreateStepExecutionContextand constrainbailtypeThe
bailfunction in the step execution context currently acceptsresult?: unknown, allowing steps to pass incompatible results without compile-time validation. SinceRESULT_SCHEMAis available in scope (theexecute()method operates within theWorkflow<INPUT_SCHEMA, RESULT_SCHEMA>context), bothbailFnandcreateStepExecutionContextshould use it to ensure type safety. The framework currently works around this with a runtime cast (bailSignal.result as z.infer<RESULT_SCHEMA>at lines 1824 and 2498), but steps should have compile-time guarantees. UpdatecreateStepExecutionContextto accept aRESULT_SCHEMAgeneric and typebail: (result?: z.infer<RESULT_SCHEMA>) => never.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.ts` around lines 2261 - 2279, The step execution context currently allows bail to accept unknown; update createStepExecutionContext to take an extra RESULT_SCHEMA generic and ensure the bail function is typed as (result?: z.infer<RESULT_SCHEMA>) => never so steps get compile-time guarantees; change the call site in execute() to pass the Workflow's RESULT_SCHEMA generic into createStepExecutionContext and update bailFn (and any related bailSignal usage) to use z.infer<RESULT_SCHEMA> instead of relying on runtime casts (e.g., remove casts like bailSignal.result as z.infer<RESULT_SCHEMA> and surface the proper typed bail through createStepExecutionContext and bailFn).
🧹 Nitpick comments (1)
packages/core/src/workflow/core.ts (1)
75-83:WorkflowAbortSignalsilently relies on the"WORKFLOW_CANCELLED"string contract
WorkflowAbortSignalpasses"WORKFLOW_CANCELLED"tosuper(...)so that the existingstepError.message === "WORKFLOW_CANCELLED"checks at Lines 2378 and 2521 treat it as a cancellation. This is intentional but invisible — if the string literal or the detection predicate ever diverge, abort signals will be misrouted. A brief comment here explaining the dependency would help future maintainers.💡 Suggested clarification comment
class WorkflowAbortSignal extends Error { readonly reason?: string; constructor(reason?: string) { + // Message must remain "WORKFLOW_CANCELLED" so the existing WORKFLOW_CANCELLED + // detection branches also handle abort signals; isWorkflowAbortSignal then + // promotes the step-supplied reason over controller/registry defaults. super("WORKFLOW_CANCELLED"); this.name = "WorkflowAbortSignal"; this.reason = reason; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.ts` around lines 75 - 83, The WorkflowAbortSignal class relies on passing the literal "WORKFLOW_CANCELLED" to Error so external checks that compare stepError.message === "WORKFLOW_CANCELLED" treat it as a cancellation; add a short clarifying comment above class WorkflowAbortSignal that documents this contract and its dependency on those message equality checks, and to make it robust, extract the literal into a shared constant (e.g., WORKFLOW_CANCELLED) and use that constant inside WorkflowAbortSignal and wherever the stepError.message is compared so the contract is explicit and less error-prone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 2493-2518: The outer catch currently handles a WorkflowBailSignal
by calling completeSuccessfulExecution directly and thus skips the step-data
update, emitting the "step-complete" event, and firing hooks.onStepEnd; change
the outer-catch branch that checks isWorkflowBailSignal(error) to delegate to
completeBail(...) instead of calling completeSuccessfulExecution so that
executionContext.stepData is updated, the step-complete event is emitted, and
hooks?.onStepEnd?.(stateManager.state) runs; use the same parameters/step info
you already compute (bailStepIndex, bailStep, bailStepName, finalResult) when
invoking completeBail to match the inner-catch behavior (refer to completeBail,
executionContext, completeSuccessfulExecution, and isWorkflowBailSignal).
---
Outside diff comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 2261-2279: The step execution context currently allows bail to
accept unknown; update createStepExecutionContext to take an extra RESULT_SCHEMA
generic and ensure the bail function is typed as (result?:
z.infer<RESULT_SCHEMA>) => never so steps get compile-time guarantees; change
the call site in execute() to pass the Workflow's RESULT_SCHEMA generic into
createStepExecutionContext and update bailFn (and any related bailSignal usage)
to use z.infer<RESULT_SCHEMA> instead of relying on runtime casts (e.g., remove
casts like bailSignal.result as z.infer<RESULT_SCHEMA> and surface the proper
typed bail through createStepExecutionContext and bailFn).
---
Nitpick comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 75-83: The WorkflowAbortSignal class relies on passing the literal
"WORKFLOW_CANCELLED" to Error so external checks that compare stepError.message
=== "WORKFLOW_CANCELLED" treat it as a cancellation; add a short clarifying
comment above class WorkflowAbortSignal that documents this contract and its
dependency on those message equality checks, and to make it robust, extract the
literal into a shared constant (e.g., WORKFLOW_CANCELLED) and use that constant
inside WorkflowAbortSignal and wherever the stepError.message is compared so the
contract is explicit and less error-prone.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/src/workflow/core.ts (1)
2532-2542:reasonFromAbortErrorlogic duplicated fromresolveCancellationReason— consider extracting.
resolveCancellationReason(line 1889) is defined inside the for-loop closure and therefore unavailable in the outer catch. The inline check here re-implements the sameisWorkflowAbortSignal(error) && error.reasonextraction. If more control-flow paths are added in the future, this duplication will become a maintenance burden.♻️ Suggested optional refactor — lift a small helper to outer scope
Define a module-level (or
executeInternal-scoped) helper:const extractAbortReason = (err: unknown): string | undefined => isWorkflowAbortSignal(err) && err.reason ? err.reason : undefined;Then both
resolveCancellationReasonand the outer catch can delegate to it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.ts` around lines 2532 - 2542, The code duplicates the abort-reason extraction logic (isWorkflowAbortSignal && .reason) in both resolveCancellationReason (inside the for-loop) and the outer catch where reasonFromAbortError is computed; extract that logic into a small shared helper (e.g., extractAbortReason) at executeInternal/module scope and replace both resolveCancellationReason and the outer catch's inline check to call extractAbortReason(error) so both locations delegate to the same function and avoid duplication; ensure the helper returns string | undefined and keep existing callers (resolveCancellationReason, the outer catch) unchanged in behavior.packages/core/src/workflow/internal/utils.ts (1)
93-99: Unnecessary optional chaining on non-optionalexecutionContext.
executionContext: WorkflowExecutionContextis typed as non-optional, soexecutionContext?.stepData.get(stepId)silently returnsundefinedifexecutionContextis somehow absent at runtime, masking a programming error instead of surfacing it as a TypeError. Remove the?.:🔧 Proposed fix
- getStepResult: <T = unknown>(stepId: string) => { - const stepData = executionContext?.stepData.get(stepId); + getStepResult: <T = unknown>(stepId: string) => { + const stepData = executionContext.stepData.get(stepId); if (!stepData || stepData.output === undefined) { return null; } return stepData.output as T; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/internal/utils.ts` around lines 93 - 99, The getStepResult function is using optional chaining on a non-optional executionContext which can mask programming errors; change executionContext?.stepData.get(stepId) to executionContext.stepData.get(stepId) in getStepResult so a missing executionContext throws immediately (or add an explicit guard if you want a clearer error), referencing the getStepResult method and the executionContext variable to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 1683-1748: completeBail currently calls hooks?.onStepEnd before
the stateManager is updated with the bail result, so hooks see stale state; fix
by updating stateManager with the bail result (e.g. call stateManager.update({
data: finalResult, result: finalResult }) or the project-equivalent update
method) immediately after setting stepData/output/status and before invoking
hooks?.onStepEnd, then call completeSuccessfulExecution as before; reference
symbols: completeBail, hooks?.onStepEnd, stateManager.update,
completeSuccessfulExecution, stateManager.state.
---
Nitpick comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 2532-2542: The code duplicates the abort-reason extraction logic
(isWorkflowAbortSignal && .reason) in both resolveCancellationReason (inside the
for-loop) and the outer catch where reasonFromAbortError is computed; extract
that logic into a small shared helper (e.g., extractAbortReason) at
executeInternal/module scope and replace both resolveCancellationReason and the
outer catch's inline check to call extractAbortReason(error) so both locations
delegate to the same function and avoid duplication; ensure the helper returns
string | undefined and keep existing callers (resolveCancellationReason, the
outer catch) unchanged in behavior.
In `@packages/core/src/workflow/internal/utils.ts`:
- Around line 93-99: The getStepResult function is using optional chaining on a
non-optional executionContext which can mask programming errors; change
executionContext?.stepData.get(stepId) to executionContext.stepData.get(stepId)
in getStepResult so a missing executionContext throws immediately (or add an
explicit guard if you want a clearer error), referencing the getStepResult
method and the executionContext variable to locate the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/workflow/core.ts (1)
1719-1754: Redundant doublestateManager.update— optional cleanup
completeBailcallsstateManager.update({ data: finalResult, result: finalResult })at lines 1719–1728 (correctly, beforeonStepEnd) and then immediately delegates tocompleteSuccessfulExecution(finalResult, ...)at line 1754, which performs the identicalstateManager.updateagain (lines 1592–1601). The second write is a no-op but can mislead readers into thinkingcompleteSuccessfulExecutionis the authoritative owner of the state update.♻️ Suggested refactor — skip re-update in `completeSuccessfulExecution` when already set
Pass a flag or use a separate internal variant that skips the state update since it was already applied in
completeBail:- const completeSuccessfulExecution = async ( - result: z.infer<RESULT_SCHEMA> | null, - bailInfo?: { - stepId: string; - stepName: string; - stepIndex: number; - }, - ): Promise<WorkflowExecutionResult<RESULT_SCHEMA, RESUME_SCHEMA>> => { - if (result === null) { - stateManager.update({ result: null }); - } else { - stateManager.update({ data: result, result }); - } + const completeSuccessfulExecution = async ( + result: z.infer<RESULT_SCHEMA> | null, + bailInfo?: { stepId: string; stepName: string; stepIndex: number }, + skipStateUpdate = false, + ): Promise<WorkflowExecutionResult<RESULT_SCHEMA, RESUME_SCHEMA>> => { + if (!skipStateUpdate) { + if (result === null) { + stateManager.update({ result: null }); + } else { + stateManager.update({ data: result, result }); + } + }Then in
completeBailline 1754:- return completeSuccessfulExecution(finalResult, { stepId: step.id, stepName, stepIndex }); + return completeSuccessfulExecution(finalResult, { stepId: step.id, stepName, stepIndex }, true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/workflow/core.ts` around lines 1719 - 1754, completeBail currently updates stateManager with data/result and then calls completeSuccessfulExecution which performs the same stateManager.update again; change completeSuccessfulExecution to accept an optional flag (e.g., skipStateUpdate or internal variant like completeSuccessfulExecutionNoUpdate) and in completeBail call that variant or pass the flag to avoid the redundant stateManager.update call, ensuring you still call hooks?.onStepEnd?.(stateManager.state) and emit the same event before returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 2266-2271: The andTap implementation swallows WorkflowBailSignal
and WorkflowAbortSignal because it catches all errors and returns context.data;
update the andTap execute wrapper to detect and re-throw signals using the core
helpers (isWorkflowBailSignal and isWorkflowAbortSignal) before logging and
returning, so that bailFn/abortFn propagate; also audit other nested step
handlers that compare error.message and replace those checks with
isWorkflowBailSignal/isWorkflowAbortSignal to consistently re-throw signals
(refer to symbols: andTap, bailFn, abortFn, WorkflowBailSignal,
WorkflowAbortSignal, isWorkflowBailSignal, isWorkflowAbortSignal).
---
Nitpick comments:
In `@packages/core/src/workflow/core.ts`:
- Around line 1719-1754: completeBail currently updates stateManager with
data/result and then calls completeSuccessfulExecution which performs the same
stateManager.update again; change completeSuccessfulExecution to accept an
optional flag (e.g., skipStateUpdate or internal variant like
completeSuccessfulExecutionNoUpdate) and in completeBail call that variant or
pass the flag to avoid the redundant stateManager.update call, ensuring you
still call hooks?.onStepEnd?.(stateManager.state) and emit the same event before
returning.
| const bailFn = (result?: z.infer<RESULT_SCHEMA>): never => { | ||
| throw new WorkflowBailSignal<z.infer<RESULT_SCHEMA>>(result); | ||
| }; | ||
| const abortFn = (): never => { | ||
| throw new WorkflowAbortSignal(`${WORKFLOW_ABORT_REASON_DEFAULT}: ${stepName}`); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for try/catch blocks in step implementations that might swallow bail/abort signals
find packages/core/src/workflow/steps -name "*.ts" -type f | while read file; do
echo "=== File: $file ==="
rg -n -A 10 'catch\s*\(' "$file"
doneRepository: VoltAgent/voltagent
Length of output: 6167
🏁 Script executed:
#!/bin/bash
# Check if nested step files reference WorkflowBailSignal or abort/bail handling
rg -n --type=ts 'WorkflowBailSignal|WorkflowAbortSignal|isWorkflowBailSignal|isWorkflowAbortSignal' packages/core/src/workflow/steps/Repository: VoltAgent/voltagent
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Also check the core workflow file for how signals are defined and used
rg -n --type=ts 'class WorkflowBailSignal|class WorkflowAbortSignal|isWorkflowBailSignal|isWorkflowAbortSignal' packages/core/src/workflow/Repository: VoltAgent/voltagent
Length of output: 902
🏁 Script executed:
#!/bin/bash
# Check the full context of and-tap.ts to see if it should be re-throwing
cat -n packages/core/src/workflow/steps/and-tap.tsRepository: VoltAgent/voltagent
Length of output: 2269
🏁 Script executed:
#!/bin/bash
# Check the full context of and-race.ts to see the catch handling
cat -n packages/core/src/workflow/steps/and-race.tsRepository: VoltAgent/voltagent
Length of output: 10037
🏁 Script executed:
#!/bin/bash
# Search for actual usage of WorkflowBailSignal/WorkflowAbortSignal in core.ts to understand when they're thrown
rg -n -B 5 -A 5 'throw new WorkflowBailSignal|throw new WorkflowAbortSignal' packages/core/src/workflow/core.tsRepository: VoltAgent/voltagent
Length of output: 891
Fix signal propagation in nested steps, especially andTap
The concern is partially valid. Most nested step implementations do re-throw errors, allowing WorkflowBailSignal and WorkflowAbortSignal to propagate. However, andTap (lines 54-59 of and-tap.ts) catches all errors without re-throwing:
} catch (error) {
getGlobalLogger()...error("Error executing tap step", { error: error });
}
return context.data as DATA; // ❌ Swallows bail/abort signalsSince bail() and abort() are provided in the context, users can (intentionally or accidentally) call them within a tap's execute function. These calls would be silently swallowed, breaking the abort/bail mechanism.
Additionally, other nested steps handle errors through fragile message checks (error.message === "WORKFLOW_SUSPENDED") rather than using the proper signal checkers (isWorkflowBailSignal, isWorkflowAbortSignal) defined in core.ts. This works for the common path but creates maintenance risk.
Fix: In andTap, explicitly check and re-throw signals before logging. Throughout nested step implementations, use isWorkflowBailSignal() and isWorkflowAbortSignal() for explicit signal handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/workflow/core.ts` around lines 2266 - 2271, The andTap
implementation swallows WorkflowBailSignal and WorkflowAbortSignal because it
catches all errors and returns context.data; update the andTap execute wrapper
to detect and re-throw signals using the core helpers (isWorkflowBailSignal and
isWorkflowAbortSignal) before logging and returning, so that bailFn/abortFn
propagate; also audit other nested step handlers that compare error.message and
replace those checks with isWorkflowBailSignal/isWorkflowAbortSignal to
consistently re-throw signals (refer to symbols: andTap, bailFn, abortFn,
WorkflowBailSignal, WorkflowAbortSignal, isWorkflowBailSignal,
isWorkflowAbortSignal).
1 participant
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
Workflow step context did not expose execution primitives for terminal flow control and stable introspection. There was no
bail(),abort(), typedgetStepResult(), orgetInitData()API in the step runtime/context surface.What is the new behavior?
Adds execution primitives to workflow step context:
bail(result?): complete the workflow early ascompletedwith an optional final resultabort(): terminate the workflow ascancelledgetStepResult(stepId): read prior step output safely from contextgetInitData(): read original workflow input across normal runs and resume cyclesAlso includes:
fixes (issue)
N/A
Notes for reviewers
pnpm --filter @voltagent/core test -- src/workflow/core.spec.ts src/workflow/core.spec-d.tspnpm --filter voltagent-example-with-workflow exec tsx -e '...execution-primitives smoke...'(printsexecution-primitives smoke ok)docs/workflow-parity-plans/is intentionally not included in this PR.Summary by cubic
Adds execution primitives to the workflow step context for early finish, cancellation, and safe introspection. Runtime now types bail payloads, handles bail/abort consistently (including nested steps), and preserves the original input across resumes.
New Features
Bug Fixes
Written for commit f5d1d02. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests