Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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({
Expand Down
27 changes: 27 additions & 0 deletions packages/loopover-mcp/lib/cli-error.js
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion packages/loopover-mcp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
22 changes: 21 additions & 1 deletion test/unit/mcp-cli-basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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 <github-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.
Expand Down
51 changes: 40 additions & 11 deletions test/unit/support/mcp-cli-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,24 @@ export async function closeFixtureServer() {
}

export function run(args: string[], env: Record<string, string> = {}) {
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<string, string> = {}) {
Expand All @@ -42,7 +50,7 @@ export function runAsync(args: string[], env: Record<string, string> = {}) {
},
(error, stdout, stderr) => {
if (error) {
reject(new Error(`${error.message}\n${stderr}`));
reject(new Error(`${error.message}\n${stdout}\n${stderr}`));
return;
}
resolve(stdout);
Expand All @@ -51,6 +59,27 @@ export function runAsync(args: string[], env: Record<string, string> = {}) {
});
}

/** 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<string, string> = {}) {
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"] });
}
Expand Down
Loading