fix(miner): index-parse deny-hooks --history and route failures through the shared CLI error contract - #9886
Conversation
…gh the shared CLI error contract
The deny-hooks CLI hand-rolled its arg parsing with args.filter((a) => !a.startsWith("--")),
so --history's value (a path, which does not start with --) leaked into the positional list.
Placed before the repo — deny-hooks refresh --history h.json acme/widgets — it stole the
repoFullName slot and the command operated on a nonsense repo identifier.
Replace the filter with an index-based parseDenyHooksArgs loop (mirroring parseRepoIdentifierArgs
in portfolio-queue-cli) that consumes --history <value> as a flag+value pair, rejects a missing
or flag-like value with the usage error, and rejects any unknown -prefixed token with
Unknown option: <token>. Route every failure path through reportCliFailure so --json emits a
parseable { ok: false, error } object on stdout, and return the shared default exit code 2
everywhere instead of the old bespoke 1 on the catch-all.
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 18:59:13 UTC
Review summary Nits — 5 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9886 +/- ##
==========================================
+ Coverage 79.26% 79.28% +0.02%
==========================================
Files 281 282 +1
Lines 58474 58531 +57
Branches 6768 6783 +15
==========================================
+ Hits 46351 46408 +57
Misses 11840 11840
Partials 283 283
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fix(miner): index-parse deny-hooks --history and route failures through the shared CLI error contract
The deny-hooks CLI hand-rolled its arg parsing with args.filter((a) => !a.startsWith("--")),
so --history's value (a path, which does not start with --) leaked into the positional list.
Placed before the repo — deny-hooks refresh --history h.json acme/widgets — it stole the
repoFullName slot and the command operated on a nonsense repo identifier.
Replace the filter with an index-based parseDenyHooksArgs loop (mirroring parseRepoIdentifierArgs
in portfolio-queue-cli) that consumes --history as a flag+value pair, rejects a missing
or flag-like value with the usage error, and rejects any unknown -prefixed token with
Unknown option: . Route every failure path through reportCliFailure so --json emits a
parseable { ok: false, error } object on stdout, and return the shared default exit code 2
everywhere instead of the old bespoke 1 on the catch-all.
Closes #9687