Skip to content

Commit 88321cd

Browse files
fix(miner): route orb-export, deny-hook-synthesis, and laptop-init through openLocalStoreDb (#8319)
Replace hand-rolled DatabaseSync open sequences with openLocalStoreDb so all three stores get crash-safe cleanup registration. Tests import .ts via variable specifier so CI's pre-test miner build does not leave codecov/patch at 0% on the .ts paths. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent ee4fa21 commit 88321cd

6 files changed

Lines changed: 74 additions & 34 deletions

File tree

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
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";
7-
import { dirname } from "node:path";
8-
import { DatabaseSync } from "node:sqlite";
6+
import type { DatabaseSync } from "node:sqlite";
97
import {
108
aggregateBlockerHistory,
119
canonicalizeChangedPath,
@@ -23,7 +21,7 @@ import {
2321
import type { DenyRuleProposal, SynthesisConfig } from "@loopover/engine";
2422
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";
2523
import type { DenyRule } from "./deny-hooks.js";
26-
import { resolveLocalStoreDbPath } from "./local-store.js";
24+
import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js";
2725
import { DENY_HOOK_SYNTHESIS_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js";
2826

2927
// 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 {
149147
*/
150148
export function initDenyHookSynthesisStore(dbPath: string = resolveDenyHookSynthesisDbPath()): DenyHookSynthesisStore {
151149
const resolvedPath = normalizeDbPath(dbPath);
152-
mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 });
153-
const db = new DatabaseSync(resolvedPath);
154-
chmodSync(resolvedPath, 0o600);
155-
db.exec("PRAGMA busy_timeout = 5000");
150+
// openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and
151+
// treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319).
152+
const db = openLocalStoreDb(resolvedPath);
156153
db.exec(`
157154
CREATE TABLE IF NOT EXISTS deny_rule_proposals (
158155
repo_full_name TEXT NOT NULL,

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +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 { resolveLocalStoreDbPath } from "./local-store.js";
8+
import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js";
99

1010
const githubApiBaseUrl = "https://api.github.com";
1111
const githubApiVersion = "2022-11-28";
@@ -53,9 +53,11 @@ export function resolveLaptopStateDbPath(env: Record<string, string | undefined>
5353
export function initLaptopState(env: Record<string, string | undefined> = process.env): LaptopInitResult {
5454
const stateDir = resolveMinerStateDir(env);
5555
const dbPath = resolveLaptopStateDbPath(env);
56-
mkdirSync(stateDir, { recursive: true, mode: 0o700 });
56+
// Sample before openLocalStoreDb: the helper creates the parent dir + file, so `created` must be read first.
5757
const created = !existsSync(dbPath);
58-
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);
5961
db.exec(`
6062
CREATE TABLE IF NOT EXISTS laptop_meta (
6163
key TEXT PRIMARY KEY,
@@ -68,7 +70,6 @@ export function initLaptopState(env: Record<string, string | undefined> = proces
6870
db.prepare("INSERT INTO laptop_meta (key, value) VALUES ('initialized_at', ?)")
6971
.run(new Date().toISOString());
7072
}
71-
chmodSync(dbPath, 0o600);
7273
db.close();
7374
return { stateDir, dbPath, created };
7475
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
1-
import { chmodSync, mkdirSync } from "node:fs";
2-
import { dirname } from "node:path";
3-
import { DatabaseSync } from "node:sqlite";
41
import { createHash, createHmac } from "node:crypto";
52
import { generateAnonSecret, hmacAnonymize as engineHmacAnonymize } from "@loopover/engine";
63
import { readPrOutcomes } from "./pr-outcome.js";
74
import type { NormalizedPrOutcomePayload, PrOutcomeLedgerReader } from "./pr-outcome.js";
85
import { initEventLedger } from "./event-ledger.js";
96
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
10-
import { resolveLocalStoreDbPath } from "./local-store.js";
7+
import { openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js";
118

129
// Optional anonymized Orb telemetry export (#4277, network send wired in #5681). The self-host Orb collector
1310
// (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(
113110
*/
114111
export function openOrbExportStore(dbPath: string = resolveOrbExportDbPath()): OrbExportStore {
115112
const resolvedPath = normalizeDbPath(dbPath);
116-
mkdirSync(dirname(resolvedPath), { recursive: true, mode: 0o700 });
117-
const db = new DatabaseSync(resolvedPath);
118-
chmodSync(resolvedPath, 0o600);
119-
db.exec("PRAGMA busy_timeout = 5000");
113+
// openLocalStoreDb centralizes the mkdir(0o700)/chmod(0o600)/busy_timeout + crash-safe cleanup registration and
114+
// treats ':memory:' as a no-file special case, so this store no longer hand-rolls that boilerplate (#8319).
115+
const db = openLocalStoreDb(resolvedPath);
120116
db.exec(`CREATE TABLE IF NOT EXISTS orb_export_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`);
121117

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

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,18 @@ 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 { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js";
15+
import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js";
16+
17+
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced
18+
// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the
19+
// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796).
20+
const DENY_HOOK_SYNTHESIS_MODULE = "../../packages/loopover-miner/lib/deny-hook-synthesis.ts";
21+
const {
1122
aggregateBlockerHistory,
1223
changedPathToDenyGlob,
1324
initDenyHookSynthesisStore,
@@ -16,12 +27,7 @@ import {
1627
resolveEffectiveDenyRules,
1728
setProposalStatuses,
1829
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";
24-
import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js";
30+
} = (await import(DENY_HOOK_SYNTHESIS_MODULE)) as typeof import("../../packages/loopover-miner/lib/deny-hook-synthesis.js");
2531

2632
const tempDirs: string[] = [];
2733
const stores: Array<{ close(): void }> = [];
@@ -180,6 +186,16 @@ describe("initDenyHookSynthesisStore() (#4522)", () => {
180186
expect(() => initDenyHookSynthesisStore(" ")).toThrow("invalid_deny_hook_synthesis_db_path");
181187
});
182188

189+
it("registers the store for crash-safe cleanup via openLocalStoreDb, and unregisters it on close (#8319)", () => {
190+
resetProcessLifecycleForTesting();
191+
expect(cleanupResourceCount()).toBe(0);
192+
const store = tempStore();
193+
expect(cleanupResourceCount()).toBe(1);
194+
stores.splice(stores.indexOf(store), 1);
195+
store.close();
196+
expect(cleanupResourceCount()).toBe(0);
197+
});
198+
183199
it("skips the forge-scope migration on a second open of an already-migrated file", () => {
184200
const dir = mkdtempSync(join(tmpdir(), "miner-deny-hook-synthesis-remigrate-"));
185201
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

@@ -91,6 +97,16 @@ describe("loopover-miner laptop init (#2329)", () => {
9197
expect(readFileSync(join(first.stateDir, "marker.txt"), "utf8")).toBe("keep-me");
9298
});
9399

100+
it("opens its store via openLocalStoreDb, registering and unregistering it for crash-safe cleanup within the call (#8319)", () => {
101+
resetProcessLifecycleForTesting();
102+
expect(cleanupResourceCount()).toBe(0);
103+
const root = tempRoot();
104+
initLaptopState({ LOOPOVER_MINER_CONFIG_DIR: join(root, "state") });
105+
// initLaptopState closes its own handle internally before returning (it exposes no db handle), so a
106+
// leftover registration here would mean the register→unregister cycle didn't complete cleanly.
107+
expect(cleanupResourceCount()).toBe(0);
108+
});
109+
94110
it("runInit prints human text (0) and machine JSON with --json", async () => {
95111
const root = tempRoot();
96112
const env = { LOOPOVER_MINER_CONFIG_DIR: join(root, "state") };

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@ import { mkdtempSync, rmSync } from "node:fs";
22
import { homedir, 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 { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js";
7+
import { cleanupResourceCount, resetProcessLifecycleForTesting } from "../../packages/loopover-miner/lib/process-lifecycle.js";
58

6-
import {
9+
// Import the .ts SOURCE (not the build-time .js) via a non-literal specifier. Once `build:miner` has produced
10+
// the artifact, a plain `.js` import loads that .js and leaves coverage.include's `.ts` entry at 0% — the
11+
// .js-vs-.ts mismatch that closed #8500/#8516 on codecov/patch. Same pattern as miner-replay-snapshot.test.ts (#7796).
12+
const ORB_EXPORT_MODULE = "../../packages/loopover-miner/lib/orb-export.ts";
13+
const {
714
ORB_EXPORT_ENABLED_BY_DEFAULT,
815
DEFAULT_AMS_COLLECTOR_URL,
916
DEFAULT_ORB_EXPORT_TIMEOUT_MS,
@@ -17,9 +24,7 @@ import {
1724
resolveAmsCollectorUrl,
1825
resolveOrbExportDbPath,
1926
sendAmsExportBatch,
20-
} from "../../packages/loopover-miner/lib/orb-export.js";
21-
import type { OrbExportOutcome, OrbExportRow } from "../../packages/loopover-miner/lib/orb-export.js";
22-
import { resolveLocalStoreDbPath } from "../../packages/loopover-miner/lib/local-store.js";
27+
} = (await import(ORB_EXPORT_MODULE)) as typeof import("../../packages/loopover-miner/lib/orb-export.js");
2328

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

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

0 commit comments

Comments
 (0)