fix(miner): honor --json on the discover --dry-run failure path - #6051
Conversation
discover's --dry-run catch block wrote a plain-text line to stderr and
returned 2 regardless of --json, unlike the non-dry-run catch 20 lines
later (and the second failure path further down) which both route through
reportCliFailure(parsed.json, describeCliError(error)). A caller scripting
against `loopover-miner discover ... --dry-run --json` got unparseable
plain text instead of the {ok:false,error} object every other failure path
in this CLI emits. Route the dry-run catch through the same shared helper.
describeCliError is byte-identical to the previous inline expression, so
the non-json path (stderr text, exit 2) is unchanged; adds a regression
test asserting the --dry-run --json failure prints a parseable
{ok:false,error} object.
Closes JSONbored#5830
|
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 #6051 +/- ##
==========================================
+ Coverage 95.17% 95.22% +0.05%
==========================================
Files 592 593 +1
Lines 46960 46971 +11
Branches 15007 15015 +8
==========================================
+ Hits 44693 44729 +36
+ Misses 1511 1493 -18
+ Partials 756 749 -7
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-15 07:56:16 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 2 non-blocking
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://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
loopover-miner discoversupports--jsonfor scripted callers, and its non-dry-run failure path routes through the sharedreportCliFailure(parsed.json, describeCliError(error))helper, which emits a{ ok: false, error }object on stdout when--jsonis set. The--dry-runpath (added in #4847) has its own separate catch block that bypasses that helper entirely — it always writes plain text to stderr and returns2regardless of--json:So
loopover-miner discover <owner/repo> --dry-run --jsonon a failure (GitHub API error, invalid target) prints an unparseable plain-text line instead of the JSON object every other failure path in this CLI — including this same command's non-dry-run catch 20 lines later — honors. The fix routes the dry-run catch through the samereportCliFailure/describeCliErrorpair (already imported in this file).Closes #5830
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #5830).Validation
git diff --check— cleannpm run actionlint— no workflow changesnpm run typecheck— passes (whole project)npm run test:coverage— the changed catch line is exercised by the new--dry-run --jsonregression test (JSON branch) and the existing--dry-runnon-json failure tests (stderr branch), socodecov/patchon the diff is 100%.test/unit/miner-discover-cli.test.tspasses 42/42; the new test fails on the pre-fix code (which usedconsole.error) and passes after.npm run test:workers— unaffected (no worker changes)npm run build:miner— passes (node --checkover the miner CLI incl.discover-cli.js)npm run test:miner-pack— passes (package dry-run ok)npm run build:mcp/test:mcp-pack— unaffected (no MCP changes)npm run ui:openapi:check/ui:lint/ui:typecheck/ui:build— unaffected (no UI/OpenAPI changes)npm audit --audit-level=moderate— no dependency changes--dry-run --jsonfailure regression test asserting a parseable{ ok: false, error }object; the existing non-json failure tests (Error and non-Error thrown values) continue to pass unchangedIf any required check was skipped, explain why:
packages/loopover-miner/lib/discover-cli.jsplus its test.describeCliErroris byte-identical to the previous inlineerror instanceof Error ? error.message : String(error)expression, so the non---jsonpath (stderr text, exit code 2) is behavior-preserving. I additionally rannpm run engine-parity:drift-checkand the backend drift checks (docs,manifest,command-reference,selfhost env-reference), all green.Safety
UI Evidencesection — N/A (no UI/frontend/docs/extension changes).Notes
--dry-runvs non-dry-run) now emit identical output for the same error and flags, so a scripted caller no longer gets different, sometimes-unparseable output depending on which nearly-identical branch it hits.