From ad7dc9a5379071224e533663414b7fe389b7ff70 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 11 Jul 2026 03:50:53 -0700 Subject: [PATCH 1/2] feat(miner-governor): closed-loop discovery re-entry trigger Adds shouldReenter (packages/gittensory-engine/src/miner/loop-reentry- policy.ts) + attemptLoopReentry (packages/gittensory-miner/lib/loop- reentry.js): the final piece that turns single miner runs into an unattended repeat loop -- on a resolved outcome (merged, or rejected- and-disengaged), decide whether to re-enter discovery and, when allowed, actually dequeue the next candidate and transition run-state. Not wired into any scheduler/cron as part of this change, per the issue's own "manual owner sign-off before enabling by default in any profile" deliverable. Two independent hard ceilings, per the issue's own top-slop-at-scale- risk framing (re-entering too fast, or looping on a permanently- rejected repo): - A per-repo circuit breaker: N consecutive disengaged (closed- without-merge) pr_outcome events for a repo pauses further re-entry for that repo specifically, computed from real pr-outcome.js history via countConsecutiveDisengagements (walks backward from the most recent PR until a merged outcome breaks the streak). - A hard rate/session cap: independent of any repo's own history, a conservative ceiling (default 4/hour, 20/session) on re-entries, computed from real event-ledger history via countReentriesSince. Every call appends exactly one loop_reentry_decision event to the ledger, whether or not re-entry was allowed, with the full reason set and (when the caller supplies one) the just-completed cycle's loop-closure.js LoopClosureSummary threaded through verbatim for traceability -- auditable regardless of this function's own return value. Discovered and fixed while wiring this in: 10 other packages/ gittensory-miner/lib/*.js files (governor-kill-switch, loop-closure, pr-outcome, rejection-state-machine, and others -- none from this session) were missing from the package's own `node --check` build script, an accumulating pre-existing gap. Added all of them alongside this issue's own loop-reentry.js. Test-covered per the issue's own explicit deliverable: "merged outcome -> re-entry fires once; rejected outcome with high repeated-blocker tally -> re-entry is suppressed and the repo is paused" (both test names verbatim), plus the independent rate/session caps, fail-closed validation, and the optional loopSummary/runState threading. --- packages/gittensory-engine/src/index.ts | 9 + .../src/miner/loop-reentry-policy.ts | 70 ++++++ .../test/loop-reentry-policy.test.ts | 114 ++++++++++ .../gittensory-miner/lib/loop-reentry.d.ts | 48 ++++ packages/gittensory-miner/lib/loop-reentry.js | 121 +++++++++++ packages/gittensory-miner/package.json | 2 +- test/unit/miner-loop-reentry.test.ts | 205 ++++++++++++++++++ 7 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 packages/gittensory-engine/src/miner/loop-reentry-policy.ts create mode 100644 packages/gittensory-engine/test/loop-reentry-policy.test.ts create mode 100644 packages/gittensory-miner/lib/loop-reentry.d.ts create mode 100644 packages/gittensory-miner/lib/loop-reentry.js create mode 100644 test/unit/miner-loop-reentry.test.ts diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index e0deabcccf..62d88e5356 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -244,6 +244,15 @@ export { type SubmissionGateDecision, type SubmissionGateMode, } from "./miner/submission-gate.js"; +export { + DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS, + DEFAULT_MAX_REENTRIES_PER_HOUR, + DEFAULT_MAX_REENTRIES_PER_SESSION, + shouldReenter, + type LoopReentryCandidate, + type LoopReentryDecision, + type LoopReentryOutcome, +} from "./miner/loop-reentry-policy.js"; export { codingAgentModeExecutes, isGlobalMinerCodingAgentPause, diff --git a/packages/gittensory-engine/src/miner/loop-reentry-policy.ts b/packages/gittensory-engine/src/miner/loop-reentry-policy.ts new file mode 100644 index 0000000000..381ac416d0 --- /dev/null +++ b/packages/gittensory-engine/src/miner/loop-reentry-policy.ts @@ -0,0 +1,70 @@ +// Closed-loop discovery re-entry policy (#2338): the pure decision half of "on a resolved outcome (merged, or +// rejected-and-disengaged), automatically re-invoke discovery to select the next candidate." Deliberately split +// from the miner-side orchestrator (packages/gittensory-miner/lib/loop-reentry.js), which owns the REAL IO -- +// reading recent event-ledger history to compute the tallies this policy consumes, dequeuing the next +// candidate, and transitioning run-state -- mirroring this session's established engine (pure) / miner-lib +// (stateful) split for every other governor primitive. +// +// TOP SLOP-AT-SCALE RISK: this issue's own framing calls out "a bug here (re-entering too fast, ignoring a +// circuit-breaker, or looping on a permanently-rejected repo) is the top slop-at-scale risk for the whole miner +// subsystem." Both failure modes get an INDEPENDENT hard ceiling here, neither one masking the other: +// - A per-repo circuit breaker: N consecutive disengaged (rejected) outcomes on the SAME repo pauses further +// re-entry for that repo, regardless of how much of the hour/session rate budget remains. +// - A hard rate/session cap: independent of any repo's own history, a conservative ceiling on how many +// re-entries may fire in a rolling hour or across the whole session. +// Both reasons are collected (not short-circuited) so a caller logging the decision sees every ceiling that +// was hit, not just the first one checked. + +/** The terminal outcome that just resolved for the repo the caller is considering re-entering on. */ +export type LoopReentryOutcome = "merged" | "disengaged" | "other"; + +export const DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS = 3; +export const DEFAULT_MAX_REENTRIES_PER_HOUR = 4; +export const DEFAULT_MAX_REENTRIES_PER_SESSION = 20; + +export type LoopReentryCandidate = { + repoFullName: string; + outcome: LoopReentryOutcome; + /** Caller-computed count of CONSECUTIVE `"disengaged"` outcomes for this repo, ending with (and including, + * when `outcome === "disengaged"`) this one. Any non-disengaged outcome resets this to 0 -- the caller owns + * that computation, this policy only consumes the resulting integer (mirrors `reputation-throttle.ts`'s + * caller-supplied `RepoOutcomeHistory`). */ + consecutiveDisengagements: number; + maxConsecutiveDisengagements?: number | undefined; + /** Caller-tracked re-entry counters for the hard rate/session cap -- independent of the per-repo circuit + * breaker above. */ + reentriesThisHour: number; + maxReentriesPerHour?: number | undefined; + reentriesThisSession: number; + maxReentriesPerSession?: number | undefined; +}; + +export type LoopReentryDecision = { + reenter: boolean; + /** Always populated when `reenter` is `false`; every ceiling that was hit, not just the first. */ + reasons: string[]; +}; + +/** + * Decide whether the loop may re-enter discovery for this repo. Pure; identical inputs always yield the + * identical decision. `outcome === "merged"` alone never bypasses the rate/session cap -- a healthy repo can + * still be rate-limited if the operator-wide ceiling is already spent. + */ +export function shouldReenter(candidate: LoopReentryCandidate): LoopReentryDecision { + const reasons: string[] = []; + const maxConsecutiveDisengagements = candidate.maxConsecutiveDisengagements ?? DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS; + const maxReentriesPerHour = candidate.maxReentriesPerHour ?? DEFAULT_MAX_REENTRIES_PER_HOUR; + const maxReentriesPerSession = candidate.maxReentriesPerSession ?? DEFAULT_MAX_REENTRIES_PER_SESSION; + + if (candidate.outcome === "disengaged" && candidate.consecutiveDisengagements >= maxConsecutiveDisengagements) { + reasons.push(`repo_paused_after_consecutive_disengagements:${candidate.consecutiveDisengagements}>=${maxConsecutiveDisengagements}`); + } + if (candidate.reentriesThisHour >= maxReentriesPerHour) { + reasons.push(`hourly_reentry_cap_reached:${candidate.reentriesThisHour}>=${maxReentriesPerHour}`); + } + if (candidate.reentriesThisSession >= maxReentriesPerSession) { + reasons.push(`session_reentry_cap_reached:${candidate.reentriesThisSession}>=${maxReentriesPerSession}`); + } + + return { reenter: reasons.length === 0, reasons }; +} diff --git a/packages/gittensory-engine/test/loop-reentry-policy.test.ts b/packages/gittensory-engine/test/loop-reentry-policy.test.ts new file mode 100644 index 0000000000..af86561c18 --- /dev/null +++ b/packages/gittensory-engine/test/loop-reentry-policy.test.ts @@ -0,0 +1,114 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS, + DEFAULT_MAX_REENTRIES_PER_HOUR, + DEFAULT_MAX_REENTRIES_PER_SESSION, + shouldReenter, + type LoopReentryCandidate, +} from "../dist/index.js"; + +function baseCandidate(overrides: Partial = {}): LoopReentryCandidate { + return { + repoFullName: "acme/widgets", + outcome: "merged", + consecutiveDisengagements: 0, + reentriesThisHour: 0, + reentriesThisSession: 0, + ...overrides, + }; +} + +test("barrel: the public entrypoint re-exports the loop-reentry policy (#2338)", () => { + assert.equal(typeof shouldReenter, "function"); + assert.equal(typeof DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS, "number"); +}); + +test("a merged outcome with every counter well within limits re-enters cleanly", () => { + const decision = shouldReenter(baseCandidate({ outcome: "merged" })); + assert.deepEqual(decision, { reenter: true, reasons: [] }); +}); + +test("an 'other' outcome (neither merged nor disengaged) is never subject to the per-repo circuit breaker", () => { + const decision = shouldReenter(baseCandidate({ outcome: "other" })); + assert.deepEqual(decision, { reenter: true, reasons: [] }); +}); + +test("circuit breaker: a disengaged outcome at or beyond the consecutive-disengagement ceiling pauses the repo", () => { + const decision = shouldReenter(baseCandidate({ outcome: "disengaged", consecutiveDisengagements: 3, maxConsecutiveDisengagements: 3 })); + assert.equal(decision.reenter, false); + assert.deepEqual(decision.reasons, ["repo_paused_after_consecutive_disengagements:3>=3"]); +}); + +test("circuit breaker: a disengaged outcome below the ceiling still re-enters", () => { + const decision = shouldReenter(baseCandidate({ outcome: "disengaged", consecutiveDisengagements: 2, maxConsecutiveDisengagements: 3 })); + assert.equal(decision.reenter, true); +}); + +test("circuit breaker: a HIGH consecutiveDisengagements count never pauses a repo whose outcome ISN'T disengaged", () => { + // Exercises the && short-circuit's left-false side distinctly from the right-side threshold check -- a + // repo could have a high historical tally but just landed a merge, which must not be treated as a pause. + const decision = shouldReenter(baseCandidate({ outcome: "merged", consecutiveDisengagements: 99, maxConsecutiveDisengagements: 3 })); + assert.equal(decision.reenter, true); +}); + +test("rate cap: an hourly re-entry ceiling at or beyond the limit blocks, independent of repo history", () => { + const decision = shouldReenter(baseCandidate({ reentriesThisHour: 4, maxReentriesPerHour: 4 })); + assert.equal(decision.reenter, false); + assert.deepEqual(decision.reasons, ["hourly_reentry_cap_reached:4>=4"]); +}); + +test("rate cap: an hourly count below the limit does not block", () => { + const decision = shouldReenter(baseCandidate({ reentriesThisHour: 3, maxReentriesPerHour: 4 })); + assert.equal(decision.reenter, true); +}); + +test("rate cap: a session re-entry ceiling at or beyond the limit blocks, independent of the hourly cap", () => { + const decision = shouldReenter(baseCandidate({ reentriesThisSession: 20, maxReentriesPerSession: 20 })); + assert.equal(decision.reenter, false); + assert.deepEqual(decision.reasons, ["session_reentry_cap_reached:20>=20"]); +}); + +test("rate cap: a session count below the limit does not block", () => { + const decision = shouldReenter(baseCandidate({ reentriesThisSession: 19, maxReentriesPerSession: 20 })); + assert.equal(decision.reenter, true); +}); + +test("every ceiling that is exceeded is reported, not just the first one checked", () => { + const decision = shouldReenter( + baseCandidate({ + outcome: "disengaged", + consecutiveDisengagements: 5, + maxConsecutiveDisengagements: 3, + reentriesThisHour: 10, + maxReentriesPerHour: 4, + reentriesThisSession: 30, + maxReentriesPerSession: 20, + }), + ); + assert.equal(decision.reenter, false); + assert.equal(decision.reasons.length, 3); +}); + +test("default thresholds apply when the candidate omits its own overrides", () => { + const justUnderDefault = shouldReenter( + baseCandidate({ outcome: "disengaged", consecutiveDisengagements: DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS - 1 }), + ); + assert.equal(justUnderDefault.reenter, true); + + const atDefault = shouldReenter(baseCandidate({ outcome: "disengaged", consecutiveDisengagements: DEFAULT_MAX_CONSECUTIVE_DISENGAGEMENTS })); + assert.equal(atDefault.reenter, false); + + const atHourlyDefault = shouldReenter(baseCandidate({ reentriesThisHour: DEFAULT_MAX_REENTRIES_PER_HOUR })); + assert.equal(atHourlyDefault.reenter, false); + + const atSessionDefault = shouldReenter(baseCandidate({ reentriesThisSession: DEFAULT_MAX_REENTRIES_PER_SESSION })); + assert.equal(atSessionDefault.reenter, false); +}); + +test("a caller-supplied threshold overrides the default rather than being ignored", () => { + // A count that would pass under the DEFAULT ceiling must still block under a stricter caller override. + const decision = shouldReenter(baseCandidate({ outcome: "disengaged", consecutiveDisengagements: 1, maxConsecutiveDisengagements: 1 })); + assert.equal(decision.reenter, false); +}); diff --git a/packages/gittensory-miner/lib/loop-reentry.d.ts b/packages/gittensory-miner/lib/loop-reentry.d.ts new file mode 100644 index 0000000000..75fcda5a34 --- /dev/null +++ b/packages/gittensory-miner/lib/loop-reentry.d.ts @@ -0,0 +1,48 @@ +export const LOOP_REENTRY_DECISION_EVENT: "loop_reentry_decision"; + +export type LoopReentryOutcome = "merged" | "disengaged" | "other"; + +export type LoopReentryCandidateInput = { + repoFullName: string; + outcome: LoopReentryOutcome; + maxConsecutiveDisengagements?: number; + maxReentriesPerHour?: number; + maxReentriesPerSession?: number; +}; + +export interface LoopReentryEventLedger { + appendEvent(event: { type: string; repoFullName?: string; payload: Record }): { id: number; seq: number; type: string; repoFullName: string | null; payload: Record; createdAt: string }; + readEvents(filter?: { since?: number; repoFullName?: string }): Array<{ type: string; repoFullName?: string | null; payload?: Record; createdAt: string }>; +} + +export interface LoopReentryPortfolioQueue { + dequeueNext(): { repoFullName: string; identifier: string; priority: number; status: string; enqueuedAt: string } | null; +} + +export interface LoopReentryRunState { + setRunState(repoFullName: string, state: string): unknown; +} + +export type LoopReentryDeps = { + eventLedger: LoopReentryEventLedger; + portfolioQueue: LoopReentryPortfolioQueue; + runState?: LoopReentryRunState; + nowMs?: number; + sessionStartMs?: number; + /** The just-completed cycle's read-only summary (loop-closure.js's `buildLoopClosureSummary`), threaded + * through verbatim into the audit event's payload for traceability. Not used to compute the circuit- + * breaker/rate-cap tallies -- see loop-reentry.js's own comment on why. */ + loopSummary?: unknown; +}; + +export type LoopReentryResult = { + decision: { reenter: boolean; reasons: string[] }; + dequeued: { repoFullName: string; identifier: string; priority: number; status: string; enqueuedAt: string } | null; + event: { id: number; seq: number; type: string; repoFullName: string | null; payload: Record; createdAt: string }; +}; + +export function countConsecutiveDisengagements(eventLedger: LoopReentryEventLedger, repoFullName: string): number; + +export function countReentriesSince(eventLedger: LoopReentryEventLedger, sinceMs: number): number; + +export function attemptLoopReentry(candidate: LoopReentryCandidateInput, deps: LoopReentryDeps): LoopReentryResult; diff --git a/packages/gittensory-miner/lib/loop-reentry.js b/packages/gittensory-miner/lib/loop-reentry.js new file mode 100644 index 0000000000..4fe248a315 --- /dev/null +++ b/packages/gittensory-miner/lib/loop-reentry.js @@ -0,0 +1,121 @@ +import { shouldReenter } from "@jsonbored/gittensory-engine"; + +import { readPrOutcomes } from "./pr-outcome.js"; + +// Closed-loop discovery re-entry orchestrator (#2338): the real-IO half of "on a resolved outcome (merged, or +// rejected-and-disengaged), automatically re-invoke discovery to select the next candidate." The DECISION +// itself (shouldReenter, @jsonbored/gittensory-engine) is pure; this module owns everything that decision +// needs real state for -- reading the repo's own pr_outcome history to compute the per-repo consecutive- +// disengagement tally, reading recent re-entry events for the hourly/session rate cap, and (only when allowed) +// actually dequeuing the next candidate and transitioning run-state. +// +// NOT WIRED INTO ANY AUTOMATIC SCHEDULE: per this issue's own "manual owner sign-off before enabling by +// default in any profile" deliverable, this is a callable function ready for that sign-off -- it is not invoked +// by manage-poll.js or any cron/scheduler as part of this change. +// +// AUDITABILITY: every call appends exactly one `loop_reentry_decision` event to the ledger, whether or not the +// decision allowed re-entry, so the full decision trail (including every suppressed re-entry and why) survives +// independently of this function's own return value. + +export const LOOP_REENTRY_DECISION_EVENT = "loop_reentry_decision"; +const HOUR_MS = 60 * 60 * 1000; + +/** A `pr_outcome` "closed" decision is this module's practical proxy for "disengaged" -- pr-outcome.js's own + * vocabulary is exactly `"merged" | "closed"` (no separate "disengaged" literal); a PR that closed without + * merging IS the rejected/disengaged case rejection-state-machine.js's own `isRejectedPr` checks for. */ +function isDisengagedOutcome(outcome) { + return outcome?.decision === "closed"; +} + +/** + * Count a repo's CONSECUTIVE disengaged (closed-without-merge) PR outcomes, walking backward from the most + * recently recorded PR for that repo until a merged outcome breaks the streak (or history runs out). + */ +export function countConsecutiveDisengagements(eventLedger, repoFullName) { + const outcomes = [...readPrOutcomes(eventLedger, { repoFullName }).values()]; + let count = 0; + for (let i = outcomes.length - 1; i >= 0; i -= 1) { + if (!isDisengagedOutcome(outcomes[i])) break; + count += 1; + } + return count; +} + +/** Count prior re-entries (successful, i.e. `reentered: true`) recorded at or after `sinceMs`. */ +export function countReentriesSince(eventLedger, sinceMs) { + return eventLedger + .readEvents({}) + .filter((event) => event.type === LOOP_REENTRY_DECISION_EVENT && event.payload?.reentered === true && Date.parse(event.createdAt) >= sinceMs) + .length; +} + +/** + * 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. + * + * @param {{ repoFullName: string, outcome: "merged"|"disengaged"|"other", maxConsecutiveDisengagements?: number, maxReentriesPerHour?: number, maxReentriesPerSession?: number }} candidate + * @param {{ eventLedger: object, portfolioQueue: object, runState?: object, nowMs?: number, sessionStartMs?: number }} deps + */ +export function attemptLoopReentry(candidate, deps) { + if (!candidate || typeof candidate !== "object") throw new Error("invalid_loop_reentry_candidate"); + const repoFullName = typeof candidate.repoFullName === "string" ? candidate.repoFullName.trim() : ""; + if (!repoFullName) throw new Error("invalid_repo_full_name"); + if (!["merged", "disengaged", "other"].includes(candidate.outcome)) throw new Error("invalid_outcome"); + + if (!deps || typeof deps !== "object") throw new Error("invalid_loop_reentry_deps"); + const { eventLedger, portfolioQueue, runState, nowMs = Date.now(), sessionStartMs = 0 } = deps; + if (!eventLedger || typeof eventLedger.appendEvent !== "function" || typeof eventLedger.readEvents !== "function") { + throw new Error("invalid_event_ledger"); + } + if (!portfolioQueue || typeof portfolioQueue.dequeueNext !== "function") { + throw new Error("invalid_portfolio_queue"); + } + + const consecutiveDisengagements = countConsecutiveDisengagements(eventLedger, repoFullName); + const reentriesThisHour = countReentriesSince(eventLedger, nowMs - HOUR_MS); + const reentriesThisSession = countReentriesSince(eventLedger, sessionStartMs); + + const decision = shouldReenter({ + repoFullName, + outcome: candidate.outcome, + consecutiveDisengagements, + maxConsecutiveDisengagements: candidate.maxConsecutiveDisengagements, + reentriesThisHour, + maxReentriesPerHour: candidate.maxReentriesPerHour, + reentriesThisSession, + maxReentriesPerSession: candidate.maxReentriesPerSession, + }); + + let dequeued = null; + if (decision.reenter) { + dequeued = portfolioQueue.dequeueNext(); + if (runState && typeof runState.setRunState === "function") { + runState.setRunState(repoFullName, "discovering"); + } + } + + const event = eventLedger.appendEvent({ + type: LOOP_REENTRY_DECISION_EVENT, + repoFullName, + payload: { + outcome: candidate.outcome, + reentered: decision.reenter, + reasons: decision.reasons, + consecutiveDisengagements, + reentriesThisHour, + reentriesThisSession, + dequeuedIdentifier: dequeued ? dequeued.identifier : null, + // The just-completed cycle's read-only summary (loop-closure.js's buildLoopClosureSummary), when the + // caller supplies one -- threaded through verbatim for audit traceability. Optional: the circuit-breaker + // and rate-cap tallies above are computed directly from pr-outcome/event-ledger history (a + // LoopClosureSummary's own byType COUNTS aren't detailed enough to derive a per-repo consecutive- + // disengagement streak from), so this is context, not a computational input. + loopSummary: deps.loopSummary ?? null, + }, + }); + + return { decision, dequeued, event }; +} diff --git a/packages/gittensory-miner/package.json b/packages/gittensory-miner/package.json index 65fde4cb5b..7b36ec1f59 100644 --- a/packages/gittensory-miner/package.json +++ b/packages/gittensory-miner/package.json @@ -32,7 +32,7 @@ "expected-engine.version" ], "scripts": { - "build": "node --check bin/gittensory-miner.js && node --check lib/version.js && node --check lib/local-store.js && node --check lib/cli.js && node --check lib/deny-check.js && node --check lib/run-state-cli.js && node --check lib/update-check.js && node --check lib/worktree-allocator.js && node --check lib/opportunity-fanout.js && node --check lib/ci-poller.js && node --check lib/run-state.js && node --check lib/deny-hooks.js && node --check lib/deny-hook-synthesis.js && node --check lib/pretooluse-hook.js && node --check lib/event-ledger.js && node --check lib/event-ledger-cli.js && node --check lib/claim-ledger.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-adjudication.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-discovery.js && node --check lib/opportunity-ranker.js && node --check lib/plan-store.js && node --check lib/plan-store-cli.js && node --check lib/rejection-templates.js && node --check lib/governor-ledger.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-open-pr.js && node --check lib/governor-write-rate-limit.js && node --check lib/governor-run-halt.js && node --check lib/attempt-log.js && node --check lib/manage-status.js && node --check lib/manage-poll.js && node --check lib/status.js && node --check lib/laptop-init.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-task-generation.js && node --check lib/calibration-types.js && node --check lib/calibration.js" + "build": "node --check bin/gittensory-miner.js && node --check lib/version.js && node --check lib/local-store.js && node --check lib/cli.js && node --check lib/deny-check.js && node --check lib/run-state-cli.js && node --check lib/update-check.js && node --check lib/worktree-allocator.js && node --check lib/opportunity-fanout.js && node --check lib/ci-poller.js && node --check lib/run-state.js && node --check lib/deny-hooks.js && node --check lib/deny-hook-synthesis.js && node --check lib/pretooluse-hook.js && node --check lib/event-ledger.js && node --check lib/event-ledger-cli.js && node --check lib/claim-ledger.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-adjudication.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-discovery.js && node --check lib/opportunity-ranker.js && node --check lib/plan-store.js && node --check lib/plan-store-cli.js && node --check lib/rejection-templates.js && node --check lib/governor-ledger.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-open-pr.js && node --check lib/governor-write-rate-limit.js && node --check lib/governor-run-halt.js && node --check lib/governor-kill-switch.js && node --check lib/attempt-log.js && node --check lib/manage-status.js && node --check lib/manage-poll.js && node --check lib/status.js && node --check lib/laptop-init.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-task-generation.js && node --check lib/calibration-types.js && node --check lib/calibration.js && node --check lib/discover-cli.js && node --check lib/feasibility-cli.js && node --check lib/gate-verdict-poller.js && node --check lib/loop-closure.js && node --check lib/loop-reentry.js && node --check lib/orb-export.js && node --check lib/portfolio-dashboard.js && node --check lib/pr-outcome.js && node --check lib/prediction-ledger.js && node --check lib/rejection-state-machine.js" }, "dependencies": { "@jsonbored/gittensory-engine": "*" diff --git a/test/unit/miner-loop-reentry.test.ts b/test/unit/miner-loop-reentry.test.ts new file mode 100644 index 0000000000..d5af47f40a --- /dev/null +++ b/test/unit/miner-loop-reentry.test.ts @@ -0,0 +1,205 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@jsonbored/gittensory-engine", async () => { + return import("../../packages/gittensory-engine/src/index"); +}); + +import { attemptLoopReentry, countConsecutiveDisengagements, countReentriesSince, LOOP_REENTRY_DECISION_EVENT } from "../../packages/gittensory-miner/lib/loop-reentry.js"; +import { initEventLedger } from "../../packages/gittensory-miner/lib/event-ledger.js"; +import { initPortfolioQueueStore } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; +import { initRunStateStore } from "../../packages/gittensory-miner/lib/run-state.js"; +import { recordPrOutcomeSnapshot } from "../../packages/gittensory-miner/lib/pr-outcome.js"; + +const roots: string[] = []; +const closers: Array<{ close(): void }> = []; + +function tempPath(prefix: string) { + const root = mkdtempSync(join(tmpdir(), `gittensory-miner-${prefix}-`)); + roots.push(root); + return join(root, "db.sqlite3"); +} + +function tempEventLedger() { + const ledger = initEventLedger(tempPath("loop-reentry-events")); + closers.push(ledger); + return ledger; +} + +function tempPortfolioQueue() { + const queue = initPortfolioQueueStore(tempPath("loop-reentry-queue")); + closers.push(queue); + return queue; +} + +function tempRunState() { + const store = initRunStateStore(tempPath("loop-reentry-runstate")); + closers.push(store); + return store; +} + +afterEach(() => { + for (const closer of closers.splice(0)) closer.close(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +describe("attemptLoopReentry (#2338)", () => { + it("merged outcome: re-entry fires once, dequeuing the next candidate and transitioning run-state to discovering", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + const runState = tempRunState(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-42" }); + + const result = attemptLoopReentry( + { repoFullName: "acme/widgets", outcome: "merged" }, + { eventLedger, portfolioQueue, runState }, + ); + + expect(result.decision.reenter).toBe(true); + expect(result.decision.reasons).toEqual([]); + expect(result.dequeued?.identifier).toBe("issue-42"); + expect(runState.getRunState("acme/widgets")).toBe("discovering"); + + const events = eventLedger.readEvents({ repoFullName: "acme/widgets" }); + expect(events).toHaveLength(1); + expect(events[0]?.type).toBe(LOOP_REENTRY_DECISION_EVENT); + expect(events[0]?.payload).toMatchObject({ reentered: true, outcome: "merged", dequeuedIdentifier: "issue-42" }); + }); + + it("rejected outcome with a high repeated-blocker (consecutive disengagement) tally: re-entry is suppressed and the repo stays paused", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + const runState = tempRunState(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-99" }); + + // Seed three consecutive closed-without-merge outcomes for this repo -- at the default ceiling. + for (let prNumber = 1; prNumber <= 3; prNumber += 1) { + recordPrOutcomeSnapshot( + { repoFullName: "acme/widgets", prNumber, decision: "closed", closedAt: new Date().toISOString(), reason: "stale" }, + { eventLedger }, + ); + } + expect(countConsecutiveDisengagements(eventLedger, "acme/widgets")).toBe(3); + + const result = attemptLoopReentry( + { repoFullName: "acme/widgets", outcome: "disengaged" }, + { eventLedger, portfolioQueue, runState }, + ); + + expect(result.decision.reenter).toBe(false); + expect(result.decision.reasons).toEqual(["repo_paused_after_consecutive_disengagements:3>=3"]); + expect(result.dequeued).toBeNull(); + expect(runState.getRunState("acme/widgets")).toBeNull(); + + // The candidate remains queued -- it was never dequeued. + expect(portfolioQueue.listQueue("acme/widgets")).toHaveLength(1); + + const events = eventLedger.readEvents({ repoFullName: "acme/widgets" }); + const decisionEvents = events.filter((event) => event.type === LOOP_REENTRY_DECISION_EVENT); + expect(decisionEvents).toHaveLength(1); + expect(decisionEvents[0]?.payload).toMatchObject({ reentered: false, dequeuedIdentifier: null }); + }); + + it("a single merged outcome after a run of closed outcomes resets the consecutive-disengagement streak to zero", () => { + const eventLedger = tempEventLedger(); + for (let prNumber = 1; prNumber <= 2; prNumber += 1) { + recordPrOutcomeSnapshot({ repoFullName: "acme/widgets", prNumber, decision: "closed", closedAt: new Date().toISOString(), reason: "stale" }, { eventLedger }); + } + recordPrOutcomeSnapshot({ repoFullName: "acme/widgets", prNumber: 3, decision: "merged", closedAt: new Date().toISOString() }, { eventLedger }); + + expect(countConsecutiveDisengagements(eventLedger, "acme/widgets")).toBe(0); + }); + + it("the hourly rate cap suppresses re-entry independent of the per-repo circuit breaker, and does not move the run-state or dequeue", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + const runState = tempRunState(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-1" }); + + const now = Date.now(); + for (let i = 0; i < 4; i += 1) { + eventLedger.appendEvent({ + type: LOOP_REENTRY_DECISION_EVENT, + repoFullName: "other/repo", + payload: { reentered: true }, + }); + } + expect(countReentriesSince(eventLedger, now - 60 * 60 * 1000)).toBe(4); + + const result = attemptLoopReentry( + { repoFullName: "acme/widgets", outcome: "merged", maxReentriesPerHour: 4 }, + { eventLedger, portfolioQueue, runState, nowMs: now }, + ); + + expect(result.decision.reenter).toBe(false); + expect(result.decision.reasons).toEqual(["hourly_reentry_cap_reached:4>=4"]); + expect(runState.getRunState("acme/widgets")).toBeNull(); + expect(portfolioQueue.listQueue("acme/widgets")).toHaveLength(1); + }); + + it("the session rate cap suppresses re-entry independent of the hourly cap, and does not move the run-state or dequeue", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + const runState = tempRunState(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-1" }); + + for (let i = 0; i < 20; i += 1) { + eventLedger.appendEvent({ type: LOOP_REENTRY_DECISION_EVENT, repoFullName: "other/repo", payload: { reentered: true } }); + } + + const result = attemptLoopReentry( + { repoFullName: "acme/widgets", outcome: "merged", maxReentriesPerHour: 1_000, maxReentriesPerSession: 20 }, + { eventLedger, portfolioQueue, runState, sessionStartMs: 0 }, + ); + + expect(result.decision.reenter).toBe(false); + expect(result.decision.reasons).toEqual(["session_reentry_cap_reached:20>=20"]); + expect(runState.getRunState("acme/widgets")).toBeNull(); + }); + + it("fails closed on a malformed candidate or missing dependency rather than silently allowing", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + + expect(() => attemptLoopReentry(null as never, { eventLedger, portfolioQueue })).toThrow("invalid_loop_reentry_candidate"); + expect(() => attemptLoopReentry({ outcome: "merged" } as never, { eventLedger, portfolioQueue })).toThrow("invalid_repo_full_name"); + expect(() => attemptLoopReentry({ repoFullName: "", outcome: "merged" }, { eventLedger, portfolioQueue })).toThrow("invalid_repo_full_name"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "bogus" as never }, { eventLedger, portfolioQueue })).toThrow("invalid_outcome"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, null as never)).toThrow("invalid_loop_reentry_deps"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger })).toThrow("invalid_portfolio_queue"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { portfolioQueue })).toThrow("invalid_event_ledger"); + }); + + it("threads a caller-supplied loopSummary verbatim into the audit event payload for traceability", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-1" }); + const loopSummary = { sinceSeq: 10, lastSeq: 42, events: { total: 3, byType: { pr_outcome: 1 } }, queue: { total: 1, byStatus: { queued: 1 } }, runState: "idle" }; + + const result = attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger, portfolioQueue, loopSummary }); + + expect(result.event.payload.loopSummary).toEqual(loopSummary); + }); + + it("records a null loopSummary in the audit payload when the caller supplies none", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-1" }); + + const result = attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger, portfolioQueue }); + + expect(result.event.payload.loopSummary).toBeNull(); + }); + + it("proceeds without a runState dependency (it is optional) and without touching it", () => { + const eventLedger = tempEventLedger(); + const portfolioQueue = tempPortfolioQueue(); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue-1" }); + + const result = attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger, portfolioQueue }); + expect(result.decision.reenter).toBe(true); + expect(result.dequeued?.identifier).toBe("issue-1"); + }); +}); From 53b3f4f4496ab94a7705ea67556d5d1fd794c324 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sat, 11 Jul 2026 05:40:46 -0700 Subject: [PATCH 2/2] fix(miner-governor): fix pre-existing EventLedger#appendEvent type drift in pr-outcome.d.ts pr-outcome.d.ts's RecordPrOutcomeOptions.eventLedger hand-declared a looser { appendEvent(event: { ...payload: unknown }): unknown } shape instead of reusing EventLedger's real AppendEventInput/LedgerEntry contract, so a genuine EventLedger (from initEventLedger) never type-checked against it -- only a same-shaped stub did. This branch's own test was the first to pass a real EventLedger in, surfacing the gap. Fixes it at the source (import + reuse the real types) and updates the one other mock ledger (miner-pr-outcome.test.ts) that relied on the looser shape so it can't silently drift from the real contract again. Also restores the two `as never` casts missing from two deliberately-invalid-deps assertions in miner-loop-reentry.test.ts that were inconsistent with the identical pattern on the four lines directly above them. No behavior change -- purely type-declaration correctness; the runtime fail-closed behavior these tests assert was always correct and remains covered. --- packages/gittensory-miner/lib/pr-outcome.d.ts | 7 +++++-- test/unit/miner-loop-reentry.test.ts | 4 ++-- test/unit/miner-pr-outcome.test.ts | 6 ++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/gittensory-miner/lib/pr-outcome.d.ts b/packages/gittensory-miner/lib/pr-outcome.d.ts index 551a3abd86..4fe170faae 100644 --- a/packages/gittensory-miner/lib/pr-outcome.d.ts +++ b/packages/gittensory-miner/lib/pr-outcome.d.ts @@ -1,3 +1,5 @@ +import type { AppendEventInput, LedgerEntry } from "./event-ledger.js"; + export const MINER_PR_OUTCOME_EVENT: "pr_outcome"; export const MINER_PR_OUTCOME_DECISIONS: readonly ["merged", "closed"]; @@ -20,8 +22,9 @@ export interface PrOutcomeInput { export interface RecordPrOutcomeOptions { /** Optional at the type level so a caller can pass an unusable ledger to exercise the fail-closed guard; the - * writer throws `invalid_event_ledger` at runtime when this is absent or lacks `appendEvent`. */ - eventLedger?: { appendEvent(event: { type: string; repoFullName: string; payload: unknown }): unknown }; + * writer throws `invalid_event_ledger` at runtime when this is absent or lacks `appendEvent`. Reuses the + * real EventLedger#appendEvent signature so a genuine EventLedger (not just a same-shaped stub) type-checks. */ + eventLedger?: { appendEvent(event: AppendEventInput): LedgerEntry }; } export interface PrOutcomeLedgerReader { diff --git a/test/unit/miner-loop-reentry.test.ts b/test/unit/miner-loop-reentry.test.ts index d5af47f40a..7f3a63c0b6 100644 --- a/test/unit/miner-loop-reentry.test.ts +++ b/test/unit/miner-loop-reentry.test.ts @@ -168,8 +168,8 @@ describe("attemptLoopReentry (#2338)", () => { expect(() => attemptLoopReentry({ repoFullName: "", outcome: "merged" }, { eventLedger, portfolioQueue })).toThrow("invalid_repo_full_name"); expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "bogus" as never }, { eventLedger, portfolioQueue })).toThrow("invalid_outcome"); expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, null as never)).toThrow("invalid_loop_reentry_deps"); - expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger })).toThrow("invalid_portfolio_queue"); - expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { portfolioQueue })).toThrow("invalid_event_ledger"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { eventLedger } as never)).toThrow("invalid_portfolio_queue"); + expect(() => attemptLoopReentry({ repoFullName: "acme/widgets", outcome: "merged" }, { portfolioQueue } as never)).toThrow("invalid_event_ledger"); }); it("threads a caller-supplied loopSummary verbatim into the audit event payload for traceability", () => { diff --git a/test/unit/miner-pr-outcome.test.ts b/test/unit/miner-pr-outcome.test.ts index 848c19decc..067882de02 100644 --- a/test/unit/miner-pr-outcome.test.ts +++ b/test/unit/miner-pr-outcome.test.ts @@ -6,15 +6,17 @@ import { readPrOutcomes, recordPrOutcomeSnapshot, } from "../../packages/gittensory-miner/lib/pr-outcome.js"; +import type { AppendEventInput, LedgerEntry } from "../../packages/gittensory-miner/lib/event-ledger.js"; // A minimal injected event ledger (the DI shape the writer/reader accept), so these stay pure unit tests with no // SQLite file. `_events` is exposed so a test can inject crafted rows for the reader's defensive skip branches. -function mockLedger(): { appendEvent: (e: unknown) => unknown; readEvents: (filter?: { repoFullName?: string }) => unknown[]; _events: Array> } { +// Typed against the real EventLedger#appendEvent contract so this mock can't silently drift from it. +function mockLedger(): { appendEvent: (e: AppendEventInput) => LedgerEntry; readEvents: (filter?: { repoFullName?: string }) => unknown[]; _events: Array> } { const events: Array> = []; let seq = 0; return { appendEvent: (e) => { - const entry = { ...(e as object), seq: ++seq } as Record; + const entry = { id: ++seq, seq, type: e.type, repoFullName: e.repoFullName ?? null, payload: e.payload, createdAt: new Date().toISOString() }; events.push(entry); return entry; },