fix(mcp): parse --json=false and --exit-code=false as real booleans

packages/loopover-mcp/bin/loopover-mcp.ts

@@ -5462,6 +5462,12 @@ Use --profile <name> or LOOPOVER_PROFILE to run login, logout, whoami, status, d
 function parseOptions(args: any) {
   const options: any = {};
   const repeatable = new Set(["label", "issue", "id", "commit", "changedFile", "test", "testFile", "validation", "validationCommand", "validationStatus", "validationSummary", "validationDuration", "scenarioNote"]);
+  // Boolean flags that must parse their inline `--key=value` form to a REAL boolean (#8689): the
+  // generic inline-equals handler below stores the raw string, so `--json=false` became the truthy
+  // string "false" and ENABLED JSON output — the opposite of what the flag says. Parsing here keeps
+  // every consumer's check an explicit boolean comparison (the `--refresh`/`--help` `=== true`
+  // convention already established at every other boolean flag's consumer, e.g. line ~4033).
+  const booleanFlags = new Set(["json", "exitCode"]);
   for (let index = 0; index < args.length; index += 1) {
     const arg = args[index];
     if (arg === "--json") {
@@ -5484,6 +5490,10 @@ function parseOptions(args: any) {
       const inlineKey = camel(arg.slice(2, equals));
       const inlineValue = arg.slice(equals + 1);
       if (repeatable.has(inlineKey)) options[inlineKey] = [...(options[inlineKey] ?? []), inlineValue];
+      // Only the exact strings "false" and "" disable a boolean flag (`--json=false`,
+      // `--exit-code=false`); any other value (`--json=true`, bare `--json` elsewhere) keeps the
+      // flag's previous enabling behavior, so ONLY the `=false` form changes meaning.
+      else if (booleanFlags.has(inlineKey)) options[inlineKey] = inlineValue !== "false" && inlineValue !== "";
       else options[inlineKey] = inlineValue;
       continue;
     }
@@ -5937,7 +5947,9 @@ async function doctor(options: any) {
   }
   // Opt-in: let `doctor` gate CI/pre-commit by exiting non-zero when a check fails. The default
   // stays exit 0 so existing scripts that ignore the exit code keep working.
-  return options.exitCode && payload.status === "needs_attention" ? 1 : 0;
+  // Explicit `=== true` (the `--refresh`/`--help` convention): parseOptions maps `--exit-code=false`
+  // to boolean false, which must keep the exit code at 0 even when checks need attention (#8689).
+  return options.exitCode === true && payload.status === "needs_attention" ? 1 : 0;
 }
 
 function doctorStatus(checks: any) {

packages/loopover-mcp/lib/cli-error.ts

@@ -10,9 +10,10 @@ export function reportCliFailure(wantsJson: boolean, message: string, exitCode =
   return exitCode;
 }
 
-/** True when argv includes `--json` or `--json=...` (used before a full parse result exists). */
+/** True when argv includes `--json` or `--json=...` (used before a full parse result exists).
+ *  `--json=false` explicitly DISABLES JSON output (#8689), so it must not count as wanting JSON. */
 export function argsWantJson(args: readonly string[]): boolean {
-  return args.some((arg) => arg === "--json" || arg?.startsWith("--json="));
+  return args.some((arg) => arg === "--json" || (arg?.startsWith("--json=") && arg !== "--json=false"));
 }
 
 /** Normalize a thrown value to a safe error string for CLI output. */

test/unit/mcp-cli-bool-flag-parsing.test.ts

@@ -0,0 +1,135 @@
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
+import { closeFixtureServer, startFixtureServer } from "./support/mcp-cli-harness";
+
+// #8689: the generic `--key=value` inline-equals handler in parseOptions stored the raw string for
+// every flag, so `--json=false` became the truthy string "false" and ENABLED JSON output (and
+// `--exit-code=false` still escalated doctor's exit code) — the opposite of what the flags say.
+// These scenarios run the CLI in-process via the exported runCli (the #8587 pattern from
+// mcp-cli-basics.test.ts / mcp-cli-doctor.test.ts): the bin reads LOOPOVER_API_URL,
+// LOOPOVER_NPM_REGISTRY_URL, and LOOPOVER_CONFIG_DIR at module load, so one fixture server and
+// config dir are fixed before the dynamic import; call-time env (LOOPOVER_TOKEN and friends,
+// LOOPOVER_SKIP_NPM_VERSION_CHECK) varies per test. Only the committed .ts source is imported.
+type BinModule = { runCli: (args: string[]) => Promise<number | void> };
+
+// TS5097: keep the .ts specifier out of a literal import() position (same indirection as the template).
+const BIN_MODULE = "../../packages/loopover-mcp/bin/loopover-mcp.ts";
+
+let sharedConfigDir = "";
+let mod: BinModule;
+
+beforeAll(async () => {
+  sharedConfigDir = mkdtempSync(join(tmpdir(), "loopover-bool-flags-inprocess-"));
+  const apiUrl = await startFixtureServer();
+  // The bin reads these at module load, so set the env BEFORE importing (hence the dynamic import).
+  process.env.LOOPOVER_API_URL = apiUrl;
+  process.env.LOOPOVER_NPM_REGISTRY_URL = apiUrl;
+  process.env.LOOPOVER_API_TIMEOUT_MS = "2000";
+  process.env.LOOPOVER_CONFIG_DIR = sharedConfigDir;
+  mod = (await import(BIN_MODULE)) as unknown as BinModule;
+}, 120_000);
+
+afterAll(async () => {
+  await closeFixtureServer();
+  if (sharedConfigDir) rmSync(sharedConfigDir, { recursive: true, force: true });
+  delete process.env.LOOPOVER_API_URL;
+  delete process.env.LOOPOVER_NPM_REGISTRY_URL;
+  delete process.env.LOOPOVER_API_TIMEOUT_MS;
+  delete process.env.LOOPOVER_CONFIG_DIR;
+});
+
+async function captureStdout(fn: () => Promise<unknown>): Promise<string> {
+  const chunks: string[] = [];
+  const spy = vi.spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array): boolean => {
+    chunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"));
+    return true;
+  });
+  try {
+    await fn();
+  } finally {
+    spy.mockRestore();
+  }
+  return chunks.join("");
+}
+
+/** Set (string) or delete (undefined) env vars around a call, restoring the previous values after —
+ *  these are the vars the bin reads at CALL time (not module load), so per-test variation is safe. */
+async function withEnv<T>(overrides: Record<string, string | undefined>, fn: () => Promise<T>): Promise<T> {
+  const saved = new Map<string, string | undefined>();
+  for (const [key, value] of Object.entries(overrides)) {
+    saved.set(key, process.env[key]);
+    if (value === undefined) delete process.env[key];
+    else process.env[key] = value;
+  }
+  try {
+    return await fn();
+  } finally {
+    for (const [key, value] of saved) {
+      if (value === undefined) delete process.env[key];
+      else process.env[key] = value;
+    }
+  }
+}
+
+const AUTHED = { LOOPOVER_TOKEN: "session-token", LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" };
+const UNAUTHED = {
+  LOOPOVER_API_TOKEN: undefined,
+  LOOPOVER_TOKEN: undefined,
+  LOOPOVER_MCP_TOKEN: undefined,
+  LOOPOVER_SKIP_NPM_VERSION_CHECK: "true",
+};
+
+describe("loopover-mcp CLI — boolean `--flag=value` parsing (#8689)", () => {
+  it("disables JSON output for whoami --json=false (plain text, not JSON)", async () => {
+    const out = await withEnv(AUTHED, () => captureStdout(() => mod.runCli(["whoami", "--json=false"])));
+    // The plain-text arm prints just the login — never a JSON document.
+    expect(out).toBe("JSONbored\n");
+    expect(out.trimStart().startsWith("{")).toBe(false);
+    expect(out).not.toContain('"login"');
+  });
+
+  it("keeps bare --json and --json=true enabling whoami JSON output as before", async () => {
+    for (const flag of ["--json", "--json=true"]) {
+      const out = await withEnv(AUTHED, () => captureStdout(() => mod.runCli(["whoami", flag])));
+      const payload = JSON.parse(out) as { login: string; profile: string };
+      expect(payload.login).toBe("JSONbored");
+      expect(payload.profile).toBe("default");
+    }
+  });
+
+  it("keeps doctor --exit-code=false at exit code 0 against a failing check", async () => {
+    // No token configured -> the auth check fails -> status "needs_attention"; the explicit
+    // `--exit-code=false` must keep the in-process return (the would-be process exit code) at 0.
+    let exitCode: number | void = undefined;
+    const out = await withEnv(UNAUTHED, () =>
+      captureStdout(async () => {
+        exitCode = await mod.runCli(["doctor", "--exit-code=false", "--json"]);
+      }),
+    );
+    const payload = JSON.parse(out) as { status: string; checks: Array<{ name: string; status: string }> };
+    expect(payload.status).toBe("needs_attention");
+    expect(payload.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "auth", status: "fail" })]));
+    expect(exitCode).toBe(0);
+  });
+
+  it("keeps bare --exit-code and --exit-code=true escalating doctor's exit code on a failing check", async () => {
+    for (const flag of ["--exit-code", "--exit-code=true"]) {
+      let exitCode: number | void = undefined;
+      const out = await withEnv(UNAUTHED, () =>
+        captureStdout(async () => {
+          exitCode = await mod.runCli(["doctor", flag, "--json"]);
+        }),
+      );
+      expect((JSON.parse(out) as { status: string }).status).toBe("needs_attention");
+      expect(exitCode).toBe(1);
+    }
+  });
+
+  it("still passes non-boolean inline `--key=value` options through as raw strings", async () => {
+    // The boolean parse is scoped to json/exitCode: `--print=codex` keeps its string value.
+    const out = await captureStdout(() => mod.runCli(["init-client", "--print=codex"]));
+    expect(out).toContain("[mcp_servers.loopover]");
+  });
+});

test/unit/mcp-cli-error.test.ts

@@ -34,6 +34,13 @@ describe("mcp cli-error (#5928)", () => {
     expect(argsWantJson([])).toBe(false);
   });
 
+  it("argsWantJson treats the explicit --json=false disable as NOT wanting JSON (#8689)", () => {
+    expect(argsWantJson(["whoami", "--json=false"])).toBe(false);
+    expect(argsWantJson(["whoami", "--json=true"])).toBe(true);
+    // A later enabling form still wins over an earlier disable, matching parseOptions' last-write behavior.
+    expect(argsWantJson(["whoami", "--json=false", "--json"])).toBe(true);
+  });
+
   it("describeCliError normalizes thrown values", () => {
     expect(describeCliError(new Error("boom"))).toBe("boom");
     expect(describeCliError("plain")).toBe("plain");