Skip to content

fix(mcp): treat a bare help positional as --help in parseOptions (#6257) - #6362

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nickmopen:fix/mcp-bare-help-positional
Jul 16, 2026
Merged

fix(mcp): treat a bare help positional as --help in parseOptions (#6257)#6362
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nickmopen:fix/mcp-bare-help-positional

Conversation

@nickmopen

Copy link
Copy Markdown
Contributor

Summary

loopover-mcp decision-pack help (and repo-decision help / review-pr help) threw a confusing "Pass --login…" error instead of printing usage, because a bare help positional (no dashes) was silently dropped. These three commands are dispatched with a parsed options object and only check options.help === true, but parseOptions skips any non--- argument — so options.help was never set. The other four command-dispatch siblings (lint-pr-text/validate-config/slop-risk/issue-slop) receive raw args and already special-case args[0] === "help".

Fix

In parseOptions, set options.help = true for a standalone help positional — symmetric with how --help already resolves to options.help = true, and the DRY equivalent of the sibling args[0] === "help" special-case that fixes all three option-consuming commands at once:

if (!arg?.startsWith("--")) {
  if (arg === "help") options.help = true;
  continue;
}

A help consumed as a --key value value (e.g. --query help) is skipped via the --key value branch's index += 1 before ever reaching this check, so only a genuinely standalone help positional is affected. The four already-correct commands are unchanged (they check args[0] before parsing).

loopover-mcp decision-pack help now prints usage; verified manually and by tests.

Scope / Validation / Safety

  • Conventional Commit title; focused; Closes #6257.
  • Added a bare-help test for all three commands (decision-pack/repo-decision/review-pr), asserting they print Usage: and not the Pass --login error, alongside their existing --help tests.
  • All 172 mcp-cli tests pass (24 files) — no regression to any other command that shares parseOptions; npm run build:mcp (syntax) green.
  • No behavior change for the already-correct commands or for help passed as a flag value; no secrets; no apps//site//CNAME changes.

packages/loopover-mcp/bin/** isn't in Codecov's collected scope (only src/**, packages/loopover-engine/src/**, packages/loopover-miner/lib/** are); the fix is verified by the test/unit/mcp-cli-* suite that invokes the real bin.

Closes #6257

…SONbored#6257)

decision-pack/repo-decision/review-pr are dispatched with parsed `options` and
only check `options.help === true`, but parseOptions dropped any non-`--` arg, so
a dashless `loopover-mcp decision-pack help` never set options.help and fell
through to a confusing "Pass --login…" error instead of printing usage — while
the raw-args commands (lint-pr-text/validate-config/slop-risk/issue-slop) already
special-cased `args[0] === "help"`.

Set options.help = true for a standalone `help` positional, symmetric with how
`--help` already resolves to options.help. A `help` consumed as a `--key value`
value is skipped before this check, so only a bare positional is affected. Adds a
bare-`help` test for all three commands alongside their existing --help tests.
@nickmopen
nickmopen requested a review from JSONbored as a code owner July 16, 2026 04:24
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (d5d8da8) to head (b562a14).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6362   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         598      598           
  Lines       47202    47202           
  Branches    15022    15022           
=======================================
  Hits        45128    45128           
  Misses       1290     1290           
  Partials      784      784           
Flag Coverage Δ
shard-1 44.14% <ø> (ø)
shard-2 36.51% <ø> (+0.12%) ⬆️
shard-3 32.54% <ø> (+<0.01%) ⬆️
shard-4 34.68% <ø> (+0.06%) ⬆️
shard-5 31.59% <ø> (+<0.01%) ⬆️
shard-6 44.86% <ø> (-0.02%) ⬇️

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

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 05:13:07 UTC

3 files · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a small, targeted fix in parseOptions that sets options.help = true when a bare `help` positional is encountered (mirroring the existing --help handling), fixing the confusing '--login' error for decision-pack/repo-decision/review-pr help. The fix is correctly scoped: since --key value consumption does `index += 1` before reaching this branch, a help value passed to a flag (e.g. --query help) won't be misinterpreted as a standalone help. Tests are added for all three affected commands, asserting Usage output and absence of the --login error, and CI is green across all 172 mcp-cli tests.

Nits — 4 non-blocking
  • The long comment block above the fix (loopover-mcp.js) is fairly verbose for a two-line change; could be trimmed to one line per repo convention.
  • No test added for the `--key value` edge case explicitly mentioned in the PR description (e.g. `--query help`) to guard against regression of that exclusion logic.
  • Consider adding a single regression test asserting `--query help` (or similar) still sets options.query = 'help' and does not set options.help, to lock in the described `index += 1` exclusion behavior.
  • packages/loopover-mcp/bin/loopover-mcp.js: could shorten the inline comment to reference fix(mcp): bare help positional (no dashes) is silently swallowed for decision-pack/repo-decision/review-pr #6257 without restating the whole rationale already in the PR description.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6257
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low 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: 141 registered-repo PR(s), 88 merged, 9 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nickmopen; Gittensor profile; 141 PR(s), 9 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: minor
Linked issue satisfaction

Addressed
The fix modifies parseOptions to set options.help=true for a bare 'help' positional, which addresses all three affected commands (decision-pack, repo-decision, review-pr) at the root cause without touching the already-correct raw-args commands, and adds tests for all three as required.

Review context
  • Author: nickmopen
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 141 PR(s), 9 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
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://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.

  • 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 2e33517 into JSONbored:main Jul 16, 2026
16 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 16, 2026
12 tasks
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(mcp): bare help positional (no dashes) is silently swallowed for decision-pack/repo-decision/review-pr

1 participant