Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
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 @@ -1777,7 +1777,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 @@ -1786,7 +1791,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 @@ -1801,7 +1806,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 @@ -1819,7 +1824,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 @@ -1832,7 +1837,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 @@ -1849,6 +1854,7 @@ class QwenAgent implements Agent {
userHooks: this.settings.getUserHooks(),
projectHooks: this.settings.getProjectHooks(),
},
sessionMcpServers,
);
// PR 14b fix #2 (codex review round 1): register the MCP guardrail
// budget-event callback BEFORE `config.initialize()`. Pre-fix the
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/commands/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('mcp command', () => {

mcpCommand.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 +47,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;
}

function setProjectServerStatus(
name: string | undefined,
status: McpApprovalStatus,
all: boolean,
): 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).
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) => {
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) => {
setProjectServerStatus(
argv['name'] as string | undefined,
'rejected',
argv['all'] as boolean,
);
},
};
2 changes: 2 additions & 0 deletions packages/cli/src/commands/mcp/list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ vi.mock('@qwen-code/qwen-code-core', () => ({
},
ExtensionManager: vi.fn(),
getErrorMessage: (e: unknown) => (e instanceof Error ? e.message : String(e)),
isGatedMcpScope: (scope: string | undefined) =>
scope === 'project' || scope === 'workspace',
}));
vi.mock('@modelcontextprotocol/sdk/client/index.js');

Expand Down
Loading
Loading