Skip to content

Commit f041695

Browse files
kerobbiSamMorrowDrums
authored andcommitted
make search_issues field value enrichment best-effort
1 parent 4665185 commit f041695

2 files changed

Lines changed: 68 additions & 8 deletions

File tree

pkg/github/issues.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2177,9 +2177,9 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n
21772177
return enrichment, nil
21782178
}
21792179

2180-
// searchIssuesHandler runs the REST issues search, enriches each hit with custom field values
2181-
// fetched via a single follow-up GraphQL nodes() query, and applies any post-process options
2182-
// (e.g. IFC labelling).
2180+
// searchIssuesHandler runs the REST issues search, enriches each hit (best-effort) with custom
2181+
// field values fetched via a single follow-up GraphQL nodes() query, and applies any post-process
2182+
// options (e.g. IFC labelling).
21832183
func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[string]any, mode searchMode, options ...searchOption) (*mcp.CallToolResult, error) {
21842184
const errorPrefix = "failed to search issues"
21852185

@@ -2206,15 +2206,18 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st
22062206
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil
22072207
}
22082208

2209+
// The field value enrichment is best-effort: a failure here (e.g. a server whose
2210+
// GraphQL schema predates the issueFieldValues field) must never fail the search.
22092211
var fieldValuesByID map[string][]MinimalFieldValue
22102212
if len(result.Issues) > 0 {
22112213
gqlClient, err := deps.GetGQLClient(ctx)
22122214
if err != nil {
2213-
return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub GraphQL client", err), nil
2214-
}
2215-
fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues)
2216-
if err != nil {
2217-
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, errorPrefix+": failed to fetch issue field values", err), nil
2215+
_, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, errorPrefix+": failed to get GitHub GraphQL client", err)
2216+
} else {
2217+
fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues)
2218+
if err != nil {
2219+
_, _ = ghErrors.NewGitHubGraphQLErrorToCtx(ctx, errorPrefix+": failed to fetch issue field values", err)
2220+
}
22182221
}
22192222
}
22202223

pkg/github/issues_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,63 @@ func Test_SearchIssues_FieldValuesEnrichment(t *testing.T) {
16871687
assert.Empty(t, response.Items[1].FieldValues)
16881688
}
16891689

1690+
func Test_SearchIssues_FieldValuesEnrichmentUnsupported(t *testing.T) {
1691+
// Verify search_issues still returns its REST hits when the server's GraphQL
1692+
// schema does not support the issueFieldValues enrichment.
1693+
serverTool := SearchIssues(translations.NullTranslationHelper)
1694+
1695+
mockSearchResult := &github.IssuesSearchResult{
1696+
Total: github.Ptr(1),
1697+
IncompleteResults: github.Ptr(false),
1698+
Issues: []*github.Issue{
1699+
{
1700+
Number: github.Ptr(42),
1701+
Title: github.Ptr("Bug: Something is broken"),
1702+
State: github.Ptr("open"),
1703+
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
1704+
NodeID: github.Ptr("I_node_42"),
1705+
User: &github.User{Login: github.Ptr("user1")},
1706+
},
1707+
},
1708+
}
1709+
1710+
restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1711+
GetSearchIssues: mockResponse(t, http.StatusOK, mockSearchResult),
1712+
})
1713+
1714+
gqlVars := map[string]any{
1715+
"ids": []any{"I_node_42"},
1716+
}
1717+
gqlResponse := githubv4mock.ErrorResponse("Field 'issueFieldValues' doesn't exist on type 'Issue'")
1718+
1719+
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}}}}}}"
1720+
matcher := githubv4mock.NewQueryMatcher(nodesQueryString, gqlVars, gqlResponse)
1721+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1722+
1723+
deps := BaseDeps{
1724+
Client: mustNewGHClient(t, restClient),
1725+
GQLClient: gqlClient,
1726+
}
1727+
handler := serverTool.Handler(deps)
1728+
1729+
request := createMCPRequest(map[string]any{
1730+
"query": "repo:owner/repo is:open",
1731+
})
1732+
1733+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1734+
require.NoError(t, err)
1735+
require.False(t, result.IsError, "expected result to not be an error")
1736+
1737+
textContent := getTextResult(t, result)
1738+
1739+
var response SearchIssuesResponse
1740+
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response))
1741+
require.Equal(t, 1, *response.Total)
1742+
require.Len(t, response.Items, 1)
1743+
assert.Equal(t, 42, *response.Items[0].Number)
1744+
assert.Empty(t, response.Items[0].FieldValues)
1745+
}
1746+
16901747
func Test_CreateIssue(t *testing.T) {
16911748
// Verify tool definition once
16921749
serverTool := IssueWrite(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)