Skip to content

fix(engine): agent-sdk-driver usage fields can crash iterate-loop's budget accumulator #5827

Description

@JSONbored

Context

packages/loopover-engine/src/miner/iterate-loop.ts's runIterateLoopCore folds each iteration's driver usage into the running meter total:

tracker.totals = accumulateAttemptUsage(tracker.totals, {
  tokens: driverResult.tokensUsed ?? 0,
  turns: driverResult.turnsUsed ?? 0,
  wallClockMs: iterationElapsedMs,
  costUsd: driverResult.costUsd ?? 0,
});

This call (around line 335) sits outside the loop's two try/catch blocks (around deps.driver.run() and runSelfReview()), and it wasn't there before PR #5437 ("wire attempt-metering.ts into the iterate loop for a real mid-attempt budget abort", which closed #5395). accumulateAttemptUsage in packages/loopover-engine/src/miner/attempt-metering.ts deliberately throws a RangeError via assertNonNegativeFiniteNumber when any usage field is negative or non-finite — its own test (test/attempt-metering.test.ts) documents this as intentional, so a caller can never silently poison the running total.

The problem is the value that now feeds it. packages/loopover-engine/src/miner/agent-sdk-driver.ts (~line 178) sets:

const turnsUsed = typeof resultMessage?.num_turns === "number" ? resultMessage.num_turns : undefined;
...
const costUsd = typeof resultMessage?.total_cost_usd === "number" ? resultMessage.total_cost_usd : undefined;

This only checks typeof === "number" — it does not reject NaN, Infinity, or a negative number the way packages/loopover-engine/src/miner/cli-subprocess-driver.ts's finiteNonNegativeNumber helper (~line 141) already does for the analogous untrusted-subprocess-output fields (cost/token counts parsed from CLI stdout). A malformed or buggy Agent SDK result message (e.g. num_turns: -1 or NaN) therefore reaches accumulateAttemptUsage unguarded, throws, and rejects the whole runIterateLoopCore promise — bypassing the module's own documented "fail closed" guarantees (its header comment states every iteration's decision is recorded via logDecision/safeAppendAttemptLogEvent before the function returns control). An uncaught throw here means an attempt's outcome is never logged at all, which is worse than a normal governed rejection.

Neither packages/loopover-engine/test/iterate-loop.test.ts nor packages/loopover-engine/test/agent-sdk-driver.test.ts currently exercises a negative/NaN num_turns or total_cost_usd from a driver result.

Requirements

  • In agent-sdk-driver.ts, normalize turnsUsed and costUsd (and tokensUsed, via tokensFromResultMessage, if it has the same gap) using the same finite/non-negative validation pattern already established in cli-subprocess-driver.ts's finiteNonNegativeNumber — an invalid value should degrade to undefined (the driver's existing "field absent" contract), not propagate a NaN/negative number downstream.
  • In iterate-loop.ts, the accumulateAttemptUsage call must not be able to throw an uncaught error out of runIterateLoopCore: either wrap it so a thrown RangeError is treated the same way the surrounding driver/self-review try/catch blocks treat a failure (a recorded, logged abort — not a bare promise rejection), or ensure it can no longer throw once the driver-level fix above is in place, with an explicit regression test proving it.
  • Preserve attempt-metering.ts's existing "throw on invalid input" contract for its own direct callers/tests — this fix is about never handing it invalid input from the Agent SDK path, not weakening the primitive itself.

Deliverables

  • agent-sdk-driver.ts: finite/non-negative normalization for turnsUsed and costUsd (and tokensUsed if applicable), mirroring cli-subprocess-driver.ts's finiteNonNegativeNumber.
  • iterate-loop.ts: the accumulateAttemptUsage call site can no longer let an uncaught RangeError escape runIterateLoopCore without the loop's normal decision-logging/abort path running first.
  • Regression tests in test/agent-sdk-driver.test.ts covering a result message with num_turns: -1, num_turns: NaN, and total_cost_usd: -1/NaN.
  • Regression test in test/iterate-loop.test.ts covering a driver result with an out-of-contract usage value, asserting the loop still records a decision/log entry rather than rejecting uncaught.

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov; target 99%+ patch coverage on every changed line and branch, including both the valid and invalid-input arms of the new normalization and the loop's new guarded path. Add the regression tests listed above — this is exactly the "regression test for every fix" case the repo's contributing guide requires.

Expected Outcome

A malformed or out-of-range num_turns/total_cost_usd value from the Agent SDK's result message can no longer crash runIterateLoopCore with an uncaught RangeError; the iterate loop's documented "every iteration's decision is recorded before this function returns control" guarantee holds even under a driver-level data anomaly, matching the validation discipline cli-subprocess-driver.ts already applies to the equivalent CLI-subprocess path.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions