- 
                Notifications
    You must be signed in to change notification settings 
- Fork 83
Support composite MCP servers #4560
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
| I changed the issue to #4635 | 
a1b63cd    to
    a3e6c99      
    Compare
  
    | if err != nil { | ||
| if req.Context().Err() != nil { | ||
| return "", fmt.Errorf("failed to check component server OAuth: %w", req.Context().Err()) | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to continue if err != nil here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled components are already skipped in the code above, so I'm on the fence about whether to continue here. If we return the error here, at least it can bubble up to the user.
5c0db82    to
    ec283cb      
    Compare
  
    Signed-off-by: Nick Hale <[email protected]>
Signed-off-by: Nick Hale <[email protected]>
        
          
                pkg/mcp/types.go
              
                Outdated
          
        
      | if !strings.HasPrefix(scope, mcpServerName+"-") { | ||
| finalScope = fmt.Sprintf("%s-%s", mcpServerName, scope) | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the scope should be what the scope is at this point. We don't need to modify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Signed-off-by: Nick Hale <[email protected]>
This change introduces "Composite" MCP Servers and support for:
This change does not include support for:
Support for these will be included in follow up PRs.
Addresses part of #3553