From dca28eee66c6909b101ea8d15b6c42a585ff668f Mon Sep 17 00:00:00 2001 From: Wolfie Date: Mon, 3 Aug 2026 01:48:51 -0700 Subject: [PATCH] fix(utils): retain one rolled log per process day --- packages/utils/CHANGELOG.md | 3 + packages/utils/src/logger.ts | 63 +++++++++++++---- .../utils/test/logger-multiprocess.test.ts | 70 ++++++++++++++----- 3 files changed, 105 insertions(+), 31 deletions(-) diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 498d25ff2..df2b6506e 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -74,6 +74,9 @@ ### Fixed - Fixed Bun test-runtime detection treating application-owned `NODE_ENV=test` and `BUN_ENV=test` values as test-runner signals ([#7261](https://github.com/can1357/oh-my-pi/issues/7261)). +### Changed + +- Changed stale process-log retention from the newest five files globally to one newest file per completed process and day within the current and previous four local calendar days. This preserves bounded daily diagnostic coverage while continuing to remove one-use audit files. ## [17.2.1] - 2026-07-30 diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index cc7becee1..8ecaf0ed2 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -53,9 +53,11 @@ function emitToSinks(level: LogLevel, message: string, context: Record = []; + const staleLogsByProcessDay = new Map>(); for (const entry of entries) { if (!entry.isFile()) continue; const logMatch = PROCESS_LOG_PATTERN.exec(entry.name); const auditMatch = PROCESS_AUDIT_PATTERN.exec(entry.name); - const pidText = logMatch?.[1] ?? auditMatch?.[1]; + const pidText = logMatch?.[2] ?? auditMatch?.[1]; if (!pidText || processIsRunning(Number(pidText))) continue; const entryPath = path.join(dir, entry.name); if (auditMatch) { + if (RETAINED_STALE_AUDIT_FILES === 0) { + try { + fs.rmSync(entryPath, { force: true }); + } catch { + // Retention is best-effort; logging must still initialize. + } + } + continue; + } + if (!logMatch?.[1]) continue; + if (logMatch[1] < cutoffDate || logMatch[1] > currentDate) { try { fs.rmSync(entryPath, { force: true }); } catch { - // Retention is best-effort; logging must still initialize. + // Another process may have pruned the same stale namespace. } continue; } try { - staleLogs.push({ path: entryPath, mtimeMs: fs.statSync(entryPath).mtimeMs }); + const key = `${pidText}:${logMatch[1]}`; + const staleLogs = staleLogsByProcessDay.get(key) ?? []; + staleLogs.push({ + path: entryPath, + mtimeMs: fs.statSync(entryPath).mtimeMs, + rollover: Number(logMatch[3] ?? 0), + }); + staleLogsByProcessDay.set(key, staleLogs); } catch { // Another process may have pruned the same stale namespace. } } - staleLogs.sort((a, b) => b.mtimeMs - a.mtimeMs); - for (const stale of staleLogs.slice(RETAINED_STALE_LOG_FILES)) { - try { - fs.rmSync(stale.path, { force: true }); - } catch { - // Another process may have pruned the same stale namespace. + for (const staleLogs of staleLogsByProcessDay.values()) { + staleLogs.sort( + (a, b) => b.mtimeMs - a.mtimeMs || b.rollover - a.rollover || (a.path < b.path ? -1 : a.path > b.path ? 1 : 0), + ); + for (const stale of staleLogs.slice(RETAINED_STALE_LOGS_PER_PROCESS_DAY)) { + try { + fs.rmSync(stale.path, { force: true }); + } catch { + // Another process may have pruned the same stale namespace. + } } } } diff --git a/packages/utils/test/logger-multiprocess.test.ts b/packages/utils/test/logger-multiprocess.test.ts index a5259b271..c0c4d9d76 100644 --- a/packages/utils/test/logger-multiprocess.test.ts +++ b/packages/utils/test/logger-multiprocess.test.ts @@ -43,27 +43,63 @@ describe("multiprocess file logging", () => { it("prunes completed PID namespaces across short-lived invocations", async () => { const logsDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-logger-retention-")); roots.push(logsDir); - const exited = Array.from({ length: 7 }, () => - Bun.spawn([process.execPath, "--version"], { stdout: "ignore", stderr: "ignore" }), - ); - expect(await Promise.all(exited.map(proc => proc.exited))).toEqual(Array(7).fill(0)); - - const date = "2026-07-01"; - for (const [index, proc] of exited.entries()) { - const logPath = path.join(logsDir, `omp.${date}.${proc.pid}.log`); - await Bun.write(logPath, `completed process ${proc.pid}`); - await fs.utimes(logPath, index + 1, index + 1); - await Bun.write(path.join(logsDir, `.omp.${proc.pid}-audit.json`), "{}"); - } + // macOS process identifiers are far below these values, so the fixtures + // are deterministically completed rather than briefly lingering as zombies. + const exitedPids = [9_000_001, 9_000_002]; await Bun.write(path.join(logsDir, ".release"), ""); const probePath = await makeProbe(logsDir); - const current = Bun.spawn([process.execPath, probePath], { stdout: "ignore", stderr: "pipe" }); - expect(await current.exited).toBe(0); + const seed = Bun.spawn([process.execPath, probePath], { + stdin: "pipe", + stdout: "ignore", + stderr: "pipe", + }); + seed.stdin.end(); + expect(await seed.exited).toBe(0); + const seedLog = (await fs.readdir(logsDir)).find(name => name.endsWith(`.${seed.pid}.log`)); + const seedDate = seedLog?.match(/^omp\.(\d{4}-\d{2}-\d{2})\./)?.[1]; + if (!seedDate) throw new Error("probe did not create a dated log"); + const baseDate = new Date(`${seedDate}T12:00:00`); + const localDate = (daysAgo: number): string => { + const date = new Date(baseDate); + date.setDate(date.getDate() - daysAgo); + return ( + `${date.getFullYear()}-${String(date.getMonth() + 1).padStart(2, "0")}-` + + String(date.getDate()).padStart(2, "0") + ); + }; + const retainedNames: string[] = []; + const expiredNames: string[] = []; + for (const pid of exitedPids) { + for (let daysAgo = -1; daysAgo <= 5; daysAgo++) { + const name = `omp.${localDate(daysAgo)}.${pid}.log`; + await Bun.write(path.join(logsDir, name), name); + await fs.utimes(path.join(logsDir, name), 2, 2); + (daysAgo > 0 && daysAgo < 5 ? retainedNames : expiredNames).push(name); + } + const rolloverName = `omp.${localDate(0)}.${pid}.log.1`; + await Bun.write(path.join(logsDir, rolloverName), rolloverName); + await fs.utimes(path.join(logsDir, rolloverName), 2, 2); + retainedNames.push(rolloverName); + await Bun.write(path.join(logsDir, `.omp.${pid}-audit.json`), "{}"); + } + + let currentPid = 0; + for (let restart = 0; restart < 2; restart++) { + const current = Bun.spawn([process.execPath, probePath], { + stdin: "pipe", + stdout: "ignore", + stderr: "pipe", + }); + current.stdin.end(); + expect(await current.exited).toBe(0); + currentPid = current.pid; + } const entries = await fs.readdir(logsDir); - const completedLogs = entries.filter(name => name.startsWith(`omp.${date}.`)); - expect(completedLogs).toHaveLength(5); - expect(entries.filter(name => name.endsWith("-audit.json"))).toEqual([`.omp.${current.pid}-audit.json`]); + for (const expected of retainedNames) expect(entries).toContain(expected); + for (const expired of expiredNames) expect(entries).not.toContain(expired); + expect(entries.filter(name => name.endsWith(".log.1"))).toHaveLength(exitedPids.length); + expect(entries.filter(name => name.endsWith("-audit.json"))).toEqual([`.omp.${currentPid}-audit.json`]); }); });