-
Notifications
You must be signed in to change notification settings - Fork 30
BFS traversal visits all branches regardless of conditional edge results, causing unintended action execution #45
Description
This bug was discovered with the help of Claude Opus 4.5 while debugging workflow conditional branching in a production application.
Version
@inngest/workflow-kit: 0.2.1-alpha
Description
The BFS traversal in graph.js queues all child nodes before evaluating conditional edges. This causes both "if" and "else" branches to be visited. While the handler is correctly skipped when a conditional fails, the children of skipped nodes are still queued and visited, leading to unintended action execution.
Root Cause
In graph.js, the bfs function:
graph.forEachOutNeighbor(next, (id, node) => {
if (seen.has(id)) return;
nodes.push(node);
queue.push(id); // ← Children queued BEFORE conditional is checked
});
for (let node of nodes) {
// Conditional checked HERE, but children already in queue
await cb(node.action, edge);
}
Example
Workflow DAG:
$source → wait_for_event → notify_success (if received=true)
→ mark_overdue (else) → send_reminder → escalate
When wait_for_event returns { received: true }:
| Expected | Actual |
|---|---|
| notify_success executes | notify_success executes |
| mark_overdue skipped | mark_overdue handler skipped ✓ |
| send_reminder never visited | send_reminder visited (handler skipped because $.state.mark_overdue is undefined) |
| escalate never visited | escalate executed because $.state.send_reminder is undefined, and !!undefined === false matches "else" conditional |
The Compound Problem
BFS visits all nodes regardless of parent conditional
Skipped actions don't write state → $.state.<skipped_action> returns undefined
"else" conditionals treat undefined as falsy → they execute
This causes error/escalation paths to run even on successful workflows
**Minimal Reproduction**
const workflow = {
actions: [
{ id: "decision", kind: "wait_for_event", inputs: { timeout: "1s" } },
{ id: "success_path", kind: "log", inputs: { message: "Success!" } },
{ id: "failure_path", kind: "log", inputs: { message: "Failed" } },
{ id: "escalate", kind: "notify", inputs: { to: "admin" } },
],
edges: [
{ from: "$source", to: "decision" },
{ from: "decision", to: "success_path", conditional: { type: "if", ref: "!ref($.output.received)" } },
{ from: "decision", to: "failure_path", conditional: { type: "else", ref: "!ref($.output.received)" } },
{ from: "failure_path", to: "escalate" },
],
};
// When decision returns { received: true }:
// Expected: success_path runs, failure_path and escalate don't
// Actual: success_path runs, failure_path skipped, escalate RUNS (because $.state.failure_path is undefined)
Current Workaround
Add conditionals to every edge in a branch that reference the original decision point:
edges: [
{ from: "decision", to: "success_path", conditional: { type: "if", ref: "!ref($.output.received)" } },
{ from: "decision", to: "failure_path", conditional: { type: "else", ref: "!ref($.output.received)" } },
// Must add conditional to ALL downstream edges:
{ from: "failure_path", to: "escalate", conditional: { type: "else", ref: "!ref($.state.decision.received)" } },
];
For the final branch decision (where one branch may have been skipped entirely), use type: "match" with explicit values to avoid undefined being treated as falsy:
{ from: "wait_2", to: "success_late", conditional: { type: "match", ref: "!ref($.state.wait_2.received)", value: true } },
{ from: "wait_2", to: "escalate", conditional: { type: "match", ref: "!ref($.state.wait_2.received)", value: false } },
Suggested Fix
The conditional should be evaluated before queuing children, or children of skipped nodes should not be queued:
for (let node of nodes) {
const shouldExecute = evaluateConditional(edge);
if (!shouldExecute) {
// Don't queue this node's children
continue;
}
await cb(node.action, edge);
// Only now queue children
queueChildren(node);
}