From a61833792378cd139eeec0bc04c236e6dbe63087 Mon Sep 17 00:00:00 2001
From: Sipke Schoorstra <sipkeschoorstra@outlook.com>
Date: Tue, 11 Aug 2026 22:08:44 +0200
Subject: [PATCH] docs(core): give FaultSignal handlers a working completion
 path

Review caught that the contract advertised something that cannot work. It
offered handlers three ways to terminalize the faulted activity - cancel,
complete, or reschedule - but CompleteActivityAsync returns immediately unless
the activity is Running, and throughout the handler it is still Faulted, since
recovery runs only after the handler returns. Completing inline did nothing at
all, silently, leaving the child Running.

Measured, same container, handler completing the faulted child:

  inline complete                    -> child Running,   no output, Running/Suspended
  TransitionTo(Running), complete    -> child Completed, "after",   Finished/Finished

So a supported path exists; it just needed writing down. Document it on
FaultSignal, note that it is not licence to call RecoverFromFault (which also
rewrites the fault counts), and note that cancelling and rescheduling need no
equivalent step. Cover it with an integration test asserting that completing the
child with a substitute result fires the container's completion callback and
resumes its sequencing.

Refs #7911

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../Signals/FaultSignal.cs                    | 18 +++++++++++
 .../FaultSignals/FaultSignalTests.cs          | 32 +++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs
index ff6305340..50c839117 100644
--- a/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs
+++ b/src/modules/Elsa.Workflows.Core/Signals/FaultSignal.cs
@@ -65,6 +65,24 @@ namespace Elsa.Workflows.Signals;
 ///   </item>
 /// </list>
 /// <para>
+/// <b>Completing the faulted activity takes one extra step.</b> <c>CompleteActivityAsync</c> returns immediately unless
+/// the activity is <see cref="ActivityStatus.Running"/>, and throughout the handler it is still
+/// <see cref="ActivityStatus.Faulted"/>, because recovery runs only once the handler has returned. Completing it inline
+/// therefore does nothing at all, silently. A handler that wants to complete the activity — substituting a result for
+/// the work that failed, say — has to move it out of the faulted state first:
+/// </para>
+/// <code>
+/// context.StopPropagation();
+/// signal.FaultedContext.TransitionTo(ActivityStatus.Running);
+/// await signal.FaultedContext.CompleteActivityAsync(substituteResult);
+/// </code>
+/// <para>
+/// This is not licence to call <c>RecoverFromFault()</c>, which also rewrites the fault counts and remains the
+/// middleware's job alone. Completing this way fires the enclosing container's completion callback, so the container's
+/// normal sequencing resumes. Cancelling needs no equivalent step, because <c>CancelActivityAsync</c> already accepts a
+/// faulted activity, and rescheduling needs none either.
+/// </para>
+/// <para>
 /// That last rule is not stylistic. <c>RecoverFromFault()</c> is asymmetric: it <i>sets</i> the faulting context's
 /// <see cref="ActivityExecutionContext.AggregateFaultCount"/> to zero, which is idempotent, but <i>decrements</i> the
 /// count on every ancestor, which is not. A second call is therefore harmless for the faulting context and harmful for
diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs
index 6552a7dff..962e72a62 100644
--- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs
+++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FaultSignals/FaultSignalTests.cs
@@ -145,6 +145,38 @@ public class FaultSignalTests(ITestOutputHelper testOutputHelper)
         Assert.All(ancestors, x => Assert.Equal(-1, x.AggregateFaultCount));
     }
 
+    [Fact(DisplayName = "A handler can complete the faulted child with a substitute result")]
+    public async Task HandlerThatCompletesChildWithSubstituteResult_ResumesTheContainer()
+    {
+        // Arrange
+        var container = new FaultHandlingContainer(async (signal, context) =>
+        {
+            context.StopPropagation();
+
+            // CompleteActivityAsync no-ops on a non-Running activity, and the middleware recovers only once this
+            // handler has returned, so the faulted child has to be moved out of Faulted first. This is not the same as
+            // RecoverFromFault, which would also rewrite the fault counts.
+            signal.FaultedContext.TransitionTo(ActivityStatus.Running);
+            await signal.FaultedContext.CompleteActivityAsync("substitute");
+        })
+        {
+            Activities =
+            {
+                _faultingActivity,
+                new WriteLine("after")
+            }
+        };
+
+        // Act
+        var result = await RunAsync(container);
+
+        // Assert: completing the child fires the container's completion callback, so sequencing resumes.
+        Assert.Equal(ActivityStatus.Completed, result.GetActivityStatus(_faultingActivity));
+        Assert.Equal(new[] { "after" }, _fixture.CapturingTextWriter.Lines);
+        Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus);
+        AssertFaultCounts(result, expected: 0);
+    }
+
     [Fact(DisplayName = "A handler that cancels the faulted child leaves it Canceled, not Running")]
     public async Task HandlerThatCancelsChild_LeavesItCanceled()
     {