diff --git a/packages/gittensory-miner/lib/attempt-cli.d.ts b/packages/gittensory-miner/lib/attempt-cli.d.ts index 3dc05778e3..858b474a10 100644 --- a/packages/gittensory-miner/lib/attempt-cli.d.ts +++ b/packages/gittensory-miner/lib/attempt-cli.d.ts @@ -13,6 +13,7 @@ import type { resolveAmsPolicy } from "./ams-policy.js"; import type { checkMinerKillSwitch } from "./governor-kill-switch.js"; import type { resolveMinerGoalSpec } from "./miner-goal-spec.js"; import type { ClaimConflictResult, resolveClaimConflict } from "./claim-conflict-resolver.js"; +import type { getAttemptHistory } from "./portfolio-queue.js"; type CommonAttemptResultFields = { repoFullName: string; @@ -91,6 +92,7 @@ export type RunAttemptOptions = { resolveMinerGoalSpec?: typeof resolveMinerGoalSpec; runMinerAttempt?: typeof runMinerAttempt; resolveClaimConflict?: typeof resolveClaimConflict; + getAttemptHistory?: typeof getAttemptHistory; /** Invoked with the real structured result at every return point, in addition to (never instead of) the * plain exit-code return -- the loop orchestrator's real hook into what actually happened. */ onResult?: (result: AttemptCliResult) => void; diff --git a/packages/gittensory-miner/lib/attempt-cli.js b/packages/gittensory-miner/lib/attempt-cli.js index 75ffa7c153..78e135321d 100644 --- a/packages/gittensory-miner/lib/attempt-cli.js +++ b/packages/gittensory-miner/lib/attempt-cli.js @@ -9,8 +9,9 @@ // checkSubmissionFreshness cannot see (two miners submitting almost simultaneously). // // KNOWN, DOCUMENTED GAPS (not fabricated -- see attempt-input-builder.js's own header for the full list): -// governor.convergenceInput is an honest first-attempt-shaped literal, not a real per-issue attempt-history -// query (attempt-log.js's schema has no repo+issue index, and reenqueue counts aren't tracked anywhere yet). +// governor.reputationHistory/selfPlagiarismCandidate/selfPlagiarismRecentSubmissions are omitted (chokepoint.ts's +// own design treats that as "skip that stage entirely"). governor.convergenceInput is now a real per-issue +// portfolio-queue.js read (#5654), not a placeholder. import { resolveCodingAgentModeFromConfig, resolveFirstConfiguredCodingAgentDriverName } from "@loopover/engine"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; @@ -33,6 +34,7 @@ import { buildCodingTaskSpec } from "./coding-task-spec.js"; import { resolveAmsPolicy } from "./ams-policy.js"; import { checkMinerKillSwitch } from "./governor-kill-switch.js"; import { buildAttemptGovernorContext, buildAttemptLoopInput } from "./attempt-input-builder.js"; +import { getAttemptHistory } from "./portfolio-queue.js"; import { runMinerAttempt } from "./attempt-runner.js"; const ATTEMPT_USAGE = @@ -399,7 +401,14 @@ export async function runAttempt(args, options = {}) { amsPolicySpec: amsPolicy.spec, branchRef: worktreeResult.branchName, }); - const governor = buildAttemptGovernorContext(env, amsPolicy.spec, repoPaused); + + // Real per-issue attempt history (#5654): portfolio-queue.js's own claim/reclaim/requeue/done counters, + // keyed the same way opportunity-fanout.js enqueues issue-shaped candidates (`issue:`). No + // apiBaseUrl: this file has no multi-forge host context of its own today, so this reads (and every + // pre-#5563 single-forge caller already reads) the github.com default. + const readAttemptHistory = options.getAttemptHistory ?? getAttemptHistory; + const convergenceInput = readAttemptHistory(parsed.repoFullName, `issue:${parsed.issueNumber}`); + const governor = buildAttemptGovernorContext(env, amsPolicy.spec, repoPaused, convergenceInput); // Real soft-claim (#5393): recorded once we've committed to a real attempt (past feasibility), so a // sibling miner process on this machine sees it via claimLedger.listClaims/listActiveClaims while this diff --git a/packages/gittensory-miner/lib/attempt-input-builder.d.ts b/packages/gittensory-miner/lib/attempt-input-builder.d.ts index 9df919cc5e..770b8fdf4f 100644 --- a/packages/gittensory-miner/lib/attempt-input-builder.d.ts +++ b/packages/gittensory-miner/lib/attempt-input-builder.d.ts @@ -1,4 +1,10 @@ -import type { AmsPolicySpec, CodingAgentExecutionMode, IterateLoopInput, SelfReviewContext } from "@loopover/engine"; +import type { + AmsPolicySpec, + CodingAgentExecutionMode, + IterateLoopInput, + PortfolioConvergenceInput, + SelfReviewContext, +} from "@loopover/engine"; import type { AttemptGovernorContext } from "./attempt-runner.js"; import type { CodingTaskSpecResult } from "./coding-task-spec.js"; @@ -6,6 +12,7 @@ export function buildAttemptGovernorContext( env: Record, amsPolicySpec: AmsPolicySpec, repoPaused?: boolean, + convergenceInput?: PortfolioConvergenceInput, ): AttemptGovernorContext; export type BuildAttemptLoopInputInput = { diff --git a/packages/gittensory-miner/lib/attempt-input-builder.js b/packages/gittensory-miner/lib/attempt-input-builder.js index 3b2f11c8ac..aeadebc6dd 100644 --- a/packages/gittensory-miner/lib/attempt-input-builder.js +++ b/packages/gittensory-miner/lib/attempt-input-builder.js @@ -6,16 +6,14 @@ import { isGlobalMinerKillSwitch, isGlobalMinerLiveModeOptIn } from "@loopover/e // same discipline as coding-task-spec.js's own composers. // // KNOWN, DOCUMENTED GAPS (not fabricated -- explicitly left as real, narrow follow-ups): -// - governor.convergenceInput is a first-attempt-shaped literal ({ attempts: 0, consecutiveFailures: 0, -// reenqueues: 0, reachedDone: false }), not a real per-issue query. attempt-log.js's schema has no -// repo+issue index (attemptId embeds a timestamp, so it's not a stable group key), and reenqueue counts -// aren't tracked ANYWHERE yet (non-convergence.ts's own header says that belongs on the portfolio-queue -// table once it grows attempt-history columns -- a real, separate schema change, not something to fake -// here). This literal is only ever an UNDER-estimate (reads "fresh, no prior failures" even on a real -// Nth attempt), which fails toward LETTING an attempt through, not blocking one -- documented, not silent. // - governor.reputationHistory/selfPlagiarismCandidate/selfPlagiarismRecentSubmissions are omitted, which // chokepoint.ts's own design treats as "skip that stage entirely" -- an honest absence, not a fabricated // "clean" verdict. +// +// governor.convergenceInput is now a REAL per-issue attempt-history query (#5654): the caller (attempt-cli.js) +// reads it from portfolio-queue.js's own getAttemptHistory and passes it in here, this composer staying pure +// over it same as every other already-computed dependency below. The zero-state fallback only fires when a +// caller genuinely omits the argument -- an honest first-attempt shape, not the old hardcoded literal. /** * Assemble the real Governor chokepoint context for one attempt. rateLimitBuckets/rateLimitBackoffAttempts/ @@ -26,18 +24,24 @@ import { isGlobalMinerKillSwitch, isGlobalMinerLiveModeOptIn } from "@loopover/e * (miner-goal-spec.js's resolveMinerGoalSpec) -- this composer stays pure and just threads whatever the * caller already resolved through; passing nothing keeps the prior fails-open-on-that-axis-only behavior. * + * `convergenceInput` (#5654) is the caller's own real portfolio-queue.js `getAttemptHistory` read -- this + * composer stays pure and just threads it through, same as `repoPaused`. Omitted (never fabricated) falls + * back to the honest first-attempt-shaped zero-state, so a caller that hasn't wired a real read yet (or an + * item genuinely absent from the queue) still produces a well-formed `PortfolioConvergenceInput`. + * * @param {Record} env * @param {import("@loopover/engine").AmsPolicySpec} amsPolicySpec * @param {boolean} [repoPaused] + * @param {import("@loopover/engine").PortfolioConvergenceInput} [convergenceInput] * @returns {import("./attempt-runner.js").AttemptGovernorContext} */ -export function buildAttemptGovernorContext(env, amsPolicySpec, repoPaused) { +export function buildAttemptGovernorContext(env, amsPolicySpec, repoPaused, convergenceInput) { return { killSwitchGlobal: isGlobalMinerKillSwitch(env), killSwitchRepoPaused: repoPaused, liveModeGlobalOptIn: isGlobalMinerLiveModeOptIn(env), capLimits: amsPolicySpec.capLimits, - convergenceInput: { attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }, + convergenceInput: convergenceInput ?? { attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }, }; } @@ -71,8 +75,9 @@ export function buildAttemptLoopInput(input) { // Real mid-attempt budget (#5395): the SAME Governor cap ceilings that already bound cross-cycle spend // (loop-cli.js's after-the-fact governorState.saveCapUsage) now also bound this ONE attempt in progress, // via the engine's real accumulateAttemptUsage/evaluateAttemptBudget -- a runaway attempt can no longer - // burn through the entire cross-cycle budget before anything reacts. No maxTokens: no driver reports a - // real per-iteration token count today, so that axis has no real ceiling to set (never fabricated). + // burn through the entire cross-cycle budget before anything reacts. No maxTokens: every driver now + // reports a real per-iteration token count (#5653), but no policy field sets a token ceiling yet -- that + // axis genuinely has no real ceiling to set today, never fabricated. budget: { maxTurns: input.amsPolicySpec.capLimits.turns, maxWallClockMs: input.amsPolicySpec.capLimits.elapsedMs, diff --git a/packages/gittensory-miner/lib/portfolio-queue.d.ts b/packages/gittensory-miner/lib/portfolio-queue.d.ts index 15c9ff9d57..9a316a0d10 100644 --- a/packages/gittensory-miner/lib/portfolio-queue.d.ts +++ b/packages/gittensory-miner/lib/portfolio-queue.d.ts @@ -25,6 +25,15 @@ export type QueueLeaseEntry = { leasedAt: string | null; }; +/** A real per-item PortfolioConvergenceInput (non-convergence.ts, #5654), read from this store's own + * attempt-history counters -- see getAttemptHistory. */ +export type QueueAttemptHistory = { + attempts: number; + consecutiveFailures: number; + reenqueues: number; + reachedDone: boolean; +}; + export type PortfolioQueueStore = { dbPath: string; enqueue(item: EnqueueItem): QueueEntry; @@ -40,6 +49,7 @@ export type PortfolioQueueStore = { entries: QueueEntry[], ) => Array<{ repoFullName: string; identifier: string; apiBaseUrl?: string }>, ): QueueEntry[]; + getAttemptHistory(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueAttemptHistory; close(): void; }; @@ -59,4 +69,6 @@ export function markDone(repoFullName: string, identifier: string, apiBaseUrl?: export function markFailed(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; +export function getAttemptHistory(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueAttemptHistory; + export function closeDefaultPortfolioQueueStore(): void; diff --git a/packages/gittensory-miner/lib/portfolio-queue.js b/packages/gittensory-miner/lib/portfolio-queue.js index d86069241e..094b84eda0 100644 --- a/packages/gittensory-miner/lib/portfolio-queue.js +++ b/packages/gittensory-miner/lib/portfolio-queue.js @@ -147,6 +147,24 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) migrationDb.exec("DROP TABLE miner_portfolio_queue"); migrationDb.exec("ALTER TABLE miner_portfolio_queue_v3 RENAME TO miner_portfolio_queue"); }, + // v3 -> v4 (#5654): three attempt-history counters feeding non-convergence.ts's real + // PortfolioConvergenceInput (see getAttemptHistory below) -- additive columns, same + // defensive column-presence guard as the leased_at migration above. + (migrationDb) => { + const existingColumns = migrationDb + .prepare("PRAGMA table_info(miner_portfolio_queue)") + .all() + .map((column) => column.name); + if (!existingColumns.includes("attempts_count")) { + migrationDb.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN attempts_count INTEGER NOT NULL DEFAULT 0"); + } + if (!existingColumns.includes("consecutive_failures")) { + migrationDb.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN consecutive_failures INTEGER NOT NULL DEFAULT 0"); + } + if (!existingColumns.includes("reenqueue_count")) { + migrationDb.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN reenqueue_count INTEGER NOT NULL DEFAULT 0"); + } + }, ]); // `rowid` is a stable, unique key assigned once at first insert (re-enqueue updates in place, never re-inserts), @@ -172,24 +190,33 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) // flips it to 'in_progress', RETURNING it — so two processes sharing the file can't both claim the same row (a // separate SELECT-then-UPDATE would race). Deliberately global (no api_base_url filter): the queue is a single // cross-host priority ordering, not a per-host one. - // Claiming stamps `leased_at` with the caller-supplied claim time; leaving 'in_progress' (done/failed/reclaim) - // clears it back to NULL so only genuinely in-flight rows carry a lease. + // Claiming stamps `leased_at` with the caller-supplied claim time and increments the attempt-history + // `attempts_count` (#5654, non-convergence.ts's real PortfolioConvergenceInput.attempts) -- leaving + // 'in_progress' (done/failed/reclaim) clears leased_at back to NULL so only genuinely in-flight rows carry + // a lease. const dequeueStatement = db.prepare(` - UPDATE miner_portfolio_queue SET status = 'in_progress', leased_at = ? + UPDATE miner_portfolio_queue SET status = 'in_progress', leased_at = ?, attempts_count = attempts_count + 1 WHERE rowid = ( SELECT rowid FROM miner_portfolio_queue WHERE status = 'queued' ${ORDER} LIMIT 1 ) RETURNING * `); // RETURNING (rather than a separate post-UPDATE SELECT) makes the "nothing to mark done" case observable - // directly from one atomic statement. + // directly from one atomic statement. consecutive_failures resets to 0 on reaching done (#5654) -- the + // active failure streak breaks the moment an attempt actually succeeds; reenqueue_count is a lifetime + // total and deliberately untouched here (see getAttemptHistory's own doc comment). const markDoneStatement = db.prepare(` - UPDATE miner_portfolio_queue SET status = 'done', leased_at = NULL + UPDATE miner_portfolio_queue SET status = 'done', leased_at = NULL, consecutive_failures = 0 WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status <> 'done' RETURNING * `); + // Releasing an in-flight item back to queued WITHOUT reaching done is exactly non-convergence.ts's own + // "cycling queued -> in_progress -> queued without ever reaching done" reenqueue trigger (#5654) -- same + // counters, same increment, as reclaimStuckItem below (both are this same transition, just different + // callers: a run-halt release here vs. a stale-lease sweep there). const markFailedStatement = db.prepare(` - UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL + UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL, + consecutive_failures = consecutive_failures + 1, reenqueue_count = reenqueue_count + 1 WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'in_progress' RETURNING * `); @@ -203,24 +230,36 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) const listInProgressStatement = db.prepare( `SELECT * FROM miner_portfolio_queue WHERE status = 'in_progress' ${ORDER}`, ); + // A stale-lease sweep release is the SAME "in_progress -> queued without reaching done" event as + // markFailedStatement above (#5654) -- same counters, same increment. const reclaimStatement = db.prepare(` - UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL + UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL, + consecutive_failures = consecutive_failures + 1, reenqueue_count = reenqueue_count + 1 WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'in_progress' RETURNING * `); // Requeue only ever targets a COMPLETED ('done') row — an in-flight item is released via reclaimStatement, and // an already-'queued' item is a no-op — so a caller's manual requeue can never disturb an active claim. The // row keeps its rowid/enqueued_at, so it re-enters the queue at its original FIFO position, not the back. + // Deliberately leaves attempts_count/consecutive_failures/reenqueue_count untouched (#5654): this is a + // manual reopen of ALREADY-COMPLETED work, not the stuck queued->in_progress->queued cycle those counters + // track -- reachedDone (derived live from status) simply reads false again once requeued, same as any + // other non-done row, until the item is claimed and completed again. const requeueStatement = db.prepare(` UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'done' RETURNING * `); + // Same attempts_count increment as dequeueStatement (#5654) -- batchClaim's per-item claim is just as much + // a real attempt as the single-item dequeueNext path. const claimTargetStatement = db.prepare(` - UPDATE miner_portfolio_queue SET status = 'in_progress', leased_at = ? + UPDATE miner_portfolio_queue SET status = 'in_progress', leased_at = ?, attempts_count = attempts_count + 1 WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'queued' RETURNING * `); + const attemptHistoryStatement = db.prepare( + "SELECT attempts_count, consecutive_failures, reenqueue_count, status FROM miner_portfolio_queue WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ?", + ); return { dbPath: resolvedPath, @@ -313,6 +352,28 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) throw error; } }, + /** + * A real `PortfolioConvergenceInput` (non-convergence.ts) for one queue item (#5654), replacing the + * first-attempt-shaped literal attempt-input-builder.js previously hardcoded. An item never enqueued here + * (not yet tracked at all) reads the same honest zero-state as a genuine first attempt -- absence of + * history is not evidence of a problem, same rule non-convergence.ts's own header documents. `reachedDone` + * is derived live from the row's current `status`, not a separate persisted flag (see requeueStatement's + * comment above for why that's the deliberate choice). + */ + getAttemptHistory(repoFullName, identifier, apiBaseUrl) { + const row = attemptHistoryStatement.get( + normalizeApiBaseUrl(apiBaseUrl), + normalizeRepoFullName(repoFullName), + normalizeIdentifier(identifier), + ); + if (!row) return { attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }; + return { + attempts: row.attempts_count, + consecutiveFailures: row.consecutive_failures, + reenqueues: row.reenqueue_count, + reachedDone: row.status === "done", + }; + }, close() { db.close(); }, @@ -344,6 +405,10 @@ export function markFailed(repoFullName, identifier, apiBaseUrl) { return getDefaultPortfolioQueueStore().markFailed(repoFullName, identifier, apiBaseUrl); } +export function getAttemptHistory(repoFullName, identifier, apiBaseUrl) { + return getDefaultPortfolioQueueStore().getAttemptHistory(repoFullName, identifier, apiBaseUrl); +} + export function closeDefaultPortfolioQueueStore() { if (!defaultPortfolioQueueStore) return; defaultPortfolioQueueStore.close(); diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index 5c81b2c9dd..b296e56205 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -13,6 +13,7 @@ import { closeDefaultAttemptLog, initAttemptLog } from "../../packages/gittensor import type { AttemptLog } from "../../packages/gittensory-miner/lib/attempt-log.js"; import { closeDefaultGovernorLedger, initGovernorLedger } from "../../packages/gittensory-miner/lib/governor-ledger.js"; import { closeDefaultWorktreeAllocator, openWorktreeAllocator } from "../../packages/gittensory-miner/lib/worktree-allocator.js"; +import { closeDefaultPortfolioQueueStore } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; import { buildAttemptDeps, parseAttemptArgs, runAttempt } from "../../packages/gittensory-miner/lib/attempt-cli.js"; import type { PrepareAttemptWorktreeResult } from "../../packages/gittensory-miner/lib/attempt-worktree.js"; import { DEFAULT_AMS_POLICY_SPEC, DEFAULT_MINER_GOAL_SPEC, parseFocusManifest } from "../../packages/gittensory-engine/src/index"; @@ -65,6 +66,9 @@ function readyPipelineOptions(overrides: Record = {}) { resolveAmsPolicy: async () => ({ spec: DEFAULT_AMS_POLICY_SPEC, source: "default" as const, warnings: [] }), checkMinerKillSwitch: () => ({ scope: "none" as const, active: false }), resolveMinerGoalSpec: () => ({ present: false, spec: DEFAULT_MINER_GOAL_SPEC, warnings: [] }), + // Never touches the real (filesystem-backed) default portfolio-queue store (#5654) -- a test that cares + // about a real convergenceInput value overrides this explicitly. + getAttemptHistory: () => ({ attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }), ...overrides, }; } @@ -90,6 +94,8 @@ afterEach(() => { closeDefaultEventLedger(); closeDefaultAttemptLog(); closeDefaultGovernorLedger(); + closeDefaultPortfolioQueueStore(); + vi.unstubAllEnvs(); vi.restoreAllMocks(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); @@ -391,6 +397,59 @@ describe("runAttempt (#5132)", () => { expect(summaryCalls[0]).not.toHaveProperty("tokensUsed"); }); + it("REGRESSION (#5654): the real portfolio-queue attempt history is read for THIS issue and threads into governor.convergenceInput, not the old hardcoded literal", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const realHistory = { attempts: 4, consecutiveFailures: 3, reenqueues: 3, reachedDone: false }; + const getAttemptHistorySpy = vi.fn().mockReturnValue(realHistory); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "abandon", + loopResult: { outcome: "abandon", totalTurnsUsed: 0, totalCostUsd: 0, iterationsUsed: 0 }, + }); + + await runAttempt(["acme/widgets", "42", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ getAttemptHistory: getAttemptHistorySpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(getAttemptHistorySpy).toHaveBeenCalledWith("acme/widgets", "issue:42"); + const [input] = runMinerAttemptSpy.mock.calls[0]!; + expect(input.governor.convergenceInput).toEqual(realHistory); + }); + + it("REGRESSION (#5654): when options.getAttemptHistory is omitted, runAttempt falls back to the REAL portfolio-queue.js default, not a fabricated result", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-attempt-cli-portfolio-")); + roots.push(root); + vi.stubEnv("GITTENSORY_MINER_PORTFOLIO_QUEUE_DB", join(root, "portfolio-queue.sqlite3")); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "abandon", + loopResult: { outcome: "abandon", totalTurnsUsed: 0, totalCostUsd: 0, iterationsUsed: 0 }, + }); + const { getAttemptHistory: _omitted, ...optionsWithoutGetAttemptHistory } = readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }); + + await runAttempt(["acme/widgets", "99", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...optionsWithoutGetAttemptHistory, + }); + + // The item was never enqueued in this fresh store -- the real default read honestly returns the + // zero-state, same shape as the hardcoded literal it replaced, but genuinely read from disk. + const [input] = runMinerAttemptSpy.mock.calls[0]!; + expect(input.governor.convergenceInput).toEqual({ attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }); + }); + it("#5185: writes attempt_outcome_summary with the real provider/cost on a non-submitted outcome too", async () => { const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/test/unit/miner-attempt-input-builder.test.ts b/test/unit/miner-attempt-input-builder.test.ts index f566f2e7de..430c469cdf 100644 --- a/test/unit/miner-attempt-input-builder.test.ts +++ b/test/unit/miner-attempt-input-builder.test.ts @@ -58,11 +58,17 @@ describe("buildAttemptGovernorContext (#5132)", () => { expect(ctx.killSwitchRepoPaused).toBeUndefined(); }); - it("REGRESSION: convergenceInput is an honest first-attempt-shaped literal, not fabricated real history", () => { + it("convergenceInput defaults to the honest first-attempt-shaped zero-state when the caller omits it", () => { const ctx = buildAttemptGovernorContext({}, DEFAULT_AMS_POLICY_SPEC); expect(ctx.convergenceInput).toEqual({ attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }); }); + it("REGRESSION (#5654): a real convergenceInput the caller passes threads through unchanged, not fabricated", () => { + const realHistory = { attempts: 4, consecutiveFailures: 3, reenqueues: 3, reachedDone: false }; + const ctx = buildAttemptGovernorContext({}, DEFAULT_AMS_POLICY_SPEC, undefined, realHistory); + expect(ctx.convergenceInput).toEqual(realHistory); + }); + it("omits rateLimitBuckets/rateLimitBackoffAttempts/capUsage so the persisted governor-state store auto-supplies them", () => { const ctx = buildAttemptGovernorContext({}, DEFAULT_AMS_POLICY_SPEC); expect(ctx).not.toHaveProperty("rateLimitBuckets"); diff --git a/test/unit/miner-migrate-cli.test.ts b/test/unit/miner-migrate-cli.test.ts index b7995dee8c..c512dd7ff1 100644 --- a/test/unit/miner-migrate-cli.test.ts +++ b/test/unit/miner-migrate-cli.test.ts @@ -85,15 +85,19 @@ describe("gittensory-miner migrate (#4871)", () => { const results = runMigrateChecks(env); const portfolioQueue = results.find((result) => result.name === "portfolio-queue"); - // Runs BOTH post-baseline migrations in sequence: v1->v2 adds leased_at, v2->v3 adds api_base_url (#5563). - expect(portfolioQueue).toMatchObject({ ok: true, status: "migrated", versionBefore: 1, versionAfter: 3 }); + // Runs ALL THREE post-baseline migrations in sequence: v1->v2 adds leased_at, v2->v3 adds api_base_url + // (#5563), v3->v4 adds the attempt-history counters (#5654). + expect(portfolioQueue).toMatchObject({ ok: true, status: "migrated", versionBefore: 1, versionAfter: 4 }); const verifyDb = new DatabaseSync(dbPath, { readOnly: true }); try { const columns = verifyDb.prepare("PRAGMA table_info(miner_portfolio_queue)").all().map((column) => column.name); expect(columns).toContain("leased_at"); expect(columns).toContain("api_base_url"); - expect(verifyDb.prepare("PRAGMA user_version").get()?.user_version).toBe(3); + expect(columns).toContain("attempts_count"); + expect(columns).toContain("consecutive_failures"); + expect(columns).toContain("reenqueue_count"); + expect(verifyDb.prepare("PRAGMA user_version").get()?.user_version).toBe(4); } finally { verifyDb.close(); } diff --git a/test/unit/miner-portfolio-queue.test.ts b/test/unit/miner-portfolio-queue.test.ts index dee0e43b03..4dd182dc17 100644 --- a/test/unit/miner-portfolio-queue.test.ts +++ b/test/unit/miner-portfolio-queue.test.ts @@ -8,12 +8,19 @@ import { closeDefaultPortfolioQueueStore, dequeueNext, enqueue, + getAttemptHistory, initPortfolioQueueStore, markDone, markFailed, resolvePortfolioQueueDbPath, } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; +vi.mock("@loopover/engine", async () => { + return import("../../packages/gittensory-engine/src/index"); +}); + +import { classifyPortfolioConvergence } from "../../packages/gittensory-engine/src/index"; + const roots: string[] = []; const stores: Array<{ close(): void }> = []; @@ -358,4 +365,205 @@ describe("gittensory-miner portfolio/queue store (#2292)", () => { expect(store.listQueue().map((entry) => entry.repoFullName)).toEqual(["acme/widgets"]); }); }); + + describe("real attempt-history counters feeding non-convergence.ts's PortfolioConvergenceInput (#5654)", () => { + it("getAttemptHistory reads the honest zero-state for an item never enqueued at all", () => { + const store = tempStore(); + expect(store.getAttemptHistory("o/a", "issue:404")).toEqual({ + attempts: 0, + consecutiveFailures: 0, + reenqueues: 0, + reachedDone: false, + }); + }); + + it("increments attempts_count on every real claim -- both dequeueNext and batchClaim", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "1", priority: 1 }); + store.enqueue({ repoFullName: "o/a", identifier: "2", priority: 1 }); + + store.dequeueNext(); + expect(store.getAttemptHistory("o/a", "1").attempts).toBe(1); + + store.batchClaim((entries) => entries.map((entry) => ({ repoFullName: entry.repoFullName, identifier: entry.identifier, apiBaseUrl: entry.apiBaseUrl }))); + expect(store.getAttemptHistory("o/a", "2").attempts).toBe(1); + + // Re-claiming after a release increments again -- a real cumulative lifetime count, not a flag. + store.markFailed("o/a", "1"); + store.dequeueNext(); + expect(store.getAttemptHistory("o/a", "1").attempts).toBe(2); + }); + + it("markFailed and reclaimStuckItem both increment consecutiveFailures and reenqueues for the same in_progress -> queued without reaching done transition", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "halted", priority: 1 }); + store.enqueue({ repoFullName: "o/a", identifier: "stuck", priority: 1 }); + store.dequeueNext(); + store.dequeueNext(); + + store.markFailed("o/a", "halted"); + expect(store.getAttemptHistory("o/a", "halted")).toMatchObject({ consecutiveFailures: 1, reenqueues: 1 }); + + store.reclaimStuckItem("o/a", "stuck"); + expect(store.getAttemptHistory("o/a", "stuck")).toMatchObject({ consecutiveFailures: 1, reenqueues: 1 }); + }); + + it("markDone resets consecutiveFailures to 0 but leaves the lifetime reenqueues total untouched", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "work", priority: 1 }); + store.dequeueNext(); + store.markFailed("o/a", "work"); + store.dequeueNext(); + store.markFailed("o/a", "work"); + expect(store.getAttemptHistory("o/a", "work")).toMatchObject({ consecutiveFailures: 2, reenqueues: 2 }); + + store.dequeueNext(); + store.markDone("o/a", "work"); + expect(store.getAttemptHistory("o/a", "work")).toMatchObject({ consecutiveFailures: 0, reenqueues: 2, reachedDone: true }); + }); + + it("requeueItem (a manual reopen of already-completed work) touches no counter and un-sets reachedDone", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "work", priority: 1 }); + store.dequeueNext(); + store.markDone("o/a", "work"); + const beforeRequeue = store.getAttemptHistory("o/a", "work"); + expect(beforeRequeue).toMatchObject({ attempts: 1, consecutiveFailures: 0, reenqueues: 0, reachedDone: true }); + + store.requeueItem("o/a", "work"); + // reachedDone is derived live from status, not a separate flag -- once requeued the row is 'queued' + // again, so this genuinely reads false until the item is claimed and completes again. + expect(store.getAttemptHistory("o/a", "work")).toEqual({ ...beforeRequeue, reachedDone: false }); + }); + + it("REGRESSION: reachedDone always reflects the CURRENT status, never a fabricated 'ever reached done' flag", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "work", priority: 1 }); + expect(store.getAttemptHistory("o/a", "work").reachedDone).toBe(false); // queued + store.dequeueNext(); + expect(store.getAttemptHistory("o/a", "work").reachedDone).toBe(false); // in_progress + store.markDone("o/a", "work"); + expect(store.getAttemptHistory("o/a", "work").reachedDone).toBe(true); // done + }); + + it("getAttemptHistory is scoped by apiBaseUrl, mirroring every other #5563 forge-scoped read", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + store.dequeueNext(); // claims the github.com row (enqueued first) + expect(store.getAttemptHistory("acme/widgets", "issue:1", "https://api.github.com").attempts).toBe(1); + expect(store.getAttemptHistory("acme/widgets", "issue:1", "https://ghe.example.com/api/v3").attempts).toBe(0); + }); + + it("module-level getAttemptHistory delegates to the default portfolio-queue store", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-default-")); + roots.push(root); + vi.stubEnv("GITTENSORY_MINER_PORTFOLIO_QUEUE_DB", join(root, "portfolio-queue.sqlite3")); + enqueue({ repoFullName: "o/a", identifier: "work", priority: 1 }); + dequeueNext(); + expect(getAttemptHistory("o/a", "work").attempts).toBe(1); + }); + + it("REGRESSION: a genuinely non-convergent item (repeated requeue without ever reaching done) now produces a PortfolioConvergenceInput the real detector classifies as non_convergent -- the first real exercise of that detector (#5654)", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "o/a", identifier: "flaky", priority: 1 }); + for (let attempt = 0; attempt < 3; attempt += 1) { + store.dequeueNext(); + store.markFailed("o/a", "flaky"); + } + const history = store.getAttemptHistory("o/a", "flaky"); + expect(classifyPortfolioConvergence(history).status).toBe("non_convergent"); + }); + + it("migrates an existing pre-#5654 file (already at the api_base_url v3 shape) by backfilling the counters to 0, preserving every row", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-legacy-v3-")); + roots.push(root); + const dbPath = join(root, "legacy-v3.sqlite3"); + const legacy = new DatabaseSync(dbPath); + legacy.exec(` + CREATE TABLE miner_portfolio_queue ( + api_base_url TEXT NOT NULL, + repo_full_name TEXT NOT NULL, + identifier TEXT NOT NULL, + priority REAL NOT NULL DEFAULT 0, + status TEXT NOT NULL DEFAULT 'queued' CHECK (status IN ('queued', 'in_progress', 'done')), + enqueued_at TEXT NOT NULL, + leased_at TEXT, + PRIMARY KEY (api_base_url, repo_full_name, identifier) + ) + `); + legacy.exec("PRAGMA user_version = 3"); + legacy.exec( + "INSERT INTO miner_portfolio_queue (api_base_url, repo_full_name, identifier, priority, status, enqueued_at, leased_at) VALUES ('https://api.github.com', 'acme/widgets', 'issue:5', 3, 'queued', '2026-01-01T00:00:00.000Z', NULL)", + ); + legacy.close(); + + const store = initPortfolioQueueStore(dbPath); + stores.push(store); + expect(store.getAttemptHistory("acme/widgets", "issue:5")).toEqual({ + attempts: 0, + consecutiveFailures: 0, + reenqueues: 0, + reachedDone: false, + }); + // The pre-existing row itself is untouched by the additive migration. + expect(store.listQueue("acme/widgets")).toEqual([ + { + apiBaseUrl: "https://api.github.com", + repoFullName: "acme/widgets", + identifier: "issue:5", + priority: 3, + status: "queued", + enqueuedAt: "2026-01-01T00:00:00.000Z", + }, + ]); + // A real claim on the migrated row proves the new columns are genuinely writable, not just present. + store.dequeueNext(); + expect(store.getAttemptHistory("acme/widgets", "issue:5").attempts).toBe(1); + }); + + it("REGRESSION: a v3 file that (unusually) already carries all three new columns is not re-altered into a duplicate-column error", () => { + // Mirrors the leased_at migration's own defensive column-presence guard one migration index up: a file + // that already has the three new columns (e.g. from a partially-applied prior migration attempt) must + // not crash re-adding columns that already exist -- each of the three checks must independently skip. + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-legacy-partial-v4-")); + roots.push(root); + const dbPath = join(root, "legacy-partial-v4.sqlite3"); + const legacy = new DatabaseSync(dbPath); + legacy.exec(` + CREATE TABLE miner_portfolio_queue ( + api_base_url TEXT NOT NULL, + repo_full_name TEXT NOT NULL, + identifier TEXT NOT NULL, + priority REAL NOT NULL DEFAULT 0, + status TEXT NOT NULL DEFAULT 'queued' CHECK (status IN ('queued', 'in_progress', 'done')), + enqueued_at TEXT NOT NULL, + leased_at TEXT, + attempts_count INTEGER NOT NULL DEFAULT 0, + consecutive_failures INTEGER NOT NULL DEFAULT 0, + reenqueue_count INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY (api_base_url, repo_full_name, identifier) + ) + `); + legacy.exec("PRAGMA user_version = 3"); + legacy.exec( + "INSERT INTO miner_portfolio_queue (api_base_url, repo_full_name, identifier, priority, status, enqueued_at, leased_at, attempts_count, consecutive_failures, reenqueue_count) VALUES ('https://api.github.com', 'acme/widgets', 'issue:9', 1, 'queued', '2026-01-01T00:00:00.000Z', NULL, 7, 2, 5)", + ); + legacy.close(); + + let opened: ReturnType | undefined; + expect(() => { + opened = initPortfolioQueueStore(dbPath); + }).not.toThrow(); + const store = opened!; + stores.push(store); + // Every pre-existing counter survives untouched -- no column was re-added, no data was reset. + expect(store.getAttemptHistory("acme/widgets", "issue:9")).toEqual({ + attempts: 7, + consecutiveFailures: 2, + reenqueues: 5, + reachedDone: false, + }); + }); + }); });