Skip to content

Commit 8b850d0

Browse files
SamMorrowDrumsSamMorrowDrums
committed
refactor: Apply HandlerFunc pattern to resources for stateless NewToolsetGroup
This change applies the same HandlerFunc pattern used by tools to resources, allowing NewToolsetGroup to be fully stateless (only requiring translations). Key changes: - Add ResourceHandlerFunc type to toolsets package - Update ServerResourceTemplate to use HandlerFunc instead of direct Handler - Add HasHandler() and Handler(deps) methods to ServerResourceTemplate - Update RegisterResourceTemplates to take deps parameter - Refactor repository resource definitions to use HandlerFunc pattern - Make AllResources(t) stateless (only takes translations) - Make NewToolsetGroup(t) stateless (only takes translations) - Update generate_docs.go - no longer needs mock clients - Update tests to use new patterns This resolves the concern about mixed concerns in doc generation - the toolset metadata and resource templates can now be created without any runtime dependencies, while handlers are generated on-demand when deps are provided during registration.
1 parent a03b3bf commit 8b850d0

File tree

9 files changed

+104
-81
lines changed

9 files changed

+104
-81
lines changed

‎cmd/github-mcp-server/generate_docs.go‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package main
22

33
import (
4-
"context"
54
"fmt"
65
"net/url"
76
"os"
@@ -12,7 +11,6 @@ import (
1211
"github.com/github/github-mcp-server/pkg/github"
1312
"github.com/github/github-mcp-server/pkg/toolsets"
1413
"github.com/github/github-mcp-server/pkg/translations"
15-
gogithub "github.com/google/go-github/v79/github"
1614
"github.com/google/jsonschema-go/jsonschema"
1715
"github.com/modelcontextprotocol/go-sdk/mcp"
1816
"github.com/spf13/cobra"
@@ -31,11 +29,6 @@ func init() {
3129
rootCmd.AddCommand(generateDocsCmd)
3230
}
3331

34-
// mockGetClient returns a mock GitHub client for documentation generation
35-
func mockGetClient(_ context.Context) (*gogithub.Client, error) {
36-
return gogithub.NewClient(nil), nil
37-
}
38-
3932
func generateAllDocs() error {
4033
if err := generateReadmeDocs("README.md"); err != nil {
4134
return fmt.Errorf("failed to generate README docs: %w", err)
@@ -56,8 +49,8 @@ func generateReadmeDocs(readmePath string) error {
5649
// Create translation helper
5750
t, _ := translations.TranslationHelper()
5851

59-
// Create toolset group with mock clients (no deps needed for doc generation)
60-
tsg := github.NewToolsetGroup(t, mockGetClient, nil)
52+
// Create toolset group - stateless, no dependencies needed for doc generation
53+
tsg := github.NewToolsetGroup(t)
6154

6255
// Generate toolsets documentation
6356
toolsetsDoc := generateToolsetsDoc(tsg)
@@ -299,8 +292,8 @@ func generateRemoteToolsetsDoc() string {
299292
// Create translation helper
300293
t, _ := translations.TranslationHelper()
301294

302-
// Create toolset group with mock clients
303-
tsg := github.NewToolsetGroup(t, mockGetClient, nil)
295+
// Create toolset group - stateless
296+
tsg := github.NewToolsetGroup(t)
304297

305298
// Generate table header
306299
buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n")

‎internal/ghmcp/server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
162162
ContentWindowSize: cfg.ContentWindowSize,
163163
}
164164

165-
// Create toolset group with all tools, resources, and prompts
166-
tsg := github.NewToolsetGroup(cfg.Translator, getClient, getRawClient)
165+
// Create toolset group with all tools, resources, and prompts (stateless)
166+
tsg := github.NewToolsetGroup(cfg.Translator)
167167

168168
// Clean tool names (WithTools will resolve any deprecated aliases)
169169
enabledTools := github.CleanTools(cfg.EnabledTools)

‎pkg/github/repository_resource.go‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,71 +29,79 @@ var (
2929
repositoryResourcePrContentURITemplate = uritemplate.MustNew("repo://{owner}/{repo}/refs/pull/{prNumber}/head/contents{/path*}")
3030
)
3131

32-
// GetRepositoryResourceContent defines the resource template and handler for getting repository content.
33-
func GetRepositoryResourceContent(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
32+
// GetRepositoryResourceContent defines the resource template for getting repository content.
33+
func GetRepositoryResourceContent(t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
3434
return toolsets.NewServerResourceTemplate(
3535
ToolsetMetadataRepos,
3636
mcp.ResourceTemplate{
3737
Name: "repository_content",
3838
URITemplate: repositoryResourceContentURITemplate.Raw(),
3939
Description: t("RESOURCE_REPOSITORY_CONTENT_DESCRIPTION", "Repository Content"),
4040
},
41-
RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate),
41+
repositoryResourceContentsHandlerFunc(repositoryResourceContentURITemplate),
4242
)
4343
}
4444

45-
// GetRepositoryResourceBranchContent defines the resource template and handler for getting repository content for a branch.
46-
func GetRepositoryResourceBranchContent(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
45+
// GetRepositoryResourceBranchContent defines the resource template for getting repository content for a branch.
46+
func GetRepositoryResourceBranchContent(t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
4747
return toolsets.NewServerResourceTemplate(
4848
ToolsetMetadataRepos,
4949
mcp.ResourceTemplate{
5050
Name: "repository_content_branch",
5151
URITemplate: repositoryResourceBranchContentURITemplate.Raw(),
5252
Description: t("RESOURCE_REPOSITORY_CONTENT_BRANCH_DESCRIPTION", "Repository Content for specific branch"),
5353
},
54-
RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceBranchContentURITemplate),
54+
repositoryResourceContentsHandlerFunc(repositoryResourceBranchContentURITemplate),
5555
)
5656
}
5757

58-
// GetRepositoryResourceCommitContent defines the resource template and handler for getting repository content for a commit.
59-
func GetRepositoryResourceCommitContent(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
58+
// GetRepositoryResourceCommitContent defines the resource template for getting repository content for a commit.
59+
func GetRepositoryResourceCommitContent(t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
6060
return toolsets.NewServerResourceTemplate(
6161
ToolsetMetadataRepos,
6262
mcp.ResourceTemplate{
6363
Name: "repository_content_commit",
6464
URITemplate: repositoryResourceCommitContentURITemplate.Raw(),
6565
Description: t("RESOURCE_REPOSITORY_CONTENT_COMMIT_DESCRIPTION", "Repository Content for specific commit"),
6666
},
67-
RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceCommitContentURITemplate),
67+
repositoryResourceContentsHandlerFunc(repositoryResourceCommitContentURITemplate),
6868
)
6969
}
7070

71-
// GetRepositoryResourceTagContent defines the resource template and handler for getting repository content for a tag.
72-
func GetRepositoryResourceTagContent(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
71+
// GetRepositoryResourceTagContent defines the resource template for getting repository content for a tag.
72+
func GetRepositoryResourceTagContent(t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
7373
return toolsets.NewServerResourceTemplate(
7474
ToolsetMetadataRepos,
7575
mcp.ResourceTemplate{
7676
Name: "repository_content_tag",
7777
URITemplate: repositoryResourceTagContentURITemplate.Raw(),
7878
Description: t("RESOURCE_REPOSITORY_CONTENT_TAG_DESCRIPTION", "Repository Content for specific tag"),
7979
},
80-
RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceTagContentURITemplate),
80+
repositoryResourceContentsHandlerFunc(repositoryResourceTagContentURITemplate),
8181
)
8282
}
8383

84-
// GetRepositoryResourcePrContent defines the resource template and handler for getting repository content for a pull request.
85-
func GetRepositoryResourcePrContent(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
84+
// GetRepositoryResourcePrContent defines the resource template for getting repository content for a pull request.
85+
func GetRepositoryResourcePrContent(t translations.TranslationHelperFunc) toolsets.ServerResourceTemplate {
8686
return toolsets.NewServerResourceTemplate(
8787
ToolsetMetadataRepos,
8888
mcp.ResourceTemplate{
8989
Name: "repository_content_pr",
9090
URITemplate: repositoryResourcePrContentURITemplate.Raw(),
9191
Description: t("RESOURCE_REPOSITORY_CONTENT_PR_DESCRIPTION", "Repository Content for specific pull request"),
9292
},
93-
RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourcePrContentURITemplate),
93+
repositoryResourceContentsHandlerFunc(repositoryResourcePrContentURITemplate),
9494
)
9595
}
9696

97+
// repositoryResourceContentsHandlerFunc returns a ResourceHandlerFunc that creates handlers on-demand.
98+
func repositoryResourceContentsHandlerFunc(resourceURITemplate *uritemplate.Template) toolsets.ResourceHandlerFunc {
99+
return func(deps any) mcp.ResourceHandler {
100+
d := deps.(ToolDependencies)
101+
return RepositoryResourceContentsHandler(d.GetClient, d.GetRawClient, resourceURITemplate)
102+
}
103+
}
104+
97105
// RepositoryResourceContentsHandler returns a handler function for repository content requests.
98106
func RepositoryResourceContentsHandler(getClient GetClientFn, getRawClient raw.GetRawClientFn, resourceURITemplate *uritemplate.Template) mcp.ResourceHandler {
99107
return func(ctx context.Context, request *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) {

‎pkg/github/repository_resource_test.go‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"testing"
88

99
"github.com/github/github-mcp-server/pkg/raw"
10-
"github.com/github/github-mcp-server/pkg/translations"
1110
"github.com/google/go-github/v79/github"
1211
"github.com/migueleliasweb/go-github-mock/src/mock"
1312
"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -28,7 +27,7 @@ func Test_repositoryResourceContents(t *testing.T) {
2827
name string
2928
mockedClient *http.Client
3029
uri string
31-
handlerFn func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler
30+
handlerFn func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler
3231
expectedResponseType resourceResponseType
3332
expectError string
3433
expectedResult *mcp.ReadResourceResult
@@ -46,8 +45,8 @@ func Test_repositoryResourceContents(t *testing.T) {
4645
),
4746
),
4847
uri: "repo:///repo/contents/README.md",
49-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
50-
return GetRepositoryResourceContent(getClient, getRawClient, t).Handler
48+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
49+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate)
5150
},
5251
expectedResponseType: resourceResponseTypeText, // Ignored as error is expected
5352
expectError: "owner is required",
@@ -65,8 +64,8 @@ func Test_repositoryResourceContents(t *testing.T) {
6564
),
6665
),
6766
uri: "repo://owner//refs/heads/main/contents/README.md",
68-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
69-
return GetRepositoryResourceBranchContent(getClient, getRawClient, t).Handler
67+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
68+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceBranchContentURITemplate)
7069
},
7170
expectedResponseType: resourceResponseTypeText, // Ignored as error is expected
7271
expectError: "repo is required",
@@ -84,8 +83,8 @@ func Test_repositoryResourceContents(t *testing.T) {
8483
),
8584
),
8685
uri: "repo://owner/repo/contents/data.png",
87-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
88-
return GetRepositoryResourceContent(getClient, getRawClient, t).Handler
86+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
87+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate)
8988
},
9089
expectedResponseType: resourceResponseTypeBlob,
9190
expectedResult: &mcp.ReadResourceResult{
@@ -108,8 +107,8 @@ func Test_repositoryResourceContents(t *testing.T) {
108107
),
109108
),
110109
uri: "repo://owner/repo/contents/README.md",
111-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
112-
return GetRepositoryResourceContent(getClient, getRawClient, t).Handler
110+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
111+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate)
113112
},
114113
expectedResponseType: resourceResponseTypeText,
115114
expectedResult: &mcp.ReadResourceResult{
@@ -134,8 +133,8 @@ func Test_repositoryResourceContents(t *testing.T) {
134133
),
135134
),
136135
uri: "repo://owner/repo/contents/pkg/github/actions.go",
137-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
138-
return GetRepositoryResourceContent(getClient, getRawClient, t).Handler
136+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
137+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate)
139138
},
140139
expectedResponseType: resourceResponseTypeText,
141140
expectedResult: &mcp.ReadResourceResult{
@@ -158,8 +157,8 @@ func Test_repositoryResourceContents(t *testing.T) {
158157
),
159158
),
160159
uri: "repo://owner/repo/refs/heads/main/contents/README.md",
161-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
162-
return GetRepositoryResourceBranchContent(getClient, getRawClient, t).Handler
160+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
161+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceBranchContentURITemplate)
163162
},
164163
expectedResponseType: resourceResponseTypeText,
165164
expectedResult: &mcp.ReadResourceResult{
@@ -182,8 +181,8 @@ func Test_repositoryResourceContents(t *testing.T) {
182181
),
183182
),
184183
uri: "repo://owner/repo/refs/tags/v1.0.0/contents/README.md",
185-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
186-
return GetRepositoryResourceTagContent(getClient, getRawClient, t).Handler
184+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
185+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceTagContentURITemplate)
187186
},
188187
expectedResponseType: resourceResponseTypeText,
189188
expectedResult: &mcp.ReadResourceResult{
@@ -206,8 +205,8 @@ func Test_repositoryResourceContents(t *testing.T) {
206205
),
207206
),
208207
uri: "repo://owner/repo/sha/abc123/contents/README.md",
209-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
210-
return GetRepositoryResourceCommitContent(getClient, getRawClient, t).Handler
208+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
209+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceCommitContentURITemplate)
211210
},
212211
expectedResponseType: resourceResponseTypeText,
213212
expectedResult: &mcp.ReadResourceResult{
@@ -238,8 +237,8 @@ func Test_repositoryResourceContents(t *testing.T) {
238237
),
239238
),
240239
uri: "repo://owner/repo/refs/pull/42/head/contents/README.md",
241-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
242-
return GetRepositoryResourcePrContent(getClient, getRawClient, t).Handler
240+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
241+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourcePrContentURITemplate)
243242
},
244243
expectedResponseType: resourceResponseTypeText,
245244
expectedResult: &mcp.ReadResourceResult{
@@ -261,8 +260,8 @@ func Test_repositoryResourceContents(t *testing.T) {
261260
),
262261
),
263262
uri: "repo://owner/repo/contents/nonexistent.md",
264-
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) mcp.ResourceHandler {
265-
return GetRepositoryResourceContent(getClient, getRawClient, t).Handler
263+
handlerFn: func(getClient GetClientFn, getRawClient raw.GetRawClientFn) mcp.ResourceHandler {
264+
return RepositoryResourceContentsHandler(getClient, getRawClient, repositoryResourceContentURITemplate)
266265
},
267266
expectedResponseType: resourceResponseTypeText, // Ignored as error is expected
268267
expectError: "404 Not Found",
@@ -273,7 +272,7 @@ func Test_repositoryResourceContents(t *testing.T) {
273272
t.Run(tc.name, func(t *testing.T) {
274273
client := github.NewClient(tc.mockedClient)
275274
mockRawClient := raw.NewClient(client, base)
276-
handler := tc.handlerFn(stubGetClientFn(client), stubGetRawClientFn(mockRawClient), translations.NullTranslationHelper)
275+
handler := tc.handlerFn(stubGetClientFn(client), stubGetRawClientFn(mockRawClient))
277276

278277
request := &mcp.ReadResourceRequest{
279278
Params: &mcp.ReadResourceParams{

‎pkg/github/resources.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
11
package github
22

33
import (
4-
"github.com/github/github-mcp-server/pkg/raw"
54
"github.com/github/github-mcp-server/pkg/toolsets"
65
"github.com/github/github-mcp-server/pkg/translations"
76
)
87

98
// AllResources returns all resource templates with their embedded toolset metadata.
10-
// Resource template functions return ServerResourceTemplate directly with toolset info.
11-
func AllResources(t translations.TranslationHelperFunc, getClient GetClientFn, getRawClient raw.GetRawClientFn) []toolsets.ServerResourceTemplate {
9+
// Resource definitions are stateless - handlers are generated on-demand during registration.
10+
func AllResources(t translations.TranslationHelperFunc) []toolsets.ServerResourceTemplate {
1211
return []toolsets.ServerResourceTemplate{
1312
// Repository resources
14-
GetRepositoryResourceContent(getClient, getRawClient, t),
15-
GetRepositoryResourceBranchContent(getClient, getRawClient, t),
16-
GetRepositoryResourceCommitContent(getClient, getRawClient, t),
17-
GetRepositoryResourceTagContent(getClient, getRawClient, t),
18-
GetRepositoryResourcePrContent(getClient, getRawClient, t),
13+
GetRepositoryResourceContent(t),
14+
GetRepositoryResourceBranchContent(t),
15+
GetRepositoryResourceCommitContent(t),
16+
GetRepositoryResourceTagContent(t),
17+
GetRepositoryResourcePrContent(t),
1918
}
2019
}

‎pkg/github/tools_validation_test.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@ func TestAllToolsHaveValidToolsetID(t *testing.T) {
5858

5959
// TestAllResourcesHaveRequiredMetadata validates that all resources have mandatory metadata
6060
func TestAllResourcesHaveRequiredMetadata(t *testing.T) {
61-
// Resources need client functions, but we can pass nil for validation
62-
// since we're not actually calling handlers
63-
resources := AllResources(stubTranslation, nil, nil)
61+
// Resources are now stateless - no client functions needed
62+
resources := AllResources(stubTranslation)
6463

6564
require.NotEmpty(t, resources, "AllResources should return at least one resource")
6665

@@ -70,9 +69,9 @@ func TestAllResourcesHaveRequiredMetadata(t *testing.T) {
7069
assert.NotEmpty(t, res.Toolset.ID,
7170
"Resource %q must have a Toolset.ID", res.Template.Name)
7271

73-
// Handler must be set
74-
assert.NotNil(t, res.Handler,
75-
"Resource %q must have a Handler", res.Template.Name)
72+
// HandlerFunc must be set
73+
assert.True(t, res.HasHandler(),
74+
"Resource %q must have a HandlerFunc", res.Template.Name)
7675
})
7776
}
7877
}
@@ -98,7 +97,7 @@ func TestAllPromptsHaveRequiredMetadata(t *testing.T) {
9897

9998
// TestAllResourcesHaveValidToolsetID validates that all resources belong to known toolsets
10099
func TestAllResourcesHaveValidToolsetID(t *testing.T) {
101-
resources := AllResources(stubTranslation, nil, nil)
100+
resources := AllResources(stubTranslation)
102101
validToolsetIDs := GetValidToolsetIDs()
103102

104103
for _, res := range resources {
@@ -153,7 +152,7 @@ func TestNoDuplicateToolNames(t *testing.T) {
153152

154153
// TestNoDuplicateResourceNames ensures all resources have unique names
155154
func TestNoDuplicateResourceNames(t *testing.T) {
156-
resources := AllResources(stubTranslation, nil, nil)
155+
resources := AllResources(stubTranslation)
157156
seen := make(map[string]bool)
158157

159158
for _, res := range resources {

0 commit comments

Comments
 (0)