fix(engine): reject non-finite agent-sdk usage so it can't crash the iterate loop (#5827) - #6078
Conversation
…iterate loop (JSONbored#5827) agent-sdk-driver.ts extracted num_turns/total_cost_usd (and the usage token fields) with a bare `typeof === "number"` check, so a malformed Agent SDK result message (num_turns: -1, NaN, Infinity) passed an out-of-contract value straight through. That value fed accumulateAttemptUsage in iterate-loop.ts's runIterateLoopCore, which deliberately throws a RangeError on negative/non-finite input — and that call sits outside the loop's driver/self-review try/catch, so the throw rejected runIterateLoopCore before any decision was logged, violating the loop's "every iteration's decision is recorded before returning" guarantee. Harden the driver's usage extraction with a finiteNonNegativeNumber helper mirroring cli-subprocess-driver.ts's existing one — an invalid value degrades to undefined (the driver's "field absent" contract). Add a matching call-site clamp in runIterateLoopCore so no current or future driver can crash the loop instead of being governed. attempt-metering.ts's throw-on-invalid contract is unchanged; this only stops feeding it invalid input. Regression tests cover negative/NaN/Infinity num_turns, total_cost_usd, and token fields, and the loop completing (decision logged, poisoned axis clamped) rather than rejecting uncaught. Because Codecov measures the engine src via the vitest suite (not the engine's node --test suite), the driver + loop guards are covered from both test surfaces.
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6078 +/- ##
===========================================
+ Coverage 78.14% 95.23% +17.09%
===========================================
Files 595 595
Lines 46998 47010 +12
Branches 15015 15010 -5
===========================================
+ Hits 36726 44772 +8046
+ Misses 8132 1493 -6639
+ Partials 2140 745 -1395
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-15 08:53:14 UTC
⏸️ Suggested Action - Manual Review Review summary Nits — 4 non-blocking
Flagged checks (non-blocking)
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
Closes #5827.
packages/loopover-engine/src/miner/agent-sdk-driver.tsextracted the driver's usage fields from the SDKresult message with a bare
typeof === "number"check:That does not reject
NaN,Infinity, or a negative number the waycli-subprocess-driver.ts'sfiniteNonNegativeNumberalready does for the analogous untrusted CLI-stdout fields. A malformed Agent SDKresult message (
num_turns: -1,NaN, …) therefore reachedaccumulateAttemptUsage(
attempt-metering.ts) unguarded. That primitive deliberately throws aRangeErroron negative/non-finiteinput — and the call site in
runIterateLoopCore(iterate-loop.ts) sits outside the loop's driver/self-review try/catch blocks, so the throw rejected the whole
runIterateLoopCorepromise before any decisionwas logged, violating the loop's documented "every iteration's decision is recorded before this function returns
control" guarantee (worse than a normal governed rejection — the attempt's outcome is never logged at all).
tokensFromResultMessagehad the sametypeof-only gap.Fix (root cause + defense in depth):
agent-sdk-driver.ts: add a localfiniteNonNegativeNumberhelper mirroringcli-subprocess-driver.ts's ofthe same name, and use it for
turnsUsed,costUsd, and thetokensFromResultMessagefields. An invalid valuedegrades to
undefined— the driver's existing "field absent" contract — instead of propagating downstream.iterate-loop.ts: clamp the values feedingaccumulateAttemptUsagewith a localfiniteNonNegativeUsagehelper (invalid → 0), so no current or future driver can crash the loop instead of being governed.
attempt-metering.ts's throw-on-invalid contract is unchanged (its own direct callers/tests still rely on it) —this fix is about never handing it invalid input, not weakening the primitive.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:cigate plusnpm audit --audit-level=moderate(0 vulnerabilities). The new helper branches (valid vs. out-of-contract) are covered on both arms. Notepackages/loopover-engine/src/**is measured by Codecov through the vitest suite (not the engine's ownnode --testsuite), so the driver + loop guards are tested from both surfaces: enginenode --test(test/agent-sdk-driver.test.ts,test/iterate-loop.test.ts) and vitest (test/unit/agent-sdk-driver.test.ts, newtest/unit/iterate-loop-usage-guard.test.ts). The regression tests were confirmed to FAIL against the pre-fix source.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Notes on the Safety boxes: this is a backend-only hardening of a miner-runtime driver + orchestrator — no UI, API/OpenAPI, or auth/CORS/session surface. The applicable negative-path testing is the drivers' own out-of-contract-input paths, which the new tests cover (negative/NaN/Infinity values degrade safely; the loop still records its decision). The fix strengthens a fail-closed guarantee rather than relaxing it.
UI Evidence
Not applicable — backend-only change to the miner engine; no visible UI, frontend, docs, or extension surface.
Notes
packages/loopover-engine/test/agent-sdk-driver.test.tsand its vitest mirrortest/unit/agent-sdk-driver.test.tscovernum_turns/total_cost_usd= -1/NaN/Infinity →undefined, and a negative/NaN/Infinity token field being ignored rather than poisoning the sum.packages/loopover-engine/test/iterate-loop.test.tsand its vitest mirrortest/unit/iterate-loop-usage-guard.test.tscover the loop still completing (decision logged, poisoned axis clamped to 0, valid axis preserved) rather than rejecting uncaught. All new tests were verified to fail on the pre-fix source.