From dd866550833cce3e1b4bcf336bfea8b4f3db2cd1 Mon Sep 17 00:00:00 2001 From: tryeverything24 Date: Sun, 19 Jul 2026 17:17:56 -0700 Subject: [PATCH] chore(miner): migrate batch 4.1 foundational lib modules to TypeScript Converts claim-adjudication, governor-chokepoint-persisted, sentry, idea-feasibility, rejection-state-machine, attempt-worktree, portfolio-discovery, and process-lifecycle from plain .js + hand-maintained .d.ts to real .ts under the existing in-place tsc emit pipeline from #7299. Extends targeted unit tests to drive every converted file to 100% statement/branch/function/line coverage. --- .../loopover-miner/lib/attempt-worktree.d.ts | 69 ++++--- .../loopover-miner/lib/attempt-worktree.js | 111 +++++------ .../loopover-miner/lib/attempt-worktree.ts | 109 +++++++++++ .../lib/claim-adjudication.d.ts | 36 ++-- .../loopover-miner/lib/claim-adjudication.js | 17 +- .../loopover-miner/lib/claim-adjudication.ts | 52 ++++++ .../lib/governor-chokepoint-persisted.d.ts | 17 +- .../lib/governor-chokepoint-persisted.js | 63 +++---- .../lib/governor-chokepoint-persisted.ts | 57 ++++++ .../loopover-miner/lib/idea-feasibility.d.ts | 64 +++---- .../loopover-miner/lib/idea-feasibility.js | 69 +++---- .../loopover-miner/lib/idea-feasibility.ts | 114 ++++++++++++ .../lib/portfolio-discovery.d.ts | 41 ++-- .../loopover-miner/lib/portfolio-discovery.js | 176 +++++++++--------- .../loopover-miner/lib/portfolio-discovery.ts | 138 ++++++++++++++ .../loopover-miner/lib/process-lifecycle.d.ts | 79 +++++--- .../loopover-miner/lib/process-lifecycle.js | 138 +++++++------- .../loopover-miner/lib/process-lifecycle.ts | 143 ++++++++++++++ .../lib/rejection-state-machine.d.ts | 73 +++++--- .../lib/rejection-state-machine.js | 53 +++--- .../lib/rejection-state-machine.ts | 98 ++++++++++ packages/loopover-miner/lib/sentry.d.ts | 35 ++-- packages/loopover-miner/lib/sentry.js | 69 +++---- packages/loopover-miner/lib/sentry.ts | 67 +++++++ test/unit/miner-attempt-worktree.test.ts | 21 +++ ...iner-governor-chokepoint-persisted.test.ts | 19 +- test/unit/miner-portfolio-discovery.test.ts | 21 ++- .../miner-rejection-state-machine.test.ts | 12 ++ 28 files changed, 1392 insertions(+), 569 deletions(-) create mode 100644 packages/loopover-miner/lib/attempt-worktree.ts create mode 100644 packages/loopover-miner/lib/claim-adjudication.ts create mode 100644 packages/loopover-miner/lib/governor-chokepoint-persisted.ts create mode 100644 packages/loopover-miner/lib/idea-feasibility.ts create mode 100644 packages/loopover-miner/lib/portfolio-discovery.ts create mode 100644 packages/loopover-miner/lib/process-lifecycle.ts create mode 100644 packages/loopover-miner/lib/rejection-state-machine.ts create mode 100644 packages/loopover-miner/lib/sentry.ts diff --git a/packages/loopover-miner/lib/attempt-worktree.d.ts b/packages/loopover-miner/lib/attempt-worktree.d.ts index 645c519db8..8770375570 100644 --- a/packages/loopover-miner/lib/attempt-worktree.d.ts +++ b/packages/loopover-miner/lib/attempt-worktree.d.ts @@ -1,31 +1,46 @@ import type { WorktreeExecFn } from "@loopover/engine"; import type { RunGitFn } from "./repo-clone.js"; - -export function createRealWorktreeExec(timeoutMs?: number): WorktreeExecFn; - +/** + * Real child_process-backed implementation of the engine's WorktreeExecFn contract. Resolves (never + * rejects) on error/timeout, mirroring coding-agent-construction.js's createRealCliSubprocessSpawn -- a + * failed `git worktree add`'s stderr is the diagnosable signal, not something to lose to an unhandled + * rejection. + */ +export declare function createRealWorktreeExec(timeoutMs?: number): WorktreeExecFn; export type PrepareAttemptWorktreeOptions = { - baseBranch?: string; - cloneBaseDir?: string; - env?: Record; - exec?: WorktreeExecFn; - timeoutMs?: number; - remoteUrl?: string; - runGit?: RunGitFn; + baseBranch?: string; + cloneBaseDir?: string; + env?: Record; + exec?: WorktreeExecFn; + timeoutMs?: number; + remoteUrl?: string; + runGit?: RunGitFn; }; - -export type PrepareAttemptWorktreeResult = - | { ok: true; worktreePath: string; branchName: string; repoPath: string } - | { ok: false; repoPath?: string; error: string }; - -export function prepareAttemptWorktree( - repoFullName: string, - attemptId: string, - options?: PrepareAttemptWorktreeOptions, -): Promise; - -export function cleanupAttemptWorktree( - repoPath: string, - worktreePath: string, - attemptOk: boolean, - options?: { exec?: WorktreeExecFn; timeoutMs?: number }, -): Promise<{ ok: boolean; removed: boolean; error?: string }>; +export type PrepareAttemptWorktreeResult = { + ok: true; + worktreePath: string; + branchName: string; + repoPath: string; +} | { + ok: false; + repoPath?: string; + error: string; +}; +/** + * Prepare a real, isolated git worktree for one attempt: ensure the target repo's base clone exists and is + * current, then create a fresh `git worktree` off it on a deterministically-named branch. Fails closed + * (`ok: false`) on any step's failure rather than handing back a half-prepared directory. + */ +export declare function prepareAttemptWorktree(repoFullName: string, attemptId: string, options?: PrepareAttemptWorktreeOptions): Promise; +/** + * Tear down an attempt's worktree once the attempt concludes, per the engine's own retention policy: a + * failed attempt's worktree is RETAINED for post-mortem inspection, a succeeded one is removed. + */ +export declare function cleanupAttemptWorktree(repoPath: string, worktreePath: string, attemptOk: boolean, options?: { + exec?: WorktreeExecFn; + timeoutMs?: number; +}): Promise<{ + ok: boolean; + removed: boolean; + error?: string; +}>; diff --git a/packages/loopover-miner/lib/attempt-worktree.js b/packages/loopover-miner/lib/attempt-worktree.js index 67ecd62edf..2954348975 100644 --- a/packages/loopover-miner/lib/attempt-worktree.js +++ b/packages/loopover-miner/lib/attempt-worktree.js @@ -1,94 +1,79 @@ import { spawn } from "node:child_process"; import { addWorktree, removeWorktree, shouldRetainWorktree } from "@loopover/engine"; import { ensureRepoCloned } from "./repo-clone.js"; - // Real attempt-worktree preparation (#5132, Wave 3.5 follow-up). Composes ensureRepoCloned (repo-clone.js, // the missing base-clone-management step) with @loopover/engine's already-built, already-tested // addWorktree/removeWorktree primitives -- which existed but were never called from this package, so // `workingDirectory` handed to runIterateLoop was always just an empty directory with no real git repo in // it. This is the caller that finally exercises them for real. - const DEFAULT_TIMEOUT_MS = 120_000; - /** * Real child_process-backed implementation of the engine's WorktreeExecFn contract. Resolves (never * rejects) on error/timeout, mirroring coding-agent-construction.js's createRealCliSubprocessSpawn -- a * failed `git worktree add`'s stderr is the diagnosable signal, not something to lose to an unhandled * rejection. - * - * @returns {import("@loopover/engine").WorktreeExecFn} */ export function createRealWorktreeExec(timeoutMs = DEFAULT_TIMEOUT_MS) { - return (cmd, args, opts) => - new Promise((resolve) => { - const child = spawn(cmd, args, { cwd: opts.cwd, stdio: ["ignore", "pipe", "pipe"] }); - let stdout = ""; - let stderr = ""; - const timer = setTimeout(() => { - child.kill("SIGKILL"); - resolve({ code: null, stdout, stderr: `${stderr}\ntimed_out_after_${timeoutMs}ms`.trim() }); - }, timeoutMs); - child.stdout?.on("data", (chunk) => { - stdout += chunk.toString("utf8"); - }); - child.stderr?.on("data", (chunk) => { - stderr += chunk.toString("utf8"); - }); - child.on("error", (err) => { - clearTimeout(timer); - resolve({ code: null, stdout, stderr: err.message }); - }); - child.on("close", (code) => { - clearTimeout(timer); - resolve({ code, stdout, stderr }); - }); + return (cmd, args, opts) => new Promise((resolve) => { + const child = spawn(cmd, [...args], { cwd: opts.cwd, stdio: ["ignore", "pipe", "pipe"] }); + let stdout = ""; + let stderr = ""; + const timer = setTimeout(() => { + child.kill("SIGKILL"); + resolve({ code: null, stdout, stderr: `${stderr}\ntimed_out_after_${timeoutMs}ms`.trim() }); + }, timeoutMs); + child.stdout?.on("data", (chunk) => { + stdout += chunk.toString("utf8"); + }); + child.stderr?.on("data", (chunk) => { + stderr += chunk.toString("utf8"); + }); + child.on("error", (err) => { + clearTimeout(timer); + resolve({ code: null, stdout, stderr: err.message }); + }); + child.on("close", (code) => { + clearTimeout(timer); + resolve({ code, stdout, stderr }); + }); }); } - /** * Prepare a real, isolated git worktree for one attempt: ensure the target repo's base clone exists and is * current, then create a fresh `git worktree` off it on a deterministically-named branch. Fails closed * (`ok: false`) on any step's failure rather than handing back a half-prepared directory. - * - * @param {string} repoFullName - * @param {string} attemptId - * @param {{ - * baseBranch?: string, cloneBaseDir?: string, env?: Record, - * exec?: import("@loopover/engine").WorktreeExecFn, timeoutMs?: number, - * remoteUrl?: string, runGit?: import("./repo-clone.js").RunGitFn, - * }} [options] - * @returns {Promise<{ ok: boolean, worktreePath?: string, branchName?: string, repoPath?: string, error?: string }>} */ export async function prepareAttemptWorktree(repoFullName, attemptId, options = {}) { - const cloneResult = await ensureRepoCloned(repoFullName, { - baseBranch: options.baseBranch, - cloneBaseDir: options.cloneBaseDir, - env: options.env, - timeoutMs: options.timeoutMs, - remoteUrl: options.remoteUrl, - runGit: options.runGit, - }); - if (!cloneResult.ok) return { ok: false, error: cloneResult.error ?? "ensure_repo_cloned_failed" }; - - const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); - const baseBranch = typeof options.baseBranch === "string" && options.baseBranch.trim() ? options.baseBranch.trim() : "main"; - const added = await addWorktree({ exec, repoPath: cloneResult.repoPath, baseBranch, attemptId }); - if (!added.ok) return { ok: false, repoPath: cloneResult.repoPath, error: added.error ?? "git_worktree_add_failed" }; - - return { ok: true, worktreePath: added.plan.worktreePath, branchName: added.plan.branchName, repoPath: cloneResult.repoPath }; + // Spread-omit rather than pass `undefined` explicitly -- EnsureRepoClonedOptions' optional fields don't + // declare `| undefined`, and exactOptionalPropertyTypes treats those as different. + const cloneResult = await ensureRepoCloned(repoFullName, { + ...(options.baseBranch !== undefined ? { baseBranch: options.baseBranch } : {}), + ...(options.cloneBaseDir !== undefined ? { cloneBaseDir: options.cloneBaseDir } : {}), + ...(options.env !== undefined ? { env: options.env } : {}), + ...(options.timeoutMs !== undefined ? { timeoutMs: options.timeoutMs } : {}), + ...(options.remoteUrl !== undefined ? { remoteUrl: options.remoteUrl } : {}), + ...(options.runGit !== undefined ? { runGit: options.runGit } : {}), + }); + // ensureRepoCloned's own EnsureRepoClonedResult declares `error` optional, but every one of its real ok:false + // return sites (repo-clone.ts) sets a real, non-empty error string -- a non-null assertion here rather than a + // fake fallback string, since that fallback would be genuinely unreachable dead code. + if (!cloneResult.ok) + return { ok: false, error: cloneResult.error }; + const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); + const baseBranch = typeof options.baseBranch === "string" && options.baseBranch.trim() ? options.baseBranch.trim() : "main"; + const added = await addWorktree({ exec, repoPath: cloneResult.repoPath, baseBranch, attemptId }); + // Same reasoning as above: the engine's addWorktree always sets a real error string (git stderr or a synthetic + // exit-code message) on ok:false -- see worktree-plan.ts's addWorktree. + if (!added.ok) + return { ok: false, repoPath: cloneResult.repoPath, error: added.error }; + return { ok: true, worktreePath: added.plan.worktreePath, branchName: added.plan.branchName, repoPath: cloneResult.repoPath }; } - /** * Tear down an attempt's worktree once the attempt concludes, per the engine's own retention policy: a * failed attempt's worktree is RETAINED for post-mortem inspection, a succeeded one is removed. - * - * @param {string} repoPath - * @param {string} worktreePath - * @param {boolean} attemptOk - * @param {{ exec?: import("@loopover/engine").WorktreeExecFn, timeoutMs?: number }} [options] - * @returns {Promise<{ ok: boolean, removed: boolean, error?: string }>} */ export async function cleanupAttemptWorktree(repoPath, worktreePath, attemptOk, options = {}) { - const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); - return removeWorktree({ exec, repoPath, worktreePath, retain: shouldRetainWorktree(attemptOk) }); + const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); + return removeWorktree({ exec, repoPath, worktreePath, retain: shouldRetainWorktree(attemptOk) }); } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiYXR0ZW1wdC13b3JrdHJlZS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbImF0dGVtcHQtd29ya3RyZWUudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsT0FBTyxFQUFFLEtBQUssRUFBRSxNQUFNLG9CQUFvQixDQUFDO0FBQzNDLE9BQU8sRUFBRSxXQUFXLEVBQUUsY0FBYyxFQUFFLG9CQUFvQixFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUFFckYsT0FBTyxFQUFFLGdCQUFnQixFQUFFLE1BQU0saUJBQWlCLENBQUM7QUFHbkQsMkdBQTJHO0FBQzNHLGdHQUFnRztBQUNoRyxxR0FBcUc7QUFDckcsMEdBQTBHO0FBQzFHLCtEQUErRDtBQUUvRCxNQUFNLGtCQUFrQixHQUFHLE9BQU8sQ0FBQztBQUVuQzs7Ozs7R0FLRztBQUNILE1BQU0sVUFBVSxzQkFBc0IsQ0FBQyxTQUFTLEdBQUcsa0JBQWtCO0lBQ25FLE9BQU8sQ0FBQyxHQUFHLEVBQUUsSUFBSSxFQUFFLElBQUksRUFBRSxFQUFFLENBQ3pCLElBQUksT0FBTyxDQUFDLENBQUMsT0FBTyxFQUFFLEVBQUU7UUFDdEIsTUFBTSxLQUFLLEdBQUcsS0FBSyxDQUFDLEdBQUcsRUFBRSxDQUFDLEdBQUcsSUFBSSxDQUFDLEVBQUUsRUFBRSxHQUFHLEVBQUUsSUFBSSxDQUFDLEdBQUcsRUFBRSxLQUFLLEVBQUUsQ0FBQyxRQUFRLEVBQUUsTUFBTSxFQUFFLE1BQU0sQ0FBQyxFQUFFLENBQUMsQ0FBQztRQUMxRixJQUFJLE1BQU0sR0FBRyxFQUFFLENBQUM7UUFDaEIsSUFBSSxNQUFNLEdBQUcsRUFBRSxDQUFDO1FBQ2hCLE1BQU0sS0FBSyxHQUFHLFVBQVUsQ0FBQyxHQUFHLEVBQUU7WUFDNUIsS0FBSyxDQUFDLElBQUksQ0FBQyxTQUFTLENBQUMsQ0FBQztZQUN0QixPQUFPLENBQUMsRUFBRSxJQUFJLEVBQUUsSUFBSSxFQUFFLE1BQU0sRUFBRSxNQUFNLEVBQUUsR0FBRyxNQUFNLHFCQUFxQixTQUFTLElBQUksQ0FBQyxJQUFJLEVBQUUsRUFBRSxDQUFDLENBQUM7UUFDOUYsQ0FBQyxFQUFFLFNBQVMsQ0FBQyxDQUFDO1FBQ2QsS0FBSyxDQUFDLE1BQU0sRUFBRSxFQUFFLENBQUMsTUFBTSxFQUFFLENBQUMsS0FBSyxFQUFFLEVBQUU7WUFDakMsTUFBTSxJQUFJLEtBQUssQ0FBQyxRQUFRLENBQUMsTUFBTSxDQUFDLENBQUM7UUFDbkMsQ0FBQyxDQUFDLENBQUM7UUFDSCxLQUFLLENBQUMsTUFBTSxFQUFFLEVBQUUsQ0FBQyxNQUFNLEVBQUUsQ0FBQyxLQUFLLEVBQUUsRUFBRTtZQUNqQyxNQUFNLElBQUksS0FBSyxDQUFDLFFBQVEsQ0FBQyxNQUFNLENBQUMsQ0FBQztRQUNuQyxDQUFDLENBQUMsQ0FBQztRQUNILEtBQUssQ0FBQyxFQUFFLENBQUMsT0FBTyxFQUFFLENBQUMsR0FBRyxFQUFFLEVBQUU7WUFDeEIsWUFBWSxDQUFDLEtBQUssQ0FBQyxDQUFDO1lBQ3BCLE9BQU8sQ0FBQyxFQUFFLElBQUksRUFBRSxJQUFJLEVBQUUsTUFBTSxFQUFFLE1BQU0sRUFBRSxHQUFHLENBQUMsT0FBTyxFQUFFLENBQUMsQ0FBQztRQUN2RCxDQUFDLENBQUMsQ0FBQztRQUNILEtBQUssQ0FBQyxFQUFFLENBQUMsT0FBTyxFQUFFLENBQUMsSUFBSSxFQUFFLEVBQUU7WUFDekIsWUFBWSxDQUFDLEtBQUssQ0FBQyxDQUFDO1lBQ3BCLE9BQU8sQ0FBQyxFQUFFLElBQUksRUFBRSxNQUFNLEVBQUUsTUFBTSxFQUFFLENBQUMsQ0FBQztRQUNwQyxDQUFDLENBQUMsQ0FBQztJQUNMLENBQUMsQ0FBQyxDQUFDO0FBQ1AsQ0FBQztBQWdCRDs7OztHQUlHO0FBQ0gsTUFBTSxDQUFDLEtBQUssVUFBVSxzQkFBc0IsQ0FDMUMsWUFBb0IsRUFDcEIsU0FBaUIsRUFDakIsVUFBeUMsRUFBRTtJQUUzQyx3R0FBd0c7SUFDeEcsbUZBQW1GO0lBQ25GLE1BQU0sV0FBVyxHQUFHLE1BQU0sZ0JBQWdCLENBQUMsWUFBWSxFQUFFO1FBQ3ZELEdBQUcsQ0FBQyxPQUFPLENBQUMsVUFBVSxLQUFLLFNBQVMsQ0FBQyxDQUFDLENBQUMsRUFBRSxVQUFVLEVBQUUsT0FBTyxDQUFDLFVBQVUsRUFBRSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7UUFDL0UsR0FBRyxDQUFDLE9BQU8sQ0FBQyxZQUFZLEtBQUssU0FBUyxDQUFDLENBQUMsQ0FBQyxFQUFFLFlBQVksRUFBRSxPQUFPLENBQUMsWUFBWSxFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQztRQUNyRixHQUFHLENBQUMsT0FBTyxDQUFDLEdBQUcsS0FBSyxTQUFTLENBQUMsQ0FBQyxDQUFDLEVBQUUsR0FBRyxFQUFFLE9BQU8sQ0FBQyxHQUFHLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDO1FBQzFELEdBQUcsQ0FBQyxPQUFPLENBQUMsU0FBUyxLQUFLLFNBQVMsQ0FBQyxDQUFDLENBQUMsRUFBRSxTQUFTLEVBQUUsT0FBTyxDQUFDLFNBQVMsRUFBRSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7UUFDNUUsR0FBRyxDQUFDLE9BQU8sQ0FBQyxTQUFTLEtBQUssU0FBUyxDQUFDLENBQUMsQ0FBQyxFQUFFLFNBQVMsRUFBRSxPQUFPLENBQUMsU0FBUyxFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQztRQUM1RSxHQUFHLENBQUMsT0FBTyxDQUFDLE1BQU0sS0FBSyxTQUFTLENBQUMsQ0FBQyxDQUFDLEVBQUUsTUFBTSxFQUFFLE9BQU8sQ0FBQyxNQUFNLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDO0tBQ3BFLENBQUMsQ0FBQztJQUNILDhHQUE4RztJQUM5Ryw4R0FBOEc7SUFDOUcsc0ZBQXNGO0lBQ3RGLElBQUksQ0FBQyxXQUFXLENBQUMsRUFBRTtRQUFFLE9BQU8sRUFBRSxFQUFFLEVBQUUsS0FBSyxFQUFFLEtBQUssRUFBRSxXQUFXLENBQUMsS0FBTSxFQUFFLENBQUM7SUFFckUsTUFBTSxJQUFJLEdBQUcsT0FBTyxDQUFDLElBQUksSUFBSSxzQkFBc0IsQ0FBQyxPQUFPLENBQUMsU0FBUyxDQUFDLENBQUM7SUFDdkUsTUFBTSxVQUFVLEdBQUcsT0FBTyxPQUFPLENBQUMsVUFBVSxLQUFLLFFBQVEsSUFBSSxPQUFPLENBQUMsVUFBVSxDQUFDLElBQUksRUFBRSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsVUFBVSxDQUFDLElBQUksRUFBRSxDQUFDLENBQUMsQ0FBQyxNQUFNLENBQUM7SUFDNUgsTUFBTSxLQUFLLEdBQUcsTUFBTSxXQUFXLENBQUMsRUFBRSxJQUFJLEVBQUUsUUFBUSxFQUFFLFdBQVcsQ0FBQyxRQUFRLEVBQUUsVUFBVSxFQUFFLFNBQVMsRUFBRSxDQUFDLENBQUM7SUFDakcsK0dBQStHO0lBQy9HLHdFQUF3RTtJQUN4RSxJQUFJLENBQUMsS0FBSyxDQUFDLEVBQUU7UUFBRSxPQUFPLEVBQUUsRUFBRSxFQUFFLEtBQUssRUFBRSxRQUFRLEVBQUUsV0FBVyxDQUFDLFFBQVEsRUFBRSxLQUFLLEVBQUUsS0FBSyxDQUFDLEtBQU0sRUFBRSxDQUFDO0lBRXpGLE9BQU8sRUFBRSxFQUFFLEVBQUUsSUFBSSxFQUFFLFlBQVksRUFBRSxLQUFLLENBQUMsSUFBSSxDQUFDLFlBQVksRUFBRSxVQUFVLEVBQUUsS0FBSyxDQUFDLElBQUksQ0FBQyxVQUFVLEVBQUUsUUFBUSxFQUFFLFdBQVcsQ0FBQyxRQUFRLEVBQUUsQ0FBQztBQUNoSSxDQUFDO0FBRUQ7OztHQUdHO0FBQ0gsTUFBTSxDQUFDLEtBQUssVUFBVSxzQkFBc0IsQ0FDMUMsUUFBZ0IsRUFDaEIsWUFBb0IsRUFDcEIsU0FBa0IsRUFDbEIsVUFBeUQsRUFBRTtJQUUzRCxNQUFNLElBQUksR0FBRyxPQUFPLENBQUMsSUFBSSxJQUFJLHNCQUFzQixDQUFDLE9BQU8sQ0FBQyxTQUFTLENBQUMsQ0FBQztJQUN2RSxPQUFPLGNBQWMsQ0FBQyxFQUFFLElBQUksRUFBRSxRQUFRLEVBQUUsWUFBWSxFQUFFLE1BQU0sRUFBRSxvQkFBb0IsQ0FBQyxTQUFTLENBQUMsRUFBRSxDQUFDLENBQUM7QUFDbkcsQ0FBQyJ9 \ No newline at end of file diff --git a/packages/loopover-miner/lib/attempt-worktree.ts b/packages/loopover-miner/lib/attempt-worktree.ts new file mode 100644 index 0000000000..ca17223a5e --- /dev/null +++ b/packages/loopover-miner/lib/attempt-worktree.ts @@ -0,0 +1,109 @@ +import { spawn } from "node:child_process"; +import { addWorktree, removeWorktree, shouldRetainWorktree } from "@loopover/engine"; +import type { WorktreeExecFn } from "@loopover/engine"; +import { ensureRepoCloned } from "./repo-clone.js"; +import type { RunGitFn } from "./repo-clone.js"; + +// Real attempt-worktree preparation (#5132, Wave 3.5 follow-up). Composes ensureRepoCloned (repo-clone.js, +// the missing base-clone-management step) with @loopover/engine's already-built, already-tested +// addWorktree/removeWorktree primitives -- which existed but were never called from this package, so +// `workingDirectory` handed to runIterateLoop was always just an empty directory with no real git repo in +// it. This is the caller that finally exercises them for real. + +const DEFAULT_TIMEOUT_MS = 120_000; + +/** + * Real child_process-backed implementation of the engine's WorktreeExecFn contract. Resolves (never + * rejects) on error/timeout, mirroring coding-agent-construction.js's createRealCliSubprocessSpawn -- a + * failed `git worktree add`'s stderr is the diagnosable signal, not something to lose to an unhandled + * rejection. + */ +export function createRealWorktreeExec(timeoutMs = DEFAULT_TIMEOUT_MS): WorktreeExecFn { + return (cmd, args, opts) => + new Promise((resolve) => { + const child = spawn(cmd, [...args], { cwd: opts.cwd, stdio: ["ignore", "pipe", "pipe"] }); + let stdout = ""; + let stderr = ""; + const timer = setTimeout(() => { + child.kill("SIGKILL"); + resolve({ code: null, stdout, stderr: `${stderr}\ntimed_out_after_${timeoutMs}ms`.trim() }); + }, timeoutMs); + child.stdout?.on("data", (chunk) => { + stdout += chunk.toString("utf8"); + }); + child.stderr?.on("data", (chunk) => { + stderr += chunk.toString("utf8"); + }); + child.on("error", (err) => { + clearTimeout(timer); + resolve({ code: null, stdout, stderr: err.message }); + }); + child.on("close", (code) => { + clearTimeout(timer); + resolve({ code, stdout, stderr }); + }); + }); +} + +export type PrepareAttemptWorktreeOptions = { + baseBranch?: string; + cloneBaseDir?: string; + env?: Record; + exec?: WorktreeExecFn; + timeoutMs?: number; + remoteUrl?: string; + runGit?: RunGitFn; +}; + +export type PrepareAttemptWorktreeResult = + | { ok: true; worktreePath: string; branchName: string; repoPath: string } + | { ok: false; repoPath?: string; error: string }; + +/** + * Prepare a real, isolated git worktree for one attempt: ensure the target repo's base clone exists and is + * current, then create a fresh `git worktree` off it on a deterministically-named branch. Fails closed + * (`ok: false`) on any step's failure rather than handing back a half-prepared directory. + */ +export async function prepareAttemptWorktree( + repoFullName: string, + attemptId: string, + options: PrepareAttemptWorktreeOptions = {}, +): Promise { + // Spread-omit rather than pass `undefined` explicitly -- EnsureRepoClonedOptions' optional fields don't + // declare `| undefined`, and exactOptionalPropertyTypes treats those as different. + const cloneResult = await ensureRepoCloned(repoFullName, { + ...(options.baseBranch !== undefined ? { baseBranch: options.baseBranch } : {}), + ...(options.cloneBaseDir !== undefined ? { cloneBaseDir: options.cloneBaseDir } : {}), + ...(options.env !== undefined ? { env: options.env } : {}), + ...(options.timeoutMs !== undefined ? { timeoutMs: options.timeoutMs } : {}), + ...(options.remoteUrl !== undefined ? { remoteUrl: options.remoteUrl } : {}), + ...(options.runGit !== undefined ? { runGit: options.runGit } : {}), + }); + // ensureRepoCloned's own EnsureRepoClonedResult declares `error` optional, but every one of its real ok:false + // return sites (repo-clone.ts) sets a real, non-empty error string -- a non-null assertion here rather than a + // fake fallback string, since that fallback would be genuinely unreachable dead code. + if (!cloneResult.ok) return { ok: false, error: cloneResult.error! }; + + const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); + const baseBranch = typeof options.baseBranch === "string" && options.baseBranch.trim() ? options.baseBranch.trim() : "main"; + const added = await addWorktree({ exec, repoPath: cloneResult.repoPath, baseBranch, attemptId }); + // Same reasoning as above: the engine's addWorktree always sets a real error string (git stderr or a synthetic + // exit-code message) on ok:false -- see worktree-plan.ts's addWorktree. + if (!added.ok) return { ok: false, repoPath: cloneResult.repoPath, error: added.error! }; + + return { ok: true, worktreePath: added.plan.worktreePath, branchName: added.plan.branchName, repoPath: cloneResult.repoPath }; +} + +/** + * Tear down an attempt's worktree once the attempt concludes, per the engine's own retention policy: a + * failed attempt's worktree is RETAINED for post-mortem inspection, a succeeded one is removed. + */ +export async function cleanupAttemptWorktree( + repoPath: string, + worktreePath: string, + attemptOk: boolean, + options: { exec?: WorktreeExecFn; timeoutMs?: number } = {}, +): Promise<{ ok: boolean; removed: boolean; error?: string }> { + const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); + return removeWorktree({ exec, repoPath, worktreePath, retain: shouldRetainWorktree(attemptOk) }); +} diff --git a/packages/loopover-miner/lib/claim-adjudication.d.ts b/packages/loopover-miner/lib/claim-adjudication.d.ts index e195bc19aa..f612e501e4 100644 --- a/packages/loopover-miner/lib/claim-adjudication.d.ts +++ b/packages/loopover-miner/lib/claim-adjudication.d.ts @@ -1,21 +1,29 @@ +import type { DuplicateClaimMember } from "@loopover/engine"; /** An observed claim on an issue: a PR/claimant number plus when it claimed the linked issue (if known). */ export type ObservedClaim = { - number: number; - claimedAt?: string | null | undefined; + number: number; + claimedAt?: string | null | undefined; }; - /** The engine `DuplicateClaimMember` shape this module bridges an {@link ObservedClaim} to. */ -export type ClaimMember = { - number: number; - linkedIssueClaimedAt: string | null; -}; - +export type ClaimMember = DuplicateClaimMember; /** The adjudication result: the go/no-go `isWinner`, plus a DISPLAY-only `winnerNumber` (null when not determinable). */ export type ClaimAdjudication = { - isWinner: boolean; - winnerNumber: number | null; + isWinner: boolean; + winnerNumber: number | null; }; - -export function toClaimMember(claim: ObservedClaim): ClaimMember; - -export function adjudicateSoftClaim(self: ObservedClaim, competing?: readonly ObservedClaim[]): ClaimAdjudication; +/** + * Map an observed claim record to the engine's `DuplicateClaimMember`. The field names deliberately DIFFER — the + * local ledger / observed data expose `claimedAt`, the engine election reads `linkedIssueClaimedAt` — so the bridge + * is explicit (they are not interchangeable by accident of naming). `createdAt` is intentionally omitted: the + * election ignores it (an older PR can claim a linked issue later by editing its body). Pure. + */ +export declare function toClaimMember(claim: ObservedClaim): ClaimMember; +/** + * Adjudicate whether THIS miner's soft-claim wins a contested issue. `self` is this miner's claim and `competing` + * is the publicly-observable set of OTHER open PRs linking the same issue; each entry is `{ number, claimedAt }`. + * Returns the go/no-go `isWinner` (driven ONLY by `isDuplicateClusterWinnerByClaim`) plus a DISPLAY-only + * `winnerNumber` (from `resolveDuplicateClusterWinnerNumber`, for surfacing "you lost this claim to PR #N" to the + * operator — never for the decision). Pure — no IO. Fail-closed: a missing/sparse claim time loses; the winner is + * `null` when the ordering is too sparse to be sure (it never guesses). An empty `competing` list ⇒ trivial winner. + */ +export declare function adjudicateSoftClaim(self: ObservedClaim, competing?: readonly ObservedClaim[]): ClaimAdjudication; diff --git a/packages/loopover-miner/lib/claim-adjudication.js b/packages/loopover-miner/lib/claim-adjudication.js index f063c40eb8..42b85246a2 100644 --- a/packages/loopover-miner/lib/claim-adjudication.js +++ b/packages/loopover-miner/lib/claim-adjudication.js @@ -7,7 +7,6 @@ // PRs linking it IS the public signal of a contested claim). The caller assembles that set — exactly like the // maintainer-side callers in src/ do — and passes it here. import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "@loopover/engine"; - /** * Map an observed claim record to the engine's `DuplicateClaimMember`. The field names deliberately DIFFER — the * local ledger / observed data expose `claimedAt`, the engine election reads `linkedIssueClaimedAt` — so the bridge @@ -15,9 +14,8 @@ import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } * election ignores it (an older PR can claim a linked issue later by editing its body). Pure. */ export function toClaimMember(claim) { - return { number: claim.number, linkedIssueClaimedAt: claim.claimedAt ?? null }; + return { number: claim.number, linkedIssueClaimedAt: claim.claimedAt ?? null }; } - /** * Adjudicate whether THIS miner's soft-claim wins a contested issue. `self` is this miner's claim and `competing` * is the publicly-observable set of OTHER open PRs linking the same issue; each entry is `{ number, claimedAt }`. @@ -27,10 +25,11 @@ export function toClaimMember(claim) { * `null` when the ordering is too sparse to be sure (it never guesses). An empty `competing` list ⇒ trivial winner. */ export function adjudicateSoftClaim(self, competing = []) { - const selfMember = toClaimMember(self); - const siblings = competing.map(toClaimMember); - return { - isWinner: isDuplicateClusterWinnerByClaim(selfMember, siblings), - winnerNumber: resolveDuplicateClusterWinnerNumber(selfMember, siblings), - }; + const selfMember = toClaimMember(self); + const siblings = competing.map(toClaimMember); + return { + isWinner: isDuplicateClusterWinnerByClaim(selfMember, siblings), + winnerNumber: resolveDuplicateClusterWinnerNumber(selfMember, siblings), + }; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiY2xhaW0tYWRqdWRpY2F0aW9uLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiY2xhaW0tYWRqdWRpY2F0aW9uLnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLG9IQUFvSDtBQUNwSCx1R0FBdUc7QUFDdkcsb0hBQW9IO0FBQ3BILEVBQUU7QUFDRixnSEFBZ0g7QUFDaEgsa0hBQWtIO0FBQ2xILDhHQUE4RztBQUM5RywyREFBMkQ7QUFDM0QsT0FBTyxFQUFFLCtCQUErQixFQUFFLG1DQUFtQyxFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUFrQnhHOzs7OztHQUtHO0FBQ0gsTUFBTSxVQUFVLGFBQWEsQ0FBQyxLQUFvQjtJQUNoRCxPQUFPLEVBQUUsTUFBTSxFQUFFLEtBQUssQ0FBQyxNQUFNLEVBQUUsb0JBQW9CLEVBQUUsS0FBSyxDQUFDLFNBQVMsSUFBSSxJQUFJLEVBQUUsQ0FBQztBQUNqRixDQUFDO0FBRUQ7Ozs7Ozs7R0FPRztBQUNILE1BQU0sVUFBVSxtQkFBbUIsQ0FBQyxJQUFtQixFQUFFLFlBQXNDLEVBQUU7SUFDL0YsTUFBTSxVQUFVLEdBQUcsYUFBYSxDQUFDLElBQUksQ0FBQyxDQUFDO0lBQ3ZDLE1BQU0sUUFBUSxHQUFHLFNBQVMsQ0FBQyxHQUFHLENBQUMsYUFBYSxDQUFDLENBQUM7SUFDOUMsT0FBTztRQUNMLFFBQVEsRUFBRSwrQkFBK0IsQ0FBQyxVQUFVLEVBQUUsUUFBUSxDQUFDO1FBQy9ELFlBQVksRUFBRSxtQ0FBbUMsQ0FBQyxVQUFVLEVBQUUsUUFBUSxDQUFDO0tBQ3hFLENBQUM7QUFDSixDQUFDIn0= \ No newline at end of file diff --git a/packages/loopover-miner/lib/claim-adjudication.ts b/packages/loopover-miner/lib/claim-adjudication.ts new file mode 100644 index 0000000000..5e2e7e76b0 --- /dev/null +++ b/packages/loopover-miner/lib/claim-adjudication.ts @@ -0,0 +1,52 @@ +// Soft-claim adjudication (#4291). Decides which of several miners claiming the same issue proceeds, by REUSING the +// maintainer-side duplicate-cluster election (`isDuplicateClusterWinnerByClaim` from @loopover/engine) +// rather than reimplementing it — so the miner and the maintainer gate agree on exactly one winner by construction. +// +// The local claim ledger is 100% client-side and cannot see other miners' claims, so the competing-claim signal +// must come from something publicly observable: the OPEN PRs that link the same issue (an issue with several open +// PRs linking it IS the public signal of a contested claim). The caller assembles that set — exactly like the +// maintainer-side callers in src/ do — and passes it here. +import { isDuplicateClusterWinnerByClaim, resolveDuplicateClusterWinnerNumber } from "@loopover/engine"; +import type { DuplicateClaimMember } from "@loopover/engine"; + +/** An observed claim on an issue: a PR/claimant number plus when it claimed the linked issue (if known). */ +export type ObservedClaim = { + number: number; + claimedAt?: string | null | undefined; +}; + +/** The engine `DuplicateClaimMember` shape this module bridges an {@link ObservedClaim} to. */ +export type ClaimMember = DuplicateClaimMember; + +/** The adjudication result: the go/no-go `isWinner`, plus a DISPLAY-only `winnerNumber` (null when not determinable). */ +export type ClaimAdjudication = { + isWinner: boolean; + winnerNumber: number | null; +}; + +/** + * Map an observed claim record to the engine's `DuplicateClaimMember`. The field names deliberately DIFFER — the + * local ledger / observed data expose `claimedAt`, the engine election reads `linkedIssueClaimedAt` — so the bridge + * is explicit (they are not interchangeable by accident of naming). `createdAt` is intentionally omitted: the + * election ignores it (an older PR can claim a linked issue later by editing its body). Pure. + */ +export function toClaimMember(claim: ObservedClaim): ClaimMember { + return { number: claim.number, linkedIssueClaimedAt: claim.claimedAt ?? null }; +} + +/** + * Adjudicate whether THIS miner's soft-claim wins a contested issue. `self` is this miner's claim and `competing` + * is the publicly-observable set of OTHER open PRs linking the same issue; each entry is `{ number, claimedAt }`. + * Returns the go/no-go `isWinner` (driven ONLY by `isDuplicateClusterWinnerByClaim`) plus a DISPLAY-only + * `winnerNumber` (from `resolveDuplicateClusterWinnerNumber`, for surfacing "you lost this claim to PR #N" to the + * operator — never for the decision). Pure — no IO. Fail-closed: a missing/sparse claim time loses; the winner is + * `null` when the ordering is too sparse to be sure (it never guesses). An empty `competing` list ⇒ trivial winner. + */ +export function adjudicateSoftClaim(self: ObservedClaim, competing: readonly ObservedClaim[] = []): ClaimAdjudication { + const selfMember = toClaimMember(self); + const siblings = competing.map(toClaimMember); + return { + isWinner: isDuplicateClusterWinnerByClaim(selfMember, siblings), + winnerNumber: resolveDuplicateClusterWinnerNumber(selfMember, siblings), + }; +} diff --git a/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts b/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts index e98c9c34b7..d63f3f5581 100644 --- a/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts +++ b/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts @@ -1,18 +1,9 @@ import type { GovernorChokepointInput } from "@loopover/engine"; -import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; import type { EvaluateGovernorChokepointGateResult } from "./governor-chokepoint.js"; import type { GovernorState } from "./governor-state.js"; - -// rateLimitBuckets/rateLimitBackoffAttempts/capUsage are required on GovernorChokepointInput itself, but this -// wrapper auto-supplies them from persisted state when the caller omits them -- loosen just those three to -// optional so a caller that WANTS the persisted defaults doesn't have to fake a value just to satisfy the type. -export type GovernorChokepointInputPersisted = Omit & - Partial>; - -export function evaluateGovernorChokepointGatePersisted( - input: GovernorChokepointInputPersisted, - options?: { +import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; +export type GovernorChokepointInputPersisted = Omit & Partial>; +export declare function evaluateGovernorChokepointGatePersisted(input: GovernorChokepointInputPersisted, options?: { governorState?: GovernorState; append?: (event: AppendGovernorEventInput) => GovernorLedgerEntry; - }, -): EvaluateGovernorChokepointGateResult; +}): EvaluateGovernorChokepointGateResult; diff --git a/packages/loopover-miner/lib/governor-chokepoint-persisted.js b/packages/loopover-miner/lib/governor-chokepoint-persisted.js index d7443575b5..e2fe86a39e 100644 --- a/packages/loopover-miner/lib/governor-chokepoint-persisted.js +++ b/packages/loopover-miner/lib/governor-chokepoint-persisted.js @@ -1,46 +1,25 @@ import { evaluateGovernorChokepointGate } from "./governor-chokepoint.js"; import { openGovernorState } from "./governor-state.js"; - -// The real cross-attempt integration point for #5134: composes governor-chokepoint.js's existing, UNMODIFIED -// evaluateGovernorChokepointGate (still exactly as pure-per-call as before -- every existing caller/test of -// it is untouched) with governor-state.js's persistence, so attempt N+1's decision actually sees attempt N's -// rate-limit/backoff outcome. Kept as a separate composing function rather than changing -// evaluateGovernorChokepointGate itself: this issue is flagged as the safety-critical core of its gap-fill -// batch, and a caller-controlled wrapper is a smaller, more isolated surface to review than a behavior change -// to an already-relied-upon function. -// -// capUsage is LOADED here (so a caller that doesn't track its own running totals still gets real prior state -// instead of silently starting from zero every call) but deliberately NOT saved here: budget-cap.ts's -// GovernorCapUsage has no mutator (unlike write-rate-limit.ts's buckets/backoff, nothing computes "the next -// capUsage" from a verdict -- the caller is the only one who knows how much THIS attempt actually spent, -// which isn't known until after the attempt runs, not at the gate-check moment). Saving the next capUsage is -// the caller's job via `saveCapUsage` once the attempt's real spend/turns/elapsed are known. - -/** - * @param {import("./governor-chokepoint-persisted.js").GovernorChokepointInputPersisted} input - * @param {{ - * governorState?: import("./governor-state.js").GovernorState, - * append?: (event: unknown) => unknown, - * }} [options] - * @returns {import("./governor-chokepoint.js").EvaluateGovernorChokepointGateResult} - */ export function evaluateGovernorChokepointGatePersisted(input, options = {}) { - const ownsGovernorState = options.governorState === undefined; - const governorState = options.governorState ?? openGovernorState(); - try { - const persistedRateLimit = governorState.loadRateLimitState(); - const persistedCapUsage = governorState.loadCapUsage(); - const resolvedInput = { - ...input, - rateLimitBuckets: input.rateLimitBuckets ?? persistedRateLimit.buckets, - rateLimitBackoffAttempts: input.rateLimitBackoffAttempts ?? persistedRateLimit.backoffAttempts, - capUsage: input.capUsage ?? persistedCapUsage, - }; - const gateOptions = options.append === undefined ? {} : { append: options.append }; - const result = evaluateGovernorChokepointGate(resolvedInput, gateOptions); - governorState.saveRateLimitState({ buckets: result.rateLimitBuckets, backoffAttempts: result.rateLimitBackoffAttempts }); - return result; - } finally { - if (ownsGovernorState) governorState.close(); - } + const ownsGovernorState = options.governorState === undefined; + const governorState = options.governorState ?? openGovernorState(); + try { + const persistedRateLimit = governorState.loadRateLimitState(); + const persistedCapUsage = governorState.loadCapUsage(); + const resolvedInput = { + ...input, + rateLimitBuckets: input.rateLimitBuckets ?? persistedRateLimit.buckets, + rateLimitBackoffAttempts: input.rateLimitBackoffAttempts ?? persistedRateLimit.backoffAttempts, + capUsage: input.capUsage ?? persistedCapUsage, + }; + const gateOptions = options.append === undefined ? {} : { append: options.append }; + const result = evaluateGovernorChokepointGate(resolvedInput, gateOptions); + governorState.saveRateLimitState({ buckets: result.rateLimitBuckets, backoffAttempts: result.rateLimitBackoffAttempts }); + return result; + } + finally { + if (ownsGovernorState) + governorState.close(); + } } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZ292ZXJub3ItY2hva2Vwb2ludC1wZXJzaXN0ZWQuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJnb3Zlcm5vci1jaG9rZXBvaW50LXBlcnNpc3RlZC50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFDQSxPQUFPLEVBQUUsOEJBQThCLEVBQUUsTUFBTSwwQkFBMEIsQ0FBQztBQUUxRSxPQUFPLEVBQUUsaUJBQWlCLEVBQUUsTUFBTSxxQkFBcUIsQ0FBQztBQTRCeEQsTUFBTSxVQUFVLHVDQUF1QyxDQUNyRCxLQUF1QyxFQUN2QyxVQUdJLEVBQUU7SUFFTixNQUFNLGlCQUFpQixHQUFHLE9BQU8sQ0FBQyxhQUFhLEtBQUssU0FBUyxDQUFDO0lBQzlELE1BQU0sYUFBYSxHQUFHLE9BQU8sQ0FBQyxhQUFhLElBQUksaUJBQWlCLEVBQUUsQ0FBQztJQUNuRSxJQUFJLENBQUM7UUFDSCxNQUFNLGtCQUFrQixHQUFHLGFBQWEsQ0FBQyxrQkFBa0IsRUFBRSxDQUFDO1FBQzlELE1BQU0saUJBQWlCLEdBQUcsYUFBYSxDQUFDLFlBQVksRUFBRSxDQUFDO1FBQ3ZELE1BQU0sYUFBYSxHQUE0QjtZQUM3QyxHQUFHLEtBQUs7WUFDUixnQkFBZ0IsRUFBRSxLQUFLLENBQUMsZ0JBQWdCLElBQUksa0JBQWtCLENBQUMsT0FBTztZQUN0RSx3QkFBd0IsRUFBRSxLQUFLLENBQUMsd0JBQXdCLElBQUksa0JBQWtCLENBQUMsZUFBZTtZQUM5RixRQUFRLEVBQUUsS0FBSyxDQUFDLFFBQVEsSUFBSSxpQkFBaUI7U0FDOUMsQ0FBQztRQUNGLE1BQU0sV0FBVyxHQUFHLE9BQU8sQ0FBQyxNQUFNLEtBQUssU0FBUyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsTUFBTSxFQUFFLE9BQU8sQ0FBQyxNQUFNLEVBQUUsQ0FBQztRQUNuRixNQUFNLE1BQU0sR0FBRyw4QkFBOEIsQ0FBQyxhQUFhLEVBQUUsV0FBVyxDQUFDLENBQUM7UUFDMUUsYUFBYSxDQUFDLGtCQUFrQixDQUFDLEVBQUUsT0FBTyxFQUFFLE1BQU0sQ0FBQyxnQkFBZ0IsRUFBRSxlQUFlLEVBQUUsTUFBTSxDQUFDLHdCQUF3QixFQUFFLENBQUMsQ0FBQztRQUN6SCxPQUFPLE1BQU0sQ0FBQztJQUNoQixDQUFDO1lBQVMsQ0FBQztRQUNULElBQUksaUJBQWlCO1lBQUUsYUFBYSxDQUFDLEtBQUssRUFBRSxDQUFDO0lBQy9DLENBQUM7QUFDSCxDQUFDIn0= \ No newline at end of file diff --git a/packages/loopover-miner/lib/governor-chokepoint-persisted.ts b/packages/loopover-miner/lib/governor-chokepoint-persisted.ts new file mode 100644 index 0000000000..24295d75dd --- /dev/null +++ b/packages/loopover-miner/lib/governor-chokepoint-persisted.ts @@ -0,0 +1,57 @@ +import type { GovernorChokepointInput } from "@loopover/engine"; +import { evaluateGovernorChokepointGate } from "./governor-chokepoint.js"; +import type { EvaluateGovernorChokepointGateResult } from "./governor-chokepoint.js"; +import { openGovernorState } from "./governor-state.js"; +import type { GovernorState } from "./governor-state.js"; +import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; + +// The real cross-attempt integration point for #5134: composes governor-chokepoint.js's existing, UNMODIFIED +// evaluateGovernorChokepointGate (still exactly as pure-per-call as before -- every existing caller/test of +// it is untouched) with governor-state.js's persistence, so attempt N+1's decision actually sees attempt N's +// rate-limit/backoff outcome. Kept as a separate composing function rather than changing +// evaluateGovernorChokepointGate itself: this issue is flagged as the safety-critical core of its gap-fill +// batch, and a caller-controlled wrapper is a smaller, more isolated surface to review than a behavior change +// to an already-relied-upon function. +// +// capUsage is LOADED here (so a caller that doesn't track its own running totals still gets real prior state +// instead of silently starting from zero every call) but deliberately NOT saved here: budget-cap.ts's +// GovernorCapUsage has no mutator (unlike write-rate-limit.ts's buckets/backoff, nothing computes "the next +// capUsage" from a verdict -- the caller is the only one who knows how much THIS attempt actually spent, +// which isn't known until after the attempt runs, not at the gate-check moment). Saving the next capUsage is +// the caller's job via `saveCapUsage` once the attempt's real spend/turns/elapsed are known. + +// rateLimitBuckets/rateLimitBackoffAttempts/capUsage are required on GovernorChokepointInput itself, but this +// wrapper auto-supplies them from persisted state when the caller omits them -- loosen just those three to +// optional so a caller that WANTS the persisted defaults doesn't have to fake a value just to satisfy the type. +export type GovernorChokepointInputPersisted = Omit< + GovernorChokepointInput, + "rateLimitBuckets" | "rateLimitBackoffAttempts" | "capUsage" +> & + Partial>; + +export function evaluateGovernorChokepointGatePersisted( + input: GovernorChokepointInputPersisted, + options: { + governorState?: GovernorState; + append?: (event: AppendGovernorEventInput) => GovernorLedgerEntry; + } = {}, +): EvaluateGovernorChokepointGateResult { + const ownsGovernorState = options.governorState === undefined; + const governorState = options.governorState ?? openGovernorState(); + try { + const persistedRateLimit = governorState.loadRateLimitState(); + const persistedCapUsage = governorState.loadCapUsage(); + const resolvedInput: GovernorChokepointInput = { + ...input, + rateLimitBuckets: input.rateLimitBuckets ?? persistedRateLimit.buckets, + rateLimitBackoffAttempts: input.rateLimitBackoffAttempts ?? persistedRateLimit.backoffAttempts, + capUsage: input.capUsage ?? persistedCapUsage, + }; + const gateOptions = options.append === undefined ? {} : { append: options.append }; + const result = evaluateGovernorChokepointGate(resolvedInput, gateOptions); + governorState.saveRateLimitState({ buckets: result.rateLimitBuckets, backoffAttempts: result.rateLimitBackoffAttempts }); + return result; + } finally { + if (ownsGovernorState) governorState.close(); + } +} diff --git a/packages/loopover-miner/lib/idea-feasibility.d.ts b/packages/loopover-miner/lib/idea-feasibility.d.ts index 9f468155d0..e9fafcdba1 100644 --- a/packages/loopover-miner/lib/idea-feasibility.d.ts +++ b/packages/loopover-miner/lib/idea-feasibility.d.ts @@ -1,51 +1,37 @@ -import type { - FeasibilityClaimStatus, - FeasibilityDuplicateClusterRisk, - FeasibilityGateInput, - FeasibilityGateResult, - FeasibilityIssueStatus, - FeasibilityVerdict, -} from "@loopover/engine"; - +import type { FeasibilityClaimStatus, FeasibilityDuplicateClusterRisk, FeasibilityGateInput, FeasibilityGateResult, FeasibilityIssueStatus, FeasibilityVerdict } from "@loopover/engine"; /** A schema-validated idea submission (#4779). This structural gate only reads `acceptanceHints`, but accepts * the full submission so callers can pass the idea through unchanged. */ export type IdeaFeasibilityInput = { - title?: string | undefined; - body?: string | undefined; - targetRepo?: string | undefined; - constraints?: readonly string[] | undefined; - acceptanceHints?: readonly string[] | undefined; - priority?: "normal" | "high" | undefined; + title?: string | undefined; + body?: string | undefined; + targetRepo?: string | undefined; + constraints?: readonly string[] | undefined; + acceptanceHints?: readonly string[] | undefined; + priority?: "normal" | "high" | undefined; }; - /** Objectively-resolved intake signals for the idea (resolved by the caller, never guessed from prose). */ export type ResolvedIdeaSignals = { - targetResolvable: boolean; - claimStatus: FeasibilityClaimStatus; - duplicateClusterRisk: FeasibilityDuplicateClusterRisk; + targetResolvable: boolean; + claimStatus: FeasibilityClaimStatus; + duplicateClusterRisk: FeasibilityDuplicateClusterRisk; }; - export type AssessIdeaFeasibilityOptions = { - buildFeasibilityVerdict?: (input: FeasibilityGateInput) => FeasibilityGateResult; + buildFeasibilityVerdict?: (input: FeasibilityGateInput) => FeasibilityGateResult; }; - export type IdeaFeasibilityDisposition = "proceed" | "flag" | "reject"; - export type IdeaFeasibilityResult = { - disposition: IdeaFeasibilityDisposition; - verdict: FeasibilityVerdict; - issueStatus: FeasibilityIssueStatus; - reasons: string[]; - summary: string; + disposition: IdeaFeasibilityDisposition; + verdict: FeasibilityVerdict; + issueStatus: FeasibilityIssueStatus; + reasons: string[]; + summary: string; }; - -export function deriveIdeaIssueStatus( - idea: IdeaFeasibilityInput, - resolved: Pick, -): FeasibilityIssueStatus; - -export function assessIdeaFeasibility( - idea: IdeaFeasibilityInput, - resolved: ResolvedIdeaSignals, - options?: AssessIdeaFeasibilityOptions, -): IdeaFeasibilityResult; +/** + * Derive the feasibility `issueStatus` for a freeform idea from objective, structural signals only — never from + * a semantic read of the prose. + */ +export declare function deriveIdeaIssueStatus(idea: IdeaFeasibilityInput, resolved: Pick): FeasibilityIssueStatus; +/** + * Assess a schema-validated idea's feasibility before compute is allocated. + */ +export declare function assessIdeaFeasibility(idea: IdeaFeasibilityInput, resolved: ResolvedIdeaSignals, options?: AssessIdeaFeasibilityOptions): IdeaFeasibilityResult; diff --git a/packages/loopover-miner/lib/idea-feasibility.js b/packages/loopover-miner/lib/idea-feasibility.js index 0fed105d9c..03cf768a2f 100644 --- a/packages/loopover-miner/lib/idea-feasibility.js +++ b/packages/loopover-miner/lib/idea-feasibility.js @@ -20,53 +20,46 @@ * a content-moderation policy call, not this deterministic structural gate. */ import { buildFeasibilityVerdict } from "@loopover/engine"; - /** Verdict → caller-facing disposition. `go` proceeds to compute; `raise`/`avoid` gate it. */ -const DISPOSITION_BY_VERDICT = { go: "proceed", raise: "flag", avoid: "reject" }; - +const DISPOSITION_BY_VERDICT = { + go: "proceed", + raise: "flag", + avoid: "reject", +}; /** * Derive the feasibility `issueStatus` for a freeform idea from objective, structural signals only — never from * a semantic read of the prose. - * - * @param {{ acceptanceHints?: readonly string[] }} idea schema-validated idea submission (#4779) - * @param {{ targetResolvable: boolean }} resolved objectively-resolved intake signals - * @returns {"missing" | "invalid" | "ready"} */ export function deriveIdeaIssueStatus(idea, resolved) { - // Out of the loop's scope: the idea does not resolve to a repo the loop can act on. - if (!resolved.targetResolvable) return "missing"; - // Impossible to evaluate objectively: no declared success signal, so the loop could never test its own output. - // Count CONTENT, not array length (#6766): a blank/whitespace-only hint declares nothing testable, so it must - // not pass as an objective signal just by occupying a slot. - const objectiveSignals = (idea.acceptanceHints ?? []).filter( - (hint) => typeof hint === "string" && hint.trim() !== "", - ).length; - if (objectiveSignals === 0) return "invalid"; - return "ready"; + // Out of the loop's scope: the idea does not resolve to a repo the loop can act on. + if (!resolved.targetResolvable) + return "missing"; + // Impossible to evaluate objectively: no declared success signal, so the loop could never test its own output. + // Count CONTENT, not array length (#6766): a blank/whitespace-only hint declares nothing testable, so it must + // not pass as an objective signal just by occupying a slot. + const objectiveSignals = (idea.acceptanceHints ?? []).filter((hint) => typeof hint === "string" && hint.trim() !== "").length; + if (objectiveSignals === 0) + return "invalid"; + return "ready"; } - /** * Assess a schema-validated idea's feasibility before compute is allocated. - * - * @param {{ acceptanceHints?: readonly string[] }} idea - * @param {{ targetResolvable: boolean, claimStatus: string, duplicateClusterRisk: string }} resolved - * @param {{ buildFeasibilityVerdict?: Function }} [options] test seam; defaults to the engine composer - * @returns {{ disposition: "proceed"|"flag"|"reject", verdict: string, issueStatus: string, reasons: string[], summary: string }} */ export function assessIdeaFeasibility(idea, resolved, options = {}) { - const buildVerdict = options.buildFeasibilityVerdict ?? buildFeasibilityVerdict; - const issueStatus = deriveIdeaIssueStatus(idea, resolved); - const verdict = buildVerdict({ - found: resolved.targetResolvable, - claimStatus: resolved.claimStatus, - duplicateClusterRisk: resolved.duplicateClusterRisk, - issueStatus, - }); - return { - disposition: DISPOSITION_BY_VERDICT[verdict.verdict], - verdict: verdict.verdict, - issueStatus, - reasons: [...verdict.avoidReasons, ...verdict.raiseReasons], - summary: verdict.summary, - }; + const buildVerdict = options.buildFeasibilityVerdict ?? buildFeasibilityVerdict; + const issueStatus = deriveIdeaIssueStatus(idea, resolved); + const verdict = buildVerdict({ + found: resolved.targetResolvable, + claimStatus: resolved.claimStatus, + duplicateClusterRisk: resolved.duplicateClusterRisk, + issueStatus, + }); + return { + disposition: DISPOSITION_BY_VERDICT[verdict.verdict], + verdict: verdict.verdict, + issueStatus, + reasons: [...verdict.avoidReasons, ...verdict.raiseReasons], + summary: verdict.summary, + }; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaWRlYS1mZWFzaWJpbGl0eS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbImlkZWEtZmVhc2liaWxpdHkudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUE7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7O0dBb0JHO0FBQ0gsT0FBTyxFQUFFLHVCQUF1QixFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUEwQzNELDhGQUE4RjtBQUM5RixNQUFNLHNCQUFzQixHQUEyRDtJQUNyRixFQUFFLEVBQUUsU0FBUztJQUNiLEtBQUssRUFBRSxNQUFNO0lBQ2IsS0FBSyxFQUFFLFFBQVE7Q0FDaEIsQ0FBQztBQUVGOzs7R0FHRztBQUNILE1BQU0sVUFBVSxxQkFBcUIsQ0FDbkMsSUFBMEIsRUFDMUIsUUFBdUQ7SUFFdkQsb0ZBQW9GO0lBQ3BGLElBQUksQ0FBQyxRQUFRLENBQUMsZ0JBQWdCO1FBQUUsT0FBTyxTQUFTLENBQUM7SUFDakQsK0dBQStHO0lBQy9HLDhHQUE4RztJQUM5Ryw0REFBNEQ7SUFDNUQsTUFBTSxnQkFBZ0IsR0FBRyxDQUFDLElBQUksQ0FBQyxlQUFlLElBQUksRUFBRSxDQUFDLENBQUMsTUFBTSxDQUMxRCxDQUFDLElBQUksRUFBRSxFQUFFLENBQUMsT0FBTyxJQUFJLEtBQUssUUFBUSxJQUFJLElBQUksQ0FBQyxJQUFJLEVBQUUsS0FBSyxFQUFFLENBQ3pELENBQUMsTUFBTSxDQUFDO0lBQ1QsSUFBSSxnQkFBZ0IsS0FBSyxDQUFDO1FBQUUsT0FBTyxTQUFTLENBQUM7SUFDN0MsT0FBTyxPQUFPLENBQUM7QUFDakIsQ0FBQztBQUVEOztHQUVHO0FBQ0gsTUFBTSxVQUFVLHFCQUFxQixDQUNuQyxJQUEwQixFQUMxQixRQUE2QixFQUM3QixVQUF3QyxFQUFFO0lBRTFDLE1BQU0sWUFBWSxHQUFHLE9BQU8sQ0FBQyx1QkFBdUIsSUFBSSx1QkFBdUIsQ0FBQztJQUNoRixNQUFNLFdBQVcsR0FBRyxxQkFBcUIsQ0FBQyxJQUFJLEVBQUUsUUFBUSxDQUFDLENBQUM7SUFDMUQsTUFBTSxPQUFPLEdBQUcsWUFBWSxDQUFDO1FBQzNCLEtBQUssRUFBRSxRQUFRLENBQUMsZ0JBQWdCO1FBQ2hDLFdBQVcsRUFBRSxRQUFRLENBQUMsV0FBVztRQUNqQyxvQkFBb0IsRUFBRSxRQUFRLENBQUMsb0JBQW9CO1FBQ25ELFdBQVc7S0FDWixDQUFDLENBQUM7SUFDSCxPQUFPO1FBQ0wsV0FBVyxFQUFFLHNCQUFzQixDQUFDLE9BQU8sQ0FBQyxPQUFPLENBQUM7UUFDcEQsT0FBTyxFQUFFLE9BQU8sQ0FBQyxPQUFPO1FBQ3hCLFdBQVc7UUFDWCxPQUFPLEVBQUUsQ0FBQyxHQUFHLE9BQU8sQ0FBQyxZQUFZLEVBQUUsR0FBRyxPQUFPLENBQUMsWUFBWSxDQUFDO1FBQzNELE9BQU8sRUFBRSxPQUFPLENBQUMsT0FBTztLQUN6QixDQUFDO0FBQ0osQ0FBQyJ9 \ No newline at end of file diff --git a/packages/loopover-miner/lib/idea-feasibility.ts b/packages/loopover-miner/lib/idea-feasibility.ts new file mode 100644 index 0000000000..639791e2a7 --- /dev/null +++ b/packages/loopover-miner/lib/idea-feasibility.ts @@ -0,0 +1,114 @@ +/** Pre-execution feasibility check for a freeform Rent-a-Loop idea (#5671). + * + * Runs post-schema-validation and pre-compute-allocation on an idea submission (the intake shape defined in + * #4779), so a customer can no longer burn paid or free-trial compute on an idea that was never going to + * succeed. It is the freeform-text counterpart to the metadata `feasibility` CLI (`feasibility-cli.js`, #4270). + * + * REUSED from feasibility-cli.js AS-IS: + * - the engine's pure `buildFeasibilityVerdict` composer and its `avoid > raise > go` precedence — an idea + * inherits exactly the same verdict machinery a metadata-resolved issue does, so there is no second, + * divergent decision surface; + * - the injectable-verdict test seam (`options.buildFeasibilityVerdict`), matching the CLI's convention. + * + * NEW for freeform text (#5671, per the #4779 rubric): + * - `deriveIdeaIssueStatus`, which computes the `issueStatus` discriminant from the idea's OWN structure + * instead of a resolved GitHub issue. An idea with no objective success signal is `invalid` (impossible to + * evaluate objectively) and is rejected before compute; an unresolvable target repo is `missing` (out of the + * loop's scope) and is flagged. + * + * OUT OF SCOPE (stays with #5136): judging abusive/illegal or semantically off-topic intent from prose — that is + * a content-moderation policy call, not this deterministic structural gate. + */ +import { buildFeasibilityVerdict } from "@loopover/engine"; +import type { + FeasibilityClaimStatus, + FeasibilityDuplicateClusterRisk, + FeasibilityGateInput, + FeasibilityGateResult, + FeasibilityIssueStatus, + FeasibilityVerdict, +} from "@loopover/engine"; + +/** A schema-validated idea submission (#4779). This structural gate only reads `acceptanceHints`, but accepts + * the full submission so callers can pass the idea through unchanged. */ +export type IdeaFeasibilityInput = { + title?: string | undefined; + body?: string | undefined; + targetRepo?: string | undefined; + constraints?: readonly string[] | undefined; + acceptanceHints?: readonly string[] | undefined; + priority?: "normal" | "high" | undefined; +}; + +/** Objectively-resolved intake signals for the idea (resolved by the caller, never guessed from prose). */ +export type ResolvedIdeaSignals = { + targetResolvable: boolean; + claimStatus: FeasibilityClaimStatus; + duplicateClusterRisk: FeasibilityDuplicateClusterRisk; +}; + +export type AssessIdeaFeasibilityOptions = { + buildFeasibilityVerdict?: (input: FeasibilityGateInput) => FeasibilityGateResult; +}; + +export type IdeaFeasibilityDisposition = "proceed" | "flag" | "reject"; + +export type IdeaFeasibilityResult = { + disposition: IdeaFeasibilityDisposition; + verdict: FeasibilityVerdict; + issueStatus: FeasibilityIssueStatus; + reasons: string[]; + summary: string; +}; + +/** Verdict → caller-facing disposition. `go` proceeds to compute; `raise`/`avoid` gate it. */ +const DISPOSITION_BY_VERDICT: Record = { + go: "proceed", + raise: "flag", + avoid: "reject", +}; + +/** + * Derive the feasibility `issueStatus` for a freeform idea from objective, structural signals only — never from + * a semantic read of the prose. + */ +export function deriveIdeaIssueStatus( + idea: IdeaFeasibilityInput, + resolved: Pick, +): FeasibilityIssueStatus { + // Out of the loop's scope: the idea does not resolve to a repo the loop can act on. + if (!resolved.targetResolvable) return "missing"; + // Impossible to evaluate objectively: no declared success signal, so the loop could never test its own output. + // Count CONTENT, not array length (#6766): a blank/whitespace-only hint declares nothing testable, so it must + // not pass as an objective signal just by occupying a slot. + const objectiveSignals = (idea.acceptanceHints ?? []).filter( + (hint) => typeof hint === "string" && hint.trim() !== "", + ).length; + if (objectiveSignals === 0) return "invalid"; + return "ready"; +} + +/** + * Assess a schema-validated idea's feasibility before compute is allocated. + */ +export function assessIdeaFeasibility( + idea: IdeaFeasibilityInput, + resolved: ResolvedIdeaSignals, + options: AssessIdeaFeasibilityOptions = {}, +): IdeaFeasibilityResult { + const buildVerdict = options.buildFeasibilityVerdict ?? buildFeasibilityVerdict; + const issueStatus = deriveIdeaIssueStatus(idea, resolved); + const verdict = buildVerdict({ + found: resolved.targetResolvable, + claimStatus: resolved.claimStatus, + duplicateClusterRisk: resolved.duplicateClusterRisk, + issueStatus, + }); + return { + disposition: DISPOSITION_BY_VERDICT[verdict.verdict], + verdict: verdict.verdict, + issueStatus, + reasons: [...verdict.avoidReasons, ...verdict.raiseReasons], + summary: verdict.summary, + }; +} diff --git a/packages/loopover-miner/lib/portfolio-discovery.d.ts b/packages/loopover-miner/lib/portfolio-discovery.d.ts index fa1052facf..c7c5560030 100644 --- a/packages/loopover-miner/lib/portfolio-discovery.d.ts +++ b/packages/loopover-miner/lib/portfolio-discovery.d.ts @@ -1,29 +1,28 @@ +/** Local orchestration: materialize ranked fan-out rows into the portfolio queue (#2292). */ import type { EventLedger } from "./event-ledger.js"; import type { PortfolioQueueStore } from "./portfolio-queue.js"; - export type EnqueueRankedDiscoveryInput = { - repoFullName: string; - issueNumber: number; - title: string; - labels?: string[]; - rankScore: number; + repoFullName: string; + issueNumber: number; + title: string; + labels?: string[]; + rankScore: number; }; - export type EnqueueRankedDiscoveryOptions = { - queueStore: PortfolioQueueStore; - eventLedger?: EventLedger; - minRankScore?: number | null; - apiBaseUrl?: string; + queueStore: PortfolioQueueStore; + eventLedger?: EventLedger; + minRankScore?: number | null; + apiBaseUrl?: string; }; - export type EnqueueRankedDiscoverySummary = { - enqueued: number; - skippedBelowMinRank: number; - skippedInvalid: number; - eventsAppended: number; + enqueued: number; + skippedBelowMinRank: number; + skippedInvalid: number; + eventsAppended: number; }; - -export function enqueueRankedDiscovery( - rankedIssues: readonly EnqueueRankedDiscoveryInput[], - options: EnqueueRankedDiscoveryOptions, -): EnqueueRankedDiscoverySummary; +/** + * Enqueue ranked discovery rows into the local portfolio backlog. Uses each row's `rankScore` as queue priority + * (the #2292 placeholder field). Optionally appends `discovered_issue` audit events when an event ledger is supplied. + * Never calls GitHub — callers rank locally first via `rankCandidateIssues`. + */ +export declare function enqueueRankedDiscovery(rankedIssues: readonly EnqueueRankedDiscoveryInput[], options: EnqueueRankedDiscoveryOptions): EnqueueRankedDiscoverySummary; diff --git a/packages/loopover-miner/lib/portfolio-discovery.js b/packages/loopover-miner/lib/portfolio-discovery.js index 821955120e..c27ee18020 100644 --- a/packages/loopover-miner/lib/portfolio-discovery.js +++ b/packages/loopover-miner/lib/portfolio-discovery.js @@ -1,100 +1,100 @@ -/** Local orchestration: materialize ranked fan-out rows into the portfolio queue (#2292). */ - function normalizeMinRankScore(minRankScore) { - if (minRankScore === undefined || minRankScore === null) return 0; - if (typeof minRankScore !== "number" || !Number.isFinite(minRankScore) || minRankScore < 0) { - throw new Error("invalid_min_rank_score"); - } - return minRankScore; + if (minRankScore === undefined || minRankScore === null) + return 0; + if (typeof minRankScore !== "number" || !Number.isFinite(minRankScore) || minRankScore < 0) { + throw new Error("invalid_min_rank_score"); + } + return minRankScore; } - function normalizeRankedIssue(issue) { - if (!issue || typeof issue !== "object") return null; - const repoFullName = typeof issue.repoFullName === "string" ? issue.repoFullName.trim() : ""; - const [owner, repo, extra] = repoFullName.split("/"); - if (!owner || !repo || extra !== undefined) return null; - if (!Number.isInteger(issue.issueNumber) || issue.issueNumber <= 0) return null; - if (typeof issue.rankScore !== "number" || !Number.isFinite(issue.rankScore) || issue.rankScore < 0) { - return null; - } - const title = typeof issue.title === "string" ? issue.title.trim() : ""; - if (!title) return null; - const labels = Array.isArray(issue.labels) - ? issue.labels.filter((label) => typeof label === "string" && label.trim()).map((label) => label.trim()) - : []; - return { - repoFullName: `${owner}/${repo}`, - issueNumber: issue.issueNumber, - title, - labels, - rankScore: issue.rankScore, - }; + if (!issue || typeof issue !== "object") + return null; + const i = issue; + const repoFullName = typeof i.repoFullName === "string" ? i.repoFullName.trim() : ""; + const [owner, repo, extra] = repoFullName.split("/"); + if (!owner || !repo || extra !== undefined) + return null; + if (!Number.isInteger(i.issueNumber) || i.issueNumber <= 0) + return null; + if (typeof i.rankScore !== "number" || !Number.isFinite(i.rankScore) || i.rankScore < 0) { + return null; + } + const title = typeof i.title === "string" ? i.title.trim() : ""; + if (!title) + return null; + const labels = Array.isArray(i.labels) + ? i.labels.filter((label) => typeof label === "string" && label.trim() !== "").map((label) => label.trim()) + : []; + return { + repoFullName: `${owner}/${repo}`, + issueNumber: i.issueNumber, + title, + labels, + rankScore: i.rankScore, + }; } - /** * Enqueue ranked discovery rows into the local portfolio backlog. Uses each row's `rankScore` as queue priority * (the #2292 placeholder field). Optionally appends `discovered_issue` audit events when an event ledger is supplied. * Never calls GitHub — callers rank locally first via `rankCandidateIssues`. */ -export function enqueueRankedDiscovery(rankedIssues, options = {}) { - if (!Array.isArray(rankedIssues)) throw new Error("invalid_ranked_issues"); - const queueStore = options.queueStore; - if (!queueStore || typeof queueStore.enqueue !== "function") throw new Error("invalid_queue_store"); - - let eventLedger = null; - if (options.eventLedger !== undefined) { - eventLedger = options.eventLedger; - if (!eventLedger || typeof eventLedger.appendEvent !== "function") { - throw new Error("invalid_event_ledger"); - } - } - - const minRankScore = normalizeMinRankScore(options.minRankScore); - // #5563: threaded through from the caller's already-resolved forge host, so a non-default (GitHub Enterprise) - // tenant's ranked issues land in the queue scoped to their own host instead of colliding with a same-named - // owner/repo on github.com. Omitted/nullish falls through to the queue store's own github.com default. - const apiBaseUrl = options.apiBaseUrl; - - const summary = { - enqueued: 0, - skippedBelowMinRank: 0, - skippedInvalid: 0, - eventsAppended: 0, - }; - - for (const issue of rankedIssues) { - const normalized = normalizeRankedIssue(issue); - if (!normalized) { - summary.skippedInvalid += 1; - continue; - } - if (normalized.rankScore < minRankScore) { - summary.skippedBelowMinRank += 1; - continue; +export function enqueueRankedDiscovery(rankedIssues, options) { + if (!Array.isArray(rankedIssues)) + throw new Error("invalid_ranked_issues"); + const queueStore = options.queueStore; + if (!queueStore || typeof queueStore.enqueue !== "function") + throw new Error("invalid_queue_store"); + let eventLedger = null; + if (options.eventLedger !== undefined) { + eventLedger = options.eventLedger; + if (!eventLedger || typeof eventLedger.appendEvent !== "function") { + throw new Error("invalid_event_ledger"); + } } - - queueStore.enqueue({ - repoFullName: normalized.repoFullName, - identifier: `issue:${normalized.issueNumber}`, - priority: normalized.rankScore, - apiBaseUrl, - }); - summary.enqueued += 1; - - if (eventLedger) { - eventLedger.appendEvent({ - type: "discovered_issue", - repoFullName: normalized.repoFullName, - payload: { - issueNumber: normalized.issueNumber, - rankScore: normalized.rankScore, - title: normalized.title, - labels: normalized.labels, - }, - }); - summary.eventsAppended += 1; + const minRankScore = normalizeMinRankScore(options.minRankScore); + // #5563: threaded through from the caller's already-resolved forge host, so a non-default (GitHub Enterprise) + // tenant's ranked issues land in the queue scoped to their own host instead of colliding with a same-named + // owner/repo on github.com. Omitted/nullish falls through to the queue store's own github.com default. + const apiBaseUrl = options.apiBaseUrl; + const summary = { + enqueued: 0, + skippedBelowMinRank: 0, + skippedInvalid: 0, + eventsAppended: 0, + }; + for (const issue of rankedIssues) { + const normalized = normalizeRankedIssue(issue); + if (!normalized) { + summary.skippedInvalid += 1; + continue; + } + if (normalized.rankScore < minRankScore) { + summary.skippedBelowMinRank += 1; + continue; + } + // Spread-omit rather than pass `undefined` explicitly -- EnqueueItem's `apiBaseUrl` doesn't declare + // `| undefined`, and exactOptionalPropertyTypes treats those as different. + queueStore.enqueue({ + repoFullName: normalized.repoFullName, + identifier: `issue:${normalized.issueNumber}`, + priority: normalized.rankScore, + ...(apiBaseUrl !== undefined ? { apiBaseUrl } : {}), + }); + summary.enqueued += 1; + if (eventLedger) { + eventLedger.appendEvent({ + type: "discovered_issue", + repoFullName: normalized.repoFullName, + payload: { + issueNumber: normalized.issueNumber, + rankScore: normalized.rankScore, + title: normalized.title, + labels: normalized.labels, + }, + }); + summary.eventsAppended += 1; + } } - } - - return summary; + return summary; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicG9ydGZvbGlvLWRpc2NvdmVyeS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbInBvcnRmb2xpby1kaXNjb3ZlcnkudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBa0NBLFNBQVMscUJBQXFCLENBQUMsWUFBdUM7SUFDcEUsSUFBSSxZQUFZLEtBQUssU0FBUyxJQUFJLFlBQVksS0FBSyxJQUFJO1FBQUUsT0FBTyxDQUFDLENBQUM7SUFDbEUsSUFBSSxPQUFPLFlBQVksS0FBSyxRQUFRLElBQUksQ0FBQyxNQUFNLENBQUMsUUFBUSxDQUFDLFlBQVksQ0FBQyxJQUFJLFlBQVksR0FBRyxDQUFDLEVBQUUsQ0FBQztRQUMzRixNQUFNLElBQUksS0FBSyxDQUFDLHdCQUF3QixDQUFDLENBQUM7SUFDNUMsQ0FBQztJQUNELE9BQU8sWUFBWSxDQUFDO0FBQ3RCLENBQUM7QUFFRCxTQUFTLG9CQUFvQixDQUFDLEtBQWM7SUFDMUMsSUFBSSxDQUFDLEtBQUssSUFBSSxPQUFPLEtBQUssS0FBSyxRQUFRO1FBQUUsT0FBTyxJQUFJLENBQUM7SUFDckQsTUFBTSxDQUFDLEdBQUcsS0FBZ0MsQ0FBQztJQUMzQyxNQUFNLFlBQVksR0FBRyxPQUFPLENBQUMsQ0FBQyxZQUFZLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsWUFBWSxDQUFDLElBQUksRUFBRSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7SUFDckYsTUFBTSxDQUFDLEtBQUssRUFBRSxJQUFJLEVBQUUsS0FBSyxDQUFDLEdBQUcsWUFBWSxDQUFDLEtBQUssQ0FBQyxHQUFHLENBQUMsQ0FBQztJQUNyRCxJQUFJLENBQUMsS0FBSyxJQUFJLENBQUMsSUFBSSxJQUFJLEtBQUssS0FBSyxTQUFTO1FBQUUsT0FBTyxJQUFJLENBQUM7SUFDeEQsSUFBSSxDQUFDLE1BQU0sQ0FBQyxTQUFTLENBQUMsQ0FBQyxDQUFDLFdBQVcsQ0FBQyxJQUFLLENBQUMsQ0FBQyxXQUFzQixJQUFJLENBQUM7UUFBRSxPQUFPLElBQUksQ0FBQztJQUNwRixJQUFJLE9BQU8sQ0FBQyxDQUFDLFNBQVMsS0FBSyxRQUFRLElBQUksQ0FBQyxNQUFNLENBQUMsUUFBUSxDQUFDLENBQUMsQ0FBQyxTQUFTLENBQUMsSUFBSSxDQUFDLENBQUMsU0FBUyxHQUFHLENBQUMsRUFBRSxDQUFDO1FBQ3hGLE9BQU8sSUFBSSxDQUFDO0lBQ2QsQ0FBQztJQUNELE1BQU0sS0FBSyxHQUFHLE9BQU8sQ0FBQyxDQUFDLEtBQUssS0FBSyxRQUFRLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxLQUFLLENBQUMsSUFBSSxFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQztJQUNoRSxJQUFJLENBQUMsS0FBSztRQUFFLE9BQU8sSUFBSSxDQUFDO0lBQ3hCLE1BQU0sTUFBTSxHQUFHLEtBQUssQ0FBQyxPQUFPLENBQUMsQ0FBQyxDQUFDLE1BQU0sQ0FBQztRQUNwQyxDQUFDLENBQUMsQ0FBQyxDQUFDLE1BQU0sQ0FBQyxNQUFNLENBQUMsQ0FBQyxLQUFLLEVBQW1CLEVBQUUsQ0FBQyxPQUFPLEtBQUssS0FBSyxRQUFRLElBQUksS0FBSyxDQUFDLElBQUksRUFBRSxLQUFLLEVBQUUsQ0FBQyxDQUFDLEdBQUcsQ0FBQyxDQUFDLEtBQUssRUFBRSxFQUFFLENBQUMsS0FBSyxDQUFDLElBQUksRUFBRSxDQUFDO1FBQzVILENBQUMsQ0FBQyxFQUFFLENBQUM7SUFDUCxPQUFPO1FBQ0wsWUFBWSxFQUFFLEdBQUcsS0FBSyxJQUFJLElBQUksRUFBRTtRQUNoQyxXQUFXLEVBQUUsQ0FBQyxDQUFDLFdBQXFCO1FBQ3BDLEtBQUs7UUFDTCxNQUFNO1FBQ04sU0FBUyxFQUFFLENBQUMsQ0FBQyxTQUFTO0tBQ3ZCLENBQUM7QUFDSixDQUFDO0FBRUQ7Ozs7R0FJRztBQUNILE1BQU0sVUFBVSxzQkFBc0IsQ0FDcEMsWUFBb0QsRUFDcEQsT0FBc0M7SUFFdEMsSUFBSSxDQUFDLEtBQUssQ0FBQyxPQUFPLENBQUMsWUFBWSxDQUFDO1FBQUUsTUFBTSxJQUFJLEtBQUssQ0FBQyx1QkFBdUIsQ0FBQyxDQUFDO0lBQzNFLE1BQU0sVUFBVSxHQUFHLE9BQU8sQ0FBQyxVQUFVLENBQUM7SUFDdEMsSUFBSSxDQUFDLFVBQVUsSUFBSSxPQUFPLFVBQVUsQ0FBQyxPQUFPLEtBQUssVUFBVTtRQUFFLE1BQU0sSUFBSSxLQUFLLENBQUMscUJBQXFCLENBQUMsQ0FBQztJQUVwRyxJQUFJLFdBQVcsR0FBdUIsSUFBSSxDQUFDO0lBQzNDLElBQUksT0FBTyxDQUFDLFdBQVcsS0FBSyxTQUFTLEVBQUUsQ0FBQztRQUN0QyxXQUFXLEdBQUcsT0FBTyxDQUFDLFdBQVcsQ0FBQztRQUNsQyxJQUFJLENBQUMsV0FBVyxJQUFJLE9BQU8sV0FBVyxDQUFDLFdBQVcsS0FBSyxVQUFVLEVBQUUsQ0FBQztZQUNsRSxNQUFNLElBQUksS0FBSyxDQUFDLHNCQUFzQixDQUFDLENBQUM7UUFDMUMsQ0FBQztJQUNILENBQUM7SUFFRCxNQUFNLFlBQVksR0FBRyxxQkFBcUIsQ0FBQyxPQUFPLENBQUMsWUFBWSxDQUFDLENBQUM7SUFDakUsOEdBQThHO0lBQzlHLDJHQUEyRztJQUMzRyx1R0FBdUc7SUFDdkcsTUFBTSxVQUFVLEdBQUcsT0FBTyxDQUFDLFVBQVUsQ0FBQztJQUV0QyxNQUFNLE9BQU8sR0FBa0M7UUFDN0MsUUFBUSxFQUFFLENBQUM7UUFDWCxtQkFBbUIsRUFBRSxDQUFDO1FBQ3RCLGNBQWMsRUFBRSxDQUFDO1FBQ2pCLGNBQWMsRUFBRSxDQUFDO0tBQ2xCLENBQUM7SUFFRixLQUFLLE1BQU0sS0FBSyxJQUFJLFlBQVksRUFBRSxDQUFDO1FBQ2pDLE1BQU0sVUFBVSxHQUFHLG9CQUFvQixDQUFDLEtBQUssQ0FBQyxDQUFDO1FBQy9DLElBQUksQ0FBQyxVQUFVLEVBQUUsQ0FBQztZQUNoQixPQUFPLENBQUMsY0FBYyxJQUFJLENBQUMsQ0FBQztZQUM1QixTQUFTO1FBQ1gsQ0FBQztRQUNELElBQUksVUFBVSxDQUFDLFNBQVMsR0FBRyxZQUFZLEVBQUUsQ0FBQztZQUN4QyxPQUFPLENBQUMsbUJBQW1CLElBQUksQ0FBQyxDQUFDO1lBQ2pDLFNBQVM7UUFDWCxDQUFDO1FBRUQsb0dBQW9HO1FBQ3BHLDJFQUEyRTtRQUMzRSxVQUFVLENBQUMsT0FBTyxDQUFDO1lBQ2pCLFlBQVksRUFBRSxVQUFVLENBQUMsWUFBWTtZQUNyQyxVQUFVLEVBQUUsU0FBUyxVQUFVLENBQUMsV0FBVyxFQUFFO1lBQzdDLFFBQVEsRUFBRSxVQUFVLENBQUMsU0FBUztZQUM5QixHQUFHLENBQUMsVUFBVSxLQUFLLFNBQVMsQ0FBQyxDQUFDLENBQUMsRUFBRSxVQUFVLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDO1NBQ3BELENBQUMsQ0FBQztRQUNILE9BQU8sQ0FBQyxRQUFRLElBQUksQ0FBQyxDQUFDO1FBRXRCLElBQUksV0FBVyxFQUFFLENBQUM7WUFDaEIsV0FBVyxDQUFDLFdBQVcsQ0FBQztnQkFDdEIsSUFBSSxFQUFFLGtCQUFrQjtnQkFDeEIsWUFBWSxFQUFFLFVBQVUsQ0FBQyxZQUFZO2dCQUNyQyxPQUFPLEVBQUU7b0JBQ1AsV0FBVyxFQUFFLFVBQVUsQ0FBQyxXQUFXO29CQUNuQyxTQUFTLEVBQUUsVUFBVSxDQUFDLFNBQVM7b0JBQy9CLEtBQUssRUFBRSxVQUFVLENBQUMsS0FBSztvQkFDdkIsTUFBTSxFQUFFLFVBQVUsQ0FBQyxNQUFNO2lCQUMxQjthQUNGLENBQUMsQ0FBQztZQUNILE9BQU8sQ0FBQyxjQUFjLElBQUksQ0FBQyxDQUFDO1FBQzlCLENBQUM7SUFDSCxDQUFDO0lBRUQsT0FBTyxPQUFPLENBQUM7QUFDakIsQ0FBQyJ9 \ No newline at end of file diff --git a/packages/loopover-miner/lib/portfolio-discovery.ts b/packages/loopover-miner/lib/portfolio-discovery.ts new file mode 100644 index 0000000000..5c7850dda3 --- /dev/null +++ b/packages/loopover-miner/lib/portfolio-discovery.ts @@ -0,0 +1,138 @@ +/** Local orchestration: materialize ranked fan-out rows into the portfolio queue (#2292). */ +import type { EventLedger } from "./event-ledger.js"; +import type { PortfolioQueueStore } from "./portfolio-queue.js"; + +export type EnqueueRankedDiscoveryInput = { + repoFullName: string; + issueNumber: number; + title: string; + labels?: string[]; + rankScore: number; +}; + +export type EnqueueRankedDiscoveryOptions = { + queueStore: PortfolioQueueStore; + eventLedger?: EventLedger; + minRankScore?: number | null; + apiBaseUrl?: string; +}; + +export type EnqueueRankedDiscoverySummary = { + enqueued: number; + skippedBelowMinRank: number; + skippedInvalid: number; + eventsAppended: number; +}; + +type NormalizedRankedIssue = { + repoFullName: string; + issueNumber: number; + title: string; + labels: string[]; + rankScore: number; +}; + +function normalizeMinRankScore(minRankScore: number | null | undefined): number { + if (minRankScore === undefined || minRankScore === null) return 0; + if (typeof minRankScore !== "number" || !Number.isFinite(minRankScore) || minRankScore < 0) { + throw new Error("invalid_min_rank_score"); + } + return minRankScore; +} + +function normalizeRankedIssue(issue: unknown): NormalizedRankedIssue | null { + if (!issue || typeof issue !== "object") return null; + const i = issue as Record; + const repoFullName = typeof i.repoFullName === "string" ? i.repoFullName.trim() : ""; + const [owner, repo, extra] = repoFullName.split("/"); + if (!owner || !repo || extra !== undefined) return null; + if (!Number.isInteger(i.issueNumber) || (i.issueNumber as number) <= 0) return null; + if (typeof i.rankScore !== "number" || !Number.isFinite(i.rankScore) || i.rankScore < 0) { + return null; + } + const title = typeof i.title === "string" ? i.title.trim() : ""; + if (!title) return null; + const labels = Array.isArray(i.labels) + ? i.labels.filter((label): label is string => typeof label === "string" && label.trim() !== "").map((label) => label.trim()) + : []; + return { + repoFullName: `${owner}/${repo}`, + issueNumber: i.issueNumber as number, + title, + labels, + rankScore: i.rankScore, + }; +} + +/** + * Enqueue ranked discovery rows into the local portfolio backlog. Uses each row's `rankScore` as queue priority + * (the #2292 placeholder field). Optionally appends `discovered_issue` audit events when an event ledger is supplied. + * Never calls GitHub — callers rank locally first via `rankCandidateIssues`. + */ +export function enqueueRankedDiscovery( + rankedIssues: readonly EnqueueRankedDiscoveryInput[], + options: EnqueueRankedDiscoveryOptions, +): EnqueueRankedDiscoverySummary { + if (!Array.isArray(rankedIssues)) throw new Error("invalid_ranked_issues"); + const queueStore = options.queueStore; + if (!queueStore || typeof queueStore.enqueue !== "function") throw new Error("invalid_queue_store"); + + let eventLedger: EventLedger | null = null; + if (options.eventLedger !== undefined) { + eventLedger = options.eventLedger; + if (!eventLedger || typeof eventLedger.appendEvent !== "function") { + throw new Error("invalid_event_ledger"); + } + } + + const minRankScore = normalizeMinRankScore(options.minRankScore); + // #5563: threaded through from the caller's already-resolved forge host, so a non-default (GitHub Enterprise) + // tenant's ranked issues land in the queue scoped to their own host instead of colliding with a same-named + // owner/repo on github.com. Omitted/nullish falls through to the queue store's own github.com default. + const apiBaseUrl = options.apiBaseUrl; + + const summary: EnqueueRankedDiscoverySummary = { + enqueued: 0, + skippedBelowMinRank: 0, + skippedInvalid: 0, + eventsAppended: 0, + }; + + for (const issue of rankedIssues) { + const normalized = normalizeRankedIssue(issue); + if (!normalized) { + summary.skippedInvalid += 1; + continue; + } + if (normalized.rankScore < minRankScore) { + summary.skippedBelowMinRank += 1; + continue; + } + + // Spread-omit rather than pass `undefined` explicitly -- EnqueueItem's `apiBaseUrl` doesn't declare + // `| undefined`, and exactOptionalPropertyTypes treats those as different. + queueStore.enqueue({ + repoFullName: normalized.repoFullName, + identifier: `issue:${normalized.issueNumber}`, + priority: normalized.rankScore, + ...(apiBaseUrl !== undefined ? { apiBaseUrl } : {}), + }); + summary.enqueued += 1; + + if (eventLedger) { + eventLedger.appendEvent({ + type: "discovered_issue", + repoFullName: normalized.repoFullName, + payload: { + issueNumber: normalized.issueNumber, + rankScore: normalized.rankScore, + title: normalized.title, + labels: normalized.labels, + }, + }); + summary.eventsAppended += 1; + } + } + + return summary; +} diff --git a/packages/loopover-miner/lib/process-lifecycle.d.ts b/packages/loopover-miner/lib/process-lifecycle.d.ts index cecd7d9e3a..eb0849b57f 100644 --- a/packages/loopover-miner/lib/process-lifecycle.d.ts +++ b/packages/loopover-miner/lib/process-lifecycle.d.ts @@ -1,37 +1,54 @@ -/** Process lifecycle / crash-safety for the miner CLI (#4826). Local stores register on open and the CLI installs - * signal/error handlers once at startup so an interrupted run closes every open ledger cleanly. */ - +/** Process lifecycle / crash-safety for the miner CLI (#4826). The CLI dispatches through a chain of bare + * `process.exit()` calls with no cleanup hook, so a SIGINT/SIGTERM mid-run — or an uncaught exception — used to + * kill the process mid-write, leaving whatever local SQLite ledger it was touching in an undefined state. This + * module is the single cleanup chokepoint: local stores register themselves when opened (see `local-store.js`), and + * `installCliSignalHandlers` (called once at CLI startup) flushes/closes every still-open resource before exiting + * cleanly on a signal, and logs + exits non-zero on an uncaught exception / unhandled rejection instead of crashing + * silently. Cleanup ONLY — no command business logic lives here. Every dependency (`process`, `log`, `exit`) is + * injectable so the handlers are unit-testable without actually signalling the test runner. */ /** A closable store (`{ close() }`) or a plain cleanup callback. */ -export type CleanupResource = { close: () => void } | (() => void); - +export type CleanupResource = { + close: () => void; +} | (() => void); /** The subset of `process` the handlers use; injectable for tests. */ export type ProcessLike = { - on: (event: string, listener: (...args: unknown[]) => void) => unknown; - exit: (code?: number) => void; + on: (event: string, listener: (...args: unknown[]) => void) => unknown; + exit: (code?: number) => void; }; - export type InstallCliSignalHandlersOptions = { - process?: ProcessLike; - log?: (message: string) => void; - exit?: (code: number) => void; - /** Called (in addition to `log`) for uncaughtException/unhandledRejection specifically -- not the clean - * SIGINT/SIGTERM exits, which are not errors. AWAITED before the process exits, so it should both capture - * AND flush (see captureMinerErrorAndFlush in bin/loopover-miner.js) -- a synchronous capture alone only - * queues the event, which process.exit() would then likely never deliver. No-op default. Never expected to - * throw/reject. */ - captureError?: (error: unknown, context?: Record) => void | Promise; - /** Reinstall even if handlers were already installed (mainly for tests). */ - force?: boolean; + process?: ProcessLike; + log?: (message: string) => void; + exit?: (code: number) => void; + /** Called (in addition to `log`) for uncaughtException/unhandledRejection specifically -- not the clean + * SIGINT/SIGTERM exits, which are not errors. AWAITED before the process exits, so it should both capture + * AND flush (see captureMinerErrorAndFlush in bin/loopover-miner.js) -- a synchronous capture alone only + * queues the event, which process.exit() would then likely never deliver. No-op default. Never expected to + * throw/reject. */ + captureError?: (error: unknown, context?: Record) => void | Promise; + /** Reinstall even if handlers were already installed (mainly for tests). */ + force?: boolean; }; - -/** Register a resource to close on exit; returns an idempotent unregister function. */ -export function registerCleanupResource(resource: CleanupResource | null | undefined): () => void; - -export function cleanupResourceCount(): number; - -export function closeAllCleanupResources(options?: { onError?: (error: unknown) => void }): void; - -/** Install signal + error handlers once. Returns false if already installed (and `force` was not set). */ -export function installCliSignalHandlers(options?: InstallCliSignalHandlersOptions): boolean; - -export function resetProcessLifecycleForTesting(): void; +/** + * Register a resource to be closed on clean exit or crash. Returns an idempotent unregister function (call it from + * the resource's own normal `close()` so a resource closed during the happy path is not double-closed at exit). + */ +export declare function registerCleanupResource(resource: CleanupResource | null | undefined): () => void; +/** Number of currently-registered cleanup resources (exposed for tests / diagnostics). */ +export declare function cleanupResourceCount(): number; +/** + * Close every registered resource, swallowing each individual failure (a store that fails to close must not stop + * the others from closing) and reporting it via `options.onError`. Idempotent: the registry is emptied afterwards. + */ +export declare function closeAllCleanupResources(options?: { + onError?: (error: unknown) => void; +}): void; +/** + * Install top-level signal + error handlers once. On SIGINT/SIGTERM: close all resources and exit with the + * conventional 128+signal code. On uncaughtException/unhandledRejection: log the error, AWAIT the optional + * captureError hook (so a captured Sentry event has a chance to actually flush before the process exits), + * close all resources, and exit non-zero. No-op (returns false) if already installed unless `options.force` is + * set. All of `process`, `log`, `exit`, and `captureError` are injectable for testing. + */ +export declare function installCliSignalHandlers(options?: InstallCliSignalHandlersOptions): boolean; +/** Test-only: clear the registry and the installed flag so each test starts from a clean lifecycle. */ +export declare function resetProcessLifecycleForTesting(): void; diff --git a/packages/loopover-miner/lib/process-lifecycle.js b/packages/loopover-miner/lib/process-lifecycle.js index 72a53e5da6..2189df7871 100644 --- a/packages/loopover-miner/lib/process-lifecycle.js +++ b/packages/loopover-miner/lib/process-lifecycle.js @@ -6,56 +6,55 @@ * cleanly on a signal, and logs + exits non-zero on an uncaught exception / unhandled rejection instead of crashing * silently. Cleanup ONLY — no command business logic lives here. Every dependency (`process`, `log`, `exit`) is * injectable so the handlers are unit-testable without actually signalling the test runner. */ - // 128 + signal number, the conventional shell exit code for a process terminated by that signal (SIGINT=2 -> 130, // SIGTERM=15 -> 143). const SIGNAL_EXIT_CODES = Object.freeze({ SIGINT: 130, SIGTERM: 143 }); - /** Resources to close on exit. A resource is either a `{ close() }` object (e.g. an open SQLite store) or a plain * cleanup function. Held in insertion order so cleanup is deterministic. */ const cleanupResources = new Set(); let handlersInstalled = false; - /** Render any thrown value as a single log-safe string, preferring an Error's stack. */ function describeError(value) { - if (value instanceof Error) return value.stack ?? value.message; - return String(value); + if (value instanceof Error) + return value.stack ?? value.message; + return String(value); } - /** * Register a resource to be closed on clean exit or crash. Returns an idempotent unregister function (call it from * the resource's own normal `close()` so a resource closed during the happy path is not double-closed at exit). */ export function registerCleanupResource(resource) { - if (resource === null || resource === undefined) return () => {}; - cleanupResources.add(resource); - return () => { - cleanupResources.delete(resource); - }; + if (resource === null || resource === undefined) + return () => { }; + cleanupResources.add(resource); + return () => { + cleanupResources.delete(resource); + }; } - /** Number of currently-registered cleanup resources (exposed for tests / diagnostics). */ export function cleanupResourceCount() { - return cleanupResources.size; + return cleanupResources.size; } - /** * Close every registered resource, swallowing each individual failure (a store that fails to close must not stop * the others from closing) and reporting it via `options.onError`. Idempotent: the registry is emptied afterwards. */ export function closeAllCleanupResources(options = {}) { - const onError = typeof options.onError === "function" ? options.onError : null; - for (const resource of [...cleanupResources]) { - try { - if (typeof resource === "function") resource(); - else resource.close(); - } catch (error) { - if (onError) onError(error); + const onError = typeof options.onError === "function" ? options.onError : null; + for (const resource of [...cleanupResources]) { + try { + if (typeof resource === "function") + resource(); + else + resource.close(); + } + catch (error) { + if (onError) + onError(error); + } } - } - cleanupResources.clear(); + cleanupResources.clear(); } - /** * Install top-level signal + error handlers once. On SIGINT/SIGTERM: close all resources and exit with the * conventional 128+signal code. On uncaughtException/unhandledRejection: log the error, AWAIT the optional @@ -64,57 +63,52 @@ export function closeAllCleanupResources(options = {}) { * set. All of `process`, `log`, `exit`, and `captureError` are injectable for testing. */ export function installCliSignalHandlers(options = {}) { - const proc = options.process ?? process; - const log = typeof options.log === "function" ? options.log : (message) => console.error(message); - const exit = typeof options.exit === "function" ? options.exit : (code) => proc.exit(code); - // Optional Sentry (or any) capture hook -- decoupled from a specific implementation so this module stays - // fully unit-testable without mocking Sentry (#6011). No-op default matches this module's pre-existing - // behavior for every caller that doesn't pass one. - const captureError = typeof options.captureError === "function" ? options.captureError : () => {}; - - if (handlersInstalled && options.force !== true) return false; - handlersInstalled = true; - - const runCleanup = () => { - closeAllCleanupResources({ - onError: (error) => log(`loopover-miner: cleanup error while exiting: ${describeError(error)}`), + const proc = options.process ?? process; + const log = typeof options.log === "function" ? options.log : (message) => console.error(message); + const exit = typeof options.exit === "function" ? options.exit : (code) => proc.exit(code); + // Optional Sentry (or any) capture hook -- decoupled from a specific implementation so this module stays + // fully unit-testable without mocking Sentry (#6011). No-op default matches this module's pre-existing + // behavior for every caller that doesn't pass one. + const captureError = typeof options.captureError === "function" ? options.captureError : () => { }; + if (handlersInstalled && options.force !== true) + return false; + handlersInstalled = true; + const runCleanup = () => { + closeAllCleanupResources({ + onError: (error) => log(`loopover-miner: cleanup error while exiting: ${describeError(error)}`), + }); + }; + for (const [signal, code] of Object.entries(SIGNAL_EXIT_CODES)) { + proc.on(signal, () => { + log(`loopover-miner: received ${signal}, closing open resources and exiting.`); + runCleanup(); + exit(code); + }); + } + // Awaited (not fire-and-forget): captureError is expected to both capture AND flush before returning (see + // captureMinerErrorAndFlush in bin/loopover-miner.js) -- Sentry.captureException only QUEUES an event, and + // process.exit() tears the process down immediately without waiting for any pending HTTP delivery, so a + // synchronous capture-then-exit would make the crash-capture path a near-total no-op in practice. Node does + // not require these handlers to be synchronous: nothing exits the process until this handler itself calls + // `exit()`, so awaiting first is safe. captureError's own default is a synchronous no-op, so `await`-ing it + // is a harmless no-op for every caller that doesn't pass one. + proc.on("uncaughtException", async (error) => { + log(`loopover-miner: uncaught exception: ${describeError(error)}`); + await captureError(error, { kind: "uncaughtException" }); + runCleanup(); + exit(1); }); - }; - - for (const [signal, code] of Object.entries(SIGNAL_EXIT_CODES)) { - proc.on(signal, () => { - log(`loopover-miner: received ${signal}, closing open resources and exiting.`); - runCleanup(); - exit(code); + proc.on("unhandledRejection", async (reason) => { + log(`loopover-miner: unhandled promise rejection: ${describeError(reason)}`); + await captureError(reason, { kind: "unhandledRejection" }); + runCleanup(); + exit(1); }); - } - - // Awaited (not fire-and-forget): captureError is expected to both capture AND flush before returning (see - // captureMinerErrorAndFlush in bin/loopover-miner.js) -- Sentry.captureException only QUEUES an event, and - // process.exit() tears the process down immediately without waiting for any pending HTTP delivery, so a - // synchronous capture-then-exit would make the crash-capture path a near-total no-op in practice. Node does - // not require these handlers to be synchronous: nothing exits the process until this handler itself calls - // `exit()`, so awaiting first is safe. captureError's own default is a synchronous no-op, so `await`-ing it - // is a harmless no-op for every caller that doesn't pass one. - proc.on("uncaughtException", async (error) => { - log(`loopover-miner: uncaught exception: ${describeError(error)}`); - await captureError(error, { kind: "uncaughtException" }); - runCleanup(); - exit(1); - }); - - proc.on("unhandledRejection", async (reason) => { - log(`loopover-miner: unhandled promise rejection: ${describeError(reason)}`); - await captureError(reason, { kind: "unhandledRejection" }); - runCleanup(); - exit(1); - }); - - return true; + return true; } - /** Test-only: clear the registry and the installed flag so each test starts from a clean lifecycle. */ export function resetProcessLifecycleForTesting() { - cleanupResources.clear(); - handlersInstalled = false; + cleanupResources.clear(); + handlersInstalled = false; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicHJvY2Vzcy1saWZlY3ljbGUuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJwcm9jZXNzLWxpZmVjeWNsZS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTs7Ozs7OzsrRkFPK0Y7QUF5Qi9GLGtIQUFrSDtBQUNsSCxzQkFBc0I7QUFDdEIsTUFBTSxpQkFBaUIsR0FBMkIsTUFBTSxDQUFDLE1BQU0sQ0FBQyxFQUFFLE1BQU0sRUFBRSxHQUFHLEVBQUUsT0FBTyxFQUFFLEdBQUcsRUFBRSxDQUFDLENBQUM7QUFFL0Y7NEVBQzRFO0FBQzVFLE1BQU0sZ0JBQWdCLEdBQUcsSUFBSSxHQUFHLEVBQW1CLENBQUM7QUFDcEQsSUFBSSxpQkFBaUIsR0FBRyxLQUFLLENBQUM7QUFFOUIsd0ZBQXdGO0FBQ3hGLFNBQVMsYUFBYSxDQUFDLEtBQWM7SUFDbkMsSUFBSSxLQUFLLFlBQVksS0FBSztRQUFFLE9BQU8sS0FBSyxDQUFDLEtBQUssSUFBSSxLQUFLLENBQUMsT0FBTyxDQUFDO0lBQ2hFLE9BQU8sTUFBTSxDQUFDLEtBQUssQ0FBQyxDQUFDO0FBQ3ZCLENBQUM7QUFFRDs7O0dBR0c7QUFDSCxNQUFNLFVBQVUsdUJBQXVCLENBQUMsUUFBNEM7SUFDbEYsSUFBSSxRQUFRLEtBQUssSUFBSSxJQUFJLFFBQVEsS0FBSyxTQUFTO1FBQUUsT0FBTyxHQUFHLEVBQUUsR0FBRSxDQUFDLENBQUM7SUFDakUsZ0JBQWdCLENBQUMsR0FBRyxDQUFDLFFBQVEsQ0FBQyxDQUFDO0lBQy9CLE9BQU8sR0FBRyxFQUFFO1FBQ1YsZ0JBQWdCLENBQUMsTUFBTSxDQUFDLFFBQVEsQ0FBQyxDQUFDO0lBQ3BDLENBQUMsQ0FBQztBQUNKLENBQUM7QUFFRCwwRkFBMEY7QUFDMUYsTUFBTSxVQUFVLG9CQUFvQjtJQUNsQyxPQUFPLGdCQUFnQixDQUFDLElBQUksQ0FBQztBQUMvQixDQUFDO0FBRUQ7OztHQUdHO0FBQ0gsTUFBTSxVQUFVLHdCQUF3QixDQUFDLFVBQWtELEVBQUU7SUFDM0YsTUFBTSxPQUFPLEdBQUcsT0FBTyxPQUFPLENBQUMsT0FBTyxLQUFLLFVBQVUsQ0FBQyxDQUFDLENBQUMsT0FBTyxDQUFDLE9BQU8sQ0FBQyxDQUFDLENBQUMsSUFBSSxDQUFDO0lBQy9FLEtBQUssTUFBTSxRQUFRLElBQUksQ0FBQyxHQUFHLGdCQUFnQixDQUFDLEVBQUUsQ0FBQztRQUM3QyxJQUFJLENBQUM7WUFDSCxJQUFJLE9BQU8sUUFBUSxLQUFLLFVBQVU7Z0JBQUUsUUFBUSxFQUFFLENBQUM7O2dCQUMxQyxRQUFRLENBQUMsS0FBSyxFQUFFLENBQUM7UUFDeEIsQ0FBQztRQUFDLE9BQU8sS0FBSyxFQUFFLENBQUM7WUFDZixJQUFJLE9BQU87Z0JBQUUsT0FBTyxDQUFDLEtBQUssQ0FBQyxDQUFDO1FBQzlCLENBQUM7SUFDSCxDQUFDO0lBQ0QsZ0JBQWdCLENBQUMsS0FBSyxFQUFFLENBQUM7QUFDM0IsQ0FBQztBQUVEOzs7Ozs7R0FNRztBQUNILE1BQU0sVUFBVSx3QkFBd0IsQ0FBQyxVQUEyQyxFQUFFO0lBQ3BGLE1BQU0sSUFBSSxHQUFHLE9BQU8sQ0FBQyxPQUFPLElBQUssT0FBa0MsQ0FBQztJQUNwRSxNQUFNLEdBQUcsR0FBRyxPQUFPLE9BQU8sQ0FBQyxHQUFHLEtBQUssVUFBVSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsR0FBRyxDQUFDLENBQUMsQ0FBQyxDQUFDLE9BQWUsRUFBRSxFQUFFLENBQUMsT0FBTyxDQUFDLEtBQUssQ0FBQyxPQUFPLENBQUMsQ0FBQztJQUMxRyxNQUFNLElBQUksR0FBRyxPQUFPLE9BQU8sQ0FBQyxJQUFJLEtBQUssVUFBVSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsSUFBSSxDQUFDLENBQUMsQ0FBQyxDQUFDLElBQVksRUFBRSxFQUFFLENBQUMsSUFBSSxDQUFDLElBQUksQ0FBQyxJQUFJLENBQUMsQ0FBQztJQUNuRyx5R0FBeUc7SUFDekcsdUdBQXVHO0lBQ3ZHLG1EQUFtRDtJQUNuRCxNQUFNLFlBQVksR0FBRyxPQUFPLE9BQU8sQ0FBQyxZQUFZLEtBQUssVUFBVSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsWUFBWSxDQUFDLENBQUMsQ0FBQyxHQUFHLEVBQUUsR0FBRSxDQUFDLENBQUM7SUFFbEcsSUFBSSxpQkFBaUIsSUFBSSxPQUFPLENBQUMsS0FBSyxLQUFLLElBQUk7UUFBRSxPQUFPLEtBQUssQ0FBQztJQUM5RCxpQkFBaUIsR0FBRyxJQUFJLENBQUM7SUFFekIsTUFBTSxVQUFVLEdBQUcsR0FBRyxFQUFFO1FBQ3RCLHdCQUF3QixDQUFDO1lBQ3ZCLE9BQU8sRUFBRSxDQUFDLEtBQUssRUFBRSxFQUFFLENBQUMsR0FBRyxDQUFDLGdEQUFnRCxhQUFhLENBQUMsS0FBSyxDQUFDLEVBQUUsQ0FBQztTQUNoRyxDQUFDLENBQUM7SUFDTCxDQUFDLENBQUM7SUFFRixLQUFLLE1BQU0sQ0FBQyxNQUFNLEVBQUUsSUFBSSxDQUFDLElBQUksTUFBTSxDQUFDLE9BQU8sQ0FBQyxpQkFBaUIsQ0FBQyxFQUFFLENBQUM7UUFDL0QsSUFBSSxDQUFDLEVBQUUsQ0FBQyxNQUFNLEVBQUUsR0FBRyxFQUFFO1lBQ25CLEdBQUcsQ0FBQyw0QkFBNEIsTUFBTSx1Q0FBdUMsQ0FBQyxDQUFDO1lBQy9FLFVBQVUsRUFBRSxDQUFDO1lBQ2IsSUFBSSxDQUFDLElBQUksQ0FBQyxDQUFDO1FBQ2IsQ0FBQyxDQUFDLENBQUM7SUFDTCxDQUFDO0lBRUQsMEdBQTBHO0lBQzFHLDJHQUEyRztJQUMzRyx3R0FBd0c7SUFDeEcsNEdBQTRHO0lBQzVHLDBHQUEwRztJQUMxRyw0R0FBNEc7SUFDNUcsOERBQThEO0lBQzlELElBQUksQ0FBQyxFQUFFLENBQUMsbUJBQW1CLEVBQUUsS0FBSyxFQUFFLEtBQWMsRUFBRSxFQUFFO1FBQ3BELEdBQUcsQ0FBQyx1Q0FBdUMsYUFBYSxDQUFDLEtBQUssQ0FBQyxFQUFFLENBQUMsQ0FBQztRQUNuRSxNQUFNLFlBQVksQ0FBQyxLQUFLLEVBQUUsRUFBRSxJQUFJLEVBQUUsbUJBQW1CLEVBQUUsQ0FBQyxDQUFDO1FBQ3pELFVBQVUsRUFBRSxDQUFDO1FBQ2IsSUFBSSxDQUFDLENBQUMsQ0FBQyxDQUFDO0lBQ1YsQ0FBQyxDQUFDLENBQUM7SUFFSCxJQUFJLENBQUMsRUFBRSxDQUFDLG9CQUFvQixFQUFFLEtBQUssRUFBRSxNQUFlLEVBQUUsRUFBRTtRQUN0RCxHQUFHLENBQUMsZ0RBQWdELGFBQWEsQ0FBQyxNQUFNLENBQUMsRUFBRSxDQUFDLENBQUM7UUFDN0UsTUFBTSxZQUFZLENBQUMsTUFBTSxFQUFFLEVBQUUsSUFBSSxFQUFFLG9CQUFvQixFQUFFLENBQUMsQ0FBQztRQUMzRCxVQUFVLEVBQUUsQ0FBQztRQUNiLElBQUksQ0FBQyxDQUFDLENBQUMsQ0FBQztJQUNWLENBQUMsQ0FBQyxDQUFDO0lBRUgsT0FBTyxJQUFJLENBQUM7QUFDZCxDQUFDO0FBRUQsdUdBQXVHO0FBQ3ZHLE1BQU0sVUFBVSwrQkFBK0I7SUFDN0MsZ0JBQWdCLENBQUMsS0FBSyxFQUFFLENBQUM7SUFDekIsaUJBQWlCLEdBQUcsS0FBSyxDQUFDO0FBQzVCLENBQUMifQ== \ No newline at end of file diff --git a/packages/loopover-miner/lib/process-lifecycle.ts b/packages/loopover-miner/lib/process-lifecycle.ts new file mode 100644 index 0000000000..a89178ae0c --- /dev/null +++ b/packages/loopover-miner/lib/process-lifecycle.ts @@ -0,0 +1,143 @@ +/** Process lifecycle / crash-safety for the miner CLI (#4826). The CLI dispatches through a chain of bare + * `process.exit()` calls with no cleanup hook, so a SIGINT/SIGTERM mid-run — or an uncaught exception — used to + * kill the process mid-write, leaving whatever local SQLite ledger it was touching in an undefined state. This + * module is the single cleanup chokepoint: local stores register themselves when opened (see `local-store.js`), and + * `installCliSignalHandlers` (called once at CLI startup) flushes/closes every still-open resource before exiting + * cleanly on a signal, and logs + exits non-zero on an uncaught exception / unhandled rejection instead of crashing + * silently. Cleanup ONLY — no command business logic lives here. Every dependency (`process`, `log`, `exit`) is + * injectable so the handlers are unit-testable without actually signalling the test runner. */ + +/** A closable store (`{ close() }`) or a plain cleanup callback. */ +export type CleanupResource = { close: () => void } | (() => void); + +/** The subset of `process` the handlers use; injectable for tests. */ +export type ProcessLike = { + on: (event: string, listener: (...args: unknown[]) => void) => unknown; + exit: (code?: number) => void; +}; + +export type InstallCliSignalHandlersOptions = { + process?: ProcessLike; + log?: (message: string) => void; + exit?: (code: number) => void; + /** Called (in addition to `log`) for uncaughtException/unhandledRejection specifically -- not the clean + * SIGINT/SIGTERM exits, which are not errors. AWAITED before the process exits, so it should both capture + * AND flush (see captureMinerErrorAndFlush in bin/loopover-miner.js) -- a synchronous capture alone only + * queues the event, which process.exit() would then likely never deliver. No-op default. Never expected to + * throw/reject. */ + captureError?: (error: unknown, context?: Record) => void | Promise; + /** Reinstall even if handlers were already installed (mainly for tests). */ + force?: boolean; +}; + +// 128 + signal number, the conventional shell exit code for a process terminated by that signal (SIGINT=2 -> 130, +// SIGTERM=15 -> 143). +const SIGNAL_EXIT_CODES: Record = Object.freeze({ SIGINT: 130, SIGTERM: 143 }); + +/** Resources to close on exit. A resource is either a `{ close() }` object (e.g. an open SQLite store) or a plain + * cleanup function. Held in insertion order so cleanup is deterministic. */ +const cleanupResources = new Set(); +let handlersInstalled = false; + +/** Render any thrown value as a single log-safe string, preferring an Error's stack. */ +function describeError(value: unknown): string { + if (value instanceof Error) return value.stack ?? value.message; + return String(value); +} + +/** + * Register a resource to be closed on clean exit or crash. Returns an idempotent unregister function (call it from + * the resource's own normal `close()` so a resource closed during the happy path is not double-closed at exit). + */ +export function registerCleanupResource(resource: CleanupResource | null | undefined): () => void { + if (resource === null || resource === undefined) return () => {}; + cleanupResources.add(resource); + return () => { + cleanupResources.delete(resource); + }; +} + +/** Number of currently-registered cleanup resources (exposed for tests / diagnostics). */ +export function cleanupResourceCount(): number { + return cleanupResources.size; +} + +/** + * Close every registered resource, swallowing each individual failure (a store that fails to close must not stop + * the others from closing) and reporting it via `options.onError`. Idempotent: the registry is emptied afterwards. + */ +export function closeAllCleanupResources(options: { onError?: (error: unknown) => void } = {}): void { + const onError = typeof options.onError === "function" ? options.onError : null; + for (const resource of [...cleanupResources]) { + try { + if (typeof resource === "function") resource(); + else resource.close(); + } catch (error) { + if (onError) onError(error); + } + } + cleanupResources.clear(); +} + +/** + * Install top-level signal + error handlers once. On SIGINT/SIGTERM: close all resources and exit with the + * conventional 128+signal code. On uncaughtException/unhandledRejection: log the error, AWAIT the optional + * captureError hook (so a captured Sentry event has a chance to actually flush before the process exits), + * close all resources, and exit non-zero. No-op (returns false) if already installed unless `options.force` is + * set. All of `process`, `log`, `exit`, and `captureError` are injectable for testing. + */ +export function installCliSignalHandlers(options: InstallCliSignalHandlersOptions = {}): boolean { + const proc = options.process ?? (process as unknown as ProcessLike); + const log = typeof options.log === "function" ? options.log : (message: string) => console.error(message); + const exit = typeof options.exit === "function" ? options.exit : (code: number) => proc.exit(code); + // Optional Sentry (or any) capture hook -- decoupled from a specific implementation so this module stays + // fully unit-testable without mocking Sentry (#6011). No-op default matches this module's pre-existing + // behavior for every caller that doesn't pass one. + const captureError = typeof options.captureError === "function" ? options.captureError : () => {}; + + if (handlersInstalled && options.force !== true) return false; + handlersInstalled = true; + + const runCleanup = () => { + closeAllCleanupResources({ + onError: (error) => log(`loopover-miner: cleanup error while exiting: ${describeError(error)}`), + }); + }; + + for (const [signal, code] of Object.entries(SIGNAL_EXIT_CODES)) { + proc.on(signal, () => { + log(`loopover-miner: received ${signal}, closing open resources and exiting.`); + runCleanup(); + exit(code); + }); + } + + // Awaited (not fire-and-forget): captureError is expected to both capture AND flush before returning (see + // captureMinerErrorAndFlush in bin/loopover-miner.js) -- Sentry.captureException only QUEUES an event, and + // process.exit() tears the process down immediately without waiting for any pending HTTP delivery, so a + // synchronous capture-then-exit would make the crash-capture path a near-total no-op in practice. Node does + // not require these handlers to be synchronous: nothing exits the process until this handler itself calls + // `exit()`, so awaiting first is safe. captureError's own default is a synchronous no-op, so `await`-ing it + // is a harmless no-op for every caller that doesn't pass one. + proc.on("uncaughtException", async (error: unknown) => { + log(`loopover-miner: uncaught exception: ${describeError(error)}`); + await captureError(error, { kind: "uncaughtException" }); + runCleanup(); + exit(1); + }); + + proc.on("unhandledRejection", async (reason: unknown) => { + log(`loopover-miner: unhandled promise rejection: ${describeError(reason)}`); + await captureError(reason, { kind: "unhandledRejection" }); + runCleanup(); + exit(1); + }); + + return true; +} + +/** Test-only: clear the registry and the installed flag so each test starts from a clean lifecycle. */ +export function resetProcessLifecycleForTesting(): void { + cleanupResources.clear(); + handlersInstalled = false; +} diff --git a/packages/loopover-miner/lib/rejection-state-machine.d.ts b/packages/loopover-miner/lib/rejection-state-machine.d.ts index 35b15d5600..574353da0b 100644 --- a/packages/loopover-miner/lib/rejection-state-machine.d.ts +++ b/packages/loopover-miner/lib/rejection-state-machine.d.ts @@ -1,37 +1,48 @@ -import type { RejectionReason, RejectionContext } from "./rejection-templates.js"; - +import type { RejectionContext, RejectionReason } from "./rejection-templates.js"; export type PrOutcomeFields = { - state: string | null; - merged: boolean; - mergedAt: string | null; - closedAt: string | null; + state: string | null; + merged: boolean; + mergedAt: string | null; + closedAt: string | null; }; - export type RejectionSignal = { - gateClosed?: boolean; - supersededByDuplicate?: boolean; + gateClosed?: boolean; + supersededByDuplicate?: boolean; }; - export type RejectionTransition = { - outcome: "disengaged"; - reason: RejectionReason; - note: string; - fields: PrOutcomeFields; + outcome: "disengaged"; + reason: RejectionReason; + note: string; + fields: PrOutcomeFields; }; - -/** Per-PR terminal outcome for a rejected (closed-without-merge) PR. */ -export const DISENGAGED_OUTCOME: "disengaged"; - -export function extractPrOutcomeFields(prPayload: unknown): PrOutcomeFields; - -export function isRejectedPr( - fields: { state?: string | null; merged?: boolean } | null | undefined, -): boolean; - -export function classifyRejectionReason(signal?: RejectionSignal): RejectionReason; - -export function resolveRejection( - prPayload: unknown, - signal: RejectionSignal | undefined, - context: RejectionContext, -): RejectionTransition | null; +/** Per-PR terminal outcome for a rejected (closed-without-merge) PR. A poller adds this to its own outcome + * vocabulary alongside ready / needs-work / open. */ +export declare const DISENGAGED_OUTCOME = "disengaged"; +/** + * Pull the terminal-outcome fields from a `GET /pulls/{n}` payload the poller already has. Pure — no API call. + * Missing/malformed fields normalize to null/false so a partial payload never throws here. + */ +export declare function extractPrOutcomeFields(prPayload: unknown): PrOutcomeFields; +/** + * True when a PR is closed WITHOUT a merge — the rejection this state machine acts on. A merged PR (even though + * GitHub also marks it `state: "closed"`) is NOT a rejection. Pure. + */ +export declare function isRejectedPr(fields: { + state?: string | null; + merged?: boolean; +} | null | undefined): boolean; +/** + * Classify a detected rejection into one of the rejection-reason buckets from the available signal. + * Precedence: an explicit gate close outranks a duplicate signal (the gate is the more specific, actionable + * cause). With neither signal, defaults to `maintainer_close_no_reason` (the documented zero-signal fallback). + * Pure. + */ +export declare function classifyRejectionReason(signal?: RejectionSignal): RejectionReason; +/** + * The full transition. Given a PR payload, an optional gate/duplicate signal, and the render context + * (`{ repoFullName, prNumber }`), decide whether the PR is a rejection and, if so, produce the disengaged + * transition: the classified reason and the rendered courtesy note (this is `renderRejectionMessage`'s first + * real caller). Returns null when the PR is not a rejection (still open, or merged) — nothing to disengage. + * Pure and deterministic; the caller persists `{ outcome, reason, note }` via its local event ledger. + */ +export declare function resolveRejection(prPayload: unknown, signal: RejectionSignal | undefined, context: RejectionContext): RejectionTransition | null; diff --git a/packages/loopover-miner/lib/rejection-state-machine.js b/packages/loopover-miner/lib/rejection-state-machine.js index 687543b717..c5ee1e4f6b 100644 --- a/packages/loopover-miner/lib/rejection-state-machine.js +++ b/packages/loopover-miner/lib/rejection-state-machine.js @@ -15,67 +15,58 @@ // • This surfaces the PR's terminal fields from a payload the poller already fetches (ci-poller.js's // `fetchHeadSha` GETs the full `/pulls/{n}` body, :155-163, and discards all but `head.sha`) via a pure // extractor — no second API call, and no behavioral change to the existing fetch. - import { renderRejectionMessage } from "./rejection-templates.js"; - /** Per-PR terminal outcome for a rejected (closed-without-merge) PR. A poller adds this to its own outcome * vocabulary alongside ready / needs-work / open. */ export const DISENGAGED_OUTCOME = "disengaged"; - /** * Pull the terminal-outcome fields from a `GET /pulls/{n}` payload the poller already has. Pure — no API call. * Missing/malformed fields normalize to null/false so a partial payload never throws here. - * @param {unknown} prPayload - * @returns {{ state: string | null, merged: boolean, mergedAt: string | null, closedAt: string | null }} */ export function extractPrOutcomeFields(prPayload) { - const p = prPayload && typeof prPayload === "object" ? prPayload : {}; - return { - state: typeof p.state === "string" ? p.state : null, - merged: p.merged === true, - mergedAt: typeof p.merged_at === "string" ? p.merged_at : null, - closedAt: typeof p.closed_at === "string" ? p.closed_at : null, - }; + const p = (prPayload && typeof prPayload === "object" ? prPayload : {}); + return { + state: typeof p.state === "string" ? p.state : null, + merged: p.merged === true, + mergedAt: typeof p.merged_at === "string" ? p.merged_at : null, + closedAt: typeof p.closed_at === "string" ? p.closed_at : null, + }; } - /** * True when a PR is closed WITHOUT a merge — the rejection this state machine acts on. A merged PR (even though * GitHub also marks it `state: "closed"`) is NOT a rejection. Pure. - * @param {{ state?: string | null, merged?: boolean }} fields */ export function isRejectedPr(fields) { - const f = fields && typeof fields === "object" ? fields : {}; - return f.state === "closed" && f.merged !== true; + const f = (fields && typeof fields === "object" ? fields : {}); + return f.state === "closed" && f.merged !== true; } - /** * Classify a detected rejection into one of the rejection-reason buckets from the available signal. * Precedence: an explicit gate close outranks a duplicate signal (the gate is the more specific, actionable * cause). With neither signal, defaults to `maintainer_close_no_reason` (the documented zero-signal fallback). * Pure. - * @param {{ gateClosed?: boolean, supersededByDuplicate?: boolean }} [signal] - * @returns {"gate_close" | "superseded_by_duplicate" | "maintainer_close_no_reason"} */ export function classifyRejectionReason(signal = {}) { - const s = signal && typeof signal === "object" ? signal : {}; - if (s.gateClosed === true) return "gate_close"; - if (s.supersededByDuplicate === true) return "superseded_by_duplicate"; - return "maintainer_close_no_reason"; + const s = (signal && typeof signal === "object" ? signal : {}); + if (s.gateClosed === true) + return "gate_close"; + if (s.supersededByDuplicate === true) + return "superseded_by_duplicate"; + return "maintainer_close_no_reason"; } - /** * The full transition. Given a PR payload, an optional gate/duplicate signal, and the render context * (`{ repoFullName, prNumber }`), decide whether the PR is a rejection and, if so, produce the disengaged * transition: the classified reason and the rendered courtesy note (this is `renderRejectionMessage`'s first * real caller). Returns null when the PR is not a rejection (still open, or merged) — nothing to disengage. * Pure and deterministic; the caller persists `{ outcome, reason, note }` via its local event ledger. - * @returns {{ outcome: string, reason: string, note: string, - * fields: ReturnType } | null} */ export function resolveRejection(prPayload, signal, context) { - const fields = extractPrOutcomeFields(prPayload); - if (!isRejectedPr(fields)) return null; - const reason = classifyRejectionReason(signal); - const note = renderRejectionMessage(reason, context); // throws on malformed context — a half-note never emits - return { outcome: DISENGAGED_OUTCOME, reason, note, fields }; + const fields = extractPrOutcomeFields(prPayload); + if (!isRejectedPr(fields)) + return null; + const reason = classifyRejectionReason(signal); + const note = renderRejectionMessage(reason, context); // throws on malformed context — a half-note never emits + return { outcome: DISENGAGED_OUTCOME, reason, note, fields }; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicmVqZWN0aW9uLXN0YXRlLW1hY2hpbmUuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJyZWplY3Rpb24tc3RhdGUtbWFjaGluZS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSwwR0FBMEc7QUFDMUcsMEZBQTBGO0FBQzFGLDJHQUEyRztBQUMzRyw0R0FBNEc7QUFDNUcsRUFBRTtBQUNGLHFEQUFxRDtBQUNyRCw2R0FBNkc7QUFDN0csNEdBQTRHO0FBQzVHLDhHQUE4RztBQUM5Ryw0R0FBNEc7QUFDNUcsa0dBQWtHO0FBQ2xHLGtIQUFrSDtBQUNsSCxnSEFBZ0g7QUFDaEgsY0FBYztBQUNkLHVHQUF1RztBQUN2Ryw0R0FBNEc7QUFDNUcsc0ZBQXNGO0FBRXRGLE9BQU8sRUFBRSxzQkFBc0IsRUFBRSxNQUFNLDBCQUEwQixDQUFDO0FBc0JsRTtzREFDc0Q7QUFDdEQsTUFBTSxDQUFDLE1BQU0sa0JBQWtCLEdBQUcsWUFBWSxDQUFDO0FBRS9DOzs7R0FHRztBQUNILE1BQU0sVUFBVSxzQkFBc0IsQ0FBQyxTQUFrQjtJQUN2RCxNQUFNLENBQUMsR0FBRyxDQUFDLFNBQVMsSUFBSSxPQUFPLFNBQVMsS0FBSyxRQUFRLENBQUMsQ0FBQyxDQUFDLFNBQVMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUE0QixDQUFDO0lBQ25HLE9BQU87UUFDTCxLQUFLLEVBQUUsT0FBTyxDQUFDLENBQUMsS0FBSyxLQUFLLFFBQVEsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEtBQUssQ0FBQyxDQUFDLENBQUMsSUFBSTtRQUNuRCxNQUFNLEVBQUUsQ0FBQyxDQUFDLE1BQU0sS0FBSyxJQUFJO1FBQ3pCLFFBQVEsRUFBRSxPQUFPLENBQUMsQ0FBQyxTQUFTLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsU0FBUyxDQUFDLENBQUMsQ0FBQyxJQUFJO1FBQzlELFFBQVEsRUFBRSxPQUFPLENBQUMsQ0FBQyxTQUFTLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsU0FBUyxDQUFDLENBQUMsQ0FBQyxJQUFJO0tBQy9ELENBQUM7QUFDSixDQUFDO0FBRUQ7OztHQUdHO0FBQ0gsTUFBTSxVQUFVLFlBQVksQ0FBQyxNQUFzRTtJQUNqRyxNQUFNLENBQUMsR0FBRyxDQUFDLE1BQU0sSUFBSSxPQUFPLE1BQU0sS0FBSyxRQUFRLENBQUMsQ0FBQyxDQUFDLE1BQU0sQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFnRCxDQUFDO0lBQzlHLE9BQU8sQ0FBQyxDQUFDLEtBQUssS0FBSyxRQUFRLElBQUksQ0FBQyxDQUFDLE1BQU0sS0FBSyxJQUFJLENBQUM7QUFDbkQsQ0FBQztBQUVEOzs7OztHQUtHO0FBQ0gsTUFBTSxVQUFVLHVCQUF1QixDQUFDLFNBQTBCLEVBQUU7SUFDbEUsTUFBTSxDQUFDLEdBQUcsQ0FBQyxNQUFNLElBQUksT0FBTyxNQUFNLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxNQUFNLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBb0IsQ0FBQztJQUNsRixJQUFJLENBQUMsQ0FBQyxVQUFVLEtBQUssSUFBSTtRQUFFLE9BQU8sWUFBWSxDQUFDO0lBQy9DLElBQUksQ0FBQyxDQUFDLHFCQUFxQixLQUFLLElBQUk7UUFBRSxPQUFPLHlCQUF5QixDQUFDO0lBQ3ZFLE9BQU8sNEJBQTRCLENBQUM7QUFDdEMsQ0FBQztBQUVEOzs7Ozs7R0FNRztBQUNILE1BQU0sVUFBVSxnQkFBZ0IsQ0FDOUIsU0FBa0IsRUFDbEIsTUFBbUMsRUFDbkMsT0FBeUI7SUFFekIsTUFBTSxNQUFNLEdBQUcsc0JBQXNCLENBQUMsU0FBUyxDQUFDLENBQUM7SUFDakQsSUFBSSxDQUFDLFlBQVksQ0FBQyxNQUFNLENBQUM7UUFBRSxPQUFPLElBQUksQ0FBQztJQUN2QyxNQUFNLE1BQU0sR0FBRyx1QkFBdUIsQ0FBQyxNQUFNLENBQUMsQ0FBQztJQUMvQyxNQUFNLElBQUksR0FBRyxzQkFBc0IsQ0FBQyxNQUFNLEVBQUUsT0FBTyxDQUFDLENBQUMsQ0FBQyx3REFBd0Q7SUFDOUcsT0FBTyxFQUFFLE9BQU8sRUFBRSxrQkFBa0IsRUFBRSxNQUFNLEVBQUUsSUFBSSxFQUFFLE1BQU0sRUFBRSxDQUFDO0FBQy9ELENBQUMifQ== \ No newline at end of file diff --git a/packages/loopover-miner/lib/rejection-state-machine.ts b/packages/loopover-miner/lib/rejection-state-machine.ts new file mode 100644 index 0000000000..6f29068eaf --- /dev/null +++ b/packages/loopover-miner/lib/rejection-state-machine.ts @@ -0,0 +1,98 @@ +// Rejection state machine (#4278): the missing detector + classifier that turns a closed-without-merge PR +// into a rejection-reason bucket and, for the first time, drives `renderRejectionMessage` +// (rejection-templates.js, which until now had zero callers outside its own test). Pure classification and +// content only — no GitHub calls, no network, no writes. The caller (a poller) persists the result locally. +// +// DESIGN DECISIONS (called out explicitly by #4278): +// • "disengaged" is a per-PR OUTCOME, not a per-repo run-state. A rejection is about one PR, so it belongs +// with the `manage-poll.js` outcome family (ready / needs-work / open), NOT `run-state.js`'s RUN_STATES +// (idle / discovering / planning / preparing). `DISENGAGED_OUTCOME` is defined HERE and left for a poller +// to adopt — this module deliberately does NOT mutate manage-poll.js's or run-state.js's enum as a side +// effect (the issue explicitly warns against silently expanding another module's vocabulary). +// • Zero-signal fallback: with no gate/duplicate signal, a rejection classifies as `maintainer_close_no_reason` +// — the courteous, non-assuming bucket — rather than being left unclassified, so a rejection ALWAYS renders +// a note. +// • This surfaces the PR's terminal fields from a payload the poller already fetches (ci-poller.js's +// `fetchHeadSha` GETs the full `/pulls/{n}` body, :155-163, and discards all but `head.sha`) via a pure +// extractor — no second API call, and no behavioral change to the existing fetch. + +import { renderRejectionMessage } from "./rejection-templates.js"; +import type { RejectionContext, RejectionReason } from "./rejection-templates.js"; + +export type PrOutcomeFields = { + state: string | null; + merged: boolean; + mergedAt: string | null; + closedAt: string | null; +}; + +export type RejectionSignal = { + gateClosed?: boolean; + supersededByDuplicate?: boolean; +}; + +export type RejectionTransition = { + outcome: "disengaged"; + reason: RejectionReason; + note: string; + fields: PrOutcomeFields; +}; + +/** Per-PR terminal outcome for a rejected (closed-without-merge) PR. A poller adds this to its own outcome + * vocabulary alongside ready / needs-work / open. */ +export const DISENGAGED_OUTCOME = "disengaged"; + +/** + * Pull the terminal-outcome fields from a `GET /pulls/{n}` payload the poller already has. Pure — no API call. + * Missing/malformed fields normalize to null/false so a partial payload never throws here. + */ +export function extractPrOutcomeFields(prPayload: unknown): PrOutcomeFields { + const p = (prPayload && typeof prPayload === "object" ? prPayload : {}) as Record; + return { + state: typeof p.state === "string" ? p.state : null, + merged: p.merged === true, + mergedAt: typeof p.merged_at === "string" ? p.merged_at : null, + closedAt: typeof p.closed_at === "string" ? p.closed_at : null, + }; +} + +/** + * True when a PR is closed WITHOUT a merge — the rejection this state machine acts on. A merged PR (even though + * GitHub also marks it `state: "closed"`) is NOT a rejection. Pure. + */ +export function isRejectedPr(fields: { state?: string | null; merged?: boolean } | null | undefined): boolean { + const f = (fields && typeof fields === "object" ? fields : {}) as { state?: string | null; merged?: boolean }; + return f.state === "closed" && f.merged !== true; +} + +/** + * Classify a detected rejection into one of the rejection-reason buckets from the available signal. + * Precedence: an explicit gate close outranks a duplicate signal (the gate is the more specific, actionable + * cause). With neither signal, defaults to `maintainer_close_no_reason` (the documented zero-signal fallback). + * Pure. + */ +export function classifyRejectionReason(signal: RejectionSignal = {}): RejectionReason { + const s = (signal && typeof signal === "object" ? signal : {}) as RejectionSignal; + if (s.gateClosed === true) return "gate_close"; + if (s.supersededByDuplicate === true) return "superseded_by_duplicate"; + return "maintainer_close_no_reason"; +} + +/** + * The full transition. Given a PR payload, an optional gate/duplicate signal, and the render context + * (`{ repoFullName, prNumber }`), decide whether the PR is a rejection and, if so, produce the disengaged + * transition: the classified reason and the rendered courtesy note (this is `renderRejectionMessage`'s first + * real caller). Returns null when the PR is not a rejection (still open, or merged) — nothing to disengage. + * Pure and deterministic; the caller persists `{ outcome, reason, note }` via its local event ledger. + */ +export function resolveRejection( + prPayload: unknown, + signal: RejectionSignal | undefined, + context: RejectionContext, +): RejectionTransition | null { + const fields = extractPrOutcomeFields(prPayload); + if (!isRejectedPr(fields)) return null; + const reason = classifyRejectionReason(signal); + const note = renderRejectionMessage(reason, context); // throws on malformed context — a half-note never emits + return { outcome: DISENGAGED_OUTCOME, reason, note, fields }; +} diff --git a/packages/loopover-miner/lib/sentry.d.ts b/packages/loopover-miner/lib/sentry.d.ts index ea60381bad..ece789250c 100644 --- a/packages/loopover-miner/lib/sentry.d.ts +++ b/packages/loopover-miner/lib/sentry.d.ts @@ -1,17 +1,24 @@ -/** Opt-in Sentry error tracking for the miner CLI. Complete no-op unless LOOPOVER_MINER_SENTRY_DSN is set. */ - -/** Initialize Sentry from `env` (default `process.env`). Returns whether it activated. */ -export function initMinerSentry(env?: Record): Promise; - +/** Opt-in Sentry error tracking for the miner CLI (#6011). Complete no-op unless LOOPOVER_MINER_SENTRY_DSN is + * set -- an operator points this at their OWN Sentry project; this is a published, independently-installed CLI + * (@loopover/miner), so nothing here is ever auto-enabled or phones home by default, mirroring the main repo's + * self-host Sentry integration (src/selfhost/sentry.ts). `@sentry/node` is lazy-imported only inside + * `initMinerSentry()` so a miner invocation that never opts in pays zero module-load cost -- this CLI runs very + * frequently under an unattended loop (lib/loop-cli.js). Unlike the main repo, there is no structured JSON-log + * forwarding here: this package's own logger (lib/logger.js) writes plain `key=value` lines, not JSON, so + * capture is explicit (`captureMinerError`) at each call site rather than a console-override. */ +/** Initialize Sentry from `env` (default `process.env`). Returns whether it activated. Call once, as early as + * possible in a bin's startup -- after `loadMinerFileSecrets()` (so a `_FILE`-mounted DSN resolves first) and + * before `installCliSignalHandlers()` (so a startup crash is still captured). */ +export declare function initMinerSentry(env?: Record): Promise; /** Capture an error with optional structured context. No-op when Sentry is off. Never throws. */ -export function captureMinerError(error: unknown, context?: Record): void; - -/** Flush buffered events before the process exits. No-op when off. */ -export function flushMinerSentry(timeoutMs?: number): Promise; - +export declare function captureMinerError(error: unknown, context?: Record): void; +/** Flush buffered events before the process exits. No-op when off. Never throws or hangs past `timeoutMs`. */ +export declare function flushMinerSentry(timeoutMs?: number): Promise; /** Capture AND flush before returning -- the crash-path convenience wrapper for - * installCliSignalHandlers' `captureError` hook. */ -export function captureMinerErrorAndFlush(error: unknown, context?: Record): Promise; - + * installCliSignalHandlers' `captureError` hook (process-lifecycle.js). A bare `captureMinerError()` only + * QUEUES the event in Sentry's transport; `process.exit()` tears the process down immediately afterward + * without waiting for any pending HTTP delivery, so the crash-capture path needs this awaited flush or it is + * very likely a near-total no-op in practice. */ +export declare function captureMinerErrorAndFlush(error: unknown, context?: Record): Promise; /** Test-only: reset module state so one test's activation can't leak into the next. */ -export function resetMinerSentryForTesting(): void; +export declare function resetMinerSentryForTesting(): void; diff --git a/packages/loopover-miner/lib/sentry.js b/packages/loopover-miner/lib/sentry.js index c38a133805..ef6f6b5349 100644 --- a/packages/loopover-miner/lib/sentry.js +++ b/packages/loopover-miner/lib/sentry.js @@ -6,60 +6,61 @@ * frequently under an unattended loop (lib/loop-cli.js). Unlike the main repo, there is no structured JSON-log * forwarding here: this package's own logger (lib/logger.js) writes plain `key=value` lines, not JSON, so * capture is explicit (`captureMinerError`) at each call site rather than a console-override. */ - let Sentry; let active = false; - /** Initialize Sentry from `env` (default `process.env`). Returns whether it activated. Call once, as early as * possible in a bin's startup -- after `loadMinerFileSecrets()` (so a `_FILE`-mounted DSN resolves first) and * before `installCliSignalHandlers()` (so a startup crash is still captured). */ export async function initMinerSentry(env = process.env) { - if (!env.LOOPOVER_MINER_SENTRY_DSN) return false; - const mod = await import("@sentry/node"); - Sentry = mod; - Sentry.init({ - dsn: env.LOOPOVER_MINER_SENTRY_DSN, - environment: env.LOOPOVER_MINER_SENTRY_ENVIRONMENT ?? "production", - }); - active = true; - return true; + if (!env.LOOPOVER_MINER_SENTRY_DSN) + return false; + const mod = await import("@sentry/node"); + Sentry = mod; + Sentry.init({ + dsn: env.LOOPOVER_MINER_SENTRY_DSN, + environment: env.LOOPOVER_MINER_SENTRY_ENVIRONMENT ?? "production", + }); + active = true; + return true; } - /** Capture an error with optional structured context. No-op when Sentry is off. Never throws. */ export function captureMinerError(error, context) { - if (!active || !Sentry) return; - try { - Sentry.withScope((scope) => { - if (context) scope.setContext("miner", context); - Sentry.captureException(error instanceof Error ? error : new Error(String(error))); - }); - } catch { - /* Sentry capture must never crash the caller it's instrumenting. */ - } + if (!active || !Sentry) + return; + try { + Sentry.withScope((scope) => { + if (context) + scope.setContext("miner", context); + Sentry.captureException(error instanceof Error ? error : new Error(String(error))); + }); + } + catch { + /* Sentry capture must never crash the caller it's instrumenting. */ + } } - /** Flush buffered events before the process exits. No-op when off. Never throws or hangs past `timeoutMs`. */ export async function flushMinerSentry(timeoutMs = 2000) { - if (!active || !Sentry) return; - try { - await Sentry.flush(timeoutMs); - } catch { - /* Best-effort -- a flush failure must never block process exit. */ - } + if (!active || !Sentry) + return; + try { + await Sentry.flush(timeoutMs); + } + catch { + /* Best-effort -- a flush failure must never block process exit. */ + } } - /** Capture AND flush before returning -- the crash-path convenience wrapper for * installCliSignalHandlers' `captureError` hook (process-lifecycle.js). A bare `captureMinerError()` only * QUEUES the event in Sentry's transport; `process.exit()` tears the process down immediately afterward * without waiting for any pending HTTP delivery, so the crash-capture path needs this awaited flush or it is * very likely a near-total no-op in practice. */ export async function captureMinerErrorAndFlush(error, context) { - captureMinerError(error, context); - await flushMinerSentry(); + captureMinerError(error, context); + await flushMinerSentry(); } - /** Test-only: reset module state so one test's activation can't leak into the next. */ export function resetMinerSentryForTesting() { - Sentry = undefined; - active = false; + Sentry = undefined; + active = false; } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2VudHJ5LmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsic2VudHJ5LnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBOzs7Ozs7O2lHQU9pRztBQUlqRyxJQUFJLE1BQTRCLENBQUM7QUFDakMsSUFBSSxNQUFNLEdBQUcsS0FBSyxDQUFDO0FBRW5COztpRkFFaUY7QUFDakYsTUFBTSxDQUFDLEtBQUssVUFBVSxlQUFlLENBQUMsTUFBMEMsT0FBTyxDQUFDLEdBQUc7SUFDekYsSUFBSSxDQUFDLEdBQUcsQ0FBQyx5QkFBeUI7UUFBRSxPQUFPLEtBQUssQ0FBQztJQUNqRCxNQUFNLEdBQUcsR0FBRyxNQUFNLE1BQU0sQ0FBQyxjQUFjLENBQUMsQ0FBQztJQUN6QyxNQUFNLEdBQUcsR0FBRyxDQUFDO0lBQ2IsTUFBTSxDQUFDLElBQUksQ0FBQztRQUNWLEdBQUcsRUFBRSxHQUFHLENBQUMseUJBQXlCO1FBQ2xDLFdBQVcsRUFBRSxHQUFHLENBQUMsaUNBQWlDLElBQUksWUFBWTtLQUNuRSxDQUFDLENBQUM7SUFDSCxNQUFNLEdBQUcsSUFBSSxDQUFDO0lBQ2QsT0FBTyxJQUFJLENBQUM7QUFDZCxDQUFDO0FBRUQsaUdBQWlHO0FBQ2pHLE1BQU0sVUFBVSxpQkFBaUIsQ0FBQyxLQUFjLEVBQUUsT0FBaUM7SUFDakYsSUFBSSxDQUFDLE1BQU0sSUFBSSxDQUFDLE1BQU07UUFBRSxPQUFPO0lBQy9CLElBQUksQ0FBQztRQUNILE1BQU0sQ0FBQyxTQUFTLENBQUMsQ0FBQyxLQUFLLEVBQUUsRUFBRTtZQUN6QixJQUFJLE9BQU87Z0JBQUUsS0FBSyxDQUFDLFVBQVUsQ0FBQyxPQUFPLEVBQUUsT0FBTyxDQUFDLENBQUM7WUFDaEQsTUFBTyxDQUFDLGdCQUFnQixDQUFDLEtBQUssWUFBWSxLQUFLLENBQUMsQ0FBQyxDQUFDLEtBQUssQ0FBQyxDQUFDLENBQUMsSUFBSSxLQUFLLENBQUMsTUFBTSxDQUFDLEtBQUssQ0FBQyxDQUFDLENBQUMsQ0FBQztRQUN0RixDQUFDLENBQUMsQ0FBQztJQUNMLENBQUM7SUFBQyxNQUFNLENBQUM7UUFDUCxvRUFBb0U7SUFDdEUsQ0FBQztBQUNILENBQUM7QUFFRCw4R0FBOEc7QUFDOUcsTUFBTSxDQUFDLEtBQUssVUFBVSxnQkFBZ0IsQ0FBQyxTQUFTLEdBQUcsSUFBSTtJQUNyRCxJQUFJLENBQUMsTUFBTSxJQUFJLENBQUMsTUFBTTtRQUFFLE9BQU87SUFDL0IsSUFBSSxDQUFDO1FBQ0gsTUFBTSxNQUFNLENBQUMsS0FBSyxDQUFDLFNBQVMsQ0FBQyxDQUFDO0lBQ2hDLENBQUM7SUFBQyxNQUFNLENBQUM7UUFDUCxtRUFBbUU7SUFDckUsQ0FBQztBQUNILENBQUM7QUFFRDs7OztpREFJaUQ7QUFDakQsTUFBTSxDQUFDLEtBQUssVUFBVSx5QkFBeUIsQ0FBQyxLQUFjLEVBQUUsT0FBaUM7SUFDL0YsaUJBQWlCLENBQUMsS0FBSyxFQUFFLE9BQU8sQ0FBQyxDQUFDO0lBQ2xDLE1BQU0sZ0JBQWdCLEVBQUUsQ0FBQztBQUMzQixDQUFDO0FBRUQsdUZBQXVGO0FBQ3ZGLE1BQU0sVUFBVSwwQkFBMEI7SUFDeEMsTUFBTSxHQUFHLFNBQVMsQ0FBQztJQUNuQixNQUFNLEdBQUcsS0FBSyxDQUFDO0FBQ2pCLENBQUMifQ== \ No newline at end of file diff --git a/packages/loopover-miner/lib/sentry.ts b/packages/loopover-miner/lib/sentry.ts new file mode 100644 index 0000000000..cc5a1b2c24 --- /dev/null +++ b/packages/loopover-miner/lib/sentry.ts @@ -0,0 +1,67 @@ +/** Opt-in Sentry error tracking for the miner CLI (#6011). Complete no-op unless LOOPOVER_MINER_SENTRY_DSN is + * set -- an operator points this at their OWN Sentry project; this is a published, independently-installed CLI + * (@loopover/miner), so nothing here is ever auto-enabled or phones home by default, mirroring the main repo's + * self-host Sentry integration (src/selfhost/sentry.ts). `@sentry/node` is lazy-imported only inside + * `initMinerSentry()` so a miner invocation that never opts in pays zero module-load cost -- this CLI runs very + * frequently under an unattended loop (lib/loop-cli.js). Unlike the main repo, there is no structured JSON-log + * forwarding here: this package's own logger (lib/logger.js) writes plain `key=value` lines, not JSON, so + * capture is explicit (`captureMinerError`) at each call site rather than a console-override. */ + +type SentryNs = typeof import("@sentry/node"); + +let Sentry: SentryNs | undefined; +let active = false; + +/** Initialize Sentry from `env` (default `process.env`). Returns whether it activated. Call once, as early as + * possible in a bin's startup -- after `loadMinerFileSecrets()` (so a `_FILE`-mounted DSN resolves first) and + * before `installCliSignalHandlers()` (so a startup crash is still captured). */ +export async function initMinerSentry(env: Record = process.env): Promise { + if (!env.LOOPOVER_MINER_SENTRY_DSN) return false; + const mod = await import("@sentry/node"); + Sentry = mod; + Sentry.init({ + dsn: env.LOOPOVER_MINER_SENTRY_DSN, + environment: env.LOOPOVER_MINER_SENTRY_ENVIRONMENT ?? "production", + }); + active = true; + return true; +} + +/** Capture an error with optional structured context. No-op when Sentry is off. Never throws. */ +export function captureMinerError(error: unknown, context?: Record): void { + if (!active || !Sentry) return; + try { + Sentry.withScope((scope) => { + if (context) scope.setContext("miner", context); + Sentry!.captureException(error instanceof Error ? error : new Error(String(error))); + }); + } catch { + /* Sentry capture must never crash the caller it's instrumenting. */ + } +} + +/** Flush buffered events before the process exits. No-op when off. Never throws or hangs past `timeoutMs`. */ +export async function flushMinerSentry(timeoutMs = 2000): Promise { + if (!active || !Sentry) return; + try { + await Sentry.flush(timeoutMs); + } catch { + /* Best-effort -- a flush failure must never block process exit. */ + } +} + +/** Capture AND flush before returning -- the crash-path convenience wrapper for + * installCliSignalHandlers' `captureError` hook (process-lifecycle.js). A bare `captureMinerError()` only + * QUEUES the event in Sentry's transport; `process.exit()` tears the process down immediately afterward + * without waiting for any pending HTTP delivery, so the crash-capture path needs this awaited flush or it is + * very likely a near-total no-op in practice. */ +export async function captureMinerErrorAndFlush(error: unknown, context?: Record): Promise { + captureMinerError(error, context); + await flushMinerSentry(); +} + +/** Test-only: reset module state so one test's activation can't leak into the next. */ +export function resetMinerSentryForTesting(): void { + Sentry = undefined; + active = false; +} diff --git a/test/unit/miner-attempt-worktree.test.ts b/test/unit/miner-attempt-worktree.test.ts index 9b0b68d7a8..a36ac2b049 100644 --- a/test/unit/miner-attempt-worktree.test.ts +++ b/test/unit/miner-attempt-worktree.test.ts @@ -117,6 +117,27 @@ describe("prepareAttemptWorktree / cleanupAttemptWorktree (#5132)", () => { expect(execSpy).not.toHaveBeenCalled(); }); + it("threads a custom env and an injected runGit through to ensureRepoCloned when cloneBaseDir/remoteUrl are omitted", async () => { + // No cloneBaseDir/remoteUrl override here -- exercises the option's own default-resolution path (env -> + // LOOPOVER_MINER_REPO_CLONE_DIR, remoteUrl -> the default https://github.com/.git), with a fully + // mocked runGit/exec so nothing ever touches the network or a real git binary. + const root = tempRoot("loopover-miner-attempt-worktree-envrungit-"); + const runGit = vi.fn(async (_args: string[], _cwd: string, _timeoutMs: number) => ({ ok: true, stdout: "", stderr: "" })); + const exec = vi.fn(async () => ({ code: 0, stdout: "", stderr: "" })); + + const result = await prepareAttemptWorktree("acme/widgets", "attempt-env", { + env: { LOOPOVER_MINER_REPO_CLONE_DIR: root }, + runGit, + exec, + }); + + expect(result.ok).toBe(true); + expect(runGit).toHaveBeenCalled(); + const [firstArgs, firstCwd] = runGit.mock.calls[0] ?? []; + expect(firstArgs).toEqual(["clone", "https://github.com/acme/widgets.git", join(root, "acme", "widgets")]); + expect(firstCwd).toBe(root); + }); + it("returns ok:false with git's real stderr when git worktree add fails (e.g. an unknown base branch)", async () => { // Real git subprocess round trip (origin init + a real clone + a failing `git worktree add`). See the // REGRESSION test above for why this needs an explicit timeout. diff --git a/test/unit/miner-governor-chokepoint-persisted.test.ts b/test/unit/miner-governor-chokepoint-persisted.test.ts index 9b3cde30ae..808ca89bd2 100644 --- a/test/unit/miner-governor-chokepoint-persisted.test.ts +++ b/test/unit/miner-governor-chokepoint-persisted.test.ts @@ -8,7 +8,7 @@ vi.mock("@loopover/engine", async () => { }); import { evaluateGovernorChokepointGatePersisted } from "../../packages/loopover-miner/lib/governor-chokepoint-persisted.js"; -import { initGovernorLedger } from "../../packages/loopover-miner/lib/governor-ledger.js"; +import { closeDefaultGovernorLedger, initGovernorLedger, readGovernorEvents } from "../../packages/loopover-miner/lib/governor-ledger.js"; import { openGovernorState } from "../../packages/loopover-miner/lib/governor-state.js"; const roots: string[] = []; @@ -177,6 +177,23 @@ describe("evaluateGovernorChokepointGatePersisted (#5134)", () => { expect(reopened.loadRateLimitState().buckets.global.open_pr?.count).toBe(1); }); + it("REGRESSION: uses the REAL default appendGovernorEvent (not just an injected override) when options.append is omitted", () => { + const { root } = tempStore(); + process.env.LOOPOVER_MINER_GOVERNOR_STATE_DB = join(root, "governor-state.sqlite3"); + process.env.LOOPOVER_MINER_GOVERNOR_LEDGER_DB = join(root, "governor-ledger-default.sqlite3"); + try { + const result = evaluateGovernorChokepointGatePersisted(baseInput()); + expect(result.decision.allowed).toBe(true); + // The event actually landed in the REAL default ledger (module-singleton appendGovernorEvent), not just + // some caller-supplied stub -- proves the `options.append === undefined` branch truly ran the default. + expect(readGovernorEvents({ repoFullName: "acme/widgets" })).toHaveLength(1); + } finally { + closeDefaultGovernorLedger(); + delete process.env.LOOPOVER_MINER_GOVERNOR_STATE_DB; + delete process.env.LOOPOVER_MINER_GOVERNOR_LEDGER_DB; + } + }); + it("still saves the mutated rate-limit state even when the gate denies (a denial still consumes a backoff attempt)", () => { const { governorState, ledger } = tempStore(); const policies = { diff --git a/test/unit/miner-portfolio-discovery.test.ts b/test/unit/miner-portfolio-discovery.test.ts index 9bf96f75c9..c49406b243 100644 --- a/test/unit/miner-portfolio-discovery.test.ts +++ b/test/unit/miner-portfolio-discovery.test.ts @@ -106,20 +106,39 @@ describe("loopover-miner portfolio discovery (#2292)", () => { const summary = enqueueRankedDiscovery( [ rankedIssue({ issueNumber: 1, rankScore: 30 }), + null, + "not-an-object", { repoFullName: "bad", issueNumber: 2, title: "x", rankScore: 40 }, + { repoFullName: 123, issueNumber: 5, title: "x", rankScore: 10 }, rankedIssue({ issueNumber: 3, title: "", rankScore: 50 }), + rankedIssue({ issueNumber: 6, title: 123, rankScore: 10 } as unknown as Partial), + rankedIssue({ issueNumber: 4, rankScore: -5 }), + rankedIssue({ issueNumber: 7, rankScore: "not-a-number" } as unknown as Partial), + rankedIssue({ issueNumber: 8, rankScore: Number.POSITIVE_INFINITY }), + rankedIssue({ issueNumber: 1.5, rankScore: 10 }), + rankedIssue({ issueNumber: -1, rankScore: 10 }), ] as EnqueueRankedDiscoveryInput[], { queueStore }, ); expect(summary).toEqual({ enqueued: 1, skippedBelowMinRank: 0, - skippedInvalid: 2, + skippedInvalid: 11, eventsAppended: 0, }); expect(queueStore.listQueue()[0]?.identifier).toBe("issue:1"); }); + it("defaults a row's labels to [] when the field is absent or not an array", () => { + const queueStore = tempQueueStore(); + const summary = enqueueRankedDiscovery( + [{ repoFullName: "acme/widgets", issueNumber: 9, title: "No labels field", rankScore: 5 } as EnqueueRankedDiscoveryInput], + { queueStore }, + ); + expect(summary.enqueued).toBe(1); + expect(queueStore.listQueue()[0]?.identifier).toBe("issue:9"); + }); + it("refreshes priority for done items but leaves in_progress rows unchanged", () => { const queueStore = tempQueueStore(); enqueueRankedDiscovery([rankedIssue({ issueNumber: 7, rankScore: 10 })], { queueStore }); diff --git a/test/unit/miner-rejection-state-machine.test.ts b/test/unit/miner-rejection-state-machine.test.ts index 25cd410a4f..bcc2f8a03e 100644 --- a/test/unit/miner-rejection-state-machine.test.ts +++ b/test/unit/miner-rejection-state-machine.test.ts @@ -30,6 +30,12 @@ describe("loopover-miner rejection state machine (#4278)", () => { }); }); + it("carries a real merged_at string through when the PR was actually merged", () => { + expect( + extractPrOutcomeFields({ state: "closed", merged: true, merged_at: "2026-07-10T00:00:00Z", closed_at: null }), + ).toEqual({ state: "closed", merged: true, mergedAt: "2026-07-10T00:00:00Z", closedAt: null }); + }); + it("detects closed-without-merge as a rejection, but not a merged or open PR", () => { expect(isRejectedPr({ state: "closed", merged: false })).toBe(true); expect(isRejectedPr({ state: "closed", merged: true })).toBe(false); // merged PRs are also state:closed @@ -48,6 +54,12 @@ describe("loopover-miner rejection state machine (#4278)", () => { expect(classifyRejectionReason({ gateClosed: true, supersededByDuplicate: true })).toBe("gate_close"); }); + it("treats a non-object signal the same as an absent one, defaulting to maintainer_close_no_reason", () => { + // A caller outside TS's guarantees (plain JS, or a malformed payload) can hand this a non-object; the + // runtime guard must fall back to {} rather than throw on a null/primitive .gateClosed read. + expect(classifyRejectionReason(null as unknown as undefined)).toBe("maintainer_close_no_reason"); + }); + it("resolveRejection drives the renderer and returns the disengaged transition for each reason", () => { for (const [signal, reason] of [ [{ gateClosed: true }, "gate_close"],