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
30 changes: 28 additions & 2 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,35 @@ function stdioToolDescription(name) {
return tool.description;
}

/** Shared CLI failure output (#5928): mirrors @loopover/miner's cli-error.js contract so a `--json`
* invocation of this CLI never lets a thrown error escape as a raw Node stack trace — when `--json`
* is set, emit a parseable `{ ok: false, error }` object on stdout; otherwise log plain text to stderr. */
function reportCliFailure(wantsJson, message) {
if (wantsJson) {
console.log(JSON.stringify({ ok: false, error: message }, null, 2));
} else {
console.error(message);
}
}

/** True when `--json` (or `--json=...`) appears anywhere in argv, ahead of any parsed option result. */
function argsWantJson(args) {
return args.some((arg) => arg === "--json" || arg?.startsWith("--json="));
}

/** Normalize a thrown value to a safe error string for CLI output. */
function describeCliError(error) {
return error instanceof Error ? error.message : String(error);
}

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) {
reportCliFailure(argsWantJson(cliArgs), describeCliError(error));
process.exit(1);
}
}

const server = new McpServer({
Expand Down
23 changes: 22 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,27 @@ 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/) });

// the non-`--json` path is unchanged: plain text on stderr, nothing on stdout.
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 now land on stdout (the --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