diff --git a/packages/loopover-miner/lib/attempt-worktree.d.ts b/packages/loopover-miner/lib/attempt-worktree.d.ts index 645c519db8..b3b86bd180 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; - 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; +}; +/** + * 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; +/** + * 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..547755df72 100644 --- a/packages/loopover-miner/lib/attempt-worktree.js +++ b/packages/loopover-miner/lib/attempt-worktree.js @@ -1,94 +1,65 @@ 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 }; + const cloneResult = await ensureRepoCloned(repoFullName, options); + 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 }; } - /** * 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) }); +export function cleanupAttemptWorktree(repoPath, worktreePath, attemptOk, options = {}) { + const exec = options.exec ?? createRealWorktreeExec(options.timeoutMs); + return removeWorktree({ exec, repoPath, worktreePath, retain: shouldRetainWorktree(attemptOk) }); } +//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiYXR0ZW1wdC13b3JrdHJlZS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbImF0dGVtcHQtd29ya3RyZWUudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsT0FBTyxFQUFFLEtBQUssRUFBRSxNQUFNLG9CQUFvQixDQUFDO0FBQzNDLE9BQU8sRUFBRSxXQUFXLEVBQUUsY0FBYyxFQUFFLG9CQUFvQixFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUFFckYsT0FBTyxFQUFFLGdCQUFnQixFQUFFLE1BQU0saUJBQWlCLENBQUM7QUFHbkQsMkdBQTJHO0FBQzNHLGdHQUFnRztBQUNoRyxxR0FBcUc7QUFDckcsMEdBQTBHO0FBQzFHLCtEQUErRDtBQUUvRCxNQUFNLGtCQUFrQixHQUFHLE9BQU8sQ0FBQztBQWdCbkM7Ozs7O0dBS0c7QUFDSCxNQUFNLFVBQVUsc0JBQXNCLENBQUMsU0FBUyxHQUFHLGtCQUFrQjtJQUNuRSxPQUFPLENBQUMsR0FBRyxFQUFFLElBQUksRUFBRSxJQUFJLEVBQUUsRUFBRSxDQUN6QixJQUFJLE9BQU8sQ0FBcUIsQ0FBQyxPQUFPLEVBQUUsRUFBRTtRQUMxQyxNQUFNLEtBQUssR0FBRyxLQUFLLENBQUMsR0FBRyxFQUFFLENBQUMsR0FBRyxJQUFJLENBQUMsRUFBRSxFQUFFLEdBQUcsRUFBRSxJQUFJLENBQUMsR0FBRyxFQUFFLEtBQUssRUFBRSxDQUFDLFFBQVEsRUFBRSxNQUFNLEVBQUUsTUFBTSxDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQzFGLElBQUksTUFBTSxHQUFHLEVBQUUsQ0FBQztRQUNoQixJQUFJLE1BQU0sR0FBRyxFQUFFLENBQUM7UUFDaEIsTUFBTSxLQUFLLEdBQUcsVUFBVSxDQUFDLEdBQUcsRUFBRTtZQUM1QixLQUFLLENBQUMsSUFBSSxDQUFDLFNBQVMsQ0FBQyxDQUFDO1lBQ3RCLE9BQU8sQ0FBQyxFQUFFLElBQUksRUFBRSxJQUFJLEVBQUUsTUFBTSxFQUFFLE1BQU0sRUFBRSxHQUFHLE1BQU0scUJBQXFCLFNBQVMsSUFBSSxDQUFDLElBQUksRUFBRSxFQUFFLENBQUMsQ0FBQztRQUM5RixDQUFDLEVBQUUsU0FBUyxDQUFDLENBQUM7UUFDZCxLQUFLLENBQUMsTUFBTSxFQUFFLEVBQUUsQ0FBQyxNQUFNLEVBQUUsQ0FBQyxLQUFLLEVBQUUsRUFBRTtZQUNqQyxNQUFNLElBQUksS0FBSyxDQUFDLFFBQVEsQ0FBQyxNQUFNLENBQUMsQ0FBQztRQUNuQyxDQUFDLENBQUMsQ0FBQztRQUNILEtBQUssQ0FBQyxNQUFNLEVBQUUsRUFBRSxDQUFDLE1BQU0sRUFBRSxDQUFDLEtBQUssRUFBRSxFQUFFO1lBQ2pDLE1BQU0sSUFBSSxLQUFLLENBQUMsUUFBUSxDQUFDLE1BQU0sQ0FBQyxDQUFDO1FBQ25DLENBQUMsQ0FBQyxDQUFDO1FBQ0gsS0FBSyxDQUFDLEVBQUUsQ0FBQyxPQUFPLEVBQUUsQ0FBQyxHQUFHLEVBQUUsRUFBRTtZQUN4QixZQUFZLENBQUMsS0FBSyxDQUFDLENBQUM7WUFDcEIsT0FBTyxDQUFDLEVBQUUsSUFBSSxFQUFFLElBQUksRUFBRSxNQUFNLEVBQUUsTUFBTSxFQUFFLEdBQUcsQ0FBQyxPQUFPLEVBQUUsQ0FBQyxDQUFDO1FBQ3ZELENBQUMsQ0FBQyxDQUFDO1FBQ0gsS0FBSyxDQUFDLEVBQUUsQ0FBQyxPQUFPLEVBQUUsQ0FBQyxJQUFJLEVBQUUsRUFBRTtZQUN6QixZQUFZLENBQUMsS0FBSyxDQUFDLENBQUM7WUFDcEIsT0FBTyxDQUFDLEVBQUUsSUFBSSxFQUFFLE1BQU0sRUFBRSxNQUFNLEVBQUUsQ0FBQyxDQUFDO1FBQ3BDLENBQUMsQ0FBQyxDQUFDO0lBQ0wsQ0FBQyxDQUFDLENBQUM7QUFDUCxDQUFDO0FBRUQ7Ozs7R0FJRztBQUNILE1BQU0sQ0FBQyxLQUFLLFVBQVUsc0JBQXNCLENBQzFDLFlBQW9CLEVBQ3BCLFNBQWlCLEVBQ2pCLFVBQXlDLEVBQUU7SUFFM0MsTUFBTSxXQUFXLEdBQUcsTUFBTSxnQkFBZ0IsQ0FBQyxZQUFZLEVBQUUsT0FBTyxDQUFDLENBQUM7SUFDbEUsSUFBSSxDQUFDLFdBQVcsQ0FBQyxFQUFFO1FBQUUsT0FBTyxFQUFFLEVBQUUsRUFBRSxLQUFLLEVBQUUsS0FBSyxFQUFFLFdBQVcsQ0FBQyxLQUFLLElBQUksMkJBQTJCLEVBQUUsQ0FBQztJQUVuRyxNQUFNLElBQUksR0FBRyxPQUFPLENBQUMsSUFBSSxJQUFJLHNCQUFzQixDQUFDLE9BQU8sQ0FBQyxTQUFTLENBQUMsQ0FBQztJQUN2RSxNQUFNLFVBQVUsR0FBRyxPQUFPLE9BQU8sQ0FBQyxVQUFVLEtBQUssUUFBUSxJQUFJLE9BQU8sQ0FBQyxVQUFVLENBQUMsSUFBSSxFQUFFLENBQUMsQ0FBQyxDQUFDLE9BQU8sQ0FBQyxVQUFVLENBQUMsSUFBSSxFQUFFLENBQUMsQ0FBQyxDQUFDLE1BQU0sQ0FBQztJQUM1SCxNQUFNLEtBQUssR0FBRyxNQUFNLFdBQVcsQ0FBQyxFQUFFLElBQUksRUFBRSxRQUFRLEVBQUUsV0FBVyxDQUFDLFFBQVEsRUFBRSxVQUFVLEVBQUUsU0FBUyxFQUFFLENBQUMsQ0FBQztJQUNqRyxJQUFJLENBQUMsS0FBSyxDQUFDLEVBQUU7UUFBRSxPQUFPLEVBQUUsRUFBRSxFQUFFLEtBQUssRUFBRSxRQUFRLEVBQUUsV0FBVyxDQUFDLFFBQVEsRUFBRSxLQUFLLEVBQUUsS0FBSyxDQUFDLEtBQUssSUFBSSx5QkFBeUIsRUFBRSxDQUFDO0lBRXJILE9BQU8sRUFBRSxFQUFFLEVBQUUsSUFBSSxFQUFFLFlBQVksRUFBRSxLQUFLLENBQUMsSUFBSSxDQUFDLFlBQVksRUFBRSxVQUFVLEVBQUUsS0FBSyxDQUFDLElBQUksQ0FBQyxVQUFVLEVBQUUsUUFBUSxFQUFFLFdBQVcsQ0FBQyxRQUFRLEVBQUUsQ0FBQztBQUNoSSxDQUFDO0FBRUQ7OztHQUdHO0FBQ0gsTUFBTSxVQUFVLHNCQUFzQixDQUNwQyxRQUFnQixFQUNoQixZQUFvQixFQUNwQixTQUFrQixFQUNsQixVQUF5RCxFQUFFO0lBRTNELE1BQU0sSUFBSSxHQUFHLE9BQU8sQ0FBQyxJQUFJLElBQUksc0JBQXNCLENBQUMsT0FBTyxDQUFDLFNBQVMsQ0FBQyxDQUFDO0lBQ3ZFLE9BQU8sY0FBYyxDQUFDLEVBQUUsSUFBSSxFQUFFLFFBQVEsRUFBRSxZQUFZLEVBQUUsTUFBTSxFQUFFLG9CQUFvQixDQUFDLFNBQVMsQ0FBQyxFQUFFLENBQUMsQ0FBQztBQUNuRyxDQUFDIn0= \ 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..cb58dd5b30 --- /dev/null +++ b/packages/loopover-miner/lib/attempt-worktree.ts @@ -0,0 +1,95 @@ +import { spawn } from "node:child_process"; +import { addWorktree, removeWorktree, shouldRetainWorktree } from "@loopover/engine"; +import type { WorktreeExecFn, WorktreeExecResult } 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; + +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 }; + +/** + * 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 }); + }); + }); +} + +/** + * 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 { + const cloneResult = await ensureRepoCloned(repoFullName, options); + 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 }; +} + +/** + * 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 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..7ff3744828 100644 --- a/packages/loopover-miner/lib/claim-adjudication.d.ts +++ b/packages/loopover-miner/lib/claim-adjudication.d.ts @@ -1,21 +1,31 @@ /** 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; + number: number; + linkedIssueClaimedAt: string | null; }; - /** 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..aecddeafa9 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiY2xhaW0tYWRqdWRpY2F0aW9uLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiY2xhaW0tYWRqdWRpY2F0aW9uLnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLG9IQUFvSDtBQUNwSCx1R0FBdUc7QUFDdkcsb0hBQW9IO0FBQ3BILEVBQUU7QUFDRixnSEFBZ0g7QUFDaEgsa0hBQWtIO0FBQ2xILDhHQUE4RztBQUM5RywyREFBMkQ7QUFDM0QsT0FBTyxFQUFFLCtCQUErQixFQUFFLG1DQUFtQyxFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUFvQnhHOzs7OztHQUtHO0FBQ0gsTUFBTSxVQUFVLGFBQWEsQ0FBQyxLQUFvQjtJQUNoRCxPQUFPLEVBQUUsTUFBTSxFQUFFLEtBQUssQ0FBQyxNQUFNLEVBQUUsb0JBQW9CLEVBQUUsS0FBSyxDQUFDLFNBQVMsSUFBSSxJQUFJLEVBQUUsQ0FBQztBQUNqRixDQUFDO0FBRUQ7Ozs7Ozs7R0FPRztBQUNILE1BQU0sVUFBVSxtQkFBbUIsQ0FBQyxJQUFtQixFQUFFLFlBQXNDLEVBQUU7SUFDL0YsTUFBTSxVQUFVLEdBQUcsYUFBYSxDQUFDLElBQUksQ0FBQyxDQUFDO0lBQ3ZDLE1BQU0sUUFBUSxHQUFHLFNBQVMsQ0FBQyxHQUFHLENBQUMsYUFBYSxDQUFDLENBQUM7SUFDOUMsT0FBTztRQUNMLFFBQVEsRUFBRSwrQkFBK0IsQ0FBQyxVQUFVLEVBQUUsUUFBUSxDQUFDO1FBQy9ELFlBQVksRUFBRSxtQ0FBbUMsQ0FBQyxVQUFVLEVBQUUsUUFBUSxDQUFDO0tBQ3hFLENBQUM7QUFDSixDQUFDIn0= \ 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..cabaa74b8b --- /dev/null +++ b/packages/loopover-miner/lib/claim-adjudication.ts @@ -0,0 +1,54 @@ +// 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"; + +/** 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 = { + number: number; + linkedIssueClaimedAt: string | null; +}; + +/** 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..b4a5d50bb8 100644 --- a/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts +++ b/packages/loopover-miner/lib/governor-chokepoint-persisted.d.ts @@ -1,18 +1,10 @@ import type { GovernorChokepointInput } from "@loopover/engine"; -import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; import type { EvaluateGovernorChokepointGateResult } from "./governor-chokepoint.js"; +import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.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?: { +export type GovernorChokepointInputPersisted = Omit & Partial>; +export type EvaluateGovernorChokepointGatePersistedOptions = { governorState?: GovernorState; append?: (event: AppendGovernorEventInput) => GovernorLedgerEntry; - }, -): EvaluateGovernorChokepointGateResult; +}; +export declare function evaluateGovernorChokepointGatePersisted(input: GovernorChokepointInputPersisted, options?: EvaluateGovernorChokepointGatePersistedOptions): EvaluateGovernorChokepointGateResult; diff --git a/packages/loopover-miner/lib/governor-chokepoint-persisted.js b/packages/loopover-miner/lib/governor-chokepoint-persisted.js index d7443575b5..73e67330f4 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZ292ZXJub3ItY2hva2Vwb2ludC1wZXJzaXN0ZWQuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJnb3Zlcm5vci1jaG9rZXBvaW50LXBlcnNpc3RlZC50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFDQSxPQUFPLEVBQUUsOEJBQThCLEVBQUUsTUFBTSwwQkFBMEIsQ0FBQztBQUcxRSxPQUFPLEVBQUUsaUJBQWlCLEVBQUUsTUFBTSxxQkFBcUIsQ0FBQztBQTZCeEQsTUFBTSxVQUFVLHVDQUF1QyxDQUNyRCxLQUF1QyxFQUN2QyxVQUEwRCxFQUFFO0lBRTVELE1BQU0saUJBQWlCLEdBQUcsT0FBTyxDQUFDLGFBQWEsS0FBSyxTQUFTLENBQUM7SUFDOUQsTUFBTSxhQUFhLEdBQUcsT0FBTyxDQUFDLGFBQWEsSUFBSSxpQkFBaUIsRUFBRSxDQUFDO0lBQ25FLElBQUksQ0FBQztRQUNILE1BQU0sa0JBQWtCLEdBQUcsYUFBYSxDQUFDLGtCQUFrQixFQUFFLENBQUM7UUFDOUQsTUFBTSxpQkFBaUIsR0FBRyxhQUFhLENBQUMsWUFBWSxFQUFFLENBQUM7UUFDdkQsTUFBTSxhQUFhLEdBQTRCO1lBQzdDLEdBQUcsS0FBSztZQUNSLGdCQUFnQixFQUFFLEtBQUssQ0FBQyxnQkFBZ0IsSUFBSSxrQkFBa0IsQ0FBQyxPQUFPO1lBQ3RFLHdCQUF3QixFQUFFLEtBQUssQ0FBQyx3QkFBd0IsSUFBSSxrQkFBa0IsQ0FBQyxlQUFlO1lBQzlGLFFBQVEsRUFBRSxLQUFLLENBQUMsUUFBUSxJQUFJLGlCQUFpQjtTQUM5QyxDQUFDO1FBQ0YsTUFBTSxXQUFXLEdBQUcsT0FBTyxDQUFDLE1BQU0sS0FBSyxTQUFTLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxNQUFNLEVBQUUsT0FBTyxDQUFDLE1BQU0sRUFBRSxDQUFDO1FBQ25GLE1BQU0sTUFBTSxHQUFHLDhCQUE4QixDQUFDLGFBQWEsRUFBRSxXQUFXLENBQUMsQ0FBQztRQUMxRSxhQUFhLENBQUMsa0JBQWtCLENBQUMsRUFBRSxPQUFPLEVBQUUsTUFBTSxDQUFDLGdCQUFnQixFQUFFLGVBQWUsRUFBRSxNQUFNLENBQUMsd0JBQXdCLEVBQUUsQ0FBQyxDQUFDO1FBQ3pILE9BQU8sTUFBTSxDQUFDO0lBQ2hCLENBQUM7WUFBUyxDQUFDO1FBQ1QsSUFBSSxpQkFBaUI7WUFBRSxhQUFhLENBQUMsS0FBSyxFQUFFLENBQUM7SUFDL0MsQ0FBQztBQUNILENBQUMifQ== \ 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..420aed2ec3 --- /dev/null +++ b/packages/loopover-miner/lib/governor-chokepoint-persisted.ts @@ -0,0 +1,56 @@ +import type { GovernorChokepointInput } from "@loopover/engine"; +import { evaluateGovernorChokepointGate } from "./governor-chokepoint.js"; +import type { EvaluateGovernorChokepointGateResult } from "./governor-chokepoint.js"; +import type { AppendGovernorEventInput, GovernorLedgerEntry } from "./governor-ledger.js"; +import { openGovernorState } from "./governor-state.js"; +import type { GovernorState } 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. + +// 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 type EvaluateGovernorChokepointGatePersistedOptions = { + governorState?: GovernorState; + append?: (event: AppendGovernorEventInput) => GovernorLedgerEntry; +}; + +export function evaluateGovernorChokepointGatePersisted( + input: GovernorChokepointInputPersisted, + options: EvaluateGovernorChokepointGatePersistedOptions = {}, +): 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..200f940913 100644 --- a/packages/loopover-miner/lib/idea-feasibility.js +++ b/packages/loopover-miner/lib/idea-feasibility.js @@ -20,53 +20,42 @@ * 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" }; - /** * 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaWRlYS1mZWFzaWJpbGl0eS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbImlkZWEtZmVhc2liaWxpdHkudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUE7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7O0dBb0JHO0FBQ0gsT0FBTyxFQUFFLHVCQUF1QixFQUFFLE1BQU0sa0JBQWtCLENBQUM7QUEwQzNELDhGQUE4RjtBQUM5RixNQUFNLHNCQUFzQixHQUEyRCxFQUFFLEVBQUUsRUFBRSxTQUFTLEVBQUUsS0FBSyxFQUFFLE1BQU0sRUFBRSxLQUFLLEVBQUUsUUFBUSxFQUFFLENBQUM7QUFFekk7OztHQUdHO0FBQ0gsTUFBTSxVQUFVLHFCQUFxQixDQUNuQyxJQUEwQixFQUMxQixRQUF1RDtJQUV2RCxvRkFBb0Y7SUFDcEYsSUFBSSxDQUFDLFFBQVEsQ0FBQyxnQkFBZ0I7UUFBRSxPQUFPLFNBQVMsQ0FBQztJQUNqRCwrR0FBK0c7SUFDL0csOEdBQThHO0lBQzlHLDREQUE0RDtJQUM1RCxNQUFNLGdCQUFnQixHQUFHLENBQUMsSUFBSSxDQUFDLGVBQWUsSUFBSSxFQUFFLENBQUMsQ0FBQyxNQUFNLENBQzFELENBQUMsSUFBSSxFQUFFLEVBQUUsQ0FBQyxPQUFPLElBQUksS0FBSyxRQUFRLElBQUksSUFBSSxDQUFDLElBQUksRUFBRSxLQUFLLEVBQUUsQ0FDekQsQ0FBQyxNQUFNLENBQUM7SUFDVCxJQUFJLGdCQUFnQixLQUFLLENBQUM7UUFBRSxPQUFPLFNBQVMsQ0FBQztJQUM3QyxPQUFPLE9BQU8sQ0FBQztBQUNqQixDQUFDO0FBRUQ7O0dBRUc7QUFDSCxNQUFNLFVBQVUscUJBQXFCLENBQ25DLElBQTBCLEVBQzFCLFFBQTZCLEVBQzdCLFVBQXdDLEVBQUU7SUFFMUMsTUFBTSxZQUFZLEdBQUcsT0FBTyxDQUFDLHVCQUF1QixJQUFJLHVCQUF1QixDQUFDO0lBQ2hGLE1BQU0sV0FBVyxHQUFHLHFCQUFxQixDQUFDLElBQUksRUFBRSxRQUFRLENBQUMsQ0FBQztJQUMxRCxNQUFNLE9BQU8sR0FBRyxZQUFZLENBQUM7UUFDM0IsS0FBSyxFQUFFLFFBQVEsQ0FBQyxnQkFBZ0I7UUFDaEMsV0FBVyxFQUFFLFFBQVEsQ0FBQyxXQUFXO1FBQ2pDLG9CQUFvQixFQUFFLFFBQVEsQ0FBQyxvQkFBb0I7UUFDbkQsV0FBVztLQUNaLENBQUMsQ0FBQztJQUNILE9BQU87UUFDTCxXQUFXLEVBQUUsc0JBQXNCLENBQUMsT0FBTyxDQUFDLE9BQU8sQ0FBQztRQUNwRCxPQUFPLEVBQUUsT0FBTyxDQUFDLE9BQU87UUFDeEIsV0FBVztRQUNYLE9BQU8sRUFBRSxDQUFDLEdBQUcsT0FBTyxDQUFDLFlBQVksRUFBRSxHQUFHLE9BQU8sQ0FBQyxZQUFZLENBQUM7UUFDM0QsT0FBTyxFQUFFLE9BQU8sQ0FBQyxPQUFPO0tBQ3pCLENBQUM7QUFDSixDQUFDIn0= \ 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..0b24ba0619 --- /dev/null +++ b/packages/loopover-miner/lib/idea-feasibility.ts @@ -0,0 +1,110 @@ +/** 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..b7299c6263 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..63944c784c 100644 --- a/packages/loopover-miner/lib/portfolio-discovery.js +++ b/packages/loopover-miner/lib/portfolio-discovery.js @@ -1,100 +1,99 @@ /** 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 candidate = issue; + const repoFullName = typeof candidate.repoFullName === "string" ? candidate.repoFullName.trim() : ""; + const [owner, repo, extra] = repoFullName.split("/"); + if (!owner || !repo || extra !== undefined) + return null; + if (!Number.isInteger(candidate.issueNumber) || candidate.issueNumber <= 0) + return null; + if (typeof candidate.rankScore !== "number" || !Number.isFinite(candidate.rankScore) || candidate.rankScore < 0) { + return null; + } + const title = typeof candidate.title === "string" ? candidate.title.trim() : ""; + if (!title) + return null; + const labels = Array.isArray(candidate.labels) + ? candidate.labels.filter((label) => typeof label === "string" && label.trim() !== "").map((label) => label.trim()) + : []; + return { + repoFullName: `${owner}/${repo}`, + issueNumber: candidate.issueNumber, + title, + labels, + rankScore: candidate.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; + 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; + } + 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicG9ydGZvbGlvLWRpc2NvdmVyeS5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbInBvcnRmb2xpby1kaXNjb3ZlcnkudHMiXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsNkZBQTZGO0FBMkI3RixTQUFTLHFCQUFxQixDQUFDLFlBQXVDO0lBQ3BFLElBQUksWUFBWSxLQUFLLFNBQVMsSUFBSSxZQUFZLEtBQUssSUFBSTtRQUFFLE9BQU8sQ0FBQyxDQUFDO0lBQ2xFLElBQUksT0FBTyxZQUFZLEtBQUssUUFBUSxJQUFJLENBQUMsTUFBTSxDQUFDLFFBQVEsQ0FBQyxZQUFZLENBQUMsSUFBSSxZQUFZLEdBQUcsQ0FBQyxFQUFFLENBQUM7UUFDM0YsTUFBTSxJQUFJLEtBQUssQ0FBQyx3QkFBd0IsQ0FBQyxDQUFDO0lBQzVDLENBQUM7SUFDRCxPQUFPLFlBQVksQ0FBQztBQUN0QixDQUFDO0FBRUQsU0FBUyxvQkFBb0IsQ0FBQyxLQUFjO0lBQzFDLElBQUksQ0FBQyxLQUFLLElBQUksT0FBTyxLQUFLLEtBQUssUUFBUTtRQUFFLE9BQU8sSUFBSSxDQUFDO0lBQ3JELE1BQU0sU0FBUyxHQUFHLEtBQWdDLENBQUM7SUFDbkQsTUFBTSxZQUFZLEdBQUcsT0FBTyxTQUFTLENBQUMsWUFBWSxLQUFLLFFBQVEsQ0FBQyxDQUFDLENBQUMsU0FBUyxDQUFDLFlBQVksQ0FBQyxJQUFJLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDO0lBQ3JHLE1BQU0sQ0FBQyxLQUFLLEVBQUUsSUFBSSxFQUFFLEtBQUssQ0FBQyxHQUFHLFlBQVksQ0FBQyxLQUFLLENBQUMsR0FBRyxDQUFDLENBQUM7SUFDckQsSUFBSSxDQUFDLEtBQUssSUFBSSxDQUFDLElBQUksSUFBSSxLQUFLLEtBQUssU0FBUztRQUFFLE9BQU8sSUFBSSxDQUFDO0lBQ3hELElBQUksQ0FBQyxNQUFNLENBQUMsU0FBUyxDQUFDLFNBQVMsQ0FBQyxXQUFXLENBQUMsSUFBSyxTQUFTLENBQUMsV0FBc0IsSUFBSSxDQUFDO1FBQUUsT0FBTyxJQUFJLENBQUM7SUFDcEcsSUFBSSxPQUFPLFNBQVMsQ0FBQyxTQUFTLEtBQUssUUFBUSxJQUFJLENBQUMsTUFBTSxDQUFDLFFBQVEsQ0FBQyxTQUFTLENBQUMsU0FBUyxDQUFDLElBQUksU0FBUyxDQUFDLFNBQVMsR0FBRyxDQUFDLEVBQUUsQ0FBQztRQUNoSCxPQUFPLElBQUksQ0FBQztJQUNkLENBQUM7SUFDRCxNQUFNLEtBQUssR0FBRyxPQUFPLFNBQVMsQ0FBQyxLQUFLLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxTQUFTLENBQUMsS0FBSyxDQUFDLElBQUksRUFBRSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7SUFDaEYsSUFBSSxDQUFDLEtBQUs7UUFBRSxPQUFPLElBQUksQ0FBQztJQUN4QixNQUFNLE1BQU0sR0FBRyxLQUFLLENBQUMsT0FBTyxDQUFDLFNBQVMsQ0FBQyxNQUFNLENBQUM7UUFDNUMsQ0FBQyxDQUFDLFNBQVMsQ0FBQyxNQUFNLENBQUMsTUFBTSxDQUFDLENBQUMsS0FBSyxFQUFtQixFQUFFLENBQUMsT0FBTyxLQUFLLEtBQUssUUFBUSxJQUFJLEtBQUssQ0FBQyxJQUFJLEVBQUUsS0FBSyxFQUFFLENBQUMsQ0FBQyxHQUFHLENBQUMsQ0FBQyxLQUFLLEVBQUUsRUFBRSxDQUFDLEtBQUssQ0FBQyxJQUFJLEVBQUUsQ0FBQztRQUNwSSxDQUFDLENBQUMsRUFBRSxDQUFDO0lBQ1AsT0FBTztRQUNMLFlBQVksRUFBRSxHQUFHLEtBQUssSUFBSSxJQUFJLEVBQUU7UUFDaEMsV0FBVyxFQUFFLFNBQVMsQ0FBQyxXQUFxQjtRQUM1QyxLQUFLO1FBQ0wsTUFBTTtRQUNOLFNBQVMsRUFBRSxTQUFTLENBQUMsU0FBUztLQUMvQixDQUFDO0FBQ0osQ0FBQztBQUVEOzs7O0dBSUc7QUFDSCxNQUFNLFVBQVUsc0JBQXNCLENBQ3BDLFlBQW9ELEVBQ3BELFVBQXlDLEVBQW1DO0lBRTVFLElBQUksQ0FBQyxLQUFLLENBQUMsT0FBTyxDQUFDLFlBQVksQ0FBQztRQUFFLE1BQU0sSUFBSSxLQUFLLENBQUMsdUJBQXVCLENBQUMsQ0FBQztJQUMzRSxNQUFNLFVBQVUsR0FBRyxPQUFPLENBQUMsVUFBVSxDQUFDO0lBQ3RDLElBQUksQ0FBQyxVQUFVLElBQUksT0FBTyxVQUFVLENBQUMsT0FBTyxLQUFLLFVBQVU7UUFBRSxNQUFNLElBQUksS0FBSyxDQUFDLHFCQUFxQixDQUFDLENBQUM7SUFFcEcsSUFBSSxXQUFXLEdBQXVCLElBQUksQ0FBQztJQUMzQyxJQUFJLE9BQU8sQ0FBQyxXQUFXLEtBQUssU0FBUyxFQUFFLENBQUM7UUFDdEMsV0FBVyxHQUFHLE9BQU8sQ0FBQyxXQUFXLENBQUM7UUFDbEMsSUFBSSxDQUFDLFdBQVcsSUFBSSxPQUFPLFdBQVcsQ0FBQyxXQUFXLEtBQUssVUFBVSxFQUFFLENBQUM7WUFDbEUsTUFBTSxJQUFJLEtBQUssQ0FBQyxzQkFBc0IsQ0FBQyxDQUFDO1FBQzFDLENBQUM7SUFDSCxDQUFDO0lBRUQsTUFBTSxZQUFZLEdBQUcscUJBQXFCLENBQUMsT0FBTyxDQUFDLFlBQVksQ0FBQyxDQUFDO0lBQ2pFLDhHQUE4RztJQUM5RywyR0FBMkc7SUFDM0csdUdBQXVHO0lBQ3ZHLE1BQU0sVUFBVSxHQUFHLE9BQU8sQ0FBQyxVQUFVLENBQUM7SUFFdEMsTUFBTSxPQUFPLEdBQWtDO1FBQzdDLFFBQVEsRUFBRSxDQUFDO1FBQ1gsbUJBQW1CLEVBQUUsQ0FBQztRQUN0QixjQUFjLEVBQUUsQ0FBQztRQUNqQixjQUFjLEVBQUUsQ0FBQztLQUNsQixDQUFDO0lBRUYsS0FBSyxNQUFNLEtBQUssSUFBSSxZQUFZLEVBQUUsQ0FBQztRQUNqQyxNQUFNLFVBQVUsR0FBRyxvQkFBb0IsQ0FBQyxLQUFLLENBQUMsQ0FBQztRQUMvQyxJQUFJLENBQUMsVUFBVSxFQUFFLENBQUM7WUFDaEIsT0FBTyxDQUFDLGNBQWMsSUFBSSxDQUFDLENBQUM7WUFDNUIsU0FBUztRQUNYLENBQUM7UUFDRCxJQUFJLFVBQVUsQ0FBQyxTQUFTLEdBQUcsWUFBWSxFQUFFLENBQUM7WUFDeEMsT0FBTyxDQUFDLG1CQUFtQixJQUFJLENBQUMsQ0FBQztZQUNqQyxTQUFTO1FBQ1gsQ0FBQztRQUVELFVBQVUsQ0FBQyxPQUFPLENBQUM7WUFDakIsWUFBWSxFQUFFLFVBQVUsQ0FBQyxZQUFZO1lBQ3JDLFVBQVUsRUFBRSxTQUFTLFVBQVUsQ0FBQyxXQUFXLEVBQUU7WUFDN0MsUUFBUSxFQUFFLFVBQVUsQ0FBQyxTQUFTO1lBQzlCLEdBQUcsQ0FBQyxVQUFVLEtBQUssU0FBUyxDQUFDLENBQUMsQ0FBQyxFQUFFLFVBQVUsRUFBRSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7U0FDcEQsQ0FBQyxDQUFDO1FBQ0gsT0FBTyxDQUFDLFFBQVEsSUFBSSxDQUFDLENBQUM7UUFFdEIsSUFBSSxXQUFXLEVBQUUsQ0FBQztZQUNoQixXQUFXLENBQUMsV0FBVyxDQUFDO2dCQUN0QixJQUFJLEVBQUUsa0JBQWtCO2dCQUN4QixZQUFZLEVBQUUsVUFBVSxDQUFDLFlBQVk7Z0JBQ3JDLE9BQU8sRUFBRTtvQkFDUCxXQUFXLEVBQUUsVUFBVSxDQUFDLFdBQVc7b0JBQ25DLFNBQVMsRUFBRSxVQUFVLENBQUMsU0FBUztvQkFDL0IsS0FBSyxFQUFFLFVBQVUsQ0FBQyxLQUFLO29CQUN2QixNQUFNLEVBQUUsVUFBVSxDQUFDLE1BQU07aUJBQzFCO2FBQ0YsQ0FBQyxDQUFDO1lBQ0gsT0FBTyxDQUFDLGNBQWMsSUFBSSxDQUFDLENBQUM7UUFDOUIsQ0FBQztJQUNILENBQUM7SUFFRCxPQUFPLE9BQU8sQ0FBQztBQUNqQixDQUFDIn0= \ 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..6020380128 --- /dev/null +++ b/packages/loopover-miner/lib/portfolio-discovery.ts @@ -0,0 +1,129 @@ +/** 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; +}; + +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): EnqueueRankedDiscoveryInput | null { + if (!issue || typeof issue !== "object") return null; + const candidate = issue as Record; + const repoFullName = typeof candidate.repoFullName === "string" ? candidate.repoFullName.trim() : ""; + const [owner, repo, extra] = repoFullName.split("/"); + if (!owner || !repo || extra !== undefined) return null; + if (!Number.isInteger(candidate.issueNumber) || (candidate.issueNumber as number) <= 0) return null; + if (typeof candidate.rankScore !== "number" || !Number.isFinite(candidate.rankScore) || candidate.rankScore < 0) { + return null; + } + const title = typeof candidate.title === "string" ? candidate.title.trim() : ""; + if (!title) return null; + const labels = Array.isArray(candidate.labels) + ? candidate.labels.filter((label): label is string => typeof label === "string" && label.trim() !== "").map((label) => label.trim()) + : []; + return { + repoFullName: `${owner}/${repo}`, + issueNumber: candidate.issueNumber as number, + title, + labels, + rankScore: candidate.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 = {} as 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; + } + + 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..18c72f4ec9 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicHJvY2Vzcy1saWZlY3ljbGUuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJwcm9jZXNzLWxpZmVjeWNsZS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQTs7Ozs7OzsrRkFPK0Y7QUF5Qi9GLGtIQUFrSDtBQUNsSCxzQkFBc0I7QUFDdEIsTUFBTSxpQkFBaUIsR0FBRyxNQUFNLENBQUMsTUFBTSxDQUFDLEVBQUUsTUFBTSxFQUFFLEdBQUcsRUFBRSxPQUFPLEVBQUUsR0FBRyxFQUFFLENBQUMsQ0FBQztBQUV2RTs0RUFDNEU7QUFDNUUsTUFBTSxnQkFBZ0IsR0FBRyxJQUFJLEdBQUcsRUFBbUIsQ0FBQztBQUNwRCxJQUFJLGlCQUFpQixHQUFHLEtBQUssQ0FBQztBQUU5Qix3RkFBd0Y7QUFDeEYsU0FBUyxhQUFhLENBQUMsS0FBYztJQUNuQyxJQUFJLEtBQUssWUFBWSxLQUFLO1FBQUUsT0FBTyxLQUFLLENBQUMsS0FBSyxJQUFJLEtBQUssQ0FBQyxPQUFPLENBQUM7SUFDaEUsT0FBTyxNQUFNLENBQUMsS0FBSyxDQUFDLENBQUM7QUFDdkIsQ0FBQztBQUVEOzs7R0FHRztBQUNILE1BQU0sVUFBVSx1QkFBdUIsQ0FBQyxRQUE0QztJQUNsRixJQUFJLFFBQVEsS0FBSyxJQUFJLElBQUksUUFBUSxLQUFLLFNBQVM7UUFBRSxPQUFPLEdBQUcsRUFBRSxHQUFFLENBQUMsQ0FBQztJQUNqRSxnQkFBZ0IsQ0FBQyxHQUFHLENBQUMsUUFBUSxDQUFDLENBQUM7SUFDL0IsT0FBTyxHQUFHLEVBQUU7UUFDVixnQkFBZ0IsQ0FBQyxNQUFNLENBQUMsUUFBUSxDQUFDLENBQUM7SUFDcEMsQ0FBQyxDQUFDO0FBQ0osQ0FBQztBQUVELDBGQUEwRjtBQUMxRixNQUFNLFVBQVUsb0JBQW9CO0lBQ2xDLE9BQU8sZ0JBQWdCLENBQUMsSUFBSSxDQUFDO0FBQy9CLENBQUM7QUFFRDs7O0dBR0c7QUFDSCxNQUFNLFVBQVUsd0JBQXdCLENBQUMsVUFBa0QsRUFBRTtJQUMzRixNQUFNLE9BQU8sR0FBRyxPQUFPLE9BQU8sQ0FBQyxPQUFPLEtBQUssVUFBVSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsT0FBTyxDQUFDLENBQUMsQ0FBQyxJQUFJLENBQUM7SUFDL0UsS0FBSyxNQUFNLFFBQVEsSUFBSSxDQUFDLEdBQUcsZ0JBQWdCLENBQUMsRUFBRSxDQUFDO1FBQzdDLElBQUksQ0FBQztZQUNILElBQUksT0FBTyxRQUFRLEtBQUssVUFBVTtnQkFBRSxRQUFRLEVBQUUsQ0FBQzs7Z0JBQzFDLFFBQVEsQ0FBQyxLQUFLLEVBQUUsQ0FBQztRQUN4QixDQUFDO1FBQUMsT0FBTyxLQUFLLEVBQUUsQ0FBQztZQUNmLElBQUksT0FBTztnQkFBRSxPQUFPLENBQUMsS0FBSyxDQUFDLENBQUM7UUFDOUIsQ0FBQztJQUNILENBQUM7SUFDRCxnQkFBZ0IsQ0FBQyxLQUFLLEVBQUUsQ0FBQztBQUMzQixDQUFDO0FBRUQ7Ozs7OztHQU1HO0FBQ0gsTUFBTSxVQUFVLHdCQUF3QixDQUFDLFVBQTJDLEVBQUU7SUFDcEYsTUFBTSxJQUFJLEdBQWdCLE9BQU8sQ0FBQyxPQUFPLElBQUksT0FBTyxDQUFDO0lBQ3JELE1BQU0sR0FBRyxHQUE4QixPQUFPLE9BQU8sQ0FBQyxHQUFHLEtBQUssVUFBVSxDQUFDLENBQUMsQ0FBQyxPQUFPLENBQUMsR0FBRyxDQUFDLENBQUMsQ0FBQyxDQUFDLE9BQU8sRUFBRSxFQUFFLENBQUMsT0FBTyxDQUFDLEtBQUssQ0FBQyxPQUFPLENBQUMsQ0FBQztJQUM3SCxNQUFNLElBQUksR0FBMkIsT0FBTyxPQUFPLENBQUMsSUFBSSxLQUFLLFVBQVUsQ0FBQyxDQUFDLENBQUMsT0FBTyxDQUFDLElBQUksQ0FBQyxDQUFDLENBQUMsQ0FBQyxJQUFJLEVBQUUsRUFBRSxDQUFDLElBQUksQ0FBQyxJQUFJLENBQUMsSUFBSSxDQUFDLENBQUM7SUFDbkgseUdBQXlHO0lBQ3pHLHVHQUF1RztJQUN2RyxtREFBbUQ7SUFDbkQsTUFBTSxZQUFZLEdBQ2hCLE9BQU8sT0FBTyxDQUFDLFlBQVksS0FBSyxVQUFVLENBQUMsQ0FBQyxDQUFDLE9BQU8sQ0FBQyxZQUFZLENBQUMsQ0FBQyxDQUFDLEdBQUcsRUFBRSxHQUFFLENBQUMsQ0FBQztJQUUvRSxJQUFJLGlCQUFpQixJQUFJLE9BQU8sQ0FBQyxLQUFLLEtBQUssSUFBSTtRQUFFLE9BQU8sS0FBSyxDQUFDO0lBQzlELGlCQUFpQixHQUFHLElBQUksQ0FBQztJQUV6QixNQUFNLFVBQVUsR0FBRyxHQUFHLEVBQUU7UUFDdEIsd0JBQXdCLENBQUM7WUFDdkIsT0FBTyxFQUFFLENBQUMsS0FBSyxFQUFFLEVBQUUsQ0FBQyxHQUFHLENBQUMsZ0RBQWdELGFBQWEsQ0FBQyxLQUFLLENBQUMsRUFBRSxDQUFDO1NBQ2hHLENBQUMsQ0FBQztJQUNMLENBQUMsQ0FBQztJQUVGLEtBQUssTUFBTSxDQUFDLE1BQU0sRUFBRSxJQUFJLENBQUMsSUFBSSxNQUFNLENBQUMsT0FBTyxDQUFDLGlCQUFpQixDQUFDLEVBQUUsQ0FBQztRQUMvRCxJQUFJLENBQUMsRUFBRSxDQUFDLE1BQU0sRUFBRSxHQUFHLEVBQUU7WUFDbkIsR0FBRyxDQUFDLDRCQUE0QixNQUFNLHVDQUF1QyxDQUFDLENBQUM7WUFDL0UsVUFBVSxFQUFFLENBQUM7WUFDYixJQUFJLENBQUMsSUFBSSxDQUFDLENBQUM7UUFDYixDQUFDLENBQUMsQ0FBQztJQUNMLENBQUM7SUFFRCwwR0FBMEc7SUFDMUcsMkdBQTJHO0lBQzNHLHdHQUF3RztJQUN4Ryw0R0FBNEc7SUFDNUcsMEdBQTBHO0lBQzFHLDRHQUE0RztJQUM1Ryw4REFBOEQ7SUFDOUQsSUFBSSxDQUFDLEVBQUUsQ0FBQyxtQkFBbUIsRUFBRSxLQUFLLEVBQUUsS0FBSyxFQUFFLEVBQUU7UUFDM0MsR0FBRyxDQUFDLHVDQUF1QyxhQUFhLENBQUMsS0FBSyxDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQ25FLE1BQU0sWUFBWSxDQUFDLEtBQUssRUFBRSxFQUFFLElBQUksRUFBRSxtQkFBbUIsRUFBRSxDQUFDLENBQUM7UUFDekQsVUFBVSxFQUFFLENBQUM7UUFDYixJQUFJLENBQUMsQ0FBQyxDQUFDLENBQUM7SUFDVixDQUFDLENBQUMsQ0FBQztJQUVILElBQUksQ0FBQyxFQUFFLENBQUMsb0JBQW9CLEVBQUUsS0FBSyxFQUFFLE1BQU0sRUFBRSxFQUFFO1FBQzdDLEdBQUcsQ0FBQyxnREFBZ0QsYUFBYSxDQUFDLE1BQU0sQ0FBQyxFQUFFLENBQUMsQ0FBQztRQUM3RSxNQUFNLFlBQVksQ0FBQyxNQUFNLEVBQUUsRUFBRSxJQUFJLEVBQUUsb0JBQW9CLEVBQUUsQ0FBQyxDQUFDO1FBQzNELFVBQVUsRUFBRSxDQUFDO1FBQ2IsSUFBSSxDQUFDLENBQUMsQ0FBQyxDQUFDO0lBQ1YsQ0FBQyxDQUFDLENBQUM7SUFFSCxPQUFPLElBQUksQ0FBQztBQUNkLENBQUM7QUFFRCx1R0FBdUc7QUFDdkcsTUFBTSxVQUFVLCtCQUErQjtJQUM3QyxnQkFBZ0IsQ0FBQyxLQUFLLEVBQUUsQ0FBQztJQUN6QixpQkFBaUIsR0FBRyxLQUFLLENBQUM7QUFDNUIsQ0FBQyJ9 \ 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..52919b0ffe --- /dev/null +++ b/packages/loopover-miner/lib/process-lifecycle.ts @@ -0,0 +1,144 @@ +/** 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 = 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: ProcessLike = options.process ?? process; + const log: (message: string) => void = typeof options.log === "function" ? options.log : (message) => console.error(message); + const exit: (code: number) => void = 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: (error: unknown, context?: Record) => void | Promise = + 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); + }); + + proc.on("unhandledRejection", async (reason) => { + 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..5e59fa1de8 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..0bee1eb603 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicmVqZWN0aW9uLXN0YXRlLW1hY2hpbmUuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJyZWplY3Rpb24tc3RhdGUtbWFjaGluZS50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSwwR0FBMEc7QUFDMUcsMEZBQTBGO0FBQzFGLDJHQUEyRztBQUMzRyw0R0FBNEc7QUFDNUcsRUFBRTtBQUNGLHFEQUFxRDtBQUNyRCw2R0FBNkc7QUFDN0csNEdBQTRHO0FBQzVHLDhHQUE4RztBQUM5Ryw0R0FBNEc7QUFDNUcsa0dBQWtHO0FBQ2xHLGtIQUFrSDtBQUNsSCxnSEFBZ0g7QUFDaEgsY0FBYztBQUNkLHVHQUF1RztBQUN2Ryw0R0FBNEc7QUFDNUcsc0ZBQXNGO0FBRXRGLE9BQU8sRUFBRSxzQkFBc0IsRUFBRSxNQUFNLDBCQUEwQixDQUFDO0FBc0JsRTtzREFDc0Q7QUFDdEQsTUFBTSxDQUFDLE1BQU0sa0JBQWtCLEdBQUcsWUFBcUIsQ0FBQztBQUV4RDs7O0dBR0c7QUFDSCxNQUFNLFVBQVUsc0JBQXNCLENBQUMsU0FBa0I7SUFDdkQsTUFBTSxDQUFDLEdBQUcsQ0FBQyxTQUFTLElBQUksT0FBTyxTQUFTLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxTQUFTLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBNEIsQ0FBQztJQUNuRyxPQUFPO1FBQ0wsS0FBSyxFQUFFLE9BQU8sQ0FBQyxDQUFDLEtBQUssS0FBSyxRQUFRLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxLQUFLLENBQUMsQ0FBQyxDQUFDLElBQUk7UUFDbkQsTUFBTSxFQUFFLENBQUMsQ0FBQyxNQUFNLEtBQUssSUFBSTtRQUN6QixRQUFRLEVBQUUsT0FBTyxDQUFDLENBQUMsU0FBUyxLQUFLLFFBQVEsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLFNBQVMsQ0FBQyxDQUFDLENBQUMsSUFBSTtRQUM5RCxRQUFRLEVBQUUsT0FBTyxDQUFDLENBQUMsU0FBUyxLQUFLLFFBQVEsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLFNBQVMsQ0FBQyxDQUFDLENBQUMsSUFBSTtLQUMvRCxDQUFDO0FBQ0osQ0FBQztBQUVEOzs7R0FHRztBQUNILE1BQU0sVUFBVSxZQUFZLENBQUMsTUFBc0U7SUFDakcsTUFBTSxDQUFDLEdBQUcsTUFBTSxJQUFJLE9BQU8sTUFBTSxLQUFLLFFBQVEsQ0FBQyxDQUFDLENBQUMsTUFBTSxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUM7SUFDN0QsT0FBTyxDQUFDLENBQUMsS0FBSyxLQUFLLFFBQVEsSUFBSSxDQUFDLENBQUMsTUFBTSxLQUFLLElBQUksQ0FBQztBQUNuRCxDQUFDO0FBRUQ7Ozs7O0dBS0c7QUFDSCxNQUFNLFVBQVUsdUJBQXVCLENBQUMsU0FBMEIsRUFBRTtJQUNsRSxNQUFNLENBQUMsR0FBRyxNQUFNLElBQUksT0FBTyxNQUFNLEtBQUssUUFBUSxDQUFDLENBQUMsQ0FBQyxNQUFNLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQztJQUM3RCxJQUFJLENBQUMsQ0FBQyxVQUFVLEtBQUssSUFBSTtRQUFFLE9BQU8sWUFBWSxDQUFDO0lBQy9DLElBQUksQ0FBQyxDQUFDLHFCQUFxQixLQUFLLElBQUk7UUFBRSxPQUFPLHlCQUF5QixDQUFDO0lBQ3ZFLE9BQU8sNEJBQTRCLENBQUM7QUFDdEMsQ0FBQztBQUVEOzs7Ozs7R0FNRztBQUNILE1BQU0sVUFBVSxnQkFBZ0IsQ0FDOUIsU0FBa0IsRUFDbEIsTUFBbUMsRUFDbkMsT0FBeUI7SUFFekIsTUFBTSxNQUFNLEdBQUcsc0JBQXNCLENBQUMsU0FBUyxDQUFDLENBQUM7SUFDakQsSUFBSSxDQUFDLFlBQVksQ0FBQyxNQUFNLENBQUM7UUFBRSxPQUFPLElBQUksQ0FBQztJQUN2QyxNQUFNLE1BQU0sR0FBRyx1QkFBdUIsQ0FBQyxNQUFNLENBQUMsQ0FBQztJQUMvQyxNQUFNLElBQUksR0FBRyxzQkFBc0IsQ0FBQyxNQUFNLEVBQUUsT0FBTyxDQUFDLENBQUMsQ0FBQyx3REFBd0Q7SUFDOUcsT0FBTyxFQUFFLE9BQU8sRUFBRSxrQkFBa0IsRUFBRSxNQUFNLEVBQUUsSUFBSSxFQUFFLE1BQU0sRUFBRSxDQUFDO0FBQy9ELENBQUMifQ== \ 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..b6debe3532 --- /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" as const; + +/** + * 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 : {}; + 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 : {}; + 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..2042f39e1e 100644 --- a/packages/loopover-miner/lib/sentry.js +++ b/packages/loopover-miner/lib/sentry.js @@ -6,60 +6,62 @@ * 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; + const sentry = Sentry; + 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,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2VudHJ5LmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsic2VudHJ5LnRzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBOzs7Ozs7O2lHQU9pRztBQU1qRyxJQUFJLE1BQTRCLENBQUM7QUFDakMsSUFBSSxNQUFNLEdBQUcsS0FBSyxDQUFDO0FBRW5COztpRkFFaUY7QUFDakYsTUFBTSxDQUFDLEtBQUssVUFBVSxlQUFlLENBQUMsTUFBMEMsT0FBTyxDQUFDLEdBQUc7SUFDekYsSUFBSSxDQUFDLEdBQUcsQ0FBQyx5QkFBeUI7UUFBRSxPQUFPLEtBQUssQ0FBQztJQUNqRCxNQUFNLEdBQUcsR0FBRyxNQUFNLE1BQU0sQ0FBQyxjQUFjLENBQUMsQ0FBQztJQUN6QyxNQUFNLEdBQUcsR0FBRyxDQUFDO0lBQ2IsTUFBTSxDQUFDLElBQUksQ0FBQztRQUNWLEdBQUcsRUFBRSxHQUFHLENBQUMseUJBQXlCO1FBQ2xDLFdBQVcsRUFBRSxHQUFHLENBQUMsaUNBQWlDLElBQUksWUFBWTtLQUNuRSxDQUFDLENBQUM7SUFDSCxNQUFNLEdBQUcsSUFBSSxDQUFDO0lBQ2QsT0FBTyxJQUFJLENBQUM7QUFDZCxDQUFDO0FBRUQsaUdBQWlHO0FBQ2pHLE1BQU0sVUFBVSxpQkFBaUIsQ0FBQyxLQUFjLEVBQUUsT0FBaUM7SUFDakYsSUFBSSxDQUFDLE1BQU0sSUFBSSxDQUFDLE1BQU07UUFBRSxPQUFPO0lBQy9CLE1BQU0sTUFBTSxHQUFHLE1BQU0sQ0FBQztJQUN0QixJQUFJLENBQUM7UUFDSCxNQUFNLENBQUMsU0FBUyxDQUFDLENBQUMsS0FBSyxFQUFFLEVBQUU7WUFDekIsSUFBSSxPQUFPO2dCQUFFLEtBQUssQ0FBQyxVQUFVLENBQUMsT0FBTyxFQUFFLE9BQU8sQ0FBQyxDQUFDO1lBQ2hELE1BQU0sQ0FBQyxnQkFBZ0IsQ0FBQyxLQUFLLFlBQVksS0FBSyxDQUFDLENBQUMsQ0FBQyxLQUFLLENBQUMsQ0FBQyxDQUFDLElBQUksS0FBSyxDQUFDLE1BQU0sQ0FBQyxLQUFLLENBQUMsQ0FBQyxDQUFDLENBQUM7UUFDckYsQ0FBQyxDQUFDLENBQUM7SUFDTCxDQUFDO0lBQUMsTUFBTSxDQUFDO1FBQ1Asb0VBQW9FO0lBQ3RFLENBQUM7QUFDSCxDQUFDO0FBRUQsOEdBQThHO0FBQzlHLE1BQU0sQ0FBQyxLQUFLLFVBQVUsZ0JBQWdCLENBQUMsU0FBUyxHQUFHLElBQUk7SUFDckQsSUFBSSxDQUFDLE1BQU0sSUFBSSxDQUFDLE1BQU07UUFBRSxPQUFPO0lBQy9CLElBQUksQ0FBQztRQUNILE1BQU0sTUFBTSxDQUFDLEtBQUssQ0FBQyxTQUFTLENBQUMsQ0FBQztJQUNoQyxDQUFDO0lBQUMsTUFBTSxDQUFDO1FBQ1AsbUVBQW1FO0lBQ3JFLENBQUM7QUFDSCxDQUFDO0FBRUQ7Ozs7aURBSWlEO0FBQ2pELE1BQU0sQ0FBQyxLQUFLLFVBQVUseUJBQXlCLENBQUMsS0FBYyxFQUFFLE9BQWlDO0lBQy9GLGlCQUFpQixDQUFDLEtBQUssRUFBRSxPQUFPLENBQUMsQ0FBQztJQUNsQyxNQUFNLGdCQUFnQixFQUFFLENBQUM7QUFDM0IsQ0FBQztBQUVELHVGQUF1RjtBQUN2RixNQUFNLFVBQVUsMEJBQTBCO0lBQ3hDLE1BQU0sR0FBRyxTQUFTLENBQUM7SUFDbkIsTUFBTSxHQUFHLEtBQUssQ0FBQztBQUNqQixDQUFDIn0= \ 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..d5a3e962b6 --- /dev/null +++ b/packages/loopover-miner/lib/sentry.ts @@ -0,0 +1,70 @@ +/** 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. */ + +// @sentry/node is NEVER imported at module top level -- it loads lazily inside initMinerSentry() (see the module +// comment above), so `typeof import(...)` gives us its types without pulling it into the module-load path. +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; + const sentry = Sentry; + 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-fallbacks.test.ts b/test/unit/miner-attempt-worktree-fallbacks.test.ts new file mode 100644 index 0000000000..bd58f04ab8 --- /dev/null +++ b/test/unit/miner-attempt-worktree-fallbacks.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it, vi } from "vitest"; + +// prepareAttemptWorktree fails closed with a generic marker when its own dependencies report failure WITHOUT a +// specific error string (the `?? "ensure_repo_cloned_failed"` / `?? "git_worktree_add_failed"` fallbacks). The +// real ensureRepoCloned/addWorktree always attach an error on failure, so these defensive fallbacks are only +// reachable by injecting an error-less failure -- hence the module mocks here, kept in their own file so the +// real-git integration coverage in miner-attempt-worktree.test.ts stays unmocked. +vi.mock("../../packages/loopover-miner/lib/repo-clone.js", () => ({ + ensureRepoCloned: vi.fn(), +})); +vi.mock("@loopover/engine", () => ({ + addWorktree: vi.fn(), + removeWorktree: vi.fn(), + shouldRetainWorktree: vi.fn(), +})); + +import { addWorktree } from "@loopover/engine"; +import { prepareAttemptWorktree } from "../../packages/loopover-miner/lib/attempt-worktree.js"; +import { ensureRepoCloned } from "../../packages/loopover-miner/lib/repo-clone.js"; + +describe("prepareAttemptWorktree defensive fallbacks (#5132)", () => { + it("falls back to ensure_repo_cloned_failed when the clone fails without a specific error", async () => { + vi.mocked(ensureRepoCloned).mockResolvedValue({ ok: false, repoPath: "" } as never); + + const result = await prepareAttemptWorktree("acme/widgets", "attempt-1", {}); + + expect(result).toEqual({ ok: false, error: "ensure_repo_cloned_failed" }); + expect(addWorktree).not.toHaveBeenCalled(); + }); + + it("falls back to git_worktree_add_failed when the worktree add fails without a specific error", async () => { + vi.mocked(ensureRepoCloned).mockResolvedValue({ ok: true, repoPath: "/tmp/repo" } as never); + vi.mocked(addWorktree).mockResolvedValue({ + ok: false, + plan: { attemptId: "attempt-1", worktreePath: "", branchName: "" }, + } as never); + + const result = await prepareAttemptWorktree("acme/widgets", "attempt-1", { exec: vi.fn() }); + + expect(result).toEqual({ ok: false, repoPath: "/tmp/repo", error: "git_worktree_add_failed" }); + }); +}); diff --git a/test/unit/miner-portfolio-discovery.test.ts b/test/unit/miner-portfolio-discovery.test.ts index 9bf96f75c9..798f6efd96 100644 --- a/test/unit/miner-portfolio-discovery.test.ts +++ b/test/unit/miner-portfolio-discovery.test.ts @@ -120,6 +120,46 @@ describe("loopover-miner portfolio discovery (#2292)", () => { expect(queueStore.listQueue()[0]?.identifier).toBe("issue:1"); }); + it("rejects every malformed-row shape and counts each as invalid", () => { + const queueStore = tempQueueStore(); + const summary = enqueueRankedDiscovery( + [ + null, // not an object + 42, // not an object + { repoFullName: 123, issueNumber: 1, title: "x", rankScore: 5 }, // non-string repoFullName -> "" -> no owner + { repoFullName: "acme/widgets/extra", issueNumber: 1, title: "x", rankScore: 5 }, // three path segments + { repoFullName: "acme/widgets", issueNumber: 1.5, title: "x", rankScore: 5 }, // non-integer issueNumber + { repoFullName: "acme/widgets", issueNumber: 0, title: "x", rankScore: 5 }, // non-positive issueNumber + { repoFullName: "acme/widgets", issueNumber: 1, title: "x", rankScore: "high" }, // non-number rankScore + { repoFullName: "acme/widgets", issueNumber: 1, title: "x", rankScore: -1 }, // negative rankScore + { repoFullName: "acme/widgets", issueNumber: 1, title: 42, rankScore: 5 }, // non-string title + ] as unknown as EnqueueRankedDiscoveryInput[], + { queueStore }, + ); + expect(summary).toEqual({ + enqueued: 0, + skippedBelowMinRank: 0, + skippedInvalid: 9, + eventsAppended: 0, + }); + expect(queueStore.listQueue()).toEqual([]); + }); + + it("normalizes a valid row's title and labels: trims, coerces non-array labels to [], and drops blank/non-string labels", () => { + const queueStore = tempQueueStore(); + const eventLedger = tempEventLedger(); + enqueueRankedDiscovery( + [ + { repoFullName: "acme/widgets", issueNumber: 5, title: " Trim me ", rankScore: 20, labels: "not-an-array" }, + { repoFullName: "acme/widgets", issueNumber: 6, title: "Second", rankScore: 20, labels: [" keep me ", "", 42, " "] }, + ] as unknown as EnqueueRankedDiscoveryInput[], + { queueStore, eventLedger }, + ); + const events = eventLedger.readEvents(); + expect(events[0]?.payload).toMatchObject({ issueNumber: 5, title: "Trim me", labels: [] }); + expect(events[1]?.payload).toMatchObject({ issueNumber: 6, title: "Second", labels: ["keep me"] }); + }); + 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..0eeb0027ec 100644 --- a/test/unit/miner-rejection-state-machine.test.ts +++ b/test/unit/miner-rejection-state-machine.test.ts @@ -30,6 +30,22 @@ describe("loopover-miner rejection state machine (#4278)", () => { }); }); + it("surfaces a string merged_at from a merged payload (the merged-timestamp branch)", () => { + expect( + extractPrOutcomeFields({ + state: "closed", + merged: true, + merged_at: "2026-07-09T18:00:00Z", + closed_at: "2026-07-09T18:00:00Z", + }), + ).toEqual({ + state: "closed", + merged: true, + mergedAt: "2026-07-09T18:00:00Z", + closedAt: "2026-07-09T18:00:00Z", + }); + }); + 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 @@ -42,6 +58,7 @@ describe("loopover-miner rejection state machine (#4278)", () => { expect(classifyRejectionReason({ supersededByDuplicate: true })).toBe("superseded_by_duplicate"); expect(classifyRejectionReason({})).toBe("maintainer_close_no_reason"); expect(classifyRejectionReason()).toBe("maintainer_close_no_reason"); // zero-signal fallback + expect(classifyRejectionReason(null as never)).toBe("maintainer_close_no_reason"); // non-object signal coerced }); it("prefers the gate cause when both gate and duplicate signals are present", () => {