dagucloud / dagucloud/dagu

bug: max_active_steps treats only 0 as unlimited — a negative value deadlocks the run and an inherited limit cannot be removed

Closed
#2,771 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.