elsa-workflows / elsa-workflows/elsa-core

Apply workflow activation strategies atomically to dispatched workflow definitions

Open
#8,042 0 comments 0 reactions 0 assignees View on GitHub
bug core prio high triaged
Dominant language
C#
Stars
7.9k
Forks
1.5k
Avg merge
15h 22m
Merged PRs (30d)
114

Description

## Summary

Implement the design follow-up for #7948 by making workflow activation strategies apply to workflow-definition dispatches and by enforcing exclusivity atomically.

`CorrelationId` should remain descriptive metadata by default. A workflow definition that needs singleton behavior opts into the existing activation strategy model (`SingletonStrategy`, `CorrelatedSingletonStrategy`, `CorrelationStrategy`, or a custom strategy). The selected strategy must govern every path that creates a new workflow instance, including asynchronous definition dispatch.

## Current behavior

The runtime currently has two different start paths:

- `DefaultWorkflowStarter.StartWorkflowAsync` resolves the workflow and evaluates its configured activation strategy before creating and running an instance.
- `TriggerInvoker` uses `IWorkflowStarter`, so trigger-based starts use the activation strategy.
- `DispatchWorkflowCommandHandler.HandleAsync(DispatchWorkflowDefinitionCommand, ...)` creates an `IWorkflowClient` and calls `CreateAndRunInstanceAsync` directly. This bypasses `IWorkflowStarter` and the activation strategy.

The existing correlated strategies also use a check-then-act implementation: they query `IWorkflowInstanceStore.CountAsync` and return a boolean. Evaluation and instance creation are separate operations, so two concurrent requests can both be admitted before either instance becomes visible in persistence.

As a result, merely adding a call to `CanStartWorkflowAsync` in the dispatch handler would align the code paths but would not close the concurrency race described by #7948.

## Design decision

### 1. Activation strategy is the definition-level admission policy

Honor `Workflow.Options.ActivationStrategyType` whenever a new workflow instance is requested, regardless of whether the request came from:

- synchronous workflow execution;
- a workflow trigger; or
- `IWorkflowDispatcher.DispatchAsync(DispatchWorkflowDefinitionRequest, ...)`.

Do not add a competing `SingleInstancePerCorrelationId` flag to `DispatchWorkflowDefinitionRequest`. That would duplicate the activation strategy model and allow individual callers to override a workflow-definition invariant.

The existing explicit-instance-ID replay protection (`InstanceId` / `SkipIfInstanceExists`) remains an orthogonal idempotency mechanism and must retain its current behavior.

### 2. Use one shared policy-enforcing start boundary

Refactor definition dispatch so it enters the same policy-enforcing start boundary as direct and trigger-based starts. Prefer routing the command handler through `IWorkflowStarter`, or extract an equivalent shared activation/start coordinator used by both `DefaultWorkflowStarter` and the dispatch handler.

The shared path must preserve all dispatch data currently passed to `CreateAndRunWorkflowInstanceRequest`, including:

- the requested/generated instance ID;
- definition version ID;
- correlation ID;
- input and properties;
- parent workflow instance ID;
- trigger activity ID; and
- scheduling activity execution ID, scheduling workflow instance ID, and scheduling call-stack depth.

If `StartWorkflowRequest` is extended to carry these fields, additions should be optional and backward-compatible. Avoid removing or changing existing public members.

### 3. Make exclusive activation atomic

Do not implement exclusive strategies as an unprotected `CountAsync` followed by creation.

For built-in exclusive strategies, serialize evaluation and the first durable persistence of the admitted instance using the configured `IDistributedLockProvider`, or provide an equivalent persistence-backed atomic activation claim. The exclusivity scope is:

| Strategy | Exclusivity scope |
| --- | --- |
| `SingletonStrategy` | tenant + workflow definition ID |
| `CorrelatedSingletonStrategy` | tenant + workflow definition ID + correlation ID |
| `CorrelationStrategy` | tenant + correlation ID |

Requirements for a lock-based implementation:

- Derive a deterministic key from the strategy's exclusivity scope.
- Include tenant scope so unrelated tenants never block one another.
- Do not put raw correlation IDs in lock names or logs; hash the canonical scope components.
- Acquire the lock before evaluating the strategy.
- Re-evaluate after acquiring the lock.
- Hold the lock until the admitted instance is durably visible. Releasing it after in-memory object creation is insufficient.
- Treat lock acquisition timeout/provider failure as infrastructure failure, not as an activation denial.
- Document that cross-node correctness requires a lock provider shared by all runtime nodes. If Elsa intends to guarantee this solely from shared workflow persistence, use a persistence-backed atomic claim instead.

Avoid a single unique index on `WorkflowInstance.CorrelationId`: correlation IDs are not universally unique, and the required scope differs by activation strategy. Such an index would also break definitions using the default/allow-always behavior.

Preserve compatibility for custom `IWorkflowActivationStrategy` implementations. If a new optional capability is needed to provide an atomic exclusivity scope/lease, do not make existing custom strategies fail to load merely because they implement the current boolean contract.

### 4. Define denial and failure semantics

- Activation denial is an expected outcome: no instance is created or run.
- Direct/trigger-based starts continue to surface `StartWorkflowResponse.CannotStart`.
- Background `DispatchAsync` continues to mean accepted/queued, not successfully activated. Because activation happens later, it must not claim synchronously that an instance was created.
- When a dispatched activation is denied, emit an actionable structured log and, if an existing suitable runtime notification pattern exists, a notification containing the definition handle, strategy type, and correlation ID. Do not include sensitive raw correlation values where operational policy requires redaction.
- If a workflow definition names an activation strategy that cannot be resolved, fail closed with an actionable error instead of silently allowing the workflow to start.
- `CorrelationStrategy` and `CorrelatedSingletonStrategy` require a non-empty correlation ID. A missing correlation ID must produce an explicit validation/activation failure rather than treating every null correlation as the same singleton bucket.

### 5. Do not auto-resume on definition dispatch

If activation is denied because a correlated running instance exists, the definition-dispatch command should not automatically resume that instance. A definition dispatch does not carry a bookmark/stimulus that identifies what should be resumed. Resumption remains the responsibility of the existing stimulus/bookmark path.

## Acceptance criteria

- [ ] A dispatched workflow definition with no configured activation strategy retains the existing allow behavior.
- [ ] `AllowAlwaysStrategy` permits multiple instances with the same correlation ID.
- [ ] `SingletonStrategy` permits only one running instance of a definition, regardless of correlation ID.
- [ ] `CorrelatedSingletonStrategy` permits only one running instance for the same tenant, definition ID, and correlation ID.
- [ ] `CorrelatedSingletonStrategy` still permits the same correlation ID for different definitions.
- [ ] `CorrelationStrategy` permits only one running instance for the same tenant and correlation ID, including across different definitions.
- [ ] After the prior matching instance reaches a terminal status, a new matching instance can be admitted, preserving current strategy semantics.
- [ ] Two concurrent dispatches in one process cannot create duplicate running instances for an exclusive activation scope.
- [ ] Two concurrent dispatches handled by separate runtime nodes using a shared distributed lock provider cannot create duplicate running instances for an exclusive activation scope.
- [ ] The activation decision is re-evaluated while holding the exclusivity lease, and the lease is held until the admitted instance is durably visible.
- [ ] Explicit-instance-ID dispatch idempotency remains functional and does not conflict with activation-strategy admission.
- [ ] A dispatched activation denial creates no workflow instance and is observable through structured logging/notification.
- [ ] A configured but unregistered activation strategy fails closed.
- [ ] A correlation-dependent strategy invoked without a correlation ID fails explicitly and creates no instance.
- [ ] Definition dispatch preserves parent, trigger, input, properties, and scheduling/call-stack metadata.
- [ ] Existing custom activation strategies remain source-compatible unless a separately approved breaking change is documented.

## Suggested tests

Add focused unit tests for:

- dispatch command routing through the shared start/admission boundary;
- preservation of all `DispatchWorkflowDefinitionCommand` fields;
- activation-denied behavior and observability;
- missing strategy resolution and missing correlation ID;
- lock-key scope for each built-in exclusive strategy; and
- preservation of `InstanceId` / `SkipIfInstanceExists` behavior.

Add concurrency-focused integration tests that release two starts simultaneously and assert the resulting instance count. Cover at least:

1. same process, same definition and correlation;
2. separate service scopes/runtime nodes sharing persistence and a distributed lock provider;
3. same correlation with different definitions under `CorrelatedSingletonStrategy`;
4. same correlation with different definitions under `CorrelationStrategy`; and
5. a second start after the first matching instance becomes terminal.

The regression test must assert persisted workflow instances, not only returned `CannotStart` flags.

## Likely implementation areas

- `src/modules/Elsa.Workflows.Runtime/Handlers/DispatchWorkflowRequestHandler.cs`
- `src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowStarter.cs`
- `src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowActivationStrategyEvaluator.cs`
- `src/modules/Elsa.Workflows.Runtime/Requests/StartWorkflowRequest.cs`
- `src/modules/Elsa.Workflows.Runtime/ActivationValidators/*Strategy.cs`
- runtime DI registration in `WorkflowRuntimeFeature`
- corresponding runtime unit and integration test projects

## Out of scope

- Making `CorrelationId` globally unique for every workflow definition.
- Automatically resuming an existing instance from a definition-dispatch request.
- Changing background dispatch into synchronous workflow execution.
- Removing or replacing explicit instance-ID idempotency.

## Related issue

- #7948

Contributor guide

Open the contributing guide

Research direction

Start with DispatchWorkflowRequestHandler.cs, DefaultWorkflowStarter.cs, DefaultWorkflowActivationStrategyEvaluator.cs, StartWorkflowRequest.cs, and the ActivationValidators strategy files. Read the runtime DI registration and run the existing runtime unit and integration tests first. Done means definition dispatch uses the shared admission boundary, exclusive starts are atomic and observable, metadata and idempotency are preserved, and the listed concurrency cases pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend, distributed-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.