Skip to content

Commit 96fbc83

Browse files
committed
refactor(miner): centralize database initialization with openLocalStoreDb (#8319)
This update refactors the database initialization across multiple modules to utilize the new openLocalStoreDb function. This change centralizes the creation of directories, file permissions, and busy timeout settings, enhancing code maintainability and ensuring crash-safe cleanup registration. The affected files include deny-hook-synthesis.ts, laptop-init.ts, and orb-export.ts, with corresponding tests updated to verify the new behavior. Closes #8319
1 parent 1d83593 commit 96fbc83

6 files changed

Lines changed: 74 additions & 29 deletions

File tree

packages/loopover-miner/lib/deny-hook-synthesis.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
// this module is now a thin wrapper that re-exports those pure helpers and keeps the local SQLite store for
44
// refresh + maintainer review before any synthesized rule takes effect. Approved rules merge with
55
// {@link DEFAULT_DENY_RULES}; unapproved proposals never block tool calls. No behavior change.
6-
import { chmodSync, mkdirSync } from "node:fs";
76
import { homedir } from "node:os";
8-
import { dirname, join } from "node:path";
9-
import { DatabaseSync } from "node:sqlite";
7+
import { join } from "node:path";
8+
import type { DatabaseSync } from "node:sqlite";
109
import {
1110
aggregateBlockerHistory,
1211
canonicalizeChangedPath,
@@ -25,6 +24,7 @@ import type { DenyRuleProposal, SynthesisConfig } from "@loopover/engine";
2524
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";
2625
import type { DenyRule } from "./deny-hooks.js";
2726
import { DENY_HOOK_SYNTHESIS_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js";
27+
import { openLocalStoreDb } from "./local-store.js";
2828

2929
// Re-export the pure synthesis helpers from the engine so this module's public API is unchanged after #5667
3030
// moved derivation/audit into @loopover/engine. Only the SQLite store below (and its forge/db-path helpers) is
@@ -162,10 +162,9 @@ function ensureDenyRuleProposalsForgeScope(db: DatabaseSync): void {
162162
*/
163163
export function initDenyHookSynthesisStore(dbPath: string = resolveDenyHookSynthesisDbPath()): DenyHookSynthesisStore {
164164
const resolvedPath = normalizeDbPath(dbPath);
165-
mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 });
166-
const db = new DatabaseSync(resolvedPath);
167-
chmodSync(resolvedPath, 0o600);
168-
db.exec("PRAGMA busy_timeout = 5000");
165+
// openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and
166+
// treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319).
167+
const db = openLocalStoreDb(resolvedPath);
169168
db.exec(`
170169
CREATE TABLE IF NOT EXISTS deny_rule_proposals (
171170
repo_full_name TEXT NOT NULL,

packages/loopover-miner/lib/laptop-init.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { accessSync, chmodSync, constants, existsSync, mkdirSync } from "node:fs";
1+
import { accessSync, constants, existsSync } from "node:fs";
22
import { homedir } from "node:os";
33
import { delimiter, join } from "node:path";
44
import { DatabaseSync } from "node:sqlite";
55
import { applySchemaMigrations } from "./schema-version.js";
66
import { reportCliFailure } from "./cli-error.js";
77
import { resolveGitHubToken } from "./github-token-resolution.js";
8+
import { openLocalStoreDb } from "./local-store.js";
89

910
const githubApiBaseUrl = "https://api.github.com";
1011
const githubApiVersion = "2022-11-28";
@@ -52,9 +53,11 @@ export function resolveLaptopStateDbPath(env: Record<string, string | undefined>
5253
export function initLaptopState(env: Record<string, string | undefined> = process.env): LaptopInitResult {
5354
const stateDir = resolveMinerStateDir(env);
5455
const dbPath = resolveLaptopStateDbPath(env);
55-
mkdirSync(stateDir, { recursive: true, mode: 0o700 });
56+
// Sample before openLocalStoreDb: the helper creates the parent dir + file, so `created` must be read first.
5657
const created = !existsSync(dbPath);
57-
const db = new DatabaseSync(dbPath);
58+
// openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration
59+
// (#8319) -- this was previously the one store with no busy-timeout and no crash-safety registration at all.
60+
const db = openLocalStoreDb(dbPath);
5861
db.exec(`
5962
CREATE TABLE IF NOT EXISTS laptop_meta (
6063
key TEXT PRIMARY KEY,
@@ -67,7 +70,6 @@ export function initLaptopState(env: Record<string, string | undefined> = proces
6770
db.prepare("INSERT INTO laptop_meta (key, value) VALUES ('initialized_at', ?)")
6871
.run(new Date().toISOString());
6972
}
70-
chmodSync(dbPath, 0o600);
7173
db.close();
7274
return { stateDir, dbPath, created };
7375
}

packages/loopover-miner/lib/orb-export.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
import { chmodSync, mkdirSync } from "node:fs";
21
import { homedir } from "node:os";
3-
import { dirname, join } from "node:path";
4-
import { DatabaseSync } from "node:sqlite";
2+
import { join } from "node:path";
53
import { createHash, createHmac } from "node:crypto";
64
import { generateAnonSecret, hmacAnonymize as engineHmacAnonymize } from "@loopover/engine";
75
import { readPrOutcomes } from "./pr-outcome.js";
86
import type { NormalizedPrOutcomePayload, PrOutcomeLedgerReader } from "./pr-outcome.js";
97
import { initEventLedger } from "./event-ledger.js";
108
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
9+
import { openLocalStoreDb } from "./local-store.js";
1110

1211
// Optional anonymized Orb telemetry export (#4277, network send wired in #5681). The self-host Orb collector
1312
// (src/selfhost/orb-collector.ts, #1255) is ALWAYS-ON for a maintainer's own instance; a miner runs on a
@@ -125,10 +124,9 @@ export function buildAnonymizedOrbBatch(
125124
*/
126125
export function openOrbExportStore(dbPath: string = resolveOrbExportDbPath()): OrbExportStore {
127126
const resolvedPath = normalizeDbPath(dbPath);
128-
mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 });
129-
const db = new DatabaseSync(resolvedPath);
130-
chmodSync(resolvedPath, 0o600);
131-
db.exec("PRAGMA busy_timeout = 5000");
127+
// openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and
128+
// treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319).
129+
const db = openLocalStoreDb(resolvedPath);
132130
db.exec(`CREATE TABLE IF NOT EXISTS orb_export_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`);
133131

134132
const getStatement = db.prepare("SELECT value FROM orb_export_meta WHERE key = ?");

test/unit/miner-deny-hook-synthesis.test.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,17 @@ import {
77
DEFAULT_DENY_RULES,
88
evaluateDenyHooks,
99
} from "../../packages/loopover-miner/lib/deny-hooks.js";
10-
import {
10+
import type { DenyRuleProposal } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
11+
// #7525: normalizeRepoFullName is defined in the engine and re-exported unchanged by the miner-lib module
12+
// above; import it from the engine source directly so the guard's src branches are the ones exercised.
13+
import { normalizeRepoFullName } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
14+
import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js";
15+
16+
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced
17+
// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the
18+
// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796).
19+
const DENY_HOOK_SYNTHESIS_MODULE = "../../packages/loopover-miner/lib/deny-hook-synthesis.ts";
20+
const {
1121
aggregateBlockerHistory,
1222
changedPathToDenyGlob,
1323
initDenyHookSynthesisStore,
@@ -16,11 +26,7 @@ import {
1626
resolveEffectiveDenyRules,
1727
setProposalStatuses,
1828
synthesizeDenyRuleProposals,
19-
} from "../../packages/loopover-miner/lib/deny-hook-synthesis.js";
20-
import type { DenyRuleProposal } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
21-
// #7525: normalizeRepoFullName is defined in the engine and re-exported unchanged by the miner-lib module
22-
// above; import it from the engine source directly so the guard's src branches are the ones exercised.
23-
import { normalizeRepoFullName } from "../../packages/loopover-engine/src/miner/deny-hook-synthesis";
29+
} = (await import(DENY_HOOK_SYNTHESIS_MODULE)) as typeof import("../../packages/loopover-miner/lib/deny-hook-synthesis.js");
2430

2531
const tempDirs: string[] = [];
2632
const stores: Array<{ close(): void }> = [];
@@ -163,6 +169,16 @@ describe("initDenyHookSynthesisStore() (#4522)", () => {
163169
expect(() => initDenyHookSynthesisStore(" ")).toThrow("invalid_deny_hook_synthesis_db_path");
164170
});
165171

172+
it("registers the store for crash-safe cleanup via openLocalStoreDb, and unregisters it on close (#8319)", () => {
173+
resetProcessLifecycleForTesting();
174+
expect(cleanupResourceCount()).toBe(0);
175+
const store = tempStore();
176+
expect(cleanupResourceCount()).toBe(1);
177+
stores.splice(stores.indexOf(store), 1);
178+
store.close();
179+
expect(cleanupResourceCount()).toBe(0);
180+
});
181+
166182
it("skips the forge-scope migration on a second open of an already-migrated file", () => {
167183
const dir = mkdtempSync(join(tmpdir(), "miner-deny-hook-synthesis-remigrate-"));
168184
tempDirs.push(dir);

test/unit/miner-laptop-init.test.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,19 @@ vi.mock("node:sqlite", async (importOriginal) => {
2929
}
3030
return { ...actual, DatabaseSync: RecordingDatabaseSync };
3131
});
32-
import {
32+
import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js";
33+
34+
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced
35+
// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the
36+
// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796).
37+
const LAPTOP_INIT_MODULE = "../../packages/loopover-miner/lib/laptop-init.ts";
38+
const {
3339
checkDockerPresent,
3440
checkLaptopStateSqlite,
3541
initLaptopState,
3642
resolveLaptopStateDbPath,
3743
runInit,
38-
} from "../../packages/loopover-miner/lib/laptop-init.js";
44+
} = (await import(LAPTOP_INIT_MODULE)) as typeof import("../../packages/loopover-miner/lib/laptop-init.js");
3945

4046
const roots: string[] = [];
4147

@@ -83,6 +89,16 @@ describe("loopover-miner laptop init (#2329)", () => {
8389
expect(readFileSync(join(first.stateDir, "marker.txt"), "utf8")).toBe("keep-me");
8490
});
8591

92+
it("opens its store via openLocalStoreDb, registering and unregistering it for crash-safe cleanup within the call (#8319)", () => {
93+
resetProcessLifecycleForTesting();
94+
expect(cleanupResourceCount()).toBe(0);
95+
const root = tempRoot();
96+
initLaptopState({ LOOPOVER_MINER_CONFIG_DIR: join(root, "state") });
97+
// initLaptopState closes its own handle internally before returning (it exposes no db handle), so a
98+
// leftover registration here would mean the register→unregister cycle didn't complete cleanly.
99+
expect(cleanupResourceCount()).toBe(0);
100+
});
101+
86102
it("runInit prints human text (0) and machine JSON with --json", async () => {
87103
const root = tempRoot();
88104
const env = { LOOPOVER_MINER_CONFIG_DIR: join(root, "state") };

test/unit/miner-orb-export.test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@ import { mkdtempSync, rmSync } from "node:fs";
22
import { tmpdir } from "node:os";
33
import { join } from "node:path";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import type { OrbExportOutcome, OrbExportRow } from "../../packages/loopover-miner/lib/orb-export.js";
6+
import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js";
57

6-
import {
8+
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced
9+
// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the
10+
// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796).
11+
const ORB_EXPORT_MODULE = "../../packages/loopover-miner/lib/orb-export.ts";
12+
const {
713
ORB_EXPORT_ENABLED_BY_DEFAULT,
814
DEFAULT_AMS_COLLECTOR_URL,
915
DEFAULT_ORB_EXPORT_TIMEOUT_MS,
@@ -17,8 +23,7 @@ import {
1723
resolveAmsCollectorUrl,
1824
resolveOrbExportDbPath,
1925
sendAmsExportBatch,
20-
} from "../../packages/loopover-miner/lib/orb-export.js";
21-
import type { OrbExportOutcome, OrbExportRow } from "../../packages/loopover-miner/lib/orb-export.js";
26+
} = (await import(ORB_EXPORT_MODULE)) as typeof import("../../packages/loopover-miner/lib/orb-export.js");
2227

2328
let dir: string;
2429
function storePath() {
@@ -68,6 +73,15 @@ describe("orb-export store (#4277)", () => {
6873
expect(store.getCursor()).toBe("2026-01-02T00:00:00Z");
6974
store.close();
7075
});
76+
77+
it("registers the store for crash-safe cleanup via openLocalStoreDb, and unregisters it on close (#8319)", () => {
78+
resetProcessLifecycleForTesting();
79+
expect(cleanupResourceCount()).toBe(0);
80+
const store = openOrbExportStore(storePath());
81+
expect(cleanupResourceCount()).toBe(1);
82+
store.close();
83+
expect(cleanupResourceCount()).toBe(0);
84+
});
7185
});
7286

7387
describe("hmacAnonymize", () => {

0 commit comments

Comments
 (0)