-
Notifications
You must be signed in to change notification settings - Fork 8.2k
fix: MCP server update to support partial updates (PATCH semantics) #10732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API as update_server()
participant Config as Existing Config
participant NewConfig as New Config
User->>API: Send update request
API->>Config: Retrieve existing server config
Config-->>API: Return existing config
rect rgb(240, 248, 255)
Note over API: New inheritance logic
alt existing.env exists AND new.env missing
API->>NewConfig: Copy env from existing
end
alt existing.args exists AND new.args missing
API->>NewConfig: Copy args from existing
end
end
API->>API: Assign updated config to server list
API->>API: Refresh and upload files
API-->>User: Return updated server
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings, 2 inconclusive)
✅ Passed checks (2 passed)
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. Comment |
…ix/mcp-keep-env-vars
|
Closing this in favor of #10749 |
Description
This PR fixes an issue where updating an MCP server configuration would unintentionally clear existing fields like
envandargsif they were not included in the request payload.Reasoning
The endpoint
@router.patch("/servers/{server_name}")implies a PATCH operation (partial update). However, the previous implementation performed a full replacement of the configuration object.This change modifies update_server to merge the new configuration with the existing one, preserving critical fields like
envandargswhen they are not explicitly provided in the update. This aligns the backend behavior with proper PATCH semantics.CI Fix
Fixed a bash syntax error in
typescript_test.ymlthat occurs when multiple test suites are selected (e.g.,@components|@api).|character which was interpreted by bash as a pipe operator because it was unquoted.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.