Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 18 additions & 32 deletions packages/gittensory-miner/lib/loop-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@
//
// REAL, NOT FABRICATED: this loop is the first production caller of governor-state.js's `saveCapUsage`
// (turnsTaken from runMinerAttempt's own real `loopResult.totalTurnsUsed`, elapsedMs from real wall-clock
// measurement) and of a genuine per-identifier convergence history (attempts/consecutiveFailures/reenqueues
// tracked in this process's own memory across its own cycles) -- both were previously honest zero/placeholder
// literals (see attempt-input-builder.js's own header) because a ONE-SHOT `attempt` CLI invocation has no
// cross-call history to draw on. A long-running loop genuinely does.
//
// DOCUMENTED GAP: convergence/cap-usage history is IN-MEMORY, scoped to this loop process's own lifetime (cap
// usage itself persists across restarts via governor-state.js; per-identifier convergence counters do not --
// a durable version needs attempt-log.js to grow a repo+issue index, the same separate schema change
// attempt-input-builder.js's header already flags as out of scope here).
// measurement). Its per-identifier convergence history (attempts/consecutiveFailures/reenqueues) is the real,
// SQLite-persisted portfolio-queue attempt-history (portfolio-queue.js's getAttemptHistory, #5654) that the
// dequeueNext claim + markDone/markFailed calls below already maintain -- the same source a one-shot `attempt`
// invocation reads (#5654), so both share one source of truth and the counters survive a loop-daemon restart
// (crash/deploy/systemd bounce) instead of resetting with the process (#5677).

import { checkMinerKillSwitch } from "./governor-kill-switch.js";
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
Expand Down Expand Up @@ -169,14 +165,6 @@ function parseIssueNumberFromIdentifier(identifier) {
return match ? Number(match[1]) : null;
}

function convergenceKey(repoFullName, identifier) {
return `${repoFullName}:${identifier}`;
}

function zeroConvergence() {
return { attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false };
}

/**
* Run one full discover -> claim -> attempt -> observe -> reenter cycle repeatedly until a kill-switch trips,
* the run-loop boundary gate halts (non-convergence or a real budget/turn/elapsed cap), re-entry is declined,
Expand Down Expand Up @@ -290,7 +278,6 @@ export async function runLoop(args, options = {}) {
}

let usage = governorState.loadCapUsage();
const convergenceHistory = new Map();
const cycles = [];
let sinceSeq = eventLedger.readEvents({}).at(-1)?.seq ?? 0;
let haltReason = null;
Expand Down Expand Up @@ -352,9 +339,15 @@ export async function runLoop(args, options = {}) {
continue;
}

const key = convergenceKey(claimed.repoFullName, claimed.identifier);
const amsPolicy = await resolveAmsPolicyFn(claimed.repoFullName, { env });
const convergenceInput = convergenceHistory.get(key) ?? zeroConvergence();
// Real, SQLite-persisted per-item convergence history (#5677): the dequeueNext claim above already recorded
// this attempt and the markDone/markFailed calls below record the outcome, so reading it back here shares one
// source of truth with attempt-cli.js (#5654) and survives a loop-daemon restart instead of resetting.
const convergenceInput = portfolioQueue.getAttemptHistory(
claimed.repoFullName,
claimed.identifier,
claimed.apiBaseUrl,
);

const boundary = evaluateBoundaryGateFn(
{
Expand All @@ -377,9 +370,6 @@ export async function runLoop(args, options = {}) {
break;
}

convergenceInput.attempts += 1;
convergenceHistory.set(key, convergenceInput);

const cycleStartMs = nowMsFn();
let lastResult = null;
const attemptArgv = [
Expand Down Expand Up @@ -420,19 +410,15 @@ export async function runLoop(args, options = {}) {
// (reenqueues threshold) rather than silently retried forever.
const permanentBlock = attemptOutcome === "blocked_rejection_signaled";

if (submitted) {
portfolioQueue.markDone(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl);
convergenceInput.reachedDone = true;
convergenceInput.consecutiveFailures = 0;
} else if (permanentBlock) {
if (submitted || permanentBlock) {
// Both terminal -- a submitted PR is done, and a repo-wide AI-usage-policy ban never resolves on retry --
// so neither is re-queued. markDone also clears the persisted consecutive-failure streak.
portfolioQueue.markDone(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl);
convergenceInput.consecutiveFailures += 1;
} else {
// Any other blocked/abandoned/stale/governed outcome may resolve on a later retry, so requeue it; markFailed
// records the re-enqueue + consecutive failure the non-convergence detector reads on the next cycle.
portfolioQueue.markFailed(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl);
convergenceInput.consecutiveFailures += 1;
convergenceInput.reenqueues += 1;
}
convergenceHistory.set(key, convergenceInput);

let reentryOutcome = "other";
let prNumber = null;
Expand Down
40 changes: 40 additions & 0 deletions test/unit/miner-loop-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,46 @@ describe("runLoop (#5135)", () => {
expect(runDiscoverSpy).toHaveBeenCalledTimes(1);
});

it("REGRESSION (#5677): reads convergence history from the persisted portfolio queue, so a stuck item's re-enqueue streak survives a loop-daemon restart instead of resetting to fresh", async () => {
const { eventLedger, governorLedger, portfolioQueue, runState, governorState, paths } = tempStores();
vi.spyOn(console, "log").mockImplementation(() => undefined);
const item = { repoFullName: "acme/widgets", identifier: "issue:7" };

// Prior loop cycles that never reached done: each claim -> markFailed persists one re-enqueue + consecutive
// failure on the queue row (portfolio-queue.js, #5654/#5661), taking the item to the non-convergence
// threshold -- the state a real long-running loop would have accumulated before a restart.
const { maxReenqueues } = DEFAULT_AMS_POLICY_SPEC.convergenceThresholds;
portfolioQueue.enqueue(item);
for (let cycle = 0; cycle < maxReenqueues; cycle += 1) {
portfolioQueue.dequeueNext();
portfolioQueue.markFailed(item.repoFullName, item.identifier);
}
expect(portfolioQueue.getAttemptHistory(item.repoFullName, item.identifier).reenqueues).toBe(maxReenqueues);
for (const closer of [eventLedger, governorLedger, portfolioQueue, runState, governorState]) closer.close();

// Restart: a brand-new loop process -- fresh handles to the SAME on-disk files, with no in-memory Map to
// inherit. Pre-#5677 the fresh Map would read a zero convergence history and attempt the stuck item anyway.
const runAttemptSpy = vi.fn();
const exitCode = await runLoop(["acme/widgets", "--miner-login", "alice", "--json"], {
openGovernorState: () => openGovernorState(paths.governorStatePath),
initEventLedger: () => initEventLedger(paths.eventLedgerPath),
initGovernorLedger: () => initGovernorLedger(paths.governorLedgerPath),
initPortfolioQueue: () => initPortfolioQueueStore(paths.portfolioQueuePath),
initRunStateStore: () => initRunStateStore(paths.runStatePath),
runDiscover: vi.fn(async () => 0),
runAttempt: runAttemptSpy,
...readyLoopOptions(),
});

expect(exitCode).toBe(0);
// Halted before this run's FIRST attempt: the persisted re-enqueue streak (not a reset in-memory Map) was read
// straight away, so the stuck item is caught immediately after the restart rather than re-attempted from zero.
expect(runAttemptSpy).not.toHaveBeenCalled();
const after = reopenAfterRun(paths);
expect(after.governorLedger.readGovernorEvents({}).some((event) => event.reason === "non_convergence_detected")).toBe(true);
expect(after.portfolioQueue.listQueue()[0]).toMatchObject({ status: "queued" });
});

it("REGRESSION: runs a full cycle end to end -- claims, attempts, polls real PR disposition, records the outcome, and re-enters", async () => {
const { eventLedger, governorLedger, portfolioQueue, runState, governorState, paths } = tempStores();
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
Expand Down