diff --git a/packages/loopover-miner/lib/attempt-cli.ts b/packages/loopover-miner/lib/attempt-cli.ts index a896ec5bcc..d2fc788b57 100644 --- a/packages/loopover-miner/lib/attempt-cli.ts +++ b/packages/loopover-miner/lib/attempt-cli.ts @@ -20,7 +20,7 @@ import { resolveCodingAgentModeFromConfig, resolveFirstConfiguredCodingAgentDriverName, } from "@loopover/engine"; -import type { AttemptDbFork, AttemptDbForkConfig, CodingAgentExecutionMode, FeasibilityVerdict, LocalWriteActionSpec } from "@loopover/engine"; +import type { AttemptDbFork, AttemptDbForkConfig, CodingAgentExecutionMode, FeasibilityVerdict, LocalWriteActionSpec, SignalStore } from "@loopover/engine"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; import { resolveAttemptDbForkConfig } from "./attempt-db-fork-config.js"; import { constructProductionCodingAgentDriver } from "./coding-agent-construction.js"; @@ -33,8 +33,9 @@ import { resolveMinerGoalSpec } from "./miner-goal-spec.js"; import { resolveClaimConflict } from "./claim-conflict-resolver.js"; import type { ClaimConflictResult, resolveClaimConflict as ResolveClaimConflictFn } from "./claim-conflict-resolver.js"; import { parsePrNumberFromExecResult } from "./pr-number-parse.js"; -import { initEventLedger } from "./event-ledger.js"; +import { appendEvent, initEventLedger, readEvents } from "./event-ledger.js"; import type { EventLedger } from "./event-ledger.js"; +import { createSignalTrackingStore } from "./signal-tracking-store.js"; import { initAttemptLog } from "./attempt-log.js"; import type { AttemptLog } from "./attempt-log.js"; import { initGovernorLedger } from "./governor-ledger.js"; @@ -137,6 +138,7 @@ export type RunAttemptOptions = { initEventLedger?: () => EventLedger; initAttemptLog?: () => AttemptLog; initGovernorLedger?: () => GovernorLedger; + initSignalTrackingStore?: () => SignalStore; buildAttemptDeps?: typeof buildAttemptDeps; resolveRejectionSignaled?: typeof ResolveRejectionSignaledFn; fetchImpl?: SelfReviewContextFetch; @@ -291,6 +293,44 @@ export function buildAttemptDeps( }; } +// #8543: the default SignalStore, backed by the miner's own shared local event ledger -- same singleton +// discover-cli.ts's initDefaultSignalTrackingStore already uses, no extra lifecycle management needed here. +function initDefaultSignalTrackingStore(): SignalStore { + return createSignalTrackingStore({ appendEvent, readEvents }); +} + +// #8543: records each avoid/raise reason a feasibility verdict fired as a rule-fired signal, so AMS can +// precision-score feasibility rules the same way discover-cli.ts's recordEligibilityExclusionSignals already +// does for eligibility exclusions. Best-effort, mirrored verbatim from that sibling: a store-open failure or a +// single write failure never changes the CLI's console output, JSON result shape, or exit code -- the +// infeasible branch still returns 4 either way. Every ready:false occurrence records, no dedup across repeated +// attempts at the same issue -- each attempt is a distinct decision instance. +async function recordFeasibilityVerdictSignals( + feasibility: { avoidReasons: readonly string[]; raiseReasons: readonly string[] }, + target: { repoFullName: string; issueNumber: number }, + options: Pick, + nowMs: number, +): Promise { + let store: SignalStore | null = null; + try { + store = (options.initSignalTrackingStore ?? initDefaultSignalTrackingStore)(); + } catch { + store = null; + } + if (!store) return; + const occurredAt = new Date(nowMs).toISOString(); + const targetKey = `${target.repoFullName}#issue-${target.issueNumber}`; + const fired: Array<{ ruleId: string; outcome: "avoid" | "raise" }> = [ + ...feasibility.avoidReasons.map((ruleId) => ({ ruleId, outcome: "avoid" as const })), + ...feasibility.raiseReasons.map((ruleId) => ({ ruleId, outcome: "raise" as const })), + ]; + for (const entry of fired) { + await store + .recordRuleFired({ ruleId: entry.ruleId, targetKey, outcome: entry.outcome, occurredAt }) + .catch(() => undefined); + } +} + /** * Run the `attempt` CLI subcommand end to end: resolveRejectionSignaled (before consuming a worktree slot) -> * acquire a concurrency slot -> assemble real AttemptDeps -> prepare a REAL git worktree -> fetch a real @@ -520,6 +560,12 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {} if (!codingTaskSpec.ready) { const reason = `infeasible_${codingTaskSpec.verdict}`; + await recordFeasibilityVerdictSignals( + codingTaskSpec.feasibility, + { repoFullName: parsed.repoFullName, issueNumber: parsed.issueNumber }, + options, + nowMs, + ); attemptLog.appendAttemptLogEvent({ eventType: "attempt_aborted", attemptId, diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index e7f3231cd2..971200db07 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -87,6 +87,17 @@ function fakeLoopResult(overrides: Record = {}) { }; } +/** A no-op SignalStore double (#8543), for tests that reach the infeasible branch but don't themselves assert + * on signal capture -- keeps them off the real on-disk event-ledger fallback under ~/.config, the same leak + * class discover-cli.ts's own initDefaultSignalTrackingStore comment already documents. */ +function fakeSignalStore() { + return { + recordRuleFired: vi.fn(async (_event: { ruleId: string; targetKey: string; outcome: string; occurredAt: string }) => undefined), + recordHumanOverride: vi.fn(async () => undefined), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + }; +} + /** The default set of injected options a test needs to reach past every real dependency and into (or * through) the final runMinerAttempt call, without doing any real network/git/subprocess work. */ function readyPipelineOptions(overrides: Record = {}) { @@ -1192,6 +1203,7 @@ describe("runAttempt (#5132)", () => { initEventLedger: () => eventLedger, initAttemptLog: () => attemptLog, initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => fakeSignalStore(), ...readyPipelineOptions({ buildCodingTaskSpec: () => ({ ready: false, @@ -1225,6 +1237,157 @@ describe("runAttempt (#5132)", () => { expect(cleanupAttemptWorktreeSpy).toHaveBeenCalledWith(expect.any(String), expect.any(String), true); }); + describe("feasibility-verdict signal capture (#8543)", () => { + it("records one rule-fired signal per avoid AND raise reason, with the exact ruleId/outcome/targetKey shape", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const store = fakeSignalStore(); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => store, + ...readyPipelineOptions({ + buildCodingTaskSpec: () => ({ + ready: false, + verdict: "avoid", + feasibility: { + verdict: "avoid", + avoidReasons: ["claim_status_solved", "issue_quality_do_not_use"], + raiseReasons: ["duplicate_cluster_high"], + summary: "not feasible", + }, + }), + }), + }); + + expect(store.recordRuleFired).toHaveBeenCalledTimes(3); + expect(store.recordRuleFired).toHaveBeenCalledWith( + expect.objectContaining({ ruleId: "claim_status_solved", outcome: "avoid", targetKey: "acme/widgets#issue-7" }), + ); + expect(store.recordRuleFired).toHaveBeenCalledWith( + expect.objectContaining({ ruleId: "issue_quality_do_not_use", outcome: "avoid", targetKey: "acme/widgets#issue-7" }), + ); + expect(store.recordRuleFired).toHaveBeenCalledWith( + expect.objectContaining({ ruleId: "duplicate_cluster_high", outcome: "raise", targetKey: "acme/widgets#issue-7" }), + ); + // No metadata (raw-context capture is a separate issue -- not this one) and a well-formed ISO timestamp. + for (const [event] of store.recordRuleFired.mock.calls) { + expect(event).not.toHaveProperty("metadata"); + expect(new Date(event.occurredAt).toISOString()).toBe(event.occurredAt); + } + }); + + it("records zero fired events on the ready:true path", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const store = fakeSignalStore(); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => store, + ...readyPipelineOptions({ runMinerAttempt: vi.fn(async () => ({ outcome: "submitted", loopResult: fakeLoopResult() }) as never) }), + }); + + expect(store.recordRuleFired).not.toHaveBeenCalled(); + }); + + it("a throwing initSignalTrackingStore never changes the exit code, console output, or JSON result", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + attemptId: "infeasible-attempt", + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => { + throw new Error("store unavailable"); + }, + ...readyPipelineOptions({ + buildCodingTaskSpec: () => ({ + ready: false, + verdict: "raise", + feasibility: { verdict: "raise", avoidReasons: [], raiseReasons: ["target_not_found"], summary: "issue not found" }, + }), + }), + }); + + expect(exitCode).toBe(4); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + outcome: "blocked_infeasible", + reason: "infeasible_raise", + verdict: "raise", + avoidReasons: [], + raiseReasons: ["target_not_found"], + repoFullName: "acme/widgets", + issueNumber: 7, + minerLogin: "alice", + base: "main", + mode: "dry_run", + attemptId: "infeasible-attempt", + }); + }); + + it("a store whose recordRuleFired rejects never changes the exit code, console output, or JSON result", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const rejectingStore = { + recordRuleFired: vi.fn(async () => { + throw new Error("write failed"); + }), + recordHumanOverride: vi.fn(async () => undefined), + queryRuleHistory: vi.fn(async () => ({ fired: [], overrides: [] })), + }; + + const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + attemptId: "infeasible-attempt", + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => rejectingStore, + ...readyPipelineOptions({ + buildCodingTaskSpec: () => ({ + ready: false, + verdict: "raise", + feasibility: { verdict: "raise", avoidReasons: [], raiseReasons: ["target_not_found"], summary: "issue not found" }, + }), + }), + }); + + expect(exitCode).toBe(4); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + outcome: "blocked_infeasible", + reason: "infeasible_raise", + verdict: "raise", + avoidReasons: [], + raiseReasons: ["target_not_found"], + repoFullName: "acme/widgets", + issueNumber: 7, + minerLogin: "alice", + base: "main", + mode: "dry_run", + attemptId: "infeasible-attempt", + }); + expect(rejectingStore.recordRuleFired).toHaveBeenCalledTimes(1); + }); + }); + it("REGRESSION: infeasible WITHOUT --json prints the feasibility verdict on stderr and exits 4", async () => { const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); const error = vi.spyOn(console, "error").mockImplementation(() => undefined); @@ -1236,6 +1399,7 @@ describe("runAttempt (#5132)", () => { initEventLedger: () => eventLedger, initAttemptLog: () => attemptLog, initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => fakeSignalStore(), ...readyPipelineOptions({ buildCodingTaskSpec: () => ({ ready: false, @@ -1879,6 +2043,7 @@ describe("runAttempt: real claim-ledger wiring (#5393)", () => { initEventLedger: () => eventLedger, initAttemptLog: () => attemptLog, initGovernorLedger: () => governorLedger, + initSignalTrackingStore: () => fakeSignalStore(), ...readyPipelineOptions({ buildCodingTaskSpec: () => ({ ready: false,