block / block/buzz

Workflow `delay` steps accept durations the executor always rejects, so the documented `5m` and `1h` examples can never run

Open
#3,021 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
32.7k
Forks
4.3k
Avg merge
1d 13h
Merged PRs (30d)
253

Description

## Summary

`WorkflowDef::validate()` has no arm for action definitions, so a `delay` step's `duration` is never checked at save time. The executor caps a delay at a hard-coded 270 seconds. The doc comments on that field give `"5m"` and `"1h"` as the examples, and both are over the cap. A workflow using either one validates, saves, shows as enabled, and then dies at the same step on every run.

## Why it happens

`crates/buzz-workflow/src/executor.rs:670-682`:

```rust
Delay { duration } => {
let secs = parse_duration_secs(duration)?;
// Cap delay at 270 seconds (4.5 minutes) — must be less than default_timeout_secs (300s)
// to avoid non-deterministic StepTimeout. ...
const MAX_DELAY_SECS: u64 = 270;
if secs > MAX_DELAY_SECS {
return Err(WorkflowError::InvalidDefinition(format!(
"delay exceeds maximum of {MAX_DELAY_SECS} seconds (got {secs}s); ..."
)));
}
```

The schema side documents the opposite. `crates/buzz-workflow/src/schema.rs:142-146`:

```rust
/// Pause execution for a duration (e.g. `"5m"`, `"1h"`).
Delay {
/// Duration string (e.g. `"5m"`, `"1h"`).
duration: String,
},
```

`5m` is 300s and `1h` is 3600s, both over 270.

`validate()` (`schema.rs:151-228`) checks the name, steps, step-id shape, duplicate ids and the schedule trigger. There's no `ActionDef` arm at all, so the failure is deferred to run time, once per run, forever.

The crate's own test locks the broken case in:

```rust
" - id: wait\n action: delay\n duration: 5m\n",
```
`crates/buzz-workflow/src/schema.rs:348`, inside `parse_all_action_types`, which asserts the definition parses fine.

There's already a precedent for fixing this in `validate()`. The schedule-interval branch at `schema.rs:214-227` calls `executor::parse_duration_secs` and rejects sub-60s intervals, with a comment explaining the runtime constraint behind it. Same pattern, one arm over.

Worth flagging separately: the 270 constant is a bare literal rather than derived from the timeout it's protecting against. `executor.rs:1136-1138` shows a step can override `default_timeout_secs` via `step.timeout_secs`, so a step with a two-hour timeout still can't delay past 270s.

## Impact

`VISION.md:123` describes workflows as "Channel-scoped YAML-as-code automation ... Every step traced." A workflow that saves clean and then fails identically on every run is that surface failing in the least debuggable way. The YAML matches the documentation, the editor accepted it, and the only signal is a run-time error the user has to go find.

## Suggested fix

Four small pieces:

1. Promote `MAX_DELAY_SECS` out of the function body in `executor.rs` and make it `pub(crate)`, or move it next to `parse_duration_secs`.
2. Add an `ActionDef::Delay` arm to `validate()` that calls `executor::parse_duration_secs` and rejects anything over `MAX_DELAY_SECS`, mirroring `schema.rs:214-227`. That also catches unparseable duration strings at save time, which currently fail only at run time too.
3. Fix the two doc comments at `schema.rs:142-146` to use examples that work (`"30s"`, `"4m"`) and state the ceiling.
4. Update `parse_all_action_types` (`schema.rs:335-351`) to use a duration under the cap, and add a test asserting `duration: 5m` is now rejected.

If long delays are meant to work, the real fix is the scheduled resume pattern the comment mentions as future work, but that's a much bigger change. Rejecting at save time is worth doing either way so nobody saves a workflow that can't run.

The schedule-interval validation tests near `schema.rs:445` are the template for the new rejection test.

Happy to send the PR.

Contributor guide

Open the contributing guide

Research direction

Start with validate() and the schedule-interval checks in crates/buzz-workflow/src/schema.rs:151-228, then inspect parse_duration_secs and MAX_DELAY_SECS in crates/buzz-workflow/src/executor.rs:670-682. Run the schema tests, including parse_all_action_types and the validation tests near schema.rs:445. Done means invalid or over-cap delay durations are rejected at save time, valid examples remain covered, and the documentation matches the limit.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.