Skip to content
16 changes: 11 additions & 5 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1780,7 +1780,12 @@ class QwenAgent implements Agent {
resume?: boolean,
): Promise<Config> {
this.settings = loadSettings(cwd);
const mergedMcpServers = { ...this.settings.merged.mcpServers };
// ACP/IDE-injected servers are session-level: they must outrank a project
// `.mcp.json` and stay un-gated. Collect them separately and pass them as
// `sessionMcpServers` (top precedence tier) rather than merging into
// `settings.mcpServers`, where `assembleMcpServers` would demote them below
// `.mcp.json` (#4615).
const sessionMcpServers: Record<string, MCPServerConfig> = {};

for (const server of mcpServers) {
const stdioServer = toStdioServer(server);
Expand All @@ -1789,7 +1794,7 @@ class QwenAgent implements Agent {
for (const { name: envName, value } of stdioServer.env) {
env[envName] = value;
}
mergedMcpServers[stdioServer.name] = new MCPServerConfig(
sessionMcpServers[stdioServer.name] = new MCPServerConfig(
stdioServer.command,
stdioServer.args,
env,
Expand All @@ -1804,7 +1809,7 @@ class QwenAgent implements Agent {
for (const { name: headerName, value } of sseServer.headers) {
headers[headerName] = value;
}
mergedMcpServers[sseServer.name] = new MCPServerConfig(
sessionMcpServers[sseServer.name] = new MCPServerConfig(
undefined,
undefined,
undefined,
Expand All @@ -1822,7 +1827,7 @@ class QwenAgent implements Agent {
for (const { name: headerName, value } of httpServer.headers) {
headers[headerName] = value;
}
mergedMcpServers[httpServer.name] = new MCPServerConfig(
sessionMcpServers[httpServer.name] = new MCPServerConfig(
undefined,
undefined,
undefined,
Expand All @@ -1835,7 +1840,7 @@ class QwenAgent implements Agent {
}
}

const settings = { ...this.settings.merged, mcpServers: mergedMcpServers };
const settings = this.settings.merged;
const argvForSession = {
...this.argv,
...(resume ? { resume: sessionId } : { sessionId }),
Expand All @@ -1852,6 +1857,7 @@ class QwenAgent implements Agent {
userHooks: this.settings.getUserHooks(),
projectHooks: this.settings.getProjectHooks(),
},
sessionMcpServers,
// CRITICAL: close over `this.settings` (LoadedSettings instance), NOT
// over the local `settings` snapshot built above. `LoadedSettings.
// setValue` replaces `_merged`, so a closure over the snapshot would
Expand Down
18 changes: 14 additions & 4 deletions packages/cli/src/commands/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ describe('mcp command', () => {
expect(typeof mcpCommand.handler).toBe('function');
});

it('should have exactly one option (help flag)', () => {
it('should have exactly one option (help flag)', async () => {
// Test to ensure that the global 'gemini' flags are not added to the mcp command
const yargsInstance = yargs();
const builtYargs = mcpCommand.builder(yargsInstance);
const builder = mcpCommand.builder;
if (typeof builder !== 'function') {
throw new Error('mcp command builder must be a function');
}
const builtYargs = await builder(yargsInstance);
const options = builtYargs.getOptions();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] TypeScript error: Property 'getOptions' does not exist on type 'Argv<{}> | PromiseLike<Argv<{}>>'. The builder function (typed as CommandModule['builder']) can return Argv | PromiseLike<Argv>. The runtime typeof builder !== 'function' guard narrows it to a function, but the return type is still the union.

Suggested change
const options = builtYargs.getOptions();
const builtYargs = await builder(yargsInstance);
const options = builtYargs.getOptions();

(Make the test callback async and await the builder result to unwrap the PromiseLike.)

— qwen3.7-max via Qwen Code /review


// Should have exactly 1 option (help flag)
Expand All @@ -35,9 +39,13 @@ describe('mcp command', () => {
version: vi.fn().mockReturnThis(),
};

mcpCommand.builder(mockYargs as unknown as Argv);
const builder = mcpCommand.builder;
if (typeof builder !== 'function') {
throw new Error('mcp command builder must be a function');
}
builder(mockYargs as unknown as Argv);

expect(mockYargs.command).toHaveBeenCalledTimes(4);
expect(mockYargs.command).toHaveBeenCalledTimes(6);

// Verify that the specific subcommands are registered
const commandCalls = mockYargs.command.mock.calls;
Expand All @@ -47,6 +55,8 @@ describe('mcp command', () => {
expect(commandNames).toContain('remove <name>');
expect(commandNames).toContain('list');
expect(commandNames).toContain('reconnect [server-name]');
expect(commandNames).toContain('approve [name]');
expect(commandNames).toContain('reject [name]');

expect(mockYargs.demandCommand).toHaveBeenCalledWith(
1,
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/commands/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { addCommand } from './mcp/add.js';
import { removeCommand } from './mcp/remove.js';
import { listCommand } from './mcp/list.js';
import { reconnectCommand } from './mcp/reconnect.js';
import { approveCommand, rejectCommand } from './mcp/approve.js';

export const mcpCommand: CommandModule = {
command: 'mcp',
Expand All @@ -20,6 +21,8 @@ export const mcpCommand: CommandModule = {
.command(removeCommand)
.command(listCommand)
.command(reconnectCommand)
.command(approveCommand)
.command(rejectCommand)
.demandCommand(1, 'You need at least one command before continuing.')
.version(false),
handler: () => {
Expand Down
146 changes: 146 additions & 0 deletions packages/cli/src/commands/mcp/approve.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';

const mockWriteStdoutLine = vi.hoisted(() => vi.fn());
vi.mock('../../utils/stdioHelpers.js', () => ({
writeStdoutLine: mockWriteStdoutLine,
writeStderrLine: vi.fn(),
clearScreen: vi.fn(),
}));

import { approveCommand, rejectCommand } from './approve.js';
import {
loadMcpApprovals,
resetMcpApprovalsForTesting,
MCP_APPROVALS_FILENAME,
} from '../../config/mcpApprovals.js';
import { loadProjectMcpServers } from '../../config/mcpJson.js';

describe('qwen mcp approve / reject', () => {
let dir: string;
let cwdSpy: ReturnType<typeof vi.spyOn>;

const output = () =>
mockWriteStdoutLine.mock.calls.map((c) => c[0]).join('\n');

const run = async (
cmd: typeof approveCommand,
argv: Record<string, unknown>,
) => {
await (cmd.handler as (a: Record<string, unknown>) => Promise<void>)({
_: [],
$0: 'qwen',
...argv,
});
};

beforeEach(() => {
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcp-approve-'));
process.env['QWEN_CODE_MCP_APPROVALS_PATH'] = path.join(
dir,
MCP_APPROVALS_FILENAME,
);
resetMcpApprovalsForTesting();
cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(dir);
mockWriteStdoutLine.mockClear();
});

afterEach(() => {
delete process.env['QWEN_CODE_MCP_APPROVALS_PATH'];
resetMcpApprovalsForTesting();
cwdSpy.mockRestore();
fs.rmSync(dir, { recursive: true, force: true });
});

const writeMcpJson = (servers: Record<string, unknown>) =>
fs.writeFileSync(
path.join(dir, '.mcp.json'),
JSON.stringify({ mcpServers: servers }),
);

const stateOf = (name: string) => {
resetMcpApprovalsForTesting();
const { servers } = loadProjectMcpServers(dir);
return loadMcpApprovals().getState(dir, name, servers[name]!);
};

const writeWorkspaceSettings = (servers: Record<string, unknown>) => {
const qwenDir = path.join(dir, '.qwen');
fs.mkdirSync(qwenDir, { recursive: true });
fs.writeFileSync(
path.join(qwenDir, 'settings.json'),
JSON.stringify({ mcpServers: servers }),
);
};

/** Read the persisted approval status straight off disk (scope-agnostic). */
const persistedStatus = (name: string): string | undefined => {
const raw = fs.readFileSync(
process.env['QWEN_CODE_MCP_APPROVALS_PATH']!,
'utf-8',
);
return JSON.parse(raw)[dir]?.[name]?.status;
};

it('reports when there are no gated servers', async () => {
await run(approveCommand, { name: 'slack', all: false });
expect(output()).toContain('No approval-requiring MCP servers found');
});

it('approves a named project server (pending -> approved)', async () => {
writeMcpJson({ slack: { command: 'node', args: ['slack.js'] } });
expect(stateOf('slack')).toBe('pending');

await run(approveCommand, { name: 'slack', all: false });

expect(stateOf('slack')).toBe('approved');
expect(output()).toContain('Approved MCP server "slack"');
});

it('approves a workspace .qwen/settings.json server', async () => {
writeWorkspaceSettings({ ws: { command: 'node', args: ['ws.js'] } });

await run(approveCommand, { name: 'ws', all: false });

expect(output()).toContain('Approved MCP server "ws"');
expect(persistedStatus('ws')).toBe('approved');
});

it('rejects a named project server', async () => {
writeMcpJson({ slack: { command: 'node' } });
await run(rejectCommand, { name: 'slack', all: false });
expect(stateOf('slack')).toBe('rejected');
});

it('approves all with --all', async () => {
writeMcpJson({ a: { command: 'a' }, b: { command: 'b' } });
await run(approveCommand, { name: undefined, all: true });
expect(stateOf('a')).toBe('approved');
expect(stateOf('b')).toBe('approved');
});

it('reports an unknown server name', async () => {
writeMcpJson({ slack: { command: 'node' } });
await run(approveCommand, { name: 'ghost', all: false });
expect(output()).toContain('not found');
expect(stateOf('slack')).toBe('pending');
});

it('binds approval to the config hash: editing .mcp.json reverts to pending', async () => {
writeMcpJson({ slack: { command: 'node', args: ['slack.js'] } });
await run(approveCommand, { name: 'slack', all: false });
expect(stateOf('slack')).toBe('approved');

// Edit the server's command — approval must no longer apply.
writeMcpJson({ slack: { command: 'curl', args: ['slack.js'] } });
expect(stateOf('slack')).toBe('pending');
});
});
132 changes: 132 additions & 0 deletions packages/cli/src/commands/mcp/approve.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

// Files for 'qwen mcp approve' / 'qwen mcp reject' commands (issue #4615).
import type { CommandModule } from 'yargs';
import type { MCPServerConfig } from '@qwen-code/qwen-code-core';
import { isGatedMcpScope } from '@qwen-code/qwen-code-core';
import { writeStdoutLine } from '../../utils/stdioHelpers.js';
import { loadSettings } from '../../config/settings.js';
import { assembleMcpServers } from '../../config/mcpServers.js';
import {
loadMcpApprovals,
type McpApprovalStatus,
} from '../../config/mcpApprovals.js';

/**
* All gated (approval-requiring) servers visible from `cwd` — project
* `.mcp.json` plus workspace `.qwen/settings.json` (#4615). Non-gated sources
* (user/system/extension) never need approval and are excluded.
*/
function loadGatedServers(cwd: string): Record<string, MCPServerConfig> {
const settings = loadSettings(cwd);
const all = assembleMcpServers(settings.merged.mcpServers, cwd);
const gated: Record<string, MCPServerConfig> = {};
for (const [serverName, config] of Object.entries(all)) {
if (isGatedMcpScope(config.scope)) {
gated[serverName] = config;
}
}
return gated;
}

async function setProjectServerStatus(
name: string | undefined,
status: McpApprovalStatus,
all: boolean,
): Promise<void> {
const cwd = process.cwd();
const servers = loadGatedServers(cwd);

const names = Object.keys(servers);
if (names.length === 0) {
writeStdoutLine(
'No approval-requiring MCP servers found (looked in .mcp.json and .qwen/settings.json).',
);
return;
}

const verb = status === 'approved' ? 'Approved' : 'Rejected';
const approvals = loadMcpApprovals();

const targets = all ? names : name ? [name] : [];
if (targets.length === 0) {
writeStdoutLine('Specify a server name or pass --all.');
return;
}

for (const target of targets) {
const config = servers[target];
if (!config) {
writeStdoutLine(
`Server "${target}" not found. Available: ${names.join(', ')}`,
);
continue;
}
// The decision binds to this exact config's hash: editing the server in
// its source file later returns it to pending (issue #4615).
await approvals.setState(cwd, target, config, status);
writeStdoutLine(
`${verb} MCP server "${target}" (bound to its current config).`,
);
}

if (status === 'approved') {
writeStdoutLine(
'Approved servers connect in your next interactive session.',
);
}
}

export const approveCommand: CommandModule = {
command: 'approve [name]',
describe:
'Approve a gated MCP server (.mcp.json or workspace .qwen/settings.json)',
builder: (yargs) =>
yargs
.usage('Usage: qwen mcp approve [options] [name]')
.positional('name', {
describe: 'Name of the gated server to approve',
type: 'string',
})
.option('all', {
describe: 'Approve all gated servers in this workspace',
type: 'boolean',
default: false,
}),
handler: async (argv) => {
await setProjectServerStatus(
argv['name'] as string | undefined,
'approved',
argv['all'] as boolean,
);
},
};

export const rejectCommand: CommandModule = {
command: 'reject [name]',
describe:
'Reject a gated MCP server (.mcp.json or workspace .qwen/settings.json)',
builder: (yargs) =>
yargs
.usage('Usage: qwen mcp reject [options] [name]')
.positional('name', {
describe: 'Name of the gated server to reject',
type: 'string',
})
.option('all', {
describe: 'Reject all gated servers in this workspace',
type: 'boolean',
default: false,
}),
handler: async (argv) => {
await setProjectServerStatus(
argv['name'] as string | undefined,
'rejected',
argv['all'] as boolean,
);
},
};
Loading
Loading