Skip to content

Commit 78c89a3

Browse files
Fix default toolsets behavior when not in dynamic mode
When no toolsets are specified and dynamic mode is disabled, the server should use the default toolsets. The bug was introduced when adding dynamic toolsets support: 1. CleanToolsets(nil) was converting nil to empty slice 2. Empty slice passed to WithToolsets means 'no toolsets' 3. This resulted in zero tools being registered Fix: Preserve nil for non-dynamic mode (nil = use defaults in WithToolsets) and only set empty slice when dynamic mode is enabled without explicit toolsets.
1 parent 7a78ec6 commit 78c89a3

File tree

1 file changed

+17
-15
lines changed

1 file changed

+17
-15
lines changed

internal/ghmcp/server.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,24 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
103103
repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
104104
}
105105

106-
enabledToolsets := cfg.EnabledToolsets
107-
108-
// Clean up the passed toolsets (removes duplicates, whitespace)
109-
enabledToolsets, invalidToolsets := github.CleanToolsets(enabledToolsets)
110-
111-
if len(invalidToolsets) > 0 {
112-
fmt.Fprintf(os.Stderr, "Invalid toolsets ignored: %s\n", strings.Join(invalidToolsets, ", "))
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
111+
if cfg.EnabledToolsets != nil {
112+
// Clean up explicitly passed toolsets (removes duplicates, whitespace)
113+
var invalidToolsets []string
114+
enabledToolsets, invalidToolsets = github.CleanToolsets(cfg.EnabledToolsets)
115+
if len(invalidToolsets) > 0 {
116+
fmt.Fprintf(os.Stderr, "Invalid toolsets ignored: %s\n", strings.Join(invalidToolsets, ", "))
117+
}
118+
} else if cfg.DynamicToolsets {
119+
// Dynamic mode with no toolsets specified: start with no toolsets enabled
120+
// so users can enable them on demand via the dynamic tools
121+
enabledToolsets = []string{}
113122
}
123+
// else: enabledToolsets stays nil, which means "use defaults" in WithToolsets
114124

115125
// Generate instructions based on enabled toolsets
116126
instructions := github.GenerateInstructions(enabledToolsets)
@@ -162,14 +172,6 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
162172
// Clean tool names (WithTools will resolve any deprecated aliases)
163173
enabledTools := github.CleanTools(cfg.EnabledTools)
164174

165-
// For dynamic toolsets mode:
166-
// - If toolsets are explicitly provided (including "default"), honor them
167-
// - If no toolsets are specified (nil), start with no toolsets enabled (empty slice)
168-
// so users can enable them on demand via the dynamic tools
169-
if cfg.DynamicToolsets && cfg.EnabledToolsets == nil {
170-
enabledToolsets = []string{}
171-
}
172-
173175
// Apply filters based on configuration
174176
// - WithReadOnly: filters out write tools when true
175177
// - WithToolsets: nil=defaults, empty=none, handles "all"/"default" keywords

0 commit comments

Comments
 (0)