Skip to content

Conversation

@calmini
Copy link
Contributor

@calmini calmini commented Jul 8, 2025

User description

  • add gemini-cli and codex-cli client manager

PR Type

Enhancement


Description

  • Add support for Gemini CLI and Codex CLI client managers

  • Implement TOML configuration handling for Codex CLI

  • Register new client managers in the client registry

  • Add required TOML dependencies to project


Changes diagram

flowchart LR
  A["Client Registry"] --> B["Gemini CLI Manager"]
  A --> C["Codex CLI Manager"]
  B --> D["JSON Config (~/.gemini/settings.json)"]
  C --> E["TOML Config (~/.codex/config.toml)"]
  F["TOML Dependencies"] --> C
Loading

Changes walkthrough 📝

Relevant files
Enhancement
client_registry.py
Register new CLI client managers                                                 

src/mcpm/clients/client_registry.py

  • Import new GeminiCliManager and CodexCliManager classes
  • Register both managers in the _managers dictionary with keys
    "gemini-cli" and "codex-cli"
  • +4/-0     
    __init__.py
    Update module exports for new managers                                     

    src/mcpm/clients/managers/init.py

  • Import GeminiCliManager and CodexCliManager classes
  • Add missing VSCodeManager import
  • Update __all__ list to include all new managers
  • +6/-0     
    codex_cli.py
    Implement Codex CLI client manager                                             

    src/mcpm/clients/managers/codex_cli.py

  • Implement CodexCliManager class extending JSONClientManager
  • Handle TOML configuration format using tomli and tomli_w
  • Configure for ~/.codex/config.toml with mcp_servers key
  • Include client detection and info methods
  • +110/-0 
    gemini_cli.py
    Implement Gemini CLI client manager                                           

    src/mcpm/clients/managers/gemini_cli.py

  • Implement GeminiCliManager class extending JSONClientManager
  • Configure for ~/.gemini/settings.json with standard JSON format
  • Include default settings like contextFileName, autoAccept, and theme
  • Add client detection and info methods
  • +66/-0   
    Dependencies
    pyproject.toml
    Add TOML handling dependencies                                                     

    pyproject.toml

  • Add tomli>=2.2.1 dependency for TOML reading
  • Add tomli-w>=1.2.0 dependency for TOML writing
  • +2/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @calmini calmini changed the title feat: support more cli-client feat: support more cli-clients Jul 8, 2025
    @qodo-merge-pro qodo-merge-pro bot changed the title feat: support more cli-clients feat: support more cli-client Jul 8, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 8, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit c1d0d28)

    Here are some key observations to aid the review process:

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

    Incorrect URL

    The download URL points to a generic OpenAI Codex repository that may not be the actual CLI tool being integrated. This should be verified and corrected to point to the actual Codex CLI tool.

    download_url = "https://github.com/openai/codex"
    configure_key_name = "mcp_servers"  # Codex uses mcp_servers instead of mcpServers
    Incorrect URL

    The download URL points to a repository that may not exist or be the correct Gemini CLI tool. This should be verified and updated to point to the actual Gemini CLI repository.

    download_url = "https://github.com/google-gemini/gemini-cli"
    Missing Validation

    The TOML loading and saving operations lack proper error handling for malformed TOML files. The current exception handling is too broad and may mask specific TOML parsing errors that could help with debugging.

        # Codex uses TOML format instead of JSON
        with open(self.config_path, "rb") as f:
            config = tomli.load(f)
    
        # Ensure mcp_servers key exists
        if self.configure_key_name not in config:
            config[self.configure_key_name] = {}
    
        return config
    except Exception as e:
        logger.error(f"Error loading Codex config: {e}")
        return self._get_empty_config()

    @calmini calmini marked this pull request as draft July 8, 2025 12:33
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 8, 2025

    PR Code Suggestions ✨

    Latest suggestions up to c1d0d28
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Handle empty directory path case

    Handle the case where self.config_path might be just a filename without a
    directory path. The os.path.dirname() call could return an empty string, causing
    os.makedirs() to fail or behave unexpectedly.

    src/mcpm/clients/managers/codex_cli.py [91-110]

     def _save_config(self, config: Dict[str, Any]) -> bool:
         """Save configuration to client config file
     
         Args:
             config: Configuration to save
     
         Returns:
             bool: Success or failure
         """
         try:
             # Create directory if it doesn't exist
    -        os.makedirs(os.path.dirname(self.config_path), exist_ok=True)
    +        config_dir = os.path.dirname(self.config_path)
    +        if config_dir:
    +            os.makedirs(config_dir, exist_ok=True)
     
             # Codex uses TOML format instead of JSON
             with open(self.config_path, "wb") as f:
                 tomli_w.dump(config, f)
             return True
         except Exception as e:
             logger.error(f"Error saving Codex config: {e}")
             return False
    • Apply / Chat
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion correctly identifies that os.path.dirname can return an empty string, and while os.makedirs('', exist_ok=True) doesn't error, explicitly checking for an empty directory path improves code clarity and robustness.

    Low
    • Update

    Previous suggestions

    Suggestions up to commit c1d0d28
    CategorySuggestion                                                                                                                                    Impact
    General
    Verify default configuration values

    The hardcoded default values like "GEMINI.md", False, and "Default" may not
    align with the actual Gemini CLI defaults. Consider making these configurable or
    verify they match the official Gemini CLI documentation to prevent configuration
    conflicts.

    src/mcpm/clients/managers/gemini_cli.py [37-45]

     def _get_empty_config(self) -> Dict[str, Any]:
         """Get empty config structure for Gemini CLI"""
         return {
             "mcpServers": {},
             # Include other default settings that Gemini CLI expects
             "contextFileName": "GEMINI.md",
             "autoAccept": False,
    -        "theme": "Default"
    +        "theme": "default"
         }
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly points out that hardcoded default values for an external tool should be verified, which is important for preventing potential configuration issues.

    Low
    Handle empty directory path safely

    The os.path.dirname() call on self.config_path could fail if the path is just a
    filename without directory components. This would cause os.makedirs() to receive
    an empty string, potentially creating unexpected behavior.

    src/mcpm/clients/managers/codex_cli.py [91-110]

     def _save_config(self, config: Dict[str, Any]) -> bool:
         """Save configuration to client config file
     
         Args:
             config: Configuration to save
     
         Returns:
             bool: Success or failure
         """
         try:
             # Create directory if it doesn't exist
    -        os.makedirs(os.path.dirname(self.config_path), exist_ok=True)
    +        config_dir = os.path.dirname(self.config_path)
    +        if config_dir:
    +            os.makedirs(config_dir, exist_ok=True)
     
             # Codex uses TOML format instead of JSON
             with open(self.config_path, "wb") as f:
                 tomli_w.dump(config, f)
             return True
         except Exception as e:
             logger.error(f"Error saving Codex config: {e}")
             return False
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion correctly identifies an edge case where os.path.dirname could return an empty string, and the proposed change improves code robustness by handling this case explicitly.

    Low

    @calmini calmini marked this pull request as ready for review July 9, 2025 01:48
    🤖 Generated with [Claude Code](https://claude.ai/code)
    
    Co-Authored-By: Claude <[email protected]>
    @calmini calmini merged commit 17ec81e into main Jul 9, 2025
    7 checks passed
    @calmini calmini deleted the calmini/cli-coding-client-supports branch July 9, 2025 09:57
    mcpm-semantic-release bot pushed a commit that referenced this pull request Jul 9, 2025
    # [2.3.0](v2.2.0...v2.3.0) (2025-07-09)
    
    ### Features
    
    * support more cli-client ([#210](#210)) ([17ec81e](17ec81e))
    @mcpm-semantic-release
    Copy link

    🎉 This PR is included in version 2.3.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    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.

    3 participants