Workflow: cross-app client operations - #1201
Conversation
Adds an optional app_id argument to every client-level workflow operation on DaprWorkflowClient and its async counterpart: schedule_new_workflow, get_workflow_state, wait_for_workflow_start, wait_for_workflow_completion, raise_workflow_event, terminate_workflow, pause_workflow, resume_workflow and purge_workflow. When set, the operation targets a workflow instance owned by another app in the same namespace, and the target app's WorkflowAccessPolicy decides whether it is permitted. When unset or equal to the local app, the behaviour is unchanged. The vendored durabletask client carries the value as a TaskRouter with targetAppID on each request, built by the new new_task_router helper. An older runtime ignores the field and applies the operation locally. Signed-off-by: joshvanl <me@joshvanl.dev>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1201 +/- ##
==========================================
+ Coverage 83.15% 83.70% +0.54%
==========================================
Files 123 123
Lines 10260 10266 +6
==========================================
+ Hits 8532 8593 +61
+ Misses 1728 1673 -55 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Proto provenance is inconsistent with generated output, and cross-app purge semantics are documented incorrectly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds cross-app routing to synchronous and asynchronous workflow client operations using TaskRouter.
Changes:
- Adds optional
app_idforwarding across workflow lifecycle operations. - Regenerates durabletask protocol bindings and improves regeneration provenance.
- Adds unit and integration coverage for cross-app routing.
File summaries
| File | Description |
|---|---|
tools/regen_durabletask_protos.sh |
Supports local proto sources and records provenance. |
tests/integration/test_workflow_cross_app.py |
Tests cross-app workflow operations end to end. |
tests/integration/conftest.py |
Refines app-channel readiness checks. |
tests/integration/apps/workflow_host.py |
Provides the remote workflow host. |
tests/ext/workflow/test_workflow_client.py |
Tests sync app_id forwarding. |
tests/ext/workflow/test_workflow_client_aio.py |
Tests async app_id forwarding. |
tests/ext/workflow/durabletask/test_orchestration_executor.py |
Updates action-router assertions. |
tests/ext/workflow/durabletask/test_client_routing.py |
Verifies routing on generated requests. |
dapr/ext/workflow/dapr_workflow_client.py |
Exposes cross-app sync operations. |
dapr/ext/workflow/aio/dapr_workflow_client.py |
Exposes cross-app async operations. |
dapr/ext/workflow/_durabletask/internal/PROTO_SOURCE_COMMIT_HASH |
Updates recorded proto source revision. |
dapr/ext/workflow/_durabletask/internal/orchestrator_service_pb2.pyi |
Adds typed operation-router fields. |
dapr/ext/workflow/_durabletask/internal/orchestrator_service_pb2.py |
Updates generated operation protocol descriptors. |
dapr/ext/workflow/_durabletask/internal/orchestrator_actions_pb2.pyi |
Updates generated action types. |
dapr/ext/workflow/_durabletask/internal/orchestrator_actions_pb2.py |
Updates generated action descriptors. |
dapr/ext/workflow/_durabletask/internal/orchestration_pb2.pyi |
Adds generated workflow metadata types. |
dapr/ext/workflow/_durabletask/internal/orchestration_pb2.py |
Updates orchestration descriptors. |
dapr/ext/workflow/_durabletask/internal/history_events_pb2.pyi |
Adds child-retry metadata typing. |
dapr/ext/workflow/_durabletask/internal/history_events_pb2.py |
Updates history event descriptors. |
dapr/ext/workflow/_durabletask/internal/helpers.py |
Keeps routing on enclosing workflow actions. |
dapr/ext/workflow/_durabletask/internal/backend_service_pb2.pyi |
Adds unique-instance request typing. |
dapr/ext/workflow/_durabletask/internal/backend_service_pb2.py |
Updates backend protocol descriptors. |
dapr/ext/workflow/_durabletask/client.py |
Builds and attaches sync task routers. |
dapr/ext/workflow/_durabletask/aio/client.py |
Builds and attaches async task routers. |
Review details
Files not reviewed (5)
- dapr/ext/workflow/_durabletask/internal/backend_service_pb2.py: Generated file
- dapr/ext/workflow/_durabletask/internal/history_events_pb2.py: Generated file
- dapr/ext/workflow/_durabletask/internal/orchestration_pb2.py: Generated file
- dapr/ext/workflow/_durabletask/internal/orchestrator_actions_pb2.py: Generated file
- dapr/ext/workflow/_durabletask/internal/orchestrator_service_pb2.py: Generated file
Suppressed comments (1)
tests/ext/workflow/durabletask/test_orchestration_executor.py:770
- This is the
without_app_idcase, so the docstring still describes the opposite scenario.
"""Tests that the workflow action carries correct router fields when app_id is specified"""
- Files reviewed: 19/24 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: joshvanl <me@joshvanl.dev>
sicoyle
left a comment
There was a problem hiding this comment.
this is great!! thanks 🚀 few comments so far pls
we already have examples/workflow/multi-app1.py and multi-app2.py showing the context-level cross-app calls. A client-level equivalent would go a long way for discoverability — right now there's no example anywhere showing you can pass app_id straight to schedule_new_workflow.
| schedule = pb.ScheduleTaskAction( | ||
| name=name, | ||
| input=get_string_value(encoded_input), | ||
| router=router, |
There was a problem hiding this comment.
Heads up — I think this one goes a bit beyond what the PR description covers. Dropping the router off the inner ScheduleTaskAction (and CreateChildWorkflowAction below) changes the wire format for cross-app activities and child workflows, which is separate from the client-level app_id feature this PR is about. Given our N-2 policy, I can't approve this unless every runtime in that window already reads WorkflowAction.router for these actions — if any of them still look at the inner field, cross-app calls break silently against a supported daprd, and the executor tests that would've caught it got relaxed in this PR rather than kept. Could you confirm the earliest runtime version that honors the outer router?
There was a problem hiding this comment.
the field is reserved in the pinned protos, so it cannot be set. No runtime in the N-2 window ever read it. Dapr 1.16.0 ships durabletask-go v0.9.0, 1.17.9 ships v0.11.5, 1.18.0 ships v0.12.1, 1.18.4 ships v0.12.5. In all four, applier.go builds the event with Router: action.Router, the outer field. Dapr itself never reads the inner field on any of those versions or master. The two dropped assertions referenced action.scheduleTask.router, which now raises AttributeError.
| @@ -322,8 +322,15 @@ class CreateInstanceRequest(_message.Message): | |||
| EXECUTIONID_FIELD_NUMBER: _builtins.int | |||
There was a problem hiding this comment.
can you pls confirm there are no manual edits to this file. seems commit 6732be7 includes a manual edit...
There was a problem hiding this comment.
commit 6732be7 is a clean regeneration, not a hand edit. Re-running the script against a clean checkout produces a byte-identical directory
| assert state.runtime_status.name == 'COMPLETED' | ||
|
|
||
| caller_client.purge_workflow(instance_id, app_id=HOST_APP_ID) | ||
| assert host_client.get_workflow_state(instance_id) is None |
There was a problem hiding this comment.
every other status check in file polls with a deadline. can you do the same here pls to avoid flake potential in future
| @@ -0,0 +1,170 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
every docstring here promises the target app's WorkflowAccessPolicy decides whether the operation is allowed, but nothing tests a rejected app_id. That's the security-relevant half of the feature — can we get coverage for the denial case pls?
There was a problem hiding this comment.
I removed the WAP tests bc they require mTLS which is difficult to setup in selfhosted
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Adds an optional app_id argument to every client-level workflow operation on DaprWorkflowClient and its async counterpart: schedule_new_workflow, get_workflow_state, wait_for_workflow_start, wait_for_workflow_completion, raise_workflow_event, terminate_workflow, pause_workflow, resume_workflow and purge_workflow. When set, the operation targets a workflow instance owned by another app in the same namespace, and the target app's WorkflowAccessPolicy decides whether it is permitted. When unset or equal to the local app, the behaviour is unchanged.
The vendored durabletask client carries the value as a TaskRouter with targetAppID on each request, built by the new new_task_router helper. An older runtime ignores the field and applies the operation locally.