Skip to content

Commit 011065a

Browse files
refactor: rename toolsets package to registry with builder pattern
- Rename pkg/toolsets to pkg/registry (better reflects its purpose) - Split monolithic toolsets.go into focused files: - registry.go: Core Registry struct and MCP methods - builder.go: Builder pattern for creating Registry instances - filters.go: All filtering logic (toolsets, read-only, feature flags) - resources.go: ServerResourceTemplate type - prompts.go: ServerPrompt type - errors.go: Error types - server_tool.go: ServerTool and ToolsetMetadata (existing) - Fix lint: Rename RegistryBuilder to Builder (avoid stuttering) - Update all imports across ~45 files This refactoring improves code organization and makes the registry's purpose clearer. The builder pattern provides a clean API: reg := registry.NewBuilder(). SetTools(tools). WithReadOnly(true). WithToolsets([]string{"repos"}). Build()
1 parent 6b6a874 commit 011065a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1900
-1793
lines changed

cmd/github-mcp-server/generate_docs.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"strings"
99

1010
"github.com/github/github-mcp-server/pkg/github"
11-
"github.com/github/github-mcp-server/pkg/toolsets"
11+
"github.com/github/github-mcp-server/pkg/registry"
1212
"github.com/github/github-mcp-server/pkg/translations"
1313
"github.com/google/jsonschema-go/jsonschema"
1414
"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -50,8 +50,8 @@ func generateReadmeDocs(readmePath string) error {
5050
// Create translation helper
5151
t, _ := translations.TranslationHelper()
5252

53-
// Create toolset group - stateless, no dependencies needed for doc generation
54-
r := github.NewRegistry(t)
53+
// Build registry - stateless, no dependencies needed for doc generation
54+
r := github.NewRegistry(t).Build()
5555

5656
// Generate toolsets documentation
5757
toolsetsDoc := generateToolsetsDoc(r)
@@ -104,7 +104,7 @@ func generateRemoteServerDocs(docsPath string) error {
104104
return os.WriteFile(docsPath, []byte(updatedContent), 0600) //#nosec G306
105105
}
106106

107-
func generateToolsetsDoc(r *toolsets.Registry) string {
107+
func generateToolsetsDoc(r *registry.Registry) string {
108108
var buf strings.Builder
109109

110110
// Add table header and separator
@@ -123,7 +123,7 @@ func generateToolsetsDoc(r *toolsets.Registry) string {
123123
return strings.TrimSuffix(buf.String(), "\n")
124124
}
125125

126-
func generateToolsDoc(r *toolsets.Registry) string {
126+
func generateToolsDoc(r *registry.Registry) string {
127127
// AllTools() returns tools sorted by toolset ID then tool name.
128128
// We iterate once, grouping by toolset as we encounter them.
129129
tools := r.AllTools()
@@ -133,7 +133,7 @@ func generateToolsDoc(r *toolsets.Registry) string {
133133

134134
var buf strings.Builder
135135
var toolBuf strings.Builder
136-
var currentToolsetID toolsets.ToolsetID
136+
var currentToolsetID registry.ToolsetID
137137
firstSection := true
138138

139139
writeSection := func() {
@@ -299,8 +299,8 @@ func generateRemoteToolsetsDoc() string {
299299
// Create translation helper
300300
t, _ := translations.TranslationHelper()
301301

302-
// Create toolset group - stateless
303-
r := github.NewRegistry(t)
302+
// Build registry - stateless
303+
r := github.NewRegistry(t).Build()
304304

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

e2e/e2e_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func setupMCPClient(t *testing.T, options ...clientOption) *mcp.ClientSession {
178178
// so that there is a shared setup mechanism, but let's wait till we feel more friction.
179179
enabledToolsets := opts.enabledToolsets
180180
if enabledToolsets == nil {
181-
enabledToolsets = github.NewRegistry(translations.NullTranslationHelper).DefaultToolsetIDs()
181+
enabledToolsets = github.NewRegistry(translations.NullTranslationHelper).Build().DefaultToolsetIDs()
182182
}
183183

184184
ghServer, err := ghmcp.NewMCPServer(ghmcp.MCPServerConfig{

internal/ghmcp/server.go

Lines changed: 102 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
"github.com/github/github-mcp-server/pkg/lockdown"
1919
mcplog "github.com/github/github-mcp-server/pkg/log"
2020
"github.com/github/github-mcp-server/pkg/raw"
21-
"github.com/github/github-mcp-server/pkg/toolsets"
21+
"github.com/github/github-mcp-server/pkg/registry"
2222
"github.com/github/github-mcp-server/pkg/translations"
2323
gogithub "github.com/google/go-github/v79/github"
2424
"github.com/modelcontextprotocol/go-sdk/mcp"
@@ -69,146 +69,153 @@ type MCPServerConfig struct {
6969
RepoAccessTTL *time.Duration
7070
}
7171

72-
func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
73-
apiHost, err := parseAPIHost(cfg.Host)
74-
if err != nil {
75-
return nil, fmt.Errorf("failed to parse API host: %w", err)
76-
}
72+
// githubClients holds all the GitHub API clients created for a server instance.
73+
type githubClients struct {
74+
rest *gogithub.Client
75+
gql *githubv4.Client
76+
gqlHTTP *http.Client // retained for middleware to modify transport
77+
raw *raw.Client
78+
repoAccess *lockdown.RepoAccessCache
79+
}
7780

78-
// Construct our REST client
81+
// createGitHubClients creates all the GitHub API clients needed by the server.
82+
func createGitHubClients(cfg MCPServerConfig, apiHost apiHost) (*githubClients, error) {
83+
// Construct REST client
7984
restClient := gogithub.NewClient(nil).WithAuthToken(cfg.Token)
8085
restClient.UserAgent = fmt.Sprintf("github-mcp-server/%s", cfg.Version)
8186
restClient.BaseURL = apiHost.baseRESTURL
8287
restClient.UploadURL = apiHost.uploadURL
8388

84-
// Construct our GraphQL client
85-
// We're using NewEnterpriseClient here unconditionally as opposed to NewClient because we already
86-
// did the necessary API host parsing so that github.com will return the correct URL anyway.
89+
// Construct GraphQL client
90+
// We use NewEnterpriseClient unconditionally since we already parsed the API host
8791
gqlHTTPClient := &http.Client{
8892
Transport: &bearerAuthTransport{
8993
transport: http.DefaultTransport,
9094
token: cfg.Token,
9195
},
92-
} // We're going to wrap the Transport later in beforeInit
93-
gqlClient := githubv4.NewEnterpriseClient(apiHost.graphqlURL.String(), gqlHTTPClient)
94-
repoAccessOpts := []lockdown.RepoAccessOption{}
95-
if cfg.RepoAccessTTL != nil {
96-
repoAccessOpts = append(repoAccessOpts, lockdown.WithTTL(*cfg.RepoAccessTTL))
9796
}
97+
gqlClient := githubv4.NewEnterpriseClient(apiHost.graphqlURL.String(), gqlHTTPClient)
9898

99-
repoAccessLogger := cfg.Logger.With("component", "lockdown")
100-
repoAccessOpts = append(repoAccessOpts, lockdown.WithLogger(repoAccessLogger))
99+
// Create raw content client (shares REST client's HTTP transport)
100+
rawClient := raw.NewClient(restClient, apiHost.rawURL)
101+
102+
// Set up repo access cache for lockdown mode
101103
var repoAccessCache *lockdown.RepoAccessCache
102104
if cfg.LockdownMode {
103-
repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
105+
opts := []lockdown.RepoAccessOption{
106+
lockdown.WithLogger(cfg.Logger.With("component", "lockdown")),
107+
}
108+
if cfg.RepoAccessTTL != nil {
109+
opts = append(opts, lockdown.WithTTL(*cfg.RepoAccessTTL))
110+
}
111+
repoAccessCache = lockdown.GetInstance(gqlClient, opts...)
104112
}
105113

106-
// Determine enabled toolsets based on configuration:
107-
// - nil means "use defaults" (unless dynamic mode without explicit toolsets)
108-
// - empty slice means "no toolsets" (for dynamic mode to enable on demand)
109-
// - explicit list means "use these toolsets"
110-
var enabledToolsets []string
114+
return &githubClients{
115+
rest: restClient,
116+
gql: gqlClient,
117+
gqlHTTP: gqlHTTPClient,
118+
raw: rawClient,
119+
repoAccess: repoAccessCache,
120+
}, nil
121+
}
122+
123+
// resolveEnabledToolsets determines which toolsets should be enabled based on config.
124+
// Returns nil for "use defaults", empty slice for "none", or explicit list.
125+
func resolveEnabledToolsets(cfg MCPServerConfig) []string {
111126
if cfg.EnabledToolsets != nil {
112-
enabledToolsets = cfg.EnabledToolsets
113-
} else if cfg.DynamicToolsets {
114-
// Dynamic mode with no toolsets specified: start with no toolsets enabled
115-
// so users can enable them on demand via the dynamic tools
116-
enabledToolsets = []string{}
127+
return cfg.EnabledToolsets
117128
}
118-
// else: enabledToolsets stays nil, which means "use defaults" in WithToolsets
119-
120-
// Generate instructions based on enabled toolsets
121-
instructions := github.GenerateInstructions(enabledToolsets)
122-
123-
getClient := func(_ context.Context) (*gogithub.Client, error) {
124-
return restClient, nil // closing over client
129+
if cfg.DynamicToolsets {
130+
// Dynamic mode with no toolsets specified: start empty so users enable on demand
131+
return []string{}
125132
}
133+
// nil means "use defaults" in WithToolsets
134+
return nil
135+
}
126136

127-
getGQLClient := func(_ context.Context) (*githubv4.Client, error) {
128-
return gqlClient, nil // closing over client
137+
func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
138+
apiHost, err := parseAPIHost(cfg.Host)
139+
if err != nil {
140+
return nil, fmt.Errorf("failed to parse API host: %w", err)
129141
}
130142

131-
getRawClient := func(ctx context.Context) (*raw.Client, error) {
132-
client, err := getClient(ctx)
133-
if err != nil {
134-
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
135-
}
136-
return raw.NewClient(client, apiHost.rawURL), nil // closing over client
143+
clients, err := createGitHubClients(cfg, apiHost)
144+
if err != nil {
145+
return nil, fmt.Errorf("failed to create GitHub clients: %w", err)
137146
}
138147

148+
enabledToolsets := resolveEnabledToolsets(cfg)
149+
150+
// Create the MCP server
139151
ghServer := github.NewServer(cfg.Version, &mcp.ServerOptions{
140-
Instructions: instructions,
141-
Logger: cfg.Logger,
142-
CompletionHandler: github.CompletionsHandler(getClient),
152+
Instructions: github.GenerateInstructions(enabledToolsets),
153+
Logger: cfg.Logger,
154+
CompletionHandler: github.CompletionsHandler(func(_ context.Context) (*gogithub.Client, error) {
155+
return clients.rest, nil
156+
}),
143157
})
144158

145159
// Add middlewares
146160
ghServer.AddReceivingMiddleware(addGitHubAPIErrorToContext)
147-
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, restClient, gqlHTTPClient))
148-
149-
// Create the dependencies struct for tool handlers
150-
deps := github.ToolDependencies{
151-
GetClient: getClient,
152-
GetGQLClient: getGQLClient,
153-
GetRawClient: getRawClient,
154-
RepoAccessCache: repoAccessCache,
155-
T: cfg.Translator,
156-
Flags: github.FeatureFlags{LockdownMode: cfg.LockdownMode},
157-
ContentWindowSize: cfg.ContentWindowSize,
158-
}
159-
160-
// Create toolset group with all tools, resources, and prompts (stateless)
161-
r := github.NewRegistry(cfg.Translator)
162-
163-
// Clean tool names (WithTools will resolve any deprecated aliases)
164-
enabledTools := github.CleanTools(cfg.EnabledTools)
165-
166-
// Apply filters based on configuration
167-
// - WithDeprecatedToolAliases: adds backward compatibility aliases
168-
// - WithReadOnly: filters out write tools when true
169-
// - WithToolsets: nil=defaults, empty=none, handles "all"/"default" keywords
170-
// - WithTools: additional tools that bypass toolset filtering (additive, resolves aliases)
171-
// - WithFeatureChecker: filters based on feature flags
172-
filteredReg := r.
173-
WithDeprecatedToolAliases(github.DeprecatedToolAliases).
161+
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, clients.rest, clients.gqlHTTP))
162+
163+
// Create dependencies for tool handlers
164+
deps := github.NewBaseDeps(
165+
clients.rest,
166+
clients.gql,
167+
clients.raw,
168+
clients.repoAccess,
169+
cfg.Translator,
170+
github.FeatureFlags{LockdownMode: cfg.LockdownMode},
171+
cfg.ContentWindowSize,
172+
)
173+
174+
// Build and register the tool/resource/prompt registry
175+
registry := github.NewRegistry(cfg.Translator).
176+
WithDeprecatedAliases(github.DeprecatedToolAliases).
174177
WithReadOnly(cfg.ReadOnly).
175178
WithToolsets(enabledToolsets).
176-
WithTools(enabledTools).
177-
WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures))
179+
WithTools(github.CleanTools(cfg.EnabledTools)).
180+
WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures)).
181+
Build()
178182

179-
// Warn about unrecognized toolset names (likely typos)
180-
if unrecognized := filteredReg.UnrecognizedToolsets(); len(unrecognized) > 0 {
183+
if unrecognized := registry.UnrecognizedToolsets(); len(unrecognized) > 0 {
181184
fmt.Fprintf(os.Stderr, "Warning: unrecognized toolsets ignored: %s\n", strings.Join(unrecognized, ", "))
182185
}
183186

184-
// Register all mcp functionality with the server
185-
// Use background context for local server (no per-request actor context)
186-
filteredReg.RegisterAll(context.Background(), ghServer, deps)
187+
// Register GitHub tools/resources/prompts from the registry.
188+
// In dynamic mode with no explicit toolsets, this is a no-op since enabledToolsets
189+
// is empty - users enable toolsets at runtime via the dynamic tools below (but can
190+
// enable toolsets or tools explicitly that do need registration).
191+
registry.RegisterAll(context.Background(), ghServer, deps)
187192

188-
// Register dynamic toolset management if configured
189-
// Dynamic tools get access to the filtered toolset group which tracks enabled state.
190-
// ToolsForToolset() returns all tools for a toolset regardless of enabled status,
191-
// so dynamic tools can enable any toolset at runtime.
193+
// Register dynamic toolset management tools (enable/disable) - these are separate
194+
// meta-tools that control the registry, not part of the registry itself
192195
if cfg.DynamicToolsets {
193-
dynamicDeps := github.DynamicToolDependencies{
194-
Server: ghServer,
195-
Registry: filteredReg,
196-
ToolDeps: deps,
197-
T: cfg.Translator,
198-
}
199-
dynamicTools := github.DynamicTools(filteredReg)
200-
for _, tool := range dynamicTools {
201-
tool.RegisterFunc(ghServer, dynamicDeps)
202-
}
196+
registerDynamicTools(ghServer, registry, deps, cfg.Translator)
203197
}
204198

205199
return ghServer, nil
206200
}
207201

202+
// registerDynamicTools adds the dynamic toolset enable/disable tools to the server.
203+
func registerDynamicTools(server *mcp.Server, registry *registry.Registry, deps *github.BaseDeps, t translations.TranslationHelperFunc) {
204+
dynamicDeps := github.DynamicToolDependencies{
205+
Server: server,
206+
Registry: registry,
207+
ToolDeps: deps,
208+
T: t,
209+
}
210+
for _, tool := range github.DynamicTools(registry) {
211+
tool.RegisterFunc(server, dynamicDeps)
212+
}
213+
}
214+
208215
// createFeatureChecker returns a FeatureFlagChecker that checks if a flag name
209216
// is present in the provided list of enabled features. For the local server,
210217
// this is populated from the --features CLI flag.
211-
func createFeatureChecker(enabledFeatures []string) toolsets.FeatureFlagChecker {
218+
func createFeatureChecker(enabledFeatures []string) registry.FeatureFlagChecker {
212219
// Build a set for O(1) lookup
213220
featureSet := make(map[string]bool, len(enabledFeatures))
214221
for _, f := range enabledFeatures {

0 commit comments

Comments
 (0)