diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 5fef9a0794..9bfb159faf 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -9,6 +9,7 @@ import { buildFeasibilityVerdict } from "@loopover/engine"; import { z } from "zod"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; import { formatTable } from "../lib/format-table.js"; +import { argsWantJson, describeCliError, reportCliFailure } from "../lib/cli-error.js"; // Read name/version from this package's own package.json (always present in any install -- // global, npx, or local -- npm ships it regardless of the "files" allowlist) instead of hand-synced @@ -587,8 +588,12 @@ function stdioToolDescription(name) { } if (cliArgs[0] && cliArgs[0] !== "--stdio") { - const exitCode = await runCli(cliArgs); - process.exit(typeof exitCode === "number" ? exitCode : 0); + try { + const exitCode = await runCli(cliArgs); + process.exit(typeof exitCode === "number" ? exitCode : 0); + } catch (error) { + process.exit(reportCliFailure(argsWantJson(cliArgs), describeCliError(error), 1)); + } } const server = new McpServer({ diff --git a/packages/loopover-mcp/lib/cli-error.js b/packages/loopover-mcp/lib/cli-error.js new file mode 100644 index 0000000000..0e8d7032c6 --- /dev/null +++ b/packages/loopover-mcp/lib/cli-error.js @@ -0,0 +1,27 @@ +/** Shared CLI failure output (#5928): when `--json` is set, emit a parseable `{ ok: false, error }` object on + * stdout (matching each command's success-path JSON stream); otherwise log plain text to stderr. */ + +/** + * @param {boolean} wantsJson + * @param {string} message + * @param {number} [exitCode] + * @returns {number} + */ +export function reportCliFailure(wantsJson, message, exitCode = 2) { + if (wantsJson) { + console.log(JSON.stringify({ ok: false, error: message }, null, 2)); + } else { + console.error(message); + } + return exitCode; +} + +/** True when argv includes `--json` or `--json=...` (used before a full parse result exists). */ +export function argsWantJson(args) { + return args.some((arg) => arg === "--json" || arg?.startsWith("--json=")); +} + +/** Normalize a thrown value to a safe error string for CLI output. */ +export function describeCliError(error) { + return error instanceof Error ? error.message : String(error); +} diff --git a/packages/loopover-mcp/package.json b/packages/loopover-mcp/package.json index ca86f4ed30..edf0c71f46 100644 --- a/packages/loopover-mcp/package.json +++ b/packages/loopover-mcp/package.json @@ -35,7 +35,7 @@ "CHANGELOG.md" ], "scripts": { - "build": "node --check bin/loopover-mcp.js && node --check lib/local-branch.js && node --check lib/format-table.js && node --check scripts/gittensor-score-preview.mjs" + "build": "node --check bin/loopover-mcp.js && node --check lib/cli-error.js && node --check lib/local-branch.js && node --check lib/format-table.js && node --check scripts/gittensor-score-preview.mjs" }, "dependencies": { "@loopover/engine": "^1.0.0", diff --git a/test/unit/mcp-cli-basics.test.ts b/test/unit/mcp-cli-basics.test.ts index 014722f4a8..cf6609da45 100644 --- a/test/unit/mcp-cli-basics.test.ts +++ b/test/unit/mcp-cli-basics.test.ts @@ -2,7 +2,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { closeFixtureServer, createPacketRepo, run, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; +import { closeFixtureServer, createPacketRepo, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; import mcpPackageJson from "../../packages/loopover-mcp/package.json"; describe("loopover-mcp CLI — basics", () => { @@ -162,6 +162,26 @@ describe("loopover-mcp CLI — basics", () => { expect(() => run(["bogus-command"])).toThrow(/loopover-mcp --help/); }); + it("emits a parseable { ok: false, error } object on stdout for --json failures instead of a raw stack trace (#5928)", () => { + const unknownCommand = runExpectingFailure(["bogus-command", "--json"]); + expect(unknownCommand.status).not.toBe(0); + expect(unknownCommand.stderr).toBe(""); + expect(JSON.parse(unknownCommand.stdout)).toMatchObject({ ok: false, error: expect.stringMatching(/Unknown command: bogus-command/) }); + + const missingLogin = runExpectingFailure(["preflight", "--json"]); + expect(missingLogin.status).not.toBe(0); + expect(JSON.parse(missingLogin.stdout)).toMatchObject({ ok: false, error: expect.stringMatching(/Pass --login or set LOOPOVER_LOGIN/) }); + + const badProfile = runExpectingFailure(["init-client", "--print", "codex", "--agent-profile", "autopilot", "--json"]); + expect(badProfile.status).not.toBe(0); + expect(JSON.parse(badProfile.stdout)).toMatchObject({ ok: false, error: expect.stringMatching(/Unsupported agent profile/) }); + + const plainFailure = runExpectingFailure(["bogus-command"]); + expect(plainFailure.status).not.toBe(0); + expect(plainFailure.stdout).toBe(""); + expect(plainFailure.stderr).toMatch(/Unknown command: bogus-command/); + }); + it("suggests the closest command for a near-miss typo", () => { // The suggestion is interpolated into the message, so matching `status` (not the literal // template token) confirms it ran rather than just matching the throw's printed source line. diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 6135bbc390..cec8135c9c 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -14,16 +14,24 @@ export async function closeFixtureServer() { } export function run(args: string[], env: Record = {}) { - return execFileSync("node", [bin, ...args], { - encoding: "utf8", - env: { - ...process.env, - LOOPOVER_API_TIMEOUT_MS: "1000", - LOOPOVER_CONFIG_DIR: mkdtempSync(join(tmpdir(), "loopover-cli-config-")), - ...env, - }, - stdio: ["ignore", "pipe", "pipe"], - }); + try { + return execFileSync("node", [bin, ...args], { + encoding: "utf8", + env: { + ...process.env, + LOOPOVER_API_TIMEOUT_MS: "1000", + LOOPOVER_CONFIG_DIR: mkdtempSync(join(tmpdir(), "loopover-cli-config-")), + ...env, + }, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (error) { + // A failing command's message may land on stdout (--json contract) or stderr (plain text) — + // fold both into the thrown Error's message so callers can keep matching it with toThrow(/regex/). + const failure = error as NodeJS.ErrnoException & { stdout?: string }; + if (failure instanceof Error && failure.stdout) failure.message = `${failure.message}\n${failure.stdout}`; + throw failure; + } } export function runAsync(args: string[], env: Record = {}) { @@ -42,7 +50,7 @@ export function runAsync(args: string[], env: Record = {}) { }, (error, stdout, stderr) => { if (error) { - reject(new Error(`${error.message}\n${stderr}`)); + reject(new Error(`${error.message}\n${stdout}\n${stderr}`)); return; } resolve(stdout); @@ -51,6 +59,27 @@ export function runAsync(args: string[], env: Record = {}) { }); } +/** Run a command expected to fail and return its exit code + both streams, instead of throwing — + * for asserting the shape of the failure output itself (e.g. the --json `{ ok: false, error }` contract). */ +export function runExpectingFailure(args: string[], env: Record = {}) { + try { + execFileSync("node", [bin, ...args], { + encoding: "utf8", + env: { + ...process.env, + LOOPOVER_API_TIMEOUT_MS: "1000", + LOOPOVER_CONFIG_DIR: mkdtempSync(join(tmpdir(), "loopover-cli-config-")), + ...env, + }, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (error) { + const failure = error as NodeJS.ErrnoException & { status?: number | null; stdout?: string; stderr?: string }; + return { status: failure.status ?? null, stdout: failure.stdout ?? "", stderr: failure.stderr ?? "" }; + } + throw new Error(`expected \`node ${bin} ${args.join(" ")}\` to fail`); +} + export function git(cwd: string, ...args: string[]) { execFileSync("git", args, { cwd, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); }