Skip to content

fix(miner): honor --json on purge arg-parse errors - #6033

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-5915
Jul 15, 2026
Merged

fix(miner): honor --json on purge arg-parse errors#6033
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-5915

Conversation

@nghetienhiep

Copy link
Copy Markdown
Contributor

Summary

loopover-miner purge was already --json-aware on its success/dry-run/partial-failure outcomes, but runPurge's top-level arg-parse failure path used a bare console.error(...); return 2 instead of the shared reportCliFailure/argsWantJson contract (cli-error.js) that the rest of the CLI suite (attempt-cli, claim-ledger-cli, discover-cli, etc.) already uses.

Scripted callers that always pass --json and expect {ok:false,error} regardless of outcome got plain stderr text instead on a parse failure (e.g. purge --repo bad-value --json).

Behavior

  • With --json: an arg-parse failure now prints { "ok": false, "error": "..." } on stdout and exits 2.
  • Without --json: unchanged — still prints the human-readable message to stderr and exits 2.

Tests

Added a regression test in test/unit/miner-purge-cli.test.ts covering the arg-parse error WITH --json, asserting the JSON envelope shape, that nothing is written to stderr, and the exit code. The pre-existing test right above it already covers the non---json stderr arm.

npx vitest run test/unit/miner-purge-cli.test.ts --coverage --coverage.include='packages/loopover-miner/lib/purge-cli.js' → 100% statements/branches/functions/lines on the changed file.

Closes #5915

runPurge's arg-parse failure path used a bare console.error/return 2
instead of the shared reportCliFailure/argsWantJson contract, so
`purge --json` on a bad --repo value printed plain stderr text
instead of the documented {ok:false,error} envelope.

Closes JSONbored#5915
@nghetienhiep
nghetienhiep requested a review from JSONbored as a code owner July 15, 2026 07:07
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.16%. Comparing base (0c8c4c4) to head (ca53d8f).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #6033       +/-   ##
===========================================
+ Coverage   78.84%   95.16%   +16.31%     
===========================================
  Files         592      592               
  Lines       46949    46950        +1     
  Branches    15003    15003               
===========================================
+ Hits        37019    44680     +7661     
+ Misses       7754     1512     -6242     
+ Partials     2176      758     -1418     
Flag Coverage Δ
shard-1 43.92% <0.00%> (?)
shard-2 35.58% <0.00%> (-0.95%) ⬇️
shard-3 32.74% <0.00%> (?)
shard-4 31.95% <0.00%> (+<0.01%) ⬆️
shard-5 32.26% <0.00%> (-0.27%) ⬇️
shard-6 44.94% <100.00%> (+0.32%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-miner/lib/purge-cli.js 100.00% <100.00%> (ø)

... and 210 files with indirect coverage changes

@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 15, 2026
@loopover-orb

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-15 07:22:54 UTC

2 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR fixes runPurge's arg-parse error path to use the shared reportCliFailure/argsWantJson contract instead of a bare console.error, matching the established pattern used elsewhere in the CLI suite (e.g. runCli's unknown-command handling in cli.js). The change is minimal, correctly targeted, and closes issue #5915 as claimed. The added test asserts the exact JSON envelope shape and exit code, mirroring the existing convention seen in miner-cli.test.ts's analogous #4836 regression test.

Nits — 2 non-blocking
  • The test only checks `log.mock.calls[0]` without asserting `log` was called exactly once, so a stray extra console.log elsewhere in the parse path would go unnoticed.
  • Consider a one-line JSDoc or comment noting that this arg-parse path now shares the cli-error.js contract, to prevent regression if someone re-adds a bare console.error later.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5915
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 28 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetienhiep; Gittensor profile; 56 PR(s), 0 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The diff replaces the raw console.error/return 2 with reportCliFailure(argsWantJson(args), parsed.error) exactly as requested, and adds a regression test asserting the {ok:false,error} JSON envelope with --json.

Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 396a875 into JSONbored:main Jul 15, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AMS: purge-cli.js arg-parse error path bypasses the --json contract

1 participant