From 2c0b37a5d559c333ee1db5eceff6ba0af9f236fe Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:57:06 +0000 Subject: [PATCH] fix(miner): re-entry must transition the dequeued repo's run-state, and no-op on an empty queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit attemptLoopReentry flipped run-state to "discovering" for the just-finished repoFullName, but the portfolio queue is global so the dequeued candidate is frequently a different repo — and it did so even when the queue was empty, parking a finished repo at "discovering" forever. Gate the transition on a non-null dequeue and write dequeued.repoFullName instead. --- packages/loopover-miner/lib/loop-reentry.ts | 12 +++-- test/unit/miner-loop-reentry.test.ts | 52 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/packages/loopover-miner/lib/loop-reentry.ts b/packages/loopover-miner/lib/loop-reentry.ts index 80b997f8f0..88df7c3b9c 100644 --- a/packages/loopover-miner/lib/loop-reentry.ts +++ b/packages/loopover-miner/lib/loop-reentry.ts @@ -138,9 +138,11 @@ export function countReentriesSince(eventLedger: LoopReentryEventLedger, sinceMs /** * Evaluate and (if allowed) PERFORM re-entry for one resolved outcome: reads real history to compute the * circuit-breaker and rate-cap tallies, consults the pure `shouldReenter` policy, and -- only when it allows -- - * dequeues the next candidate and transitions run-state to `"discovering"`. Always appends exactly one audit - * event. Fails closed (throws) on a malformed candidate or missing required dependency, mirroring - * `recordManagePollSnapshot`'s own validation style. + * dequeues the next candidate and transitions THAT candidate's run-state (`dequeued.repoFullName`, the repo about + * to be worked -- NOT the just-finished `repoFullName`) to `"discovering"`. When the queue is empty + * (`dequeued === null`) no run-state transition happens, so a finished repo is never parked at `"discovering"`. + * Always appends exactly one audit event. Fails closed (throws) on a malformed candidate or missing required + * dependency, mirroring `recordManagePollSnapshot`'s own validation style. */ export function attemptLoopReentry( candidate: LoopReentryCandidateInput, @@ -182,8 +184,8 @@ export function attemptLoopReentry( let dequeued: LoopReentryResult["dequeued"] = null; if (decision.reenter) { dequeued = portfolioQueue.dequeueNext(); - if (runState && typeof runState.setRunState === "function") { - runState.setRunState(repoFullName, "discovering"); + if (dequeued && runState && typeof runState.setRunState === "function") { + runState.setRunState(dequeued.repoFullName, "discovering"); } } diff --git a/test/unit/miner-loop-reentry.test.ts b/test/unit/miner-loop-reentry.test.ts index 141db2137e..4718281c85 100644 --- a/test/unit/miner-loop-reentry.test.ts +++ b/test/unit/miner-loop-reentry.test.ts @@ -234,4 +234,56 @@ describe("attemptLoopReentry (#2338)", () => { expect(result.decision.reenter).toBe(true); expect(result.dequeued?.identifier).toBe("issue-1"); }); + + it("re-entry transitions the DEQUEUED repo's run-state, not the just-finished one (#9683): dequeues b/b while a/a finished, so setRunState hits b/b and never a/a", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + // The portfolio queue is deliberately global: the next candidate is frequently for a different repo than the + // one that just finished. Only b/b is queued, so dequeueNext returns b/b even though a/a is the finished repo. + portfolioQueue.enqueue({ repoFullName: "b/b", identifier: "issue-b" }); + const setRunState = vi.fn(); + + const result = attemptLoopReentry( + { killSwitchScope: "none", repoFullName: "a/a", outcome: "merged" }, + { eventLedger, portfolioQueue, runState: { setRunState } }, + ); + + expect(result.decision.reenter).toBe(true); + expect(result.dequeued?.repoFullName).toBe("b/b"); + expect(setRunState).toHaveBeenCalledTimes(1); + expect(setRunState).toHaveBeenCalledWith("b/b", "discovering"); + expect(setRunState).not.toHaveBeenCalledWith("a/a", expect.anything()); + }); + + it("re-entry with an empty queue (#9683): dequeueNext returns null, so setRunState is never called and the finished repo is not parked at discovering", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + // Nothing enqueued: dequeueNext() returns null even though decision.reenter is true. + const setRunState = vi.fn(); + + const result = attemptLoopReentry( + { killSwitchScope: "none", repoFullName: "a/a", outcome: "merged" }, + { eventLedger, portfolioQueue, runState: { setRunState } }, + ); + + expect(result.decision.reenter).toBe(true); + expect(result.dequeued).toBeNull(); + expect(setRunState).not.toHaveBeenCalled(); + }); + + it("suppressed re-entry (#9683): when decision.reenter is false, setRunState is never called (unchanged behaviour)", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + portfolioQueue.enqueue({ repoFullName: "b/b", identifier: "issue-b" }); + const setRunState = vi.fn(); + + const result = attemptLoopReentry( + { killSwitchScope: "global", repoFullName: "a/a", outcome: "merged" }, + { eventLedger, portfolioQueue, runState: { setRunState } }, + ); + + expect(result.decision.reenter).toBe(false); + expect(result.dequeued).toBeNull(); + expect(setRunState).not.toHaveBeenCalled(); + }); });