From 6a73b09d15cbafeaa955ca69accbfd26a0987461 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Tue, 9 Jun 2026 08:11:01 +0000 Subject: [PATCH] opentui(ts): rotate the NDJSON log file (bounded disk use) The ring buffer is bounded (2000) but the NDJSON file was append-only and grew forever. Add size-based rotation mirroring opencode's keep-N model: track bytes written in-process (seeded from statSync on open, so we avoid a statSync on every write) and, when the next line would cross LOG_MAX_BYTES (5 MiB), shift .log -> .log.1 -> ... -> .log.5 (LOG_KEEP=5, oldest dropped) and resume on a fresh file. Rotation is best-effort and fully try/catch-wrapped: any fs failure leaves us appending to the existing file rather than crashing logging. Adds a temp-dir rotation test (seeds >5 MiB to force a rotation on next write). --- ui-tui-opentui-v2/src/boundary/log.ts | 54 +++++++++++++++++++++++++- ui-tui-opentui-v2/src/test/log.test.ts | 38 ++++++++++++++++-- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/ui-tui-opentui-v2/src/boundary/log.ts b/ui-tui-opentui-v2/src/boundary/log.ts index d57014f34ed..c97add497c0 100644 --- a/ui-tui-opentui-v2/src/boundary/log.ts +++ b/ui-tui-opentui-v2/src/boundary/log.ts @@ -16,7 +16,7 @@ * console/stdout/stderr; file + ring only. It's the single approved logging path * for the whole engine. Level filter via HERMES_TUI_LOG_LEVEL (default INFO). */ -import { appendFileSync, mkdirSync } from 'node:fs' +import { appendFileSync, mkdirSync, renameSync, statSync, unlinkSync } from 'node:fs' import { homedir } from 'node:os' import { dirname, join } from 'node:path' @@ -70,6 +70,15 @@ export interface LogEntry { const RING_LIMIT = 2000 +// Size-based rotation for the append-only NDJSON file (mirrors opencode's +// keep-N model, but size- rather than time-keyed since we write one growing +// file). When the live file crosses LOG_MAX_BYTES we shift +// `.log` → `.log.1` → … → `.log.${LOG_KEEP}` (dropping the oldest) and resume on +// a fresh empty `.log`. Rotation is best-effort: any failure leaves us writing +// to the existing file (logging must never crash the engine). +const LOG_MAX_BYTES = 5 * 1024 * 1024 +const LOG_KEEP = 5 + function defaultLogFile(): string { const explicit = process.env.HERMES_TUI_LOG_FILE?.trim() if (explicit) return explicit @@ -92,6 +101,11 @@ export class Log { private file: string | null private fileBroken = false private minPriority: number + // Bytes in the live log file. Seeded from statSync on open (counter approach — + // we avoid a statSync on EVERY write); incremented by each line's byte length + // and reset to 0 after a rotation. Rotation triggers when this would cross + // LOG_MAX_BYTES, so the live file stays bounded without per-write fs stats. + private fileBytes = 0 constructor(file: string | null = defaultLogFile(), level: LogLevel = defaultLevel()) { this.file = file @@ -102,6 +116,11 @@ export class Log { } catch { this.fileBroken = true } + try { + this.fileBytes = statSync(this.file).size + } catch { + this.fileBytes = 0 // no existing file (or unreadable) → start the counter at 0 + } } } @@ -109,6 +128,34 @@ export class Log { this.minPriority = PRIORITY[level] } + /** + * Best-effort size-based rotation: `.log.${LOG_KEEP}` is dropped, every other + * `.log.N` shifts up, the live `.log` becomes `.log.1`, and the counter resets + * so writing continues on a fresh file. Any fs failure is swallowed and we keep + * writing to the existing file — rotation must never crash logging. + */ + private rotate(file: string): void { + try { + try { + unlinkSync(`${file}.${LOG_KEEP}`) + } catch { + // oldest slot may not exist yet — fine + } + for (let i = LOG_KEEP - 1; i >= 1; i--) { + try { + renameSync(`${file}.${i}`, `${file}.${i + 1}`) + } catch { + // that slot may not exist yet — fine + } + } + renameSync(file, `${file}.1`) + this.fileBytes = 0 + } catch { + // rotation failed (e.g. live file vanished) — leave the counter alone and + // keep appending to the existing path; better an oversized log than none. + } + } + private write(level: LogLevel, scope: string, msg: string, data?: unknown): void { if (PRIORITY[level] < this.minPriority) return const entry: LogEntry = @@ -118,7 +165,10 @@ export class Log { if (this.file && !this.fileBroken) { try { - appendFileSync(this.file, safeStringify(entry) + '\n') + const line = safeStringify(entry) + '\n' + if (this.fileBytes > 0 && this.fileBytes + Buffer.byteLength(line) > LOG_MAX_BYTES) this.rotate(this.file) + appendFileSync(this.file, line) + this.fileBytes += Buffer.byteLength(line) } catch { this.fileBroken = true // stop hammering a broken path; the ring keeps working } diff --git a/ui-tui-opentui-v2/src/test/log.test.ts b/ui-tui-opentui-v2/src/test/log.test.ts index 7b258bc56ad..6da49994714 100644 --- a/ui-tui-opentui-v2/src/test/log.test.ts +++ b/ui-tui-opentui-v2/src/test/log.test.ts @@ -1,10 +1,13 @@ /** - * Log hardening (boundary/log.ts): safeStringify never throws on a circular ref / - * BigInt / hostile toJSON, so one bad `data` payload can't flip `fileBroken` and - * kill file logging for the session. + * Log hardening (boundary/log.ts): + * - safeStringify never throws on a circular ref / BigInt / hostile toJSON, + * so one bad `data` payload can't flip `fileBroken` and kill file logging. + * - the NDJSON file rotates by size (counter-driven), keeping disk bounded. + * Rotation is exercised with a real temp dir; since LOG_MAX_BYTES (5 MiB) is not + * exported, we seed the live file above the cap so the next write must rotate. */ import { describe, expect, test } from 'bun:test' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -74,6 +77,33 @@ describe('Log file logging survives a poison payload', () => { }) }) +describe('Log file rotation', () => { + test('rotates the live file once it crosses the byte cap', () => { + const dir = mkdtempSync(join(tmpdir(), 'hermes-log-rotate-')) + const file = join(dir, 'opentui-v2.log') + try { + // Seed the live file ABOVE the 5 MiB cap so the very next write rotates. + writeFileSync(file, 'x'.repeat(5 * 1024 * 1024 + 10) + '\n') + const log = new Log(file, 'debug') + log.info('test', 'first write after seed') // crosses the cap -> rotates + log.info('test', 'second write on fresh file') + + const names = readdirSync(dir).sort() + expect(names).toContain('opentui-v2.log') + expect(names).toContain('opentui-v2.log.1') // the seeded oversized file + // The fresh live file holds the post-rotation writes, not the seed. + const live = readFileLines(file) + expect(live.length).toBe(2) + expect(live[0]).toContain('first write after seed') + // The rotated-out file is the big seed. + const rotated = readFileLines(`${file}.1`) + expect(rotated[0]?.startsWith('xxxx')).toBe(true) + } finally { + rmSync(dir, { recursive: true, force: true }) + } + }) +}) + function readFileLines(path: string): string[] { // trailing newline produces an empty tail we drop const text = readFileSync(path, 'utf8')