fix: close workflow review gaps
This commit is contained in:
@@ -76,10 +76,14 @@ not duplicate the lineage's scope membership.
|
||||
|
||||
Lineage remains a virtual worldview: committed scope state plus writes visible
|
||||
to one branch. Gathering several lineages does not require turning lineage
|
||||
ancestry into a multi-parent graph. A partial gather can create an intermediate
|
||||
lineage under the branches' common parent, retaining multi-input provenance in
|
||||
activation-token metadata. A final gather can merge that lineage with remaining
|
||||
siblings and resume the blocked parent continuation.
|
||||
ancestry into a multi-parent graph. Every input lineage must belong to the same
|
||||
runtime scope. Because lineages have one parent, their ancestry chains have at
|
||||
most one deepest shared lineage: the lowest common ancestor is the deterministic
|
||||
merge base. Different scopes or no shared ancestor fail before state mutation.
|
||||
A partial gather creates its intermediate lineage under that merge base and
|
||||
retains multi-input provenance in activation-token metadata. A final gather can
|
||||
merge that lineage with remaining siblings and resume the blocked parent
|
||||
continuation.
|
||||
|
||||
The first gather merge policy is fail-closed:
|
||||
|
||||
@@ -92,11 +96,30 @@ State-field reducers remain the source of truth for legitimate concurrent
|
||||
merges. The gather policy determines what happens when patches cannot be
|
||||
merged; the initial behavior is to fail rather than choose a last writer.
|
||||
|
||||
Gather slots have declaration order, and that order is the canonical reducer
|
||||
replay order. After choosing the merge base, the runtime applies each selected
|
||||
lineage's writes after that base in declared-slot order, never arrival,
|
||||
scheduler, or frame-id order. A bucket accepts exactly one token for each slot;
|
||||
a second token for the same activation and slot fails the activation instead of
|
||||
making an alternative-path race decide the result. Order-sensitive reducers
|
||||
such as append are therefore deterministic in synchronous and asynchronous
|
||||
execution.
|
||||
|
||||
Branch execution order may be deterministic in the synchronous runtime and
|
||||
overlap in the asynchronous runtime. Both modes must produce equivalent graph
|
||||
semantics. Scheduler order decides when compatible work progresses, never which
|
||||
arrivals belong together.
|
||||
|
||||
An unhandled branch failure makes its gather activation terminally failed and
|
||||
stops further branch admission. The runtime requests cancellation of admitted
|
||||
siblings, awaits every sibling's settlement, and accepts no later state or
|
||||
trace commits from them; external effects that already occurred cannot be
|
||||
rolled back. It then marks the failed activation's uncommitted lineages
|
||||
abandoned and permanently non-mergeable, invalidates and removes every pending
|
||||
gather token, marks sibling frames cancelled or failed, and persists the failed
|
||||
run. Restore may inspect those frames and lineages but cannot schedule them or
|
||||
consume a token from the failed activation.
|
||||
|
||||
`END` and explicit `EndNode` represent workflow/subgraph termination, not a
|
||||
generic way to complete any child frame. A foreach item returns through a
|
||||
back-edge targeting its owning `ForeachNode`; the runtime completes that item
|
||||
@@ -153,9 +176,10 @@ node is a pass-through marker with no barrier contract.
|
||||
|
||||
- Edge identity gains gather-slot significance only when its target is a
|
||||
gather; ordinary edge semantics stay unchanged.
|
||||
- Workflow validation must prove that every gather slot has an incoming edge,
|
||||
reject slots on non-gather targets, and preserve one successor per ordinary
|
||||
`(node, outcome)` pair.
|
||||
- Workflow validation must require every gather-target edge to name exactly one
|
||||
declared slot, reject missing or unknown gather slots, prove that every slot
|
||||
has an incoming edge, reject slots on non-gather targets, and preserve one
|
||||
successor per ordinary `(node, outcome)` pair.
|
||||
- Checkpoints must persist pending gather arrivals and activation provenance so
|
||||
interruption/resume cannot mix loop iterations or subgraph invocations.
|
||||
- Runtime operations should resolve a frame, its lineage, and its scope through
|
||||
@@ -169,8 +193,9 @@ node is a pass-through marker with no barrier contract.
|
||||
ordinary node output.
|
||||
- Fork/gather should generalize concurrent-foreach lineage and barrier helpers,
|
||||
not create a second state-patch system.
|
||||
- Runtime branch failures remain execution failures in the first version. Skip,
|
||||
collect, race, first-success, cancellation, and timeout policies are deferred.
|
||||
- Runtime branch failures remain execution failures with the terminal cleanup
|
||||
above. Skip, collect, race, first-success, configurable cancellation, and
|
||||
timeout policies are deferred.
|
||||
|
||||
## Open Questions
|
||||
|
||||
|
||||
@@ -4,7 +4,8 @@
|
||||
|
||||
Implemented on 2026-09-04. This document specifies canonical foreach
|
||||
body-return semantics. It does not include the separately planned ergonomic
|
||||
Python DSL or authorize fork/gather implementation.
|
||||
Python DSL or the explicit fork/gather implementation described by
|
||||
[ADR-0006](../../adr/0006-explicit-fork-and-topology-driven-gather.md).
|
||||
|
||||
## Purpose
|
||||
|
||||
|
||||
@@ -93,6 +93,17 @@ behavior. The counter is incremented before user code or external capability
|
||||
code begins, so failures and interrupts still consume the attempt that caused
|
||||
them.
|
||||
|
||||
For a durable run, admission is checkpointed before dispatch. The checkpoint
|
||||
contains the incremented counter, assigned step number, selected frame and
|
||||
node, and an admitted-but-not-completed marker. Dispatch may begin only after
|
||||
that checkpoint succeeds. If the process stops at that boundary, restore keeps
|
||||
the attempt consumed, clears the abandoned admission marker, and requeues the
|
||||
frame; a retry is a new attempt with a new step number. The in-memory executor
|
||||
applies the same counter transition without requiring a persistence backend.
|
||||
As with any crash after external dispatch and before result persistence, retry
|
||||
may repeat external effects; the budget records attempts and does not provide
|
||||
exactly-once execution.
|
||||
|
||||
If `steps_executed == max_steps`, the next attempted dispatch is denied. A
|
||||
budget of one therefore admits exactly one step. The denied step does not
|
||||
increment the counter and does not invoke a handler.
|
||||
@@ -132,6 +143,15 @@ Reserved async attempts remain consumed even if one handler raises. This
|
||||
matches the rule that admission, rather than successful completion, consumes
|
||||
the budget and avoids making counts depend on task completion timing.
|
||||
|
||||
The runtime awaits every handler in an admitted batch before finalizing any
|
||||
result. It then finalizes in reserved ready-queue order. Handled foreach item
|
||||
failures follow their declared `skip` or `collect` policy. At the first
|
||||
unhandled failure in that order, preceding successful results have committed,
|
||||
the run fails, and later sibling results are discarded without state or trace
|
||||
commits. Because all handler tasks have already settled, no sibling can mutate
|
||||
the failed checkpoint afterward; external effects performed inside a handler
|
||||
remain outside rollback.
|
||||
|
||||
## Exhaustion Behavior
|
||||
|
||||
Exhaustion is a runtime failure, not a workflow outcome. The runtime raises a
|
||||
@@ -160,16 +180,19 @@ administrative rerun can raise the budget is outside ordinary resume semantics.
|
||||
|
||||
## Persistence and Resume
|
||||
|
||||
`RunLimits` and `steps_executed` are serialized inside the existing persisted
|
||||
`RunState` checkpoint. An interrupted run resumes with its original maximum and
|
||||
cumulative count.
|
||||
`RunLimits` and `steps_executed` are serialized inside the persisted `RunState`
|
||||
checkpoint. An interrupted run resumes with its original maximum and cumulative
|
||||
count.
|
||||
|
||||
The persisted run envelope may remain at version 1 because adding dataclass
|
||||
fields with defaults is structurally additive. Loading an older checkpoint
|
||||
that lacks these fields yields the default limit and a zero count. The
|
||||
repository has no declared production migration requirement for reconstructing
|
||||
historical counts that were never recorded; if real stored checkpoints exist,
|
||||
their migration policy must be established before release.
|
||||
A checkpoint that predates step budgets receives one explicit, prospective
|
||||
upgrade: assign the default limit and `steps_executed = 0`, mark the envelope as
|
||||
budget-initialized, and persist the upgraded checkpoint before admitting any
|
||||
new work. Attempts made before the upgrade cannot be reconstructed and are
|
||||
explicitly outside the new budget; every attempt after it is cumulative. A
|
||||
missing counter on an already budget-initialized envelope is corrupt state, not
|
||||
another request for defaults. If the one-time upgrade cannot be persisted,
|
||||
resume fails before dispatch. The persisted envelope version or equivalent
|
||||
migration marker must distinguish these cases.
|
||||
|
||||
Subgraph scopes do not receive independent counters. They are part of the same
|
||||
run and consume the root run's budget. This prevents an outer workflow from
|
||||
@@ -258,6 +281,8 @@ with the run.
|
||||
- A batch claims no more frames than the remaining budget.
|
||||
- Step numbers follow ready-queue order rather than completion order.
|
||||
- Reserved attempts remain counted when one async handler fails.
|
||||
- A still-running sibling settles before an unhandled handler failure is
|
||||
checkpointed, and its later result does not commit state or trace data.
|
||||
- Sync and async runs produce the same count for equivalent serial execution.
|
||||
|
||||
### Persistence and API
|
||||
@@ -265,7 +290,10 @@ with the run.
|
||||
- Limits and counts round-trip through `dump_run_state()` and
|
||||
`load_run_state()`.
|
||||
- A stored interrupted run resumes without resetting or replacing its budget.
|
||||
- Older additive checkpoints receive documented defaults.
|
||||
- A pre-budget checkpoint receives its defaults once, persists the upgraded
|
||||
envelope before dispatch, and cannot receive another fresh budget on reload.
|
||||
- Stopping after the admission checkpoint but before handler start leaves the
|
||||
attempt consumed; retrying the requeued frame consumes a new attempt.
|
||||
- Run inspection exposes effective maximum, executed, and remaining counts.
|
||||
- Trace entries expose deterministic step numbers without becoming the source
|
||||
of enforcement truth.
|
||||
|
||||
@@ -234,6 +234,7 @@ An iteration body returns through its immediate owning foreach:
|
||||
g.connect(each, "loop", record)
|
||||
g.connect(record, "ok", each)
|
||||
g.connect(each, "done", END)
|
||||
g.connect(each, "completed_with_errors", END)
|
||||
```
|
||||
|
||||
Region conflicts, unreachable nodes, body terminals, non-local returns, empty
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
"Loads exact artifact version 3 and edits the seeded builder rather than rebuilding",
|
||||
"Warns that step IDs and state paths must be inspected rather than guessed",
|
||||
"Validates locally and remotely before saving version 4",
|
||||
"Passes an explicit source binding through bindings=... when creating the deployment",
|
||||
"Creates and validates a deployment before starting a durable run"
|
||||
]
|
||||
},
|
||||
|
||||
@@ -133,7 +133,7 @@ its real contract; do not assume those names exist.
|
||||
## Deployment Diagnosis And Durable Runs
|
||||
|
||||
```python
|
||||
from wf_client import DeploymentRequired, WorkflowClientError
|
||||
from wf_client import DeploymentRequired, ProtocolError, WorkflowClientError
|
||||
|
||||
artifact = await app.workflow("invoice", version=2)
|
||||
|
||||
@@ -144,6 +144,12 @@ except DeploymentRequired as error:
|
||||
print("unresolved", error.unresolved_logical_sources)
|
||||
for diagnostic in error.diagnostics:
|
||||
print(diagnostic.code, diagnostic.message)
|
||||
except ProtocolError as error:
|
||||
print("server error", error.code, error.message, error.data)
|
||||
raise
|
||||
except WorkflowClientError as error:
|
||||
print(type(error).__name__, error)
|
||||
raise
|
||||
|
||||
deployment = await artifact.deploy(
|
||||
"invoice.production",
|
||||
|
||||
@@ -154,13 +154,14 @@ def analyze_control_regions(workflow: Workflow) -> ControlRegionAnalysis:
|
||||
# unreachable. The `END` token has no node to record.
|
||||
if isinstance(target_node, EndNode):
|
||||
visited_nodes.add(target_id)
|
||||
recorded_target = owner_stack_by_node.get(target_id)
|
||||
if recorded_target is None:
|
||||
owner_stack_by_node[target_id] = target_stack
|
||||
elif recorded_target != target_stack:
|
||||
record_region_conflict(
|
||||
target_id, (recorded_target, target_stack)
|
||||
)
|
||||
if target_id not in conflicted:
|
||||
recorded_target = owner_stack_by_node.get(target_id)
|
||||
if recorded_target is None:
|
||||
owner_stack_by_node[target_id] = target_stack
|
||||
elif recorded_target != target_stack:
|
||||
record_region_conflict(
|
||||
target_id, (recorded_target, target_stack)
|
||||
)
|
||||
if target_stack:
|
||||
issues.append(
|
||||
ControlRegionIssue(
|
||||
|
||||
@@ -18,9 +18,10 @@ from wf_core.runtime.ops.state import (
|
||||
class LineageStateView:
|
||||
"""Committed state plus writes visible inside one child lineage.
|
||||
|
||||
Today concurrent foreach supplies the writes from barrier metadata. Future
|
||||
native subgraphs and fork/gather should use the same primitive instead of
|
||||
rebuilding foreach-specific overlay logic.
|
||||
Concurrent foreach item writes live in ``RunState.lineages``; the barrier
|
||||
keeps only each item's lineage identity. Future native subgraphs and
|
||||
fork/gather should reuse this primitive instead of rebuilding lineage
|
||||
overlay logic.
|
||||
"""
|
||||
|
||||
base_state: Mapping[str, Any]
|
||||
|
||||
@@ -94,7 +94,6 @@ def _step_foreach_serial(
|
||||
advance_frame(run, frame, outcome=outcome, next_node_id=next_node_id)
|
||||
return run
|
||||
|
||||
loop_start = index.next_node_id(frame.node_id, "loop")
|
||||
item = iterable[loop_index]
|
||||
loop_start, child_id = _admit_item_frame(
|
||||
run=run,
|
||||
@@ -292,7 +291,6 @@ def _admit_concurrent_children(
|
||||
raise WorkflowExecutionError("concurrent foreach requires concurrent policy")
|
||||
|
||||
barrier = activation.barrier
|
||||
loop_start = index.next_node_id(frame.node_id, "loop")
|
||||
while (
|
||||
barrier.next_index < len(iterable)
|
||||
and len(barrier.active_frame_ids) < step.concurrent.max_active
|
||||
|
||||
@@ -213,10 +213,7 @@ def wake_parent_for_child_progress(run: RunState, child_frame_id: str) -> None:
|
||||
load_foreach_activation,
|
||||
)
|
||||
|
||||
try:
|
||||
owner = item_frame_owner(child)
|
||||
except WorkflowExecutionError:
|
||||
raise
|
||||
owner = item_frame_owner(child)
|
||||
if owner is not None:
|
||||
activation = load_foreach_activation(
|
||||
parent, owner.foreach_node_id, owner.activation_id
|
||||
|
||||
@@ -19,6 +19,7 @@ from wf_core import (
|
||||
execute_workflow_async,
|
||||
resume_workflow_async,
|
||||
)
|
||||
from wf_core.runtime.foreach_state import item_frame_owner
|
||||
|
||||
|
||||
async def test_concurrent_foreach_interrupt_returns_before_refill() -> None:
|
||||
@@ -44,6 +45,9 @@ async def test_resume_prioritizes_interrupted_item_before_siblings() -> None:
|
||||
{"route": _interrupt_on_b},
|
||||
)
|
||||
interrupted_trace_len = len(run.trace)
|
||||
interrupted_owner = item_frame_owner(run.frames["root:each#0:1"])
|
||||
assert interrupted_owner is not None
|
||||
interrupted_activation_id = interrupted_owner.activation_id
|
||||
|
||||
resumed = await resume_workflow_async(
|
||||
workflow,
|
||||
@@ -52,15 +56,11 @@ async def test_resume_prioritizes_interrupted_item_before_siblings() -> None:
|
||||
resume_payload={},
|
||||
)
|
||||
|
||||
from wf_core.runtime.foreach_state import item_frame_owner
|
||||
|
||||
interrupted_owner = item_frame_owner(run.frames["root:each#0:1"])
|
||||
assert interrupted_owner is not None
|
||||
assert resumed.status is RunStatus.COMPLETED
|
||||
assert resumed.state["seen"] == ["a", "b", "c"]
|
||||
resumed_owner = item_frame_owner(resumed.frames["root:each#0:1"])
|
||||
assert resumed_owner is not None
|
||||
assert resumed_owner.activation_id == interrupted_owner.activation_id
|
||||
assert resumed_owner.activation_id == interrupted_activation_id
|
||||
assert resumed.trace[interrupted_trace_len].frame_id == "root:each#0:1"
|
||||
assert resumed.trace[interrupted_trace_len].step_type == "interrupt"
|
||||
assert resumed.trace[interrupted_trace_len].outcome == "submitted"
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from wf_core.errors import WorkflowExecutionError
|
||||
@@ -65,8 +67,13 @@ def test_closing_stale_activation_fails_closed() -> None:
|
||||
second = load_or_begin_foreach_activation(frame, "each", mode="serial")
|
||||
save_foreach_activation(frame, second)
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="stale|closed|active"):
|
||||
with pytest.raises(
|
||||
WorkflowExecutionError, match="cannot close stale foreach activation"
|
||||
) as exc_info:
|
||||
close_foreach_activation(frame, first)
|
||||
message = str(exc_info.value)
|
||||
assert repr(first.id) in message
|
||||
assert "'root'" in message
|
||||
|
||||
|
||||
def test_activation_json_round_trip_through_frame_metadata() -> None:
|
||||
@@ -75,7 +82,7 @@ def test_activation_json_round_trip_through_frame_metadata() -> None:
|
||||
activation.barrier.next_index = 2
|
||||
save_foreach_activation(frame, activation)
|
||||
|
||||
dumped = dict(frame.metadata)
|
||||
dumped = json.loads(json.dumps(frame.metadata))
|
||||
restored_frame = ExecutionFrame(
|
||||
id="root", kind="workflow", node_id="each", metadata=dumped
|
||||
)
|
||||
|
||||
@@ -28,18 +28,6 @@ from wf_core.runtime.foreach_state import item_frame_owner
|
||||
from wf_core.runtime.scheduler import add_frame
|
||||
|
||||
|
||||
def _node_use(node_id: str, *, node: str = "record") -> NodeUse:
|
||||
return NodeUse.model_validate(
|
||||
{
|
||||
"id": node_id,
|
||||
"type": "node",
|
||||
"node": node,
|
||||
"input": [{"target": "value", "path": "context.item"}],
|
||||
"output": [{"source": "seen", "target": "state.seen"}],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _serial_workflow() -> Workflow:
|
||||
foreach = ForeachNode.model_validate(
|
||||
{
|
||||
@@ -1360,8 +1348,14 @@ def test_nonlocal_runtime_return_fails_closed_when_validation_is_bypassed() -> N
|
||||
|
||||
from wf_core.runtime.ops.flow import advance_frame
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="non-local|ancestor|immediate"):
|
||||
with pytest.raises(
|
||||
WorkflowExecutionError, match="targets non-immediate ancestor"
|
||||
) as exc_info:
|
||||
advance_frame(run, run.frames["inner-item"], outcome="ok", next_node_id="outer")
|
||||
message = str(exc_info.value)
|
||||
assert "'inner-item'" in message
|
||||
assert "'outer'" in message
|
||||
assert "'inner'" in message
|
||||
|
||||
|
||||
def test_root_frame_targeting_foreach_enters_normally() -> None:
|
||||
@@ -1415,8 +1409,13 @@ def test_completed_activation_cannot_consume_later_activation_result_or_wake() -
|
||||
|
||||
assert second.id != first.id
|
||||
|
||||
with pytest.raises(WorkflowExecutionError, match="closed|superseded"):
|
||||
with pytest.raises(
|
||||
WorkflowExecutionError, match="closed or superseded"
|
||||
) as exc_info:
|
||||
require_foreach_activation(parent, "each", first.id)
|
||||
message = str(exc_info.value)
|
||||
assert repr(first.id) in message
|
||||
assert "'each'" in message
|
||||
|
||||
run = RunState(
|
||||
workflow_name="activation_isolation",
|
||||
|
||||
@@ -361,21 +361,29 @@ def _failed_result(
|
||||
|
||||
|
||||
def test_pending_item_result_rejects_error_index_mismatch() -> None:
|
||||
with pytest.raises(WorkflowExecutionError, match="error.*index|index.*error"):
|
||||
with pytest.raises(WorkflowExecutionError, match="error identity") as exc_info:
|
||||
PendingItemResult.from_metadata(
|
||||
_failed_result(
|
||||
index=0, frame_id="child-0", error_index=7, error_frame="child-0"
|
||||
)
|
||||
)
|
||||
message = str(exc_info.value)
|
||||
assert "index 7" in message
|
||||
assert "index 0" in message
|
||||
assert "frame 'child-0'" in message
|
||||
|
||||
|
||||
def test_pending_item_result_rejects_error_frame_mismatch() -> None:
|
||||
with pytest.raises(WorkflowExecutionError, match="error.*frame|frame.*error"):
|
||||
with pytest.raises(WorkflowExecutionError, match="error identity") as exc_info:
|
||||
PendingItemResult.from_metadata(
|
||||
_failed_result(
|
||||
index=0, frame_id="child-0", error_index=0, error_frame="other"
|
||||
)
|
||||
)
|
||||
message = str(exc_info.value)
|
||||
assert "frame 'other'" in message
|
||||
assert "frame 'child-0'" in message
|
||||
assert "index 0" in message
|
||||
|
||||
|
||||
def test_pending_item_result_accepts_matching_error_identity() -> None:
|
||||
|
||||
@@ -417,6 +417,42 @@ def test_foreach_body_cannot_target_explicit_end_node() -> None:
|
||||
assert matching[0].path == "edges[1]"
|
||||
|
||||
|
||||
def test_explicit_end_reached_from_three_regions_stays_conflicted() -> None:
|
||||
workflow = _workflow(
|
||||
start="start",
|
||||
nodes=[
|
||||
_node("start"),
|
||||
_foreach("f1"),
|
||||
_node("b1"),
|
||||
_foreach("f2"),
|
||||
_node("b2"),
|
||||
{"id": "stop", "type": "end", "outcome": "ok"},
|
||||
],
|
||||
edges=[
|
||||
{"from": "start", "outcome": "direct", "to": "stop"},
|
||||
{"from": "start", "outcome": "left", "to": "f1"},
|
||||
{"from": "start", "outcome": "right", "to": "f2"},
|
||||
{"from": "f1", "outcome": "loop", "to": "b1"},
|
||||
{"from": "b1", "outcome": "ok", "to": "stop"},
|
||||
{"from": "f1", "outcome": "done", "to": END},
|
||||
{"from": "f2", "outcome": "loop", "to": "b2"},
|
||||
{"from": "b2", "outcome": "ok", "to": "stop"},
|
||||
{"from": "f2", "outcome": "done", "to": END},
|
||||
],
|
||||
)
|
||||
|
||||
analysis = analyze_control_regions(workflow)
|
||||
|
||||
conflicts = [
|
||||
issue
|
||||
for issue in analysis.issues
|
||||
if issue.kind == ControlRegionIssueKind.FOREACH_REGION_CONFLICT
|
||||
and issue.path == "nodes[stop]"
|
||||
]
|
||||
assert len(conflicts) == 1
|
||||
assert "stop" not in analysis.owner_stack_by_node
|
||||
|
||||
|
||||
def test_every_unreachable_node_is_reported() -> None:
|
||||
workflow = _workflow(
|
||||
start="work",
|
||||
|
||||
Reference in New Issue
Block a user