diff --git a/packages/loopover-miner/lib/portfolio-queue.d.ts b/packages/loopover-miner/lib/portfolio-queue.d.ts index 9a316a0d10..7c2053e278 100644 --- a/packages/loopover-miner/lib/portfolio-queue.d.ts +++ b/packages/loopover-miner/lib/portfolio-queue.d.ts @@ -50,6 +50,7 @@ export type PortfolioQueueStore = { ) => Array<{ repoFullName: string; identifier: string; apiBaseUrl?: string }>, ): QueueEntry[]; getAttemptHistory(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueAttemptHistory; + purgeByRepo(repoFullName: string): number; close(): void; }; diff --git a/packages/loopover-miner/lib/portfolio-queue.js b/packages/loopover-miner/lib/portfolio-queue.js index f93c43f9b3..8045f061f3 100644 --- a/packages/loopover-miner/lib/portfolio-queue.js +++ b/packages/loopover-miner/lib/portfolio-queue.js @@ -1,6 +1,7 @@ import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; import { applySchemaMigrations } from "./schema-version.js"; +import { PORTFOLIO_QUEUE_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js"; // The miner's local portfolio/queue store (#2292): a 100% client-side, prioritized backlog of candidate work // items across every repo the miner has been pointed at ("what should I look at next, across everything I'm @@ -385,6 +386,13 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) reachedDone: row.status === "done", }; }, + // Explicit, operator-invoked right-to-be-forgotten purge (#5564, wired here by #6599) — never runs + // automatically. Distinct from this store's normal enqueue/release/expire lifecycle: deletes every row for + // a repo outright. normalizeRepoFullName throws on a missing/malformed name rather than letting a typo + // silently purge nothing and report success. + purgeByRepo(repoFullName) { + return purgeStoreByRepo(db, PORTFOLIO_QUEUE_PURGE_SPEC, normalizeRepoFullName(repoFullName)); + }, close() { db.close(); }, diff --git a/packages/loopover-miner/lib/purge-cli.d.ts b/packages/loopover-miner/lib/purge-cli.d.ts index bbba9301bc..0f84a01e6d 100644 --- a/packages/loopover-miner/lib/purge-cli.d.ts +++ b/packages/loopover-miner/lib/purge-cli.d.ts @@ -2,6 +2,8 @@ import type { ClaimLedger } from "./claim-ledger.js"; import type { EventLedger } from "./event-ledger.js"; import type { GovernorLedger } from "./governor-ledger.js"; import type { PredictionLedger } from "./prediction-ledger.js"; +import type { PortfolioQueueStore } from "./portfolio-queue.js"; +import type { RunStateStore } from "./run-state.js"; export const ATTEMPT_LOG_NOT_PURGEABLE_NOTE: string; @@ -33,6 +35,8 @@ export type PurgeCliOptions = { initEventLedger?: () => EventLedger; initGovernorLedger?: () => GovernorLedger; initPredictionLedger?: () => PredictionLedger; + initPortfolioQueueStore?: () => PortfolioQueueStore; + initRunStateStore?: () => RunStateStore; resolveDbPaths?: Record string>; }; diff --git a/packages/loopover-miner/lib/purge-cli.js b/packages/loopover-miner/lib/purge-cli.js index 91b2bc81e1..4fc7fb8c4e 100644 --- a/packages/loopover-miner/lib/purge-cli.js +++ b/packages/loopover-miner/lib/purge-cli.js @@ -1,6 +1,8 @@ // `loopover-miner purge` (#5564): an explicit, operator-invoked right-to-be-forgotten path across the local -// ledgers. Deletes every row for one repo from the four stores that have a real `repoColumn` (claim-ledger, -// event-ledger, governor-ledger, prediction-ledger), via each store's own `purgeByRepo` method (which reuses +// ledgers. Deletes every row for one repo from the six stores that have a real `repoColumn` (claim-ledger, +// event-ledger, governor-ledger, prediction-ledger, portfolio-queue, run-state — the last two wired in by +// #6599, which is why the four ledgers above read as the whole set in older comments), via each store's own +// `purgeByRepo` method (which reuses // `store-maintenance.js`'s shared, identifier-guarded `purgeStoreByRepo`). `attempt-log.js` is deliberately // reported as not-purgeable rather than silently skipped or approximated: its payload is a free-form // `Record` with no dedicated repo column, so a precise per-repo match isn't possible there @@ -15,12 +17,16 @@ import { openClaimLedger, resolveClaimLedgerDbPath } from "./claim-ledger.js"; import { initEventLedger, resolveEventLedgerDbPath } from "./event-ledger.js"; import { initGovernorLedger, resolveGovernorLedgerDbPath } from "./governor-ledger.js"; import { initPredictionLedger, resolvePredictionLedgerDbPath } from "./prediction-ledger.js"; +import { initPortfolioQueueStore, resolvePortfolioQueueDbPath } from "./portfolio-queue.js"; +import { initRunStateStore, resolveRunStateDbPath } from "./run-state.js"; import { resolveAttemptLogDbPath } from "./attempt-log.js"; import { CLAIM_LEDGER_PURGE_SPEC, EVENT_LEDGER_PURGE_SPEC, GOVERNOR_LEDGER_PURGE_SPEC, PREDICTION_LEDGER_PURGE_SPEC, + PORTFOLIO_QUEUE_PURGE_SPEC, + RUN_STATE_PURGE_SPEC, countStoreByRepo, describeError, } from "./store-maintenance.js"; @@ -36,6 +42,8 @@ const REAL_PURGE_TARGETS = [ { name: "event-ledger", optionKey: "initEventLedger", opener: initEventLedger, resolveDbPath: resolveEventLedgerDbPath, spec: EVENT_LEDGER_PURGE_SPEC }, { name: "governor-ledger", optionKey: "initGovernorLedger", opener: initGovernorLedger, resolveDbPath: resolveGovernorLedgerDbPath, spec: GOVERNOR_LEDGER_PURGE_SPEC }, { name: "prediction-ledger", optionKey: "initPredictionLedger", opener: initPredictionLedger, resolveDbPath: resolvePredictionLedgerDbPath, spec: PREDICTION_LEDGER_PURGE_SPEC }, + { name: "portfolio-queue", optionKey: "initPortfolioQueueStore", opener: initPortfolioQueueStore, resolveDbPath: resolvePortfolioQueueDbPath, spec: PORTFOLIO_QUEUE_PURGE_SPEC }, + { name: "run-state", optionKey: "initRunStateStore", opener: initRunStateStore, resolveDbPath: resolveRunStateDbPath, spec: RUN_STATE_PURGE_SPEC }, ]; function parseRepoArg(value, usage) { diff --git a/packages/loopover-miner/lib/run-state.d.ts b/packages/loopover-miner/lib/run-state.d.ts index 2446e12142..fb1bed014d 100644 --- a/packages/loopover-miner/lib/run-state.d.ts +++ b/packages/loopover-miner/lib/run-state.d.ts @@ -19,6 +19,7 @@ export type RunStateStore = { getRunState(repoFullName: string, apiBaseUrl?: string): RunState | null; setRunState(repoFullName: string, state: RunState, apiBaseUrl?: string): RunStateWrite; listRunStates(): RunStateRow[]; + purgeByRepo(repoFullName: string): number; close(): void; }; diff --git a/packages/loopover-miner/lib/run-state.js b/packages/loopover-miner/lib/run-state.js index de9bfaa843..6fdddf29fb 100644 --- a/packages/loopover-miner/lib/run-state.js +++ b/packages/loopover-miner/lib/run-state.js @@ -1,6 +1,7 @@ import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; import { applySchemaMigrations } from "./schema-version.js"; +import { purgeStoreByRepo, RUN_STATE_PURGE_SPEC } from "./store-maintenance.js"; export const RUN_STATES = Object.freeze(["idle", "discovering", "planning", "preparing"]); @@ -133,6 +134,12 @@ export function initRunStateStore(dbPath = resolveRunStateDbPath()) { updatedAt: row.updated_at, })); }, + // Explicit, operator-invoked right-to-be-forgotten purge (#5564, wired here by #6599) — never runs + // automatically. Deletes every tracked-state row for a repo outright. normalizeRepoFullName throws on a + // missing/malformed name rather than letting a typo silently purge nothing and report success. + purgeByRepo(repoFullName) { + return purgeStoreByRepo(db, RUN_STATE_PURGE_SPEC, normalizeRepoFullName(repoFullName)); + }, close() { db.close(); }, diff --git a/packages/loopover-miner/lib/store-maintenance.d.ts b/packages/loopover-miner/lib/store-maintenance.d.ts index f54483b6bb..438c38d383 100644 --- a/packages/loopover-miner/lib/store-maintenance.d.ts +++ b/packages/loopover-miner/lib/store-maintenance.d.ts @@ -13,6 +13,8 @@ export const CLAIM_LEDGER_PURGE_SPEC: LedgerPurgeSpec; export const EVENT_LEDGER_PURGE_SPEC: LedgerPurgeSpec; export const GOVERNOR_LEDGER_PURGE_SPEC: LedgerPurgeSpec; export const PREDICTION_LEDGER_PURGE_SPEC: LedgerPurgeSpec; +export const PORTFOLIO_QUEUE_PURGE_SPEC: LedgerPurgeSpec; +export const RUN_STATE_PURGE_SPEC: LedgerPurgeSpec; export type StoreIntegrityResult = { name: string; ok: boolean; detail: string }; export type LedgerRetentionPolicy = { maxAgeMs?: number; maxRows?: number }; diff --git a/packages/loopover-miner/lib/store-maintenance.js b/packages/loopover-miner/lib/store-maintenance.js index 073d2a1bfe..faba3a8f28 100644 --- a/packages/loopover-miner/lib/store-maintenance.js +++ b/packages/loopover-miner/lib/store-maintenance.js @@ -32,6 +32,11 @@ export const CLAIM_LEDGER_PURGE_SPEC = { table: "miner_claims", repoColumn: "rep export const EVENT_LEDGER_PURGE_SPEC = { table: "miner_event_ledger", repoColumn: "repo_full_name" }; export const GOVERNOR_LEDGER_PURGE_SPEC = { table: "governor_events", repoColumn: "repo_full_name" }; export const PREDICTION_LEDGER_PURGE_SPEC = { table: "predictions", repoColumn: "repo_full_name" }; +// These two stores keep repo_full_name inside their PRIMARY KEY rather than as a plain column, but that is +// irrelevant to purging: a DELETE by repo works the same either way, so they are purgeable for the same reason +// the four above are, and were simply never wired up (#6599). +export const PORTFOLIO_QUEUE_PURGE_SPEC = { table: "miner_portfolio_queue", repoColumn: "repo_full_name" }; +export const RUN_STATE_PURGE_SPEC = { table: "miner_run_state", repoColumn: "repo_full_name" }; const SQL_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/; diff --git a/test/unit/miner-portfolio-queue.test.ts b/test/unit/miner-portfolio-queue.test.ts index 84f4b70d3d..646d4d5bd9 100644 --- a/test/unit/miner-portfolio-queue.test.ts +++ b/test/unit/miner-portfolio-queue.test.ts @@ -644,4 +644,32 @@ describe("loopover-miner portfolio/queue store (#2292)", () => { }).not.toThrow(); }); }); + + describe("purgeByRepo (#6599)", () => { + it("deletes every queued item for one repo and leaves other repos untouched", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "owner/repo-a", identifier: "issue:1" }); + store.enqueue({ repoFullName: "owner/repo-a", identifier: "issue:2" }); + store.enqueue({ repoFullName: "owner/repo-b", identifier: "issue:3" }); + + expect(store.purgeByRepo("owner/repo-a")).toBe(2); + expect(store.listQueue("owner/repo-a")).toEqual([]); + expect(store.listQueue("owner/repo-b")).toHaveLength(1); + }); + + it("returns 0 when nothing matches the repo", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "owner/repo-b", identifier: "issue:1" }); + expect(store.purgeByRepo("owner/repo-a")).toBe(0); + expect(store.listQueue("owner/repo-b")).toHaveLength(1); + }); + + it("rejects a missing/malformed repoFullName rather than silently no-opping", () => { + // A typo'd repo must not report a successful purge of nothing — the operator would believe the + // right-to-be-forgotten request was honored. + const store = tempStore(); + expect(() => store.purgeByRepo(undefined as never)).toThrow("invalid_repo_full_name"); + expect(() => store.purgeByRepo("no-slash")).toThrow("invalid_repo_full_name"); + }); + }); }); diff --git a/test/unit/miner-purge-cli.test.ts b/test/unit/miner-purge-cli.test.ts index 89f6d79385..b9909bc50d 100644 --- a/test/unit/miner-purge-cli.test.ts +++ b/test/unit/miner-purge-cli.test.ts @@ -6,6 +6,8 @@ import { openClaimLedger, closeDefaultClaimLedger } from "../../packages/loopove import { initEventLedger, closeDefaultEventLedger } from "../../packages/loopover-miner/lib/event-ledger.js"; import { initGovernorLedger, closeDefaultGovernorLedger } from "../../packages/loopover-miner/lib/governor-ledger.js"; import { initPredictionLedger, closeDefaultPredictionLedger } from "../../packages/loopover-miner/lib/prediction-ledger.js"; +import { initPortfolioQueueStore } from "../../packages/loopover-miner/lib/portfolio-queue.js"; +import { initRunStateStore } from "../../packages/loopover-miner/lib/run-state.js"; import { initAttemptLog, closeDefaultAttemptLog } from "../../packages/loopover-miner/lib/attempt-log.js"; import { ATTEMPT_LOG_NOT_PURGEABLE_NOTE, @@ -69,12 +71,14 @@ describe("parsePurgeArgs (#5564)", () => { }); describe("runPurge --dry-run (#5564)", () => { - it("counts matching rows across the four real stores without writing anything, and reports attempt-log as not-purgeable", async () => { + it("counts matching rows across the six real stores without writing anything, and reports attempt-log as not-purgeable", async () => { const root = tempDir(); const claimDbPath = join(root, "claim-ledger.sqlite3"); const eventDbPath = join(root, "event-ledger.sqlite3"); const governorDbPath = join(root, "governor-ledger.sqlite3"); const predictionDbPath = join(root, "prediction-ledger.sqlite3"); + const portfolioDbPath = join(root, "portfolio-queue.sqlite3"); + const runStateDbPath = join(root, "run-state.sqlite3"); const attemptLogDbPath = join(root, "attempt-log.sqlite3"); // never created — dry run must not touch it const claimLedger = openClaimLedger(claimDbPath); @@ -108,11 +112,26 @@ describe("runPurge --dry-run (#5564)", () => { }); predictionLedger.close(); + // #6599: both of these persist repo_full_name and were silently left out of the purge entirely. Each gets + // 2 rows for the target repo and 1 for another repo, so the count proves the sweep is repo-scoped. + const portfolioQueue = initPortfolioQueueStore(portfolioDbPath); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1" }); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:2" }); + portfolioQueue.enqueue({ repoFullName: "acme/other", identifier: "issue:3" }); + portfolioQueue.close(); + + const runState = initRunStateStore(runStateDbPath); + runState.setRunState("acme/widgets", "planning"); + runState.setRunState("acme/other", "idle"); + runState.close(); + const resolveDbPaths = { "claim-ledger": () => claimDbPath, "event-ledger": () => eventDbPath, "governor-ledger": () => governorDbPath, "prediction-ledger": () => predictionDbPath, + "portfolio-queue": () => portfolioDbPath, + "run-state": () => runStateDbPath, "attempt-log": () => attemptLogDbPath, }; @@ -127,6 +146,8 @@ describe("runPurge --dry-run (#5564)", () => { { store: "event-ledger", wouldPurge: 1 }, { store: "governor-ledger", wouldPurge: 1 }, { store: "prediction-ledger", wouldPurge: 0 }, + { store: "portfolio-queue", wouldPurge: 2 }, + { store: "run-state", wouldPurge: 1 }, ], attemptLogNote: ATTEMPT_LOG_NOT_PURGEABLE_NOTE, attemptLogTotalRows: 0, @@ -283,11 +304,15 @@ describe("runPurge (real, #5564)", () => { const event = fakeStore(1); const governor = fakeStore(0); const prediction = fakeStore(3); + const portfolioQueue = fakeStore(4); // #6599 + const runState = fakeStore(1); // #6599 const options = { openClaimLedger: () => claim, initEventLedger: () => event, initGovernorLedger: () => governor, initPredictionLedger: () => prediction, + initPortfolioQueueStore: () => portfolioQueue, + initRunStateStore: () => runState, }; const log = vi.spyOn(console, "log").mockImplementation(() => undefined); @@ -296,28 +321,30 @@ describe("runPurge (real, #5564)", () => { expect(summary).toMatchObject({ outcome: "purged", repoFullName: "acme/widgets", - totalPurged: 6, + totalPurged: 11, // 6 from the four original stores + 5 newly covered by #6599 stores: [ { store: "claim-ledger", purged: 2 }, { store: "event-ledger", purged: 1 }, { store: "governor-ledger", purged: 0 }, { store: "prediction-ledger", purged: 3 }, + { store: "portfolio-queue", purged: 4 }, + { store: "run-state", purged: 1 }, { store: "attempt-log", purged: null, note: ATTEMPT_LOG_NOT_PURGEABLE_NOTE }, ], }); expect(typeof summary.purgedAt).toBe("string"); - for (const store of [claim, event, governor, prediction]) { + for (const store of [claim, event, governor, prediction, portfolioQueue, runState]) { expect(store.purgeByRepo).toHaveBeenCalledWith("acme/widgets"); } // Injected stores are caller-owned: runPurge must not close them. - for (const store of [claim, event, governor, prediction]) { + for (const store of [claim, event, governor, prediction, portfolioQueue, runState]) { expect(store.close).not.toHaveBeenCalled(); } log.mockClear(); expect(runPurge(["--repo", "acme/widgets"], options as never)).toBe(0); const text = String(log.mock.calls[0]?.[0]); - expect(text).toContain("Purged 6 row(s) for acme/widgets"); + expect(text).toContain("Purged 11 row(s) for acme/widgets"); expect(text).toContain("claim-ledger=2"); expect(text).toContain(ATTEMPT_LOG_NOT_PURGEABLE_NOTE); }); diff --git a/test/unit/miner-run-state.test.ts b/test/unit/miner-run-state.test.ts index 6d84e31f42..30380aeba4 100644 --- a/test/unit/miner-run-state.test.ts +++ b/test/unit/miner-run-state.test.ts @@ -359,4 +359,44 @@ describe("loopover-miner run-state store (#2289)", () => { }).not.toThrow(); }); }); + + describe("purgeByRepo (#6599)", () => { + // Closed via this block's own afterEach rather than per-test, so a failing assertion still releases the + // SQLite handle — an open handle makes the outer afterEach's rmSync fail on Windows. + const openStores: Array<{ close: () => void }> = []; + afterEach(() => { + for (const store of openStores.splice(0)) store.close(); + }); + + function tempStore() { + const store = initRunStateStore(join(tempRoot(), "nested", "run-state.sqlite3")); + openStores.push(store); + return store; + } + + it("deletes the tracked state for one repo and leaves other repos untouched", () => { + const store = tempStore(); + store.setRunState("owner/repo-a", "planning"); + store.setRunState("owner/repo-b", "preparing"); + + expect(store.purgeByRepo("owner/repo-a")).toBe(1); + expect(store.getRunState("owner/repo-a")).toBeNull(); + expect(store.listRunStates()).toHaveLength(1); + }); + + it("returns 0 when nothing matches the repo", () => { + const store = tempStore(); + store.setRunState("owner/repo-b", "planning"); + expect(store.purgeByRepo("owner/repo-a")).toBe(0); + expect(store.listRunStates()).toHaveLength(1); + }); + + it("rejects a missing/malformed repoFullName rather than silently no-opping", () => { + // A typo'd repo must not report a successful purge of nothing — the operator would believe the + // right-to-be-forgotten request was honored. + const store = tempStore(); + expect(() => store.purgeByRepo(undefined as never)).toThrow("invalid_repo_full_name"); + expect(() => store.purgeByRepo("no-slash")).toThrow("invalid_repo_full_name"); + }); + }); });