Skip to content

feat: enterprise host support #28

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

Merged
merged 4 commits into from
Mar 24, 2025
Merged

feat: enterprise host support #28

merged 4 commits into from
Mar 24, 2025

Conversation

SamMorrowDrums
Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums commented Mar 20, 2025

Adds support for a gh-host param so that enterprises work. Tested it out with failing attempt to call localhost:

go run cmd/github-mcp-server/main.go stdio --gh-host github.localhost

GitHub MCP Server running on stdio
{
  "jsonrpc": "2.0",
  "id": 3,
  "error": {
    "code": -32603,
    "message": "failed to get user: Get \"https://api.github.localhost/user\": dial tcp [::1]:443: connect: connection refused"
  }
}

So the override is working well.

@Copilot Copilot AI review requested due to automatic review settings March 20, 2025 16:06
Copy link
Contributor

@Copilot 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 PR adds rudimentary enterprise support by introducing a new "gh-host" parameter, allowing the GitHub MCP server to direct API calls to GitHub Enterprise endpoints.

  • Added the "gh-host" persistent flag.
  • Bound "gh-host" to viper.
  • Updated the GitHub client to use enterprise-specific base and upload URLs when provided.

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@SamMorrowDrums SamMorrowDrums changed the title Rudimentary enterprise support feat: enterprise host support Mar 20, 2025
@toby
Copy link
Member

toby commented Mar 20, 2025

Awesome! Do think it would make sense to support an env var (in addition to or instead of the flag)? Just because we're using them elsewhere.

Comment on lines 98 to 127
if host := viper.GetString("gh-host"); host != "" {
parsedURL, err := url.Parse(fmt.Sprintf("https://api.%s/", host))
if err != nil {
return fmt.Errorf("failed to parse provided GitHub host URL: %w", err)
}

uploadURL, err := url.Parse(fmt.Sprintf("https://uploads.%s/", host))
if err != nil {
return fmt.Errorf("failed to parse provided GitHub host URL: %w", err)
}

ghClient.BaseURL = parsedURL
ghClient.UploadURL = uploadURL
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can use the WithEnterpriseURLs() function in the gogithub.Client instead?

@SamMorrowDrums
Copy link
Collaborator Author

Awesome! Do think it would make sense to support an env var (in addition to or instead of the flag)? Just because we're using them elsewhere.

Yeah, that's easy. I was considering doing so too. Just didn't 😅

Copy link
Collaborator

@juruen juruen left a comment

Choose a reason for hiding this comment

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

🚀

@SamMorrowDrums SamMorrowDrums merged commit 647a5e5 into main Mar 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants