elsa-workflows / elsa-workflows/elsa-core
Collapse stimulus dual/triple entry: IStimulusSender vs IStimulusDispatcher vs IWorkflowDispatcher Trigger/Resume
- 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
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