fix(miner): stop hooks check --tool/--input consuming an adjacent flag as their value - #6062
Conversation
…g as their value
parseDenyCheckArgs took the next token unconditionally for --tool/--name
and --input, so `hooks check --tool --input '{}'` silently set
options.tool = "--input" and then fell through to the generic usage string
on the following token, instead of the specific "Missing value for --tool."
error every sibling flag parser in this package gives for the same mistake.
Adds the same `!value || value.startsWith("-")` guard already used by
attempt-cli.js and claim-ledger-cli.js to both value-consuming branches,
rejecting --input's value before it is JSON-parsed. Messages reuse this
file's existing conventions ("Missing value for --tool." /
"Missing value for --input.").
Closes JSONbored#5833
|
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 #6062 +/- ##
==========================================
- Coverage 95.22% 95.22% -0.01%
==========================================
Files 594 594
Lines 46996 46974 -22
Branches 15015 15015
==========================================
- Hits 44754 44732 -22
Misses 1493 1493
Partials 749 749
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 08:18:32 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
parseDenyCheckArgsinpackages/loopover-miner/lib/deny-check.jsconsumed the next token unconditionally for its--tool/--nameand--inputbranches, with no check that the consumed token is itself a flag. Soloopover-miner hooks check --tool --input '{}'silently setoptions.tool = "--input", and the loop's next iteration hit the literal'{}'— which matches no known flag and doesn't start with-— falling through to the genericDENY_CHECK_USAGEstring instead of the specific, actionableMissing value for --tool.error this file already uses for the same class of mistake.Every sibling flag parser in this package already guards this:
attempt-cli.js(if (!value || value.startsWith("-")) ...) andclaim-ledger-cli.js(four instances). This PR adds that same guard to both value-consuming branches, rejecting--input's value before it is JSON-parsed. Messages reuse this file's existing conventions (Missing value for --tool./Missing value for --input., the latter already emitted byparseToolInputfor a missing value).Closes #5833
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #5833).Validation
git diff --check— cleannpm run actionlint— no workflow changesnpm run typecheck— passes (whole project)npm run test:coverage— both arms of both new guards are exercised (!valuevia--tool/--tool Write --inputwith no value;value.startsWith("-")via--tool --input {},--name --json, and--tool Write --input --json), and the pass-through arm by the existing valid-parse tests, socodecov/patchon the diff is 100% branch-counted.test/unit/miner-cli-deny-check.test.tspasses 5/5; the new test fails on the pre-fix code and passes after, with all four pre-existing tests unchanged and green on both.npm run test:workers— unaffected (no worker changes)npm run build:miner— passes (node --checkover the miner CLI incl.deny-check.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--tool's value, as--name's value, and as--input's value, plus both missing-value cases, each asserting the specific "Missing value" error rather than the generic usage fallbackIf any required check was skipped, explain why:
packages/loopover-miner/lib/deny-check.jsplus its test. It introduces no workflow, MCP, UI, OpenAPI, or dependency changes. 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
hooks checknow reports the same specific, actionable error the rest of the package's flag parsers already give.