fix(daemon): honor max_rotated_files == 0 in rotate_log_files

[phil-opp]

Issue

rotate_log_files (binaries/daemon/src/log.rs) rotates node logs by:

1. deleting the oldest rotated file log_<node>.<max_files>.jsonl,
2. shifting .N -> .N+1 for i in (1..max_files),
3. renaming the current file to log_<node>.1.jsonl.

When max_files == 0:

• the delete step targets log_<node>.0.jsonl, which is never created,
• the shift range (1..0) is empty,
• but the current file is still renamed to log_<node>.1.jsonl.

That .1 file is never pruned on subsequent rotations either, so a node configured with max_rotated_files: 0 keeps a rotated log forever instead of retaining zero — directly contradicting the requested value.

max_rotated_files is user-configurable through the dataflow descriptor (libraries/message/src/descriptor.rs) and reaches this public library function via binaries/daemon/src/spawn/prepared.rs (.unwrap_or(DEFAULT_MAX_ROTATED_FILES) only substitutes the default when the option is None, not when it is Some(0)), so 0 is a reachable input.

Fix

Special-case max_files == 0: discard the current log and prune any rotated files a previous (non-zero) configuration may have left behind, so the "retain no rotated copies" contract holds regardless of history. All other values keep their existing behavior.

Validation

• rotate_with_zero_max_files_discards_current — the current file is gone, no .1 is left, and a repeated rotation does not accumulate files.
• rotate_with_zero_max_files_prunes_preexisting_rotated_files — pre-existing .1/.2 rotated files are removed too.
• cargo test -p dora-daemon --lib log:: — 12 passed (existing rotation tests unchanged).
• cargo fmt --all -- --check and cargo clippy -p dora-daemon -- -D warnings clean.

The pre-existing-rotated-file pruning was added after an automated /review pass noted the original fix only removed the current file.

---

🤖 This is a machine-generated pull request opened by Claude Code as part of an automated code-review pass. Please review carefully before merging.

🤖 Generated with Claude Code

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[phil-opp]

🤖 Automated review by Claude — this is a fully automated review with no human in the loop.

I reviewed this diff and found no issues. The max_files == 0 special case correctly discards the current log and prunes any leftover rotated files, fixing the prior behavior where the shift range (1..0) was empty and the current file was still renamed to .1 and never pruned. Both regression tests assert real filesystem effects and cover the single/repeated-rotation and pre-existing-rotated-files cases.

One minor, non-blocking observation: the prune loop stops at the first missing index, so it assumes rotated files are contiguous. That holds for files produced by this rotation scheme, so it's fine in practice.

---

Generated by Claude Code

[phil-opp]

I found one issue with the premise of this change: max_rotated_files: 0 isn't reachable through the descriptor path, so the user-facing scenario this fixes can't actually occur, and the two new tests exercise an input the descriptor contract forbids.

Descriptor validation rejects 0 before it can reach rotate_log_files:

• libraries/core/src/descriptor/validate.rs:400-402 — if n == 0 { bail!("\max_rotated_files` must be at least 1"); }`
• The daemon caller binaries/daemon/src/spawn/prepared.rs:895-899 obtains the value via this same validating accessor (self.node.max_rotated_files()?) and only then applies .unwrap_or(DEFAULT_MAX_ROTATED_FILES). A config of 0 therefore fails node spawn with "must be at least 1"; it never reaches rotate_log_files(..., 0). Since the diff only touches log.rs, that validation floor is unchanged.

That leaves two things worth resolving:

1. rotate_with_zero_max_files_discards_current and rotate_with_zero_max_files_prunes_preexisting_rotated_files assert behavior for max_files == 0, which the validation layer defines as invalid. They lock in the public function's contract but not a reachable regression.
2. There's now a semantic contradiction: validate.rs treats 0 as an error ("must be at least 1"), while log.rs now treats 0 as a valid "retain no rotated copies" value.

If the intent is for 0 to mean "keep none", the validation floor in validate.rs should be lowered to match (and the descriptor/schema docs updated) — that would also make the new tests exercise a real, reachable path. If 0 is meant to remain invalid, this branch is unreachable defensive code and should be framed/commented as such rather than as a user-facing fix. The branch logic itself (discard current + prune rotated files) is correct for the "keep none" semantics.

---

🤖 Automated review by Claude (Claude Code). This review was generated fully automatically with no human in the loop and has not been vetted by a maintainer — treat it as advisory.

---

Generated by Claude Code