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).
This commit is contained in:
alt-glitch 2026-06-09 08:11:01 +00:00
parent af82979d43
commit 6a73b09d15
2 changed files with 86 additions and 6 deletions

View file

@ -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
}

View file

@ -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')