From c8e40de4aad5896abe5c0ca05eb12966c4cc41ed Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 2 Oct 2026 12:48:27 +0200 Subject: [PATCH 1/2] feat(search): add typed projected result outputs --- pkg/github/issues.go | 420 +++++++++++++++++------- pkg/github/pullrequests.go | 216 ++++++++---- pkg/github/repositories.go | 67 ++-- pkg/github/search.go | 109 ++++-- pkg/github/search_utils.go | 83 ++--- pkg/github/typed_read_normalizers.go | 82 +++++ pkg/github/typed_search_outputs_test.go | 166 ++++++++++ 7 files changed, 871 insertions(+), 272 deletions(-) create mode 100644 pkg/github/typed_read_normalizers.go create mode 100644 pkg/github/typed_search_outputs_test.go diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 4d7e72b58a..3adf308a3c 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "slices" "strconv" "strings" "time" @@ -1970,7 +1971,7 @@ func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inve ) WithPagination(schema) - return NewTool( + return NewTool[SearchIssuesInput, SearchIssuesOutput]( ToolsetMetadataIssues, mcp.Tool{ Name: "search_issues", @@ -1982,16 +1983,14 @@ func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inve InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input SearchIssuesInput) (*mcp.CallToolResult, SearchIssuesOutput, error) { options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} - fields, err := OptionalStringArrayParam(args, "fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - options = append(options, withFieldsFiltering(deps, "search_issues", fields)) - result, err := searchIssuesHandler(ctx, deps, args, mode, options...) - return result, nil, err - }) + options = append(options, withFieldsFiltering(deps, "search_issues", input.Fields)) + result, response, err := searchIssuesHandler(ctx, deps, input, mode, options...) + return result, structuredSearchIssuesOutput(response, input.Fields), err + }, + normalizeTypedReadArguments(nil, false), + ) } // searchIssuesIFCPostProcess returns a searchPostProcessFn that attaches the @@ -2146,6 +2145,150 @@ type SearchIssuesResponse struct { Items []SearchIssueResult `json:"items"` } +type SearchIssuesOutput struct { + Total *int `json:"total_count,omitempty"` + IncompleteResults *bool `json:"incomplete_results,omitempty"` + Items []SearchIssueOutputItem `json:"items"` +} + +type SearchIssueOutputItem struct { + Number *int `json:"number,omitempty"` + Title *string `json:"title,omitempty"` + Body *string `json:"body,omitempty"` + State *string `json:"state,omitempty"` + StateReason *string `json:"state_reason,omitempty"` + Draft *bool `json:"draft,omitempty"` + Locked *bool `json:"locked,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` + User *MinimalUser `json:"user,omitempty"` + AuthorAssociation *string `json:"author_association,omitempty"` + Labels []string `json:"labels,omitempty"` + Assignee *string `json:"assignee,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"` + Type *string `json:"type,omitempty"` + RepositoryURL *string `json:"repository_url,omitempty"` + PullRequest *SearchIssuePullRequest `json:"pull_request,omitempty"` + FieldValues []MinimalFieldValue `json:"field_values,omitempty"` +} + +type SearchIssuePullRequest struct { + URL *string `json:"url,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` + DiffURL *string `json:"diff_url,omitempty"` + PatchURL *string `json:"patch_url,omitempty"` +} + +func structuredSearchIssuesOutput(response SearchIssuesResponse, fields []string) SearchIssuesOutput { + output := SearchIssuesOutput{ + Total: response.Total, + IncompleteResults: response.IncompleteResults, + Items: make([]SearchIssueOutputItem, 0, len(response.Items)), + } + for _, item := range response.Items { + output.Items = append(output.Items, searchIssueOutputItem(item, fields)) + } + return output +} + +func searchIssueOutputItem(result SearchIssueResult, fields []string) SearchIssueOutputItem { + var output SearchIssueOutputItem + issue := result.Issue + if issue == nil { + return output + } + selected := func(field string) bool { + return len(fields) == 0 || slices.Contains(fields, field) + } + minimal := convertToMinimalIssue(issue) + if selected("number") && issue.Number != nil { + output.Number = new(minimal.Number) + } + if selected("title") && issue.Title != nil { + output.Title = new(minimal.Title) + } + if selected("body") && issue.Body != nil { + output.Body = new(minimal.Body) + } + if selected("state") && issue.State != nil { + output.State = new(minimal.State) + } + if selected("state_reason") && issue.StateReason != nil { + output.StateReason = new(minimal.StateReason) + } + if selected("draft") && issue.Draft != nil { + output.Draft = new(minimal.Draft) + } + if selected("locked") && issue.Locked != nil { + output.Locked = new(minimal.Locked) + } + if selected("html_url") && issue.HTMLURL != nil { + output.HTMLURL = new(minimal.HTMLURL) + } + if selected("user") && issue.User != nil { + output.User = minimal.User + } + if selected("author_association") && issue.AuthorAssociation != nil { //nolint:staticcheck // Keep the legacy search projection available. + output.AuthorAssociation = new(minimal.AuthorAssociation) + } + if selected("labels") && issue.Labels != nil { + output.Labels = minimal.Labels + } + if selected("assignee") && issue.Assignee != nil { //nolint:staticcheck // Keep the legacy search projection available. + login := issue.Assignee.GetLogin() //nolint:staticcheck // Keep the legacy search projection available. + output.Assignee = &login + } + if selected("assignees") && issue.Assignees != nil { + output.Assignees = minimal.Assignees + } + if selected("milestone") && issue.Milestone != nil { + output.Milestone = new(minimal.Milestone) + } + if selected("comments") && issue.Comments != nil { + output.Comments = new(minimal.Comments) + } + if selected("reactions") && issue.Reactions != nil { + output.Reactions = minimal.Reactions + } + if selected("created_at") && issue.CreatedAt != nil { + output.CreatedAt = new(minimal.CreatedAt) + } + if selected("updated_at") && issue.UpdatedAt != nil { + output.UpdatedAt = new(minimal.UpdatedAt) + } + if selected("closed_at") && issue.ClosedAt != nil { + output.ClosedAt = new(minimal.ClosedAt) + } + if selected("closed_by") && issue.ClosedBy != nil { + output.ClosedBy = new(minimal.ClosedBy) + } + if selected("type") && issue.Type != nil { + output.Type = new(issue.Type.GetName()) + } + if selected("repository_url") && issue.RepositoryURL != nil { + output.RepositoryURL = new(issue.GetRepositoryURL()) + } + if selected("pull_request") && issue.PullRequestLinks != nil { + links := issue.PullRequestLinks + output.PullRequest = &SearchIssuePullRequest{ + URL: links.URL, + HTMLURL: links.HTMLURL, + DiffURL: links.DiffURL, + PatchURL: links.PatchURL, + } + } + if selected("field_values") && result.FieldValues != nil { + output.FieldValues = result.FieldValues + } + return output +} + // searchIssuesNodesQuery batches a nodes(ids:) lookup over the REST search results to retrieve // each issue's custom field values in a single GraphQL request. type searchIssuesNodesQuery struct { @@ -2337,43 +2480,44 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n // searchIssuesHandler runs the REST issues search, enriches each hit with custom field values // fetched via a single follow-up GraphQL nodes() query, and applies any post-process options // (e.g. IFC labelling). -func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[string]any, mode searchMode, options ...searchOption) (*mcp.CallToolResult, error) { +func searchIssuesHandler(ctx context.Context, deps ToolDependencies, input SearchIssuesInput, mode searchMode, options ...searchOption) (*mcp.CallToolResult, SearchIssuesResponse, error) { const errorPrefix = "failed to search issues" - query, opts, err := prepareSearchArgs(args, "issue", mode) + var output SearchIssuesResponse + query, opts, err := prepareSearchArgs(input, "issue", mode) if err != nil { - return utils.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), output, nil } client, err := deps.GetClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub client", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub client", err), output, nil } result, resp, err := client.Search.Issues(ctx, query, opts) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix, err), nil + return utils.NewToolResultErrorFromErr(errorPrefix, err), output, nil } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to read response body", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to read response body", err), output, nil } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), output, nil } var fieldValuesByID map[string][]MinimalFieldValue if len(result.Issues) > 0 { gqlClient, err := deps.GetGQLClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub GraphQL client", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub GraphQL client", err), output, nil } fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues) if err != nil { const enrichmentError = errorPrefix + ": failed to fetch issue field values" if !isUnsupportedIssueFieldValuesSchemaError(err) { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, enrichmentError, err), nil + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, enrichmentError, err), output, nil } // Older GHES schemas can lack this optional enrichment. Preserve the REST // search results while retaining the compatibility failure for observability. @@ -2401,12 +2545,13 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st opt(&cfg) } + output = response filtered := false var payload any = response if len(cfg.fields) > 0 { filteredItems, err := filterEachField(response.Items, cfg.fields) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to filter results", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to filter results", err), SearchIssuesResponse{}, nil } payload = map[string]any{ "total_count": response.Total, @@ -2418,7 +2563,7 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st r, err := json.Marshal(payload) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to marshal response", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to marshal response", err), SearchIssuesResponse{}, nil } if cfg.fieldsTool != "" { @@ -2429,7 +2574,7 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st if cfg.postProcess != nil { cfg.postProcess(ctx, result, callResult) } - return callResult, nil + return callResult, output, nil } // IssueWriteUIResourceURI is the URI for the issue_write tool's MCP App UI resource. @@ -3415,6 +3560,95 @@ func patchIssue(ctx context.Context, client *github.Client, owner, repo string, } // ListIssues creates a tool to list issues in a GitHub repository. +type ListIssuesInput struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + State string `json:"state,omitempty"` + Labels []string `json:"labels,omitempty"` + OrderBy string `json:"orderBy,omitempty"` + Direction string `json:"direction,omitempty"` + Since string `json:"since,omitempty"` + FieldFilters []IssueFieldFilterInput `json:"field_filters,omitempty"` + Fields []string `json:"fields,omitempty"` + After string `json:"after,omitempty"` + PerPage *int `json:"perPage,omitempty"` + Page json.RawMessage `json:"page,omitempty"` +} + +type IssueFieldFilterInput struct { + FieldName string `json:"field_name"` + Value string `json:"value"` +} + +type ListIssuesOutput struct { + Issues []ListIssueOutput `json:"issues"` + TotalCount int `json:"totalCount"` + PageInfo MinimalPageInfo `json:"pageInfo"` +} + +type ListIssueOutput struct { + Number *int `json:"number,omitempty"` + Title *string `json:"title,omitempty"` + Body *string `json:"body,omitempty"` + State *string `json:"state,omitempty"` + User *MinimalUser `json:"user,omitempty"` + Labels *[]string `json:"labels,omitempty"` + Assignees *[]string `json:"assignees,omitempty"` + Comments *int `json:"comments,omitempty"` + CreatedAt *string `json:"created_at,omitempty"` + UpdatedAt *string `json:"updated_at,omitempty"` + FieldValues *[]MinimalFieldValue `json:"field_values,omitempty"` +} + +func structuredListIssuesOutput(response MinimalIssuesResponse, fields []string) ListIssuesOutput { + output := ListIssuesOutput{ + Issues: make([]ListIssueOutput, 0, len(response.Issues)), + TotalCount: response.TotalCount, + PageInfo: response.PageInfo, + } + selected := func(field string) bool { + return len(fields) == 0 || slices.Contains(fields, field) + } + for _, issue := range response.Issues { + item := ListIssueOutput{} + if selected("number") { + item.Number = new(issue.Number) + } + if selected("title") { + item.Title = new(issue.Title) + } + if selected("body") && issue.Body != "" { + item.Body = new(issue.Body) + } + if selected("state") { + item.State = new(issue.State) + } + if selected("user") && issue.User != nil { + item.User = issue.User + } + if selected("labels") && len(issue.Labels) > 0 { + item.Labels = &issue.Labels + } + if selected("assignees") { + item.Assignees = &issue.Assignees + } + if selected("comments") && issue.Comments != 0 { + item.Comments = new(issue.Comments) + } + if selected("created_at") && issue.CreatedAt != "" { + item.CreatedAt = new(issue.CreatedAt) + } + if selected("updated_at") && issue.UpdatedAt != "" { + item.UpdatedAt = new(issue.UpdatedAt) + } + if selected("field_values") && len(issue.FieldValues) > 0 { + item.FieldValues = &issue.FieldValues + } + output.Issues = append(output.Issues, item) + } + return output +} + func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ Type: "object", @@ -3480,7 +3714,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { ) WithCursorPagination(schema) - st := NewTool( + st := NewTool[ListIssuesInput, ListIssuesOutput]( ToolsetMetadataIssues, mcp.Tool{ Name: "list_issues", @@ -3492,26 +3726,19 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - owner, err := RequiredParam[string](args, "owner") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - repo, err := RequiredParam[string](args, "repo") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input ListIssuesInput) (*mcp.CallToolResult, ListIssuesOutput, error) { + var output ListIssuesOutput + var err error + if input.Owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), output, nil } - - fields, err := OptionalStringArrayParam(args, "fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.Repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), output, nil } + owner, repo := input.Owner, input.Repo // Set optional parameters if provided - state, err := OptionalParam[string](args, "state") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } + state := input.State // Normalize and filter by state state = strings.ToUpper(state) @@ -3525,20 +3752,9 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } // Get labels - labels, err := OptionalStringArrayParam(args, "labels") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - - orderBy, err := OptionalParam[string](args, "orderBy") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - - direction, err := OptionalParam[string](args, "direction") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } + labels := input.Labels + orderBy := input.OrderBy + direction := input.Direction // Normalize and validate orderBy orderBy = strings.ToUpper(orderBy) @@ -3558,10 +3774,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { direction = "DESC" } - since, err := OptionalParam[string](args, "since") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } + since := input.Since // There are two optional parameters: since and labels. var sinceTime time.Time @@ -3569,35 +3782,40 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { if since != "" { sinceTime, err = parseISOTimestamp(since) if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), nil, nil + return utils.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), output, nil } hasSince = true } hasLabels := len(labels) > 0 - rawFilters, err := parseRawFieldFilters(args) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + rawFilters := make([]rawFieldFilter, 0, len(input.FieldFilters)) + for _, filter := range input.FieldFilters { + if filter.FieldName == "" { + return utils.NewToolResultError("field_filters entry: missing required parameter: field_name"), output, nil + } + if filter.Value == "" { + return utils.NewToolResultError(fmt.Sprintf("field_filters entry %q: missing required parameter: value", filter.FieldName)), output, nil + } + rawFilters = append(rawFilters, rawFieldFilter{Name: filter.FieldName, Value: filter.Value}) } // Get pagination parameters and convert to GraphQL format - pagination, err := OptionalCursorPaginationParams(args) - if err != nil { - return nil, nil, err + pagination := CursorPaginationParams{PerPage: 30, After: input.After} + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } // Check if someone tried to use page-based pagination instead of cursor-based - if _, pageProvided := args["page"]; pageProvided { - return utils.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil, nil + if len(input.Page) > 0 { + return utils.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), output, nil } // Check if pagination parameters were explicitly provided - _, perPageProvided := args["perPage"] - paginationExplicit := perPageProvided + paginationExplicit := input.PerPage != nil paginationParams, err := pagination.ToGraphQLParams() if err != nil { - return nil, nil, err + return nil, output, err } // Use default of 30 if pagination was not explicitly provided @@ -3608,7 +3826,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { client, err := deps.GetGQLClient(ctx) if err != nil { - return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil, nil + return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), output, nil } // Resolve field filters by looking up the repo's issue fields so we can @@ -3617,11 +3835,11 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { if len(rawFilters) > 0 { fields, err := fetchIssueFields(ctx, client, owner, repo) if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to look up issue fields for field_filters", err), nil, nil + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to look up issue fields for field_filters", err), output, nil } fieldFilters, err = resolveFieldFilters(rawFilters, fields) if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + return utils.NewToolResultError(err.Error()), output, nil } } @@ -3674,7 +3892,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { ctx, "failed to list issues", issueFieldsErr, - ), nil, nil + ), output, nil } issueQueryWithoutFieldValues := getIssueQueryTypeWithoutFieldValues(hasLabels, hasSince) @@ -3689,7 +3907,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { ctx, "failed to list issues", fmt.Errorf("issue-fields query failed: %w; fallback query failed: %w", issueFieldsErr, fallbackErr), - ), nil, nil + ), output, nil } resp = convertToMinimalIssuesResponseWithoutFieldValues(issueQueryWithoutFieldValues.getIssueFragmentWithoutFieldValues()) @@ -3698,10 +3916,10 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { filtered := false var payload any = resp - if len(fields) > 0 { - filteredIssues, err := filterEachField(resp.Issues, fields) + if len(input.Fields) > 0 { + filteredIssues, err := filterEachField(resp.Issues, input.Fields) if err != nil { - return utils.NewToolResultErrorFromErr("failed to filter issues", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to filter issues", err), output, nil } payload = map[string]any{ "issues": filteredIssues, @@ -3713,15 +3931,17 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { r, err := json.Marshal(payload) if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to marshal response", err), output, nil } recordFieldsUsageFor(ctx, deps, "list_issues", resp, filtered, len(r)) result := utils.NewToolResultText(string(r)) result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelListIssues(isPrivate)) - return result, nil, nil - }) + return result, structuredListIssuesOutput(resp, input.Fields), nil + }, + normalizeTypedReadArguments([]string{"state", "orderBy", "direction"}, true), + ) return st } @@ -3731,46 +3951,6 @@ type rawFieldFilter struct { Value string } -// parseRawFieldFilters extracts the optional field_filters parameter into a list of -// {name, value} pairs. The value is always a string here; type-aware coercion happens -// later in resolveFieldFilters once we know each field's data_type. -func parseRawFieldFilters(args map[string]any) ([]rawFieldFilter, error) { - raw, ok := args["field_filters"] - if !ok { - return nil, nil - } - - var entries []map[string]any - switch v := raw.(type) { - case []any: - for _, f := range v { - entry, ok := f.(map[string]any) - if !ok { - return nil, fmt.Errorf("each field_filters entry must be an object") - } - entries = append(entries, entry) - } - case []map[string]any: - entries = v - default: - return nil, fmt.Errorf("field_filters must be an array") - } - - filters := make([]rawFieldFilter, 0, len(entries)) - for _, entry := range entries { - fieldName, err := RequiredParam[string](entry, "field_name") - if err != nil { - return nil, fmt.Errorf("field_filters entry: %s", err.Error()) - } - value, err := RequiredParam[string](entry, "value") - if err != nil { - return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) - } - filters = append(filters, rawFieldFilter{Name: fieldName, Value: value}) - } - return filters, nil -} - // resolveFieldFilters matches each raw filter against a known field definition and // coerces the value into the right typed slot on IssueFieldValueFilter. Matching is // case-insensitive on field name; option names are also matched case-insensitively for diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index d5e02fee71..4cb523f65b 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "slices" "github.com/go-viper/mapstructure/v2" "github.com/google/go-github/v92/github" @@ -1325,6 +1326,134 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor } // ListPullRequests creates a tool to list pull requests in a GitHub repository. +type ListPullRequestsInput struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + State string `json:"state,omitempty"` + Head string `json:"head,omitempty"` + Base string `json:"base,omitempty"` + Sort string `json:"sort,omitempty"` + Direction string `json:"direction,omitempty"` + Fields []string `json:"fields,omitempty"` + Page *int `json:"page,omitempty"` + PerPage *int `json:"perPage,omitempty"` +} + +type ListPullRequestOutput struct { + Number *int `json:"number,omitempty"` + Title *string `json:"title,omitempty"` + Body *string `json:"body,omitempty"` + State *string `json:"state,omitempty"` + Draft *bool `json:"draft,omitempty"` + Merged *bool `json:"merged,omitempty"` + MergeableState *string `json:"mergeable_state,omitempty"` + HTMLURL *string `json:"html_url,omitempty"` + 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"` +} + +func structuredListPullRequestsOutput(pullRequests []MinimalPullRequest, fields []string) []ListPullRequestOutput { + output := make([]ListPullRequestOutput, 0, len(pullRequests)) + selected := func(field string) bool { + return len(fields) == 0 || slices.Contains(fields, field) + } + for _, pr := range pullRequests { + item := ListPullRequestOutput{} + if selected("number") { + item.Number = new(pr.Number) + } + if selected("title") { + item.Title = new(pr.Title) + } + if selected("body") && pr.Body != "" { + item.Body = new(pr.Body) + } + if selected("state") { + item.State = new(pr.State) + } + if selected("draft") { + item.Draft = new(pr.Draft) + } + if selected("merged") { + item.Merged = new(pr.Merged) + } + if selected("mergeable_state") && pr.MergeableState != "" { + item.MergeableState = new(pr.MergeableState) + } + if selected("html_url") { + item.HTMLURL = new(pr.HTMLURL) + } + if selected("user") && pr.User != nil { + item.User = pr.User + } + if selected("labels") && len(pr.Labels) > 0 { + item.Labels = &pr.Labels + } + if selected("assignees") && len(pr.Assignees) > 0 { + item.Assignees = &pr.Assignees + } + if selected("requested_reviewers") && len(pr.RequestedReviewers) > 0 { + item.RequestedReviewers = &pr.RequestedReviewers + } + if selected("merged_by") && pr.MergedBy != "" { + item.MergedBy = new(pr.MergedBy) + } + if selected("head") && pr.Head != nil { + item.Head = pr.Head + } + if selected("base") && pr.Base != nil { + item.Base = pr.Base + } + if selected("additions") && pr.Additions != 0 { + item.Additions = new(pr.Additions) + } + if selected("deletions") && pr.Deletions != 0 { + item.Deletions = new(pr.Deletions) + } + if selected("changed_files") && pr.ChangedFiles != 0 { + item.ChangedFiles = new(pr.ChangedFiles) + } + if selected("commits") && pr.Commits != 0 { + item.Commits = new(pr.Commits) + } + if selected("comments") && pr.Comments != 0 { + item.Comments = new(pr.Comments) + } + if selected("created_at") && pr.CreatedAt != "" { + item.CreatedAt = new(pr.CreatedAt) + } + if selected("updated_at") && pr.UpdatedAt != "" { + item.UpdatedAt = new(pr.UpdatedAt) + } + if selected("closed_at") && pr.ClosedAt != "" { + item.ClosedAt = new(pr.ClosedAt) + } + if selected("merged_at") && pr.MergedAt != "" { + item.MergedAt = new(pr.MergedAt) + } + if selected("milestone") && pr.Milestone != "" { + item.Milestone = new(pr.Milestone) + } + output = append(output, item) + } + return output +} + func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ Type: "object", @@ -1369,7 +1498,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool ) WithPagination(schema) - return NewTool( + return NewTool[ListPullRequestsInput, []ListPullRequestOutput]( ToolsetMetadataPullRequests, mcp.Tool{ Name: "list_pull_requests", @@ -1381,50 +1510,27 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - owner, err := RequiredParam[string](args, "owner") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input ListPullRequestsInput) (*mcp.CallToolResult, []ListPullRequestOutput, error) { + if input.Owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), nil, nil } - repo, err := RequiredParam[string](args, "repo") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.Repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), nil, nil } - state, err := OptionalParam[string](args, "state") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + pagination := PaginationParams{Page: 1, PerPage: 30} + if input.Page != nil { + pagination.Page = *input.Page } - head, err := OptionalParam[string](args, "head") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - base, err := OptionalParam[string](args, "base") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - sort, err := OptionalParam[string](args, "sort") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - direction, err := OptionalParam[string](args, "direction") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - fields, err := OptionalStringArrayParam(args, "fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - pagination, err := OptionalPaginationParams(args) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } opts := &github.PullRequestListOptions{ - State: state, - Head: head, - Base: base, - Sort: sort, - Direction: direction, + State: input.State, + Head: input.Head, + Base: input.Base, + Sort: input.Sort, + Direction: input.Direction, ListOptions: github.ListOptions{ PerPage: pagination.PerPage, Page: pagination.Page, @@ -1435,7 +1541,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - prs, resp, err := client.PullRequests.List(ctx, owner, repo, opts) + prs, resp, err := client.PullRequests.List(ctx, input.Owner, input.Repo, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list pull requests", @@ -1462,8 +1568,8 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool filtered := false var payload any = minimalPRs - if len(fields) > 0 { - filteredPRs, err := filterEachField(minimalPRs, fields) + if len(input.Fields) > 0 { + filteredPRs, err := filterEachField(minimalPRs, input.Fields) if err != nil { return utils.NewToolResultErrorFromErr("failed to filter pull requests", err), nil, nil } @@ -1481,9 +1587,11 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool result := utils.NewToolResultText(string(r)) // Pull request titles/bodies are user-authored (untrusted); // confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoUserContent) - return result, nil, nil - }) + result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoUserContent) + return result, structuredListPullRequestsOutput(minimalPRs, input.Fields), nil + }, + normalizeTypedReadArguments(nil, false), + ) } // MergePullRequest creates a tool to merge a pull request. @@ -1652,7 +1760,7 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo ) WithPagination(schema) - return NewTool( + return NewTool[SearchIssuesInput, SearchIssuesOutput]( ToolsetMetadataPullRequests, mcp.Tool{ Name: "search_pull_requests", @@ -1664,16 +1772,14 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input SearchIssuesInput) (*mcp.CallToolResult, SearchIssuesOutput, error) { options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} - fields, err := OptionalStringArrayParam(args, "fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - options = append(options, withFieldsFiltering(deps, "search_pull_requests", fields)) - result, err := searchHandler(ctx, deps.GetClient, args, "pr", "failed to search pull requests", options...) - return result, nil, err - }) + options = append(options, withFieldsFiltering(deps, "search_pull_requests", input.Fields)) + result, response, err := searchHandler(ctx, deps.GetClient, input, "pr", "failed to search pull requests", options...) + return result, structuredSearchIssuesOutput(response, input.Fields), err + }, + normalizeTypedReadArguments(nil, false), + ) } // UpdatePullRequestBranch creates a tool to update a pull request branch with the latest changes from the base branch. diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 4a005e6b3c..f1c0194efc 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -312,8 +312,15 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool { } // ListBranches creates a tool to list branches in a GitHub repository. +type RepositoryListInput struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Page *int `json:"page,omitempty"` + PerPage *int `json:"perPage,omitempty"` +} + func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + return NewTool[RepositoryListInput, []MinimalBranch]( ToolsetMetadataRepos, mcp.Tool{ Name: "list_branches", @@ -338,18 +345,19 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { }), }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - owner, err := RequiredParam[string](args, "owner") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryListInput) (*mcp.CallToolResult, []MinimalBranch, error) { + if input.Owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), nil, nil } - repo, err := RequiredParam[string](args, "repo") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.Repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), nil, nil } - pagination, err := OptionalPaginationParams(args) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + pagination := PaginationParams{Page: 1, PerPage: 30} + if input.Page != nil { + pagination.Page = *input.Page + } + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } opts := &github.BranchListOptions{ @@ -364,7 +372,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - branches, resp, err := client.Repositories.ListBranches(ctx, owner, repo, opts) + branches, resp, err := client.Repositories.ListBranches(ctx, input.Owner, input.Repo, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list branches", @@ -397,9 +405,10 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { // Branches are structural repo metadata that only collaborators // with push access can create, so integrity is trusted. // Confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) - return result, nil, nil + result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoMetadata) + return result, minimalBranches, nil }, + normalizeTypedReadArguments(nil, false), ) } @@ -1852,7 +1861,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { // ListTags creates a tool to list tags in a GitHub repository. func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { - return NewTool( + return NewTool[RepositoryListInput, []MinimalTag]( ToolsetMetadataRepos, mcp.Tool{ Name: "list_tags", @@ -1877,18 +1886,19 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { }), }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - owner, err := RequiredParam[string](args, "owner") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryListInput) (*mcp.CallToolResult, []MinimalTag, error) { + if input.Owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), nil, nil } - repo, err := RequiredParam[string](args, "repo") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.Repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), nil, nil } - pagination, err := OptionalPaginationParams(args) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + pagination := PaginationParams{Page: 1, PerPage: 30} + if input.Page != nil { + pagination.Page = *input.Page + } + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } opts := &github.ListOptions{ @@ -1901,7 +1911,7 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - tags, resp, err := client.Repositories.ListTags(ctx, owner, repo, opts) + tags, resp, err := client.Repositories.ListTags(ctx, input.Owner, input.Repo, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list tags", @@ -1935,9 +1945,10 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { // Tags are structural repo metadata created by collaborators with // push access, so integrity is trusted. Confidentiality follows // repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) - return result, nil, nil + result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoMetadata) + return result, minimalTags, nil }, + normalizeTypedReadArguments(nil, false), ) } diff --git a/pkg/github/search.go b/pkg/github/search.go index 18c4d466db..c731b2b701 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "slices" ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/ifc" @@ -192,6 +193,60 @@ func attachSearchRepositoriesIFCLabel(ctx context.Context, deps ToolDependencies } // SearchCode creates a tool to search for code across GitHub repositories. +type SearchCodeInput struct { + Query string `json:"query"` + Sort string `json:"sort,omitempty"` + Order string `json:"order,omitempty"` + Fields []string `json:"fields,omitempty"` + Page *int `json:"page,omitempty"` + PerPage *int `json:"perPage,omitempty"` +} + +type SearchCodeOutput struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []SearchCodeOutputItem `json:"items"` +} + +type SearchCodeOutputItem struct { + Name *string `json:"name,omitempty"` + Path *string `json:"path,omitempty"` + SHA *string `json:"sha,omitempty"` + Repository *string `json:"repository,omitempty"` + TextMatches []*github.TextMatch `json:"text_matches,omitempty"` +} + +func structuredSearchCodeOutput(result MinimalCodeSearchResult, fields []string) SearchCodeOutput { + output := SearchCodeOutput{ + TotalCount: result.TotalCount, + IncompleteResults: result.IncompleteResults, + Items: make([]SearchCodeOutputItem, 0, len(result.Items)), + } + for _, item := range result.Items { + selected := func(field string) bool { + return len(fields) == 0 || slices.Contains(fields, field) + } + projected := SearchCodeOutputItem{} + if selected("name") { + projected.Name = new(item.Name) + } + if selected("path") { + projected.Path = new(item.Path) + } + if selected("sha") { + projected.SHA = new(item.SHA) + } + if selected("repository") { + projected.Repository = new(item.Repository) + } + if selected("text_matches") && item.TextMatches != nil { + projected.TextMatches = item.TextMatches + } + output.Items = append(output.Items, projected) + } + return output +} + func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ Type: "object", @@ -218,7 +273,7 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { ) WithPagination(schema) - return NewTool( + return NewTool[SearchCodeInput, SearchCodeOutput]( ToolsetMetadataRepos, mcp.Tool{ Name: "search_code", @@ -230,31 +285,23 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { - query, err := RequiredParam[string](args, "query") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - sort, err := OptionalParam[string](args, "sort") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input SearchCodeInput) (*mcp.CallToolResult, SearchCodeOutput, error) { + var output SearchCodeOutput + if input.Query == "" { + return utils.NewToolResultError("missing required parameter: query"), output, nil } - order, err := OptionalParam[string](args, "order") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + pagination := PaginationParams{Page: 1, PerPage: 30} + if input.Page != nil { + pagination.Page = *input.Page } - fields, err := OptionalStringArrayParam(args, "fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - pagination, err := OptionalPaginationParams(args) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } + query := input.Query opts := &github.SearchOptions{ - Sort: sort, - Order: order, + Sort: input.Sort, + Order: input.Order, TextMatch: true, ListOptions: github.ListOptions{ PerPage: pagination.PerPage, @@ -264,7 +311,7 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { client, err := deps.GetClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), output, nil } result, resp, err := client.Search.Code(ctx, query, opts) @@ -273,16 +320,16 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { fmt.Sprintf("failed to search code with query '%s'", query), resp, err, - ), nil, nil + ), output, nil } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return utils.NewToolResultErrorFromErr("failed to read response body", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to read response body", err), output, nil } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search code", resp, body), nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to search code", resp, body), output, nil } minimalItems := make([]MinimalCodeResult, 0, len(result.CodeResults)) @@ -307,10 +354,10 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { filtered := false var payload any = minimalResult - if len(fields) > 0 { - filteredItems, err := filterEachField(minimalItems, fields) + if len(input.Fields) > 0 { + filteredItems, err := filterEachField(minimalItems, input.Fields) if err != nil { - return utils.NewToolResultErrorFromErr("failed to filter code search results", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to filter code search results", err), output, nil } payload = map[string]any{ "total_count": minimalResult.TotalCount, @@ -322,7 +369,7 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { r, err := json.Marshal(payload) if err != nil { - return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil + return utils.NewToolResultErrorFromErr("failed to marshal response", err), output, nil } recordSearchCodeFieldsUsage(ctx, deps, minimalResult, filtered, len(r)) @@ -338,8 +385,10 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { } } callResult = attachJoinedIFCLabel(ctx, deps, callResult, visibilities, ifc.LabelSearchIssues) - return callResult, nil, nil + output = structuredSearchCodeOutput(*minimalResult, input.Fields) + return callResult, output, nil }, + normalizeTypedReadArguments(nil, false), ) } diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 47b8aa26bd..5c82f4b60f 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -53,6 +53,17 @@ type searchConfig struct { fieldsDeps ToolDependencies } +type SearchIssuesInput struct { + Query string `json:"query"` + Owner string `json:"owner,omitempty"` + Repo string `json:"repo,omitempty"` + Sort string `json:"sort,omitempty"` + Order string `json:"order,omitempty"` + Fields []string `json:"fields,omitempty"` + Page *int `json:"page,omitempty"` + PerPage *int `json:"perPage,omitempty"` +} + type searchOption func(*searchConfig) // withSearchPostProcess registers a callback invoked after a successful search @@ -87,46 +98,31 @@ const ( // prepareSearchArgs resolves the search query string and REST search options from the tool args, // applying the standard is: / repo:/ munging shared by search_issues and // search_pull_requests. -func prepareSearchArgs(args map[string]any, targetType string, mode searchMode) (string, *github.SearchOptions, error) { - query, err := RequiredParam[string](args, "query") - if err != nil { - return "", nil, err +func prepareSearchArgs(input SearchIssuesInput, targetType string, mode searchMode) (string, *github.SearchOptions, error) { + query := input.Query + if query == "" { + return "", nil, fmt.Errorf("missing required parameter: query") } if !hasSpecificFilter(query, "is", targetType) { query = fmt.Sprintf("is:%s %s", targetType, query) } - owner, err := OptionalParam[string](args, "owner") - if err != nil { - return "", nil, err + if input.Owner != "" && input.Repo != "" && !hasRepoFilter(query) { + query = fmt.Sprintf("repo:%s/%s %s", input.Owner, input.Repo, query) } - repo, err := OptionalParam[string](args, "repo") - if err != nil { - return "", nil, err + pagination := PaginationParams{Page: 1, PerPage: 30} + if input.Page != nil { + pagination.Page = *input.Page } - - if owner != "" && repo != "" && !hasRepoFilter(query) { - query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) - } - - sort, err := OptionalParam[string](args, "sort") - if err != nil { - return "", nil, err - } - order, err := OptionalParam[string](args, "order") - if err != nil { - return "", nil, err - } - pagination, err := OptionalPaginationParams(args) - if err != nil { - return "", nil, err + if input.PerPage != nil { + pagination.PerPage = *input.PerPage } opts := &github.SearchOptions{ - Sort: sort, - Order: order, + Sort: input.Sort, + Order: input.Order, ListOptions: github.ListOptions{ Page: pagination.Page, PerPage: pagination.PerPage, @@ -172,36 +168,37 @@ func applySemanticSearch(query string, opts *github.SearchOptions) string { func searchHandler( ctx context.Context, getClient GetClientFn, - args map[string]any, + input SearchIssuesInput, targetType string, errorPrefix string, options ...searchOption, -) (*mcp.CallToolResult, error) { +) (*mcp.CallToolResult, SearchIssuesResponse, error) { + var output SearchIssuesResponse cfg := searchConfig{} for _, opt := range options { opt(&cfg) } - query, opts, err := prepareSearchArgs(args, targetType, searchModeLexical) + query, opts, err := prepareSearchArgs(input, targetType, searchModeLexical) if err != nil { - return utils.NewToolResultError(err.Error()), nil + return utils.NewToolResultError(err.Error()), output, nil } client, err := getClient(ctx) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub client", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub client", err), output, nil } result, resp, err := client.Search.Issues(ctx, query, opts) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix, err), nil + return utils.NewToolResultErrorFromErr(errorPrefix, err), output, nil } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to read response body", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to read response body", err), output, nil } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), output, nil } // result.Issues are raw *github.Issue objects marshaled directly below rather than through @@ -215,7 +212,7 @@ func searchHandler( if len(cfg.fields) > 0 { filteredItems, err := filterEachField(result.Issues, cfg.fields) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to filter results", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to filter results", err), output, nil } payload = map[string]any{ "total_count": result.Total, @@ -227,7 +224,7 @@ func searchHandler( r, err := json.Marshal(payload) if err != nil { - return utils.NewToolResultErrorFromErr(errorPrefix+": failed to marshal response", err), nil + return utils.NewToolResultErrorFromErr(errorPrefix+": failed to marshal response", err), output, nil } if cfg.fieldsTool != "" { @@ -238,5 +235,13 @@ func searchHandler( if cfg.postProcess != nil { cfg.postProcess(ctx, result, callResult) } - return callResult, nil + output = SearchIssuesResponse{ + Total: result.Total, + IncompleteResults: result.IncompleteResults, + Items: make([]SearchIssueResult, 0, len(result.Issues)), + } + for _, issue := range result.Issues { + output.Items = append(output.Items, SearchIssueResult{Issue: issue}) + } + return callResult, output, nil } diff --git a/pkg/github/typed_read_normalizers.go b/pkg/github/typed_read_normalizers.go new file mode 100644 index 0000000000..6dcd56b658 --- /dev/null +++ b/pkg/github/typed_read_normalizers.go @@ -0,0 +1,82 @@ +package github + +import ( + "encoding/json" + "fmt" + "math" + "strconv" + "strings" + + "github.com/github/github-mcp-server/pkg/inventory" +) + +func normalizeTypedReadArguments(uppercaseFields []string, preserveZeroPage bool) inventory.InputNormalizer { + return func(raw json.RawMessage) (json.RawMessage, error) { + var args map[string]json.RawMessage + if err := json.Unmarshal(raw, &args); err != nil { + return nil, err + } + if args == nil { + return raw, nil + } + + for _, field := range uppercaseFields { + var value string + if err := json.Unmarshal(args[field], &value); err == nil { + normalized, err := json.Marshal(strings.ToUpper(value)) + if err != nil { + return nil, err + } + args[field] = normalized + } + } + + for _, field := range []string{"page", "perPage"} { + value, exists := args[field] + if !exists { + continue + } + if field == "page" && preserveZeroPage { + continue + } + var number any + if err := json.Unmarshal(value, &number); err != nil { + return nil, err + } + if text, ok := number.(string); ok { + parsed, err := strconv.ParseFloat(text, 64) + if err != nil || math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed != math.Trunc(parsed) || parsed > float64(math.MaxInt) || parsed < float64(math.MinInt) { + return nil, fmt.Errorf("parameter %s is not a valid number", field) + } + number = int(parsed) + } + if field == "page" && !preserveZeroPage { + if parsed, ok := number.(float64); ok && parsed == 0 { + delete(args, field) + continue + } + if parsed, ok := number.(int); ok && parsed == 0 { + delete(args, field) + continue + } + } + if field == "perPage" { + if parsed, ok := number.(float64); ok && parsed == 0 { + delete(args, field) + continue + } + if parsed, ok := number.(int); ok && parsed == 0 { + delete(args, field) + continue + } + } + normalized, err := json.Marshal(number) + if err != nil { + return nil, err + } + args[field] = normalized + } + + return json.Marshal(args) + } +} diff --git a/pkg/github/typed_search_outputs_test.go b/pkg/github/typed_search_outputs_test.go new file mode 100644 index 0000000000..7818853509 --- /dev/null +++ b/pkg/github/typed_search_outputs_test.go @@ -0,0 +1,166 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v92/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { + tools := []inventory.ServerTool{ + SearchCode(translations.NullTranslationHelper), + SearchIssues(translations.NullTranslationHelper), + SearchPullRequests(translations.NullTranslationHelper), + ListIssues(translations.NullTranslationHelper), + ListPullRequests(translations.NullTranslationHelper), + ListBranches(translations.NullTranslationHelper), + ListTags(translations.NullTranslationHelper), + } + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetSearchCode: mockResponse(t, http.StatusOK, &github.CodeSearchResult{ + Total: new(1), + IncompleteResults: new(false), + CodeResults: []*github.CodeResult{{ + Name: new("main.go"), + Path: new("cmd/main.go"), + SHA: new("abc123"), + }}, + }), + GetReposTagsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.RepositoryTag{ + {Name: new("v1.0.0"), Commit: &github.Commit{SHA: new("abc123")}}, + }), + })), + } + + for _, protocolVersion := range []string{"2025-11-25", inventory.ProtocolVersionMultiRoundTrip} { + t.Run(protocolVersion, func(t *testing.T) { + server := mcp.NewServer(&mcp.Implementation{Name: "typed-projected-read-test", Version: "v1"}, nil) + server.AddReceivingMiddleware(InjectDepsMiddleware(deps)) + for _, tool := range tools { + tool.RegisterFunc(server, deps) + } + + session := connectCommentVisibilityClient(t, server, protocolVersion) + list, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + require.Len(t, list.Tools, len(tools)) + for _, tool := range list.Tools { + if protocolVersion == "2025-11-25" { + assert.Nil(t, tool.OutputSchema, "legacy clients must not see outputSchema for %s", tool.Name) + continue + } + + require.NotNil(t, tool.OutputSchema, "%s must publish its typed output schema", tool.Name) + schemaJSON, err := json.Marshal(tool.OutputSchema) + require.NoError(t, err) + var schema jsonschema.Schema + require.NoError(t, json.Unmarshal(schemaJSON, &schema)) + resolved, err := schema.Resolve(nil) + require.NoError(t, err) + require.NoError(t, resolved.Validate(projectedReadOutputSample(tool.Name)), "%s output must conform to its schema", tool.Name) + } + + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "list_tags", + Arguments: map[string]any{"owner": "owner", "repo": "repo"}, + }) + require.NoError(t, err) + require.False(t, result.IsError, result) + require.Equal(t, `[{"name":"v1.0.0","sha":"abc123"}]`, getTextResult(t, result).Text) + if protocolVersion == "2025-11-25" { + assert.Nil(t, result.StructuredContent) + } else { + structured, err := json.Marshal(result.StructuredContent) + require.NoError(t, err) + assert.JSONEq(t, getTextResult(t, result).Text, string(structured)) + } + + result, err = session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "search_code", + Arguments: map[string]any{ + "query": "main.go", + "fields": []any{"name"}, + }, + }) + require.NoError(t, err) + require.False(t, result.IsError, result) + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go"}]}`, getTextResult(t, result).Text) + if protocolVersion == "2025-11-25" { + assert.Nil(t, result.StructuredContent) + } else { + structured, err := json.Marshal(result.StructuredContent) + require.NoError(t, err) + assert.JSONEq(t, getTextResult(t, result).Text, string(structured)) + } + }) + } +} + +func projectedReadOutputSample(name string) any { + switch name { + case "search_code", "search_issues", "search_pull_requests": + return map[string]any{"total_count": 0, "incomplete_results": false, "items": []any{}} + case "list_issues": + return map[string]any{ + "issues": []any{}, "totalCount": 0, + "pageInfo": map[string]any{"hasNextPage": false, "hasPreviousPage": false}, + } + case "list_pull_requests", "list_branches", "list_tags": + return []any{} + default: + panic("unexpected projected read tool: " + name) + } +} + +func TestTypedProjectedReadOutputsRespectFieldSelection(t *testing.T) { + code := structuredSearchCodeOutput(MinimalCodeSearchResult{ + TotalCount: 1, + IncompleteResults: false, + Items: []MinimalCodeResult{{Name: "main.go", Path: "cmd/main.go", SHA: "abc123", Repository: "owner/repo"}}, + }, []string{"name"}) + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go"}]}`, mustMarshalJSON(t, code)) + + searchIssues := structuredSearchIssuesOutput(SearchIssuesResponse{ + Total: new(1), + IncompleteResults: new(false), + Items: []SearchIssueResult{{ + Issue: &github.Issue{Number: new(42), Title: new("A title"), Body: new("A body")}, + }}, + }, []string{"title"}) + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"title":"A title"}]}`, mustMarshalJSON(t, searchIssues)) + + issues := structuredListIssuesOutput(MinimalIssuesResponse{ + Issues: []MinimalIssue{{Number: 42, Title: "A title", Body: "A body", State: "OPEN", Assignees: []string{}}}, + TotalCount: 1, + }, []string{"title"}) + assert.JSONEq(t, `{"issues":[{"title":"A title"}],"totalCount":1,"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`, mustMarshalJSON(t, issues)) + + pullRequests := structuredListPullRequestsOutput([]MinimalPullRequest{{ + Number: 42, Title: "A title", Body: "A body", State: "open", HTMLURL: "https://example.test/pr/42", + }}, []string{"title"}) + assert.JSONEq(t, `[{"title":"A title"}]`, mustMarshalJSON(t, pullRequests)) +} + +func TestNormalizeTypedReadArgumentsPreservesLegacyValues(t *testing.T) { + normalize := normalizeTypedReadArguments([]string{"state", "orderBy", "direction"}, true) + normalized, err := normalize(json.RawMessage(`{"state":"open","orderBy":"updated_at","direction":"asc","perPage":"25","page":null}`)) + require.NoError(t, err) + assert.JSONEq(t, `{"state":"OPEN","orderBy":"UPDATED_AT","direction":"ASC","perPage":25,"page":null}`, string(normalized)) +} + +func mustMarshalJSON(t *testing.T, value any) string { + t.Helper() + encoded, err := json.Marshal(value) + require.NoError(t, err) + return string(encoded) +} From 1447006ba105d032ceb322dcdf7eb07f4f257920 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Sat, 3 Oct 2026 14:30:08 +0200 Subject: [PATCH 2/2] fix(search): preserve field_values presence and add nested schema fixtures Address review findings on the typed search/list projected-output tools (search_code, search_issues, search_pull_requests, list_issues, list_pull_requests, list_branches, list_tags): - issues.go/pullrequests.go: change FieldValues to a pointer-to-slice (*[]MinimalFieldValue) so a successful-but-empty [] is preserved in structuredContent, matching legacy text output instead of being dropped by omitempty. - search.go: related adjustments to keep typed search outputs aligned with the field_values presence fix. - typed_search_outputs_test.go: add representative nested/non-empty projected item fixtures so schema-conformance tests actually exercise item shapes instead of only validating empty collections. - Add/update toolsnaps for list_branches, list_issues, list_pull_requests, list_tags, search_code, search_issues, and search_pull_requests to reflect the schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../__toolsnaps__/list_branches_output.snap | 30 ++ .../__toolsnaps__/list_issues_output.snap | 245 ++++++++++++ .../list_pull_requests_output.snap | 335 ++++++++++++++++ .../__toolsnaps__/list_tags_output.snap | 26 ++ .../__toolsnaps__/search_code_output.snap | 136 +++++++ .../__toolsnaps__/search_issues_output.snap | 374 ++++++++++++++++++ .../search_pull_requests_output.snap | 374 ++++++++++++++++++ pkg/github/issues.go | 44 ++- pkg/github/pullrequests.go | 20 +- pkg/github/search.go | 55 ++- pkg/github/typed_search_outputs_test.go | 233 ++++++++++- 11 files changed, 1831 insertions(+), 41 deletions(-) create mode 100644 pkg/github/__toolsnaps__/list_branches_output.snap create mode 100644 pkg/github/__toolsnaps__/list_issues_output.snap create mode 100644 pkg/github/__toolsnaps__/list_pull_requests_output.snap create mode 100644 pkg/github/__toolsnaps__/list_tags_output.snap create mode 100644 pkg/github/__toolsnaps__/search_code_output.snap create mode 100644 pkg/github/__toolsnaps__/search_issues_output.snap create mode 100644 pkg/github/__toolsnaps__/search_pull_requests_output.snap diff --git a/pkg/github/__toolsnaps__/list_branches_output.snap b/pkg/github/__toolsnaps__/list_branches_output.snap new file mode 100644 index 0000000000..147d9a7efe --- /dev/null +++ b/pkg/github/__toolsnaps__/list_branches_output.snap @@ -0,0 +1,30 @@ +{ + "inputSchema": null, + "name": "list_branches", + "outputSchema": { + "items": { + "additionalProperties": false, + "properties": { + "name": { + "type": "string" + }, + "protected": { + "type": "boolean" + }, + "sha": { + "type": "string" + } + }, + "required": [ + "name", + "sha", + "protected" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_issues_output.snap b/pkg/github/__toolsnaps__/list_issues_output.snap new file mode 100644 index 0000000000..9c71265fea --- /dev/null +++ b/pkg/github/__toolsnaps__/list_issues_output.snap @@ -0,0 +1,245 @@ +{ + "inputSchema": null, + "name": "list_issues", + "outputSchema": { + "additionalProperties": false, + "properties": { + "issues": { + "items": { + "additionalProperties": false, + "properties": { + "assignees": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "body": { + "type": [ + "null", + "string" + ] + }, + "comments": { + "description": "Number of comments on the issue.", + "type": [ + "null", + "integer" + ] + }, + "created_at": { + "description": "Creation time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "field_values": { + "items": { + "additionalProperties": false, + "properties": { + "field": { + "type": "string" + }, + "value": { + "type": "string" + }, + "values": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + } + }, + "required": [ + "field" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "labels": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "number": { + "type": [ + "null", + "integer" + ] + }, + "state": { + "description": "Issue state: OPEN or CLOSED.", + "type": [ + "null", + "string" + ] + }, + "title": { + "type": [ + "null", + "string" + ] + }, + "updated_at": { + "description": "Last update time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "user": { + "additionalProperties": false, + "properties": { + "avatar_url": { + "type": "string" + }, + "details": { + "additionalProperties": false, + "properties": { + "bio": { + "type": "string" + }, + "blog": { + "type": "string" + }, + "company": { + "type": "string" + }, + "created_at": { + "description": "Account creation time in RFC3339 format.", + "type": "string" + }, + "email": { + "type": "string" + }, + "followers": { + "type": "integer" + }, + "following": { + "type": "integer" + }, + "hireable": { + "type": "boolean" + }, + "location": { + "type": "string" + }, + "name": { + "type": "string" + }, + "owned_private_repos": { + "type": "integer" + }, + "private_gists": { + "type": "integer" + }, + "public_gists": { + "type": "integer" + }, + "public_repos": { + "type": "integer" + }, + "total_private_repos": { + "type": "integer" + }, + "twitter_username": { + "type": "string" + }, + "updated_at": { + "description": "Last profile update time in RFC3339 format.", + "type": "string" + } + }, + "required": [ + "public_repos", + "public_gists", + "followers", + "following", + "created_at", + "updated_at" + ], + "type": [ + "null", + "object" + ] + }, + "id": { + "type": "integer" + }, + "login": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + }, + "required": [ + "login" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "pageInfo": { + "additionalProperties": false, + "properties": { + "endCursor": { + "description": "Cursor for the last issue in this page.", + "type": "string" + }, + "hasNextPage": { + "description": "Whether another page of results is available.", + "type": "boolean" + }, + "hasPreviousPage": { + "description": "Whether a previous page of results is available.", + "type": "boolean" + }, + "startCursor": { + "description": "Cursor for the first issue in this page.", + "type": "string" + } + }, + "required": [ + "hasNextPage", + "hasPreviousPage" + ], + "type": "object" + }, + "totalCount": { + "description": "Total number of issues matching the filters.", + "type": "integer" + } + }, + "required": [ + "issues", + "totalCount", + "pageInfo" + ], + "type": "object" + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_pull_requests_output.snap b/pkg/github/__toolsnaps__/list_pull_requests_output.snap new file mode 100644 index 0000000000..2d70947044 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_pull_requests_output.snap @@ -0,0 +1,335 @@ +{ + "inputSchema": null, + "name": "list_pull_requests", + "outputSchema": { + "items": { + "additionalProperties": false, + "properties": { + "additions": { + "description": "Number of lines added.", + "type": [ + "null", + "integer" + ] + }, + "assignees": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "base": { + "additionalProperties": false, + "properties": { + "ref": { + "type": "string" + }, + "repo": { + "additionalProperties": false, + "properties": { + "description": { + "type": "string" + }, + "full_name": { + "type": "string" + } + }, + "required": [ + "full_name" + ], + "type": [ + "null", + "object" + ] + }, + "sha": { + "type": "string" + } + }, + "required": [ + "ref", + "sha" + ], + "type": [ + "null", + "object" + ] + }, + "body": { + "type": [ + "null", + "string" + ] + }, + "changed_files": { + "description": "Number of files changed.", + "type": [ + "null", + "integer" + ] + }, + "closed_at": { + "description": "Closing time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "comments": { + "description": "Number of comments on the pull request.", + "type": [ + "null", + "integer" + ] + }, + "commits": { + "description": "Number of commits in the pull request.", + "type": [ + "null", + "integer" + ] + }, + "created_at": { + "description": "Creation time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "deletions": { + "description": "Number of lines removed.", + "type": [ + "null", + "integer" + ] + }, + "draft": { + "type": [ + "null", + "boolean" + ] + }, + "head": { + "additionalProperties": false, + "properties": { + "ref": { + "type": "string" + }, + "repo": { + "additionalProperties": false, + "properties": { + "description": { + "type": "string" + }, + "full_name": { + "type": "string" + } + }, + "required": [ + "full_name" + ], + "type": [ + "null", + "object" + ] + }, + "sha": { + "type": "string" + } + }, + "required": [ + "ref", + "sha" + ], + "type": [ + "null", + "object" + ] + }, + "html_url": { + "type": [ + "null", + "string" + ] + }, + "labels": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "mergeable_state": { + "type": [ + "null", + "string" + ] + }, + "merged": { + "type": [ + "null", + "boolean" + ] + }, + "merged_at": { + "description": "Merge time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "merged_by": { + "type": [ + "null", + "string" + ] + }, + "milestone": { + "type": [ + "null", + "string" + ] + }, + "number": { + "type": [ + "null", + "integer" + ] + }, + "requested_reviewers": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "state": { + "description": "Pull request state: open or closed.", + "type": [ + "null", + "string" + ] + }, + "title": { + "type": [ + "null", + "string" + ] + }, + "updated_at": { + "description": "Last update time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "user": { + "additionalProperties": false, + "properties": { + "avatar_url": { + "type": "string" + }, + "details": { + "additionalProperties": false, + "properties": { + "bio": { + "type": "string" + }, + "blog": { + "type": "string" + }, + "company": { + "type": "string" + }, + "created_at": { + "description": "Account creation time in RFC3339 format.", + "type": "string" + }, + "email": { + "type": "string" + }, + "followers": { + "type": "integer" + }, + "following": { + "type": "integer" + }, + "hireable": { + "type": "boolean" + }, + "location": { + "type": "string" + }, + "name": { + "type": "string" + }, + "owned_private_repos": { + "type": "integer" + }, + "private_gists": { + "type": "integer" + }, + "public_gists": { + "type": "integer" + }, + "public_repos": { + "type": "integer" + }, + "total_private_repos": { + "type": "integer" + }, + "twitter_username": { + "type": "string" + }, + "updated_at": { + "description": "Last profile update time in RFC3339 format.", + "type": "string" + } + }, + "required": [ + "public_repos", + "public_gists", + "followers", + "following", + "created_at", + "updated_at" + ], + "type": [ + "null", + "object" + ] + }, + "id": { + "type": "integer" + }, + "login": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + }, + "required": [ + "login" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": "object" + }, + "type": [ + "null", + "array" + ] + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_tags_output.snap b/pkg/github/__toolsnaps__/list_tags_output.snap new file mode 100644 index 0000000000..2975b18ad3 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_tags_output.snap @@ -0,0 +1,26 @@ +{ + "inputSchema": null, + "name": "list_tags", + "outputSchema": { + "items": { + "additionalProperties": false, + "properties": { + "name": { + "type": "string" + }, + "sha": { + "type": "string" + } + }, + "required": [ + "name", + "sha" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/search_code_output.snap b/pkg/github/__toolsnaps__/search_code_output.snap new file mode 100644 index 0000000000..68ebc8c905 --- /dev/null +++ b/pkg/github/__toolsnaps__/search_code_output.snap @@ -0,0 +1,136 @@ +{ + "inputSchema": null, + "name": "search_code", + "outputSchema": { + "additionalProperties": false, + "properties": { + "incomplete_results": { + "description": "Whether GitHub returned an incomplete result set.", + "type": "boolean" + }, + "items": { + "items": { + "additionalProperties": false, + "properties": { + "name": { + "type": [ + "null", + "string" + ] + }, + "path": { + "type": [ + "null", + "string" + ] + }, + "repository": { + "type": [ + "null", + "string" + ] + }, + "sha": { + "type": [ + "null", + "string" + ] + }, + "text_matches": { + "description": "Matching code fragments returned by GitHub.", + "items": { + "additionalProperties": false, + "properties": { + "fragment": { + "description": "Source fragment containing the match.", + "type": [ + "null", + "string" + ] + }, + "matches": { + "description": "Matched text and its offsets in the fragment.", + "items": { + "additionalProperties": false, + "properties": { + "indices": { + "description": "Start and end offsets of the match in the fragment.", + "items": { + "type": "integer" + }, + "type": [ + "null", + "array" + ] + }, + "text": { + "description": "Matched text.", + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "type": [ + "null", + "array" + ] + }, + "object_type": { + "description": "GitHub object type containing the match.", + "type": [ + "null", + "string" + ] + }, + "object_url": { + "description": "API URL of the matched file or object.", + "type": [ + "null", + "string" + ] + }, + "property": { + "description": "Property containing the matched text.", + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "type": [ + "null", + "array" + ] + } + }, + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "total_count": { + "description": "Total number of matching code results.", + "type": "integer" + } + }, + "required": [ + "total_count", + "incomplete_results", + "items" + ], + "type": "object" + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/search_issues_output.snap b/pkg/github/__toolsnaps__/search_issues_output.snap new file mode 100644 index 0000000000..82b00688f6 --- /dev/null +++ b/pkg/github/__toolsnaps__/search_issues_output.snap @@ -0,0 +1,374 @@ +{ + "inputSchema": null, + "name": "search_issues", + "outputSchema": { + "additionalProperties": false, + "properties": { + "incomplete_results": { + "description": "Whether GitHub returned an incomplete result set.", + "type": [ + "null", + "boolean" + ] + }, + "items": { + "items": { + "additionalProperties": false, + "properties": { + "assignee": { + "type": [ + "null", + "string" + ] + }, + "assignees": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "author_association": { + "type": [ + "null", + "string" + ] + }, + "body": { + "type": [ + "null", + "string" + ] + }, + "closed_at": { + "description": "Closing time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "closed_by": { + "type": [ + "null", + "string" + ] + }, + "comments": { + "description": "Number of comments on the issue.", + "type": [ + "null", + "integer" + ] + }, + "created_at": { + "description": "Creation time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "draft": { + "type": [ + "null", + "boolean" + ] + }, + "field_values": { + "items": { + "additionalProperties": false, + "properties": { + "field": { + "type": "string" + }, + "value": { + "type": "string" + }, + "values": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + } + }, + "required": [ + "field" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "html_url": { + "type": [ + "null", + "string" + ] + }, + "labels": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "locked": { + "type": [ + "null", + "boolean" + ] + }, + "milestone": { + "type": [ + "null", + "string" + ] + }, + "number": { + "type": [ + "null", + "integer" + ] + }, + "pull_request": { + "additionalProperties": false, + "properties": { + "diff_url": { + "type": [ + "null", + "string" + ] + }, + "html_url": { + "type": [ + "null", + "string" + ] + }, + "patch_url": { + "type": [ + "null", + "string" + ] + }, + "url": { + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "reactions": { + "additionalProperties": false, + "properties": { + "+1": { + "type": "integer" + }, + "-1": { + "type": "integer" + }, + "confused": { + "type": "integer" + }, + "eyes": { + "type": "integer" + }, + "heart": { + "type": "integer" + }, + "hooray": { + "type": "integer" + }, + "laugh": { + "type": "integer" + }, + "rocket": { + "type": "integer" + }, + "total_count": { + "type": "integer" + } + }, + "required": [ + "total_count", + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": [ + "null", + "object" + ] + }, + "repository_url": { + "type": [ + "null", + "string" + ] + }, + "state": { + "description": "Issue state, typically open or closed.", + "type": [ + "null", + "string" + ] + }, + "state_reason": { + "type": [ + "null", + "string" + ] + }, + "title": { + "type": [ + "null", + "string" + ] + }, + "type": { + "type": [ + "null", + "string" + ] + }, + "updated_at": { + "description": "Last update time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "user": { + "additionalProperties": false, + "properties": { + "avatar_url": { + "type": "string" + }, + "details": { + "additionalProperties": false, + "properties": { + "bio": { + "type": "string" + }, + "blog": { + "type": "string" + }, + "company": { + "type": "string" + }, + "created_at": { + "description": "Account creation time in RFC3339 format.", + "type": "string" + }, + "email": { + "type": "string" + }, + "followers": { + "type": "integer" + }, + "following": { + "type": "integer" + }, + "hireable": { + "type": "boolean" + }, + "location": { + "type": "string" + }, + "name": { + "type": "string" + }, + "owned_private_repos": { + "type": "integer" + }, + "private_gists": { + "type": "integer" + }, + "public_gists": { + "type": "integer" + }, + "public_repos": { + "type": "integer" + }, + "total_private_repos": { + "type": "integer" + }, + "twitter_username": { + "type": "string" + }, + "updated_at": { + "description": "Last profile update time in RFC3339 format.", + "type": "string" + } + }, + "required": [ + "public_repos", + "public_gists", + "followers", + "following", + "created_at", + "updated_at" + ], + "type": [ + "null", + "object" + ] + }, + "id": { + "type": "integer" + }, + "login": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + }, + "required": [ + "login" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "total_count": { + "description": "Total number of matching issues or pull requests.", + "type": [ + "null", + "integer" + ] + } + }, + "required": [ + "items" + ], + "type": "object" + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/search_pull_requests_output.snap b/pkg/github/__toolsnaps__/search_pull_requests_output.snap new file mode 100644 index 0000000000..45e170ea42 --- /dev/null +++ b/pkg/github/__toolsnaps__/search_pull_requests_output.snap @@ -0,0 +1,374 @@ +{ + "inputSchema": null, + "name": "search_pull_requests", + "outputSchema": { + "additionalProperties": false, + "properties": { + "incomplete_results": { + "description": "Whether GitHub returned an incomplete result set.", + "type": [ + "null", + "boolean" + ] + }, + "items": { + "items": { + "additionalProperties": false, + "properties": { + "assignee": { + "type": [ + "null", + "string" + ] + }, + "assignees": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "author_association": { + "type": [ + "null", + "string" + ] + }, + "body": { + "type": [ + "null", + "string" + ] + }, + "closed_at": { + "description": "Closing time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "closed_by": { + "type": [ + "null", + "string" + ] + }, + "comments": { + "description": "Number of comments on the issue.", + "type": [ + "null", + "integer" + ] + }, + "created_at": { + "description": "Creation time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "draft": { + "type": [ + "null", + "boolean" + ] + }, + "field_values": { + "items": { + "additionalProperties": false, + "properties": { + "field": { + "type": "string" + }, + "value": { + "type": "string" + }, + "values": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + } + }, + "required": [ + "field" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "html_url": { + "type": [ + "null", + "string" + ] + }, + "labels": { + "items": { + "type": "string" + }, + "type": [ + "null", + "array" + ] + }, + "locked": { + "type": [ + "null", + "boolean" + ] + }, + "milestone": { + "type": [ + "null", + "string" + ] + }, + "number": { + "type": [ + "null", + "integer" + ] + }, + "pull_request": { + "additionalProperties": false, + "properties": { + "diff_url": { + "type": [ + "null", + "string" + ] + }, + "html_url": { + "type": [ + "null", + "string" + ] + }, + "patch_url": { + "type": [ + "null", + "string" + ] + }, + "url": { + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "reactions": { + "additionalProperties": false, + "properties": { + "+1": { + "type": "integer" + }, + "-1": { + "type": "integer" + }, + "confused": { + "type": "integer" + }, + "eyes": { + "type": "integer" + }, + "heart": { + "type": "integer" + }, + "hooray": { + "type": "integer" + }, + "laugh": { + "type": "integer" + }, + "rocket": { + "type": "integer" + }, + "total_count": { + "type": "integer" + } + }, + "required": [ + "total_count", + "+1", + "-1", + "laugh", + "confused", + "heart", + "hooray", + "rocket", + "eyes" + ], + "type": [ + "null", + "object" + ] + }, + "repository_url": { + "type": [ + "null", + "string" + ] + }, + "state": { + "description": "Issue state, typically open or closed.", + "type": [ + "null", + "string" + ] + }, + "state_reason": { + "type": [ + "null", + "string" + ] + }, + "title": { + "type": [ + "null", + "string" + ] + }, + "type": { + "type": [ + "null", + "string" + ] + }, + "updated_at": { + "description": "Last update time in RFC 3339 format.", + "type": [ + "null", + "string" + ] + }, + "user": { + "additionalProperties": false, + "properties": { + "avatar_url": { + "type": "string" + }, + "details": { + "additionalProperties": false, + "properties": { + "bio": { + "type": "string" + }, + "blog": { + "type": "string" + }, + "company": { + "type": "string" + }, + "created_at": { + "description": "Account creation time in RFC3339 format.", + "type": "string" + }, + "email": { + "type": "string" + }, + "followers": { + "type": "integer" + }, + "following": { + "type": "integer" + }, + "hireable": { + "type": "boolean" + }, + "location": { + "type": "string" + }, + "name": { + "type": "string" + }, + "owned_private_repos": { + "type": "integer" + }, + "private_gists": { + "type": "integer" + }, + "public_gists": { + "type": "integer" + }, + "public_repos": { + "type": "integer" + }, + "total_private_repos": { + "type": "integer" + }, + "twitter_username": { + "type": "string" + }, + "updated_at": { + "description": "Last profile update time in RFC3339 format.", + "type": "string" + } + }, + "required": [ + "public_repos", + "public_gists", + "followers", + "following", + "created_at", + "updated_at" + ], + "type": [ + "null", + "object" + ] + }, + "id": { + "type": "integer" + }, + "login": { + "type": "string" + }, + "profile_url": { + "type": "string" + } + }, + "required": [ + "login" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "total_count": { + "description": "Total number of matching issues or pull requests.", + "type": [ + "null", + "integer" + ] + } + }, + "required": [ + "items" + ], + "type": "object" + } +} \ No newline at end of file diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 3adf308a3c..51ff97dd04 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -2146,8 +2146,8 @@ type SearchIssuesResponse struct { } type SearchIssuesOutput struct { - Total *int `json:"total_count,omitempty"` - IncompleteResults *bool `json:"incomplete_results,omitempty"` + Total *int `json:"total_count,omitempty" jsonschema:"Total number of matching issues or pull requests."` + IncompleteResults *bool `json:"incomplete_results,omitempty" jsonschema:"Whether GitHub returned an incomplete result set."` Items []SearchIssueOutputItem `json:"items"` } @@ -2155,7 +2155,7 @@ type SearchIssueOutputItem struct { Number *int `json:"number,omitempty"` Title *string `json:"title,omitempty"` Body *string `json:"body,omitempty"` - State *string `json:"state,omitempty"` + State *string `json:"state,omitempty" jsonschema:"Issue state, typically open or closed."` StateReason *string `json:"state_reason,omitempty"` Draft *bool `json:"draft,omitempty"` Locked *bool `json:"locked,omitempty"` @@ -2166,16 +2166,16 @@ type SearchIssueOutputItem struct { Assignee *string `json:"assignee,omitempty"` Assignees []string `json:"assignees,omitempty"` Milestone *string `json:"milestone,omitempty"` - Comments *int `json:"comments,omitempty"` + Comments *int `json:"comments,omitempty" jsonschema:"Number of comments on the issue."` Reactions *MinimalReactions `json:"reactions,omitempty"` - CreatedAt *string `json:"created_at,omitempty"` - UpdatedAt *string `json:"updated_at,omitempty"` - ClosedAt *string `json:"closed_at,omitempty"` + CreatedAt *string `json:"created_at,omitempty" jsonschema:"Creation time in RFC 3339 format."` + UpdatedAt *string `json:"updated_at,omitempty" jsonschema:"Last update time in RFC 3339 format."` + ClosedAt *string `json:"closed_at,omitempty" jsonschema:"Closing time in RFC 3339 format."` ClosedBy *string `json:"closed_by,omitempty"` Type *string `json:"type,omitempty"` RepositoryURL *string `json:"repository_url,omitempty"` PullRequest *SearchIssuePullRequest `json:"pull_request,omitempty"` - FieldValues []MinimalFieldValue `json:"field_values,omitempty"` + FieldValues *[]MinimalFieldValue `json:"field_values,omitempty"` } type SearchIssuePullRequest struct { @@ -2284,7 +2284,7 @@ func searchIssueOutputItem(result SearchIssueResult, fields []string) SearchIssu } } if selected("field_values") && result.FieldValues != nil { - output.FieldValues = result.FieldValues + output.FieldValues = &result.FieldValues } return output } @@ -3582,29 +3582,41 @@ type IssueFieldFilterInput struct { type ListIssuesOutput struct { Issues []ListIssueOutput `json:"issues"` - TotalCount int `json:"totalCount"` - PageInfo MinimalPageInfo `json:"pageInfo"` + TotalCount int `json:"totalCount" jsonschema:"Total number of issues matching the filters."` + PageInfo ListIssuePageInfo `json:"pageInfo"` } type ListIssueOutput struct { Number *int `json:"number,omitempty"` Title *string `json:"title,omitempty"` Body *string `json:"body,omitempty"` - State *string `json:"state,omitempty"` + State *string `json:"state,omitempty" jsonschema:"Issue state: OPEN or CLOSED."` User *MinimalUser `json:"user,omitempty"` Labels *[]string `json:"labels,omitempty"` Assignees *[]string `json:"assignees,omitempty"` - Comments *int `json:"comments,omitempty"` - CreatedAt *string `json:"created_at,omitempty"` - UpdatedAt *string `json:"updated_at,omitempty"` + Comments *int `json:"comments,omitempty" jsonschema:"Number of comments on the issue."` + CreatedAt *string `json:"created_at,omitempty" jsonschema:"Creation time in RFC 3339 format."` + UpdatedAt *string `json:"updated_at,omitempty" jsonschema:"Last update time in RFC 3339 format."` FieldValues *[]MinimalFieldValue `json:"field_values,omitempty"` } +type ListIssuePageInfo struct { + HasNextPage bool `json:"hasNextPage" jsonschema:"Whether another page of results is available."` + HasPreviousPage bool `json:"hasPreviousPage" jsonschema:"Whether a previous page of results is available."` + StartCursor string `json:"startCursor,omitempty" jsonschema:"Cursor for the first issue in this page."` + EndCursor string `json:"endCursor,omitempty" jsonschema:"Cursor for the last issue in this page."` +} + func structuredListIssuesOutput(response MinimalIssuesResponse, fields []string) ListIssuesOutput { output := ListIssuesOutput{ Issues: make([]ListIssueOutput, 0, len(response.Issues)), TotalCount: response.TotalCount, - PageInfo: response.PageInfo, + PageInfo: ListIssuePageInfo{ + HasNextPage: response.PageInfo.HasNextPage, + HasPreviousPage: response.PageInfo.HasPreviousPage, + StartCursor: response.PageInfo.StartCursor, + EndCursor: response.PageInfo.EndCursor, + }, } selected := func(field string) bool { return len(fields) == 0 || slices.Contains(fields, field) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 4cb523f65b..a8493bc521 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -1343,7 +1343,7 @@ type ListPullRequestOutput struct { Number *int `json:"number,omitempty"` Title *string `json:"title,omitempty"` Body *string `json:"body,omitempty"` - State *string `json:"state,omitempty"` + State *string `json:"state,omitempty" jsonschema:"Pull request state: open or closed."` Draft *bool `json:"draft,omitempty"` Merged *bool `json:"merged,omitempty"` MergeableState *string `json:"mergeable_state,omitempty"` @@ -1355,15 +1355,15 @@ type ListPullRequestOutput struct { 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"` + Additions *int `json:"additions,omitempty" jsonschema:"Number of lines added."` + Deletions *int `json:"deletions,omitempty" jsonschema:"Number of lines removed."` + ChangedFiles *int `json:"changed_files,omitempty" jsonschema:"Number of files changed."` + Commits *int `json:"commits,omitempty" jsonschema:"Number of commits in the pull request."` + Comments *int `json:"comments,omitempty" jsonschema:"Number of comments on the pull request."` + CreatedAt *string `json:"created_at,omitempty" jsonschema:"Creation time in RFC 3339 format."` + UpdatedAt *string `json:"updated_at,omitempty" jsonschema:"Last update time in RFC 3339 format."` + ClosedAt *string `json:"closed_at,omitempty" jsonschema:"Closing time in RFC 3339 format."` + MergedAt *string `json:"merged_at,omitempty" jsonschema:"Merge time in RFC 3339 format."` Milestone *string `json:"milestone,omitempty"` } diff --git a/pkg/github/search.go b/pkg/github/search.go index c731b2b701..80a7e577b5 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -203,17 +203,30 @@ type SearchCodeInput struct { } type SearchCodeOutput struct { - TotalCount int `json:"total_count"` - IncompleteResults bool `json:"incomplete_results"` + TotalCount int `json:"total_count" jsonschema:"Total number of matching code results."` + IncompleteResults bool `json:"incomplete_results" jsonschema:"Whether GitHub returned an incomplete result set."` Items []SearchCodeOutputItem `json:"items"` } type SearchCodeOutputItem struct { - Name *string `json:"name,omitempty"` - Path *string `json:"path,omitempty"` - SHA *string `json:"sha,omitempty"` - Repository *string `json:"repository,omitempty"` - TextMatches []*github.TextMatch `json:"text_matches,omitempty"` + Name *string `json:"name,omitempty"` + Path *string `json:"path,omitempty"` + SHA *string `json:"sha,omitempty"` + Repository *string `json:"repository,omitempty"` + TextMatches []*SearchCodeTextMatch `json:"text_matches,omitempty" jsonschema:"Matching code fragments returned by GitHub."` +} + +type SearchCodeTextMatch struct { + ObjectURL *string `json:"object_url,omitempty" jsonschema:"API URL of the matched file or object."` + ObjectType *string `json:"object_type,omitempty" jsonschema:"GitHub object type containing the match."` + Property *string `json:"property,omitempty" jsonschema:"Property containing the matched text."` + Fragment *string `json:"fragment,omitempty" jsonschema:"Source fragment containing the match."` + Matches []*SearchCodeTextMatchHit `json:"matches,omitempty" jsonschema:"Matched text and its offsets in the fragment."` +} + +type SearchCodeTextMatchHit struct { + Text *string `json:"text,omitempty" jsonschema:"Matched text."` + Indices []int `json:"indices,omitempty" jsonschema:"Start and end offsets of the match in the fragment."` } func structuredSearchCodeOutput(result MinimalCodeSearchResult, fields []string) SearchCodeOutput { @@ -240,7 +253,33 @@ func structuredSearchCodeOutput(result MinimalCodeSearchResult, fields []string) projected.Repository = new(item.Repository) } if selected("text_matches") && item.TextMatches != nil { - projected.TextMatches = item.TextMatches + projected.TextMatches = make([]*SearchCodeTextMatch, 0, len(item.TextMatches)) + for _, match := range item.TextMatches { + if match == nil { + projected.TextMatches = append(projected.TextMatches, nil) + continue + } + projectedMatch := &SearchCodeTextMatch{ + ObjectURL: match.ObjectURL, + ObjectType: match.ObjectType, + Property: match.Property, + Fragment: match.Fragment, + } + if match.Matches != nil { + projectedMatch.Matches = make([]*SearchCodeTextMatchHit, 0, len(match.Matches)) + for _, hit := range match.Matches { + if hit == nil { + projectedMatch.Matches = append(projectedMatch.Matches, nil) + continue + } + projectedMatch.Matches = append(projectedMatch.Matches, &SearchCodeTextMatchHit{ + Text: hit.Text, + Indices: hit.Indices, + }) + } + } + projected.TextMatches = append(projected.TextMatches, projectedMatch) + } } output.Items = append(output.Items, projected) } diff --git a/pkg/github/typed_search_outputs_test.go b/pkg/github/typed_search_outputs_test.go index 7818853509..24cfc77fa8 100644 --- a/pkg/github/typed_search_outputs_test.go +++ b/pkg/github/typed_search_outputs_test.go @@ -6,8 +6,11 @@ import ( "net/http" "testing" + "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v92/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -18,14 +21,18 @@ import ( func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { tools := []inventory.ServerTool{ SearchCode(translations.NullTranslationHelper), - SearchIssues(translations.NullTranslationHelper), + SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeGHES)), SearchPullRequests(translations.NullTranslationHelper), ListIssues(translations.NullTranslationHelper), ListPullRequests(translations.NullTranslationHelper), ListBranches(translations.NullTranslationHelper), ListTags(translations.NullTranslationHelper), } + for _, tool := range tools { + assert.Equal(t, []string{"repo"}, tool.ScopeAccess.Scopes, "%s must retain repository read scope", tool.Tool.Name) + } deps := BaseDeps{ + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetSearchCode: mockResponse(t, http.StatusOK, &github.CodeSearchResult{ Total: new(1), @@ -34,11 +41,38 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { Name: new("main.go"), Path: new("cmd/main.go"), SHA: new("abc123"), + Repository: &github.Repository{ + FullName: new("owner/repo"), + Private: new(false), + }, + TextMatches: []*github.TextMatch{{ + ObjectType: new("FileContent"), + Property: new("content"), + Fragment: new("func main()"), + Matches: []*github.Match{{ + Text: new("main"), + Indices: []int{5, 9}, + }}, + }}, + }}, + }), + GetSearchIssues: mockResponse(t, http.StatusOK, &github.IssuesSearchResult{ + Total: new(1), + IncompleteResults: new(false), + Issues: []*github.Issue{{ + ID: new(int64(42)), + URL: new("https://api.github.com/repos/owner/repo/issues/42"), + Number: new(42), + Title: new("A title"), + Body: new("A body"), + State: new("open"), }}, }), + GetReposBranchesByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Branch{}), GetReposTagsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.RepositoryTag{ {Name: new("v1.0.0"), Commit: &github.Commit{SHA: new("abc123")}}, }), + GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(false)}), })), } @@ -54,6 +88,7 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { list, err := session.ListTools(context.Background(), nil) require.NoError(t, err) require.Len(t, list.Tools, len(tools)) + outputSchemas := make(map[string]*jsonschema.Resolved, len(list.Tools)) for _, tool := range list.Tools { if protocolVersion == "2025-11-25" { assert.Nil(t, tool.OutputSchema, "legacy clients must not see outputSchema for %s", tool.Name) @@ -61,6 +96,10 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { } require.NotNil(t, tool.OutputSchema, "%s must publish its typed output schema", tool.Name) + require.NoError(t, toolsnaps.Test(tool.Name+"_output", mcp.Tool{ + Name: tool.Name, + OutputSchema: tool.OutputSchema, + })) schemaJSON, err := json.Marshal(tool.OutputSchema) require.NoError(t, err) var schema jsonschema.Schema @@ -68,6 +107,23 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { resolved, err := schema.Resolve(nil) require.NoError(t, err) require.NoError(t, resolved.Validate(projectedReadOutputSample(tool.Name)), "%s output must conform to its schema", tool.Name) + require.NoError(t, resolved.Validate(projectedReadNullOutputSample(tool.Name)), "%s output schema must accept nullable optional fields", tool.Name) + outputSchemas[tool.Name] = resolved + } + + branches, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "list_branches", + Arguments: map[string]any{"owner": "owner", "repo": "repo"}, + }) + require.NoError(t, err) + require.False(t, branches.IsError, branches) + assert.Equal(t, "[]", getTextResult(t, branches).Text) + assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(false)), mustMarshalJSON(t, branches.Meta["ifc"])) + if protocolVersion == inventory.ProtocolVersionMultiRoundTrip { + assert.Equal(t, "[]", mustMarshalJSON(t, branches.StructuredContent)) + require.NoError(t, outputSchemas["list_branches"].Validate(branches.StructuredContent)) + } else { + assert.Nil(t, branches.StructuredContent) } result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ @@ -77,12 +133,14 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError, result) require.Equal(t, `[{"name":"v1.0.0","sha":"abc123"}]`, getTextResult(t, result).Text) + assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(false)), mustMarshalJSON(t, result.Meta["ifc"])) if protocolVersion == "2025-11-25" { assert.Nil(t, result.StructuredContent) } else { structured, err := json.Marshal(result.StructuredContent) require.NoError(t, err) assert.JSONEq(t, getTextResult(t, result).Text, string(structured)) + require.NoError(t, outputSchemas["list_tags"].Validate(result.StructuredContent)) } result, err = session.CallTool(context.Background(), &mcp.CallToolParams{ @@ -94,28 +152,162 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { }) require.NoError(t, err) require.False(t, result.IsError, result) - assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go"}]}`, getTextResult(t, result).Text) + if protocolVersion == "2025-11-25" { + assert.Equal(t, `{"incomplete_results":false,"items":[{"name":"main.go"}],"total_count":1}`, getTextResult(t, result).Text) + } + assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelSearchIssues([]bool{false})), mustMarshalJSON(t, result.Meta["ifc"])) if protocolVersion == "2025-11-25" { assert.Nil(t, result.StructuredContent) } else { + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go"}]}`, getTextResult(t, result).Text) structured, err := json.Marshal(result.StructuredContent) require.NoError(t, err) assert.JSONEq(t, getTextResult(t, result).Text, string(structured)) + require.NoError(t, outputSchemas["search_code"].Validate(result.StructuredContent)) + } + + result, err = session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "search_code", + Arguments: map[string]any{"query": "main.go"}, + }) + require.NoError(t, err) + require.False(t, result.IsError, result) + if protocolVersion == "2025-11-25" { + assert.Equal(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go","path":"cmd/main.go","sha":"abc123","repository":"owner/repo","text_matches":[{"object_type":"FileContent","property":"content","fragment":"func main()","matches":[{"text":"main","indices":[5,9]}]}]}]}`, getTextResult(t, result).Text) + } + if protocolVersion == inventory.ProtocolVersionMultiRoundTrip { + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"name":"main.go","path":"cmd/main.go","sha":"abc123","repository":"owner/repo","text_matches":[{"object_type":"FileContent","property":"content","fragment":"func main()","matches":[{"text":"main","indices":[5,9]}]}]}]}`, getTextResult(t, result).Text) + require.NoError(t, outputSchemas["search_code"].Validate(result.StructuredContent)) + assert.JSONEq(t, getTextResult(t, result).Text, mustMarshalJSON(t, result.StructuredContent)) + } + + result, err = session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "search_issues", + Arguments: map[string]any{"query": "bug"}, + }) + require.NoError(t, err) + require.False(t, result.IsError, result) + if protocolVersion == "2025-11-25" { + assert.Equal(t, `{"total_count":1,"incomplete_results":false,"items":[{"body":"A body","id":42,"number":42,"state":"open","title":"A title","url":"https://api.github.com/repos/owner/repo/issues/42"}]}`, getTextResult(t, result).Text) + assert.Nil(t, result.StructuredContent) + } else { + assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"number":42,"title":"A title","body":"A body","state":"open"}]}`, getTextResult(t, result).Text) + assert.JSONEq(t, getTextResult(t, result).Text, mustMarshalJSON(t, result.StructuredContent)) + require.NoError(t, outputSchemas["search_issues"].Validate(result.StructuredContent)) } + + failure, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "search_code", + Arguments: map[string]any{"query": ""}, + }) + require.NoError(t, err) + require.True(t, failure.IsError) + assert.Nil(t, failure.StructuredContent, "error results must not expose typed output") + assert.Equal(t, "missing required parameter: query", getErrorResult(t, failure).Text) }) } } func projectedReadOutputSample(name string) any { switch name { - case "search_code", "search_issues", "search_pull_requests": - return map[string]any{"total_count": 0, "incomplete_results": false, "items": []any{}} + case "search_code": + return map[string]any{ + "total_count": 1, "incomplete_results": false, + "items": []any{map[string]any{ + "name": "main.go", "path": "cmd/main.go", "sha": "abc123", "repository": "owner/repo", + "text_matches": []any{map[string]any{ + "object_url": "https://api.github.com/repos/owner/repo", + "object_type": "FileContent", "property": "content", "fragment": "func main()", + "matches": []any{map[string]any{"text": "main", "indices": []any{5, 9}}}, + }}, + }}, + } + case "search_issues", "search_pull_requests": + return map[string]any{ + "total_count": 1, "incomplete_results": false, + "items": []any{map[string]any{ + "number": 42, "title": "A title", "body": "A body", "state": "open", + "state_reason": "completed", "draft": false, "locked": false, + "html_url": "https://github.com/owner/repo/issues/42", + "user": map[string]any{"login": "octocat", "profile_url": "https://github.com/octocat"}, + "author_association": "OWNER", "labels": []any{"bug"}, + "assignee": "octocat", "assignees": []any{"octocat"}, "milestone": "v1", + "comments": 3, + "reactions": map[string]any{ + "total_count": 1, "+1": 1, "-1": 0, "laugh": 0, "confused": 0, + "heart": 0, "hooray": 0, "rocket": 0, "eyes": 0, + }, + "created_at": "2026-01-02T03:04:05Z", "updated_at": "2026-01-03T03:04:05Z", + "closed_at": "2026-01-04T03:04:05Z", "closed_by": "maintainer", + "type": "Bug", "repository_url": "https://api.github.com/repos/owner/repo", + "pull_request": map[string]any{ + "url": "https://api.github.com/repos/owner/repo/pulls/42", + "html_url": "https://github.com/owner/repo/pull/42", + "diff_url": "https://github.com/owner/repo/pull/42.diff", + "patch_url": "https://github.com/owner/repo/pull/42.patch", + }, + "field_values": []any{map[string]any{"field": "Priority", "value": "P1"}}, + }}, + } case "list_issues": return map[string]any{ - "issues": []any{}, "totalCount": 0, + "issues": []any{map[string]any{ + "number": 42, "title": "A title", "body": "A body", "state": "OPEN", + "user": map[string]any{"login": "octocat"}, "labels": []any{"bug"}, + "assignees": []any{"octocat"}, "comments": 3, + "created_at": "2026-01-02T03:04:05Z", "updated_at": "2026-01-03T03:04:05Z", + "field_values": []any{map[string]any{"field": "Priority", "value": "P1"}}, + }}, + "totalCount": 1, + "pageInfo": map[string]any{ + "hasNextPage": false, "hasPreviousPage": false, + "startCursor": "start", "endCursor": "end", + }, + } + case "list_pull_requests": + return []any{map[string]any{ + "number": 42, "title": "A title", "body": "A body", "state": "open", + "draft": false, "merged": false, "mergeable_state": "clean", + "html_url": "https://github.com/owner/repo/pull/42", + "user": map[string]any{"login": "octocat"}, + "labels": []any{"bug"}, "assignees": []any{"octocat"}, + "requested_reviewers": []any{"reviewer"}, "merged_by": "maintainer", + "head": map[string]any{"ref": "feature", "sha": "abc123"}, + "base": map[string]any{"ref": "main", "sha": "def456"}, + "additions": 5, "deletions": 2, "changed_files": 1, "commits": 1, + "comments": 3, "created_at": "2026-01-02T03:04:05Z", + "updated_at": "2026-01-03T03:04:05Z", "closed_at": "2026-01-04T03:04:05Z", + "merged_at": "2026-01-04T03:04:05Z", "milestone": "v1", + }} + case "list_branches": + return []any{map[string]any{"name": "main", "sha": "abc123", "protected": true}} + case "list_tags": + return []any{map[string]any{"name": "v1.0.0", "sha": "abc123"}} + default: + panic("unexpected projected read tool: " + name) + } +} + +func projectedReadNullOutputSample(name string) any { + switch name { + case "search_code": + return map[string]any{ + "total_count": 0, "incomplete_results": false, + "items": []any{map[string]any{"name": nil}}, + } + case "search_issues", "search_pull_requests": + return map[string]any{ + "total_count": nil, "incomplete_results": nil, + "items": []any{map[string]any{"title": nil}}, + } + case "list_issues": + return map[string]any{ + "issues": []any{map[string]any{"state": nil}}, "totalCount": 0, "pageInfo": map[string]any{"hasNextPage": false, "hasPreviousPage": false}, } - case "list_pull_requests", "list_branches", "list_tags": + case "list_pull_requests": + return []any{map[string]any{"state": nil}} + case "list_branches", "list_tags": return []any{} default: panic("unexpected projected read tool: " + name) @@ -134,11 +326,38 @@ func TestTypedProjectedReadOutputsRespectFieldSelection(t *testing.T) { Total: new(1), IncompleteResults: new(false), Items: []SearchIssueResult{{ - Issue: &github.Issue{Number: new(42), Title: new("A title"), Body: new("A body")}, + Issue: &github.Issue{Number: new(42), Title: new("A title"), Body: new("A body")}, + FieldValues: []MinimalFieldValue{}, }}, }, []string{"title"}) assert.JSONEq(t, `{"total_count":1,"incomplete_results":false,"items":[{"title":"A title"}]}`, mustMarshalJSON(t, searchIssues)) + emptyFieldValues := structuredSearchIssuesOutput(SearchIssuesResponse{ + Items: []SearchIssueResult{{Issue: &github.Issue{}, FieldValues: []MinimalFieldValue{}}}, + }, []string{"field_values"}) + assert.Equal(t, `{"items":[{"field_values":[]}]}`, mustMarshalJSON(t, emptyFieldValues)) + + unavailableFieldValues := structuredSearchIssuesOutput(SearchIssuesResponse{ + Items: []SearchIssueResult{{Issue: &github.Issue{}}}, + }, []string{"field_values"}) + assert.Equal(t, `{"items":[{}]}`, mustMarshalJSON(t, unavailableFieldValues)) + + codeMatches := structuredSearchCodeOutput(MinimalCodeSearchResult{ + Items: []MinimalCodeResult{{ + TextMatches: []*github.TextMatch{{ + ObjectURL: new("https://api.github.com/repos/owner/repo"), + ObjectType: new("FileContent"), + Property: new("content"), + Fragment: new("func main()"), + Matches: []*github.Match{{ + Text: new("main"), + Indices: []int{5, 9}, + }}, + }}, + }}, + }, []string{"text_matches"}) + assert.Equal(t, `{"total_count":0,"incomplete_results":false,"items":[{"text_matches":[{"object_url":"https://api.github.com/repos/owner/repo","object_type":"FileContent","property":"content","fragment":"func main()","matches":[{"text":"main","indices":[5,9]}]}]}]}`, mustMarshalJSON(t, codeMatches)) + issues := structuredListIssuesOutput(MinimalIssuesResponse{ Issues: []MinimalIssue{{Number: 42, Title: "A title", Body: "A body", State: "OPEN", Assignees: []string{}}}, TotalCount: 1,