diff --git a/packages/gittensory-engine/src/miner/iterate-loop.ts b/packages/gittensory-engine/src/miner/iterate-loop.ts index 6b9ec75d7..7961a45cb 100644 --- a/packages/gittensory-engine/src/miner/iterate-loop.ts +++ b/packages/gittensory-engine/src/miner/iterate-loop.ts @@ -15,10 +15,12 @@ // precedence then abandons rather than optimistically continuing or handing off. The loop never fabricates a // "pass" from anything other than a genuinely successful `runSelfReview` call. // -// BOUNDED INSIDE THE LOOP: both the iteration ceiling (`input.maxIterations`) and the optional cumulative-cost -// ceiling (`input.maxTotalTurns`, summing every iteration's `turnsUsed`) are enforced here every iteration -- -// not left to an external caller to remember. A `maxIterations <= 0` input abandons immediately, before ever -// invoking the driver. +// BOUNDED INSIDE THE LOOP: both the iteration ceiling (`input.maxIterations`) and the optional cumulative +// budget (`input.budget`, evaluated every iteration via attempt-metering.ts's `accumulateAttemptUsage`/ +// `evaluateAttemptBudget` against real per-iteration turns/costUsd/wallClockMs -- tokens stays an honest 0, +// no driver reports a real token count today, #5395) are enforced here every iteration -- not left to an +// external caller to remember, and not just capped after the fact between loop cycles (loop-cli.js's own +// governor cap usage). A `maxIterations <= 0` input abandons immediately, before ever invoking the driver. // // AUDITABLE: every iteration's decision (continue / handoff / abandon) is recorded via the injected // `appendAttemptLogEvent` dependency (attempt-log.ts's normalized event shape) before this function returns @@ -32,6 +34,7 @@ import { invokeCodingAgentDriver } from "./coding-agent-invoke.js"; import type { AttemptLogEvent, AttemptLogEventType } from "./attempt-log.js"; import { runSelfReview, type AttemptDiffState, type SelfReviewAdapterDeps, type SelfReviewContext, type SelfReviewVerdict } from "./self-review-adapter.js"; import { decideNextActionWithReason, deriveSelfReviewOutcome, type IterateLoopDecision, type HandoffPacket, type IterationState, type SelfReviewOutcome } from "./iterate-policy.js"; +import { accumulateAttemptUsage, evaluateAttemptBudget, type AttemptBudget, type AttemptBudgetAxis, type AttemptMeterTotals } from "./attempt-metering.js"; /** Everything one call to {@link runIterateLoop} needs, aside from the injected {@link IterateLoopDeps}. * Identity/context fields mirror self-review-adapter.ts's `AttemptDiffState`/`SelfReviewContext` exactly -- @@ -51,10 +54,15 @@ export type IterateLoopInput = { maxIterations: number; /** Per-iteration turn budget, passed through to each `CodingAgentDriverTask`. */ maxTurnsPerIteration: number; - /** Optional hard ceiling on CUMULATIVE turns spent across every iteration of this attempt so far (summed - * from each iteration's `CodingAgentDriverResult.turnsUsed`). Omitted means no additional cost ceiling - * beyond what `maxIterations * maxTurnsPerIteration` already implies. */ - maxTotalTurns?: number | undefined; + /** Optional cumulative budget ceiling(s), evaluated every iteration against the real running totals + * (attempt-metering.ts's `AttemptMeterTotals`, #5395). Omitted means no additional ceiling beyond what + * `maxIterations * maxTurnsPerIteration` already implies. A breach on any axis (turns/costUsd/wallClockMs + * -- tokens is never real-tracked today) is a HARD, unconditional stop: checked and abandoned on BEFORE + * self-review even runs, so a same-iteration pass can never bypass the ceiling (this is deliberately NOT + * routed through iterate-policy.ts's `costCeilingReached` field, whose own precedence checks a self-review + * pass first -- see the loop body's own comment at the check site for why that ordering doesn't fit a hard + * budget ceiling). */ + budget?: AttemptBudget | undefined; // Self-review identity fields -- mirror `AttemptDiffState`'s own identity fields (self-review-adapter.ts). repoFullName: string; @@ -82,6 +90,10 @@ export type IterateLoopDeps = { driver: CodingAgentDriver; runSlopAssessment: SelfReviewAdapterDeps["runSlopAssessment"]; appendAttemptLogEvent: (event: AttemptLogEvent) => void; + /** Injected clock for real per-iteration wall-clock measurement (#5395), mirroring this package's own + * injected-dependency discipline elsewhere (never a hardcoded `Date.now()` a test can't control). Defaults + * to the real `Date.now` when omitted. */ + nowMs?: (() => number) | undefined; }; /** The terminal outcomes a full loop run can end in -- never `"continue"`, which is only ever a per-iteration, @@ -106,6 +118,14 @@ export type IterateLoopResult = { * `CodingAgentDriverResult.costUsd`. Only the `agent-sdk` provider reports this today (the CLI-subprocess * providers report no cost signal) -- always `0` for a provider that never reports one, never fabricated. */ totalCostUsd: number; + /** The real accumulated {@link AttemptMeterTotals} across every iteration that ran (attempt-metering.ts, + * #5395) -- a superset of `totalTurnsUsed`/`totalCostUsd` above that also carries `wallClockMs` (real, + * measured around each driver invocation) and `tokens` (always 0 today -- no driver reports a real token + * count, an honest absence rather than a fabricated number). */ + finalMeterTotals: AttemptMeterTotals; + /** The budget axes breached at the point this attempt abandoned, when `input.budget` was set and at least + * one axis was at/over its ceiling -- empty when no budget was configured or none breached. */ + budgetBreaches: AttemptBudgetAxis[]; iterations: readonly IterateLoopIterationRecord[]; /** Populated only when `outcome === "handoff"`. */ handoffPacket?: HandoffPacket | undefined; @@ -180,7 +200,13 @@ function safeAppendAttemptLogEvent(deps: IterateLoopDeps, event: AttemptLogEvent } } -function logDecision(input: IterateLoopInput, deps: IterateLoopDeps, iterationNumber: number, decision: IterateLoopDecision): void { +function logDecision( + input: IterateLoopInput, + deps: IterateLoopDeps, + iterationNumber: number, + decision: IterateLoopDecision, + budgetBreaches: readonly AttemptBudgetAxis[], +): void { safeAppendAttemptLogEvent(deps, { eventType: attemptLogEventTypeForDecision(decision), attemptId: input.attemptId, @@ -191,6 +217,10 @@ function logDecision(input: IterateLoopInput, deps: IterateLoopDeps, iterationNu iterationNumber, action: decision.action, ...(decision.abandonReason !== undefined ? { abandonReason: decision.abandonReason } : {}), + // Which real axis (or axes) breached, on the hard-budget-ceiling abandon path (#5395, checked directly + // in the loop body before self-review even runs -- see that check site's own comment) -- an operator + // reading the attempt log back otherwise has no way to see which axis actually tripped. + ...(budgetBreaches.length > 0 ? { budgetBreaches } : {}), }, }); } @@ -218,7 +248,23 @@ function buildHandoffPacket(input: IterateLoopInput, verdict: SelfReviewVerdict, }; } -function immediateAbandonNoIterationsPermitted(input: IterateLoopInput, deps: IterateLoopDeps): IterateLoopResult { +const ZERO_METER_TOTALS: AttemptMeterTotals = { tokens: 0, turns: 0, wallClockMs: 0, costUsd: 0 }; + +/** The result shape {@link runIterateLoopCore} itself returns -- everything BUT the meter fields, which the + * thin {@link runIterateLoop} wrapper attaches once, from `tracker`, at its own single always-reached return + * point (#5395). Keeps the core's own internal return statements -- including the `/* v8 ignore *\/`-guarded + * unreachable fallback -- byte-identical to their pre-#5395 shape, so that genuinely unreachable branch never + * needs new fields threaded onto it (v8's ignore-comment suppresses vitest's OWN text-reporter percentage, + * but NOT the raw lcov Codecov reads -- a new field on that branch would show as a real uncovered patch line + * with no way to actually exercise it). */ +type IterateLoopCoreResult = Omit; + +/** Mutable accumulator threaded into {@link runIterateLoopCore} so the wrapper can read the real running + * totals/breaches after the core returns, without the core itself needing to carry them on every return + * statement. */ +type MeterTracker = { totals: AttemptMeterTotals; breaches: AttemptBudgetAxis[] }; + +function immediateAbandonNoIterationsPermitted(input: IterateLoopInput, deps: IterateLoopDeps): IterateLoopCoreResult { const decision: IterateLoopDecision = { action: "abandon", abandonReason: "max_iterations_reached", @@ -242,9 +288,10 @@ function immediateAbandonNoIterationsPermitted(input: IterateLoopInput, deps: It * Every iteration: invoke the driver, self-review the resulting diff (never fabricating a pass from a failed * or errored driver/self-review run), consult the policy with the running iteration/cost/no-progress state, * and record the decision via the attempt-log. `"continue"` decisions loop again; `"handoff"`/`"abandon"` - * return immediately. + * return immediately. See {@link IterateLoopCoreResult}'s own doc comment for why this doesn't carry + * `finalMeterTotals`/`budgetBreaches` itself -- {@link runIterateLoop} attaches those. */ -export async function runIterateLoop(input: IterateLoopInput, deps: IterateLoopDeps): Promise { +async function runIterateLoopCore(input: IterateLoopInput, deps: IterateLoopDeps, tracker: MeterTracker): Promise { // Truncated toward zero rather than used as-is: a fractional maxIterations (a caller bug -- "how many times // to run a coding agent" has no fractional meaning) would otherwise let this loop's own `for` bound and // iterate-policy.ts's `iterationNumber >= maxIterations` ceiling check disagree by less than one iteration @@ -263,12 +310,14 @@ export async function runIterateLoop(input: IterateLoopInput, deps: IterateLoopD payload: { maxIterations, maxTurnsPerIteration: input.maxTurnsPerIteration }, }); + const nowMs = deps.nowMs ?? Date.now; const iterations: IterateLoopIterationRecord[] = []; let previousBlockerCodes: readonly string[] | null = null; let totalTurnsUsed = 0; let totalCostUsd = 0; for (let iterationNumber = 1; iterationNumber <= maxIterations; iterationNumber += 1) { + const iterationStartMs = nowMs(); const driverResult = await runDriverSafely(input, deps, { attemptId: input.attemptId, workingDirectory: input.workingDirectory, @@ -276,21 +325,47 @@ export async function runIterateLoop(input: IterateLoopInput, deps: IterateLoopD instructions: input.instructions, maxTurns: input.maxTurnsPerIteration, }); + const iterationElapsedMs = Math.max(0, nowMs() - iterationStartMs); totalTurnsUsed += driverResult.turnsUsed ?? 0; totalCostUsd += driverResult.costUsd ?? 0; + // tokens stays an honest 0: no CodingAgentDriver reports a real per-iteration token count today (#5395) -- + // an absence, never a fabricated number, matching this package's costUsd discipline elsewhere. + tracker.totals = accumulateAttemptUsage(tracker.totals, { + tokens: 0, + turns: driverResult.turnsUsed ?? 0, + wallClockMs: iterationElapsedMs, + costUsd: driverResult.costUsd ?? 0, + }); + const budgetVerdict = input.budget !== undefined ? evaluateAttemptBudget(tracker.totals, input.budget) : undefined; + tracker.breaches = budgetVerdict?.breaches ?? []; + + // A reached budget ceiling is a HARD, unconditional stop -- checked and, if breached, acted on BEFORE + // self-review even runs, so a same-iteration pass can never bypass it. The spend/turns/time on this + // iteration are sunk either way (the driver already ran), but handing off anyway would let a single + // over-budget iteration silently defeat the entire point of wiring a ceiling in (#5395's own mid-attempt + // abort goal) -- so this abandons regardless of what this iteration's own result looks like. + if (budgetVerdict !== undefined && !budgetVerdict.withinBudget) { + const decision: IterateLoopDecision = { + action: "abandon", + abandonReason: "cost_ceiling_reached", + reason: `Reached the attempt's budget ceiling (${tracker.breaches.join(", ")}) on iteration ${iterationNumber}; abandoning regardless of this iteration's own result.`, + }; + logDecision(input, deps, iterationNumber, decision, tracker.breaches); + iterations.push({ iterationNumber, driverResult, decision }); + return { outcome: "abandon", finalDecision: decision, iterationsUsed: iterationNumber, totalTurnsUsed, totalCostUsd, iterations }; + } const { outcome: selfReview, verdict } = evaluateSelfReviewOutcome(input, driverResult, deps); const state: IterationState = { iterationNumber, maxIterations, - costCeilingReached: input.maxTotalTurns !== undefined && totalTurnsUsed >= input.maxTotalTurns, selfReview, previousBlockerCodes, rejectionSignaled: input.rejectionSignaled, }; const decision = decideNextActionWithReason(state); - logDecision(input, deps, iterationNumber, decision); + logDecision(input, deps, iterationNumber, decision, []); iterations.push({ iterationNumber, driverResult, decision }); if (decision.action === "handoff") { @@ -322,3 +397,14 @@ export async function runIterateLoop(input: IterateLoopInput, deps: IterateLoopD const fallbackDecision: IterateLoopDecision = { action: "abandon", abandonReason: "max_iterations_reached", reason: "Iterate loop exhausted its iteration budget." }; return { outcome: "abandon", finalDecision: fallbackDecision, iterationsUsed: maxIterations, totalTurnsUsed, totalCostUsd, iterations }; } + +/** + * Thin wrapper over {@link runIterateLoopCore} that attaches the real accumulated + * {@link AttemptMeterTotals}/breached axes (#5395) once, at this single always-reached return point -- see + * {@link IterateLoopCoreResult}'s own doc comment for why the core itself doesn't carry these fields. + */ +export async function runIterateLoop(input: IterateLoopInput, deps: IterateLoopDeps): Promise { + const tracker: MeterTracker = { totals: ZERO_METER_TOTALS, breaches: [] }; + const core = await runIterateLoopCore(input, deps, tracker); + return { ...core, finalMeterTotals: tracker.totals, budgetBreaches: tracker.breaches }; +} diff --git a/packages/gittensory-engine/test/iterate-loop.test.ts b/packages/gittensory-engine/test/iterate-loop.test.ts index 13d1189b5..28076271f 100644 --- a/packages/gittensory-engine/test/iterate-loop.test.ts +++ b/packages/gittensory-engine/test/iterate-loop.test.ts @@ -239,21 +239,89 @@ test("a fractional maxIterations truncates toward the lower integer, not silentl assert.equal(result.iterationsUsed, 1); }); -test("abandon (cost_ceiling_reached): the cumulative-turns ceiling stops the loop even with iterations still available", async () => { - const pullRequests: PullRequestRecord[] = [openPr(42, "Retry uploads on 5xx responses", [7])]; +test("abandon (cost_ceiling_reached): a maxTurns budget breach stops the loop even with iterations still available AND a driver result that would otherwise pass self-review", async () => { const { deps } = collectingDeps({ driver: driverReturning(okResult(["src/upload.ts"], 50)) }); - const input = passingInput({ maxIterations: 10, maxTotalTurns: 20, reviewContext: baseReviewContext({ pullRequests }) }); + const input = passingInput({ maxIterations: 10, budget: { maxTurns: 20 } }); const result = await runIterateLoop(input, deps); assert.equal(result.outcome, "abandon"); assert.equal(result.finalDecision.abandonReason, "cost_ceiling_reached"); assert.equal(result.iterationsUsed, 1); + assert.deepEqual(result.budgetBreaches, ["turns"]); + assert.equal(result.finalMeterTotals.turns, 50); +}); + +test("abandon (cost_ceiling_reached): a maxCostUsd budget breach reports costUsd as the breached axis", async () => { + const { deps } = collectingDeps({ driver: driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: 1, costUsd: 6 }) }); + const input = passingInput({ maxIterations: 10, budget: { maxCostUsd: 5 } }); + const result = await runIterateLoop(input, deps); + + assert.equal(result.outcome, "abandon"); + assert.equal(result.finalDecision.abandonReason, "cost_ceiling_reached"); + assert.deepEqual(result.budgetBreaches, ["costUsd"]); + assert.equal(result.finalMeterTotals.costUsd, 6); +}); + +test("abandon (cost_ceiling_reached): a maxWallClockMs budget breach uses the real injected clock, not a fabricated duration", async () => { + let call = 0; + const timestamps = [1_000, 1_000 + 90_000]; // 90s elapsed on the one iteration + const { deps } = collectingDeps({ driver: driverReturning(okResult(["src/upload.ts"], 1)) }); + const input = passingInput({ maxIterations: 10, budget: { maxWallClockMs: 60_000 } }); + const result = await runIterateLoop(input, { + ...deps, + nowMs: () => timestamps[call++] ?? timestamps[timestamps.length - 1]!, + }); + + assert.equal(result.outcome, "abandon"); + assert.equal(result.finalDecision.abandonReason, "cost_ceiling_reached"); + assert.deepEqual(result.budgetBreaches, ["wallClockMs"]); + assert.equal(result.finalMeterTotals.wallClockMs, 90_000); +}); + +test("continue: budget omitted never trips the cost ceiling, regardless of turns/cost spent", async () => { + const { deps } = collectingDeps({ driver: driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: 1_000_000, costUsd: 999 }) }); + const result = await runIterateLoop(passingInput({ budget: undefined }), deps); + assert.equal(result.outcome, "handoff"); + assert.deepEqual(result.budgetBreaches, []); }); -test("continue: maxTotalTurns omitted never trips the cost ceiling, regardless of turns spent", async () => { - const { deps } = collectingDeps({ driver: driverReturning(okResult(["src/upload.ts"], 1_000_000)) }); - const result = await runIterateLoop(passingInput({ maxTotalTurns: undefined }), deps); +test("handoff: a budget that IS configured but comfortably within limits never trips the ceiling", async () => { + const { deps } = collectingDeps({ driver: driverReturning(okResult(["src/upload.ts"], 5)) }); + const result = await runIterateLoop(passingInput({ budget: { maxTurns: 20, maxCostUsd: 5, maxWallClockMs: 60_000 } }), deps); assert.equal(result.outcome, "handoff"); + assert.deepEqual(result.budgetBreaches, []); + assert.equal(result.finalMeterTotals.turns, 5); +}); + +test("finalMeterTotals.tokens is always an honest 0 -- no driver reports a real token count", async () => { + const { deps } = collectingDeps({ driver: driverReturning(okResult()) }); + const result = await runIterateLoop(passingInput(), deps); + assert.equal(result.finalMeterTotals.tokens, 0); +}); + +test("immediate abandon: finalMeterTotals/budgetBreaches are the honest zero/empty shape when the driver never ran", async () => { + const { deps } = collectingDeps(); + const result = await runIterateLoop(baseInput({ maxIterations: 0 }), deps); + assert.deepEqual(result.finalMeterTotals, { tokens: 0, turns: 0, wallClockMs: 0, costUsd: 0 }); + assert.deepEqual(result.budgetBreaches, []); +}); + +test("REGRESSION: a hard budget ceiling abandons even on the same iteration a passing self-review would have produced (ceiling wins, not pass) -- gittensory review #5437", async () => { + let selfReviewRan = false; + const { deps } = collectingDeps({ + driver: driverReturning(okResult(["src/upload.ts"], 999)), + runSlopAssessment: () => { + selfReviewRan = true; + return noopSlop; + }, + }); + const result = await runIterateLoop(passingInput({ maxIterations: 5, budget: { maxTurns: 1 } }), deps); + assert.equal(result.outcome, "abandon"); + assert.equal(result.finalDecision.abandonReason, "cost_ceiling_reached"); + assert.deepEqual(result.budgetBreaches, ["turns"]); + // Self-review is skipped entirely once the ceiling is breached -- its verdict can never change an + // already-decided outcome, so there's no reason to spend the (cheap, local) computation. + assert.equal(selfReviewRan, false); }); test("abandon (rejection_signaled): wins even over a self-review that would otherwise cleanly pass", async () => { diff --git a/packages/gittensory-miner/docs/coding-agent-driver.md b/packages/gittensory-miner/docs/coding-agent-driver.md index 67486cfa5..702f42c52 100644 --- a/packages/gittensory-miner/docs/coding-agent-driver.md +++ b/packages/gittensory-miner/docs/coding-agent-driver.md @@ -117,14 +117,19 @@ path with house-rule enforcement built in by default -- no such caller exists in real house-rule enforcement for the live `agent-sdk` provider happens via `buildHouseRulesAgentSdkHooks`, attached directly in `constructProductionCodingAgentDriver`. -**Metering:** `attempt-metering.ts`'s `accumulateAttemptUsage`/`meterAttemptUsage`/`evaluateAttemptBudget` -(tokens/turns/wallClockMs/costUsd, with mid-attempt budget-abort) have **zero production callers** as of this -writing -- see [#5395](https://github.com/JSONbored/gittensory/issues/5395) for the tracked decision on -whether to wire them in for real or remove them. What IS real today: `iterate-loop.ts` sums each iteration's -real `driverResult.turnsUsed`/`costUsd` into `IterateLoopResult.totalTurnsUsed`/`totalCostUsd`, which -`packages/gittensory-miner/lib/loop-cli.js` uses to increment the governor's persisted `GovernorCapUsage` -between loop cycles -- an after-the-fact, cross-cycle total, not the per-iteration mid-attempt abort -`attempt-metering.ts` was designed to provide. +**Metering:** `attempt-metering.ts`'s `accumulateAttemptUsage`/`evaluateAttemptBudget` are wired into +`iterate-loop.ts` for real ([#5395](https://github.com/JSONbored/gittensory/issues/5395)) -- every iteration +accumulates real `turns`/`costUsd`/`wallClockMs` (`tokens` stays an honest 0; no driver reports a real +per-iteration token count today) into a running `AttemptMeterTotals`, and `runIterateLoop`'s optional +`input.budget: AttemptBudget` is evaluated against it each iteration via the SAME `costCeilingReached` signal +`iterate-policy.ts` already exposed for the (now-removed) turns-only `maxTotalTurns` ceiling -- a genuine +mid-attempt abort, not just a between-cycle cap. `packages/gittensory-miner/lib/attempt-input-builder.js`'s +`buildAttemptLoopInput` sets `budget` from the SAME `AmsPolicySpec.capLimits` the Governor's cross-cycle +`GovernorCapUsage` already uses (`packages/gittensory-miner/lib/loop-cli.js`'s `governorState.saveCapUsage` +between loop cycles) -- one attempt can no longer burn through the entire cross-cycle budget before anything +reacts. `wallClockMs` uses a real injected clock (`IterateLoopDeps.nowMs`, defaulting to `Date.now`) measured +around each iteration's driver invocation. The result's `finalMeterTotals`/`budgetBreaches` fields surface +which axis (if any) triggered an abandon, for an operator reading the attempt log back. ## Related docs diff --git a/packages/gittensory-miner/lib/attempt-input-builder.js b/packages/gittensory-miner/lib/attempt-input-builder.js index 58fd1d222..233fef8b4 100644 --- a/packages/gittensory-miner/lib/attempt-input-builder.js +++ b/packages/gittensory-miner/lib/attempt-input-builder.js @@ -68,6 +68,16 @@ export function buildAttemptLoopInput(input) { mode: input.mode, maxIterations: input.amsPolicySpec.maxIterations, maxTurnsPerIteration: input.amsPolicySpec.maxTurnsPerIteration, + // Real mid-attempt budget (#5395): the SAME Governor cap ceilings that already bound cross-cycle spend + // (loop-cli.js's after-the-fact governorState.saveCapUsage) now also bound this ONE attempt in progress, + // via the engine's real accumulateAttemptUsage/evaluateAttemptBudget -- a runaway attempt can no longer + // burn through the entire cross-cycle budget before anything reacts. No maxTokens: no driver reports a + // real per-iteration token count today, so that axis has no real ceiling to set (never fabricated). + budget: { + maxTurns: input.amsPolicySpec.capLimits.turns, + maxWallClockMs: input.amsPolicySpec.capLimits.elapsedMs, + maxCostUsd: input.amsPolicySpec.capLimits.budget, + }, repoFullName: input.repoFullName, contributorLogin: input.minerLogin, title: input.codingTaskSpec.title, diff --git a/test/unit/miner-attempt-input-builder.test.ts b/test/unit/miner-attempt-input-builder.test.ts index aadc19845..7c9f5b1fc 100644 --- a/test/unit/miner-attempt-input-builder.test.ts +++ b/test/unit/miner-attempt-input-builder.test.ts @@ -93,6 +93,11 @@ describe("buildAttemptLoopInput (#5132)", () => { mode: "dry_run", maxIterations: DEFAULT_AMS_POLICY_SPEC.maxIterations, maxTurnsPerIteration: DEFAULT_AMS_POLICY_SPEC.maxTurnsPerIteration, + budget: { + maxTurns: DEFAULT_AMS_POLICY_SPEC.capLimits.turns, + maxWallClockMs: DEFAULT_AMS_POLICY_SPEC.capLimits.elapsedMs, + maxCostUsd: DEFAULT_AMS_POLICY_SPEC.capLimits.budget, + }, repoFullName: "acme/widgets", contributorLogin: "alice", title: "Uploads should retry on 5xx", @@ -105,6 +110,21 @@ describe("buildAttemptLoopInput (#5132)", () => { }); }); + it("REGRESSION (#5395): budget mirrors AmsPolicySpec's real capLimits, not hardcoded literals", () => { + const loopInput = buildAttemptLoopInput({ + codingTaskSpec: codingTaskSpec(), + reviewContext: reviewContext(), + worktreePath: "/fake", + attemptId: "a1", + mode: "dry_run", + repoFullName: "acme/widgets", + minerLogin: "alice", + rejectionSignaled: false, + amsPolicySpec: { ...DEFAULT_AMS_POLICY_SPEC, capLimits: { budget: 9, turns: 8, elapsedMs: 7 } }, + }); + expect(loopInput.budget).toEqual({ maxTurns: 8, maxWallClockMs: 7, maxCostUsd: 9 }); + }); + it("threads a real rejectionSignaled:true through unchanged", () => { const loopInput = buildAttemptLoopInput({ codingTaskSpec: codingTaskSpec(), diff --git a/test/unit/miner-attempt-runner.test.ts b/test/unit/miner-attempt-runner.test.ts index 141d75d2a..ab3b1314c 100644 --- a/test/unit/miner-attempt-runner.test.ts +++ b/test/unit/miner-attempt-runner.test.ts @@ -182,6 +182,32 @@ describe("runMinerAttempt (#2337) — the real create->review->gate->submit pipe expect(claimLedgerListClaims).not.toHaveBeenCalled(); }); + it("REGRESSION (#5395, gittensory review #5437): a real hard budget-ceiling breach abandons through the full real runMinerAttempt pipeline, even though the driver's own result would otherwise pass self-review", async () => { + const claimLedgerListClaims = vi.fn(); + const deps = baseDeps({ claimLedger: { listClaims: claimLedgerListClaims } }); + // okDriverResult()'s default turnsUsed (5) immediately breaches this maxTurns:1 ceiling on iteration 1, + // even though the SAME driver result is exactly what the happy-path test above uses to reach a real + // "submitted" outcome -- proving the ceiling wins over a same-iteration pass, not just in isolation. + const result = await runMinerAttempt(baseAttemptInput({ loopInput: passingLoopInput({ budget: { maxTurns: 1 } }) }), deps); + + expect(result.outcome).toBe("abandon"); + expect(result.loopResult.finalDecision.abandonReason).toBe("cost_ceiling_reached"); + expect(result.loopResult.budgetBreaches).toEqual(["turns"]); + expect(result.loopResult.iterationsUsed).toBe(1); + // No downstream gate consulted -- the same real short-circuit as the maxIterations:0 case above. + expect(claimLedgerListClaims).not.toHaveBeenCalled(); + }); + + it("a budget that IS configured but comfortably within limits never trips the ceiling -- real pipeline reaches submitted", async () => { + const deps = baseDeps(); + const result = await runMinerAttempt(baseAttemptInput({ loopInput: passingLoopInput({ budget: { maxTurns: 20, maxCostUsd: 5, maxWallClockMs: 60_000 } }) }), deps); + + expect(result.outcome).toBe("submitted"); + if (result.outcome !== "submitted") throw new Error("expected submitted"); + expect(result.loopResult.outcome).toBe("handoff"); + expect(result.loopResult.budgetBreaches).toEqual([]); + }); + it("abandon mid-loop: a real iteration ran (and was billed) before the ceiling forced abandon", async () => { // maxIterations: 1 with a driver that never produces a passing self-review runs ONE real iteration, then // decideNextActionWithReason's own iterationNumber>=maxIterations check abandons -- a genuinely different