Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
115 changes: 0 additions & 115 deletions src/Dapr.Workflow.Abstractions/HistoryEventKind.cs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The protos describe the type of history event returned. Is this not something meaningful to provide to the user?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lifecycle stage is captured by Started / Completed / Failed booleans on each result record — covers pending (Started && !Completed && !Failed) and matches go-sdk / python-sdk. HistoryEventKind removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's premature to think that this might be the final state of the lifecycle functionality. A wall of flags in which one is only ever true seems cognitively confusing. I can anticipate developers preemptively asking if there's ever a situation in which more than one flag could ever be true at once - as this reflects a specific named lifecycle, no, in which case, it feels like a simpler effect here is to simply reflect the lifecycle enums of the original implementation. They needn't all be provided as only a subset are ever exposed here, but the idea is to think about how the SDK will be consumed most easily by developers, not just what's one of many ways to expose the concept.

Further, should additional lifecycle events be added, a wall of flag evaluations must be extended to properly rule out new lifecycle flags. But if the new enums aren't necessary, they can simply be ignored.

All told, let's use enums instead of flags.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Pushing back here: Started/Completed/Failed mirrors the go-sdk ActivityResult struct and python-sdk ActivityResult dataclass, which is the whole point of this PR (cross-SDK parity Cassie flagged in #1801). Restoring a .NET-only enum would drop us back out of sync. The booleans are also not mutually exclusive over time — Started=true, Completed=false, Failed=false is the pending state — so an enum would actually lose information. Happy to switch if you want parity drift, but want to call that out explicitly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a PropagatedHistoryTaskStatus enum (Pending/Completed/Failed) with a computed Status property on both PropagatedHistoryActivityResult and PropagatedHistoryChildWorkflowResult, so callers can switch on a single value instead of reading the three flags — and the enum extends cleanly if the runtime adds lifecycle states later.

I kept the Started/Completed/Failed booleans alongside it: those are the field-level parity with the go-sdk ActivityResult/ChildWorkflowResult structs and the python-sdk dataclasses, and Status is just a projection of them (with Failed taking precedence over Completed). That keeps the .NET surface in sync with the other SDKs while giving developers the single enum to consume. 5987d9c

@WhitWaldo WhitWaldo May 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The design decisions behind the other SDKs isn't of much concern to me - those maintainers should do what is most appropriate per their own language conventions and express the functionality in ways that make it easier for their developers to consume their SDKs.

I weigh the perceived value of parity with other SDKs with the cost of expanding the public API and having to maintain two equivalent things going forward. I don't think a wall of flags is a developer-friendly way to communicate the state of something, so I appreciate you restoring the enum here.

Please modify the visibility of the flag fields to make them internal - should there ever be a need to make them publicly accessible, we can always introduce them later, but I'd rather this remain internal and not a part of the public API today.

This file was deleted.

121 changes: 85 additions & 36 deletions src/Dapr.Workflow.Abstractions/PropagatedHistory.cs
Comment thread
WhitWaldo marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -15,71 +15,120 @@ namespace Dapr.Workflow;

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;

/// <summary>
/// Contains the workflow history that was propagated from ancestor workflow instances.
/// Each entry corresponds to a single ancestor's history.
/// Workflow history propagated from one or more ancestor workflows to a child workflow or activity.
/// </summary>
/// <remarks>
/// A workflow receives propagated history when it is scheduled with a
/// <see cref="HistoryPropagationScope"/> other than <see cref="HistoryPropagationScope.None"/>.
/// Use <see cref="WorkflowContext.GetPropagatedHistory"/> to retrieve the propagated history
/// inside a workflow implementation.
/// A propagated history is an ordered list of <see cref="PropagatedHistoryEntry"/> values,
/// one per ancestor workflow. Order is execution order: index 0 is the oldest ancestor,
/// the last entry is the immediate parent.
/// <para>
/// Use the <c>Get*</c> / <c>TryGet*</c> methods to walk the list by app, instance, or workflow name.
/// Mirrors the <c>PropagatedHistory</c> type in the Go and Python SDKs.
/// </para>
/// </remarks>
public sealed class PropagatedHistory
{
private readonly IReadOnlyList<PropagatedHistoryEntry> _entries;
private readonly IReadOnlyList<PropagatedHistoryEntry> _workflows;
Comment thread
WhitWaldo marked this conversation as resolved.
Outdated

/// <summary>
/// Initializes a new instance of <see cref="PropagatedHistory"/> with the given entries.
/// Initializes a new <see cref="PropagatedHistory"/> from the given workflow entries.
/// </summary>
/// <param name="entries">The propagated history entries from ancestor workflows.</param>
public PropagatedHistory(IReadOnlyList<PropagatedHistoryEntry> entries)
/// <param name="workflows">
/// Workflow entries in execution order (ancestor first, immediate parent last).
/// </param>
public PropagatedHistory(IReadOnlyList<PropagatedHistoryEntry> workflows)
{
_entries = entries ?? throw new ArgumentNullException(nameof(entries));
_workflows = workflows ?? throw new ArgumentNullException(nameof(workflows));
}

/// <summary>
/// Gets the ordered list of propagated history entries.
/// The first entry corresponds to the immediate parent workflow; subsequent entries
/// correspond to progressively older ancestors when <see cref="HistoryPropagationScope.Lineage"/> is used.
/// Returns every workflow entry in the propagated history, in execution order
/// (ancestor first, immediate parent last).
/// </summary>
public IReadOnlyList<PropagatedHistoryEntry> Entries => _entries;
public IReadOnlyList<PropagatedHistoryEntry> GetWorkflows() => _workflows;

/// <summary>
/// Returns a new <see cref="PropagatedHistory"/> containing only entries from the specified App ID.
/// Returns an ordered, deduplicated list of app IDs in this propagated history.
/// </summary>
/// <param name="appId">The Dapr App ID to filter by.</param>
/// <returns>A filtered <see cref="PropagatedHistory"/> instance.</returns>
public PropagatedHistory FilterByAppId(string appId)
public IReadOnlyList<string> GetAppIds()
{
ArgumentException.ThrowIfNullOrWhiteSpace(appId);
return new PropagatedHistory(
_entries.Where(e => string.Equals(e.AppId, appId, StringComparison.OrdinalIgnoreCase)).ToList());
var seen = new HashSet<string>(StringComparer.Ordinal);
Comment thread
nelson-parente marked this conversation as resolved.
Outdated
var result = new List<string>(_workflows.Count);
foreach (var workflow in _workflows)
{
if (seen.Add(workflow.AppId))
{
result.Add(workflow.AppId);
}
}

return result;
}

/// <summary>
/// Returns a new <see cref="PropagatedHistory"/> containing only the entry with the specified instance ID.
/// Returns every workflow entry whose name matches, in execution order. Useful when the
/// list contains the same name more than once (e.g. recursion or ContinueAsNew).
/// </summary>
/// <param name="instanceId">The workflow instance ID to filter by.</param>
/// <returns>A filtered <see cref="PropagatedHistory"/> instance.</returns>
public PropagatedHistory FilterByInstanceId(string instanceId)
/// <param name="name">The workflow name to filter by.</param>
/// <returns>An empty list when no match is found.</returns>
public IReadOnlyList<PropagatedHistoryEntry> GetWorkflowsByName(string name)
{
ArgumentException.ThrowIfNullOrWhiteSpace(instanceId);
return new PropagatedHistory(
_entries.Where(e => string.Equals(e.InstanceId, instanceId, StringComparison.Ordinal)).ToList());
ArgumentException.ThrowIfNullOrWhiteSpace(name);
return _workflows
.Where(w => string.Equals(w.Name, name, StringComparison.Ordinal))
.ToList();
Comment thread
WhitWaldo marked this conversation as resolved.
}

/// <summary>
/// Tries to return the most recent workflow entry whose name matches.
/// </summary>
/// <param name="name">The workflow name to look up.</param>
/// <param name="result">When this method returns <see langword="true"/>, the last matching workflow entry; otherwise <see langword="null"/>.</param>
/// <returns><see langword="true"/> if a matching entry was found; otherwise <see langword="false"/>.</returns>
public bool TryGetLastWorkflowByName(string name, [NotNullWhen(true)] out PropagatedHistoryEntry? result)
{
ArgumentException.ThrowIfNullOrWhiteSpace(name);
for (var i = _workflows.Count - 1; i >= 0; i--)
{
if (string.Equals(_workflows[i].Name, name, StringComparison.Ordinal))
{
result = _workflows[i];
return true;
Comment thread
nelson-parente marked this conversation as resolved.
Outdated
}
}

result = null;
return false;
}

/// <summary>
/// Returns every workflow entry produced by the given app, in execution order.
/// </summary>
/// <param name="appId">The Dapr App ID to filter by.</param>
/// <returns>An empty list when no match is found.</returns>
public IReadOnlyList<PropagatedHistoryEntry> GetWorkflowsByAppId(string appId)
{
ArgumentException.ThrowIfNullOrWhiteSpace(appId);
return _workflows
.Where(w => string.Equals(w.AppId, appId, StringComparison.Ordinal))
.ToList();
Comment thread
nelson-parente marked this conversation as resolved.
Outdated
}

/// <summary>
/// Returns a new <see cref="PropagatedHistory"/> containing only entries for the specified workflow name.
/// Returns every workflow entry produced by the given instance, in execution order.
/// Usually a single entry, except when the same instance reappears via ContinueAsNew.
/// </summary>
/// <param name="workflowName">The workflow name to filter by.</param>
/// <returns>A filtered <see cref="PropagatedHistory"/> instance.</returns>
public PropagatedHistory FilterByWorkflowName(string workflowName)
/// <param name="instanceId">The workflow instance ID to filter by.</param>
/// <returns>An empty list when no match is found.</returns>
public IReadOnlyList<PropagatedHistoryEntry> GetWorkflowsByInstanceId(string instanceId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same idea as before - I don't know that this feature will always return only workflows. Today it might, but I'd like to leave our options open so we can avoid breaking changes. Please revert to "FilterByWorkflowName".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — GetWorkflowsByName()FilterByWorkflowName(), and for consistency GetWorkflowsByAppId()FilterByAppId() and GetWorkflowsByInstanceId()FilterByInstanceId(). TryGetLastWorkflowByName() stays (the TryGet pattern from the earlier round; "ByWorkflowName" lines up with FilterByWorkflowName). 5987d9c

{
ArgumentException.ThrowIfNullOrWhiteSpace(workflowName);
return new PropagatedHistory(
_entries.Where(e => string.Equals(e.WorkflowName, workflowName, StringComparison.Ordinal)).ToList());
ArgumentException.ThrowIfNullOrWhiteSpace(instanceId);
return _workflows
.Where(w => string.Equals(w.InstanceId, instanceId, StringComparison.Ordinal))
.ToList();
}
}
Comment thread
WhitWaldo marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// ------------------------------------------------------------------------
// Copyright 2026 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

namespace Dapr.Workflow;

/// <summary>
/// A reconstructed view of a single activity invocation surfaced through propagated workflow history.
/// </summary>
/// <param name="Name">The scheduled name of the activity.</param>
/// <param name="Started">Whether the activity was scheduled in the propagated history.</param>
/// <param name="Completed">Whether the activity completed successfully.</param>
/// <param name="Failed">Whether the activity failed.</param>
/// <param name="Input">The JSON-encoded input payload, or <c>null</c> when unset.</param>
/// <param name="Output">The JSON-encoded output payload, or <c>null</c> when the activity has not completed.</param>
/// <param name="FailureDetails">The failure details when <paramref name="Failed"/> is true, otherwise <c>null</c>.</param>
/// <remarks>
/// Mirrors the <c>ActivityResult</c> type in the Go and Python SDKs so cross-language
/// quickstarts and audit patterns line up.
/// </remarks>
public sealed record PropagatedHistoryActivityResult(
string Name,
bool Started,
bool Completed,
bool Failed,
string? Input,
string? Output,
WorkflowTaskFailureDetails? FailureDetails);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No interest in reflecting what type of event was recorded here? It's in the protos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Captured via Started / Completed / Failed booleans rather than a separate event-kind enum, matching go-sdk ActivityResult and python-sdk.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again, don't care about making other SDKs. I care about consistency and ease of use in the .NET SDK itself. Workflow status is reflected by enum. Let's reflect activity status by enum as well.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// ------------------------------------------------------------------------
// Copyright 2026 The Dapr Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ------------------------------------------------------------------------

namespace Dapr.Workflow;

/// <summary>
/// A reconstructed view of a single child workflow invocation surfaced through propagated workflow history.
/// </summary>
/// <param name="Name">The scheduled name of the child workflow.</param>
/// <param name="Started">Whether the child workflow was scheduled in the propagated history.</param>
/// <param name="Completed">Whether the child workflow completed successfully.</param>
/// <param name="Failed">Whether the child workflow failed.</param>
/// <param name="Output">The JSON-encoded output payload, or <c>null</c> when the child workflow has not completed.</param>
/// <param name="FailureDetails">The failure details when <paramref name="Failed"/> is true, otherwise <c>null</c>.</param>
/// <remarks>
/// Mirrors the <c>ChildWorkflowResult</c> type in the Go and Python SDKs.
/// </remarks>
public sealed record PropagatedHistoryChildWorkflowResult(
string Name,
bool Started,
bool Completed,
bool Failed,
string? Output,
WorkflowTaskFailureDetails? FailureDetails);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No interest in reflecting what type of event was recorded here? It's in the protos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as PropagatedHistoryActivityResultStarted / Completed / Failed cover the lifecycle without a separate enum, matching go-sdk / python-sdk.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Flags are more than fine to retain internally, but the point of the SDK Is to simplify development and lower the cognitive burden on developers.

It's a burden on developers to properly map which combination of flags represents what action, especially when we already produce a list of all possible workflow states as enums. It doesn't strike me as consistent to have activities reported as a wall of flags and workflows reported as an easy-to-understand enum.

Loading
Loading