Skip to content

Conversation

@JoJoJoJoJoJoJo
Copy link
Contributor

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo commented Jun 30, 2025

User description

Resolves #190


PR Type

Bug fix


Description

  • Fix TypeError in mcpm add command for claude-desktop client

  • Update method signature to accept optional alias_name parameter

  • Ensure compatibility with base class method calls


Changes diagram

flowchart LR
  A["mcpm add command"] --> B["activate_profile()"]
  B --> C["_format_router_server()"]
  C --> D["TypeError: wrong argument count"]
  E["Fixed method signature"] --> F["Accepts alias_name parameter"]
  F --> G["Successful profile activation"]
Loading

Changes walkthrough 📝

Relevant files
Bug fix
claude_desktop.py
Fix method signature compatibility issue                                 

src/mcpm/clients/managers/claude_desktop.py

  • Updated _format_router_server method signature to accept optional
    alias_name parameter
  • Added comprehensive docstring explaining the fix
  • Forward alias_name to format_server_url_with_proxy_headers function
  • Maintain compatibility with base class method signature
  • +15/-2   
    Miscellaneous
    rollout-2025-06-30T06-58-13-60c64452-c812-4ba8-ab56-6f2e1247bdd8.jsonl
    Codex automation session log                                                         

    .github/codex/home/sessions/rollout-2025-06-30T06-58-13-60c64452-c812-4ba8-ab56-6f2e1247bdd8.jsonl

  • Automated session log from Codex fix attempt
  • Contains debugging steps and patch application
  • Documents the fix implementation process
  • +33/-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.
  • @JoJoJoJoJoJoJo JoJoJoJoJoJoJo marked this pull request as ready for review June 30, 2025 07:11
    Copilot AI review requested due to automatic review settings June 30, 2025 07:11
    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    190 - PR Code Verified

    Compliant requirements:

    • Fix TypeError in mcpm add command when using @claude-desktop as active client
    • Error occurs because ClaudeDesktopManager._format_router_server() takes 3 positional arguments but 4 were given
    • Fix should allow commands like mcpm add %profilename @claude-desktop to work properly

    Requires further human verification:

    • Verify that mcpm cp command still works correctly after the fix
    • Test the actual mcpm add command with claude-desktop to ensure the TypeError is resolved

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

    Method Signature

    The updated method signature correctly matches the base class expectation by accepting the optional alias_name parameter. The implementation properly forwards all parameters to the underlying function.

    def _format_router_server(
        self, profile_name: str, base_url: str, alias_name: str | None = None
    ) -> ServerConfig:
        """Construct a ServerConfig for a router entry.
    
        The parent JSONClientManager passes in an optional ``alias_name`` which
        should be used as the final server name inside the client config when
        provided.  Accepting the parameter keeps the overriding method
        signature compatible with the base implementation and prevents the
        ``TypeError`` that occurred when ``activate_profile`` tried to forward
        three arguments.
        """
        return format_server_url_with_proxy_headers(
            self.client_key, profile_name, base_url, alias_name
        )

    Copy link
    Contributor

    Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull Request Overview

    This pull request fixes the “TypeError” raised by the incorrect method signature in ClaudeDesktopManager by updating _format_router_server to accept an optional alias_name parameter. It adjusts the method signature and forwards the alias_name to the URL formatting helper function.

    • Updated _format_router_server signature to include the alias_name parameter.
    • Ensured compatibility with the parent class by preventing a TypeError when three arguments are passed.

    Reviewed Changes

    Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

    File Description
    src/mcpm/clients/managers/claude_desktop.py Updated _format_router_server method to forward alias_name and resolve the TypeError.
    .github/codex/home/sessions/rollout-2025-06-30T06-58-13-60c64452-c812-4ba8-ab56-6f2e1247bdd8.jsonl Session log demonstrating the issue and confirmation of the fix.
    Comments suppressed due to low confidence (1)

    src/mcpm/clients/managers/claude_desktop.py:130

    • One definition returns a server config using format_server_url_with_proxy_headers while the duplicate later uses format_server_url; it is recommended to standardize on one approach for clarity and consistency.
            )
    

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 30, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @JoJoJoJoJoJoJo JoJoJoJoJoJoJo merged commit 42ebf9b into main Jul 1, 2025
    7 checks passed
    @JoJoJoJoJoJoJo JoJoJoJoJoJoJo deleted the codex-fix-190-fixed-mcpm-add-crash-for-the-claude-desktop-client branch July 1, 2025 08:02
    mcpm-semantic-release bot pushed a commit that referenced this pull request Jul 1, 2025
    ## [1.14.2](v1.14.1...v1.14.2) (2025-07-01)
    
    ### Bug Fixes
    
    * automated fix for [#190](#190) via Codex ([#192](#192)) ([42ebf9b](42ebf9b))
    @mcpm-semantic-release
    Copy link

    🎉 This PR is included in version 1.14.2 🎉

    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.

    [General]: mcpm add with @claude-desktop as the active client raises a TypeError

    3 participants