From 1877b64897fbe7c3505ab2f9fe12faa99b889a29 Mon Sep 17 00:00:00 2001 From: kai392 Date: Thu, 30 Jul 2026 04:18:18 +0800 Subject: [PATCH] fix(miner): give `queue dashboard` the same store-failure handling as its siblings (#9690) `queue dashboard` was the only `queue` subcommand that let a store failure escape `runQueueCli`. `runPortfolioDashboard` opened the portfolio-queue store OUTSIDE its `try` and had only a `finally`, so an opener failure (a corrupt portfolio-queue.sqlite3, an unreadable state dir, an invalid LOOPOVER_MINER_PORTFOLIO_QUEUE_DB) propagated out of the CLI as a raw Node stack trace and exit 1 -- where every sibling subcommand prints `{ "ok": false, "error": ... }` under --json and exits 2. Mirror `runQueueClaimBatch` (portfolio-queue-cli.ts:520-541): open the store inside the `try`, `catch` into `reportCliFailure(parsed.json, describeCliError(error))`, and close with `?.` in `finally` (the initializer may throw before assigning). Import `describeCliError`. The success path, parse-error path, and `ownsQueue` ownership rule are unchanged. Four named regression tests cover both `parsed.json` arms of the new catch, the owned-store mid-run-throw close, and the `?.` opener-failure guard; all four fail against the current code. Closes #9690 Co-Authored-By: Claude Opus 4.8 --- .../loopover-miner/lib/portfolio-dashboard.ts | 13 +++- test/unit/miner-portfolio-dashboard.test.ts | 61 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/packages/loopover-miner/lib/portfolio-dashboard.ts b/packages/loopover-miner/lib/portfolio-dashboard.ts index c8aecba390..33bf3dafbc 100644 --- a/packages/loopover-miner/lib/portfolio-dashboard.ts +++ b/packages/loopover-miner/lib/portfolio-dashboard.ts @@ -8,7 +8,7 @@ // any future consumer beyond this CLI's own text/JSON output. import { initPortfolioQueueStore } from "./portfolio-queue.js"; -import { argsWantJson, reportCliFailure } from "./cli-error.js"; +import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; const QUEUE_STATUS_KEYS = ["queued", "in_progress", "done"] as const; type QueueStatusKey = (typeof QUEUE_STATUS_KEYS)[number]; @@ -149,16 +149,23 @@ export function runPortfolioDashboard( if ("error" in parsed) { return reportCliFailure(argsWantJson(args), parsed.error); } + // Open the store INSIDE the try so an opener failure (a corrupt portfolio-queue.sqlite3, an unreadable state + // dir, an invalid LOOPOVER_MINER_PORTFOLIO_QUEUE_DB) returns 2 like every sibling `queue` subcommand instead of + // escaping runQueueCli with a raw stack trace; the finally guards the close with `?.` since the initializer may + // have thrown before assigning (mirrors runQueueClaimBatch, portfolio-queue-cli.ts:520-541). const ownsQueue = options.initPortfolioQueue === undefined; - const portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); + let portfolioQueue: { listQueue(repoFullName: string | null): unknown[]; close(): void } | undefined; try { + portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)(); const summary = collectPortfolioDashboard( { portfolioQueue }, { nowMs: Number.isFinite(options.nowMs) ? (options.nowMs as number) : Date.now() }, ); console.log(parsed.json ? JSON.stringify(summary, null, 2) : renderPortfolioDashboardTable(summary)); return 0; + } catch (error) { + return reportCliFailure(parsed.json, describeCliError(error)); } finally { - if (ownsQueue) portfolioQueue.close(); + if (ownsQueue) portfolioQueue?.close(); } } diff --git a/test/unit/miner-portfolio-dashboard.test.ts b/test/unit/miner-portfolio-dashboard.test.ts index d50a37b04d..ff6b784f43 100644 --- a/test/unit/miner-portfolio-dashboard.test.ts +++ b/test/unit/miner-portfolio-dashboard.test.ts @@ -184,3 +184,64 @@ describe("runPortfolioDashboard (#4287)", () => { expect(String(log.mock.calls[0]?.[0])).toContain("portfolio queue is empty"); }); }); + +describe("runPortfolioDashboard store-failure handling (#9690)", () => { + it("REGRESSION: an opener failure returns 2 instead of escaping runQueueCli (was: threw)", () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const boom = () => { + throw new Error("invalid_portfolio_queue_db_path"); + }; + // Before the fix the store was opened OUTSIDE the try, so this threw straight out of runQueueCli. + expect(runPortfolioDashboard([], { initPortfolioQueue: boom })).toBe(2); + expect(error).toHaveBeenCalledWith("invalid_portfolio_queue_db_path"); + }); + + it("REGRESSION: an opener failure under --json emits { ok: false, error } on stdout and returns 2", () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const boom = () => { + throw new Error("invalid_portfolio_queue_db_path"); + }; + expect(runPortfolioDashboard(["--json"], { initPortfolioQueue: boom })).toBe(2); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + ok: false, + error: "invalid_portfolio_queue_db_path", + }); + expect(error).not.toHaveBeenCalled(); + }); + + it("REGRESSION: a store whose listQueue throws mid-run still closes the owned store and returns 2", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const store = { + listQueue: () => { + throw new Error("db_read_failed"); + }, + close: vi.fn(), + }; + // No injected initPortfolioQueue → ownsQueue=true → the default factory is used and must be closed in `finally`. + const pqModule = await import("../../packages/loopover-miner/lib/portfolio-queue"); + const initSpy = vi.spyOn(pqModule, "initPortfolioQueueStore").mockReturnValue(store as never); + try { + expect(runPortfolioDashboard([])).toBe(2); + expect(error).toHaveBeenCalledWith("db_read_failed"); + expect(store.close).toHaveBeenCalledTimes(1); + } finally { + initSpy.mockRestore(); + } + }); + + it("REGRESSION: an owned-store opener failure returns 2 with nothing to close (`?.` guard)", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + // ownsQueue=true AND the default factory throws before assigning → the `?.` in finally must short-circuit. + const pqModule = await import("../../packages/loopover-miner/lib/portfolio-queue"); + const initSpy = vi.spyOn(pqModule, "initPortfolioQueueStore").mockImplementation(() => { + throw new Error("invalid_portfolio_queue_db_path"); + }); + try { + expect(runPortfolioDashboard([])).toBe(2); + expect(error).toHaveBeenCalledWith("invalid_portfolio_queue_db_path"); + } finally { + initSpy.mockRestore(); + } + }); +});