Skip to content

Commit dc66d5b

Browse files
fix(cli,mcp): harden MCP probe and call params
1 parent b9858b7 commit dc66d5b

9 files changed

Lines changed: 563 additions & 166 deletions

File tree

packages/cli/src/commands/mcp.mjs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,8 @@ function stdioSpecFromServer(target, server) {
355355

356356
function listMcpTools(spec, timeoutMs) {
357357
return new Promise((resolve, reject) => {
358-
const child = spawn(spec.command, spec.args || [], mcpSpawnOptions(spec.env));
358+
const childSpec = mcpSpawnCommand(spec);
359+
const child = spawn(childSpec.command, childSpec.args, mcpSpawnOptions(spec.env));
359360
let stderr = "";
360361
let settled = false;
361362
let timer;
@@ -447,10 +448,33 @@ export function mcpSpawnOptions(env = {}, os = platform()) {
447448
return {
448449
env: { ...process.env, ...(env || {}) },
449450
stdio: ["pipe", "pipe", "pipe"],
450-
shell: os === "win32",
451+
shell: false,
451452
};
452453
}
453454

455+
export function mcpSpawnCommand(spec, os = platform()) {
456+
const args = spec.args || [];
457+
if (os !== "win32" || isWindowsExecutable(spec.command)) {
458+
return { command: spec.command, args };
459+
}
460+
461+
return {
462+
command: process.env.ComSpec || "cmd.exe",
463+
args: ["/d", "/s", "/c", wrapWindowsCommandLine([spec.command, ...args])],
464+
};
465+
}
466+
467+
function isWindowsExecutable(command) {
468+
return /\.(?:exe|com)$/i.test(command);
469+
}
470+
471+
function wrapWindowsCommandLine(parts) {
472+
const commandLine = parts
473+
.map((part) => shellQuoteForPlatform(part, "win32"))
474+
.join(" ");
475+
return `"${commandLine}"`;
476+
}
477+
454478
function extractServer(target, config) {
455479
if (target === "cursor" || target === "claude-desktop") return config?.mcpServers?.qveris;
456480
if (target === "opencode") return config?.mcp?.qveris;

packages/cli/test/mcp.test.mjs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import test from "node:test";
88

99
import {
1010
extractEnvAssignmentValue,
11+
mcpSpawnCommand,
1112
mcpSpawnOptions,
1213
resolveProbeTimeoutMs,
1314
shellQuoteForPlatform,
@@ -58,16 +59,47 @@ test("mcp configure enforces owner-only permissions on existing config files", (
5859
}
5960
});
6061

61-
test("mcp live probe enables shell execution on Windows only", () => {
62+
test("mcp live probe disables shell execution for spawn options", () => {
6263
const windowsOptions = mcpSpawnOptions({ QVERIS_API_KEY: "sk-test" }, "win32");
63-
assert.equal(windowsOptions.shell, true);
64+
assert.equal(windowsOptions.shell, false);
6465
assert.equal(windowsOptions.env.QVERIS_API_KEY, "sk-test");
6566
assert.deepEqual(windowsOptions.stdio, ["pipe", "pipe", "pipe"]);
6667

6768
const posixOptions = mcpSpawnOptions({}, "darwin");
6869
assert.equal(posixOptions.shell, false);
6970
});
7071

72+
test("mcp live probe runs Windows executables directly", () => {
73+
const spec = mcpSpawnCommand(
74+
{
75+
command: "C:\\Program Files\\nodejs\\node.exe",
76+
args: ["fake server.mjs", "--package=@qverisai/mcp"],
77+
},
78+
"win32",
79+
);
80+
81+
assert.equal(spec.command, "C:\\Program Files\\nodejs\\node.exe");
82+
assert.deepEqual(spec.args, ["fake server.mjs", "--package=@qverisai/mcp"]);
83+
});
84+
85+
test("mcp live probe wraps Windows command shims through cmd.exe without shell option", () => {
86+
const spec = mcpSpawnCommand(
87+
{
88+
command: "npx",
89+
args: ["fake server.mjs", "--package=@qverisai/mcp"],
90+
},
91+
"win32",
92+
);
93+
94+
assert.equal(spec.command.endsWith("cmd.exe"), true);
95+
assert.deepEqual(spec.args, [
96+
"/d",
97+
"/s",
98+
"/c",
99+
"\"\"npx\" \"fake server.mjs\" \"--package=@qverisai/mcp\"\"",
100+
]);
101+
});
102+
71103
test("mcp command quoting follows the target shell platform", () => {
72104
assert.equal(shellQuoteForPlatform("L'Ondon", "darwin"), `'L'\\''Ondon'`);
73105
assert.equal(shellQuoteForPlatform('say "hi"', "win32"), `"say ""hi"""`);

packages/mcp/src/api/client.test.ts

Lines changed: 92 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -99,19 +99,31 @@ describe('QverisClient', () => {
9999
});
100100

101101
it('should throw ApiError on non-OK response', async () => {
102-
fetchMock.mockResolvedValueOnce({
103-
ok: false,
104-
status: 401,
105-
statusText: 'Unauthorized',
106-
json: async () => ({ message: 'Invalid API key' }),
107-
});
108-
109-
await expect(client.searchTools({ query: 'test' })).rejects.toEqual({
110-
status: 401,
111-
message: 'Invalid API key',
112-
details: { message: 'Invalid API key' },
113-
});
114-
});
102+
fetchMock.mockResolvedValueOnce({
103+
ok: false,
104+
status: 401,
105+
statusText: 'Unauthorized',
106+
headers: new Headers({ 'x-request-id': 'req-401' }),
107+
json: async () => ({ message: 'Invalid API key' }),
108+
});
109+
110+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
111+
status: 401,
112+
message: 'Invalid API key',
113+
details: { message: 'Invalid API key' },
114+
observability: {
115+
source: 'qveris_api',
116+
operation: 'discover',
117+
method: 'POST',
118+
endpoint: '/search',
119+
url: 'https://qveris.ai/api/v1/search',
120+
timeout_ms: 30000,
121+
http_status: 401,
122+
request_id: 'req-401',
123+
error_type: 'http_error',
124+
},
125+
});
126+
});
115127

116128
it('should handle non-JSON error response', async () => {
117129
fetchMock.mockResolvedValueOnce({
@@ -123,12 +135,39 @@ describe('QverisClient', () => {
123135
},
124136
});
125137

126-
await expect(client.searchTools({ query: 'test' })).rejects.toEqual({
127-
status: 500,
128-
message: 'Internal Server Error',
129-
details: undefined,
130-
});
131-
});
138+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
139+
status: 500,
140+
message: 'Internal Server Error',
141+
observability: {
142+
operation: 'discover',
143+
endpoint: '/search',
144+
http_status: 500,
145+
error_type: 'http_error',
146+
},
147+
});
148+
});
149+
150+
it('should convert fetch network failures to observable ApiError', async () => {
151+
fetchMock.mockRejectedValueOnce(
152+
new Error('fetch failed', { cause: new Error('ECONNRESET') })
153+
);
154+
155+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
156+
status: 0,
157+
message: 'fetch failed',
158+
cause: 'ECONNRESET',
159+
observability: {
160+
source: 'qveris_api',
161+
operation: 'discover',
162+
method: 'POST',
163+
endpoint: '/search',
164+
url: 'https://qveris.ai/api/v1/search',
165+
timeout_ms: 30000,
166+
http_status: 0,
167+
error_type: 'network_error',
168+
},
169+
});
170+
});
132171
});
133172

134173
describe('getToolsByIds', () => {
@@ -235,21 +274,27 @@ describe('QverisClient', () => {
235274
});
236275

237276
it('should throw ApiError on non-OK response', async () => {
238-
fetchMock.mockResolvedValueOnce({
239-
ok: false,
240-
status: 400,
241-
statusText: 'Bad Request',
242-
json: async () => ({ message: 'Invalid tool_ids' }),
243-
});
244-
245-
await expect(
246-
client.getToolsByIds({ tool_ids: [] })
247-
).rejects.toEqual({
248-
status: 400,
249-
message: 'Invalid tool_ids',
250-
details: { message: 'Invalid tool_ids' },
251-
});
252-
});
277+
fetchMock.mockResolvedValueOnce({
278+
ok: false,
279+
status: 400,
280+
statusText: 'Bad Request',
281+
json: async () => ({ message: 'Invalid tool_ids' }),
282+
});
283+
284+
await expect(
285+
client.getToolsByIds({ tool_ids: [] })
286+
).rejects.toMatchObject({
287+
status: 400,
288+
message: 'Invalid tool_ids',
289+
details: { message: 'Invalid tool_ids' },
290+
observability: {
291+
operation: 'inspect',
292+
endpoint: '/tools/by-ids',
293+
http_status: 400,
294+
error_type: 'http_error',
295+
},
296+
});
297+
});
253298

254299
it('should handle non-JSON error response', async () => {
255300
fetchMock.mockResolvedValueOnce({
@@ -261,14 +306,19 @@ describe('QverisClient', () => {
261306
},
262307
});
263308

264-
await expect(
265-
client.getToolsByIds({ tool_ids: ['tool-1'] })
266-
).rejects.toEqual({
267-
status: 500,
268-
message: 'Internal Server Error',
269-
details: undefined,
270-
});
271-
});
309+
await expect(
310+
client.getToolsByIds({ tool_ids: ['tool-1'] })
311+
).rejects.toMatchObject({
312+
status: 500,
313+
message: 'Internal Server Error',
314+
observability: {
315+
operation: 'inspect',
316+
endpoint: '/tools/by-ids',
317+
http_status: 500,
318+
error_type: 'http_error',
319+
},
320+
});
321+
});
272322
});
273323

274324
describe('executeTool', () => {

0 commit comments

Comments
 (0)