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/github/issues.go b/pkg/github/issues.go index 83fd46c3..cd708555 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) { @@ -436,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 1eeec224..512ba8a6 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) }) } } @@ -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 a33bcec7..2010f561 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,286 @@ 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"` +} + +// 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"` + 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 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 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(), + 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) }) } } 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, ", ")) } }