Skip to content

fix(miner): discover-cli --dry-run --json breaks the JSON error contract on failure #5830

Description

@JSONbored

Context

packages/loopover-miner/lib/discover-cli.js implements loopover-miner discover, which supports --json for scripted/machine callers. On the non-dry-run path, a failure during the fetch/search/rank flow is reported via the shared reportCliFailure(parsed.json, describeCliError(error)) helper (imported from ./cli-error.js), which correctly emits a {ok:false,error} JSON object when --json was passed and a human-readable line otherwise:

} catch (error) {
  return reportCliFailure(parsed.json, describeCliError(error));
}

The --dry-run path (added for #4847, ~20 lines earlier in the same function) has its own, separate try/catch around the same fetch/search/rank calls, but its catch block bypasses reportCliFailure entirely and always writes plain text to stderr regardless of --json:

try {
  const fanOut = parsed.search !== null ? await searchTargets(...) : await fetchTargets(...);
  ...
} catch (error) {
  console.error(error instanceof Error ? error.message : String(error));
  return 2;
}

This means loopover-miner discover <owner/repo> --dry-run --json on a failure (e.g. a GitHub API error, an invalid target) prints a plain-text line instead of a JSON object, breaking the {ok:false,error} contract that every other failure path in this CLI (including this same command's non-dry-run path, 20 lines later) honors. A scripted caller parsing --json output for discover --dry-run would get unparseable output on this specific error path.

Requirements

  • The --dry-run catch block must route through reportCliFailure(parsed.json, describeCliError(error)), identically to the non-dry-run catch block in the same function, so the JSON contract holds on both paths.
  • No behavior change for the non---json case beyond what reportCliFailure already does for the non-dry-run path (i.e. the exit code and message format should match the existing failure-reporting convention, not be a new one).

Deliverables

  • discover-cli.js's dry-run catch block updated to use reportCliFailure/describeCliError instead of a bare console.error.
  • Regression test in test/unit/miner-discover-cli.test.ts (or the closest existing dry-run test in that file) covering --dry-run --json with a thrown/rejected fetch, asserting the output is valid JSON matching the {ok:false,error} shape used elsewhere in this CLI.

Test Coverage Requirements

packages/loopover-miner/lib/** is in Codecov's coverage.include; target 99%+ patch coverage on the changed lines and both branches (--json and non---json) of the fixed catch block, plus the regression test above.

Expected Outcome

loopover-miner discover ... --dry-run --json emits a well-formed {ok:false,error} JSON object on failure, exactly like the non-dry-run path already does — a scripted caller no longer gets unparseable plain text depending on which of the two nearly-identical failure paths it happens to hit.

Links & Resources

  • packages/loopover-miner/lib/discover-cli.js (dry-run catch block vs. the non-dry-run catch block ~20 lines later in the same function)
  • packages/loopover-miner/lib/cli-error.js (reportCliFailure, describeCliError, argsWantJson — the shared contract)
  • test/unit/miner-discover-cli.test.ts (existing coverage to extend)

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