Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions packages/cli/src/commands/mcp.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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])],
};
}
Comment thread
sheep-cloud12138 marked this conversation as resolved.

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;
Expand Down
36 changes: 34 additions & 2 deletions packages/cli/test/mcp.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import test from "node:test";

import {
extractEnvAssignmentValue,
mcpSpawnCommand,
mcpSpawnOptions,
resolveProbeTimeoutMs,
shellQuoteForPlatform,
Expand Down Expand Up @@ -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"""`);
Expand Down
12 changes: 12 additions & 0 deletions packages/mcp/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/mcp/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
117 changes: 53 additions & 64 deletions packages/mcp/src/tools/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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', {
Expand All @@ -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', {
Expand All @@ -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',
Expand All @@ -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', {
Expand All @@ -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);
});
});
Expand Down
74 changes: 32 additions & 42 deletions packages/mcp/src/tools/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}

Loading