diff --git a/packages/cli/src/commands/mcp/add.test.ts b/packages/cli/src/commands/mcp/add.test.ts index d24db1a260a..5bcf833d0c1 100644 --- a/packages/cli/src/commands/mcp/add.test.ts +++ b/packages/cli/src/commands/mcp/add.test.ts @@ -63,7 +63,7 @@ describe('mcp add command', () => { mockSetValue = vi.fn(); mockWriteStderrLine.mockClear(); mockedLoadSettings.mockReturnValue({ - forScope: () => ({ settings: {} }), + forScope: () => ({ settings: {}, originalSettings: {} }), setValue: mockSetValue, workspace: { path: '/path/to/project' }, user: { path: '/home/user' }, @@ -175,7 +175,7 @@ describe('mcp add command', () => { const setupMocks = (cwd: string, workspacePath: string) => { vi.spyOn(process, 'cwd').mockReturnValue(cwd); mockedLoadSettings.mockReturnValue({ - forScope: () => ({ settings: {} }), + forScope: () => ({ settings: {}, originalSettings: {} }), setValue: mockSetValue, workspace: { path: workspacePath }, user: { path: '/home/user' }, @@ -325,15 +325,17 @@ describe('mcp add command', () => { const updatedArgs = ['new']; beforeEach(() => { + const serverData = { + mcpServers: { + [serverName]: { + command: initialCommand, + }, + }, + }; mockedLoadSettings.mockReturnValue({ forScope: () => ({ - settings: { - mcpServers: { - [serverName]: { - command: initialCommand, - }, - }, - }, + settings: serverData, + originalSettings: serverData, }), setValue: mockSetValue, workspace: { path: '/path/to/project' }, diff --git a/packages/cli/src/commands/mcp/add.ts b/packages/cli/src/commands/mcp/add.ts index 3ecb3384b45..9b8327b6c48 100644 --- a/packages/cli/src/commands/mcp/add.ts +++ b/packages/cli/src/commands/mcp/add.ts @@ -166,8 +166,11 @@ async function addMcpServer( break; } - const existingSettings = settings.forScope(settingsScope).settings; - const mcpServers = existingSettings.mcpServers || {}; + const existingSettings = settings.forScope(settingsScope).originalSettings; + const mcpServers = (existingSettings.mcpServers || {}) as Record< + string, + MCPServerConfig + >; const isExistingServer = !!mcpServers[name]; if (isExistingServer) { diff --git a/packages/cli/src/commands/mcp/remove.test.ts b/packages/cli/src/commands/mcp/remove.test.ts index e2fb6d6d213..09a3562048c 100644 --- a/packages/cli/src/commands/mcp/remove.test.ts +++ b/packages/cli/src/commands/mcp/remove.test.ts @@ -67,7 +67,12 @@ describe('mcp remove command', () => { }, }; mockedLoadSettings.mockReturnValue({ - forScope: () => ({ settings: mockSettings }), + workspace: { path: '/home/user/project' }, + user: { path: '/home/user' }, + forScope: () => ({ + settings: mockSettings, + originalSettings: mockSettings, + }), setValue: mockSetValue, }); mockWriteStdoutLine.mockClear(); diff --git a/packages/cli/src/commands/mcp/remove.ts b/packages/cli/src/commands/mcp/remove.ts index 3de482d8d0d..69b0bb7109c 100644 --- a/packages/cli/src/commands/mcp/remove.ts +++ b/packages/cli/src/commands/mcp/remove.ts @@ -7,7 +7,7 @@ // File for 'qwen mcp remove' command import type { CommandModule } from 'yargs'; import { loadSettings, SettingScope } from '../../config/settings.js'; -import { writeStdoutLine } from '../../utils/stdioHelpers.js'; +import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js'; import { MCPOAuthTokenStorage } from '@qwen-code/qwen-code-core'; async function removeMcpServer( @@ -20,9 +20,20 @@ async function removeMcpServer( const settingsScope = scope === 'user' ? SettingScope.User : SettingScope.Workspace; const settings = loadSettings(); + const inHome = settings.workspace.path === settings.user.path; - const existingSettings = settings.forScope(settingsScope).settings; - const mcpServers = existingSettings.mcpServers || {}; + if (scope === 'project' && inHome) { + writeStderrLine( + 'Error: Please use --scope user to edit settings in the home directory.', + ); + process.exit(1); + } + + const existingSettings = settings.forScope(settingsScope).originalSettings; + const mcpServers = (existingSettings.mcpServers || {}) as Record< + string, + Record + >; if (!mcpServers[name]) { writeStdoutLine(`Server "${name}" not found in ${scope} settings.`); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index 2459b7df683..20dbefb85b4 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -470,8 +470,20 @@ export class LoadedSettings { value = stripRuntimeSnapshotPrefix(value); } const settingsFile = this.forScope(scope); - setNestedPropertySafe(settingsFile.settings, key, value); + // Write the raw value to originalSettings (preserves ${VAR} tokens for + // settings like mcpServers that may contain env-var references). setNestedPropertySafe(settingsFile.originalSettings, key, value); + // Write the env-resolved version to .settings so that consumers of + // settings.merged see actual values, not literal ${VAR} strings. + const resolved = resolveEnvVarsInObject( + { [key]: value } as Settings, + getHomeEnvFallbackVars(), + ); + setNestedPropertySafe( + settingsFile.settings, + key, + (resolved as Record)[key], + ); this._merged = this.computeMergedSettings(); const replacePath = key === 'mcpServers' ? key.split('.') : []; saveSettings(settingsFile, createSettingsUpdate(key, value), replacePath);