fix: do not resume interrupted workflows that are already finished#7435
fix: do not resume interrupted workflows that are already finished#7435jwdb wants to merge 1 commit intoelsa-workflows:mainfrom
Conversation
Greptile SummaryThis PR fixes an infinite restart loop for workflow instances that have Confidence Score: 4/5Safe to merge; the fix is correct and well-tested, with only a minor test robustness concern. The production change is a single-line addition to a query filter that is clearly correct given the two-value WorkflowStatus enum. The only finding is a P2 test fragility around hardcoded timestamps, which does not affect correctness of the fix. test/integration/Elsa.Workflows.IntegrationTests/GracefulShutdown/RestartInterruptedWorkflowsTests.cs — hardcoded absolute timestamps may become fragile with mocked clocks.
|
| Filename | Overview |
|---|---|
| src/modules/Elsa.Workflows.Runtime/Tasks/RestartInterruptedWorkflowsTask.cs | Adds WorkflowStatus = WorkflowStatus.Running to the instance filter, correctly excluding finished workflows from the restart scan. |
| test/integration/Elsa.Workflows.IntegrationTests/GracefulShutdown/RestartInterruptedWorkflowsTests.cs | New integration test validating the filter; uses hardcoded absolute timestamps from April 2026 which could be fragile in environments using a mocked system clock. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[RestartInterruptedWorkflowsTask.ExecuteAsync] --> B[CreateWorkflowInstanceFilter]
B --> C{Filter conditions}
C --> D[IsExecuting = true]
C --> E[BeforeLastUpdated = cutoffTimestamp]
C --> F["WorkflowStatus = Running NEW"]
D & E & F --> G[Query WorkflowInstanceStore]
G --> H{Result set}
H -->|Running + stale + executing| I[RestartWorkflowAsync]
H -->|Finished + stale + executing| J[Excluded by new filter]
I --> K[Workflow resumed]
J --> L[No restart - bug fixed]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
test/integration/Elsa.Workflows.IntegrationTests/GracefulShutdown/RestartInterruptedWorkflowsTests.cs:46-47
**Hardcoded timestamps may cause flaky tests with a mocked clock**
The seeded `UpdatedAt`/`CreatedAt` values are pinned to `2026-04-13T12:00:00Z`. The `RestartInterruptedWorkflowsTask` computes its `BeforeLastUpdated` cutoff via `ISystemClock.UtcNow - InactivityThreshold`. If the test service provider registers a mocked or frozen clock set to a time on or before that date (or very close to it), the seeded instances would fall *after* the cutoff and the filter would return zero results — causing the `Assert.Equal(3, ...)` assertion to fail silently. Using a relative offset like `DateTimeOffset.UtcNow.AddDays(-7)` keeps the test robust regardless of clock configuration.
Reviews (1): Last reviewed commit: "Fix: do not resume interrupted workflows..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for (var i = 0; i < count; i++) | ||
| { |
There was a problem hiding this comment.
The seeded UpdatedAt/CreatedAt values are pinned to 2026-04-13T12:00:00Z. The RestartInterruptedWorkflowsTask computes its BeforeLastUpdated cutoff via ISystemClock.UtcNow - InactivityThreshold. If the test service provider registers a mocked or frozen clock set to a time on or before that date (or very close to it), the seeded instances would fall after the cutoff and the filter would return zero results — causing the Assert.Equal(3, ...) assertion to fail silently. Using a relative offset like DateTimeOffset.UtcNow.AddDays(-7) keeps the test robust regardless of clock configuration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/integration/Elsa.Workflows.IntegrationTests/GracefulShutdown/RestartInterruptedWorkflowsTests.cs
Line: 46-47
Comment:
**Hardcoded timestamps may cause flaky tests with a mocked clock**
The seeded `UpdatedAt`/`CreatedAt` values are pinned to `2026-04-13T12:00:00Z`. The `RestartInterruptedWorkflowsTask` computes its `BeforeLastUpdated` cutoff via `ISystemClock.UtcNow - InactivityThreshold`. If the test service provider registers a mocked or frozen clock set to a time on or before that date (or very close to it), the seeded instances would fall *after* the cutoff and the filter would return zero results — causing the `Assert.Equal(3, ...)` assertion to fail silently. Using a relative offset like `DateTimeOffset.UtcNow.AddDays(-7)` keeps the test robust regardless of clock configuration.
How can I resolve this? If you propose a fix, please make it concise.
|
@dotnet-policy-service agree |
1 participant
Purpose
Prevent restarting interrupted but finished workflows that have IsExecuting = 1 but WorkflowStatus = Finished. Currently these end up in a infinite restart cycle.
Scope
Select one primary concern:
Description
Problem
This solves an infinite restart of interrupted workflows that ended up in a semi-invalid state.
Solution
Extend the filter for the restart to ignore finished workflows
Commit Convention
We recommend using conventional commit prefixes:
fix:– Bug fixes (behavior change)feat:– New featuresrefactor:– Code changes without behavior changedocs:– Documentation updateschore:– Maintenance, tooling, or dependency updatestest:– Test additions or modificationsClear commit messages make reviews easier and history more meaningful.
Checklist