Skip to content

Commit b07fb53

Browse files
committed
fix: address reviewed regressions for PR #3265
1 parent c7fcdf2 commit b07fb53

3 files changed

Lines changed: 63 additions & 2 deletions

File tree

pkg/github/issues.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1878,6 +1878,10 @@ func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inve
18781878
},
18791879
Required: []string{"query"},
18801880
}
1881+
if mode == searchModeLexical {
1882+
schema.Properties["search_type"].Enum = []any{"lexical"}
1883+
schema.Properties["search_type"].Description = "Search engine. Only lexical search is supported on this host."
1884+
}
18811885
schema.Properties["fields"] = fieldsSchemaProperty(
18821886
"Subset of fields to return for each issue result. If omitted, all fields are returned. Use this to reduce response size when you only need specific fields; omitting 'body', 'reactions', and 'labels' in particular drops the largest per-result data.",
18831887
searchIssuesItemFieldEnum,
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/github/github-mcp-server/pkg/translations"
8+
"github.com/github/github-mcp-server/pkg/utils"
9+
"github.com/google/jsonschema-go/jsonschema"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func Test_SearchModeNaturalLanguageAndSyntax(t *testing.T) {
14+
for _, q := range []string{
15+
"login does not work after password reset", "connecting to postgres and redis", "should I use hooks or plugins",
16+
`why does "this AND that" fail`, `explain "foo OR bar"`, `why "NOT ready" appears`,
17+
`explain "label:bug" text`, `why "escaped \" AND operator" fails`,
18+
} {
19+
t.Run(q, func(t *testing.T) {
20+
got, err := resolveIssuesSearchMode(searchModeSemantic, map[string]any{"query": q})
21+
require.NoError(t, err)
22+
require.Equal(t, searchModeSemantic, got)
23+
})
24+
}
25+
for _, q := range []string{"foo AND bar", "foo OR bar", "foo NOT bar", "foo AND(bar OR baz)", `label:"needs triage"`, `repo:owner/repo`, `-author:bot`, `(label:bug OR label:critical)`} {
26+
t.Run(q, func(t *testing.T) {
27+
got, err := resolveIssuesSearchMode(searchModeSemantic, map[string]any{"query": q})
28+
require.NoError(t, err)
29+
require.Equal(t, searchModeLexical, got)
30+
got, err = resolveIssuesSearchMode(searchModeSemantic, map[string]any{"query": q, "search_type": "semantic"})
31+
require.NoError(t, err)
32+
require.Equal(t, searchModeSemantic, got)
33+
})
34+
}
35+
}
36+
37+
func Test_SearchIssuesGHESRejectsSemanticOverride(t *testing.T) {
38+
got, err := resolveIssuesSearchMode(searchModeLexical, map[string]any{"query": "question", "search_type": "semantic"})
39+
require.ErrorContains(t, err, "not supported")
40+
require.NotEqual(t, searchModeSemantic, got)
41+
tool := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeGHES))
42+
require.Equal(t, []any{"lexical"}, tool.Tool.InputSchema.(*jsonschema.Schema).Properties["search_type"].Enum)
43+
// No client: the capability error must be returned before attempting a request.
44+
deps := &BaseDeps{}
45+
request := createMCPRequest(map[string]any{"query": "question", "search_type": "semantic"})
46+
result, err := tool.Handler(deps)(ContextWithDeps(context.Background(), deps), &request)
47+
require.NoError(t, err)
48+
require.True(t, result.IsError)
49+
}

pkg/github/search_utils.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,24 @@ const (
8585
)
8686

8787
// booleanSearchOpPattern matches OR / AND / NOT as free-standing operators.
88-
var booleanSearchOpPattern = regexp.MustCompile(`(?i)(^|\s)(OR|AND|NOT)(\s|$)`)
88+
var booleanSearchOpPattern = regexp.MustCompile(`(^|[\s(])(OR|AND|NOT)([\s()]|$)`)
89+
90+
// quotedIssueSearchText masks literal contents while retaining a nonempty
91+
// qualifier value (label:"needs triage"). Escaped quotes stay inside the literal.
92+
var quotedIssueSearchText = regexp.MustCompile(`"(?:\\.|[^"\\])*(?:"|$)`)
8993

9094
// qualifierPattern matches GitHub search qualifiers such as label:bug or -author:octocat.
9195
var qualifierPattern = regexp.MustCompile(`(^|\s|\W)-?[\w.]+:\S+`)
9296

9397
// looksLikeLexicalIssueSearch reports whether the raw caller query uses GitHub
9498
// issues search syntax that semantic search mishandles.
9599
func looksLikeLexicalIssueSearch(query string) bool {
100+
query = quotedIssueSearchText.ReplaceAllString(query, `""`)
96101
return qualifierPattern.MatchString(query) || booleanSearchOpPattern.MatchString(query)
97102
}
98103

99104
// resolveIssuesSearchMode chooses lexical vs semantic for search_issues.
100-
// An explicit search_type wins. Otherwise GHES stays lexical, and Dotcom uses
105+
// An explicit search_type wins within the host capability boundary. Dotcom uses
101106
// lexical for scoped or syntax-like queries so keyword search matches REST.
102107
func resolveIssuesSearchMode(defaultMode searchMode, args map[string]any) (searchMode, error) {
103108
searchType, err := OptionalParam[string](args, "search_type")
@@ -110,6 +115,9 @@ func resolveIssuesSearchMode(defaultMode searchMode, args map[string]any) (searc
110115
case "lexical":
111116
return searchModeLexical, nil
112117
case "semantic":
118+
if defaultMode == searchModeLexical {
119+
return 0, fmt.Errorf("semantic issue search is not supported on this host")
120+
}
113121
return searchModeSemantic, nil
114122
default:
115123
return 0, fmt.Errorf(`invalid search_type %q: must be "lexical" or "semantic"`, searchType)

0 commit comments

Comments
 (0)