diff --git a/packages/loopover-miner/lib/manage-poll.ts b/packages/loopover-miner/lib/manage-poll.ts index 856c692be..6a5ec4963 100644 --- a/packages/loopover-miner/lib/manage-poll.ts +++ b/packages/loopover-miner/lib/manage-poll.ts @@ -302,8 +302,24 @@ export async function runManagePoll( const ownsEventLedger = options.initEventLedger === undefined; const ownsPortfolioQueue = options.initPortfolioQueue === undefined; - const eventLedger = (options.initEventLedger ?? initEventLedger)(); - const portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); + + // Each opener gets its OWN try/catch (mirroring runDiscover, discover-cli.ts:631-637): an opener throwing must + // return 2 via reportCliFailure instead of propagating an unhandled throw, and must close whatever sibling + // handle this function already opened, or that SQLite handle leaks. + let eventLedger: EventLedger; + try { + eventLedger = (options.initEventLedger ?? initEventLedger)(); + } catch (error) { + return reportCliFailure(parsed.json, describeCliError(error)); + } + + let portfolioQueue: PortfolioQueueStore; + try { + portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); + } catch (error) { + if (ownsEventLedger) eventLedger.close(); + return reportCliFailure(parsed.json, describeCliError(error)); + } try { const result = await recordManagePollSnapshot( diff --git a/packages/loopover-miner/lib/manage-status.ts b/packages/loopover-miner/lib/manage-status.ts index ad9ddff61..1a6ebffa0 100644 --- a/packages/loopover-miner/lib/manage-status.ts +++ b/packages/loopover-miner/lib/manage-status.ts @@ -278,9 +278,34 @@ export function runManageStatus( const ownsPortfolioQueue = options.initPortfolioQueue === undefined; const ownsEventLedger = options.initEventLedger === undefined; const ownsRunStateStore = options.initRunStateStore === undefined; - const portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); - const eventLedger = (options.initEventLedger ?? initEventLedger)(); - const runStateStore = (options.initRunStateStore ?? initRunStateStore)(); + + // Each opener gets its OWN try/catch (mirroring runDiscover, discover-cli.ts:631-637): an opener throwing must + // return 2 via reportCliFailure instead of propagating an unhandled throw, and must close whatever sibling + // handles this function already opened, or those SQLite handles leak. + let portfolioQueue: PortfolioQueueStore; + try { + portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); + } catch (error) { + return reportCliFailure(parsed.json, describeCliError(error)); + } + + let eventLedger: EventLedger; + try { + eventLedger = (options.initEventLedger ?? initEventLedger)(); + } catch (error) { + if (ownsPortfolioQueue) portfolioQueue.close(); + return reportCliFailure(parsed.json, describeCliError(error)); + } + + let runStateStore: RunStateStore; + try { + runStateStore = (options.initRunStateStore ?? initRunStateStore)(); + } catch (error) { + if (ownsPortfolioQueue) portfolioQueue.close(); + if (ownsEventLedger) eventLedger.close(); + return reportCliFailure(parsed.json, describeCliError(error)); + } + try { const rows = collectManageStatus({ portfolioQueue, eventLedger }); const runPortfolio = collectRunPortfolio({ portfolioQueue, eventLedger, runStateStore }); diff --git a/test/unit/miner-manage-poll.test.ts b/test/unit/miner-manage-poll.test.ts index 32d96beaf..f9349c992 100644 --- a/test/unit/miner-manage-poll.test.ts +++ b/test/unit/miner-manage-poll.test.ts @@ -6,6 +6,8 @@ import { closeDefaultEventLedger, initEventLedger, } from "../../packages/loopover-miner/lib/event-ledger"; +import type { EventLedger } from "../../packages/loopover-miner/lib/event-ledger"; +import * as eventLedgerModule from "../../packages/loopover-miner/lib/event-ledger"; import { MANAGE_PR_UPDATE_EVENT, collectManageStatus, @@ -329,6 +331,73 @@ describe("loopover-miner manage poll (#2323/#2325)", () => { }); }); + it("returns 2 and prints the error (honoring --json) when the event-ledger opener itself throws", async () => { + const initStores = { + initEventLedger: () => { + throw new Error("boom"); + }, + }; + + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + expect(await runManagePoll(["acme/widgets", "4"], initStores)).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom"); + + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + expect(await runManagePoll(["acme/widgets", "4", "--json"], initStores)).toBe(2); + expect(JSON.parse(String(log.mock.calls.at(-1)?.[0]))).toEqual({ ok: false, error: "boom" }); + }); + + it("REGRESSION: a store-open failure exits 2 instead of throwing, and closes the handle already opened", async () => { + const root = mkdtempSync(join(tmpdir(), "loopover-miner-manage-poll-open-failure-")); + roots.push(root); + const previousEventLedgerDbPath = process.env.LOOPOVER_MINER_EVENT_LEDGER_DB; + process.env.LOOPOVER_MINER_EVENT_LEDGER_DB = join(root, "event-ledger.sqlite3"); + try { + // The event ledger is opened for real (no override, so runManagePoll owns and would normally close it + // itself) so this proves the actual owned-close path, not just that an injected test-double was passed + // through untouched. + const realInitEventLedger = eventLedgerModule.initEventLedger; + let openedEventLedger: ReturnType | undefined; + let closeSpy: ReturnType | undefined; + vi.spyOn(eventLedgerModule, "initEventLedger").mockImplementation((...args) => { + openedEventLedger = realInitEventLedger(...args); + closeSpy = vi.spyOn(openedEventLedger, "close"); + return openedEventLedger; + }); + + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + expect( + await runManagePoll(["acme/widgets", "4"], { + initPortfolioQueue: () => { + throw new Error("boom: portfolio-queue open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: portfolio-queue open failed"); + expect(openedEventLedger).toBeDefined(); + expect(closeSpy).toHaveBeenCalledTimes(1); + } finally { + if (previousEventLedgerDbPath === undefined) delete process.env.LOOPOVER_MINER_EVENT_LEDGER_DB; + else process.env.LOOPOVER_MINER_EVENT_LEDGER_DB = previousEventLedgerDbPath; + } + }); + + it("leaves an injected (non-owned) event ledger open when the portfolio-queue opener throws", async () => { + const injectedEventLedger = { appendEvent: () => null, close: vi.fn() }; + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + expect( + await runManagePoll(["acme/widgets", "4"], { + initEventLedger: () => injectedEventLedger as unknown as EventLedger, + initPortfolioQueue: () => { + throw new Error("boom: portfolio-queue open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: portfolio-queue open failed"); + expect(injectedEventLedger.close).not.toHaveBeenCalled(); + }); + it("parseManagePollArgs rejects an invalid owner/repo argument", () => { expect(parseManagePollArgs(["acme", "42"])).toEqual({ error: "Repository must be in owner/repo form.", diff --git a/test/unit/miner-manage-status.test.ts b/test/unit/miner-manage-status.test.ts index 5e244c4b5..0eebe90d4 100644 --- a/test/unit/miner-manage-status.test.ts +++ b/test/unit/miner-manage-status.test.ts @@ -19,14 +19,19 @@ import { closeDefaultEventLedger, initEventLedger, } from "../../packages/loopover-miner/lib/event-ledger"; +import type { EventLedger } from "../../packages/loopover-miner/lib/event-ledger"; +import * as eventLedgerModule from "../../packages/loopover-miner/lib/event-ledger"; import { closeDefaultPortfolioQueueStore, initPortfolioQueueStore, } from "../../packages/loopover-miner/lib/portfolio-queue"; +import type { PortfolioQueueStore } from "../../packages/loopover-miner/lib/portfolio-queue"; +import * as portfolioQueueModule from "../../packages/loopover-miner/lib/portfolio-queue"; import { closeDefaultRunStateStore, initRunStateStore, } from "../../packages/loopover-miner/lib/run-state"; +import type { RunStateStore } from "../../packages/loopover-miner/lib/run-state"; const roots: string[] = []; const stores: Array<{ close(): void }> = []; @@ -350,6 +355,123 @@ describe("loopover-miner manage status (#2325)", () => { }); }); + it("returns 2 and prints the error (honoring --json) when the portfolio-queue opener itself throws", () => { + const initStores = { + initPortfolioQueue: () => { + throw new Error("boom"); + }, + initEventLedger: () => ({ readEvents: () => [], close() {} }) as unknown as EventLedger, + initRunStateStore: () => ({ listRunStates: () => [], close() {} }) as unknown as RunStateStore, + }; + + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(runManageStatus([], initStores)).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom"); + + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + expect(runManageStatus(["--json"], initStores)).toBe(2); + expect(JSON.parse(String(log.mock.calls.at(-1)?.[0]))).toEqual({ ok: false, error: "boom" }); + }); + + it("REGRESSION: a store-open failure exits 2 instead of throwing, and closes the handle already opened", () => { + const root = mkdtempSync(join(tmpdir(), "loopover-miner-manage-status-open-failure-")); + roots.push(root); + vi.stubEnv("LOOPOVER_MINER_CONFIG_DIR", root); + + // The portfolio queue is opened for real (no override, so runManageStatus owns and would normally close it + // itself) so this proves the actual owned-close path, not just that an injected test-double was passed + // through untouched. + const realInitPortfolioQueue = portfolioQueueModule.initPortfolioQueueStore; + let openedPortfolioQueue: ReturnType | undefined; + let closeSpy: ReturnType | undefined; + vi.spyOn(portfolioQueueModule, "initPortfolioQueueStore").mockImplementation((...args) => { + openedPortfolioQueue = realInitPortfolioQueue(...args); + closeSpy = vi.spyOn(openedPortfolioQueue, "close"); + return openedPortfolioQueue; + }); + + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + expect( + runManageStatus([], { + initEventLedger: () => { + throw new Error("boom: event-ledger open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: event-ledger open failed"); + expect(openedPortfolioQueue).toBeDefined(); + expect(closeSpy).toHaveBeenCalledTimes(1); + }); + + it("leaves an injected (non-owned) portfolio queue open when the event-ledger opener throws", () => { + const injectedPortfolioQueue = { listQueue: () => [], close: vi.fn() }; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + + expect( + runManageStatus([], { + initPortfolioQueue: () => injectedPortfolioQueue as unknown as PortfolioQueueStore, + initEventLedger: () => { + throw new Error("boom: event-ledger open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: event-ledger open failed"); + expect(injectedPortfolioQueue.close).not.toHaveBeenCalled(); + }); + + it("REGRESSION: a run-state-store open failure closes both an owned portfolio queue and an owned event ledger", () => { + const root = mkdtempSync(join(tmpdir(), "loopover-miner-manage-status-run-state-open-failure-")); + roots.push(root); + vi.stubEnv("LOOPOVER_MINER_CONFIG_DIR", root); + + const realInitPortfolioQueue = portfolioQueueModule.initPortfolioQueueStore; + let portfolioQueueCloseSpy: ReturnType | undefined; + vi.spyOn(portfolioQueueModule, "initPortfolioQueueStore").mockImplementation((...args) => { + const store = realInitPortfolioQueue(...args); + portfolioQueueCloseSpy = vi.spyOn(store, "close"); + return store; + }); + + const realInitEventLedger = eventLedgerModule.initEventLedger; + let eventLedgerCloseSpy: ReturnType | undefined; + vi.spyOn(eventLedgerModule, "initEventLedger").mockImplementation((...args) => { + const store = realInitEventLedger(...args); + eventLedgerCloseSpy = vi.spyOn(store, "close"); + return store; + }); + + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + expect( + runManageStatus([], { + initRunStateStore: () => { + throw new Error("boom: run-state open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: run-state open failed"); + expect(portfolioQueueCloseSpy).toHaveBeenCalledTimes(1); + expect(eventLedgerCloseSpy).toHaveBeenCalledTimes(1); + }); + + it("leaves injected (non-owned) portfolio queue and event ledger open when the run-state-store opener throws", () => { + const injectedPortfolioQueue = { listQueue: () => [], close: vi.fn() }; + const injectedEventLedger = { readEvents: () => [], close: vi.fn() }; + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + + expect( + runManageStatus([], { + initPortfolioQueue: () => injectedPortfolioQueue as unknown as PortfolioQueueStore, + initEventLedger: () => injectedEventLedger as unknown as EventLedger, + initRunStateStore: () => { + throw new Error("boom: run-state open failed"); + }, + }), + ).toBe(2); + expect(String(error.mock.calls.at(-1)?.[0])).toContain("boom: run-state open failed"); + expect(injectedPortfolioQueue.close).not.toHaveBeenCalled(); + expect(injectedEventLedger.close).not.toHaveBeenCalled(); + }); + it("rejects unknown CLI options", () => { const error = vi.spyOn(console, "error").mockImplementation(() => {}); expect(runManageStatus(["--verbose"])).toBe(2);