Context
PR #2258 introduced Status.SKIPPED: a node bypassed via cancel_node where downstream nodes still execute. CANCELLED is the complementary case: the node terminates intentionally, downstream does not execute, but other branches continue and the graph can still complete successfully. No error propagates.
This is listed as item 6 in #2387 (graph looping). Opening it separately since CANCELLED can be implemented independently of full cycle support.
Desired behavior
When a hook signals cancellation on a node:
- The node does not execute
- It records
Status.CANCELLED
- Its outgoing edges are not traversed; downstream nodes do not run
- Other branches that don't depend on the cancelled node continue normally
This is the TypeScript SDK's existing behavior. From _runBeforeNodeCall in graph.ts:
if (beforeEvent.cancel) {
const result = new NodeResult({ nodeId: node.id, status: Status.CANCELLED, duration: 0 })
nodeState.status = Status.CANCELLED
// _findReady returns [] when status !== COMPLETED, so downstream is never scheduled
return result
}
API recommendation
@yananym flagged the naming tension in their review on #2258: the TS SDK uses cancel for abort-branch behavior, but our PR used cancel_node for bypass-and-continue (SKIPPED). The words are semantically reversed.
I'd recommend Option A: rename cancel_node to skip_node in #2258 before it merges, then use cancel_node for CANCELLED:
skip_node: bool | str = False — bypass this node, downstream runs (Status.SKIPPED)
cancel_node: bool | str = False — abort this branch, downstream does not run (Status.CANCELLED)
The rename is low-cost right now: #2258 hasn't merged yet, and cancel_node was broken before the fix (it raised RuntimeError), so no one was relying on its behavior. After merge, any rename becomes a breaking change requiring a deprecation cycle.
Option B — keeping cancel_node = SKIPPED and adding a new field (e.g., abort_node) for CANCELLED — would preserve the current name but cement a mismatch with the TS SDK and leave cancel_node meaning the opposite of what "cancel" suggests.
If Option A looks right to maintainers, I'll update #2258 with the rename before it lands.
Implementation notes
- Add
Status.CANCELLED to the Status enum
- Add
cancelled_nodes: set[GraphNode] to GraphState
_execute_node: CANCELLED nodes go into cancelled_nodes, not completed_nodes
_find_newly_ready_nodes: does not traverse outgoing edges of CANCELLED nodes
GraphResult: add cancelled_nodes: int counter
- Serialize/deserialize
cancelled_nodes alongside failed_nodes
References
Context
PR #2258 introduced
Status.SKIPPED: a node bypassed viacancel_nodewhere downstream nodes still execute. CANCELLED is the complementary case: the node terminates intentionally, downstream does not execute, but other branches continue and the graph can still complete successfully. No error propagates.This is listed as item 6 in #2387 (graph looping). Opening it separately since CANCELLED can be implemented independently of full cycle support.
Desired behavior
When a hook signals cancellation on a node:
Status.CANCELLEDThis is the TypeScript SDK's existing behavior. From
_runBeforeNodeCallingraph.ts:API recommendation
@yananym flagged the naming tension in their review on #2258: the TS SDK uses
cancelfor abort-branch behavior, but our PR usedcancel_nodefor bypass-and-continue (SKIPPED). The words are semantically reversed.I'd recommend Option A: rename
cancel_nodetoskip_nodein #2258 before it merges, then usecancel_nodefor CANCELLED:skip_node: bool | str = False— bypass this node, downstream runs (Status.SKIPPED)cancel_node: bool | str = False— abort this branch, downstream does not run (Status.CANCELLED)The rename is low-cost right now: #2258 hasn't merged yet, and
cancel_nodewas broken before the fix (it raised RuntimeError), so no one was relying on its behavior. After merge, any rename becomes a breaking change requiring a deprecation cycle.Option B — keeping
cancel_node= SKIPPED and adding a new field (e.g.,abort_node) for CANCELLED — would preserve the current name but cement a mismatch with the TS SDK and leavecancel_nodemeaning the opposite of what "cancel" suggests.If Option A looks right to maintainers, I'll update #2258 with the rename before it lands.
Implementation notes
Status.CANCELLEDto theStatusenumcancelled_nodes: set[GraphNode]toGraphState_execute_node: CANCELLED nodes go intocancelled_nodes, notcompleted_nodes_find_newly_ready_nodes: does not traverse outgoing edges of CANCELLED nodesGraphResult: addcancelled_nodes: intcountercancelled_nodesalongsidefailed_nodesReferences
cancel_nodebug reportStatus.SKIPPED(bypass + continue downstream)graph.ts_runBeforeNodeCall