Python: [BREAKING] Durable Task multi-workflow hosting and sub-workflows#6696
Conversation
moonbox3
added
documentation
There was a problem hiding this comment.
Pull request overview
Adds multi-workflow hosting and sub-workflow composition to the Python Durable Task hosting stack (standalone DurableAIAgentWorker, Azure Functions AgentFunctionApp, and DurableWorkflowClient) by introducing per-workflow durable naming (dafx-{workflow}), scoping inner durable primitives by workflow, and executing WorkflowExecutor nodes as child orchestrations with nested HITL request propagation/routing.
Changes:
- Introduces a shared durable naming/validation layer for stable per-workflow orchestration + scoped executor identities, and updates worker/client/hosts to use it.
- Executes
WorkflowExecutornodes as durable child orchestrations, including bubbling nested HITL requests to the top-level instance via qualified request ids and routing responses back down. - Updates samples, unit/integration tests, and adds an ADR documenting the decisions and tradeoffs.
Reviewed changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/samples/04-hosting/durabletask/README.md | Adds workflow-hosting sample catalog entries (including sub-workflows/HITL). |
| python/samples/04-hosting/durabletask/12_subworkflow_hitl/worker.py | New standalone worker sample: nested HITL pause inside a sub-workflow. |
| python/samples/04-hosting/durabletask/12_subworkflow_hitl/README.md | Documentation for the durabletask nested sub-workflow HITL sample. |
| python/samples/04-hosting/durabletask/12_subworkflow_hitl/client.py | New client sample driving nested HITL via qualified request ids. |
| python/samples/04-hosting/durabletask/11_subworkflow/worker.py | New standalone worker sample: composed workflow via child orchestration. |
| python/samples/04-hosting/durabletask/11_subworkflow/README.md | Documentation for the durabletask sub-workflow composition sample. |
| python/samples/04-hosting/durabletask/11_subworkflow/client.py | New client sample for the composed (outer+inner) workflow. |
| python/samples/04-hosting/durabletask/09_workflow_hitl/worker.py | Ensures workflow has a stable explicit name for durable hosting. |
| python/samples/04-hosting/durabletask/09_workflow_hitl/client.py | Updates client to target the named workflow orchestration. |
| python/samples/04-hosting/durabletask/08_workflow/worker.py | Ensures workflow has a stable explicit name for durable hosting. |
| python/samples/04-hosting/durabletask/08_workflow/client.py | Updates client to target the named workflow orchestration. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/requirements.txt | New Azure Functions sample deps (local editable installs). |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/README.md | New Azure Functions sample docs for nested sub-workflow HITL. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/local.settings.json.sample | New Azure Functions sample local settings template. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/host.json | New Azure Functions sample host configuration. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/function_app.py | New Azure Functions sample implementing nested sub-workflow HITL. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/demo.http | New REST-client demo for nested HITL routes and qualified ids. |
| python/samples/04-hosting/azure_functions/13_subworkflow_hitl/.gitignore | Ignores local settings/venv for the new sample. |
| python/samples/04-hosting/azure_functions/12_workflow_hitl/README.md | Updates docs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/12_workflow_hitl/function_app.py | Names the workflow so routes/orchestrator are per-workflow. |
| python/samples/04-hosting/azure_functions/12_workflow_hitl/demo.http | Updates demo URLs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/11_workflow_parallel/README.md | Updates docs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/11_workflow_parallel/function_app.py | Names the workflow so routes/orchestrator are per-workflow. |
| python/samples/04-hosting/azure_functions/11_workflow_parallel/demo.http | Updates demo URLs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/10_workflow_no_shared_state/README.md | Updates docs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/10_workflow_no_shared_state/function_app.py | Names the workflow so routes/orchestrator are per-workflow. |
| python/samples/04-hosting/azure_functions/10_workflow_no_shared_state/demo.http | Updates demo URLs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/09_workflow_shared_state/README.md | Updates docs to per-workflow route shape. |
| python/samples/04-hosting/azure_functions/09_workflow_shared_state/function_app.py | Names the workflow so routes/orchestrator are per-workflow. |
| python/samples/04-hosting/azure_functions/09_workflow_shared_state/demo.http | Updates demo URLs to per-workflow route shape. |
| python/packages/durabletask/tests/test_workflow_serialization.py | Adds tests for stripping forged sub-workflow envelope markers. |
| python/packages/durabletask/tests/test_workflow_registration.py | Adds sub-workflow classification + recursive hosted-workflow collection tests. |
| python/packages/durabletask/tests/test_workflow_naming.py | New tests for per-workflow naming/validation + qualified request id helpers. |
| python/packages/durabletask/tests/test_workflow_client.py | Updates client tests for per-workflow orchestration targeting + nested HITL routing. |
| python/packages/durabletask/tests/test_worker.py | Updates worker tests for scoped names, multi-workflow registration, and sub-workflow registration. |
| python/packages/durabletask/tests/test_subworkflow_orchestration.py | New tests for child-orchestration dispatch and sub-workflow input/result handling. |
| python/packages/durabletask/tests/integration_tests/test_12_dt_subworkflow_hitl.py | New integration test: nested HITL in durabletask standalone host. |
| python/packages/durabletask/tests/integration_tests/test_11_dt_subworkflow.py | New integration test: sub-workflow composition on durabletask standalone host. |
| python/packages/durabletask/tests/integration_tests/test_09_dt_workflow_hitl.py | Updates integration test to target named per-workflow orchestration. |
| python/packages/durabletask/tests/integration_tests/test_08_dt_workflow.py | Updates integration test to schedule dafx-{workflow} orchestration. |
| python/packages/durabletask/tests/integration_tests/conftest.py | Relaxes env gating for no-LLM samples. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/serialization.py | Adds reserved sub-workflow envelope key + boundary stripping helper. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/registration.py | Adds WorkflowExecutor planning and recursive hosted-workflow collection. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/orchestrator.py | Implements child-orchestration execution for sub-workflows + status surfacing for nested HITL. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/naming.py | New canonical durable naming + validation + qualified request id helpers. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/dt_context.py | Adds call_sub_orchestrator support to the durabletask context adapter. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/context.py | Extends context interface to support child orchestration calls. |
| python/packages/durabletask/agent_framework_durabletask/_workflows/client.py | Adds per-workflow targeting, ownership validation, nested HITL gather/routing, and input hardening. |
| python/packages/durabletask/agent_framework_durabletask/_worker.py | Enables multi-workflow hosting, scoped durable names, sub-workflow recursion, and validations. |
| python/packages/durabletask/agent_framework_durabletask/init.py | Exports new naming/validation/collection helpers. |
| python/packages/azurefunctions/tests/test_app.py | Updates Azure Functions host tests for multi-workflow, scoping, and nested HITL plumbing. |
| python/packages/azurefunctions/tests/integration_tests/test_13_workflow_subworkflow_hitl.py | New integration test for nested sub-workflow HITL via Functions routes. |
| python/packages/azurefunctions/tests/integration_tests/test_12_workflow_hitl.py | Updates integration test routes to per-workflow shape. |
| python/packages/azurefunctions/tests/integration_tests/test_11_workflow_parallel.py | Updates integration test routes to per-workflow shape. |
| python/packages/azurefunctions/tests/integration_tests/test_10_workflow_no_shared_state.py | Updates integration test routes to per-workflow shape. |
| python/packages/azurefunctions/tests/integration_tests/test_09_workflow_shared_state.py | Updates integration test routes to per-workflow shape. |
| python/packages/azurefunctions/tests/integration_tests/conftest.py | Relaxes env gating for no-LLM Functions sample. |
| python/packages/azurefunctions/agent_framework_azurefunctions/_workflow_af_context.py | Adds call_sub_orchestrator support to the Azure Functions context adapter. |
| docs/decisions/0030-durabletask-multiworkflow-and-subworkflows.md | ADR capturing the multi-workflow + sub-workflow durable hosting decisions. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 91%
✓ Correctness
The implementation is well-structured: trust-boundary stripping handles both dict and string inputs, closure captures for per-workflow routes are correct, recursive HITL resolution is bounded by request-id string length, and decorator ordering follows Azure Functions v2 conventions. No high-severity correctness issues found. One minor docstring inaccuracy noted. The naming, registration, sub-workflow HITL addressing, and trust boundary stripping logic are all correct. Test coverage is thorough — qualified request id round-trips, deeply nested routing, auto::N leaf ids, ownership validation, and envelope stripping are all well-tested. Two tests in test_workflow_naming.py are placed in the wrong test class (TestSubworkflowRequestIdQualification instead of TestWorkflowNameRoundTrip) but this is a minor organizational issue with no functional impact. No correctness bugs found. The code in the samples looks correct. There is one documentation bug: the parent durabletask README.md describes the qualified request id format as
{executor}:{requestId}(using::separator, two parts) when every other reference in the PR — tests, sample docstrings, the sub-sample README, and the PR description itself — consistently uses{executor}~{ordinal}~{requestId}(using~separator, three parts). The PR rationale explicitly states the separator is~rather than:to avoid colliding with the framework's ownauto::Nrequest ids.
✓ Test Coverage
Test coverage for this PR is comprehensive and well-structured. New naming helpers, registration logic, sub-workflow orchestration, and HITL request qualification all have dedicated unit tests. The Azure Functions app tests cover multi-workflow registration, duplicate/invalid name rejection, sub-workflow nesting, scoped orchestration ownership, and nested HITL gather/resolve. A new integration test (test_13_subworkflow_hitl) exercises the end-to-end nested HITL flow. Two minor gaps: (1) no unit test verifies the trust boundary integration (that
strip_subworkflow_markersis called on untrusted HTTP input before scheduling), and (2) theAzureFunctionsWorkflowContext.call_sub_orchestratoradapter method has no dedicated unit test. Neither gap is blocking given the function-level tests and integration coverage already in place. Test coverage for this PR is strong overall, with dedicated test files for naming, serialization, registration, client HITL, and sub-workflow orchestration. However, there are a few notable gaps: (1)await_workflow_outputandstream_workflowhave no tests for the new_is_owned_orchestrationownership validation path, even thoughget_runtime_status,get_pending_hitl_requests, andsend_hitl_responsedo; (2)workflow_scoped_executor_idandworkflow_executor_activity_namein naming.py have zero direct unit tests; (3) thesubworkflow_counterdeterministic instance-id derivation in the orchestrator has no dedicated test verifying counter persistence across supersteps. Test coverage for this PR is extensive and well-structured. New unit tests cover naming/validation, registration (including sub-workflows), orchestration dispatch, serialization trust boundaries, and the client-side sub-workflow HITL qualified-request-id scheme. Integration tests cover both the standalone durabletask worker and Azure Functions hosts. Two minor gaps: (1) theTestOwnershipValidationclass tests foreign-instance rejection forget_runtime_status,get_pending_hitl_requests, andsend_hitl_response, but omitsawait_workflow_outputwhich has the same ownership check (client.py:177-178) with no unit-level coverage of thatValueErrorpath; (2) two tests inTestSubworkflowRequestIdQualificationactually testworkflow_name_from_orchestratorrather than request-id qualification, which is a minor organizational issue. Test coverage is thorough: unit tests exist for naming/validation (test_workflow_naming.py), sub-workflow orchestration (test_subworkflow_orchestration.py), and integration tests for both the standalone DT worker (test_11_dt_subworkflow.py, test_12_dt_subworkflow_hitl.py) and Azure Functions (test_13_workflow_subworkflow_hitl.py) hosting paths. One documentation bug was found: the README uses the wrong separator character in the qualified request id format.
✓ Failure Modes
The diff is well-structured with thorough validation and trust-boundary handling. One concrete failure mode: in
configure_workflow, the top-levelself._workflowsdict is mutated before the registration loop that can raise ValueError on a cross-call sub-workflow name collision. This leaves the worker in an unrecoverable inconsistent state (the name is 'registered' but primitives are not, and retrying is blocked by the duplicate check). The recursive HITL resolution and naming logic are sound, and the sub-workflow envelope trust boundary is properly enforced on both hosts. The::separator collides with the framework's internalauto::Nnamespace (which is why~was chosen), and the two-part format omits the ordinal hop that keeps fan-out children independently addressable. A user copying this format to parse or build request ids would produce ids that fail to route. The sample code and sample-level README are correct.
✓ Design Approach
The Azure Functions multi-workflow registration currently leaves a cross-workflow boundary gap when two workflow names differ only by case. Registration accepts both names as distinct, but the status/respond ownership check later compares orchestration names case-insensitively, so one workflow’s routes can read or inject events into the other’s instances. The sub-workflow hosting design is mostly coherent, but one behavioral gap remains: child workflows do not propagate their event stream back to the parent durable workflow, so nested intermediate emissions disappear from
stream_workflow()even though the in-processWorkflowExecutorforwards them through the parent surface. I found one design-level inconsistency in the new workflow naming contract: it now explicitly allows mixed-case workflow names, but the runtime’s ownership checks treat orchestration names case-insensitively while registration stores them case-sensitively. That means two co-hosted workflows whose names differ only by case can still collide at the status/HITL boundary, undermining the PR’s isolation goal. I found one nonblocking design/documentation mismatch in the new durabletask sample catalog. The new12_subworkflow_hitlentry documents the nested HITL request-id shape as{executor}::{requestId}, but the shipped contract and tests in this PR use~-qualified ids such asreview_sub~0~{requestId}. That inconsistency would send readers toward an addressing scheme the client/host do not accept.
Suggestions
- Add an ownership-validation unit test for
await_workflow_output(foreign instance raisesValueError), matching the pattern already established inTestOwnershipValidationfor the other client methods. TheValueErrorpath at client.py:177-178 currently has zero unit coverage.
Automated review by ahmedmuhsin's agents
Foundation for hosting multiple workflows (and later sub-workflows) on one
durable task host. Adds a host-agnostic naming module that derives the stable
durable names a hosted workflow registers under.
- New `_workflows/naming.py`:
- `workflow_orchestrator_name(name)` -> `dafx-{name}` (orchestration name,
aligned byte-for-byte with .NET `WorkflowNamingHelper`).
- `workflow_name_from_orchestrator(name)` -> reverse, `None` when not prefixed.
- `validate_workflow_name(name)` -> rejects empty / malformed / auto-generated
`WorkflowBuilder-<uuid>` names (validate-and-reject rather than silently
sanitize, since the name becomes a durable identity and an HTTP route segment).
- `is_auto_generated_workflow_name(name)`, `DURABLE_NAME_PREFIX`.
- Export the helpers from the package public API.
- Mark `WORKFLOW_ORCHESTRATOR_NAME` deprecated in favor of per-workflow names
(kept functional; the single-workflow path still uses it until phase 1).
- 39 unit tests covering round-trips and validation.
Design: docs/design/durabletask-multiworkflow-and-subworkflows.md
…es (phase 1)
Enables hosting more than one MAF workflow on a single standalone Durable Task
worker, and aligns both hosts on workflow-scoped durable names so two co-hosted
workflows that reuse an executor id cannot collide.
Naming (shared, host-agnostic):
- orchestration: dafx-{workflowName} (matches .NET; the name DT tooling surfaces)
- non-agent activity / agent entity: dafx-{workflowName}-{executorId} (scoped)
- New naming helpers workflow_scoped_executor_id / workflow_executor_activity_name.
Standalone worker (agent-framework-durabletask):
- configure_workflow is now additive: stores workflows keyed by Workflow.name,
rejects duplicate / auto-generated (WorkflowBuilder-<uuid>) / invalid names,
registers one orchestrator per workflow plus its scoped activities/entities.
- The shared orchestrator dispatches scoped names derived from workflow.name.
- New registered_workflow_names property.
Client (DurableWorkflowClient):
- Optional default workflow_name on the client; start/run/stream accept a per-call
workflow_name and target dafx-{name}.
- Opt-in ownership validation on status/HITL methods: when a workflow name is
resolvable, an instance whose orchestration name does not match is treated as
not-found (status -> None, pending -> [], send_hitl_response / await -> raise),
mirroring the Azure Functions route-scoping check.
Azure Functions host (agent-framework-azurefunctions):
- Registration now uses the same scoped names so the shared orchestrator's
dispatch matches (single workflow per app for now; flat workflow/* routes kept).
- Workflow name is validated up front; workflow agents register under the scoped
entity id; _is_workflow_orchestration scopes to dafx-{workflow.name}.
Samples + tests:
- Durable Task and Azure Functions workflow samples now name their workflow.
- Unit tests cover multi-workflow registration, name validation, client targeting,
and ownership; integration tests target the named workflows.
WORKFLOW_ORCHESTRATOR_NAME remains exported (deprecated). This is a hard switch:
in-flight single-workflow instances created before upgrade (under the old
workflow_orchestrator name) will not resume.
Design: docs/design/durabletask-multiworkflow-and-subworkflows.md
…ow routes (phase 2)
Completes multi-workflow hosting on the Azure Functions host, building on the
shared scoped-naming foundation from the worker phase.
AgentFunctionApp:
- New `workflows=` parameter accepting a list (keyed by each `Workflow.name`) or a
name->Workflow mapping; the existing `workflow=` is a single-workflow alias.
Both may be combined. Duplicate names and mapping-key/name mismatches are rejected.
- Each workflow registers its own `dafx-{name}` orchestration, workflow-scoped
activities/entities, and per-workflow HTTP routes:
`workflow/{name}/run`, `workflow/{name}/status/{instanceId}`,
`workflow/{name}/respond/{instanceId}/{requestId}`. Routes are always
per-workflow (even for a single workflow) so callers don't change URLs as an app
grows from one workflow to many.
- Route ownership check is per-workflow (`_is_owned_orchestration(status, name)`):
a leaked instance id for another orchestration -- or another workflow -- is
treated as not-found, extending the route-scoping defense.
- `get_agent(context, name, workflow_name=...)` resolves a workflow agent under its
scoped id; bare `agents=` registration keeps the standalone surface. New
`workflows` introspection property; `.workflow` now returns the sole workflow
(or None when several are hosted).
- Removed the now-unused flat-URL helper `_build_status_url` (handlers inline
per-workflow URLs).
Samples + tests:
- Azure Functions workflow samples (09-12) name their workflow; integration tests
target the per-workflow routes.
- Unit tests cover multi-workflow registration, duplicate/mapping/auto-name
rejection, and per-workflow ownership.
Note: sample README / demo.http route docs are updated in the docs phase.
Design: docs/design/durabletask-multiworkflow-and-subworkflows.md
…ase 3)
Run WorkflowExecutor nodes as durable child orchestrations on both hosts.
- Protocol: add call_sub_orchestrator to WorkflowOrchestrationContext, implemented by the durabletask and Azure Functions adapters.
- Registration: planner classifies WorkflowExecutor as subworkflow_executors; collect_hosted_workflows walks nested workflows (parent first, deduped by name). Both hosts recursively register every nested workflow's orchestration/agents/activities once; only top-level workflows get HTTP routes. Names validated up front before any registration side effects.
- Orchestrator: dispatch WorkflowExecutor nodes via call_sub_orchestrator(dafx-{innerName}) with deterministic child instance ids ({instanceId}::{executorId}::{counter}), a trusted-input marker carrying nesting depth (bounded at 25), and outputs routed as messages (default) or parent outputs (allow_direct_output).
- Tests: registration/collect, orchestrator prepare/process/unwrap, recursive registration on both hosts. Sample: 11_subworkflow.
Surface a nested sub-workflow's human-in-the-loop request behind the top-level instance (B2 single addressing surface).
- Orchestrator records dispatched sub-workflow child instance ids in its custom status (subworkflows map) before suspending in task_all, so the read side can reach a child's pending request while the parent is paused.
- Read side (durabletask client get_pending_hitl_requests; AF status route) recurses into nested child statuses, qualifying each nested request id as {executorId}::{requestId} (accumulated for deeper nesting).
- Write side (durabletask client send_hitl_response; AF respond route) splits a qualified id on '::', resolves the owning child orchestration via the parent's subworkflows map, and raises the event on the leaf child with the bare request id. Unknown/inactive sub-workflow -> error/404.
- Shared SUBWORKFLOW_REQUEST_SEPARATOR ('::') in naming so both hosts and the client agree. respondUrl/respond always targets the top-level instance.
- Tests: TestSubworkflowHitl (durabletask client, 7), TestAgentFunctionAppSubworkflowHitl (AF, 7). Sample: 12_subworkflow_hitl (HITL pause inside an embedded sub-workflow).
…-workflows (phase 5)
- Add ADR-0030 capturing the multi-workflow and sub-workflow hosting decisions (naming, scoped inner names, per-workflow routes, child-orchestration sub-workflows, hard-switch migration, B2 sub-workflow HITL, scoped agent addressing) with considered alternatives; mark the design doc as implemented and link the ADR.
- Update Azure Functions workflow samples (09-12) README/demo.http to the per-workflow route shape (workflow/{name}/run|status|respond) introduced in phase 2.
- Extend the durabletask sample catalog with the workflow hosting patterns (08-12), including the new 11_subworkflow and 12_subworkflow_hitl samples.
…gration tests Post-review hardening of the multi-workflow / sub-workflow durable hosting: - Trust boundary: strip the reserved sub-workflow envelope key from untrusted client input at both host boundaries (DurableWorkflowClient.start_workflow and the AF start route) so a forged envelope cannot reach the trusted pickle path. - Nested HITL addressing: qualify nested pending requests by (executorId, ordinal) using a '~' separator (was '::', which collided with core's auto::N functional request ids); the parent status subworkflows map is now a per-executor list so multiple children dispatched in one superstep stay independently addressable. - Reject two different workflow instances that share a name (the same instance reused by sibling nodes is still deduped); validate executor ids (separator-free, length-bounded) when hosting durably. - Remove the arbitrary sub-workflow nesting depth cap: a WorkflowExecutor wraps a concrete Workflow so the nesting tree is finite at build time, and the durable instance-id length limit is the natural ceiling (matches .NET, which has none). Tests/samples: - New durabletask integration tests for sub-workflow composition (11) and nested sub-workflow HITL (12); new no-agent AF sub-workflow HITL sample (13) + test. - Exempt no-agent samples from the model-credential gate in both integration conftests so the nested-HITL plumbing is covered deterministically. - Update durabletask sample 12 docs to the new qualified-id format. Validated: 484 unit tests; durabletask integration 08/09/11/12 and AF 12/13 pass against the live emulators; pyright 0 errors; ruff clean.
- Unquote df.DurableOrchestrationClient annotations so pyupgrade passes.
- Narrow the split_subworkflow_request_id result before unpacking in a naming test so the strict type checkers pass.
- Correct the durabletask sample catalog to the {executor}~{ordinal}~{requestId} qualified id format.
- Reword the Azure Functions sub-workflow sample intro so it does not imply a difference from a same-numbered sample.
- Drop internal shorthand (B2, phase labels) from code comments.
The route ownership guard compares the durable orchestration name with casefold(), but registration kept raw names as distinct keys. Hosting 'Orders' and 'orders' therefore succeeded while either workflow's status/respond route could operate on the other's instances. Reject case-insensitive name collisions at registration (within a composition via collect_hosted_workflows, and across registration calls via the case-folded _registered_orchestrations map and the top-level guard in both hosts) so the case-folded ownership boundary stays real. Single names of any case remain valid; only collisions are rejected.
Drop the ADR and design exploration documents and the dangling docstring reference to them.
ahmedmuhsin
force-pushed
the
feature/python-durabletask-subworkflows-multiworkflow
branch
from
a612639 to
5df15ef
Compare
…op deprecated orchestrator-name symbols Extract a shared _parse_custom_status helper in DurableWorkflowClient to remove duplicated custom-status JSON parsing across three call sites. Drop the now-unused single-workflow compatibility shims WORKFLOW_ORCHESTRATOR_NAME and WorkflowRegistrationPlan.orchestrator_name, replaced by per-workflow workflow_orchestrator_name(name).
ahmedmuhsin
added
the
breaking change
github-actions
Bot
changed the title
… re-exports The constant was removed from agent-framework-durabletask, but the core azure lazy-loading namespace still re-exported it, breaking pyright in packages/core. Remove it from both the runtime _IMPORTS map and the .pyi stub.
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 86%
✓ Correctness
No actionable issues found in this dimension.
✓ Security Reliability
No actionable issues found in this dimension.
✓ Test Coverage
Test coverage for this PR is generally strong: sub-workflow registration, nested HITL gather/resolve, naming validation, and orchestration scoping all have thorough unit tests. The integration test for sample 13 covers the end-to-end nested HITL flow. Two coverage gaps stand out: (1) the new
workflow_nameparameter onget_agenthas no unit test exercising the successful workflow-scoped lookup path, and (2) there is no unit test verifying the trust boundary (strip_subworkflow_markers) is applied at the Azure Functions HTTP entry point (the function itself is tested in durabletask but not the call site where an attacker would submit a forged envelope). Test coverage is comprehensive across the new multi-workflow hosting, sub-workflow composition, and nested HITL features. Unit tests, integration tests, and security boundary tests are well-structured with meaningful assertions. Two new ownership-validation error paths in the client (await_workflow_output and stream_workflow) lack dedicated unit tests, unlike the analogous paths in get_runtime_status, get_pending_hitl_requests, and send_hitl_response which each have explicit rejection tests in TestOwnershipValidation. The test coverage for the new multi-workflow hosting, sub-workflow composition, and nested HITL features is thorough. Key paths are well-tested: naming round-trips, ownership validation, qualified request-id routing (including deeply nested, fan-out ordinals, and double-colon leaf ids), trust boundary stripping, and registration deduplication/collision detection. Two minor gaps exist: (1) no test exercises the out-of-bounds ordinal path insend_hitl_response(the implementation handles it at client.py:524 by returning None, but only the 'no subworkflows map at all' case is tested), and (2)await_workflow_outputownership rejection is not directly tested (though the shared_is_owned_orchestrationlogic is exercised by other methods). Additionally, twoworkflow_name_from_orchestratortests are misplaced insideTestSubworkflowRequestIdQualificationinstead ofTestWorkflowNameRoundTrip.
✓ Design Approach
I found one blocking design issue in the new multi-workflow registration flow: the durable-task worker records a top-level workflow as configured before it finishes checking whether that workflow’s nested sub-workflows can actually be registered. A nested-name collision therefore leaves the worker in a self-contradictory state where the workflow appears configured even though its durable primitives were never fully registered, and a retry is then rejected as a duplicate. The child-orchestration design mostly lines up with the new multi-workflow model, but it drops one existing
WorkflowExecutorbehavior: intermediate events from a sub-workflow no longer bubble up through the parent durable workflow's event stream. That leaves nested workflows observably different from the in-process model the PR says it is aligning with. I found one design issue: the new collision handling still allowsconfigure_workflow()to leave the worker partially configured when a nested sub-workflow name collides with a workflow that was registered earlier in a separate call. The current tests cover collisions within one composition, but they do not exercise the cross-registration path where the parent workflow is registered before the nested-name conflict is discovered. I did not find a design-level correctness problem in the new sub-workflow HITL sample itself.
Suggestions
- Add a test for send_hitl_response when the executor_id exists in the subworkflows map but the ordinal is out of bounds (e.g., 'sub
5req-9' when there's only 1 child). The implementation handles this at client.py:524 but the edge case is only covered by the 'no subworkflows map at all' test. - Consider adding a test_await_workflow_output_rejects_foreign_instance to TestOwnershipValidation: await_workflow_output has its own ownership check (client.py:177-178) but the rejection path is not exercised directly.
Automated review by ahmedmuhsin's agents
…orkflow events Make configure_workflow / AgentFunctionApp registration atomic: check every cross-call name collision before mutating any state, so a colliding nested sub-workflow no longer leaves a host partially configured (with the top-level name stuck in the registry). Applied to both the standalone worker and the Functions app. Bubble sub-workflow intermediate events: a workflow run as a child orchestration now returns a SUBWORKFLOW_RESULT_KEY envelope carrying its outputs plus event timeline, and the parent re-tags the child's intermediate events with the WorkflowExecutor node id and republishes them, matching the in-process WorkflowExecutor contract. Top-level runs still return a bare outputs list. Adds cross-registration atomicity tests on both hosts and unit tests for the result envelope and event bubbling. Resolves review threads on _worker.py, orchestrator.py, and test coverage.
The shared run_workflow_orchestrator now returns list | dict (the sub-workflow result envelope), so the azurefunctions _workflow.py wrapper that delegates to it must widen its Generator return annotation to match. Caught by the package-level pyright in CI (Package Checks), which type-checks the whole package, not just the files changed in the previous commit.
There was a problem hiding this comment.
Additional review pass 🔎 (updated)
Correction: an earlier version of this comment flagged a
WORKFLOW_ORCHESTRATOR_NAME"documented-but-removed deprecated alias" inconsistency. That was my mistake — it referenced a design doc / ADR that are not part of this PR (this PR has nodocs/changes). I've withdrawn that finding; apologies for the noise. The re-verified review is below.
Reviewed on top of the existing Copilot / automated reviews — all of their findings are already addressed, with matching tests:
- Case-insensitive name-collision guard —
registration.pydedupes onname.casefold(); covered bytest_rejects_case_insensitive_name_collision. - Atomic cross-registration on nested collision — validate-before-mutate; covered by
test_cross_registration_nested_collision_is_atomic(worker + app hosts). - Child intermediate-event bubbling —
test_intermediate_events_bubbled_retagged_with_node_id, plus bare-list / no-events envelope cases. - Ownership scoping —
_is_owned_orchestrationis applied onawait_workflow_output/stream_workflowand exercised byTestOwnershipValidation(foreign-instance rejection for runtime-status / pending-HITL / send-HITL), alongside the forged-envelope trust-boundary test.
ℹ️ CI note
The two red checks (markdown-link-check / merge-gatekeeper) trace to a flaky 404 on the root README.md stargazers badge (line 9) — a file this PR doesn't touch; merge-gatekeeper fails only as a cascade of that. A re-run should clear both.
🟢 Overall
High-quality, well-tested breaking change (multi-workflow hosting + sub-workflows). Naming/validation, atomic registration, the pickle trust boundary (forged sub-workflow envelope stripped before deserialization at every entry point), nested-HITL request-id addressing, and replay-stable child instance ids all look solid. LGTM.
Motivation & Context
The Durable Task hosting layer only ran a single workflow per worker or function app. It registered one fixed orchestrator name and named each executor's durable activity or entity by the bare executor id. Two workflows could not be co-hosted, and a workflow could not embed another workflow as a node. This change runs multiple workflows per host and composes workflows from nested sub-workflows. It brings the durable execution model in line with the in-process MAF workflow model and aligns the orchestration name with the .NET durable host.
Description & Review Guide
Multiple workflows per host.
DurableAIAgentWorker.configure_workflowandAgentFunctionApp(workflows=...)register any number of workflows. Each workflow gets its owndafx-{name}orchestration, and on the function app its ownworkflow/{name}/run,status, andrespondroutes. The orchestration name matches the .NET durable host.Per-workflow durable names. Inner activities and agent entities are scoped as
dafx-{workflow}-{executor}so two co-hosted workflows that reuse an executor id resolve to distinct primitives instead of shadowing each other. Agent conversation state stays isolated by the entity key, which is still the orchestration instance id.Sub-workflows as child orchestrations. A
WorkflowExecutornode runs its inner workflow as a durable child orchestration. Both hosts walk the composition and register every reachable workflow once, deduped by name. The shared engine and both host context adapters gained acall_sub_orchestratorprimitive.Nested human-in-the-loop behind one surface. A
request_infopause inside a sub-workflow is recorded on the child instance. The parent records its child instance ids in custom status, and the read side bubbles nested pending requests up to the top-level instance with a qualified request id of the form{executor}~{ordinal}~{requestId}, nested deeper for deeper levels. The caller always talks to the top-level run, and the host routes the response to the owning child. The~{ordinal}~hop keeps each child of a fan-out node independently addressable. The separator is~rather than:so it never collides with the framework's ownauto::Nfunctional-workflow request ids.Workflow and executor identity validation. Workflow names must be explicit and stable. Auto generated
WorkflowBuilder-{uuid}names are rejected because they change on every build and would break durable resume. Two different workflow instances that share a name are rejected, while the same instance reused across nodes is deduped. Executor ids are validated for durable hosting so they stay free of the reserved separator and within the durable name length limit.Trust boundary for the sub-orchestration envelope. The envelope carries the parent serialized child payload and is reconstructed with pickle on the trusted side. A real envelope is only ever built internally after the trust boundary, so both hosts strip the reserved envelope key from untrusted client input before scheduling a run. A forged envelope cannot reach the trusted deserialization path.
Samples. New durabletask samples cover sub-workflow composition and nested sub-workflow HITL, a new no-agent Azure Functions sample covers sub-workflow HITL, and the existing function app samples move to the per-workflow route shape.
This is a breaking change for the single-workflow hosting path. The Azure Functions HTTP routes move from
workflow/run,workflow/status/{instanceId}, andworkflow/respond/{instanceId}/{requestId}to the per-workflow shapeworkflow/{name}/run,workflow/{name}/status/{instanceId}, andworkflow/{name}/respond/{instanceId}/{requestId}, so an existing client calling the old routes gets a 404 and must include the workflow name. The previously exportedWORKFLOW_ORCHESTRATOR_NAMEconstant and the unusedWorkflowRegistrationPlan.orchestrator_namefield are removed; both hosts and the client now derive the orchestration name fromworkflow_orchestrator_name(name). The orchestration name itself changes from the fixedworkflow_orchestratortodafx-{name}, so an orchestration started under the old name will not resume after the upgrade; this lands before any rollout that relies on resume across the change. New public helpers are exported for hosts and clients, includingworkflow_orchestrator_name,validate_workflow_name,validate_executor_id,collect_hosted_workflows, and the request id qualification helpers.Sub-workflow nesting is not capped by a depth counter. A
WorkflowExecutorwraps a concreteWorkflow, so the nesting tree is finite at build time and the durable instance id length limit is the natural ceiling. This matches the .NET host, which imposes no limit.The nested HITL addressing and routing across both hosts, the trust boundary that strips the sub-workflow envelope key from untrusted input, and the per-workflow naming and validation that keeps two co-hosted workflows from colliding.
Related Issue
Related to #2614 — this is the Python side of Durable Agents workflow support. #2614 is a cross-language tracking issue (Python and .NET), so this PR references it without a closing keyword and the issue stays open for the remaining work. It extends the Durable Task workflow hosting already in the repository with multi-workflow hosting and sub-workflow composition.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.