From b562a147339c299c4508ff441ccbfd3af81dcd70 Mon Sep 17 00:00:00 2001 From: Nick M Date: Wed, 15 Jul 2026 23:24:09 -0500 Subject: [PATCH] fix(mcp): treat a bare `help` positional as --help in parseOptions (#6257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/loopover-mcp/bin/loopover-mcp.js | 10 +++++++++- test/unit/mcp-cli-packets.test.ts | 12 ++++++++++++ test/unit/mcp-cli-review-pr.test.ts | 6 ++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 100084c6b1..582427b187 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -2898,7 +2898,15 @@ function parseOptions(args) { options.json = true; continue; } - if (!arg?.startsWith("--")) continue; + if (!arg?.startsWith("--")) { + // A bare `help` positional means the same thing as `--help` (#6257): the option-consuming commands + // (decision-pack/repo-decision/review-pr) only check `options.help === true`, so without this a + // dashless `loopover-mcp decision-pack help` fell through to a confusing "Pass --login…" error instead + // of printing usage — while the raw-args commands (lint-pr-text etc.) already special-cased it. A `help` + // consumed as a `--key value` value is skipped via `index += 1` below, so only a STANDALONE `help` here. + if (arg === "help") options.help = true; + continue; + } // Support the inline `--key=value` form (e.g. `--format=table`) alongside the space-separated // `--key value` form; splitting here keeps every existing space-separated option unchanged (#2231). const equals = arg.indexOf("="); diff --git a/test/unit/mcp-cli-packets.test.ts b/test/unit/mcp-cli-packets.test.ts index 0840585ad5..0bfd673e62 100644 --- a/test/unit/mcp-cli-packets.test.ts +++ b/test/unit/mcp-cli-packets.test.ts @@ -80,6 +80,12 @@ describe("loopover-mcp CLI — packets", () => { expect(help).toMatch(/contributor decision pack/); }); + it("prints decision-pack help for a bare `help` positional too, not a --login error (#6257)", () => { + const help = run(["decision-pack", "help"]); + expect(help).toMatch(/Usage: loopover-mcp decision-pack/); + expect(help).not.toMatch(/Pass --login/); + }); + it("prints repo-decision help without requiring --login/--repo or making a network call", () => { const help = run(["repo-decision", "--help"]); expect(help).toMatch(/Usage: loopover-mcp repo-decision/); @@ -87,6 +93,12 @@ describe("loopover-mcp CLI — packets", () => { expect(help).toMatch(/repo decision/); }); + it("prints repo-decision help for a bare `help` positional too, not a --login error (#6257)", () => { + const help = run(["repo-decision", "help"]); + expect(help).toMatch(/Usage: loopover-mcp repo-decision/); + expect(help).not.toMatch(/Pass --login/); + }); + it("ignores incompatible decision-pack cache entries and clears cache entries on request", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); const url = await startFixtureServer(); diff --git a/test/unit/mcp-cli-review-pr.test.ts b/test/unit/mcp-cli-review-pr.test.ts index 39aabd82df..33dc847548 100644 --- a/test/unit/mcp-cli-review-pr.test.ts +++ b/test/unit/mcp-cli-review-pr.test.ts @@ -400,6 +400,12 @@ describe("loopover-mcp CLI — review-pr", () => { expect(help).toMatch(/preflight \+ slop-risk \+ PR-text-lint/); }); + it("prints help for a bare `help` positional too, not a --login error (#6257)", () => { + const help = run(["review-pr", "help"]); + expect(help).toMatch(/Usage: loopover-mcp review-pr/); + expect(help).not.toMatch(/Pass --login/); + }); + it("suggests review-pr for close typos", () => { expect(() => run(["review-pr-x"])).toThrow(/Did you mean `review-pr`\?/); });