From efe9d40b58f1c91173d0fef01175047b2dc1e747 Mon Sep 17 00:00:00 2001 From: Adam Holt Date: Mon, 16 Feb 2026 14:10:28 +0100 Subject: [PATCH 1/4] Token scopes context (#1997) * Move scope storage into its own context key, separately from token info. This allows us to provide scopes seperately in the remote server, where we have scopes before we do the auth. * Skip token extraction if token info already exists in context. This is to avoid redundant token extraction in remote setup where token info may have already been extracted earlier in the request lifecycle. * Check for existing scopes in context before fetching from GitHub API in scope challenge middleware * Return error type for unknown tools in inventory builder and handle it in HTTP handler --- pkg/context/token.go | 30 +++++++++++++++++--------- pkg/http/handler.go | 15 +++++++++++-- pkg/http/middleware/pat_scope.go | 12 +++++++---- pkg/http/middleware/pat_scope_test.go | 19 +++++++++------- pkg/http/middleware/scope_challenge.go | 18 +++++++++------- pkg/http/middleware/token.go | 11 +++++++++- pkg/inventory/builder.go | 8 ++++++- 7 files changed, 79 insertions(+), 34 deletions(-) diff --git a/pkg/context/token.go b/pkg/context/token.go index beddb02b..97091a92 100644 --- a/pkg/context/token.go +++ b/pkg/context/token.go @@ -6,27 +6,37 @@ import ( "github.com/github/github-mcp-server/pkg/utils" ) -// tokenCtxKey is a context key for authentication token information -type tokenCtx string - -var tokenCtxKey tokenCtx = "tokenctx" +type tokenCtxKey struct{} type TokenInfo struct { - Token string - TokenType utils.TokenType - ScopesFetched bool - Scopes []string + Token string + TokenType utils.TokenType } // WithTokenInfo adds TokenInfo to the context func WithTokenInfo(ctx context.Context, tokenInfo *TokenInfo) context.Context { - return context.WithValue(ctx, tokenCtxKey, tokenInfo) + return context.WithValue(ctx, tokenCtxKey{}, tokenInfo) } // GetTokenInfo retrieves the authentication token from the context func GetTokenInfo(ctx context.Context) (*TokenInfo, bool) { - if tokenInfo, ok := ctx.Value(tokenCtxKey).(*TokenInfo); ok { + if tokenInfo, ok := ctx.Value(tokenCtxKey{}).(*TokenInfo); ok { return tokenInfo, true } return nil, false } + +type tokenScopesKey struct{} + +// WithTokenScopes adds token scopes to the context +func WithTokenScopes(ctx context.Context, scopes []string) context.Context { + return context.WithValue(ctx, tokenScopesKey{}, scopes) +} + +// GetTokenScopes retrieves token scopes from the context +func GetTokenScopes(ctx context.Context) ([]string, bool) { + if scopes, ok := ctx.Value(tokenScopesKey{}).([]string); ok { + return scopes, true + } + return nil, false +} diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 875d54bb..3c6c5302 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -2,6 +2,7 @@ package http import ( "context" + "errors" "log/slog" "net/http" @@ -178,6 +179,14 @@ func withInsiders(next http.Handler) http.Handler { func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { inv, err := h.inventoryFactoryFunc(r) if err != nil { + if errors.Is(err, inventory.ErrUnknownTools) { + w.WriteHeader(http.StatusBadRequest) + if _, writeErr := w.Write([]byte(err.Error())); writeErr != nil { + h.logger.Error("failed to write response", "error", writeErr) + } + return + } + w.WriteHeader(http.StatusInternalServerError) return } @@ -278,8 +287,10 @@ func PATScopeFilter(b *inventory.Builder, r *http.Request, fetcher scopes.Fetche // Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header. // Fine-grained PATs and other token types don't support this, so we skip filtering. if tokenInfo.TokenType == utils.TokenTypePersonalAccessToken { - if tokenInfo.ScopesFetched { - return b.WithFilter(github.CreateToolScopeFilter(tokenInfo.Scopes)) + // Check if scopes are already in context (should be set by WithPATScopes). If not, fetch them. + existingScopes, ok := ghcontext.GetTokenScopes(ctx) + if ok { + return b.WithFilter(github.CreateToolScopeFilter(existingScopes)) } scopesList, err := fetcher.FetchTokenScopes(ctx, tokenInfo.Token) diff --git a/pkg/http/middleware/pat_scope.go b/pkg/http/middleware/pat_scope.go index 8b77b3d3..bb1efdc0 100644 --- a/pkg/http/middleware/pat_scope.go +++ b/pkg/http/middleware/pat_scope.go @@ -26,6 +26,13 @@ func WithPATScopes(logger *slog.Logger, scopeFetcher scopes.FetcherInterface) fu // Only classic PATs (ghp_ prefix) return OAuth scopes via X-OAuth-Scopes header. // Fine-grained PATs and other token types don't support this, so we skip filtering. if tokenInfo.TokenType == utils.TokenTypePersonalAccessToken { + existingScopes, ok := ghcontext.GetTokenScopes(ctx) + if ok { + logger.Debug("using existing scopes from context", "scopes", existingScopes) + next.ServeHTTP(w, r) + return + } + scopesList, err := scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) if err != nil { logger.Warn("failed to fetch PAT scopes", "error", err) @@ -33,11 +40,8 @@ func WithPATScopes(logger *slog.Logger, scopeFetcher scopes.FetcherInterface) fu return } - tokenInfo.Scopes = scopesList - tokenInfo.ScopesFetched = true - // Store fetched scopes in context for downstream use - ctx := ghcontext.WithTokenInfo(ctx, tokenInfo) + ctx = ghcontext.WithTokenScopes(ctx, scopesList) next.ServeHTTP(w, r.WithContext(ctx)) return diff --git a/pkg/http/middleware/pat_scope_test.go b/pkg/http/middleware/pat_scope_test.go index eb472bcf..0607b8cf 100644 --- a/pkg/http/middleware/pat_scope_test.go +++ b/pkg/http/middleware/pat_scope_test.go @@ -111,12 +111,13 @@ func TestWithPATScopes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var capturedTokenInfo *ghcontext.TokenInfo + var capturedScopes []string + var scopesFound bool var nextHandlerCalled bool nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { nextHandlerCalled = true - capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + capturedScopes, scopesFound = ghcontext.GetTokenScopes(r.Context()) w.WriteHeader(http.StatusOK) }) @@ -141,10 +142,9 @@ func TestWithPATScopes(t *testing.T) { assert.Equal(t, tt.expectNextHandlerCalled, nextHandlerCalled, "next handler called mismatch") - if tt.expectNextHandlerCalled && tt.tokenInfo != nil { - require.NotNil(t, capturedTokenInfo, "expected token info in context") - assert.Equal(t, tt.expectScopesFetched, capturedTokenInfo.ScopesFetched) - assert.Equal(t, tt.expectedScopes, capturedTokenInfo.Scopes) + if tt.expectNextHandlerCalled { + assert.Equal(t, tt.expectScopesFetched, scopesFound, "scopes found mismatch") + assert.Equal(t, tt.expectedScopes, capturedScopes) } }) } @@ -154,9 +154,12 @@ func TestWithPATScopes_PreservesExistingTokenInfo(t *testing.T) { logger := slog.Default() var capturedTokenInfo *ghcontext.TokenInfo + var capturedScopes []string + var scopesFound bool nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { capturedTokenInfo, _ = ghcontext.GetTokenInfo(r.Context()) + capturedScopes, scopesFound = ghcontext.GetTokenScopes(r.Context()) w.WriteHeader(http.StatusOK) }) @@ -182,6 +185,6 @@ func TestWithPATScopes_PreservesExistingTokenInfo(t *testing.T) { require.NotNil(t, capturedTokenInfo) assert.Equal(t, originalTokenInfo.Token, capturedTokenInfo.Token) assert.Equal(t, originalTokenInfo.TokenType, capturedTokenInfo.TokenType) - assert.True(t, capturedTokenInfo.ScopesFetched) - assert.Equal(t, []string{"repo", "user"}, capturedTokenInfo.Scopes) + assert.True(t, scopesFound) + assert.Equal(t, []string{"repo", "user"}, capturedScopes) } diff --git a/pkg/http/middleware/scope_challenge.go b/pkg/http/middleware/scope_challenge.go index 52679724..1a86bf93 100644 --- a/pkg/http/middleware/scope_challenge.go +++ b/pkg/http/middleware/scope_challenge.go @@ -94,17 +94,19 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter return } - // Get OAuth scopes from GitHub API - activeScopes, err := scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) - if err != nil { - next.ServeHTTP(w, r) - return + // Get OAuth scopes for Token. First check if scopes are already in context, then fetch from GitHub if not present. + // This allows Remote Server to pass scope info to avoid redundant GitHub API calls. + activeScopes, ok := ghcontext.GetTokenScopes(ctx) + if !ok || (len(activeScopes) == 0 && tokenInfo.Token != "") { + activeScopes, err = scopeFetcher.FetchTokenScopes(ctx, tokenInfo.Token) + if err != nil { + next.ServeHTTP(w, r) + return + } } // Store active scopes in context for downstream use - tokenInfo.Scopes = activeScopes - tokenInfo.ScopesFetched = true - ctx = ghcontext.WithTokenInfo(ctx, tokenInfo) + ctx = ghcontext.WithTokenScopes(ctx, activeScopes) r = r.WithContext(ctx) // Check if user has the required scopes diff --git a/pkg/http/middleware/token.go b/pkg/http/middleware/token.go index c362ea20..012bbabe 100644 --- a/pkg/http/middleware/token.go +++ b/pkg/http/middleware/token.go @@ -13,6 +13,16 @@ import ( func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Check if token info already exists in context, if it does, skip extraction. + // In remote setup, we may have already extracted token info earlier. + if _, ok := ghcontext.GetTokenInfo(ctx); ok { + // Token info already exists in context, skip extraction + next.ServeHTTP(w, r) + return + } + tokenType, token, err := utils.ParseAuthorizationHeader(r) if err != nil { // For missing Authorization header, return 401 with WWW-Authenticate header per MCP spec @@ -25,7 +35,6 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl return } - ctx := r.Context() ctx = ghcontext.WithTokenInfo(ctx, &ghcontext.TokenInfo{ Token: token, TokenType: tokenType, diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index 35ccd593..6d2f080a 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -2,12 +2,18 @@ package inventory import ( "context" + "errors" "fmt" "maps" "slices" "strings" ) +var ( + // ErrUnknownTools is returned when tools specified via WithTools() are not recognized. + ErrUnknownTools = errors.New("unknown tools specified in WithTools") +) + // ToolFilter is a function that determines if a tool should be included. // Returns true if the tool should be included, false to exclude it. type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error) @@ -219,7 +225,7 @@ func (b *Builder) Build() (*Inventory, error) { // Error out if there are unrecognized tools if len(unrecognizedTools) > 0 { - return nil, fmt.Errorf("unrecognized tools: %s", strings.Join(unrecognizedTools, ", ")) + return nil, fmt.Errorf("%w: %s", ErrUnknownTools, strings.Join(unrecognizedTools, ", ")) } } From f04c137bdd1654980ffc2f86ad3e77f569ef0224 Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Tue, 17 Feb 2026 09:37:59 +0000 Subject: [PATCH 2/4] Reduce context usage for getting a Pull Request (#2017) * introduce minimal pr type * update to use time.RFC3339 * confine change to single PR --- pkg/github/minimal_types.go | 132 ++++++++++++++++++++++++++++++++ pkg/github/pullrequests.go | 7 +- pkg/github/pullrequests_test.go | 12 +-- 3 files changed, 140 insertions(+), 11 deletions(-) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index a33bcec7..f1dcfe06 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -1,6 +1,8 @@ package github import ( + "time" + "github.com/google/go-github/v82/github" ) @@ -134,8 +136,138 @@ type MinimalProject struct { OwnerType string `json:"owner_type,omitempty"` } +// MinimalPullRequest is the trimmed output type for pull request objects to reduce verbosity. +type MinimalPullRequest struct { + Number int `json:"number"` + Title string `json:"title"` + Body string `json:"body,omitempty"` + State string `json:"state"` + Draft bool `json:"draft"` + Merged bool `json:"merged"` + MergeableState string `json:"mergeable_state,omitempty"` + HTMLURL string `json:"html_url"` + User *MinimalUser `json:"user,omitempty"` + Labels []string `json:"labels,omitempty"` + Assignees []string `json:"assignees,omitempty"` + RequestedReviewers []string `json:"requested_reviewers,omitempty"` + MergedBy string `json:"merged_by,omitempty"` + Head *MinimalPRBranch `json:"head,omitempty"` + Base *MinimalPRBranch `json:"base,omitempty"` + Additions int `json:"additions,omitempty"` + Deletions int `json:"deletions,omitempty"` + ChangedFiles int `json:"changed_files,omitempty"` + Commits int `json:"commits,omitempty"` + Comments int `json:"comments,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + ClosedAt string `json:"closed_at,omitempty"` + MergedAt string `json:"merged_at,omitempty"` + Milestone string `json:"milestone,omitempty"` +} + +// MinimalPRBranch is the trimmed output type for pull request branch references. +type MinimalPRBranch struct { + Ref string `json:"ref"` + SHA string `json:"sha"` + Repo *MinimalPRBranchRepo `json:"repo,omitempty"` +} + +// MinimalPRBranchRepo is the trimmed repo info nested inside a PR branch. +type MinimalPRBranchRepo struct { + FullName string `json:"full_name"` + Description string `json:"description,omitempty"` +} + // Helper functions +func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest { + m := MinimalPullRequest{ + Number: pr.GetNumber(), + Title: pr.GetTitle(), + Body: pr.GetBody(), + State: pr.GetState(), + Draft: pr.GetDraft(), + Merged: pr.GetMerged(), + MergeableState: pr.GetMergeableState(), + HTMLURL: pr.GetHTMLURL(), + User: convertToMinimalUser(pr.GetUser()), + Additions: pr.GetAdditions(), + Deletions: pr.GetDeletions(), + ChangedFiles: pr.GetChangedFiles(), + Commits: pr.GetCommits(), + Comments: pr.GetComments(), + } + + if pr.CreatedAt != nil { + m.CreatedAt = pr.CreatedAt.Format(time.RFC3339) + } + if pr.UpdatedAt != nil { + m.UpdatedAt = pr.UpdatedAt.Format(time.RFC3339) + } + if pr.ClosedAt != nil { + m.ClosedAt = pr.ClosedAt.Format(time.RFC3339) + } + if pr.MergedAt != nil { + m.MergedAt = pr.MergedAt.Format(time.RFC3339) + } + + for _, label := range pr.Labels { + if label != nil { + m.Labels = append(m.Labels, label.GetName()) + } + } + + for _, assignee := range pr.Assignees { + if assignee != nil { + m.Assignees = append(m.Assignees, assignee.GetLogin()) + } + } + + for _, reviewer := range pr.RequestedReviewers { + if reviewer != nil { + m.RequestedReviewers = append(m.RequestedReviewers, reviewer.GetLogin()) + } + } + + if mergedBy := pr.GetMergedBy(); mergedBy != nil { + m.MergedBy = mergedBy.GetLogin() + } + + if head := pr.Head; head != nil { + m.Head = convertToMinimalPRBranch(head) + } + + if base := pr.Base; base != nil { + m.Base = convertToMinimalPRBranch(base) + } + + if milestone := pr.GetMilestone(); milestone != nil { + m.Milestone = milestone.GetTitle() + } + + return m +} + +func convertToMinimalPRBranch(branch *github.PullRequestBranch) *MinimalPRBranch { + if branch == nil { + return nil + } + + b := &MinimalPRBranch{ + Ref: branch.GetRef(), + SHA: branch.GetSHA(), + } + + if repo := branch.GetRepo(); repo != nil { + b.Repo = &MinimalPRBranchRepo{ + FullName: repo.GetFullName(), + Description: repo.GetDescription(), + } + } + + return b +} + func convertToMinimalProject(fullProject *github.ProjectV2) *MinimalProject { if fullProject == nil { return nil diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 1043870f..58edc07d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -186,12 +186,9 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende } } - r, err := json.Marshal(pr) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } + minimalPR := convertToMinimalPullRequest(pr) - return utils.NewToolResultText(string(r)), nil + return MarshalledTextResult(minimalPR), nil } func GetPullRequestDiff(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 52dbb74a..570b1906 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -127,14 +127,14 @@ func Test_GetPullRequest(t *testing.T) { // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedPR github.PullRequest + // Unmarshal and verify the minimal result + var returnedPR MinimalPullRequest err = json.Unmarshal([]byte(textContent.Text), &returnedPR) require.NoError(t, err) - assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) - assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) - assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) - assert.Equal(t, *tc.expectedPR.HTMLURL, *returnedPR.HTMLURL) + assert.Equal(t, tc.expectedPR.GetNumber(), returnedPR.Number) + assert.Equal(t, tc.expectedPR.GetTitle(), returnedPR.Title) + assert.Equal(t, tc.expectedPR.GetState(), returnedPR.State) + assert.Equal(t, tc.expectedPR.GetHTMLURL(), returnedPR.HTMLURL) }) } } From dc7e789dc46fe5b603d8f6c9049cb8b9c93bc267 Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:33:15 +0000 Subject: [PATCH 3/4] Optimize context usage for getting an issue using the `issue_read` tool (#2022) * minimize context usage using minimal types for get issue comments * preserver reactions field * add back author association --- pkg/github/issues.go | 7 +-- pkg/github/issues_test.go | 14 ++--- pkg/github/minimal_types.go | 103 ++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 12 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 83fd46c3..dcdec6d4 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -376,12 +376,9 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies, } } - r, err := json.Marshal(issue) - if err != nil { - return nil, fmt.Errorf("failed to marshal issue: %w", err) - } + minimalIssue := convertToMinimalIssue(issue) - return utils.NewToolResultText(string(r)), nil + return MarshalledTextResult(minimalIssue), nil } func GetIssueComments(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 1eeec224..c8ff3484 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -345,15 +345,15 @@ func Test_GetIssue(t *testing.T) { textContent := getTextResult(t, result) - var returnedIssue github.Issue + var returnedIssue MinimalIssue err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) - assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) - assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) - assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) - assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) - assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) - assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + assert.Equal(t, tc.expectedIssue.GetNumber(), returnedIssue.Number) + assert.Equal(t, tc.expectedIssue.GetTitle(), returnedIssue.Title) + assert.Equal(t, tc.expectedIssue.GetBody(), returnedIssue.Body) + assert.Equal(t, tc.expectedIssue.GetState(), returnedIssue.State) + assert.Equal(t, tc.expectedIssue.GetHTMLURL(), returnedIssue.HTMLURL) + assert.Equal(t, tc.expectedIssue.GetUser().GetLogin(), returnedIssue.User.Login) }) } } diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index f1dcfe06..4031bfa2 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -136,6 +136,43 @@ type MinimalProject struct { OwnerType string `json:"owner_type,omitempty"` } +// MinimalReactions is the trimmed output type for reaction summaries, dropping the API URL. +type MinimalReactions struct { + TotalCount int `json:"total_count"` + PlusOne int `json:"+1"` + MinusOne int `json:"-1"` + Laugh int `json:"laugh"` + Confused int `json:"confused"` + Heart int `json:"heart"` + Hooray int `json:"hooray"` + Rocket int `json:"rocket"` + Eyes int `json:"eyes"` +} + +// MinimalIssue is the trimmed output type for issue objects to reduce verbosity. +type MinimalIssue struct { + Number int `json:"number"` + Title string `json:"title"` + Body string `json:"body,omitempty"` + State string `json:"state"` + StateReason string `json:"state_reason,omitempty"` + Draft bool `json:"draft,omitempty"` + Locked bool `json:"locked,omitempty"` + HTMLURL string `json:"html_url"` + User *MinimalUser `json:"user,omitempty"` + AuthorAssociation string `json:"author_association,omitempty"` + Labels []string `json:"labels,omitempty"` + Assignees []string `json:"assignees,omitempty"` + Milestone string `json:"milestone,omitempty"` + Comments int `json:"comments,omitempty"` + Reactions *MinimalReactions `json:"reactions,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + ClosedAt string `json:"closed_at,omitempty"` + ClosedBy string `json:"closed_by,omitempty"` + IssueType string `json:"issue_type,omitempty"` +} + // MinimalPullRequest is the trimmed output type for pull request objects to reduce verbosity. type MinimalPullRequest struct { Number int `json:"number"` @@ -180,6 +217,72 @@ type MinimalPRBranchRepo struct { // Helper functions +func convertToMinimalIssue(issue *github.Issue) MinimalIssue { + m := MinimalIssue{ + Number: issue.GetNumber(), + Title: issue.GetTitle(), + Body: issue.GetBody(), + State: issue.GetState(), + StateReason: issue.GetStateReason(), + Draft: issue.GetDraft(), + Locked: issue.GetLocked(), + HTMLURL: issue.GetHTMLURL(), + User: convertToMinimalUser(issue.GetUser()), + AuthorAssociation: issue.GetAuthorAssociation(), + Comments: issue.GetComments(), + } + + if issue.CreatedAt != nil { + m.CreatedAt = issue.CreatedAt.Format(time.RFC3339) + } + if issue.UpdatedAt != nil { + m.UpdatedAt = issue.UpdatedAt.Format(time.RFC3339) + } + if issue.ClosedAt != nil { + m.ClosedAt = issue.ClosedAt.Format(time.RFC3339) + } + + for _, label := range issue.Labels { + if label != nil { + m.Labels = append(m.Labels, label.GetName()) + } + } + + for _, assignee := range issue.Assignees { + if assignee != nil { + m.Assignees = append(m.Assignees, assignee.GetLogin()) + } + } + + if closedBy := issue.GetClosedBy(); closedBy != nil { + m.ClosedBy = closedBy.GetLogin() + } + + if milestone := issue.GetMilestone(); milestone != nil { + m.Milestone = milestone.GetTitle() + } + + if issueType := issue.GetType(); issueType != nil { + m.IssueType = issueType.GetName() + } + + if r := issue.Reactions; r != nil { + m.Reactions = &MinimalReactions{ + TotalCount: r.GetTotalCount(), + PlusOne: r.GetPlusOne(), + MinusOne: r.GetMinusOne(), + Laugh: r.GetLaugh(), + Confused: r.GetConfused(), + Heart: r.GetHeart(), + Hooray: r.GetHooray(), + Rocket: r.GetRocket(), + Eyes: r.GetEyes(), + } + } + + return m +} + func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest { m := MinimalPullRequest{ Number: pr.GetNumber(), From 543a1fa01936c6e1ea36e04ce66ca14691691ece Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Tue, 17 Feb 2026 12:36:28 +0000 Subject: [PATCH 4/4] use minimal types for issue comments to optimize context usage (#2024) --- pkg/github/issues.go | 8 +++---- pkg/github/issues_test.go | 8 +++---- pkg/github/minimal_types.go | 45 +++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index dcdec6d4..cd708555 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -433,12 +433,12 @@ func GetIssueComments(ctx context.Context, client *github.Client, deps ToolDepen comments = filteredComments } - r, err := json.Marshal(comments) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + minimalComments := make([]MinimalIssueComment, 0, len(comments)) + for _, comment := range comments { + minimalComments = append(minimalComments, convertToMinimalIssueComment(comment)) } - return utils.NewToolResultText(string(r)), nil + return MarshalledTextResult(minimalComments), nil } func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependencies, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index c8ff3484..512ba8a6 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2020,16 +2020,16 @@ func Test_GetIssueComments(t *testing.T) { textContent := getTextResult(t, result) // Unmarshal and verify the result - var returnedComments []*github.IssueComment + var returnedComments []MinimalIssueComment err = json.Unmarshal([]byte(textContent.Text), &returnedComments) require.NoError(t, err) assert.Equal(t, len(tc.expectedComments), len(returnedComments)) for i := range tc.expectedComments { require.NotNil(t, tc.expectedComments[i].User) require.NotNil(t, returnedComments[i].User) - assert.Equal(t, tc.expectedComments[i].GetID(), returnedComments[i].GetID()) - assert.Equal(t, tc.expectedComments[i].GetBody(), returnedComments[i].GetBody()) - assert.Equal(t, tc.expectedComments[i].GetUser().GetLogin(), returnedComments[i].GetUser().GetLogin()) + assert.Equal(t, tc.expectedComments[i].GetID(), returnedComments[i].ID) + assert.Equal(t, tc.expectedComments[i].GetBody(), returnedComments[i].Body) + assert.Equal(t, tc.expectedComments[i].GetUser().GetLogin(), returnedComments[i].User.Login) } }) } diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 4031bfa2..2010f561 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -173,6 +173,18 @@ type MinimalIssue struct { IssueType string `json:"issue_type,omitempty"` } +// MinimalIssueComment is the trimmed output type for issue comment objects to reduce verbosity. +type MinimalIssueComment struct { + ID int64 `json:"id"` + Body string `json:"body,omitempty"` + HTMLURL string `json:"html_url"` + User *MinimalUser `json:"user,omitempty"` + AuthorAssociation string `json:"author_association,omitempty"` + Reactions *MinimalReactions `json:"reactions,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` +} + // MinimalPullRequest is the trimmed output type for pull request objects to reduce verbosity. type MinimalPullRequest struct { Number int `json:"number"` @@ -283,6 +295,39 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue { return m } +func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment { + m := MinimalIssueComment{ + ID: comment.GetID(), + Body: comment.GetBody(), + HTMLURL: comment.GetHTMLURL(), + User: convertToMinimalUser(comment.GetUser()), + AuthorAssociation: comment.GetAuthorAssociation(), + } + + if comment.CreatedAt != nil { + m.CreatedAt = comment.CreatedAt.Format(time.RFC3339) + } + if comment.UpdatedAt != nil { + m.UpdatedAt = comment.UpdatedAt.Format(time.RFC3339) + } + + if r := comment.Reactions; r != nil { + m.Reactions = &MinimalReactions{ + TotalCount: r.GetTotalCount(), + PlusOne: r.GetPlusOne(), + MinusOne: r.GetMinusOne(), + Laugh: r.GetLaugh(), + Confused: r.GetConfused(), + Heart: r.GetHeart(), + Hooray: r.GetHooray(), + Rocket: r.GetRocket(), + Eyes: r.GetEyes(), + } + } + + return m +} + func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest { m := MinimalPullRequest{ Number: pr.GetNumber(),