elsa-workflows / elsa-workflows/elsa-core

[Discussion] Restore ability to publish workflows with empty/disabled scheduling triggers (Cron/Timer) — disable triggers properly instead of via empty required values

Open
#7,738 4 comments 0 reactions 0 assignees View on GitHub
discussion enhancement triaged
Dominant language
C#
Stars
7.9k
Forks
1.5k
Avg merge
15h 22m
Merged PRs (30d)
114

Description

## Context

In **Elsa 3.6**, required activity-input validation was tightened so that publishing a workflow now fails when a required property has an invalid value. A concrete consequence: a `Cron` activity with an **empty `CronExpression`** can no longer be published.

Before 3.6, an empty `CronExpression` was allowed at publish time. It (correctly) produced no working trigger, but it let users **publish the workflow anyway** — and some users relied on this as an informal way to **temporarily disable a Cron/Timer trigger** without deleting the activity.

A customer upgrading to 3.6 has hit this: workflows they could previously publish with a blank cron now fail to publish. This issue is to decide how we want to handle "disabled / not-yet-configured" triggers properly, rather than relying on empty values as a sentinel.

## How it works today (3.6, confirmed in code)

- `WorkflowDefinitionPublisher.PublishAsync` hard-gates on validation — **any** `WorkflowValidationError` blocks publish; there is no warning tier:
```csharp
var validationErrors = (await workflowValidator.ValidateAsync(workflowGraph.Workflow, ct)).ToList();
if (validationErrors.Any())
return new(false, validationErrors, new([]));
```
- `TriggerIndexer` indexes a trigger for every activity where `Activity.GetCanStartWorkflow() && Activity is ITrigger`.
- `Cron.GetTriggerPayload` returns `new CronTriggerPayload(cronExpression)` even when the expression is blank.
- `CronTriggerPayloadValidator` parses it via Cronos; a blank/invalid expression throws `CronFormatException` → a `WorkflowValidationError` → **publish blocked**.

Relevant files:
- `src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs`
- `src/modules/Elsa.Workflows.Runtime/Services/TriggerIndexer.cs`
- `src/modules/Elsa.Workflows.Runtime/Handlers/ValidateWorkflowRequestHandler.cs`
- `src/modules/Elsa.Scheduling/Activities/Cron.cs`
- `src/modules/Elsa.Scheduling/TriggerPayloadValidators/CronTriggerPayloadValidator.cs`

## Key insight: a disable switch already exists

`GetCanStartWorkflow()` is effectively a per-activity "is this a trigger" toggle (the designer's *Can start workflow* / start setting). When it is **off**, the activity is **not indexed as a trigger at all** → no required-value validation runs for it, publish succeeds, and no schedule is created.

So the "disable this trigger" capability the team is discussing **mostly already exists** — the gap is discoverability/UX (users reached for "blank the expression" instead of the toggle), not a missing feature.

## Proposal under discussion

Restore the customer's ability to publish, but **without** re-blessing empty-string-as-disable globally. Layered approach:

**1. Immediate, narrow unblock — treat a blank/whitespace required scheduling value as "no trigger".**
When a required scheduling input (e.g. `Cron.CronExpression`, `Timer` interval) is null/empty/whitespace, do **not** index a trigger for it (exclude it before it becomes a `StoredTrigger`). This restores both publish and the old empty-means-disabled behavior, but only for genuinely empty values; a *filled-but-malformed* expression still errors (the genuinely valuable part of the 3.6 validation).
> Note: silencing the validator alone is **not** enough — an empty payload would still create a `StoredTrigger` that throws Cronos at schedule time (the #7033 family). The trigger must not be created in the first place.

**2. Right model (medium term) — make disabling a trigger first-class and discoverable.**
Promote `GetCanStartWorkflow()` (or an explicit per-trigger `Enabled` flag, if its semantics aren't a clean fit) as the sanctioned way to disable a trigger in the designer, so nobody needs to blank a field to disable a schedule.

**3. Optional — a real warnings tier.**
If we want a general "publish with warnings" escape hatch, add a `severity` to `WorkflowValidationError` and only block on `Error`, making warnings an **explicit, opt-in** path — not the default downgrade for all required-property validation.

## Why not simply downgrade all required-property validation to warnings

| Concern | Blanket "error → warning" | Layered approach |
|---|---|---|
| Customer unblocked | ✅ | ✅ |
| Reverts the deliberate 3.6 correctness fix | ❌ across **all** activities | ✅ scoped to blank scheduling values |
| Silent runtime failures | ❌ ignored warnings → e.g. Cronos throws at schedule time (#7033-shaped) | ✅ blank = no trigger, malformed = still caught |
| Empty-as-disable anti-pattern | persists | replaced by a real toggle |

Empty-string-as-disable is a sentinel anti-pattern; the recommendation is to restore it **narrowly** as "blank = no trigger" for relief, and steer users to `CanStartWorkflow` / an explicit `Enabled` flag as the real answer.

## Open questions for the team

1. Do we need existing empty-`CronExpression` workflows to **republish as-is** (→ we need the blank-handling in #1, not just a toggle), or is migrating users to the disable toggle acceptable?
2. Should a blank required value be **completely silent** (treated as intentionally-disabled), or surface as a **non-blocking warning** so it's visible in the designer?
3. Do we want a first-class per-trigger `Enabled` property, or is `CanStartWorkflow` sufficient as the disable mechanism?
4. Should this ship in the **3.6 patch line** (restore-functionality fix) and the toggle/warnings work land in a later minor?

## Related

- #7033 — Quartz `Couldn't store trigger … RunWorkflowJob does not exist` (a malformed/empty cron reaching the scheduler manifests in this family of runtime errors).

Contributor guide

Open the contributing guide

Research direction

Trace validation and trigger indexing through WorkflowDefinitionPublisher.cs, TriggerIndexer.cs, ValidateWorkflowRequestHandler.cs, Cron.cs, and CronTriggerPayloadValidator.cs. Resolve whether blank scheduling values should skip trigger creation, how CanStartWorkflow should be used, and whether warnings are needed; done requires an agreed behavior and acceptance criteria for empty and malformed values.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.