Skip to content

Conversation

dan1els
Copy link

@dan1els dan1els commented Sep 30, 2025

User description

Added QwenCliManager to handle Qwen CLI MCP server configurations

Includes implementation and tests for Qwen CLI integration


PR Type

Enhancement


Description

  • Add Qwen CLI client manager support

  • Integrate QwenCliManager into client registry

  • Include comprehensive test coverage

  • Add project reference documentation


Diagram Walkthrough

flowchart LR
  A["QwenCliManager"] --> B["Client Registry"]
  A --> C["JSON Config Handler"]
  A --> D["Installation Check"]
  B --> E["MCP Server Management"]
Loading

File Walkthrough

Relevant files
Enhancement
client_registry.py
Register Qwen CLI in client registry                                         

src/mcpm/clients/client_registry.py

  • Import QwenCliManager from managers module
  • Register "qwen-cli" key in CLIENT_MANAGERS dictionary
+2/-0     
__init__.py
Export QwenCliManager in package init                                       

src/mcpm/clients/managers/init.py

  • Import QwenCliManager class
  • Add QwenCliManager to __all__ exports list
+2/-0     
qwen_cli.py
Implement Qwen CLI manager class                                                 

src/mcpm/clients/managers/qwen_cli.py

  • Implement QwenCliManager class extending JSONClientManager
  • Add config path handling for ~/.qwen/settings.json
  • Implement installation check via shutil.which
  • Provide client info with download URL and description
+65/-0   
Tests
test_qwen_cli.py
Add comprehensive Qwen CLI manager tests                                 

tests/test_clients/test_qwen_cli.py

  • Test QwenCliManager initialization with default and custom paths
  • Test empty config structure generation
  • Test client installation detection with mocked shutil.which
  • Test client info retrieval functionality
+57/-0   
Documentation
QWEN.md
Add Qwen project reference documentation                                 

QWEN.md

  • Add reference documentation pointing to CLAUDE.md
+1/-0     

Summary by CodeRabbit

  • New Features

    • Added Qwen CLI client integration with configuration handling, installation detection, and client info display.
  • Documentation

    • Added QWEN guide referencing project conventions.
  • Tests

    • Added tests for Qwen CLI: initialization, default config structure, installation checks, and info output.

Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Adds a Qwen CLI client manager implementation, exposes it in the managers package, registers it in the client registry, adds unit tests for its behavior, and introduces a one-line QWEN.md reference file.

Changes

Cohort / File(s) Summary of Changes
Docs reference
QWEN.md
New doc file containing: "Reference CLAUDE.md for project conventions."
Client registry wiring
src/mcpm/clients/client_registry.py
Imports QwenCliManager and registers "qwen-cli" in the _CLIENT_MANAGERS mapping.
Managers API exposure
src/mcpm/clients/managers/__init__.py
Imports QwenCliManager and adds it to __all__ for public export.
Qwen CLI manager implementation
src/mcpm/clients/managers/qwen_cli.py
Adds QwenCliManager (subclass of JSONClientManager) with class metadata (client_key, display_name, download_url), configurable config path (default ~/.qwen/settings.json), _get_empty_config(), is_client_installed() using shutil.which (handles qwen.exe on Windows), and get_client_info().
Unit tests
tests/test_clients/test_qwen_cli.py
New tests for constructor defaults/override, _get_empty_config structure, is_client_installed() with patched shutil.which (including Windows exe check), and get_client_info() contents.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Registry as ClientRegistry
  participant Manager as QwenCliManager
  participant OS as OS/Shutil

  User->>Registry: request manager("qwen-cli")
  Registry->>Manager: instantiate (config path default or override)

  User->>Manager: is_client_installed()
  Manager->>OS: which("qwen" or "qwen.exe")
  OS-->>Manager: path | none
  Manager-->>User: True | False

  User->>Manager: get_client_info()
  Manager-->>User: {name, download_url, config_file, description}

  User->>Manager: _get_empty_config()
  Manager-->>User: {mcpServers: {}, theme: "Qwen Dark", selectedAuthType: "openai"}
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A whisk of code, a hop of delight,
I wired Qwen’s CLI snug and right.
Registry knows its little name,
Tests nibble edges, tame the game.
Config burrows safe and bright. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title “feat: add Qwen CLI client manager support” accurately captures the main change by indicating that Qwen CLI client manager functionality is being introduced using a concise conventional commit style. It focuses on the primary feature without extraneous details or noise. Teammates scanning the PR list can immediately grasp the purpose of the changeset from the title.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c46aace and 7b3c6d1.

📒 Files selected for processing (5)
  • QWEN.md (1 hunks)
  • src/mcpm/clients/client_registry.py (2 hunks)
  • src/mcpm/clients/managers/__init__.py (2 hunks)
  • src/mcpm/clients/managers/qwen_cli.py (1 hunks)
  • tests/test_clients/test_qwen_cli.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/test_clients/test_qwen_cli.py
  • QWEN.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Always format Python code with ruff.

Files:

  • src/mcpm/clients/managers/__init__.py
  • src/mcpm/clients/managers/qwen_cli.py
  • src/mcpm/clients/client_registry.py
🧬 Code graph analysis (3)
src/mcpm/clients/managers/__init__.py (1)
src/mcpm/clients/managers/qwen_cli.py (1)
  • QwenCliManager (15-61)
src/mcpm/clients/managers/qwen_cli.py (2)
src/mcpm/clients/base.py (1)
  • JSONClientManager (164-379)
src/mcpm/clients/client_registry.py (1)
  • get_client_info (97-110)
src/mcpm/clients/client_registry.py (1)
src/mcpm/clients/managers/qwen_cli.py (1)
  • QwenCliManager (15-61)
🔇 Additional comments (10)
src/mcpm/clients/managers/__init__.py (2)

16-16: LGTM!

The import follows the established pattern and is correctly ordered alphabetically.


30-30: LGTM!

The export is correctly added to __all__ in alphabetical order.

src/mcpm/clients/client_registry.py (2)

22-22: LGTM!

The import is correctly ordered and follows the established pattern.


54-54: LGTM!

The registry entry correctly maps "qwen-cli" to QwenCliManager and maintains alphabetical ordering.

src/mcpm/clients/managers/qwen_cli.py (6)

1-13: LGTM!

Imports are appropriate and follow the standard pattern for client managers.


50-61: LGTM!

The method correctly overrides the base implementation and adds a useful "description" field. All values are consistent with the class attributes.


42-48: self._system Initialization Verified
The _system attribute is defined in BaseClientManager (line 37), so the platform-specific executable check is valid.


33-40: Ignore unused override warning _get_empty_config in Qwen CLI is invoked by JSONClientManager._load_config (see src/mcpm/clients/base.py:403), so it’s not dead code.

Likely an incorrect or invalid review comment.


15-21: No action required: download_url is correct
The URL https://github.com/QwenLM/qwen-code points to the official Qwen Code repository for the Qwen CLI tool.


23-31: Ignore override-order concern
The default path is set before super().__init__(), and BaseClientManager only overwrites self.config_path when config_path_override is provided. The override logic works as intended—no changes needed.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Portability Assumption

The executable name is hardcoded to "qwen" or "qwen.exe" based on OS. If the CLI installs under a different name/path or via Node/npm (e.g., "qwen-cli"), detection may fail. Consider allowing an env/config override or checking multiple candidates.

def is_client_installed(self) -> bool:
    """Check if Qwen CLI is installed
    Returns:
        bool: True if qwen command is available, False otherwise
    """
    qwen_executable = "qwen.exe" if self._system == "Windows" else "qwen"
    return shutil.which(qwen_executable) is not None
Default Config Validity

The default JSON structure includes "theme" and "selectedAuthType" with specific values. Verify these keys and defaults match the actual Qwen CLI schema to avoid corrupting or misconfiguring user settings.

def _get_empty_config(self) -> Dict[str, Any]:
    """Get empty config structure for Qwen CLI"""
    return {
        "mcpServers": {},
        # Include other default settings that Qwen CLI expects
        "theme": "Qwen Dark",
        "selectedAuthType": "openai",
    }

Copy link
Contributor

qodo-merge-pro bot commented Sep 30, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add test case for Windows

Add a new test case for the is_client_installed method to verify the correct
behavior on Windows, where the executable name is different.

tests/test_clients/test_qwen_cli.py [37-47]

 def test_qwen_cli_manager_is_client_installed():
     """Test QwenCliManager is_client_installed method"""
     manager = QwenCliManager()
     
     # Mock shutil.which to return a path (simulating installed client)
-    with patch("shutil.which", return_value="/usr/local/bin/qwen"):
+    with patch("shutil.which", return_value="/usr/local/bin/qwen") as mock_which:
         assert manager.is_client_installed() is True
+        mock_which.assert_called_with("qwen")
     
     # Mock shutil.which to return None (simulating uninstalled client)
-    with patch("shutil.which", return_value=None):
+    with patch("shutil.which", return_value=None) as mock_which:
         assert manager.is_client_installed() is False
+        mock_which.assert_called_with("qwen")
 
+def test_qwen_cli_manager_is_client_installed_windows():
+    """Test QwenCliManager is_client_installed method on Windows"""
+    manager = QwenCliManager()
+    
+    with patch.object(manager, "_system", "Windows"):
+        # Mock shutil.which to return a path (simulating installed client)
+        with patch("shutil.which", return_value="C:\\Program Files\\qwen\\qwen.exe") as mock_which:
+            assert manager.is_client_installed() is True
+            mock_which.assert_called_with("qwen.exe")
+        
+        # Mock shutil.which to return None (simulating uninstalled client)
+        with patch("shutil.which", return_value=None) as mock_which:
+            assert manager.is_client_installed() is False
+            mock_which.assert_called_with("qwen.exe")
+
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out a gap in test coverage for platform-specific logic (Windows) and provides a robust test case to cover it, improving the overall quality of the tests.

Medium
Avoid redundant config path logic

Refactor the init method to remove redundant config_path_override logic by
setting the default config_path before calling super().init and letting the
parent class handle the override.

src/mcpm/clients/managers/qwen_cli.py [23-35]

 def __init__(self, config_path_override: str | None = None):
     """Initialize the Qwen CLI client manager
 
     Args:
         config_path_override: Optional path to override the default config file location
     """
+    # Qwen CLI stores its settings in ~/.qwen/settings.json
+    self.config_path = os.path.expanduser("~/.qwen/settings.json")
     super().__init__(config_path_override=config_path_override)
 
-    if config_path_override:
-        self.config_path = config_path_override
-    else:
-        # Qwen CLI stores its settings in ~/.qwen/settings.json
-        self.config_path = os.path.expanduser("~/.qwen/settings.json")
-
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies redundant logic for handling config_path_override and proposes a cleaner implementation that relies on the parent class, improving code structure and maintainability.

Low
  • Update

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/mcpm/clients/managers/qwen_cli.py (1)

23-35: Simplify config path assignment.

The conditional assignment of config_path is redundant because the parent JSONClientManager.__init__ does not set self.config_path, so the explicit check and assignment in lines 31-32 are unnecessary.

Apply this diff to simplify the logic:

     def __init__(self, config_path_override: str | None = None):
         """Initialize the Qwen CLI client manager
 
         Args:
             config_path_override: Optional path to override the default config file location
         """
         super().__init__(config_path_override=config_path_override)
 
-        if config_path_override:
-            self.config_path = config_path_override
-        else:
-            # Qwen CLI stores its settings in ~/.qwen/settings.json
-            self.config_path = os.path.expanduser("~/.qwen/settings.json")
+        # Qwen CLI stores its settings in ~/.qwen/settings.json
+        self.config_path = config_path_override or os.path.expanduser("~/.qwen/settings.json")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf2dc92 and d5940e2.

📒 Files selected for processing (5)
  • QWEN.md (1 hunks)
  • src/mcpm/clients/client_registry.py (2 hunks)
  • src/mcpm/clients/managers/__init__.py (2 hunks)
  • src/mcpm/clients/managers/qwen_cli.py (1 hunks)
  • tests/test_clients/test_qwen_cli.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Always format Python code with ruff.

Files:

  • src/mcpm/clients/managers/__init__.py
  • tests/test_clients/test_qwen_cli.py
  • src/mcpm/clients/client_registry.py
  • src/mcpm/clients/managers/qwen_cli.py
🧠 Learnings (1)
📚 Learning: 2025-08-06T08:20:03.609Z
Learnt from: CR
PR: pathintegral-institute/mcpm.sh#0
File: GEMINI.md:0-0
Timestamp: 2025-08-06T08:20:03.609Z
Learning: Reference `CLAUDE.md` for project conventions.

Applied to files:

  • QWEN.md
🧬 Code graph analysis (4)
src/mcpm/clients/managers/__init__.py (1)
src/mcpm/clients/managers/qwen_cli.py (1)
  • QwenCliManager (15-65)
tests/test_clients/test_qwen_cli.py (2)
src/mcpm/clients/managers/qwen_cli.py (4)
  • QwenCliManager (15-65)
  • _get_empty_config (37-44)
  • is_client_installed (46-52)
  • get_client_info (54-65)
src/mcpm/clients/client_registry.py (1)
  • get_client_info (97-110)
src/mcpm/clients/client_registry.py (1)
src/mcpm/clients/managers/qwen_cli.py (1)
  • QwenCliManager (15-65)
src/mcpm/clients/managers/qwen_cli.py (2)
src/mcpm/clients/base.py (1)
  • JSONClientManager (164-379)
src/mcpm/clients/client_registry.py (1)
  • get_client_info (97-110)
🔇 Additional comments (13)
QWEN.md (1)

1-1: LGTM! Follows project conventions.

The reference to CLAUDE.md aligns with the established documentation pattern for client integrations.

Based on learnings.

src/mcpm/clients/client_registry.py (2)

22-22: LGTM! Import follows project conventions.

The import is correctly placed in alphabetical order with other manager imports.


54-54: LGTM! Registry entry is correctly structured.

The registration of QwenCliManager under the key "qwen-cli" follows the established pattern for CLI-based clients and maintains alphabetical ordering.

src/mcpm/clients/managers/__init__.py (2)

16-16: LGTM! Import is correctly positioned.

The import maintains alphabetical ordering with the other client manager imports.


30-30: LGTM! Public API export is correct.

The addition to __all__ properly exposes QwenCliManager and maintains alphabetical ordering.

tests/test_clients/test_qwen_cli.py (4)

12-24: LGTM! Initialization tests are thorough.

The test correctly validates both default config path behavior and custom config path override, covering the key initialization scenarios.


27-34: LGTM! Empty config structure is validated.

The test correctly verifies all expected keys in the empty config, including the MCP-specific mcpServers and Qwen-specific defaults.


37-47: LGTM! Installation detection is properly tested.

The test effectively uses mocking to verify both installed and uninstalled states, ensuring the is_client_installed method behaves correctly.


50-57: LGTM! Client info metadata is validated.

The test verifies all expected metadata fields returned by get_client_info, ensuring consistency with the manager's class attributes.

src/mcpm/clients/managers/qwen_cli.py (4)

1-12: LGTM! Module structure is clean.

The imports and docstring follow the project's conventions for client manager modules.


15-21: LGTM! Class attributes are well-defined.

The client metadata (key, display name, download URL) follows the established pattern and provides clear identification for the Qwen CLI integration.


46-52: LGTM! Installation check is correctly implemented.

The method properly handles platform differences (Windows vs. Unix-like systems) for the executable name and uses shutil.which appropriately.


54-65: LGTM! Client info is comprehensive.

The method returns all expected metadata fields, including the Qwen-specific description, which aligns with the project's client info pattern.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/mcpm/clients/managers/qwen_cli.py (1)

12-12: Consider removing unused logger or adding log statements.

The logger is instantiated but never used in this implementation. Consider removing it or adding appropriate log statements in methods like is_client_installed or get_client_info for debugging purposes.

tests/test_clients/test_qwen_cli.py (1)

27-34: Consider merging with test_qwen_cli_manager_get_empty_config_structure.

This test overlaps significantly with test_qwen_cli_manager_get_empty_config_structure (lines 68-81), which provides more comprehensive validation. Consider merging them to avoid duplication.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d5940e2 and c46aace.

📒 Files selected for processing (5)
  • QWEN.md (1 hunks)
  • src/mcpm/clients/client_registry.py (2 hunks)
  • src/mcpm/clients/managers/__init__.py (2 hunks)
  • src/mcpm/clients/managers/qwen_cli.py (1 hunks)
  • tests/test_clients/test_qwen_cli.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • QWEN.md
  • src/mcpm/clients/client_registry.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Always format Python code with ruff.

Files:

  • src/mcpm/clients/managers/__init__.py
  • tests/test_clients/test_qwen_cli.py
  • src/mcpm/clients/managers/qwen_cli.py
🧬 Code graph analysis (3)
src/mcpm/clients/managers/__init__.py (1)
src/mcpm/clients/managers/qwen_cli.py (1)
  • QwenCliManager (15-61)
tests/test_clients/test_qwen_cli.py (1)
src/mcpm/clients/managers/qwen_cli.py (4)
  • QwenCliManager (15-61)
  • _get_empty_config (33-40)
  • is_client_installed (42-48)
  • get_client_info (50-61)
src/mcpm/clients/managers/qwen_cli.py (2)
src/mcpm/clients/base.py (1)
  • JSONClientManager (164-379)
src/mcpm/clients/client_registry.py (1)
  • get_client_info (97-110)
🔇 Additional comments (12)
src/mcpm/clients/managers/qwen_cli.py (5)

50-61: LGTM! Good extension of parent's get_client_info.

The method appropriately extends the parent's get_client_info by adding a description field, providing additional context about the client.


47-47: Incorrect issue—_system is defined
self._system is initialized in BaseClientManager (src/mcpm/clients/base.py:37), so no verification is needed.

Likely an incorrect or invalid review comment.


21-21: download_url confirmed The URL points to the official Qwen Code CLI repository, which includes installation instructions and releases.


33-40: Confirm Qwen CLI default values
Please verify that the theme ("Qwen Dark") and selectedAuthType ("openai") defaults in _get_empty_config match the built-in defaults defined by Qwen Code CLI’s settings.json or adjust them to align with the official documentation.


23-31: Initialization order is correct; no changes necessary.

QwenCliManager sets the default config_path before calling super().__init__, and BaseClientManager’s __init__ will override self.config_path only if config_path_override is provided—otherwise the default remains intact.

Likely an incorrect or invalid review comment.

src/mcpm/clients/managers/__init__.py (2)

16-16: LGTM!

The import is correctly placed in alphabetical order and follows the established pattern.


30-30: LGTM!

The export is correctly placed in alphabetical order within __all__.

tests/test_clients/test_qwen_cli.py (5)

12-24: LGTM!

The test provides comprehensive coverage of initialization scenarios, including both default and custom config paths.


37-49: LGTM!

The test properly mocks shutil.which and validates both installed and uninstalled scenarios with appropriate assertions.


52-65: LGTM!

The test properly validates Windows-specific executable detection by mocking the _system attribute and verifying that qwen.exe is checked instead of qwen.


68-81: LGTM!

Comprehensive validation of the empty config structure with specific value checks.


84-91: LGTM!

The test thoroughly validates all fields returned by get_client_info, including the custom description field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant