Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions packages/loopover-miner/lib/deny-hook-synthesis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions packages/loopover-miner/lib/laptop-init.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -53,9 +53,11 @@ export function resolveLaptopStateDbPath(env: Record<string, string | undefined>
export function initLaptopState(env: Record<string, string | undefined> = 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,
Expand All @@ -68,7 +70,6 @@ export function initLaptopState(env: Record<string, string | undefined> = 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 };
}
Expand Down
12 changes: 4 additions & 8 deletions packages/loopover-miner/lib/orb-export.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = ?");
Expand Down
30 changes: 23 additions & 7 deletions test/unit/miner-deny-hook-synthesis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 }> = [];
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 18 additions & 2 deletions test/unit/miner-laptop-init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];

Expand Down Expand Up @@ -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") };
Expand Down
22 changes: 18 additions & 4 deletions test/unit/miner-orb-export.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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", () => {
Expand Down