Clarify semantic of cancel wf commands (#849)

Author: mjameswh

core/src/worker/workflow/machines/workflow_machines.rs

@@ -58,9 +58,7 @@ use temporal_sdk_core_protos::{
         workflow_activation::{
             workflow_activation_job, NotifyHasPatch, UpdateRandomSeed, WorkflowActivation,
         },
-        workflow_commands::{
-            request_cancel_external_workflow_execution as cancel_we, ContinueAsNewWorkflowExecution,
-        },
+        workflow_commands::ContinueAsNewWorkflowExecution,
     },
     temporal::api::{
         command::v1::{command::Attributes as ProtoCmdAttrs, Command as ProtoCommand},
@@ -1397,28 +1395,17 @@ impl WorkflowMachines {
                     CommandID::ChildWorkflowStart(attrs.child_workflow_seq),
                 )?,
                 WFCommand::RequestCancelExternalWorkflow(attrs) => {
-                    let (we, only_child) = match attrs.target {
-                        None => {
-                            return Err(WFMachinesError::Fatal(
-                                "Cancel external workflow command had empty target field"
-                                    .to_string(),
-                            ))
-                        }
-                        Some(cancel_we::Target::ChildWorkflowId(wfid)) => (
-                            NamespacedWorkflowExecution {
-                                namespace: self.worker_config.namespace.clone(),
-                                workflow_id: wfid,
-                                run_id: "".to_string(),
-                            },
-                            true,
-                        ),
-                        Some(cancel_we::Target::WorkflowExecution(we)) => (we, false),
-                    };
+                    let we = attrs.workflow_execution.ok_or_else(|| {
+                        WFMachinesError::Fatal(
+                            "Cancel external workflow command had no workflow_execution field"
+                                .to_string(),
+                        )
+                    })?;
                     self.add_cmd_to_wf_task(
                         new_external_cancel(
                             attrs.seq,
                             we,
-                            only_child,
+                            false,
                             format!("Cancel requested by workflow with run id {}", self.run_id),
                         ),
                         CommandID::CancelExternal(attrs.seq).into(),

sdk-core-protos/protos/local/temporal/sdk/core/workflow_commands/workflow_commands.proto

@@ -269,17 +269,13 @@ message CancelChildWorkflowExecution {
     uint32 child_workflow_seq = 1;
 }
 
-// Request cancellation of an external workflow execution (which may be a started child)
+// Request cancellation of an external workflow execution. For cancellation of a child workflow,
+// prefer `CancelChildWorkflowExecution` instead, as it guards against cancel-before-start issues.
 message RequestCancelExternalWorkflowExecution {
     // Lang's incremental sequence number, used as the operation identifier
     uint32 seq = 1;
-    // What workflow is being targeted
-    oneof target {
-        // A specific workflow instance
-        common.NamespacedWorkflowExecution workflow_execution = 2;
-        // The desired target must be a child of the issuing workflow, and this is its workflow id
-        string child_workflow_id = 3;
-    }
+    // The workflow instance being targeted
+    common.NamespacedWorkflowExecution workflow_execution = 2;
 }
 
 // Send a signal to an external or child workflow

sdk/src/lib.rs

@@ -716,8 +716,6 @@ pub enum CancellableID {
         seqnum: u32,
         /// Identifying information about the workflow to be cancelled
         execution: NamespacedWorkflowExecution,
-        /// Set to true if this workflow is a child of the issuing workflow
-        only_child: bool,
     },
 }
 

sdk/src/workflow_context.rs

@@ -34,7 +34,6 @@ use temporal_sdk_core_protos::{
         common::NamespacedWorkflowExecution,
         workflow_activation::resolve_child_workflow_execution_start::Status as ChildWorkflowStartStatus,
         workflow_commands::{
-            request_cancel_external_workflow_execution as cancel_we,
             signal_external_workflow_execution as sig_we, workflow_command,
             CancelChildWorkflowExecution, ModifyWorkflowProperties,
             RequestCancelExternalWorkflowExecution, SetPatchMarker,
@@ -311,14 +310,13 @@ impl WfContext {
         &self,
         target: NamespacedWorkflowExecution,
     ) -> impl Future<Output = CancelExternalWfResult> {
-        let target = cancel_we::Target::WorkflowExecution(target);
         let seq = self.seq_nums.write().next_cancel_external_wf_seq();
         let (cmd, unblocker) = WFCommandFut::new();
         self.send(
             CommandCreateRequest {
                 cmd: RequestCancelExternalWorkflowExecution {
                     seq,
-                    target: Some(target),
+                    workflow_execution: Some(target),
                 }
                 .into(),
                 unblocker,
@@ -700,7 +698,6 @@ impl ChildWorkflow {
                     workflow_id: self.opts.workflow_id.clone(),
                     ..Default::default()
                 },
-                only_child: true,
             });
         cx.send(
             CommandSubscribeChildWorkflowCompletion {

sdk/src/workflow_future.rs

@@ -22,9 +22,8 @@ use temporal_sdk_core_protos::{
             WorkflowActivationJob,
         },
         workflow_commands::{
-            request_cancel_external_workflow_execution as cancel_we, update_response,
-            workflow_command, CancelChildWorkflowExecution, CancelSignalWorkflow, CancelTimer,
-            CancelWorkflowExecution, CompleteWorkflowExecution, FailWorkflowExecution,
+            update_response, workflow_command, CancelChildWorkflowExecution, CancelSignalWorkflow,
+            CancelTimer, CancelWorkflowExecution, CompleteWorkflowExecution, FailWorkflowExecution,
             RequestCancelActivity, RequestCancelExternalWorkflowExecution,
             RequestCancelLocalActivity, ScheduleActivity, ScheduleLocalActivity,
             StartChildWorkflowExecution, StartTimer, UpdateResponse,
@@ -514,22 +513,12 @@ impl WorkflowFuture {
                                 CancelSignalWorkflow { seq },
                             ));
                         }
-                        CancellableID::ExternalWorkflow {
-                            seqnum,
-                            execution,
-                            only_child,
-                        } => {
+                        CancellableID::ExternalWorkflow { seqnum, execution } => {
                             activation_cmds.push(
                                 workflow_command::Variant::RequestCancelExternalWorkflowExecution(
                                     RequestCancelExternalWorkflowExecution {
                                         seq: seqnum,
-                                        target: Some(if only_child {
-                                            cancel_we::Target::ChildWorkflowId(
-                                                execution.workflow_id,
-                                            )
-                                        } else {
-                                            cancel_we::Target::WorkflowExecution(execution)
-                                        }),
+                                        workflow_execution: Some(execution),
                                     },
                                 ),
                             );