Skip to content

Conversation

@GabrielDrapor
Copy link
Contributor

@GabrielDrapor GabrielDrapor commented Jul 11, 2025

User description

since the Streamable HTTP is not stable enought to trust


PR Type

Enhancement


Description

  • Add SSE (Server-Sent Events) transport support to run commands

  • Extend both mcpm run and mcpm profile run with --sse flag

  • Update UI panels and logging for SSE mode indication

  • Add mutual exclusion validation between --http and --sse flags


Changes diagram

flowchart LR
  A["mcpm run/profile run"] --> B["Transport Selection"]
  B --> C["stdio (default)"]
  B --> D["HTTP (--http)"]
  B --> E["SSE (--sse)"]
  E --> F["FastMCP Proxy"]
  D --> F
  C --> F
  F --> G["Server Execution"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
run.py
Add SSE support to profile run command                                     

src/mcpm/commands/profile/run.py

  • Add --sse flag and sse_mode parameter to profile run command
  • Update FastMCP proxy initialization with SSE transport support
  • Modify UI panels to show SSE-specific titles and URLs
  • Add validation to prevent simultaneous --http and --sse usage
  • +49/-18 
    run.py
    Add SSE support to server run command                                       

    src/mcpm/commands/run.py

  • Add --sse flag and sse_mode parameter to server run command
  • Update FastMCP proxy calls with SSE transport configuration
  • Modify server information panels for SSE mode display
  • Add mutual exclusion validation between HTTP and SSE flags
  • +55/-22 

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    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

    Inconsistent Logging

    The KeyboardInterrupt handler has inconsistent logging behavior - it logs both info and warning messages for HTTP/SSE modes, which could create duplicate or confusing output.

    logger.info("Profile execution interrupted")
    if http_mode or sse_mode:
        logger.warning("\nProfile execution interrupted")
    return 130
    Inconsistent Logging

    Similar to profile run, the KeyboardInterrupt handler logs both info and warning messages for HTTP/SSE modes, potentially causing duplicate output.

    logger.info("Server execution interrupted")
    if http_mode or sse_mode:
        logger.warning("\nServer execution interrupted")
    return 130
    Exit Code Inconsistency

    The validation error for mutually exclusive flags uses sys.exit(1) while other error cases use return 1, creating inconsistent exit handling patterns.

    # Validate mutually exclusive options
    if http and sse:
        logger.error("Error: Cannot use both --http and --sse flags together")
        sys.exit(1)

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Use consistent error handling pattern

    Using sys.exit(1) directly in the middle of the function can bypass cleanup
    logic and is inconsistent with the function's return pattern. Return an error
    code instead to maintain consistency.

    src/mcpm/commands/run.py [189-192]

     # Validate mutually exclusive options
     if http and sse:
         logger.error("Error: Cannot use both --http and --sse flags together")
    -    sys.exit(1)
    +    return 1
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly points out that using sys.exit(1) is inconsistent with the error handling in the rest of the function, and replacing it with return 1 improves code consistency.

    Low
    Organization
    best practice
    Improve SSE option help text

    The new --sse option lacks consistent help text formatting compared to existing
    patterns. The help text should be more descriptive and follow the established
    pattern of explaining the functionality clearly.

    src/mcpm/commands/profile/run.py [137-139]

    -@click.option("--sse", is_flag=True, help="Run profile over SSE instead of stdio")
    -@click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP / SSE mode (default: {DEFAULT_PORT})")
    -@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP / SSE mode (default: 127.0.0.1)")
    +@click.option("--sse", is_flag=True, help="Run profile over Server-Sent Events (SSE) instead of stdio")
    +@click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP/SSE mode (default: {DEFAULT_PORT})")
    +@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP/SSE mode (default: 127.0.0.1)")
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - When implementing command-line interfaces with Click, use consistent help option patterns and provide clear, structured help text with examples. Include both short (-h) and long (--help) options, and format examples using backslash-escaped blocks for proper display.

    Low
    Standardize SSE option help formatting

    The new --sse option help text is inconsistent with the established pattern and
    lacks clarity. The spacing in "HTTP / SSE" should be consistent across both
    files and the SSE acronym should be expanded for clarity.

    src/mcpm/commands/run.py [135-137]

    -@click.option("--sse", is_flag=True, help="Run server over SSE instead of stdio")
    -@click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP / SSE mode (default: {DEFAULT_PORT})")
    -@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP / SSE mode (default: 127.0.0.1)")
    +@click.option("--sse", is_flag=True, help="Run server over Server-Sent Events (SSE) instead of stdio")
    +@click.option("--port", type=int, default=DEFAULT_PORT, help=f"Port for HTTP/SSE mode (default: {DEFAULT_PORT})")
    +@click.option("--host", type=str, default="127.0.0.1", help="Host address for HTTP/SSE mode (default: 127.0.0.1)")
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why:
    Relevant best practice - When implementing command-line interfaces with Click, use consistent help option patterns and provide clear, structured help text with examples. Include both short (-h) and long (--help) options, and format examples using backslash-escaped blocks for proper display.

    Low
    • More

    @github-actions
    Copy link
    Contributor

    Summary

    Adds “–sse” support to both mcpm run and mcpm profile run, enabling FastMCP proxy to serve over Server-Sent Events in addition to stdio and HTTP.

    Notes

    • Updates CLI flags, help text and validation (HTTP/SSE mutually exclusive).
    • Propagates sse_mode through helper functions and chooses correct transport/action.
    • Improves console panels and logging to reflect new mode.

    Review

    Nice, focused extension and code is consistent with existing patterns.
    A couple of small nits before merge:

    • Consider extracting mode/transport selection into a tiny helper to avoid repeated ternaries.
    • Unit / integration tests for the new flag would guard against regressions.
    • README / docs may need a quick mention of the new SSE option.

    View workflow run

    @GabrielDrapor GabrielDrapor merged commit e66ea87 into main Jul 11, 2025
    8 of 9 checks passed
    @GabrielDrapor GabrielDrapor deleted the Jiarui/run-sse branch July 11, 2025 10:01
    @rafaelsales
    Copy link

    rafaelsales commented Jul 12, 2025

    Can you release this?
    @GabrielDrapor

    @mcpm-semantic-release
    Copy link

    🎉 This PR is included in version 2.5.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    @GabrielDrapor
    Copy link
    Contributor Author

    @rafaelsales 👆 we've released it 😄, please upgrade your MCPM

    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.

    4 participants