From 858598a2009fbb56aaf6dbc163c8591ded944ef8 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sun, 12 Jul 2026 02:00:02 -0300 Subject: [PATCH] fix(db): stop legacy log-archive migration from deleting the live app-logger directory and crashing startup on a stat/stream race (#6401) (#6898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit archiveLegacyRequestLogs() swept the entire DATA_DIR/logs directory as a single "legacy" target and recursively deleted it after zipping. Since PR #6234 moved the default app-log path to DATA_DIR/logs/application, the migration was deleting the live file logger's own directory on every boot until its marker file existed (#6799). Separately, yazl's addFile() does an internal stat-then-stream read; if a target file grows between those two steps (e.g. an actively-written log), yazl emits "error" directly on the ZipFile instance. That event had no listener, so Node re-threw it as an uncaughtException that crashed the whole process at startup (#6401) — misdiagnosed upstream as Turbopack/Windows chunk corruption because the stack trace pointed into a bundled chunk. Fix: - listArchiveTargets() now enumerates DATA_DIR/logs entries individually and skips the live app-logger directory (resolved via logEnv.getAppLogFilePath()), so the shared parent directory is never deleted wholesale. - createLegacyArchive() wires a zipFile.on("error", ...) handler so a stat/stream race rejects the promise (caught by the existing try/catch) instead of escaping as an uncaughtException. Regression test: tests/unit/usage-migrations-legacy-archive-safety.test.ts (RED on unfixed code, GREEN after fix). Updated tests/unit/request-log-migration.test.ts to the corrected contract — DATA_DIR/logs itself now survives the archive sweep. Gates run clean: file-size, complexity, cognitive-complexity, changelog-integrity, typecheck:core, eslint (suppressions), and the existing usage-migrations/request-log-migration unit suites. --- .../fixes/6401-6401-usage-migrations.md | 1 + src/lib/usage/migrations.ts | 61 ++++++-- tests/unit/request-log-migration.test.ts | 6 +- ...e-migrations-legacy-archive-safety.test.ts | 141 ++++++++++++++++++ 4 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 changelog.d/fixes/6401-6401-usage-migrations.md create mode 100644 tests/unit/usage-migrations-legacy-archive-safety.test.ts diff --git a/changelog.d/fixes/6401-6401-usage-migrations.md b/changelog.d/fixes/6401-6401-usage-migrations.md new file mode 100644 index 0000000000..3d623f9dc6 --- /dev/null +++ b/changelog.d/fixes/6401-6401-usage-migrations.md @@ -0,0 +1 @@ +- fix(db): stop legacy log-archive migration from deleting the live app-logger directory and crashing startup on a stat/stream race (#6401, #6799) diff --git a/src/lib/usage/migrations.ts b/src/lib/usage/migrations.ts index 10c8d6ede0..df41bb2be5 100644 --- a/src/lib/usage/migrations.ts +++ b/src/lib/usage/migrations.ts @@ -13,6 +13,7 @@ import path from "path"; import { ZipFile } from "yazl"; import { getDbInstance, isCloud, isBuildPhase, DATA_DIR } from "../db/core"; import { getLegacyDotDataDir, isSamePath } from "../dataPaths"; +import { getAppLogFilePath } from "../logEnv"; import { protectPayloadForLog } from "../logPayloads"; import { sanitizePII } from "../piiSanitizer"; import { writeCallArtifact, type CallLogArtifact } from "./callLogArtifacts"; @@ -110,17 +111,45 @@ function ensureArchiveDir() { fs.mkdirSync(LOG_ARCHIVES_DIR, { recursive: true }); } -function listArchiveTargets(): ArchiveTarget[] { +/** + * Directory the live file logger writes to (`buildLogger()` in + * `src/shared/utils/logger.ts`), so the legacy-log sweep below never zips-then-deletes + * a directory it does not own. Since PR #6234 (fix for #6197) the default app log path + * moved from a `process.cwd()`-anchored location to `DATA_DIR/logs/application/app.log`, + * landing it inside the same `DATA_DIR/logs` tree this "legacy" migration sweeps (#6799). + */ +function getLiveAppLogDir(): string | null { + try { + return path.dirname(getAppLogFilePath()); + } catch { + return null; + } +} + +function listRequestLogArchiveEntries(): ArchiveTarget[] { + if (!CURRENT_REQUEST_LOGS_DIR || !fs.existsSync(CURRENT_REQUEST_LOGS_DIR)) return []; + + const liveAppLogDir = getLiveAppLogDir(); + const entries = fs.readdirSync(CURRENT_REQUEST_LOGS_DIR); const targets: ArchiveTarget[] = []; - if (CURRENT_REQUEST_LOGS_DIR && fs.existsSync(CURRENT_REQUEST_LOGS_DIR)) { + for (const entry of entries) { + const entryPath = path.join(CURRENT_REQUEST_LOGS_DIR, entry); + if (liveAppLogDir && isSamePath(entryPath, liveAppLogDir)) continue; + targets.push({ - sourcePath: CURRENT_REQUEST_LOGS_DIR, - archiveRoot: "data/logs", + sourcePath: entryPath, + archiveRoot: path.posix.join("data/logs", entry), deleteAfterArchive: true, }); } + return targets; +} + +function listArchiveTargets(): ArchiveTarget[] { + const targets: ArchiveTarget[] = listRequestLogArchiveEntries(); + if (CURRENT_REQUEST_SUMMARY_FILE && fs.existsSync(CURRENT_REQUEST_SUMMARY_FILE)) { targets.push({ sourcePath: CURRENT_REQUEST_SUMMARY_FILE, @@ -173,11 +202,27 @@ function createLegacyArchive(targets: ArchiveTarget[]): Promise { const zipFile = new ZipFile(); const output = fs.createWriteStream(archivePath); - output.on("close", () => resolve(archiveFilename)); - output.on("error", (error) => { + let settled = false; + // yazl detects a stat->stream size mismatch (a file growing while being zipped, e.g. + // an actively-written log — #6401) by emitting "error" on the ZipFile instance itself, + // not on `output`. Left unwired, that "error" event has no listener and Node re-throws + // it as an uncaughtException that crashes the process. Wiring it here converts that + // crash into a normal rejection the caller's try/catch already handles. + const fail = (error: Error) => { + if (settled) return; + settled = true; + output.destroy(); fs.rmSync(archivePath, { force: true }); reject(error); + }; + + output.on("close", () => { + if (settled) return; + settled = true; + resolve(archiveFilename); }); + output.on("error", fail); + zipFile.on("error", fail); zipFile.outputStream.pipe(output); try { @@ -186,9 +231,7 @@ function createLegacyArchive(targets: ArchiveTarget[]): Promise { } zipFile.end(); } catch (error) { - fs.rmSync(archivePath, { force: true }); - zipFile.end(); - reject(error); + fail(error as Error); } }); } diff --git a/tests/unit/request-log-migration.test.ts b/tests/unit/request-log-migration.test.ts index c35be628a1..00178b9fc1 100644 --- a/tests/unit/request-log-migration.test.ts +++ b/tests/unit/request-log-migration.test.ts @@ -61,7 +61,11 @@ test("archives legacy request log layout into a zip and removes old files", asyn const archiveFilename = await migrations.archiveLegacyRequestLogs(); assert.match(archiveFilename || "", /_legacy-request-logs\.zip$/); - assert.equal(fs.existsSync(LEGACY_LOGS_DIR), false); + // DATA_DIR/logs itself is preserved (not recursively removed) because it is shared + // with the live app logger's own logs/application subdirectory since #6234 — only the + // individual legacy entries inside it are archived-then-deleted (#6799). + assert.equal(fs.existsSync(LEGACY_LOGS_DIR), true); + assert.equal(fs.existsSync(path.join(LEGACY_LOGS_DIR, "session-a")), false); assert.equal(fs.existsSync(LEGACY_CALL_LOGS_DIR), false); assert.equal(fs.existsSync(LEGACY_SUMMARY_FILE), false); assert.equal(fs.existsSync(MARKER_PATH), true); diff --git a/tests/unit/usage-migrations-legacy-archive-safety.test.ts b/tests/unit/usage-migrations-legacy-archive-safety.test.ts new file mode 100644 index 0000000000..8e5eeb7465 --- /dev/null +++ b/tests/unit/usage-migrations-legacy-archive-safety.test.ts @@ -0,0 +1,141 @@ +// Regression tests for #6401 / #6799. +// +// #6799: archiveLegacyRequestLogs() treated the ENTIRE DATA_DIR/logs directory as a +// single "legacy request log" archive target and recursively deleted it after zipping — +// including DATA_DIR/logs/application/, which is the live file-logger's own directory +// since PR #6234 moved the default APP_LOG_FILE_PATH to DATA_DIR/logs/application/app.log. +// +// #6401: yazl's addFile() does an internal stat()-then-stream-read (TOCTOU). Because the +// live logger keeps appending to files under the swept directory while the migration is +// zipping them, the file can grow between yazl's stat and its later read, and yazl throws +// "file data stream has unexpected number of bytes" as an error event on a stream that +// was never wired into archiveLegacyRequestLogs()'s own try/catch — surfacing as an +// uncaught exception that crashes the process at boot. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const ORIGINAL_HOME = process.env.HOME; +const ORIGINAL_USERPROFILE = process.env.USERPROFILE; +const ORIGINAL_DATA_DIR = process.env.DATA_DIR; +const ORIGINAL_NEXT_PHASE = process.env.NEXT_PHASE; + +const TEST_HOME_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-6401-6799-home-")); +const TEST_DATA_DIR = path.join(TEST_HOME_DIR, "data"); + +process.env.HOME = TEST_HOME_DIR; +process.env.USERPROFILE = TEST_HOME_DIR; +process.env.DATA_DIR = TEST_DATA_DIR; +delete process.env.NEXT_PHASE; + +const migrations = await import("../../src/lib/usage/migrations.ts"); +const { getDbInstance, resetDbInstance } = await import("../../src/lib/db/core.ts"); + +const APP_LOG_DIR = path.join(TEST_DATA_DIR, "logs", "application"); +const APP_LOG_FILE = path.join(APP_LOG_DIR, "app.log"); +const CURRENT_REQUEST_LOGS_DIR = path.join(TEST_DATA_DIR, "logs"); + +test.before(() => { + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +}); + +test.after(() => { + try { + const db = getDbInstance(); + if (db?.open) db.close(); + } catch { + // Database may already be closed. + } + resetDbInstance?.(); + + if (ORIGINAL_HOME === undefined) delete process.env.HOME; + else process.env.HOME = ORIGINAL_HOME; + if (ORIGINAL_USERPROFILE === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = ORIGINAL_USERPROFILE; + if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = ORIGINAL_DATA_DIR; + if (ORIGINAL_NEXT_PHASE === undefined) delete process.env.NEXT_PHASE; + else process.env.NEXT_PHASE = ORIGINAL_NEXT_PHASE; + + fs.rmSync(TEST_HOME_DIR, { recursive: true, force: true }); +}); + +test("#6799: archiveLegacyRequestLogs() must not delete the live app-logger directory (DATA_DIR/logs/application)", async () => { + fs.mkdirSync(APP_LOG_DIR, { recursive: true }); + fs.writeFileSync(APP_LOG_FILE, '{"level":30,"msg":"server starting"}\n'); + + assert.equal( + fs.existsSync(APP_LOG_FILE), + true, + "precondition: live app.log exists before migration runs" + ); + + await migrations.archiveLegacyRequestLogs(); + + assert.equal( + fs.existsSync(APP_LOG_FILE), + true, + "archiveLegacyRequestLogs() deleted the live app.log file — DATA_DIR/logs/application must be excluded from the legacy archive sweep" + ); + assert.equal( + fs.existsSync(CURRENT_REQUEST_LOGS_DIR), + true, + "archiveLegacyRequestLogs() deleted the entire DATA_DIR/logs directory, including the live application log subtree" + ); +}); + +test("#6401: archiveLegacyRequestLogs does not crash the process when a legacy target is being actively appended to (yazl TOCTOU)", async (t) => { + const legacyLogFile = path.join(CURRENT_REQUEST_LOGS_DIR, "legacy-request.log"); + fs.mkdirSync(CURRENT_REQUEST_LOGS_DIR, { recursive: true }); + fs.writeFileSync(legacyLogFile, "x".repeat(64 * 1024)); + + // Clear the marker written by the previous test so this run re-enters the archive path. + const markerPath = path.join(TEST_DATA_DIR, "log_archives", "legacy-request-logs.json"); + fs.rmSync(markerPath, { force: true }); + + let writing = true; + const writer = setInterval(() => { + if (!writing) return; + try { + fs.appendFileSync(legacyLogFile, "y".repeat(4096)); + } catch { + /* ignore */ + } + }, 2); + writer.unref(); + + let uncaught: unknown = null; + const onUncaught = (err: unknown) => { + uncaught = err; + }; + process.on("uncaughtException", onUncaught); + + t.after(() => { + writing = false; + clearInterval(writer); + process.off("uncaughtException", onUncaught); + }); + + let rejected: unknown = null; + try { + await migrations.archiveLegacyRequestLogs(); + } catch (err) { + rejected = err; + } + + await new Promise((r) => setTimeout(r, 800)); + + writing = false; + clearInterval(writer); + + assert.equal( + uncaught, + null, + `archiveLegacyRequestLogs() let an uncaughtException escape instead of handling/rejecting it: ${String(uncaught)}` + ); + // The function itself already has an outer try/catch at call sites; here we assert it + // resolves or rejects cleanly rather than crashing the process via uncaughtException. + void rejected; +});