fix(miner): surface verification payload in attempt-cli JSON for verification_failed - #9372
Conversation
|
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 #9372 +/- ##
==========================================
+ Coverage 75.38% 75.50% +0.12%
==========================================
Files 275 276 +1
Lines 58023 58314 +291
Branches 6181 6278 +97
==========================================
+ Hits 43739 44030 +291
Misses 14014 14014
Partials 270 270
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-27 14:55:02 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed 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://loopover.ai/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.
|
What
attempt-runner.ts'sAttemptResultunion includes averification_failedoutcome (added by #8807'starget-repo verification gate):
But
attempt-cli.ts'srunAttemptbuilt itsfinalResultby conditionally spreading the siblingoptional fields —
reason,decision,spec,execResult— via...("field" in result ? { field: result.field } : {}), with no equivalent forverification.So on a
verification_failedoutcome the CLI's--jsonoutput (andoptions.onResultpayload) silentlydropped the verification detail.
Change
finalResultnow spreadsverificationwith the same conditional-spread pattern already used for itssiblings:
...("verification" in result ? { verification: result.verification } : {}).AttemptCliResult'sattempt_${RunMinerAttemptResult["outcome"]}union member gains an optionalverification?: unknownfield, matching the shape of the other optional fields already there(
execResult?: unknown).attempt-runner.ts'sAttemptResult/verification_failedoutcome is untouched — this only changesattempt-cli.ts's consumption of it. No other outcome's fields change.Validation
test/unit/miner-attempt-cli.test.ts: withrunMinerAttemptmocked to return averification_failedoutcome carrying averificationpayload,runAttempt's--jsonoutput hasoutcome: "attempt_verification_failed"and the exactverificationobject — dropped before this fix.npx vitest run test/unit/miner-attempt-cli.test.ts -t "verification payload"passes. Both branches ofthe new conditional spread are covered (this test drives the present side; the existing
submitted/abandon/handoff outcome tests drive the absent side). Every changed line is line+branch covered.
(Three unrelated tests in this file —
#5185 broken appendAttemptLogEvent,unexpected allocator failure,#8808 null resolution— fail identically on cleanmainon this Windows dev box, i.e.pre-existing platform-baseline failures, not touched by this change.)
Closes #9328