From f969d44a9ab2b42486b90aba50af9379596d3f5c Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 16 Jul 2026 15:49:27 +0100 Subject: [PATCH] make search_issues field value enrichment best-effort --- pkg/github/issues.go | 19 +++++++------ pkg/github/issues_test.go | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index bbccdc35d8..d1b80a00dd 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1967,9 +1967,9 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n return enrichment, nil } -// 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). +// searchIssuesHandler runs the REST issues search, enriches each hit (best-effort) 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, options ...searchOption) (*mcp.CallToolResult, error) { const errorPrefix = "failed to search issues" @@ -1996,15 +1996,18 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil } + // The field value enrichment is best-effort: a failure here (e.g. a server whose + // GraphQL schema predates the issueFieldValues field) must never fail the search. 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 - } - fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues) - if err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse(ctx, errorPrefix+": failed to fetch issue field values", err), nil + _, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, errorPrefix+": failed to get GitHub GraphQL client", err) + } else { + fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues) + if err != nil { + _, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, errorPrefix+": failed to fetch issue field values", err) + } } } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index c3ea692464..21920f2799 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -1420,6 +1420,63 @@ func Test_SearchIssues_FieldValuesEnrichment(t *testing.T) { assert.Empty(t, response.Items[1].FieldValues) } +func Test_SearchIssues_FieldValuesEnrichmentUnsupported(t *testing.T) { + // Verify search_issues still returns its REST hits when the server's GraphQL + // schema does not support the issueFieldValues enrichment. + serverTool := SearchIssues(translations.NullTranslationHelper) + + mockSearchResult := &github.IssuesSearchResult{ + Total: github.Ptr(1), + IncompleteResults: github.Ptr(false), + Issues: []*github.Issue{ + { + Number: github.Ptr(42), + Title: github.Ptr("Bug: Something is broken"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + NodeID: github.Ptr("I_node_42"), + User: &github.User{Login: github.Ptr("user1")}, + }, + }, + } + + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetSearchIssues: mockResponse(t, http.StatusOK, mockSearchResult), + }) + + gqlVars := map[string]any{ + "ids": []any{"I_node_42"}, + } + gqlResponse := githubv4mock.ErrorResponse("Field 'issueFieldValues' doesn't exist on type 'Issue'") + + const nodesQueryString = "query($ids:[ID!]!){nodes(ids: $ids){... on Issue{id,issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}}}}" + matcher := githubv4mock.NewQueryMatcher(nodesQueryString, gqlVars, gqlResponse) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) + + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "query": "repo:owner/repo is:open", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, "expected result to not be an error") + + textContent := getTextResult(t, result) + + var response SearchIssuesResponse + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + require.Equal(t, 1, *response.Total) + require.Len(t, response.Items, 1) + assert.Equal(t, 42, *response.Items[0].Number) + assert.Empty(t, response.Items[0].FieldValues) +} + func Test_CreateIssue(t *testing.T) { // Verify tool definition once serverTool := IssueWrite(translations.NullTranslationHelper)