diff --git a/test/unit/mcp-cli-basics.test.ts b/test/unit/mcp-cli-basics.test.ts index db78195ab3..3e236e7815 100644 --- a/test/unit/mcp-cli-basics.test.ts +++ b/test/unit/mcp-cli-basics.test.ts @@ -1,35 +1,90 @@ 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, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { closeFixtureServer, createPacketRepo, run, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; import mcpPackageJson from "../../packages/loopover-mcp/package.json"; +// #8587: JSON/plain business-payload cases call the exported runCli in-process (the same dispatcher the +// spawned bin runs; pattern from mcp-cli-contributor-profile-inprocess.test.ts). Cases that assert process +// exit behavior itself (toThrow on argv errors, runExpectingFailure envelopes) or startup-resolved state +// (version banner, LOOPOVER_API_URL / LOOPOVER_CONFIG_DIR provenance — module-load reads in +// bin/loopover-mcp.ts) stay real subprocesses. +type BinModule = { + runCli: (args: string[]) => Promise; +}; + +// Only the committed .ts source is imported (never dist); the variable indirection mirrors the template's +// MODULES array so tsc does not statically flag the .ts specifier (allowImportingTsExtensions is off). +const BIN_MODULE = "../../packages/loopover-mcp/bin/loopover-mcp.ts"; + +let mod: BinModule; +let sharedConfigDir = ""; + +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} + +function runInProcess(args: string[]): Promise { + return captureStdout(() => mod.runCli(args)); +} + describe("loopover-mcp CLI — basics", () => { let tempDir: string | null = null; - afterEach(async () => { + beforeAll(async () => { + sharedConfigDir = mkdtempSync(join(tmpdir(), "loopover-basics-inprocess-")); + const apiUrl = await startFixtureServer(); + // The bin reads these at module load, so set them BEFORE the dynamic import... + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_CONFIG_DIR = sharedConfigDir; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; + // ...and delete the module-load ones right after import: the kept subprocess tests below assert DEFAULT + // config provenance (apiUrlSource "default"), which an inherited LOOPOVER_API_URL would break. + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_CONFIG_DIR; + }, 120_000); + + afterAll(async () => { await closeFixtureServer(); + if (sharedConfigDir) rmSync(sharedConfigDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; + }); + + afterEach(() => { if (tempDir) rmSync(tempDir, { recursive: true, force: true }); tempDir = null; }); - it("prints MCP client snippets without mutating client config", () => { - const codex = run(["init-client", "--print", "codex"]); + it("prints MCP client snippets without mutating client config", async () => { + const codex = await runInProcess(["init-client", "--print", "codex"]); expect(codex).toContain("[mcp_servers.loopover]"); expect(codex).toContain('args = ["--stdio"]'); - const claude = JSON.parse(run(["init-client", "--print", "claude", "--json"])) as { snippet: string }; + const claude = JSON.parse(await runInProcess(["init-client", "--print", "claude", "--json"])) as { snippet: string }; expect(claude.snippet).toContain('"mcpServers"'); expect(claude.snippet).toContain('"loopover"'); - const cursor = JSON.parse(run(["init-client", "--print", "cursor", "--json"])) as { snippet: string }; + const cursor = JSON.parse(await runInProcess(["init-client", "--print", "cursor", "--json"])) as { snippet: string }; expect(cursor.snippet).toBe(claude.snippet); - const generic = JSON.parse(run(["init-client", "--print", "mcp", "--json"])) as { snippet: string }; + const generic = JSON.parse(await runInProcess(["init-client", "--print", "mcp", "--json"])) as { snippet: string }; expect(generic.snippet).toBe(claude.snippet); - const vscode = JSON.parse(run(["init-client", "--print", "vscode", "--json"])) as { snippet: string }; + const vscode = JSON.parse(await runInProcess(["init-client", "--print", "vscode", "--json"])) as { snippet: string }; // VS Code uses a `servers` map with an explicit transport type, not the `mcpServers` shape. expect(vscode.snippet).toContain('"servers"'); expect(vscode.snippet).toContain('"type": "stdio"'); @@ -37,8 +92,8 @@ describe("loopover-mcp CLI — basics", () => { expect(vscode.snippet).not.toContain('"mcpServers"'); }); - it("prints human-approved agent profile instructions for supported MCP clients", () => { - const payload = JSON.parse(run(["init-client", "--print", "codex", "--agent-profile", "miner-planner", "--json"])) as { + it("prints human-approved agent profile instructions for supported MCP clients", async () => { + const payload = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--agent-profile", "miner-planner", "--json"])) as { agentProfile: { id: string; title: string; @@ -60,16 +115,16 @@ describe("loopover-mcp CLI — basics", () => { expect(payload.notes.join("\n")).toMatch(/human-approved/i); expect(JSON.stringify(payload)).not.toMatch(/github_pat_|gh[pousr]_|[A-Z0-9_]*TOKEN=|PRIVATE_KEY=/); - const plain = run(["init-client", "--print", "claude", "--agent-profile", "repo-owner-intake"]); + const plain = await runInProcess(["init-client", "--print", "claude", "--agent-profile", "repo-owner-intake"]); expect(plain).toContain('"mcpServers"'); expect(plain).toContain("LoopOver agent profile: Repo-owner intake"); expect(plain).toContain("loopover_repo_owner_intake_readiness"); expect(plain).toMatch(/do not.*publish public output/i); }); - it("supports all documented agent profiles without changing MCP server config", () => { + it("supports all documented agent profiles without changing MCP server config", async () => { for (const profile of ["miner-planner", "maintainer-triage", "repo-owner-intake"]) { - const payload = JSON.parse(run(["init-client", "--print", "mcp", "--agent-profile", profile, "--json"])) as { + const payload = JSON.parse(await runInProcess(["init-client", "--print", "mcp", "--agent-profile", profile, "--json"])) as { args: string[]; snippet: string; agentProfile: { id: string; boundaries: string[]; whenNotToUse: string }; @@ -83,8 +138,8 @@ describe("loopover-mcp CLI — basics", () => { } }); - it("prints the gate-throttled miner-auto-dev profile with a plan→implement→push driving loop (#781)", () => { - const payload = JSON.parse(run(["init-client", "--print", "codex", "--agent-profile", "miner-auto-dev", "--json"])) as { + it("prints the gate-throttled miner-auto-dev profile with a plan→implement→push driving loop (#781)", async () => { + const payload = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--agent-profile", "miner-auto-dev", "--json"])) as { agentProfile: { id: string; title: string; recommendedTools: string[]; drivingLoop: string[]; boundaries: string[]; whenNotToUse: string }; notes: string[]; }; @@ -101,7 +156,7 @@ describe("loopover-mcp CLI — basics", () => { expect(payload.notes.join("\n")).toMatch(/runs LOCALLY|after the LoopOver gate/i); expect(payload.notes.join("\n")).not.toMatch(/keep all GitHub writes human-approved/i); // the rendered markdown carries the driving loop too - const plain = run(["init-client", "--print", "claude", "--agent-profile", "miner-auto-dev"]); + const plain = await runInProcess(["init-client", "--print", "claude", "--agent-profile", "miner-auto-dev"]); expect(plain).toContain("LoopOver agent profile: Miner auto-dev"); expect(plain).toMatch(/Driving loop/); expect(JSON.stringify(payload)).not.toMatch(/github_pat_|gh[pousr]_|PRIVATE_KEY=/); @@ -139,22 +194,20 @@ describe("loopover-mcp CLI — basics", () => { it("redacts private account-state workspace intelligence from preflight output", async () => { tempDir = createPacketRepo(); - const url = await startFixtureServer(); - - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; - const jsonOutput = await runAsync(["preflight", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], env); - const payload = JSON.parse(jsonOutput) as { workspaceIntelligence: { blockers: { accountState: string[] }; rerunWhen: string } }; - expect(payload.workspaceIntelligence.blockers.accountState).toEqual([]); - expect(payload.workspaceIntelligence.rerunWhen).toBe("Rerun after any branch, base, or PR state changes before opening/submitting."); - expect(jsonOutput).not.toMatch(/Open PR count|Credibility|account\/queue maturity|projected score/i); - - const humanOutput = await runAsync(["preflight", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/loopover"], env); - expect(humanOutput).not.toContain("Account/queue blockers:"); - expect(humanOutput).not.toMatch(/Open PR count|Credibility|account\/queue maturity|projected score/i); + process.env.LOOPOVER_TOKEN = "session-token"; + try { + const jsonOutput = await runInProcess(["preflight", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"]); + const payload = JSON.parse(jsonOutput) as { workspaceIntelligence: { blockers: { accountState: string[] }; rerunWhen: string } }; + expect(payload.workspaceIntelligence.blockers.accountState).toEqual([]); + expect(payload.workspaceIntelligence.rerunWhen).toBe("Rerun after any branch, base, or PR state changes before opening/submitting."); + expect(jsonOutput).not.toMatch(/Open PR count|Credibility|account\/queue maturity|projected score/i); + + const humanOutput = await runInProcess(["preflight", "--login", "JSONbored", "--cwd", tempDir, "--repo", "JSONbored/loopover"]); + expect(humanOutput).not.toContain("Account/queue blockers:"); + expect(humanOutput).not.toMatch(/Open PR count|Credibility|account\/queue maturity|projected score/i); + } finally { + delete process.env.LOOPOVER_TOKEN; + } }); it("guides unknown commands to --help", () => { @@ -191,8 +244,8 @@ describe("loopover-mcp CLI — basics", () => { expect(() => run(["doctr"])).toThrow(/Did you mean `doctor`\?/); }); - it("prints shell completion scripts for bash, zsh, and fish", () => { - const bash = run(["completion", "bash"]); + it("prints shell completion scripts for bash, zsh, and fish", async () => { + const bash = await runInProcess(["completion", "bash"]); expect(bash).toContain("_loopover_mcp()"); expect(bash).toContain("complete -F _loopover_mcp loopover-mcp"); expect(bash).toContain("analyze-branch"); @@ -201,13 +254,13 @@ describe("loopover-mcp CLI — basics", () => { expect(bash).toContain("tools"); expect(bash).toContain("plan status explain packet"); - const zsh = run(["completion", "zsh"]); + const zsh = await runInProcess(["completion", "zsh"]); expect(zsh).toContain("#compdef loopover-mcp"); expect(zsh).toContain("_describe 'command' commands"); expect(zsh).toContain("commands=(login logout whoami config status changelog completion version tools doctor"); expect(zsh).toContain("list create switch remove"); - const fish = run(["completion", "fish"]); + const fish = await runInProcess(["completion", "fish"]); expect(fish).toContain("complete -c loopover-mcp"); expect(fish).toContain("complete -c loopover-mcp -n __fish_use_subcommand -a config"); expect(fish).toContain("complete -c loopover-mcp -n __fish_use_subcommand -a completion"); @@ -215,8 +268,8 @@ describe("loopover-mcp CLI — basics", () => { expect(fish).toContain("__fish_seen_subcommand_from agent"); }); - it("prints a PowerShell argument-completer script", () => { - const ps = run(["completion", "powershell"]); + it("prints a PowerShell argument-completer script", async () => { + const ps = await runInProcess(["completion", "powershell"]); expect(ps).toContain("Register-ArgumentCompleter -Native -CommandName loopover-mcp"); expect(ps).toContain("[System.Management.Automation.CompletionResult]::new"); expect(ps).toContain("$commands = @('login', 'logout'"); @@ -225,8 +278,8 @@ describe("loopover-mcp CLI — basics", () => { ); }); - it("emits completion as machine-readable json", () => { - const payload = JSON.parse(run(["completion", "zsh", "--json"])) as { shell: string; script: string }; + it("emits completion as machine-readable json", async () => { + const payload = JSON.parse(await runInProcess(["completion", "zsh", "--json"])) as { shell: string; script: string }; expect(payload.shell).toBe("zsh"); expect(payload.script).toContain("#compdef loopover-mcp"); }); @@ -283,14 +336,20 @@ describe("loopover-mcp CLI — basics", () => { } }); - it("reports enabled unsupported source upload environment settings via config", () => { - const payload = JSON.parse(run(["config", "--json"], { LOOPOVER_UPLOAD_SOURCE: "true" })) as { - sourceUpload: { default: boolean; enabled: boolean; source: string; supported: boolean }; - }; - expect(payload.sourceUpload).toEqual({ default: false, enabled: true, source: "LOOPOVER_UPLOAD_SOURCE", supported: false }); + it("reports enabled unsupported source upload environment settings via config", async () => { + // LOOPOVER_UPLOAD_SOURCE is read at call time (not module load), so it can vary per in-process call. + process.env.LOOPOVER_UPLOAD_SOURCE = "true"; + try { + const payload = JSON.parse(await runInProcess(["config", "--json"])) as { + sourceUpload: { default: boolean; enabled: boolean; source: string; supported: boolean }; + }; + expect(payload.sourceUpload).toEqual({ default: false, enabled: true, source: "LOOPOVER_UPLOAD_SOURCE", supported: false }); - const out = run(["config"], { LOOPOVER_UPLOAD_SOURCE: "true" }); - expect(out).toContain("Source upload: enabled via LOOPOVER_UPLOAD_SOURCE (unsupported; unset LOOPOVER_UPLOAD_SOURCE)"); + const out = await runInProcess(["config"]); + expect(out).toContain("Source upload: enabled via LOOPOVER_UPLOAD_SOURCE (unsupported; unset LOOPOVER_UPLOAD_SOURCE)"); + } finally { + delete process.env.LOOPOVER_UPLOAD_SOURCE; + } }); it("attributes API URL and token to a named profile from the config file", () => { diff --git a/test/unit/mcp-cli-doctor.test.ts b/test/unit/mcp-cli-doctor.test.ts index d7fd54ae70..55fde95666 100644 --- a/test/unit/mcp-cli-doctor.test.ts +++ b/test/unit/mcp-cli-doctor.test.ts @@ -1,50 +1,185 @@ import { execFileSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +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 { bin, closeFixtureServer, createPacketRepo, git, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from "vitest"; +import { + bin, + closeFixtureServer, + createPacketRepo, + git, + startFixtureServer, +} from "./support/mcp-cli-harness"; import mcpPackageJson from "../../packages/loopover-mcp/package.json"; +// 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"; + // A "higher-core prerelease" fixture (release outranks any prerelease of the same core, but a // HIGHER-core prerelease still beats a lower-core release) needs a version strictly above the local // package's own -- computed instead of hardcoded so it stays correct across every future release. const oneMinorAboveLocal = (() => { - const [major, minor] = mcpPackageJson.version.split(".").map(Number) as [number, number, number]; + const [major, minor] = mcpPackageJson.version.split(".").map(Number) as [ + number, + number, + number, + ]; return `${major}.${minor + 1}.0`; })(); +// #8587: these doctor/status scenarios now run the CLI in-process (same shape as +// mcp-cli-contributor-profile-inprocess.test.ts) instead of spawning a subprocess per call. 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; per-test variation goes through +// `fixtureOptions` (the harness route handlers read the options object at request time, so mutating it +// between tests changes responses without restarting the server) and through call-time env vars +// (LOOPOVER_TOKEN and friends, LOOPOVER_SKIP_NPM_VERSION_CHECK, GITTENSOR_*, LOOPOVER_UPLOAD_SOURCE), +// which the bin reads on every invocation. Only the committed .ts source is imported. +type BinModule = { runCli: (args: string[]) => Promise }; +type FixtureOptions = NonNullable[0]>; + +const fixtureOptions: FixtureOptions = {}; +let sharedConfigDir = ""; +let apiUrl = ""; +let mod: BinModule; + +beforeAll(async () => { + sharedConfigDir = mkdtempSync(join(tmpdir(), "loopover-doctor-inprocess-")); + apiUrl = await startFixtureServer(fixtureOptions); + // 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): Promise { + 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( + overrides: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + 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; + } + } +} + +function setFixture(overrides: FixtureOptions) { + Object.assign(fixtureOptions, overrides); +} + describe("loopover-mcp CLI — doctor", () => { let tempDir: string | null = null; - afterEach(async () => { - await closeFixtureServer(); + afterEach(() => { + for (const key of Object.keys(fixtureOptions) as Array< + keyof FixtureOptions + >) + delete fixtureOptions[key]; + rmSync(join(sharedConfigDir, "config.json"), { force: true }); if (tempDir) rmSync(tempDir, { recursive: true, force: true }); tempDir = null; }); it("runs doctor against a local health/session fixture", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); + const cwd = tempDir; const secretRoot = join(tempDir, "secret-gittensor"); - const secretConfigDir = join(tempDir, "secret-config"); - mkdirSync(secretConfigDir, { recursive: true }); - writeFileSync(join(secretConfigDir, "config.json"), JSON.stringify({ apiUrl: url }), { mode: 0o600 }); + // configured=true is existsSync(configPath) at call time, so seeding the shared config dir works in-process. + writeFileSync( + join(sharedConfigDir, "config.json"), + JSON.stringify({ apiUrl }), + { mode: 0o600 }, + ); const payload = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: secretConfigDir, - GITTENSOR_ROOT: secretRoot, - GITTENSOR_SCORE_PREVIEW_CMD: `node ${join(process.cwd(), "test/fixtures/local-scorer/scorer-malformed.mjs")}`, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + GITTENSOR_ROOT: secretRoot, + GITTENSOR_SCORE_PREVIEW_CMD: `node ${join(process.cwd(), "test/fixtures/local-scorer/scorer-malformed.mjs")}`, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }, + () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), ) as { status: string; config: { configured: boolean }; - checklist: Array<{ id: string; title: string; status: string; checks?: Array<{ name: string; status: string; detail: string; remediation?: string }> }>; + checklist: Array<{ + id: string; + title: string; + status: string; + checks?: Array<{ + name: string; + status: string; + detail: string; + remediation?: string; + }>; + }>; nextCommand: { command: string; reason: string }; - checks: Array<{ name: string; status: string; detail: string; remediation?: string }>; + checks: Array<{ + name: string; + status: string; + detail: string; + remediation?: string; + }>; }; const serialized = JSON.stringify(payload); @@ -54,7 +189,11 @@ describe("loopover-mcp CLI — doctor", () => { expect(payload.checklist).toEqual( expect.arrayContaining([ expect.objectContaining({ id: "auth", title: "Auth", status: "pass" }), - expect.objectContaining({ id: "api_compatibility", title: "API compatibility", status: "pass" }), + expect.objectContaining({ + id: "api_compatibility", + title: "API compatibility", + status: "pass", + }), // Not asserting status here: this group's own "client_path" sub-check (findExecutable("loopover-mcp")) // does a plain PATH scan for a "loopover-mcp" executable, which resolves via node_modules/.bin's npm- // workspace bin-link -- but npm only creates that symlink if bin/loopover-mcp.js already exists at @@ -63,13 +202,30 @@ describe("loopover-mcp CLI — doctor", () => { // never created there, regardless of a later build -- this group's status is genuinely "warn" in that // environment and "pass" only where something else (a real global install, e.g.) already put // loopover-mcp on PATH. Checked explicitly below instead, tolerating either. - expect.objectContaining({ id: "local_repo_readiness", title: "Local repo readiness" }), - expect.objectContaining({ id: "scorer_availability", title: "Scorer availability", status: "warn" }), - expect.objectContaining({ id: "output_safety", title: "Output safety", status: "pass" }), - expect.objectContaining({ id: "next_command", title: "Next command", status: "warn" }), + expect.objectContaining({ + id: "local_repo_readiness", + title: "Local repo readiness", + }), + expect.objectContaining({ + id: "scorer_availability", + title: "Scorer availability", + status: "warn", + }), + expect.objectContaining({ + id: "output_safety", + title: "Output safety", + status: "pass", + }), + expect.objectContaining({ + id: "next_command", + title: "Next command", + status: "warn", + }), ]), ); - const localRepoReadiness = payload.checklist.find((group) => group.id === "local_repo_readiness"); + const localRepoReadiness = payload.checklist.find( + (group) => group.id === "local_repo_readiness", + ); expect(["pass", "warn"]).toContain(localRepoReadiness?.status); expect(payload.nextCommand).toMatchObject({ command: "loopover-mcp doctor --json", @@ -78,7 +234,11 @@ describe("loopover-mcp CLI — doctor", () => { expect(payload.checks).toEqual( expect.arrayContaining([ expect.objectContaining({ name: "api_health", status: "pass" }), - expect.objectContaining({ name: "auth", status: "pass", detail: expect.stringContaining("JSONbored") }), + expect.objectContaining({ + name: "auth", + status: "pass", + detail: expect.stringContaining("JSONbored"), + }), expect.objectContaining({ name: "source_upload", status: "pass" }), expect.objectContaining({ name: "git_metadata", status: "pass" }), expect.objectContaining({ name: "version", status: "pass" }), @@ -87,54 +247,91 @@ describe("loopover-mcp CLI — doctor", () => { expect.objectContaining({ name: "gittensor_root", status: "pass" }), ]), ); - const localScorer = payload.checks.find((check) => check.name === "local_scorer"); + const localScorer = payload.checks.find( + (check) => check.name === "local_scorer", + ); expect(localScorer?.detail).toMatch(/malformed_json/); - expect(localScorer?.detail).not.toMatch(join(process.cwd(), "test/fixtures")); + expect(localScorer?.detail).not.toMatch( + join(process.cwd(), "test/fixtures"), + ); }); it("shell-quotes doctor next command values derived from local repo metadata", async () => { tempDir = createPacketRepo(); - git(tempDir, "remote", "set-url", "origin", "git@github.com:owner/repo$(touch /tmp/av_pwned).git"); - const url = await startFixtureServer(); + const cwd = tempDir; + git( + tempDir, + "remote", + "set-url", + "origin", + "git@github.com:owner/repo$(touch /tmp/av_pwned).git", + ); const env = { - LOOPOVER_API_URL: url, LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, GITTENSOR_SCORE_PREVIEW_CMD: `node ${join(process.cwd(), "test/fixtures/local-scorer/scorer-success.mjs")}`, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; - const payload = JSON.parse(await runAsync(["doctor", "--cwd", tempDir, "--json"], env)) as { nextCommand: { command: string } }; - expect(payload.nextCommand.command).toBe("loopover-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); + const payload = JSON.parse( + await withEnv(env, () => + captureStdout(() => mod.runCli(["doctor", "--cwd", cwd, "--json"])), + ), + ) as { nextCommand: { command: string } }; + expect(payload.nextCommand.command).toBe( + "loopover-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json", + ); expect(payload.nextCommand.command).not.toContain("--repo owner/repo$("); - const humanOutput = await runAsync(["doctor", "--cwd", tempDir], env); - expect(humanOutput).toContain("loopover-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json"); + const humanOutput = await withEnv(env, () => + captureStdout(() => mod.runCli(["doctor", "--cwd", cwd])), + ); + expect(humanOutput).toContain( + "loopover-mcp review-pr --login JSONbored --repo 'owner/repo$(touch /tmp/av_pwned)' --json", + ); expect(humanOutput).not.toContain("--repo owner/repo$("); }); it("uses doctor as a first-run auth checklist when no local session is configured", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); + const cwd = tempDir; const payload = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_API_TOKEN: "", - LOOPOVER_TOKEN: "", - LOOPOVER_MCP_TOKEN: "", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), + await withEnv( + { + LOOPOVER_API_TOKEN: undefined, + LOOPOVER_TOKEN: undefined, + LOOPOVER_MCP_TOKEN: undefined, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }, + () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), ) as { status: string; - checklist: Array<{ id: string; status: string; checks?: Array<{ name: string; status: string }> }>; + checklist: Array<{ + id: string; + status: string; + checks?: Array<{ name: string; status: string }>; + }>; nextCommand: { command: string; reason: string }; }; const auth = payload.checklist.find((group) => group.id === "auth"); expect(payload.status).toBe("needs_attention"); expect(auth).toMatchObject({ status: "fail" }); - expect(auth?.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "auth", status: "fail" })])); + expect(auth?.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "auth", status: "fail" }), + ]), + ); expect(payload.nextCommand).toMatchObject({ command: "loopover-mcp login --profile default", reason: expect.stringContaining("Authenticate"), @@ -143,16 +340,24 @@ describe("loopover-mcp CLI — doctor", () => { }); it("reports a stale global install with an exact upgrade command and npx fallback", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ latestVersion: "9.9.9" }); + setFixture({ latestVersion: "9.9.9" }); const payload = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), - ) as { package: { state: string; latestVersion: string; updateAvailable: boolean; upgradeCommand: string; npxFallback: string } }; + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + package: { + state: string; + latestVersion: string; + updateAvailable: boolean; + upgradeCommand: string; + npxFallback: string; + }; + }; expect(payload.package).toMatchObject({ state: "stale", @@ -164,18 +369,31 @@ describe("loopover-mcp CLI — doctor", () => { }); it("reports a current install without upgrade guidance", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ latestVersion: mcpPackageJson.version, minMcpVersion: "0.5.0" }); + setFixture({ + latestVersion: mcpPackageJson.version, + minMcpVersion: "0.5.0", + }); const payload = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), ) as { - package: { state: string; updateAvailable: boolean; upgradeCommand?: string }; - apiCompatibility: { status: string; source: string; minVersion: string; latestRecommendedVersion: string; apiVersion: string }; + package: { + state: string; + updateAvailable: boolean; + upgradeCommand?: string; + }; + apiCompatibility: { + status: string; + source: string; + minVersion: string; + latestRecommendedVersion: string; + apiVersion: string; + }; }; expect(payload.package.state).toBe("current"); @@ -191,125 +409,195 @@ describe("loopover-mcp CLI — doctor", () => { }); it("orders prerelease npm versions correctly (release outranks prerelease of the same core)", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); // Local 0.5.0 (release) vs latest 0.5.0-rc.1 (prerelease) -> local is ahead, not stale. - const aheadUrl = await startFixtureServer({ latestVersion: "0.5.0-rc.1" }); + setFixture({ latestVersion: "0.5.0-rc.1" }); const ahead = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: aheadUrl, - LOOPOVER_NPM_REGISTRY_URL: aheadUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), ) as { package: { state: string; updateAvailable: boolean } }; - expect(ahead.package).toMatchObject({ state: "ahead", updateAvailable: false }); - await closeFixtureServer(); + expect(ahead.package).toMatchObject({ + state: "ahead", + updateAvailable: false, + }); // Local (mcpPackageJson.version) vs a higher-core prerelease (one minor above) -> stale. - const staleUrl = await startFixtureServer({ latestVersion: `${oneMinorAboveLocal}-rc.1` }); + setFixture({ latestVersion: `${oneMinorAboveLocal}-rc.1` }); const stale = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: staleUrl, - LOOPOVER_NPM_REGISTRY_URL: staleUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), ) as { package: { state: string } }; expect(stale.package.state).toBe("stale"); }); it("treats an unavailable npm registry as a warning, not a hard failure", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ npmStatus: 500, compatibilityStatus: 404 }); + const cwd = tempDir; + setFixture({ npmStatus: 500, compatibilityStatus: 404 }); + const env = { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }; const status = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), - ) as { package: { state: string; updateAvailable: boolean } }; + await withEnv(env, () => + captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + package: { state: string; updateAvailable: boolean }; + }; expect(status.package.state).toBe("unavailable"); expect(status.package.updateAvailable).toBe(false); const doctor = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), - ) as { status: string; checks: Array<{ name: string; status: string; remediation?: string }> }; - expect(doctor.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "version", status: "warn" })])); - expect(doctor.checks).not.toEqual(expect.arrayContaining([expect.objectContaining({ name: "version", status: "error" })])); + await withEnv(env, () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), + ) as { + status: string; + checks: Array<{ name: string; status: string; remediation?: string }>; + }; + expect(doctor.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "version", status: "warn" }), + ]), + ); + expect(doctor.checks).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "version", status: "error" }), + ]), + ); }); it("flags a stale install in doctor with upgrade remediation", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ latestVersion: oneMinorAboveLocal }); + const cwd = tempDir; + setFixture({ latestVersion: oneMinorAboveLocal }); const payload = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), - ) as { checks: Array<{ name: string; status: string; remediation?: string }> }; + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), + ) as { + checks: Array<{ name: string; status: string; remediation?: string }>; + }; const version = payload.checks.find((check) => check.name === "version"); expect(version).toMatchObject({ status: "warn" }); - expect(version?.remediation).toContain("npm install -g @loopover/mcp@latest"); + expect(version?.remediation).toContain( + "npm install -g @loopover/mcp@latest", + ); expect(version?.remediation).toContain("npx @loopover/mcp@latest"); }); it("reports API compatibility as unavailable when the API does not advertise a minimum version", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ compatibilityStatus: 404 }); + const cwd = tempDir; + setFixture({ compatibilityStatus: 404 }); + const env = { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }; const payload = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { apiCompatibility: { status: string } }; + await withEnv(env, () => + captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + apiCompatibility: { status: string }; + }; expect(payload.apiCompatibility.status).toBe("unavailable"); const doctor = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), + await withEnv(env, () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), ) as { checks: Array<{ name: string; status: string }> }; - expect(doctor.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "api_compatibility", status: "warn" })])); + expect(doctor.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "api_compatibility", status: "warn" }), + ]), + ); }); it("falls back to legacy health compatibility when the endpoint is unavailable", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ compatibilityStatus: 503, minMcpVersion: "0.4.0" }); + setFixture({ compatibilityStatus: 503, minMcpVersion: "0.4.0" }); const payload = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { apiCompatibility: { status: string; source: string; minVersion: string } }; - expect(payload.apiCompatibility).toMatchObject({ status: "compatible", source: "health", minVersion: "0.4.0" }); + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + apiCompatibility: { status: string; source: string; minVersion: string }; + }; + expect(payload.apiCompatibility).toMatchObject({ + status: "compatible", + source: "health", + minVersion: "0.4.0", + }); }); it("uses API recommended package metadata when the npm registry is unavailable", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ npmStatus: 500, latestRecommendedMcpVersion: oneMinorAboveLocal }); + setFixture({ + npmStatus: 500, + latestRecommendedMcpVersion: oneMinorAboveLocal, + }); const payload = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }), - ) as { package: { state: string; latestStatus: string; latestVersion: string; upgradeCommand: string } }; + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, + }, + () => captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + package: { + state: string; + latestStatus: string; + latestVersion: string; + upgradeCommand: string; + }; + }; expect(payload.package).toMatchObject({ state: "stale", latestStatus: "api", @@ -319,42 +607,70 @@ describe("loopover-mcp CLI — doctor", () => { }); it("prints API compatibility unknown when the minimum version is unparseable (#6263)", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ minMcpVersion: "not-a-semver" }); + setFixture({ minMcpVersion: "not-a-semver" }); const env = { - LOOPOVER_API_URL: url, LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; - const statusJson = JSON.parse(await runAsync(["status", "--json"], env)) as { + const statusJson = JSON.parse( + await withEnv(env, () => + captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { apiCompatibility: { status: string; minVersion: string }; }; - expect(statusJson.apiCompatibility).toMatchObject({ status: "unknown", minVersion: "not-a-semver" }); + expect(statusJson.apiCompatibility).toMatchObject({ + status: "unknown", + minVersion: "not-a-semver", + }); // Human-readable status used to omit this arm entirely; keep it visible like doctor(). - const statusOutput = await runAsync(["status"], env); + const statusOutput = await withEnv(env, () => + captureStdout(() => mod.runCli(["status"])), + ); expect(statusOutput).toContain("unsupported minimum client version"); expect(statusOutput).toContain("not-a-semver"); }); it("flags API compatibility mismatches with upgrade guidance", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ minMcpVersion: "9.0.0" }); + const cwd = tempDir; + setFixture({ minMcpVersion: "9.0.0" }); const env = { - LOOPOVER_API_URL: url, LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; - const status = JSON.parse(await runAsync(["status", "--json"], env)) as { apiCompatibility: { status: string; minVersion: string; upgradeCommand: string } }; + const status = JSON.parse( + await withEnv(env, () => + captureStdout(() => mod.runCli(["status", "--json"])), + ), + ) as { + apiCompatibility: { + status: string; + minVersion: string; + upgradeCommand: string; + }; + }; expect(status.apiCompatibility).toMatchObject({ status: "incompatible", minVersion: "9.0.0", upgradeCommand: "npm install -g @loopover/mcp@latest", }); - const doctor = JSON.parse(await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], env)) as { + const doctor = JSON.parse( + await withEnv(env, () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), + ) as { checklist: Array<{ id: string; status: string }>; nextCommand: { command: string; reason: string }; checks: Array<{ name: string; status: string; remediation?: string }>; @@ -368,7 +684,11 @@ describe("loopover-mcp CLI — doctor", () => { }), ]), ); - expect(doctor.checklist).toEqual(expect.arrayContaining([expect.objectContaining({ id: "api_compatibility", status: "fail" })])); + expect(doctor.checklist).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "api_compatibility", status: "fail" }), + ]), + ); expect(doctor.nextCommand).toMatchObject({ command: "npm install -g @loopover/mcp@latest", reason: expect.stringContaining("Upgrade"), @@ -377,25 +697,46 @@ describe("loopover-mcp CLI — doctor", () => { it("keeps source upload unsupported and fail-closed in the doctor checklist", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); + const cwd = tempDir; const payload = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - LOOPOVER_UPLOAD_SOURCE: "true", - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + LOOPOVER_UPLOAD_SOURCE: "true", + }, + () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ), ) as { sourceUploadSupported: boolean; - checklist: Array<{ id: string; status: string; checks?: Array<{ name: string; status: string; remediation?: string }> }>; + checklist: Array<{ + id: string; + status: string; + checks?: Array<{ name: string; status: string; remediation?: string }>; + }>; nextCommand: { command: string; reason: string }; }; - const outputSafety = payload.checklist.find((group) => group.id === "output_safety"); + const outputSafety = payload.checklist.find( + (group) => group.id === "output_safety", + ); expect(payload.sourceUploadSupported).toBe(false); expect(outputSafety).toMatchObject({ status: "fail" }); - expect(outputSafety?.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "source_upload", status: "fail" })])); + expect(outputSafety?.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "source_upload", status: "fail" }), + ]), + ); expect(payload.nextCommand).toMatchObject({ command: "unset LOOPOVER_UPLOAD_SOURCE", reason: expect.stringContaining("metadata"), @@ -404,22 +745,34 @@ describe("loopover-mcp CLI — doctor", () => { it("points missing local repo readiness at an explicit repo-aware doctor command", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); + const cwd = tempDir; const payload = JSON.parse( - await runAsync(["doctor", "--cwd", tempDir, "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), + await withEnv( + { + LOOPOVER_TOKEN: "session-token", + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }, + () => + captureStdout(() => mod.runCli(["doctor", "--cwd", cwd, "--json"])), + ), ) as { - checklist: Array<{ id: string; status: string; checks?: Array<{ name: string; status: string; detail: string }> }>; + checklist: Array<{ + id: string; + status: string; + checks?: Array<{ name: string; status: string; detail: string }>; + }>; nextCommand: { command: string; reason: string }; }; - const repoReadiness = payload.checklist.find((group) => group.id === "local_repo_readiness"); + const repoReadiness = payload.checklist.find( + (group) => group.id === "local_repo_readiness", + ); expect(repoReadiness).toMatchObject({ status: "warn" }); - expect(repoReadiness?.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "git_metadata", status: "warn" })])); + expect(repoReadiness?.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "git_metadata", status: "warn" }), + ]), + ); expect(payload.nextCommand).toMatchObject({ command: "loopover-mcp doctor --repo owner/repo --json", reason: expect.stringContaining("git checkout"), @@ -429,18 +782,41 @@ describe("loopover-mcp CLI — doctor", () => { it("does not print configured tokens or local absolute paths in status or doctor output", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ latestVersion: "9.9.9", minMcpVersion: "9.0.0" }); + const cwd = tempDir; + setFixture({ latestVersion: "9.9.9", minMcpVersion: "9.0.0" }); const env = { - LOOPOVER_API_URL: url, - LOOPOVER_NPM_REGISTRY_URL: url, LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, + LOOPOVER_SKIP_NPM_VERSION_CHECK: undefined, }; - const statusOutput = await runAsync(["status"], env); - const statusJsonOutput = await runAsync(["status", "--json"], env); - const doctorOutput = await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover"], env); - const doctorJsonOutput = await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], env); - for (const output of [statusOutput, statusJsonOutput, doctorOutput, doctorJsonOutput]) { + const statusOutput = await withEnv(env, () => + captureStdout(() => mod.runCli(["status"])), + ); + const statusJsonOutput = await withEnv(env, () => + captureStdout(() => mod.runCli(["status", "--json"])), + ); + const doctorOutput = await withEnv(env, () => + captureStdout(() => + mod.runCli(["doctor", "--cwd", cwd, "--repo", "JSONbored/loopover"]), + ), + ); + const doctorJsonOutput = await withEnv(env, () => + captureStdout(() => + mod.runCli([ + "doctor", + "--cwd", + cwd, + "--repo", + "JSONbored/loopover", + "--json", + ]), + ), + ); + for (const output of [ + statusOutput, + statusJsonOutput, + doctorOutput, + doctorJsonOutput, + ]) { expect(output).not.toContain("session-token"); expect(output).not.toContain(tempDir); expect(output).not.toMatch(/"configPath"/); @@ -451,23 +827,38 @@ describe("loopover-mcp CLI — doctor", () => { }); it("keeps doctor exit code 0 by default even when a check fails", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); // No token configured -> the auth check fails -> status "needs_attention". - const payload = JSON.parse( - await runAsync(["doctor", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_CONFIG_DIR: tempDir, + let exitCode: number | void = undefined; + const out = await withEnv( + { + LOOPOVER_API_TOKEN: undefined, + LOOPOVER_TOKEN: undefined, + LOOPOVER_MCP_TOKEN: undefined, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { status: string; checks: Array<{ name: string; status: string }> }; + }, + () => + captureStdout(async () => { + exitCode = await mod.runCli(["doctor", "--json"]); + }), + ); + const payload = JSON.parse(out) as { + status: string; + checks: Array<{ name: string; status: string }>; + }; + expect(exitCode).toBe(0); expect(payload.status).toBe("needs_attention"); - expect(payload.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "auth", status: "fail" })])); + expect(payload.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "auth", status: "fail" }), + ]), + ); }); + // KEPT as a real subprocess (#8587 rule (a)): the subject is the process exit code itself — the + // entrypoint's `process.exit(await runCli(...))` wiring, which is unreachable in-process. Reuses the + // file's shared fixture server (default options) instead of starting a second one. it("exits non-zero from doctor --exit-code when a check fails", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); let exitCode = 0; let stdout = ""; try { @@ -476,7 +867,7 @@ describe("loopover-mcp CLI — doctor", () => { env: { ...process.env, LOOPOVER_API_TIMEOUT_MS: "1000", - LOOPOVER_API_URL: url, + LOOPOVER_API_URL: apiUrl, LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }, @@ -489,21 +880,27 @@ describe("loopover-mcp CLI — doctor", () => { } expect(exitCode).toBe(1); // The diagnostic report is still printed; only the process exit code changes. - expect((JSON.parse(stdout) as { status: string }).status).toBe("needs_attention"); + expect((JSON.parse(stdout) as { status: string }).status).toBe( + "needs_attention", + ); }); it("keeps doctor --exit-code at 0 when checks pass", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - // runAsync resolves only on a zero exit code, so reaching the assertion proves exit 0. - const payload = JSON.parse( - await runAsync(["doctor", "--exit-code", "--json"], { - LOOPOVER_API_URL: url, + // In-process, the doctor handler RETURNS the would-be exit code; 0 here proves --exit-code stays + // quiet when checks pass (the original subprocess proved it via runAsync resolving on exit 0). + let exitCode: number | void = undefined; + const out = await withEnv( + { LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { status: string }; + }, + () => + captureStdout(async () => { + exitCode = await mod.runCli(["doctor", "--exit-code", "--json"]); + }), + ); + const payload = JSON.parse(out) as { status: string }; + expect(exitCode).toBe(0); expect(payload.status).toMatch(/ok|warnings/); }); }); diff --git a/test/unit/mcp-cli-maintain-tools.test.ts b/test/unit/mcp-cli-maintain-tools.test.ts index ba693f5c6e..8af766fb86 100644 --- a/test/unit/mcp-cli-maintain-tools.test.ts +++ b/test/unit/mcp-cli-maintain-tools.test.ts @@ -1,66 +1,145 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { closeFixtureServer, run, startFixtureServer } from "./support/mcp-cli-harness"; -const bin = join(process.cwd(), "packages/loopover-mcp/dist/bin/loopover-mcp.js"); +// 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"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from "vitest"; +import { + closeFixtureServer, + startFixtureServer, +} from "./support/mcp-cli-harness"; // #6152: the maintain CLI's REST surface, exposed as stdio tools. These assert the proxy contract -- that each // tool reaches the endpoint its CLI subcommand already calls, with the same method and body -- rather than // re-testing the endpoints themselves, which test/unit/mcp-cli-maintain.test.ts already covers via the CLI. +// #8587: the server is exercised in-process over InMemoryTransport (the bin's exported McpServer + runCli, +// imported from the committed .ts source; the isProcessEntrypoint guard makes the import safe) instead of +// spawning `node dist/bin/loopover-mcp.js --stdio` per test. One fixture server + config dir serves the whole +// file because the bin reads LOOPOVER_API_URL / LOOPOVER_CONFIG_DIR at module load. +type BinModule = { + runCli: (args: string[]) => Promise; + server: { connect: (transport: unknown) => Promise }; +}; + +let mod: BinModule; +let tempDir = ""; let client: Client | null = null; -let transport: StdioClientTransport | null = null; -let configDir: string | null = null; -let capturedRequests: Array<{ url: string; method: string }>; +let capturedRequests: Array<{ url: string; method: string }> = []; -async function connect() { - configDir = mkdtempSync(join(tmpdir(), "loopover-maintain-tools-")); - capturedRequests = []; +beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "loopover-maintain-tools-")); const apiUrl = await startFixtureServer({ onApiRequest: (request) => { const url = request.url ?? ""; - if (/pending-actions|settings|gate-precision|outcome-calibration|automation-state/.test(url)) capturedRequests.push({ url, method: request.method ?? "GET" }); - }, - }); - transport = new StdioClientTransport({ - command: "node", - args: [bin, "--stdio"], - env: { - ...process.env, - LOOPOVER_CONFIG_DIR: configDir, - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_API_TIMEOUT_MS: "5000", + if ( + /pending-actions|settings|gate-precision|outcome-calibration|automation-state/.test( + url, + ) + ) + capturedRequests.push({ url, method: request.method ?? "GET" }); }, }); - client = new Client({ name: "maintain-tools-test", version: "0.0.1" }); - await client.connect(transport); + // Set the env BEFORE the dynamic import: the bin reads these at module load. LOOPOVER_TOKEN is call-time. + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = tempDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); + +async function connect() { + capturedRequests = []; + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await mod.server.connect(serverTransport); + client = new Client( + { name: "maintain-tools-test", version: "0.0.1" }, + { capabilities: {} }, + ); + await client.connect(clientTransport); } afterEach(async () => { await client?.close().catch(() => undefined); client = null; - transport = null; - await closeFixtureServer(); - if (configDir) rmSync(configDir, { recursive: true, force: true }); - configDir = null; }); +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} + const REPO = { owner: "owner", repo: "repo" }; /** Every #6152 tool (plus #7758's outcome-calibration sibling), with an argument set the fixture serves * and a field its real payload carries. */ const MAINTAIN_TOOLS = [ { name: "loopover_list_pending_actions", args: REPO, contains: "pa-1" }, - { name: "loopover_decide_pending_action", args: { ...REPO, id: "pa-1", decision: "accept" }, contains: "accepted" }, - { name: "loopover_set_agent_paused", args: { ...REPO, paused: true }, contains: "agentPaused" }, - { name: "loopover_set_action_autonomy", args: { ...REPO, action: "merge", level: "auto" }, contains: "autonomy" }, - { name: "loopover_get_gate_precision", args: REPO, contains: "falsePositiveRate" }, - { name: "loopover_get_outcome_calibration", args: REPO, contains: "positiveRate" }, - { name: "loopover_get_automation_state", args: REPO, contains: "permissionReadiness" }, + { + name: "loopover_decide_pending_action", + args: { ...REPO, id: "pa-1", decision: "accept" }, + contains: "accepted", + }, + { + name: "loopover_set_agent_paused", + args: { ...REPO, paused: true }, + contains: "agentPaused", + }, + { + name: "loopover_set_action_autonomy", + args: { ...REPO, action: "merge", level: "auto" }, + contains: "autonomy", + }, + { + name: "loopover_get_gate_precision", + args: REPO, + contains: "falsePositiveRate", + }, + { + name: "loopover_get_outcome_calibration", + args: REPO, + contains: "positiveRate", + }, + { + name: "loopover_get_automation_state", + args: REPO, + contains: "permissionReadiness", + }, ] as const; describe("loopover-mcp maintain stdio proxies (#6152)", () => { @@ -71,8 +150,11 @@ describe("loopover-mcp maintain stdio proxies (#6152)", () => { }); it("lists all 7 maintain tools via `loopover-mcp tools --json` with non-empty descriptions", async () => { - await connect(); - const payload = JSON.parse(run(["tools", "--json"])) as { tools: Array<{ name: string; description: string; category?: string }> }; + const payload = JSON.parse( + await captureStdout(() => mod.runCli(["tools", "--json"])), + ) as { + tools: Array<{ name: string; description: string; category?: string }>; + }; for (const tool of MAINTAIN_TOOLS) { const entry = payload.tools.find((t) => t.name === tool.name); expect(entry, `missing descriptor for ${tool.name}`).toBeTruthy(); @@ -83,11 +165,15 @@ describe("loopover-mcp maintain stdio proxies (#6152)", () => { for (const tool of MAINTAIN_TOOLS) { it(`${tool.name} proxies to its REST endpoint and returns the payload`, async () => { await connect(); - const result = await client!.callTool({ name: tool.name, arguments: { ...tool.args } }); + const result = await client!.callTool({ + name: tool.name, + arguments: { ...tool.args }, + }); expect(result.isError).toBeFalsy(); expect(JSON.stringify(result)).toContain(tool.contains); expect(capturedRequests.length).toBeGreaterThan(0); - for (const request of capturedRequests) expect(request.url).toContain("/v1/repos/owner/repo/"); + for (const request of capturedRequests) + expect(request.url).toContain("/v1/repos/owner/repo/"); }); // The fixture serves owner/repo only and 404s anything else, so an unregistered repo exercises the same @@ -95,7 +181,10 @@ describe("loopover-mcp maintain stdio proxies (#6152)", () => { // error rather than a silent empty success. it(`${tool.name} surfaces an API failure as a tool error`, async () => { await connect(); - const result = await client!.callTool({ name: tool.name, arguments: { ...tool.args, owner: "nobody", repo: "missing" } }); + const result = await client!.callTool({ + name: tool.name, + arguments: { ...tool.args, owner: "nobody", repo: "missing" }, + }); expect(result.isError).toBe(true); expect(JSON.stringify(result.content)).toMatch(/404|not_found/); }); @@ -108,24 +197,42 @@ describe("loopover-mcp maintain stdio proxies (#6152)", () => { // MCP layer before the handler, so it can never reach the URL either.) it("list_pending_actions advertises no status filter, which this server's route could not honour", async () => { await connect(); - const tool = (await client!.listTools()).tools.find((entry) => entry.name === "loopover_list_pending_actions"); - expect(tool, "loopover_list_pending_actions is not registered").toBeTruthy(); - expect(Object.keys(tool!.inputSchema.properties ?? {}).sort()).toEqual(["owner", "repo"]); + const tool = (await client!.listTools()).tools.find( + (entry) => entry.name === "loopover_list_pending_actions", + ); + expect( + tool, + "loopover_list_pending_actions is not registered", + ).toBeTruthy(); + expect(Object.keys(tool!.inputSchema.properties ?? {}).sort()).toEqual([ + "owner", + "repo", + ]); - const result = await client!.callTool({ name: "loopover_list_pending_actions", arguments: { ...REPO, status: "rejected" } }); + const result = await client!.callTool({ + name: "loopover_list_pending_actions", + arguments: { ...REPO, status: "rejected" }, + }); expect(result.isError).toBeFalsy(); - for (const request of capturedRequests) expect(request.url).not.toContain("status="); + for (const request of capturedRequests) + expect(request.url).not.toContain("status="); }); it("set_action_autonomy read-merge-writes so the other action classes survive", async () => { await connect(); - const result = await client!.callTool({ name: "loopover_set_action_autonomy", arguments: { ...REPO, action: "merge", level: "auto" } }); + const result = await client!.callTool({ + name: "loopover_set_action_autonomy", + arguments: { ...REPO, action: "merge", level: "auto" }, + }); expect(result.isError).toBeFalsy(); // The fixture's stored autonomy is { label: "auto" }; a blind PUT of just `merge` would drop it. const payload = JSON.stringify(result); expect(payload).toContain("label"); expect(payload).toContain("merge"); - expect(capturedRequests.map((request) => request.method)).toEqual(["GET", "PUT"]); + expect(capturedRequests.map((request) => request.method)).toEqual([ + "GET", + "PUT", + ]); }); it("rejects an unknown action class and an unknown autonomy level before any API call", async () => { @@ -134,7 +241,10 @@ describe("loopover-mcp maintain stdio proxies (#6152)", () => { { ...REPO, action: "bogus", level: "auto" }, { ...REPO, action: "merge", level: "bogus" }, ]) { - const result = await client!.callTool({ name: "loopover_set_action_autonomy", arguments: args }); + const result = await client!.callTool({ + name: "loopover_set_action_autonomy", + arguments: args, + }); expect(result.isError).toBe(true); } expect(capturedRequests).toEqual([]); diff --git a/test/unit/mcp-cli-maintain.test.ts b/test/unit/mcp-cli-maintain.test.ts index b524491a21..9b15732a26 100644 --- a/test/unit/mcp-cli-maintain.test.ts +++ b/test/unit/mcp-cli-maintain.test.ts @@ -1,122 +1,292 @@ import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; import { AUTONOMY_LEVELS } from "../../src/settings/autonomy"; -import { closeFixtureServer, repoOnboardingPackFixture, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; + +// 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"; +import { + closeFixtureServer, + repoOnboardingPackFixture, + startFixtureServer, +} from "./support/mcp-cli-harness"; // #6153: MAINTAIN_AUTONOMY_LEVELS is a hand-synced copy of the live enum (the CLI reaches @loopover/engine only // through its published export map, which doesn't surface AUTONOMY_LEVELS), so nothing but a test can catch the // two drifting apart. The source is parsed rather than imported because bin/loopover-mcp.js is an executable // entrypoint that starts a server on import. -const CLI_SOURCE = readFileSync(join(process.cwd(), "packages/loopover-mcp/dist/bin/loopover-mcp.js"), "utf8"); +const CLI_SOURCE = readFileSync( + join(process.cwd(), "packages/loopover-mcp/dist/bin/loopover-mcp.js"), + "utf8", +); /** The `maintain set-level` levels the committed CLI source really accepts. */ function declaredLevels(): string[] { - const raw = /const MAINTAIN_AUTONOMY_LEVELS = \[([^\]]*)\];/.exec(CLI_SOURCE)?.[1] ?? ""; + const raw = + /const MAINTAIN_AUTONOMY_LEVELS = \[([^\]]*)\];/.exec(CLI_SOURCE)?.[1] ?? + ""; return [...raw.matchAll(/"([^"]+)"/g)].map((m) => m[1]!); } -describe("loopover-mcp CLI — maintain (#784)", () => { - let tempDir: string | null = null; +// #8587: these cases assert JSON / plain business output that the exported runCli produces in-process (the +// isProcessEntrypoint guard lets the committed .ts source be imported without hijacking argv), so they no longer +// spawn dist/bin/loopover-mcp.js per call. One fixture server + config dir serves the whole file because the bin +// reads LOOPOVER_API_URL / LOOPOVER_CONFIG_DIR at module load; startFixtureServer reads `fixtureOptions` per +// request, so per-test response overrides (repoDocRefresh) and the capture arrays need no server restart. +type BinModule = { runCli: (args: string[]) => Promise }; + +let tempDir = ""; +let mod: BinModule; +const issueDraftBodies: Array<{ + dryRun?: boolean; + create?: boolean; + limit?: number; +}> = []; +const planIssuesBodies: Array<{ + goal?: string; + dryRun?: boolean; + create?: boolean; + limit?: number; +}> = []; +const apiRequests: Array<{ url: string; method: string }> = []; +const fixtureOptions: Parameters[0] = { + onIssueDraftRequest: (body) => void issueDraftBodies.push(body), + onPlanIssuesRequest: (body) => void planIssuesBodies.push(body), + onApiRequest: (request) => + void apiRequests.push({ + url: request.url ?? "", + method: request.method ?? "", + }), +}; + +beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); + const url = await startFixtureServer(fixtureOptions); + // The bin reads LOOPOVER_API_URL and LOOPOVER_CONFIG_DIR at module load, so set the env BEFORE importing + // (hence the dynamic import). LOOPOVER_TOKEN is read at call time. + process.env.LOOPOVER_API_URL = url; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = tempDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); - afterEach(async () => { - await closeFixtureServer(); - if (tempDir) rmSync(tempDir, { recursive: true, force: true }); - tempDir = null; - }); +beforeEach(() => { + issueDraftBodies.length = 0; + planIssuesBodies.length = 0; + apiRequests.length = 0; + fixtureOptions.repoDocRefresh = undefined; +}); - async function env(options: Parameters[0] = {}) { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(options); - return { LOOPOVER_API_URL: url, LOOPOVER_TOKEN: "session-token", LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_API_TIMEOUT_MS: "1000" }; +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} +/** In-process stand-in for the subprocess harness: dispatch through the exported runCli, return stdout. */ +function cli(args: string[]): Promise { + return captureStdout(() => mod.runCli(args)); +} + +describe("loopover-mcp CLI — maintain (#784)", () => { it("status lists the agent approval queue (plain + json)", async () => { - const e = await env(); - const out = await runAsync(["maintain", "status", "--repo", "owner/repo"], e); + const out = await cli(["maintain", "status", "--repo", "owner/repo"]); expect(out).toMatch(/Agent approval queue for owner\/repo: 1 pending/); expect(out).toMatch(/pa-1\s+merge on #7\s+clean/); - const json = JSON.parse(await runAsync(["maintain", "status", "--repo", "owner/repo", "--json"], e)) as { pendingActions: Array<{ id: string; actionClass: string }> }; - expect(json.pendingActions[0]).toMatchObject({ id: "pa-1", actionClass: "merge" }); + const json = JSON.parse( + await cli(["maintain", "status", "--repo", "owner/repo", "--json"]), + ) as { pendingActions: Array<{ id: string; actionClass: string }> }; + expect(json.pendingActions[0]).toMatchObject({ + id: "pa-1", + actionClass: "merge", + }); }); it("queue lists pending action ids that maintain approve can consume (#2236)", async () => { - const e = await env(); - const plain = await runAsync(["maintain", "queue", "--repo", "owner/repo"], e); + const plain = await cli(["maintain", "queue", "--repo", "owner/repo"]); expect(plain).toMatch(/Pending agent actions for owner\/repo: 1\./); expect(plain).toMatch(/pa-1\s+merge\s+#7\s+clean/); - const payload = JSON.parse(await runAsync(["maintain", "pending", "--repo", "owner/repo", "--json"], e)) as { - pendingActions: Array<{ id: string; actionClass: string; pullNumber: number }>; + const payload = JSON.parse( + await cli(["maintain", "pending", "--repo", "owner/repo", "--json"]), + ) as { + pendingActions: Array<{ + id: string; + actionClass: string; + pullNumber: number; + }>; }; expect(payload.pendingActions).toHaveLength(1); - expect(payload.pendingActions[0]).toMatchObject({ id: "pa-1", actionClass: "merge", pullNumber: 7 }); + expect(payload.pendingActions[0]).toMatchObject({ + id: "pa-1", + actionClass: "merge", + pullNumber: 7, + }); expect(plain).toContain(payload.pendingActions[0]!.id); - expect(await runAsync(["maintain", "approve", payload.pendingActions[0]!.id, "--repo", "owner/repo"], e)).toMatch( - /Accepted pa-1: accepted \(completed\)/, - ); + expect( + await cli([ + "maintain", + "approve", + payload.pendingActions[0]!.id, + "--repo", + "owner/repo", + ]), + ).toMatch(/Accepted pa-1: accepted \(completed\)/); }); it("approve executes a staged action; reject cancels one", async () => { - const e = await env(); - expect(await runAsync(["maintain", "approve", "pa-1", "--repo", "owner/repo"], e)).toMatch(/Accepted pa-1: accepted \(completed\)/); - expect(await runAsync(["maintain", "reject", "pa-1", "--repo", "owner/repo"], e)).toMatch(/Rejected pa-1: rejected/); + expect( + await cli(["maintain", "approve", "pa-1", "--repo", "owner/repo"]), + ).toMatch(/Accepted pa-1: accepted \(completed\)/); + expect( + await cli(["maintain", "reject", "pa-1", "--repo", "owner/repo"]), + ).toMatch(/Rejected pa-1: rejected/); }); it("pause and resume toggle the repo kill-switch", async () => { - const e = await env(); - expect(await runAsync(["maintain", "pause", "--repo", "owner/repo"], e)).toMatch(/Agent actions paused for owner\/repo/); - expect(await runAsync(["maintain", "resume", "--repo", "owner/repo"], e)).toMatch(/Agent actions resumed for owner\/repo/); + expect(await cli(["maintain", "pause", "--repo", "owner/repo"])).toMatch( + /Agent actions paused for owner\/repo/, + ); + expect(await cli(["maintain", "resume", "--repo", "owner/repo"])).toMatch( + /Agent actions resumed for owner\/repo/, + ); }); it("set-level merges one action class into the autonomy dial (read-merge-write)", async () => { - const e = await env(); - const json = JSON.parse(await runAsync(["maintain", "set-level", "merge", "auto_with_approval", "--repo", "owner/repo", "--json"], e)) as { autonomy: Record }; + const json = JSON.parse( + await cli([ + "maintain", + "set-level", + "merge", + "auto_with_approval", + "--repo", + "owner/repo", + "--json", + ]), + ) as { autonomy: Record }; // existing label:auto preserved + merge added - expect(json.autonomy).toMatchObject({ label: "auto", merge: "auto_with_approval" }); - const plain = await runAsync(["maintain", "set-level", "merge", "auto", "--repo", "owner/repo"], e); + expect(json.autonomy).toMatchObject({ + label: "auto", + merge: "auto_with_approval", + }); + const plain = await cli([ + "maintain", + "set-level", + "merge", + "auto", + "--repo", + "owner/repo", + ]); expect(plain).toMatch(/Set merge autonomy to auto for owner\/repo/); }); it("precision reports gate false-positive telemetry (plain + json), passing the window through", async () => { - const e = await env(); - const out = await runAsync(["maintain", "precision", "--repo", "owner/repo"], e); - expect(out).toMatch(/Gate precision for owner\/repo \(all history\): 11 blocked, 2 blocked-then-merged, false-positive rate 18%/); + const out = await cli(["maintain", "precision", "--repo", "owner/repo"]); + expect(out).toMatch( + /Gate precision for owner\/repo \(all history\): 11 blocked, 2 blocked-then-merged, false-positive rate 18%/, + ); expect(out).toMatch(/duplicate-pr: 8 blocked, 2 merged anyway \(25% FP\)/); // A per-type rate of null (below sample) is rendered without an FP suffix. expect(out).toMatch(/missing-linked-issue: 3 blocked, 0 merged anyway$/m); expect(out).toMatch(/Highest false-positive gate: `duplicate-pr`/); - const json = JSON.parse(await runAsync(["maintain", "precision", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli(["maintain", "precision", "--repo", "owner/repo", "--json"]), + ) as { overall: { blocked: number; falsePositiveRate: number }; }; - expect(json.overall).toMatchObject({ blocked: 11, falsePositiveRate: 0.182 }); + expect(json.overall).toMatchObject({ + blocked: 11, + falsePositiveRate: 0.182, + }); // --window-days bounds the ledger; the CLI forwards it as ?windowDays and reflects it in the summary. - const scoped = await runAsync(["maintain", "precision", "--repo", "owner/repo", "--window-days", "30"], e); + const scoped = await cli([ + "maintain", + "precision", + "--repo", + "owner/repo", + "--window-days", + "30", + ]); expect(scoped).toMatch(/Gate precision for owner\/repo \(last 30d\)/); }); it("generate-issue-drafts dry-runs by default and never forwards create (#6757)", async () => { - const bodies: Array<{ dryRun?: boolean; create?: boolean; limit?: number }> = []; - const e = await env({ onIssueDraftRequest: (b) => bodies.push(b) }); - const out = await runAsync(["maintain", "generate-issue-drafts", "--repo", "owner/repo"], e); + const out = await cli([ + "maintain", + "generate-issue-drafts", + "--repo", + "owner/repo", + ]); // A bare invocation must send {create:false, dryRun:true} — the tool can never silently create. - expect(bodies[0]).toMatchObject({ create: false, dryRun: true }); - expect(out).toMatch(/Contributor issue drafts for owner\/repo \(dry-run\): 1 proposed, 0 created/); + expect(issueDraftBodies[0]).toMatchObject({ create: false, dryRun: true }); + expect(out).toMatch( + /Contributor issue drafts for owner\/repo \(dry-run\): 1 proposed, 0 created/, + ); // The generated draft title carries an ANSI escape; the plain-text path must strip it (#6261). expect(out).toContain("Add cursor pagination"); - expect(out).not.toContain(""); + expect(out).not.toContain("[31m"); }); it("generate-issue-drafts --create forwards {create:true, dryRun:false} and reports created issues (#6757)", async () => { - const bodies: Array<{ dryRun?: boolean; create?: boolean; limit?: number }> = []; - const e = await env({ onIssueDraftRequest: (b) => bodies.push(b) }); - const out = await runAsync(["maintain", "generate-issue-drafts", "--repo", "owner/repo", "--create", "--limit", "3"], e); + const out = await cli([ + "maintain", + "generate-issue-drafts", + "--repo", + "owner/repo", + "--create", + "--limit", + "3", + ]); // --create maps to the exact {create:true, dryRun:false} shape the route's create-safety guard demands, // and --limit is forwarded as a number. - expect(bodies[0]).toMatchObject({ create: true, dryRun: false, limit: 3 }); + expect(issueDraftBodies[0]).toMatchObject({ + create: true, + dryRun: false, + limit: 3, + }); expect(out).toMatch(/\(create\): 1 proposed, 1 created/); expect(out).toMatch(/#42 https:\/\/github\.com\/owner\/repo\/issues\/42/); - const json = JSON.parse(await runAsync(["maintain", "generate-issue-drafts", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli([ + "maintain", + "generate-issue-drafts", + "--repo", + "owner/repo", + "--json", + ]), + ) as { dryRun: boolean; createRequested: boolean; }; @@ -124,29 +294,65 @@ describe("loopover-mcp CLI — maintain (#784)", () => { }); it("plan-issues requires --goal and dry-runs by default, never forwarding create (#7764)", async () => { - const bodies: Array<{ goal?: string; dryRun?: boolean; create?: boolean; limit?: number }> = []; - const e = await env({ onPlanIssuesRequest: (b) => bodies.push(b) }); // Missing --goal fails before any request is made. - await expect(runAsync(["maintain", "plan-issues", "--repo", "owner/repo"], e)).rejects.toThrow(/planning goal/); - const out = await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "Improve docs"], e); + await expect( + cli(["maintain", "plan-issues", "--repo", "owner/repo"]), + ).rejects.toThrow(/planning goal/); + const out = await cli([ + "maintain", + "plan-issues", + "--repo", + "owner/repo", + "--goal", + "Improve docs", + ]); // A bare invocation must send {create:false, dryRun:true} — the CLI can never silently create. - expect(bodies[0]).toMatchObject({ goal: "Improve docs", create: false, dryRun: true }); - expect(out).toMatch(/Issue plan for owner\/repo \(dry-run, status=ok\): 1 proposed, 0 created/); + expect(planIssuesBodies[0]).toMatchObject({ + goal: "Improve docs", + create: false, + dryRun: true, + }); + expect(out).toMatch( + /Issue plan for owner\/repo \(dry-run, status=ok\): 1 proposed, 0 created/, + ); // The AI-generated draft title carries an ANSI escape; the plain-text path must strip it (#6261). expect(out).toContain("Add cursor pagination"); expect(out).not.toContain("[31m"); }); it("plan-issues --create forwards {create:true, dryRun:false} and reports created issues (#7764)", async () => { - const bodies: Array<{ goal?: string; dryRun?: boolean; create?: boolean; limit?: number }> = []; - const e = await env({ onPlanIssuesRequest: (b) => bodies.push(b) }); - const out = await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "Ship it", "--create", "--limit", "3"], e); + const out = await cli([ + "maintain", + "plan-issues", + "--repo", + "owner/repo", + "--goal", + "Ship it", + "--create", + "--limit", + "3", + ]); // --create maps to the exact {create:true, dryRun:false} shape the route's create-safety guard demands, // and --limit is forwarded as a number. - expect(bodies[0]).toMatchObject({ goal: "Ship it", create: true, dryRun: false, limit: 3 }); + expect(planIssuesBodies[0]).toMatchObject({ + goal: "Ship it", + create: true, + dryRun: false, + limit: 3, + }); expect(out).toMatch(/\(create, status=ok\): 0 proposed, 1 created/); expect(out).toMatch(/#51 https:\/\/github\.com\/owner\/repo\/issues\/51/); - const json = JSON.parse(await runAsync(["maintain", "plan-issues", "--repo", "owner/repo", "--goal", "x", "--json"], e)) as { + const json = JSON.parse( + await cli([ + "maintain", + "plan-issues", + "--repo", + "owner/repo", + "--goal", + "x", + "--json", + ]), + ) as { dryRun: boolean; createRequested: boolean; }; @@ -154,48 +360,94 @@ describe("loopover-mcp CLI — maintain (#784)", () => { }); it("outcome-calibration reports slop-band merge rates + recommendation outcomes (plain + json), passing the window through (#6735)", async () => { - const e = await env(); - const out = await runAsync(["maintain", "outcome-calibration", "--repo", "owner/repo"], e); - expect(out).toMatch(/Outcome calibration for owner\/repo \(all history\): recommendations 14 positive, 3 negative, 3 pending \(positive rate 82%\)/); - expect(out).toMatch(/clean: 75% merge rate over 12 PR\(s\) \(9 merged, 3 closed\)/); + const out = await cli([ + "maintain", + "outcome-calibration", + "--repo", + "owner/repo", + ]); + expect(out).toMatch( + /Outcome calibration for owner\/repo \(all history\): recommendations 14 positive, 3 negative, 3 pending \(positive rate 82%\)/, + ); + expect(out).toMatch( + /clean: 75% merge rate over 12 PR\(s\) \(9 merged, 3 closed\)/, + ); expect(out).toMatch(/high: 25% merge rate over 4 PR\(s\)/); expect(out).toMatch(/Higher-slop bands merge less often/); - const json = JSON.parse(await runAsync(["maintain", "outcome-calibration", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli([ + "maintain", + "outcome-calibration", + "--repo", + "owner/repo", + "--json", + ]), + ) as { recommendations: { positive: number; positiveRate: number }; slop: Array<{ band: string }>; }; - expect(json.recommendations).toMatchObject({ positive: 14, positiveRate: 0.82 }); + expect(json.recommendations).toMatchObject({ + positive: 14, + positiveRate: 0.82, + }); expect(json.slop.map((band) => band.band)).toEqual(["clean", "high"]); // --window-days bounds the recommendation window; the CLI forwards it as ?windowDays and reflects it. - const scoped = await runAsync(["maintain", "outcome-calibration", "--repo", "owner/repo", "--window-days", "30"], e); + const scoped = await cli([ + "maintain", + "outcome-calibration", + "--repo", + "owner/repo", + "--window-days", + "30", + ]); expect(scoped).toMatch(/Outcome calibration for owner\/repo \(last 30d\)/); }); it("onboarding-pack mirrors the session-gated API payload and forwards refresh", async () => { - const requests: string[] = []; - const e = await env({ onApiRequest: (request) => requests.push(request.url ?? "") }); - const json = JSON.parse( - await runAsync(["maintain", "onboarding-pack", "--repo", "owner/repo", "--refresh", "--json"], e), + await cli([ + "maintain", + "onboarding-pack", + "--repo", + "owner/repo", + "--refresh", + "--json", + ]), ); expect(json).toEqual(repoOnboardingPackFixture); - expect(requests.at(-1)).toBe("/v1/repos/owner/repo/onboarding-pack/preview?refresh=true"); + expect(apiRequests.at(-1)?.url).toBe( + "/v1/repos/owner/repo/onboarding-pack/preview?refresh=true", + ); - const plain = await runAsync(["maintain", "onboarding-pack", "--repo", "owner/repo"], e); - expect(plain).toContain("LoopOver onboarding pack preview for owner/repo (preview-only, not published)."); + const plain = await cli([ + "maintain", + "onboarding-pack", + "--repo", + "owner/repo", + ]); + expect(plain).toContain( + "LoopOver onboarding pack preview for owner/repo (preview-only, not published).", + ); expect(plain).toContain(repoOnboardingPackFixture.preview.previewMarkdown); - expect(requests.at(-1)).toBe("/v1/repos/owner/repo/onboarding-pack/preview"); + expect(apiRequests.at(-1)?.url).toBe( + "/v1/repos/owner/repo/onboarding-pack/preview", + ); }); it("audit-feed shows the agent audit feed (plain + json), with output parity between the surfaces (#6733)", async () => { - const e = await env(); - const out = await runAsync(["maintain", "audit-feed", "--repo", "owner/repo"], e); + const out = await cli(["maintain", "audit-feed", "--repo", "owner/repo"]); expect(out).toMatch(/Agent audit feed for owner\/repo: 2 events\./); - expect(out).toMatch(/2026-05-30T00:00:00\.000Z {2}github_app\.merged {2}loopover {2}success {2}merged #7/); + expect(out).toMatch( + /2026-05-30T00:00:00\.000Z {2}github_app\.merged {2}loopover {2}success {2}merged #7/, + ); // A null detail is dropped from the line rather than printed as the string "null". - expect(out).toMatch(/github_app\.review_evasion_closed {2}loopover {2}denied$/m); + expect(out).toMatch( + /github_app\.review_evasion_closed {2}loopover {2}denied$/m, + ); // Parity: --json re-serializes the API payload untouched, so the same events reach both surfaces. - const json = JSON.parse(await runAsync(["maintain", "audit-feed", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli(["maintain", "audit-feed", "--repo", "owner/repo", "--json"]), + ) as { repoFullName: string; events: Array<{ id: string }>; }; @@ -204,53 +456,111 @@ describe("loopover-mcp CLI — maintain (#784)", () => { }); it("audit-feed forwards --since/--limit/--pull to the route and scopes the header to the pull (#6733)", async () => { - const e = await env(); // The API validates these (ISO since, limit 1..200, positive pull), so the CLI must forward them verbatim // rather than re-deciding locally -- this pins that they actually arrive. const payload = JSON.parse( - await runAsync( - ["maintain", "audit-feed", "--repo", "owner/repo", "--since", "2026-05-29T00:00:00.000Z", "--limit", "1", "--pull", "7", "--json"], - e, - ), - ) as { echoedQuery: { since: string; limit: string; pull: string }; events: unknown[] }; - expect(payload.echoedQuery).toEqual({ since: "2026-05-29T00:00:00.000Z", limit: "1", pull: "7" }); + await cli([ + "maintain", + "audit-feed", + "--repo", + "owner/repo", + "--since", + "2026-05-29T00:00:00.000Z", + "--limit", + "1", + "--pull", + "7", + "--json", + ]), + ) as { + echoedQuery: { since: string; limit: string; pull: string }; + events: unknown[]; + }; + expect(payload.echoedQuery).toEqual({ + since: "2026-05-29T00:00:00.000Z", + limit: "1", + pull: "7", + }); expect(payload.events).toHaveLength(1); // The ?pull= branch echoes pullNumber, and the plain-text header reflects that scope. - const scoped = await runAsync(["maintain", "audit-feed", "--repo", "owner/repo", "--pull", "7"], e); + const scoped = await cli([ + "maintain", + "audit-feed", + "--repo", + "owner/repo", + "--pull", + "7", + ]); expect(scoped).toMatch(/Agent audit feed for owner\/repo#7: /); }); it("audit-feed omits absent flags from the query entirely, so the route applies its own defaults (#6733)", async () => { - const e = await env(); - const payload = JSON.parse(await runAsync(["maintain", "audit-feed", "--repo", "owner/repo", "--json"], e)) as { - echoedQuery: { since: string | null; limit: string | null; pull: string | null }; + const payload = JSON.parse( + await cli(["maintain", "audit-feed", "--repo", "owner/repo", "--json"]), + ) as { + echoedQuery: { + since: string | null; + limit: string | null; + pull: string | null; + }; }; - expect(payload.echoedQuery).toEqual({ since: null, limit: null, pull: null }); + expect(payload.echoedQuery).toEqual({ + since: null, + limit: null, + pull: null, + }); }); it("automation-state shows the derived agent automation view (plain + json), with output parity (#6742)", async () => { - const e = await env(); - const out = await runAsync(["maintain", "automation-state", "--repo", "owner/repo"], e); - expect(out).toMatch(/Agent automation for owner\/repo: mode=live, 2 acting class\(es\), 3 pending approval\(s\)\./); + const out = await cli([ + "maintain", + "automation-state", + "--repo", + "owner/repo", + ]); + expect(out).toMatch( + /Agent automation for owner\/repo: mode=live, 2 acting class\(es\), 3 pending approval\(s\)\./, + ); expect(out).toMatch(/permission readiness: ready/); expect(out).toMatch(/acting classes: merge, close/); // Parity: --json re-serializes the API payload untouched, so the derived fields reach both surfaces. - const json = JSON.parse(await runAsync(["maintain", "automation-state", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli([ + "maintain", + "automation-state", + "--repo", + "owner/repo", + "--json", + ]), + ) as { repoFullName: string; mode: string; permissionReadiness: string; pendingActionCount: number; }; - expect(json).toMatchObject({ repoFullName: "owner/repo", mode: "live", permissionReadiness: "ready", pendingActionCount: 3 }); + expect(json).toMatchObject({ + repoFullName: "owner/repo", + mode: "live", + permissionReadiness: "ready", + pendingActionCount: 3, + }); }); it("refresh-docs reports a newly opened repo-doc PR (plain + json), with output parity between the surfaces (#6743)", async () => { - const e = await env({ - repoDocRefresh: { opened: true, reused: false, pullNumber: 42, url: "https://github.com/owner/repo/pull/42", claudeMode: "symlink" }, - }); - const out = await runAsync(["maintain", "refresh-docs", "--repo", "owner/repo"], e); - expect(out).toBe("Opened a new repo-doc pull request for owner/repo: https://github.com/owner/repo/pull/42\n"); - const json = JSON.parse(await runAsync(["maintain", "refresh-docs", "--repo", "owner/repo", "--json"], e)) as { + fixtureOptions.repoDocRefresh = { + opened: true, + reused: false, + pullNumber: 42, + url: "https://github.com/owner/repo/pull/42", + claudeMode: "symlink", + }; + const out = await cli(["maintain", "refresh-docs", "--repo", "owner/repo"]); + expect(out).toBe( + "Opened a new repo-doc pull request for owner/repo: https://github.com/owner/repo/pull/42\n", + ); + const json = JSON.parse( + await cli(["maintain", "refresh-docs", "--repo", "owner/repo", "--json"]), + ) as { opened: boolean; pullNumber: number; }; @@ -258,50 +568,106 @@ describe("loopover-mcp CLI — maintain (#784)", () => { }); it("refresh-docs reports the already-open PR when the route reuses one (#6743)", async () => { - const e = await env({ - repoDocRefresh: { opened: true, reused: true, pullNumber: 42, url: "https://github.com/owner/repo/pull/42", claudeMode: "copy" }, - }); - const out = await runAsync(["maintain", "refresh-docs", "--repo", "owner/repo"], e); - expect(out).toBe("Found the already-open repo-doc pull request for owner/repo: https://github.com/owner/repo/pull/42\n"); + fixtureOptions.repoDocRefresh = { + opened: true, + reused: true, + pullNumber: 42, + url: "https://github.com/owner/repo/pull/42", + claudeMode: "copy", + }; + const out = await cli(["maintain", "refresh-docs", "--repo", "owner/repo"]); + expect(out).toBe( + "Found the already-open repo-doc pull request for owner/repo: https://github.com/owner/repo/pull/42\n", + ); }); it("refresh-docs reports why no PR was opened, sanitizing the reason (#6743)", async () => { - const e = await env({ repoDocRefresh: { opened: false, reason: "no changes needed" } }); - const out = await runAsync(["maintain", "refresh-docs", "--repo", "owner/repo"], e); - expect(out).toBe("No repo-doc pull request opened for owner/repo: no changes needed\n"); + fixtureOptions.repoDocRefresh = { + opened: false, + reason: "no changes needed", + }; + const out = await cli(["maintain", "refresh-docs", "--repo", "owner/repo"]); + expect(out).toBe( + "No repo-doc pull request opened for owner/repo: no changes needed\n", + ); }); it("propose stages a new action (plain + json), POSTing to the bare pending-actions path", async () => { - const requests: Array<{ url: string; method: string }> = []; - const e = await env({ onApiRequest: (request) => void requests.push({ url: request.url ?? "", method: request.method ?? "" }) }); - const plain = await runAsync(["maintain", "propose", "review", "7", "--repo", "owner/repo", "--reason", "needs a look"], e); - expect(plain).toMatch(/Staged review on owner\/repo#7 \(pending\), id pa-1\./); + const plain = await cli([ + "maintain", + "propose", + "review", + "7", + "--repo", + "owner/repo", + "--reason", + "needs a look", + ]); + expect(plain).toMatch( + /Staged review on owner\/repo#7 \(pending\), id pa-1\./, + ); // The bare create path (no trailing slash) — distinct from the decision `/:id/:decision` POST. - expect(requests.at(-1)).toEqual({ url: "/v1/repos/owner/repo/agent/pending-actions", method: "POST" }); - const json = JSON.parse(await runAsync(["maintain", "propose", "merge", "7", "--repo", "owner/repo", "--merge-method", "squash", "--json"], e)) as { + expect(apiRequests.at(-1)).toEqual({ + url: "/v1/repos/owner/repo/agent/pending-actions", + method: "POST", + }); + const json = JSON.parse( + await cli([ + "maintain", + "propose", + "merge", + "7", + "--repo", + "owner/repo", + "--merge-method", + "squash", + "--json", + ]), + ) as { created: boolean; action: { actionClass: string; pullNumber: number }; }; - expect(json).toMatchObject({ created: true, action: { actionClass: "merge", pullNumber: 7 } }); + expect(json).toMatchObject({ + created: true, + action: { actionClass: "merge", pullNumber: 7 }, + }); }); it("propose validates the action class and pull number before any request", async () => { - const e = await env(); - await expect(runAsync(["maintain", "propose", "--repo", "owner/repo"], e)).rejects.toThrow(/Usage: loopover-mcp maintain propose/); - await expect(runAsync(["maintain", "propose", "review", "--repo", "owner/repo"], e)).rejects.toThrow(/Usage: loopover-mcp maintain propose/); - await expect(runAsync(["maintain", "propose", "bogus", "7", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown action class/); - await expect(runAsync(["maintain", "propose", "review", "0", "--repo", "owner/repo"], e)).rejects.toThrow(/Invalid pull number/); - await expect(runAsync(["maintain", "propose", "review", "1.5", "--repo", "owner/repo"], e)).rejects.toThrow(/Invalid pull number/); + await expect( + cli(["maintain", "propose", "--repo", "owner/repo"]), + ).rejects.toThrow(/Usage: loopover-mcp maintain propose/); + await expect( + cli(["maintain", "propose", "review", "--repo", "owner/repo"]), + ).rejects.toThrow(/Usage: loopover-mcp maintain propose/); + await expect( + cli(["maintain", "propose", "bogus", "7", "--repo", "owner/repo"]), + ).rejects.toThrow(/Unknown action class/); + await expect( + cli(["maintain", "propose", "review", "0", "--repo", "owner/repo"]), + ).rejects.toThrow(/Invalid pull number/); + await expect( + cli(["maintain", "propose", "review", "1.5", "--repo", "owner/repo"]), + ).rejects.toThrow(/Invalid pull number/); }, 45_000); it("validates inputs: --repo required, id required for approve, known subcommand + action/level", async () => { - const e = await env(); - await expect(runAsync(["maintain", "status"], e)).rejects.toThrow(/Pass --repo/); - await expect(runAsync(["maintain", "approve", "--repo", "owner/repo"], e)).rejects.toThrow(/Pass the pending-action id/); - await expect(runAsync(["maintain", "bogus", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown maintain subcommand/); - await expect(runAsync(["maintain", "set-level", "merge", "--repo", "owner/repo"], e)).rejects.toThrow(/Usage: loopover-mcp maintain set-level/); - await expect(runAsync(["maintain", "set-level", "bogus", "auto", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown action/); - await expect(runAsync(["maintain", "set-level", "merge", "bogus", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown level/); + await expect(cli(["maintain", "status"])).rejects.toThrow(/Pass --repo/); + await expect( + cli(["maintain", "approve", "--repo", "owner/repo"]), + ).rejects.toThrow(/Pass the pending-action id/); + await expect( + cli(["maintain", "bogus", "--repo", "owner/repo"]), + ).rejects.toThrow(/Unknown maintain subcommand/); + await expect( + cli(["maintain", "set-level", "merge", "--repo", "owner/repo"]), + ).rejects.toThrow(/Usage: loopover-mcp maintain set-level/); + await expect( + cli(["maintain", "set-level", "bogus", "auto", "--repo", "owner/repo"]), + ).rejects.toThrow(/Unknown action/); + await expect( + cli(["maintain", "set-level", "merge", "bogus", "--repo", "owner/repo"]), + ).rejects.toThrow(/Unknown level/); }, 45_000); // Pins the INVARIANT (the two lists agree), not today's three values -- restating the literal here would just @@ -314,24 +680,43 @@ describe("loopover-mcp CLI — maintain (#784)", () => { // server-side. The fixture's PUT /settings echoes any autonomy body back as a success, exactly like a server // with no enum -- so a rejection here can only have come from the CLI's own check, before any round-trip. it("rejects levels #4620 removed server-side, client-side rather than via a 400 (#6153)", async () => { - const e = await env(); for (const removed of ["suggest", "propose"]) { // Derived from the live enum for the same reason as above: the point is that the error names exactly the // levels the server accepts, not that it names three particular strings. - await expect(runAsync(["maintain", "set-level", "review", removed, "--repo", "owner/repo"], e)).rejects.toThrow( - new RegExp(`Unknown level: ${removed}\\. Use ${AUTONOMY_LEVELS.join(", ")}\\.`), + await expect( + cli([ + "maintain", + "set-level", + "review", + removed, + "--repo", + "owner/repo", + ]), + ).rejects.toThrow( + new RegExp( + `Unknown level: ${removed}\\. Use ${AUTONOMY_LEVELS.join(", ")}\\.`, + ), ); } // The dial still accepts every level the server does -- the fix narrowed the list, it didn't break it. - const json = JSON.parse(await runAsync(["maintain", "set-level", "review", "observe", "--repo", "owner/repo", "--json"], e)) as { + const json = JSON.parse( + await cli([ + "maintain", + "set-level", + "review", + "observe", + "--repo", + "owner/repo", + "--json", + ]), + ) as { autonomy: Record; }; expect(json.autonomy).toMatchObject({ review: "observe" }); }, 45_000); it("prints help when invoked with no subcommand", async () => { - const e = await env(); - const out = await runAsync(["maintain"], e); + const out = await cli(["maintain"]); expect(out).toMatch(/Usage: loopover-mcp maintain/); expect(out).toMatch(/approve /); expect(out).toMatch(/propose /); diff --git a/test/unit/mcp-cli-monitor-open-prs.test.ts b/test/unit/mcp-cli-monitor-open-prs.test.ts index d424619785..1f773722f7 100644 --- a/test/unit/mcp-cli-monitor-open-prs.test.ts +++ b/test/unit/mcp-cli-monitor-open-prs.test.ts @@ -2,132 +2,255 @@ // /v1/contributors/:login/open-pr-monitor already served this; only the stdio/CLI surface was missing. // These pin the three things that can silently rot: the tool is registered, both surfaces hit the same // route, and `monitor-open-prs --json` stays byte-identical to what the tool returns for one input. +// #8587: converted to in-process — the stdio proxy connects to the bin's exported `server` over an +// InMemoryTransport pair, and the CLI mirror calls the exported runCli with stdout captured. The fixture +// server starts once BEFORE the dynamic import (the bin reads LOOPOVER_API_URL at module load); per-test +// response overrides mutate `fixtureOptions`, which the harness reads per request. Only the exit-code / +// failure-envelope case still spawns a real subprocess. import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -// Any CLI command that calls the API must go through runAsync: the fixture server lives in this process, -// so run()'s execFileSync would block the event loop and the child's fetch would abort before a response. -import { closeFixtureServer, openPrMonitorFixture, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; -const bin = join(process.cwd(), "packages/loopover-mcp/dist/bin/loopover-mcp.js"); +// 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"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { + closeFixtureServer, + openPrMonitorFixture, + runExpectingFailure, + startFixtureServer, +} from "./support/mcp-cli-harness"; -let client: Client; -let transport: StdioClientTransport; -let configDir: string; -let apiUrl: string; -let capturedRequests: Array<{ url: string; method: string }>; +type BinModule = { + runCli: (args: string[]) => Promise; + server: { connect: (transport: unknown) => Promise }; +}; -async function connect() { +const capturedRequests: Array<{ url: string; method: string }> = []; +const fixtureOptions: NonNullable[0]> = { + onApiRequest: (request) => { + if (request.url && request.url.includes("/open-pr-monitor")) { + capturedRequests.push({ + url: request.url ?? "", + method: request.method ?? "GET", + }); + } + }, +}; +let mod: BinModule; +let apiUrl = ""; +let configDir = ""; + +beforeAll(async () => { configDir = mkdtempSync(join(tmpdir(), "loopover-monitor-open-prs-")); - capturedRequests = []; - apiUrl = await startFixtureServer({ - onApiRequest: (request) => { - if (request.url && request.url.includes("/open-pr-monitor")) { - capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" }); - } - }, - }); - transport = new StdioClientTransport({ - command: "node", - args: [bin, "--stdio"], - env: { - ...process.env, - LOOPOVER_CONFIG_DIR: configDir, - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_API_TIMEOUT_MS: "5000", - }, - }); - client = new Client({ name: "monitor-open-prs-test", version: "0.0.1" }); - await client.connect(transport); -} + apiUrl = await startFixtureServer(fixtureOptions); + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = configDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); -async function disconnect() { - await client.close().catch(() => undefined); +afterAll(async () => { await closeFixtureServer(); if (configDir) rmSync(configDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); + +beforeEach(() => { + capturedRequests.length = 0; +}); + +afterEach(() => { + delete fixtureOptions.openPrMonitor; +}); + +async function connectClient() { + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await mod.server.connect(serverTransport); + const client = new Client( + { name: "monitor-open-prs-test", version: "0.0.1" }, + { capabilities: {} }, + ); + await client.connect(clientTransport); + return client; } -describe("loopover_monitor_open_prs stdio proxy", () => { - beforeEach(connect); - afterEach(disconnect); +async function captureStdout( + fn: () => Promise, +): Promise { + 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(""); +} +describe("loopover_monitor_open_prs stdio proxy", () => { it("registers the tool in the stdio server tool list", async () => { - const { tools } = await client.listTools(); - expect(tools.map((t) => t.name)).toContain("loopover_monitor_open_prs"); + const client = await connectClient(); + try { + const { tools } = await client.listTools(); + expect(tools.map((t) => t.name)).toContain("loopover_monitor_open_prs"); + } finally { + await client.close().catch(() => undefined); + } }); it("proxies login to /v1/contributors/:login/open-pr-monitor via apiGet and returns the monitor", async () => { - const result = await client.callTool({ name: "loopover_monitor_open_prs", arguments: { login: "JSONbored" } }); - expect(capturedRequests.length).toBe(1); - const captured = capturedRequests[0]!; - expect(captured.url).toContain("/v1/contributors/JSONbored/open-pr-monitor"); - expect(captured.method).toBe("GET"); - expect(result.isError).toBeFalsy(); - const text = JSON.stringify(result); - expect(text).toContain("JSONbored/loopover"); - expect(text).toContain("failing_checks"); - // The tool summary is the API's own sentence, not a second one invented client-side. - expect(text).toContain(openPrMonitorFixture().summary); + const client = await connectClient(); + try { + const result = await client.callTool({ + name: "loopover_monitor_open_prs", + arguments: { login: "JSONbored" }, + }); + expect(capturedRequests.length).toBe(1); + const captured = capturedRequests[0]!; + expect(captured.url).toContain( + "/v1/contributors/JSONbored/open-pr-monitor", + ); + expect(captured.method).toBe("GET"); + expect(result.isError).toBeFalsy(); + const text = JSON.stringify(result); + expect(text).toContain("JSONbored/loopover"); + expect(text).toContain("failing_checks"); + // The tool summary is the API's own sentence, not a second one invented client-side. + expect(text).toContain(openPrMonitorFixture().summary); + } finally { + await client.close().catch(() => undefined); + } }); }); describe("loopover-mcp monitor-open-prs CLI", () => { - beforeEach(connect); - afterEach(disconnect); - it("--json emits exactly the payload the MCP tool surfaces for the same login (mirror parity)", async () => { - const viaTool = await client.callTool({ name: "loopover_monitor_open_prs", arguments: { login: "JSONbored" } }); - const toolData = (viaTool as { structuredContent?: unknown }).structuredContent; - const viaCli = JSON.parse(await runAsync(["monitor-open-prs", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" })); - expect(viaCli).toEqual(openPrMonitorFixture()); - if (toolData !== undefined) expect(viaCli).toEqual(toolData); + const client = await connectClient(); + try { + const viaTool = await client.callTool({ + name: "loopover_monitor_open_prs", + arguments: { login: "JSONbored" }, + }); + const toolData = (viaTool as { structuredContent?: unknown }) + .structuredContent; + const viaCli = JSON.parse( + await captureStdout(() => + mod.runCli(["monitor-open-prs", "--login", "JSONbored", "--json"]), + ), + ); + expect(viaCli).toEqual(openPrMonitorFixture()); + if (toolData !== undefined) expect(viaCli).toEqual(toolData); + } finally { + await client.close().catch(() => undefined); + } }); it("prints the API summary, guidance, and a next-step line per open PR", async () => { - const out = await runAsync(["monitor-open-prs", "--login", "JSONbored"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + const out = await captureStdout(() => + mod.runCli(["monitor-open-prs", "--login", "JSONbored"]), + ); const fixture = openPrMonitorFixture(); expect(out).toContain(fixture.summary); expect(out).toContain(fixture.guidance[0]!); - expect(out).toContain("JSONbored/loopover#42 [failing_checks] fix(queue): drain stale entries"); + expect(out).toContain( + "JSONbored/loopover#42 [failing_checks] fix(queue): drain stale entries", + ); expect(out).toContain(" - Fix the failing check, then push."); }); it("resolves the login from LOOPOVER_LOGIN, then GITHUB_LOGIN, the way decision-pack does", async () => { - const viaLoopoverLogin = await runAsync(["monitor-open-prs", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", LOOPOVER_LOGIN: "JSONbored" }); + process.env.LOOPOVER_LOGIN = "JSONbored"; + let viaLoopoverLogin = ""; + try { + viaLoopoverLogin = await captureStdout(() => + mod.runCli(["monitor-open-prs", "--json"]), + ); + } finally { + delete process.env.LOOPOVER_LOGIN; + } expect(JSON.parse(viaLoopoverLogin)).toEqual(openPrMonitorFixture()); - const viaGithubLogin = await runAsync(["monitor-open-prs", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", GITHUB_LOGIN: "JSONbored" }); + + process.env.GITHUB_LOGIN = "JSONbored"; + let viaGithubLogin = ""; + try { + viaGithubLogin = await captureStdout(() => + mod.runCli(["monitor-open-prs", "--json"]), + ); + } finally { + delete process.env.GITHUB_LOGIN; + } expect(JSON.parse(viaGithubLogin)).toEqual(openPrMonitorFixture()); }); it("fails with the shared login-required message when no login is resolvable", () => { - const failure = runExpectingFailure(["monitor-open-prs"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", LOOPOVER_LOGIN: "", GITHUB_LOGIN: "" }); + const failure = runExpectingFailure(["monitor-open-prs"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_LOGIN: "", + GITHUB_LOGIN: "", + }); expect(failure.status).toBe(1); - expect(`${failure.stdout}${failure.stderr}`).toMatch(/Pass --login or set LOOPOVER_LOGIN\./); + expect(`${failure.stdout}${failure.stderr}`).toMatch( + /Pass --login or set LOOPOVER_LOGIN\./, + ); }); // #6261: the API composes the summary/guidance and echoes PR titles back from third-party repos, so a hostile // string must not be able to repaint the terminal. --json stays raw on purpose: JSON.stringify escapes U+001B. it("strips ANSI escapes from API-chosen text on the plain-text path but not from --json", async () => { - await closeFixtureServer(); - const hostileUrl = await startFixtureServer({ openPrMonitor: { summary: "FAKE PASS", guidance: ["rewritten"] } }); - const env = { LOOPOVER_API_URL: hostileUrl, LOOPOVER_TOKEN: "session-token" }; + fixtureOptions.openPrMonitor = { + summary: "FAKE PASS", + guidance: ["rewritten"], + }; - const plain = await runAsync(["monitor-open-prs", "--login", "JSONbored"], env); + const plain = await captureStdout(() => + mod.runCli(["monitor-open-prs", "--login", "JSONbored"]), + ); expect(plain).not.toContain(""); expect(plain).toContain("FAKE PASS"); expect(plain).toContain("rewritten"); - const asJson = await runAsync(["monitor-open-prs", "--login", "JSONbored", "--json"], env); + const asJson = await captureStdout(() => + mod.runCli(["monitor-open-prs", "--login", "JSONbored", "--json"]), + ); expect(JSON.parse(asJson).summary).toBe("FAKE PASS"); }); - it("documents itself in --help and in the shell-completion command list", () => { - expect(run(["--help"])).toContain("loopover-mcp monitor-open-prs --login [--json]"); - expect(run(["monitor-open-prs", "--help"])).toContain("Mirrors the loopover_monitor_open_prs MCP tool"); - expect(run(["completion", "bash"])).toContain("monitor-open-prs"); + it("documents itself in --help and in the shell-completion command list", async () => { + expect(await captureStdout(() => mod.runCli(["--help"]))).toContain( + "loopover-mcp monitor-open-prs --login [--json]", + ); + expect( + await captureStdout(() => mod.runCli(["monitor-open-prs", "--help"])), + ).toContain("Mirrors the loopover_monitor_open_prs MCP tool"); + expect( + await captureStdout(() => mod.runCli(["completion", "bash"])), + ).toContain("monitor-open-prs"); }); }); diff --git a/test/unit/mcp-cli-notifications.test.ts b/test/unit/mcp-cli-notifications.test.ts index 5f3705d8dd..302c3d943c 100644 --- a/test/unit/mcp-cli-notifications.test.ts +++ b/test/unit/mcp-cli-notifications.test.ts @@ -3,113 +3,273 @@ // stdio/CLI surface was missing. These pin: `notifications --json` stays byte-identical to the route, the // plain-text path lists the feed, `notifications-read` forwards --id (or marks all), and login resolution matches // the sibling contributor commands. -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -// Any CLI command that calls the API must go through runAsync: the fixture server lives in this process, -// so run()'s execFileSync would block the event loop and the child's fetch would abort before a response. -import { closeFixtureServer, notificationsFixture, notificationsReadFixture, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// 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"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { + closeFixtureServer, + notificationsFixture, + notificationsReadFixture, + runExpectingFailure, + startFixtureServer, +} from "./support/mcp-cli-harness"; + +// #8587: these scenarios run the CLI in-process (same shape as mcp-cli-contributor-profile-inprocess.test.ts) +// instead of spawning a subprocess per call. The bin reads LOOPOVER_API_URL and LOOPOVER_CONFIG_DIR at module +// load, so ONE fixture server + config dir are fixed before the dynamic import; per-test variation goes +// through `fixtureOptions` (the harness route handlers read the options object at request time) and through +// call-time env vars (LOOPOVER_LOGIN / GITHUB_LOGIN are read on every invocation). Only the two +// runExpectingFailure cases stay real subprocesses: they assert the process exit code and the CLI failure +// envelope, which only the process entrypoint produces. Only the committed .ts source is imported. +type BinModule = { runCli: (args: string[]) => Promise }; +type FixtureOptions = NonNullable[0]>; let apiUrl: string; -let markReadBodies: unknown[]; +let markReadBodies: unknown[] = []; +let configDir = ""; +let mod: BinModule; +const fixtureOptions: FixtureOptions = { + onMarkNotificationsRead: (body) => markReadBodies.push(body), +}; + +beforeAll(async () => { + configDir = mkdtempSync(join(tmpdir(), "loopover-notifications-inprocess-")); + apiUrl = await startFixtureServer(fixtureOptions); + // 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_API_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = configDir; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_API_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; +}); -async function connect() { +beforeEach(() => { markReadBodies = []; - apiUrl = await startFixtureServer({ onMarkNotificationsRead: (body) => markReadBodies.push(body) }); + delete fixtureOptions.notifications; +}); + +async function captureStdout(fn: () => Promise): Promise { + 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(""); } -async function disconnect() { - await closeFixtureServer(); +/** Set (string) or delete (undefined) env vars around a call, restoring the previous values after — + * LOOPOVER_LOGIN / GITHUB_LOGIN are read at CALL time, so per-test variation is safe in-process. */ +async function withEnv( + overrides: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + 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; + } + } } describe("loopover-mcp notifications CLI", () => { - beforeEach(connect); - afterEach(disconnect); - it("--json emits exactly the feed the route returns", async () => { - const out = await runAsync(["notifications", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + const out = await captureStdout(() => + mod.runCli(["notifications", "--login", "JSONbored", "--json"]), + ); expect(JSON.parse(out)).toEqual(notificationsFixture()); }); it("prints the unread count and a line per notification", async () => { - const out = await runAsync(["notifications", "--login", "JSONbored"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + const out = await captureStdout(() => + mod.runCli(["notifications", "--login", "JSONbored"]), + ); expect(out).toContain("LoopOver notifications for JSONbored: 1 unread."); - expect(out).toContain("JSONbored/loopover#42 Your pull request JSONbored/loopover#42 was merged."); - expect(out).toContain("JSONbored/loopover#7 Changes requested on JSONbored/loopover#7."); + expect(out).toContain( + "JSONbored/loopover#42 Your pull request JSONbored/loopover#42 was merged.", + ); + expect(out).toContain( + "JSONbored/loopover#7 Changes requested on JSONbored/loopover#7.", + ); }); it("resolves the login from LOOPOVER_LOGIN, then GITHUB_LOGIN, like the sibling contributor commands", async () => { - const viaLoopoverLogin = await runAsync(["notifications", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", LOOPOVER_LOGIN: "JSONbored" }); + const viaLoopoverLogin = await withEnv( + { LOOPOVER_LOGIN: "JSONbored", GITHUB_LOGIN: undefined }, + () => captureStdout(() => mod.runCli(["notifications", "--json"])), + ); expect(JSON.parse(viaLoopoverLogin)).toEqual(notificationsFixture()); - const viaGithubLogin = await runAsync(["notifications", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", GITHUB_LOGIN: "JSONbored" }); + const viaGithubLogin = await withEnv( + { LOOPOVER_LOGIN: undefined, GITHUB_LOGIN: "JSONbored" }, + () => captureStdout(() => mod.runCli(["notifications", "--json"])), + ); expect(JSON.parse(viaGithubLogin)).toEqual(notificationsFixture()); }); + // KEPT as a real subprocess (#8587 rule (a)): asserts the process exit code and the failure output of the + // entrypoint's catch, which only a spawned process produces. it("fails with the shared login-required message when no login is resolvable", () => { - const failure = runExpectingFailure(["notifications"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", LOOPOVER_LOGIN: "", GITHUB_LOGIN: "" }); + const failure = runExpectingFailure(["notifications"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_LOGIN: "", + GITHUB_LOGIN: "", + }); expect(failure.status).toBe(1); - expect(`${failure.stdout}${failure.stderr}`).toMatch(/Pass --login /); + expect(`${failure.stdout}${failure.stderr}`).toMatch( + /Pass --login /, + ); }); // #6261: the API chooses the notification title text, so a hostile string must not repaint the terminal. it("strips ANSI escapes from API-chosen text on the plain-text path but not from --json", async () => { - await closeFixtureServer(); const esc = String.fromCharCode(27); const hostileTitle = `${esc}[31mFAKE MERGE${esc}[0m`; - const hostileUrl = await startFixtureServer({ - notifications: { - unreadCount: 1, - notifications: [{ id: "x", eventType: "pull_request_merged", repoFullName: "acme/x", pullNumber: 1, title: hostileTitle, body: "b", deeplink: "https://x", status: "delivered", createdAt: "2026-06-01T00:00:00.000Z" }], - }, - }); - const env = { LOOPOVER_API_URL: hostileUrl, LOOPOVER_TOKEN: "session-token" }; + // The fixture server reads fixtureOptions at request time, so this swaps the feed for a hostile one + // without restarting the server (beforeEach clears it again). + fixtureOptions.notifications = { + unreadCount: 1, + notifications: [ + { + id: "x", + eventType: "pull_request_merged", + repoFullName: "acme/x", + pullNumber: 1, + title: hostileTitle, + body: "b", + deeplink: "https://x", + status: "delivered", + createdAt: "2026-06-01T00:00:00.000Z", + }, + ], + }; - const plain = await runAsync(["notifications", "--login", "JSONbored"], env); + const plain = await captureStdout(() => + mod.runCli(["notifications", "--login", "JSONbored"]), + ); expect(plain).not.toContain(esc); expect(plain).toContain("FAKE MERGE"); - const asJson = await runAsync(["notifications", "--login", "JSONbored", "--json"], env); + const asJson = await captureStdout(() => + mod.runCli(["notifications", "--login", "JSONbored", "--json"]), + ); expect(JSON.parse(asJson).notifications[0].title).toBe(hostileTitle); }); - it("documents itself in --help, in its own --help, and in the shell-completion command list", () => { - expect(run(["--help"])).toContain("loopover-mcp notifications --login [--json]"); - expect(run(["notifications", "--help"])).toContain("Mirrors the loopover_list_notifications MCP tool"); - expect(run(["completion", "bash"])).toContain("notifications"); + it("documents itself in --help, in its own --help, and in the shell-completion command list", async () => { + expect(await captureStdout(() => mod.runCli(["--help"]))).toContain( + "loopover-mcp notifications --login [--json]", + ); + expect( + await captureStdout(() => mod.runCli(["notifications", "--help"])), + ).toContain("Mirrors the loopover_list_notifications MCP tool"); + expect( + await captureStdout(() => mod.runCli(["completion", "bash"])), + ).toContain("notifications"); }); }); describe("loopover-mcp notifications-read CLI", () => { - beforeEach(connect); - afterEach(disconnect); - it("--json emits exactly the { login, marked } the route returns", async () => { - const out = await runAsync(["notifications-read", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + const out = await captureStdout(() => + mod.runCli(["notifications-read", "--login", "JSONbored", "--json"]), + ); expect(JSON.parse(out)).toEqual(notificationsReadFixture()); }); it("prints the marked count on the plain-text path", async () => { - const out = await runAsync(["notifications-read", "--login", "JSONbored"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); - expect(out).toContain("Marked 2 LoopOver notification(s) read for JSONbored."); + const out = await captureStdout(() => + mod.runCli(["notifications-read", "--login", "JSONbored"]), + ); + expect(out).toContain( + "Marked 2 LoopOver notification(s) read for JSONbored.", + ); }); it("marks all (empty body) when no --id is given", async () => { - await runAsync(["notifications-read", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + await captureStdout(() => + mod.runCli(["notifications-read", "--login", "JSONbored", "--json"]), + ); expect(markReadBodies).toEqual([{}]); }); it("forwards repeated --id flags as an ids array", async () => { - await runAsync(["notifications-read", "--login", "JSONbored", "--id", "d-42", "--id", "d-7", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + await captureStdout(() => + mod.runCli([ + "notifications-read", + "--login", + "JSONbored", + "--id", + "d-42", + "--id", + "d-7", + "--json", + ]), + ); expect(markReadBodies).toEqual([{ ids: ["d-42", "d-7"] }]); }); + // KEPT as a real subprocess (#8587 rule (a)): asserts the process exit code and the failure output of the + // entrypoint's catch, which only a spawned process produces. it("fails with the shared login-required message when no login is resolvable", () => { - const failure = runExpectingFailure(["notifications-read"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token", LOOPOVER_LOGIN: "", GITHUB_LOGIN: "" }); + const failure = runExpectingFailure(["notifications-read"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_LOGIN: "", + GITHUB_LOGIN: "", + }); expect(failure.status).toBe(1); - expect(`${failure.stdout}${failure.stderr}`).toMatch(/Pass --login /); + expect(`${failure.stdout}${failure.stderr}`).toMatch( + /Pass --login /, + ); }); - it("documents itself in --help, in its own --help, and in the shell-completion command list", () => { - expect(run(["--help"])).toContain("loopover-mcp notifications-read --login [--id ]... [--json]"); - expect(run(["notifications-read", "--help"])).toContain("Mirrors the loopover_mark_notifications_read MCP tool"); - expect(run(["completion", "bash"])).toContain("notifications-read"); + it("documents itself in --help, in its own --help, and in the shell-completion command list", async () => { + expect(await captureStdout(() => mod.runCli(["--help"]))).toContain( + "loopover-mcp notifications-read --login [--id ]... [--json]", + ); + expect( + await captureStdout(() => mod.runCli(["notifications-read", "--help"])), + ).toContain("Mirrors the loopover_mark_notifications_read MCP tool"); + expect( + await captureStdout(() => mod.runCli(["completion", "bash"])), + ).toContain("notifications-read"); }); }); diff --git a/test/unit/mcp-cli-packets.test.ts b/test/unit/mcp-cli-packets.test.ts index a3f566290f..7f7a9d8024 100644 --- a/test/unit/mcp-cli-packets.test.ts +++ b/test/unit/mcp-cli-packets.test.ts @@ -1,49 +1,146 @@ +import { createServer, request as forwardToFixture, type Server } from "node:http"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; + +// 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"; import { - capturePacketValidation, closeFixtureServer, createPacketRepo, decisionPackCacheFile, git, readDecisionPackCacheText, - run, - runAsync, startFixtureServer, } from "./support/mcp-cli-harness"; +// #8587: converted from per-test subprocess spawns (run/runAsync) to one in-process import of the committed +// bin source, driving the exported runCli directly. LOOPOVER_API_URL and LOOPOVER_CONFIG_DIR are read at +// module load, so the whole file shares one fixture server (its per-request behavior stays configurable via +// the mutable `fixtureOptions` object the harness reads on every request) and one config dir (the cache +// subdir is wiped between tests, mirroring the fresh temp config dir each spawn used to get). The fixture +// server sits behind a tiny local proxy, and the proxy's port is what the bin sees: flipping `apiOnline` +// severs incoming requests, reproducing the "API unavailable" scenarios that previously worked by closing +// the per-test fixture server the subprocess pointed at. +type BinModule = { runCli: (args: string[]) => Promise }; + +const fixtureOptions: { + decisionPackStatus?: number; + decisionPackErrorBody?: string; + decisionPackErrorContentType?: string; + repoDecisionStatus?: number; + repoDecisionErrorBody?: string; + repoDecisionErrorContentType?: string; + packetMarkdown?: string; + onPacketRequest?: (body: unknown) => void; +} = {}; +const packetRequests: unknown[] = []; +let configDir = ""; +let proxy: Server | null = null; +let apiOnline = true; +let mod: BinModule; + +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} + +async function runCliJson(args: string[]): Promise { + return JSON.parse(await captureStdout(() => mod.runCli(args))) as unknown; +} + +/** In-process equivalent of the harness's capturePacketValidation: same argv, same capture, no spawn. */ +async function captureInProcessPacketValidation(repoDir: string, validationArgs: string[]) { + packetRequests.length = 0; + await captureStdout(() => mod.runCli(["agent", "packet", "--login", "oktofeesh1", "--cwd", repoDir, "--base", "HEAD", ...validationArgs, "--json"])); + return (packetRequests[0] as { validation: Array<{ command: string; status: string; exitCode?: number; summary?: string }> }).validation; +} + describe("loopover-mcp CLI — packets", () => { let tempDir: string | null = null; - afterEach(async () => { + beforeAll(async () => { + configDir = mkdtempSync(join(tmpdir(), "loopover-cli-packets-inprocess-")); + fixtureOptions.onPacketRequest = (body) => packetRequests.push(body); + const fixtureUrl = new URL(await startFixtureServer(fixtureOptions)); + proxy = createServer((request, response) => { + if (!apiOnline) { + request.destroy(); + return; + } + const upstream = forwardToFixture( + { hostname: fixtureUrl.hostname, port: fixtureUrl.port, path: request.url, method: request.method, headers: request.headers }, + (upstreamResponse) => { + response.writeHead(upstreamResponse.statusCode ?? 500, upstreamResponse.headers); + upstreamResponse.pipe(response); + }, + ); + request.pipe(upstream); + }); + await new Promise((resolve) => proxy?.listen(0, "127.0.0.1", () => resolve())); + const address = proxy.address(); + if (!address || typeof address === "string") throw new Error("proxy did not bind a TCP port"); + // The bin reads LOOPOVER_API_URL and LOOPOVER_CONFIG_DIR at module load, so set the env BEFORE importing. + process.env.LOOPOVER_API_URL = `http://127.0.0.1:${address.port}`; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = configDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; + }, 120_000); + + afterAll(async () => { await closeFixtureServer(); + if (proxy) await new Promise((resolve) => proxy?.close(() => resolve())); + if (configDir) rmSync(configDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; + }); + + afterEach(() => { if (tempDir) rmSync(tempDir, { recursive: true, force: true }); tempDir = null; + // Fresh decision-pack cache per test, standing in for the fresh temp config dir each subprocess got. + if (configDir) rmSync(join(configDir, "cache"), { recursive: true, force: true }); + apiOnline = true; + delete fixtureOptions.decisionPackStatus; + delete fixtureOptions.decisionPackErrorBody; + delete fixtureOptions.decisionPackErrorContentType; + delete fixtureOptions.repoDecisionStatus; + delete fixtureOptions.repoDecisionErrorBody; + delete fixtureOptions.repoDecisionErrorContentType; + delete fixtureOptions.packetMarkdown; + packetRequests.length = 0; + process.env.LOOPOVER_TOKEN = "session-token"; + delete process.env.LOOPOVER_API_TOKEN; + delete process.env.LOOPOVER_MCP_TOKEN; }); it("caches last-good decision packs and returns explicitly stale local fallback when the API is unavailable", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_API_TIMEOUT_MS: "1000", - }; - - const online = JSON.parse(await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env)) as { status: string; source: string }; + const online = (await runCliJson(["decision-pack", "--login", "JSONbored", "--json"])) as { status: string; source: string }; expect(online).toMatchObject({ status: "ready", source: "snapshot" }); - const cacheText = readDecisionPackCacheText(tempDir); + const cacheText = readDecisionPackCacheText(configDir); expect(cacheText).toMatch(/"authCacheKey":/); expect(cacheText).not.toContain("session-token"); expect(cacheText).not.toMatch(/must stay local|wallet-value|hotkey-value|\/tmp\/source/i); - await closeFixtureServer(); + apiOnline = false; - const offline = JSON.parse(await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env)) as { + const offline = (await runCliJson(["decision-pack", "--login", "JSONbored", "--json"])) as { source: string; stale: boolean; freshness: string; @@ -59,7 +156,7 @@ describe("loopover-mcp CLI — packets", () => { expect(offline.cachedAt).toEqual(expect.any(String)); expect(offline.cache.rerunGuidance).toMatch(/Retry when LoopOver API access is restored/); - const repoDecision = JSON.parse(await runAsync(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"], env)) as { + const repoDecision = (await runCliJson(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"])) as { status: string; source: string; stale: boolean; @@ -73,72 +170,56 @@ describe("loopover-mcp CLI — packets", () => { }); }); - it("prints decision-pack help without requiring --login or making a network call", () => { - const help = run(["decision-pack", "--help"]); + it("prints decision-pack help without requiring --login or making a network call", async () => { + const help = await captureStdout(() => mod.runCli(["decision-pack", "--help"])); expect(help).toMatch(/Usage: loopover-mcp decision-pack/); expect(help).toMatch(/loopover_get_decision_pack/); 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"]); + it("prints decision-pack help for a bare `help` positional too, not a --login error (#6257)", async () => { + const help = await captureStdout(() => mod.runCli(["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"]); + it("prints repo-decision help without requiring --login/--repo or making a network call", async () => { + const help = await captureStdout(() => mod.runCli(["repo-decision", "--help"])); expect(help).toMatch(/Usage: loopover-mcp repo-decision/); expect(help).toMatch(/loopover_explain_repo_decision/); 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"]); + it("prints repo-decision help for a bare `help` positional too, not a --login error (#6257)", async () => { + const help = await captureStdout(() => mod.runCli(["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(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_API_TIMEOUT_MS: "1000", - }; - - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); - const cachePath = decisionPackCacheFile(tempDir); + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); + const cachePath = decisionPackCacheFile(configDir); const entry = JSON.parse(readFileSync(cachePath, "utf8")); writeFileSync(cachePath, `${JSON.stringify({ ...entry, schemaVersion: 999 }, null, 2)}\n`, { mode: 0o600 }); - await closeFixtureServer(); + apiOnline = false; - await expect(runAsync(["decision-pack", "--login", "JSONbored", "--json"], env)).rejects.toThrow(/fetch failed|ECONNREFUSED|AbortError|aborted/i); + await expect(captureStdout(() => mod.runCli(["decision-pack", "--login", "JSONbored", "--json"]))).rejects.toThrow( + /fetch failed|ECONNREFUSED|AbortError|aborted/i, + ); - const cleared = JSON.parse(run(["cache", "clear", "--json"], env)) as { status: string; removed: number }; + const cleared = (await runCliJson(["cache", "clear", "--json"])) as { status: string; removed: number }; expect(cleared).toMatchObject({ status: "cleared", removed: 1 }); - const cacheStatus = JSON.parse(run(["cache", "status", "--json"], env)) as { entries: number }; + const cacheStatus = (await runCliJson(["cache", "status", "--json"])) as { entries: number }; expect(cacheStatus.entries).toBe(0); }); it("lists cached decision packs with safe metadata only", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_API_TIMEOUT_MS: "1000", - }; - - const empty = JSON.parse(run(["cache", "list", "--json"], env)) as { count: number; entries: unknown[] }; + const empty = (await runCliJson(["cache", "list", "--json"])) as { count: number; entries: unknown[] }; expect(empty).toMatchObject({ count: 0, entries: [] }); - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); - const listed = JSON.parse(run(["cache", "list", "--json"], env)) as { + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); + const listed = (await runCliJson(["cache", "list", "--json"])) as { count: number; entries: Array<{ login: string; cachedAt: string; apiVersion: string; packageVersion: string; bytes: number }>; }; @@ -153,25 +234,16 @@ describe("loopover-mcp CLI — packets", () => { expect(serialized).not.toContain("session-token"); expect(serialized).not.toMatch(/authCacheKey/); - const human = run(["cache", "list"], env); + const human = await captureStdout(() => mod.runCli(["cache", "list"])); expect(human).toContain("jsonbored"); }); it("cache list --format ndjson streams one JSON object per cached entry", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_API_TIMEOUT_MS: "1000", - }; - // Empty cache → zero ndjson lines (not a wrapper object). - expect(run(["cache", "list", "--format", "ndjson"], env).trim()).toBe(""); + expect((await captureStdout(() => mod.runCli(["cache", "list", "--format", "ndjson"]))).trim()).toBe(""); - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); - const lines = run(["cache", "list", "--format", "ndjson"], env).trim().split("\n"); + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); + const lines = (await captureStdout(() => mod.runCli(["cache", "list", "--format", "ndjson"]))).trim().split("\n"); expect(lines).toHaveLength(1); const [firstLine] = lines as [string]; const entry = JSON.parse(firstLine) as { login: string; bytes: number }; @@ -182,60 +254,22 @@ describe("loopover-mcp CLI — packets", () => { }); it("does not use stale decision-pack cache created by a different local token", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const fixtureOptions: { decisionPackStatus?: number } = {}; - const url = await startFixtureServer(fixtureOptions); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }; - - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); fixtureOptions.decisionPackStatus = 429; + process.env.LOOPOVER_TOKEN = "different-session-token"; - await expect( - runAsync(["decision-pack", "--login", "JSONbored", "--json"], { - ...env, - LOOPOVER_TOKEN: "different-session-token", - }), - ).rejects.toThrow(/LoopOver API 429/); + await expect(captureStdout(() => mod.runCli(["decision-pack", "--login", "JSONbored", "--json"]))).rejects.toThrow(/LoopOver API 429/); }); it("does not use stale decision-pack cache for authorization failures", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const fixtureOptions: { decisionPackStatus?: number } = {}; - const url = await startFixtureServer(fixtureOptions); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }; - - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); fixtureOptions.decisionPackStatus = 403; - await expect(runAsync(["decision-pack", "--login", "JSONbored", "--json"], env)).rejects.toThrow(/LoopOver API 403/); + await expect(captureStdout(() => mod.runCli(["decision-pack", "--login", "JSONbored", "--json"]))).rejects.toThrow(/LoopOver API 403/); }); it("does not use stale decision-pack cache for non-JSON authorization failures", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const fixtureOptions: { - decisionPackStatus?: number; - decisionPackErrorBody?: string; - decisionPackErrorContentType?: string; - repoDecisionStatus?: number; - repoDecisionErrorBody?: string; - repoDecisionErrorContentType?: string; - } = {}; - const url = await startFixtureServer(fixtureOptions); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }; - - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); fixtureOptions.decisionPackStatus = 403; fixtureOptions.decisionPackErrorBody = "forbidden"; fixtureOptions.decisionPackErrorContentType = "text/html"; @@ -243,83 +277,74 @@ describe("loopover-mcp CLI — packets", () => { fixtureOptions.repoDecisionErrorBody = "forbidden"; fixtureOptions.repoDecisionErrorContentType = "text/html"; - await expect(runAsync(["decision-pack", "--login", "JSONbored", "--json"], env)).rejects.toThrow(/LoopOver API 403/); - await expect(runAsync(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"], env)).rejects.toThrow(/LoopOver API 403/); + await expect(captureStdout(() => mod.runCli(["decision-pack", "--login", "JSONbored", "--json"]))).rejects.toThrow(/LoopOver API 403/); + await expect(captureStdout(() => mod.runCli(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"]))).rejects.toThrow( + /LoopOver API 403/, + ); }); it("does not use stale decision-pack cache when local credentials are missing", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }; - - await runAsync(["decision-pack", "--login", "JSONbored", "--json"], env); - const withoutToken = { - ...env, - LOOPOVER_API_TOKEN: "", - LOOPOVER_TOKEN: "", - LOOPOVER_MCP_TOKEN: "", - }; + await runCliJson(["decision-pack", "--login", "JSONbored", "--json"]); + process.env.LOOPOVER_API_TOKEN = ""; + process.env.LOOPOVER_TOKEN = ""; + process.env.LOOPOVER_MCP_TOKEN = ""; - await expect(runAsync(["decision-pack", "--login", "JSONbored", "--json"], withoutToken)).rejects.toThrow(/Run `loopover-mcp login`/); - await expect(runAsync(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"], withoutToken)).rejects.toThrow( + await expect(captureStdout(() => mod.runCli(["decision-pack", "--login", "JSONbored", "--json"]))).rejects.toThrow(/Run `loopover-mcp login`/); + await expect(captureStdout(() => mod.runCli(["repo-decision", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"]))).rejects.toThrow( /Run `loopover-mcp login`/, ); }); it("runs base-agent CLI commands against API fixtures", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }; - - const plan = JSON.parse(await runAsync(["agent", "plan", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"], env)) as { + const plan = (await runCliJson(["agent", "plan", "--login", "JSONbored", "--repo", "JSONbored/loopover", "--json"])) as { run: { id: string; status: string }; actions: Array<{ actionType: string }>; }; expect(plan.run).toMatchObject({ id: "run-1", status: "completed" }); expect(plan.actions[0]).toMatchObject({ actionType: "choose_next_work" }); - const planText = await runAsync(["agent", "plan", "--login", "JSONbored", "--repo", "JSONbored/loopover"], env); + const planText = await captureStdout(() => mod.runCli(["agent", "plan", "--login", "JSONbored", "--repo", "JSONbored/loopover"])); expect(planText).toContain("why now:"); expect(planText).toContain("impact:"); expect(planText).toContain("rerun:"); expect(planText).not.toMatch(/wallet|hotkey|raw trust|payout|farming|private reviewability|public score estimate/i); - const statusPayload = JSON.parse(await runAsync(["agent", "status", "run-1", "--json"], env)) as { run: { id: string } }; + const statusPayload = (await runCliJson(["agent", "status", "run-1", "--json"])) as { run: { id: string } }; expect(statusPayload.run.id).toBe("run-1"); - const explain = JSON.parse(await runAsync(["agent", "explain", "run-1", "--json"], env)) as { topAction: { actionType: string } }; + const explain = (await runCliJson(["agent", "explain", "run-1", "--json"])) as { topAction: { actionType: string } }; expect(explain.topAction.actionType).toBe("choose_next_work"); }); it("prints copy-paste public-safe markdown for agent packet output", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - git(tempDir, "init"); - git(tempDir, "config", "user.email", "test@example.com"); - git(tempDir, "config", "user.name", "LoopOver Test"); - git(tempDir, "config", "commit.gpgsign", "false"); - git(tempDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); - writeFileSync(join(tempDir, "README.md"), "fixture\n"); - git(tempDir, "add", "README.md"); - git(tempDir, "commit", "-m", "initial commit"); - git(tempDir, "checkout", "-b", "codex/public-safe-pr-packets"); - mkdirSync(join(tempDir, "src")); - writeFileSync(join(tempDir, "src/packet.ts"), "export const packet = true;\n"); - const url = await startFixtureServer(); - const output = await runAsync( - ["agent", "packet", "--login", "oktofeesh1", "--cwd", tempDir, "--base", "HEAD", "--body", "Closes #39", "--validation", "passed|npm test|packet tests passed"], - { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }, + const repoDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); + tempDir = repoDir; + git(repoDir, "init"); + git(repoDir, "config", "user.email", "test@example.com"); + git(repoDir, "config", "user.name", "LoopOver Test"); + git(repoDir, "config", "commit.gpgsign", "false"); + git(repoDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); + writeFileSync(join(repoDir, "README.md"), "fixture\n"); + git(repoDir, "add", "README.md"); + git(repoDir, "commit", "-m", "initial commit"); + git(repoDir, "checkout", "-b", "codex/public-safe-pr-packets"); + mkdirSync(join(repoDir, "src")); + writeFileSync(join(repoDir, "src/packet.ts"), "export const packet = true;\n"); + const output = await captureStdout(() => + mod.runCli([ + "agent", + "packet", + "--login", + "oktofeesh1", + "--cwd", + repoDir, + "--base", + "HEAD", + "--body", + "Closes #39", + "--validation", + "passed|npm test|packet tests passed", + ]), ); expect(output).toContain("# Public-safe PR packet"); @@ -329,16 +354,17 @@ describe("loopover-mcp CLI — packets", () => { }); it("rejects unsafe server-provided packet markdown before non-json output", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - git(tempDir, "init"); - git(tempDir, "config", "user.email", "test@example.com"); - git(tempDir, "config", "user.name", "LoopOver Test"); - git(tempDir, "config", "commit.gpgsign", "false"); - git(tempDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); - writeFileSync(join(tempDir, "README.md"), "fixture\n"); - git(tempDir, "add", "README.md"); - git(tempDir, "commit", "-m", "initial commit"); - git(tempDir, "checkout", "-b", "codex/public-safe-pr-packets"); + const repoDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); + tempDir = repoDir; + git(repoDir, "init"); + git(repoDir, "config", "user.email", "test@example.com"); + git(repoDir, "config", "user.name", "LoopOver Test"); + git(repoDir, "config", "commit.gpgsign", "false"); + git(repoDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); + writeFileSync(join(repoDir, "README.md"), "fixture\n"); + git(repoDir, "add", "README.md"); + git(repoDir, "commit", "-m", "initial commit"); + git(repoDir, "checkout", "-b", "codex/public-safe-pr-packets"); for (const unsafePhrase of [ "score: 1.15", @@ -352,42 +378,33 @@ describe("loopover-mcp CLI — packets", () => { "trust_score: 0.4", "log path C:\\Users\\alice\\workspace\\raw.log", ]) { - await closeFixtureServer(); - const url = await startFixtureServer({ packetMarkdown: `# Public-safe PR packet\n\n- ${unsafePhrase}\n` }); - await expect( - runAsync( - ["agent", "packet", "--login", "oktofeesh1", "--cwd", tempDir, "--base", "HEAD"], - { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_API_TIMEOUT_MS: "3000", - }, - ), - ).rejects.toThrow("Refusing to print unsafe public packet markdown from the server."); + fixtureOptions.packetMarkdown = `# Public-safe PR packet\n\n- ${unsafePhrase}\n`; + await expect(captureStdout(() => mod.runCli(["agent", "packet", "--login", "oktofeesh1", "--cwd", repoDir, "--base", "HEAD"]))).rejects.toThrow( + "Refusing to print unsafe public packet markdown from the server.", + ); } }, 45000); it("sends bounded structured validation summaries without local logs", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - git(tempDir, "init"); - git(tempDir, "config", "user.email", "test@example.com"); - git(tempDir, "config", "user.name", "LoopOver Test"); - git(tempDir, "config", "commit.gpgsign", "false"); - git(tempDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); - writeFileSync(join(tempDir, "README.md"), "fixture\n"); - git(tempDir, "add", "README.md"); - git(tempDir, "commit", "-m", "initial commit"); - const requests: unknown[] = []; - const url = await startFixtureServer({ onPacketRequest: (body) => requests.push(body) }); - await runAsync( - [ + const repoDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); + tempDir = repoDir; + git(repoDir, "init"); + git(repoDir, "config", "user.email", "test@example.com"); + git(repoDir, "config", "user.name", "LoopOver Test"); + git(repoDir, "config", "commit.gpgsign", "false"); + git(repoDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); + writeFileSync(join(repoDir, "README.md"), "fixture\n"); + git(repoDir, "add", "README.md"); + git(repoDir, "commit", "-m", "initial commit"); + packetRequests.length = 0; + await captureStdout(() => + mod.runCli([ "agent", "packet", "--login", "oktofeesh1", "--cwd", - tempDir, + repoDir, "--base", "HEAD", "--validation", @@ -401,15 +418,10 @@ describe("loopover-mcp CLI — packets", () => { "--validation-summary", "lint failed at C:/Users/alice/raw.log and /tmp/raw.log", "--json", - ], - { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }, + ]), ); - const packet = requests[0] as { validation: Array<{ command: string; status: string; durationMs?: number; exitCode?: number; summary?: string }> }; + const packet = packetRequests[0] as { validation: Array<{ command: string; status: string; durationMs?: number; exitCode?: number; summary?: string }> }; expect(packet.validation).toEqual( expect.arrayContaining([ expect.objectContaining({ command: "npm run test:unit", status: "focused", durationMs: 1234, exitCode: 0 }), @@ -421,20 +433,20 @@ describe("loopover-mcp CLI — packets", () => { }); it("sends branch eligibility metadata without local source contents", async () => { - tempDir = createPacketRepo(); - mkdirSync(join(tempDir, "src")); - writeFileSync(join(tempDir, "src/eligible.ts"), "export const source = 'must stay local';\n"); - git(tempDir, "add", "src/eligible.ts"); - const requests: unknown[] = []; - const url = await startFixtureServer({ onPacketRequest: (body) => requests.push(body) }); - await runAsync( - [ + const repoDir = createPacketRepo(); + tempDir = repoDir; + mkdirSync(join(repoDir, "src")); + writeFileSync(join(repoDir, "src/eligible.ts"), "export const source = 'must stay local';\n"); + git(repoDir, "add", "src/eligible.ts"); + packetRequests.length = 0; + await captureStdout(() => + mod.runCli([ "agent", "packet", "--login", "oktofeesh1", "--cwd", - tempDir, + repoDir, "--base", "HEAD", "--body", @@ -448,40 +460,35 @@ describe("loopover-mcp CLI — packets", () => { "--branch-eligibility-stale", "false", "--json", - ], - { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }, + ]), ); - const packet = requests[0] as { branchEligibility: { status: string; source: string; reason: string; stale: boolean }; changedFiles: Array<{ path: string }> }; + const packet = packetRequests[0] as { branchEligibility: { status: string; source: string; reason: string; stale: boolean }; changedFiles: Array<{ path: string }> }; expect(packet.branchEligibility).toMatchObject({ status: "ineligible", source: "github_metadata", reason: "head branch is not eligible", stale: false }); expect(packet.changedFiles).toEqual(expect.arrayContaining([expect.objectContaining({ path: "src/eligible.ts" })])); expect(JSON.stringify(packet)).not.toMatch(/must stay local|export const source/); }); it("classifies nonzero validation status phrases as failed", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - git(tempDir, "init"); - git(tempDir, "config", "user.email", "test@example.com"); - git(tempDir, "config", "user.name", "LoopOver Test"); - git(tempDir, "config", "commit.gpgsign", "false"); - git(tempDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); - writeFileSync(join(tempDir, "README.md"), "fixture\n"); - git(tempDir, "add", "README.md"); - git(tempDir, "commit", "-m", "initial commit"); - const requests: unknown[] = []; - const url = await startFixtureServer({ onPacketRequest: (body) => requests.push(body) }); - await runAsync( - [ + const repoDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); + tempDir = repoDir; + git(repoDir, "init"); + git(repoDir, "config", "user.email", "test@example.com"); + git(repoDir, "config", "user.name", "LoopOver Test"); + git(repoDir, "config", "commit.gpgsign", "false"); + git(repoDir, "remote", "add", "origin", "git@github.com:JSONbored/loopover.git"); + writeFileSync(join(repoDir, "README.md"), "fixture\n"); + git(repoDir, "add", "README.md"); + git(repoDir, "commit", "-m", "initial commit"); + packetRequests.length = 0; + await captureStdout(() => + mod.runCli([ "agent", "packet", "--login", "oktofeesh1", "--cwd", - tempDir, + repoDir, "--base", "HEAD", "--validation-command", @@ -489,21 +496,17 @@ describe("loopover-mcp CLI — packets", () => { "--validation-status", "status: 2", "--json", - ], - { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - }, + ]), ); - const packet = requests[0] as { validation: Array<{ command: string; status: string; exitCode?: number }> }; + const packet = packetRequests[0] as { validation: Array<{ command: string; status: string; exitCode?: number }> }; expect(packet.validation).toEqual(expect.arrayContaining([expect.objectContaining({ command: "npm test", status: "failed", exitCode: 2 })])); }); it("classifies bare nonzero validation statuses as failed", async () => { - tempDir = createPacketRepo(); - const validation = await capturePacketValidation(tempDir, [ + const repoDir = createPacketRepo(); + tempDir = repoDir; + const validation = await captureInProcessPacketValidation(repoDir, [ "--validation", "npm test|1", "--validation-command", @@ -521,8 +524,9 @@ describe("loopover-mcp CLI — packets", () => { }); it("does not infer HTTP status summaries as process exit codes", async () => { - tempDir = createPacketRepo(); - const validation = await capturePacketValidation(tempDir, ["--validation", "npm run e2e|HTTP status 200 OK"]); + const repoDir = createPacketRepo(); + tempDir = repoDir; + const validation = await captureInProcessPacketValidation(repoDir, ["--validation", "npm run e2e|HTTP status 200 OK"]); expect(validation).toEqual( expect.arrayContaining([expect.objectContaining({ command: "npm run e2e", status: "not_run", summary: "HTTP status 200 OK" })]), @@ -531,8 +535,9 @@ describe("loopover-mcp CLI — packets", () => { }); it("infers expanded validation failures from summaries when status is absent", async () => { - tempDir = createPacketRepo(); - const validation = await capturePacketValidation(tempDir, ["--validation-command", "npm test", "--validation-summary", "exit code 1"]); + const repoDir = createPacketRepo(); + tempDir = repoDir; + const validation = await captureInProcessPacketValidation(repoDir, ["--validation-command", "npm test", "--validation-summary", "exit code 1"]); expect(validation).toEqual( expect.arrayContaining([expect.objectContaining({ command: "npm test", status: "failed", exitCode: 1, summary: "exit code 1" })]), @@ -540,8 +545,9 @@ describe("loopover-mcp CLI — packets", () => { }); it("redacts space-containing local paths and private metric values from validation text", async () => { - tempDir = createPacketRepo(); - const validation = await capturePacketValidation(tempDir, [ + const repoDir = createPacketRepo(); + tempDir = repoDir; + const validation = await captureInProcessPacketValidation(repoDir, [ "--validation-command", "node /Users/Alice Smith/project/run.js", "--validation-status", diff --git a/test/unit/mcp-cli-pr-outcomes.test.ts b/test/unit/mcp-cli-pr-outcomes.test.ts index 2ea2579aeb..c3530a7745 100644 --- a/test/unit/mcp-cli-pr-outcomes.test.ts +++ b/test/unit/mcp-cli-pr-outcomes.test.ts @@ -1,91 +1,175 @@ // #6747: CLI + stdio mirrors for loopover_pr_outcome. The host MCP tool already existed; this pins the // REST-backed stdio proxy and shell CLI against the same fixture payload. +// #8587: converted to in-process — the stdio proxy connects to the bin's exported `server` over an +// InMemoryTransport pair, and the CLI mirror calls the exported runCli with stdout captured. The fixture +// server starts once BEFORE the dynamic import (the bin reads LOOPOVER_API_URL at module load); per-test +// response overrides mutate `fixtureOptions`, which the harness reads per request. Only the exit-code / +// failure-envelope case still spawns a real subprocess. import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { closeFixtureServer, prOutcomesFixture, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; -const bin = join(process.cwd(), "packages/loopover-mcp/dist/bin/loopover-mcp.js"); +// 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"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { + closeFixtureServer, + prOutcomesFixture, + runExpectingFailure, + startFixtureServer, +} from "./support/mcp-cli-harness"; -let client: Client; -let transport: StdioClientTransport; -let configDir: string; -let apiUrl: string; -let capturedRequests: Array<{ url: string; method: string }>; +type BinModule = { + runCli: (args: string[]) => Promise; + server: { connect: (transport: unknown) => Promise }; +}; -async function connect() { +const capturedRequests: Array<{ url: string; method: string }> = []; +const fixtureOptions: NonNullable[0]> = { + onApiRequest: (request) => { + if (request.url && request.url.includes("/pr-outcomes")) { + capturedRequests.push({ + url: request.url ?? "", + method: request.method ?? "GET", + }); + } + }, +}; +let mod: BinModule; +let apiUrl = ""; +let configDir = ""; + +beforeAll(async () => { configDir = mkdtempSync(join(tmpdir(), "loopover-pr-outcomes-")); - capturedRequests = []; - apiUrl = await startFixtureServer({ - onApiRequest: (request) => { - if (request.url && request.url.includes("/pr-outcomes")) { - capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" }); - } - }, - }); - transport = new StdioClientTransport({ - command: "node", - args: [bin, "--stdio"], - env: { - ...process.env, - LOOPOVER_CONFIG_DIR: configDir, - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_API_TIMEOUT_MS: "5000", - }, - }); - client = new Client({ name: "pr-outcomes-test", version: "0.0.1" }); - await client.connect(transport); -} + apiUrl = await startFixtureServer(fixtureOptions); + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = configDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); -async function disconnect() { - await client.close().catch(() => undefined); +afterAll(async () => { await closeFixtureServer(); if (configDir) rmSync(configDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); + +beforeEach(() => { + capturedRequests.length = 0; +}); + +afterEach(() => { + delete fixtureOptions.prOutcomes; +}); + +async function connectClient() { + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await mod.server.connect(serverTransport); + const client = new Client( + { name: "pr-outcomes-test", version: "0.0.1" }, + { capabilities: {} }, + ); + await client.connect(clientTransport); + return client; } -describe("loopover_pr_outcome stdio proxy (#6747)", () => { - beforeEach(connect); - afterEach(disconnect); +async function captureStdout( + fn: () => Promise, +): Promise { + 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(""); +} +describe("loopover_pr_outcome stdio proxy (#6747)", () => { it("registers the tool in the stdio server tool list", async () => { - const { tools } = await client.listTools(); - expect(tools.map((t) => t.name)).toContain("loopover_pr_outcome"); + const client = await connectClient(); + try { + const { tools } = await client.listTools(); + expect(tools.map((t) => t.name)).toContain("loopover_pr_outcome"); + } finally { + await client.close().catch(() => undefined); + } }); it("proxies login (+ optional limit) to GET /v1/contributors/:login/pr-outcomes", async () => { - const result = await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored", limit: 10 } }); - expect(capturedRequests.length).toBe(1); - const captured = capturedRequests[0]!; - expect(captured.url).toContain("/v1/contributors/JSONbored/pr-outcomes"); - expect(captured.url).toContain("limit=10"); - expect(captured.method).toBe("GET"); - expect(result.isError).toBeFalsy(); - const text = JSON.stringify(result); - expect(text).toContain("JSONbored/loopover"); - expect(text).toContain(prOutcomesFixture().summary); + const client = await connectClient(); + try { + const result = await client.callTool({ + name: "loopover_pr_outcome", + arguments: { login: "JSONbored", limit: 10 }, + }); + expect(capturedRequests.length).toBe(1); + const captured = capturedRequests[0]!; + expect(captured.url).toContain("/v1/contributors/JSONbored/pr-outcomes"); + expect(captured.url).toContain("limit=10"); + expect(captured.method).toBe("GET"); + expect(result.isError).toBeFalsy(); + const text = JSON.stringify(result); + expect(text).toContain("JSONbored/loopover"); + expect(text).toContain(prOutcomesFixture().summary); + } finally { + await client.close().catch(() => undefined); + } }); }); describe("loopover-mcp pr-outcomes CLI (#6747)", () => { - beforeEach(connect); - afterEach(disconnect); - it("--json emits exactly the payload the MCP tool surfaces for the same login (mirror parity)", async () => { - const viaTool = await client.callTool({ name: "loopover_pr_outcome", arguments: { login: "JSONbored" } }); - const toolData = (viaTool as { structuredContent?: unknown }).structuredContent; - const viaCli = JSON.parse( - await runAsync(["pr-outcomes", "--login", "JSONbored", "--json"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }), - ); - expect(viaCli).toEqual(prOutcomesFixture()); - if (toolData !== undefined) expect(viaCli).toEqual(toolData); + const client = await connectClient(); + try { + const viaTool = await client.callTool({ + name: "loopover_pr_outcome", + arguments: { login: "JSONbored" }, + }); + const toolData = (viaTool as { structuredContent?: unknown }) + .structuredContent; + const viaCli = JSON.parse( + await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored", "--json"]), + ), + ); + expect(viaCli).toEqual(prOutcomesFixture()); + if (toolData !== undefined) expect(viaCli).toEqual(toolData); + } finally { + await client.close().catch(() => undefined); + } }); it("prints the API summary and one line per outcome", async () => { - const out = await runAsync(["pr-outcomes", "--login", "JSONbored"], { LOOPOVER_API_URL: apiUrl, LOOPOVER_TOKEN: "session-token" }); + const out = await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored"]), + ); const fixture = prOutcomesFixture(); expect(out).toContain(fixture.summary); expect(out).toContain("JSONbored/loopover#42 [merged]"); @@ -93,18 +177,25 @@ describe("loopover-mcp pr-outcomes CLI (#6747)", () => { }); it("forwards --limit and resolves login from LOOPOVER_LOGIN / GITHUB_LOGIN", async () => { - await runAsync(["pr-outcomes", "--json", "--limit", "5"], { - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_LOGIN: "JSONbored", - }); + process.env.LOOPOVER_LOGIN = "JSONbored"; + try { + await captureStdout(() => + mod.runCli(["pr-outcomes", "--json", "--limit", "5"]), + ); + } finally { + delete process.env.LOOPOVER_LOGIN; + } expect(capturedRequests.at(-1)?.url).toContain("limit=5"); - const viaGithubLogin = await runAsync(["pr-outcomes", "--json"], { - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - GITHUB_LOGIN: "JSONbored", - }); + process.env.GITHUB_LOGIN = "JSONbored"; + let viaGithubLogin = ""; + try { + viaGithubLogin = await captureStdout(() => + mod.runCli(["pr-outcomes", "--json"]), + ); + } finally { + delete process.env.GITHUB_LOGIN; + } expect(JSON.parse(viaGithubLogin)).toEqual(prOutcomesFixture()); }); @@ -116,64 +207,98 @@ describe("loopover-mcp pr-outcomes CLI (#6747)", () => { GITHUB_LOGIN: "", }); expect(noLogin.status).toBe(1); - expect(`${noLogin.stdout}${noLogin.stderr}`).toMatch(/Pass --login or set LOOPOVER_LOGIN\./); + expect(`${noLogin.stdout}${noLogin.stderr}`).toMatch( + /Pass --login or set LOOPOVER_LOGIN\./, + ); - const badLimit = runExpectingFailure(["pr-outcomes", "--login", "JSONbored", "--limit", "0"], { - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - }); + const badLimit = runExpectingFailure( + ["pr-outcomes", "--login", "JSONbored", "--limit", "0"], + { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }, + ); expect(badLimit.status).toBe(1); - expect(`${badLimit.stdout}${badLimit.stderr}`).toMatch(/integer between 1 and 100/); + expect(`${badLimit.stdout}${badLimit.stderr}`).toMatch( + /integer between 1 and 100/, + ); - const bareLimit = runExpectingFailure(["pr-outcomes", "--login", "JSONbored", "--limit", "101"], { - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - }); + const bareLimit = runExpectingFailure( + ["pr-outcomes", "--login", "JSONbored", "--limit", "101"], + { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }, + ); expect(bareLimit.status).toBe(1); }); it("falls back when the API omits summary and prints null pull numbers / empty attributions", async () => { - await closeFixtureServer(); - const sparseUrl = await startFixtureServer({ - prOutcomes: { - summary: " ", - outcomes: [{ repoFullName: "a/b", pullNumber: null, outcome: "merged", attribution: "", deeplink: "https://x", recordedAt: "t" }], - }, - }); - const env = { LOOPOVER_API_URL: sparseUrl, LOOPOVER_TOKEN: "session-token" }; - const plain = await runAsync(["pr-outcomes", "--login", "JSONbored"], env); + fixtureOptions.prOutcomes = { + summary: " ", + outcomes: [ + { + repoFullName: "a/b", + pullNumber: null, + outcome: "merged", + attribution: "", + deeplink: "https://x", + recordedAt: "t", + }, + ], + }; + const plain = await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored"]), + ); expect(plain).toContain("LoopOver post-merge outcomes for JSONbored."); expect(plain).toContain("a/b#? [merged]"); }); it("strips ANSI escapes from API-chosen text on the plain-text path but not from --json", async () => { - await closeFixtureServer(); - const hostileUrl = await startFixtureServer({ - prOutcomes: { summary: "\u001b[31mFAKE PASS\u001b[0m", outcomes: [{ repoFullName: "a/b", pullNumber: 1, outcome: "merged", attribution: "\u001b[2Krewritten", deeplink: "https://x", recordedAt: "t" }] }, - }); - const env = { LOOPOVER_API_URL: hostileUrl, LOOPOVER_TOKEN: "session-token" }; + fixtureOptions.prOutcomes = { + summary: "\u001b[31mFAKE PASS\u001b[0m", + outcomes: [ + { + repoFullName: "a/b", + pullNumber: 1, + outcome: "merged", + attribution: "\u001b[2Krewritten", + deeplink: "https://x", + recordedAt: "t", + }, + ], + }; - const plain = await runAsync(["pr-outcomes", "--login", "JSONbored"], env); + const plain = await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored"]), + ); expect(plain).not.toContain("\u001b"); expect(plain).toContain("FAKE PASS"); expect(plain).toContain("rewritten"); - const asJson = await runAsync(["pr-outcomes", "--login", "JSONbored", "--json"], env); + const asJson = await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored", "--json"]), + ); expect(JSON.parse(asJson).summary).toBe("\u001b[31mFAKE PASS\u001b[0m"); }); it("ignores a bare --limit flag (no value) and still returns outcomes", async () => { - const out = await runAsync(["pr-outcomes", "--login", "JSONbored", "--limit", "--json"], { - LOOPOVER_API_URL: apiUrl, - LOOPOVER_TOKEN: "session-token", - }); + const out = await captureStdout(() => + mod.runCli(["pr-outcomes", "--login", "JSONbored", "--limit", "--json"]), + ); expect(JSON.parse(out)).toEqual(prOutcomesFixture()); expect(capturedRequests.at(-1)?.url).not.toContain("limit="); }); - it("documents itself in --help and in the shell-completion command list", () => { - expect(run(["--help"])).toContain("loopover-mcp pr-outcomes --login [--limit N] [--json]"); - expect(run(["pr-outcomes", "--help"])).toContain("Mirrors the loopover_pr_outcome MCP tool"); - expect(run(["completion", "bash"])).toContain("pr-outcomes"); + it("documents itself in --help and in the shell-completion command list", async () => { + expect(await captureStdout(() => mod.runCli(["--help"]))).toContain( + "loopover-mcp pr-outcomes --login [--limit N] [--json]", + ); + expect( + await captureStdout(() => mod.runCli(["pr-outcomes", "--help"])), + ).toContain("Mirrors the loopover_pr_outcome MCP tool"); + expect( + await captureStdout(() => mod.runCli(["completion", "bash"])), + ).toContain("pr-outcomes"); }); }); diff --git a/test/unit/mcp-cli-profiles.test.ts b/test/unit/mcp-cli-profiles.test.ts index 0fbc56c219..44ba3898b0 100644 --- a/test/unit/mcp-cli-profiles.test.ts +++ b/test/unit/mcp-cli-profiles.test.ts @@ -2,27 +2,111 @@ import { type IncomingMessage } from "node:http"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { closeFixtureServer, run, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { closeFixtureServer, runAsync, startFixtureServer } from "./support/mcp-cli-harness"; import mcpPackageJson from "../../packages/loopover-mcp/package.json"; +// #8587: business-payload cases (status/changelog/telemetry-headers/device-flow login and the seeded +// profile-list case) call the exported runCli in-process (pattern from +// mcp-cli-contributor-profile-inprocess.test.ts). Multi-profile scenarios stay real subprocesses: the bin +// resolves the active profile from argv/config AT MODULE LOAD (bin/loopover-mcp.ts ~lines 365-368), so +// `--profile` selection and `profile switch` visibility only exist across separate process startups. +// One fixture server (started before the import, closed in afterAll) serves both transports. +type BinModule = { + runCli: (args: string[]) => Promise; +}; + +// Only the committed .ts source is imported (never dist); the variable indirection mirrors the template's +// MODULES array so tsc does not statically flag the .ts specifier (allowImportingTsExtensions is off). +const BIN_MODULE = "../../packages/loopover-mcp/bin/loopover-mcp.ts"; + +let mod: BinModule; +let sharedConfigDir = ""; +let apiUrl = ""; +const capturedRequests: Array<{ url: string | undefined; authorization: string | undefined; headers: IncomingMessage["headers"] }> = []; + +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} + +function runInProcess(args: string[]): Promise { + return captureStdout(() => mod.runCli(args)); +} + describe("loopover-mcp CLI — profiles", () => { let tempDir: string | null = null; - afterEach(async () => { + beforeAll(async () => { + sharedConfigDir = mkdtempSync(join(tmpdir(), "loopover-profiles-inprocess-")); + // Seed the config the in-process module loads at import: the ndjson test's two credential-free profiles + // (default + active "beta"); profile list needs only names to enumerate, so the fixture carries no + // session token — the streaming format is what that test exercises. + writeFileSync( + join(sharedConfigDir, "config.json"), + JSON.stringify( + { + apiUrl: "https://api.example.test", + activeProfile: "beta", + profiles: { + default: { session: { login: "default-user", scopes: [] } }, + beta: { session: { login: "beta-user", scopes: [] } }, + }, + }, + null, + 2, + ), + ); + apiUrl = await startFixtureServer({ + onApiRequest: (request) => capturedRequests.push({ url: request.url, authorization: request.headers.authorization, headers: request.headers }), + // #6792: only the device-flow login test polls these routes; a transient 429 precedes success. + deviceFlowStart: { deviceCode: "device-code-1", userCode: "ABCD-1234", verificationUri: "https://github.com/login/device", interval: 0 }, + deviceFlowPollResponses: [ + { status: 429, retryAfterSeconds: 1, body: { error: "rate_limited", routeClass: "normal" } }, + { body: { token: "device-session-token", login: "JSONbored", expiresAt: "2026-06-02T00:00:00.000Z", scopes: ["repo"] } }, + ], + }); + // The bin reads these at module load, so set them BEFORE the dynamic import... + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_CONFIG_DIR = sharedConfigDir; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; + // ...and delete the module-load ones right after import so the kept subprocess tests only see the env + // they pass explicitly. + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_CONFIG_DIR; + }, 120_000); + + afterAll(async () => { await closeFixtureServer(); + if (sharedConfigDir) rmSync(sharedConfigDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; + }); + + beforeEach(() => { + capturedRequests.length = 0; + }); + + afterEach(() => { if (tempDir) rmSync(tempDir, { recursive: true, force: true }); tempDir = null; }); it("stores, switches, and reports named MCP profiles without mixing sessions", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const requests: Array<{ url: string | undefined; authorization: string | undefined }> = []; - const url = await startFixtureServer({ - onApiRequest: (request) => requests.push({ url: request.url, authorization: request.headers.authorization }), - }); const env = { - LOOPOVER_API_URL: url, + LOOPOVER_API_URL: apiUrl, LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; @@ -48,7 +132,7 @@ describe("loopover-mcp CLI — profiles", () => { expect(secondWhoami).toMatchObject({ profile: "okto", login: "oktofeesh1" }); expect(switched.activeProfile).toBe("jsonbored"); expect(activeWhoami).toMatchObject({ profile: "jsonbored", login: "JSONbored" }); - expect(requests).toEqual( + expect(capturedRequests).toEqual( expect.arrayContaining([ expect.objectContaining({ url: "/v1/auth/session", authorization: "Bearer session-jsonbored" }), expect.objectContaining({ url: "/v1/auth/session", authorization: "Bearer session-okto" }), @@ -57,29 +141,10 @@ describe("loopover-mcp CLI — profiles", () => { expect(JSON.stringify(list)).not.toMatch(/session-jsonbored|session-okto|github-jsonbored|github-okto|loopover-cli-/); }, 45_000); - it("profile list --format ndjson streams one JSON object per profile (and --json stays pretty)", () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const configPath = join(tempDir, "config.json"); - // Two credential-free profiles (default + active "beta"); profile list needs only names to enumerate, - // so the fixture carries no session token — the streaming format is what this exercises. - writeFileSync( - configPath, - JSON.stringify( - { - apiUrl: "https://api.example.test", - activeProfile: "beta", - profiles: { - default: { session: { login: "default-user", scopes: [] } }, - beta: { session: { login: "beta-user", scopes: [] } }, - }, - }, - null, - 2, - ), - ); - const env = { LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" }; - - const lines = run(["profile", "list", "--format", "ndjson"], env).trim().split("\n"); + it("profile list --format ndjson streams one JSON object per profile (and --json stays pretty)", async () => { + // The two-profile fixture (default + active "beta") is the config the in-process module loaded at + // import — see the beforeAll seed. + const lines = (await runInProcess(["profile", "list", "--format", "ndjson"])).trim().split("\n"); expect(lines).toHaveLength(2); const parsed = lines.map((line) => JSON.parse(line) as { name: string; active: boolean }); expect(parsed.map((p) => p.name).sort()).toEqual(["beta", "default"]); @@ -88,19 +153,15 @@ describe("loopover-mcp CLI — profiles", () => { // Each line is a bare profile object — not the {activeProfile, profiles} wrapper. for (const line of lines) expect(line).not.toContain("activeProfile"); // --json still returns the pretty wrapper object (unchanged behavior). - const pretty = JSON.parse(run(["profile", "list", "--json"], env)) as { activeProfile: string; profiles: unknown[] }; + const pretty = JSON.parse(await runInProcess(["profile", "list", "--json"])) as { activeProfile: string; profiles: unknown[] }; expect(pretty).toMatchObject({ activeProfile: "beta" }); expect(pretty.profiles).toHaveLength(2); }); it("keeps environment tokens ahead of active profile sessions", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const requests: Array<{ url: string | undefined; authorization: string | undefined }> = []; - const url = await startFixtureServer({ - onApiRequest: (request) => requests.push({ url: request.url, authorization: request.headers.authorization }), - }); const env = { - LOOPOVER_API_URL: url, + LOOPOVER_API_URL: apiUrl, LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; @@ -112,7 +173,7 @@ describe("loopover-mcp CLI — profiles", () => { expect(whoami).toMatchObject({ profile: "jsonbored", login: "oktofeesh1" }); expect(status).toMatchObject({ auth: { login: "oktofeesh1" }, profile: { tokenSource: "environment" } }); - expect(requests).toEqual(expect.arrayContaining([expect.objectContaining({ url: "/v1/auth/session", authorization: "Bearer session-okto" })])); + expect(capturedRequests).toEqual(expect.arrayContaining([expect.objectContaining({ url: "/v1/auth/session", authorization: "Bearer session-okto" })])); }); it("removes the default profile without rehydrating its legacy session token", async () => { @@ -149,12 +210,8 @@ describe("loopover-mcp CLI — profiles", () => { it("logs out only the selected profile and reports missing profiles safely", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const requests: Array<{ url: string | undefined; authorization: string | undefined }> = []; - const url = await startFixtureServer({ - onApiRequest: (request) => requests.push({ url: request.url, authorization: request.headers.authorization }), - }); const env = { - LOOPOVER_API_URL: url, + LOOPOVER_API_URL: apiUrl, LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; @@ -178,44 +235,36 @@ describe("loopover-mcp CLI — profiles", () => { expect(missingStatus).toMatchObject({ auth: { status: "unauthenticated" }, profile: { name: "missing", configured: false, authenticated: false } }); expect(doctor.profile).toMatchObject({ name: "missing", configured: false }); expect(doctor.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "auth", status: "fail" })])); - expect(requests).toEqual(expect.arrayContaining([expect.objectContaining({ url: "/v1/auth/logout", authorization: "Bearer session-jsonbored" })])); + expect(capturedRequests).toEqual(expect.arrayContaining([expect.objectContaining({ url: "/v1/auth/logout", authorization: "Bearer session-jsonbored" })])); expect(JSON.stringify({ logout, list, missingStatus, doctor })).not.toMatch(/session-jsonbored|session-okto|github-jsonbored|github-okto|loopover-cli-/); }, 45_000); it("reports package status and prints the packaged changelog", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer(); - const status = JSON.parse( - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { package: { name: string; version: string; latestStatus: string }; api: { status: string }; auth: { login: string } }; - - expect(status.package).toMatchObject({ name: "@loopover/mcp", version: mcpPackageJson.version, latestStatus: "skipped" }); - expect(status.api.status).toBe("ok"); - expect(status.auth.login).toBe("JSONbored"); - - const changelog = JSON.parse(run(["changelog", "--json"])) as { package: { version: string }; changelog: string }; + process.env.LOOPOVER_TOKEN = "session-token"; + try { + const status = JSON.parse(await runInProcess(["status", "--json"])) as { package: { name: string; version: string; latestStatus: string }; api: { status: string }; auth: { login: string } }; + + expect(status.package).toMatchObject({ name: "@loopover/mcp", version: mcpPackageJson.version, latestStatus: "skipped" }); + expect(status.api.status).toBe("ok"); + expect(status.auth.login).toBe("JSONbored"); + } finally { + delete process.env.LOOPOVER_TOKEN; + } + + const changelog = JSON.parse(await runInProcess(["changelog", "--json"])) as { package: { version: string }; changelog: string }; expect(changelog.package.version).toBe(mcpPackageJson.version); expect(changelog.changelog).toContain("# Changelog"); }); it("sends redacted MCP package telemetry headers to the API", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const requests: Array<{ url: string | undefined; headers: IncomingMessage["headers"] }> = []; - const url = await startFixtureServer({ onApiRequest: (request) => requests.push({ url: request.url, headers: request.headers }) }); - - await runAsync(["status", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }); + process.env.LOOPOVER_TOKEN = "session-token"; + try { + await runInProcess(["status", "--json"]); + } finally { + delete process.env.LOOPOVER_TOKEN; + } - const sessionRequest = requests.find((request) => request.url === "/v1/auth/session"); + const sessionRequest = capturedRequests.find((request) => request.url === "/v1/auth/session"); expect(sessionRequest?.headers["x-loopover-mcp-package"]).toBe("@loopover/mcp"); expect(sessionRequest?.headers["x-loopover-mcp-version"]).toBe(mcpPackageJson.version); expect(sessionRequest?.headers["x-loopover-mcp-client"]).toBe("loopover-mcp-cli"); @@ -225,26 +274,11 @@ describe("loopover-mcp CLI — profiles", () => { client: sessionRequest?.headers["x-loopover-mcp-client"], }); expect(telemetryHeaders).not.toContain("session-token"); - expect(telemetryHeaders).not.toContain(tempDir); + expect(telemetryHeaders).not.toContain(sharedConfigDir); }); it("#6792: loginWithDeviceFlow backs off and keeps polling through a transient 429 from our own rate limiter instead of aborting", async () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const url = await startFixtureServer({ - deviceFlowStart: { deviceCode: "device-code-1", userCode: "ABCD-1234", verificationUri: "https://github.com/login/device", interval: 0 }, - deviceFlowPollResponses: [ - { status: 429, retryAfterSeconds: 1, body: { error: "rate_limited", routeClass: "normal" } }, - { body: { token: "device-session-token", login: "JSONbored", expiresAt: "2026-06-02T00:00:00.000Z", scopes: ["repo"] } }, - ], - }); - - const login = JSON.parse( - await runAsync(["login", "--json"], { - LOOPOVER_API_URL: url, - LOOPOVER_CONFIG_DIR: tempDir, - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }), - ) as { status: string; login: string }; + const login = JSON.parse(await runInProcess(["login", "--json"])) as { status: string; login: string }; expect(login).toMatchObject({ status: "authenticated", login: "JSONbored" }); }, 20000); diff --git a/test/unit/mcp-cli-review-pr.test.ts b/test/unit/mcp-cli-review-pr.test.ts index 9fdc72df85..13d1a74f5c 100644 --- a/test/unit/mcp-cli-review-pr.test.ts +++ b/test/unit/mcp-cli-review-pr.test.ts @@ -1,42 +1,102 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; + +// 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"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from "vitest"; import { closeFixtureServer, createPacketRepo, localBranchAnalysisFixture, run, - runAsync, startFixtureServer, } from "./support/mcp-cli-harness"; +// #8587: review-pr cases run in-process through the bin's exported runCli (the same dispatcher + +// presentation code the subprocess ran), with stdout captured. The fixture server starts once BEFORE +// the dynamic import because the bin reads LOOPOVER_API_URL at module load; per-test response +// overrides mutate `fixtureOptions`, which the harness reads per request. Only the typo-suggestion +// case still spawns a real subprocess (CLI-level argv error). +type BinModule = { runCli: (args: string[]) => Promise }; + +const fixtureOptions: NonNullable[0]> = + {}; +let mod: BinModule; +let configDir = ""; + +beforeAll(async () => { + configDir = mkdtempSync(join(tmpdir(), "loopover-review-pr-inprocess-")); + const apiUrl = await startFixtureServer(fixtureOptions); + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_TOKEN = "session-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = configDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_TOKEN; + delete process.env.LOOPOVER_API_TIMEOUT_MS; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); + +async function captureStdout( + fn: () => Promise, +): Promise { + 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(""); +} + describe("loopover-mcp CLI — review-pr", () => { let tempDir: string | null = null; - afterEach(async () => { - await closeFixtureServer(); + afterEach(() => { if (tempDir) rmSync(tempDir, { recursive: true, force: true }); tempDir = null; + delete fixtureOptions.localBranchAnalysis; + delete fixtureOptions.slopRiskStatus; + delete fixtureOptions.prTextLintStatus; }); it("composes preflight + slop-risk + pr-text-lint into one passing report", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; + const repoDir = createPacketRepo(); + tempDir = repoDir; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--commit", @@ -46,8 +106,7 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -74,13 +133,13 @@ describe("loopover-mcp CLI — review-pr", () => { /wallet|hotkey|coldkey|reward|trust score/i, ); - const plain = await runAsync( - [ + const plain = await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--commit", @@ -89,8 +148,7 @@ describe("loopover-mcp CLI — review-pr", () => { "Composes preflight + slop-risk + lint-pr-text into one report. Validated with npm test.", "--linked-issue", "1968", - ], - env, + ]), ); expect(plain).toMatch(/Pre-PR review: pass/); expect(plain).toMatch(/- preflight: pass/); @@ -101,27 +159,21 @@ describe("loopover-mcp CLI — review-pr", () => { }); it("flags a warn overall status when the PR body is empty (weak lint verdict)", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; + const repoDir = createPacketRepo(); + tempDir = repoDir; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -136,36 +188,30 @@ describe("loopover-mcp CLI — review-pr", () => { }); it("maps needs_work preflight to a warning instead of passing (regression)", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer({ - localBranchAnalysis: { - ...localBranchAnalysisFixture(), - preflight: { - status: "needs_work", - findings: [ - { - code: "missing_test_evidence", - severity: "warning", - title: "Missing test evidence", - }, - ], - }, + const repoDir = createPacketRepo(); + tempDir = repoDir; + fixtureOptions.localBranchAnalysis = { + ...localBranchAnalysisFixture(), + preflight: { + status: "needs_work", + findings: [ + { + code: "missing_test_evidence", + severity: "warning", + title: "Missing test evidence", + }, + ], }, - }); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--commit", @@ -175,8 +221,7 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -192,36 +237,30 @@ describe("loopover-mcp CLI — review-pr", () => { }); it("maps hold preflight to a failing section", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer({ - localBranchAnalysis: { - ...localBranchAnalysisFixture(), - preflight: { - status: "hold", - findings: [ - { - code: "lane_hold", - severity: "critical", - title: "Lane unavailable", - }, - ], - }, + const repoDir = createPacketRepo(); + tempDir = repoDir; + fixtureOptions.localBranchAnalysis = { + ...localBranchAnalysisFixture(), + preflight: { + status: "hold", + findings: [ + { + code: "lane_hold", + severity: "critical", + title: "Lane unavailable", + }, + ], }, - }); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--commit", @@ -231,8 +270,7 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -248,24 +286,19 @@ describe("loopover-mcp CLI — review-pr", () => { }); it("reads the PR body from --body-file", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer(); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; - const bodyPath = join(tempDir, "pr-body.md"); + const repoDir = createPacketRepo(); + tempDir = repoDir; + const bodyPath = join(repoDir, "pr-body.md"); writeFileSync(bodyPath, "Fixes #1968\n\nValidated with npm test.", "utf8"); const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--body-file", @@ -273,30 +306,25 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { prTextLint: { verdict: string } }; expect(json.prTextLint.verdict).toBe("strong"); }); it("degrades gracefully when the slop-risk endpoint fails, without losing the other sections", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer({ slopRiskStatus: 500 }); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; + const repoDir = createPacketRepo(); + tempDir = repoDir; + fixtureOptions.slopRiskStatus = 500; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--body", @@ -304,8 +332,7 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -323,43 +350,38 @@ describe("loopover-mcp CLI — review-pr", () => { // The pr-text-lint section still succeeded even though slop-risk failed. expect(json.prTextLint).toMatchObject({ verdict: "strong" }); - const plain = await runAsync( - [ + const plain = await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--body", "Validated with npm test.", "--linked-issue", "1968", - ], - env, + ]), ); expect(plain).toMatch(/Slop risk: unavailable \(LoopOver API 500/); expect(plain).toMatch(/PR text lint: strong/); }); it("degrades gracefully when the pr-text-lint endpoint fails, without losing the other sections", async () => { - tempDir = createPacketRepo(); - const url = await startFixtureServer({ prTextLintStatus: 503 }); - const env = { - LOOPOVER_API_URL: url, - LOOPOVER_TOKEN: "session-token", - LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", - }; + const repoDir = createPacketRepo(); + tempDir = repoDir; + fixtureOptions.prTextLintStatus = 503; const json = JSON.parse( - await runAsync( - [ + await captureStdout(() => + mod.runCli([ "review-pr", "--login", "JSONbored", "--cwd", - tempDir, + repoDir, "--repo", "JSONbored/loopover", "--body", @@ -367,8 +389,7 @@ describe("loopover-mcp CLI — review-pr", () => { "--linked-issue", "1968", "--json", - ], - env, + ]), ), ) as { overallStatus: string; @@ -388,20 +409,20 @@ describe("loopover-mcp CLI — review-pr", () => { it("requires --login", async () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - await expect(runAsync(["review-pr", "--cwd", tempDir], {})).rejects.toThrow( + await expect(mod.runCli(["review-pr", "--cwd", tempDir])).rejects.toThrow( /Pass --login/, ); }); - it("prints help", () => { - const help = run(["review-pr", "--help"]); + it("prints help", async () => { + const help = await captureStdout(() => mod.runCli(["review-pr", "--help"])); expect(help).toMatch(/Usage: loopover-mcp review-pr/); expect(help).toMatch(/loopover_review_pr_before_push/); 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"]); + it("prints help for a bare `help` positional too, not a --login error (#6257)", async () => { + const help = await captureStdout(() => mod.runCli(["review-pr", "help"])); expect(help).toMatch(/Usage: loopover-mcp review-pr/); expect(help).not.toMatch(/Pass --login/); }); diff --git a/test/unit/mcp-cli-telemetry.test.ts b/test/unit/mcp-cli-telemetry.test.ts index 56342af644..b82b43a4f6 100644 --- a/test/unit/mcp-cli-telemetry.test.ts +++ b/test/unit/mcp-cli-telemetry.test.ts @@ -1,12 +1,91 @@ -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { closeFixtureServer, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; + +// 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"; +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from "vitest"; +import { + closeFixtureServer, + run, + runAsync, + runExpectingFailure, + startFixtureServer, +} from "./support/mcp-cli-harness"; // #6239: local MCP usage telemetry is opt-in and defaults OFF (per #6228's privacy decision). The opt-in is // a top-level `telemetryEnabled` flag persisted in the same config file `login` uses, and status/doctor/config // surface the current state. These tests exercise both the default-off state and the enable/disable toggle. + +// #8587: most of this file KEEPS real subprocesses on purpose (rule (d)): the bin loads its config file ONCE +// at module import (`const config = loadConfig()`), and `telemetry status` / `status` / `doctor` / `config` +// all report that import-time snapshot. Every scenario that toggles the opt-in and then re-reads it through a +// later invocation therefore genuinely depends on fresh-process config loading — in-process, the re-read +// would keep reporting the stale snapshot (and `telemetry enable` would clobber a session `login` just wrote, +// since it merges into the import-time config). Only the malformed-persisted-value case converts: its config +// file is seeded BEFORE the module import, exactly mirroring a spawned process parsing it at startup. +type BinModule = { runCli: (args: string[]) => Promise }; + +let inProcessConfigDir = ""; +let mod: BinModule; + +beforeAll(async () => { + inProcessConfigDir = mkdtempSync( + join(tmpdir(), "loopover-telemetry-inprocess-"), + ); + // A non-boolean (legacy/hand-edited) value must not be read as an opt-in; written before the import so the + // bin's load-time config parse sees it, like a spawned CLI would at startup. + writeFileSync( + join(inProcessConfigDir, "config.json"), + JSON.stringify({ telemetryEnabled: "true", profiles: {} }), + { mode: 0o600 }, + ); + // Inert API URL: the converted case never calls the API, but the bin resolves this at module load. + process.env.LOOPOVER_API_URL = "http://127.0.0.1:9"; + process.env.LOOPOVER_CONFIG_DIR = inProcessConfigDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(BIN_MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(() => { + if (inProcessConfigDir) + rmSync(inProcessConfigDir, { recursive: true, force: true }); + delete process.env.LOOPOVER_API_URL; + delete process.env.LOOPOVER_CONFIG_DIR; + delete process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK; +}); + +async function captureStdout(fn: () => Promise): Promise { + 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(""); +} describe("loopover-mcp CLI — telemetry opt-in", () => { let tempDir: string | null = null; @@ -19,39 +98,70 @@ describe("loopover-mcp CLI — telemetry opt-in", () => { it("defaults telemetry to off and persists an explicit opt-in across CLI invocations", () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); const configPath = join(tempDir, "config.json"); - const env = { LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" }; + const env = { + LOOPOVER_CONFIG_DIR: tempDir, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }; // Default: nothing configured -> disabled, and status alone never writes a config file. - const before = JSON.parse(run(["telemetry", "status", "--json"], env)) as { telemetry: { enabled: boolean; default: boolean } }; + const before = JSON.parse(run(["telemetry", "status", "--json"], env)) as { + telemetry: { enabled: boolean; default: boolean }; + }; expect(before.telemetry).toEqual({ enabled: false, default: false }); expect(existsSync(configPath)).toBe(false); // Enabling persists the flag to disk... - const enabled = JSON.parse(run(["telemetry", "enable", "--json"], env)) as { status: string; telemetry: { enabled: boolean } }; - expect(enabled).toMatchObject({ status: "telemetry_enabled", telemetry: { enabled: true, default: false } }); - const saved = JSON.parse(readFileSync(configPath, "utf8")) as { telemetryEnabled?: boolean }; + const enabled = JSON.parse(run(["telemetry", "enable", "--json"], env)) as { + status: string; + telemetry: { enabled: boolean }; + }; + expect(enabled).toMatchObject({ + status: "telemetry_enabled", + telemetry: { enabled: true, default: false }, + }); + const saved = JSON.parse(readFileSync(configPath, "utf8")) as { + telemetryEnabled?: boolean; + }; expect(saved.telemetryEnabled).toBe(true); // ...so a *fresh* process (new invocation) reads the opt-in back as enabled. - const persisted = JSON.parse(run(["telemetry", "status", "--json"], env)) as { telemetry: { enabled: boolean } }; + const persisted = JSON.parse( + run(["telemetry", "status", "--json"], env), + ) as { telemetry: { enabled: boolean } }; expect(persisted.telemetry.enabled).toBe(true); // Disabling clears the flag; with no other durable state, the config file is removed entirely // (rather than left holding `telemetryEnabled: false`). - const disabled = JSON.parse(run(["telemetry", "disable", "--json"], env)) as { status: string; telemetry: { enabled: boolean } }; - expect(disabled).toMatchObject({ status: "telemetry_disabled", telemetry: { enabled: false } }); + const disabled = JSON.parse( + run(["telemetry", "disable", "--json"], env), + ) as { status: string; telemetry: { enabled: boolean } }; + expect(disabled).toMatchObject({ + status: "telemetry_disabled", + telemetry: { enabled: false }, + }); expect(existsSync(configPath)).toBe(false); - const afterDisable = JSON.parse(run(["telemetry", "status", "--json"], env)) as { telemetry: { enabled: boolean } }; + const afterDisable = JSON.parse( + run(["telemetry", "status", "--json"], env), + ) as { telemetry: { enabled: boolean } }; expect(afterDisable.telemetry.enabled).toBe(false); }); it("prints human-readable telemetry state and toggles", () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const env = { LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" }; + const env = { + LOOPOVER_CONFIG_DIR: tempDir, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }; - expect(run(["telemetry", "enable"], env)).toContain("Local MCP usage telemetry enabled."); - expect(run(["telemetry", "status"], env)).toContain("Telemetry: enabled (opt-in)"); - expect(run(["telemetry", "disable"], env)).toContain("Local MCP usage telemetry disabled."); + expect(run(["telemetry", "enable"], env)).toContain( + "Local MCP usage telemetry enabled.", + ); + expect(run(["telemetry", "status"], env)).toContain( + "Telemetry: enabled (opt-in)", + ); + expect(run(["telemetry", "disable"], env)).toContain( + "Local MCP usage telemetry disabled.", + ); // A bare `telemetry` invocation defaults to the status view. expect(run(["telemetry"], env)).toContain("Telemetry: disabled (default)"); }); @@ -67,11 +177,23 @@ describe("loopover-mcp CLI — telemetry opt-in", () => { }; // Default-off surfaces everywhere. - const statusOff = JSON.parse(await runAsync(["status", "--json"], env)) as { telemetry: { enabled: boolean } }; - const configOff = JSON.parse(await runAsync(["config", "--json"], env)) as { telemetry: { enabled: boolean } }; - const doctorOff = JSON.parse(await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], env)) as { + const statusOff = JSON.parse(await runAsync(["status", "--json"], env)) as { + telemetry: { enabled: boolean }; + }; + const configOff = JSON.parse(await runAsync(["config", "--json"], env)) as { + telemetry: { enabled: boolean }; + }; + const doctorOff = JSON.parse( + await runAsync( + ["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], + env, + ), + ) as { telemetry: { enabled: boolean }; - checklist: Array<{ id: string; checks?: Array<{ name: string; status: string }> }>; + checklist: Array<{ + id: string; + checks?: Array<{ name: string; status: string }>; + }>; checks: Array<{ name: string; status: string; detail: string }>; }; expect(statusOff.telemetry.enabled).toBe(false); @@ -79,16 +201,37 @@ describe("loopover-mcp CLI — telemetry opt-in", () => { expect(doctorOff.telemetry.enabled).toBe(false); // The telemetry check is a pass that lives under the Output safety group and states the default. expect(doctorOff.checks).toEqual( - expect.arrayContaining([expect.objectContaining({ name: "telemetry", status: "pass", detail: expect.stringContaining("disabled (default)") })]), + expect.arrayContaining([ + expect.objectContaining({ + name: "telemetry", + status: "pass", + detail: expect.stringContaining("disabled (default)"), + }), + ]), + ); + const outputSafetyOff = doctorOff.checklist.find( + (group) => group.id === "output_safety", + ); + expect(outputSafetyOff?.checks).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "telemetry", status: "pass" }), + ]), ); - const outputSafetyOff = doctorOff.checklist.find((group) => group.id === "output_safety"); - expect(outputSafetyOff?.checks).toEqual(expect.arrayContaining([expect.objectContaining({ name: "telemetry", status: "pass" })])); // After an explicit opt-in, every reporter reflects it. await runAsync(["telemetry", "enable", "--json"], env); - const statusOn = JSON.parse(await runAsync(["status", "--json"], env)) as { telemetry: { enabled: boolean } }; - const configOn = JSON.parse(await runAsync(["config", "--json"], env)) as { telemetry: { enabled: boolean } }; - const doctorOn = JSON.parse(await runAsync(["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], env)) as { + const statusOn = JSON.parse(await runAsync(["status", "--json"], env)) as { + telemetry: { enabled: boolean }; + }; + const configOn = JSON.parse(await runAsync(["config", "--json"], env)) as { + telemetry: { enabled: boolean }; + }; + const doctorOn = JSON.parse( + await runAsync( + ["doctor", "--cwd", tempDir, "--repo", "JSONbored/loopover", "--json"], + env, + ), + ) as { telemetry: { enabled: boolean }; checks: Array<{ name: string; status: string; detail: string }>; }; @@ -96,7 +239,13 @@ describe("loopover-mcp CLI — telemetry opt-in", () => { expect(configOn.telemetry.enabled).toBe(true); expect(doctorOn.telemetry.enabled).toBe(true); expect(doctorOn.checks).toEqual( - expect.arrayContaining([expect.objectContaining({ name: "telemetry", status: "pass", detail: expect.stringContaining("enabled (opt-in)") })]), + expect.arrayContaining([ + expect.objectContaining({ + name: "telemetry", + status: "pass", + detail: expect.stringContaining("enabled (opt-in)"), + }), + ]), ); // Human-readable status and config lines mention the state too. @@ -116,43 +265,74 @@ describe("loopover-mcp CLI — telemetry opt-in", () => { LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", }; - await runAsync(["login", "--profile", "jsonbored", "--github-token", "github-jsonbored", "--json"], env); - const enabled = JSON.parse(await runAsync(["telemetry", "enable", "--json"], env)) as { telemetry: { enabled: boolean } }; + await runAsync( + [ + "login", + "--profile", + "jsonbored", + "--github-token", + "github-jsonbored", + "--json", + ], + env, + ); + const enabled = JSON.parse( + await runAsync(["telemetry", "enable", "--json"], env), + ) as { telemetry: { enabled: boolean } }; expect(enabled.telemetry.enabled).toBe(true); // The opt-in persists alongside the existing session, without clobbering it. - const savedAfterEnable = JSON.parse(readFileSync(configPath, "utf8")) as { telemetryEnabled?: boolean; profiles?: Record }; + const savedAfterEnable = JSON.parse(readFileSync(configPath, "utf8")) as { + telemetryEnabled?: boolean; + profiles?: Record; + }; expect(savedAfterEnable.telemetryEnabled).toBe(true); - expect(savedAfterEnable.profiles?.jsonbored?.session?.login).toBe("JSONbored"); + expect(savedAfterEnable.profiles?.jsonbored?.session?.login).toBe( + "JSONbored", + ); // Disabling telemetry clears only the flag; the authenticated profile survives, so the file stays. await runAsync(["telemetry", "disable", "--json"], env); - const savedAfterDisable = JSON.parse(readFileSync(configPath, "utf8")) as { telemetryEnabled?: boolean; profiles?: Record }; + const savedAfterDisable = JSON.parse(readFileSync(configPath, "utf8")) as { + telemetryEnabled?: boolean; + profiles?: Record; + }; expect(savedAfterDisable.telemetryEnabled).toBeUndefined(); - expect(savedAfterDisable.profiles?.jsonbored?.session?.login).toBe("JSONbored"); + expect(savedAfterDisable.profiles?.jsonbored?.session?.login).toBe( + "JSONbored", + ); // Telemetry output never leaks the persisted session token or the temp path. - expect(JSON.stringify({ enabled })).not.toMatch(/session-jsonbored|github-jsonbored|loopover-cli-/); + expect(JSON.stringify({ enabled })).not.toMatch( + /session-jsonbored|github-jsonbored|loopover-cli-/, + ); }); - it("treats a malformed persisted telemetryEnabled value as the privacy-preserving default", () => { - tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const configPath = join(tempDir, "config.json"); - // A non-boolean (legacy/hand-edited) value must not be read as an opt-in. - writeFileSync(configPath, JSON.stringify({ telemetryEnabled: "true", profiles: {} }), { mode: 0o600 }); - const env = { LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" }; - - const status = JSON.parse(run(["telemetry", "status", "--json"], env)) as { telemetry: { enabled: boolean } }; + it("treats a malformed persisted telemetryEnabled value as the privacy-preserving default", async () => { + // In-process (#8587): the malformed config was written into the in-process config dir BEFORE the bin + // module was imported (see beforeAll) — the same order a spawned CLI sees the file at startup, so this + // still exercises the load-time normalizeConfig dropping the non-boolean value. + const status = JSON.parse( + await captureStdout(() => mod.runCli(["telemetry", "status", "--json"])), + ) as { telemetry: { enabled: boolean } }; expect(status.telemetry.enabled).toBe(false); }); it("rejects an unknown telemetry subcommand in both plain and --json modes", () => { tempDir = mkdtempSync(join(tmpdir(), "loopover-cli-")); - const env = { LOOPOVER_CONFIG_DIR: tempDir, LOOPOVER_SKIP_NPM_VERSION_CHECK: "true" }; + const env = { + LOOPOVER_CONFIG_DIR: tempDir, + LOOPOVER_SKIP_NPM_VERSION_CHECK: "true", + }; - expect(() => run(["telemetry", "bogus"], env)).toThrow(/Unknown telemetry command: bogus\. Use enable \| disable \| status\./); + expect(() => run(["telemetry", "bogus"], env)).toThrow( + /Unknown telemetry command: bogus\. Use enable \| disable \| status\./, + ); const failure = runExpectingFailure(["telemetry", "bogus", "--json"], env); expect(failure.status).not.toBe(0); - expect(JSON.parse(failure.stdout)).toMatchObject({ ok: false, error: expect.stringMatching(/Unknown telemetry command: bogus/) }); + expect(JSON.parse(failure.stdout)).toMatchObject({ + ok: false, + error: expect.stringMatching(/Unknown telemetry command: bogus/), + }); }); });