diff --git a/packages/loopover-miner/lib/event-ledger.ts b/packages/loopover-miner/lib/event-ledger.ts index 361af3fc7..c72052df9 100644 --- a/packages/loopover-miner/lib/event-ledger.ts +++ b/packages/loopover-miner/lib/event-ledger.ts @@ -1,4 +1,4 @@ -import type { DatabaseSync, SQLOutputValue } from "node:sqlite"; +import { DatabaseSync, type SQLOutputValue } from "node:sqlite"; import { isDeepStrictEqual } from "node:util"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; import { applySchemaMigrations } from "./schema-version.js"; @@ -256,6 +256,48 @@ export function initEventLedger(dbPath: string = resolveEventLedgerDbPath()): Ev }; } +export type ReadOnlyEventLedger = { + dbPath: string; + readEvents(): LedgerEntry[]; + close(): void; +}; + +/** + * Strictly read-only ledger access for advisory-only callers (#10002) that must never create, migrate, or + * retention-prune the ledger file -- everything {@link initEventLedger} always does on open. Opens the DB file + * in SQLite's own `readonly` mode (driver-enforced: an attempted write throws, this isn't just a by-convention + * guarantee) and touches the filesystem in no other way -- no `mkdirSync`/`chmodSync`, no `CREATE TABLE IF NOT + * EXISTS`, no migrations, no retention pruning. Same pattern as claim-ledger.js's `openClaimLedgerReadOnly`. The + * caller MUST only call this against a path it has already confirmed exists (e.g. via `existsSync`); a + * read-only connection to a nonexistent file throws. Throws if the expected table is missing too (a file exists + * at this path but isn't a real event ledger) -- callers should treat that identically to any other open/query + * failure. + */ +export function openEventLedgerReadOnly(dbPath: string): ReadOnlyEventLedger { + const resolvedPath = normalizeDbPath(dbPath); + // `readOnly` (camelCase) -- node:sqlite silently IGNORES `readonly` (lowercase) as an unrecognized option and + // opens read-write anyway, defeating the entire point of this function. + const db = new DatabaseSync(resolvedPath, { readOnly: true }); + let readAllStatement; + try { + readAllStatement = db.prepare("SELECT * FROM miner_event_ledger ORDER BY seq ASC"); + } catch (error) { + // The table doesn't exist (a file exists at this path but isn't a real event ledger) -- close the + // connection we already opened before rethrowing, so this never leaks a file handle. + db.close(); + throw error; + } + return { + dbPath: resolvedPath, + readEvents(): LedgerEntry[] { + return readAllStatement.all().map((row) => rowToEntry(asEventDbRow(row))); + }, + close(): void { + db.close(); + }, + }; +} + function getDefaultEventLedger(): EventLedger { defaultEventLedger ??= initEventLedger(); return defaultEventLedger; diff --git a/packages/loopover-miner/lib/status.ts b/packages/loopover-miner/lib/status.ts index e0bab4043..99230ba30 100644 --- a/packages/loopover-miner/lib/status.ts +++ b/packages/loopover-miner/lib/status.ts @@ -14,7 +14,7 @@ import { } from "./laptop-init.js"; import { resolveMinerVersion } from "./version.js"; import { checkStoreIntegrity, describeError } from "./store-maintenance.js"; -import { initEventLedger, resolveEventLedgerDbPath } from "./event-ledger.js"; +import { openEventLedgerReadOnly, resolveEventLedgerDbPath } from "./event-ledger.js"; import { buildAmsBacktestProposals, readAmsThresholdBacktestRuns } from "./ams-calibration.js"; import { resolveGovernorLedgerDbPath } from "./governor-ledger.js"; import { hasGitHubTokenSource } from "./github-token-resolution.js"; @@ -533,25 +533,33 @@ export function checkCodingAgentCredential( /** #8186: current backtest-cleared min-rank proposals, mirrored from the ORB advisor's posture -- full * evidence per line and an explicit nothing-applies-automatically stance baked into the detail. Always * ok:true (informational -- a proposal is an opportunity, not a fault) and fail-open on a ledger blip - * (doctor must keep working on a box whose ledger is broken; store integrity has its own check). */ + * (doctor must keep working on a box whose ledger is broken; store integrity has its own check). + * #10002: `doctor` is documented read-only, so this must never be the thing that creates, migrates, or + * retention-prunes the event ledger -- a missing ledger file short-circuits to the same "nothing to propose" + * detail without opening a handle, and an existing one is read through `openEventLedgerReadOnly`'s + * driver-enforced read-only connection (mirrors `checkStoreIntegrity`'s `existsSync` + `readOnly: true` + * pattern), never `initEventLedger`, which every *writer* still legitimately uses. */ export function checkAmsBacktestProposals(env: Record = process.env, nowMs: number = Date.now()): DoctorCheck { + const dbPath = resolveEventLedgerDbPath(env); + if (!existsSync(dbPath)) { + return { name: "ams-backtest-proposals", ok: true, detail: "no backtest-cleared min-rank proposals (nothing applies automatically)" }; + } + let eventLedger: ReturnType | undefined; try { - const eventLedger = initEventLedger(resolveEventLedgerDbPath(env)); - try { - const proposals = buildAmsBacktestProposals(readAmsThresholdBacktestRuns(eventLedger), nowMs); - if (proposals.length === 0) { - return { name: "ams-backtest-proposals", ok: true, detail: "no backtest-cleared min-rank proposals (nothing applies automatically)" }; - } - const lines = proposals.map( - (proposal) => - `min-rank ${proposal.currentThreshold} -> ${proposal.candidateThreshold} (visible ${proposal.visibleVerdict}/${proposal.visibleCases}, held-out ${proposal.heldOutVerdict}/${proposal.heldOutCases})`, - ); - return { name: "ams-backtest-proposals", ok: true, detail: `${lines.join("; ")} -- nothing applies automatically (apply-min-rank needs the config flag AND --approve)` }; - } finally { - eventLedger.close(); + eventLedger = openEventLedgerReadOnly(dbPath); + const proposals = buildAmsBacktestProposals(readAmsThresholdBacktestRuns(eventLedger), nowMs); + if (proposals.length === 0) { + return { name: "ams-backtest-proposals", ok: true, detail: "no backtest-cleared min-rank proposals (nothing applies automatically)" }; } + const lines = proposals.map( + (proposal) => + `min-rank ${proposal.currentThreshold} -> ${proposal.candidateThreshold} (visible ${proposal.visibleVerdict}/${proposal.visibleCases}, held-out ${proposal.heldOutVerdict}/${proposal.heldOutCases})`, + ); + return { name: "ams-backtest-proposals", ok: true, detail: `${lines.join("; ")} -- nothing applies automatically (apply-min-rank needs the config flag AND --approve)` }; } catch { return { name: "ams-backtest-proposals", ok: true, detail: "event ledger unreadable; proposals unavailable" }; + } finally { + eventLedger?.close(); } } diff --git a/test/unit/miner-status.test.ts b/test/unit/miner-status.test.ts index 0af1b3b2a..54ff57012 100644 --- a/test/unit/miner-status.test.ts +++ b/test/unit/miner-status.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { resolveEventLedgerDbPath } from "../../packages/loopover-miner/lib/event-ledger"; @@ -185,6 +185,17 @@ describe("loopover-miner status/doctor (#2288)", () => { expect(runDoctor([], env)).toBe(1); // a failed check makes doctor exit non-zero }); + it("doctor does not create the event ledger on a fresh laptop (#10002)", () => { + const env = { LOOPOVER_MINER_CONFIG_DIR: join(tempRoot(), "state") }; + const eventLedgerPath = resolveEventLedgerDbPath(env); + const checks = runDoctorChecks(env); + // The read-only contract status.ts:37-39 declares: no doctor check may create the ledger file. + expect(existsSync(eventLedgerPath)).toBe(false); + const storeIntegrity = checks.find((check) => check.name === "store-integrity:event-ledger"); + expect(storeIntegrity?.ok).toBe(true); + expect(storeIntegrity?.detail).toContain("not created yet"); + }); + it("doctor flags a corrupted laptop-state store via the deep integrity sweep (#8641)", () => { const env = { LOOPOVER_MINER_CONFIG_DIR: join(tempRoot(), "state") }; const laptopPath = resolveLaptopStateDbPath(env); @@ -592,11 +603,14 @@ describe("checkAmsBacktestProposals (#8186)", () => { const dir = mkdtempSync(join(tmpdir(), "miner-status-ams-")); try { const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const dbPath = resolveEventLedgerDbPath(env); const empty = checkAmsBacktestProposals(env); expect(empty.ok).toBe(true); expect(empty.detail).toContain("no backtest-cleared min-rank proposals"); + // #10002: doctor is documented read-only -- it must never be the thing that creates the ledger file. + expect(existsSync(dbPath)).toBe(false); - const ledger = initEventLedger(resolveEventLedgerDbPath(env)); + const ledger = initEventLedger(dbPath); for (let i = 1; i <= 60; i += 1) { ledger.appendEvent({ type: "discovered_issue", repoFullName: "acme/widgets", payload: { issueNumber: i, rankScore: 0.15, title: "t", labels: [] } }); ledger.appendEvent({ type: "pr_outcome", repoFullName: "acme/widgets", payload: { prNumber: 1000 + i, decision: "closed", closedAt: "2026-07-10T00:00:00Z", reason: null, issueNumber: i } }); @@ -610,11 +624,53 @@ describe("checkAmsBacktestProposals (#8186)", () => { expect(withProposal.detail).toContain("min-rank 0 -> 0.2"); expect(withProposal.detail).toContain("nothing applies automatically"); - const broken = checkAmsBacktestProposals({ LOOPOVER_MINER_EVENT_LEDGER_DB: "/dev/null/nope/ledger.sqlite" }); + // Fail-open arm: a file that EXISTS at the ledger path but isn't a real SQLite database. + const brokenDbPath = join(dir, "broken-ledger.sqlite3"); + writeFileSync(brokenDbPath, "this is not a sqlite database"); + const broken = checkAmsBacktestProposals({ LOOPOVER_MINER_EVENT_LEDGER_DB: brokenDbPath }); expect(broken.ok).toBe(true); expect(broken.detail).toContain("unavailable"); } finally { rmSync(dir, { recursive: true, force: true }); } }); + + it("REGRESSION: doctor does not create or retention-prune the event ledger (#10002)", async () => { + const { mkdtempSync, rmSync } = await import("node:fs"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + const { initEventLedger, resolveEventLedgerDbPath } = await import("../../packages/loopover-miner/lib/event-ledger"); + const { checkAmsBacktestProposals } = await import("../../packages/loopover-miner/lib/status"); + + const dir = mkdtempSync(join(tmpdir(), "miner-status-ams-regression-")); + try { + const env = { LOOPOVER_MINER_CONFIG_DIR: dir }; + const dbPath = resolveEventLedgerDbPath(env); + + // A legitimate writer -- never doctor -- creates and seeds the ledger. + const seedLedger = initEventLedger(dbPath); + seedLedger.appendEvent({ type: "discovered_issue", repoFullName: "acme/widgets", payload: { issueNumber: 1, rankScore: 0.1, title: "a", labels: [] } }); + seedLedger.appendEvent({ type: "discovered_issue", repoFullName: "acme/widgets", payload: { issueNumber: 2, rankScore: 0.1, title: "b", labels: [] } }); + seedLedger.appendEvent({ type: "discovered_issue", repoFullName: "acme/widgets", payload: { issueNumber: 3, rankScore: 0.1, title: "c", labels: [] } }); + seedLedger.close(); + + // An operator has opted into retention pruning; doctor must not be a trigger for it. + vi.stubEnv("LOOPOVER_MINER_LEDGER_RETENTION_MAX_ROWS", "1"); + try { + const result = checkAmsBacktestProposals(env); + expect(result.ok).toBe(true); + } finally { + vi.unstubAllEnvs(); + } + + const verifyLedger = initEventLedger(dbPath); + try { + expect(verifyLedger.readEvents().length).toBe(3); + } finally { + verifyLedger.close(); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); });