fix(engine): reject non-finite agent-sdk usage so it can't crash the iterate loop (#5827)

packages/loopover-engine/src/miner/agent-sdk-driver.ts

@@ -76,16 +76,25 @@ function asRecord(value: unknown): Record<string, unknown> | null {
   return typeof value === "object" && value !== null ? (value as Record<string, unknown>) : null;
 }
 
+/** A finite, non-negative number, else undefined — mirrors `cli-subprocess-driver.ts`'s helper of the same name
+ *  so both drivers reject the same out-of-contract usage values (`NaN`, `Infinity`, negatives) from untrusted
+ *  driver output. A malformed value degrades to the driver's existing "field absent" contract (undefined) rather
+ *  than propagating downstream, where `attempt-metering.ts`'s accumulateAttemptUsage would throw a RangeError and
+ *  reject the whole iterate loop before its decision is ever logged (#5827). */
+function finiteNonNegativeNumber(value: unknown): number | undefined {
+  return typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : undefined;
+}
+
 /** Real token count from the SDK's own result message (#5653). Both `SDKResultSuccess` and `SDKResultError`
  *  declare `usage: NonNullableUsage` unconditionally -- present whenever a result message arrived at all, same
  *  as `total_cost_usd`. `NonNullableUsage`'s `input_tokens`/`output_tokens` are themselves non-nullable numbers
  *  once `usage` exists, but this driver reads `resultMessage` as a loosely-typed record (like every other field
- *  read here), so both are re-validated defensively rather than trusted from an untyped source. Returns
- *  undefined (never a fabricated 0) when `usage` is absent or malformed. */
+ *  read here), so both are re-validated defensively (finite + non-negative, #5827) rather than trusted from an
+ *  untyped source. Returns undefined (never a fabricated 0) when `usage` is absent or malformed. */
 function tokensFromResultMessage(resultMessage: Record<string, unknown> | null): number | undefined {
   const usage = asRecord(resultMessage?.usage);
-  const inputTokens = typeof usage?.input_tokens === "number" ? usage.input_tokens : undefined;
-  const outputTokens = typeof usage?.output_tokens === "number" ? usage.output_tokens : undefined;
+  const inputTokens = finiteNonNegativeNumber(usage?.input_tokens);
+  const outputTokens = finiteNonNegativeNumber(usage?.output_tokens);
   if (inputTokens === undefined && outputTokens === undefined) return undefined;
   return (inputTokens ?? 0) + (outputTokens ?? 0);
 }
@@ -175,13 +184,14 @@ export function createAgentSdkCodingAgentDriver(
         };
       }
 
-      const turnsUsed =
-        typeof resultMessage?.num_turns === "number" ? resultMessage.num_turns : undefined;
+      // finiteNonNegativeNumber (not a bare typeof check): a malformed num_turns/total_cost_usd (NaN, Infinity,
+      // negative) must degrade to undefined here, or it reaches accumulateAttemptUsage unguarded and throws a
+      // RangeError that rejects runIterateLoopCore before any decision is logged (#5827).
+      const turnsUsed = finiteNonNegativeNumber(resultMessage?.num_turns);
       // Real dollar cost: the SDK's own SDKResultSuccess/SDKResultError message types both declare
       // `total_cost_usd: number` unconditionally -- present whenever a result message arrived at all, success
       // or not (the session was billed either way), absent only when the stream produced no result message.
-      const costUsd =
-        typeof resultMessage?.total_cost_usd === "number" ? resultMessage.total_cost_usd : undefined;
+      const costUsd = finiteNonNegativeNumber(resultMessage?.total_cost_usd);
       const tokensUsed = tokensFromResultMessage(resultMessage);
       const resultText =
         typeof resultMessage?.result === "string" ? redactSecrets(resultMessage.result) : "";

packages/loopover-engine/src/miner/iterate-loop.ts

@@ -276,6 +276,16 @@ function logDecision(
  * respectively) before ever reaching its `"continue"` fallthrough, so `outcome.kind === "fail"` is guaranteed
  * whenever this is reached from the real call site below, not just the common case.
  */
+/** A finite, non-negative usage value, else 0. accumulateAttemptUsage (attempt-metering.ts) deliberately THROWS
+ *  a RangeError on a negative/non-finite input to protect its own direct callers; this call site sits outside the
+ *  loop's driver/self-review try/catch blocks, so an uncaught throw here would reject runIterateLoopCore before
+ *  its decision is logged, violating the loop's "every iteration's decision is recorded before returning" contract
+ *  (#5827). The Agent SDK driver now degrades bad usage fields to undefined at the source, but this call takes any
+ *  driver's result — clamp here too so no current or future driver can crash the loop instead of being governed. */
+function finiteNonNegativeUsage(value: number | undefined): number {
+  return typeof value === "number" && Number.isFinite(value) && value >= 0 ? value : 0;
+}
+
 function blockerCodesFromContinuingOutcome(outcome: SelfReviewOutcome): readonly string[] {
   if (outcome.kind === "fail") return outcome.blockerCodes;
   /* v8 ignore next -- unreachable: see this function's own doc comment above. */
@@ -410,10 +420,10 @@ async function runIterateLoopCore(input: IterateLoopInput, deps: IterateLoopDeps
     // that reports one (Agent SDK's own result-message usage, or CLI JSON/JSONL stdout) -- 0 only when the
     // driver genuinely reports no token signal for this iteration, same honest-absence discipline as costUsd.
     tracker.totals = accumulateAttemptUsage(tracker.totals, {
-      tokens: driverResult.tokensUsed ?? 0,
-      turns: driverResult.turnsUsed ?? 0,
+      tokens: finiteNonNegativeUsage(driverResult.tokensUsed),
+      turns: finiteNonNegativeUsage(driverResult.turnsUsed),
       wallClockMs: iterationElapsedMs,
-      costUsd: driverResult.costUsd ?? 0,
+      costUsd: finiteNonNegativeUsage(driverResult.costUsd),
     });
     const budgetVerdict = input.budget !== undefined ? evaluateAttemptBudget(tracker.totals, input.budget) : undefined;
     tracker.breaches = budgetVerdict?.breaches ?? [];

packages/loopover-engine/test/agent-sdk-driver.test.ts

@@ -196,6 +196,47 @@ test("tokensUsed is undefined when usage exists but is malformed (not an object,
   assert.equal(nonNumericResult.tokensUsed, undefined);
 });
 
+// #5827: a malformed num_turns/total_cost_usd (negative, NaN, Infinity) must degrade to undefined — the same
+// finite/non-negative discipline cli-subprocess-driver.ts applies — so it never reaches accumulateAttemptUsage
+// (attempt-metering.ts), which throws a RangeError on such input and would reject the whole iterate loop.
+test("turnsUsed degrades to undefined for a negative, NaN, or Infinity num_turns (#5827)", async () => {
+  for (const badTurns of [-1, Number.NaN, Number.POSITIVE_INFINITY]) {
+    const driver = driverWith({
+      query: queryYielding([{ type: "result", subtype: "success", is_error: false, num_turns: badTurns, result: "done" }]),
+    });
+    const result = await driver.run(task);
+    assert.equal(result.turnsUsed, undefined, `num_turns=${String(badTurns)} should degrade to undefined`);
+  }
+});
+
+test("costUsd degrades to undefined for a negative, NaN, or Infinity total_cost_usd (#5827)", async () => {
+  for (const badCost of [-0.5, Number.NaN, Number.POSITIVE_INFINITY]) {
+    const driver = driverWith({
+      query: queryYielding([{ type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", total_cost_usd: badCost }]),
+    });
+    const result = await driver.run(task);
+    assert.equal(result.costUsd, undefined, `total_cost_usd=${String(badCost)} should degrade to undefined`);
+  }
+});
+
+test("tokensUsed ignores a negative/NaN/Infinity usage field instead of poisoning the sum (#5827)", async () => {
+  // output_tokens is out-of-contract (negative); input_tokens is valid — the sum uses only the valid field.
+  const partiallyBad = driverWith({
+    query: queryYielding([
+      { type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", usage: { input_tokens: 100, output_tokens: -5 } },
+    ]),
+  });
+  assert.equal((await partiallyBad.run(task)).tokensUsed, 100);
+
+  // Both fields out-of-contract → undefined (never a fabricated 0 or a NaN/Infinity sum).
+  const allBad = driverWith({
+    query: queryYielding([
+      { type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", usage: { input_tokens: Number.NaN, output_tokens: Number.POSITIVE_INFINITY } },
+    ]),
+  });
+  assert.equal((await allBad.run(task)).tokensUsed, undefined);
+});
+
 test("tokensUsed sums whichever of input/output tokens IS a real number, when only input is present", async () => {
   const driver = driverWith({
     query: queryYielding([

packages/loopover-engine/test/iterate-loop.test.ts

@@ -262,6 +262,37 @@ test("abandon (cost_ceiling_reached): a maxCostUsd budget breach reports costUsd
   assert.equal(result.finalMeterTotals.costUsd, 6);
 });
 
+// #5827: an out-of-contract usage value from a driver (negative/NaN/Infinity) must NOT let accumulateAttemptUsage
+// throw a RangeError out of runIterateLoopCore before the iteration's decision is logged. The loop must still
+// complete, record its decision, and clamp the poisoned axis to 0 rather than rejecting uncaught.
+test("does not reject uncaught when a driver reports an out-of-contract usage value; still logs a decision (#5827)", async () => {
+  for (const badTurns of [-1, Number.NaN, Number.POSITIVE_INFINITY]) {
+    const { deps, events } = collectingDeps({
+      driver: driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: badTurns as number }),
+    });
+    const result = await runIterateLoop(passingInput({ maxIterations: 3 }), deps);
+
+    assert.equal(result.outcome, "handoff", `turnsUsed=${String(badTurns)} must not crash the loop`);
+    assert.ok(
+      events.some((event) => event.actionClass === "iterate_loop"),
+      "an iterate_loop decision must still be logged despite the out-of-contract usage value",
+    );
+    // The poisoned axis is clamped to 0, not propagated as a negative/NaN/Infinity total.
+    assert.equal(result.finalMeterTotals.turns, 0);
+  }
+});
+
+test("clamps only the out-of-contract axis, preserving a valid axis on the same iteration (#5827)", async () => {
+  const { deps } = collectingDeps({
+    driver: driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: Number.NaN, costUsd: 0.02 }),
+  });
+  const result = await runIterateLoop(passingInput({ maxIterations: 3 }), deps);
+
+  assert.equal(result.outcome, "handoff");
+  assert.equal(result.finalMeterTotals.turns, 0); // NaN clamped
+  assert.equal(result.finalMeterTotals.costUsd, 0.02); // valid axis preserved
+});
+
 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

test/unit/agent-sdk-driver.test.ts

@@ -236,6 +236,43 @@ describe("createAgentSdkCodingAgentDriver", () => {
     expect((await nonNumericFields.run(task)).tokensUsed).toBeUndefined();
   });
 
+  // #5827: a malformed num_turns/total_cost_usd (negative, NaN, Infinity) must degrade to undefined — the same
+  // finite/non-negative discipline cli-subprocess-driver.ts applies — so it never reaches accumulateAttemptUsage,
+  // which throws a RangeError on such input and would reject the whole iterate loop before its decision is logged.
+  it("turnsUsed degrades to undefined for a negative, NaN, or Infinity num_turns (#5827)", async () => {
+    for (const badTurns of [-1, Number.NaN, Number.POSITIVE_INFINITY]) {
+      const driver = driverWith({
+        query: queryYielding([{ type: "result", subtype: "success", is_error: false, num_turns: badTurns, result: "done" }]),
+      });
+      expect((await driver.run(task)).turnsUsed).toBeUndefined();
+    }
+  });
+
+  it("costUsd degrades to undefined for a negative, NaN, or Infinity total_cost_usd (#5827)", async () => {
+    for (const badCost of [-0.5, Number.NaN, Number.POSITIVE_INFINITY]) {
+      const driver = driverWith({
+        query: queryYielding([{ type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", total_cost_usd: badCost }]),
+      });
+      expect((await driver.run(task)).costUsd).toBeUndefined();
+    }
+  });
+
+  it("tokensUsed ignores a negative/NaN/Infinity usage field instead of poisoning the sum (#5827)", async () => {
+    const partiallyBad = driverWith({
+      query: queryYielding([
+        { type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", usage: { input_tokens: 100, output_tokens: -5 } },
+      ]),
+    });
+    expect((await partiallyBad.run(task)).tokensUsed).toBe(100);
+
+    const allBad = driverWith({
+      query: queryYielding([
+        { type: "result", subtype: "success", is_error: false, num_turns: 2, result: "done", usage: { input_tokens: Number.NaN, output_tokens: Number.POSITIVE_INFINITY } },
+      ]),
+    });
+    expect((await allBad.run(task)).tokensUsed).toBeUndefined();
+  });
+
   it("tokensUsed sums whichever of input/output tokens IS a real number, when only input is present", async () => {
     const driver = driverWith({
       query: queryYielding([

test/unit/iterate-loop-usage-guard.test.ts

@@ -0,0 +1,86 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  parseFocusManifest,
+  runIterateLoop,
+  type AttemptLogEvent,
+  type CodingAgentDriver,
+  type CodingAgentDriverResult,
+  type IterateLoopDeps,
+  type IterateLoopInput,
+  type SelfReviewContext,
+  type SelfReviewSlopAssessment,
+} from "../../packages/loopover-engine/src/index";
+
+// Codecov measures packages/loopover-engine/src/** via the vitest suite (NOT the engine's own node --test
+// suite), so runIterateLoop's #5827 finiteNonNegativeUsage guard needs coverage here, importing from src.
+
+const REPO = { fullName: "acme/widgets", owner: "acme", name: "widgets", isInstalled: true, isRegistered: true, isPrivate: false };
+const noopSlop: SelfReviewSlopAssessment = { slopRisk: 0, band: "clean", findings: [] };
+
+function baseReviewContext(): SelfReviewContext {
+  return {
+    manifest: parseFocusManifest({ gate: { duplicates: "block", linkedIssue: "advisory" } }),
+    repo: REPO,
+    issues: [{ repoFullName: "acme/widgets", number: 7, title: "Uploads should retry on 5xx", state: "open", labels: [], linkedPrs: [] }],
+    pullRequests: [],
+  };
+}
+
+function passingInput(overrides: Partial<IterateLoopInput> = {}): IterateLoopInput {
+  return {
+    attemptId: "attempt-1",
+    workingDirectory: "/tmp/attempt-1",
+    acceptanceCriteriaPath: "/tmp/attempt-1/acceptance-criteria.json",
+    instructions: "Add retry to the upload client",
+    mode: "live",
+    maxIterations: 3,
+    maxTurnsPerIteration: 20,
+    repoFullName: "acme/widgets",
+    contributorLogin: "miner1",
+    title: "Add retry to the upload client",
+    reviewContext: baseReviewContext(),
+    rejectionSignaled: false,
+    body: "Closes #7",
+    linkedIssues: [7],
+    ...overrides,
+  };
+}
+
+function driverReturning(result: CodingAgentDriverResult): CodingAgentDriver {
+  return { async run() { return result; } };
+}
+
+function collectingDeps(driver: CodingAgentDriver): { deps: IterateLoopDeps; events: AttemptLogEvent[] } {
+  const events: AttemptLogEvent[] = [];
+  const deps: IterateLoopDeps = {
+    driver,
+    runSlopAssessment: () => noopSlop,
+    appendAttemptLogEvent: (event) => {
+      events.push(event);
+    },
+  };
+  return { deps, events };
+}
+
+describe("runIterateLoop usage guard (#5827)", () => {
+  it("does not reject uncaught when a driver reports an out-of-contract usage value; still logs a decision", async () => {
+    for (const badTurns of [-1, Number.NaN, Number.POSITIVE_INFINITY]) {
+      const { deps, events } = collectingDeps(driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: badTurns as number }));
+      const result = await runIterateLoop(passingInput(), deps);
+
+      expect(result.outcome).toBe("handoff");
+      expect(events.some((event) => event.actionClass === "iterate_loop")).toBe(true);
+      expect(result.finalMeterTotals.turns).toBe(0); // poisoned axis clamped, not propagated
+    }
+  });
+
+  it("clamps only the out-of-contract axis, preserving a valid axis on the same iteration", async () => {
+    const { deps } = collectingDeps(driverReturning({ ok: true, changedFiles: ["src/upload.ts"], summary: "x", turnsUsed: Number.NaN, costUsd: 0.02 }));
+    const result = await runIterateLoop(passingInput(), deps);
+
+    expect(result.outcome).toBe("handoff");
+    expect(result.finalMeterTotals.turns).toBe(0);
+    expect(result.finalMeterTotals.costUsd).toBe(0.02);
+  });
+});