Skip to content

Commit 8fef582

Browse files
sheep-cloud12138sheep-cloud12138
committed
fix(cli,mcp): harden MCP probe and observability
1 parent 8256aa4 commit 8fef582

9 files changed

Lines changed: 485 additions & 56 deletions

File tree

‎packages/cli/src/commands/mcp.mjs‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ export function resolveProbeTimeoutMs(timeout) {
444444
return Math.max(1000, usableSeconds * 1000);
445445
}
446446

447-
export function mcpSpawnOptions(env = {}, os = platform()) {
447+
export function mcpSpawnOptions(env = {}, _os = platform()) {
448448
return {
449449
env: { ...process.env, ...(env || {}) },
450450
stdio: ["pipe", "pipe", "pipe"],
@@ -470,11 +470,37 @@ function isWindowsExecutable(command) {
470470

471471
function wrapWindowsCommandLine(parts) {
472472
const commandLine = parts
473-
.map((part) => shellQuoteForPlatform(part, "win32"))
473+
.map((part) => quoteWindowsCommandArgument(part))
474474
.join(" ");
475475
return `"${commandLine}"`;
476476
}
477477

478+
function quoteWindowsCommandArgument(value) {
479+
const text = String(value).replaceAll("%", "%%");
480+
let quoted = "\"";
481+
let backslashes = 0;
482+
483+
for (const char of text) {
484+
if (char === "\\") {
485+
backslashes += 1;
486+
continue;
487+
}
488+
if (char === "\"") {
489+
quoted += "\\".repeat(backslashes * 2 + 1);
490+
quoted += char;
491+
backslashes = 0;
492+
continue;
493+
}
494+
quoted += "\\".repeat(backslashes);
495+
backslashes = 0;
496+
quoted += char;
497+
}
498+
499+
quoted += "\\".repeat(backslashes * 2);
500+
quoted += "\"";
501+
return quoted;
502+
}
503+
478504
function extractServer(target, config) {
479505
if (target === "cursor" || target === "claude-desktop") return config?.mcpServers?.qveris;
480506
if (target === "opencode") return config?.mcp?.qveris;

‎packages/cli/test/mcp.test.mjs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,29 @@ test("mcp live probe wraps Windows command shims through cmd.exe without shell o
100100
]);
101101
});
102102

103+
test("mcp live probe safely quotes Windows command shim arguments", () => {
104+
const spec = mcpSpawnCommand(
105+
{
106+
command: "C:\\Tools\\npm shim\\npx.cmd",
107+
args: [
108+
"--name=foo\"bar",
109+
"C:\\temp\\tail\\",
110+
"100% ready",
111+
"safe&literal",
112+
],
113+
},
114+
"win32",
115+
);
116+
117+
assert.equal(spec.command.endsWith("cmd.exe"), true);
118+
assert.deepEqual(spec.args, [
119+
"/d",
120+
"/s",
121+
"/c",
122+
"\"\"C:\\Tools\\npm shim\\npx.cmd\" \"--name=foo\\\"bar\" \"C:\\temp\\tail\\\\\" \"100%% ready\" \"safe&literal\"\"",
123+
]);
124+
});
125+
103126
test("mcp command quoting follows the target shell platform", () => {
104127
assert.equal(shellQuoteForPlatform("L'Ondon", "darwin"), `'L'\\''Ondon'`);
105128
assert.equal(shellQuoteForPlatform('say "hi"', "win32"), `"say ""hi"""`);

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

Lines changed: 85 additions & 35 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', () => {
@@ -242,14 +281,20 @@ describe('QverisClient', () => {
242281
json: async () => ({ message: 'Invalid tool_ids' }),
243282
});
244283

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-
});
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)