elsa-workflows / elsa-workflows/elsa-core

Collapse stimulus dual/triple entry: IStimulusSender vs IStimulusDispatcher vs IWorkflowDispatcher Trigger/Resume

Open
#8,149 0 comments 0 reactions 0 assignees View on GitHub
core elsa 3 maintenance prio low tech debt triaged
Dominant language
C#
Stars
7.9k
Forks
1.5k
Avg merge
15h 22m
Merged PRs (30d)
114

Description

## Problem

Stimulus delivery has **three overlapping public entry surfaces** that all collapse to the same broker (`StimulusSender`), with different request shapes, return types, and durability. Callers must know which interface means “sync with results” vs “fire-and-forget background” vs “background with optional transactional outbox” — accidental complexity that fights subtractive clarity.

## Evidence (main)

### 1. Sync broker (canonical work)

`IStimulusSender` / `StimulusSender` — hashes stimulus, triggers new workflows, resumes bookmarks, unmatched-enqueue to bookmark queue. Returns `SendStimulusResult`.

### 2. Thin async wrapper (parallel API)

`IStimulusDispatcher` / `BackgroundStimulusDispatcher` — only implementation; wraps `DispatchStimulusRequest` → `DispatchStimulusCommand` → `DispatchStimulusCommandHandler` → **`IStimulusSender.SendAsync`**. Always returns `DispatchStimulusResponse.Empty`. Uses `CommandStrategy.Background` + `CancellationToken.None`.

Default DI: `WorkflowRuntimeFeature.StimulusDispatcher` → `BackgroundStimulusDispatcher` only (no Local / Transactional / Validating stack).

### 3. Workflow-dispatcher trigger/resume overloads (third async path)

`IWorkflowDispatcher.DispatchAsync(DispatchTriggerWorkflowsRequest)` and `DispatchAsync(DispatchResumeWorkflowsRequest)` → background (or **transactional outbox**) commands → `DispatchWorkflowCommandHandler` → **same** `IStimulusSender.SendAsync`.

Both Trigger and Resume handlers map metadata identically and call the full `SendAsync` (trigger-new + resume). Resume does **not** mean resume-only; gating is metadata fields inside `StimulusSender`, not the command type. So Trigger vs Resume is a **false dual**.

In-repo async consumers of Trigger/Resume are essentially the obsolete `StimulusProxyWorkflowInbox` (see #8136) plus the dispatcher stack itself — while the “modern” async Event path invented a separate `IStimulusDispatcher`.

### 4. EventPublisher is the dual-entry switch

`EventPublisher.PublishAsync(..., asynchronous: …)`:

- `false` → `IStimulusSender`
- `true` → `IStimulusDispatcher`

`PublishEvent` activity **always** passes `asynchronous: true` (see sibling issue on outbox bypass). API event endpoints default sync (`asynchronous` defaults false).

### Naming leak

Both `IStimulusSender` and `IStimulusDispatcher` expose `SendAsync` with overlapping XML docs (“new instances as well as resume”). Workflow side consistently uses `DispatchAsync` on `IWorkflowDispatcher`.

## Why it matters

- **Maintainability:** three mental models / request DTOs / command types for one job.
- **Correctness risk:** durability and cancellation semantics diverge by which entry you pick (StimulusDispatcher never hits `TransactionalWorkflowDispatcher`; WorkflowDispatcher Trigger/Resume can).
- **Subtractive clarity:** #8136 removes obsolete inbox DI; this issue is the **remaining** modern dual stack after that cut.

## Proposed subtractive direction

Prefer **one sync story + one async story**, not three:

1. Keep `IStimulusSender` as the only synchronous stimulus broker (results, hashing, unmatched enqueue).
2. Pick **one** async façade:
- either route all async stimulus through `IWorkflowDispatcher` Trigger (and delete `IStimulusDispatcher` / `DispatchStimulusCommand`), **or**
- keep `IStimulusDispatcher` but make it the sole async entry and **remove** Trigger/Resume overloads from `IWorkflowDispatcher` once obsolete inbox is gone.
3. Collapse Trigger vs Resume into one request/command (or document that Resume is metadata-only and delete the duplicate type).
4. Make `EventPublisher` / `PublishEvent` call that single async façade (no bool fork between two interfaces).
5. Do **not** invent a new framework — delete or quarantine the redundant path.

## Not a duplicate of

- #8134 / #8135 — bookmark hash default / unmatched-enqueue field drops (StimulusSender internals)
- #8136 — obsolete `IBookmarkResumer` / `IWorkflowInbox` DI (related cleanup; this is the modern Sender↔Dispatcher↔WorkflowDispatcher overlap)
- #8145 — BackgroundActivityInvokerMiddleware property-bag IPC
- #7313 — broad runtime inventory epic

## Milestone

Unset — Elsa 3 Issue Triage / Crew Lead place. Soft proposal: Backlog / tech-debt lane. Do not auto-assign Engineer.

Contributor guide

Open the contributing guide

Research direction

Trace IStimulusSender, IStimulusDispatcher, IWorkflowDispatcher, EventPublisher, and the Trigger/Resume command handlers, then compare their request, result, durability, and cancellation behavior. Review the related #8136 cleanup and in-repo consumers before choosing a single async façade; done means the redundant public paths and duplicate Trigger/Resume model are removed or explicitly consolidated without losing required dispatch behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend-api-design
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.