diff --git a/README.md b/README.md index 32f8eb82bc..efae2b8ac4 100644 --- a/README.md +++ b/README.md @@ -1308,6 +1308,11 @@ The following sets of tools are available: - `path`: Path to the file to delete (string, required) - `repo`: Repository name (string, required) +- **delete_repository** - Delete repository + - **Required OAuth Scopes**: `delete_repo` + - `owner`: Repository owner (username or organization) (string, required) + - `repo`: Repository name (string, required) + - **fork_repository** - Fork repository - **Required OAuth Scopes**: `repo` - `organization`: Organization to fork to (string, optional) diff --git a/internal/ghmcp/oauth.go b/internal/ghmcp/oauth.go index 35e48f5bbc..3648189217 100644 --- a/internal/ghmcp/oauth.go +++ b/internal/ghmcp/oauth.go @@ -103,14 +103,6 @@ type oauthAuthenticator interface { // delayed response from an older prompt from affecting a newer flow. const oauthElicitIDPrefix = "github_authorization:" -// protocolVersionNoServerElicitation is the first MCP protocol version that -// forbids server-initiated JSON-RPC requests (SEP-2322): from this version on -// the server may not send elicitation/create while serving a request and must -// instead return an InputRequests map from the tool call (multi round-trip -// requests). It mirrors the go-sdk's internal constant of the same value, which -// the SDK does not export. -const protocolVersionNoServerElicitation = "2026-07-28" - // serverMayInitiateElicitation reports whether the server is permitted to send // elicitation requests to the client itself, which the spec allows only before // protocol version 2026-07-28. A nil or un-negotiated session (only reached in @@ -120,7 +112,7 @@ func serverMayInitiateElicitation(ss *mcp.ServerSession) bool { return true } params := ss.InitializeParams() - return params == nil || params.ProtocolVersion < protocolVersionNoServerElicitation + return params == nil || params.ProtocolVersion < inventory.ProtocolVersionMultiRoundTrip } // createOAuthToolMiddleware returns tool-handler middleware that authorizes the diff --git a/pkg/github/__toolsnaps__/delete_repository.snap b/pkg/github/__toolsnaps__/delete_repository.snap new file mode 100644 index 0000000000..a75845b3f4 --- /dev/null +++ b/pkg/github/__toolsnaps__/delete_repository.snap @@ -0,0 +1,27 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "readOnlyHint": false, + "title": "Delete repository" + }, + "description": "Delete a GitHub repository after the user confirms the exact owner/repository name", + "inputSchema": { + "properties": { + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "name": "delete_repository" +} \ No newline at end of file diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index c5a73d9667..f6737c5df0 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -40,6 +40,7 @@ const ( PostReposForksByOwnerByRepo = "POST /repos/{owner}/{repo}/forks" GetReposSubscriptionByOwnerByRepo = "GET /repos/{owner}/{repo}/subscription" PutReposSubscriptionByOwnerByRepo = "PUT /repos/{owner}/{repo}/subscription" + DeleteReposByOwnerByRepo = "DELETE /repos/{owner}/{repo}" DeleteReposSubscriptionByOwnerByRepo = "DELETE /repos/{owner}/{repo}/subscription" ListCollaborators = "GET /repos/{owner}/{repo}/collaborators" diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 560e8c1bac..1c4b23c75c 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -704,6 +704,119 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool ) } +const ( + deleteRepositoryConfirmationID = "delete_repository_confirmation" + deleteRepositoryConfirmationField = "repository_name" +) + +// DeleteRepository creates a tool that deletes a GitHub repository after the +// user confirms its full name through elicitation. +func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool { + tool := NewTool( + ToolsetMetadataRepos, + mcp.Tool{ + Name: "delete_repository", + Description: t("TOOL_DELETE_REPOSITORY_DESCRIPTION", "Delete a GitHub repository after the user confirms the exact owner/repository name"), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_DELETE_REPOSITORY_USER_TITLE", "Delete repository"), + ReadOnlyHint: false, + DestructiveHint: github.Ptr(true), + }, + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + }, + Required: []string{"owner", "repo"}, + }, + }, + []scopes.Scope{scopes.DeleteRepo}, + func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + + fullName := owner + "/" + repo + var responses mcp.InputResponseMap + if req != nil && req.Params != nil { + responses = req.Params.InputResponses + } + response, ok := responses[deleteRepositoryConfirmationID] + if !ok { + return &mcp.CallToolResult{ + InputRequests: mcp.InputRequestMap{ + deleteRepositoryConfirmationID: &mcp.ElicitParams{ + Mode: "form", + Message: fmt.Sprintf("Type %q to confirm permanent deletion of this repository.", fullName), + RequestedSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + deleteRepositoryConfirmationField: { + Type: "string", + Title: "Repository name", + Description: fmt.Sprintf("Enter %s exactly to confirm deletion", fullName), + }, + }, + Required: []string{deleteRepositoryConfirmationField}, + }, + }, + }, + }, nil, nil + } + + confirmation, ok := response.(*mcp.ElicitResult) + if !ok { + return utils.NewToolResultError("Repository deletion confirmation was invalid. The repository was not deleted."), nil, nil + } + if confirmation.Action != "accept" { + return utils.NewToolResultError("Repository deletion was not confirmed. The repository was not deleted."), nil, nil + } + confirmedName, ok := confirmation.Content[deleteRepositoryConfirmationField].(string) + if !ok || confirmedName != fullName { + return utils.NewToolResultError(fmt.Sprintf("Repository name confirmation did not match %q. The repository was not deleted.", fullName)), nil, nil + } + + client, err := deps.GetClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + resp, err := client.Repositories.Delete(ctx, owner, repo) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + fmt.Sprintf("failed to delete repository: %s", fullName), + resp, + err, + ), nil, nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusNoContent { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, nil, fmt.Errorf("failed to read response body: %w", err) + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to delete repository", resp, body), nil, nil + } + + return utils.NewToolResultText(fmt.Sprintf("Repository %s was deleted.", fullName)), nil, nil + }, + ) + tool.MinimumProtocolVersion = inventory.ProtocolVersionMultiRoundTrip + return tool +} + // FetchRepoIsPrivate returns whether a repository is private. It is a thin // wrapper around the GitHub Repositories.Get endpoint provided as a shared // helper for IFC label computation across tools. diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 332b212a17..432a0ea754 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -12,7 +12,9 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/raw" + "github.com/github/github-mcp-server/pkg/scopes" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v89/github" @@ -2984,6 +2986,171 @@ func Test_PushFiles(t *testing.T) { } } +func Test_DeleteRepository(t *testing.T) { + serverTool := DeleteRepository(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + schema, ok := tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok, "InputSchema should be *jsonschema.Schema") + assert.Equal(t, "delete_repository", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.ElementsMatch(t, []string{"owner", "repo"}, schema.Required) + require.NotNil(t, tool.Annotations) + require.NotNil(t, tool.Annotations.DestructiveHint) + assert.True(t, *tool.Annotations.DestructiveHint) + assert.Equal(t, inventory.ProtocolVersionMultiRoundTrip, serverTool.MinimumProtocolVersion) + assert.Equal(t, []string{string(scopes.DeleteRepo)}, serverTool.RequiredScopes) + + t.Run("requests exact repository name through elicitation", func(t *testing.T) { + result := invokeDeleteRepository(t, serverTool, NewMockedHTTPClient(), nil) + + require.False(t, result.IsError) + require.Len(t, result.InputRequests, 1) + inputRequest, ok := result.InputRequests[deleteRepositoryConfirmationID].(*mcp.ElicitParams) + require.True(t, ok) + assert.Equal(t, "form", inputRequest.Mode) + assert.Contains(t, inputRequest.Message, `"owner/repo"`) + + requestedSchema, ok := inputRequest.RequestedSchema.(*jsonschema.Schema) + require.True(t, ok) + assert.ElementsMatch(t, []string{deleteRepositoryConfirmationField}, requestedSchema.Required) + assert.Contains(t, requestedSchema.Properties, deleteRepositoryConfirmationField) + }) + + t.Run("deletes after exact confirmation", func(t *testing.T) { + client := NewMockedHTTPClient( + WithRequestMatchHandler( + DeleteReposByOwnerByRepo, + mockResponse(t, http.StatusNoContent, nil), + ), + ) + result := invokeDeleteRepository(t, serverTool, client, &mcp.ElicitResult{ + Action: "accept", + Content: map[string]any{ + deleteRepositoryConfirmationField: "owner/repo", + }, + }) + + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "owner/repo was deleted") + }) + + t.Run("completes multi-round-trip elicitation before deleting", func(t *testing.T) { + httpClient := NewMockedHTTPClient( + WithRequestMatchHandler( + DeleteReposByOwnerByRepo, + mockResponse(t, http.StatusNoContent, nil), + ), + ) + deps := BaseDeps{Client: mustNewGHClient(t, httpClient)} + + inv, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{serverTool}). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) + + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + server.AddReceivingMiddleware(func(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, request mcp.Request) (mcp.Result, error) { + return next(ContextWithDeps(ctx, deps), method, request) + } + }) + inv.RegisterTools(context.Background(), server, deps) + + serverTransport, clientTransport := mcp.NewInMemoryTransports() + serverSession, err := server.Connect(context.Background(), serverTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = serverSession.Close() }) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + return &mcp.ElicitResult{ + Action: "accept", + Content: map[string]any{ + deleteRepositoryConfirmationField: "owner/repo", + }, + }, nil + }, + }) + clientSession, err := client.Connect(context.Background(), clientTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = clientSession.Close() }) + + result, err := clientSession.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "delete_repository", + Arguments: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + }) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "owner/repo was deleted") + }) + + t.Run("refuses mismatched confirmation", func(t *testing.T) { + result := invokeDeleteRepository(t, serverTool, NewMockedHTTPClient(), &mcp.ElicitResult{ + Action: "accept", + Content: map[string]any{ + deleteRepositoryConfirmationField: "owner/another-repo", + }, + }) + + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "did not match") + }) + + t.Run("refuses declined confirmation", func(t *testing.T) { + result := invokeDeleteRepository(t, serverTool, NewMockedHTTPClient(), &mcp.ElicitResult{ + Action: "decline", + }) + + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "was not confirmed") + }) + + t.Run("returns GitHub API errors", func(t *testing.T) { + client := NewMockedHTTPClient( + WithRequestMatchHandler( + DeleteReposByOwnerByRepo, + mockResponse(t, http.StatusForbidden, map[string]any{"message": "Requires admin permissions"}), + ), + ) + result := invokeDeleteRepository(t, serverTool, client, &mcp.ElicitResult{ + Action: "accept", + Content: map[string]any{ + deleteRepositoryConfirmationField: "owner/repo", + }, + }) + + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, "failed to delete repository") + }) +} + +func invokeDeleteRepository(t *testing.T, tool inventory.ServerTool, httpClient *http.Client, confirmation *mcp.ElicitResult) *mcp.CallToolResult { + t.Helper() + + deps := BaseDeps{Client: mustNewGHClient(t, httpClient)} + handler := tool.Handler(deps) + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + }) + if confirmation != nil { + request.Params.InputResponses = mcp.InputResponseMap{ + deleteRepositoryConfirmationID: confirmation, + } + } + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.NotNil(t, result) + return result +} + func Test_ListBranches(t *testing.T) { // Verify tool definition once serverTool := ListBranches(translations.NullTranslationHelper) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index af571f8426..f9b51159b5 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -232,6 +232,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent GetReleaseByTag(t), CreateOrUpdateFile(t), CreateRepository(t), + DeleteRepository(t), ForkRepository(t), CreateBranch(t), PushFiles(t), diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index b4d509c3e5..4e799c1b3f 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -2,6 +2,7 @@ package http import ( "context" + "encoding/json" "log/slog" "net/http" "net/http/httptest" @@ -46,6 +47,7 @@ type allScopesFetcher struct{} func (f allScopesFetcher) FetchTokenScopes(_ context.Context, _ string) ([]string, error) { return []string{ string(scopes.Repo), + string(scopes.DeleteRepo), string(scopes.WriteOrg), string(scopes.User), string(scopes.Gist), @@ -906,6 +908,95 @@ func TestCrossOriginProtection(t *testing.T) { } } +func TestHTTPToolMinimumProtocolVersion(t *testing.T) { + apiHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + inventoryFactory := func(_ *http.Request) (*inventory.Inventory, error) { + return inventory.NewBuilder(). + SetTools([]inventory.ServerTool{github.DeleteRepository(translations.NullTranslationHelper)}). + WithToolsets([]string{"all"}). + Build() + } + handler := NewHTTPMcpHandler( + context.Background(), + &ServerConfig{Version: "test"}, + github.BaseDeps{}, + translations.NullTranslationHelper, + slog.Default(), + apiHost, + WithInventoryFactory(inventoryFactory), + WithScopeFetcher(allScopesFetcher{}), + ) + + router := chi.NewRouter() + handler.RegisterMiddleware(router) + handler.RegisterRoutes(router) + + for _, tt := range []struct { + name string + protocolVersion string + wantDeleteRepoTool bool + }{ + { + name: "current protocol includes delete repository", + protocolVersion: inventory.ProtocolVersionMultiRoundTrip, + wantDeleteRepoTool: true, + }, + { + name: "legacy protocol hides delete repository", + protocolVersion: "2025-11-25", + wantDeleteRepoTool: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + body := strings.Replace( + `{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"_meta":{"io.modelcontextprotocol/protocolVersion":"PROTOCOL_VERSION","io.modelcontextprotocol/clientCapabilities":{"elicitation":{"form":{}}},"io.modelcontextprotocol/clientInfo":{"name":"test","version":"v0.0.1"}}}}`, + "PROTOCOL_VERSION", + tt.protocolVersion, + 1, + ) + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON) + req.Header.Set(headers.AuthorizationHeader, "Bearer test-token") + req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", ")) + req.Header.Set("Mcp-Protocol-Version", tt.protocolVersion) + req.Header.Set("Mcp-Method", "tools/list") + req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_")) + + recorder := httptest.NewRecorder() + router.ServeHTTP(recorder, req) + require.Equal(t, http.StatusOK, recorder.Code, "response body: %s", recorder.Body.String()) + + var response struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + responseBody := recorder.Body.String() + for line := range strings.SplitSeq(responseBody, "\n") { + if data, ok := strings.CutPrefix(line, "data: "); ok { + responseBody = data + break + } + } + require.NoError(t, json.Unmarshal([]byte(responseBody), &response)) + + toolNames := make([]string, 0, len(response.Result.Tools)) + for _, tool := range response.Result.Tools { + toolNames = append(toolNames, tool.Name) + } + if tt.wantDeleteRepoTool { + assert.Contains(t, toolNames, "delete_repository") + } else { + assert.NotContains(t, toolNames, "delete_repository") + } + }) + } +} + // TestInsidersRoutePreservesUIMeta is a regression test for the bug where // _meta.ui was stripped from tools/list responses on the HTTP /insiders route. // diff --git a/pkg/inventory/protocol_version.go b/pkg/inventory/protocol_version.go new file mode 100644 index 0000000000..1cb8ce0065 --- /dev/null +++ b/pkg/inventory/protocol_version.go @@ -0,0 +1,74 @@ +package inventory + +import ( + "context" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// ProtocolVersionMultiRoundTrip is the first MCP protocol version that supports +// multi-round-trip input requests. +const ProtocolVersionMultiRoundTrip = "2026-07-28" + +func addToolProtocolVersionMiddleware(server *mcp.Server, tools []ServerTool) { + minimumVersions := make(map[string]string) + for _, tool := range tools { + minimum := tool.MinimumProtocolVersion + if minimum == "" || minimum <= minimumVersions[tool.Tool.Name] { + continue + } + minimumVersions[tool.Tool.Name] = minimum + } + if len(minimumVersions) == 0 { + return + } + + server.AddReceivingMiddleware(toolProtocolVersionMiddleware(minimumVersions)) +} + +func toolProtocolVersionMiddleware(minimumVersions map[string]string) mcp.Middleware { + return func(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, request mcp.Request) (mcp.Result, error) { + switch req := request.(type) { + case *mcp.CallToolRequest: + if req.Params != nil { + if minimum := minimumVersions[req.Params.Name]; !protocolVersionAllowed(req.ProtocolVersion(), minimum) { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: fmt.Sprintf( + "Tool %q requires MCP protocol version %s or later.", + req.Params.Name, + minimum, + )}}, + IsError: true, + }, nil + } + } + case *mcp.ListToolsRequest: + result, err := next(ctx, method, request) + if err != nil { + return nil, err + } + list, ok := result.(*mcp.ListToolsResult) + if !ok { + return result, nil + } + + tools := make([]*mcp.Tool, 0, len(list.Tools)) + for _, tool := range list.Tools { + if protocolVersionAllowed(req.ProtocolVersion(), minimumVersions[tool.Name]) { + tools = append(tools, tool) + } + } + list.Tools = tools + return list, nil + } + + return next(ctx, method, request) + } + } +} + +func protocolVersionAllowed(protocolVersion, minimum string) bool { + return minimum == "" || protocolVersion >= minimum +} diff --git a/pkg/inventory/protocol_version_test.go b/pkg/inventory/protocol_version_test.go new file mode 100644 index 0000000000..4b3965ca2f --- /dev/null +++ b/pkg/inventory/protocol_version_test.go @@ -0,0 +1,119 @@ +package inventory + +import ( + "context" + "errors" + "testing" + + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestToolMinimumProtocolVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + protocolVersion string + wantVersionedTool bool + }{ + { + name: "current protocol lists and calls versioned tool", + protocolVersion: ProtocolVersionMultiRoundTrip, + wantVersionedTool: true, + }, + { + name: "legacy protocol hides and refuses versioned tool", + protocolVersion: "2025-11-25", + wantVersionedTool: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var versionedToolCalls int + tools := []ServerTool{ + protocolTestTool("always_available", "", nil), + protocolTestTool("versioned", ProtocolVersionMultiRoundTrip, func() { + versionedToolCalls++ + }), + } + inv, err := NewBuilder(). + SetTools(tools). + WithToolsets([]string{"all"}). + Build() + require.NoError(t, err) + + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + inv.RegisterTools(context.Background(), server, nil) + if tt.protocolVersion < ProtocolVersionMultiRoundTrip { + server.AddReceivingMiddleware(func(next mcp.MethodHandler) mcp.MethodHandler { + return func(ctx context.Context, method string, request mcp.Request) (mcp.Result, error) { + if method == "server/discover" { + return nil, errors.New("legacy server does not support discovery") + } + return next(ctx, method, request) + } + }) + } + + serverTransport, clientTransport := mcp.NewInMemoryTransports() + serverSession, err := server.Connect(context.Background(), serverTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = serverSession.Close() }) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, nil) + clientSession, err := client.Connect(context.Background(), clientTransport, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = clientSession.Close() }) + + listResult, err := clientSession.ListTools(context.Background(), nil) + require.NoError(t, err) + toolNames := make([]string, 0, len(listResult.Tools)) + for _, tool := range listResult.Tools { + toolNames = append(toolNames, tool.Name) + } + assert.Contains(t, toolNames, "always_available") + if tt.wantVersionedTool { + assert.Contains(t, toolNames, "versioned") + } else { + assert.NotContains(t, toolNames, "versioned") + } + + callResult, err := clientSession.CallTool(context.Background(), &mcp.CallToolParams{Name: "versioned"}) + require.NoError(t, err) + if tt.wantVersionedTool { + assert.False(t, callResult.IsError) + assert.Equal(t, 1, versionedToolCalls) + } else { + assert.True(t, callResult.IsError) + assert.Zero(t, versionedToolCalls) + } + }) + } +} + +func protocolTestTool(name, minimumProtocolVersion string, onCall func()) ServerTool { + return ServerTool{ + Tool: mcp.Tool{ + Name: name, + InputSchema: &jsonschema.Schema{Type: "object"}, + }, + Toolset: ToolsetMetadata{ID: "test"}, + HandlerFunc: func(any) mcp.ToolHandler { + return func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if onCall != nil { + onCall() + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "called"}}, + }, nil + } + }, + MinimumProtocolVersion: minimumProtocolVersion, + } +} diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index 915ed0aa1c..86ae978df9 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -220,7 +220,9 @@ func shouldStripMCPAppsMetadata(ctx context.Context, featureFlagEnabled bool) bo // falsely report the flag off, even when the actual request arrived on the // /insiders route. func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any, middleware ...ToolHandlerMiddleware) { - for _, tool := range r.ToolsForRegistration(ctx) { + tools := r.ToolsForRegistration(ctx) + addToolProtocolVersionMiddleware(s, tools) + for _, tool := range tools { tool.RegisterFunc(s, deps, middleware...) } } diff --git a/pkg/inventory/server_tool.go b/pkg/inventory/server_tool.go index 44a062ba2e..71f6db08a3 100644 --- a/pkg/inventory/server_tool.go +++ b/pkg/inventory/server_tool.go @@ -81,6 +81,10 @@ type ServerTool struct { // Returns (enabled, error). On error, the tool should be treated as disabled. Enabled func(ctx context.Context) (bool, error) + // MinimumProtocolVersion is the oldest MCP protocol version that may list or + // call this tool. Empty means the tool is available on every version. + MinimumProtocolVersion string + // RequiredScopes specifies the minimum OAuth scopes required for this tool. // These are the scopes that must be present for the tool to function. RequiredScopes []string diff --git a/pkg/scopes/scopes.go b/pkg/scopes/scopes.go index cb1b7681a7..084ecaf1c6 100644 --- a/pkg/scopes/scopes.go +++ b/pkg/scopes/scopes.go @@ -20,6 +20,9 @@ const ( // PublicRepo grants access to public repositories PublicRepo Scope = "public_repo" + // DeleteRepo grants permission to delete repositories + DeleteRepo Scope = "delete_repo" + // ReadOrg grants read-only access to organization membership, teams, and projects ReadOrg Scope = "read:org" diff --git a/pkg/scopes/scopes_test.go b/pkg/scopes/scopes_test.go index b8e0d8e421..94e21a9cd7 100644 --- a/pkg/scopes/scopes_test.go +++ b/pkg/scopes/scopes_test.go @@ -33,6 +33,11 @@ func TestExpandScopes(t *testing.T) { required: []Scope{PublicRepo}, expected: []string{"public_repo", "repo"}, }, + { + name: "delete_repo returns just delete_repo", + required: []Scope{DeleteRepo}, + expected: []string{"delete_repo"}, + }, { name: "security_events also accepts repo (parent)", required: []Scope{SecurityEvents},