Merge branch 'main' into fix-mcp-apps-update-before-submit

Author: mattdholloway

README.md

@@ -153,7 +153,7 @@ Example for `https://octocorp.ghe.com` with GitHub PAT token:
 ```
 {
     ...
-    "proxima-github": {
+    "github-octocorp": {
       "type": "http",
       "url": "https://copilot-api.octocorp.ghe.com/mcp",
       "headers": {

pkg/github/issues.go

@@ -688,7 +688,12 @@ func AddIssueComment(t translations.TranslationHelperFunc) inventory.ServerTool
 				return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to create comment", resp, body), nil, nil
 			}
 
-			r, err := json.Marshal(createdComment)
+			minimalResponse := MinimalResponse{
+				ID:  fmt.Sprintf("%d", createdComment.GetID()),
+				URL: createdComment.GetHTMLURL(),
+			}
+
+			r, err := json.Marshal(minimalResponse)
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
 			}

pkg/github/issues_test.go

@@ -458,13 +458,12 @@ func Test_AddIssueComment(t *testing.T) {
 			// Parse the result and get the text content if no error
 			textContent := getTextResult(t, result)
 
-			// Unmarshal and verify the result
-			var returnedComment github.IssueComment
-			err = json.Unmarshal([]byte(textContent.Text), &returnedComment)
+			// Unmarshal and verify the result contains minimal response
+			var minimalResponse MinimalResponse
+			err = json.Unmarshal([]byte(textContent.Text), &minimalResponse)
 			require.NoError(t, err)
-			assert.Equal(t, *tc.expectedComment.ID, *returnedComment.ID)
-			assert.Equal(t, *tc.expectedComment.Body, *returnedComment.Body)
-			assert.Equal(t, *tc.expectedComment.User.Login, *returnedComment.User.Login)
+			assert.Equal(t, fmt.Sprintf("%d", tc.expectedComment.GetID()), minimalResponse.ID)
+			assert.Equal(t, tc.expectedComment.GetHTMLURL(), minimalResponse.URL)
 
 		})
 	}

pkg/github/minimal_types.go

@@ -612,6 +612,41 @@ func convertToMinimalCommit(commit *github.RepositoryCommit, includeDiffs bool)
 	return minimalCommit
 }
 
+// MinimalPageInfo contains pagination cursor information.
+type MinimalPageInfo struct {
+	HasNextPage     bool   `json:"has_next_page"`
+	HasPreviousPage bool   `json:"has_previous_page"`
+	StartCursor     string `json:"start_cursor,omitempty"`
+	EndCursor       string `json:"end_cursor,omitempty"`
+}
+
+// MinimalReviewComment is the trimmed output type for PR review comment objects.
+type MinimalReviewComment struct {
+	Body      string `json:"body,omitempty"`
+	Path      string `json:"path"`
+	Line      *int   `json:"line,omitempty"`
+	Author    string `json:"author,omitempty"`
+	CreatedAt string `json:"created_at,omitempty"`
+	UpdatedAt string `json:"updated_at,omitempty"`
+	HTMLURL   string `json:"html_url"`
+}
+
+// MinimalReviewThread is the trimmed output type for PR review thread objects.
+type MinimalReviewThread struct {
+	IsResolved  bool                   `json:"is_resolved"`
+	IsOutdated  bool                   `json:"is_outdated"`
+	IsCollapsed bool                   `json:"is_collapsed"`
+	Comments    []MinimalReviewComment `json:"comments"`
+	TotalCount  int                    `json:"total_count"`
+}
+
+// MinimalReviewThreadsResponse is the trimmed output for a paginated list of PR review threads.
+type MinimalReviewThreadsResponse struct {
+	ReviewThreads []MinimalReviewThread `json:"review_threads"`
+	TotalCount    int                   `json:"total_count"`
+	PageInfo      MinimalPageInfo       `json:"page_info"`
+}
+
 func convertToMinimalPRFiles(files []*github.CommitFile) []MinimalPRFile {
 	result := make([]MinimalPRFile, 0, len(files))
 	for _, f := range files {
@@ -636,3 +671,61 @@ func convertToMinimalBranch(branch *github.Branch) MinimalBranch {
 		Protected: branch.GetProtected(),
 	}
 }
+
+func convertToMinimalReviewThreadsResponse(query reviewThreadsQuery) MinimalReviewThreadsResponse {
+	threads := query.Repository.PullRequest.ReviewThreads
+
+	minimalThreads := make([]MinimalReviewThread, 0, len(threads.Nodes))
+	for _, thread := range threads.Nodes {
+		minimalThreads = append(minimalThreads, convertToMinimalReviewThread(thread))
+	}
+
+	return MinimalReviewThreadsResponse{
+		ReviewThreads: minimalThreads,
+		TotalCount:    int(threads.TotalCount),
+		PageInfo: MinimalPageInfo{
+			HasNextPage:     bool(threads.PageInfo.HasNextPage),
+			HasPreviousPage: bool(threads.PageInfo.HasPreviousPage),
+			StartCursor:     string(threads.PageInfo.StartCursor),
+			EndCursor:       string(threads.PageInfo.EndCursor),
+		},
+	}
+}
+
+func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread {
+	comments := make([]MinimalReviewComment, 0, len(thread.Comments.Nodes))
+	for _, c := range thread.Comments.Nodes {
+		comments = append(comments, convertToMinimalReviewComment(c))
+	}
+
+	return MinimalReviewThread{
+		IsResolved:  bool(thread.IsResolved),
+		IsOutdated:  bool(thread.IsOutdated),
+		IsCollapsed: bool(thread.IsCollapsed),
+		Comments:    comments,
+		TotalCount:  int(thread.Comments.TotalCount),
+	}
+}
+
+func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment {
+	m := MinimalReviewComment{
+		Body:    string(c.Body),
+		Path:    string(c.Path),
+		Author:  string(c.Author.Login),
+		HTMLURL: c.URL.String(),
+	}
+
+	if c.Line != nil {
+		line := int(*c.Line)
+		m.Line = &line
+	}
+
+	if !c.CreatedAt.IsZero() {
+		m.CreatedAt = c.CreatedAt.Format(time.RFC3339)
+	}
+	if !c.UpdatedAt.IsZero() {
+		m.UpdatedAt = c.UpdatedAt.Format(time.RFC3339)
+	}
+
+	return m
+}

pkg/github/pullrequests.go

@@ -406,24 +406,7 @@ func GetPullRequestReviewComments(ctx context.Context, gqlClient *githubv4.Clien
 		}
 	}
 
-	// Build response with review threads and pagination info
-	response := map[string]any{
-		"reviewThreads": query.Repository.PullRequest.ReviewThreads.Nodes,
-		"pageInfo": map[string]any{
-			"hasNextPage":     query.Repository.PullRequest.ReviewThreads.PageInfo.HasNextPage,
-			"hasPreviousPage": query.Repository.PullRequest.ReviewThreads.PageInfo.HasPreviousPage,
-			"startCursor":     string(query.Repository.PullRequest.ReviewThreads.PageInfo.StartCursor),
-			"endCursor":       string(query.Repository.PullRequest.ReviewThreads.PageInfo.EndCursor),
-		},
-		"totalCount": int(query.Repository.PullRequest.ReviewThreads.TotalCount),
-	}
-
-	r, err := json.Marshal(response)
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal response: %w", err)
-	}
-
-	return utils.NewToolResultText(string(r)), nil
+	return MarshalledTextResult(convertToMinimalReviewThreadsResponse(query)), nil
 }
 
 func GetPullRequestReviews(ctx context.Context, client *github.Client, deps ToolDependencies, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) {

pkg/github/pullrequests_test.go

@@ -1619,45 +1619,35 @@ func Test_GetPullRequestComments(t *testing.T) {
 			},
 			expectError: false,
 			validateResult: func(t *testing.T, textContent string) {
-				var result map[string]any
+				var result MinimalReviewThreadsResponse
 				err := json.Unmarshal([]byte(textContent), &result)
 				require.NoError(t, err)
 
-				// Validate response structure
-				assert.Contains(t, result, "reviewThreads")
-				assert.Contains(t, result, "pageInfo")
-				assert.Contains(t, result, "totalCount")
-
 				// Validate review threads
-				threads := result["reviewThreads"].([]any)
-				assert.Len(t, threads, 1)
+				assert.Len(t, result.ReviewThreads, 1)
 
-				thread := threads[0].(map[string]any)
-				assert.Equal(t, "RT_kwDOA0xdyM4AX1Yz", thread["ID"])
-				assert.Equal(t, false, thread["IsResolved"])
-				assert.Equal(t, false, thread["IsOutdated"])
-				assert.Equal(t, false, thread["IsCollapsed"])
+				thread := result.ReviewThreads[0]
+				assert.Equal(t, false, thread.IsResolved)
+				assert.Equal(t, false, thread.IsOutdated)
+				assert.Equal(t, false, thread.IsCollapsed)
 
 				// Validate comments within thread
-				comments := thread["Comments"].(map[string]any)
-				commentNodes := comments["Nodes"].([]any)
-				assert.Len(t, commentNodes, 2)
+				assert.Len(t, thread.Comments, 2)
 
 				// Validate first comment
-				comment1 := commentNodes[0].(map[string]any)
-				assert.Equal(t, "PRRC_kwDOA0xdyM4AX1Y0", comment1["ID"])
-				assert.Equal(t, "This looks good", comment1["Body"])
-				assert.Equal(t, "file1.go", comment1["Path"])
+				comment1 := thread.Comments[0]
+				assert.Equal(t, "This looks good", comment1.Body)
+				assert.Equal(t, "file1.go", comment1.Path)
+				assert.Equal(t, "reviewer1", comment1.Author)
 
 				// Validate pagination info
-				pageInfo := result["pageInfo"].(map[string]any)
-				assert.Equal(t, false, pageInfo["hasNextPage"])
-				assert.Equal(t, false, pageInfo["hasPreviousPage"])
-				assert.Equal(t, "cursor1", pageInfo["startCursor"])
-				assert.Equal(t, "cursor2", pageInfo["endCursor"])
+				assert.Equal(t, false, result.PageInfo.HasNextPage)
+				assert.Equal(t, false, result.PageInfo.HasPreviousPage)
+				assert.Equal(t, "cursor1", result.PageInfo.StartCursor)
+				assert.Equal(t, "cursor2", result.PageInfo.EndCursor)
 
 				// Validate total count
-				assert.Equal(t, float64(1), result["totalCount"])
+				assert.Equal(t, 1, result.TotalCount)
 			},
 		},
 		{
@@ -1761,27 +1751,22 @@ func Test_GetPullRequestComments(t *testing.T) {
 			expectError:     false,
 			lockdownEnabled: true,
 			validateResult: func(t *testing.T, textContent string) {
-				var result map[string]any
+				var result MinimalReviewThreadsResponse
 				err := json.Unmarshal([]byte(textContent), &result)
 				require.NoError(t, err)
 
 				// Validate that only maintainer comment is returned
-				threads := result["reviewThreads"].([]any)
-				assert.Len(t, threads, 1)
+				assert.Len(t, result.ReviewThreads, 1)
 
-				thread := threads[0].(map[string]any)
-				comments := thread["Comments"].(map[string]any)
+				thread := result.ReviewThreads[0]
 
 				// Should only have 1 comment (maintainer) after filtering
-				assert.Equal(t, float64(1), comments["TotalCount"])
-
-				commentNodes := comments["Nodes"].([]any)
-				assert.Len(t, commentNodes, 1)
+				assert.Equal(t, 1, thread.TotalCount)
+				assert.Len(t, thread.Comments, 1)
 
-				comment := commentNodes[0].(map[string]any)
-				author := comment["Author"].(map[string]any)
-				assert.Equal(t, "maintainer", author["Login"])
-				assert.Equal(t, "Maintainer review comment", comment["Body"])
+				comment := thread.Comments[0]
+				assert.Equal(t, "maintainer", comment.Author)
+				assert.Equal(t, "Maintainer review comment", comment.Body)
 			},
 		},
 	}

pkg/http/oauth/oauth.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/github/github-mcp-server/pkg/http/headers"
+	"github.com/github/github-mcp-server/pkg/utils"
 	"github.com/go-chi/chi/v5"
 	"github.com/modelcontextprotocol/go-sdk/auth"
 	"github.com/modelcontextprotocol/go-sdk/oauthex"
@@ -16,9 +17,6 @@ import (
 const (
 	// OAuthProtectedResourcePrefix is the well-known path prefix for OAuth protected resource metadata.
 	OAuthProtectedResourcePrefix = "/.well-known/oauth-protected-resource"
-
-	// DefaultAuthorizationServer is GitHub's OAuth authorization server.
-	DefaultAuthorizationServer = "https://github.com/login/oauth"
 )
 
 // SupportedScopes lists all OAuth scopes that may be required by MCP tools.
@@ -55,22 +53,27 @@ type Config struct {
 
 // AuthHandler handles OAuth-related HTTP endpoints.
 type AuthHandler struct {
-	cfg *Config
+	cfg     *Config
+	apiHost utils.APIHostResolver
 }
 
 // NewAuthHandler creates a new OAuth auth handler.
-func NewAuthHandler(cfg *Config) (*AuthHandler, error) {
+func NewAuthHandler(cfg *Config, apiHost utils.APIHostResolver) (*AuthHandler, error) {
 	if cfg == nil {
 		cfg = &Config{}
 	}
 
-	// Default authorization server to GitHub
-	if cfg.AuthorizationServer == "" {
-		cfg.AuthorizationServer = DefaultAuthorizationServer
+	if apiHost == nil {
+		var err error
+		apiHost, err = utils.NewAPIHost("https://api.github.com")
+		if err != nil {
+			return nil, fmt.Errorf("failed to create default API host: %w", err)
+		}
 	}
 
 	return &AuthHandler{
-		cfg: cfg,
+		cfg:     cfg,
+		apiHost: apiHost,
 	}, nil
 }
 
@@ -95,15 +98,28 @@ func (h *AuthHandler) RegisterRoutes(r chi.Router) {
 
 func (h *AuthHandler) metadataHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		ctx := r.Context()
 		resourcePath := resolveResourcePath(
 			strings.TrimPrefix(r.URL.Path, OAuthProtectedResourcePrefix),
 			h.cfg.ResourcePath,
 		)
 		resourceURL := h.buildResourceURL(r, resourcePath)
 
+		var authorizationServerURL string
+		if h.cfg.AuthorizationServer != "" {
+			authorizationServerURL = h.cfg.AuthorizationServer
+		} else {
+			authURL, err := h.apiHost.AuthorizationServerURL(ctx)
+			if err != nil {
+				http.Error(w, fmt.Sprintf("failed to resolve authorization server URL: %v", err), http.StatusInternalServerError)
+				return
+			}
+			authorizationServerURL = authURL.String()
+		}
+
 		metadata := &oauthex.ProtectedResourceMetadata{
 			Resource:               resourceURL,
-			AuthorizationServers:   []string{h.cfg.AuthorizationServer},
+			AuthorizationServers:   []string{authorizationServerURL},
 			ResourceName:           "GitHub MCP Server",
 			ScopesSupported:        SupportedScopes,
 			BearerMethodsSupported: []string{"header"},