[aidevelopers2]

Summary

• add a Cancelling workflow sub-status for accepted cancellation requests
• persist non-finished workflow instances as Running / Cancelling before dispatching background cancellation work
• keep the existing cancellation pipeline responsible for the final transition to Cancelled
• update the API client enum and cover the immediate state persistence/dispatch behavior with unit tests

Fixes #5187

Tests

• .nuke/temp/dotnet-unix/dotnet test test/unit/Elsa.Workflows.Runtime.UnitTests/Elsa.Workflows.Runtime.UnitTests.csproj --filter WorkflowCancellationServiceTests

[greptile-apps]

PR author is not in the allowed authors list.

[Copilot]

Pull request overview

Adds an intermediate Cancelling workflow sub-status so cancellation requests are reflected immediately (before background cancellation completes), improving UI/API consistency during bulk cancellation operations.

Changes:

• Introduce WorkflowSubStatus.Cancelling in core and API client enums.
• Persist non-finished workflow instances as Running/Cancelling (including WorkflowState) before dispatching cancellation work.
• Add unit tests covering the save-before-dispatch behavior and the “instance not found” path.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
test/unit/Elsa.Workflows.Runtime.UnitTests/Services/WorkflowCancellationServiceTests.cs	Verifies instances are persisted as Cancelling before dispatch and that missing instances do not dispatch.
src/modules/Elsa.Workflows.Runtime/Services/WorkflowCancellationService.cs	Marks cancellable instances as Running/Cancelling and saves them before dispatching cancellation requests.
src/modules/Elsa.Workflows.Core/Enums/WorkflowSubStatus.cs	Adds the Cancelling sub-status to the core enum.
src/modules/Elsa.Workflows.Core/Contexts/WorkflowExecutionContext.cs	Maps Cancelling to main Running status for execution-context status derivation.
src/clients/Elsa.Api.Client/Resources/WorkflowInstances/Enums/WorkflowSubStatus.cs	Adds Cancelling to the API client enum to keep it in sync with server responses.

[elsa-workflows-review-bot]

Elsa 3 Code Review: REQUEST_CHANGES + HIGH @ 3608864

Code Review, Round 1/6

Thank you for taking on #5187. Showing an intermediate Cancelling state is a good goal, and appending the enum value at the end keeps the existing numeric values stable. I can't approve this yet, for three reasons. The branch can't merge into current main. The way the state is persisted can overwrite a running workflow. And an instance can get stuck in Cancelling with nothing to recover it.

Blocking

1. Rebase onto current main (it is conflicted). The branch is 1 commit ahead and about 4,300 behind, based on main from 2026-05-22, and GitHub reports it as dirty. Since then the repository moved under core/:

   • core/src/modules/Elsa.Workflows.Runtime/Services/WorkflowCancellationService.cs
   • core/src/modules/Elsa.Workflows.Core/Enums/WorkflowSubStatus.cs and core/src/modules/Elsa.Workflows.Core/Contexts/WorkflowExecutionContext.cs
   • core/src/clients/Elsa.Api.Client/Resources/WorkflowInstances/Enums/WorkflowSubStatus.cs
   • the tests under core/test/unit/Elsa.Workflows.Runtime.UnitTests/

   The service's logic hasn't changed on main, so the port should be mechanical.

2. Saving the whole instance snapshot can overwrite a running workflow. MarkWorkflowInstancesAsCancellingAsync calls IWorkflowInstanceStore.SaveManyAsync with the full WorkflowInstance, including the WorkflowState read a moment earlier. It does this for every non-finished instance, including ones in Executing.

   • Lost update. If an execution cycle owns the instance at that moment, this write replaces the newer state that the cycle has committed, or is about to commit, with the stale snapshot. That can lose activity progress and bookmarks.
   • It skips the guard main uses. The cancel path on main goes through LocalWorkflowClient.CancelAsync, which first calls DemandUnownedAsync. That guard exists to stop writes to an owned instance, and this write doesn't go through it.
   • What to change. Please don't persist the snapshot. Update only SubStatus and UpdatedAt, and only for instances that are not currently owned or executing. Instances that are executing can keep their real sub-status until the cancel runs.

3. An instance can get stuck in Cancelling. The instance is saved as Cancelling before dispatch, but nothing undoes that if the cancellation never completes:

   • dispatcher.DispatchAsync throws: the instances are already saved as Cancelling, and nothing reverts them.
   • The background CancelWorkflowsCommandHandler fails: for example, DemandUnownedAsync rejects an owned instance, or the canceler throws. The state stays Cancelling.
   • Recovery doesn't cover it. InterruptedRecoveryScanner only scans Interrupted, and crash recovery looks at Executing with a stale liveness timestamp, so a Cancelling instance is never picked up again. A formerly Suspended instance would also drop out of "suspended" filters and views for good.

   Please add one of these: restore the previous sub-status when dispatch or the cancel fails, or give the cancel path a recovery or retry for Cancelling. Either way, add a test for it.

The rest checks out

• Final transition is intact. LocalWorkflowClient.CancelAsync returns early only when Status != Running. Because Cancelling maps to Running, both in the PR's GetMainStatus change and in the persisted instance, the canceler still runs and moves the instance to Cancelled. ValidateStatusTransition allows leaving Cancelling, because its main status isn't Finished.
• Enum is in sync within this repository. The server enum and Elsa.Api.Client both get Cancelling appended last. elsa-core has no other copy of this enum, and there is no studio code under core to update. Elsa Studio lives in a separate repository and will need to render Cancelling; the maintainers can coordinate that. Note that an older client that deserializes the enum by name will reject the new value, so this belongs in a minor release, not a patch.
• Other code that switches on the sub-status. BulkDispatchWorkflows and DrainOrchestrator fall through to a default for values they don't know, so they are safe. GetMainStatus is the only switch that throws on an unknown value, and the PR updates it.
• Tests. The save-before-dispatch ordering test and the not-found test are good. After the rebase, please add tests for items 2 and 3: an executing or owned instance is not overwritten, and a failed dispatch does not leave Cancelling behind.

Housekeeping

• CLA. The license/cla check is still pending. Please reply to the CLA bot with @dotnet-policy-service agree.
• CI. The only CI on this head ran in May against the old base, so it says nothing about current main. Pushing the rebase will start fresh runs.
• Maintainer edits. "Allow edits by maintainers" is on. If you can't rebase, the maintainers may take this over and finish it on top of your commit, keeping your authorship.

There are no advisory bot threads to answer. Copilot left an overview with no inline comments, and Greptile didn't run for this author.

— Code Review (Elsa 3), via Elsa Review

[JohnyKid123]

Hey @aidevelopers2! Are you still working on this? If not, I'd be happy to carry it forward with full credit to you. I'd bring it up to date with main, fix the review points around state saving and failed cancellations, and add tests. I'll be using some AI help and will say so in the PR.

[elsa-3-lead]

Thanks for offering, @JohnyKid123. Let's give @aidevelopers2 a few days to reply here. If there's no answer by then, feel free to carry it forward.

A few pointers so it lands smoothly:

• Open a new PR against main that links this one and Inconsistent Workflow Cancellation Status Display #5187, and keep the original commits or add a Co-authored-by: trailer so the credit stays with @aidevelopers2.
• Address the open review points on this PR, in particular how state is saved when the cancellation is accepted and what happens when a cancellation fails (the instance shouldn't get stuck in Cancelling).
• Add tests covering both paths, plus the immediate Cancelling status.
• Disclosing AI help in the PR description is fine; it gets the same review as any other change.

We'll close this one in favour of yours once it's up.

[JohnyKid123]

Hi maintainers! I'm planning to carry forward @aidevelopers2's work from #7528 with full credit, focusing first on the existing review blockers.

For the initial change, I'll use conditional IWorkflowInstanceStore updates (following TryMarkInterruptedAsync), avoid touching execution snapshots, and restore the previous status when an observed cancellation failure is terminal. Executing instances will keep their current sub-status, and I'll leave transactional outbox/crash recovery for a follow-up.

The one point I'd appreciate confirmation on is whether this narrower scope is acceptable for #5187, given the remaining crash-before-dispatch window. If you'd prefer atomic outbox recovery as part of the initial fix, please let me know.

I'll include tests and disclose AI assistance as before. Thanks!

[elsa-3-lead]

Thanks, @JohnyKid123. @aidevelopers2 hasn't replied since the 22nd, so please go ahead and carry it forward.

Yes, that narrower scope is fine for #5187. It covers what Code Review asked for in item 3 ("restore the previous sub-status when dispatch or the cancel fails"), so atomic outbox recovery doesn't need to be part of the first PR. A few things to keep it safe:

• Conditional writes both ways. Mark Cancelling only when the instance is unowned and not Executing, and touch only SubStatus and UpdatedAt. Apply the same rule when restoring: revert only if the instance is still Cancelling, so a cancel that already finished, or a resumed run, isn't clobbered.
• Cover the dispatch failure too. If DispatchAsync throws, restore the previous sub-status before rethrowing, as well as when the background handler hits a terminal failure.
• Name the gap. Say in the PR description that a crash between marking and dispatching can still leave an instance in Cancelling, and link a follow-up issue for recovery. That keeps the remaining window tracked rather than implicit.
• Tests. Cover an executing or owned instance not being overwritten, a failed dispatch not leaving Cancelling behind, a terminal handler failure restoring the old status, and the immediate Cancelling status on the happy path.

Housekeeping: open the new PR against main (code now lives under core/), link #7528 and #5187, keep @aidevelopers2's commit or add a Co-authored-by: trailer, and sign the CLA when the bot asks. Because Cancelling is a new enum value that older clients can't deserialize by name, this will land in a minor release rather than a patch.

[JohnyKid123]

Follow-up for #5187: opened #9115 against current main, carrying forward @aidevelopers2’s work from this PR with conditional store updates (Phase A scope from the earlier comment). Happy to close or adjust this one once maintainers prefer a single open PR.

[elsa-3-lead]

Thanks, @JohnyKid123. One open PR is easier to track, so review continues on #9115 (milestone 3.11, since it adds a new WorkflowSubStatus member). I'm closing this one as superseded by #9115. @aidevelopers2's work is carried forward there with credit. If anything from here didn't make it into #9115, please mention it there.