Skip to content

Split PR review creation, commenting, submission and deletion #410

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
658 changes: 633 additions & 25 deletions e2e/e2e_test.go

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
require (
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/fsnotify/fsnotify v1.8.0 // indirect
github.com/go-viper/mapstructure/v2 v2.2.1 // indirect
github.com/go-viper/mapstructure/v2 v2.2.1
github.com/google/go-github/v71 v71.0.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand All @@ -25,13 +25,16 @@ require (
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/rogpeppe/go-internal v1.13.1 // indirect
github.com/sagikazarmark/locafero v0.9.0 // indirect
github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466
github.com/sourcegraph/conc v0.3.0 // indirect
github.com/spf13/afero v1.14.0 // indirect
github.com/spf13/cast v1.7.1 // indirect
github.com/spf13/pflag v1.0.6 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
github.com/yosida95/uritemplate/v3 v3.0.2 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/oauth2 v0.29.0 // indirect
golang.org/x/sys v0.31.0 // indirect
golang.org/x/text v0.23.0 // indirect
golang.org/x/time v0.5.0 // indirect
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWN
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/sagikazarmark/locafero v0.9.0 h1:GbgQGNtTrEmddYDSAH9QLRyfAHY12md+8YFTqyMTC9k=
github.com/sagikazarmark/locafero v0.9.0/go.mod h1:UBUyz37V+EdMS3hDF3QWIiVr/2dPrx49OMO0Bn0hJqk=
github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7 h1:cYCy18SHPKRkvclm+pWm1Lk4YrREb4IOIb/YdFO0p2M=
github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8=
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0=
github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=
Expand All @@ -69,6 +73,8 @@ github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zI
github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4=
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98=
golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik=
golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
Expand Down
187 changes: 176 additions & 11 deletions internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import (
"fmt"
"io"
"log"
"net/http"
"net/url"
"os"
"os/signal"
"strings"
"syscall"

"github.com/github/github-mcp-server/pkg/github"
Expand All @@ -15,6 +18,7 @@ import (
gogithub "github.com/google/go-github/v69/github"
"github.com/mark3labs/mcp-go/mcp"
"github.com/mark3labs/mcp-go/server"
"github.com/shurcooL/githubv4"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -44,25 +48,43 @@ type MCPServerConfig struct {
}

func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
ghClient := gogithub.NewClient(nil).WithAuthToken(cfg.Token)
ghClient.UserAgent = fmt.Sprintf("github-mcp-server/%s", cfg.Version)

if cfg.Host != "" {
var err error
ghClient, err = ghClient.WithEnterpriseURLs(cfg.Host, cfg.Host)
if err != nil {
return nil, fmt.Errorf("failed to create GitHub client with host: %w", err)
}
apiHost, err := parseAPIHost(cfg.Host)
if err != nil {
return nil, fmt.Errorf("failed to parse API host: %w", err)
}

// Construct our REST client
restClient := gogithub.NewClient(nil).WithAuthToken(cfg.Token)
restClient.UserAgent = fmt.Sprintf("github-mcp-server/%s", cfg.Version)
restClient.BaseURL = apiHost.baseRESTURL
Copy link
Collaborator Author

@williammartin williammartin May 19, 2025

Choose a reason for hiding this comment

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

I chose to go away from WithEnterpriseURLS because there are hosts that are not enterprise but require different URLs e.g. review labs or development environments.

Sadly, githubv4 doesn't offer quite the same options, so I commented below why that is. It also doesn't export the internal graphql.Client so we can't just set it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a shame! Might need to propose exporting it.

restClient.UploadURL = apiHost.uploadURL

// Construct our GraphQL client
// We're using NewEnterpriseClient here unconditionally as opposed to NewClient because we already
// did the necessary API host parsing so that github.com will return the correct URL anyway.
gqlHTTPClient := &http.Client{
Transport: &bearerAuthTransport{
transport: http.DefaultTransport,
token: cfg.Token,
},
} // We're going to wrap the Transport later in beforeInit
gqlClient := githubv4.NewEnterpriseClient(apiHost.graphqlURL.String(), gqlHTTPClient)

// When a client send an initialize request, update the user agent to include the client info.
beforeInit := func(_ context.Context, _ any, message *mcp.InitializeRequest) {
ghClient.UserAgent = fmt.Sprintf(
userAgent := fmt.Sprintf(
"github-mcp-server/%s (%s/%s)",
cfg.Version,
message.Params.ClientInfo.Name,
message.Params.ClientInfo.Version,
)

restClient.UserAgent = userAgent

gqlHTTPClient.Transport = &userAgentTransport{
transport: gqlHTTPClient.Transport,
agent: userAgent,
}
}

hooks := &server.Hooks{
Expand All @@ -83,14 +105,19 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
}

getClient := func(_ context.Context) (*gogithub.Client, error) {
return ghClient, nil // closing over client
return restClient, nil // closing over client
}

getGQLClient := func(_ context.Context) (*githubv4.Client, error) {
return gqlClient, nil // closing over client
}

// Create default toolsets
toolsets, err := github.InitToolsets(
enabledToolsets,
cfg.ReadOnly,
getClient,
getGQLClient,
cfg.Translator,
)
if err != nil {
Expand Down Expand Up @@ -213,3 +240,141 @@ func RunStdioServer(cfg StdioServerConfig) error {

return nil
}

type apiHost struct {
baseRESTURL *url.URL
graphqlURL *url.URL
uploadURL *url.URL
}

func newDotcomHost() (apiHost, error) {
baseRestURL, err := url.Parse("https://api.github.com/")
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse dotcom REST URL: %w", err)
}

gqlURL, err := url.Parse("https://api.github.com/graphql")
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse dotcom GraphQL URL: %w", err)
}

uploadURL, err := url.Parse("https://uploads.github.com")
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse dotcom Upload URL: %w", err)
}

return apiHost{
baseRESTURL: baseRestURL,
graphqlURL: gqlURL,
uploadURL: uploadURL,
}, nil
}

func newGHECHost(hostname string) (apiHost, error) {
u, err := url.Parse(hostname)
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHEC URL: %w", err)
}

// Unsecured GHEC would be an error
if u.Scheme == "http" {
return apiHost{}, fmt.Errorf("GHEC URL must be HTTPS")
}

restURL, err := url.Parse(fmt.Sprintf("https://api.%s/", u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHEC REST URL: %w", err)
}

gqlURL, err := url.Parse(fmt.Sprintf("https://api.%s/graphql", u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHEC GraphQL URL: %w", err)
}

uploadURL, err := url.Parse(fmt.Sprintf("https://uploads.%s", u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHEC Upload URL: %w", err)
}

return apiHost{
baseRESTURL: restURL,
graphqlURL: gqlURL,
uploadURL: uploadURL,
}, nil
}

func newGHESHost(hostname string) (apiHost, error) {
u, err := url.Parse(hostname)
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHES URL: %w", err)
}

restURL, err := url.Parse(fmt.Sprintf("%s://%s/api/v3/", u.Scheme, u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHES REST URL: %w", err)
}

gqlURL, err := url.Parse(fmt.Sprintf("%s://%s/api/graphql", u.Scheme, u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHES GraphQL URL: %w", err)
}

uploadURL, err := url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, u.Hostname()))
if err != nil {
return apiHost{}, fmt.Errorf("failed to parse GHES Upload URL: %w", err)
}

return apiHost{
baseRESTURL: restURL,
graphqlURL: gqlURL,
uploadURL: uploadURL,
}, nil
}

// Note that this does not handle ports yet, so development environments are out.
func parseAPIHost(s string) (apiHost, error) {
if s == "" {
return newDotcomHost()
}

u, err := url.Parse(s)
if err != nil {
return apiHost{}, fmt.Errorf("could not parse host as URL: %s", s)
}

if u.Scheme == "" {
return apiHost{}, fmt.Errorf("host must have a scheme (http or https): %s", s)
}

if strings.HasSuffix(u.Hostname(), "github.com") {
return newDotcomHost()
}

if strings.HasSuffix(u.Hostname(), "ghe.com") {
return newGHECHost(s)
}

return newGHESHost(s)
}

type userAgentTransport struct {
transport http.RoundTripper
agent string
}

func (t *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) {
req = req.Clone(req.Context())
req.Header.Set("User-Agent", t.agent)
return t.transport.RoundTrip(req)
}

type bearerAuthTransport struct {
transport http.RoundTripper
token string
}

func (t *bearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
req = req.Clone(req.Context())
req.Header.Set("Authorization", "Bearer "+t.token)
return t.transport.RoundTrip(req)
}
Loading