feat(miner): wire attempt-metering.ts into the iterate loop for a real mid-attempt budget abort

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<IterateLoopResult, "finalMeterTotals" | "budgetBreaches">;
+
+/** 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<IterateLoopResult> {
+async function runIterateLoopCore(input: IterateLoopInput, deps: IterateLoopDeps, tracker: MeterTracker): Promise<IterateLoopCoreResult> {
   // 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,34 +310,62 @@ 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,
       acceptanceCriteriaPath: input.acceptanceCriteriaPath,
       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<IterateLoopResult> {
+  const tracker: MeterTracker = { totals: ZERO_METER_TOTALS, breaches: [] };
+  const core = await runIterateLoopCore(input, deps, tracker);
+  return { ...core, finalMeterTotals: tracker.totals, budgetBreaches: tracker.breaches };
+}

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 () => {