fix(cli,mcp): harden MCP probe and call params

Author: sheep-cloud12138

packages/cli/src/commands/mcp.mjs

@@ -355,7 +355,8 @@ function stdioSpecFromServer(target, server) {
 
 function listMcpTools(spec, timeoutMs) {
   return new Promise((resolve, reject) => {
-    const child = spawn(spec.command, spec.args || [], mcpSpawnOptions(spec.env));
+    const childSpec = mcpSpawnCommand(spec);
+    const child = spawn(childSpec.command, childSpec.args, mcpSpawnOptions(spec.env));
     let stderr = "";
     let settled = false;
     let timer;
@@ -447,10 +448,33 @@ export function mcpSpawnOptions(env = {}, os = platform()) {
   return {
     env: { ...process.env, ...(env || {}) },
     stdio: ["pipe", "pipe", "pipe"],
-    shell: os === "win32",
+    shell: false,
   };
 }
 
+export function mcpSpawnCommand(spec, os = platform()) {
+  const args = spec.args || [];
+  if (os !== "win32" || isWindowsExecutable(spec.command)) {
+    return { command: spec.command, args };
+  }
+
+  return {
+    command: process.env.ComSpec || "cmd.exe",
+    args: ["/d", "/s", "/c", wrapWindowsCommandLine([spec.command, ...args])],
+  };
+}
+
+function isWindowsExecutable(command) {
+  return /\.(?:exe|com)$/i.test(command);
+}
+
+function wrapWindowsCommandLine(parts) {
+  const commandLine = parts
+    .map((part) => shellQuoteForPlatform(part, "win32"))
+    .join(" ");
+  return `"${commandLine}"`;
+}
+
 function extractServer(target, config) {
   if (target === "cursor" || target === "claude-desktop") return config?.mcpServers?.qveris;
   if (target === "opencode") return config?.mcp?.qveris;

packages/cli/test/mcp.test.mjs

@@ -8,6 +8,7 @@ import test from "node:test";
 
 import {
   extractEnvAssignmentValue,
+  mcpSpawnCommand,
   mcpSpawnOptions,
   resolveProbeTimeoutMs,
   shellQuoteForPlatform,
@@ -58,16 +59,47 @@ test("mcp configure enforces owner-only permissions on existing config files", (
   }
 });
 
-test("mcp live probe enables shell execution on Windows only", () => {
+test("mcp live probe disables shell execution for spawn options", () => {
   const windowsOptions = mcpSpawnOptions({ QVERIS_API_KEY: "sk-test" }, "win32");
-  assert.equal(windowsOptions.shell, true);
+  assert.equal(windowsOptions.shell, false);
   assert.equal(windowsOptions.env.QVERIS_API_KEY, "sk-test");
   assert.deepEqual(windowsOptions.stdio, ["pipe", "pipe", "pipe"]);
 
   const posixOptions = mcpSpawnOptions({}, "darwin");
   assert.equal(posixOptions.shell, false);
 });
 
+test("mcp live probe runs Windows executables directly", () => {
+  const spec = mcpSpawnCommand(
+    {
+      command: "C:\\Program Files\\nodejs\\node.exe",
+      args: ["fake server.mjs", "--package=@qverisai/mcp"],
+    },
+    "win32",
+  );
+
+  assert.equal(spec.command, "C:\\Program Files\\nodejs\\node.exe");
+  assert.deepEqual(spec.args, ["fake server.mjs", "--package=@qverisai/mcp"]);
+});
+
+test("mcp live probe wraps Windows command shims through cmd.exe without shell option", () => {
+  const spec = mcpSpawnCommand(
+    {
+      command: "npx",
+      args: ["fake server.mjs", "--package=@qverisai/mcp"],
+    },
+    "win32",
+  );
+
+  assert.equal(spec.command.endsWith("cmd.exe"), true);
+  assert.deepEqual(spec.args, [
+    "/d",
+    "/s",
+    "/c",
+    "\"\"npx\" \"fake server.mjs\" \"--package=@qverisai/mcp\"\"",
+  ]);
+});
+
 test("mcp command quoting follows the target shell platform", () => {
   assert.equal(shellQuoteForPlatform("L'Ondon", "darwin"), `'L'\\''Ondon'`);
   assert.equal(shellQuoteForPlatform('say "hi"', "win32"), `"say ""hi"""`);

packages/mcp/src/index.test.ts

@@ -119,6 +119,16 @@ describe('MCP public tool interface', () => {
       'call',
       { tool_id: 'weather.tool.v1' },
     );
+    const invalidCallParams = await executeQverisMcpTool(
+      client,
+      'session-1',
+      'call',
+      {
+        tool_id: 'weather.tool.v1',
+        search_id: 'search-1',
+        params_to_tool: '{"city":"London"}',
+      },
+    );
     const unknown = await executeQverisMcpTool(client, 'session-1', 'not_a_tool', {});
     const apiError = await executeQverisMcpTool(
       client,
@@ -131,6 +141,8 @@ describe('MCP public tool interface', () => {
     expect(payload(missingQuery).error).toContain('query');
     expect(missingCallParams.isError).toBe(true);
     expect(payload(missingCallParams).error).toContain('search_id');
+    expect(invalidCallParams.isError).toBe(true);
+    expect(payload(invalidCallParams).error).toContain('JSON object');
     expect(unknown.isError).toBe(true);
     expect(payload(unknown).available_tools).toEqual([
       'discover',

packages/mcp/src/index.ts

@@ -232,7 +232,7 @@ export async function executeQverisMcpTool(
       const missingFields: string[] = [];
       if (!input.tool_id) missingFields.push('tool_id');
       if (!input.search_id) missingFields.push('search_id');
-      if (!input.params_to_tool) missingFields.push('params_to_tool');
+      if (input.params_to_tool === undefined) missingFields.push('params_to_tool');
 
       if (missingFields.length > 0) {
         return {

packages/mcp/src/tools/execute.test.ts

@@ -53,7 +53,7 @@ describe('call (execute_tool)', () => {
       vi.restoreAllMocks();
     });
 
-    it('should parse JSON params and call client.executeTool', async () => {
+    it('should pass object params and call client.executeTool', async () => {
       const mockResponse: ExecuteResponse = {
         execution_id: 'exec-123',
         tool_id: 'weather-tool',
@@ -67,13 +67,13 @@ describe('call (execute_tool)', () => {
 
       const result = await executeExecuteTool(
         mockClient,
-        {
-          tool_id: 'weather-tool',
-          search_id: 'search-123',
-          params_to_tool: '{"city": "Tokyo", "units": "metric"}',
-        },
-        'default-session'
-      );
+        {
+          tool_id: 'weather-tool',
+          search_id: 'search-123',
+          params_to_tool: { city: 'Tokyo', units: 'metric' },
+        },
+        'default-session'
+      );
 
       expect(executeToolMock).toHaveBeenCalledWith('weather-tool', {
         search_id: 'search-123',
@@ -96,13 +96,13 @@ describe('call (execute_tool)', () => {
 
       await executeExecuteTool(
         mockClient,
-        {
-          tool_id: 'tool-1',
-          search_id: 'search-123',
-          params_to_tool: '{}',
-          session_id: 'custom-session',
-        },
-        'default-session'
+        {
+          tool_id: 'tool-1',
+          search_id: 'search-123',
+          params_to_tool: {},
+          session_id: 'custom-session',
+        },
+        'default-session'
       );
 
       expect(executeToolMock).toHaveBeenCalledWith('tool-1', {
@@ -124,13 +124,13 @@ describe('call (execute_tool)', () => {
 
       await executeExecuteTool(
         mockClient,
-        {
-          tool_id: 'tool-1',
-          search_id: 'search-123',
-          params_to_tool: '{}',
-          max_response_size: 102400,
-        },
-        'default-session'
+        {
+          tool_id: 'tool-1',
+          search_id: 'search-123',
+          params_to_tool: {},
+          max_response_size: 102400,
+        },
+        'default-session'
       );
 
       expect(executeToolMock).toHaveBeenCalledWith('tool-1', {
@@ -141,35 +141,24 @@ describe('call (execute_tool)', () => {
       });
     });
 
-    it('should throw error for invalid JSON in params_to_tool', async () => {
-      await expect(
-        executeExecuteTool(
-          mockClient,
-          {
-            tool_id: 'tool-1',
-            search_id: 'search-123',
-            params_to_tool: 'not valid json',
-          },
-          'default-session'
-        )
-      ).rejects.toThrow('Invalid JSON in params_to_tool');
-    });
-
-    it('should include original params in JSON parse error message', async () => {
-      await expect(
-        executeExecuteTool(
-          mockClient,
-          {
-            tool_id: 'tool-1',
-            search_id: 'search-123',
-            params_to_tool: '{broken',
-          },
-          'default-session'
-        )
-      ).rejects.toThrow('Received: {broken');
-    });
-
-    it('should handle complex nested parameters', async () => {
+    it.each(['not valid json', null, ['city']])(
+      'should reject non-object params_to_tool: %s',
+      async (paramsToTool) => {
+        await expect(
+          executeExecuteTool(
+            mockClient,
+            {
+              tool_id: 'tool-1',
+              search_id: 'search-123',
+              params_to_tool: paramsToTool as unknown as Record<string, unknown>,
+            },
+            'default-session'
+          )
+        ).rejects.toThrow('params_to_tool must be a JSON object');
+      }
+    );
+
+    it('should handle complex nested parameters', async () => {
       const complexParams = {
         filters: {
           category: 'electronics',
@@ -189,12 +178,12 @@ describe('call (execute_tool)', () => {
 
       await executeExecuteTool(
         mockClient,
-        {
-          tool_id: 'search-products',
-          search_id: 'search-123',
-          params_to_tool: JSON.stringify(complexParams),
-        },
-        'default-session'
+        {
+          tool_id: 'search-products',
+          search_id: 'search-123',
+          params_to_tool: complexParams,
+        },
+        'default-session'
       );
 
       expect(executeToolMock).toHaveBeenCalledWith('search-products', {
@@ -212,13 +201,13 @@ describe('call (execute_tool)', () => {
       await expect(
         executeExecuteTool(
           mockClient,
-          {
-            tool_id: 'tool-1',
-            search_id: 'search-123',
-            params_to_tool: '{}',
-          },
-          'session'
-        )
+          {
+            tool_id: 'tool-1',
+            search_id: 'search-123',
+            params_to_tool: {},
+          },
+          'session'
+        )
       ).rejects.toEqual(error);
     });
   });

packages/mcp/src/tools/execute.ts

@@ -26,15 +26,14 @@ export interface ExecuteToolInput {
    */
   search_id: string;
 
-  /**
-   * Dictionary of parameters to pass to the remote tool.
-   * Keys are parameter names, values can be of any type.
-   * Can be provided as an object directly or as a JSON string (legacy).
-   *
-   * @example {"city": "London", "units": "metric"}
-   * @example {"query": "AI news", "limit": 10}
-   */
-  params_to_tool: Record<string, unknown> | string;
+  /**
+   * Dictionary of parameters to pass to the remote tool.
+   * Keys are parameter names, values can be of any type.
+   *
+   * @example {"city": "London", "units": "metric"}
+   * @example {"query": "AI news", "limit": 10}
+   */
+  params_to_tool: Record<string, unknown>;
 
   /**
    * Session identifier for tracking user sessions.
@@ -103,37 +102,28 @@ export const executeToolSchema = {
  * @param input - Execution parameters
  * @param defaultSessionId - Fallback session ID if not provided in input
  * @returns Execution result from the tool
- * @throws {Error} If params_to_tool is not valid JSON
- */
-export async function executeExecuteTool(
-  client: QverisClient,
-  input: ExecuteToolInput,
-  defaultSessionId: string
-): Promise<ExecuteResponse> {
-  // Resolve parameters: accept object directly or JSON string (legacy compat)
-  let parameters: Record<string, unknown>;
-  if (typeof input.params_to_tool === 'string') {
-    try {
-      parameters = JSON.parse(input.params_to_tool) as Record<string, unknown>;
-    } catch (error) {
-      throw new Error(
-        `Invalid JSON in params_to_tool: ${error instanceof Error ? error.message : 'Unknown parse error'}. ` +
-        `Received: ${input.params_to_tool}`
-      );
-    }
-  } else if (typeof input.params_to_tool === 'object' && input.params_to_tool !== null) {
-    parameters = input.params_to_tool;
-  } else {
-    parameters = {};
-  }
-
-  const response = await client.executeTool(input.tool_id, {
-    search_id: input.search_id,
-    session_id: input.session_id ?? defaultSessionId,
-    parameters,
-    max_response_size: input.max_response_size,
-  });
-
-  return response;
-}
+ * @throws {Error} If params_to_tool is not a JSON object
+ */
+export async function executeExecuteTool(
+  client: QverisClient,
+  input: ExecuteToolInput,
+  defaultSessionId: string
+): Promise<ExecuteResponse> {
+  if (!isParamsObject(input.params_to_tool)) {
+    throw new Error('params_to_tool must be a JSON object.');
+  }
+
+  const response = await client.executeTool(input.tool_id, {
+    search_id: input.search_id,
+    session_id: input.session_id ?? defaultSessionId,
+    parameters: input.params_to_tool,
+    max_response_size: input.max_response_size,
+  });
+
+  return response;
+}
+
+function isParamsObject(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null && !Array.isArray(value);
+}