From c77ac798b78823b8ebe1ea9ff4f84ef78193bcce Mon Sep 17 00:00:00 2001 From: Kelsey Myers <52179263+kelsey-myers@users.noreply.github.com> Date: Fri, 15 May 2026 07:36:05 -0700 Subject: [PATCH 1/2] Add custom field filtering to list_issues --- README.md | 1 + pkg/github/__toolsnaps__/list_issues.snap | 32 ++ pkg/github/issues.go | 148 +++++++- pkg/github/issues_test.go | 398 +++++++++++++++++++--- 4 files changed, 521 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index e4f70b6222..603af51f4e 100644 --- a/README.md +++ b/README.md @@ -875,6 +875,7 @@ The following sets of tools are available: - **Required OAuth Scopes**: `repo` - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) + - `field_filters`: Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field. (object[], optional) - `labels`: Filter by labels (string[], optional) - `orderBy`: Order issues by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) diff --git a/pkg/github/__toolsnaps__/list_issues.snap b/pkg/github/__toolsnaps__/list_issues.snap index a4be59bb0c..293ea5cf64 100644 --- a/pkg/github/__toolsnaps__/list_issues.snap +++ b/pkg/github/__toolsnaps__/list_issues.snap @@ -18,6 +18,38 @@ ], "type": "string" }, + "field_filters": { + "description": "Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field.", + "items": { + "properties": { + "date_value": { + "description": "For date fields, the date to match (YYYY-MM-DD).", + "type": "string" + }, + "field_name": { + "description": "Name of the custom field (e.g. \"Priority\").", + "type": "string" + }, + "number_value": { + "description": "For number fields, the numeric value to match.", + "type": "number" + }, + "single_select_value": { + "description": "For single-select fields, the option name to match (e.g. \"P1\").", + "type": "string" + }, + "text_value": { + "description": "For text fields, the text value to match.", + "type": "string" + } + }, + "required": [ + "field_name" + ], + "type": "object" + }, + "type": "array" + }, "labels": { "description": "Filter by labels", "items": { diff --git a/pkg/github/issues.go b/pkg/github/issues.go index d7f6f31d0a..b20560fb8a 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -9,6 +9,7 @@ import ( "strings" "time" + ghcontext "github.com/github/github-mcp-server/pkg/context" ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/ifc" "github.com/github/github-mcp-server/pkg/inventory" @@ -199,7 +200,7 @@ type IssueQueryFragment struct { // ListIssuesQuery is the root query structure for fetching issues with optional label filtering. type ListIssuesQuery struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` IsPrivate githubv4.Boolean } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -207,7 +208,7 @@ type ListIssuesQuery struct { // ListIssuesQueryTypeWithLabels is the query structure for fetching issues with optional label filtering. type ListIssuesQueryTypeWithLabels struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` IsPrivate githubv4.Boolean } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -215,7 +216,7 @@ type ListIssuesQueryTypeWithLabels struct { // ListIssuesQueryWithSince is the query structure for fetching issues without label filtering but with since filtering. type ListIssuesQueryWithSince struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` IsPrivate githubv4.Boolean } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -223,11 +224,21 @@ type ListIssuesQueryWithSince struct { // ListIssuesQueryTypeWithLabelsWithSince is the query structure for fetching issues with both label and since filtering. type ListIssuesQueryTypeWithLabelsWithSince struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` IsPrivate githubv4.Boolean } `graphql:"repository(owner: $owner, name: $repo)"` } +// IssueFieldValueFilter mirrors the GraphQL IssueFieldValueFilter input. Exactly one typed value +// field should be set per filter (the monolith resolver rejects multiple). +type IssueFieldValueFilter struct { + FieldName githubv4.String `json:"fieldName"` + TextValue *githubv4.String `json:"textValue,omitempty"` + DateValue *githubv4.String `json:"dateValue,omitempty"` + NumberValue *githubv4.Float `json:"numberValue,omitempty"` + SingleSelectOptionValue *githubv4.String `json:"singleSelectOptionValue,omitempty"` +} + // Implement the interface for all query types func (q *ListIssuesQueryTypeWithLabels) GetIssueFragment() IssueQueryFragment { return q.Repository.Issues @@ -1569,6 +1580,36 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { Type: "string", Description: "Filter by date (ISO 8601 timestamp)", }, + "field_filters": { + Type: "array", + Description: "Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field.", + Items: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "field_name": { + Type: "string", + Description: "Name of the custom field (e.g. \"Priority\").", + }, + "single_select_value": { + Type: "string", + Description: "For single-select fields, the option name to match (e.g. \"P1\").", + }, + "text_value": { + Type: "string", + Description: "For text fields, the text value to match.", + }, + "number_value": { + Type: "number", + Description: "For number fields, the numeric value to match.", + }, + "date_value": { + Type: "string", + Description: "For date fields, the date to match (YYYY-MM-DD).", + }, + }, + Required: []string{"field_name"}, + }, + }, }, Required: []string{"owner", "repo"}, } @@ -1664,6 +1705,11 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } hasLabels := len(labels) > 0 + fieldFilters, err := parseFieldFilters(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + // Get pagination parameters and convert to GraphQL format pagination, err := OptionalCursorPaginationParams(args) if err != nil { @@ -1696,12 +1742,13 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } vars := map[string]any{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "states": states, - "orderBy": githubv4.IssueOrderField(orderBy), - "direction": githubv4.OrderDirection(direction), - "first": githubv4.Int(*paginationParams.First), + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "states": states, + "orderBy": githubv4.IssueOrderField(orderBy), + "direction": githubv4.OrderDirection(direction), + "first": githubv4.Int(*paginationParams.First), + "issueFieldValues": fieldFilters, } if paginationParams.After != nil { @@ -1726,7 +1773,11 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } issueQuery := getIssueQueryType(hasLabels, hasSince) - if err := client.Query(ctx, issueQuery, vars); err != nil { + // The list_issues query references the issue_fields-gated IssueFieldValueFilter + // input type unconditionally, so we always opt into the feature via header. This + // is a no-op once the flag is globally rolled out. + ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields") + if err := client.Query(ctxWithFeatures, issueQuery, vars); err != nil { return ghErrors.NewGitHubGraphQLErrorResponse( ctx, "failed to list issues", @@ -1752,6 +1803,81 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { }) } +// parseFieldFilters extracts the optional field_filters parameter and converts it to +// a slice of IssueFieldValueFilter for the GraphQL issueFieldValues variable. Validates that exactly one typed value is set per filter. +func parseFieldFilters(args map[string]any) ([]IssueFieldValueFilter, error) { + raw, ok := args["field_filters"] + if !ok { + return []IssueFieldValueFilter{}, 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([]IssueFieldValueFilter, 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()) + } + + filter := IssueFieldValueFilter{FieldName: githubv4.String(fieldName)} + valueCount := 0 + + // Use OptionalParamOK uniformly so type errors propagate and so that + // number_value: 0 is treated as a set value (not as absent). + if v, ok, err := OptionalParamOK[string](entry, "single_select_value"); err != nil { + return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) + } else if ok && v != "" { + filter.SingleSelectOptionValue = githubv4.NewString(githubv4.String(v)) + valueCount++ + } + if v, ok, err := OptionalParamOK[string](entry, "text_value"); err != nil { + return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) + } else if ok && v != "" { + filter.TextValue = githubv4.NewString(githubv4.String(v)) + valueCount++ + } + if v, ok, err := OptionalParamOK[string](entry, "date_value"); err != nil { + return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) + } else if ok && v != "" { + filter.DateValue = githubv4.NewString(githubv4.String(v)) + valueCount++ + } + if v, ok, err := OptionalParamOK[float64](entry, "number_value"); err != nil { + return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) + } else if ok { + n := githubv4.Float(v) + filter.NumberValue = &n + valueCount++ + } + + if valueCount == 0 { + return nil, fmt.Errorf("field_filters entry %q: exactly one of single_select_value, text_value, date_value, or number_value is required", fieldName) + } + if valueCount > 1 { + return nil, fmt.Errorf("field_filters entry %q: only one of single_select_value, text_value, date_value, or number_value can be set", fieldName) + } + + filters = append(filters, filter) + } + + return filters, nil +} + // parseISOTimestamp parses an ISO 8601 timestamp string into a time.Time object. // Returns the parsed time or an error if parsing fails. // Example formats supported: "2023-01-15T14:30:00Z", "2023-01-15" diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index c89aefb8c7..98604bf28a 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -13,6 +13,8 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/http/headers" + transportpkg "github.com/github/github-mcp-server/pkg/http/transport" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v87/github" "github.com/google/jsonschema-go/jsonschema" @@ -1468,56 +1470,63 @@ func Test_ListIssues(t *testing.T) { mockErrorRepoNotFound := githubv4mock.ErrorResponse("repository not found") - // Variables matching what GraphQL receives after JSON marshaling/unmarshaling + // Variables matching what GraphQL receives after JSON marshaling/unmarshaling. + // issueFieldValues is always sent as an (empty by default) list because the query + // declares the variable unconditionally; the server treats an empty list as no filter. varsListAll := map[string]any{ - "owner": "owner", - "repo": "repo", - "states": []any{"OPEN", "CLOSED"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "owner", + "repo": "repo", + "states": []any{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } varsOpenOnly := map[string]any{ - "owner": "owner", - "repo": "repo", - "states": []any{"OPEN"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "owner", + "repo": "repo", + "states": []any{"OPEN"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } varsClosedOnly := map[string]any{ - "owner": "owner", - "repo": "repo", - "states": []any{"CLOSED"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "owner", + "repo": "repo", + "states": []any{"CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } varsWithLabels := map[string]any{ - "owner": "owner", - "repo": "repo", - "states": []any{"OPEN", "CLOSED"}, - "labels": []any{"bug", "enhancement"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "owner", + "repo": "repo", + "states": []any{"OPEN", "CLOSED"}, + "labels": []any{"bug", "enhancement"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } varsRepoNotFound := map[string]any{ - "owner": "owner", - "repo": "nonexistent-repo", - "states": []any{"OPEN", "CLOSED"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "owner", + "repo": "nonexistent-repo", + "states": []any{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } tests := []struct { @@ -1589,8 +1598,8 @@ func Test_ListIssues(t *testing.T) { // Define the actual query strings that match the implementation issueFieldValuesSelection := "issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}" - qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" - qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -1682,6 +1691,300 @@ func Test_ListIssues(t *testing.T) { } } +func Test_ListIssues_FieldFilters(t *testing.T) { + t.Parallel() + + serverTool := ListIssues(translations.NullTranslationHelper) + + mockIssues := []map[string]any{ + { + "number": 1, + "title": "An issue", + "body": "body", + "state": "OPEN", + "databaseId": 1, + "createdAt": "2026-01-01T00:00:00Z", + "updatedAt": "2026-01-01T00:00:00Z", + "author": map[string]any{"login": "user1"}, + "labels": map[string]any{"nodes": []map[string]any{}}, + "comments": map[string]any{"totalCount": 0}, + }, + } + + pageInfo := map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + } + + response := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issues": map[string]any{ + "nodes": mockIssues, + "pageInfo": pageInfo, + "totalCount": 1, + }, + "isPrivate": false, + }, + }) + + qNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + + baseVars := func() map[string]any { + return map[string]any{ + "owner": "owner", + "repo": "repo", + "states": []any{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + } + + t.Run("single select field filter", func(t *testing.T) { + vars := baseVars() + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, + } + matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("text field filter combined with labels", func(t *testing.T) { + vars := baseVars() + vars["labels"] = []any{"bug"} + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Notes", "textValue": "needs triage"}, + } + matcher := githubv4mock.NewQueryMatcher(qWithLabels, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "labels": []any{"bug"}, + "field_filters": []any{ + map[string]any{"field_name": "Notes", "text_value": "needs triage"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("number and date field filters", func(t *testing.T) { + vars := baseVars() + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Estimate", "numberValue": float64(2.5)}, + map[string]any{"fieldName": "Due", "dateValue": "2026-06-01"}, + } + matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Estimate", "number_value": 2.5}, + map[string]any{"field_name": "Due", "date_value": "2026-06-01"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("validation error when no value provided", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, "exactly one of") + }) + + t.Run("validation error when multiple values provided", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "single_select_value": "P1", "text_value": "high"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, "only one of") + }) + + t.Run("validation error when field_name missing", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"single_select_value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, "field_name") + }) + + // Query string fragments for the `since` variants. Built by string concatenation + // because they only differ from the base variants by the variable declaration and + // the filterBy clause. + qNoLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qNoLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] + qLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qWithLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] + + t.Run("number_value of zero is treated as set", func(t *testing.T) { + vars := baseVars() + zero := float64(0) + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Estimate", "numberValue": zero}, + } + matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Estimate", "number_value": zero}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("field filter with since", func(t *testing.T) { + vars := baseVars() + vars["since"] = "2026-01-01T00:00:00Z" + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, + } + matcher := githubv4mock.NewQueryMatcher(qNoLabelsWithSince, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "since": "2026-01-01T00:00:00Z", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("field filter with labels and since", func(t *testing.T) { + vars := baseVars() + vars["labels"] = []any{"bug"} + vars["since"] = "2026-01-01T00:00:00Z" + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, + } + matcher := githubv4mock.NewQueryMatcher(qLabelsWithSince, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "labels": []any{"bug"}, + "since": "2026-01-01T00:00:00Z", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + + t.Run("sends GraphQL-Features: issue_fields header", func(t *testing.T) { + vars := baseVars() + vars["issueFieldValues"] = []any{} + matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) + + // Build a transport chain matching production: GraphQLFeaturesTransport + // wraps a header-capturing spy, which forwards to the mock's RoundTripper. + // This verifies the handler sets the issue_fields context value and the + // transport translates it into the outgoing header. + mockClient := githubv4mock.NewMockedHTTPClient(matcher) + spy := &headerCaptureTransport{inner: mockClient.Transport} + httpClient := &http.Client{ + Transport: &transportpkg.GraphQLFeaturesTransport{Transport: spy}, + } + gqlClient := githubv4.NewClient(httpClient) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"}) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + assert.Equal(t, "issue_fields", spy.captured.Get(headers.GraphQLFeaturesHeader)) + }) +} + +// headerCaptureTransport records the headers of the most recent request that passed +// through it before forwarding to the inner RoundTripper. +type headerCaptureTransport struct { + inner http.RoundTripper + captured http.Header +} + +func (t *headerCaptureTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.captured = req.Header.Clone() + return t.inner.RoundTrip(req) +} + func Test_ListIssues_IFC_InsidersMode(t *testing.T) { t.Parallel() @@ -1722,16 +2025,17 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) { }) } - query := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + query := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" vars := map[string]any{ - "owner": "octocat", - "repo": "hello", - "states": []any{"OPEN", "CLOSED"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), + "owner": "octocat", + "repo": "hello", + "states": []any{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + "issueFieldValues": []any{}, } reqParams := map[string]any{"owner": "octocat", "repo": "hello"} From 879cb46b0543af2e2a37e43da89563f1d4a8703f Mon Sep 17 00:00:00 2001 From: Kelsey Myers <52179263+kelsey-myers@users.noreply.github.com> Date: Wed, 20 May 2026 08:16:43 -0700 Subject: [PATCH 2/2] Flatten schema --- README.md | 2 +- pkg/github/__toolsnaps__/list_issues.snap | 23 +-- pkg/github/issue_fields.go | 237 ++++++++++++++++++++++ pkg/github/issues.go | 158 +++++++++------ pkg/github/issues_test.go | 189 +++++++++++++---- 5 files changed, 496 insertions(+), 113 deletions(-) create mode 100644 pkg/github/issue_fields.go diff --git a/README.md b/README.md index 603af51f4e..ce21ee2737 100644 --- a/README.md +++ b/README.md @@ -875,7 +875,7 @@ The following sets of tools are available: - **Required OAuth Scopes**: `repo` - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) - - `field_filters`: Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field. (object[], optional) + - `field_filters`: Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date). (object[], optional) - `labels`: Filter by labels (string[], optional) - `orderBy`: Order issues by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) diff --git a/pkg/github/__toolsnaps__/list_issues.snap b/pkg/github/__toolsnaps__/list_issues.snap index 293ea5cf64..b1d1c7a21d 100644 --- a/pkg/github/__toolsnaps__/list_issues.snap +++ b/pkg/github/__toolsnaps__/list_issues.snap @@ -19,32 +19,21 @@ "type": "string" }, "field_filters": { - "description": "Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field.", + "description": "Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date).", "items": { "properties": { - "date_value": { - "description": "For date fields, the date to match (YYYY-MM-DD).", - "type": "string" - }, "field_name": { - "description": "Name of the custom field (e.g. \"Priority\").", - "type": "string" - }, - "number_value": { - "description": "For number fields, the numeric value to match.", - "type": "number" - }, - "single_select_value": { - "description": "For single-select fields, the option name to match (e.g. \"P1\").", + "description": "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", "type": "string" }, - "text_value": { - "description": "For text fields, the text value to match.", + "value": { + "description": "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", "type": "string" } }, "required": [ - "field_name" + "field_name", + "value" ], "type": "object" }, diff --git a/pkg/github/issue_fields.go b/pkg/github/issue_fields.go new file mode 100644 index 0000000000..4860f515de --- /dev/null +++ b/pkg/github/issue_fields.go @@ -0,0 +1,237 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/scopes" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" +) + +// IssueField represents a repository issue field definition. +type IssueField struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + DataType string `json:"data_type"` + Visibility string `json:"visibility"` + Options []IssueSingleSelectFieldOption `json:"options,omitempty"` +} + +// IssueSingleSelectFieldOption represents an option for a single_select issue field. +type IssueSingleSelectFieldOption struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description,omitempty"` + Color string `json:"color"` + Priority *int `json:"priority,omitempty"` +} + +// issueFieldNode is the GraphQL fragment for a single issue field in the IssueFields union. +// Only the fragment matching __typename is populated; read from the matching fragment. +type issueFieldNode struct { + TypeName githubv4.String `graphql:"__typename"` + IssueFieldText struct { + ID githubv4.ID + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String + } `graphql:"... on IssueFieldText"` + IssueFieldNumber struct { + ID githubv4.ID + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String + } `graphql:"... on IssueFieldNumber"` + IssueFieldDate struct { + ID githubv4.ID + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String + } `graphql:"... on IssueFieldDate"` + IssueFieldSingleSelect struct { + ID githubv4.ID + Name githubv4.String + Description githubv4.String + DataType githubv4.String + Visibility githubv4.String + Options []struct { + ID githubv4.ID + Name githubv4.String + Description githubv4.String + Color githubv4.String + Priority *int + } + } `graphql:"... on IssueFieldSingleSelect"` +} + +// issueFieldsRepoQuery is the GraphQL query for listing issue fields on a repository. +// The monolith enforces ORGANIZATION_ISSUE_FIELDS_LIMIT = 25 fields per organization +type issueFieldsRepoQuery struct { + Repository struct { + IssueFields struct { + Nodes []issueFieldNode + } `graphql:"issueFields(first: 25)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +// issueFieldsOrgQuery is the GraphQL query for listing issue fields on an organization. +type issueFieldsOrgQuery struct { + Organization struct { + IssueFields struct { + Nodes []issueFieldNode + } `graphql:"issueFields(first: 25)"` + } `graphql:"organization(login: $login)"` +} + +// ListIssueFields creates a tool to list issue field definitions for a repository or organization. +func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataIssues, + mcp.Tool{ + Name: "list_issue_fields", + Description: t("TOOL_LIST_ISSUE_FIELDS_DESCRIPTION", "List issue fields for a repository or organization. Returns field definitions including name, type (text, number, date, single_select), and for single_select fields the list of valid option names. When repo is omitted, returns org-level fields directly."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_LIST_ISSUE_FIELDS_USER_TITLE", "List issue fields"), + ReadOnlyHint: true, + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "The account owner of the repository or organization. The name is not case sensitive.", + }, + "repo": { + Type: "string", + Description: "The name of the repository. When provided, returns fields for this specific repository (inherited from its organization). When omitted, returns org-level fields directly.", + }, + }, + Required: []string{"owner"}, + }, + }, + []scopes.Scope{scopes.Repo, scopes.ReadOrg}, + 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 := OptionalParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), nil, nil + } + + fields, err := fetchIssueFields(ctx, gqlClient, owner, repo) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to list issue fields", err), nil, nil + } + + r, err := json.Marshal(fields) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal issue fields", err), nil, nil + } + + return utils.NewToolResultText(string(r)), nil, nil + }) +} + +// fetchIssueFields returns the issue field definitions for the given owner. +// If repo is provided, fields are scoped to that repository (inherited from its +// organization); otherwise fields are returned directly from the organization. +func fetchIssueFields(ctx context.Context, gqlClient *githubv4.Client, owner, repo string) ([]IssueField, error) { + ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields") + if repo != "" { + var query issueFieldsRepoQuery + vars := map[string]any{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + } + if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { + return nil, err + } + return issueFieldsFromNodes(query.Repository.IssueFields.Nodes), nil + } + + var query issueFieldsOrgQuery + vars := map[string]any{ + "login": githubv4.String(owner), + } + if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { + return nil, err + } + return issueFieldsFromNodes(query.Organization.IssueFields.Nodes), nil +} + +// issueFieldsFromNodes converts GraphQL issue field union nodes into IssueField values. +// Read from the fragment matching __typename; the other fragments are zero-valued. +func issueFieldsFromNodes(nodes []issueFieldNode) []IssueField { + fields := make([]IssueField, 0, len(nodes)) + for _, node := range nodes { + var f IssueField + switch string(node.TypeName) { + case "IssueFieldSingleSelect": + opts := make([]IssueSingleSelectFieldOption, 0, len(node.IssueFieldSingleSelect.Options)) + for _, o := range node.IssueFieldSingleSelect.Options { + opts = append(opts, IssueSingleSelectFieldOption{ + ID: fmt.Sprintf("%v", o.ID), + Name: string(o.Name), + Description: string(o.Description), + Color: string(o.Color), + Priority: o.Priority, + }) + } + f = IssueField{ + ID: fmt.Sprintf("%v", node.IssueFieldSingleSelect.ID), + Name: string(node.IssueFieldSingleSelect.Name), + Description: string(node.IssueFieldSingleSelect.Description), + DataType: string(node.IssueFieldSingleSelect.DataType), + Visibility: string(node.IssueFieldSingleSelect.Visibility), + Options: opts, + } + case "IssueFieldText": + f = IssueField{ + ID: fmt.Sprintf("%v", node.IssueFieldText.ID), + Name: string(node.IssueFieldText.Name), + Description: string(node.IssueFieldText.Description), + DataType: string(node.IssueFieldText.DataType), + Visibility: string(node.IssueFieldText.Visibility), + } + case "IssueFieldNumber": + f = IssueField{ + ID: fmt.Sprintf("%v", node.IssueFieldNumber.ID), + Name: string(node.IssueFieldNumber.Name), + Description: string(node.IssueFieldNumber.Description), + DataType: string(node.IssueFieldNumber.DataType), + Visibility: string(node.IssueFieldNumber.Visibility), + } + case "IssueFieldDate": + f = IssueField{ + ID: fmt.Sprintf("%v", node.IssueFieldDate.ID), + Name: string(node.IssueFieldDate.Name), + Description: string(node.IssueFieldDate.Description), + DataType: string(node.IssueFieldDate.DataType), + Visibility: string(node.IssueFieldDate.Visibility), + } + default: + continue + } + fields = append(fields, f) + } + return fields +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index b20560fb8a..88b59c49d4 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strconv" "strings" "time" @@ -1582,32 +1583,20 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { }, "field_filters": { Type: "array", - Description: "Filter by custom issue field values. Each entry must specify field_name and exactly one typed value field.", + Description: "Filter by custom issue field values. Each entry takes a field_name and a value; the server looks up the field and coerces the value to its type (single-select option name, text, number, or YYYY-MM-DD date).", Items: &jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ "field_name": { Type: "string", - Description: "Name of the custom field (e.g. \"Priority\").", + Description: "Name of the custom field (e.g. \"Priority\"). Case-insensitive.", }, - "single_select_value": { + "value": { Type: "string", - Description: "For single-select fields, the option name to match (e.g. \"P1\").", - }, - "text_value": { - Type: "string", - Description: "For text fields, the text value to match.", - }, - "number_value": { - Type: "number", - Description: "For number fields, the numeric value to match.", - }, - "date_value": { - Type: "string", - Description: "For date fields, the date to match (YYYY-MM-DD).", + Description: "Value to filter on. For single-select fields, the option name (e.g. \"P1\"). For dates, YYYY-MM-DD. For numbers, the numeric value as a string. For text, the text value.", }, }, - Required: []string{"field_name"}, + Required: []string{"field_name", "value"}, }, }, }, @@ -1705,7 +1694,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } hasLabels := len(labels) > 0 - fieldFilters, err := parseFieldFilters(args) + rawFilters, err := parseRawFieldFilters(args) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } @@ -1741,6 +1730,20 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil, nil } + // Resolve field filters by looking up the repo's issue fields so we can + // coerce each value into the right typed slot on IssueFieldValueFilter. + fieldFilters := []IssueFieldValueFilter{} + 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 + } + fieldFilters, err = resolveFieldFilters(rawFilters, fields) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + vars := map[string]any{ "owner": githubv4.String(owner), "repo": githubv4.String(repo), @@ -1803,12 +1806,19 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { }) } -// parseFieldFilters extracts the optional field_filters parameter and converts it to -// a slice of IssueFieldValueFilter for the GraphQL issueFieldValues variable. Validates that exactly one typed value is set per filter. -func parseFieldFilters(args map[string]any) ([]IssueFieldValueFilter, error) { +// rawFieldFilter is the user-supplied {field_name, value} pair before type resolution. +type rawFieldFilter struct { + Name string + 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 []IssueFieldValueFilter{}, nil + return nil, nil } var entries []map[string]any @@ -1827,55 +1837,83 @@ func parseFieldFilters(args map[string]any) ([]IssueFieldValueFilter, error) { return nil, fmt.Errorf("field_filters must be an array") } - filters := make([]IssueFieldValueFilter, 0, len(entries)) + 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()) } - - filter := IssueFieldValueFilter{FieldName: githubv4.String(fieldName)} - valueCount := 0 - - // Use OptionalParamOK uniformly so type errors propagate and so that - // number_value: 0 is treated as a set value (not as absent). - if v, ok, err := OptionalParamOK[string](entry, "single_select_value"); err != nil { - return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) - } else if ok && v != "" { - filter.SingleSelectOptionValue = githubv4.NewString(githubv4.String(v)) - valueCount++ - } - if v, ok, err := OptionalParamOK[string](entry, "text_value"); err != nil { - return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) - } else if ok && v != "" { - filter.TextValue = githubv4.NewString(githubv4.String(v)) - valueCount++ - } - if v, ok, err := OptionalParamOK[string](entry, "date_value"); err != nil { - return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) - } else if ok && v != "" { - filter.DateValue = githubv4.NewString(githubv4.String(v)) - valueCount++ - } - if v, ok, err := OptionalParamOK[float64](entry, "number_value"); err != nil { + value, err := RequiredParam[string](entry, "value") + if err != nil { return nil, fmt.Errorf("field_filters entry %q: %s", fieldName, err.Error()) - } else if ok { - n := githubv4.Float(v) - filter.NumberValue = &n - valueCount++ } + filters = append(filters, rawFieldFilter{Name: fieldName, Value: value}) + } + return filters, nil +} - if valueCount == 0 { - return nil, fmt.Errorf("field_filters entry %q: exactly one of single_select_value, text_value, date_value, or number_value is required", fieldName) - } - if valueCount > 1 { - return nil, fmt.Errorf("field_filters entry %q: only one of single_select_value, text_value, date_value, or number_value can be set", fieldName) +// 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 +// single-select fields. +func resolveFieldFilters(rawFilters []rawFieldFilter, fields []IssueField) ([]IssueFieldValueFilter, error) { + byName := make(map[string]IssueField, len(fields)) + knownNames := make([]string, 0, len(fields)) + for _, f := range fields { + byName[strings.ToLower(f.Name)] = f + knownNames = append(knownNames, f.Name) + } + + out := make([]IssueFieldValueFilter, 0, len(rawFilters)) + for _, rf := range rawFilters { + field, ok := byName[strings.ToLower(rf.Name)] + if !ok { + return nil, fmt.Errorf("field_filters: unknown field %q. Known fields: %s", rf.Name, strings.Join(knownNames, ", ")) } - filters = append(filters, filter) + filter := IssueFieldValueFilter{FieldName: githubv4.String(field.Name)} + switch field.DataType { + case "SINGLE_SELECT": + // Validate the option name against the field's options so we fail fast + // with a useful error instead of an opaque GraphQL one. + var matched string + for _, o := range field.Options { + if strings.EqualFold(o.Name, rf.Value) { + matched = o.Name + break + } + } + if matched == "" { + optionNames := make([]string, 0, len(field.Options)) + for _, o := range field.Options { + optionNames = append(optionNames, o.Name) + } + return nil, fmt.Errorf("field_filters: %q is not a valid option for %q. Valid options: %s", rf.Value, field.Name, strings.Join(optionNames, ", ")) + } + v := githubv4.String(matched) + filter.SingleSelectOptionValue = &v + case "TEXT": + v := githubv4.String(rf.Value) + filter.TextValue = &v + case "DATE": + if _, err := time.Parse("2006-01-02", rf.Value); err != nil { + return nil, fmt.Errorf("field_filters: %q is not a valid date for %q (expected YYYY-MM-DD): %s", rf.Value, field.Name, err.Error()) + } + v := githubv4.String(rf.Value) + filter.DateValue = &v + case "NUMBER": + n, err := strconv.ParseFloat(rf.Value, 64) + if err != nil { + return nil, fmt.Errorf("field_filters: %q is not a valid number for %q: %s", rf.Value, field.Name, err.Error()) + } + v := githubv4.Float(n) + filter.NumberValue = &v + default: + return nil, fmt.Errorf("field_filters: field %q has unsupported data_type %q", field.Name, field.DataType) + } + out = append(out, filter) } - - return filters, nil + return out, nil } // parseISOTimestamp parses an ISO 8601 timestamp string into a time.Time object. diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 98604bf28a..6fe2c3f376 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1729,6 +1729,59 @@ func Test_ListIssues_FieldFilters(t *testing.T) { }, }) + // Field-lookup matcher used by every subtest that supplies field_filters. + // The handler calls fetchIssueFields(owner, repo) before issuing the issues query. + fieldsResponse := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "id": "IFSS_1", + "name": "Priority", + "dataType": "SINGLE_SELECT", + "visibility": "ALL", + "options": []any{ + map[string]any{"id": "OPT_P1", "name": "P1", "color": "red"}, + map[string]any{"id": "OPT_P2", "name": "P2", "color": "yellow"}, + }, + }, + map[string]any{ + "__typename": "IssueFieldText", + "id": "IFT_1", + "name": "Notes", + "dataType": "TEXT", + "visibility": "ALL", + }, + map[string]any{ + "__typename": "IssueFieldNumber", + "id": "IFN_1", + "name": "Estimate", + "dataType": "NUMBER", + "visibility": "ALL", + }, + map[string]any{ + "__typename": "IssueFieldDate", + "id": "IFD_1", + "name": "Due", + "dataType": "DATE", + "visibility": "ALL", + }, + }, + }, + }, + }) + fieldsMatcher := func() githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + issueFieldsRepoQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + }, + fieldsResponse, + ) + } + qNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name},... on IssueFieldNumber{name},... on IssueFieldSingleSelect{name},... on IssueFieldText{name}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" @@ -1750,7 +1803,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, } matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1758,7 +1811,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "owner": "owner", "repo": "repo", "field_filters": []any{ - map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + map[string]any{"field_name": "Priority", "value": "P1"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) @@ -1773,7 +1826,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { map[string]any{"fieldName": "Notes", "textValue": "needs triage"}, } matcher := githubv4mock.NewQueryMatcher(qWithLabels, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1782,7 +1835,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "repo": "repo", "labels": []any{"bug"}, "field_filters": []any{ - map[string]any{"field_name": "Notes", "text_value": "needs triage"}, + map[string]any{"field_name": "Notes", "value": "needs triage"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) @@ -1797,7 +1850,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { map[string]any{"fieldName": "Due", "dateValue": "2026-06-01"}, } matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1805,8 +1858,8 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "owner": "owner", "repo": "repo", "field_filters": []any{ - map[string]any{"field_name": "Estimate", "number_value": 2.5}, - map[string]any{"field_name": "Due", "date_value": "2026-06-01"}, + map[string]any{"field_name": "Estimate", "value": "2.5"}, + map[string]any{"field_name": "Due", "value": "2026-06-01"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) @@ -1814,7 +1867,33 @@ func Test_ListIssues_FieldFilters(t *testing.T) { require.False(t, res.IsError, getTextResult(t, res).Text) }) - t.Run("validation error when no value provided", func(t *testing.T) { + t.Run("number field accepts zero values", func(t *testing.T) { + for _, value := range []string{"0", "0.0"} { + t.Run(value, func(t *testing.T) { + vars := baseVars() + vars["issueFieldValues"] = []any{ + map[string]any{"fieldName": "Estimate", "numberValue": float64(0)}, + } + matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Estimate", "value": value}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + }) + } + }) + + t.Run("validation error when value missing", func(t *testing.T) { gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1829,10 +1908,13 @@ func Test_ListIssues_FieldFilters(t *testing.T) { res, err := handler(ContextWithDeps(context.Background(), deps), &req) require.NoError(t, err) require.True(t, res.IsError) - assert.Contains(t, getTextResult(t, res).Text, "exactly one of") + text := getTextResult(t, res).Text + assert.Contains(t, text, "field_filters entry") + assert.Contains(t, text, "Priority") + assert.Contains(t, text, "value") }) - t.Run("validation error when multiple values provided", func(t *testing.T) { + t.Run("validation error when field_name missing", func(t *testing.T) { gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1841,17 +1923,19 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "owner": "owner", "repo": "repo", "field_filters": []any{ - map[string]any{"field_name": "Priority", "single_select_value": "P1", "text_value": "high"}, + map[string]any{"value": "P1"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) require.NoError(t, err) require.True(t, res.IsError) - assert.Contains(t, getTextResult(t, res).Text, "only one of") + text := getTextResult(t, res).Text + assert.Contains(t, text, "field_filters entry") + assert.Contains(t, text, "field_name") }) - t.Run("validation error when field_name missing", func(t *testing.T) { - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher("", nil, response))) + t.Run("error when field is unknown", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher())) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1859,29 +1943,39 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "owner": "owner", "repo": "repo", "field_filters": []any{ - map[string]any{"single_select_value": "P1"}, + map[string]any{"field_name": "NotARealField", "value": "x"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) require.NoError(t, err) require.True(t, res.IsError) - assert.Contains(t, getTextResult(t, res).Text, "field_name") + text := getTextResult(t, res).Text + assert.Contains(t, text, "unknown field") + assert.Contains(t, text, "Priority") }) - // Query string fragments for the `since` variants. Built by string concatenation - // because they only differ from the base variants by the variable declaration and - // the filterBy clause. - qNoLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qNoLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] - qLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qWithLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] + t.Run("error when single-select option is invalid", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher())) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) - t.Run("number_value of zero is treated as set", func(t *testing.T) { - vars := baseVars() - zero := float64(0) - vars["issueFieldValues"] = []any{ - map[string]any{"fieldName": "Estimate", "numberValue": zero}, - } - matcher := githubv4mock.NewQueryMatcher(qNoLabels, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "value": "P9"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + text := getTextResult(t, res).Text + assert.Contains(t, text, "not a valid option") + assert.Contains(t, text, "P1") + }) + + t.Run("error when number value is non-numeric", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher())) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1889,14 +1983,39 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "owner": "owner", "repo": "repo", "field_filters": []any{ - map[string]any{"field_name": "Estimate", "number_value": zero}, + map[string]any{"field_name": "Estimate", "value": "not-a-number"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) require.NoError(t, err) - require.False(t, res.IsError, getTextResult(t, res).Text) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, "not a valid number") }) + t.Run("error when date value is malformed", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher())) + deps := BaseDeps{GQLClient: gqlClient} + handler := serverTool.Handler(deps) + + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Due", "value": "06/01/2026"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, "not a valid date") + }) + + // Query string fragments for the `since` variants. Built by string concatenation + // because they only differ from the base variants by the variable declaration and + // the filterBy clause. + qNoLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qNoLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] + qLabelsWithSince := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})" + qWithLabels[len("query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"):] + t.Run("field filter with since", func(t *testing.T) { vars := baseVars() vars["since"] = "2026-01-01T00:00:00Z" @@ -1904,7 +2023,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, } matcher := githubv4mock.NewQueryMatcher(qNoLabelsWithSince, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1913,7 +2032,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "repo": "repo", "since": "2026-01-01T00:00:00Z", "field_filters": []any{ - map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + map[string]any{"field_name": "Priority", "value": "P1"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req) @@ -1929,7 +2048,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { map[string]any{"fieldName": "Priority", "singleSelectOptionValue": "P1"}, } matcher := githubv4mock.NewQueryMatcher(qLabelsWithSince, vars, response) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(fieldsMatcher(), matcher)) deps := BaseDeps{GQLClient: gqlClient} handler := serverTool.Handler(deps) @@ -1939,7 +2058,7 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "labels": []any{"bug"}, "since": "2026-01-01T00:00:00Z", "field_filters": []any{ - map[string]any{"field_name": "Priority", "single_select_value": "P1"}, + map[string]any{"field_name": "Priority", "value": "P1"}, }, }) res, err := handler(ContextWithDeps(context.Background(), deps), &req)