bug: max_active_steps treats only 0 as unlimited — a negative value deadlocks the run and an inherited limit cannot be removed
- Dominant language
- Go
- Stars
- 4k
- Forks
- 332
- Avg merge
- 19h 9m
- Merged PRs (30d)
- 137
Description
## Describe the bug
`max_active_steps` treats only an exact `0` as unlimited, while `timeout_sec` treats any non-positive value as "no limit". The asymmetry has two consequences: a negative `max_active_steps` fails the run instead of removing the limit, and — since base-config inheritance landed in v2.16.6 — a DAG cannot remove a limit it inherits from `base.yaml`.
`internal/runtime/runner.go:350-351`:
```go
// 2. maxActiveRuns is 0 (unlimited) OR running < maxActiveRuns
if !r.isCanceled() && (r.maxActiveRuns == 0 || running < r.maxActiveRuns) {
activeReadyCh = readyCh
}
```
With `max_active_steps: -1`, neither branch is true, so `activeReadyCh` stays nil and the next check fires:
```go
// internal/runtime/runner.go:367-372
if running == 0 && len(activeReadyCh) == 0 && !plan.CheckFinished() {
r.setLastError(ErrDeadlockDetected)
logger.Error(ctx, "Deadlock detected: no runnable nodes remaining")
break
}
```
The run fails immediately with `Deadlock detected: no runnable nodes remaining`, before any step starts.
`timeout_sec` has no such problem: every guard is `r.timeout > 0` (`internal/runtime/runner.go:159`, `:221`, `:1482`), so `-1` simply means "no timeout". Neither field carries a `minimum` in `internal/cmn/schema/dag.schema.json`, so both values pass validation.
## Why it matters beyond the footgun
`Timeout` and `MaxActiveSteps` are plain `int` on the spec (`internal/spec/dag.go`), not the pointer types that #2770 introduced for `queue`, `overlap_policy`, `catchup_window`, `max_active_runs` and `skip_if_successful`. Base composition is `mergo.Merge(effective, current, WithOverride)`, which only lets a **zero** field inherit — so a DAG that writes `max_active_steps: 0` does not override an inherited value, it re-inherits it.
That leaves no way to express "this DAG has no step limit" once `base.yaml` sets one:
- `max_active_steps: 0` → the inherited limit stays in force
- `max_active_steps: -1` → the run fails with the deadlock error above
`timeout_sec` escapes this by accident: `-1` is non-zero, so it wins the merge, and the runner reads any non-positive value as no timeout.
## To Reproduce
```yaml
# dags/wide.yaml
max_active_steps: -1
steps:
- id: a
command: echo a
- id: b
command: echo b
```
`dagu start dags/wide.yaml` → the run ends immediately, no step executes, and the log carries `Deadlock detected: no runnable nodes remaining`.
Inheritance case:
```yaml
# base.yaml
max_active_steps: 4
```
```yaml
# dags/wide.yaml
max_active_steps: 0 # intended: no limit
steps: [...]
```
The DAG still runs at most 4 steps at a time.
## Expected behavior
`max_active_steps` behaves like `timeout_sec`: any non-positive value means unlimited, and a DAG can override an inherited limit.
## Environment
- Dagu version: v2.16.6 (`origin/main`, `58fed633d`)
- OS: macOS 15 (arm64)
- Installation method: binary
## Additional context
The minimal change is the unlimited test at `internal/runtime/runner.go:351`:
```diff
-if !r.isCanceled() && (r.maxActiveRuns == 0 || running < r.maxActiveRuns) {
+if !r.isCanceled() && (r.maxActiveRuns <= 0 || running < r.maxActiveRuns) {
```
`MaxActiveSteps` reaches the runner in one place (`runner.go:99`, from `internal/runtime/agent/agent.go:1821`) and is otherwise only reported to the API (`internal/service/frontend/api/v1/transformer.go:713`), so nothing else reads the sentinel.
That alone fixes both the footgun and the inheritance gap, because `-1` then becomes a non-zero spelling of "unlimited" that wins the merge. Making `Timeout` and `MaxActiveSteps` pointer fields the way #2770 did for the scheduling fields would be the more consistent fix, and would also let `0` mean "unlimited" on override — but it is a larger change and not required for the above.
Not submitting a PR; filing this so the behavior is recorded.
Contributor guide
Research direction
Start in internal/runtime/runner.go at the max-active-steps guard and reproduce the issue with the provided wide DAG and dagu start command. Read internal/spec/dag.go, internal/cmn/schema/dag.schema.json, and the base-config merge behavior to check the unlimited-value path. Done means non-positive max_active_steps values allow execution and an inherited limit can be removed without a deadlock.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, devops
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100