Repository navigation
fix: apply PLUK_LOG_MAX_BYTES cap even when the log has fewer than PLUK_LOG_KEEP_LINES lines - #284
Conversation
…nes lines Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Read rotateLogFileIfNeeded at ab723a7 (src/run-dir.ts:153-227) and its single caller (src/cli.ts:212). This looks correct to me.
Checked the new early return (src/run-dir.ts:190, drop === 0 && kept.length === usableLines.length) against each path:
- Fewer than
keepLineslines, overmaxBytes(the #283 case):keptis the whole file, thewhileloop at:186drops oldest lines until it fits,drop > 0→ rewrite. New test intest/run-dir.test.js:252pins it; byte math (8 bytes/line, 20-byte cap → last two lines) is right. - More than
keepLineslines, already undermaxBytesafter the line trim:drop === 0butkept.length < usableLines.length→ still rewrites. The pre-existing line-trim behaviour is preserved. - Single line that alone exceeds
maxBytes: loop conditiondrop < kept.length - 1is false immediately,kept.length === usableLines.length === 1→false. Matches the doc comment at:147-149and the existing "never drops the newest line" test. - Under
maxBytes: already short-circuited at:166, unchanged.
README PLUK_LOG_KEEP_LINES row (README.md:92) already describes the post-fix contract ("within the PLUK_LOG_MAX_BYTES cap"), so no docs change is needed. The rewritten test/default-fallbacks.test.js case was pinning the bug, not a feature, so replacing it is the right call.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Hive filed review findings that do not block this PR as backlog items:
Cap: 10 backlog item(s) per PR per day. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Removes the early return in rotateLogFileIfNeeded that skipped the byte-cap loop when the log had <= keepLines lines. Returns false only when nothing is dropped (e.g. single oversized line). Updates the pinned fallback test, adds a regression test and changelog entry.
Closes #283
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88