mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(cli): report failed MCP probes to automation (#116664)
* fix(cli): fail MCP probes without losing output * test(cli): use shared MCP probe temp cleanup
This commit is contained in:
committed by
GitHub
parent
c892a712e6
commit
31ccf56a81
+1
-1
@@ -602,7 +602,7 @@ Use `--json` for scripts and dashboards. Field sets can grow over time, so consu
|
||||
}
|
||||
```
|
||||
|
||||
`probe --json` opens a live MCP client session and prints its result directly; unlike `status`/`doctor`, the output has no top-level `path` field. `resources` and `prompts` keys are present only when the server actually advertises that capability (a server without prompts omits the `prompts` key rather than reporting `false`). Use `probe` for reachability and capability proof, not for static config audits.
|
||||
`probe --json` opens a live MCP client session and prints its result directly; unlike `status`/`doctor`, the output has no top-level `path` field. `resources` and `prompts` keys are present only when the server actually advertises that capability (a server without prompts omits the `prompts` key rather than reporting `false`). The command prints the complete result before exiting nonzero when diagnostics are present or a selected enabled server did not connect, so automation can inspect partial successes. Use `probe` for reachability and capability proof, not for static config audits.
|
||||
|
||||
</Accordion>
|
||||
</AccordionGroup>
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
async function createTempHome(): Promise<string> {
|
||||
return tempDirs.make("openclaw-mcp-probe-process-");
|
||||
}
|
||||
|
||||
async function writeConfig(home: string, servers: Record<string, unknown>): Promise<string> {
|
||||
const configPath = path.join(home, "openclaw.json");
|
||||
await fs.writeFile(configPath, `${JSON.stringify({ mcp: { servers } })}\n`, "utf8");
|
||||
return configPath;
|
||||
}
|
||||
|
||||
async function writeProbeServer(filePath: string): Promise<void> {
|
||||
await fs.writeFile(
|
||||
filePath,
|
||||
`let buffer = "";
|
||||
function send(message) {
|
||||
process.stdout.write(JSON.stringify(message) + "\\n");
|
||||
}
|
||||
function handle(message) {
|
||||
if (message.method === "initialize") {
|
||||
send({
|
||||
jsonrpc: "2.0",
|
||||
id: message.id,
|
||||
result: {
|
||||
protocolVersion: message.params?.protocolVersion ?? "2025-03-26",
|
||||
capabilities: { tools: {} },
|
||||
serverInfo: { name: "probe-process-test", version: "1.0.0" },
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (message.method === "tools/list") {
|
||||
send({
|
||||
jsonrpc: "2.0",
|
||||
id: message.id,
|
||||
result: { tools: [{ name: "ping", inputSchema: { type: "object" } }] },
|
||||
});
|
||||
}
|
||||
}
|
||||
process.stdin.setEncoding("utf8");
|
||||
process.stdin.on("data", (chunk) => {
|
||||
buffer += chunk;
|
||||
while (true) {
|
||||
const newline = buffer.indexOf("\\n");
|
||||
if (newline < 0) return;
|
||||
const line = buffer.slice(0, newline).replace(/\\r$/, "");
|
||||
buffer = buffer.slice(newline + 1);
|
||||
if (line.trim()) handle(JSON.parse(line));
|
||||
}
|
||||
});
|
||||
process.stdin.on("end", () => process.exit(0));
|
||||
process.on("SIGTERM", () => process.exit(0));
|
||||
`,
|
||||
"utf8",
|
||||
);
|
||||
}
|
||||
|
||||
function runProbe(home: string, args: string[]) {
|
||||
const env: NodeJS.ProcessEnv = {
|
||||
...process.env,
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
OPENCLAW_CONFIG_PATH: path.join(home, "openclaw.json"),
|
||||
OPENCLAW_STATE_DIR: path.join(home, "state"),
|
||||
OPENCLAW_TEST_FAST: "1",
|
||||
MCP_TEST_ARGS_JSON: JSON.stringify(args),
|
||||
};
|
||||
delete env.VITEST;
|
||||
delete env.VITEST_POOL_ID;
|
||||
delete env.VITEST_WORKER_ID;
|
||||
const mcpCliUrl = new URL("./mcp-cli.ts", import.meta.url).href;
|
||||
const oneShotExitUrl = new URL("./one-shot-exit.ts", import.meta.url).href;
|
||||
const script = `
|
||||
import { Command } from "commander";
|
||||
import { registerMcpCli } from ${JSON.stringify(mcpCliUrl)};
|
||||
import { runCliWithExitFinalization } from ${JSON.stringify(oneShotExitUrl)};
|
||||
const program = new Command();
|
||||
program.exitOverride();
|
||||
registerMcpCli(program);
|
||||
await runCliWithExitFinalization({
|
||||
run: async () => {
|
||||
await program.parseAsync(JSON.parse(process.env.MCP_TEST_ARGS_JSON), { from: "user" });
|
||||
},
|
||||
onError: (error) => { throw error; },
|
||||
});
|
||||
`;
|
||||
return spawnSync(process.execPath, ["--import", "tsx", "--input-type=module", "--eval", script], {
|
||||
encoding: "utf8",
|
||||
env,
|
||||
maxBuffer: 4 * 1024 * 1024,
|
||||
timeout: 30_000,
|
||||
});
|
||||
}
|
||||
|
||||
describe("mcp probe process exit", () => {
|
||||
it("prints named JSON diagnostics before exiting nonzero", async () => {
|
||||
const home = await createTempHome();
|
||||
const configPath = await writeConfig(home, {
|
||||
broken: { command: path.join(home, "missing-mcp-server") },
|
||||
});
|
||||
|
||||
const result = runProbe(home, ["mcp", "probe", "broken", "--json"]);
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.status).toBe(1);
|
||||
const output = JSON.parse(result.stdout) as {
|
||||
diagnostics: Array<{ message: string; serverName: string }>;
|
||||
servers: Record<string, unknown>;
|
||||
};
|
||||
expect(output.servers).toEqual({});
|
||||
expect(output.diagnostics).toEqual([expect.objectContaining({ serverName: "broken" })]);
|
||||
expect(result.stderr).toContain(`MCP probe failed for "broken" in ${configPath}:`);
|
||||
});
|
||||
|
||||
it("preserves mixed partial text output before exiting nonzero", async () => {
|
||||
const home = await createTempHome();
|
||||
const serverPath = path.join(home, "probe-server.mjs");
|
||||
await writeProbeServer(serverPath);
|
||||
await writeConfig(home, {
|
||||
healthy: { command: process.execPath, args: [serverPath] },
|
||||
broken: { command: path.join(home, "missing-mcp-server") },
|
||||
});
|
||||
|
||||
const result = runProbe(home, ["mcp", "probe"]);
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stdout).toContain("- healthy: 1 tools");
|
||||
expect(result.stdout).toContain("! broken:");
|
||||
});
|
||||
|
||||
it("fails when an enabled server is omitted without a diagnostic", async () => {
|
||||
const home = await createTempHome();
|
||||
await writeConfig(home, { incomplete: {} });
|
||||
|
||||
const result = runProbe(home, ["mcp", "probe", "incomplete", "--json"]);
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.status).toBe(1);
|
||||
expect(JSON.parse(result.stdout)).toMatchObject({ servers: {}, diagnostics: [] });
|
||||
expect(result.stderr).toContain('MCP probe did not connect to "incomplete"');
|
||||
});
|
||||
|
||||
it("keeps healthy output successful and ignores disabled entries", async () => {
|
||||
const home = await createTempHome();
|
||||
const serverPath = path.join(home, "probe-server.mjs");
|
||||
await writeProbeServer(serverPath);
|
||||
await writeConfig(home, {
|
||||
healthy: { command: process.execPath, args: [serverPath] },
|
||||
disabled: { enabled: false },
|
||||
});
|
||||
|
||||
const result = runProbe(home, ["mcp", "probe", "--json"]);
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.status).toBe(0);
|
||||
expect(JSON.parse(result.stdout)).toMatchObject({
|
||||
diagnostics: [],
|
||||
servers: { healthy: { tools: 1 } },
|
||||
});
|
||||
});
|
||||
});
|
||||
+42
-18
@@ -39,6 +39,7 @@ import { defaultRuntime } from "../runtime.js";
|
||||
import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js";
|
||||
import { formatCliCommand } from "./command-format.js";
|
||||
import { resolveGatewayAuthOptions } from "./gateway-secret-options.js";
|
||||
import { requestExitAfterOneShotOutput } from "./one-shot-exit.js";
|
||||
import { applyParentDefaultHelpAction } from "./program/parent-default-help.js";
|
||||
|
||||
function fail(message: string): never {
|
||||
@@ -558,6 +559,30 @@ function applyMcpProbeInitializeTimeout(server: Record<string, unknown>): Record
|
||||
};
|
||||
}
|
||||
|
||||
function resolveMcpProbeIssue(params: {
|
||||
result: ReturnType<typeof formatMcpProbeResult>;
|
||||
servers: Record<string, Record<string, unknown>>;
|
||||
path: string;
|
||||
}): string | undefined {
|
||||
if (params.result.diagnostics.length > 0) {
|
||||
const first = expectDefined(params.result.diagnostics[0], "diagnostics entry at 0");
|
||||
return `MCP probe failed for "${first.serverName}" in ${params.path}: ${first.message}`;
|
||||
}
|
||||
for (const [name, server] of Object.entries(params.servers)) {
|
||||
if (server.enabled !== false && !params.result.servers[name]) {
|
||||
return `MCP probe did not connect to "${name}" in ${params.path}.`;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function failOnMcpProbeIssues(params: Parameters<typeof resolveMcpProbeIssue>[0]): void {
|
||||
const probeIssue = resolveMcpProbeIssue(params);
|
||||
if (probeIssue) {
|
||||
fail(probeIssue);
|
||||
}
|
||||
}
|
||||
|
||||
async function probeMcpServersOrFail(params: {
|
||||
config: OpenClawConfig;
|
||||
servers: Record<string, Record<string, unknown>>;
|
||||
@@ -577,15 +602,7 @@ async function probeMcpServersOrFail(params: {
|
||||
});
|
||||
try {
|
||||
const result = formatMcpProbeResult(await runtime.getCatalog());
|
||||
if (result.diagnostics.length > 0) {
|
||||
const first = expectDefined(result.diagnostics[0], "diagnostics entry at 0");
|
||||
fail(`MCP probe failed for "${first.serverName}" in ${params.path}: ${first.message}`);
|
||||
}
|
||||
for (const name of Object.keys(params.servers)) {
|
||||
if (!result.servers[name]) {
|
||||
fail(`MCP probe did not connect to "${name}" in ${params.path}.`);
|
||||
}
|
||||
}
|
||||
failOnMcpProbeIssues({ result, servers: params.servers, path: params.path });
|
||||
return result;
|
||||
} finally {
|
||||
await runtime.dispose();
|
||||
@@ -782,16 +799,23 @@ export function registerMcpCli(program: Command) {
|
||||
const result = formatMcpProbeResult(await runtime.getCatalog());
|
||||
if (opts.json) {
|
||||
printJson(result);
|
||||
return;
|
||||
} else {
|
||||
defaultRuntime.log(`MCP probe (${loaded.path}):`);
|
||||
for (const [serverName, server] of Object.entries(result.servers)) {
|
||||
defaultRuntime.log(
|
||||
`- ${serverName}: ${server.tools} tools${server.resources ? ", resources" : ""}${server.prompts ? ", prompts" : ""}`,
|
||||
);
|
||||
}
|
||||
for (const diagnostic of result.diagnostics) {
|
||||
defaultRuntime.log(`! ${diagnostic.serverName}: ${diagnostic.message}`);
|
||||
}
|
||||
}
|
||||
defaultRuntime.log(`MCP probe (${loaded.path}):`);
|
||||
for (const [serverName, server] of Object.entries(result.servers)) {
|
||||
defaultRuntime.log(
|
||||
`- ${serverName}: ${server.tools} tools${server.resources ? ", resources" : ""}${server.prompts ? ", prompts" : ""}`,
|
||||
);
|
||||
}
|
||||
for (const diagnostic of result.diagnostics) {
|
||||
defaultRuntime.log(`! ${diagnostic.serverName}: ${diagnostic.message}`);
|
||||
const probeIssue = resolveMcpProbeIssue({ result, servers, path: loaded.path });
|
||||
if (probeIssue) {
|
||||
defaultRuntime.error(probeIssue);
|
||||
if (!requestExitAfterOneShotOutput(defaultRuntime, 1)) {
|
||||
defaultRuntime.exit(1);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
await runtime.dispose();
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { requestExitAfterOneShotOutput, runCliWithExitFinalization } from "./one-shot-exit.js";
|
||||
@@ -257,4 +258,42 @@ describe("one-shot CLI exit", () => {
|
||||
});
|
||||
expect(exit).toHaveBeenCalledWith(0);
|
||||
});
|
||||
|
||||
it("drains large piped stdout before a requested nonzero exit", () => {
|
||||
const env = { ...process.env };
|
||||
delete env.VITEST;
|
||||
delete env.VITEST_POOL_ID;
|
||||
delete env.VITEST_WORKER_ID;
|
||||
const oneShotExitUrl = new URL("./one-shot-exit.ts", import.meta.url).href;
|
||||
const runtimeUrl = new URL("../runtime.ts", import.meta.url).href;
|
||||
const payloadBytes = 1024 * 1024;
|
||||
const script = `
|
||||
import { requestExitAfterOneShotOutput, runCliWithExitFinalization } from ${JSON.stringify(oneShotExitUrl)};
|
||||
import { defaultRuntime } from ${JSON.stringify(runtimeUrl)};
|
||||
await runCliWithExitFinalization({
|
||||
run: async () => {
|
||||
process.stdout.write("x".repeat(${payloadBytes}));
|
||||
requestExitAfterOneShotOutput(defaultRuntime, 7);
|
||||
},
|
||||
onError: (error) => { throw error; },
|
||||
});
|
||||
`;
|
||||
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
["--import", "tsx", "--input-type=module", "--eval", script],
|
||||
{
|
||||
encoding: "utf8",
|
||||
env,
|
||||
maxBuffer: 2 * payloadBytes,
|
||||
timeout: 30_000,
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(result.status).toBe(7);
|
||||
expect(result.signal).toBeNull();
|
||||
expect(result.stderr).toBe("");
|
||||
expect(result.stdout).toHaveLength(payloadBytes);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user