Make search_issues semantic by default (#2964)
* Make search_issues semantic by default * initialize description depending on the host --------- Co-authored-by: Iulia B <iulia-b@github.com> Co-authored-by: Iulia Bejan <64602043+iulia-b@users.noreply.github.com>
This commit is contained in:
@@ -976,7 +976,7 @@ The following sets of tools are available:
|
||||
- `owner`: Optional repository owner. If provided with repo, only issues for this repository are listed. (string, optional)
|
||||
- `page`: Page number for pagination (min 1) (number, optional)
|
||||
- `perPage`: Results per page for pagination (min 1, max 100) (number, optional)
|
||||
- `query`: Search query using GitHub issues search syntax (string, required)
|
||||
- `query`: The search query, as natural language. When the user gives alternative wordings, include them as plain words rather than joining them with OR. (string, required)
|
||||
- `repo`: Optional repository name. If provided with owner, only issues for this repository are listed. (string, optional)
|
||||
- `sort`: Sort field by number of matches of categories, defaults to best match (string, optional)
|
||||
|
||||
|
||||
@@ -138,6 +138,11 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
|
||||
return nil, fmt.Errorf("failed to parse API host: %w", err)
|
||||
}
|
||||
|
||||
hostType, err := utils.ParseHostType(cfg.Host)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to classify API host: %w", err)
|
||||
}
|
||||
|
||||
clients, err := createGitHubClients(cfg, apiHost)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create GitHub clients: %w", err)
|
||||
@@ -165,7 +170,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
|
||||
obs,
|
||||
)
|
||||
// Build and register the tool/resource/prompt inventory
|
||||
inventoryBuilder := github.NewInventory(cfg.Translator).
|
||||
inventoryBuilder := github.NewInventory(cfg.Translator, github.WithHost(hostType)).
|
||||
WithDeprecatedAliases(github.DeprecatedToolAliases).
|
||||
WithReadOnly(cfg.ReadOnly).
|
||||
WithToolsets(github.ResolvedEnabledToolsets(cfg.EnabledToolsets, cfg.EnabledTools)).
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
"readOnlyHint": true,
|
||||
"title": "Search issues"
|
||||
},
|
||||
"description": "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue",
|
||||
"description": "Search issues using natural-language semantic matching. Best for conceptual or paraphrased queries (e.g. \"login fails after password reset\"). Already scoped to is:issue.",
|
||||
"inputSchema": {
|
||||
"properties": {
|
||||
"fields": {
|
||||
@@ -64,7 +64,7 @@
|
||||
"type": "number"
|
||||
},
|
||||
"query": {
|
||||
"description": "Search query using GitHub issues search syntax",
|
||||
"description": "The search query, as natural language. When the user gives alternative wordings, include them as plain words rather than joining them with OR.",
|
||||
"type": "string"
|
||||
},
|
||||
"repo": {
|
||||
|
||||
@@ -10,9 +10,9 @@ import (
|
||||
// This function is stateless - no dependencies are captured.
|
||||
// Handlers are generated on-demand during registration via RegisterAll(ctx, server, deps).
|
||||
// The "default" keyword in WithToolsets will expand to toolsets marked with Default: true.
|
||||
func NewInventory(t translations.TranslationHelperFunc) *inventory.Builder {
|
||||
func NewInventory(t translations.TranslationHelperFunc, opts ...ToolOption) *inventory.Builder {
|
||||
return inventory.NewBuilder().
|
||||
SetTools(AllTools(t)).
|
||||
SetTools(AllTools(t, opts...)).
|
||||
SetResources(AllResources(t)).
|
||||
SetPrompts(AllPrompts(t))
|
||||
}
|
||||
|
||||
+35
-6
@@ -1610,14 +1610,43 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri
|
||||
return utils.NewToolResultText(string(r)), nil
|
||||
}
|
||||
|
||||
// The two search engines want opposite things from a caller, so steering advice
|
||||
// for one is counterproductive for the other: semantic rewards paraphrased
|
||||
// natural language and degrades on boolean operators, while lexical needs the
|
||||
// caller's literal keywords and handles OR fine. The description has to describe
|
||||
// the engine the host will actually use.
|
||||
const (
|
||||
searchIssuesSemanticDescription = "Search issues using natural-language semantic matching. Best for conceptual or paraphrased queries (e.g. \"login fails after password reset\"). Already scoped to is:issue."
|
||||
searchIssuesLexicalDescription = "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue"
|
||||
|
||||
searchIssuesSemanticQueryDescription = "The search query, as natural language. When the user gives alternative wordings, include them as plain words rather than joining them with OR."
|
||||
searchIssuesLexicalQueryDescription = "Search query using GitHub issues search syntax"
|
||||
)
|
||||
|
||||
// SearchIssues creates a tool to search for issues.
|
||||
func SearchIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
|
||||
func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inventory.ServerTool {
|
||||
cfg := newToolConfig(opts)
|
||||
|
||||
// Semantic is the default; however as it is not available on GHES, we fall back to
|
||||
// lexical search for that host type.
|
||||
mode := searchModeSemantic
|
||||
if cfg.hostType == utils.HostTypeGHES {
|
||||
mode = searchModeLexical
|
||||
}
|
||||
|
||||
toolDescription := searchIssuesSemanticDescription
|
||||
queryDescription := searchIssuesSemanticQueryDescription
|
||||
if mode == searchModeLexical {
|
||||
toolDescription = searchIssuesLexicalDescription
|
||||
queryDescription = searchIssuesLexicalQueryDescription
|
||||
}
|
||||
|
||||
schema := &jsonschema.Schema{
|
||||
Type: "object",
|
||||
Properties: map[string]*jsonschema.Schema{
|
||||
"query": {
|
||||
Type: "string",
|
||||
Description: "Search query using GitHub issues search syntax",
|
||||
Description: queryDescription,
|
||||
},
|
||||
"owner": {
|
||||
Type: "string",
|
||||
@@ -1662,7 +1691,7 @@ func SearchIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
|
||||
ToolsetMetadataIssues,
|
||||
mcp.Tool{
|
||||
Name: "search_issues",
|
||||
Description: t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue"),
|
||||
Description: t("TOOL_SEARCH_ISSUES_DESCRIPTION", toolDescription),
|
||||
Annotations: &mcp.ToolAnnotations{
|
||||
Title: t("TOOL_SEARCH_ISSUES_USER_TITLE", "Search issues"),
|
||||
ReadOnlyHint: true,
|
||||
@@ -1677,7 +1706,7 @@ func SearchIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
|
||||
return utils.NewToolResultError(err.Error()), nil, nil
|
||||
}
|
||||
options = append(options, withFieldsFiltering(deps, "search_issues", fields))
|
||||
result, err := searchIssuesHandler(ctx, deps, args, options...)
|
||||
result, err := searchIssuesHandler(ctx, deps, args, mode, options...)
|
||||
return result, nil, err
|
||||
})
|
||||
}
|
||||
@@ -1991,10 +2020,10 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n
|
||||
// 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).
|
||||
func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[string]any, options ...searchOption) (*mcp.CallToolResult, error) {
|
||||
func searchIssuesHandler(ctx context.Context, deps ToolDependencies, args map[string]any, mode searchMode, options ...searchOption) (*mcp.CallToolResult, error) {
|
||||
const errorPrefix = "failed to search issues"
|
||||
|
||||
query, opts, err := prepareSearchArgs(args, "issue")
|
||||
query, opts, err := prepareSearchArgs(args, "issue", mode)
|
||||
if err != nil {
|
||||
return utils.NewToolResultError(err.Error()), nil
|
||||
}
|
||||
|
||||
+42
-32
@@ -1113,11 +1113,12 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue repo:owner/repo is:open",
|
||||
"sort": "created",
|
||||
"order": "desc",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue repo:owner/repo is:open",
|
||||
"sort": "created",
|
||||
"order": "desc",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1139,11 +1140,12 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "repo:test-owner/test-repo is:issue is:open",
|
||||
"sort": "created",
|
||||
"order": "asc",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "repo:test-owner/test-repo is:issue is:open",
|
||||
"sort": "created",
|
||||
"order": "asc",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1165,9 +1167,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue bug",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue bug",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1186,9 +1189,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue feature",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue feature",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1218,9 +1222,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1238,9 +1243,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue repo:github/github-mcp-server critical",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue repo:github/github-mcp-server critical",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1260,9 +1266,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue repo:octocat/Hello-World bug",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue repo:octocat/Hello-World bug",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1280,9 +1287,10 @@ func Test_SearchIssues(t *testing.T) {
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
@@ -1303,6 +1311,7 @@ func Test_SearchIssues(t *testing.T) {
|
||||
"q": "is:issue field.priority:P1",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
"advanced_search": "true",
|
||||
},
|
||||
).andThen(
|
||||
@@ -1316,14 +1325,15 @@ func Test_SearchIssues(t *testing.T) {
|
||||
expectedResult: mockSearchResult,
|
||||
},
|
||||
{
|
||||
name: "query without field. qualifier does not set advanced_search",
|
||||
name: "semantic search sets search_type",
|
||||
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
|
||||
GetSearchIssues: expectQueryParams(
|
||||
t,
|
||||
map[string]string{
|
||||
"q": "is:issue is:open",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"q": "is:issue is:open",
|
||||
"page": "1",
|
||||
"per_page": "30",
|
||||
"search_type": "semantic",
|
||||
},
|
||||
).andThen(
|
||||
mockResponse(t, http.StatusOK, mockSearchResult),
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/github/github-mcp-server/pkg/translations"
|
||||
"github.com/github/github-mcp-server/pkg/utils"
|
||||
"github.com/google/jsonschema-go/jsonschema"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func Test_stripFreeTextQuotes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
query string
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "leaves an unquoted query alone",
|
||||
query: "is:issue sticky sidebar",
|
||||
expected: "is:issue sticky sidebar",
|
||||
},
|
||||
{
|
||||
name: "strips quotes around free text",
|
||||
query: `is:issue "sticky sidebar"`,
|
||||
expected: "is:issue sticky sidebar",
|
||||
},
|
||||
{
|
||||
name: "preserves quotes around a multi-word qualifier value",
|
||||
query: `is:issue label:"needs triage"`,
|
||||
expected: `is:issue label:"needs triage"`,
|
||||
},
|
||||
{
|
||||
name: "strips free text but preserves the qualifier alongside it",
|
||||
query: `is:issue label:"needs triage" "sticky sidebar"`,
|
||||
expected: `is:issue label:"needs triage" sticky sidebar`,
|
||||
},
|
||||
{
|
||||
name: "preserves quotes on a hyphenated qualifier",
|
||||
query: `is:issue state-reason:"not planned"`,
|
||||
expected: `is:issue state-reason:"not planned"`,
|
||||
},
|
||||
{
|
||||
name: "preserves quotes on a dotted custom field qualifier",
|
||||
query: `is:issue field.priority:"P1 urgent"`,
|
||||
expected: `is:issue field.priority:"P1 urgent"`,
|
||||
},
|
||||
{
|
||||
name: "preserves quotes on a negated qualifier",
|
||||
query: `is:issue -label:"wont fix"`,
|
||||
expected: `is:issue -label:"wont fix"`,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
assert.Equal(t, tt.expected, stripFreeTextQuotes(tt.query))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_searchIssuesTool_descriptionMatchesEngine(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// The description has to describe the engine the host will actually use.
|
||||
// Steering a lexical-only host toward paraphrased natural language is actively misleading.
|
||||
semantic := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeDotcom))
|
||||
lexical := SearchIssues(translations.NullTranslationHelper, WithHost(utils.HostTypeGHES))
|
||||
|
||||
require.Equal(t, "search_issues", semantic.Tool.Name)
|
||||
require.Equal(t, "search_issues", lexical.Tool.Name)
|
||||
|
||||
assert.Equal(t, searchIssuesSemanticDescription, semantic.Tool.Description)
|
||||
assert.Equal(t, searchIssuesLexicalDescription, lexical.Tool.Description)
|
||||
|
||||
semanticSchema, ok := semantic.Tool.InputSchema.(*jsonschema.Schema)
|
||||
require.True(t, ok)
|
||||
lexicalSchema, ok := lexical.Tool.InputSchema.(*jsonschema.Schema)
|
||||
require.True(t, ok)
|
||||
|
||||
assert.Equal(t, searchIssuesSemanticQueryDescription, semanticSchema.Properties["query"].Description)
|
||||
assert.Equal(t, searchIssuesLexicalQueryDescription, lexicalSchema.Properties["query"].Description)
|
||||
}
|
||||
@@ -74,17 +74,27 @@ func withFieldsFiltering(deps ToolDependencies, tool string, fields []string) se
|
||||
}
|
||||
}
|
||||
|
||||
// searchMode selects the engine used to run a search. It maps to the endpoint's
|
||||
// search_type parameter.
|
||||
type searchMode int
|
||||
|
||||
const (
|
||||
// searchModeLexical is the API default, so search_type can be omitted.
|
||||
searchModeLexical searchMode = iota
|
||||
searchModeSemantic
|
||||
)
|
||||
|
||||
// prepareSearchArgs resolves the search query string and REST search options from the tool args,
|
||||
// applying the standard is:<type> / repo:<owner>/<repo> munging shared by search_issues and
|
||||
// search_pull_requests.
|
||||
func prepareSearchArgs(args map[string]any, searchType string) (string, *github.SearchOptions, error) {
|
||||
func prepareSearchArgs(args map[string]any, targetType string, mode searchMode) (string, *github.SearchOptions, error) {
|
||||
query, err := RequiredParam[string](args, "query")
|
||||
if err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
|
||||
if !hasSpecificFilter(query, "is", searchType) {
|
||||
query = fmt.Sprintf("is:%s %s", searchType, query)
|
||||
if !hasSpecificFilter(query, "is", targetType) {
|
||||
query = fmt.Sprintf("is:%s %s", targetType, query)
|
||||
}
|
||||
|
||||
owner, err := OptionalParam[string](args, "owner")
|
||||
@@ -128,14 +138,42 @@ func prepareSearchArgs(args map[string]any, searchType string) (string, *github.
|
||||
opts.AdvancedSearch = github.Ptr(true)
|
||||
}
|
||||
|
||||
// Lexical is the API default, so it leaves search_type unset.
|
||||
if mode == searchModeSemantic {
|
||||
query = applySemanticSearch(query, opts)
|
||||
}
|
||||
|
||||
return query, opts, nil
|
||||
}
|
||||
|
||||
// qualifierQuotePattern matches a quoted qualifier value, e.g. label:"needs
|
||||
// triage". The quotes there are meaningful — they delimit a value containing
|
||||
// spaces — so they must survive stripFreeTextQuotes.
|
||||
var qualifierQuotePattern = regexp.MustCompile(`([-\w.]+:)"([^"]*)"`)
|
||||
|
||||
// stripFreeTextQuotes removes quotes around free text while preserving them
|
||||
// around qualifier values — since these delimit a value containing spaces.
|
||||
func stripFreeTextQuotes(query string) string {
|
||||
const sentinel = "\x00"
|
||||
|
||||
// Hide qualifier quotes behind a sentinel that cannot appear in a query,
|
||||
// strip what remains, then restore them.
|
||||
protected := qualifierQuotePattern.ReplaceAllString(query, "${1}"+sentinel+"${2}"+sentinel)
|
||||
stripped := strings.ReplaceAll(protected, `"`, "")
|
||||
return strings.ReplaceAll(stripped, sentinel, `"`)
|
||||
}
|
||||
|
||||
// applySemanticSearch switches the request to the semantic index.
|
||||
func applySemanticSearch(query string, opts *github.SearchOptions) string {
|
||||
opts.SearchType = "semantic"
|
||||
return stripFreeTextQuotes(query)
|
||||
}
|
||||
|
||||
func searchHandler(
|
||||
ctx context.Context,
|
||||
getClient GetClientFn,
|
||||
args map[string]any,
|
||||
searchType string,
|
||||
targetType string,
|
||||
errorPrefix string,
|
||||
options ...searchOption,
|
||||
) (*mcp.CallToolResult, error) {
|
||||
@@ -143,7 +181,7 @@ func searchHandler(
|
||||
for _, opt := range options {
|
||||
opt(&cfg)
|
||||
}
|
||||
query, opts, err := prepareSearchArgs(args, searchType)
|
||||
query, opts, err := prepareSearchArgs(args, targetType, searchModeLexical)
|
||||
if err != nil {
|
||||
return utils.NewToolResultError(err.Error()), nil
|
||||
}
|
||||
|
||||
+29
-2
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
"github.com/github/github-mcp-server/pkg/inventory"
|
||||
"github.com/github/github-mcp-server/pkg/translations"
|
||||
"github.com/github/github-mcp-server/pkg/utils"
|
||||
)
|
||||
|
||||
type GetClientFn func(context.Context) (*github.Client, error)
|
||||
@@ -180,9 +181,35 @@ var (
|
||||
}
|
||||
)
|
||||
|
||||
// ToolOption configures how tools are built. Options carry deployment
|
||||
// capabilities that are known when the inventory is constructed, so a tool's
|
||||
// description and its behaviour are decided from the same value and cannot
|
||||
// drift apart.
|
||||
type ToolOption func(*toolConfig)
|
||||
|
||||
type toolConfig struct {
|
||||
// hostType is the deployment the tools will talk to. The zero value is
|
||||
// dotcom, which is also what an empty GITHUB_HOST resolves to.
|
||||
hostType utils.HostType
|
||||
}
|
||||
|
||||
// WithHost tells the tools which deployment they will talk to, so those with
|
||||
// host-specific capabilities can adapt. Derive it from utils.ParseHostType.
|
||||
func WithHost(h utils.HostType) ToolOption {
|
||||
return func(c *toolConfig) { c.hostType = h }
|
||||
}
|
||||
|
||||
func newToolConfig(opts []ToolOption) toolConfig {
|
||||
var cfg toolConfig
|
||||
for _, opt := range opts {
|
||||
opt(&cfg)
|
||||
}
|
||||
return cfg
|
||||
}
|
||||
|
||||
// AllTools returns all tools with their embedded toolset metadata.
|
||||
// Tool functions return ServerTool directly with toolset info.
|
||||
func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool {
|
||||
func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []inventory.ServerTool {
|
||||
return withCSVOutput([]inventory.ServerTool{
|
||||
// Context tools
|
||||
GetMe(t),
|
||||
@@ -219,7 +246,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool {
|
||||
|
||||
// Issue tools
|
||||
IssueRead(t),
|
||||
SearchIssues(t),
|
||||
SearchIssues(t, opts...),
|
||||
ListIssues(t),
|
||||
ListIssueTypes(t),
|
||||
ListIssueFields(t),
|
||||
|
||||
+12
-3
@@ -328,11 +328,20 @@ func hasStaticConfig(cfg *ServerConfig) bool {
|
||||
// inventory, which then installs a checker and resolves the flag before
|
||||
// registering tools with the MCP server.
|
||||
func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFunc) ([]inventory.ServerTool, []inventory.ServerResourceTemplate, []inventory.ServerPrompt) {
|
||||
// Tools with host-specific capabilities need to know the deployment they
|
||||
// will talk to. An unparseable host is not fatal here: NewAPIHost rejects
|
||||
// it later with a clearer error, so fall back to the dotcom default.
|
||||
hostType, err := utils.ParseHostType(cfg.Host)
|
||||
if err != nil {
|
||||
hostType = utils.HostTypeDotcom
|
||||
}
|
||||
opts := []github.ToolOption{github.WithHost(hostType)}
|
||||
|
||||
if !hasStaticConfig(cfg) {
|
||||
return github.AllTools(t), github.AllResources(t), github.AllPrompts(t)
|
||||
return github.AllTools(t, opts...), github.AllResources(t), github.AllPrompts(t)
|
||||
}
|
||||
|
||||
b := github.NewInventory(t).
|
||||
b := github.NewInventory(t, opts...).
|
||||
WithReadOnly(cfg.ReadOnly).
|
||||
WithToolsets(github.ResolvedEnabledToolsets(cfg.EnabledToolsets, cfg.EnabledTools))
|
||||
|
||||
@@ -348,7 +357,7 @@ func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFun
|
||||
if err != nil {
|
||||
// Fall back to all tools if there's an error (e.g. unknown tool names).
|
||||
// The error will surface again at per-request time if relevant.
|
||||
return github.AllTools(t), github.AllResources(t), github.AllPrompts(t)
|
||||
return github.AllTools(t, opts...), github.AllResources(t), github.AllPrompts(t)
|
||||
}
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -783,6 +783,60 @@ func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTo
|
||||
return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx)
|
||||
}
|
||||
|
||||
// TestStaticInventoryAppliesHostCapabilities guards against HTTP deployments
|
||||
// silently getting dotcom behaviour. ServerConfig.Host can point at GHES, where
|
||||
// semantic issue search 403s, so the static inventory has to classify the host
|
||||
// rather than fall through to the zero value.
|
||||
func TestStaticInventoryAppliesHostCapabilities(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
host string
|
||||
wantDescription string
|
||||
}{
|
||||
{
|
||||
name: "empty host defaults to dotcom",
|
||||
host: "",
|
||||
wantDescription: "semantic",
|
||||
},
|
||||
{
|
||||
name: "dotcom",
|
||||
host: "https://github.com",
|
||||
wantDescription: "semantic",
|
||||
},
|
||||
{
|
||||
name: "GHES falls back to lexical",
|
||||
host: "https://ghes.example.com",
|
||||
wantDescription: "lexical",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cfg := &ServerConfig{Version: "test", Host: tt.host}
|
||||
staticTools, _, _ := buildStaticInventory(cfg, translations.NullTranslationHelper)
|
||||
|
||||
var found bool
|
||||
for _, st := range staticTools {
|
||||
if st.Tool.Name != "search_issues" {
|
||||
continue
|
||||
}
|
||||
found = true
|
||||
isSemantic := strings.Contains(st.Tool.Description, "semantic matching")
|
||||
if tt.wantDescription == "semantic" {
|
||||
assert.True(t, isSemantic, "expected semantic description, got: %s", st.Tool.Description)
|
||||
} else {
|
||||
assert.False(t, isSemantic, "expected lexical description, got: %s", st.Tool.Description)
|
||||
}
|
||||
}
|
||||
require.True(t, found, "search_issues should be in the static inventory")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCrossOriginProtection(t *testing.T) {
|
||||
jsonRPCBody := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-03-26","capabilities":{},"clientInfo":{"name":"test","version":"0.1"}}}`
|
||||
|
||||
|
||||
+7
-3
@@ -129,6 +129,10 @@ func RunHTTPServer(cfg ServerConfig) error {
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to parse API host: %w", err)
|
||||
}
|
||||
hostType, err := utils.ParseHostType(cfg.Host)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to classify API host: %w", err)
|
||||
}
|
||||
|
||||
repoAccessOpts := []lockdown.RepoAccessOption{
|
||||
lockdown.WithLogger(logger.With("component", "lockdown")),
|
||||
@@ -156,7 +160,7 @@ func RunHTTPServer(cfg ServerConfig) error {
|
||||
)
|
||||
|
||||
// Initialize the global tool scope map
|
||||
err = initGlobalToolScopeMap(t)
|
||||
err = initGlobalToolScopeMap(t, hostType)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to initialize tool scope map: %w", err)
|
||||
}
|
||||
@@ -239,10 +243,10 @@ func resolveListenAddress(host string, port int) string {
|
||||
return net.JoinHostPort(host, strconv.Itoa(port))
|
||||
}
|
||||
|
||||
func initGlobalToolScopeMap(t translations.TranslationHelperFunc) error {
|
||||
func initGlobalToolScopeMap(t translations.TranslationHelperFunc, hostType utils.HostType) error {
|
||||
// Build inventory with all tools to extract scope information
|
||||
inv, err := inventory.NewBuilder().
|
||||
SetTools(github.AllTools(t)).
|
||||
SetTools(github.AllTools(t, github.WithHost(hostType))).
|
||||
Build()
|
||||
|
||||
if err != nil {
|
||||
|
||||
@@ -6,10 +6,48 @@ import (
|
||||
|
||||
ghcontext "github.com/github/github-mcp-server/pkg/context"
|
||||
"github.com/github/github-mcp-server/pkg/github"
|
||||
"github.com/github/github-mcp-server/pkg/utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestInitGlobalToolScopeMapUsesHost(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
hostType utils.HostType
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "dotcom uses semantic search",
|
||||
hostType: utils.HostTypeDotcom,
|
||||
want: "Search issues using natural-language semantic matching. Best for conceptual or paraphrased queries (e.g. \"login fails after password reset\"). Already scoped to is:issue.",
|
||||
},
|
||||
{
|
||||
name: "GHES uses lexical search",
|
||||
hostType: utils.HostTypeGHES,
|
||||
want: "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
translations := make(map[string]string)
|
||||
translator := func(key, defaultValue string) string {
|
||||
if value, ok := translations[key]; ok {
|
||||
return value
|
||||
}
|
||||
translations[key] = defaultValue
|
||||
return defaultValue
|
||||
}
|
||||
|
||||
require.NoError(t, initGlobalToolScopeMap(translator, tt.hostType))
|
||||
|
||||
tool := github.SearchIssues(translator, github.WithHost(tt.hostType))
|
||||
assert.Equal(t, tt.want, tool.Tool.Description)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateHTTPFeatureChecker(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
+45
-5
@@ -235,13 +235,53 @@ func parseAPIHost(s string) (APIHost, error) {
|
||||
return APIHost{}, fmt.Errorf("host must have a scheme (http or https): %s", s)
|
||||
}
|
||||
|
||||
if u.Hostname() == "github.com" || strings.HasSuffix(u.Hostname(), ".github.com") {
|
||||
switch classifyHost(u) {
|
||||
case HostTypeDotcom:
|
||||
return newDotcomHost()
|
||||
}
|
||||
|
||||
if u.Hostname() == "ghe.com" || strings.HasSuffix(u.Hostname(), ".ghe.com") {
|
||||
case HostTypeGHEC:
|
||||
return newGHECHost(s)
|
||||
default:
|
||||
return newGHESHost(s)
|
||||
}
|
||||
}
|
||||
|
||||
// HostType identifies which GitHub deployment a host refers to. Tools use this
|
||||
// to skip capabilities that only exist on some deployments.
|
||||
type HostType int
|
||||
|
||||
const (
|
||||
HostTypeDotcom HostType = iota
|
||||
HostTypeGHEC
|
||||
HostTypeGHES
|
||||
)
|
||||
|
||||
func classifyHost(u *url.URL) HostType {
|
||||
switch {
|
||||
case u.Hostname() == "github.com" || strings.HasSuffix(u.Hostname(), ".github.com"):
|
||||
return HostTypeDotcom
|
||||
case u.Hostname() == "ghe.com" || strings.HasSuffix(u.Hostname(), ".ghe.com"):
|
||||
return HostTypeGHEC
|
||||
default:
|
||||
return HostTypeGHES
|
||||
}
|
||||
}
|
||||
|
||||
// ParseHostType classifies a host string. An empty string means github.com,
|
||||
// matching NewAPIHost. It returns an error only when the string is not a URL
|
||||
// with a scheme.
|
||||
func ParseHostType(s string) (HostType, error) {
|
||||
if s == "" {
|
||||
return HostTypeDotcom, nil
|
||||
}
|
||||
|
||||
return newGHESHost(s)
|
||||
u, err := url.Parse(s)
|
||||
if err != nil {
|
||||
return HostTypeDotcom, fmt.Errorf("could not parse host as URL: %s", s)
|
||||
}
|
||||
|
||||
if u.Scheme == "" {
|
||||
return HostTypeDotcom, fmt.Errorf("host must have a scheme (http or https): %s", s)
|
||||
}
|
||||
|
||||
return classifyHost(u), nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user