diff --git a/packages/loopover-miner/lib/deny-hook-synthesis.ts b/packages/loopover-miner/lib/deny-hook-synthesis.ts index 8fa520540..a9b330de9 100644 --- a/packages/loopover-miner/lib/deny-hook-synthesis.ts +++ b/packages/loopover-miner/lib/deny-hook-synthesis.ts @@ -3,9 +3,7 @@ // this module is now a thin wrapper that re-exports those pure helpers and keeps the local SQLite store for // refresh + maintainer review before any synthesized rule takes effect. Approved rules merge with // {@link DEFAULT_DENY_RULES}; unapproved proposals never block tool calls. No behavior change. -import { chmodSync, mkdirSync } from "node:fs"; -import { dirname } from "node:path"; -import { DatabaseSync } from "node:sqlite"; +import type { DatabaseSync } from "node:sqlite"; import { aggregateBlockerHistory, canonicalizeChangedPath, @@ -23,7 +21,7 @@ import { import type { DenyRuleProposal, SynthesisConfig } from "@loopover/engine"; import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import type { DenyRule } from "./deny-hooks.js"; -import { resolveLocalStoreDbPath } from "./local-store.js"; +import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; import { DENY_HOOK_SYNTHESIS_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js"; // Re-export the pure synthesis helpers from the engine so this module's public API is unchanged after #5667 @@ -149,10 +147,9 @@ function ensureDenyRuleProposalsForgeScope(db: DatabaseSync): void { */ export function initDenyHookSynthesisStore(dbPath: string = resolveDenyHookSynthesisDbPath()): DenyHookSynthesisStore { const resolvedPath = normalizeDbPath(dbPath); - mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 }); - const db = new DatabaseSync(resolvedPath); - chmodSync(resolvedPath, 0o600); - db.exec("PRAGMA busy_timeout = 5000"); + // openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and + // treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319). + const db = openLocalStoreDb(resolvedPath); db.exec(` CREATE TABLE IF NOT EXISTS deny_rule_proposals ( repo_full_name TEXT NOT NULL, diff --git a/packages/loopover-miner/lib/laptop-init.ts b/packages/loopover-miner/lib/laptop-init.ts index dae99ab47..e604b09df 100644 --- a/packages/loopover-miner/lib/laptop-init.ts +++ b/packages/loopover-miner/lib/laptop-init.ts @@ -1,11 +1,11 @@ -import { accessSync, chmodSync, constants, existsSync, mkdirSync } from "node:fs"; +import { accessSync, constants, existsSync } from "node:fs"; import { homedir } from "node:os"; import { delimiter, join } from "node:path"; import { DatabaseSync } from "node:sqlite"; import { applySchemaMigrations } from "./schema-version.js"; import { reportCliFailure } from "./cli-error.js"; import { resolveGitHubToken } from "./github-token-resolution.js"; -import { resolveLocalStoreDbPath } from "./local-store.js"; +import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; const githubApiBaseUrl = "https://api.github.com"; const githubApiVersion = "2022-11-28"; @@ -53,9 +53,11 @@ export function resolveLaptopStateDbPath(env: Record export function initLaptopState(env: Record = process.env): LaptopInitResult { const stateDir = resolveMinerStateDir(env); const dbPath = resolveLaptopStateDbPath(env); - mkdirSync(stateDir, { recursive: true, mode: 0o700 }); + // Sample before openLocalStoreDb: the helper creates the parent dir + file, so `created` must be read first. const created = !existsSync(dbPath); - const db = new DatabaseSync(dbPath); + // openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration + // (#8319) -- this was previously the one store with no busy-timeout and no crash-safety registration at all. + const db = openLocalStoreDb(dbPath); db.exec(` CREATE TABLE IF NOT EXISTS laptop_meta ( key TEXT PRIMARY KEY, @@ -68,7 +70,6 @@ export function initLaptopState(env: Record = proces db.prepare("INSERT INTO laptop_meta (key, value) VALUES ('initialized_at', ?)") .run(new Date().toISOString()); } - chmodSync(dbPath, 0o600); db.close(); return { stateDir, dbPath, created }; } diff --git a/packages/loopover-miner/lib/orb-export.ts b/packages/loopover-miner/lib/orb-export.ts index 98138c164..bb388917e 100644 --- a/packages/loopover-miner/lib/orb-export.ts +++ b/packages/loopover-miner/lib/orb-export.ts @@ -1,13 +1,10 @@ -import { chmodSync, mkdirSync } from "node:fs"; -import { dirname } from "node:path"; -import { DatabaseSync } from "node:sqlite"; import { createHash, createHmac } from "node:crypto"; import { generateAnonSecret, hmacAnonymize as engineHmacAnonymize } from "@loopover/engine"; import { readPrOutcomes } from "./pr-outcome.js"; import type { NormalizedPrOutcomePayload, PrOutcomeLedgerReader } from "./pr-outcome.js"; import { initEventLedger } from "./event-ledger.js"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; -import { resolveLocalStoreDbPath } from "./local-store.js"; +import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; // Optional anonymized Orb telemetry export (#4277, network send wired in #5681). The self-host Orb collector // (src/selfhost/orb-collector.ts, #1255) is ALWAYS-ON for a maintainer's own instance; a miner runs on a @@ -113,10 +110,9 @@ export function buildAnonymizedOrbBatch( */ export function openOrbExportStore(dbPath: string = resolveOrbExportDbPath()): OrbExportStore { const resolvedPath = normalizeDbPath(dbPath); - mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 }); - const db = new DatabaseSync(resolvedPath); - chmodSync(resolvedPath, 0o600); - db.exec("PRAGMA busy_timeout = 5000"); + // openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and + // treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319). + const db = openLocalStoreDb(resolvedPath); db.exec(`CREATE TABLE IF NOT EXISTS orb_export_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`); const getStatement = db.prepare("SELECT value FROM orb_export_meta WHERE key = ?"); diff --git a/test/unit/miner-deny-hook-synthesis.test.ts b/test/unit/miner-deny-hook-synthesis.test.ts index 26f443285..a3debc147 100644 --- a/test/unit/miner-deny-hook-synthesis.test.ts +++ b/test/unit/miner-deny-hook-synthesis.test.ts @@ -7,7 +7,18 @@ import { DEFAULT_DENY_RULES, evaluateDenyHooks, } from "../../packages/loopover-miner/lib/deny-hooks.js"; -import { +import type { DenyRuleProposal } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis"; +// #7525: normalizeRepoFullName is defined in the engine and re-exported unchanged by the miner-lib module +// above; import it from the engine source directly so the guard's src branches are the ones exercised. +import { normalizeRepoFullName } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis"; +import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js"; +import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js"; + +// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced +// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the +// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796). +const DENY_HOOK_SYNTHESIS_MODULE = "../../packages/loopover-miner/lib/deny-hook-synthesis.ts"; +const { aggregateBlockerHistory, changedPathToDenyGlob, initDenyHookSynthesisStore, @@ -16,12 +27,7 @@ import { resolveEffectiveDenyRules, setProposalStatuses, synthesizeDenyRuleProposals, -} from "../../packages/loopover-miner/lib/deny-hook-synthesis.js"; -import type { DenyRuleProposal } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis"; -// #7525: normalizeRepoFullName is defined in the engine and re-exported unchanged by the miner-lib module -// above; import it from the engine source directly so the guard's src branches are the ones exercised. -import { normalizeRepoFullName } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis"; -import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js"; +} = (await import(DENY_HOOK_SYNTHESIS_MODULE)) as typeof import("../../packages/loopover-miner/lib/deny-hook-synthesis.js"); const tempDirs: string[] = []; const stores: Array<{ close(): void }> = []; @@ -180,6 +186,16 @@ describe("initDenyHookSynthesisStore() (#4522)", () => { expect(() => initDenyHookSynthesisStore(" ")).toThrow("invalid_deny_hook_synthesis_db_path"); }); + it("registers the store for crash-safe cleanup via openLocalStoreDb, and unregisters it on close (#8319)", () => { + resetProcessLifecycleForTesting(); + expect(cleanupResourceCount()).toBe(0); + const store = tempStore(); + expect(cleanupResourceCount()).toBe(1); + stores.splice(stores.indexOf(store), 1); + store.close(); + expect(cleanupResourceCount()).toBe(0); + }); + it("skips the forge-scope migration on a second open of an already-migrated file", () => { const dir = mkdtempSync(join(tmpdir(), "miner-deny-hook-synthesis-remigrate-")); tempDirs.push(dir); diff --git a/test/unit/miner-laptop-init.test.ts b/test/unit/miner-laptop-init.test.ts index e769c9cb0..3318979ea 100644 --- a/test/unit/miner-laptop-init.test.ts +++ b/test/unit/miner-laptop-init.test.ts @@ -29,13 +29,19 @@ vi.mock("node:sqlite", async (importOriginal) => { } return { ...actual, DatabaseSync: RecordingDatabaseSync }; }); -import { +import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js"; + +// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced +// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the +// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796). +const LAPTOP_INIT_MODULE = "../../packages/loopover-miner/lib/laptop-init.ts"; +const { checkDockerPresent, checkLaptopStateSqlite, initLaptopState, resolveLaptopStateDbPath, runInit, -} from "../../packages/loopover-miner/lib/laptop-init.js"; +} = (await import(LAPTOP_INIT_MODULE)) as typeof import("../../packages/loopover-miner/lib/laptop-init.js"); const roots: string[] = []; @@ -91,6 +97,16 @@ describe("loopover-miner laptop init (#2329)", () => { expect(readFileSync(join(first.stateDir, "marker.txt"), "utf8")).toBe("keep-me"); }); + it("opens its store via openLocalStoreDb, registering and unregistering it for crash-safe cleanup within the call (#8319)", () => { + resetProcessLifecycleForTesting(); + expect(cleanupResourceCount()).toBe(0); + const root = tempRoot(); + initLaptopState({ LOOPOVER_MINER_CONFIG_DIR: join(root, "state") }); + // initLaptopState closes its own handle internally before returning (it exposes no db handle), so a + // leftover registration here would mean the register→unregister cycle didn't complete cleanly. + expect(cleanupResourceCount()).toBe(0); + }); + it("runInit prints human text (0) and machine JSON with --json", async () => { const root = tempRoot(); const env = { LOOPOVER_MINER_CONFIG_DIR: join(root, "state") }; diff --git a/test/unit/miner-orb-export.test.ts b/test/unit/miner-orb-export.test.ts index 08dcc7603..51942effb 100644 --- a/test/unit/miner-orb-export.test.ts +++ b/test/unit/miner-orb-export.test.ts @@ -2,8 +2,15 @@ import { mkdtempSync, rmSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OrbExportOutcome, OrbExportRow } from "../../packages/loopover-miner/lib/orb-export.js"; +import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js"; +import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js"; -import { +// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced +// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the +// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796). +const ORB_EXPORT_MODULE = "../../packages/loopover-miner/lib/orb-export.ts"; +const { ORB_EXPORT_ENABLED_BY_DEFAULT, DEFAULT_AMS_COLLECTOR_URL, DEFAULT_ORB_EXPORT_TIMEOUT_MS, @@ -17,9 +24,7 @@ import { resolveAmsCollectorUrl, resolveOrbExportDbPath, sendAmsExportBatch, -} from "../../packages/loopover-miner/lib/orb-export.js"; -import type { OrbExportOutcome, OrbExportRow } from "../../packages/loopover-miner/lib/orb-export.js"; -import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js"; +} = (await import(ORB_EXPORT_MODULE)) as typeof import("../../packages/loopover-miner/lib/orb-export.js"); let dir: string; function storePath() { @@ -69,6 +74,15 @@ describe("orb-export store (#4277)", () => { expect(store.getCursor()).toBe("2026-01-02T00:00:00Z"); store.close(); }); + + it("registers the store for crash-safe cleanup via openLocalStoreDb, and unregisters it on close (#8319)", () => { + resetProcessLifecycleForTesting(); + expect(cleanupResourceCount()).toBe(0); + const store = openOrbExportStore(storePath()); + expect(cleanupResourceCount()).toBe(1); + store.close(); + expect(cleanupResourceCount()).toBe(0); + }); }); describe("hmacAnonymize", () => {