Merge pull request #1752 from QwenLM/fix/mcp-oauth-branding-updates

fix(mcp): improve MCP server management and authentication

Author: tanzhenxin

integration-tests/mcp_server_cyclic_schema.test.ts

@@ -14,7 +14,7 @@
  * schema object which has stricter typing and recursion restrictions.
  * If this test fails, it's likely because either the GenAI SDK or Gemini API
  * has become more restrictive about the type of tool parameter schemas that
- * are accepted. If this occurs: Gemini CLI previously attempted to detect
+ * are accepted. If this occurs: Qwen Code previously attempted to detect
  * such tools and proactively remove them from the set of tools provided in
  * the Gemini API call (as FunctionDeclaration objects). It may be appropriate
  * to resurrect that behavior but note that it's difficult to keep the

packages/cli/src/commands/mcp/add.test.ts

@@ -65,22 +65,50 @@ describe('mcp add command', () => {
     });
   });
 
-  it('should add a stdio server to project settings', async () => {
+  it('should add a stdio server to user settings by default', async () => {
     await parser.parseAsync(
       'add my-server /path/to/server arg1 arg2 -e FOO=bar',
     );
 
-    expect(mockSetValue).toHaveBeenCalledWith(
-      SettingScope.Workspace,
-      'mcpServers',
-      {
-        'my-server': {
-          command: '/path/to/server',
-          args: ['arg1', 'arg2'],
-          env: { FOO: 'bar' },
-        },
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'my-server': {
+        command: '/path/to/server',
+        args: ['arg1', 'arg2'],
+        env: { FOO: 'bar' },
+      },
+    });
+  });
+
+  it('should auto-detect http transport when commandOrUrl is an https URL', async () => {
+    await parser.parseAsync('add http-server https://example.com/mcp');
+
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'http-server': {
+        httpUrl: 'https://example.com/mcp',
       },
+    });
+  });
+
+  it('should auto-detect http transport when commandOrUrl is an http URL', async () => {
+    await parser.parseAsync('add http-server http://localhost:8080/mcp');
+
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'http-server': {
+        httpUrl: 'http://localhost:8080/mcp',
+      },
+    });
+  });
+
+  it('should respect explicit transport even when commandOrUrl is a URL', async () => {
+    await parser.parseAsync(
+      'add --transport sse sse-server https://example.com/sse-endpoint',
     );
+
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'sse-server': {
+        url: 'https://example.com/sse-endpoint',
+      },
+    });
   });
 
   it('should add an sse server to user settings', async () => {
@@ -96,55 +124,43 @@ describe('mcp add command', () => {
     });
   });
 
-  it('should add an http server to project settings', async () => {
+  it('should add an http server to user settings by default', async () => {
     await parser.parseAsync(
       'add --transport http http-server https://example.com/mcp -H "Authorization: Bearer your-token"',
     );
 
-    expect(mockSetValue).toHaveBeenCalledWith(
-      SettingScope.Workspace,
-      'mcpServers',
-      {
-        'http-server': {
-          httpUrl: 'https://example.com/mcp',
-          headers: { Authorization: 'Bearer your-token' },
-        },
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'http-server': {
+        httpUrl: 'https://example.com/mcp',
+        headers: { Authorization: 'Bearer your-token' },
       },
-    );
+    });
   });
 
   it('should handle MCP server args with -- separator', async () => {
     await parser.parseAsync(
       'add my-server npx -- -y http://example.com/some-package',
     );
 
-    expect(mockSetValue).toHaveBeenCalledWith(
-      SettingScope.Workspace,
-      'mcpServers',
-      {
-        'my-server': {
-          command: 'npx',
-          args: ['-y', 'http://example.com/some-package'],
-        },
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'my-server': {
+        command: 'npx',
+        args: ['-y', 'http://example.com/some-package'],
       },
-    );
+    });
   });
 
   it('should handle unknown options as MCP server args', async () => {
     await parser.parseAsync(
       'add test-server npx -y http://example.com/some-package',
     );
 
-    expect(mockSetValue).toHaveBeenCalledWith(
-      SettingScope.Workspace,
-      'mcpServers',
-      {
-        'test-server': {
-          command: 'npx',
-          args: ['-y', 'http://example.com/some-package'],
-        },
+    expect(mockSetValue).toHaveBeenCalledWith(SettingScope.User, 'mcpServers', {
+      'test-server': {
+        command: 'npx',
+        args: ['-y', 'http://example.com/some-package'],
       },
-    );
+    });
   });
 
   describe('when handling scope and directory', () => {
@@ -166,10 +182,10 @@ describe('mcp add command', () => {
         setupMocks('/path/to/project', '/path/to/project');
       });
 
-      it('should use project scope by default', async () => {
+      it('should use user scope by default', async () => {
         await parser.parseAsync(`add ${serverName} ${command}`);
         expect(mockSetValue).toHaveBeenCalledWith(
-          SettingScope.Workspace,
+          SettingScope.User,
           'mcpServers',
           expect.any(Object),
         );
@@ -199,10 +215,10 @@ describe('mcp add command', () => {
         setupMocks('/path/to/project/subdir', '/path/to/project');
       });
 
-      it('should use project scope by default', async () => {
+      it('should use user scope by default', async () => {
         await parser.parseAsync(`add ${serverName} ${command}`);
         expect(mockSetValue).toHaveBeenCalledWith(
-          SettingScope.Workspace,
+          SettingScope.User,
           'mcpServers',
           expect.any(Object),
         );
@@ -214,22 +230,14 @@ describe('mcp add command', () => {
         setupMocks('/home/user', '/home/user');
       });
 
-      it('should show an error by default', async () => {
-        const mockProcessExit = vi
-          .spyOn(process, 'exit')
-          .mockImplementation((() => {
-            throw new Error('process.exit called');
-          }) as (code?: number) => never);
-
-        await expect(
-          parser.parseAsync(`add ${serverName} ${command}`),
-        ).rejects.toThrow('process.exit called');
-
-        expect(mockWriteStderrLine).toHaveBeenCalledWith(
-          'Error: Please use --scope user to edit settings in the home directory.',
+      it('should use user scope by default without error', async () => {
+        await parser.parseAsync(`add ${serverName} ${command}`);
+        expect(mockSetValue).toHaveBeenCalledWith(
+          SettingScope.User,
+          'mcpServers',
+          expect.any(Object),
         );
-        expect(mockProcessExit).toHaveBeenCalledWith(1);
-        expect(mockSetValue).not.toHaveBeenCalled();
+        expect(mockWriteStderrLine).not.toHaveBeenCalled();
       });
 
       it('should show an error when --scope=project is used explicitly', async () => {
@@ -266,16 +274,16 @@ describe('mcp add command', () => {
         setupMocks('/home/user/some/dir', '/home/user/some/dir');
       });
 
-      it('should use project scope by default', async () => {
+      it('should use user scope by default', async () => {
         await parser.parseAsync(`add ${serverName} ${command}`);
         expect(mockSetValue).toHaveBeenCalledWith(
-          SettingScope.Workspace,
+          SettingScope.User,
           'mcpServers',
           expect.any(Object),
         );
       });
 
-      it('should write to the WORKSPACE scope, not the USER scope', async () => {
+      it('should write to the USER scope by default', async () => {
         await parser.parseAsync(`add my-new-server echo`);
 
         // We expect setValue to be called once.
@@ -284,8 +292,8 @@ describe('mcp add command', () => {
         // We get the scope that setValue was called with.
         const calledScope = mockSetValue.mock.calls[0][0];
 
-        // We assert that the scope was Workspace, not User.
-        expect(calledScope).toBe(SettingScope.Workspace);
+        // We assert that the scope was User by default.
+        expect(calledScope).toBe(SettingScope.User);
       });
     });
 
@@ -294,10 +302,10 @@ describe('mcp add command', () => {
         setupMocks('/tmp/foo', '/tmp/foo');
       });
 
-      it('should use project scope by default', async () => {
+      it('should use user scope by default', async () => {
         await parser.parseAsync(`add ${serverName} ${command}`);
         expect(mockSetValue).toHaveBeenCalledWith(
-          SettingScope.Workspace,
+          SettingScope.User,
           'mcpServers',
           expect.any(Object),
         );
@@ -328,12 +336,12 @@ describe('mcp add command', () => {
       });
     });
 
-    it('should update the existing server in the project scope', async () => {
+    it('should update the existing server in the user scope by default', async () => {
       await parser.parseAsync(
         `add ${serverName} ${updatedCommand} ${updatedArgs.join(' ')}`,
       );
       expect(mockSetValue).toHaveBeenCalledWith(
-        SettingScope.Workspace,
+        SettingScope.User,
         'mcpServers',
         expect.objectContaining({
           [serverName]: expect.objectContaining({

packages/cli/src/commands/mcp/add.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-// File for 'gemini mcp add' command
+// File for 'qwen mcp add' command
 import type { CommandModule } from 'yargs';
 import { loadSettings, SettingScope } from '../../config/settings.js';
 import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js';
@@ -159,14 +159,14 @@ export const addCommand: CommandModule = {
         alias: 's',
         describe: 'Configuration scope (user or project)',
         type: 'string',
-        default: 'project',
+        default: 'user',
         choices: ['user', 'project'],
       })
       .option('transport', {
         alias: 't',
-        describe: 'Transport type (stdio, sse, http)',
+        describe:
+          'Transport type (stdio, sse, http). Auto-detected from URL if not specified.',
         type: 'string',
-        default: 'stdio',
         choices: ['stdio', 'sse', 'http'],
       })
       .option('env', {
@@ -211,6 +211,20 @@ export const addCommand: CommandModule = {
           const existingArgs = (argv['args'] as Array<string | number>) || [];
           argv['args'] = [...existingArgs, ...(argv['--'] as string[])];
         }
+
+        // Auto-detect transport from URL if not explicitly specified
+        if (!argv['transport']) {
+          const commandOrUrl = argv['commandOrUrl'] as string;
+          if (
+            commandOrUrl &&
+            (commandOrUrl.startsWith('http://') ||
+              commandOrUrl.startsWith('https://'))
+          ) {
+            argv['transport'] = 'http';
+          } else {
+            argv['transport'] = 'stdio';
+          }
+        }
       }),
   handler: async (argv) => {
     await addMcpServer(

packages/cli/src/commands/mcp/list.ts

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-// File for 'gemini mcp list' command
+// File for 'qwen mcp list' command
 import type { CommandModule } from 'yargs';
 import { loadSettings } from '../../config/settings.js';
 import { writeStdoutLine } from '../../utils/stdioHelpers.js';

packages/cli/src/commands/mcp/remove.test.ts

@@ -11,6 +11,7 @@ import { removeCommand } from './remove.js';
 
 const mockWriteStdoutLine = vi.hoisted(() => vi.fn());
 const mockWriteStderrLine = vi.hoisted(() => vi.fn());
+const mockDeleteCredentials = vi.hoisted(() => vi.fn());
 
 vi.mock('../../utils/stdioHelpers.js', () => ({
   writeStdoutLine: mockWriteStdoutLine,
@@ -35,6 +36,17 @@ vi.mock('../../config/settings.js', async () => {
   };
 });
 
+vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
+  const actual =
+    await importOriginal<typeof import('@qwen-code/qwen-code-core')>();
+  return {
+    ...actual,
+    MCPOAuthTokenStorage: vi.fn(() => ({
+      deleteCredentials: mockDeleteCredentials,
+    })),
+  };
+});
+
 const mockedLoadSettings = loadSettings as vi.Mock;
 
 describe('mcp remove command', () => {
@@ -59,24 +71,45 @@ describe('mcp remove command', () => {
       setValue: mockSetValue,
     });
     mockWriteStdoutLine.mockClear();
+    mockDeleteCredentials.mockClear();
+  });
+
+  it('should remove a server from user settings by default', async () => {
+    await parser.parseAsync('remove test-server');
+
+    expect(mockSetValue).toHaveBeenCalledWith(
+      SettingScope.User,
+      'mcpServers',
+      {},
+    );
   });
 
-  it('should remove a server from project settings', async () => {
+  it('should clean up OAuth tokens when removing a server', async () => {
+    await parser.parseAsync('remove test-server');
+
+    expect(mockDeleteCredentials).toHaveBeenCalledWith('test-server');
+  });
+
+  it('should not fail if OAuth token cleanup fails', async () => {
+    mockDeleteCredentials.mockRejectedValue(new Error('cleanup failed'));
+
     await parser.parseAsync('remove test-server');
 
+    // Server should still be removed from settings despite token cleanup failure
     expect(mockSetValue).toHaveBeenCalledWith(
-      SettingScope.Workspace,
+      SettingScope.User,
       'mcpServers',
       {},
     );
   });
 
-  it('should show a message if server not found', async () => {
+  it('should not clean up OAuth tokens if server not found', async () => {
     await parser.parseAsync('remove non-existent-server');
 
     expect(mockSetValue).not.toHaveBeenCalled();
+    expect(mockDeleteCredentials).not.toHaveBeenCalled();
     expect(mockWriteStdoutLine).toHaveBeenCalledWith(
-      'Server "non-existent-server" not found in project settings.',
+      'Server "non-existent-server" not found in user settings.',
     );
   });
 });