Skip to content

Commit ee37dd8

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

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
@@ -2180,9 +2180,9 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n
21802180
return enrichment, nil
21812181
}
21822182

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

@@ -2209,15 +2209,18 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[st
22092209
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil
22102210
}
22112211

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

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)