fix(logging): close rotated log fd immediately + expose logging.rotation.retention (#683)

[kriszyp]

Fixes #683 — two log-management issues in the rotator.

1. FD handling during rotation (root cause)

moveLogFile closed the module-global descriptor (hdbLogger.closeLogFile(), which only touches mainLogFd) instead of the rotating logger's own descriptor. That never reset the logger's internal logFD, so after a rename the stale fd kept the moved file open — and with compress enabled, the subsequently unlinked inode — until the logger's 10s safety timeout fired. Effects per rotation:

• Disk space pinned on the deleted inode for up to 10s.
• Up to 10s of log writes (including the "hdb.log rotated" notify) landing in the moved/deleted file instead of the fresh hdb.log.

Under high write volume + small maxSize, this recurs continuously and presents as leaking fds / disk to lsof/df.

Fix: close the logger's own closeLogFile immediately after the rename (before the compress step, so the inode is released before the potentially-slow gzip). The next write then reopens a fresh log file right away.

Also fixed an over-broad early-return: the ENOENT handler in the size check returned from the whole audit tick, so retention cleanup was skipped whenever the active log was momentarily absent (idle logging or mid-rotation). Now a missing active log only skips the rotation checks; retention still runs.

2. Old-log deletion — logging.rotation.retention

The rotator already deletes rotated logs older than retention, but the key was absent from both the JSON schema and the Joi validator — it only worked accidentally via Joi's allowUnknown, so it was undocumented and unvalidated. Exposed it in both, with a D/H/M duration validator mirroring the existing interval/maxSize validators, plus schema docs. (Reusing the already-wired retention rather than adding a new maxAgeDays — the "or equivalent" the issue allowed.)

Tests

• logRotator.test.js: added reopen-a-fresh-file-after-rotation and retention-deletion cases (7 passing).
• configValidator.test.js: added retention unit/value/valid cases (49 passing).
• Full logging dir: 51 passing. tsc build clean.

Cross-model review (Gemini + Harper domain adjudication)

No blockers or regressions. Gemini's two "blocker" flags were adjudicated out:

• "nullish-coalescing drops this, crashes on this.logFD" — refuted: both closeLogFile variants are plain closures (over logFD / mainLogFd), not instance methods; no this.
• "throw err in the async tick crashes the process" — pre-existing, unchanged reachability (branch was only inverted). Legitimate hardening follow-up, out of scope here.

Acted on the domain pass's one in-scope note: since the ENOENT change makes retention run in more ticks, wrapped the retention readdir in try/catch so a not-yet-created rotated dir can't throw out of the tick.

Noted for later (pre-existing, not touched): the interval validator message says M (minutes) but convertToMS treats capital M as months; retention's docs/message follow the actual behavior (M=months).

[gemini-code-assist]

Code Review

This pull request introduces a log retention configuration option to automatically delete rotated logs older than a specified age, alongside validation logic and unit tests. The review feedback suggests hardening the validateRotationRetention validator to reject negative, zero, non-string, or empty values to prevent accidental log deletion or runtime TypeErrors. Additionally, the reviewer recommends updating the unit tests to use valid retention units (avoiding '30s') and expanding test coverage to verify these edge cases.

[claude]

Reviewed; no blockers found.

[kriszyp]

Follow-up commit (46494d2): corrected the rotation duration units rather than just noting the discrepancy.

convertToMS treats capital M as months and lowercase m as minutes. The existing interval validator both mislabeled M as "minutes" and rejected lowercase m entirely — so minute-granularity rotation intervals (e.g. 30m) were impossible despite convertToMS supporting them. Both the interval and the new retention validators now accept the full grammar convertToMS handles — D/d (days), H/h (hours), M (months), m (minutes) — with corrected messages and schema docs. Added a test that interval now accepts 30m.

[Ethan-Arrowood]

Nice fix, and good catch on the units grammar.

_{sent with Claude Fable 5}