From 3986cb96b5be6755840496fc17c68e372b17ef3b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 11:59:00 -0700 Subject: [PATCH] feat(miner): wire per-repo kill switch, real claim-ledger, and CI-status observation into the real attempt/loop pipeline Closes #5392, Closes #5393, Closes #5394 - attempt-cli.js now resolves the real MinerGoalSpec from the already-cloned worktree and threads killSwitch.paused into both checkMinerKillSwitch and the governor context, closing the per-repo pause gap attempt-input-builder.js's own header had documented since #5132. - attempt-cli.js now records a real soft-claim via claim-ledger.js's claimIssue once an attempt passes feasibility, releasing it on every terminal outcome -- the ledger was previously never written to in the real pipeline, so freshness/dedup checks against it were always no-ops. - loop-cli.js polls real GitHub check-run status (ci-poller.js) for every submitted PR before the disposition poll, recording a ci_status_observed event and surfacing ciConclusion in --json cycle output. gate-verdict-poller.js (#4273) was the originally preferred source but has no real caller-reachable endpoint today (documented in its own header) -- ci-poller.js is the issue's own documented fallback. --- .../gittensory-miner/lib/attempt-cli.d.ts | 2 + packages/gittensory-miner/lib/attempt-cli.js | 31 ++- .../lib/attempt-input-builder.d.ts | 1 + .../lib/attempt-input-builder.js | 14 +- .../lib/gate-verdict-poller.js | 13 ++ packages/gittensory-miner/lib/loop-cli.d.ts | 4 + packages/gittensory-miner/lib/loop-cli.js | 28 ++- test/unit/miner-attempt-cli.test.ts | 211 +++++++++++++++++- test/unit/miner-attempt-input-builder.test.ts | 7 +- test/unit/miner-loop-cli.test.ts | 107 ++++++++- 10 files changed, 400 insertions(+), 18 deletions(-) diff --git a/packages/gittensory-miner/lib/attempt-cli.d.ts b/packages/gittensory-miner/lib/attempt-cli.d.ts index 5e0002aa5f..e348813ed8 100644 --- a/packages/gittensory-miner/lib/attempt-cli.d.ts +++ b/packages/gittensory-miner/lib/attempt-cli.d.ts @@ -11,6 +11,7 @@ import type { cleanupAttemptWorktree, prepareAttemptWorktree } from "./attempt-w import type { buildCodingTaskSpec } from "./coding-task-spec.js"; import type { resolveAmsPolicy } from "./ams-policy.js"; import type { checkMinerKillSwitch } from "./governor-kill-switch.js"; +import type { resolveMinerGoalSpec } from "./miner-goal-spec.js"; type CommonAttemptResultFields = { repoFullName: string; @@ -76,6 +77,7 @@ export type RunAttemptOptions = { buildCodingTaskSpec?: typeof buildCodingTaskSpec; resolveAmsPolicy?: typeof resolveAmsPolicy; checkMinerKillSwitch?: typeof checkMinerKillSwitch; + resolveMinerGoalSpec?: typeof resolveMinerGoalSpec; runMinerAttempt?: typeof runMinerAttempt; /** 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. */ diff --git a/packages/gittensory-miner/lib/attempt-cli.js b/packages/gittensory-miner/lib/attempt-cli.js index 2dfabd76cf..1a77f6240a 100644 --- a/packages/gittensory-miner/lib/attempt-cli.js +++ b/packages/gittensory-miner/lib/attempt-cli.js @@ -7,8 +7,6 @@ // runs, not just checks-and-reports-blocked. // // KNOWN, DOCUMENTED GAPS (not fabricated -- see attempt-input-builder.js's own header for the full list): -// governor.killSwitchRepoPaused only checks the GLOBAL env-var kill switch, not yet a real per-repo -// `.gittensory-miner.yml` pause (the resolver exists, miner-goal-spec.js/#5255, not wired in HERE yet); and // 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). @@ -18,6 +16,7 @@ import { runSlopAssessment } from "./slop-assessment.js"; import { fetchLiveIssueSnapshot } from "./live-issue-snapshot.js"; import { executeLocalWrite } from "./execute-local-write.js"; import { openClaimLedger } from "./claim-ledger.js"; +import { resolveMinerGoalSpec } from "./miner-goal-spec.js"; import { initEventLedger } from "./event-ledger.js"; import { initAttemptLog } from "./attempt-log.js"; import { initGovernorLedger } from "./governor-ledger.js"; @@ -131,7 +130,7 @@ export function buildAttemptDeps(env, ledgers) { * SelfReviewContext -> build a real coding-task spec (blocks on an infeasible verdict) -> resolve the real * AmsPolicySpec execution policy -> assemble the real IterateLoopInput + Governor context -> call * runMinerAttempt for real. The worktree is cleaned up (or retained, per the real outcome) in `finally`. - * See this file's header for the documented gaps (per-repo kill-switch pause, real convergence history). + * See this file's header for the documented gaps (real convergence history). */ export async function runAttempt(args, options = {}) { const parsed = parseAttemptArgs(args); @@ -161,6 +160,7 @@ export async function runAttempt(args, options = {}) { let governorLedger = null; let allocation = null; let worktreeResult = null; + let claimedIssue = false; try { allocator = (options.openWorktreeAllocator ?? openWorktreeAllocator)(); @@ -332,8 +332,18 @@ export async function runAttempt(args, options = {}) { } const amsPolicy = await (options.resolveAmsPolicy ?? resolveAmsPolicy)(parsed.repoFullName, { env }); + + // Real per-repo pause (#5392): read straight from the already-cloned worktree's own .gittensory-miner.yml + // (resolveMinerGoalSpec never throws -- a missing/malformed file degrades to killSwitch.paused: false, so + // this can't fail this attempt on its own). Threaded into BOTH checkMinerKillSwitch (killSwitchScope, used + // by the freshness/submission gate) and the governor context (killSwitchRepoPaused, used by the Governor + // chokepoint) -- the same two places the GLOBAL kill switch already reaches. + const resolveGoalSpec = options.resolveMinerGoalSpec ?? resolveMinerGoalSpec; + const minerGoalSpec = resolveGoalSpec(worktreeResult.repoPath); + const repoPaused = minerGoalSpec.spec.killSwitch.paused; + const checkKillSwitch = options.checkMinerKillSwitch ?? checkMinerKillSwitch; - const killSwitchScope = checkKillSwitch({ env }).scope; + const killSwitchScope = checkKillSwitch({ env, repoPaused }).scope; const loopInput = buildAttemptLoopInput({ codingTaskSpec, @@ -347,7 +357,14 @@ export async function runAttempt(args, options = {}) { amsPolicySpec: amsPolicy.spec, branchRef: worktreeResult.branchName, }); - const governor = buildAttemptGovernorContext(env, amsPolicy.spec); + const governor = buildAttemptGovernorContext(env, amsPolicy.spec, repoPaused); + + // 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 + // attempt is in flight. Released in `finally` on every terminal outcome -- mirrors the worktree + // allocation slot's own acquire-then-always-release pattern below. + claimLedger.claimIssue(parsed.repoFullName, parsed.issueNumber, `attempt:${attemptId}`); + claimedIssue = true; const runAttemptPipeline = options.runMinerAttempt ?? runMinerAttempt; const result = await runAttemptPipeline( @@ -422,6 +439,10 @@ export async function runAttempt(args, options = {}) { const cleanupWorktree = options.cleanupAttemptWorktree ?? cleanupAttemptWorktree; await cleanupWorktree(worktreeResult.repoPath, worktreeResult.worktreePath, worktreeResult.attemptOk ?? true); } + // Every terminal outcome past the claim point (submitted/abandon/stale/blocked/governed, or an + // unexpected throw) releases the soft-claim -- a claim that outlives its own attempt process would + // wrongly tell a sibling miner this issue is still in flight. + if (claimedIssue && claimLedger) claimLedger.releaseClaim(parsed.repoFullName, parsed.issueNumber); if (allocation && allocator) allocator.release(attemptId); allocator?.close(); claimLedger?.close(); diff --git a/packages/gittensory-miner/lib/attempt-input-builder.d.ts b/packages/gittensory-miner/lib/attempt-input-builder.d.ts index 2c0d5b1d74..58ac4ae4d1 100644 --- a/packages/gittensory-miner/lib/attempt-input-builder.d.ts +++ b/packages/gittensory-miner/lib/attempt-input-builder.d.ts @@ -5,6 +5,7 @@ import type { CodingTaskSpecResult } from "./coding-task-spec.js"; export function buildAttemptGovernorContext( env: Record, amsPolicySpec: AmsPolicySpec, + repoPaused?: boolean, ): 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 f6d6edaa3d..58fd1d2225 100644 --- a/packages/gittensory-miner/lib/attempt-input-builder.js +++ b/packages/gittensory-miner/lib/attempt-input-builder.js @@ -6,11 +6,6 @@ import { isGlobalMinerKillSwitch, isGlobalMinerLiveModeOptIn } from "@jsonbored/ // same discipline as coding-task-spec.js's own composers. // // KNOWN, DOCUMENTED GAPS (not fabricated -- explicitly left as real, narrow follow-ups): -// - governor.killSwitchRepoPaused is omitted (undefined). The real resolver exists (miner-goal-spec.js's -// resolveMinerGoalSpec, #5255) but isn't wired in HERE yet -- this composer only takes `env`, not a -// `repoPath` to read a real .gittensory-miner.yml from. Only the GLOBAL kill switch (env var) is checked -// until that follow-up lands; a per-repo pause silently can't be detected yet (fails open on that one -// axis only, matching checkMinerKillSwitch's own documented fallback for an omitted repoPaused). // - 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 @@ -27,14 +22,19 @@ import { isGlobalMinerKillSwitch, isGlobalMinerLiveModeOptIn } from "@jsonbored/ * capUsage are deliberately omitted -- evaluateGovernorChokepointGatePersisted (#5134) auto-loads them from * the persisted governor-state store when absent. * + * `repoPaused` (#5392) is the caller's own resolved `MinerGoalSpec.killSwitch.paused` for the target repo + * (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. + * * @param {Record} env * @param {import("@jsonbored/gittensory-engine").AmsPolicySpec} amsPolicySpec + * @param {boolean} [repoPaused] * @returns {import("./attempt-runner.js").AttemptGovernorContext} */ -export function buildAttemptGovernorContext(env, amsPolicySpec) { +export function buildAttemptGovernorContext(env, amsPolicySpec, repoPaused) { return { killSwitchGlobal: isGlobalMinerKillSwitch(env), - killSwitchRepoPaused: undefined, + killSwitchRepoPaused: repoPaused, liveModeGlobalOptIn: isGlobalMinerLiveModeOptIn(env), capLimits: amsPolicySpec.capLimits, convergenceInput: { attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }, diff --git a/packages/gittensory-miner/lib/gate-verdict-poller.js b/packages/gittensory-miner/lib/gate-verdict-poller.js index ac10364142..6297a71aa2 100644 --- a/packages/gittensory-miner/lib/gate-verdict-poller.js +++ b/packages/gittensory-miner/lib/gate-verdict-poller.js @@ -10,6 +10,19 @@ // gate verdict are two different signals a caller can record independently. // // Fully testable via injected `fetchFn`/`sleepFn` (mirrors `ci-poller.js`) — no real network in tests. +// +// UNWIRED (#5394 investigation): no production caller exists anywhere in this package, and the endpoint this +// module was built to poll doesn't have a real match today. The only real route serving a contributor their +// own open-PR state is GET /v1/contributors/:login/open-pr-monitor (src/api/routes.ts, backed by +// buildContributorOpenPrMonitor, src/signals/contributor-open-pr-monitor.ts) — but its response shape is a +// LIST of `{ repoFullName, number, classification: OpenPrWorkClassification, ... }` packets across every open +// PR for that login, not the single decided `{ disposition | gateDisposition | verdict }` field this module's +// own `readGateDisposition` expects for ONE targeted PR. `loop-cli.js`'s real CI/gate-status observation +// (#5394) uses `ci-poller.js`'s real GitHub check-run polling instead — the documented fallback for exactly +// this case. Wiring this module for real needs either a new single-PR gate-decision route or a rewrite of +// `readGateDisposition`/`mapGateDisposition` against `open-pr-monitor`'s real `classification` vocabulary — +// deliberately left as a separate follow-up rather than guessed at here. + import { fetchWithRetry } from "./http-retry.js"; /** The typed gate verdicts, decided ones first, `pending` (not-yet-decided) last. */ diff --git a/packages/gittensory-miner/lib/loop-cli.d.ts b/packages/gittensory-miner/lib/loop-cli.d.ts index 18f748355d..d218756182 100644 --- a/packages/gittensory-miner/lib/loop-cli.d.ts +++ b/packages/gittensory-miner/lib/loop-cli.d.ts @@ -5,6 +5,7 @@ import type { EventLedger } from "./event-ledger.js"; import type { GovernorLedger } from "./governor-ledger.js"; import type { RunStateStore } from "./run-state.js"; import type { PollPrDispositionOptions } from "./pr-disposition-poller.js"; +import type { CheckRunConclusion, PollCheckRunsOptions } from "./ci-poller.js"; export type ParsedLoopArgs = | { error: string } @@ -30,6 +31,7 @@ export type LoopCycleSummary = { attemptOutcome?: AttemptCliResult["outcome"] | "attempt_error"; reentryOutcome?: "merged" | "disengaged" | "other"; prNumber?: number | null; + ciConclusion?: CheckRunConclusion | null; reentered?: boolean; reasons?: string[]; }; @@ -51,11 +53,13 @@ export type RunLoopOptions = { checkMinerKillSwitch?: (input?: { env?: Record; repoPaused?: boolean }) => { scope: "global" | "repo" | "none"; active: boolean }; evaluateRunLoopBoundaryGate?: (input: unknown, options?: unknown) => { verdict: { reason: string }; canClaimNext: boolean }; pollPrDisposition?: (repoFullName: string, prNumber: number, options?: PollPrDispositionOptions) => Promise<{ state: "open" | "closed"; merged: boolean; closedAt: string | null; attempts: number }>; + pollCheckRuns?: (repoFullName: string, prNumber: number, options?: PollCheckRunsOptions) => Promise<{ conclusion: CheckRunConclusion; checks: unknown[]; headSha: string; attempts: number }>; recordPrOutcomeSnapshot?: (input: unknown, options?: unknown) => unknown; buildLoopClosureSummary?: (sources: unknown, options?: unknown) => { sinceSeq: number | null; lastSeq: number }; attemptLoopReentry?: (candidate: unknown, deps: unknown) => { decision: { reenter: boolean; reasons: string[] }; dequeued: { repoFullName: string; identifier: string; priority: number; status: string; enqueuedAt: string } | null }; attemptOptions?: Record; prDispositionOptions?: PollPrDispositionOptions; + ciPollOptions?: PollCheckRunsOptions; }; export function runLoop(args: string[], options?: RunLoopOptions): Promise; diff --git a/packages/gittensory-miner/lib/loop-cli.js b/packages/gittensory-miner/lib/loop-cli.js index 80b258c57b..8d9fdcc418 100644 --- a/packages/gittensory-miner/lib/loop-cli.js +++ b/packages/gittensory-miner/lib/loop-cli.js @@ -4,8 +4,9 @@ // existed; this is the first caller that actually chains them into a real repeat-until-halted run. // // STRUCTURE (one cycle): kill-switch check -> real-per-repo-policy-aware run-loop boundary gate (before -// claiming) -> real runAttempt -> real PR-disposition poll (pr-disposition-poller.js, on a submitted outcome) -// -> real loop-closure summary -> real attemptLoopReentry decision. `attemptLoopReentry`'s own dequeue is the +// claiming) -> real runAttempt -> real CI-status poll (ci-poller.js, #5394) + real PR-disposition poll +// (pr-disposition-poller.js, on a submitted outcome) -> real loop-closure summary -> real attemptLoopReentry +// decision. `attemptLoopReentry`'s own dequeue is the // AUTHORITATIVE claim for every cycle after the first (its own doc: "if allowed -- dequeues the next // candidate") -- this loop does not ALSO call portfolioQueue.dequeueNext() on a successful reentry, which // would silently double-claim (the reentry's own claim would then leak as a permanently 'in_progress', never- @@ -35,6 +36,7 @@ import { runDiscover } from "./discover-cli.js"; import { runAttempt } from "./attempt-cli.js"; import { resolveAmsPolicy } from "./ams-policy.js"; import { pollPrDisposition, classifyPrDisposition } from "./pr-disposition-poller.js"; +import { pollCheckRuns } from "./ci-poller.js"; import { recordPrOutcomeSnapshot } from "./pr-outcome.js"; import { buildLoopClosureSummary } from "./loop-closure.js"; import { attemptLoopReentry } from "./loop-reentry.js"; @@ -193,11 +195,13 @@ function zeroConvergence() { * checkMinerKillSwitch?: typeof checkMinerKillSwitch, * evaluateRunLoopBoundaryGate?: typeof evaluateRunLoopBoundaryGate, * pollPrDisposition?: typeof pollPrDisposition, + * pollCheckRuns?: typeof pollCheckRuns, * recordPrOutcomeSnapshot?: typeof recordPrOutcomeSnapshot, * buildLoopClosureSummary?: typeof buildLoopClosureSummary, * attemptLoopReentry?: typeof attemptLoopReentry, * attemptOptions?: Record, * prDispositionOptions?: Record, + * ciPollOptions?: Record, * }} [options] * @returns {Promise} */ @@ -234,6 +238,7 @@ export async function runLoop(args, options = {}) { const checkKillSwitchFn = options.checkMinerKillSwitch ?? checkMinerKillSwitch; const evaluateBoundaryGateFn = options.evaluateRunLoopBoundaryGate ?? evaluateRunLoopBoundaryGate; const pollPrDispositionFn = options.pollPrDisposition ?? pollPrDisposition; + const pollCheckRunsFn = options.pollCheckRuns ?? pollCheckRuns; const recordPrOutcomeSnapshotFn = options.recordPrOutcomeSnapshot ?? recordPrOutcomeSnapshot; const buildLoopClosureSummaryFn = options.buildLoopClosureSummary ?? buildLoopClosureSummary; const attemptLoopReentryFn = options.attemptLoopReentry ?? attemptLoopReentry; @@ -385,9 +390,27 @@ export async function runLoop(args, options = {}) { let reentryOutcome = "other"; let prNumber = null; let prDisposition = null; + let ciConclusion = null; if (submitted) { prNumber = parsePrNumberFromExecResult(lastResult?.execResult, claimed.repoFullName); if (prNumber !== null) { + // Real CI-status observation (#5394): recorded BEFORE the disposition poll below, so a submitted + // PR's check-run state is captured even while it's still open, not just at its eventual merge/close. + // gate-verdict-poller.js (#4273) was the originally preferred source for this signal but has no real + // caller-reachable endpoint today (see its own header) -- ci-poller.js's real GitHub check-run + // polling is the documented fallback for exactly this case. + const ciStatus = await pollCheckRunsFn(claimed.repoFullName, prNumber, { + githubToken, + apiBaseUrl: options.apiBaseUrl, + ...(options.ciPollOptions ?? {}), + }); + ciConclusion = ciStatus.conclusion; + eventLedger.appendEvent({ + type: "ci_status_observed", + repoFullName: claimed.repoFullName, + payload: { prNumber, conclusion: ciStatus.conclusion, checkCount: ciStatus.checks.length, source: "ci-poller" }, + }); + prDisposition = await pollPrDispositionFn(claimed.repoFullName, prNumber, { githubToken, apiBaseUrl: options.apiBaseUrl, @@ -427,6 +450,7 @@ export async function runLoop(args, options = {}) { attemptOutcome, reentryOutcome, prNumber, + ciConclusion, reentered: reentry.decision.reenter, reasons: reentry.decision.reasons, }); diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index 0df8a00dda..25cc8fbd74 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -14,7 +14,7 @@ import { closeDefaultGovernorLedger, initGovernorLedger } from "../../packages/g import { closeDefaultWorktreeAllocator, openWorktreeAllocator } from "../../packages/gittensory-miner/lib/worktree-allocator.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, parseFocusManifest } from "../../packages/gittensory-engine/src/index"; +import { DEFAULT_AMS_POLICY_SPEC, DEFAULT_MINER_GOAL_SPEC, parseFocusManifest } from "../../packages/gittensory-engine/src/index"; const roots: string[] = []; // Only ever holds ledgers a test itself must close -- runAttempt tests inject theirs via DI and runAttempt's @@ -63,6 +63,7 @@ function readyPipelineOptions(overrides: Record = {}) { buildCodingTaskSpec: () => fakeCodingTaskSpec(), 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: [] }), ...overrides, }; } @@ -701,3 +702,209 @@ describe("runAttempt (#5132)", () => { expect(onResult).toHaveBeenLastCalledWith(expect.objectContaining({ outcome: "attempt_submitted", spec: expect.objectContaining({ command: "gh pr create" }) })); }); }); + +describe("runAttempt: real per-repo kill switch (#5392)", () => { + it("resolves the real MinerGoalSpec from the worktree's repoPath and threads killSwitch.paused through to checkMinerKillSwitch and the governor context", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const worktreeResult = fakeWorktreeResult(); + const resolveMinerGoalSpecSpy = vi.fn().mockReturnValue({ present: true, spec: { ...DEFAULT_MINER_GOAL_SPEC, killSwitch: { paused: true } }, warnings: [] }); + const checkMinerKillSwitchSpy = vi.fn().mockReturnValue({ scope: "repo" as const, active: true }); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ outcome: "governed", decision: { allowed: false }, loopResult: {} }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ + resolveMinerGoalSpec: resolveMinerGoalSpecSpy, + checkMinerKillSwitch: checkMinerKillSwitchSpy, + runMinerAttempt: runMinerAttemptSpy, + }), + }); + + expect(resolveMinerGoalSpecSpy).toHaveBeenCalledWith(worktreeResult.repoPath); + expect(checkMinerKillSwitchSpy).toHaveBeenCalledWith({ env: { MINER_CODING_AGENT_PROVIDER: "noop" }, repoPaused: true }); + const [input] = runMinerAttemptSpy.mock.calls[0]!; + expect(input.killSwitchScope).toBe("repo"); + expect(input.governor.killSwitchRepoPaused).toBe(true); + }); + + it("REGRESSION: reads a real .gittensory-miner.yml killSwitch.paused:true from the worktree's real repoPath, end to end", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const repoRoot = mkdtempSync(join(tmpdir(), "gittensory-miner-attempt-cli-repo-")); + roots.push(repoRoot); + writeFileSync(join(repoRoot, ".gittensory-miner.yml"), "killSwitch:\n paused: true\n"); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ outcome: "governed", decision: { allowed: false }, loopResult: {} }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ + resolveMinerGoalSpec: undefined, // use the real, non-injected resolver against the real repoRoot below + checkMinerKillSwitch: undefined, // use the real resolver too, so it actually reacts to repoPaused + prepareAttemptWorktree: async () => ({ ok: true, worktreePath: repoRoot, repoPath: repoRoot, branchName: "gittensory/attempt/real" }), + runMinerAttempt: runMinerAttemptSpy, + }), + }); + + const [input] = runMinerAttemptSpy.mock.calls[0]!; + expect(input.killSwitchScope).toBe("repo"); + expect(input.governor.killSwitchRepoPaused).toBe(true); + }); + + it("does not gate on a repo pause when no .gittensory-miner.yml exists (real resolver, real empty dir)", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const repoRoot = mkdtempSync(join(tmpdir(), "gittensory-miner-attempt-cli-repo-")); + roots.push(repoRoot); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ outcome: "abandon", loopResult: {} }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ + resolveMinerGoalSpec: undefined, + checkMinerKillSwitch: undefined, + prepareAttemptWorktree: async () => ({ ok: true, worktreePath: repoRoot, repoPath: repoRoot, branchName: "gittensory/attempt/real" }), + runMinerAttempt: runMinerAttemptSpy, + }), + }); + + const [input] = runMinerAttemptSpy.mock.calls[0]!; + expect(input.killSwitchScope).toBe("none"); + expect(input.governor.killSwitchRepoPaused).toBe(false); + }); +}); + +describe("runAttempt: real claim-ledger wiring (#5393)", () => { + it("REGRESSION: claims the real issue before invoking runMinerAttempt, and releases it once the attempt finishes", async () => { + // claimLedger is closed in runAttempt's own `finally` block once it returns (matching the file's own + // "runAttempt tests inject theirs via DI" convention above) -- so the released-after state is asserted via + // a spy recorded DURING the call, not by re-querying the ledger once it's already closed. + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const releaseClaimSpy = vi.spyOn(claimLedger, "releaseClaim"); + let activeClaimsDuringAttempt: unknown[] = []; + const runMinerAttemptSpy = vi.fn().mockImplementation(async () => { + activeClaimsDuringAttempt = claimLedger.listActiveClaims("acme/widgets"); + return { + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1 }, + execResult: { code: 0 }, + loopResult: {}, + }; + }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }), + }); + + // Active (visible to a sibling miner process) while the real attempt was running... + expect(activeClaimsDuringAttempt).toHaveLength(1); + expect(activeClaimsDuringAttempt[0]).toMatchObject({ repoFullName: "acme/widgets", issueNumber: 7, status: "active" }); + // ...and released once the attempt concluded. + expect(releaseClaimSpy).toHaveBeenCalledWith("acme/widgets", 7); + }); + + it("releases the real claim even on a non-submitted terminal outcome", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const releaseClaimSpy = vi.spyOn(claimLedger, "releaseClaim"); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ runMinerAttempt: async () => ({ outcome: "abandon", loopResult: {} }) }), + }); + + expect(releaseClaimSpy).toHaveBeenCalledWith("acme/widgets", 7); + }); + + it("REGRESSION: releases the real claim even when runMinerAttempt throws unexpectedly", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + const releaseClaimSpy = vi.spyOn(claimLedger, "releaseClaim"); + + const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ + runMinerAttempt: async () => { + throw new Error("boom"); + }, + }), + }); + + expect(exitCode).toBe(2); + expect(releaseClaimSpy).toHaveBeenCalledWith("acme/widgets", 7); + }); + + it("never claims when the attempt is blocked before feasibility is even checked (rejection-signaled)", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "error").mockImplementation(() => undefined); + const claimIssueSpy = vi.spyOn(claimLedger, "claimIssue"); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + resolveRejectionSignaled: async () => true, + }); + + expect(claimIssueSpy).not.toHaveBeenCalled(); + }); + + it("never claims when the coding-task-spec is infeasible", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const claimIssueSpy = vi.spyOn(claimLedger, "claimIssue"); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ + buildCodingTaskSpec: () => ({ + ready: false, + verdict: "avoid", + feasibility: { verdict: "avoid", avoidReasons: ["already_claimed"], raiseReasons: [], summary: "not feasible" }, + }), + }), + }); + + expect(claimIssueSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/test/unit/miner-attempt-input-builder.test.ts b/test/unit/miner-attempt-input-builder.test.ts index 87c8b638d0..aadc19845a 100644 --- a/test/unit/miner-attempt-input-builder.test.ts +++ b/test/unit/miner-attempt-input-builder.test.ts @@ -48,7 +48,12 @@ describe("buildAttemptGovernorContext (#5132)", () => { expect(ctx.liveModeGlobalOptIn).toBe(false); }); - it("REGRESSION: killSwitchRepoPaused is omitted (documented gap, not fabricated as false)", () => { + it("REGRESSION: killSwitchRepoPaused threads the caller's real per-repo pause value through (#5392)", () => { + const ctx = buildAttemptGovernorContext({}, DEFAULT_AMS_POLICY_SPEC, true); + expect(ctx.killSwitchRepoPaused).toBe(true); + }); + + it("killSwitchRepoPaused defaults to undefined when the caller omits it", () => { const ctx = buildAttemptGovernorContext({}, DEFAULT_AMS_POLICY_SPEC); expect(ctx.killSwitchRepoPaused).toBeUndefined(); }); diff --git a/test/unit/miner-loop-cli.test.ts b/test/unit/miner-loop-cli.test.ts index 29ce61a511..32208b0c3d 100644 --- a/test/unit/miner-loop-cli.test.ts +++ b/test/unit/miner-loop-cli.test.ts @@ -216,6 +216,7 @@ describe("runLoop (#5135)", () => { return 0; }); const pollPrDispositionSpy = vi.fn().mockResolvedValue({ state: "closed", merged: true, closedAt: "2026-07-12T00:00:00Z", attempts: 1 }); + const pollCheckRunsSpy = vi.fn().mockResolvedValue({ conclusion: "success", checks: [{ name: "test" }], headSha: "abc123", attempts: 1 }); const exitCode = await runLoop(["acme/widgets", "--miner-login", "alice", "--max-cycles", "2", "--json"], { env: { GITHUB_TOKEN: "ghp_loop_test" }, @@ -227,6 +228,7 @@ describe("runLoop (#5135)", () => { runDiscover: runDiscoverSpy, runAttempt: runAttemptSpy, pollPrDisposition: pollPrDispositionSpy, + pollCheckRuns: pollCheckRunsSpy, ...readyLoopOptions(), }); @@ -239,6 +241,8 @@ describe("runLoop (#5135)", () => { // reach the poller -- an unauthenticated poll would silently hit GitHub's much lower rate limit or fail // outright against a private repo. expect(pollPrDispositionSpy).toHaveBeenCalledWith("acme/widgets", 123, expect.objectContaining({ githubToken: "ghp_loop_test" })); + // REGRESSION (#5394): the real CI-status poll ran BEFORE the disposition poll, on the real submitted PR. + expect(pollCheckRunsSpy).toHaveBeenCalledWith("acme/widgets", 123, expect.objectContaining({ githubToken: "ghp_loop_test" })); const after = reopenAfterRun(paths); @@ -247,6 +251,11 @@ describe("runLoop (#5135)", () => { expect(prOutcomeEvents).toHaveLength(1); expect(prOutcomeEvents[0]?.payload).toMatchObject({ prNumber: 123, decision: "merged" }); + // REGRESSION (#5394): the real CI-status observation was recorded in the loop's own event ledger. + const ciStatusEvents = after.eventLedger.readEvents({}).filter((e) => e.type === "ci_status_observed"); + expect(ciStatusEvents).toHaveLength(1); + expect(ciStatusEvents[0]?.payload).toMatchObject({ prNumber: 123, conclusion: "success", checkCount: 1 }); + // The claimed item resolved to done (real success), not left in_progress or requeued. expect(after.portfolioQueue.listQueue()).toEqual([expect.objectContaining({ identifier: "issue:7", status: "done" })]); @@ -260,7 +269,103 @@ describe("runLoop (#5135)", () => { expect(reentryEvents[0]?.payload).toMatchObject({ reentered: true, outcome: "merged" }); const printed = JSON.parse(String(log.mock.calls[0]?.[0])); - expect(printed.cycles[0]).toMatchObject({ outcome: "attempted", attemptOutcome: "attempt_submitted", reentryOutcome: "merged", prNumber: 123 }); + expect(printed.cycles[0]).toMatchObject({ + outcome: "attempted", + attemptOutcome: "attempt_submitted", + reentryOutcome: "merged", + prNumber: 123, + ciConclusion: "success", + }); + }); + + it("REGRESSION (#5394): polls CI status before PR disposition, on the same PR", async () => { + const { eventLedger, governorLedger, portfolioQueue, runState, governorState } = tempStores(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const item = { repoFullName: "acme/widgets", identifier: "issue:7" }; + const runDiscoverSpy = primeOnceDiscover(portfolioQueue, item); + const runAttemptSpy = vi.fn(async (_args: string[], options?: Record) => { + (options?.onResult as ((result: unknown) => void) | undefined)?.({ + outcome: "attempt_submitted", + repoFullName: "acme/widgets", + issueNumber: 7, + minerLogin: "alice", + base: "main", + mode: "dry_run", + attemptId: "loop-attempt-order", + submissionMode: "observe", + totalTurnsUsed: 1, + totalCostUsd: 0, + iterationsUsed: 1, + execResult: { action: "open_pr", stdout: "https://github.com/acme/widgets/pull/55\n", stderr: "", code: 0, timedOut: false }, + }); + return 0; + }); + const callOrder: string[] = []; + const pollCheckRunsSpy = vi.fn(async () => { + callOrder.push("ci"); + return { conclusion: "pending" as const, checks: [], headSha: "abc", attempts: 1 }; + }); + const pollPrDispositionSpy = vi.fn(async () => { + callOrder.push("disposition"); + return { state: "open" as const, merged: false, closedAt: null, attempts: 1 }; + }); + + await runLoop(["acme/widgets", "--miner-login", "alice", "--max-cycles", "1", "--json"], { + openGovernorState: () => governorState, + initEventLedger: () => eventLedger, + initGovernorLedger: () => governorLedger, + initPortfolioQueue: () => portfolioQueue, + initRunStateStore: () => runState, + runDiscover: runDiscoverSpy, + runAttempt: runAttemptSpy, + pollCheckRuns: pollCheckRunsSpy, + pollPrDisposition: pollPrDispositionSpy, + ...readyLoopOptions(), + }); + + expect(callOrder).toEqual(["ci", "disposition"]); + }); + + it("never polls CI status when the submitted attempt's PR number can't be parsed", async () => { + const { eventLedger, governorLedger, portfolioQueue, runState, governorState, paths } = tempStores(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const item = { repoFullName: "acme/widgets", identifier: "issue:7" }; + const runDiscoverSpy = primeOnceDiscover(portfolioQueue, item); + const runAttemptSpy = vi.fn(async (_args: string[], options?: Record) => { + (options?.onResult as ((result: unknown) => void) | undefined)?.({ + outcome: "attempt_submitted", + repoFullName: "acme/widgets", + issueNumber: 7, + minerLogin: "alice", + base: "main", + mode: "dry_run", + attemptId: "loop-attempt-no-pr", + submissionMode: "observe", + totalTurnsUsed: 1, + totalCostUsd: 0, + iterationsUsed: 1, + execResult: { action: "open_pr", stdout: "no url printed here\n", stderr: "", code: 0, timedOut: false }, + }); + return 0; + }); + const pollCheckRunsSpy = vi.fn(); + + await runLoop(["acme/widgets", "--miner-login", "alice", "--max-cycles", "1", "--json"], { + openGovernorState: () => governorState, + initEventLedger: () => eventLedger, + initGovernorLedger: () => governorLedger, + initPortfolioQueue: () => portfolioQueue, + initRunStateStore: () => runState, + runDiscover: runDiscoverSpy, + runAttempt: runAttemptSpy, + pollCheckRuns: pollCheckRunsSpy, + ...readyLoopOptions(), + }); + + expect(pollCheckRunsSpy).not.toHaveBeenCalled(); + expect(reopenAfterRun(paths).eventLedger.readEvents({}).filter((e) => e.type === "ci_status_observed")).toHaveLength(0); + const printed = JSON.parse(String(log.mock.calls[0]?.[0])); + expect(printed.cycles[0]).toMatchObject({ outcome: "attempted", prNumber: null, ciConclusion: null }); }); it("REGRESSION: a repeatedly-blocked (non-permanent) outcome requeues the item and eventually halts on real non-convergence, not forever", async () => {