Skip to content

feat: adds api key functionality onto mcp composer#9498

Merged
lucaseduoli merged 26 commits into
mainfrom
feat/api-key-composer
Aug 25, 2025
Merged

feat: adds api key functionality onto mcp composer#9498
lucaseduoli merged 26 commits into
mainfrom
feat/api-key-composer

Conversation

@lucaseduoli
Copy link
Copy Markdown
Collaborator

@lucaseduoli lucaseduoli commented Aug 22, 2025

This pull request refactors project authentication and MCP server installation logic for the MCP Projects API, improving security and reliability. It introduces a custom authentication dependency for project endpoints, automates API key generation based on project settings and feature flags, and enhances MCP server config management by matching servers via SSE URL instead of server name. The changes also improve feedback and error handling during installation and server checks.

Authentication and Authorization Improvements

  • Added a custom async dependency verify_project_auth_from_request that validates project authentication for each project endpoint, replacing previous checks and ensuring consistent user authorization based on project settings and feature flags. [1] [2] [3] [4]
  • Automated API key generation for MCP projects during configuration installation, triggered by project authentication settings and feature flags, and included the API key in the MCP proxy args when required. [1] [2]

MCP Server Configuration Management

  • Refactored MCP server config handling to remove any existing servers with the same SSE URL before installing or reinstalling, preventing duplicates and ensuring proper server replacement.
  • Improved installation feedback by reporting whether the server was installed or reinstalled, listing replaced servers, and specifying the authentication method used.

Server Discovery and Matching

  • Changed MCP server matching logic in check_installed_mcp_servers to use SSE URL instead of server name, ensuring accurate detection of installed servers across different clients and configurations. [1] [2] [3] [4]

Summary by CodeRabbit

  • New Features
    • Per-project authentication for MCP endpoints with API key support when required.
    • Installer now generates and injects API keys when needed, detects reinstalls, and cleans up duplicate servers by SSE URL.
    • Check-installed now identifies servers via SSE URL for more reliable detection.
  • UI
    • Auth modal updated: API key input removed; contextual reinstall notice and post-save alert added.
    • MCP Server tab: dedicated “Generate API key” button, improved install/refresh states, and better header generation.
    • Success alert shows up to three lines before truncation.
  • Breaking Changes
    • Removed API key and basic/ bearer auth fields from settings/types.

@lucaseduoli lucaseduoli requested a review from mfortman11 August 22, 2025 15:28
@lucaseduoli lucaseduoli self-assigned this Aug 22, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 22, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds per-project authentication to MCP project endpoints, introduces SSE URL/config utilities, updates install/check-installed flows to use SSE URLs and optional API key generation, removes api_key from AuthSettings schema and frontend types, and adjusts frontend MCP UI and AuthModal to reflect new auth model and reinstall messaging.

Changes

Cohort / File(s) Change Summary
Backend: MCP per-project auth & install flow
src/backend/base/langflow/api/v1/mcp_projects.py
Added verify_project_auth(+_from_request); switched endpoints to DI-based project auth; introduced get_project_sse_url/get_config_path/config_contains_sse_url/remove_server_by_sse_url; updated install_mcp_config to optionally create/use API keys and de-duplicate by SSE URL; updated check_installed to use SSE URLs; expanded imports.
Backend: Auth schema
src/backend/base/langflow/api/v1/schemas.py
Removed AuthSettings.api_key and corresponding SecretStr import.
Frontend: MCP Server UI & install/auth integration
src/frontend/src/pages/MainPage/pages/homePage/components/McpServerTab.tsx
Added MemoizedApiKeyButton; reworked auth gate to isAuthApiKey; headers generation now uses local apiKey; JSON generation reflects apiKey; installer UI/behavior updates (hover, refresh, disabled rules); passes installedClients to AuthModal.
Frontend: AuthModal
src/frontend/src/modals/authModal/index.tsx
Added installedClients prop; removed API key field/state and save logic; replaced API key input with guidance text; layout/footer updates with reinstall notice.
Frontend: Types
src/frontend/src/types/mcp/index.ts
Removed api_key, username, password, bearer_token, iam_endpoint from AuthSettingsType.
Frontend: Minor UI
src/frontend/src/alerts/success/index.tsx
Increased title line clamp from 2 to 3.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant API as API (MCP Project Endpoints)
  participant Auth as verify_project_auth_from_request
  participant UserSvc as User/Key Services
  participant MCP as MCP Proxy/Handlers

  Client->>API: Request (SSE/Messages) with headers/query
  API->>Auth: Verify(project_id, request)
  Auth->>UserSvc: Check superuser / API key validity
  UserSvc-->>Auth: User context
  Auth-->>API: Authorized User
  API->>MCP: Handle project-scoped request
  MCP-->>Client: Response/Events
Loading
sequenceDiagram
  autonumber
  participant UI as Frontend UI
  participant API as install_mcp_config
  participant Util as SSE/Config Utils
  participant Key as API Key Service

  UI->>API: Install MCP (project_id, client)
  API->>Util: get_project_sse_url(project_id)
  API->>Util: get_config_path(client)
  API->>Util: remove_server_by_sse_url(config, sse_url)
  alt Auth required for project
    API->>Key: create_api_key(ApiKeyCreate)
    Key-->>API: generated_api_key
    API->>UI: Install with --headers x-api-key
  else No auth required
    API->>UI: Install without headers
  end
  API-->>UI: Result (install/reinstall, replaced servers, key info)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

size:XXL, lgtm

Suggested reviewers

  • mfortman11
  • ogabrielluiz
  • Cristhianzl
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/api-key-composer

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions Bot added the enhancement New feature or request label Aug 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@lucaseduoli lucaseduoli marked this pull request as ready for review August 22, 2025 17:43
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@lucaseduoli lucaseduoli added this pull request to the merge queue Aug 22, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 22, 2025
@github-actions github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Aug 22, 2025
@lucaseduoli lucaseduoli enabled auto-merge August 22, 2025 23:04
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
6 Security Hotspots

See analysis details on SonarQube Cloud

@lucaseduoli lucaseduoli added this pull request to the merge queue Aug 22, 2025
codeflash-ai Bot added a commit that referenced this pull request Aug 22, 2025
…`feat/api-key-composer`)

**Key optimizations**
- Avoid 
  - repeated `config_data["mcpServers"]` lookups by assigning to `mcpServers` once.
  - redundant creation/appending to temporary list (`servers_to_remove` is built in one pass).
  - separate `removed_servers` append inside the loop, use a single `.extend()` call for better cache and memory usage.
  - repeated logger calls (which are the biggest runtime cost): use **one call** for all affected servers.
- Preserve all comments and behavior as required.

**Behavioral notes**
- If `servers_to_remove` is empty, the logger is not called, which matches the original net effect (logger is only called inside the remove-loop).
- If any removed, all names are listed in one log message; this is a valid equivalent of multiple individual debug calls, per Python logging and your requirements.
- Otherwise, the function's outputs, side effects, and signatures remain unchanged.
Comment on lines +902 to +918
msg = f"Could not determine Windows Claude config path in WSL: {e!s}"
raise ValueError(msg) from e
# Regular Windows
return Path(os.environ["APPDATA"]) / "Claude" / "claude_desktop_config.json"

msg = "Unsupported operating system for Claude configuration"
raise ValueError(msg)

msg = "Unsupported client"
raise ValueError(msg)


def remove_server_by_sse_url(config_data: dict, sse_url: str) -> tuple[dict, list[str]]:
"""Remove any MCP servers that use the specified SSE URL from config data.

Returns:
tuple: (updated_config, list_of_removed_server_names)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚡️Codeflash found 116% (1.16x) speedup for remove_server_by_sse_url in src/backend/base/langflow/api/v1/mcp_projects.py

⏱️ Runtime : 1.95 milliseconds 901 microseconds (best of 155 runs)

📝 Explanation and details

Key optimizations

  • Avoid
    • repeated config_data["mcpServers"] lookups by assigning to mcpServers once.
    • redundant creation/appending to temporary list (servers_to_remove is built in one pass).
    • separate removed_servers append inside the loop, use a single .extend() call for better cache and memory usage.
    • repeated logger calls (which are the biggest runtime cost): use one call for all affected servers.
  • Preserve all comments and behavior as required.

Behavioral notes

  • If servers_to_remove is empty, the logger is not called, which matches the original net effect (logger is only called inside the remove-loop).
  • If any removed, all names are listed in one log message; this is a valid equivalent of multiple individual debug calls, per Python logging and your requirements.
  • Otherwise, the function's outputs, side effects, and signatures remain unchanged.

Correctness verification report:

Test Status
⏪ Replay Tests 🔘 None Found
⚙️ Existing Unit Tests 🔘 None Found
🔎 Concolic Coverage Tests 🔘 None Found
🌀 Generated Regression Tests 38 Passed
📊 Tests Coverage 100.0%
🌀 Generated Regression Tests and Runtime
import copy  # for deep copying dicts in tests

# imports
import pytest  # used for our unit tests
from langflow.api.v1.mcp_projects import remove_server_by_sse_url
# function to test
from langflow.logging import logger

# unit tests

# ------------------ BASIC TEST CASES ------------------

def test_remove_single_server_by_matching_sse_url():
    # Setup: One server with matching SSE URL
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar", "http://sse.example.com"]},
            "server2": {"args": ["baz", "qux", "http://other.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    # Make a deep copy to ensure input is not mutated unexpectedly
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_remove_no_server_when_no_match():
    # Setup: No server has matching SSE URL
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar", "http://not-this.com"]},
            "server2": {"args": ["baz", "qux", "http://other.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_remove_multiple_servers_with_same_sse_url():
    # Setup: Multiple servers have the same SSE URL
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "http://sse.example.com"]},
            "server2": {"args": ["bar", "http://sse.example.com"]},
            "server3": {"args": ["baz", "http://other.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_remove_server_when_args_is_empty():
    # Setup: One server has empty args
    config = {
        "mcpServers": {
            "server1": {"args": []},
            "server2": {"args": ["foo", "http://sse.example.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_remove_server_when_args_key_missing():
    # Setup: One server missing 'args' key
    config = {
        "mcpServers": {
            "server1": {},
            "server2": {"args": ["foo", "http://sse.example.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

# ------------------ EDGE TEST CASES ------------------

def test_no_mcp_servers_key_in_config():
    # Setup: config_data does not have 'mcpServers' key
    config = {
        "otherKey": 123
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_empty_mcp_servers_dict():
    # Setup: 'mcpServers' is an empty dict
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_server_args_is_not_a_list():
    # Setup: 'args' is not a list (should be robust)
    config = {
        "mcpServers": {
            "server1": {"args": "notalist"},
            "server2": {"args": ["foo", "http://sse.example.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    # Should not remove server1, should remove server2
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_server_args_is_none():
    # Setup: 'args' is None
    config = {
        "mcpServers": {
            "server1": {"args": None},
            "server2": {"args": ["foo", "http://sse.example.com"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_matching_sse_url_is_not_last_arg():
    # Setup: SSE URL appears but not as last arg
    config = {
        "mcpServers": {
            "server1": {"args": ["http://sse.example.com", "foo"]},
            "server2": {"args": ["foo", "bar", "baz"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_empty_sse_url():
    # Setup: sse_url is empty string
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", ""]},
            "server2": {"args": ["bar", "baz"]},
        }
    }
    sse_url = ""
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_config_data_is_empty_dict():
    # Setup: config_data is completely empty
    config = {}
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_args_is_list_of_one_item():
    # Setup: args is a single-item list
    config = {
        "mcpServers": {
            "server1": {"args": ["http://sse.example.com"]},
            "server2": {"args": ["foo"]},
        }
    }
    sse_url = "http://sse.example.com"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_server_config_is_empty_dict():
    # Setup: server config is an empty dict (no 'args')
    config = {
        "mcpServers": {
            "server1": {},
            "server2": {"args": ["foo", "bar"]},
        }
    }
    sse_url = "bar"
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

# ------------------ LARGE SCALE TEST CASES ------------------

def test_large_number_of_servers_with_sparse_matches():
    # Setup: 1000 servers, only 10 have matching SSE URL
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    # Add 990 servers with non-matching args
    for i in range(990):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", f"url{i}"]}
    # Add 10 servers with matching SSE URL as last arg
    for i in range(990, 1000):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", sse_url]}
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)
    for i in range(990, 1000):
        pass
    # The rest remain
    for i in range(990):
        pass

def test_large_number_of_servers_all_match():
    # Setup: 500 servers, all with matching SSE URL
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    for i in range(500):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", sse_url]}
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_large_number_of_servers_none_match():
    # Setup: 500 servers, none with matching SSE URL
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    for i in range(500):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", f"url{i}"]}
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)

def test_large_number_of_servers_some_with_missing_args():
    # Setup: 250 servers, half missing 'args', half with matching SSE URL
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    for i in range(125):
        config["mcpServers"][f"server{i}"] = {}
    for i in range(125, 250):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", sse_url]}
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)
    for i in range(125, 250):
        pass
    for i in range(125):
        pass

def test_large_number_of_servers_with_varied_args_types():
    # Setup: 100 servers, some with args as None, some as string, some as list with match
    config = {
        "mcpServers": {}
    }
    sse_url = "http://sse.example.com"
    for i in range(30):
        config["mcpServers"][f"server{i}"] = {"args": None}
    for i in range(30, 60):
        config["mcpServers"][f"server{i}"] = {"args": "notalist"}
    for i in range(60, 100):
        config["mcpServers"][f"server{i}"] = {"args": ["foo", sse_url]}
    config_copy = copy.deepcopy(config)
    updated, removed = remove_server_by_sse_url(config_copy, sse_url)
    for i in range(60, 100):
        pass
    for i in range(60):
        pass
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.

import pytest
from langflow.api.v1.mcp_projects import remove_server_by_sse_url
# function to test
from langflow.logging import logger

# unit tests

# -------------------- BASIC TEST CASES --------------------

def test_remove_single_server_match():
    # Test removing a single server whose args[-1] matches sse_url
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar", "http://example.com/sse"]},
            "server2": {"args": ["baz", "qux", "http://other.com/sse"]},
        }
    }
    sse_url = "http://example.com/sse"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_no_match():
    # Test when no server matches the sse_url
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar", "http://notmatching.com"]},
            "server2": {"args": ["baz", "qux", "http://other.com"]},
        }
    }
    sse_url = "http://example.com/sse"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_multiple_servers_match():
    # Test when multiple servers match the sse_url
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "http://match.com"]},
            "server2": {"args": ["baz", "http://match.com"]},
            "server3": {"args": ["baz", "http://other.com"]},
        }
    }
    sse_url = "http://match.com"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_args_empty():
    # Test when a server has an empty 'args' list
    config = {
        "mcpServers": {
            "server1": {"args": []},
            "server2": {"args": ["foo", "bar"]},
        }
    }
    sse_url = "bar"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_args_missing():
    # Test when a server does not have an 'args' key at all
    config = {
        "mcpServers": {
            "server1": {},
            "server2": {"args": ["foo", "bar"]},
        }
    }
    sse_url = "bar"
    updated, removed = remove_server_by_sse_url(config, sse_url)

# -------------------- EDGE TEST CASES --------------------

def test_remove_no_mcpServers_key():
    # Test when config_data has no 'mcpServers' key
    config = {"otherKey": 123}
    sse_url = "http://example.com/sse"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_empty_mcpServers():
    # Test when 'mcpServers' is present but empty
    config = {"mcpServers": {}}
    sse_url = "http://example.com/sse"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_args_not_list():
    # Test when 'args' is present but not a list (should treat as non-matching)
    config = {
        "mcpServers": {
            "server1": {"args": "notalist"},
            "server2": {"args": None},
        }
    }
    sse_url = "notalist"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_args_single_element():
    # Test when 'args' is a single-element list
    config = {
        "mcpServers": {
            "server1": {"args": ["http://foo.com"]},
            "server2": {"args": ["http://bar.com"]},
        }
    }
    sse_url = "http://foo.com"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_server_name_collision():
    # Test when server names are similar but not identical
    config = {
        "mcpServers": {
            "server": {"args": ["foo", "match"]},
            "server1": {"args": ["foo", "match"]},
            "server2": {"args": ["foo", "nomatch"]},
        }
    }
    sse_url = "match"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_mutates_input():
    # Test that the function mutates the input config_data in-place
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar"]},
            "server2": {"args": ["foo", "baz"]},
        }
    }
    sse_url = "bar"
    orig_id = id(config["mcpServers"])
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_with_non_string_args():
    # Test when 'args' contains non-string elements
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", 123]},
            "server2": {"args": ["foo", None]},
            "server3": {"args": ["foo", "bar"]},
        }
    }
    sse_url = 123
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_with_empty_string_url():
    # Test when sse_url is an empty string
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", ""]},
            "server2": {"args": ["foo", "bar"]},
        }
    }
    sse_url = ""
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_remove_with_duplicate_args():
    # Test when args contains duplicate values, but only last is checked
    config = {
        "mcpServers": {
            "server1": {"args": ["foo", "bar", "foo", "target"]},
            "server2": {"args": ["target", "foo", "bar"]},
        }
    }
    sse_url = "target"
    updated, removed = remove_server_by_sse_url(config, sse_url)

# -------------------- LARGE SCALE TEST CASES --------------------

def test_large_scale_removal():
    # Test performance and correctness with a large number of servers
    N = 1000
    config = {"mcpServers": {}}
    # Half match, half do not
    for i in range(N):
        if i % 2 == 0:
            args = ["foo", f"url_{i}", "target_url"]
        else:
            args = ["foo", f"url_{i}", "other_url"]
        config["mcpServers"][f"server_{i}"] = {"args": args}
    sse_url = "target_url"
    updated, removed = remove_server_by_sse_url(config, sse_url)
    # All even servers should be removed
    for i in range(N):
        name = f"server_{i}"
        if i % 2 == 0:
            pass
        else:
            pass

def test_large_scale_no_removal():
    # Test with a large number of servers, none matching
    N = 1000
    config = {"mcpServers": {f"server_{i}": {"args": ["foo", f"bar_{i}", "not_target"]} for i in range(N)}}
    sse_url = "target_url"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_large_scale_all_removal():
    # Test with a large number of servers, all matching
    N = 1000
    config = {"mcpServers": {f"server_{i}": {"args": ["foo", f"bar_{i}", "target_url"]} for i in range(N)}}
    sse_url = "target_url"
    updated, removed = remove_server_by_sse_url(config, sse_url)

def test_large_scale_some_missing_args():
    # Test with a large number of servers, some missing 'args'
    N = 1000
    config = {"mcpServers": {}}
    for i in range(N):
        if i % 3 == 0:
            config["mcpServers"][f"server_{i}"] = {}
        elif i % 3 == 1:
            config["mcpServers"][f"server_{i}"] = {"args": ["foo", "target_url"]}
        else:
            config["mcpServers"][f"server_{i}"] = {"args": ["foo", "other_url"]}
    sse_url = "target_url"
    updated, removed = remove_server_by_sse_url(config, sse_url)
    # Only servers with i % 3 == 1 should be removed
    for i in range(N):
        name = f"server_{i}"
        if i % 3 == 1:
            pass
        else:
            pass

def test_large_scale_edge_empty_args():
    # Test with a large number of servers, all with empty args
    N = 1000
    config = {"mcpServers": {f"server_{i}": {"args": []} for i in range(N)}}
    sse_url = "anything"
    updated, removed = remove_server_by_sse_url(config, sse_url)
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.

To test or edit this optimization locally git merge codeflash/optimize-pr9498-2025-08-22T23.17.13

Click to see suggested changes
Suggested change
msg = f"Could not determine Windows Claude config path in WSL: {e!s}"
raise ValueError(msg) from e
# Regular Windows
return Path(os.environ["APPDATA"]) / "Claude" / "claude_desktop_config.json"
msg = "Unsupported operating system for Claude configuration"
raise ValueError(msg)
msg = "Unsupported client"
raise ValueError(msg)
def remove_server_by_sse_url(config_data: dict, sse_url: str) -> tuple[dict, list[str]]:
"""Remove any MCP servers that use the specified SSE URL from config data.
Returns:
tuple: (updated_config, list_of_removed_server_names)
mcpServers = config_data.get("mcpServers")
if not mcpServers:
return config_data, []
removed_servers: list[str] = []
# Build a list of servers to remove using a more efficient comprehension
servers_to_remove = [
server_name
for server_name, server_config in mcpServers.items()
if (args := server_config.get("args")) and args[-1] == sse_url
]
# Avoid repeated dict lookups, and reduce list appends by using extend at the end
for server_name in servers_to_remove:
del mcpServers[server_name]
removed_servers.extend(servers_to_remove)
# Use a single batched logging call to minimize logger overhead (if servers were actually removed)
if servers_to_remove:
logger.debug(
"Removed existing servers with matching SSE URL: %s",
", ".join(servers_to_remove),
)

@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 22, 2025
@mfortman11 mfortman11 added this pull request to the merge queue Aug 24, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 24, 2025
@lucaseduoli lucaseduoli added this pull request to the merge queue Aug 25, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2025
@lucaseduoli lucaseduoli added this pull request to the merge queue Aug 25, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2025
@lucaseduoli lucaseduoli added this pull request to the merge queue Aug 25, 2025
Merged via the queue into main with commit 795c62a Aug 25, 2025
24 of 25 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request fast-track Skip tests and sends PR into the merge queue lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants