Fix nested composite task completion - #267
andystaples merged 3 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
andystaples
left a comment
There was a problem hiding this comment.
The corrected nested-composite completion behavior is preferable. With the fix applied unconditionally and no persisted compatibility mechanism, accepting it also means accepting a narrow replay-compatibility break. The remaining concern is making that release/upgrade impact explicit, rather than retaining the bug.
Replay compatibility: This does not affect ordinary, non-nested when_all/when_any usage. The clearest risk is an existing instance that already progressed past a nested race under the old behavior. For example, in when_any([cancel, when_all(tasks)]), the inner when_all can finish first without notifying its parent; cancellation later wins and the orchestration records cancellation-branch activity calls. After this change, replay selects the inner when_all instead. I reproduced an old history scheduling cleanup that replays as finish with this PR and fails with NonDeterminismError. Not every nested use breaks: cancellation-first histories remain consistent, and instances still blocked at the composite can generally regain progress without contradicting an existing continuation.
Transitive dependency risk: Both durabletask-azuremanaged (the durabletask.azuremanaged package) and azure-functions-durable v2 inherit this behavior. Their current core requirements are open-ended: durabletask>=1.10.1 and durabletask[opentelemetry]>=1.10.1, respectively. Users who pin only the provider can therefore receive the changed behavior on a fresh dependency resolution or dependency upgrade without changing their provider version. The Functions package being an RC does not isolate this risk, and a core major-version bump alone would not isolate it either given those existing constraints.
Recommended CHANGELOG updates: Document both the user-visible fix and the replay-breaking upgrade implications under ## Unreleased in all three affected changelogs:
CHANGELOG.mddurabletask-azuremanaged/CHANGELOG.mdazure-functions-durable/CHANGELOG.md
The notes should identify the affected nested-history scenario, explain that pinning the provider alone does not pin core behavior, and recommend pinning/locking core until affected running instances can be drained, isolated on the previous deployment, or deliberately recovered. Instances already stuck because of the bug may need recovery rather than simply waiting to drain. Include this warning for upgrades from earlier Functions v2 prereleases, ideally landing the correction and its guidance before 2.0 GA. Keep package/dependency version changes in the coordinated release PR.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Empty when_all composites can still fail to notify enclosing composites and remain incomplete.
Review effort: Lite
Findings: None
What changed in this PR
Fixes nested composite task completion by propagating completion notifications to parent when_all and when_any tasks.
Changes:
- Adds parent notification propagation.
- Adds nested success, failure, and race regression tests.
- Documents replay implications across affected packages.
| File | Description |
|---|---|
tests/durabletask/test_orchestration_executor.py |
Adds nested composite regression tests. |
durabletask/task.py |
Propagates composite completion to parent tasks. |
durabletask-azuremanaged/CHANGELOG.md |
Documents provider impact. |
CHANGELOG.md |
Documents the core behavior change and replay impact. |
azure-functions-durable/CHANGELOG.md |
Documents compatibility API impact and replay warning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bernd Verst (berndverst)
left a comment
There was a problem hiding this comment.
The completion-propagation fix is appropriately scoped. The intentional replay-compatibility change is acceptable with the upgrade guidance documented in #266 and the package changelogs. The remaining documentation refinement and executor-level replay coverage suggestions are non-blocking.
Summary
when_alltasks, plus nestedwhen_anynotification.when_any([cancel, when_all(tasks)])while keeping the standard DTS Fan-out visualization.Closes #266