Skip to content

feat(sdk): 💥 add typed child workflow handle #1182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chris-olszewski merged 13 commits into from
Mar 26, 2026
Merged

feat(sdk): 💥 add typed child workflow handle #1182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b9d6d93
refactor: move UntypedWorkflow from client to common crate
chris-olszewski Mar 25, 2026
72abc25
feat(sdk): add ChildWorkflowExecutionError type
chris-olszewski Mar 25, 2026
289d003
feat(sdk): typed child workflow API
chris-olszewski Mar 25, 2026
abbc3d4
update sdk readme
chris-olszewski Mar 25, 2026
551657d
remove duped test
chris-olszewski Mar 25, 2026
003b403
fix: avoid panic from child workflow payload converter failure
chris-olszewski Mar 26, 2026
7704d7d
fix: avoid nested result for child workflow signal
chris-olszewski Mar 26, 2026
1ab1be5
test: regression test for child cancel seq
chris-olszewski Mar 26, 2026
d3c65f6
fix: preexisting bugs claude found
chris-olszewski Mar 26, 2026
e822903
test: failing child workflow tests
chris-olszewski Mar 26, 2026
92cfdf2
fix: handle missing child wfl result, issue correct cancel cmd
chris-olszewski Mar 26, 2026
7f19656
pr feedback
chris-olszewski Mar 26, 2026
b9b2fa1
remove faulty comment
chris-olszewski Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 1 addition & 22 deletions crates/client/src/workflow_handle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
grpc::WorkflowService,
};
use std::{fmt::Debug, marker::PhantomData};
pub use temporalio_common::UntypedWorkflow;
use temporalio_common::{
HasWorkflowDefinition, QueryDefinition, SignalDefinition, UpdateDefinition, WorkflowDefinition,
data_converters::{RawValue, SerializationContextData},
Expand Down Expand Up @@ -148,28 +149,6 @@ impl WorkflowExecutionInfo {
/// and output.
pub type UntypedWorkflowHandle<CT> = WorkflowHandle<CT, UntypedWorkflow>;

/// Marker type for untyped workflow handles. Stores the workflow type name.
pub struct UntypedWorkflow {
name: String,
}
impl UntypedWorkflow {
/// Create a new `UntypedWorkflow` with the given workflow type name.
pub fn new(name: impl Into<String>) -> Self {
Self { name: name.into() }
}
}
impl WorkflowDefinition for UntypedWorkflow {
type Input = RawValue;
type Output = RawValue;
fn name(&self) -> &str {
&self.name
}
}

impl HasWorkflowDefinition for UntypedWorkflow {
type Run = Self;
}

/// Marker type for sending untyped signals. Stores the signal name for runtime lookup.
///
/// Use with `handle.signal(UntypedSignal::new("signal_name"), raw_payload)`.
Expand Down
3 changes: 2 additions & 1 deletion crates/common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ mod workflow_definition;
pub use activity_definition::ActivityDefinition;
pub use priority::Priority;
pub use workflow_definition::{
HasWorkflowDefinition, QueryDefinition, SignalDefinition, UpdateDefinition, WorkflowDefinition,
HasWorkflowDefinition, QueryDefinition, SignalDefinition, UntypedWorkflow, UpdateDefinition,
WorkflowDefinition,
};

macro_rules! dbg_panic {
Expand Down
25 changes: 24 additions & 1 deletion crates/common/src/workflow_definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::data_converters::{TemporalDeserializable, TemporalSerializable};
use crate::data_converters::{RawValue, TemporalDeserializable, TemporalSerializable};

/// Implement on a marker struct to define a workflow.
///
Expand All @@ -23,6 +23,29 @@ pub trait HasWorkflowDefinition: WorkflowDefinition {
type Run: WorkflowDefinition;
}

/// Marker type for untyped workflow handles. Stores the workflow type name. Uses [`RawValue`]
/// for both input and output.
pub struct UntypedWorkflow {
name: String,
}
impl UntypedWorkflow {
/// Create a new `UntypedWorkflow` with the given workflow type name.
pub fn new(name: impl Into<String>) -> Self {
Self { name: name.into() }
}
}
impl WorkflowDefinition for UntypedWorkflow {
type Input = RawValue;
type Output = RawValue;
fn name(&self) -> &str {
&self.name
}
}
Comment on lines +37 to +43

@claude claude Bot Mar 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Both UntypedWorkflow::Input = RawValue serialization and UntypedWorkflow::Output = RawValue deserialization always return WrongEncoding when the worker is configured with PayloadConverter::serde_json() directly (not the default composite converter). Fix: add as_serde and from_serde overrides to RawValue's TemporalSerializable/TemporalDeserializable impls in data_converters.rs.

Extended reasoning...

What the bug is and how it manifests

RawValue manually implements TemporalSerializable (overriding to_payload/to_payloads) and TemporalDeserializable (overriding from_payload/from_payloads), but does NOT override as_serde() or from_serde(). Since RawValue derives only Clone, Debug, Default — not serde::Serialize or serde::Deserialize — the blanket impls for those traits do not apply. Both trait defaults unconditionally return Err(PayloadConversionError::WrongEncoding). As a result, any worker using PayloadConverter::serde_json() directly will fail every UntypedWorkflow child workflow interaction.

The specific code paths that trigger it

For input serialization (as_serde side): BaseWorkflowContext::child_workflow calls payload_converter.to_payloads::<RawValue>(&ctx, &input). Under PayloadConverter::Serde, GenericPayloadConverter::to_payloads::<RawValue> takes the non-() branch (data_converters.rs ~465–472) and calls val.as_serde()?. Because RawValue does not override as_serde(), this falls back to the trait default and immediately returns Err(WrongEncoding), causing ChildWorkflowStartFut::eager(...) to be returned before any command is sent to Core — the child is never started.

For output deserialization (from_serde side): ChildWorkflowFut::poll (new code introduced by this PR) calls payload_converter.from_payloads::<Output>(&ctx, payloads) where Output = RawValue. Under PayloadConverter::Serde, GenericPayloadConverter::from_payloads::<RawValue> requires payloads.len() == 1 then calls T::from_serde(pc.as_ref(), context, payload) (data_converters.rs:503–507). RawValue::from_serde falls back to the default returning Err(WrongEncoding), so started.result().await always yields Err(ChildWorkflowExecutionError::Serialization(WrongEncoding)) even for successfully-completed child workflows.

Why existing code does not prevent it

The default PayloadConverter is Composite([UseWrappers, serde_json]). The UseWrappers arm calls T::to_payloads(val, context) and T::from_payloads(context, payloads) directly — bypassing as_serde/from_serde entirely — and these are correctly overridden on RawValue. So the default converter works fine, and all existing integration tests (including the new untyped_child_workflow_happy_path) pass because they all use the default converter.

Why this is introduced by the PR (addressing the pre-existing refutation)

One verifier argued that as_serde was pre-existing since UntypedWorkflow::Input = RawValue existed before in crates/client/src/workflow_handle.rs. However, the critical distinction is that the old ChildWorkflowOptions accepted Vec<Payload> for its input field, passing them directly to the proto command without going through any type-based serialization. This PR changes the API to route child workflow input through payload_converter.to_payloads::<WD::Input>(), which is the new code path that first exposes the as_serde gap for RawValue in workflow execution. The from_serde gap for the result path (ChildWorkflowFut) is unambiguously new code introduced entirely by this PR.

Impact and fix

Any user who constructs DataConverter::new(PayloadConverter::serde_json(), ...) — a documented public API — and uses UntypedWorkflow as a child workflow will get Serialization(WrongEncoding) on every start or result await. Fix: in data_converters.rs, add fn as_serde(&self) -> Result<...> { ... } to RawValue's TemporalSerializable impl (returning an appropriate serde representation of its payload bytes, or redirecting to to_payloads) and add fn from_serde(...) -> Result<Self, ...> { Ok(RawValue { payloads: vec![payload] }) } to its TemporalDeserializable impl.

Step-by-step proof

  1. Worker is configured with DataConverter::new(PayloadConverter::serde_json(), ...).
  2. Parent calls ctx.child_workflow(UntypedWorkflow::new("my-wf"), RawValue::new(vec![]), opts).await.
  3. BaseWorkflowContext::child_workflow calls payload_converter.to_payloads::<RawValue>(&ctx, &input).
  4. PayloadConverter::Serde arm: RawValue is not (), so val.as_serde()? is called.
  5. RawValue::as_serde() returns Err(WrongEncoding) (trait default).
  6. child_workflow() returns ChildWorkflowStartFut::Errored(Serialization(WrongEncoding)).
  7. .await on the future immediately yields Err(ChildWorkflowExecutionError::Serialization(WrongEncoding)) — child never starts. For the deserialization path: even if the child completes successfully, step 3 instead calls from_payloads::<RawValue> which calls RawValue::from_serde(...) → Err(WrongEncoding) → Err(ChildWorkflowExecutionError::Serialization(WrongEncoding)).


impl HasWorkflowDefinition for UntypedWorkflow {
type Run = Self;
}

/// Implement on a marker struct to define a query.
///
/// Typically, you will want to use the `#[query]` attribute inside a `#[workflow_methods]` macro
Expand Down
Loading
Loading